[PATCH v2 0/5] cpufreq: intel_pstate: Address some HWP-related oddities

[PATCH v2 0/5] cpufreq: intel_pstate: Address some HWP-related oddities

[Rafael J. Wysocki]

Hi All,

The v2 is here to address feedback from Doug and one issue found by me.

The purpose of this series is to address some peculiarities related to
taking CPUs offline/online and switching between different operation
modes with HWP enabled that have become visible after allowing the
driver to work in the passive mode with HWP enabled in 5.9-rc1 (and
one that was there earlier, but can be addressed easily after the
changes madein 5.9-rc1).

Please refer to the patch changelogs for details.

For easier testing/review, the series is available from the git branch at:

 git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git \
 intel_pstate-testing

Thanks,
Rafael

[PATCH v2 1/5] cpufreq: intel_pstate: Refuse to turn off with HWP enabled

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

After commit f6ebbcf08f37 ("cpufreq: intel_pstate: Implement passive
mode with HWP enabled") it is possible to change the driver status
to "off" via sysfs with HWP enabled, which effectively causes the
driver to unregister itself, but HWP remains active and it forces the
minimum performance, so even if another cpufreq driver is loaded,
it will not be able to control the CPU frequency.

For this reason, make the driver refuse to change the status to
"off" with HWP enabled.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

-> v2: No changes.

---
 drivers/cpufreq/intel_pstate.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index e0220a6fbc69..bcda1e700a73 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -2716,9 +2716,15 @@ static int intel_pstate_update_status(const char *buf, size_t size)
 {
 	int ret;
 
-	if (size == 3 && !strncmp(buf, "off", size))
-		return intel_pstate_driver ?
-			intel_pstate_unregister_driver() : -EINVAL;
+	if (size == 3 && !strncmp(buf, "off", size)) {
+		if (!intel_pstate_driver)
+			return -EINVAL;
+
+		if (hwp_active)
+			return -EBUSY;
+
+		return intel_pstate_unregister_driver();
+	}
 
 	if (size == 6 && !strncmp(buf, "active", size)) {
 		if (intel_pstate_driver) {
-- 
2.26.2

[PATCH v2 2/5] cpufreq: intel_pstate: Always return last EPP value from sysfs

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

Make the energy_performance_preference policy attribute in sysfs
always return the last EPP value written to it instead of the one
currently in the HWP Request MSR to avoid possible confusion when
the performance scaling algorithm is used in the active mode with
HWP enabled (in which case the EPP is forced to 0 regardless of
what value it has been set to via sysfs).

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

-> v2: No changes.

---
 drivers/cpufreq/intel_pstate.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index bcda1e700a73..3d18934fa975 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -606,13 +606,10 @@ static const unsigned int epp_values[] = {
 
 static int intel_pstate_get_energy_pref_index(struct cpudata *cpu_data, int *raw_epp)
 {
-	s16 epp;
+	s16 epp = cpu_data->epp_cached;
 	int index = -EINVAL;
 
 	*raw_epp = 0;
-	epp = intel_pstate_get_epp(cpu_data, 0);
-	if (epp < 0)
-		return epp;
 
 	if (boot_cpu_has(X86_FEATURE_HWP_EPP)) {
 		if (epp == HWP_EPP_PERFORMANCE)
@@ -644,6 +641,8 @@ static int intel_pstate_get_energy_pref_index(struct cpudata *cpu_data, int *raw
 
 static int intel_pstate_set_epp(struct cpudata *cpu, u32 epp)
 {
+	int ret;
+
 	/*
 	 * Use the cached HWP Request MSR value, because in the active mode the
 	 * register itself may be updated by intel_pstate_hwp_boost_up() or
@@ -659,7 +658,11 @@ static int intel_pstate_set_epp(struct cpudata *cpu, u32 epp)
 	 * function, so it cannot run in parallel with the update below.
 	 */
 	WRITE_ONCE(cpu->hwp_req_cached, value);
-	return wrmsrl_on_cpu(cpu->cpu, MSR_HWP_REQUEST, value);
+	ret = wrmsrl_on_cpu(cpu->cpu, MSR_HWP_REQUEST, value);
+	if (!ret)
+		cpu->epp_cached = epp;
+
+	return ret;
 }
 
 static int intel_pstate_set_energy_pref_index(struct cpudata *cpu_data,
@@ -762,10 +765,8 @@ static ssize_t store_energy_performance_preference(
 			cpufreq_stop_governor(policy);
 			ret = intel_pstate_set_epp(cpu, epp);
 			err = cpufreq_start_governor(policy);
-			if (!ret) {
-				cpu->epp_cached = epp;
+			if (!ret)
 				ret = err;
-			}
 		}
 	}
 
@@ -2378,6 +2379,13 @@ static int intel_pstate_cpu_init(struct cpufreq_policy *policy)
 	 */
 	policy->policy = CPUFREQ_POLICY_POWERSAVE;
 
+
+	if (hwp_active) {
+		struct cpudata *cpu = all_cpu_data[policy->cpu];
+
+		cpu->epp_cached = intel_pstate_get_epp(cpu, 0);
+	}
+
 	return 0;
 }
 
@@ -2585,7 +2593,7 @@ static int intel_cpufreq_cpu_init(struct cpufreq_policy *policy)
 		policy->transition_delay_us = INTEL_CPUFREQ_TRANSITION_DELAY_HWP;
 		rdmsrl_on_cpu(cpu->cpu, MSR_HWP_REQUEST, &value);
 		WRITE_ONCE(cpu->hwp_req_cached, value);
-		cpu->epp_cached = (value & GENMASK_ULL(31, 24)) >> 24;
+		cpu->epp_cached = intel_pstate_get_epp(cpu, value);
 	} else {
 		turbo_max = cpu->pstate.turbo_pstate;
 		policy->transition_delay_us = INTEL_CPUFREQ_TRANSITION_DELAY;
-- 
2.26.2

[PATCH v2 3/5] cpufreq: intel_pstate: Add ->offline and ->online callbacks

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

Add ->offline and ->online driver callbacks to prepare for taking a
CPU offline and to restore its working configuration when it goes
back online, respectively, to avoid invoking the ->init callback on
every CPU online which is quite a bit of unnecessary overhead.

Define ->offline and ->online so that they can be used in the
passive mode as well as in the active mode and because ->offline
will do the majority of ->stop_cpu work, the passive mode does
not need that callback any more, so drop it.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

-> v2: Typo fixes and changelog edits (Doug).

---
 drivers/cpufreq/intel_pstate.c | 38 ++++++++++++++++++++++++++++------
 1 file changed, 32 insertions(+), 6 deletions(-)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 3d18934fa975..98836ac299db 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -2297,28 +2297,51 @@ static int intel_pstate_verify_policy(struct cpufreq_policy_data *policy)
 	return 0;
 }
 
-static void intel_cpufreq_stop_cpu(struct cpufreq_policy *policy)
+static int intel_pstate_cpu_offline(struct cpufreq_policy *policy)
 {
+	pr_debug("CPU %d going offline\n", policy->cpu);
+
+	intel_pstate_exit_perf_limits(policy);
+
+	/*
+	 * If the CPU is an SMT thread and it goes offline with the performance
+	 * settings different from the minimum, it will prevent its sibling
+	 * from getting to lower performance levels, so force the minimum
+	 * performance on CPU offline to prevent that from happening.
+	 */
 	if (hwp_active)
 		intel_pstate_hwp_force_min_perf(policy->cpu);
 	else
 		intel_pstate_set_min_pstate(all_cpu_data[policy->cpu]);
+
+	return 0;
+}
+
+static int intel_pstate_cpu_online(struct cpufreq_policy *policy)
+{
+	pr_debug("CPU %d going online\n", policy->cpu);
+
+	intel_pstate_init_acpi_perf_limits(policy);
+
+	if (hwp_active)
+		wrmsrl_on_cpu(policy->cpu, MSR_HWP_REQUEST,
+			      all_cpu_data[policy->cpu]->hwp_req_cached);
+
+	return 0;
 }
 
 static void intel_pstate_stop_cpu(struct cpufreq_policy *policy)
 {
-	pr_debug("CPU %d exiting\n", policy->cpu);
+	pr_debug("CPU %d stopping\n", policy->cpu);
 
 	intel_pstate_clear_update_util_hook(policy->cpu);
 	if (hwp_active)
 		intel_pstate_hwp_save_state(policy);
-
-	intel_cpufreq_stop_cpu(policy);
 }
 
 static int intel_pstate_cpu_exit(struct cpufreq_policy *policy)
 {
-	intel_pstate_exit_perf_limits(policy);
+	pr_debug("CPU %d exiting\n", policy->cpu);
 
 	policy->fast_switch_possible = false;
 
@@ -2398,6 +2421,8 @@ static struct cpufreq_driver intel_pstate = {
 	.init		= intel_pstate_cpu_init,
 	.exit		= intel_pstate_cpu_exit,
 	.stop_cpu	= intel_pstate_stop_cpu,
+	.offline	= intel_pstate_cpu_offline,
+	.online		= intel_pstate_cpu_online,
 	.update_limits	= intel_pstate_update_limits,
 	.name		= "intel_pstate",
 };
@@ -2652,7 +2677,8 @@ static struct cpufreq_driver intel_cpufreq = {
 	.fast_switch	= intel_cpufreq_fast_switch,
 	.init		= intel_cpufreq_cpu_init,
 	.exit		= intel_cpufreq_cpu_exit,
-	.stop_cpu	= intel_cpufreq_stop_cpu,
+	.offline	= intel_pstate_cpu_offline,
+	.online		= intel_pstate_cpu_online,
 	.update_limits	= intel_pstate_update_limits,
 	.name		= "intel_cpufreq",
 };
-- 
2.26.2

[PATCH v2 4/5] cpufreq: intel_pstate: Free memory only when turning off

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

When intel_pstate switches the operation mode from "active" to
"passive" or the other way around, freeing its data structures
representing CPUs and allocating them again from scratch is not
necessary and wasteful.  Moreover, if these data structures are
preserved, the cached HWP Request MSR value from there may be
written to the MSR to start with to reinitialize it and help to
restore the EPP value set previously (it is set to 0xFF when CPUs
go offline to allow their SMT siblings to use the full range of
EPP values and that also happens when the driver gets unregistered).

Accordingly, modify the driver to only do a full cleanup on driver
object registration errors and when its status is changed to "off"
via sysfs and to write the cached HWP Request MSR value back to
the MSR on CPU init if the data structure representing the given
CPU is still there.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

-> v2: Rearrange intel_pstate_init_cpu() to restore some of the previous
       behavior of it to retain the current active-mode EPP management.

---
 drivers/cpufreq/intel_pstate.c | 54 +++++++++++++---------------------
 1 file changed, 21 insertions(+), 33 deletions(-)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 98836ac299db..37731d45f0ea 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -2098,25 +2098,28 @@ static int intel_pstate_init_cpu(unsigned int cpunum)
 
 		all_cpu_data[cpunum] = cpu;
 
-		cpu->epp_default = -EINVAL;
-		cpu->epp_powersave = -EINVAL;
-		cpu->epp_saved = -EINVAL;
-	}
+		cpu->cpu = cpunum;
 
-	cpu = all_cpu_data[cpunum];
-
-	cpu->cpu = cpunum;
+		cpu->epp_default = -EINVAL;
 
-	if (hwp_active) {
-		const struct x86_cpu_id *id;
+		if (hwp_active) {
+			const struct x86_cpu_id *id;
 
-		intel_pstate_hwp_enable(cpu);
+			intel_pstate_hwp_enable(cpu);
 
-		id = x86_match_cpu(intel_pstate_hwp_boost_ids);
-		if (id && intel_pstate_acpi_pm_profile_server())
-			hwp_boost = true;
+			id = x86_match_cpu(intel_pstate_hwp_boost_ids);
+			if (id && intel_pstate_acpi_pm_profile_server())
+				hwp_boost = true;
+		}
+	} else if (hwp_active) {
+		cpu->epp_policy = 0;
+		wrmsrl_on_cpu(cpunum, MSR_HWP_REQUEST,
+			      cpu->hwp_req_cached);
 	}
 
+	cpu->epp_powersave = -EINVAL;
+	cpu->epp_saved = -EINVAL;
+
 	intel_pstate_get_cpu_pstates(cpu);
 
 	pr_debug("controlling: cpu %d\n", cpunum);
@@ -2701,9 +2704,6 @@ static void intel_pstate_driver_cleanup(void)
 	}
 	put_online_cpus();
 
-	if (intel_pstate_driver == &intel_pstate)
-		intel_pstate_sysfs_hide_hwp_dynamic_boost();
-
 	intel_pstate_driver = NULL;
 }
 
@@ -2729,14 +2729,6 @@ static int intel_pstate_register_driver(struct cpufreq_driver *driver)
 	return 0;
 }
 
-static int intel_pstate_unregister_driver(void)
-{
-	cpufreq_unregister_driver(intel_pstate_driver);
-	intel_pstate_driver_cleanup();
-
-	return 0;
-}
-
 static ssize_t intel_pstate_show_status(char *buf)
 {
 	if (!intel_pstate_driver)
@@ -2748,8 +2740,6 @@ static ssize_t intel_pstate_show_status(char *buf)
 
 static int intel_pstate_update_status(const char *buf, size_t size)
 {
-	int ret;
-
 	if (size == 3 && !strncmp(buf, "off", size)) {
 		if (!intel_pstate_driver)
 			return -EINVAL;
@@ -2757,7 +2747,8 @@ static int intel_pstate_update_status(const char *buf, size_t size)
 		if (hwp_active)
 			return -EBUSY;
 
-		return intel_pstate_unregister_driver();
+		cpufreq_unregister_driver(intel_pstate_driver);
+		intel_pstate_driver_cleanup();
 	}
 
 	if (size == 6 && !strncmp(buf, "active", size)) {
@@ -2765,9 +2756,7 @@ static int intel_pstate_update_status(const char *buf, size_t size)
 			if (intel_pstate_driver == &intel_pstate)
 				return 0;
 
-			ret = intel_pstate_unregister_driver();
-			if (ret)
-				return ret;
+			cpufreq_unregister_driver(intel_pstate_driver);
 		}
 
 		return intel_pstate_register_driver(&intel_pstate);
@@ -2778,9 +2767,8 @@ static int intel_pstate_update_status(const char *buf, size_t size)
 			if (intel_pstate_driver == &intel_cpufreq)
 				return 0;
 
-			ret = intel_pstate_unregister_driver();
-			if (ret)
-				return ret;
+			cpufreq_unregister_driver(intel_pstate_driver);
+			intel_pstate_sysfs_hide_hwp_dynamic_boost();
 		}
 
 		return intel_pstate_register_driver(&intel_cpufreq);
-- 
2.26.2

[PATCH v2 5/5] cpufreq: intel_pstate: Restore cached EPP value during offline

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

Because hwp_req_cached contains the effective EPP value (0) when the
"performance" scaling algorithm is used in the active mode, replace
it with the cached EPP value during CPU offline to prevent it from
being used (unexpectedly) after switching over from the active mode
to the passive mode.

Also rename intel_pstate_hwp_force_min_perf() because it will do more
than just forcing the minimum performance now.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

-> v2: New patch.

---
 drivers/cpufreq/intel_pstate.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 37731d45f0ea..61d7179bccdd 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -904,14 +904,25 @@ static void intel_pstate_hwp_set(unsigned int cpu)
 	wrmsrl_on_cpu(cpu, MSR_HWP_REQUEST, value);
 }
 
-static void intel_pstate_hwp_force_min_perf(int cpu)
+static void intel_pstate_hwp_offline(int cpu)
 {
-	u64 value;
+	struct cpudata *cpudata = all_cpu_data[cpu];
+	u64 value = READ_ONCE(cpudata->hwp_req_cached);
 	int min_perf;
 
-	value = all_cpu_data[cpu]->hwp_req_cached;
+	if (boot_cpu_has(X86_FEATURE_HWP_EPP)) {
+		/*
+		 * In case the EPP has been set to "performance" by the
+		 * active mode "performance" scaling algorithm, replace that
+		 * temporary value with the cached EPP one.
+		 */
+		value &= ~GENMASK_ULL(31, 24);
+		value |= HWP_ENERGY_PERF_PREFERENCE(cpudata->epp_cached);
+		WRITE_ONCE(cpudata->hwp_req_cached, value);
+	}
+
 	value &= ~GENMASK_ULL(31, 0);
-	min_perf = HWP_LOWEST_PERF(all_cpu_data[cpu]->hwp_cap_cached);
+	min_perf = HWP_LOWEST_PERF(cpudata->hwp_cap_cached);
 
 	/* Set hwp_max = hwp_min */
 	value |= HWP_MAX_PERF(min_perf);
@@ -2313,7 +2324,7 @@ static int intel_pstate_cpu_offline(struct cpufreq_policy *policy)
 	 * performance on CPU offline to prevent that from happening.
 	 */
 	if (hwp_active)
-		intel_pstate_hwp_force_min_perf(policy->cpu);
+		intel_pstate_hwp_offline(policy->cpu);
 	else
 		intel_pstate_set_min_pstate(all_cpu_data[policy->cpu]);
 
-- 
2.26.2

Re: [PATCH v2 0/5] cpufreq: intel_pstate: Address some HWP-related oddities

[Srinivas Pandruvada]

[-- Attachment #1: Type: text/plain, Size: 3955 bytes --]

On Mon, 2020-08-24 at 19:39 +0200, Rafael J. Wysocki wrote:
> Hi All,
> 
> The v2 is here to address feedback from Doug and one issue found by
> me.
> 
> The purpose of this series is to address some peculiarities related
> to
> taking CPUs offline/online and switching between different operation
> modes with HWP enabled that have become visible after allowing the
> driver to work in the passive mode with HWP enabled in 5.9-rc1 (and
> one that was there earlier, but can be addressed easily after the
> changes madein 5.9-rc1).
> 
> Please refer to the patch changelogs for details.
> 
> For easier testing/review, the series is available from the git
> branch at:
> 
>  git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git \
>  intel_pstate-testing
> 

Applied these patches to 5.9-rc2

- After s3  limits got messed up.
 # cat /sys/power/mem_sleep 
s2idle [deep]

- In the dmesg unchecked MSR for HWP register

1.
Before test

sudo rdmsr -a 0x774
80002b0c
80002b0c
80002d0c
80002b0c
80002b0c
80002b0c
80002b0c
80002b0c
80002d0c
80002b0c
80002b0c
80002b0c
80002d0c
80002b0c
80002b0c
80002b0c
80002b0c
80002b0c
80002d0d
7f002b0c



cd /sys/devices/system/cpu/intel_pstate/
[root@otcpl-perf-test-skx-i9 intel_pstate]# grep . *
hwp_dynamic_boost:0
max_perf_pct:100
min_perf_pct:27
no_turbo:0
num_pstates:32
status:active
turbo_pct:32

cd ../cpu1/cpufreq/
[root@otcpl-perf-test-skx-i9 cpufreq]# grep . *
affected_cpus:1
base_frequency:3300000
cpuinfo_max_freq:4300000
cpuinfo_min_freq:1200000
cpuinfo_transition_latency:0
energy_performance_available_preferences:default performance
balance_performance balance_power power 
energy_performance_preference:balance_performance
related_cpus:1
scaling_available_governors:performance powersave
scaling_cur_freq:1200000
scaling_driver:intel_pstate
scaling_governor:powersave
scaling_max_freq:4300000
scaling_min_freq:1200000
scaling_setspeed:<unsupported>


2. Now change the EPP

# echo 127 > energy_performance_preference
sudo rdmsr -a 0x774
80002b0c
80002b0c
80002d0c
80002b0c
80002b0c
80002b0c
80002b0c
80002b0c
80002d0c
80002b0c
80002b0c
80002b0c
80002d0c
80002b0c
80002b0c
80002b0c
80002b0c
80002b0c
80002d0d

Good here

3. Offline/online good

[root@otcpl-perf-test-skx-i9 cpufreq]# echo 0 >
/sys/devices/system/cpu/cpu1/online 
[root@otcpl-perf-test-skx-i9 cpufreq]# echo ` >
/sys/devices/system/cpu/cpu1/online 
> echo ` > /sys/devices/system/cpu/cpu1/online ^C
[root@otcpl-perf-test-skx-i9 cpufreq]# echo 1 >
/sys/devices/system/cpu/cpu1/online 

 sudo rdmsr -a 0x774
80002b0c
80002b0c
80002d0c
80002b0c
80002b0c
80002b0c
80002b0c
80002b0c
80002d0c
80002b0c
80002b0c
80002b0c
80002d0c
80002b0c
80002b0c
80002b0c
80002b0c
80002b0c
80002d0d
7f002b0c

Good. Online restored the setting

4. Now S3

rtcwake -m mem -s 10

All limits are now messed up

sudo rdmsr -a 0x774
80002b0c
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00

5. Now switch to passive
Again bad, some CPU max/min is 0

sudo rdmsr -a 0x774
80002b0d
7f002b0f
80002b0c
80002d0e
80002b0c
80002b0d
80002b0f
80002b2b
80002b0c
80002d1d
80000000
80002b0c
80002b0c
80000000
80000000
80000000
80000000
80000000
80000000
80000000

6.
Switched back to active to restore back

Lost EPP setting but rest are good.

sudo rdmsr -a 0x774
80002b0c
7f002b0c
80002b0c
80002d0c
80002b0c
80002b0c
80002b0c
80002b0c
80002b0c
80002d0c
80002b0c
80002b0c
80002b0c
80002d0c
80002b0c
80002b0c
80002b0c
80002b0c
80002b0c
80002d0d

7. S3 again

rtcwake -m mem -s 10

Again messed up

 sudo rdmsr -a 0x774
80002b0c
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00

This time dmesg didn't have unchecked MSR in dmesg (I think because I
didn't change EPP before)

Thanks,
Srinivas

> Thanks,
> Rafael
> 
> 
> 

[-- Attachment #2: dmesg.txt --]
[-- Type: text/plain, Size: 15375 bytes --]

[  416.434555] printk: Suspending console(s) (use no_console_suspend to debug)
[  416.436700] e1000e: EEE TX LPI TIMER: 00000011
[  416.442714] sd 1:0:0:0: [sda] Synchronizing SCSI cache
[  416.444511] sd 6:0:0:0: [sdb] Synchronizing SCSI cache
[  416.444606] sd 6:0:0:0: [sdb] Stopping disk
[  416.625785] sd 1:0:0:0: [sda] Stopping disk
[  416.988913] PM: suspend devices took 0.554 seconds
[  417.017162] ACPI: Preparing to enter system sleep state S3
[  417.017813] PM: Saving platform NVS memory
[  417.018033] Disabling non-boot CPUs ...
[  417.018765] intel_pstate: CPU 1 stopping
[  417.018768] intel_pstate: CPU 1 going offline
[  417.020144] smpboot: CPU 1 is now offline
[  417.022401] intel_pstate: CPU 2 stopping
[  417.022405] intel_pstate: CPU 2 going offline
[  417.023750] smpboot: CPU 2 is now offline
[  417.025089] intel_pstate: CPU 3 stopping
[  417.025093] intel_pstate: CPU 3 going offline
[  417.026481] smpboot: CPU 3 is now offline
[  417.028685] intel_pstate: CPU 4 stopping
[  417.028689] intel_pstate: CPU 4 going offline
[  417.030023] smpboot: CPU 4 is now offline
[  417.032171] intel_pstate: CPU 5 stopping
[  417.032176] intel_pstate: CPU 5 going offline
[  417.033499] smpboot: CPU 5 is now offline
[  417.034809] intel_pstate: CPU 6 stopping
[  417.034814] intel_pstate: CPU 6 going offline
[  417.036235] smpboot: CPU 6 is now offline
[  417.038330] intel_pstate: CPU 7 stopping
[  417.038334] intel_pstate: CPU 7 going offline
[  417.039679] smpboot: CPU 7 is now offline
[  417.041750] intel_pstate: CPU 8 stopping
[  417.041754] intel_pstate: CPU 8 going offline
[  417.044789] smpboot: CPU 8 is now offline
[  417.045953] intel_pstate: CPU 9 stopping
[  417.045956] intel_pstate: CPU 9 going offline
[  417.047253] smpboot: CPU 9 is now offline
[  417.049294] intel_pstate: CPU 10 stopping
[  417.049298] intel_pstate: CPU 10 going offline
[  417.050623] smpboot: CPU 10 is now offline
[  417.053509] intel_pstate: CPU 11 stopping
[  417.053513] intel_pstate: CPU 11 going offline
[  417.054814] smpboot: CPU 11 is now offline
[  417.055746] intel_pstate: CPU 12 stopping
[  417.055750] intel_pstate: CPU 12 going offline
[  417.057095] smpboot: CPU 12 is now offline
[  417.058158] intel_pstate: CPU 13 stopping
[  417.058162] intel_pstate: CPU 13 going offline
[  417.059500] smpboot: CPU 13 is now offline
[  417.060524] intel_pstate: CPU 14 stopping
[  417.060528] intel_pstate: CPU 14 going offline
[  417.061897] smpboot: CPU 14 is now offline
[  417.063000] intel_pstate: CPU 15 stopping
[  417.063003] intel_pstate: CPU 15 going offline
[  417.064369] smpboot: CPU 15 is now offline
[  417.065368] intel_pstate: CPU 16 stopping
[  417.065371] intel_pstate: CPU 16 going offline
[  417.066663] smpboot: CPU 16 is now offline
[  417.067633] intel_pstate: CPU 17 stopping
[  417.067636] intel_pstate: CPU 17 going offline
[  417.068992] smpboot: CPU 17 is now offline
[  417.069995] intel_pstate: CPU 18 stopping
[  417.069999] intel_pstate: CPU 18 going offline
[  417.071347] smpboot: CPU 18 is now offline
[  417.072377] intel_pstate: CPU 19 stopping
[  417.072380] intel_pstate: CPU 19 going offline
[  417.073757] smpboot: CPU 19 is now offline
[18430761567.317871] [Firmware Bug]: TSC ADJUST differs: CPU0 0 --> -3998613504. Restoring
[  417.076929] x86/cpu: VMX (outside TXT) disabled by BIOS

  417.076930] ACPI: Low-level resume complete
[  417.076990] PM: Restoring platform NVS memory
[  417.081736] smpboot: Scheduler frequency invariance went wobbly, disabling!
[  417.081782] Enabling non-boot CPUs ...
[  417.081823] x86: Booting SMP configuration:
[  417.081823] smpboot: Booting Node 0 Processor 1 APIC 0x2
[  417.085503] intel_pstate: CPU 1 going online
[  417.085513] unchecked MSR access error: WRMSR to 0x774 (tried to write 0x000000007f002b0c) at rIP: 0xffffffffa206e334 (native_write_msr+0x4/0x20)
[  417.085513] Call Trace:
[  417.085517]  __wrmsr_on_cpu+0x33/0x40
[  417.085520]  generic_exec_single+0x59/0x90
[  417.085523]  smp_call_function_single+0xde/0x120
[  417.085523]  ? __rdmsr_on_cpu+0x40/0x40
[  417.085526]  ? intel_pstate_init_acpi_perf_limits.isra.10+0x114/0x220
[  417.085527]  wrmsrl_on_cpu+0x58/0x80
[  417.085528]  intel_pstate_cpu_online+0x63/0x70
[  417.085530]  cpufreq_online+0x94/0x8a0
[  417.085532]  ? cpufreq_online+0x8a0/0x8a0
[  417.085533]  cpuhp_cpufreq_online+0xa/0x10
[  417.085535]  cpuhp_invoke_callback+0x94/0x540
[  417.085537]  ? padata_free_shell+0x80/0x80
[  417.085539]  ? sort_range+0x20/0x20
[  417.085540]  cpuhp_thread_fun+0xb0/0x110
[  417.085541]  smpboot_thread_fn+0xc5/0x160
[  417.085543]  kthread+0x116/0x130
[  417.085545]  ? kthread_park+0x80/0x80
[  417.085547]  ret_from_fork+0x22/0x30
[  417.085550] intel_pstate: set_policy cpuinfo.max 4300000 policy->max 4300000
[  417.085552] unchecked MSR access error: RDMSR from 0x771 at rIP: 0xffffffffa206e253 (native_read_msr+0x3/0x30)
[  417.085552] Call Trace:
[  417.085553]  __rdmsr_on_cpu+0x2f/0x40
[  417.085554]  generic_exec_single+0x59/0x90
[  417.085555]  smp_call_function_single+0xde/0x120
[  417.085556]  ? pldmfw_flash_image+0xb30/0xb30
[  417.085556]  rdmsrl_on_cpu+0x57/0x80
[  417.085557]  intel_pstate_get_hwp_max+0x34/0x90
[  417.085558]  intel_pstate_update_perf_limits+0xe3/0x1f0
[  417.085559]  intel_pstate_set_policy+0x4c/0x175
[  417.085561]  cpufreq_set_policy+0x11c/0x350
[  417.085562]  cpufreq_online+0x7e8/0x8a0
[  417.085563]  ? cpufreq_online+0x8a0/0x8a0
[  417.085564]  cpuhp_cpufreq_online+0xa/0x10
[  417.085565]  cpuhp_invoke_callback+0x94/0x540
[  417.085566]  ? padata_free_shell+0x80/0x80
[  417.085567]  ? sort_range+0x20/0x20
[  417.085567]  cpuhp_thread_fun+0xb0/0x110
[  417.085568]  smpboot_thread_fn+0xc5/0x160
[  417.085569]  kthread+0x116/0x130
[  417.085570]  ? kthread_park+0x80/0x80
[  417.085571]  ret_from_fork+0x22/0x30
[  417.085572] intel_pstate: cpu:1 max_state 0 min_policy_perf:0 max_policy_perf:0
[  417.085573] intel_pstate: cpu:1 global_min:0 global_max:0
[  417.085573] intel_pstate: cpu:1 max_perf_ratio:0 min_perf_ratio:0
[  417.085699] CPU1 is up
[  417.085723] smpboot: Booting Node 0 Processor 2 APIC 0x4
[  417.089401] intel_pstate: CPU 2 going online
[  417.089409] intel_pstate: set_policy cpuinfo.max 4300000 policy->max 4300000
[  417.089411] intel_pstate: cpu:2 max_state 0 min_policy_perf:0 max_policy_perf:0
[  417.089411] intel_pstate: cpu:2 global_min:0 global_max:0
[  417.089412] intel_pstate: cpu:2 max_perf_ratio:0 min_perf_ratio:0
[  417.089522] CPU2 is up
[  417.089547] smpboot: Booting Node 0 Processor 3 APIC 0x6
[  417.093177] intel_pstate: CPU 3 going online
[  417.093184] intel_pstate: set_policy cpuinfo.max 4500000 policy->max 4500000
[  417.093186] intel_pstate: cpu:3 max_state 0 min_policy_perf:0 max_policy_perf:0
[  417.093186] intel_pstate: cpu:3 global_min:0 global_max:0
[  417.093187] intel_pstate: cpu:3 max_perf_ratio:0 min_perf_ratio:0
[  417.093310] CPU3 is up
[  417.093334] smpboot: Booting Node 0 Processor 4 APIC 0x8
[  417.097093] intel_pstate: CPU 4 going online
[  417.097101] intel_pstate: set_policy cpuinfo.max 4300000 policy->max 4300000
[  417.097102] intel_pstate: cpu:4 max_state 0 min_policy_perf:0 max_policy_perf:0
[  417.097103] intel_pstate: cpu:4 global_min:0 global_max:0
[  417.097103] intel_pstate: cpu:4 max_perf_ratio:0 min_perf_ratio:0
[  417.097234] CPU4 is up
[  417.097263] smpboot: Booting Node 0 Processor 5 APIC 0x10
[  417.101005] intel_pstate: CPU 5 going online
[  417.101014] intel_pstate: set_policy cpuinfo.max 4300000 policy->max 4300000
[  417.101015] intel_pstate: cpu:5 max_state 0 min_policy_perf:0 max_policy_perf:0
[  417.101016] intel_pstate: cpu:5 global_min:0 global_max:0
[  417.101016] intel_pstate: cpu:5 max_perf_ratio:0 min_perf_ratio:0
[  417.101146] CPU5 is up
[  417.101171] smpboot: Booting Node 0 Processor 6 APIC 0x12
[  417.104799] intel_pstate: CPU 6 going online
[  417.104806] intel_pstate: set_policy cpuinfo.max 4300000 policy->max 4300000
[  417.104808] intel_pstate: cpu:6 max_state 0 min_policy_perf:0 max_policy_perf:0
[  417.104809] intel_pstate: cpu:6 global_min:0 global_max:0
[  417.104809] intel_pstate: cpu:6 max_perf_ratio:0 min_perf_ratio:0
[  417.104952] CPU6 is up
[  417.104975] smpboot: Booting Node 0 Processor 7 APIC 0x14
[  417.108606] intel_pstate: CPU 7 going online
[  417.108615] intel_pstate: set_policy cpuinfo.max 4300000 policy->max 4300000
[  417.108616] intel_pstate: cpu:7 max_state 0 min_policy_perf:0 max_policy_perf:0
[  417.108617] intel_pstate: cpu:7 global_min:0 global_max:0
[  417.108617] intel_pstate: cpu:7 max_perf_ratio:0 min_perf_ratio:0
[  417.108760] CPU7 is up
[  417.108784] smpboot: Booting Node 0 Processor 8 APIC 0x16
[  417.112458] intel_pstate: CPU 8 going online
[  417.112465] intel_pstate: set_policy cpuinfo.max 4300000 policy->max 4300000
[  417.112467] intel_pstate: cpu:8 max_state 0 min_policy_perf:0 max_policy_perf:0
[  417.112467] intel_pstate: cpu:8 global_min:0 global_max:0
[  417.112468] intel_pstate: cpu:8 max_perf_ratio:0 min_perf_ratio:0
[  417.112620] CPU8 is up
[  417.112643] smpboot: Booting Node 0 Processor 9 APIC 0x18
[  417.116398] intel_pstate: CPU 9 going online
[  417.116406] intel_pstate: set_policy cpuinfo.max 4500000 policy->max 4500000
[  417.116407] intel_pstate: cpu:9 max_state 0 min_policy_perf:0 max_policy_perf:0
[  417.116408] intel_pstate: cpu:9 global_min:0 global_max:0
[  417.116408] intel_pstate: cpu:9 max_perf_ratio:0 min_perf_ratio:0
[  417.116572] CPU9 is up
[  417.116596] smpboot: Booting Node 0 Processor 10 APIC 0x1
[  417.117710] intel_pstate: CPU 10 going online
[  417.117717] intel_pstate: set_policy cpuinfo.max 4300000 policy->max 4300000
[  417.117719] intel_pstate: cpu:10 max_state 0 min_policy_perf:0 max_policy_perf:0
[  417.117719] intel_pstate: cpu:10 global_min:0 global_max:0
[  417.117720] intel_pstate: cpu:10 max_perf_ratio:0 min_perf_ratio:0
[  417.117951] CPU10 is up
[  417.117976] smpboot: Booting Node 0 Processor 11 APIC 0x3
[  417.119177] intel_pstate: CPU 11 going online
[  417.119184] intel_pstate: set_policy cpuinfo.max 4300000 policy->max 4300000
[  417.119186] intel_pstate: cpu:11 max_state 0 min_policy_perf:0 max_policy_perf:0
[  417.119186] intel_pstate: cpu:11 global_min:0 global_max:0
[  417.119187] intel_pstate: cpu:11 max_perf_ratio:0 min_perf_ratio:0
[  417.119383] CPU11 is up
[  417.119407] smpboot: Booting Node 0 Processor 12 APIC 0x5
[  417.120603] intel_pstate: CPU 12 going online
[  417.120610] intel_pstate: set_policy cpuinfo.max 4300000 policy->max 4300000
[  417.120612] intel_pstate: cpu:12 max_state 0 min_policy_perf:0 max_policy_perf:0
[  417.120613] intel_pstate: cpu:12 global_min:0 global_max:0
[  417.120613] intel_pstate: cpu:12 max_perf_ratio:0 min_perf_ratio:0
[  417.120796] CPU12 is up
[  417.120820] smpboot: Booting Node 0 Processor 13 APIC 0x7
[  417.121939] intel_pstate: CPU 13 going online
[  417.121946] intel_pstate: set_policy cpuinfo.max 4500000 policy->max 4500000
[  417.121948] intel_pstate: cpu:13 max_state 0 min_policy_perf:0 max_policy_perf:0
[  417.121949] intel_pstate: cpu:13 global_min:0 global_max:0
[  417.121949] intel_pstate: cpu:13 max_perf_ratio:0 min_perf_ratio:0
[  417.122145] CPU13 is up
[  417.122172] smpboot: Booting Node 0 Processor 14 APIC 0x9
[  417.123417] intel_pstate: CPU 14 going online
[  417.123424] intel_pstate: set_policy cpuinfo.max 4300000 policy->max 4300000
[  417.123426] intel_pstate: cpu:14 max_state 0 min_policy_perf:0 max_policy_perf:0
[  417.123426] intel_pstate: cpu:14 global_min:0 global_max:0
[  417.123427] intel_pstate: cpu:14 max_perf_ratio:0 min_perf_ratio:0
[  417.123625] CPU14 is up
[  417.123649] smpboot: Booting Node 0 Processor 15 APIC 0x11
[  417.124883] intel_pstate: CPU 15 going online
[  417.124890] intel_pstate: set_policy cpuinfo.max 4300000 policy->max 4300000
[  417.124892] intel_pstate: cpu:15 max_state 0 min_policy_perf:0 max_policy_perf:0
[  417.124893] intel_pstate: cpu:15 global_min:0 global_max:0
[  417.124893] intel_pstate: cpu:15 max_perf_ratio:0 min_perf_ratio:0
[  417.125108] CPU15 is up
[  417.125132] smpboot: Booting Node 0 Processor 16 APIC 0x13
[  417.126320] intel_pstate: CPU 16 going online
[  417.126327] intel_pstate: set_policy cpuinfo.max 4300000 policy->max 4300000
[  417.126329] intel_pstate: cpu:16 max_state 0 min_policy_perf:0 max_policy_perf:0
[  417.126329] intel_pstate: cpu:16 global_min:0 global_max:0
[  417.126330] intel_pstate: cpu:16 max_perf_ratio:0 min_perf_ratio:0
[  417.126551] CPU16 is up
[  417.126575] smpboot: Booting Node 0 Processor 17 APIC 0x15
[  417.127658] intel_pstate: CPU 17 going online
[  417.127665] intel_pstate: set_policy cpuinfo.max 4300000 policy->max 4300000
[  417.127667] intel_pstate: cpu:17 max_state 0 min_policy_perf:0 max_policy_perf:0
[  417.127668] intel_pstate: cpu:17 global_min:0 global_max:0
[  417.127668] intel_pstate: cpu:17 max_perf_ratio:0 min_perf_ratio:0
[  417.127917] CPU17 is up
[  417.127940] smpboot: Booting Node 0 Processor 18 APIC 0x17
[  417.129146] intel_pstate: CPU 18 going online
[  417.129153] intel_pstate: set_policy cpuinfo.max 4300000 policy->max 4300000
[  417.129155] intel_pstate: cpu:18 max_state 0 min_policy_perf:0 max_policy_perf:0
[  417.129155] intel_pstate: cpu:18 global_min:0 global_max:0
[  417.129156] intel_pstate: cpu:18 max_perf_ratio:0 min_perf_ratio:0
[  417.129401] CPU18 is up
[  417.129431] smpboot: Booting Node 0 Processor 19 APIC 0x19
[  417.130681] intel_pstate: CPU 19 going online
[  417.130689] intel_pstate: set_policy cpuinfo.max 4500000 policy->max 4500000
[  417.130691] intel_pstate: cpu:19 max_state 0 min_policy_perf:0 max_policy_perf:0
[  417.130691] intel_pstate: cpu:19 global_min:0 global_max:0
[  417.130692] intel_pstate: cpu:19 max_perf_ratio:0 min_perf_ratio:0
[  417.130935] CPU19 is up
[  417.139360] ACPI: Waking up from system sleep state S3
[  417.176979] pci 0000:16:05.0: disabled boot interrupts on device [8086:2034]
[  417.177008] pci 0000:64:05.0: disabled boot interrupts on device [8086:2034]
[  417.177022] pci 0000:b2:05.0: disabled boot interrupts on device [8086:2034]
[  417.177378] usb usb1: root hub lost power or was reset
[  417.177379] usb usb2: root hub lost power or was reset
[  417.187092] sd 1:0:0:0: [sda] Starting disk
[  417.187147] sd 6:0:0:0: [sdb] Starting disk
[  417.516343] usb 1-2: reset high-speed USB device number 2 using xhci_hcd
[  417.519960] ata3: SATA link down (SStatus 4 SControl 300)
[  417.519974] ata8: SATA link down (SStatus 4 SControl 300)
[  417.520359] ata5: SATA link down (SStatus 4 SControl 300)
[  417.520441] ata6: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
[  417.520616] ata7: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
[  417.520675] ata2: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
[  417.520698] ata4: SATA link down (SStatus 4 SControl 300)
[  417.520712] ata1: SATA link down (SStatus 4 SControl 300)
[  417.521676] ata7.00: configured for UDMA/133
[  417.521709] ata7.00: Enabling discard_zeroes_data
[  417.524724] ata6.00: configured for UDMA/133
[  417.561597] ata2.00: configured for UDMA/133
[  418.982436] usb 1-2.3: reset high-speed USB device number 3 using xhci_hcd
[  419.072372] PM: resume devices took 1.896 seconds
[  419.072386] OOM killer enabled.
[  419.072386] Restarting tasks ... done.
[  419.072802] PM: suspend exit

RE: [PATCH v2 0/5] cpufreq: intel_pstate: Address some HWP-related oddities

[Doug Smythies]

Hi Srinivas,

I think there is a disconnect between your written
description of what is going on and your supporting MSR reads.

On 2020.08.24 16:56 Srinivas Pandruvada wrote:
> On Mon, 2020-08-24 at 19:39 +0200, Rafael J. Wysocki wrote:
> > Hi All,
> >
> > The v2 is here to address feedback from Doug and one issue found by
> > me.
> >
> > The purpose of this series is to address some peculiarities related
> > to
> > taking CPUs offline/online and switching between different operation
> > modes with HWP enabled that have become visible after allowing the
> > driver to work in the passive mode with HWP enabled in 5.9-rc1 (and
> > one that was there earlier, but can be addressed easily after the
> > changes madein 5.9-rc1).
> >
> > Please refer to the patch changelogs for details.
> >
> > For easier testing/review, the series is available from the git
> > branch at:
> >
> >  git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git \
> >  intel_pstate-testing
> >
> 
> Applied these patches to 5.9-rc2

So did I, and the issues I reported the other day are fine now.
I did try a few of the things you were doing.

> 
> - After s3  limits got messed up.
>  # cat /sys/power/mem_sleep
> s2idle [deep]
> 
> - In the dmesg unchecked MSR for HWP register
> 
> 1.
> Before test
> 
> sudo rdmsr -a 0x774
> 80002b0c
> 80002b0c
> 80002d0c
> 80002b0c
> 80002b0c
> 80002b0c
> 80002b0c
> 80002b0c
> 80002d0c
> 80002b0c
> 80002b0c
> 80002b0c
> 80002d0c
> 80002b0c
> 80002b0c
> 80002b0c
> 80002b0c
> 80002b0c
> 80002d0d
> 7f002b0c

?? This looks like the MSR
read for further below, and for 
CPU 19 instead of 1.

> cd /sys/devices/system/cpu/intel_pstate/
> [root@otcpl-perf-test-skx-i9 intel_pstate]# grep . *
> hwp_dynamic_boost:0
> max_perf_pct:100
> min_perf_pct:27
> no_turbo:0
> num_pstates:32
> status:active
> turbo_pct:32
> 
> cd ../cpu1/cpufreq/
> [root@otcpl-perf-test-skx-i9 cpufreq]# grep . *
> affected_cpus:1
> base_frequency:3300000
> cpuinfo_max_freq:4300000
> cpuinfo_min_freq:1200000
> cpuinfo_transition_latency:0
> energy_performance_available_preferences:default performance
> balance_performance balance_power power
> energy_performance_preference:balance_performance
> related_cpus:1
> scaling_available_governors:performance powersave
> scaling_cur_freq:1200000
> scaling_driver:intel_pstate
> scaling_governor:powersave
> scaling_max_freq:4300000
> scaling_min_freq:1200000
> scaling_setspeed:<unsupported>
> 
> 
> 2. Now change the EPP
> 
> # echo 127 > energy_performance_preference
> sudo rdmsr -a 0x774
> 80002b0c
> 80002b0c
> 80002d0c
> 80002b0c
> 80002b0c
> 80002b0c
> 80002b0c
> 80002b0c
> 80002d0c
> 80002b0c
> 80002b0c
> 80002b0c
> 80002d0c
> 80002b0c
> 80002b0c
> 80002b0c
> 80002b0c
> 80002b0c
> 80002d0d

This looks like the original MSR read.

> 
> Good here
> 
> 3. Offline/online good
> 
> [root@otcpl-perf-test-skx-i9 cpufreq]# echo 0 >
> /sys/devices/system/cpu/cpu1/online
> [root@otcpl-perf-test-skx-i9 cpufreq]# echo ` >
> /sys/devices/system/cpu/cpu1/online
> > echo ` > /sys/devices/system/cpu/cpu1/online ^C
> [root@otcpl-perf-test-skx-i9 cpufreq]# echo 1 >
> /sys/devices/system/cpu/cpu1/online
> 
>  sudo rdmsr -a 0x774
> 80002b0c
> 80002b0c
> 80002d0c
> 80002b0c
> 80002b0c
> 80002b0c
> 80002b0c
> 80002b0c
> 80002d0c
> 80002b0c
> 80002b0c
> 80002b0c
> 80002d0c
> 80002b0c
> 80002b0c
> 80002b0c
> 80002b0c
> 80002b0c
> 80002d0d
> 7f002b0c

O.K.

> 
> Good. Online restored the setting
> 
> 4. Now S3
> 
> rtcwake -m mem -s 10

Cool command. I did not know about it.
I tried it.
> 
> All limits are now messed up
> 
> sudo rdmsr -a 0x774
> 80002b0c
> 8000ff00
> 8000ff00
> 8000ff00
> 8000ff00
> 8000ff00
> 8000ff00
> 8000ff00
> 8000ff00
> 8000ff00
> 8000ff00
> 8000ff00
> 8000ff00
> 8000ff00
> 8000ff00
> 8000ff00
> 8000ff00
> 8000ff00
> 8000ff00
> 8000ff00
> 

Yes, I got the same:

# /home/doug/c/msr-decoder (edited)
6.) 0x774: IA32_HWP_REQUEST:    CPU 0-5 :
    raw: 80002E08 : 8000FF01 : 8000FF01 : 8000FF01 : 8000FF01 : 8000FF01 :
    min:        8 :        1 :        1 :        1 :        1 :        1 :
    max:       46 :      255 :      255 :      255 :      255 :      255 :
    des:        0 :        0 :        0 :        0 :        0 :        0 :
    epp:      128 :      128 :      128 :      128 :      128 :      128 :
    act:        0 :        0 :        0 :        0 :        0 :        0 :

> 5. Now switch to passive
> Again bad, some CPU max/min is 0
> 
> sudo rdmsr -a 0x774
> 80002b0d
> 7f002b0f

Hmmm... Now seems to be CPU 1

> 80002b0c
> 80002d0e
> 80002b0c
> 80002b0d
> 80002b0f
> 80002b2b
> 80002b0c
> 80002d1d
> 80000000
> 80002b0c
> 80002b0c
> 80000000
> 80000000
> 80000000
> 80000000
> 80000000
> 80000000
> 80000000

MSR 774 was good for me, but in general my decoder was having troubles.

0x774: IA32_HWP_REQUEST:    CPU 0-5 :
sh: 0: getcwd() failed: No such file or directory
    raw: 80002E2E : 7F002E2E : 80002E2E : 80002E2E : 80002E2E : 80002E2E :
    min:       46 :       46 :       46 :       46 :       46 :       46 :
    max:       46 :       46 :       46 :       46 :       46 :       46 :
    des:        0 :        0 :        0 :        0 :        0 :        0 :
    epp:      128 :      127 :      128 :      128 :      128 :      128 :
    act:        0 :        0 :        0 :        0 :        0 :        0 : 

> 
> 6.
> Switched back to active to restore back
> 
> Lost EPP setting but rest are good.
> 
> sudo rdmsr -a 0x774
> 80002b0c
> 7f002b0c

And again, now seems to be CPU1.

> 80002b0c
> 80002d0c
> 80002b0c
> 80002b0c
> 80002b0c
> 80002b0c
> 80002b0c
> 80002d0c
> 80002b0c
> 80002b0c
> 80002b0c
> 80002d0c
> 80002b0c
> 80002b0c
> 80002b0c
> 80002b0c
> 80002b0c
> 80002d0d
> 
> 7. S3 again
> 
> rtcwake -m mem -s 10
> 
> Again messed up
> 
>  sudo rdmsr -a 0x774
> 80002b0c
> 8000ff00
> 8000ff00
> 8000ff00
> 8000ff00
> 8000ff00
> 8000ff00
> 8000ff00
> 8000ff00
> 8000ff00
> 8000ff00
> 8000ff00
> 8000ff00
> 8000ff00
> 8000ff00
> 8000ff00
> 8000ff00
> 8000ff00
> 8000ff00
> 8000ff00
> 
> This time dmesg didn't have unchecked MSR in dmesg (I think because I
> didn't change EPP before)
> 
> Thanks,
> Srinivas
> 
> > Thanks,
> > Rafael
> >
> >
> >

Re: [PATCH v2 2/5] cpufreq: intel_pstate: Always return last EPP value from sysfs

[Artem Bityutskiy]

On Mon, 2020-08-24 at 19:42 +0200, Rafael J. Wysocki wrote:
> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> 
> Make the energy_performance_preference policy attribute in sysfs
> always return the last EPP value written to it instead of the one
> currently in the HWP Request MSR to avoid possible confusion when
> the performance scaling algorithm is used in the active mode with
> HWP enabled (in which case the EPP is forced to 0 regardless of
> what value it has been set to via sysfs).

Why is this a good idea, I wonder. If there was a prior discussion,
please, point to it.

The general approach to changing settings via sysfs is often like this:

1. Write new value.
2. Read it back and verify that it is the same. Because there is no
better way to verify that the kernel "accepted" the value.

Let's say I write 'balanced' to energy_performance_preference. I read
it back, and it contains 'balanced', so I am happy, I trust the kernel
changed EPP to "balanced".

If the kernel, in fact, uses something else, I want to know about it
and have my script fail. Why caching the value and making my script
_think_ it succeeded is a good idea.

In other words, in my usage scenarios at list, I prefer kernel telling
the true EPP value, not some "cached, but not used" value.

Artem.

Re: [PATCH v2 2/5] cpufreq: intel_pstate: Always return last EPP value from sysfs

[Rafael J. Wysocki]

On Tue, Aug 25, 2020 at 8:20 AM Artem Bityutskiy <dedekind1@gmail.com> wrote:
>
> On Mon, 2020-08-24 at 19:42 +0200, Rafael J. Wysocki wrote:
> > From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> >
> > Make the energy_performance_preference policy attribute in sysfs
> > always return the last EPP value written to it instead of the one
> > currently in the HWP Request MSR to avoid possible confusion when
> > the performance scaling algorithm is used in the active mode with
> > HWP enabled (in which case the EPP is forced to 0 regardless of
> > what value it has been set to via sysfs).
>
> Why is this a good idea, I wonder. If there was a prior discussion,
> please, point to it.
>
> The general approach to changing settings via sysfs is often like this:
>
> 1. Write new value.
> 2. Read it back and verify that it is the same. Because there is no
> better way to verify that the kernel "accepted" the value.

If the write is successful (ie. no errors returned and the value
returned is equal to the number of written characters), the kernel
*has* accepted the written value, but it may not have taken effect.
These are two different things.

The written value may take an effect immediately or it may take an
effect later, depending on the current configuration etc.  If you
don't see the effect of it immediately, it doesn't matter that there
was a failure of some sort.

> Let's say I write 'balanced' to energy_performance_preference. I read
> it back, and it contains 'balanced', so I am happy, I trust the kernel
> changed EPP to "balanced".
>
> If the kernel, in fact, uses something else, I want to know about it
> and have my script fail.

Why do you want it to fail then?

> Why caching the value and making my script _think_ it succeeded is a good idea.

Because when you change the scaling algorithm or the driver's
operation mode, the value you have written will take effect.

In this particular case it is explained in the driver documentation
that the performance scaling algorithm in the active mode overrides
the sysfs value and that's the only case when it can be overridden.
So whatever you write to this attribute will not take effect
immediately anyway, but it may take an effect later.

> In other words, in my usage scenarios at list, I prefer kernel telling
> the true EPP value, not some "cached, but not used" value.

An alternative is to fail writes to energy_performance_preference if
the driver works in the active mode and the scaling algorithm for the
scaling CPU is performance and *then* to make reads from it return the
value in the register.

Accepting a write and returning a different value in a subsequent read
is confusing.

Thanks!

Re: [PATCH v2 2/5] cpufreq: intel_pstate: Always return last EPP value from sysfs

[Srinivas Pandruvada]

On Tue, 2020-08-25 at 16:51 +0200, Rafael J. Wysocki wrote:
> On Tue, Aug 25, 2020 at 8:20 AM Artem Bityutskiy <dedekind1@gmail.com
> > wrote:
> > On Mon, 2020-08-24 at 19:42 +0200, Rafael J. Wysocki wrote:
> > > From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> > > 
> > > Make the energy_performance_preference policy attribute in sysfs
> > > always return the last EPP value written to it instead of the one
> > > currently in the HWP Request MSR to avoid possible confusion when
> > > the performance scaling algorithm is used in the active mode with
> > > HWP enabled (in which case the EPP is forced to 0 regardless of
> > > what value it has been set to via sysfs).
> > 
> > Why is this a good idea, I wonder. If there was a prior discussion,
> > please, point to it.
> > 
> > The general approach to changing settings via sysfs is often like
> > this:
> > 
> > 1. Write new value.
> > 2. Read it back and verify that it is the same. Because there is no
> > better way to verify that the kernel "accepted" the value.
> 
> If the write is successful (ie. no errors returned and the value
> returned is equal to the number of written characters), the kernel
> *has* accepted the written value, but it may not have taken effect.
> These are two different things.
> 
> The written value may take an effect immediately or it may take an
> effect later, depending on the current configuration etc.  If you
> don't see the effect of it immediately, it doesn't matter that there
> was a failure of some sort.
> 
> > Let's say I write 'balanced' to energy_performance_preference. I
> > read
> > it back, and it contains 'balanced', so I am happy, I trust the
> > kernel
> > changed EPP to "balanced".
> > 
> > If the kernel, in fact, uses something else, I want to know about
> > it
> > and have my script fail.
> 
> Why do you want it to fail then?
> 
> > Why caching the value and making my script _think_ it succeeded is
> > a good idea.
> 
> Because when you change the scaling algorithm or the driver's
> operation mode, the value you have written will take effect.
> 
> In this particular case it is explained in the driver documentation
> that the performance scaling algorithm in the active mode overrides
> the sysfs value and that's the only case when it can be overridden.
> So whatever you write to this attribute will not take effect
> immediately anyway, but it may take an effect later.

In some cases without even changing active/passive this is happening
when there was some error previously. For example:

#cat energy_performance_preference 
127
[root@otcpl-perf-test-skx-i9 cpufreq]# rdmsr -p 1 0x774
8000ff00

I think we should show reality. In mode change can be a special case
and use the stored value to restore in new mode.

Thanks,
Srinivas

> > In other words, in my usage scenarios at list, I prefer kernel
> > telling
> > the true EPP value, not some "cached, but not used" value.
> 
> An alternative is to fail writes to energy_performance_preference if
> the driver works in the active mode and the scaling algorithm for the
> scaling CPU is performance and *then* to make reads from it return
> the
> value in the register.
> 
> Accepting a write and returning a different value in a subsequent
> read
> is confusing.
> 
> Thanks!

Re: [PATCH v2 0/5] cpufreq: intel_pstate: Address some HWP-related oddities

[Srinivas Pandruvada]

Hi Doug,

On Mon, 2020-08-24 at 18:00 -0700, Doug Smythies wrote:
> Hi Srinivas,
> 
> I think there is a disconnect between your written
> description of what is going on and your supporting MSR reads.
> 
I reproduced again.
I see the copy paste individual at the first place swapped.
I pasted the full output by direct copy - paste from the screen.

But the issues are still there.

[labuser@otcpl-perf-test-skx-i9 ~]$ sudo -s
[root@otcpl-perf-test-skx-i9 labuser]# rdmsr -a 0x774
80002b0c
80002b0c
80002b0c
80002d0c
80002b0c
80002b0c
80002b0c
80002b0c
80002b0c
80002d0c
80002b0c
80002b0c
80002b0c
80002d0c
80002b0c
80002b0c
80002b0c
80002b0c
80002b0c
80002d0d
[root@otcpl-perf-test-skx-i9 labuser]# cd /sys/devices/system/cpu/cpu1
[root@otcpl-perf-test-skx-i9 cpu1]# cd cpufreq/
[root@otcpl-perf-test-skx-i9 cpufreq]# echo 127 >
energy_performance_preference 
[root@otcpl-perf-test-skx-i9 cpufreq]# cat
energy_performance_preference 
127
[root@otcpl-perf-test-skx-i9 cpufreq]# rdmsr -a 0x774
80002b0c
7f002b0c
80002b0c
80002d0c
80002b0c
80002b0c
80002b0c
80002b0c
80002b0c
80002d0c
80002b0c
80002b0c
80002b0c
80002d0c
80002b0c
80002b0c
80002b0c
80002b0c
80002b0c
80002d0d
[root@otcpl-perf-test-skx-i9 cpufreq]# echo 0 >
/sys/devices/system/cpu/cpu1/online 
[root@otcpl-perf-test-skx-i9 cpufreq]# echo 1 >
/sys/devices/system/cpu/cpu1/online 
[root@otcpl-perf-test-skx-i9 cpufreq]# rdmsr -a 0x774
80002b0c
80002b0c
80002d0c
80002b0c
80002b0c
80002b0c
80002b0c
80002b0c
80002d0c
80002b0c
80002b0c
80002b0c
80002d0c
80002b0c
80002b0c
80002b0c
80002b0c
80002b0c
80002d0d
7f002b0c
[root@otcpl-perf-test-skx-i9 cpufreq]# rdmsr -p 1 0x774
7f002b0c
[root@otcpl-perf-test-skx-i9 cpufreq]# rdmsr -p 19 0x774
80002d0d
[root@otcpl-perf-test-skx-i9 cpufreq]# rtcwake -m mem -s 10
rtcwake: wakeup from "mem" using /dev/rtc0 at Tue Aug 25 15:04:02 2020
[root@otcpl-perf-test-skx-i9 cpufreq]# rdmsr -a 0x774
80002b0c
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
8000ff00
[root@otcpl-perf-test-skx-i9 cpufreq]# dmesg > ~/temp/dmesg.txt
[root@otcpl-perf-test-skx-i9 cpufreq]# cat
energy_performance_preference 
127
[root@otcpl-perf-test-skx-i9 cpufreq]# rdmsr -p 1 0x774
8000ff00
[root@otcpl-perf-test-skx-i9 cpufreq]# echo passive >
/sys/devices/system/
clockevents/  clocksource/  container/    cpu/          edac/         m
achinecheck/ memory/       node/         
[root@otcpl-perf-test-skx-i9 cpufreq]# echo passive >
/sys/devices/system/cpu/intel_pstate/status 
[root@otcpl-perf-test-skx-i9 cpufreq]# rdmsr -a 0x774
80002b0c
7f000000
80000000
80002d0c
80002b0c
80000000
80000000
80002b0c
80000000
80002d0c
80000000
80002b0c
80002b0d
80002d0c
80000000
80000000
80000000
80002b0d
80000000
80000000
[root@otcpl-perf-test-skx-i9 cpufreq]# rdmsr -a 0x774
80002b0c
7f000000
80000000
80002d0c
80002b0c
80000000
80000000
80002b0c
80000000
80002d0c
80000000
80002b0c
80002b0d
80002d0c
80000000
80002b0c
80000000
80002b0d
80000000
80000000

> On 2020.08.24 16:56 Srinivas Pandruvada wrote:
> > On Mon, 2020-08-24 at 19:39 +0200, Rafael J. Wysocki wrote:
> > > Hi All,
> > > 
> > > The v2 is here to address feedback from Doug and one issue found
> > > by
> > > me.
> > > 
> > > The purpose of this series is to address some peculiarities
> > > related
> > > to
> > > taking CPUs offline/online and switching between different
> > > operation
> > > modes with HWP enabled that have become visible after allowing
> > > the
> > > driver to work in the passive mode with HWP enabled in 5.9-rc1
> > > (and
> > > one that was there earlier, but can be addressed easily after the
> > > changes madein 5.9-rc1).
> > > 
> > > Please refer to the patch changelogs for details.
> > > 
> > > For easier testing/review, the series is available from the git
> > > branch at:
> > > 
> > >  git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-
> > > pm.git \
> > >  intel_pstate-testing
> > > 
> > 
> > Applied these patches to 5.9-rc2
> 
> So did I, and the issues I reported the other day are fine now.
> I did try a few of the things you were doing.
> 
> > - After s3  limits got messed up.
> >  # cat /sys/power/mem_sleep
> > s2idle [deep]
> > 
> > - In the dmesg unchecked MSR for HWP register
> > 
> > 1.
> > Before test
> > 
> > sudo rdmsr -a 0x774
> > 80002b0c
> > 80002b0c
> > 80002d0c
> > 80002b0c
> > 80002b0c
> > 80002b0c
> > 80002b0c
> > 80002b0c
> > 80002d0c
> > 80002b0c
> > 80002b0c
> > 80002b0c
> > 80002d0c
> > 80002b0c
> > 80002b0c
> > 80002b0c
> > 80002b0c
> > 80002b0c
> > 80002d0d
> > 7f002b0c
> 
> ?? This looks like the MSR
> read for further below, and for 
> CPU 19 instead of 1.
> 
> > cd /sys/devices/system/cpu/intel_pstate/
> > [root@otcpl-perf-test-skx-i9 intel_pstate]# grep . *
> > hwp_dynamic_boost:0
> > max_perf_pct:100
> > min_perf_pct:27
> > no_turbo:0
> > num_pstates:32
> > status:active
> > turbo_pct:32
> > 
> > cd ../cpu1/cpufreq/
> > [root@otcpl-perf-test-skx-i9 cpufreq]# grep . *
> > affected_cpus:1
> > base_frequency:3300000
> > cpuinfo_max_freq:4300000
> > cpuinfo_min_freq:1200000
> > cpuinfo_transition_latency:0
> > energy_performance_available_preferences:default performance
> > balance_performance balance_power power
> > energy_performance_preference:balance_performance
> > related_cpus:1
> > scaling_available_governors:performance powersave
> > scaling_cur_freq:1200000
> > scaling_driver:intel_pstate
> > scaling_governor:powersave
> > scaling_max_freq:4300000
> > scaling_min_freq:1200000
> > scaling_setspeed:<unsupported>
> > 
> > 
> > 2. Now change the EPP
> > 
> > # echo 127 > energy_performance_preference
> > sudo rdmsr -a 0x774
> > 80002b0c
> > 80002b0c
> > 80002d0c
> > 80002b0c
> > 80002b0c
> > 80002b0c
> > 80002b0c
> > 80002b0c
> > 80002d0c
> > 80002b0c
> > 80002b0c
> > 80002b0c
> > 80002d0c
> > 80002b0c
> > 80002b0c
> > 80002b0c
> > 80002b0c
> > 80002b0c
> > 80002d0d
> 
> This looks like the original MSR read.
> 
> > Good here
> > 
> > 3. Offline/online good
> > 
> > [root@otcpl-perf-test-skx-i9 cpufreq]# echo 0 >
> > /sys/devices/system/cpu/cpu1/online
> > [root@otcpl-perf-test-skx-i9 cpufreq]# echo ` >
> > /sys/devices/system/cpu/cpu1/online
> > > echo ` > /sys/devices/system/cpu/cpu1/online ^C
> > [root@otcpl-perf-test-skx-i9 cpufreq]# echo 1 >
> > /sys/devices/system/cpu/cpu1/online
> > 
> >  sudo rdmsr -a 0x774
> > 80002b0c
> > 80002b0c
> > 80002d0c
> > 80002b0c
> > 80002b0c
> > 80002b0c
> > 80002b0c
> > 80002b0c
> > 80002d0c
> > 80002b0c
> > 80002b0c
> > 80002b0c
> > 80002d0c
> > 80002b0c
> > 80002b0c
> > 80002b0c
> > 80002b0c
> > 80002b0c
> > 80002d0d
> > 7f002b0c
> 
> O.K.
> 
> > Good. Online restored the setting
> > 
> > 4. Now S3
> > 
> > rtcwake -m mem -s 10
> 
> Cool command. I did not know about it.
> I tried it.
> > All limits are now messed up
> > 
> > sudo rdmsr -a 0x774
> > 80002b0c
> > 8000ff00
> > 8000ff00
> > 8000ff00
> > 8000ff00
> > 8000ff00
> > 8000ff00
> > 8000ff00
> > 8000ff00
> > 8000ff00
> > 8000ff00
> > 8000ff00
> > 8000ff00
> > 8000ff00
> > 8000ff00
> > 8000ff00
> > 8000ff00
> > 8000ff00
> > 8000ff00
> > 8000ff00
> > 
> 
> Yes, I got the same:
> 
> # /home/doug/c/msr-decoder (edited)
> 6.) 0x774: IA32_HWP_REQUEST:    CPU 0-5 :
>     raw: 80002E08 : 8000FF01 : 8000FF01 : 8000FF01 : 8000FF01 :
> 8000FF01 :
>     min:        8 :        1 :        1 :        1 :        1
> :        1 :
>     max:       46 :      255 :      255 :      255 :      255
> :      255 :
>     des:        0 :        0 :        0 :        0 :        0
> :        0 :
>     epp:      128 :      128 :      128 :      128 :      128
> :      128 :
>     act:        0 :        0 :        0 :        0 :        0
> :        0 :
> 
> > 5. Now switch to passive
> > Again bad, some CPU max/min is 0
> > 
> > sudo rdmsr -a 0x774
> > 80002b0d
> > 7f002b0f
> 
> Hmmm... Now seems to be CPU 1
> 
> > 80002b0c
> > 80002d0e
> > 80002b0c
> > 80002b0d
> > 80002b0f
> > 80002b2b
> > 80002b0c
> > 80002d1d
> > 80000000
> > 80002b0c
> > 80002b0c
> > 80000000
> > 80000000
> > 80000000
> > 80000000
> > 80000000
> > 80000000
> > 80000000
> 
> MSR 774 was good for me, but in general my decoder was having
> troubles.
> 
> 0x774: IA32_HWP_REQUEST:    CPU 0-5 :
> sh: 0: getcwd() failed: No such file or directory
>     raw: 80002E2E : 7F002E2E : 80002E2E : 80002E2E : 80002E2E :
> 80002E2E :
>     min:       46 :       46 :       46 :       46 :       46
> :       46 :
>     max:       46 :       46 :       46 :       46 :       46
> :       46 :
>     des:        0 :        0 :        0 :        0 :        0
> :        0 :
>     epp:      128 :      127 :      128 :      128 :      128
> :      128 :
>     act:        0 :        0 :        0 :        0 :        0
> :        0 : 
> 
> > 6.
> > Switched back to active to restore back
> > 
> > Lost EPP setting but rest are good.
> > 
> > sudo rdmsr -a 0x774
> > 80002b0c
> > 7f002b0c
> 
> And again, now seems to be CPU1.
> 
> > 80002b0c
> > 80002d0c
> > 80002b0c
> > 80002b0c
> > 80002b0c
> > 80002b0c
> > 80002b0c
> > 80002d0c
> > 80002b0c
> > 80002b0c
> > 80002b0c
> > 80002d0c
> > 80002b0c
> > 80002b0c
> > 80002b0c
> > 80002b0c
> > 80002b0c
> > 80002d0d
> > 
> > 7. S3 again
> > 
> > rtcwake -m mem -s 10
> > 
> > Again messed up
> > 
> >  sudo rdmsr -a 0x774
> > 80002b0c
> > 8000ff00
> > 8000ff00
> > 8000ff00
> > 8000ff00
> > 8000ff00
> > 8000ff00
> > 8000ff00
> > 8000ff00
> > 8000ff00
> > 8000ff00
> > 8000ff00
> > 8000ff00
> > 8000ff00
> > 8000ff00
> > 8000ff00
> > 8000ff00
> > 8000ff00
> > 8000ff00
> > 8000ff00
> > 
> > This time dmesg didn't have unchecked MSR in dmesg (I think because
> > I
> > didn't change EPP before)
> > 
> > Thanks,
> > Srinivas
> > 
> > > Thanks,
> > > Rafael
> > > 
> > > 
> > > 

Re: [PATCH v2 2/5] cpufreq: intel_pstate: Always return last EPP value from sysfs

[Rafael J. Wysocki]

On Tue, Aug 25, 2020 at 5:06 PM Srinivas Pandruvada
<srinivas.pandruvada@linux.intel.com> wrote:
>
> On Tue, 2020-08-25 at 16:51 +0200, Rafael J. Wysocki wrote:
> > On Tue, Aug 25, 2020 at 8:20 AM Artem Bityutskiy <dedekind1@gmail.com
> > > wrote:
> > > On Mon, 2020-08-24 at 19:42 +0200, Rafael J. Wysocki wrote:
> > > > From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> > > >
> > > > Make the energy_performance_preference policy attribute in sysfs
> > > > always return the last EPP value written to it instead of the one
> > > > currently in the HWP Request MSR to avoid possible confusion when
> > > > the performance scaling algorithm is used in the active mode with
> > > > HWP enabled (in which case the EPP is forced to 0 regardless of
> > > > what value it has been set to via sysfs).
> > >
> > > Why is this a good idea, I wonder. If there was a prior discussion,
> > > please, point to it.
> > >
> > > The general approach to changing settings via sysfs is often like
> > > this:
> > >
> > > 1. Write new value.
> > > 2. Read it back and verify that it is the same. Because there is no
> > > better way to verify that the kernel "accepted" the value.
> >
> > If the write is successful (ie. no errors returned and the value
> > returned is equal to the number of written characters), the kernel
> > *has* accepted the written value, but it may not have taken effect.
> > These are two different things.
> >
> > The written value may take an effect immediately or it may take an
> > effect later, depending on the current configuration etc.  If you
> > don't see the effect of it immediately, it doesn't matter that there
> > was a failure of some sort.
> >
> > > Let's say I write 'balanced' to energy_performance_preference. I
> > > read
> > > it back, and it contains 'balanced', so I am happy, I trust the
> > > kernel
> > > changed EPP to "balanced".
> > >
> > > If the kernel, in fact, uses something else, I want to know about
> > > it
> > > and have my script fail.
> >
> > Why do you want it to fail then?
> >
> > > Why caching the value and making my script _think_ it succeeded is
> > > a good idea.
> >
> > Because when you change the scaling algorithm or the driver's
> > operation mode, the value you have written will take effect.
> >
> > In this particular case it is explained in the driver documentation
> > that the performance scaling algorithm in the active mode overrides
> > the sysfs value and that's the only case when it can be overridden.
> > So whatever you write to this attribute will not take effect
> > immediately anyway, but it may take an effect later.
>
> In some cases without even changing active/passive this is happening
> when there was some error previously. For example:
>
> #cat energy_performance_preference
> 127
> [root@otcpl-perf-test-skx-i9 cpufreq]# rdmsr -p 1 0x774
> 8000ff00
>
> I think we should show reality. In mode change can be a special case
> and use the stored value to restore in new mode.

OK, so I'll make it fail on attempts to change the EPP from 0
(performance) in the active mode with the performance "governor".

Cheers!

Re: [PATCH v2 2/5] cpufreq: intel_pstate: Always return last EPP value from sysfs

[Srinivas Pandruvada]

On Tue, 2020-08-25 at 17:14 +0200, Rafael J. Wysocki wrote:
> On Tue, Aug 25, 2020 at 5:06 PM Srinivas Pandruvada
> <srinivas.pandruvada@linux.intel.com> wrote:
> > On Tue, 2020-08-25 at 16:51 +0200, Rafael J. Wysocki wrote:
> > > On Tue, Aug 25, 2020 at 8:20 AM Artem Bityutskiy <
> > > dedekind1@gmail.com
> > > > wrote:
> > > > On Mon, 2020-08-24 at 19:42 +0200, Rafael J. Wysocki wrote:
> > > > > From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> > > > > 
> > > > > Make the energy_performance_preference policy attribute in
> > > > > sysfs
> > > > > always return the last EPP value written to it instead of the
> > > > > one
> > > > > currently in the HWP Request MSR to avoid possible confusion
> > > > > when
> > > > > the performance scaling algorithm is used in the active mode
> > > > > with
> > > > > HWP enabled (in which case the EPP is forced to 0 regardless
> > > > > of
> > > > > what value it has been set to via sysfs).
> > > > 
> > > > Why is this a good idea, I wonder. If there was a prior
> > > > discussion,
> > > > please, point to it.
> > > > 
> > > > The general approach to changing settings via sysfs is often
> > > > like
> > > > this:
> > > > 
> > > > 1. Write new value.
> > > > 2. Read it back and verify that it is the same. Because there
> > > > is no
> > > > better way to verify that the kernel "accepted" the value.
> > > 
> > > If the write is successful (ie. no errors returned and the value
> > > returned is equal to the number of written characters), the
> > > kernel
> > > *has* accepted the written value, but it may not have taken
> > > effect.
> > > These are two different things.
> > > 
> > > The written value may take an effect immediately or it may take
> > > an
> > > effect later, depending on the current configuration etc.  If you
> > > don't see the effect of it immediately, it doesn't matter that
> > > there
> > > was a failure of some sort.
> > > 
> > > > Let's say I write 'balanced' to energy_performance_preference.
> > > > I
> > > > read
> > > > it back, and it contains 'balanced', so I am happy, I trust the
> > > > kernel
> > > > changed EPP to "balanced".
> > > > 
> > > > If the kernel, in fact, uses something else, I want to know
> > > > about
> > > > it
> > > > and have my script fail.
> > > 
> > > Why do you want it to fail then?
> > > 
> > > > Why caching the value and making my script _think_ it succeeded
> > > > is
> > > > a good idea.
> > > 
> > > Because when you change the scaling algorithm or the driver's
> > > operation mode, the value you have written will take effect.
> > > 
> > > In this particular case it is explained in the driver
> > > documentation
> > > that the performance scaling algorithm in the active mode
> > > overrides
> > > the sysfs value and that's the only case when it can be
> > > overridden.
> > > So whatever you write to this attribute will not take effect
> > > immediately anyway, but it may take an effect later.
> > 
> > In some cases without even changing active/passive this is
> > happening
> > when there was some error previously. For example:
> > 
> > #cat energy_performance_preference
> > 127
> > [root@otcpl-perf-test-skx-i9 cpufreq]# rdmsr -p 1 0x774
> > 8000ff00
> > 
> > I think we should show reality. In mode change can be a special
> > case
> > and use the stored value to restore in new mode.
> 
> OK, so I'll make it fail on attempts to change the EPP from 0
> (performance) in the active mode with the performance "governor".
> 
Here the scaling governor is powersave.

# cat scaling_governor 
powersave


Thanks,
Srinivas

> Cheers!

Re: [PATCH v2 2/5] cpufreq: intel_pstate: Always return last EPP value from sysfs

[Rafael J. Wysocki]

On Tue, Aug 25, 2020 at 5:27 PM Srinivas Pandruvada
<srinivas.pandruvada@linux.intel.com> wrote:
>
> On Tue, 2020-08-25 at 17:14 +0200, Rafael J. Wysocki wrote:
> > On Tue, Aug 25, 2020 at 5:06 PM Srinivas Pandruvada
> > <srinivas.pandruvada@linux.intel.com> wrote:
> > > On Tue, 2020-08-25 at 16:51 +0200, Rafael J. Wysocki wrote:
> > > > On Tue, Aug 25, 2020 at 8:20 AM Artem Bityutskiy <
> > > > dedekind1@gmail.com
> > > > > wrote:
> > > > > On Mon, 2020-08-24 at 19:42 +0200, Rafael J. Wysocki wrote:
> > > > > > From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> > > > > >
> > > > > > Make the energy_performance_preference policy attribute in
> > > > > > sysfs
> > > > > > always return the last EPP value written to it instead of the
> > > > > > one
> > > > > > currently in the HWP Request MSR to avoid possible confusion
> > > > > > when
> > > > > > the performance scaling algorithm is used in the active mode
> > > > > > with
> > > > > > HWP enabled (in which case the EPP is forced to 0 regardless
> > > > > > of
> > > > > > what value it has been set to via sysfs).
> > > > >
> > > > > Why is this a good idea, I wonder. If there was a prior
> > > > > discussion,
> > > > > please, point to it.
> > > > >
> > > > > The general approach to changing settings via sysfs is often
> > > > > like
> > > > > this:
> > > > >
> > > > > 1. Write new value.
> > > > > 2. Read it back and verify that it is the same. Because there
> > > > > is no
> > > > > better way to verify that the kernel "accepted" the value.
> > > >
> > > > If the write is successful (ie. no errors returned and the value
> > > > returned is equal to the number of written characters), the
> > > > kernel
> > > > *has* accepted the written value, but it may not have taken
> > > > effect.
> > > > These are two different things.
> > > >
> > > > The written value may take an effect immediately or it may take
> > > > an
> > > > effect later, depending on the current configuration etc.  If you
> > > > don't see the effect of it immediately, it doesn't matter that
> > > > there
> > > > was a failure of some sort.
> > > >
> > > > > Let's say I write 'balanced' to energy_performance_preference.
> > > > > I
> > > > > read
> > > > > it back, and it contains 'balanced', so I am happy, I trust the
> > > > > kernel
> > > > > changed EPP to "balanced".
> > > > >
> > > > > If the kernel, in fact, uses something else, I want to know
> > > > > about
> > > > > it
> > > > > and have my script fail.
> > > >
> > > > Why do you want it to fail then?
> > > >
> > > > > Why caching the value and making my script _think_ it succeeded
> > > > > is
> > > > > a good idea.
> > > >
> > > > Because when you change the scaling algorithm or the driver's
> > > > operation mode, the value you have written will take effect.
> > > >
> > > > In this particular case it is explained in the driver
> > > > documentation
> > > > that the performance scaling algorithm in the active mode
> > > > overrides
> > > > the sysfs value and that's the only case when it can be
> > > > overridden.
> > > > So whatever you write to this attribute will not take effect
> > > > immediately anyway, but it may take an effect later.
> > >
> > > In some cases without even changing active/passive this is
> > > happening
> > > when there was some error previously. For example:
> > >
> > > #cat energy_performance_preference
> > > 127
> > > [root@otcpl-perf-test-skx-i9 cpufreq]# rdmsr -p 1 0x774
> > > 8000ff00
> > >
> > > I think we should show reality. In mode change can be a special
> > > case
> > > and use the stored value to restore in new mode.
> >
> > OK, so I'll make it fail on attempts to change the EPP from 0
> > (performance) in the active mode with the performance "governor".
> >
> Here the scaling governor is powersave.
>
> # cat scaling_governor
> powersave

What I'm saying is that reads from energy_performance_preference will
still return the register value, but writes to it will fail on
attempts to change to anything different from "performance" when in
the active mode and the current governor is "performance".

Cheers!

RE: [PATCH v2 0/5] cpufreq: intel_pstate: Address some HWP-related oddities

[Doug Smythies]

Hi Srinivas,

Thanks for your reply.

On 2020.08.25 08:12 Srinivas Pandruvada wrote:
> On Mon, 2020-08-24 at 18:00 -0700, Doug Smythies wrote:
> > I think there is a disconnect between your written
> > description of what is going on and your supporting MSR reads.
> >
> I reproduced again.
> I see the copy paste individual at the first place swapped.

Yes, and that had me confused, initially.

> I pasted the full output by direct copy - paste from the screen.
> 
> But the issues are still there.

Agreed.
I didn't try your offline/online of CPU 1 part previously,
but did now, and get the same results as you.

I did not know that "rdmsr -a 0x774" lists
stuff in the order that CPU were last brought on-line.
I had assumed the list was in CPU order. Weird.

My example (nothing new here, just me catching up.
The offline/online order was cpu1, then cpu3, then cpu2):

root@s18:/sys/devices/system/cpu# grep . cpu*/cpufreq/energy_performance_preference
cpu0/cpufreq/energy_performance_preference:balance_performance
cpu1/cpufreq/energy_performance_preference:127
cpu2/cpufreq/energy_performance_preference:125
cpu3/cpufreq/energy_performance_preference:126
cpu4/cpufreq/energy_performance_preference:balance_performance
cpu5/cpufreq/energy_performance_preference:balance_performance
root@s18:/sys/devices/system/cpu# rdmsr -p 0 0x774
80002e2e
root@s18:/sys/devices/system/cpu# rdmsr -p 1 0x774
7f002e2e
root@s18:/sys/devices/system/cpu# rdmsr -p 2 0x774
7d002e2e
root@s18:/sys/devices/system/cpu# rdmsr -p 3 0x774
7e002e2e
root@s18:/sys/devices/system/cpu# rdmsr -p 4 0x774
80002e2e
root@s18:/sys/devices/system/cpu# rdmsr -p 5 0x774
80002e2e
root@s18:/sys/devices/system/cpu# rdmsr -a 0x774
80002e2e
80002e2e
80002e2e
7f002e2e
7e002e2e
7d002e2e

... Doug

Re: [PATCH v2 2/5] cpufreq: intel_pstate: Always return last EPP value from sysfs

[Artem Bityutskiy]

Thanks for answer Rafael, it looks like there are 2 different things
now.

1. What kernel returns when I _read_ e_p_p file - truth or "cached" ?

2. How kernel behaves when I _write_ to e_p_p file something it cannot
provide - error or success.

For #1, I think we need to keep it simple and always return true policy
value. Does not matter what someone wrote there. If some process wrote
"powersave", but kernel uses EPP 0 anyway, the other process probably
wants to know the truth and get "performance" when reading e_p_p.

On Tue, 2020-08-25 at 16:51 +0200, Rafael J. Wysocki wrote:
> An alternative is to fail writes to energy_performance_preference if
> the driver works in the active mode and the scaling algorithm for the
> scaling CPU is performance and *then* to make reads from it return the
> value in the register.

Yes, this is #2. This sounds like the _right_ way to do it.

Suppose my script wants to exercise the system with 4 different EPP
policies. It changes the policy and runs measurements, each run takes
few _days_.

Now, my script asks for "powersave". Kernel _knows_ it cannot provide
it (performance+active enabled). Why would it not return error ("can't
do") instead of success ("yes, Sir!")?

Note, I deliberately use simple words like "my script" instead of "a
user-space process" to make it easier to convey the idea.

Anyway, if kernel returns error, I can go and improve my script WRT
controlling the performance+active mode knobs.

Re: [PATCH v2 3/5] cpufreq: intel_pstate: Add ->offline and ->online callbacks

[Jinjie Ruan]

On 2020/8/25 1:43, Rafael J. Wysocki wrote:
> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> 
> Add ->offline and ->online driver callbacks to prepare for taking a
> CPU offline and to restore its working configuration when it goes
> back online, respectively, to avoid invoking the ->init callback on
> every CPU online which is quite a bit of unnecessary overhead.
> 
> Define ->offline and ->online so that they can be used in the
> passive mode as well as in the active mode and because ->offline
> will do the majority of ->stop_cpu work, the passive mode does
> not need that callback any more, so drop it.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> 
> -> v2: Typo fixes and changelog edits (Doug).
> 
> ---
>  drivers/cpufreq/intel_pstate.c | 38 ++++++++++++++++++++++++++++------
>  1 file changed, 32 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index 3d18934fa975..98836ac299db 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -2297,28 +2297,51 @@ static int intel_pstate_verify_policy(struct cpufreq_policy_data *policy)
>  	return 0;
>  }
>  
> -static void intel_cpufreq_stop_cpu(struct cpufreq_policy *policy)
> +static int intel_pstate_cpu_offline(struct cpufreq_policy *policy)
>  {
> +	pr_debug("CPU %d going offline\n", policy->cpu);
> +
> +	intel_pstate_exit_perf_limits(policy);
> +
> +	/*
> +	 * If the CPU is an SMT thread and it goes offline with the performance
> +	 * settings different from the minimum, it will prevent its sibling
> +	 * from getting to lower performance levels, so force the minimum
> +	 * performance on CPU offline to prevent that from happening.
> +	 */
>  	if (hwp_active)
>  		intel_pstate_hwp_force_min_perf(policy->cpu);
>  	else
>  		intel_pstate_set_min_pstate(all_cpu_data[policy->cpu]);
> +
> +	return 0;
> +}
> +
> +static int intel_pstate_cpu_online(struct cpufreq_policy *policy)
> +{
> +	pr_debug("CPU %d going online\n", policy->cpu);
> +
> +	intel_pstate_init_acpi_perf_limits(policy);
> +
> +	if (hwp_active)
> +		wrmsrl_on_cpu(policy->cpu, MSR_HWP_REQUEST,
> +			      all_cpu_data[policy->cpu]->hwp_req_cached);
> +
> +	return 0;
>  }

On Ice Lake server, there seems a bug when CONFIG_X86_INTEL_PSTATE=y and
not configure intel_pstate=xxx in command line.

Although the Performance tuner is used, the CPU have the lowest
frequency in scaling_cur_freq after the CPU goes offline and then goes
online, running the same infinite loop load.

How to produce:

echo performance > /sys/devices/system/cpu/cpu12/cpufreq/scaling_governor

cat while_true.c
#include <stdio.h>
void main(void)
{
        while(1);
}


[root@localhost freq_test]# cat test.sh
#!/bin/bash

cat /sys/devices/system/cpu/cpu${1}/cpufreq/scaling_cur_freq
cat /sys/devices/system/cpu/cpu${1}/cpufreq/scaling_governor
taskset -c ${1} ./while_true &
sleep 1s

cat /sys/devices/system/cpu/cpu${1}/cpufreq/scaling_cur_freq

echo 0 > /sys/devices/system/cpu/cpu${1}/online

sleep 1s
cat /sys/devices/system/cpu/cpu${1}/cpufreq/scaling_cur_freq

sleep 1s

echo 1 > /sys/devices/system/cpu/cpu${1}/online
cat /sys/devices/system/cpu/cpu${1}/cpufreq/scaling_cur_freq

taskset -c ${1} ./while_true &

sleep 1s
cat /sys/devices/system/cpu/cpu${1}/cpufreq/scaling_cur_freq

sleep 1s
cat /sys/devices/system/cpu/cpu${1}/cpufreq/scaling_cur_freq

sleep 1s
cat /sys/devices/system/cpu/cpu${1}/cpufreq/scaling_cur_freq


[root@localhost freq_test]# sh test.sh 40
2300000
performance
2299977
cat: /sys/devices/system/cpu/cpu40/cpufreq/scaling_cur_freq: Device or
resource busy
2300000
2300022
2300000
2299953
[root@localhost freq_test]# sh test.sh 50
2300000
performance
2300000
cat: /sys/devices/system/cpu/cpu50/cpufreq/scaling_cur_freq: Device or
resource busy
2300000
2299977
2300022
2299977
[root@localhost freq_test]# sh test.sh 20
2300000
performance
2299977
cat: /sys/devices/system/cpu/cpu20/cpufreq/scaling_cur_freq: Device or
resource busy
800000
800000
800000
799992
[root@localhost freq_test]# sh test.sh 21
2300000
performance
2300000
cat: /sys/devices/system/cpu/cpu21/cpufreq/scaling_cur_freq: Device or
resource busy
800000
800000
800000
800000

[root@localhost freq_test]# cat
/sys/devices/system/cpu/cpu21/cpufreq/scaling_max_freq
2300000
[root@localhost freq_test]# cat
/sys/devices/system/cpu/cpu21/cpufreq/scaling_min_freq
800000

>  
>  static void intel_pstate_stop_cpu(struct cpufreq_policy *policy)
>  {
> -	pr_debug("CPU %d exiting\n", policy->cpu);
> +	pr_debug("CPU %d stopping\n", policy->cpu);
>  
>  	intel_pstate_clear_update_util_hook(policy->cpu);
>  	if (hwp_active)
>  		intel_pstate_hwp_save_state(policy);
> -
> -	intel_cpufreq_stop_cpu(policy);
>  }
>  
>  static int intel_pstate_cpu_exit(struct cpufreq_policy *policy)
>  {
> -	intel_pstate_exit_perf_limits(policy);
> +	pr_debug("CPU %d exiting\n", policy->cpu);
>  
>  	policy->fast_switch_possible = false;
>  
> @@ -2398,6 +2421,8 @@ static struct cpufreq_driver intel_pstate = {
>  	.init		= intel_pstate_cpu_init,
>  	.exit		= intel_pstate_cpu_exit,
>  	.stop_cpu	= intel_pstate_stop_cpu,
> +	.offline	= intel_pstate_cpu_offline,
> +	.online		= intel_pstate_cpu_online,
>  	.update_limits	= intel_pstate_update_limits,
>  	.name		= "intel_pstate",
>  };
> @@ -2652,7 +2677,8 @@ static struct cpufreq_driver intel_cpufreq = {
>  	.fast_switch	= intel_cpufreq_fast_switch,
>  	.init		= intel_cpufreq_cpu_init,
>  	.exit		= intel_cpufreq_cpu_exit,
> -	.stop_cpu	= intel_cpufreq_stop_cpu,
> +	.offline	= intel_pstate_cpu_offline,
> +	.online		= intel_pstate_cpu_online,
>  	.update_limits	= intel_pstate_update_limits,
>  	.name		= "intel_cpufreq",
>  };

Re: [PATCH v2 3/5] cpufreq: intel_pstate: Add ->offline and ->online callbacks

[Doug Smythies]

On Fri, Nov 3, 2023 at 3:57 AM Jinjie Ruan <ruanjinjie@huawei.com> wrote:
> On 2020/8/25 1:43, Rafael J. Wysocki wrote:
> > From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> >
> > Add ->offline and ->online driver callbacks to prepare for taking a
> > CPU offline and to restore its working configuration when it goes
> > back online, respectively, to avoid invoking the ->init callback on
> > every CPU online which is quite a bit of unnecessary overhead.
> >
> > Define ->offline and ->online so that they can be used in the
> > passive mode as well as in the active mode and because ->offline
> > will do the majority of ->stop_cpu work, the passive mode does
> > not need that callback any more, so drop it.
> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >
> > -> v2: Typo fixes and changelog edits (Doug).
> >
> > ---
> >  drivers/cpufreq/intel_pstate.c | 38 ++++++++++++++++++++++++++++------
> >  1 file changed, 32 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> > index 3d18934fa975..98836ac299db 100644
> > --- a/drivers/cpufreq/intel_pstate.c
> > +++ b/drivers/cpufreq/intel_pstate.c
> > @@ -2297,28 +2297,51 @@ static int intel_pstate_verify_policy(struct cpufreq_policy_data *policy)
> >       return 0;
> >  }
> >
> > -static void intel_cpufreq_stop_cpu(struct cpufreq_policy *policy)
> > +static int intel_pstate_cpu_offline(struct cpufreq_policy *policy)
> >  {
> > +     pr_debug("CPU %d going offline\n", policy->cpu);
> > +
> > +     intel_pstate_exit_perf_limits(policy);
> > +
> > +     /*
> > +      * If the CPU is an SMT thread and it goes offline with the performance
> > +      * settings different from the minimum, it will prevent its sibling
> > +      * from getting to lower performance levels, so force the minimum
> > +      * performance on CPU offline to prevent that from happening.
> > +      */
> >       if (hwp_active)
> >               intel_pstate_hwp_force_min_perf(policy->cpu);
> >       else
> >               intel_pstate_set_min_pstate(all_cpu_data[policy->cpu]);
> > +
> > +     return 0;
> > +}
> > +
> > +static int intel_pstate_cpu_online(struct cpufreq_policy *policy)
> > +{
> > +     pr_debug("CPU %d going online\n", policy->cpu);
> > +
> > +     intel_pstate_init_acpi_perf_limits(policy);
> > +
> > +     if (hwp_active)
> > +             wrmsrl_on_cpu(policy->cpu, MSR_HWP_REQUEST,
> > +                           all_cpu_data[policy->cpu]->hwp_req_cached);
> > +
> > +     return 0;
> >  }
>
> On Ice Lake server, there seems a bug when CONFIG_X86_INTEL_PSTATE=y and
> not configure intel_pstate=xxx in command line.
>
> Although the Performance tuner is used, the CPU have the lowest
> frequency in scaling_cur_freq after the CPU goes offline and then goes
> online, running the same infinite loop load.
>
> How to produce:
>
> echo performance > /sys/devices/system/cpu/cpu12/cpufreq/scaling_governor
>
> cat while_true.c
> #include <stdio.h>
> void main(void)
> {
>         while(1);
> }
>
>
> [root@localhost freq_test]# cat test.sh
> #!/bin/bash
>
> cat /sys/devices/system/cpu/cpu${1}/cpufreq/scaling_cur_freq
> cat /sys/devices/system/cpu/cpu${1}/cpufreq/scaling_governor
> taskset -c ${1} ./while_true &
> sleep 1s
>
> cat /sys/devices/system/cpu/cpu${1}/cpufreq/scaling_cur_freq
>
> echo 0 > /sys/devices/system/cpu/cpu${1}/online
>
> sleep 1s
> cat /sys/devices/system/cpu/cpu${1}/cpufreq/scaling_cur_freq
>
> sleep 1s
>
> echo 1 > /sys/devices/system/cpu/cpu${1}/online
> cat /sys/devices/system/cpu/cpu${1}/cpufreq/scaling_cur_freq
>
> taskset -c ${1} ./while_true &
>
> sleep 1s
> cat /sys/devices/system/cpu/cpu${1}/cpufreq/scaling_cur_freq
>
> sleep 1s
> cat /sys/devices/system/cpu/cpu${1}/cpufreq/scaling_cur_freq
>
> sleep 1s
> cat /sys/devices/system/cpu/cpu${1}/cpufreq/scaling_cur_freq
>
>
> [root@localhost freq_test]# sh test.sh 40
> 2300000
> performance
> 2299977
> cat: /sys/devices/system/cpu/cpu40/cpufreq/scaling_cur_freq: Device or
> resource busy
> 2300000
> 2300022
> 2300000
> 2299953
> [root@localhost freq_test]# sh test.sh 50
> 2300000
> performance
> 2300000
> cat: /sys/devices/system/cpu/cpu50/cpufreq/scaling_cur_freq: Device or
> resource busy
> 2300000
> 2299977
> 2300022
> 2299977
> [root@localhost freq_test]# sh test.sh 20
> 2300000
> performance
> 2299977
> cat: /sys/devices/system/cpu/cpu20/cpufreq/scaling_cur_freq: Device or
> resource busy
> 800000
> 800000
> 800000
> 799992
> [root@localhost freq_test]# sh test.sh 21
> 2300000
> performance
> 2300000
> cat: /sys/devices/system/cpu/cpu21/cpufreq/scaling_cur_freq: Device or
> resource busy
> 800000
> 800000
> 800000
> 800000
>
> [root@localhost freq_test]# cat
> /sys/devices/system/cpu/cpu21/cpufreq/scaling_max_freq
> 2300000
> [root@localhost freq_test]# cat
> /sys/devices/system/cpu/cpu21/cpufreq/scaling_min_freq
> 800000

Hi,

I followed your "how to reproduce" notes exactly.
So far, I have been unable to reproduce your issue.

I am using kernel 6.6.
My processor is:
Intel(R) Core(TM) i5-10600K CPU @ 4.10GHz

Results:
root@s19:/home/doug/pstate# ./test.sh 8
800000
performance
4799994
cat: /sys/devices/system/cpu/cpu8/cpufreq/scaling_cur_freq: Device or
resource busy
4799999
4800000
4800001
4799996
root@s19:/home/doug/pstate# ./test.sh 7
800000
performance
4800001
cat: /sys/devices/system/cpu/cpu7/cpufreq/scaling_cur_freq: Device or
resource busy
4799967
4800028
4800006
4799997
root@s19:/home/doug/pstate# ./test.sh 6
800000
performance
4800001
cat: /sys/devices/system/cpu/cpu6/cpufreq/scaling_cur_freq: Device or
resource busy
4799983
4800001
4799993
4800002
root@s19:/home/doug/pstate# ./test.sh 5
800000
performance
4799990
cat: /sys/devices/system/cpu/cpu5/cpufreq/scaling_cur_freq: Device or
resource busy
4800006
4800002
4800011
4799980
root@s19:/home/doug/pstate# ./test.sh 4
4799940
performance
4799985
cat: /sys/devices/system/cpu/cpu4/cpufreq/scaling_cur_freq: Device or
resource busy
4799975
4799994
4799984
4799996
root@s19:/home/doug/pstate# ./test.sh 3
4799986
performance
4799990
cat: /sys/devices/system/cpu/cpu3/cpufreq/scaling_cur_freq: Device or
resource busy
4799976
4800015
4800000
4799995

... Doug

Re: [PATCH v2 3/5] cpufreq: intel_pstate: Add ->offline and ->online callbacks

[Jinjie Ruan]

On 2023/11/3 23:56, Doug Smythies wrote:
> On Fri, Nov 3, 2023 at 3:57 AM Jinjie Ruan <ruanjinjie@huawei.com> wrote:
>> On 2020/8/25 1:43, Rafael J. Wysocki wrote:
>>> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
>>>
>>> Add ->offline and ->online driver callbacks to prepare for taking a
>>> CPU offline and to restore its working configuration when it goes
>>> back online, respectively, to avoid invoking the ->init callback on
>>> every CPU online which is quite a bit of unnecessary overhead.
>>>
>>> Define ->offline and ->online so that they can be used in the
>>> passive mode as well as in the active mode and because ->offline
>>> will do the majority of ->stop_cpu work, the passive mode does
>>> not need that callback any more, so drop it.
>>>
>>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>> ---
>>>
>>> -> v2: Typo fixes and changelog edits (Doug).
>>>
>>> ---
>>>  drivers/cpufreq/intel_pstate.c | 38 ++++++++++++++++++++++++++++------
>>>  1 file changed, 32 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
>>> index 3d18934fa975..98836ac299db 100644
>>> --- a/drivers/cpufreq/intel_pstate.c
>>> +++ b/drivers/cpufreq/intel_pstate.c
>>> @@ -2297,28 +2297,51 @@ static int intel_pstate_verify_policy(struct cpufreq_policy_data *policy)
>>>       return 0;
>>>  }
>>>
>>> -static void intel_cpufreq_stop_cpu(struct cpufreq_policy *policy)
>>> +static int intel_pstate_cpu_offline(struct cpufreq_policy *policy)
>>>  {
>>> +     pr_debug("CPU %d going offline\n", policy->cpu);
>>> +
>>> +     intel_pstate_exit_perf_limits(policy);
>>> +
>>> +     /*
>>> +      * If the CPU is an SMT thread and it goes offline with the performance
>>> +      * settings different from the minimum, it will prevent its sibling
>>> +      * from getting to lower performance levels, so force the minimum
>>> +      * performance on CPU offline to prevent that from happening.
>>> +      */
>>>       if (hwp_active)
>>>               intel_pstate_hwp_force_min_perf(policy->cpu);
>>>       else
>>>               intel_pstate_set_min_pstate(all_cpu_data[policy->cpu]);
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +static int intel_pstate_cpu_online(struct cpufreq_policy *policy)
>>> +{
>>> +     pr_debug("CPU %d going online\n", policy->cpu);
>>> +
>>> +     intel_pstate_init_acpi_perf_limits(policy);
>>> +
>>> +     if (hwp_active)
>>> +             wrmsrl_on_cpu(policy->cpu, MSR_HWP_REQUEST,
>>> +                           all_cpu_data[policy->cpu]->hwp_req_cached);
>>> +
>>> +     return 0;
>>>  }
>>
>> On Ice Lake server, there seems a bug when CONFIG_X86_INTEL_PSTATE=y and
>> not configure intel_pstate=xxx in command line.
>>
>> Although the Performance tuner is used, the CPU have the lowest
>> frequency in scaling_cur_freq after the CPU goes offline and then goes
>> online, running the same infinite loop load.
>>
>> How to produce:
>>
>> echo performance > /sys/devices/system/cpu/cpu12/cpufreq/scaling_governor
>>
>> cat while_true.c
>> #include <stdio.h>
>> void main(void)
>> {
>>         while(1);
>> }
>>
>>
>> [root@localhost freq_test]# cat test.sh
>> #!/bin/bash
>>
>> cat /sys/devices/system/cpu/cpu${1}/cpufreq/scaling_cur_freq
>> cat /sys/devices/system/cpu/cpu${1}/cpufreq/scaling_governor
>> taskset -c ${1} ./while_true &
>> sleep 1s
>>
>> cat /sys/devices/system/cpu/cpu${1}/cpufreq/scaling_cur_freq
>>
>> echo 0 > /sys/devices/system/cpu/cpu${1}/online
>>
>> sleep 1s
>> cat /sys/devices/system/cpu/cpu${1}/cpufreq/scaling_cur_freq
>>
>> sleep 1s
>>
>> echo 1 > /sys/devices/system/cpu/cpu${1}/online
>> cat /sys/devices/system/cpu/cpu${1}/cpufreq/scaling_cur_freq
>>
>> taskset -c ${1} ./while_true &
>>
>> sleep 1s
>> cat /sys/devices/system/cpu/cpu${1}/cpufreq/scaling_cur_freq
>>
>> sleep 1s
>> cat /sys/devices/system/cpu/cpu${1}/cpufreq/scaling_cur_freq
>>
>> sleep 1s
>> cat /sys/devices/system/cpu/cpu${1}/cpufreq/scaling_cur_freq
>>
>>
>> [root@localhost freq_test]# sh test.sh 40
>> 2300000
>> performance
>> 2299977
>> cat: /sys/devices/system/cpu/cpu40/cpufreq/scaling_cur_freq: Device or
>> resource busy
>> 2300000
>> 2300022
>> 2300000
>> 2299953
>> [root@localhost freq_test]# sh test.sh 50
>> 2300000
>> performance
>> 2300000
>> cat: /sys/devices/system/cpu/cpu50/cpufreq/scaling_cur_freq: Device or
>> resource busy
>> 2300000
>> 2299977
>> 2300022
>> 2299977
>> [root@localhost freq_test]# sh test.sh 20
>> 2300000
>> performance
>> 2299977
>> cat: /sys/devices/system/cpu/cpu20/cpufreq/scaling_cur_freq: Device or
>> resource busy
>> 800000
>> 800000
>> 800000
>> 799992
>> [root@localhost freq_test]# sh test.sh 21
>> 2300000
>> performance
>> 2300000
>> cat: /sys/devices/system/cpu/cpu21/cpufreq/scaling_cur_freq: Device or
>> resource busy
>> 800000
>> 800000
>> 800000
>> 800000
>>
>> [root@localhost freq_test]# cat
>> /sys/devices/system/cpu/cpu21/cpufreq/scaling_max_freq
>> 2300000
>> [root@localhost freq_test]# cat
>> /sys/devices/system/cpu/cpu21/cpufreq/scaling_min_freq
>> 800000
> 
> Hi,
> 
> I followed your "how to reproduce" notes exactly.
> So far, I have been unable to reproduce your issue.

It seems that this issue is platform-specific.

The following CPU family has the issue:

1、Products formerly Haswell

2、Model name:            Intel(R) Xeon(R) Platinum 8380 CPU @
2.30GHz(Ice Lake server)

But the following CPU family do not have the issue:

1、Model name:                         Intel(R) Xeon(R) CPU E5-2620 v2 @
2.10GHz

2、Model name:     Intel(R) Xeon(R) CPU E5-2698 v3 @ 2.30GHz

> 
> I am using kernel 6.6.
> My processor is:
> Intel(R) Core(TM) i5-10600K CPU @ 4.10GHz
> 
> Results:
> root@s19:/home/doug/pstate# ./test.sh 8
> 800000
> performance
> 4799994
> cat: /sys/devices/system/cpu/cpu8/cpufreq/scaling_cur_freq: Device or
> resource busy
> 4799999
> 4800000
> 4800001
> 4799996
> root@s19:/home/doug/pstate# ./test.sh 7
> 800000
> performance
> 4800001
> cat: /sys/devices/system/cpu/cpu7/cpufreq/scaling_cur_freq: Device or
> resource busy
> 4799967
> 4800028
> 4800006
> 4799997
> root@s19:/home/doug/pstate# ./test.sh 6
> 800000
> performance
> 4800001
> cat: /sys/devices/system/cpu/cpu6/cpufreq/scaling_cur_freq: Device or
> resource busy
> 4799983
> 4800001
> 4799993
> 4800002
> root@s19:/home/doug/pstate# ./test.sh 5
> 800000
> performance
> 4799990
> cat: /sys/devices/system/cpu/cpu5/cpufreq/scaling_cur_freq: Device or
> resource busy
> 4800006
> 4800002
> 4800011
> 4799980
> root@s19:/home/doug/pstate# ./test.sh 4
> 4799940
> performance
> 4799985
> cat: /sys/devices/system/cpu/cpu4/cpufreq/scaling_cur_freq: Device or
> resource busy
> 4799975
> 4799994
> 4799984
> 4799996
> root@s19:/home/doug/pstate# ./test.sh 3
> 4799986
> performance
> 4799990
> cat: /sys/devices/system/cpu/cpu3/cpufreq/scaling_cur_freq: Device or
> resource busy
> 4799976
> 4800015
> 4800000
> 4799995
> 
> ... Doug
>