Hi all,
This patchset implements one new AMD CPU frequency driver
"amd-pstate-epp” instance for better performance and power control.
CPPC has a parameter called energy preference performance (EPP).
The EPP is used in the CCLK DPM controller to drive the frequency that a core
is going to operate during short periods of activity.
EPP values will be utilized for different OS profiles (balanced, performance, power savings).
AMD Energy Performance Preference (EPP) provides a hint to the hardware
if software wants to bias toward performance (0x0) or energy efficiency (0xff)
The lowlevel power firmware will calculate the runtime frequency according to the EPP preference
value. So the EPP hint will impact the CPU cores frequency responsiveness.
We use the RAPL interface with "perf" tool to get the energy data of the package power.
Performance Per Watt (PPW) Calculation:
The PPW calculation is referred by below paper:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsoftware.intel.com%2Fcontent%2Fdam%2Fdevelop%2Fexternal%2Fus%2Fen%2Fdocuments%2Fperformance-per-what-paper.pdf&data=04%7C01%7CPerry.Yuan%40amd.com%7Cac66e8ce98044e9b062708d9ab47c8d8%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637729147708574423%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=TPOvCE%2Frbb0ptBreWNxHqOi9YnVhcHGKG88vviDLb00%3D&reserved=0
Below formula is referred from below spec to measure the PPW:
(F / t) / P = F * t / (t * E) = F / E,
"F" is the number of frames per second.
"P" is power measured in watts.
"E" is energy measured in joules.
Gitsouce Benchmark Data on ROME Server CPU
+------------------------------+------------------------------+------------+------------------+
| Kernel Module | PPW (1 / s * J) |Energy(J) | PPW Improvement (%)|
+==============================+==============================+============+==================+
| acpi-cpufreq:schedutil | 5.85658E-05 | 17074.8 | base |
+------------------------------+------------------------------+------------+------------------+
| acpi-cpufreq:ondemand | 5.03079E-05 | 19877.6 | -14.10% |
+------------------------------+------------------------------+------------+------------------+
| acpi-cpufreq:performance | 5.88132E-05 | 17003 | 0.42% |
+------------------------------+------------------------------+------------+------------------+
| amd-pstate:ondemand | 4.60295E-05 | 21725.2 | -21.41% |
+------------------------------+------------------------------+------------+------------------+
| amd-pstate:schedutil | 4.70026E-05 | 21275.4 | -19.7% |
+------------------------------+------------------------------+------------+------------------+
| amd-pstate:performance | 5.80094E-05 | 17238.6 | -0.95% |
+------------------------------+------------------------------+------------+------------------+
| EPP:performance | 5.8292E-05 | 17155 | -0.47% |
+------------------------------+------------------------------+------------+------------------+
| EPP: balance performance: | 6.71709E-05 | 14887.4 | 14.69% |
+------------------------------+------------------------------+------------+------------------+
| EPP:power | 6.66951E-05 | 4993.6 | 13.88% |
+------------------------------+------------------------------+------------+------------------+
Tbench Benchmark Data on ROME Server CPU
+---------------------------------------------+-------------------+--------------+-------------+------------------+
| Kernel Module | PPW MB / (s * J) |Throughput(MB/s)| Energy (J)|PPW Improvement(%)|
+=============================================+===================+==============+=============+==================+
| acpi_cpufreq: schedutil | 46.39 | 17191 | 37057.3 | base |
+---------------------------------------------+-------------------+--------------+-------------+------------------+
| acpi_cpufreq: ondemand | 51.51 | 19269.5 | 37406.5 | 11.04 % |
+---------------------------------------------+-------------------+--------------+-------------+------------------+
| acpi_cpufreq: performance | 45.96 | 17063.7 | 37123.7 | -0.74 % |
+---------------------------------------------+-------------------+--------------+-------------+------------------+
| EPP:powersave: performance(0) | 54.46 | 20263.1 | 37205 | 17.87 % |
+---------------------------------------------+-------------------+--------------+-------------+------------------+
| EPP:powersave: balance performance | 55.03 | 20481.9 | 37221.5 | 19.14 % |
+---------------------------------------------+-------------------+--------------+-------------+------------------+
| EPP:powersave: balance_power | 54.43 | 20245.9 | 37194.2 | 17.77 % |
+---------------------------------------------+-------------------+--------------+-------------+------------------+
| EPP:powersave: power(255) | 54.26 | 20181.7 | 37197.4 | 17.40 % |
+---------------------------------------------+-------------------+--------------+-------------+------------------+
| amd-pstate: schedutil | 48.22 | 17844.9 | 37006.6 | 3.80 % |
+---------------------------------------------+-------------------+--------------+-------------+------------------+
| amd-pstate: ondemand | 61.30 | 22988 | 37503.4 | 33.72 % |
+---------------------------------------------+-------------------+--------------+-------------+------------------+
| amd-pstate: performance | 54.52 | 20252.6 | 37147.8 | 17.81 % |
+---------------------------------------------+-------------------+--------------+-------------+------------------+
changes from v1:
* rebased to v6.0
* drive feedbacks from Mario for the suspend/resume patch
* drive feedbacks from Nathan for the EPP support on msr type
* fix some typos and code style indent problems
* update commit comments for patch 4/7
* change the `epp_enabled` module param name to `epp`
* set the default epp mode to be false
* add testing for the x86_energy_perf_policy utility patchset(will
send that utility patchset with another thread)
Perry Yuan (9):
ACPI: CPPC: Add AMD pstate energy performance preference cppc control
cpufreq: amd_pstate: add module parameter to load amd pstate EPP
driver
cpufreq: cpufreq: export cpufreq cpu release and acquire
x86/msr: Add the MSR definition for AMD CPPC boost state
Documentation: amd-pstate: add EPP profiles introduction
cpufreq: amd_pstate: add AMD pstate EPP support for shared memory type
processor
cpufreq: amd_pstate: add AMD Pstate EPP support for the MSR based
processors
cpufreq: amd_pstate: implement amd pstate cpu online and offline
callback
cpufreq: amd-pstate: implement suspend and resume callbacks
Documentation/admin-guide/pm/amd-pstate.rst | 19 +
arch/x86/include/asm/msr-index.h | 7 +
drivers/acpi/cppc_acpi.c | 128 ++-
drivers/cpufreq/amd-pstate.c | 949 +++++++++++++++++++-
drivers/cpufreq/cpufreq.c | 2 +
include/acpi/cppc_acpi.h | 17 +
6 files changed, 1115 insertions(+), 7 deletions(-)
--
2.34.1
cpufreq_cpu_release" and "cpufreq_cpu_acquire" are only used internally by
drivers/cpufreq/cpufreq.c currently.
Export them so that other drivers such as the AMD P-state driver can use them as well.
Signed-off-by: Perry Yuan <[email protected]>
---
drivers/cpufreq/cpufreq.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 69b3d61852ac..a491fea4985e 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -262,6 +262,7 @@ void cpufreq_cpu_release(struct cpufreq_policy *policy)
cpufreq_cpu_put(policy);
}
+EXPORT_SYMBOL_GPL(cpufreq_cpu_release);
/**
* cpufreq_cpu_acquire - Find policy for a CPU, mark it as busy and lock it.
@@ -291,6 +292,7 @@ struct cpufreq_policy *cpufreq_cpu_acquire(unsigned int cpu)
return policy;
}
+EXPORT_SYMBOL_GPL(cpufreq_cpu_acquire);
/*********************************************************************
* EXTERNALLY AFFECTING FREQUENCY CHANGES *
--
2.34.1
add suspend and resume support for the AMD processors by amd_pstate_epp
driver instance.
When the CPPC is suspended, EPP driver will set EPP profile to 'power'
profile and set max/min perf to lowest perf value.
When resume happens, it will restore the MSR registers with
previous cached value.
Signed-off-by: Perry Yuan <[email protected]>
---
drivers/cpufreq/amd-pstate.c | 40 ++++++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 8d99714022e3..a4683fdddde6 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -1472,6 +1472,44 @@ static int amd_pstate_epp_cpu_offline(struct cpufreq_policy *policy)
return amd_pstate_cpu_offline(policy);
}
+static int amd_pstate_epp_suspend(struct cpufreq_policy *policy)
+{
+ struct amd_cpudata *cpudata = all_cpu_data[policy->cpu];
+ int ret;
+
+ /* avoid suspending when EPP is not enabled */
+ if (!epp)
+ return 0;
+
+ /* set this flag to avoid setting core offline*/
+ cpudata->suspended = true;
+
+ /* disable CPPC in lowlevel firmware */
+ ret = amd_pstate_enable(false);
+ if (ret)
+ pr_err("failed to suspend, return %d\n", ret);
+
+ return 0;
+}
+
+static int amd_pstate_epp_resume(struct cpufreq_policy *policy)
+{
+ struct amd_cpudata *cpudata = all_cpu_data[policy->cpu];
+
+ if (cpudata->suspended) {
+ mutex_lock(&amd_pstate_limits_lock);
+
+ /* enable amd pstate from suspend state*/
+ amd_pstate_epp_reenable(cpudata);
+
+ mutex_unlock(&amd_pstate_limits_lock);
+
+ cpudata->suspended = false;
+ }
+
+ return 0;
+}
+
static void amd_pstate_verify_cpu_policy(struct amd_cpudata *cpudata,
struct cpufreq_policy_data *policy)
{
@@ -1508,6 +1546,8 @@ static struct cpufreq_driver amd_pstate_epp_driver = {
.update_limits = amd_pstate_epp_update_limits,
.offline = amd_pstate_epp_cpu_offline,
.online = amd_pstate_epp_cpu_online,
+ .suspend = amd_pstate_epp_suspend,
+ .resume = amd_pstate_epp_resume,
.name = "amd_pstate_epp",
.attr = amd_pstate_epp_attr,
};
--
2.34.1
This MSR can be used to check whether the CPU frequency boost state
is enabled in the hardware control. User can change the boost state in
the BIOS setting,amd_pstate driver will update the boost state according
to this msr value.
AMD Processor Programming Reference (PPR)
Link: https://www.amd.com/system/files/TechDocs/40332.pdf [p1095]
Link: https://www.amd.com/system/files/TechDocs/56569-A1-PUB.zip [p162]
Signed-off-by: Perry Yuan <[email protected]>
---
arch/x86/include/asm/msr-index.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 6674bdb096f3..e5ea1c9f747b 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -569,6 +569,7 @@
#define MSR_AMD_CPPC_CAP2 0xc00102b2
#define MSR_AMD_CPPC_REQ 0xc00102b3
#define MSR_AMD_CPPC_STATUS 0xc00102b4
+#define MSR_AMD_CPPC_HW_CTL 0xc0010015
#define AMD_CPPC_LOWEST_PERF(x) (((x) >> 0) & 0xff)
#define AMD_CPPC_LOWNONLIN_PERF(x) (((x) >> 8) & 0xff)
@@ -579,6 +580,8 @@
#define AMD_CPPC_MIN_PERF(x) (((x) & 0xff) << 8)
#define AMD_CPPC_DES_PERF(x) (((x) & 0xff) << 16)
#define AMD_CPPC_ENERGY_PERF_PREF(x) (((x) & 0xff) << 24)
+#define AMD_CPPC_PRECISION_BOOST_BIT 25
+#define AMD_CPPC_PRECISION_BOOST_ENABLED BIT_ULL(AMD_CPPC_PRECISION_BOOST_BIT)
/* AMD Performance Counter Global Status and Control MSRs */
#define MSR_AMD64_PERF_CNTR_GLOBAL_STATUS 0xc0000300
--
2.34.1
The patch adds online and offline driver callback support to allow cpu cores go
offline and help to restore the previous working states when core goes
back online later for EPP driver mode.
Signed-off-by: Perry Yuan <[email protected]>
---
drivers/cpufreq/amd-pstate.c | 101 ++++++++++++++++++++++++++++++++++-
1 file changed, 100 insertions(+), 1 deletion(-)
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 58418808aadf..8d99714022e3 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -115,7 +115,8 @@ struct amd_aperf_mperf {
* @cppc_cap1_cached: Cached value of the last CPPC Capabilities MSR
* @update_util: Cpufreq utility callback information
* @sample: the stored performance sample
-
+ * @suspended: Whether or not the driver has been suspended.
+ *
* The amd_cpudata is key private data for each CPU thread in AMD P-State, and
* represents all the attributes and goals that AMD P-State requests at runtime.
*/
@@ -155,6 +156,7 @@ struct amd_cpudata {
u64 cppc_cap1_cached;
struct update_util_data update_util;
struct amd_aperf_mperf sample;
+ bool suspended;
};
/**
@@ -215,6 +217,9 @@ static DEFINE_SPINLOCK(amd_pstate_cpu_lock);
static bool cppc_boost __read_mostly;
struct kobject *amd_pstate_kobj;
+/* the flag whether the pstate driver is exiting */
+static bool pstate_exiting;
+
#ifdef CONFIG_ACPI_CPPC_LIB
static s16 amd_pstate_get_epp(struct amd_cpudata *cpudata, u64 cppc_req_cached)
{
@@ -1377,6 +1382,96 @@ static int amd_pstate_epp_set_policy(struct cpufreq_policy *policy)
return 0;
}
+static void amd_pstate_epp_reenable(struct amd_cpudata *cpudata)
+{
+ struct cppc_perf_ctrls perf_ctrls;
+ u64 value, max_perf;
+ int ret;
+
+ ret = amd_pstate_enable(true);
+ if (ret)
+ pr_err("failed to enable amd pstate during resume, return %d\n", ret);
+
+ value = READ_ONCE(cpudata->cppc_req_cached);
+ max_perf = READ_ONCE(cpudata->highest_perf);
+
+ if (boot_cpu_has(X86_FEATURE_CPPC)) {
+ wrmsrl_on_cpu(cpudata->cpu, MSR_AMD_CPPC_REQ, value);
+ } else {
+ perf_ctrls.max_perf = max_perf;
+ perf_ctrls.energy_perf = AMD_CPPC_ENERGY_PERF_PREF(cpudata->epp_cached);
+ cppc_set_perf(cpudata->cpu, &perf_ctrls);
+ }
+}
+
+static int amd_pstate_epp_cpu_online(struct cpufreq_policy *policy)
+{
+ struct amd_cpudata *cpudata = all_cpu_data[policy->cpu];
+
+ pr_debug("AMD CPU Core %d going online\n", cpudata->cpu);
+
+ if (epp) {
+ amd_pstate_epp_reenable(cpudata);
+ cpudata->suspended = false;
+ }
+
+ return 0;
+}
+
+static void amd_pstate_epp_offline(struct cpufreq_policy *policy)
+{
+ struct amd_cpudata *cpudata = all_cpu_data[policy->cpu];
+ struct cppc_perf_ctrls perf_ctrls;
+ int min_perf;
+ u64 value;
+
+ min_perf = READ_ONCE(cpudata->lowest_perf);
+ value = READ_ONCE(cpudata->cppc_req_cached);
+
+ mutex_lock(&amd_pstate_limits_lock);
+ if (boot_cpu_has(X86_FEATURE_CPPC)) {
+ cpudata->epp_policy = CPUFREQ_POLICY_UNKNOWN;
+
+ /* Set max perf same as min perf */
+ value &= ~AMD_CPPC_MAX_PERF(~0L);
+ value |= AMD_CPPC_MAX_PERF(min_perf);
+ value &= ~AMD_CPPC_MIN_PERF(~0L);
+ value |= AMD_CPPC_MIN_PERF(min_perf);
+ wrmsrl_on_cpu(cpudata->cpu, MSR_AMD_CPPC_REQ, value);
+ } else {
+ perf_ctrls.desired_perf = 0;
+ perf_ctrls.max_perf = min_perf;
+ perf_ctrls.energy_perf = AMD_CPPC_ENERGY_PERF_PREF(AMD_CPPC_EPP_POWERSAVE);
+ cppc_set_perf(cpudata->cpu, &perf_ctrls);
+ }
+ mutex_unlock(&amd_pstate_limits_lock);
+}
+
+static int amd_pstate_cpu_offline(struct cpufreq_policy *policy)
+{
+ struct amd_cpudata *cpudata = all_cpu_data[policy->cpu];
+
+ pr_debug("AMD CPU Core %d going offline\n", cpudata->cpu);
+
+ if (cpudata->suspended)
+ return 0;
+
+ if (pstate_exiting)
+ return 0;
+
+ if (epp)
+ amd_pstate_epp_offline(policy);
+
+ return 0;
+}
+
+static int amd_pstate_epp_cpu_offline(struct cpufreq_policy *policy)
+{
+ amd_pstate_clear_update_util_hook(policy->cpu);
+
+ return amd_pstate_cpu_offline(policy);
+}
+
static void amd_pstate_verify_cpu_policy(struct amd_cpudata *cpudata,
struct cpufreq_policy_data *policy)
{
@@ -1411,6 +1506,8 @@ static struct cpufreq_driver amd_pstate_epp_driver = {
.init = amd_pstate_epp_cpu_init,
.exit = amd_pstate_epp_cpu_exit,
.update_limits = amd_pstate_epp_update_limits,
+ .offline = amd_pstate_epp_cpu_offline,
+ .online = amd_pstate_epp_cpu_online,
.name = "amd_pstate_epp",
.attr = amd_pstate_epp_attr,
};
@@ -1480,6 +1577,7 @@ static int __init amd_pstate_init(void)
pr_err("amd-pstate: Sysfs attribute export failed with error %d.\n",
ret);
}
+ pstate_exiting = false;
return ret;
}
@@ -1494,6 +1592,7 @@ static void __exit amd_pstate_exit(void)
{
unsigned int cpu;
+ pstate_exiting = true;
cpufreq_unregister_driver(default_pstate_driver);
amd_pstate_enable(false);
--
2.34.1
Although I am very much in favor of having some kernel interface to the
EPP MSR for AMD CPUs just as for Intel, I have some reservations about
the units in the tables, and whether performance per watt, measured in
this way by these benchmarks, is an appropriate figure of merit for
cpufreq governors.
On 10/10/22 11:22, Perry Yuan wrote:
> The PPW calculation is referred by below paper:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsoftware.intel.com%2Fcontent%2Fdam%2Fdevelop%2Fexternal%2Fus%2Fen%2Fdocuments%2Fperformance-per-what-paper.pdf&data=04%7C01%7CPerry.Yuan%40amd.com%7Cac66e8ce98044e9b062708d9ab47c8d8%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637729147708574423%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=TPOvCE%2Frbb0ptBreWNxHqOi9YnVhcHGKG88vviDLb00%3D&reserved=0
>
> Below formula is referred from below spec to measure the PPW:
>
> (F / t) / P = F * t / (t * E) = F / E,
>
> "F" is the number of frames per second.
> "P" is power measured in watts.
> "E" is energy measured in joules.
In the whitepaper, "F" is not the number of frames per second. It is
the number of frames. The number of frames per second is "F/t", where
"t" is the number of seconds. Following the dimensional analysis:
Frames
--------- / Watts
seconds
Frames Joules
= --------- / ---------
seconds seconds
Frames seconds
= --------- * ---------
seconds Joules
Frames
= ---------
Joules
All the seconds cancel, and performance per watt reduces to completed
work divided by energy, as you would expect. However, in the benchmark
tables, seconds always appear in the PPW unit.
Furthermore...
> Gitsouce Benchmark Data on ROME Server CPU
> +------------------------------+------------------------------+------------+------------------+
> | Kernel Module | PPW (1 / s * J) |Energy(J) | PPW Improvement (%)|
> +==============================+==============================+============+==================+
> | acpi-cpufreq:schedutil | 5.85658E-05 | 17074.8 | base |
> +------------------------------+------------------------------+------------+------------------+
> | acpi-cpufreq:ondemand | 5.03079E-05 | 19877.6 | -14.10% |
> +------------------------------+------------------------------+------------+------------------+
> | acpi-cpufreq:performance | 5.88132E-05 | 17003 | 0.42% |
> +------------------------------+------------------------------+------------+------------------+
> | amd-pstate:ondemand | 4.60295E-05 | 21725.2 | -21.41% |
> +------------------------------+------------------------------+------------+------------------+
> | amd-pstate:schedutil | 4.70026E-05 | 21275.4 | -19.7% |
> +------------------------------+------------------------------+------------+------------------+
> | amd-pstate:performance | 5.80094E-05 | 17238.6 | -0.95% |
> +------------------------------+------------------------------+------------+------------------+
> | EPP:performance | 5.8292E-05 | 17155 | -0.47% |
> +------------------------------+------------------------------+------------+------------------+
> | EPP: balance performance: | 6.71709E-05 | 14887.4 | 14.69% |
> +------------------------------+------------------------------+------------+------------------+
> | EPP:power | 6.66951E-05 | 4993.6 | 13.88% |
> +------------------------------+------------------------------+------------+------------------+
The numbers in the PPW column are equal to 1/Energy, so the math works
out even if the units are mislabeled. But neither the actual performance
nor anything that can be used to derive it appear in the table.
As far as I can tell, this benchmark, which compiles git from source,
should be entirely CPU bound. That is, it is occupying at least one CPU
core for the entire runtime. [1] For such tasks, to a first order
approximation you can run the CPU at 1/2 frequency and finish the task
with 1/4 the energy in 2x the time. Since the time units vanish,
"performance per watt" can look good when performance and watts are both
low. So you very much need to have performance in the table.
I can think of a couple ways to handle this problem. The empirical
approach would be to use the userspace governor and scaling_setspeed to
iteratively find a fixed frequency with similar benchmark performance to
each driver/governor, and then report the energy usage. The "benchmark"
should probably be a sum of multiple runtime benchmarks, or a harmonic
mean of multiple rate benchmarks, because the advantage a governor is
supposed to have is the ability to adapt to different workloads and/or
different phases of computation.
Alternately, one might use or perf^3/watt as the figure of merit. That's
an ED2P metric [2], and you'd be comparing governors on their ability to
make the CPU look like a "better" CPU by identifying tasks that waste a
lot of available cycles stalled on things outside the CPU core clock
domain (DRAM, I/O) and running them at lower frequency and higher
instructions per available cycle.
I've heard about perf^2/watt being used, but I don't know what, if any,
theoretical basis it has.
On another note, If PPW of CPU-bound tasks is maximized based on energy
counted with the CPU package energy MSR only (assuming it's even
calibrated), without including DRAM and baseline consumers like fans,
HDDs, southbridge, displays, NICs, radios, ect., then the PPW of the
system as a whole is certain to be worse. This is the idea behind
race-to-idle. On the other hand, CPU package power can be the correct
measure for deadline-type workloads where finishing the task sooner
doesn't allow powering down the machine. That's stuff like
line-speed-limited network servers and scrolling in web browsers. In
that case, the only thing that goes to sleep when the task is done is
the CPU, so the only energy that counts is the energy burnt in the CPU.
> Tbench Benchmark Data on ROME Server CPU
> +---------------------------------------------+-------------------+--------------+-------------+------------------+
> | Kernel Module | PPW MB / (s * J) |Throughput(MB/s)| Energy (J)|PPW Improvement(%)|
> +=============================================+===================+==============+=============+==================+
> | acpi_cpufreq: schedutil | 46.39 | 17191 | 37057.3 | base |
> +---------------------------------------------+-------------------+--------------+-------------+------------------+
> | acpi_cpufreq: ondemand | 51.51 | 19269.5 | 37406.5 | 11.04 % |
> +---------------------------------------------+-------------------+--------------+-------------+------------------+
> | acpi_cpufreq: performance | 45.96 | 17063.7 | 37123.7 | -0.74 % |
> +---------------------------------------------+-------------------+--------------+-------------+------------------+
> | EPP:powersave: performance(0) | 54.46 | 20263.1 | 37205 | 17.87 % |
> +---------------------------------------------+-------------------+--------------+-------------+------------------+
> | EPP:powersave: balance performance | 55.03 | 20481.9 | 37221.5 | 19.14 % |
> +---------------------------------------------+-------------------+--------------+-------------+------------------+
> | EPP:powersave: balance_power | 54.43 | 20245.9 | 37194.2 | 17.77 % |
> +---------------------------------------------+-------------------+--------------+-------------+------------------+
> | EPP:powersave: power(255) | 54.26 | 20181.7 | 37197.4 | 17.40 % |
> +---------------------------------------------+-------------------+--------------+-------------+------------------+
> | amd-pstate: schedutil | 48.22 | 17844.9 | 37006.6 | 3.80 % |
> +---------------------------------------------+-------------------+--------------+-------------+------------------+
> | amd-pstate: ondemand | 61.30 | 22988 | 37503.4 | 33.72 % |
> +---------------------------------------------+-------------------+--------------+-------------+------------------+
> | amd-pstate: performance | 54.52 | 20252.6 | 37147.8 | 17.81 % |
> +---------------------------------------------+-------------------+--------------+-------------+------------------+
For this one it seems like PPW is calculated as Throughput/Energy * 100?
The benchmark looks a lot like the result of running the script at [3].
It looks like the script would multiply by 99 though? And also the
bogus time units do not appear in the script, so if that's a newer
version I'm glad it's fixed.
But I ran tbench on my own machine, single-thread to reduce the impact
of background activity, and got this:
+---------------+---------------------+--------+
| CPU Frequency | Throughput ( MB/s ) | Perf % |
+===============+=====================+========+
| 1 GHz | 85.78 | Base |
| 2 GHz | 174.35 | 203 % |
| 3 GHz | 264.04 | 308 % |
| 4 GHz | 352.86 | 411 % |
+---------------+---------------------+--------+
Which implies tbench is 100% clock-frequency-bound [1, 4], and so this
benchmark is equivalent to measuring the average clock frequency over
the runtime. I think that means the most interesting number in your
table is the throughput.
Somehow, amd-pstate:ondemand is running the CPU faster on average than
even amd-pstate:performance and EPP:powersave:performance, which
*should* be choosing the highest possible frequency at all times.
1. As I understand it, the intent in the schedutil governor is to run
CPU-bound tasks at maximum performance, and if you want to trade energy
for time userspace should set cpu.uclamp.max in the cgroup. Any
CPU-bound benchmark that runs slower under the schedutil governor than
under the performance governor can then be considered a bug. There are
many such bugs, and tbench is one of them. But I agree with the
philosphy: 1:1 scaling with CPU frequency is the best possible, and no
governor should be running such a workload below scaling_max_frequency.
2. http://www.eecs.umich.edu/courses/eecs470/OLD/w14/lectures/470L14W14.pdf
3.
https://patchwork.kernel.org/project/linux-pm/patch/[email protected]/
4. I suspect the >100% scaling is due to the relative overhead of
background tasks and scheduling being less at higher clock frequency.
On Tue, Oct 11, 2022 at 12:22:39AM +0800, Yuan, Perry wrote:
> Hi all,
>
> This patchset implements one new AMD CPU frequency driver
> "amd-pstate-epp?? instance for better performance and power control.
> CPPC has a parameter called energy preference performance (EPP).
> The EPP is used in the CCLK DPM controller to drive the frequency that a core
> is going to operate during short periods of activity.
> EPP values will be utilized for different OS profiles (balanced, performance, power savings).
>
> AMD Energy Performance Preference (EPP) provides a hint to the hardware
> if software wants to bias toward performance (0x0) or energy efficiency (0xff)
> The lowlevel power firmware will calculate the runtime frequency according to the EPP preference
> value. So the EPP hint will impact the CPU cores frequency responsiveness.
>
> We use the RAPL interface with "perf" tool to get the energy data of the package power.
> Performance Per Watt (PPW) Calculation:
>
> The PPW calculation is referred by below paper:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsoftware.intel.com%2Fcontent%2Fdam%2Fdevelop%2Fexternal%2Fus%2Fen%2Fdocuments%2Fperformance-per-what-paper.pdf&data=04%7C01%7CPerry.Yuan%40amd.com%7Cac66e8ce98044e9b062708d9ab47c8d8%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637729147708574423%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=TPOvCE%2Frbb0ptBreWNxHqOi9YnVhcHGKG88vviDLb00%3D&reserved=0
>
> Below formula is referred from below spec to measure the PPW:
>
> (F / t) / P = F * t / (t * E) = F / E,
>
> "F" is the number of frames per second.
> "P" is power measured in watts.
> "E" is energy measured in joules.
>
> Gitsouce Benchmark Data on ROME Server CPU
> +------------------------------+------------------------------+------------+------------------+
> | Kernel Module | PPW (1 / s * J) |Energy(J) | PPW Improvement (%)|
> +==============================+==============================+============+==================+
> | acpi-cpufreq:schedutil | 5.85658E-05 | 17074.8 | base |
> +------------------------------+------------------------------+------------+------------------+
> | acpi-cpufreq:ondemand | 5.03079E-05 | 19877.6 | -14.10% |
> +------------------------------+------------------------------+------------+------------------+
> | acpi-cpufreq:performance | 5.88132E-05 | 17003 | 0.42% |
> +------------------------------+------------------------------+------------+------------------+
> | amd-pstate:ondemand | 4.60295E-05 | 21725.2 | -21.41% |
> +------------------------------+------------------------------+------------+------------------+
> | amd-pstate:schedutil | 4.70026E-05 | 21275.4 | -19.7% |
> +------------------------------+------------------------------+------------+------------------+
> | amd-pstate:performance | 5.80094E-05 | 17238.6 | -0.95% |
> +------------------------------+------------------------------+------------+------------------+
> | EPP:performance | 5.8292E-05 | 17155 | -0.47% |
> +------------------------------+------------------------------+------------+------------------+
> | EPP: balance performance: | 6.71709E-05 | 14887.4 | 14.69% |
> +------------------------------+------------------------------+------------+------------------+
> | EPP:power | 6.66951E-05 | 4993.6 | 13.88% |
> +------------------------------+------------------------------+------------+------------------+
>
> Tbench Benchmark Data on ROME Server CPU
> +---------------------------------------------+-------------------+--------------+-------------+------------------+
> | Kernel Module | PPW MB / (s * J) |Throughput(MB/s)| Energy (J)|PPW Improvement(%)|
> +=============================================+===================+==============+=============+==================+
> | acpi_cpufreq: schedutil | 46.39 | 17191 | 37057.3 | base |
> +---------------------------------------------+-------------------+--------------+-------------+------------------+
> | acpi_cpufreq: ondemand | 51.51 | 19269.5 | 37406.5 | 11.04 % |
> +---------------------------------------------+-------------------+--------------+-------------+------------------+
> | acpi_cpufreq: performance | 45.96 | 17063.7 | 37123.7 | -0.74 % |
> +---------------------------------------------+-------------------+--------------+-------------+------------------+
> | EPP:powersave: performance(0) | 54.46 | 20263.1 | 37205 | 17.87 % |
> +---------------------------------------------+-------------------+--------------+-------------+------------------+
> | EPP:powersave: balance performance | 55.03 | 20481.9 | 37221.5 | 19.14 % |
> +---------------------------------------------+-------------------+--------------+-------------+------------------+
> | EPP:powersave: balance_power | 54.43 | 20245.9 | 37194.2 | 17.77 % |
> +---------------------------------------------+-------------------+--------------+-------------+------------------+
> | EPP:powersave: power(255) | 54.26 | 20181.7 | 37197.4 | 17.40 % |
> +---------------------------------------------+-------------------+--------------+-------------+------------------+
> | amd-pstate: schedutil | 48.22 | 17844.9 | 37006.6 | 3.80 % |
> +---------------------------------------------+-------------------+--------------+-------------+------------------+
> | amd-pstate: ondemand | 61.30 | 22988 | 37503.4 | 33.72 % |
> +---------------------------------------------+-------------------+--------------+-------------+------------------+
> | amd-pstate: performance | 54.52 | 20252.6 | 37147.8 | 17.81 % |
> +---------------------------------------------+-------------------+--------------+-------------+------------------+
>
Please add the test cycle number in the average test result.
Since Jasmine has submitted the TBench and Gitsource benchmark test suite
into amd-pstate-ut test module, I suggest to align the test script and make
sure everybody can use that script to get the similar accurate performance
per watt data even on the EPP mode.
https://lore.kernel.org/lkml/[email protected]/
Thanks,
Ray
> changes from v1:
> * rebased to v6.0
> * drive feedbacks from Mario for the suspend/resume patch
> * drive feedbacks from Nathan for the EPP support on msr type
> * fix some typos and code style indent problems
> * update commit comments for patch 4/7
> * change the `epp_enabled` module param name to `epp`
> * set the default epp mode to be false
> * add testing for the x86_energy_perf_policy utility patchset(will
> send that utility patchset with another thread)
>
> Perry Yuan (9):
> ACPI: CPPC: Add AMD pstate energy performance preference cppc control
> cpufreq: amd_pstate: add module parameter to load amd pstate EPP
> driver
> cpufreq: cpufreq: export cpufreq cpu release and acquire
> x86/msr: Add the MSR definition for AMD CPPC boost state
> Documentation: amd-pstate: add EPP profiles introduction
> cpufreq: amd_pstate: add AMD pstate EPP support for shared memory type
> processor
> cpufreq: amd_pstate: add AMD Pstate EPP support for the MSR based
> processors
> cpufreq: amd_pstate: implement amd pstate cpu online and offline
> callback
> cpufreq: amd-pstate: implement suspend and resume callbacks
>
> Documentation/admin-guide/pm/amd-pstate.rst | 19 +
> arch/x86/include/asm/msr-index.h | 7 +
> drivers/acpi/cppc_acpi.c | 128 ++-
> drivers/cpufreq/amd-pstate.c | 949 +++++++++++++++++++-
> drivers/cpufreq/cpufreq.c | 2 +
> include/acpi/cppc_acpi.h | 17 +
> 6 files changed, 1115 insertions(+), 7 deletions(-)
>
> --
> 2.34.1
>
On Tue, Oct 11, 2022 at 12:22:43AM +0800, Yuan, Perry wrote:
> This MSR can be used to check whether the CPU frequency boost state
> is enabled in the hardware control. User can change the boost state in
> the BIOS setting,amd_pstate driver will update the boost state according
> to this msr value.
>
> AMD Processor Programming Reference (PPR)
> Link: https://www.amd.com/system/files/TechDocs/40332.pdf [p1095]
> Link: https://www.amd.com/system/files/TechDocs/56569-A1-PUB.zip [p162]
>
> Signed-off-by: Perry Yuan <[email protected]>
> ---
> arch/x86/include/asm/msr-index.h | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> index 6674bdb096f3..e5ea1c9f747b 100644
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -569,6 +569,7 @@
> #define MSR_AMD_CPPC_CAP2 0xc00102b2
> #define MSR_AMD_CPPC_REQ 0xc00102b3
> #define MSR_AMD_CPPC_STATUS 0xc00102b4
> +#define MSR_AMD_CPPC_HW_CTL 0xc0010015
>
> #define AMD_CPPC_LOWEST_PERF(x) (((x) >> 0) & 0xff)
> #define AMD_CPPC_LOWNONLIN_PERF(x) (((x) >> 8) & 0xff)
> @@ -579,6 +580,8 @@
> #define AMD_CPPC_MIN_PERF(x) (((x) & 0xff) << 8)
> #define AMD_CPPC_DES_PERF(x) (((x) & 0xff) << 16)
> #define AMD_CPPC_ENERGY_PERF_PREF(x) (((x) & 0xff) << 24)
> +#define AMD_CPPC_PRECISION_BOOST_BIT 25
> +#define AMD_CPPC_PRECISION_BOOST_ENABLED BIT_ULL(AMD_CPPC_PRECISION_BOOST_BIT)
I had commented the MSR_AMD_CPPC_HW_CTL is duplicated with MSR_K7_HWCR
https://lore.kernel.org/lkml/YtX+uF/[email protected]/
https://lore.kernel.org/lkml/[email protected]/
Could you please make sure address the commments?
Thanks,
Ray
[Public]
> -----Original Message-----
> From: Limonciello, Mario <[email protected]>
> Sent: Friday, October 21, 2022 12:05 AM
> To: Yuan, Perry <[email protected]>; Huang, Ray
> <[email protected]>
> Cc: [email protected]; [email protected]; Sharma, Deepak
> <[email protected]>; Fontenot, Nathan
> <[email protected]>; Deucher, Alexander
> <[email protected]>; Huang, Shimmer
> <[email protected]>; Du, Xiaojian <[email protected]>; Meng,
> Li (Jassmine) <[email protected]>; [email protected]; linux-
> [email protected]
> Subject: RE: [RESEND PATCH V2 4/9] x86/msr: Add the MSR definition for
> AMD CPPC boost state
>
> [Public]
>
>
>
> > -----Original Message-----
> > From: Yuan, Perry <[email protected]>
> > Sent: Thursday, October 20, 2022 11:01
> > To: Huang, Ray <[email protected]>
> > Cc: [email protected]; [email protected]; Sharma,
> > Deepak <[email protected]>; Limonciello, Mario
> > <[email protected]>; Fontenot, Nathan
> > <[email protected]>; Deucher, Alexander
> > <[email protected]>; Huang, Shimmer
> <[email protected]>;
> > Du, Xiaojian <[email protected]>; Meng, Li (Jassmine)
> > <[email protected]>; [email protected]; linux-
> > [email protected]
> > Subject: RE: [RESEND PATCH V2 4/9] x86/msr: Add the MSR definition for
> > AMD CPPC boost state
> >
> > [AMD Official Use Only - General]
> >
> > Hi Ray.
> >
> > > -----Original Message-----
> > > From: Huang, Ray <[email protected]>
> > > Sent: Monday, October 17, 2022 5:57 PM
> > > To: Yuan, Perry <[email protected]>
> > > Cc: [email protected]; [email protected]; Sharma,
> > > Deepak <[email protected]>; Limonciello, Mario
> > > <[email protected]>; Fontenot, Nathan
> > > <[email protected]>; Deucher, Alexander
> > > <[email protected]>; Huang, Shimmer
> <[email protected]>;
> > > Du, Xiaojian <[email protected]>;
> > Meng,
> > > Li (Jassmine) <[email protected]>; [email protected]; linux-
> > > [email protected]
> > > Subject: Re: [RESEND PATCH V2 4/9] x86/msr: Add the MSR definition
> > > for AMD CPPC boost state
> > >
> > > On Tue, Oct 11, 2022 at 12:22:43AM +0800, Yuan, Perry wrote:
> > > > This MSR can be used to check whether the CPU frequency boost
> > > > state is enabled in the hardware control. User can change the
> > > > boost state in the BIOS setting,amd_pstate driver will update the
> > > > boost state according to this msr value.
> > > >
> > > > AMD Processor Programming Reference (PPR)
> > > > Link: https://www.amd.com/system/files/TechDocs/40332.pdf [p1095]
> > > > Link: https://www.amd.com/system/files/TechDocs/56569-A1-PUB.zip
> > > > [p162]
> > > >
> > > > Signed-off-by: Perry Yuan <[email protected]>
> > > > ---
> > > > arch/x86/include/asm/msr-index.h | 3 +++
> > > > 1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/arch/x86/include/asm/msr-index.h
> > > > b/arch/x86/include/asm/msr-index.h
> > > > index 6674bdb096f3..e5ea1c9f747b 100644
> > > > --- a/arch/x86/include/asm/msr-index.h
> > > > +++ b/arch/x86/include/asm/msr-index.h
> > > > @@ -569,6 +569,7 @@
> > > > #define MSR_AMD_CPPC_CAP2 0xc00102b2
> > > > #define MSR_AMD_CPPC_REQ 0xc00102b3
> > > > #define MSR_AMD_CPPC_STATUS 0xc00102b4
> > > > +#define MSR_AMD_CPPC_HW_CTL 0xc0010015
> > > >
> > > > #define AMD_CPPC_LOWEST_PERF(x) (((x) >> 0) & 0xff)
> > > > #define AMD_CPPC_LOWNONLIN_PERF(x) (((x) >> 8) & 0xff)
> > > > @@ -579,6 +580,8 @@
> > > > #define AMD_CPPC_MIN_PERF(x) (((x) & 0xff) << 8)
> > > > #define AMD_CPPC_DES_PERF(x) (((x) & 0xff) << 16)
> > > > #define AMD_CPPC_ENERGY_PERF_PREF(x) (((x) & 0xff) << 24)
> > > > +#define AMD_CPPC_PRECISION_BOOST_BIT 25
> > > > +#define AMD_CPPC_PRECISION_BOOST_ENABLED
> > > BIT_ULL(AMD_CPPC_PRECISION_BOOST_BIT)
> > >
> > > I had commented the MSR_AMD_CPPC_HW_CTL is duplicated with
> > > MSR_K7_HWCR
> > >
> > > https://lore.kernel.org/lkml/YtX+uF/[email protected]/
> > > https://lore.kernel.org/lkml/[email protected]/
> > >
> > > Could you please make sure address the commments?
> > >
> > > Thanks,
> > > Ray
> >
> > If I rename that the MSR definition string, that will cause lots of
> > driver file change.
> > So I suggest to add one new MSR macro for the CPPC, the MSR_K7_HWCR is
> > mismatching in the CPPC Pstate driver.
> > If you refuse to use this new one, I will reuse that old one.
>
> To avoid changing too much stuff at once how about if you give an alias?
> IE something like:
>
> #define MSR_AMD_CPPC_HW_CTL MSR_K7_HWCR
I prefer this code suggestion.
Then the new name string can be matched with CPPC/Pstate driver.
>
> >
> > Perry.
[Public]
> -----Original Message-----
> From: Yuan, Perry <[email protected]>
> Sent: Thursday, October 20, 2022 11:01
> To: Huang, Ray <[email protected]>
> Cc: [email protected]; [email protected]; Sharma, Deepak
> <[email protected]>; Limonciello, Mario
> <[email protected]>; Fontenot, Nathan
> <[email protected]>; Deucher, Alexander
> <[email protected]>; Huang, Shimmer
> <[email protected]>; Du, Xiaojian <[email protected]>; Meng,
> Li (Jassmine) <[email protected]>; [email protected]; linux-
> [email protected]
> Subject: RE: [RESEND PATCH V2 4/9] x86/msr: Add the MSR definition for
> AMD CPPC boost state
>
> [AMD Official Use Only - General]
>
> Hi Ray.
>
> > -----Original Message-----
> > From: Huang, Ray <[email protected]>
> > Sent: Monday, October 17, 2022 5:57 PM
> > To: Yuan, Perry <[email protected]>
> > Cc: [email protected]; [email protected]; Sharma, Deepak
> > <[email protected]>; Limonciello, Mario
> > <[email protected]>; Fontenot, Nathan
> > <[email protected]>; Deucher, Alexander
> > <[email protected]>; Huang, Shimmer
> > <[email protected]>; Du, Xiaojian <[email protected]>;
> Meng,
> > Li (Jassmine) <[email protected]>; [email protected]; linux-
> > [email protected]
> > Subject: Re: [RESEND PATCH V2 4/9] x86/msr: Add the MSR definition for
> > AMD CPPC boost state
> >
> > On Tue, Oct 11, 2022 at 12:22:43AM +0800, Yuan, Perry wrote:
> > > This MSR can be used to check whether the CPU frequency boost state is
> > > enabled in the hardware control. User can change the boost state in
> > > the BIOS setting,amd_pstate driver will update the boost state
> > > according to this msr value.
> > >
> > > AMD Processor Programming Reference (PPR)
> > > Link: https://www.amd.com/system/files/TechDocs/40332.pdf [p1095]
> > > Link: https://www.amd.com/system/files/TechDocs/56569-A1-PUB.zip
> > > [p162]
> > >
> > > Signed-off-by: Perry Yuan <[email protected]>
> > > ---
> > > arch/x86/include/asm/msr-index.h | 3 +++
> > > 1 file changed, 3 insertions(+)
> > >
> > > diff --git a/arch/x86/include/asm/msr-index.h
> > > b/arch/x86/include/asm/msr-index.h
> > > index 6674bdb096f3..e5ea1c9f747b 100644
> > > --- a/arch/x86/include/asm/msr-index.h
> > > +++ b/arch/x86/include/asm/msr-index.h
> > > @@ -569,6 +569,7 @@
> > > #define MSR_AMD_CPPC_CAP2 0xc00102b2
> > > #define MSR_AMD_CPPC_REQ 0xc00102b3
> > > #define MSR_AMD_CPPC_STATUS 0xc00102b4
> > > +#define MSR_AMD_CPPC_HW_CTL 0xc0010015
> > >
> > > #define AMD_CPPC_LOWEST_PERF(x) (((x) >> 0) & 0xff)
> > > #define AMD_CPPC_LOWNONLIN_PERF(x) (((x) >> 8) & 0xff)
> > > @@ -579,6 +580,8 @@
> > > #define AMD_CPPC_MIN_PERF(x) (((x) & 0xff) << 8)
> > > #define AMD_CPPC_DES_PERF(x) (((x) & 0xff) << 16)
> > > #define AMD_CPPC_ENERGY_PERF_PREF(x) (((x) & 0xff) << 24)
> > > +#define AMD_CPPC_PRECISION_BOOST_BIT 25
> > > +#define AMD_CPPC_PRECISION_BOOST_ENABLED
> > BIT_ULL(AMD_CPPC_PRECISION_BOOST_BIT)
> >
> > I had commented the MSR_AMD_CPPC_HW_CTL is duplicated with
> > MSR_K7_HWCR
> >
> > https://lore.kernel.org/lkml/YtX+uF/[email protected]/
> > https://lore.kernel.org/lkml/[email protected]/
> >
> > Could you please make sure address the commments?
> >
> > Thanks,
> > Ray
>
> If I rename that the MSR definition string, that will cause lots of driver file
> change.
> So I suggest to add one new MSR macro for the CPPC, the MSR_K7_HWCR is
> mismatching in the CPPC Pstate driver.
> If you refuse to use this new one, I will reuse that old one.
To avoid changing too much stuff at once how about if you give an alias?
IE something like:
#define MSR_AMD_CPPC_HW_CTL MSR_K7_HWCR
>
> Perry.
[AMD Official Use Only - General]
Hi Ray.
> -----Original Message-----
> From: Huang, Ray <[email protected]>
> Sent: Monday, October 17, 2022 5:57 PM
> To: Yuan, Perry <[email protected]>
> Cc: [email protected]; [email protected]; Sharma, Deepak
> <[email protected]>; Limonciello, Mario
> <[email protected]>; Fontenot, Nathan
> <[email protected]>; Deucher, Alexander
> <[email protected]>; Huang, Shimmer
> <[email protected]>; Du, Xiaojian <[email protected]>; Meng,
> Li (Jassmine) <[email protected]>; [email protected]; linux-
> [email protected]
> Subject: Re: [RESEND PATCH V2 4/9] x86/msr: Add the MSR definition for
> AMD CPPC boost state
>
> On Tue, Oct 11, 2022 at 12:22:43AM +0800, Yuan, Perry wrote:
> > This MSR can be used to check whether the CPU frequency boost state is
> > enabled in the hardware control. User can change the boost state in
> > the BIOS setting,amd_pstate driver will update the boost state
> > according to this msr value.
> >
> > AMD Processor Programming Reference (PPR)
> > Link: https://www.amd.com/system/files/TechDocs/40332.pdf [p1095]
> > Link: https://www.amd.com/system/files/TechDocs/56569-A1-PUB.zip
> > [p162]
> >
> > Signed-off-by: Perry Yuan <[email protected]>
> > ---
> > arch/x86/include/asm/msr-index.h | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/arch/x86/include/asm/msr-index.h
> > b/arch/x86/include/asm/msr-index.h
> > index 6674bdb096f3..e5ea1c9f747b 100644
> > --- a/arch/x86/include/asm/msr-index.h
> > +++ b/arch/x86/include/asm/msr-index.h
> > @@ -569,6 +569,7 @@
> > #define MSR_AMD_CPPC_CAP2 0xc00102b2
> > #define MSR_AMD_CPPC_REQ 0xc00102b3
> > #define MSR_AMD_CPPC_STATUS 0xc00102b4
> > +#define MSR_AMD_CPPC_HW_CTL 0xc0010015
> >
> > #define AMD_CPPC_LOWEST_PERF(x) (((x) >> 0) & 0xff)
> > #define AMD_CPPC_LOWNONLIN_PERF(x) (((x) >> 8) & 0xff)
> > @@ -579,6 +580,8 @@
> > #define AMD_CPPC_MIN_PERF(x) (((x) & 0xff) << 8)
> > #define AMD_CPPC_DES_PERF(x) (((x) & 0xff) << 16)
> > #define AMD_CPPC_ENERGY_PERF_PREF(x) (((x) & 0xff) << 24)
> > +#define AMD_CPPC_PRECISION_BOOST_BIT 25
> > +#define AMD_CPPC_PRECISION_BOOST_ENABLED
> BIT_ULL(AMD_CPPC_PRECISION_BOOST_BIT)
>
> I had commented the MSR_AMD_CPPC_HW_CTL is duplicated with
> MSR_K7_HWCR
>
> https://lore.kernel.org/lkml/YtX+uF/[email protected]/
> https://lore.kernel.org/lkml/[email protected]/
>
> Could you please make sure address the commments?
>
> Thanks,
> Ray
If I rename that the MSR definition string, that will cause lots of driver file change.
So I suggest to add one new MSR macro for the CPPC, the MSR_K7_HWCR is mismatching in the CPPC Pstate driver.
If you refuse to use this new one, I will reuse that old one.
Perry.
+ Boris,
On Fri, Oct 21, 2022 at 12:05:21AM +0800, Limonciello, Mario wrote:
> [Public]
>
>
>
> > -----Original Message-----
> > From: Yuan, Perry <[email protected]>
> > Sent: Thursday, October 20, 2022 11:01
> > To: Huang, Ray <[email protected]>
> > Cc: [email protected]; [email protected]; Sharma, Deepak
> > <[email protected]>; Limonciello, Mario
> > <[email protected]>; Fontenot, Nathan
> > <[email protected]>; Deucher, Alexander
> > <[email protected]>; Huang, Shimmer
> > <[email protected]>; Du, Xiaojian <[email protected]>; Meng,
> > Li (Jassmine) <[email protected]>; [email protected]; linux-
> > [email protected]
> > Subject: RE: [RESEND PATCH V2 4/9] x86/msr: Add the MSR definition for
> > AMD CPPC boost state
> >
> > [AMD Official Use Only - General]
> >
> > Hi Ray.
> >
> > > -----Original Message-----
> > > From: Huang, Ray <[email protected]>
> > > Sent: Monday, October 17, 2022 5:57 PM
> > > To: Yuan, Perry <[email protected]>
> > > Cc: [email protected]; [email protected]; Sharma, Deepak
> > > <[email protected]>; Limonciello, Mario
> > > <[email protected]>; Fontenot, Nathan
> > > <[email protected]>; Deucher, Alexander
> > > <[email protected]>; Huang, Shimmer
> > > <[email protected]>; Du, Xiaojian <[email protected]>;
> > Meng,
> > > Li (Jassmine) <[email protected]>; [email protected]; linux-
> > > [email protected]
> > > Subject: Re: [RESEND PATCH V2 4/9] x86/msr: Add the MSR definition for
> > > AMD CPPC boost state
> > >
> > > On Tue, Oct 11, 2022 at 12:22:43AM +0800, Yuan, Perry wrote:
> > > > This MSR can be used to check whether the CPU frequency boost state is
> > > > enabled in the hardware control. User can change the boost state in
> > > > the BIOS setting,amd_pstate driver will update the boost state
> > > > according to this msr value.
> > > >
> > > > AMD Processor Programming Reference (PPR)
> > > > Link: https://www.amd.com/system/files/TechDocs/40332.pdf [p1095]
> > > > Link: https://www.amd.com/system/files/TechDocs/56569-A1-PUB.zip
> > > > [p162]
> > > >
> > > > Signed-off-by: Perry Yuan <[email protected]>
> > > > ---
> > > > arch/x86/include/asm/msr-index.h | 3 +++
> > > > 1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/arch/x86/include/asm/msr-index.h
> > > > b/arch/x86/include/asm/msr-index.h
> > > > index 6674bdb096f3..e5ea1c9f747b 100644
> > > > --- a/arch/x86/include/asm/msr-index.h
> > > > +++ b/arch/x86/include/asm/msr-index.h
> > > > @@ -569,6 +569,7 @@
> > > > #define MSR_AMD_CPPC_CAP2 0xc00102b2
> > > > #define MSR_AMD_CPPC_REQ 0xc00102b3
> > > > #define MSR_AMD_CPPC_STATUS 0xc00102b4
> > > > +#define MSR_AMD_CPPC_HW_CTL 0xc0010015
> > > >
> > > > #define AMD_CPPC_LOWEST_PERF(x) (((x) >> 0) & 0xff)
> > > > #define AMD_CPPC_LOWNONLIN_PERF(x) (((x) >> 8) & 0xff)
> > > > @@ -579,6 +580,8 @@
> > > > #define AMD_CPPC_MIN_PERF(x) (((x) & 0xff) << 8)
> > > > #define AMD_CPPC_DES_PERF(x) (((x) & 0xff) << 16)
> > > > #define AMD_CPPC_ENERGY_PERF_PREF(x) (((x) & 0xff) << 24)
> > > > +#define AMD_CPPC_PRECISION_BOOST_BIT 25
> > > > +#define AMD_CPPC_PRECISION_BOOST_ENABLED
> > > BIT_ULL(AMD_CPPC_PRECISION_BOOST_BIT)
> > >
> > > I had commented the MSR_AMD_CPPC_HW_CTL is duplicated with
> > > MSR_K7_HWCR
> > >
> > > https://lore.kernel.org/lkml/YtX+uF/[email protected]/
> > > https://lore.kernel.org/lkml/[email protected]/
> > >
> > > Could you please make sure address the commments?
> > >
> > > Thanks,
> > > Ray
> >
> > If I rename that the MSR definition string, that will cause lots of driver file
> > change.
> > So I suggest to add one new MSR macro for the CPPC, the MSR_K7_HWCR is
> > mismatching in the CPPC Pstate driver.
> > If you refuse to use this new one, I will reuse that old one.
>
> To avoid changing too much stuff at once how about if you give an alias?
> IE something like:
>
> #define MSR_AMD_CPPC_HW_CTL MSR_K7_HWCR
>
The mainly concern is that HWCR is for legacy ACPI P-State control not for
CPPC. I talked with hardware guys before, it's not suggested to mix them up
together. This register has been defined for a long time even before Zen
processor.
Thanks,
Ray
On Fri, Oct 21, 2022 at 01:22:11PM +0800, Huang Rui wrote:
> > > If I rename that the MSR definition string, that will cause lots of driver file
> > > change.
> > > So I suggest to add one new MSR macro for the CPPC, the MSR_K7_HWCR is
> > > mismatching in the CPPC Pstate driver.
> > > If you refuse to use this new one, I will reuse that old one.
> >
> > To avoid changing too much stuff at once how about if you give an alias?
> > IE something like:
> >
> > #define MSR_AMD_CPPC_HW_CTL MSR_K7_HWCR
Why would you all even think about adding a new name and not use
MSR_K7_HWCR?
The other code uses it just fine, do git grep MSR_K7_HWCR.
We have waaay too many MSRs, no need to unnecessarily confuse people
with an alias or rename stuff. Just use MSR_K7_HWCR like everything else
does.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
[AMD Official Use Only - General]
Hi Boris.
> -----Original Message-----
> From: Borislav Petkov <[email protected]>
> Sent: Friday, October 21, 2022 5:38 PM
> To: Huang, Ray <[email protected]>
> Cc: Limonciello, Mario <[email protected]>; Yuan, Perry
> <[email protected]>; [email protected];
> [email protected]; Sharma, Deepak <[email protected]>;
> Fontenot, Nathan <[email protected]>; Deucher, Alexander
> <[email protected]>; Huang, Shimmer
> <[email protected]>; Du, Xiaojian <[email protected]>; Meng,
> Li (Jassmine) <[email protected]>; [email protected]; linux-
> [email protected]
> Subject: Re: [RESEND PATCH V2 4/9] x86/msr: Add the MSR definition for
> AMD CPPC boost state
>
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
>
>
> On Fri, Oct 21, 2022 at 01:22:11PM +0800, Huang Rui wrote:
> > > > If I rename that the MSR definition string, that will cause lots
> > > > of driver file change.
> > > > So I suggest to add one new MSR macro for the CPPC, the
> > > > MSR_K7_HWCR is mismatching in the CPPC Pstate driver.
> > > > If you refuse to use this new one, I will reuse that old one.
> > >
> > > To avoid changing too much stuff at once how about if you give an alias?
> > > IE something like:
> > >
> > > #define MSR_AMD_CPPC_HW_CTL MSR_K7_HWCR
>
> Why would you all even think about adding a new name and not use
> MSR_K7_HWCR?
>
> The other code uses it just fine, do git grep MSR_K7_HWCR.
>
> We have waaay too many MSRs, no need to unnecessarily confuse people
> with an alias or rename stuff. Just use MSR_K7_HWCR like everything else
> does.
>
> --
> Regards/Gruss,
> Boris.
>
Sure, we can continue to use that MSR definition as you suggested.
Thank you comment on this.
Perry.
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpeop
> le.kernel.org%2Ftglx%2Fnotes-about-
> netiquette&data=05%7C01%7CPerry.Yuan%40amd.com%7Ce3d156c553
> 2a490a2f2d08dab347f1e8%7C3dd8961fe4884e608e11a82d994e183d%7C0%7
> C0%7C638019418849896672%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4w
> LjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C
> %7C%7C&sdata=wVEYx3BNksqXxaycPl81BjVotrWMlcJnVNdbFO7bQto%
> 3D&reserved=0
[AMD Official Use Only - General]
Hi Ray.
> -----Original Message-----
> From: Huang, Ray <[email protected]>
> Sent: Friday, October 21, 2022 1:22 PM
> To: Limonciello, Mario <[email protected]>
> Cc: Yuan, Perry <[email protected]>; [email protected];
> [email protected]; Sharma, Deepak <[email protected]>;
> Fontenot, Nathan <[email protected]>; Deucher, Alexander
> <[email protected]>; Huang, Shimmer
> <[email protected]>; Du, Xiaojian <[email protected]>; Meng,
> Li (Jassmine) <[email protected]>; [email protected]; linux-
> [email protected]; Borislav Petkov <[email protected]>
> Subject: Re: [RESEND PATCH V2 4/9] x86/msr: Add the MSR definition for
> AMD CPPC boost state
>
> + Boris,
>
> On Fri, Oct 21, 2022 at 12:05:21AM +0800, Limonciello, Mario wrote:
> > [Public]
> >
> >
> >
> > > -----Original Message-----
> > > From: Yuan, Perry <[email protected]>
> > > Sent: Thursday, October 20, 2022 11:01
> > > To: Huang, Ray <[email protected]>
> > > Cc: [email protected]; [email protected]; Sharma,
> > > Deepak <[email protected]>; Limonciello, Mario
> > > <[email protected]>; Fontenot, Nathan
> > > <[email protected]>; Deucher, Alexander
> > > <[email protected]>; Huang, Shimmer
> <[email protected]>;
> > > Du, Xiaojian <[email protected]>; Meng, Li (Jassmine)
> > > <[email protected]>; [email protected]; linux-
> > > [email protected]
> > > Subject: RE: [RESEND PATCH V2 4/9] x86/msr: Add the MSR definition
> > > for AMD CPPC boost state
> > >
> > > [AMD Official Use Only - General]
> > >
> > > Hi Ray.
> > >
> > > > -----Original Message-----
> > > > From: Huang, Ray <[email protected]>
> > > > Sent: Monday, October 17, 2022 5:57 PM
> > > > To: Yuan, Perry <[email protected]>
> > > > Cc: [email protected]; [email protected]; Sharma,
> > > > Deepak <[email protected]>; Limonciello, Mario
> > > > <[email protected]>; Fontenot, Nathan
> > > > <[email protected]>; Deucher, Alexander
> > > > <[email protected]>; Huang, Shimmer
> > > > <[email protected]>; Du, Xiaojian <[email protected]>;
> > > Meng,
> > > > Li (Jassmine) <[email protected]>; [email protected]; linux-
> > > > [email protected]
> > > > Subject: Re: [RESEND PATCH V2 4/9] x86/msr: Add the MSR definition
> > > > for AMD CPPC boost state
> > > >
> > > > On Tue, Oct 11, 2022 at 12:22:43AM +0800, Yuan, Perry wrote:
> > > > > This MSR can be used to check whether the CPU frequency boost
> > > > > state is enabled in the hardware control. User can change the
> > > > > boost state in the BIOS setting,amd_pstate driver will update
> > > > > the boost state according to this msr value.
> > > > >
> > > > > AMD Processor Programming Reference (PPR)
> > > > > Link: https://www.amd.com/system/files/TechDocs/40332.pdf
> > > > > [p1095]
> > > > > Link: https://www.amd.com/system/files/TechDocs/56569-A1-
> PUB.zip
> > > > > [p162]
> > > > >
> > > > > Signed-off-by: Perry Yuan <[email protected]>
> > > > > ---
> > > > > arch/x86/include/asm/msr-index.h | 3 +++
> > > > > 1 file changed, 3 insertions(+)
> > > > >
> > > > > diff --git a/arch/x86/include/asm/msr-index.h
> > > > > b/arch/x86/include/asm/msr-index.h
> > > > > index 6674bdb096f3..e5ea1c9f747b 100644
> > > > > --- a/arch/x86/include/asm/msr-index.h
> > > > > +++ b/arch/x86/include/asm/msr-index.h
> > > > > @@ -569,6 +569,7 @@
> > > > > #define MSR_AMD_CPPC_CAP2 0xc00102b2
> > > > > #define MSR_AMD_CPPC_REQ 0xc00102b3
> > > > > #define MSR_AMD_CPPC_STATUS 0xc00102b4
> > > > > +#define MSR_AMD_CPPC_HW_CTL 0xc0010015
> > > > >
> > > > > #define AMD_CPPC_LOWEST_PERF(x) (((x) >> 0) & 0xff)
> > > > > #define AMD_CPPC_LOWNONLIN_PERF(x) (((x) >> 8) & 0xff)
> > > > > @@ -579,6 +580,8 @@
> > > > > #define AMD_CPPC_MIN_PERF(x) (((x) & 0xff) << 8)
> > > > > #define AMD_CPPC_DES_PERF(x) (((x) & 0xff) << 16)
> > > > > #define AMD_CPPC_ENERGY_PERF_PREF(x) (((x) & 0xff) << 24)
> > > > > +#define AMD_CPPC_PRECISION_BOOST_BIT 25
> > > > > +#define AMD_CPPC_PRECISION_BOOST_ENABLED
> > > > BIT_ULL(AMD_CPPC_PRECISION_BOOST_BIT)
> > > >
> > > > I had commented the MSR_AMD_CPPC_HW_CTL is duplicated with
> > > > MSR_K7_HWCR
> > > >
> > > > https://lore.kernel.org/lkml/YtX+uF/[email protected]/
> > > > https://lore.kernel.org/lkml/[email protected]/
> > > >
> > > > Could you please make sure address the commments?
> > > >
> > > > Thanks,
> > > > Ray
> > >
> > > If I rename that the MSR definition string, that will cause lots of
> > > driver file change.
> > > So I suggest to add one new MSR macro for the CPPC, the MSR_K7_HWCR
> > > is mismatching in the CPPC Pstate driver.
> > > If you refuse to use this new one, I will reuse that old one.
> >
> > To avoid changing too much stuff at once how about if you give an alias?
> > IE something like:
> >
> > #define MSR_AMD_CPPC_HW_CTL MSR_K7_HWCR
> >
>
> The mainly concern is that HWCR is for legacy ACPI P-State control not for
> CPPC. I talked with hardware guys before, it's not suggested to mix them up
> together. This register has been defined for a long time even before Zen
> processor.
>
> Thanks,
> Ray
I have removed the code not to write boost state to that MSR, just check the boost state from the MSR bit value.
It will not cause any problems, I have tested and confirmed that the BIT value will be changed after BOOST ON/OFF switched in BIOS setting.
So we can just check the boost state here for pstate driver notification.
Perry.
On Mon, Oct 24, 2022 at 10:56:50AM +0800, Yuan, Perry wrote:
> [AMD Official Use Only - General]
>
> Hi Ray.
>
> > -----Original Message-----
> > From: Huang, Ray <[email protected]>
> > Sent: Friday, October 21, 2022 1:22 PM
> > To: Limonciello, Mario <[email protected]>
> > Cc: Yuan, Perry <[email protected]>; [email protected];
> > [email protected]; Sharma, Deepak <[email protected]>;
> > Fontenot, Nathan <[email protected]>; Deucher, Alexander
> > <[email protected]>; Huang, Shimmer
> > <[email protected]>; Du, Xiaojian <[email protected]>; Meng,
> > Li (Jassmine) <[email protected]>; [email protected]; linux-
> > [email protected]; Borislav Petkov <[email protected]>
> > Subject: Re: [RESEND PATCH V2 4/9] x86/msr: Add the MSR definition for
> > AMD CPPC boost state
> >
> > + Boris,
> >
> > On Fri, Oct 21, 2022 at 12:05:21AM +0800, Limonciello, Mario wrote:
> > > [Public]
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Yuan, Perry <[email protected]>
> > > > Sent: Thursday, October 20, 2022 11:01
> > > > To: Huang, Ray <[email protected]>
> > > > Cc: [email protected]; [email protected]; Sharma,
> > > > Deepak <[email protected]>; Limonciello, Mario
> > > > <[email protected]>; Fontenot, Nathan
> > > > <[email protected]>; Deucher, Alexander
> > > > <[email protected]>; Huang, Shimmer
> > <[email protected]>;
> > > > Du, Xiaojian <[email protected]>; Meng, Li (Jassmine)
> > > > <[email protected]>; [email protected]; linux-
> > > > [email protected]
> > > > Subject: RE: [RESEND PATCH V2 4/9] x86/msr: Add the MSR definition
> > > > for AMD CPPC boost state
> > > >
> > > > [AMD Official Use Only - General]
> > > >
> > > > Hi Ray.
> > > >
> > > > > -----Original Message-----
> > > > > From: Huang, Ray <[email protected]>
> > > > > Sent: Monday, October 17, 2022 5:57 PM
> > > > > To: Yuan, Perry <[email protected]>
> > > > > Cc: [email protected]; [email protected]; Sharma,
> > > > > Deepak <[email protected]>; Limonciello, Mario
> > > > > <[email protected]>; Fontenot, Nathan
> > > > > <[email protected]>; Deucher, Alexander
> > > > > <[email protected]>; Huang, Shimmer
> > > > > <[email protected]>; Du, Xiaojian <[email protected]>;
> > > > Meng,
> > > > > Li (Jassmine) <[email protected]>; [email protected]; linux-
> > > > > [email protected]
> > > > > Subject: Re: [RESEND PATCH V2 4/9] x86/msr: Add the MSR definition
> > > > > for AMD CPPC boost state
> > > > >
> > > > > On Tue, Oct 11, 2022 at 12:22:43AM +0800, Yuan, Perry wrote:
> > > > > > This MSR can be used to check whether the CPU frequency boost
> > > > > > state is enabled in the hardware control. User can change the
> > > > > > boost state in the BIOS setting,amd_pstate driver will update
> > > > > > the boost state according to this msr value.
> > > > > >
> > > > > > AMD Processor Programming Reference (PPR)
> > > > > > Link: https://www.amd.com/system/files/TechDocs/40332.pdf
> > > > > > [p1095]
> > > > > > Link: https://www.amd.com/system/files/TechDocs/56569-A1-
> > PUB.zip
> > > > > > [p162]
> > > > > >
> > > > > > Signed-off-by: Perry Yuan <[email protected]>
> > > > > > ---
> > > > > > arch/x86/include/asm/msr-index.h | 3 +++
> > > > > > 1 file changed, 3 insertions(+)
> > > > > >
> > > > > > diff --git a/arch/x86/include/asm/msr-index.h
> > > > > > b/arch/x86/include/asm/msr-index.h
> > > > > > index 6674bdb096f3..e5ea1c9f747b 100644
> > > > > > --- a/arch/x86/include/asm/msr-index.h
> > > > > > +++ b/arch/x86/include/asm/msr-index.h
> > > > > > @@ -569,6 +569,7 @@
> > > > > > #define MSR_AMD_CPPC_CAP2 0xc00102b2
> > > > > > #define MSR_AMD_CPPC_REQ 0xc00102b3
> > > > > > #define MSR_AMD_CPPC_STATUS 0xc00102b4
> > > > > > +#define MSR_AMD_CPPC_HW_CTL 0xc0010015
> > > > > >
> > > > > > #define AMD_CPPC_LOWEST_PERF(x) (((x) >> 0) & 0xff)
> > > > > > #define AMD_CPPC_LOWNONLIN_PERF(x) (((x) >> 8) & 0xff)
> > > > > > @@ -579,6 +580,8 @@
> > > > > > #define AMD_CPPC_MIN_PERF(x) (((x) & 0xff) << 8)
> > > > > > #define AMD_CPPC_DES_PERF(x) (((x) & 0xff) << 16)
> > > > > > #define AMD_CPPC_ENERGY_PERF_PREF(x) (((x) & 0xff) << 24)
> > > > > > +#define AMD_CPPC_PRECISION_BOOST_BIT 25
> > > > > > +#define AMD_CPPC_PRECISION_BOOST_ENABLED
> > > > > BIT_ULL(AMD_CPPC_PRECISION_BOOST_BIT)
> > > > >
> > > > > I had commented the MSR_AMD_CPPC_HW_CTL is duplicated with
> > > > > MSR_K7_HWCR
> > > > >
> > > > > https://lore.kernel.org/lkml/YtX+uF/[email protected]/
> > > > > https://lore.kernel.org/lkml/[email protected]/
> > > > >
> > > > > Could you please make sure address the commments?
> > > > >
> > > > > Thanks,
> > > > > Ray
> > > >
> > > > If I rename that the MSR definition string, that will cause lots of
> > > > driver file change.
> > > > So I suggest to add one new MSR macro for the CPPC, the MSR_K7_HWCR
> > > > is mismatching in the CPPC Pstate driver.
> > > > If you refuse to use this new one, I will reuse that old one.
> > >
> > > To avoid changing too much stuff at once how about if you give an alias?
> > > IE something like:
> > >
> > > #define MSR_AMD_CPPC_HW_CTL MSR_K7_HWCR
> > >
> >
> > The mainly concern is that HWCR is for legacy ACPI P-State control not for
> > CPPC. I talked with hardware guys before, it's not suggested to mix them up
> > together. This register has been defined for a long time even before Zen
> > processor.
> >
> > Thanks,
> > Ray
>
> I have removed the code not to write boost state to that MSR, just check the boost state from the MSR bit value.
> It will not cause any problems, I have tested and confirmed that the BIT value will be changed after BOOST ON/OFF switched in BIOS setting.
> So we can just check the boost state here for pstate driver notification.
>
If we found MSR_K7_HWCR would impact the max frequency in CPPC, we should
report a defect or issue to firmware team. (Then we can add a quirk
function to workaround this in amd-pstate)
Thanks,
Ray
[AMD Official Use Only - General]
Hi Ray.
> -----Original Message-----
> From: Huang, Ray <[email protected]>
> Sent: Tuesday, October 25, 2022 8:32 AM
> To: Yuan, Perry <[email protected]>
> Cc: Limonciello, Mario <[email protected]>;
> [email protected]; [email protected]; Sharma, Deepak
> <[email protected]>; Fontenot, Nathan
> <[email protected]>; Deucher, Alexander
> <[email protected]>; Huang, Shimmer
> <[email protected]>; Du, Xiaojian <[email protected]>; Meng,
> Li (Jassmine) <[email protected]>; [email protected]; linux-
> [email protected]; Borislav Petkov <[email protected]>
> Subject: Re: [RESEND PATCH V2 4/9] x86/msr: Add the MSR definition for
> AMD CPPC boost state
>
> On Mon, Oct 24, 2022 at 10:56:50AM +0800, Yuan, Perry wrote:
> > [AMD Official Use Only - General]
> >
> > Hi Ray.
> >
> > > -----Original Message-----
> > > From: Huang, Ray <[email protected]>
> > > Sent: Friday, October 21, 2022 1:22 PM
> > > To: Limonciello, Mario <[email protected]>
> > > Cc: Yuan, Perry <[email protected]>; [email protected];
> > > [email protected]; Sharma, Deepak <[email protected]>;
> > > Fontenot, Nathan <[email protected]>; Deucher, Alexander
> > > <[email protected]>; Huang, Shimmer
> <[email protected]>;
> > > Du, Xiaojian <[email protected]>; Meng, Li (Jassmine)
> > > <[email protected]>; [email protected]; linux-
> > > [email protected]; Borislav Petkov <[email protected]>
> > > Subject: Re: [RESEND PATCH V2 4/9] x86/msr: Add the MSR definition
> > > for AMD CPPC boost state
> > >
> > > + Boris,
> > >
> > > On Fri, Oct 21, 2022 at 12:05:21AM +0800, Limonciello, Mario wrote:
> > > > [Public]
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Yuan, Perry <[email protected]>
> > > > > Sent: Thursday, October 20, 2022 11:01
> > > > > To: Huang, Ray <[email protected]>
> > > > > Cc: [email protected]; [email protected]; Sharma,
> > > > > Deepak <[email protected]>; Limonciello, Mario
> > > > > <[email protected]>; Fontenot, Nathan
> > > > > <[email protected]>; Deucher, Alexander
> > > > > <[email protected]>; Huang, Shimmer
> > > <[email protected]>;
> > > > > Du, Xiaojian <[email protected]>; Meng, Li (Jassmine)
> > > > > <[email protected]>; [email protected]; linux-
> > > > > [email protected]
> > > > > Subject: RE: [RESEND PATCH V2 4/9] x86/msr: Add the MSR
> > > > > definition for AMD CPPC boost state
> > > > >
> > > > > [AMD Official Use Only - General]
> > > > >
> > > > > Hi Ray.
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Huang, Ray <[email protected]>
> > > > > > Sent: Monday, October 17, 2022 5:57 PM
> > > > > > To: Yuan, Perry <[email protected]>
> > > > > > Cc: [email protected]; [email protected];
> > > > > > Sharma, Deepak <[email protected]>; Limonciello, Mario
> > > > > > <[email protected]>; Fontenot, Nathan
> > > > > > <[email protected]>; Deucher, Alexander
> > > > > > <[email protected]>; Huang, Shimmer
> > > > > > <[email protected]>; Du, Xiaojian
> <[email protected]>;
> > > > > Meng,
> > > > > > Li (Jassmine) <[email protected]>; [email protected];
> > > > > > linux- [email protected]
> > > > > > Subject: Re: [RESEND PATCH V2 4/9] x86/msr: Add the MSR
> > > > > > definition for AMD CPPC boost state
> > > > > >
> > > > > > On Tue, Oct 11, 2022 at 12:22:43AM +0800, Yuan, Perry wrote:
> > > > > > > This MSR can be used to check whether the CPU frequency
> > > > > > > boost state is enabled in the hardware control. User can
> > > > > > > change the boost state in the BIOS setting,amd_pstate driver
> > > > > > > will update the boost state according to this msr value.
> > > > > > >
> > > > > > > AMD Processor Programming Reference (PPR)
> > > > > > > Link: https://www.amd.com/system/files/TechDocs/40332.pdf
> > > > > > > [p1095]
> > > > > > > Link: https://www.amd.com/system/files/TechDocs/56569-A1-
> > > PUB.zip
> > > > > > > [p162]
> > > > > > >
> > > > > > > Signed-off-by: Perry Yuan <[email protected]>
> > > > > > > ---
> > > > > > > arch/x86/include/asm/msr-index.h | 3 +++
> > > > > > > 1 file changed, 3 insertions(+)
> > > > > > >
> > > > > > > diff --git a/arch/x86/include/asm/msr-index.h
> > > > > > > b/arch/x86/include/asm/msr-index.h
> > > > > > > index 6674bdb096f3..e5ea1c9f747b 100644
> > > > > > > --- a/arch/x86/include/asm/msr-index.h
> > > > > > > +++ b/arch/x86/include/asm/msr-index.h
> > > > > > > @@ -569,6 +569,7 @@
> > > > > > > #define MSR_AMD_CPPC_CAP2 0xc00102b2
> > > > > > > #define MSR_AMD_CPPC_REQ 0xc00102b3
> > > > > > > #define MSR_AMD_CPPC_STATUS 0xc00102b4
> > > > > > > +#define MSR_AMD_CPPC_HW_CTL 0xc0010015
> > > > > > >
> > > > > > > #define AMD_CPPC_LOWEST_PERF(x) (((x) >> 0) &
> 0xff)
> > > > > > > #define AMD_CPPC_LOWNONLIN_PERF(x) (((x) >> 8) & 0xff)
> > > > > > > @@ -579,6 +580,8 @@
> > > > > > > #define AMD_CPPC_MIN_PERF(x) (((x) & 0xff) << 8)
> > > > > > > #define AMD_CPPC_DES_PERF(x) (((x) & 0xff) << 16)
> > > > > > > #define AMD_CPPC_ENERGY_PERF_PREF(x) (((x) & 0xff)
> << 24)
> > > > > > > +#define AMD_CPPC_PRECISION_BOOST_BIT 25
> > > > > > > +#define AMD_CPPC_PRECISION_BOOST_ENABLED
> > > > > > BIT_ULL(AMD_CPPC_PRECISION_BOOST_BIT)
> > > > > >
> > > > > > I had commented the MSR_AMD_CPPC_HW_CTL is duplicated with
> > > > > > MSR_K7_HWCR
> > > > > >
> > > > > > https://lore.kernel.org/lkml/YtX+uF/[email protected]/
> > > > > > https://lore.kernel.org/lkml/[email protected]/
> > > > > >
> > > > > > Could you please make sure address the commments?
> > > > > >
> > > > > > Thanks,
> > > > > > Ray
> > > > >
> > > > > If I rename that the MSR definition string, that will cause lots
> > > > > of driver file change.
> > > > > So I suggest to add one new MSR macro for the CPPC, the
> > > > > MSR_K7_HWCR is mismatching in the CPPC Pstate driver.
> > > > > If you refuse to use this new one, I will reuse that old one.
> > > >
> > > > To avoid changing too much stuff at once how about if you give an alias?
> > > > IE something like:
> > > >
> > > > #define MSR_AMD_CPPC_HW_CTL MSR_K7_HWCR
> > > >
> > >
> > > The mainly concern is that HWCR is for legacy ACPI P-State control
> > > not for CPPC. I talked with hardware guys before, it's not suggested
> > > to mix them up together. This register has been defined for a long
> > > time even before Zen processor.
> > >
> > > Thanks,
> > > Ray
> >
> > I have removed the code not to write boost state to that MSR, just check
> the boost state from the MSR bit value.
> > It will not cause any problems, I have tested and confirmed that the BIT
> value will be changed after BOOST ON/OFF switched in BIOS setting.
> > So we can just check the boost state here for pstate driver notification.
> >
>
> If we found MSR_K7_HWCR would impact the max frequency in CPPC, we
> should report a defect or issue to firmware team. (Then we can add a quirk
> function to workaround this in amd-pstate)
>
> Thanks,
> Ray
Sure , I will revise the patch and still using the MSR_K7_HWCR to check freq boost state in pstate driver.
Thanks for the feedback.
Perry.