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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 16/02/2026 9:31 am, Bertrand Marquis wrote:
> Hi Jan,
>
>> On 16 Feb 2026, at 10:21, Jan Beulich <jbeulich@xxxxxxxx> 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 <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?
>>
> 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