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

[Qemu-devel] [PATCH v3 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.

There are no changes since v3, I've just rebased this all on
top of current target-arm.next (which will be in master early
next week as I've just sent out the pullreq).

These have already been reviewed by Peter C, but when I spoke
to Edgar at KVM Forum he said he hadn't been able to sort out
which tree these would apply to, so I figured I'd better resend
them so there's a chance for further review/testing if desired.

thanks
-- PMM

Changes since v2:
 * rebased onto target-arm.next
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.



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 v3 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 v3 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..bef451b 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're 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

[Qemu-devel] [PATCH v3 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>
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 fe64b51..844495f 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)
 {
@@ -165,12 +166,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;
@@ -179,7 +195,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;
@@ -191,7 +207,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++) {
@@ -202,9 +218,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),
@@ -221,11 +260,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 = {
@@ -235,6 +276,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 edca3e0..9c46702 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 v3 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>
Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
 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 58ac02e..4ef8db1 100644
--- a/hw/cpu/a15mpcore.c
+++ b/hw/cpu/a15mpcore.c
@@ -52,10 +52,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 v3 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>
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 91e45e0..a067748 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1044,8 +1044,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 v3 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>
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 a067748..e9324f5 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -396,7 +396,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;
@@ -413,6 +413,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);
@@ -967,7 +970,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 v3 0/6] ARM: enable TZ in the GIC

[Edgar E. Iglesias]

On Fri, Sep 04, 2015 at 05:22:56PM +0100, Peter Maydell wrote:
> 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.
> 
> There are no changes since v3, I've just rebased this all on
> top of current target-arm.next (which will be in master early
> next week as I've just sent out the pullreq).
> 
> These have already been reviewed by Peter C, but when I spoke
> to Edgar at KVM Forum he said he hadn't been able to sort out
> which tree these would apply to, so I figured I'd better resend
> them so there's a chance for further review/testing if desired.
>

Thanks Peter,

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Tested-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

Cheers,
Edgar

 
> thanks
> -- PMM
> 
> Changes since v2:
>  * rebased onto target-arm.next
> 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.
> 
> 
> 
> 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
> 

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

[Sergey Fedorov]

Hi Peter,

On 04.09.2015 19:22, Peter Maydell wrote:
> +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);

Why don't just pass a pointer to arm_boot_info structure itself rather
than its secure_boot element to arm_linux_init()?

> +} ARMLinuxBootIfClass;

Best regards,
Sergey

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

[Peter Maydell]

On 7 September 2015 at 13:19, Sergey Fedorov <serge.fdrv@gmail.com> wrote:
> Hi Peter,
>
> On 04.09.2015 19:22, Peter Maydell wrote:
>> +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);
>
> Why don't just pass a pointer to arm_boot_info structure itself rather
> than its secure_boot element to arm_linux_init()?

See review discussion on v1. The arm_boot_info structure includes
fields that use data types that are only available to source files
compiled per-target, and the GIC source files are compiled once-only.

thanks
-- PMM

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

[Sergey Fedorov]

On 07.09.2015 16:17, Peter Maydell wrote:
> See review discussion on v1. The arm_boot_info structure includes
> fields that use data types that are only available to source files
> compiled per-target, and the GIC source files are compiled once-only.
Ah, sorry for that.

Best,
Sergey