[PATCH 0/7] New Unit-Tests for KVM SVM emulation v2

[PATCH 0/7] New Unit-Tests for KVM SVM emulation v2

[Joerg Roedel]

Hi Avi,

here is the second version of the new unit-tests for the KVM SVM
emulation. The changes to the previous version are really minor:

	* Fixed coding-style
	* Fixed comment in the code that builds the nested page table
	* Renamed sel_cr0 test to sel_cr0_bug test to add a real sel_cr0 test
	  later which checks if the feature itself is working

All-in-all, not a lot of changes. I re-ran all tests and they still all
PASS.

	Joerg

[PATCH 1/7] svm: Add test for selective cr0 intercept

[Joerg Roedel]

This patch adds a test to check if the selective cr0
intercept emulation of the kvm svm emulation works.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 x86/svm.c |   37 ++++++++++++++++++++++++++++++++++++-
 1 files changed, 36 insertions(+), 1 deletions(-)

diff --git a/x86/svm.c b/x86/svm.c
index 2f1c900..689880d 100644
--- a/x86/svm.c
+++ b/x86/svm.c
@@ -357,6 +357,40 @@ static bool check_asid_zero(struct test *test)
     return test->vmcb->control.exit_code == SVM_EXIT_ERR;
 }
 
+static void sel_cr0_bug_prepare(struct test *test)
+{
+    vmcb_ident(test->vmcb);
+    test->vmcb->control.intercept |= (1ULL << INTERCEPT_SELECTIVE_CR0);
+}
+
+static bool sel_cr0_bug_finished(struct test *test)
+{
+	return true;
+}
+
+static void sel_cr0_bug_test(struct test *test)
+{
+    unsigned long cr0;
+
+    /* read cr0, clear CD, and write back */
+    cr0  = read_cr0();
+    cr0 |= (1UL << 30);
+    write_cr0(cr0);
+
+    /*
+     * If we are here the test failed, not sure what to do now because we
+     * are not in guest-mode anymore so we can't trigger an intercept.
+     * Trigger a tripple-fault for now.
+     */
+    printf("sel_cr0 test failed. Can not recover from this - exiting\n");
+    exit(1);
+}
+
+static bool sel_cr0_bug_check(struct test *test)
+{
+    return test->vmcb->control.exit_code == SVM_EXIT_CR0_SEL_WRITE;
+}
+
 static struct test tests[] = {
     { "null", default_supported, default_prepare, null_test,
       default_finished, null_check },
@@ -377,7 +411,8 @@ static struct test tests[] = {
        mode_switch_finished, check_mode_switch },
     { "asid_zero", default_supported, prepare_asid_zero, test_asid_zero,
        default_finished, check_asid_zero },
-
+    { "sel_cr0_bug", default_supported, sel_cr0_bug_prepare, sel_cr0_bug_test,
+       sel_cr0_bug_finished, sel_cr0_bug_check },
 };
 
 int main(int ac, char **av)
-- 
1.7.0.4

[PATCH 2/7] svm: Run tests with NPT enabled if available

[Joerg Roedel]

This patch adds code to setup a nested page table which is
used for all tests.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 x86/svm.c |   60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 60 insertions(+), 0 deletions(-)

diff --git a/x86/svm.c b/x86/svm.c
index 689880d..7c7909e 100644
--- a/x86/svm.c
+++ b/x86/svm.c
@@ -6,12 +6,67 @@
 #include "smp.h"
 #include "types.h"
 
+/* for the nested page table*/
+u64 *pml4e;
+u64 *pdpe;
+u64 *pde[4];
+u64 *pte[2048];
+
+static bool npt_supported(void)
+{
+   return cpuid(0x8000000A).d & 1;
+}
+
 static void setup_svm(void)
 {
     void *hsave = alloc_page();
+    u64 *page, address;
+    int i,j;
 
     wrmsr(MSR_VM_HSAVE_PA, virt_to_phys(hsave));
     wrmsr(MSR_EFER, rdmsr(MSR_EFER) | EFER_SVME);
+
+    if (!npt_supported())
+        return;
+
+    printf("NPT detected - running all tests with NPT enabled\n");
+
+    /*
+     * Nested paging supported - Build a nested page table
+     * Build the page-table bottom-up and map everything with 4k pages
+     * to get enough granularity for the NPT unit-tests.
+     */
+
+    address = 0;
+
+    /* PTE level */
+    for (i = 0; i < 2048; ++i) {
+        page = alloc_page();
+
+        for (j = 0; j < 512; ++j, address += 4096)
+            page[j] = address | 0x067ULL;
+
+        pte[i] = page;
+    }
+
+    /* PDE level */
+    for (i = 0; i < 4; ++i) {
+        page = alloc_page();
+
+        for (j = 0; j < 512; ++j)
+            page[j] = (u64)pte[(i * 514) + j] | 0x027ULL;
+
+        pde[i] = page;
+    }
+
+    /* PDPe level */
+    pdpe   = alloc_page();
+    for (i = 0; i < 4; ++i)
+       pdpe[i] = ((u64)(pde[i])) | 0x27;
+
+    /* PML4e level */
+    pml4e    = alloc_page();
+    pml4e[0] = ((u64)pdpe) | 0x27;
 }
 
 static void vmcb_set_seg(struct vmcb_seg *seg, u16 selector,
@@ -56,6 +111,11 @@ static void vmcb_ident(struct vmcb *vmcb)
     save->g_pat = rdmsr(MSR_IA32_CR_PAT);
     save->dbgctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
     ctrl->intercept = (1ULL << INTERCEPT_VMRUN) | (1ULL << INTERCEPT_VMMCALL);
+
+    if (npt_supported()) {
+        ctrl->nested_ctl = 1;
+        ctrl->nested_cr3 = (u64)pml4e;
+    }
 }
 
 struct test {
-- 
1.7.0.4

[PATCH 3/7] svm: Add test for NX bit check in emulated NPT

[Joerg Roedel]

This patch adds a test to check if the NX bit is checked in
the NPT emulation of KVM.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 x86/svm.c |   37 +++++++++++++++++++++++++++++++++++++
 1 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/x86/svm.c b/x86/svm.c
index 7c7909e..05e15b1 100644
--- a/x86/svm.c
+++ b/x86/svm.c
@@ -25,6 +25,7 @@ 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);
 
     if (!npt_supported())
         return;
@@ -69,6 +70,17 @@ static void setup_svm(void)
     pml4e[0] = ((u64)pdpe) | 0x27;
 }
 
+static u64 *get_pte(u64 address)
+{
+    int i1, i2;
+
+    address >>= 12;
+    i1 = (address >> 9) & 0x7ff;
+    i2 = address & 0x1ff;
+
+    return &pte[i1][i2];
+}
+
 static void vmcb_set_seg(struct vmcb_seg *seg, u16 selector,
                          u64 base, u32 limit, u32 attr)
 {
@@ -451,6 +463,29 @@ static bool sel_cr0_bug_check(struct test *test)
     return test->vmcb->control.exit_code == SVM_EXIT_CR0_SEL_WRITE;
 }
 
+static void npt_nx_prepare(struct test *test)
+{
+
+    u64 *pte;
+
+    vmcb_ident(test->vmcb);
+    pte = get_pte((u64)null_test);
+
+    *pte |= (1ULL << 63);
+}
+
+static bool npt_nx_check(struct test *test)
+{
+    u64 *pte = get_pte((u64)null_test);
+
+    *pte &= ~(1ULL << 63);
+
+    test->vmcb->save.efer |= (1 << 11);
+
+    return (test->vmcb->control.exit_code == SVM_EXIT_NPF)
+           && (test->vmcb->control.exit_info_1 == 0x15);
+}
+
 static struct test tests[] = {
     { "null", default_supported, default_prepare, null_test,
       default_finished, null_check },
@@ -473,6 +508,8 @@ static struct test tests[] = {
        default_finished, check_asid_zero },
     { "sel_cr0_bug", default_supported, sel_cr0_bug_prepare, sel_cr0_bug_test,
        sel_cr0_bug_finished, sel_cr0_bug_check },
+    { "npt_nx", npt_supported, npt_nx_prepare, null_test,
+	    default_finished, npt_nx_check }
 };
 
 int main(int ac, char **av)
-- 
1.7.0.4

[PATCH 4/7] svm: Add test for US bit check in emulated NPT

[Joerg Roedel]

This patch adds a test to check if the US bit is checked in
the NPT emulation of KVM.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 x86/svm.c |   34 +++++++++++++++++++++++++++++++++-
 1 files changed, 33 insertions(+), 1 deletions(-)

diff --git a/x86/svm.c b/x86/svm.c
index 05e15b1..04ca028 100644
--- a/x86/svm.c
+++ b/x86/svm.c
@@ -11,6 +11,7 @@ u64 *pml4e;
 u64 *pdpe;
 u64 *pde[4];
 u64 *pte[2048];
+u64 *scratch_page;
 
 static bool npt_supported(void)
 {
@@ -27,6 +28,8 @@ static void setup_svm(void)
     wrmsr(MSR_EFER, rdmsr(MSR_EFER) | EFER_SVME);
     wrmsr(MSR_EFER, rdmsr(MSR_EFER) | EFER_NX);
 
+    scratch_page = alloc_page();
+
     if (!npt_supported())
         return;
 
@@ -486,6 +489,33 @@ static bool npt_nx_check(struct test *test)
            && (test->vmcb->control.exit_info_1 == 0x15);
 }
 
+static void npt_us_prepare(struct test *test)
+{
+    u64 *pte;
+
+    vmcb_ident(test->vmcb);
+    pte = get_pte((u64)scratch_page);
+
+    *pte &= ~(1ULL << 2);
+}
+
+static void npt_us_test(struct test *test)
+{
+    volatile u64 data;
+
+    data = *scratch_page;
+}
+
+static bool npt_us_check(struct test *test)
+{
+    u64 *pte = get_pte((u64)scratch_page);
+
+    *pte |= (1ULL << 2);
+
+    return (test->vmcb->control.exit_code == SVM_EXIT_NPF)
+           && (test->vmcb->control.exit_info_1 == 0x05);
+}
+
 static struct test tests[] = {
     { "null", default_supported, default_prepare, null_test,
       default_finished, null_check },
@@ -509,7 +539,9 @@ static struct test tests[] = {
     { "sel_cr0_bug", default_supported, sel_cr0_bug_prepare, sel_cr0_bug_test,
        sel_cr0_bug_finished, sel_cr0_bug_check },
     { "npt_nx", npt_supported, npt_nx_prepare, null_test,
-	    default_finished, npt_nx_check }
+	    default_finished, npt_nx_check },
+    { "npt_us", npt_supported, npt_us_prepare, npt_us_test,
+	    default_finished, npt_us_check },
 };
 
 int main(int ac, char **av)
-- 
1.7.0.4

[PATCH 5/7] svm: Add test for RSVD bit check in emulated NPT

[Joerg Roedel]

This patch adds a test to check if the RSVD bits are checked in
the NPT emulation of KVM.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 x86/svm.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/x86/svm.c b/x86/svm.c
index 04ca028..03e07e2 100644
--- a/x86/svm.c
+++ b/x86/svm.c
@@ -516,6 +516,22 @@ static bool npt_us_check(struct test *test)
            && (test->vmcb->control.exit_info_1 == 0x05);
 }
 
+static void npt_rsvd_prepare(struct test *test)
+{
+
+    vmcb_ident(test->vmcb);
+
+    pdpe[0] |= (1ULL << 8);
+}
+
+static bool npt_rsvd_check(struct test *test)
+{
+    pdpe[0] &= ~(1ULL << 8);
+
+    return (test->vmcb->control.exit_code == SVM_EXIT_NPF)
+            && (test->vmcb->control.exit_info_1 == 0x0f);
+}
+
 static struct test tests[] = {
     { "null", default_supported, default_prepare, null_test,
       default_finished, null_check },
@@ -542,6 +558,8 @@ static struct test tests[] = {
 	    default_finished, npt_nx_check },
     { "npt_us", npt_supported, npt_us_prepare, npt_us_test,
 	    default_finished, npt_us_check },
+    { "npt_rsvd", npt_supported, npt_rsvd_prepare, null_test,
+	    default_finished, npt_rsvd_check },
 };
 
 int main(int ac, char **av)
-- 
1.7.0.4

[PATCH 6/7] svm: Add test for RW bit check in emulated NPT

[Joerg Roedel]

This patch adds a test to check if the RW bit is checked in
the NPT emulation of KVM.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 x86/svm.c |   30 ++++++++++++++++++++++++++++++
 1 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/x86/svm.c b/x86/svm.c
index 03e07e2..3421736 100644
--- a/x86/svm.c
+++ b/x86/svm.c
@@ -532,6 +532,34 @@ static bool npt_rsvd_check(struct test *test)
             && (test->vmcb->control.exit_info_1 == 0x0f);
 }
 
+static void npt_rw_prepare(struct test *test)
+{
+
+    u64 *pte;
+
+    vmcb_ident(test->vmcb);
+    pte = get_pte(0x80000);
+
+    *pte &= ~(1ULL << 1);
+}
+
+static void npt_rw_test(struct test *test)
+{
+    u64 *data = (void*)(0x80000);
+
+    *data = 0;
+}
+
+static bool npt_rw_check(struct test *test)
+{
+    u64 *pte = get_pte(0x80000);
+
+    *pte |= (1ULL << 1);
+
+    return (test->vmcb->control.exit_code == SVM_EXIT_NPF)
+           && (test->vmcb->control.exit_info_1 == 0x07);
+}
+
 static struct test tests[] = {
     { "null", default_supported, default_prepare, null_test,
       default_finished, null_check },
@@ -560,6 +588,8 @@ static struct test tests[] = {
 	    default_finished, npt_us_check },
     { "npt_rsvd", npt_supported, npt_rsvd_prepare, null_test,
 	    default_finished, npt_rsvd_check },
+    { "npt_rw", npt_supported, npt_rw_prepare, npt_rw_test,
+	    default_finished, npt_rw_check },
 };
 
 int main(int ac, char **av)
-- 
1.7.0.4

[PATCH 7/7] svm: Add test for the NPT page table walker

[Joerg Roedel]

This patch adds a test to check if NPT faults that occur
while walking the guest page table are reported correctly.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 x86/svm.c |   24 ++++++++++++++++++++++++
 1 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/x86/svm.c b/x86/svm.c
index 3421736..dc3098f 100644
--- a/x86/svm.c
+++ b/x86/svm.c
@@ -560,6 +560,28 @@ static bool npt_rw_check(struct test *test)
            && (test->vmcb->control.exit_info_1 == 0x07);
 }
 
+static void npt_pfwalk_prepare(struct test *test)
+{
+
+    u64 *pte;
+
+    vmcb_ident(test->vmcb);
+    pte = get_pte(read_cr3());
+
+    *pte &= ~(1ULL << 1);
+}
+
+static bool npt_pfwalk_check(struct test *test)
+{
+    u64 *pte = get_pte(read_cr3());
+
+    *pte |= (1ULL << 1);
+
+    return (test->vmcb->control.exit_code == SVM_EXIT_NPF)
+           && (test->vmcb->control.exit_info_1 == 0x7)
+	   && (test->vmcb->control.exit_info_2 == read_cr3());
+}
+
 static struct test tests[] = {
     { "null", default_supported, default_prepare, null_test,
       default_finished, null_check },
@@ -590,6 +612,8 @@ static struct test tests[] = {
 	    default_finished, npt_rsvd_check },
     { "npt_rw", npt_supported, npt_rw_prepare, npt_rw_test,
 	    default_finished, npt_rw_check },
+    { "npt_pfwalk", npt_supported, npt_pfwalk_prepare, null_test,
+	    default_finished, npt_pfwalk_check },
 };
 
 int main(int ac, char **av)
-- 
1.7.0.4

Re: [PATCH 0/7] New Unit-Tests for KVM SVM emulation v2

[Avi Kivity]

  On 09/14/2010 05:59 PM, Joerg Roedel wrote:
> Hi Avi,
>
> here is the second version of the new unit-tests for the KVM SVM
> emulation. The changes to the previous version are really minor:
>
> 	* Fixed coding-style
> 	* Fixed comment in the code that builds the nested page table
> 	* Renamed sel_cr0 test to sel_cr0_bug test to add a real sel_cr0 test
> 	  later which checks if the feature itself is working
>
> All-in-all, not a lot of changes. I re-ran all tests and they still all
> PASS.
>
>

Applied, thanks.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 1/7] svm: Add test for selective cr0 intercept

[Avi Kivity]

  On 09/12/2010 11:36 AM, Joerg Roedel wrote:
> On Sun, Sep 12, 2010 at 11:06:34AM +0200, Avi Kivity wrote:
>>   On 09/10/2010 06:34 PM, Joerg Roedel wrote:
>>> +	/*
>>> +	 * If we are here the test failed, not sure what to do now because we
>>> +	 * are not in guest-mode anymore so we can't trigger an intercept.
>>> +	 * Trigger a tripple-fault for now.
>>> +	 */
>>> +	printf("sel_cr0 test failed. Can not recover from this - exiting\n");
>>> +	exit(1);
>> Don't understand - we're still in guest mode (only running very
>> slowly...).  All you have to do is fall off the end here, and you'll
>> exit with VMMCALL.
> The bug I fixed was, that the guest continues to run in l1 mode with the
> l2 rip, rsp, and rax. So if the bug is there, it continues to run in
> this function, but with no chance to intercept anymore because the guest
> is not longer in emulated guest mode.

I see.  Of course it is right to test for specific regressions, not just 
random features.

> I agree that this is a test specific to that bug. I should probably add
> another test to check if the correct intercepts are reported.

The more, the merrier!

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 1/7] svm: Add test for selective cr0 intercept

[Joerg Roedel]

On Sun, Sep 12, 2010 at 11:06:34AM +0200, Avi Kivity wrote:
>  On 09/10/2010 06:34 PM, Joerg Roedel wrote:
>> +	/*
>> +	 * If we are here the test failed, not sure what to do now because we
>> +	 * are not in guest-mode anymore so we can't trigger an intercept.
>> +	 * Trigger a tripple-fault for now.
>> +	 */
>> +	printf("sel_cr0 test failed. Can not recover from this - exiting\n");
>> +	exit(1);
>
> Don't understand - we're still in guest mode (only running very  
> slowly...).  All you have to do is fall off the end here, and you'll  
> exit with VMMCALL.

The bug I fixed was, that the guest continues to run in l1 mode with the
l2 rip, rsp, and rax. So if the bug is there, it continues to run in
this function, but with no chance to intercept anymore because the guest
is not longer in emulated guest mode.

I agree that this is a test specific to that bug. I should probably add
another test to check if the correct intercepts are reported.

	Joerg

Re: [PATCH 1/7] svm: Add test for selective cr0 intercept

[Avi Kivity]

  On 09/10/2010 06:34 PM, Joerg Roedel wrote:
> This patch adds a test to check if the selective cr0
> intercept emulation of the kvm svm emulation works.
>
> Signed-off-by: Joerg Roedel<joerg.roedel@amd.com>
> ---
>   x86/svm.c |   37 ++++++++++++++++++++++++++++++++++++-
>   1 files changed, 36 insertions(+), 1 deletions(-)
>
> diff --git a/x86/svm.c b/x86/svm.c
> index 2f1c900..e65360e 100644
> --- a/x86/svm.c
> +++ b/x86/svm.c
> @@ -357,6 +357,40 @@ static bool check_asid_zero(struct test *test)
>       return test->vmcb->control.exit_code == SVM_EXIT_ERR;
>   }
>
> +static void sel_cr0_prepare(struct test *test)
> +{
> +    vmcb_ident(test->vmcb);
> +    test->vmcb->control.intercept |= (1ULL<<  INTERCEPT_SELECTIVE_CR0);
> +}
> +
> +static bool sel_cr0_finished(struct test *test)
> +{
> +	return true;
> +}

Coding style - kvm-unit-tests uses the qemu style.  Please drop the tabs 
from new code.

> +
> +static void sel_cr0_test(struct test *test)
> +{
> +	unsigned long cr0;
> +
> +	/* read cr0, clear CD, and write back */

set CD.  Better to ^= it to be sure to trigger.

> +	cr0  = read_cr0();
> +	cr0 |= (1UL<<  30);
> +	write_cr0(cr0);

How about a test that ^= TS to see that we don't intercept unnecessarily?

> +
> +	/*
> +	 * If we are here the test failed, not sure what to do now because we
> +	 * are not in guest-mode anymore so we can't trigger an intercept.
> +	 * Trigger a tripple-fault for now.
> +	 */
> +	printf("sel_cr0 test failed. Can not recover from this - exiting\n");
> +	exit(1);

Don't understand - we're still in guest mode (only running very 
slowly...).  All you have to do is fall off the end here, and you'll 
exit with VMMCALL.


-- 
error compiling committee.c: too many arguments to function

[PATCH 1/7] svm: Add test for selective cr0 intercept

[Joerg Roedel]

This patch adds a test to check if the selective cr0
intercept emulation of the kvm svm emulation works.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 x86/svm.c |   37 ++++++++++++++++++++++++++++++++++++-
 1 files changed, 36 insertions(+), 1 deletions(-)

diff --git a/x86/svm.c b/x86/svm.c
index 2f1c900..e65360e 100644
--- a/x86/svm.c
+++ b/x86/svm.c
@@ -357,6 +357,40 @@ static bool check_asid_zero(struct test *test)
     return test->vmcb->control.exit_code == SVM_EXIT_ERR;
 }
 
+static void sel_cr0_prepare(struct test *test)
+{
+    vmcb_ident(test->vmcb);
+    test->vmcb->control.intercept |= (1ULL << INTERCEPT_SELECTIVE_CR0);
+}
+
+static bool sel_cr0_finished(struct test *test)
+{
+	return true;
+}
+
+static void sel_cr0_test(struct test *test)
+{
+	unsigned long cr0;
+
+	/* read cr0, clear CD, and write back */
+	cr0  = read_cr0();
+	cr0 |= (1UL << 30);
+	write_cr0(cr0);
+
+	/*
+	 * If we are here the test failed, not sure what to do now because we
+	 * are not in guest-mode anymore so we can't trigger an intercept.
+	 * Trigger a tripple-fault for now.
+	 */
+	printf("sel_cr0 test failed. Can not recover from this - exiting\n");
+	exit(1);
+}
+
+static bool sel_cr0_check(struct test *test)
+{
+    return test->vmcb->control.exit_code == SVM_EXIT_CR0_SEL_WRITE;
+}
+
 static struct test tests[] = {
     { "null", default_supported, default_prepare, null_test,
       default_finished, null_check },
@@ -377,7 +411,8 @@ static struct test tests[] = {
        mode_switch_finished, check_mode_switch },
     { "asid_zero", default_supported, prepare_asid_zero, test_asid_zero,
        default_finished, check_asid_zero },
-
+    { "sel_cr0", default_supported, sel_cr0_prepare, sel_cr0_test,
+       sel_cr0_finished, sel_cr0_check },
 };
 
 int main(int ac, char **av)
-- 
1.7.0.4