[Xen-devel] [PATCH] x86/apic: enable x2APIC mode before doing any setup

Roger Pau Monne posted 1 patch 18 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/xen tags/patchew/20190806131346.50881-1-roger.pau@citrix.com
xen/arch/x86/apic.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

[Xen-devel] [PATCH] x86/apic: enable x2APIC mode before doing any setup

Posted by Roger Pau Monne 18 weeks ago
Current code calls apic_x2apic_probe which does some initialization
and setup before having enabled x2APIC mode (if it's not already
enabled by the firmware).

This can lead to issues if the APIC ID doesn't match the x2APIC ID, as
apic_x2apic_probe calls init_apic_ldr_x2apic_cluster which depending
on the APIC mode might set cpu_2_logical_apicid using the APIC ID
instead of the x2APIC ID (because x2APIC might not be enabled yet).

Fix this by enabling x2APIC before calling apic_x2apic_probe.

As a remark, this was discovered while I was trying to figure out why
one of my test boxes didn't report any iommu faults. The root cause
was that the iommu MSI address field was set using the stale value in
cpu_2_logical_apicid, and thus the iommu fault interrupt would get
lost. Even if the MSI address field gets sets to a correct value
afterwards as soon as a single iommu fault is pending no further
interrupts would get injected, so losing a single iommu fault
interrupt is fatal.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Wei Liu <wl@xen.org>
---
 xen/arch/x86/apic.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c
index 9c3c998d34..bd69299a27 100644
--- a/xen/arch/x86/apic.c
+++ b/xen/arch/x86/apic.c
@@ -952,17 +952,17 @@ void __init x2apic_bsp_setup(void)
 
     force_iommu = 1;
 
-    orig_name = genapic.name;
-    genapic = *apic_x2apic_probe();
-    if ( genapic.name != orig_name )
-        printk("Switched to APIC driver %s\n", genapic.name);
-
     if ( !x2apic_enabled )
     {
         x2apic_enabled = true;
         __enable_x2apic();
     }
 
+    orig_name = genapic.name;
+    genapic = *apic_x2apic_probe();
+    if ( genapic.name != orig_name )
+        printk("Switched to APIC driver %s\n", genapic.name);
+
 restore_out:
     restore_IO_APIC_setup(ioapic_entries);
     unmask_8259A();
-- 
2.20.1 (Apple Git-117)


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] x86/apic: enable x2APIC mode before doing any setup

Posted by Jan Beulich 18 weeks ago
On 06.08.2019 15:13, Roger Pau Monne wrote:
> Current code calls apic_x2apic_probe which does some initialization
> and setup before having enabled x2APIC mode (if it's not already
> enabled by the firmware).
> 
> This can lead to issues if the APIC ID doesn't match the x2APIC ID, as
> apic_x2apic_probe calls init_apic_ldr_x2apic_cluster which depending
> on the APIC mode might set cpu_2_logical_apicid using the APIC ID
> instead of the x2APIC ID (because x2APIC might not be enabled yet).
> 
> Fix this by enabling x2APIC before calling apic_x2apic_probe.
> 
> As a remark, this was discovered while I was trying to figure out why
> one of my test boxes didn't report any iommu faults. The root cause
> was that the iommu MSI address field was set using the stale value in
> cpu_2_logical_apicid, and thus the iommu fault interrupt would get
> lost. Even if the MSI address field gets sets to a correct value
> afterwards as soon as a single iommu fault is pending no further
> interrupts would get injected, so losing a single iommu fault
> interrupt is fatal.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

I'm surprised we got away with this for so long.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel