* [PATCH] tools/xen-detect: avoid possible pitfall with cpuid()
@ 2021-12-03 12:09 Jan Beulich
2022-01-04 9:52 ` Ping: " Jan Beulich
2022-01-05 14:49 ` Anthony PERARD
0 siblings, 2 replies; 3+ messages in thread
From: Jan Beulich @ 2021-12-03 12:09 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Andrew Cooper, Wei Liu, Roger Pau Monné
The 64-bit form forces %ecx to 0 while the 32-bit one so far didn't - it
only ended up that way when "pv_context" is zero. While presently no
leaf queried by callers has separate subleaves, let's avoid chancing it.
While there
- replace references to operands by number,
- relax constraints where possible,
- limit PUSH/POP to just the registers not also used as input,
all where applicable also for the 64-bit variant.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
I'm pretty sure %edx also wouldn't need to be subject to PUSH/POP here,
but I didn't want to go more "against" the comment than obviously
justifiable by the input registers used. In fact I've observed gcc to
pick %edx for putting "pv_context" in.
--- a/tools/misc/xen-detect.c
+++ b/tools/misc/xen-detect.c
@@ -52,17 +52,19 @@ static void cpuid(uint32_t idx, uint32_t
#ifdef __i386__
/* Use the stack to avoid reg constraint failures with some gcc flags */
asm volatile (
- "push %%eax; push %%ebx; push %%ecx; push %%edx\n\t"
- "test %1,%1 ; jz 1f ; ud2a ; .ascii \"xen\" ; 1: cpuid\n\t"
- "mov %%eax,(%2); mov %%ebx,4(%2)\n\t"
- "mov %%ecx,8(%2); mov %%edx,12(%2)\n\t"
- "pop %%edx; pop %%ecx; pop %%ebx; pop %%eax\n\t"
- : : "a" (idx), "c" (pv_context), "S" (regs) : "memory" );
+ "push %%ebx; push %%edx\n\t"
+ "test %[pv],%[pv] ; jz 1f ; ud2a ; .ascii \"xen\" ; 1: cpuid\n\t"
+ "mov %%eax,(%[regs]); mov %%ebx,4(%[regs])\n\t"
+ "mov %%ecx,8(%[regs]); mov %%edx,12(%[regs])\n\t"
+ "pop %%edx; pop %%ebx\n\t"
+ : "+a" (idx), "=c" (idx /* dummy */)
+ : "c" (0), [pv] "r" (pv_context), [regs] "SD" (regs)
+ : "memory" );
#else
asm volatile (
- "test %5,%5 ; jz 1f ; ud2a ; .ascii \"xen\" ; 1: cpuid\n\t"
+ "test %[pv],%[pv] ; jz 1f ; ud2a ; .ascii \"xen\" ; 1: cpuid\n\t"
: "=a" (regs[0]), "=b" (regs[1]), "=c" (regs[2]), "=d" (regs[3])
- : "0" (idx), "1" (pv_context), "2" (0) );
+ : "0" (idx), "2" (0), [pv] "r" (pv_context) );
#endif
}
^ permalink raw reply [flat|nested] 3+ messages in thread
* Ping: [PATCH] tools/xen-detect: avoid possible pitfall with cpuid()
2021-12-03 12:09 [PATCH] tools/xen-detect: avoid possible pitfall with cpuid() Jan Beulich
@ 2022-01-04 9:52 ` Jan Beulich
2022-01-05 14:49 ` Anthony PERARD
1 sibling, 0 replies; 3+ messages in thread
From: Jan Beulich @ 2022-01-04 9:52 UTC (permalink / raw)
To: Wei Liu, Anthony Perard; +Cc: Andrew Cooper, Roger Pau Monné, xen-devel
On 03.12.2021 13:09, Jan Beulich wrote:
> The 64-bit form forces %ecx to 0 while the 32-bit one so far didn't - it
> only ended up that way when "pv_context" is zero. While presently no
> leaf queried by callers has separate subleaves, let's avoid chancing it.
>
> While there
> - replace references to operands by number,
> - relax constraints where possible,
> - limit PUSH/POP to just the registers not also used as input,
> all where applicable also for the 64-bit variant.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Any chance of getting an ack or otherwise here? (Anthony, I realize you've
not been on the Cc list originally, as the patch we sent before the
maintainership adjustment.)
Jan
> ---
> I'm pretty sure %edx also wouldn't need to be subject to PUSH/POP here,
> but I didn't want to go more "against" the comment than obviously
> justifiable by the input registers used. In fact I've observed gcc to
> pick %edx for putting "pv_context" in.
>
> --- a/tools/misc/xen-detect.c
> +++ b/tools/misc/xen-detect.c
> @@ -52,17 +52,19 @@ static void cpuid(uint32_t idx, uint32_t
> #ifdef __i386__
> /* Use the stack to avoid reg constraint failures with some gcc flags */
> asm volatile (
> - "push %%eax; push %%ebx; push %%ecx; push %%edx\n\t"
> - "test %1,%1 ; jz 1f ; ud2a ; .ascii \"xen\" ; 1: cpuid\n\t"
> - "mov %%eax,(%2); mov %%ebx,4(%2)\n\t"
> - "mov %%ecx,8(%2); mov %%edx,12(%2)\n\t"
> - "pop %%edx; pop %%ecx; pop %%ebx; pop %%eax\n\t"
> - : : "a" (idx), "c" (pv_context), "S" (regs) : "memory" );
> + "push %%ebx; push %%edx\n\t"
> + "test %[pv],%[pv] ; jz 1f ; ud2a ; .ascii \"xen\" ; 1: cpuid\n\t"
> + "mov %%eax,(%[regs]); mov %%ebx,4(%[regs])\n\t"
> + "mov %%ecx,8(%[regs]); mov %%edx,12(%[regs])\n\t"
> + "pop %%edx; pop %%ebx\n\t"
> + : "+a" (idx), "=c" (idx /* dummy */)
> + : "c" (0), [pv] "r" (pv_context), [regs] "SD" (regs)
> + : "memory" );
> #else
> asm volatile (
> - "test %5,%5 ; jz 1f ; ud2a ; .ascii \"xen\" ; 1: cpuid\n\t"
> + "test %[pv],%[pv] ; jz 1f ; ud2a ; .ascii \"xen\" ; 1: cpuid\n\t"
> : "=a" (regs[0]), "=b" (regs[1]), "=c" (regs[2]), "=d" (regs[3])
> - : "0" (idx), "1" (pv_context), "2" (0) );
> + : "0" (idx), "2" (0), [pv] "r" (pv_context) );
> #endif
> }
>
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] tools/xen-detect: avoid possible pitfall with cpuid()
2021-12-03 12:09 [PATCH] tools/xen-detect: avoid possible pitfall with cpuid() Jan Beulich
2022-01-04 9:52 ` Ping: " Jan Beulich
@ 2022-01-05 14:49 ` Anthony PERARD
1 sibling, 0 replies; 3+ messages in thread
From: Anthony PERARD @ 2022-01-05 14:49 UTC (permalink / raw)
To: Jan Beulich
Cc: xen-devel, Ian Jackson, Andrew Cooper, Wei Liu, Roger Pau Monné
On Fri, Dec 03, 2021 at 01:09:04PM +0100, Jan Beulich wrote:
> The 64-bit form forces %ecx to 0 while the 32-bit one so far didn't - it
> only ended up that way when "pv_context" is zero. While presently no
> leaf queried by callers has separate subleaves, let's avoid chancing it.
>
> While there
> - replace references to operands by number,
> - relax constraints where possible,
> - limit PUSH/POP to just the registers not also used as input,
> all where applicable also for the 64-bit variant.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Thanks,
--
Anthony PERARD
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-01-05 14:50 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-03 12:09 [PATCH] tools/xen-detect: avoid possible pitfall with cpuid() Jan Beulich
2022-01-04 9:52 ` Ping: " Jan Beulich
2022-01-05 14:49 ` Anthony PERARD
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.