Re: [PATCH] xen/arm: smccc: Preserve argument types in v1.1 SMC

[Jan Beulich <jbeulich@xxxxxxxx>]

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 <bertrand.marquis@xxxxxxx>
> ---
>  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