* [PATCH v2 0/4] KVM: x86: Reset fixes
@ 2015-04-02 0:10 Nadav Amit
2015-04-02 0:10 ` [PATCH v2 1/4] KVM: x86: INIT and reset sequences are different Nadav Amit
` (4 more replies)
0 siblings, 5 replies; 15+ messages in thread
From: Nadav Amit @ 2015-04-02 0:10 UTC (permalink / raw)
To: pbonzini; +Cc: kvm, Nadav Amit
This set includes 2 previous patches that deal with the INIT flow that is not
distinguished from regular boot, and allowing the VM to change BSP (which is
used in very certain testing environments). The next 2 patches are new, dealing
with regression that cause DR0-DR3 not to be reset (even when QEMU initiates
the RESET) and CR2 not cleared after INIT.
The second patch regarding BSP requires an additional fix for QEMU, as
otherwise reset fails. A separate patch was submitted to QEMU mailing-list.
Thanks for reviewing the patches.
Nadav Amit (4):
KVM: x86: INIT and reset sequences are different
KVM: x86: BSP in MSR_IA32_APICBASE is writable
KVM: x86: DR0-DR3 are not clear on reset
KVM: x86: Clear CR2 on VCPU reset
arch/x86/include/asm/kvm_host.h | 7 ++++---
arch/x86/kvm/lapic.c | 13 ++++++-------
arch/x86/kvm/lapic.h | 2 +-
arch/x86/kvm/svm.c | 4 ++--
arch/x86/kvm/vmx.c | 30 +++++++++++++++++-------------
arch/x86/kvm/x86.c | 35 +++++++++++++++++++++++++++--------
include/linux/kvm_host.h | 7 ++++++-
7 files changed, 63 insertions(+), 35 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH v2 1/4] KVM: x86: INIT and reset sequences are different 2015-04-02 0:10 [PATCH v2 0/4] KVM: x86: Reset fixes Nadav Amit @ 2015-04-02 0:10 ` Nadav Amit 2015-04-02 2:17 ` Bandan Das ` (2 more replies) 2015-04-02 0:10 ` [PATCH v2 2/4] KVM: x86: BSP in MSR_IA32_APICBASE is writable Nadav Amit ` (3 subsequent siblings) 4 siblings, 3 replies; 15+ messages in thread From: Nadav Amit @ 2015-04-02 0:10 UTC (permalink / raw) To: pbonzini; +Cc: kvm, Nadav Amit x86 architecture defines differences between the reset and INIT sequences. INIT does not initialize the FPU (including MMX, XMM, YMM, etc.), TSC, PMU, MSRs (in general), MTRRs machine-check, APIC ID, APIC arbitration ID and BSP. EFER is supposed NOT to be reset according to the SDM, but leaving the LMA and LME untouched causes failed VM-entry. Therefore we reset EFER (although it is unclear whether the rest of EFER bits should be reset). References (from Intel SDM): "If the MP protocol has completed and a BSP is chosen, subsequent INITs (either to a specific processor or system wide) do not cause the MP protocol to be repeated." [8.4.2: MP Initialization Protocol Requirements and Restrictions] [Table 9-1. IA-32 Processor States Following Power-up, Reset, or INIT] "If the processor is reset by asserting the INIT# pin, the x87 FPU state is not changed." [9.2: X87 FPU INITIALIZATION] "The state of the local APIC following an INIT reset is the same as it is after a power-up or hardware reset, except that the APIC ID and arbitration ID registers are not affected." [10.4.7.3: Local APIC State After an INIT Reset (“Wait-for-SIPI” State)] Signed-off-by: Nadav Amit <namit@cs.technion.ac.il> --- arch/x86/include/asm/kvm_host.h | 6 +++--- arch/x86/kvm/lapic.c | 11 ++++++----- arch/x86/kvm/lapic.h | 2 +- arch/x86/kvm/svm.c | 2 +- arch/x86/kvm/vmx.c | 30 +++++++++++++++++------------- arch/x86/kvm/x86.c | 17 ++++++++++------- 6 files changed, 38 insertions(+), 30 deletions(-) diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index bf5a160..59f4374 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -701,7 +701,7 @@ struct kvm_x86_ops { /* Create, but do not attach this VCPU */ struct kvm_vcpu *(*vcpu_create)(struct kvm *kvm, unsigned id); void (*vcpu_free)(struct kvm_vcpu *vcpu); - void (*vcpu_reset)(struct kvm_vcpu *vcpu); + void (*vcpu_reset)(struct kvm_vcpu *vcpu, bool init_event); void (*prepare_guest_switch)(struct kvm_vcpu *vcpu); void (*vcpu_load)(struct kvm_vcpu *vcpu, int cpu); @@ -989,7 +989,7 @@ void kvm_pic_clear_all(struct kvm_pic *pic, int irq_source_id); void kvm_inject_nmi(struct kvm_vcpu *vcpu); -int fx_init(struct kvm_vcpu *vcpu); +int fx_init(struct kvm_vcpu *vcpu, bool init_event); void kvm_mmu_pte_write(struct kvm_vcpu *vcpu, gpa_t gpa, const u8 *new, int bytes); @@ -1134,7 +1134,7 @@ int kvm_cpu_has_injectable_intr(struct kvm_vcpu *v); int kvm_cpu_has_interrupt(struct kvm_vcpu *vcpu); int kvm_arch_interrupt_allowed(struct kvm_vcpu *vcpu); int kvm_cpu_get_interrupt(struct kvm_vcpu *v); -void kvm_vcpu_reset(struct kvm_vcpu *vcpu); +void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event); void kvm_vcpu_reload_apic_access_page(struct kvm_vcpu *vcpu); void kvm_arch_mmu_notifier_invalidate_page(struct kvm *kvm, unsigned long address); diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index bd4e34d..17da6fc 100644 --- a/arch/x86/kvm/lapic.c +++ b/arch/x86/kvm/lapic.c @@ -1534,7 +1534,7 @@ void kvm_lapic_set_base(struct kvm_vcpu *vcpu, u64 value) } -void kvm_lapic_reset(struct kvm_vcpu *vcpu) +void kvm_lapic_reset(struct kvm_vcpu *vcpu, bool init_event) { struct kvm_lapic *apic; int i; @@ -1548,7 +1548,8 @@ void kvm_lapic_reset(struct kvm_vcpu *vcpu) /* Stop the timer in case it's a reset to an active apic */ hrtimer_cancel(&apic->lapic_timer.timer); - kvm_apic_set_id(apic, vcpu->vcpu_id); + if (!init_event) + kvm_apic_set_id(apic, vcpu->vcpu_id); kvm_apic_set_version(apic->vcpu); for (i = 0; i < APIC_LVT_NUM; i++) @@ -1689,7 +1690,7 @@ int kvm_create_lapic(struct kvm_vcpu *vcpu) APIC_DEFAULT_PHYS_BASE | MSR_IA32_APICBASE_ENABLE); static_key_slow_inc(&apic_sw_disabled.key); /* sw disabled at reset */ - kvm_lapic_reset(vcpu); + kvm_lapic_reset(vcpu, false); kvm_iodevice_init(&apic->dev, &apic_mmio_ops); return 0; @@ -2023,8 +2024,8 @@ void kvm_apic_accept_events(struct kvm_vcpu *vcpu) pe = xchg(&apic->pending_events, 0); if (test_bit(KVM_APIC_INIT, &pe)) { - kvm_lapic_reset(vcpu); - kvm_vcpu_reset(vcpu); + kvm_lapic_reset(vcpu, true); + kvm_vcpu_reset(vcpu, true); if (kvm_vcpu_is_bsp(apic->vcpu)) vcpu->arch.mp_state = KVM_MP_STATE_RUNNABLE; else diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h index 0bc6c65..e4c82dc 100644 --- a/arch/x86/kvm/lapic.h +++ b/arch/x86/kvm/lapic.h @@ -48,7 +48,7 @@ int kvm_apic_has_interrupt(struct kvm_vcpu *vcpu); int kvm_apic_accept_pic_intr(struct kvm_vcpu *vcpu); int kvm_get_apic_interrupt(struct kvm_vcpu *vcpu); void kvm_apic_accept_events(struct kvm_vcpu *vcpu); -void kvm_lapic_reset(struct kvm_vcpu *vcpu); +void kvm_lapic_reset(struct kvm_vcpu *vcpu, bool init_event); u64 kvm_lapic_get_cr8(struct kvm_vcpu *vcpu); void kvm_lapic_set_tpr(struct kvm_vcpu *vcpu, unsigned long cr8); void kvm_lapic_set_eoi(struct kvm_vcpu *vcpu); diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index 155534c..1ef4c0d 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -1195,7 +1195,7 @@ static void init_vmcb(struct vcpu_svm *svm) enable_gif(svm); } -static void svm_vcpu_reset(struct kvm_vcpu *vcpu) +static void svm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event) { struct vcpu_svm *svm = to_svm(vcpu); u32 dummy; diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index fdd9f8b..e9c94c6 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -4694,7 +4694,7 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx) return 0; } -static void vmx_vcpu_reset(struct kvm_vcpu *vcpu) +static void vmx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event) { struct vcpu_vmx *vmx = to_vmx(vcpu); struct msr_data apic_base_msr; @@ -4705,11 +4705,15 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu) vmx->vcpu.arch.regs[VCPU_REGS_RDX] = get_rdx_init_val(); kvm_set_cr8(&vmx->vcpu, 0); - apic_base_msr.data = APIC_DEFAULT_PHYS_BASE | MSR_IA32_APICBASE_ENABLE; - if (kvm_vcpu_is_bsp(&vmx->vcpu)) - apic_base_msr.data |= MSR_IA32_APICBASE_BSP; - apic_base_msr.host_initiated = true; - kvm_set_apic_base(&vmx->vcpu, &apic_base_msr); + + if (!init_event) { + apic_base_msr.data = APIC_DEFAULT_PHYS_BASE | + MSR_IA32_APICBASE_ENABLE; + if (kvm_vcpu_is_bsp(&vmx->vcpu)) + apic_base_msr.data |= MSR_IA32_APICBASE_BSP; + apic_base_msr.host_initiated = true; + kvm_set_apic_base(&vmx->vcpu, &apic_base_msr); + } vmx_segment_cache_clear(vmx); @@ -4733,9 +4737,12 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu) vmcs_write32(GUEST_LDTR_LIMIT, 0xffff); vmcs_write32(GUEST_LDTR_AR_BYTES, 0x00082); - vmcs_write32(GUEST_SYSENTER_CS, 0); - vmcs_writel(GUEST_SYSENTER_ESP, 0); - vmcs_writel(GUEST_SYSENTER_EIP, 0); + if (!init_event) { + vmcs_write32(GUEST_SYSENTER_CS, 0); + vmcs_writel(GUEST_SYSENTER_ESP, 0); + vmcs_writel(GUEST_SYSENTER_EIP, 0); + vmcs_write64(GUEST_IA32_DEBUGCTL, 0); + } vmcs_writel(GUEST_RFLAGS, 0x02); kvm_rip_write(vcpu, 0xfff0); @@ -4750,14 +4757,11 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu) vmcs_write32(GUEST_INTERRUPTIBILITY_INFO, 0); vmcs_write32(GUEST_PENDING_DBG_EXCEPTIONS, 0); - /* Special registers */ - vmcs_write64(GUEST_IA32_DEBUGCTL, 0); - setup_msrs(vmx); vmcs_write32(VM_ENTRY_INTR_INFO_FIELD, 0); /* 22.2.1 */ - if (cpu_has_vmx_tpr_shadow()) { + if (cpu_has_vmx_tpr_shadow() && !init_event) { vmcs_write64(VIRTUAL_APIC_PAGE_ADDR, 0); if (vm_need_tpr_shadow(vmx->vcpu.kvm)) vmcs_write64(VIRTUAL_APIC_PAGE_ADDR, diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index cc2c759..324e639 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -6961,7 +6961,7 @@ int kvm_arch_vcpu_ioctl_set_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu) return 0; } -int fx_init(struct kvm_vcpu *vcpu) +int fx_init(struct kvm_vcpu *vcpu, bool init_event) { int err; @@ -6969,7 +6969,9 @@ int fx_init(struct kvm_vcpu *vcpu) if (err) return err; - fpu_finit(&vcpu->arch.guest_fpu); + if (!init_event) + fpu_finit(&vcpu->arch.guest_fpu); + if (cpu_has_xsaves) vcpu->arch.guest_fpu.state->xsave.xsave_hdr.xcomp_bv = host_xcr0 | XSTATE_COMPACTION_ENABLED; @@ -7049,7 +7051,7 @@ int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu) r = vcpu_load(vcpu); if (r) return r; - kvm_vcpu_reset(vcpu); + kvm_vcpu_reset(vcpu, false); kvm_mmu_setup(vcpu); vcpu_put(vcpu); @@ -7087,7 +7089,7 @@ void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu) kvm_x86_ops->vcpu_free(vcpu); } -void kvm_vcpu_reset(struct kvm_vcpu *vcpu) +void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event) { atomic_set(&vcpu->arch.nmi_queued, 0); vcpu->arch.nmi_pending = 0; @@ -7111,13 +7113,14 @@ void kvm_vcpu_reset(struct kvm_vcpu *vcpu) kvm_async_pf_hash_reset(vcpu); vcpu->arch.apf.halted = false; - kvm_pmu_reset(vcpu); + if (!init_event) + kvm_pmu_reset(vcpu); memset(vcpu->arch.regs, 0, sizeof(vcpu->arch.regs)); vcpu->arch.regs_avail = ~0; vcpu->arch.regs_dirty = ~0; - kvm_x86_ops->vcpu_reset(vcpu); + kvm_x86_ops->vcpu_reset(vcpu, init_event); } void kvm_vcpu_deliver_sipi_vector(struct kvm_vcpu *vcpu, u8 vector) @@ -7299,7 +7302,7 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu) goto fail_free_mce_banks; } - r = fx_init(vcpu); + r = fx_init(vcpu, false); if (r) goto fail_free_wbinvd_dirty_mask; -- 1.9.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/4] KVM: x86: INIT and reset sequences are different 2015-04-02 0:10 ` [PATCH v2 1/4] KVM: x86: INIT and reset sequences are different Nadav Amit @ 2015-04-02 2:17 ` Bandan Das 2015-04-07 13:23 ` Paolo Bonzini 2015-04-07 13:23 ` Paolo Bonzini 2015-04-07 13:57 ` Paolo Bonzini 2 siblings, 1 reply; 15+ messages in thread From: Bandan Das @ 2015-04-02 2:17 UTC (permalink / raw) To: Nadav Amit; +Cc: pbonzini, kvm Nadav Amit <namit@cs.technion.ac.il> writes: > x86 architecture defines differences between the reset and INIT sequences. > INIT does not initialize the FPU (including MMX, XMM, YMM, etc.), TSC, PMU, > MSRs (in general), MTRRs machine-check, APIC ID, APIC arbitration ID and BSP. > > EFER is supposed NOT to be reset according to the SDM, but leaving the LMA and > LME untouched causes failed VM-entry. Therefore we reset EFER (although it is > unclear whether the rest of EFER bits should be reset). Thanks! This was actually in my todo list. #INIT and #RESET are actually separate pins on the processor. So, shouldn't we differentiate between the two too by having (*vcpu_init) and (*vcpu_reset) separate ? Bandan > References (from Intel SDM): > > "If the MP protocol has completed and a BSP is chosen, subsequent INITs (either > to a specific processor or system wide) do not cause the MP protocol to be > repeated." [8.4.2: MP Initialization Protocol Requirements and Restrictions] > > [Table 9-1. IA-32 Processor States Following Power-up, Reset, or INIT] > > "If the processor is reset by asserting the INIT# pin, the x87 FPU state is not > changed." [9.2: X87 FPU INITIALIZATION] > > "The state of the local APIC following an INIT reset is the same as it is after > a power-up or hardware reset, except that the APIC ID and arbitration ID > registers are not affected." [10.4.7.3: Local APIC State After an INIT Reset > (“Wait-for-SIPI” State)] > > Signed-off-by: Nadav Amit <namit@cs.technion.ac.il> > --- > arch/x86/include/asm/kvm_host.h | 6 +++--- > arch/x86/kvm/lapic.c | 11 ++++++----- > arch/x86/kvm/lapic.h | 2 +- > arch/x86/kvm/svm.c | 2 +- > arch/x86/kvm/vmx.c | 30 +++++++++++++++++------------- > arch/x86/kvm/x86.c | 17 ++++++++++------- > 6 files changed, 38 insertions(+), 30 deletions(-) > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index bf5a160..59f4374 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -701,7 +701,7 @@ struct kvm_x86_ops { > /* Create, but do not attach this VCPU */ > struct kvm_vcpu *(*vcpu_create)(struct kvm *kvm, unsigned id); > void (*vcpu_free)(struct kvm_vcpu *vcpu); > - void (*vcpu_reset)(struct kvm_vcpu *vcpu); > + void (*vcpu_reset)(struct kvm_vcpu *vcpu, bool init_event); > > void (*prepare_guest_switch)(struct kvm_vcpu *vcpu); > void (*vcpu_load)(struct kvm_vcpu *vcpu, int cpu); > @@ -989,7 +989,7 @@ void kvm_pic_clear_all(struct kvm_pic *pic, int irq_source_id); > > void kvm_inject_nmi(struct kvm_vcpu *vcpu); > > -int fx_init(struct kvm_vcpu *vcpu); > +int fx_init(struct kvm_vcpu *vcpu, bool init_event); > > void kvm_mmu_pte_write(struct kvm_vcpu *vcpu, gpa_t gpa, > const u8 *new, int bytes); > @@ -1134,7 +1134,7 @@ int kvm_cpu_has_injectable_intr(struct kvm_vcpu *v); > int kvm_cpu_has_interrupt(struct kvm_vcpu *vcpu); > int kvm_arch_interrupt_allowed(struct kvm_vcpu *vcpu); > int kvm_cpu_get_interrupt(struct kvm_vcpu *v); > -void kvm_vcpu_reset(struct kvm_vcpu *vcpu); > +void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event); > void kvm_vcpu_reload_apic_access_page(struct kvm_vcpu *vcpu); > void kvm_arch_mmu_notifier_invalidate_page(struct kvm *kvm, > unsigned long address); > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > index bd4e34d..17da6fc 100644 > --- a/arch/x86/kvm/lapic.c > +++ b/arch/x86/kvm/lapic.c > @@ -1534,7 +1534,7 @@ void kvm_lapic_set_base(struct kvm_vcpu *vcpu, u64 value) > > } > > -void kvm_lapic_reset(struct kvm_vcpu *vcpu) > +void kvm_lapic_reset(struct kvm_vcpu *vcpu, bool init_event) > { > struct kvm_lapic *apic; > int i; > @@ -1548,7 +1548,8 @@ void kvm_lapic_reset(struct kvm_vcpu *vcpu) > /* Stop the timer in case it's a reset to an active apic */ > hrtimer_cancel(&apic->lapic_timer.timer); > > - kvm_apic_set_id(apic, vcpu->vcpu_id); > + if (!init_event) > + kvm_apic_set_id(apic, vcpu->vcpu_id); > kvm_apic_set_version(apic->vcpu); > > for (i = 0; i < APIC_LVT_NUM; i++) > @@ -1689,7 +1690,7 @@ int kvm_create_lapic(struct kvm_vcpu *vcpu) > APIC_DEFAULT_PHYS_BASE | MSR_IA32_APICBASE_ENABLE); > > static_key_slow_inc(&apic_sw_disabled.key); /* sw disabled at reset */ > - kvm_lapic_reset(vcpu); > + kvm_lapic_reset(vcpu, false); > kvm_iodevice_init(&apic->dev, &apic_mmio_ops); > > return 0; > @@ -2023,8 +2024,8 @@ void kvm_apic_accept_events(struct kvm_vcpu *vcpu) > pe = xchg(&apic->pending_events, 0); > > if (test_bit(KVM_APIC_INIT, &pe)) { > - kvm_lapic_reset(vcpu); > - kvm_vcpu_reset(vcpu); > + kvm_lapic_reset(vcpu, true); > + kvm_vcpu_reset(vcpu, true); > if (kvm_vcpu_is_bsp(apic->vcpu)) > vcpu->arch.mp_state = KVM_MP_STATE_RUNNABLE; > else > diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h > index 0bc6c65..e4c82dc 100644 > --- a/arch/x86/kvm/lapic.h > +++ b/arch/x86/kvm/lapic.h > @@ -48,7 +48,7 @@ int kvm_apic_has_interrupt(struct kvm_vcpu *vcpu); > int kvm_apic_accept_pic_intr(struct kvm_vcpu *vcpu); > int kvm_get_apic_interrupt(struct kvm_vcpu *vcpu); > void kvm_apic_accept_events(struct kvm_vcpu *vcpu); > -void kvm_lapic_reset(struct kvm_vcpu *vcpu); > +void kvm_lapic_reset(struct kvm_vcpu *vcpu, bool init_event); > u64 kvm_lapic_get_cr8(struct kvm_vcpu *vcpu); > void kvm_lapic_set_tpr(struct kvm_vcpu *vcpu, unsigned long cr8); > void kvm_lapic_set_eoi(struct kvm_vcpu *vcpu); > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > index 155534c..1ef4c0d 100644 > --- a/arch/x86/kvm/svm.c > +++ b/arch/x86/kvm/svm.c > @@ -1195,7 +1195,7 @@ static void init_vmcb(struct vcpu_svm *svm) > enable_gif(svm); > } > > -static void svm_vcpu_reset(struct kvm_vcpu *vcpu) > +static void svm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event) > { > struct vcpu_svm *svm = to_svm(vcpu); > u32 dummy; > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index fdd9f8b..e9c94c6 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -4694,7 +4694,7 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx) > return 0; > } > > -static void vmx_vcpu_reset(struct kvm_vcpu *vcpu) > +static void vmx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event) > { > struct vcpu_vmx *vmx = to_vmx(vcpu); > struct msr_data apic_base_msr; > @@ -4705,11 +4705,15 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu) > > vmx->vcpu.arch.regs[VCPU_REGS_RDX] = get_rdx_init_val(); > kvm_set_cr8(&vmx->vcpu, 0); > - apic_base_msr.data = APIC_DEFAULT_PHYS_BASE | MSR_IA32_APICBASE_ENABLE; > - if (kvm_vcpu_is_bsp(&vmx->vcpu)) > - apic_base_msr.data |= MSR_IA32_APICBASE_BSP; > - apic_base_msr.host_initiated = true; > - kvm_set_apic_base(&vmx->vcpu, &apic_base_msr); > + > + if (!init_event) { > + apic_base_msr.data = APIC_DEFAULT_PHYS_BASE | > + MSR_IA32_APICBASE_ENABLE; > + if (kvm_vcpu_is_bsp(&vmx->vcpu)) > + apic_base_msr.data |= MSR_IA32_APICBASE_BSP; > + apic_base_msr.host_initiated = true; > + kvm_set_apic_base(&vmx->vcpu, &apic_base_msr); > + } > > vmx_segment_cache_clear(vmx); > > @@ -4733,9 +4737,12 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu) > vmcs_write32(GUEST_LDTR_LIMIT, 0xffff); > vmcs_write32(GUEST_LDTR_AR_BYTES, 0x00082); > > - vmcs_write32(GUEST_SYSENTER_CS, 0); > - vmcs_writel(GUEST_SYSENTER_ESP, 0); > - vmcs_writel(GUEST_SYSENTER_EIP, 0); > + if (!init_event) { > + vmcs_write32(GUEST_SYSENTER_CS, 0); > + vmcs_writel(GUEST_SYSENTER_ESP, 0); > + vmcs_writel(GUEST_SYSENTER_EIP, 0); > + vmcs_write64(GUEST_IA32_DEBUGCTL, 0); > + } > > vmcs_writel(GUEST_RFLAGS, 0x02); > kvm_rip_write(vcpu, 0xfff0); > @@ -4750,14 +4757,11 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu) > vmcs_write32(GUEST_INTERRUPTIBILITY_INFO, 0); > vmcs_write32(GUEST_PENDING_DBG_EXCEPTIONS, 0); > > - /* Special registers */ > - vmcs_write64(GUEST_IA32_DEBUGCTL, 0); > - > setup_msrs(vmx); > > vmcs_write32(VM_ENTRY_INTR_INFO_FIELD, 0); /* 22.2.1 */ > > - if (cpu_has_vmx_tpr_shadow()) { > + if (cpu_has_vmx_tpr_shadow() && !init_event) { > vmcs_write64(VIRTUAL_APIC_PAGE_ADDR, 0); > if (vm_need_tpr_shadow(vmx->vcpu.kvm)) > vmcs_write64(VIRTUAL_APIC_PAGE_ADDR, > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index cc2c759..324e639 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -6961,7 +6961,7 @@ int kvm_arch_vcpu_ioctl_set_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu) > return 0; > } > > -int fx_init(struct kvm_vcpu *vcpu) > +int fx_init(struct kvm_vcpu *vcpu, bool init_event) > { > int err; > > @@ -6969,7 +6969,9 @@ int fx_init(struct kvm_vcpu *vcpu) > if (err) > return err; > > - fpu_finit(&vcpu->arch.guest_fpu); > + if (!init_event) > + fpu_finit(&vcpu->arch.guest_fpu); > + > if (cpu_has_xsaves) > vcpu->arch.guest_fpu.state->xsave.xsave_hdr.xcomp_bv = > host_xcr0 | XSTATE_COMPACTION_ENABLED; > @@ -7049,7 +7051,7 @@ int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu) > r = vcpu_load(vcpu); > if (r) > return r; > - kvm_vcpu_reset(vcpu); > + kvm_vcpu_reset(vcpu, false); > kvm_mmu_setup(vcpu); > vcpu_put(vcpu); > > @@ -7087,7 +7089,7 @@ void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu) > kvm_x86_ops->vcpu_free(vcpu); > } > > -void kvm_vcpu_reset(struct kvm_vcpu *vcpu) > +void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event) > { > atomic_set(&vcpu->arch.nmi_queued, 0); > vcpu->arch.nmi_pending = 0; > @@ -7111,13 +7113,14 @@ void kvm_vcpu_reset(struct kvm_vcpu *vcpu) > kvm_async_pf_hash_reset(vcpu); > vcpu->arch.apf.halted = false; > > - kvm_pmu_reset(vcpu); > + if (!init_event) > + kvm_pmu_reset(vcpu); > > memset(vcpu->arch.regs, 0, sizeof(vcpu->arch.regs)); > vcpu->arch.regs_avail = ~0; > vcpu->arch.regs_dirty = ~0; > > - kvm_x86_ops->vcpu_reset(vcpu); > + kvm_x86_ops->vcpu_reset(vcpu, init_event); > } > > void kvm_vcpu_deliver_sipi_vector(struct kvm_vcpu *vcpu, u8 vector) > @@ -7299,7 +7302,7 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu) > goto fail_free_mce_banks; > } > > - r = fx_init(vcpu); > + r = fx_init(vcpu, false); > if (r) > goto fail_free_wbinvd_dirty_mask; ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/4] KVM: x86: INIT and reset sequences are different 2015-04-02 2:17 ` Bandan Das @ 2015-04-07 13:23 ` Paolo Bonzini 2015-04-07 16:17 ` Bandan Das 0 siblings, 1 reply; 15+ messages in thread From: Paolo Bonzini @ 2015-04-07 13:23 UTC (permalink / raw) To: Bandan Das, Nadav Amit; +Cc: kvm On 02/04/2015 04:17, Bandan Das wrote: >> > x86 architecture defines differences between the reset and INIT sequences. >> > INIT does not initialize the FPU (including MMX, XMM, YMM, etc.), TSC, PMU, >> > MSRs (in general), MTRRs machine-check, APIC ID, APIC arbitration ID and BSP. >> > >> > EFER is supposed NOT to be reset according to the SDM, but leaving the LMA and >> > LME untouched causes failed VM-entry. Therefore we reset EFER (although it is >> > unclear whether the rest of EFER bits should be reset). > Thanks! This was actually in my todo list. #INIT and #RESET are actually separate pins > on the processor. So, shouldn't we differentiate between the two too by having > (*vcpu_init) and (*vcpu_reset) separate ? I think a bool argument is good enough. QEMU has different functions, and init ends up doing save/reset/restore which is pretty ugly. Paolo ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/4] KVM: x86: INIT and reset sequences are different 2015-04-07 13:23 ` Paolo Bonzini @ 2015-04-07 16:17 ` Bandan Das 2015-04-07 16:22 ` Paolo Bonzini 0 siblings, 1 reply; 15+ messages in thread From: Bandan Das @ 2015-04-07 16:17 UTC (permalink / raw) To: Paolo Bonzini; +Cc: Nadav Amit, kvm Paolo Bonzini <pbonzini@redhat.com> writes: > On 02/04/2015 04:17, Bandan Das wrote: >>> > x86 architecture defines differences between the reset and INIT sequences. >>> > INIT does not initialize the FPU (including MMX, XMM, YMM, etc.), TSC, PMU, >>> > MSRs (in general), MTRRs machine-check, APIC ID, APIC arbitration ID and BSP. >>> > >>> > EFER is supposed NOT to be reset according to the SDM, but leaving the LMA and >>> > LME untouched causes failed VM-entry. Therefore we reset EFER (although it is >>> > unclear whether the rest of EFER bits should be reset). >> Thanks! This was actually in my todo list. #INIT and #RESET are actually separate pins >> on the processor. So, shouldn't we differentiate between the two too by having >> (*vcpu_init) and (*vcpu_reset) separate ? > > I think a bool argument is good enough. QEMU has different functions, > and init ends up doing save/reset/restore which is pretty ugly. Right, I meant that init could just be a wrapper so that it atleast shows up in a backtrace - could be helpful for debugging. > Paolo > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/4] KVM: x86: INIT and reset sequences are different 2015-04-07 16:17 ` Bandan Das @ 2015-04-07 16:22 ` Paolo Bonzini 2015-04-07 16:35 ` Bandan Das 0 siblings, 1 reply; 15+ messages in thread From: Paolo Bonzini @ 2015-04-07 16:22 UTC (permalink / raw) To: Bandan Das; +Cc: Nadav Amit, kvm On 07/04/2015 18:17, Bandan Das wrote: > > I think a bool argument is good enough. QEMU has different functions, > > and init ends up doing save/reset/restore which is pretty ugly. > > Right, I meant that init could just be a wrapper so that it atleast shows up in > a backtrace - could be helpful for debugging. I suspect that the compiler would inline any sensible implementation and it wouldn't show up in the backtraces. :( Paolo ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/4] KVM: x86: INIT and reset sequences are different 2015-04-07 16:22 ` Paolo Bonzini @ 2015-04-07 16:35 ` Bandan Das 0 siblings, 0 replies; 15+ messages in thread From: Bandan Das @ 2015-04-07 16:35 UTC (permalink / raw) To: Paolo Bonzini; +Cc: Nadav Amit, kvm Paolo Bonzini <pbonzini@redhat.com> writes: > On 07/04/2015 18:17, Bandan Das wrote: >> > I think a bool argument is good enough. QEMU has different functions, >> > and init ends up doing save/reset/restore which is pretty ugly. >> >> Right, I meant that init could just be a wrapper so that it atleast shows up in >> a backtrace - could be helpful for debugging. > > I suspect that the compiler would inline any sensible implementation and > it wouldn't show up in the backtraces. :( noinline ? :) Anyway, it's probably not worth the trouble, that could be easily figured out. Thanks, Bandan > Paolo ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/4] KVM: x86: INIT and reset sequences are different 2015-04-02 0:10 ` [PATCH v2 1/4] KVM: x86: INIT and reset sequences are different Nadav Amit 2015-04-02 2:17 ` Bandan Das @ 2015-04-07 13:23 ` Paolo Bonzini 2015-04-07 13:57 ` Paolo Bonzini 2 siblings, 0 replies; 15+ messages in thread From: Paolo Bonzini @ 2015-04-07 13:23 UTC (permalink / raw) To: Nadav Amit; +Cc: kvm On 02/04/2015 02:10, Nadav Amit wrote: > x86 architecture defines differences between the reset and INIT sequences. > INIT does not initialize the FPU (including MMX, XMM, YMM, etc.), TSC, PMU, > MSRs (in general), MTRRs machine-check, APIC ID, APIC arbitration ID and BSP. > > EFER is supposed NOT to be reset according to the SDM, but leaving the LMA and > LME untouched causes failed VM-entry. Therefore we reset EFER (although it is > unclear whether the rest of EFER bits should be reset). Do you get failed VM-entry even if LME=1, LMA=0? LMA obviously should be reset, but LME=1/PG=0/PAE=0 is shown as valid in Figure 4-1 of the SDM. Paolo ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/4] KVM: x86: INIT and reset sequences are different 2015-04-02 0:10 ` [PATCH v2 1/4] KVM: x86: INIT and reset sequences are different Nadav Amit 2015-04-02 2:17 ` Bandan Das 2015-04-07 13:23 ` Paolo Bonzini @ 2015-04-07 13:57 ` Paolo Bonzini 2 siblings, 0 replies; 15+ messages in thread From: Paolo Bonzini @ 2015-04-07 13:57 UTC (permalink / raw) To: Nadav Amit; +Cc: kvm On 02/04/2015 02:10, Nadav Amit wrote: > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > index 155534c..1ef4c0d 100644 > --- a/arch/x86/kvm/svm.c > +++ b/arch/x86/kvm/svm.c > @@ -1195,7 +1195,7 @@ static void init_vmcb(struct vcpu_svm *svm) > enable_gif(svm); > } > > -static void svm_vcpu_reset(struct kvm_vcpu *vcpu) > +static void svm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event) > { > struct vcpu_svm *svm = to_svm(vcpu); > u32 dummy; Please move this code: svm->vcpu.arch.apic_base = APIC_DEFAULT_PHYS_BASE | MSR_IA32_APICBASE_ENABLE; if (kvm_vcpu_is_reset_bsp(&svm->vcpu)) svm->vcpu.arch.apic_base |= MSR_IA32_APICBASE_BSP; from svm_create_vcpu to svm_vcpu_reset, so that it can be wrapped with "if (!init_event)" as in the VMX case. Paolo ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 2/4] KVM: x86: BSP in MSR_IA32_APICBASE is writable 2015-04-02 0:10 [PATCH v2 0/4] KVM: x86: Reset fixes Nadav Amit 2015-04-02 0:10 ` [PATCH v2 1/4] KVM: x86: INIT and reset sequences are different Nadav Amit @ 2015-04-02 0:10 ` Nadav Amit 2015-04-02 0:10 ` [PATCH v2 3/4] KVM: x86: DR0-DR3 are not clear on reset Nadav Amit ` (2 subsequent siblings) 4 siblings, 0 replies; 15+ messages in thread From: Nadav Amit @ 2015-04-02 0:10 UTC (permalink / raw) To: pbonzini; +Cc: kvm, Nadav Amit After reset, the CPU can change the BSP, which will be used upon INIT. Reset should return the BSP which QEMU asked for, and therefore handled accordingly. To quote: "If the MP protocol has completed and a BSP is chosen, subsequent INITs (either to a specific processor or system wide) do not cause the MP protocol to be repeated." [Intel SDM 8.4.2: MP Initialization Protocol Requirements and Restrictions] Signed-off-by: Nadav Amit <namit@cs.technion.ac.il> --- arch/x86/kvm/lapic.c | 2 -- arch/x86/kvm/svm.c | 2 +- arch/x86/kvm/vmx.c | 2 +- arch/x86/kvm/x86.c | 2 +- include/linux/kvm_host.h | 7 ++++++- 5 files changed, 9 insertions(+), 6 deletions(-) diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index 17da6fc..b0dbf68 100644 --- a/arch/x86/kvm/lapic.c +++ b/arch/x86/kvm/lapic.c @@ -1498,8 +1498,6 @@ void kvm_lapic_set_base(struct kvm_vcpu *vcpu, u64 value) return; } - if (!kvm_vcpu_is_bsp(apic->vcpu)) - value &= ~MSR_IA32_APICBASE_BSP; vcpu->arch.apic_base = value; /* update jump label if enable bit changes */ diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index 1ef4c0d..ef5bf21 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -1261,7 +1261,7 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm, unsigned int id) svm->vcpu.arch.apic_base = APIC_DEFAULT_PHYS_BASE | MSR_IA32_APICBASE_ENABLE; - if (kvm_vcpu_is_bsp(&svm->vcpu)) + if (kvm_vcpu_is_reset_bsp(&svm->vcpu)) svm->vcpu.arch.apic_base |= MSR_IA32_APICBASE_BSP; svm_init_osvw(&svm->vcpu); diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index e9c94c6..5925c65 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -4709,7 +4709,7 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event) if (!init_event) { apic_base_msr.data = APIC_DEFAULT_PHYS_BASE | MSR_IA32_APICBASE_ENABLE; - if (kvm_vcpu_is_bsp(&vmx->vcpu)) + if (kvm_vcpu_is_reset_bsp(&vmx->vcpu)) apic_base_msr.data |= MSR_IA32_APICBASE_BSP; apic_base_msr.host_initiated = true; kvm_set_apic_base(&vmx->vcpu, &apic_base_msr); diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 324e639..c93b1b5 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -7264,7 +7264,7 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu) vcpu->arch.pv.pv_unhalted = false; vcpu->arch.emulate_ctxt.ops = &emulate_ops; - if (!irqchip_in_kernel(kvm) || kvm_vcpu_is_bsp(vcpu)) + if (!irqchip_in_kernel(kvm) || kvm_vcpu_is_reset_bsp(vcpu)) vcpu->arch.mp_state = KVM_MP_STATE_RUNNABLE; else vcpu->arch.mp_state = KVM_MP_STATE_UNINITIALIZED; diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index 0f574eb..8365cae 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -968,11 +968,16 @@ static inline int kvm_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args) #endif /* CONFIG_HAVE_KVM_EVENTFD */ #ifdef CONFIG_KVM_APIC_ARCHITECTURE -static inline bool kvm_vcpu_is_bsp(struct kvm_vcpu *vcpu) +static inline bool kvm_vcpu_is_reset_bsp(struct kvm_vcpu *vcpu) { return vcpu->kvm->bsp_vcpu_id == vcpu->vcpu_id; } +static inline bool kvm_vcpu_is_bsp(struct kvm_vcpu *vcpu) +{ + return (vcpu->arch.apic_base & MSR_IA32_APICBASE_BSP) != 0; +} + bool kvm_vcpu_compatible(struct kvm_vcpu *vcpu); #else -- 1.9.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 3/4] KVM: x86: DR0-DR3 are not clear on reset 2015-04-02 0:10 [PATCH v2 0/4] KVM: x86: Reset fixes Nadav Amit 2015-04-02 0:10 ` [PATCH v2 1/4] KVM: x86: INIT and reset sequences are different Nadav Amit 2015-04-02 0:10 ` [PATCH v2 2/4] KVM: x86: BSP in MSR_IA32_APICBASE is writable Nadav Amit @ 2015-04-02 0:10 ` Nadav Amit 2015-04-02 0:10 ` [PATCH v2 4/4] KVM: x86: Clear CR2 on VCPU reset Nadav Amit 2015-04-07 14:01 ` [PATCH v2 0/4] KVM: x86: Reset fixes Paolo Bonzini 4 siblings, 0 replies; 15+ messages in thread From: Nadav Amit @ 2015-04-02 0:10 UTC (permalink / raw) To: pbonzini; +Cc: kvm, Nadav Amit DR0-DR3 are not cleared as they should during reset and when they are set from userspace. It appears to be caused by c77fb5fe6f03 ("KVM: x86: Allow the guest to run with dirty debug registers"). Force their reload on these situations. Signed-off-by: Nadav Amit <namit@cs.technion.ac.il> --- arch/x86/include/asm/kvm_host.h | 1 + arch/x86/kvm/x86.c | 14 ++++++++++++++ 2 files changed, 15 insertions(+) diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index 59f4374..495fcb6 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -345,6 +345,7 @@ struct kvm_pmu { enum { KVM_DEBUGREG_BP_ENABLED = 1, KVM_DEBUGREG_WONT_EXIT = 2, + KVM_DEBUGREG_RELOAD = 4, }; struct kvm_vcpu_arch { diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index c93b1b5..e4ac17e 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -801,6 +801,17 @@ unsigned long kvm_get_cr8(struct kvm_vcpu *vcpu) } EXPORT_SYMBOL_GPL(kvm_get_cr8); +static void kvm_update_dr0123(struct kvm_vcpu *vcpu) +{ + int i; + + if (!(vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP)) { + for (i = 0; i < KVM_NR_DB_REGS; i++) + vcpu->arch.eff_db[i] = vcpu->arch.db[i]; + vcpu->arch.switch_db_regs |= KVM_DEBUGREG_RELOAD; + } +} + static void kvm_update_dr6(struct kvm_vcpu *vcpu) { if (!(vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP)) @@ -3150,6 +3161,7 @@ static int kvm_vcpu_ioctl_x86_set_debugregs(struct kvm_vcpu *vcpu, return -EINVAL; memcpy(vcpu->arch.db, dbgregs->db, sizeof(vcpu->arch.db)); + kvm_update_dr0123(vcpu); vcpu->arch.dr6 = dbgregs->dr6; kvm_update_dr6(vcpu); vcpu->arch.dr7 = dbgregs->dr7; @@ -6322,6 +6334,7 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) set_debugreg(vcpu->arch.eff_db[2], 2); set_debugreg(vcpu->arch.eff_db[3], 3); set_debugreg(vcpu->arch.dr6, 6); + vcpu->arch.switch_db_regs &= ~KVM_DEBUGREG_RELOAD; } trace_kvm_entry(vcpu->vcpu_id); @@ -7098,6 +7111,7 @@ void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event) kvm_clear_exception_queue(vcpu); memset(vcpu->arch.db, 0, sizeof(vcpu->arch.db)); + kvm_update_dr0123(vcpu); vcpu->arch.dr6 = DR6_INIT; kvm_update_dr6(vcpu); vcpu->arch.dr7 = DR7_FIXED_1; -- 1.9.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 4/4] KVM: x86: Clear CR2 on VCPU reset 2015-04-02 0:10 [PATCH v2 0/4] KVM: x86: Reset fixes Nadav Amit ` (2 preceding siblings ...) 2015-04-02 0:10 ` [PATCH v2 3/4] KVM: x86: DR0-DR3 are not clear on reset Nadav Amit @ 2015-04-02 0:10 ` Nadav Amit 2015-04-14 10:03 ` Wanpeng Li 2015-04-07 14:01 ` [PATCH v2 0/4] KVM: x86: Reset fixes Paolo Bonzini 4 siblings, 1 reply; 15+ messages in thread From: Nadav Amit @ 2015-04-02 0:10 UTC (permalink / raw) To: pbonzini; +Cc: kvm, Nadav Amit CR2 is not cleared as it should after reset. See Intel SDM table named "IA-32 Processor States Following Power-up, Reset, or INIT". Signed-off-by: Nadav Amit <namit@cs.technion.ac.il> --- arch/x86/kvm/x86.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index e4ac17e..8fdad04 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -7117,6 +7117,8 @@ void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event) vcpu->arch.dr7 = DR7_FIXED_1; kvm_update_dr7(vcpu); + vcpu->arch.cr2 = 0; + kvm_make_request(KVM_REQ_EVENT, vcpu); vcpu->arch.apf.msr_val = 0; vcpu->arch.st.msr_val = 0; -- 1.9.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 4/4] KVM: x86: Clear CR2 on VCPU reset 2015-04-02 0:10 ` [PATCH v2 4/4] KVM: x86: Clear CR2 on VCPU reset Nadav Amit @ 2015-04-14 10:03 ` Wanpeng Li 2015-04-14 10:31 ` Nadav Amit 0 siblings, 1 reply; 15+ messages in thread From: Wanpeng Li @ 2015-04-14 10:03 UTC (permalink / raw) To: Nadav Amit; +Cc: pbonzini, kvm Hi Nadav, On Thu, Apr 02, 2015 at 03:10:38AM +0300, Nadav Amit wrote: >CR2 is not cleared as it should after reset. See Intel SDM table named "IA-32 >Processor States Following Power-up, Reset, or INIT". How you trigger the reset instead of the "Power-up" one? Regards, Wanpeng Li > >Signed-off-by: Nadav Amit <namit@cs.technion.ac.il> >--- > arch/x86/kvm/x86.c | 2 ++ > 1 file changed, 2 insertions(+) > >diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >index e4ac17e..8fdad04 100644 >--- a/arch/x86/kvm/x86.c >+++ b/arch/x86/kvm/x86.c >@@ -7117,6 +7117,8 @@ void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event) > vcpu->arch.dr7 = DR7_FIXED_1; > kvm_update_dr7(vcpu); > >+ vcpu->arch.cr2 = 0; >+ > kvm_make_request(KVM_REQ_EVENT, vcpu); > vcpu->arch.apf.msr_val = 0; > vcpu->arch.st.msr_val = 0; >-- >1.9.1 > >-- >To unsubscribe from this list: send the line "unsubscribe kvm" in >the body of a message to majordomo@vger.kernel.org >More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 4/4] KVM: x86: Clear CR2 on VCPU reset 2015-04-14 10:03 ` Wanpeng Li @ 2015-04-14 10:31 ` Nadav Amit 0 siblings, 0 replies; 15+ messages in thread From: Nadav Amit @ 2015-04-14 10:31 UTC (permalink / raw) To: Wanpeng Li; +Cc: Nadav Amit, pbonzini, kvm Wanpeng Li <wanpeng.li@linux.intel.com> wrote: > Hi Nadav, > On Thu, Apr 02, 2015 at 03:10:38AM +0300, Nadav Amit wrote: >> CR2 is not cleared as it should after reset. See Intel SDM table named "IA-32 >> Processor States Following Power-up, Reset, or INIT". > > How you trigger the reset instead of the "Power-up" one? I sent an IPI of INIT for the KVM “reset” flow. I posted a unit-test: http://www.spinics.net/lists/kvm/msg115525.html The actual reset is handled by qemu, but KVM is still able to introduce bugs in it, as it did in not reseting DR0-DR3. Nadav ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 0/4] KVM: x86: Reset fixes 2015-04-02 0:10 [PATCH v2 0/4] KVM: x86: Reset fixes Nadav Amit ` (3 preceding siblings ...) 2015-04-02 0:10 ` [PATCH v2 4/4] KVM: x86: Clear CR2 on VCPU reset Nadav Amit @ 2015-04-07 14:01 ` Paolo Bonzini 4 siblings, 0 replies; 15+ messages in thread From: Paolo Bonzini @ 2015-04-07 14:01 UTC (permalink / raw) To: Nadav Amit; +Cc: kvm On 02/04/2015 02:10, Nadav Amit wrote: > This set includes 2 previous patches that deal with the INIT flow that is not > distinguished from regular boot, and allowing the VM to change BSP (which is > used in very certain testing environments). The next 2 patches are new, dealing > with regression that cause DR0-DR3 not to be reset (even when QEMU initiates > the RESET) and CR2 not cleared after INIT. > > The second patch regarding BSP requires an additional fix for QEMU, as > otherwise reset fails. A separate patch was submitted to QEMU mailing-list. > > Thanks for reviewing the patches. > > Nadav Amit (4): > KVM: x86: INIT and reset sequences are different > KVM: x86: BSP in MSR_IA32_APICBASE is writable > KVM: x86: DR0-DR3 are not clear on reset > KVM: x86: Clear CR2 on VCPU reset > > arch/x86/include/asm/kvm_host.h | 7 ++++--- > arch/x86/kvm/lapic.c | 13 ++++++------- > arch/x86/kvm/lapic.h | 2 +- > arch/x86/kvm/svm.c | 4 ++-- > arch/x86/kvm/vmx.c | 30 +++++++++++++++++------------- > arch/x86/kvm/x86.c | 35 +++++++++++++++++++++++++++-------- > include/linux/kvm_host.h | 7 ++++++- > 7 files changed, 63 insertions(+), 35 deletions(-) > Applying patches 2-4, thanks. Paolo ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2015-04-14 10:31 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-04-02 0:10 [PATCH v2 0/4] KVM: x86: Reset fixes Nadav Amit 2015-04-02 0:10 ` [PATCH v2 1/4] KVM: x86: INIT and reset sequences are different Nadav Amit 2015-04-02 2:17 ` Bandan Das 2015-04-07 13:23 ` Paolo Bonzini 2015-04-07 16:17 ` Bandan Das 2015-04-07 16:22 ` Paolo Bonzini 2015-04-07 16:35 ` Bandan Das 2015-04-07 13:23 ` Paolo Bonzini 2015-04-07 13:57 ` Paolo Bonzini 2015-04-02 0:10 ` [PATCH v2 2/4] KVM: x86: BSP in MSR_IA32_APICBASE is writable Nadav Amit 2015-04-02 0:10 ` [PATCH v2 3/4] KVM: x86: DR0-DR3 are not clear on reset Nadav Amit 2015-04-02 0:10 ` [PATCH v2 4/4] KVM: x86: Clear CR2 on VCPU reset Nadav Amit 2015-04-14 10:03 ` Wanpeng Li 2015-04-14 10:31 ` Nadav Amit 2015-04-07 14:01 ` [PATCH v2 0/4] KVM: x86: Reset fixes Paolo Bonzini
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.