From 968c974203629a624234ca197db131b22b41efce Mon Sep 17 00:00:00 2001 From: Roger Pau Monne <roger.pau@citrix.com> To: xen-devel@lists.xenproject.org Cc: Jan Beulich <jbeulich@suse.com> Cc: Andrew Cooper <andrew.cooper3@citrix.com> Cc: "Roger Pau Monné" <roger.pau@citrix.com> Cc: Wei Liu <wl@xen.org> Date: Wed, 21 Feb 2024 16:55:12 +0100 Subject: [PATCH] x86/altcall: use an union as register type for function parameters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The current code for alternative calls uses the caller parameter types as the types for the register variables that serve as function parameters: uint8_t foo; [...] alternative_call(myfunc, foo); Would expand roughly into: register unint8_t a1_ asm("rdi") = foo; register unsigned long a2_ asm("rsi"); [...] asm volatile ("call *%c[addr](%%rip)"...); However under certain circumstances clang >= 16.0.0 with -O2 can generate incorrect code, given the following example: unsigned int func(uint8_t t) { return t; } static void bar(uint8_t b) { int ret_; register uint8_t di asm("rdi") = b; register unsigned long si asm("rsi"); register unsigned long dx asm("rdx"); register unsigned long cx asm("rcx"); register unsigned long r8 asm("r8"); register unsigned long r9 asm("r9"); register unsigned long r10 asm("r10"); register unsigned long r11 asm("r11"); asm volatile ( "call %c[addr]" : "+r" (di), "=r" (si), "=r" (dx), "=r" (cx), "=r" (r8), "=r" (r9), "=r" (r10), "=r" (r11), "=a" (ret_) : [addr] "i" (&(func)), "g" (func) : "memory" ); } void foo(unsigned int a) { bar(a); } Clang generates the following code: func: # @func movl %edi, %eax retq foo: # @foo callq func retq Note the truncation of the unsigned int parameter 'a' of foo() to uint8_t when passed into bar() is lost. The above can be worked around by using an union when defining the register variables, so that `di` becomes: register union { uint8_t e; unsigned long r; } di asm("rdi") = { .e = b }; Which results in following code generated for `foo()`: foo: # @foo movzbl %dil, %edi callq func retq So the truncation is not longer lost. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- xen/arch/x86/include/asm/alternative.h | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/xen/arch/x86/include/asm/alternative.h b/xen/arch/x86/include/asm/alternative.h index a1cd6a9fe5b8..837dfd953d2f 100644 --- a/xen/arch/x86/include/asm/alternative.h +++ b/xen/arch/x86/include/asm/alternative.h @@ -167,9 +167,18 @@ extern void alternative_branches(void); #define ALT_CALL_arg5 "r8" #define ALT_CALL_arg6 "r9" -#define ALT_CALL_ARG(arg, n) \ - register typeof(arg) a ## n ## _ asm ( ALT_CALL_arg ## n ) = \ - ({ BUILD_BUG_ON(sizeof(arg) > sizeof(void *)); (arg); }) +/* + * Use an union with an unsigned long in order to prevent clang >= 16.0.0 from + * skipping a possible truncation of the value. By using the union any + * truncation is carried before the call instruction. + */ +#define ALT_CALL_ARG(arg, n) \ + register union { \ + typeof(arg) e; \ + unsigned long r; \ + } a ## n ## _ asm ( ALT_CALL_arg ## n ) = { \ + .e = ({ BUILD_BUG_ON(sizeof(arg) > sizeof(void *)); (arg); }) \ + } #define ALT_CALL_NO_ARG(n) \ register unsigned long a ## n ## _ asm ( ALT_CALL_arg ## n ) -- 2.43.0