| Message ID | 20220127154639.2090164-7-peter.maydell@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | arm: Fix handling of unrecognized functions in PSCI emulation | expand |
On 1/28/22 02:46, Peter Maydell wrote: > Change the Xilinx ZynqMP-based board xlnx-zcu102 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. > > Note that this means that EL3 guest code will have no way > to power on secondary cores, because we don't model any > kind of power controller that does that on this SoC. > > Signed-off-by: Peter Maydell<peter.maydell@linaro.org> > --- > Again, if anybody knows the real-hardware EL3 behaviour for > CPUs that would be great. > --- > hw/arm/xlnx-zcu102.c | 1 + > hw/arm/xlnx-zynqmp.c | 13 ++++++++----- > 2 files changed, 9 insertions(+), 5 deletions(-) Acked-by: Richard Henderson <richard.henderson@linaro.org> r~
On 27.01.22 16:46, Peter Maydell wrote: > Change the Xilinx ZynqMP-based board xlnx-zcu102 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. > > Note that this means that EL3 guest code will have no way > to power on secondary cores, because we don't model any > kind of power controller that does that on this SoC. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> It's been a while since I worked with ZynqMP, but typically your ATF in EL3 will want to talk to a microblaze firmware blob on the PMU. I only see a stand alone PMU machine for microblaze and a PMU IRQ handling I/O block in QEMU, but nothing that would listen to the events. So I'm fairly sure it will be broken after this patch - and really only worked by accident before. I've added Michal Simek and Stefano Stabellini (both Xilinx) to CC to clarify and determine the best path forward here - either disallow EL3 in the model or build proper PMU emulation in QEMU which then handles those PSCI triggered IPI events. Alex [1] https://github.com/Xilinx/arm-trusted-firmware/blob/master/plat/xilinx/zynqmp/plat_psci.c > --- > Again, if anybody knows the real-hardware EL3 behaviour for > CPUs that would be great. > --- > hw/arm/xlnx-zcu102.c | 1 + > hw/arm/xlnx-zynqmp.c | 13 ++++++++----- > 2 files changed, 9 insertions(+), 5 deletions(-) > > diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c > index 45eb19ab3b7..4c84bb932aa 100644 > --- a/hw/arm/xlnx-zcu102.c > +++ b/hw/arm/xlnx-zcu102.c > @@ -236,6 +236,7 @@ static void xlnx_zcu102_init(MachineState *machine) > s->binfo.ram_size = ram_size; > s->binfo.loader_start = 0; > s->binfo.modify_dtb = zcu102_modify_dtb; > + s->binfo.psci_conduit = QEMU_PSCI_CONDUIT_SMC; > arm_load_kernel(s->soc.boot_cpu_ptr, machine, &s->binfo); > } > > diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c > index 1c52a575aad..17305fe7b76 100644 > --- a/hw/arm/xlnx-zynqmp.c > +++ b/hw/arm/xlnx-zynqmp.c > @@ -215,7 +215,10 @@ static void xlnx_zynqmp_create_rpu(MachineState *ms, XlnxZynqMPState *s, > > name = object_get_canonical_path_component(OBJECT(&s->rpu_cpu[i])); > if (strcmp(name, boot_cpu)) { > - /* Secondary CPUs start in PSCI powered-down state */ > + /* > + * Secondary CPUs start in powered-down state. > + * TODO: check this is what EL3 firmware expects. > + */ > object_property_set_bool(OBJECT(&s->rpu_cpu[i]), > "start-powered-off", true, &error_abort); > } else { > @@ -435,12 +438,12 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp) > for (i = 0; i < num_apus; i++) { > const char *name; > > - object_property_set_int(OBJECT(&s->apu_cpu[i]), "psci-conduit", > - QEMU_PSCI_CONDUIT_SMC, &error_abort); > - > name = object_get_canonical_path_component(OBJECT(&s->apu_cpu[i])); > if (strcmp(name, boot_cpu)) { > - /* Secondary CPUs start in PSCI powered-down state */ > + /* > + * Secondary CPUs start in powered-down state. > + * TODO: check this is what EL3 firmware expects. > + */ > object_property_set_bool(OBJECT(&s->apu_cpu[i]), > "start-powered-off", true, &error_abort); > } else {
On Mon, 7 Feb 2022 at 14:21, Alexander Graf <agraf@csgraf.de> wrote: > > > On 27.01.22 16:46, Peter Maydell wrote: > > Change the Xilinx ZynqMP-based board xlnx-zcu102 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. > > > > Note that this means that EL3 guest code will have no way > > to power on secondary cores, because we don't model any > > kind of power controller that does that on this SoC. > > > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > > > It's been a while since I worked with ZynqMP, but typically your ATF in > EL3 will want to talk to a microblaze firmware blob on the PMU. > > I only see a stand alone PMU machine for microblaze and a PMU IRQ > handling I/O block in QEMU, but nothing that would listen to the events. > So I'm fairly sure it will be broken after this patch - and really only > worked by accident before. Edgar submitted a power-control model patchset: https://patchew.org/QEMU/20220203140141.310870-1-edgar.iglesias@gmail.com/ thanks -- PMM
On 07.02.22 16:22, Peter Maydell wrote: > On Mon, 7 Feb 2022 at 14:21, Alexander Graf <agraf@csgraf.de> wrote: >> >> On 27.01.22 16:46, Peter Maydell wrote: >>> Change the Xilinx ZynqMP-based board xlnx-zcu102 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. >>> >>> Note that this means that EL3 guest code will have no way >>> to power on secondary cores, because we don't model any >>> kind of power controller that does that on this SoC. >>> >>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> >> >> It's been a while since I worked with ZynqMP, but typically your ATF in >> EL3 will want to talk to a microblaze firmware blob on the PMU. >> >> I only see a stand alone PMU machine for microblaze and a PMU IRQ >> handling I/O block in QEMU, but nothing that would listen to the events. >> So I'm fairly sure it will be broken after this patch - and really only >> worked by accident before. > Edgar submitted a power-control model patchset: > https://patchew.org/QEMU/20220203140141.310870-1-edgar.iglesias@gmail.com/ Ah, nice. Would this also work for Versal? Thanks, Alex
On Mon, Feb 7, 2022 at 4:33 PM Alexander Graf <agraf@csgraf.de> wrote: > > On 07.02.22 16:22, Peter Maydell wrote: > > On Mon, 7 Feb 2022 at 14:21, Alexander Graf <agraf@csgraf.de> wrote: > >> > >> On 27.01.22 16:46, Peter Maydell wrote: > >>> Change the Xilinx ZynqMP-based board xlnx-zcu102 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. > >>> > >>> Note that this means that EL3 guest code will have no way > >>> to power on secondary cores, because we don't model any > >>> kind of power controller that does that on this SoC. > >>> > >>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > >> > >> It's been a while since I worked with ZynqMP, but typically your ATF in > >> EL3 will want to talk to a microblaze firmware blob on the PMU. > >> > >> I only see a stand alone PMU machine for microblaze and a PMU IRQ > >> handling I/O block in QEMU, but nothing that would listen to the events. > >> So I'm fairly sure it will be broken after this patch - and really only > >> worked by accident before. > > Edgar submitted a power-control model patchset: > > > https://patchew.org/QEMU/20220203140141.310870-1-edgar.iglesias@gmail.com/ > > > Ah, nice. Would this also work for Versal? > > > Thanks, > > Alex > Hi, Both Versal and ZynqMP require MicroBlaze firmware to run the reference implementations of Trusted Firmware. We never supported this in upstream QEMU but we do support it with our fork (by running multiple QEMU instances co-simulating). Having said that, we do have tons of EL3 test-cases that we use to validate QEMU that run with EL3 enabled in upstream. So there's two user flows: 1. Direct boots using QEMUs builtin PSCI (Most users use this to run Linux, Xen, U-boot, etc) 2. Firmware boot at EL3 without QEMUs builtin PSCI (Mostly used by test-code) Number #2 is the one affected here and that by accident used to have the builtin PSCI support enabled but now requires more power control modelling to keep working. Unless I'm missing something, the -kernel boots will continue to use the builtin PSCI implementation. Cheers, Edgar
On 07.02.22 16:52, Edgar E. Iglesias wrote: > > > On Mon, Feb 7, 2022 at 4:33 PM Alexander Graf <agraf@csgraf.de> wrote: > > > On 07.02.22 16:22, Peter Maydell wrote: > > On Mon, 7 Feb 2022 at 14:21, Alexander Graf <agraf@csgraf.de> wrote: > >> > >> On 27.01.22 16:46, Peter Maydell wrote: > >>> Change the Xilinx ZynqMP-based board xlnx-zcu102 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. > >>> > >>> Note that this means that EL3 guest code will have no way > >>> to power on secondary cores, because we don't model any > >>> kind of power controller that does that on this SoC. > >>> > >>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > >> > >> It's been a while since I worked with ZynqMP, but typically > your ATF in > >> EL3 will want to talk to a microblaze firmware blob on the PMU. > >> > >> I only see a stand alone PMU machine for microblaze and a PMU IRQ > >> handling I/O block in QEMU, but nothing that would listen to > the events. > >> So I'm fairly sure it will be broken after this patch - and > really only > >> worked by accident before. > > Edgar submitted a power-control model patchset: > > > https://patchew.org/QEMU/20220203140141.310870-1-edgar.iglesias@gmail.com/ > > > Ah, nice. Would this also work for Versal? > > > Thanks, > > Alex > > > Hi, > > Both Versal and ZynqMP require MicroBlaze firmware to run the > reference implementations of Trusted Firmware. We never supported this > in upstream QEMU but we do support it with our fork (by running > multiple QEMU instances co-simulating). > > Having said that, we do have tons of EL3 test-cases that we use to > validate QEMU that run with EL3 enabled in upstream. > > So there's two user flows: > 1. Direct boots using QEMUs builtin PSCI (Most users use this to run > Linux, Xen, U-boot, etc) > 2. Firmware boot at EL3 without QEMUs builtin PSCI (Mostly used by > test-code) > > Number #2 is the one affected here and that by accident used to have > the builtin PSCI support enabled but now requires more power control > modelling to keep working. > Unless I'm missing something, the -kernel boots will continue to use > the builtin PSCI implementation. So nobody is using upstream QEMU to validate and prototype ATF/EL1s/EL0s code? That's a shame :). I suppose there is little value without the bitstream emulation and R cluster. Do you have plans to bring multi process emulation upstream some day to enable these there? Alex
On 07.02.22 17:06, Philippe Mathieu-Daudé wrote: > On 7/2/22 16:59, Alexander Graf wrote: >> >> On 07.02.22 16:52, Edgar E. Iglesias wrote: > >>> Both Versal and ZynqMP require MicroBlaze firmware to run the >>> reference implementations of Trusted Firmware. We never supported >>> this in upstream QEMU but we do support it with our fork (by running >>> multiple QEMU instances co-simulating). >>> >>> Having said that, we do have tons of EL3 test-cases that we use to >>> validate QEMU that run with EL3 enabled in upstream. >>> >>> So there's two user flows: >>> 1. Direct boots using QEMUs builtin PSCI (Most users use this to run >>> Linux, Xen, U-boot, etc) >>> 2. Firmware boot at EL3 without QEMUs builtin PSCI (Mostly used by >>> test-code) >>> >>> Number #2 is the one affected here and that by accident used to have >>> the builtin PSCI support enabled but now requires more power control >>> modelling to keep working. >>> Unless I'm missing something, the -kernel boots will continue to use >>> the builtin PSCI implementation. >> >> >> So nobody is using upstream QEMU to validate and prototype >> ATF/EL1s/EL0s code? That's a shame :). I suppose there is little >> value without the bitstream emulation and R cluster. Do you have >> plans to bring multi process emulation upstream some day to enable >> these there? > > The R cluster is already in mainstream, isn't it? In that case, wouldn't it make sense to build an emulation model of the PMU behavior so that normal ATF works out of the box? Thanks, Alex
On Mon, Feb 7, 2022 at 5:24 PM Alexander Graf <agraf@csgraf.de> wrote: > > On 07.02.22 17:06, Philippe Mathieu-Daudé wrote: > > On 7/2/22 16:59, Alexander Graf wrote: > >> > >> On 07.02.22 16:52, Edgar E. Iglesias wrote: > > > >>> Both Versal and ZynqMP require MicroBlaze firmware to run the > >>> reference implementations of Trusted Firmware. We never supported > >>> this in upstream QEMU but we do support it with our fork (by running > >>> multiple QEMU instances co-simulating). > >>> > >>> Having said that, we do have tons of EL3 test-cases that we use to > >>> validate QEMU that run with EL3 enabled in upstream. > >>> > >>> So there's two user flows: > >>> 1. Direct boots using QEMUs builtin PSCI (Most users use this to run > >>> Linux, Xen, U-boot, etc) > >>> 2. Firmware boot at EL3 without QEMUs builtin PSCI (Mostly used by > >>> test-code) > >>> > >>> Number #2 is the one affected here and that by accident used to have > >>> the builtin PSCI support enabled but now requires more power control > >>> modelling to keep working. > >>> Unless I'm missing something, the -kernel boots will continue to use > >>> the builtin PSCI implementation. > >> > >> > >> So nobody is using upstream QEMU to validate and prototype > >> ATF/EL1s/EL0s code? That's a shame :). I suppose there is little > >> value without the bitstream emulation and R cluster. Do you have > >> plans to bring multi process emulation upstream some day to enable > >> these there? > > > > The R cluster is already in mainstream, isn't it? > > > In that case, wouldn't it make sense to build an emulation model of the > PMU behavior so that normal ATF works out of the box? > > > Thanks, > > Alex > Yes, that makes sense and there are several ways to implement it. To fully support the programmability of the PMU we'd need to model the MicroBlazes together with the ARM cores. But PMU support does not really conflict with this patch series, or is there something I'm missing? Cheers, Edgar
On 07.02.22 19:59, Philippe Mathieu-Daudé wrote: > On 7/2/22 19:13, Edgar E. Iglesias wrote: >> >> On Mon, Feb 7, 2022 at 5:24 PM Alexander Graf <agraf@csgraf.de >> <mailto:agraf@csgraf.de>> wrote: >> >> >> On 07.02.22 17:06, Philippe Mathieu-Daudé wrote: >> > On 7/2/22 16:59, Alexander Graf wrote: >> >> >> >> On 07.02.22 16:52, Edgar E. Iglesias wrote: >> > >> >>> Both Versal and ZynqMP require MicroBlaze firmware to run the >> >>> reference implementations of Trusted Firmware. We never >> supported >> >>> this in upstream QEMU but we do support it with our fork (by >> running >> >>> multiple QEMU instances co-simulating). >> >>> >> >>> Having said that, we do have tons of EL3 test-cases that we >> use to >> >>> validate QEMU that run with EL3 enabled in upstream. >> >>> >> >>> So there's two user flows: >> >>> 1. Direct boots using QEMUs builtin PSCI (Most users use this >> to run >> >>> Linux, Xen, U-boot, etc) >> >>> 2. Firmware boot at EL3 without QEMUs builtin PSCI (Mostly >> used by >> >>> test-code) >> >>> >> >>> Number #2 is the one affected here and that by accident used to >> have >> >>> the builtin PSCI support enabled but now requires more power >> control >> >>> modelling to keep working. >> >>> Unless I'm missing something, the -kernel boots will continue >> to use >> >>> the builtin PSCI implementation. >> >> >> >> >> >> So nobody is using upstream QEMU to validate and prototype >> >> ATF/EL1s/EL0s code? That's a shame :). I suppose there is little >> >> value without the bitstream emulation and R cluster. Do you have >> >> plans to bring multi process emulation upstream some day to >> enable >> >> these there? >> > >> > The R cluster is already in mainstream, isn't it? >> >> >> In that case, wouldn't it make sense to build an emulation model >> of the >> PMU behavior so that normal ATF works out of the box? >> >> >> Thanks, >> >> Alex >> >> >> Yes, that makes sense and there are several ways to implement it. To >> fully support the programmability of the PMU we'd need to model the >> MicroBlazes together with the ARM cores. >> >> But PMU support does not really conflict with this patch series, or >> is there something I'm missing? > > My understanding is Alex generically wonders about code coverage, not > about the ZynqMP in particular :) I'm more curious what the purpose of zynqmp / versal simulation in QEMU is. What we're saying here is that we only care about "Linux at EL2 and below" plus a Xilinx validation test suite. I understand how multi-QEMU emulation may be difficult, but EL3 simulation with Cortex-A plus Cortex-R clusters and a simulated PMU sounds like it would get you a very long way on simulation coverage. That said, Xilinx probably knows their user base the best, so if they decide that the ability to run TrustZone code is not something they believe their users need in QEMU, I'm definitely happy with that stance. Alex
diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c index 45eb19ab3b7..4c84bb932aa 100644 --- a/hw/arm/xlnx-zcu102.c +++ b/hw/arm/xlnx-zcu102.c @@ -236,6 +236,7 @@ static void xlnx_zcu102_init(MachineState *machine) s->binfo.ram_size = ram_size; s->binfo.loader_start = 0; s->binfo.modify_dtb = zcu102_modify_dtb; + s->binfo.psci_conduit = QEMU_PSCI_CONDUIT_SMC; arm_load_kernel(s->soc.boot_cpu_ptr, machine, &s->binfo); } diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c index 1c52a575aad..17305fe7b76 100644 --- a/hw/arm/xlnx-zynqmp.c +++ b/hw/arm/xlnx-zynqmp.c @@ -215,7 +215,10 @@ static void xlnx_zynqmp_create_rpu(MachineState *ms, XlnxZynqMPState *s, name = object_get_canonical_path_component(OBJECT(&s->rpu_cpu[i])); if (strcmp(name, boot_cpu)) { - /* Secondary CPUs start in PSCI powered-down state */ + /* + * Secondary CPUs start in powered-down state. + * TODO: check this is what EL3 firmware expects. + */ object_property_set_bool(OBJECT(&s->rpu_cpu[i]), "start-powered-off", true, &error_abort); } else { @@ -435,12 +438,12 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp) for (i = 0; i < num_apus; i++) { const char *name; - object_property_set_int(OBJECT(&s->apu_cpu[i]), "psci-conduit", - QEMU_PSCI_CONDUIT_SMC, &error_abort); - name = object_get_canonical_path_component(OBJECT(&s->apu_cpu[i])); if (strcmp(name, boot_cpu)) { - /* Secondary CPUs start in PSCI powered-down state */ + /* + * Secondary CPUs start in powered-down state. + * TODO: check this is what EL3 firmware expects. + */ object_property_set_bool(OBJECT(&s->apu_cpu[i]), "start-powered-off", true, &error_abort); } else {
Change the Xilinx ZynqMP-based board xlnx-zcu102 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. Note that this means that EL3 guest code will have no way to power on secondary cores, because we don't model any kind of power controller that does that on this SoC. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- Again, if anybody knows the real-hardware EL3 behaviour for CPUs that would be great. --- hw/arm/xlnx-zcu102.c | 1 + hw/arm/xlnx-zynqmp.c | 13 ++++++++----- 2 files changed, 9 insertions(+), 5 deletions(-)