Re: [PATCH 7/9] x86/hvm: Disable MPX by default

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 16/06/2020 10:33, Jan Beulich wrote:
> On 15.06.2020 16:15, Andrew Cooper wrote:
>> @@ -479,6 +497,18 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t 
>> domid, bool restore,
>>          goto out;
>>      }
>>  
>> +    /*
>> +     * Account for feature which have been disabled by default since Xen 
>> 4.13,
>> +     * so migrated-in VM's don't risk seeing features disappearing.
>> +     */
>> +    if ( restore )
>> +    {
>> +        if ( di.hvm )
>> +        {
>> +            p->feat.mpx = test_bit(X86_FEATURE_MPX, host_featureset);
> Why do you derive this from the host featureset instead of the max
> one for the guest type?

Because that is how the logic worked for 4.13.

Also, because we don't have easy access to the actual guest max
featureset at this point.  I could add two new sysctl subops to
get_featureset, but the reason for not doing so before are still
applicable now.

There is a theoretical case where host MPX is visible but guest max is
hidden, and that is down to the vmentry controls.  As this doesn't exist
in real hardware, I'm not terribly concerned about it.

>  Also, while you modify p here, ...
>
>> +        }
>> +    }
>> +
>>      if ( featureset )
>>      {
>>          uint32_t disabled_features[FEATURESET_NR_ENTRIES],
> ... the code in this if()'s body ignores p altogether.

That is correct.

> I realize the
> only caller of the function passes NULL for "featureset", but I'd
> like to understand the rationale here anyway before giving an R-b.

The meaning of 'featureset' is "here are the exact bits I want you to use".

It has existed since my original CPUID work in 4.6/4.7.  Currently, the
only user in XenServer, and its a stopgap on the way to the "proper"
solution which I've been working on for the past several years.

~Andrew