[PATCH 01/11] nEPT: Support LOAD_IA32_EFER entry/exit controls for L1

[PATCH 01/11] nEPT: Support LOAD_IA32_EFER entry/exit controls for L1

[Jun Nakajima]

Recent KVM, since http://kerneltrap.org/mailarchive/linux-kvm/2010/5/2/6261577
switch the EFER MSR when EPT is used and the host and guest have different
NX bits. So if we add support for nested EPT (L1 guest using EPT to run L2)
and want to be able to run recent KVM as L1, we need to allow L1 to use this
EFER switching feature.

To do this EFER switching, KVM uses VM_ENTRY/EXIT_LOAD_IA32_EFER if available,
and if it isn't, it uses the generic VM_ENTRY/EXIT_MSR_LOAD. This patch adds
support for the former (the latter is still unsupported).

Nested entry and exit emulation (prepare_vmcs_02 and load_vmcs12_host_state,
respectively) already handled VM_ENTRY/EXIT_LOAD_IA32_EFER correctly. So all
that's left to do in this patch is to properly advertise this feature to L1.

Note that vmcs12's VM_ENTRY/EXIT_LOAD_IA32_EFER are emulated by L0, by using
vmx_set_efer (which itself sets one of several vmcs02 fields), so we always
support this feature, regardless of whether the host supports it.

Signed-off-by: Nadav Har'El <nyh@il.ibm.com>
Signed-off-by: Jun Nakajima <jun.nakajima@intel.com>
Signed-off-by: Xinhao Xu <xinhao.xu@intel.com>
---
 arch/x86/kvm/vmx.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 6667042..9e0ec9d 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -2057,6 +2057,7 @@ static __init void nested_vmx_setup_ctls_msrs(void)
 #else
 	nested_vmx_exit_ctls_high = 0;
 #endif
+	nested_vmx_exit_ctls_high |= VM_EXIT_LOAD_IA32_EFER;
 
 	/* entry controls */
 	rdmsr(MSR_IA32_VMX_ENTRY_CTLS,
@@ -2064,6 +2065,7 @@ static __init void nested_vmx_setup_ctls_msrs(void)
 	nested_vmx_entry_ctls_low = 0;
 	nested_vmx_entry_ctls_high &=
 		VM_ENTRY_LOAD_IA32_PAT | VM_ENTRY_IA32E_MODE;
+	nested_vmx_entry_ctls_high |= VM_ENTRY_LOAD_IA32_EFER;
 
 	/* cpu-based controls */
 	rdmsr(MSR_IA32_VMX_PROCBASED_CTLS,
@@ -7050,10 +7052,18 @@ static void prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12)
 	vcpu->arch.cr0_guest_owned_bits &= ~vmcs12->cr0_guest_host_mask;
 	vmcs_writel(CR0_GUEST_HOST_MASK, ~vcpu->arch.cr0_guest_owned_bits);
 
-	/* Note: IA32_MODE, LOAD_IA32_EFER are modified by vmx_set_efer below */
-	vmcs_write32(VM_EXIT_CONTROLS,
-		vmcs12->vm_exit_controls | vmcs_config.vmexit_ctrl);
-	vmcs_write32(VM_ENTRY_CONTROLS, vmcs12->vm_entry_controls |
+	/* L2->L1 exit controls are emulated - the hardware exit is to L0 so
+	 * we should use its exit controls. Note that IA32_MODE, LOAD_IA32_EFER
+	 * bits are further modified by vmx_set_efer() below.
+	 */
+	vmcs_write32(VM_EXIT_CONTROLS, vmcs_config.vmexit_ctrl);
+
+	/* vmcs12's VM_ENTRY_LOAD_IA32_EFER and VM_ENTRY_IA32E_MODE are
+	 * emulated by vmx_set_efer(), below.
+	 */
+	vmcs_write32(VM_ENTRY_CONTROLS,
+		(vmcs12->vm_entry_controls & ~VM_ENTRY_LOAD_IA32_EFER &
+			~VM_ENTRY_IA32E_MODE) |
 		(vmcs_config.vmentry_ctrl & ~VM_ENTRY_IA32E_MODE));
 
 	if (vmcs12->vm_entry_controls & VM_ENTRY_LOAD_IA32_PAT)
-- 
1.8.2.1.610.g562af5b

[PATCH 02/11] nEPT: Add EPT tables support to paging_tmpl.h

[Jun Nakajima]

This is the first patch in a series which adds nested EPT support to KVM's
nested VMX. Nested EPT means emulating EPT for an L1 guest so that L1 can use
EPT when running a nested guest L2. When L1 uses EPT, it allows the L2 guest
to set its own cr3 and take its own page faults without either of L0 or L1
getting involved. This often significanlty improves L2's performance over the
previous two alternatives (shadow page tables over EPT, and shadow page
tables over shadow page tables).

This patch adds EPT support to paging_tmpl.h.

paging_tmpl.h contains the code for reading and writing page tables. The code
for 32-bit and 64-bit tables is very similar, but not identical, so
paging_tmpl.h is #include'd twice in mmu.c, once with PTTTYPE=32 and once
with PTTYPE=64, and this generates the two sets of similar functions.

There are subtle but important differences between the format of EPT tables
and that of ordinary x86 64-bit page tables, so for nested EPT we need a
third set of functions to read the guest EPT table and to write the shadow
EPT table.

So this patch adds third PTTYPE, PTTYPE_EPT, which creates functions (prefixed
with "EPT") which correctly read and write EPT tables.

Signed-off-by: Nadav Har'El <nyh@il.ibm.com>
Signed-off-by: Jun Nakajima <jun.nakajima@intel.com>
Signed-off-by: Xinhao Xu <xinhao.xu@intel.com>
---
 arch/x86/kvm/mmu.c         |  35 ++----------
 arch/x86/kvm/paging_tmpl.h | 133 ++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 130 insertions(+), 38 deletions(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 956ca35..cb9c6fd 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -2480,26 +2480,6 @@ static pfn_t pte_prefetch_gfn_to_pfn(struct kvm_vcpu *vcpu, gfn_t gfn,
 	return gfn_to_pfn_memslot_atomic(slot, gfn);
 }
 
-static bool prefetch_invalid_gpte(struct kvm_vcpu *vcpu,
-				  struct kvm_mmu_page *sp, u64 *spte,
-				  u64 gpte)
-{
-	if (is_rsvd_bits_set(&vcpu->arch.mmu, gpte, PT_PAGE_TABLE_LEVEL))
-		goto no_present;
-
-	if (!is_present_gpte(gpte))
-		goto no_present;
-
-	if (!(gpte & PT_ACCESSED_MASK))
-		goto no_present;
-
-	return false;
-
-no_present:
-	drop_spte(vcpu->kvm, spte);
-	return true;
-}
-
 static int direct_pte_prefetch_many(struct kvm_vcpu *vcpu,
 				    struct kvm_mmu_page *sp,
 				    u64 *start, u64 *end)
@@ -3399,16 +3379,6 @@ static bool sync_mmio_spte(u64 *sptep, gfn_t gfn, unsigned access,
 	return false;
 }
 
-static inline unsigned gpte_access(struct kvm_vcpu *vcpu, u64 gpte)
-{
-	unsigned access;
-
-	access = (gpte & (PT_WRITABLE_MASK | PT_USER_MASK)) | ACC_EXEC_MASK;
-	access &= ~(gpte >> PT64_NX_SHIFT);
-
-	return access;
-}
-
 static inline bool is_last_gpte(struct kvm_mmu *mmu, unsigned level, unsigned gpte)
 {
 	unsigned index;
@@ -3418,6 +3388,11 @@ static inline bool is_last_gpte(struct kvm_mmu *mmu, unsigned level, unsigned gp
 	return mmu->last_pte_bitmap & (1 << index);
 }
 
+#define PTTYPE_EPT 18 /* arbitrary */
+#define PTTYPE PTTYPE_EPT
+#include "paging_tmpl.h"
+#undef PTTYPE
+
 #define PTTYPE 64
 #include "paging_tmpl.h"
 #undef PTTYPE
diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
index 105dd5b..e13b6c5 100644
--- a/arch/x86/kvm/paging_tmpl.h
+++ b/arch/x86/kvm/paging_tmpl.h
@@ -50,6 +50,22 @@
 	#define PT_LEVEL_BITS PT32_LEVEL_BITS
 	#define PT_MAX_FULL_LEVELS 2
 	#define CMPXCHG cmpxchg
+#elif PTTYPE == PTTYPE_EPT
+	#define pt_element_t u64
+	#define guest_walker guest_walkerEPT
+	#define FNAME(name) EPT_##name
+	#define PT_BASE_ADDR_MASK PT64_BASE_ADDR_MASK
+	#define PT_LVL_ADDR_MASK(lvl) PT64_LVL_ADDR_MASK(lvl)
+	#define PT_LVL_OFFSET_MASK(lvl) PT64_LVL_OFFSET_MASK(lvl)
+	#define PT_INDEX(addr, level) PT64_INDEX(addr, level)
+	#define PT_LEVEL_BITS PT64_LEVEL_BITS
+	#ifdef CONFIG_X86_64
+	#define PT_MAX_FULL_LEVELS 4
+	#define CMPXCHG cmpxchg
+	#else
+	#define CMPXCHG cmpxchg64
+	#define PT_MAX_FULL_LEVELS 2
+	#endif
 #else
 	#error Invalid PTTYPE value
 #endif
@@ -80,6 +96,7 @@ static gfn_t gpte_to_gfn_lvl(pt_element_t gpte, int lvl)
 	return (gpte & PT_LVL_ADDR_MASK(lvl)) >> PAGE_SHIFT;
 }
 
+#if PTTYPE != PTTYPE_EPT
 static int FNAME(cmpxchg_gpte)(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu,
 			       pt_element_t __user *ptep_user, unsigned index,
 			       pt_element_t orig_pte, pt_element_t new_pte)
@@ -102,7 +119,52 @@ static int FNAME(cmpxchg_gpte)(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu,
 
 	return (ret != orig_pte);
 }
+#endif
+
+static unsigned FNAME(gpte_access)(struct kvm_vcpu *vcpu, u64 gpte)
+{
+	unsigned access;
+
+#if PTTYPE == PTTYPE_EPT
+	/* We rely here that ACC_WRITE_MASK==VMX_EPT_WRITABLE_MASK */
+	access = (gpte & VMX_EPT_WRITABLE_MASK) | ACC_USER_MASK |
+		((gpte & VMX_EPT_EXECUTABLE_MASK) ? ACC_EXEC_MASK : 0);
+#else
+	access = (gpte & (PT_WRITABLE_MASK | PT_USER_MASK)) | ACC_EXEC_MASK;
+	access &= ~(gpte >> PT64_NX_SHIFT);
+#endif
+
+	return access;
+}
+
+static inline int FNAME(is_present_gpte)(unsigned long pte)
+{
+#if PTTYPE == PTTYPE_EPT
+	return pte & (VMX_EPT_READABLE_MASK | VMX_EPT_WRITABLE_MASK |
+			VMX_EPT_EXECUTABLE_MASK);
+#else
+	return is_present_gpte(pte);
+#endif
+}
+
+static inline int FNAME(check_write_user_access)(struct kvm_vcpu *vcpu,
+					   bool write_fault, bool user_fault,
+					   unsigned long pte)
+{
+#if PTTYPE == PTTYPE_EPT
+	if (unlikely(write_fault && !(pte & VMX_EPT_WRITABLE_MASK)
+				 && (user_fault || is_write_protection(vcpu))))
+		return false;
+	return true;
+#else
+	u32 access = ((kvm_x86_ops->get_cpl(vcpu) == 3) ? PFERR_USER_MASK : 0)
+                | (write_fault ? PFERR_WRITE_MASK : 0);
 
+	return !permission_fault(vcpu->arch.walk_mmu, vcpu->arch.access, access);
+#endif
+}
+
+#if PTTYPE != PTTYPE_EPT
 static int FNAME(update_accessed_dirty_bits)(struct kvm_vcpu *vcpu,
 					     struct kvm_mmu *mmu,
 					     struct guest_walker *walker,
@@ -139,6 +201,7 @@ static int FNAME(update_accessed_dirty_bits)(struct kvm_vcpu *vcpu,
 	}
 	return 0;
 }
+#endif
 
 /*
  * Fetch a guest pte for a guest virtual address
@@ -147,7 +210,6 @@ static int FNAME(walk_addr_generic)(struct guest_walker *walker,
 				    struct kvm_vcpu *vcpu, struct kvm_mmu *mmu,
 				    gva_t addr, u32 access)
 {
-	int ret;
 	pt_element_t pte;
 	pt_element_t __user *uninitialized_var(ptep_user);
 	gfn_t table_gfn;
@@ -162,7 +224,9 @@ static int FNAME(walk_addr_generic)(struct guest_walker *walker,
 	gfn_t gfn;
 
 	trace_kvm_mmu_pagetable_walk(addr, access);
+#if PTTYPE != PTTYPE_EPT
 retry_walk:
+#endif
 	walker->level = mmu->root_level;
 	pte           = mmu->get_cr3(vcpu);
 
@@ -215,17 +279,20 @@ retry_walk:
 
 		trace_kvm_mmu_paging_element(pte, walker->level);
 
-		if (unlikely(!is_present_gpte(pte)))
+		if (unlikely(!FNAME(is_present_gpte)(pte)))
 			goto error;
 
 		if (unlikely(is_rsvd_bits_set(&vcpu->arch.mmu, pte,
 					      walker->level))) {
-			errcode |= PFERR_RSVD_MASK | PFERR_PRESENT_MASK;
+			errcode |= PFERR_PRESENT_MASK;
+#if PTTYPE != PTTYPE_EPT
+			errcode |= PFERR_RSVD_MASK;
+#endif
 			goto error;
 		}
 
 		accessed_dirty &= pte;
-		pte_access = pt_access & gpte_access(vcpu, pte);
+		pte_access = pt_access & FNAME(gpte_access)(vcpu, pte);
 
 		walker->ptes[walker->level - 1] = pte;
 	} while (!is_last_gpte(mmu, walker->level, pte));
@@ -247,6 +314,7 @@ retry_walk:
 
 	walker->gfn = real_gpa >> PAGE_SHIFT;
 
+#if PTTYPE != PTTYPE_EPT
 	if (!write_fault)
 		protect_clean_gpte(&pte_access, pte);
 	else
@@ -257,12 +325,15 @@ retry_walk:
 		accessed_dirty &= pte >> (PT_DIRTY_SHIFT - PT_ACCESSED_SHIFT);
 
 	if (unlikely(!accessed_dirty)) {
+		int ret;
+
 		ret = FNAME(update_accessed_dirty_bits)(vcpu, mmu, walker, write_fault);
 		if (unlikely(ret < 0))
 			goto error;
 		else if (ret)
 			goto retry_walk;
 	}
+#endif
 
 	walker->pt_access = pt_access;
 	walker->pte_access = pte_access;
@@ -293,6 +364,7 @@ static int FNAME(walk_addr)(struct guest_walker *walker,
 					access);
 }
 
+#if PTTYPE != PTTYPE_EPT
 static int FNAME(walk_addr_nested)(struct guest_walker *walker,
 				   struct kvm_vcpu *vcpu, gva_t addr,
 				   u32 access)
@@ -300,6 +372,29 @@ static int FNAME(walk_addr_nested)(struct guest_walker *walker,
 	return FNAME(walk_addr_generic)(walker, vcpu, &vcpu->arch.nested_mmu,
 					addr, access);
 }
+#endif
+
+static bool FNAME(prefetch_invalid_gpte)(struct kvm_vcpu *vcpu,
+				  struct kvm_mmu_page *sp, u64 *spte,
+				  u64 gpte)
+{
+	if (is_rsvd_bits_set(&vcpu->arch.mmu, gpte, PT_PAGE_TABLE_LEVEL))
+		goto no_present;
+
+	if (!is_present_gpte(gpte))
+		goto no_present;
+
+#if PTTYPE != PTTYPE_EPT
+	if (!(gpte & PT_ACCESSED_MASK))
+		goto no_present;
+#endif
+
+	return false;
+
+no_present:
+	drop_spte(vcpu->kvm, spte);
+	return true;
+}
 
 static bool
 FNAME(prefetch_gpte)(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp,
@@ -309,13 +404,13 @@ FNAME(prefetch_gpte)(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp,
 	gfn_t gfn;
 	pfn_t pfn;
 
-	if (prefetch_invalid_gpte(vcpu, sp, spte, gpte))
+	if (FNAME(prefetch_invalid_gpte)(vcpu, sp, spte, gpte))
 		return false;
 
 	pgprintk("%s: gpte %llx spte %p\n", __func__, (u64)gpte, spte);
 
 	gfn = gpte_to_gfn(gpte);
-	pte_access = sp->role.access & gpte_access(vcpu, gpte);
+	pte_access = sp->role.access & FNAME(gpte_access)(vcpu, gpte);
 	protect_clean_gpte(&pte_access, gpte);
 	pfn = pte_prefetch_gfn_to_pfn(vcpu, gfn,
 			no_dirty_log && (pte_access & ACC_WRITE_MASK));
@@ -394,6 +489,18 @@ static void FNAME(pte_prefetch)(struct kvm_vcpu *vcpu, struct guest_walker *gw,
 	}
 }
 
+#if PTTYPE == PTTYPE_EPT
+static void FNAME(link_shadow_page)(u64 *sptep, struct kvm_mmu_page *sp)
+{
+	u64 spte;
+
+	spte = __pa(sp->spt) | VMX_EPT_READABLE_MASK | VMX_EPT_WRITABLE_MASK |
+		VMX_EPT_EXECUTABLE_MASK;
+
+	mmu_spte_set(sptep, spte);
+}
+#endif
+
 /*
  * Fetch a shadow pte for a specific level in the paging hierarchy.
  * If the guest tries to write a write-protected page, we need to
@@ -446,7 +553,11 @@ static int FNAME(fetch)(struct kvm_vcpu *vcpu, gva_t addr,
 			goto out_gpte_changed;
 
 		if (sp)
+#if PTTYPE == PTTYPE_EPT
+			FNAME(link_shadow_page)(it.sptep, sp);
+#else
 			link_shadow_page(it.sptep, sp);
+#endif
 	}
 
 	for (;
@@ -466,7 +577,11 @@ static int FNAME(fetch)(struct kvm_vcpu *vcpu, gva_t addr,
 
 		sp = kvm_mmu_get_page(vcpu, direct_gfn, addr, it.level-1,
 				      true, direct_access, it.sptep);
+#if PTTYPE == PTTYPE_EPT
+		FNAME(link_shadow_page)(it.sptep, sp);
+#else
 		link_shadow_page(it.sptep, sp);
+#endif
 	}
 
 	clear_sp_write_flooding_count(it.sptep);
@@ -724,6 +839,7 @@ static gpa_t FNAME(gva_to_gpa)(struct kvm_vcpu *vcpu, gva_t vaddr, u32 access,
 	return gpa;
 }
 
+#if PTTYPE != PTTYPE_EPT
 static gpa_t FNAME(gva_to_gpa_nested)(struct kvm_vcpu *vcpu, gva_t vaddr,
 				      u32 access,
 				      struct x86_exception *exception)
@@ -742,6 +858,7 @@ static gpa_t FNAME(gva_to_gpa_nested)(struct kvm_vcpu *vcpu, gva_t vaddr,
 
 	return gpa;
 }
+#endif
 
 /*
  * Using the cached information from sp->gfns is safe because:
@@ -782,14 +899,14 @@ static int FNAME(sync_page)(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp)
 					  sizeof(pt_element_t)))
 			return -EINVAL;
 
-		if (prefetch_invalid_gpte(vcpu, sp, &sp->spt[i], gpte)) {
+		if (FNAME(prefetch_invalid_gpte)(vcpu, sp, &sp->spt[i], gpte)) {
 			vcpu->kvm->tlbs_dirty++;
 			continue;
 		}
 
 		gfn = gpte_to_gfn(gpte);
 		pte_access = sp->role.access;
-		pte_access &= gpte_access(vcpu, gpte);
+		pte_access &= FNAME(gpte_access)(vcpu, gpte);
 		protect_clean_gpte(&pte_access, gpte);
 
 		if (sync_mmio_spte(&sp->spt[i], gfn, pte_access, &nr_present))
-- 
1.8.2.1.610.g562af5b

[PATCH 03/11] nEPT: MMU context for nested EPT

[Jun Nakajima]

KVM's existing shadow MMU code already supports nested TDP. To use it, we
need to set up a new "MMU context" for nested EPT, and create a few callbacks
for it (nested_ept_*()). This context should also use the EPT versions of
the page table access functions (defined in the previous patch).
Then, we need to switch back and forth between this nested context and the
regular MMU context when switching between L1 and L2 (when L1 runs this L2
with EPT).

Signed-off-by: Nadav Har'El <nyh@il.ibm.com>
Signed-off-by: Jun Nakajima <jun.nakajima@intel.com>
Signed-off-by: Xinhao Xu <xinhao.xu@intel.com>
---
 arch/x86/kvm/mmu.c | 38 ++++++++++++++++++++++++++++++++++++++
 arch/x86/kvm/mmu.h |  1 +
 arch/x86/kvm/vmx.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 91 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index cb9c6fd..99bfc5e 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -3644,6 +3644,44 @@ int kvm_init_shadow_mmu(struct kvm_vcpu *vcpu, struct kvm_mmu *context)
 }
 EXPORT_SYMBOL_GPL(kvm_init_shadow_mmu);
 
+int kvm_init_shadow_EPT_mmu(struct kvm_vcpu *vcpu, struct kvm_mmu *context)
+{
+	ASSERT(vcpu);
+	ASSERT(!VALID_PAGE(vcpu->arch.mmu.root_hpa));
+
+	context->shadow_root_level = kvm_x86_ops->get_tdp_level();
+
+	context->nx = is_nx(vcpu); /* TODO: ? */
+	context->new_cr3 = paging_new_cr3;
+	context->page_fault = EPT_page_fault;
+	context->gva_to_gpa = EPT_gva_to_gpa;
+	context->sync_page = EPT_sync_page;
+	context->invlpg = EPT_invlpg;
+	context->update_pte = EPT_update_pte;
+	context->free = paging_free;
+	context->root_level = context->shadow_root_level;
+	context->root_hpa = INVALID_PAGE;
+	context->direct_map = false;
+
+	/* TODO: reset_rsvds_bits_mask() is not built for EPT, we need
+	   something different.
+	 */
+	reset_rsvds_bits_mask(vcpu, context);
+
+
+	/* TODO: I copied these from kvm_init_shadow_mmu, I don't know why
+	   they are done, or why they write to vcpu->arch.mmu and not context
+	 */
+	vcpu->arch.mmu.base_role.cr4_pae = !!is_pae(vcpu);
+	vcpu->arch.mmu.base_role.cr0_wp  = is_write_protection(vcpu);
+	vcpu->arch.mmu.base_role.smep_andnot_wp =
+		kvm_read_cr4_bits(vcpu, X86_CR4_SMEP) &&
+		!is_write_protection(vcpu);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(kvm_init_shadow_EPT_mmu);
+
 static int init_kvm_softmmu(struct kvm_vcpu *vcpu)
 {
 	int r = kvm_init_shadow_mmu(vcpu, vcpu->arch.walk_mmu);
diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
index 6987108..19dd5ab 100644
--- a/arch/x86/kvm/mmu.h
+++ b/arch/x86/kvm/mmu.h
@@ -54,6 +54,7 @@ int kvm_mmu_get_spte_hierarchy(struct kvm_vcpu *vcpu, u64 addr, u64 sptes[4]);
 void kvm_mmu_set_mmio_spte_mask(u64 mmio_mask);
 int handle_mmio_page_fault_common(struct kvm_vcpu *vcpu, u64 addr, bool direct);
 int kvm_init_shadow_mmu(struct kvm_vcpu *vcpu, struct kvm_mmu *context);
+int kvm_init_shadow_EPT_mmu(struct kvm_vcpu *vcpu, struct kvm_mmu *context);
 
 static inline unsigned int kvm_mmu_available_pages(struct kvm *kvm)
 {
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 9e0ec9d..6ab53ca 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -918,6 +918,11 @@ static inline bool nested_cpu_has_virtual_nmis(struct vmcs12 *vmcs12,
 	return vmcs12->pin_based_vm_exec_control & PIN_BASED_VIRTUAL_NMIS;
 }
 
+static inline int nested_cpu_has_ept(struct vmcs12 *vmcs12)
+{
+	return nested_cpu_has2(vmcs12, SECONDARY_EXEC_ENABLE_EPT);
+}
+
 static inline bool is_exception(u32 intr_info)
 {
 	return (intr_info & (INTR_INFO_INTR_TYPE_MASK | INTR_INFO_VALID_MASK))
@@ -6873,6 +6878,46 @@ static void vmx_set_supported_cpuid(u32 func, struct kvm_cpuid_entry2 *entry)
 		entry->ecx |= bit(X86_FEATURE_VMX);
 }
 
+/* Callbacks for nested_ept_init_mmu_context: */
+
+static unsigned long nested_ept_get_cr3(struct kvm_vcpu *vcpu)
+{
+	/* return the page table to be shadowed - in our case, EPT12 */
+	return get_vmcs12(vcpu)->ept_pointer;
+}
+
+static void nested_ept_inject_page_fault(struct kvm_vcpu *vcpu,
+	struct x86_exception *fault)
+{
+	struct vmcs12 *vmcs12;
+	nested_vmx_vmexit(vcpu);
+	vmcs12 = get_vmcs12(vcpu);
+	/*
+	 * Note no need to set vmcs12->vm_exit_reason as it is already copied
+	 * from vmcs02 in nested_vmx_vmexit() above, i.e., EPT_VIOLATION.
+	 */
+	vmcs12->exit_qualification = fault->error_code;
+	vmcs12->guest_physical_address = fault->address;
+}
+
+static int nested_ept_init_mmu_context(struct kvm_vcpu *vcpu)
+{
+	int r = kvm_init_shadow_EPT_mmu(vcpu, &vcpu->arch.mmu);
+
+	vcpu->arch.mmu.set_cr3           = vmx_set_cr3;
+	vcpu->arch.mmu.get_cr3           = nested_ept_get_cr3;
+	vcpu->arch.mmu.inject_page_fault = nested_ept_inject_page_fault;
+
+	vcpu->arch.walk_mmu              = &vcpu->arch.nested_mmu;
+
+	return r;
+}
+
+static void nested_ept_uninit_mmu_context(struct kvm_vcpu *vcpu)
+{
+	vcpu->arch.walk_mmu = &vcpu->arch.mmu;
+}
+
 /*
  * prepare_vmcs02 is called when the L1 guest hypervisor runs its nested
  * L2 guest. L1 has a vmcs for L2 (vmcs12), and this function "merges" it
@@ -7090,6 +7135,11 @@ static void prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12)
 		vmx_flush_tlb(vcpu);
 	}
 
+	if (nested_cpu_has_ept(vmcs12)) {
+		kvm_mmu_unload(vcpu);
+		nested_ept_init_mmu_context(vcpu);
+	}
+
 	if (vmcs12->vm_entry_controls & VM_ENTRY_LOAD_IA32_EFER)
 		vcpu->arch.efer = vmcs12->guest_ia32_efer;
 	if (vmcs12->vm_entry_controls & VM_ENTRY_IA32E_MODE)
@@ -7420,7 +7470,8 @@ void load_vmcs12_host_state(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12)
 	vcpu->arch.cr4_guest_owned_bits = ~vmcs_readl(CR4_GUEST_HOST_MASK);
 	kvm_set_cr4(vcpu, vmcs12->host_cr4);
 
-	/* shadow page tables on either EPT or shadow page tables */
+	if (nested_cpu_has_ept(vmcs12))
+		nested_ept_uninit_mmu_context(vcpu);
 	kvm_set_cr3(vcpu, vmcs12->host_cr3);
 	kvm_mmu_reset_context(vcpu);
 
-- 
1.8.2.1.610.g562af5b

[PATCH 04/11] nEPT: Fix cr3 handling in nested exit and entry

[Jun Nakajima]

The existing code for handling cr3 and related VMCS fields during nested
exit and entry wasn't correct in all cases:

If L2 is allowed to control cr3 (and this is indeed the case in nested EPT),
during nested exit we must copy the modified cr3 from vmcs02 to vmcs12, and
we forgot to do so. This patch adds this copy.

If L0 isn't controlling cr3 when running L2 (i.e., L0 is using EPT), and
whoever does control cr3 (L1 or L2) is using PAE, the processor might have
saved PDPTEs and we should also save them in vmcs12 (and restore later).

Signed-off-by: Nadav Har'El <nyh@il.ibm.com>
Signed-off-by: Jun Nakajima <jun.nakajima@intel.com>
Signed-off-by: Xinhao Xu <xinhao.xu@intel.com>
---
 arch/x86/kvm/vmx.c | 37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 6ab53ca..26a1b6f 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -7163,10 +7163,26 @@ static void prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12)
 	vmx_set_cr4(vcpu, vmcs12->guest_cr4);
 	vmcs_writel(CR4_READ_SHADOW, nested_read_cr4(vmcs12));
 
-	/* shadow page tables on either EPT or shadow page tables */
+	/*
+	 * Note that kvm_set_cr3() and kvm_mmu_reset_context() will do the
+	 * right thing, and set GUEST_CR3 and/or EPT_POINTER in all supported
+	 * settings: 1. shadow page tables on shadow page tables, 2. shadow
+	 * page tables on EPT, 3. EPT on EPT.
+	 */
 	kvm_set_cr3(vcpu, vmcs12->guest_cr3);
 	kvm_mmu_reset_context(vcpu);
 
+	/*
+	 * Additionally, except when L0 is using shadow page tables, L1 or
+	 * L2 control guest_cr3 for L2, so they may also have saved PDPTEs
+	 */
+	if (enable_ept) {
+		vmcs_write64(GUEST_PDPTR0, vmcs12->guest_pdptr0);
+		vmcs_write64(GUEST_PDPTR1, vmcs12->guest_pdptr1);
+		vmcs_write64(GUEST_PDPTR2, vmcs12->guest_pdptr2);
+		vmcs_write64(GUEST_PDPTR3, vmcs12->guest_pdptr3);
+	}
+
 	kvm_register_write(vcpu, VCPU_REGS_RSP, vmcs12->guest_rsp);
 	kvm_register_write(vcpu, VCPU_REGS_RIP, vmcs12->guest_rip);
 }
@@ -7398,6 +7414,25 @@ void prepare_vmcs12(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12)
 	vmcs12->guest_pending_dbg_exceptions =
 		vmcs_readl(GUEST_PENDING_DBG_EXCEPTIONS);
 
+	/*
+	 * In some cases (usually, nested EPT), L2 is allowed to change its
+	 * own CR3 without exiting. If it has changed it, we must keep it.
+	 * Of course, if L0 is using shadow page tables, GUEST_CR3 was defined
+	 * by L0, not L1 or L2, so we mustn't unconditionally copy it to vmcs12.
+	 */
+	if (enable_ept)
+		vmcs12->guest_cr3 = vmcs_read64(GUEST_CR3);
+	/*
+	 * Additionally, except when L0 is using shadow page tables, L1 or
+	 * L2 control guest_cr3 for L2, so save their PDPTEs
+	 */
+	if (enable_ept) {
+		vmcs12->guest_pdptr0 = vmcs_read64(GUEST_PDPTR0);
+		vmcs12->guest_pdptr1 = vmcs_read64(GUEST_PDPTR1);
+		vmcs12->guest_pdptr2 = vmcs_read64(GUEST_PDPTR2);
+		vmcs12->guest_pdptr3 = vmcs_read64(GUEST_PDPTR3);
+	}
+
 	/* TODO: These cannot have changed unless we have MSR bitmaps and
 	 * the relevant bit asks not to trap the change */
 	vmcs12->guest_ia32_debugctl = vmcs_read64(GUEST_IA32_DEBUGCTL);
-- 
1.8.2.1.610.g562af5b

[PATCH 05/11] nEPT: Fix wrong test in kvm_set_cr3

[Jun Nakajima]

kvm_set_cr3() attempts to check if the new cr3 is a valid guest physical
address. The problem is that with nested EPT, cr3 is an *L2* physical
address, not an L1 physical address as this test expects.

As the comment above this test explains, it isn't necessary, and doesn't
correspond to anything a real processor would do. So this patch removes it.

Note that this wrong test could have also theoretically caused problems
in nested NPT, not just in nested EPT. However, in practice, the problem
was avoided: nested_svm_vmexit()/vmrun() do not call kvm_set_cr3 in the
nested NPT case, and instead set the vmcb (and arch.cr3) directly, thus
circumventing the problem. Additional potential calls to the buggy function
are avoided in that we don't trap cr3 modifications when nested NPT is
enabled. However, because in nested VMX we did want to use kvm_set_cr3()
(as requested in Avi Kivity's review of the original nested VMX patches),
we can't avoid this problem and need to fix it.

Signed-off-by: Nadav Har'El <nyh@il.ibm.com>
Signed-off-by: Jun Nakajima <jun.nakajima@intel.com>
Signed-off-by: Xinhao Xu <xinhao.xu@intel.com>
---
 arch/x86/kvm/x86.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index e172132..c34590d 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -659,17 +659,6 @@ int kvm_set_cr3(struct kvm_vcpu *vcpu, unsigned long cr3)
 		 */
 	}
 
-	/*
-	 * Does the new cr3 value map to physical memory? (Note, we
-	 * catch an invalid cr3 even in real-mode, because it would
-	 * cause trouble later on when we turn on paging anyway.)
-	 *
-	 * A real CPU would silently accept an invalid cr3 and would
-	 * attempt to use it - with largely undefined (and often hard
-	 * to debug) behavior on the guest side.
-	 */
-	if (unlikely(!gfn_to_memslot(vcpu->kvm, cr3 >> PAGE_SHIFT)))
-		return 1;
 	vcpu->arch.cr3 = cr3;
 	__set_bit(VCPU_EXREG_CR3, (ulong *)&vcpu->arch.regs_avail);
 	vcpu->arch.mmu.new_cr3(vcpu);
-- 
1.8.2.1.610.g562af5b

[PATCH 06/11] nEPT: Some additional comments

[Jun Nakajima]

Some additional comments to preexisting code:
Explain who (L0 or L1) handles EPT violation and misconfiguration exits.
Don't mention "shadow on either EPT or shadow" as the only two options.

Signed-off-by: Nadav Har'El <nyh@il.ibm.com>
Signed-off-by: Jun Nakajima <jun.nakajima@intel.com>
Signed-off-by: Xinhao Xu <xinhao.xu@intel.com>
---
 arch/x86/kvm/vmx.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 26a1b6f..66ead51 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -6127,7 +6127,20 @@ static bool nested_vmx_exit_handled(struct kvm_vcpu *vcpu)
 		return nested_cpu_has2(vmcs12,
 			SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES);
 	case EXIT_REASON_EPT_VIOLATION:
+		/*
+		 * L0 always deals with the EPT violation. If nested EPT is
+		 * used, and the nested mmu code discovers that the address is
+		 * missing in the guest EPT table (EPT12), the EPT violation
+		 * will be injected with nested_ept_inject_page_fault()
+		 */
+		return 0;
 	case EXIT_REASON_EPT_MISCONFIG:
+		/*
+		 * L2 never uses directly L1's EPT, but rather L0's own EPT
+		 * table (shadow on EPT) or a merged EPT table that L0 built
+		 * (EPT on EPT). So any problems with the structure of the
+		 * table is L0's fault.
+		 */
 		return 0;
 	case EXIT_REASON_WBINVD:
 		return nested_cpu_has2(vmcs12, SECONDARY_EXEC_WBINVD_EXITING);
-- 
1.8.2.1.610.g562af5b

[PATCH 07/11] nEPT: Advertise EPT to L1

[Jun Nakajima]

Advertise the support of EPT to the L1 guest, through the appropriate MSR.

This is the last patch of the basic Nested EPT feature, so as to allow
bisection through this patch series: The guest will not see EPT support until
this last patch, and will not attempt to use the half-applied feature.

Signed-off-by: Nadav Har'El <nyh@il.ibm.com>
Signed-off-by: Jun Nakajima <jun.nakajima@intel.com>
Signed-off-by: Xinhao Xu <xinhao.xu@intel.com>
---
 arch/x86/kvm/vmx.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 66ead51..76df3a8 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -2027,6 +2027,7 @@ static u32 nested_vmx_secondary_ctls_low, nested_vmx_secondary_ctls_high;
 static u32 nested_vmx_pinbased_ctls_low, nested_vmx_pinbased_ctls_high;
 static u32 nested_vmx_exit_ctls_low, nested_vmx_exit_ctls_high;
 static u32 nested_vmx_entry_ctls_low, nested_vmx_entry_ctls_high;
+static u32 nested_vmx_ept_caps;
 static __init void nested_vmx_setup_ctls_msrs(void)
 {
 	/*
@@ -2102,6 +2103,18 @@ static __init void nested_vmx_setup_ctls_msrs(void)
 	nested_vmx_secondary_ctls_low = 0;
 	nested_vmx_secondary_ctls_high &=
 		SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES;
+	if (enable_ept) {
+		/* nested EPT: emulate EPT also to L1 */
+		nested_vmx_secondary_ctls_high |= SECONDARY_EXEC_ENABLE_EPT;
+		nested_vmx_ept_caps = VMX_EPT_PAGE_WALK_4_BIT;
+		nested_vmx_ept_caps |=
+			VMX_EPT_INVEPT_BIT | VMX_EPT_EXTENT_GLOBAL_BIT |
+			VMX_EPT_EXTENT_CONTEXT_BIT |
+			VMX_EPT_EXTENT_INDIVIDUAL_BIT;
+		nested_vmx_ept_caps &= vmx_capability.ept;
+	} else
+		nested_vmx_ept_caps = 0;
+
 }
 
 static inline bool vmx_control_verify(u32 control, u32 low, u32 high)
@@ -2201,8 +2214,8 @@ static int vmx_get_vmx_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 *pdata)
 					nested_vmx_secondary_ctls_high);
 		break;
 	case MSR_IA32_VMX_EPT_VPID_CAP:
-		/* Currently, no nested ept or nested vpid */
-		*pdata = 0;
+		/* Currently, no nested vpid support */
+		*pdata = nested_vmx_ept_caps;
 		break;
 	default:
 		return 0;
-- 
1.8.2.1.610.g562af5b

[PATCH 08/11] nEPT: Nested INVEPT

[Jun Nakajima]

If we let L1 use EPT, we should probably also support the INVEPT instruction.

In our current nested EPT implementation, when L1 changes its EPT table for
L2 (i.e., EPT12), L0 modifies the shadow EPT table (EPT02), and in the course
of this modification already calls INVEPT. Therefore, when L1 calls INVEPT,
we don't really need to do anything. In particular we *don't* need to call
the real INVEPT again. All we do in our INVEPT is verify the validity of the
call, and its parameters, and then do nothing.

In KVM Forum 2010, Dong et al. presented "Nested Virtualization Friendly KVM"
and classified our current nested EPT implementation as "shadow-like virtual
EPT". He recommended instead a different approach, which he called "VTLB-like
virtual EPT". If we had taken that alternative approach, INVEPT would have had
a bigger role: L0 would only rebuild the shadow EPT table when L1 calls INVEPT.

Signed-off-by: Nadav Har'El <nyh@il.ibm.com>
Signed-off-by: Jun Nakajima <jun.nakajima@intel.com>
Signed-off-by: Xinhao Xu <xinhao.xu@intel.com>
---
 arch/x86/include/asm/vmx.h      |  4 +-
 arch/x86/include/uapi/asm/vmx.h |  1 +
 arch/x86/kvm/vmx.c              | 83 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 87 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
index b6fbf86..0ce54f3 100644
--- a/arch/x86/include/asm/vmx.h
+++ b/arch/x86/include/asm/vmx.h
@@ -376,7 +376,9 @@ enum vmcs_field {
 #define VMX_EPTP_WB_BIT				(1ull << 14)
 #define VMX_EPT_2MB_PAGE_BIT			(1ull << 16)
 #define VMX_EPT_1GB_PAGE_BIT			(1ull << 17)
-#define VMX_EPT_AD_BIT				    (1ull << 21)
+#define VMX_EPT_INVEPT_BIT			(1ull << 20)
+#define VMX_EPT_AD_BIT				(1ull << 21)
+#define VMX_EPT_EXTENT_INDIVIDUAL_BIT		(1ull << 24)
 #define VMX_EPT_EXTENT_CONTEXT_BIT		(1ull << 25)
 #define VMX_EPT_EXTENT_GLOBAL_BIT		(1ull << 26)
 
diff --git a/arch/x86/include/uapi/asm/vmx.h b/arch/x86/include/uapi/asm/vmx.h
index 2871fcc..5662cef 100644
--- a/arch/x86/include/uapi/asm/vmx.h
+++ b/arch/x86/include/uapi/asm/vmx.h
@@ -65,6 +65,7 @@
 #define EXIT_REASON_EOI_INDUCED         45
 #define EXIT_REASON_EPT_VIOLATION       48
 #define EXIT_REASON_EPT_MISCONFIG       49
+#define EXIT_REASON_INVEPT		        50
 #define EXIT_REASON_WBINVD              54
 #define EXIT_REASON_XSETBV              55
 #define EXIT_REASON_APIC_WRITE          56
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 76df3a8..c67eb06 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -5879,6 +5879,87 @@ static int handle_vmptrst(struct kvm_vcpu *vcpu)
 	return 1;
 }
 
+/* Emulate the INVEPT instruction */
+static int handle_invept(struct kvm_vcpu *vcpu)
+{
+	u32 vmx_instruction_info;
+	unsigned long type;
+	gva_t gva;
+	struct x86_exception e;
+	struct {
+		u64 eptp, gpa;
+	} operand;
+
+	if (!(nested_vmx_secondary_ctls_high & SECONDARY_EXEC_ENABLE_EPT) ||
+	    !(nested_vmx_ept_caps & VMX_EPT_INVEPT_BIT)) {
+		kvm_queue_exception(vcpu, UD_VECTOR);
+		return 1;
+	}
+
+	if (!nested_vmx_check_permission(vcpu))
+		return 1;
+
+	if (!kvm_read_cr0_bits(vcpu, X86_CR0_PE)) {
+		kvm_queue_exception(vcpu, UD_VECTOR);
+		return 1;
+	}
+
+	/* According to the Intel VMX instruction reference, the memory
+	 * operand is read even if it isn't needed (e.g., for type==global)
+	 */
+	vmx_instruction_info = vmcs_read32(VMX_INSTRUCTION_INFO);
+	if (get_vmx_mem_address(vcpu, vmcs_readl(EXIT_QUALIFICATION),
+			vmx_instruction_info, &gva))
+		return 1;
+	if (kvm_read_guest_virt(&vcpu->arch.emulate_ctxt, gva, &operand,
+				sizeof(operand), &e)) {
+		kvm_inject_page_fault(vcpu, &e);
+		return 1;
+	}
+
+	type = kvm_register_read(vcpu, (vmx_instruction_info >> 28) & 0xf);
+
+	switch (type) {
+	case VMX_EPT_EXTENT_GLOBAL:
+		if (!(nested_vmx_ept_caps & VMX_EPT_EXTENT_GLOBAL_BIT))
+			nested_vmx_failValid(vcpu,
+				VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID);
+		else {
+			/*
+			 * Do nothing: when L1 changes EPT12, we already
+			 * update EPT02 (the shadow EPT table) and call INVEPT.
+			 * So when L1 calls INVEPT, there's nothing left to do.
+			 */
+			nested_vmx_succeed(vcpu);
+		}
+		break;
+	case VMX_EPT_EXTENT_CONTEXT:
+		if (!(nested_vmx_ept_caps & VMX_EPT_EXTENT_CONTEXT_BIT))
+			nested_vmx_failValid(vcpu,
+				VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID);
+		else {
+			/* Do nothing */
+			nested_vmx_succeed(vcpu);
+		}
+		break;
+	case VMX_EPT_EXTENT_INDIVIDUAL_ADDR:
+		if (!(nested_vmx_ept_caps & VMX_EPT_EXTENT_INDIVIDUAL_BIT))
+			nested_vmx_failValid(vcpu,
+				VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID);
+		else {
+			/* Do nothing */
+			nested_vmx_succeed(vcpu);
+		}
+		break;
+	default:
+		nested_vmx_failValid(vcpu,
+			VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID);
+	}
+
+	skip_emulated_instruction(vcpu);
+	return 1;
+}
+
 /*
  * The exit handlers return 1 if the exit was handled fully and guest execution
  * may resume.  Otherwise they set the kvm_run parameter to indicate what needs
@@ -5923,6 +6004,7 @@ static int (*const kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu) = {
 	[EXIT_REASON_PAUSE_INSTRUCTION]       = handle_pause,
 	[EXIT_REASON_MWAIT_INSTRUCTION]	      = handle_invalid_op,
 	[EXIT_REASON_MONITOR_INSTRUCTION]     = handle_invalid_op,
+	[EXIT_REASON_INVEPT]                  = handle_invept,
 };
 
 static const int kvm_vmx_max_exit_handlers =
@@ -6107,6 +6189,7 @@ static bool nested_vmx_exit_handled(struct kvm_vcpu *vcpu)
 	case EXIT_REASON_VMPTRST: case EXIT_REASON_VMREAD:
 	case EXIT_REASON_VMRESUME: case EXIT_REASON_VMWRITE:
 	case EXIT_REASON_VMOFF: case EXIT_REASON_VMON:
+	case EXIT_REASON_INVEPT:
 		/*
 		 * VMX instructions trap unconditionally. This allows L1 to
 		 * emulate them for its L2 guest, i.e., allows 3-level nesting!
-- 
1.8.2.1.610.g562af5b

[PATCH 09/11] nEPT: Documentation

[Jun Nakajima]

Update the documentation to no longer say that nested EPT is not supported.

Signed-off-by: Nadav Har'El <nyh@il.ibm.com>
Signed-off-by: Jun Nakajima <jun.nakajima@intel.com>
Signed-off-by: Xinhao Xu <xinhao.xu@intel.com>
---
 Documentation/virtual/kvm/nested-vmx.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/virtual/kvm/nested-vmx.txt b/Documentation/virtual/kvm/nested-vmx.txt
index 8ed937d..cdf7839 100644
--- a/Documentation/virtual/kvm/nested-vmx.txt
+++ b/Documentation/virtual/kvm/nested-vmx.txt
@@ -38,8 +38,8 @@ The current code supports running Linux guests under KVM guests.
 Only 64-bit guest hypervisors are supported.
 
 Additional patches for running Windows under guest KVM, and Linux under
-guest VMware server, and support for nested EPT, are currently running in
-the lab, and will be sent as follow-on patchsets.
+guest VMware server, are currently running in the lab, and will be sent as
+follow-on patchsets.
 
 
 Running nested VMX
-- 
1.8.2.1.610.g562af5b

[PATCH 10/11] nEPT: Miscelleneous cleanups

[Jun Nakajima]

Some trivial code cleanups not really related to nested EPT.

Signed-off-by: Nadav Har'El <nyh@il.ibm.com>
Signed-off-by: Jun Nakajima <jun.nakajima@intel.com>
Signed-off-by: Xinhao Xu <xinhao.xu@intel.com>
---
 arch/x86/kvm/vmx.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index c67eb06..95304cc 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -616,7 +616,6 @@ static void nested_release_page_clean(struct page *page)
 static u64 construct_eptp(unsigned long root_hpa);
 static void kvm_cpu_vmxon(u64 addr);
 static void kvm_cpu_vmxoff(void);
-static void vmx_set_cr3(struct kvm_vcpu *vcpu, unsigned long cr3);
 static int vmx_set_tss_addr(struct kvm *kvm, unsigned int addr);
 static void vmx_set_segment(struct kvm_vcpu *vcpu,
 			    struct kvm_segment *var, int seg);
@@ -912,8 +911,7 @@ static inline bool nested_cpu_has2(struct vmcs12 *vmcs12, u32 bit)
 		(vmcs12->secondary_vm_exec_control & bit);
 }
 
-static inline bool nested_cpu_has_virtual_nmis(struct vmcs12 *vmcs12,
-	struct kvm_vcpu *vcpu)
+static inline bool nested_cpu_has_virtual_nmis(struct vmcs12 *vmcs12)
 {
 	return vmcs12->pin_based_vm_exec_control & PIN_BASED_VIRTUAL_NMIS;
 }
@@ -6321,7 +6319,7 @@ static int vmx_handle_exit(struct kvm_vcpu *vcpu)
 
 	if (unlikely(!cpu_has_virtual_nmis() && vmx->soft_vnmi_blocked &&
 	    !(is_guest_mode(vcpu) && nested_cpu_has_virtual_nmis(
-	                                get_vmcs12(vcpu), vcpu)))) {
+					get_vmcs12(vcpu))))) {
 		if (vmx_interrupt_allowed(vcpu)) {
 			vmx->soft_vnmi_blocked = 0;
 		} else if (vmx->vnmi_blocked_time > 1000000000LL &&
-- 
1.8.2.1.610.g562af5b

[PATCH 11/11] nEPT: Provide the correct exit qualification upon EPT

[Jun Nakajima]

Save [2:0] of exit qualificaiton at EPT violation, and use the information when injecting EPT violation.

Signed-off-by: Jun Nakajima <jun.nakajima@intel.com>
Signed-off-by: Xinhao Xu <xinhao.xu@intel.com>
---
 arch/x86/include/asm/kvm_host.h | 2 ++
 arch/x86/kvm/paging_tmpl.h      | 5 +++++
 arch/x86/kvm/vmx.c              | 3 +++
 3 files changed, 10 insertions(+)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 4979778..e029bba 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -504,6 +504,8 @@ struct kvm_vcpu_arch {
 	 * instruction.
 	 */
 	bool write_fault_to_shadow_pgtable;
+
+	unsigned long exit_qualification; /* set at EPT violation at this point */
 };
 
 struct kvm_lpage_info {
diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
index e13b6c5..bd370e7 100644
--- a/arch/x86/kvm/paging_tmpl.h
+++ b/arch/x86/kvm/paging_tmpl.h
@@ -349,7 +349,12 @@ error:
 
 	walker->fault.vector = PF_VECTOR;
 	walker->fault.error_code_valid = true;
+#if PTTYPE != PTTYPE_EPT
 	walker->fault.error_code = errcode;
+#else
+	/* Reuse bits [2:0] of EPT violation */
+	walker->fault.error_code = vcpu->arch.exit_qualification & 0x7;
+#endif
 	walker->fault.address = addr;
 	walker->fault.nested_page_fault = mmu != vcpu->arch.walk_mmu;
 
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 95304cc..61e2853 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -425,6 +425,7 @@ struct vcpu_vmx {
 	ktime_t entry_time;
 	s64 vnmi_blocked_time;
 	u32 exit_reason;
+	unsigned long exit_qualification;
 
 	bool rdtscp_enabled;
 
@@ -5074,6 +5075,8 @@ static int handle_ept_violation(struct kvm_vcpu *vcpu)
 	/* ept page table is present? */
 	error_code |= (exit_qualification >> 3) & 0x1;
 
+    vcpu->arch.exit_qualification = exit_qualification;
+
 	return kvm_mmu_page_fault(vcpu, gpa, error_code, NULL, 0);
 }
 
-- 
1.8.2.1.610.g562af5b

RE: [PATCH 11/11] nEPT: Provide the correct exit qualification upon EPT

[Xu, Xinhao]

>From 2df72c1e8e3b167a6008ab11e2a68d734c68e425 Mon Sep 17 00:00:00 2001
From: Xinhao Xu <xinhao.xu@intel.com>
Date: Sat, 27 Apr 2013 05:45:49 +0800
Subject: [PATCH] nEPT: Check EPT misconfiguration while walking addr & move pte
 check code to vmx.c

I add code to detect EPT misconfiguration and inject it to L1.
Now L1 can correctly go to ept_misconfig handler(instead of
wrongly going to fast_page_fault), it will try to handle mmio
page fault, if failed, it is a real ept misconfiguration.

For scalability, Xiantao suggests me moving vendor specific
code out from common code. In order to do this, I add new ops
in kvm_mmu struct, check_tdp_pte, to provide an interface to
check fault while walking address. The better way is to do
fault detecting at here, but so far I just have checked ept
misconfiguration. More patches will be added in future.

Signed-off-by: Jun Nakajima <jun.nakajima@intel.com>
Signed-off-by: Xinhao Xu <xinhao.xu@intel.com>
---
 arch/x86/include/asm/kvm_emulate.h |    2 +
 arch/x86/include/asm/kvm_host.h    |    2 +
 arch/x86/kvm/cpuid.c               |    1 +
 arch/x86/kvm/mmu.c                 |    5 --
 arch/x86/kvm/mmu.h                 |    5 ++
 arch/x86/kvm/paging_tmpl.h         |   25 ++++++++---
 arch/x86/kvm/vmx.c                 |   79 +++++++++++++++++++++++++++++++++++-
 7 files changed, 106 insertions(+), 13 deletions(-)

diff --git a/arch/x86/include/asm/kvm_emulate.h b/arch/x86/include/asm/kvm_emulate.h
index 15f960c..c74f7c6 100644
--- a/arch/x86/include/asm/kvm_emulate.h
+++ b/arch/x86/include/asm/kvm_emulate.h
@@ -21,6 +21,8 @@ struct x86_exception {
 	u8 vector;
 	bool error_code_valid;
 	u16 error_code;
+	u64 exit_qualification;
+	bool fault_checked_by_vendor; /* is this fault checked by vendor */
 	bool nested_page_fault;
 	u64 address; /* cr2 or nested page fault gpa */
 };
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index e029bba..0fedfcd 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -262,6 +262,8 @@ struct kvm_mmu {
 	void (*invlpg)(struct kvm_vcpu *vcpu, gva_t gva);
 	void (*update_pte)(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp,
 			   u64 *spte, const void *pte);
+	bool (*check_tdp_pte)(struct kvm_vcpu *vcpu, u64 pte, int level,
+			struct x86_exception *fault);
 	hpa_t root_hpa;
 	int root_level;
 	int shadow_root_level;
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index a20ecb5..51dd1f9 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -621,6 +621,7 @@ int cpuid_maxphyaddr(struct kvm_vcpu *vcpu)
 not_found:
 	return 36;
 }
+EXPORT_SYMBOL_GPL(cpuid_maxphyaddr);
 
 /*
  * If no match is found, check whether we exceed the vCPU's limit
diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 99bfc5e..e1f5ea1 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -230,11 +230,6 @@ static bool set_mmio_spte(u64 *sptep, gfn_t gfn, pfn_t pfn, unsigned access)
 	return false;
 }
 
-static inline u64 rsvd_bits(int s, int e)
-{
-	return ((1ULL << (e - s + 1)) - 1) << s;
-}
-
 void kvm_mmu_set_mask_ptes(u64 user_mask, u64 accessed_mask,
 		u64 dirty_mask, u64 nx_mask, u64 x_mask)
 {
diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
index 19dd5ab..8aebd5a 100644
--- a/arch/x86/kvm/mmu.h
+++ b/arch/x86/kvm/mmu.h
@@ -91,6 +91,11 @@ static inline bool is_write_protection(struct kvm_vcpu *vcpu)
 	return kvm_read_cr0_bits(vcpu, X86_CR0_WP);
 }
 
+static inline u64 rsvd_bits(int s, int e)
+{
+	return ((1ULL << (e - s + 1)) - 1) << s;
+}
+
 /*
  * Will a fault with a given page-fault error code (pfec) cause a permission
  * fault with the given access (in ACC_* format)?
diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
index bd370e7..09e7154 100644
--- a/arch/x86/kvm/paging_tmpl.h
+++ b/arch/x86/kvm/paging_tmpl.h
@@ -240,6 +240,7 @@ retry_walk:
 	}
 #endif
 	walker->max_level = walker->level;
+	walker->fault.fault_checked_by_vendor = false;
 	ASSERT((!is_long_mode(vcpu) && is_pae(vcpu)) ||
 	       (mmu->get_cr3(vcpu) & CR3_NONPAE_RESERVED_BITS) == 0);
 
@@ -282,14 +283,22 @@ retry_walk:
 		if (unlikely(!FNAME(is_present_gpte)(pte)))
 			goto error;
 
+#if PTTYPE != PTTYPE_EPT
 		if (unlikely(is_rsvd_bits_set(&vcpu->arch.mmu, pte,
 					      walker->level))) {
 			errcode |= PFERR_PRESENT_MASK;
-#if PTTYPE != PTTYPE_EPT
 			errcode |= PFERR_RSVD_MASK;
-#endif
 			goto error;
 		}
+#else
+                if (vcpu->arch.mmu.check_tdp_pte)
+                {
+                        if (vcpu->arch.mmu.check_tdp_pte(vcpu, pte,
+                                walker->level, &walker->fault))
+                        goto error;
+                }
+
+#endif
 
 		accessed_dirty &= pte;
 		pte_access = pt_access & FNAME(gpte_access)(vcpu, pte);
@@ -349,12 +358,14 @@ error:
 
 	walker->fault.vector = PF_VECTOR;
 	walker->fault.error_code_valid = true;
-#if PTTYPE != PTTYPE_EPT
 	walker->fault.error_code = errcode;
-#else
-	/* Reuse bits [2:0] of EPT violation */
-	walker->fault.error_code = vcpu->arch.exit_qualification & 0x7;
-#endif
+	/*
+	 * If it is not fault checked in  check_tdp_pte, for now, it is a
+	 * EPT violation, set correct exit qualification according to Intel
+	 * SDM. This part will be changed in future.*/
+        if (!walker->fault.fault_checked_by_vendor)
+                walker->fault.exit_qualification = ((walker->pte_access &
+					0x7) << 3) | access;
 	walker->fault.address = addr;
 	walker->fault.nested_page_fault = mmu != vcpu->arch.walk_mmu;
 
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 61e2853..a450c84 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -7006,10 +7006,86 @@ static void nested_ept_inject_page_fault(struct kvm_vcpu *vcpu,
 	 * Note no need to set vmcs12->vm_exit_reason as it is already copied
 	 * from vmcs02 in nested_vmx_vmexit() above, i.e., EPT_VIOLATION.
 	 */
-	vmcs12->exit_qualification = fault->error_code;
+	if (fault->fault_checked_by_vendor && fault->exit_qualification == 0)
+                vmcs12->vm_exit_reason = EXIT_REASON_EPT_MISCONFIG;
+        else
+                vmcs12->vm_exit_reason = EXIT_REASON_EPT_VIOLATION;
+        vmcs12->exit_qualification = fault->exit_qualification;
 	vmcs12->guest_physical_address = fault->address;
 }
 
+static bool nested_ept_rsvd_bits_check(struct kvm_vcpu *vcpu, u64 pte, int level)
+{
+	int maxphyaddr = cpuid_maxphyaddr(vcpu);
+	u64 rsvd_mask = rsvd_bits(maxphyaddr, 51);
+
+	switch (level)
+	{
+		case 4:
+			rsvd_mask |= rsvd_bits(3, 7);
+			break;
+		case 3:
+		case 2:
+			if (pte & (1 << 7))
+				rsvd_mask |= rsvd_bits(PAGE_SHIFT, PAGE_SHIFT + 9 * (level - 1) - 1);
+			else
+				rsvd_mask |= rsvd_bits(3, 6);
+			break;
+		case 1:
+			break;
+		default:
+			/* impossible to go to here */
+			BUG();
+	}
+	return pte & rsvd_mask;
+}
+
+static bool nested_ept_rwx_bits_check(u64 pte)
+{
+	/* write only or write/execute only */
+	uint8_t rwx_bits = pte & 7;
+
+	switch (rwx_bits)
+	{
+		case 0x2:
+		case 0x6:
+			return 1;
+		case 0x4:
+			if (!(nested_vmx_ept_caps & 0x1))
+				return 1;
+		default:
+			return 0;
+	}
+}
+
+static bool nested_ept_memtype_check(u64 pte, int level)
+{
+	if (level == 1 || (level == 2 && (pte & (1ULL << 7)))) {
+		/* 0x38, namely bits 5:3, stands for EPT memory type */
+		u64 ept_mem_type = (pte & 0x38) >> 3;
+
+		if (ept_mem_type == 0x2 || ept_mem_type == 0x3 ||
+			ept_mem_type == 0x7)
+			return 1;
+	}
+	return 0;
+}
+
+bool nested_check_ept_pte(struct kvm_vcpu *vcpu, u64 pte, int level,
+				struct x86_exception *fault)
+{
+	bool r;
+	r = nested_ept_rsvd_bits_check(vcpu, pte, level) ||
+		nested_ept_rwx_bits_check(pte) ||
+		nested_ept_memtype_check(pte, level);
+	if (r)
+	{
+		fault->exit_qualification = 0;
+		fault->fault_checked_by_vendor = true;
+	}
+	return r;
+}
+
 static int nested_ept_init_mmu_context(struct kvm_vcpu *vcpu)
 {
 	int r = kvm_init_shadow_EPT_mmu(vcpu, &vcpu->arch.mmu);
@@ -7017,6 +7093,7 @@ static int nested_ept_init_mmu_context(struct kvm_vcpu *vcpu)
 	vcpu->arch.mmu.set_cr3           = vmx_set_cr3;
 	vcpu->arch.mmu.get_cr3           = nested_ept_get_cr3;
 	vcpu->arch.mmu.inject_page_fault = nested_ept_inject_page_fault;
+	vcpu->arch.mmu.check_tdp_pte     = nested_check_ept_pte;
 
 	vcpu->arch.walk_mmu              = &vcpu->arch.nested_mmu;
 
-- 
1.7.1


-----Original Message-----
From: kvm-owner@vger.kernel.org [mailto:kvm-owner@vger.kernel.org] On Behalf Of Jun Nakajima
Sent: Friday, April 26, 2013 2:44 PM
To: kvm@vger.kernel.org
Subject: [PATCH 11/11] nEPT: Provide the correct exit qualification upon EPT

Save [2:0] of exit qualificaiton at EPT violation, and use the information when injecting EPT violation.

Signed-off-by: Jun Nakajima <jun.nakajima@intel.com>
Signed-off-by: Xinhao Xu <xinhao.xu@intel.com>
---
 arch/x86/include/asm/kvm_host.h | 2 ++
 arch/x86/kvm/paging_tmpl.h      | 5 +++++
 arch/x86/kvm/vmx.c              | 3 +++
 3 files changed, 10 insertions(+)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index 4979778..e029bba 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -504,6 +504,8 @@ struct kvm_vcpu_arch {
 	 * instruction.
 	 */
 	bool write_fault_to_shadow_pgtable;
+
+	unsigned long exit_qualification; /* set at EPT violation at this 
+point */
 };
 
 struct kvm_lpage_info {
diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h index e13b6c5..bd370e7 100644
--- a/arch/x86/kvm/paging_tmpl.h
+++ b/arch/x86/kvm/paging_tmpl.h
@@ -349,7 +349,12 @@ error:
 
 	walker->fault.vector = PF_VECTOR;
 	walker->fault.error_code_valid = true;
+#if PTTYPE != PTTYPE_EPT
 	walker->fault.error_code = errcode;
+#else
+	/* Reuse bits [2:0] of EPT violation */
+	walker->fault.error_code = vcpu->arch.exit_qualification & 0x7; #endif
 	walker->fault.address = addr;
 	walker->fault.nested_page_fault = mmu != vcpu->arch.walk_mmu;
 
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 95304cc..61e2853 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -425,6 +425,7 @@ struct vcpu_vmx {
 	ktime_t entry_time;
 	s64 vnmi_blocked_time;
 	u32 exit_reason;
+	unsigned long exit_qualification;
 
 	bool rdtscp_enabled;
 
@@ -5074,6 +5075,8 @@ static int handle_ept_violation(struct kvm_vcpu *vcpu)
 	/* ept page table is present? */
 	error_code |= (exit_qualification >> 3) & 0x1;
 
+    vcpu->arch.exit_qualification = exit_qualification;
+
 	return kvm_mmu_page_fault(vcpu, gpa, error_code, NULL, 0);  }
 
--
1.8.2.1.610.g562af5b

--
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 11/11] nEPT: Provide the correct exit qualification upon EPT

[Jan Kiszka]

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

On 2013-04-27 08:42, Xu, Xinhao wrote:
> From 2df72c1e8e3b167a6008ab11e2a68d734c68e425 Mon Sep 17 00:00:00 2001
> From: Xinhao Xu <xinhao.xu@intel.com>
> Date: Sat, 27 Apr 2013 05:45:49 +0800
> Subject: [PATCH] nEPT: Check EPT misconfiguration while walking addr & move pte
>  check code to vmx.c

Please fix up the subject when posting a patch in reply to another one.

> 
> I add code to detect EPT misconfiguration and inject it to L1.
> Now L1 can correctly go to ept_misconfig handler(instead of
> wrongly going to fast_page_fault), it will try to handle mmio
> page fault, if failed, it is a real ept misconfiguration.
> 
> For scalability, Xiantao suggests me moving vendor specific
> code out from common code. In order to do this, I add new ops
> in kvm_mmu struct, check_tdp_pte, to provide an interface to
> check fault while walking address. The better way is to do
> fault detecting at here, but so far I just have checked ept
> misconfiguration. More patches will be added in future.

Seems there are some issues remaining. I can boot Linux as L2 when I
remove this patch. When it's applied, L2 becomes pretty slow and
eventually resets during kernel boot of L2. L1 remains stable.

Jan



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

RE: [PATCH 11/11] nEPT: Provide the correct exit qualification upon EPT

[Xu, Xinhao]

Hi, Jan
Can you provide details of your test environment?

-----Original Message-----
From: Jan Kiszka [mailto:jan.kiszka@web.de] 
Sent: Sunday, April 28, 2013 5:36 PM
To: Xu, Xinhao
Cc: Nakajima, Jun; kvm@vger.kernel.org
Subject: Re: [PATCH 11/11] nEPT: Provide the correct exit qualification upon EPT

On 2013-04-27 08:42, Xu, Xinhao wrote:
> From 2df72c1e8e3b167a6008ab11e2a68d734c68e425 Mon Sep 17 00:00:00 2001
> From: Xinhao Xu <xinhao.xu@intel.com>
> Date: Sat, 27 Apr 2013 05:45:49 +0800
> Subject: [PATCH] nEPT: Check EPT misconfiguration while walking addr & 
> move pte  check code to vmx.c

Please fix up the subject when posting a patch in reply to another one.

> 
> I add code to detect EPT misconfiguration and inject it to L1.
> Now L1 can correctly go to ept_misconfig handler(instead of wrongly 
> going to fast_page_fault), it will try to handle mmio page fault, if 
> failed, it is a real ept misconfiguration.
> 
> For scalability, Xiantao suggests me moving vendor specific code out 
> from common code. In order to do this, I add new ops in kvm_mmu 
> struct, check_tdp_pte, to provide an interface to check fault while 
> walking address. The better way is to do fault detecting at here, but 
> so far I just have checked ept misconfiguration. More patches will be 
> added in future.

Seems there are some issues remaining. I can boot Linux as L2 when I remove this patch. When it's applied, L2 becomes pretty slow and eventually resets during kernel boot of L2. L1 remains stable.

Jan

Re: [PATCH 11/11] nEPT: Provide the correct exit qualification upon EPT

[Jan Kiszka]

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

On 2013-05-02 08:59, Xu, Xinhao wrote:
> Hi, Jan
> Can you provide details of your test environment?

Pretty simple: Run a Linux 3.9(-rc5) kernel as L2 on top of the same
kernel with QEMU (git head at that time). I've attached the config.

Jan

> 
> -----Original Message-----
> From: Jan Kiszka [mailto:jan.kiszka@web.de] 
> Sent: Sunday, April 28, 2013 5:36 PM
> To: Xu, Xinhao
> Cc: Nakajima, Jun; kvm@vger.kernel.org
> Subject: Re: [PATCH 11/11] nEPT: Provide the correct exit qualification upon EPT
> 
> On 2013-04-27 08:42, Xu, Xinhao wrote:
>> From 2df72c1e8e3b167a6008ab11e2a68d734c68e425 Mon Sep 17 00:00:00 2001
>> From: Xinhao Xu <xinhao.xu@intel.com>
>> Date: Sat, 27 Apr 2013 05:45:49 +0800
>> Subject: [PATCH] nEPT: Check EPT misconfiguration while walking addr & 
>> move pte  check code to vmx.c
> 
> Please fix up the subject when posting a patch in reply to another one.
> 
>>
>> I add code to detect EPT misconfiguration and inject it to L1.
>> Now L1 can correctly go to ept_misconfig handler(instead of wrongly 
>> going to fast_page_fault), it will try to handle mmio page fault, if 
>> failed, it is a real ept misconfiguration.
>>
>> For scalability, Xiantao suggests me moving vendor specific code out 
>> from common code. In order to do this, I add new ops in kvm_mmu 
>> struct, check_tdp_pte, to provide an interface to check fault while 
>> walking address. The better way is to do fault detecting at here, but 
>> so far I just have checked ept misconfiguration. More patches will be 
>> added in future.
> 
> Seems there are some issues remaining. I can boot Linux as L2 when I remove this patch. When it's applied, L2 becomes pretty slow and eventually resets during kernel boot of L2. L1 remains stable.
> 
> Jan
> 
> 

-- 
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux

[-- Attachment #2: .config.bz2 --]
[-- Type: application/x-bzip, Size: 20476 bytes --]

Re: [PATCH 11/11] nEPT: Provide the correct exit qualification upon EPT

[Paolo Bonzini]

Il 26/04/2013 08:43, Jun Nakajima ha scritto:
> diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
> index e13b6c5..bd370e7 100644
> --- a/arch/x86/kvm/paging_tmpl.h
> +++ b/arch/x86/kvm/paging_tmpl.h
> @@ -349,7 +349,12 @@ error:
>  
>  	walker->fault.vector = PF_VECTOR;
>  	walker->fault.error_code_valid = true;
> +#if PTTYPE != PTTYPE_EPT
>  	walker->fault.error_code = errcode;
> +#else
> +	/* Reuse bits [2:0] of EPT violation */
> +	walker->fault.error_code = vcpu->arch.exit_qualification & 0x7;
> +#endif
>  	walker->fault.address = addr;
>  	walker->fault.nested_page_fault = mmu != vcpu->arch.walk_mmu;
>  

I'm not sure that this is a step in the right direction.

errcode is dropped completely, but it would be needed to rebuild bits
3:5 of the exit qualification.

Perhaps it is better to access vcpu->arch.exit_qualification in
nested_ept_inject_page_fault, and mix it with the error code from
walker->fault to compute bits 3:5?

Paolo

Re: [PATCH 11/11] nEPT: Provide the correct exit qualification upon EPT

[Nakajima, Jun]

On Mon, Apr 29, 2013 at 8:37 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 26/04/2013 08:43, Jun Nakajima ha scritto:
>> diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
>> index e13b6c5..bd370e7 100644
>> --- a/arch/x86/kvm/paging_tmpl.h
>> +++ b/arch/x86/kvm/paging_tmpl.h
>> @@ -349,7 +349,12 @@ error:
>>
>>       walker->fault.vector = PF_VECTOR;
>>       walker->fault.error_code_valid = true;
>> +#if PTTYPE != PTTYPE_EPT
>>       walker->fault.error_code = errcode;
>> +#else
>> +     /* Reuse bits [2:0] of EPT violation */
>> +     walker->fault.error_code = vcpu->arch.exit_qualification & 0x7;
>> +#endif
>>       walker->fault.address = addr;
>>       walker->fault.nested_page_fault = mmu != vcpu->arch.walk_mmu;
>>
>
> I'm not sure that this is a step in the right direction.
>
> errcode is dropped completely, but it would be needed to rebuild bits
> 3:5 of the exit qualification.
>
> Perhaps it is better to access vcpu->arch.exit_qualification in
> nested_ept_inject_page_fault, and mix it with the error code from
> walker->fault to compute bits 3:5?

Yes. We need to generate those bits from the walk of the guest EPT
page tables, and combine them.

I'll update the patches, fixing the issues in Xinhao's patch.

--
Jun
Intel Open Source Technology Center

Re: [PATCH 10/11] nEPT: Miscelleneous cleanups

[Paolo Bonzini]

Il 26/04/2013 08:43, Jun Nakajima ha scritto:
> Some trivial code cleanups not really related to nested EPT.
> 
> Signed-off-by: Nadav Har'El <nyh@il.ibm.com>
> Signed-off-by: Jun Nakajima <jun.nakajima@intel.com>
> Signed-off-by: Xinhao Xu <xinhao.xu@intel.com>
> ---
>  arch/x86/kvm/vmx.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index c67eb06..95304cc 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -616,7 +616,6 @@ static void nested_release_page_clean(struct page *page)
>  static u64 construct_eptp(unsigned long root_hpa);
>  static void kvm_cpu_vmxon(u64 addr);
>  static void kvm_cpu_vmxoff(void);
> -static void vmx_set_cr3(struct kvm_vcpu *vcpu, unsigned long cr3);
>  static int vmx_set_tss_addr(struct kvm *kvm, unsigned int addr);
>  static void vmx_set_segment(struct kvm_vcpu *vcpu,
>  			    struct kvm_segment *var, int seg);
> @@ -912,8 +911,7 @@ static inline bool nested_cpu_has2(struct vmcs12 *vmcs12, u32 bit)
>  		(vmcs12->secondary_vm_exec_control & bit);
>  }
>  
> -static inline bool nested_cpu_has_virtual_nmis(struct vmcs12 *vmcs12,
> -	struct kvm_vcpu *vcpu)
> +static inline bool nested_cpu_has_virtual_nmis(struct vmcs12 *vmcs12)
>  {
>  	return vmcs12->pin_based_vm_exec_control & PIN_BASED_VIRTUAL_NMIS;
>  }
> @@ -6321,7 +6319,7 @@ static int vmx_handle_exit(struct kvm_vcpu *vcpu)
>  
>  	if (unlikely(!cpu_has_virtual_nmis() && vmx->soft_vnmi_blocked &&
>  	    !(is_guest_mode(vcpu) && nested_cpu_has_virtual_nmis(
> -	                                get_vmcs12(vcpu), vcpu)))) {
> +					get_vmcs12(vcpu))))) {
>  		if (vmx_interrupt_allowed(vcpu)) {
>  			vmx->soft_vnmi_blocked = 0;
>  		} else if (vmx->vnmi_blocked_time > 1000000000LL &&
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [PATCH 07/11] nEPT: Advertise EPT to L1

[Jan Kiszka]

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

On 2013-04-26 08:43, Jun Nakajima wrote:
> Advertise the support of EPT to the L1 guest, through the appropriate MSR.
> 
> This is the last patch of the basic Nested EPT feature, so as to allow
> bisection through this patch series: The guest will not see EPT support until
> this last patch, and will not attempt to use the half-applied feature.
> 
> Signed-off-by: Nadav Har'El <nyh@il.ibm.com>
> Signed-off-by: Jun Nakajima <jun.nakajima@intel.com>
> Signed-off-by: Xinhao Xu <xinhao.xu@intel.com>
> ---
>  arch/x86/kvm/vmx.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 66ead51..76df3a8 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -2027,6 +2027,7 @@ static u32 nested_vmx_secondary_ctls_low, nested_vmx_secondary_ctls_high;
>  static u32 nested_vmx_pinbased_ctls_low, nested_vmx_pinbased_ctls_high;
>  static u32 nested_vmx_exit_ctls_low, nested_vmx_exit_ctls_high;
>  static u32 nested_vmx_entry_ctls_low, nested_vmx_entry_ctls_high;
> +static u32 nested_vmx_ept_caps;
>  static __init void nested_vmx_setup_ctls_msrs(void)
>  {
>  	/*
> @@ -2102,6 +2103,18 @@ static __init void nested_vmx_setup_ctls_msrs(void)
>  	nested_vmx_secondary_ctls_low = 0;
>  	nested_vmx_secondary_ctls_high &=
>  		SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES;
> +	if (enable_ept) {
> +		/* nested EPT: emulate EPT also to L1 */
> +		nested_vmx_secondary_ctls_high |= SECONDARY_EXEC_ENABLE_EPT;
> +		nested_vmx_ept_caps = VMX_EPT_PAGE_WALK_4_BIT;
> +		nested_vmx_ept_caps |=
> +			VMX_EPT_INVEPT_BIT | VMX_EPT_EXTENT_GLOBAL_BIT |
> +			VMX_EPT_EXTENT_CONTEXT_BIT |
> +			VMX_EPT_EXTENT_INDIVIDUAL_BIT;
> +		nested_vmx_ept_caps &= vmx_capability.ept;
> +	} else
> +		nested_vmx_ept_caps = 0;
> +
>  }
>  
>  static inline bool vmx_control_verify(u32 control, u32 low, u32 high)
> @@ -2201,8 +2214,8 @@ static int vmx_get_vmx_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 *pdata)
>  					nested_vmx_secondary_ctls_high);
>  		break;
>  	case MSR_IA32_VMX_EPT_VPID_CAP:
> -		/* Currently, no nested ept or nested vpid */
> -		*pdata = 0;
> +		/* Currently, no nested vpid support */
> +		*pdata = nested_vmx_ept_caps;
>  		break;
>  	default:
>  		return 0;
> 

This patch depends on #defines from the next one.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH 02/11] nEPT: Add EPT tables support to paging_tmpl.h

[Paolo Bonzini]

Il 26/04/2013 08:43, Jun Nakajima ha scritto:
> This is the first patch in a series which adds nested EPT support to KVM's
> nested VMX. Nested EPT means emulating EPT for an L1 guest so that L1 can use
> EPT when running a nested guest L2. When L1 uses EPT, it allows the L2 guest
> to set its own cr3 and take its own page faults without either of L0 or L1
> getting involved. This often significanlty improves L2's performance over the
> previous two alternatives (shadow page tables over EPT, and shadow page
> tables over shadow page tables).
> 
> This patch adds EPT support to paging_tmpl.h.
> 
> paging_tmpl.h contains the code for reading and writing page tables. The code
> for 32-bit and 64-bit tables is very similar, but not identical, so
> paging_tmpl.h is #include'd twice in mmu.c, once with PTTTYPE=32 and once
> with PTTYPE=64, and this generates the two sets of similar functions.
> 
> There are subtle but important differences between the format of EPT tables
> and that of ordinary x86 64-bit page tables, so for nested EPT we need a
> third set of functions to read the guest EPT table and to write the shadow
> EPT table.
> 
> So this patch adds third PTTYPE, PTTYPE_EPT, which creates functions (prefixed
> with "EPT") which correctly read and write EPT tables.
> 
> Signed-off-by: Nadav Har'El <nyh@il.ibm.com>
> Signed-off-by: Jun Nakajima <jun.nakajima@intel.com>
> Signed-off-by: Xinhao Xu <xinhao.xu@intel.com>
> ---
>  arch/x86/kvm/mmu.c         |  35 ++----------
>  arch/x86/kvm/paging_tmpl.h | 133 ++++++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 130 insertions(+), 38 deletions(-)

I would split this patch so that first prefetch_invalid_gpte and
gpte_access are moved to paging_tmpl.h (adding the FNAME everywhere).
The second patch then can add the EPT special cases.

> 
> +static inline int FNAME(check_write_user_access)(struct kvm_vcpu *vcpu,
> +					   bool write_fault, bool user_fault,
> +					   unsigned long pte)
> +{
> +#if PTTYPE == PTTYPE_EPT
> +	if (unlikely(write_fault && !(pte & VMX_EPT_WRITABLE_MASK)
> +				 && (user_fault || is_write_protection(vcpu))))
> +		return false;
> +	return true;
> +#else
> +	u32 access = ((kvm_x86_ops->get_cpl(vcpu) == 3) ? PFERR_USER_MASK : 0)
> +                | (write_fault ? PFERR_WRITE_MASK : 0);
>  
> +	return !permission_fault(vcpu->arch.walk_mmu, vcpu->arch.access, access);
> +#endif
> +}
> +

I think check_write_user_access doesn't exist anymore?  Perhaps a wrong
conflict resolution.

Paolo

Re: [PATCH 02/11] nEPT: Add EPT tables support to paging_tmpl.h

[Marcelo Tosatti]

On Thu, Apr 25, 2013 at 11:43:22PM -0700, Jun Nakajima wrote:
> This is the first patch in a series which adds nested EPT support to KVM's
> nested VMX. Nested EPT means emulating EPT for an L1 guest so that L1 can use
> EPT when running a nested guest L2. When L1 uses EPT, it allows the L2 guest
> to set its own cr3 and take its own page faults without either of L0 or L1
> getting involved. This often significanlty improves L2's performance over the
> previous two alternatives (shadow page tables over EPT, and shadow page
> tables over shadow page tables).
> 
> This patch adds EPT support to paging_tmpl.h.
> 
> paging_tmpl.h contains the code for reading and writing page tables. The code
> for 32-bit and 64-bit tables is very similar, but not identical, so
> paging_tmpl.h is #include'd twice in mmu.c, once with PTTTYPE=32 and once
> with PTTYPE=64, and this generates the two sets of similar functions.
> 
> There are subtle but important differences between the format of EPT tables
> and that of ordinary x86 64-bit page tables, so for nested EPT we need a
> third set of functions to read the guest EPT table and to write the shadow
> EPT table.
> 
> So this patch adds third PTTYPE, PTTYPE_EPT, which creates functions (prefixed
> with "EPT") which correctly read and write EPT tables.
> 
> Signed-off-by: Nadav Har'El <nyh@il.ibm.com>
> Signed-off-by: Jun Nakajima <jun.nakajima@intel.com>
> Signed-off-by: Xinhao Xu <xinhao.xu@intel.com>
> ---
>  arch/x86/kvm/mmu.c         |  35 ++----------
>  arch/x86/kvm/paging_tmpl.h | 133 ++++++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 130 insertions(+), 38 deletions(-)
> 
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index 956ca35..cb9c6fd 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -2480,26 +2480,6 @@ static pfn_t pte_prefetch_gfn_to_pfn(struct kvm_vcpu *vcpu, gfn_t gfn,
>  	return gfn_to_pfn_memslot_atomic(slot, gfn);
>  }
>  
> -static bool prefetch_invalid_gpte(struct kvm_vcpu *vcpu,
> -				  struct kvm_mmu_page *sp, u64 *spte,
> -				  u64 gpte)
> -{
> -	if (is_rsvd_bits_set(&vcpu->arch.mmu, gpte, PT_PAGE_TABLE_LEVEL))
> -		goto no_present;
> -
> -	if (!is_present_gpte(gpte))
> -		goto no_present;
> -
> -	if (!(gpte & PT_ACCESSED_MASK))
> -		goto no_present;
> -
> -	return false;
> -
> -no_present:
> -	drop_spte(vcpu->kvm, spte);
> -	return true;
> -}
> -
>  static int direct_pte_prefetch_many(struct kvm_vcpu *vcpu,
>  				    struct kvm_mmu_page *sp,
>  				    u64 *start, u64 *end)
> @@ -3399,16 +3379,6 @@ static bool sync_mmio_spte(u64 *sptep, gfn_t gfn, unsigned access,
>  	return false;
>  }
>  
> -static inline unsigned gpte_access(struct kvm_vcpu *vcpu, u64 gpte)
> -{
> -	unsigned access;
> -
> -	access = (gpte & (PT_WRITABLE_MASK | PT_USER_MASK)) | ACC_EXEC_MASK;
> -	access &= ~(gpte >> PT64_NX_SHIFT);
> -
> -	return access;
> -}
> -
>  static inline bool is_last_gpte(struct kvm_mmu *mmu, unsigned level, unsigned gpte)
>  {
>  	unsigned index;
> @@ -3418,6 +3388,11 @@ static inline bool is_last_gpte(struct kvm_mmu *mmu, unsigned level, unsigned gp
>  	return mmu->last_pte_bitmap & (1 << index);
>  }
>  
> +#define PTTYPE_EPT 18 /* arbitrary */
> +#define PTTYPE PTTYPE_EPT
> +#include "paging_tmpl.h"
> +#undef PTTYPE
> +

This breaks 

#if PTTYPE == 64
        if (walker->level == PT32E_ROOT_LEVEL) {
                pte = mmu->get_pdptr(vcpu, (addr >> 30) & 3);
                trace_kvm_mmu_paging_element(pte, walker->level);

At walk_addr_generic.

>  #define PTTYPE 64
>  #include "paging_tmpl.h"
>  #undef PTTYPE
> diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
> index 105dd5b..e13b6c5 100644
> --- a/arch/x86/kvm/paging_tmpl.h
> +++ b/arch/x86/kvm/paging_tmpl.h
> @@ -50,6 +50,22 @@
>  	#define PT_LEVEL_BITS PT32_LEVEL_BITS
>  	#define PT_MAX_FULL_LEVELS 2
>  	#define CMPXCHG cmpxchg
> +#elif PTTYPE == PTTYPE_EPT
> +	#define pt_element_t u64
> +	#define guest_walker guest_walkerEPT
> +	#define FNAME(name) EPT_##name
> +	#define PT_BASE_ADDR_MASK PT64_BASE_ADDR_MASK
> +	#define PT_LVL_ADDR_MASK(lvl) PT64_LVL_ADDR_MASK(lvl)
> +	#define PT_LVL_OFFSET_MASK(lvl) PT64_LVL_OFFSET_MASK(lvl)
> +	#define PT_INDEX(addr, level) PT64_INDEX(addr, level)
> +	#define PT_LEVEL_BITS PT64_LEVEL_BITS
> +	#ifdef CONFIG_X86_64
> +	#define PT_MAX_FULL_LEVELS 4
> +	#define CMPXCHG cmpxchg
> +	#else
> +	#define CMPXCHG cmpxchg64
> +	#define PT_MAX_FULL_LEVELS 2
> +	#endif
>  #else
>  	#error Invalid PTTYPE value
>  #endif
> @@ -80,6 +96,7 @@ static gfn_t gpte_to_gfn_lvl(pt_element_t gpte, int lvl)
>  	return (gpte & PT_LVL_ADDR_MASK(lvl)) >> PAGE_SHIFT;
>  }
>  
> +#if PTTYPE != PTTYPE_EPT

What is the reasoning for this? Please restrict #ifdef changes to code
where there is a difference in pagetable format (that is, code that
knows pagetable format).

>  static int FNAME(cmpxchg_gpte)(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu,
>  			       pt_element_t __user *ptep_user, unsigned index,
>  			       pt_element_t orig_pte, pt_element_t new_pte)
> @@ -102,7 +119,52 @@ static int FNAME(cmpxchg_gpte)(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu,
>  
>  	return (ret != orig_pte);
>  }
> +#endif
> +
> +static unsigned FNAME(gpte_access)(struct kvm_vcpu *vcpu, u64 gpte)
> +{
> +	unsigned access;
> +
> +#if PTTYPE == PTTYPE_EPT
> +	/* We rely here that ACC_WRITE_MASK==VMX_EPT_WRITABLE_MASK */
> +	access = (gpte & VMX_EPT_WRITABLE_MASK) | ACC_USER_MASK |
> +		((gpte & VMX_EPT_EXECUTABLE_MASK) ? ACC_EXEC_MASK : 0);

User bit is meaningless for when emulating EPT. Why return it?

> +#else
> +	access = (gpte & (PT_WRITABLE_MASK | PT_USER_MASK)) | ACC_EXEC_MASK;
> +	access &= ~(gpte >> PT64_NX_SHIFT);
> +#endif
> +
> +	return access;
> +}
> +
> +static inline int FNAME(is_present_gpte)(unsigned long pte)
> +{
> +#if PTTYPE == PTTYPE_EPT
> +	return pte & (VMX_EPT_READABLE_MASK | VMX_EPT_WRITABLE_MASK |
> +			VMX_EPT_EXECUTABLE_MASK);

This should be VMX_EPT_READABLE_MASK (which is what maps to the
normal pagetable present bit).

> +#else
> +	return is_present_gpte(pte);
> +#endif
> +}
> +
> +static inline int FNAME(check_write_user_access)(struct kvm_vcpu *vcpu,
> +					   bool write_fault, bool user_fault,
> +					   unsigned long pte)
> +{

Apparently unused.

> +#if PTTYPE == PTTYPE_EPT
> +	if (unlikely(write_fault && !(pte & VMX_EPT_WRITABLE_MASK)
> +				 && (user_fault || is_write_protection(vcpu))))
> +		return false;
> +	return true;
> +#else
> +	u32 access = ((kvm_x86_ops->get_cpl(vcpu) == 3) ? PFERR_USER_MASK : 0)
> +                | (write_fault ? PFERR_WRITE_MASK : 0);
>  
> +	return !permission_fault(vcpu->arch.walk_mmu, vcpu->arch.access, access);
> +#endif
> +}
> +
> +#if PTTYPE != PTTYPE_EPT

Should emulate EPT dirty bit support?

>  static int FNAME(update_accessed_dirty_bits)(struct kvm_vcpu *vcpu,
>  					     struct kvm_mmu *mmu,
>  					     struct guest_walker *walker,
> @@ -139,6 +201,7 @@ static int FNAME(update_accessed_dirty_bits)(struct kvm_vcpu *vcpu,
>  	}
>  	return 0;
>  }
> +#endif
>  
>  /*
>   * Fetch a guest pte for a guest virtual address
> @@ -147,7 +210,6 @@ static int FNAME(walk_addr_generic)(struct guest_walker *walker,
>  				    struct kvm_vcpu *vcpu, struct kvm_mmu *mmu,
>  				    gva_t addr, u32 access)
>  {
> -	int ret;
>  	pt_element_t pte;
>  	pt_element_t __user *uninitialized_var(ptep_user);
>  	gfn_t table_gfn;
> @@ -162,7 +224,9 @@ static int FNAME(walk_addr_generic)(struct guest_walker *walker,
>  	gfn_t gfn;
>  
>  	trace_kvm_mmu_pagetable_walk(addr, access);
> +#if PTTYPE != PTTYPE_EPT
>  retry_walk:
> +#endif
>  	walker->level = mmu->root_level;
>  	pte           = mmu->get_cr3(vcpu);
>  
> @@ -215,17 +279,20 @@ retry_walk:
>  
>  		trace_kvm_mmu_paging_element(pte, walker->level);
>  
> -		if (unlikely(!is_present_gpte(pte)))
> +		if (unlikely(!FNAME(is_present_gpte)(pte)))
>  			goto error;
>  
>  		if (unlikely(is_rsvd_bits_set(&vcpu->arch.mmu, pte,
>  					      walker->level))) {
> -			errcode |= PFERR_RSVD_MASK | PFERR_PRESENT_MASK;
> +			errcode |= PFERR_PRESENT_MASK;
> +#if PTTYPE != PTTYPE_EPT
> +			errcode |= PFERR_RSVD_MASK;
> +#endif
>  			goto error;
>  		}
>  
>  		accessed_dirty &= pte;
> -		pte_access = pt_access & gpte_access(vcpu, pte);
> +		pte_access = pt_access & FNAME(gpte_access)(vcpu, pte);
>  
>  		walker->ptes[walker->level - 1] = pte;
>  	} while (!is_last_gpte(mmu, walker->level, pte));
> @@ -247,6 +314,7 @@ retry_walk:
>  
>  	walker->gfn = real_gpa >> PAGE_SHIFT;
>  
> +#if PTTYPE != PTTYPE_EPT
>  	if (!write_fault)
>  		protect_clean_gpte(&pte_access, pte);
>  	else
> @@ -257,12 +325,15 @@ retry_walk:
>  		accessed_dirty &= pte >> (PT_DIRTY_SHIFT - PT_ACCESSED_SHIFT);
>  
>  	if (unlikely(!accessed_dirty)) {
> +		int ret;
> +
>  		ret = FNAME(update_accessed_dirty_bits)(vcpu, mmu, walker, write_fault);
>  		if (unlikely(ret < 0))
>  			goto error;
>  		else if (ret)
>  			goto retry_walk;
>  	}
> +#endif
>  
>  	walker->pt_access = pt_access;
>  	walker->pte_access = pte_access;
> @@ -293,6 +364,7 @@ static int FNAME(walk_addr)(struct guest_walker *walker,
>  					access);
>  }
>  
> +#if PTTYPE != PTTYPE_EPT
>  static int FNAME(walk_addr_nested)(struct guest_walker *walker,
>  				   struct kvm_vcpu *vcpu, gva_t addr,
>  				   u32 access)
> @@ -300,6 +372,29 @@ static int FNAME(walk_addr_nested)(struct guest_walker *walker,
>  	return FNAME(walk_addr_generic)(walker, vcpu, &vcpu->arch.nested_mmu,
>  					addr, access);
>  }
> +#endif
> +
> +static bool FNAME(prefetch_invalid_gpte)(struct kvm_vcpu *vcpu,
> +				  struct kvm_mmu_page *sp, u64 *spte,
> +				  u64 gpte)
> +{
> +	if (is_rsvd_bits_set(&vcpu->arch.mmu, gpte, PT_PAGE_TABLE_LEVEL))
> +		goto no_present;
> +
> +	if (!is_present_gpte(gpte))
> +		goto no_present;
> +
> +#if PTTYPE != PTTYPE_EPT
> +	if (!(gpte & PT_ACCESSED_MASK))
> +		goto no_present;
> +#endif
> +
> +	return false;
> +
> +no_present:
> +	drop_spte(vcpu->kvm, spte);
> +	return true;
> +}

Please split code move into a separate patch.

>  static bool
>  FNAME(prefetch_gpte)(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp,
> @@ -309,13 +404,13 @@ FNAME(prefetch_gpte)(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp,
>  	gfn_t gfn;
>  	pfn_t pfn;
>  
> -	if (prefetch_invalid_gpte(vcpu, sp, spte, gpte))
> +	if (FNAME(prefetch_invalid_gpte)(vcpu, sp, spte, gpte))
>  		return false;
>  
>  	pgprintk("%s: gpte %llx spte %p\n", __func__, (u64)gpte, spte);
>  
>  	gfn = gpte_to_gfn(gpte);
> -	pte_access = sp->role.access & gpte_access(vcpu, gpte);
> +	pte_access = sp->role.access & FNAME(gpte_access)(vcpu, gpte);
>  	protect_clean_gpte(&pte_access, gpte);
>  	pfn = pte_prefetch_gfn_to_pfn(vcpu, gfn,
>  			no_dirty_log && (pte_access & ACC_WRITE_MASK));
> @@ -394,6 +489,18 @@ static void FNAME(pte_prefetch)(struct kvm_vcpu *vcpu, struct guest_walker *gw,
>  	}
>  }
>  
> +#if PTTYPE == PTTYPE_EPT
> +static void FNAME(link_shadow_page)(u64 *sptep, struct kvm_mmu_page *sp)
> +{
> +	u64 spte;
> +
> +	spte = __pa(sp->spt) | VMX_EPT_READABLE_MASK | VMX_EPT_WRITABLE_MASK |
> +		VMX_EPT_EXECUTABLE_MASK;
> +
> +	mmu_spte_set(sptep, spte);
> +}
> +#endif

Should be setting the permission bits to map the L2 guest EPT pagetable entry?

Re: [PATCH 02/11] nEPT: Add EPT tables support to paging_tmpl.h

[Nakajima, Jun]

Thanks for the comments.

This patch was mostly just mechanical rebase of the original patch,
and I'm going to clean it up.

On Thu, May 2, 2013 at 4:54 PM, Marcelo Tosatti <mtosatti@redhat.com> wrote:
> On Thu, Apr 25, 2013 at 11:43:22PM -0700, Jun Nakajima wrote:
>> This is the first patch in a series which adds nested EPT support to KVM's
>> nested VMX. Nested EPT means emulating EPT for an L1 guest so that L1 can use
>> EPT when running a nested guest L2. When L1 uses EPT, it allows the L2 guest
>> to set its own cr3 and take its own page faults without either of L0 or L1
>> getting involved. This often significanlty improves L2's performance over the
>> previous two alternatives (shadow page tables over EPT, and shadow page
>> tables over shadow page tables).
>>
>> This patch adds EPT support to paging_tmpl.h.
>>
>> paging_tmpl.h contains the code for reading and writing page tables. The code
>> for 32-bit and 64-bit tables is very similar, but not identical, so
>> paging_tmpl.h is #include'd twice in mmu.c, once with PTTTYPE=32 and once
>> with PTTYPE=64, and this generates the two sets of similar functions.
>>
>> There are subtle but important differences between the format of EPT tables
>> and that of ordinary x86 64-bit page tables, so for nested EPT we need a
>> third set of functions to read the guest EPT table and to write the shadow
>> EPT table.
>>
>> So this patch adds third PTTYPE, PTTYPE_EPT, which creates functions (prefixed
>> with "EPT") which correctly read and write EPT tables.
>>
>> Signed-off-by: Nadav Har'El <nyh@il.ibm.com>
>> Signed-off-by: Jun Nakajima <jun.nakajima@intel.com>
>> Signed-off-by: Xinhao Xu <xinhao.xu@intel.com>
>> ---
>>  arch/x86/kvm/mmu.c         |  35 ++----------
>>  arch/x86/kvm/paging_tmpl.h | 133 ++++++++++++++++++++++++++++++++++++++++++---
>>  2 files changed, 130 insertions(+), 38 deletions(-)
>>
>> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
>> index 956ca35..cb9c6fd 100644
>> --- a/arch/x86/kvm/mmu.c
>> +++ b/arch/x86/kvm/mmu.c
>> @@ -2480,26 +2480,6 @@ static pfn_t pte_prefetch_gfn_to_pfn(struct kvm_vcpu *vcpu, gfn_t gfn,
>>       return gfn_to_pfn_memslot_atomic(slot, gfn);
>>  }
>>
>> -static bool prefetch_invalid_gpte(struct kvm_vcpu *vcpu,
>> -                               struct kvm_mmu_page *sp, u64 *spte,
>> -                               u64 gpte)
>> -{
>> -     if (is_rsvd_bits_set(&vcpu->arch.mmu, gpte, PT_PAGE_TABLE_LEVEL))
>> -             goto no_present;
>> -
>> -     if (!is_present_gpte(gpte))
>> -             goto no_present;
>> -
>> -     if (!(gpte & PT_ACCESSED_MASK))
>> -             goto no_present;
>> -
>> -     return false;
>> -
>> -no_present:
>> -     drop_spte(vcpu->kvm, spte);
>> -     return true;
>> -}
>> -
>>  static int direct_pte_prefetch_many(struct kvm_vcpu *vcpu,
>>                                   struct kvm_mmu_page *sp,
>>                                   u64 *start, u64 *end)
>> @@ -3399,16 +3379,6 @@ static bool sync_mmio_spte(u64 *sptep, gfn_t gfn, unsigned access,
>>       return false;
>>  }
>>
>> -static inline unsigned gpte_access(struct kvm_vcpu *vcpu, u64 gpte)
>> -{
>> -     unsigned access;
>> -
>> -     access = (gpte & (PT_WRITABLE_MASK | PT_USER_MASK)) | ACC_EXEC_MASK;
>> -     access &= ~(gpte >> PT64_NX_SHIFT);
>> -
>> -     return access;
>> -}
>> -
>>  static inline bool is_last_gpte(struct kvm_mmu *mmu, unsigned level, unsigned gpte)
>>  {
>>       unsigned index;
>> @@ -3418,6 +3388,11 @@ static inline bool is_last_gpte(struct kvm_mmu *mmu, unsigned level, unsigned gp
>>       return mmu->last_pte_bitmap & (1 << index);
>>  }
>>
>> +#define PTTYPE_EPT 18 /* arbitrary */
>> +#define PTTYPE PTTYPE_EPT
>> +#include "paging_tmpl.h"
>> +#undef PTTYPE
>> +
>
> This breaks
>
> #if PTTYPE == 64
>         if (walker->level == PT32E_ROOT_LEVEL) {
>                 pte = mmu->get_pdptr(vcpu, (addr >> 30) & 3);
>                 trace_kvm_mmu_paging_element(pte, walker->level);
>
> At walk_addr_generic.

This code path is not required for EPT page walk, as far as I
understand. Or am I missing something?

>
>>  #define PTTYPE 64
>>  #include "paging_tmpl.h"
>>  #undef PTTYPE
>> diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
>> index 105dd5b..e13b6c5 100644
>> --- a/arch/x86/kvm/paging_tmpl.h
>> +++ b/arch/x86/kvm/paging_tmpl.h
>> @@ -50,6 +50,22 @@
>>       #define PT_LEVEL_BITS PT32_LEVEL_BITS
>>       #define PT_MAX_FULL_LEVELS 2
>>       #define CMPXCHG cmpxchg
>> +#elif PTTYPE == PTTYPE_EPT
>> +     #define pt_element_t u64
>> +     #define guest_walker guest_walkerEPT
>> +     #define FNAME(name) EPT_##name
>> +     #define PT_BASE_ADDR_MASK PT64_BASE_ADDR_MASK
>> +     #define PT_LVL_ADDR_MASK(lvl) PT64_LVL_ADDR_MASK(lvl)
>> +     #define PT_LVL_OFFSET_MASK(lvl) PT64_LVL_OFFSET_MASK(lvl)
>> +     #define PT_INDEX(addr, level) PT64_INDEX(addr, level)
>> +     #define PT_LEVEL_BITS PT64_LEVEL_BITS
>> +     #ifdef CONFIG_X86_64
>> +     #define PT_MAX_FULL_LEVELS 4
>> +     #define CMPXCHG cmpxchg
>> +     #else
>> +     #define CMPXCHG cmpxchg64
>> +     #define PT_MAX_FULL_LEVELS 2
>> +     #endif
>>  #else
>>       #error Invalid PTTYPE value
>>  #endif
>> @@ -80,6 +96,7 @@ static gfn_t gpte_to_gfn_lvl(pt_element_t gpte, int lvl)
>>       return (gpte & PT_LVL_ADDR_MASK(lvl)) >> PAGE_SHIFT;
>>  }
>>
>> +#if PTTYPE != PTTYPE_EPT
>
> What is the reasoning for this? Please restrict #ifdef changes to code
> where there is a difference in pagetable format (that is, code that
> knows pagetable format).

Since we don't implement EPT A/D support at this point,
update_accessed_dirty_bit() is commented out. And thus this routine is
unused for EPT.

>
>>  static int FNAME(cmpxchg_gpte)(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu,
>>                              pt_element_t __user *ptep_user, unsigned index,
>>                              pt_element_t orig_pte, pt_element_t new_pte)
>> @@ -102,7 +119,52 @@ static int FNAME(cmpxchg_gpte)(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu,
>>
>>       return (ret != orig_pte);
>>  }
>> +#endif
>> +
>> +static unsigned FNAME(gpte_access)(struct kvm_vcpu *vcpu, u64 gpte)
>> +{
>> +     unsigned access;
>> +
>> +#if PTTYPE == PTTYPE_EPT
>> +     /* We rely here that ACC_WRITE_MASK==VMX_EPT_WRITABLE_MASK */
>> +     access = (gpte & VMX_EPT_WRITABLE_MASK) | ACC_USER_MASK |
>> +             ((gpte & VMX_EPT_EXECUTABLE_MASK) ? ACC_EXEC_MASK : 0);
>
> User bit is meaningless for when emulating EPT. Why return it?
>

I'll clean it up.


>> +#else
>> +     access = (gpte & (PT_WRITABLE_MASK | PT_USER_MASK)) | ACC_EXEC_MASK;
>> +     access &= ~(gpte >> PT64_NX_SHIFT);
>> +#endif
>> +
>> +     return access;
>> +}
>> +
>> +static inline int FNAME(is_present_gpte)(unsigned long pte)
>> +{
>> +#if PTTYPE == PTTYPE_EPT
>> +     return pte & (VMX_EPT_READABLE_MASK | VMX_EPT_WRITABLE_MASK |
>> +                     VMX_EPT_EXECUTABLE_MASK);
>
> This should be VMX_EPT_READABLE_MASK (which is what maps to the
> normal pagetable present bit).

That mapping/simplification could be moot, but  VMX_EPT_READABLE_MASK
should be okay because we don't support 100b (execute-only) in EPT
paging-structure entries.

>
>> +#else
>> +     return is_present_gpte(pte);
>> +#endif
>> +}
>> +
>> +static inline int FNAME(check_write_user_access)(struct kvm_vcpu *vcpu,
>> +                                        bool write_fault, bool user_fault,
>> +                                        unsigned long pte)
>> +{
>
> Apparently unused.

Will remove it.

>
>> +#if PTTYPE == PTTYPE_EPT
>> +     if (unlikely(write_fault && !(pte & VMX_EPT_WRITABLE_MASK)
>> +                              && (user_fault || is_write_protection(vcpu))))
>> +             return false;
>> +     return true;
>> +#else
>> +     u32 access = ((kvm_x86_ops->get_cpl(vcpu) == 3) ? PFERR_USER_MASK : 0)
>> +                | (write_fault ? PFERR_WRITE_MASK : 0);
>>
>> +     return !permission_fault(vcpu->arch.walk_mmu, vcpu->arch.access, access);
>> +#endif
>> +}
>> +
>> +#if PTTYPE != PTTYPE_EPT
>
> Should emulate EPT dirty bit support?

No, at this point because the processors in the market don't support
the feature yet. We want to look at EPT large page support, for
example, before that.

>
>>  static int FNAME(update_accessed_dirty_bits)(struct kvm_vcpu *vcpu,
>>                                            struct kvm_mmu *mmu,
>>                                            struct guest_walker *walker,
>> @@ -139,6 +201,7 @@ static int FNAME(update_accessed_dirty_bits)(struct kvm_vcpu *vcpu,
>>       }
>>       return 0;
>>  }
>> +#endif
>>
>>  /*
>>   * Fetch a guest pte for a guest virtual address
>> @@ -147,7 +210,6 @@ static int FNAME(walk_addr_generic)(struct guest_walker *walker,
>>                                   struct kvm_vcpu *vcpu, struct kvm_mmu *mmu,
>>                                   gva_t addr, u32 access)
>>  {
>> -     int ret;
>>       pt_element_t pte;
>>       pt_element_t __user *uninitialized_var(ptep_user);
>>       gfn_t table_gfn;
>> @@ -162,7 +224,9 @@ static int FNAME(walk_addr_generic)(struct guest_walker *walker,
>>       gfn_t gfn;
>>
>>       trace_kvm_mmu_pagetable_walk(addr, access);
>> +#if PTTYPE != PTTYPE_EPT
>>  retry_walk:
>> +#endif
>>       walker->level = mmu->root_level;
>>       pte           = mmu->get_cr3(vcpu);
>>
>> @@ -215,17 +279,20 @@ retry_walk:
>>
>>               trace_kvm_mmu_paging_element(pte, walker->level);
>>
>> -             if (unlikely(!is_present_gpte(pte)))
>> +             if (unlikely(!FNAME(is_present_gpte)(pte)))
>>                       goto error;
>>
>>               if (unlikely(is_rsvd_bits_set(&vcpu->arch.mmu, pte,
>>                                             walker->level))) {
>> -                     errcode |= PFERR_RSVD_MASK | PFERR_PRESENT_MASK;
>> +                     errcode |= PFERR_PRESENT_MASK;
>> +#if PTTYPE != PTTYPE_EPT
>> +                     errcode |= PFERR_RSVD_MASK;
>> +#endif
>>                       goto error;
>>               }
>>
>>               accessed_dirty &= pte;
>> -             pte_access = pt_access & gpte_access(vcpu, pte);
>> +             pte_access = pt_access & FNAME(gpte_access)(vcpu, pte);
>>
>>               walker->ptes[walker->level - 1] = pte;
>>       } while (!is_last_gpte(mmu, walker->level, pte));
>> @@ -247,6 +314,7 @@ retry_walk:
>>
>>       walker->gfn = real_gpa >> PAGE_SHIFT;
>>
>> +#if PTTYPE != PTTYPE_EPT
>>       if (!write_fault)
>>               protect_clean_gpte(&pte_access, pte);
>>       else
>> @@ -257,12 +325,15 @@ retry_walk:
>>               accessed_dirty &= pte >> (PT_DIRTY_SHIFT - PT_ACCESSED_SHIFT);
>>
>>       if (unlikely(!accessed_dirty)) {
>> +             int ret;
>> +
>>               ret = FNAME(update_accessed_dirty_bits)(vcpu, mmu, walker, write_fault);
>>               if (unlikely(ret < 0))
>>                       goto error;
>>               else if (ret)
>>                       goto retry_walk;
>>       }
>> +#endif
>>
>>       walker->pt_access = pt_access;
>>       walker->pte_access = pte_access;
>> @@ -293,6 +364,7 @@ static int FNAME(walk_addr)(struct guest_walker *walker,
>>                                       access);
>>  }
>>
>> +#if PTTYPE != PTTYPE_EPT
>>  static int FNAME(walk_addr_nested)(struct guest_walker *walker,
>>                                  struct kvm_vcpu *vcpu, gva_t addr,
>>                                  u32 access)
>> @@ -300,6 +372,29 @@ static int FNAME(walk_addr_nested)(struct guest_walker *walker,
>>       return FNAME(walk_addr_generic)(walker, vcpu, &vcpu->arch.nested_mmu,
>>                                       addr, access);
>>  }
>> +#endif
>> +
>> +static bool FNAME(prefetch_invalid_gpte)(struct kvm_vcpu *vcpu,
>> +                               struct kvm_mmu_page *sp, u64 *spte,
>> +                               u64 gpte)
>> +{
>> +     if (is_rsvd_bits_set(&vcpu->arch.mmu, gpte, PT_PAGE_TABLE_LEVEL))
>> +             goto no_present;
>> +
>> +     if (!is_present_gpte(gpte))
>> +             goto no_present;
>> +
>> +#if PTTYPE != PTTYPE_EPT
>> +     if (!(gpte & PT_ACCESSED_MASK))
>> +             goto no_present;
>> +#endif
>> +
>> +     return false;
>> +
>> +no_present:
>> +     drop_spte(vcpu->kvm, spte);
>> +     return true;
>> +}
>
> Please split code move into a separate patch.

Will do.

>
>>  static bool
>>  FNAME(prefetch_gpte)(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp,
>> @@ -309,13 +404,13 @@ FNAME(prefetch_gpte)(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp,
>>       gfn_t gfn;
>>       pfn_t pfn;
>>
>> -     if (prefetch_invalid_gpte(vcpu, sp, spte, gpte))
>> +     if (FNAME(prefetch_invalid_gpte)(vcpu, sp, spte, gpte))
>>               return false;
>>
>>       pgprintk("%s: gpte %llx spte %p\n", __func__, (u64)gpte, spte);
>>
>>       gfn = gpte_to_gfn(gpte);
>> -     pte_access = sp->role.access & gpte_access(vcpu, gpte);
>> +     pte_access = sp->role.access & FNAME(gpte_access)(vcpu, gpte);
>>       protect_clean_gpte(&pte_access, gpte);
>>       pfn = pte_prefetch_gfn_to_pfn(vcpu, gfn,
>>                       no_dirty_log && (pte_access & ACC_WRITE_MASK));
>> @@ -394,6 +489,18 @@ static void FNAME(pte_prefetch)(struct kvm_vcpu *vcpu, struct guest_walker *gw,
>>       }
>>  }
>>
>> +#if PTTYPE == PTTYPE_EPT
>> +static void FNAME(link_shadow_page)(u64 *sptep, struct kvm_mmu_page *sp)
>> +{
>> +     u64 spte;
>> +
>> +     spte = __pa(sp->spt) | VMX_EPT_READABLE_MASK | VMX_EPT_WRITABLE_MASK |
>> +             VMX_EPT_EXECUTABLE_MASK;
>> +
>> +     mmu_spte_set(sptep, spte);
>> +}
>> +#endif
>
> Should be setting the permission bits to map the L2 guest EPT pagetable entry?
>

It's doing the equivalent things that are done in the shadow page
table entries. VMX_EPT_EXECUTABLE_MASK is added because
VMX_EPT_READABLE_MASK in the guest will imply executable as well (no
execute-only support). It would be helpful if somebody explains what
link_shadow_page() is supposed to do exactly in the shadow page code.
For example, it always adds PT_WRITABLE_MASK; looks like it checks if
it was a write-fault before calling it, though.

--
Jun
Intel Open Source Technology Center