nEPT: Nested EPT support for Nested VMX

Add a module option "nested_ept" determining whether to enable Nested EPT.

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).

nested_ept is currently enabled by default (when nested VMX is enabled), unless L0 doesn't have EPT or disabled it with ept=0.

Users would not normally want to explicitly disable this option. One reason why one might want to disable it is to force L1 to make due without the EPT capability, when anticipating a future need to migrate this L1 to another host which doesn't have EPT. Note that currently there is no API to turn off nested EPT for just a single L1 guest. However, obviously, an individual L1 guest may choose not to use EPT - the nested_cpu_has_ept() checks if L1 actually used EPT when running L2.

In the future, we can support emulation of EPT for L1 *always*, even when L0 itself doesn't have EPT. This so-called "EPT on shadow page tables" mode has some theoretical advantages over the baseline "shadow page tables on shadow page tables" mode typically used when EPT is not available to L0 - namely that L2's cr3 changes and page faults can be handled in L0 and do not need to be propagated to L1. However, currently we do not support this mode, and it is becoming less interesting as newer processors all support EPT.

Quoted 1 line

Add a module option "nested_ept" determining whether to enable Nested EPT.

...

Quoted 9 lines

In the future, we can support emulation of EPT for L1 *always*, even when L0 itself doesn't have EPT. This so-called "EPT on shadow page tables" mode has some theoretical advantages over the baseline "shadow page tables on shadow page tables" mode typically used when EPT is not available to L0 - namely that L2's cr3 changes and page faults can be handled in L0 and do not need to be propagated to L1. However, currently we do not support this mode, and it is becoming less interesting as newer processors all support EPT.

I think we can live without this.

But we do need a way to control what
features are exposed to the guest, for compatibility and live migration purposes, as we do with cpuid. So we need some way for host userspace to write to the vmx read-only feature reporting MSRs.

On Thu, Nov 10, 2011, Avi Kivity wrote about "Re: [PATCH 01/10] nEPT: Module option":

Quoted 13 lines · Nadav Har'El

Quoted 1 line

Add a module option "nested_ept" determining whether to enable Nested EPT.

...

Quoted 9 lines

In the future, we can support emulation of EPT for L1 *always*, even when L0 itself doesn't have EPT. This so-called "EPT on shadow page tables" mode has some theoretical advantages over the baseline "shadow page tables on shadow page tables" mode typically used when EPT is not available to L0 - namely that L2's cr3 changes and page faults can be handled in L0 and do not need to be propagated to L1. However, currently we do not support this mode, and it is becoming less interesting as newer processors all support EPT.

Quoted 1 line

I think we can live without this.

By "this", do you mean without the "nested_ept" option, or without the hypothetical "EPT on shadow page tables" feature?

If the former, then I agree we can "live" without it, but since it was trivial to add, I don't see what harm it can do, and its nice that we can return with a single L0 option to the old shadow-on-ept paging. Is there anything specific you don't like about having this option?

About the latter, I agree - as I said, there isn't much point to go and write this (quite complicated) 3-level shadowing when all new processors have EPT anyway. So I didn't.

Quoted 4 lines

But we do need a way to control what
features are exposed to the guest, for compatibility and live migration purposes, as we do with cpuid. So we need some way for host userspace to write to the vmx read-only feature reporting MSRs.

I think this is a general issue (which we already discussed earlier), of nested VMX and not specific to nested EPT. I already put all the capabilities which the MSR report in variables initialized in a single function, nested_vmx_setup_ctls_msrs(), so once we devise an appropriate userspace interface to set these, we can do so easily.

Does nested SVM also have a similar problem, of whether or not it advertises new or optional SVM features to L1? If it does have this problem, how was it solved there?

Quoted 2 lines

By "this", do you mean without the "nested_ept" option, or without the hypothetical "EPT on shadow page tables" feature?

Er, both. The feature should be controlled on a per-guest basis, not per host.
..
It's just redundant, since we do need a per-guest control.

On Thu, Nov 10, 2011, Avi Kivity wrote about "Re: [PATCH 01/10] nEPT: Module option":

Quoted 7 lines

Quoted 2 lines

By "this", do you mean without the "nested_ept" option, or without the hypothetical "EPT on shadow page tables" feature?

Er, both. The feature should be controlled on a per-guest basis, not per host.
..
It's just redundant, since we do need a per-guest control.

I agreed that per-guest control would have been nicer, but since we don't have an API for specifying that per guest since EPT is not, unfortunately, a CPUID feature, I thought that at least a host-level flag would be useful.

Why would it be useful? I agree it isn't the most important option since sliced bread, but if, for example, one day we discover a bug with nested EPT, L0 can disable it for all L1 guests and basically force them to use shadow page tables on EPT.

It was also useful for me to have this option for benchmarking, because I can force back the old shadow-on-EPT method with just a single option in L0 (instead of needing to give "ept=0" option in L1s).

If you really don't like the existance of this option, I can easily remove it of course.

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_*()). We then need to switch back and forth between this nested context and the regular MMU context when switching between L1 and L2.

arch/x86/kvm/vmx.c +1 -0

+

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_*()). We then need to switch back and forth between this nested context and the regular MMU context when switching between L1 and L2.

arch/x86/kvm/vmx.c +5 -0

+static int nested_ept_init_mmu_context(struct kvm_vcpu *vcpu)
+{
+	int r = kvm_init_shadow_mmu(vcpu, &vcpu->arch.mmu);
+
+	vcpu->arch.mmu.set_cr3           = vmx_set_cr3;

Quoted 3 lines

arch/x86/kvm/vmx.c +3 -0

+static int nested_ept_init_mmu_context(struct kvm_vcpu *vcpu)
+{
+	int r = kvm_init_shadow_mmu(vcpu, &vcpu->arch.mmu);

...

arch/x86/kvm/vmx.c static void vmx_set_supported_cpuid(u32 · line 6489

Quoted 1 line

arch/x86/kvm/vmx.c +1 -0

+	vcpu->arch.walk_mmu              = &vcpu->arch.nested_mmu;

...

kvm_init_shadow_mmu() will cause ->page_fault to be set to something like paging64_page_fault(), which is geared to reading EPT ptes. How does this work?

On Thu, Nov 10, 2011, Avi Kivity wrote about "Re: [PATCH 02/10] nEPT: MMU context for nested EPT":

Quoted 10 lines

Quoted 3 lines

arch/x86/kvm/vmx.c +3 -0

+static int nested_ept_init_mmu_context(struct kvm_vcpu *vcpu)
+{
+	int r = kvm_init_shadow_mmu(vcpu, &vcpu->arch.mmu);

...

arch/x86/kvm/vmx.c static void vmx_set_supported_cpuid(u32 · line 6489

Quoted 1 line

arch/x86/kvm/vmx.c +1 -0

+	vcpu->arch.walk_mmu              = &vcpu->arch.nested_mmu;

...

kvm_init_shadow_mmu() will cause ->page_fault to be set to something like paging64_page_fault(), which is geared to reading EPT ptes. How does this work?

Hi,

I'm afraid I didn't understand the problem.

Nested EPT's merging of two EPT tables (EPT01 and EPT12) works just like normal shadow page tables' merging of two CR3s (host cr3 and guest cr3):

When L0 receives a "page fault" from L2 (actually an EPT violation - real guest #PF don't cause exits), L0 first looks it up in the shadowed table, which is basically EPT12. If the address is there, L0 handles the fault itself (updating the shadow EPT table, EPT02 using the normal shadow pte building code). But if the address wasn't in the shadowed page table (EPT12), mmu->inject_page_fault() is called, which in our case actually causes L1 to get an EPT-violation (not #PF - see kvm_propagate_fault()).

Please note that all this logic is shared with the existing nested NPT code (which itself shared most of the code with the preexisting shadow page tables code). All this code sharing makes it really difficult to understand at first glance why the code is really working, but once you understood why one of these cases works, the others work similarly. And it does in fact work - in typical cases which I tried, at least.

If you still think I'm missing something, I won't be entirely surprised ( :-) ), so let me know.

This is all correct, but the code in question parses the EPT12 table using the ia32 page table format. They're sufficiently similar so that it works, but it isn't correct.

Bit 0: EPT readable, ia32 present
Bit 1: Writable; ia32 meaning dependent on cr0.wp
Bit 2: EPT executable, ia32 user (so, this implementation will interpret a non-executable EPT mapping, if someone could find a use for it, as a L2 kernel only mapping)
....

walk_addr() will also write to bits 6/7, which the L1 won't expect.

On Thu, Nov 10, 2011, Avi Kivity wrote about "Re: [PATCH 02/10] nEPT: MMU context for nested EPT":

Quoted 10 lines

This is all correct, but the code in question parses the EPT12 table using the ia32 page table format. They're sufficiently similar so that it works, but it isn't correct.

Bit 0: EPT readable, ia32 present
Bit 1: Writable; ia32 meaning dependent on cr0.wp
Bit 2: EPT executable, ia32 user (so, this implementation will interpret a non-executable EPT mapping, if someone could find a use for it, as a L2 kernel only mapping)
....

This is a very good point.

I was under the mistaken (?) impression that the page-table shadowing code will just copy these bits as-is from the shadowed table (EPT12) to the shadow table (EPT02), without caring what they actually mean.

I knew we had
a problem when building, not copying, PTEs, and hence the patch to link_shadow_page).

Also I realized we sometimes need to actually walk the TDP EPT12+cr3 (e.g., to see if an EPT violation is L1's fault), but I thought this was just the normal TDP walk, which already knows how to correctly read the EPT table.

Quoted 1 line

walk_addr() will also write to bits 6/7, which the L1 won't expect.

I didn't notice this :(

Back to the drawing board, I guess. I need to figure out exactly what needs to be fixed, and how to do this with the least obtrusive changes to the existing use case (normal shadow page tables, and nested EPT).

host may write-protect a page. Second, the shadow and guest ptes may be in different formats (ept vs ia32).

In fact that happens to accidentally work, no? Intermediate ptes are always present/write/user, which translates to read/write/execute in EPT.

Don't optimize for least changes, optimize for best result afterwards.

We need a third variant of walk_addr_generic that parses EPT format PTEs. Whether that's best done by writing paging_ept.h or modifying paging_tmpl.h, I don't know.

On Sat, Nov 12, 2011, Avi Kivity wrote about "Re: [PATCH 02/10] nEPT: MMU context for nested EPT":

Quoted 2 lines

host may write-protect a page. Second, the shadow and guest ptes may be in different formats (ept vs ia32).

I'm afraid I've lost you here... The shadow table and the to-be-shadowed table are both ia32 (this is the normal shadow table code), or both ept (the nested tdp code). When are they supposed to be in different formats (ept vs ia32)?

I'm also puzzled in what situation will the host will write-protect an EPT02 (shadow EPT) page?

Quoted 2 lines

In fact that happens to accidentally work, no? Intermediate ptes are always present/write/user, which translates to read/write/execute in EPT.

It didn't work because it also used to set the "accessed" bit, bit 5, which on EPT is reserved and caused EPT misconfiguration. So I had to fix link_shadow_page, or nested EPT would not work at all.

Quoted 1 line

Don't optimize for least changes, optimize for best result afterwards.

As I'm sure you remember, two years ago, in September 6 2009, you wrote in your blog about the newly contributed nested VMX patch set, and in particular its nested EPT (which predated the nested NPT contribution).

Nested EPT was, for some workloads, a huge performance improvement, but you (if I understand correctly) did not want that code in KVM because it, basically, optimized for getting the job done, in the most correct and most efficient manner - but without regard of how cleanly this fit with other types of shadowing (normal shadow page tables, and nested NPT), or how much of the code was being duplicated or circumvented.

So this time around, I couldn't really "not optimize for least changes". This time, the nested EPT had to fit (like a square peg in a round hole ;-)), into the preexisting MMU and NPT shadowing. I couldn't really just write the most correct and most efficient code (which Orit Wasserman already did, two years earlier).

This time I needed to figure out the least obtrusive
way of changing the existing code. The hardest thing about doing this was trying to understand all the complexities and subtleties of the existing MMU code in KVM, which already does 101 different cases in one overloaded piece of code, which is not commented or documented. And of course, add to that all the complexities (some might even say "cruft") which the underlying x86 architecture itself has acrued over the years. So it's not surprising I've missed some of the important subtleties which didn't have any effect in the typical case I've tried. Like I said, in my tests nested EPT *did* work. And even getting to that point was hard enough :-)

Quoted 3 lines

We need a third variant of walk_addr_generic that parses EPT format PTEs. Whether that's best done by writing paging_ept.h or modifying paging_tmpl.h, I don't know.

Thanks. I'll think about everything you've said in this thread (I'm still not convinced I understood all your points, so just understanding them will be the first step). I'll see what I can do to improve the patch.

But I have to be honest - I'm not sure how quickly I can finish this. I really appreciate all your comments about nested VMX in the last two years - most of them have been spot-on, 100% correct, and really helpful for making me understand things which I had previously misunderstood. However, since you are (of course) extremely familiar with every nook and cranny of KVM, what normally happens is that every comment which took you 5 minutes to figure out, takes me 5 days to fully understand, and to actually write, debug and test the fixed code. Every review that takes you two days to go through (and is very much appreciated!) takes me several months to fix each and every thing you asked for.

Don't get me wrong, I *am* planning to continue working (part-time) on nested VMX, and nested EPT in particular. But if you want it to pick up the pace, I could use some help with actual coding from people who have much more intimate knowledge of the non-nested-VMX parts of KVM than I have.

In the meantime, if anybody wants to experiment with a much faster Nested VMX than we had before, you can try my current patch. It may not be perfect, but in many ways it is better than the old shadow-on-ept code. And in simple (64 bit, 4k page) kvm-over-kvm configurations like I tried, it works well.

On Sat, Nov 12, 2011, Avi Kivity wrote about "Re: [PATCH 02/10] nEPT: MMU context for nested EPT":

Quoted 2 lines

host may write-protect a page. Second, the shadow and guest ptes may be in different formats (ept vs ia32).

I'm afraid I've lost you here... The shadow table and the to-be-shadowed table are both ia32 (this is the normal shadow table code), or both ept (the nested tdp code). When are they supposed to be in different formats (ept vs ia32)?

I'm also puzzled in what situation will the host will write-protect an EPT02 (shadow EPT) page?

Quoted 2 lines

In fact that happens to accidentally work, no? Intermediate ptes are always present/write/user, which translates to read/write/execute in EPT.

It didn't work because it also used to set the "accessed" bit, bit 5, which on EPT is reserved and caused EPT misconfiguration. So I had to fix link_shadow_page, or nested EPT would not work at all.

Quoted 1 line

Don't optimize for least changes, optimize for best result afterwards.

As I'm sure you remember, two years ago, in September 6 2009, you wrote in your blog about the newly contributed nested VMX patch set, and in particular its nested EPT (which predated the nested NPT contribution).

Nested EPT was, for some workloads, a huge performance improvement, but you (if I understand correctly) did not want that code in KVM because it, basically, optimized for getting the job done, in the most correct and most efficient manner - but without regard of how cleanly this fit with other types of shadowing (normal shadow page tables, and nested NPT), or how much of the code was being duplicated or circumvented.

So this time around, I couldn't really "not optimize for least changes". This time, the nested EPT had to fit (like a square peg in a round hole ;-)), into the preexisting MMU and NPT shadowing. I couldn't really just write the most correct and most efficient code (which Orit Wasserman already did, two years earlier). This time I needed to figure out the least obtrusive way of changing the existing code. The hardest thing about doing this was trying to understand all the complexities and subtleties of the existing MMU code in KVM, which already does 101 different cases in one overloaded piece of code, which is not commented or documented. And of course, add to that all the complexities (some might even say "cruft") which the underlying x86 architecture itself has acrued over the years. So it's not surprising I've missed some of the important subtleties which didn't have any effect in the typical case I've tried. Like I said, in my tests nested EPT *did* work. And even getting to that point was hard enough :-)

Quoted 3 lines

We need a third variant of walk_addr_generic that parses EPT format PTEs. Whether that's best done by writing paging_ept.h or modifying paging_tmpl.h, I don't know.

Thanks. I'll think about everything you've said in this thread (I'm still not convinced I understood all your points, so just understanding them will be the first step). I'll see what I can do to improve the patch.

But I have to be honest - I'm not sure how quickly I can finish this. I really appreciate all your comments about nested VMX in the last two years - most of them have been spot-on, 100% correct, and really helpful for making me understand things which I had previously misunderstood. However, since you are (of course) extremely familiar with every nook and cranny of KVM, what normally happens is that every comment which took you 5 minutes to figure out, takes me 5 days to fully understand, and to actually write, debug and test the fixed code. Every review that takes you two days to go through (and is very much appreciated!) takes me several months to fix each and every thing you asked for.

Don't get me wrong, I *am* planning to continue working (part-time) on nested VMX, and nested EPT in particular. But if you want it to pick up the pace, I could use some help with actual coding from people who have much more intimate knowledge of the non-nested-VMX parts of KVM than I have.

In the meantime, if anybody wants to experiment with a much faster Nested VMX than we had before, you can try my current patch. It may not be perfect, but in many ways it is better than the old shadow-on-ept code. And in simple (64 bit, 4k page) kvm-over-kvm configurations like I tried, it works well.

Nadav.

From 032ba0221ea690f61ecc4f6d946090d18d6f4dfb Mon Sep 17 00:00:00 2001

Maybe this patch can help, this is roughly what Avi wants (I hope) done very quickly. I'm sorry I don't have setup to run nested VMX at the moment so i can't test it.

Orit

arch/x86/kvm/mmu.c +0 -0

index 9335e1b..bbe212f 100644

arch/x86/kvm/mmu.c +0 -0

 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);

arch/x86/kvm/paging_tmpl.h +5 -0

index 507e2b8..70d4cfd 100644
@@ -39,6 +39,21 @@
 	#define CMPXCHG cmpxchg64
 	#define PT_MAX_FULL_LEVELS 2
 	#endif
+#elif PTTYPE == EPT
+	#define pt_element_t u64
+	#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)

arch/x86/kvm/paging_tmpl.h +0 -0

 #elif PTTYPE == 32
 	#define pt_element_t u32
 	#define guest_walker guest_walker32

arch/x86/kvm/paging_tmpl.h +1 -0

 	const int write_fault = access & PFERR_WRITE_MASK;
 	const int user_fault  = access & PFERR_USER_MASK;
 	const int fetch_fault = access & PFERR_FETCH_MASK;
+#endif
 	u16 errcode = 0;
 
 	trace_kvm_mmu_pagetable_walk(addr, write_fault, user_fault,

arch/x86/kvm/paging_tmpl.h +0 -0

 	for (;;) {
 		gfn_t real_gfn;

@@ -186,9 +215,14 @@ retry_walk:
 		pte_gpa   = gfn_to_gpa(table_gfn) + offset;
 		walker->table_gfn[walker->level - 1] = table_gfn;

arch/x86/kvm/paging_tmpl.h +1 -0

 		if (unlikely(real_gfn == UNMAPPED_GVA))
 			goto error;
 		real_gfn = gpa_to_gfn(real_gfn);

@@ -221,6 +255,7 @@ retry_walk:
 			eperm = true;
 #endif
 
+#if PTTYPE != EPT
 		if (!eperm && unlikely(!(pte & PT_ACCESSED_MASK))) {
 			int ret;
 			trace_kvm_mmu_set_accessed_bit(table_gfn, index,

Quoted 10 lines · Orit Wasserman

Maybe this patch can help, this is roughly what Avi wants (I hope) done very quickly. I'm sorry I don't have setup to run nested VMX at the moment so i can't test it.

Orit

arch/x86/kvm/mmu.c +0 -0

index 9335e1b..bbe212f 100644

arch/x86/kvm/mmu.c static bool sync_mmio_spte(u64 *sptep, gfn_t gfn, unsigned access, · line 3186

Quoted 7 lines

arch/x86/kvm/mmu.c +4 -0

@@ -3180,6 +3180,10 @@ static bool sync_mmio_spte(u64 *sptep, gfn_t gfn, unsigned access,
 #include "paging_tmpl.h"
 #undef PTTYPE
 
+#define PTTYPE EPT
+#include "paging_tmpl.h"
+#undef PTTYPE
+

Yes, that's the key.

Quoted 20 lines

arch/x86/kvm/mmu.c +0 -0

 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);

arch/x86/kvm/paging_tmpl.h +9 -0

index 507e2b8..70d4cfd 100644
@@ -39,6 +39,21 @@
 	#define CMPXCHG cmpxchg64
 	#define PT_MAX_FULL_LEVELS 2
 	#endif
+#elif PTTYPE == EPT
+	#define pt_element_t u64
+	#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

arch/x86/kvm/paging_tmpl.h line 56

Quoted 6 lines

arch/x86/kvm/paging_tmpl.h +6 -0

+	#define PT_MAX_FULL_LEVELS 4
+	#define CMPXCHG cmpxchg
+	#else
+	#define CMPXCHG cmpxchg64
+	#define PT_MAX_FULL_LEVELS 2
+	#endif

The various masks should be defined here, to avoid lots of #ifdefs later.

Quoted 4 lines

arch/x86/kvm/paging_tmpl.h +0 -0

 #elif PTTYPE == 32
 	#define pt_element_t u32
 	#define guest_walker guest_walker32

arch/x86/kvm/paging_tmpl.h static unsigned FNAME(gpte_access)(struct kvm_vcpu *vcpu, pt_element_t gpte, · line 130

Quoted 10 lines

arch/x86/kvm/paging_tmpl.h +4 -0

@@ -106,14 +121,19 @@ static unsigned FNAME(gpte_access)(struct kvm_vcpu *vcpu, pt_element_t gpte,
 {
 	unsigned access;
 
+#if PTTYPE == EPT       
 	access = (gpte & (PT_WRITABLE_MASK | PT_USER_MASK)) | ACC_EXEC_MASK;
+#else
+	access = (gpte & EPT_WRITABLE_MASK) | EPT_EXEC_MASK;
 	if (last && !is_dirty_gpte(gpte))
 		access &= ~ACC_WRITE_MASK;
+#endif

Like here, you could make is_dirty_gpte() local to paging_tmpl() returning true for EPT and the dirty bit otherwise.

arch/x86/kvm/paging_tmpl.h static unsigned FNAME(gpte_access)(struct kvm_vcpu *vcpu, pt_element_t gpte, · line 134

Quoted 4 lines

arch/x86/kvm/paging_tmpl.h +0 -0

 #if PTTYPE == 64
 	if (vcpu->arch.mmu.nx)
 		access &= ~(gpte >> PT64_NX_SHIFT);

The ept X bit is lost.

Could do something like

   access &= (gpte >> PT_X_NX_SHIFT) ^ PT_X_NX_SENSE;

arch/x86/kvm/paging_tmpl.h static int FNAME(walk_addr_generic)(struct guest_walker *walker, · line 176

Quoted 5 lines

arch/x86/kvm/paging_tmpl.h +5 -0

+#if PTTYPE == EPT
+	const int write_fault = access & EPT_WRITABLE_MASK;
+	const int user_fault  = 0;
+	const int fetch_fault = 0;
+#else

EPT has fetch permissions (but not user permissions); anyway translate_nested_gpa() already does this.

Quoted 8 lines

arch/x86/kvm/paging_tmpl.h +1 -0

 	const int write_fault = access & PFERR_WRITE_MASK;
 	const int user_fault  = access & PFERR_USER_MASK;
 	const int fetch_fault = access & PFERR_FETCH_MASK;
+#endif
 	u16 errcode = 0;
 
 	trace_kvm_mmu_pagetable_walk(addr, write_fault, user_fault,

arch/x86/kvm/paging_tmpl.h retry_walk: · line 205

Quoted 6 lines

arch/x86/kvm/paging_tmpl.h +3 -0

@@ -174,6 +200,9 @@ retry_walk:
 	       (mmu->get_cr3(vcpu) & CR3_NONPAE_RESERVED_BITS) == 0);
 
 	pt_access = ACC_ALL;
+#if PTTYPE == EPT 
+	pt_access =  PT_PRESENT_MASK | EPT_WRITABLE_MASK | EPT_EXEC_MASK;
+#endif

pt_access is not in EPT or ia32 format - it's our own format (xwu). So this doesn't need changing. Updating gpte_access() is sufficient.

Quoted 10 lines

arch/x86/kvm/paging_tmpl.h +2 -1

 	for (;;) {
 		gfn_t real_gfn;

@@ -186,9 +215,14 @@ retry_walk:
 		pte_gpa   = gfn_to_gpa(table_gfn) + offset;
 		walker->table_gfn[walker->level - 1] = table_gfn;
 		walker->pte_gpa[walker->level - 1] = pte_gpa;
-
+#if PTTYPE == EPT 
+		real_gfn = mmu->translate_gpa(vcpu, gfn_to_gpa(table_gfn),

arch/x86/kvm/paging_tmpl.h retry_walk: · line 225

Quoted 6 lines

arch/x86/kvm/paging_tmpl.h +4 -0

+					      EPT_WRITABLE_MASK);
+#else
 		real_gfn = mmu->translate_gpa(vcpu, gfn_to_gpa(table_gfn),
 					      PFERR_USER_MASK|PFERR_WRITE_MASK);
+#endif
+

Unneeded, I think.

Quoted 8 lines

arch/x86/kvm/paging_tmpl.h +0 -0

 		if (unlikely(real_gfn == UNMAPPED_GVA))
 			goto error;
 		real_gfn = gpa_to_gfn(real_gfn);

@@ -221,6 +255,7 @@ retry_walk:
 			eperm = true;
 #endif
 

arch/x86/kvm/paging_tmpl.h retry_walk: · line 273

Quoted 10 lines

arch/x86/kvm/paging_tmpl.h +2 -1

+#if PTTYPE != EPT
 		if (!eperm && unlikely(!(pte & PT_ACCESSED_MASK))) {
 			int ret;
 			trace_kvm_mmu_set_accessed_bit(table_gfn, index,

@@ -235,7 +270,7 @@ retry_walk:
 			mark_page_dirty(vcpu->kvm, table_gfn);
 			pte |= PT_ACCESSED_MASK;
 		}
-
+#endif

If PT_ACCESSED_MASK is 0 for EPT, this goes away without #ifdef.

arch/x86/kvm/paging_tmpl.h retry_walk: · line 288

Quoted 8 lines

arch/x86/kvm/paging_tmpl.h +2 -1

+#if PTTYPE != EPT
 			/* check if the kernel is fetching from user page */
 			if (unlikely(pte_access & PT_USER_MASK) &&
 			    kvm_read_cr4_bits(vcpu, X86_CR4_SMEP))
 				if (fetch_fault && !user_fault)
 					eperm = true;
-
+#endif

Same here.

Quoted 21 lines

Quoted 21 lines

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);

arch/x86/kvm/paging_tmpl.h +9 -0

index 507e2b8..70d4cfd 100644
@@ -39,6 +39,21 @@
 	#define CMPXCHG cmpxchg64
 	#define PT_MAX_FULL_LEVELS 2
 	#endif
+#elif PTTYPE == EPT
+	#define pt_element_t u64
+	#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

Quoted 9 lines

arch/x86/kvm/paging_tmpl.h line 56

Quoted 6 lines

arch/x86/kvm/paging_tmpl.h +6 -0

+	#define PT_MAX_FULL_LEVELS 4
+	#define CMPXCHG cmpxchg
+	#else
+	#define CMPXCHG cmpxchg64
+	#define PT_MAX_FULL_LEVELS 2
+	#endif

The various masks should be defined here, to avoid lots of #ifdefs later.

That what I did first but than I was afraid that the MASK will be changed for mmu.c too. so I decided on ifdefs.
The more I think about it I think we need rapper function for mask checking (at least for this file). What do you think ?

Quoted 9 lines

Quoted 9 lines

arch/x86/kvm/paging_tmpl.h +2 -1

 	for (;;) {
 		gfn_t real_gfn;

@@ -186,9 +215,14 @@ retry_walk:
 		pte_gpa   = gfn_to_gpa(table_gfn) + offset;
 		walker->table_gfn[walker->level - 1] = table_gfn;
 		walker->pte_gpa[walker->level - 1] = pte_gpa;
-
+#if PTTYPE == EPT 
+		real_gfn = mmu->translate_gpa(vcpu, gfn_to_gpa(table_gfn),

Quoted 8 lines

arch/x86/kvm/paging_tmpl.h retry_walk: · line 225

Quoted 6 lines

arch/x86/kvm/paging_tmpl.h +4 -0

+					      EPT_WRITABLE_MASK);
+#else
 		real_gfn = mmu->translate_gpa(vcpu, gfn_to_gpa(table_gfn),
 					      PFERR_USER_MASK|PFERR_WRITE_MASK);
+#endif
+

Unneeded, I think.

Is it because translate_nested_gpa always set USER_MASK ?

Quoted 11 lines

Quoted 10 lines

arch/x86/kvm/paging_tmpl.h retry_walk: · line 225

Quoted 8 lines

arch/x86/kvm/paging_tmpl.h +6 -0

+#if PTTYPE == EPT 
+		real_gfn = mmu->translate_gpa(vcpu, gfn_to_gpa(table_gfn),
+					      EPT_WRITABLE_MASK);
+#else
 		real_gfn = mmu->translate_gpa(vcpu, gfn_to_gpa(table_gfn),
 					      PFERR_USER_MASK|PFERR_WRITE_MASK);
+#endif
+

Unneeded, I think.

Quoted 1 line

Is it because translate_nested_gpa always set USER_MASK ?

Yes... maybe that function needs to do something like

   access |= mmu->default_access;

Quoted 2 lines

Maybe this patch can help, this is roughly what Avi wants (I hope) done very quickly. I'm sorry I don't have setup to run nested VMX at the moment so i can't test it.

...

arch/x86/kvm/mmu.c static bool sync_mmio_spte(u64 *sptep, gfn_t gfn, unsigned access, · line 3185

Quoted 3 lines

arch/x86/kvm/mmu.c +3 -0

+#define PTTYPE EPT
+#include "paging_tmpl.h"
+#undef PTTYPE

Yes, that's the key.

On Sun, Nov 13, 2011, Avi Kivity wrote about "Re: [PATCH 02/10] nEPT: MMU context for nested EPT":

Quoted 9 lines · Orit Wasserman

Quoted 2 lines

Maybe this patch can help, this is roughly what Avi wants (I hope) done very quickly. I'm sorry I don't have setup to run nested VMX at the moment so i can't test it.

...

arch/x86/kvm/mmu.c static bool sync_mmio_spte(u64 *sptep, gfn_t gfn, unsigned access, · line 3185

Quoted 3 lines

arch/x86/kvm/mmu.c +3 -0

+#define PTTYPE EPT
+#include "paging_tmpl.h"
+#undef PTTYPE

Yes, that's the key.

I'm now preparing a patch based on such ideas.

One downside of this approach is that mmu.c (and therefore the x86 module) will now include EPT-specific functions that are of no use or relevance to the SVM code. It's not a terrible disaster, but it's "unclean". I'll try to think if there's a cleaner way.

Maybe this patch can help, this is roughly what Avi wants (I hope) done very quickly. I'm sorry I don't have setup to run nested VMX at the moment so i can't test it.

Quoted 6 lines

arch/x86/kvm/mmu.c +6 -0

+static int nested_ept_init_mmu_context(struct kvm_vcpu *vcpu)
+{
+	int r = kvm_init_shadow_mmu(vcpu, &vcpu->arch.mmu);
+
+	vcpu->arch.nested_mmu.gva_to_gpa = EPT_gva_to_gpa_nested;
+

arch/x86/kvm/mmu.c static int init_kvm_softmmu(struct kvm_vcpu *vcpu) · line 3395

Quoted 2 lines

arch/x86/kvm/mmu.c +2 -0

+	return r;
+}

..
I didn't see you actually call this function anywhere - how is it supposed to work?
..
It seems we need a fifth case in that function.
..

On Wed, Nov 23, 2011, Nadav Har'El wrote about "Re: [PATCH 02/10] nEPT: MMU context for nested EPT":

Quoted 6 lines

Quoted 6 lines

arch/x86/kvm/mmu.c +6 -0

+static int nested_ept_init_mmu_context(struct kvm_vcpu *vcpu)
+{
+	int r = kvm_init_shadow_mmu(vcpu, &vcpu->arch.mmu);
+
+	vcpu->arch.nested_mmu.gva_to_gpa = EPT_gva_to_gpa_nested;
+

Quoted 8 lines

arch/x86/kvm/mmu.c static int init_kvm_softmmu(struct kvm_vcpu *vcpu) · line 3395

Quoted 2 lines

arch/x86/kvm/mmu.c +2 -0

+	return r;
+}

..
I didn't see you actually call this function anywhere - how is it supposed to work?
..
It seems we need a fifth case in that function.
..

On second thought, why is this modifying nested_mmu.gva_to_gpa, and not mmu.gva_to_gpa? Isn't the nested_mmu the L2 CR3, which is *not* in EPT format, and what we really want to change is the outer mmu, which is EPT12 and is indeed in EPT format?
Or am I missing something?

On Sun, Nov 13, 2011, Orit Wasserman wrote about "Re: [PATCH 02/10] nEPT: MMU context for nested EPT":

Quoted 5 lines

+++ b/arch/x86/kvm/mmu.h

arch/x86/kvm/mmu.h +0 -0

@@ -48,6 +48,11 @@
 #define PFERR_RSVD_MASK (1U << 3)
 #define PFERR_FETCH_MASK (1U << 4)
 

arch/x86/kvm/mmu.h line 52

Quoted 2 lines

arch/x86/kvm/mmu.h +2 -0

+#define EPT_WRITABLE_MASK 2
+#define EPT_EXEC_MASK 4

This is another example of the "unclean" movement of VMX-specific things into x86 :( We already have VMX_EPT_WRITABLE_MASK and friends in vmx.h. I'll need to think what is less ugly: to move them to mmu.h, or to include vmx.h in mmu.c, or perhaps even create a new include file, ept.h. Avi, do you have a preference?

The last thing I want to do is to repeat the same definitions in two places.

When the existing KVM MMU code creates a shadow page table, it assumes it has the normal x86 page table format. This is obviously correct for normal shadow page tables, and also correct for AMD's NPT. Unfortunately, Intel's EPT page tables differ in subtle ways from ordinary page tables, so when we create a shadow EPT table (i.e., in nested EPT), we need to slightly modify the way in which this table table is built.

In particular, when mmu.c's link_shadow_page() creates non-leaf page table entries, it used to enable the "present", "accessed", "writable" and "user" flags on these entries. While this is correct for ordinary page tables, it is wrong in EPT tables - where these bits actually have completely different meaning (compare PT_*_MASK from mmu.h to VMX_EPT_*_MASK from vmx.h). In particular, leaving the code as-is causes bit 5 of the PTE to be turned on (supposedly for PT_ACCESSED_MASK), which is a reserved bit in EPT and causes an "EPT Misconfiguration" failure.

So we must move link_shadow_page's list of extra bits to a new mmu context field, which is set differently for nested EPT.

Signed-off-by Nadav Har'El <[email protected]>

---
 arch/x86/include/asm/kvm_host.h |    1 +
 arch/x86/kvm/mmu.c              |   16 +++++++++-------
 arch/x86/kvm/paging_tmpl.h      |    6 ++++--
 arch/x86/kvm/vmx.c              |    3 +++
 4 files changed, 17 insertions(+), 9 deletions(-)

arch/x86/include/asm/kvm_host.h +1 -0

@@ -287,6 +287,7 @@ struct kvm_mmu {
 	bool nx;
 
 	u64 pdptrs[4]; /* pae */
+	u64 link_shadow_page_set_bits;
 };
 
 struct kvm_vcpu_arch {

arch/x86/kvm/vmx.c +3 -0

@@ -6485,6 +6485,9 @@ static int nested_ept_init_mmu_context(s
 	vcpu->arch.mmu.get_pdptr         = nested_ept_get_pdptr;
 	vcpu->arch.mmu.inject_page_fault = nested_ept_inject_page_fault;
 	vcpu->arch.mmu.shadow_root_level = get_ept_level();
+	vcpu->arch.mmu.link_shadow_page_set_bits =
+		VMX_EPT_READABLE_MASK | VMX_EPT_WRITABLE_MASK |
+		VMX_EPT_EXECUTABLE_MASK;
 
 	vcpu->arch.walk_mmu              = &vcpu->arch.nested_mmu;
 

arch/x86/kvm/mmu.c +1 -1

@@ -1782,14 +1782,9 @@ static void shadow_walk_next(struct kvm_
 	return __shadow_walk_next(iterator, *iterator->sptep);
 }
 
-static void link_shadow_page(u64 *sptep, struct kvm_mmu_page *sp)
+static void link_shadow_page(u64 *sptep, struct kvm_mmu_page *sp, u64 set_bits)

Quoted 5 lines

arch/x86/include/asm/kvm_host.h +1 -0

@@ -287,6 +287,7 @@ struct kvm_mmu {
 	bool nx;
 
 	u64 pdptrs[4]; /* pae */
+	u64 link_shadow_page_set_bits;

...

arch/x86/kvm/mmu.c static void shadow_walk_next(struct kvm_ · line 1788

Quoted 11 lines

arch/x86/kvm/mmu.c +2 -6

+static void link_shadow_page(u64 *sptep, struct kvm_mmu_page *sp, u64 set_bits)
 {
-	u64 spte;
-
-	spte = __pa(sp->spt)
-		| PT_PRESENT_MASK | PT_ACCESSED_MASK
-		| PT_WRITABLE_MASK | PT_USER_MASK;
-	mmu_spte_set(sptep, spte);
+	mmu_spte_set(sptep, __pa(sp->spt) | set_bits);
 }

Minor nit: you can just use link_shadow_page_set_bits here instead of passing it around (unless later you have a different value for the parameter?)

On Thu, Nov 10, 2011, Avi Kivity wrote about "Re: [PATCH 04/10] nEPT: Fix page table format in nested EPT":

Quoted 6 lines

Quoted 5 lines

arch/x86/include/asm/kvm_host.h +1 -0

@@ -287,6 +287,7 @@ struct kvm_mmu {
 	bool nx;
 
 	u64 pdptrs[4]; /* pae */
+	u64 link_shadow_page_set_bits;

...

Quoted 15 lines

arch/x86/kvm/mmu.c static void shadow_walk_next(struct kvm_ · line 1788

Quoted 11 lines

arch/x86/kvm/mmu.c +2 -6

+static void link_shadow_page(u64 *sptep, struct kvm_mmu_page *sp, u64 set_bits)
 {
-	u64 spte;
-
-	spte = __pa(sp->spt)
-		| PT_PRESENT_MASK | PT_ACCESSED_MASK
-		| PT_WRITABLE_MASK | PT_USER_MASK;
-	mmu_spte_set(sptep, spte);
+	mmu_spte_set(sptep, __pa(sp->spt) | set_bits);
 }

Minor nit: you can just use link_shadow_page_set_bits here instead of passing it around (unless later you have a different value for the parameter?)

The problem was that link_shadow_page did not take an kvm_mmu parameter, so I don't know where to find this link_shadow_page_set_bits. So either I pass the pointer to the entire kvm_mmu to link_shadow_page, or I just pass the only field which I need... I thought that passing the single field I need was cleaner - but I can easily change it if you prefer to pass the kvm_mmu.

Quoted 23 lines · Nadav Har'El

On Thu, Nov 10, 2011, Avi Kivity wrote about "Re: [PATCH 04/10] nEPT: Fix page table format in nested EPT":

Quoted 21 lines

Quoted 5 lines

arch/x86/include/asm/kvm_host.h +1 -0

@@ -287,6 +287,7 @@ struct kvm_mmu {
 	bool nx;
 
 	u64 pdptrs[4]; /* pae */
+	u64 link_shadow_page_set_bits;

...

arch/x86/kvm/mmu.c static void shadow_walk_next(struct kvm_ · line 1788

Quoted 11 lines

arch/x86/kvm/mmu.c +2 -6

+static void link_shadow_page(u64 *sptep, struct kvm_mmu_page *sp, u64 set_bits)
 {
-	u64 spte;
-
-	spte = __pa(sp->spt)
-		| PT_PRESENT_MASK | PT_ACCESSED_MASK
-		| PT_WRITABLE_MASK | PT_USER_MASK;
-	mmu_spte_set(sptep, spte);
+	mmu_spte_set(sptep, __pa(sp->spt) | set_bits);
 }

Minor nit: you can just use link_shadow_page_set_bits here instead of passing it around (unless later you have a different value for the parameter?)

Quoted 6 lines

The problem was that link_shadow_page did not take an kvm_mmu parameter, so I don't know where to find this link_shadow_page_set_bits. So either I pass the pointer to the entire kvm_mmu to link_shadow_page, or I just pass the only field which I need... I thought that passing the single field I need was cleaner - but I can easily change it if you prefer to pass the kvm_mmu.

Ah, doesn't matter either way.

When the existing KVM MMU code creates a shadow page table, it assumes it has the normal x86 page table format. This is obviously correct for normal shadow page tables, and also correct for AMD's NPT. Unfortunately, Intel's EPT page tables differ in subtle ways from ordinary page tables, so when we create a shadow EPT table (i.e., in nested EPT), we need to slightly modify the way in which this table table is built.

In particular, when mmu.c's link_shadow_page() creates non-leaf page table entries, it used to enable the "present", "accessed", "writable" and "user" flags on these entries. While this is correct for ordinary page tables, it is wrong in EPT tables - where these bits actually have completely different meaning (compare PT_*_MASK from mmu.h to VMX_EPT_*_MASK from vmx.h). In particular, leaving the code as-is causes bit 5 of the PTE to be turned on (supposedly for PT_ACCESSED_MASK), which is a reserved bit in EPT and causes an "EPT Misconfiguration" failure.

So we must move link_shadow_page's list of extra bits to a new mmu context field, which is set differently for nested EPT.

Signed-off-by Nadav Har'El <[email protected]>

---
 arch/x86/include/asm/kvm_host.h |    1 +
 arch/x86/kvm/mmu.c              |   16 +++++++++-------
 arch/x86/kvm/paging_tmpl.h      |    6 ++++--
 arch/x86/kvm/vmx.c              |    3 +++
 4 files changed, 17 insertions(+), 9 deletions(-)

arch/x86/include/asm/kvm_host.h +1 -0

@@ -287,6 +287,7 @@ struct kvm_mmu {
 	bool nx;
 
 	u64 pdptrs[4]; /* pae */
+	u64 link_shadow_page_set_bits;
 };
 
 struct kvm_vcpu_arch {

arch/x86/kvm/vmx.c +3 -0

@@ -6485,6 +6485,9 @@ static int nested_ept_init_mmu_context(s
 	vcpu->arch.mmu.get_pdptr         = nested_ept_get_pdptr;
 	vcpu->arch.mmu.inject_page_fault = nested_ept_inject_page_fault;
 	vcpu->arch.mmu.shadow_root_level = get_ept_level();
+	vcpu->arch.mmu.link_shadow_page_set_bits =
+		VMX_EPT_READABLE_MASK | VMX_EPT_WRITABLE_MASK |
+		VMX_EPT_EXECUTABLE_MASK;
 
 	vcpu->arch.walk_mmu              = &vcpu->arch.nested_mmu;
 

arch/x86/kvm/mmu.c +9 -7

@@ -1782,14 +1782,9 @@ static void shadow_walk_next(struct kvm_
 	return __shadow_walk_next(iterator, *iterator->sptep);
 }
 
-static void link_shadow_page(u64 *sptep, struct kvm_mmu_page *sp)
+static void link_shadow_page(u64 *sptep, struct kvm_mmu_page *sp, u64 set_bits)
 {
-	u64 spte;
-
-	spte = __pa(sp->spt)
-		| PT_PRESENT_MASK | PT_ACCESSED_MASK
-		| PT_WRITABLE_MASK | PT_USER_MASK;
-	mmu_spte_set(sptep, spte);
+	mmu_spte_set(sptep, __pa(sp->spt) | set_bits);
 }
 
 static void drop_large_spte(struct kvm_vcpu *vcpu, u64 *sptep)

@@ -3366,6 +3361,13 @@ int kvm_init_shadow_mmu(struct kvm_vcpu 
 	vcpu->arch.mmu.base_role.cr0_wp  = is_write_protection(vcpu);
 	vcpu->arch.mmu.base_role.smep_andnot_wp
 		= smep && !is_write_protection(vcpu);
+	/*
+	 * link_shadow() should apply these bits in shadow page tables, and
+	 * in shadow NPT tables (nested NPT). For nested EPT, different bits
+	 * apply.
+	 */
+	vcpu->arch.mmu.link_shadow_page_set_bits = PT_PRESENT_MASK |
+		PT_ACCESSED_MASK | PT_WRITABLE_MASK | PT_USER_MASK;
 
 	return r;
 }

arch/x86/kvm/paging_tmpl.h +2 -1

@@ -515,7 +515,8 @@ static u64 *FNAME(fetch)(struct kvm_vcpu
 			goto out_gpte_changed;
 
 		if (sp)
-			link_shadow_page(it.sptep, sp);
+			link_shadow_page(it.sptep, sp,
+				vcpu->arch.mmu.link_shadow_page_set_bits);
 	}
 
 	for (;

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

arch/x86/kvm/vmx.c +10 -0

+	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 {
+			/*
+			 * We efficiently handle the common case, of L1
+			 * invalidating the last eptp it used to run L2.
+			 * TODO: Instead of saving one last_eptp02, look up
+			 * operand.eptp in the shadow EPT table cache, to

arch/x86/kvm/vmx.c +5 -0

+			nested_vmx_succeed(vcpu);
+		}
+		break;
+	case VMX_EPT_EXTENT_INDIVIDUAL_ADDR:
+		if (!(nested_vmx_ept_caps & VMX_EPT_EXTENT_INDIVIDUAL_BIT))

arch/x86/kvm/vmx.c +14 -0

+			else
+				ept_sync_global();
+			nested_vmx_succeed(vcpu);
+		}
+		break;
+	default:
+		nested_vmx_failValid(vcpu,
+			VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID);
+	}
+
+	skip_emulated_instruction(vcpu);
+	return 1;
+}
+

Quoted 1 line

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

..

arch/x86/kvm/vmx.c static int handle_vmptrst(struct kvm_vcp · line 5646

Quoted 6 lines

arch/x86/kvm/vmx.c +6 -0

+			if (vmcs12 && nested_cpu_has_ept(vmcs12) &&
+			    (vmcs12->ept_pointer == operand.eptp) &&
+			    vmx->nested.last_eptp02)
+				ept_sync_context(vmx->nested.last_eptp02);
+			else
+				ept_sync_global();

Are either of these needed? Won't a write to a shadowed EPT table cause them anyway?

On Thu, Nov 10, 2011, Avi Kivity wrote about "Re: [PATCH 08/10] nEPT: Nested INVEPT":

Quoted 12 lines · Nadav Har'El

Quoted 1 line

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

..

arch/x86/kvm/vmx.c static int handle_vmptrst(struct kvm_vcp · line 5646

Quoted 6 lines

arch/x86/kvm/vmx.c +6 -0

+			if (vmcs12 && nested_cpu_has_ept(vmcs12) &&
+			    (vmcs12->ept_pointer == operand.eptp) &&
+			    vmx->nested.last_eptp02)
+				ept_sync_context(vmx->nested.last_eptp02);
+			else
+				ept_sync_global();

Are either of these needed? Won't a write to a shadowed EPT table cause them anyway?

This is very good point... You're right that as it stands, any changes to the guest EPT table (EPT12) will cause changes to the shadow EPT table (EPT02), and these already cause KVM to do an INVEPT, so no point to do this again when the guest asks. So basically, I can have INVEPT emulated by doing absolutely nothing (after checking all the checks), right?

I wonder if I am missing any reason why a hypervisor might want to do INVEPT without changing the EPT12 table first.

The following patches add nested EPT support to Nested VMX.

Nested EPT means emulating EPT for an L1 guest, allowing it 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. In many workloads this significanlty improves L2's performance over the previous two alternatives (shadow page tables over ept, and shadow page tables over shadow page tables). Our paper [1] described these three options, and the advantages of nested EPT ("multidimensional paging").

Nested EPT is enabled by default (if the hardware supports EPT), so users do not have to do anything special to enjoy the performance improvement that this patch gives to L2 guests.

Just as a non-scientific, non-representative indication of the kind of dramatic performance improvement you may see in workloads that have a lot of context switches and page faults, here is a measurement of the time an example single-threaded "make" took in L2 (kvm over kvm):

shadow over shadow: 105 seconds
("ept=0" forces this)

shadow over EPT: 87 seconds
(the previous default; Can be forced now with "nested_ept=0")

EPT over EPT: 29 seconds
(the default after this patch)

Note that the same test on L1 (with EPT) took 25 seconds, so for this example workload, performance of nested virtualization is now very close to that of single-level virtualization.

This patchset is missing a fairly hairy patch that makes reading L2 virtual addresses work.

The standard example is L1 passing a bit of
hardware (emulated in L0) to a L2; when L2 accesses it, the instruction will fault and need to be handled in L0, transparently to L1. The emulation can cause a fault to be injected to L2, or and EPT violation or misconfiguration injected to L1.

Hi,

On Thu, Nov 10, 2011, Avi Kivity wrote about "Re: [PATCH 0/10] nEPT: Nested EPT support for Nested VMX":

Quoted 2 lines

This patchset is missing a fairly hairy patch that makes reading L2 virtual addresses work.

This was supposed to be part of the nested TDP code that is already in the code. To read an L2 virtual address, the code is supposed, if I understand correctly, to walk the "walk" mmu (EPT01 and guest_cr3) and then use the EPT table - just like the normal EPT case which uses the EPT table and the guest_cr3.

I even believed that this inner "walk mmu" will work fine without any rewrite needed for ia32/ept differences, because it works (or so I believed) just like normal EPT, with the first table being an EPT table, and the second table being a normal page table.

I also believed that the fault injection part was also correct: I thought that the code already knows when to handle the fault in L2 (when the address is missing in cr3), in L1 (when the translation is missing in EPT12) or else, in L0.

So what is the "hairy" missing part?

Quoted 6 lines

The standard example is L1 passing a bit of
hardware (emulated in L0) to a L2; when L2 accesses it, the instruction will fault and need to be handled in L0, transparently to L1. The emulation can cause a fault to be injected to L2, or and EPT violation or misconfiguration injected to L1.

I don't understand the example. You are refering to nested device assignment from L1 to L2 (so L1 stops caring about the device)? Since we don't emulate an IOMMU for L1, how can that be done?

Quoted 4 lines

I also believed that the fault injection part was also correct: I thought that the code already knows when to handle the fault in L2 (when the address is missing in cr3), in L1 (when the translation is missing in EPT12) or else, in L0.

It does, but it needs to propagate the fault code correctly. The exit reason (ept violation vs ept misconfiguration) is meaningless, since we don't encode anything about it from ept12 into ept02. In particular an ept violation could lead to

- no fault, ept02 updated, instruction retried
- no fault, instruction emulated
- L2 fault
- ept violation, need to compute ept12 permissions for exit qualification
- ept misconfiguration

(the second and third cases occur when it is impossible to create an ept02 mapping - when L0 emulates a gpa that L1 assigns to L2 via ept12).

On Sun, Nov 13, 2011, Avi Kivity wrote about "Re: [PATCH 0/10] nEPT: Nested EPT support for Nested VMX":

Quoted 10 lines

Quoted 4 lines

I also believed that the fault injection part was also correct: I thought that the code already knows when to handle the fault in L2 (when the address is missing in cr3), in L1 (when the translation is missing in EPT12) or else, in L0.

It does, but it needs to propagate the fault code correctly. The exit reason (ept violation vs ept misconfiguration) is meaningless, since we don't encode anything about it from ept12 into ept02. In particular an ept violation could lead to

Quoted 8 lines

- no fault, ept02 updated, instruction retried
- no fault, instruction emulated
- L2 fault
- ept violation, need to compute ept12 permissions for exit qualification
- ept misconfiguration

(the second and third cases occur when it is impossible to create an ept02 mapping - when L0 emulates a gpa that L1 assigns to L2 via ept12).

I'm now trying to figure out this part, and I think I am beginning to understand the mess you are referring to:

In nested_ept_inject_page_fault I now assume the exit reason is always EPT VIOLATION and have

	vmcs12->exit_qualification = fault->error_code;

But fault->error_code is not in the exit qualification format but in the PFERR_* format, which has different meanings for the bits... Moreover, PFERR_RSVD_MASK should cause an EPT MISCONFIG, not EPT VIOLATION. Is this what you meant above?

I didn't quite understand what you meant in the 4th case about needing to compute ept12 permissions. I'm assuming that if the EPT violation was caused because L0 decreased permissions from what L1 thought, then L0 will solve the problem itself and not inject it to L1. So if we are injecting the fault to L1, don't we already know the correct fault reason and don't need to compute it?

There's another complication: when the fault comes from an EPT violation in L2, handle_ept_violation() calls mmu.page_fault() with an error_code of exit_qualification & 0x3. This means that the error_code in this case is *not* in the expected PFERR_* format, and we need to know that in nested_ept_inject_page_fault. Moreover, in the original EPT visolation's exit qualification, there were various other bits which we lose (and don't have a direct parallel in PFERR_* anyway), so when we reinject the fault, L1 doesn't get them.

What a mess :(