On Tue, Dec 14, 2021, Aaron Lewis wrote:
> Set up a test framework that verifies an exception occurring in L2 is
> forwarded to the right place (L0?, L1?, L2?). To add a test to this
> framework just add the exception and callbacks to the
> vmx_exception_tests array.
The bulk of this patch belongs in patch 04. It's nearly impossible to properly
review the guts of vmx_exception_test() without seeing how the hooks are actually
used, and it also adds a test without any testcases, which is odd.
The introduction of test_set_guest_restartable() and test_set_guest_finished()
can and should go in a separate patch. It would be nice if a few existing tests
were converted to use test_set_guest_finished(), but certainly not necessary,
and that might cause too much scope screep.
Exposing exception_mnemonic() should also go in a separate patch. E.g. if someone
else is working on a test that wants to use exception_mnemonic(), then the two
in-flight series can share a single patch. That's not very likely to happen in
KUT, but it's good practice in general.
> This framework tests two things:
> 1) It tests that an exception is handled by L2.
> 2) It tests that an exception is handled by L1.
> To test that this happens, each exception is triggered twice; once with
> just an L2 exception handler registered, and again with both an L2
> exception handler registered and L1's exception bitmap set. The
> expectation is that the first exception will be handled by L2 and the
> second by L1.
>
> To implement this support was added to vmx.c to allow more than one
> L2 test be run in a single test. Previously there was a hard limit of
> only being allowed to set the L2 guest code once in a given test. That
> is no longer a limitation with the addition of
> test_set_guest_restartable().
>
> Support was also added to allow the test to complete without running
> through the entirety of the L2 guest code. Calling the function
> test_set_guest_finished() marks the guest code as completed, allowing
> it to end without running to the end.
>
> Signed-off-by: Aaron Lewis <aaronlewis@xxxxxxxxxx>
> ---
...
> diff --git a/x86/unittests.cfg b/x86/unittests.cfg
> index 9fcdcae..0353b69 100644
> --- a/x86/unittests.cfg
> +++ b/x86/unittests.cfg
> @@ -368,6 +368,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
Why add a new test case instead of folding this into "vmx"? It's quite speedy.
The "vmx" bucket definitely needs some cleanup, but I don't thinking adding a bunch
of one-off tests is the way forward.
> +
> [debug]
> file = debug.flat
> arch = x86_64
> diff --git a/x86/vmx.c b/x86/vmx.c
> index f4fbb94..9908746 100644
> --- a/x86/vmx.c
> +++ b/x86/vmx.c
> @@ -1895,6 +1895,23 @@ void test_set_guest(test_guest_func func)
> v2_guest_main = func;
> }
>
> +/*
> + * Set the target of the first enter_guest call and reset the RIP so 'func'
> + * will start from the beginning. This can be called multiple times per test.
> + */
> +void test_set_guest_restartable(test_guest_func func)
Hmm, "restartable" is somewhat confusing as it implies that other guests aren't
restartable, and sometimes people refer to resuming a guest after a VM-Exit as
"restarting" the guest. Maybe test_override_guest()?
> +{
> + assert(current->v2);
> + v2_guest_main = func;
These two lines can be shared with the existing test_set_guest(). It's kinda silly
since it's just two lines, but it is helpful to show the relationship between the
two helpers. E.g.
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)
{
TEST_ASSERT_MSG(!v2_guest_main, "Already set guest func.");
__test_set_guest(func);
}
void test_override_guest(test_guest_func func)
{
__test_set_guest(func);
init_vmcs_guest();
}
> + init_vmcs_guest();
> + guest_finished = 0;
This seems unnecessary, can't the test simply not set this flag? Ah, after
running and debugging, the issue is that vmx_l2_ac_test() doesn't do a vmcall()
and so that test runs to completion. As annoying as I find guest_entry to be, I
do think it's better to leave this alone and add the vmcall() to vmx_l2_ac_test().
> +}
> +
> +void test_set_guest_finished(void)
> +{
> + guest_finished = 1;
> +}
> +
> static void check_for_guest_termination(union exit_reason exit_reason)
> {
> if (is_hypercall(exit_reason)) {
> diff --git a/x86/vmx.h b/x86/vmx.h
> index 4423986..5321a7e 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_set_guest_restartable(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
> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> index 3d57ed6..018db2f 100644
> --- a/x86/vmx_tests.c
> +++ b/x86/vmx_tests.c
> @@ -10701,6 +10701,93 @@ static void vmx_pf_vpid_test(void)
> __vmx_pf_vpid_test(invalidate_tlb_new_vpid, 1);
> }
>
> +struct vmx_exception_test {
> + u8 vector;
> + void (*guest_code)(void);
> + void (*init_test)(void);
> + void (*uninit_test)(void);
The init/uninit helpers are unnecessary, the #DB test can instead set EFLAGS.TF
from L2 and then rely on test_override_guest() to restore vmcs.GUEST_RFLAGS. The
#AC test can do the same thing (stuff state without restoring). It's a little
gross, but it yields much cleaner code in the test loop and we're already relying
on test_override_guest() to restore guest state (RIP).
> +};
> +
> +struct vmx_exception_test vmx_exception_tests[] = {
> +};
> +
> +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));
> +
> + test_set_guest_finished();
Just call test_set_guest_finished() after all tests run.
> + handle_exception(vector, old_handler);
> +}
> +
> +static void handle_exception_in_l1(u32 vector)
> +{
> + handler old_handler = handle_exception(vector, vmx_exception_handler);
> + u32 old_eb = vmcs_read(EXC_BITMAP);
> +
> + vmx_exception_test_vector = 0xff;
No need to install the handler or set the vector, just let L2 expode on the
unexpected exception.
> +
> + 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));
> +
> + test_set_guest_finished();
> +
> + vmcs_write(EXC_BITMAP, old_eb);
> + handle_exception(vector, old_handler);
> +}
> +
> +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];
> +
> + TEST_ASSERT(t->guest_code);
Eh, I wouldn't bother, especially once (un)init_test go bye bye.
This is what I ended up with (pulling in code from the next patch):
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();
}
