[Qemu-devel] [PATCH v1 0/5] xlnx-zynqmp: Fix various issues with KVM runs

[Qemu-devel] [PATCH v1 0/5] xlnx-zynqmp: Fix various issues with KVM runs

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

This series fixes a bunch of issues with KVM runs using the ZynqMP
machine.

When using KVM, we don't have support for TrustZone, the RPU subsystem
with Cortex R5s and we also need to use the in-kernel GIC model.

This adds a set of properties to the ZynqMP allowing the creation
of the KVM friendly machine.

I've opted to default the ZynqMP to the KVM friendly machine for
consistensy between KVM and TCG and also because some of the options
are currently of limited use (i.e the RPU) or were actually disabled to
start with (e.g TrustZone sneaked in as we enabled EL3 on the A53s).
I'm OK either way though, so if people think differently we can change
the defaults.

Comments welcome!

Cheers,
Edgar

Edgar E. Iglesias (5):
  xlnx-zynqmp: Add a secure prop to en/disable ARM Security Extensions
  xlnx-zynqmp: Break out RPU setup into a separate function
  xlnx-zynqmp: Make the RPU subsystem optional
  xlnx-zynqmp: Delay realization of GIC until post CPU realization
  xlnx-zynqmp: Use the in kernel GIC model for KVM runs

 hw/arm/xlnx-zynqmp.c         | 156 +++++++++++++++++++++++++++++--------------
 include/hw/arm/xlnx-zynqmp.h |   5 ++
 2 files changed, 110 insertions(+), 51 deletions(-)

-- 
2.5.0

[Qemu-devel] [PATCH v1 1/5] xlnx-zynqmp: Add a secure prop to en/disable ARM Security Extensions

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Add a secure prop to en/disable ARM Security Extensions.
This is particulary useful for KVM runs.

Default to disabled to match the behavior of KVM.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 hw/arm/xlnx-zynqmp.c         | 3 +++
 include/hw/arm/xlnx-zynqmp.h | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 4d504da..965a250 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -238,6 +238,8 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
         }
         g_free(name);
 
+        object_property_set_bool(OBJECT(&s->apu_cpu[i]),
+                                 s->secure, "has_el3", NULL);
         object_property_set_int(OBJECT(&s->apu_cpu[i]), GIC_BASE_ADDR,
                                 "reset-cbar", &error_abort);
         object_property_set_bool(OBJECT(&s->apu_cpu[i]), true, "realized",
@@ -370,6 +372,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
 
 static Property xlnx_zynqmp_props[] = {
     DEFINE_PROP_STRING("boot-cpu", XlnxZynqMPState, boot_cpu),
+    DEFINE_PROP_BOOL("secure", XlnxZynqMPState, secure, false),
     DEFINE_PROP_END_OF_LIST()
 };
 
diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h
index 2332596..38d4c8c 100644
--- a/include/hw/arm/xlnx-zynqmp.h
+++ b/include/hw/arm/xlnx-zynqmp.h
@@ -84,6 +84,9 @@ typedef struct XlnxZynqMPState {
 
     char *boot_cpu;
     ARMCPU *boot_cpu_ptr;
+
+    /* Has the ARM Security extensions?  */
+    bool secure;
 }  XlnxZynqMPState;
 
 #define XLNX_ZYNQMP_H
-- 
2.5.0

[Qemu-devel] [PATCH v1 2/5] xlnx-zynqmp: Break out RPU setup into a separate function

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Break out RPU setup into a separate function. This is in
preparation for making the RPU optional.

No functional change.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 hw/arm/xlnx-zynqmp.c | 53 ++++++++++++++++++++++++++++++----------------------
 1 file changed, 31 insertions(+), 22 deletions(-)

diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 965a250..250ecc4 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -83,6 +83,36 @@ static inline int arm_gic_ppi_index(int cpu_nr, int ppi_index)
     return GIC_NUM_SPI_INTR + cpu_nr * GIC_INTERNAL + ppi_index;
 }
 
+static void xlnx_zynqmp_setup_rpu(XlnxZynqMPState *s, const char *boot_cpu,
+                                  Error **errp)
+{
+    Error *err = NULL;
+    int i;
+
+    for (i = 0; i < XLNX_ZYNQMP_NUM_RPU_CPUS; i++) {
+        char *name;
+
+        name = object_get_canonical_path_component(OBJECT(&s->rpu_cpu[i]));
+        if (strcmp(name, boot_cpu)) {
+            /* Secondary CPUs start in PSCI powered-down state */
+            object_property_set_bool(OBJECT(&s->rpu_cpu[i]), true,
+                                     "start-powered-off", &error_abort);
+        } else {
+            s->boot_cpu_ptr = &s->rpu_cpu[i];
+        }
+        g_free(name);
+
+        object_property_set_bool(OBJECT(&s->rpu_cpu[i]), true, "reset-hivecs",
+                                 &error_abort);
+        object_property_set_bool(OBJECT(&s->rpu_cpu[i]), true, "realized",
+                                 &err);
+        if (err) {
+            error_propagate(errp, err);
+            return;
+        }
+    }
+}
+
 static void xlnx_zynqmp_init(Object *obj)
 {
     XlnxZynqMPState *s = XLNX_ZYNQMP(obj);
@@ -260,28 +290,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
         qdev_connect_gpio_out(DEVICE(&s->apu_cpu[i]), 1, irq);
     }
 
-    for (i = 0; i < XLNX_ZYNQMP_NUM_RPU_CPUS; i++) {
-        char *name;
-
-        name = object_get_canonical_path_component(OBJECT(&s->rpu_cpu[i]));
-        if (strcmp(name, boot_cpu)) {
-            /* Secondary CPUs start in PSCI powered-down state */
-            object_property_set_bool(OBJECT(&s->rpu_cpu[i]), true,
-                                     "start-powered-off", &error_abort);
-        } else {
-            s->boot_cpu_ptr = &s->rpu_cpu[i];
-        }
-        g_free(name);
-
-        object_property_set_bool(OBJECT(&s->rpu_cpu[i]), true, "reset-hivecs",
-                                 &error_abort);
-        object_property_set_bool(OBJECT(&s->rpu_cpu[i]), true, "realized",
-                                 &err);
-        if (err) {
-            error_propagate(errp, err);
-            return;
-        }
-    }
+    xlnx_zynqmp_setup_rpu(s, boot_cpu, errp);
 
     if (!s->boot_cpu_ptr) {
         error_setg(errp, "ZynqMP Boot cpu %s not found", boot_cpu);
-- 
2.5.0

[Qemu-devel] [PATCH v1 3/5] xlnx-zynqmp: Make the RPU subsystem optional

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

The way we currently model the RPU subsystem is of quite
limited use. In addition to that, it causes problems for
KVM and for GDB debugging.

Make the RPU optional by adding a has_rpu property and
default to having it disabled.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 hw/arm/xlnx-zynqmp.c         | 50 +++++++++++++++++++++++++++++++++++++-------
 include/hw/arm/xlnx-zynqmp.h |  2 ++
 2 files changed, 44 insertions(+), 8 deletions(-)

diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 250ecc4..c180206 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -83,6 +83,40 @@ static inline int arm_gic_ppi_index(int cpu_nr, int ppi_index)
     return GIC_NUM_SPI_INTR + cpu_nr * GIC_INTERNAL + ppi_index;
 }
 
+static bool xlnx_zynqmp_get_has_rpu(Object *obj, Error **errp)
+{
+    XlnxZynqMPState *s = XLNX_ZYNQMP(obj);
+
+    return s->has_rpu;
+}
+
+static void xlnx_zynqmp_set_has_rpu(Object *obj, bool value, Error **errp)
+{
+    XlnxZynqMPState *s = XLNX_ZYNQMP(obj);
+    int i;
+
+    if (s->has_rpu == value) {
+        /* Nothing to do.  */
+        return;
+    }
+
+    /* We don't support clearing the flag.  */
+    if (s->has_rpu) {
+        error_setg(errp, "has_rpu is already set to %u",
+                   s->has_rpu);
+        return;
+    }
+
+    /* Create the Cortex R5s.  */
+    for (i = 0; i < XLNX_ZYNQMP_NUM_RPU_CPUS; i++) {
+        object_initialize(&s->rpu_cpu[i], sizeof(s->rpu_cpu[i]),
+                          "cortex-r5-" TYPE_ARM_CPU);
+        object_property_add_child(obj, "rpu-cpu[*]", OBJECT(&s->rpu_cpu[i]),
+                                  &error_abort);
+    }
+    s->has_rpu = value;
+}
+
 static void xlnx_zynqmp_setup_rpu(XlnxZynqMPState *s, const char *boot_cpu,
                                   Error **errp)
 {
@@ -118,6 +152,11 @@ static void xlnx_zynqmp_init(Object *obj)
     XlnxZynqMPState *s = XLNX_ZYNQMP(obj);
     int i;
 
+    object_property_add_bool(obj, "has_rpu",
+                             xlnx_zynqmp_get_has_rpu,
+                             xlnx_zynqmp_set_has_rpu,
+                             &error_abort);
+
     for (i = 0; i < XLNX_ZYNQMP_NUM_APU_CPUS; i++) {
         object_initialize(&s->apu_cpu[i], sizeof(s->apu_cpu[i]),
                           "cortex-a53-" TYPE_ARM_CPU);
@@ -125,13 +164,6 @@ static void xlnx_zynqmp_init(Object *obj)
                                   &error_abort);
     }
 
-    for (i = 0; i < XLNX_ZYNQMP_NUM_RPU_CPUS; i++) {
-        object_initialize(&s->rpu_cpu[i], sizeof(s->rpu_cpu[i]),
-                          "cortex-r5-" TYPE_ARM_CPU);
-        object_property_add_child(obj, "rpu-cpu[*]", OBJECT(&s->rpu_cpu[i]),
-                                  &error_abort);
-    }
-
     object_property_add_link(obj, "ddr-ram", TYPE_MEMORY_REGION,
                              (Object **)&s->ddr_ram,
                              qdev_prop_allow_set_link_before_realize,
@@ -290,7 +322,9 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
         qdev_connect_gpio_out(DEVICE(&s->apu_cpu[i]), 1, irq);
     }
 
-    xlnx_zynqmp_setup_rpu(s, boot_cpu, errp);
+    if (s->has_rpu) {
+        xlnx_zynqmp_setup_rpu(s, boot_cpu, errp);
+    }
 
     if (!s->boot_cpu_ptr) {
         error_setg(errp, "ZynqMP Boot cpu %s not found", boot_cpu);
diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h
index 38d4c8c..68f6eb0 100644
--- a/include/hw/arm/xlnx-zynqmp.h
+++ b/include/hw/arm/xlnx-zynqmp.h
@@ -87,6 +87,8 @@ typedef struct XlnxZynqMPState {
 
     /* Has the ARM Security extensions?  */
     bool secure;
+    /* Has the RPU subsystem?  */
+    bool has_rpu;
 }  XlnxZynqMPState;
 
 #define XLNX_ZYNQMP_H
-- 
2.5.0

[Qemu-devel] [PATCH v1 4/5] xlnx-zynqmp: Delay realization of GIC until post CPU realization

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Delay the realization of the GIC until after CPUs are
realized. This is needed for KVM as the in-kernel GIC
model will fail if it is realized with no available CPUs.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 hw/arm/xlnx-zynqmp.c | 56 +++++++++++++++++++++++++++++-----------------------
 1 file changed, 31 insertions(+), 25 deletions(-)

diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index c180206..0648028 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -258,33 +258,9 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
     qdev_prop_set_uint32(DEVICE(&s->gic), "num-irq", GIC_NUM_SPI_INTR + 32);
     qdev_prop_set_uint32(DEVICE(&s->gic), "revision", 2);
     qdev_prop_set_uint32(DEVICE(&s->gic), "num-cpu", XLNX_ZYNQMP_NUM_APU_CPUS);
-    object_property_set_bool(OBJECT(&s->gic), true, "realized", &err);
-    if (err) {
-        error_propagate(errp, err);
-        return;
-    }
-    assert(ARRAY_SIZE(xlnx_zynqmp_gic_regions) == XLNX_ZYNQMP_GIC_REGIONS);
-    for (i = 0; i < XLNX_ZYNQMP_GIC_REGIONS; i++) {
-        SysBusDevice *gic = SYS_BUS_DEVICE(&s->gic);
-        const XlnxZynqMPGICRegion *r = &xlnx_zynqmp_gic_regions[i];
-        MemoryRegion *mr = sysbus_mmio_get_region(gic, r->region_index);
-        uint32_t addr = r->address;
-        int j;
-
-        sysbus_mmio_map(gic, r->region_index, addr);
-
-        for (j = 0; j < XLNX_ZYNQMP_GIC_ALIASES; j++) {
-            MemoryRegion *alias = &s->gic_mr[i][j];
-
-            addr += XLNX_ZYNQMP_GIC_REGION_SIZE;
-            memory_region_init_alias(alias, OBJECT(s), "zynqmp-gic-alias", mr,
-                                     0, XLNX_ZYNQMP_GIC_REGION_SIZE);
-            memory_region_add_subregion(system_memory, addr, alias);
-        }
-    }
 
+    /* Realize APUs before realizing the GIC. KVM requires this.  */
     for (i = 0; i < XLNX_ZYNQMP_NUM_APU_CPUS; i++) {
-        qemu_irq irq;
         char *name;
 
         object_property_set_int(OBJECT(&s->apu_cpu[i]), QEMU_PSCI_CONDUIT_SMC,
@@ -310,6 +286,36 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
             error_propagate(errp, err);
             return;
         }
+    }
+
+    object_property_set_bool(OBJECT(&s->gic), true, "realized", &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+
+    assert(ARRAY_SIZE(xlnx_zynqmp_gic_regions) == XLNX_ZYNQMP_GIC_REGIONS);
+    for (i = 0; i < XLNX_ZYNQMP_GIC_REGIONS; i++) {
+        SysBusDevice *gic = SYS_BUS_DEVICE(&s->gic);
+        const XlnxZynqMPGICRegion *r = &xlnx_zynqmp_gic_regions[i];
+        MemoryRegion *mr = sysbus_mmio_get_region(gic, r->region_index);
+        uint32_t addr = r->address;
+        int j;
+
+        sysbus_mmio_map(gic, r->region_index, addr);
+
+        for (j = 0; j < XLNX_ZYNQMP_GIC_ALIASES; j++) {
+            MemoryRegion *alias = &s->gic_mr[i][j];
+
+            addr += XLNX_ZYNQMP_GIC_REGION_SIZE;
+            memory_region_init_alias(alias, OBJECT(s), "zynqmp-gic-alias", mr,
+                                     0, XLNX_ZYNQMP_GIC_REGION_SIZE);
+            memory_region_add_subregion(system_memory, addr, alias);
+        }
+    }
+
+    for (i = 0; i < XLNX_ZYNQMP_NUM_APU_CPUS; i++) {
+        qemu_irq irq;
 
         sysbus_connect_irq(SYS_BUS_DEVICE(&s->gic), i,
                            qdev_get_gpio_in(DEVICE(&s->apu_cpu[i]),
-- 
2.5.0

[Qemu-devel] [PATCH v1 5/5] xlnx-zynqmp: Use the in kernel GIC model for KVM runs

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Use the in kernel GIC model when running with KVM enabled.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 hw/arm/xlnx-zynqmp.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 0648028..c3878da 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -22,6 +22,8 @@
 #include "hw/arm/xlnx-zynqmp.h"
 #include "hw/intc/arm_gic_common.h"
 #include "exec/address-spaces.h"
+#include "sysemu/kvm.h"
+#include "kvm_arm.h"
 
 #define GIC_NUM_SPI_INTR 160
 
@@ -169,7 +171,7 @@ static void xlnx_zynqmp_init(Object *obj)
                              qdev_prop_allow_set_link_before_realize,
                              OBJ_PROP_LINK_UNREF_ON_RELEASE, &error_abort);
 
-    object_initialize(&s->gic, sizeof(s->gic), TYPE_ARM_GIC);
+    object_initialize(&s->gic, sizeof(s->gic), gic_class_name());
     qdev_set_parent_bus(DEVICE(&s->gic), sysbus_get_default());
 
     for (i = 0; i < XLNX_ZYNQMP_NUM_GEMS; i++) {
-- 
2.5.0

[Qemu-devel] [PATCH v1 0/5] xlnx-zynqmp: Fix various issues with KVM runs

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

(This is a resend of the same v1, I managed to typo my own work email
address in CC in the first send-email :-))

This series fixes a bunch of issues with KVM runs using the ZynqMP
machine.

When using KVM, we don't have support for TrustZone, the RPU subsystem
with Cortex R5s and we also need to use the in-kernel GIC model.

This adds a set of properties to the ZynqMP allowing the creation
of the KVM friendly machine.

I've opted to default the ZynqMP to the KVM friendly machine for
consistensy between KVM and TCG and also because some of the options
are currently of limited use (i.e the RPU) or were actually disabled to
start with (e.g TrustZone sneaked in as we enabled EL3 on the A53s).
I'm OK either way though, so if people think differently we can change
the defaults.

Comments welcome!

Cheers,
Edgar

Edgar E. Iglesias (5):
  xlnx-zynqmp: Add a secure prop to en/disable ARM Security Extensions
  xlnx-zynqmp: Break out RPU setup into a separate function
  xlnx-zynqmp: Make the RPU subsystem optional
  xlnx-zynqmp: Delay realization of GIC until post CPU realization
  xlnx-zynqmp: Use the in kernel GIC model for KVM runs

 hw/arm/xlnx-zynqmp.c         | 156 +++++++++++++++++++++++++++++--------------
 include/hw/arm/xlnx-zynqmp.h |   5 ++
 2 files changed, 110 insertions(+), 51 deletions(-)

-- 
2.5.0