KVM VMX: register state after reset violates spec

KVM VMX: register state after reset violates spec

[Julian Stecklina]

Hello,

we have noticed that at least on 3.6.8 with VMX after a VCPU has been
reset via the INIT-SIPI-SIPI sequence its register state violates
Intel's specification.

Specifically for our case we see at the end of vmx_vcpu_reset the
following vcpu state:

regs_avail=ffefffff regs_dirty=00010010
EIP=00000000 EAX=000006e8 EBX=00000001 ECX=80000001 EDX=00000600
ESI=0000d238 EDI=00000000 EBP=00000000 ESP=00000000

although EAX, EBX, ECX, ESI, EDI, EBP, ESP should _all_ be zero. See
http://download.intel.com/products/processor/manual/253668.pdf section
9.1.1 (page 9-2).

Shouldn't vmx_vcpu_reset actively clear those registers? And from a
quick glance at the SVM code the problem might exist there, too.

A workaround is to use qemu-kvm with -kvm-no-irqchip.

Julian

[PATCH] KVM VMX: Make register state after reset conform to specification.

[Julian Stecklina]

Signed-off-by: Julian Stecklina <jsteckli@os.inf.tu-dresden.de>
---
 arch/x86/kvm/vmx.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index ff66a3b..ec5a3b3 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -3948,7 +3948,6 @@ static int vmx_vcpu_reset(struct kvm_vcpu *vcpu)
 
 	vmx->soft_vnmi_blocked = 0;
 
-	vmx->vcpu.arch.regs[VCPU_REGS_RDX] = get_rdx_init_val();
 	kvm_set_cr8(&vmx->vcpu, 0);
 	msr = 0xfee00000 | MSR_IA32_APICBASE_ENABLE;
 	if (kvm_vcpu_is_bsp(&vmx->vcpu))
@@ -3999,6 +3998,14 @@ static int vmx_vcpu_reset(struct kvm_vcpu *vcpu)
 		kvm_rip_write(vcpu, 0xfff0);
 	else
 		kvm_rip_write(vcpu, 0);
+
+	kvm_register_write(vcpu, VCPU_REGS_RAX, 0);
+	kvm_register_write(vcpu, VCPU_REGS_RBX, 0);
+	kvm_register_write(vcpu, VCPU_REGS_RCX, 0);
+	kvm_register_write(vcpu, VCPU_REGS_RDX, get_rdx_init_val());
+	kvm_register_write(vcpu, VCPU_REGS_RSI, 0);
+	kvm_register_write(vcpu, VCPU_REGS_RDI, 0);
+	kvm_register_write(vcpu, VCPU_REGS_RBP, 0);
 	kvm_register_write(vcpu, VCPU_REGS_RSP, 0);
 
 	vmcs_writel(GUEST_DR7, 0x400);
-- 
1.7.11.7

[PATCH] KVM VMX: Make register state after reset conform to specification.

[Julian Stecklina]

Signed-off-by: Julian Stecklina <jsteckli@os.inf.tu-dresden.de>
---
 arch/x86/kvm/vmx.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index ff66a3b..ec5a3b3 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -3948,7 +3948,6 @@ static int vmx_vcpu_reset(struct kvm_vcpu *vcpu)
 
 	vmx->soft_vnmi_blocked = 0;
 
-	vmx->vcpu.arch.regs[VCPU_REGS_RDX] = get_rdx_init_val();
 	kvm_set_cr8(&vmx->vcpu, 0);
 	msr = 0xfee00000 | MSR_IA32_APICBASE_ENABLE;
 	if (kvm_vcpu_is_bsp(&vmx->vcpu))
@@ -3999,6 +3998,14 @@ static int vmx_vcpu_reset(struct kvm_vcpu *vcpu)
 		kvm_rip_write(vcpu, 0xfff0);
 	else
 		kvm_rip_write(vcpu, 0);
+
+	kvm_register_write(vcpu, VCPU_REGS_RAX, 0);
+	kvm_register_write(vcpu, VCPU_REGS_RBX, 0);
+	kvm_register_write(vcpu, VCPU_REGS_RCX, 0);
+	kvm_register_write(vcpu, VCPU_REGS_RDX, get_rdx_init_val());
+	kvm_register_write(vcpu, VCPU_REGS_RSI, 0);
+	kvm_register_write(vcpu, VCPU_REGS_RDI, 0);
+	kvm_register_write(vcpu, VCPU_REGS_RBP, 0);
 	kvm_register_write(vcpu, VCPU_REGS_RSP, 0);
 
 	vmcs_writel(GUEST_DR7, 0x400);
-- 
1.7.11.7

Re: KVM VMX: register state after reset violates spec

[Gleb Natapov]

On Thu, Nov 29, 2012 at 03:07:38PM +0100, Julian Stecklina wrote:
> Hello,
> 
> we have noticed that at least on 3.6.8 with VMX after a VCPU has been
> reset via the INIT-SIPI-SIPI sequence its register state violates
> Intel's specification.
> 
> Specifically for our case we see at the end of vmx_vcpu_reset the
> following vcpu state:
> 
> regs_avail=ffefffff regs_dirty=00010010
> EIP=00000000 EAX=000006e8 EBX=00000001 ECX=80000001 EDX=00000600
> ESI=0000d238 EDI=00000000 EBP=00000000 ESP=00000000
> 
> although EAX, EBX, ECX, ESI, EDI, EBP, ESP should _all_ be zero. See
> http://download.intel.com/products/processor/manual/253668.pdf section
> 9.1.1 (page 9-2).
> 
> Shouldn't vmx_vcpu_reset actively clear those registers? And from a
> quick glance at the SVM code the problem might exist there, too.
> 
It should, so why not move the fix to kvm_vcpu_reset() so it will work
for both. Also what about R8-R15? Intel SDM says nothing about them in
the section you mention, but in Volume 1 section 3.4.1.1 is says:

 Registers only available in 64-bit mode (R8-R15 and XMM8-XMM15) are
 preserved across transitions from 64-bit mode into compatibility mode
 then back into 64-bit mode. However, values of R8-R15 and XMM8-XMM15
 are undefined after transitions from 64-bit mode through compatibility
 mode to legacy or real mode and then back through compatibility mode to
 64-bit mode.

I take it that they are undefined on the first transition to 64-bit mode
too. AMD spec says that they should be zeroed on reset, so lets do that.
Also SVM does not set EDX to correct value on reset. It should be:

 Stepping ID (bits 3:0)—This field identifies the processor-revision level.
 Extended Model (bits 19:16) and Model (bits 7:4)—These fields combine to
           differentiate processor models within a instruction family. For
           example, two processors may share the same microarchitecture but
           differ in their feature set. Such processors are considered different
           models within the same instruction family. This is a split field,
           comprising an extended-model portion in bits 19:16 with a legacy
           portion in bits 7:4
 Extended Family (bits 27:20) and Family (bits 11:8)—These fields combine to
           differentiate processors by their microarchitecture.

> A workaround is to use qemu-kvm with -kvm-no-irqchip.
> 
> Julian
> 
> --
> 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

--
			Gleb.

Re: KVM VMX: register state after reset violates spec

[Julian Stecklina]

Thus spake Gleb Natapov <gleb@redhat.com>:

> On Thu, Nov 29, 2012 at 03:07:38PM +0100, Julian Stecklina wrote:
>> Hello,
>> 
>> we have noticed that at least on 3.6.8 with VMX after a VCPU has been
>> reset via the INIT-SIPI-SIPI sequence its register state violates
>> Intel's specification.
[...]
>> Shouldn't vmx_vcpu_reset actively clear those registers? And from a
>> quick glance at the SVM code the problem might exist there, too.
>> 
> It should, so why not move the fix to kvm_vcpu_reset() so it will work
> for both. Also what about R8-R15? Intel SDM says nothing about them in
> the section you mention, but in Volume 1 section 3.4.1.1 is says:
[...]
> I take it that they are undefined on the first transition to 64-bit mode
> too. AMD spec says that they should be zeroed on reset, so lets do that.
> Also SVM does not set EDX to correct value on reset.

I'll post a revised patch later today.

Julian

[PATCH v2] KVM: x86: Make register state after reset conform to specification

[Julian Stecklina]

Floating point initialization is moved to kvm_arch_vcpu_init. Added general purpose
register clearing to the same function. SVM code now properly initializes
EDX.

Signed-off-by: Julian Stecklina <jsteckli@os.inf.tu-dresden.de>
---
 arch/x86/kvm/cpuid.c |  1 +
 arch/x86/kvm/svm.c   |  7 +++++--
 arch/x86/kvm/vmx.c   |  7 +------
 arch/x86/kvm/x86.c   | 12 +++++++++++-
 4 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 0595f13..aa468c2 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -659,6 +659,7 @@ void kvm_cpuid(struct kvm_vcpu *vcpu, u32 *eax, u32 *ebx, u32 *ecx, u32 *edx)
 	} else
 		*eax = *ebx = *ecx = *edx = 0;
 }
+EXPORT_SYMBOL_GPL(kvm_cpuid);
 
 void kvm_emulate_cpuid(struct kvm_vcpu *vcpu)
 {
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index baead95..642213a 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -20,6 +20,7 @@
 #include "mmu.h"
 #include "kvm_cache_regs.h"
 #include "x86.h"
+#include "cpuid.h"
 
 #include <linux/module.h>
 #include <linux/mod_devicetable.h>
@@ -1190,6 +1191,7 @@ static void init_vmcb(struct vcpu_svm *svm)
 static int svm_vcpu_reset(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
+	u32 dummy, eax;
 
 	init_vmcb(svm);
 
@@ -1198,8 +1200,9 @@ static int svm_vcpu_reset(struct kvm_vcpu *vcpu)
 		svm->vmcb->save.cs.base = svm->vcpu.arch.sipi_vector << 12;
 		svm->vmcb->save.cs.selector = svm->vcpu.arch.sipi_vector << 8;
 	}
-	vcpu->arch.regs_avail = ~0;
-	vcpu->arch.regs_dirty = ~0;
+
+	kvm_cpuid(vcpu, &eax, &dummy, &dummy, &dummy);
+	kvm_register_write(vcpu, VCPU_REGS_RDX, eax);
 
 	return 0;
 }
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index ff66a3b..363fb03 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -3942,7 +3942,7 @@ static int vmx_vcpu_reset(struct kvm_vcpu *vcpu)
 	u64 msr;
 	int ret;
 
-	vcpu->arch.regs_avail = ~((1 << VCPU_REGS_RIP) | (1 << VCPU_REGS_RSP));
+	vcpu->arch.regs_avail = ~(1 << VCPU_REGS_RIP);
 
 	vmx->rmode.vm86_active = 0;
 
@@ -3955,10 +3955,6 @@ static int vmx_vcpu_reset(struct kvm_vcpu *vcpu)
 		msr |= MSR_IA32_APICBASE_BSP;
 	kvm_set_apic_base(&vmx->vcpu, msr);
 
-	ret = fx_init(&vmx->vcpu);
-	if (ret != 0)
-		goto out;
-
 	vmx_segment_cache_clear(vmx);
 
 	seg_setup(VCPU_SREG_CS);
@@ -3999,7 +3995,6 @@ static int vmx_vcpu_reset(struct kvm_vcpu *vcpu)
 		kvm_rip_write(vcpu, 0xfff0);
 	else
 		kvm_rip_write(vcpu, 0);
-	kvm_register_write(vcpu, VCPU_REGS_RSP, 0);
 
 	vmcs_writel(GUEST_DR7, 0x400);
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 2966c84..9640ea5 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -6045,6 +6045,8 @@ void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
 
 int kvm_arch_vcpu_reset(struct kvm_vcpu *vcpu)
 {
+	int ret;
+
 	atomic_set(&vcpu->arch.nmi_queued, 0);
 	vcpu->arch.nmi_pending = 0;
 	vcpu->arch.nmi_injected = false;
@@ -6066,7 +6068,15 @@ int kvm_arch_vcpu_reset(struct kvm_vcpu *vcpu)
 
 	kvm_pmu_reset(vcpu);
 
-	return kvm_x86_ops->vcpu_reset(vcpu);
+	memset(vcpu->arch.regs, 0, sizeof(vcpu->arch.regs));
+	vcpu->arch.regs_avail = ~0;
+	vcpu->arch.regs_dirty = ~0;
+
+	if ((ret = fx_init(vcpu)) != 0) goto out;
+
+	ret = kvm_x86_ops->vcpu_reset(vcpu);
+out:
+	return ret;
 }
 
 int kvm_arch_hardware_enable(void *garbage)
-- 
1.7.11.7

Re: KVM VMX: register state after reset violates spec

[Julian Stecklina]

Thus spake Gleb Natapov <gleb@redhat.com>:

> It should, so why not move the fix to kvm_vcpu_reset() so it will work
> for both. Also what about R8-R15? Intel SDM says nothing about them in
> the section you mention, but in Volume 1 section 3.4.1.1 is says:
[...]
> I take it that they are undefined on the first transition to 64-bit mode
> too. AMD spec says that they should be zeroed on reset, so lets do that.
> Also SVM does not set EDX to correct value on reset. It should be:

I have posted a new version of the patch taking your suggestions into
account. The VMX version is working for me. I could not test it on AMD
hardware, though.

Julian

Re: [PATCH v2] KVM: x86: Make register state after reset conform to specification

[Gleb Natapov]

On Mon, Dec 03, 2012 at 02:46:55PM +0100, Julian Stecklina wrote:
> Floating point initialization is moved to kvm_arch_vcpu_init. Added general purpose
Actually Intel SDM Table 9-1 footnote 5 says: "The state of the x87 FPU and MMX
registers is not changed by the execution of an INIT.". So lets drop
this part of the patch for now since vcpu_reset() does not distinguish between reset
and INIT currently.

> register clearing to the same function. SVM code now properly initializes
> EDX.
> 
> Signed-off-by: Julian Stecklina <jsteckli@os.inf.tu-dresden.de>
> ---
>  arch/x86/kvm/cpuid.c |  1 +
>  arch/x86/kvm/svm.c   |  7 +++++--
>  arch/x86/kvm/vmx.c   |  7 +------
>  arch/x86/kvm/x86.c   | 12 +++++++++++-
>  4 files changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 0595f13..aa468c2 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -659,6 +659,7 @@ void kvm_cpuid(struct kvm_vcpu *vcpu, u32 *eax, u32 *ebx, u32 *ecx, u32 *edx)
>  	} else
>  		*eax = *ebx = *ecx = *edx = 0;
>  }
> +EXPORT_SYMBOL_GPL(kvm_cpuid);
>  
>  void kvm_emulate_cpuid(struct kvm_vcpu *vcpu)
>  {
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index baead95..642213a 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -20,6 +20,7 @@
>  #include "mmu.h"
>  #include "kvm_cache_regs.h"
>  #include "x86.h"
> +#include "cpuid.h"
>  
>  #include <linux/module.h>
>  #include <linux/mod_devicetable.h>
> @@ -1190,6 +1191,7 @@ static void init_vmcb(struct vcpu_svm *svm)
>  static int svm_vcpu_reset(struct kvm_vcpu *vcpu)
>  {
>  	struct vcpu_svm *svm = to_svm(vcpu);
> +	u32 dummy, eax;
>  
>  	init_vmcb(svm);
>  
> @@ -1198,8 +1200,9 @@ static int svm_vcpu_reset(struct kvm_vcpu *vcpu)
>  		svm->vmcb->save.cs.base = svm->vcpu.arch.sipi_vector << 12;
>  		svm->vmcb->save.cs.selector = svm->vcpu.arch.sipi_vector << 8;
>  	}
> -	vcpu->arch.regs_avail = ~0;
> -	vcpu->arch.regs_dirty = ~0;
> +
> +	kvm_cpuid(vcpu, &eax, &dummy, &dummy, &dummy);
You are passing uninitialized eax here. It is used by kvm_cpuid().
Should set it to 1.

> +	kvm_register_write(vcpu, VCPU_REGS_RDX, eax);
>  
>  	return 0;
>  }
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index ff66a3b..363fb03 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -3942,7 +3942,7 @@ static int vmx_vcpu_reset(struct kvm_vcpu *vcpu)
>  	u64 msr;
>  	int ret;
>  
> -	vcpu->arch.regs_avail = ~((1 << VCPU_REGS_RIP) | (1 << VCPU_REGS_RSP));
> +	vcpu->arch.regs_avail = ~(1 << VCPU_REGS_RIP);
VCPU_REGS_RIP is part of registers array too, why not dropping the line?

>  
>  	vmx->rmode.vm86_active = 0;
>  
> @@ -3955,10 +3955,6 @@ static int vmx_vcpu_reset(struct kvm_vcpu *vcpu)
>  		msr |= MSR_IA32_APICBASE_BSP;
>  	kvm_set_apic_base(&vmx->vcpu, msr);
>  
> -	ret = fx_init(&vmx->vcpu);
> -	if (ret != 0)
> -		goto out;
> -
>  	vmx_segment_cache_clear(vmx);
>  
>  	seg_setup(VCPU_SREG_CS);
> @@ -3999,7 +3995,6 @@ static int vmx_vcpu_reset(struct kvm_vcpu *vcpu)
>  		kvm_rip_write(vcpu, 0xfff0);
>  	else
>  		kvm_rip_write(vcpu, 0);
> -	kvm_register_write(vcpu, VCPU_REGS_RSP, 0);
>  
>  	vmcs_writel(GUEST_DR7, 0x400);
>  
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 2966c84..9640ea5 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -6045,6 +6045,8 @@ void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
>  
>  int kvm_arch_vcpu_reset(struct kvm_vcpu *vcpu)
>  {
> +	int ret;
> +
>  	atomic_set(&vcpu->arch.nmi_queued, 0);
>  	vcpu->arch.nmi_pending = 0;
>  	vcpu->arch.nmi_injected = false;
> @@ -6066,7 +6068,15 @@ int kvm_arch_vcpu_reset(struct kvm_vcpu *vcpu)
>  
>  	kvm_pmu_reset(vcpu);
>  
> -	return kvm_x86_ops->vcpu_reset(vcpu);
> +	memset(vcpu->arch.regs, 0, sizeof(vcpu->arch.regs));
> +	vcpu->arch.regs_avail = ~0;
> +	vcpu->arch.regs_dirty = ~0;
> +
> +	if ((ret = fx_init(vcpu)) != 0) goto out;
> +
> +	ret = kvm_x86_ops->vcpu_reset(vcpu);
> +out:
> +	return ret;
>  }
>  
>  int kvm_arch_hardware_enable(void *garbage)
> -- 
> 1.7.11.7

--
			Gleb.

[PATCH v3] KVM: x86: Make register state after reset conform to specification

[Julian Stecklina]

VMX behaves now as SVM wrt to FPU initialization. Code has been moved to
generic code path. General-purpose registers are now cleared on reset and
INIT.  SVM code properly initializes EDX.

Signed-off-by: Julian Stecklina <jsteckli@os.inf.tu-dresden.de>
---
 arch/x86/kvm/cpuid.c |  1 +
 arch/x86/kvm/svm.c   | 14 ++++++--------
 arch/x86/kvm/vmx.c   |  7 -------
 arch/x86/kvm/x86.c   |  8 ++++++++
 4 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 0595f13..aa468c2 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -659,6 +659,7 @@ void kvm_cpuid(struct kvm_vcpu *vcpu, u32 *eax, u32 *ebx, u32 *ecx, u32 *edx)
 	} else
 		*eax = *ebx = *ecx = *edx = 0;
 }
+EXPORT_SYMBOL_GPL(kvm_cpuid);
 
 void kvm_emulate_cpuid(struct kvm_vcpu *vcpu)
 {
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index baead95..6c50121 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -20,6 +20,7 @@
 #include "mmu.h"
 #include "kvm_cache_regs.h"
 #include "x86.h"
+#include "cpuid.h"
 
 #include <linux/module.h>
 #include <linux/mod_devicetable.h>
@@ -1190,6 +1191,8 @@ static void init_vmcb(struct vcpu_svm *svm)
 static int svm_vcpu_reset(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
+	u32 dummy;
+	u32 eax = 1;
 
 	init_vmcb(svm);
 
@@ -1198,8 +1201,9 @@ static int svm_vcpu_reset(struct kvm_vcpu *vcpu)
 		svm->vmcb->save.cs.base = svm->vcpu.arch.sipi_vector << 12;
 		svm->vmcb->save.cs.selector = svm->vcpu.arch.sipi_vector << 8;
 	}
-	vcpu->arch.regs_avail = ~0;
-	vcpu->arch.regs_dirty = ~0;
+
+	kvm_cpuid(vcpu, &eax, &dummy, &dummy, &dummy);
+	kvm_register_write(vcpu, VCPU_REGS_RDX, eax);
 
 	return 0;
 }
@@ -1257,10 +1261,6 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm, unsigned int id)
 	init_vmcb(svm);
 	kvm_write_tsc(&svm->vcpu, 0);
 
-	err = fx_init(&svm->vcpu);
-	if (err)
-		goto free_page4;
-
 	svm->vcpu.arch.apic_base = 0xfee00000 | MSR_IA32_APICBASE_ENABLE;
 	if (kvm_vcpu_is_bsp(&svm->vcpu))
 		svm->vcpu.arch.apic_base |= MSR_IA32_APICBASE_BSP;
@@ -1269,8 +1269,6 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm, unsigned int id)
 
 	return &svm->vcpu;
 
-free_page4:
-	__free_page(hsave_page);
 free_page3:
 	__free_pages(nested_msrpm_pages, MSRPM_ALLOC_ORDER);
 free_page2:
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index ff66a3b..85cecfe 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -3942,8 +3942,6 @@ static int vmx_vcpu_reset(struct kvm_vcpu *vcpu)
 	u64 msr;
 	int ret;
 
-	vcpu->arch.regs_avail = ~((1 << VCPU_REGS_RIP) | (1 << VCPU_REGS_RSP));
-
 	vmx->rmode.vm86_active = 0;
 
 	vmx->soft_vnmi_blocked = 0;
@@ -3955,10 +3953,6 @@ static int vmx_vcpu_reset(struct kvm_vcpu *vcpu)
 		msr |= MSR_IA32_APICBASE_BSP;
 	kvm_set_apic_base(&vmx->vcpu, msr);
 
-	ret = fx_init(&vmx->vcpu);
-	if (ret != 0)
-		goto out;
-
 	vmx_segment_cache_clear(vmx);
 
 	seg_setup(VCPU_SREG_CS);
@@ -3999,7 +3993,6 @@ static int vmx_vcpu_reset(struct kvm_vcpu *vcpu)
 		kvm_rip_write(vcpu, 0xfff0);
 	else
 		kvm_rip_write(vcpu, 0);
-	kvm_register_write(vcpu, VCPU_REGS_RSP, 0);
 
 	vmcs_writel(GUEST_DR7, 0x400);
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 2966c84..2e031bf 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -6066,6 +6066,10 @@ int kvm_arch_vcpu_reset(struct kvm_vcpu *vcpu)
 
 	kvm_pmu_reset(vcpu);
 
+	memset(vcpu->arch.regs, 0, sizeof(vcpu->arch.regs));
+	vcpu->arch.regs_avail = ~0;
+	vcpu->arch.regs_dirty = ~0;
+
 	return kvm_x86_ops->vcpu_reset(vcpu);
 }
 
@@ -6229,6 +6233,10 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
 	if (!zalloc_cpumask_var(&vcpu->arch.wbinvd_dirty_mask, GFP_KERNEL))
 		goto fail_free_mce_banks;
 
+	r = fx_init(vcpu);
+	if (r)
+		goto fail_free_mce_banks;
+
 	kvm_async_pf_hash_reset(vcpu);
 	kvm_pmu_init(vcpu);
 
-- 
1.7.11.7

Re: [PATCH v3] KVM: x86: Make register state after reset conform to specification

[Gleb Natapov]

On Tue, Dec 04, 2012 at 01:13:30PM +0100, Julian Stecklina wrote:
> VMX behaves now as SVM wrt to FPU initialization. Code has been moved to
> generic code path. General-purpose registers are now cleared on reset and
> INIT.  SVM code properly initializes EDX.
> 
Looks good overall, small bug bellow and regenerate the patch against
git://git.kernel.org/pub/scm/virt/kvm/kvm.git queue branch please.

> Signed-off-by: Julian Stecklina <jsteckli@os.inf.tu-dresden.de>
> ---
>  arch/x86/kvm/cpuid.c |  1 +
>  arch/x86/kvm/svm.c   | 14 ++++++--------
>  arch/x86/kvm/vmx.c   |  7 -------
>  arch/x86/kvm/x86.c   |  8 ++++++++
>  4 files changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 0595f13..aa468c2 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -659,6 +659,7 @@ void kvm_cpuid(struct kvm_vcpu *vcpu, u32 *eax, u32 *ebx, u32 *ecx, u32 *edx)
>  	} else
>  		*eax = *ebx = *ecx = *edx = 0;
>  }
> +EXPORT_SYMBOL_GPL(kvm_cpuid);
>  
>  void kvm_emulate_cpuid(struct kvm_vcpu *vcpu)
>  {
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index baead95..6c50121 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -20,6 +20,7 @@
>  #include "mmu.h"
>  #include "kvm_cache_regs.h"
>  #include "x86.h"
> +#include "cpuid.h"
>  
>  #include <linux/module.h>
>  #include <linux/mod_devicetable.h>
> @@ -1190,6 +1191,8 @@ static void init_vmcb(struct vcpu_svm *svm)
>  static int svm_vcpu_reset(struct kvm_vcpu *vcpu)
>  {
>  	struct vcpu_svm *svm = to_svm(vcpu);
> +	u32 dummy;
> +	u32 eax = 1;
>  
>  	init_vmcb(svm);
>  
> @@ -1198,8 +1201,9 @@ static int svm_vcpu_reset(struct kvm_vcpu *vcpu)
>  		svm->vmcb->save.cs.base = svm->vcpu.arch.sipi_vector << 12;
>  		svm->vmcb->save.cs.selector = svm->vcpu.arch.sipi_vector << 8;
>  	}
> -	vcpu->arch.regs_avail = ~0;
> -	vcpu->arch.regs_dirty = ~0;
> +
> +	kvm_cpuid(vcpu, &eax, &dummy, &dummy, &dummy);
> +	kvm_register_write(vcpu, VCPU_REGS_RDX, eax);
>  
>  	return 0;
>  }
> @@ -1257,10 +1261,6 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm, unsigned int id)
>  	init_vmcb(svm);
>  	kvm_write_tsc(&svm->vcpu, 0);
>  
> -	err = fx_init(&svm->vcpu);
> -	if (err)
> -		goto free_page4;
> -
>  	svm->vcpu.arch.apic_base = 0xfee00000 | MSR_IA32_APICBASE_ENABLE;
>  	if (kvm_vcpu_is_bsp(&svm->vcpu))
>  		svm->vcpu.arch.apic_base |= MSR_IA32_APICBASE_BSP;
> @@ -1269,8 +1269,6 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm, unsigned int id)
>  
>  	return &svm->vcpu;
>  
> -free_page4:
> -	__free_page(hsave_page);
>  free_page3:
>  	__free_pages(nested_msrpm_pages, MSRPM_ALLOC_ORDER);
>  free_page2:
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index ff66a3b..85cecfe 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -3942,8 +3942,6 @@ static int vmx_vcpu_reset(struct kvm_vcpu *vcpu)
>  	u64 msr;
>  	int ret;
>  
> -	vcpu->arch.regs_avail = ~((1 << VCPU_REGS_RIP) | (1 << VCPU_REGS_RSP));
> -
>  	vmx->rmode.vm86_active = 0;
>  
>  	vmx->soft_vnmi_blocked = 0;
> @@ -3955,10 +3953,6 @@ static int vmx_vcpu_reset(struct kvm_vcpu *vcpu)
>  		msr |= MSR_IA32_APICBASE_BSP;
>  	kvm_set_apic_base(&vmx->vcpu, msr);
>  
> -	ret = fx_init(&vmx->vcpu);
> -	if (ret != 0)
> -		goto out;
> -
>  	vmx_segment_cache_clear(vmx);
>  
>  	seg_setup(VCPU_SREG_CS);
> @@ -3999,7 +3993,6 @@ static int vmx_vcpu_reset(struct kvm_vcpu *vcpu)
>  		kvm_rip_write(vcpu, 0xfff0);
>  	else
>  		kvm_rip_write(vcpu, 0);
> -	kvm_register_write(vcpu, VCPU_REGS_RSP, 0);
>  
>  	vmcs_writel(GUEST_DR7, 0x400);
>  
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 2966c84..2e031bf 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -6066,6 +6066,10 @@ int kvm_arch_vcpu_reset(struct kvm_vcpu *vcpu)
>  
>  	kvm_pmu_reset(vcpu);
>  
> +	memset(vcpu->arch.regs, 0, sizeof(vcpu->arch.regs));
> +	vcpu->arch.regs_avail = ~0;
> +	vcpu->arch.regs_dirty = ~0;
> +
>  	return kvm_x86_ops->vcpu_reset(vcpu);
>  }
>  
> @@ -6229,6 +6233,10 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
>  	if (!zalloc_cpumask_var(&vcpu->arch.wbinvd_dirty_mask, GFP_KERNEL))
>  		goto fail_free_mce_banks;
>  
> +	r = fx_init(vcpu);
> +	if (r)
> +		goto fail_free_mce_banks;
> +
You need to free vcpu->arch.wbinvd_dirty_mask in case fx_init() fails.

>  	kvm_async_pf_hash_reset(vcpu);
>  	kvm_pmu_init(vcpu);
>  
> -- 
> 1.7.11.7

--
			Gleb.

[PATCH v4] KVM: x86: Make register state after reset conform to specification

[Julian Stecklina]

VMX behaves now as SVM wrt to FPU initialization. Code has been moved to
generic code path. General-purpose registers are now cleared on reset and
INIT.  SVM code properly initializes EDX.

Signed-off-by: Julian Stecklina <jsteckli@os.inf.tu-dresden.de>
---
 arch/x86/kvm/cpuid.c |  1 +
 arch/x86/kvm/svm.c   | 14 ++++++--------
 arch/x86/kvm/vmx.c   |  7 -------
 arch/x86/kvm/x86.c   | 10 ++++++++++
 4 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 52f6166..a20ecb5 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -661,6 +661,7 @@ void kvm_cpuid(struct kvm_vcpu *vcpu, u32 *eax, u32 *ebx, u32 *ecx, u32 *edx)
 	} else
 		*eax = *ebx = *ecx = *edx = 0;
 }
+EXPORT_SYMBOL_GPL(kvm_cpuid);
 
 void kvm_emulate_cpuid(struct kvm_vcpu *vcpu)
 {
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index dcb79527..d29d3cd 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -20,6 +20,7 @@
 #include "mmu.h"
 #include "kvm_cache_regs.h"
 #include "x86.h"
+#include "cpuid.h"
 
 #include <linux/module.h>
 #include <linux/mod_devicetable.h>
@@ -1193,6 +1194,8 @@ static void init_vmcb(struct vcpu_svm *svm)
 static int svm_vcpu_reset(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
+	u32 dummy;
+	u32 eax = 1;
 
 	init_vmcb(svm);
 
@@ -1201,8 +1204,9 @@ static int svm_vcpu_reset(struct kvm_vcpu *vcpu)
 		svm->vmcb->save.cs.base = svm->vcpu.arch.sipi_vector << 12;
 		svm->vmcb->save.cs.selector = svm->vcpu.arch.sipi_vector << 8;
 	}
-	vcpu->arch.regs_avail = ~0;
-	vcpu->arch.regs_dirty = ~0;
+
+	kvm_cpuid(vcpu, &eax, &dummy, &dummy, &dummy);
+	kvm_register_write(vcpu, VCPU_REGS_RDX, eax);
 
 	return 0;
 }
@@ -1259,10 +1263,6 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm, unsigned int id)
 	svm->asid_generation = 0;
 	init_vmcb(svm);
 
-	err = fx_init(&svm->vcpu);
-	if (err)
-		goto free_page4;
-
 	svm->vcpu.arch.apic_base = 0xfee00000 | MSR_IA32_APICBASE_ENABLE;
 	if (kvm_vcpu_is_bsp(&svm->vcpu))
 		svm->vcpu.arch.apic_base |= MSR_IA32_APICBASE_BSP;
@@ -1271,8 +1271,6 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm, unsigned int id)
 
 	return &svm->vcpu;
 
-free_page4:
-	__free_page(hsave_page);
 free_page3:
 	__free_pages(nested_msrpm_pages, MSRPM_ALLOC_ORDER);
 free_page2:
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 2fd2046..4ea3cb3 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -3934,8 +3934,6 @@ static int vmx_vcpu_reset(struct kvm_vcpu *vcpu)
 	u64 msr;
 	int ret;
 
-	vcpu->arch.regs_avail = ~((1 << VCPU_REGS_RIP) | (1 << VCPU_REGS_RSP));
-
 	vmx->rmode.vm86_active = 0;
 
 	vmx->soft_vnmi_blocked = 0;
@@ -3947,10 +3945,6 @@ static int vmx_vcpu_reset(struct kvm_vcpu *vcpu)
 		msr |= MSR_IA32_APICBASE_BSP;
 	kvm_set_apic_base(&vmx->vcpu, msr);
 
-	ret = fx_init(&vmx->vcpu);
-	if (ret != 0)
-		goto out;
-
 	vmx_segment_cache_clear(vmx);
 
 	seg_setup(VCPU_SREG_CS);
@@ -3991,7 +3985,6 @@ static int vmx_vcpu_reset(struct kvm_vcpu *vcpu)
 		kvm_rip_write(vcpu, 0xfff0);
 	else
 		kvm_rip_write(vcpu, 0);
-	kvm_register_write(vcpu, VCPU_REGS_RSP, 0);
 
 	vmcs_writel(GUEST_GDTR_BASE, 0);
 	vmcs_write32(GUEST_GDTR_LIMIT, 0xffff);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 3bdaf29..57c76e8 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -6461,6 +6461,10 @@ static int kvm_vcpu_reset(struct kvm_vcpu *vcpu)
 
 	kvm_pmu_reset(vcpu);
 
+	memset(vcpu->arch.regs, 0, sizeof(vcpu->arch.regs));
+	vcpu->arch.regs_avail = ~0;
+	vcpu->arch.regs_dirty = ~0;
+
 	return kvm_x86_ops->vcpu_reset(vcpu);
 }
 
@@ -6629,11 +6633,17 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
 	if (!zalloc_cpumask_var(&vcpu->arch.wbinvd_dirty_mask, GFP_KERNEL))
 		goto fail_free_mce_banks;
 
+	r = fx_init(vcpu);
+	if (r)
+		goto fail_free_wbinvd_dirty_mask;
+
 	vcpu->arch.ia32_tsc_adjust_msr = 0x0;
 	kvm_async_pf_hash_reset(vcpu);
 	kvm_pmu_init(vcpu);
 
 	return 0;
+fail_free_wbinvd_dirty_mask:
+	free_cpumask_var(vcpu->arch.wbinvd_dirty_mask);
 fail_free_mce_banks:
 	kfree(vcpu->arch.mce_banks);
 fail_free_lapic:
-- 
1.7.11.7

Re: [PATCH v4] KVM: x86: Make register state after reset conform to specification

[Gleb Natapov]

On Wed, Dec 05, 2012 at 01:00:59PM +0100, Julian Stecklina wrote:
> VMX behaves now as SVM wrt to FPU initialization. Code has been moved to
> generic code path. General-purpose registers are now cleared on reset and
> INIT.  SVM code properly initializes EDX.
> 
> Signed-off-by: Julian Stecklina <jsteckli@os.inf.tu-dresden.de>
> ---
>  arch/x86/kvm/cpuid.c |  1 +
>  arch/x86/kvm/svm.c   | 14 ++++++--------
>  arch/x86/kvm/vmx.c   |  7 -------
>  arch/x86/kvm/x86.c   | 10 ++++++++++
>  4 files changed, 17 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 52f6166..a20ecb5 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -661,6 +661,7 @@ void kvm_cpuid(struct kvm_vcpu *vcpu, u32 *eax, u32 *ebx, u32 *ecx, u32 *edx)
>  	} else
>  		*eax = *ebx = *ecx = *edx = 0;
>  }
> +EXPORT_SYMBOL_GPL(kvm_cpuid);
>  
>  void kvm_emulate_cpuid(struct kvm_vcpu *vcpu)
>  {
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index dcb79527..d29d3cd 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -20,6 +20,7 @@
>  #include "mmu.h"
>  #include "kvm_cache_regs.h"
>  #include "x86.h"
> +#include "cpuid.h"
>  
>  #include <linux/module.h>
>  #include <linux/mod_devicetable.h>
> @@ -1193,6 +1194,8 @@ static void init_vmcb(struct vcpu_svm *svm)
>  static int svm_vcpu_reset(struct kvm_vcpu *vcpu)
>  {
>  	struct vcpu_svm *svm = to_svm(vcpu);
> +	u32 dummy;
> +	u32 eax = 1;
>  
>  	init_vmcb(svm);
>  
> @@ -1201,8 +1204,9 @@ static int svm_vcpu_reset(struct kvm_vcpu *vcpu)
>  		svm->vmcb->save.cs.base = svm->vcpu.arch.sipi_vector << 12;
>  		svm->vmcb->save.cs.selector = svm->vcpu.arch.sipi_vector << 8;
>  	}
> -	vcpu->arch.regs_avail = ~0;
> -	vcpu->arch.regs_dirty = ~0;
> +
> +	kvm_cpuid(vcpu, &eax, &dummy, &dummy, &dummy);
> +	kvm_register_write(vcpu, VCPU_REGS_RDX, eax);
>  
>  	return 0;
>  }
> @@ -1259,10 +1263,6 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm, unsigned int id)
>  	svm->asid_generation = 0;
>  	init_vmcb(svm);
>  
> -	err = fx_init(&svm->vcpu);
> -	if (err)
> -		goto free_page4;
> -
>  	svm->vcpu.arch.apic_base = 0xfee00000 | MSR_IA32_APICBASE_ENABLE;
>  	if (kvm_vcpu_is_bsp(&svm->vcpu))
>  		svm->vcpu.arch.apic_base |= MSR_IA32_APICBASE_BSP;
> @@ -1271,8 +1271,6 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm, unsigned int id)
>  
>  	return &svm->vcpu;
>  
> -free_page4:
> -	__free_page(hsave_page);
>  free_page3:
>  	__free_pages(nested_msrpm_pages, MSRPM_ALLOC_ORDER);
>  free_page2:
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 2fd2046..4ea3cb3 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -3934,8 +3934,6 @@ static int vmx_vcpu_reset(struct kvm_vcpu *vcpu)
>  	u64 msr;
>  	int ret;
>  
> -	vcpu->arch.regs_avail = ~((1 << VCPU_REGS_RIP) | (1 << VCPU_REGS_RSP));
> -
>  	vmx->rmode.vm86_active = 0;
>  
>  	vmx->soft_vnmi_blocked = 0;
> @@ -3947,10 +3945,6 @@ static int vmx_vcpu_reset(struct kvm_vcpu *vcpu)
>  		msr |= MSR_IA32_APICBASE_BSP;
>  	kvm_set_apic_base(&vmx->vcpu, msr);
>  
> -	ret = fx_init(&vmx->vcpu);
> -	if (ret != 0)
> -		goto out;
> -
Label "out" is now unused. Compiler complains.

>  	vmx_segment_cache_clear(vmx);
>  
>  	seg_setup(VCPU_SREG_CS);
> @@ -3991,7 +3985,6 @@ static int vmx_vcpu_reset(struct kvm_vcpu *vcpu)
>  		kvm_rip_write(vcpu, 0xfff0);
>  	else
>  		kvm_rip_write(vcpu, 0);
> -	kvm_register_write(vcpu, VCPU_REGS_RSP, 0);
>  
>  	vmcs_writel(GUEST_GDTR_BASE, 0);
>  	vmcs_write32(GUEST_GDTR_LIMIT, 0xffff);
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 3bdaf29..57c76e8 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -6461,6 +6461,10 @@ static int kvm_vcpu_reset(struct kvm_vcpu *vcpu)
>  
>  	kvm_pmu_reset(vcpu);
>  
> +	memset(vcpu->arch.regs, 0, sizeof(vcpu->arch.regs));
> +	vcpu->arch.regs_avail = ~0;
> +	vcpu->arch.regs_dirty = ~0;
> +
>  	return kvm_x86_ops->vcpu_reset(vcpu);
>  }
>  
> @@ -6629,11 +6633,17 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
>  	if (!zalloc_cpumask_var(&vcpu->arch.wbinvd_dirty_mask, GFP_KERNEL))
>  		goto fail_free_mce_banks;
>  
> +	r = fx_init(vcpu);
> +	if (r)
> +		goto fail_free_wbinvd_dirty_mask;
> +
>  	vcpu->arch.ia32_tsc_adjust_msr = 0x0;
>  	kvm_async_pf_hash_reset(vcpu);
>  	kvm_pmu_init(vcpu);
>  
>  	return 0;
> +fail_free_wbinvd_dirty_mask:
> +	free_cpumask_var(vcpu->arch.wbinvd_dirty_mask);
>  fail_free_mce_banks:
>  	kfree(vcpu->arch.mce_banks);
>  fail_free_lapic:
> -- 
> 1.7.11.7

--
			Gleb.

[PATCH v5] KVM: x86: Make register state after reset conform to specification

[Julian Stecklina]

VMX behaves now as SVM wrt to FPU initialization. Code has been moved to
generic code path. General-purpose registers are now cleared on reset and
INIT.  SVM code properly initializes EDX.

Signed-off-by: Julian Stecklina <jsteckli@os.inf.tu-dresden.de>
---
 arch/x86/kvm/cpuid.c |  1 +
 arch/x86/kvm/svm.c   | 14 ++++++--------
 arch/x86/kvm/vmx.c   |  8 --------
 arch/x86/kvm/x86.c   | 10 ++++++++++
 4 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 52f6166..a20ecb5 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -661,6 +661,7 @@ void kvm_cpuid(struct kvm_vcpu *vcpu, u32 *eax, u32 *ebx, u32 *ecx, u32 *edx)
 	} else
 		*eax = *ebx = *ecx = *edx = 0;
 }
+EXPORT_SYMBOL_GPL(kvm_cpuid);
 
 void kvm_emulate_cpuid(struct kvm_vcpu *vcpu)
 {
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index dcb79527..d29d3cd 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -20,6 +20,7 @@
 #include "mmu.h"
 #include "kvm_cache_regs.h"
 #include "x86.h"
+#include "cpuid.h"
 
 #include <linux/module.h>
 #include <linux/mod_devicetable.h>
@@ -1193,6 +1194,8 @@ static void init_vmcb(struct vcpu_svm *svm)
 static int svm_vcpu_reset(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
+	u32 dummy;
+	u32 eax = 1;
 
 	init_vmcb(svm);
 
@@ -1201,8 +1204,9 @@ static int svm_vcpu_reset(struct kvm_vcpu *vcpu)
 		svm->vmcb->save.cs.base = svm->vcpu.arch.sipi_vector << 12;
 		svm->vmcb->save.cs.selector = svm->vcpu.arch.sipi_vector << 8;
 	}
-	vcpu->arch.regs_avail = ~0;
-	vcpu->arch.regs_dirty = ~0;
+
+	kvm_cpuid(vcpu, &eax, &dummy, &dummy, &dummy);
+	kvm_register_write(vcpu, VCPU_REGS_RDX, eax);
 
 	return 0;
 }
@@ -1259,10 +1263,6 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm, unsigned int id)
 	svm->asid_generation = 0;
 	init_vmcb(svm);
 
-	err = fx_init(&svm->vcpu);
-	if (err)
-		goto free_page4;
-
 	svm->vcpu.arch.apic_base = 0xfee00000 | MSR_IA32_APICBASE_ENABLE;
 	if (kvm_vcpu_is_bsp(&svm->vcpu))
 		svm->vcpu.arch.apic_base |= MSR_IA32_APICBASE_BSP;
@@ -1271,8 +1271,6 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm, unsigned int id)
 
 	return &svm->vcpu;
 
-free_page4:
-	__free_page(hsave_page);
 free_page3:
 	__free_pages(nested_msrpm_pages, MSRPM_ALLOC_ORDER);
 free_page2:
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 2fd2046..6adbad6 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -3934,8 +3934,6 @@ static int vmx_vcpu_reset(struct kvm_vcpu *vcpu)
 	u64 msr;
 	int ret;
 
-	vcpu->arch.regs_avail = ~((1 << VCPU_REGS_RIP) | (1 << VCPU_REGS_RSP));
-
 	vmx->rmode.vm86_active = 0;
 
 	vmx->soft_vnmi_blocked = 0;
@@ -3947,10 +3945,6 @@ static int vmx_vcpu_reset(struct kvm_vcpu *vcpu)
 		msr |= MSR_IA32_APICBASE_BSP;
 	kvm_set_apic_base(&vmx->vcpu, msr);
 
-	ret = fx_init(&vmx->vcpu);
-	if (ret != 0)
-		goto out;
-
 	vmx_segment_cache_clear(vmx);
 
 	seg_setup(VCPU_SREG_CS);
@@ -3991,7 +3985,6 @@ static int vmx_vcpu_reset(struct kvm_vcpu *vcpu)
 		kvm_rip_write(vcpu, 0xfff0);
 	else
 		kvm_rip_write(vcpu, 0);
-	kvm_register_write(vcpu, VCPU_REGS_RSP, 0);
 
 	vmcs_writel(GUEST_GDTR_BASE, 0);
 	vmcs_write32(GUEST_GDTR_LIMIT, 0xffff);
@@ -4041,7 +4034,6 @@ static int vmx_vcpu_reset(struct kvm_vcpu *vcpu)
 	/* HACK: Don't enable emulation on guest boot/reset */
 	vmx->emulation_required = 0;
 
-out:
 	return ret;
 }
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 3bdaf29..57c76e8 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -6461,6 +6461,10 @@ static int kvm_vcpu_reset(struct kvm_vcpu *vcpu)
 
 	kvm_pmu_reset(vcpu);
 
+	memset(vcpu->arch.regs, 0, sizeof(vcpu->arch.regs));
+	vcpu->arch.regs_avail = ~0;
+	vcpu->arch.regs_dirty = ~0;
+
 	return kvm_x86_ops->vcpu_reset(vcpu);
 }
 
@@ -6629,11 +6633,17 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
 	if (!zalloc_cpumask_var(&vcpu->arch.wbinvd_dirty_mask, GFP_KERNEL))
 		goto fail_free_mce_banks;
 
+	r = fx_init(vcpu);
+	if (r)
+		goto fail_free_wbinvd_dirty_mask;
+
 	vcpu->arch.ia32_tsc_adjust_msr = 0x0;
 	kvm_async_pf_hash_reset(vcpu);
 	kvm_pmu_init(vcpu);
 
 	return 0;
+fail_free_wbinvd_dirty_mask:
+	free_cpumask_var(vcpu->arch.wbinvd_dirty_mask);
 fail_free_mce_banks:
 	kfree(vcpu->arch.mce_banks);
 fail_free_lapic:
-- 
1.7.11.7

Re: [PATCH v4] KVM: x86: Make register state after reset conform to specification

[Julian Stecklina]

Thus spake Gleb Natapov <gleb@redhat.com>:

>> -	ret = fx_init(&vmx->vcpu);
>> -	if (ret != 0)
>> -		goto out;
>> -
> Label "out" is now unused. Compiler complains.

5th time's the charm. ;) Patch is updated.

Julian

Re: [PATCH v5] KVM: x86: Make register state after reset conform to specification

[Gleb Natapov]

On Wed, Dec 05, 2012 at 03:26:19PM +0100, Julian Stecklina wrote:
> VMX behaves now as SVM wrt to FPU initialization. Code has been moved to
> generic code path. General-purpose registers are now cleared on reset and
> INIT.  SVM code properly initializes EDX.
> 
> Signed-off-by: Julian Stecklina <jsteckli@os.inf.tu-dresden.de>
Applied to queue. Thanks.

> ---
>  arch/x86/kvm/cpuid.c |  1 +
>  arch/x86/kvm/svm.c   | 14 ++++++--------
>  arch/x86/kvm/vmx.c   |  8 --------
>  arch/x86/kvm/x86.c   | 10 ++++++++++
>  4 files changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 52f6166..a20ecb5 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -661,6 +661,7 @@ void kvm_cpuid(struct kvm_vcpu *vcpu, u32 *eax, u32 *ebx, u32 *ecx, u32 *edx)
>  	} else
>  		*eax = *ebx = *ecx = *edx = 0;
>  }
> +EXPORT_SYMBOL_GPL(kvm_cpuid);
>  
>  void kvm_emulate_cpuid(struct kvm_vcpu *vcpu)
>  {
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index dcb79527..d29d3cd 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -20,6 +20,7 @@
>  #include "mmu.h"
>  #include "kvm_cache_regs.h"
>  #include "x86.h"
> +#include "cpuid.h"
>  
>  #include <linux/module.h>
>  #include <linux/mod_devicetable.h>
> @@ -1193,6 +1194,8 @@ static void init_vmcb(struct vcpu_svm *svm)
>  static int svm_vcpu_reset(struct kvm_vcpu *vcpu)
>  {
>  	struct vcpu_svm *svm = to_svm(vcpu);
> +	u32 dummy;
> +	u32 eax = 1;
>  
>  	init_vmcb(svm);
>  
> @@ -1201,8 +1204,9 @@ static int svm_vcpu_reset(struct kvm_vcpu *vcpu)
>  		svm->vmcb->save.cs.base = svm->vcpu.arch.sipi_vector << 12;
>  		svm->vmcb->save.cs.selector = svm->vcpu.arch.sipi_vector << 8;
>  	}
> -	vcpu->arch.regs_avail = ~0;
> -	vcpu->arch.regs_dirty = ~0;
> +
> +	kvm_cpuid(vcpu, &eax, &dummy, &dummy, &dummy);
> +	kvm_register_write(vcpu, VCPU_REGS_RDX, eax);
>  
>  	return 0;
>  }
> @@ -1259,10 +1263,6 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm, unsigned int id)
>  	svm->asid_generation = 0;
>  	init_vmcb(svm);
>  
> -	err = fx_init(&svm->vcpu);
> -	if (err)
> -		goto free_page4;
> -
>  	svm->vcpu.arch.apic_base = 0xfee00000 | MSR_IA32_APICBASE_ENABLE;
>  	if (kvm_vcpu_is_bsp(&svm->vcpu))
>  		svm->vcpu.arch.apic_base |= MSR_IA32_APICBASE_BSP;
> @@ -1271,8 +1271,6 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm, unsigned int id)
>  
>  	return &svm->vcpu;
>  
> -free_page4:
> -	__free_page(hsave_page);
>  free_page3:
>  	__free_pages(nested_msrpm_pages, MSRPM_ALLOC_ORDER);
>  free_page2:
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 2fd2046..6adbad6 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -3934,8 +3934,6 @@ static int vmx_vcpu_reset(struct kvm_vcpu *vcpu)
>  	u64 msr;
>  	int ret;
>  
> -	vcpu->arch.regs_avail = ~((1 << VCPU_REGS_RIP) | (1 << VCPU_REGS_RSP));
> -
>  	vmx->rmode.vm86_active = 0;
>  
>  	vmx->soft_vnmi_blocked = 0;
> @@ -3947,10 +3945,6 @@ static int vmx_vcpu_reset(struct kvm_vcpu *vcpu)
>  		msr |= MSR_IA32_APICBASE_BSP;
>  	kvm_set_apic_base(&vmx->vcpu, msr);
>  
> -	ret = fx_init(&vmx->vcpu);
> -	if (ret != 0)
> -		goto out;
> -
>  	vmx_segment_cache_clear(vmx);
>  
>  	seg_setup(VCPU_SREG_CS);
> @@ -3991,7 +3985,6 @@ static int vmx_vcpu_reset(struct kvm_vcpu *vcpu)
>  		kvm_rip_write(vcpu, 0xfff0);
>  	else
>  		kvm_rip_write(vcpu, 0);
> -	kvm_register_write(vcpu, VCPU_REGS_RSP, 0);
>  
>  	vmcs_writel(GUEST_GDTR_BASE, 0);
>  	vmcs_write32(GUEST_GDTR_LIMIT, 0xffff);
> @@ -4041,7 +4034,6 @@ static int vmx_vcpu_reset(struct kvm_vcpu *vcpu)
>  	/* HACK: Don't enable emulation on guest boot/reset */
>  	vmx->emulation_required = 0;
>  
> -out:
>  	return ret;
>  }
>  
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 3bdaf29..57c76e8 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -6461,6 +6461,10 @@ static int kvm_vcpu_reset(struct kvm_vcpu *vcpu)
>  
>  	kvm_pmu_reset(vcpu);
>  
> +	memset(vcpu->arch.regs, 0, sizeof(vcpu->arch.regs));
> +	vcpu->arch.regs_avail = ~0;
> +	vcpu->arch.regs_dirty = ~0;
> +
>  	return kvm_x86_ops->vcpu_reset(vcpu);
>  }
>  
> @@ -6629,11 +6633,17 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
>  	if (!zalloc_cpumask_var(&vcpu->arch.wbinvd_dirty_mask, GFP_KERNEL))
>  		goto fail_free_mce_banks;
>  
> +	r = fx_init(vcpu);
> +	if (r)
> +		goto fail_free_wbinvd_dirty_mask;
> +
>  	vcpu->arch.ia32_tsc_adjust_msr = 0x0;
>  	kvm_async_pf_hash_reset(vcpu);
>  	kvm_pmu_init(vcpu);
>  
>  	return 0;
> +fail_free_wbinvd_dirty_mask:
> +	free_cpumask_var(vcpu->arch.wbinvd_dirty_mask);
>  fail_free_mce_banks:
>  	kfree(vcpu->arch.mce_banks);
>  fail_free_lapic:
> -- 
> 1.7.11.7

--
			Gleb.