On 21/03/2016 20:59, David Matlack wrote:
> On Sat, Mar 19, 2016 at 2:52 AM, Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote:
>> Check that it doesn't cause an infinite stream of vmexits.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
>> ---
>> x86/vmx.h | 9 +++++++--
>> x86/vmx_tests.c | 26 ++++++++++++++++++++++++++
>> 2 files changed, 33 insertions(+), 2 deletions(-)
>>
>> diff --git a/x86/vmx.h b/x86/vmx.h
>> index 0cb995d..146f082 100644
>> --- a/x86/vmx.h
>> +++ b/x86/vmx.h
>> @@ -549,12 +549,17 @@ static inline int vmcs_save(struct vmcs **vmcs)
>> return ret;
>> }
>>
>> -static inline void invept(unsigned long type, u64 eptp)
>> +static inline bool invept(unsigned long type, u64 eptp)
>> {
>> + bool ret;
>> + u64 rflags = read_rflags() | X86_EFLAGS_CF | X86_EFLAGS_ZF;
>
> Why explicitly set these bits prior to invept?
Cargo culting to some extent, but it does check that a successful INVEPT
clears the bits.
>> +
>> struct {
>> u64 eptp, gpa;
>> } operand = {eptp, 0};
>> - asm volatile("invept %0, %1\n" ::"m"(operand),"r"(type));
>> + asm volatile("push %1; popf; invept %2, %3; setbe %0"
>> + : "=q" (ret) : "r" (rflags), "m"(operand),"r"(type) : "cc");
>> + return ret;
>> }
>>
>> static inline void invvpid(unsigned long type, u16 vpid, u64 gva)
>> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
>> index 0145cad..dd3af58 100644
>> --- a/x86/vmx_tests.c
>> +++ b/x86/vmx_tests.c
>> @@ -1029,6 +1029,28 @@ t1:
>> // Test EPT access to L1 MMIO
>> vmx_set_test_stage(6);
>> report("EPT - MMIO access", *((u32 *)0xfee00030UL) == apic_version);
>> +
>> + // Test invalid operand for INVEPT
>> + vmcall();
>> + report("EPT - unsupported INVEPT", vmx_get_test_stage() == 7);
>> +}
>> +
>> +bool invept_test(int type, u64 eptp)
>> +{
>> + bool ret, supported;
>> +
>> + supported = ept_vpid.val & (EPT_CAP_INVEPT_SINGLE >> INVEPT_SINGLE << type);
>> + ret = invept(type, eptp);
>> +
>> + if (ret == !supported)
>> + return false;
>
> Nit: Is there a precedent for using bool as the return type and
> indicating success with false? I found this a little confusing; the
> pattern I'm more familiar with is an int return type and 0 for
> success.
It's modeled against the opcode wrappers in vmx.h (for example invept).
I don't really like bool return values indeed (I've probably complained
about them several times in the past for KVM code...), but it's the
common convention in VMX tests, see for example
if (vmx_on()) {
printf("%s : vmxon failed.\n", __func__);
return 1;
}
in vmx.c.
Paolo
>> +
>> + if (!supported)
>> + printf("WARNING: unsupported invept passed!\n");
>> + else
>> + printf("WARNING: invept failed!\n");
>> +
>> + return true;
>> }
>>
>> static int ept_exit_handler()
>> @@ -1084,6 +1106,10 @@ static int ept_exit_handler()
>> data_page1_pte_pte & (~EPT_PRESENT));
>> ept_sync(INVEPT_SINGLE, eptp);
>> break;
>> + case 6:
>> + if (!invept_test(0, eptp))
>> + vmx_inc_test_stage();
>> + break;
>> // Should not reach here
>> default:
>> printf("ERROR - unexpected stage, %d.\n",
>> --
>> 2.5.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
>
--
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
