[PATCH for-4.22 v2 0/8] x86/mwait-idle: sync up with Linux 7.0-rc

[PATCH for-4.22 v2 0/8] x86/mwait-idle: sync up with Linux 7.0-rc

[Jan Beulich]

Includes a few custom changes, too.

1: arrange for BSP MSR adjustments during S3 resume
2: clean up BYT/CHT auto demotion disable
3: latch struct idle_cpu contents
4: move pre-initialized struct idle_cpu instances
5: Remove the 'preferred_cstates' parameter
6: drop const from struct cpuidle_state arrays
7: Add cmdline option to adjust C-states table
8: Add C-states validation

Jan

[PATCH for-4.22 v2 1/8] x86/mwait-idle: arrange for BSP MSR adjustments during S3 resume

[Jan Beulich]

mwait_idle_cpu_init() is only called for APs, yet MSR writes will
typically need re-doing post-S3 even for the BSP. When multiple cores /
threads are present (and to come back online) in a package, for package
scope MSRs this may be covered by APs doing the writes, but we can't rely
on that.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/acpi/power.c
+++ b/xen/arch/x86/acpi/power.c
@@ -28,6 +28,7 @@
 #include <asm/io_apic.h>
 #include <asm/irq.h>
 #include <asm/microcode.h>
+#include <asm/mwait.h>
 #include <asm/prot-key.h>
 #include <asm/spec_ctrl.h>
 #include <asm/tboot.h>
@@ -299,6 +300,7 @@ static int enter_state(u32 state)
     acpi_sleep_post(state);
     if ( hvm_cpu_up() )
         BUG();
+    mwait_idle_resume();
     cpufreq_add_cpu(0);
 
  enable_cpu:
--- a/xen/arch/x86/cpu/mwait-idle.c
+++ b/xen/arch/x86/cpu/mwait-idle.c
@@ -1677,6 +1677,28 @@ static int __init mwait_idle_probe(void)
 	return 0;
 }
 
+static void mwait_idle_cpu_tweak(unsigned int cpu)
+{
+	if (icpu->auto_demotion_disable_flags)
+		on_selected_cpus(cpumask_of(cpu), auto_demotion_disable, NULL, 1);
+
+	if (icpu->byt_auto_demotion_disable_flag)
+		on_selected_cpus(cpumask_of(cpu), byt_auto_demotion_disable, NULL, 1);
+
+	switch (icpu->c1e_promotion) {
+	case C1E_PROMOTION_DISABLE:
+		on_selected_cpus(cpumask_of(cpu), c1e_promotion_disable, NULL, 1);
+		break;
+
+	case C1E_PROMOTION_ENABLE:
+		on_selected_cpus(cpumask_of(cpu), c1e_promotion_enable, NULL, 1);
+		break;
+
+	case C1E_PROMOTION_PRESERVE:
+		break;
+	}
+}
+
 static int cf_check mwait_idle_cpu_init(
     struct notifier_block *nfb, unsigned long action, void *hcpu)
 {
@@ -1759,24 +1781,7 @@ static int cf_check mwait_idle_cpu_init(
 		dev->count++;
 	}
 
-	if (icpu->auto_demotion_disable_flags)
-		on_selected_cpus(cpumask_of(cpu), auto_demotion_disable, NULL, 1);
-
-	if (icpu->byt_auto_demotion_disable_flag)
-		on_selected_cpus(cpumask_of(cpu), byt_auto_demotion_disable, NULL, 1);
-
-	switch (icpu->c1e_promotion) {
-	case C1E_PROMOTION_DISABLE:
-		on_selected_cpus(cpumask_of(cpu), c1e_promotion_disable, NULL, 1);
-		break;
-
-	case C1E_PROMOTION_ENABLE:
-		on_selected_cpus(cpumask_of(cpu), c1e_promotion_enable, NULL, 1);
-		break;
-
-	case C1E_PROMOTION_PRESERVE:
-		break;
-	}
+	mwait_idle_cpu_tweak(cpu);
 
 	return NOTIFY_DONE;
 }
@@ -1808,6 +1813,14 @@ int __init mwait_idle_init(struct notifi
 	return err;
 }
 
+void mwait_idle_resume(void)
+{
+	if (!icpu)
+		return;
+
+	mwait_idle_cpu_tweak(smp_processor_id());
+}
+
 /* Helper function for HPET. */
 bool __init mwait_pc10_supported(void)
 {
--- a/xen/arch/x86/include/asm/mwait.h
+++ b/xen/arch/x86/include/asm/mwait.h
@@ -14,9 +14,12 @@
 #define MWAIT_ECX_INTERRUPT_BREAK	0x1
 
 void mwait_idle_with_hints(unsigned int eax, unsigned int ecx);
+
 #ifdef CONFIG_INTEL
+void mwait_idle_resume(void);
 bool mwait_pc10_supported(void);
 #else
+static inline void mwait_idle_resume(void) {}
 static inline bool mwait_pc10_supported(void)
 {
     return false;

[PATCH for-4.22 v2 2/8] x86/mwait-idle: clean up BYT/CHT auto demotion disable

[Jan Beulich]

Bay Trail (BYT) and Cherry Trail (CHT) platforms have a very specific way
of disabling auto-demotion via specific MSR bits. Clean up the code so that
BYT/CHT-specifics do not show up in the common 'struct idle_cpu' data
structure.

Remove the 'byt_auto_demotion_disable_flag' flag from 'struct idle_cpu',
because a better coding pattern is to avoid very case-specific fields like
'bool byt_auto_demotion_disable_flag' in a common data structure, which is
used for all platforms, not only BYT/CHT. The code is just more readable
when common data structures contain only commonly used fields.

Instead, match BYT/CHT in the 'intel_idle_init_cstates_icpu()' function,
and introduce a small helper to take care of BYT/CHT auto-demotion. This
is consistent with how platform-specific things are done for other
platforms.

No intended functional changes.

Inspired by (and description largely taken from) Linux'es c93d13b661a6
("intel_idle: clean up BYT/CHT auto demotion disable").

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: Init "first" to "true" in mwait_idle_cpu_init().

--- a/xen/arch/x86/cpu/mwait-idle.c
+++ b/xen/arch/x86/cpu/mwait-idle.c
@@ -103,7 +103,6 @@ struct idle_cpu {
 	 * Indicate which enable bits to clear here.
 	 */
 	unsigned long auto_demotion_disable_flags;
-	bool byt_auto_demotion_disable_flag;
 	enum c1e_promotion c1e_promotion;
 };
 
@@ -1144,7 +1143,7 @@ static void cf_check auto_demotion_disab
 	wrmsrl(MSR_PKG_CST_CONFIG_CONTROL, msr_bits);
 }
 
-static void cf_check byt_auto_demotion_disable(void *dummy)
+static void byt_cht_auto_demotion_disable(void)
 {
 	wrmsrl(MSR_CC6_DEMOTION_POLICY_CONFIG, 0);
 	wrmsrl(MSR_MC6_DEMOTION_POLICY_CONFIG, 0);
@@ -1195,13 +1194,11 @@ static const struct idle_cpu idle_cpu_sn
 static const struct idle_cpu idle_cpu_byt = {
 	.state_table = byt_cstates,
 	.c1e_promotion = C1E_PROMOTION_DISABLE,
-	.byt_auto_demotion_disable_flag = true,
 };
 
 static const struct idle_cpu idle_cpu_cht = {
 	.state_table = cht_cstates,
 	.c1e_promotion = C1E_PROMOTION_DISABLE,
-	.byt_auto_demotion_disable_flag = true,
 };
 
 static const struct idle_cpu idle_cpu_ivb = {
@@ -1677,14 +1674,11 @@ static int __init mwait_idle_probe(void)
 	return 0;
 }
 
-static void mwait_idle_cpu_tweak(unsigned int cpu)
+static void mwait_idle_cpu_tweak(unsigned int cpu, bool bsp)
 {
 	if (icpu->auto_demotion_disable_flags)
 		on_selected_cpus(cpumask_of(cpu), auto_demotion_disable, NULL, 1);
 
-	if (icpu->byt_auto_demotion_disable_flag)
-		on_selected_cpus(cpumask_of(cpu), byt_auto_demotion_disable, NULL, 1);
-
 	switch (icpu->c1e_promotion) {
 	case C1E_PROMOTION_DISABLE:
 		on_selected_cpus(cpumask_of(cpu), c1e_promotion_disable, NULL, 1);
@@ -1697,12 +1691,24 @@ static void mwait_idle_cpu_tweak(unsigne
 	case C1E_PROMOTION_PRESERVE:
 		break;
 	}
+
+	/* Pkg-scope MSRs on 1-socket-only systems need writing only once. */
+	if (!bsp)
+		return;
+
+	switch (boot_cpu_data.vfm) {
+	case INTEL_ATOM_SILVERMONT:
+	case INTEL_ATOM_AIRMONT:
+		byt_cht_auto_demotion_disable();
+		break;
+	}
 }
 
 static int cf_check mwait_idle_cpu_init(
     struct notifier_block *nfb, unsigned long action, void *hcpu)
 {
 	unsigned int cpu = (unsigned long)hcpu, cstate;
+	static bool first = true;
 	struct acpi_processor_power *dev = processor_powers[cpu];
 
 	switch (action) {
@@ -1781,7 +1787,8 @@ static int cf_check mwait_idle_cpu_init(
 		dev->count++;
 	}
 
-	mwait_idle_cpu_tweak(cpu);
+	mwait_idle_cpu_tweak(cpu, first);
+	first = false;
 
 	return NOTIFY_DONE;
 }
@@ -1818,7 +1825,7 @@ void mwait_idle_resume(void)
 	if (!icpu)
 		return;
 
-	mwait_idle_cpu_tweak(smp_processor_id());
+	mwait_idle_cpu_tweak(smp_processor_id(), true);
 }
 
 /* Helper function for HPET. */

[PATCH for-4.22 v2 3/8] x86/mwait-idle: latch struct idle_cpu contents

[Jan Beulich]

Rather than storing a pointer (and needing to keep all struct instances in
memory post-init), and rather than (like the Linux counterpart has it)
keeping individual variables, simply copy the respective structure
instance. By implication, subsequent updates now need doing to the copy.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>

--- a/xen/arch/x86/cpu/mwait-idle.c
+++ b/xen/arch/x86/cpu/mwait-idle.c
@@ -106,14 +106,14 @@ struct idle_cpu {
 	enum c1e_promotion c1e_promotion;
 };
 
-static const struct idle_cpu *__ro_after_init icpu;
+static struct idle_cpu __ro_after_init icpu;
 
-static const struct cpuidle_state {
+struct cpuidle_state {
 	char		name[16];
 	unsigned int	flags;
 	unsigned int	exit_latency; /* in US */
 	unsigned int	target_residency; /* in US */
-} *__ro_after_init cpuidle_state_table;
+};
 
 #define CPUIDLE_FLAG_DISABLED		0x1
 /*
@@ -1097,7 +1097,7 @@ static void cf_check mwait_idle(void)
 	 * leave_mm() to avoid costly and often unnecessary wakeups
 	 * for flushing the user TLB's associated with the active mm.
 	 */
-	if (cpuidle_state_table[].flags & CPUIDLE_FLAG_TLB_FLUSHED)
+	if (icpu.state_table[].flags & CPUIDLE_FLAG_TLB_FLUSHED)
 		leave_mm(cpu);
 #endif
 
@@ -1139,7 +1139,7 @@ static void cf_check auto_demotion_disab
 	u64 msr_bits;
 
 	rdmsrl(MSR_PKG_CST_CONFIG_CONTROL, msr_bits);
-	msr_bits &= ~(icpu->auto_demotion_disable_flags);
+	msr_bits &= ~icpu.auto_demotion_disable_flags;
 	wrmsrl(MSR_PKG_CST_CONFIG_CONTROL, msr_bits);
 }
 
@@ -1371,10 +1371,10 @@ static void __init ivt_idle_state_table_
 		/* 1 and 2 socket systems use default ivt_cstates */
 		break;
 	case 2: case 3:
-		cpuidle_state_table = ivt_cstates_4s;
+		icpu.state_table = ivt_cstates_4s;
 		break;
 	default:
-		cpuidle_state_table = ivt_cstates_8s;
+		icpu.state_table = ivt_cstates_8s;
 		break;
 	}
 }
@@ -1525,14 +1525,12 @@ static void __init adl_idle_state_table_
 		adl_l_cstates[1].flags |= CPUIDLE_FLAG_DISABLED;
 
 		/* Disable C1E by clearing the "C1E promotion" bit. */
-		idle_cpu_adl.c1e_promotion = C1E_PROMOTION_DISABLE;
-		idle_cpu_adl_l.c1e_promotion = C1E_PROMOTION_DISABLE;
+		icpu.c1e_promotion = C1E_PROMOTION_DISABLE;
 		return;
 	}
 
 	/* Make sure C1E is enabled by default */
-	idle_cpu_adl.c1e_promotion = C1E_PROMOTION_ENABLE;
-	idle_cpu_adl_l.c1e_promotion = C1E_PROMOTION_ENABLE;
+	icpu.c1e_promotion = C1E_PROMOTION_ENABLE;
 }
 
 /*
@@ -1593,6 +1591,7 @@ static int __init mwait_idle_probe(void)
 {
 	unsigned int eax, ebx, ecx;
 	const struct x86_cpu_id *id;
+	const struct idle_cpu *idle_cpu;
 	const char *str;
 
 	if (boot_cpu_data.vendor != X86_VENDOR_INTEL)
@@ -1627,8 +1626,8 @@ static int __init mwait_idle_probe(void)
 
 	pr_debug(PREFIX "MWAIT substates: %#x\n", mwait_substates);
 
-	icpu = id->driver_data;
-	cpuidle_state_table = icpu->state_table;
+	idle_cpu = id->driver_data;
+	icpu = *idle_cpu;
 
 	if (boot_cpu_has(X86_FEATURE_XEN_ARAT))
 		lapic_timer_reliable_states = LAPIC_TIMER_ALWAYS_RELIABLE;
@@ -1644,7 +1643,7 @@ static int __init mwait_idle_probe(void)
 		const char *ss;
 
 		do {
-			const struct cpuidle_state *state = icpu->state_table;
+			const struct cpuidle_state *state = idle_cpu->state_table;
 			unsigned int bit = 1;
 
 			ss = strchr(str, ',');
@@ -1676,10 +1675,10 @@ static int __init mwait_idle_probe(void)
 
 static void mwait_idle_cpu_tweak(unsigned int cpu, bool bsp)
 {
-	if (icpu->auto_demotion_disable_flags)
+	if (icpu.auto_demotion_disable_flags)
 		on_selected_cpus(cpumask_of(cpu), auto_demotion_disable, NULL, 1);
 
-	switch (icpu->c1e_promotion) {
+	switch (icpu.c1e_promotion) {
 	case C1E_PROMOTION_DISABLE:
 		on_selected_cpus(cpumask_of(cpu), c1e_promotion_disable, NULL, 1);
 		break;
@@ -1732,11 +1731,11 @@ static int cf_check mwait_idle_cpu_init(
 
 	dev->count = 1;
 
-	for (cstate = 0; cpuidle_state_table[cstate].target_residency; ++cstate) {
+	for (cstate = 0; icpu.state_table[cstate].target_residency; ++cstate) {
 		unsigned int num_substates, hint, state;
 		struct acpi_processor_cx *cx;
 
-		hint = flg2MWAIT(cpuidle_state_table[cstate].flags);
+		hint = flg2MWAIT(icpu.state_table[cstate].flags);
 		state = MWAIT_HINT2CSTATE(hint) + 1;
 
 		if (state > max_cstate()) {
@@ -1752,10 +1751,10 @@ static int cf_check mwait_idle_cpu_init(
 			continue;
 
 		/* if state marked as disabled, skip it */
-		if (cpuidle_state_table[cstate].flags &
+		if (icpu.state_table[cstate].flags &
 		    CPUIDLE_FLAG_DISABLED) {
 			printk(XENLOG_DEBUG PREFIX "state %s is disabled\n",
-			       cpuidle_state_table[cstate].name);
+			       icpu.state_table[cstate].name);
 			continue;
 		}
 
@@ -1773,15 +1772,15 @@ static int cf_check mwait_idle_cpu_init(
 		cx->type = state;
 		cx->address = hint;
 		cx->entry_method = ACPI_CSTATE_EM_FFH;
-		cx->latency = cpuidle_state_table[cstate].exit_latency;
+		cx->latency = icpu.state_table[cstate].exit_latency;
 		cx->target_residency =
-			cpuidle_state_table[cstate].target_residency;
-		if ((cpuidle_state_table[cstate].flags &
+			icpu.state_table[cstate].target_residency;
+		if ((icpu.state_table[cstate].flags &
 		     CPUIDLE_FLAG_IRQ_ENABLE) &&
 		    /* cstate_restore_tsc() needs to be a no-op */
 		    boot_cpu_has(X86_FEATURE_NONSTOP_TSC))
 			cx->irq_enable_early = true;
-		if (cpuidle_state_table[cstate].flags & CPUIDLE_FLAG_IBRS)
+		if (icpu.state_table[cstate].flags & CPUIDLE_FLAG_IBRS)
 			cx->ibrs_disable = true;
 
 		dev->count++;
@@ -1822,7 +1821,7 @@ int __init mwait_idle_init(struct notifi
 
 void mwait_idle_resume(void)
 {
-	if (!icpu)
+	if (!icpu.state_table)
 		return;
 
 	mwait_idle_cpu_tweak(smp_processor_id(), true);

[PATCH for-4.22 v2 4/8] x86/mwait-idle: move pre-initialized struct idle_cpu instances

[Jan Beulich]

Now that they're not referenced anymore post-init, they can themselves
move into .init.rodata. (idle_cpu_adl{,_l} can also become const in the
first place.)

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>

--- a/xen/arch/x86/cpu/mwait-idle.c
+++ b/xen/arch/x86/cpu/mwait-idle.c
@@ -1167,132 +1167,132 @@ static void cf_check c1e_promotion_disab
 	wrmsrl(MSR_IA32_POWER_CTL, msr_bits);
 }
 
-static const struct idle_cpu idle_cpu_nehalem = {
+static const struct idle_cpu __initconstrel idle_cpu_nehalem = {
 	.state_table = nehalem_cstates,
 	.auto_demotion_disable_flags = NHM_C1_AUTO_DEMOTE | NHM_C3_AUTO_DEMOTE,
 	.c1e_promotion = C1E_PROMOTION_DISABLE,
 };
 
-static const struct idle_cpu idle_cpu_atom = {
+static const struct idle_cpu __initconstrel idle_cpu_atom = {
 	.state_table = atom_cstates,
 };
 
-static const struct idle_cpu idle_cpu_tangier = {
+static const struct idle_cpu __initconstrel idle_cpu_tangier = {
 	.state_table = tangier_cstates,
 };
 
-static const struct idle_cpu idle_cpu_lincroft = {
+static const struct idle_cpu __initconstrel idle_cpu_lincroft = {
 	.state_table = atom_cstates,
 	.auto_demotion_disable_flags = ATM_LNC_C6_AUTO_DEMOTE,
 };
 
-static const struct idle_cpu idle_cpu_snb = {
+static const struct idle_cpu __initconstrel idle_cpu_snb = {
 	.state_table = snb_cstates,
 	.c1e_promotion = C1E_PROMOTION_DISABLE,
 };
 
-static const struct idle_cpu idle_cpu_byt = {
+static const struct idle_cpu __initconstrel idle_cpu_byt = {
 	.state_table = byt_cstates,
 	.c1e_promotion = C1E_PROMOTION_DISABLE,
 };
 
-static const struct idle_cpu idle_cpu_cht = {
+static const struct idle_cpu __initconstrel idle_cpu_cht = {
 	.state_table = cht_cstates,
 	.c1e_promotion = C1E_PROMOTION_DISABLE,
 };
 
-static const struct idle_cpu idle_cpu_ivb = {
+static const struct idle_cpu __initconstrel idle_cpu_ivb = {
 	.state_table = ivb_cstates,
 	.c1e_promotion = C1E_PROMOTION_DISABLE,
 };
 
-static const struct idle_cpu idle_cpu_ivt = {
+static const struct idle_cpu __initconstrel idle_cpu_ivt = {
 	.state_table = ivt_cstates,
 	.c1e_promotion = C1E_PROMOTION_DISABLE,
 };
 
-static const struct idle_cpu idle_cpu_hsw = {
+static const struct idle_cpu __initconstrel idle_cpu_hsw = {
 	.state_table = hsw_cstates,
 	.c1e_promotion = C1E_PROMOTION_DISABLE,
 };
 
-static const struct idle_cpu idle_cpu_bdw = {
+static const struct idle_cpu __initconstrel idle_cpu_bdw = {
 	.state_table = bdw_cstates,
 	.c1e_promotion = C1E_PROMOTION_DISABLE,
 };
 
-static const struct idle_cpu idle_cpu_skl = {
+static const struct idle_cpu __initconstrel idle_cpu_skl = {
 	.state_table = skl_cstates,
 	.c1e_promotion = C1E_PROMOTION_DISABLE,
 };
 
-static const struct idle_cpu idle_cpu_skx = {
+static const struct idle_cpu __initconstrel idle_cpu_skx = {
 	.state_table = skx_cstates,
 	.c1e_promotion = C1E_PROMOTION_DISABLE,
 };
 
-static const struct idle_cpu idle_cpu_icx = {
+static const struct idle_cpu __initconstrel idle_cpu_icx = {
 	.state_table = icx_cstates,
 	.c1e_promotion = C1E_PROMOTION_DISABLE,
 };
 
-static struct idle_cpu __ro_after_init idle_cpu_adl = {
+static const struct idle_cpu __initconstrel idle_cpu_adl = {
 	.state_table = adl_cstates,
 };
 
-static struct idle_cpu __ro_after_init idle_cpu_adl_l = {
+static const struct idle_cpu __initconstrel idle_cpu_adl_l = {
 	.state_table = adl_l_cstates,
 };
 
-static const struct idle_cpu idle_cpu_mtl_l = {
+static const struct idle_cpu __initconstrel idle_cpu_mtl_l = {
 	.state_table = mtl_l_cstates,
 };
 
-static const struct idle_cpu idle_cpu_gmt = {
+static const struct idle_cpu __initconstrel idle_cpu_gmt = {
 	.state_table = gmt_cstates,
 };
 
-static const struct idle_cpu idle_cpu_spr = {
+static const struct idle_cpu __initconstrel idle_cpu_spr = {
 	.state_table = spr_cstates,
 	.c1e_promotion = C1E_PROMOTION_DISABLE,
 };
 
-static const struct idle_cpu idle_cpu_gnr = {
+static const struct idle_cpu __initconstrel idle_cpu_gnr = {
 	.state_table = gnr_cstates,
 	.c1e_promotion = C1E_PROMOTION_DISABLE,
 };
 
-static const struct idle_cpu idle_cpu_gnrd = {
+static const struct idle_cpu __initconstrel idle_cpu_gnrd = {
 	.state_table = gnrd_cstates,
 	.c1e_promotion = C1E_PROMOTION_DISABLE,
 };
 
-static const struct idle_cpu idle_cpu_avn = {
+static const struct idle_cpu __initconstrel idle_cpu_avn = {
 	.state_table = avn_cstates,
 	.c1e_promotion = C1E_PROMOTION_DISABLE,
 };
 
-static const struct idle_cpu idle_cpu_bxt = {
+static const struct idle_cpu __initconstrel idle_cpu_bxt = {
 	.state_table = bxt_cstates,
 	.c1e_promotion = C1E_PROMOTION_DISABLE,
 };
 
-static const struct idle_cpu idle_cpu_dnv = {
+static const struct idle_cpu __initconstrel idle_cpu_dnv = {
 	.state_table = dnv_cstates,
 	.c1e_promotion = C1E_PROMOTION_DISABLE,
 };
 
-static const struct idle_cpu idle_cpu_snr = {
+static const struct idle_cpu __initconstrel idle_cpu_snr = {
 	.state_table = snr_cstates,
 	.c1e_promotion = C1E_PROMOTION_DISABLE,
 };
 
-static const struct idle_cpu idle_cpu_grr = {
+static const struct idle_cpu __initconstrel idle_cpu_grr = {
 	.state_table = grr_cstates,
 	.c1e_promotion = C1E_PROMOTION_DISABLE,
 };
 
-static const struct idle_cpu idle_cpu_srf = {
+static const struct idle_cpu __initconstrel idle_cpu_srf = {
 	.state_table = srf_cstates,
 	.c1e_promotion = C1E_PROMOTION_DISABLE,
 };

[PATCH for-4.22 v2 5/8] x86/mwait-idle: Remove the 'preferred_cstates' parameter

[Jan Beulich]

From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

Remove the 'preferred_cstates' module parameter as it is not really useful.

The parameter currently only affects Alder Lake, where it controls C1/C1E
preference, with C1E being the default. The parameter does not support any
other platform. For example, Meteor Lake has a similar C1/C1E limitation,
but the parameter does not support Meteor Lake. This indicates that the
parameter is not very useful.

Generally, independent C1 and C1E are important for server platforms where
low latency is key. However, they are not as important for client platforms,
like Alder Lake, where C1E providing better energy savings is generally
preferred.

The parameter was originally introduced for Sapphire Rapids Xeon:
da0e58c038e6 intel_idle: add 'preferred_cstates' module argument

Later it was added to Alder Lake:
d1cf8bbfed1ed ("intel_idle: Add AlderLake support")

But it was removed from Sapphire Rapids when firmware fixed the C1/C1E
limitation:
1548fac47a114 ("intel_idle: make SPR C1 and C1E be independent")

So Alder Lake is the only platform left where this parameter has any effect.
Remove this parameter to simplify the driver and reduce maintenance burden.

Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Link: https://patch.msgid.link/20251215111300.132803-1-dedekind1@gmail.com
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Origin: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git a36dc37b5672
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>

--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -2071,12 +2071,6 @@ compression is selected at build time fr
 ### ple_window (Intel)
 > `= <integer>`
 
-### preferred-cstates (x86)
-> `= ( <integer> | List of ( C1 | C1E | C2 | ... )`
-
-This is a mask of C-states which are to be used preferably.  This option is
-applicable only on hardware were certain C-states are exclusive of one another.
-
 ### probe-port-aliases (x86)
 > `= <boolean>`
 
--- a/xen/arch/x86/cpu/mwait-idle.c
+++ b/xen/arch/x86/cpu/mwait-idle.c
@@ -72,19 +72,6 @@ boolean_param("mwait-idle", opt_mwait_id
 
 static unsigned int mwait_substates;
 
-/*
- * Some platforms come with mutually exclusive C-states, so that if one is
- * enabled, the other C-states must not be used. Example: C1 and C1E on
- * Sapphire Rapids platform. This parameter allows for selecting the
- * preferred C-states among the groups of mutually exclusive C-states - the
- * selected C-states will be registered, the other C-states from the mutually
- * exclusive group won't be registered. If the platform has no mutually
- * exclusive C-states, this parameter has no effect.
- */
-static unsigned int __ro_after_init preferred_states_mask;
-static char __initdata preferred_states[64];
-string_param("preferred-cstates", preferred_states);
-
 #define LAPIC_TIMER_ALWAYS_RELIABLE 0xFFFFFFFF
 /* Reliable LAPIC Timer States, bit 1 for C1 etc. Default to only C1. */
 static unsigned int lapic_timer_reliable_states = (1 << 1);
@@ -1512,28 +1499,6 @@ static void __init skx_idle_state_table_
 }
 
 /*
- * adl_idle_state_table_update - Adjust AlderLake idle states table.
- */
-static void __init adl_idle_state_table_update(void)
-{
-	/* Check if user prefers C1 over C1E. */
-	if ((preferred_states_mask & BIT(1, U)) &&
-	    !(preferred_states_mask & BIT(2, U))) {
-		adl_cstates[0].flags &= ~CPUIDLE_FLAG_DISABLED;
-		adl_cstates[1].flags |= CPUIDLE_FLAG_DISABLED;
-		adl_l_cstates[0].flags &= ~CPUIDLE_FLAG_DISABLED;
-		adl_l_cstates[1].flags |= CPUIDLE_FLAG_DISABLED;
-
-		/* Disable C1E by clearing the "C1E promotion" bit. */
-		icpu.c1e_promotion = C1E_PROMOTION_DISABLE;
-		return;
-	}
-
-	/* Make sure C1E is enabled by default */
-	icpu.c1e_promotion = C1E_PROMOTION_ENABLE;
-}
-
-/*
  * spr_idle_state_table_update - Adjust Sapphire Rapids idle states table.
  */
 static void __init spr_idle_state_table_update(void)
@@ -1579,11 +1544,6 @@ static void __init mwait_idle_state_tabl
 	case INTEL_EMERALDRAPIDS_X:
 		spr_idle_state_table_update();
 		break;
-	case INTEL_ALDERLAKE:
-	case INTEL_ALDERLAKE_L:
-	case INTEL_ATOM_GRACEMONT:
-		adl_idle_state_table_update();
-		break;
 	}
 }
 
@@ -1592,7 +1552,6 @@ static int __init mwait_idle_probe(void)
 	unsigned int eax, ebx, ecx;
 	const struct x86_cpu_id *id;
 	const struct idle_cpu *idle_cpu;
-	const char *str;
 
 	if (boot_cpu_data.vendor != X86_VENDOR_INTEL)
 		return -ENODEV;
@@ -1635,39 +1594,6 @@ static int __init mwait_idle_probe(void)
 	pr_debug(PREFIX "lapic_timer_reliable_states %#x\n",
 		 lapic_timer_reliable_states);
 
-	str = preferred_states;
-	if (isdigit(str[0]))
-		preferred_states_mask = simple_strtoul(str, &str, 0);
-	else if (str[0])
-	{
-		const char *ss;
-
-		do {
-			const struct cpuidle_state *state = idle_cpu->state_table;
-			unsigned int bit = 1;
-
-			ss = strchr(str, ',');
-			if (!ss)
-				ss = strchr(str, '\0');
-
-			for (; state->name[0]; ++state) {
-				bit <<= 1;
-				if (!cmdline_strcmp(str, state->name)) {
-					preferred_states_mask |= bit;
-					break;
-				}
-			}
-			if (!state->name[0])
-				break;
-
-			str = ss + 1;
-		} while (*ss);
-
-		str -= str == ss + 1;
-	}
-	if (str[0])
-		printk("unrecognized \"preferred-cstates=%s\"\n", str);
-
 	mwait_idle_state_table_update();
 
 	return 0;

[PATCH for-4.22 v2 6/8] x86/mwait-idle: drop const from struct cpuidle_state arrays

[Jan Beulich]

A subsequent change will want to be able to alter them based on a new
command line option. (Note that some were __ro_after_init already.)

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>

--- a/xen/arch/x86/cpu/mwait-idle.c
+++ b/xen/arch/x86/cpu/mwait-idle.c
@@ -83,7 +83,7 @@ enum c1e_promotion {
 };
 
 struct idle_cpu {
-	const struct cpuidle_state *state_table;
+	struct cpuidle_state *state_table;
 
 	/*
 	 * Hardware C-state auto-demotion may not always be optimal.
@@ -139,7 +139,7 @@ struct cpuidle_state {
  * which is also the index into the MWAIT hint array.
  * Thus C0 is a dummy.
  */
-static const struct cpuidle_state nehalem_cstates[] = {
+static struct cpuidle_state __ro_after_init nehalem_cstates[] = {
 	{
 		.name = "C1",
 		.flags = MWAIT2flg(0x00),
@@ -167,7 +167,7 @@ static const struct cpuidle_state nehale
 	{}
 };
 
-static const struct cpuidle_state snb_cstates[] = {
+static struct cpuidle_state __ro_after_init snb_cstates[] = {
 	{
 		.name = "C1",
 		.flags = MWAIT2flg(0x00),
@@ -201,7 +201,7 @@ static const struct cpuidle_state snb_cs
 	{}
 };
 
-static const struct cpuidle_state byt_cstates[] = {
+static struct cpuidle_state __ro_after_init byt_cstates[] = {
 	{
 		.name = "C1",
 		.flags = MWAIT2flg(0x00),
@@ -235,7 +235,7 @@ static const struct cpuidle_state byt_cs
 	{}
 };
 
-static const struct cpuidle_state cht_cstates[] = {
+static struct cpuidle_state __ro_after_init cht_cstates[] = {
 	{
 		.name = "C1",
 		.flags = MWAIT2flg(0x00),
@@ -269,7 +269,7 @@ static const struct cpuidle_state cht_cs
 	{}
 };
 
-static const struct cpuidle_state ivb_cstates[] = {
+static struct cpuidle_state __ro_after_init ivb_cstates[] = {
 	{
 		.name = "C1",
 		.flags = MWAIT2flg(0x00),
@@ -303,7 +303,7 @@ static const struct cpuidle_state ivb_cs
 	{}
 };
 
-static const struct cpuidle_state ivt_cstates[] = {
+static struct cpuidle_state __ro_after_init ivt_cstates[] = {
 	{
 		.name = "C1",
 		.flags = MWAIT2flg(0x00),
@@ -331,7 +331,7 @@ static const struct cpuidle_state ivt_cs
 	{}
 };
 
-static const struct cpuidle_state ivt_cstates_4s[] = {
+static struct cpuidle_state __ro_after_init ivt_cstates_4s[] = {
 	{
 		.name = "C1",
 		.flags = MWAIT2flg(0x00),
@@ -359,7 +359,7 @@ static const struct cpuidle_state ivt_cs
 	{}
 };
 
-static const struct cpuidle_state ivt_cstates_8s[] = {
+static struct cpuidle_state __ro_after_init ivt_cstates_8s[] = {
 	{
 		.name = "C1",
 		.flags = MWAIT2flg(0x00),
@@ -387,7 +387,7 @@ static const struct cpuidle_state ivt_cs
 	{}
 };
 
-static const struct cpuidle_state hsw_cstates[] = {
+static struct cpuidle_state __ro_after_init hsw_cstates[] = {
 	{
 		.name = "C1",
 		.flags = MWAIT2flg(0x00),
@@ -439,7 +439,7 @@ static const struct cpuidle_state hsw_cs
 	{}
 };
 
-static const struct cpuidle_state bdw_cstates[] = {
+static struct cpuidle_state __ro_after_init bdw_cstates[] = {
 	{
 		.name = "C1",
 		.flags = MWAIT2flg(0x00),
@@ -565,7 +565,7 @@ static struct cpuidle_state __ro_after_i
 	{}
 };
 
-static const struct cpuidle_state icx_cstates[] = {
+static struct cpuidle_state __ro_after_init icx_cstates[] = {
        {
                .name = "C1",
                .flags = MWAIT2flg(0x00) | CPUIDLE_FLAG_IRQ_ENABLE,
@@ -665,7 +665,7 @@ static struct cpuidle_state __ro_after_i
 	{}
 };
 
-static const struct cpuidle_state mtl_l_cstates[] = {
+static struct cpuidle_state __ro_after_init mtl_l_cstates[] = {
 	{
 		.name = "C1E",
 		.flags = MWAIT2flg(0x01),
@@ -687,7 +687,7 @@ static const struct cpuidle_state mtl_l_
 	{}
 };
 
-static const struct cpuidle_state gmt_cstates[] = {
+static struct cpuidle_state __ro_after_init gmt_cstates[] = {
 	{
 		.name = "C1",
 		.flags = MWAIT2flg(0x00) | CPUIDLE_FLAG_DISABLED,
@@ -743,7 +743,7 @@ static struct cpuidle_state __ro_after_i
 	{}
 };
 
-static const struct cpuidle_state gnr_cstates[] = {
+static struct cpuidle_state __ro_after_init gnr_cstates[] = {
 	{
 		.name = "C1",
 		.flags = MWAIT2flg(0x00),
@@ -771,7 +771,7 @@ static const struct cpuidle_state gnr_cs
 	{}
 };
 
-static const struct cpuidle_state gnrd_cstates[] = {
+static struct cpuidle_state __ro_after_init gnrd_cstates[] = {
 	{
 		.name = "C1",
 		.flags = MWAIT2flg(0x00),
@@ -799,7 +799,7 @@ static const struct cpuidle_state gnrd_c
 	{}
 };
 
-static const struct cpuidle_state atom_cstates[] = {
+static struct cpuidle_state __ro_after_init atom_cstates[] = {
 	{
 		.name = "C1E",
 		.flags = MWAIT2flg(0x00),
@@ -827,7 +827,7 @@ static const struct cpuidle_state atom_c
 	{}
 };
 
-static const struct cpuidle_state tangier_cstates[] = {
+static struct cpuidle_state __ro_after_init tangier_cstates[] = {
 	{
 		.name = "C1",
 		.flags = MWAIT2flg(0x00),
@@ -861,7 +861,7 @@ static const struct cpuidle_state tangie
 	{}
 };
 
-static const struct cpuidle_state avn_cstates[] = {
+static struct cpuidle_state __ro_after_init avn_cstates[] = {
 	{
 		.name = "C1",
 		.flags = MWAIT2flg(0x00),
@@ -923,7 +923,7 @@ static struct cpuidle_state __ro_after_i
 	{}
 };
 
-static const struct cpuidle_state dnv_cstates[] = {
+static struct cpuidle_state __ro_after_init dnv_cstates[] = {
 	{
 		.name = "C1",
 		.flags = MWAIT2flg(0x00),
@@ -949,7 +949,7 @@ static const struct cpuidle_state dnv_cs
  * Note, depending on HW and FW revision, SnowRidge SoC may or may not support
  * C6, and this is indicated in the CPUID mwait leaf.
  */
-static const struct cpuidle_state snr_cstates[] = {
+static struct cpuidle_state __ro_after_init snr_cstates[] = {
 	{
 		.name = "C1",
 		.flags = MWAIT2flg(0x00),
@@ -971,7 +971,7 @@ static const struct cpuidle_state snr_cs
 	{}
 };
 
-static const struct cpuidle_state grr_cstates[] = {
+static struct cpuidle_state __ro_after_init grr_cstates[] = {
 	{
 		.name = "C1",
 		.flags = MWAIT2flg(0x00),
@@ -993,7 +993,7 @@ static const struct cpuidle_state grr_cs
 	{}
 };
 
-static const struct cpuidle_state srf_cstates[] = {
+static struct cpuidle_state __ro_after_init srf_cstates[] = {
 	{
 		.name = "C1",
 		.flags = MWAIT2flg(0x00),

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

[Jan Beulich]

From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

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@linux.intel.com>
Link: https://patch.msgid.link/20251216080402.156988-3-dedekind1@gmail.com
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
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@suse.com>
---
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
+	       "Failed to adjust C-states with data from 'mwait-idle.table'\n");
+}
+
 static int __init mwait_idle_probe(void)
 {
 	unsigned int eax, ebx, ecx;
@@ -1596,6 +1759,8 @@ static int __init mwait_idle_probe(void)
 
 	mwait_idle_state_table_update();
 
+	cmdline_table_adjust();
+
 	return 0;
 }
 

[PATCH for-4.22 v2 8/8] x86/mwait-idle: Add C-states validation

[Jan Beulich]

From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

Add validation for C-states specified via the "table=" module parameter.
Treat this module parameter as untrusted input and validate it thoroughly.

Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Link: https://patch.msgid.link/20251216080402.156988-4-dedekind1@gmail.com
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Origin: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git be6a150829b3

Add __init to validate_cmdline_cstate(). Other adjustments to fit our env.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: Const-ify validate_cmdline_cstate()'s parameters.

--- a/xen/arch/x86/cpu/mwait-idle.c
+++ b/xen/arch/x86/cpu/mwait-idle.c
@@ -72,6 +72,11 @@ boolean_param("mwait-idle", opt_mwait_id
 
 /* The maximum allowed length for the 'table' module parameter  */
 #define MAX_CMDLINE_TABLE_LEN 256
+/* Maximum allowed C-state latency */
+#define MAX_CMDLINE_LATENCY_US (5 * 1000 /* USEC_PER_MSEC */)
+/* Maximum allowed C-state target residency */
+#define MAX_CMDLINE_RESIDENCY_US (100 * 1000 /* USEC_PER_MSEC */)
+
 static char cmdline_table_str[MAX_CMDLINE_TABLE_LEN] __initdata;
 string_param("mwait-idle.table", cmdline_table_str);
 
@@ -1590,6 +1595,41 @@ static char *__init get_cmdline_field(ch
 }
 
 /**
+ * validate_cmdline_cstate - Validate a C-state from cmdline.
+ * @state: The C-state to validate.
+ * @prev_state: The previous C-state in the table or NULL.
+ *
+ * Return: 0 if the C-state is valid or -EINVAL otherwise.
+ */
+static int __init validate_cmdline_cstate(const struct cpuidle_state *state,
+					  const struct cpuidle_state *prev_state)
+{
+	if (state->exit_latency == 0)
+		/* Exit latency 0 can only be used for the POLL state */
+		return -EINVAL;
+
+	if (state->exit_latency > MAX_CMDLINE_LATENCY_US)
+		return -EINVAL;
+
+	if (state->target_residency > MAX_CMDLINE_RESIDENCY_US)
+		return -EINVAL;
+
+	if (state->target_residency < state->exit_latency)
+		return -EINVAL;
+
+	if (!prev_state)
+		return 0;
+
+	if (state->exit_latency <= prev_state->exit_latency)
+		return -EINVAL;
+
+	if (state->target_residency <= prev_state->target_residency)
+		return -EINVAL;
+
+	return 0;
+}
+
+/**
  * 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
@@ -1697,6 +1737,21 @@ static void __init cmdline_table_adjust(
 		       state->name, state->exit_latency, state->target_residency);
 	}
 
+	/* Validate the adjusted C-states */
+	for (i = 0; i < state_count; i++) {
+		struct cpuidle_state *prev_state;
+
+		state = &cmdline_states[i];
+		prev_state = i ? &cmdline_states[i - 1] : NULL;
+
+		if (validate_cmdline_cstate(state, prev_state)) {
+			printk(XENLOG_ERR PREFIX
+			       "C-state '%s' validation failed\n",
+			       state->name);
+			goto error;
+		}
+	}
+
 	/* Copy the adjusted C-states table back */
 	for (i = 0; i < state_count; i++)
 		icpu.state_table[i] = cmdline_states[i];

Re: [PATCH for-4.22 v2 0/8] x86/mwait-idle: sync up with Linux 7.0-rc

[Jan Beulich]

On 12.05.2026 17:34, Jan Beulich wrote:
> Includes a few custom changes, too.
> 
> 1: arrange for BSP MSR adjustments during S3 resume
> 2: clean up BYT/CHT auto demotion disable
> 3: latch struct idle_cpu contents
> 4: move pre-initialized struct idle_cpu instances
> 5: Remove the 'preferred_cstates' parameter
> 6: drop const from struct cpuidle_state arrays
> 7: Add cmdline option to adjust C-states table
> 8: Add C-states validation

I probably should have Cc-ed you on the v2 submission.

Jan

Re: [PATCH for-4.22 v2 1/8] x86/mwait-idle: arrange for BSP MSR adjustments during S3 resume

[Roger Pau Monné]

On Tue, May 12, 2026 at 05:35:33PM +0200, Jan Beulich wrote:
> mwait_idle_cpu_init() is only called for APs, yet MSR writes will
> typically need re-doing post-S3 even for the BSP. When multiple cores /
> threads are present (and to come back online) in a package, for package
> scope MSRs this may be covered by APs doing the writes, but we can't rely
> on that.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

It's a bit unfortunate that we end up doing self-IPIs for the BSP
also, but that should be fixed elsewhere.

Acked-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks, Roger.

Re: [PATCH for-4.22 v2 2/8] x86/mwait-idle: clean up BYT/CHT auto demotion disable

[Roger Pau Monné]

On Tue, May 12, 2026 at 05:36:06PM +0200, Jan Beulich wrote:
> Bay Trail (BYT) and Cherry Trail (CHT) platforms have a very specific way
> of disabling auto-demotion via specific MSR bits. Clean up the code so that
> BYT/CHT-specifics do not show up in the common 'struct idle_cpu' data
> structure.
> 
> Remove the 'byt_auto_demotion_disable_flag' flag from 'struct idle_cpu',
> because a better coding pattern is to avoid very case-specific fields like
> 'bool byt_auto_demotion_disable_flag' in a common data structure, which is
> used for all platforms, not only BYT/CHT. The code is just more readable
> when common data structures contain only commonly used fields.
> 
> Instead, match BYT/CHT in the 'intel_idle_init_cstates_icpu()' function,
> and introduce a small helper to take care of BYT/CHT auto-demotion. This
> is consistent with how platform-specific things are done for other
> platforms.
> 
> No intended functional changes.
> 
> Inspired by (and description largely taken from) Linux'es c93d13b661a6
> ("intel_idle: clean up BYT/CHT auto demotion disable").
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks, Roger.

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

[Roger Pau Monné]

On Tue, May 12, 2026 at 05:38:08PM +0200, Jan Beulich wrote:
> From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> 
> 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@linux.intel.com>
> Link: https://patch.msgid.link/20251216080402.156988-3-dedekind1@gmail.com
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 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@suse.com>

Acked-by: Roger Pau Monné <roger.pau@citrix.com>

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.

Re: [PATCH for-4.22 v2 8/8] x86/mwait-idle: Add C-states validation

[Roger Pau Monné]

On Tue, May 12, 2026 at 05:38:32PM +0200, Jan Beulich wrote:
> From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> 
> Add validation for C-states specified via the "table=" module parameter.
> Treat this module parameter as untrusted input and validate it thoroughly.
> 
> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> Link: https://patch.msgid.link/20251216080402.156988-4-dedekind1@gmail.com
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Origin: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git be6a150829b3
> 
> Add __init to validate_cmdline_cstate(). Other adjustments to fit our env.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Roger Pau Monné <roger.pau@citrix.com>

One comment below.

> ---
> v2: Const-ify validate_cmdline_cstate()'s parameters.
> 
> --- a/xen/arch/x86/cpu/mwait-idle.c
> +++ b/xen/arch/x86/cpu/mwait-idle.c
> @@ -72,6 +72,11 @@ boolean_param("mwait-idle", opt_mwait_id
>  
>  /* The maximum allowed length for the 'table' module parameter  */
>  #define MAX_CMDLINE_TABLE_LEN 256
> +/* Maximum allowed C-state latency */
> +#define MAX_CMDLINE_LATENCY_US (5 * 1000 /* USEC_PER_MSEC */)
> +/* Maximum allowed C-state target residency */
> +#define MAX_CMDLINE_RESIDENCY_US (100 * 1000 /* USEC_PER_MSEC */)
> +
>  static char cmdline_table_str[MAX_CMDLINE_TABLE_LEN] __initdata;
>  string_param("mwait-idle.table", cmdline_table_str);
>  
> @@ -1590,6 +1595,41 @@ static char *__init get_cmdline_field(ch
>  }
>  
>  /**
> + * validate_cmdline_cstate - Validate a C-state from cmdline.
> + * @state: The C-state to validate.
> + * @prev_state: The previous C-state in the table or NULL.
> + *
> + * Return: 0 if the C-state is valid or -EINVAL otherwise.
> + */
> +static int __init validate_cmdline_cstate(const struct cpuidle_state *state,
> +					  const struct cpuidle_state *prev_state)
> +{
> +	if (state->exit_latency == 0)
> +		/* Exit latency 0 can only be used for the POLL state */
> +		return -EINVAL;
> +
> +	if (state->exit_latency > MAX_CMDLINE_LATENCY_US)
> +		return -EINVAL;
> +
> +	if (state->target_residency > MAX_CMDLINE_RESIDENCY_US)
> +		return -EINVAL;
> +
> +	if (state->target_residency < state->exit_latency)
> +		return -EINVAL;
> +
> +	if (!prev_state)
> +		return 0;
> +
> +	if (state->exit_latency <= prev_state->exit_latency)
> +		return -EINVAL;
> +
> +	if (state->target_residency <= prev_state->target_residency)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +/**
>   * 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
> @@ -1697,6 +1737,21 @@ static void __init cmdline_table_adjust(
>  		       state->name, state->exit_latency, state->target_residency);
>  	}
>  
> +	/* Validate the adjusted C-states */
> +	for (i = 0; i < state_count; i++) {
> +		struct cpuidle_state *prev_state;

const?  FWIW, you could also init the field at definition, but I
understand this might diverge too much from the Linux code.

Thanks, Roger.

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

[Jan Beulich]

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@linux.intel.com>
>>
>> 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@linux.intel.com>
>> Link: https://patch.msgid.link/20251216080402.156988-3-dedekind1@gmail.com
>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>> 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@suse.com>
> 
> Acked-by: Roger Pau Monné <roger.pau@citrix.com>

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

Re: [PATCH for-4.22 v2 8/8] x86/mwait-idle: Add C-states validation

[Jan Beulich]

On 14.05.2026 17:30, Roger Pau Monné wrote:
> On Tue, May 12, 2026 at 05:38:32PM +0200, Jan Beulich wrote:
>> From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
>>
>> Add validation for C-states specified via the "table=" module parameter.
>> Treat this module parameter as untrusted input and validate it thoroughly.
>>
>> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
>> Link: https://patch.msgid.link/20251216080402.156988-4-dedekind1@gmail.com
>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>> Origin: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git be6a150829b3
>>
>> Add __init to validate_cmdline_cstate(). Other adjustments to fit our env.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Acked-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks.

>> @@ -1697,6 +1737,21 @@ static void __init cmdline_table_adjust(
>>  		       state->name, state->exit_latency, state->target_residency);
>>  	}
>>  
>> +	/* Validate the adjusted C-states */
>> +	for (i = 0; i < state_count; i++) {
>> +		struct cpuidle_state *prev_state;
> 
> const?

Oh, yes, I should have done that with the other v2 change.

>  FWIW, you could also init the field at definition, but I
> understand this might diverge too much from the Linux code.

Right, hence why I left it. Generally I agree that using initializers is
preferable over separate assignments.

Jan

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

[Roger Pau Monné]

On Fri, May 15, 2026 at 08:57:45AM +0200, Jan Beulich wrote:
> 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@linux.intel.com>
> >>
> >> 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@linux.intel.com>
> >> Link: https://patch.msgid.link/20251216080402.156988-3-dedekind1@gmail.com
> >> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >> 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@suse.com>
> > 
> > Acked-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> 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.

It seems like the outlier to me.  All other printk() instances in the
function use an explicit prefix, so I would think it might be best to
also add an explicit prefix here.

Thanks, Roger.