[kvm-unit-tests PATCH 00/39] x86/access: nVMX: Big overhaul

[kvm-unit-tests PATCH 00/39] x86/access: nVMX: Big overhaul

[Sean Christopherson]

This started out as a very simple test (patch 39/39) to expose a KVM bug
where KVM doesn't sync a shadow MMU on a vmcs12->vpid change.  Except the
test didn't fail.  And it turns out, completely removing INVLPG from the
base access test doesn't fail when using shadow paging either.

The underlying problem in both cases is that the access test is flat out
stupid when it comes to handling page tables.  Instead of allocating page
tables once and manipulating them on each iteration, it "allocates" a new
paging structure when necessary on every. single. iteration.  In addition
to being incredibly inefficient (allocation also zeros the entire 4kb page,
so the test zeros absurd amounts of memory), writing upper level PTEs on
every iteration triggers write-protection mechanisms in KVM.  In effect,
KVM ends up synchronizing the relevant SPTEs on every iteration, which
again is ridiculously slow and makes it all but impossible to actually
test that KVM handles other TLB invalidation scenarios.

Trying to solve that mess by pre-allocating the page tables exposed a
whole pile of 5-level paging issues.  I'd say the test's 5-level support
is held together by duct tape, but I've fixed many things with duct tape
that are far less fragile.

The second half of this series is cleanups in the nVMX code to prepare
for adding the (INV)VPID variants.  Not directly related to the access
tests, but it annoyed me to no end that simply checking if INVVPID is
supported was non-trivial.

Sean Christopherson (39):
  x86/access: Add proper defines for hardcoded addresses
  x86/access: Cache CR3 to improve performance
  x86/access:  Use do-while loop for what is obviously a do-while loop
  x86/access: Stop pretending the test is SMP friendly
  x86/access: Refactor so called "page table pool" logic
  x86/access: Stash root page table level in test environment
  x86/access: Hoist page table allocator helpers above "init" helper
  x86/access: Rename variables in page table walkers
  x86/access: Abort if page table insertion hits an unexpected level
  x86/access: Make SMEP place nice with 5-level paging
  x86/access: Use upper half of virtual address space
  x86/access: Print the index when dumping PTEs
  x86/access: Pre-allocate all page tables at (sub)test init
  x86/access: Don't write page tables if desired PTE is same as current
    PTE
  x86/access: Preserve A/D bits when writing paging structure entries
  x86/access: Make toggling of PRESENT bit a "higher order" action
  x86/access: Manually override PMD in effective permissions sub-test
  x86/access: Remove manual override of PUD/PMD in prefetch sub-test
  x86/access: Remove PMD/PT target overrides
  x86/access: Remove timeout overrides now that performance doesn't suck
  nVMX: Skip EPT tests if INVEPT(SINGLE_CONTEXT) is unsupported
  nVMX: Hoist assert macros to the top of vmx.h
  nVMX: Add a non-reporting assertion macro
  nVMX: Assert success in unchecked INVEPT/INVVPID helpers
  nVMX: Drop less-than-useless ept_sync() wrapper
  nVMX: Move EPT capability check helpers to vmx.h
  nVMX: Drop unused and useless vpid_sync() helper
  nVMX: Remove "v1" version of INVVPID test
  nVMX: Add helper to check if INVVPID type is supported
  nVMX: Add helper to check if INVVPID is supported
  nVMX: Add helper to get first supported INVVPID type
  nVMX: Use helper to check for EPT A/D support
  nVMX: Add helpers to check for 4/5-level EPT support
  nVMX: Fix name of macro defining EPT execute only capability
  nVMX: Add helper to check if a memtype is supported for EPT structures
  nVMX: Get rid of horribly named "ctrl" boolean in test_ept_eptp()
  nVMX: Rename awful "ctrl" booleans to "is_ctrl_valid"
  nVMX: Add helper to check if VPID is supported
  x86/access: nVMX: Add "access" test variants to invalidate via
    (INV)VPID

 x86/access.c      | 391 ++++++++++++++++++++++++++++------------------
 x86/unittests.cfg |  10 +-
 x86/vmx.c         |  71 +--------
 x86/vmx.h         | 229 ++++++++++++++++++---------
 x86/vmx_tests.c   | 327 +++++++++++++++++---------------------
 5 files changed, 543 insertions(+), 485 deletions(-)

-- 
2.34.0.rc2.393.gf8c9666880-goog

[kvm-unit-tests PATCH 01/39] x86/access: Add proper defines for hardcoded addresses

[Sean Christopherson]

Use defines to differentiate between the paging structure and code/data
physical address ranges to reduce the magic ever so slightly.

No functional change intended.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/access.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/x86/access.c b/x86/access.c
index c5e71db..2e0636a 100644
--- a/x86/access.c
+++ b/x86/access.c
@@ -15,6 +15,10 @@ static _Bool verbose = false;
 typedef unsigned long pt_element_t;
 static int invalid_mask;
 
+/* Test code/data is at 32MiB, paging structures at 33MiB. */
+#define AT_CODE_DATA_PHYS	  32 * 1024 * 1024
+#define AT_PAGING_STRUCTURES_PHYS 33 * 1024 * 1024
+
 #define PT_BASE_ADDR_MASK ((pt_element_t)((((pt_element_t)1 << 36) - 1) & PAGE_MASK))
 #define PT_PSE_BASE_ADDR_MASK (PT_BASE_ADDR_MASK & ~(1ull << 21))
 
@@ -273,7 +277,7 @@ static void ac_env_int(ac_pool_t *pool)
 	set_idt_entry(14, &page_fault, 0);
 	set_idt_entry(0x20, &kernel_entry, 3);
 
-	pool->pt_pool = 33 * 1024 * 1024;
+	pool->pt_pool = AT_PAGING_STRUCTURES_PHYS;
 	pool->pt_pool_size = 120 * 1024 * 1024 - pool->pt_pool;
 	pool->pt_pool_current = 0;
 }
@@ -284,7 +288,7 @@ static void ac_test_init(ac_test_t *at, void *virt, int page_table_levels)
 	set_cr0_wp(1);
 	at->flags = 0;
 	at->virt = virt;
-	at->phys = 32 * 1024 * 1024;
+	at->phys = AT_CODE_DATA_PHYS;
 	at->page_table_levels = page_table_levels;
 }
 
-- 
2.34.0.rc2.393.gf8c9666880-goog

[kvm-unit-tests PATCH 02/39] x86/access: Cache CR3 to improve performance

[Sean Christopherson]

Add a shadow for CR3, which avoids a significant number of VM-Exits when
KVM is using shadow paging.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/access.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/x86/access.c b/x86/access.c
index 2e0636a..eb0ba60 100644
--- a/x86/access.c
+++ b/x86/access.c
@@ -189,6 +189,7 @@ typedef struct {
 static void ac_test_show(ac_test_t *at);
 
 static unsigned long shadow_cr0;
+static unsigned long shadow_cr3;
 static unsigned long shadow_cr4;
 static unsigned long long shadow_efer;
 
@@ -515,7 +516,7 @@ static void ac_set_expected_status(ac_test_t *at)
 static void __ac_setup_specific_pages(ac_test_t *at, ac_pool_t *pool, bool reuse,
 				      u64 pd_page, u64 pt_page)
 {
-	unsigned long root = read_cr3();
+	unsigned long root = shadow_cr3;
 	int flags = at->flags;
 	bool skip = true;
 
@@ -635,7 +636,7 @@ static void ac_setup_specific_pages(ac_test_t *at, ac_pool_t *pool,
 
 static void dump_mapping(ac_test_t *at)
 {
-	unsigned long root = read_cr3();
+	unsigned long root = shadow_cr3;
 	int flags = at->flags;
 	int i;
 
@@ -1084,6 +1085,7 @@ int ac_test_run(int page_table_levels)
 
 	shadow_cr0 = read_cr0();
 	shadow_cr4 = read_cr4();
+	shadow_cr3 = read_cr3();
 	shadow_efer = rdmsr(MSR_EFER);
 
 	if (cpuid_maxphyaddr() >= 52) {
-- 
2.34.0.rc2.393.gf8c9666880-goog

[kvm-unit-tests PATCH 03/39] x86/access: Use do-while loop for what is obviously a do-while loop

[Sean Christopherson]

Make unnecessarily confusing code less confusing.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/access.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/x86/access.c b/x86/access.c
index eb0ba60..60bb379 100644
--- a/x86/access.c
+++ b/x86/access.c
@@ -355,9 +355,10 @@ static int ac_test_bump(ac_test_t *at)
 {
 	int ret;
 
-	ret = ac_test_bump_one(at);
-	while (ret && !ac_test_legal(at))
+	do {
 		ret = ac_test_bump_one(at);
+	} while (ret && !ac_test_legal(at));
+
 	return ret;
 }
 
-- 
2.34.0.rc2.393.gf8c9666880-goog

[kvm-unit-tests PATCH 04/39] x86/access: Stop pretending the test is SMP friendly

[Sean Christopherson]

Remove SMP "support", the test is not remotely SMP friendly.  It can
barely survive one CPU modifying page tables, two would be pure carnage.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/access.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/x86/access.c b/x86/access.c
index 60bb379..749b50c 100644
--- a/x86/access.c
+++ b/x86/access.c
@@ -5,8 +5,6 @@
 #include "x86/vm.h"
 #include "access.h"
 
-#define smp_id() 0
-
 #define true 1
 #define false 0
 
@@ -1142,8 +1140,7 @@ int ac_test_run(int page_table_levels)
 	}
 
 	ac_env_int(&pool);
-	ac_test_init(&at, (void *)(0x123400000000 + 16 * smp_id()),
-		page_table_levels);
+	ac_test_init(&at, (void *)(0x123400000000), page_table_levels);
 	do {
 		++tests;
 		successes += ac_test_exec(&at, &pool);
-- 
2.34.0.rc2.393.gf8c9666880-goog

[kvm-unit-tests PATCH 05/39] x86/access: Refactor so called "page table pool" logic

[Sean Christopherson]

Track the index instead of raw physical address in the page table pool
to make the "enough room" check a bit less magical.

Opportunistically append "_pa" to pt_pool to clarify that it's the base
physical address of the pool.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/access.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/x86/access.c b/x86/access.c
index 749b50c..47b3d00 100644
--- a/x86/access.c
+++ b/x86/access.c
@@ -159,8 +159,7 @@ static inline void *va(pt_element_t phys)
 }
 
 typedef struct {
-	pt_element_t pt_pool;
-	unsigned pt_pool_size;
+	pt_element_t pt_pool_pa;
 	unsigned pt_pool_current;
 } ac_pool_t;
 
@@ -276,8 +275,7 @@ static void ac_env_int(ac_pool_t *pool)
 	set_idt_entry(14, &page_fault, 0);
 	set_idt_entry(0x20, &kernel_entry, 3);
 
-	pool->pt_pool = AT_PAGING_STRUCTURES_PHYS;
-	pool->pt_pool_size = 120 * 1024 * 1024 - pool->pt_pool;
+	pool->pt_pool_pa = AT_PAGING_STRUCTURES_PHYS;
 	pool->pt_pool_current = 0;
 }
 
@@ -362,15 +360,18 @@ static int ac_test_bump(ac_test_t *at)
 
 static pt_element_t ac_test_alloc_pt(ac_pool_t *pool)
 {
-	pt_element_t ret = pool->pt_pool + pool->pt_pool_current;
-	pool->pt_pool_current += PAGE_SIZE;
-	memset(va(ret), 0, PAGE_SIZE);
-	return ret;
+	pt_element_t pt;
+
+	pt = pool->pt_pool_pa + (pool->pt_pool_current * PAGE_SIZE);
+	pool->pt_pool_current++;
+	memset(va(pt), 0, PAGE_SIZE);
+	return pt;
 }
 
 static _Bool ac_test_enough_room(ac_pool_t *pool)
 {
-	return pool->pt_pool_current + 5 * PAGE_SIZE <= pool->pt_pool_size;
+	/* '120' is completely arbitrary. */
+	return (pool->pt_pool_current + 5) < 120;
 }
 
 static void ac_test_reset_pt_pool(ac_pool_t *pool)
-- 
2.34.0.rc2.393.gf8c9666880-goog

Re: [kvm-unit-tests PATCH 05/39] x86/access: Refactor so called "page table pool" logic

[Paolo Bonzini]

On 11/25/21 02:28, Sean Christopherson wrote:
>   static _Bool ac_test_enough_room(ac_pool_t *pool)
>   {
> -	return pool->pt_pool_current + 5 * PAGE_SIZE <= pool->pt_pool_size;
> +	/* '120' is completely arbitrary. */
> +	return (pool->pt_pool_current + 5) < 120;

Since the initialization was:

	pool->pt_pool_size = 120 * 1024 * 1024 - pool->pt_pool;

This should be 120 * 2^20 / 2^12 = 120 * 256 = 30720, which
is also arbitrary of course.

At this point I'm not sure there's a huge improvement, but I'll trust 
you and go on reviewing...

Paolo

>   }
>   
>   static void ac_test_reset_pt_pool(ac_pool_t *pool)

[kvm-unit-tests PATCH 06/39] x86/access: Stash root page table level in test environment

[Sean Christopherson]

Track the root page table level in the test environment, a future commit
will use it to guage whether or not the number of page tables being
allocated is reasonable.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/access.c | 132 ++++++++++++++++++++++++++-------------------------
 1 file changed, 68 insertions(+), 64 deletions(-)

diff --git a/x86/access.c b/x86/access.c
index 47b3d00..ba20359 100644
--- a/x86/access.c
+++ b/x86/access.c
@@ -160,8 +160,9 @@ static inline void *va(pt_element_t phys)
 
 typedef struct {
 	pt_element_t pt_pool_pa;
-	unsigned pt_pool_current;
-} ac_pool_t;
+	unsigned int pt_pool_current;
+	int pt_levels;
+} ac_pt_env_t;
 
 typedef struct {
 	unsigned flags;
@@ -174,7 +175,7 @@ typedef struct {
 	pt_element_t ignore_pde;
 	int expected_fault;
 	unsigned expected_error;
-	int page_table_levels;
+	int pt_levels;
 } ac_test_t;
 
 typedef struct {
@@ -269,24 +270,25 @@ static void set_efer_nx(int nx)
 	}
 }
 
-static void ac_env_int(ac_pool_t *pool)
+static void ac_env_int(ac_pt_env_t *pt_env, int page_table_levels)
 {
 	extern char page_fault, kernel_entry;
 	set_idt_entry(14, &page_fault, 0);
 	set_idt_entry(0x20, &kernel_entry, 3);
 
-	pool->pt_pool_pa = AT_PAGING_STRUCTURES_PHYS;
-	pool->pt_pool_current = 0;
+	pt_env->pt_pool_pa = AT_PAGING_STRUCTURES_PHYS;
+	pt_env->pt_pool_current = 0;
+	pt_env->pt_levels = page_table_levels;
 }
 
-static void ac_test_init(ac_test_t *at, void *virt, int page_table_levels)
+static void ac_test_init(ac_test_t *at, void *virt, ac_pt_env_t *pt_env)
 {
 	set_efer_nx(1);
 	set_cr0_wp(1);
 	at->flags = 0;
 	at->virt = virt;
 	at->phys = AT_CODE_DATA_PHYS;
-	at->page_table_levels = page_table_levels;
+	at->pt_levels = pt_env->pt_levels;
 }
 
 static int ac_test_bump_one(ac_test_t *at)
@@ -358,25 +360,25 @@ static int ac_test_bump(ac_test_t *at)
 	return ret;
 }
 
-static pt_element_t ac_test_alloc_pt(ac_pool_t *pool)
+static pt_element_t ac_test_alloc_pt(ac_pt_env_t *pt_env)
 {
 	pt_element_t pt;
 
-	pt = pool->pt_pool_pa + (pool->pt_pool_current * PAGE_SIZE);
-	pool->pt_pool_current++;
+	pt = pt_env->pt_pool_pa + (pt_env->pt_pool_current * PAGE_SIZE);
+	pt_env->pt_pool_current++;
 	memset(va(pt), 0, PAGE_SIZE);
 	return pt;
 }
 
-static _Bool ac_test_enough_room(ac_pool_t *pool)
+static _Bool ac_test_enough_room(ac_pt_env_t *pt_env)
 {
 	/* '120' is completely arbitrary. */
-	return (pool->pt_pool_current + 5) < 120;
+	return (pt_env->pt_pool_current + 5) < 120;
 }
 
-static void ac_test_reset_pt_pool(ac_pool_t *pool)
+static void ac_test_reset_pt_pool(ac_pt_env_t *pt_env)
 {
-	pool->pt_pool_current = 0;
+	pt_env->pt_pool_current = 0;
 }
 
 static pt_element_t ac_test_permissions(ac_test_t *at, unsigned flags,
@@ -513,18 +515,18 @@ static void ac_set_expected_status(ac_test_t *at)
 	ac_emulate_access(at, at->flags);
 }
 
-static void __ac_setup_specific_pages(ac_test_t *at, ac_pool_t *pool, bool reuse,
+static void __ac_setup_specific_pages(ac_test_t *at, ac_pt_env_t *pt_env, bool reuse,
 				      u64 pd_page, u64 pt_page)
 {
 	unsigned long root = shadow_cr3;
 	int flags = at->flags;
 	bool skip = true;
 
-	if (!ac_test_enough_room(pool))
-		ac_test_reset_pt_pool(pool);
+	if (!ac_test_enough_room(pt_env))
+		ac_test_reset_pt_pool(pt_env);
 
 	at->ptep = 0;
-	for (int i = at->page_table_levels; i >= 1 && (i >= 2 || !F(AC_PDE_PSE)); --i) {
+	for (int i = at->pt_levels; i >= 1 && (i >= 2 || !F(AC_PDE_PSE)); --i) {
 		pt_element_t *vroot = va(root & PT_BASE_ADDR_MASK);
 		unsigned index = PT_INDEX((unsigned long)at->virt, i);
 		pt_element_t pte = 0;
@@ -552,18 +554,18 @@ static void __ac_setup_specific_pages(ac_test_t *at, ac_pool_t *pool, bool reuse
 		switch (i) {
 		case 5:
 		case 4:
-			pte = ac_test_alloc_pt(pool);
+			pte = ac_test_alloc_pt(pt_env);
 			pte |= PT_PRESENT_MASK | PT_WRITABLE_MASK | PT_USER_MASK;
 			break;
 		case 3:
-			pte = pd_page ? pd_page : ac_test_alloc_pt(pool);
+			pte = pd_page ? pd_page : ac_test_alloc_pt(pt_env);
 			pte |= PT_PRESENT_MASK | PT_USER_MASK;
 			if (!F(AC_PDPTE_NO_WRITABLE))
 				pte |= PT_WRITABLE_MASK;
 			break;
 		case 2:
 			if (!F(AC_PDE_PSE)) {
-				pte = pt_page ? pt_page : ac_test_alloc_pt(pool);
+				pte = pt_page ? pt_page : ac_test_alloc_pt(pt_env);
 				/* The protection key is ignored on non-leaf entries.  */
 				if (F(AC_PKU_PKEY))
 					pte |= 2ull << 59;
@@ -623,15 +625,15 @@ static void __ac_setup_specific_pages(ac_test_t *at, ac_pool_t *pool, bool reuse
 	ac_set_expected_status(at);
 }
 
-static void ac_test_setup_pte(ac_test_t *at, ac_pool_t *pool)
+static void ac_test_setup_pte(ac_test_t *at, ac_pt_env_t *pt_env)
 {
-	__ac_setup_specific_pages(at, pool, false, 0, 0);
+	__ac_setup_specific_pages(at, pt_env, false, 0, 0);
 }
 
-static void ac_setup_specific_pages(ac_test_t *at, ac_pool_t *pool,
+static void ac_setup_specific_pages(ac_test_t *at, ac_pt_env_t *pt_env,
 				    u64 pd_page, u64 pt_page)
 {
-	return __ac_setup_specific_pages(at, pool, false, pd_page, pt_page);
+	return __ac_setup_specific_pages(at, pt_env, false, pd_page, pt_page);
 }
 
 static void dump_mapping(ac_test_t *at)
@@ -641,7 +643,7 @@ static void dump_mapping(ac_test_t *at)
 	int i;
 
 	printf("Dump mapping: address: %p\n", at->virt);
-	for (i = at->page_table_levels; i >= 1 && (i >= 2 || !F(AC_PDE_PSE)); --i) {
+	for (i = at->pt_levels; i >= 1 && (i >= 2 || !F(AC_PDE_PSE)); --i) {
 		pt_element_t *vroot = va(root & PT_BASE_ADDR_MASK);
 		unsigned index = PT_INDEX((unsigned long)at->virt, i);
 		pt_element_t pte = vroot[index];
@@ -818,20 +820,20 @@ static void ac_test_show(ac_test_t *at)
  * This test case is used to triger the bug which is fixed by
  * commit e09e90a5 in the kvm tree
  */
-static int corrupt_hugepage_triger(ac_pool_t *pool, int page_table_levels)
+static int corrupt_hugepage_triger(ac_pt_env_t *pt_env)
 {
 	ac_test_t at1, at2;
 
-	ac_test_init(&at1, (void *)(0x123400000000), page_table_levels);
-	ac_test_init(&at2, (void *)(0x666600000000), page_table_levels);
+	ac_test_init(&at1, (void *)(0x123400000000), pt_env);
+	ac_test_init(&at2, (void *)(0x666600000000), pt_env);
 
 	at2.flags = AC_CPU_CR0_WP_MASK | AC_PDE_PSE_MASK | AC_PDE_PRESENT_MASK;
-	ac_test_setup_pte(&at2, pool);
+	ac_test_setup_pte(&at2, pt_env);
 	if (!ac_test_do_access(&at2))
 		goto err;
 
 	at1.flags = at2.flags | AC_PDE_WRITABLE_MASK;
-	ac_test_setup_pte(&at1, pool);
+	ac_test_setup_pte(&at1, pt_env);
 	if (!ac_test_do_access(&at1))
 		goto err;
 
@@ -856,18 +858,18 @@ err:
  * This test case is used to triger the bug which is fixed by
  * commit 3ddf6c06e13e in the kvm tree
  */
-static int check_pfec_on_prefetch_pte(ac_pool_t *pool, int page_table_levels)
+static int check_pfec_on_prefetch_pte(ac_pt_env_t *pt_env)
 {
 	ac_test_t at1, at2;
 
-	ac_test_init(&at1, (void *)(0x123406001000), page_table_levels);
-	ac_test_init(&at2, (void *)(0x123406003000), page_table_levels);
+	ac_test_init(&at1, (void *)(0x123406001000), pt_env);
+	ac_test_init(&at2, (void *)(0x123406003000), pt_env);
 
 	at1.flags = AC_PDE_PRESENT_MASK | AC_PTE_PRESENT_MASK;
-	ac_setup_specific_pages(&at1, pool, 30 * 1024 * 1024, 30 * 1024 * 1024);
+	ac_setup_specific_pages(&at1, pt_env, 30 * 1024 * 1024, 30 * 1024 * 1024);
 
 	at2.flags = at1.flags | AC_PTE_NX_MASK;
-	ac_setup_specific_pages(&at2, pool, 30 * 1024 * 1024, 30 * 1024 * 1024);
+	ac_setup_specific_pages(&at2, pt_env, 30 * 1024 * 1024, 30 * 1024 * 1024);
 
 	if (!ac_test_do_access(&at1)) {
 		printf("%s: prepare fail\n", __FUNCTION__);
@@ -901,22 +903,22 @@ err:
  *
  * Note: to trigger this bug, hugepage should be disabled on host.
  */
-static int check_large_pte_dirty_for_nowp(ac_pool_t *pool, int page_table_levels)
+static int check_large_pte_dirty_for_nowp(ac_pt_env_t *pt_env)
 {
 	ac_test_t at1, at2;
 
-	ac_test_init(&at1, (void *)(0x123403000000), page_table_levels);
-	ac_test_init(&at2, (void *)(0x666606000000), page_table_levels);
+	ac_test_init(&at1, (void *)(0x123403000000), pt_env);
+	ac_test_init(&at2, (void *)(0x666606000000), pt_env);
 
 	at2.flags = AC_PDE_PRESENT_MASK | AC_PDE_PSE_MASK;
-	ac_test_setup_pte(&at2, pool);
+	ac_test_setup_pte(&at2, pt_env);
 	if (!ac_test_do_access(&at2)) {
 		printf("%s: read on the first mapping fail.\n", __FUNCTION__);
 		goto err;
 	}
 
 	at1.flags = at2.flags | AC_ACCESS_WRITE_MASK;
-	ac_test_setup_pte(&at1, pool);
+	ac_test_setup_pte(&at1, pt_env);
 	if (!ac_test_do_access(&at1)) {
 		printf("%s: write on the second mapping fail.\n", __FUNCTION__);
 		goto err;
@@ -935,7 +937,7 @@ err:
 	return 0;
 }
 
-static int check_smep_andnot_wp(ac_pool_t *pool, int page_table_levels)
+static int check_smep_andnot_wp(ac_pt_env_t *pt_env)
 {
 	ac_test_t at1;
 	int err_prepare_andnot_wp, err_smep_andnot_wp;
@@ -944,7 +946,7 @@ static int check_smep_andnot_wp(ac_pool_t *pool, int page_table_levels)
 		return 1;
 	}
 
-	ac_test_init(&at1, (void *)(0x123406001000), page_table_levels);
+	ac_test_init(&at1, (void *)(0x123406001000), pt_env);
 
 	at1.flags = AC_PDE_PRESENT_MASK | AC_PTE_PRESENT_MASK |
 		    AC_PDE_USER_MASK | AC_PTE_USER_MASK |
@@ -952,7 +954,7 @@ static int check_smep_andnot_wp(ac_pool_t *pool, int page_table_levels)
 		    AC_CPU_CR4_SMEP_MASK |
 		    AC_CPU_CR0_WP_MASK |
 		    AC_ACCESS_WRITE_MASK;
-	ac_test_setup_pte(&at1, pool);
+	ac_test_setup_pte(&at1, pt_env);
 
 	/*
 	 * Here we write the ro user page when
@@ -985,13 +987,13 @@ err:
 	return 0;
 }
 
-static int check_effective_sp_permissions(ac_pool_t *pool, int page_table_levels)
+static int check_effective_sp_permissions(ac_pt_env_t *pt_env)
 {
 	unsigned long ptr1 = 0x123480000000;
 	unsigned long ptr2 = ptr1 + SZ_2M;
 	unsigned long ptr3 = ptr1 + SZ_1G;
 	unsigned long ptr4 = ptr3 + SZ_2M;
-	pt_element_t pmd = ac_test_alloc_pt(pool);
+	pt_element_t pmd = ac_test_alloc_pt(pt_env);
 	ac_test_t at1, at2, at3, at4;
 	int err_read_at1, err_write_at2;
 	int err_read_at3, err_write_at4;
@@ -1006,24 +1008,24 @@ static int check_effective_sp_permissions(ac_pool_t *pool, int page_table_levels
 	 * pud1 and pud2 point to the same pmd page.
 	 */
 
-	ac_test_init(&at1, (void *)(ptr1), page_table_levels);
+	ac_test_init(&at1, (void *)(ptr1), pt_env);
 	at1.flags = AC_PDE_PRESENT_MASK | AC_PTE_PRESENT_MASK |
 		    AC_PDE_USER_MASK | AC_PTE_USER_MASK |
 		    AC_PDE_ACCESSED_MASK | AC_PTE_ACCESSED_MASK |
 		    AC_PTE_WRITABLE_MASK | AC_ACCESS_USER_MASK;
-	__ac_setup_specific_pages(&at1, pool, false, pmd, 0);
+	__ac_setup_specific_pages(&at1, pt_env, false, pmd, 0);
 
-	ac_test_init(&at2, (void *)(ptr2), page_table_levels);
+	ac_test_init(&at2, (void *)(ptr2), pt_env);
 	at2.flags = at1.flags | AC_PDE_WRITABLE_MASK | AC_PTE_DIRTY_MASK | AC_ACCESS_WRITE_MASK;
-	__ac_setup_specific_pages(&at2, pool, true, pmd, 0);
+	__ac_setup_specific_pages(&at2, pt_env, true, pmd, 0);
 
-	ac_test_init(&at3, (void *)(ptr3), page_table_levels);
+	ac_test_init(&at3, (void *)(ptr3), pt_env);
 	at3.flags = AC_PDPTE_NO_WRITABLE_MASK | at1.flags;
-	__ac_setup_specific_pages(&at3, pool, true, pmd, 0);
+	__ac_setup_specific_pages(&at3, pt_env, true, pmd, 0);
 
-	ac_test_init(&at4, (void *)(ptr4), page_table_levels);
+	ac_test_init(&at4, (void *)(ptr4), pt_env);
 	at4.flags = AC_PDPTE_NO_WRITABLE_MASK | at2.flags;
-	__ac_setup_specific_pages(&at4, pool, true, pmd, 0);
+	__ac_setup_specific_pages(&at4, pt_env, true, pmd, 0);
 
 	err_read_at1 = ac_test_do_access(&at1);
 	if (!err_read_at1) {
@@ -1052,19 +1054,19 @@ static int check_effective_sp_permissions(ac_pool_t *pool, int page_table_levels
 	return 1;
 }
 
-static int ac_test_exec(ac_test_t *at, ac_pool_t *pool)
+static int ac_test_exec(ac_test_t *at, ac_pt_env_t *pt_env)
 {
 	int r;
 
 	if (verbose) {
 		ac_test_show(at);
 	}
-	ac_test_setup_pte(at, pool);
+	ac_test_setup_pte(at, pt_env);
 	r = ac_test_do_access(at);
 	return r;
 }
 
-typedef int (*ac_test_fn)(ac_pool_t *pool, int page_table_levels);
+typedef int (*ac_test_fn)(ac_pt_env_t *pt_env);
 const ac_test_fn ac_test_cases[] =
 {
 	corrupt_hugepage_triger,
@@ -1074,10 +1076,10 @@ const ac_test_fn ac_test_cases[] =
 	check_effective_sp_permissions,
 };
 
-int ac_test_run(int page_table_levels)
+int ac_test_run(int pt_levels)
 {
 	ac_test_t at;
-	ac_pool_t pool;
+	ac_pt_env_t pt_env;
 	int i, tests, successes;
 
 	printf("run\n");
@@ -1140,16 +1142,18 @@ int ac_test_run(int page_table_levels)
 			successes++;
 	}
 
-	ac_env_int(&pool);
-	ac_test_init(&at, (void *)(0x123400000000), page_table_levels);
+	ac_env_int(&pt_env, pt_levels);
+	ac_test_init(&at, (void *)(0x123400000000), &pt_env);
 	do {
 		++tests;
-		successes += ac_test_exec(&at, &pool);
+		successes += ac_test_exec(&at, &pt_env);
 	} while (ac_test_bump(&at));
 
 	for (i = 0; i < ARRAY_SIZE(ac_test_cases); i++) {
+		ac_env_int(&pt_env, pt_levels);
+
 		++tests;
-		successes += ac_test_cases[i](&pool, page_table_levels);
+		successes += ac_test_cases[i](&pt_env);
 	}
 
 	printf("\n%d tests, %d failures\n", tests, tests - successes);
-- 
2.34.0.rc2.393.gf8c9666880-goog

[kvm-unit-tests PATCH 07/39] x86/access: Hoist page table allocator helpers above "init" helper

[Sean Christopherson]

Move the page table allocator above ac_test_init(), a future commit will
handle all page table allocation during init.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/access.c | 42 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/x86/access.c b/x86/access.c
index ba20359..f69071b 100644
--- a/x86/access.c
+++ b/x86/access.c
@@ -281,6 +281,27 @@ static void ac_env_int(ac_pt_env_t *pt_env, int page_table_levels)
 	pt_env->pt_levels = page_table_levels;
 }
 
+static pt_element_t ac_test_alloc_pt(ac_pt_env_t *pt_env)
+{
+	pt_element_t pt;
+
+	pt = pt_env->pt_pool_pa + (pt_env->pt_pool_current * PAGE_SIZE);
+	pt_env->pt_pool_current++;
+	memset(va(pt), 0, PAGE_SIZE);
+	return pt;
+}
+
+static _Bool ac_test_enough_room(ac_pt_env_t *pt_env)
+{
+	/* '120' is completely arbitrary. */
+	return (pt_env->pt_pool_current + 5) < 120;
+}
+
+static void ac_test_reset_pt_pool(ac_pt_env_t *pt_env)
+{
+	pt_env->pt_pool_current = 0;
+}
+
 static void ac_test_init(ac_test_t *at, void *virt, ac_pt_env_t *pt_env)
 {
 	set_efer_nx(1);
@@ -360,27 +381,6 @@ static int ac_test_bump(ac_test_t *at)
 	return ret;
 }
 
-static pt_element_t ac_test_alloc_pt(ac_pt_env_t *pt_env)
-{
-	pt_element_t pt;
-
-	pt = pt_env->pt_pool_pa + (pt_env->pt_pool_current * PAGE_SIZE);
-	pt_env->pt_pool_current++;
-	memset(va(pt), 0, PAGE_SIZE);
-	return pt;
-}
-
-static _Bool ac_test_enough_room(ac_pt_env_t *pt_env)
-{
-	/* '120' is completely arbitrary. */
-	return (pt_env->pt_pool_current + 5) < 120;
-}
-
-static void ac_test_reset_pt_pool(ac_pt_env_t *pt_env)
-{
-	pt_env->pt_pool_current = 0;
-}
-
 static pt_element_t ac_test_permissions(ac_test_t *at, unsigned flags,
 					bool writable, bool user,
 					bool executable)
-- 
2.34.0.rc2.393.gf8c9666880-goog

[kvm-unit-tests PATCH 08/39] x86/access: Rename variables in page table walkers

[Sean Christopherson]

Rename variables in the page table walkers to get rid of the awful root
and vroot terminology, which is obsolete/wrong for everything except the
top-level entry.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/access.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/x86/access.c b/x86/access.c
index f69071b..fb8c194 100644
--- a/x86/access.c
+++ b/x86/access.c
@@ -518,7 +518,7 @@ static void ac_set_expected_status(ac_test_t *at)
 static void __ac_setup_specific_pages(ac_test_t *at, ac_pt_env_t *pt_env, bool reuse,
 				      u64 pd_page, u64 pt_page)
 {
-	unsigned long root = shadow_cr3;
+	unsigned long parent_pte = shadow_cr3;
 	int flags = at->flags;
 	bool skip = true;
 
@@ -527,8 +527,9 @@ static void __ac_setup_specific_pages(ac_test_t *at, ac_pt_env_t *pt_env, bool r
 
 	at->ptep = 0;
 	for (int i = at->pt_levels; i >= 1 && (i >= 2 || !F(AC_PDE_PSE)); --i) {
-		pt_element_t *vroot = va(root & PT_BASE_ADDR_MASK);
+		pt_element_t *parent_pt = va(parent_pte & PT_BASE_ADDR_MASK);
 		unsigned index = PT_INDEX((unsigned long)at->virt, i);
+		pt_element_t *ptep = &parent_pt[index];
 		pt_element_t pte = 0;
 
 		/*
@@ -539,13 +540,13 @@ static void __ac_setup_specific_pages(ac_test_t *at, ac_pt_env_t *pt_env, bool r
 			goto next;
 		}
 		skip = false;
-		if (reuse && vroot[index]) {
+		if (reuse && *ptep) {
 			switch (i) {
 			case 2:
-				at->pdep = &vroot[index];
+				at->pdep = ptep;
 				break;
 			case 1:
-				at->ptep = &vroot[index];
+				at->ptep = ptep;
 				break;
 			}
 			goto next;
@@ -593,7 +594,7 @@ static void __ac_setup_specific_pages(ac_test_t *at, ac_pt_env_t *pt_env, bool r
 				pte |= 1ull << 36;
 			if (F(AC_PDE_BIT13))
 				pte |= 1ull << 13;
-			at->pdep = &vroot[index];
+			at->pdep = ptep;
 			break;
 		case 1:
 			pte = at->phys & PT_BASE_ADDR_MASK;
@@ -615,12 +616,12 @@ static void __ac_setup_specific_pages(ac_test_t *at, ac_pt_env_t *pt_env, bool r
 				pte |= 1ull << 51;
 			if (F(AC_PTE_BIT36))
 				pte |= 1ull << 36;
-			at->ptep = &vroot[index];
+			at->ptep = ptep;
 			break;
 		}
-		vroot[index] = pte;
+		*ptep = pte;
  next:
-		root = vroot[index];
+		parent_pte = *ptep;
 	}
 	ac_set_expected_status(at);
 }
@@ -638,18 +639,18 @@ static void ac_setup_specific_pages(ac_test_t *at, ac_pt_env_t *pt_env,
 
 static void dump_mapping(ac_test_t *at)
 {
-	unsigned long root = shadow_cr3;
+	unsigned long parent_pte = shadow_cr3;
 	int flags = at->flags;
 	int i;
 
 	printf("Dump mapping: address: %p\n", at->virt);
 	for (i = at->pt_levels; i >= 1 && (i >= 2 || !F(AC_PDE_PSE)); --i) {
-		pt_element_t *vroot = va(root & PT_BASE_ADDR_MASK);
+		pt_element_t *parent_pt = va(parent_pte & PT_BASE_ADDR_MASK);
 		unsigned index = PT_INDEX((unsigned long)at->virt, i);
-		pt_element_t pte = vroot[index];
+		pt_element_t pte = parent_pt[index];
 
 		printf("------L%d: %lx\n", i, pte);
-		root = vroot[index];
+		parent_pte = pte;
 	}
 }
 
-- 
2.34.0.rc2.393.gf8c9666880-goog

[kvm-unit-tests PATCH 09/39] x86/access: Abort if page table insertion hits an unexpected level

[Sean Christopherson]

Assert that inserting PTEs doesn't encounter an unhandled level instead
of silently ignoring the bug and shoving a not-present PTE into the page
tables.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/access.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/x86/access.c b/x86/access.c
index fb8c194..06a2420 100644
--- a/x86/access.c
+++ b/x86/access.c
@@ -530,7 +530,7 @@ static void __ac_setup_specific_pages(ac_test_t *at, ac_pt_env_t *pt_env, bool r
 		pt_element_t *parent_pt = va(parent_pte & PT_BASE_ADDR_MASK);
 		unsigned index = PT_INDEX((unsigned long)at->virt, i);
 		pt_element_t *ptep = &parent_pt[index];
-		pt_element_t pte = 0;
+		pt_element_t pte;
 
 		/*
 		 * Reuse existing page tables along the path to the test code and data
@@ -618,7 +618,10 @@ static void __ac_setup_specific_pages(ac_test_t *at, ac_pt_env_t *pt_env, bool r
 				pte |= 1ull << 36;
 			at->ptep = ptep;
 			break;
+		default:
+			assert(0);
 		}
+
 		*ptep = pte;
  next:
 		parent_pte = *ptep;
-- 
2.34.0.rc2.393.gf8c9666880-goog

[kvm-unit-tests PATCH 10/39] x86/access: Make SMEP place nice with 5-level paging

[Sean Christopherson]

Rework the walker to clear the USER bit on the test's text region when
enabling SMEP.  The KUT library assumes 4-level paging (see PAGE_LEVEL),
and completely botches 5-level paging.  Through sheer dumb luck, the test
works with 5-level paging, likely because of an unintentional collision
with the test's own PTEs.

Punt on the library for the time being as the access test is obviously
more than capable of walking page tables, and fixing the library properly
will involve poking many more tests.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/access.c | 86 +++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 61 insertions(+), 25 deletions(-)

diff --git a/x86/access.c b/x86/access.c
index 06a2420..5bd446c 100644
--- a/x86/access.c
+++ b/x86/access.c
@@ -191,6 +191,43 @@ static unsigned long shadow_cr3;
 static unsigned long shadow_cr4;
 static unsigned long long shadow_efer;
 
+typedef void (*walk_fn)(pt_element_t *ptep, int level, unsigned long virt);
+
+/* Returns the size of the range covered by the last processed entry. */
+static unsigned long walk_va(ac_test_t *at, int min_level, unsigned long virt,
+			     walk_fn callback, bool leaf_only)
+{
+	unsigned long parent_pte = shadow_cr3;
+	int i;
+
+	for (i = at->pt_levels; i >= min_level; --i) {
+		pt_element_t *parent_pt = va(parent_pte & PT_BASE_ADDR_MASK);
+		unsigned int index = PT_INDEX(virt, i);
+		pt_element_t *ptep = &parent_pt[index];
+
+		assert(!leaf_only || (*ptep & PT_PRESENT_MASK));
+
+		if (!leaf_only || i == 1 || (*ptep & PT_PAGE_SIZE_MASK))
+			callback(ptep, i, virt);
+
+		if (i == 1 || *ptep & PT_PAGE_SIZE_MASK)
+			break;
+
+		parent_pte = *ptep;
+	}
+
+	return 1ul << PGDIR_BITS(i);
+}
+
+static void walk_ptes(ac_test_t *at, unsigned long virt, unsigned long end,
+		      walk_fn callback)
+{
+	unsigned long page_size;
+
+	for ( ; virt < end; virt = ALIGN_DOWN(virt + page_size, page_size))
+		page_size = walk_va(at, 1, virt, callback, true);
+}
+
 static void set_cr0_wp(int wp)
 {
 	unsigned long cr0 = shadow_cr0;
@@ -204,23 +241,24 @@ static void set_cr0_wp(int wp)
 	}
 }
 
-static void clear_user_mask(struct pte_search search, void *va)
+static void clear_user_mask(pt_element_t *ptep, int level, unsigned long virt)
 {
-	*search.pte &= ~PT_USER_MASK;
+	*ptep &= ~PT_USER_MASK;
 }
 
-static void set_user_mask(struct pte_search search, void *va)
+static void set_user_mask(pt_element_t *ptep, int level, unsigned long virt)
 {
-	*search.pte |= PT_USER_MASK;
+	*ptep |= PT_USER_MASK;
 
 	/* Flush to avoid spurious #PF */
-	invlpg(va);
+	invlpg((void*)virt);
 }
 
-static unsigned set_cr4_smep(int smep)
+static unsigned set_cr4_smep(ac_test_t *at, int smep)
 {
 	extern char stext, etext;
-	size_t len = (size_t)&etext - (size_t)&stext;
+	unsigned long code_start = (unsigned long)&stext;
+	unsigned long code_end = (unsigned long)&etext;
 	unsigned long cr4 = shadow_cr4;
 	unsigned r;
 
@@ -231,10 +269,10 @@ static unsigned set_cr4_smep(int smep)
 		return 0;
 
 	if (smep)
-		walk_pte(&stext, len, clear_user_mask);
+		walk_ptes(at, code_start, code_end, clear_user_mask);
 	r = write_cr4_checking(cr4);
 	if (r || !smep)
-		walk_pte(&stext, len, set_user_mask);
+		walk_ptes(at, code_start, code_end, set_user_mask);
 	if (!r)
 		shadow_cr4 = cr4;
 	return r;
@@ -640,21 +678,18 @@ static void ac_setup_specific_pages(ac_test_t *at, ac_pt_env_t *pt_env,
 	return __ac_setup_specific_pages(at, pt_env, false, pd_page, pt_page);
 }
 
+static void __dump_pte(pt_element_t *ptep, int level, unsigned long virt)
+{
+	printf("------L%d: %lx\n", level, *ptep);
+}
+
 static void dump_mapping(ac_test_t *at)
 {
-	unsigned long parent_pte = shadow_cr3;
+	unsigned long virt = (unsigned long)at->virt;
 	int flags = at->flags;
-	int i;
 
 	printf("Dump mapping: address: %p\n", at->virt);
-	for (i = at->pt_levels; i >= 1 && (i >= 2 || !F(AC_PDE_PSE)); --i) {
-		pt_element_t *parent_pt = va(parent_pte & PT_BASE_ADDR_MASK);
-		unsigned index = PT_INDEX((unsigned long)at->virt, i);
-		pt_element_t pte = parent_pt[index];
-
-		printf("------L%d: %lx\n", i, pte);
-		parent_pte = pte;
-	}
+	walk_va(at, F(AC_PDE_PSE) ? 2 : 1, virt, __dump_pte, false);
 }
 
 static void ac_test_check(ac_test_t *at, _Bool *success_ret, _Bool cond,
@@ -719,7 +754,7 @@ static int ac_test_do_access(ac_test_t *at)
 			   (F(AC_PKU_AD) ? 4 : 0));
 	}
 
-	set_cr4_smep(F(AC_CPU_CR4_SMEP));
+	set_cr4_smep(at, F(AC_CPU_CR4_SMEP));
 
 	if (F(AC_ACCESS_TWICE)) {
 		asm volatile ("mov $fixed2, %%rsi \n\t"
@@ -977,7 +1012,7 @@ static int check_smep_andnot_wp(ac_pt_env_t *pt_env)
 	err_smep_andnot_wp = ac_test_do_access(&at1);
 
 clean_up:
-	set_cr4_smep(0);
+	set_cr4_smep(&at1, 0);
 
 	if (!err_prepare_andnot_wp)
 		goto err;
@@ -1103,6 +1138,9 @@ int ac_test_run(int pt_levels)
 		invalid_mask |= AC_PTE_BIT36_MASK;
 	}
 
+	ac_env_int(&pt_env, pt_levels);
+	ac_test_init(&at, (void *)(0x123400000000), &pt_env);
+
 	if (this_cpu_has(X86_FEATURE_PKU)) {
 		set_cr4_pke(1);
 		set_cr4_pke(0);
@@ -1124,13 +1162,13 @@ int ac_test_run(int pt_levels)
 
 	if (!this_cpu_has(X86_FEATURE_SMEP)) {
 		tests++;
-		if (set_cr4_smep(1) == GP_VECTOR) {
+		if (set_cr4_smep(&at, 1) == GP_VECTOR) {
 			successes++;
 			invalid_mask |= AC_CPU_CR4_SMEP_MASK;
 			printf("CR4.SMEP not available, disabling SMEP tests\n");
 		} else {
 			printf("Set SMEP in CR4 - expect #GP: FAIL!\n");
-			set_cr4_smep(0);
+			set_cr4_smep(&at, 0);
 		}
 	}
 
@@ -1146,8 +1184,6 @@ int ac_test_run(int pt_levels)
 			successes++;
 	}
 
-	ac_env_int(&pt_env, pt_levels);
-	ac_test_init(&at, (void *)(0x123400000000), &pt_env);
 	do {
 		++tests;
 		successes += ac_test_exec(&at, &pt_env);
-- 
2.34.0.rc2.393.gf8c9666880-goog

[kvm-unit-tests PATCH 11/39] x86/access: Use upper half of virtual address space

[Sean Christopherson]

Use the upper half of the virtual address space so that 5-level paging
doesn't collide with the core infrastucture in the top-level PTE, which
hides bugs, e.g. SMEP + 5-level, and is generally a nightmare to debug.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/access.c | 50 ++++++++++++++++++++++++++++++--------------------
 1 file changed, 30 insertions(+), 20 deletions(-)

diff --git a/x86/access.c b/x86/access.c
index 5bd446c..6ccdb76 100644
--- a/x86/access.c
+++ b/x86/access.c
@@ -34,7 +34,7 @@ static int invalid_mask;
 #define EFER_NX_MASK            (1ull << 11)
 
 #define PT_INDEX(address, level)       \
-	  ((address) >> (12 + ((level)-1) * 9)) & 511
+	  (((address) >> (12 + ((level)-1) * 9)) & 511)
 
 /*
  * page table access check tests
@@ -340,12 +340,21 @@ static void ac_test_reset_pt_pool(ac_pt_env_t *pt_env)
 	pt_env->pt_pool_current = 0;
 }
 
-static void ac_test_init(ac_test_t *at, void *virt, ac_pt_env_t *pt_env)
+static void ac_test_init(ac_test_t *at, unsigned long virt, ac_pt_env_t *pt_env)
 {
+	/*
+	 * The KUT infrastructure, e.g. this function, must use a different
+	 * top-level SPTE than the test, otherwise modifying SPTEs can affect
+	 * normal behavior, e.g. crash the test due to marking code SPTEs
+	 * USER when CR4.SMEP=1.
+	 */
+	assert(PT_INDEX(virt, pt_env->pt_levels) !=
+	       PT_INDEX((unsigned long)ac_test_init, pt_env->pt_levels));
+
 	set_efer_nx(1);
 	set_cr0_wp(1);
 	at->flags = 0;
-	at->virt = virt;
+	at->virt = (void *)virt;
 	at->phys = AT_CODE_DATA_PHYS;
 	at->pt_levels = pt_env->pt_levels;
 }
@@ -571,12 +580,13 @@ static void __ac_setup_specific_pages(ac_test_t *at, ac_pt_env_t *pt_env, bool r
 		pt_element_t pte;
 
 		/*
-		 * Reuse existing page tables along the path to the test code and data
-		 * (which is in the bottom 2MB).
+		 * Reuse existing page tables along the highest index, some
+		 * tests rely on sharing upper level paging structures between
+		 * two separate sub-tests.
 		 */
-		if (skip && i >= 2 && index == 0) {
+		if (skip && i >= 2 && index == 511 && (*ptep & PT_PRESENT_MASK))
 			goto next;
-		}
+
 		skip = false;
 		if (reuse && *ptep) {
 			switch (i) {
@@ -863,8 +873,8 @@ static int corrupt_hugepage_triger(ac_pt_env_t *pt_env)
 {
 	ac_test_t at1, at2;
 
-	ac_test_init(&at1, (void *)(0x123400000000), pt_env);
-	ac_test_init(&at2, (void *)(0x666600000000), pt_env);
+	ac_test_init(&at1, 0xffff923400000000ul, pt_env);
+	ac_test_init(&at2, 0xffffe66600000000ul, pt_env);
 
 	at2.flags = AC_CPU_CR0_WP_MASK | AC_PDE_PSE_MASK | AC_PDE_PRESENT_MASK;
 	ac_test_setup_pte(&at2, pt_env);
@@ -901,8 +911,8 @@ static int check_pfec_on_prefetch_pte(ac_pt_env_t *pt_env)
 {
 	ac_test_t at1, at2;
 
-	ac_test_init(&at1, (void *)(0x123406001000), pt_env);
-	ac_test_init(&at2, (void *)(0x123406003000), pt_env);
+	ac_test_init(&at1, 0xffff923406001000ul, pt_env);
+	ac_test_init(&at2, 0xffff923406003000ul, pt_env);
 
 	at1.flags = AC_PDE_PRESENT_MASK | AC_PTE_PRESENT_MASK;
 	ac_setup_specific_pages(&at1, pt_env, 30 * 1024 * 1024, 30 * 1024 * 1024);
@@ -946,8 +956,8 @@ static int check_large_pte_dirty_for_nowp(ac_pt_env_t *pt_env)
 {
 	ac_test_t at1, at2;
 
-	ac_test_init(&at1, (void *)(0x123403000000), pt_env);
-	ac_test_init(&at2, (void *)(0x666606000000), pt_env);
+	ac_test_init(&at1, 0xffff923403000000ul, pt_env);
+	ac_test_init(&at2, 0xffffe66606000000ul, pt_env);
 
 	at2.flags = AC_PDE_PRESENT_MASK | AC_PDE_PSE_MASK;
 	ac_test_setup_pte(&at2, pt_env);
@@ -985,7 +995,7 @@ static int check_smep_andnot_wp(ac_pt_env_t *pt_env)
 		return 1;
 	}
 
-	ac_test_init(&at1, (void *)(0x123406001000), pt_env);
+	ac_test_init(&at1, 0xffff923406001000ul, pt_env);
 
 	at1.flags = AC_PDE_PRESENT_MASK | AC_PTE_PRESENT_MASK |
 		    AC_PDE_USER_MASK | AC_PTE_USER_MASK |
@@ -1028,7 +1038,7 @@ err:
 
 static int check_effective_sp_permissions(ac_pt_env_t *pt_env)
 {
-	unsigned long ptr1 = 0x123480000000;
+	unsigned long ptr1 = 0xffff923480000000;
 	unsigned long ptr2 = ptr1 + SZ_2M;
 	unsigned long ptr3 = ptr1 + SZ_1G;
 	unsigned long ptr4 = ptr3 + SZ_2M;
@@ -1047,22 +1057,22 @@ static int check_effective_sp_permissions(ac_pt_env_t *pt_env)
 	 * pud1 and pud2 point to the same pmd page.
 	 */
 
-	ac_test_init(&at1, (void *)(ptr1), pt_env);
+	ac_test_init(&at1, ptr1, pt_env);
 	at1.flags = AC_PDE_PRESENT_MASK | AC_PTE_PRESENT_MASK |
 		    AC_PDE_USER_MASK | AC_PTE_USER_MASK |
 		    AC_PDE_ACCESSED_MASK | AC_PTE_ACCESSED_MASK |
 		    AC_PTE_WRITABLE_MASK | AC_ACCESS_USER_MASK;
 	__ac_setup_specific_pages(&at1, pt_env, false, pmd, 0);
 
-	ac_test_init(&at2, (void *)(ptr2), pt_env);
+	ac_test_init(&at2, ptr2, pt_env);
 	at2.flags = at1.flags | AC_PDE_WRITABLE_MASK | AC_PTE_DIRTY_MASK | AC_ACCESS_WRITE_MASK;
 	__ac_setup_specific_pages(&at2, pt_env, true, pmd, 0);
 
-	ac_test_init(&at3, (void *)(ptr3), pt_env);
+	ac_test_init(&at3, ptr3, pt_env);
 	at3.flags = AC_PDPTE_NO_WRITABLE_MASK | at1.flags;
 	__ac_setup_specific_pages(&at3, pt_env, true, pmd, 0);
 
-	ac_test_init(&at4, (void *)(ptr4), pt_env);
+	ac_test_init(&at4, ptr4, pt_env);
 	at4.flags = AC_PDPTE_NO_WRITABLE_MASK | at2.flags;
 	__ac_setup_specific_pages(&at4, pt_env, true, pmd, 0);
 
@@ -1139,7 +1149,7 @@ int ac_test_run(int pt_levels)
 	}
 
 	ac_env_int(&pt_env, pt_levels);
-	ac_test_init(&at, (void *)(0x123400000000), &pt_env);
+	ac_test_init(&at, 0xffff923400000000ul, &pt_env);
 
 	if (this_cpu_has(X86_FEATURE_PKU)) {
 		set_cr4_pke(1);
-- 
2.34.0.rc2.393.gf8c9666880-goog

[kvm-unit-tests PATCH 12/39] x86/access: Print the index when dumping PTEs

[Sean Christopherson]

Print the index of a PTE in addition to its level.  If there's a test bug
that causes an unwanted collision, the index is critical information to
understanding what has gone wrong.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/access.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/x86/access.c b/x86/access.c
index 6ccdb76..6c1e20e 100644
--- a/x86/access.c
+++ b/x86/access.c
@@ -690,7 +690,7 @@ static void ac_setup_specific_pages(ac_test_t *at, ac_pt_env_t *pt_env,
 
 static void __dump_pte(pt_element_t *ptep, int level, unsigned long virt)
 {
-	printf("------L%d: %lx\n", level, *ptep);
+	printf("------L%d I%lu: %lx\n", level, PT_INDEX(virt, level), *ptep);
 }
 
 static void dump_mapping(ac_test_t *at)
-- 
2.34.0.rc2.393.gf8c9666880-goog

[kvm-unit-tests PATCH 13/39] x86/access: Pre-allocate all page tables at (sub)test init

[Sean Christopherson]

Pre-allocate the page tables for each test instead of allocating page
tables on every. single. iteration.  In addition to being abysmally slow,
constantly allocating new page tables obliterates any hope of providing
meaningful test coverage for shadow paging, as using a new upper level
PTE for every iteration causes KVM to sync children, which prevents
exposing TLB flushing bugs in KVM.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/access.c | 169 ++++++++++++++++++++++++++++++---------------------
 1 file changed, 101 insertions(+), 68 deletions(-)

diff --git a/x86/access.c b/x86/access.c
index 6c1e20e..abc6590 100644
--- a/x86/access.c
+++ b/x86/access.c
@@ -176,6 +176,9 @@ typedef struct {
 	int expected_fault;
 	unsigned expected_error;
 	int pt_levels;
+
+	/* 5-level paging, 1-based to avoid math. */
+	pt_element_t page_tables[6];
 } ac_test_t;
 
 typedef struct {
@@ -323,25 +326,25 @@ static pt_element_t ac_test_alloc_pt(ac_pt_env_t *pt_env)
 {
 	pt_element_t pt;
 
+	/*
+	 * Each test needs at most pt_levels-1 structures per virtual address,
+	 * and no existing scenario uses more than four addresses.
+	 */
+	assert(pt_env->pt_pool_current < (4 * (pt_env->pt_levels - 1)));
+
 	pt = pt_env->pt_pool_pa + (pt_env->pt_pool_current * PAGE_SIZE);
 	pt_env->pt_pool_current++;
 	memset(va(pt), 0, PAGE_SIZE);
 	return pt;
 }
 
-static _Bool ac_test_enough_room(ac_pt_env_t *pt_env)
+static void __ac_test_init(ac_test_t *at, unsigned long virt,
+			   ac_pt_env_t *pt_env, ac_test_t *buddy)
 {
-	/* '120' is completely arbitrary. */
-	return (pt_env->pt_pool_current + 5) < 120;
-}
+	unsigned long buddy_virt = buddy ? (unsigned long)buddy->virt : 0;
+	pt_element_t *root_pt = va(shadow_cr3 & PT_BASE_ADDR_MASK);
+	int i;
 
-static void ac_test_reset_pt_pool(ac_pt_env_t *pt_env)
-{
-	pt_env->pt_pool_current = 0;
-}
-
-static void ac_test_init(ac_test_t *at, unsigned long virt, ac_pt_env_t *pt_env)
-{
 	/*
 	 * The KUT infrastructure, e.g. this function, must use a different
 	 * top-level SPTE than the test, otherwise modifying SPTEs can affect
@@ -349,7 +352,7 @@ static void ac_test_init(ac_test_t *at, unsigned long virt, ac_pt_env_t *pt_env)
 	 * USER when CR4.SMEP=1.
 	 */
 	assert(PT_INDEX(virt, pt_env->pt_levels) !=
-	       PT_INDEX((unsigned long)ac_test_init, pt_env->pt_levels));
+	       PT_INDEX((unsigned long)__ac_test_init, pt_env->pt_levels));
 
 	set_efer_nx(1);
 	set_cr0_wp(1);
@@ -357,6 +360,33 @@ static void ac_test_init(ac_test_t *at, unsigned long virt, ac_pt_env_t *pt_env)
 	at->virt = (void *)virt;
 	at->phys = AT_CODE_DATA_PHYS;
 	at->pt_levels = pt_env->pt_levels;
+
+	at->page_tables[0] = -1ull;
+	at->page_tables[1] = -1ull;
+
+	/*
+	 * Zap the existing top-level PTE as it may be reused from a previous
+	 * sub-test.  This allows runtime PTE modification to assert that two
+	 * overlapping walks don't try to install different paging structures.
+	 */
+	root_pt[PT_INDEX(virt, pt_env->pt_levels)] = 0;
+
+	for (i = at->pt_levels; i > 1; i--) {
+		/*
+		 * Buddies can reuse any part of the walk that share the same
+		 * index.  This is weird, but intentional, as several tests
+		 * want different walks to merge at lower levels.
+		 */
+		if (buddy && PT_INDEX(virt, i) == PT_INDEX(buddy_virt, i))
+			at->page_tables[i] = buddy->page_tables[i];
+		else
+			at->page_tables[i] = ac_test_alloc_pt(pt_env);
+	}
+}
+
+static void ac_test_init(ac_test_t *at, unsigned long virt, ac_pt_env_t *pt_env)
+{
+	__ac_test_init(at, virt, pt_env, NULL);
 }
 
 static int ac_test_bump_one(ac_test_t *at)
@@ -372,6 +402,9 @@ static _Bool ac_test_legal(ac_test_t *at)
 	int flags = at->flags;
 	unsigned reserved;
 
+	if (F(AC_CPU_CR4_SMEP))
+		return false;
+
 	if (F(AC_ACCESS_FETCH) && F(AC_ACCESS_WRITE))
 		return false;
 
@@ -562,59 +595,60 @@ static void ac_set_expected_status(ac_test_t *at)
 	ac_emulate_access(at, at->flags);
 }
 
-static void __ac_setup_specific_pages(ac_test_t *at, ac_pt_env_t *pt_env, bool reuse,
-				      u64 pd_page, u64 pt_page)
+static pt_element_t ac_get_pt(ac_test_t *at, int i, pt_element_t *ptep)
+{
+	pt_element_t pte;
+
+	pte = *ptep;
+	if (pte && !(pte & PT_PAGE_SIZE_MASK) &&
+	    (pte & PT_BASE_ADDR_MASK) != at->page_tables[i]) {
+		printf("\nPT collision.  VA = 0x%lx, level = %d, index = %ld, found PT = 0x%lx, want PT = 0x%lx\n",
+			(unsigned long)at->virt, i,
+			PT_INDEX((unsigned long)at->virt, i),
+			pte, at->page_tables[i]);
+		abort();
+	}
+
+	pte = at->page_tables[i];
+	return pte;
+}
+
+static void __ac_setup_specific_pages(ac_test_t *at, u64 pd_page, u64 pt_page)
 {
 	unsigned long parent_pte = shadow_cr3;
 	int flags = at->flags;
-	bool skip = true;
-
-	if (!ac_test_enough_room(pt_env))
-		ac_test_reset_pt_pool(pt_env);
+	int i;
 
 	at->ptep = 0;
-	for (int i = at->pt_levels; i >= 1 && (i >= 2 || !F(AC_PDE_PSE)); --i) {
+	for (i = at->pt_levels; i >= 1 && (i >= 2 || !F(AC_PDE_PSE)); --i) {
 		pt_element_t *parent_pt = va(parent_pte & PT_BASE_ADDR_MASK);
 		unsigned index = PT_INDEX((unsigned long)at->virt, i);
 		pt_element_t *ptep = &parent_pt[index];
 		pt_element_t pte;
 
-		/*
-		 * Reuse existing page tables along the highest index, some
-		 * tests rely on sharing upper level paging structures between
-		 * two separate sub-tests.
-		 */
-		if (skip && i >= 2 && index == 511 && (*ptep & PT_PRESENT_MASK))
-			goto next;
-
-		skip = false;
-		if (reuse && *ptep) {
-			switch (i) {
-			case 2:
-				at->pdep = ptep;
-				break;
-			case 1:
-				at->ptep = ptep;
-				break;
-			}
-			goto next;
-		}
-
 		switch (i) {
 		case 5:
 		case 4:
-			pte = ac_test_alloc_pt(pt_env);
+			pte = ac_get_pt(at, i, ptep);
 			pte |= PT_PRESENT_MASK | PT_WRITABLE_MASK | PT_USER_MASK;
 			break;
 		case 3:
-			pte = pd_page ? pd_page : ac_test_alloc_pt(pt_env);
+			if (pd_page)
+				pte = pd_page;
+			else
+				pte = ac_get_pt(at, i, ptep);
+
 			pte |= PT_PRESENT_MASK | PT_USER_MASK;
 			if (!F(AC_PDPTE_NO_WRITABLE))
 				pte |= PT_WRITABLE_MASK;
 			break;
 		case 2:
 			if (!F(AC_PDE_PSE)) {
-				pte = pt_page ? pt_page : ac_test_alloc_pt(pt_env);
+				if (pt_page)
+					pte = pt_page;
+				else
+					pte = ac_get_pt(at, i, ptep);
+
 				/* The protection key is ignored on non-leaf entries.  */
 				if (F(AC_PKU_PKEY))
 					pte |= 2ull << 59;
@@ -671,21 +705,20 @@ static void __ac_setup_specific_pages(ac_test_t *at, ac_pt_env_t *pt_env, bool r
 		}
 
 		*ptep = pte;
- next:
-		parent_pte = *ptep;
+
+		parent_pte = pte;
 	}
 	ac_set_expected_status(at);
 }
 
-static void ac_test_setup_pte(ac_test_t *at, ac_pt_env_t *pt_env)
+static void ac_test_setup_pte(ac_test_t *at)
 {
-	__ac_setup_specific_pages(at, pt_env, false, 0, 0);
+	__ac_setup_specific_pages(at, 0, 0);
 }
 
-static void ac_setup_specific_pages(ac_test_t *at, ac_pt_env_t *pt_env,
-				    u64 pd_page, u64 pt_page)
+static void ac_setup_specific_pages(ac_test_t *at, u64 pd_page, u64 pt_page)
 {
-	return __ac_setup_specific_pages(at, pt_env, false, pd_page, pt_page);
+	return __ac_setup_specific_pages(at, pd_page, pt_page);
 }
 
 static void __dump_pte(pt_element_t *ptep, int level, unsigned long virt)
@@ -874,15 +907,15 @@ static int corrupt_hugepage_triger(ac_pt_env_t *pt_env)
 	ac_test_t at1, at2;
 
 	ac_test_init(&at1, 0xffff923400000000ul, pt_env);
-	ac_test_init(&at2, 0xffffe66600000000ul, pt_env);
+	__ac_test_init(&at2, 0xffffe66600000000ul, pt_env, &at1);
 
 	at2.flags = AC_CPU_CR0_WP_MASK | AC_PDE_PSE_MASK | AC_PDE_PRESENT_MASK;
-	ac_test_setup_pte(&at2, pt_env);
+	ac_test_setup_pte(&at2);
 	if (!ac_test_do_access(&at2))
 		goto err;
 
 	at1.flags = at2.flags | AC_PDE_WRITABLE_MASK;
-	ac_test_setup_pte(&at1, pt_env);
+	ac_test_setup_pte(&at1);
 	if (!ac_test_do_access(&at1))
 		goto err;
 
@@ -912,13 +945,13 @@ static int check_pfec_on_prefetch_pte(ac_pt_env_t *pt_env)
 	ac_test_t at1, at2;
 
 	ac_test_init(&at1, 0xffff923406001000ul, pt_env);
-	ac_test_init(&at2, 0xffff923406003000ul, pt_env);
+	__ac_test_init(&at2, 0xffff923406003000ul, pt_env, &at1);
 
 	at1.flags = AC_PDE_PRESENT_MASK | AC_PTE_PRESENT_MASK;
-	ac_setup_specific_pages(&at1, pt_env, 30 * 1024 * 1024, 30 * 1024 * 1024);
+	ac_setup_specific_pages(&at1, 30 * 1024 * 1024, 30 * 1024 * 1024);
 
 	at2.flags = at1.flags | AC_PTE_NX_MASK;
-	ac_setup_specific_pages(&at2, pt_env, 30 * 1024 * 1024, 30 * 1024 * 1024);
+	ac_setup_specific_pages(&at2, 30 * 1024 * 1024, 30 * 1024 * 1024);
 
 	if (!ac_test_do_access(&at1)) {
 		printf("%s: prepare fail\n", __FUNCTION__);
@@ -957,17 +990,17 @@ static int check_large_pte_dirty_for_nowp(ac_pt_env_t *pt_env)
 	ac_test_t at1, at2;
 
 	ac_test_init(&at1, 0xffff923403000000ul, pt_env);
-	ac_test_init(&at2, 0xffffe66606000000ul, pt_env);
+	__ac_test_init(&at2, 0xffffe66606000000ul, pt_env, &at1);
 
 	at2.flags = AC_PDE_PRESENT_MASK | AC_PDE_PSE_MASK;
-	ac_test_setup_pte(&at2, pt_env);
+	ac_test_setup_pte(&at2);
 	if (!ac_test_do_access(&at2)) {
 		printf("%s: read on the first mapping fail.\n", __FUNCTION__);
 		goto err;
 	}
 
 	at1.flags = at2.flags | AC_ACCESS_WRITE_MASK;
-	ac_test_setup_pte(&at1, pt_env);
+	ac_test_setup_pte(&at1);
 	if (!ac_test_do_access(&at1)) {
 		printf("%s: write on the second mapping fail.\n", __FUNCTION__);
 		goto err;
@@ -1003,7 +1036,7 @@ static int check_smep_andnot_wp(ac_pt_env_t *pt_env)
 		    AC_CPU_CR4_SMEP_MASK |
 		    AC_CPU_CR0_WP_MASK |
 		    AC_ACCESS_WRITE_MASK;
-	ac_test_setup_pte(&at1, pt_env);
+	ac_test_setup_pte(&at1);
 
 	/*
 	 * Here we write the ro user page when
@@ -1062,19 +1095,19 @@ static int check_effective_sp_permissions(ac_pt_env_t *pt_env)
 		    AC_PDE_USER_MASK | AC_PTE_USER_MASK |
 		    AC_PDE_ACCESSED_MASK | AC_PTE_ACCESSED_MASK |
 		    AC_PTE_WRITABLE_MASK | AC_ACCESS_USER_MASK;
-	__ac_setup_specific_pages(&at1, pt_env, false, pmd, 0);
+	__ac_setup_specific_pages(&at1, pmd, 0);
 
-	ac_test_init(&at2, ptr2, pt_env);
+	__ac_test_init(&at2, ptr2, pt_env, &at1);
 	at2.flags = at1.flags | AC_PDE_WRITABLE_MASK | AC_PTE_DIRTY_MASK | AC_ACCESS_WRITE_MASK;
-	__ac_setup_specific_pages(&at2, pt_env, true, pmd, 0);
+	__ac_setup_specific_pages(&at2, pmd, 0);
 
-	ac_test_init(&at3, ptr3, pt_env);
+	__ac_test_init(&at3, ptr3, pt_env, &at1);
 	at3.flags = AC_PDPTE_NO_WRITABLE_MASK | at1.flags;
-	__ac_setup_specific_pages(&at3, pt_env, true, pmd, 0);
+	__ac_setup_specific_pages(&at3, pmd, 0);
 
-	ac_test_init(&at4, ptr4, pt_env);
+	__ac_test_init(&at4, ptr4, pt_env, &at2);
 	at4.flags = AC_PDPTE_NO_WRITABLE_MASK | at2.flags;
-	__ac_setup_specific_pages(&at4, pt_env, true, pmd, 0);
+	__ac_setup_specific_pages(&at4, pmd, 0);
 
 	err_read_at1 = ac_test_do_access(&at1);
 	if (!err_read_at1) {
@@ -1110,7 +1143,7 @@ static int ac_test_exec(ac_test_t *at, ac_pt_env_t *pt_env)
 	if (verbose) {
 		ac_test_show(at);
 	}
-	ac_test_setup_pte(at, pt_env);
+	ac_test_setup_pte(at);
 	r = ac_test_do_access(at);
 	return r;
 }
-- 
2.34.0.rc2.393.gf8c9666880-goog

Re: [kvm-unit-tests PATCH 13/39] x86/access: Pre-allocate all page tables at (sub)test init

[Paolo Bonzini]

On 11/25/21 02:28, Sean Christopherson wrote:
> -static _Bool ac_test_enough_room(ac_pt_env_t *pt_env)
> +static void __ac_test_init(ac_test_t *at, unsigned long virt,
> +			   ac_pt_env_t *pt_env, ac_test_t *buddy)
>   {
> -	/* '120' is completely arbitrary. */
> -	return (pt_env->pt_pool_current + 5) < 120;
> -}

Ok, so the early change to the "pool" logic was mostly cosmetic as it 
goes away.

Paolo

[kvm-unit-tests PATCH 14/39] x86/access: Don't write page tables if desired PTE is same as current PTE

[Sean Christopherson]

Avoid spurious PTE writes, KVM doesn't check if old==new when handling
write-protected SPTEs and triggers an MMU sync when using shadow paging
even if the SPTE is unchanged.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/access.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/x86/access.c b/x86/access.c
index abc6590..53b1221 100644
--- a/x86/access.c
+++ b/x86/access.c
@@ -704,7 +704,8 @@ static void __ac_setup_specific_pages(ac_test_t *at, u64 pd_page, u64 pt_page)
 			assert(0);
 		}
 
-		*ptep = pte;
+		if (pte != *ptep)
+			*ptep = pte;
 
 		parent_pte = pte;
 	}
-- 
2.34.0.rc2.393.gf8c9666880-goog

[kvm-unit-tests PATCH 15/39] x86/access: Preserve A/D bits when writing paging structure entries

[Sean Christopherson]

Preserve A/D bits for paging structure entries to avoid pointless writes
to PTEs between test iterations, and more importantly to avoid triggering
MMU syncs due to writing upper-level PTEs.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/access.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/x86/access.c b/x86/access.c
index 53b1221..c4db368 100644
--- a/x86/access.c
+++ b/x86/access.c
@@ -609,7 +609,11 @@ static pt_element_t ac_get_pt(ac_test_t *at, int i, pt_element_t *ptep)
 		abort();
 	}
 
-	pte = at->page_tables[i];
+	/*
+	 * Preserve A/D bits to avoid writing upper level PTEs,
+	 * which cannot be unsyc'd when KVM uses shadow paging.
+	 */
+	pte = at->page_tables[i] | (pte & (PT_DIRTY_MASK | PT_ACCESSED_MASK));
 	return pte;
 }
 
-- 
2.34.0.rc2.393.gf8c9666880-goog

[kvm-unit-tests PATCH 16/39] x86/access: Make toggling of PRESENT bit a "higher order" action

[Sean Christopherson]

Make the PRESENT bit a higher order bit so that it isn't toggled on every
iteration, which is a wee bit problematic when trying to expose bugs in
KVM's TLB flushing since a fault is architecturally guaranteed to flush
TLB entries for the faulting virtual address.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/access.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/x86/access.c b/x86/access.c
index c4db368..24ddeec 100644
--- a/x86/access.c
+++ b/x86/access.c
@@ -37,26 +37,30 @@ static int invalid_mask;
 	  (((address) >> (12 + ((level)-1) * 9)) & 511)
 
 /*
- * page table access check tests
+ * Page table access check tests.  Each number/bit represent an individual
+ * test case.  The main test will bump a counter by 1 to run all permutations
+ * of the below test cases (sans illegal combinations).
+ *
+ * Keep the PRESENT and reserved bits in the higher numbers so that they aren't
+ * toggled on every test, e.g. to keep entries in the TLB.
  */
-
 enum {
-	AC_PTE_PRESENT_BIT,
 	AC_PTE_WRITABLE_BIT,
 	AC_PTE_USER_BIT,
 	AC_PTE_ACCESSED_BIT,
 	AC_PTE_DIRTY_BIT,
 	AC_PTE_NX_BIT,
+	AC_PTE_PRESENT_BIT,
 	AC_PTE_BIT51_BIT,
 	AC_PTE_BIT36_BIT,
 
-	AC_PDE_PRESENT_BIT,
 	AC_PDE_WRITABLE_BIT,
 	AC_PDE_USER_BIT,
 	AC_PDE_ACCESSED_BIT,
 	AC_PDE_DIRTY_BIT,
 	AC_PDE_PSE_BIT,
 	AC_PDE_NX_BIT,
+	AC_PDE_PRESENT_BIT,
 	AC_PDE_BIT51_BIT,
 	AC_PDE_BIT36_BIT,
 	AC_PDE_BIT13_BIT,
-- 
2.34.0.rc2.393.gf8c9666880-goog

[kvm-unit-tests PATCH 17/39] x86/access: Manually override PMD in effective permissions sub-test

[Sean Christopherson]

Manually override the PMD in the effective permissions sub-test when
splicing two walks together, this will eventually allow dropping the
overrides from the main PTE insertion helper.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/access.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/x86/access.c b/x86/access.c
index 24ddeec..3e1a684 100644
--- a/x86/access.c
+++ b/x86/access.c
@@ -1084,18 +1084,17 @@ static int check_effective_sp_permissions(ac_pt_env_t *pt_env)
 	unsigned long ptr2 = ptr1 + SZ_2M;
 	unsigned long ptr3 = ptr1 + SZ_1G;
 	unsigned long ptr4 = ptr3 + SZ_2M;
-	pt_element_t pmd = ac_test_alloc_pt(pt_env);
 	ac_test_t at1, at2, at3, at4;
 	int err_read_at1, err_write_at2;
 	int err_read_at3, err_write_at4;
 
 	/*
 	 * pgd[]   pud[]        pmd[]            virtual address pointers
-	 *                   /->pmd1(u--)->pte1(uw-)->page1 <- ptr1 (u--)
-	 *      /->pud1(uw-)--->pmd2(uw-)->pte2(uw-)->page2 <- ptr2 (uw-)
-	 * pgd-|           (shared pmd[] as above)
-	 *      \->pud2(u--)--->pmd1(u--)->pte1(uw-)->page1 <- ptr3 (u--)
-	 *                   \->pmd2(uw-)->pte2(uw-)->page2 <- ptr4 (u--)
+	 *                   /->pmd(u--)->pte1(uw-)->page1 <- ptr1 (u--)
+	 *      /->pud1(uw-)--->pmd(uw-)->pte2(uw-)->page2 <- ptr2 (uw-)
+	 * pgd-|
+	 *      \->pud2(u--)--->pmd(u--)->pte1(uw-)->page1 <- ptr3 (u--)
+	 *                   \->pmd(uw-)->pte2(uw-)->page2 <- ptr4 (u--)
 	 * pud1 and pud2 point to the same pmd page.
 	 */
 
@@ -1104,19 +1103,23 @@ static int check_effective_sp_permissions(ac_pt_env_t *pt_env)
 		    AC_PDE_USER_MASK | AC_PTE_USER_MASK |
 		    AC_PDE_ACCESSED_MASK | AC_PTE_ACCESSED_MASK |
 		    AC_PTE_WRITABLE_MASK | AC_ACCESS_USER_MASK;
-	__ac_setup_specific_pages(&at1, pmd, 0);
+	__ac_setup_specific_pages(&at1, 0, 0);
 
 	__ac_test_init(&at2, ptr2, pt_env, &at1);
 	at2.flags = at1.flags | AC_PDE_WRITABLE_MASK | AC_PTE_DIRTY_MASK | AC_ACCESS_WRITE_MASK;
-	__ac_setup_specific_pages(&at2, pmd, 0);
+	__ac_setup_specific_pages(&at2, 0, 0);
 
 	__ac_test_init(&at3, ptr3, pt_env, &at1);
+	/* Override the PMD (1-based index) to point at ptr1's PMD. */
+	at3.page_tables[3] = at1.page_tables[3];
 	at3.flags = AC_PDPTE_NO_WRITABLE_MASK | at1.flags;
-	__ac_setup_specific_pages(&at3, pmd, 0);
+	__ac_setup_specific_pages(&at3, 0, 0);
 
+	/* Alias ptr2, only the PMD will differ; manually override the PMD. */
 	__ac_test_init(&at4, ptr4, pt_env, &at2);
+	at4.page_tables[3] = at1.page_tables[3];
 	at4.flags = AC_PDPTE_NO_WRITABLE_MASK | at2.flags;
-	__ac_setup_specific_pages(&at4, pmd, 0);
+	__ac_setup_specific_pages(&at4, 0, 0);
 
 	err_read_at1 = ac_test_do_access(&at1);
 	if (!err_read_at1) {
-- 
2.34.0.rc2.393.gf8c9666880-goog

[kvm-unit-tests PATCH 18/39] x86/access: Remove manual override of PUD/PMD in prefetch sub-test

[Sean Christopherson]

Drop the overrides from the check_pfec_on_prefetch_pte() test now that
pre-allocating page tables will naturally have them use the same PUD and
PMD entries.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/access.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/x86/access.c b/x86/access.c
index 3e1a684..956a450 100644
--- a/x86/access.c
+++ b/x86/access.c
@@ -957,10 +957,10 @@ static int check_pfec_on_prefetch_pte(ac_pt_env_t *pt_env)
 	__ac_test_init(&at2, 0xffff923406003000ul, pt_env, &at1);
 
 	at1.flags = AC_PDE_PRESENT_MASK | AC_PTE_PRESENT_MASK;
-	ac_setup_specific_pages(&at1, 30 * 1024 * 1024, 30 * 1024 * 1024);
+	ac_setup_specific_pages(&at1, 0, 0);
 
 	at2.flags = at1.flags | AC_PTE_NX_MASK;
-	ac_setup_specific_pages(&at2, 30 * 1024 * 1024, 30 * 1024 * 1024);
+	ac_setup_specific_pages(&at2, 0, 0);
 
 	if (!ac_test_do_access(&at1)) {
 		printf("%s: prepare fail\n", __FUNCTION__);
-- 
2.34.0.rc2.393.gf8c9666880-goog

[kvm-unit-tests PATCH 19/39] x86/access: Remove PMD/PT target overrides

[Sean Christopherson]

Drop the now-unused overrides from the PTE insertion helper.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/access.c | 47 +++++++++++++++--------------------------------
 1 file changed, 15 insertions(+), 32 deletions(-)

diff --git a/x86/access.c b/x86/access.c
index 956a450..b58ea3e 100644
--- a/x86/access.c
+++ b/x86/access.c
@@ -621,7 +621,7 @@ static pt_element_t ac_get_pt(ac_test_t *at, int i, pt_element_t *ptep)
 	return pte;
 }
 
-static void __ac_setup_specific_pages(ac_test_t *at, u64 pd_page, u64 pt_page)
+static void ac_test_setup_ptes(ac_test_t *at)
 {
 	unsigned long parent_pte = shadow_cr3;
 	int flags = at->flags;
@@ -641,21 +641,14 @@ static void __ac_setup_specific_pages(ac_test_t *at, u64 pd_page, u64 pt_page)
 			pte |= PT_PRESENT_MASK | PT_WRITABLE_MASK | PT_USER_MASK;
 			break;
 		case 3:
-			if (pd_page)
-				pte = pd_page;
-			else
-				pte = ac_get_pt(at, i, ptep);
-
+			pte = ac_get_pt(at, i, ptep);
 			pte |= PT_PRESENT_MASK | PT_USER_MASK;
 			if (!F(AC_PDPTE_NO_WRITABLE))
 				pte |= PT_WRITABLE_MASK;
 			break;
 		case 2:
 			if (!F(AC_PDE_PSE)) {
-				if (pt_page)
-					pte = pt_page;
-				else
-					pte = ac_get_pt(at, i, ptep);
+				pte = ac_get_pt(at, i, ptep);
 
 				/* The protection key is ignored on non-leaf entries.  */
 				if (F(AC_PKU_PKEY))
@@ -720,16 +713,6 @@ static void __ac_setup_specific_pages(ac_test_t *at, u64 pd_page, u64 pt_page)
 	ac_set_expected_status(at);
 }
 
-static void ac_test_setup_pte(ac_test_t *at)
-{
-	__ac_setup_specific_pages(at, 0, 0);
-}
-
-static void ac_setup_specific_pages(ac_test_t *at, u64 pd_page, u64 pt_page)
-{
-	return __ac_setup_specific_pages(at, pd_page, pt_page);
-}
-
 static void __dump_pte(pt_element_t *ptep, int level, unsigned long virt)
 {
 	printf("------L%d I%lu: %lx\n", level, PT_INDEX(virt, level), *ptep);
@@ -919,12 +902,12 @@ static int corrupt_hugepage_triger(ac_pt_env_t *pt_env)
 	__ac_test_init(&at2, 0xffffe66600000000ul, pt_env, &at1);
 
 	at2.flags = AC_CPU_CR0_WP_MASK | AC_PDE_PSE_MASK | AC_PDE_PRESENT_MASK;
-	ac_test_setup_pte(&at2);
+	ac_test_setup_ptes(&at2);
 	if (!ac_test_do_access(&at2))
 		goto err;
 
 	at1.flags = at2.flags | AC_PDE_WRITABLE_MASK;
-	ac_test_setup_pte(&at1);
+	ac_test_setup_ptes(&at1);
 	if (!ac_test_do_access(&at1))
 		goto err;
 
@@ -957,10 +940,10 @@ static int check_pfec_on_prefetch_pte(ac_pt_env_t *pt_env)
 	__ac_test_init(&at2, 0xffff923406003000ul, pt_env, &at1);
 
 	at1.flags = AC_PDE_PRESENT_MASK | AC_PTE_PRESENT_MASK;
-	ac_setup_specific_pages(&at1, 0, 0);
+	ac_test_setup_ptes(&at1);
 
 	at2.flags = at1.flags | AC_PTE_NX_MASK;
-	ac_setup_specific_pages(&at2, 0, 0);
+	ac_test_setup_ptes(&at2);
 
 	if (!ac_test_do_access(&at1)) {
 		printf("%s: prepare fail\n", __FUNCTION__);
@@ -1002,14 +985,14 @@ static int check_large_pte_dirty_for_nowp(ac_pt_env_t *pt_env)
 	__ac_test_init(&at2, 0xffffe66606000000ul, pt_env, &at1);
 
 	at2.flags = AC_PDE_PRESENT_MASK | AC_PDE_PSE_MASK;
-	ac_test_setup_pte(&at2);
+	ac_test_setup_ptes(&at2);
 	if (!ac_test_do_access(&at2)) {
 		printf("%s: read on the first mapping fail.\n", __FUNCTION__);
 		goto err;
 	}
 
 	at1.flags = at2.flags | AC_ACCESS_WRITE_MASK;
-	ac_test_setup_pte(&at1);
+	ac_test_setup_ptes(&at1);
 	if (!ac_test_do_access(&at1)) {
 		printf("%s: write on the second mapping fail.\n", __FUNCTION__);
 		goto err;
@@ -1045,7 +1028,7 @@ static int check_smep_andnot_wp(ac_pt_env_t *pt_env)
 		    AC_CPU_CR4_SMEP_MASK |
 		    AC_CPU_CR0_WP_MASK |
 		    AC_ACCESS_WRITE_MASK;
-	ac_test_setup_pte(&at1);
+	ac_test_setup_ptes(&at1);
 
 	/*
 	 * Here we write the ro user page when
@@ -1103,23 +1086,23 @@ static int check_effective_sp_permissions(ac_pt_env_t *pt_env)
 		    AC_PDE_USER_MASK | AC_PTE_USER_MASK |
 		    AC_PDE_ACCESSED_MASK | AC_PTE_ACCESSED_MASK |
 		    AC_PTE_WRITABLE_MASK | AC_ACCESS_USER_MASK;
-	__ac_setup_specific_pages(&at1, 0, 0);
+	ac_test_setup_ptes(&at1);
 
 	__ac_test_init(&at2, ptr2, pt_env, &at1);
 	at2.flags = at1.flags | AC_PDE_WRITABLE_MASK | AC_PTE_DIRTY_MASK | AC_ACCESS_WRITE_MASK;
-	__ac_setup_specific_pages(&at2, 0, 0);
+	ac_test_setup_ptes(&at2);
 
 	__ac_test_init(&at3, ptr3, pt_env, &at1);
 	/* Override the PMD (1-based index) to point at ptr1's PMD. */
 	at3.page_tables[3] = at1.page_tables[3];
 	at3.flags = AC_PDPTE_NO_WRITABLE_MASK | at1.flags;
-	__ac_setup_specific_pages(&at3, 0, 0);
+	ac_test_setup_ptes(&at3);
 
 	/* Alias ptr2, only the PMD will differ; manually override the PMD. */
 	__ac_test_init(&at4, ptr4, pt_env, &at2);
 	at4.page_tables[3] = at1.page_tables[3];
 	at4.flags = AC_PDPTE_NO_WRITABLE_MASK | at2.flags;
-	__ac_setup_specific_pages(&at4, 0, 0);
+	ac_test_setup_ptes(&at4);
 
 	err_read_at1 = ac_test_do_access(&at1);
 	if (!err_read_at1) {
@@ -1155,7 +1138,7 @@ static int ac_test_exec(ac_test_t *at, ac_pt_env_t *pt_env)
 	if (verbose) {
 		ac_test_show(at);
 	}
-	ac_test_setup_pte(at);
+	ac_test_setup_ptes(at);
 	r = ac_test_do_access(at);
 	return r;
 }
-- 
2.34.0.rc2.393.gf8c9666880-goog

[kvm-unit-tests PATCH 20/39] x86/access: Remove timeout overrides now that performance doesn't suck

[Sean Christopherson]

The access test now takes less than 5 seconds when TDP is enabled, and
is well under the default 90 second timeout when TDP is disabled.  Ditto
for VMX's #PF interception variant, which is no longer being penalized by
unnecessary CR exits and other general stupidity.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/unittests.cfg | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index 4402287..f3f9f17 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -117,13 +117,11 @@ extra_params = -cpu qemu64,+x2apic,+tsc-deadline -append tscdeadline_immed
 file = access_test.flat
 arch = x86_64
 extra_params = -cpu max
-timeout = 180
 
 [access-reduced-maxphyaddr]
 file = access_test.flat
 arch = x86_64
 extra_params = -cpu IvyBridge,phys-bits=36,host-phys-bits=off
-timeout = 180
 check = /sys/module/kvm_intel/parameters/allow_smaller_maxphyaddr=Y
 
 [smap]
@@ -358,7 +356,6 @@ file = vmx.flat
 extra_params = -cpu max,+vmx -append vmx_pf_exception_test
 arch = x86_64
 groups = vmx nested_exception
-timeout = 300
 
 [vmx_pf_exception_test_reduced_maxphyaddr]
 file = vmx.flat
@@ -366,7 +363,6 @@ extra_params = -cpu IvyBridge,phys-bits=36,host-phys-bits=off,+vmx -append vmx_p
 arch = x86_64
 groups = vmx nested_exception
 check = /sys/module/kvm_intel/parameters/allow_smaller_maxphyaddr=Y
-timeout = 300
 
 [debug]
 file = debug.flat
-- 
2.34.0.rc2.393.gf8c9666880-goog

[kvm-unit-tests PATCH 21/39] nVMX: Skip EPT tests if INVEPT(SINGLE_CONTEXT) is unsupported

[Sean Christopherson]

EPT can technically be supported without INVEPT(SINGLE_CONTEXT), skip the
EPT tests if SINGLE_CONTEXT isn't supported as it's heavily used (without
the result being checked, yay).

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/vmx.h       | 8 ++++++++
 x86/vmx_tests.c | 5 +++++
 2 files changed, 13 insertions(+)

diff --git a/x86/vmx.h b/x86/vmx.h
index dd869c2..472b28a 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -725,6 +725,14 @@ extern union vmx_ctrl_msr ctrl_exit_rev;
 extern union vmx_ctrl_msr ctrl_enter_rev;
 extern union vmx_ept_vpid  ept_vpid;
 
+static inline bool is_invept_type_supported(u64 type)
+{
+	if (type < INVEPT_SINGLE || type > INVEPT_GLOBAL)
+		return false;
+
+	return ept_vpid.val & (EPT_CAP_INVEPT_SINGLE << (type - INVEPT_SINGLE));
+}
+
 extern u64 *bsp_vmxon_region;
 extern bool launched;
 
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 97fa8ce..cbf22e3 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -1148,8 +1148,13 @@ static int ept_init_common(bool have_ad)
 	int ret;
 	struct pci_dev pcidev;
 
+	/* INVEPT is required by the EPT violation handler. */
+	if (!is_invept_type_supported(INVEPT_SINGLE))
+		return VMX_TEST_EXIT;
+
 	if (setup_ept(have_ad))
 		return VMX_TEST_EXIT;
+
 	data_page1 = alloc_page();
 	data_page2 = alloc_page();
 	*((u32 *)data_page1) = MAGIC_VAL_1;
-- 
2.34.0.rc2.393.gf8c9666880-goog

[kvm-unit-tests PATCH 22/39] nVMX: Hoist assert macros to the top of vmx.h

[Sean Christopherson]

Move VMX's assert macros to the top of vmx.h so that they can be used in
inlined helpers.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/vmx.h | 110 +++++++++++++++++++++++++++---------------------------
 1 file changed, 55 insertions(+), 55 deletions(-)

diff --git a/x86/vmx.h b/x86/vmx.h
index 472b28a..c1a8f6a 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -7,6 +7,61 @@
 #include "asm/page.h"
 #include "asm/io.h"
 
+void __abort_test(void);
+
+#define TEST_ASSERT(cond)					\
+do {								\
+	if (!(cond)) {						\
+		report_fail("%s:%d: Assertion failed: %s",	\
+			    __FILE__, __LINE__, #cond);		\
+		dump_stack();					\
+		__abort_test();					\
+	}							\
+	report_passed();					\
+} while (0)
+
+#define TEST_ASSERT_MSG(cond, fmt, args...)			\
+do {								\
+	if (!(cond)) {						\
+		report_fail("%s:%d: Assertion failed: %s\n" fmt,\
+			    __FILE__, __LINE__, #cond, ##args);	\
+		dump_stack();					\
+		__abort_test();					\
+	}							\
+	report_passed();					\
+} while (0)
+
+#define __TEST_EQ(a, b, a_str, b_str, assertion, fmt, args...)	\
+do {								\
+	typeof(a) _a = a;					\
+	typeof(b) _b = b;					\
+	if (_a != _b) {						\
+		char _bin_a[BINSTR_SZ];				\
+		char _bin_b[BINSTR_SZ];				\
+		binstr(_a, _bin_a);				\
+		binstr(_b, _bin_b);				\
+		report_fail("%s:%d: %s failed: (%s) == (%s)\n"	\
+			    "\tLHS: %#018lx - %s - %lu\n"	\
+			    "\tRHS: %#018lx - %s - %lu%s" fmt,	\
+			    __FILE__, __LINE__,			\
+			    assertion ? "Assertion" : "Expectation", a_str, b_str,	\
+			    (unsigned long) _a, _bin_a, (unsigned long) _a,		\
+			    (unsigned long) _b, _bin_b, (unsigned long) _b,		\
+			    fmt[0] == '\0' ? "" : "\n", ## args);			\
+		dump_stack();					\
+		if (assertion)					\
+			__abort_test();				\
+	}							\
+	report_passed();					\
+} while (0)
+
+#define TEST_ASSERT_EQ(a, b) __TEST_EQ(a, b, #a, #b, 1, "")
+#define TEST_ASSERT_EQ_MSG(a, b, fmt, args...) \
+	__TEST_EQ(a, b, #a, #b, 1, fmt, ## args)
+#define TEST_EXPECT_EQ(a, b) __TEST_EQ(a, b, #a, #b, 0, "")
+#define TEST_EXPECT_EQ_MSG(a, b, fmt, args...) \
+	__TEST_EQ(a, b, #a, #b, 0, fmt, ## args)
+
 struct vmcs_hdr {
 	u32 revision_id:31;
 	u32 shadow_vmcs:1;
@@ -926,59 +981,4 @@ void test_set_guest(test_guest_func func);
 void test_add_teardown(test_teardown_func func, void *data);
 void test_skip(const char *msg);
 
-void __abort_test(void);
-
-#define TEST_ASSERT(cond) \
-do { \
-	if (!(cond)) { \
-		report_fail("%s:%d: Assertion failed: %s", \
-			    __FILE__, __LINE__, #cond); \
-		dump_stack(); \
-		__abort_test(); \
-	} \
-	report_passed(); \
-} while (0)
-
-#define TEST_ASSERT_MSG(cond, fmt, args...) \
-do { \
-	if (!(cond)) { \
-		report_fail("%s:%d: Assertion failed: %s\n" fmt, \
-			    __FILE__, __LINE__, #cond, ##args); \
-		dump_stack(); \
-		__abort_test(); \
-	} \
-	report_passed(); \
-} while (0)
-
-#define __TEST_EQ(a, b, a_str, b_str, assertion, fmt, args...) \
-do { \
-	typeof(a) _a = a; \
-	typeof(b) _b = b; \
-	if (_a != _b) { \
-		char _bin_a[BINSTR_SZ]; \
-		char _bin_b[BINSTR_SZ]; \
-		binstr(_a, _bin_a); \
-		binstr(_b, _bin_b); \
-		report_fail("%s:%d: %s failed: (%s) == (%s)\n" \
-			    "\tLHS: %#018lx - %s - %lu\n" \
-			    "\tRHS: %#018lx - %s - %lu%s" fmt, \
-			    __FILE__, __LINE__, \
-			    assertion ? "Assertion" : "Expectation", a_str, b_str, \
-			    (unsigned long) _a, _bin_a, (unsigned long) _a, \
-			    (unsigned long) _b, _bin_b, (unsigned long) _b, \
-			    fmt[0] == '\0' ? "" : "\n", ## args); \
-		dump_stack(); \
-		if (assertion) \
-			__abort_test(); \
-	} \
-	report_passed(); \
-} while (0)
-
-#define TEST_ASSERT_EQ(a, b) __TEST_EQ(a, b, #a, #b, 1, "")
-#define TEST_ASSERT_EQ_MSG(a, b, fmt, args...) \
-	__TEST_EQ(a, b, #a, #b, 1, fmt, ## args)
-#define TEST_EXPECT_EQ(a, b) __TEST_EQ(a, b, #a, #b, 0, "")
-#define TEST_EXPECT_EQ_MSG(a, b, fmt, args...) \
-	__TEST_EQ(a, b, #a, #b, 0, fmt, ## args)
-
 #endif
-- 
2.34.0.rc2.393.gf8c9666880-goog

[kvm-unit-tests PATCH 23/39] nVMX: Add a non-reporting assertion macro

[Sean Christopherson]

Add a version of VMX's TEST_ASSERT that doesn't report.  The output of
basic assertions is annoying, and other than inflating the number of
tests to make KUT look good, there's no value in reporting that KUT is
working as intended.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/vmx.h | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/x86/vmx.h b/x86/vmx.h
index c1a8f6a..47b0461 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -9,7 +9,7 @@
 
 void __abort_test(void);
 
-#define TEST_ASSERT(cond)					\
+#define __TEST_ASSERT(cond)					\
 do {								\
 	if (!(cond)) {						\
 		report_fail("%s:%d: Assertion failed: %s",	\
@@ -17,6 +17,11 @@ do {								\
 		dump_stack();					\
 		__abort_test();					\
 	}							\
+} while (0)
+
+#define TEST_ASSERT(cond)					\
+do {								\
+	__TEST_ASSERT(cond);					\
 	report_passed();					\
 } while (0)
 
-- 
2.34.0.rc2.393.gf8c9666880-goog

[kvm-unit-tests PATCH 24/39] nVMX: Assert success in unchecked INVEPT/INVVPID helpers

[Sean Christopherson]

Assert if INVEPT or INVVPID fails instead of silently ignoring potential
problems and hoping they'll show up later.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/vmx.h       | 26 ++++++++++++++++++--------
 x86/vmx_tests.c |  6 +++---
 2 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/x86/vmx.h b/x86/vmx.h
index 47b0461..28e28f1 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -912,28 +912,38 @@ static inline int vmcs_save(struct vmcs **vmcs)
 	return ret;
 }
 
-static inline bool invept(unsigned long type, u64 eptp)
+static inline int __invept(unsigned long type, u64 eptp)
 {
-	bool ret;
+	bool failed = false;
 	u64 rflags = read_rflags() | X86_EFLAGS_CF | X86_EFLAGS_ZF;
 
 	struct {
 		u64 eptp, gpa;
 	} operand = {eptp, 0};
 	asm volatile("push %1; popf; invept %2, %3; setbe %0"
-		     : "=q" (ret) : "r" (rflags), "m"(operand),"r"(type) : "cc");
-	return ret;
+		     : "=q" (failed) : "r" (rflags), "m"(operand),"r"(type) : "cc");
+	return failed ? -1: 0;
 }
 
-static inline bool invvpid(unsigned long type, u64 vpid, u64 gla)
+static inline void invept(unsigned long type, u64 eptp)
 {
-	bool ret;
+	__TEST_ASSERT(!__invept(type, eptp));
+}
+
+static inline int __invvpid(unsigned long type, u64 vpid, u64 gla)
+{
+	bool failed = false;
 	u64 rflags = read_rflags() | X86_EFLAGS_CF | X86_EFLAGS_ZF;
 
 	struct invvpid_operand operand = {vpid, gla};
 	asm volatile("push %1; popf; invvpid %2, %3; setbe %0"
-		     : "=q" (ret) : "r" (rflags), "m"(operand),"r"(type) : "cc");
-	return ret;
+		     : "=q" (failed) : "r" (rflags), "m"(operand),"r"(type) : "cc");
+	return failed ? -1: 0;
+}
+
+static inline void invvpid(unsigned long type, u64 vpid, u64 gla)
+{
+	__TEST_ASSERT(!__invvpid(type, vpid, gla));
 }
 
 void enable_vmx(void);
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index cbf22e3..0df69ee 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -1248,7 +1248,7 @@ static bool invept_test(int type, u64 eptp)
 	bool ret, supported;
 
 	supported = ept_vpid.val & (EPT_CAP_INVEPT_SINGLE >> INVEPT_SINGLE << type);
-	ret = invept(type, eptp);
+	ret = __invept(type, eptp);
 
 	if (ret == !supported)
 		return false;
@@ -1551,7 +1551,7 @@ static bool invvpid_test(int type, u16 vpid)
 
 	supported = ept_vpid.val &
 		(VPID_CAP_INVVPID_ADDR >> INVVPID_ADDR << type);
-	ret = invvpid(type, vpid, 0);
+	ret = __invvpid(type, vpid, 0);
 
 	if (ret == !supported)
 		return false;
@@ -3280,7 +3280,7 @@ static void try_invvpid(u64 type, u64 vpid, u64 gla)
 	 * that we can tell if it is updated by INVVPID.
 	 */
 	vmcs_read(~0);
-	rc = invvpid(type, vpid, gla);
+	rc = __invvpid(type, vpid, gla);
 	report(!rc == valid, "INVVPID type %ld VPID %lx GLA %lx %s", type,
 	       vpid, gla,
 	       valid ? "passes" : "fails");
-- 
2.34.0.rc2.393.gf8c9666880-goog

[kvm-unit-tests PATCH 25/39] nVMX: Drop less-than-useless ept_sync() wrapper

[Sean Christopherson]

Drop ept_sync(), it's nothing more than a wrapper to invept() with
open-coded "assertions" that the desired flavor of INVEPT is supported.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/vmx.c       | 21 ---------------------
 x86/vmx.h       |  1 -
 x86/vmx_tests.c | 26 +++++++++++++-------------
 3 files changed, 13 insertions(+), 35 deletions(-)

diff --git a/x86/vmx.c b/x86/vmx.c
index 7a2f7a3..554cc74 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -1144,27 +1144,6 @@ void check_ept_ad(unsigned long *pml4, u64 guest_cr3,
 	       !!(expected_gpa_ad & EPT_DIRTY_FLAG));
 }
 
-
-void ept_sync(int type, u64 eptp)
-{
-	switch (type) {
-	case INVEPT_SINGLE:
-		if (ept_vpid.val & EPT_CAP_INVEPT_SINGLE) {
-			invept(INVEPT_SINGLE, eptp);
-			break;
-		}
-		/* else fall through */
-	case INVEPT_GLOBAL:
-		if (ept_vpid.val & EPT_CAP_INVEPT_ALL) {
-			invept(INVEPT_GLOBAL, eptp);
-			break;
-		}
-		/* else fall through */
-	default:
-		printf("WARNING: invept is not supported!\n");
-	}
-}
-
 void set_ept_pte(unsigned long *pml4, unsigned long guest_addr,
 		 int level, u64 pte_val)
 {
diff --git a/x86/vmx.h b/x86/vmx.h
index 28e28f1..0212ca6 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -953,7 +953,6 @@ int init_vmcs(struct vmcs **vmcs);
 const char *exit_reason_description(u64 reason);
 void print_vmexit_info(union exit_reason exit_reason);
 void print_vmentry_failure_info(struct vmentry_result *result);
-void ept_sync(int type, u64 eptp);
 void vpid_sync(int type, u16 vpid);
 void install_ept_entry(unsigned long *pml4, int pte_level,
 		unsigned long guest_addr, unsigned long pte,
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 0df69ee..78a53e1 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -1335,7 +1335,7 @@ static int ept_exit_handler_common(union exit_reason exit_reason, bool have_ad)
 			clear_ept_ad(pml4, guest_cr3, (unsigned long)data_page1);
 			clear_ept_ad(pml4, guest_cr3, (unsigned long)data_page2);
 			if (have_ad)
-				ept_sync(INVEPT_SINGLE, eptp);;
+				invept(INVEPT_SINGLE, eptp);
 			if (*((u32 *)data_page1) == MAGIC_VAL_3 &&
 					*((u32 *)data_page2) == MAGIC_VAL_2) {
 				vmx_inc_test_stage();
@@ -1348,14 +1348,14 @@ static int ept_exit_handler_common(union exit_reason exit_reason, bool have_ad)
 		case 1:
 			install_ept(pml4, (unsigned long)data_page1,
  				(unsigned long)data_page1, EPT_WA);
-			ept_sync(INVEPT_SINGLE, eptp);
+			invept(INVEPT_SINGLE, eptp);
 			break;
 		case 2:
 			install_ept(pml4, (unsigned long)data_page1,
  				(unsigned long)data_page1,
  				EPT_RA | EPT_WA | EPT_EA |
  				(2 << EPT_MEM_TYPE_SHIFT));
-			ept_sync(INVEPT_SINGLE, eptp);
+			invept(INVEPT_SINGLE, eptp);
 			break;
 		case 3:
 			clear_ept_ad(pml4, guest_cr3, (unsigned long)data_page1);
@@ -1363,7 +1363,7 @@ static int ept_exit_handler_common(union exit_reason exit_reason, bool have_ad)
 						1, &data_page1_pte));
 			set_ept_pte(pml4, (unsigned long)data_page1, 
 				1, data_page1_pte & ~EPT_PRESENT);
-			ept_sync(INVEPT_SINGLE, eptp);
+			invept(INVEPT_SINGLE, eptp);
 			break;
 		case 4:
 			ptep = get_pte_level((pgd_t *)guest_cr3, data_page1, /*level=*/2);
@@ -1372,12 +1372,12 @@ static int ept_exit_handler_common(union exit_reason exit_reason, bool have_ad)
 			TEST_ASSERT(get_ept_pte(pml4, guest_pte_addr, 2, &data_page1_pte_pte));
 			set_ept_pte(pml4, guest_pte_addr, 2,
 				data_page1_pte_pte & ~EPT_PRESENT);
-			ept_sync(INVEPT_SINGLE, eptp);
+			invept(INVEPT_SINGLE, eptp);
 			break;
 		case 5:
 			install_ept(pml4, (unsigned long)pci_physaddr,
 				(unsigned long)pci_physaddr, 0);
-			ept_sync(INVEPT_SINGLE, eptp);
+			invept(INVEPT_SINGLE, eptp);
 			break;
 		case 7:
 			if (!invept_test(0, eptp))
@@ -1400,7 +1400,7 @@ static int ept_exit_handler_common(union exit_reason exit_reason, bool have_ad)
 			install_ept(pml4, (unsigned long)data_page1,
  				(unsigned long)data_page1,
  				EPT_RA | EPT_WA | EPT_EA);
-			ept_sync(INVEPT_SINGLE, eptp);
+			invept(INVEPT_SINGLE, eptp);
 			break;
 		// Should not reach here
 		default:
@@ -1428,7 +1428,7 @@ static int ept_exit_handler_common(union exit_reason exit_reason, bool have_ad)
 				vmx_inc_test_stage();
 			set_ept_pte(pml4, (unsigned long)data_page1,
 				1, data_page1_pte | (EPT_PRESENT));
-			ept_sync(INVEPT_SINGLE, eptp);
+			invept(INVEPT_SINGLE, eptp);
 			break;
 		case 4:
 			check_ept_ad(pml4, guest_cr3, (unsigned long)data_page1, 0,
@@ -1440,7 +1440,7 @@ static int ept_exit_handler_common(union exit_reason exit_reason, bool have_ad)
 				vmx_inc_test_stage();
 			set_ept_pte(pml4, guest_pte_addr, 2,
 				data_page1_pte_pte | (EPT_PRESENT));
-			ept_sync(INVEPT_SINGLE, eptp);
+			invept(INVEPT_SINGLE, eptp);
 			break;
 		case 5:
 			if (exit_qual & EPT_VLT_RD)
@@ -1448,7 +1448,7 @@ static int ept_exit_handler_common(union exit_reason exit_reason, bool have_ad)
 			TEST_ASSERT(get_ept_pte(pml4, (unsigned long)pci_physaddr,
 						1, &memaddr_pte));
 			set_ept_pte(pml4, memaddr_pte, 1, memaddr_pte | EPT_RA);
-			ept_sync(INVEPT_SINGLE, eptp);
+			invept(INVEPT_SINGLE, eptp);
 			break;
 		case 6:
 			if (exit_qual & EPT_VLT_WR)
@@ -1456,7 +1456,7 @@ static int ept_exit_handler_common(union exit_reason exit_reason, bool have_ad)
 			TEST_ASSERT(get_ept_pte(pml4, (unsigned long)pci_physaddr,
 						1, &memaddr_pte));
 			set_ept_pte(pml4, memaddr_pte, 1, memaddr_pte | EPT_RA | EPT_WA);
-			ept_sync(INVEPT_SINGLE, eptp);
+			invept(INVEPT_SINGLE, eptp);
 			break;
 		default:
 			// Should not reach here
@@ -2483,7 +2483,7 @@ static unsigned long ept_twiddle(unsigned long gpa, bool mkhuge, int level,
 		pte = orig_pte;
 	pte = (pte & ~clear) | set;
 	set_ept_pte(pml4, gpa, level, pte);
-	ept_sync(INVEPT_SINGLE, eptp);
+	invept(INVEPT_SINGLE, eptp);
 
 	return orig_pte;
 }
@@ -2491,7 +2491,7 @@ static unsigned long ept_twiddle(unsigned long gpa, bool mkhuge, int level,
 static void ept_untwiddle(unsigned long gpa, int level, unsigned long orig_pte)
 {
 	set_ept_pte(pml4, gpa, level, orig_pte);
-	ept_sync(INVEPT_SINGLE, eptp);
+	invept(INVEPT_SINGLE, eptp);
 }
 
 static void do_ept_violation(bool leaf, enum ept_access_op op,
-- 
2.34.0.rc2.393.gf8c9666880-goog

[kvm-unit-tests PATCH 26/39] nVMX: Move EPT capability check helpers to vmx.h

[Sean Christopherson]

Move the EPT capability helpers to vmx.h, ept_vpid is available and
there's no reason to hide the trivial implementations.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/vmx.c | 30 ------------------------------
 x86/vmx.h | 36 ++++++++++++++++++++++++++++++------
 2 files changed, 30 insertions(+), 36 deletions(-)

diff --git a/x86/vmx.c b/x86/vmx.c
index 554cc74..eb5417b 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -1164,36 +1164,6 @@ void set_ept_pte(unsigned long *pml4, unsigned long guest_addr,
 	pt[offset] = pte_val;
 }
 
-bool ept_2m_supported(void)
-{
-	return ept_vpid.val & EPT_CAP_2M_PAGE;
-}
-
-bool ept_1g_supported(void)
-{
-	return ept_vpid.val & EPT_CAP_1G_PAGE;
-}
-
-bool ept_huge_pages_supported(int level)
-{
-	if (level == 2)
-		return ept_2m_supported();
-	else if (level == 3)
-		return ept_1g_supported();
-	else
-		return false;
-}
-
-bool ept_execute_only_supported(void)
-{
-	return ept_vpid.val & EPT_CAP_WT;
-}
-
-bool ept_ad_bits_supported(void)
-{
-	return ept_vpid.val & EPT_CAP_AD_FLAG;
-}
-
 void vpid_sync(int type, u16 vpid)
 {
 	switch(type) {
diff --git a/x86/vmx.h b/x86/vmx.h
index 0212ca6..0b7fb20 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -785,6 +785,36 @@ extern union vmx_ctrl_msr ctrl_exit_rev;
 extern union vmx_ctrl_msr ctrl_enter_rev;
 extern union vmx_ept_vpid  ept_vpid;
 
+static inline bool ept_2m_supported(void)
+{
+	return ept_vpid.val & EPT_CAP_2M_PAGE;
+}
+
+static inline bool ept_1g_supported(void)
+{
+	return ept_vpid.val & EPT_CAP_1G_PAGE;
+}
+
+static inline bool ept_huge_pages_supported(int level)
+{
+	if (level == 2)
+		return ept_2m_supported();
+	else if (level == 3)
+		return ept_1g_supported();
+	else
+		return false;
+}
+
+static inline bool ept_execute_only_supported(void)
+{
+	return ept_vpid.val & EPT_CAP_WT;
+}
+
+static inline bool ept_ad_bits_supported(void)
+{
+	return ept_vpid.val & EPT_CAP_AD_FLAG;
+}
+
 static inline bool is_invept_type_supported(u64 type)
 {
 	if (type < INVEPT_SINGLE || type > INVEPT_GLOBAL)
@@ -975,12 +1005,6 @@ void check_ept_ad(unsigned long *pml4, u64 guest_cr3,
 void clear_ept_ad(unsigned long *pml4, u64 guest_cr3,
 		  unsigned long guest_addr);
 
-bool ept_2m_supported(void);
-bool ept_1g_supported(void);
-bool ept_huge_pages_supported(int level);
-bool ept_execute_only_supported(void);
-bool ept_ad_bits_supported(void);
-
 #define        ABORT_ON_EARLY_VMENTRY_FAIL     0x1
 #define        ABORT_ON_INVALID_GUEST_STATE    0x2
 
-- 
2.34.0.rc2.393.gf8c9666880-goog

[kvm-unit-tests PATCH 27/39] nVMX: Drop unused and useless vpid_sync() helper

[Sean Christopherson]

Drop vpid_sync(), it's unused for good reason.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/vmx.c | 18 ------------------
 x86/vmx.h |  1 -
 2 files changed, 19 deletions(-)

diff --git a/x86/vmx.c b/x86/vmx.c
index eb5417b..e499704 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -1164,24 +1164,6 @@ void set_ept_pte(unsigned long *pml4, unsigned long guest_addr,
 	pt[offset] = pte_val;
 }
 
-void vpid_sync(int type, u16 vpid)
-{
-	switch(type) {
-	case INVVPID_CONTEXT_GLOBAL:
-		if (ept_vpid.val & VPID_CAP_INVVPID_CXTGLB) {
-			invvpid(INVVPID_CONTEXT_GLOBAL, vpid, 0);
-			break;
-		}
-	case INVVPID_ALL:
-		if (ept_vpid.val & VPID_CAP_INVVPID_ALL) {
-			invvpid(INVVPID_ALL, vpid, 0);
-			break;
-		}
-	default:
-		printf("WARNING: invvpid is not supported\n");
-	}
-}
-
 static void init_vmcs_ctrl(void)
 {
 	/* 26.2 CHECKS ON VMX CONTROLS AND HOST-STATE AREA */
diff --git a/x86/vmx.h b/x86/vmx.h
index 0b7fb20..4936120 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -983,7 +983,6 @@ int init_vmcs(struct vmcs **vmcs);
 const char *exit_reason_description(u64 reason);
 void print_vmexit_info(union exit_reason exit_reason);
 void print_vmentry_failure_info(struct vmentry_result *result);
-void vpid_sync(int type, u16 vpid);
 void install_ept_entry(unsigned long *pml4, int pte_level,
 		unsigned long guest_addr, unsigned long pte,
 		unsigned long *pt_page);
-- 
2.34.0.rc2.393.gf8c9666880-goog

[kvm-unit-tests PATCH 28/39] nVMX: Remove "v1" version of INVVPID test

[Sean Christopherson]

Yank out the old INVVPID and drop the version info from the new test,
which is a complete superset.  That, and the old test was apparently
trying to win an obfuscated C contest.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/vmx_tests.c | 91 ++-----------------------------------------------
 1 file changed, 2 insertions(+), 89 deletions(-)

diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 78a53e1..507e485 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -1545,92 +1545,6 @@ static int eptad_exit_handler(union exit_reason exit_reason)
 	return ept_exit_handler_common(exit_reason, true);
 }
 
-static bool invvpid_test(int type, u16 vpid)
-{
-	bool ret, supported;
-
-	supported = ept_vpid.val &
-		(VPID_CAP_INVVPID_ADDR >> INVVPID_ADDR << type);
-	ret = __invvpid(type, vpid, 0);
-
-	if (ret == !supported)
-		return false;
-
-	if (!supported)
-		printf("WARNING: unsupported invvpid passed!\n");
-	else
-		printf("WARNING: invvpid failed!\n");
-
-	return true;
-}
-
-static int vpid_init(struct vmcs *vmcs)
-{
-	u32 ctrl_cpu1;
-
-	if (!(ctrl_cpu_rev[0].clr & CPU_SECONDARY) ||
-		!(ctrl_cpu_rev[1].clr & CPU_VPID)) {
-		printf("\tVPID is not supported");
-		return VMX_TEST_EXIT;
-	}
-
-	ctrl_cpu1 = vmcs_read(CPU_EXEC_CTRL1);
-	ctrl_cpu1 |= CPU_VPID;
-	vmcs_write(CPU_EXEC_CTRL1, ctrl_cpu1);
-	return VMX_TEST_START;
-}
-
-static void vpid_main(void)
-{
-	vmx_set_test_stage(0);
-	vmcall();
-	report(vmx_get_test_stage() == 1, "INVVPID SINGLE ADDRESS");
-	vmx_set_test_stage(2);
-	vmcall();
-	report(vmx_get_test_stage() == 3, "INVVPID SINGLE");
-	vmx_set_test_stage(4);
-	vmcall();
-	report(vmx_get_test_stage() == 5, "INVVPID ALL");
-}
-
-static int vpid_exit_handler(union exit_reason exit_reason)
-{
-	u64 guest_rip;
-	u32 insn_len;
-
-	guest_rip = vmcs_read(GUEST_RIP);
-	insn_len = vmcs_read(EXI_INST_LEN);
-
-	switch (exit_reason.basic) {
-	case VMX_VMCALL:
-		switch(vmx_get_test_stage()) {
-		case 0:
-			if (!invvpid_test(INVVPID_ADDR, 1))
-				vmx_inc_test_stage();
-			break;
-		case 2:
-			if (!invvpid_test(INVVPID_CONTEXT_GLOBAL, 1))
-				vmx_inc_test_stage();
-			break;
-		case 4:
-			if (!invvpid_test(INVVPID_ALL, 1))
-				vmx_inc_test_stage();
-			break;
-		default:
-			report_fail("ERROR: unexpected stage, %d",
-					vmx_get_test_stage());
-			print_vmexit_info(exit_reason);
-			return VMX_TEST_VMEXIT;
-		}
-		vmcs_write(GUEST_RIP, guest_rip + insn_len);
-		return VMX_TEST_RESUME;
-	default:
-		report_fail("Unknown exit reason, 0x%x", exit_reason.full);
-		print_vmexit_info(exit_reason);
-	}
-	return VMX_TEST_VMEXIT;
-}
-
 #define TIMER_VECTOR	222
 
 static volatile bool timer_fired;
@@ -3391,7 +3305,7 @@ static void invvpid_test_not_in_vmx_operation(void)
  * This does not test real-address mode, virtual-8086 mode, protected mode,
  * or CPL > 0.
  */
-static void invvpid_test_v2(void)
+static void invvpid_test(void)
 {
 	u64 msr;
 	int i;
@@ -10770,7 +10684,6 @@ struct vmx_test vmx_tests[] = {
 	{ "EPT A/D disabled", ept_init, ept_main, ept_exit_handler, NULL, {0} },
 	{ "EPT A/D enabled", eptad_init, eptad_main, eptad_exit_handler, NULL, {0} },
 	{ "PML", pml_init, pml_main, pml_exit_handler, NULL, {0} },
-	{ "VPID", vpid_init, vpid_main, vpid_exit_handler, NULL, {0} },
 	{ "interrupt", interrupt_init, interrupt_main,
 		interrupt_exit_handler, NULL, {0} },
 	{ "nmi_hlt", nmi_hlt_init, nmi_hlt_main,
@@ -10794,7 +10707,7 @@ struct vmx_test vmx_tests[] = {
 	TEST(fixture_test_case1),
 	TEST(fixture_test_case2),
 	/* Opcode tests. */
-	TEST(invvpid_test_v2),
+	TEST(invvpid_test),
 	/* VM-entry tests */
 	TEST(vmx_controls_test),
 	TEST(vmx_host_state_area_test),
-- 
2.34.0.rc2.393.gf8c9666880-goog

Re: [kvm-unit-tests PATCH 28/39] nVMX: Remove "v1" version of INVVPID test

[Paolo Bonzini]

On 11/25/21 02:28, Sean Christopherson wrote:
> Yank out the old INVVPID and drop the version info from the new test,
> which is a complete superset.  That, and the old test was apparently
> trying to win an obfuscated C contest.

All of the "version 1" tests are...

Paolo

> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>   x86/vmx_tests.c | 91 ++-----------------------------------------------
>   1 file changed, 2 insertions(+), 89 deletions(-)
> 
> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> index 78a53e1..507e485 100644
> --- a/x86/vmx_tests.c
> +++ b/x86/vmx_tests.c
> @@ -1545,92 +1545,6 @@ static int eptad_exit_handler(union exit_reason exit_reason)
>   	return ept_exit_handler_common(exit_reason, true);
>   }
>   
> -static bool invvpid_test(int type, u16 vpid)
> -{
> -	bool ret, supported;
> -
> -	supported = ept_vpid.val &
> -		(VPID_CAP_INVVPID_ADDR >> INVVPID_ADDR << type);
> -	ret = __invvpid(type, vpid, 0);
> -
> -	if (ret == !supported)
> -		return false;
> -
> -	if (!supported)
> -		printf("WARNING: unsupported invvpid passed!\n");
> -	else
> -		printf("WARNING: invvpid failed!\n");
> -
> -	return true;
> -}
> -
> -static int vpid_init(struct vmcs *vmcs)
> -{
> -	u32 ctrl_cpu1;
> -
> -	if (!(ctrl_cpu_rev[0].clr & CPU_SECONDARY) ||
> -		!(ctrl_cpu_rev[1].clr & CPU_VPID)) {
> -		printf("\tVPID is not supported");
> -		return VMX_TEST_EXIT;
> -	}
> -
> -	ctrl_cpu1 = vmcs_read(CPU_EXEC_CTRL1);
> -	ctrl_cpu1 |= CPU_VPID;
> -	vmcs_write(CPU_EXEC_CTRL1, ctrl_cpu1);
> -	return VMX_TEST_START;
> -}
> -
> -static void vpid_main(void)
> -{
> -	vmx_set_test_stage(0);
> -	vmcall();
> -	report(vmx_get_test_stage() == 1, "INVVPID SINGLE ADDRESS");
> -	vmx_set_test_stage(2);
> -	vmcall();
> -	report(vmx_get_test_stage() == 3, "INVVPID SINGLE");
> -	vmx_set_test_stage(4);
> -	vmcall();
> -	report(vmx_get_test_stage() == 5, "INVVPID ALL");
> -}
> -
> -static int vpid_exit_handler(union exit_reason exit_reason)
> -{
> -	u64 guest_rip;
> -	u32 insn_len;
> -
> -	guest_rip = vmcs_read(GUEST_RIP);
> -	insn_len = vmcs_read(EXI_INST_LEN);
> -
> -	switch (exit_reason.basic) {
> -	case VMX_VMCALL:
> -		switch(vmx_get_test_stage()) {
> -		case 0:
> -			if (!invvpid_test(INVVPID_ADDR, 1))
> -				vmx_inc_test_stage();
> -			break;
> -		case 2:
> -			if (!invvpid_test(INVVPID_CONTEXT_GLOBAL, 1))
> -				vmx_inc_test_stage();
> -			break;
> -		case 4:
> -			if (!invvpid_test(INVVPID_ALL, 1))
> -				vmx_inc_test_stage();
> -			break;
> -		default:
> -			report_fail("ERROR: unexpected stage, %d",
> -					vmx_get_test_stage());
> -			print_vmexit_info(exit_reason);
> -			return VMX_TEST_VMEXIT;
> -		}
> -		vmcs_write(GUEST_RIP, guest_rip + insn_len);
> -		return VMX_TEST_RESUME;
> -	default:
> -		report_fail("Unknown exit reason, 0x%x", exit_reason.full);
> -		print_vmexit_info(exit_reason);
> -	}
> -	return VMX_TEST_VMEXIT;
> -}
> -
>   #define TIMER_VECTOR	222
>   
>   static volatile bool timer_fired;
> @@ -3391,7 +3305,7 @@ static void invvpid_test_not_in_vmx_operation(void)
>    * This does not test real-address mode, virtual-8086 mode, protected mode,
>    * or CPL > 0.
>    */
> -static void invvpid_test_v2(void)
> +static void invvpid_test(void)
>   {
>   	u64 msr;
>   	int i;
> @@ -10770,7 +10684,6 @@ struct vmx_test vmx_tests[] = {
>   	{ "EPT A/D disabled", ept_init, ept_main, ept_exit_handler, NULL, {0} },
>   	{ "EPT A/D enabled", eptad_init, eptad_main, eptad_exit_handler, NULL, {0} },
>   	{ "PML", pml_init, pml_main, pml_exit_handler, NULL, {0} },
> -	{ "VPID", vpid_init, vpid_main, vpid_exit_handler, NULL, {0} },
>   	{ "interrupt", interrupt_init, interrupt_main,
>   		interrupt_exit_handler, NULL, {0} },
>   	{ "nmi_hlt", nmi_hlt_init, nmi_hlt_main,
> @@ -10794,7 +10707,7 @@ struct vmx_test vmx_tests[] = {
>   	TEST(fixture_test_case1),
>   	TEST(fixture_test_case2),
>   	/* Opcode tests. */
> -	TEST(invvpid_test_v2),
> +	TEST(invvpid_test),
>   	/* VM-entry tests */
>   	TEST(vmx_controls_test),
>   	TEST(vmx_host_state_area_test),
> 

[kvm-unit-tests PATCH 29/39] nVMX: Add helper to check if INVVPID type is supported

[Sean Christopherson]

Add a helper to deduplicate code, now and in the future, and to avoid a
RDMSR every time a VPID test wants to do a basic functionality check.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/vmx.h       |  8 ++++++++
 x86/vmx_tests.c | 17 +++++------------
 2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/x86/vmx.h b/x86/vmx.h
index 4936120..289f175 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -823,6 +823,14 @@ static inline bool is_invept_type_supported(u64 type)
 	return ept_vpid.val & (EPT_CAP_INVEPT_SINGLE << (type - INVEPT_SINGLE));
 }
 
+static inline bool is_invvpid_type_supported(unsigned long type)
+{
+	if (type < INVVPID_ADDR || type > INVVPID_CONTEXT_LOCAL)
+		return false;
+
+	return ept_vpid.val & (VPID_CAP_INVVPID_ADDR << (type - INVVPID_ADDR));
+}
+
 extern u64 *bsp_vmxon_region;
 extern bool launched;
 
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 507e485..950f527 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -3161,14 +3161,7 @@ static void ept_access_test_force_2m_page(void)
 
 static bool invvpid_valid(u64 type, u64 vpid, u64 gla)
 {
-	u64 msr = rdmsr(MSR_IA32_VMX_EPT_VPID_CAP);
-
-	TEST_ASSERT(msr & VPID_CAP_INVVPID);
-
-	if (type < INVVPID_ADDR || type > INVVPID_CONTEXT_LOCAL)
-		return false;
-
-	if (!(msr & (1ull << (type + VPID_CAP_INVVPID_TYPES_SHIFT))))
+	if (!is_invvpid_type_supported(type))
 		return false;
 
 	if (vpid >> 16)
@@ -3321,13 +3314,13 @@ static void invvpid_test(void)
 	if (!(msr & VPID_CAP_INVVPID))
 		test_skip("INVVPID not supported.\n");
 
-	if (msr & VPID_CAP_INVVPID_ADDR)
+	if (is_invvpid_type_supported(INVVPID_ADDR))
 		types |= 1u << INVVPID_ADDR;
-	if (msr & VPID_CAP_INVVPID_CXTGLB)
+	if (is_invvpid_type_supported(INVVPID_CONTEXT_GLOBAL))
 		types |= 1u << INVVPID_CONTEXT_GLOBAL;
-	if (msr & VPID_CAP_INVVPID_ALL)
+	if (is_invvpid_type_supported(INVVPID_ALL))
 		types |= 1u << INVVPID_ALL;
-	if (msr & VPID_CAP_INVVPID_CXTLOC)
+	if (is_invvpid_type_supported(INVVPID_CONTEXT_LOCAL))
 		types |= 1u << INVVPID_CONTEXT_LOCAL;
 
 	if (!types)
-- 
2.34.0.rc2.393.gf8c9666880-goog

[kvm-unit-tests PATCH 30/39] nVMX: Add helper to check if INVVPID is supported

[Sean Christopherson]

Add a helper to check for basic INVVPID, it will gain more users in the
future.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/vmx.h       | 5 +++++
 x86/vmx_tests.c | 5 +----
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/x86/vmx.h b/x86/vmx.h
index 289f175..9f91602 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -823,6 +823,11 @@ static inline bool is_invept_type_supported(u64 type)
 	return ept_vpid.val & (EPT_CAP_INVEPT_SINGLE << (type - INVEPT_SINGLE));
 }
 
+static inline bool is_invvpid_supported(void)
+{
+	return ept_vpid.val & VPID_CAP_INVVPID;
+}
+
 static inline bool is_invvpid_type_supported(unsigned long type)
 {
 	if (type < INVVPID_ADDR || type > INVVPID_CONTEXT_LOCAL)
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 950f527..66f374a 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -3300,7 +3300,6 @@ static void invvpid_test_not_in_vmx_operation(void)
  */
 static void invvpid_test(void)
 {
-	u64 msr;
 	int i;
 	unsigned types = 0;
 	unsigned type;
@@ -3309,9 +3308,7 @@ static void invvpid_test(void)
 	    !(ctrl_cpu_rev[1].clr & CPU_VPID))
 		test_skip("VPID not supported");
 
-	msr = rdmsr(MSR_IA32_VMX_EPT_VPID_CAP);
-
-	if (!(msr & VPID_CAP_INVVPID))
+	if (!is_invvpid_supported())
 		test_skip("INVVPID not supported.\n");
 
 	if (is_invvpid_type_supported(INVVPID_ADDR))
-- 
2.34.0.rc2.393.gf8c9666880-goog

[kvm-unit-tests PATCH 31/39] nVMX: Add helper to get first supported INVVPID type

[Sean Christopherson]

Deduplicate some clever/interesting code for retrieving the first
supported INVVPID type, and opportunistically avoid RDMSR on every test.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/vmx_tests.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 66f374a..f2e24f6 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -3197,16 +3197,20 @@ static void try_invvpid(u64 type, u64 vpid, u64 gla)
 	       expected, vmcs_read(VMX_INST_ERROR));
 }
 
+static inline unsigned long get_first_supported_invvpid_type(void)
+{
+	u64 type = ffs(ept_vpid.val >> VPID_CAP_INVVPID_TYPES_SHIFT) - 1;
+
+	__TEST_ASSERT(type >= INVVPID_ADDR && type <= INVVPID_CONTEXT_LOCAL);
+	return type;
+}
+
 static void ds_invvpid(void *data)
 {
-	u64 msr = rdmsr(MSR_IA32_VMX_EPT_VPID_CAP);
-	u64 type = ffs(msr >> VPID_CAP_INVVPID_TYPES_SHIFT) - 1;
-
-	TEST_ASSERT(type >= INVVPID_ADDR && type <= INVVPID_CONTEXT_LOCAL);
 	asm volatile("invvpid %0, %1"
 		     :
 		     : "m"(*(struct invvpid_operand *)data),
-		       "r"(type));
+		       "r"(get_first_supported_invvpid_type()));
 }
 
 /*
@@ -3216,13 +3220,9 @@ static void ds_invvpid(void *data)
  */
 static void ss_invvpid(void *data)
 {
-	u64 msr = rdmsr(MSR_IA32_VMX_EPT_VPID_CAP);
-	u64 type = ffs(msr >> VPID_CAP_INVVPID_TYPES_SHIFT) - 1;
-
-	TEST_ASSERT(type >= INVVPID_ADDR && type <= INVVPID_CONTEXT_LOCAL);
 	asm volatile("sub %%rsp,%0; invvpid (%%rsp,%0,1), %1"
 		     : "+r"(data)
-		     : "r"(type));
+		     : "r"(get_first_supported_invvpid_type()));
 }
 
 static void invvpid_test_gp(void)
-- 
2.34.0.rc2.393.gf8c9666880-goog

[kvm-unit-tests PATCH 32/39] nVMX: Use helper to check for EPT A/D support

[Sean Christopherson]

Use the existing helper to check for EPT A/D support instead of rereading
the capabilities MSR and open-coding the check.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/vmx_tests.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index f2e24f6..116ae66 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -1485,7 +1485,7 @@ static int eptad_init(struct vmcs *vmcs)
 	if (r == VMX_TEST_EXIT)
 		return r;
 
-	if ((rdmsr(MSR_IA32_VMX_EPT_VPID_CAP) & EPT_CAP_AD_FLAG) == 0) {
+	if (!ept_ad_bits_supported()) {
 		printf("\tEPT A/D bits are not supported");
 		return VMX_TEST_EXIT;
 	}
@@ -4805,7 +4805,7 @@ static void test_ept_eptp(void)
 	/*
 	 * Accessed and dirty flag (bit 6)
 	 */
-	if (msr & EPT_CAP_AD_FLAG) {
+	if (ept_ad_bits_supported()) {
 		report_info("Processor supports accessed and dirty flag");
 		eptp &= ~EPTP_AD_FLAG;
 		test_eptp_ad_bit(eptp, true);
-- 
2.34.0.rc2.393.gf8c9666880-goog

[kvm-unit-tests PATCH 33/39] nVMX: Add helpers to check for 4/5-level EPT support

[Sean Christopherson]

Add helpers to check for 4-level and 5-level EPT support.  Yet another
baby step toward removing unnecessary RDMSRs...

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/vmx.h       | 10 ++++++++++
 x86/vmx_tests.c |  4 ++--
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/x86/vmx.h b/x86/vmx.h
index 9f91602..e6126e4 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -815,6 +815,16 @@ static inline bool ept_ad_bits_supported(void)
 	return ept_vpid.val & EPT_CAP_AD_FLAG;
 }
 
+static inline bool is_4_level_ept_supported(void)
+{
+	return ept_vpid.val & EPT_CAP_PWL4;
+}
+
+static inline bool is_5_level_ept_supported(void)
+{
+	return ept_vpid.val & EPT_CAP_PWL5;
+}
+
 static inline bool is_invept_type_supported(u64 type)
 {
 	if (type < INVEPT_SINGLE || type > INVEPT_GLOBAL)
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 116ae66..2bfc794 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -4735,7 +4735,7 @@ static void test_ept_eptp(void)
 		wr_bk = true;
 
 	/* Support for 4-level EPT is mandatory. */
-	report(msr & EPT_CAP_PWL4, "4-level EPT support check");
+	report(is_4_level_ept_supported(), "4-level EPT support check");
 
 	primary |= CPU_SECONDARY;
 	vmcs_write(CPU_EXEC_CTRL0, primary);
@@ -4784,7 +4784,7 @@ static void test_ept_eptp(void)
 	for (i = 0; i < 8; i++) {
 		eptp = (eptp & ~EPTP_PG_WALK_LEN_MASK) |
 		    (i << EPTP_PG_WALK_LEN_SHIFT);
-		if (i == 3 || (i == 4 && (msr & EPT_CAP_PWL5)))
+		if (i == 3 || (i == 4 && is_5_level_ept_supported()))
 			ctrl = true;
 		else
 			ctrl = false;
-- 
2.34.0.rc2.393.gf8c9666880-goog

[kvm-unit-tests PATCH 34/39] nVMX: Fix name of macro defining EPT execute only capability

[Sean Christopherson]

Rename EPT_CAP_WT to EPT_CAP_EXEC_ONLY.  In x86, "WT" generally refers to
write-through memtype, and is especially confusing considering that EPT
capabilities also report UC and WB memtypes.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/vmx.c | 2 +-
 x86/vmx.h | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/x86/vmx.c b/x86/vmx.c
index e499704..6dc9a55 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -1606,7 +1606,7 @@ static void test_vmx_caps(void)
 	       "MSR_IA32_VMX_VMCS_ENUM");
 
 	fixed0 = -1ull;
-	fixed0 &= ~(EPT_CAP_WT |
+	fixed0 &= ~(EPT_CAP_EXEC_ONLY |
 		    EPT_CAP_PWL4 |
 		    EPT_CAP_PWL5 |
 		    EPT_CAP_UC |
diff --git a/x86/vmx.h b/x86/vmx.h
index e6126e4..d3e95f5 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -697,7 +697,7 @@ enum vm_entry_failure_code {
 #define EPT_IGNORE_PAT		(1ul << 6)
 #define EPT_SUPPRESS_VE		(1ull << 63)
 
-#define EPT_CAP_WT		1ull
+#define EPT_CAP_EXEC_ONLY	(1ull << 0)
 #define EPT_CAP_PWL4		(1ull << 6)
 #define EPT_CAP_PWL5		(1ull << 7)
 #define EPT_CAP_UC		(1ull << 8)
@@ -807,7 +807,7 @@ static inline bool ept_huge_pages_supported(int level)
 
 static inline bool ept_execute_only_supported(void)
 {
-	return ept_vpid.val & EPT_CAP_WT;
+	return ept_vpid.val & EPT_CAP_EXEC_ONLY;
 }
 
 static inline bool ept_ad_bits_supported(void)
-- 
2.34.0.rc2.393.gf8c9666880-goog

Re: [kvm-unit-tests PATCH 34/39] nVMX: Fix name of macro defining EPT execute only capability

[Paolo Bonzini]

On 11/25/21 02:28, Sean Christopherson wrote:
> Rename EPT_CAP_WT to EPT_CAP_EXEC_ONLY.  In x86, "WT" generally refers to
> write-through memtype, and is especially confusing considering that EPT
> capabilities also report UC and WB memtypes.
> 
> Signed-off-by: Sean Christopherson<seanjc@google.com>

Yeah, I am not sure why it was "EPT_CAP_WT".  That one should have been 
for bit 12 (if it was useful at all).

Paolo

[kvm-unit-tests PATCH 35/39] nVMX: Add helper to check if a memtype is supported for EPT structures

[Sean Christopherson]

Add a helper to check if a given memtype can be used for EPT structures,
and use the helper to clean up the EPT test code.  An informational
message is lost along the way, but that's not necessarily a bad thing.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/vmx.h       | 11 +++++++++++
 x86/vmx_tests.c | 33 ++-------------------------------
 2 files changed, 13 insertions(+), 31 deletions(-)

diff --git a/x86/vmx.h b/x86/vmx.h
index d3e95f5..401715c 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -825,6 +825,17 @@ static inline bool is_5_level_ept_supported(void)
 	return ept_vpid.val & EPT_CAP_PWL5;
 }
 
+static inline bool is_ept_memtype_supported(int type)
+{
+	if (type == EPT_MEM_TYPE_UC)
+		return ept_vpid.val & EPT_CAP_UC;
+
+	if (type == EPT_MEM_TYPE_WB)
+		return ept_vpid.val & EPT_CAP_WB;
+
+	return false;
+}
+
 static inline bool is_invept_type_supported(u64 type)
 {
 	if (type < INVEPT_SINGLE || type > INVEPT_GLOBAL)
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 2bfc794..27150cb 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -4712,9 +4712,7 @@ static void test_ept_eptp(void)
 	u64 eptp_saved = vmcs_read(EPTP);
 	u32 primary = primary_saved;
 	u32 secondary = secondary_saved;
-	u64 msr, eptp = eptp_saved;
-	bool un_cache = false;
-	bool wr_bk = false;
+	u64 eptp = eptp_saved;
 	bool ctrl;
 	u32 i, maxphysaddr;
 	u64 j, resv_bits_mask = 0;
@@ -4725,15 +4723,6 @@ static void test_ept_eptp(void)
 		return;
 	}
 
-	/*
-	 * Memory type (bits 2:0)
-	 */
-	msr = rdmsr(MSR_IA32_VMX_EPT_VPID_CAP);
-	if (msr & EPT_CAP_UC)
-		un_cache = true;
-	if (msr & EPT_CAP_WB)
-		wr_bk = true;
-
 	/* Support for 4-level EPT is mandatory. */
 	report(is_4_level_ept_supported(), "4-level EPT support check");
 
@@ -4746,29 +4735,11 @@ static void test_ept_eptp(void)
 	vmcs_write(EPTP, eptp);
 
 	for (i = 0; i < 8; i++) {
-		if (i == 0) {
-			if (un_cache) {
-				report_info("EPT paging structure memory-type is Un-cacheable\n");
-				ctrl = true;
-			} else {
-				ctrl = false;
-			}
-		} else if (i == 6) {
-			if (wr_bk) {
-				report_info("EPT paging structure memory-type is Write-back\n");
-				ctrl = true;
-			} else {
-				ctrl = false;
-			}
-		} else {
-			ctrl = false;
-		}
-
 		eptp = (eptp & ~EPT_MEM_TYPE_MASK) | i;
 		vmcs_write(EPTP, eptp);
 		report_prefix_pushf("Enable-EPT enabled; EPT memory type %lu",
 		    eptp & EPT_MEM_TYPE_MASK);
-		if (ctrl)
+		if (is_ept_memtype_supported(i))
 			test_vmx_valid_controls();
 		else
 			test_vmx_invalid_controls();
-- 
2.34.0.rc2.393.gf8c9666880-goog

[kvm-unit-tests PATCH 36/39] nVMX: Get rid of horribly named "ctrl" boolean in test_ept_eptp()

[Sean Christopherson]

Eliminate a now-pointless and horribly name boolean in the EPT test.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/vmx_tests.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 27150cb..bdf541b 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -4713,7 +4713,6 @@ static void test_ept_eptp(void)
 	u32 primary = primary_saved;
 	u32 secondary = secondary_saved;
 	u64 eptp = eptp_saved;
-	bool ctrl;
 	u32 i, maxphysaddr;
 	u64 j, resv_bits_mask = 0;
 
@@ -4755,15 +4754,11 @@ static void test_ept_eptp(void)
 	for (i = 0; i < 8; i++) {
 		eptp = (eptp & ~EPTP_PG_WALK_LEN_MASK) |
 		    (i << EPTP_PG_WALK_LEN_SHIFT);
-		if (i == 3 || (i == 4 && is_5_level_ept_supported()))
-			ctrl = true;
-		else
-			ctrl = false;
 
 		vmcs_write(EPTP, eptp);
 		report_prefix_pushf("Enable-EPT enabled; EPT page walk length %lu",
 		    eptp & EPTP_PG_WALK_LEN_MASK);
-		if (ctrl)
+		if (i == 3 || (i == 4 && is_5_level_ept_supported()))
 			test_vmx_valid_controls();
 		else
 			test_vmx_invalid_controls();
-- 
2.34.0.rc2.393.gf8c9666880-goog

[kvm-unit-tests PATCH 37/39] nVMX: Rename awful "ctrl" booleans to "is_ctrl_valid"

[Sean Christopherson]

Rename ctrl to is_ctrl_valid in several tests.  The variables are bools
that, *** drum roll ***, track if a control setting is valid.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/vmx_tests.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index bdf541b..316105b 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -3819,7 +3819,7 @@ static void test_apic_virtual_ctls(void)
 	u32 saved_secondary = vmcs_read(CPU_EXEC_CTRL1);
 	u32 primary = saved_primary;
 	u32 secondary = saved_secondary;
-	bool ctrl = false;
+	bool is_ctrl_valid = false;
 	char str[10] = "disabled";
 	u8 i = 0, j;
 
@@ -3838,18 +3838,18 @@ static void test_apic_virtual_ctls(void)
 		for (j = 1; j < 8; j++) {
 			secondary &= ~(CPU_VIRT_X2APIC | CPU_APIC_REG_VIRT | CPU_VINTD);
 			if (primary & CPU_TPR_SHADOW) {
-				ctrl = true;
+				is_ctrl_valid = true;
 			} else {
 				if (! set_bit_pattern(j, &secondary))
-					ctrl = true;
+					is_ctrl_valid = true;
 				else
-					ctrl = false;
+					is_ctrl_valid = false;
 			}
 
 			vmcs_write(CPU_EXEC_CTRL1, secondary);
 			report_prefix_pushf("Use TPR shadow %s, virtualize x2APIC mode %s, APIC-register virtualization %s, virtual-interrupt delivery %s",
 				str, (secondary & CPU_VIRT_X2APIC) ? "enabled" : "disabled", (secondary & CPU_APIC_REG_VIRT) ? "enabled" : "disabled", (secondary & CPU_VINTD) ? "enabled" : "disabled");
-			if (ctrl)
+			if (is_ctrl_valid)
 				test_vmx_valid_controls();
 			else
 				test_vmx_invalid_controls();
@@ -3946,11 +3946,11 @@ static void test_virtual_intr_ctls(void)
 	vmcs_write(PIN_CONTROLS, saved_pin);
 }
 
-static void test_pi_desc_addr(u64 addr, bool ctrl)
+static void test_pi_desc_addr(u64 addr, bool is_ctrl_valid)
 {
 	vmcs_write(POSTED_INTR_DESC_ADDR, addr);
 	report_prefix_pushf("Process-posted-interrupts enabled; posted-interrupt-descriptor-address 0x%lx", addr);
-	if (ctrl)
+	if (is_ctrl_valid)
 		test_vmx_valid_controls();
 	else
 		test_vmx_invalid_controls();
@@ -4674,12 +4674,12 @@ done:
 	vmcs_write(PIN_CONTROLS, pin_ctrls);
 }
 
-static void test_eptp_ad_bit(u64 eptp, bool ctrl)
+static void test_eptp_ad_bit(u64 eptp, bool is_ctrl_valid)
 {
 	vmcs_write(EPTP, eptp);
 	report_prefix_pushf("Enable-EPT enabled; EPT accessed and dirty flag %s",
 	    (eptp & EPTP_AD_FLAG) ? "1": "0");
-	if (ctrl)
+	if (is_ctrl_valid)
 		test_vmx_valid_controls();
 	else
 		test_vmx_invalid_controls();
-- 
2.34.0.rc2.393.gf8c9666880-goog

[kvm-unit-tests PATCH 38/39] nVMX: Add helper to check if VPID is supported

[Sean Christopherson]

Add a helper to check for VPID support to deduplicate code, now and in
the future.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/vmx.h       | 6 ++++++
 x86/vmx_tests.c | 6 ++----
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/x86/vmx.h b/x86/vmx.h
index 401715c..4423986 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -844,6 +844,12 @@ static inline bool is_invept_type_supported(u64 type)
 	return ept_vpid.val & (EPT_CAP_INVEPT_SINGLE << (type - INVEPT_SINGLE));
 }
 
+static inline bool is_vpid_supported(void)
+{
+	return (ctrl_cpu_rev[0].clr & CPU_SECONDARY) &&
+	       (ctrl_cpu_rev[1].clr & CPU_VPID);
+}
+
 static inline bool is_invvpid_supported(void)
 {
 	return ept_vpid.val & VPID_CAP_INVVPID;
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 316105b..172d385 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -3304,8 +3304,7 @@ static void invvpid_test(void)
 	unsigned types = 0;
 	unsigned type;
 
-	if (!(ctrl_cpu_rev[0].clr & CPU_SECONDARY) ||
-	    !(ctrl_cpu_rev[1].clr & CPU_VPID))
+	if (!is_vpid_supported())
 		test_skip("VPID not supported");
 
 	if (!is_invvpid_supported())
@@ -4099,8 +4098,7 @@ static void test_vpid(void)
 	u16 vpid = 0x0000;
 	int i;
 
-	if (!((ctrl_cpu_rev[0].clr & CPU_SECONDARY) &&
-	    (ctrl_cpu_rev[1].clr & CPU_VPID))) {
+	if (!is_vpid_supported()) {
 		printf("Secondary controls and/or VPID not supported\n");
 		return;
 	}
-- 
2.34.0.rc2.393.gf8c9666880-goog

[kvm-unit-tests PATCH 39/39] x86/access: nVMX: Add "access" test variants to invalidate via (INV)VPID

[Sean Christopherson]

Add three variants of the #PF interception access test to handle TLB
invalidations by relying on VPID rules.  Intercept the access test's
INVLPG and perform invalidation by:

  1. Implicity flush on VM-Enter by disabling VPID
  2. Explicitly perform INVVPID on the target address
  3. Implicitly "flush" by moving to a new VPID

Case #3 exposes a bug where KVM fails to update unsync SPTEs when using
shadow paging and L1 changes the VPID it uses for L2, i.e. vmcs12->vpid.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/unittests.cfg |  6 ++--
 x86/vmx_tests.c   | 89 ++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 91 insertions(+), 4 deletions(-)

diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index f3f9f17..80875d2 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -284,7 +284,7 @@ arch = i386
 
 [vmx]
 file = vmx.flat
-extra_params = -cpu max,+vmx -append "-exit_monitor_from_l2_test -ept_access* -vmx_smp* -vmx_vmcs_shadow_test -atomic_switch_overflow_msrs_test -vmx_init_signal_test -vmx_apic_passthrough_tpr_threshold_test -apic_reg_virt_test -virt_x2apic_mode_test -vmx_pf_exception_test"
+extra_params = -cpu max,+vmx -append "-exit_monitor_from_l2_test -ept_access* -vmx_smp* -vmx_vmcs_shadow_test -atomic_switch_overflow_msrs_test -vmx_init_signal_test -vmx_apic_passthrough_tpr_threshold_test -apic_reg_virt_test -virt_x2apic_mode_test -vmx_pf_exception_test -vmx_pf_no_vpid_test -vmx_pf_vpid_test"
 arch = x86_64
 groups = vmx
 
@@ -353,13 +353,13 @@ groups = vmx
 
 [vmx_pf_exception_test]
 file = vmx.flat
-extra_params = -cpu max,+vmx -append vmx_pf_exception_test
+extra_params = -cpu max,+vmx -append "vmx_pf_exception_test vmx_pf_no_vpid_test vmx_pf_vpid_test vmx_pf_invvpid_test"
 arch = x86_64
 groups = vmx nested_exception
 
 [vmx_pf_exception_test_reduced_maxphyaddr]
 file = vmx.flat
-extra_params = -cpu IvyBridge,phys-bits=36,host-phys-bits=off,+vmx -append vmx_pf_exception_test
+extra_params = -cpu IvyBridge,phys-bits=36,host-phys-bits=off,+vmx -append "vmx_pf_exception_test vmx_pf_no_vpid_test vmx_pf_vpid_test vmx_pf_invvpid_test"
 arch = x86_64
 groups = vmx nested_exception
 check = /sys/module/kvm_intel/parameters/allow_smaller_maxphyaddr=Y
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 172d385..3d57ed6 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -10575,13 +10575,21 @@ static void vmx_pf_exception_test_guest(void)
 	ac_test_run(PT_LEVEL_PML4);
 }
 
-static void vmx_pf_exception_test(void)
+typedef void (*invalidate_tlb_t)(void *data);
+
+static void __vmx_pf_exception_test(invalidate_tlb_t inv_fn, void *data)
 {
 	u64 efer;
 	struct cpuid cpuid;
 
 	test_set_guest(vmx_pf_exception_test_guest);
 
+	/* Intercept INVLPG when to perform TLB invalidation from L1 (this). */
+	if (inv_fn)
+		vmcs_set_bits(CPU_EXEC_CTRL0, CPU_INVLPG);
+	else
+		vmcs_clear_bits(CPU_EXEC_CTRL0, CPU_INVLPG);
+
 	enter_guest();
 
 	while (vmcs_read(EXI_REASON) != VMX_VMCALL) {
@@ -10605,6 +10613,9 @@ static void vmx_pf_exception_test(void)
 			regs.rcx = cpuid.c;
 			regs.rdx = cpuid.d;
 			break;
+		case VMX_INVLPG:
+			inv_fn(data);
+			break;
 		default:
 			assert_msg(false,
 				"Unexpected exit to L1, exit_reason: %s (0x%lx)",
@@ -10617,6 +10628,79 @@ static void vmx_pf_exception_test(void)
 
 	assert_exit_reason(VMX_VMCALL);
 }
+
+static void vmx_pf_exception_test(void)
+{
+	__vmx_pf_exception_test(NULL, NULL);
+}
+
+static void invalidate_tlb_no_vpid(void *data)
+{
+	/* If VPID is disabled, the TLB is flushed on VM-Enter and VM-Exit. */
+}
+
+static void vmx_pf_no_vpid_test(void)
+{
+	if (is_vpid_supported())
+		vmcs_clear_bits(CPU_EXEC_CTRL1, CPU_VPID);
+
+	__vmx_pf_exception_test(invalidate_tlb_no_vpid, NULL);
+}
+
+static void invalidate_tlb_invvpid_addr(void *data)
+{
+	invvpid(INVVPID_ALL, *(u16 *)data, vmcs_read(EXI_QUALIFICATION));
+}
+
+static void invalidate_tlb_new_vpid(void *data)
+{
+	u16 *vpid = data;
+
+	/*
+	 * Bump VPID to effectively flush L2's TLB from L0's perspective.
+	 * Invalidate all VPIDs when the VPID wraps to zero as hardware/KVM is
+	 * architecturally allowed to keep TLB entries indefinitely.
+	 */
+	++(*vpid);
+	if (*vpid == 0) {
+		++(*vpid);
+		invvpid(INVVPID_ALL, 0, 0);
+	}
+	vmcs_write(VPID, *vpid);
+}
+
+static void __vmx_pf_vpid_test(invalidate_tlb_t inv_fn, u16 vpid)
+{
+	if (!is_vpid_supported())
+		test_skip("VPID unsupported");
+
+	if (!is_invvpid_supported())
+		test_skip("INVVPID unsupported");
+
+	vmcs_set_bits(CPU_EXEC_CTRL0, CPU_SECONDARY);
+	vmcs_set_bits(CPU_EXEC_CTRL1, CPU_VPID);
+	vmcs_write(VPID, vpid);
+
+	__vmx_pf_exception_test(inv_fn, &vpid);
+}
+
+static void vmx_pf_invvpid_test(void)
+{
+	if (!is_invvpid_type_supported(INVVPID_ADDR))
+		test_skip("INVVPID ADDR unsupported");
+
+	__vmx_pf_vpid_test(invalidate_tlb_invvpid_addr, 0xaaaa);
+}
+
+static void vmx_pf_vpid_test(void)
+{
+	/* Need INVVPID(ALL) to flush VPIDs upon wrap/reuse. */
+	if (!is_invvpid_type_supported(INVVPID_ALL))
+		test_skip("INVVPID ALL unsupported");
+
+	__vmx_pf_vpid_test(invalidate_tlb_new_vpid, 1);
+}
+
 #define TEST(name) { #name, .v2 = name }
 
 /* name/init/guest_main/exit_handler/syscall_handler/guest_regs */
@@ -10723,5 +10807,8 @@ struct vmx_test vmx_tests[] = {
 	TEST(vmx_mtf_test),
 	TEST(vmx_mtf_pdpte_test),
 	TEST(vmx_pf_exception_test),
+	TEST(vmx_pf_no_vpid_test),
+	TEST(vmx_pf_invvpid_test),
+	TEST(vmx_pf_vpid_test),
 	{ NULL, NULL, NULL, NULL, NULL, {0} },
 };
-- 
2.34.0.rc2.393.gf8c9666880-goog

Re: [kvm-unit-tests PATCH 00/39] x86/access: nVMX: Big overhaul

[Paolo Bonzini]

On 11/25/21 02:28, Sean Christopherson wrote:
> This started out as a very simple test (patch 39/39) to expose a KVM bug
> where KVM doesn't sync a shadow MMU on a vmcs12->vpid change.  Except the
> test didn't fail.  And it turns out, completely removing INVLPG from the
> base access test doesn't fail when using shadow paging either.
> 
> The underlying problem in both cases is that the access test is flat out
> stupid when it comes to handling page tables.  Instead of allocating page
> tables once and manipulating them on each iteration, it "allocates" a new
> paging structure when necessary on every. single. iteration.  In addition
> to being incredibly inefficient (allocation also zeros the entire 4kb page,
> so the test zeros absurd amounts of memory), writing upper level PTEs on
> every iteration triggers write-protection mechanisms in KVM.  In effect,
> KVM ends up synchronizing the relevant SPTEs on every iteration, which
> again is ridiculously slow and makes it all but impossible to actually
> test that KVM handles other TLB invalidation scenarios.
> 
> Trying to solve that mess by pre-allocating the page tables exposed a
> whole pile of 5-level paging issues.  I'd say the test's 5-level support
> is held together by duct tape, but I've fixed many things with duct tape
> that are far less fragile.
> 
> The second half of this series is cleanups in the nVMX code to prepare
> for adding the (INV)VPID variants.  Not directly related to the access
> tests, but it annoyed me to no end that simply checking if INVVPID is
> supported was non-trivial.

Queued, thanks.  The new tests are pretty slow on debug kernels (about 3 
minutes each).  I'll check next week if there's any low hanging 
fruit---or anything broken.

Paolo

> Sean Christopherson (39):
>    x86/access: Add proper defines for hardcoded addresses
>    x86/access: Cache CR3 to improve performance
>    x86/access:  Use do-while loop for what is obviously a do-while loop
>    x86/access: Stop pretending the test is SMP friendly
>    x86/access: Refactor so called "page table pool" logic
>    x86/access: Stash root page table level in test environment
>    x86/access: Hoist page table allocator helpers above "init" helper
>    x86/access: Rename variables in page table walkers
>    x86/access: Abort if page table insertion hits an unexpected level
>    x86/access: Make SMEP place nice with 5-level paging
>    x86/access: Use upper half of virtual address space
>    x86/access: Print the index when dumping PTEs
>    x86/access: Pre-allocate all page tables at (sub)test init
>    x86/access: Don't write page tables if desired PTE is same as current
>      PTE
>    x86/access: Preserve A/D bits when writing paging structure entries
>    x86/access: Make toggling of PRESENT bit a "higher order" action
>    x86/access: Manually override PMD in effective permissions sub-test
>    x86/access: Remove manual override of PUD/PMD in prefetch sub-test
>    x86/access: Remove PMD/PT target overrides
>    x86/access: Remove timeout overrides now that performance doesn't suck
>    nVMX: Skip EPT tests if INVEPT(SINGLE_CONTEXT) is unsupported
>    nVMX: Hoist assert macros to the top of vmx.h
>    nVMX: Add a non-reporting assertion macro
>    nVMX: Assert success in unchecked INVEPT/INVVPID helpers
>    nVMX: Drop less-than-useless ept_sync() wrapper
>    nVMX: Move EPT capability check helpers to vmx.h
>    nVMX: Drop unused and useless vpid_sync() helper
>    nVMX: Remove "v1" version of INVVPID test
>    nVMX: Add helper to check if INVVPID type is supported
>    nVMX: Add helper to check if INVVPID is supported
>    nVMX: Add helper to get first supported INVVPID type
>    nVMX: Use helper to check for EPT A/D support
>    nVMX: Add helpers to check for 4/5-level EPT support
>    nVMX: Fix name of macro defining EPT execute only capability
>    nVMX: Add helper to check if a memtype is supported for EPT structures
>    nVMX: Get rid of horribly named "ctrl" boolean in test_ept_eptp()
>    nVMX: Rename awful "ctrl" booleans to "is_ctrl_valid"
>    nVMX: Add helper to check if VPID is supported
>    x86/access: nVMX: Add "access" test variants to invalidate via
>      (INV)VPID
> 
>   x86/access.c      | 391 ++++++++++++++++++++++++++++------------------
>   x86/unittests.cfg |  10 +-
>   x86/vmx.c         |  71 +--------
>   x86/vmx.h         | 229 ++++++++++++++++++---------
>   x86/vmx_tests.c   | 327 +++++++++++++++++---------------------
>   5 files changed, 543 insertions(+), 485 deletions(-)
> 

Re: [kvm-unit-tests PATCH 00/39] x86/access: nVMX: Big overhaul

[Sean Christopherson]

On Fri, Nov 26, 2021, Paolo Bonzini wrote:
> On 11/25/21 02:28, Sean Christopherson wrote:
> > This started out as a very simple test (patch 39/39) to expose a KVM bug
> > where KVM doesn't sync a shadow MMU on a vmcs12->vpid change.  Except the
> > test didn't fail.  And it turns out, completely removing INVLPG from the
> > base access test doesn't fail when using shadow paging either.
> > 
> > The underlying problem in both cases is that the access test is flat out
> > stupid when it comes to handling page tables.  Instead of allocating page
> > tables once and manipulating them on each iteration, it "allocates" a new
> > paging structure when necessary on every. single. iteration.  In addition
> > to being incredibly inefficient (allocation also zeros the entire 4kb page,
> > so the test zeros absurd amounts of memory), writing upper level PTEs on
> > every iteration triggers write-protection mechanisms in KVM.  In effect,
> > KVM ends up synchronizing the relevant SPTEs on every iteration, which
> > again is ridiculously slow and makes it all but impossible to actually
> > test that KVM handles other TLB invalidation scenarios.
> > 
> > Trying to solve that mess by pre-allocating the page tables exposed a
> > whole pile of 5-level paging issues.  I'd say the test's 5-level support
> > is held together by duct tape, but I've fixed many things with duct tape
> > that are far less fragile.
> > 
> > The second half of this series is cleanups in the nVMX code to prepare
> > for adding the (INV)VPID variants.  Not directly related to the access
> > tests, but it annoyed me to no end that simply checking if INVVPID is
> > supported was non-trivial.
> 
> Queued, thanks.  The new tests are pretty slow on debug kernels (about 3
> minutes each).  I'll check next week if there's any low hanging fruit---or
> anything broken.

Hrm, by "debug kernels", do you mean with KASAN, UBSAN, etc...?  I don't think I
tested those.  And with or without EPT enabled?

The tests are definitely much slower than the base access test and its nVMX variant
as they trigger exits on every INVLPG, but three minutes is a bit extreme.

Re: [kvm-unit-tests PATCH 00/39] x86/access: nVMX: Big overhaul

[Paolo Bonzini]

On 11/29/21 20:04, Sean Christopherson wrote:
>> Queued, thanks.  The new tests are pretty slow on debug kernels (about 3
>> minutes each).  I'll check next week if there's any low hanging fruit---or
>> anything broken.
> Hrm, by "debug kernels", do you mean with KASAN, UBSAN, etc...?  I don't think I
> tested those.  And with or without EPT enabled?

It's a normal Fedora rawhide kernel config.  It doesn't have 
KASAN/UBSAN/KCSAN but it has lockdep (including CONFIG_PROVE_RCU), 
CONFIG_DEBUG_VM, CONFIG_DEBUG_LIST, etc.  I will gather a profile just 
in case it's something egregious.

Paolo

> The tests are definitely much slower than the base access test and its nVMX variant
> as they trigger exits on every INVLPG, but three minutes is a bit extreme.
>