[kvm-unit-tests PATCH v4 0/3] Add additional testing for routing L2 exceptions

[kvm-unit-tests PATCH v4 0/3] Add additional testing for routing L2 exceptions

[Aaron Lewis]

In a previous series testing was added to verify that when a #PF occured
in L2 the exception was routed to the correct place.  In this series
other exceptions are tested (ie: #GP, #UD, #DE, #DB, #BP, #AC).

v3 -> v4:
 - Add vmx_exception_test to vmx.

v2 -> v3:
 - Commits 1 and 2 from v2 were accepted upstream (bug fixes).
 - Moved exception_mnemonic() into a separate commit.
 - Moved support for running a nested guest multiple times in
   one test into a separate commit.
 - Moved the test framework into the same commit as the test itself.
 - Simplified the test framework and test code based on Sean's
   recommendations.

v1 -> v2:
 - Add guest_stack_top and guest_syscall_stack_top for aligning L2's
   stacks.
 - Refactor test to make it more extensible (ie: Added
   vmx_exception_tests array and framework around it).
 - Split test into 2 commits:
   1. Test infrustructure.
   2. Test cases.

Aaron Lewis (3):
  x86: Make exception_mnemonic() visible to the tests
  x86: Add support for running a nested guest multiple times in one test
  x86: Add test coverage for nested_vmx_reflect_vmexit() testing

 lib/x86/desc.c    |   2 +-
 lib/x86/desc.h    |   1 +
 x86/unittests.cfg |   9 +++-
 x86/vmx.c         |  24 ++++++++-
 x86/vmx.h         |   2 +
 x86/vmx_tests.c   | 129 ++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 163 insertions(+), 4 deletions(-)

-- 
2.35.0.rc0.227.g00780c9af4-goog

[kvm-unit-tests PATCH v4 1/3] x86: Make exception_mnemonic() visible to the tests

[Aaron Lewis]

exception_mnemonic() is a useful function for more than just desc.c.
Make it global, so it can be used in other KUT tests.

Signed-off-by: Aaron Lewis <aaronlewis@google.com>
---
 lib/x86/desc.c | 2 +-
 lib/x86/desc.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/x86/desc.c b/lib/x86/desc.c
index 16b7256..c2eb16e 100644
--- a/lib/x86/desc.c
+++ b/lib/x86/desc.c
@@ -91,7 +91,7 @@ struct ex_record {
 
 extern struct ex_record exception_table_start, exception_table_end;
 
-static const char* exception_mnemonic(int vector)
+const char* exception_mnemonic(int vector)
 {
 	switch(vector) {
 	case 0: return "#DE";
diff --git a/lib/x86/desc.h b/lib/x86/desc.h
index 9b81da0..ad6277b 100644
--- a/lib/x86/desc.h
+++ b/lib/x86/desc.h
@@ -224,6 +224,7 @@ void set_intr_alt_stack(int e, void *fn);
 void print_current_tss_info(void);
 handler handle_exception(u8 v, handler fn);
 void unhandled_exception(struct ex_regs *regs, bool cpu);
+const char* exception_mnemonic(int vector);
 
 bool test_for_exception(unsigned int ex, void (*trigger_func)(void *data),
 			void *data);
-- 
2.35.0.rc0.227.g00780c9af4-goog

[kvm-unit-tests PATCH v4 2/3] x86: Add support for running a nested guest multiple times in one test

[Aaron Lewis]

KUT has a limit of only being able to run one nested guest per vmx test.
This is limiting and not necessary.  Add support for allowing a test to
run guest code multiple times.

Signed-off-by: Aaron Lewis <aaronlewis@google.com>
---
 x86/vmx.c | 24 ++++++++++++++++++++++--
 x86/vmx.h |  2 ++
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/x86/vmx.c b/x86/vmx.c
index f4fbb94..51eed8c 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -1884,15 +1884,35 @@ void test_add_teardown(test_teardown_func func, void *data)
 	step->data = data;
 }
 
+static void __test_set_guest(test_guest_func func)
+{
+	assert(current->v2);
+	v2_guest_main = func;
+}
+
 /*
  * Set the target of the first enter_guest call. Can only be called once per
  * test. Must be called before first enter_guest call.
  */
 void test_set_guest(test_guest_func func)
 {
-	assert(current->v2);
 	TEST_ASSERT_MSG(!v2_guest_main, "Already set guest func.");
-	v2_guest_main = func;
+	__test_set_guest(func);
+}
+
+/*
+ * Set the target of the enter_guest call and reset the RIP so 'func' will
+ * start from the beginning.  This can be called multiple times per test.
+ */
+void test_override_guest(test_guest_func func)
+{
+	__test_set_guest(func);
+	init_vmcs_guest();
+}
+
+void test_set_guest_finished(void)
+{
+	guest_finished = 1;
 }
 
 static void check_for_guest_termination(union exit_reason exit_reason)
diff --git a/x86/vmx.h b/x86/vmx.h
index 4423986..11cb665 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -1055,7 +1055,9 @@ void hypercall(u32 hypercall_no);
 typedef void (*test_guest_func)(void);
 typedef void (*test_teardown_func)(void *data);
 void test_set_guest(test_guest_func func);
+void test_override_guest(test_guest_func func);
 void test_add_teardown(test_teardown_func func, void *data);
 void test_skip(const char *msg);
+void test_set_guest_finished(void);
 
 #endif
-- 
2.35.0.rc0.227.g00780c9af4-goog

[kvm-unit-tests PATCH v4 3/3] x86: Add test coverage for nested_vmx_reflect_vmexit() testing

[Aaron Lewis]

Add a framework and test cases to ensure exceptions that occur in L2 are
forwarded to the correct place by nested_vmx_reflect_vmexit().

Add testing for exceptions: #GP, #UD, #DE, #DB, #BP, and #AC.

Signed-off-by: Aaron Lewis <aaronlewis@google.com>
Change-Id: I0196071571671f06165983b5055ed7382fa3e1fb
---
 x86/unittests.cfg |   9 +++-
 x86/vmx_tests.c   | 129 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 137 insertions(+), 1 deletion(-)

diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index 9a70ba3..6ec7a98 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -288,7 +288,7 @@ arch = i386
 
 [vmx]
 file = vmx.flat
-extra_params = -cpu max,+vmx -append "-exit_monitor_from_l2_test -ept_access* -vmx_smp* -vmx_vmcs_shadow_test -atomic_switch_overflow_msrs_test -vmx_init_signal_test -vmx_apic_passthrough_tpr_threshold_test -apic_reg_virt_test -virt_x2apic_mode_test -vmx_pf_exception_test -vmx_pf_no_vpid_test -vmx_pf_invvpid_test -vmx_pf_vpid_test"
+extra_params = -cpu max,+vmx -append "-exit_monitor_from_l2_test -ept_access* -vmx_smp* -vmx_vmcs_shadow_test -atomic_switch_overflow_msrs_test -vmx_init_signal_test -vmx_apic_passthrough_tpr_threshold_test -apic_reg_virt_test -virt_x2apic_mode_test -vmx_pf_exception_test -vmx_pf_no_vpid_test -vmx_pf_invvpid_test -vmx_pf_vpid_test -vmx_exception_test"
 arch = x86_64
 groups = vmx
 
@@ -390,6 +390,13 @@ arch = x86_64
 groups = vmx nested_exception
 check = /sys/module/kvm_intel/parameters/allow_smaller_maxphyaddr=Y
 
+[vmx_exception_test]
+file = vmx.flat
+extra_params = -cpu max,+vmx -append vmx_exception_test
+arch = x86_64
+groups = vmx nested_exception
+timeout = 10
+
 [debug]
 file = debug.flat
 arch = x86_64
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 3d57ed6..af6f33b 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -21,6 +21,7 @@
 #include "smp.h"
 #include "delay.h"
 #include "access.h"
+#include "x86/usermode.h"
 
 #define VPID_CAP_INVVPID_TYPES_SHIFT 40
 
@@ -10701,6 +10702,133 @@ static void vmx_pf_vpid_test(void)
 	__vmx_pf_vpid_test(invalidate_tlb_new_vpid, 1);
 }
 
+static void vmx_l2_gp_test(void)
+{
+	*(volatile u64 *)NONCANONICAL = 0;
+}
+
+static void vmx_l2_ud_test(void)
+{
+	asm volatile ("ud2");
+}
+
+static void vmx_l2_de_test(void)
+{
+	asm volatile (
+		"xor %%eax, %%eax\n\t"
+		"xor %%ebx, %%ebx\n\t"
+		"xor %%edx, %%edx\n\t"
+		"idiv %%ebx\n\t"
+		::: "eax", "ebx", "edx");
+}
+
+static void vmx_l2_bp_test(void)
+{
+	asm volatile ("int3");
+}
+
+static void vmx_l2_db_test(void)
+{
+	write_rflags(read_rflags() | X86_EFLAGS_TF);
+}
+
+static uint64_t usermode_callback(void)
+{
+	/* Trigger an #AC by writing 8 bytes to a 4-byte aligned address. */
+	asm volatile(
+		"sub $0x10, %rsp\n\t"
+		"movq $0, 0x4(%rsp)\n\t"
+		"add $0x10, %rsp\n\t");
+
+	return 0;
+}
+
+static void vmx_l2_ac_test(void)
+{
+	bool raised_vector = false;
+
+	write_cr0(read_cr0() | X86_CR0_AM);
+	write_rflags(read_rflags() | X86_EFLAGS_AC);
+
+	run_in_user(usermode_callback, AC_VECTOR, 0, 0, 0, 0, &raised_vector);
+	report(raised_vector, "#AC vector raised from usermode in L2");
+	vmcall();
+}
+
+struct vmx_exception_test {
+	u8 vector;
+	void (*guest_code)(void);
+};
+
+struct vmx_exception_test vmx_exception_tests[] = {
+	{ GP_VECTOR, vmx_l2_gp_test },
+	{ UD_VECTOR, vmx_l2_ud_test },
+	{ DE_VECTOR, vmx_l2_de_test },
+	{ DB_VECTOR, vmx_l2_db_test },
+	{ BP_VECTOR, vmx_l2_bp_test },
+	{ AC_VECTOR, vmx_l2_ac_test },
+};
+
+static u8 vmx_exception_test_vector;
+
+static void vmx_exception_handler(struct ex_regs *regs)
+{
+	report(regs->vector == vmx_exception_test_vector,
+	       "Handling %s in L2's exception handler",
+	       exception_mnemonic(vmx_exception_test_vector));
+	vmcall();
+}
+
+static void handle_exception_in_l2(u8 vector)
+{
+	handler old_handler = handle_exception(vector, vmx_exception_handler);
+
+	vmx_exception_test_vector = vector;
+
+	enter_guest();
+	report(vmcs_read(EXI_REASON) == VMX_VMCALL,
+	       "%s handled by L2", exception_mnemonic(vector));
+
+	handle_exception(vector, old_handler);
+}
+
+static void handle_exception_in_l1(u32 vector)
+{
+	u32 old_eb = vmcs_read(EXC_BITMAP);
+
+	vmcs_write(EXC_BITMAP, old_eb | (1u << vector));
+
+	enter_guest();
+
+	report((vmcs_read(EXI_REASON) == VMX_EXC_NMI) &&
+	       ((vmcs_read(EXI_INTR_INFO) & 0xff) == vector),
+	       "%s handled by L1", exception_mnemonic(vector));
+
+	vmcs_write(EXC_BITMAP, old_eb);
+}
+
+static void vmx_exception_test(void)
+{
+	struct vmx_exception_test *t;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(vmx_exception_tests); i++) {
+		t = &vmx_exception_tests[i];
+
+		/*
+		 * Override the guest code before each run even though it's the
+		 * same code, the VMCS guest state needs to be reinitialized.
+		 */
+		test_override_guest(t->guest_code);
+		handle_exception_in_l2(t->vector);
+
+		test_override_guest(t->guest_code);
+		handle_exception_in_l1(t->vector);
+	}
+
+	test_set_guest_finished();
+}
+
 #define TEST(name) { #name, .v2 = name }
 
 /* name/init/guest_main/exit_handler/syscall_handler/guest_regs */
@@ -10810,5 +10938,6 @@ struct vmx_test vmx_tests[] = {
 	TEST(vmx_pf_no_vpid_test),
 	TEST(vmx_pf_invvpid_test),
 	TEST(vmx_pf_vpid_test),
+	TEST(vmx_exception_test),
 	{ NULL, NULL, NULL, NULL, NULL, {0} },
 };
-- 
2.35.0.rc0.227.g00780c9af4-goog

Re: [kvm-unit-tests PATCH v4 1/3] x86: Make exception_mnemonic() visible to the tests

[Sean Christopherson]

On Fri, Jan 21, 2022, Aaron Lewis wrote:
> exception_mnemonic() is a useful function for more than just desc.c.
> Make it global, so it can be used in other KUT tests.
> 
> Signed-off-by: Aaron Lewis <aaronlewis@google.com>
> ---

Reviewed-by: Sean Christopherson <seanjc@google.com>

Re: [kvm-unit-tests PATCH v4 2/3] x86: Add support for running a nested guest multiple times in one test

[Sean Christopherson]

On Fri, Jan 21, 2022, Aaron Lewis wrote:
> KUT has a limit of only being able to run one nested guest per vmx test.
> This is limiting and not necessary.  Add support for allowing a test to
> run guest code multiple times.
> 
> Signed-off-by: Aaron Lewis <aaronlewis@google.com>
> ---
>  x86/vmx.c | 24 ++++++++++++++++++++++--
>  x86/vmx.h |  2 ++
>  2 files changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/x86/vmx.c b/x86/vmx.c
> index f4fbb94..51eed8c 100644
> --- a/x86/vmx.c
> +++ b/x86/vmx.c
> @@ -1884,15 +1884,35 @@ void test_add_teardown(test_teardown_func func, void *data)
>  	step->data = data;
>  }
>  
> +static void __test_set_guest(test_guest_func func)
> +{
> +	assert(current->v2);
> +	v2_guest_main = func;
> +}
> +
>  /*
>   * Set the target of the first enter_guest call. Can only be called once per
>   * test. Must be called before first enter_guest call.
>   */
>  void test_set_guest(test_guest_func func)
>  {
> -	assert(current->v2);
>  	TEST_ASSERT_MSG(!v2_guest_main, "Already set guest func.");
> -	v2_guest_main = func;
> +	__test_set_guest(func);
> +}
> +
> +/*
> + * Set the target of the enter_guest call and reset the RIP so 'func' will
> + * start from the beginning.  This can be called multiple times per test.
> + */
> +void test_override_guest(test_guest_func func)
> +{
> +	__test_set_guest(func);
> +	init_vmcs_guest();
> +}
> +
> +void test_set_guest_finished(void)
> +{
> +	guest_finished = 1;

Adding test_set_guest_finished() should be a separate commit.

>  }
>  
>  static void check_for_guest_termination(union exit_reason exit_reason)
> diff --git a/x86/vmx.h b/x86/vmx.h
> index 4423986..11cb665 100644
> --- a/x86/vmx.h
> +++ b/x86/vmx.h
> @@ -1055,7 +1055,9 @@ void hypercall(u32 hypercall_no);
>  typedef void (*test_guest_func)(void);
>  typedef void (*test_teardown_func)(void *data);
>  void test_set_guest(test_guest_func func);
> +void test_override_guest(test_guest_func func);
>  void test_add_teardown(test_teardown_func func, void *data);
>  void test_skip(const char *msg);
> +void test_set_guest_finished(void);
>  
>  #endif
> -- 
> 2.35.0.rc0.227.g00780c9af4-goog
> 

Re: [kvm-unit-tests PATCH v4 3/3] x86: Add test coverage for nested_vmx_reflect_vmexit() testing

[Sean Christopherson]

On Fri, Jan 21, 2022, Aaron Lewis wrote:
> Add a framework and test cases to ensure exceptions that occur in L2 are
> forwarded to the correct place by nested_vmx_reflect_vmexit().
> 
> Add testing for exceptions: #GP, #UD, #DE, #DB, #BP, and #AC.
> 
> Signed-off-by: Aaron Lewis <aaronlewis@google.com>
> Change-Id: I0196071571671f06165983b5055ed7382fa3e1fb

Don't forget to strip the Change-Id before posting.

> ---
>  x86/unittests.cfg |   9 +++-
>  x86/vmx_tests.c   | 129 ++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 137 insertions(+), 1 deletion(-)
> 
> diff --git a/x86/unittests.cfg b/x86/unittests.cfg
> index 9a70ba3..6ec7a98 100644
> --- a/x86/unittests.cfg
> +++ b/x86/unittests.cfg
> @@ -288,7 +288,7 @@ arch = i386
>  
>  [vmx]
>  file = vmx.flat
> -extra_params = -cpu max,+vmx -append "-exit_monitor_from_l2_test -ept_access* -vmx_smp* -vmx_vmcs_shadow_test -atomic_switch_overflow_msrs_test -vmx_init_signal_test -vmx_apic_passthrough_tpr_threshold_test -apic_reg_virt_test -virt_x2apic_mode_test -vmx_pf_exception_test -vmx_pf_no_vpid_test -vmx_pf_invvpid_test -vmx_pf_vpid_test"
> +extra_params = -cpu max,+vmx -append "-exit_monitor_from_l2_test -ept_access* -vmx_smp* -vmx_vmcs_shadow_test -atomic_switch_overflow_msrs_test -vmx_init_signal_test -vmx_apic_passthrough_tpr_threshold_test -apic_reg_virt_test -virt_x2apic_mode_test -vmx_pf_exception_test -vmx_pf_no_vpid_test -vmx_pf_invvpid_test -vmx_pf_vpid_test -vmx_exception_test"
>  arch = x86_64
>  groups = vmx
>  
> @@ -390,6 +390,13 @@ arch = x86_64
>  groups = vmx nested_exception
>  check = /sys/module/kvm_intel/parameters/allow_smaller_maxphyaddr=Y
>  
> +[vmx_exception_test]
> +file = vmx.flat
> +extra_params = -cpu max,+vmx -append vmx_exception_test
> +arch = x86_64
> +groups = vmx nested_exception
> +timeout = 10

Leave this out (for now), including it in the main "vmx" test is sufficient.
I'm definitely in favor of splitting up the "vmx" behemoth, but it's probably
best to do that in a separate commit/series so that we can waste time bikeshedding
over how to organize things :-)

> +
>  [debug]
>  file = debug.flat
>  arch = x86_64
> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> index 3d57ed6..af6f33b 100644
> --- a/x86/vmx_tests.c
> +++ b/x86/vmx_tests.c
> @@ -21,6 +21,7 @@
>  #include "smp.h"
>  #include "delay.h"
>  #include "access.h"
> +#include "x86/usermode.h"
>  
>  #define VPID_CAP_INVVPID_TYPES_SHIFT 40
>  
> @@ -10701,6 +10702,133 @@ static void vmx_pf_vpid_test(void)
>  	__vmx_pf_vpid_test(invalidate_tlb_new_vpid, 1);
>  }
>  
> +static void vmx_l2_gp_test(void)
> +{
> +	*(volatile u64 *)NONCANONICAL = 0;
> +}
> +
> +static void vmx_l2_ud_test(void)
> +{
> +	asm volatile ("ud2");
> +}
> +
> +static void vmx_l2_de_test(void)
> +{
> +	asm volatile (
> +		"xor %%eax, %%eax\n\t"
> +		"xor %%ebx, %%ebx\n\t"
> +		"xor %%edx, %%edx\n\t"
> +		"idiv %%ebx\n\t"
> +		::: "eax", "ebx", "edx");
> +}
> +
> +static void vmx_l2_bp_test(void)
> +{
> +	asm volatile ("int3");
> +}
> +
> +static void vmx_l2_db_test(void)
> +{
> +	write_rflags(read_rflags() | X86_EFLAGS_TF);
> +}
> +
> +static uint64_t usermode_callback(void)
> +{
> +	/* Trigger an #AC by writing 8 bytes to a 4-byte aligned address. */
> +	asm volatile(
> +		"sub $0x10, %rsp\n\t"
> +		"movq $0, 0x4(%rsp)\n\t"
> +		"add $0x10, %rsp\n\t");

Sorry, didn't look closely at this before.  This can simply be:

	asm volatile("movq $0, 0x4(%rsp)\n\t");

as the access is expected to fault.  Or if you want to be paranoid about not
overwriting the stack:

	asm volatile("movq $0, -0x4(%rsp)\n\t");

It's probably also a good idea to call out that the stack is aligned on a 16-byte
boundary.  If you were trying to guarnatee alignment, then you would need to use
AND instead of SUB.  E.g.

        asm volatile("push  %rbp\n\t"
                     "movq  %rsp, %rbp\n\t"
                     "andq  $-0x10, %rsp\n\t"
                     "movq  $0, -0x4(%rsp)\n\t"
                     "movq  %rbp, %rsp\n\t"
                     "popq  %rbp\n\t");

But my vote would be to just add a comment, I would consider it a test bug if the
stack isn't properly aligned.

> +
> +	return 0;
> +}
> +
> +static void vmx_l2_ac_test(void)
> +{
> +	bool raised_vector = false;

Nit, hit_ac or so is more intuive.

> +
> +	write_cr0(read_cr0() | X86_CR0_AM);
> +	write_rflags(read_rflags() | X86_EFLAGS_AC);
> +
> +	run_in_user(usermode_callback, AC_VECTOR, 0, 0, 0, 0, &raised_vector);
> +	report(raised_vector, "#AC vector raised from usermode in L2");

And continuing the nits, "Usermode #AC handled in L2".

> +	vmcall();
> +}
> +

Re: [kvm-unit-tests PATCH v4 3/3] x86: Add test coverage for nested_vmx_reflect_vmexit() testing

[Aaron Lewis]

On Fri, Jan 21, 2022 at 10:00 AM Sean Christopherson <seanjc@google.com> wrote:
>
> On Fri, Jan 21, 2022, Aaron Lewis wrote:
> > Add a framework and test cases to ensure exceptions that occur in L2 are
> > forwarded to the correct place by nested_vmx_reflect_vmexit().
> >
> > Add testing for exceptions: #GP, #UD, #DE, #DB, #BP, and #AC.
> >
> > Signed-off-by: Aaron Lewis <aaronlewis@google.com>
> > Change-Id: I0196071571671f06165983b5055ed7382fa3e1fb
>
> Don't forget to strip the Change-Id before posting.

D'oh... Good catch.

>
> > ---
> >  x86/unittests.cfg |   9 +++-
> >  x86/vmx_tests.c   | 129 ++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 137 insertions(+), 1 deletion(-)
> >
> > diff --git a/x86/unittests.cfg b/x86/unittests.cfg
> > index 9a70ba3..6ec7a98 100644
> > --- a/x86/unittests.cfg
> > +++ b/x86/unittests.cfg
> > @@ -288,7 +288,7 @@ arch = i386

> > +[vmx_exception_test]
> > +file = vmx.flat
> > +extra_params = -cpu max,+vmx -append vmx_exception_test
> > +arch = x86_64
> > +groups = vmx nested_exception
> > +timeout = 10
>
> Leave this out (for now), including it in the main "vmx" test is sufficient.
> I'm definitely in favor of splitting up the "vmx" behemoth, but it's probably
> best to do that in a separate commit/series so that we can waste time bikeshedding
> over how to organize things :-)
>

Why leave this out when vmx_pf_exception_test, vmx_pf_no_vpid_test,
vmx_pf_invvpid_test, and vmx_pf_vpid_test have their own?  They seem
similar to me.

> > +
> > +static uint64_t usermode_callback(void)
> > +{
> > +     /* Trigger an #AC by writing 8 bytes to a 4-byte aligned address. */
> > +     asm volatile(
> > +             "sub $0x10, %rsp\n\t"
> > +             "movq $0, 0x4(%rsp)\n\t"
> > +             "add $0x10, %rsp\n\t");
>
> Sorry, didn't look closely at this before.  This can simply be:
>
>         asm volatile("movq $0, 0x4(%rsp)\n\t");
>
> as the access is expected to fault.  Or if you want to be paranoid about not
> overwriting the stack:
>
>         asm volatile("movq $0, -0x4(%rsp)\n\t");
>
> It's probably also a good idea to call out that the stack is aligned on a 16-byte
> boundary.  If you were trying to guarnatee alignment, then you would need to use
> AND instead of SUB.  E.g.
>
>         asm volatile("push  %rbp\n\t"
>                      "movq  %rsp, %rbp\n\t"
>                      "andq  $-0x10, %rsp\n\t"
>                      "movq  $0, -0x4(%rsp)\n\t"
>                      "movq  %rbp, %rsp\n\t"
>                      "popq  %rbp\n\t");
>
> But my vote would be to just add a comment, I would consider it a test bug if the
> stack isn't properly aligned.
>

I can improve the comment, and I agree that it would be a test bug if
the stack isn't properly aligned.

I'll switch this to using the more paranoid approach if I'm not going
to modify RSP.

> > +
> > +     return 0;
> > +}
> > +

Re: [kvm-unit-tests PATCH v4 3/3] x86: Add test coverage for nested_vmx_reflect_vmexit() testing

[Paolo Bonzini]

On 1/21/22 20:12, Aaron Lewis wrote:
>>> +[vmx_exception_test]
>>> +file = vmx.flat
>>> +extra_params = -cpu max,+vmx -append vmx_exception_test
>>> +arch = x86_64
>>> +groups = vmx nested_exception
>>> +timeout = 10
>> Leave this out (for now), including it in the main "vmx" test is sufficient.
>> I'm definitely in favor of splitting up the "vmx" behemoth, but it's probably
>> best to do that in a separate commit/series so that we can waste time bikeshedding
>> over how to organize things:-)
>>
> Why leave this out when vmx_pf_exception_test, vmx_pf_no_vpid_test,
> vmx_pf_invvpid_test, and vmx_pf_vpid_test have their own?  They seem
> similar to me.
> 

Because they take a very long time (minutes).

Paolo