cpupowerutils: easier use of msr and cpuid stuff and some more

cpupowerutils: easier use of msr and cpuid stuff and some more

[Thomas Renninger]

Compile tested on x86_64.
The new param got quickly tested, but not in all variants on all HW.

For not X86, msr.h and cpuid.h needs some dummy functions if:
#if defined(__i386__) || defined(__x86_64__)
is not defined. I can send this on top, but like to get some feedback on
this first.

It would be great if this can be pushed if there aren't sever errors
or whatever issues.

Thanks,

      Thomas

[PATCH 1/5] cpupowerutils: Move read_msr from cpufreq-aperf.c into own /lib/msr.c file

[Thomas Renninger]

Reading msrs is a function that will get re-used quite often for other stuff
soon.

Signed-off-by: Thomas Renninger <trenn@suse.de>
CC: Dominik Brodowski <linux@dominikbrodowski.net>
CC: cpufreq@vger.kernel.org
---
 Makefile              |    6 ++--
 lib/msr.c             |   88 +++++++++++++++++++++++++++++++++++++++++++++++++
 lib/msr.h             |    4 ++
 utils/cpufreq-aperf.c |   42 ++---------------------
 4 files changed, 99 insertions(+), 41 deletions(-)
 create mode 100644 lib/msr.c
 create mode 100644 lib/msr.h

diff --git a/Makefile b/Makefile
index b767d97..b04f85c 100644
--- a/Makefile
+++ b/Makefile
@@ -113,9 +113,9 @@ CPPFLAGS += -DVERSION=\"$(VERSION)\" -DPACKAGE=\"$(PACKAGE)\" \
 
 UTIL_SRC = 	utils/cpufreq-info.c utils/cpufreq-set.c utils/cpufreq-aperf.c utils/cpuidle-info.c utils/cpuid.h
 UTIL_BINS = 	utils/cpufreq-info utils/cpufreq-set utils/cpufreq-aperf utils/cpuidle-info
-LIB_HEADERS = 	lib/cpufreq.h lib/cpuidle.h lib/sysfs.h
-LIB_SRC = 	lib/cpufreq.c lib/cpuidle.c lib/sysfs.c
-LIB_OBJS = 	lib/cpufreq.o lib/cpuidle.o lib/sysfs.o
+LIB_HEADERS = 	lib/cpufreq.h lib/cpuidle.h lib/sysfs.h lib/msr.h
+LIB_SRC = 	lib/cpufreq.c lib/cpuidle.c lib/sysfs.c lib/msr.c
+LIB_OBJS = 	lib/cpufreq.o lib/cpuidle.o lib/sysfs.o lib/msr.o
 
 CFLAGS +=	-pipe
 
diff --git a/lib/msr.c b/lib/msr.c
new file mode 100644
index 0000000..f532e1a
--- /dev/null
+++ b/lib/msr.c
@@ -0,0 +1,88 @@
+#include <stdio.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <stdint.h>
+
+#include <sys/stat.h>
+#include <sys/types.h>
+
+
+#define MSR_IA32_APERF		0x000000E8
+#define MSR_IA32_MPERF		0x000000E7
+
+#define MSR_FIDVID_STATUS	0xc0010042
+
+#define MSR_IA32_PERF_STATUS	0x198
+
+
+/*
+ * read_msr
+ *
+ * Will return 0 on success and -1 on failure.
+ * Possible errno values could be:
+ * EFAULT -If the read/write did not fully complete
+ * EIO    -If the CPU does not support MSRs
+ * ENXIO  -If the CPU does not exist
+ */
+
+static int read_msr(int cpu, unsigned int idx, uint64_t *val)
+{
+	int fd;
+	char msr_file_name[64];
+
+	sprintf(msr_file_name, "/dev/cpu/%d/msr", cpu);
+	fd = open(msr_file_name, O_RDONLY);
+	if (fd < 0)
+		return -1;
+	if (lseek(fd, idx, SEEK_CUR) == -1)
+		goto err;
+	if (read(fd, val, sizeof val) != sizeof *val)
+		goto err;
+	close(fd);
+	return 0;
+ err:
+	close(fd);
+	return -1;
+}
+
+/*
+ * write_msr
+ *
+ * Will return 0 on success and -1 on failure.
+ * Possible errno values could be:
+ * EFAULT -If the read/write did not fully complete
+ * EIO    -If the CPU does not support MSRs
+ * ENXIO  -If the CPU does not exist
+ */
+
+static int write_msr(int cpu, unsigned int idx, uint64_t val)
+{
+	int fd;
+	char msr_file_name[64];
+
+	sprintf(msr_file_name, "/dev/cpu/%d/msr", cpu);
+	fd = open(msr_file_name, O_WRONLY);
+	if (fd < 0)
+		return -1;
+	if (lseek(fd, idx, SEEK_CUR) == -1)
+		goto err;
+	if (write(fd, &val, sizeof val) != sizeof val)
+		goto err;
+	close(fd);
+	return 0;
+ err:
+	close(fd);
+	return -1;
+}
+
+int msr_get_aperf(unsigned int cpu, uint64_t *aperf)
+{
+	return read_msr(cpu, MSR_IA32_APERF, aperf);
+
+}
+
+int msr_get_mperf(unsigned int cpu, uint64_t *mperf)
+{
+	return read_msr(cpu, MSR_IA32_MPERF, mperf);
+
+}
diff --git a/lib/msr.h b/lib/msr.h
new file mode 100644
index 0000000..347c50f
--- /dev/null
+++ b/lib/msr.h
@@ -0,0 +1,4 @@
+extern int msr_get_mperf(unsigned int cpu, uint64_t *mperf);
+extern int msr_get_aperf(unsigned int cpu, uint64_t *aperf);
+
+extern int msr_amd_get_fidvid(unsigned int cpu, uint32_t *fid, uint32_t *vid);
diff --git a/utils/cpufreq-aperf.c b/utils/cpufreq-aperf.c
index 77ce114..c9461ae 100644
--- a/utils/cpufreq-aperf.c
+++ b/utils/cpufreq-aperf.c
@@ -46,18 +46,14 @@
 #include <unistd.h>
 #include <getopt.h>
 #include <sys/time.h>
-#include <sys/types.h>
-#include <stdint.h>
 #include <errno.h>
-#include <fcntl.h>
 #include <string.h>
+#include <stdint.h>
 
 #include "cpufreq.h"
+#include "msr.h"
 #include "cpuid.h"
 
-#define MSR_IA32_APERF 0x000000E8
-#define MSR_IA32_MPERF 0x000000E7
-
 struct avg_perf_cpu_info
 {
 	unsigned long max_freq;
@@ -80,36 +76,6 @@ static int cpu_has_effective_freq()
 }
 
 /*
- * read_msr
- *
- * Will return 0 on success and -1 on failure.
- * Possible errno values could be:
- * EFAULT -If the read/write did not fully complete
- * EIO    -If the CPU does not support MSRs
- * ENXIO  -If the CPU does not exist
- */
-
-static int read_msr(int cpu, unsigned int idx, unsigned long long *val)
-{
-	int fd;
-	char msr_file_name[64];
-
-	sprintf(msr_file_name, "/dev/cpu/%d/msr", cpu);
-	fd = open(msr_file_name, O_RDONLY);
-	if (fd < 0)
-		return -1;
-	if (lseek(fd, idx, SEEK_CUR) == -1)
-		goto err;
-	if (read(fd, val, sizeof val) != sizeof *val)
-		goto err;
-	close(fd);
-	return 0;
- err:
-	close(fd);
-	return -1;
-}
-
-/*
  * get_aperf_mperf()
  *
  * Returns the current aperf/mperf MSR values of cpu
@@ -118,11 +84,11 @@ static int get_aperf_mperf(unsigned int cpu, uint64_t *aperf, uint64_t *mperf)
 {
 	int retval;
 
-	retval = read_msr(cpu, MSR_IA32_APERF, (unsigned long long*)aperf);
+	retval = msr_get_aperf(cpu, aperf);
 	if (retval < 0)
 		return retval;
 
-	retval = read_msr(cpu, MSR_IA32_MPERF, (unsigned long long*)mperf);
+	retval = msr_get_mperf(cpu, mperf);
 	if (retval < 0)
 		return retval;
 	return 0;
-- 
1.6.4.2

[PATCH 2/5] cpupowerutils: Let older tools make use of global read_msr functions

[Thomas Renninger]

These are not intended to be exported via library, but internal tools
can statically link/compile against msr.o to keep the code base as tiny
as possible.


Signed-off-by: Thomas Renninger <trenn@suse.de>
CC: Dominik Brodowski <linux@dominikbrodowski.net>
CC: cpufreq@vger.kernel.org
---
 debug/i386/centrino-decode.c    |   45 +++++---------------------------------
 debug/i386/powernow-k8-decode.c |   38 ++------------------------------
 debug/x86_64/Makefile           |   17 +++++++++++---
 lib/msr.c                       |   42 ++++++++++++++++++++++++++++++++++++
 lib/msr.h                       |    1 +
 5 files changed, 65 insertions(+), 78 deletions(-)

diff --git a/debug/i386/centrino-decode.c b/debug/i386/centrino-decode.c
index 7ef24cc..67e9dbf 100644
--- a/debug/i386/centrino-decode.c
+++ b/debug/i386/centrino-decode.c
@@ -22,44 +22,9 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 
-#define MCPU	32
-
-#define MSR_IA32_PERF_STATUS	0x198
-
-static int rdmsr(unsigned int cpu, unsigned int msr,
-		 unsigned int *lo, unsigned int *hi)
-{
-	int fd;
-	char file[20];
-	unsigned long long val;
-	int retval = -1;
-
-	*lo = *hi = 0;
-
-	if (cpu > MCPU)
-		goto err1;
-
-	sprintf(file, "/dev/cpu/%d/msr", cpu);
-	fd = open(file, O_RDONLY);
+#include "msr.h"
 
-	if (fd < 0)
-		goto err1;
-
-	if (lseek(fd, msr, SEEK_CUR) == -1)
-		goto err2;
-
-	if (read(fd, &val, 8) != 8)
-		goto err2;
-
-	*lo = (uint32_t )(val & 0xffffffffull);
-	*hi = (uint32_t )(val>>32 & 0xffffffffull);
-
-	retval = 0;
-err2:
-	close(fd);
-err1:
-	return retval;
-}
+#define MCPU	32
 
 static void decode (unsigned int msr)
 {
@@ -75,10 +40,11 @@ static void decode (unsigned int msr)
 
 static int decode_live(unsigned int cpu)
 {
-	unsigned int lo, hi;
+	unsigned int lo;
+	uint64_t cpu_freq;
 	int err;
 
-	err = rdmsr(cpu, MSR_IA32_PERF_STATUS, &lo, &hi);
+	err = msr_intel_get_perf_status(cpu, &cpu_freq);
 
 	if (err) {
 		printf("can't get MSR_IA32_PERF_STATUS for cpu %d\n", cpu);
@@ -87,6 +53,7 @@ static int decode_live(unsigned int cpu)
 		return 1;
 	}
 
+	lo = (uint32_t )(cpu_freq & 0xffffffffull);
 	decode(lo);
 
 	return 0;
diff --git a/debug/i386/powernow-k8-decode.c b/debug/i386/powernow-k8-decode.c
index 638a6b3..435c597 100644
--- a/debug/i386/powernow-k8-decode.c
+++ b/debug/i386/powernow-k8-decode.c
@@ -18,41 +18,9 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 
-#define MCPU 32
-
-#define MSR_FIDVID_STATUS	0xc0010042
-
-#define MSR_S_HI_CURRENT_VID	0x0000001f
-#define MSR_S_LO_CURRENT_FID	0x0000003f
-
-static int get_fidvid(uint32_t cpu, uint32_t *fid, uint32_t *vid)
-{
-	int err = 1;
-	uint64_t msr = 0;
-	int fd;
-	char file[20];
-
-	if (cpu > MCPU)
-		goto out;
-
-	sprintf(file, "/dev/cpu/%d/msr", cpu);
-
-	fd = open(file, O_RDONLY);
-	if (fd < 0)
-		goto out;
-	lseek(fd, MSR_FIDVID_STATUS, SEEK_CUR);
-	if (read(fd, &msr, 8) != 8)
-		goto err1;
-
-	*fid = ((uint32_t )(msr & 0xffffffffull)) & MSR_S_LO_CURRENT_FID;
-	*vid = ((uint32_t )(msr>>32 & 0xffffffffull)) & MSR_S_HI_CURRENT_VID;
-	err = 0;
-err1:
-	close(fd);
-out:
-	return err;
-}
+#include "msr.h"
 
+#define MCPU 32
 
 /* Return a frequency in MHz, given an input fid */
 static uint32_t find_freq_from_fid(uint32_t fid)
@@ -77,7 +45,7 @@ int main (int argc, char *argv[])
 	else
 		cpu = strtoul(argv[1], NULL, 0);
 
-	err = get_fidvid(cpu, &fid, &vid);
+	err = msr_amd_get_fidvid(cpu, &fid, &vid);
 
 	if (err) {
 		printf("can't get fid, vid from MSR\n");
diff --git a/debug/x86_64/Makefile b/debug/x86_64/Makefile
index dbf1399..d8e4db8 100644
--- a/debug/x86_64/Makefile
+++ b/debug/x86_64/Makefile
@@ -1,10 +1,19 @@
 default: all
 
-centrino-decode: centrino-decode.c
-	$(CC) $(CFLAGS) -o centrino-decode centrino-decode.c
+CFLAGS += -I ../../lib
 
-powernow-k8-decode: powernow-k8-decode.c
-	$(CC) $(CFLAGS) -o powernow-k8-decode powernow-k8-decode.c
+centrino-decode: centrino-decode.c msr.o
+	$(CC) $(CFLAGS) -c centrino-decode.c -o centrino-decode.o
+	$(CC) $(CFLAGS) -c ../../lib/msr.c -o ../../lib/msr.o
+	$(CC) $(CFLAGS) centrino-decode.o ../../lib/msr.o -o centrino-decode
+
+powernow-k8-decode: powernow-k8-decode.c msr.o
+	$(CC) $(CFLAGS) -c powernow-k8-decode.c -o powernow-k8-decode.o
+	$(CC) $(CFLAGS) -c ../../lib/msr.c -o  ../../lib/msr.o
+	$(CC) ../../lib/msr.o powernow-k8-decode.o -o powernow-k8-decode
+
+msr.o: ../../lib/msr.c
+	$(CC) -c ../../lib/msr.c -o ../../lib/msr.o
 
 all: centrino-decode powernow-k8-decode
 
diff --git a/lib/msr.c b/lib/msr.c
index f532e1a..bccea8d 100644
--- a/lib/msr.c
+++ b/lib/msr.c
@@ -1,3 +1,15 @@
+/*
+ *  (C) 2010	Thomas Renninger <trenn@suse.de>
+ *
+ *  Licensed under the terms of the GNU GPL License version 2.
+ *
+ *  msr.c:
+ *  Provides easy access to X86 specific Machine Specific Registers (MSRs)
+ *
+ *  Callers have to make sure themselves whether the machine supports
+ *  the called MSR functions, for example via cpuid.
+ */
+
 #include <stdio.h>
 #include <unistd.h>
 #include <fcntl.h>
@@ -7,13 +19,19 @@
 #include <sys/types.h>
 
 
+/* General X86 MSRs */
 #define MSR_IA32_APERF		0x000000E8
 #define MSR_IA32_MPERF		0x000000E7
 
+/* AMD specific MSRs */
 #define MSR_FIDVID_STATUS	0xc0010042
 
+/* Intel specific MSRs */
 #define MSR_IA32_PERF_STATUS	0x198
 
+/* AMD specific bits */
+#define AMD_S_HI_CURRENT_VID	0x0000001f
+#define AMD_S_LO_CURRENT_FID	0x0000003f
 
 /*
  * read_msr
@@ -75,6 +93,8 @@ static int write_msr(int cpu, unsigned int idx, uint64_t val)
 	return -1;
 }
 
+/* General X86 MSRs ********************************/
+
 int msr_get_aperf(unsigned int cpu, uint64_t *aperf)
 {
 	return read_msr(cpu, MSR_IA32_APERF, aperf);
@@ -84,5 +104,27 @@ int msr_get_aperf(unsigned int cpu, uint64_t *aperf)
 int msr_get_mperf(unsigned int cpu, uint64_t *mperf)
 {
 	return read_msr(cpu, MSR_IA32_MPERF, mperf);
+}
 
+/* AMD X86 MSRs ************************************/
+int msr_amd_get_fidvid(unsigned int cpu, uint32_t *fid, uint32_t *vid)
+{
+	uint64_t fid_vid;
+	int ret;
+
+	ret = read_msr(cpu, MSR_FIDVID_STATUS, &fid_vid);
+	if (ret)
+		return ret;
+
+	*fid = ((uint32_t )(fid_vid & 0xffffffffull)) & AMD_S_LO_CURRENT_FID;
+	*vid = ((uint32_t )(fid_vid >> 32 & 0xffffffffull))
+		& AMD_S_HI_CURRENT_VID;
+
+	return 0;
+}
+
+/* Intel X86 MSRs **********************************/
+int msr_intel_get_perf_status(unsigned int cpu, uint64_t perf_status)
+{
+       return read_msr(cpu, MSR_IA32_PERF_STATUS, &perf_status);
 }
diff --git a/lib/msr.h b/lib/msr.h
index 347c50f..8b1755e 100644
--- a/lib/msr.h
+++ b/lib/msr.h
@@ -2,3 +2,4 @@ extern int msr_get_mperf(unsigned int cpu, uint64_t *mperf);
 extern int msr_get_aperf(unsigned int cpu, uint64_t *aperf);
 
 extern int msr_amd_get_fidvid(unsigned int cpu, uint32_t *fid, uint32_t *vid);
+extern int msr_intel_get_perf_status(unsigned int cpu, uint64_t perf_status);
-- 
1.6.4.2

[PATCH 3/5] cpupowerutils: Move utils/cpuid.h to lib/cpuid.h

[Thomas Renninger]

So that utils/*.c code can make use of this as well.

Signed-off-by: Thomas Renninger <trenn@suse.de>
CC: Dominik Brodowski <linux@dominikbrodowski.net>
CC: cpufreq@vger.kernel.org
---
 Makefile      |    4 +-
 lib/cpuid.h   |   65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 utils/cpuid.h |   63 -------------------------------------------------------
 3 files changed, 67 insertions(+), 65 deletions(-)
 create mode 100644 lib/cpuid.h
 delete mode 100644 utils/cpuid.h

diff --git a/Makefile b/Makefile
index b04f85c..2cd9e83 100644
--- a/Makefile
+++ b/Makefile
@@ -111,9 +111,9 @@ WARNINGS += -Wshadow
 CPPFLAGS += -DVERSION=\"$(VERSION)\" -DPACKAGE=\"$(PACKAGE)\" \
 		-DPACKAGE_BUGREPORT=\"$(PACKAGE_BUGREPORT)\" -D_GNU_SOURCE
 
-UTIL_SRC = 	utils/cpufreq-info.c utils/cpufreq-set.c utils/cpufreq-aperf.c utils/cpuidle-info.c utils/cpuid.h
+UTIL_SRC = 	utils/cpufreq-info.c utils/cpufreq-set.c utils/cpufreq-aperf.c utils/cpuidle-info.c
 UTIL_BINS = 	utils/cpufreq-info utils/cpufreq-set utils/cpufreq-aperf utils/cpuidle-info
-LIB_HEADERS = 	lib/cpufreq.h lib/cpuidle.h lib/sysfs.h lib/msr.h
+LIB_HEADERS = 	lib/cpufreq.h lib/cpuidle.h lib/sysfs.h lib/msr.h lib/cpuid.h
 LIB_SRC = 	lib/cpufreq.c lib/cpuidle.c lib/sysfs.c lib/msr.c
 LIB_OBJS = 	lib/cpufreq.o lib/cpuidle.o lib/sysfs.o lib/msr.o
 
diff --git a/lib/cpuid.h b/lib/cpuid.h
new file mode 100644
index 0000000..5f179db
--- /dev/null
+++ b/lib/cpuid.h
@@ -0,0 +1,65 @@
+#ifndef _CPUFREQ_CPUID_H
+#define _CPUFREQ_CPUID_H
+
+#include <stdint.h>
+
+static inline void __cpuid(unsigned int *eax, unsigned int *ebx,
+				unsigned int *ecx, unsigned int *edx)
+{
+	/* ecx is often an input as well as an output. */
+	asm volatile("cpuid"
+	    : "=a" (*eax),
+	      "=b" (*ebx),
+	      "=c" (*ecx),
+	      "=d" (*edx)
+	    : "0" (*eax), "2" (*ecx));
+}
+static inline void cpuid(unsigned int op,
+			 unsigned int *eax, unsigned int *ebx,
+			 unsigned int *ecx, unsigned int *edx)
+{
+	*eax = op;
+	*ecx = 0;
+	__cpuid(eax, ebx, ecx, edx);
+}
+
+/*
+ * CPUID functions returning a single datum
+ */
+static inline unsigned int cpuid_eax(unsigned int op)
+{
+	unsigned int eax, ebx, ecx, edx;
+
+	cpuid(op, &eax, &ebx, &ecx, &edx);
+
+	return eax;
+}
+
+static inline unsigned int cpuid_ebx(unsigned int op)
+{
+	unsigned int eax, ebx, ecx, edx;
+
+	cpuid(op, &eax, &ebx, &ecx, &edx);
+
+	return ebx;
+}
+
+static inline unsigned int cpuid_ecx(unsigned int op)
+{
+	unsigned int eax, ebx, ecx, edx;
+
+	cpuid(op, &eax, &ebx, &ecx, &edx);
+
+	return ecx;
+}
+
+static inline unsigned int cpuid_edx(unsigned int op)
+{
+	unsigned int eax, ebx, ecx, edx;
+
+	cpuid(op, &eax, &ebx, &ecx, &edx);
+
+	return edx;
+}
+
+#endif /* _CPUFREQ_CPUID_H */
diff --git a/utils/cpuid.h b/utils/cpuid.h
deleted file mode 100644
index 2bac69a..0000000
--- a/utils/cpuid.h
+++ /dev/null
@@ -1,63 +0,0 @@
-#ifndef _CPUFREQ_CPUID_H
-#define _CPUFREQ_CPUID_H
-
-static inline void __cpuid(unsigned int *eax, unsigned int *ebx,
-				unsigned int *ecx, unsigned int *edx)
-{
-	/* ecx is often an input as well as an output. */
-	asm volatile("cpuid"
-	    : "=a" (*eax),
-	      "=b" (*ebx),
-	      "=c" (*ecx),
-	      "=d" (*edx)
-	    : "0" (*eax), "2" (*ecx));
-}
-static inline void cpuid(unsigned int op,
-			 unsigned int *eax, unsigned int *ebx,
-			 unsigned int *ecx, unsigned int *edx)
-{
-	*eax = op;
-	*ecx = 0;
-	__cpuid(eax, ebx, ecx, edx);
-}
-
-/*
- * CPUID functions returning a single datum
- */
-static inline unsigned int cpuid_eax(unsigned int op)
-{
-	unsigned int eax, ebx, ecx, edx;
-
-	cpuid(op, &eax, &ebx, &ecx, &edx);
-
-	return eax;
-}
-
-static inline unsigned int cpuid_ebx(unsigned int op)
-{
-	unsigned int eax, ebx, ecx, edx;
-
-	cpuid(op, &eax, &ebx, &ecx, &edx);
-
-	return ebx;
-}
-
-static inline unsigned int cpuid_ecx(unsigned int op)
-{
-	unsigned int eax, ebx, ecx, edx;
-
-	cpuid(op, &eax, &ebx, &ecx, &edx);
-
-	return ecx;
-}
-
-static inline unsigned int cpuid_edx(unsigned int op)
-{
-	unsigned int eax, ebx, ecx, edx;
-
-	cpuid(op, &eax, &ebx, &ecx, &edx);
-
-	return edx;
-}
-
-#endif /* _CPUFREQ_CPUID_H */
-- 
1.6.4.2

[PATCH 4/5] cpupowerutils: Add get_cpu_info(..) func to cpuid.h

[Thomas Renninger]

In fact the info is fetch through /proc/cpuinfo and not through cpuid for now.
This call can fetch:
	cpu_info->vendor
	cpu_info->family
	cpu_info->model
	cpu_info->stepping
	cpu_info->cpuid_level
	cpu_info->ext_cpuid_level

Signed-off-by: Thomas Renninger <trenn@suse.de>
CC: Dominik Brodowski <linux@dominikbrodowski.net>
CC: cpufreq@vger.kernel.org
---
 lib/cpuid.h |   86 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 86 insertions(+), 0 deletions(-)

diff --git a/lib/cpuid.h b/lib/cpuid.h
index 5f179db..c00c58d 100644
--- a/lib/cpuid.h
+++ b/lib/cpuid.h
@@ -3,6 +3,22 @@
 
 #include <stdint.h>
 
+enum cpupower_cpu_vendor {X86_VENDOR_UNKNOWN = 0, X86_VENDOR_INTEL,
+			  X86_VENDOR_AMD, X86_VENDOR_MAX};
+
+static const char *cpu_vendor_table[] = {
+	"Unknown", "GenuineIntel", "AuthenticAMD",
+};
+
+struct cpupower_cpu_info {
+	enum cpupower_cpu_vendor vendor;
+	unsigned int family;
+	unsigned int model;
+	unsigned int stepping;
+	unsigned int cpuid_level;
+	uint32_t     ext_cpuid_level;
+};
+
 static inline void __cpuid(unsigned int *eax, unsigned int *ebx,
 				unsigned int *ecx, unsigned int *edx)
 {
@@ -62,4 +78,74 @@ static inline unsigned int cpuid_edx(unsigned int op)
 	return edx;
 }
 
+/* get_cpu_info
+ *
+ * Extract CPU vendor, family, model, stepping info from /proc/cpuinfo
+ *
+ * Returns 0 on success or a negativ error code
+ *
+ * TBD: Should there be a cpuid alternative for this if /proc is not mounted?
+ */
+int get_cpu_info(unsigned int cpu, struct cpupower_cpu_info *cpu_info)
+{
+	FILE *fp;
+	char value[64];
+	unsigned int proc, x;
+	unsigned int unknown = 0xffffff;
+
+	cpu_info->vendor		= X86_VENDOR_UNKNOWN;
+	cpu_info->family		= unknown;
+	cpu_info->model			= unknown;
+	cpu_info->stepping		= unknown;
+	cpu_info->cpuid_level		= cpuid_eax(0);
+	cpu_info->ext_cpuid_level	= cpuid_eax(0x80000000);
+
+
+	fp = fopen("/proc/cpuinfo", "r");
+	if (!fp)
+		return -EIO;
+
+	while (!feof(fp)) {
+		if (!fgets(value, 64, fp))
+			continue;
+		value[63 - 1] = '\0';
+
+		if (!strncmp(value, "processor\t: ", 12)) {
+			sscanf(value, "processor\t: %u", &proc);
+		}
+		if (proc != cpu)
+			continue;
+
+		/* Get CPU vendor */
+		if (!strncmp(value, "vendor_id", 9))
+			for (x = 1; x < X86_VENDOR_MAX; x++) {
+				if (strstr(value, cpu_vendor_table[x]))
+					cpu_info->vendor = x;
+			}
+		/* Get CPU family, etc. */
+		else if (!strncmp(value, "cpu family\t: ", 13)) {
+			sscanf(value, "cpu family\t: %u",
+			       &cpu_info->family);
+		}
+		else if (!strncmp(value, "model\t\t: ", 9)) {
+			sscanf(value, "model\t\t: %u",
+			       &cpu_info->model);
+		}
+		else if (!strncmp(value, "stepping\t: ", 10)) {
+			sscanf(value, "stepping\t: %u",
+			       &cpu_info->stepping);
+
+			/* Exit -> all values must have been set */
+			if (cpu_info->vendor == X86_VENDOR_UNKNOWN ||
+			    cpu_info->family == unknown ||
+			    cpu_info->model == unknown ||
+			    cpu_info->stepping == unknown)
+				return -EINVAL;
+
+			return 0;
+		}
+	}
+	return -ENODEV;
+}
+
 #endif /* _CPUFREQ_CPUID_H */
-- 
1.6.4.2

[PATCH 5/5] cpupowerutils: Introduce -b/-t --boost/--turbo cpufreq-info param

[Thomas Renninger]

Prints out this by default (also works with --cpu X param):
  Analyzing Boost Capabilities on CPU 0:
  Supported: yes
  Active: yes

With activation one has to be careful...
On AMD, it's enough if any of the CPUs shows "Active: no" and boost mode
is not active (cmp with powernow-k8 kernel code).

Not exactly sure how this works on Intel,
Intel® 64 and IA-32 Architectures Software Developer’s Manual
Volume 3A, System Programming Guide, Part 1
14.3.2.2 OS Control of Opportunistic Processor Performance Operation
says:
------
The DISENAGE bit in IA32_PERF_CTL (..) is not shared between logical
processors in a physical package.
------
So it sounds as if boost mode can get disabled on every processor
socket, or even more fine grained?

Possible further enhancements:
   cpufreq-set --turbo_active (or similar)
to enable/disable turbo/boost mode.

For AMD there already is:
/sys/devices/system/cpu/cpu0/cpufreq/cpb
but this could all get handled in userspace and this recently introduced
interface could get removed again.
Then enabling/disabling could all be done on CPU 0 which cannot be taken
offline.
  -> To be discussed.

For Intel, some input about which other cores are affected by disabling
boost mode on a specific core is needed.
  -> To be discussed.

Potentially dangerous is if cores get offlined while boost mode got disbled,
then the userspace tool would not be able to enable it again.
But as this stuff is meant for debugging and performance measuring only,
it should be enough to document this a bit in a manpage...


Signed-off-by: Thomas Renninger <trenn@suse.de>
CC: Dominik Brodowski <linux@dominikbrodowski.net>
CC: cpufreq@vger.kernel.org
CC: Borislav Petkov <borislav.petkov@amd.com>
CC: Len Brown <len.brown@intel.com>
---
 lib/cpufreq.c        |   37 +++++++++++++++++++++++++++++++++++++
 lib/cpufreq.h        |    8 ++++++++
 lib/msr.c            |   37 +++++++++++++++++++++++++++++++++++++
 lib/msr.h            |    5 +++++
 utils/cpufreq-info.c |   29 ++++++++++++++++++++++++++++-
 5 files changed, 115 insertions(+), 1 deletions(-)

diff --git a/lib/cpufreq.c b/lib/cpufreq.c
index ae7d8c5..0b5fe9f 100644
--- a/lib/cpufreq.c
+++ b/lib/cpufreq.c
@@ -12,6 +12,8 @@
 
 #include "cpufreq.h"
 #include "sysfs.h"
+#include "cpuid.h"
+#include "msr.h"
 
 int cpufreq_cpu_exists(unsigned int cpu)
 {
@@ -188,3 +190,38 @@ unsigned long cpufreq_get_transitions(unsigned int cpu) {
 
 	return (ret);
 }
+
+int cpufreq_has_boost_support(unsigned int cpu, int *support, int *active)
+{
+	struct cpupower_cpu_info cpu_info;
+	int ret;
+
+	*support = *active = 0;
+
+	ret = get_cpu_info(0, &cpu_info);
+	if (ret)
+		return ret;
+
+	if (cpu_info.vendor == X86_VENDOR_INTEL) {
+		ret = msr_intel_has_boost_support(cpu);
+		if (ret <= 0)
+			return ret;
+		*support = ret;
+		ret = msr_intel_boost_is_active(cpu);
+		if (ret <= 0)
+			return ret;
+		*active = ret;
+	} else if (cpu_info.vendor == X86_VENDOR_AMD) {
+		if (cpu_info.ext_cpuid_level < 0x80000007)
+			return 0;
+		if ((cpuid_edx(0x80000007) >> 9) & 0x1)
+			*support = 1;
+		else
+			return 0;
+		ret = msr_amd_boost_is_active(cpu);
+		if (ret <= 0)
+			return ret;
+		*active = ret;
+	}
+	return 0;
+}
diff --git a/lib/cpufreq.h b/lib/cpufreq.h
index 03be906..506b6fc 100644
--- a/lib/cpufreq.h
+++ b/lib/cpufreq.h
@@ -208,6 +208,14 @@ extern int cpufreq_modify_policy_governor(unsigned int cpu, char *governor);
 
 extern int cpufreq_set_frequency(unsigned int cpu, unsigned long target_frequency);
 
+/* get boost mode support/activation
+ *
+ * Check whether Intel's "Turbo Boost Technology" or AMD's
+ * "Dynamic Speed Boost Technology" is supported and if, whether it's activated
+ */
+extern int cpufreq_has_boost_support(unsigned int cpu, int *support, int *active);
+
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/msr.c b/lib/msr.c
index bccea8d..9921ff1 100644
--- a/lib/msr.c
+++ b/lib/msr.c
@@ -18,6 +18,8 @@
 #include <sys/stat.h>
 #include <sys/types.h>
 
+#define MSR_IA32_MISC_ENABLES	0x1a0
+
 
 /* General X86 MSRs */
 #define MSR_IA32_APERF		0x000000E8
@@ -25,9 +27,11 @@
 
 /* AMD specific MSRs */
 #define MSR_FIDVID_STATUS	0xc0010042
+#define MSR_K7_HWCR		0xc0010015
 
 /* Intel specific MSRs */
 #define MSR_IA32_PERF_STATUS	0x198
+#define MSR_IA32_MISC_ENABLES	0x1a0
 
 /* AMD specific bits */
 #define AMD_S_HI_CURRENT_VID	0x0000001f
@@ -123,8 +127,41 @@ int msr_amd_get_fidvid(unsigned int cpu, uint32_t *fid, uint32_t *vid)
 	return 0;
 }
 
+int msr_amd_boost_is_active(unsigned int cpu)
+{
+	uint64_t k7_hwcr;
+	int ret;
+
+	ret = read_msr(cpu, MSR_K7_HWCR, &k7_hwcr);
+	if (ret)
+		return ret;
+	return !((k7_hwcr >> 25) & 0x1);
+}
+
 /* Intel X86 MSRs **********************************/
 int msr_intel_get_perf_status(unsigned int cpu, uint64_t perf_status)
 {
        return read_msr(cpu, MSR_IA32_PERF_STATUS, &perf_status);
 }
+
+int msr_intel_has_boost_support(unsigned int cpu)
+{
+	uint64_t misc_enables;
+	int ret;
+
+	ret = read_msr(cpu, MSR_IA32_MISC_ENABLES, &misc_enables);
+	if (ret)
+		return ret;
+	return (misc_enables >> 38) & 0x1;
+}
+
+int msr_intel_boost_is_active(unsigned int cpu)
+{
+	uint64_t perf_status;
+	int ret;
+
+	ret = read_msr(cpu, MSR_IA32_PERF_STATUS, &perf_status);
+	if (ret)
+		return ret;
+	return (perf_status >> 32) & 0x1;
+}
diff --git a/lib/msr.h b/lib/msr.h
index 8b1755e..6b743ce 100644
--- a/lib/msr.h
+++ b/lib/msr.h
@@ -3,3 +3,8 @@ extern int msr_get_aperf(unsigned int cpu, uint64_t *aperf);
 
 extern int msr_amd_get_fidvid(unsigned int cpu, uint32_t *fid, uint32_t *vid);
 extern int msr_intel_get_perf_status(unsigned int cpu, uint64_t perf_status);
+
+extern int msr_intel_has_boost_support(unsigned int cpu);
+extern int msr_intel_boost_is_active(unsigned int cpu);
+
+extern int msr_amd_boost_is_active(unsigned int cpu);
diff --git a/utils/cpufreq-info.c b/utils/cpufreq-info.c
index 38d906a..be0c9e3 100644
--- a/utils/cpufreq-info.c
+++ b/utils/cpufreq-info.c
@@ -16,6 +16,7 @@
 #include <getopt.h>
 
 #include "cpufreq.h"
+#include "cpuid.h"
 
 
 #define _(String) gettext (String)
@@ -286,6 +287,24 @@ static void debug_output(unsigned int cpu, unsigned int all) {
 		debug_output_one(cpu);
 }
 
+/* --boost / -b */
+
+static int get_boost_mode(unsigned int cpu) {
+	int support, active, ret;
+
+	ret = cpufreq_has_boost_support(cpu, &support, &active);
+	if (ret) {
+		printf(gettext ("Error while evaluating Boost Capabilities"
+				" on CPU %d:\n"), cpu);
+		return ret;
+	}
+
+      	printf(gettext ("Analyzing Boost Capabilities on CPU %d:\n"), cpu);
+	printf("Supported: %s\n", support ? "yes" : "no");
+	printf("Active: %s\n", active ? "yes" : "no");
+
+	return 0;
+}
 
 /* --freq / -f */
 
@@ -473,6 +492,8 @@ static void print_help(void) {
 static struct option info_opts[] = {
 	{ .name="cpu",		.has_arg=required_argument,	.flag=NULL,	.val='c'},
 	{ .name="debug",	.has_arg=no_argument,		.flag=NULL,	.val='e'},
+	{ .name="turbo",	.has_arg=no_argument,		.flag=NULL,	.val='t'},
+	{ .name="boost",	.has_arg=no_argument,		.flag=NULL,	.val='b'},
 	{ .name="freq",		.has_arg=no_argument,		.flag=NULL,	.val='f'},
 	{ .name="hwfreq",	.has_arg=no_argument,		.flag=NULL,	.val='w'},
 	{ .name="hwlimits",	.has_arg=no_argument,		.flag=NULL,	.val='l'},
@@ -501,7 +522,7 @@ int main(int argc, char **argv) {
 	textdomain (PACKAGE);
 
 	do {
-		ret = getopt_long(argc, argv, "c:hoefwldpgrasmy", info_opts, NULL);
+		ret = getopt_long(argc, argv, "c:hoefwldpgrasmytb", info_opts, NULL);
 		switch (ret) {
 		case '?':
 			output_param = '?';
@@ -514,6 +535,8 @@ int main(int argc, char **argv) {
 		case -1:
 			cont = 0;
 			break;
+		case 't':
+		case 'b':
 		case 'o':
 		case 'a':
 		case 'r':
@@ -587,6 +610,10 @@ int main(int argc, char **argv) {
 		print_header();
 		print_help();
 		break;
+	case 't':
+	case 'b':
+		get_boost_mode(cpu);
+		break;
 	case 'o':
 		proc_cpufreq_output();
 		break;
-- 
1.6.4.2

Re: [PATCH 3/5] cpupowerutils: Move utils/cpuid.h to lib/cpuid.h

[Thomas Renninger]

On Tuesday 05 October 2010 14:23:12 Thomas Renninger wrote:
> So that utils/*.c code can make use of this as well.
Should be:
So that lib/*.c code can make use of this as well.

    Thomas

Re: [PATCH 5/5] cpupowerutils: Introduce -b/-t --boost/--turbo cpufreq-info param

[Mattia Dongili]

On Tue, Oct 05, 2010 at 02:23:14PM +0200, Thomas Renninger wrote:
> Prints out this by default (also works with --cpu X param):
>   Analyzing Boost Capabilities on CPU 0:
>   Supported: yes
>   Active: yes

It'd be nice to also have manpage updates with these additional options.

Thanks
-- 
mattia
:wq!

Re: [PATCH 5/5] cpupowerutils: Introduce -b/-t --boost/--turbo cpufreq-info param

[Thomas Renninger]

On Tuesday 05 October 2010 15:25:10 Mattia Dongili wrote:
> On Tue, Oct 05, 2010 at 02:23:14PM +0200, Thomas Renninger wrote:
> > Prints out this by default (also works with --cpu X param):
> >   Analyzing Boost Capabilities on CPU 0:
> >   Supported: yes
> >   Active: yes
> 
> It'd be nice to also have manpage updates with these additional
> options.
Yep.
As mentioned in the changelog some feedback how exactly disabling
boost mode on Intel works, would be great.
Code should be fine, but the specifics (disabled on one/all cores, etc.)
should get mentioned in a man page.

BTW: I forgot to close the file descriptor in patch 4/5 in:
get_cpu_info(..).

     Thomas

[PATCH] cpupowerutils: Add get_cpu_info(..) func to cpuid.h V2

[Thomas Renninger]

In fact the info is fetch through /proc/cpuinfo and not through cpuid for now.
This call can fetch:
	cpu_info->vendor
	cpu_info->family
	cpu_info->model
	cpu_info->stepping
	cpu_info->cpuid_level
	cpu_info->ext_cpuid_level

V2: Close file descriptor in get_cpu_info(..)

->TBD: split this up into cpuid.h and cpuid.c

Signed-off-by: Thomas Renninger <trenn@suse.de>
CC: Dominik Brodowski <linux@dominikbrodowski.net>
CC: cpufreq@vger.kernel.org
---
 lib/cpuid.h |   88 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 88 insertions(+), 0 deletions(-)

diff --git a/lib/cpuid.h b/lib/cpuid.h
index 5f179db..469baa2 100644
--- a/lib/cpuid.h
+++ b/lib/cpuid.h
@@ -3,6 +3,22 @@
 
 #include <stdint.h>
 
+enum cpupower_cpu_vendor {X86_VENDOR_UNKNOWN = 0, X86_VENDOR_INTEL,
+			  X86_VENDOR_AMD, X86_VENDOR_MAX};
+
+static const char *cpu_vendor_table[] = {
+	"Unknown", "GenuineIntel", "AuthenticAMD",
+};
+
+struct cpupower_cpu_info {
+	enum cpupower_cpu_vendor vendor;
+	unsigned int family;
+	unsigned int model;
+	unsigned int stepping;
+	unsigned int cpuid_level;
+	uint32_t     ext_cpuid_level;
+};
+
 static inline void __cpuid(unsigned int *eax, unsigned int *ebx,
 				unsigned int *ecx, unsigned int *edx)
 {
@@ -62,4 +78,76 @@ static inline unsigned int cpuid_edx(unsigned int op)
 	return edx;
 }
 
+/* get_cpu_info
+ *
+ * Extract CPU vendor, family, model, stepping info from /proc/cpuinfo
+ *
+ * Returns 0 on success or a negativ error code
+ *
+ * TBD: Should there be a cpuid alternative for this if /proc is not mounted?
+ */
+int get_cpu_info(unsigned int cpu, struct cpupower_cpu_info *cpu_info)
+{
+	FILE *fp;
+	char value[64];
+	unsigned int proc, x;
+	unsigned int unknown = 0xffffff;
+
+	cpu_info->vendor		= X86_VENDOR_UNKNOWN;
+	cpu_info->family		= unknown;
+	cpu_info->model			= unknown;
+	cpu_info->stepping		= unknown;
+	cpu_info->cpuid_level		= cpuid_eax(0);
+	cpu_info->ext_cpuid_level	= cpuid_eax(0x80000000);
+
+
+	fp = fopen("/proc/cpuinfo", "r");
+	if (!fp)
+		return -EIO;
+
+	while (!feof(fp)) {
+		if (!fgets(value, 64, fp))
+			continue;
+		value[63 - 1] = '\0';
+
+		if (!strncmp(value, "processor\t: ", 12)) {
+			sscanf(value, "processor\t: %u", &proc);
+		}
+		if (proc != cpu)
+			continue;
+
+		/* Get CPU vendor */
+		if (!strncmp(value, "vendor_id", 9))
+			for (x = 1; x < X86_VENDOR_MAX; x++) {
+				if (strstr(value, cpu_vendor_table[x]))
+					cpu_info->vendor = x;
+			}
+		/* Get CPU family, etc. */
+		else if (!strncmp(value, "cpu family\t: ", 13)) {
+			sscanf(value, "cpu family\t: %u",
+			       &cpu_info->family);
+		}
+		else if (!strncmp(value, "model\t\t: ", 9)) {
+			sscanf(value, "model\t\t: %u",
+			       &cpu_info->model);
+		}
+		else if (!strncmp(value, "stepping\t: ", 10)) {
+			sscanf(value, "stepping\t: %u",
+			       &cpu_info->stepping);
+
+			/* Exit -> all values must have been set */
+			if (cpu_info->vendor == X86_VENDOR_UNKNOWN ||
+			    cpu_info->family == unknown ||
+			    cpu_info->model == unknown ||
+			    cpu_info->stepping == unknown)
+				return -EINVAL;
+
+			fclose(fp);
+			return 0;
+		}
+	}
+	fclose(fp);
+	return -ENODEV;
+}
+
 #endif /* _CPUFREQ_CPUID_H */
-- 
1.6.4.2

Re: cpupowerutils: easier use of msr and cpuid stuff and some more

[Dominik Brodowski]

Applied, thanks Thomas.

A few comments:
- cpuid.h probably should be split up into a ".h" and a ".c" part
- Compiling msr.c:                                                      [OK]     
	  lib/msr.c:80: warning: ‘write_msr’ defined but not used
- instead of adding two options to cpufreq-info doing the same,
  I only added one (--boost / -b) 

Best,
	Dominik

Re: cpupowerutils: easier use of msr and cpuid stuff and some more

[Thomas Renninger]

On Tuesday 05 October 2010 16:15:32 Dominik Brodowski wrote:
> Applied, thanks Thomas.
> 
> A few comments:
> - cpuid.h probably should be split up into a ".h" and a ".c" part
Yep, I agree.

> - Compiling msr.c:                                                      [OK]     
> 	  lib/msr.c:80: warning: ‘write_msr’ defined but not used
Hm, I think I would comment it out for now instead of deleting.
It will probably be needed soon.

> - instead of adding two options to cpufreq-info doing the same,
>   I only added one (--boost / -b) 
I agree.

I can send updates if you pushed them in.
That would be easier for me, instead of reworking/reposting.
On sever bugs/build failures, etc., I will of course repost.

Thanks,

   Thomas

Re: cpupowerutils: easier use of msr and cpuid stuff and some more

[Dominik Brodowski]

Hey,

On Tue, Oct 05, 2010 at 04:37:57PM +0200, Thomas Renninger wrote:
> On Tuesday 05 October 2010 16:15:32 Dominik Brodowski wrote:
> > - instead of adding two options to cpufreq-info doing the same,
> >   I only added one (--boost / -b) 
> I agree.
> 
> I can send updates if you pushed them in.

For the -b / -t thing, I already modified the patch; otherwise see

git://git.kernel.org/pub/scm/utils/kernel/cpufreq/cpufrequtils.git cpupowerutils 

Best,
	Dominik

Re: [PATCH 5/5] cpupowerutils: Introduce -b/-t --boost/--turbo cpufreq-info param

[Borislav Petkov]

From: Thomas Renninger <trenn@suse.de>
Date: Tue, Oct 05, 2010 at 08:23:14AM -0400

> Prints out this by default (also works with --cpu X param):
>   Analyzing Boost Capabilities on CPU 0:
>   Supported: yes
>   Active: yes

I think it would be simpler if you dump the boosting information in
cpufreq-info, i.e. without an explicit --boost option or whatever. You
can then use the "--boost" option to control the boosting like this:

cpufreq-set --boost (on|off) - toggles boosting
cpufreq-set --boost - without an option could dump the current boosting setting.

Hmm...

> With activation one has to be careful...
> On AMD, it's enough if any of the CPUs shows "Active: no" and boost mode
> is not active (cmp with powernow-k8 kernel code).

yes. But we keep it consistent so that all cores show either off or on.

<snip>

> Possible further enhancements:
>    cpufreq-set --turbo_active (or similar)

see above.

> to enable/disable turbo/boost mode.
> 
> For AMD there already is:
> /sys/devices/system/cpu/cpu0/cpufreq/cpb
> but this could all get handled in userspace and this recently introduced
> interface could get removed again.

I don't think it will be removed soon. Rather, if you use the /sysfs
interface you need kernel support for it and cpufrequtils might run on
older kernels which don't have the feature yet. So you want to do all
the detection/control in userspace, independent from the kernel version.

> Then enabling/disabling could all be done on CPU 0 which cannot be taken
> offline.
>   -> To be discussed.

You need to enable/disable the boosting on AMD by toggling bit 25 in
MSR_K7_HWCR on all cpus.

> Potentially dangerous is if cores get offlined while boost mode got
> disbled, then the userspace tool would not be able to enable it again.
> But as this stuff is meant for debugging and performance measuring
> only, it should be enough to document this a bit in a manpage...

You can issue a warning whenever you detect that a subset of the cores
has been offlined. Then the tool should fail changing the boosting
state, IMHO.

> Signed-off-by: Thomas Renninger <trenn@suse.de>
> CC: Dominik Brodowski <linux@dominikbrodowski.net>
> CC: cpufreq@vger.kernel.org
> CC: Borislav Petkov <borislav.petkov@amd.com>
> CC: Len Brown <len.brown@intel.com>
> ---
>  lib/cpufreq.c        |   37 +++++++++++++++++++++++++++++++++++++
>  lib/cpufreq.h        |    8 ++++++++
>  lib/msr.c            |   37 +++++++++++++++++++++++++++++++++++++
>  lib/msr.h            |    5 +++++
>  utils/cpufreq-info.c |   29 ++++++++++++++++++++++++++++-
>  5 files changed, 115 insertions(+), 1 deletions(-)
> 
> diff --git a/lib/cpufreq.c b/lib/cpufreq.c
> index ae7d8c5..0b5fe9f 100644
> --- a/lib/cpufreq.c
> +++ b/lib/cpufreq.c
> @@ -12,6 +12,8 @@
>  
>  #include "cpufreq.h"
>  #include "sysfs.h"
> +#include "cpuid.h"
> +#include "msr.h"
>  
>  int cpufreq_cpu_exists(unsigned int cpu)
>  {
> @@ -188,3 +190,38 @@ unsigned long cpufreq_get_transitions(unsigned int cpu) {
>  
>  	return (ret);
>  }
> +
> +int cpufreq_has_boost_support(unsigned int cpu, int *support, int *active)
> +{
> +	struct cpupower_cpu_info cpu_info;
> +	int ret;
> +
> +	*support = *active = 0;
> +
> +	ret = get_cpu_info(0, &cpu_info);
> +	if (ret)
> +		return ret;
> +
> +	if (cpu_info.vendor == X86_VENDOR_INTEL) {
> +		ret = msr_intel_has_boost_support(cpu);
> +		if (ret <= 0)
> +			return ret;
> +		*support = ret;
> +		ret = msr_intel_boost_is_active(cpu);
> +		if (ret <= 0)
> +			return ret;
> +		*active = ret;
> +	} else if (cpu_info.vendor == X86_VENDOR_AMD) {
> +		if (cpu_info.ext_cpuid_level < 0x80000007)
> +			return 0;
> +		if ((cpuid_edx(0x80000007) >> 9) & 0x1)
> +			*support = 1;
> +		else
> +			return 0;

wrap this in amd_has_boost_support()?

> +		ret = msr_amd_boost_is_active(cpu);
> +		if (ret <= 0)
> +			return ret;
> +		*active = ret;
> +	}
> +	return 0;
> +}
> diff --git a/lib/cpufreq.h b/lib/cpufreq.h
> index 03be906..506b6fc 100644
> --- a/lib/cpufreq.h
> +++ b/lib/cpufreq.h
> @@ -208,6 +208,14 @@ extern int cpufreq_modify_policy_governor(unsigned int cpu, char *governor);
>  
>  extern int cpufreq_set_frequency(unsigned int cpu, unsigned long target_frequency);
>  
> +/* get boost mode support/activation
> + *
> + * Check whether Intel's "Turbo Boost Technology" or AMD's
> + * "Dynamic Speed Boost Technology" is supported and if, whether it's activated
> + */
> +extern int cpufreq_has_boost_support(unsigned int cpu, int *support, int *active);
> +
> +
>  #ifdef __cplusplus
>  }
>  #endif
> diff --git a/lib/msr.c b/lib/msr.c
> index bccea8d..9921ff1 100644
> --- a/lib/msr.c
> +++ b/lib/msr.c
> @@ -18,6 +18,8 @@
>  #include <sys/stat.h>
>  #include <sys/types.h>
>  
> +#define MSR_IA32_MISC_ENABLES	0x1a0
> +
>  
>  /* General X86 MSRs */
>  #define MSR_IA32_APERF		0x000000E8
> @@ -25,9 +27,11 @@
>  
>  /* AMD specific MSRs */
>  #define MSR_FIDVID_STATUS	0xc0010042
> +#define MSR_K7_HWCR		0xc0010015
>  
>  /* Intel specific MSRs */
>  #define MSR_IA32_PERF_STATUS	0x198
> +#define MSR_IA32_MISC_ENABLES	0x1a0
>  
>  /* AMD specific bits */
>  #define AMD_S_HI_CURRENT_VID	0x0000001f
> @@ -123,8 +127,41 @@ int msr_amd_get_fidvid(unsigned int cpu, uint32_t *fid, uint32_t *vid)
>  	return 0;
>  }
>  
> +int msr_amd_boost_is_active(unsigned int cpu)
> +{
> +	uint64_t k7_hwcr;
> +	int ret;
> +
> +	ret = read_msr(cpu, MSR_K7_HWCR, &k7_hwcr);
> +	if (ret)
> +		return ret;
> +	return !((k7_hwcr >> 25) & 0x1);
> +}

This should be done differently on AMD: we want to
iterate over all cores and check this bit and see
whether its setting is consistent. See how it is done in
<arch/x86/kernel/cpu/cpufreq/powernow-k8.c::powernowk8_init()> in the
kernel.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH 5/5] cpupowerutils: Introduce -b/-t --boost/--turbo cpufreq-info param

[Dominik Brodowski]

Hey,

On Tue, Oct 05, 2010 at 04:47:17PM +0200, Borislav Petkov wrote:
> From: Thomas Renninger <trenn@suse.de>
> Date: Tue, Oct 05, 2010 at 08:23:14AM -0400
> 
> > Prints out this by default (also works with --cpu X param):
> >   Analyzing Boost Capabilities on CPU 0:
> >   Supported: yes
> >   Active: yes
> 
> I think it would be simpler if you dump the boosting information in
> cpufreq-info, i.e. without an explicit --boost option or whatever.

Only works if you are root, AFAICS.

Best,
	Dominik

Re: [PATCH 5/5] cpupowerutils: Introduce -b/-t --boost/--turbo cpufreq-info param

[Borislav Petkov]

From: Dominik Brodowski <linux@dominikbrodowski.net>
Date: Tue, Oct 05, 2010 at 10:52:32AM -0400

Hi Dominik,

> > > Prints out this by default (also works with --cpu X param):
> > >   Analyzing Boost Capabilities on CPU 0:
> > >   Supported: yes
> > >   Active: yes
> > 
> > I think it would be simpler if you dump the boosting information in
> > cpufreq-info, i.e. without an explicit --boost option or whatever.
> 
> Only works if you are root, AFAICS.

true, it depends on the /dev/cpu/<NUM>/msr permissions and those should
be root:root.

Ok then, we could concentrate the boosting in the cpufreq-set command
which should check for the calling user id being root or similar:

cpufreq-set --boost (on|off) - toggles boosting
cpufreq-set --boost - without an option could dump the current boosting setting.

This is much more intuitive IMHO.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH 5/5] cpupowerutils: Introduce -b/-t --boost/--turbo cpufreq-info param

[Thomas Renninger]

On Tuesday 05 October 2010 16:47:17 Borislav Petkov wrote:
> From: Thomas Renninger <trenn@suse.de>
> Date: Tue, Oct 05, 2010 at 08:23:14AM -0400
> 
> > Prints out this by default (also works with --cpu X param):
> >   Analyzing Boost Capabilities on CPU 0:
> >   Supported: yes
> >   Active: yes
> 
> I think it would be simpler if you dump the boosting information in
> cpufreq-info, i.e. without an explicit --boost option or whatever. You
> can then use the "--boost" option to control the boosting like this:
> 
> cpufreq-set --boost (on|off) - toggles boosting
Yep, I like to add this next.

> > With activation one has to be careful...
> > On AMD, it's enough if any of the CPUs shows "Active: no" and boost mode
> > is not active (cmp with powernow-k8 kernel code).
> 
> yes. But we keep it consistent so that all cores show either off or on.
I am still not sure whether cpufreq-info should access this through
/sys/devices/system/cpu/cpu0/cpufreq/cpb
or directly via msr.

Hm, if it's accessed via /sys/..cpufreq/cpb it should be enough to only touch
CPU0, right? The rest is done by powernow-k8.

I don't like cpb much. It's the only file that has no info in it's name.
There should have been a general interface:
boost_mode
supporting AMD and Intel...
Possibly this can still be done and cpb can get marked deprecated
(it shows up in 2.6.36 the first time? Which is not released yet?
Theoretically this is not part of the ABI yet...).

> > to enable/disable turbo/boost mode.
> > 
> > For AMD there already is:
> > /sys/devices/system/cpu/cpu0/cpufreq/cpb
> > but this could all get handled in userspace and this recently introduced
> > interface could get removed again.
> 
> I don't think it will be removed soon. Rather, if you use the /sysfs
> interface you need kernel support for it and cpufrequtils might run on
> older kernels which don't have the feature yet. So you want to do all
> the detection/control in userspace, independent from the kernel version.
Hmm, I'd prefer cleaner code (and only differing Intel/AMD once, either in
the kernel or in userspace), but the "you need kernel support for it and
cpufrequtils might run on older kernel" arguement is interesting.
Not sure what is more important. Need to think a bit more about this.
Comments are very welcome.
 
> > Then enabling/disabling could all be done on CPU 0 which cannot be taken
> > offline.
> >   -> To be discussed.
> 
> You need to enable/disable the boosting on AMD by toggling bit 25 in
> MSR_K7_HWCR on all cpus.
Ok.
> 
> > Potentially dangerous is if cores get offlined while boost mode got
> > disbled, then the userspace tool would not be able to enable it again.
> > But as this stuff is meant for debugging and performance measuring
> > only, it should be enough to document this a bit in a manpage...
> 
> You can issue a warning whenever you detect that a subset of the cores
> has been offlined. Then the tool should fail changing the boosting
> state, IMHO.
Good idea.

...
> > +	} else if (cpu_info.vendor == X86_VENDOR_AMD) {
> > +		if (cpu_info.ext_cpuid_level < 0x80000007)
> > +			return 0;
> > +		if ((cpuid_edx(0x80000007) >> 9) & 0x1)
> > +			*support = 1;
> > +		else
> > +			return 0;
> 
> wrap this in amd_has_boost_support()?
Yep, after splitting out cpuid.c, this could get put into:
cpuid_amd_has_boost_support.
...
> > +int msr_amd_boost_is_active(unsigned int cpu)
> > +{
> > +	uint64_t k7_hwcr;
> > +	int ret;
> > +
> > +	ret = read_msr(cpu, MSR_K7_HWCR, &k7_hwcr);
> > +	if (ret)
> > +		return ret;
> > +	return !((k7_hwcr >> 25) & 0x1);
> > +}
> 
> This should be done differently on AMD: we want to
> iterate over all cores and check this bit and see
> whether its setting is consistent. See how it is done in
> <arch/x86/kernel/cpu/cpufreq/powernow-k8.c::powernowk8_init()> in the
> kernel.
Yep and this is why the whole code should either sit in userspace
or in the kernel, duplicating the code would be stupid.
IMO it should be in userspace, it's just for debugging/monitoring, etc.
and if it's not too late already cpb sysfs file should better vanish...

      Thomas

Re: [PATCH 5/5] cpupowerutils: Introduce -b/-t --boost/--turbo cpufreq-info param

[Borislav Petkov]

From: Thomas Renninger <trenn@suse.de>
Date: Tue, Oct 05, 2010 at 11:18:06AM -0400

> > yes. But we keep it consistent so that all cores show either off or on.
> I am still not sure whether cpufreq-info should access this through
> /sys/devices/system/cpu/cpu0/cpufreq/cpb
> or directly via msr.
> 
> Hm, if it's accessed via /sys/..cpufreq/cpb it should be enough to only touch
> CPU0, right? The rest is done by powernow-k8.
> 
> I don't like cpb much. It's the only file that has no info in it's name.
> There should have been a general interface:
> boost_mode
> supporting AMD and Intel...
> Possibly this can still be done and cpb can get marked deprecated
> (it shows up in 2.6.36 the first time? Which is not released yet?
> Theoretically this is not part of the ABI yet...).

No, cpb got introduced in .35.

And no, we don't want to remove it because we need to be able to toggle
CPB without the need for installing a special tool for that.

> > > to enable/disable turbo/boost mode.
> > > 
> > > For AMD there already is:
> > > /sys/devices/system/cpu/cpu0/cpufreq/cpb
> > > but this could all get handled in userspace and this recently introduced
> > > interface could get removed again.
> > 
> > I don't think it will be removed soon. Rather, if you use the /sysfs
> > interface you need kernel support for it and cpufrequtils might run on
> > older kernels which don't have the feature yet. So you want to do all
> > the detection/control in userspace, independent from the kernel version.
> Hmm, I'd prefer cleaner code (and only differing Intel/AMD once, either in
> the kernel or in userspace), but the "you need kernel support for it and
> cpufrequtils might run on older kernel" arguement is interesting.
> Not sure what is more important. Need to think a bit more about this.
> Comments are very welcome.

Ok, look at it this way: On the one hand, you need to be able to toggle
boosting without having to install a tool for that. On the other, it is
also important that cpufrequtils runs on as many kernels as possible. So
you want to implement the toggling in userspace too. And I don't see an
issue with code duplication because you already have most of it - you
only need to iterate over the num_online_cpus and toggle the bit on each
MSR. 10 additional lines tops.

<snip>

> > This should be done differently on AMD: we want to
> > iterate over all cores and check this bit and see
> > whether its setting is consistent. See how it is done in
> > <arch/x86/kernel/cpu/cpufreq/powernow-k8.c::powernowk8_init()> in the
> > kernel.
> Yep and this is why the whole code should either sit in userspace
> or in the kernel, duplicating the code would be stupid.
> IMO it should be in userspace, it's just for debugging/monitoring, etc.
> and if it's not too late already cpb sysfs file should better vanish...

see above.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH 5/5] cpupowerutils: Introduce -b/-t --boost/--turbo cpufreq-info param

[Thomas Renninger]

Hi,

and sorry for the late response...

On Tuesday 05 October 2010 05:56:24 pm Borislav Petkov wrote:
> From: Thomas Renninger <trenn@suse.de>
> Date: Tue, Oct 05, 2010 at 11:18:06AM -0400
> 
> > > yes. But we keep it consistent so that all cores show either off or on.
> > I am still not sure whether cpufreq-info should access this through
> > /sys/devices/system/cpu/cpu0/cpufreq/cpb
> > or directly via msr.
> > 
> > Hm, if it's accessed via /sys/..cpufreq/cpb it should be enough to only touch
> > CPU0, right? The rest is done by powernow-k8.
> > 
> > I don't like cpb much. It's the only file that has no info in it's name.
> > There should have been a general interface:
> > boost_mode
> > supporting AMD and Intel...
> > Possibly this can still be done and cpb can get marked deprecated
> > (it shows up in 2.6.36 the first time? Which is not released yet?
> > Theoretically this is not part of the ABI yet...).
> 
> No, cpb got introduced in .35.
> 
> And no, we don't want to remove it because we need to be able to toggle
> CPB without the need for installing a special tool for that.
Maybe not removing, but having a sysfs file for each cpu even you can
only enable/disable boost for the whole machine looks wrong.
Ok, there may be future machines where you could disable it per CPU socket?,
but what for...
Also the dependency to powernow-k8/cpufreq looks wrong.
After looking closer at this, better would have been:
/sys/devices/system/cpu/boost_mode
if capable. I also didn't think about it, even I looked at the cpb
patch(es) rather close.
Anyway, this is not cpupowerutils most important feature...

> > > > to enable/disable turbo/boost mode.
> > > > 
> > > > For AMD there already is:
> > > > /sys/devices/system/cpu/cpu0/cpufreq/cpb
> > > > but this could all get handled in userspace and this recently introduced
> > > > interface could get removed again.
> > > 
> > > I don't think it will be removed soon. Rather, if you use the /sysfs
> > > interface you need kernel support for it and cpufrequtils might run on
> > > older kernels which don't have the feature yet. So you want to do all
> > > the detection/control in userspace, independent from the kernel version.
> > Hmm, I'd prefer cleaner code (and only differing Intel/AMD once, either in
> > the kernel or in userspace), but the "you need kernel support for it and
> > cpufrequtils might run on older kernel" arguement is interesting.
> > Not sure what is more important. Need to think a bit more about this.
> > Comments are very welcome.
> 
> Ok, look at it this way: On the one hand, you need to be able to toggle
> boosting without having to install a tool for that. On the other, it is
> also important that cpufrequtils runs on as many kernels as possible. So
> you want to implement the toggling in userspace too. And I don't see an
> issue with code duplication because you already have most of it - you
> only need to iterate over the num_online_cpus and toggle the bit on each
> MSR. 10 additional lines tops.
I thought a bit about this and I do not see a way to implement this in
userspace. The reason is CPU onlining/offlining.
If userspace could lock CPUs to not get offlined, then set cpb enable
or disable bits on all cores or break out if not all CPUs are online is
what you need.
But you cannot do: "lock CPUs to not get offlined" in userspace and if
you could you would certainly run into quite some other issues.

So for now that param may get implemented through:
/sys/devices/system/cpu/cpu0/cpufreq/cpb
at some time, but there is no urgent need.
I better test a bit more the cpuidle stuff, possibly start writing on a
manpage, announce the latest changes, etc., etc..

Thanks,

       Thomas

Re: [PATCH 5/5] cpupowerutils: Introduce -b/-t --boost/--turbo cpufreq-info param

[Borislav Petkov]

From: Thomas Renninger <trenn@suse.de>
Date: Wed, Oct 13, 2010 at 05:17:18PM -0400

(adding Andreas, I'm sure he'd like to pitch in here too)

Hi Thomas,

> > > Possibly this can still be done and cpb can get marked deprecated
> > > (it shows up in 2.6.36 the first time? Which is not released yet?
> > > Theoretically this is not part of the ABI yet...).
> > 
> > No, cpb got introduced in .35.
> > 
> > And no, we don't want to remove it because we need to be able to toggle
> > CPB without the need for installing a special tool for that.
> Maybe not removing, but having a sysfs file for each cpu even you can
> only enable/disable boost for the whole machine looks wrong.
> Ok, there may be future machines where you could disable it per CPU socket?,
> but what for...

I see your point, having this file per-cpu is redundant since we do the
boosting per system. And yes, I really don't have a sensible usecase
where you might want to have a finer granularity with boost control,
i.e. disable single cores only.

> Also the dependency to powernow-k8/cpufreq looks wrong.
> After looking closer at this, better would have been:
> /sys/devices/system/cpu/boost_mode
> if capable.

Are you saying the boosting code should go somewhere else? We put it in
powernow-k8 since it has mostly to do with Pstates and all...


> > > Hmm, I'd prefer cleaner code (and only differing Intel/AMD once, either in
> > > the kernel or in userspace), but the "you need kernel support for it and
> > > cpufrequtils might run on older kernel" arguement is interesting.
> > > Not sure what is more important. Need to think a bit more about this.
> > > Comments are very welcome.
> > 
> > Ok, look at it this way: On the one hand, you need to be able to toggle
> > boosting without having to install a tool for that. On the other, it is
> > also important that cpufrequtils runs on as many kernels as possible. So
> > you want to implement the toggling in userspace too. And I don't see an
> > issue with code duplication because you already have most of it - you
> > only need to iterate over the num_online_cpus and toggle the bit on each
> > MSR. 10 additional lines tops.
> I thought a bit about this and I do not see a way to implement this in
> userspace. The reason is CPU onlining/offlining.
> If userspace could lock CPUs to not get offlined, then set cpb enable
> or disable bits on all cores or break out if not all CPUs are online is
> what you need.
> But you cannot do: "lock CPUs to not get offlined" in userspace and if
> you could you would certainly run into quite some other issues.

I think you're referring to the maybe-possible case when cores
disappear from under you while you're toggling boosting, right? Hmm,
we could maybe do get/put_cpu() in <arch/x86/kernel/msr.c> to have it
hotplug-safe to a certain degree but there might be cases where this
might not fly...

> So for now that param may get implemented through:
> /sys/devices/system/cpu/cpu0/cpufreq/cpb
> at some time, but there is no urgent need.

On the other hand, having a single system-wide sysfs knob is simpler for
userspace. Hmm, let me dream a bit more on this. However, if we do it
this way, cpufreq will depend on a kernel version. Implementing boosting
in it would be trivial, though :)

Thanks.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632