[PATCH 00/16] arm: Fix handling of unrecognized functions in PSCI emulation

[Peter Maydell <peter.maydell@linaro.org>]

This series fixes our handling of PSCI calls where the function ID is
not recognized. These are supposed to return an error value, but
currently we instead emulate the SMC or HVC instruction to trap to the
guest at EL3 or EL2. Particularly of note for code review:
 * patches 4-9 include some "is this the right behaviour for
   this hardware" questions for the maintainers of those boards
 * patch 15 has a DTB API question, as well as being a change in
   what we edit in a DTB we are passed by the user
 * testing of the affected machines would be welcome

We tried to fix that bug in commit 9fcd15b9193e819b, but ran into
regressions and so reverted it in commit 4825eaae4fdd56fba0f for
release 7.0.  This series fixes the underlying problems causing the
regressions and reverts the revert.  It also fixes some other bugs
that were preventing booting of guests on the midway board and that
meant that the Linux kernel I tested couldn't bring up the secondary
CPUs when running with more than one guest CPU.

The regressions happened on boards which enabled PSCI emulation while
still running guest code at EL3. This used to work (as long as the
guest code itself wasn't trying to implement PSCI!)  because of the
fall-through-to-emulate-the-insn behaviour, but once the PSCI
emulation handles all SMC calls, most EL3 guest code will stop working
correctly. The solution this patchset adopts is to avoid enabling
QEMU's PSCI emulation when running guest code at EL3.

The affected boards are:
 * orangepi-pc, mcimx6ul-evk, mcimx7d-sabre, highbank, midway,
   xlnx-zcu102 (for any EL3 guest code)
 * xlnx-versal-virt (only for EL3 code run via -kernel)
 * virt (only for EL3 code run via -kernel or generic-loader)
For all these cases we will no longer enable PSCI emulation.
(This might in theory break guest code that used to work because
it was relying on running under QEMU and having the PSCI emulation
despite being at EL3 itself, but hopefully such code is rare.)

In order to only enable PSCI emulation when the guest is running at an
exception level lower than the level that our PSCI emulation
"firmware" would be running at, we make the arm_load_kernel() code in
boot.c responsible for setting the CPU properties psci-conduit and
start-powered-off. This is because only that code knows what EL it is
going to start the guest at (which depends on things like whether it
has decided that the guest is a Linux kernel or not).

The complicated case in all of this is the highbank and midway board
models, because of all the boards which enable QEMU's PSCI emulation
only these also use the boot.c secure_board_setup flag to say "run a
fragment of QEMU-provided boot code in the guest at EL3 to set
something up before running the guest at EL2 or EL1". That fragment of
code includes use of the SMC instruction, so when PSCI emulation
starts making that a NOP rather than a trap-to-guest-EL3 the setup
code will change behaviour. Fortunately, for this specific board's use
case the NOP is fine. The purpose of the setup code is to arrange that
unknown SMCs act as NOPs, because guest code may use a
highbank/midway-specific SMC ABI to enable the L2x0 cache
controller. So when the PSCI emulation starts to NOP the unknown SMCs
the setup code won't actively break, and the guest behaviour will
still be OK. (See patch 11's commit message for fuller details.)

Patches 1 and 2 make the relevant CPU properties settable after the
CPU object has been realized. This is necessary because
arm_load_kernel() runs very late, after the whole machine (including
the CPU objects) has been fully initialized.  (It was the restriction
on setting these properties before realize that previously led us to
set them in the SoC emulation code and thus to do it unconditionally.)

Patch 3 provides the "set up psci conduit" functionality in the boot.c
code; this is opt-in per board by setting a field in the arm_boot_info
struct.

Patches 4 to 9 move the individual boards across to using the new
approach. In each case I had to make a decision about the behaviour of
secondary CPUs when running guest firmware at EL3 -- should the
secondaries start off powered-down (waiting for the guest to power
them up via some kind of emulated power-control device), or powered-up
(so they all start running the firmware at once)? In a few cases I was
able to find the answer to this; in the rest I have erred on the side
of retaining the current "start powered down" behaviour, and added a
TODO comment to that effect. If you know the actual hardware
behaviour, let me know.

Patch 10 is the revert-the-revert patch.

Patch 11 removes the highbank/midway board use of the secure_board_setup
functionality; the commit message has the details about why this is safe.

Patches 12 to 14 are more minor cleanups that allow, and follow on from,
dropping the highbank-specific secondary CPU boot stub code.

Patch 15 is a change that is somewhat unrelated, but is necessary to
get the highbank board to successfully boot in SMP and to get the
midway board to start a Linux guest at all.

I have tested this with make check/check-acceptance and also with some
test images I have locally (including highbank and midway), but I
don't have test images for most of these boards, and in particular I
don't really have anything that exercises "run guest EL3 code" for
most of them. Testing would be welcome.

thanks
-- PMM

Peter Maydell (16):
  target/arm: make psci-conduit settable after realize
  cpu.c: Make start-powered-off settable after realize
  hw/arm/boot: Support setting psci-conduit based on guest EL
  hw/arm: imx: Don't enable PSCI conduit when booting guest in EL3
  hw/arm: allwinner: Don't enable PSCI conduit when booting guest in EL3
  hw/arm/xlnx-zcu102: Don't enable PSCI conduit when booting guest in
    EL3
  hw/arm/versal: Let boot.c handle PSCI enablement
  hw/arm/virt: Let boot.c handle PSCI enablement
  hw/arm: highbank: For EL3 guests, don't enable PSCI, start all cores
  Revert "Revert "arm: tcg: Adhere to SMCCC 1.3 section 5.2""
  hw/arm/highbank: Drop use of secure_board_setup
  hw/arm/boot: Prevent setting both psci_conduit and secure_board_setup
  hw/arm/boot: Don't write secondary boot stub if using PSCI
  hw/arm/highbank: Drop unused secondary boot stub code
  hw/arm/boot: Drop nb_cpus field from arm_boot_info
  hw/arm/boot: Drop existing dtb /psci node rather than retaining it

 include/hw/arm/boot.h        |  14 ++++-
 include/hw/arm/xlnx-versal.h |   1 -
 cpu.c                        |  22 ++++++-
 hw/arm/allwinner-h3.c        |   9 ++-
 hw/arm/aspeed.c              |   1 -
 hw/arm/boot.c                | 107 +++++++++++++++++++++++++++++------
 hw/arm/exynos4_boards.c      |   1 -
 hw/arm/fsl-imx6ul.c          |   2 -
 hw/arm/fsl-imx7.c            |   8 +--
 hw/arm/highbank.c            |  72 +----------------------
 hw/arm/imx25_pdk.c           |   3 +-
 hw/arm/kzm.c                 |   1 -
 hw/arm/mcimx6ul-evk.c        |   2 +-
 hw/arm/mcimx7d-sabre.c       |   2 +-
 hw/arm/npcm7xx.c             |   3 -
 hw/arm/orangepi.c            |   5 +-
 hw/arm/raspi.c               |   1 -
 hw/arm/realview.c            |   1 -
 hw/arm/sabrelite.c           |   1 -
 hw/arm/sbsa-ref.c            |   1 -
 hw/arm/vexpress.c            |   1 -
 hw/arm/virt.c                |  13 +----
 hw/arm/xilinx_zynq.c         |   1 -
 hw/arm/xlnx-versal-virt.c    |   6 +-
 hw/arm/xlnx-versal.c         |   5 +-
 hw/arm/xlnx-zcu102.c         |   1 +
 hw/arm/xlnx-zynqmp.c         |  13 +++--
 target/arm/cpu.c             |   6 +-
 target/arm/psci.c            |  35 ++----------
 29 files changed, 164 insertions(+), 174 deletions(-)

-- 
2.25.1