Re: [Qemu-devel] [PATCH v2 11/15] target-arm: Enable CPU has_el3 prop during VE init

[Greg Bellows]

On 15 December 2014 at 11:06, Peter Maydell <address@hidden> wrote:

On 11 December 2014 at 23:29, Greg Bellows <address@hidden> wrote:
> Adds setting of the CPU has_el3 property based on the vexpress machine
> secure state property during initialization.  This enables/disables EL3
> state during start-up.  Changes include adding an additional secure state
> boolean during vexpress CPU initialization.  Also enables the ARM secure boot
> by default.
>
> Signed-off-by: Greg Bellows <address@hidden>
>
> ---
>
> 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 feautre to be disabled.
> - Add setting of arm_boot_info.secure_boot to true
> ---
>  hw/arm/vexpress.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
> index c82c32e..d3fb2bd 100644
> --- a/hw/arm/vexpress.c
> +++ b/hw/arm/vexpress.c
> @@ -196,7 +196,7 @@ struct VEDBoardInfo {
>  };
>
>  static void init_cpus(const char *cpu_model, const char *privdev,
> -                      hwaddr periphbase, qemu_irq *pic)
> +                      hwaddr periphbase, qemu_irq *pic, bool secure)
>  {
>      ObjectClass *cpu_oc = cpu_class_by_name(TYPE_ARM_CPU, cpu_model);
>      DeviceState *dev;
> @@ -213,6 +213,15 @@ static void init_cpus(const char *cpu_model, const char *privdev,
>          Object *cpuobj = object_new(object_class_get_name(cpu_oc));
>          Error *err = NULL;
>
> +        if (!secure) {
> +            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);
> +            }

This will break trying to use '-machine secure=off' with '-cpu host'.
We should just silently continue if the has_el3 property doesn't
exist on the CPU object.

Fixed

 

> +        }
> +
>          if (object_property_find(cpuobj, "reset-cbar", NULL)) {
>              object_property_set_int(cpuobj, periphbase,
>                                      "reset-cbar", &error_abort);
> @@ -288,7 +297,7 @@ static void a9_daughterboard_init(const VexpressMachineState *vms,
>      memory_region_add_subregion(sysmem, 0x60000000, ram);
>
>      /* 0x1e000000 A9MPCore (SCU) private memory region */
> -    init_cpus(cpu_model, "a9mpcore_priv", 0x1e000000, pic);
> +    init_cpus(cpu_model, "a9mpcore_priv", 0x1e000000, pic, vms->secure);
>
>      /* Daughterboard peripherals : 0x10020000 .. 0x20000000 */
>
> @@ -374,7 +383,7 @@ static void a15_daughterboard_init(const VexpressMachineState *vms,
>      memory_region_add_subregion(sysmem, 0x80000000, ram);
>
>      /* 0x2c000000 A15MPCore private memory region (GIC) */
> -    init_cpus(cpu_model, "a15mpcore_priv", 0x2c000000, pic);
> +    init_cpus(cpu_model, "a15mpcore_priv", 0x2c000000, pic, vms->secure);
>
>      /* A15 daughterboard peripherals: */
>
> @@ -699,6 +708,8 @@ static void vexpress_common_init(MachineState *machine)
>      daughterboard->bootinfo.smp_bootreg_addr = map[VE_SYSREGS] + 0x30;
>      daughterboard->bootinfo.gic_cpu_if_addr = daughterboard->gic_cpu_if_addr;
>      daughterboard->bootinfo.modify_dtb = vexpress_modify_dtb;
> +    /* Indicate that when booting Linux we should be in secure state */
> +    daughterboard->bootinfo.secure_boot = true;
>      arm_load_kernel(ARM_CPU(first_cpu), &daughterboard->bootinfo);
>  }

thanks
-- PMM