* [PATCH v3 1/1] tools/power x86_energy_perf_policy: fix cpuid for i686
@ 2013-02-14 19:35 Benson Leung
2013-02-14 19:35 ` Benson Leung
2013-02-19 20:18 ` [PATCH v4 0/1] " Benson Leung
0 siblings, 2 replies; 6+ messages in thread
From: Benson Leung @ 2013-02-14 19:35 UTC (permalink / raw)
To: len.brown, colin.king, linux-kernel; +Cc: bleung, vapier
Hi Len,
Please take a look at this patch to the x86_energy_perf_policy that allows
the tool to work when built for i686 with PIC enabled.
Thanks,
Benson
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 1/1] tools/power x86_energy_perf_policy: fix cpuid for i686
2013-02-14 19:35 [PATCH v3 1/1] tools/power x86_energy_perf_policy: fix cpuid for i686 Benson Leung
@ 2013-02-14 19:35 ` Benson Leung
2013-02-18 5:06 ` Len Brown
2013-02-18 5:17 ` Mike Frysinger
2013-02-19 20:18 ` [PATCH v4 0/1] " Benson Leung
1 sibling, 2 replies; 6+ messages in thread
From: Benson Leung @ 2013-02-14 19:35 UTC (permalink / raw)
To: len.brown, colin.king, linux-kernel; +Cc: bleung, vapier
x86_energy_perf_policy reads cpuid using the cpuid instruction.
On i686, when building with PIC, this clobbers ebx, the PIC register.
Fixed using the same cpuid accessor function that vapier@gentoo.org
created for i7z:
http://code.google.com/p/i7z/issues/detail?id=31
Signed-off-by: Benson Leung <bleung@chromium.org>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
v3 : No changes.
v2 : make cpuid static inline. Remove unused eax from validate_cpuid and
clean up return variables from cpuid().
---
.../x86_energy_perf_policy.c | 30 +++++++++++++++++-----
1 file changed, 23 insertions(+), 7 deletions(-)
diff --git a/tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy.c b/tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy.c
index 40b3e54..f8f4e63 100644
--- a/tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy.c
+++ b/tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy.c
@@ -130,20 +130,36 @@ void cmdline(int argc, char **argv)
}
}
+static inline void cpuid(unsigned int info, unsigned int *eax,
+ unsigned int *ebx, unsigned int *ecx,
+ unsigned int *edx)
+{
+ unsigned int _eax = info, _ebx, _ecx, _edx;
+ asm volatile ("mov %%ebx, %%edi;" /* save ebx (for PIC) */
+ "cpuid;"
+ "mov %%ebx, %%esi;" /* pass to caller */
+ "mov %%edi, %%ebx;" /* restore ebx */
+ :"+a" (_eax), "=S" (_ebx), "=c" (_ecx), "=d" (_edx)
+ : /* inputs: eax is handled above */
+ :"edi" /* clobbers: we hit edi directly */);
+ if (eax) *eax = _eax;
+ if (ebx) *ebx = _ebx;
+ if (ecx) *ecx = _ecx;
+ if (edx) *edx = _edx;
+}
+
/*
* validate_cpuid()
* returns on success, quietly exits on failure (make verbose with -v)
*/
void validate_cpuid(void)
{
- unsigned int eax, ebx, ecx, edx, max_level;
+ unsigned int ebx, ecx, edx, max_level;
unsigned int fms, family, model, stepping;
- eax = ebx = ecx = edx = 0;
-
- asm("cpuid" : "=a" (max_level), "=b" (ebx), "=c" (ecx),
- "=d" (edx) : "a" (0));
+ ebx = ecx = edx = 0;
+ cpuid(0, &max_level, &ebx, &ecx, &edx);
if (ebx != 0x756e6547 || edx != 0x49656e69 || ecx != 0x6c65746e) {
if (verbose)
fprintf(stderr, "%.4s%.4s%.4s != GenuineIntel",
@@ -151,7 +167,7 @@ void validate_cpuid(void)
exit(1);
}
- asm("cpuid" : "=a" (fms), "=c" (ecx), "=d" (edx) : "a" (1) : "ebx");
+ cpuid(1, &fms, NULL, NULL, &edx);
family = (fms >> 8) & 0xf;
model = (fms >> 4) & 0xf;
stepping = fms & 0xf;
@@ -173,7 +189,7 @@ void validate_cpuid(void)
* Support for MSR_IA32_ENERGY_PERF_BIAS
* is indicated by CPUID.06H.ECX.bit3
*/
- asm("cpuid" : "=a" (eax), "=b" (ebx), "=c" (ecx), "=d" (edx) : "a" (6));
+ cpuid(6, NULL, NULL, &ecx, NULL);
if (verbose)
printf("CPUID.06H.ECX: 0x%x\n", ecx);
if (!(ecx & (1 << 3))) {
--
1.8.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/1] tools/power x86_energy_perf_policy: fix cpuid for i686
2013-02-14 19:35 ` Benson Leung
@ 2013-02-18 5:06 ` Len Brown
2013-02-18 5:17 ` Mike Frysinger
1 sibling, 0 replies; 6+ messages in thread
From: Len Brown @ 2013-02-18 5:06 UTC (permalink / raw)
To: Benson Leung; +Cc: colin.king, linux-kernel, vapier
Hi Benson,
checkpatch.pl doesn't love this syntax:
ERROR: spaces required around that ':' (ctx:ExV)
#113: FILE: tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy.c:142:
+ :"+a" (_eax), "=S" (_ebx), "=c" (_ecx), "=d" (_edx)
^
ERROR: spaces required around that ':' (ctx:ExV)
#115: FILE: tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy.c:144:
+ :"edi" /* clobbers: we hit edi directly */);
^
ERROR: trailing statements should be on next line
#116: FILE: tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy.c:145:
+ if (eax) *eax = _eax;
ERROR: trailing statements should be on next line
#117: FILE: tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy.c:146:
+ if (ebx) *ebx = _ebx;
ERROR: trailing statements should be on next line
#118: FILE: tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy.c:147:
+ if (ecx) *ecx = _ecx;
ERROR: trailing statements should be on next line
#119: FILE: tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy.c:148:
+ if (edx) *edx = _edx;
total: 6 errors, 2 warnings, 57 lines checked
/home/lenb/Documents/epp.eml has style problems, please review.
If any of these errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
Commit Body is:
--------------------------
tools/power x86_energy_perf_policy: fix cpuid for i686
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/1] tools/power x86_energy_perf_policy: fix cpuid for i686
2013-02-14 19:35 ` Benson Leung
2013-02-18 5:06 ` Len Brown
@ 2013-02-18 5:17 ` Mike Frysinger
1 sibling, 0 replies; 6+ messages in thread
From: Mike Frysinger @ 2013-02-18 5:17 UTC (permalink / raw)
To: Benson Leung; +Cc: len.brown, colin.king, linux-kernel
[-- Attachment #1: Type: Text/Plain, Size: 532 bytes --]
On Thursday 14 February 2013 14:35:58 Benson Leung wrote:
> + asm volatile ("mov %%ebx, %%edi;" /* save ebx (for PIC) */
> + "cpuid;"
> + "mov %%ebx, %%esi;" /* pass to caller */
> + "mov %%edi, %%ebx;" /* restore ebx */
> + :"+a" (_eax), "=S" (_ebx), "=c" (_ecx), "=d" (_edx)
> + : /* inputs: eax is handled above */
> + :"edi" /* clobbers: we hit edi directly */);
i've written a better version since:
https://github.com/linux-test-project/ltp/blob/master/include/ltp_cpuid.h
-mike
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v4 0/1] tools/power x86_energy_perf_policy: fix cpuid for i686
2013-02-14 19:35 [PATCH v3 1/1] tools/power x86_energy_perf_policy: fix cpuid for i686 Benson Leung
2013-02-14 19:35 ` Benson Leung
@ 2013-02-19 20:18 ` Benson Leung
2013-02-19 20:18 ` [PATCH v4 1/1] " Benson Leung
1 sibling, 1 reply; 6+ messages in thread
From: Benson Leung @ 2013-02-19 20:18 UTC (permalink / raw)
To: len.brown, colin.king, linux-kernel; +Cc: bleung, vapier
Thanks Len and Mike for your comments. I've fixed the style problems so this
is now checkpatch clean.
I've also moved to Mike's better cpuid from ltp.
Thanks,
Benson
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v4 1/1] tools/power x86_energy_perf_policy: fix cpuid for i686
2013-02-19 20:18 ` [PATCH v4 0/1] " Benson Leung
@ 2013-02-19 20:18 ` Benson Leung
0 siblings, 0 replies; 6+ messages in thread
From: Benson Leung @ 2013-02-19 20:18 UTC (permalink / raw)
To: len.brown, colin.king, linux-kernel; +Cc: bleung, vapier
x86_energy_perf_policy reads cpuid using the cpuid instruction.
On i686, when building with PIC, this clobbers ebx, the PIC register.
Fixed using the cpuid accessor function that vapier@gentoo.org wrote:
https://raw.github.com/linux-test-project/ltp/master/include/ltp_cpuid.h
Signed-off-by: Benson Leung <bleung@chromium.org>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
v4 : Cleaned up checkpatch.pl style errors. Used vapier's newer better cpuid.
v3 : No changes.
v2 : make cpuid static inline. Remove unused eax from validate_cpuid and
clean up return variables from cpuid().
---
.../x86_energy_perf_policy.c | 42 ++++++++++++++++++----
1 file changed, 35 insertions(+), 7 deletions(-)
diff --git a/tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy.c b/tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy.c
index 40b3e54..4372bd2 100644
--- a/tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy.c
+++ b/tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy.c
@@ -130,20 +130,48 @@ void cmdline(int argc, char **argv)
}
}
+static inline void cpuid(unsigned int info, unsigned int *eax,
+ unsigned int *ebx, unsigned int *ecx,
+ unsigned int *edx)
+{
+#if defined(__i386__) || defined(__x86_64__)
+ unsigned int _eax = info, _ebx, _ecx, _edx;
+ asm volatile (
+# ifdef __i386__
+ "xchg %%ebx, %%esi;" /* save ebx (for PIC) */
+ "cpuid;"
+ "xchg %%esi, %%ebx;" /* restore ebx & pass to caller */
+ : "=S" (_ebx),
+#else
+ "cpuid;"
+ : "=b" (_ebx),
+#endif
+ "+a" (_eax), "=c" (_ecx), "=d" (_edx)
+ : /* inputs: eax is handled above */
+ );
+ if (eax)
+ *eax = _eax;
+ if (ebx)
+ *ebx = _ebx;
+ if (ecx)
+ *ecx = _ecx;
+ if (edx)
+ *edx = _edx;
+#endif
+}
+
/*
* validate_cpuid()
* returns on success, quietly exits on failure (make verbose with -v)
*/
void validate_cpuid(void)
{
- unsigned int eax, ebx, ecx, edx, max_level;
+ unsigned int ebx, ecx, edx, max_level;
unsigned int fms, family, model, stepping;
- eax = ebx = ecx = edx = 0;
-
- asm("cpuid" : "=a" (max_level), "=b" (ebx), "=c" (ecx),
- "=d" (edx) : "a" (0));
+ ebx = ecx = edx = 0;
+ cpuid(0, &max_level, &ebx, &ecx, &edx);
if (ebx != 0x756e6547 || edx != 0x49656e69 || ecx != 0x6c65746e) {
if (verbose)
fprintf(stderr, "%.4s%.4s%.4s != GenuineIntel",
@@ -151,7 +179,7 @@ void validate_cpuid(void)
exit(1);
}
- asm("cpuid" : "=a" (fms), "=c" (ecx), "=d" (edx) : "a" (1) : "ebx");
+ cpuid(1, &fms, NULL, NULL, &edx);
family = (fms >> 8) & 0xf;
model = (fms >> 4) & 0xf;
stepping = fms & 0xf;
@@ -173,7 +201,7 @@ void validate_cpuid(void)
* Support for MSR_IA32_ENERGY_PERF_BIAS
* is indicated by CPUID.06H.ECX.bit3
*/
- asm("cpuid" : "=a" (eax), "=b" (ebx), "=c" (ecx), "=d" (edx) : "a" (6));
+ cpuid(6, NULL, NULL, &ecx, NULL);
if (verbose)
printf("CPUID.06H.ECX: 0x%x\n", ecx);
if (!(ecx & (1 << 3))) {
--
1.8.1.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-02-19 20:18 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-02-14 19:35 [PATCH v3 1/1] tools/power x86_energy_perf_policy: fix cpuid for i686 Benson Leung
2013-02-14 19:35 ` Benson Leung
2013-02-18 5:06 ` Len Brown
2013-02-18 5:17 ` Mike Frysinger
2013-02-19 20:18 ` [PATCH v4 0/1] " Benson Leung
2013-02-19 20:18 ` [PATCH v4 1/1] " Benson Leung
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox