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

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

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.

Regards
Bertrand

> 
> Cheers,
> 
> -- 
> Julien Grall