On 12/02/2015 02:02 PM, Jan Beulich wrote:
On 02.12.15 at 14:46, <ross.lagerw...@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, before taking down the non-boot CPUs, call fixup_irqs()
with a CPU mask of only the boot CPU to reset the IRQ affinities
correctly.
Signed-off-by: Ross Lagerwall <ross.lagerw...@citrix.com>
---
Even though in this case interested people may know, missing info
on changes from previous version here.
+/* CPU(s) have been removed from mask. Re-set irq affinities. */
+void fixup_irqs(const cpumask_t *mask, bool_t verbose)
The comment doesn't match reality. And I wonder whether it
wouldn't be reasonable to imply "verbose" (either from mask
equaling &cpu_online_map, or by introducing SYS_STATE_shutdown
and/or SYS_STATE_reboot).
I considered introducing a new SYS_STATE but decided that a function
parameter was clearer rather than implying it from some other global state.
@@ -2385,16 +2382,27 @@ void fixup_irqs(void)
spin_unlock(&desc->lock);
- if ( break_affinity && set_affinity )
- printk("Broke affinity for irq %i\n", irq);
- else if ( !set_affinity )
- printk("Cannot set affinity for irq %i\n", irq);
+ if ( verbose )
+ {
+ if ( break_affinity && set_affinity )
+ printk("Broke affinity for irq %i\n", irq);
+ else if ( !set_affinity )
+ printk("Cannot set affinity for irq %i\n", irq);
+ }
How about
if ( !verbose )
continue;
limiting churn on code?
OK.
--- a/xen/arch/x86/smp.c
+++ b/xen/arch/x86/smp.c
@@ -286,6 +286,7 @@ void __stop_this_cpu(void)
static void stop_this_cpu(void *dummy)
{
+ fixup_eoi();
__stop_this_cpu();
Is this really needed during shutdown?
Possibly not, but I think it's cleaner to do the same as what is used
for CPU down.
@@ -298,6 +299,13 @@ static void stop_this_cpu(void *dummy)
void smp_send_stop(void)
{
int timeout = 10;
+ cpumask_t online;
+
+ cpumask_clear(&online);
+ cpumask_set_cpu(smp_processor_id(), &online);
That's what we have cpumask_of() for.
OK.
--
Ross Lagerwall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel