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

[Qemu-devel] [PATCH v2 0/4] 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

ChangeLog:

v1 -> v2:
* Create the RPU in ZynqMP realize()
* Bailout on errors when creating the RPU
* Mention default setup changes in commit messages
* Fix spelling in commit msg

Edgar E. Iglesias (4):
  xlnx-zynqmp: Add a secure prop to en/disable ARM Security Extensions
  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         | 121 ++++++++++++++++++++++++++-----------------
 include/hw/arm/xlnx-zynqmp.h |   5 ++
 2 files changed, 78 insertions(+), 48 deletions(-)

-- 
2.5.0

[Qemu-devel] [PATCH v2 1/4] 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 particularly useful for KVM runs.

Default to disabled to match the behavior of KVM.

This changes the default setup from having the ARM Security
Extensions to not longer having them.

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 v2 2/4] 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.

This changes the default setup from having the RPU to not
longer having it.

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

diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 965a250..3a8af6a 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -83,6 +83,41 @@ 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_create_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;
+
+        object_initialize(&s->rpu_cpu[i], sizeof(s->rpu_cpu[i]),
+                          "cortex-r5-" TYPE_ARM_CPU);
+        object_property_add_child(OBJECT(s), "rpu-cpu[*]",
+                                  OBJECT(&s->rpu_cpu[i]), &error_abort);
+
+        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);
@@ -95,13 +130,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,
@@ -260,23 +288,8 @@ 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 (s->has_rpu) {
+        xlnx_zynqmp_create_rpu(s, boot_cpu, &err);
         if (err) {
             error_propagate(errp, err);
             return;
@@ -373,6 +386,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_BOOL("has_rpu", XlnxZynqMPState, has_rpu, false),
     DEFINE_PROP_END_OF_LIST()
 };
 
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 v2 3/4] 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.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
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 3a8af6a..db5b82b 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -224,33 +224,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,
@@ -276,6 +252,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 v2 4/4] 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.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
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 db5b82b..9a1bc94 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
 
@@ -135,7 +137,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

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

[Alistair Francis]

On Wed, May 25, 2016 at 3:52 AM, Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Add a secure prop to en/disable ARM Security Extensions.
> This is particularly useful for KVM runs.
>
> Default to disabled to match the behavior of KVM.
>
> This changes the default setup from having the ARM Security
> Extensions to not longer having them.
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

Looks good to me

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Thanks,

Alistair

> ---
>  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
>
>

Re: [Qemu-devel] [PATCH v2 2/4] xlnx-zynqmp: Make the RPU subsystem optional

[Alistair Francis]

On Wed, May 25, 2016 at 3:52 AM, Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
> 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.
>
> This changes the default setup from having the RPU to not
> longer having it.
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Thanks,

Alistair

> ---
>  hw/arm/xlnx-zynqmp.c         | 62 +++++++++++++++++++++++++++-----------------
>  include/hw/arm/xlnx-zynqmp.h |  2 ++
>  2 files changed, 40 insertions(+), 24 deletions(-)
>
> diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
> index 965a250..3a8af6a 100644
> --- a/hw/arm/xlnx-zynqmp.c
> +++ b/hw/arm/xlnx-zynqmp.c
> @@ -83,6 +83,41 @@ 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_create_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;
> +
> +        object_initialize(&s->rpu_cpu[i], sizeof(s->rpu_cpu[i]),
> +                          "cortex-r5-" TYPE_ARM_CPU);
> +        object_property_add_child(OBJECT(s), "rpu-cpu[*]",
> +                                  OBJECT(&s->rpu_cpu[i]), &error_abort);
> +
> +        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);
> @@ -95,13 +130,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,
> @@ -260,23 +288,8 @@ 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 (s->has_rpu) {
> +        xlnx_zynqmp_create_rpu(s, boot_cpu, &err);
>          if (err) {
>              error_propagate(errp, err);
>              return;
> @@ -373,6 +386,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_BOOL("has_rpu", XlnxZynqMPState, has_rpu, false),
>      DEFINE_PROP_END_OF_LIST()
>  };
>
> 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
>
>

Re: [Qemu-devel] [PATCH v2 3/4] xlnx-zynqmp: Delay realization of GIC until post CPU realization

[Alistair Francis]

On Wed, May 25, 2016 at 3:52 AM, Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
> 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.
>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Thanks,

Alistair

> ---
>  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 3a8af6a..db5b82b 100644
> --- a/hw/arm/xlnx-zynqmp.c
> +++ b/hw/arm/xlnx-zynqmp.c
> @@ -224,33 +224,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,
> @@ -276,6 +252,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
>
>

Re: [Qemu-devel] [PATCH v2 4/4] xlnx-zynqmp: Use the in kernel GIC model for KVM runs

[Alistair Francis]

On Wed, May 25, 2016 at 3:52 AM, Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Use the in kernel GIC model when running with KVM enabled.
>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Thanks,

Alistair

> ---
>  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 db5b82b..9a1bc94 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
>
> @@ -135,7 +137,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
>
>

Re: [Qemu-devel] [PATCH v2 0/4] xlnx-zynqmp: Fix various issues with KVM runs

[Peter Maydell]

On 25 May 2016 at 11:52, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> 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.



Applied to target-arm.next, thanks.

-- PMM