1. Patchew
  2. Xen
  3. View series

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

Mykola Kvach posted 1 patch 7 hours ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/cde430238469198082114121b14d88bce9f1ceef.1774263939.git.mykola._5Fkvach@epam.com
xen/arch/arm/vpsci.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
[PATCH v2] xen/arm: vpsci: ignore upper 32 bits for SMC32 PSCI arguments
Posted by Mykola Kvach 7 hours ago 
From: Mykola Kvach <mykola_kvach@epam.com>

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.

No functional change is intended for PSCI 0.1.

Suggested-by: Julien Grall <julien@xen.org>
Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
---
v2:
 - introduce PSCI_ARG_CONV() to centralize convention-dependent argument
   decoding for PSCI v0.2+ calls;
 - use smccc_is_conv_64(fid) instead of open-coding per-call SMC32 checks;
 - keep PSCI 0.1 handling unchanged, except switch on the already-decoded
   fid instead of re-reading x0/r0.

Link to discussion: https://patchew.org/Xen/cover.1751020456.git.mykola._5Fkvach@epam.com/072270e0940b6bcc2743d56a336363f4719ba60a.1751020456.git.mykola._5Fkvach@epam.com/#7070f416-119c-49f8-acd0-82c6e31f0fc6@xen.org
---
 xen/arch/arm/vpsci.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c
index 7ba9ccd94b..65dea5cf6c 100644
--- a/xen/arch/arm/vpsci.c
+++ b/xen/arch/arm/vpsci.c
@@ -230,13 +230,16 @@ static int32_t do_psci_1_0_features(uint32_t psci_func_id)
 #define PSCI_ARG32(reg, n) PSCI_ARG(reg, n)
 #endif
 
+#define PSCI_ARG_CONV(reg, n, conv_64) \
+    ((conv_64) ? PSCI_ARG(reg, n) : PSCI_ARG32(reg, n))
+
 /*
  * PSCI 0.1 calls. It will return false if the function ID is not
  * handled.
  */
 bool do_vpsci_0_1_call(struct cpu_user_regs *regs, uint32_t fid)
 {
-    switch ( (uint32_t)get_user_reg(regs, 0) )
+    switch ( fid )
     {
     case PSCI_cpu_off:
     {
@@ -271,6 +274,8 @@ bool do_vpsci_0_2_call(struct cpu_user_regs *regs, uint32_t fid)
      * adding/removing a function. SSSC_SMCCC_*_REVISION should be
      * updated once per release.
      */
+    bool is_conv_64 = smccc_is_conv_64(fid);
+
     switch ( fid )
     {
     case PSCI_0_2_FN32_PSCI_VERSION:
@@ -303,9 +308,9 @@ 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);
+        register_t vcpuid = PSCI_ARG_CONV(regs, 1, is_conv_64);
+        register_t epoint = PSCI_ARG_CONV(regs, 2, is_conv_64);
+        register_t cid = PSCI_ARG_CONV(regs, 3, is_conv_64);
 
         perfc_incr(vpsci_cpu_on);
         PSCI_SET_RESULT(regs, do_psci_0_2_cpu_on(vcpuid, epoint, cid));
@@ -316,8 +321,8 @@ bool do_vpsci_0_2_call(struct cpu_user_regs *regs, uint32_t fid)
     case PSCI_0_2_FN64_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 = PSCI_ARG_CONV(regs, 2, is_conv_64);
+        register_t cid = PSCI_ARG_CONV(regs, 3, is_conv_64);
 
         perfc_incr(vpsci_cpu_suspend);
         PSCI_SET_RESULT(regs, do_psci_0_2_cpu_suspend(pstate, epoint, cid));
@@ -327,7 +332,7 @@ 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);
+        register_t taff = PSCI_ARG_CONV(regs, 1, is_conv_64);
         uint32_t laff = PSCI_ARG32(regs, 2);
 
         perfc_incr(vpsci_cpu_affinity_info);
-- 
2.43.0
Re: [PATCH v2] xen/arm: vpsci: ignore upper 32 bits for SMC32 PSCI arguments
Posted by Bertrand Marquis 2 hours ago 
Hi Mykola,

> On 23 Mar 2026, at 12:11, Mykola Kvach <xakep.amatop@gmail.com> wrote:
> 
> From: Mykola Kvach <mykola_kvach@epam.com>
> 
> 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.
> 
> No functional change is intended for PSCI 0.1.
> 
> Suggested-by: Julien Grall <julien@xen.org>
> Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>

Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

PS: this could be treated as a bug fix but as no implementation we know of has
been falling into this error, i do not think it would be needed to backport this.

Cheers
Bertrand

> ---
> v2:
> - introduce PSCI_ARG_CONV() to centralize convention-dependent argument
>   decoding for PSCI v0.2+ calls;
> - use smccc_is_conv_64(fid) instead of open-coding per-call SMC32 checks;
> - keep PSCI 0.1 handling unchanged, except switch on the already-decoded
>   fid instead of re-reading x0/r0.
> 
> Link to discussion: https://patchew.org/Xen/cover.1751020456.git.mykola._5Fkvach@epam.com/072270e0940b6bcc2743d56a336363f4719ba60a.1751020456.git.mykola._5Fkvach@epam.com/#7070f416-119c-49f8-acd0-82c6e31f0fc6@xen.org
> ---
> xen/arch/arm/vpsci.c | 19 ++++++++++++-------
> 1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c
> index 7ba9ccd94b..65dea5cf6c 100644
> --- a/xen/arch/arm/vpsci.c
> +++ b/xen/arch/arm/vpsci.c
> @@ -230,13 +230,16 @@ static int32_t do_psci_1_0_features(uint32_t psci_func_id)
> #define PSCI_ARG32(reg, n) PSCI_ARG(reg, n)
> #endif
> 
> +#define PSCI_ARG_CONV(reg, n, conv_64) \
> +    ((conv_64) ? PSCI_ARG(reg, n) : PSCI_ARG32(reg, n))
> +
> /*
>  * PSCI 0.1 calls. It will return false if the function ID is not
>  * handled.
>  */
> bool do_vpsci_0_1_call(struct cpu_user_regs *regs, uint32_t fid)
> {
> -    switch ( (uint32_t)get_user_reg(regs, 0) )
> +    switch ( fid )
>     {
>     case PSCI_cpu_off:
>     {
> @@ -271,6 +274,8 @@ bool do_vpsci_0_2_call(struct cpu_user_regs *regs, uint32_t fid)
>      * adding/removing a function. SSSC_SMCCC_*_REVISION should be
>      * updated once per release.
>      */
> +    bool is_conv_64 = smccc_is_conv_64(fid);
> +
>     switch ( fid )
>     {
>     case PSCI_0_2_FN32_PSCI_VERSION:
> @@ -303,9 +308,9 @@ 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);
> +        register_t vcpuid = PSCI_ARG_CONV(regs, 1, is_conv_64);
> +        register_t epoint = PSCI_ARG_CONV(regs, 2, is_conv_64);
> +        register_t cid = PSCI_ARG_CONV(regs, 3, is_conv_64);
> 
>         perfc_incr(vpsci_cpu_on);
>         PSCI_SET_RESULT(regs, do_psci_0_2_cpu_on(vcpuid, epoint, cid));
> @@ -316,8 +321,8 @@ bool do_vpsci_0_2_call(struct cpu_user_regs *regs, uint32_t fid)
>     case PSCI_0_2_FN64_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 = PSCI_ARG_CONV(regs, 2, is_conv_64);
> +        register_t cid = PSCI_ARG_CONV(regs, 3, is_conv_64);
> 
>         perfc_incr(vpsci_cpu_suspend);
>         PSCI_SET_RESULT(regs, do_psci_0_2_cpu_suspend(pstate, epoint, cid));
> @@ -327,7 +332,7 @@ 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);
> +        register_t taff = PSCI_ARG_CONV(regs, 1, is_conv_64);
>         uint32_t laff = PSCI_ARG32(regs, 2);
> 
>         perfc_incr(vpsci_cpu_affinity_info);
> -- 
> 2.43.0
>

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.