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

[Peter Maydell]

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:
c8b89685-7490-328b-51a3-48711c140a84@tribudubois.net/">https://lore.kernel.org/qemu-devel/c8b89685-7490-328b-51a3-48711c140a84@tribudubois.net/

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.

thanks
-- PMM