Re: [Xen-devel] [V2 PATCH 1/9] x86/hvm: pkeys, add pkeys support for cpuid handling

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 01.12.15 at 21:03, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 27/11/15 09:51, Huaitong Han wrote:
>> --- a/xen/arch/x86/hvm/hvm.c
>> +++ b/xen/arch/x86/hvm/hvm.c
>> @@ -4582,6 +4582,18 @@ void hvm_cpuid(unsigned int input, unsigned int *eax, 
>> unsigned int *ebx,
>>          /* Don't expose INVPCID to non-hap hvm. */
>>          if ( (count == 0) && !hap_enabled(d) )
>>              *ebx &= ~cpufeat_mask(X86_FEATURE_INVPCID);
>> +
>> +        /* X86_FEATURE_PKU is not yet implemented for shadow paging
>> +         *
>> +         * Hypervisor gets guest pkru value from XSAVE state, because
>> +         * Hypervisor CR4 without X86_CR4_PKE disables RDPKRU instruction.
>> +         */
>> +        if ( (count == 0) && (!hap_enabled(d) || !cpu_has_xsave) )
>> +            *ecx &= ~cpufeat_mask(X86_FEATURE_PKU);
>> +
>> +        if ( (count == 0) && cpu_has_pku )
>> +            *ecx |= (v->arch.hvm_vcpu.guest_cr[4] & X86_CR4_PKE) ?
>> +                     cpufeat_mask(X86_FEATURE_OSPKE) : 0;
> 
> This is still buggy.  cpu_has_pku has no relevance to whether OSPKE
> becomes visible.
> 
> Visibility of OSPKE is determined solely by v->arch.hvm_vcpu.guest_cr[4]
> & X86_CR4_PKE and nothing else.

Actually I wouldn't mind guarding against the case where the CR4 flag
is wrongly set for whatever reason, but that ought to check the PKU
bit in *ecx, not the host flag. Same applies to the cpu_has_xsave
check - this too should check the guest flag, not the host one.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel