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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

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>

One possible adjustment below.

> ---
> For the initial attempt, I've left the new option as a standalone one. It
> may be worth integrating with "mwait-idle", but I think much of the
> parsing would then want doing differently. It'll then likely be much
> harder to apply future Linux changes there.
> ---
> v2: Correct example in cmdline doc.
> 
> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -1916,6 +1916,23 @@ Print boot time MTRR state.
>  Use the MWAIT idle driver (with model specific C-state knowledge) instead
>  of the ACPI based one.
>  
> +### mwait-idle.table (x86)
> +> `= <string>`
> +
> + Define the C-states table from a user input string. Expected format is
> + 'name:latency:residency', where:
> + - name: The C-state name.
> + - latency: The C-state exit latency in us.
> + - residency: The C-state target residency in us.
> +
> + Multiple C-states can be defined by separating them with commas:
> + 'name1:latency1:residency1,name2:latency2:residency2'
> +
> + Example: mwait-idle.table=C1:1:1,C1E:5:10,C6:100:600
> +
> + To leave latency or residency unchanged, use an empty field, for example:
> + 'C1:1:1,C1E::10' - leaves C1E latency unchanged.
> +
>  ### nmi (x86)
>  > `= ignore | dom0 | fatal`
>  
> --- a/xen/arch/x86/cpu/mwait-idle.c
> +++ b/xen/arch/x86/cpu/mwait-idle.c
> @@ -70,6 +70,11 @@
>  static __initdata bool opt_mwait_idle = true;
>  boolean_param("mwait-idle", opt_mwait_idle);
>  
> +/* The maximum allowed length for the 'table' module parameter  */
> +#define MAX_CMDLINE_TABLE_LEN 256
> +static char cmdline_table_str[MAX_CMDLINE_TABLE_LEN] __initdata;
> +string_param("mwait-idle.table", cmdline_table_str);
> +
>  static unsigned int mwait_substates;
>  
>  #define LAPIC_TIMER_ALWAYS_RELIABLE 0xFFFFFFFF
> @@ -122,6 +127,9 @@ struct cpuidle_state {
>   */
>  #define CPUIDLE_FLAG_IBRS            0x20000
>  
> +/* C-states data from the 'mwait-idle.table' cmdline parameter */
> +static struct cpuidle_state cmdline_states[ACPI_PROCESSOR_MAX_POWER] 
> __initdata;
> +
>  /*
>   * MWAIT takes an 8-bit "hint" in EAX "suggesting"
>   * the C-state (top nibble) and sub-state (bottom nibble)
> @@ -1547,6 +1555,161 @@ static void __init mwait_idle_state_tabl
>       }
>  }
>  
> + /**
> +  * get_cmdline_field - Get the current field from a cmdline string.
> +  * @args: The cmdline string to get the current field from.
> +  * @field: Pointer to the current field upon return.
> +  * @sep: The fields separator character.
> +  *
> +  * Examples:
> +  *   Input: args="C1:1:1,C1E:2:10", sep=':'
> +  *   Output: field="C1", return "1:1,C1E:2:10"
> +  *   Input: args="C1:1:1,C1E:2:10", sep=','
> +  *   Output: field="C1:1:1", return "C1E:2:10"
> +  *   Ipnut: args="::", sep=':'
> +  *   Output: field="", return ":"
> +  *
> +  * Return: The continuation of the cmdline string after the field or NULL.
> +  */
> +static char *__init get_cmdline_field(char *args, char **field, char sep)
> +{
> +     unsigned int i;
> +
> +     for (i = 0; args[i] && !isspace(args[i]); i++) {
> +             if (args[i] == sep)
> +                     break;
> +     }
> +
> +     *field = args;
> +
> +     if (args[i] != sep)
> +             return NULL;
> +
> +     args[i] = '\0';
> +     return args + i + 1;
> +}
> +
> +/**
> + * 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?

Thanks, Roger.