On 16/02/2026 9:31 am, Bertrand Marquis wrote: > 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.
This patch potentially wants backporting, but even if it ends up not being, fixing the SMCCC types should not be mixed with the auto conversion; they're both subtle changes and unrelated. ~Andrew
