[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/2] kvm: apic: set APIC base as part of kvm_api
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH 2/2] kvm: apic: set APIC base as part of kvm_apic_put |
Date: |
Thu, 22 Sep 2016 16:09:27 +0100 |
User-agent: |
Mutt/1.7.0 (2016-08-17) |
* Paolo Bonzini (address@hidden) wrote:
> From: "Dr. David Alan Gilbert" <address@hidden>
>
> 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 <address@hidden>
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
> 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 / address@hidden / Manchester, UK