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

[Mykola Kvach <xakep.amatop@xxxxxxxxx>]

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>
---
v3:
 - use PSCI_ARG_CONV for SYSTEM_SUSPEND

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@xxxxxxxx/072270e0940b6bcc2743d56a336363f4719ba60a.1751020456.git.mykola._5Fkvach@xxxxxxxx/#7070f416-119c-49f8-acd0-82c6e31f0fc6@xxxxxxx
---
 xen/arch/arm/vpsci.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c
index bd87ec430d..ac6af6118f 100644
--- a/xen/arch/arm/vpsci.c
+++ b/xen/arch/arm/vpsci.c
@@ -305,13 +305,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:
     {
@@ -346,6 +349,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:
@@ -378,9 +383,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));
@@ -391,8 +396,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));
@@ -402,7 +407,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);
@@ -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));
-- 
2.43.0