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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 31.03.2026 20:31, Mykola Kvach 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, AFFINITY_INFO and SYSTEM_SUSPEND
> to read SMC32 arguments via PSCI_ARG32(), while keeping the SMC64
> handling unchanged.
> 
> No functional change is intended for PSCI 0.1.
> 
> Suggested-by: Julien Grall <julien@xxxxxxx>
> Signed-off-by: Mykola Kvach <mykola_kvach@xxxxxxxx>
> Reviewed-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>

I thought I might as well include this in my next commit sweep, but isn't
this R-b being invalidated by ...

> ---
> v3:
>  - use PSCI_ARG_CONV for SYSTEM_SUSPEND

... this change. That's ...

> @@ -422,14 +427,8 @@ bool do_vpsci_0_2_call(struct cpu_user_regs *regs, 
> uint32_t fid)
>      case PSCI_1_0_FN32_SYSTEM_SUSPEND:
>      case PSCI_1_0_FN64_SYSTEM_SUSPEND:
>      {
> -        register_t epoint = PSCI_ARG(regs, 1);
> -        register_t cid = PSCI_ARG(regs, 2);
> -
> -        if ( fid == PSCI_1_0_FN32_SYSTEM_SUSPEND )
> -        {
> -            epoint &= GENMASK(31, 0);
> -            cid &= GENMASK(31, 0);
> -        }
> +        register_t epoint = PSCI_ARG_CONV(regs, 1, is_conv_64);
> +        register_t cid = PSCI_ARG_CONV(regs, 2, is_conv_64);
>  
>          perfc_incr(vpsci_system_suspend);
>          PSCI_SET_RESULT(regs, do_psci_1_0_system_suspend(epoint, cid));

... this hunk aiui, which is far from merely cosmetic imo. While
behavior looks to remain the same for PSCI_1_0_FN32_SYSTEM_SUSPEND, it
clearly changes for PSCI_1_0_FN64_SYSTEM_SUSPEND. That may be intended
and for the better, but the change clearly wasn't reviewed by Bertrand,
nor - when offering the R-b - did he ask for this extra change.

Jan