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 ?
> 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?
Jan
