Re: [PATCH v6 07/11] hvf: Add Apple Silicon support

From: Alexander Graf <agraf@csgraf.de>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: Eduardo Habkost <ehabkost@redhat.com>,
	Richard Henderson <richard.henderson@linaro.org>,
	QEMU Developers <qemu-devel@nongnu.org>,
	Cameron Esfahani <dirty@apple.com>,
	Roman Bolshakov <r.bolshakov@yadro.com>,
	qemu-arm <qemu-arm@nongnu.org>, Frank Yang <lfy@google.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Peter Collingbourne <pcc@google.com>
Subject: Re: [PATCH v6 07/11] hvf: Add Apple Silicon support
Date: Thu, 11 Feb 2021 14:06:23 +0100	[thread overview]
Message-ID: <37018444-82a8-96c0-b5ce-da056646a1b8@csgraf.de> (raw)
In-Reply-To: <CAFEAcA_ze+J7geayqgaV274anQubqiv56qan7wo8EkxZ14Nydw@mail.gmail.com>

On 10.02.21 23:39, Peter Maydell wrote:
> On Wed, 10 Feb 2021 at 22:21, Alexander Graf <agraf@csgraf.de> wrote:
>>
>> On 28.01.21 16:52, Peter Maydell wrote:
>>> On Wed, 20 Jan 2021 at 22:44, Alexander Graf <agraf@csgraf.de> wrote:
>>>> +            break;
>>>> +        case EC_AA64_SMC:
>>>> +            cpu_synchronize_state(cpu);
>>>> +            if (arm_is_psci_call(arm_cpu, EXCP_SMC)) {
>>>> +                arm_handle_psci_call(arm_cpu);
>>> Have you checked that all the PSCI code really can cope
>>> with being called from a non-TCG accelerator? (As an example
>>> the CPU_SUSPEND implementation calls the TCG wfi helper...)
>>
>> I have not explicitly tried it, but I don't see why the TCG
>> implementation of wfi should in principle break with hvf.
> Because the TCG implementation of wfi is "set some state fields
> and then longjump out to the TCG exec_cpu code-execution loop",
> and hvf doesn't use that loop.

I can confirm that it breaks, but are you really sure about the longjmp 
not working?

What would you prefer instead? Duplicate the PSCI implementation for HVF?

>
>>>> +            } else {
>>>> +                DPRINTF("unknown SMC! %016llx", env->xregs[0]);
>>>> +                env->xregs[0] = -1;
>>> This should inject an UNDEF exception into the guest. (Compare
>>> the pre_smc helper in target/arm/op_helper.c for TCG.)
>>
>> That would break Windows, which is one of the main use cases for hvf
>> support in QEMU.
> Why is Windows making bogus SMC calls ?

Let me have a quick at my crystal ball ... mmmmmmhhhh ... it's a bit 
blurry unfortunately.

I really don't think I'm the right person to answer that question :). 
But the Windows loader does invoke weird SMC calls on boot:

BdsDxe: starting Boot0002 "UEFI QEMU NVMe Ctrl 1234 1" from 
PciRoot(0x0)/Pci(0x1,0x0)/NVMe(0x1,00-00-00-00-00-00-00-00)
ConvertPages: failed to find range 102000 - 102FFF
ConvertPages: range BCCE4000 - BCD05FFF covers multiple entries
ConvertPages: range BCCE0000 - BCCEAFFF covers multiple entries
hvf_unknown_smc unknown SMC! 00000000c3000001
SetUefiImageMemoryAttributes - 0x00000000BF500000 - 0x0000000000040000 
(0x0000000000000008)
SetUefiImageMemoryAttributes - 0x00000000BC190000 - 0x0000000000040000 
(0x0000000000000008)
SetUefiImageMemoryAttributes - 0x00000000BC140000 - 0x0000000000040000 
(0x0000000000000008)
SetUefiImageMemoryAttributes - 0x00000000BF4C0000 - 0x0000000000030000 
(0x0000000000000008)
SetUefiImageMemoryAttributes - 0x00000000BC0F0000 - 0x0000000000040000 
(0x0000000000000008)
SetUefiImageMemoryAttributes - 0x00000000BBCB0000 - 0x0000000000040000 
(0x0000000000000008)
SetUefiImageMemoryAttributes - 0x00000000BBB00000 - 0x0000000000030000 
(0x0000000000000008)
SetUefiImageMemoryAttributes - 0x00000000BBAC0000 - 0x0000000000030000 
(0x0000000000000008)
hvf_unknown_smc unknown SMC! 0000000084000000
hvf_unknown_smc unknown SMC! 000000008400000a
hvf_unknown_smc unknown SMC! 000000008400000a
hvf_unknown_smc unknown SMC! 000000008400000a
hvf_unknown_smc unknown SMC! 000000008400000a
hvf_unknown_smc unknown SMC! 000000008400000a
hvf_unknown_smc unknown SMC! 000000008400000a
hvf_unknown_smc unknown SMC! 000000008400000a
hvf_unknown_smc unknown SMC! 000000008400000a
hvf_unknown_smc unknown SMC! 000000008400000a
hvf_unknown_smc unknown SMC! 000000008400000a

Alex