[PATCH] nSVM: Test: Test VMRUN's canonicalization of segement base addresses

[PATCH] nSVM: Test: Test VMRUN's canonicalization of segement base addresses

[Krish Sadhukhan]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=yes, Size: 1904 bytes --]

According to section "Canonicalization and Consistency Checks" in APM vol 2,

    VMRUN canonicalizes (i.e., sign-extend to bit 63) all base addresses
    in the segment registers that have been loaded.

Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
---
 x86/svm_tests.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/x86/svm_tests.c b/x86/svm_tests.c
index d689e73..8387bea 100644
--- a/x86/svm_tests.c
+++ b/x86/svm_tests.c
@@ -2499,6 +2499,34 @@ static void test_msrpm_iopm_bitmap_addrs(void)
 	vmcb->control.intercept = saved_intercept;
 }
 
+#define TEST_CANONICAL(seg_base, msg)					\
+	saved_addr = seg_base;						\
+	seg_base = (seg_base & ((1ul << addr_limit) - 1)) | noncanonical_mask; \
+	report(svm_vmrun() == SVM_EXIT_VMMCALL, "Test %s.base for canonical form: %lx", msg, seg_base);							\
+	seg_base = saved_addr;
+
+/*
+ * VMRUN canonicalizes (i.e., sign-extend to bit 63) all base addresses
+ • in the segment registers that have been loaded.
+ */
+static void test_vmrun_canonicalization(void)
+{
+	u64 saved_addr;
+	u8 addr_limit = cpuid_maxphyaddr();
+	u64 noncanonical_mask = NONCANONICAL & ~((1ul << addr_limit) - 1);
+
+	TEST_CANONICAL(vmcb->save.es.base, "ES");
+	TEST_CANONICAL(vmcb->save.cs.base, "CS");
+	TEST_CANONICAL(vmcb->save.ss.base, "SS");
+	TEST_CANONICAL(vmcb->save.ds.base, "DS");
+	TEST_CANONICAL(vmcb->save.fs.base, "FS");
+	TEST_CANONICAL(vmcb->save.gs.base, "GS");
+	TEST_CANONICAL(vmcb->save.gdtr.base, "GDTR");
+	TEST_CANONICAL(vmcb->save.ldtr.base, "LDTR");
+	TEST_CANONICAL(vmcb->save.idtr.base, "IDTR");
+	TEST_CANONICAL(vmcb->save.tr.base, "TR");
+}
+
 static void svm_guest_state_test(void)
 {
 	test_set_guest(basic_guest_main);
@@ -2508,6 +2536,7 @@ static void svm_guest_state_test(void)
 	test_cr4();
 	test_dr();
 	test_msrpm_iopm_bitmap_addrs();
+	test_vmrun_canonicalization();
 }
 
 
-- 
2.27.0

Re: [PATCH] nSVM: Test: Test VMRUN's canonicalization of segement base addresses

[Jim Mattson]

On Thu, May 20, 2021 at 3:06 PM Krish Sadhukhan
<krish.sadhukhan@oracle.com> wrote:
>
> According to section "Canonicalization and Consistency Checks" in APM vol 2,
>
>     VMRUN canonicalizes (i.e., sign-extend to bit 63) all base addresses
>     in the segment registers that have been loaded.
>
> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
> ---
>  x86/svm_tests.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>
> diff --git a/x86/svm_tests.c b/x86/svm_tests.c
> index d689e73..8387bea 100644
> --- a/x86/svm_tests.c
> +++ b/x86/svm_tests.c
> @@ -2499,6 +2499,34 @@ static void test_msrpm_iopm_bitmap_addrs(void)
>         vmcb->control.intercept = saved_intercept;
>  }
>
> +#define TEST_CANONICAL(seg_base, msg)                                  \
> +       saved_addr = seg_base;                                          \
> +       seg_base = (seg_base & ((1ul << addr_limit) - 1)) | noncanonical_mask; \
> +       report(svm_vmrun() == SVM_EXIT_VMMCALL, "Test %s.base for canonical form: %lx", msg, seg_base);                                                 \
> +       seg_base = saved_addr;

This is messy. Why not just set seg_base to NONCANONICAL before
svm_vmrun() and then check to see that it's equal to
canonicalize(NONCANONICAL) after #VMEXIT?

> +/*
> + * VMRUN canonicalizes (i.e., sign-extend to bit 63) all base addresses
> + • in the segment registers that have been loaded.
> + */
> +static void test_vmrun_canonicalization(void)
> +{
> +       u64 saved_addr;
> +       u8 addr_limit = cpuid_maxphyaddr();

What constitutes a canonical address depends on the maximum *virtual*
address width supported, not the maximum physical address width
supported.

> +       u64 noncanonical_mask = NONCANONICAL & ~((1ul << addr_limit) - 1);
> +
> +       TEST_CANONICAL(vmcb->save.es.base, "ES");
> +       TEST_CANONICAL(vmcb->save.cs.base, "CS");
> +       TEST_CANONICAL(vmcb->save.ss.base, "SS");
> +       TEST_CANONICAL(vmcb->save.ds.base, "DS");
> +       TEST_CANONICAL(vmcb->save.fs.base, "FS");
> +       TEST_CANONICAL(vmcb->save.gs.base, "GS");
> +       TEST_CANONICAL(vmcb->save.gdtr.base, "GDTR");
> +       TEST_CANONICAL(vmcb->save.ldtr.base, "LDTR");
> +       TEST_CANONICAL(vmcb->save.idtr.base, "IDTR");
> +       TEST_CANONICAL(vmcb->save.tr.base, "TR");

There are only 8 segment registers. GDTR and IDTR are not segment
registers. They may be canonicalized by VMRUN/#VMEXIT, but they are
not segment registers.

> +}
> +
>  static void svm_guest_state_test(void)
>  {
>         test_set_guest(basic_guest_main);
> @@ -2508,6 +2536,7 @@ static void svm_guest_state_test(void)
>         test_cr4();
>         test_dr();
>         test_msrpm_iopm_bitmap_addrs();
> +       test_vmrun_canonicalization();
>  }
>
>
> --
> 2.27.0
>