[v2] arm: Fix SMC reporting to EL2 when QEMU provides PSCI

Commit Message

Jan Kiszka Sept. 22, 2017, 7:02 a.m. UTC

From: Jan Kiszka <[e-mail]>

This properly forwards SMC events to EL2 when PSCI is provided by QEMU
itself and, thus, ARM_FEATURE_EL3 is off.

Found and tested with the Jailhouse hypervisor. Solution based on
suggestions by Peter Maydell.

Signed-off-by: Jan Kiszka <[e-mail]>
---

Changes in v2:
 - implemented Peter's suggestions - and it turned out that the
   arm_is_psci_call became indeed obsolete

 target/arm/helper.c    |  9 ++++++++-
 target/arm/op_helper.c | 27 +++++++++++++++++----------
 2 files changed, 25 insertions(+), 11 deletions(-)

Comments

Peter Maydell Sept. 25, 2017, 7:25 p.m. UTC | #1

On 22 September 2017 at 08:02, Jan Kiszka <[e-mail]> wrote:
> From: Jan Kiszka <[e-mail]>
>
> This properly forwards SMC events to EL2 when PSCI is provided by QEMU
> itself and, thus, ARM_FEATURE_EL3 is off.
>
> Found and tested with the Jailhouse hypervisor. Solution based on
> suggestions by Peter Maydell.
>
> Signed-off-by: Jan Kiszka <[e-mail]>
> ---

Applied to target-arm.next, thanks.

-- PMM

Patch

diff --git a/target/arm/helper.c b/target/arm/helper.c
index fa60040361..539eef0187 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -3717,7 +3717,14 @@  static void hcr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
 
     if (arm_feature(env, ARM_FEATURE_EL3)) {
         valid_mask &= ~HCR_HCD;
-    } else {
+    } else if (cpu->psci_conduit != QEMU_PSCI_CONDUIT_SMC) {
+        /* Architecturally HCR.TSC is RES0 if EL3 is not implemented.
+         * However, if we're using the SMC PSCI conduit then QEMU is
+         * effectively acting like EL3 firmware and so the guest at
+         * EL2 should retain the ability to prevent EL1 from being
+         * able to make SMC calls into the ersatz firmware, so in
+         * that case HCR.TSC should be read/write.
+         */
         valid_mask &= ~HCR_TSC;
     }
 
diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
index 2a85666579..26a0a5cf35 100644
--- a/target/arm/op_helper.c
+++ b/target/arm/op_helper.c
@@ -901,22 +901,29 @@  void HELPER(pre_smc)(CPUARMState *env, uint32_t syndrome)
      */
     bool undef = arm_feature(env, ARM_FEATURE_AARCH64) ? smd : smd && !secure;
 
-    if (arm_is_psci_call(cpu, EXCP_SMC)) {
-        /* If PSCI is enabled and this looks like a valid PSCI call then
-         * that overrides the architecturally mandated SMC behaviour.
+    if (!arm_feature(env, ARM_FEATURE_EL3) &&
+        cpu->psci_conduit != QEMU_PSCI_CONDUIT_SMC) {
+        /* If we have no EL3 then SMC always UNDEFs and can't be
+         * trapped to EL2. PSCI-via-SMC is a sort of ersatz EL3
+         * firmware within QEMU, and we want an EL2 guest to be able
+         * to forbid its EL1 from making PSCI calls into QEMU's
+         * "firmware" via HCR.TSC, so for these purposes treat
+         * PSCI-via-SMC as implying an EL3.
          */
-        return;
-    }
-
-    if (!arm_feature(env, ARM_FEATURE_EL3)) {
-        /* If we have no EL3 then SMC always UNDEFs */
         undef = true;
     } else if (!secure && cur_el == 1 && (env->cp15.hcr_el2 & HCR_TSC)) {
-        /* In NS EL1, HCR controlled routing to EL2 has priority over SMD. */
+        /* In NS EL1, HCR controlled routing to EL2 has priority over SMD.
+         * We also want an EL2 guest to be able to forbid its EL1 from
+         * making PSCI calls into QEMU's "firmware" via HCR.TSC.
+         */
         raise_exception(env, EXCP_HYP_TRAP, syndrome, 2);
     }
 
-    if (undef) {
+    /* If PSCI is enabled and this looks like a valid PSCI call then
+     * suppress the UNDEF -- we'll catch the SMC exception and
+     * implement the PSCI call behaviour there.
+     */
+    if (undef && !arm_is_psci_call(cpu, EXCP_SMC)) {
         raise_exception(env, EXCP_UDEF, syn_uncategorized(),
                         exception_target_el(env));
     }