[kvm-unit-tests PATCH 00/12] nSVM: NPT improvements and cleanups

[kvm-unit-tests PATCH 00/12] nSVM: NPT improvements and cleanups

[Sean Christopherson]

The first chunk of this series (everything up to the lib/vmalloc patch)
are cleanups and bug fixes for existing nSVM tests that I collected on
my first attempt at the new NPT test.  I originally wanted to piggyback
the existing "v1" nSVM tests and implemented the fixes/cleanups, but that
approach didn't go so well because of the v1 infrastructure limitations.

The common lib/vmalloc changes are to allow arch code to pass arbitrary
data to its setup_mmu() function.  x86-64 uses the param to avoid marking
PTEs a USER so that tests can enable SMEP (#PF if supervisor mode fetches
from a USER PTE) without exploding or having to duplicate all page tables.

The "new" test targets nested NPT by running L1 and L2 with different
EFER.NX and CR4.SMEP settings to verify that KVM uses the correct MMU
settings when injecting page faults.

Sean Christopherson (12):
  nSVM: Provide expected and actual exit codes on VMRUN test failure
  nSVM: Replace open coded NX manipulation with appropriate macros
  nSVM: Reset the VMCB before every v1 test
  nSVM: Explicitly save/update/restore EFER.NX for NPT NX test
  nSVM: Remove NPT reserved bits tests (new one on the way)
  nSVM: Stop forcing EFER.NX=1 for all tests
  nSVM: Remove a superfluous modification of guest EFER.NX in NPT NX
    test
  nSVM: Clear guest's EFER.NX in NPT NX test
  lib/vmalloc: Let arch code pass a value to its setup_mmu() helper
  x86: Let tests omit PT_USER_MASK when configuring virtual memory
  x86: Add GBPAGES CPUID macro, clean up CPUID comments
  nSVM: Add test for NPT reserved bit and #NPF error code behavior

 lib/arm/mmu.c       |   2 +-
 lib/s390x/mmu.c     |   3 +-
 lib/vmalloc.c       |   9 +-
 lib/vmalloc.h       |   4 +-
 lib/x86/processor.h |  15 +--
 lib/x86/vm.c        |  15 ++-
 s390x/uv-host.c     |   2 +-
 x86/svm.c           |  10 +-
 x86/svm_tests.c     | 220 +++++++++++++++++++++++++++++++-------------
 9 files changed, 196 insertions(+), 84 deletions(-)

-- 
2.32.0.288.g62a8d224e6-goog

[kvm-unit-tests PATCH 01/12] nSVM: Provide expected and actual exit codes on VMRUN test failure

[Sean Christopherson]

Be kind to debuggers and include the expected and actual exit codes if
VMRUN doesn't yield the expected exit reason.

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

diff --git a/x86/svm_tests.c b/x86/svm_tests.c
index 2c85a30..df4c60a 100644
--- a/x86/svm_tests.c
+++ b/x86/svm_tests.c
@@ -2186,6 +2186,7 @@ static void basic_guest_main(struct svm_test *test)
 				  exit_code, test_name)			\
 {									\
 	u64 tmp, mask;							\
+	u32 r;								\
 	int i;								\
 									\
 	for (i = start; i <= end; i = i + inc) {			\
@@ -2203,8 +2204,9 @@ static void basic_guest_main(struct svm_test *test)
 		case 4:							\
 			vmcb->save.cr4 = tmp;				\
 		}							\
-		report(svm_vmrun() == exit_code, "Test CR%d " test_name "%d:%d: %lx",\
-		    cr, end, start, tmp);				\
+		r = svm_vmrun();					\
+		report(r == exit_code, "Test CR%d %s%d:%d: %lx, wanted exit 0x%x, got 0x%x",\
+		       cr, test_name, end, start, tmp, exit_code, r);	\
 	}								\
 }
 
-- 
2.32.0.288.g62a8d224e6-goog

[kvm-unit-tests PATCH 02/12] nSVM: Replace open coded NX manipulation with appropriate macros

[Sean Christopherson]

Use PT64_NX_MASK and EFER_NX to set/clear the NX bits in the NPT tests.

No functional change intended.

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

diff --git a/x86/svm_tests.c b/x86/svm_tests.c
index df4c60a..4bfde2c 100644
--- a/x86/svm_tests.c
+++ b/x86/svm_tests.c
@@ -707,16 +707,16 @@ static void npt_nx_prepare(struct svm_test *test)
     vmcb_ident(vmcb);
     pte = npt_get_pte((u64)null_test);
 
-    *pte |= (1ULL << 63);
+    *pte |= PT64_NX_MASK;
 }
 
 static bool npt_nx_check(struct svm_test *test)
 {
     u64 *pte = npt_get_pte((u64)null_test);
 
-    *pte &= ~(1ULL << 63);
+    *pte &= ~PT64_NX_MASK;
 
-    vmcb->save.efer |= (1 << 11);
+    vmcb->save.efer |= EFER_NX;
 
     return (vmcb->control.exit_code == SVM_EXIT_NPF)
            && (vmcb->control.exit_info_1 == 0x100000015ULL);
-- 
2.32.0.288.g62a8d224e6-goog

[kvm-unit-tests PATCH 03/12] nSVM: Reset the VMCB before every v1 test

[Sean Christopherson]

Refresh the VMCB before every v1 test to fix bugs where tests neglect to
initialize the VMCB and end up taking a dependency on previous tests,
e.g. looking at you mode_test and next_rip.  This will also allow tests
to modify VMCB fields without having to do their own manual save/restore.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/svm.c       |  2 ++
 x86/svm_tests.c | 13 -------------
 2 files changed, 2 insertions(+), 13 deletions(-)

diff --git a/x86/svm.c b/x86/svm.c
index 9fbc0b2..6e5872d 100644
--- a/x86/svm.c
+++ b/x86/svm.c
@@ -250,6 +250,8 @@ static void test_run(struct svm_test *test)
 	u64 vmcb_phys = virt_to_phys(vmcb);
 
 	irq_disable();
+	vmcb_ident(vmcb);
+
 	test->prepare(test);
 	guest_main = test->guest_func;
 	vmcb->save.rip = (ulong)test_thunk;
diff --git a/x86/svm_tests.c b/x86/svm_tests.c
index 4bfde2c..aa74cfe 100644
--- a/x86/svm_tests.c
+++ b/x86/svm_tests.c
@@ -667,7 +667,6 @@ static bool check_asid_zero(struct svm_test *test)
 
 static void sel_cr0_bug_prepare(struct svm_test *test)
 {
-    vmcb_ident(vmcb);
     vmcb->control.intercept |= (1ULL << INTERCEPT_SELECTIVE_CR0);
 }
 
@@ -704,7 +703,6 @@ static void npt_nx_prepare(struct svm_test *test)
 
     u64 *pte;
 
-    vmcb_ident(vmcb);
     pte = npt_get_pte((u64)null_test);
 
     *pte |= PT64_NX_MASK;
@@ -727,7 +725,6 @@ static void npt_np_prepare(struct svm_test *test)
     u64 *pte;
 
     scratch_page = alloc_page();
-    vmcb_ident(vmcb);
     pte = npt_get_pte((u64)scratch_page);
 
     *pte &= ~1ULL;
@@ -753,7 +750,6 @@ static void npt_us_prepare(struct svm_test *test)
     u64 *pte;
 
     scratch_page = alloc_page();
-    vmcb_ident(vmcb);
     pte = npt_get_pte((u64)scratch_page);
 
     *pte &= ~(1ULL << 2);
@@ -780,7 +776,6 @@ static void npt_rsvd_prepare(struct svm_test *test)
 {
     u64 *pde;
 
-    vmcb_ident(vmcb);
     pde = npt_get_pde((u64) null_test);
 
     save_pde = *pde;
@@ -802,7 +797,6 @@ static void npt_rw_prepare(struct svm_test *test)
 
     u64 *pte;
 
-    vmcb_ident(vmcb);
     pte = npt_get_pte(0x80000);
 
     *pte &= ~(1ULL << 1);
@@ -830,7 +824,6 @@ static void npt_rw_pfwalk_prepare(struct svm_test *test)
 
     u64 *pte;
 
-    vmcb_ident(vmcb);
     pte = npt_get_pte(read_cr3());
 
     *pte &= ~(1ULL << 1);
@@ -850,7 +843,6 @@ static bool npt_rw_pfwalk_check(struct svm_test *test)
 static void npt_rsvd_pfwalk_prepare(struct svm_test *test)
 {
     u64 *pdpe;
-    vmcb_ident(vmcb);
 
     pdpe = npt_get_pml4e();
     pdpe[0] |= (1ULL << 8);
@@ -867,7 +859,6 @@ static bool npt_rsvd_pfwalk_check(struct svm_test *test)
 
 static void npt_l1mmio_prepare(struct svm_test *test)
 {
-    vmcb_ident(vmcb);
 }
 
 u32 nested_apic_version1;
@@ -894,7 +885,6 @@ static void npt_rw_l1mmio_prepare(struct svm_test *test)
 
     u64 *pte;
 
-    vmcb_ident(vmcb);
     pte = npt_get_pte(0xfee00080);
 
     *pte &= ~(1ULL << 1);
@@ -1940,8 +1930,6 @@ static void init_startup_prepare(struct svm_test *test)
     struct segment_desc64 *tss_entry;
     int i;
 
-    vmcb_ident(vmcb);
-
     on_cpu(1, get_tss_entry, &tss_entry);
 
     orig_cpu_count = cpu_online_count;
@@ -1976,7 +1964,6 @@ static volatile bool init_intercept;
 static void init_intercept_prepare(struct svm_test *test)
 {
     init_intercept = false;
-    vmcb_ident(vmcb);
     vmcb->control.intercept |= (1ULL << INTERCEPT_INIT);
 }
 
-- 
2.32.0.288.g62a8d224e6-goog

[kvm-unit-tests PATCH 04/12] nSVM: Explicitly save/update/restore EFER.NX for NPT NX test

[Sean Christopherson]

Explicitly set EFER.NX in the NPT NX test instead of assuming all tests
will run with EFER.NX=1, and use the test's scratch field to save/restore
EFER.  There is no need to force EFER.NX=1 for all tests, and a future
test will verify that a #NPT occurs when EFER.NX=0 and PTE.NX=1, i.e.
wants the exact opposite.

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

diff --git a/x86/svm_tests.c b/x86/svm_tests.c
index aa74cfe..506bd75 100644
--- a/x86/svm_tests.c
+++ b/x86/svm_tests.c
@@ -700,9 +700,11 @@ static bool sel_cr0_bug_check(struct svm_test *test)
 
 static void npt_nx_prepare(struct svm_test *test)
 {
-
     u64 *pte;
 
+    test->scratch = rdmsr(MSR_EFER);
+    wrmsr(MSR_EFER, test->scratch | EFER_NX);
+
     pte = npt_get_pte((u64)null_test);
 
     *pte |= PT64_NX_MASK;
@@ -712,6 +714,8 @@ static bool npt_nx_check(struct svm_test *test)
 {
     u64 *pte = npt_get_pte((u64)null_test);
 
+    wrmsr(MSR_EFER, test->scratch);
+
     *pte &= ~PT64_NX_MASK;
 
     vmcb->save.efer |= EFER_NX;
-- 
2.32.0.288.g62a8d224e6-goog

[kvm-unit-tests PATCH 05/12] nSVM: Remove NPT reserved bits tests (new one on the way)

[Sean Christopherson]

Remove two of nSVM's NPT reserved bits test, a soon-to-be-added test will
provide a superset of their functionality, e.g. the current tests are
limited in the sense that they test a single entry and a single bit,
e.g. don't test conditionally-reserved bits.

The npt_rsvd test in particular is quite nasty as it subtly relies on
EFER.NX=1; dropping the test will allow cleaning up the EFER.NX weirdness
(it's forced for _all_ tests, presumably to get the desired PFEC.FETCH=1
for this one test).

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

diff --git a/x86/svm_tests.c b/x86/svm_tests.c
index 506bd75..96add48 100644
--- a/x86/svm_tests.c
+++ b/x86/svm_tests.c
@@ -774,28 +774,6 @@ static bool npt_us_check(struct svm_test *test)
            && (vmcb->control.exit_info_1 == 0x100000005ULL);
 }
 
-u64 save_pde;
-
-static void npt_rsvd_prepare(struct svm_test *test)
-{
-    u64 *pde;
-
-    pde = npt_get_pde((u64) null_test);
-
-    save_pde = *pde;
-    *pde = (1ULL << 19) | (1ULL << 7) | 0x27;
-}
-
-static bool npt_rsvd_check(struct svm_test *test)
-{
-    u64 *pde = npt_get_pde((u64) null_test);
-
-    *pde = save_pde;
-
-    return (vmcb->control.exit_code == SVM_EXIT_NPF)
-            && (vmcb->control.exit_info_1 == 0x10000001dULL);
-}
-
 static void npt_rw_prepare(struct svm_test *test)
 {
 
@@ -844,23 +822,6 @@ static bool npt_rw_pfwalk_check(struct svm_test *test)
 	   && (vmcb->control.exit_info_2 == read_cr3());
 }
 
-static void npt_rsvd_pfwalk_prepare(struct svm_test *test)
-{
-    u64 *pdpe;
-
-    pdpe = npt_get_pml4e();
-    pdpe[0] |= (1ULL << 8);
-}
-
-static bool npt_rsvd_pfwalk_check(struct svm_test *test)
-{
-    u64 *pdpe = npt_get_pml4e();
-    pdpe[0] &= ~(1ULL << 8);
-
-    return (vmcb->control.exit_code == SVM_EXIT_NPF)
-            && (vmcb->control.exit_info_1 == 0x20000000fULL);
-}
-
 static void npt_l1mmio_prepare(struct svm_test *test)
 {
 }
@@ -2719,15 +2680,9 @@ struct svm_test svm_tests[] = {
     { "npt_us", npt_supported, npt_us_prepare,
       default_prepare_gif_clear, npt_us_test,
       default_finished, npt_us_check },
-    { "npt_rsvd", npt_supported, npt_rsvd_prepare,
-      default_prepare_gif_clear, null_test,
-      default_finished, npt_rsvd_check },
     { "npt_rw", npt_supported, npt_rw_prepare,
       default_prepare_gif_clear, npt_rw_test,
       default_finished, npt_rw_check },
-    { "npt_rsvd_pfwalk", npt_supported, npt_rsvd_pfwalk_prepare,
-      default_prepare_gif_clear, null_test,
-      default_finished, npt_rsvd_pfwalk_check },
     { "npt_rw_pfwalk", npt_supported, npt_rw_pfwalk_prepare,
       default_prepare_gif_clear, null_test,
       default_finished, npt_rw_pfwalk_check },
-- 
2.32.0.288.g62a8d224e6-goog

Re: [kvm-unit-tests PATCH 05/12] nSVM: Remove NPT reserved bits tests (new one on the way)

[Paolo Bonzini]

On 22/06/21 23:00, Sean Christopherson wrote:
> Remove two of nSVM's NPT reserved bits test, a soon-to-be-added test will
> provide a superset of their functionality, e.g. the current tests are
> limited in the sense that they test a single entry and a single bit,
> e.g. don't test conditionally-reserved bits.
> 
> The npt_rsvd test in particular is quite nasty as it subtly relies on
> EFER.NX=1; dropping the test will allow cleaning up the EFER.NX weirdness
> (it's forced for_all_  tests, presumably to get the desired PFEC.FETCH=1
> for this one test).
> 
> Signed-off-by: Sean Christopherson<seanjc@google.com>
> ---
>   x86/svm_tests.c | 45 ---------------------------------------------
>   1 file changed, 45 deletions(-)

This exposes a KVM bug, reproducible with

	./x86/run x86/svm.flat -smp 2 -cpu max,+svm -m 4g \
		-append 'npt_rw npt_rw_pfwalk'

While running npt_rw_pfwalk, the #NPF gets an incorrect EXITINFO2
(address for the NPF location; on my machine it gets 0xbfede6f0 instead of
0xbfede000).  The same tests work with QEMU from git.

I didn't quite finish analyzing it, but my current theory is
that KVM receives a pagewalk NPF for a *different* page walk that is caused
by read-only page tables; then it finds that the page walk to 0xbfede6f0
*does fail* (after all the correct and wrong EXITINFO2 belong to the same pfn)
and therefore injects it anyway.  This theory is because the 0x6f0 offset in
the page table corresponds to the 0xde000 part of the faulting address.
Maxim will look into it while I'm away.

Paolo

Re: [kvm-unit-tests PATCH 05/12] nSVM: Remove NPT reserved bits tests (new one on the way)

[Sean Christopherson]

On Thu, Jun 24, 2021, Paolo Bonzini wrote:
> On 22/06/21 23:00, Sean Christopherson wrote:
> > Remove two of nSVM's NPT reserved bits test, a soon-to-be-added test will
> > provide a superset of their functionality, e.g. the current tests are
> > limited in the sense that they test a single entry and a single bit,
> > e.g. don't test conditionally-reserved bits.
> > 
> > The npt_rsvd test in particular is quite nasty as it subtly relies on
> > EFER.NX=1; dropping the test will allow cleaning up the EFER.NX weirdness
> > (it's forced for_all_  tests, presumably to get the desired PFEC.FETCH=1
> > for this one test).
> > 
> > Signed-off-by: Sean Christopherson<seanjc@google.com>
> > ---
> >   x86/svm_tests.c | 45 ---------------------------------------------
> >   1 file changed, 45 deletions(-)
> 
> This exposes a KVM bug, reproducible with
> 
> 	./x86/run x86/svm.flat -smp 2 -cpu max,+svm -m 4g \
> 		-append 'npt_rw npt_rw_pfwalk'

Any chance you're running against an older KVM version?  The test passes if I
run against a build with my MMU pile on top of kvm/queue, but fails on a random
older KVM.

Side topic, these tests all fail to invalidate TLB entries after modifying PTEs.
I suspect they work in part because KVM flushes and syncs on all nested SVM
transitions...

> While running npt_rw_pfwalk, the #NPF gets an incorrect EXITINFO2
> (address for the NPF location; on my machine it gets 0xbfede6f0 instead of
> 0xbfede000).  The same tests work with QEMU from git.
> 
> I didn't quite finish analyzing it, but my current theory is
> that KVM receives a pagewalk NPF for a *different* page walk that is caused
> by read-only page tables; then it finds that the page walk to 0xbfede6f0
> *does fail* (after all the correct and wrong EXITINFO2 belong to the same pfn)
> and therefore injects it anyway.  This theory is because the 0x6f0 offset in
> the page table corresponds to the 0xde000 part of the faulting address.
> Maxim will look into it while I'm away.
> 
> Paolo
> 

Re: [kvm-unit-tests PATCH 05/12] nSVM: Remove NPT reserved bits tests (new one on the way)

[Paolo Bonzini]

On 24/06/21 19:43, Sean Christopherson wrote:
>> 	./x86/run x86/svm.flat -smp 2 -cpu max,+svm -m 4g \
>> 		-append 'npt_rw npt_rw_pfwalk'
> Any chance you're running against an older KVM version?  The test passes if I
> run against a build with my MMU pile on top of kvm/queue, but fails on a random
> older KVM.

I'm running it against the current kvm/queue.

Paolo

Re: [kvm-unit-tests PATCH 05/12] nSVM: Remove NPT reserved bits tests (new one on the way)

[Sean Christopherson]

On Thu, Jun 24, 2021, Paolo Bonzini wrote:
> On 24/06/21 19:43, Sean Christopherson wrote:
> > > 	./x86/run x86/svm.flat -smp 2 -cpu max,+svm -m 4g \
> > > 		-append 'npt_rw npt_rw_pfwalk'
> > Any chance you're running against an older KVM version?  The test passes if I
> > run against a build with my MMU pile on top of kvm/queue, but fails on a random
> > older KVM.
> 
> I'm running it against the current kvm/queue.

Huh.  Still passes for me on 292fedba687b ("Merge branch 'kvm-c-bit' into HEAD").
I'm running on AMD EPYC 7B12 / Rome, if that helps at all.

Re: [kvm-unit-tests PATCH 05/12] nSVM: Remove NPT reserved bits tests (new one on the way)

[Maxim Levitsky]

On Thu, 2021-06-24 at 17:43 +0000, Sean Christopherson wrote:
> On Thu, Jun 24, 2021, Paolo Bonzini wrote:
> > On 22/06/21 23:00, Sean Christopherson wrote:
> > > Remove two of nSVM's NPT reserved bits test, a soon-to-be-added test will
> > > provide a superset of their functionality, e.g. the current tests are
> > > limited in the sense that they test a single entry and a single bit,
> > > e.g. don't test conditionally-reserved bits.
> > > 
> > > The npt_rsvd test in particular is quite nasty as it subtly relies on
> > > EFER.NX=1; dropping the test will allow cleaning up the EFER.NX weirdness
> > > (it's forced for_all_  tests, presumably to get the desired PFEC.FETCH=1
> > > for this one test).
> > > 
> > > Signed-off-by: Sean Christopherson<seanjc@google.com>
> > > ---
> > >   x86/svm_tests.c | 45 ---------------------------------------------
> > >   1 file changed, 45 deletions(-)
> > 
> > This exposes a KVM bug, reproducible with
> > 
> > 	./x86/run x86/svm.flat -smp 2 -cpu max,+svm -m 4g \
> > 		-append 'npt_rw npt_rw_pfwalk'
> 
> Any chance you're running against an older KVM version?  The test passes if I
> run against a build with my MMU pile on top of kvm/queue, but fails on a random
> older KVM.
> 
> Side topic, these tests all fail to invalidate TLB entries after modifying PTEs.
> I suspect they work in part because KVM flushes and syncs on all nested SVM
> transitions...

I also now tried to reproduce and the test passes.

Best regards,
	Maxim Levitsky

> 
> > While running npt_rw_pfwalk, the #NPF gets an incorrect EXITINFO2
> > (address for the NPF location; on my machine it gets 0xbfede6f0 instead of
> > 0xbfede000).  The same tests work with QEMU from git.
> > 
> > I didn't quite finish analyzing it, but my current theory is
> > that KVM receives a pagewalk NPF for a *different* page walk that is caused
> > by read-only page tables; then it finds that the page walk to 0xbfede6f0
> > *does fail* (after all the correct and wrong EXITINFO2 belong to the same pfn)
> > and therefore injects it anyway.  This theory is because the 0x6f0 offset in
> > the page table corresponds to the 0xde000 part of the faulting address.
> > Maxim will look into it while I'm away.
> > 
> > Paolo
> > 

[kvm-unit-tests PATCH 06/12] nSVM: Stop forcing EFER.NX=1 for all tests

[Sean Christopherson]

Don't force EFER.NX=1 for SVM tests now that the one NPT test that needs
EFER.NX=1 does its own housekeeping.

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

diff --git a/x86/svm.c b/x86/svm.c
index 6e5872d..0959189 100644
--- a/x86/svm.c
+++ b/x86/svm.c
@@ -295,7 +295,7 @@ static void set_additional_vcpu_msr(void *msr_efer)
 	void *hsave = alloc_page();
 
 	wrmsr(MSR_VM_HSAVE_PA, virt_to_phys(hsave));
-	wrmsr(MSR_EFER, (ulong)msr_efer | EFER_SVME | EFER_NX);
+	wrmsr(MSR_EFER, (ulong)msr_efer | EFER_SVME);
 }
 
 static void setup_svm(void)
@@ -306,7 +306,6 @@ static void setup_svm(void)
 
 	wrmsr(MSR_VM_HSAVE_PA, virt_to_phys(hsave));
 	wrmsr(MSR_EFER, rdmsr(MSR_EFER) | EFER_SVME);
-	wrmsr(MSR_EFER, rdmsr(MSR_EFER) | EFER_NX);
 
 	io_bitmap = (void *) ALIGN((ulong)io_bitmap_area, PAGE_SIZE);
 
-- 
2.32.0.288.g62a8d224e6-goog

[kvm-unit-tests PATCH 07/12] nSVM: Remove a superfluous modification of guest EFER.NX in NPT NX test

[Sean Christopherson]

Remove a bizarre modification of the guest's EFER.NX from the NPT NX
test.  For reasons unknown, the NPT NX test forces EFER.NX in the guest
_after_ running the test.  Now that the v1 infrastructure saves/restores
guest EFER across the test, the motivation, whatever thay may have been,
is moot because the forced EFER.NX value won't persist.

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

diff --git a/x86/svm_tests.c b/x86/svm_tests.c
index 96add48..b1783f8 100644
--- a/x86/svm_tests.c
+++ b/x86/svm_tests.c
@@ -718,8 +718,6 @@ static bool npt_nx_check(struct svm_test *test)
 
     *pte &= ~PT64_NX_MASK;
 
-    vmcb->save.efer |= EFER_NX;
-
     return (vmcb->control.exit_code == SVM_EXIT_NPF)
            && (vmcb->control.exit_info_1 == 0x100000015ULL);
 }
-- 
2.32.0.288.g62a8d224e6-goog

[kvm-unit-tests PATCH 08/12] nSVM: Clear guest's EFER.NX in NPT NX test

[Sean Christopherson]

Clear the guest's EFER.NX when testing that KVM supports the NX bit in
nested NPT.  The guest's EFER (and CR0 and CR4) should not affect NPT
behavior in any way.

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

diff --git a/x86/svm_tests.c b/x86/svm_tests.c
index b1783f8..fdef620 100644
--- a/x86/svm_tests.c
+++ b/x86/svm_tests.c
@@ -705,6 +705,9 @@ static void npt_nx_prepare(struct svm_test *test)
     test->scratch = rdmsr(MSR_EFER);
     wrmsr(MSR_EFER, test->scratch | EFER_NX);
 
+    /* Clear the guest's EFER.NX, it should not affect NPT behavior. */
+    vmcb->save.efer &= ~EFER_NX;
+
     pte = npt_get_pte((u64)null_test);
 
     *pte |= PT64_NX_MASK;
-- 
2.32.0.288.g62a8d224e6-goog

[kvm-unit-tests PATCH 09/12] lib/vmalloc: Let arch code pass a value to its setup_mmu() helper

[Sean Christopherson]

Add an inner __setup_vm() that takes an opaque param and passes said
param along to setup_mmu().  x86 will use the param to configure its page
tables for kernel vs. user so that tests that want to enable SMEP (fault
if kernel executes user page) can do so without resorting to hacks and
without breaking tests that need user pages, i.e. that run user code.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 lib/arm/mmu.c   | 2 +-
 lib/s390x/mmu.c | 3 ++-
 lib/vmalloc.c   | 9 +++++++--
 lib/vmalloc.h   | 4 +++-
 lib/x86/vm.c    | 2 +-
 s390x/uv-host.c | 2 +-
 6 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/lib/arm/mmu.c b/lib/arm/mmu.c
index 7628f79..e1a72fe 100644
--- a/lib/arm/mmu.c
+++ b/lib/arm/mmu.c
@@ -153,7 +153,7 @@ void mmu_set_range_sect(pgd_t *pgtable, uintptr_t virt_offset,
 	}
 }
 
-void *setup_mmu(phys_addr_t phys_end)
+void *setup_mmu(phys_addr_t phys_end, void *unused)
 {
 	struct mem_region *r;
 
diff --git a/lib/s390x/mmu.c b/lib/s390x/mmu.c
index c973443..6f9e650 100644
--- a/lib/s390x/mmu.c
+++ b/lib/s390x/mmu.c
@@ -343,7 +343,8 @@ static void setup_identity(pgd_t *pgtable, phys_addr_t start_addr,
 	}
 }
 
-void *setup_mmu(phys_addr_t phys_end){
+void *setup_mmu(phys_addr_t phys_end, void *unused)
+{
 	pgd_t *page_root;
 
 	/* allocate a region-1 table */
diff --git a/lib/vmalloc.c b/lib/vmalloc.c
index aa7cc41..5726825 100644
--- a/lib/vmalloc.c
+++ b/lib/vmalloc.c
@@ -206,7 +206,7 @@ void init_alloc_vpage(void *top)
 	spin_unlock(&lock);
 }
 
-void setup_vm()
+void __setup_vm(void *opaque)
 {
 	phys_addr_t base, top;
 
@@ -228,7 +228,7 @@ void setup_vm()
 
 	find_highmem();
 	phys_alloc_get_unused(&base, &top);
-	page_root = setup_mmu(top);
+	page_root = setup_mmu(top, opaque);
 	if (base != top) {
 		base = PAGE_ALIGN(base) >> PAGE_SHIFT;
 		top = top >> PAGE_SHIFT;
@@ -240,3 +240,8 @@ void setup_vm()
 	alloc_ops = &vmalloc_ops;
 	spin_unlock(&lock);
 }
+
+void setup_vm(void)
+{
+	__setup_vm(NULL);
+}
diff --git a/lib/vmalloc.h b/lib/vmalloc.h
index 346f94f..0269fdd 100644
--- a/lib/vmalloc.h
+++ b/lib/vmalloc.h
@@ -14,9 +14,11 @@ extern void *alloc_vpage(void);
 extern void init_alloc_vpage(void *top);
 /* Set up the virtual allocator; also sets up the page allocator if needed */
 extern void setup_vm(void);
+/* As above, plus passes an opaque value to setup_mmu(). */
+extern void __setup_vm(void *opaque);
 
 /* Set up paging */
-extern void *setup_mmu(phys_addr_t top);
+extern void *setup_mmu(phys_addr_t top, void *opaque);
 /* Walk the page table and resolve the virtual address to a physical address */
 extern phys_addr_t virt_to_pte_phys(pgd_t *pgtable, void *virt);
 /* Map the virtual address to the physical address for the given page tables */
diff --git a/lib/x86/vm.c b/lib/x86/vm.c
index e223bb4..221d427 100644
--- a/lib/x86/vm.c
+++ b/lib/x86/vm.c
@@ -147,7 +147,7 @@ static void set_additional_vcpu_vmregs(struct vm_vcpu_info *info)
 	write_cr0(info->cr0);
 }
 
-void *setup_mmu(phys_addr_t end_of_memory)
+void *setup_mmu(phys_addr_t end_of_memory, void *unused)
 {
     pgd_t *cr3 = alloc_page();
     struct vm_vcpu_info info;
diff --git a/s390x/uv-host.c b/s390x/uv-host.c
index 49c66f1..426a67f 100644
--- a/s390x/uv-host.c
+++ b/s390x/uv-host.c
@@ -446,7 +446,7 @@ static void setup_vmem(void)
 {
 	uint64_t asce, mask;
 
-	setup_mmu(get_max_ram_size());
+	setup_mmu(get_max_ram_size(), NULL);
 	asce = stctg(1);
 	lctlg(13, asce);
 	mask = extract_psw_mask() | 0x0000C00000000000UL;
-- 
2.32.0.288.g62a8d224e6-goog

[kvm-unit-tests PATCH 10/12] x86: Let tests omit PT_USER_MASK when configuring virtual memory

[Sean Christopherson]

Let tests opt out of setting PT_USER_MASK so that they can set CR4.SMEP
and/or CR4.SMAP without having to manually modify all PTEs, which is
beyond painful.  Keep user pages the default to avoid having to update
existing tests.

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

diff --git a/lib/x86/vm.c b/lib/x86/vm.c
index 221d427..5cd2ee4 100644
--- a/lib/x86/vm.c
+++ b/lib/x86/vm.c
@@ -4,6 +4,8 @@
 #include "alloc_page.h"
 #include "smp.h"
 
+static pteval_t pte_opt_mask;
+
 pteval_t *install_pte(pgd_t *cr3,
 		      int pte_level,
 		      void *virt,
@@ -23,7 +25,7 @@ pteval_t *install_pte(pgd_t *cr3,
             else
                 pt_page = 0;
 	    memset(new_pt, 0, PAGE_SIZE);
-	    pt[offset] = virt_to_phys(new_pt) | PT_PRESENT_MASK | PT_WRITABLE_MASK | PT_USER_MASK;
+	    pt[offset] = virt_to_phys(new_pt) | PT_PRESENT_MASK | PT_WRITABLE_MASK | pte_opt_mask;
 	}
 	pt = phys_to_virt(pt[offset] & PT_ADDR_MASK);
     }
@@ -93,12 +95,12 @@ pteval_t *get_pte_level(pgd_t *cr3, void *virt, int pte_level)
 pteval_t *install_large_page(pgd_t *cr3, phys_addr_t phys, void *virt)
 {
     return install_pte(cr3, 2, virt,
-		       phys | PT_PRESENT_MASK | PT_WRITABLE_MASK | PT_USER_MASK | PT_PAGE_SIZE_MASK, 0);
+		       phys | PT_PRESENT_MASK | PT_WRITABLE_MASK | pte_opt_mask | PT_PAGE_SIZE_MASK, 0);
 }
 
 pteval_t *install_page(pgd_t *cr3, phys_addr_t phys, void *virt)
 {
-    return install_pte(cr3, 1, virt, phys | PT_PRESENT_MASK | PT_WRITABLE_MASK | PT_USER_MASK, 0);
+    return install_pte(cr3, 1, virt, phys | PT_PRESENT_MASK | PT_WRITABLE_MASK | pte_opt_mask, 0);
 }
 
 void install_pages(pgd_t *cr3, phys_addr_t phys, size_t len, void *virt)
@@ -147,12 +149,17 @@ static void set_additional_vcpu_vmregs(struct vm_vcpu_info *info)
 	write_cr0(info->cr0);
 }
 
-void *setup_mmu(phys_addr_t end_of_memory, void *unused)
+void *setup_mmu(phys_addr_t end_of_memory, void *opt_mask)
 {
     pgd_t *cr3 = alloc_page();
     struct vm_vcpu_info info;
     int i;
 
+    if (opt_mask)
+	pte_opt_mask = *(pteval_t *)opt_mask;
+    else
+	pte_opt_mask = PT_USER_MASK;
+
     memset(cr3, 0, PAGE_SIZE);
 
 #ifdef __x86_64__
-- 
2.32.0.288.g62a8d224e6-goog

[kvm-unit-tests PATCH 11/12] x86: Add GBPAGES CPUID macro, clean up CPUID comments

[Sean Christopherson]

Add a GBPAGES CPUID macro for a future NPT test and reorganize the
entries to be explicitly Basic vs. Extended, with a hint that Basic leafs
come from Intel and Extended leafs come from AMD.  Organizing by Intel
vs. AMD is at best misleading, e.g. if both support a feature, and at
worst flat out wrong, e.g. AMD defined NX and LM (not sure about RDPRU,
but avoiding such questions is the whole point of organizing by type).

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 lib/x86/processor.h | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/lib/x86/processor.h b/lib/x86/processor.h
index 8db13e9..173520f 100644
--- a/lib/x86/processor.h
+++ b/lib/x86/processor.h
@@ -141,7 +141,7 @@ static inline bool is_intel(void)
  */
 
 /*
- * Intel CPUID features
+ * Basic Leafs, a.k.a. Intel defined
  */
 #define	X86_FEATURE_MWAIT		(CPUID(0x1, 0, ECX, 3))
 #define	X86_FEATURE_VMX			(CPUID(0x1, 0, ECX, 5))
@@ -174,15 +174,16 @@ static inline bool is_intel(void)
 #define	X86_FEATURE_SPEC_CTRL		(CPUID(0x7, 0, EDX, 26))
 #define	X86_FEATURE_ARCH_CAPABILITIES	(CPUID(0x7, 0, EDX, 29))
 #define	X86_FEATURE_PKS			(CPUID(0x7, 0, ECX, 31))
+
+/*
+ * Extended Leafs, a.k.a. AMD defined
+ */
+#define	X86_FEATURE_SVM			(CPUID(0x80000001, 0, ECX, 2))
 #define	X86_FEATURE_NX			(CPUID(0x80000001, 0, EDX, 20))
+#define	X86_FEATURE_GBPAGES		(CPUID(0x80000001, 0, EDX, 26))
+#define	X86_FEATURE_RDTSCP		(CPUID(0x80000001, 0, EDX, 27))
 #define	X86_FEATURE_LM			(CPUID(0x80000001, 0, EDX, 29))
 #define	X86_FEATURE_RDPRU		(CPUID(0x80000008, 0, EBX, 4))
-
-/*
- * AMD CPUID features
- */
-#define	X86_FEATURE_SVM			(CPUID(0x80000001, 0, ECX, 2))
-#define	X86_FEATURE_RDTSCP		(CPUID(0x80000001, 0, EDX, 27))
 #define	X86_FEATURE_AMD_IBPB		(CPUID(0x80000008, 0, EBX, 12))
 #define	X86_FEATURE_NPT			(CPUID(0x8000000A, 0, EDX, 0))
 #define	X86_FEATURE_NRIPS		(CPUID(0x8000000A, 0, EDX, 3))
-- 
2.32.0.288.g62a8d224e6-goog

[kvm-unit-tests PATCH 12/12] nSVM: Add test for NPT reserved bit and #NPF error code behavior

[Sean Christopherson]

Add a test to verify that KVM generates the correct #NPF and PFEC when
host and guest EFER.NX and CR4.SMEP values diverge, and that KVM
correctly detects reserved bits in the first place.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/svm.c       |   5 +-
 x86/svm_tests.c | 143 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 147 insertions(+), 1 deletion(-)

diff --git a/x86/svm.c b/x86/svm.c
index 0959189..f185ca0 100644
--- a/x86/svm.c
+++ b/x86/svm.c
@@ -13,6 +13,7 @@
 #include "alloc_page.h"
 #include "isr.h"
 #include "apic.h"
+#include "vmalloc.h"
 
 /* for the nested page table*/
 u64 *pte[2048];
@@ -397,12 +398,14 @@ test_wanted(const char *name, char *filters[], int filter_count)
 
 int main(int ac, char **av)
 {
+	/* Omit PT_USER_MASK to allow tested host.CR4.SMEP=1. */
+	pteval_t opt_mask = 0;
 	int i = 0;
 
 	ac--;
 	av++;
 
-	setup_vm();
+	__setup_vm(&opt_mask);
 
 	if (!this_cpu_has(X86_FEATURE_SVM)) {
 		printf("SVM not availble\n");
diff --git a/x86/svm_tests.c b/x86/svm_tests.c
index fdef620..1aaf983 100644
--- a/x86/svm_tests.c
+++ b/x86/svm_tests.c
@@ -2499,6 +2499,148 @@ static void svm_guest_state_test(void)
 	test_vmrun_canonicalization();
 }
 
+static void __svm_npt_rsvd_bits_test(u64 *pxe, u64 rsvd_bits, u64 efer,
+				     ulong cr4, u64 guest_efer, ulong guest_cr4)
+{
+	u64 pxe_orig = *pxe;
+	int exit_reason;
+	u64 pfec;
+
+	wrmsr(MSR_EFER, efer);
+	write_cr4(cr4);
+
+	vmcb->save.efer = guest_efer;
+	vmcb->save.cr4  = guest_cr4;
+
+	*pxe |= rsvd_bits;
+
+	exit_reason = svm_vmrun();
+
+	report(exit_reason == SVM_EXIT_NPF,
+	       "Wanted #NPF on rsvd bits = 0x%lx, got exit = 0x%x", rsvd_bits, exit_reason);
+
+	if (pxe == npt_get_pdpe() || pxe == npt_get_pml4e()) {
+		/*
+		 * The guest's page tables will blow up on a bad PDPE/PML4E,
+		 * before starting the final walk of the guest page.
+		 */
+		pfec = 0x20000000full;
+	} else {
+		/* RSVD #NPF on final walk of guest page. */
+		pfec = 0x10000000dULL;
+
+		/* PFEC.FETCH=1 if NX=1 *or* SMEP=1. */
+		if ((cr4 & X86_CR4_SMEP) || (efer & EFER_NX))
+			pfec |= 0x10;
+
+	}
+
+	report(vmcb->control.exit_info_1 == pfec,
+	       "Wanted PFEC = 0x%lx, got PFEC = %lx, PxE = 0x%lx.  "
+	       "host.NX = %u, host.SMEP = %u, guest.NX = %u, guest.SMEP = %u",
+	       pfec, vmcb->control.exit_info_1, *pxe,
+	       !!(efer & EFER_NX), !!(cr4 & X86_CR4_SMEP),
+	       !!(guest_efer & EFER_NX), !!(guest_cr4 & X86_CR4_SMEP));
+
+	*pxe = pxe_orig;
+}
+
+static void _svm_npt_rsvd_bits_test(u64 *pxe, u64 pxe_rsvd_bits,  u64 efer,
+				    ulong cr4, u64 guest_efer, ulong guest_cr4)
+{
+	u64 rsvd_bits;
+	int i;
+
+	/*
+	 * Test all combinations of guest/host EFER.NX and CR4.SMEP.  If host
+	 * EFER.NX=0, use NX as the reserved bit, otherwise use the passed in
+	 * @pxe_rsvd_bits.
+	 */
+	for (i = 0; i < 16; i++) {
+		if (i & 1) {
+			rsvd_bits = pxe_rsvd_bits;
+			efer |= EFER_NX;
+		} else {
+			rsvd_bits = PT64_NX_MASK;
+			efer &= ~EFER_NX;
+		}
+		if (i & 2)
+			cr4 |= X86_CR4_SMEP;
+		else
+			cr4 &= ~X86_CR4_SMEP;
+		if (i & 4)
+			guest_efer |= EFER_NX;
+		else
+			guest_efer &= ~EFER_NX;
+		if (i & 8)
+			guest_cr4 |= X86_CR4_SMEP;
+		else
+			guest_cr4 &= ~X86_CR4_SMEP;
+
+		__svm_npt_rsvd_bits_test(pxe, rsvd_bits, efer, cr4,
+					 guest_efer, guest_cr4);
+	}
+}
+
+static u64 get_random_bits(u64 hi, u64 low)
+{
+	u64 rsvd_bits;
+
+	do {
+		rsvd_bits = (rdtsc() << low) & GENMASK_ULL(hi, low);
+	} while (!rsvd_bits);
+
+	return rsvd_bits;
+}
+
+
+static void svm_npt_rsvd_bits_test(void)
+{
+	u64   saved_efer, host_efer, sg_efer, guest_efer;
+	ulong saved_cr4,  host_cr4,  sg_cr4,  guest_cr4;
+
+	if (!npt_supported()) {
+		report_skip("NPT not supported");
+		return;
+	}
+
+	saved_efer = host_efer  = rdmsr(MSR_EFER);
+	saved_cr4  = host_cr4   = read_cr4();
+	sg_efer    = guest_efer = vmcb->save.efer;
+	sg_cr4     = guest_cr4  = vmcb->save.cr4;
+
+	test_set_guest(basic_guest_main);
+
+	/*
+	 * 4k PTEs don't have reserved bits if MAXPHYADDR >= 52, just skip the
+	 * sub-test.  The NX test is still valid, but the extra bit of coverage
+	 * isn't worth the extra complexity.
+	 */
+	if (cpuid_maxphyaddr() >= 52)
+		goto skip_pte_test;
+
+	_svm_npt_rsvd_bits_test(npt_get_pte((u64)basic_guest_main),
+				get_random_bits(51, cpuid_maxphyaddr()),
+				host_efer, host_cr4, guest_efer, guest_cr4);
+
+skip_pte_test:
+	_svm_npt_rsvd_bits_test(npt_get_pde((u64)basic_guest_main),
+				get_random_bits(20, 13) | PT_PAGE_SIZE_MASK,
+				host_efer, host_cr4, guest_efer, guest_cr4);
+
+	_svm_npt_rsvd_bits_test(npt_get_pdpe(),
+				PT_PAGE_SIZE_MASK |
+					(this_cpu_has(X86_FEATURE_GBPAGES) ? get_random_bits(29, 13) : 0),
+				host_efer, host_cr4, guest_efer, guest_cr4);
+
+	_svm_npt_rsvd_bits_test(npt_get_pml4e(), BIT_ULL(8),
+				host_efer, host_cr4, guest_efer, guest_cr4);
+
+	wrmsr(MSR_EFER, saved_efer);
+	write_cr4(saved_cr4);
+	vmcb->save.efer = sg_efer;
+	vmcb->save.cr4  = sg_cr4;
+}
 
 static bool volatile svm_errata_reproduced = false;
 static unsigned long volatile physical = 0;
@@ -2741,6 +2883,7 @@ struct svm_test svm_tests[] = {
       host_rflags_finished, host_rflags_check },
     TEST(svm_cr4_osxsave_test),
     TEST(svm_guest_state_test),
+    TEST(svm_npt_rsvd_bits_test),
     TEST(svm_vmrun_errata_test),
     TEST(svm_vmload_vmsave),
     { NULL, NULL, NULL, NULL, NULL, NULL, NULL }
-- 
2.32.0.288.g62a8d224e6-goog

Re: [kvm-unit-tests PATCH 00/12] nSVM: NPT improvements and cleanups

[Paolo Bonzini]

On 22/06/21 23:00, Sean Christopherson wrote:
> The first chunk of this series (everything up to the lib/vmalloc patch)
> are cleanups and bug fixes for existing nSVM tests that I collected on
> my first attempt at the new NPT test.  I originally wanted to piggyback
> the existing "v1" nSVM tests and implemented the fixes/cleanups, but that
> approach didn't go so well because of the v1 infrastructure limitations.
> 
> The common lib/vmalloc changes are to allow arch code to pass arbitrary
> data to its setup_mmu() function.  x86-64 uses the param to avoid marking
> PTEs a USER so that tests can enable SMEP (#PF if supervisor mode fetches
> from a USER PTE) without exploding or having to duplicate all page tables.
> 
> The "new" test targets nested NPT by running L1 and L2 with different
> EFER.NX and CR4.SMEP settings to verify that KVM uses the correct MMU
> settings when injecting page faults.
> 
> Sean Christopherson (12):
>    nSVM: Provide expected and actual exit codes on VMRUN test failure
>    nSVM: Replace open coded NX manipulation with appropriate macros
>    nSVM: Reset the VMCB before every v1 test
>    nSVM: Explicitly save/update/restore EFER.NX for NPT NX test
>    nSVM: Remove NPT reserved bits tests (new one on the way)
>    nSVM: Stop forcing EFER.NX=1 for all tests
>    nSVM: Remove a superfluous modification of guest EFER.NX in NPT NX
>      test
>    nSVM: Clear guest's EFER.NX in NPT NX test
>    lib/vmalloc: Let arch code pass a value to its setup_mmu() helper
>    x86: Let tests omit PT_USER_MASK when configuring virtual memory
>    x86: Add GBPAGES CPUID macro, clean up CPUID comments
>    nSVM: Add test for NPT reserved bit and #NPF error code behavior
> 
>   lib/arm/mmu.c       |   2 +-
>   lib/s390x/mmu.c     |   3 +-
>   lib/vmalloc.c       |   9 +-
>   lib/vmalloc.h       |   4 +-
>   lib/x86/processor.h |  15 +--
>   lib/x86/vm.c        |  15 ++-
>   s390x/uv-host.c     |   2 +-
>   x86/svm.c           |  10 +-
>   x86/svm_tests.c     | 220 +++++++++++++++++++++++++++++++-------------
>   9 files changed, 196 insertions(+), 84 deletions(-)
> 

Queued, thanks.

Paolo