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.maydell@linaro.org>
---
If anybody knows for definite that the secondaries should be
powered-down at startup for guest firmware, we can delete the TODO.
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
Hi Peter,
On Thu, Jan 27, 2022 at 4:46 PM Peter Maydell <peter.maydell@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.maydell@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 <nieklinnenbank@gmail.com>
Tested-by: Niek Linnenbank <nieklinnenbank@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
On Sun, 30 Jan 2022 23:35:37 +0100
Niek Linnenbank <nieklinnenbank@gmail.com> wrote:
Hi,
(CC:ing Samuel for his intimate Allwinner BootROM knowledge)
> Hi Peter,
>
>
>
> On Thu, Jan 27, 2022 at 4:46 PM Peter Maydell <peter.maydell@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.maydell@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 <nieklinnenbank@gmail.com>
> Tested-by: Niek Linnenbank <nieklinnenbank@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.
So when it comes to firmware, indeed the secondaries seem to be off when
the first user provided code (boot0/SPL) is entered. However there is an
MPIDR read in the BROM, with the corresponding "branch if not primary
core". I think this is because the BROM is mapped at the reset vector, so
upon SMP firmware releasing the reset line it always starts in ROM, then
gets diverted to the actual entry point.
Maybe Samuel can confirm that the secondary cores are power gated when the
SoCs comes out of reset?
Cheers,
Andre
> > 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
> >
> >
>
On 1/31/22 4:52 AM, Andre Przywara wrote:
> On Sun, 30 Jan 2022 23:35:37 +0100
> Niek Linnenbank <nieklinnenbank@gmail.com> wrote:
>> On Thu, Jan 27, 2022 at 4:46 PM Peter Maydell <peter.maydell@linaro.org>
>> wrote:
>>> 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.
>
> So when it comes to firmware, indeed the secondaries seem to be off when
> the first user provided code (boot0/SPL) is entered. However there is an
> MPIDR read in the BROM, with the corresponding "branch if not primary
> core". I think this is because the BROM is mapped at the reset vector, so
> upon SMP firmware releasing the reset line it always starts in ROM, then
> gets diverted to the actual entry point.
Yes, I believe this is an accurate explanation.
> Maybe Samuel can confirm that the secondary cores are power gated when the
> SoCs comes out of reset?
The secondary cores are powered up but held in reset, as described in the
datasheet (sections 4.4.3.5/8/11 "CPU1/2/3 Reset Register"). The boot ROM does
not touch any of the CPU Configuration registers, so they hold their default
values into U-Boot execution. Looking at the registers from the U-Boot shell
confirms the datasheet:
=> md.l 1f01500 20
01f01500: 00000000 00000000 00000000 00000000 ................
^ output gating[0] is disabled for all CPUs
01f01510: 00000000 00000000 00000000 00000000 ................
01f01520: 00000001 00000000 00000000 00000000 ................
01f01530: 00000000 00000000 00000000 00000000 ................
01f01540: 00000000 00000000 00000000 00000000 ................
^^ ^^ ^^ ^^
power switches[1] are enabled for all CPUs
01f01550: 00000000 00000000 00000000 00000000 ................
01f01560: 00000000 00000000 00000000 00000000 ................
01f01570: 00000000 00000000 00000000 00000000 ................
=> md.l 1f01c40 40
01f01c40: 00000003 00000000 00000001 00000000 ................
^ nCPUPORESET and nCORERESET deasserted
01f01c50: 00000000 00000000 00000000 00000000 ................
01f01c60: 00000000 00000000 00000000 00000000 ................
01f01c70: 00000000 00000000 00000000 00000000 ................
01f01c80: 00000001 00000000 00000000 00000000 ................
^ nCPUPORESET deasserted; nCORERESET asserted
01f01c90: 00000000 00000000 00000000 00000000 ................
01f01ca0: 00000000 00000000 00000000 00000000 ................
01f01cb0: 00000000 00000000 00000000 00000000 ................
01f01cc0: 00000001 00000000 00000000 00000000 ................
^ nCPUPORESET deasserted; nCORERESET asserted
01f01cd0: 00000000 00000000 00000000 00000000 ................
01f01ce0: 00000000 00000000 00000000 00000000 ................
01f01cf0: 00000000 00000000 00000000 00000000 ................
01f01d00: 00000001 00000000 00000000 00000000 ................
^ nCPUPORESET deasserted; nCORERESET asserted
01f01d10: 00000000 00000000 00000000 00000000 ................
01f01d20: 00000000 00000000 00000000 00000000 ................
01f01d30: 00000000 00000000 00000000 00000000 ................
Since these CPU configuration registers are in the "CPUS" (ARISC) power domain,
it is not possible to reset them to their defaults using software. However, on
64-bit SoCs the CPUCFG registers are in the "CPU_SUBSYS" (ARM) power domain, and
they can be fully reset by toggling CPU_SYS_RESET[2][3]. Using A64 as an
example, the default value for the Cluster Reset Control Register 0x11101101,
which also matches the manual (section 3.4.5.8). Again, this has core 0 reset
deasserted and the remaining core resets asserted. So far, this behavior has
been consistent across every Allwinner SoC I have worked with.
Regards,
Samuel
[0]:
https://github.com/crust-firmware/crust/blob/master/platform/h3/include/platform/prcm.h#L55
[1]:
https://github.com/crust-firmware/crust/blob/master/platform/h3/include/platform/prcm.h#L69
[2]:
https://github.com/crust-firmware/crust/blob/master/platform/a64/include/platform/cpucfg.h#L98
[3]:
https://github.com/crust-firmware/crust/blob/master/drivers/css/sun50i-a64-css.c#L24
© 2016 - 2026 Red Hat, Inc.