[PATCH v2] Enable CPU SMEP feature for KVM

[PATCH v2] Enable CPU SMEP feature for KVM

[Yang, Wei Y]

[-- Attachment #1: Type: text/plain, Size: 2173 bytes --]

This patch matches with "[PATCH v2] Enable CPU SMEP feature support for QEMU-KVM", no changes since v1.

Enable newly documented SMEP (Supervisor Mode Execution Protection) CPU feature in KVM module.

Intel new CPU supports SMEP (Supervisor Mode Execution Protection). SMEP prevents kernel from executing code in application. Updated Intel SDM describes this CPU feature. The document will be published soon.

This patch is based on Fenghua's SMEP patch series, as referred by: https://lkml.org/lkml/2011/5/17/523
This patch enables guests' usage of SMEP. 
Currently, we don't enable this feature for guests with shadow page tables.

    Signed-off-by: Yang Wei <wei.y.yang@intel.com>

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 5b4cdcb..7b88e76 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -2743,8 +2743,11 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx)
 
 	vmcs_writel(CR0_GUEST_HOST_MASK, ~0UL);
 	vmx->vcpu.arch.cr4_guest_owned_bits = KVM_CR4_GUEST_OWNED_BITS;
-	if (enable_ept)
+	if (enable_ept) {
+		if (boot_cpu_has(X86_FEATURE_SMEP))
+			vmx->vcpu.arch.cr4_guest_owned_bits |= X86_CR4_SMEP;
 		vmx->vcpu.arch.cr4_guest_owned_bits |= X86_CR4_PGE;
+	}
 	vmcs_writel(CR4_GUEST_HOST_MASK, ~vmx->vcpu.arch.cr4_guest_owned_bits);
 
 	kvm_write_tsc(&vmx->vcpu, 0);
@@ -4366,6 +4369,12 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
 
 static void vmx_set_supported_cpuid(u32 func, struct kvm_cpuid_entry2 *entry)  {
+	switch (func) {
+		case 7:
+			if (!enable_ept)
+				entry->ebx &= ~(1U << 7); /* Clear SMEP bit */
+		break;
+	}
 }
 
 static struct kvm_x86_ops vmx_x86_ops = { diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 934b4c6..64ad4f6 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -68,7 +68,7 @@
 	(~(unsigned long)(X86_CR4_VME | X86_CR4_PVI | X86_CR4_TSD | X86_CR4_DE\
 			  | X86_CR4_PSE | X86_CR4_PAE | X86_CR4_MCE	\
 			  | X86_CR4_PGE | X86_CR4_PCE | X86_CR4_OSFXSR	\
-			  | X86_CR4_OSXSAVE \
+			  | X86_CR4_OSXSAVE | X86_CR4_SMEP \
 			  | X86_CR4_OSXMMEXCPT | X86_CR4_VMXE))
 
 #define CR8_RESERVED_BITS (~(unsigned long)X86_CR8_TPR)

[-- Attachment #2: kvm-smep.patch --]
[-- Type: application/octet-stream, Size: 1431 bytes --]

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 5b4cdcb..bfbfb86 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -2743,8 +2743,11 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx)

 	vmcs_writel(CR0_GUEST_HOST_MASK, ~0UL);
 	vmx->vcpu.arch.cr4_guest_owned_bits = KVM_CR4_GUEST_OWNED_BITS;
-	if (enable_ept)
+	if (enable_ept) {
+		if (boot_cpu_has(X86_FEATURE_SMEP))
+			vmx->vcpu.arch.cr4_guest_owned_bits |= X86_CR4_SMEP;
 		vmx->vcpu.arch.cr4_guest_owned_bits |= X86_CR4_PGE;
+	}
 	vmcs_writel(CR4_GUEST_HOST_MASK, ~vmx->vcpu.arch.cr4_guest_owned_bits);

 	kvm_write_tsc(&vmx->vcpu, 0);
@@ -4366,6 +4369,12 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)

 static void vmx_set_supported_cpuid(u32 func, struct kvm_cpuid_entry2 *entry)
 {
+	switch (func) {
+		case 7:
+			if (!enable_ept)
+				entry->ebx &= ~(1U << 7); /* Clear SMEP bit */
+		break;
+	}
 }

 static struct kvm_x86_ops vmx_x86_ops = {
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 934b4c6..64ad4f6 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -68,7 +68,7 @@
 	(~(unsigned long)(X86_CR4_VME | X86_CR4_PVI | X86_CR4_TSD | X86_CR4_DE\
 			  | X86_CR4_PSE | X86_CR4_PAE | X86_CR4_MCE	\
 			  | X86_CR4_PGE | X86_CR4_PCE | X86_CR4_OSFXSR	\
-			  | X86_CR4_OSXSAVE \
+			  | X86_CR4_OSXSAVE | X86_CR4_SMEP \
 			  | X86_CR4_OSXMMEXCPT | X86_CR4_VMXE))

 #define CR8_RESERVED_BITS (~(unsigned long)X86_CR8_TPR)

Re: [PATCH v2] Enable CPU SMEP feature for KVM

[Avi Kivity]

On 05/22/2011 08:23 AM, Yang, Wei Y wrote:
> This patch matches with "[PATCH v2] Enable CPU SMEP feature support for QEMU-KVM", no changes since v1.
>
> Enable newly documented SMEP (Supervisor Mode Execution Protection) CPU feature in KVM module.
>
> Intel new CPU supports SMEP (Supervisor Mode Execution Protection). SMEP prevents kernel from executing code in application. Updated Intel SDM describes this CPU feature. The document will be published soon.
>
> This patch is based on Fenghua's SMEP patch series, as referred by: https://lkml.org/lkml/2011/5/17/523
> This patch enables guests' usage of SMEP.
> Currently, we don't enable this feature for guests with shadow page tables.

Why not?  I see nothing that conflicts with shadow.

Missing:
   update kvm_set_cr4() to reject SMEP if it's disabled in cpuid
   drop SMEP from cr4_guest_owned_bits if SMEP is disabled in cpuid
   update walk_addr_generic() to fault if SMEP is enabled and fetching 
from a user page

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

RE: [PATCH v2] Enable CPU SMEP feature for KVM

[Yang, Wei Y]

> This patch matches with "[PATCH v2] Enable CPU SMEP feature support for QEMU-KVM", no changes since v1.
>
> Enable newly documented SMEP (Supervisor Mode Execution Protection) CPU feature in KVM module.
>
> Intel new CPU supports SMEP (Supervisor Mode Execution Protection). SMEP prevents kernel from executing code in application. Updated Intel SDM describes this CPU feature. The document will be published soon.
>
> This patch is based on Fenghua's SMEP patch series, as referred by: https://lkml.org/lkml/2011/5/17/523
> This patch enables guests' usage of SMEP.
> Currently, we don't enable this feature for guests with shadow page tables.

> Why not?  I see nothing that conflicts with shadow.

We don't need to enable it for shadow page table, because shadow has mask against guest/shadow PTE, which may cause problem.  Let's keep shadow as it is because it's already very complex. Assume SMEP machines should have EPT.

> Missing:
>   update kvm_set_cr4() to reject SMEP if it's disabled in cupid

Yes, I will check it.

>   drop SMEP from cr4_guest_owned_bits if SMEP is disabled in cupid

SMEP BIT is not included in KVM_CR4_GUEST_OWNED_BITS.

>   update walk_addr_generic() to fault if SMEP is enabled and fetching 

Comments above.

> from a user page

Re: [PATCH v2] Enable CPU SMEP feature for KVM

[Avi Kivity]

On 05/22/2011 11:08 AM, Yang, Wei Y wrote:
> >  This patch matches with "[PATCH v2] Enable CPU SMEP feature support for QEMU-KVM", no changes since v1.
> >
> >  Enable newly documented SMEP (Supervisor Mode Execution Protection) CPU feature in KVM module.
> >
> >  Intel new CPU supports SMEP (Supervisor Mode Execution Protection). SMEP prevents kernel from executing code in application. Updated Intel SDM describes this CPU feature. The document will be published soon.
> >
> >  This patch is based on Fenghua's SMEP patch series, as referred by: https://lkml.org/lkml/2011/5/17/523
> >  This patch enables guests' usage of SMEP.
> >  Currently, we don't enable this feature for guests with shadow page tables.
>
> >  Why not?  I see nothing that conflicts with shadow.
>
> We don't need to enable it for shadow page table, because shadow has mask against guest/shadow PTE, which may cause problem.  Let's keep shadow as it is because it's already very complex. Assume SMEP machines should have EPT.
>

I don't understand why.  Can you elaborate?

Shadow implements the U bit, which is all that is needed by SMEP as far 
as I can tell.


> >    update walk_addr_generic() to fault if SMEP is enabled and fetching
>
> Comments above.
>
> >  from a user page
>

Needs to be done even from EPT, in case walk_addr_generic() is invoked 
by the emulator.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [PATCH v2] Enable CPU SMEP feature for KVM

[Avi Kivity]

On 05/24/2011 05:53 AM, Haitao Shan wrote:
>
>
>     I don't understand why.  Can you elaborate?
>
>     Shadow implements the U bit, which is all that is needed by SMEP
>     as far as I can tell.
>
> Basically, all SMEP-capable platform has EPT, which is on by default 
> in KVM. Thus, we naturally thought there was little value to add it to 
> SPT.

We try to keep features orthogonal.  That has value for testing, and 
results in clearer code.

> Another thing that we are not so sure of is whether SPT has tricky 
> usages on U bit (for optimization or whatever). With SMEP, this trick 
> may be easily broken.

In fact it does, we play with the U bit to emulate cr0.wp.  I'll be 
happy to write the patch to handle this issue, since I'm familiar with 
the code.

> Anyway, we are investigating enabling SMEP with SPT now.
>

Great, thanks.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.