[PATCH 0/5] powernow-k8: Core Performance Boost and effective frequency support

[PATCH 0/5] powernow-k8: Core Performance Boost and effective frequency support

[Borislav Petkov]

From: Borislav Petkov <borislav.petkov@amd.com>

Hi all,

here's a small patchset that implements two new performance-related
features on AMD: Core Performance Boost and Effective Frequency
reporting (APERF/MPERF). Details for each are to be found in the
respective commit messages, as Andrew requested :).

 5/5 overloads the /sys/devices/system/cpu/cpu3/cpufreq/scaling_cur_freq
interface to show the actual core frequency when boosting. Let me know
if the way it is done is agreeable.

Thanks,
Boris.

[PATCH 1/5] cpufreq: Unify sysfs attribute definition macros

[Borislav Petkov]

From: Borislav Petkov <borislav.petkov@amd.com>

Multiple modules used to define those which are with identical
functionality and were needlessly replicated among the different cpufreq
drivers. Push them into the header and remove duplication.

Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
---
 drivers/cpufreq/cpufreq.c              |   40 +++++++++-----------------
 drivers/cpufreq/cpufreq_conservative.c |   48 ++++++++++---------------------
 drivers/cpufreq/cpufreq_ondemand.c     |   40 ++++++++------------------
 include/linux/cpufreq.h                |   30 ++++++++++++++++++++
 4 files changed, 72 insertions(+), 86 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 2d5d575..6bfea8d 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -662,32 +662,20 @@ static ssize_t show_bios_limit(struct cpufreq_policy *policy, char *buf)
 	return sprintf(buf, "%u\n", policy->cpuinfo.max_freq);
 }
 
-#define define_one_ro(_name) \
-static struct freq_attr _name = \
-__ATTR(_name, 0444, show_##_name, NULL)
-
-#define define_one_ro0400(_name) \
-static struct freq_attr _name = \
-__ATTR(_name, 0400, show_##_name, NULL)
-
-#define define_one_rw(_name) \
-static struct freq_attr _name = \
-__ATTR(_name, 0644, show_##_name, store_##_name)
-
-define_one_ro0400(cpuinfo_cur_freq);
-define_one_ro(cpuinfo_min_freq);
-define_one_ro(cpuinfo_max_freq);
-define_one_ro(cpuinfo_transition_latency);
-define_one_ro(scaling_available_governors);
-define_one_ro(scaling_driver);
-define_one_ro(scaling_cur_freq);
-define_one_ro(bios_limit);
-define_one_ro(related_cpus);
-define_one_ro(affected_cpus);
-define_one_rw(scaling_min_freq);
-define_one_rw(scaling_max_freq);
-define_one_rw(scaling_governor);
-define_one_rw(scaling_setspeed);
+define_one_freq_ro_perm(cpuinfo_cur_freq, 0400);
+define_one_freq_ro(cpuinfo_min_freq);
+define_one_freq_ro(cpuinfo_max_freq);
+define_one_freq_ro(cpuinfo_transition_latency);
+define_one_freq_ro(scaling_available_governors);
+define_one_freq_ro(scaling_driver);
+define_one_freq_ro(scaling_cur_freq);
+define_one_freq_ro(bios_limit);
+define_one_freq_ro(related_cpus);
+define_one_freq_ro(affected_cpus);
+define_one_freq_rw(scaling_min_freq);
+define_one_freq_rw(scaling_max_freq);
+define_one_freq_rw(scaling_governor);
+define_one_freq_rw(scaling_setspeed);
 
 static struct attribute *default_attrs[] = {
 	&cpuinfo_min_freq.attr,
diff --git a/drivers/cpufreq/cpufreq_conservative.c b/drivers/cpufreq/cpufreq_conservative.c
index 599a40b..0fddbca 100644
--- a/drivers/cpufreq/cpufreq_conservative.c
+++ b/drivers/cpufreq/cpufreq_conservative.c
@@ -178,12 +178,8 @@ static ssize_t show_sampling_rate_min(struct kobject *kobj,
 	return sprintf(buf, "%u\n", min_sampling_rate);
 }
 
-#define define_one_ro(_name)		\
-static struct global_attr _name =	\
-__ATTR(_name, 0444, show_##_name, NULL)
-
-define_one_ro(sampling_rate_max);
-define_one_ro(sampling_rate_min);
+define_one_global_ro(sampling_rate_max);
+define_one_global_ro(sampling_rate_min);
 
 /* cpufreq_conservative Governor Tunables */
 #define show_one(file_name, object)					\
@@ -221,12 +217,8 @@ show_one_old(freq_step);
 show_one_old(sampling_rate_min);
 show_one_old(sampling_rate_max);
 
-#define define_one_ro_old(object, _name)	\
-static struct freq_attr object =		\
-__ATTR(_name, 0444, show_##_name##_old, NULL)
-
-define_one_ro_old(sampling_rate_min_old, sampling_rate_min);
-define_one_ro_old(sampling_rate_max_old, sampling_rate_max);
+define_one_freq_ro_old(sampling_rate_min);
+define_one_freq_ro_old(sampling_rate_max);
 
 /*** delete after deprecation time ***/
 
@@ -364,16 +356,12 @@ static ssize_t store_freq_step(struct kobject *a, struct attribute *b,
 	return count;
 }
 
-#define define_one_rw(_name) \
-static struct global_attr _name = \
-__ATTR(_name, 0644, show_##_name, store_##_name)
-
-define_one_rw(sampling_rate);
-define_one_rw(sampling_down_factor);
-define_one_rw(up_threshold);
-define_one_rw(down_threshold);
-define_one_rw(ignore_nice_load);
-define_one_rw(freq_step);
+define_one_global_rw(sampling_rate);
+define_one_global_rw(sampling_down_factor);
+define_one_global_rw(up_threshold);
+define_one_global_rw(down_threshold);
+define_one_global_rw(ignore_nice_load);
+define_one_global_rw(freq_step);
 
 static struct attribute *dbs_attributes[] = {
 	&sampling_rate_max.attr,
@@ -409,16 +397,12 @@ write_one_old(down_threshold);
 write_one_old(ignore_nice_load);
 write_one_old(freq_step);
 
-#define define_one_rw_old(object, _name)	\
-static struct freq_attr object =		\
-__ATTR(_name, 0644, show_##_name##_old, store_##_name##_old)
-
-define_one_rw_old(sampling_rate_old, sampling_rate);
-define_one_rw_old(sampling_down_factor_old, sampling_down_factor);
-define_one_rw_old(up_threshold_old, up_threshold);
-define_one_rw_old(down_threshold_old, down_threshold);
-define_one_rw_old(ignore_nice_load_old, ignore_nice_load);
-define_one_rw_old(freq_step_old, freq_step);
+define_one_freq_rw_old(sampling_rate);
+define_one_freq_rw_old(sampling_down_factor);
+define_one_freq_rw_old(up_threshold);
+define_one_freq_rw_old(down_threshold);
+define_one_freq_rw_old(ignore_nice_load);
+define_one_freq_rw_old(freq_step);
 
 static struct attribute *dbs_attributes_old[] = {
 	&sampling_rate_max_old.attr,
diff --git a/drivers/cpufreq/cpufreq_ondemand.c b/drivers/cpufreq/cpufreq_ondemand.c
index bd444dc..cb07f48 100644
--- a/drivers/cpufreq/cpufreq_ondemand.c
+++ b/drivers/cpufreq/cpufreq_ondemand.c
@@ -234,12 +234,8 @@ static ssize_t show_sampling_rate_min(struct kobject *kobj,
 	return sprintf(buf, "%u\n", min_sampling_rate);
 }
 
-#define define_one_ro(_name)		\
-static struct global_attr _name =	\
-__ATTR(_name, 0444, show_##_name, NULL)
-
-define_one_ro(sampling_rate_max);
-define_one_ro(sampling_rate_min);
+define_one_global_ro(sampling_rate_max);
+define_one_global_ro(sampling_rate_min);
 
 /* cpufreq_ondemand Governor Tunables */
 #define show_one(file_name, object)					\
@@ -274,12 +270,8 @@ show_one_old(powersave_bias);
 show_one_old(sampling_rate_min);
 show_one_old(sampling_rate_max);
 
-#define define_one_ro_old(object, _name)       \
-static struct freq_attr object =               \
-__ATTR(_name, 0444, show_##_name##_old, NULL)
-
-define_one_ro_old(sampling_rate_min_old, sampling_rate_min);
-define_one_ro_old(sampling_rate_max_old, sampling_rate_max);
+define_one_freq_ro_old(sampling_rate_min);
+define_one_freq_ro_old(sampling_rate_max);
 
 /*** delete after deprecation time ***/
 
@@ -376,14 +368,10 @@ static ssize_t store_powersave_bias(struct kobject *a, struct attribute *b,
 	return count;
 }
 
-#define define_one_rw(_name) \
-static struct global_attr _name = \
-__ATTR(_name, 0644, show_##_name, store_##_name)
-
-define_one_rw(sampling_rate);
-define_one_rw(up_threshold);
-define_one_rw(ignore_nice_load);
-define_one_rw(powersave_bias);
+define_one_global_rw(sampling_rate);
+define_one_global_rw(up_threshold);
+define_one_global_rw(ignore_nice_load);
+define_one_global_rw(powersave_bias);
 
 static struct attribute *dbs_attributes[] = {
 	&sampling_rate_max.attr,
@@ -415,14 +403,10 @@ write_one_old(up_threshold);
 write_one_old(ignore_nice_load);
 write_one_old(powersave_bias);
 
-#define define_one_rw_old(object, _name)       \
-static struct freq_attr object =               \
-__ATTR(_name, 0644, show_##_name##_old, store_##_name##_old)
-
-define_one_rw_old(sampling_rate_old, sampling_rate);
-define_one_rw_old(up_threshold_old, up_threshold);
-define_one_rw_old(ignore_nice_load_old, ignore_nice_load);
-define_one_rw_old(powersave_bias_old, powersave_bias);
+define_one_freq_rw_old(sampling_rate);
+define_one_freq_rw_old(up_threshold);
+define_one_freq_rw_old(ignore_nice_load);
+define_one_freq_rw_old(powersave_bias);
 
 static struct attribute *dbs_attributes_old[] = {
        &sampling_rate_max_old.attr,
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 4de02b1..c8d731c 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -278,6 +278,27 @@ struct freq_attr {
 	ssize_t (*store)(struct cpufreq_policy *, const char *, size_t count);
 };
 
+#define define_one_freq_ro(_name)		\
+static struct freq_attr _name =			\
+__ATTR(_name, 0444, show_##_name, NULL)
+
+#define define_one_freq_ro_perm(_name, _perm)	\
+static struct freq_attr _name =			\
+__ATTR(_name, _perm, show_##_name, NULL)
+
+#define define_one_freq_ro_old(_name)		\
+static struct freq_attr _name##_old =		\
+__ATTR(_name, 0444, show_##_name##_old, NULL)
+
+#define define_one_freq_rw(_name)		\
+static struct freq_attr _name =			\
+__ATTR(_name, 0644, show_##_name, store_##_name)
+
+#define define_one_freq_rw_old(_name)		\
+static struct freq_attr _name##_old =		\
+__ATTR(_name, 0644, show_##_name##_old, store_##_name##_old)
+
+
 struct global_attr {
 	struct attribute attr;
 	ssize_t (*show)(struct kobject *kobj,
@@ -286,6 +307,15 @@ struct global_attr {
 			 const char *c, size_t count);
 };
 
+#define define_one_global_ro(_name)		\
+static struct global_attr _name =		\
+__ATTR(_name, 0444, show_##_name, NULL)
+
+#define define_one_global_rw(_name)		\
+static struct global_attr _name =		\
+__ATTR(_name, 0644, show_##_name, store_##_name)
+
+
 /*********************************************************************
  *                        CPUFREQ 2.6. INTERFACE                     *
  *********************************************************************/
-- 
1.7.0

[PATCH 2/5] powernow-k8: Add core performance boost support

[Borislav Petkov]

From: Borislav Petkov <borislav.petkov@amd.com>

Starting with F10h, revE, AMD processors add support for a dynamic
core boosting feature called Core Performance Boost. When a specific
condition is present, a subset of the cores on a system are boosted
beyond their P0 operating frequency to speed up the performance of
single-threaded applications.

In the normal case, the system comes out of reset with core boosting
enabled. This patch adds a sysfs knob with which core boosting can be
switched on or off for benchmarking purposes.

While at it, cleanup the driver init codepath and update copyrights.

Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
---
 arch/x86/kernel/cpu/cpufreq/powernow-k8.c |  115 ++++++++++++++++++++++++++---
 arch/x86/kernel/cpu/cpufreq/powernow-k8.h |    3 +-
 2 files changed, 106 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kernel/cpu/cpufreq/powernow-k8.c b/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
index d360b56..90fda2c 100644
--- a/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
+++ b/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
@@ -1,6 +1,5 @@
-
 /*
- *   (c) 2003-2006 Advanced Micro Devices, Inc.
+ *   (c) 2003-2010 Advanced Micro Devices, Inc.
  *  Your use of this code is subject to the terms and conditions of the
  *  GNU general public license version 2. See "COPYING" or
  *  http://www.gnu.org/licenses/gpl.html
@@ -54,6 +53,10 @@ static DEFINE_PER_CPU(struct powernow_k8_data *, powernow_data);
 
 static int cpu_family = CPU_OPTERON;
 
+/* core performance boost */
+static bool cpb_capable, cpb_disabled;
+static struct msr __percpu *msrs;
+
 #ifndef CONFIG_SMP
 static inline const struct cpumask *cpu_core_mask(int cpu)
 {
@@ -1393,8 +1396,69 @@ out:
 	return khz;
 }
 
+static void _cpb_toggle_msrs(bool t)
+{
+	int cpu;
+
+	rdmsr_on_cpus(cpu_online_mask, MSR_K7_HWCR, msrs);
+
+	for_each_cpu(cpu, cpu_online_mask) {
+		struct msr *reg = per_cpu_ptr(msrs, cpu);
+		if (t)
+			reg->l &= ~BIT(25);
+		else
+			reg->l |= BIT(25);
+	}
+	wrmsr_on_cpus(cpu_online_mask, MSR_K7_HWCR, msrs);
+}
+
+/*
+ * Switch on/off core performance boosting.
+ *
+ * 0=disable
+ * 1=enable.
+ */
+static void cpb_toggle(bool t)
+{
+	if (!cpb_capable)
+		return;
+
+	if (t && cpb_disabled) {
+		cpb_disabled = false;
+		_cpb_toggle_msrs(t);
+		printk(KERN_INFO PFX "Core Boosting enabled.\n");
+	} else if (!(t || cpb_disabled)) {
+		cpb_disabled = true;
+		_cpb_toggle_msrs(t);
+		printk(KERN_INFO PFX "Core Boosting disabled.\n");
+	}
+}
+
+static ssize_t store_cpb(struct cpufreq_policy *policy, const char *buf,
+				 size_t count)
+{
+	int ret = -EINVAL;
+	unsigned long val = 0;
+
+	ret = strict_strtoul(buf, 10, &val);
+	if (!ret && (val == 0 || val == 1) && cpb_capable)
+		cpb_toggle(val);
+	else
+		return -EINVAL;
+
+	return count;
+}
+
+static ssize_t show_cpb(struct cpufreq_policy *policy, char *buf)
+{
+	return sprintf(buf, "%u\n", !cpb_disabled);
+}
+
+define_one_freq_rw(cpb);
+
 static struct freq_attr *powernow_k8_attr[] = {
 	&cpufreq_freq_attr_scaling_available_freqs,
+	&cpb,
 	NULL,
 };
 
@@ -1410,10 +1474,17 @@ static struct cpufreq_driver cpufreq_amd64_driver = {
 	.attr		= powernow_k8_attr,
 };
 
+static bool __cpuinit check_cpb_capable(void)
+{
+	u32 tmp = cpuid_edx(CPUID_FREQ_VOLT_CAPABILITIES);
+
+	return (tmp & CPB_CAPABLE);
+}
+
 /* driver entry point for init */
 static int __cpuinit powernowk8_init(void)
 {
-	unsigned int i, supported_cpus = 0;
+	unsigned int i, supported_cpus = 0, cpu;
 
 	for_each_online_cpu(i) {
 		int rc;
@@ -1422,15 +1493,36 @@ static int __cpuinit powernowk8_init(void)
 			supported_cpus++;
 	}
 
-	if (supported_cpus == num_online_cpus()) {
-		printk(KERN_INFO PFX "Found %d %s "
-			"processors (%d cpu cores) (" VERSION ")\n",
-			num_online_nodes(),
-			boot_cpu_data.x86_model_id, supported_cpus);
-		return cpufreq_register_driver(&cpufreq_amd64_driver);
+	if (supported_cpus != num_online_cpus())
+		return -ENODEV;
+
+	printk(KERN_INFO PFX "Found %d %s (%d cpu cores) (" VERSION ")\n",
+		num_online_nodes(), boot_cpu_data.x86_model_id, supported_cpus);
+
+	cpb_capable = check_cpb_capable();
+	if (cpb_capable) {
+
+		msrs = msrs_alloc();
+		if (!msrs) {
+			printk(KERN_ERR "%s: Error allocating msrs!\n", __func__);
+			return -ENOMEM;
+		}
+
+		rdmsr_on_cpus(cpu_online_mask, MSR_K7_HWCR, msrs);
+
+		for_each_cpu(cpu, cpu_online_mask) {
+			struct msr *reg = per_cpu_ptr(msrs, cpu);
+			cpb_disabled |= !!(reg->l & BIT(25));
+		}
+
+		printk(KERN_INFO PFX "Core Performance Boosting: ");
+		if (cpb_disabled)
+			printk(KERN_CONT "off.\n");
+		else
+			printk(KERN_CONT "on.\n");
 	}
 
-	return -ENODEV;
+	return cpufreq_register_driver(&cpufreq_amd64_driver);
 }
 
 /* driver entry point for term */
@@ -1438,6 +1530,9 @@ static void __exit powernowk8_exit(void)
 {
 	dprintk("exit\n");
 
+	msrs_free(msrs);
+	msrs = NULL;
+
 	cpufreq_unregister_driver(&cpufreq_amd64_driver);
 }
 
diff --git a/arch/x86/kernel/cpu/cpufreq/powernow-k8.h b/arch/x86/kernel/cpu/cpufreq/powernow-k8.h
index 02ce824..3be308e 100644
--- a/arch/x86/kernel/cpu/cpufreq/powernow-k8.h
+++ b/arch/x86/kernel/cpu/cpufreq/powernow-k8.h
@@ -5,7 +5,6 @@
  *  http://www.gnu.org/licenses/gpl.html
  */
 
-
 enum pstate {
 	HW_PSTATE_INVALID = 0xff,
 	HW_PSTATE_0 = 0,
@@ -55,7 +54,6 @@ struct powernow_k8_data {
 	struct cpumask *available_cores;
 };
 
-
 /* processor's cpuid instruction support */
 #define CPUID_PROCESSOR_SIGNATURE	1	/* function 1 */
 #define CPUID_XFAM			0x0ff00000	/* extended family */
@@ -67,6 +65,7 @@ struct powernow_k8_data {
 #define CPUID_GET_MAX_CAPABILITIES	0x80000000
 #define CPUID_FREQ_VOLT_CAPABILITIES	0x80000007
 #define P_STATE_TRANSITION_CAPABLE	6
+#define CPB_CAPABLE			0x00000200
 
 /* Model Specific Registers for p-state transitions. MSRs are 64-bit. For     */
 /* writes (wrmsr - opcode 0f 30), the register number is placed in ecx, and   */
-- 
1.7.0

[PATCH 3/5] cpufreq: Add APERF/MPERF support for AMD processors

[Borislav Petkov]

From: Mark Langsdorf <mark.langsdorf@amd.com>

Starting with model 10 of Family 0x10, AMD processors may have
support for APERF/MPERF.  Add support for identifying it and using
it within cpufreq.  Move the APERF/MPERF functions out of the
acpi-cpufreq code and into their own file so they can easily be
shared.

Signed-off-by: Mark Langsdorf <mark.langsdorf@amd.com>
Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
---
 arch/x86/kernel/cpu/amd.c                  |    6 +++
 arch/x86/kernel/cpu/cpufreq/Makefile       |    4 +-
 arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c |   44 +-----------------------
 arch/x86/kernel/cpu/cpufreq/mperf.c        |   50 ++++++++++++++++++++++++++++
 arch/x86/kernel/cpu/cpufreq/mperf.h        |    9 +++++
 arch/x86/kernel/cpu/cpufreq/powernow-k8.c  |    8 ++++
 6 files changed, 77 insertions(+), 44 deletions(-)
 create mode 100644 arch/x86/kernel/cpu/cpufreq/mperf.c
 create mode 100644 arch/x86/kernel/cpu/cpufreq/mperf.h

diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index e485825..796f662 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -537,6 +537,12 @@ static void __cpuinit init_amd(struct cpuinfo_x86 *c)
 		set_cpu_cap(c, X86_FEATURE_MFENCE_RDTSC);
 	}
 
+	if (c->cpuid_level >= 6) {
+		unsigned ecx = cpuid_ecx(6);
+		if (ecx & 0x01)
+			set_cpu_cap(c, X86_FEATURE_APERFMPERF);
+	}
+
 #ifdef CONFIG_X86_64
 	if (c->x86 == 0x10) {
 		/* do this for boot cpu */
diff --git a/arch/x86/kernel/cpu/cpufreq/Makefile b/arch/x86/kernel/cpu/cpufreq/Makefile
index 1840c0a..bd54bf6 100644
--- a/arch/x86/kernel/cpu/cpufreq/Makefile
+++ b/arch/x86/kernel/cpu/cpufreq/Makefile
@@ -2,8 +2,8 @@
 # K8 systems. ACPI is preferred to all other hardware-specific drivers.
 # speedstep-* is preferred over p4-clockmod.
 
-obj-$(CONFIG_X86_POWERNOW_K8)		+= powernow-k8.o
-obj-$(CONFIG_X86_ACPI_CPUFREQ)		+= acpi-cpufreq.o
+obj-$(CONFIG_X86_POWERNOW_K8)		+= powernow-k8.o mperf.o
+obj-$(CONFIG_X86_ACPI_CPUFREQ)		+= acpi-cpufreq.o mperf.o
 obj-$(CONFIG_X86_PCC_CPUFREQ)		+= pcc-cpufreq.o
 obj-$(CONFIG_X86_POWERNOW_K6)		+= powernow-k6.o
 obj-$(CONFIG_X86_POWERNOW_K7)		+= powernow-k7.o
diff --git a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
index 1b1920f..dc68e5c 100644
--- a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
+++ b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
@@ -45,6 +45,7 @@
 #include <asm/msr.h>
 #include <asm/processor.h>
 #include <asm/cpufeature.h>
+#include "mperf.h"
 
 #define dprintk(msg...) cpufreq_debug_printk(CPUFREQ_DEBUG_DRIVER, \
 		"acpi-cpufreq", msg)
@@ -70,8 +71,6 @@ struct acpi_cpufreq_data {
 
 static DEFINE_PER_CPU(struct acpi_cpufreq_data *, acfreq_data);
 
-static DEFINE_PER_CPU(struct aperfmperf, acfreq_old_perf);
-
 /* acpi_perf_data is a pointer to percpu data. */
 static struct acpi_processor_performance *acpi_perf_data;
 
@@ -239,45 +238,6 @@ static u32 get_cur_val(const struct cpumask *mask)
 	return cmd.val;
 }
 
-/* Called via smp_call_function_single(), on the target CPU */
-static void read_measured_perf_ctrs(void *_cur)
-{
-	struct aperfmperf *am = _cur;
-
-	get_aperfmperf(am);
-}
-
-/*
- * Return the measured active (C0) frequency on this CPU since last call
- * to this function.
- * Input: cpu number
- * Return: Average CPU frequency in terms of max frequency (zero on error)
- *
- * We use IA32_MPERF and IA32_APERF MSRs to get the measured performance
- * over a period of time, while CPU is in C0 state.
- * IA32_MPERF counts at the rate of max advertised frequency
- * IA32_APERF counts at the rate of actual CPU frequency
- * Only IA32_APERF/IA32_MPERF ratio is architecturally defined and
- * no meaning should be associated with absolute values of these MSRs.
- */
-static unsigned int get_measured_perf(struct cpufreq_policy *policy,
-				      unsigned int cpu)
-{
-	struct aperfmperf perf;
-	unsigned long ratio;
-	unsigned int retval;
-
-	if (smp_call_function_single(cpu, read_measured_perf_ctrs, &perf, 1))
-		return 0;
-
-	ratio = calc_aperfmperf_ratio(&per_cpu(acfreq_old_perf, cpu), &perf);
-	per_cpu(acfreq_old_perf, cpu) = perf;
-
-	retval = (policy->cpuinfo.max_freq * ratio) >> APERFMPERF_SHIFT;
-
-	return retval;
-}
-
 static unsigned int get_cur_freq_on_cpu(unsigned int cpu)
 {
 	struct acpi_cpufreq_data *data = per_cpu(acfreq_data, cpu);
@@ -701,7 +661,7 @@ static int acpi_cpufreq_cpu_init(struct cpufreq_policy *policy)
 
 	/* Check for APERF/MPERF support in hardware */
 	if (cpu_has(c, X86_FEATURE_APERFMPERF))
-		acpi_cpufreq_driver.getavg = get_measured_perf;
+		acpi_cpufreq_driver.getavg = cpufreq_get_measured_perf;
 
 	dprintk("CPU%u - ACPI performance management activated.\n", cpu);
 	for (i = 0; i < perf->state_count; i++)
diff --git a/arch/x86/kernel/cpu/cpufreq/mperf.c b/arch/x86/kernel/cpu/cpufreq/mperf.c
new file mode 100644
index 0000000..bc5a5df
--- /dev/null
+++ b/arch/x86/kernel/cpu/cpufreq/mperf.c
@@ -0,0 +1,50 @@
+#include <linux/kernel.h>
+#include <linux/smp.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/cpufreq.h>
+#include <linux/slab.h>
+
+#include "mperf.h"
+
+static DEFINE_PER_CPU(struct aperfmperf, acfreq_old_perf);
+
+/* Called via smp_call_function_single(), on the target CPU */
+static void read_measured_perf_ctrs(void *_cur)
+{
+	struct aperfmperf *am = _cur;
+
+	get_aperfmperf(am);
+}
+
+/*
+ * Return the measured active (C0) frequency on this CPU since last call
+ * to this function.
+ * Input: cpu number
+ * Return: Average CPU frequency in terms of max frequency (zero on error)
+ *
+ * We use IA32_MPERF and IA32_APERF MSRs to get the measured performance
+ * over a period of time, while CPU is in C0 state.
+ * IA32_MPERF counts at the rate of max advertised frequency
+ * IA32_APERF counts at the rate of actual CPU frequency
+ * Only IA32_APERF/IA32_MPERF ratio is architecturally defined and
+ * no meaning should be associated with absolute values of these MSRs.
+ */
+unsigned int cpufreq_get_measured_perf(struct cpufreq_policy *policy,
+					unsigned int cpu)
+{
+	struct aperfmperf perf;
+	unsigned long ratio;
+	unsigned int retval;
+
+	if (smp_call_function_single(cpu, read_measured_perf_ctrs, &perf, 1))
+		return 0;
+
+	ratio = calc_aperfmperf_ratio(&per_cpu(acfreq_old_perf, cpu), &perf);
+	per_cpu(acfreq_old_perf, cpu) = perf;
+
+	retval = (policy->cpuinfo.max_freq * ratio) >> APERFMPERF_SHIFT;
+
+	return retval;
+}
+EXPORT_SYMBOL_GPL(cpufreq_get_measured_perf);
diff --git a/arch/x86/kernel/cpu/cpufreq/mperf.h b/arch/x86/kernel/cpu/cpufreq/mperf.h
new file mode 100644
index 0000000..5dbf295
--- /dev/null
+++ b/arch/x86/kernel/cpu/cpufreq/mperf.h
@@ -0,0 +1,9 @@
+/*
+ *  (c) 2010 Advanced Micro Devices, Inc.
+ *  Your use of this code is subject to the terms and conditions of the
+ *  GNU general public license version 2. See "COPYING" or
+ *  http://www.gnu.org/licenses/gpl.html
+ */
+
+unsigned int cpufreq_get_measured_perf(struct cpufreq_policy *policy,
+					unsigned int cpu);
diff --git a/arch/x86/kernel/cpu/cpufreq/powernow-k8.c b/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
index 90fda2c..427ef07 100644
--- a/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
+++ b/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
@@ -45,6 +45,7 @@
 #define PFX "powernow-k8: "
 #define VERSION "version 2.20.00"
 #include "powernow-k8.h"
+#include "mperf.h"
 
 /* serialize freq changes  */
 static DEFINE_MUTEX(fidvid_mutex);
@@ -57,6 +58,8 @@ static int cpu_family = CPU_OPTERON;
 static bool cpb_capable, cpb_disabled;
 static struct msr __percpu *msrs;
 
+static struct cpufreq_driver cpufreq_amd64_driver;
+
 #ifndef CONFIG_SMP
 static inline const struct cpumask *cpu_core_mask(int cpu)
 {
@@ -1251,6 +1254,7 @@ static int __cpuinit powernowk8_cpu_init(struct cpufreq_policy *pol)
 	struct powernow_k8_data *data;
 	struct init_on_cpu init_on_cpu;
 	int rc;
+	struct cpuinfo_x86 *c = &cpu_data(pol->cpu);
 
 	if (!cpu_online(pol->cpu))
 		return -ENODEV;
@@ -1325,6 +1329,10 @@ static int __cpuinit powernowk8_cpu_init(struct cpufreq_policy *pol)
 		return -EINVAL;
 	}
 
+	/* Check for APERF/MPERF support in hardware */
+	if (cpu_has(c, X86_FEATURE_APERFMPERF))
+		cpufreq_amd64_driver.getavg = cpufreq_get_measured_perf;
+
 	cpufreq_frequency_table_get_attr(data->powernow_table, pol->cpu);
 
 	if (cpu_family == CPU_HW_PSTATE)
-- 
1.7.0

[PATCH 4/5] powernow-k8: Fix frequency reporting

[Borislav Petkov]

From: Mark Langsdorf <mark.langsdorf@amd.com>

With F10, model 10, all valid frequencies are in the ACPI _PST table.

Cc: <stable@kernel.org> # 33.x 32.x
Signed-off-by: Mark Langsdorf <mark.langsdorf@amd.com>
Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
---
 arch/x86/kernel/cpu/cpufreq/powernow-k8.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/cpu/cpufreq/powernow-k8.c b/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
index 427ef07..f411250 100644
--- a/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
+++ b/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
@@ -935,7 +935,8 @@ static int fill_powernow_table_pstate(struct powernow_k8_data *data,
 		powernow_table[i].index = index;
 
 		/* Frequency may be rounded for these */
-		if (boot_cpu_data.x86 == 0x10 || boot_cpu_data.x86 == 0x11) {
+		if ((boot_cpu_data.x86 == 0x10 && boot_cpu_data.x86_model < 10)
+				 || boot_cpu_data.x86 == 0x11) {
 			powernow_table[i].frequency =
 				freq_from_fid_did(lo & 0x3f, (lo >> 6) & 7);
 		} else
-- 
1.7.0

[PATCH 5/5] cpufreq: Add support for actual freq

[Borislav Petkov]

From: Borislav Petkov <borislav.petkov@amd.com>

Modify the scaling_cur_freq interface to show the actual core frequency
when boosting is supported and enabled on the core.

Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
---
 arch/x86/kernel/cpu/cpufreq/powernow-k8.c |   18 ++++++++++++++++++
 drivers/cpufreq/cpufreq.c                 |    9 ++++++++-
 include/linux/cpufreq.h                   |    1 +
 3 files changed, 27 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/cpu/cpufreq/powernow-k8.c b/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
index f411250..a36a18c 100644
--- a/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
+++ b/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
@@ -58,6 +58,8 @@ static int cpu_family = CPU_OPTERON;
 static bool cpb_capable, cpb_disabled;
 static struct msr __percpu *msrs;
 
+static bool can_aperf_mperf;
+
 static struct cpufreq_driver cpufreq_amd64_driver;
 
 #ifndef CONFIG_SMP
@@ -1465,6 +1467,18 @@ static ssize_t show_cpb(struct cpufreq_policy *policy, char *buf)
 
 define_one_freq_rw(cpb);
 
+static ssize_t show_actual_freq(struct cpufreq_policy *policy, char *buf)
+{
+	unsigned long freq;
+
+	if (can_aperf_mperf)
+		freq = cpufreq_get_measured_perf(policy, policy->cpu);
+	else
+		freq = policy->cur;
+
+	return sprintf(buf, "%lu\n", freq);
+}
+
 static struct freq_attr *powernow_k8_attr[] = {
 	&cpufreq_freq_attr_scaling_available_freqs,
 	&cpb,
@@ -1478,6 +1492,7 @@ static struct cpufreq_driver cpufreq_amd64_driver = {
 	.init		= powernowk8_cpu_init,
 	.exit		= __devexit_p(powernowk8_cpu_exit),
 	.get		= powernowk8_get,
+	.cur_freq	= show_actual_freq,
 	.name		= "powernow-k8",
 	.owner		= THIS_MODULE,
 	.attr		= powernow_k8_attr,
@@ -1531,6 +1546,9 @@ static int __cpuinit powernowk8_init(void)
 			printk(KERN_CONT "on.\n");
 	}
 
+	if (boot_cpu_has(X86_FEATURE_APERFMPERF))
+		can_aperf_mperf = true;
+
 	return cpufreq_register_driver(&cpufreq_amd64_driver);
 }
 
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 6bfea8d..bf34307 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -457,7 +457,14 @@ show_one(cpuinfo_max_freq, cpuinfo.max_freq);
 show_one(cpuinfo_transition_latency, cpuinfo.transition_latency);
 show_one(scaling_min_freq, min);
 show_one(scaling_max_freq, max);
-show_one(scaling_cur_freq, cur);
+
+static ssize_t show_scaling_cur_freq(struct cpufreq_policy *policy, char *buf)
+{
+	if (cpufreq_driver->cur_freq)
+		return cpufreq_driver->cur_freq(policy, buf);
+
+	return sprintf(buf, "%u\n", policy->cur);
+}
 
 static int __cpufreq_set_policy(struct cpufreq_policy *data,
 				struct cpufreq_policy *policy);
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index c8d731c..5744312 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -237,6 +237,7 @@ struct cpufreq_driver {
 	int	(*exit)		(struct cpufreq_policy *policy);
 	int	(*suspend)	(struct cpufreq_policy *policy, pm_message_t pmsg);
 	int	(*resume)	(struct cpufreq_policy *policy);
+	ssize_t (*cur_freq)	(struct cpufreq_policy *policy, char *buf);
 	struct freq_attr	**attr;
 };
 
-- 
1.7.0

Re: [PATCH 1/5] cpufreq: Unify sysfs attribute definition macros

[Thomas Renninger]

On Monday 22 March 2010 19:38:37 Borislav Petkov wrote:
> From: Borislav Petkov <borislav.petkov@amd.com>
> 
> Multiple modules used to define those which are with identical
> functionality and were needlessly replicated among the different cpufreq
> drivers. Push them into the header and remove duplication.
> 
...
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index 4de02b1..c8d731c 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -278,6 +278,27 @@ struct freq_attr {
>  	ssize_t (*store)(struct cpufreq_policy *, const char *, size_t count);
>  };
...
>  
> +#define define_one_global_ro(_name)		\
> +static struct global_attr _name =		\
> +__ATTR(_name, 0444, show_##_name, NULL)
> +
> +#define define_one_global_rw(_name)		\
> +static struct global_attr _name =		\
> +__ATTR(_name, 0644, show_##_name, store_##_name)

These sound like too general names in global space.
And are unrelated to cpufreq(.h).
Eventually you get them into sysfs.h with another name
or just duplicate them?

   Thomas

Re: [PATCH 2/5] powernow-k8: Add core performance boost support

[Thomas Renninger]

On Monday 22 March 2010 19:38:38 Borislav Petkov wrote:
> From: Borislav Petkov <borislav.petkov@amd.com>
> 
> Starting with F10h, revE, AMD processors add support for a dynamic
> core boosting feature called Core Performance Boost. When a specific
> condition is present, a subset of the cores on a system are boosted
> beyond their P0 operating frequency to speed up the performance of
> single-threaded applications.
> 
> In the normal case, the system comes out of reset with core boosting
> enabled. This patch adds a sysfs knob with which core boosting can be
> switched on or off for benchmarking purposes.
> 
> While at it, cleanup the driver init codepath and update copyrights.
> 
> Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
> ---
>  arch/x86/kernel/cpu/cpufreq/powernow-k8.c |  115 ++++++++++++++++++++++++++---
>  arch/x86/kernel/cpu/cpufreq/powernow-k8.h |    3 +-
>  2 files changed, 106 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/cpufreq/powernow-k8.c b/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
> index d360b56..90fda2c 100644
> --- a/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
> +++ b/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
> @@ -1,6 +1,5 @@
> -
>  /*
> - *   (c) 2003-2006 Advanced Micro Devices, Inc.
> + *   (c) 2003-2010 Advanced Micro Devices, Inc.
>   *  Your use of this code is subject to the terms and conditions of the
>   *  GNU general public license version 2. See "COPYING" or
>   *  http://www.gnu.org/licenses/gpl.html
> @@ -54,6 +53,10 @@ static DEFINE_PER_CPU(struct powernow_k8_data *, powernow_data);
>  
>  static int cpu_family = CPU_OPTERON;
>  
> +/* core performance boost */
> +static bool cpb_capable, cpb_disabled;
Whatabout using a cpufeature (arch/x86/include/asm/cpufeature.h)
instead of cpb_capable. Then people could see this feature in 
/proc/cpuinfo and other code parts could check for it easily if
needed later.
It could already be set in arch/x86/kernel/cpu/amd.c and
powernow-k8 could use cpu_has(cpu, X86_FEATURE_CPB);

Instead of cpb_disabled, I'd use cpb_enabled. Checking for
!cpb_disabled whether it's enabled, is ugly to read.

> +static struct msr __percpu *msrs;
> +
>  #ifndef CONFIG_SMP
>  static inline const struct cpumask *cpu_core_mask(int cpu)
>  {
> @@ -1393,8 +1396,69 @@ out:
>  	return khz;
>  }
>  
> +static void _cpb_toggle_msrs(bool t)
> +{
> +	int cpu;
> +
> +	rdmsr_on_cpus(cpu_online_mask, MSR_K7_HWCR, msrs);
> +
> +	for_each_cpu(cpu, cpu_online_mask) {
> +		struct msr *reg = per_cpu_ptr(msrs, cpu);
> +		if (t)
> +			reg->l &= ~BIT(25);
> +		else
> +			reg->l |= BIT(25);
> +	}
> +	wrmsr_on_cpus(cpu_online_mask, MSR_K7_HWCR, msrs);
> +}
> +
> +/*
> + * Switch on/off core performance boosting.
> + *
> + * 0=disable
> + * 1=enable.
> + */
> +static void cpb_toggle(bool t)
> +{
> +	if (!cpb_capable)
> +		return;
> +
> +	if (t && cpb_disabled) {
> +		cpb_disabled = false;
cpb_enabled = true
is better.
> +		_cpb_toggle_msrs(t);
> +		printk(KERN_INFO PFX "Core Boosting enabled.\n");
Always printk on every toggle?
That should not happen often and a user might want to get noticed if
an app does this behind his back -> should be fine w/ or w/o, just not 
sure whether it's intended.

...

     Thomas

Re: [PATCH 3/5] cpufreq: Add APERF/MPERF support for AMD processors

[Thomas Renninger]

On Monday 22 March 2010 19:38:39 Borislav Petkov wrote:
> From: Mark Langsdorf <mark.langsdorf@amd.com>
> 
> Starting with model 10 of Family 0x10, AMD processors may have
> support for APERF/MPERF.  Add support for identifying it and using
> it within cpufreq.  Move the APERF/MPERF functions out of the
> acpi-cpufreq code and into their own file so they can easily be
> shared.
> 
> Signed-off-by: Mark Langsdorf <mark.langsdorf@amd.com>
> Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
> ---
>  arch/x86/kernel/cpu/amd.c                  |    6 +++
>  arch/x86/kernel/cpu/cpufreq/Makefile       |    4 +-
>  arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c |   44 +-----------------------
>  arch/x86/kernel/cpu/cpufreq/mperf.c        |   50 ++++++++++++++++++++++++++++
>  arch/x86/kernel/cpu/cpufreq/mperf.h        |    9 +++++
>  arch/x86/kernel/cpu/cpufreq/powernow-k8.c  |    8 ++++
>  6 files changed, 77 insertions(+), 44 deletions(-)
>  create mode 100644 arch/x86/kernel/cpu/cpufreq/mperf.c
>  create mode 100644 arch/x86/kernel/cpu/cpufreq/mperf.h
> 
> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> index e485825..796f662 100644
> --- a/arch/x86/kernel/cpu/amd.c
> +++ b/arch/x86/kernel/cpu/amd.c
> @@ -537,6 +537,12 @@ static void __cpuinit init_amd(struct cpuinfo_x86 *c)
>  		set_cpu_cap(c, X86_FEATURE_MFENCE_RDTSC);
>  	}
>  
> +	if (c->cpuid_level >= 6) {
> +		unsigned ecx = cpuid_ecx(6);
> +		if (ecx & 0x01)
> +			set_cpu_cap(c, X86_FEATURE_APERFMPERF);
> +	}
This is nearly identical to (beside c->cpuid_level > 6),
in arch/x86/kernel/cpu/intel.c:
if (c->cpuid_level > 6) {
                unsigned ecx = cpuid_ecx(6);
                if (ecx & 0x01)
                        set_cpu_cap(c, X86_FEATURE_APERFMPERF);
}
I expect you are correct... or could it get moved to general x86 init code?

     Thomas

Re: [PATCH 1/5] cpufreq: Unify sysfs attribute definition macros

[Borislav Petkov]

From: Thomas Renninger <trenn@suse.de>
Date: Tue, Mar 23, 2010 at 12:07:29PM +0100

> > +#define define_one_global_ro(_name)		\
> > +static struct global_attr _name =		\
> > +__ATTR(_name, 0444, show_##_name, NULL)
> > +
> > +#define define_one_global_rw(_name)		\
> > +static struct global_attr _name =		\
> > +__ATTR(_name, 0644, show_##_name, store_##_name)
> 
> These sound like too general names in global space.
> And are unrelated to cpufreq(.h).

maybe call them cpufreq_define_(global|freq)_* then?

> Eventually you get them into sysfs.h with another name
> or just duplicate them?

Well, struct freq_attr for example is cpufreq-specific attribute,
AFAICT. So, keeping them in cpufreq.h should be fine, no?

-- 
Regards/Gruss,
Boris.

--
Advanced Micro Devices, Inc.
Operating Systems Research Center

Re: [PATCH 5/5] cpufreq: Add support for actual freq

[Thomas Renninger]

On Monday 22 March 2010 19:38:41 Borislav Petkov wrote:
> From: Borislav Petkov <borislav.petkov@amd.com>
> 
> Modify the scaling_cur_freq interface to show the actual core frequency
> when boosting is supported and enabled on the core.
This looks wrong.

scaling_cur freq should show the frequency the kernel/cpufreq
subsystem thinks it's in.
You show the average freq and the time of the measured average
frequency depends on when the cpufreq subsystem called getavg()
the last time.
Also the time frame of the average freq the cpufreq subsystem
gets when calling getavg() now depends on whether and how often
userspace calls scaling_cur_freq which influences switching policy.

Latest cpufrequtils (ver 006) supports cpufreq-aperf to check whether
cores enter boost mode. Len Brown afaik also has a userspace tool, but
if it has any advantages, it should IMO get integrated into cpufrequtils
which people know to use when looking at cpufreq.

I once thought about adding scaling_avg_freq which gets an own
aperf_mperf counter, but you don't know whether another app read out the
average freq in between and your expected measured time frame is wrong then.
You could remember aperf/mperf per pid and free the saved aperf/mperf value
if the process dies..., but what for if this can be read out in userspace.

    Thomas

Re: [PATCH 1/5] cpufreq: Unify sysfs attribute definition macros

[Thomas Renninger]

On Tuesday 23 March 2010 12:44:09 Borislav Petkov wrote:
> From: Thomas Renninger <trenn@suse.de>
> Date: Tue, Mar 23, 2010 at 12:07:29PM +0100
> 
> > > +#define define_one_global_ro(_name)		\
> > > +static struct global_attr _name =		\
> > > +__ATTR(_name, 0444, show_##_name, NULL)
> > > +
> > > +#define define_one_global_rw(_name)		\
> > > +static struct global_attr _name =		\
> > > +__ATTR(_name, 0644, show_##_name, store_##_name)
> > 
> > These sound like too general names in global space.
> > And are unrelated to cpufreq(.h).
> 
> maybe call them cpufreq_define_(global|freq)_* then?
> 
> > Eventually you get them into sysfs.h with another name
> > or just duplicate them?
> 
> Well, struct freq_attr for example is cpufreq-specific attribute,
> AFAICT. So, keeping them in cpufreq.h should be fine, no?
You don't need much of these (one or two?).
I'd leave this cleanup out for your patch series.
You care about the boost and aperf/mperf stuff and not about this
cleanup?

    Thomas

Re: [PATCH 2/5] powernow-k8: Add core performance boost support

[Borislav Petkov]

(Adding hpa to Cc)

From: Thomas Renninger <trenn@suse.de>
Date: Tue, Mar 23, 2010 at 12:17:16PM +0100

> > +/* core performance boost */
> > +static bool cpb_capable, cpb_disabled;
> Whatabout using a cpufeature (arch/x86/include/asm/cpufeature.h)
> instead of cpb_capable. Then people could see this feature in 
> /proc/cpuinfo and other code parts could check for it easily if
> needed later.

I don't have a problem with that per se. It's just that /proc/cpuinfo
is a widely used interface and, AFAIR, changing it is not taken that
lightly.

Peter, what do you think?

> It could already be set in arch/x86/kernel/cpu/amd.c and
> powernow-k8 could use cpu_has(cpu, X86_FEATURE_CPB);

I'd still like to cache the cpb_capable value locally instead of getting
x86_cpuinfo percpu var and querying it. Especially if this happens often
and not only at driver init.

> Instead of cpb_disabled, I'd use cpb_enabled. Checking for
> !cpb_disabled whether it's enabled, is ugly to read.

Fair enough, cpb_disabled reflects the bit semantics in the MSR but why
not, don't matter which to me.

[..]

> > +		_cpb_toggle_msrs(t);
> > +		printk(KERN_INFO PFX "Core Boosting enabled.\n");
> Always printk on every toggle?
> That should not happen often and a user might want to get noticed if
> an app does this behind his back -> should be fine w/ or w/o, just not 
> sure whether it's intended.

Well, actually, this should be on by default and the user or an app
shouldn't be fidgeting with it all the time. It's there only for
benchmarking purposes so that you can disable it when you really have
to. But I guess it actually is going to get used if its there so maybe
we should have to rethink our approach. Hmm...

-- 
Regards/Gruss,
Boris.

--
Advanced Micro Devices, Inc.
Operating Systems Research Center

Re: [PATCH 3/5] cpufreq: Add APERF/MPERF support for AMD processors

[Borislav Petkov]

From: Thomas Renninger <trenn@suse.de>
Date: Tue, Mar 23, 2010 at 12:26:22PM +0100

> On Monday 22 March 2010 19:38:39 Borislav Petkov wrote:
> > From: Mark Langsdorf <mark.langsdorf@amd.com>
> > 
> > Starting with model 10 of Family 0x10, AMD processors may have
> > support for APERF/MPERF.  Add support for identifying it and using
> > it within cpufreq.  Move the APERF/MPERF functions out of the
> > acpi-cpufreq code and into their own file so they can easily be
> > shared.
> > 
> > Signed-off-by: Mark Langsdorf <mark.langsdorf@amd.com>
> > Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
> > ---
> >  arch/x86/kernel/cpu/amd.c                  |    6 +++
> >  arch/x86/kernel/cpu/cpufreq/Makefile       |    4 +-
> >  arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c |   44 +-----------------------
> >  arch/x86/kernel/cpu/cpufreq/mperf.c        |   50 ++++++++++++++++++++++++++++
> >  arch/x86/kernel/cpu/cpufreq/mperf.h        |    9 +++++
> >  arch/x86/kernel/cpu/cpufreq/powernow-k8.c  |    8 ++++
> >  6 files changed, 77 insertions(+), 44 deletions(-)
> >  create mode 100644 arch/x86/kernel/cpu/cpufreq/mperf.c
> >  create mode 100644 arch/x86/kernel/cpu/cpufreq/mperf.h
> > 
> > diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> > index e485825..796f662 100644
> > --- a/arch/x86/kernel/cpu/amd.c
> > +++ b/arch/x86/kernel/cpu/amd.c
> > @@ -537,6 +537,12 @@ static void __cpuinit init_amd(struct cpuinfo_x86 *c)
> >  		set_cpu_cap(c, X86_FEATURE_MFENCE_RDTSC);
> >  	}
> >  
> > +	if (c->cpuid_level >= 6) {
> > +		unsigned ecx = cpuid_ecx(6);
> > +		if (ecx & 0x01)
> > +			set_cpu_cap(c, X86_FEATURE_APERFMPERF);
> > +	}
> This is nearly identical to (beside c->cpuid_level > 6),
> in arch/x86/kernel/cpu/intel.c:
> if (c->cpuid_level > 6) {
>                 unsigned ecx = cpuid_ecx(6);
>                 if (ecx & 0x01)
>                         set_cpu_cap(c, X86_FEATURE_APERFMPERF);
> }
> I expect you are correct... or could it get moved to general x86 init code?

Sounds good...

-- 
Regards/Gruss,
Boris.

--
Advanced Micro Devices, Inc.
Operating Systems Research Center

Re: [PATCH 1/5] cpufreq: Unify sysfs attribute definition macros

[Borislav Petkov]

From: Thomas Renninger <trenn@suse.de>
Date: Tue, Mar 23, 2010 at 12:55:30PM +0100

> > Well, struct freq_attr for example is cpufreq-specific attribute,
> > AFAICT. So, keeping them in cpufreq.h should be fine, no?
> You don't need much of these (one or two?).

I don't think I get what you mean here..?

> I'd leave this cleanup out for your patch series.
> You care about the boost and aperf/mperf stuff and not about this
> cleanup?

Why? What's wrong with cleaning up obviously duplicated code?

-- 
Regards/Gruss,
Boris.

--
Advanced Micro Devices, Inc.
Operating Systems Research Center

Re: [PATCH 1/5] cpufreq: Unify sysfs attribute definition macros

[Thomas Renninger]

On Tuesday 23 March 2010 13:05:34 Borislav Petkov wrote:
> From: Thomas Renninger <trenn@suse.de>
> Date: Tue, Mar 23, 2010 at 12:55:30PM +0100
> 
> > > Well, struct freq_attr for example is cpufreq-specific attribute,
> > > AFAICT. So, keeping them in cpufreq.h should be fine, no?
> > You don't need much of these (one or two?).
> 
> I don't think I get what you mean here..?
I expect you did this because you use it in your follow up patches, e.g.
+define_one_freq_rw(cpb);
If you don't want to argue about global names, but get your boost code
in quickly, just also duplicate the macro and if you have some time left
you could still do an on-top cleanup later.

   Thomas

Re: [PATCH 2/5] powernow-k8: Add core performance boost support

[Dominik Brodowski]

Hey,

On Tue, Mar 23, 2010 at 12:58:58PM +0100, Borislav Petkov wrote:
> > It could already be set in arch/x86/kernel/cpu/amd.c and
> > powernow-k8 could use cpu_has(cpu, X86_FEATURE_CPB);
> 
> I'd still like to cache the cpb_capable value locally instead of getting
> x86_cpuinfo percpu var and querying it. Especially if this happens often
> and not only at driver init.

If it's a percpu var, isn't it local anyway?

Best,
	Dominik

Re: [PATCH 2/5] powernow-k8: Add core performance boost support

[Borislav Petkov]

From: Dominik Brodowski <linux@dominikbrodowski.net>
Date: Tue, Mar 23, 2010 at 02:27:37PM +0100

Hi Dominik,

> On Tue, Mar 23, 2010 at 12:58:58PM +0100, Borislav Petkov wrote:
> > > It could already be set in arch/x86/kernel/cpu/amd.c and
> > > powernow-k8 could use cpu_has(cpu, X86_FEATURE_CPB);
> > 
> > I'd still like to cache the cpb_capable value locally instead of getting
> > x86_cpuinfo percpu var and querying it. Especially if this happens often
> > and not only at driver init.
> 
> If it's a percpu var, isn't it local anyway?

I meant driver-local. So that I don't have to deref even the per_cpu var
and thus save some cycles.

-- 
Regards/Gruss,
Boris.

--
Advanced Micro Devices, Inc.
Operating Systems Research Center

Re: [PATCH 5/5] cpufreq: Add support for actual freq

[Borislav Petkov]

From: Thomas Renninger <trenn@suse.de>
Date: Tue, Mar 23, 2010 at 12:51:18PM +0100

Hi,

> scaling_cur freq should show the frequency the kernel/cpufreq
> subsystem thinks it's in.

Well, we have also
/sys/devices/system/cpu/cpu<NUM>/cpufreq/cpuinfo_cur_freq and it reads
also policy->cur. Why not show the actual frequency in scaling_cur_freq
then?

> You show the average freq and the time of the measured average
> frequency depends on when the cpufreq subsystem called getavg()
> the last time.
> Also the time frame of the average freq the cpufreq subsystem
> gets when calling getavg() now depends on whether and how often
> userspace calls scaling_cur_freq which influences switching policy.
> 
> Latest cpufrequtils (ver 006) supports cpufreq-aperf to check whether
> cores enter boost mode. Len Brown afaik also has a userspace tool, but
> if it has any advantages, it should IMO get integrated into cpufrequtils
> which people know to use when looking at cpufreq.
> 
> I once thought about adding scaling_avg_freq which gets an own
> aperf_mperf counter, but you don't know whether another app read out the
> average freq in between and your expected measured time frame is wrong then.
> You could remember aperf/mperf per pid and free the saved aperf/mperf value
> if the process dies..., but what for if this can be read out in userspace.

Ah yes, I see what you mean. Yet, I still don't like the idea of having
to use a special userspace tool just to read the actual frequency. How
about we hook into <drivers/cpufreq/cpufreq_ondemand.c:dbs_check_cpu()>
and passively output the freq_avg after being computed in

                freq_avg = __cpufreq_driver_getavg(policy, j);
                if (freq_avg <= 0)
                        freq_avg = policy->cur;

through sysfs. Hmm...?

-- 
Regards/Gruss,
Boris.

--
Advanced Micro Devices, Inc.
Operating Systems Research Center

Re: [PATCH 5/5] cpufreq: Add support for actual freq

[Dominik Brodowski]

Hey,

On Tue, Mar 23, 2010 at 03:23:49PM +0100, Borislav Petkov wrote:
> > scaling_cur freq should show the frequency the kernel/cpufreq
> > subsystem thinks it's in.
> 
> Well, we have also
> /sys/devices/system/cpu/cpu<NUM>/cpufreq/cpuinfo_cur_freq and it reads
> also policy->cur.

No. cpuinfo_cur_freq calls __cpufreq_get() which itself calls the driver's
->get() callback, which is exactly for determining the current frequency. I
assume CPUFREQ_CONST_LOOP is set, else we'd get into trouble; but as long as
that's the case I see no problem in using ->get() / cpuinfo_cur_freq to read
out the current actual frequency.

> Why not show the actual frequency in scaling_cur_freq then?

Quoting Documentation/cpu-freq/user-guide.txt :

cpuinfo_cur_freq :              Current frequency of the CPU as obtained from
                                the hardware, in KHz. This is the frequency
                                the CPU actually runs at.

scaling_cur_freq :              Current frequency of the CPU as determined by
                                the governor and cpufreq core, in KHz. This is
                                the frequency the kernel thinks the CPU runs
                                at.

So scaling_cur_freq may not be mis-used for this; cpuinfo_cur_freq may be
used for it, though. IMVHO.

Best,
	Dominik

Re: [PATCH 2/5] powernow-k8: Add core performance boost support

[Dominik Brodowski]

Hey Borislav,

On Tue, Mar 23, 2010 at 03:19:34PM +0100, Borislav Petkov wrote:
> From: Dominik Brodowski <linux@dominikbrodowski.net>
> Date: Tue, Mar 23, 2010 at 02:27:37PM +0100
> 
> Hi Dominik,
> 
> > On Tue, Mar 23, 2010 at 12:58:58PM +0100, Borislav Petkov wrote:
> > > > It could already be set in arch/x86/kernel/cpu/amd.c and
> > > > powernow-k8 could use cpu_has(cpu, X86_FEATURE_CPB);
> > > 
> > > I'd still like to cache the cpb_capable value locally instead of getting
> > > x86_cpuinfo percpu var and querying it. Especially if this happens often
> > > and not only at driver init.
> > 
> > If it's a percpu var, isn't it local anyway?
> 
> I meant driver-local. So that I don't have to deref even the per_cpu var
> and thus save some cycles.

Well, it doesn't seem to be used in any hot path (and if it were, using a
per-cpu var was better anyway, because of no contention etc.). If it really
saves some cycles, I'm more than fine with keeping it; still, informing the
user in /proc/cpuinfo seems like a sensible thing to do.

Best,
	Dominik

Re: [PATCH 5/5] cpufreq: Add support for actual freq

[Thomas Renninger]

On Tuesday 23 March 2010 15:23:49 Borislav Petkov wrote:
> From: Thomas Renninger <trenn@suse.de>
> Date: Tue, Mar 23, 2010 at 12:51:18PM +0100
> 
> Hi,
> 
> > scaling_cur freq should show the frequency the kernel/cpufreq
> > subsystem thinks it's in.
> 
> Well, we have also
> /sys/devices/system/cpu/cpu<NUM>/cpufreq/cpuinfo_cur_freq and it reads
> also policy->cur. Why not show the actual frequency in scaling_cur_freq
> then?
> 
> > You show the average freq and the time of the measured average
> > frequency depends on when the cpufreq subsystem called getavg()
> > the last time.
> > Also the time frame of the average freq the cpufreq subsystem
> > gets when calling getavg() now depends on whether and how often
> > userspace calls scaling_cur_freq which influences switching policy.
> > 
> > Latest cpufrequtils (ver 006) supports cpufreq-aperf to check whether
> > cores enter boost mode. Len Brown afaik also has a userspace tool, but
> > if it has any advantages, it should IMO get integrated into cpufrequtils
> > which people know to use when looking at cpufreq.
> > 
> > I once thought about adding scaling_avg_freq which gets an own
> > aperf_mperf counter, but you don't know whether another app read out the
> > average freq in between and your expected measured time frame is wrong then.
> > You could remember aperf/mperf per pid and free the saved aperf/mperf value
> > if the process dies..., but what for if this can be read out in userspace.
> 
> Ah yes, I see what you mean. Yet, I still don't like the idea of having
> to use a special userspace tool just to read the actual frequency. How
> about we hook into <drivers/cpufreq/cpufreq_ondemand.c:dbs_check_cpu()>
> and passively output the freq_avg after being computed in
> 
>                 freq_avg = __cpufreq_driver_getavg(policy, j);
>                 if (freq_avg <= 0)
>                         freq_avg = policy->cur;
> 
> through sysfs. Hmm...?
But what data would you get then? It's defintely not cur_freq.
It's some kind of average, but you don't know the time frame.

I posted a solution with an extra aperf/mperf save for scaling_avg_freq
(or similar).
First read would return nothing valid.
cat scaling_avg_freq > /dev/null
watch -n1 cat scaling_avg_freq
would return the average freq of the last seconds, exactly the same
what you can do with cpufreq-aperf from userspace.
But if another user app does the same, it's messed up.
You can find out the pid of the process doing the cat and remember
aperf/mperf for it if it does not exist yet..., but now it gets to a
point where cpufreq-aperf is really more convenient and straight foward.

Possibly documenting cpufreq-aperf in Documentation/cpu-freq would
be worth it. Also mentioning "boost" somewhere would be great:
grep -i boost Documentation/cpu-freq/ -r
Documentation/cpu-freq/pcc-cpufreq.txt:This is due to "turbo boost" ...

Another idea is to have a separate cpufreq_avg_freq and update it on
every target() call, but that's overhead...


   Thomas

Re: [PATCH 2/5] powernow-k8: Add core performance boost support

[Borislav Petkov]

From: Dominik Brodowski <linux@dominikbrodowski.net>
Date: Tue, Mar 23, 2010 at 03:47:57PM +0100

> > > If it's a percpu var, isn't it local anyway?
> > 
> > I meant driver-local. So that I don't have to deref even the per_cpu var
> > and thus save some cycles.
> 
> Well, it doesn't seem to be used in any hot path (and if it were, using a
> per-cpu var was better anyway, because of no contention etc.). If it really
> saves some cycles, I'm more than fine with keeping it; still, informing the
> user in /proc/cpuinfo seems like a sensible thing to do.

No, the idea was to do the local caching _in addition_ to advertising it
over /proc/cpuinfo. Let's see what the x86 maintainers think though...

-- 
Regards/Gruss,
Boris.

--
Advanced Micro Devices, Inc.
Operating Systems Research Center

Re: [PATCH 5/5] cpufreq: Add support for actual freq

[Borislav Petkov]

From: Thomas Renninger <trenn@suse.de>
Date: Tue, Mar 23, 2010 at 04:12:18PM +0100

Hi Thomas,

> would return the average freq of the last seconds, exactly the same
> what you can do with cpufreq-aperf from userspace.
> But if another user app does the same, it's messed up.
> You can find out the pid of the process doing the cat and remember
> aperf/mperf for it if it does not exist yet..., but now it gets to a
> point where cpufreq-aperf is really more convenient and straight foward.
> 
> Possibly documenting cpufreq-aperf in Documentation/cpu-freq would
> be worth it. Also mentioning "boost" somewhere would be great:
> grep -i boost Documentation/cpu-freq/ -r
> Documentation/cpu-freq/pcc-cpufreq.txt:This is due to "turbo boost" ...
> 
> Another idea is to have a separate cpufreq_avg_freq and update it on
> every target() call, but that's overhead...

you got me persuaded - if we want to have the effective frequency
reported in the kernel, we have to come up with a slick solution that
wouldn't incur overhead and won't be disrupted by others accessing those
MSRs. Let's leave it to cpufreq-aperf for now and revisit this when
needed.

Thanks.

-- 
Regards/Gruss,
Boris.

--
Advanced Micro Devices, Inc.
Operating Systems Research Center