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

[Mykola Kvach <xakep.amatop@xxxxxxxxx>]

Hi Bertrand and Julien,

Thanks for the suggestions.

On Sat, Mar 21, 2026 at 2:44 PM Bertrand Marquis
<Bertrand.Marquis@xxxxxxx> wrote:
>
> Hi Julien
>
> > On 21 Mar 2026, at 11:34, Julien Grall <julien@xxxxxxx> wrote:
> >
> > Hi Bertrand,
> >
> > On 19/03/2026 07:47, Bertrand Marquis wrote:
> >> 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.
> >
> > I would definitely agree with that. But...
> >
> >> 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
> >
> > I am not sure about this suggestion. Not all 32-bit call have a matching 
> > 64-bit call (e.g. PSCI_VERSION).
> >
> > Also, it seems that so far the function ID is always matching between the 
> > two convention, it is unclear whether this is guaranteed.
>
> PSCI is an SMCCC spec and smccc defines the fid format with fast call or not, 
> 32 or 64 and function id and that is the same for 32
> and 64 but there is no enforcement to provide both 32 and 64 versions (for 
> example there are some ffa calls which are only available
> in 32 bit mode so 64bit version would get an UNSUPPORTED back).
>
> I am not suggesting we accept both versions when only one is supported here, 
> we can still have the switch base on function id and
> for specific one reject if it is not 32 bit format.
>
> Anyway as said this was more a suggestion than a request so maybe better to 
> skip that for now.

I have taken the first part and introduced a helper to centralize
the convention-dependent argument decoding, so the 32-bit argument
handling is no longer open-coded at each call site.

For the second part, I have not switched this patch to dispatch on
smccc_get_fn(fid). While that could reduce some duplication in the
switch, it would also broaden the change beyond the original fix.

In particular, a function-number-based dispatch would require
separating bare PSCI function-number definitions from the full SMCCC
FIDs, and it would also need explicit checks in the handlers for calls
that are only defined in the 32-bit convention, so that unsupported
64-bit forms are still rejected appropriately.

So for now I would prefer to keep the switch on the full FID and keep
this patch focused on fixing the convention-dependent argument decoding
for the affected PSCI calls.

A follow-up cleanup can revisit switching to smccc_get_fn(fid) if we
decide that the extra refactoring is worthwhile.

Regards,
Mykola

>
> Regards
> Bertrand
>
> >
> > Cheers,
> >
> > --
> > Julien Grall
>
>