Re: [PATCH] x86/x2apic: introduce a mixed physical/cluster mode

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 24/11/2023 7:54 pm, Neowutran wrote:
> Hi, 
> I did some more tests and research, indeed this patch improved/solved my 
> specific case. 
>
> Starting point: 
>
> I am using Xen version 4.17.2 (exactly this source 
> https://github.com/QubesOS/qubes-vmm-xen).
> In the bios (a Asus motherboard), I configured the "local apic" parameter to 
> "X2APIC".
> For Xen, I did not set the parameter "x2apic-mode" nor the parameter 
> "x2apic_phys". 
>
> Case 1:
> I tryied to boot just like that, result: system is unusuably slow
>
> Case 2:
> Then, I applied a backport of the patch  
> https://lore.kernel.org/xen-devel/20231106142739.19650-1-roger.pau@xxxxxxxxxx/raw
>  
> to the original Xen version of QubesOS and I recompiled. 
> (https://github.com/neowutran/qubes-vmm-xen/blob/x2apic3/X2APIC.patch)
> Result: it work, the system is usable. 
>
> Case 3:
> Then, I applied the patch 
> https://github.com/xen-project/xen/commit/26a449ce32cef33f2cb50602be19fcc0c4223ba9
> to the original Xen version of QubesOS and I recompiled.
> (https://github.com/neowutran/qubes-vmm-xen/blob/x2apic4/X2APIC.patch)
> Result: system is  
> unusuably slow. 
>
>
> In "Case 2", the value returned by the function "apic_x2apic_probe" is 
> "&apic_x2apic_mixed". 
> In "Case 3", the value returned by the function "apic_x2apic_probe" is 
> "&apic_x2apic_cluster". 
>
>
> -------------------
> If you want / need, details for the function "apic_x2apic_probe":
>
> Known "input" value:
>
> "CONFIG_X2APIC_PHYSICAL" is not defined
> "iommu_intremap == iommu_intremap_off" = false
> "acpi_gbl_FADT.flags & ACPI_FADT_APIC_PHYSICAL" -> 0
> "acpi_gbl_FADT.flags" = 247205 (in decimal)
> "CONFIG_X2APIC_PHYSICAL" is not defined
> "CONFIG_X2APIC_MIXED" is defined, because it is the default choice
> "x2apic_mode" = 0
> "x2apic_phys" = -1
>
>
>
> Trace log (I did some call "printk" to trace what was going on)
> Case 2:
> (XEN) NEOWUTRAN: X2APIC_MODE: 0 
> (XEN) NEOWUTRAN: X2APIC_PHYS: -1 
> (XEN) NEOWUTRAN: acpi_gbl_FADT.flags: 247205 
> (XEN) NEOWUTRAN IOMMU_INTREMAP: different 
> (XEN) Neowutran: PASSE 2 
> (XEN) Neowutran: PASSE 4 
> (XEN) NEOWUTRAN: X2APIC_MODE: 3 
> (XEN) Neowutran: PASSE 7 
> (XEN) NEOWUTRAN: X2APIC_MODE: 3 
>  
> (XEN) NEOWUTRAN: X2APIC_PHYS: -1 
> (XEN) NEOWUTRAN: acpi_gbl_FADT.flags: 247205 
> (XEN) NEOWUTRAN IOMMU_INTREMAP: different 
>
> Case 3:
> (XEN) NEOWUTRAN2: X2APIC_PHYS: -1 
> (XEN) NEOWUTRAN2: acpi_gbl_FADT.flags: 247205 
> (XEN) NEOWUTRAN2 IOMMU_INTREMAP: different 
> (XEN) Neowutran2: Passe 1 
> (XEN) NEOWUTRAN2: X2APIC_PHYS: 0 
> (XEN) Neowutran2: Passe 6 
> (XEN) Neowutran2: Passe 7 
> (XEN) NEOWUTRAN2: X2APIC_PHYS: 0 
> (XEN) NEOWUTRAN2: acpi_gbl_FADT.flags: 247205 
> (XEN) NEOWUTRAN2 IOMMU_INTREMAP: different 
> (XEN) Neowutran2: Passe 2 
> (XEN) Neowutran2: Passe 4 
> (XEN) Neowutran2: Passe 7
>
>
>
> If you require the full logs, I could publish the full logs somewhere.
> ----------------------
>
> ( However I do not understand if the root issue is a buggy motherboard, a
> bug in xen, or if the parameter "X2APIC_PHYSICAL" should have been set
> by the QubesOS project, or something else)

Hello,

Thankyou for the analysis.

For your base version of QubeOS Xen, was that 4.13.2-5 ?   I can't see
any APIC changes in the patchqueue, and I believe all relevant bugfixes
are in 4.17.2, but I'd just like to confirm.

First, by "unusable slow", other than the speed, did everything else
appear to operate adequately?  Any chance you could guess the slowdown. 
i.e. was it half the speed, or "seconds per log console line during
boot" levels of slow?


Having re-reviewed 26a449ce32, the patch is correct but the reasoning is
wrong.

ACPI_FADT_APIC_CLUSTER predates x2APIC by almost a decade (it appeared
in ACPI 3.0), and is not relevant outside of xAPIC mode.  xAPIC has 2
different logical destination modes, cluster and flat, and their
applicability is dependent on whether you have fewer or more than 8
local APICs, hence that property being called out in the ACPI spec.

x2APIC does not have this property.  DFR was removed from the
architecture, and logical mode is strictly cluster.  So the bit should
never have been interpreted on an x2APIC code path.

Not that it matters in your case - the bit isn't set in your FADT, hence
why case 1 and 3 have the same behaviour.


This brings us to case 2, where mixed mode does seem to resolve the perf
problem.

Since that patch was written, I've learnt how cluster delivery mode
works for external interrupts, and Xen should never ever have been using
it (Xen appears to be alone in OS software here).  For an external
interrupt in Logical cluster mode, it always sends to the lowest ID in
the cluster.  If that APIC decides that the local processor is too busy
to handle the interrupt now, it forwards the interrupt to the next APIC
in the cluster, and this cycle continues until one APIC accepts the message.

You get most interrupts hitting the lowest APIC in the cluster, but the
interrupt can be forwarded between APICs for an unbounded quantity of
time depending on system utilisation.


Could you please take case 2 and confirm what happens when booting with
x2apic-mode={physical,cluster}?  If the pattern holds, the physical
should be fine, and cluster should see the same problems as case 1 and 3.

Thanks,

~Andrew