2020-01-30 14:48:58

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH 0/2] intel_idle: Two new module parameters

Hi All,

This series adds to module parameters to intel_idle (on top of the material
already in the mainline since Monday), one to make it use ACPI even if that
is not enabled for the processor model present in the system and the other
one to allow passing a list of idle states to disable by default to it.

Details in the patch changelogs.

Thanks,
Rafael




2020-01-30 14:49:07

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH 1/2] intel_idle: Introduce 'use_acpi' module parameter

From: Rafael J. Wysocki <[email protected]>

For diagnostics, it is generally useful to be able to make intel_idle
take the system's ACPI tables into consideration even if that is not
required for the processor model in there, so introduce a new module
parameter, 'use_acpi', to make that happen and update the documentation
to cover it.

While at it, fix the 'no_acpi' module parameter name in the
documentation.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
Documentation/admin-guide/pm/intel_idle.rst | 13 +++++++++----
drivers/idle/intel_idle.c | 11 +++++++++--
2 files changed, 18 insertions(+), 6 deletions(-)

Index: linux-pm/drivers/idle/intel_idle.c
===================================================================
--- linux-pm.orig/drivers/idle/intel_idle.c
+++ linux-pm/drivers/idle/intel_idle.c
@@ -1131,6 +1131,10 @@ static bool no_acpi __read_mostly;
module_param(no_acpi, bool, 0444);
MODULE_PARM_DESC(no_acpi, "Do not use ACPI _CST for building the idle states list");

+static bool use_acpi __read_mostly; /* No effect if no_acpi is set. */
+module_param(use_acpi, bool, 0444);
+MODULE_PARM_DESC(use_acpi, "Use ACPI _CST for building the idle states list");
+
static struct acpi_processor_power acpi_state_table __initdata;

/**
@@ -1258,6 +1262,8 @@ static bool __init intel_idle_off_by_def
return true;
}
#else /* !CONFIG_ACPI_PROCESSOR_CSTATE */
+#define use_acpi (false)
+
static inline bool intel_idle_acpi_cst_extract(void) { return false; }
static inline void intel_idle_init_cstates_acpi(struct cpuidle_driver *drv) { }
static inline bool intel_idle_off_by_default(u32 mwait_hint) { return false; }
@@ -1460,7 +1466,8 @@ static void __init intel_idle_init_cstat
/* Structure copy. */
drv->states[drv->state_count] = cpuidle_state_table[cstate];

- if (icpu->use_acpi && intel_idle_off_by_default(mwait_hint) &&
+ if ((icpu->use_acpi || use_acpi) &&
+ intel_idle_off_by_default(mwait_hint) &&
!(cpuidle_state_table[cstate].flags & CPUIDLE_FLAG_ALWAYS_ENABLE))
drv->states[drv->state_count].flags |= CPUIDLE_FLAG_OFF;

@@ -1607,7 +1614,7 @@ static int __init intel_idle_init(void)
icpu = (const struct idle_cpu *)id->driver_data;
if (icpu) {
cpuidle_state_table = icpu->state_table;
- if (icpu->use_acpi)
+ if (icpu->use_acpi || use_acpi)
intel_idle_acpi_cst_extract();
} else if (!intel_idle_acpi_cst_extract()) {
return -ENODEV;
Index: linux-pm/Documentation/admin-guide/pm/intel_idle.rst
===================================================================
--- linux-pm.orig/Documentation/admin-guide/pm/intel_idle.rst
+++ linux-pm/Documentation/admin-guide/pm/intel_idle.rst
@@ -60,6 +60,9 @@ of the system. The former are always us
recognized by ``intel_idle`` and the latter are used if that is required for
the given processor model (which is the case for all server processor models
recognized by ``intel_idle``) or if the processor model is not recognized.
+[There is a module parameter that can be used to make the driver use the ACPI
+tables with any processor model recognized by it; see
+`below <intel-idle-parameters_>`_.]

If the ACPI tables are going to be used for building the list of available idle
states, ``intel_idle`` first looks for a ``_CST`` object under one of the ACPI
@@ -165,7 +168,7 @@ and ``idle=nomwait``. If any of them is
``MWAIT`` instruction is not allowed to be used, so the initialization of
``intel_idle`` will fail.

-Apart from that there are two module parameters recognized by ``intel_idle``
+Apart from that there are three module parameters recognized by ``intel_idle``
itself that can be set via the kernel command line (they cannot be updated via
sysfs, so that is the only way to change their values).

@@ -186,9 +189,11 @@ QoS) feature can be used to prevent ``CP
even if they have been enumerated (see :ref:`cpu-pm-qos` in :doc:`cpuidle`).
Setting ``max_cstate`` to 0 causes the ``intel_idle`` initialization to fail.

-The ``noacpi`` module parameter (which is 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 (it is unset by default).
+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).


.. _intel-idle-core-and-package-idle-states:



2020-02-02 14:25:32

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 1/2] intel_idle: Introduce 'use_acpi' module parameter

Hi "Rafael,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on next-20200130]
[cannot apply to acpi/next linux/master v5.5]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url: https://github.com/0day-ci/linux/commits/Rafael-J-Wysocki/intel_idle-Two-new-module-parameters/20200202-161814
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 94f2630b18975bb56eee5d1a36371db967643479
config: i386-randconfig-a001-20200202 (attached as .config)
compiler: gcc-6 (Debian 6.3.0-18+deb9u1) 6.3.0 20170516
reproduce:
# save the attached .config to linux build tree
make ARCH=i386

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>

All error/warnings (new ones prefixed by >>):

drivers//idle/intel_idle.c: In function 'intel_idle_init_cstates_icpu':
>> drivers//idle/intel_idle.c:1265:18: error: expected identifier before '(' token
#define use_acpi (false)
^
>> drivers//idle/intel_idle.c:1469:14: note: in expansion of macro 'use_acpi'
if ((icpu->use_acpi || use_acpi) &&
^~~~~~~~
drivers//idle/intel_idle.c: In function 'intel_idle_init':
>> drivers//idle/intel_idle.c:1265:18: error: expected identifier before '(' token
#define use_acpi (false)
^
drivers//idle/intel_idle.c:1617:13: note: in expansion of macro 'use_acpi'
if (icpu->use_acpi || use_acpi)
^~~~~~~~

vim +/use_acpi +1469 drivers//idle/intel_idle.c

1427
1428 static void __init intel_idle_init_cstates_icpu(struct cpuidle_driver *drv)
1429 {
1430 int cstate;
1431
1432 switch (boot_cpu_data.x86_model) {
1433 case INTEL_FAM6_IVYBRIDGE_X:
1434 ivt_idle_state_table_update();
1435 break;
1436 case INTEL_FAM6_ATOM_GOLDMONT:
1437 case INTEL_FAM6_ATOM_GOLDMONT_PLUS:
1438 bxt_idle_state_table_update();
1439 break;
1440 case INTEL_FAM6_SKYLAKE:
1441 sklh_idle_state_table_update();
1442 break;
1443 }
1444
1445 for (cstate = 0; cstate < CPUIDLE_STATE_MAX; ++cstate) {
1446 unsigned int mwait_hint;
1447
1448 if (intel_idle_max_cstate_reached(cstate))
1449 break;
1450
1451 if (!cpuidle_state_table[cstate].enter &&
1452 !cpuidle_state_table[cstate].enter_s2idle)
1453 break;
1454
1455 /* If marked as unusable, skip this state. */
1456 if (cpuidle_state_table[cstate].flags & CPUIDLE_FLAG_UNUSABLE) {
1457 pr_debug("state %s is disabled\n",
1458 cpuidle_state_table[cstate].name);
1459 continue;
1460 }
1461
1462 mwait_hint = flg2MWAIT(cpuidle_state_table[cstate].flags);
1463 if (!intel_idle_verify_cstate(mwait_hint))
1464 continue;
1465
1466 /* Structure copy. */
1467 drv->states[drv->state_count] = cpuidle_state_table[cstate];
1468
> 1469 if ((icpu->use_acpi || use_acpi) &&
1470 intel_idle_off_by_default(mwait_hint) &&
1471 !(cpuidle_state_table[cstate].flags & CPUIDLE_FLAG_ALWAYS_ENABLE))
1472 drv->states[drv->state_count].flags |= CPUIDLE_FLAG_OFF;
1473
1474 drv->state_count++;
1475 }
1476
1477 if (icpu->byt_auto_demotion_disable_flag) {
1478 wrmsrl(MSR_CC6_DEMOTION_POLICY_CONFIG, 0);
1479 wrmsrl(MSR_MC6_DEMOTION_POLICY_CONFIG, 0);
1480 }
1481 }
1482

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/[email protected] Intel Corporation


Attachments:
(No filename) (3.76 kB)
.config.gz (38.56 kB)
Download all attachments

2020-02-03 12:24:20

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH v2 0/2] intel_idle: Two new module parameters

On Thursday, January 30, 2020 3:44:40 PM CET Rafael J. Wysocki wrote:
> Hi All,
>
> This series adds to module parameters to intel_idle (on top of the material
> already in the mainline since Monday), one to make it use ACPI even if that
> is not enabled for the processor model present in the system and the other
> one to allow passing a list of idle states to disable by default to it.
>
> Details in the patch changelogs.

This is a small update of the original set fixing a build issue in the first
patch and addressing a couple of review comments in the second one.

Thanks,
Rafael



2020-02-03 12:24:20

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH v2 1/2] intel_idle: Introduce 'use_acpi' module parameter

From: Rafael J. Wysocki <[email protected]>

For diagnostics, it is generally useful to be able to make intel_idle
take the system's ACPI tables into consideration even if that is not
required for the processor model in there, so introduce a new module
parameter, 'use_acpi', to make that happen and update the documentation
to cover it.

While at it, fix the 'no_acpi' module parameter name in the
documentation.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---

-> v2:
* Fix build for CONFIG_ACPI unset.

---
Documentation/admin-guide/pm/intel_idle.rst | 13 +++++++++----
drivers/idle/intel_idle.c | 11 +++++++++--
2 files changed, 18 insertions(+), 6 deletions(-)

Index: linux-pm/drivers/idle/intel_idle.c
===================================================================
--- linux-pm.orig/drivers/idle/intel_idle.c
+++ linux-pm/drivers/idle/intel_idle.c
@@ -1131,6 +1131,10 @@ static bool no_acpi __read_mostly;
module_param(no_acpi, bool, 0444);
MODULE_PARM_DESC(no_acpi, "Do not use ACPI _CST for building the idle states list");

+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 struct acpi_processor_power acpi_state_table __initdata;

/**
@@ -1258,6 +1262,8 @@ static bool __init intel_idle_off_by_def
return true;
}
#else /* !CONFIG_ACPI_PROCESSOR_CSTATE */
+#define force_use_acpi (false)
+
static inline bool intel_idle_acpi_cst_extract(void) { return false; }
static inline void intel_idle_init_cstates_acpi(struct cpuidle_driver *drv) { }
static inline bool intel_idle_off_by_default(u32 mwait_hint) { return false; }
@@ -1460,7 +1466,8 @@ static void __init intel_idle_init_cstat
/* Structure copy. */
drv->states[drv->state_count] = cpuidle_state_table[cstate];

- if (icpu->use_acpi && intel_idle_off_by_default(mwait_hint) &&
+ if ((icpu->use_acpi || force_use_acpi) &&
+ intel_idle_off_by_default(mwait_hint) &&
!(cpuidle_state_table[cstate].flags & CPUIDLE_FLAG_ALWAYS_ENABLE))
drv->states[drv->state_count].flags |= CPUIDLE_FLAG_OFF;

@@ -1607,7 +1614,7 @@ static int __init intel_idle_init(void)
icpu = (const struct idle_cpu *)id->driver_data;
if (icpu) {
cpuidle_state_table = icpu->state_table;
- if (icpu->use_acpi)
+ if (icpu->use_acpi || force_use_acpi)
intel_idle_acpi_cst_extract();
} else if (!intel_idle_acpi_cst_extract()) {
return -ENODEV;
Index: linux-pm/Documentation/admin-guide/pm/intel_idle.rst
===================================================================
--- linux-pm.orig/Documentation/admin-guide/pm/intel_idle.rst
+++ linux-pm/Documentation/admin-guide/pm/intel_idle.rst
@@ -60,6 +60,9 @@ of the system. The former are always us
recognized by ``intel_idle`` and the latter are used if that is required for
the given processor model (which is the case for all server processor models
recognized by ``intel_idle``) or if the processor model is not recognized.
+[There is a module parameter that can be used to make the driver use the ACPI
+tables with any processor model recognized by it; see
+`below <intel-idle-parameters_>`_.]

If the ACPI tables are going to be used for building the list of available idle
states, ``intel_idle`` first looks for a ``_CST`` object under one of the ACPI
@@ -165,7 +168,7 @@ and ``idle=nomwait``. If any of them is
``MWAIT`` instruction is not allowed to be used, so the initialization of
``intel_idle`` will fail.

-Apart from that there are two module parameters recognized by ``intel_idle``
+Apart from that there are three module parameters recognized by ``intel_idle``
itself that can be set via the kernel command line (they cannot be updated via
sysfs, so that is the only way to change their values).

@@ -186,9 +189,11 @@ QoS) feature can be used to prevent ``CP
even if they have been enumerated (see :ref:`cpu-pm-qos` in :doc:`cpuidle`).
Setting ``max_cstate`` to 0 causes the ``intel_idle`` initialization to fail.

-The ``noacpi`` module parameter (which is 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 (it is unset by default).
+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).


.. _intel-idle-core-and-package-idle-states: