[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: |
Ard Biesheuvel |
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:41:04 +0100 |
On 19 February 2016 at 22:03, Laszlo Ersek <address@hidden> 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 <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);
>>
>> /*
>>
>