[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] hw/arm/virt: Assume EL3 boot rom will handle PS
From: |
Laszlo Ersek |
Subject: |
Re: [Qemu-devel] [PATCH] hw/arm/virt: Assume EL3 boot rom will handle PSCI if one is provided |
Date: |
Fri, 19 Feb 2016 22:03:44 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 |
Ard, any opinion on this?
Thanks
Laszlo
On 02/19/16 17:21, Peter Maydell wrote:
> If the user passes us an EL3 boot rom, then it is going to want to
> implement the PSCI interface itself. In this case, disable QEMU's
> internal PSCI implementation so it does not get in the way, and
> instead start all CPUs in an SMP configuration at once (the boot
> rom will catch them all and pen up the secondaries until needed).
> The boot rom code is also responsible for editing the device tree
> to include any necessary information about its own PSCI implementation
> before eventually passing it to a NonSecure guest.
>
> (This "start all CPUs at once" approach is what both ARM Trusted
> Firmware and UEFI expect, since it is what the ARM Foundation Model
> does; the other approach would be to provide some emulated hardware
> for "start the secondaries" but this is simplest.)
>
> This is a compatibility break, but I don't believe that anybody
> was using a secure boot ROM with an SMP configuration. Such a setup
> would be somewhat broken since there was nothing preventing nonsecure
> guest code from calling the QEMU PSCI function to start up a secondary
> core in a way that completely bypassed the secure world.
>
> Signed-off-by: Peter Maydell <address@hidden>
> ---
> This is slightly RFC-ish because I don't actually have any secure boot
> rom code that will cope with SMP right now. But I think that since this
> is a compat break we're better off putting it into 2.6 than not.
>
> hw/arm/virt.c | 33 ++++++++++++++++++++++++++-------
> 1 file changed, 26 insertions(+), 7 deletions(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 4499e2c..0f01902 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -73,6 +73,7 @@ typedef struct VirtBoardInfo {
> uint32_t clock_phandle;
> uint32_t gic_phandle;
> uint32_t v2m_phandle;
> + bool using_psci;
> } VirtBoardInfo;
>
> typedef struct {
> @@ -231,6 +232,10 @@ static void fdt_add_psci_node(const VirtBoardInfo *vbi)
> void *fdt = vbi->fdt;
> ARMCPU *armcpu = ARM_CPU(qemu_get_cpu(0));
>
> + if (!vbi->using_psci) {
> + return;
> + }
> +
> qemu_fdt_add_subnode(fdt, "/psci");
> if (armcpu->psci_version == 2) {
> const char comp[] = "arm,psci-0.2\0arm,psci";
> @@ -342,7 +347,7 @@ static void fdt_add_cpu_nodes(const VirtBoardInfo *vbi)
> qemu_fdt_setprop_string(vbi->fdt, nodename, "compatible",
> armcpu->dtb_compatible);
>
> - if (vbi->smp_cpus > 1) {
> + if (vbi->using_psci && vbi->smp_cpus > 1) {
> qemu_fdt_setprop_string(vbi->fdt, nodename,
> "enable-method", "psci");
> }
> @@ -1081,6 +1086,7 @@ static void machvirt_init(MachineState *machine)
> VirtGuestInfoState *guest_info_state = g_malloc0(sizeof
> *guest_info_state);
> VirtGuestInfo *guest_info = &guest_info_state->info;
> char **cpustr;
> + bool firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0);
>
> if (!cpu_model) {
> cpu_model = "cortex-a15";
> @@ -1146,6 +1152,16 @@ static void machvirt_init(MachineState *machine)
> memory_region_init(secure_sysmem, OBJECT(machine), "secure-memory",
> UINT64_MAX);
> memory_region_add_subregion_overlap(secure_sysmem, 0, sysmem, -1);
> +
> + if (firmware_loaded) {
> + /* If we have an EL3 boot ROM then the assumption is that it will
> + * implement PSCI itself, so disable QEMU's internal
> implementation
> + * so it doesn't get in the way. Instead of starting secondary
> + * CPUs in PSCI powerdown state we will start them all running
> and
> + * let the boot ROM sort them out.
> + */
> + vbi->using_psci = false;
> + }
> }
>
> create_fdt(vbi);
> @@ -1175,12 +1191,15 @@ static void machvirt_init(MachineState *machine)
> object_property_set_bool(cpuobj, false, "has_el3", NULL);
> }
>
> - object_property_set_int(cpuobj, QEMU_PSCI_CONDUIT_HVC,
> "psci-conduit",
> - NULL);
> + if (vbi->using_psci) {
> + object_property_set_int(cpuobj, QEMU_PSCI_CONDUIT_HVC,
> + "psci-conduit", NULL);
>
> - /* Secondary CPUs start in PSCI powered-down state */
> - if (n > 0) {
> - object_property_set_bool(cpuobj, true, "start-powered-off",
> NULL);
> + /* Secondary CPUs start in PSCI powered-down state */
> + if (n > 0) {
> + object_property_set_bool(cpuobj, true,
> + "start-powered-off", NULL);
> + }
> }
>
> if (object_property_find(cpuobj, "reset-cbar", NULL)) {
> @@ -1249,7 +1268,7 @@ static void machvirt_init(MachineState *machine)
> vbi->bootinfo.board_id = -1;
> vbi->bootinfo.loader_start = vbi->memmap[VIRT_MEM].base;
> vbi->bootinfo.get_dtb = machvirt_dtb;
> - vbi->bootinfo.firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0);
> + vbi->bootinfo.firmware_loaded = firmware_loaded;
> arm_load_kernel(ARM_CPU(first_cpu), &vbi->bootinfo);
>
> /*
>