On 27.01.22 16:46, Peter Maydell wrote:
> Instead of setting the CPU psci-conduit and start-powered-off
> properties in the xlnx-versal-virt board code, set the arm_boot_info
> psci_conduit field so that the boot.c code can do it.
>
> This will fix a corner case where we were incorrectly enabling PSCI
> emulation when booting guest code into EL3 because it was an ELF file
> passed to -kernel. (EL3 guest code started via -bios, -pflash, or
> the generic loader was already being run with PSCI emulation
> disabled.)
>
> Note that EL3 guest code has no way to turn on the secondary CPUs
> because there's no emulated power controller, but this was already
> true for EL3 guest code run via -bios, -pflash, or the generic
> loader.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Let's try with the same CCs here too - Versal uses the same mechanism as
ZynqMP :)
Alex
> ---
> include/hw/arm/xlnx-versal.h | 1 -
> hw/arm/xlnx-versal-virt.c | 6 ++++--
> hw/arm/xlnx-versal.c | 5 +----
> 3 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/include/hw/arm/xlnx-versal.h b/include/hw/arm/xlnx-versal.h
> index 895ba12c61e..2de487564e4 100644
> --- a/include/hw/arm/xlnx-versal.h
> +++ b/include/hw/arm/xlnx-versal.h
> @@ -89,7 +89,6 @@ struct Versal {
>
> struct {
> MemoryRegion *mr_ddr;
> - uint32_t psci_conduit;
> } cfg;
> };
>
> diff --git a/hw/arm/xlnx-versal-virt.c b/hw/arm/xlnx-versal-virt.c
> index 0c5edc898e1..1b25342501b 100644
> --- a/hw/arm/xlnx-versal-virt.c
> +++ b/hw/arm/xlnx-versal-virt.c
> @@ -626,6 +626,9 @@ static void versal_virt_init(MachineState *machine)
> * 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.
> + *
> + * Even if the user provides a kernel filename, arm_load_kernel()
> + * may suppress PSCI if it's going to boot that guest code at EL3.
> */
> if (machine->kernel_filename) {
> psci_conduit = QEMU_PSCI_CONDUIT_SMC;
> @@ -635,8 +638,6 @@ static void versal_virt_init(MachineState *machine)
> TYPE_XLNX_VERSAL);
> object_property_set_link(OBJECT(&s->soc), "ddr", OBJECT(machine->ram),
> &error_abort);
> - object_property_set_int(OBJECT(&s->soc), "psci-conduit", psci_conduit,
> - &error_abort);
> sysbus_realize(SYS_BUS_DEVICE(&s->soc), &error_fatal);
>
> fdt_create(s);
> @@ -677,6 +678,7 @@ static void versal_virt_init(MachineState *machine)
> s->binfo.loader_start = 0x0;
> s->binfo.get_dtb = versal_virt_get_dtb;
> s->binfo.modify_dtb = versal_virt_modify_dtb;
> + s->binfo.psci_conduit = psci_conduit;
> if (machine->kernel_filename) {
> arm_load_kernel(&s->soc.fpd.apu.cpu[0], machine, &s->binfo);
> } else {
> diff --git a/hw/arm/xlnx-versal.c b/hw/arm/xlnx-versal.c
> index b2705b6925e..458ba33815f 100644
> --- a/hw/arm/xlnx-versal.c
> +++ b/hw/arm/xlnx-versal.c
> @@ -35,10 +35,8 @@ static void versal_create_apu_cpus(Versal *s)
> object_initialize_child(OBJECT(s), "apu-cpu[*]", &s->fpd.apu.cpu[i],
> XLNX_VERSAL_ACPU_TYPE);
> obj = OBJECT(&s->fpd.apu.cpu[i]);
> - object_property_set_int(obj, "psci-conduit", s->cfg.psci_conduit,
> - &error_abort);
> if (i) {
> - /* Secondary CPUs start in PSCI powered-down state */
> + /* Secondary CPUs start in powered-down state */
> object_property_set_bool(obj, "start-powered-off", true,
> &error_abort);
> }
> @@ -481,7 +479,6 @@ static void versal_init(Object *obj)
> static Property versal_properties[] = {
> DEFINE_PROP_LINK("ddr", Versal, cfg.mr_ddr, TYPE_MEMORY_REGION,
> MemoryRegion *),
> - DEFINE_PROP_UINT32("psci-conduit", Versal, cfg.psci_conduit, 0),
> DEFINE_PROP_END_OF_LIST()
> };
>