2012-02-22 03:15:45

by Benson Leung

[permalink] [raw]
Subject: [PATCH] tools/power x86_energy_perf_policy: fix cpuid for i686

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 [email protected]
created for i7z:
http://code.google.com/p/i7z/issues/detail?id=31

Signed-off-by: Benson Leung <[email protected]>
---
.../x86_energy_perf_policy.c | 25 ++++++++++++++++----
1 files changed, 20 insertions(+), 5 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 33c5c7e..576fbf4 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,6 +130,23 @@ void cmdline(int argc, char **argv)
}
}

+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)
@@ -141,9 +158,7 @@ void validate_cpuid(void)

eax = ebx = ecx = edx = 0;

- asm("cpuid" : "=a" (max_level), "=b" (ebx), "=c" (ecx),
- "=d" (edx) : "a" (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 +166,7 @@ void validate_cpuid(void)
exit(1);
}

- asm("cpuid" : "=a" (fms), "=c" (ecx), "=d" (edx) : "a" (1) : "ebx");
+ cpuid(1, &fms, NULL, &ecx, &edx);
family = (fms >> 8) & 0xf;
model = (fms >> 4) & 0xf;
stepping = fms & 0xf;
@@ -173,7 +188,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, &eax, &ebx, &ecx, &edx);
if (verbose)
printf("CPUID.06H.ECX: 0x%x\n", ecx);
if (!(ecx & (1 << 3))) {
--
1.7.1


2012-02-22 03:41:38

by Mike Frysinger

[permalink] [raw]
Subject: Re: [PATCH] tools/power x86_energy_perf_policy: fix cpuid for i686

On Tue, Feb 21, 2012 at 22:15, Benson Leung wrote:
> --- 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
>
> +void cpuid (unsigned int info, unsigned int *eax, unsigned int *ebx,
> + ? ? ? ? ? unsigned int *ecx, unsigned int *edx)

static (maybe inline?), and probably want to adjust the style by
dropping the spaces before the open paren in this func
-mike

2012-02-22 04:04:16

by Benson Leung

[permalink] [raw]
Subject: [PATCH v2] tools/power x86_energy_perf_policy: fix cpuid for i686

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 [email protected]
created for i7z:
http://code.google.com/p/i7z/issues/detail?id=31

v2 : make cpuid static inline. Remove unused eax from validate_cpuid and
clean up return variables from cpuid().

Signed-off-by: Benson Leung <[email protected]>
---
.../x86_energy_perf_policy.c | 30 +++++++++++++++----
1 files 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 33c5c7e..e13e4e5 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.7.1

2012-02-22 04:40:50

by Mike Frysinger

[permalink] [raw]
Subject: Re: [PATCH v2] tools/power x86_energy_perf_policy: fix cpuid for i686

On Tue, Feb 21, 2012 at 23:03, Benson Leung <[email protected]> wrote:
> 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 [email protected]
> created for i7z:
> http://code.google.com/p/i7z/issues/detail?id=31

not sure if a-b or s-o-b ... i'll post both and you can pick most appropriate ;)

Acked-by: Mike Frysinger <[email protected]>
Signed-off-by: Mike Frysinger <[email protected]>
-mike