Fix PAE guests on KVM 32 bit host

Fix PAE guests on KVM 32 bit host

[Joerg Roedel]

[resend due to PBKAC using git-send-email]

This small series of patches fixes a boot problem with PAE guests on a 32 bit
KVM host. These guests try to access the EFER register when running on AMD, get
an GP and crashing very soon in the boot process. These patches fix that. They
where tested with 32 bit legacy and PAE Linux and Vista 32 bit.

Joerg

diffstat:

 arch/x86/kvm/vmx.c |   10 ++++++----
 arch/x86/kvm/x86.c |   19 ++++++++-----------
 2 files changed, 14 insertions(+), 15 deletions(-)





-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

[PATCH 1/2] VMX: unifdef the EFER specific code

[Joerg Roedel]

To allow access to the EFER register in 32bit KVM the EFER specific code has to
be exported to the x86 generic code. This patch does this in a backwards
compatible manner.

Signed-off-by: Joerg Roedel <joerg.roedel-5C7GfCeVMHo@public.gmane.org>
---
 arch/x86/kvm/vmx.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 3d8949a..2ca24ac 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -759,6 +759,9 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 *pdata)
 		break;
 	case MSR_EFER:
 		return kvm_get_msr_common(vcpu, msr_index, pdata);
+#else
+	case MSR_EFER:
+		return 1;
 #endif
 	case MSR_IA32_TIME_STAMP_COUNTER:
 		data = guest_read_tsc();
@@ -811,6 +814,9 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 data)
 	case MSR_GS_BASE:
 		vmcs_writel(GUEST_GS_BASE, data);
 		break;
+#else
+	case MSR_EFER:
+		return 1;
 #endif
 	case MSR_IA32_SYSENTER_CS:
 		vmcs_write32(GUEST_SYSENTER_CS, data);
@@ -1332,7 +1338,6 @@ static void vmx_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
 	vcpu->arch.cr4 = cr4;
 }
 
-#ifdef CONFIG_X86_64
 
 static void vmx_set_efer(struct kvm_vcpu *vcpu, u64 efer)
 {
@@ -1356,7 +1361,6 @@ static void vmx_set_efer(struct kvm_vcpu *vcpu, u64 efer)
 	setup_msrs(vmx);
 }
 
-#endif
 
 static u64 vmx_get_segment_base(struct kvm_vcpu *vcpu, int seg)
 {
@@ -2662,9 +2666,7 @@ static struct kvm_x86_ops vmx_x86_ops = {
 	.set_cr0 = vmx_set_cr0,
 	.set_cr3 = vmx_set_cr3,
 	.set_cr4 = vmx_set_cr4,
-#ifdef CONFIG_X86_64
 	.set_efer = vmx_set_efer,
-#endif
 	.get_idt = vmx_get_idt,
 	.set_idt = vmx_set_idt,
 	.get_gdt = vmx_get_gdt,
-- 
1.5.3.7




-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH 1/2] VMX: unifdef the EFER specific code

[Avi Kivity]

Joerg Roedel wrote:
> To allow access to the EFER register in 32bit KVM the EFER specific code has to
> be exported to the x86 generic code. This patch does this in a backwards
> compatible manner.
>
> Signed-off-by: Joerg Roedel <joerg.roedel-5C7GfCeVMHo@public.gmane.org>
> ---
>  arch/x86/kvm/vmx.c |   10 ++++++----
>  1 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 3d8949a..2ca24ac 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -759,6 +759,9 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 *pdata)
>  		break;
>  	case MSR_EFER:
>  		return kvm_get_msr_common(vcpu, msr_index, pdata);
> +#else
> +	case MSR_EFER:
> +		return 1;
>  #endif
>   

You're leaving *pdata uninitialized, which would confuse guests that 
read EFER.  But the better solution is to call kvm_get_msr_common().

>  	case MSR_IA32_TIME_STAMP_COUNTER:
>  		data = guest_read_tsc();
> @@ -811,6 +814,9 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 data)
>  	case MSR_GS_BASE:
>  		vmcs_writel(GUEST_GS_BASE, data);
>  		break;
> +#else
> +	case MSR_EFER:
> +		return 1;
>  #endif
>   

And kvm_set_msr_common() here.


-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

[PATCH 2/2] X86: allow access to EFER in 32bit KVM

[Joerg Roedel]

This patch makes the EFER register accessible on a 32bit KVM host. This is
necessary to boot 32 bit PAE guests under SVM.

Signed-off-by: Joerg Roedel <joerg.roedel-5C7GfCeVMHo@public.gmane.org>
---
 arch/x86/kvm/x86.c |   19 ++++++++-----------
 1 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 8f94a0b..4022e13 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -41,7 +41,7 @@
 			  | X86_CR4_OSXMMEXCPT | X86_CR4_VMXE))
 
 #define CR8_RESERVED_BITS (~(unsigned long)X86_CR8_TPR)
-#define EFER_RESERVED_BITS 0xfffffffffffff2fe
+#define EFER_RESERVED_BITS 0xfffffffffffff2feULL
 
 #define VM_STAT(x) offsetof(struct kvm, stat.x), KVM_STAT_VM
 #define VCPU_STAT(x) offsetof(struct kvm_vcpu, stat.x), KVM_STAT_VCPU
@@ -421,8 +421,6 @@ static u32 emulated_msrs[] = {
 	MSR_IA32_MISC_ENABLE,
 };
 
-#ifdef CONFIG_X86_64
-
 static void set_efer(struct kvm_vcpu *vcpu, u64 efer)
 {
 	if (efer & EFER_RESERVED_BITS) {
@@ -432,12 +430,19 @@ static void set_efer(struct kvm_vcpu *vcpu, u64 efer)
 		return;
 	}
 
+#ifdef CONFIG_X86_64
 	if (is_paging(vcpu)
 	    && (vcpu->arch.shadow_efer & EFER_LME) != (efer & EFER_LME)) {
 		printk(KERN_DEBUG "set_efer: #GP, change LME while paging\n");
 		kvm_inject_gp(vcpu, 0);
 		return;
 	}
+#else
+	if (efer & EFER_LME) {
+		printk(KERN_DEBUG "set_efer: #GP, enable longmode on 32bit host\n");
+		kvm_inject_gp(vcpu, 0);
+	}
+#endif
 
 	kvm_x86_ops->set_efer(vcpu, efer);
 
@@ -447,8 +452,6 @@ static void set_efer(struct kvm_vcpu *vcpu, u64 efer)
 	vcpu->arch.shadow_efer = efer;
 }
 
-#endif
-
 /*
  * Writes msr value into into the appropriate "register".
  * Returns 0 on success, non-0 otherwise.
@@ -471,11 +474,9 @@ static int do_set_msr(struct kvm_vcpu *vcpu, unsigned index, u64 *data)
 int kvm_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data)
 {
 	switch (msr) {
-#ifdef CONFIG_X86_64
 	case MSR_EFER:
 		set_efer(vcpu, data);
 		break;
-#endif
 	case MSR_IA32_MC0_STATUS:
 		pr_unimpl(vcpu, "%s: MSR_IA32_MC0_STATUS 0x%llx, nop\n",
 		       __FUNCTION__, data);
@@ -548,11 +549,9 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
 	case MSR_IA32_MISC_ENABLE:
 		data = vcpu->arch.ia32_misc_enable_msr;
 		break;
-#ifdef CONFIG_X86_64
 	case MSR_EFER:
 		data = vcpu->arch.shadow_efer;
 		break;
-#endif
 	default:
 		pr_unimpl(vcpu, "unhandled rdmsr: 0x%x\n", msr);
 		return 1;
@@ -2853,9 +2852,7 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
 	set_cr8(vcpu, sregs->cr8);
 
 	mmu_reset_needed |= vcpu->arch.shadow_efer != sregs->efer;
-#ifdef CONFIG_X86_64
 	kvm_x86_ops->set_efer(vcpu, sregs->efer);
-#endif
 	kvm_set_apic_base(vcpu, sregs->apic_base);
 
 	kvm_x86_ops->decache_cr4_guest_bits(vcpu);
-- 
1.5.3.7




-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH 2/2] X86: allow access to EFER in 32bit KVM

[Avi Kivity]

Joerg Roedel wrote:
> This patch makes the EFER register accessible on a 32bit KVM host. This is
> necessary to boot 32 bit PAE guests under SVM.
>
>   


>  static void set_efer(struct kvm_vcpu *vcpu, u64 efer)
>  {
>  	if (efer & EFER_RESERVED_BITS) {
> @@ -432,12 +430,19 @@ static void set_efer(struct kvm_vcpu *vcpu, u64 efer)
>  		return;
>  	}
>  
> +#ifdef CONFIG_X86_64
>  	if (is_paging(vcpu)
>  	    && (vcpu->arch.shadow_efer & EFER_LME) != (efer & EFER_LME)) {
>  		printk(KERN_DEBUG "set_efer: #GP, change LME while paging\n");
>  		kvm_inject_gp(vcpu, 0);
>  		return;
>  	}
> +#else
> +	if (efer & EFER_LME) {
> +		printk(KERN_DEBUG "set_efer: #GP, enable longmode on 32bit host\n");
> +		kvm_inject_gp(vcpu, 0);
> +	}
> +#endif
>   

We should, in addition, check the various EFER bits against host cpu 
capabilities (with cpu_has()). This will ensure that any attempt to set 
an invalid bit will fail.  On Intel cpus that don't have EFER, there are 
no valid bits, and if we detect that in vmx_set_efer, we can make it 
work on hosts that don't have EFER.

 


-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH 2/2] X86: allow access to EFER in 32bit KVM

[Joerg Roedel]

On Wed, Jan 30, 2008 at 03:11:36PM +0200, Avi Kivity wrote:
> Joerg Roedel wrote:
> >This patch makes the EFER register accessible on a 32bit KVM host. This is
> >necessary to boot 32 bit PAE guests under SVM.
> >
> >  
> 
> 
> > static void set_efer(struct kvm_vcpu *vcpu, u64 efer)
> > {
> > 	if (efer & EFER_RESERVED_BITS) {
> >@@ -432,12 +430,19 @@ static void set_efer(struct kvm_vcpu *vcpu, u64 efer)
> > 		return;
> > 	}
> > +#ifdef CONFIG_X86_64
> > 	if (is_paging(vcpu)
> > 	    && (vcpu->arch.shadow_efer & EFER_LME) != (efer & EFER_LME)) {
> > 		printk(KERN_DEBUG "set_efer: #GP, change LME while paging\n");
> > 		kvm_inject_gp(vcpu, 0);
> > 		return;
> > 	}
> >+#else
> >+	if (efer & EFER_LME) {
> >+		printk(KERN_DEBUG "set_efer: #GP, enable longmode on 32bit host\n");
> >+		kvm_inject_gp(vcpu, 0);
> >+	}
> >+#endif
> >  
> 
> We should, in addition, check the various EFER bits against host cpu capabilities (with cpu_has()). This 
> will ensure that any attempt to set an invalid bit will fail.  On Intel cpus that don't have EFER, there are 
> no valid bits, and if we detect that in vmx_set_efer, we can make it work on hosts that don't have EFER.

Ok, makes sense. I will update the patches.

Joerg

-- 
           |           AMD Saxony Limited Liability Company & Co. KG
 Operating |         Wilschdorfer Landstr. 101, 01109 Dresden, Germany
 System    |                  Register Court Dresden: HRA 4896
 Research  |              General Partner authorized to represent:
 Center    |             AMD Saxony LLC (Wilmington, Delaware, US)
           | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy



-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Fix PAE guests on KVM 32 bit host

[Joerg-hiXKcy4yTwrP0Z7Jsv878P8+0UxHXcjY, Roedel <joerg.roedel]

This small series of patches fixes a boot problem with PAE guests on a 32 bit
KVM host. These guests try to access the EFER register when running on AMD, get
an GP and crashing very soon in the boot process. These patches fix that. They
where tested with 32 bit legacy and PAE Linux and Vista 32 bit.

Joerg

diffstat:

 arch/x86/kvm/vmx.c |   10 ++++++----
 arch/x86/kvm/x86.c |   19 ++++++++-----------
 2 files changed, 14 insertions(+), 15 deletions(-)





-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

[PATCH 1/2] VMX: unifdef the EFER specific code

[Joerg-hiXKcy4yTwrP0Z7Jsv878P8+0UxHXcjY, Roedel <joerg.roedel]

From: Joerg Roedel <joerg.roedel-5C7GfCeVMHo@public.gmane.org>

To allow access to the EFER register in 32bit KVM the EFER specific code has to
be exported to the x86 generic code. This patch does this in a backwards
compatible manner.

Signed-off-by: Joerg Roedel <joerg.roedel-5C7GfCeVMHo@public.gmane.org>
---
 arch/x86/kvm/vmx.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 3d8949a..2ca24ac 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -759,6 +759,9 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 *pdata)
 		break;
 	case MSR_EFER:
 		return kvm_get_msr_common(vcpu, msr_index, pdata);
+#else
+	case MSR_EFER:
+		return 1;
 #endif
 	case MSR_IA32_TIME_STAMP_COUNTER:
 		data = guest_read_tsc();
@@ -811,6 +814,9 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 data)
 	case MSR_GS_BASE:
 		vmcs_writel(GUEST_GS_BASE, data);
 		break;
+#else
+	case MSR_EFER:
+		return 1;
 #endif
 	case MSR_IA32_SYSENTER_CS:
 		vmcs_write32(GUEST_SYSENTER_CS, data);
@@ -1332,7 +1338,6 @@ static void vmx_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
 	vcpu->arch.cr4 = cr4;
 }
 
-#ifdef CONFIG_X86_64
 
 static void vmx_set_efer(struct kvm_vcpu *vcpu, u64 efer)
 {
@@ -1356,7 +1361,6 @@ static void vmx_set_efer(struct kvm_vcpu *vcpu, u64 efer)
 	setup_msrs(vmx);
 }
 
-#endif
 
 static u64 vmx_get_segment_base(struct kvm_vcpu *vcpu, int seg)
 {
@@ -2662,9 +2666,7 @@ static struct kvm_x86_ops vmx_x86_ops = {
 	.set_cr0 = vmx_set_cr0,
 	.set_cr3 = vmx_set_cr3,
 	.set_cr4 = vmx_set_cr4,
-#ifdef CONFIG_X86_64
 	.set_efer = vmx_set_efer,
-#endif
 	.get_idt = vmx_get_idt,
 	.set_idt = vmx_set_idt,
 	.get_gdt = vmx_get_gdt,
-- 
1.5.3.7




-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/