Hi Jan, > On 16 Feb 2026, at 10:21, Jan Beulich <[email protected]> wrote: > > On 16.02.2026 08:47, Bertrand Marquis wrote: >> The SMCCC v1.1 inline helper currently forces a1-a4 into >> unsigned long and uses in/out constraints for r0-r3. In >> contrast, a5-a7 are passed with their original types via >> read-only constraints. On arm64 this means a 32-bit signed >> value in a1-a4 is converted to a 64-bit unsigned value, while >> the same value in a5-a7 keeps its signed 32-bit form. For >> example, a negative int in a2 is widened to unsigned long, but >> a negative int in a5 is passed as a 32-bit signed value, so the >> SMC sees different encodings depending on argument position. >> >> Switch the helper to use typed input registers arg0-arg7 >> derived from the call arguments (keeping a0 cast to u32) and >> separate output registers r0-r3. This preserves argument types >> consistently across all positions. Argument evaluation order >> is unchanged, so we do not reintroduce the issue fixed in >> "e00dc325bd9e" ("xen/arm: smccc-1.1: Handle function result as >> parameters"). >> >> This also aligns Xen's SMCCC parameter handling with Linux's type- >> preserving behavior (same externally visible argument handling, >> independent implementation) to avoid surprising differences >> between a1-a4 and a5-a7. >> >> Current callers (PSCI, SCMI, platform SMC pass-through, OP-TEE, >> and exynos5) pass unsigned values; exynos5 passes an int CPU id >> which should always be > 0. > > Reported-by: Andrew ?
Ack, sorry forgot that. > >> Signed-off-by: Bertrand Marquis <[email protected]> >> --- >> xen/arch/arm/include/asm/smccc.h | 69 +++++++++++++------------------- >> 1 file changed, 27 insertions(+), 42 deletions(-) >> >> diff --git a/xen/arch/arm/include/asm/smccc.h >> b/xen/arch/arm/include/asm/smccc.h >> index 441b3ab65dee..5b30dd57b69d 100644 >> --- a/xen/arch/arm/include/asm/smccc.h >> +++ b/xen/arch/arm/include/asm/smccc.h >> @@ -99,87 +99,68 @@ struct arm_smccc_res { >> #define __count_args(...) \ >> ___count_args(__VA_ARGS__, 7, 6, 5, 4, 3, 2, 1, 0) >> >> -#define __constraint_write_0 \ >> - "+r" (r0), "=&r" (r1), "=&r" (r2), "=&r" (r3) >> -#define __constraint_write_1 \ >> - "+r" (r0), "+r" (r1), "=&r" (r2), "=&r" (r3) >> -#define __constraint_write_2 \ >> - "+r" (r0), "+r" (r1), "+r" (r2), "=&r" (r3) >> -#define __constraint_write_3 \ >> - "+r" (r0), "+r" (r1), "+r" (r2), "+r" (r3) >> -#define __constraint_write_4 __constraint_write_3 >> -#define __constraint_write_5 __constraint_write_4 >> -#define __constraint_write_6 __constraint_write_5 >> -#define __constraint_write_7 __constraint_write_6 >> - >> -#define __constraint_read_0 >> -#define __constraint_read_1 >> -#define __constraint_read_2 >> -#define __constraint_read_3 >> -#define __constraint_read_4 "r" (r4) >> -#define __constraint_read_5 __constraint_read_4, "r" (r5) >> -#define __constraint_read_6 __constraint_read_5, "r" (r6) >> -#define __constraint_read_7 __constraint_read_6, "r" (r7) >> +#define __constraint_read_0 "r" (arg0) >> +#define __constraint_read_1 __constraint_read_0, "r" (arg1) >> +#define __constraint_read_2 __constraint_read_1, "r" (arg2) >> +#define __constraint_read_3 __constraint_read_2, "r" (arg3) >> +#define __constraint_read_4 __constraint_read_3, "r" (arg4) >> +#define __constraint_read_5 __constraint_read_4, "r" (arg5) >> +#define __constraint_read_6 __constraint_read_5, "r" (arg6) >> +#define __constraint_read_7 __constraint_read_6, "r" (arg7) >> >> #define __declare_arg_0(a0, res) \ >> struct arm_smccc_res *___res = (res); \ >> - register unsigned long r0 ASM_REG(0) = (uint32_t)(a0); \ >> - register unsigned long r1 ASM_REG(1); \ >> - register unsigned long r2 ASM_REG(2); \ >> - register unsigned long r3 ASM_REG(3) >> + register unsigned long arg0 ASM_REG(0) = (uint32_t)(a0) >> >> #define __declare_arg_1(a0, a1, res) \ >> typeof(a1) __a1 = (a1); \ >> struct arm_smccc_res *___res = (res); \ >> - register unsigned long r0 ASM_REG(0) = (uint32_t)(a0); \ >> - register unsigned long r1 ASM_REG(1) = __a1; \ >> - register unsigned long r2 ASM_REG(2); \ >> - register unsigned long r3 ASM_REG(3) >> + register unsigned long arg0 ASM_REG(0) = (uint32_t)(a0);\ >> + register typeof(a1) arg1 ASM_REG(1) = __a1 > > Is it intentional that you switch to typeof() rather than directly going > to auto? This was it'll be more churn, aiui. And if deliberately going > only half a step, perhaps worth saying so in the description? > Yes it is because Andrew wants to rebase his serie on top of this patch. Cheers Bertrand > Jan
