Next Generation Xilinx Zynq SoC

In preparation for support for Cortex a53. Use "axx" to describe the shareable features. Some of the CP15 registers (such as ACTLR) are specific to implementation, but we currently just RAZ them so continue with that as the policy for all cortex A processors under a shared definition.

The cache sizes and geometeries, the L1 I-cache policy and the physical address range differ between A53 and A57 so those particulars are left as A57 specific. The rest are moved to the generalisation.

Quoted 9 lines · Peter Crosthwaite

In preparation for support for Cortex a53. Use "axx" to describe the shareable features. Some of the CP15 registers (such as ACTLR) are specific to implementation, but we currently just RAZ them so continue with that as the policy for all cortex A processors under a shared definition.

The cache sizes and geometeries, the L1 I-cache policy and the physical address range differ between A53 and A57 so those particulars are left as A57 specific. The rest are moved to the generalisation.

I'm not a huge fan of this as it seems unlikely that every CPU with a "Cortex-Axx" is going to look so similar to the A57.

Call the reginfo table cortex_a53_a57_cp_reginfo if you want to share it, but I would leave the initfns completely separate personally.

-- PMM

Similar to a53, but with different L1 I cache policy, phys addr size and

different cache geometries. The cache sizes is implementation configurable, but use these values (from Xilinx MPSoC) as a default until cache size configurability is added.

Reviewed-by Alex Bennée <[email protected]>

Signed-off-by Peter Crosthwaite <[email protected]>

---
Changed since v2:
Added dtb compatible string

 target-arm/cpu64.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

target-arm/cpu64.c +16 -1

index 3eb58c6..728d9a7 100644
@@ -149,6 +149,21 @@ static void aarch64_a57_initfn(Object *obj)
     cpu->ccsidr[2] = 0x70ffe07a; /* 2048KB L2 cache */
 }
 
+static void aarch64_a53_initfn(Object *obj)
+{
+    ARMCPU *cpu = ARM_CPU(obj);
+
+    aarch64_axx_initfn(cpu);
+
+    cpu->dtb_compatible = "arm,cortex-a53";
+    cpu->midr = 0x410fd034;
+    cpu->ctr = 0x84448004; /* L1Ip = VIPT */
+    cpu->id_aa64mmfr0 = 0x00001122; /* 40 bit physical addr */
+    cpu->ccsidr[0] = 0x700fe01a; /* 32KB L1 dcache */
+    cpu->ccsidr[1] = 0x201fe00a; /* 32KB L1 icache */
+    cpu->ccsidr[2] = 0x707fe07a; /* 1024KB L2 cache */
+}
+
 #ifdef CONFIG_USER_ONLY
 static void aarch64_any_initfn(Object *obj)
 {

@@ -176,6 +191,7 @@ typedef struct ARMCPUInfo {
 
 static const ARMCPUInfo aarch64_cpus[] = {
     { .name = "cortex-a57",         .initfn = aarch64_a57_initfn },
+    { .name = "cortex-a53",         .initfn = aarch64_a53_initfn },
 #ifdef CONFIG_USER_ONLY
     { .name = "any",         .initfn = aarch64_any_initfn },
 #endif
--

2.3.1.2.g90df61e.dirty

Hi,

Quoted 1 line · Peter Crosthwaite

Similar to a53, but with different L1 I cache policy, phys addr size and

             ^^^

I guess a57 :)

With quad Cortex-A53 CPUs.

Signed-off-by Peter Crosthwaite <[email protected]>

---
changed since v2:
Added [*] to cpu child property name.
changed since v1:
Add &error_abort to CPU child adder call.

 default-configs/aarch64-softmmu.mak |  2 +-
 hw/arm/Makefile.objs                |  1 +
 hw/arm/xlnx-zynqmp.c                | 72 +++++++++++++++++++++++++++++++++++++
 include/hw/arm/xlnx-zynqmp.h        | 21 +++++++++++
 4 files changed, 95 insertions(+), 1 deletion(-)
 create mode 100644 hw/arm/xlnx-zynqmp.c
 create mode 100644 include/hw/arm/xlnx-zynqmp.h

default-configs/aarch64-softmmu.mak +1 -1

index 6d3b5c7..96dd994 100644
@@ -3,4 +3,4 @@
 # We support all the 32 bit boards so need all their config
 include arm-softmmu.mak
 
-# Currently no 64-bit specific config requirements
+CONFIG_XLNX_ZYNQMP=y

hw/arm/Makefile.objs +1 -0

index 2577f68..d7cd5f4 100644
@@ -10,3 +10,4 @@ obj-$(CONFIG_DIGIC) += digic.o
 obj-y += omap1.o omap2.o strongarm.o
 obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10.o cubieboard.o
 obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o
+obj-$(CONFIG_XLNX_ZYNQMP) += xlnx-zynqmp.o

hw/arm/xlnx-zynqmp.c new file +72 -0

new file mode 100644
index 0000000..41c207a
@@ -0,0 +1,72 @@
+/*
+ * Xilinx Zynq MPSoC emulation
+ *
+ * Copyright (C) 2015 Xilinx Inc
+ * Written by Peter Crosthwaite <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * for more details.
+ */
+
+#include "hw/arm/xlnx-zynqmp.h"
+
+static void xlnx_zynqmp_init(Object *obj)
+{
+    XlnxZynqMPState *s = XLNX_ZYNQMP(obj);
+    int i;
+
+    for (i = 0; i < XLNX_ZYNQMP_NUM_CPUS; i++) {
+        object_initialize(&s->cpu[i], sizeof(s->cpu[i]),
+                          "cortex-a53-" TYPE_ARM_CPU);
+        object_property_add_child(obj, "cpu[*]", OBJECT(&s->cpu[i]),
+                                  &error_abort);
+    }
+}
+
+#define ERR_PROP_CHECK_RETURN(err, errp) do { \
+    if (err) { \
+        error_propagate((errp), (err)); \
+        return; \
+    } \
+} while (0)
+
+static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
+{
+    XlnxZynqMPState *s = XLNX_ZYNQMP(dev);
+    uint8_t i;
+    Error *err = NULL;
+
+    for (i = 0; i < XLNX_ZYNQMP_NUM_CPUS; i++) {
+        object_property_set_bool(OBJECT(&s->cpu[i]), true, "realized", &err);
+        ERR_PROP_CHECK_RETURN(err, errp);
+    }
+}
+
+static void xlnx_zynqmp_class_init(ObjectClass *oc, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(oc);
+
+    dc->realize = xlnx_zynqmp_realize;
+}
+
+static const TypeInfo xlnx_zynqmp_type_info = {
+    .name = TYPE_XLNX_ZYNQMP,
+    .parent = TYPE_DEVICE,
+    .instance_size = sizeof(XlnxZynqMPState),
+    .instance_init = xlnx_zynqmp_init,
+    .class_init = xlnx_zynqmp_class_init,
+};
+
+static void xlnx_zynqmp_register_types(void)
+{
+    type_register_static(&xlnx_zynqmp_type_info);
+}
+
+type_init(xlnx_zynqmp_register_types)

include/hw/arm/xlnx-zynqmp.h new file +5 -0

new file mode 100644
index 0000000..d6b3b92
@@ -0,0 +1,21 @@
+#ifndef XLNX_ZYNQMP_H_
+
+#include "qemu-common.h"
+#include "hw/arm/arm.h"
+

include/hw/arm/xlnx-zynqmp.h new file +6 -1

+
+    ARMCPU cpu[XLNX_ZYNQMP_NUM_CPUS];
+}  XlnxZynqMPState;
+
+#define XLNX_ZYNQMP_H_
+#endif
--

2.3.1.2.g90df61e.dirty

With quad Cortex-A53 CPUs.

Signed-off-by Peter Crosthwaite <[email protected]>

---
changed since v2:
Added [*] to cpu child property name.
changed since v1:
Add &error_abort to CPU child adder call.

 default-configs/aarch64-softmmu.mak |  2 +-
 hw/arm/Makefile.objs                |  1 +
 hw/arm/xlnx-zynqmp.c                | 72 +++++++++++++++++++++++++++++++++++++
 include/hw/arm/xlnx-zynqmp.h        | 21 +++++++++++
 4 files changed, 95 insertions(+), 1 deletion(-)
 create mode 100644 hw/arm/xlnx-zynqmp.c
 create mode 100644 include/hw/arm/xlnx-zynqmp.h

default-configs/aarch64-softmmu.mak +1 -1

index 6d3b5c7..96dd994 100644
@@ -3,4 +3,4 @@
 # We support all the 32 bit boards so need all their config
 include arm-softmmu.mak
 
-# Currently no 64-bit specific config requirements
+CONFIG_XLNX_ZYNQMP=y

hw/arm/Makefile.objs +0 -0

index 2577f68..d7cd5f4 100644

hw/arm/xlnx-zynqmp.c new file +10 -0

new file mode 100644
index 0000000..41c207a
@@ -0,0 +1,72 @@
+/*
+ * Xilinx Zynq MPSoC emulation
+ *
+ * Copyright (C) 2015 Xilinx Inc
+ * Written by Peter Crosthwaite <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.

hw/arm/xlnx-zynqmp.c new file +8 -0

+{
+    XlnxZynqMPState *s = XLNX_ZYNQMP(obj);
+    int i;
+
+    for (i = 0; i < XLNX_ZYNQMP_NUM_CPUS; i++) {
+        object_initialize(&s->cpu[i], sizeof(s->cpu[i]),
+                          "cortex-a53-" TYPE_ARM_CPU);
+        object_property_add_child(obj, "cpu[*]", OBJECT(&s->cpu[i]),

include/hw/arm/xlnx-zynqmp.h new file +20 -1

+
+#include "qemu-common.h"
+#include "hw/arm/arm.h"
+
+#define TYPE_XLNX_ZYNQMP "xlnx,zynqmp"
+#define XLNX_ZYNQMP(obj) OBJECT_CHECK(XlnxZynqMPState, (obj), \
+                                       TYPE_XLNX_ZYNQMP)
+
+#define XLNX_ZYNQMP_NUM_CPUS 4
+
+typedef struct XlnxZynqMPState {
+    /*< private >*/
+    DeviceState parent_obj;
+    /*< public >*/
+
+    ARMCPU cpu[XLNX_ZYNQMP_NUM_CPUS];
+}  XlnxZynqMPState;
+
+#define XLNX_ZYNQMP_H_
+#endif
--

2.3.1.2.g90df61e.dirty

Quoted 25 lines · Peter Crosthwaite

With quad Cortex-A53 CPUs.

Signed-off-by Peter Crosthwaite <[email protected]>

---
changed since v2:
Added [*] to cpu child property name.
changed since v1:
Add &error_abort to CPU child adder call.

 default-configs/aarch64-softmmu.mak |  2 +-
 hw/arm/Makefile.objs                |  1 +
 hw/arm/xlnx-zynqmp.c                | 72 +++++++++++++++++++++++++++++++++++++
 include/hw/arm/xlnx-zynqmp.h        | 21 +++++++++++
 4 files changed, 95 insertions(+), 1 deletion(-)
 create mode 100644 hw/arm/xlnx-zynqmp.c
 create mode 100644 include/hw/arm/xlnx-zynqmp.h

default-configs/aarch64-softmmu.mak +0 -0

index 6d3b5c7..96dd994 100644
@@ -3,4 +3,4 @@
 # We support all the 32 bit boards so need all their config
 include arm-softmmu.mak
 

hw/arm/Makefile.objs obj-$(CONFIG_DIGIC) += digic.o · line 13

Quoted 11 lines

default-configs/aarch64-softmmu.mak +1 -1

-# Currently no 64-bit specific config requirements
+CONFIG_XLNX_ZYNQMP=y

hw/arm/Makefile.objs +1 -0

index 2577f68..d7cd5f4 100644
@@ -10,3 +10,4 @@ obj-$(CONFIG_DIGIC) += digic.o
 obj-y += omap1.o omap2.o strongarm.o
 obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10.o cubieboard.o
 obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o
+obj-$(CONFIG_XLNX_ZYNQMP) += xlnx-zynqmp.o

Can this be a common-obj- ?

Quoted 21 lines

hw/arm/xlnx-zynqmp.c new file +15 -0

new file mode 100644
index 0000000..41c207a
@@ -0,0 +1,72 @@
+/*
+ * Xilinx Zynq MPSoC emulation
+ *
+ * Copyright (C) 2015 Xilinx Inc
+ * Written by Peter Crosthwaite <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * for more details.

hw/arm/xlnx-zynqmp.c line 20

Quoted 5 lines

hw/arm/xlnx-zynqmp.c new file +5 -0

+ */
+
+#include "hw/arm/xlnx-zynqmp.h"
+
+static void xlnx_zynqmp_init(Object *obj)

We don't abbreviate 'Xilinx' in existing type and function names, so it's a bit inconsistent to do so here.

Quoted 14 lines

hw/arm/xlnx-zynqmp.c new file +14 -0

+{
+    XlnxZynqMPState *s = XLNX_ZYNQMP(obj);
+    int i;
+
+    for (i = 0; i < XLNX_ZYNQMP_NUM_CPUS; i++) {
+        object_initialize(&s->cpu[i], sizeof(s->cpu[i]),
+                          "cortex-a53-" TYPE_ARM_CPU);
+        object_property_add_child(obj, "cpu[*]", OBJECT(&s->cpu[i]),
+                                  &error_abort);
+    }
+}
+
+#define ERR_PROP_CHECK_RETURN(err, errp) do { \
+    if (err) { \

hw/arm/xlnx-zynqmp.c line 38

Quoted 4 lines

hw/arm/xlnx-zynqmp.c new file +4 -0

+        error_propagate((errp), (err)); \
+        return; \
+    } \
+} while (0)

I don't think our QOM error handling is so ugly as to justify hiding it behind a macro (particularly not one with hidden control flow).

Quoted 33 lines

hw/arm/xlnx-zynqmp.c new file +33 -0

+static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
+{
+    XlnxZynqMPState *s = XLNX_ZYNQMP(dev);
+    uint8_t i;
+    Error *err = NULL;
+
+    for (i = 0; i < XLNX_ZYNQMP_NUM_CPUS; i++) {
+        object_property_set_bool(OBJECT(&s->cpu[i]), true, "realized", &err);
+        ERR_PROP_CHECK_RETURN(err, errp);
+    }
+}
+
+static void xlnx_zynqmp_class_init(ObjectClass *oc, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(oc);
+
+    dc->realize = xlnx_zynqmp_realize;
+}
+
+static const TypeInfo xlnx_zynqmp_type_info = {
+    .name = TYPE_XLNX_ZYNQMP,
+    .parent = TYPE_DEVICE,
+    .instance_size = sizeof(XlnxZynqMPState),
+    .instance_init = xlnx_zynqmp_init,
+    .class_init = xlnx_zynqmp_class_init,
+};
+
+static void xlnx_zynqmp_register_types(void)
+{
+    type_register_static(&xlnx_zynqmp_type_info);
+}
+
+type_init(xlnx_zynqmp_register_types)

include/hw/arm/xlnx-zynqmp.h

Quoted 6 lines

include/hw/arm/xlnx-zynqmp.h new file +0 -0

new file mode 100644
index 0000000..d6b3b92
@@ -0,0 +1,21 @@

This file needs a copyright-and-license comment.

include/hw/arm/xlnx-zynqmp.h line 1

Quoted 1 line

include/hw/arm/xlnx-zynqmp.h new file +1 -0

+#ifndef XLNX_ZYNQMP_H_

No trailing underscore, please.

Quoted 21 lines

include/hw/arm/xlnx-zynqmp.h new file +20 -1

+
+#include "qemu-common.h"
+#include "hw/arm/arm.h"
+
+#define TYPE_XLNX_ZYNQMP "xlnx,zynqmp"
+#define XLNX_ZYNQMP(obj) OBJECT_CHECK(XlnxZynqMPState, (obj), \
+                                       TYPE_XLNX_ZYNQMP)
+
+#define XLNX_ZYNQMP_NUM_CPUS 4
+
+typedef struct XlnxZynqMPState {
+    /*< private >*/
+    DeviceState parent_obj;
+    /*< public >*/
+
+    ARMCPU cpu[XLNX_ZYNQMP_NUM_CPUS];
+}  XlnxZynqMPState;
+
+#define XLNX_ZYNQMP_H_
+#endif
--

Quoted 2 lines

2.3.1.2.g90df61e.dirty

thanks
-- PMM

Quoted 12 lines · Peter Maydell

hw/arm/Makefile.objs obj-$(CONFIG_DIGIC) += digic.o · line 13

Quoted 7 lines · Peter Crosthwaite

hw/arm/Makefile.objs +1 -0

@@ -10,3 +10,4 @@ obj-$(CONFIG_DIGIC) += digic.o
 obj-y += omap1.o omap2.o strongarm.o
 obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10.o cubieboard.o
 obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o
+obj-$(CONFIG_XLNX_ZYNQMP) += xlnx-zynqmp.o

Can this be a common-obj- ?

Seems to build fine. I'll make this change. It's inconsistent with surrounding code so I guess this is a new policy?

Quoted 3 lines

We don't abbreviate 'Xilinx' in existing type and function names, so it's a bit inconsistent to do so here.

I want to fix this policy going forward as I am matching the device tree vendor code and the 2 less chars does save on a few 80 char wraps (particularly on lines with more than one "xilinx"). The existing code in tree is already self inconsistent between C variable names and literal string in this matter, e,g:

hw/char/xilinx_uartlite.c:49:#define TYPE_XILINX_UARTLITE "xlnx.xps-uartlite" hw/char/xilinx_uartlite.c:221:
"xlnx.xps-uartlite", R_MAX * 4); hw/dma/xilinx_axidma.c:36:#define TYPE_XILINX_AXI_DMA "xlnx.axi-dma"
hw/dma/xilinx_axidma.c:601: "xlnx.axi-dma",
R_MAX * 4 * 2);
hw/intc/xilinx_intc.c:40:#define TYPE_XILINX_INTC "xlnx.xps-intc"
hw/intc/xilinx_intc.c:171: memory_region_init_io(&p->mmio, obj,
&pic_ops, p, "xlnx.xps-intc",

With a mix of XILINX and "xlnx". I think "xlnx' is universally correct going forward.

Quoted 4 lines

I don't think our QOM error handling is so ugly as to justify hiding it behind a macro (particularly not one with hidden control flow).

Fixing. This is discussed on list so I guess I can join that discussion. It is a little verbose having every 1 line API call have 4 lines or error handling.

Quoted 15 lines · Peter Crosthwaite

Quoted 12 lines · Peter Maydell

hw/arm/Makefile.objs obj-$(CONFIG_DIGIC) += digic.o · line 13

Quoted 7 lines · Peter Crosthwaite

hw/arm/Makefile.objs +1 -0

@@ -10,3 +10,4 @@ obj-$(CONFIG_DIGIC) += digic.o
 obj-y += omap1.o omap2.o strongarm.o
 obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10.o cubieboard.o
 obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o
+obj-$(CONFIG_XLNX_ZYNQMP) += xlnx-zynqmp.o

Can this be a common-obj- ?

Quoted 2 lines

Seems to build fine. I'll make this change. It's inconsistent with surrounding code so I guess this is a new policy?

Historically this makefile's objects were only built for ARM targets anyway, so it didn't make much difference. Now we have aarch64 it avoids building a .o twice

Quoted 20 lines · Peter Maydell

Quoted 17 lines · Peter Crosthwaite

Quoted 12 lines · Peter Maydell

hw/arm/Makefile.objs obj-$(CONFIG_DIGIC) += digic.o · line 13

Quoted 7 lines · Peter Crosthwaite

hw/arm/Makefile.objs +1 -0

@@ -10,3 +10,4 @@ obj-$(CONFIG_DIGIC) += digic.o
 obj-y += omap1.o omap2.o strongarm.o
 obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10.o cubieboard.o
 obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o
+obj-$(CONFIG_XLNX_ZYNQMP) += xlnx-zynqmp.o

Can this be a common-obj- ?

Seems to build fine. I'll make this change. It's inconsistent with surrounding code so I guess this is a new policy?

Quoted 3 lines

Historically this makefile's objects were only built for ARM targets anyway, so it didn't make much difference. Now we have aarch64 it avoids building a .o twice

I looked a bit more closely at this, and hw/arm is treated specially (the hw/$(TARGET_BASE_ARCH) directories are pulled in directly by Makefile.target and only for obj-y),

so for now new object files in this directory should just be in obj-. I might see if we can add support for common-obj-y, but that's a separate patch.

thanks
-- PMM

hw/arm/xlnx-zynqmp.c new file +13 -0

+static void xlnx_zynqmp_init(Object *obj)
+{
+    XlnxZynqMPState *s = XLNX_ZYNQMP(obj);
+    int i;
+
+    for (i = 0; i < XLNX_ZYNQMP_NUM_CPUS; i++) {
+        object_initialize(&s->cpu[i], sizeof(s->cpu[i]),
+                          "cortex-a53-" TYPE_ARM_CPU);
+        object_property_add_child(obj, "cpu[*]", OBJECT(&s->cpu[i]),
+                                  &error_abort);
+    }
+}
 
+static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)

Quoted 12 lines · Peter Crosthwaite

hw/arm/xlnx-zynqmp.c new file +12 -0

+static void xlnx_zynqmp_init(Object *obj)
+{
+    XlnxZynqMPState *s = XLNX_ZYNQMP(obj);
+    int i;
+
+    for (i = 0; i < XLNX_ZYNQMP_NUM_CPUS; i++) {
+        object_initialize(&s->cpu[i], sizeof(s->cpu[i]),
+                          "cortex-a53-" TYPE_ARM_CPU);
+        object_property_add_child(obj, "cpu[*]", OBJECT(&s->cpu[i]),
+                                  &error_abort);
+    }
+}

hw/arm/xlnx-zynqmp.c line 50

Quoted 11 lines

hw/arm/xlnx-zynqmp.c new file +11 -0

+static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
+{
+    XlnxZynqMPState *s = XLNX_ZYNQMP(dev);
+    uint8_t i;
+    Error *err = NULL;
+
+    for (i = 0; i < XLNX_ZYNQMP_NUM_CPUS; i++) {
+        object_property_set_bool(OBJECT(&s->cpu[i]), true, "realized", &err);
+        ERR_PROP_CHECK_RETURN(err, errp);
+    }
+}

This seems to be a touch short on CPU property setting. I would expect you want most or all of:
* has_el3

* psci-conduit
* start-powered-off for the secondaries

* reset-cbar

(Some of those may be new in master since this series was posted.)

thanks
-- PMM

And connect IRQ outputs to the CPUs.

Quoted 1 line · Peter Crosthwaite

And connect IRQ outputs to the CPUs.

Can you make sure the body of the commit message makes sense when read as a standalone text without the subject line, please?

Otherwise

Reviewed-by Peter Maydell <[email protected]>

thanks
-- PMM

Connect the GPIO outputs from the individual CPUs for the timers to the GIC.

Signed-off-by Peter Crosthwaite <[email protected]>

---
 hw/arm/xlnx-zynqmp.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

hw/arm/xlnx-zynqmp.c +0 -0

index 9465185..29954f5 100644

hw/arm/xlnx-zynqmp.c +10 -1

+}
+
 static void xlnx_zynqmp_init(Object *obj)
 {
     XlnxZynqMPState *s = XLNX_ZYNQMP(obj);

@@ -60,11 +68,19 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
     sysbus_mmio_map(SYS_BUS_DEVICE(&s->gic), 1, GIC_CPU_ADDR);
 
     for (i = 0; i < XLNX_ZYNQMP_NUM_CPUS; i++) {
+        qemu_irq irq;
+
         object_property_set_bool(OBJECT(&s->cpu[i]), true, "realized", &err);
         ERR_PROP_CHECK_RETURN(err, errp);
 
         sysbus_connect_irq(SYS_BUS_DEVICE(&s->gic), i,
                            qdev_get_gpio_in(DEVICE(&s->cpu[i]), ARM_CPU_IRQ));
+        irq = qdev_get_gpio_in(DEVICE(&s->gic),
+                               arm_gic_ppi_index(i, ARM_PHYS_TIMER_PPI));
+        qdev_connect_gpio_out(DEVICE(&s->cpu[i]), 0, irq);
+        irq = qdev_get_gpio_in(DEVICE(&s->gic),
+                               arm_gic_ppi_index(i, ARM_VIRT_TIMER_PPI));
+        qdev_connect_gpio_out(DEVICE(&s->cpu[i]), 1, irq);
     }
 }
 
--

2.3.1.2.g90df61e.dirty

Quoted 13 lines · Peter Crosthwaite

Connect the GPIO outputs from the individual CPUs for the timers to the GIC.

Signed-off-by Peter Crosthwaite <[email protected]>

---
 hw/arm/xlnx-zynqmp.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

hw/arm/xlnx-zynqmp.c +0 -0

index 9465185..29954f5 100644

hw/arm/xlnx-zynqmp.c line 26

Quoted 8 lines

hw/arm/xlnx-zynqmp.c +3 -0

@@ -19,9 +19,17 @@
 
 #define GIC_NUM_SPI_INTR 128
 
+#define ARM_PHYS_TIMER_PPI  30
+#define ARM_VIRT_TIMER_PPI  27
+
 #define GIC_DIST_ADDR       0xf9010000
 #define GIC_CPU_ADDR        0xf9020000

Hey Peter,

I'm wondering if the #define's should be in the header file?

hw/arm/xlnx-zynqmp.c line 30

Quoted 4 lines

hw/arm/xlnx-zynqmp.c +3 -0

+static inline int arm_gic_ppi_index(int cpu_nr, int ppi_index)
+{
+    return GIC_NUM_SPI_INTR + cpu_nr * 32 + ppi_index;

Should the 32 also be a #define? Everything else is.

Thanks,

Alistair

Quoted 29 lines

hw/arm/xlnx-zynqmp.c +10 -1

+}
+
 static void xlnx_zynqmp_init(Object *obj)
 {
     XlnxZynqMPState *s = XLNX_ZYNQMP(obj);

@@ -60,11 +68,19 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
     sysbus_mmio_map(SYS_BUS_DEVICE(&s->gic), 1, GIC_CPU_ADDR);
 
     for (i = 0; i < XLNX_ZYNQMP_NUM_CPUS; i++) {
+        qemu_irq irq;
+
         object_property_set_bool(OBJECT(&s->cpu[i]), true, "realized", &err);
         ERR_PROP_CHECK_RETURN(err, errp);
 
         sysbus_connect_irq(SYS_BUS_DEVICE(&s->gic), i,
                            qdev_get_gpio_in(DEVICE(&s->cpu[i]), ARM_CPU_IRQ));
+        irq = qdev_get_gpio_in(DEVICE(&s->gic),
+                               arm_gic_ppi_index(i, ARM_PHYS_TIMER_PPI));
+        qdev_connect_gpio_out(DEVICE(&s->cpu[i]), 0, irq);
+        irq = qdev_get_gpio_in(DEVICE(&s->gic),
+                               arm_gic_ppi_index(i, ARM_VIRT_TIMER_PPI));
+        qdev_connect_gpio_out(DEVICE(&s->cpu[i]), 1, irq);
     }
 }
 
--

2.3.1.2.g90df61e.dirty

In preparation for migrating the state struct and type cast macro to a public header. The acronym "GEM" on it's own is not specific enough to be used in a more global namespace so preface with "cadence". Fix the capitalisation of "gem" in the state type while touching the typename. Also preface the GEM_MAXREG macro as this will need to migrate to public header.

Reviewed-by Alistair Francis <[email protected]>

Reviewed-by Alex Bennée <[email protected]>

Signed-off-by Peter Crosthwaite <[email protected]>

Quoted 1 line · Peter Crosthwaite

In preparation for migrating the state struct and type cast macro to a public

Quoted 8 lines

header. The acronym "GEM" on it's own is not specific enough to be used in a more global namespace so preface with "cadence". Fix the capitalisation of "gem" in the state type while touching the typename. Also preface the GEM_MAXREG macro as this will need to migrate to public header.

Reviewed-by Alistair Francis <[email protected]>

Reviewed-by Alex Bennée <[email protected]>

Signed-off-by Peter Crosthwaite <[email protected]>

Reviewed-by Peter Maydell <[email protected]>

thanks
-- PMM

To allow using the device with modern SoC programming conventions. The state struct needs to be visible to embed the device in SoC containers.

Reviewed-by Alistair Francis <[email protected]>

Signed-off-by Peter Crosthwaite <[email protected]>

---
changed since v1:
Fix /* Public */ comment spacing (Alistair review)

 hw/net/cadence_gem.c         | 43 +-------------------------------------
 include/hw/net/cadence_gem.h | 49 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 42 deletions(-)
 create mode 100644 include/hw/net/cadence_gem.h

hw/net/cadence_gem.c +1 -42

index 5994306..dafe914 100644
@@ -24,8 +24,7 @@
 
 #include <zlib.h> /* For crc32 */
 
-#include "hw/sysbus.h"
-#include "net/net.h"
+#include "hw/net/cadence_gem.h"
 #include "net/checksum.h"
 
 #ifdef CADENCE_GEM_ERR_DEBUG

@@ -141,8 +140,6 @@
 #define GEM_DESCONF6      (0x00000294/4)
 #define GEM_DESCONF7      (0x00000298/4)
 
-#define CADENCE_GEM_MAXREG        (0x00000640/4) /* Last valid GEM address */
-
 /*****************************************/
 #define GEM_NWCTRL_TXSTART     0x00000200 /* Transmit Enable */
 #define GEM_NWCTRL_TXENA       0x00000008 /* Transmit Enable */

@@ -349,44 +346,6 @@ static inline void rx_desc_set_sar(unsigned *desc, int sar_idx)
     desc[1] |= R_DESC_1_RX_SAR_MATCH;
 }
 
-#define TYPE_CADENCE_GEM "cadence_gem"
-#define CADENCE_GEM(obj) OBJECT_CHECK(CadenceGEMState, (obj), TYPE_CADENCE_GEM)
-
-typedef struct CadenceGEMState {
-    SysBusDevice parent_obj;
-
-    MemoryRegion iomem;
-    NICState *nic;
-    NICConf conf;
-    qemu_irq irq;
-
-    /* GEM registers backing store */
-    uint32_t regs[CADENCE_GEM_MAXREG];
-    /* Mask of register bits which are write only */
-    uint32_t regs_wo[CADENCE_GEM_MAXREG];
-    /* Mask of register bits which are read only */
-    uint32_t regs_ro[CADENCE_GEM_MAXREG];
-    /* Mask of register bits which are clear on read */
-    uint32_t regs_rtc[CADENCE_GEM_MAXREG];
-    /* Mask of register bits which are write 1 to clear */
-    uint32_t regs_w1c[CADENCE_GEM_MAXREG];
-
-    /* PHY registers backing store */
-    uint16_t phy_regs[32];
-
-    uint8_t phy_loop; /* Are we in phy loopback? */
-
-    /* The current DMA descriptor pointers */
-    uint32_t rx_desc_addr;
-    uint32_t tx_desc_addr;
-
-    uint8_t can_rx_state; /* Debug only */
-
-    unsigned rx_desc[2];
-
-    bool sar_active[4];
-} CadenceGEMState;
-
 /* The broadcast MAC address: 0xFFFFFFFFFFFF */
 static const uint8_t broadcast_addr[] = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF };
 

include/hw/net/cadence_gem.h new file +0 -0

new file mode 100644
index 0000000..12de820

Quoted 88 lines · Peter Crosthwaite

To allow using the device with modern SoC programming conventions. The state struct needs to be visible to embed the device in SoC containers.

Reviewed-by Alistair Francis <[email protected]>

Signed-off-by Peter Crosthwaite <[email protected]>

---
changed since v1:
Fix /* Public */ comment spacing (Alistair review)

 hw/net/cadence_gem.c         | 43 +-------------------------------------
 include/hw/net/cadence_gem.h | 49 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 42 deletions(-)
 create mode 100644 include/hw/net/cadence_gem.h

hw/net/cadence_gem.c +1 -42

index 5994306..dafe914 100644
@@ -24,8 +24,7 @@
 
 #include <zlib.h> /* For crc32 */
 
-#include "hw/sysbus.h"
-#include "net/net.h"
+#include "hw/net/cadence_gem.h"
 #include "net/checksum.h"
 
 #ifdef CADENCE_GEM_ERR_DEBUG

@@ -141,8 +140,6 @@
 #define GEM_DESCONF6      (0x00000294/4)
 #define GEM_DESCONF7      (0x00000298/4)
 
-#define CADENCE_GEM_MAXREG        (0x00000640/4) /* Last valid GEM address */
-
 /*****************************************/
 #define GEM_NWCTRL_TXSTART     0x00000200 /* Transmit Enable */
 #define GEM_NWCTRL_TXENA       0x00000008 /* Transmit Enable */

@@ -349,44 +346,6 @@ static inline void rx_desc_set_sar(unsigned *desc, int sar_idx)
     desc[1] |= R_DESC_1_RX_SAR_MATCH;
 }
 
-#define TYPE_CADENCE_GEM "cadence_gem"
-#define CADENCE_GEM(obj) OBJECT_CHECK(CadenceGEMState, (obj), TYPE_CADENCE_GEM)
-
-typedef struct CadenceGEMState {
-    SysBusDevice parent_obj;
-
-    MemoryRegion iomem;
-    NICState *nic;
-    NICConf conf;
-    qemu_irq irq;
-
-    /* GEM registers backing store */
-    uint32_t regs[CADENCE_GEM_MAXREG];
-    /* Mask of register bits which are write only */
-    uint32_t regs_wo[CADENCE_GEM_MAXREG];
-    /* Mask of register bits which are read only */
-    uint32_t regs_ro[CADENCE_GEM_MAXREG];
-    /* Mask of register bits which are clear on read */
-    uint32_t regs_rtc[CADENCE_GEM_MAXREG];
-    /* Mask of register bits which are write 1 to clear */
-    uint32_t regs_w1c[CADENCE_GEM_MAXREG];
-
-    /* PHY registers backing store */
-    uint16_t phy_regs[32];
-
-    uint8_t phy_loop; /* Are we in phy loopback? */
-
-    /* The current DMA descriptor pointers */
-    uint32_t rx_desc_addr;
-    uint32_t tx_desc_addr;
-
-    uint8_t can_rx_state; /* Debug only */
-
-    unsigned rx_desc[2];
-
-    bool sar_active[4];
-} CadenceGEMState;
-
 /* The broadcast MAC address: 0xFFFFFFFFFFFF */
 static const uint8_t broadcast_addr[] = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF };
 

include/hw/net/cadence_gem.h new file +0 -0

new file mode 100644
index 0000000..12de820

include/hw/net/cadence_gem.h line 1

Quoted 1 line

include/hw/net/cadence_gem.h new file +1 -0

@@ -0,0 +1,49 @@
+#ifndef CADENCE_GEM_H_

This new file needs a copyright-and-license comment, and the include guard should lose the trailing '_', but otherwise

Reviewed-by Peter Maydell <[email protected]>

thanks
-- PMM

In preparation for migrating the state struct and type cast macro to a public header. The acronym "UART" on it's own is not specific enough to be used in a more global namespace so preface with "cadence". Fix the capitalisation of "uart" in the state type while touching the typename. Preface macros used by the state struct itself with CADENCE_UART so they don't conflict in namespace either.

Quoted 6 lines · Peter Crosthwaite

In preparation for migrating the state struct and type cast macro to a public header. The acronym "UART" on it's own is not specific enough to be used in a more global namespace so preface with "cadence". Fix the capitalisation of "uart" in the state type while touching the typename. Preface macros used by the state struct itself with CADENCE_UART so they don't conflict in namespace either.

This is another non-standalone commit message. Other than that:

Reviewed-by Peter Maydell <[email protected]>

hw/char/cadence_uart.c typedef struct { · line 140

Quoted 4 lines

hw/char/cadence_uart.c +2 -1

-    s->r[R_SR] |= s->rx_count == RX_FIFO_SIZE ? UART_SR_INTR_RFUL : 0;
+    s->r[R_SR] |= s->rx_count == CADENCE_UART_RX_FIFO_SIZE ? UART_SR_INTR_RFUL
+                                                           : 0;

These look kind of ugly as the line length got long enough to wrap.

    if (s->rx_count == CADENCE_UART_RX_FIFO_SIZE) {
        s->r[R_SR] |= UART_SR_INTR_RFUL;
    }

looks nicer to me (and makes it clearer that the bit only updates if the condition is true, rather than leaving you to figure it out from the combination of the OR and the ternary operator with a zero operand), but that kind of change doesn't belong in this no-content-change patch. You could do it in a later patch, or not do it at all if you don't think it's important enough (I don't care strongly either way).

-- PMM

To allow using the device with modern SoC programming conventions. The state struct needs to be visible to embed the device in SoC containers.

Reviewed-by Alistair Francis <[email protected]>

Signed-off-by Peter Crosthwaite <[email protected]>

Quoted 5 lines · Peter Crosthwaite

To allow using the device with modern SoC programming conventions. The state struct needs to be visible to embed the device in SoC containers.

Reviewed-by Alistair Francis <[email protected]>

Signed-off-by Peter Crosthwaite <[email protected]>

Non-standalone commit message body, header file needs copyright/license comment, include guard shouldn't have trailing _. Otherwise:

Reviewed-by Peter Maydell <[email protected]>

thanks
-- PMM

Add a machine model for the Xilinx ZynqMP SoC EP108 board.

Signed-off-by Peter Crosthwaite <[email protected]>

---
Chaned since v1:
Change board name to ep108

 hw/arm/Makefile.objs |  2 +-
 hw/arm/xlnx-ep108.c  | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 1 deletion(-)
 create mode 100644 hw/arm/xlnx-ep108.c

hw/arm/Makefile.objs +1 -1

index d7cd5f4..a75a182 100644
@@ -10,4 +10,4 @@ obj-$(CONFIG_DIGIC) += digic.o
 obj-y += omap1.o omap2.o strongarm.o
 obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10.o cubieboard.o
 obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o
-obj-$(CONFIG_XLNX_ZYNQMP) += xlnx-zynqmp.o
+obj-$(CONFIG_XLNX_ZYNQMP) += xlnx-zynqmp.o xlnx-ep108.o

hw/arm/xlnx-ep108.c new file +53 -1

new file mode 100644
index 0000000..81704bb
@@ -0,0 +1,53 @@
+/*
+ * Xilinx ZynqMP EP108 board
+ *
+ * Copyright (C) 2015 Xilinx Inc
+ * Written by Peter Crosthwaite <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * for more details.
+ */
+
+#include "hw/arm/xlnx-zynqmp.h"
+#include "hw/boards.h"
+#include "qemu/error-report.h"
+
+typedef struct XlnxEP108 {
+    XlnxZynqMPState soc;
+} XlnxEP108;
+
+static void xlnx_ep108_init(MachineState *machine)
+{
+    XlnxEP108 *s = g_new0(XlnxEP108, 1);
+    Error *err = NULL;
+
+    object_initialize(&s->soc, sizeof(s->soc), TYPE_XLNX_ZYNQMP);
+    object_property_add_child(OBJECT(machine), "soc", OBJECT(&s->soc),
+                              &error_abort);
+
+    object_property_set_bool(OBJECT(&s->soc), true, "realized", &err);
+    if (err) {
+        error_report("%s", error_get_pretty(err));
+        exit(1);
+    }
+}
+
+static QEMUMachine xlnx_ep108_machine = {
+    .name = "xlnx-ep108",
+    .desc = "Xilinx ZynqMP EP108 board",
+    .init = xlnx_ep108_init,
+};
+
+static void xlnx_ep108_machine_init(void)
+{
+    qemu_register_machine(&xlnx_ep108_machine);
+}
+
+machine_init(xlnx_ep108_machine_init);
--

2.3.1.2.g90df61e.dirty

Add a machine model for the Xilinx ZynqMP SoC EP108 board.

Signed-off-by Peter Crosthwaite <[email protected]>

Zynq MPSoC supports external DDR RAM. Add a RAM at 0 to the model.

Signed-off-by Peter Crosthwaite <[email protected]>

---
changed since v1:
Add ram size clamps and warnings

 hw/arm/xlnx-ep108.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

hw/arm/xlnx-ep108.c +21 -1

index 81704bb..6e89456 100644
@@ -18,11 +18,16 @@
 #include "hw/arm/xlnx-zynqmp.h"
 #include "hw/boards.h"
 #include "qemu/error-report.h"
+#include "exec/address-spaces.h"
 
 typedef struct XlnxEP108 {
     XlnxZynqMPState soc;
+    MemoryRegion ddr_ram;
 } XlnxEP108;
 
+/* Max 2GB RAM */
+#define EP108_MAX_RAM_SIZE 0x80000000ull
+
 static void xlnx_ep108_init(MachineState *machine)
 {
     XlnxEP108 *s = g_new0(XlnxEP108, 1);

@@ -37,6 +42,22 @@ static void xlnx_ep108_init(MachineState *machine)
         error_report("%s", error_get_pretty(err));
         exit(1);
     }
+
+    if (machine->ram_size > EP108_MAX_RAM_SIZE) {
+        error_report("WARNING: RAM size " RAM_ADDR_FMT " above max supported, "
+                     "reduced to %llx", machine->ram_size, EP108_MAX_RAM_SIZE);
+        machine->ram_size = EP108_MAX_RAM_SIZE;
+    }
+
+    if (machine->ram_size <= 0x08000000) {
+        error_report("WARNING: RAM size " RAM_ADDR_FMT " is small for EP108\n",
+                     machine->ram_size);
+    }
+
+    memory_region_init_ram(&s->ddr_ram, NULL, "ddr-ram", machine->ram_size,
+                           &error_abort);
+    vmstate_register_ram_global(&s->ddr_ram);
+    memory_region_add_subregion(get_system_memory(), 0, &s->ddr_ram);
 }
 
 static QEMUMachine xlnx_ep108_machine = {
--

2.3.1.2.g90df61e.dirty

Zynq MPSoC supports external DDR RAM. Add a RAM at 0 to the model.

Signed-off-by Peter Crosthwaite <[email protected]>

---
changed since v1:
Add ram size clamps and warnings

 hw/arm/xlnx-ep108.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

hw/arm/xlnx-ep108.c +5 -0

index 81704bb..6e89456 100644
@@ -18,11 +18,16 @@
 #include "hw/arm/xlnx-zynqmp.h"
 #include "hw/boards.h"
 #include "qemu/error-report.h"
+#include "exec/address-spaces.h"
 
 typedef struct XlnxEP108 {
     XlnxZynqMPState soc;
+    MemoryRegion ddr_ram;
 } XlnxEP108;
 
+/* Max 2GB RAM */
+#define EP108_MAX_RAM_SIZE 0x80000000ull
+
 static void xlnx_ep108_init(MachineState *machine)
 {
     XlnxEP108 *s = g_new0(XlnxEP108, 1);

@@ -37,6 +42,22 @@ static void xlnx_ep108_init(MachineState *machine)
         error_report("%s", error_get_pretty(err));
         exit(1);
     }

Quoted 39 lines · Peter Crosthwaite

Zynq MPSoC supports external DDR RAM. Add a RAM at 0 to the model.

Signed-off-by Peter Crosthwaite <[email protected]>

---
changed since v1:
Add ram size clamps and warnings

 hw/arm/xlnx-ep108.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

hw/arm/xlnx-ep108.c +9 -0

index 81704bb..6e89456 100644
@@ -18,11 +18,16 @@
 #include "hw/arm/xlnx-zynqmp.h"
 #include "hw/boards.h"
 #include "qemu/error-report.h"
+#include "exec/address-spaces.h"
 
 typedef struct XlnxEP108 {
     XlnxZynqMPState soc;
+    MemoryRegion ddr_ram;
 } XlnxEP108;
 
+/* Max 2GB RAM */
+#define EP108_MAX_RAM_SIZE 0x80000000ull
+
 static void xlnx_ep108_init(MachineState *machine)
 {
     XlnxEP108 *s = g_new0(XlnxEP108, 1);

@@ -37,6 +42,22 @@ static void xlnx_ep108_init(MachineState *machine)
         error_report("%s", error_get_pretty(err));
         exit(1);
     }
+
+    if (machine->ram_size > EP108_MAX_RAM_SIZE) {
+        error_report("WARNING: RAM size " RAM_ADDR_FMT " above max supported, "
+                     "reduced to %llx", machine->ram_size, EP108_MAX_RAM_SIZE);

hw/arm/xlnx-ep108.c static void xlnx_ep108_init(MachineState *machine) · line 54

Quoted 6 lines

hw/arm/xlnx-ep108.c +6 -0

+        machine->ram_size = EP108_MAX_RAM_SIZE;
+    }
+
+    if (machine->ram_size <= 0x08000000) {
+        error_report("WARNING: RAM size " RAM_ADDR_FMT " is small for EP108\n",
+                     machine->ram_size);

error_report() strings don't need the trailing \n.

hw/arm/xlnx-ep108.c static void xlnx_ep108_init(MachineState *machine) · line 59

Quoted 5 lines

hw/arm/xlnx-ep108.c +5 -0

+    }
+
+    memory_region_init_ram(&s->ddr_ram, NULL, "ddr-ram", machine->ram_size,
+                           &error_abort);
+    vmstate_register_ram_global(&s->ddr_ram);

This should probably be using memory_region_allocate_system_memory().

hw/arm/xlnx-ep108.c static void xlnx_ep108_init(MachineState *machine) · line 63

Quoted 4 lines

hw/arm/xlnx-ep108.c +1 -0

+    memory_region_add_subregion(get_system_memory(), 0, &s->ddr_ram);
 }
 
 static QEMUMachine xlnx_ep108_machine = {

-- PMM

Using standard ARM bootloader.

Signed-off-by Peter Crosthwaite <[email protected]>

---
 hw/arm/xlnx-ep108.c | 8 ++++++++
 1 file changed, 8 insertions(+)

hw/arm/xlnx-ep108.c +8 -1

index 6e89456..a86f595 100644
@@ -28,6 +28,8 @@ typedef struct XlnxEP108 {
 /* Max 2GB RAM */
 #define EP108_MAX_RAM_SIZE 0x80000000ull
 
+static struct arm_boot_info xlnx_ep108_binfo;
+
 static void xlnx_ep108_init(MachineState *machine)
 {
     XlnxEP108 *s = g_new0(XlnxEP108, 1);

@@ -58,6 +60,12 @@ static void xlnx_ep108_init(MachineState *machine)
                            &error_abort);
     vmstate_register_ram_global(&s->ddr_ram);
     memory_region_add_subregion(get_system_memory(), 0, &s->ddr_ram);
+
+    xlnx_ep108_binfo.ram_size = machine->ram_size;
+    xlnx_ep108_binfo.kernel_filename = machine->kernel_filename;
+    xlnx_ep108_binfo.kernel_cmdline = machine->kernel_cmdline;
+    xlnx_ep108_binfo.initrd_filename = machine->initrd_filename;
+    arm_load_kernel(&s->soc.cpu[0], &xlnx_ep108_binfo);
 }
 
 static QEMUMachine xlnx_ep108_machine = {
--

2.3.1.2.g90df61e.dirty

Using standard ARM bootloader.

Signed-off-by Peter Crosthwaite <[email protected]>

---
 hw/arm/xlnx-ep108.c | 8 ++++++++
 1 file changed, 8 insertions(+)

hw/arm/xlnx-ep108.c +2 -0

index 6e89456..a86f595 100644
@@ -28,6 +28,8 @@ typedef struct XlnxEP108 {
 /* Max 2GB RAM */
 #define EP108_MAX_RAM_SIZE 0x80000000ull
 
+static struct arm_boot_info xlnx_ep108_binfo;
+
 static void xlnx_ep108_init(MachineState *machine)
 {
     XlnxEP108 *s = g_new0(XlnxEP108, 1);

hw/arm/xlnx-ep108.c +1 -1

+    arm_load_kernel(&s->soc.cpu[0], &xlnx_ep108_binfo);
 }
 
 static QEMUMachine xlnx_ep108_machine = {
--

2.3.1.2.g90df61e.dirty

Quoted 1 line · Peter Crosthwaite

Using standard ARM bootloader.

Commit msg, etc.

Quoted 13 lines

Signed-off-by Peter Crosthwaite <[email protected]>

---
 hw/arm/xlnx-ep108.c | 8 ++++++++
 1 file changed, 8 insertions(+)

hw/arm/xlnx-ep108.c +0 -0

index 6e89456..a86f595 100644
@@ -28,6 +28,8 @@ typedef struct XlnxEP108 {
 /* Max 2GB RAM */
 #define EP108_MAX_RAM_SIZE 0x80000000ull
 

Quoted 6 lines

hw/arm/xlnx-ep108.c +2 -0

+static struct arm_boot_info xlnx_ep108_binfo;
+
 static void xlnx_ep108_init(MachineState *machine)
 {
     XlnxEP108 *s = g_new0(XlnxEP108, 1);

hw/arm/xlnx-ep108.c static void xlnx_ep108_init(MachineState *machine) · line 67

Quoted 8 lines

hw/arm/xlnx-ep108.c +5 -0

@@ -58,6 +60,12 @@ static void xlnx_ep108_init(MachineState *machine)
                            &error_abort);
     vmstate_register_ram_global(&s->ddr_ram);
     memory_region_add_subregion(get_system_memory(), 0, &s->ddr_ram);
+
+    xlnx_ep108_binfo.ram_size = machine->ram_size;
+    xlnx_ep108_binfo.kernel_filename = machine->kernel_filename;
+    xlnx_ep108_binfo.kernel_cmdline = machine->kernel_cmdline;
+    xlnx_ep108_binfo.initrd_filename = machine->initrd_filename;

nb_cpus, loader_start ?

Quoted 6 lines

hw/arm/xlnx-ep108.c +1 -1

+    arm_load_kernel(&s->soc.cpu[0], &xlnx_ep108_binfo);
 }
 
 static QEMUMachine xlnx_ep108_machine = {
--

2.3.1.2.g90df61e.dirty

thanks
-- PMM

Quoted 19 lines · Peter Maydell

Quoted 1 line · Peter Crosthwaite

Using standard ARM bootloader.

Commit msg, etc.

Quoted 13 lines

Signed-off-by Peter Crosthwaite <[email protected]>

---
 hw/arm/xlnx-ep108.c | 8 ++++++++
 1 file changed, 8 insertions(+)

hw/arm/xlnx-ep108.c +0 -0

index 6e89456..a86f595 100644
@@ -28,6 +28,8 @@ typedef struct XlnxEP108 {
 /* Max 2GB RAM */
 #define EP108_MAX_RAM_SIZE 0x80000000ull
 

Quoted 17 lines

Quoted 6 lines

hw/arm/xlnx-ep108.c +2 -0

+static struct arm_boot_info xlnx_ep108_binfo;
+
 static void xlnx_ep108_init(MachineState *machine)
 {
     XlnxEP108 *s = g_new0(XlnxEP108, 1);

hw/arm/xlnx-ep108.c static void xlnx_ep108_init(MachineState *machine) · line 67

Quoted 8 lines

hw/arm/xlnx-ep108.c +5 -0

@@ -58,6 +60,12 @@ static void xlnx_ep108_init(MachineState *machine)
                            &error_abort);
     vmstate_register_ram_global(&s->ddr_ram);
     memory_region_add_subregion(get_system_memory(), 0, &s->ddr_ram);
+
+    xlnx_ep108_binfo.ram_size = machine->ram_size;
+    xlnx_ep108_binfo.kernel_filename = machine->kernel_filename;
+    xlnx_ep108_binfo.kernel_cmdline = machine->kernel_cmdline;
+    xlnx_ep108_binfo.initrd_filename = machine->initrd_filename;

nb_cpus, loader_start ?

I don't see any valid use for non-0 nb_cpus for aarch64 bootloading. I think PSCI just handles it all? default_write_secondary will write in smpboot contents which is AA32.

Loader start is 0. I'll make it explicit.

Regards,
Peter

Quoted 10 lines

Quoted 6 lines

hw/arm/xlnx-ep108.c +1 -1

+    arm_load_kernel(&s->soc.cpu[0], &xlnx_ep108_binfo);
 }
 
 static QEMUMachine xlnx_ep108_machine = {
--

2.3.1.2.g90df61e.dirty

thanks
-- PMM

Use SMC PSCI, with the standard policy of secondaries starting in power-off.

Reviewed-by Alistair Francis <[email protected]>

Signed-off-by Peter Crosthwaite <[email protected]>

---
changed since v1:
Add &error_abort to property setter calls

 hw/arm/xlnx-zynqmp.c | 8 ++++++++
 1 file changed, 8 insertions(+)

hw/arm/xlnx-zynqmp.c +0 -0

index e015025..0265fba 100644

Quoted 20 lines · Peter Crosthwaite

Use SMC PSCI, with the standard policy of secondaries starting in power-off.

Reviewed-by Alistair Francis <[email protected]>

Signed-off-by Peter Crosthwaite <[email protected]>

---
changed since v1:
Add &error_abort to property setter calls

 hw/arm/xlnx-zynqmp.c | 8 ++++++++
 1 file changed, 8 insertions(+)

hw/arm/xlnx-zynqmp.c +0 -0

index e015025..0265fba 100644
@@ -97,6 +97,14 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
     for (i = 0; i < XLNX_ZYNQMP_NUM_CPUS; i++) {
         qemu_irq irq;
 

hw/arm/xlnx-zynqmp.c static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp) · line 106

Quoted 7 lines

hw/arm/xlnx-zynqmp.c +7 -0

+        object_property_set_int(OBJECT(&s->cpu[i]), QEMU_PSCI_CONDUIT_SMC,
+                                "psci-conduit", &error_abort);
+        if (i > 0) {
+            /* Secondary CPUs start in PSCI powered-down state */
+            object_property_set_bool(OBJECT(&s->cpu[i]), true,
+                                     "start-powered-off", &error_abort);
+        }

...oh, here's the PSCI config. I think you might as well have rolled this into the initial "create the CPUs" patch.

-- PMM

Hi Peter and all,

Xilinx's next gen SoC has been announced. This series adds a SoC and board.

Series start with addition of ARM cortex A53 support (P1 and P2). The Soc skeleton is then added with GIC, EMACs and UARTs are added. The pre-existing models for GEM and UART are not SoC friendly (no visible state struct), so those are refactored for SoC.

Create a model of the EP108 board. Currently this doesn't have any EP108 specific features but is a usable board exposing the user visible features of the raw SoC.

changed since v3:
Included CPU thread kick fix
Addressed Alistair review

changed since v2:
Fix CPU child prop adder
Add DTS compat string

changed since v1:
Addressed Alistair review (individual changes on resp. patches) Changed board name to EP108
Changed naming scheme to "zynqmp" / "ZYNQMP" (Michal review)

Regards,
Peter

Peter Crosthwaite (16):
cpus: Don't kick un-realized cpus.
target-arm: cpu64: Factor out ARM cortex init
target-arm: cpu64: Add support for cortex-a53
arm: Introduce Xilinx ZynqMP SoC
arm: xlnx-zynqmp: Add GIC
arm: xlnx-zynqmp: Connect CPU Timers to GIC
net: cadence_gem: Clean up variable names
net: cadence_gem: Split state struct and type into header
arm: xilinx-zynqmp: Add GEM support
char: cadence_uart: Clean up variable names
char: cadence_uart: Split state struct and type into header
arm: xilinx-zynqmp: Add UART support
arm: Add xlnx-ep108 machine
arm: xilinx-ep108: Add external RAM
arm: xilinx-ep108: Add bootloading
arm: xlnx-zynqmp: Add PSCI setup

 cpus.c                              |   2 +-
 default-configs/aarch64-softmmu.mak |   2 +-
 hw/arm/Makefile.objs                |   1 +
 hw/arm/xlnx-ep108.c                 |  82 ++++++++++++++++++
 hw/arm/xlnx-zynqmp.c                | 168 ++++++++++++++++++++++++++++++++++++
 hw/char/cadence_uart.c              | 115 ++++++++++--------------
 hw/net/cadence_gem.c                |  95 ++++++--------------
 include/hw/arm/xlnx-zynqmp.h        |  29 +++++++
 include/hw/char/cadence_uart.h      |  35 ++++++++
 include/hw/net/cadence_gem.h        |  49 +++++++++++
 target-arm/cpu64.c                  |  50 ++++++++---
 11 files changed, 476 insertions(+), 152 deletions(-)
 create mode 100644 hw/arm/xlnx-ep108.c
 create mode 100644 hw/arm/xlnx-zynqmp.c
 create mode 100644 include/hw/arm/xlnx-zynqmp.h
 create mode 100644 include/hw/char/cadence_uart.h
 create mode 100644 include/hw/net/cadence_gem.h

--
2.3.1.2.g90df61e.dirty

There are 4x Cadence GEMs in ZynqMP. Add them.

Signed-off-by Peter Crosthwaite <[email protected]>

Quoted 3 lines · Peter Crosthwaite

There are 4x Cadence GEMs in ZynqMP. Add them.

Signed-off-by Peter Crosthwaite <[email protected]>

Reviewed-by Peter Maydell <[email protected]>

(other than the use of that error-check macro...)

thanks
-- PMM

There are 2x Cadence UARTs in Zynq MP. Add them.

Reviewed-by Alistair Francis <[email protected]>

Signed-off-by Peter Crosthwaite <[email protected]>