Re: [Xen-devel] [PATCH 04/27] x86/cpuid: Move featuresets into struct cpuid_policy

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 04.01.17 at 16:10, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 04/01/17 14:35, Jan Beulich wrote:
>>>>> On 04.01.17 at 13:39, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> +static void __init calculate_host_policy(void)
>>>  {
>>> -    unsigned int max, tmp;
>>> -
>>> -    max = cpuid_eax(0);
>>> -
>>> -    if ( max >= 1 )
>>> -        cpuid(0x1, &tmp, &tmp,
>>> -              &raw_featureset[FEATURESET_1c],
>>> -              &raw_featureset[FEATURESET_1d]);
>>> -    if ( max >= 7 )
>>> -        cpuid_count(0x7, 0, &tmp,
>>> -                    &raw_featureset[FEATURESET_7b0],
>>> -                    &raw_featureset[FEATURESET_7c0],
>>> -                    &raw_featureset[FEATURESET_7d0]);
>>> -    if ( max >= 0xd )
>>> -        cpuid_count(0xd, 1,
>>> -                    &raw_featureset[FEATURESET_Da1],
>>> -                    &tmp, &tmp, &tmp);
>>> -
>>> -    max = cpuid_eax(0x80000000);
>>> -    if ( (max >> 16) != 0x8000 )
>>> -        return;
>>> +    struct cpuid_policy *p = &host_policy;
>>>  
>>> -    if ( max >= 0x80000001 )
>>> -        cpuid(0x80000001, &tmp, &tmp,
>>> -              &raw_featureset[FEATURESET_e1c],
>>> -              &raw_featureset[FEATURESET_e1d]);
>>> -    if ( max >= 0x80000007 )
>>> -        cpuid(0x80000007, &tmp, &tmp, &tmp,
>>> -              &raw_featureset[FEATURESET_e7d]);
>>> -    if ( max >= 0x80000008 )
>>> -        cpuid(0x80000008, &tmp,
>>> -              &raw_featureset[FEATURESET_e8b],
>>> -              &tmp, &tmp);
>>> +    memcpy(p->fs, boot_cpu_data.x86_capability, sizeof(p->fs));
>> What are the plans for keeping this up-to-date wrt later
>> adjustments to boot_cpu_data.x86_capability?  Wouldn't it be
>> better for the field to be a pointer, and the above to be a simple
>> assignment of &boot_cpu_data.x86_capability?
> 
> The fs field is temporary and removed in patch 20.
> 
> calculate_host_policy() is called immediately before dom0 is
> constructed, which is after AP bringup.  Realistically,
> boot_cpu_data.x86_capability won't be changing by this point, even for
> PCPU hotplug.
> 
>>
>>> +static void __init calculate_pv_max_policy(void)
>>>  {
>>> +    struct cpuid_policy *p = &pv_max_policy;
>> I assume later patches will add further uses of this variable?
> 
> Yes.
> 
>> Otherwise ...
>>
>>> @@ -185,10 +159,12 @@ static void __init calculate_pv_featureset(void)
>>>      __set_bit(X86_FEATURE_CMP_LEGACY, pv_featureset);
>>>  
>>>      sanitise_featureset(pv_featureset);
>>> +    cpuid_featureset_to_policy(pv_featureset, p);
>> ... using &pv_max_policy directly here would seem more friendly
>> to readers.
> 
> Expressing it this way makes shorter diffs along the series.

Okay then:
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan


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