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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
