Re: [PATCH v2 09/15] x86: Out-of-inline the policy<->featureset convertors

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 04/04/2023 4:01 pm, Jan Beulich wrote:
> On 04.04.2023 11:52, Andrew Cooper wrote:
>> These are already getting over-large for being inline functions, and are only
>> going to grow more over time.  Out of line them, yielding the following net
>> delta from bloat-o-meter:
>>
>>   add/remove: 2/0 grow/shrink: 0/4 up/down: 276/-1877 (-1601)
>>
>> Switch to the newer cpu_policy terminology while doing so.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Acked-by: Jan Beulich <jbeulich@xxxxxxxx>

Thanks.

>
> I take it you have a reason to ...
>
>> --- a/xen/lib/x86/cpuid.c
>> +++ b/xen/lib/x86/cpuid.c
>> @@ -60,6 +60,48 @@ const char *x86_cpuid_vendor_to_str(unsigned int vendor)
>>      }
>>  }
>>  
>> +void x86_cpu_policy_to_featureset(
>> +    const struct cpu_policy *p, uint32_t fs[FEATURESET_NR_ENTRIES])
>> +{
>> +    fs[FEATURESET_1d]        = p->basic._1d;
>> +    fs[FEATURESET_1c]        = p->basic._1c;
>> +    fs[FEATURESET_e1d]       = p->extd.e1d;
>> +    fs[FEATURESET_e1c]       = p->extd.e1c;
>> +    fs[FEATURESET_Da1]       = p->xstate.Da1;
>> +    fs[FEATURESET_7b0]       = p->feat._7b0;
>> +    fs[FEATURESET_7c0]       = p->feat._7c0;
>> +    fs[FEATURESET_e7d]       = p->extd.e7d;
>> +    fs[FEATURESET_e8b]       = p->extd.e8b;
>> +    fs[FEATURESET_7d0]       = p->feat._7d0;
>> +    fs[FEATURESET_7a1]       = p->feat._7a1;
>> +    fs[FEATURESET_e21a]      = p->extd.e21a;
>> +    fs[FEATURESET_7b1]       = p->feat._7b1;
>> +    fs[FEATURESET_7d2]       = p->feat._7d2;
>> +    fs[FEATURESET_7c1]       = p->feat._7c1;
>> +    fs[FEATURESET_7d1]       = p->feat._7d1;
>> +}
>> +
>> +void x86_cpu_featureset_to_policy(
>> +    const uint32_t fs[FEATURESET_NR_ENTRIES], struct cpu_policy *p)
>> +{
>> +    p->basic._1d             = fs[FEATURESET_1d];
>> +    p->basic._1c             = fs[FEATURESET_1c];
>> +    p->extd.e1d              = fs[FEATURESET_e1d];
>> +    p->extd.e1c              = fs[FEATURESET_e1c];
>> +    p->xstate.Da1            = fs[FEATURESET_Da1];
>> +    p->feat._7b0             = fs[FEATURESET_7b0];
>> +    p->feat._7c0             = fs[FEATURESET_7c0];
>> +    p->extd.e7d              = fs[FEATURESET_e7d];
>> +    p->extd.e8b              = fs[FEATURESET_e8b];
>> +    p->feat._7d0             = fs[FEATURESET_7d0];
>> +    p->feat._7a1             = fs[FEATURESET_7a1];
>> +    p->extd.e21a             = fs[FEATURESET_e21a];
>> +    p->feat._7b1             = fs[FEATURESET_7b1];
>> +    p->feat._7d2             = fs[FEATURESET_7d2];
>> +    p->feat._7c1             = fs[FEATURESET_7c1];
>> +    p->feat._7d1             = fs[FEATURESET_7d1];
>> +}
> ... add quite a few padding blanks in here, unlike in the originals?

Yeah.  There was already one misalignment, and I haven't quite decided
on the MSR syntax yet but it's going to be longer still.

Here specifically, we've got p->arch_caps.{a,d} at a minimum, so column
width is based on the MSR name.

This is just a guestimate of "plenty for now".

~Andrew