Re: [PATCH for-4.22 v2 7/8] x86/mwait-idle: Add cmdline option to adjust C-states table

[Jan Beulich <jbeulich@xxxxxxxx>]

On 14.05.2026 17:18, Roger Pau Monné wrote:
> On Tue, May 12, 2026 at 05:38:08PM +0200, Jan Beulich wrote:
>> From: Artem Bityutskiy <artem.bityutskiy@xxxxxxxxxxxxxxx>
>>
>> Add a new module parameter that allows adjusting the C-states table used by
>> the driver.
>>
>> Currently, the C-states table is hardcoded in the driver based on the CPU
>> model. The goal is to have good enough defaults for most users.
>>
>> However, C-state characteristics, such as exit latency and residency, can
>> vary between different variants of the same CPU model and BIOS settings.
>> Moreover, different platform usage models and user preferences may benefit
>> from different C-state target_residency values.
>>
>> Provide a way for users to adjust the C-states table via a module parameter
>> "table". The general format is:
>> "state1:latency1:target_residency1,state2:latency2:target_residency2,..."
>>
>> In other words, represent each C-state by its name, exit latency (in
>> microseconds), and target residency (in microseconds), separated by colons.
>> Separate multiple C-states by commas.
>>
>> For example, suppose a CPU has 3 C-states with the following
>> characteristics:
>>   C1:  exit_latency=1, target_residency=2
>>   C1E: exit_latency=10, target_residency=10
>>   C6:  exit_latency=100, target_residency=500
>>
>> Users can specify a custom C-states table as follows:
>>
>> 1. intel_idle.table="C1:2:2,C1E:5:20,C6:150:600"
>>    Result: C1:  exit_latency=2, target_residency=2
>>            C1E: exit_latency=5, target_residency=20
>>            C6:  exit_latency=150, target_residency=600
>> 2. intel_idle.table="C6::400"
>>    Result: C1:  exit_latency=1, target_residency=2 (unchanged)
>>            C1E: exit_latency=10, target_residency=10 (unchanged)
>>            C6:  exit_latency=100, target_residency=400
>>                 (only target_residency changed)
>>
>> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@xxxxxxxxxxxxxxx>
>> Link: https://patch.msgid.link/20251216080402.156988-3-dedekind1@xxxxxxxxx
>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
>> Origin: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
>> 111f77a23348
>>
>> Add __init to get_cmdline_field(). Put cmdline_table_str[] in .init.data.
>> Other adjustments to fit our env.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> Acked-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Thanks.

>> +/**
>> + * cmdline_table_adjust - Adjust the C-states table with data from cmdline.
>> + *
>> + * Adjust the C-states table with data from the 'mwait-idle.table' parameter
>> + * (if specified).
>> + */
>> +static void __init cmdline_table_adjust(void)
>> +{
>> +    char *args = cmdline_table_str;
>> +    struct cpuidle_state *state;
>> +    unsigned int i, state_count;
>> +
>> +    if (args[0] == '\0')
>> +            /* The 'mwait-idle.table' module parameter was not specified */
>> +            return;
>> +
>> +    /* Create a copy of the C-states table */
>> +    for (i = 0;
>> +         i < ARRAY_SIZE(cmdline_states) && icpu.state_table[i].name[0];
>> +         i++)
>> +            cmdline_states[i] = icpu.state_table[i];
>> +
>> +    state_count = i;
>> +
>> +    /*
>> +     * Adjust the C-states table copy with data from the 'mwait-idle.table'
>> +     * module parameter.
>> +     */
>> +    while (args) {
>> +            char *fields, *name, *val;
>> +
>> +            /*
>> +             * Get the next C-state definition, which is expected to be
>> +             * '<name>:<latency_us>:<target_residency_us>'. Treat "empty"
>> +             * fields as unchanged. For example,
>> +             * '<name>::<target_residency_us>' leaves the latency unchanged.
>> +             */
>> +            args = get_cmdline_field(args, &fields, ',');
>> +
>> +            /* name */
>> +            fields = get_cmdline_field(fields, &name, ':');
>> +            if (!fields)
>> +                    goto error;
>> +
>> +            /* Find the C-state by its name */
>> +            state = NULL;
>> +            for (i = 0; i < state_count; i++) {
>> +                    if (!strcmp(name, cmdline_states[i].name)) {
>> +                            state = &cmdline_states[i];
>> +                            break;
>> +                    }
>> +            }
>> +
>> +            if (!state) {
>> +                    printk(XENLOG_ERR PREFIX "C-state '%s' was not found\n",
>> +                           name);
>> +                    continue;
>> +            }
>> +
>> +            /* Latency */
>> +            fields = get_cmdline_field(fields, &val, ':');
>> +            if (!fields)
>> +                    goto error;
>> +
>> +            if (*val) {
>> +                    const char *end;
>> +                    unsigned long n = simple_strtoul(val, &end, 0);
>> +
>> +                    state->exit_latency = n;
>> +                    if (*end || state->exit_latency != n)
>> +                            goto error;
>> +            }
>> +
>> +            /* Target residency */
>> +            fields = get_cmdline_field(fields, &val, ':');
>> +
>> +            if (*val) {
>> +                    const char *end;
>> +                    unsigned long n = simple_strtoul(val, &end, 0);
>> +
>> +                    state->target_residency = n;
>> +                    if (*end || state->target_residency != n)
>> +                            goto error;
>> +            }
>> +
>> +            /*
>> +             * Allow for 3 more fields, but ignore them. Helps to make
>> +             * possible future extensions of the cmdline format backward
>> +             * compatible.
>> +             */
>> +            for (i = 0; fields && i < 3; i++) {
>> +                    fields = get_cmdline_field(fields, &val, ':');
>> +                    if (!fields)
>> +                            break;
>> +            }
>> +
>> +            if (fields) {
>> +                    printk(XENLOG_ERR PREFIX
>> +                           "Too many fields for C-state '%s'\n",
>> +                           state->name);
>> +                    goto error;
>> +            }
>> +
>> +            printk(XENLOG_INFO PREFIX
>> +                   "C-state from cmdline: name=%s, latency=%u, 
>> residency=%u\n",
>> +                   state->name, state->exit_latency, 
>> state->target_residency);
>> +    }
>> +
>> +    /* Copy the adjusted C-states table back */
>> +    for (i = 0; i < state_count; i++)
>> +            icpu.state_table[i] = cmdline_states[i];
>> +
>> +    printk(XENLOG_INFO PREFIX
>> +           "Adjusted C-states with data from 'mwait-idle.table'\n");
>> +    return;
>> +
>> + error:
>> +    printk(PREFIX
> 
> XENLOG_ERR ahead of the prefix maybe?

I did already raise the level from info to warning, compared to the Linux
original. I didn't want to go yet farther with this, hence why I'd prefer
to leave out XENLOG_* altogether here.

Jan