Re: [Qemu-devel] [PATCH] kvm: apic: save and restore x2APIC LDR

From: "Radim Krčmář" <rkrcmar@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: qemu-devel@nongnu.org, Eduardo Habkost <ehabkost@redhat.com>,
	"Dr . David Alan Gilbert" <dgilbert@redhat.com>,
	Richard Henderson <rth@twiddle.net>
Subject: Re: [Qemu-devel] [PATCH] kvm: apic: save and restore x2APIC LDR
Date: Wed, 22 Nov 2017 21:57:37 +0100	[thread overview]
Message-ID: <20171122205736.GA21184@flask> (raw)
In-Reply-To: <adada925-d23d-af1c-8965-f18904f5184d@redhat.com>

2017-11-22 20:26+0100, Paolo Bonzini:
> On 22/11/2017 19:09, Radim Krčmář wrote:
> > QEMU saves only 8 bits of APIC LDR, which means that it does not support
> > x2APIC.  The correct way of fixing this would be to save and restore the
> > full 32 bit register, but because x2APIC LDR is a function of x2APIC ID,
> > we can also compute it and keep the migration format untouched.
> > 
> > KVM always expected the LDR format to follow the xAPIC/x2APIC standard,
> > but pre 4.1 KVMs used non-standard x2APIC ID in case the OS changed
> > xAPIC ID before switching to x2APIC, which means that QEMU has to use
> > the kvm_x2apic_api feature to derive the x2APIC ID.
> > 
> > This bug has also been addressed on the KVM side with patch 5849d75a5c9b
> > ("KVM: lapic: Fixup LDR on load in x2apic").
> 
> > +    if (s->apicbase & MSR_IA32_APICBASE_EXTD) {
> > +        kvm_apic_set_reg(kapic, 0xd, kvm_apic_calc_x2apic_ldr(s));
> 
> Is this correct if the kernel doesn't support the new-style x2APIC API?

Should be: QEMU will use the APIC_ID register in that case, which
contains the x2APIC ID that KVM used to compute the LDR from.

(old-style APIC_ID just cannot store more than 8 bits and isn't tied to
 vcpu_id.)

> In the end, it seems simpler to just fix it in the kernel.

We already have the workaround in KVM, so dropping this one doesn't make
that much of a difference.  I perceive it as solely QEMU bug, though. :)