On 2018-04-16 20:28, Jan Kiszka wrote:
> On 2018-04-16 19:46, Francois-Frederic Ozog wrote:
>> From: Francois-Frederic Ozog <francois.o...@linaro.org>
>>
>> inmates cannot use X86_FEATURE_VMX from regular cpuid
>> as vcpu maks the bit explicitely on non-root cells.
>>
>> use cpuid leaf 0 to detect AuthenticAMD and change to VMMCALL
>>
>> use string compare for clearer code
>>
>> Signed-off-by: Francois-Frederic Ozog <francois.o...@linaro.org>
>> ---
>>  inmates/lib/x86/hypercall.c | 21 ++++++++++++++-------
>>  1 file changed, 14 insertions(+), 7 deletions(-)
>>
>> diff --git a/inmates/lib/x86/hypercall.c b/inmates/lib/x86/hypercall.c
>> index ac99f0c3..07529d4d 100644
>> --- a/inmates/lib/x86/hypercall.c
>> +++ b/inmates/lib/x86/hypercall.c
>> @@ -40,19 +40,26 @@
>>  
>>  #define X86_FEATURE_VMX     (1 << 5)
>>  
>> -bool jailhouse_use_vmcall;
>> +bool jailhouse_use_vmcall = true;
>> +
>> +static const char* AUTHENTIC_AMD = "AuthenticAMD";
>> +#define U32_SUBSTR(s, n) (((const u32*)s)[n])
>>  
>>  void hypercall_init(void)
>>  {
>> -    u32 eax = 1, ecx = 0;
>> +    u32 ebx, ecx, edx;
>>  
>>      asm volatile(
>> +            "xor %%eax, %%eax \n\t"
>>              "cpuid"
>> -            : "=c" (ecx)
>> -            : "a" (eax), "c" (ecx)
>> -            : "rbx", "rdx", "memory"
>> +            : "=b" (ebx), "=c" (ecx), "=d" (edx)
>> +            : 
>> +            : "memory"
>>      );
>>  
>> -    if (ecx & X86_FEATURE_VMX)
>> -            jailhouse_use_vmcall = true;
>> +    if (ebx == U32_SUBSTR(AUTHENTIC_AMD, 0)
>> +            && ecx == U32_SUBSTR(AUTHENTIC_AMD, 2)
>> +            && edx == U32_SUBSTR(AUTHENTIC_AMD, 1)
>> +            )
>> +            jailhouse_use_vmcall = false;
>>  }
>>
> 
> I would go with this diff now unless you have concerns:
> 
> diff --git a/inmates/lib/x86/hypercall.c b/inmates/lib/x86/hypercall.c
> index ac99f0c3e..fc5cd5ac8 100644
> --- a/inmates/lib/x86/hypercall.c
> +++ b/inmates/lib/x86/hypercall.c
> @@ -38,21 +38,24 @@
>  
>  #include <inmate.h>
>  
> -#define X86_FEATURE_VMX      (1 << 5)
> +#define X86_FEATURE_VMX              (1 << 5)
>  
> -bool jailhouse_use_vmcall;
> +bool jailhouse_use_vmcall = true;
> +
> +#define AUTHENTIC_AMD(n)     (((const u32 *)"AuthenticAMD")[n])
>  
>  void hypercall_init(void)
>  {
> -     u32 eax = 1, ecx = 0;
> +     u32 eax, ebx, ecx, edx;
>  
> -     asm volatile(
> -             "cpuid"
> -             : "=c" (ecx)
> -             : "a" (eax), "c" (ecx)
> -             : "rbx", "rdx", "memory"
> +     asm volatile("cpuid"
> +             : "=a" (eax), "=b" (ebx), "=c" (ecx), "=d" (edx)
> +             : "a" (0)
> +             : "memory"
>       );
>  
> -     if (ecx & X86_FEATURE_VMX)
> -             jailhouse_use_vmcall = true;
> +     if (ebx == AUTHENTIC_AMD(0) &&
> +         edx == AUTHENTIC_AMD(1) &&
> +         ecx == AUTHENTIC_AMD(2))
> +             jailhouse_use_vmcall = false;
>  }
> 
> We need to account for Intel cpuid clobbering eax. Moreover, I played a
> bit with the style. Newer gcc even avoids a static string.
> 
> Still need to validate this on an AMD system, though.

Works fine, just tested in QEMU.

Jan

-- 
You received this message because you are subscribed to the Google Groups 
"Jailhouse" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to jailhouse-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to