* [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