Re: [PATCH v2 1/2] x86/hvm: allow guest to use clflushopt and clwb

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 07/01/16 13:49, Jan Beulich wrote:
>>>> On 30.12.15 at 12:48, <haozhong.zhang@intel.com> wrote:
>> --- a/xen/arch/x86/hvm/hvm.c
>> +++ b/xen/arch/x86/hvm/hvm.c
>> @@ -4583,21 +4583,30 @@ void hvm_cpuid(unsigned int input, unsigned int *eax, unsigned int *ebx,
>>              *edx &= ~cpufeat_mask(X86_FEATURE_PSE36);
>>          break;
>>      case 0x7:
>> -        if ( (count == 0) && !cpu_has_smep )
>> -            *ebx &= ~cpufeat_mask(X86_FEATURE_SMEP);
>> +        if ( count == 0 )
>> +        {
>> +            if ( !cpu_has_smep )
>> +                *ebx &= ~cpufeat_mask(X86_FEATURE_SMEP);
>> +
>> +            if ( !cpu_has_smap )
>> +                *ebx &= ~cpufeat_mask(X86_FEATURE_SMAP);
>> +
>> +            /* Don't expose MPX to hvm when VMX support is not available */
>> +            if ( !(vmx_vmexit_control & VM_EXIT_CLEAR_BNDCFGS) ||
>> +                 !(vmx_vmentry_control & VM_ENTRY_LOAD_BNDCFGS) )
>> +                *ebx &= ~cpufeat_mask(X86_FEATURE_MPX);
>>  
>> -        if ( (count == 0) && !cpu_has_smap )
>> -            *ebx &= ~cpufeat_mask(X86_FEATURE_SMAP);
>> +            /* Don't expose INVPCID to non-hap hvm. */
>> +            if ( !hap_enabled(d) )
>> +                *ebx &= ~cpufeat_mask(X86_FEATURE_INVPCID);
>>  
>> -        /* Don't expose MPX to hvm when VMX support is not available */
>> -        if ( (count == 0) &&
>> -             (!(vmx_vmexit_control & VM_EXIT_CLEAR_BNDCFGS) ||
>> -              !(vmx_vmentry_control & VM_ENTRY_LOAD_BNDCFGS)) )
>> -            *ebx &= ~cpufeat_mask(X86_FEATURE_MPX);
>> +            if ( !cpu_has_clflushopt )
>> +                *ebx &= ~cpufeat_mask(X86_FEATURE_CLFLUSHOPT);
>> +
>> +            if ( !cpu_has_clwb )
>> +                *ebx &= ~cpufeat_mask(X86_FEATURE_CLWB);
> I don't think we need this: Other than other things adjusted here,
> there's nothing disabling these two features when found available,
> and there are no extra conditions to consider. Otherwise, if we
> were to follow this route, quite a bit of code would need to be
> added to other case statements in this function. But that's all (I
> think) going to be taken care of by Andrew's CPUID leveling series.

My series does take care of all of this.

I would prefer that these two changes get taken as soon as they are
ready, so I can rebase.

~Andrew