| Message ID | 20161221202604.24248-1-rkrcmar@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 21/12/2016 21:26, Radim Krčmář wrote: > This patch adds a generator for simple VMCS tests, generates few tests > for recent changes, and also pulls in some header file dependencies. > > Signed-off-by: Radim Krčmář <rkrcmar@redhat.com> > --- > x86/vmx.h | 39 +++++++++++++++++++++++++++++++++++++++ > x86/vmx_tests.c | 41 +++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 80 insertions(+) > > diff --git a/x86/vmx.h b/x86/vmx.h > index a2bacd348161..4829f9135a02 100644 > --- a/x86/vmx.h > +++ b/x86/vmx.h > @@ -44,6 +44,45 @@ struct vmentry_failure { > unsigned long flags; > }; > > +/* > + * Exit Qualifications for entry failure during or after loading guest state > + */ > +#define ENTRY_FAIL_DEFAULT 0 > +#define ENTRY_FAIL_PDPTE 2 > +#define ENTRY_FAIL_NMI 3 > +#define ENTRY_FAIL_VMCS_LINK_PTR 4 > + > +/* > + * VM-instruction error numbers > + */ > +enum vm_instruction_error_number { > + VMXERR_VMCALL_IN_VMX_ROOT_OPERATION = 1, > + VMXERR_VMCLEAR_INVALID_ADDRESS = 2, > + VMXERR_VMCLEAR_VMXON_POINTER = 3, > + VMXERR_VMLAUNCH_NONCLEAR_VMCS = 4, > + VMXERR_VMRESUME_NONLAUNCHED_VMCS = 5, > + VMXERR_VMRESUME_AFTER_VMXOFF = 6, > + VMXERR_ENTRY_INVALID_CONTROL_FIELD = 7, > + VMXERR_ENTRY_INVALID_HOST_STATE_FIELD = 8, > + VMXERR_VMPTRLD_INVALID_ADDRESS = 9, > + VMXERR_VMPTRLD_VMXON_POINTER = 10, > + VMXERR_VMPTRLD_INCORRECT_VMCS_REVISION_ID = 11, > + VMXERR_UNSUPPORTED_VMCS_COMPONENT = 12, > + VMXERR_VMWRITE_READ_ONLY_VMCS_COMPONENT = 13, > + VMXERR_VMXON_IN_VMX_ROOT_OPERATION = 15, > + VMXERR_ENTRY_INVALID_EXECUTIVE_VMCS_POINTER = 16, > + VMXERR_ENTRY_NONLAUNCHED_EXECUTIVE_VMCS = 17, > + VMXERR_ENTRY_EXECUTIVE_VMCS_POINTER_NOT_VMXON_POINTER = 18, > + VMXERR_VMCALL_NONCLEAR_VMCS = 19, > + VMXERR_VMCALL_INVALID_VM_EXIT_CONTROL_FIELDS = 20, > + VMXERR_VMCALL_INCORRECT_MSEG_REVISION_ID = 22, > + VMXERR_VMXOFF_UNDER_DUAL_MONITOR_TREATMENT_OF_SMIS_AND_SMM = 23, > + VMXERR_VMCALL_INVALID_SMM_MONITOR_FEATURES = 24, > + VMXERR_ENTRY_INVALID_VM_EXECUTION_CONTROL_FIELDS_IN_EXECUTIVE_VMCS = 25, > + VMXERR_ENTRY_EVENTS_BLOCKED_BY_MOV_SS = 26, > + VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID = 28, > +}; > + > struct vmx_test { > const char *name; > int (*init)(struct vmcs *vmcs); > diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c > index 5fd95706c418..a1400b937254 100644 > --- a/x86/vmx_tests.c > +++ b/x86/vmx_tests.c > @@ -1816,6 +1816,42 @@ int into_exit_handler() > return VMX_TEST_VMEXIT; > } > > +#define __create_invalid_vmcs_test(name, value, test) \ No need for __ here. > + static int invalid_##name##_init() \ > + { \ > + vmcs_write(name, value); \ > + return VMX_TEST_START; \ > + } \ > + static int invalid_##name##_entry_failure(struct vmentry_failure *failure) \ > + { \ > + report("VM entry failure on invalid "#name, test); \ > + return VMX_TEST_VMEXIT; \ > + } > + > +#define create_invalid_host_vmcs_test(name, value, error) \ > + __create_invalid_vmcs_test(name, value, failure->early && \ > + vmcs_read(VMX_INST_ERROR) == error) > + > +#define create_invalid_guest_vmcs_test(name, value, reason, qualification) \ > + __create_invalid_vmcs_test(name, value, !failure->early && \ > + vmcs_read(EXI_REASON) == (reason | VMX_ENTRY_FAILURE) && \ > + vmcs_read(EXI_QUALIFICATION) == qualification) > + > +#define test_invalid_vmcs(name) \ > + { "invalid "#name, invalid_##name##_init, NULL, NULL, NULL, {0}, \ > + invalid_##name##_entry_failure } > + > +#define C create_invalid_host_vmcs_test > +C(HOST_EFER, -1ull, VMXERR_ENTRY_INVALID_HOST_STATE_FIELD) No need for the "C" macro here. > +#undef C > + > +#define C create_invalid_guest_vmcs_test > +C(VMCS_LINK_PTR, 1, VMX_FAIL_STATE, ENTRY_FAIL_VMCS_LINK_PTR) > +C(GUEST_CR0, -1ull, VMX_FAIL_STATE, ENTRY_FAIL_DEFAULT) > +C(GUEST_CR4, -1ull, VMX_FAIL_STATE, ENTRY_FAIL_DEFAULT) > +C(GUEST_EFER, -1ull, VMX_FAIL_STATE, ENTRY_FAIL_DEFAULT) Same here. These tests look a bit different from the others that use the "stage" mechanism. Perhaps you can have two tests only in vmx_tests, one for host failures and one for guest failures? Thanks, Paolo > +#undef C > + > /* name/init/guest_main/exit_handler/syscall_handler/guest_regs */ > struct vmx_test vmx_tests[] = { > { "null", NULL, basic_guest_main, basic_exit_handler, NULL, {0} }, > @@ -1845,5 +1881,10 @@ struct vmx_test vmx_tests[] = { > disable_rdtscp_exit_handler, NULL, {0} }, > { "int3", int3_init, int3_guest_main, int3_exit_handler, NULL, {0} }, > { "into", into_init, into_guest_main, into_exit_handler, NULL, {0} }, > + test_invalid_vmcs(VMCS_LINK_PTR), > + test_invalid_vmcs(GUEST_CR0), > + test_invalid_vmcs(GUEST_CR4), > + test_invalid_vmcs(GUEST_EFER), > + test_invalid_vmcs(HOST_EFER), > { NULL, NULL, NULL, NULL, NULL, {0} }, > }; > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2016-12-22 11:00+0100, Paolo Bonzini: > On 21/12/2016 21:26, Radim Krčmář wrote: >> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c >> index 5fd95706c418..a1400b937254 100644 >> --- a/x86/vmx_tests.c >> +++ b/x86/vmx_tests.c >> @@ -1816,6 +1816,42 @@ int into_exit_handler() >> return VMX_TEST_VMEXIT; >> } >> >> +#define __create_invalid_vmcs_test(name, value, test) \ > > No need for __ here. Ok. >> + static int invalid_##name##_init() \ >> + { \ >> + vmcs_write(name, value); \ >> + return VMX_TEST_START; \ >> + } \ >> + static int invalid_##name##_entry_failure(struct vmentry_failure *failure) \ >> + { \ >> + report("VM entry failure on invalid "#name, test); \ >> + return VMX_TEST_VMEXIT; \ >> + } >> + >> +#define create_invalid_host_vmcs_test(name, value, error) \ >> + __create_invalid_vmcs_test(name, value, failure->early && \ >> + vmcs_read(VMX_INST_ERROR) == error) >> + >> +#define create_invalid_guest_vmcs_test(name, value, reason, qualification) \ >> + __create_invalid_vmcs_test(name, value, !failure->early && \ >> + vmcs_read(EXI_REASON) == (reason | VMX_ENTRY_FAILURE) && \ >> + vmcs_read(EXI_QUALIFICATION) == qualification) >> + >> +#define test_invalid_vmcs(name) \ >> + { "invalid "#name, invalid_##name##_init, NULL, NULL, NULL, {0}, \ >> + invalid_##name##_entry_failure } >> + >> +#define C create_invalid_host_vmcs_test >> +C(HOST_EFER, -1ull, VMXERR_ENTRY_INVALID_HOST_STATE_FIELD) > > No need for the "C" macro here. > >> +#undef C >> + >> +#define C create_invalid_guest_vmcs_test >> +C(VMCS_LINK_PTR, 1, VMX_FAIL_STATE, ENTRY_FAIL_VMCS_LINK_PTR) >> +C(GUEST_CR0, -1ull, VMX_FAIL_STATE, ENTRY_FAIL_DEFAULT) >> +C(GUEST_CR4, -1ull, VMX_FAIL_STATE, ENTRY_FAIL_DEFAULT) >> +C(GUEST_EFER, -1ull, VMX_FAIL_STATE, ENTRY_FAIL_DEFAULT) > > Same here. I can change it into something like this: create_invalid_guest_vmcs_test(VMCS_LINK_PTR, 1, VMX_FAIL_STATE, ENTRY_FAIL_VMCS_LINK_PTR) create_invalid_guest_vmcs_test(GUEST_CR0, -1ull, VMX_FAIL_STATE, ENTRY_FAIL_DEFAULT) create_invalid_guest_vmcs_test(GUEST_CR4, -1ull, VMX_FAIL_STATE, ENTRY_FAIL_DEFAULT) create_invalid_guest_vmcs_test(GUEST_EFER, -1ull, VMX_FAIL_STATE, ENTRY_FAIL_DEFAULT) but not this: create_invalid_guest_vmcs_test(VMCS_LINK_PTR, 1, VMX_FAIL_STATE, ENTRY_FAIL_VMCS_LINK_PTR) create_invalid_guest_vmcs_test(GUEST_CR0, -1ull, VMX_FAIL_STATE, ENTRY_FAIL_DEFAULT) create_invalid_guest_vmcs_test(GUEST_CR4, -1ull, VMX_FAIL_STATE, ENTRY_FAIL_DEFAULT) create_invalid_guest_vmcs_test(GUEST_EFER, -1ull, VMX_FAIL_STATE, ENTRY_FAIL_DEFAULT) I assume we don't want to anger teletype users, so the macro shortcut is to improve readability. > These tests look a bit different from the others that use the "stage" > mechanism. Perhaps you can have two tests only in vmx_tests, one for > host failures and one for guest failures? I avoided stages because their explicit stateful nature was not needed. The output is different, though ... I'll post a version that looks better. >> +#undef C >> + >> /* name/init/guest_main/exit_handler/syscall_handler/guest_regs */ >> struct vmx_test vmx_tests[] = { >> { "null", NULL, basic_guest_main, basic_exit_handler, NULL, {0} }, >> @@ -1845,5 +1881,10 @@ struct vmx_test vmx_tests[] = { >> disable_rdtscp_exit_handler, NULL, {0} }, >> { "int3", int3_init, int3_guest_main, int3_exit_handler, NULL, {0} }, >> { "into", into_init, into_guest_main, into_exit_handler, NULL, {0} }, >> + test_invalid_vmcs(VMCS_LINK_PTR), >> + test_invalid_vmcs(GUEST_CR0), >> + test_invalid_vmcs(GUEST_CR4), >> + test_invalid_vmcs(GUEST_EFER), >> + test_invalid_vmcs(HOST_EFER), >> { NULL, NULL, NULL, NULL, NULL, {0} }, >> }; >> -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Dec 22, 2016 at 07:50:53PM +0100, Radim Krčmář wrote: > 2016-12-22 11:00+0100, Paolo Bonzini: > > On 21/12/2016 21:26, Radim Krčmář wrote: > >> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c > >> index 5fd95706c418..a1400b937254 100644 > >> --- a/x86/vmx_tests.c > >> +++ b/x86/vmx_tests.c > >> @@ -1816,6 +1816,42 @@ int into_exit_handler() > >> return VMX_TEST_VMEXIT; > >> } > >> > >> +#define __create_invalid_vmcs_test(name, value, test) \ > > > > No need for __ here. > > Ok. > > >> + static int invalid_##name##_init() \ > >> + { \ > >> + vmcs_write(name, value); \ > >> + return VMX_TEST_START; \ > >> + } \ > >> + static int invalid_##name##_entry_failure(struct vmentry_failure *failure) \ > >> + { \ > >> + report("VM entry failure on invalid "#name, test); \ > >> + return VMX_TEST_VMEXIT; \ > >> + } > >> + > >> +#define create_invalid_host_vmcs_test(name, value, error) \ > >> + __create_invalid_vmcs_test(name, value, failure->early && \ > >> + vmcs_read(VMX_INST_ERROR) == error) > >> + > >> +#define create_invalid_guest_vmcs_test(name, value, reason, qualification) \ > >> + __create_invalid_vmcs_test(name, value, !failure->early && \ > >> + vmcs_read(EXI_REASON) == (reason | VMX_ENTRY_FAILURE) && \ > >> + vmcs_read(EXI_QUALIFICATION) == qualification) > >> + > >> +#define test_invalid_vmcs(name) \ > >> + { "invalid "#name, invalid_##name##_init, NULL, NULL, NULL, {0}, \ > >> + invalid_##name##_entry_failure } > >> + > >> +#define C create_invalid_host_vmcs_test > >> +C(HOST_EFER, -1ull, VMXERR_ENTRY_INVALID_HOST_STATE_FIELD) > > > > No need for the "C" macro here. > > > >> +#undef C > >> + > >> +#define C create_invalid_guest_vmcs_test > >> +C(VMCS_LINK_PTR, 1, VMX_FAIL_STATE, ENTRY_FAIL_VMCS_LINK_PTR) > >> +C(GUEST_CR0, -1ull, VMX_FAIL_STATE, ENTRY_FAIL_DEFAULT) > >> +C(GUEST_CR4, -1ull, VMX_FAIL_STATE, ENTRY_FAIL_DEFAULT) > >> +C(GUEST_EFER, -1ull, VMX_FAIL_STATE, ENTRY_FAIL_DEFAULT) > > > > Same here. > > I can change it into something like this: > > create_invalid_guest_vmcs_test(VMCS_LINK_PTR, 1, VMX_FAIL_STATE, ENTRY_FAIL_VMCS_LINK_PTR) > create_invalid_guest_vmcs_test(GUEST_CR0, -1ull, VMX_FAIL_STATE, ENTRY_FAIL_DEFAULT) > create_invalid_guest_vmcs_test(GUEST_CR4, -1ull, VMX_FAIL_STATE, ENTRY_FAIL_DEFAULT) > create_invalid_guest_vmcs_test(GUEST_EFER, -1ull, VMX_FAIL_STATE, ENTRY_FAIL_DEFAULT) > > but not this: > > create_invalid_guest_vmcs_test(VMCS_LINK_PTR, 1, VMX_FAIL_STATE, > ENTRY_FAIL_VMCS_LINK_PTR) > create_invalid_guest_vmcs_test(GUEST_CR0, -1ull, VMX_FAIL_STATE, > ENTRY_FAIL_DEFAULT) > create_invalid_guest_vmcs_test(GUEST_CR4, -1ull, VMX_FAIL_STATE, > ENTRY_FAIL_DEFAULT) > create_invalid_guest_vmcs_test(GUEST_EFER, -1ull, VMX_FAIL_STATE, > ENTRY_FAIL_DEFAULT) > > I assume we don't want to anger teletype users, so the macro shortcut is > to improve readability. Let's anger them :-) I'm tired of making my patches 80ch-ugly, and have silently violated it a few times already... > > > These tests look a bit different from the others that use the "stage" > > mechanism. Perhaps you can have two tests only in vmx_tests, one for > > host failures and one for guest failures? > > I avoided stages because their explicit stateful nature was not needed. > The output is different, though ... I'll post a version that looks > better. > > >> +#undef C > >> + > >> /* name/init/guest_main/exit_handler/syscall_handler/guest_regs */ > >> struct vmx_test vmx_tests[] = { > >> { "null", NULL, basic_guest_main, basic_exit_handler, NULL, {0} }, > >> @@ -1845,5 +1881,10 @@ struct vmx_test vmx_tests[] = { > >> disable_rdtscp_exit_handler, NULL, {0} }, > >> { "int3", int3_init, int3_guest_main, int3_exit_handler, NULL, {0} }, > >> { "into", into_init, into_guest_main, into_exit_handler, NULL, {0} }, > >> + test_invalid_vmcs(VMCS_LINK_PTR), > >> + test_invalid_vmcs(GUEST_CR0), > >> + test_invalid_vmcs(GUEST_CR4), > >> + test_invalid_vmcs(GUEST_EFER), > >> + test_invalid_vmcs(HOST_EFER), > >> { NULL, NULL, NULL, NULL, NULL, {0} }, > >> }; > >> > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/x86/vmx.h b/x86/vmx.h index a2bacd348161..4829f9135a02 100644 --- a/x86/vmx.h +++ b/x86/vmx.h @@ -44,6 +44,45 @@ struct vmentry_failure { unsigned long flags; }; +/* + * Exit Qualifications for entry failure during or after loading guest state + */ +#define ENTRY_FAIL_DEFAULT 0 +#define ENTRY_FAIL_PDPTE 2 +#define ENTRY_FAIL_NMI 3 +#define ENTRY_FAIL_VMCS_LINK_PTR 4 + +/* + * VM-instruction error numbers + */ +enum vm_instruction_error_number { + VMXERR_VMCALL_IN_VMX_ROOT_OPERATION = 1, + VMXERR_VMCLEAR_INVALID_ADDRESS = 2, + VMXERR_VMCLEAR_VMXON_POINTER = 3, + VMXERR_VMLAUNCH_NONCLEAR_VMCS = 4, + VMXERR_VMRESUME_NONLAUNCHED_VMCS = 5, + VMXERR_VMRESUME_AFTER_VMXOFF = 6, + VMXERR_ENTRY_INVALID_CONTROL_FIELD = 7, + VMXERR_ENTRY_INVALID_HOST_STATE_FIELD = 8, + VMXERR_VMPTRLD_INVALID_ADDRESS = 9, + VMXERR_VMPTRLD_VMXON_POINTER = 10, + VMXERR_VMPTRLD_INCORRECT_VMCS_REVISION_ID = 11, + VMXERR_UNSUPPORTED_VMCS_COMPONENT = 12, + VMXERR_VMWRITE_READ_ONLY_VMCS_COMPONENT = 13, + VMXERR_VMXON_IN_VMX_ROOT_OPERATION = 15, + VMXERR_ENTRY_INVALID_EXECUTIVE_VMCS_POINTER = 16, + VMXERR_ENTRY_NONLAUNCHED_EXECUTIVE_VMCS = 17, + VMXERR_ENTRY_EXECUTIVE_VMCS_POINTER_NOT_VMXON_POINTER = 18, + VMXERR_VMCALL_NONCLEAR_VMCS = 19, + VMXERR_VMCALL_INVALID_VM_EXIT_CONTROL_FIELDS = 20, + VMXERR_VMCALL_INCORRECT_MSEG_REVISION_ID = 22, + VMXERR_VMXOFF_UNDER_DUAL_MONITOR_TREATMENT_OF_SMIS_AND_SMM = 23, + VMXERR_VMCALL_INVALID_SMM_MONITOR_FEATURES = 24, + VMXERR_ENTRY_INVALID_VM_EXECUTION_CONTROL_FIELDS_IN_EXECUTIVE_VMCS = 25, + VMXERR_ENTRY_EVENTS_BLOCKED_BY_MOV_SS = 26, + VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID = 28, +}; + struct vmx_test { const char *name; int (*init)(struct vmcs *vmcs); diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c index 5fd95706c418..a1400b937254 100644 --- a/x86/vmx_tests.c +++ b/x86/vmx_tests.c @@ -1816,6 +1816,42 @@ int into_exit_handler() return VMX_TEST_VMEXIT; } +#define __create_invalid_vmcs_test(name, value, test) \ + static int invalid_##name##_init() \ + { \ + vmcs_write(name, value); \ + return VMX_TEST_START; \ + } \ + static int invalid_##name##_entry_failure(struct vmentry_failure *failure) \ + { \ + report("VM entry failure on invalid "#name, test); \ + return VMX_TEST_VMEXIT; \ + } + +#define create_invalid_host_vmcs_test(name, value, error) \ + __create_invalid_vmcs_test(name, value, failure->early && \ + vmcs_read(VMX_INST_ERROR) == error) + +#define create_invalid_guest_vmcs_test(name, value, reason, qualification) \ + __create_invalid_vmcs_test(name, value, !failure->early && \ + vmcs_read(EXI_REASON) == (reason | VMX_ENTRY_FAILURE) && \ + vmcs_read(EXI_QUALIFICATION) == qualification) + +#define test_invalid_vmcs(name) \ + { "invalid "#name, invalid_##name##_init, NULL, NULL, NULL, {0}, \ + invalid_##name##_entry_failure } + +#define C create_invalid_host_vmcs_test +C(HOST_EFER, -1ull, VMXERR_ENTRY_INVALID_HOST_STATE_FIELD) +#undef C + +#define C create_invalid_guest_vmcs_test +C(VMCS_LINK_PTR, 1, VMX_FAIL_STATE, ENTRY_FAIL_VMCS_LINK_PTR) +C(GUEST_CR0, -1ull, VMX_FAIL_STATE, ENTRY_FAIL_DEFAULT) +C(GUEST_CR4, -1ull, VMX_FAIL_STATE, ENTRY_FAIL_DEFAULT) +C(GUEST_EFER, -1ull, VMX_FAIL_STATE, ENTRY_FAIL_DEFAULT) +#undef C + /* name/init/guest_main/exit_handler/syscall_handler/guest_regs */ struct vmx_test vmx_tests[] = { { "null", NULL, basic_guest_main, basic_exit_handler, NULL, {0} }, @@ -1845,5 +1881,10 @@ struct vmx_test vmx_tests[] = { disable_rdtscp_exit_handler, NULL, {0} }, { "int3", int3_init, int3_guest_main, int3_exit_handler, NULL, {0} }, { "into", into_init, into_guest_main, into_exit_handler, NULL, {0} }, + test_invalid_vmcs(VMCS_LINK_PTR), + test_invalid_vmcs(GUEST_CR0), + test_invalid_vmcs(GUEST_CR4), + test_invalid_vmcs(GUEST_EFER), + test_invalid_vmcs(HOST_EFER), { NULL, NULL, NULL, NULL, NULL, {0} }, };
This patch adds a generator for simple VMCS tests, generates few tests for recent changes, and also pulls in some header file dependencies. Signed-off-by: Radim Krčmář <rkrcmar@redhat.com> --- x86/vmx.h | 39 +++++++++++++++++++++++++++++++++++++++ x86/vmx_tests.c | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+)