Re: [PATCH] xen/arm: vpsci: ignore upper 32 bits for SMC32 PSCI arguments

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Mykola,

> On 18 Mar 2026, at 19:56, Mykola Kvach <xakep.amatop@xxxxxxxxx> wrote:
> 
> From: Mykola Kvach <mykola_kvach@xxxxxxxx>
> 
> SMCCC DEN0028G, section 3.1, states that for AArch64 SMC/HVC calls
> using Wn, only the least significant 32 bits are significant and the
> upper 32 bits must be ignored by the implementation.
> 
> So for SMC32 PSCI calls, Xen must not treat non-zero upper bits in the
> argument registers as an error. Instead, they should be discarded when
> decoding the arguments.
> 
> Arm ARM DDI 0487J.a (D1-5406) also notes that the upper 32 bits may be
> implementation defined when entering from AArch32. Xen zeros them on
> entry, but that guarantee is only relevant for 32-bit domains.
> 
> Update PSCI v0.2+ CPU_ON, CPU_SUSPEND and AFFINITY_INFO to read SMC32
> arguments via PSCI_ARG32(), while keeping the SMC64 handling unchanged.
> 
> Suggested-by: Julien Grall <julien@xxxxxxx>
> Signed-off-by: Mykola Kvach <mykola_kvach@xxxxxxxx>
> ---
> Link to discussion: 
> https://patchew.org/Xen/cover.1751020456.git.mykola._5Fkvach@xxxxxxxx/072270e0940b6bcc2743d56a336363f4719ba60a.1751020456.git.mykola._5Fkvach@xxxxxxxx/#7070f416-119c-49f8-acd0-82c6e31f0fc6@xxxxxxx
> ---
> xen/arch/arm/vpsci.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c
> index 7ba9ccd94b..1e844ed571 100644
> --- a/xen/arch/arm/vpsci.c
> +++ b/xen/arch/arm/vpsci.c
> @@ -303,9 +303,10 @@ bool do_vpsci_0_2_call(struct cpu_user_regs *regs, 
> uint32_t fid)
>     case PSCI_0_2_FN32_CPU_ON:
>     case PSCI_0_2_FN64_CPU_ON:
>     {
> -        register_t vcpuid = PSCI_ARG(regs, 1);
> -        register_t epoint = PSCI_ARG(regs, 2);
> -        register_t cid = PSCI_ARG(regs, 3);
> +        bool smc32 = (fid == PSCI_0_2_FN32_CPU_ON);
> +        register_t vcpuid = smc32 ? PSCI_ARG32(regs, 1) : PSCI_ARG(regs, 1);
> +        register_t epoint = smc32 ? PSCI_ARG32(regs, 2) : PSCI_ARG(regs, 2);
> +        register_t cid = smc32 ? PSCI_ARG32(regs, 3) : PSCI_ARG(regs, 3);

It might be nicer to modify PSCI_ARG to take a convention argument instead of
redoing the same test everywhere, this would make the code nicer and ensure no 
PSCI_ARG
would have been forgotten.

At the end all those conventions are coming from smccc so we could:
- use smccc_is_conv_64(fid) from smccc.h to get 32 vs 64
- use smccc_get_fn to get the function id without the convention and reduce the 
number of entries
in the switch

That would allow to get the convention and fn before the switch and simplify a 
bit the code.

The smccc part is definitely not something i would enforce in this patch but 
could still be nice to do
it if you are willing to.

Cheers
Bertrand

> 
>         perfc_incr(vpsci_cpu_on);
>         PSCI_SET_RESULT(regs, do_psci_0_2_cpu_on(vcpuid, epoint, cid));
> @@ -315,9 +316,10 @@ bool do_vpsci_0_2_call(struct cpu_user_regs *regs, 
> uint32_t fid)
>     case PSCI_0_2_FN32_CPU_SUSPEND:
>     case PSCI_0_2_FN64_CPU_SUSPEND:
>     {
> +        bool smc32 = (fid == PSCI_0_2_FN32_CPU_SUSPEND);
>         uint32_t pstate = PSCI_ARG32(regs, 1);
> -        register_t epoint = PSCI_ARG(regs, 2);
> -        register_t cid = PSCI_ARG(regs, 3);
> +        register_t epoint = smc32 ? PSCI_ARG32(regs, 2) : PSCI_ARG(regs, 2);
> +        register_t cid = smc32 ? PSCI_ARG32(regs, 3) : PSCI_ARG(regs, 3);
> 
>         perfc_incr(vpsci_cpu_suspend);
>         PSCI_SET_RESULT(regs, do_psci_0_2_cpu_suspend(pstate, epoint, cid));
> @@ -327,7 +329,8 @@ bool do_vpsci_0_2_call(struct cpu_user_regs *regs, 
> uint32_t fid)
>     case PSCI_0_2_FN32_AFFINITY_INFO:
>     case PSCI_0_2_FN64_AFFINITY_INFO:
>     {
> -        register_t taff = PSCI_ARG(regs, 1);
> +        bool smc32 = (fid == PSCI_0_2_FN32_AFFINITY_INFO);
> +        register_t taff = smc32 ? PSCI_ARG32(regs, 1) : PSCI_ARG(regs, 1);
>         uint32_t laff = PSCI_ARG32(regs, 2);
> 
>         perfc_incr(vpsci_cpu_affinity_info);
> -- 
> 2.43.0
>