On 19 February 2016 at 22:03, Laszlo Ersek <ler...@redhat.com> wrote: > Ard, any opinion on this? >
I agree with Peter. Note that this is strictly about emulation, under KVM we always run at EL1 or below and PSCI calls are handled by the host kernel, not QEMU > 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 <peter.mayd...@linaro.org> >> --- >> 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); >> >> /* >> >