Re: [Xen-devel] [PATCH RFC 28/31] xen/x86: Context switch all levelling state in context_switch()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 22/01/16 09:52, Jan Beulich wrote:
>>>> On 16.12.15 at 22:24, <andrew.cooper3@xxxxxxxxxx> wrote:
>> --- a/xen/arch/x86/cpu/amd.c
>> +++ b/xen/arch/x86/cpu/amd.c
>> @@ -300,6 +300,9 @@ static void __init noinline amd_init_levelling(void)
>>              cpumask_defaults._6c &= (~0ULL << 32);
>>              cpumask_defaults._6c |= ecx;
>>      }
>> +
>> +        if (levelling_caps)
>> +            ctxt_switch_levelling = amd_ctxt_switch_levelling;
>>  }
> Indentation.
>
>> --- a/xen/arch/x86/cpu/common.c
>> +++ b/xen/arch/x86/cpu/common.c
>> @@ -86,6 +86,13 @@ static const struct cpu_dev default_cpu = {
>>  };
>>  static const struct cpu_dev *this_cpu = &default_cpu;
>>  
>> +void default_ctxt_switch_levelling(const struct domain *nextd)
> static
>
>> +{
>> +    /* Nop */
>> +}
>> +void (*ctxt_switch_levelling)(const struct domain *nextd) __read_mostly =
>> +    default_ctxt_switch_levelling;
> While current and past gcc may accept (and honor) this placement of
> the __read_mostly annotation, I think it is wrong from a strict language
> syntax pov. Imo it instead ought to be
>
> void (*__read_mostly ctxt_switch_levelling)(const struct domain *nextd) =
>
> Also - indentation again.
>
>> @@ -145,6 +145,13 @@ void intel_ctxt_switch_levelling(const struct domain 
>> *nextd)
>>      struct cpumasks *these_masks = &this_cpu(cpumasks);
>>      const struct cpumasks *masks = &cpumask_defaults;
>>  
>> +    if (cpu_has_cpuid_faulting) {
>> +            set_cpuid_faulting(nextd && is_pv_domain(nextd) &&
>> +                               !is_control_domain(nextd) &&
>> +                               !is_hardware_domain(nextd));
>> +            return;
>> +    }
> Considering that you don't even probe the masking MSRs, this seems
> inconsistent with your "always level the entire system" choice.

In the case that faulting is available, we never want to touch masking. 
Faulting is newer and strictly superior to masking.

As documented, there is no hardware which support both.  (In reality,
there is one SKU of IvyBridge CPUs which experimentally have both.)


The fact that dom0 and the hardware domain are bypassed is a bug IMO. 
However, I am preserving the existing behaviour until phase 2 when I fix
other parts of the cpuid policy.  Currently imposing faulting on dom0
causes carnage because nothing generates it a sane policy.

~Andrew

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