[PATCH] x86: Disable IOAPIC earlier during shutdown

[PATCH] x86: Disable IOAPIC earlier during shutdown

[Ross Lagerwall]

Commit fc0c3fa2ad5c ("x86/IO-APIC: fix setup of Xen internally used IRQs
(take 2)") introduced a regression on some hardware where Xen would hang
during shutdown, repeating the following message:
APIC error on CPU0: 08(08), Receive accept error

This appears to be because an interrupt (in this case from the serial
console) destined for a CPU other than the boot CPU is left unhandled so
an APIC error on CPU 0 is generated instead.

To fix this, disable the IOAPIC before each CPU's local APIC is
disabled so that these interrupts are not generated.

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
 xen/arch/x86/smp.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/smp.c b/xen/arch/x86/smp.c
index 50ff6c2..5dfdc96 100644
--- a/xen/arch/x86/smp.c
+++ b/xen/arch/x86/smp.c
@@ -299,6 +299,10 @@ void smp_send_stop(void)
 {
     int timeout = 10;
 
+    local_irq_disable();
+    disable_IO_APIC();
+    local_irq_enable();
+
     smp_call_function(stop_this_cpu, NULL, 0);
 
     /* Wait 10ms for all other CPUs to go offline. */
@@ -306,7 +310,6 @@ void smp_send_stop(void)
         mdelay(1);
 
     local_irq_disable();
-    disable_IO_APIC();
     hpet_disable();
     __stop_this_cpu();
     local_irq_enable();
-- 
2.4.3

Re: [PATCH] x86: Disable IOAPIC earlier during shutdown

[Jan Beulich]

>>> On 01.12.15 at 17:13, <ross.lagerwall@citrix.com> wrote:
> Commit fc0c3fa2ad5c ("x86/IO-APIC: fix setup of Xen internally used IRQs
> (take 2)") introduced a regression on some hardware where Xen would hang
> during shutdown, repeating the following message:
> APIC error on CPU0: 08(08), Receive accept error
> 
> This appears to be because an interrupt (in this case from the serial
> console) destined for a CPU other than the boot CPU is left unhandled so
> an APIC error on CPU 0 is generated instead.
> 
> To fix this, disable the IOAPIC before each CPU's local APIC is
> disabled so that these interrupts are not generated.

But wouldn't a similar issue occur for MSI or MSI-like (IOMMU)
interrupts? I.e. shouldn't we perhaps invoke fixup_irqs() after
having restricted cpu_online_map to just CPU0?

Jan

Re: [PATCH] x86: Disable IOAPIC earlier during shutdown

[Andrew Cooper]

On 01/12/15 16:26, Jan Beulich wrote:
>>>> On 01.12.15 at 17:13, <ross.lagerwall@citrix.com> wrote:
>> Commit fc0c3fa2ad5c ("x86/IO-APIC: fix setup of Xen internally used IRQs
>> (take 2)") introduced a regression on some hardware where Xen would hang
>> during shutdown, repeating the following message:
>> APIC error on CPU0: 08(08), Receive accept error
>>
>> This appears to be because an interrupt (in this case from the serial
>> console) destined for a CPU other than the boot CPU is left unhandled so
>> an APIC error on CPU 0 is generated instead.
>>
>> To fix this, disable the IOAPIC before each CPU's local APIC is
>> disabled so that these interrupts are not generated.
> But wouldn't a similar issue occur for MSI or MSI-like (IOMMU)
> interrupts? I.e. shouldn't we perhaps invoke fixup_irqs() after
> having restricted cpu_online_map to just CPU0?

a fixup_irq()s in __stop_this_cpu() might do it, although there will be
heavy lock contention on all the irq descriptors.

A better option would be to run fixup_irq()s once and make them all
point to cpu0, then take the others down.  This will probably involve
passing a parameter to fixup_irq()s to conditionally override its use of
the cpu_online_map.

~Andrew

Re: [PATCH] x86: Disable IOAPIC earlier during shutdown

[Jan Beulich]

>>> On 01.12.15 at 17:38, <andrew.cooper3@citrix.com> wrote:
> On 01/12/15 16:26, Jan Beulich wrote:
>>>>> On 01.12.15 at 17:13, <ross.lagerwall@citrix.com> wrote:
>>> Commit fc0c3fa2ad5c ("x86/IO-APIC: fix setup of Xen internally used IRQs
>>> (take 2)") introduced a regression on some hardware where Xen would hang
>>> during shutdown, repeating the following message:
>>> APIC error on CPU0: 08(08), Receive accept error
>>>
>>> This appears to be because an interrupt (in this case from the serial
>>> console) destined for a CPU other than the boot CPU is left unhandled so
>>> an APIC error on CPU 0 is generated instead.
>>>
>>> To fix this, disable the IOAPIC before each CPU's local APIC is
>>> disabled so that these interrupts are not generated.
>> But wouldn't a similar issue occur for MSI or MSI-like (IOMMU)
>> interrupts? I.e. shouldn't we perhaps invoke fixup_irqs() after
>> having restricted cpu_online_map to just CPU0?
> 
> a fixup_irq()s in __stop_this_cpu() might do it, although there will be
> heavy lock contention on all the irq descriptors.
> 
> A better option would be to run fixup_irq()s once and make them all
> point to cpu0, then take the others down.  This will probably involve
> passing a parameter to fixup_irq()s to conditionally override its use of
> the cpu_online_map.

The latter was what I actually had in mind, I just didn't check
whether we can re-write cpu_online_map up front (which it looks
like we can't). So yes, passing fixup_irqs() a cpumask would be
the way to go.

Jan