Re: [Qemu-devel] [PATCH v2 12/15] target-arm: Set CPU has_el3 prop during virt init

[Peter Maydell <peter.maydell@linaro.org>]

On 11 December 2014 at 23:29, Greg Bellows <greg.bellows@linaro.org> wrote:
> Adds setting of the CPU has_el3 property based on the virt machine
> secure state property during initialization.  This enables/disables EL3
> state during start-up.  Changes include adding an additional secure state
> boolean during virt CPU initialization.  Also disables the ARM secure boot
> by default.
>
> Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
>
> ---
>
> v1 -> v2
> - Changes CPU property name from "secure" to "has_el3"
> - Change conditional to handle machine state default of secure.  The check
>   now checks if the machine secure property has been disabled which causes the
>   CPU EL3 feature to be disabled.
> - Add setting of arm_boot_info.secure_boot to false
> ---
>  hw/arm/virt.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 3eacc43..19d6174 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -547,6 +547,7 @@ static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)
>
>  static void machvirt_init(MachineState *machine)
>  {
> +    VirtMachineState *vms = VIRT_MACHINE(machine);
>      qemu_irq pic[NUM_IRQS];
>      MemoryRegion *sysmem = get_system_memory();
>      int n;
> @@ -584,6 +585,16 @@ static void machvirt_init(MachineState *machine)
>          }
>          cpuobj = object_new(object_class_get_name(oc));
>
> +        if (!vms->secure) {
> +            Error *err = NULL;
> +            object_property_set_bool(cpuobj, false, "has_el3", &err);
> +            if (err) {
> +                error_report("'secure' machine property not supported "
> +                             "with %s cpu", cpu_model);
> +                exit(1);
> +            }
> +        }

As with vexpress, we should silently continue if the CPU object
doesn't have has_el3.

> +
>          object_property_set_int(cpuobj, QEMU_PSCI_CONDUIT_HVC, "psci-conduit",
>                                  NULL);
>
> @@ -630,6 +641,8 @@ 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;
> +    /* Indicate that when booting Linux we should be in non-secure state */
> +    vbi->bootinfo.secure_boot = false;

You don't need to set this, false is the default.

>      arm_load_kernel(ARM_CPU(first_cpu), &vbi->bootinfo);
>  }

thanks
-- PMM