Re: [PATCH v12 00/10] hvf: Implement Apple Silicon Support

["Philippe Mathieu-Daudé" <f4bug@amsat.org>]

On 9/21/21 15:30, Alexander Graf wrote:
> On 21.09.21 11:29, Philippe Mathieu-Daudé wrote:
>> On 9/20/21 22:21, Alexander Graf wrote:
>>> On 20.09.21 18:17, Philippe Mathieu-Daudé wrote:
>>>> On 9/20/21 15:15, Peter Maydell wrote:
>>>>> On Mon, 20 Sept 2021 at 11:11, Peter Maydell
>>>>> <peter.maydell@linaro.org> wrote:
>>>>>> On Thu, 16 Sept 2021 at 16:54, Alexander Graf <agraf@csgraf.de>
>>>>>> wrote:
>>>>>>> Now that Apple Silicon is widely available, people are obviously
>>>>>>> excited
>>>>>>> to try and run virtualized workloads on them, such as Linux and
>>>>>>> Windows.
>>>>>>>
>>>>>>> This patch set implements a fully functional version to get the ball
>>>>>>> going on that. With this applied, I can successfully run both
>>>>>>> Linux and
>>>>>>> Windows as guests. I am not aware of any limitations specific to
>>>>>>> Hypervisor.framework apart from:
>>>>>>>
>>>>>>>     - gdbstub debugging (breakpoints)
>>>>>>>     - missing GICv3 support
>>>>>>>     - Windows will not work due to UDEF SMC implementation
>>>>>>>
>>>>>>> To use hvf support, please make sure to run -M virt,highmem=off
>>>>>>> to fit
>>>>>>> in M1's physical address space limits and use -cpu host.
>>>>>> Applied to target-arm.next, thanks (with the unnecessary #include
>>>>>> in patch 6 removed).
>>>>> Turns out that the final patch breaks "make check-acceptance".
>>>>> All the orangepi boot tests timeout:
>>>>>
>>>>>    (15/58)
>>>>> tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi:
>>>>> INTERRUPTED: Test interrupted by SIGTERM\nRunner error occurred:
>>>>> Timeout reached\nOriginal status: ERROR\n{'name':
>>>>> '15-tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi',
>>>>>
>>>>> 'logdir':
>>>>> '/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-clang/tests/...
>>>>> (90.24 s)
>>>>>    (16/58)
>>>>> tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_initrd:
>>>>> INTERRUPTED: Test interrupted by SIGTERM\nRunner error occurred:
>>>>> Timeout reached\nOriginal status: ERROR\n{'name':
>>>>> '16-tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_initrd',
>>>>>
>>>>> 'logdir':
>>>>> '/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-clang...
>>>>> (90.24 s)
>>>>>    (17/58)
>>>>> tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_sd:
>>>>> INTERRUPTED: Test interrupted by SIGTERM\nRunner error occurred:
>>>>> Timeout reached\nOriginal status: ERROR\n{'name':
>>>>> '17-tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_sd',
>>>>>
>>>>> 'logdir':
>>>>> '/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-clang/tes...
>>>>> (90.24 s)
>>>> Works for me on x86_64 Fedora 34 built with
>>>> --enable-trace-backends=log --enable-debug:
>>>>
>>>> $ ./tests/venv/bin/avocado run
>>>> tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi
>>>>
>>>> Fetching asset from
>>>> tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi
>>>>
>>>> Fetching asset from
>>>> tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_initrd
>>>>
>>>> Fetching asset from
>>>> tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_initrd
>>>>
>>>> Fetching asset from
>>>> tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_sd
>>>>
>>>> Fetching asset from
>>>> tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_sd
>>>>
>>>> Fetching asset from
>>>> tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_bionic_20_08
>>>>
>>>> Fetching asset from
>>>> tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_uboot_netbsd9
>>>>
>>>> Fetching asset from
>>>> tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_uboot_netbsd9
>>>>
>>>> JOB ID     : b19f151f7320def3a432255f3a99c0dde3da95c0
>>>> JOB LOG    :
>>>> /home/phil/avocado/job-results/job-2021-09-20T18.12-b19f151/job.log
>>>>    (1/5)
>>>> tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi:
>>>>
>>>> PASS (6.29 s)
>>>>    (2/5)
>>>> tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_initrd:
>>>>
>>>> PASS (51.23 s)
>>>>    (3/5)
>>>> tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_sd:
>>>>
>>>> PASS (76.53 s)
>>>>    (4/5)
>>>> tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_bionic_20_08:
>>>>
>>>> SKIP: storage limited
>>>>    (5/5)
>>>> tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_uboot_netbsd9:
>>>>
>>>> SKIP: storage limited
>>>> RESULTS    : PASS 3 | ERROR 0 | FAIL 0 | SKIP 2 | WARN 0 | INTERRUPT
>>>> 0 |
>>>> CANCEL 0
>>>> JOB TIME   : 135.18 s
>>>>
>>>
>>> The OrangePi kernel goes into an endless loop here:
>>>
>>>   
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/compressed/head.S?h=v5.10#n637
>>
>> As you said, this is a Linux specific behavior, ...
> 
> 
> It's not really Linux specific behavior. Linux just includes preloader
> code that moves it into HYP mode when booted in SVC mode.
> 
> 
>>
>>> The reason is simple: It tries to install its own HYP code using the old
>>> "install HYP handler, then invoke it" trick based on SPSR's indication
>>> that HYP is available, but fails to do so because HYP calls get handled
>>> by QEMU instead because the PSCI conduit is configured to HVC.
>>>
>>> The patch below seems to fix it for me. Please advise how you want to
>>> proceed.
>>>
>>>
>>> Alex
>>>
>>>
>>> diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c
>>> index 27f1070145..f9b7ed1871 100644
>>> --- a/hw/arm/allwinner-h3.c
>>> +++ b/hw/arm/allwinner-h3.c
>>> @@ -237,7 +237,7 @@ static void allwinner_h3_realize(DeviceState *dev,
>>> Error **errp)
>>>
>>>            /* Provide Power State Coordination Interface */
>>>            qdev_prop_set_int32(DEVICE(&s->cpus[i]), "psci-conduit",
>>> -                            QEMU_PSCI_CONDUIT_HVC);
>>> +                            QEMU_PSCI_CONDUIT_SMC);
>>
>> ... so I'd rather follow the approach taken on the Versal board,
>> see versal_virt_init() in commit 6f16da53ffe ("hw/arm: versal:
>> Add a virtual Xilinx Versal board"):
>>
>> 1/ add "psci-conduit" property in TYPE_AW_H3,
>>     forward this property to each core in allwinner_h3_realize()
>>
>> 2/ set property in orangepi_init():
>>
>>     object_property_set_int(OBJECT(h3), "psci-conduit",
>>                             machine->kernel_filename
>>                             ? QEMU_PSCI_CONDUIT_SMC
>>                             : QEMU_PSCI_CONDUIT_HVC,
>>                             &error_abort);
>>
>> That way we don't break non-Linux firmwares.
> 
> 
> This doesn't make sense to me. We need to decide what we want from the
> OrangePi emulation:
> 
>    1) Guest owns USR, SVC. HYP's PSCI is emulated by QEMU
>    2) Guest owns USR, SVC, HYP. MON's PSCI is emulated by QEMU
>    3) Guest owns USR, SVC, HYP, MON. PSCI is handled 100% inside the guest.
> 
> We currently advertise to the guest that it can use HYP, but don't allow
> it to run anything in MON (option 2), but then at the same time override
> parts of HYP with QEMU code. That's just plain wrong - we're breaking
> hypervisors running on OrangePi :).
> 
> So if the goal of that board is to run OrangePi ATF (does it do ATF?)
> code or any other "full firmware", then we would need to move QEMU
> completely out of the conduit picture, implement option 3 and ship
> pre-HYP firmware with QEMU.
> 
> If the goal is to just make HYP and below work, the fix above is the
> best path forward IMHO.
> 
> Option 1 would require us to tell the guest that it can not make use of
> HYP. That's possible, but why should we?
> 
> I don't really see why we should even consider option 1. 2 and 3 make
> sense. 3 is a lot of work. 2 is a one line fix that makes the board at
> least do what it's supposed to do :).
So this part of Xilinx Versal:

     * When loading an OS, we turn on QEMU's PSCI implementation with SMC
     * as the PSCI conduit. When there's no -kernel, we assume the user
     * provides EL3 firmware to handle PSCI.
     */
    if (machine->kernel_filename) {
        psci_conduit = QEMU_PSCI_CONDUIT_SMC;
    }

abuses the fact that -kernel command line expect a *Linux* kernel able
to read the provided dtb which describe SMC. This won't work if we
manually provide a crafted dtb with another conduit, or if we pass any
other binary (not Linux, not particularly ELF, since -kernel can load
many formats).

I guess I understand better now.

Thanks for the detailed explanation,

Phil.