Re: [Xen-devel] [PATCH 23/27] x86/cpuid: Move all leaf 7 handling into guest_cpuid()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 05.01.17 at 15:39, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 05/01/17 14:01, Jan Beulich wrote:
>>>>> On 04.01.17 at 13:39, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> @@ -380,14 +385,42 @@ void guest_cpuid(const struct vcpu *v, unsigned int 
> leaf,
>>>      case 0x80000000 ... 0x80000000 + CPUID_GUEST_NR_EXTD - 1:
>>>          if ( leaf > p->extd.max_leaf )
>>>              return;
>>> -        break;
>>> +        goto legacy;
>>>  
>>>      default:
>>>          return;
>>>      }
>>>  
>>> +    /* Skip dynamic adjustments if we are in the wrong context. */
>>> +    if ( v != curr )
>>> +        return;
>>> +
>>> +    /*
>>> +     * Second pass:
>>> +     * - Dynamic adjustments
>>> +     */
>>> +    switch ( leaf )
>>> +    {
>>> +    case 0x7:
>>> +        switch ( subleaf )
>>> +        {
>>> +        case 0:
>>> +            /* OSPKE clear in policy.  Fast-forward CR4 back in. */
>>> +            if ( (is_pv_vcpu(v)
>>> +                  ? v->arch.pv_vcpu.ctrlreg[4]
>>> +                  : v->arch.hvm_vcpu.guest_cr[4]) & X86_CR4_PKE )
>>> +                res->c |= cpufeat_mask(X86_FEATURE_OSPKE);
>> What's wrong with doing this adjustment when v != curr?
> 
> A guests %cr4 is stale if it is running elsewhere.
> 
>> By the time the caller looks at the result, the state of guest
>> software controlled bits can't be relied upon anyway.
> 
> This particular adjustment can be done out of curr context, but others
> are harder.  I have taken the approach that it is better to do nothing
> consistently, than to expend effort filling in data we know is going to
> be wrong for the caller.

May I then suggest to add the early bailing at the time it actually
becomes necessary, or at the very least extend its comment to
make clear that this isn't always strictly needed?

Jan


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