Re: [PATCH] target/arm: Treat unknown SMC calls as NOP

["Alex Bennée" <alex.bennee@linaro.org>]

Alexander Graf <agraf@csgraf.de> writes:

> On 01.07.20 22:47, Peter Maydell wrote:
>> On Wed, 1 Jul 2020 at 21:08, Alexander Graf <agraf@csgraf.de> wrote:
>>> We currently treat unknown SMC calls as UNDEF. This behavior is different
>>> from KVM, which treats them as NOP.
>>>
>>> Unfortunately, the UNDEF exception breaks running Windows for ARM in QEMU,
>>> as that probes an OEM SMCCC call on boot, but does not expect to receive
>>> an UNDEF exception as response.
>>>
>>> So instead, let's follow the KVM path and ignore SMC calls that we don't
>>> handle. This fixes booting the Windows 10 for ARM preview in TCG for me.
>>>
>>> Signed-off-by: Alexander Graf <agraf@csgraf.de>
>>> +    if (cs->exception_index == EXCP_SMC &&
>>> +        !arm_feature(env, ARM_FEATURE_EL3) &&
>>> +        cpu->psci_conduit != QEMU_PSCI_CONDUIT_SMC) {
>> This condition says: "we got an SMC, and this CPU doesn't
>> have EL3, and we're not imitating real EL3 firmware".
>
>
> I like to think of it as "This CPU exposes an environment that looks
> like KVM, so it implements HVC calls (EL2) and is responsible for
> handling SMC calls as well.

That is a very KVM centric view of the world ;-)

I thought the aim was always to behave as the real processor would.

> The main difference between the two semantics is that in yours, you
> don't have EL3. In mine, there is an EL3, but it's virtualized by the
> same layer that implements EL2.

If you boot up with secure firmware + EL2 aware KVM kernel I assume
everything behaves as expected?

>
>
>> The architecturally correct behaviour here (since we don't
>> implement nested-virt yet, which might allow it to trap
>> to guest EL2) is to UNDEF, as far as I can see from a quick
>> look at the AArch64.CheckForSMCUndefOrTrap().
>>
>> I'm not sure why KVM makes these NOP; if I'm right about the
>> architectural behaviour then making them NOP would be a KVM bug.
>>
>> If Windows makes an SMC call on boot that seems like a guest
>> bug: it would crash on a real CPU without EL2/EL3 as well.
>
>
> I don't think there can be a real SBBR compatible CPU without EL2/EL3,
> because PSCI is a base requirement. That means either SMC calls succeed
> (Windows running in EL2) or SMC calls are trapped into EL2 and it's up
> to the hypervisor to decide what to do with them.
>
>
>>
>>       *  Conduit SMC, valid call  Trap to EL2         PSCI Call
>>       *  Conduit SMC, inval call  Trap to EL2         Undef insn
>> -     *  Conduit not SMC          Undef insn          Undef insn
>> +     *  Conduit not SMC          nop                 nop
>>
>> The line in this table that your commit message says you're
>> fixing is "Conduit SMC, inval call"; the line your code
>> change affects is "Conduit not SMC", which is not the same
>> thing. (I'd have to look at the PSCI spec to see what it
>> requires for SMCs that aren't valid PSCI calls.)
>
>
> The patch fixes the default environment, which is "Conduit HVC, PSCI
> over HVC implemented by QEMU". If the patch description wasn't clear,
> I'm happy to reword it :).
>
>
> Alex


-- 
Alex Bennée