Hi Peter,
On Thu, Jan 27, 2022 at 4:46 PM Peter Maydell <peter.mayd...@linaro.org>
wrote:
> Change the allwinner-h3 based board to use the new boot.c
> functionality to allow us to enable psci-conduit only if the guest is
> being booted in EL1 or EL2, so that if the user runs guest EL3
> firmware code our PSCI emulation doesn't get in its way.
>
> To do this we stop setting the psci-conduit property on the CPU
> objects in the SoC code, and instead set the psci_conduit field in
> the arm_boot_info struct to tell the common boot loader code that
> we'd like PSCI if the guest is starting at an EL that it makes sense
> with.
>
> This affects the orangepi-pc board.
>
> This commit leaves the secondary CPUs in the powered-down state if
> the guest is booting at EL3, which is the same behaviour as before
> this commit. The secondaries can no longer be started by that EL3
> code making a PSCI call but can still be started via the CPU
> Configuration Module registers (which we model in
> hw/misc/allwinner-cpucfg.c).
>
> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
>
While testing your patches on the orangepi-pc machine, I've found that two
acceptance tests BootLinuxConsole.test_arm_orangepi_bionic_20_08 and
BootLinuxConsole.test_arm_orangepi_uboot_netbsd9 of the orangepi-pc board
are no longer passing on current master:
ARMBIAN_ARTIFACTS_CACHED=yes AVOCADO_ALLOW_LARGE_STORAGE=yes avocado
--show=app,console run -t machine:orangepi-pc
tests/avocado/boot_linux_console.py
...
(4/5)
tests/avocado/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_bionic_20_08:
-console: U-Boot SPL 2020.04-armbian (Sep 02 2020 - 10:16:13 +0200)
\console: DRAM:
INTERRUPTED: Test interrupted by SIGTERM\nRunner error occurred: Timeout
reached\nOriginal status: ERROR\n{'name':
'4-tests/avocado/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_bionic_20_08',
'logdi>
(5/5)
tests/avocado/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_uboot_netbsd9:
/console: U-Boot SPL 2020.01+dfsg-1 (Jan 08 2020 - 08:19:44 +0000)
console: DRAM:
INTERRUPTED: Test interrupted by SIGTERM\nRunner error occurred: Timeout
reached\nOriginal status: ERROR\n{'name':
'5-tests/avocado/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_uboot_netbsd9',
'logd>
RESULTS : PASS 3 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 2 |
CANCEL 0
JOB TIME : 221.25 s
Bisecting the error I was able to trace it back to commit 5ead62185d
("memory: Make memory_region_is_mapped() succeed when mapped via an alias").
I'll try to find the original thread and respond to that with this
information.
However, with commit 5ead62185d reverted, all tested passed fine:
ARMBIAN_ARTIFACTS_CACHED=yes AVOCADO_ALLOW_LARGE_STORAGE=yes avocado
--show=app,console run -t machine:orangepi-pc
tests/avocado/boot_linux_console.py
...
PASS (16.48 s)
RESULTS : PASS 5 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 |
CANCEL 0
JOB TIME : 116.63 s
So for the orangepi-pc and allwinner-h3:
Reviewed-by: Niek Linnenbank <nieklinnenb...@gmail.com>
Tested-by: Niek Linnenbank <nieklinnenb...@gmail.com>
> ---
> If anybody knows for definite that the secondaries should be
> powered-down at startup for guest firmware, we can delete the TODO.
>
Looking at the Allwinner H3 datasheet in 4.4.3.7 page 146, it says that
for the CPU1 Status Register the default value indicates at least that its
not in a
wait for interrupt standby mode. And if I look in U-Boot's
arm/arm/cpu/armv7/sunxi/psci.c code
in the psci_cpu_on implementation, there is an explicit 'power on' part
there, suggesting the secondary CPUs
are by default off. So while I don't have any hard proof, these findings
suggest we are modeling the correct behavior
with secondary CPUs by default off.
> The allwinner-cpucfg.c code makes the reset value for the
> REG_CPU*_RST_CTRL registers "CPUX_RESET_RELEASED", which might
> suggest otherwise, but that could easily just be a QEMU error.
> ---
> hw/arm/allwinner-h3.c | 9 ++++-----
> hw/arm/orangepi.c | 1 +
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c
> index f9b7ed18711..f54aff6d2d2 100644
> --- a/hw/arm/allwinner-h3.c
> +++ b/hw/arm/allwinner-h3.c
> @@ -235,11 +235,10 @@ static void allwinner_h3_realize(DeviceState *dev,
> Error **errp)
> /* CPUs */
> for (i = 0; i < AW_H3_NUM_CPUS; i++) {
>
> - /* Provide Power State Coordination Interface */
> - qdev_prop_set_int32(DEVICE(&s->cpus[i]), "psci-conduit",
> - QEMU_PSCI_CONDUIT_SMC);
> -
> - /* Disable secondary CPUs */
> + /*
> + * Disable secondary CPUs. TODO: check whether this is what
> + * guest EL3 firmware would really expect.
> + */
> qdev_prop_set_bit(DEVICE(&s->cpus[i]), "start-powered-off",
> i > 0);
>
> diff --git a/hw/arm/orangepi.c b/hw/arm/orangepi.c
> index e7963822367..68fe9182414 100644
> --- a/hw/arm/orangepi.c
> +++ b/hw/arm/orangepi.c
> @@ -105,6 +105,7 @@ static void orangepi_init(MachineState *machine)
> }
> orangepi_binfo.loader_start = h3->memmap[AW_H3_DEV_SDRAM];
> orangepi_binfo.ram_size = machine->ram_size;
> + orangepi_binfo.psci_conduit = QEMU_PSCI_CONDUIT_SMC;
> arm_load_kernel(ARM_CPU(first_cpu), machine, &orangepi_binfo);
> }
>
> --
> 2.25.1
>
>
--
Niek Linnenbank
