Re: [Xen-devel] [PATCH v3] x86/vlapic: Don't reset APIC ID when handling INIT signal

[Chao Gao <chao.gao@xxxxxxxxx>]

On Thu, Apr 20, 2017 at 04:39:06PM +0100, Julien Grall wrote:
>Hi,
>
>On 20/04/17 14:34, Jan Beulich wrote:
>> > > > On 19.04.17 at 22:22, <chao.gao@xxxxxxxxx> wrote:
>> > According to SDM "ADVANCED PROGRAMMABLE INTERRUPT CONTROLLER (APIC) ->
>> > "EXTENDED XAPIC (X2APIC)" -> "x2APIC State Transitions", the APIC mode
>> > and APIC ID are preserved when handling INIT signal and a reset places
>> > APIC to xAPIC mode and APIC base address to 0xFEE00000h (this part
>> > is in "Local APIC" -> "Local APIC Status and Location"). So there are
>> > two problems in current code:
>> > 1. Using reset logic (aka vlapic_reset) to handle INIT signal.
>> > 2. Forgetting resetting APIC mode and base address in vlapic_reset()
>> > 
>> > This patch introduces a new function vlapic_do_init() and replaces the
>> > wrongly used vlapic_reset(). Also reset APIC mode and APIC base address
>> > in vlapic_reset().
>> > 
>> > Note that: LDR is read only in x2APIC mode. Resetting it to zero in x2APIC
>> > mode is unreasonable. This patch also doesn't reset LDR when handling INIT
>> > signal in x2APIC mode.
>> > 
>> > Signed-off-by: Chao Gao <chao.gao@xxxxxxxxx>
>> 
>> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
>> 
>> > I regard this patch as a bug fix. But I haven't seen issues caused by
>> > this bug and am not sure of the existance of such issues. Anyhow Cc
>> > Julien and leave the decision to you (Julien and Jan).
>> 
>> Julien,
>> 
>> I'm slightly in favor of taking it now, but I won't object if you decide
>> otherwise.
>
>Chao, can you assess the benefits/risks of having this patch in Xen 4.9?

I think the risk is low for only fixing SPEC mismatched behavior and
benefit is also small as cases (changing APIC ID or Performance INIT
operation) trigger this bug are rare.

Thanks
Chao

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel