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

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

[Joerg Roedel]

Hi,

here are a couple of unit-tests for the KVM unit test framework. These tests
check for the sel_cr0 bug I fixes recently and also add a number of tests for
the NPT code.
Please have a look at them and write me your thoughts (or just pick them up ;-) )

Thanks,

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

[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 |   59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 59 insertions(+), 0 deletions(-)

diff --git a/x86/svm.c b/x86/svm.c
index e65360e..0e15819 100644
--- a/x86/svm.c
+++ b/x86/svm.c
@@ -6,12 +6,66 @@
 #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 2M pages
+     */
+
+    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 +110,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 0e15819..0fa9fec 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;
@@ -68,6 +69,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)
 {
@@ -450,6 +462,29 @@ static bool sel_cr0_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 },
@@ -472,6 +507,8 @@ static struct test tests[] = {
        default_finished, check_asid_zero },
     { "sel_cr0", default_supported, sel_cr0_prepare, sel_cr0_test,
        sel_cr0_finished, sel_cr0_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 0fa9fec..a0f465f 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;
 
@@ -485,6 +488,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 },
@@ -508,7 +538,9 @@ static struct test tests[] = {
     { "sel_cr0", default_supported, sel_cr0_prepare, sel_cr0_test,
        sel_cr0_finished, sel_cr0_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 a0f465f..8711862 100644
--- a/x86/svm.c
+++ b/x86/svm.c
@@ -515,6 +515,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 },
@@ -541,6 +557,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 8711862..c520599 100644
--- a/x86/svm.c
+++ b/x86/svm.c
@@ -531,6 +531,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 },
@@ -559,6 +587,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 c520599..5dc6673 100644
--- a/x86/svm.c
+++ b/x86/svm.c
@@ -559,6 +559,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 },
@@ -589,6 +611,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 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

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

[Avi Kivity]

  On 09/10/2010 06:34 PM, Joerg Roedel wrote:
> This patch adds code to setup a nested page table which is
> used for all tests.
>

> +
> +    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 2M pages
> +     */
> +
> +    address = 0;
> +
> +    /* PTE level */

Conflicts with previous comment - these aren't 2M pages.

> +    for (i = 0; i<  2048; ++i) {
> +        page = alloc_page();
> +
> +        for (j = 0; j<  512; ++j, address += 4096)
> +            page[j] = address | 0x067ULL;

{ }

>   static void vmcb_set_seg(struct vmcb_seg *seg, u16 selector,
> @@ -56,6 +110,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 {


-- 
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 2/7] svm: Run tests with NPT enabled if available

[Joerg Roedel]

On Sun, Sep 12, 2010 at 11:10:50AM +0200, Avi Kivity wrote:
>  On 09/10/2010 06:34 PM, Joerg Roedel wrote:
>> This patch adds code to setup a nested page table which is
>> used for all tests.
>>
>
>> +
>> +    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 2M pages
>> +     */
>> +
>> +    address = 0;
>> +
>> +    /* PTE level */
>
> Conflicts with previous comment - these aren't 2M pages.

Oh right, I used 2M pages in the first version but figured out then that
2MB are not granular enough to write all the NPT tests. Thus I changed
it to 4k pages and forgot to update the comment. I will resend this one.

	Joerg

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