diff mbox series

[10/16] Revert "Revert "arm: tcg: Adhere to SMCCC 1.3 section 5.2""

Message ID 20220127154639.2090164-11-peter.maydell@linaro.org
State New
Headers show
Series arm: Fix handling of unrecognized functions in PSCI emulation | expand

Commit Message

Peter Maydell Jan. 27, 2022, 3:46 p.m. UTC
Now that we have arranged for all the affected board models
to not enable the PSCI emulation if they are running guest code
at EL3, we can revert commit 4825eaae4fdd56fba0f, thus
reinstating commit 9fcd15b9193e819b, without bringing back the
regressions that caused us to revert it.

For clarity, here is the original commit message of 9fcd15b9193e819b:

The SMCCC 1.3 spec section 5.2 says

  The Unknown SMC Function Identifier is a sign-extended value of (-1)
  that is returned in the R0, W0 or X0 registers. An implementation must
  return this error code when it receives:

    * An SMC or HVC call with an unknown Function Identifier
    * An SMC or HVC call for a removed Function Identifier
    * An SMC64/HVC64 call from AArch32 state

To comply with these statements, let's always return -1 when we encounter
an unknown HVC or SMC call.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/psci.c | 35 ++++++-----------------------------
 1 file changed, 6 insertions(+), 29 deletions(-)

Comments

Richard Henderson Jan. 31, 2022, 6:57 a.m. UTC | #1
On 1/28/22 02:46, Peter Maydell wrote:
> Now that we have arranged for all the affected board models
> to not enable the PSCI emulation if they are running guest code
> at EL3, we can revert commit 4825eaae4fdd56fba0f, thus
> reinstating commit 9fcd15b9193e819b, without bringing back the
> regressions that caused us to revert it.
> 
> For clarity, here is the original commit message of 9fcd15b9193e819b:
> 
> The SMCCC 1.3 spec section 5.2 says
> 
>    The Unknown SMC Function Identifier is a sign-extended value of (-1)
>    that is returned in the R0, W0 or X0 registers. An implementation must
>    return this error code when it receives:
> 
>      * An SMC or HVC call with an unknown Function Identifier
>      * An SMC or HVC call for a removed Function Identifier
>      * An SMC64/HVC64 call from AArch32 state
> 
> To comply with these statements, let's always return -1 when we encounter
> an unknown HVC or SMC call.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   target/arm/psci.c | 35 ++++++-----------------------------
>   1 file changed, 6 insertions(+), 29 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~
Alexander Graf Feb. 7, 2022, 2:29 p.m. UTC | #2
On 27.01.22 16:46, Peter Maydell wrote:
> Now that we have arranged for all the affected board models
> to not enable the PSCI emulation if they are running guest code
> at EL3, we can revert commit 4825eaae4fdd56fba0f, thus
> reinstating commit 9fcd15b9193e819b, without bringing back the
> regressions that caused us to revert it.
>
> For clarity, here is the original commit message of 9fcd15b9193e819b:
>
> The SMCCC 1.3 spec section 5.2 says
>
>    The Unknown SMC Function Identifier is a sign-extended value of (-1)
>    that is returned in the R0, W0 or X0 registers. An implementation must
>    return this error code when it receives:
>
>      * An SMC or HVC call with an unknown Function Identifier
>      * An SMC or HVC call for a removed Function Identifier
>      * An SMC64/HVC64 call from AArch32 state
>
> To comply with these statements, let's always return -1 when we encounter
> an unknown HVC or SMC call.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>


Wouldn't it make more sense to apply the original patch again so that 
git blame points to the correct offender? ;)


Alex
diff mbox series

Patch

diff --git a/target/arm/psci.c b/target/arm/psci.c
index 6709e280133..b279c0b9a45 100644
--- a/target/arm/psci.c
+++ b/target/arm/psci.c
@@ -27,15 +27,13 @@ 
 
 bool arm_is_psci_call(ARMCPU *cpu, int excp_type)
 {
-    /* Return true if the r0/x0 value indicates a PSCI call and
-     * the exception type matches the configured PSCI conduit. This is
-     * called before the SMC/HVC instruction is executed, to decide whether
-     * we should treat it as a PSCI call or with the architecturally
+    /*
+     * Return true if the exception type matches the configured PSCI conduit.
+     * This is called before the SMC/HVC instruction is executed, to decide
+     * whether we should treat it as a PSCI call or with the architecturally
      * defined behaviour for an SMC or HVC (which might be UNDEF or trap
      * to EL2 or to EL3).
      */
-    CPUARMState *env = &cpu->env;
-    uint64_t param = is_a64(env) ? env->xregs[0] : env->regs[0];
 
     switch (excp_type) {
     case EXCP_HVC:
@@ -52,27 +50,7 @@  bool arm_is_psci_call(ARMCPU *cpu, int excp_type)
         return false;
     }
 
-    switch (param) {
-    case QEMU_PSCI_0_2_FN_PSCI_VERSION:
-    case QEMU_PSCI_0_2_FN_MIGRATE_INFO_TYPE:
-    case QEMU_PSCI_0_2_FN_AFFINITY_INFO:
-    case QEMU_PSCI_0_2_FN64_AFFINITY_INFO:
-    case QEMU_PSCI_0_2_FN_SYSTEM_RESET:
-    case QEMU_PSCI_0_2_FN_SYSTEM_OFF:
-    case QEMU_PSCI_0_1_FN_CPU_ON:
-    case QEMU_PSCI_0_2_FN_CPU_ON:
-    case QEMU_PSCI_0_2_FN64_CPU_ON:
-    case QEMU_PSCI_0_1_FN_CPU_OFF:
-    case QEMU_PSCI_0_2_FN_CPU_OFF:
-    case QEMU_PSCI_0_1_FN_CPU_SUSPEND:
-    case QEMU_PSCI_0_2_FN_CPU_SUSPEND:
-    case QEMU_PSCI_0_2_FN64_CPU_SUSPEND:
-    case QEMU_PSCI_0_1_FN_MIGRATE:
-    case QEMU_PSCI_0_2_FN_MIGRATE:
-        return true;
-    default:
-        return false;
-    }
+    return true;
 }
 
 void arm_handle_psci_call(ARMCPU *cpu)
@@ -194,10 +172,9 @@  void arm_handle_psci_call(ARMCPU *cpu)
         break;
     case QEMU_PSCI_0_1_FN_MIGRATE:
     case QEMU_PSCI_0_2_FN_MIGRATE:
+    default:
         ret = QEMU_PSCI_RET_NOT_SUPPORTED;
         break;
-    default:
-        g_assert_not_reached();
     }
 
 err: