Jan Beulich
2008-Jan-21 15:18 UTC
[Xen-devel] [PATCH] linux/x86: avoid casting hypercall arguments to long
.., providing a little more type correctness checking and producing
better code on 64-bits. This includes replacing the directly encoded
moves to %r10 and %r8 on x86-64 by (extended) C-language constructs,
allowing the compiler''s scheduler more flexibility.
As usual, written and tested on 2.6.24-rc8 and made apply to the 2.6.18
tree without further testing.
Signed-off-by: Jan Beulich <jbeulich@novell.com>
Index: head-2008-01-21/include/asm-i386/mach-xen/asm/hypercall.h
==================================================================---
head-2008-01-21.orig/include/asm-i386/mach-xen/asm/hypercall.h 2008-01-21
13:31:43.000000000 +0100
+++ head-2008-01-21/include/asm-i386/mach-xen/asm/hypercall.h 2008-01-21
13:31:50.000000000 +0100
@@ -67,7 +67,7 @@
asm volatile ( \
HYPERCALL_STR(name) \
: "=a" (__res), "=b" (__ign1) \
- : "1" ((long)(a1)) \
+ : "1" ((a1)?:0) \
: "memory" ); \
(type)__res; \
})
@@ -78,7 +78,7 @@
asm volatile ( \
HYPERCALL_STR(name) \
: "=a" (__res), "=b" (__ign1), "=c" (__ign2) \
- : "1" ((long)(a1)), "2" ((long)(a2)) \
+ : "1" ((a1)?:0), "2" ((a2)?:0) \
: "memory" ); \
(type)__res; \
})
@@ -89,9 +89,8 @@
asm volatile ( \
HYPERCALL_STR(name) \
: "=a" (__res), "=b" (__ign1), "=c" (__ign2),
\
- "=d" (__ign3) \
- : "1" ((long)(a1)), "2" ((long)(a2)), \
- "3" ((long)(a3)) \
+ "=d" (__ign3) \
+ : "1" ((a1)?:0), "2" ((a2)?:0), "3" ((a3)?:0) \
: "memory" ); \
(type)__res; \
})
@@ -102,9 +101,9 @@
asm volatile ( \
HYPERCALL_STR(name) \
: "=a" (__res), "=b" (__ign1), "=c" (__ign2), \
- "=d" (__ign3), "=S" (__ign4) \
- : "1" ((long)(a1)), "2" ((long)(a2)), \
- "3" ((long)(a3)), "4" ((long)(a4)) \
+ "=d" (__ign3), "=S" (__ign4) \
+ : "1" ((a1)?:0), "2" ((a2)?:0), \
+ "3" ((a3)?:0), "4" ((a4)?:0) \
: "memory" ); \
(type)__res; \
})
@@ -115,10 +114,9 @@
asm volatile ( \
HYPERCALL_STR(name) \
: "=a" (__res), "=b" (__ign1), "=c" (__ign2), \
- "=d" (__ign3), "=S" (__ign4), "=D" (__ign5) \
- : "1" ((long)(a1)), "2" ((long)(a2)), \
- "3" ((long)(a3)), "4" ((long)(a4)), \
- "5" ((long)(a5)) \
+ "=d" (__ign3), "=S" (__ign4), "=D" (__ign5) \
+ : "1" ((a1)?:0), "2" ((a2)?:0), \
+ "3" ((a3)?:0), "4" ((a4)?:0), "5" ((a5)?:0) \
: "memory" ); \
(type)__res; \
})
@@ -226,7 +224,11 @@ static inline int __must_check
HYPERVISOR_update_descriptor(
u64 ma, u64 desc)
{
- return _hypercall4(int, update_descriptor, ma, ma>>32, desc,
desc>>32);
+ unsigned long ma_hi = (unsigned long)(ma>>32);
+ unsigned long ma_lo = (unsigned long)ma;
+ unsigned long desc_hi = (unsigned long)(desc>>32);
+ unsigned long desc_lo = (unsigned long)desc;
+ return _hypercall4(int, update_descriptor, ma_lo, ma_hi, desc_lo, desc_hi);
}
static inline int __must_check
Index: head-2008-01-21/include/asm-x86_64/mach-xen/asm/hypercall.h
==================================================================---
head-2008-01-21.orig/include/asm-x86_64/mach-xen/asm/hypercall.h 2008-01-21
13:31:43.000000000 +0100
+++ head-2008-01-21/include/asm-x86_64/mach-xen/asm/hypercall.h 2008-01-21
13:31:50.000000000 +0100
@@ -56,77 +56,74 @@
#define _hypercall0(type, name) \
({ \
- long __res; \
+ type __res; \
asm volatile ( \
HYPERCALL_STR(name) \
: "=a" (__res) \
: \
: "memory" ); \
- (type)__res; \
+ __res; \
})
#define _hypercall1(type, name, a1) \
({ \
- long __res, __ign1; \
+ type __res, __ign1; \
asm volatile ( \
HYPERCALL_STR(name) \
: "=a" (__res), "=D" (__ign1) \
- : "1" ((long)(a1)) \
+ : "1" ((a1)?:0) \
: "memory" ); \
- (type)__res; \
+ __res; \
})
#define _hypercall2(type, name, a1, a2) \
({ \
- long __res, __ign1, __ign2; \
+ type __res, __ign1, __ign2; \
asm volatile ( \
HYPERCALL_STR(name) \
: "=a" (__res), "=D" (__ign1), "=S" (__ign2) \
- : "1" ((long)(a1)), "2" ((long)(a2)) \
+ : "1" ((a1)?:0), "2" ((a2)?:0) \
: "memory" ); \
- (type)__res; \
+ __res; \
})
#define _hypercall3(type, name, a1, a2, a3) \
({ \
- long __res, __ign1, __ign2, __ign3; \
+ type __res, __ign1, __ign2, __ign3; \
asm volatile ( \
HYPERCALL_STR(name) \
: "=a" (__res), "=D" (__ign1), "=S" (__ign2),
\
- "=d" (__ign3) \
- : "1" ((long)(a1)), "2" ((long)(a2)), \
- "3" ((long)(a3)) \
+ "=d" (__ign3) \
+ : "1" ((a1)?:0), "2" ((a2)?:0), "3" ((a3)?:0) \
: "memory" ); \
- (type)__res; \
+ __res; \
})
#define _hypercall4(type, name, a1, a2, a3, a4) \
({ \
- long __res, __ign1, __ign2, __ign3; \
+ type __res, __ign1, __ign2, __ign3; \
+ register typeof((a4)?:0) __arg4 asm("r10") = (a4); \
asm volatile ( \
- "movq %7,%%r10; " \
HYPERCALL_STR(name) \
: "=a" (__res), "=D" (__ign1), "=S" (__ign2), \
- "=d" (__ign3) \
- : "1" ((long)(a1)), "2" ((long)(a2)), \
- "3" ((long)(a3)), "g" ((long)(a4)) \
- : "memory", "r10" ); \
- (type)__res; \
+ "=d" (__ign3), "+r" (__arg4) \
+ : "1" ((a1)?:0), "2" ((a2)?:0), "3" ((a3)?:0) \
+ : "memory" ); \
+ __res; \
})
#define _hypercall5(type, name, a1, a2, a3, a4, a5) \
({ \
- long __res, __ign1, __ign2, __ign3; \
+ type __res, __ign1, __ign2, __ign3; \
+ register typeof((a4)?:0) __arg4 asm("r10") = (a4); \
+ register typeof((a5)?:0) __arg5 asm("r8") = (a5); \
asm volatile ( \
- "movq %7,%%r10; movq %8,%%r8; " \
HYPERCALL_STR(name) \
: "=a" (__res), "=D" (__ign1), "=S" (__ign2), \
- "=d" (__ign3) \
- : "1" ((long)(a1)), "2" ((long)(a2)), \
- "3" ((long)(a3)), "g" ((long)(a4)), \
- "g" ((long)(a5)) \
- : "memory", "r10", "r8" ); \
- (type)__res; \
+ "=d" (__ign3), "+r" (__arg4), "+r" (__arg5) \
+ : "1" ((a1)?:0), "2" ((a2)?:0), "3" ((a3)?:0) \
+ : "memory" ); \
+ __res; \
})
static inline int __must_check
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Keir Fraser
2008-Jan-21 16:27 UTC
Re: [Xen-devel] [PATCH] linux/x86: avoid casting hypercall arguments to long
On 21/1/08 15:18, "Jan Beulich" <jbeulich@novell.com> wrote:> - : "1" ((long)(a1)) \ > + : "1" ((a1)?:0) \Most of the patch seems to be placing this construct all over the place. What''s it for? How is x?:0 different from x?> + register typeof((a4)?:0) __arg4 asm("r10") = (a4); \ > + register typeof((a5)?:0) __arg5 asm("r8") = (a5); \ > + "=d" (__ign3), "+r" (__arg4), "+r" (__arg5) \Is it guaranteed that gcc will keep the register variables in their assigned registers across the inline asm? -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jan Beulich
2008-Jan-21 16:42 UTC
Re: [Xen-devel] [PATCH] linux/x86: avoid casting hypercallarguments to long
>>> Keir Fraser <Keir.Fraser@cl.cam.ac.uk> 21.01.08 17:27 >>> >On 21/1/08 15:18, "Jan Beulich" <jbeulich@novell.com> wrote: > >> - : "1" ((long)(a1)) \ >> + : "1" ((a1)?:0) \ > >Most of the patch seems to be placing this construct all over the place. >What''s it for? How is x?:0 different from x?In its type: This way, the type gets promoted to int for anything smaller than int, but left alone if long (or wider) or pointer. Various gcc versions appear to have various problems when not promoting to at least int...>> + register typeof((a4)?:0) __arg4 asm("r10") = (a4); \ >> + register typeof((a5)?:0) __arg5 asm("r8") = (a5); \ >> + "=d" (__ign3), "+r" (__arg4), "+r" (__arg5) \ > >Is it guaranteed that gcc will keep the register variables in their assigned >registers across the inline asm?That''s how they document it, so I suppose this is the canonical way and can be expected to not break. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2008-Jan-21 17:18 UTC
Re: [Xen-devel] [PATCH] linux/x86: avoid casting hypercallarguments to long
On 21/1/08 16:42, "Jan Beulich" <jbeulich@novell.com> wrote:>>>> Keir Fraser <Keir.Fraser@cl.cam.ac.uk> 21.01.08 17:27 >>> >> On 21/1/08 15:18, "Jan Beulich" <jbeulich@novell.com> wrote: >> > In its type: This way, the type gets promoted to int for anything smaller > than int, but left alone if long (or wider) or pointer. Various gcc versions > appear to have various problems when not promoting to at least int...It''s not really clear that passing an int to 64-bit inline asm guarantees that the upper 32 bits of the 64-bit register are zero, is it? If you pass a 32-bit value to inline asm then gcc emits 32-bit register operands. I''ll admit it''s very *likely* that the value will turn out to be zero-extended, if you go at the argument with an explicit 64-bit access, because of x86_64 register write semantics, but not clear it''s guaranteed.>> Is it guaranteed that gcc will keep the register variables in their assigned >> registers across the inline asm? > > That''s how they document it, so I suppose this is the canonical way > and can be expected to not break.Is it documented in the ''info gcc'' documentation somewhere? -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel