Re: [Xen-devel] [PATCH 1/8] x86/smp: reset x2apic_enabled in smp_send_stop()

From: "Roger Pau Monné" <roger.pau@citrix.com>
To: David Woodhouse <dwmw2@infradead.org>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Julien Grall <julien@xen.org>, Wei Liu <wl@xen.org>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Varad Gautam <vrd@amazon.de>,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	Hongyan Xia <hongyxia@amazon.com>,
	xen-devel@lists.xenproject.org,
	Paul Durrant <pdurrant@amazon.co.uk>
Subject: Re: [Xen-devel] [PATCH 1/8] x86/smp: reset x2apic_enabled in smp_send_stop()
Date: Mon, 3 Feb 2020 17:18:29 +0100	[thread overview]
Message-ID: <20200203161829.GC4679@Air-de-Roger> (raw)
In-Reply-To: <20200201003303.2363081-1-dwmw2@infradead.org>

On Sat, Feb 01, 2020 at 12:32:56AM +0000, David Woodhouse wrote:
> From: David Woodhouse <dwmw@amazon.co.uk>
> 
> Just before smp_send_stop() re-enables interrupts when shutting down
> for reboot or kexec, it calls __stop_this_cpu() which in turn calls
> disable_local_APIC(), which puts the APIC back in to the mode Xen found
> it in at boot.
> 
> If that means turning x2APIC off and going back into xAPIC mode, then
> a timer interrupt occurring just after interrupts come back on will
> lead to a GP# when apic_timer_interrupt() attempts to ack the IRQ
> through the EOI register in x2APIC MSR 0x80b:
> 
> (XEN) Executing kexec image on cpu0
> (XEN) ----[ Xen-4.14-unstable  x86_64  debug=n   Not tainted ]----
> (XEN) CPU:    0
> (XEN) RIP:    e008:[<ffff82d08026c139>] apic_timer_interrupt+0x29/0x40
> (XEN) RFLAGS: 0000000000010046   CONTEXT: hypervisor
> (XEN) rax: 0000000000000000   rbx: 00000000000000fa   rcx: 000000000000080b
> …
> (XEN) Xen code around <ffff82d08026c139> (apic_timer_interrupt+0x29/0x40):
> (XEN)  c0 b9 0b 08 00 00 89 c2 <0f> 30 31 ff e9 0e c9 fb ff 0f 1f 40 00 66 2e 0f
> …
> (XEN) Xen call trace:
> (XEN)    [<ffff82d08026c139>] R apic_timer_interrupt+0x29/0x40
> (XEN)    [<ffff82d080283825>] S do_IRQ+0x95/0x750
> …
> (XEN)    [<ffff82d0802a0ad2>] S smp_send_stop+0x42/0xd0
> 
> We can't clear the global x2apic_enabled variable in disable_local_APIC()
> itself because that runs on each CPU. Instead, correct it (by using
> current_local_apic_mode()) in smp_send_stop() while interrupts are still
> disabled immediately after calling __stop_this_cpu() for the boot CPU,
> after all other CPUs have been stopped.
> 
> cf: d639bdd9bbe ("x86/apic: Disable the LAPIC later in smp_send_stop()")
>     ... which didn't quite fix it completely.
> 
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks!

> ---
>  xen/arch/x86/smp.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/xen/arch/x86/smp.c b/xen/arch/x86/smp.c
> index 65eb7cbda8..fac295fa6f 100644
> --- a/xen/arch/x86/smp.c
> +++ b/xen/arch/x86/smp.c
> @@ -354,6 +354,7 @@ void smp_send_stop(void)
>          disable_IO_APIC();
>          hpet_disable();
>          __stop_this_cpu();
> +        x2apic_enabled = (current_local_apic_mode() == APIC_MODE_X2APIC);

You could do this only when kexecing, but it's safe to do
unconditionally, and might be helpful if we also decide to play with
the lapic mode even when not kexecing.

Roger.

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