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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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.

(I hit a rats nest with the xstate leaf and dynamic %ebx's, which is why
those patches are still pending some more work, and I haven't yet
decided how to do the pv hardware domain leakage.)

> Which then raises the question whether a second switch() statement
> for the a second pass is all that useful in the first place (I
> realize this may depend on future plans of yours).

This switch statement will soon be far larger and more complicated than
the first-pass one, and I think it is important to separate the static
and dynamic nature of the two.

~Andrew

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