[3/3] x86/apic: do not initialize LDR and DFR for bigsmp
diff mbox series

Message ID 35bb2f38-9d50-c12c-1051-7005251685ca@suse.com
State New, archived
Headers show
Series
  • x86: (largely) LAPIC related cleanup
Related show

Commit Message

Jan Beulich Sept. 6, 2019, 2:01 p.m. UTC
Legacy apic init uses bigsmp for smp systems with 8 and more CPUs. The
bigsmp APIC implementation uses physical destination mode, but it
nevertheless initializes LDR and DFR. The LDR even ends up incorrectly with
multiple bit being set.

This does not cause a functional problem because LDR and DFR are ignored
when physical destination mode is active, but it triggered a problem on a
32-bit KVM guest which jumps into a kdump kernel.

The multiple bits set unearthed a bug in the KVM APIC implementation. The
code which creates the logical destination map for VCPUs ignores the
disabled state of the APIC and ends up overwriting an existing valid entry
and as a result, APIC calibration hangs in the guest during kdump
initialization.

Remove the bogus LDR/DFR initialization.

This is not intended to work around the KVM APIC bug. The LDR/DFR
ininitalization is wrong on its own.

Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Bandan Das <bsd@redhat.com>
[Linux commit bae3a8d3308ee69a7dbdf145911b18dfda8ade0d]

Drop init_apic_ldr_x2apic_phys() at the same time.

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

Comments

Andrew Cooper Sept. 6, 2019, 2:03 p.m. UTC | #1
On 06/09/2019 15:01, Jan Beulich wrote:
> Legacy apic init uses bigsmp for smp systems with 8 and more CPUs. The
> bigsmp APIC implementation uses physical destination mode, but it
> nevertheless initializes LDR and DFR. The LDR even ends up incorrectly with
> multiple bit being set.
>
> This does not cause a functional problem because LDR and DFR are ignored
> when physical destination mode is active, but it triggered a problem on a
> 32-bit KVM guest which jumps into a kdump kernel.
>
> The multiple bits set unearthed a bug in the KVM APIC implementation. The
> code which creates the logical destination map for VCPUs ignores the
> disabled state of the APIC and ends up overwriting an existing valid entry
> and as a result, APIC calibration hangs in the guest during kdump
> initialization.
>
> Remove the bogus LDR/DFR initialization.
>
> This is not intended to work around the KVM APIC bug. The LDR/DFR
> ininitalization is wrong on its own.
>
> Suggested-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Bandan Das <bsd@redhat.com>
> [Linux commit bae3a8d3308ee69a7dbdf145911b18dfda8ade0d]
>
> Drop init_apic_ldr_x2apic_phys() at the same time.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

Patch
diff mbox series

--- a/xen/arch/x86/genapic/delivery.c
+++ b/xen/arch/x86/genapic/delivery.c
@@ -40,11 +40,7 @@  unsigned int cpu_mask_to_apicid_flat(con
 
 void init_apic_ldr_phys(void)
 {
-	unsigned long val;
-	apic_write(APIC_DFR, APIC_DFR_FLAT);
-	/* A dummy logical ID should be fine. We only deliver in phys mode. */
-	val = apic_read(APIC_LDR) & ~APIC_LDR_MASK;
-	apic_write(APIC_LDR, val);
+	/* We only deliver in phys mode - no setup needed. */
 }
 
 void __init clustered_apic_check_phys(void)
--- a/xen/arch/x86/genapic/x2apic.c
+++ b/xen/arch/x86/genapic/x2apic.c
@@ -38,10 +38,6 @@  static inline u32 x2apic_cluster(unsigne
     return per_cpu(cpu_2_logical_apicid, cpu) >> 16;
 }
 
-static void init_apic_ldr_x2apic_phys(void)
-{
-}
-
 static void init_apic_ldr_x2apic_cluster(void)
 {
     unsigned int cpu, this_cpu = smp_processor_id();
@@ -167,7 +163,7 @@  static const struct genapic __initconstr
     APIC_INIT("x2apic_phys", NULL),
     .int_delivery_mode = dest_Fixed,
     .int_dest_mode = 0 /* physical delivery */,
-    .init_apic_ldr = init_apic_ldr_x2apic_phys,
+    .init_apic_ldr = init_apic_ldr_phys,
     .clustered_apic_check = clustered_apic_check_x2apic,
     .vector_allocation_cpumask = vector_allocation_cpumask_phys,
     .cpu_mask_to_apicid = cpu_mask_to_apicid_phys,