[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PULL 02/22] arm: tcg: Adhere to SMCCC 1.3 section 5.2

From: Alexander Graf
Subject: Re: [PULL 02/22] arm: tcg: Adhere to SMCCC 1.3 section 5.2
Date: Fri, 19 Nov 2021 11:35:42 +0100
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.3.1

On 18.11.21 22:57, Peter Maydell wrote:
On Thu, 30 Sept 2021 at 16:12, Peter Maydell <peter.maydell@linaro.org> wrote:
From: Alexander Graf <agraf@csgraf.de>

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.
TL/DR: I propose to revert this for 6.2.

This change turns out to cause regressions, for instance on the
imx6ul boards as described here:

The primary cause of that regression is that the guest code running
at EL3 expects SMCs (not related to PSCI) to do what they would if
our PSCI emulation was not present at all, but after this change
they instead set a value in R0/X0 and continue.

I had a look at fixing this, which involves deferring the "do we
want to enable PSCI emulation?" decision into the hw/arm/boot.c
code (which is the only place we finally figure out whether we're
going to be booting the guest into EL3 or not). I have some
more-or-less working prototype code, but in the course of writing
it I discovered a much harder to fix issue:

The highbank board both:
  (1) wants to enable PSCI emulation
  (2) has a bit of guest code that it wants to run at EL3 and
      to perform SMC calls that trap to the monitor vector table:
      this is the boot stub code that is written to memory by
      arm_write_secure_board_setup_dummy_smc() and which the
      highbank board enables by setting bootinfo->secure_board_setup

We can't satisfy both of those and also have the PSCI emulation
handle all SMC instruction executions regardless of function
identifier value.

There is probably a solution to this, but I'm not sure what it
is right now (it might involve having QEMU manually do the things
that we currently have the arm_write_secure_board_setup_dummy_smc
write guest code to do) and it's going to require digging through
what the highbank board actually is supposed to do here. Given
that we're already in the release cycle for 6.2, I think the
safest and simplest approach is to revert this patch for now,
which just takes us back to the behaviour we've always had
in previous releases. We can then take our time to figure out
how to clean up this mess in 7.0.

Ugh :(. Conceptually, once you tell QEMU to handle PSCI, you're basically giving up that EL to it. It sounds almost as if what these boards (imx6ul + highbank) want is an EL4 they can call into to deflect PSCI calls into from EL3 they own. We would basically have to allocate a currently undefinied/reserved instruction as "QEMU SMC" and make the EL3 code call that when it needs to call QEMU for PSCI operations. Or a PV MMIO device. Or a PV sysreg. But at the end of the day, EL3 calling into QEMU differently than on real hardware is paravirtualization.

I agree with the conclusion that we revert it for QEMU 6.2 though. The 2 guest OSs I'm aware of that rely on the behavior in the patch / spec (Windows and VMware ESXi) require more QEMU modifications to be fully functional: SMC as default conduit for Windows and EL3 PSR exposure for ESXi. So neither of them would work out of the box with 6.2 as is.

Just to double check: Is the broken monitor code that expects QEMU to partially handle SMCs only ever injected into the guest by us or is there existing guest payload code for EL3 that makes the same assumption?


reply via email to

[Prev in Thread] Current Thread [Next in Thread]