On 27.05.2025 10:48, Penny Zheng wrote:
> --- a/xen/drivers/cpufreq/cpufreq.c
> +++ b/xen/drivers/cpufreq/cpufreq.c
> @@ -69,8 +69,55 @@ enum cpufreq_xen_opt __initdata cpufreq_xen_opts[2] = {
> CPUFREQ_xen,
> CPUFREQ_none };
> unsigned int __initdata cpufreq_xen_cnt = 1;
>
> +static const char __initconst cpufreq_opts_str[][5] = {
> + [CPUFREQ_none] = "none",
> + [CPUFREQ_xen] = "xen",
> + [CPUFREQ_hwp] = "hwp",
> +};
> +
> static int __init cpufreq_cmdline_parse(const char *s, const char *e);
>
> +static bool __init cpufreq_opts_contain(enum cpufreq_xen_opt option)
> +{
> + unsigned int count = cpufreq_xen_cnt;
> +
> + while ( count )
> + {
> + if ( cpufreq_xen_opts[--count] == option )
Instead of this, "while ( count-- )" would likely be slightly better;
less risk of an edit to the loop body (however unlikely that may seem
right now) then bypassing the decrement.
> + return true;
> + }
> +
> + return false;
> +}
> +
> +static int __init handle_cpufreq_cmdline(enum cpufreq_xen_opt option)
> +{
> + int ret = 0;
> +
> + if ( cpufreq_opts_contain(option) )
> + {
> + printk(XENLOG_WARNING "Duplicate cpufreq driver option: %s\n",
> + cpufreq_opts_str[option]);
Do we really need such a warning?
> + return 0;
> + }
> +
> + cpufreq_controller = FREQCTL_xen;
> + cpufreq_xen_opts[cpufreq_xen_cnt++] = option;
This could do with (at least) an assertion that we indeed don't overrun the
array.
Jan
