mbox series

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

Message ID 20210916155404.86958-1-agraf@csgraf.de (mailing list archive)
Headers show
Series hvf: Implement Apple Silicon Support | expand

Message

Alexander Graf Sept. 16, 2021, 3:53 p.m. UTC
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.


Enjoy!

Alex

v1 -> v2:

  - New patch: hvf: Actually set SIG_IPI mask
  - New patch: hvf: Introduce hvf vcpu struct
  - New patch: hvf: arm: Mark CPU as dirty on reset
  - Removed patch: hw/arm/virt: Disable highmem when on hypervisor.framework
  - Removed patch: arm: Synchronize CPU on PSCI on
  - Fix build on 32bit arm
  - Merge vcpu kick function patch into ARM enablement
  - Implement WFI handling (allows vCPUs to sleep)
  - Synchronize system registers (fixes OVMF crashes and reboot)
  - Don't always call cpu_synchronize_state()
  - Use more fine grained iothread locking
  - Populate aa64mmfr0 from hardware
  - Make safe to ctrl-C entitlement application

v2 -> v3:

  - Removed patch: hvf: Actually set SIG_IPI mask
  - New patch: hvf: arm: Add support for GICv3
  - New patch: hvf: arm: Implement -cpu host
  - Advance PC on SMC
  - Use cp list interface for sysreg syncs
  - Do not set current_cpu
  - Fix sysreg isread mask
  - Move sysreg handling to functions
  - Remove WFI logic again
  - Revert to global iothread locking

v3 -> v4:

  - Removed patch: hvf: arm: Mark CPU as dirty on reset
  - New patch: hvf: Simplify post reset/init/loadvm hooks
  - Remove i386-softmmu target (meson.build for hvf target)
  - Combine both if statements (PSCI)
  - Use hv.h instead of Hypervisor.h for 10.15 compat
  - Remove manual inclusion of Hypervisor.h in common .c files
  - No longer include Hypervisor.h in arm hvf .c files
  - Remove unused exe_full variable
  - Reuse exe_name variable

v4 -> v5:

  - Use g_free() on destroy

v5 -> v6:

  - Switch SYSREG() macro order to the same as asm intrinsics

v6 -> v7:

  - Already merged: hvf: Add hypervisor entitlement to output binaries
  - Already merged: hvf: x86: Remove unused definitions
  - Patch split: hvf: Move common code out
    -> hvf: Move assert_hvf_ok() into common directory
    -> hvf: Move vcpu thread functions into common directory
    -> hvf: Move cpu functions into common directory
    -> hvf: Move hvf internal definitions into common header
    -> hvf: Make hvf_set_phys_mem() static
    -> hvf: Remove use of hv_uvaddr_t and hv_gpaddr_t
    -> hvf: Split out common code on vcpu init and destroy
    -> hvf: Use cpu_synchronize_state()
    -> hvf: Make synchronize functions static
    -> hvf: Remove hvf-accel-ops.h
  - New patch: hvf: arm: Implement PSCI handling
  - New patch: arm: Enable Windows 10 trusted SMCCC boot call
  - New patch: hvf: arm: Handle Windows 10 SMC call
  - Removed patch: "arm: Set PSCI to 0.2 for HVF" (included above)
  - Removed patch: "hvf: arm: Add support for GICv3" (deferred to later)
  - Remove osdep.h include from hvf_int.h
  - Synchronize SIMD registers as well
  - Prepend 0x for hex values
  - Convert DPRINTF to trace points
  - Use main event loop (fixes gdbstub issues)
  - Remove PSCI support, inject UDEF on HVC/SMC
  - Change vtimer logic to look at ctl.istatus for vtimer mask sync
  - Add kick callback again (fixes remote CPU notification)
  - Move function define to own header
  - Do not propagate SVE features for HVF
  - Remove stray whitespace change
  - Verify that EL0 and EL1 do not allow AArch32 mode
  - Only probe host CPU features once
  - Move WFI into function
  - Improve comment wording
  - Simplify HVF matching logic in meson build file

v7 -> v8:

  - checkpatch fixes
  - Do not advance for HVC, PC is already updated by hvf
    (fixes Linux boot)

v8 -> v9:

  - [Merged] hvf: Move assert_hvf_ok() into common directory
  - [Merged] hvf: Move vcpu thread functions into common directory
  - [Merged] hvf: Move cpu functions into common directory
  - [Merged] hvf: Move hvf internal definitions into common header
  - [Merged] hvf: Make hvf_set_phys_mem() static
  - [Merged] hvf: Remove use of hv_uvaddr_t and hv_gpaddr_t
  - [Merged] hvf: Split out common code on vcpu init and destroy
  - [Merged] hvf: Use cpu_synchronize_state()
  - [Merged] hvf: Make synchronize functions static
  - [Merged] hvf: Remove hvf-accel-ops.h
  - [Merged] hvf: Introduce hvf vcpu struct
  - [Merged] hvf: Simplify post reset/init/loadvm hooks
  - [Dropped] arm: Enable Windows 10 trusted SMCCC boot call
  - [Dropped] hvf: arm: Handle Windows 10 SMC call
  - [New] arm: Move PMC register definitions to cpu.h
  - [New] hvf: Add execute to dirty log permission bitmap
  - [New] hvf: Introduce hvf_arch_init() callback
  - [New] hvf: arm: Implement PSCI handling
  - [New] hvf: arm: Add rudimentary PMC support
  - [New] arm: tcg: Adhere to SMCCC 1.3 section 5.2
  - [New] hvf: arm: Adhere to SMCCC 1.3 section 5.2
  - Make kick function non-weak
  - Use arm_cpu_do_interrupt()
  - Remove CNTPCT_EL0 write case
  - Inject UDEF on invalid sysreg access
  - Add support for OS locking sysregs
  - Remove PMCCNTR_EL0 handling
  - Print PC on unhandled sysreg trace
  - Sync SP (x31) based on SP_EL0/SP_EL1
  - Fix SPSR_EL1 mapping
  - Only sync known sysregs, assert when syncing fails
  - Improve error message on unhandled ec
  - Move vtimer sync to post-exit (fixes disable corner case from
    kvm-unit-tests)
  - Add vtimer offset, migration and pause logic
  - Flush registers only after EXCP checkers (fixes PSCI on race)
  - Remove Windows specifics and just comply with SMCCC spec
  - Zero-initialize host_isar
  - Use M1 SCTLR reset value
  - Add support for cntv offsets
  - Improve code readability
  - Use new hvf_raise_exception() prototype
  - Make cpu_off function void
  - Add comment about return value, use -1 for "not found"
  - Remove cpu_synchronize_state() when halted

v9 -> v10:

  - [Dropped] hvf: arm: Adhere to SMCCC 1.3 section 5.2
  - Only handle PSCI calls for the current conduit
  - Return true/false
  - Return -1 in x0 on unknown SMC/HVC calls
  - Move to target/arm/internals.h
  - Fail -cpu host class creation gracefully
  - Adjust error message on -cpu host realize failure
  - Extend SCTLR comment that hvf returns 0 as default value
  - Return true/false
  - Report errors lazily
  - Fix comment

v10 -> v11:

  - Treat SMC as UDEF. A follow-up patch set will try to change behavior
    consistently in TCG as well as HVF.

v11 -> v12:

  - Structure SMC/HVC handling code similarly
  - Inject UDEF on HVC with SMC conduit
  - s/hvf_unknown_hvf/hvf_unknown_hvc/g
  - s/hvf_unknown_hvf/hvf_unknown_hvc/g
  - Use muldiv64
  - Declare ts at start of block
  - Improve SPAN comment

Alexander Graf (9):
  arm: Move PMC register definitions to internals.h
  hvf: Add execute to dirty log permission bitmap
  hvf: Introduce hvf_arch_init() callback
  hvf: Add Apple Silicon support
  hvf: arm: Implement -cpu host
  hvf: arm: Implement PSCI handling
  arm: Add Hypervisor.framework build target
  hvf: arm: Add rudimentary PMC support
  arm: tcg: Adhere to SMCCC 1.3 section 5.2

Peter Collingbourne (1):
  arm/hvf: Add a WFI handler

 MAINTAINERS                 |    5 +
 accel/hvf/hvf-accel-ops.c   |   21 +-
 include/sysemu/hvf_int.h    |   12 +-
 meson.build                 |    8 +
 target/arm/cpu.c            |   17 +-
 target/arm/cpu.h            |    2 +
 target/arm/helper.c         |   44 --
 target/arm/hvf/hvf.c        | 1274 +++++++++++++++++++++++++++++++++++
 target/arm/hvf/meson.build  |    3 +
 target/arm/hvf/trace-events |   11 +
 target/arm/hvf_arm.h        |   19 +
 target/arm/internals.h      |   44 ++
 target/arm/kvm_arm.h        |    2 -
 target/arm/meson.build      |    2 +
 target/arm/psci.c           |   35 +-
 target/i386/hvf/hvf.c       |   10 +
 16 files changed, 1421 insertions(+), 88 deletions(-)
 create mode 100644 target/arm/hvf/hvf.c
 create mode 100644 target/arm/hvf/meson.build
 create mode 100644 target/arm/hvf/trace-events
 create mode 100644 target/arm/hvf_arm.h

Comments

Peter Maydell Sept. 20, 2021, 10:11 a.m. UTC | #1
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).

-- PMM
Peter Maydell Sept. 20, 2021, 1:15 p.m. UTC | #2
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)

So I'm going to drop that one; we need to work out what's
going wrong with the orangepi-pc machine.

thanks
-- PMM
Philippe Mathieu-Daudé Sept. 20, 2021, 4:17 p.m. UTC | #3
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
Alexander Graf Sept. 20, 2021, 8:21 p.m. UTC | #4
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

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);

         /* Disable secondary CPUs */
         qdev_prop_set_bit(DEVICE(&s->cpus[i]), "start-powered-off",
Philippe Mathieu-Daudé Sept. 21, 2021, 9:29 a.m. UTC | #5
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, ...

> 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.
Alexander Graf Sept. 21, 2021, 1:30 p.m. UTC | #6
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 :).


Alex
Philippe Mathieu-Daudé Sept. 25, 2021, 5:22 p.m. UTC | #7
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.
Peter Maydell Sept. 25, 2021, 6:09 p.m. UTC | #8
On Sat, 25 Sept 2021 at 18:22, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> 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

...which is not a supported thing to do (it won't work with the
'virt' board either)...

>, or if we pass any
> other binary (not Linux, not particularly ELF, since -kernel can load
> many formats).

-kernel means "assume I am a Linux kernel and boot me accordingly".
Sometimes it works for other things, but there are no guarantees.
Among other things it always means "start me at EL2 or EL1, not EL3",
so PSCI handling via SMC doesn't get in anybody's way even if they're
not using it. (People who want a pure "load this ELF file" should
use the generic-loader.)

The basic distinction the versal-virt board is making here is
"we have EL3 firmware to run in the guest" vs "we don't";
for the same reason that the virt board does, in the former
case we disable QEMU's internal PSCI implementation, and in
the latter we enable it. In theory the orangepi board code
should do the same, because if we're really running guest
code at EL3 it probably is going to assume it has SMC, and
QEMU taking control instead is going to confuse it.

-- PMM