From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50747) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bn5d1-0001G8-T4 for qemu-devel@nongnu.org; Thu, 22 Sep 2016 11:09:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bn5cx-0004lz-QP for qemu-devel@nongnu.org; Thu, 22 Sep 2016 11:09:35 -0400 Received: from mx1.redhat.com ([209.132.183.28]:48864) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bn5cx-0004lu-Gf for qemu-devel@nongnu.org; Thu, 22 Sep 2016 11:09:31 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id CE10EA4532 for ; Thu, 22 Sep 2016 15:09:30 +0000 (UTC) Date: Thu, 22 Sep 2016 16:09:27 +0100 From: "Dr. David Alan Gilbert" Message-ID: <20160922150927.GF2085@work-vm> References: <1474556571-21200-1-git-send-email-pbonzini@redhat.com> <1474556571-21200-3-git-send-email-pbonzini@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1474556571-21200-3-git-send-email-pbonzini@redhat.com> Subject: Re: [Qemu-devel] [PATCH 2/2] kvm: apic: set APIC base as part of kvm_apic_put List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org * Paolo Bonzini (pbonzini@redhat.com) wrote: > From: "Dr. David Alan Gilbert" > > The parsing of KVM_SET_LAPIC's input depends on the current value of the > APIC base MSR---which indeed is stored in APICCommonState---but for historical > reasons APIC base is set through KVM_SET_SREGS together with cr8 (which is > really just the APIC TPR) and the actual "special CPU registers". > > APIC base must now be set before the actual LAPIC registers, so do that > in kvm_apic_put. It will be set again to the same value with KVM_SET_SREGS, > but that's not a big issue. > > This only happens since Linux 4.8, which checks for x2apic mode in > KVM_SET_LAPIC. However it's really a QEMU bug; until the recent > commit 78d6a05 ("x86/lapic: Load LAPIC state at post_load", 2016-09-13) > QEMU was indeed setting APIC base (via KVM_SET_SREGS) before the other > LAPIC registers. I think you've posted the wrong version here; it's not the version I tested and it's got two problems: > > Signed-off-by: Dr. David Alan Gilbert > Signed-off-by: Paolo Bonzini > --- > hw/i386/kvm/apic.c | 2 ++ > target-i386/kvm.c | 9 +++++++++ > target-i386/kvm_i386.h | 2 ++ > 3 files changed, 13 insertions(+) > > diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c > index feb0002..f57fed1 100644 > --- a/hw/i386/kvm/apic.c > +++ b/hw/i386/kvm/apic.c > @@ -15,6 +15,7 @@ > #include "hw/i386/apic_internal.h" > #include "hw/pci/msi.h" > #include "sysemu/kvm.h" > +#include "target-i386/kvm_i386.h" > > static inline void kvm_apic_set_reg(struct kvm_lapic_state *kapic, > int reg_id, uint32_t val) > @@ -130,6 +131,7 @@ static void kvm_apic_put(void *data) > struct kvm_lapic_state kapic; > int ret; > > + kvm_put_apicbase(s->cpu, s->apicbase); > kvm_put_apic_state(s, &kapic); > > ret = kvm_vcpu_ioctl(CPU(s->cpu), KVM_SET_LAPIC, &kapic); > diff --git a/target-i386/kvm.c b/target-i386/kvm.c > index 38609fd..59242e4 100644 > --- a/target-i386/kvm.c > +++ b/target-i386/kvm.c > @@ -1542,6 +1542,15 @@ static int kvm_put_one_msr(X86CPU *cpu, int index, uint64_t value) > return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MSRS, cpu->kvm_msr_buf); > } > > +void kvm_put_apicbase(X86CPU *cpu, uint64_t value) > +{ > + CPUX86State *env = &cpu->env; > + int ret; > + > + ret = kvm_put_one_msr(cpu, MSR_IA32_TSCDEADLINE, value); That's not the MSR you wanted. Dave > + assert(ret == 1); > +} > + > static int kvm_put_tscdeadline_msr(X86CPU *cpu) > { > CPUX86State *env = &cpu->env; > diff --git a/target-i386/kvm_i386.h b/target-i386/kvm_i386.h > index 42b00af..36407e0 100644 > --- a/target-i386/kvm_i386.h > +++ b/target-i386/kvm_i386.h > @@ -41,4 +41,6 @@ int kvm_device_msix_set_vector(KVMState *s, uint32_t dev_id, uint32_t vector, > int kvm_device_msix_assign(KVMState *s, uint32_t dev_id); > int kvm_device_msix_deassign(KVMState *s, uint32_t dev_id); > > +void kvm_put_apicbase(X86CPU *cpu, uint64_t value); > + > #endif > -- > 2.7.4 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK