[PATCH v2 0/4] KVM: x86: Reset fixes

[PATCH v2 0/4] KVM: x86: Reset fixes

[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

[PATCH v2 1/4] KVM: x86: INIT and reset sequences are different

[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

[PATCH v2 2/4] KVM: x86: BSP in MSR_IA32_APICBASE is writable

[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

[PATCH v2 3/4] KVM: x86: DR0-DR3 are not clear on reset

[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

[PATCH v2 4/4] KVM: x86: Clear CR2 on VCPU reset

[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

Re: [PATCH v2 1/4] KVM: x86: INIT and reset sequences are different

[Bandan Das]

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;

Re: [PATCH v2 1/4] KVM: x86: INIT and reset sequences are different

[Paolo Bonzini]

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

Re: [PATCH v2 1/4] KVM: x86: INIT and reset sequences are different

[Paolo Bonzini]

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

Re: [PATCH v2 1/4] KVM: x86: INIT and reset sequences are different

[Paolo Bonzini]

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

Re: [PATCH v2 0/4] KVM: x86: Reset fixes

[Paolo Bonzini]

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

Re: [PATCH v2 1/4] KVM: x86: INIT and reset sequences are different

[Bandan Das]

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

Re: [PATCH v2 1/4] KVM: x86: INIT and reset sequences are different

[Paolo Bonzini]

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

Re: [PATCH v2 1/4] KVM: x86: INIT and reset sequences are different

[Bandan Das]

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

Re: [PATCH v2 4/4] KVM: x86: Clear CR2 on VCPU reset

[Wanpeng Li]

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

Re: [PATCH v2 4/4] KVM: x86: Clear CR2 on VCPU reset

[Nadav Amit]

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