2022-11-17 07:37:52

by Yuan, Perry

[permalink] [raw]
Subject: [PATCH v2 0/5] AMD Pstate driver Urgent Change

The patchset changed amd-pstate driver as built-in type to resolve the
driver loading sequence problem, otherwise the acpi-cpufreq driver will
be loaded as the default cpufreq scaling driver instead of amd-pstate.

Some new kernel parameters are added to allow user to disable pstate driver
and load driver with passive mode which use governors to do the
frequency scaling control.

* `amd_pstate=disabled` or no parameters will not load pstate driver.
* `amd_pstate=passive` will load pstate driver with passive mode.

Set the `amd_pstate` driver disabled by default because of performance
degradation on a number of AMD ASICs in the passive mode driver,
especially the shared memory support processors.

EPP support for the amd_pstate driver is under review. With EPP support,
the said performance issue is resolved. Once that gets upstream,
the `active` mode amd_pstate_epp driver may be enabled by default.

Changes from v1:
* fix commit info in v1 patch PATCH 4/5
* pick up Reviewed-by flag of Gautham R. Shenoy
* pick up Tested-by flag of Wyes Karny

Perry Yuan (4):
cpufreq: amd-pstate: change amd-pstate driver to be built-in type
cpufreq: amd-pstate: add amd-pstate driver parameter for mode
selection
Documentation: amd-pstate: add driver working mode introduction
Documentation: add amd-pstate kernel command line options

Wyes Karny (1):
cpufreq: amd-pstate: cpufreq: amd-pstate: reset MSR_AMD_PERF_CTL
register at init

.../admin-guide/kernel-parameters.txt | 11 +++++
Documentation/admin-guide/pm/amd-pstate.rst | 30 +++++-------
drivers/cpufreq/Kconfig.x86 | 2 +-
drivers/cpufreq/amd-pstate.c | 49 +++++++++++++------
4 files changed, 59 insertions(+), 33 deletions(-)

--
2.25.1



2022-11-17 07:38:09

by Yuan, Perry

[permalink] [raw]
Subject: [PATCH v2 5/5] Documentation: add amd-pstate kernel command line options

Add a new amd pstate driver command line option to enable driver passive
working mode via MSR and shared memory interface to request desired
performance within abstract scale and the power management firmware
(SMU) convert the perf requests into actual hardware pstates.

Also the `disable` parameter can disable the pstate driver loading by
adding `amd_pstate=disable` to kernel command line.

Reviewed-by: Gautham R. Shenoy <[email protected]>
Tested-by: Wyes Karny <[email protected]>
Signed-off-by: Perry Yuan <[email protected]>
---
Documentation/admin-guide/kernel-parameters.txt | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index a465d5242774..42af9ca0127e 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -6959,3 +6959,14 @@
memory, and other data can't be written using
xmon commands.
off xmon is disabled.
+
+ amd_pstate= [X86]
+ disable
+ Do not enable amd_pstate as the default
+ scaling driver for the supported processors
+ passive
+ Use amd_pstate as a scaling driver, driver requests a
+ desired performance on this abstract scale and the power
+ management firmware translates the requests into actual
+ hardware states (core frequency, data fabric and memory
+ clocks etc.)
--
2.25.1


2022-11-17 07:39:07

by Yuan, Perry

[permalink] [raw]
Subject: [PATCH v2 1/5] cpufreq: amd-pstate: cpufreq: amd-pstate: reset MSR_AMD_PERF_CTL register at init

From: Wyes Karny <[email protected]>

MSR_AMD_PERF_CTL is guaranteed to be 0 on a cold boot. However, on a
kexec boot, for instance, it may have a non-zero value (if the cpu was
in a non-P0 Pstate). In such cases, the cores with non-P0 Pstates at
boot will never be pushed to P0, let alone boost frequencies.

Kexec is a common workflow for reboot on Linux and this creates a
regression in performance. Fix it by explicitly setting the
MSR_AMD_PERF_CTL to 0 during amd_pstate driver init.

Cc: [email protected]
Reviewed-by: Gautham R. Shenoy <[email protected]>
Tested-by: Wyes Karny <[email protected]>
Signed-off-by: Wyes Karny <[email protected]>
Signed-off-by: Perry Yuan <[email protected]>
---
drivers/cpufreq/amd-pstate.c | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index ace7d50cf2ac..d844c6f97caf 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -424,12 +424,22 @@ static void amd_pstate_boost_init(struct amd_cpudata *cpudata)
amd_pstate_driver.boost_enabled = true;
}

+static void amd_perf_ctl_reset(unsigned int cpu)
+{
+ wrmsrl_on_cpu(cpu, MSR_AMD_PERF_CTL, 0);
+}
+
static int amd_pstate_cpu_init(struct cpufreq_policy *policy)
{
int min_freq, max_freq, nominal_freq, lowest_nonlinear_freq, ret;
struct device *dev;
struct amd_cpudata *cpudata;

+ /*
+ * Resetting PERF_CTL_MSR will put the CPU in P0 frequency,
+ * which is ideal for initialization process.
+ */
+ amd_perf_ctl_reset(policy->cpu);
dev = get_cpu_device(policy->cpu);
if (!dev)
return -ENODEV;
--
2.25.1


2022-11-17 07:59:20

by Yuan, Perry

[permalink] [raw]
Subject: [PATCH v2 3/5] cpufreq: amd-pstate: add amd-pstate driver parameter for mode selection

When the amd_pstate driver is built-in users still need a method to be
able enable or disable it depending upon their circumstance.
Add support for an early parameter to do this.

There is some performance degradation on a number of ASICs in the
passive mode. This performance issue was originally discovered in
shared memory systems but it has been proven that certain workloads
on MSR systems also suffer performance issues.
Set the amd-pstate driver as disabled by default to temporarily
mitigate the performance problem.

1) with `amd_pstate=disable`, pstate driver will be disabled to load at
kernel booting.

2) with `amd_pstate=passive`, pstate driver will be enabled and loaded as
non-autonomous working mode supported in the low-level power management
firmware.

3) If neither parameter is specified, the driver will be disabled by
default to avoid triggering performance regressions in certain ASICs

Reviewed-by: Gautham R. Shenoy <[email protected]>
Tested-by: Wyes Karny <[email protected]>
Signed-off-by: Perry Yuan <[email protected]>
---
drivers/cpufreq/amd-pstate.c | 36 +++++++++++++++++++++++++++---------
1 file changed, 27 insertions(+), 9 deletions(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 701f49d6d240..204e39006dda 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -59,12 +59,8 @@
* we disable it by default to go acpi-cpufreq on these processors and add a
* module parameter to be able to enable it manually for debugging.
*/
-static bool shared_mem = false;
-module_param(shared_mem, bool, 0444);
-MODULE_PARM_DESC(shared_mem,
- "enable amd-pstate on processors with shared memory solution (false = disabled (default), true = enabled)");
-
static struct cpufreq_driver amd_pstate_driver;
+static int cppc_load __initdata;

static inline int pstate_enable(bool enable)
{
@@ -626,6 +622,15 @@ static int __init amd_pstate_init(void)

if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
return -ENODEV;
+ /*
+ * by default the pstate driver is disabled to load
+ * enable the amd_pstate passive mode driver explicitly
+ * with amd_pstate=passive in kernel command line
+ */
+ if (!cppc_load) {
+ pr_debug("driver load is disabled, boot with amd_pstate=passive to enable this\n");
+ return -ENODEV;
+ }

if (!acpi_cpc_valid()) {
pr_warn_once("the _CPC object is not present in SBIOS or ACPI disabled\n");
@@ -640,13 +645,11 @@ static int __init amd_pstate_init(void)
if (boot_cpu_has(X86_FEATURE_CPPC)) {
pr_debug("AMD CPPC MSR based functionality is supported\n");
amd_pstate_driver.adjust_perf = amd_pstate_adjust_perf;
- } else if (shared_mem) {
+ } else {
+ pr_debug("AMD CPPC shared memory based functionality is supported\n");
static_call_update(amd_pstate_enable, cppc_enable);
static_call_update(amd_pstate_init_perf, cppc_init_perf);
static_call_update(amd_pstate_update_perf, cppc_update_perf);
- } else {
- pr_info("This processor supports shared memory solution, you can enable it with amd_pstate.shared_mem=1\n");
- return -ENODEV;
}

/* enable amd pstate feature */
@@ -665,6 +668,21 @@ static int __init amd_pstate_init(void)
}
device_initcall(amd_pstate_init);

+static int __init amd_pstate_param(char *str)
+{
+ if (!str)
+ return -EINVAL;
+
+ if (!strcmp(str, "disable")) {
+ cppc_load = 0;
+ pr_info("driver is explicitly disabled\n");
+ } else if (!strcmp(str, "passive"))
+ cppc_load = 1;
+
+ return 0;
+}
+early_param("amd_pstate", amd_pstate_param);
+
MODULE_AUTHOR("Huang Rui <[email protected]>");
MODULE_DESCRIPTION("AMD Processor P-state Frequency Driver");
MODULE_LICENSE("GPL");
--
2.25.1


2022-11-17 08:25:46

by Yuan, Perry

[permalink] [raw]
Subject: [PATCH v2 2/5] cpufreq: amd-pstate: change amd-pstate driver to be built-in type

Currently when the amd-pstate and acpi_cpufreq are both built into
kernel as module driver, amd-pstate will not be loaded by default
in this case.

Change amd-pstate driver as built-in type, it will resolve the loading
sequence problem to allow user to make amd-pstate driver as the default
cpufreq scaling driver.

Fixes: ec437d71db77 ("cpufreq: amd-pstate: Introduce a new AMD P-State driver to support future processors")
Reviewed-by: Gautham R. Shenoy <[email protected]>
Tested-by: Wyes Karny <[email protected]>
Signed-off-by: Perry Yuan <[email protected]>
---
drivers/cpufreq/Kconfig.x86 | 2 +-
drivers/cpufreq/amd-pstate.c | 11 +----------
2 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/cpufreq/Kconfig.x86 b/drivers/cpufreq/Kconfig.x86
index 310779b07daf..00476e94db90 100644
--- a/drivers/cpufreq/Kconfig.x86
+++ b/drivers/cpufreq/Kconfig.x86
@@ -35,7 +35,7 @@ config X86_PCC_CPUFREQ
If in doubt, say N.

config X86_AMD_PSTATE
- tristate "AMD Processor P-State driver"
+ bool "AMD Processor P-State driver"
depends on X86 && ACPI
select ACPI_PROCESSOR
select ACPI_CPPC_LIB if X86_64
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index d844c6f97caf..701f49d6d240 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -663,16 +663,7 @@ static int __init amd_pstate_init(void)

return ret;
}
-
-static void __exit amd_pstate_exit(void)
-{
- cpufreq_unregister_driver(&amd_pstate_driver);
-
- amd_pstate_enable(false);
-}
-
-module_init(amd_pstate_init);
-module_exit(amd_pstate_exit);
+device_initcall(amd_pstate_init);

MODULE_AUTHOR("Huang Rui <[email protected]>");
MODULE_DESCRIPTION("AMD Processor P-state Frequency Driver");
--
2.25.1