Re: [PATCH] x86/cpuid: Calculate FEATURESET_NR_ENTRIES more helpfully

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 15/05/2023 8:46 am, Jan Beulich wrote:
> On 12.05.2023 14:45, Andrew Cooper wrote:
>> When adding new featureset words, it is convenient to split the work into
>> several patches.  However, GCC 12 spotted that the way we prefer to split the
>> work results in a real (transient) breakage whereby the policy <-> featureset
>> helpers perform out-of-bounds accesses on the featureset array.
>>
>> Fix this by having gen-cpuid.py calculate FEATURESET_NR_ENTRIES from the
>> comments describing the word blocks, rather than from the XEN_CPUFEATURE()
>> with the greatest value.
>>
>> For simplicty, require that the word blocks appear in order.  This can be
>> revisted if we find a good reason to have blocks out of order.
>>
>> No functional change.
>>
>> Reported-by: Jan Beulich <jbeulich@xxxxxxxx>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> As far as my Python goes:
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Thanks.

> Just one remark further down.
>
>> This supercedes the entire "x86: Fix transient build breakage with featureset
>> additions" series, but doesn't really feel as if it ought to be labelled v2
> Thank you for re-doing this altogether. I think it's more safe this way,
> and it now being less intrusive is imo also beneficial.

Yeah, now I've done both I do prefer this version.

>> --- a/xen/tools/gen-cpuid.py
>> +++ b/xen/tools/gen-cpuid.py
>> @@ -94,6 +118,15 @@ def parse_definitions(state):
>>      if len(state.names) == 0:
>>          raise Fail("No features found")
>>  
>> +    if state.nr_entries == 0:
>> +        raise Fail("No featureset word info found")
>> +
>> +    max_val = max(state.names.keys())
>> +    if (max_val >> 5) + 1 > state.nr_entries:
> Maybe
>
>     if (max_val >> 5) >= state.nr_entries:

Done.

~Andrew