On 08.05.2023 12:25, Jan Beulich wrote:
> On 01.05.2023 21:30, Jason Andryuk wrote:
>> Extend xen_get_cpufreq_para to return hwp parameters. These match the
>> hardware rather closely.
>>
>> We need the features bitmask to indicated fields supported by the actual
>> hardware.
>>
>> The use of uint8_t parameters matches the hardware size. uint32_t
>> entries grows the sysctl_t past the build assertion in setup.c. The
>> uint8_t ranges are supported across multiple generations, so hopefully
>> they won't change.
>
> Still it feels a little odd for values to be this narrow. Aiui the
> scaling_governor[] and scaling_{max,min}_freq fields aren't (really)
> used by HWP. So you could widen the union in struct
> xen_get_cpufreq_para (in a binary but not necessarily source compatible
> manner), gaining you 6 more uint32_t slots. Possibly the somewhat oddly
> placed scaling_cur_freq could be included as well ...
Having seen patch 9 now as well, I wonder whether here (or in a separate
patch) you don't want to limit providing inapplicable data (for example
not filling *scaling_available_governors would even avoid an allocation,
thus removing a possible reason for failure), while there (or again in a
separate patch) you'd also limit what the tool reports (inapplicable
output causes confusion / questions at best).
Jan
