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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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.

~Andrew

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