[Qemu-devel] [PATCH v2 0/6] ARM: enable TZ in the GIC

[Qemu-devel] [PATCH v2 0/6] ARM: enable TZ in the GIC

[Peter Maydell]

This patchset enables the TZ support in the GIC for the systems
where we enable TZ support in the CPU. In practice that means
just the "virt" and "vexpress" boards, since all the others
disable the CPU TZ support.

Andreas: I've cc'd you because of Peter C's patch to add
object_child_foreach_recursive(). None of the other patches
have anything particularly novel QOM-wise (since the GIC
property from v1 has gone away).

Changes since v1:
 * New patch which switches the default for the 'virt' board from
   "enable TZ" to "disable TZ". The UEFI blob can't handle TZ being
   fully enabled, so I had a choice of breaking it or breaking code
   which assumes TZ. As far as I know only the TZ test suite falls
   in the latter category. -machine secure=on will give you the
   old behaviour back.
 * rather than the property on the GIC, we take the approach Peter C
   suggested of defining an interface for devices to implement if
   they need to do firmware-equivalent setup. The API is a little
   different from Peter C's RFC patch, but the principle is the same.


The patches sit on top of the secure-timer patches I sent out earlier
today, which in turn sit on top of Edgar's hyp-timer patches. You
can find a git branch with everything at
https://git.linaro.org/people/peter.maydell/qemu-arm.git full-tz-enable


Peter Crosthwaite (1):
  qom: Add recursive version of object_child_for_each

Peter Maydell (5):
  hw/arm: new interface for devices which need to behave differently for
    kernel boot
  hw/intc/arm_gic_common: Configure IRQs as NS if doing direct NS kernel
    boot
  hw/cpu/{a15mpcore,a9mpcore}: enable TrustZone in GIC if it is enabled
    in CPUs
  hw/arm/virt: Default to not providing TrustZone support
  hw/arm/virt: Enable TZ extensions on the GIC if we are using them

 hw/arm/boot.c                    | 34 +++++++++++++++++++++++++++
 hw/arm/virt.c                    | 14 +++++++----
 hw/cpu/a15mpcore.c               | 13 ++++++++++
 hw/cpu/a9mpcore.c                | 11 +++++++++
 hw/intc/arm_gic_common.c         | 51 +++++++++++++++++++++++++++++++++++++---
 include/hw/arm/linux-boot-if.h   | 43 +++++++++++++++++++++++++++++++++
 include/hw/intc/arm_gic_common.h |  1 +
 include/qom/object.h             | 15 ++++++++++++
 qom/object.c                     | 25 +++++++++++++++++---
 9 files changed, 197 insertions(+), 10 deletions(-)
 create mode 100644 include/hw/arm/linux-boot-if.h

-- 
1.9.1

[Qemu-devel] [PATCH v2 1/6] qom: Add recursive version of object_child_for_each

[Peter Maydell]

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

Useful for iterating through an entire QOM subtree.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/qom/object.h | 15 +++++++++++++++
 qom/object.c         | 25 ++++++++++++++++++++++---
 2 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/include/qom/object.h b/include/qom/object.h
index 807978e..be7280c 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -1494,6 +1494,21 @@ int object_child_foreach(Object *obj, int (*fn)(Object *child, void *opaque),
                          void *opaque);
 
 /**
+ * object_child_foreach_recursive:
+ * @obj: the object whose children will be navigated
+ * @fn: the iterator function to be called
+ * @opaque: an opaque value that will be passed to the iterator
+ *
+ * Call @fn passing each child of @obj and @opaque to it, until @fn returns
+ * non-zero. Calls recursively, all child nodes of @obj will also be passed
+ * all the way down to the leaf nodes of the tree. Depth first ordering.
+ *
+ * Returns: The last value returned by @fn, or 0 if there is no child.
+ */
+int object_child_foreach_recursive(Object *obj,
+                                   int (*fn)(Object *child, void *opaque),
+                                   void *opaque);
+/**
  * container_get:
  * @root: root of the #path, e.g., object_get_root()
  * @path: path to the container
diff --git a/qom/object.c b/qom/object.c
index eea8edf..b7b05d3 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -775,23 +775,42 @@ void object_class_foreach(void (*fn)(ObjectClass *klass, void *opaque),
     enumerating_types = false;
 }
 
-int object_child_foreach(Object *obj, int (*fn)(Object *child, void *opaque),
-                         void *opaque)
+static int do_object_child_foreach(Object *obj,
+                                   int (*fn)(Object *child, void *opaque),
+                                   void *opaque, bool recurse)
 {
     ObjectProperty *prop, *next;
     int ret = 0;
 
     QTAILQ_FOREACH_SAFE(prop, &obj->properties, node, next) {
         if (object_property_is_child(prop)) {
-            ret = fn(prop->opaque, opaque);
+            Object *child = prop->opaque;
+
+            ret = fn(child, opaque);
             if (ret != 0) {
                 break;
             }
+            if (recurse) {
+                do_object_child_foreach(child, fn, opaque, true);
+            }
         }
     }
     return ret;
 }
 
+int object_child_foreach(Object *obj, int (*fn)(Object *child, void *opaque),
+                         void *opaque)
+{
+    return do_object_child_foreach(obj, fn, opaque, false);
+}
+
+int object_child_foreach_recursive(Object *obj,
+                                   int (*fn)(Object *child, void *opaque),
+                                   void *opaque)
+{
+    return do_object_child_foreach(obj, fn, opaque, true);
+}
+
 static void object_class_get_list_tramp(ObjectClass *klass, void *opaque)
 {
     GSList **list = opaque;
-- 
1.9.1

[Qemu-devel] [PATCH v2 2/6] hw/arm: new interface for devices which need to behave differently for kernel boot

[Peter Maydell]

For ARM we have a little minimalist bootloader in hw/arm/boot.c which
takes the place of firmware if we're directly booting a Linux kernel.
Unfortunately a few devices need special case handling in this situation
to do the initialization which on real hardware would be done by
firmware. (In particular if we're booting a kernel in NonSecure state
then we need to make a TZ-aware GIC put all its interrupts into Group 1,
or the guest will be unable to use them.)

Create a new QOM interface which can be implemented by devices which
need to do something different from their default reset behaviour.
The callback will be called after machine initialization and before
first reset.

Suggested-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/boot.c                  | 34 +++++++++++++++++++++++++++++++++
 include/hw/arm/linux-boot-if.h | 43 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 77 insertions(+)
 create mode 100644 include/hw/arm/linux-boot-if.h

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 5b969cd..4bac6dc 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -10,6 +10,7 @@
 #include "config.h"
 #include "hw/hw.h"
 #include "hw/arm/arm.h"
+#include "hw/arm/linux-boot-if.h"
 #include "sysemu/sysemu.h"
 #include "hw/boards.h"
 #include "hw/loader.h"
@@ -555,6 +556,20 @@ static void load_image_to_fw_cfg(FWCfgState *fw_cfg, uint16_t size_key,
     fw_cfg_add_bytes(fw_cfg, data_key, data, size);
 }
 
+static int do_arm_linux_init(Object *obj, void *opaque)
+{
+    if (object_dynamic_cast(obj, TYPE_ARM_LINUX_BOOT_IF)) {
+        ARMLinuxBootIf *albif = ARM_LINUX_BOOT_IF(obj);
+        ARMLinuxBootIfClass *albifc = ARM_LINUX_BOOT_IF_GET_CLASS(obj);
+        struct arm_boot_info *info = opaque;
+
+        if (albifc->arm_linux_init) {
+            albifc->arm_linux_init(albif, info->secure_boot);
+        }
+    }
+    return 0;
+}
+
 static void arm_load_kernel_notify(Notifier *notifier, void *data)
 {
     CPUState *cs;
@@ -778,6 +793,12 @@ static void arm_load_kernel_notify(Notifier *notifier, void *data)
         if (info->nb_cpus > 1) {
             info->write_secondary_boot(cpu, info);
         }
+
+        /* Notify devices which need to fake up firmware initialization
+         * that we'ro doing a direct kernel boot.
+         */
+        object_child_foreach_recursive(object_get_root(),
+                                       do_arm_linux_init, info);
     }
     info->is_linux = is_linux;
 
@@ -803,3 +824,16 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
         qemu_register_reset(do_cpu_reset, ARM_CPU(cs));
     }
 }
+
+static const TypeInfo arm_linux_boot_if_info = {
+    .name = TYPE_ARM_LINUX_BOOT_IF,
+    .parent = TYPE_INTERFACE,
+    .class_size = sizeof(ARMLinuxBootIfClass),
+};
+
+static void arm_linux_boot_register_types(void)
+{
+    type_register_static(&arm_linux_boot_if_info);
+}
+
+type_init(arm_linux_boot_register_types)
diff --git a/include/hw/arm/linux-boot-if.h b/include/hw/arm/linux-boot-if.h
new file mode 100644
index 0000000..aba4479
--- /dev/null
+++ b/include/hw/arm/linux-boot-if.h
@@ -0,0 +1,43 @@
+/*
+ * hw/arm/linux-boot-if.h : interface for devices which need to behave
+ * specially for direct boot of an ARM Linux kernel
+ */
+
+#ifndef HW_ARM_LINUX_BOOT_IF_H
+#define HW_ARM_LINUX_BOOT_IF_H
+
+#include "qom/object.h"
+
+#define TYPE_ARM_LINUX_BOOT_IF "arm-linux-boot-if"
+#define ARM_LINUX_BOOT_IF_CLASS(klass) \
+    OBJECT_CLASS_CHECK(ARMLinuxBootIfClass, (klass), TYPE_ARM_LINUX_BOOT_IF)
+#define ARM_LINUX_BOOT_IF_GET_CLASS(obj) \
+    OBJECT_GET_CLASS(ARMLinuxBootIfClass, (obj), TYPE_ARM_LINUX_BOOT_IF)
+#define ARM_LINUX_BOOT_IF(obj) \
+    INTERFACE_CHECK(ARMLinuxBootIf, (obj), TYPE_ARM_LINUX_BOOT_IF)
+
+typedef struct ARMLinuxBootIf {
+    /*< private >*/
+    Object parent_obj;
+} ARMLinuxBootIf;
+
+typedef struct ARMLinuxBootIfClass {
+    /*< private >*/
+    InterfaceClass parent_class;
+
+    /*< public >*/
+    /** arm_linux_init: configure the device for a direct boot
+     * of an ARM Linux kernel (so that device reset puts it into
+     * the state the kernel expects after firmware initialization,
+     * rather than the true hardware reset state). This callback is
+     * called once after machine construction is complete (before the
+     * first system reset).
+     *
+     * @obj: the object implementing this interface
+     * @secure_boot: true if we are booting Secure, false for NonSecure
+     * (or for a CPU which doesn't support TrustZone)
+     */
+    void (*arm_linux_init)(ARMLinuxBootIf *obj, bool secure_boot);
+} ARMLinuxBootIfClass;
+
+#endif
-- 
1.9.1

Re: [Qemu-devel] [PATCH v2 2/6] hw/arm: new interface for devices which need to behave differently for kernel boot

[Peter Crosthwaite]

On Thu, Jul 16, 2015 at 1:11 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> For ARM we have a little minimalist bootloader in hw/arm/boot.c which
> takes the place of firmware if we're directly booting a Linux kernel.
> Unfortunately a few devices need special case handling in this situation
> to do the initialization which on real hardware would be done by
> firmware. (In particular if we're booting a kernel in NonSecure state
> then we need to make a TZ-aware GIC put all its interrupts into Group 1,
> or the guest will be unable to use them.)
>
> Create a new QOM interface which can be implemented by devices which
> need to do something different from their default reset behaviour.
> The callback will be called after machine initialization and before
> first reset.
>
> Suggested-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/arm/boot.c                  | 34 +++++++++++++++++++++++++++++++++
>  include/hw/arm/linux-boot-if.h | 43 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 77 insertions(+)
>  create mode 100644 include/hw/arm/linux-boot-if.h
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index 5b969cd..4bac6dc 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -10,6 +10,7 @@
>  #include "config.h"
>  #include "hw/hw.h"
>  #include "hw/arm/arm.h"
> +#include "hw/arm/linux-boot-if.h"
>  #include "sysemu/sysemu.h"
>  #include "hw/boards.h"
>  #include "hw/loader.h"
> @@ -555,6 +556,20 @@ static void load_image_to_fw_cfg(FWCfgState *fw_cfg, uint16_t size_key,
>      fw_cfg_add_bytes(fw_cfg, data_key, data, size);
>  }
>
> +static int do_arm_linux_init(Object *obj, void *opaque)
> +{
> +    if (object_dynamic_cast(obj, TYPE_ARM_LINUX_BOOT_IF)) {
> +        ARMLinuxBootIf *albif = ARM_LINUX_BOOT_IF(obj);
> +        ARMLinuxBootIfClass *albifc = ARM_LINUX_BOOT_IF_GET_CLASS(obj);
> +        struct arm_boot_info *info = opaque;
> +
> +        if (albifc->arm_linux_init) {
> +            albifc->arm_linux_init(albif, info->secure_boot);
> +        }
> +    }
> +    return 0;
> +}
> +
>  static void arm_load_kernel_notify(Notifier *notifier, void *data)
>  {
>      CPUState *cs;
> @@ -778,6 +793,12 @@ static void arm_load_kernel_notify(Notifier *notifier, void *data)
>          if (info->nb_cpus > 1) {
>              info->write_secondary_boot(cpu, info);
>          }
> +
> +        /* Notify devices which need to fake up firmware initialization
> +         * that we'ro doing a direct kernel boot.
> +         */

"we're"

> +        object_child_foreach_recursive(object_get_root(),
> +                                       do_arm_linux_init, info);
>      }
>      info->is_linux = is_linux;
>
> @@ -803,3 +824,16 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
>          qemu_register_reset(do_cpu_reset, ARM_CPU(cs));
>      }
>  }
> +
> +static const TypeInfo arm_linux_boot_if_info = {
> +    .name = TYPE_ARM_LINUX_BOOT_IF,
> +    .parent = TYPE_INTERFACE,
> +    .class_size = sizeof(ARMLinuxBootIfClass),
> +};
> +
> +static void arm_linux_boot_register_types(void)
> +{
> +    type_register_static(&arm_linux_boot_if_info);
> +}
> +
> +type_init(arm_linux_boot_register_types)
> diff --git a/include/hw/arm/linux-boot-if.h b/include/hw/arm/linux-boot-if.h
> new file mode 100644
> index 0000000..aba4479
> --- /dev/null
> +++ b/include/hw/arm/linux-boot-if.h
> @@ -0,0 +1,43 @@
> +/*
> + * hw/arm/linux-boot-if.h : interface for devices which need to behave
> + * specially for direct boot of an ARM Linux kernel
> + */
> +
> +#ifndef HW_ARM_LINUX_BOOT_IF_H
> +#define HW_ARM_LINUX_BOOT_IF_H
> +
> +#include "qom/object.h"
> +
> +#define TYPE_ARM_LINUX_BOOT_IF "arm-linux-boot-if"
> +#define ARM_LINUX_BOOT_IF_CLASS(klass) \
> +    OBJECT_CLASS_CHECK(ARMLinuxBootIfClass, (klass), TYPE_ARM_LINUX_BOOT_IF)
> +#define ARM_LINUX_BOOT_IF_GET_CLASS(obj) \
> +    OBJECT_GET_CLASS(ARMLinuxBootIfClass, (obj), TYPE_ARM_LINUX_BOOT_IF)
> +#define ARM_LINUX_BOOT_IF(obj) \
> +    INTERFACE_CHECK(ARMLinuxBootIf, (obj), TYPE_ARM_LINUX_BOOT_IF)
> +
> +typedef struct ARMLinuxBootIf {
> +    /*< private >*/
> +    Object parent_obj;
> +} ARMLinuxBootIf;
> +
> +typedef struct ARMLinuxBootIfClass {
> +    /*< private >*/
> +    InterfaceClass parent_class;
> +
> +    /*< public >*/
> +    /** arm_linux_init: configure the device for a direct boot
> +     * of an ARM Linux kernel (so that device reset puts it into
> +     * the state the kernel expects after firmware initialization,
> +     * rather than the true hardware reset state). This callback is
> +     * called once after machine construction is complete (before the
> +     * first system reset).
> +     *
> +     * @obj: the object implementing this interface
> +     * @secure_boot: true if we are booting Secure, false for NonSecure
> +     * (or for a CPU which doesn't support TrustZone)
> +     */
> +    void (*arm_linux_init)(ARMLinuxBootIf *obj, bool secure_boot);

Can we drop the "arm_"? ARM is always going to be there in the context
as it is in the typename due to ARMLinuxBootIfClass.

So If we are going for an ARM-specific thing, it might make sense to
instead drop all the _linux_ stuff and have this call unconditional.
Then the API has wider application than just Linux boots. The struct
arm_boot_info can be made more widely visible as the one data argument
the device accepts, from which security state as well and is_linux can
be fished.

void (*fw_init)(ARMLinuxBootif *obj, struct arm_boot_info info);

Regards,
Peter

> +} ARMLinuxBootIfClass;
> +
> +#endif
> --
> 1.9.1
>
>

Re: [Qemu-devel] [PATCH v2 2/6] hw/arm: new interface for devices which need to behave differently for kernel boot

[Peter Maydell]

On 18 July 2015 at 04:55, Peter Crosthwaite
<peter.crosthwaite@petalogix.com> wrote:
> On Thu, Jul 16, 2015 at 1:11 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> For ARM we have a little minimalist bootloader in hw/arm/boot.c which
>> takes the place of firmware if we're directly booting a Linux kernel.
>> Unfortunately a few devices need special case handling in this situation
>> to do the initialization which on real hardware would be done by
>> firmware. (In particular if we're booting a kernel in NonSecure state
>> then we need to make a TZ-aware GIC put all its interrupts into Group 1,
>> or the guest will be unable to use them.)
>>
>> Create a new QOM interface which can be implemented by devices which
>> need to do something different from their default reset behaviour.
>> The callback will be called after machine initialization and before
>> first reset.
>>
>> Suggested-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>>  hw/arm/boot.c                  | 34 +++++++++++++++++++++++++++++++++
>>  include/hw/arm/linux-boot-if.h | 43 ++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 77 insertions(+)
>>  create mode 100644 include/hw/arm/linux-boot-if.h
>>
>> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
>> index 5b969cd..4bac6dc 100644
>> --- a/hw/arm/boot.c
>> +++ b/hw/arm/boot.c
>> @@ -10,6 +10,7 @@
>>  #include "config.h"
>>  #include "hw/hw.h"
>>  #include "hw/arm/arm.h"
>> +#include "hw/arm/linux-boot-if.h"
>>  #include "sysemu/sysemu.h"
>>  #include "hw/boards.h"
>>  #include "hw/loader.h"
>> @@ -555,6 +556,20 @@ static void load_image_to_fw_cfg(FWCfgState *fw_cfg, uint16_t size_key,
>>      fw_cfg_add_bytes(fw_cfg, data_key, data, size);
>>  }
>>
>> +static int do_arm_linux_init(Object *obj, void *opaque)
>> +{
>> +    if (object_dynamic_cast(obj, TYPE_ARM_LINUX_BOOT_IF)) {
>> +        ARMLinuxBootIf *albif = ARM_LINUX_BOOT_IF(obj);
>> +        ARMLinuxBootIfClass *albifc = ARM_LINUX_BOOT_IF_GET_CLASS(obj);
>> +        struct arm_boot_info *info = opaque;
>> +
>> +        if (albifc->arm_linux_init) {
>> +            albifc->arm_linux_init(albif, info->secure_boot);
>> +        }
>> +    }
>> +    return 0;
>> +}
>> +
>>  static void arm_load_kernel_notify(Notifier *notifier, void *data)
>>  {
>>      CPUState *cs;
>> @@ -778,6 +793,12 @@ static void arm_load_kernel_notify(Notifier *notifier, void *data)
>>          if (info->nb_cpus > 1) {
>>              info->write_secondary_boot(cpu, info);
>>          }
>> +
>> +        /* Notify devices which need to fake up firmware initialization
>> +         * that we'ro doing a direct kernel boot.
>> +         */
>
> "we're"
>
>> +        object_child_foreach_recursive(object_get_root(),
>> +                                       do_arm_linux_init, info);
>>      }
>>      info->is_linux = is_linux;
>>
>> @@ -803,3 +824,16 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
>>          qemu_register_reset(do_cpu_reset, ARM_CPU(cs));
>>      }
>>  }
>> +
>> +static const TypeInfo arm_linux_boot_if_info = {
>> +    .name = TYPE_ARM_LINUX_BOOT_IF,
>> +    .parent = TYPE_INTERFACE,
>> +    .class_size = sizeof(ARMLinuxBootIfClass),
>> +};
>> +
>> +static void arm_linux_boot_register_types(void)
>> +{
>> +    type_register_static(&arm_linux_boot_if_info);
>> +}
>> +
>> +type_init(arm_linux_boot_register_types)
>> diff --git a/include/hw/arm/linux-boot-if.h b/include/hw/arm/linux-boot-if.h
>> new file mode 100644
>> index 0000000..aba4479
>> --- /dev/null
>> +++ b/include/hw/arm/linux-boot-if.h
>> @@ -0,0 +1,43 @@
>> +/*
>> + * hw/arm/linux-boot-if.h : interface for devices which need to behave
>> + * specially for direct boot of an ARM Linux kernel
>> + */
>> +
>> +#ifndef HW_ARM_LINUX_BOOT_IF_H
>> +#define HW_ARM_LINUX_BOOT_IF_H
>> +
>> +#include "qom/object.h"
>> +
>> +#define TYPE_ARM_LINUX_BOOT_IF "arm-linux-boot-if"
>> +#define ARM_LINUX_BOOT_IF_CLASS(klass) \
>> +    OBJECT_CLASS_CHECK(ARMLinuxBootIfClass, (klass), TYPE_ARM_LINUX_BOOT_IF)
>> +#define ARM_LINUX_BOOT_IF_GET_CLASS(obj) \
>> +    OBJECT_GET_CLASS(ARMLinuxBootIfClass, (obj), TYPE_ARM_LINUX_BOOT_IF)
>> +#define ARM_LINUX_BOOT_IF(obj) \
>> +    INTERFACE_CHECK(ARMLinuxBootIf, (obj), TYPE_ARM_LINUX_BOOT_IF)
>> +
>> +typedef struct ARMLinuxBootIf {
>> +    /*< private >*/
>> +    Object parent_obj;
>> +} ARMLinuxBootIf;
>> +
>> +typedef struct ARMLinuxBootIfClass {
>> +    /*< private >*/
>> +    InterfaceClass parent_class;
>> +
>> +    /*< public >*/
>> +    /** arm_linux_init: configure the device for a direct boot
>> +     * of an ARM Linux kernel (so that device reset puts it into
>> +     * the state the kernel expects after firmware initialization,
>> +     * rather than the true hardware reset state). This callback is
>> +     * called once after machine construction is complete (before the
>> +     * first system reset).
>> +     *
>> +     * @obj: the object implementing this interface
>> +     * @secure_boot: true if we are booting Secure, false for NonSecure
>> +     * (or for a CPU which doesn't support TrustZone)
>> +     */
>> +    void (*arm_linux_init)(ARMLinuxBootIf *obj, bool secure_boot);
>
> Can we drop the "arm_"? ARM is always going to be there in the context
> as it is in the typename due to ARMLinuxBootIfClass.

Yeah, I guess so. I wasn't really sure what the best method
name here was.

> So If we are going for an ARM-specific thing, it might make sense to
> instead drop all the _linux_ stuff and have this call unconditional.
> Then the API has wider application than just Linux boots. The struct
> arm_boot_info can be made more widely visible as the one data argument
> the device accepts, from which security state as well and is_linux can
> be fished.

I was going to pass arm_boot_info in, but that struct requires
the target cpu.h so it can't be used in compiled-once objects
like the GIC code. Hence the single bool parameter.

I'm also not too keen on increasing the set of things we try
to handle in boot. Currently we do:
 * "firmware", ie the guest code gets to do all setup that it needs,
   just as on hardware
 * "linux kernel", where we provide the more-or-less documented
   boot environment etc for Linux kernels in particular

I think you're implying that we want to support a third thing here?

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 2/6] hw/arm: new interface for devices which need to behave differently for kernel boot

[Peter Maydell]

On 18 July 2015 at 10:00, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 18 July 2015 at 04:55, Peter Crosthwaite
> <peter.crosthwaite@petalogix.com> wrote:
>> On Thu, Jul 16, 2015 at 1:11 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> For ARM we have a little minimalist bootloader in hw/arm/boot.c which
>>> takes the place of firmware if we're directly booting a Linux kernel.
>>> Unfortunately a few devices need special case handling in this situation
>>> to do the initialization which on real hardware would be done by
>>> firmware. (In particular if we're booting a kernel in NonSecure state
>>> then we need to make a TZ-aware GIC put all its interrupts into Group 1,
>>> or the guest will be unable to use them.)
>>>
>>> Create a new QOM interface which can be implemented by devices which
>>> need to do something different from their default reset behaviour.
>>> The callback will be called after machine initialization and before
>>> first reset.
>>>
>>> Suggested-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

>>> +    /** arm_linux_init: configure the device for a direct boot
>>> +     * of an ARM Linux kernel (so that device reset puts it into
>>> +     * the state the kernel expects after firmware initialization,
>>> +     * rather than the true hardware reset state). This callback is
>>> +     * called once after machine construction is complete (before the
>>> +     * first system reset).
>>> +     *
>>> +     * @obj: the object implementing this interface
>>> +     * @secure_boot: true if we are booting Secure, false for NonSecure
>>> +     * (or for a CPU which doesn't support TrustZone)
>>> +     */
>>> +    void (*arm_linux_init)(ARMLinuxBootIf *obj, bool secure_boot);
>>
>> Can we drop the "arm_"? ARM is always going to be there in the context
>> as it is in the typename due to ARMLinuxBootIfClass.
>
> Yeah, I guess so. I wasn't really sure what the best method
> name here was.
>
>> So If we are going for an ARM-specific thing, it might make sense to
>> instead drop all the _linux_ stuff and have this call unconditional.
>> Then the API has wider application than just Linux boots. The struct
>> arm_boot_info can be made more widely visible as the one data argument
>> the device accepts, from which security state as well and is_linux can
>> be fished.
>
> I was going to pass arm_boot_info in, but that struct requires
> the target cpu.h so it can't be used in compiled-once objects
> like the GIC code. Hence the single bool parameter.
>
> I'm also not too keen on increasing the set of things we try
> to handle in boot. Currently we do:
>  * "firmware", ie the guest code gets to do all setup that it needs,
>    just as on hardware
>  * "linux kernel", where we provide the more-or-less documented
>    boot environment etc for Linux kernels in particular
>
> I think you're implying that we want to support a third thing here?

Any further comment on this? As I said, I'm unconvinced about
making this method more general than we really need. The other
patches have been reviewed so consensus on this API is I think
the only blocker.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 2/6] hw/arm: new interface for devices which need to behave differently for kernel boot

[Peter Maydell]

[oops, forgot to update Peter C's email address in the From line;
apologies to everybody else for the duplicate mail.]

On 18 July 2015 at 10:00, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 18 July 2015 at 04:55, Peter Crosthwaite
> <peter.crosthwaite@petalogix.com> wrote:
>> On Thu, Jul 16, 2015 at 1:11 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> For ARM we have a little minimalist bootloader in hw/arm/boot.c which
>>> takes the place of firmware if we're directly booting a Linux kernel.
>>> Unfortunately a few devices need special case handling in this situation
>>> to do the initialization which on real hardware would be done by
>>> firmware. (In particular if we're booting a kernel in NonSecure state
>>> then we need to make a TZ-aware GIC put all its interrupts into Group 1,
>>> or the guest will be unable to use them.)
>>>
>>> Create a new QOM interface which can be implemented by devices which
>>> need to do something different from their default reset behaviour.
>>> The callback will be called after machine initialization and before
>>> first reset.

>>> +typedef struct ARMLinuxBootIfClass {
>>> +    /*< private >*/
>>> +    InterfaceClass parent_class;
>>> +
>>> +    /*< public >*/
>>> +    /** arm_linux_init: configure the device for a direct boot
>>> +     * of an ARM Linux kernel (so that device reset puts it into
>>> +     * the state the kernel expects after firmware initialization,
>>> +     * rather than the true hardware reset state). This callback is
>>> +     * called once after machine construction is complete (before the
>>> +     * first system reset).
>>> +     *
>>> +     * @obj: the object implementing this interface
>>> +     * @secure_boot: true if we are booting Secure, false for NonSecure
>>> +     * (or for a CPU which doesn't support TrustZone)
>>> +     */
>>> +    void (*arm_linux_init)(ARMLinuxBootIf *obj, bool secure_boot);
>>
>> Can we drop the "arm_"? ARM is always going to be there in the context
>> as it is in the typename due to ARMLinuxBootIfClass.
>
> Yeah, I guess so. I wasn't really sure what the best method
> name here was.
>
>> So If we are going for an ARM-specific thing, it might make sense to
>> instead drop all the _linux_ stuff and have this call unconditional.
>> Then the API has wider application than just Linux boots. The struct
>> arm_boot_info can be made more widely visible as the one data argument
>> the device accepts, from which security state as well and is_linux can
>> be fished.
>
> I was going to pass arm_boot_info in, but that struct requires
> the target cpu.h so it can't be used in compiled-once objects
> like the GIC code. Hence the single bool parameter.
>
> I'm also not too keen on increasing the set of things we try
> to handle in boot. Currently we do:
>  * "firmware", ie the guest code gets to do all setup that it needs,
>    just as on hardware
>  * "linux kernel", where we provide the more-or-less documented
>    boot environment etc for Linux kernels in particular
>
> I think you're implying that we want to support a third thing here?

Any further comment on this? As I said, I'm unconvinced about
making this method more general than we really need. The other
patches have been reviewed so consensus on this API is I think
the only blocker.

thanks
-- PMM

[Qemu-devel] [PATCH v2 3/6] hw/intc/arm_gic_common: Configure IRQs as NS if doing direct NS kernel boot

[Peter Maydell]

If we directly boot a kernel in NonSecure on a system where the GIC
supports the security extensions then we must cause the GIC to
configure its interrupts into group 1 (NonSecure) rather than the
usual group 0, and with their initial priority set to the highest
NonSecure priority rather than the usual highest Secure priority.
Otherwise the guest kernel will be unable to use any interrupts.

Implement this behaviour, controlled by a flag which we set if
appropriate when the ARM bootloader code calls our ARMLinuxBootIf
interface callback.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/intc/arm_gic_common.c         | 51 +++++++++++++++++++++++++++++++++++++---
 include/hw/intc/arm_gic_common.h |  1 +
 2 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/hw/intc/arm_gic_common.c b/hw/intc/arm_gic_common.c
index a64d071..ae7c74e 100644
--- a/hw/intc/arm_gic_common.c
+++ b/hw/intc/arm_gic_common.c
@@ -19,6 +19,7 @@
  */
 
 #include "gic_internal.h"
+#include "hw/arm/linux-boot-if.h"
 
 static void gic_pre_save(void *opaque)
 {
@@ -124,12 +125,27 @@ static void arm_gic_common_reset(DeviceState *dev)
 {
     GICState *s = ARM_GIC_COMMON(dev);
     int i, j;
+    int resetprio;
+
+    /* If we're resetting a TZ-aware GIC as if secure firmware
+     * had set it up ready to start a kernel in non-secure,
+     * we need to set interrupt priorities to a "zero for the
+     * NS view" value. This is particularly critical for the
+     * priority_mask[] values, because if they are zero then NS
+     * code cannot ever rewrite the priority to anything else.
+     */
+    if (s->security_extn && s->irq_reset_nonsecure) {
+        resetprio = 0x80;
+    } else {
+        resetprio = 0;
+    }
+
     memset(s->irq_state, 0, GIC_MAXIRQ * sizeof(gic_irq_state));
     for (i = 0 ; i < s->num_cpu; i++) {
         if (s->revision == REV_11MPCORE) {
             s->priority_mask[i] = 0xf0;
         } else {
-            s->priority_mask[i] = 0;
+            s->priority_mask[i] = resetprio;
         }
         s->current_pending[i] = 1023;
         s->running_irq[i] = 1023;
@@ -138,7 +154,7 @@ static void arm_gic_common_reset(DeviceState *dev)
         s->bpr[i] = GIC_MIN_BPR;
         s->abpr[i] = GIC_MIN_ABPR;
         for (j = 0; j < GIC_INTERNAL; j++) {
-            s->priority1[j][i] = 0;
+            s->priority1[j][i] = resetprio;
         }
         for (j = 0; j < GIC_NR_SGIS; j++) {
             s->sgi_pending[j][i] = 0;
@@ -150,7 +166,7 @@ static void arm_gic_common_reset(DeviceState *dev)
     }
 
     for (i = 0; i < ARRAY_SIZE(s->priority2); i++) {
-        s->priority2[i] = 0;
+        s->priority2[i] = resetprio;
     }
 
     for (i = 0; i < GIC_MAXIRQ; i++) {
@@ -161,9 +177,32 @@ static void arm_gic_common_reset(DeviceState *dev)
             s->irq_target[i] = 0;
         }
     }
+    if (s->security_extn && s->irq_reset_nonsecure) {
+        for (i = 0; i < GIC_MAXIRQ; i++) {
+            GIC_SET_GROUP(i, ALL_CPU_MASK);
+        }
+    }
+
     s->ctlr = 0;
 }
 
+static void arm_gic_common_linux_init(ARMLinuxBootIf *obj,
+                                      bool secure_boot)
+{
+    GICState *s = ARM_GIC_COMMON(obj);
+
+    if (s->security_extn && !secure_boot) {
+        /* We're directly booting a kernel into NonSecure. If this GIC
+         * implements the security extensions then we must configure it
+         * to have all the interrupts be NonSecure (this is a job that
+         * is done by the Secure boot firmware in real hardware, and in
+         * this mode QEMU is acting as a minimalist firmware-and-bootloader
+         * equivalent).
+         */
+        s->irq_reset_nonsecure = true;
+    }
+}
+
 static Property arm_gic_common_properties[] = {
     DEFINE_PROP_UINT32("num-cpu", GICState, num_cpu, 1),
     DEFINE_PROP_UINT32("num-irq", GICState, num_irq, 32),
@@ -180,11 +219,13 @@ static Property arm_gic_common_properties[] = {
 static void arm_gic_common_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
+    ARMLinuxBootIfClass *albifc = ARM_LINUX_BOOT_IF_CLASS(klass);
 
     dc->reset = arm_gic_common_reset;
     dc->realize = arm_gic_common_realize;
     dc->props = arm_gic_common_properties;
     dc->vmsd = &vmstate_gic;
+    albifc->arm_linux_init = arm_gic_common_linux_init;
 }
 
 static const TypeInfo arm_gic_common_type = {
@@ -194,6 +235,10 @@ static const TypeInfo arm_gic_common_type = {
     .class_size = sizeof(ARMGICCommonClass),
     .class_init = arm_gic_common_class_init,
     .abstract = true,
+    .interfaces = (InterfaceInfo []) {
+        { TYPE_ARM_LINUX_BOOT_IF },
+        { },
+    },
 };
 
 static void register_types(void)
diff --git a/include/hw/intc/arm_gic_common.h b/include/hw/intc/arm_gic_common.h
index 899db3d..cfc1cce 100644
--- a/include/hw/intc/arm_gic_common.h
+++ b/include/hw/intc/arm_gic_common.h
@@ -118,6 +118,7 @@ typedef struct GICState {
     uint32_t num_irq;
     uint32_t revision;
     bool security_extn;
+    bool irq_reset_nonsecure; /* configure IRQs as group 1 (NS) on reset? */
     int dev_fd; /* kvm device fd if backed by kvm vgic support */
 } GICState;
 
-- 
1.9.1

Re: [Qemu-devel] [PATCH v2 3/6] hw/intc/arm_gic_common: Configure IRQs as NS if doing direct NS kernel boot

[Peter Crosthwaite]

On Thu, Jul 16, 2015 at 1:11 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> If we directly boot a kernel in NonSecure on a system where the GIC
> supports the security extensions then we must cause the GIC to
> configure its interrupts into group 1 (NonSecure) rather than the
> usual group 0, and with their initial priority set to the highest
> NonSecure priority rather than the usual highest Secure priority.
> Otherwise the guest kernel will be unable to use any interrupts.
>
> Implement this behaviour, controlled by a flag which we set if
> appropriate when the ARM bootloader code calls our ARMLinuxBootIf
> interface callback.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>

> ---
>  hw/intc/arm_gic_common.c         | 51 +++++++++++++++++++++++++++++++++++++---
>  include/hw/intc/arm_gic_common.h |  1 +
>  2 files changed, 49 insertions(+), 3 deletions(-)
>
> diff --git a/hw/intc/arm_gic_common.c b/hw/intc/arm_gic_common.c
> index a64d071..ae7c74e 100644
> --- a/hw/intc/arm_gic_common.c
> +++ b/hw/intc/arm_gic_common.c
> @@ -19,6 +19,7 @@
>   */
>
>  #include "gic_internal.h"
> +#include "hw/arm/linux-boot-if.h"
>
>  static void gic_pre_save(void *opaque)
>  {
> @@ -124,12 +125,27 @@ static void arm_gic_common_reset(DeviceState *dev)
>  {
>      GICState *s = ARM_GIC_COMMON(dev);
>      int i, j;
> +    int resetprio;
> +
> +    /* If we're resetting a TZ-aware GIC as if secure firmware
> +     * had set it up ready to start a kernel in non-secure,
> +     * we need to set interrupt priorities to a "zero for the
> +     * NS view" value. This is particularly critical for the
> +     * priority_mask[] values, because if they are zero then NS
> +     * code cannot ever rewrite the priority to anything else.
> +     */
> +    if (s->security_extn && s->irq_reset_nonsecure) {
> +        resetprio = 0x80;
> +    } else {
> +        resetprio = 0;
> +    }
> +
>      memset(s->irq_state, 0, GIC_MAXIRQ * sizeof(gic_irq_state));
>      for (i = 0 ; i < s->num_cpu; i++) {
>          if (s->revision == REV_11MPCORE) {
>              s->priority_mask[i] = 0xf0;
>          } else {
> -            s->priority_mask[i] = 0;
> +            s->priority_mask[i] = resetprio;
>          }
>          s->current_pending[i] = 1023;
>          s->running_irq[i] = 1023;
> @@ -138,7 +154,7 @@ static void arm_gic_common_reset(DeviceState *dev)
>          s->bpr[i] = GIC_MIN_BPR;
>          s->abpr[i] = GIC_MIN_ABPR;
>          for (j = 0; j < GIC_INTERNAL; j++) {
> -            s->priority1[j][i] = 0;
> +            s->priority1[j][i] = resetprio;
>          }
>          for (j = 0; j < GIC_NR_SGIS; j++) {
>              s->sgi_pending[j][i] = 0;
> @@ -150,7 +166,7 @@ static void arm_gic_common_reset(DeviceState *dev)
>      }
>
>      for (i = 0; i < ARRAY_SIZE(s->priority2); i++) {
> -        s->priority2[i] = 0;
> +        s->priority2[i] = resetprio;
>      }
>
>      for (i = 0; i < GIC_MAXIRQ; i++) {
> @@ -161,9 +177,32 @@ static void arm_gic_common_reset(DeviceState *dev)
>              s->irq_target[i] = 0;
>          }
>      }
> +    if (s->security_extn && s->irq_reset_nonsecure) {
> +        for (i = 0; i < GIC_MAXIRQ; i++) {
> +            GIC_SET_GROUP(i, ALL_CPU_MASK);
> +        }
> +    }
> +
>      s->ctlr = 0;
>  }
>
> +static void arm_gic_common_linux_init(ARMLinuxBootIf *obj,
> +                                      bool secure_boot)
> +{
> +    GICState *s = ARM_GIC_COMMON(obj);
> +
> +    if (s->security_extn && !secure_boot) {
> +        /* We're directly booting a kernel into NonSecure. If this GIC
> +         * implements the security extensions then we must configure it
> +         * to have all the interrupts be NonSecure (this is a job that
> +         * is done by the Secure boot firmware in real hardware, and in
> +         * this mode QEMU is acting as a minimalist firmware-and-bootloader
> +         * equivalent).
> +         */
> +        s->irq_reset_nonsecure = true;
> +    }
> +}
> +
>  static Property arm_gic_common_properties[] = {
>      DEFINE_PROP_UINT32("num-cpu", GICState, num_cpu, 1),
>      DEFINE_PROP_UINT32("num-irq", GICState, num_irq, 32),
> @@ -180,11 +219,13 @@ static Property arm_gic_common_properties[] = {
>  static void arm_gic_common_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> +    ARMLinuxBootIfClass *albifc = ARM_LINUX_BOOT_IF_CLASS(klass);
>
>      dc->reset = arm_gic_common_reset;
>      dc->realize = arm_gic_common_realize;
>      dc->props = arm_gic_common_properties;
>      dc->vmsd = &vmstate_gic;
> +    albifc->arm_linux_init = arm_gic_common_linux_init;
>  }
>
>  static const TypeInfo arm_gic_common_type = {
> @@ -194,6 +235,10 @@ static const TypeInfo arm_gic_common_type = {
>      .class_size = sizeof(ARMGICCommonClass),
>      .class_init = arm_gic_common_class_init,
>      .abstract = true,
> +    .interfaces = (InterfaceInfo []) {
> +        { TYPE_ARM_LINUX_BOOT_IF },
> +        { },
> +    },
>  };
>
>  static void register_types(void)
> diff --git a/include/hw/intc/arm_gic_common.h b/include/hw/intc/arm_gic_common.h
> index 899db3d..cfc1cce 100644
> --- a/include/hw/intc/arm_gic_common.h
> +++ b/include/hw/intc/arm_gic_common.h
> @@ -118,6 +118,7 @@ typedef struct GICState {
>      uint32_t num_irq;
>      uint32_t revision;
>      bool security_extn;
> +    bool irq_reset_nonsecure; /* configure IRQs as group 1 (NS) on reset? */
>      int dev_fd; /* kvm device fd if backed by kvm vgic support */
>  } GICState;
>
> --
> 1.9.1
>
>

[Qemu-devel] [PATCH v2 4/6] hw/cpu/{a15mpcore, a9mpcore}: enable TrustZone in GIC if it is enabled in CPUs

[Peter Maydell]

If the A9 and A15 CPUs which we're creating the peripherals for have
TrustZone (EL3) enabled, then also enable it in the GIC we create.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/cpu/a15mpcore.c | 13 +++++++++++++
 hw/cpu/a9mpcore.c  | 11 +++++++++++
 2 files changed, 24 insertions(+)

diff --git a/hw/cpu/a15mpcore.c b/hw/cpu/a15mpcore.c
index 49727d0..fd0c46a 100644
--- a/hw/cpu/a15mpcore.c
+++ b/hw/cpu/a15mpcore.c
@@ -56,10 +56,23 @@ static void a15mp_priv_realize(DeviceState *dev, Error **errp)
     SysBusDevice *busdev;
     int i;
     Error *err = NULL;
+    bool has_el3;
+    Object *cpuobj;
 
     gicdev = DEVICE(&s->gic);
     qdev_prop_set_uint32(gicdev, "num-cpu", s->num_cpu);
     qdev_prop_set_uint32(gicdev, "num-irq", s->num_irq);
+
+    if (!kvm_irqchip_in_kernel()) {
+        /* Make the GIC's TZ support match the CPUs. We assume that
+         * either all the CPUs have TZ, or none do.
+         */
+        cpuobj = OBJECT(qemu_get_cpu(0));
+        has_el3 = object_property_find(cpuobj, "has_el3", &error_abort) &&
+            object_property_get_bool(cpuobj, "has_el3", &error_abort);
+        qdev_prop_set_bit(gicdev, "has-security-extensions", has_el3);
+    }
+
     object_property_set_bool(OBJECT(&s->gic), true, "realized", &err);
     if (err != NULL) {
         error_propagate(errp, err);
diff --git a/hw/cpu/a9mpcore.c b/hw/cpu/a9mpcore.c
index c09358c..7046246 100644
--- a/hw/cpu/a9mpcore.c
+++ b/hw/cpu/a9mpcore.c
@@ -49,6 +49,8 @@ static void a9mp_priv_realize(DeviceState *dev, Error **errp)
                  *wdtbusdev;
     Error *err = NULL;
     int i;
+    bool has_el3;
+    Object *cpuobj;
 
     scudev = DEVICE(&s->scu);
     qdev_prop_set_uint32(scudev, "num-cpu", s->num_cpu);
@@ -62,6 +64,15 @@ static void a9mp_priv_realize(DeviceState *dev, Error **errp)
     gicdev = DEVICE(&s->gic);
     qdev_prop_set_uint32(gicdev, "num-cpu", s->num_cpu);
     qdev_prop_set_uint32(gicdev, "num-irq", s->num_irq);
+
+    /* Make the GIC's TZ support match the CPUs. We assume that
+     * either all the CPUs have TZ, or none do.
+     */
+    cpuobj = OBJECT(qemu_get_cpu(0));
+    has_el3 = object_property_find(cpuobj, "has_el3", &error_abort) &&
+        object_property_get_bool(cpuobj, "has_el3", &error_abort);
+    qdev_prop_set_bit(gicdev, "has-security-extensions", has_el3);
+
     object_property_set_bool(OBJECT(&s->gic), true, "realized", &err);
     if (err != NULL) {
         error_propagate(errp, err);
-- 
1.9.1

Re: [Qemu-devel] [PATCH v2 4/6] hw/cpu/{a15mpcore, a9mpcore}: enable TrustZone in GIC if it is enabled in CPUs

[Peter Crosthwaite]

On Thu, Jul 16, 2015 at 1:11 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> If the A9 and A15 CPUs which we're creating the peripherals for have
> TrustZone (EL3) enabled, then also enable it in the GIC we create.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>

---
P.S. My mail setup will be fixed shortly and ill be replying from the
same address as giving the RBs.

Regards,
Peter

> ---
>  hw/cpu/a15mpcore.c | 13 +++++++++++++
>  hw/cpu/a9mpcore.c  | 11 +++++++++++
>  2 files changed, 24 insertions(+)
>
> diff --git a/hw/cpu/a15mpcore.c b/hw/cpu/a15mpcore.c
> index 49727d0..fd0c46a 100644
> --- a/hw/cpu/a15mpcore.c
> +++ b/hw/cpu/a15mpcore.c
> @@ -56,10 +56,23 @@ static void a15mp_priv_realize(DeviceState *dev, Error **errp)
>      SysBusDevice *busdev;
>      int i;
>      Error *err = NULL;
> +    bool has_el3;
> +    Object *cpuobj;
>
>      gicdev = DEVICE(&s->gic);
>      qdev_prop_set_uint32(gicdev, "num-cpu", s->num_cpu);
>      qdev_prop_set_uint32(gicdev, "num-irq", s->num_irq);
> +
> +    if (!kvm_irqchip_in_kernel()) {
> +        /* Make the GIC's TZ support match the CPUs. We assume that
> +         * either all the CPUs have TZ, or none do.
> +         */
> +        cpuobj = OBJECT(qemu_get_cpu(0));
> +        has_el3 = object_property_find(cpuobj, "has_el3", &error_abort) &&
> +            object_property_get_bool(cpuobj, "has_el3", &error_abort);
> +        qdev_prop_set_bit(gicdev, "has-security-extensions", has_el3);
> +    }
> +
>      object_property_set_bool(OBJECT(&s->gic), true, "realized", &err);
>      if (err != NULL) {
>          error_propagate(errp, err);
> diff --git a/hw/cpu/a9mpcore.c b/hw/cpu/a9mpcore.c
> index c09358c..7046246 100644
> --- a/hw/cpu/a9mpcore.c
> +++ b/hw/cpu/a9mpcore.c
> @@ -49,6 +49,8 @@ static void a9mp_priv_realize(DeviceState *dev, Error **errp)
>                   *wdtbusdev;
>      Error *err = NULL;
>      int i;
> +    bool has_el3;
> +    Object *cpuobj;
>
>      scudev = DEVICE(&s->scu);
>      qdev_prop_set_uint32(scudev, "num-cpu", s->num_cpu);
> @@ -62,6 +64,15 @@ static void a9mp_priv_realize(DeviceState *dev, Error **errp)
>      gicdev = DEVICE(&s->gic);
>      qdev_prop_set_uint32(gicdev, "num-cpu", s->num_cpu);
>      qdev_prop_set_uint32(gicdev, "num-irq", s->num_irq);
> +
> +    /* Make the GIC's TZ support match the CPUs. We assume that
> +     * either all the CPUs have TZ, or none do.
> +     */
> +    cpuobj = OBJECT(qemu_get_cpu(0));
> +    has_el3 = object_property_find(cpuobj, "has_el3", &error_abort) &&
> +        object_property_get_bool(cpuobj, "has_el3", &error_abort);
> +    qdev_prop_set_bit(gicdev, "has-security-extensions", has_el3);
> +
>      object_property_set_bool(OBJECT(&s->gic), true, "realized", &err);
>      if (err != NULL) {
>          error_propagate(errp, err);
> --
> 1.9.1
>
>

[Qemu-devel] [PATCH v2 5/6] hw/arm/virt: Default to not providing TrustZone support

[Peter Maydell]

Switch the default for the 'virt' board to not providing TrustZone
support in either the CPU or the GIC. This is primarily for the
benefit of UEFI, which currently assumes there is no TrustZone
support, and does not set the GIC up correctly if it is TZ-aware.
It also means the board is consistent about its behaviour whether
we're using KVM or TCG (KVM never has TrustZone support).

If TrustZone support is required (for instance for running test
suites or TZ-aware firmware) it can be enabled with the
"-machine secure=on" command line option.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/virt.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 95b1a9a..2bcf565 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -946,8 +946,11 @@ static void virt_instance_init(Object *obj)
 {
     VirtMachineState *vms = VIRT_MACHINE(obj);
 
-    /* EL3 is enabled by default on virt */
-    vms->secure = true;
+    /* EL3 is disabled by default on virt: this makes us consistent
+     * between KVM and TCG for this board, and it also allows us to
+     * boot UEFI blobs which assume no TrustZone support.
+     */
+    vms->secure = false;
     object_property_add_bool(obj, "secure", virt_get_secure,
                              virt_set_secure, NULL);
     object_property_set_description(obj, "secure",
-- 
1.9.1

Re: [Qemu-devel] [PATCH v2 5/6] hw/arm/virt: Default to not providing TrustZone support

[Peter Crosthwaite]

On Thu, Jul 16, 2015 at 1:11 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> Switch the default for the 'virt' board to not providing TrustZone
> support in either the CPU or the GIC. This is primarily for the
> benefit of UEFI, which currently assumes there is no TrustZone
> support, and does not set the GIC up correctly if it is TZ-aware.
> It also means the board is consistent about its behaviour whether
> we're using KVM or TCG (KVM never has TrustZone support).
>
> If TrustZone support is required (for instance for running test
> suites or TZ-aware firmware) it can be enabled with the
> "-machine secure=on" command line option.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>

> ---
>  hw/arm/virt.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 95b1a9a..2bcf565 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -946,8 +946,11 @@ static void virt_instance_init(Object *obj)
>  {
>      VirtMachineState *vms = VIRT_MACHINE(obj);
>
> -    /* EL3 is enabled by default on virt */
> -    vms->secure = true;
> +    /* EL3 is disabled by default on virt: this makes us consistent
> +     * between KVM and TCG for this board, and it also allows us to
> +     * boot UEFI blobs which assume no TrustZone support.
> +     */
> +    vms->secure = false;
>      object_property_add_bool(obj, "secure", virt_get_secure,
>                               virt_set_secure, NULL);
>      object_property_set_description(obj, "secure",
> --
> 1.9.1
>
>

[Qemu-devel] [PATCH v2 6/6] hw/arm/virt: Enable TZ extensions on the GIC if we are using them

[Peter Maydell]

If we're creating a board with support for TrustZone, then enable
it on the GIC model as well as on the CPUs.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/virt.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 2bcf565..fdfa91b 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -361,7 +361,7 @@ static void create_v2m(VirtBoardInfo *vbi, qemu_irq *pic)
     fdt_add_v2m_gic_node(vbi);
 }
 
-static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic)
+static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic, bool secure)
 {
     /* We create a standalone GIC v2 */
     DeviceState *gicdev;
@@ -380,6 +380,9 @@ static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic)
      * interrupts; there are always 32 of the former (mandated by GIC spec).
      */
     qdev_prop_set_uint32(gicdev, "num-irq", NUM_IRQS + 32);
+    if (!kvm_irqchip_in_kernel()) {
+        qdev_prop_set_bit(gicdev, "has-security-extensions", secure);
+    }
     qdev_init_nofail(gicdev);
     gicbusdev = SYS_BUS_DEVICE(gicdev);
     sysbus_mmio_map(gicbusdev, 0, vbi->memmap[VIRT_GIC_DIST].base);
@@ -884,7 +887,7 @@ static void machvirt_init(MachineState *machine)
 
     create_flash(vbi);
 
-    create_gic(vbi, pic);
+    create_gic(vbi, pic, vms->secure);
 
     create_uart(vbi, pic);
 
-- 
1.9.1

Re: [Qemu-devel] [PATCH v2 6/6] hw/arm/virt: Enable TZ extensions on the GIC if we are using them

[Peter Crosthwaite]

On Thu, Jul 16, 2015 at 1:11 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> If we're creating a board with support for TrustZone, then enable
> it on the GIC model as well as on the CPUs.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>

> ---
>  hw/arm/virt.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 2bcf565..fdfa91b 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -361,7 +361,7 @@ static void create_v2m(VirtBoardInfo *vbi, qemu_irq *pic)
>      fdt_add_v2m_gic_node(vbi);
>  }
>
> -static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic)
> +static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic, bool secure)
>  {
>      /* We create a standalone GIC v2 */
>      DeviceState *gicdev;
> @@ -380,6 +380,9 @@ static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic)
>       * interrupts; there are always 32 of the former (mandated by GIC spec).
>       */
>      qdev_prop_set_uint32(gicdev, "num-irq", NUM_IRQS + 32);
> +    if (!kvm_irqchip_in_kernel()) {
> +        qdev_prop_set_bit(gicdev, "has-security-extensions", secure);
> +    }
>      qdev_init_nofail(gicdev);
>      gicbusdev = SYS_BUS_DEVICE(gicdev);
>      sysbus_mmio_map(gicbusdev, 0, vbi->memmap[VIRT_GIC_DIST].base);
> @@ -884,7 +887,7 @@ static void machvirt_init(MachineState *machine)
>
>      create_flash(vbi);
>
> -    create_gic(vbi, pic);
> +    create_gic(vbi, pic, vms->secure);
>
>      create_uart(vbi, pic);
>
> --
> 1.9.1
>
>