diff mbox series

[kvm-unit-tests] x86: nvmx: Check #NM VM-exit reflection

Message ID 20180914192257.259045-1-jmattson@google.com (mailing list archive)
State New, archived
Headers show
Series [kvm-unit-tests] x86: nvmx: Check #NM VM-exit reflection | expand

Commit Message

Jim Mattson Sept. 14, 2018, 7:22 p.m. UTC 
When L1 intercepts #NM exceptions encountered in L2, the #NM exception
should always be reflected from L0 to L1.

Signed-off-by: Jim Mattson <jmattson@google.com>
Reviewed-by: Peter Shier <pshier@google.com>
---
 lib/x86/processor.h |  1 +
 x86/unittests.cfg   |  6 +++++
 x86/vmx_tests.c     | 65 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 72 insertions(+)

Comments

Liran Alon Sept. 14, 2018, 7:34 p.m. UTC | #1 
> On 14 Sep 2018, at 22:22, Jim Mattson <jmattson@google.com> wrote:
> 
> When L1 intercepts #NM exceptions encountered in L2, the #NM exception
> should always be reflected from L0 to L1.

I would also reference the relevant KVM commit which fixes this issue.

> 
> Signed-off-by: Jim Mattson <jmattson@google.com>
> Reviewed-by: Peter Shier <pshier@google.com>
> ---
> lib/x86/processor.h |  1 +
> x86/unittests.cfg   |  6 +++++
> x86/vmx_tests.c     | 65 +++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 72 insertions(+)
> 
> diff --git a/lib/x86/processor.h b/lib/x86/processor.h
> index b8e884f..15237a5 100644
> --- a/lib/x86/processor.h
> +++ b/lib/x86/processor.h
> @@ -25,6 +25,7 @@
> 
> #define X86_CR0_PE     0x00000001
> #define X86_CR0_MP     0x00000002
> +#define X86_CR0_EM     0x00000004
> #define X86_CR0_TS     0x00000008
> #define X86_CR0_WP     0x00010000
> #define X86_CR0_AM     0x00040000
> diff --git a/x86/unittests.cfg b/x86/unittests.cfg
> index 9d39319..4a9702d 100644
> --- a/x86/unittests.cfg
> +++ b/x86/unittests.cfg
> @@ -554,6 +554,12 @@ extra_params = -cpu host,+vmx -m 2560 -append vmx_cr_load_test
> arch = x86_64
> groups = vmx
> 
> +[vmx_nm_test]
> +file = vmx.flat
> +extra_params = -cpu host,+vmx -m 2560 -append vmx_nm_test
> +arch = x86_64
> +groups = vmx
> +
> [vmx_eoi_bitmap_ioapic_scan]
> file = vmx.flat
> smp = 2
> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> index 0e9d900..5d4f39a 100644
> --- a/x86/vmx_tests.c
> +++ b/x86/vmx_tests.c
> @@ -4585,6 +4585,70 @@ static void vmx_cr_load_test(void)
> 	TEST_ASSERT(!write_cr4_checking(cr4 & ~X86_CR4_PCIDE));
> }
> 
> +static void vmx_nm_test_guest(void)
> +{
> +	write_cr0(read_cr0() | X86_CR0_TS);
> +	asm volatile("fnop");
> +}
> +
> +static void check_nm_exit(const char *test)
> +{
> +	u32 reason = vmcs_read(EXI_REASON);
> +	u32 intr_info = vmcs_read(EXI_INTR_INFO);
> +	const u32 expected = INTR_INFO_VALID_MASK | INTR_TYPE_HARD_EXCEPTION |
> +		NM_VECTOR;
> +
> +	report("%s", reason == VMX_EXC_NMI && intr_info == expected, test);
> +}
> +
> +/*
> + * This test checks that:
> + *
> + * (a) If L2 launches with CR0.TS clear, but later sets CR0.TS, then
> + *     a subsequent #NM VM-exit is reflected to L1.
> + *
> + * (b) If L2 launches with CR0.TS clear and CR0.EM set, then a
> + *     subsequent #NM VM-exit is reflected to L1.
> + */
> +static void vmx_nm_test(void)
> +{
> +	unsigned long cr0 = read_cr0();
> +
> +	test_set_guest(vmx_nm_test_guest);
> +
> +	/*
> +	 * L1 wants to intercept #NM exceptions encountered in L2.
> +	 */
> +	vmcs_write(EXC_BITMAP, 1 << NM_VECTOR);
> +
> +	/*
> +	 * Launch L2 with CR0.TS clear, but don't claim host ownership of
> +	 * any CR0 bits. L2 will set CR0.TS and then try to execute fnop,
> +	 * which will raise #NM. L0 should reflect the #NM VM-exit to L1.
> +	 */
> +	vmcs_write(CR0_MASK, 0);
> +	vmcs_write(GUEST_CR0, cr0 & ~X86_CR0_TS);
> +	enter_guest();
> +	check_nm_exit("fnop with CR0.TS set in L2 triggers #NM VM-exit to L1");
> +
> +	/*
> +	 * Re-enter L2 at the fnop instruction, with CR0.TS clear but
> +	 * CR0.EM set. The fnop will still raise #NM, and L0 should
> +	 * reflect the #NM VM-exit to L1.
> +	 */
> +	vmcs_write(GUEST_CR0, (cr0 & ~X86_CR0_TS) | X86_CR0_EM);
> +	enter_guest();
> +	check_nm_exit("fnop with CR0.EM set in L2 triggers #NM VM-exit to L1");
> +
> +	/*
> +	 * Re-enter L2 at the fnop instruction, with both CR0.TS and
> +	 * CR0.EM clear. There will be no #NM, and the L2 guest should
> +	 * exit normally.
> +	 */
> +	vmcs_write(GUEST_CR0, cr0 & ~(X86_CR0_TS | X86_CR0_EM));
> +	enter_guest();
> +}
> +
> static bool cpu_has_apicv(void)
> {
> 	return ((ctrl_cpu_rev[1].clr & CPU_APIC_REG_VIRT) &&
> @@ -5144,6 +5208,7 @@ struct vmx_test vmx_tests[] = {
> 	TEST(vmx_vmcs_shadow_test),
> 	/* Regression tests */
> 	TEST(vmx_cr_load_test),
> +	TEST(vmx_nm_test),
> 	/* EPT access tests. */
> 	TEST(ept_access_test_not_present),
> 	TEST(ept_access_test_read_only),
> -- 
> 2.19.0.397.gdd90340f6a-goog
> 

Looks good to me.
Reviewed-by: Liran Alon <liran.alon@oracle.com>
Jim Mattson Sept. 14, 2018, 7:40 p.m. UTC | #2 
On Fri, Sep 14, 2018 at 12:34 PM, Liran Alon <liran.alon@oracle.com> wrote:
>
>
>> On 14 Sep 2018, at 22:22, Jim Mattson <jmattson@google.com> wrote:
>>
>> When L1 intercepts #NM exceptions encountered in L2, the #NM exception
>> should always be reflected from L0 to L1.
>
> I would also reference the relevant KVM commit which fixes this issue.

Sadly, there isn't one yet. :-)
Krish Sadhukhan Sept. 17, 2018, 9:36 p.m. UTC | #3 
On 09/14/2018 12:22 PM, Jim Mattson wrote:
> When L1 intercepts #NM exceptions encountered in L2, the #NM exception
> should always be reflected from L0 to L1.
>
> Signed-off-by: Jim Mattson <jmattson@google.com>
> Reviewed-by: Peter Shier <pshier@google.com>
> ---
>   lib/x86/processor.h |  1 +
>   x86/unittests.cfg   |  6 +++++
>   x86/vmx_tests.c     | 65 +++++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 72 insertions(+)
>
> diff --git a/lib/x86/processor.h b/lib/x86/processor.h
> index b8e884f..15237a5 100644
> --- a/lib/x86/processor.h
> +++ b/lib/x86/processor.h
> @@ -25,6 +25,7 @@
>   
>   #define X86_CR0_PE     0x00000001
>   #define X86_CR0_MP     0x00000002
> +#define X86_CR0_EM     0x00000004
>   #define X86_CR0_TS     0x00000008
>   #define X86_CR0_WP     0x00010000
>   #define X86_CR0_AM     0x00040000
> diff --git a/x86/unittests.cfg b/x86/unittests.cfg
> index 9d39319..4a9702d 100644
> --- a/x86/unittests.cfg
> +++ b/x86/unittests.cfg
> @@ -554,6 +554,12 @@ extra_params = -cpu host,+vmx -m 2560 -append vmx_cr_load_test
>   arch = x86_64
>   groups = vmx
>   
> +[vmx_nm_test]
> +file = vmx.flat
> +extra_params = -cpu host,+vmx -m 2560 -append vmx_nm_test
> +arch = x86_64
> +groups = vmx
> +
>   [vmx_eoi_bitmap_ioapic_scan]
>   file = vmx.flat
>   smp = 2
> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> index 0e9d900..5d4f39a 100644
> --- a/x86/vmx_tests.c
> +++ b/x86/vmx_tests.c
> @@ -4585,6 +4585,70 @@ static void vmx_cr_load_test(void)
>   	TEST_ASSERT(!write_cr4_checking(cr4 & ~X86_CR4_PCIDE));
>   }
>   
> +static void vmx_nm_test_guest(void)
> +{
> +	write_cr0(read_cr0() | X86_CR0_TS);
> +	asm volatile("fnop");
> +}
> +
> +static void check_nm_exit(const char *test)
> +{
> +	u32 reason = vmcs_read(EXI_REASON);
> +	u32 intr_info = vmcs_read(EXI_INTR_INFO);
> +	const u32 expected = INTR_INFO_VALID_MASK | INTR_TYPE_HARD_EXCEPTION |
> +		NM_VECTOR;
> +
> +	report("%s", reason == VMX_EXC_NMI && intr_info == expected, test);
> +}
> +
> +/*
> + * This test checks that:
> + *
> + * (a) If L2 launches with CR0.TS clear, but later sets CR0.TS, then
> + *     a subsequent #NM VM-exit is reflected to L1.
> + *
> + * (b) If L2 launches with CR0.TS clear and CR0.EM set, then a
> + *     subsequent #NM VM-exit is reflected to L1.
> + */
> +static void vmx_nm_test(void)
> +{
> +	unsigned long cr0 = read_cr0();
> +
> +	test_set_guest(vmx_nm_test_guest);
> +
> +	/*
> +	 * L1 wants to intercept #NM exceptions encountered in L2.
> +	 */
> +	vmcs_write(EXC_BITMAP, 1 << NM_VECTOR);
> +
> +	/*
> +	 * Launch L2 with CR0.TS clear, but don't claim host ownership of
> +	 * any CR0 bits. L2 will set CR0.TS and then try to execute fnop,
> +	 * which will raise #NM. L0 should reflect the #NM VM-exit to L1.
> +	 */
> +	vmcs_write(CR0_MASK, 0);
> +	vmcs_write(GUEST_CR0, cr0 & ~X86_CR0_TS);
> +	enter_guest();
> +	check_nm_exit("fnop with CR0.TS set in L2 triggers #NM VM-exit to L1");
> +
> +	/*
> +	 * Re-enter L2 at the fnop instruction, with CR0.TS clear but
> +	 * CR0.EM set. The fnop will still raise #NM, and L0 should
> +	 * reflect the #NM VM-exit to L1.
> +	 */
> +	vmcs_write(GUEST_CR0, (cr0 & ~X86_CR0_TS) | X86_CR0_EM);
> +	enter_guest();
> +	check_nm_exit("fnop with CR0.EM set in L2 triggers #NM VM-exit to L1");
> +
> +	/*
> +	 * Re-enter L2 at the fnop instruction, with both CR0.TS and
> +	 * CR0.EM clear. There will be no #NM, and the L2 guest should
> +	 * exit normally.
> +	 */
> +	vmcs_write(GUEST_CR0, cr0 & ~(X86_CR0_TS | X86_CR0_EM));
> +	enter_guest();

There is another combination that we can test:

        "If the EM flag is set, the setting of the TS flag has no effect 
on the execution of x87 FPU/MMX/SSE/SSE2/SSE3/SSSE3/SSE4 instructions."

> +}
> +
>   static bool cpu_has_apicv(void)
>   {
>   	return ((ctrl_cpu_rev[1].clr & CPU_APIC_REG_VIRT) &&
> @@ -5144,6 +5208,7 @@ struct vmx_test vmx_tests[] = {
>   	TEST(vmx_vmcs_shadow_test),
>   	/* Regression tests */
>   	TEST(vmx_cr_load_test),
> +	TEST(vmx_nm_test),
>   	/* EPT access tests. */
>   	TEST(ept_access_test_not_present),
>   	TEST(ept_access_test_read_only),

Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
Jim Mattson Sept. 25, 2018, 8:42 p.m. UTC | #4 
On Mon, Sep 17, 2018 at 2:36 PM, Krish Sadhukhan
<krish.sadhukhan@oracle.com> wrote:

> There is another combination that we can test:
>
>        "If the EM flag is set, the setting of the TS flag has no effect on
> the execution of x87 FPU/MMX/SSE/SSE2/SSE3/SSSE3/SSE4 instructions."

Are you suggesting a test case that launches L2 with both CR0.EM and
CR0.TS set, to ensure that L1 still gets an #NM intercept? I'm happy
to add that.
diff mbox series

Patch

diff --git a/lib/x86/processor.h b/lib/x86/processor.h
index b8e884f..15237a5 100644
--- a/lib/x86/processor.h
+++ b/lib/x86/processor.h
@@ -25,6 +25,7 @@ 
 
 #define X86_CR0_PE     0x00000001
 #define X86_CR0_MP     0x00000002
+#define X86_CR0_EM     0x00000004
 #define X86_CR0_TS     0x00000008
 #define X86_CR0_WP     0x00010000
 #define X86_CR0_AM     0x00040000
diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index 9d39319..4a9702d 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -554,6 +554,12 @@  extra_params = -cpu host,+vmx -m 2560 -append vmx_cr_load_test
 arch = x86_64
 groups = vmx
 
+[vmx_nm_test]
+file = vmx.flat
+extra_params = -cpu host,+vmx -m 2560 -append vmx_nm_test
+arch = x86_64
+groups = vmx
+
 [vmx_eoi_bitmap_ioapic_scan]
 file = vmx.flat
 smp = 2
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 0e9d900..5d4f39a 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -4585,6 +4585,70 @@  static void vmx_cr_load_test(void)
 	TEST_ASSERT(!write_cr4_checking(cr4 & ~X86_CR4_PCIDE));
 }
 
+static void vmx_nm_test_guest(void)
+{
+	write_cr0(read_cr0() | X86_CR0_TS);
+	asm volatile("fnop");
+}
+
+static void check_nm_exit(const char *test)
+{
+	u32 reason = vmcs_read(EXI_REASON);
+	u32 intr_info = vmcs_read(EXI_INTR_INFO);
+	const u32 expected = INTR_INFO_VALID_MASK | INTR_TYPE_HARD_EXCEPTION |
+		NM_VECTOR;
+
+	report("%s", reason == VMX_EXC_NMI && intr_info == expected, test);
+}
+
+/*
+ * This test checks that:
+ *
+ * (a) If L2 launches with CR0.TS clear, but later sets CR0.TS, then
+ *     a subsequent #NM VM-exit is reflected to L1.
+ *
+ * (b) If L2 launches with CR0.TS clear and CR0.EM set, then a
+ *     subsequent #NM VM-exit is reflected to L1.
+ */
+static void vmx_nm_test(void)
+{
+	unsigned long cr0 = read_cr0();
+
+	test_set_guest(vmx_nm_test_guest);
+
+	/*
+	 * L1 wants to intercept #NM exceptions encountered in L2.
+	 */
+	vmcs_write(EXC_BITMAP, 1 << NM_VECTOR);
+
+	/*
+	 * Launch L2 with CR0.TS clear, but don't claim host ownership of
+	 * any CR0 bits. L2 will set CR0.TS and then try to execute fnop,
+	 * which will raise #NM. L0 should reflect the #NM VM-exit to L1.
+	 */
+	vmcs_write(CR0_MASK, 0);
+	vmcs_write(GUEST_CR0, cr0 & ~X86_CR0_TS);
+	enter_guest();
+	check_nm_exit("fnop with CR0.TS set in L2 triggers #NM VM-exit to L1");
+
+	/*
+	 * Re-enter L2 at the fnop instruction, with CR0.TS clear but
+	 * CR0.EM set. The fnop will still raise #NM, and L0 should
+	 * reflect the #NM VM-exit to L1.
+	 */
+	vmcs_write(GUEST_CR0, (cr0 & ~X86_CR0_TS) | X86_CR0_EM);
+	enter_guest();
+	check_nm_exit("fnop with CR0.EM set in L2 triggers #NM VM-exit to L1");
+
+	/*
+	 * Re-enter L2 at the fnop instruction, with both CR0.TS and
+	 * CR0.EM clear. There will be no #NM, and the L2 guest should
+	 * exit normally.
+	 */
+	vmcs_write(GUEST_CR0, cr0 & ~(X86_CR0_TS | X86_CR0_EM));
+	enter_guest();
+}
+
 static bool cpu_has_apicv(void)
 {
 	return ((ctrl_cpu_rev[1].clr & CPU_APIC_REG_VIRT) &&
@@ -5144,6 +5208,7 @@  struct vmx_test vmx_tests[] = {
 	TEST(vmx_vmcs_shadow_test),
 	/* Regression tests */
 	TEST(vmx_cr_load_test),
+	TEST(vmx_nm_test),
 	/* EPT access tests. */
 	TEST(ept_access_test_not_present),
 	TEST(ept_access_test_read_only),