| Message ID | 20250211132741.99944-1-darcari@redhat.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [v3] intel_idle: introduce 'no_native' module parameter | expand |
On Tue, 2025-02-11 at 08:27 -0500, David Arcari wrote: > Since commit 18734958e9bf ("intel_idle: Use ACPI _CST for processor models > without C-state tables") the intel_idle driver has had the ability to use > the ACPI _CST to populate C-states when the processor model is not > recognized. However, even when the processor model is recognized (native > mode) there are cases where it is useful to make the driver ignore the per > cpu idle states in lieu of ACPI C-states (such as specific application > performance). Add the 'no_native' module parameter to provide this > functionality. > > Cc: Jonathan Corbet <corbet@lwn.net> > Cc: Jacob Pan <jacob.jun.pan@linux.intel.com> > Cc: Len Brown <lenb@kernel.org> > Cc: David Arcari <darcari@redhat.com> > Cc: Artem Bityutskiy <dedekind1@gmail.com> > Cc: Prarit Bhargava <prarit@redhat.com> > Cc: linux-doc@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Signed-off-by: David Arcari <darcari@redhat.com> > --- > v3: more documentation cleanup > v2: renamed parameter, cleaned up documentation Reviewed-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com> Tested-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Hi David, kernel test robot noticed the following build errors: [auto build test ERROR on acpi/next] [also build test ERROR on amd-pstate/linux-next amd-pstate/bleeding-edge linus/master v6.14-rc2 next-20250212] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/David-Arcari/intel_idle-introduce-no_native-module-parameter/20250211-213031 base: https://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux.git next patch link: https://lore.kernel.org/r/20250211132741.99944-1-darcari%40redhat.com patch subject: [PATCH v3] intel_idle: introduce 'no_native' module parameter config: i386-buildonly-randconfig-006-20250212 (https://download.01.org/0day-ci/archive/20250212/202502121732.P7lZkbhm-lkp@intel.com/config) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250212/202502121732.P7lZkbhm-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202502121732.P7lZkbhm-lkp@intel.com/ All errors (new ones prefixed by >>): drivers/idle/intel_idle.c: In function 'intel_idle_init': >> drivers/idle/intel_idle.c:2289:27: error: 'no_acpi' undeclared (first use in this function); did you mean 'no_action'? 2289 | if (no_native && !no_acpi) { | ^~~~~~~ | no_action drivers/idle/intel_idle.c:2289:27: note: each undeclared identifier is reported only once for each function it appears in vim +2289 drivers/idle/intel_idle.c 2248 2249 static int __init intel_idle_init(void) 2250 { 2251 const struct x86_cpu_id *id; 2252 unsigned int eax, ebx, ecx; 2253 int retval; 2254 2255 /* Do not load intel_idle at all for now if idle= is passed */ 2256 if (boot_option_idle_override != IDLE_NO_OVERRIDE) 2257 return -ENODEV; 2258 2259 if (max_cstate == 0) { 2260 pr_debug("disabled\n"); 2261 return -EPERM; 2262 } 2263 2264 id = x86_match_cpu(intel_idle_ids); 2265 if (id) { 2266 if (!boot_cpu_has(X86_FEATURE_MWAIT)) { 2267 pr_debug("Please enable MWAIT in BIOS SETUP\n"); 2268 return -ENODEV; 2269 } 2270 } else { 2271 id = x86_match_cpu(intel_mwait_ids); 2272 if (!id) 2273 return -ENODEV; 2274 } 2275 2276 if (boot_cpu_data.cpuid_level < CPUID_MWAIT_LEAF) 2277 return -ENODEV; 2278 2279 cpuid(CPUID_MWAIT_LEAF, &eax, &ebx, &ecx, &mwait_substates); 2280 2281 if (!(ecx & CPUID5_ECX_EXTENSIONS_SUPPORTED) || 2282 !(ecx & CPUID5_ECX_INTERRUPT_BREAK) || 2283 !mwait_substates) 2284 return -ENODEV; 2285 2286 pr_debug("MWAIT substates: 0x%x\n", mwait_substates); 2287 2288 icpu = (const struct idle_cpu *)id->driver_data; > 2289 if (no_native && !no_acpi) { 2290 if (icpu) { 2291 pr_debug("ignoring native cpu idle states\n"); 2292 icpu = NULL; 2293 } 2294 } 2295 if (icpu) { 2296 if (icpu->state_table) 2297 cpuidle_state_table = icpu->state_table; 2298 else if (!intel_idle_acpi_cst_extract()) 2299 return -ENODEV; 2300 2301 auto_demotion_disable_flags = icpu->auto_demotion_disable_flags; 2302 if (icpu->disable_promotion_to_c1e) 2303 c1e_promotion = C1E_PROMOTION_DISABLE; 2304 if (icpu->use_acpi || force_use_acpi) 2305 intel_idle_acpi_cst_extract(); 2306 } else if (!intel_idle_acpi_cst_extract()) { 2307 return -ENODEV; 2308 } 2309 2310 pr_debug("v" INTEL_IDLE_VERSION " model 0x%X\n", 2311 boot_cpu_data.x86_model); 2312 2313 intel_idle_cpuidle_devices = alloc_percpu(struct cpuidle_device); 2314 if (!intel_idle_cpuidle_devices) 2315 return -ENOMEM; 2316 2317 intel_idle_cpuidle_driver_init(&intel_idle_driver); 2318 2319 retval = cpuidle_register_driver(&intel_idle_driver); 2320 if (retval) { 2321 struct cpuidle_driver *drv = cpuidle_get_driver(); 2322 printk(KERN_DEBUG pr_fmt("intel_idle yielding to %s\n"), 2323 drv ? drv->name : "none"); 2324 goto init_driver_fail; 2325 } 2326 2327 retval = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "idle/intel:online", 2328 intel_idle_cpu_online, NULL); 2329 if (retval < 0) 2330 goto hp_setup_fail; 2331 2332 pr_debug("Local APIC timer is reliable in %s\n", 2333 boot_cpu_has(X86_FEATURE_ARAT) ? "all C-states" : "C1"); 2334 2335 return 0; 2336 2337 hp_setup_fail: 2338 intel_idle_cpuidle_devices_uninit(); 2339 cpuidle_unregister_driver(&intel_idle_driver); 2340 init_driver_fail: 2341 free_percpu(intel_idle_cpuidle_devices); 2342 return retval; 2343
On Wed, 2025-02-12 at 18:09 +0800, kernel test robot wrote: > drivers/idle/intel_idle.c: In function 'intel_idle_init': > > > drivers/idle/intel_idle.c:2289:27: error: 'no_acpi' undeclared (first use > > > in this function); did you mean 'no_action'? > 2289 | if (no_native && !no_acpi) { > | ^~~~~~~ > | no_action > drivers/idle/intel_idle.c:2289:27: note: each undeclared identifier is > reported only once for each function it appears in David, this must be the !CONFIG_ACPI_PROCESSOR_CSTATE case. Thanks!
Hi Artem, On 2/12/25 6:32 AM, Artem Bityutskiy wrote: > On Wed, 2025-02-12 at 18:09 +0800, kernel test robot wrote: >> drivers/idle/intel_idle.c: In function 'intel_idle_init': >>>> drivers/idle/intel_idle.c:2289:27: error: 'no_acpi' undeclared (first use >>>> in this function); did you mean 'no_action'? >> 2289 | if (no_native && !no_acpi) { >> | ^~~~~~~ >> | no_action >> drivers/idle/intel_idle.c:2289:27: note: each undeclared identifier is >> reported only once for each function it appears in > > David, this must be the !CONFIG_ACPI_PROCESSOR_CSTATE case. > > Thanks! Oh - I see the problem. After a quick look I see two options: - #ifdef the code that doesn't compile - default no_acpi=true in the !CONFIG_ACPI_PROCESSOR_CSTATE case I sort of like the second option better, but I worry about the documentation. Specifically: "In the case that ACPI is not configured these flags have no impact +on functionality." I guess that is still true. Perhaps there is a better option. What do you think? Thanks, -DA
On Wed, 2025-02-12 at 07:41 -0500, David Arcari wrote: > - #ifdef the code that doesn't compile > - default no_acpi=true in the !CONFIG_ACPI_PROCESSOR_CSTATE case > > I sort of like the second option better, but I worry about the > documentation. Specifically: > > "In the case that ACPI is not configured these flags have no impact > +on functionality." > > I guess that is still true. > > Perhaps there is a better option. What do you think? I've not been involved into kernel that much for long time. In old days sprinkling #ifdefs around was an anti-pattern. Most probably nowadays too. So the second option sounds better to me. Artem.
On Wed, 2025-02-12 at 07:41 -0500, David Arcari wrote: > - #ifdef the code that doesn't compile > - default no_acpi=true in the !CONFIG_ACPI_PROCESSOR_CSTATE case > > I sort of like the second option better, but I worry about the > documentation. Specifically: The most important part is build !CONFIG_ACPI_PROCESSOR_CSTATE, run and test that it works. Thanks!
On 2/12/25 7:46 AM, Artem Bityutskiy wrote: > On Wed, 2025-02-12 at 07:41 -0500, David Arcari wrote: >> - #ifdef the code that doesn't compile >> - default no_acpi=true in the !CONFIG_ACPI_PROCESSOR_CSTATE case >> >> I sort of like the second option better, but I worry about the >> documentation. Specifically: >> >> "In the case that ACPI is not configured these flags have no impact >> +on functionality." >> >> I guess that is still true. >> >> Perhaps there is a better option. What do you think? > > I've not been involved into kernel that much for long time. In old days > sprinkling #ifdefs around was an anti-pattern. Most probably nowadays too. So > the second option sounds better to me. Another option would be to change the offending code to a function call: if (ignore_native()) { And have ignore_native() always return false when ACPI is not configured. And yes I should have built and tested the kernel with ACPI disabled. My apologies. I will do that for v4. -DA > > Artem. >
Hi David,
kernel test robot noticed the following build errors:
[auto build test ERROR on acpi/next]
[also build test ERROR on amd-pstate/linux-next amd-pstate/bleeding-edge linus/master v6.14-rc2 next-20250213]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/David-Arcari/intel_idle-introduce-no_native-module-parameter/20250211-213031
base: https://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux.git next
patch link: https://lore.kernel.org/r/20250211132741.99944-1-darcari%40redhat.com
patch subject: [PATCH v3] intel_idle: introduce 'no_native' module parameter
config: i386-buildonly-randconfig-002-20250213 (https://download.01.org/0day-ci/archive/20250213/202502131953.d3fHaDCE-lkp@intel.com/config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250213/202502131953.d3fHaDCE-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202502131953.d3fHaDCE-lkp@intel.com/
All error/warnings (new ones prefixed by >>):
In file included from drivers/idle/intel_idle.c:48:
In file included from include/trace/events/power.h:12:
In file included from include/linux/trace_events.h:6:
In file included from include/linux/ring_buffer.h:5:
In file included from include/linux/mm.h:2213:
include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
518 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
>> drivers/idle/intel_idle.c:2289:20: error: use of undeclared identifier 'no_acpi'; did you mean 'no_action'?
2289 | if (no_native && !no_acpi) {
| ^~~~~~~
| no_action
include/linux/interrupt.h:138:20: note: 'no_action' declared here
138 | extern irqreturn_t no_action(int cpl, void *dev_id);
| ^
>> drivers/idle/intel_idle.c:2289:20: warning: address of function 'no_action' will always evaluate to 'true' [-Wpointer-bool-conversion]
2289 | if (no_native && !no_acpi) {
| ~^~~~~~~
drivers/idle/intel_idle.c:2289:20: note: prefix with the address-of operator to silence this warning
2289 | if (no_native && !no_acpi) {
| ^
| &
2 warnings and 1 error generated.
vim +2289 drivers/idle/intel_idle.c
2248
2249 static int __init intel_idle_init(void)
2250 {
2251 const struct x86_cpu_id *id;
2252 unsigned int eax, ebx, ecx;
2253 int retval;
2254
2255 /* Do not load intel_idle at all for now if idle= is passed */
2256 if (boot_option_idle_override != IDLE_NO_OVERRIDE)
2257 return -ENODEV;
2258
2259 if (max_cstate == 0) {
2260 pr_debug("disabled\n");
2261 return -EPERM;
2262 }
2263
2264 id = x86_match_cpu(intel_idle_ids);
2265 if (id) {
2266 if (!boot_cpu_has(X86_FEATURE_MWAIT)) {
2267 pr_debug("Please enable MWAIT in BIOS SETUP\n");
2268 return -ENODEV;
2269 }
2270 } else {
2271 id = x86_match_cpu(intel_mwait_ids);
2272 if (!id)
2273 return -ENODEV;
2274 }
2275
2276 if (boot_cpu_data.cpuid_level < CPUID_MWAIT_LEAF)
2277 return -ENODEV;
2278
2279 cpuid(CPUID_MWAIT_LEAF, &eax, &ebx, &ecx, &mwait_substates);
2280
2281 if (!(ecx & CPUID5_ECX_EXTENSIONS_SUPPORTED) ||
2282 !(ecx & CPUID5_ECX_INTERRUPT_BREAK) ||
2283 !mwait_substates)
2284 return -ENODEV;
2285
2286 pr_debug("MWAIT substates: 0x%x\n", mwait_substates);
2287
2288 icpu = (const struct idle_cpu *)id->driver_data;
> 2289 if (no_native && !no_acpi) {
2290 if (icpu) {
2291 pr_debug("ignoring native cpu idle states\n");
2292 icpu = NULL;
2293 }
2294 }
2295 if (icpu) {
2296 if (icpu->state_table)
2297 cpuidle_state_table = icpu->state_table;
2298 else if (!intel_idle_acpi_cst_extract())
2299 return -ENODEV;
2300
2301 auto_demotion_disable_flags = icpu->auto_demotion_disable_flags;
2302 if (icpu->disable_promotion_to_c1e)
2303 c1e_promotion = C1E_PROMOTION_DISABLE;
2304 if (icpu->use_acpi || force_use_acpi)
2305 intel_idle_acpi_cst_extract();
2306 } else if (!intel_idle_acpi_cst_extract()) {
2307 return -ENODEV;
2308 }
2309
2310 pr_debug("v" INTEL_IDLE_VERSION " model 0x%X\n",
2311 boot_cpu_data.x86_model);
2312
2313 intel_idle_cpuidle_devices = alloc_percpu(struct cpuidle_device);
2314 if (!intel_idle_cpuidle_devices)
2315 return -ENOMEM;
2316
2317 intel_idle_cpuidle_driver_init(&intel_idle_driver);
2318
2319 retval = cpuidle_register_driver(&intel_idle_driver);
2320 if (retval) {
2321 struct cpuidle_driver *drv = cpuidle_get_driver();
2322 printk(KERN_DEBUG pr_fmt("intel_idle yielding to %s\n"),
2323 drv ? drv->name : "none");
2324 goto init_driver_fail;
2325 }
2326
2327 retval = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "idle/intel:online",
2328 intel_idle_cpu_online, NULL);
2329 if (retval < 0)
2330 goto hp_setup_fail;
2331
2332 pr_debug("Local APIC timer is reliable in %s\n",
2333 boot_cpu_has(X86_FEATURE_ARAT) ? "all C-states" : "C1");
2334
2335 return 0;
2336
2337 hp_setup_fail:
2338 intel_idle_cpuidle_devices_uninit();
2339 cpuidle_unregister_driver(&intel_idle_driver);
2340 init_driver_fail:
2341 free_percpu(intel_idle_cpuidle_devices);
2342 return retval;
2343
diff --git a/Documentation/admin-guide/pm/intel_idle.rst b/Documentation/admin-guide/pm/intel_idle.rst index 39bd6ecce7de..5940528146eb 100644 --- a/Documentation/admin-guide/pm/intel_idle.rst +++ b/Documentation/admin-guide/pm/intel_idle.rst @@ -192,11 +192,19 @@ even if they have been enumerated (see :ref:`cpu-pm-qos` in Documentation/admin-guide/pm/cpuidle.rst). Setting ``max_cstate`` to 0 causes the ``intel_idle`` initialization to fail. -The ``no_acpi`` and ``use_acpi`` module parameters (recognized by ``intel_idle`` -if the kernel has been configured with ACPI support) can be set to make the -driver ignore the system's ACPI tables entirely or use them for all of the -recognized processor models, respectively (they both are unset by default and -``use_acpi`` has no effect if ``no_acpi`` is set). +The ``no_acpi``, ``use_acpi`` and ``no_native`` module parameters are +recognized by ``intel_idle`` if the kernel has been configured with ACPI +support. In the case that ACPI is not configured these flags have no impact +on functionality. + +``no_acpi`` - Do not use ACPI at all. Only native mode is available, no +ACPI mode. + +``use_acpi`` - No-op in ACPI mode, the driver will consult ACPI tables for +C-states on/off status in native mode. + +``no_native`` - Work only in ACPI mode, no native mode available (ignore +all custom tables). The value of the ``states_off`` module parameter (0 by default) represents a list of idle states to be disabled by default in the form of a bitmask. diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c index 118fe1d37c22..5e5bd3fd3064 100644 --- a/drivers/idle/intel_idle.c +++ b/drivers/idle/intel_idle.c @@ -1695,6 +1695,10 @@ static bool force_use_acpi __read_mostly; /* No effect if no_acpi is set. */ module_param_named(use_acpi, force_use_acpi, bool, 0444); MODULE_PARM_DESC(use_acpi, "Use ACPI _CST for building the idle states list"); +static bool no_native __read_mostly; /* No effect if no_acpi is set. */ +module_param_named(no_native, no_native, bool, 0444); +MODULE_PARM_DESC(no_native, "Ignore cpu specific (native) idle states in lieu of ACPI idle states"); + static struct acpi_processor_power acpi_state_table __initdata; /** @@ -1836,6 +1840,7 @@ static bool __init intel_idle_off_by_default(unsigned int flags, u32 mwait_hint) } #else /* !CONFIG_ACPI_PROCESSOR_CSTATE */ #define force_use_acpi (false) +#define no_native (false) static inline bool intel_idle_acpi_cst_extract(void) { return false; } static inline void intel_idle_init_cstates_acpi(struct cpuidle_driver *drv) { } @@ -2328,6 +2333,12 @@ static int __init intel_idle_init(void) pr_debug("MWAIT substates: 0x%x\n", mwait_substates); icpu = (const struct idle_cpu *)id->driver_data; + if (no_native && !no_acpi) { + if (icpu) { + pr_debug("ignoring native cpu idle states\n"); + icpu = NULL; + } + } if (icpu) { if (icpu->state_table) cpuidle_state_table = icpu->state_table;
Since commit 18734958e9bf ("intel_idle: Use ACPI _CST for processor models without C-state tables") the intel_idle driver has had the ability to use the ACPI _CST to populate C-states when the processor model is not recognized. However, even when the processor model is recognized (native mode) there are cases where it is useful to make the driver ignore the per cpu idle states in lieu of ACPI C-states (such as specific application performance). Add the 'no_native' module parameter to provide this functionality. Cc: Jonathan Corbet <corbet@lwn.net> Cc: Jacob Pan <jacob.jun.pan@linux.intel.com> Cc: Len Brown <lenb@kernel.org> Cc: David Arcari <darcari@redhat.com> Cc: Artem Bityutskiy <dedekind1@gmail.com> Cc: Prarit Bhargava <prarit@redhat.com> Cc: linux-doc@vger.kernel.org Cc: linux-kernel@vger.kernel.org Signed-off-by: David Arcari <darcari@redhat.com> --- v3: more documentation cleanup v2: renamed parameter, cleaned up documentation Documentation/admin-guide/pm/intel_idle.rst | 18 +++++++++++++----- drivers/idle/intel_idle.c | 11 +++++++++++ 2 files changed, 24 insertions(+), 5 deletions(-)