[Qemu-devel] [PATCH target-arm v1 00/15] Next Generation Xilinx Zynq SoC

[Qemu-devel] [PATCH target-arm v1 00/15] Next Generation Xilinx Zynq SoC

[Peter Crosthwaite]

Hi Peter and all,

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

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 generic machine model that exposes just the RAW SoC itself. The
only external device modelled is DDR RAM, as driven by the -m option.
The standard bootloader and PSCI support is used.

Regards,
Peter


Peter Crosthwaite (15):
  target-arm: cpu64: Factor out ARM cortex init
  target-arm: cpu64: Add support for cortex-a53
  arm: Introduce Xilinx Zynq MPSoC
  arm: xlnx-zynq-mp: Add GIC
  arm: xlnx-zynq-mp: Connect CPU Timers to GIC
  net: cadence_gem: Clean up variable names
  net: cadence_gem: Split state struct and type into header
  arm: xilinx-zynq-mp: Add GEM support
  char: cadence_uart: Clean up variable names
  char: cadence_uart: Split state struct and type into header
  arm: xilinx-zynq-mp: Add UART support
  arm: Add xilinx-zynq-mp-generic machine
  arm: xilinx-zynq-mp-generic: Add external RAM
  arm: xilinx-zynq-mp-generic: Add bootloading
  arm: xlnx-zynq-mp: Add PSCI setup

 default-configs/aarch64-softmmu.mak |   2 +-
 hw/arm/Makefile.objs                |   1 +
 hw/arm/xlnx-zynq-mp-generic.c       |  67 +++++++++++++++
 hw/arm/xlnx-zynq-mp.c               | 167 ++++++++++++++++++++++++++++++++++++
 hw/char/cadence_uart.c              | 113 ++++++++++--------------
 hw/net/cadence_gem.c                |  95 ++++++--------------
 include/hw/arm/xlnx-zynq-mp.h       |  29 +++++++
 include/hw/char/cadence_uart.h      |  35 ++++++++
 include/hw/net/cadence_gem.h        |  49 +++++++++++
 target-arm/cpu64.c                  |  47 +++++++---
 10 files changed, 456 insertions(+), 149 deletions(-)
 create mode 100644 hw/arm/xlnx-zynq-mp-generic.c
 create mode 100644 hw/arm/xlnx-zynq-mp.c
 create mode 100644 include/hw/arm/xlnx-zynq-mp.h
 create mode 100644 include/hw/char/cadence_uart.h
 create mode 100644 include/hw/net/cadence_gem.h

-- 
2.3.0.1.g27a12f1

[Qemu-devel] [PATCH target-arm v1 01/15] target-arm: cpu64: Factor out ARM cortex init

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

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
 target-arm/cpu64.c | 32 +++++++++++++++++++-------------
 1 file changed, 19 insertions(+), 13 deletions(-)

diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c
index 823c739..5cf3121 100644
--- a/target-arm/cpu64.c
+++ b/target-arm/cpu64.c
@@ -38,22 +38,22 @@ static inline void unset_feature(CPUARMState *env, int feature)
 }
 
 #ifndef CONFIG_USER_ONLY
-static uint64_t a57_l2ctlr_read(CPUARMState *env, const ARMCPRegInfo *ri)
+static uint64_t axx_l2ctlr_read(CPUARMState *env, const ARMCPRegInfo *ri)
 {
     /* Number of processors is in [25:24]; otherwise we RAZ */
     return (smp_cpus - 1) << 24;
 }
 #endif
 
-static const ARMCPRegInfo cortexa57_cp_reginfo[] = {
+static const ARMCPRegInfo cortexaxx_cp_reginfo[] = {
 #ifndef CONFIG_USER_ONLY
     { .name = "L2CTLR_EL1", .state = ARM_CP_STATE_AA64,
       .opc0 = 3, .opc1 = 1, .crn = 11, .crm = 0, .opc2 = 2,
-      .access = PL1_RW, .readfn = a57_l2ctlr_read,
+      .access = PL1_RW, .readfn = axx_l2ctlr_read,
       .writefn = arm_cp_write_ignore },
     { .name = "L2CTLR",
       .cp = 15, .opc1 = 1, .crn = 9, .crm = 0, .opc2 = 2,
-      .access = PL1_RW, .readfn = a57_l2ctlr_read,
+      .access = PL1_RW, .readfn = axx_l2ctlr_read,
       .writefn = arm_cp_write_ignore },
 #endif
     { .name = "L2ECTLR_EL1", .state = ARM_CP_STATE_AA64,
@@ -92,10 +92,8 @@ static const ARMCPRegInfo cortexa57_cp_reginfo[] = {
     REGINFO_SENTINEL
 };
 
-static void aarch64_a57_initfn(Object *obj)
+static void aarch64_axx_initfn(ARMCPU *cpu)
 {
-    ARMCPU *cpu = ARM_CPU(obj);
-
     set_feature(&cpu->env, ARM_FEATURE_V8);
     set_feature(&cpu->env, ARM_FEATURE_VFP4);
     set_feature(&cpu->env, ARM_FEATURE_NEON);
@@ -107,13 +105,10 @@ static void aarch64_a57_initfn(Object *obj)
     set_feature(&cpu->env, ARM_FEATURE_V8_SHA256);
     set_feature(&cpu->env, ARM_FEATURE_V8_PMULL);
     set_feature(&cpu->env, ARM_FEATURE_CRC);
-    cpu->kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A57;
-    cpu->midr = 0x411fd070;
     cpu->reset_fpsid = 0x41034070;
     cpu->mvfr0 = 0x10110222;
     cpu->mvfr1 = 0x12111111;
     cpu->mvfr2 = 0x00000043;
-    cpu->ctr = 0x8444c004;
     cpu->reset_sctlr = 0x00c50838;
     cpu->id_pfr0 = 0x00000131;
     cpu->id_pfr1 = 0x00011011;
@@ -132,14 +127,25 @@ static void aarch64_a57_initfn(Object *obj)
     cpu->id_aa64pfr0 = 0x00002222;
     cpu->id_aa64dfr0 = 0x10305106;
     cpu->id_aa64isar0 = 0x00011120;
-    cpu->id_aa64mmfr0 = 0x00001124;
     cpu->dbgdidr = 0x3516d000;
     cpu->clidr = 0x0a200023;
+    cpu->dcz_blocksize = 4; /* 64 bytes */
+    define_arm_cp_regs(cpu, cortexaxx_cp_reginfo);
+}
+
+static void aarch64_a57_initfn(Object *obj)
+{
+    ARMCPU *cpu = ARM_CPU(obj);
+
+    aarch64_axx_initfn(cpu);
+
+    cpu->kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A57;
+    cpu->midr = 0x411fd070;
+    cpu->ctr = 0x8444c004; /* L1Ip = PIPT */
+    cpu->id_aa64mmfr0 = 0x00001124; /* 44 bit physical addr */
     cpu->ccsidr[0] = 0x701fe00a; /* 32KB L1 dcache */
     cpu->ccsidr[1] = 0x201fe012; /* 48KB L1 icache */
     cpu->ccsidr[2] = 0x70ffe07a; /* 2048KB L2 cache */
-    cpu->dcz_blocksize = 4; /* 64 bytes */
-    define_arm_cp_regs(cpu, cortexa57_cp_reginfo);
 }
 
 #ifdef CONFIG_USER_ONLY
-- 
2.3.0.1.g27a12f1

[Qemu-devel] [PATCH target-arm v1 02/15] target-arm: cpu64: Add support for cortex-a53

[Peter Crosthwaite]

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.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
 target-arm/cpu64.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c
index 5cf3121..0b9728e 100644
--- a/target-arm/cpu64.c
+++ b/target-arm/cpu64.c
@@ -148,6 +148,20 @@ 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->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)
 {
@@ -175,6 +189,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.0.1.g27a12f1

[Qemu-devel] [PATCH target-arm v1 03/15] arm: Introduce Xilinx Zynq MPSoC

[Peter Crosthwaite]

With quad Cortex-A53 CPUs.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
 default-configs/aarch64-softmmu.mak |  2 +-
 hw/arm/Makefile.objs                |  1 +
 hw/arm/xlnx-zynq-mp.c               | 71 +++++++++++++++++++++++++++++++++++++
 include/hw/arm/xlnx-zynq-mp.h       | 21 +++++++++++
 4 files changed, 94 insertions(+), 1 deletion(-)
 create mode 100644 hw/arm/xlnx-zynq-mp.c
 create mode 100644 include/hw/arm/xlnx-zynq-mp.h

diff --git a/default-configs/aarch64-softmmu.mak b/default-configs/aarch64-softmmu.mak
index 6d3b5c7..a8011e0 100644
--- a/default-configs/aarch64-softmmu.mak
+++ b/default-configs/aarch64-softmmu.mak
@@ -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_ZYNQ_MP=y
diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index 6088e53..9bf072b 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -8,3 +8,4 @@ obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o
 obj-$(CONFIG_DIGIC) += digic.o
 obj-y += omap1.o omap2.o strongarm.o
 obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10.o cubieboard.o
+obj-$(CONFIG_XLNX_ZYNQ_MP) += xlnx-zynq-mp.o
diff --git a/hw/arm/xlnx-zynq-mp.c b/hw/arm/xlnx-zynq-mp.c
new file mode 100644
index 0000000..d553fb0
--- /dev/null
+++ b/hw/arm/xlnx-zynq-mp.c
@@ -0,0 +1,71 @@
+/*
+ * Xilinx Zynq MPSoC emulation
+ *
+ * Copyright (C) 2015 Xilinx Inc
+ * Written by Peter Crosthwaite <peter.crosthwaite@xilinx.com>
+ *
+ * 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-zynq-mp.h"
+
+static void xlnx_zynq_mp_init(Object *obj)
+{
+    XlnxZynqMPState *s = XLNX_ZYNQ_MP(obj);
+    int i;
+
+    for (i = 0; i < XLNX_ZYNQ_MP_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]), NULL);
+    }
+}
+
+#define ERR_PROP_CHECK_RETURN(err, errp) do { \
+    if (err) { \
+        error_propagate((errp), (err)); \
+        return; \
+    } \
+} while (0)
+
+static void xlnx_zynq_mp_realize(DeviceState *dev, Error **errp)
+{
+    XlnxZynqMPState *s = XLNX_ZYNQ_MP(dev);
+    uint8_t i;
+    Error *err = NULL;
+
+    for (i = 0; i < XLNX_ZYNQ_MP_NUM_CPUS; i++) {
+        object_property_set_bool(OBJECT(&s->cpu[i]), true, "realized", &err);
+        ERR_PROP_CHECK_RETURN(err, errp);
+    }
+}
+
+static void xlnx_zynq_mp_class_init(ObjectClass *oc, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(oc);
+
+    dc->realize = xlnx_zynq_mp_realize;
+}
+
+static const TypeInfo xlnx_zynq_mp_type_info = {
+    .name = TYPE_XLNX_ZYNQ_MP,
+    .parent = TYPE_DEVICE,
+    .instance_size = sizeof(XlnxZynqMPState),
+    .instance_init = xlnx_zynq_mp_init,
+    .class_init = xlnx_zynq_mp_class_init,
+};
+
+static void xlnx_zynq_mp_register_types(void)
+{
+    type_register_static(&xlnx_zynq_mp_type_info);
+}
+
+type_init(xlnx_zynq_mp_register_types)
diff --git a/include/hw/arm/xlnx-zynq-mp.h b/include/hw/arm/xlnx-zynq-mp.h
new file mode 100644
index 0000000..f7410dc
--- /dev/null
+++ b/include/hw/arm/xlnx-zynq-mp.h
@@ -0,0 +1,21 @@
+#ifndef XLNX_ZYNQ_MP_H_
+
+#include "qemu-common.h"
+#include "hw/arm/arm.h"
+
+#define TYPE_XLNX_ZYNQ_MP "xlnx,zynq-mp"
+#define XLNX_ZYNQ_MP(obj) OBJECT_CHECK(XlnxZynqMPState, (obj), \
+                                       TYPE_XLNX_ZYNQ_MP)
+
+#define XLNX_ZYNQ_MP_NUM_CPUS 4
+
+typedef struct XlnxZynqMPState {
+    /*< private >*/
+    DeviceState parent_obj;
+    /*< public >*/
+
+    ARMCPU cpu[XLNX_ZYNQ_MP_NUM_CPUS];
+}  XlnxZynqMPState;
+
+#define XLNX_ZYNQ_MP_H_
+#endif
-- 
2.3.0.1.g27a12f1

[Qemu-devel] [PATCH target-arm v1 04/15] arm: xlnx-zynq-mp: Add GIC

[Peter Crosthwaite]

And connect IRQ outputs to the CPUs.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
 hw/arm/xlnx-zynq-mp.c         | 19 +++++++++++++++++++
 include/hw/arm/xlnx-zynq-mp.h |  2 ++
 2 files changed, 21 insertions(+)

diff --git a/hw/arm/xlnx-zynq-mp.c b/hw/arm/xlnx-zynq-mp.c
index d553fb0..9cdff13 100644
--- a/hw/arm/xlnx-zynq-mp.c
+++ b/hw/arm/xlnx-zynq-mp.c
@@ -17,6 +17,11 @@
 
 #include "hw/arm/xlnx-zynq-mp.h"
 
+#define GIC_NUM_SPI_INTR 128
+
+#define GIC_DIST_ADDR       0xf9010000
+#define GIC_CPU_ADDR        0xf9020000
+
 static void xlnx_zynq_mp_init(Object *obj)
 {
     XlnxZynqMPState *s = XLNX_ZYNQ_MP(obj);
@@ -27,6 +32,9 @@ static void xlnx_zynq_mp_init(Object *obj)
                           "cortex-a53-" TYPE_ARM_CPU);
         object_property_add_child(obj, "cpu", OBJECT(&s->cpu[i]), NULL);
     }
+
+    object_initialize(&s->gic, sizeof(s->gic), TYPE_ARM_GIC);
+    qdev_set_parent_bus(DEVICE(&s->gic), sysbus_get_default());
 }
 
 #define ERR_PROP_CHECK_RETURN(err, errp) do { \
@@ -42,9 +50,20 @@ static void xlnx_zynq_mp_realize(DeviceState *dev, Error **errp)
     uint8_t i;
     Error *err = NULL;
 
+    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_ZYNQ_MP_NUM_CPUS);
+    object_property_set_bool(OBJECT(&s->gic), true, "realized", &err);
+    ERR_PROP_CHECK_RETURN(err, errp);
+    sysbus_mmio_map(SYS_BUS_DEVICE(&s->gic), 0, GIC_DIST_ADDR);
+    sysbus_mmio_map(SYS_BUS_DEVICE(&s->gic), 1, GIC_CPU_ADDR);
+
     for (i = 0; i < XLNX_ZYNQ_MP_NUM_CPUS; i++) {
         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));
     }
 }
 
diff --git a/include/hw/arm/xlnx-zynq-mp.h b/include/hw/arm/xlnx-zynq-mp.h
index f7410dc..22b2af0 100644
--- a/include/hw/arm/xlnx-zynq-mp.h
+++ b/include/hw/arm/xlnx-zynq-mp.h
@@ -2,6 +2,7 @@
 
 #include "qemu-common.h"
 #include "hw/arm/arm.h"
+#include "hw/intc/arm_gic.h"
 
 #define TYPE_XLNX_ZYNQ_MP "xlnx,zynq-mp"
 #define XLNX_ZYNQ_MP(obj) OBJECT_CHECK(XlnxZynqMPState, (obj), \
@@ -15,6 +16,7 @@ typedef struct XlnxZynqMPState {
     /*< public >*/
 
     ARMCPU cpu[XLNX_ZYNQ_MP_NUM_CPUS];
+    GICState gic;
 }  XlnxZynqMPState;
 
 #define XLNX_ZYNQ_MP_H_
-- 
2.3.0.1.g27a12f1

[Qemu-devel] [PATCH target-arm v1 05/15] arm: xlnx-zynq-mp: Connect CPU Timers to GIC

[Peter Crosthwaite]

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

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
 hw/arm/xlnx-zynq-mp.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/hw/arm/xlnx-zynq-mp.c b/hw/arm/xlnx-zynq-mp.c
index 9cdff13..be82a66 100644
--- a/hw/arm/xlnx-zynq-mp.c
+++ b/hw/arm/xlnx-zynq-mp.c
@@ -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
 
+static inline int arm_gic_ppi_index(int cpu_nr, int ppi_index)
+{
+    return GIC_NUM_SPI_INTR + cpu_nr * 32 + ppi_index;
+}
+
 static void xlnx_zynq_mp_init(Object *obj)
 {
     XlnxZynqMPState *s = XLNX_ZYNQ_MP(obj);
@@ -59,11 +67,19 @@ static void xlnx_zynq_mp_realize(DeviceState *dev, Error **errp)
     sysbus_mmio_map(SYS_BUS_DEVICE(&s->gic), 1, GIC_CPU_ADDR);
 
     for (i = 0; i < XLNX_ZYNQ_MP_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.0.1.g27a12f1

[Qemu-devel] [PATCH target-arm v1 06/15] net: cadence_gem: Clean up variable names

[Peter Crosthwaite]

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.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
 hw/net/cadence_gem.c | 70 ++++++++++++++++++++++++++--------------------------
 1 file changed, 35 insertions(+), 35 deletions(-)

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 55b6293..5994306 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -141,7 +141,7 @@
 #define GEM_DESCONF6      (0x00000294/4)
 #define GEM_DESCONF7      (0x00000298/4)
 
-#define GEM_MAXREG        (0x00000640/4) /* Last valid GEM address */
+#define CADENCE_GEM_MAXREG        (0x00000640/4) /* Last valid GEM address */
 
 /*****************************************/
 #define GEM_NWCTRL_TXSTART     0x00000200 /* Transmit Enable */
@@ -350,9 +350,9 @@ static inline void rx_desc_set_sar(unsigned *desc, int sar_idx)
 }
 
 #define TYPE_CADENCE_GEM "cadence_gem"
-#define GEM(obj) OBJECT_CHECK(GemState, (obj), TYPE_CADENCE_GEM)
+#define CADENCE_GEM(obj) OBJECT_CHECK(CadenceGEMState, (obj), TYPE_CADENCE_GEM)
 
-typedef struct GemState {
+typedef struct CadenceGEMState {
     SysBusDevice parent_obj;
 
     MemoryRegion iomem;
@@ -361,15 +361,15 @@ typedef struct GemState {
     qemu_irq irq;
 
     /* GEM registers backing store */
-    uint32_t regs[GEM_MAXREG];
+    uint32_t regs[CADENCE_GEM_MAXREG];
     /* Mask of register bits which are write only */
-    uint32_t regs_wo[GEM_MAXREG];
+    uint32_t regs_wo[CADENCE_GEM_MAXREG];
     /* Mask of register bits which are read only */
-    uint32_t regs_ro[GEM_MAXREG];
+    uint32_t regs_ro[CADENCE_GEM_MAXREG];
     /* Mask of register bits which are clear on read */
-    uint32_t regs_rtc[GEM_MAXREG];
+    uint32_t regs_rtc[CADENCE_GEM_MAXREG];
     /* Mask of register bits which are write 1 to clear */
-    uint32_t regs_w1c[GEM_MAXREG];
+    uint32_t regs_w1c[CADENCE_GEM_MAXREG];
 
     /* PHY registers backing store */
     uint16_t phy_regs[32];
@@ -385,7 +385,7 @@ typedef struct GemState {
     unsigned rx_desc[2];
 
     bool sar_active[4];
-} GemState;
+} CadenceGEMState;
 
 /* The broadcast MAC address: 0xFFFFFFFFFFFF */
 static const uint8_t broadcast_addr[] = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF };
@@ -395,7 +395,7 @@ static const uint8_t broadcast_addr[] = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF };
  * One time initialization.
  * Set masks to identify which register bits have magical clear properties
  */
-static void gem_init_register_masks(GemState *s)
+static void gem_init_register_masks(CadenceGEMState *s)
 {
     /* Mask of register bits which are read only */
     memset(&s->regs_ro[0], 0, sizeof(s->regs_ro));
@@ -430,7 +430,7 @@ static void gem_init_register_masks(GemState *s)
  * phy_update_link:
  * Make the emulated PHY link state match the QEMU "interface" state.
  */
-static void phy_update_link(GemState *s)
+static void phy_update_link(CadenceGEMState *s)
 {
     DB_PRINT("down %d\n", qemu_get_queue(s->nic)->link_down);
 
@@ -450,7 +450,7 @@ static void phy_update_link(GemState *s)
 
 static int gem_can_receive(NetClientState *nc)
 {
-    GemState *s;
+    CadenceGEMState *s;
 
     s = qemu_get_nic_opaque(nc);
 
@@ -483,7 +483,7 @@ static int gem_can_receive(NetClientState *nc)
  * gem_update_int_status:
  * Raise or lower interrupt based on current status.
  */
-static void gem_update_int_status(GemState *s)
+static void gem_update_int_status(CadenceGEMState *s)
 {
     if (s->regs[GEM_ISR]) {
         DB_PRINT("asserting int. (0x%08x)\n", s->regs[GEM_ISR]);
@@ -495,7 +495,7 @@ static void gem_update_int_status(GemState *s)
  * gem_receive_updatestats:
  * Increment receive statistics.
  */
-static void gem_receive_updatestats(GemState *s, const uint8_t *packet,
+static void gem_receive_updatestats(CadenceGEMState *s, const uint8_t *packet,
                                     unsigned bytes)
 {
     uint64_t octets;
@@ -586,7 +586,7 @@ static unsigned calc_mac_hash(const uint8_t *mac)
  * GEM_RM_PROMISCUOUS_ACCEPT, GEM_RX_BROADCAST_ACCEPT,
  * GEM_RX_MULTICAST_HASH_ACCEPT or GEM_RX_UNICAST_HASH_ACCEPT
  */
-static int gem_mac_address_filter(GemState *s, const uint8_t *packet)
+static int gem_mac_address_filter(CadenceGEMState *s, const uint8_t *packet)
 {
     uint8_t *gem_spaddr;
     int i;
@@ -636,7 +636,7 @@ static int gem_mac_address_filter(GemState *s, const uint8_t *packet)
     return GEM_RX_REJECT;
 }
 
-static void gem_get_rx_desc(GemState *s)
+static void gem_get_rx_desc(CadenceGEMState *s)
 {
     DB_PRINT("read descriptor 0x%x\n", (unsigned)s->rx_desc_addr);
     /* read current descriptor */
@@ -660,7 +660,7 @@ static void gem_get_rx_desc(GemState *s)
  */
 static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
 {
-    GemState *s;
+    CadenceGEMState *s;
     unsigned   rxbufsize, bytes_to_copy;
     unsigned   rxbuf_offset;
     uint8_t    rxbuf[2048];
@@ -810,7 +810,7 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
  * gem_transmit_updatestats:
  * Increment transmit statistics.
  */
-static void gem_transmit_updatestats(GemState *s, const uint8_t *packet,
+static void gem_transmit_updatestats(CadenceGEMState *s, const uint8_t *packet,
                                      unsigned bytes)
 {
     uint64_t octets;
@@ -856,7 +856,7 @@ static void gem_transmit_updatestats(GemState *s, const uint8_t *packet,
  * gem_transmit:
  * Fish packets out of the descriptor ring and feed them to QEMU
  */
-static void gem_transmit(GemState *s)
+static void gem_transmit(CadenceGEMState *s)
 {
     unsigned    desc[2];
     hwaddr packet_desc_addr;
@@ -976,7 +976,7 @@ static void gem_transmit(GemState *s)
     }
 }
 
-static void gem_phy_reset(GemState *s)
+static void gem_phy_reset(CadenceGEMState *s)
 {
     memset(&s->phy_regs[0], 0, sizeof(s->phy_regs));
     s->phy_regs[PHY_REG_CONTROL] = 0x1140;
@@ -1004,7 +1004,7 @@ static void gem_phy_reset(GemState *s)
 static void gem_reset(DeviceState *d)
 {
     int i;
-    GemState *s = GEM(d);
+    CadenceGEMState *s = CADENCE_GEM(d);
 
     DB_PRINT("\n");
 
@@ -1032,13 +1032,13 @@ static void gem_reset(DeviceState *d)
     gem_update_int_status(s);
 }
 
-static uint16_t gem_phy_read(GemState *s, unsigned reg_num)
+static uint16_t gem_phy_read(CadenceGEMState *s, unsigned reg_num)
 {
     DB_PRINT("reg: %d value: 0x%04x\n", reg_num, s->phy_regs[reg_num]);
     return s->phy_regs[reg_num];
 }
 
-static void gem_phy_write(GemState *s, unsigned reg_num, uint16_t val)
+static void gem_phy_write(CadenceGEMState *s, unsigned reg_num, uint16_t val)
 {
     DB_PRINT("reg: %d value: 0x%04x\n", reg_num, val);
 
@@ -1072,10 +1072,10 @@ static void gem_phy_write(GemState *s, unsigned reg_num, uint16_t val)
  */
 static uint64_t gem_read(void *opaque, hwaddr offset, unsigned size)
 {
-    GemState *s;
+    CadenceGEMState *s;
     uint32_t retval;
 
-    s = (GemState *)opaque;
+    s = (CadenceGEMState *)opaque;
 
     offset >>= 2;
     retval = s->regs[offset];
@@ -1120,7 +1120,7 @@ static uint64_t gem_read(void *opaque, hwaddr offset, unsigned size)
 static void gem_write(void *opaque, hwaddr offset, uint64_t val,
         unsigned size)
 {
-    GemState *s = (GemState *)opaque;
+    CadenceGEMState *s = (CadenceGEMState *)opaque;
     uint32_t readonly;
 
     DB_PRINT("offset: 0x%04x write: 0x%08x ", (unsigned)offset, (unsigned)val);
@@ -1226,7 +1226,7 @@ static NetClientInfo net_gem_info = {
 static int gem_init(SysBusDevice *sbd)
 {
     DeviceState *dev = DEVICE(sbd);
-    GemState *s = GEM(dev);
+    CadenceGEMState *s = CADENCE_GEM(dev);
 
     DB_PRINT("\n");
 
@@ -1248,18 +1248,18 @@ static const VMStateDescription vmstate_cadence_gem = {
     .version_id = 2,
     .minimum_version_id = 2,
     .fields = (VMStateField[]) {
-        VMSTATE_UINT32_ARRAY(regs, GemState, GEM_MAXREG),
-        VMSTATE_UINT16_ARRAY(phy_regs, GemState, 32),
-        VMSTATE_UINT8(phy_loop, GemState),
-        VMSTATE_UINT32(rx_desc_addr, GemState),
-        VMSTATE_UINT32(tx_desc_addr, GemState),
-        VMSTATE_BOOL_ARRAY(sar_active, GemState, 4),
+        VMSTATE_UINT32_ARRAY(regs, CadenceGEMState, CADENCE_GEM_MAXREG),
+        VMSTATE_UINT16_ARRAY(phy_regs, CadenceGEMState, 32),
+        VMSTATE_UINT8(phy_loop, CadenceGEMState),
+        VMSTATE_UINT32(rx_desc_addr, CadenceGEMState),
+        VMSTATE_UINT32(tx_desc_addr, CadenceGEMState),
+        VMSTATE_BOOL_ARRAY(sar_active, CadenceGEMState, 4),
         VMSTATE_END_OF_LIST(),
     }
 };
 
 static Property gem_properties[] = {
-    DEFINE_NIC_PROPERTIES(GemState, conf),
+    DEFINE_NIC_PROPERTIES(CadenceGEMState, conf),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -1277,7 +1277,7 @@ static void gem_class_init(ObjectClass *klass, void *data)
 static const TypeInfo gem_info = {
     .name  = TYPE_CADENCE_GEM,
     .parent = TYPE_SYS_BUS_DEVICE,
-    .instance_size  = sizeof(GemState),
+    .instance_size  = sizeof(CadenceGEMState),
     .class_init = gem_class_init,
 };
 
-- 
2.3.0.1.g27a12f1

[Qemu-devel] [PATCH target-arm v1 07/15] net: cadence_gem: Split state struct and type into header

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

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
 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

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 5994306..dafe914 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -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 };
 
diff --git a/include/hw/net/cadence_gem.h b/include/hw/net/cadence_gem.h
new file mode 100644
index 0000000..e6413ff
--- /dev/null
+++ b/include/hw/net/cadence_gem.h
@@ -0,0 +1,49 @@
+#ifndef CADENCE_GEM_H_
+
+#define TYPE_CADENCE_GEM "cadence_gem"
+#define CADENCE_GEM(obj) OBJECT_CHECK(CadenceGEMState, (obj), TYPE_CADENCE_GEM)
+
+#include "net/net.h"
+#include "hw/sysbus.h"
+
+#define CADENCE_GEM_MAXREG        (0x00000640/4) /* Last valid GEM address */
+
+typedef struct CadenceGEMState {
+    /*< private >*/
+    SysBusDevice parent_obj;
+    /*< public >*/
+
+    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;
+
+#define CADENCE_GEM_H_
+#endif
-- 
2.3.0.1.g27a12f1

[Qemu-devel] [PATCH target-arm v1 08/15] arm: xilinx-zynq-mp: Add GEM support

[Peter Crosthwaite]

There are 4x Cadence GEMs in Zynq MP. Add them.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
 hw/arm/xlnx-zynq-mp.c         | 32 ++++++++++++++++++++++++++++++++
 include/hw/arm/xlnx-zynq-mp.h |  3 +++
 2 files changed, 35 insertions(+)

diff --git a/hw/arm/xlnx-zynq-mp.c b/hw/arm/xlnx-zynq-mp.c
index be82a66..2ef57d9 100644
--- a/hw/arm/xlnx-zynq-mp.c
+++ b/hw/arm/xlnx-zynq-mp.c
@@ -25,6 +25,14 @@
 #define GIC_DIST_ADDR       0xf9010000
 #define GIC_CPU_ADDR        0xf9020000
 
+static const uint64_t gem_addr[XLNX_ZYNQ_MP_NUM_GEMS] = {
+    0xFF0B0000, 0xFF0C0000, 0xFF0D0000, 0xFF0E0000,
+};
+
+static const int gem_intr[XLNX_ZYNQ_MP_NUM_GEMS] = {
+    57, 59, 61, 63,
+};
+
 static inline int arm_gic_ppi_index(int cpu_nr, int ppi_index)
 {
     return GIC_NUM_SPI_INTR + cpu_nr * 32 + ppi_index;
@@ -43,6 +51,11 @@ static void xlnx_zynq_mp_init(Object *obj)
 
     object_initialize(&s->gic, sizeof(s->gic), TYPE_ARM_GIC);
     qdev_set_parent_bus(DEVICE(&s->gic), sysbus_get_default());
+
+    for (i = 0; i < XLNX_ZYNQ_MP_NUM_GEMS; i++) {
+        object_initialize(&s->gem[i], sizeof(s->gem[i]), TYPE_CADENCE_GEM);
+        qdev_set_parent_bus(DEVICE(&s->gem[i]), sysbus_get_default());
+    }
 }
 
 #define ERR_PROP_CHECK_RETURN(err, errp) do { \
@@ -56,6 +69,7 @@ static void xlnx_zynq_mp_realize(DeviceState *dev, Error **errp)
 {
     XlnxZynqMPState *s = XLNX_ZYNQ_MP(dev);
     uint8_t i;
+    qemu_irq gic_spi[GIC_NUM_SPI_INTR];
     Error *err = NULL;
 
     qdev_prop_set_uint32(DEVICE(&s->gic), "num-irq", GIC_NUM_SPI_INTR + 32);
@@ -81,6 +95,24 @@ static void xlnx_zynq_mp_realize(DeviceState *dev, Error **errp)
                                arm_gic_ppi_index(i, ARM_VIRT_TIMER_PPI));
         qdev_connect_gpio_out(DEVICE(&s->cpu[i]), 1, irq);
     }
+
+    for (i = 0; i < GIC_NUM_SPI_INTR; i++) {
+        gic_spi[i] = qdev_get_gpio_in(DEVICE(&s->gic), i);
+    }
+
+    for (i = 0; i < XLNX_ZYNQ_MP_NUM_GEMS; i++) {
+        NICInfo *nd = &nd_table[i];
+
+        if (nd->used) {
+            qemu_check_nic_model(nd, TYPE_CADENCE_GEM);
+            qdev_set_nic_properties(DEVICE(&s->gem[i]), nd);
+        }
+        object_property_set_bool(OBJECT(&s->gem[i]), true, "realized", &err);
+        ERR_PROP_CHECK_RETURN(err, errp);
+        sysbus_mmio_map(SYS_BUS_DEVICE(&s->gem[i]), 0, gem_addr[i]);
+        sysbus_connect_irq(SYS_BUS_DEVICE(&s->gem[i]), 0,
+                           gic_spi[gem_intr[i]]);
+    }
 }
 
 static void xlnx_zynq_mp_class_init(ObjectClass *oc, void *data)
diff --git a/include/hw/arm/xlnx-zynq-mp.h b/include/hw/arm/xlnx-zynq-mp.h
index 22b2af0..470503c 100644
--- a/include/hw/arm/xlnx-zynq-mp.h
+++ b/include/hw/arm/xlnx-zynq-mp.h
@@ -3,12 +3,14 @@
 #include "qemu-common.h"
 #include "hw/arm/arm.h"
 #include "hw/intc/arm_gic.h"
+#include "hw/net/cadence_gem.h"
 
 #define TYPE_XLNX_ZYNQ_MP "xlnx,zynq-mp"
 #define XLNX_ZYNQ_MP(obj) OBJECT_CHECK(XlnxZynqMPState, (obj), \
                                        TYPE_XLNX_ZYNQ_MP)
 
 #define XLNX_ZYNQ_MP_NUM_CPUS 4
+#define XLNX_ZYNQ_MP_NUM_GEMS 4
 
 typedef struct XlnxZynqMPState {
     /*< private >*/
@@ -17,6 +19,7 @@ typedef struct XlnxZynqMPState {
 
     ARMCPU cpu[XLNX_ZYNQ_MP_NUM_CPUS];
     GICState gic;
+    CadenceGEMState gem[XLNX_ZYNQ_MP_NUM_GEMS];
 }  XlnxZynqMPState;
 
 #define XLNX_ZYNQ_MP_H_
-- 
2.3.0.1.g27a12f1

[Qemu-devel] [PATCH target-arm v1 09/15] char: cadence_uart: Clean up variable names

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

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
 hw/char/cadence_uart.c | 100 ++++++++++++++++++++++++++-----------------------
 1 file changed, 53 insertions(+), 47 deletions(-)

diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
index 7044b35..23f548d 100644
--- a/hw/char/cadence_uart.c
+++ b/hw/char/cadence_uart.c
@@ -85,8 +85,8 @@
 #define LOCAL_LOOPBACK         (0x2 << UART_MR_CHMODE_SH)
 #define REMOTE_LOOPBACK        (0x3 << UART_MR_CHMODE_SH)
 
-#define RX_FIFO_SIZE           16
-#define TX_FIFO_SIZE           16
+#define CADENCE_UART_RX_FIFO_SIZE           16
+#define CADENCE_UART_TX_FIFO_SIZE           16
 #define UART_INPUT_CLK         50000000
 
 #define R_CR       (0x00/4)
@@ -108,10 +108,11 @@
 #define R_PWID     (0x40/4)
 #define R_TTRIG    (0x44/4)
 
-#define R_MAX (R_TTRIG + 1)
+#define CADENCE_UART_R_MAX (0x48/4)
 
 #define TYPE_CADENCE_UART "cadence_uart"
-#define CADENCE_UART(obj) OBJECT_CHECK(UartState, (obj), TYPE_CADENCE_UART)
+#define CADENCE_UART(obj) OBJECT_CHECK(CadenceUARTState, (obj), \
+                                       TYPE_CADENCE_UART)
 
 typedef struct {
     /*< private >*/
@@ -119,9 +120,9 @@ typedef struct {
     /*< public >*/
 
     MemoryRegion iomem;
-    uint32_t r[R_MAX];
-    uint8_t rx_fifo[RX_FIFO_SIZE];
-    uint8_t tx_fifo[TX_FIFO_SIZE];
+    uint32_t r[CADENCE_UART_R_MAX];
+    uint8_t rx_fifo[CADENCE_UART_RX_FIFO_SIZE];
+    uint8_t tx_fifo[CADENCE_UART_TX_FIFO_SIZE];
     uint32_t rx_wpos;
     uint32_t rx_count;
     uint32_t tx_count;
@@ -129,17 +130,19 @@ typedef struct {
     CharDriverState *chr;
     qemu_irq irq;
     QEMUTimer *fifo_trigger_handle;
-} UartState;
+} CadenceUARTState;
 
-static void uart_update_status(UartState *s)
+static void uart_update_status(CadenceUARTState *s)
 {
     s->r[R_SR] = 0;
 
-    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;
     s->r[R_SR] |= !s->rx_count ? UART_SR_INTR_REMPTY : 0;
     s->r[R_SR] |= s->rx_count >= s->r[R_RTRIG] ? UART_SR_INTR_RTRIG : 0;
 
-    s->r[R_SR] |= s->tx_count == TX_FIFO_SIZE ? UART_SR_INTR_TFUL : 0;
+    s->r[R_SR] |= s->tx_count == CADENCE_UART_TX_FIFO_SIZE ? UART_SR_INTR_TFUL
+                                                           : 0;
     s->r[R_SR] |= !s->tx_count ? UART_SR_INTR_TEMPTY : 0;
     s->r[R_SR] |= s->tx_count >= s->r[R_TTRIG] ? UART_SR_TTRIG : 0;
 
@@ -150,14 +153,14 @@ static void uart_update_status(UartState *s)
 
 static void fifo_trigger_update(void *opaque)
 {
-    UartState *s = (UartState *)opaque;
+    CadenceUARTState *s = opaque;
 
     s->r[R_CISR] |= UART_INTR_TIMEOUT;
 
     uart_update_status(s);
 }
 
-static void uart_rx_reset(UartState *s)
+static void uart_rx_reset(CadenceUARTState *s)
 {
     s->rx_wpos = 0;
     s->rx_count = 0;
@@ -166,12 +169,12 @@ static void uart_rx_reset(UartState *s)
     }
 }
 
-static void uart_tx_reset(UartState *s)
+static void uart_tx_reset(CadenceUARTState *s)
 {
     s->tx_count = 0;
 }
 
-static void uart_send_breaks(UartState *s)
+static void uart_send_breaks(CadenceUARTState *s)
 {
     int break_enabled = 1;
 
@@ -181,7 +184,7 @@ static void uart_send_breaks(UartState *s)
     }
 }
 
-static void uart_parameters_setup(UartState *s)
+static void uart_parameters_setup(CadenceUARTState *s)
 {
     QEMUSerialSetParams ssp;
     unsigned int baud_rate, packet_size;
@@ -236,20 +239,20 @@ static void uart_parameters_setup(UartState *s)
 
 static int uart_can_receive(void *opaque)
 {
-    UartState *s = (UartState *)opaque;
-    int ret = MAX(RX_FIFO_SIZE, TX_FIFO_SIZE);
+    CadenceUARTState *s = opaque;
+    int ret = MAX(CADENCE_UART_RX_FIFO_SIZE, CADENCE_UART_TX_FIFO_SIZE);
     uint32_t ch_mode = s->r[R_MR] & UART_MR_CHMODE;
 
     if (ch_mode == NORMAL_MODE || ch_mode == ECHO_MODE) {
-        ret = MIN(ret, RX_FIFO_SIZE - s->rx_count);
+        ret = MIN(ret, CADENCE_UART_RX_FIFO_SIZE - s->rx_count);
     }
     if (ch_mode == REMOTE_LOOPBACK || ch_mode == ECHO_MODE) {
-        ret = MIN(ret, TX_FIFO_SIZE - s->tx_count);
+        ret = MIN(ret, CADENCE_UART_TX_FIFO_SIZE - s->tx_count);
     }
     return ret;
 }
 
-static void uart_ctrl_update(UartState *s)
+static void uart_ctrl_update(CadenceUARTState *s)
 {
     if (s->r[R_CR] & UART_CR_TXRST) {
         uart_tx_reset(s);
@@ -268,7 +271,7 @@ static void uart_ctrl_update(UartState *s)
 
 static void uart_write_rx_fifo(void *opaque, const uint8_t *buf, int size)
 {
-    UartState *s = (UartState *)opaque;
+    CadenceUARTState *s = opaque;
     uint64_t new_rx_time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
     int i;
 
@@ -276,12 +279,12 @@ static void uart_write_rx_fifo(void *opaque, const uint8_t *buf, int size)
         return;
     }
 
-    if (s->rx_count == RX_FIFO_SIZE) {
+    if (s->rx_count == CADENCE_UART_RX_FIFO_SIZE) {
         s->r[R_CISR] |= UART_INTR_ROVR;
     } else {
         for (i = 0; i < size; i++) {
             s->rx_fifo[s->rx_wpos] = buf[i];
-            s->rx_wpos = (s->rx_wpos + 1) % RX_FIFO_SIZE;
+            s->rx_wpos = (s->rx_wpos + 1) % CADENCE_UART_RX_FIFO_SIZE;
             s->rx_count++;
         }
         timer_mod(s->fifo_trigger_handle, new_rx_time +
@@ -293,7 +296,7 @@ static void uart_write_rx_fifo(void *opaque, const uint8_t *buf, int size)
 static gboolean cadence_uart_xmit(GIOChannel *chan, GIOCondition cond,
                                   void *opaque)
 {
-    UartState *s = opaque;
+    CadenceUARTState *s = opaque;
     int ret;
 
     /* instant drain the fifo when there's no back-end */
@@ -320,14 +323,15 @@ static gboolean cadence_uart_xmit(GIOChannel *chan, GIOCondition cond,
     return FALSE;
 }
 
-static void uart_write_tx_fifo(UartState *s, const uint8_t *buf, int size)
+static void uart_write_tx_fifo(CadenceUARTState *s, const uint8_t *buf,
+                               int size)
 {
     if ((s->r[R_CR] & UART_CR_TX_DIS) || !(s->r[R_CR] & UART_CR_TX_EN)) {
         return;
     }
 
-    if (size > TX_FIFO_SIZE - s->tx_count) {
-        size = TX_FIFO_SIZE - s->tx_count;
+    if (size > CADENCE_UART_TX_FIFO_SIZE - s->tx_count) {
+        size = CADENCE_UART_TX_FIFO_SIZE - s->tx_count;
         /*
          * This can only be a guest error via a bad tx fifo register push,
          * as can_receive() should stop remote loop and echo modes ever getting
@@ -345,7 +349,7 @@ static void uart_write_tx_fifo(UartState *s, const uint8_t *buf, int size)
 
 static void uart_receive(void *opaque, const uint8_t *buf, int size)
 {
-    UartState *s = (UartState *)opaque;
+    CadenceUARTState *s = opaque;
     uint32_t ch_mode = s->r[R_MR] & UART_MR_CHMODE;
 
     if (ch_mode == NORMAL_MODE || ch_mode == ECHO_MODE) {
@@ -358,7 +362,7 @@ static void uart_receive(void *opaque, const uint8_t *buf, int size)
 
 static void uart_event(void *opaque, int event)
 {
-    UartState *s = (UartState *)opaque;
+    CadenceUARTState *s = opaque;
     uint8_t buf = '\0';
 
     if (event == CHR_EVENT_BREAK) {
@@ -368,15 +372,15 @@ static void uart_event(void *opaque, int event)
     uart_update_status(s);
 }
 
-static void uart_read_rx_fifo(UartState *s, uint32_t *c)
+static void uart_read_rx_fifo(CadenceUARTState *s, uint32_t *c)
 {
     if ((s->r[R_CR] & UART_CR_RX_DIS) || !(s->r[R_CR] & UART_CR_RX_EN)) {
         return;
     }
 
     if (s->rx_count) {
-        uint32_t rx_rpos =
-                (RX_FIFO_SIZE + s->rx_wpos - s->rx_count) % RX_FIFO_SIZE;
+        uint32_t rx_rpos = (CADENCE_UART_RX_FIFO_SIZE + s->rx_wpos -
+                            s->rx_count) % CADENCE_UART_RX_FIFO_SIZE;
         *c = s->rx_fifo[rx_rpos];
         s->rx_count--;
 
@@ -393,7 +397,7 @@ static void uart_read_rx_fifo(UartState *s, uint32_t *c)
 static void uart_write(void *opaque, hwaddr offset,
                           uint64_t value, unsigned size)
 {
-    UartState *s = (UartState *)opaque;
+    CadenceUARTState *s = opaque;
 
     DB_PRINT(" offset:%x data:%08x\n", (unsigned)offset, (unsigned)value);
     offset >>= 2;
@@ -437,11 +441,11 @@ static void uart_write(void *opaque, hwaddr offset,
 static uint64_t uart_read(void *opaque, hwaddr offset,
         unsigned size)
 {
-    UartState *s = (UartState *)opaque;
+    CadenceUARTState *s = opaque;
     uint32_t c = 0;
 
     offset >>= 2;
-    if (offset >= R_MAX) {
+    if (offset >= CADENCE_UART_R_MAX) {
         c = 0;
     } else if (offset == R_TX_RX) {
         uart_read_rx_fifo(s, &c);
@@ -461,7 +465,7 @@ static const MemoryRegionOps uart_ops = {
 
 static void cadence_uart_reset(DeviceState *dev)
 {
-    UartState *s = CADENCE_UART(dev);
+    CadenceUARTState *s = CADENCE_UART(dev);
 
     s->r[R_CR] = 0x00000128;
     s->r[R_IMR] = 0;
@@ -478,7 +482,7 @@ static void cadence_uart_reset(DeviceState *dev)
 
 static int cadence_uart_init(SysBusDevice *dev)
 {
-    UartState *s = CADENCE_UART(dev);
+    CadenceUARTState *s = CADENCE_UART(dev);
 
     memory_region_init_io(&s->iomem, OBJECT(s), &uart_ops, s, "uart", 0x1000);
     sysbus_init_mmio(dev, &s->iomem);
@@ -501,7 +505,7 @@ static int cadence_uart_init(SysBusDevice *dev)
 
 static int cadence_uart_post_load(void *opaque, int version_id)
 {
-    UartState *s = opaque;
+    CadenceUARTState *s = opaque;
 
     uart_parameters_setup(s);
     uart_update_status(s);
@@ -514,13 +518,15 @@ static const VMStateDescription vmstate_cadence_uart = {
     .minimum_version_id = 2,
     .post_load = cadence_uart_post_load,
     .fields = (VMStateField[]) {
-        VMSTATE_UINT32_ARRAY(r, UartState, R_MAX),
-        VMSTATE_UINT8_ARRAY(rx_fifo, UartState, RX_FIFO_SIZE),
-        VMSTATE_UINT8_ARRAY(tx_fifo, UartState, RX_FIFO_SIZE),
-        VMSTATE_UINT32(rx_count, UartState),
-        VMSTATE_UINT32(tx_count, UartState),
-        VMSTATE_UINT32(rx_wpos, UartState),
-        VMSTATE_TIMER_PTR(fifo_trigger_handle, UartState),
+        VMSTATE_UINT32_ARRAY(r, CadenceUARTState, CADENCE_UART_R_MAX),
+        VMSTATE_UINT8_ARRAY(rx_fifo, CadenceUARTState,
+                            CADENCE_UART_RX_FIFO_SIZE),
+        VMSTATE_UINT8_ARRAY(tx_fifo, CadenceUARTState,
+                            CADENCE_UART_TX_FIFO_SIZE),
+        VMSTATE_UINT32(rx_count, CadenceUARTState),
+        VMSTATE_UINT32(tx_count, CadenceUARTState),
+        VMSTATE_UINT32(rx_wpos, CadenceUARTState),
+        VMSTATE_TIMER_PTR(fifo_trigger_handle, CadenceUARTState),
         VMSTATE_END_OF_LIST()
     }
 };
@@ -538,7 +544,7 @@ static void cadence_uart_class_init(ObjectClass *klass, void *data)
 static const TypeInfo cadence_uart_info = {
     .name          = TYPE_CADENCE_UART,
     .parent        = TYPE_SYS_BUS_DEVICE,
-    .instance_size = sizeof(UartState),
+    .instance_size = sizeof(CadenceUARTState),
     .class_init    = cadence_uart_class_init,
 };
 
-- 
2.3.0.1.g27a12f1

[Qemu-devel] [PATCH target-arm v1 10/15] char: cadence_uart: Split state struct and type into header

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

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
 hw/char/cadence_uart.c         | 29 +----------------------------
 include/hw/char/cadence_uart.h | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 28 deletions(-)
 create mode 100644 include/hw/char/cadence_uart.h

diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
index 23f548d..4509e01 100644
--- a/hw/char/cadence_uart.c
+++ b/hw/char/cadence_uart.c
@@ -16,9 +16,7 @@
  * with this program; if not, see <http://www.gnu.org/licenses/>.
  */
 
-#include "hw/sysbus.h"
-#include "sysemu/char.h"
-#include "qemu/timer.h"
+#include "hw/char/cadence_uart.h"
 
 #ifdef CADENCE_UART_ERR_DEBUG
 #define DB_PRINT(...) do { \
@@ -85,8 +83,6 @@
 #define LOCAL_LOOPBACK         (0x2 << UART_MR_CHMODE_SH)
 #define REMOTE_LOOPBACK        (0x3 << UART_MR_CHMODE_SH)
 
-#define CADENCE_UART_RX_FIFO_SIZE           16
-#define CADENCE_UART_TX_FIFO_SIZE           16
 #define UART_INPUT_CLK         50000000
 
 #define R_CR       (0x00/4)
@@ -108,29 +104,6 @@
 #define R_PWID     (0x40/4)
 #define R_TTRIG    (0x44/4)
 
-#define CADENCE_UART_R_MAX (0x48/4)
-
-#define TYPE_CADENCE_UART "cadence_uart"
-#define CADENCE_UART(obj) OBJECT_CHECK(CadenceUARTState, (obj), \
-                                       TYPE_CADENCE_UART)
-
-typedef struct {
-    /*< private >*/
-    SysBusDevice parent_obj;
-    /*< public >*/
-
-    MemoryRegion iomem;
-    uint32_t r[CADENCE_UART_R_MAX];
-    uint8_t rx_fifo[CADENCE_UART_RX_FIFO_SIZE];
-    uint8_t tx_fifo[CADENCE_UART_TX_FIFO_SIZE];
-    uint32_t rx_wpos;
-    uint32_t rx_count;
-    uint32_t tx_count;
-    uint64_t char_tx_time;
-    CharDriverState *chr;
-    qemu_irq irq;
-    QEMUTimer *fifo_trigger_handle;
-} CadenceUARTState;
 
 static void uart_update_status(CadenceUARTState *s)
 {
diff --git a/include/hw/char/cadence_uart.h b/include/hw/char/cadence_uart.h
new file mode 100644
index 0000000..0404785
--- /dev/null
+++ b/include/hw/char/cadence_uart.h
@@ -0,0 +1,35 @@
+#ifndef CADENCE_UART_H_
+
+#include "hw/sysbus.h"
+#include "sysemu/char.h"
+#include "qemu/timer.h"
+
+#define CADENCE_UART_RX_FIFO_SIZE           16
+#define CADENCE_UART_TX_FIFO_SIZE           16
+
+#define CADENCE_UART_R_MAX (0x48/4)
+
+#define TYPE_CADENCE_UART "cadence_uart"
+#define CADENCE_UART(obj) OBJECT_CHECK(CadenceUARTState, (obj), \
+                                       TYPE_CADENCE_UART)
+
+typedef struct {
+    /*< private >*/
+    SysBusDevice parent_obj;
+    /*< public >*/
+
+    MemoryRegion iomem;
+    uint32_t r[CADENCE_UART_R_MAX];
+    uint8_t rx_fifo[CADENCE_UART_RX_FIFO_SIZE];
+    uint8_t tx_fifo[CADENCE_UART_TX_FIFO_SIZE];
+    uint32_t rx_wpos;
+    uint32_t rx_count;
+    uint32_t tx_count;
+    uint64_t char_tx_time;
+    CharDriverState *chr;
+    qemu_irq irq;
+    QEMUTimer *fifo_trigger_handle;
+} CadenceUARTState;
+
+#define CADENCE_UART_H_
+#endif
-- 
2.3.0.1.g27a12f1

[Qemu-devel] [PATCH target-arm v1 11/15] arm: xilinx-zynq-mp: Add UART support

[Peter Crosthwaite]

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

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
 hw/arm/xlnx-zynq-mp.c         | 21 +++++++++++++++++++++
 include/hw/arm/xlnx-zynq-mp.h |  3 +++
 2 files changed, 24 insertions(+)

diff --git a/hw/arm/xlnx-zynq-mp.c b/hw/arm/xlnx-zynq-mp.c
index 2ef57d9..9d7e834 100644
--- a/hw/arm/xlnx-zynq-mp.c
+++ b/hw/arm/xlnx-zynq-mp.c
@@ -33,6 +33,14 @@ static const int gem_intr[XLNX_ZYNQ_MP_NUM_GEMS] = {
     57, 59, 61, 63,
 };
 
+static const uint64_t uart_addr[XLNX_ZYNQ_MP_NUM_UARTS] = {
+    0xFF000000, 0xFF010000,
+};
+
+static const int uart_intr[XLNX_ZYNQ_MP_NUM_UARTS] = {
+    21, 22,
+};
+
 static inline int arm_gic_ppi_index(int cpu_nr, int ppi_index)
 {
     return GIC_NUM_SPI_INTR + cpu_nr * 32 + ppi_index;
@@ -56,6 +64,11 @@ static void xlnx_zynq_mp_init(Object *obj)
         object_initialize(&s->gem[i], sizeof(s->gem[i]), TYPE_CADENCE_GEM);
         qdev_set_parent_bus(DEVICE(&s->gem[i]), sysbus_get_default());
     }
+
+    for (i = 0; i < XLNX_ZYNQ_MP_NUM_UARTS; i++) {
+        object_initialize(&s->uart[i], sizeof(s->uart[i]), TYPE_CADENCE_UART);
+        qdev_set_parent_bus(DEVICE(&s->uart[i]), sysbus_get_default());
+    }
 }
 
 #define ERR_PROP_CHECK_RETURN(err, errp) do { \
@@ -113,6 +126,14 @@ static void xlnx_zynq_mp_realize(DeviceState *dev, Error **errp)
         sysbus_connect_irq(SYS_BUS_DEVICE(&s->gem[i]), 0,
                            gic_spi[gem_intr[i]]);
     }
+
+    for (i = 0; i < XLNX_ZYNQ_MP_NUM_UARTS; i++) {
+        object_property_set_bool(OBJECT(&s->uart[i]), true, "realized", &err);
+        ERR_PROP_CHECK_RETURN(err, errp);
+        sysbus_mmio_map(SYS_BUS_DEVICE(&s->uart[i]), 0, uart_addr[i]);
+        sysbus_connect_irq(SYS_BUS_DEVICE(&s->uart[i]), 0,
+                           gic_spi[uart_intr[i]]);
+    }
 }
 
 static void xlnx_zynq_mp_class_init(ObjectClass *oc, void *data)
diff --git a/include/hw/arm/xlnx-zynq-mp.h b/include/hw/arm/xlnx-zynq-mp.h
index 470503c..c4ee658 100644
--- a/include/hw/arm/xlnx-zynq-mp.h
+++ b/include/hw/arm/xlnx-zynq-mp.h
@@ -4,6 +4,7 @@
 #include "hw/arm/arm.h"
 #include "hw/intc/arm_gic.h"
 #include "hw/net/cadence_gem.h"
+#include "hw/char/cadence_uart.h"
 
 #define TYPE_XLNX_ZYNQ_MP "xlnx,zynq-mp"
 #define XLNX_ZYNQ_MP(obj) OBJECT_CHECK(XlnxZynqMPState, (obj), \
@@ -11,6 +12,7 @@
 
 #define XLNX_ZYNQ_MP_NUM_CPUS 4
 #define XLNX_ZYNQ_MP_NUM_GEMS 4
+#define XLNX_ZYNQ_MP_NUM_UARTS 2
 
 typedef struct XlnxZynqMPState {
     /*< private >*/
@@ -20,6 +22,7 @@ typedef struct XlnxZynqMPState {
     ARMCPU cpu[XLNX_ZYNQ_MP_NUM_CPUS];
     GICState gic;
     CadenceGEMState gem[XLNX_ZYNQ_MP_NUM_GEMS];
+    CadenceUARTState uart[XLNX_ZYNQ_MP_NUM_UARTS];
 }  XlnxZynqMPState;
 
 #define XLNX_ZYNQ_MP_H_
-- 
2.3.0.1.g27a12f1

[Qemu-devel] [PATCH target-arm v1 12/15] arm: Add xilinx-zynq-mp-generic machine

[Peter Crosthwaite]

Add a generic machine for the Xilinx Zynq MP SoC. This is a minimal
machine that exposes the capabilities of the raw SoC as a usable
machine.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
 hw/arm/Makefile.objs          |  2 +-
 hw/arm/xlnx-zynq-mp-generic.c | 52 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 1 deletion(-)
 create mode 100644 hw/arm/xlnx-zynq-mp-generic.c

diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index 9bf072b..776fbe3 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -8,4 +8,4 @@ obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o
 obj-$(CONFIG_DIGIC) += digic.o
 obj-y += omap1.o omap2.o strongarm.o
 obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10.o cubieboard.o
-obj-$(CONFIG_XLNX_ZYNQ_MP) += xlnx-zynq-mp.o
+obj-$(CONFIG_XLNX_ZYNQ_MP) += xlnx-zynq-mp.o xlnx-zynq-mp-generic.o
diff --git a/hw/arm/xlnx-zynq-mp-generic.c b/hw/arm/xlnx-zynq-mp-generic.c
new file mode 100644
index 0000000..ff69b07
--- /dev/null
+++ b/hw/arm/xlnx-zynq-mp-generic.c
@@ -0,0 +1,52 @@
+#/*
+ * Xilinx Zynq MPSoC emulation
+ *
+ * Copyright (C) 2015 Xilinx Inc
+ * Written by Peter Crosthwaite <peter.crosthwaite@xilinx.com>
+ *
+ * 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-zynq-mp.h"
+#include "hw/boards.h"
+#include "qemu/error-report.h"
+
+typedef struct XlnxZynqMPGeneric {
+    XlnxZynqMPState soc;
+} XlnxZynqMPGeneric;
+
+static void xlnx_zynq_mp_generic_init(MachineState *machine)
+{
+    XlnxZynqMPGeneric *s = g_new0(XlnxZynqMPGeneric, 1);
+    Error *err = NULL;
+
+    object_initialize(&s->soc, sizeof(s->soc), TYPE_XLNX_ZYNQ_MP);
+    object_property_add_child(OBJECT(machine), "soc", OBJECT(&s->soc), NULL);
+
+    object_property_set_bool(OBJECT(&s->soc), true, "realized", &err);
+    if (err) {
+        error_report("%s", error_get_pretty(err));
+        exit(1);
+    }
+}
+
+static QEMUMachine xlnx_zynq_mp_generic_machine = {
+    .name = "xlnx-zynq-mp-generic",
+    .desc = "Xilinx Zynq MP SoC generic machine",
+    .init = xlnx_zynq_mp_generic_init,
+};
+
+static void xlnx_zynq_mp_generic_machine_init(void)
+{
+    qemu_register_machine(&xlnx_zynq_mp_generic_machine);
+}
+
+machine_init(xlnx_zynq_mp_generic_machine_init);
-- 
2.3.0.1.g27a12f1

[Qemu-devel] [PATCH target-arm v1 13/15] arm: xilinx-zynq-mp-generic: Add external RAM

[Peter Crosthwaite]

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

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
 hw/arm/xlnx-zynq-mp-generic.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/arm/xlnx-zynq-mp-generic.c b/hw/arm/xlnx-zynq-mp-generic.c
index ff69b07..7394e82 100644
--- a/hw/arm/xlnx-zynq-mp-generic.c
+++ b/hw/arm/xlnx-zynq-mp-generic.c
@@ -18,9 +18,11 @@
 #include "hw/arm/xlnx-zynq-mp.h"
 #include "hw/boards.h"
 #include "qemu/error-report.h"
+#include "exec/address-spaces.h"
 
 typedef struct XlnxZynqMPGeneric {
     XlnxZynqMPState soc;
+    MemoryRegion ddr_ram;
 } XlnxZynqMPGeneric;
 
 static void xlnx_zynq_mp_generic_init(MachineState *machine)
@@ -36,6 +38,11 @@ static void xlnx_zynq_mp_generic_init(MachineState *machine)
         error_report("%s", error_get_pretty(err));
         exit(1);
     }
+
+    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_zynq_mp_generic_machine = {
-- 
2.3.0.1.g27a12f1

[Qemu-devel] [PATCH target-arm v1 14/15] arm: xilinx-zynq-mp-generic: Add bootloading

[Peter Crosthwaite]

Using standard ARM bootloader.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
 hw/arm/xlnx-zynq-mp-generic.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/arm/xlnx-zynq-mp-generic.c b/hw/arm/xlnx-zynq-mp-generic.c
index 7394e82..a86f10d 100644
--- a/hw/arm/xlnx-zynq-mp-generic.c
+++ b/hw/arm/xlnx-zynq-mp-generic.c
@@ -25,6 +25,8 @@ typedef struct XlnxZynqMPGeneric {
     MemoryRegion ddr_ram;
 } XlnxZynqMPGeneric;
 
+static struct arm_boot_info xlnx_zynq_mp_generic_binfo;
+
 static void xlnx_zynq_mp_generic_init(MachineState *machine)
 {
     XlnxZynqMPGeneric *s = g_new0(XlnxZynqMPGeneric, 1);
@@ -43,6 +45,12 @@ static void xlnx_zynq_mp_generic_init(MachineState *machine)
                            &error_abort);
     vmstate_register_ram_global(&s->ddr_ram);
     memory_region_add_subregion(get_system_memory(), 0, &s->ddr_ram);
+
+    xlnx_zynq_mp_generic_binfo.ram_size = machine->ram_size;
+    xlnx_zynq_mp_generic_binfo.kernel_filename = machine->kernel_filename;
+    xlnx_zynq_mp_generic_binfo.kernel_cmdline = machine->kernel_cmdline;
+    xlnx_zynq_mp_generic_binfo.initrd_filename = machine->initrd_filename;
+    arm_load_kernel(&s->soc.cpu[0], &xlnx_zynq_mp_generic_binfo);
 }
 
 static QEMUMachine xlnx_zynq_mp_generic_machine = {
-- 
2.3.0.1.g27a12f1

[Qemu-devel] [PATCH target-arm v1 15/15] arm: xlnx-zynq-mp: Add PSCI setup

[Peter Crosthwaite]

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

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
 hw/arm/xlnx-zynq-mp.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/arm/xlnx-zynq-mp.c b/hw/arm/xlnx-zynq-mp.c
index 9d7e834..0952221 100644
--- a/hw/arm/xlnx-zynq-mp.c
+++ b/hw/arm/xlnx-zynq-mp.c
@@ -96,6 +96,14 @@ static void xlnx_zynq_mp_realize(DeviceState *dev, Error **errp)
     for (i = 0; i < XLNX_ZYNQ_MP_NUM_CPUS; i++) {
         qemu_irq irq;
 
+        object_property_set_int(OBJECT(&s->cpu[i]), QEMU_PSCI_CONDUIT_SMC,
+                                "psci-conduit", NULL);
+        if (i > 0) {
+            /* Secondary CPUs start in PSCI powered-down state */
+            object_property_set_bool(OBJECT(&s->cpu[i]), true,
+                                     "start-powered-off", NULL);
+        }
+
         object_property_set_bool(OBJECT(&s->cpu[i]), true, "realized", &err);
         ERR_PROP_CHECK_RETURN(err, errp);
 
-- 
2.3.0.1.g27a12f1

Re: [Qemu-devel] [PATCH target-arm v1 13/15] arm: xilinx-zynq-mp-generic: Add external RAM

[Alistair Francis]

On Tue, Feb 24, 2015 at 9:04 AM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> Zynq MPSoC supports external DDR RAM. Add a RAM at 0 to the model.
>
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> ---
>  hw/arm/xlnx-zynq-mp-generic.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/hw/arm/xlnx-zynq-mp-generic.c b/hw/arm/xlnx-zynq-mp-generic.c
> index ff69b07..7394e82 100644
> --- a/hw/arm/xlnx-zynq-mp-generic.c
> +++ b/hw/arm/xlnx-zynq-mp-generic.c
> @@ -18,9 +18,11 @@
>  #include "hw/arm/xlnx-zynq-mp.h"
>  #include "hw/boards.h"
>  #include "qemu/error-report.h"
> +#include "exec/address-spaces.h"
>
>  typedef struct XlnxZynqMPGeneric {
>      XlnxZynqMPState soc;
> +    MemoryRegion ddr_ram;
>  } XlnxZynqMPGeneric;
>
>  static void xlnx_zynq_mp_generic_init(MachineState *machine)
> @@ -36,6 +38,11 @@ static void xlnx_zynq_mp_generic_init(MachineState *machine)
>          error_report("%s", error_get_pretty(err));
>          exit(1);
>      }
> +
> +    memory_region_init_ram(&s->ddr_ram, NULL, "ddr-ram", machine->ram_size,
> +                           &error_abort);

Shouldn't there be a default size if none is specified? This looks
like it will cause user
issues if they don't understand that they must specify the memory size.

At least return an error if none is specified.

Thanks,

Alistair

> +    vmstate_register_ram_global(&s->ddr_ram);
> +    memory_region_add_subregion(get_system_memory(), 0, &s->ddr_ram);
>  }
>
>  static QEMUMachine xlnx_zynq_mp_generic_machine = {
> --
> 2.3.0.1.g27a12f1
>
>

Re: [Qemu-devel] [PATCH target-arm v1 03/15] arm: Introduce Xilinx Zynq MPSoC

[Michal Simek]

On 02/24/2015 12:04 AM, Peter Crosthwaite wrote:
> With quad Cortex-A53 CPUs.
> 
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> ---
>  default-configs/aarch64-softmmu.mak |  2 +-
>  hw/arm/Makefile.objs                |  1 +
>  hw/arm/xlnx-zynq-mp.c               | 71 +++++++++++++++++++++++++++++++++++++
>  include/hw/arm/xlnx-zynq-mp.h       | 21 +++++++++++
>  4 files changed, 94 insertions(+), 1 deletion(-)
>  create mode 100644 hw/arm/xlnx-zynq-mp.c
>  create mode 100644 include/hw/arm/xlnx-zynq-mp.h

Can you please use zynqmp instead of zynq-mp?
For macros ZYNQMP and for case sensitive strings ZynqMP to be align
with other projects.

Thanks,
Michal

Re: [Qemu-devel] [PATCH target-arm v1 15/15] arm: xlnx-zynq-mp: Add PSCI setup

[Alistair Francis]

On Tue, Feb 24, 2015 at 9:04 AM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> Use SMC PSCI, with the standard policy of secondaries starting in
> power-off.
>
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> ---
>  hw/arm/xlnx-zynq-mp.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/hw/arm/xlnx-zynq-mp.c b/hw/arm/xlnx-zynq-mp.c
> index 9d7e834..0952221 100644
> --- a/hw/arm/xlnx-zynq-mp.c
> +++ b/hw/arm/xlnx-zynq-mp.c
> @@ -96,6 +96,14 @@ static void xlnx_zynq_mp_realize(DeviceState *dev, Error **errp)
>      for (i = 0; i < XLNX_ZYNQ_MP_NUM_CPUS; i++) {
>          qemu_irq irq;
>
> +        object_property_set_int(OBJECT(&s->cpu[i]), QEMU_PSCI_CONDUIT_SMC,
> +                                "psci-conduit", NULL);
> +        if (i > 0) {
> +            /* Secondary CPUs start in PSCI powered-down state */
> +            object_property_set_bool(OBJECT(&s->cpu[i]), true,
> +                                     "start-powered-off", NULL);

Something (probably err like most of the others) should be passed into
errp instead of NULL.

Thanks,

Alistair

> +        }
> +
>          object_property_set_bool(OBJECT(&s->cpu[i]), true, "realized", &err);
>          ERR_PROP_CHECK_RETURN(err, errp);
>
> --
> 2.3.0.1.g27a12f1
>
>

Re: [Qemu-devel] [PATCH target-arm v1 06/15] net: cadence_gem: Clean up variable names

[Alistair Francis]

On Tue, Feb 24, 2015 at 9:04 AM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> 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.
>
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Thanks,

Alistair

> ---
>  hw/net/cadence_gem.c | 70 ++++++++++++++++++++++++++--------------------------
>  1 file changed, 35 insertions(+), 35 deletions(-)
>
> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> index 55b6293..5994306 100644
> --- a/hw/net/cadence_gem.c
> +++ b/hw/net/cadence_gem.c
> @@ -141,7 +141,7 @@
>  #define GEM_DESCONF6      (0x00000294/4)
>  #define GEM_DESCONF7      (0x00000298/4)
>
> -#define GEM_MAXREG        (0x00000640/4) /* Last valid GEM address */
> +#define CADENCE_GEM_MAXREG        (0x00000640/4) /* Last valid GEM address */
>
>  /*****************************************/
>  #define GEM_NWCTRL_TXSTART     0x00000200 /* Transmit Enable */
> @@ -350,9 +350,9 @@ static inline void rx_desc_set_sar(unsigned *desc, int sar_idx)
>  }
>
>  #define TYPE_CADENCE_GEM "cadence_gem"
> -#define GEM(obj) OBJECT_CHECK(GemState, (obj), TYPE_CADENCE_GEM)
> +#define CADENCE_GEM(obj) OBJECT_CHECK(CadenceGEMState, (obj), TYPE_CADENCE_GEM)
>
> -typedef struct GemState {
> +typedef struct CadenceGEMState {
>      SysBusDevice parent_obj;
>
>      MemoryRegion iomem;
> @@ -361,15 +361,15 @@ typedef struct GemState {
>      qemu_irq irq;
>
>      /* GEM registers backing store */
> -    uint32_t regs[GEM_MAXREG];
> +    uint32_t regs[CADENCE_GEM_MAXREG];
>      /* Mask of register bits which are write only */
> -    uint32_t regs_wo[GEM_MAXREG];
> +    uint32_t regs_wo[CADENCE_GEM_MAXREG];
>      /* Mask of register bits which are read only */
> -    uint32_t regs_ro[GEM_MAXREG];
> +    uint32_t regs_ro[CADENCE_GEM_MAXREG];
>      /* Mask of register bits which are clear on read */
> -    uint32_t regs_rtc[GEM_MAXREG];
> +    uint32_t regs_rtc[CADENCE_GEM_MAXREG];
>      /* Mask of register bits which are write 1 to clear */
> -    uint32_t regs_w1c[GEM_MAXREG];
> +    uint32_t regs_w1c[CADENCE_GEM_MAXREG];
>
>      /* PHY registers backing store */
>      uint16_t phy_regs[32];
> @@ -385,7 +385,7 @@ typedef struct GemState {
>      unsigned rx_desc[2];
>
>      bool sar_active[4];
> -} GemState;
> +} CadenceGEMState;
>
>  /* The broadcast MAC address: 0xFFFFFFFFFFFF */
>  static const uint8_t broadcast_addr[] = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF };
> @@ -395,7 +395,7 @@ static const uint8_t broadcast_addr[] = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF };
>   * One time initialization.
>   * Set masks to identify which register bits have magical clear properties
>   */
> -static void gem_init_register_masks(GemState *s)
> +static void gem_init_register_masks(CadenceGEMState *s)
>  {
>      /* Mask of register bits which are read only */
>      memset(&s->regs_ro[0], 0, sizeof(s->regs_ro));
> @@ -430,7 +430,7 @@ static void gem_init_register_masks(GemState *s)
>   * phy_update_link:
>   * Make the emulated PHY link state match the QEMU "interface" state.
>   */
> -static void phy_update_link(GemState *s)
> +static void phy_update_link(CadenceGEMState *s)
>  {
>      DB_PRINT("down %d\n", qemu_get_queue(s->nic)->link_down);
>
> @@ -450,7 +450,7 @@ static void phy_update_link(GemState *s)
>
>  static int gem_can_receive(NetClientState *nc)
>  {
> -    GemState *s;
> +    CadenceGEMState *s;
>
>      s = qemu_get_nic_opaque(nc);
>
> @@ -483,7 +483,7 @@ static int gem_can_receive(NetClientState *nc)
>   * gem_update_int_status:
>   * Raise or lower interrupt based on current status.
>   */
> -static void gem_update_int_status(GemState *s)
> +static void gem_update_int_status(CadenceGEMState *s)
>  {
>      if (s->regs[GEM_ISR]) {
>          DB_PRINT("asserting int. (0x%08x)\n", s->regs[GEM_ISR]);
> @@ -495,7 +495,7 @@ static void gem_update_int_status(GemState *s)
>   * gem_receive_updatestats:
>   * Increment receive statistics.
>   */
> -static void gem_receive_updatestats(GemState *s, const uint8_t *packet,
> +static void gem_receive_updatestats(CadenceGEMState *s, const uint8_t *packet,
>                                      unsigned bytes)
>  {
>      uint64_t octets;
> @@ -586,7 +586,7 @@ static unsigned calc_mac_hash(const uint8_t *mac)
>   * GEM_RM_PROMISCUOUS_ACCEPT, GEM_RX_BROADCAST_ACCEPT,
>   * GEM_RX_MULTICAST_HASH_ACCEPT or GEM_RX_UNICAST_HASH_ACCEPT
>   */
> -static int gem_mac_address_filter(GemState *s, const uint8_t *packet)
> +static int gem_mac_address_filter(CadenceGEMState *s, const uint8_t *packet)
>  {
>      uint8_t *gem_spaddr;
>      int i;
> @@ -636,7 +636,7 @@ static int gem_mac_address_filter(GemState *s, const uint8_t *packet)
>      return GEM_RX_REJECT;
>  }
>
> -static void gem_get_rx_desc(GemState *s)
> +static void gem_get_rx_desc(CadenceGEMState *s)
>  {
>      DB_PRINT("read descriptor 0x%x\n", (unsigned)s->rx_desc_addr);
>      /* read current descriptor */
> @@ -660,7 +660,7 @@ static void gem_get_rx_desc(GemState *s)
>   */
>  static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
>  {
> -    GemState *s;
> +    CadenceGEMState *s;
>      unsigned   rxbufsize, bytes_to_copy;
>      unsigned   rxbuf_offset;
>      uint8_t    rxbuf[2048];
> @@ -810,7 +810,7 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
>   * gem_transmit_updatestats:
>   * Increment transmit statistics.
>   */
> -static void gem_transmit_updatestats(GemState *s, const uint8_t *packet,
> +static void gem_transmit_updatestats(CadenceGEMState *s, const uint8_t *packet,
>                                       unsigned bytes)
>  {
>      uint64_t octets;
> @@ -856,7 +856,7 @@ static void gem_transmit_updatestats(GemState *s, const uint8_t *packet,
>   * gem_transmit:
>   * Fish packets out of the descriptor ring and feed them to QEMU
>   */
> -static void gem_transmit(GemState *s)
> +static void gem_transmit(CadenceGEMState *s)
>  {
>      unsigned    desc[2];
>      hwaddr packet_desc_addr;
> @@ -976,7 +976,7 @@ static void gem_transmit(GemState *s)
>      }
>  }
>
> -static void gem_phy_reset(GemState *s)
> +static void gem_phy_reset(CadenceGEMState *s)
>  {
>      memset(&s->phy_regs[0], 0, sizeof(s->phy_regs));
>      s->phy_regs[PHY_REG_CONTROL] = 0x1140;
> @@ -1004,7 +1004,7 @@ static void gem_phy_reset(GemState *s)
>  static void gem_reset(DeviceState *d)
>  {
>      int i;
> -    GemState *s = GEM(d);
> +    CadenceGEMState *s = CADENCE_GEM(d);
>
>      DB_PRINT("\n");
>
> @@ -1032,13 +1032,13 @@ static void gem_reset(DeviceState *d)
>      gem_update_int_status(s);
>  }
>
> -static uint16_t gem_phy_read(GemState *s, unsigned reg_num)
> +static uint16_t gem_phy_read(CadenceGEMState *s, unsigned reg_num)
>  {
>      DB_PRINT("reg: %d value: 0x%04x\n", reg_num, s->phy_regs[reg_num]);
>      return s->phy_regs[reg_num];
>  }
>
> -static void gem_phy_write(GemState *s, unsigned reg_num, uint16_t val)
> +static void gem_phy_write(CadenceGEMState *s, unsigned reg_num, uint16_t val)
>  {
>      DB_PRINT("reg: %d value: 0x%04x\n", reg_num, val);
>
> @@ -1072,10 +1072,10 @@ static void gem_phy_write(GemState *s, unsigned reg_num, uint16_t val)
>   */
>  static uint64_t gem_read(void *opaque, hwaddr offset, unsigned size)
>  {
> -    GemState *s;
> +    CadenceGEMState *s;
>      uint32_t retval;
>
> -    s = (GemState *)opaque;
> +    s = (CadenceGEMState *)opaque;
>
>      offset >>= 2;
>      retval = s->regs[offset];
> @@ -1120,7 +1120,7 @@ static uint64_t gem_read(void *opaque, hwaddr offset, unsigned size)
>  static void gem_write(void *opaque, hwaddr offset, uint64_t val,
>          unsigned size)
>  {
> -    GemState *s = (GemState *)opaque;
> +    CadenceGEMState *s = (CadenceGEMState *)opaque;
>      uint32_t readonly;
>
>      DB_PRINT("offset: 0x%04x write: 0x%08x ", (unsigned)offset, (unsigned)val);
> @@ -1226,7 +1226,7 @@ static NetClientInfo net_gem_info = {
>  static int gem_init(SysBusDevice *sbd)
>  {
>      DeviceState *dev = DEVICE(sbd);
> -    GemState *s = GEM(dev);
> +    CadenceGEMState *s = CADENCE_GEM(dev);
>
>      DB_PRINT("\n");
>
> @@ -1248,18 +1248,18 @@ static const VMStateDescription vmstate_cadence_gem = {
>      .version_id = 2,
>      .minimum_version_id = 2,
>      .fields = (VMStateField[]) {
> -        VMSTATE_UINT32_ARRAY(regs, GemState, GEM_MAXREG),
> -        VMSTATE_UINT16_ARRAY(phy_regs, GemState, 32),
> -        VMSTATE_UINT8(phy_loop, GemState),
> -        VMSTATE_UINT32(rx_desc_addr, GemState),
> -        VMSTATE_UINT32(tx_desc_addr, GemState),
> -        VMSTATE_BOOL_ARRAY(sar_active, GemState, 4),
> +        VMSTATE_UINT32_ARRAY(regs, CadenceGEMState, CADENCE_GEM_MAXREG),
> +        VMSTATE_UINT16_ARRAY(phy_regs, CadenceGEMState, 32),
> +        VMSTATE_UINT8(phy_loop, CadenceGEMState),
> +        VMSTATE_UINT32(rx_desc_addr, CadenceGEMState),
> +        VMSTATE_UINT32(tx_desc_addr, CadenceGEMState),
> +        VMSTATE_BOOL_ARRAY(sar_active, CadenceGEMState, 4),
>          VMSTATE_END_OF_LIST(),
>      }
>  };
>
>  static Property gem_properties[] = {
> -    DEFINE_NIC_PROPERTIES(GemState, conf),
> +    DEFINE_NIC_PROPERTIES(CadenceGEMState, conf),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>
> @@ -1277,7 +1277,7 @@ static void gem_class_init(ObjectClass *klass, void *data)
>  static const TypeInfo gem_info = {
>      .name  = TYPE_CADENCE_GEM,
>      .parent = TYPE_SYS_BUS_DEVICE,
> -    .instance_size  = sizeof(GemState),
> +    .instance_size  = sizeof(CadenceGEMState),
>      .class_init = gem_class_init,
>  };
>
> --
> 2.3.0.1.g27a12f1
>
>

Re: [Qemu-devel] [PATCH target-arm v1 03/15] arm: Introduce Xilinx Zynq MPSoC

[Alistair Francis]

On Tue, Feb 24, 2015 at 9:04 AM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> With quad Cortex-A53 CPUs.
>
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> ---
>  default-configs/aarch64-softmmu.mak |  2 +-
>  hw/arm/Makefile.objs                |  1 +
>  hw/arm/xlnx-zynq-mp.c               | 71 +++++++++++++++++++++++++++++++++++++
>  include/hw/arm/xlnx-zynq-mp.h       | 21 +++++++++++
>  4 files changed, 94 insertions(+), 1 deletion(-)
>  create mode 100644 hw/arm/xlnx-zynq-mp.c
>  create mode 100644 include/hw/arm/xlnx-zynq-mp.h
>
> diff --git a/default-configs/aarch64-softmmu.mak b/default-configs/aarch64-softmmu.mak
> index 6d3b5c7..a8011e0 100644
> --- a/default-configs/aarch64-softmmu.mak
> +++ b/default-configs/aarch64-softmmu.mak
> @@ -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_ZYNQ_MP=y
> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
> index 6088e53..9bf072b 100644
> --- a/hw/arm/Makefile.objs
> +++ b/hw/arm/Makefile.objs
> @@ -8,3 +8,4 @@ obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o
>  obj-$(CONFIG_DIGIC) += digic.o
>  obj-y += omap1.o omap2.o strongarm.o
>  obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10.o cubieboard.o
> +obj-$(CONFIG_XLNX_ZYNQ_MP) += xlnx-zynq-mp.o
> diff --git a/hw/arm/xlnx-zynq-mp.c b/hw/arm/xlnx-zynq-mp.c
> new file mode 100644
> index 0000000..d553fb0
> --- /dev/null
> +++ b/hw/arm/xlnx-zynq-mp.c
> @@ -0,0 +1,71 @@
> +/*
> + * Xilinx Zynq MPSoC emulation
> + *
> + * Copyright (C) 2015 Xilinx Inc
> + * Written by Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> + *
> + * 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-zynq-mp.h"
> +
> +static void xlnx_zynq_mp_init(Object *obj)
> +{
> +    XlnxZynqMPState *s = XLNX_ZYNQ_MP(obj);
> +    int i;
> +
> +    for (i = 0; i < XLNX_ZYNQ_MP_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]), NULL);

Hey Peter,

Something should be passed in for errp, instead of NULL.

Probably use the ERR_PROP_CHECK_RETURN macro that you create.

Thanks,

Alistair

> +    }
> +}
> +
> +#define ERR_PROP_CHECK_RETURN(err, errp) do { \
> +    if (err) { \
> +        error_propagate((errp), (err)); \
> +        return; \
> +    } \
> +} while (0)
> +
> +static void xlnx_zynq_mp_realize(DeviceState *dev, Error **errp)
> +{
> +    XlnxZynqMPState *s = XLNX_ZYNQ_MP(dev);
> +    uint8_t i;
> +    Error *err = NULL;
> +
> +    for (i = 0; i < XLNX_ZYNQ_MP_NUM_CPUS; i++) {
> +        object_property_set_bool(OBJECT(&s->cpu[i]), true, "realized", &err);
> +        ERR_PROP_CHECK_RETURN(err, errp);
> +    }
> +}
> +
> +static void xlnx_zynq_mp_class_init(ObjectClass *oc, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(oc);
> +
> +    dc->realize = xlnx_zynq_mp_realize;
> +}
> +
> +static const TypeInfo xlnx_zynq_mp_type_info = {
> +    .name = TYPE_XLNX_ZYNQ_MP,
> +    .parent = TYPE_DEVICE,
> +    .instance_size = sizeof(XlnxZynqMPState),
> +    .instance_init = xlnx_zynq_mp_init,
> +    .class_init = xlnx_zynq_mp_class_init,
> +};
> +
> +static void xlnx_zynq_mp_register_types(void)
> +{
> +    type_register_static(&xlnx_zynq_mp_type_info);
> +}
> +
> +type_init(xlnx_zynq_mp_register_types)
> diff --git a/include/hw/arm/xlnx-zynq-mp.h b/include/hw/arm/xlnx-zynq-mp.h
> new file mode 100644
> index 0000000..f7410dc
> --- /dev/null
> +++ b/include/hw/arm/xlnx-zynq-mp.h
> @@ -0,0 +1,21 @@
> +#ifndef XLNX_ZYNQ_MP_H_
> +
> +#include "qemu-common.h"
> +#include "hw/arm/arm.h"
> +
> +#define TYPE_XLNX_ZYNQ_MP "xlnx,zynq-mp"
> +#define XLNX_ZYNQ_MP(obj) OBJECT_CHECK(XlnxZynqMPState, (obj), \
> +                                       TYPE_XLNX_ZYNQ_MP)
> +
> +#define XLNX_ZYNQ_MP_NUM_CPUS 4
> +
> +typedef struct XlnxZynqMPState {
> +    /*< private >*/
> +    DeviceState parent_obj;
> +    /*< public >*/
> +
> +    ARMCPU cpu[XLNX_ZYNQ_MP_NUM_CPUS];
> +}  XlnxZynqMPState;
> +
> +#define XLNX_ZYNQ_MP_H_
> +#endif
> --
> 2.3.0.1.g27a12f1
>
>

Re: [Qemu-devel] [PATCH target-arm v1 04/15] arm: xlnx-zynq-mp: Add GIC

[Alistair Francis]

On Tue, Feb 24, 2015 at 9:04 AM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> And connect IRQ outputs to the CPUs.
>
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Thanks,

Alistair

> ---
>  hw/arm/xlnx-zynq-mp.c         | 19 +++++++++++++++++++
>  include/hw/arm/xlnx-zynq-mp.h |  2 ++
>  2 files changed, 21 insertions(+)
>
> diff --git a/hw/arm/xlnx-zynq-mp.c b/hw/arm/xlnx-zynq-mp.c
> index d553fb0..9cdff13 100644
> --- a/hw/arm/xlnx-zynq-mp.c
> +++ b/hw/arm/xlnx-zynq-mp.c
> @@ -17,6 +17,11 @@
>
>  #include "hw/arm/xlnx-zynq-mp.h"
>
> +#define GIC_NUM_SPI_INTR 128
> +
> +#define GIC_DIST_ADDR       0xf9010000
> +#define GIC_CPU_ADDR        0xf9020000
> +
>  static void xlnx_zynq_mp_init(Object *obj)
>  {
>      XlnxZynqMPState *s = XLNX_ZYNQ_MP(obj);
> @@ -27,6 +32,9 @@ static void xlnx_zynq_mp_init(Object *obj)
>                            "cortex-a53-" TYPE_ARM_CPU);
>          object_property_add_child(obj, "cpu", OBJECT(&s->cpu[i]), NULL);
>      }
> +
> +    object_initialize(&s->gic, sizeof(s->gic), TYPE_ARM_GIC);
> +    qdev_set_parent_bus(DEVICE(&s->gic), sysbus_get_default());
>  }
>
>  #define ERR_PROP_CHECK_RETURN(err, errp) do { \
> @@ -42,9 +50,20 @@ static void xlnx_zynq_mp_realize(DeviceState *dev, Error **errp)
>      uint8_t i;
>      Error *err = NULL;
>
> +    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_ZYNQ_MP_NUM_CPUS);
> +    object_property_set_bool(OBJECT(&s->gic), true, "realized", &err);
> +    ERR_PROP_CHECK_RETURN(err, errp);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(&s->gic), 0, GIC_DIST_ADDR);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(&s->gic), 1, GIC_CPU_ADDR);
> +
>      for (i = 0; i < XLNX_ZYNQ_MP_NUM_CPUS; i++) {
>          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));
>      }
>  }
>
> diff --git a/include/hw/arm/xlnx-zynq-mp.h b/include/hw/arm/xlnx-zynq-mp.h
> index f7410dc..22b2af0 100644
> --- a/include/hw/arm/xlnx-zynq-mp.h
> +++ b/include/hw/arm/xlnx-zynq-mp.h
> @@ -2,6 +2,7 @@
>
>  #include "qemu-common.h"
>  #include "hw/arm/arm.h"
> +#include "hw/intc/arm_gic.h"
>
>  #define TYPE_XLNX_ZYNQ_MP "xlnx,zynq-mp"
>  #define XLNX_ZYNQ_MP(obj) OBJECT_CHECK(XlnxZynqMPState, (obj), \
> @@ -15,6 +16,7 @@ typedef struct XlnxZynqMPState {
>      /*< public >*/
>
>      ARMCPU cpu[XLNX_ZYNQ_MP_NUM_CPUS];
> +    GICState gic;
>  }  XlnxZynqMPState;
>
>  #define XLNX_ZYNQ_MP_H_
> --
> 2.3.0.1.g27a12f1
>
>

Re: [Qemu-devel] [PATCH target-arm v1 07/15] net: cadence_gem: Split state struct and type into header

[Alistair Francis]

On Tue, Feb 24, 2015 at 9:04 AM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> To allow using the device with modern SoC programming conventions. The
> state struct needs to be visible to embed the device in SoC containers.
>
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> ---
>  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
>
> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> index 5994306..dafe914 100644
> --- a/hw/net/cadence_gem.c
> +++ b/hw/net/cadence_gem.c
> @@ -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 };
>
> diff --git a/include/hw/net/cadence_gem.h b/include/hw/net/cadence_gem.h
> new file mode 100644
> index 0000000..e6413ff
> --- /dev/null
> +++ b/include/hw/net/cadence_gem.h
> @@ -0,0 +1,49 @@
> +#ifndef CADENCE_GEM_H_
> +
> +#define TYPE_CADENCE_GEM "cadence_gem"
> +#define CADENCE_GEM(obj) OBJECT_CHECK(CadenceGEMState, (obj), TYPE_CADENCE_GEM)
> +
> +#include "net/net.h"
> +#include "hw/sysbus.h"
> +
> +#define CADENCE_GEM_MAXREG        (0x00000640/4) /* Last valid GEM address */
> +
> +typedef struct CadenceGEMState {
> +    /*< private >*/
> +    SysBusDevice parent_obj;
> +    /*< public >*/
> +
> +    MemoryRegion iomem;

Nit pic, I think it looks cleaner like this:
    /*< private >*/
    SysBusDevice parent_obj;

    /*< public >*/
    MemoryRegion iomem;

but it doesn't really matter

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

Thanks,

Alistair

> +    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;
> +
> +#define CADENCE_GEM_H_
> +#endif
> --
> 2.3.0.1.g27a12f1
>
>

Re: [Qemu-devel] [PATCH target-arm v1 09/15] char: cadence_uart: Clean up variable names

[Alistair Francis]

On Tue, Feb 24, 2015 at 9:04 AM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> 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.
>
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Thanks,

Alistair

> ---
>  hw/char/cadence_uart.c | 100 ++++++++++++++++++++++++++-----------------------
>  1 file changed, 53 insertions(+), 47 deletions(-)
>
> diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
> index 7044b35..23f548d 100644
> --- a/hw/char/cadence_uart.c
> +++ b/hw/char/cadence_uart.c
> @@ -85,8 +85,8 @@
>  #define LOCAL_LOOPBACK         (0x2 << UART_MR_CHMODE_SH)
>  #define REMOTE_LOOPBACK        (0x3 << UART_MR_CHMODE_SH)
>
> -#define RX_FIFO_SIZE           16
> -#define TX_FIFO_SIZE           16
> +#define CADENCE_UART_RX_FIFO_SIZE           16
> +#define CADENCE_UART_TX_FIFO_SIZE           16
>  #define UART_INPUT_CLK         50000000
>
>  #define R_CR       (0x00/4)
> @@ -108,10 +108,11 @@
>  #define R_PWID     (0x40/4)
>  #define R_TTRIG    (0x44/4)
>
> -#define R_MAX (R_TTRIG + 1)
> +#define CADENCE_UART_R_MAX (0x48/4)
>
>  #define TYPE_CADENCE_UART "cadence_uart"
> -#define CADENCE_UART(obj) OBJECT_CHECK(UartState, (obj), TYPE_CADENCE_UART)
> +#define CADENCE_UART(obj) OBJECT_CHECK(CadenceUARTState, (obj), \
> +                                       TYPE_CADENCE_UART)
>
>  typedef struct {
>      /*< private >*/
> @@ -119,9 +120,9 @@ typedef struct {
>      /*< public >*/
>
>      MemoryRegion iomem;
> -    uint32_t r[R_MAX];
> -    uint8_t rx_fifo[RX_FIFO_SIZE];
> -    uint8_t tx_fifo[TX_FIFO_SIZE];
> +    uint32_t r[CADENCE_UART_R_MAX];
> +    uint8_t rx_fifo[CADENCE_UART_RX_FIFO_SIZE];
> +    uint8_t tx_fifo[CADENCE_UART_TX_FIFO_SIZE];
>      uint32_t rx_wpos;
>      uint32_t rx_count;
>      uint32_t tx_count;
> @@ -129,17 +130,19 @@ typedef struct {
>      CharDriverState *chr;
>      qemu_irq irq;
>      QEMUTimer *fifo_trigger_handle;
> -} UartState;
> +} CadenceUARTState;
>
> -static void uart_update_status(UartState *s)
> +static void uart_update_status(CadenceUARTState *s)
>  {
>      s->r[R_SR] = 0;
>
> -    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;
>      s->r[R_SR] |= !s->rx_count ? UART_SR_INTR_REMPTY : 0;
>      s->r[R_SR] |= s->rx_count >= s->r[R_RTRIG] ? UART_SR_INTR_RTRIG : 0;
>
> -    s->r[R_SR] |= s->tx_count == TX_FIFO_SIZE ? UART_SR_INTR_TFUL : 0;
> +    s->r[R_SR] |= s->tx_count == CADENCE_UART_TX_FIFO_SIZE ? UART_SR_INTR_TFUL
> +                                                           : 0;
>      s->r[R_SR] |= !s->tx_count ? UART_SR_INTR_TEMPTY : 0;
>      s->r[R_SR] |= s->tx_count >= s->r[R_TTRIG] ? UART_SR_TTRIG : 0;
>
> @@ -150,14 +153,14 @@ static void uart_update_status(UartState *s)
>
>  static void fifo_trigger_update(void *opaque)
>  {
> -    UartState *s = (UartState *)opaque;
> +    CadenceUARTState *s = opaque;
>
>      s->r[R_CISR] |= UART_INTR_TIMEOUT;
>
>      uart_update_status(s);
>  }
>
> -static void uart_rx_reset(UartState *s)
> +static void uart_rx_reset(CadenceUARTState *s)
>  {
>      s->rx_wpos = 0;
>      s->rx_count = 0;
> @@ -166,12 +169,12 @@ static void uart_rx_reset(UartState *s)
>      }
>  }
>
> -static void uart_tx_reset(UartState *s)
> +static void uart_tx_reset(CadenceUARTState *s)
>  {
>      s->tx_count = 0;
>  }
>
> -static void uart_send_breaks(UartState *s)
> +static void uart_send_breaks(CadenceUARTState *s)
>  {
>      int break_enabled = 1;
>
> @@ -181,7 +184,7 @@ static void uart_send_breaks(UartState *s)
>      }
>  }
>
> -static void uart_parameters_setup(UartState *s)
> +static void uart_parameters_setup(CadenceUARTState *s)
>  {
>      QEMUSerialSetParams ssp;
>      unsigned int baud_rate, packet_size;
> @@ -236,20 +239,20 @@ static void uart_parameters_setup(UartState *s)
>
>  static int uart_can_receive(void *opaque)
>  {
> -    UartState *s = (UartState *)opaque;
> -    int ret = MAX(RX_FIFO_SIZE, TX_FIFO_SIZE);
> +    CadenceUARTState *s = opaque;
> +    int ret = MAX(CADENCE_UART_RX_FIFO_SIZE, CADENCE_UART_TX_FIFO_SIZE);
>      uint32_t ch_mode = s->r[R_MR] & UART_MR_CHMODE;
>
>      if (ch_mode == NORMAL_MODE || ch_mode == ECHO_MODE) {
> -        ret = MIN(ret, RX_FIFO_SIZE - s->rx_count);
> +        ret = MIN(ret, CADENCE_UART_RX_FIFO_SIZE - s->rx_count);
>      }
>      if (ch_mode == REMOTE_LOOPBACK || ch_mode == ECHO_MODE) {
> -        ret = MIN(ret, TX_FIFO_SIZE - s->tx_count);
> +        ret = MIN(ret, CADENCE_UART_TX_FIFO_SIZE - s->tx_count);
>      }
>      return ret;
>  }
>
> -static void uart_ctrl_update(UartState *s)
> +static void uart_ctrl_update(CadenceUARTState *s)
>  {
>      if (s->r[R_CR] & UART_CR_TXRST) {
>          uart_tx_reset(s);
> @@ -268,7 +271,7 @@ static void uart_ctrl_update(UartState *s)
>
>  static void uart_write_rx_fifo(void *opaque, const uint8_t *buf, int size)
>  {
> -    UartState *s = (UartState *)opaque;
> +    CadenceUARTState *s = opaque;
>      uint64_t new_rx_time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>      int i;
>
> @@ -276,12 +279,12 @@ static void uart_write_rx_fifo(void *opaque, const uint8_t *buf, int size)
>          return;
>      }
>
> -    if (s->rx_count == RX_FIFO_SIZE) {
> +    if (s->rx_count == CADENCE_UART_RX_FIFO_SIZE) {
>          s->r[R_CISR] |= UART_INTR_ROVR;
>      } else {
>          for (i = 0; i < size; i++) {
>              s->rx_fifo[s->rx_wpos] = buf[i];
> -            s->rx_wpos = (s->rx_wpos + 1) % RX_FIFO_SIZE;
> +            s->rx_wpos = (s->rx_wpos + 1) % CADENCE_UART_RX_FIFO_SIZE;
>              s->rx_count++;
>          }
>          timer_mod(s->fifo_trigger_handle, new_rx_time +
> @@ -293,7 +296,7 @@ static void uart_write_rx_fifo(void *opaque, const uint8_t *buf, int size)
>  static gboolean cadence_uart_xmit(GIOChannel *chan, GIOCondition cond,
>                                    void *opaque)
>  {
> -    UartState *s = opaque;
> +    CadenceUARTState *s = opaque;
>      int ret;
>
>      /* instant drain the fifo when there's no back-end */
> @@ -320,14 +323,15 @@ static gboolean cadence_uart_xmit(GIOChannel *chan, GIOCondition cond,
>      return FALSE;
>  }
>
> -static void uart_write_tx_fifo(UartState *s, const uint8_t *buf, int size)
> +static void uart_write_tx_fifo(CadenceUARTState *s, const uint8_t *buf,
> +                               int size)
>  {
>      if ((s->r[R_CR] & UART_CR_TX_DIS) || !(s->r[R_CR] & UART_CR_TX_EN)) {
>          return;
>      }
>
> -    if (size > TX_FIFO_SIZE - s->tx_count) {
> -        size = TX_FIFO_SIZE - s->tx_count;
> +    if (size > CADENCE_UART_TX_FIFO_SIZE - s->tx_count) {
> +        size = CADENCE_UART_TX_FIFO_SIZE - s->tx_count;
>          /*
>           * This can only be a guest error via a bad tx fifo register push,
>           * as can_receive() should stop remote loop and echo modes ever getting
> @@ -345,7 +349,7 @@ static void uart_write_tx_fifo(UartState *s, const uint8_t *buf, int size)
>
>  static void uart_receive(void *opaque, const uint8_t *buf, int size)
>  {
> -    UartState *s = (UartState *)opaque;
> +    CadenceUARTState *s = opaque;
>      uint32_t ch_mode = s->r[R_MR] & UART_MR_CHMODE;
>
>      if (ch_mode == NORMAL_MODE || ch_mode == ECHO_MODE) {
> @@ -358,7 +362,7 @@ static void uart_receive(void *opaque, const uint8_t *buf, int size)
>
>  static void uart_event(void *opaque, int event)
>  {
> -    UartState *s = (UartState *)opaque;
> +    CadenceUARTState *s = opaque;
>      uint8_t buf = '\0';
>
>      if (event == CHR_EVENT_BREAK) {
> @@ -368,15 +372,15 @@ static void uart_event(void *opaque, int event)
>      uart_update_status(s);
>  }
>
> -static void uart_read_rx_fifo(UartState *s, uint32_t *c)
> +static void uart_read_rx_fifo(CadenceUARTState *s, uint32_t *c)
>  {
>      if ((s->r[R_CR] & UART_CR_RX_DIS) || !(s->r[R_CR] & UART_CR_RX_EN)) {
>          return;
>      }
>
>      if (s->rx_count) {
> -        uint32_t rx_rpos =
> -                (RX_FIFO_SIZE + s->rx_wpos - s->rx_count) % RX_FIFO_SIZE;
> +        uint32_t rx_rpos = (CADENCE_UART_RX_FIFO_SIZE + s->rx_wpos -
> +                            s->rx_count) % CADENCE_UART_RX_FIFO_SIZE;
>          *c = s->rx_fifo[rx_rpos];
>          s->rx_count--;
>
> @@ -393,7 +397,7 @@ static void uart_read_rx_fifo(UartState *s, uint32_t *c)
>  static void uart_write(void *opaque, hwaddr offset,
>                            uint64_t value, unsigned size)
>  {
> -    UartState *s = (UartState *)opaque;
> +    CadenceUARTState *s = opaque;
>
>      DB_PRINT(" offset:%x data:%08x\n", (unsigned)offset, (unsigned)value);
>      offset >>= 2;
> @@ -437,11 +441,11 @@ static void uart_write(void *opaque, hwaddr offset,
>  static uint64_t uart_read(void *opaque, hwaddr offset,
>          unsigned size)
>  {
> -    UartState *s = (UartState *)opaque;
> +    CadenceUARTState *s = opaque;
>      uint32_t c = 0;
>
>      offset >>= 2;
> -    if (offset >= R_MAX) {
> +    if (offset >= CADENCE_UART_R_MAX) {
>          c = 0;
>      } else if (offset == R_TX_RX) {
>          uart_read_rx_fifo(s, &c);
> @@ -461,7 +465,7 @@ static const MemoryRegionOps uart_ops = {
>
>  static void cadence_uart_reset(DeviceState *dev)
>  {
> -    UartState *s = CADENCE_UART(dev);
> +    CadenceUARTState *s = CADENCE_UART(dev);
>
>      s->r[R_CR] = 0x00000128;
>      s->r[R_IMR] = 0;
> @@ -478,7 +482,7 @@ static void cadence_uart_reset(DeviceState *dev)
>
>  static int cadence_uart_init(SysBusDevice *dev)
>  {
> -    UartState *s = CADENCE_UART(dev);
> +    CadenceUARTState *s = CADENCE_UART(dev);
>
>      memory_region_init_io(&s->iomem, OBJECT(s), &uart_ops, s, "uart", 0x1000);
>      sysbus_init_mmio(dev, &s->iomem);
> @@ -501,7 +505,7 @@ static int cadence_uart_init(SysBusDevice *dev)
>
>  static int cadence_uart_post_load(void *opaque, int version_id)
>  {
> -    UartState *s = opaque;
> +    CadenceUARTState *s = opaque;
>
>      uart_parameters_setup(s);
>      uart_update_status(s);
> @@ -514,13 +518,15 @@ static const VMStateDescription vmstate_cadence_uart = {
>      .minimum_version_id = 2,
>      .post_load = cadence_uart_post_load,
>      .fields = (VMStateField[]) {
> -        VMSTATE_UINT32_ARRAY(r, UartState, R_MAX),
> -        VMSTATE_UINT8_ARRAY(rx_fifo, UartState, RX_FIFO_SIZE),
> -        VMSTATE_UINT8_ARRAY(tx_fifo, UartState, RX_FIFO_SIZE),
> -        VMSTATE_UINT32(rx_count, UartState),
> -        VMSTATE_UINT32(tx_count, UartState),
> -        VMSTATE_UINT32(rx_wpos, UartState),
> -        VMSTATE_TIMER_PTR(fifo_trigger_handle, UartState),
> +        VMSTATE_UINT32_ARRAY(r, CadenceUARTState, CADENCE_UART_R_MAX),
> +        VMSTATE_UINT8_ARRAY(rx_fifo, CadenceUARTState,
> +                            CADENCE_UART_RX_FIFO_SIZE),
> +        VMSTATE_UINT8_ARRAY(tx_fifo, CadenceUARTState,
> +                            CADENCE_UART_TX_FIFO_SIZE),
> +        VMSTATE_UINT32(rx_count, CadenceUARTState),
> +        VMSTATE_UINT32(tx_count, CadenceUARTState),
> +        VMSTATE_UINT32(rx_wpos, CadenceUARTState),
> +        VMSTATE_TIMER_PTR(fifo_trigger_handle, CadenceUARTState),
>          VMSTATE_END_OF_LIST()
>      }
>  };
> @@ -538,7 +544,7 @@ static void cadence_uart_class_init(ObjectClass *klass, void *data)
>  static const TypeInfo cadence_uart_info = {
>      .name          = TYPE_CADENCE_UART,
>      .parent        = TYPE_SYS_BUS_DEVICE,
> -    .instance_size = sizeof(UartState),
> +    .instance_size = sizeof(CadenceUARTState),
>      .class_init    = cadence_uart_class_init,
>  };
>
> --
> 2.3.0.1.g27a12f1
>
>

Re: [Qemu-devel] [PATCH target-arm v1 10/15] char: cadence_uart: Split state struct and type into header

[Alistair Francis]

On Tue, Feb 24, 2015 at 9:04 AM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> To allow using the device with modern SoC programming conventions. The
> state struct needs to be visible to embed the device in SoC containers.
>
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> ---
>  hw/char/cadence_uart.c         | 29 +----------------------------
>  include/hw/char/cadence_uart.h | 35 +++++++++++++++++++++++++++++++++++
>  2 files changed, 36 insertions(+), 28 deletions(-)
>  create mode 100644 include/hw/char/cadence_uart.h
>
> diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
> index 23f548d..4509e01 100644
> --- a/hw/char/cadence_uart.c
> +++ b/hw/char/cadence_uart.c
> @@ -16,9 +16,7 @@
>   * with this program; if not, see <http://www.gnu.org/licenses/>.
>   */
>
> -#include "hw/sysbus.h"
> -#include "sysemu/char.h"
> -#include "qemu/timer.h"
> +#include "hw/char/cadence_uart.h"
>
>  #ifdef CADENCE_UART_ERR_DEBUG
>  #define DB_PRINT(...) do { \
> @@ -85,8 +83,6 @@
>  #define LOCAL_LOOPBACK         (0x2 << UART_MR_CHMODE_SH)
>  #define REMOTE_LOOPBACK        (0x3 << UART_MR_CHMODE_SH)
>
> -#define CADENCE_UART_RX_FIFO_SIZE           16
> -#define CADENCE_UART_TX_FIFO_SIZE           16
>  #define UART_INPUT_CLK         50000000
>
>  #define R_CR       (0x00/4)
> @@ -108,29 +104,6 @@
>  #define R_PWID     (0x40/4)
>  #define R_TTRIG    (0x44/4)
>
> -#define CADENCE_UART_R_MAX (0x48/4)
> -
> -#define TYPE_CADENCE_UART "cadence_uart"
> -#define CADENCE_UART(obj) OBJECT_CHECK(CadenceUARTState, (obj), \
> -                                       TYPE_CADENCE_UART)
> -
> -typedef struct {
> -    /*< private >*/
> -    SysBusDevice parent_obj;
> -    /*< public >*/
> -
> -    MemoryRegion iomem;

The same nit pick as the one I had for gem applies here as well.
Although it is so minor.

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

Thanks,

Alistair

> -    uint32_t r[CADENCE_UART_R_MAX];
> -    uint8_t rx_fifo[CADENCE_UART_RX_FIFO_SIZE];
> -    uint8_t tx_fifo[CADENCE_UART_TX_FIFO_SIZE];
> -    uint32_t rx_wpos;
> -    uint32_t rx_count;
> -    uint32_t tx_count;
> -    uint64_t char_tx_time;
> -    CharDriverState *chr;
> -    qemu_irq irq;
> -    QEMUTimer *fifo_trigger_handle;
> -} CadenceUARTState;
>
>  static void uart_update_status(CadenceUARTState *s)
>  {
> diff --git a/include/hw/char/cadence_uart.h b/include/hw/char/cadence_uart.h
> new file mode 100644
> index 0000000..0404785
> --- /dev/null
> +++ b/include/hw/char/cadence_uart.h
> @@ -0,0 +1,35 @@
> +#ifndef CADENCE_UART_H_
> +
> +#include "hw/sysbus.h"
> +#include "sysemu/char.h"
> +#include "qemu/timer.h"
> +
> +#define CADENCE_UART_RX_FIFO_SIZE           16
> +#define CADENCE_UART_TX_FIFO_SIZE           16
> +
> +#define CADENCE_UART_R_MAX (0x48/4)
> +
> +#define TYPE_CADENCE_UART "cadence_uart"
> +#define CADENCE_UART(obj) OBJECT_CHECK(CadenceUARTState, (obj), \
> +                                       TYPE_CADENCE_UART)
> +
> +typedef struct {
> +    /*< private >*/
> +    SysBusDevice parent_obj;
> +    /*< public >*/
> +
> +    MemoryRegion iomem;
> +    uint32_t r[CADENCE_UART_R_MAX];
> +    uint8_t rx_fifo[CADENCE_UART_RX_FIFO_SIZE];
> +    uint8_t tx_fifo[CADENCE_UART_TX_FIFO_SIZE];
> +    uint32_t rx_wpos;
> +    uint32_t rx_count;
> +    uint32_t tx_count;
> +    uint64_t char_tx_time;
> +    CharDriverState *chr;
> +    qemu_irq irq;
> +    QEMUTimer *fifo_trigger_handle;
> +} CadenceUARTState;
> +
> +#define CADENCE_UART_H_
> +#endif
> --
> 2.3.0.1.g27a12f1
>
>

Re: [Qemu-devel] [PATCH target-arm v1 00/15] Next Generation Xilinx Zynq SoC

[Alistair Francis]

On Tue, Feb 24, 2015 at 9:04 AM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> Hi Peter and all,
>
> Xilinx's next gen SoC has been announced. This series adds a SoC and
> machine model.
>
> 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 generic machine model that exposes just the RAW SoC itself. The
> only external device modelled is DDR RAM, as driven by the -m option.
> The standard bootloader and PSCI support is used.
>
> Regards,
> Peter
>
>
> Peter Crosthwaite (15):
>   target-arm: cpu64: Factor out ARM cortex init
>   target-arm: cpu64: Add support for cortex-a53
>   arm: Introduce Xilinx Zynq MPSoC
>   arm: xlnx-zynq-mp: Add GIC
>   arm: xlnx-zynq-mp: Connect CPU Timers to GIC
>   net: cadence_gem: Clean up variable names
>   net: cadence_gem: Split state struct and type into header
>   arm: xilinx-zynq-mp: Add GEM support
>   char: cadence_uart: Clean up variable names
>   char: cadence_uart: Split state struct and type into header
>   arm: xilinx-zynq-mp: Add UART support
>   arm: Add xilinx-zynq-mp-generic machine
>   arm: xilinx-zynq-mp-generic: Add external RAM
>   arm: xilinx-zynq-mp-generic: Add bootloading
>   arm: xlnx-zynq-mp: Add PSCI setup
>
>  default-configs/aarch64-softmmu.mak |   2 +-
>  hw/arm/Makefile.objs                |   1 +
>  hw/arm/xlnx-zynq-mp-generic.c       |  67 +++++++++++++++
>  hw/arm/xlnx-zynq-mp.c               | 167 ++++++++++++++++++++++++++++++++++++

These two files need to be renamed. You can' tell what each one is
from the title.

It should be something like 'xilinx_zynqmp_soc.c' and
'xilinx_zynqmp_machine.c' (or maybe just 'xilinx_zynqmp.c').

That way it also matches the same style as the current Zynq machine.

Thanks,

Alistair

>  hw/char/cadence_uart.c              | 113 ++++++++++--------------
>  hw/net/cadence_gem.c                |  95 ++++++--------------
>  include/hw/arm/xlnx-zynq-mp.h       |  29 +++++++
>  include/hw/char/cadence_uart.h      |  35 ++++++++
>  include/hw/net/cadence_gem.h        |  49 +++++++++++
>  target-arm/cpu64.c                  |  47 +++++++---
>  10 files changed, 456 insertions(+), 149 deletions(-)
>  create mode 100644 hw/arm/xlnx-zynq-mp-generic.c
>  create mode 100644 hw/arm/xlnx-zynq-mp.c
>  create mode 100644 include/hw/arm/xlnx-zynq-mp.h
>  create mode 100644 include/hw/char/cadence_uart.h
>  create mode 100644 include/hw/net/cadence_gem.h
>
> --
> 2.3.0.1.g27a12f1
>
>

Re: [Qemu-devel] [PATCH target-arm v1 11/15] arm: xilinx-zynq-mp: Add UART support

[Alistair Francis]

On Tue, Feb 24, 2015 at 9:04 AM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> There are 2x Cadence UARTSs in Zynq MP. Add them.
>
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Thanks,

Alistair

> ---
>  hw/arm/xlnx-zynq-mp.c         | 21 +++++++++++++++++++++
>  include/hw/arm/xlnx-zynq-mp.h |  3 +++
>  2 files changed, 24 insertions(+)
>
> diff --git a/hw/arm/xlnx-zynq-mp.c b/hw/arm/xlnx-zynq-mp.c
> index 2ef57d9..9d7e834 100644
> --- a/hw/arm/xlnx-zynq-mp.c
> +++ b/hw/arm/xlnx-zynq-mp.c
> @@ -33,6 +33,14 @@ static const int gem_intr[XLNX_ZYNQ_MP_NUM_GEMS] = {
>      57, 59, 61, 63,
>  };
>
> +static const uint64_t uart_addr[XLNX_ZYNQ_MP_NUM_UARTS] = {
> +    0xFF000000, 0xFF010000,
> +};
> +
> +static const int uart_intr[XLNX_ZYNQ_MP_NUM_UARTS] = {
> +    21, 22,
> +};
> +
>  static inline int arm_gic_ppi_index(int cpu_nr, int ppi_index)
>  {
>      return GIC_NUM_SPI_INTR + cpu_nr * 32 + ppi_index;
> @@ -56,6 +64,11 @@ static void xlnx_zynq_mp_init(Object *obj)
>          object_initialize(&s->gem[i], sizeof(s->gem[i]), TYPE_CADENCE_GEM);
>          qdev_set_parent_bus(DEVICE(&s->gem[i]), sysbus_get_default());
>      }
> +
> +    for (i = 0; i < XLNX_ZYNQ_MP_NUM_UARTS; i++) {
> +        object_initialize(&s->uart[i], sizeof(s->uart[i]), TYPE_CADENCE_UART);
> +        qdev_set_parent_bus(DEVICE(&s->uart[i]), sysbus_get_default());
> +    }
>  }
>
>  #define ERR_PROP_CHECK_RETURN(err, errp) do { \
> @@ -113,6 +126,14 @@ static void xlnx_zynq_mp_realize(DeviceState *dev, Error **errp)
>          sysbus_connect_irq(SYS_BUS_DEVICE(&s->gem[i]), 0,
>                             gic_spi[gem_intr[i]]);
>      }
> +
> +    for (i = 0; i < XLNX_ZYNQ_MP_NUM_UARTS; i++) {
> +        object_property_set_bool(OBJECT(&s->uart[i]), true, "realized", &err);
> +        ERR_PROP_CHECK_RETURN(err, errp);
> +        sysbus_mmio_map(SYS_BUS_DEVICE(&s->uart[i]), 0, uart_addr[i]);
> +        sysbus_connect_irq(SYS_BUS_DEVICE(&s->uart[i]), 0,
> +                           gic_spi[uart_intr[i]]);
> +    }
>  }
>
>  static void xlnx_zynq_mp_class_init(ObjectClass *oc, void *data)
> diff --git a/include/hw/arm/xlnx-zynq-mp.h b/include/hw/arm/xlnx-zynq-mp.h
> index 470503c..c4ee658 100644
> --- a/include/hw/arm/xlnx-zynq-mp.h
> +++ b/include/hw/arm/xlnx-zynq-mp.h
> @@ -4,6 +4,7 @@
>  #include "hw/arm/arm.h"
>  #include "hw/intc/arm_gic.h"
>  #include "hw/net/cadence_gem.h"
> +#include "hw/char/cadence_uart.h"
>
>  #define TYPE_XLNX_ZYNQ_MP "xlnx,zynq-mp"
>  #define XLNX_ZYNQ_MP(obj) OBJECT_CHECK(XlnxZynqMPState, (obj), \
> @@ -11,6 +12,7 @@
>
>  #define XLNX_ZYNQ_MP_NUM_CPUS 4
>  #define XLNX_ZYNQ_MP_NUM_GEMS 4
> +#define XLNX_ZYNQ_MP_NUM_UARTS 2
>
>  typedef struct XlnxZynqMPState {
>      /*< private >*/
> @@ -20,6 +22,7 @@ typedef struct XlnxZynqMPState {
>      ARMCPU cpu[XLNX_ZYNQ_MP_NUM_CPUS];
>      GICState gic;
>      CadenceGEMState gem[XLNX_ZYNQ_MP_NUM_GEMS];
> +    CadenceUARTState uart[XLNX_ZYNQ_MP_NUM_UARTS];
>  }  XlnxZynqMPState;
>
>  #define XLNX_ZYNQ_MP_H_
> --
> 2.3.0.1.g27a12f1
>
>

Re: [Qemu-devel] [PATCH target-arm v1 13/15] arm: xilinx-zynq-mp-generic: Add external RAM

[Peter Crosthwaite]

On Mon, Feb 23, 2015 at 6:24 PM, Alistair Francis
<alistair.francis@xilinx.com> wrote:
> On Tue, Feb 24, 2015 at 9:04 AM, Peter Crosthwaite
> <peter.crosthwaite@xilinx.com> wrote:
>> Zynq MPSoC supports external DDR RAM. Add a RAM at 0 to the model.
>>
>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>> ---
>>  hw/arm/xlnx-zynq-mp-generic.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/hw/arm/xlnx-zynq-mp-generic.c b/hw/arm/xlnx-zynq-mp-generic.c
>> index ff69b07..7394e82 100644
>> --- a/hw/arm/xlnx-zynq-mp-generic.c
>> +++ b/hw/arm/xlnx-zynq-mp-generic.c
>> @@ -18,9 +18,11 @@
>>  #include "hw/arm/xlnx-zynq-mp.h"
>>  #include "hw/boards.h"
>>  #include "qemu/error-report.h"
>> +#include "exec/address-spaces.h"
>>
>>  typedef struct XlnxZynqMPGeneric {
>>      XlnxZynqMPState soc;
>> +    MemoryRegion ddr_ram;
>>  } XlnxZynqMPGeneric;
>>
>>  static void xlnx_zynq_mp_generic_init(MachineState *machine)
>> @@ -36,6 +38,11 @@ static void xlnx_zynq_mp_generic_init(MachineState *machine)
>>          error_report("%s", error_get_pretty(err));
>>          exit(1);
>>      }
>> +
>> +    memory_region_init_ram(&s->ddr_ram, NULL, "ddr-ram", machine->ram_size,
>> +                           &error_abort);
>
> Shouldn't there be a default size if none is specified? This looks
> like it will cause user
> issues if they don't understand that they must specify the memory size.
>

So vl.c provides a core default of 128MB if -m is unspecified. It
should boot with that successfully. The error case is probably when
the user overrides -m to a low value such that the boot cant work
anymore, but that would be a error caught by the boot loader I think.

Regards,
Peter

> At least return an error if none is specified.
>
> Thanks,
>
> Alistair
>
>> +    vmstate_register_ram_global(&s->ddr_ram);
>> +    memory_region_add_subregion(get_system_memory(), 0, &s->ddr_ram);
>>  }
>>
>>  static QEMUMachine xlnx_zynq_mp_generic_machine = {
>> --
>> 2.3.0.1.g27a12f1
>>
>>
>

Re: [Qemu-devel] [PATCH target-arm v1 15/15] arm: xlnx-zynq-mp: Add PSCI setup

[Peter Crosthwaite]

On Wed, Feb 25, 2015 at 11:04 PM, Alistair Francis
<alistair.francis@xilinx.com> wrote:
> On Tue, Feb 24, 2015 at 9:04 AM, Peter Crosthwaite
> <peter.crosthwaite@xilinx.com> wrote:
>> Use SMC PSCI, with the standard policy of secondaries starting in
>> power-off.
>>
>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>> ---
>>  hw/arm/xlnx-zynq-mp.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/hw/arm/xlnx-zynq-mp.c b/hw/arm/xlnx-zynq-mp.c
>> index 9d7e834..0952221 100644
>> --- a/hw/arm/xlnx-zynq-mp.c
>> +++ b/hw/arm/xlnx-zynq-mp.c
>> @@ -96,6 +96,14 @@ static void xlnx_zynq_mp_realize(DeviceState *dev, Error **errp)
>>      for (i = 0; i < XLNX_ZYNQ_MP_NUM_CPUS; i++) {
>>          qemu_irq irq;
>>
>> +        object_property_set_int(OBJECT(&s->cpu[i]), QEMU_PSCI_CONDUIT_SMC,
>> +                                "psci-conduit", NULL);
>> +        if (i > 0) {
>> +            /* Secondary CPUs start in PSCI powered-down state */
>> +            object_property_set_bool(OBJECT(&s->cpu[i]), true,
>> +                                     "start-powered-off", NULL);
>
> Something (probably err like most of the others) should be passed into
> errp instead of NULL.
>

Changing to &error_abort. Thanks.

With the CPU type being fixed, it should not be possible for this to
fail. The original code I copied from (virt.c) uses NULL as it needs
to work with non-PSCI ARM CPUs but that doesn't apply here.

Regards,
Peter

> Thanks,
>
> Alistair
>
>> +        }
>> +
>>          object_property_set_bool(OBJECT(&s->cpu[i]), true, "realized", &err);
>>          ERR_PROP_CHECK_RETURN(err, errp);
>>
>> --
>> 2.3.0.1.g27a12f1
>>
>>
>

Re: [Qemu-devel] [PATCH target-arm v1 00/15] Next Generation Xilinx Zynq SoC

[Peter Crosthwaite]

On Thu, Feb 26, 2015 at 7:38 PM, Alistair Francis
<alistair.francis@xilinx.com> wrote:
> On Tue, Feb 24, 2015 at 9:04 AM, Peter Crosthwaite
> <peter.crosthwaite@xilinx.com> wrote:
>> Hi Peter and all,
>>
>> Xilinx's next gen SoC has been announced. This series adds a SoC and
>> machine model.
>>
>> 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 generic machine model that exposes just the RAW SoC itself. The
>> only external device modelled is DDR RAM, as driven by the -m option.
>> The standard bootloader and PSCI support is used.
>>
>> Regards,
>> Peter
>>
>>
>> Peter Crosthwaite (15):
>>   target-arm: cpu64: Factor out ARM cortex init
>>   target-arm: cpu64: Add support for cortex-a53
>>   arm: Introduce Xilinx Zynq MPSoC
>>   arm: xlnx-zynq-mp: Add GIC
>>   arm: xlnx-zynq-mp: Connect CPU Timers to GIC
>>   net: cadence_gem: Clean up variable names
>>   net: cadence_gem: Split state struct and type into header
>>   arm: xilinx-zynq-mp: Add GEM support
>>   char: cadence_uart: Clean up variable names
>>   char: cadence_uart: Split state struct and type into header
>>   arm: xilinx-zynq-mp: Add UART support
>>   arm: Add xilinx-zynq-mp-generic machine
>>   arm: xilinx-zynq-mp-generic: Add external RAM
>>   arm: xilinx-zynq-mp-generic: Add bootloading
>>   arm: xlnx-zynq-mp: Add PSCI setup
>>
>>  default-configs/aarch64-softmmu.mak |   2 +-
>>  hw/arm/Makefile.objs                |   1 +
>>  hw/arm/xlnx-zynq-mp-generic.c       |  67 +++++++++++++++
>>  hw/arm/xlnx-zynq-mp.c               | 167 ++++++++++++++++++++++++++++++++++++
>
> These two files need to be renamed. You can' tell what each one is
> from the title.
>

So the unqualified "zynqmp" should imply the SoC. This currently
applies to allwinner a10 and digic so Im trying to follow that
convention. When we start using specific boards, the zynqmp will be
dropped from the name completely. We could instead use the specific
board name ep108 but I wanted that to be additive. The board would be
called:

hw/arm/xlnx-ep108.c

by current conventions. One alternative would be just go out with
ep108 board only and no generic machine if you think that works
better?

> It should be something like 'xilinx_zynqmp_soc.c' and
> 'xilinx_zynqmp_machine.c' (or maybe just 'xilinx_zynqmp.c').
>

I'm trying to start using the truncated vendor name xlnx instead of
xilinx going forward (there is already in-tree precedent) as the 3
saved chars does save on a few 80 char line wraps. This is also the
convention used in device tree compatible strings.

Regards,
Peter

> That way it also matches the same style as the current Zynq machine.
>
> Thanks,
>
> Alistair
>
>>  hw/char/cadence_uart.c              | 113 ++++++++++--------------
>>  hw/net/cadence_gem.c                |  95 ++++++--------------
>>  include/hw/arm/xlnx-zynq-mp.h       |  29 +++++++
>>  include/hw/char/cadence_uart.h      |  35 ++++++++
>>  include/hw/net/cadence_gem.h        |  49 +++++++++++
>>  target-arm/cpu64.c                  |  47 +++++++---
>>  10 files changed, 456 insertions(+), 149 deletions(-)
>>  create mode 100644 hw/arm/xlnx-zynq-mp-generic.c
>>  create mode 100644 hw/arm/xlnx-zynq-mp.c
>>  create mode 100644 include/hw/arm/xlnx-zynq-mp.h
>>  create mode 100644 include/hw/char/cadence_uart.h
>>  create mode 100644 include/hw/net/cadence_gem.h
>>
>> --
>> 2.3.0.1.g27a12f1
>>
>>
>

Re: [Qemu-devel] [PATCH target-arm v1 03/15] arm: Introduce Xilinx Zynq MPSoC

[Peter Crosthwaite]

On Thu, Feb 26, 2015 at 5:50 PM, Alistair Francis
<alistair.francis@xilinx.com> wrote:
> On Tue, Feb 24, 2015 at 9:04 AM, Peter Crosthwaite
> <peter.crosthwaite@xilinx.com> wrote:
>> With quad Cortex-A53 CPUs.
>>
>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>> ---
>>  default-configs/aarch64-softmmu.mak |  2 +-
>>  hw/arm/Makefile.objs                |  1 +
>>  hw/arm/xlnx-zynq-mp.c               | 71 +++++++++++++++++++++++++++++++++++++
>>  include/hw/arm/xlnx-zynq-mp.h       | 21 +++++++++++
>>  4 files changed, 94 insertions(+), 1 deletion(-)
>>  create mode 100644 hw/arm/xlnx-zynq-mp.c
>>  create mode 100644 include/hw/arm/xlnx-zynq-mp.h
>>
>> diff --git a/default-configs/aarch64-softmmu.mak b/default-configs/aarch64-softmmu.mak
>> index 6d3b5c7..a8011e0 100644
>> --- a/default-configs/aarch64-softmmu.mak
>> +++ b/default-configs/aarch64-softmmu.mak
>> @@ -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_ZYNQ_MP=y
>> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
>> index 6088e53..9bf072b 100644
>> --- a/hw/arm/Makefile.objs
>> +++ b/hw/arm/Makefile.objs
>> @@ -8,3 +8,4 @@ obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o
>>  obj-$(CONFIG_DIGIC) += digic.o
>>  obj-y += omap1.o omap2.o strongarm.o
>>  obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10.o cubieboard.o
>> +obj-$(CONFIG_XLNX_ZYNQ_MP) += xlnx-zynq-mp.o
>> diff --git a/hw/arm/xlnx-zynq-mp.c b/hw/arm/xlnx-zynq-mp.c
>> new file mode 100644
>> index 0000000..d553fb0
>> --- /dev/null
>> +++ b/hw/arm/xlnx-zynq-mp.c
>> @@ -0,0 +1,71 @@
>> +/*
>> + * Xilinx Zynq MPSoC emulation
>> + *
>> + * Copyright (C) 2015 Xilinx Inc
>> + * Written by Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>> + *
>> + * 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-zynq-mp.h"
>> +
>> +static void xlnx_zynq_mp_init(Object *obj)
>> +{
>> +    XlnxZynqMPState *s = XLNX_ZYNQ_MP(obj);
>> +    int i;
>> +
>> +    for (i = 0; i < XLNX_ZYNQ_MP_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]), NULL);
>
> Hey Peter,
>
> Something should be passed in for errp, instead of NULL.
>
> Probably use the ERR_PROP_CHECK_RETURN macro that you create.
>

Going with &error_abort as I think this is a fatal. I can't see a user
visible action to reach here.

Regards,
Peter

> Thanks,
>
> Alistair
>
>> +    }
>> +}
>> +
>> +#define ERR_PROP_CHECK_RETURN(err, errp) do { \
>> +    if (err) { \
>> +        error_propagate((errp), (err)); \
>> +        return; \
>> +    } \
>> +} while (0)
>> +
>> +static void xlnx_zynq_mp_realize(DeviceState *dev, Error **errp)
>> +{
>> +    XlnxZynqMPState *s = XLNX_ZYNQ_MP(dev);
>> +    uint8_t i;
>> +    Error *err = NULL;
>> +
>> +    for (i = 0; i < XLNX_ZYNQ_MP_NUM_CPUS; i++) {
>> +        object_property_set_bool(OBJECT(&s->cpu[i]), true, "realized", &err);
>> +        ERR_PROP_CHECK_RETURN(err, errp);
>> +    }
>> +}
>> +
>> +static void xlnx_zynq_mp_class_init(ObjectClass *oc, void *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(oc);
>> +
>> +    dc->realize = xlnx_zynq_mp_realize;
>> +}
>> +
>> +static const TypeInfo xlnx_zynq_mp_type_info = {
>> +    .name = TYPE_XLNX_ZYNQ_MP,
>> +    .parent = TYPE_DEVICE,
>> +    .instance_size = sizeof(XlnxZynqMPState),
>> +    .instance_init = xlnx_zynq_mp_init,
>> +    .class_init = xlnx_zynq_mp_class_init,
>> +};
>> +
>> +static void xlnx_zynq_mp_register_types(void)
>> +{
>> +    type_register_static(&xlnx_zynq_mp_type_info);
>> +}
>> +
>> +type_init(xlnx_zynq_mp_register_types)
>> diff --git a/include/hw/arm/xlnx-zynq-mp.h b/include/hw/arm/xlnx-zynq-mp.h
>> new file mode 100644
>> index 0000000..f7410dc
>> --- /dev/null
>> +++ b/include/hw/arm/xlnx-zynq-mp.h
>> @@ -0,0 +1,21 @@
>> +#ifndef XLNX_ZYNQ_MP_H_
>> +
>> +#include "qemu-common.h"
>> +#include "hw/arm/arm.h"
>> +
>> +#define TYPE_XLNX_ZYNQ_MP "xlnx,zynq-mp"
>> +#define XLNX_ZYNQ_MP(obj) OBJECT_CHECK(XlnxZynqMPState, (obj), \
>> +                                       TYPE_XLNX_ZYNQ_MP)
>> +
>> +#define XLNX_ZYNQ_MP_NUM_CPUS 4
>> +
>> +typedef struct XlnxZynqMPState {
>> +    /*< private >*/
>> +    DeviceState parent_obj;
>> +    /*< public >*/
>> +
>> +    ARMCPU cpu[XLNX_ZYNQ_MP_NUM_CPUS];
>> +}  XlnxZynqMPState;
>> +
>> +#define XLNX_ZYNQ_MP_H_
>> +#endif
>> --
>> 2.3.0.1.g27a12f1
>>
>>
>

Re: [Qemu-devel] [PATCH target-arm v1 07/15] net: cadence_gem: Split state struct and type into header

[Peter Crosthwaite]

On Thu, Feb 26, 2015 at 7:12 PM, Alistair Francis
<alistair.francis@xilinx.com> wrote:
> On Tue, Feb 24, 2015 at 9:04 AM, Peter Crosthwaite
> <peter.crosthwaite@xilinx.com> wrote:
>> To allow using the device with modern SoC programming conventions. The
>> state struct needs to be visible to embed the device in SoC containers.
>>
>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>> ---
>>  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
>>
>> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
>> index 5994306..dafe914 100644
>> --- a/hw/net/cadence_gem.c
>> +++ b/hw/net/cadence_gem.c
>> @@ -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 };
>>
>> diff --git a/include/hw/net/cadence_gem.h b/include/hw/net/cadence_gem.h
>> new file mode 100644
>> index 0000000..e6413ff
>> --- /dev/null
>> +++ b/include/hw/net/cadence_gem.h
>> @@ -0,0 +1,49 @@
>> +#ifndef CADENCE_GEM_H_
>> +
>> +#define TYPE_CADENCE_GEM "cadence_gem"
>> +#define CADENCE_GEM(obj) OBJECT_CHECK(CadenceGEMState, (obj), TYPE_CADENCE_GEM)
>> +
>> +#include "net/net.h"
>> +#include "hw/sysbus.h"
>> +
>> +#define CADENCE_GEM_MAXREG        (0x00000640/4) /* Last valid GEM address */
>> +
>> +typedef struct CadenceGEMState {
>> +    /*< private >*/
>> +    SysBusDevice parent_obj;
>> +    /*< public >*/
>> +
>> +    MemoryRegion iomem;
>
> Nit pic, I think it looks cleaner like this:
>     /*< private >*/
>     SysBusDevice parent_obj;
>
>     /*< public >*/
>     MemoryRegion iomem;
>

Fixed.

> but it doesn't really matter
>
> Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>
>

Thanks.

Regards,
Peter

> Thanks,
>
> Alistair
>
>> +    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;
>> +
>> +#define CADENCE_GEM_H_
>> +#endif
>> --
>> 2.3.0.1.g27a12f1
>>
>>
>

Re: [Qemu-devel] [PATCH target-arm v1 10/15] char: cadence_uart: Split state struct and type into header

[Peter Crosthwaite]

On Thu, Feb 26, 2015 at 7:26 PM, Alistair Francis
<alistair.francis@xilinx.com> wrote:
> On Tue, Feb 24, 2015 at 9:04 AM, Peter Crosthwaite
> <peter.crosthwaite@xilinx.com> wrote:
>> To allow using the device with modern SoC programming conventions. The
>> state struct needs to be visible to embed the device in SoC containers.
>>
>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>> ---
>>  hw/char/cadence_uart.c         | 29 +----------------------------
>>  include/hw/char/cadence_uart.h | 35 +++++++++++++++++++++++++++++++++++
>>  2 files changed, 36 insertions(+), 28 deletions(-)
>>  create mode 100644 include/hw/char/cadence_uart.h
>>
>> diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
>> index 23f548d..4509e01 100644
>> --- a/hw/char/cadence_uart.c
>> +++ b/hw/char/cadence_uart.c
>> @@ -16,9 +16,7 @@
>>   * with this program; if not, see <http://www.gnu.org/licenses/>.
>>   */
>>
>> -#include "hw/sysbus.h"
>> -#include "sysemu/char.h"
>> -#include "qemu/timer.h"
>> +#include "hw/char/cadence_uart.h"
>>
>>  #ifdef CADENCE_UART_ERR_DEBUG
>>  #define DB_PRINT(...) do { \
>> @@ -85,8 +83,6 @@
>>  #define LOCAL_LOOPBACK         (0x2 << UART_MR_CHMODE_SH)
>>  #define REMOTE_LOOPBACK        (0x3 << UART_MR_CHMODE_SH)
>>
>> -#define CADENCE_UART_RX_FIFO_SIZE           16
>> -#define CADENCE_UART_TX_FIFO_SIZE           16
>>  #define UART_INPUT_CLK         50000000
>>
>>  #define R_CR       (0x00/4)
>> @@ -108,29 +104,6 @@
>>  #define R_PWID     (0x40/4)
>>  #define R_TTRIG    (0x44/4)
>>
>> -#define CADENCE_UART_R_MAX (0x48/4)
>> -
>> -#define TYPE_CADENCE_UART "cadence_uart"
>> -#define CADENCE_UART(obj) OBJECT_CHECK(CadenceUARTState, (obj), \
>> -                                       TYPE_CADENCE_UART)
>> -
>> -typedef struct {
>> -    /*< private >*/
>> -    SysBusDevice parent_obj;
>> -    /*< public >*/
>> -
>> -    MemoryRegion iomem;
>
> The same nit pick as the one I had for gem applies here as well.
> Although it is so minor.
>

Fixed

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

Thanks,

Regards,
Peter

> Thanks,
>
> Alistair
>
>> -    uint32_t r[CADENCE_UART_R_MAX];
>> -    uint8_t rx_fifo[CADENCE_UART_RX_FIFO_SIZE];
>> -    uint8_t tx_fifo[CADENCE_UART_TX_FIFO_SIZE];
>> -    uint32_t rx_wpos;
>> -    uint32_t rx_count;
>> -    uint32_t tx_count;
>> -    uint64_t char_tx_time;
>> -    CharDriverState *chr;
>> -    qemu_irq irq;
>> -    QEMUTimer *fifo_trigger_handle;
>> -} CadenceUARTState;
>>
>>  static void uart_update_status(CadenceUARTState *s)
>>  {
>> diff --git a/include/hw/char/cadence_uart.h b/include/hw/char/cadence_uart.h
>> new file mode 100644
>> index 0000000..0404785
>> --- /dev/null
>> +++ b/include/hw/char/cadence_uart.h
>> @@ -0,0 +1,35 @@
>> +#ifndef CADENCE_UART_H_
>> +
>> +#include "hw/sysbus.h"
>> +#include "sysemu/char.h"
>> +#include "qemu/timer.h"
>> +
>> +#define CADENCE_UART_RX_FIFO_SIZE           16
>> +#define CADENCE_UART_TX_FIFO_SIZE           16
>> +
>> +#define CADENCE_UART_R_MAX (0x48/4)
>> +
>> +#define TYPE_CADENCE_UART "cadence_uart"
>> +#define CADENCE_UART(obj) OBJECT_CHECK(CadenceUARTState, (obj), \
>> +                                       TYPE_CADENCE_UART)
>> +
>> +typedef struct {
>> +    /*< private >*/
>> +    SysBusDevice parent_obj;
>> +    /*< public >*/
>> +
>> +    MemoryRegion iomem;
>> +    uint32_t r[CADENCE_UART_R_MAX];
>> +    uint8_t rx_fifo[CADENCE_UART_RX_FIFO_SIZE];
>> +    uint8_t tx_fifo[CADENCE_UART_TX_FIFO_SIZE];
>> +    uint32_t rx_wpos;
>> +    uint32_t rx_count;
>> +    uint32_t tx_count;
>> +    uint64_t char_tx_time;
>> +    CharDriverState *chr;
>> +    qemu_irq irq;
>> +    QEMUTimer *fifo_trigger_handle;
>> +} CadenceUARTState;
>> +
>> +#define CADENCE_UART_H_
>> +#endif
>> --
>> 2.3.0.1.g27a12f1
>>
>>
>

Re: [Qemu-devel] [PATCH target-arm v1 03/15] arm: Introduce Xilinx Zynq MPSoC

[Alistair Francis]

On Tue, Mar 3, 2015 at 6:08 AM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> On Thu, Feb 26, 2015 at 5:50 PM, Alistair Francis
> <alistair.francis@xilinx.com> wrote:
>> On Tue, Feb 24, 2015 at 9:04 AM, Peter Crosthwaite
>> <peter.crosthwaite@xilinx.com> wrote:
>>> With quad Cortex-A53 CPUs.
>>>
>>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>> ---
>>>  default-configs/aarch64-softmmu.mak |  2 +-
>>>  hw/arm/Makefile.objs                |  1 +
>>>  hw/arm/xlnx-zynq-mp.c               | 71 +++++++++++++++++++++++++++++++++++++
>>>  include/hw/arm/xlnx-zynq-mp.h       | 21 +++++++++++
>>>  4 files changed, 94 insertions(+), 1 deletion(-)
>>>  create mode 100644 hw/arm/xlnx-zynq-mp.c
>>>  create mode 100644 include/hw/arm/xlnx-zynq-mp.h
>>>
>>> diff --git a/default-configs/aarch64-softmmu.mak b/default-configs/aarch64-softmmu.mak
>>> index 6d3b5c7..a8011e0 100644
>>> --- a/default-configs/aarch64-softmmu.mak
>>> +++ b/default-configs/aarch64-softmmu.mak
>>> @@ -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_ZYNQ_MP=y
>>> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
>>> index 6088e53..9bf072b 100644
>>> --- a/hw/arm/Makefile.objs
>>> +++ b/hw/arm/Makefile.objs
>>> @@ -8,3 +8,4 @@ obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o
>>>  obj-$(CONFIG_DIGIC) += digic.o
>>>  obj-y += omap1.o omap2.o strongarm.o
>>>  obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10.o cubieboard.o
>>> +obj-$(CONFIG_XLNX_ZYNQ_MP) += xlnx-zynq-mp.o
>>> diff --git a/hw/arm/xlnx-zynq-mp.c b/hw/arm/xlnx-zynq-mp.c
>>> new file mode 100644
>>> index 0000000..d553fb0
>>> --- /dev/null
>>> +++ b/hw/arm/xlnx-zynq-mp.c
>>> @@ -0,0 +1,71 @@
>>> +/*
>>> + * Xilinx Zynq MPSoC emulation
>>> + *
>>> + * Copyright (C) 2015 Xilinx Inc
>>> + * Written by Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>> + *
>>> + * 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-zynq-mp.h"
>>> +
>>> +static void xlnx_zynq_mp_init(Object *obj)
>>> +{
>>> +    XlnxZynqMPState *s = XLNX_ZYNQ_MP(obj);
>>> +    int i;
>>> +
>>> +    for (i = 0; i < XLNX_ZYNQ_MP_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]), NULL);
>>
>> Hey Peter,
>>
>> Something should be passed in for errp, instead of NULL.
>>
>> Probably use the ERR_PROP_CHECK_RETURN macro that you create.
>>
>
> Going with &error_abort as I think this is a fatal. I can't see a user
> visible action to reach here.

Sounds good

Thanks,

Alistair

>
> Regards,
> Peter
>
>> Thanks,
>>
>> Alistair
>>
>>> +    }
>>> +}
>>> +
>>> +#define ERR_PROP_CHECK_RETURN(err, errp) do { \
>>> +    if (err) { \
>>> +        error_propagate((errp), (err)); \
>>> +        return; \
>>> +    } \
>>> +} while (0)
>>> +
>>> +static void xlnx_zynq_mp_realize(DeviceState *dev, Error **errp)
>>> +{
>>> +    XlnxZynqMPState *s = XLNX_ZYNQ_MP(dev);
>>> +    uint8_t i;
>>> +    Error *err = NULL;
>>> +
>>> +    for (i = 0; i < XLNX_ZYNQ_MP_NUM_CPUS; i++) {
>>> +        object_property_set_bool(OBJECT(&s->cpu[i]), true, "realized", &err);
>>> +        ERR_PROP_CHECK_RETURN(err, errp);
>>> +    }
>>> +}
>>> +
>>> +static void xlnx_zynq_mp_class_init(ObjectClass *oc, void *data)
>>> +{
>>> +    DeviceClass *dc = DEVICE_CLASS(oc);
>>> +
>>> +    dc->realize = xlnx_zynq_mp_realize;
>>> +}
>>> +
>>> +static const TypeInfo xlnx_zynq_mp_type_info = {
>>> +    .name = TYPE_XLNX_ZYNQ_MP,
>>> +    .parent = TYPE_DEVICE,
>>> +    .instance_size = sizeof(XlnxZynqMPState),
>>> +    .instance_init = xlnx_zynq_mp_init,
>>> +    .class_init = xlnx_zynq_mp_class_init,
>>> +};
>>> +
>>> +static void xlnx_zynq_mp_register_types(void)
>>> +{
>>> +    type_register_static(&xlnx_zynq_mp_type_info);
>>> +}
>>> +
>>> +type_init(xlnx_zynq_mp_register_types)
>>> diff --git a/include/hw/arm/xlnx-zynq-mp.h b/include/hw/arm/xlnx-zynq-mp.h
>>> new file mode 100644
>>> index 0000000..f7410dc
>>> --- /dev/null
>>> +++ b/include/hw/arm/xlnx-zynq-mp.h
>>> @@ -0,0 +1,21 @@
>>> +#ifndef XLNX_ZYNQ_MP_H_
>>> +
>>> +#include "qemu-common.h"
>>> +#include "hw/arm/arm.h"
>>> +
>>> +#define TYPE_XLNX_ZYNQ_MP "xlnx,zynq-mp"
>>> +#define XLNX_ZYNQ_MP(obj) OBJECT_CHECK(XlnxZynqMPState, (obj), \
>>> +                                       TYPE_XLNX_ZYNQ_MP)
>>> +
>>> +#define XLNX_ZYNQ_MP_NUM_CPUS 4
>>> +
>>> +typedef struct XlnxZynqMPState {
>>> +    /*< private >*/
>>> +    DeviceState parent_obj;
>>> +    /*< public >*/
>>> +
>>> +    ARMCPU cpu[XLNX_ZYNQ_MP_NUM_CPUS];
>>> +}  XlnxZynqMPState;
>>> +
>>> +#define XLNX_ZYNQ_MP_H_
>>> +#endif
>>> --
>>> 2.3.0.1.g27a12f1
>>>
>>>
>>
>

Re: [Qemu-devel] [PATCH target-arm v1 03/15] arm: Introduce Xilinx Zynq MPSoC

[Peter Crosthwaite]

On Tue, Feb 24, 2015 at 12:06 PM, Michal Simek <michal.simek@xilinx.com> wrote:
> On 02/24/2015 12:04 AM, Peter Crosthwaite wrote:
>> With quad Cortex-A53 CPUs.
>>
>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>> ---
>>  default-configs/aarch64-softmmu.mak |  2 +-
>>  hw/arm/Makefile.objs                |  1 +
>>  hw/arm/xlnx-zynq-mp.c               | 71 +++++++++++++++++++++++++++++++++++++
>>  include/hw/arm/xlnx-zynq-mp.h       | 21 +++++++++++
>>  4 files changed, 94 insertions(+), 1 deletion(-)
>>  create mode 100644 hw/arm/xlnx-zynq-mp.c
>>  create mode 100644 include/hw/arm/xlnx-zynq-mp.h
>
> Can you please use zynqmp instead of zynq-mp?
> For macros ZYNQMP and for case sensitive strings ZynqMP to be align
> with other projects.
>

Fixed globally in V2.

Regards,
Peter

> Thanks,
> Michal
>
>

Re: [Qemu-devel] [PATCH target-arm v1 13/15] arm: xilinx-zynq-mp-generic: Add external RAM

[Alistair Francis]

On Tue, Mar 3, 2015 at 5:40 AM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> On Mon, Feb 23, 2015 at 6:24 PM, Alistair Francis
> <alistair.francis@xilinx.com> wrote:
>> On Tue, Feb 24, 2015 at 9:04 AM, Peter Crosthwaite
>> <peter.crosthwaite@xilinx.com> wrote:
>>> Zynq MPSoC supports external DDR RAM. Add a RAM at 0 to the model.
>>>
>>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>> ---
>>>  hw/arm/xlnx-zynq-mp-generic.c | 7 +++++++
>>>  1 file changed, 7 insertions(+)
>>>
>>> diff --git a/hw/arm/xlnx-zynq-mp-generic.c b/hw/arm/xlnx-zynq-mp-generic.c
>>> index ff69b07..7394e82 100644
>>> --- a/hw/arm/xlnx-zynq-mp-generic.c
>>> +++ b/hw/arm/xlnx-zynq-mp-generic.c
>>> @@ -18,9 +18,11 @@
>>>  #include "hw/arm/xlnx-zynq-mp.h"
>>>  #include "hw/boards.h"
>>>  #include "qemu/error-report.h"
>>> +#include "exec/address-spaces.h"
>>>
>>>  typedef struct XlnxZynqMPGeneric {
>>>      XlnxZynqMPState soc;
>>> +    MemoryRegion ddr_ram;
>>>  } XlnxZynqMPGeneric;
>>>
>>>  static void xlnx_zynq_mp_generic_init(MachineState *machine)
>>> @@ -36,6 +38,11 @@ static void xlnx_zynq_mp_generic_init(MachineState *machine)
>>>          error_report("%s", error_get_pretty(err));
>>>          exit(1);
>>>      }
>>> +
>>> +    memory_region_init_ram(&s->ddr_ram, NULL, "ddr-ram", machine->ram_size,
>>> +                           &error_abort);
>>
>> Shouldn't there be a default size if none is specified? This looks
>> like it will cause user
>> issues if they don't understand that they must specify the memory size.
>>
>
> So vl.c provides a core default of 128MB if -m is unspecified. It
> should boot with that successfully. The error case is probably when
> the user overrides -m to a low value such that the boot cant work
> anymore, but that would be a error caught by the boot loader I think.

I still think that this could cause issues. If a user doesn't realise that
they have to specify -m and the boot still works, they will just assume
that the correct memory size is in the machine model. When in fact all
that is happening is they are getting a small amount of memory that is
just lucky enough to work. 128MB is very small for ZynqMP.

Maybe instead of an error or a default just have a warning if the memory
is below a certain size. That way it will still boot, but the user will know
that they should be specifying a size.

Thanks,

Alistair

>
> Regards,
> Peter
>
>> At least return an error if none is specified.
>>
>> Thanks,
>>
>> Alistair
>>
>>> +    vmstate_register_ram_global(&s->ddr_ram);
>>> +    memory_region_add_subregion(get_system_memory(), 0, &s->ddr_ram);
>>>  }
>>>
>>>  static QEMUMachine xlnx_zynq_mp_generic_machine = {
>>> --
>>> 2.3.0.1.g27a12f1
>>>
>>>
>>
>

Re: [Qemu-devel] [PATCH target-arm v1 00/15] Next Generation Xilinx Zynq SoC

[Alistair Francis]

On Tue, Mar 3, 2015 at 6:06 AM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> On Thu, Feb 26, 2015 at 7:38 PM, Alistair Francis
> <alistair.francis@xilinx.com> wrote:
>> On Tue, Feb 24, 2015 at 9:04 AM, Peter Crosthwaite
>> <peter.crosthwaite@xilinx.com> wrote:
>>> Hi Peter and all,
>>>
>>> Xilinx's next gen SoC has been announced. This series adds a SoC and
>>> machine model.
>>>
>>> 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 generic machine model that exposes just the RAW SoC itself. The
>>> only external device modelled is DDR RAM, as driven by the -m option.
>>> The standard bootloader and PSCI support is used.
>>>
>>> Regards,
>>> Peter
>>>
>>>
>>> Peter Crosthwaite (15):
>>>   target-arm: cpu64: Factor out ARM cortex init
>>>   target-arm: cpu64: Add support for cortex-a53
>>>   arm: Introduce Xilinx Zynq MPSoC
>>>   arm: xlnx-zynq-mp: Add GIC
>>>   arm: xlnx-zynq-mp: Connect CPU Timers to GIC
>>>   net: cadence_gem: Clean up variable names
>>>   net: cadence_gem: Split state struct and type into header
>>>   arm: xilinx-zynq-mp: Add GEM support
>>>   char: cadence_uart: Clean up variable names
>>>   char: cadence_uart: Split state struct and type into header
>>>   arm: xilinx-zynq-mp: Add UART support
>>>   arm: Add xilinx-zynq-mp-generic machine
>>>   arm: xilinx-zynq-mp-generic: Add external RAM
>>>   arm: xilinx-zynq-mp-generic: Add bootloading
>>>   arm: xlnx-zynq-mp: Add PSCI setup
>>>
>>>  default-configs/aarch64-softmmu.mak |   2 +-
>>>  hw/arm/Makefile.objs                |   1 +
>>>  hw/arm/xlnx-zynq-mp-generic.c       |  67 +++++++++++++++
>>>  hw/arm/xlnx-zynq-mp.c               | 167 ++++++++++++++++++++++++++++++++++++
>>
>> These two files need to be renamed. You can' tell what each one is
>> from the title.

Just re-read this, sorry about the 'need to be'.

>>
>
> So the unqualified "zynqmp" should imply the SoC. This currently
> applies to allwinner a10 and digic so Im trying to follow that
> convention. When we start using specific boards, the zynqmp will be

I see your point, but Allwinner only have one file in the hw/arm/ folder
that starts with 'allwinner', making it much clearer what it is. Digic and
Exynos also have the '_boards' suffix on their boards, which separates
the SoC from the board/boards.

With that logic the 'xlnx-zynq-mp-generic.c' probably should have a
'board' in there somewhere.

> dropped from the name completely. We could instead use the specific
> board name ep108 but I wanted that to be additive. The board would be
> called:
>
> hw/arm/xlnx-ep108.c
>
> by current conventions. One alternative would be just go out with
> ep108 board only and no generic machine if you think that works
> better?

That naming is clearer, but only if the user knows what the EP108 is.
If they are just looking to 'boot Xilinx's ZynqMP chip' then it might be a
bit confusing. In saying that, I don't think it's a big issue and I would
be fine with having a 'xlnx-ep108.c' board

>
>> It should be something like 'xilinx_zynqmp_soc.c' and
>> 'xilinx_zynqmp_machine.c' (or maybe just 'xilinx_zynqmp.c').
>>
>
> I'm trying to start using the truncated vendor name xlnx instead of
> xilinx going forward (there is already in-tree precedent) as the 3
> saved chars does save on a few 80 char line wraps. This is also the
> convention used in device tree compatible strings.

Ok, makes sense. Is it possible to change the current 'xilinx_zynq.c'
file then, that way everything matches? I guess that could break current
implementations though (epically if the actual machine name changes).

Thanks,

Alistair

>
> Regards,
> Peter
>
>> That way it also matches the same style as the current Zynq machine.
>>
>> Thanks,
>>
>> Alistair
>>
>>>  hw/char/cadence_uart.c              | 113 ++++++++++--------------
>>>  hw/net/cadence_gem.c                |  95 ++++++--------------
>>>  include/hw/arm/xlnx-zynq-mp.h       |  29 +++++++
>>>  include/hw/char/cadence_uart.h      |  35 ++++++++
>>>  include/hw/net/cadence_gem.h        |  49 +++++++++++
>>>  target-arm/cpu64.c                  |  47 +++++++---
>>>  10 files changed, 456 insertions(+), 149 deletions(-)
>>>  create mode 100644 hw/arm/xlnx-zynq-mp-generic.c
>>>  create mode 100644 hw/arm/xlnx-zynq-mp.c
>>>  create mode 100644 include/hw/arm/xlnx-zynq-mp.h
>>>  create mode 100644 include/hw/char/cadence_uart.h
>>>  create mode 100644 include/hw/net/cadence_gem.h
>>>
>>> --
>>> 2.3.0.1.g27a12f1
>>>
>>>
>>
>

Re: [Qemu-devel] [PATCH target-arm v1 13/15] arm: xilinx-zynq-mp-generic: Add external RAM

[Peter Crosthwaite]

On Mon, Mar 2, 2015 at 2:38 PM, Alistair Francis
<alistair.francis@xilinx.com> wrote:
> On Tue, Mar 3, 2015 at 5:40 AM, Peter Crosthwaite
> <peter.crosthwaite@xilinx.com> wrote:
>> On Mon, Feb 23, 2015 at 6:24 PM, Alistair Francis
>> <alistair.francis@xilinx.com> wrote:
>>> On Tue, Feb 24, 2015 at 9:04 AM, Peter Crosthwaite
>>> <peter.crosthwaite@xilinx.com> wrote:
>>>> Zynq MPSoC supports external DDR RAM. Add a RAM at 0 to the model.
>>>>
>>>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>>> ---
>>>>  hw/arm/xlnx-zynq-mp-generic.c | 7 +++++++
>>>>  1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/hw/arm/xlnx-zynq-mp-generic.c b/hw/arm/xlnx-zynq-mp-generic.c
>>>> index ff69b07..7394e82 100644
>>>> --- a/hw/arm/xlnx-zynq-mp-generic.c
>>>> +++ b/hw/arm/xlnx-zynq-mp-generic.c
>>>> @@ -18,9 +18,11 @@
>>>>  #include "hw/arm/xlnx-zynq-mp.h"
>>>>  #include "hw/boards.h"
>>>>  #include "qemu/error-report.h"
>>>> +#include "exec/address-spaces.h"
>>>>
>>>>  typedef struct XlnxZynqMPGeneric {
>>>>      XlnxZynqMPState soc;
>>>> +    MemoryRegion ddr_ram;
>>>>  } XlnxZynqMPGeneric;
>>>>
>>>>  static void xlnx_zynq_mp_generic_init(MachineState *machine)
>>>> @@ -36,6 +38,11 @@ static void xlnx_zynq_mp_generic_init(MachineState *machine)
>>>>          error_report("%s", error_get_pretty(err));
>>>>          exit(1);
>>>>      }
>>>> +
>>>> +    memory_region_init_ram(&s->ddr_ram, NULL, "ddr-ram", machine->ram_size,
>>>> +                           &error_abort);
>>>
>>> Shouldn't there be a default size if none is specified? This looks
>>> like it will cause user
>>> issues if they don't understand that they must specify the memory size.
>>>
>>
>> So vl.c provides a core default of 128MB if -m is unspecified. It
>> should boot with that successfully. The error case is probably when
>> the user overrides -m to a low value such that the boot cant work
>> anymore, but that would be a error caught by the boot loader I think.
>
> I still think that this could cause issues. If a user doesn't realise that
> they have to specify -m and the boot still works, they will just assume
> that the correct memory size is in the machine model. When in fact all
> that is happening is they are getting a small amount of memory that is
> just lucky enough to work. 128MB is very small for ZynqMP.
>

So there is precedent for upper clamps on this but not lower clamps. I
think we may actually want the same as zynq with an upper clamp:

158     /* max 2GB ram */
159     if (ram_size > 0x80000000) {
160         ram_size = 0x80000000;
161     }

> Maybe instead of an error or a default just have a warning if the memory
> is below a certain size. That way it will still boot, but the user will know
> that they should be specifying a size.
>

Adding the warning for <= 128MB. Anything more than that then the user
must have manually specified -m so not too worried about that.

Regards,
Peter

> Thanks,
>
> Alistair
>
>>
>> Regards,
>> Peter
>>
>>> At least return an error if none is specified.
>>>
>>> Thanks,
>>>
>>> Alistair
>>>
>>>> +    vmstate_register_ram_global(&s->ddr_ram);
>>>> +    memory_region_add_subregion(get_system_memory(), 0, &s->ddr_ram);
>>>>  }
>>>>
>>>>  static QEMUMachine xlnx_zynq_mp_generic_machine = {
>>>> --
>>>> 2.3.0.1.g27a12f1
>>>>
>>>>
>>>
>>
>

Re: [Qemu-devel] [PATCH target-arm v1 00/15] Next Generation Xilinx Zynq SoC

[Peter Crosthwaite]

On Mon, Mar 2, 2015 at 2:53 PM, Alistair Francis
<alistair.francis@xilinx.com> wrote:
> On Tue, Mar 3, 2015 at 6:06 AM, Peter Crosthwaite
> <peter.crosthwaite@xilinx.com> wrote:
>> On Thu, Feb 26, 2015 at 7:38 PM, Alistair Francis
>> <alistair.francis@xilinx.com> wrote:
>>> On Tue, Feb 24, 2015 at 9:04 AM, Peter Crosthwaite
>>> <peter.crosthwaite@xilinx.com> wrote:
>>>> Hi Peter and all,
>>>>
>>>> Xilinx's next gen SoC has been announced. This series adds a SoC and
>>>> machine model.
>>>>
>>>> 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 generic machine model that exposes just the RAW SoC itself. The
>>>> only external device modelled is DDR RAM, as driven by the -m option.
>>>> The standard bootloader and PSCI support is used.
>>>>
>>>> Regards,
>>>> Peter
>>>>
>>>>
>>>> Peter Crosthwaite (15):
>>>>   target-arm: cpu64: Factor out ARM cortex init
>>>>   target-arm: cpu64: Add support for cortex-a53
>>>>   arm: Introduce Xilinx Zynq MPSoC
>>>>   arm: xlnx-zynq-mp: Add GIC
>>>>   arm: xlnx-zynq-mp: Connect CPU Timers to GIC
>>>>   net: cadence_gem: Clean up variable names
>>>>   net: cadence_gem: Split state struct and type into header
>>>>   arm: xilinx-zynq-mp: Add GEM support
>>>>   char: cadence_uart: Clean up variable names
>>>>   char: cadence_uart: Split state struct and type into header
>>>>   arm: xilinx-zynq-mp: Add UART support
>>>>   arm: Add xilinx-zynq-mp-generic machine
>>>>   arm: xilinx-zynq-mp-generic: Add external RAM
>>>>   arm: xilinx-zynq-mp-generic: Add bootloading
>>>>   arm: xlnx-zynq-mp: Add PSCI setup
>>>>
>>>>  default-configs/aarch64-softmmu.mak |   2 +-
>>>>  hw/arm/Makefile.objs                |   1 +
>>>>  hw/arm/xlnx-zynq-mp-generic.c       |  67 +++++++++++++++
>>>>  hw/arm/xlnx-zynq-mp.c               | 167 ++++++++++++++++++++++++++++++++++++
>>>
>>> These two files need to be renamed. You can' tell what each one is
>>> from the title.
>
> Just re-read this, sorry about the 'need to be'.
>
>>>
>>
>> So the unqualified "zynqmp" should imply the SoC. This currently
>> applies to allwinner a10 and digic so Im trying to follow that
>> convention. When we start using specific boards, the zynqmp will be
>
> I see your point, but Allwinner only have one file in the hw/arm/ folder
> that starts with 'allwinner', making it much clearer what it is. Digic and
> Exynos also have the '_boards' suffix on their boards, which separates
> the SoC from the board/boards.
>
> With that logic the 'xlnx-zynq-mp-generic.c' probably should have a
> 'board' in there somewhere.
>
>> dropped from the name completely. We could instead use the specific
>> board name ep108 but I wanted that to be additive. The board would be
>> called:
>>
>> hw/arm/xlnx-ep108.c
>>
>> by current conventions. One alternative would be just go out with
>> ep108 board only and no generic machine if you think that works
>> better?
>
> That naming is clearer, but only if the user knows what the EP108 is.
> If they are just looking to 'boot Xilinx's ZynqMP chip' then it might be a
> bit confusing. In saying that, I don't think it's a big issue and I would
> be fine with having a 'xlnx-ep108.c' board
>

Thinking we just go with EP108? That is what Michal has done with the
Linux kernel.

>>
>>> It should be something like 'xilinx_zynqmp_soc.c' and
>>> 'xilinx_zynqmp_machine.c' (or maybe just 'xilinx_zynqmp.c').
>>>
>>
>> I'm trying to start using the truncated vendor name xlnx instead of
>> xilinx going forward (there is already in-tree precedent) as the 3
>> saved chars does save on a few 80 char line wraps. This is also the
>> convention used in device tree compatible strings.
>
> Ok, makes sense. Is it possible to change the current 'xilinx_zynq.c'
> file then, that way everything matches? I guess that could break current
> implementations though (epically if the actual machine name changes).
>

We could make this change to all the variable, fn and file names for
consistency, but we would not be able to change the machine name (or
any other user visible strings).

Regards,
Peter

> Thanks,
>
> Alistair
>
>>
>> Regards,
>> Peter
>>
>>> That way it also matches the same style as the current Zynq machine.
>>>
>>> Thanks,
>>>
>>> Alistair
>>>
>>>>  hw/char/cadence_uart.c              | 113 ++++++++++--------------
>>>>  hw/net/cadence_gem.c                |  95 ++++++--------------
>>>>  include/hw/arm/xlnx-zynq-mp.h       |  29 +++++++
>>>>  include/hw/char/cadence_uart.h      |  35 ++++++++
>>>>  include/hw/net/cadence_gem.h        |  49 +++++++++++
>>>>  target-arm/cpu64.c                  |  47 +++++++---
>>>>  10 files changed, 456 insertions(+), 149 deletions(-)
>>>>  create mode 100644 hw/arm/xlnx-zynq-mp-generic.c
>>>>  create mode 100644 hw/arm/xlnx-zynq-mp.c
>>>>  create mode 100644 include/hw/arm/xlnx-zynq-mp.h
>>>>  create mode 100644 include/hw/char/cadence_uart.h
>>>>  create mode 100644 include/hw/net/cadence_gem.h
>>>>
>>>> --
>>>> 2.3.0.1.g27a12f1
>>>>
>>>>
>>>
>>
>

Re: [Qemu-devel] [PATCH target-arm v1 13/15] arm: xilinx-zynq-mp-generic: Add external RAM

[Alistair Francis]

On Tue, Mar 3, 2015 at 8:59 AM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> On Mon, Mar 2, 2015 at 2:38 PM, Alistair Francis
> <alistair.francis@xilinx.com> wrote:
>> On Tue, Mar 3, 2015 at 5:40 AM, Peter Crosthwaite
>> <peter.crosthwaite@xilinx.com> wrote:
>>> On Mon, Feb 23, 2015 at 6:24 PM, Alistair Francis
>>> <alistair.francis@xilinx.com> wrote:
>>>> On Tue, Feb 24, 2015 at 9:04 AM, Peter Crosthwaite
>>>> <peter.crosthwaite@xilinx.com> wrote:
>>>>> Zynq MPSoC supports external DDR RAM. Add a RAM at 0 to the model.
>>>>>
>>>>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>>>> ---
>>>>>  hw/arm/xlnx-zynq-mp-generic.c | 7 +++++++
>>>>>  1 file changed, 7 insertions(+)
>>>>>
>>>>> diff --git a/hw/arm/xlnx-zynq-mp-generic.c b/hw/arm/xlnx-zynq-mp-generic.c
>>>>> index ff69b07..7394e82 100644
>>>>> --- a/hw/arm/xlnx-zynq-mp-generic.c
>>>>> +++ b/hw/arm/xlnx-zynq-mp-generic.c
>>>>> @@ -18,9 +18,11 @@
>>>>>  #include "hw/arm/xlnx-zynq-mp.h"
>>>>>  #include "hw/boards.h"
>>>>>  #include "qemu/error-report.h"
>>>>> +#include "exec/address-spaces.h"
>>>>>
>>>>>  typedef struct XlnxZynqMPGeneric {
>>>>>      XlnxZynqMPState soc;
>>>>> +    MemoryRegion ddr_ram;
>>>>>  } XlnxZynqMPGeneric;
>>>>>
>>>>>  static void xlnx_zynq_mp_generic_init(MachineState *machine)
>>>>> @@ -36,6 +38,11 @@ static void xlnx_zynq_mp_generic_init(MachineState *machine)
>>>>>          error_report("%s", error_get_pretty(err));
>>>>>          exit(1);
>>>>>      }
>>>>> +
>>>>> +    memory_region_init_ram(&s->ddr_ram, NULL, "ddr-ram", machine->ram_size,
>>>>> +                           &error_abort);
>>>>
>>>> Shouldn't there be a default size if none is specified? This looks
>>>> like it will cause user
>>>> issues if they don't understand that they must specify the memory size.
>>>>
>>>
>>> So vl.c provides a core default of 128MB if -m is unspecified. It
>>> should boot with that successfully. The error case is probably when
>>> the user overrides -m to a low value such that the boot cant work
>>> anymore, but that would be a error caught by the boot loader I think.
>>
>> I still think that this could cause issues. If a user doesn't realise that
>> they have to specify -m and the boot still works, they will just assume
>> that the correct memory size is in the machine model. When in fact all
>> that is happening is they are getting a small amount of memory that is
>> just lucky enough to work. 128MB is very small for ZynqMP.
>>
>
> So there is precedent for upper clamps on this but not lower clamps. I
> think we may actually want the same as zynq with an upper clamp:
>
> 158     /* max 2GB ram */
> 159     if (ram_size > 0x80000000) {
> 160         ram_size = 0x80000000;
> 161     }
>
>> Maybe instead of an error or a default just have a warning if the memory
>> is below a certain size. That way it will still boot, but the user will know
>> that they should be specifying a size.
>>
>
> Adding the warning for <= 128MB. Anything more than that then the user
> must have manually specified -m so not too worried about that.

Great, both sound good

Thanks,

Alistair

>
> Regards,
> Peter
>
>> Thanks,
>>
>> Alistair
>>
>>>
>>> Regards,
>>> Peter
>>>
>>>> At least return an error if none is specified.
>>>>
>>>> Thanks,
>>>>
>>>> Alistair
>>>>
>>>>> +    vmstate_register_ram_global(&s->ddr_ram);
>>>>> +    memory_region_add_subregion(get_system_memory(), 0, &s->ddr_ram);
>>>>>  }
>>>>>
>>>>>  static QEMUMachine xlnx_zynq_mp_generic_machine = {
>>>>> --
>>>>> 2.3.0.1.g27a12f1
>>>>>
>>>>>
>>>>
>>>
>>
>

Re: [Qemu-devel] [PATCH target-arm v1 00/15] Next Generation Xilinx Zynq SoC

[Alistair Francis]

On Tue, Mar 3, 2015 at 9:05 AM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> On Mon, Mar 2, 2015 at 2:53 PM, Alistair Francis
> <alistair.francis@xilinx.com> wrote:
>> On Tue, Mar 3, 2015 at 6:06 AM, Peter Crosthwaite
>> <peter.crosthwaite@xilinx.com> wrote:
>>> On Thu, Feb 26, 2015 at 7:38 PM, Alistair Francis
>>> <alistair.francis@xilinx.com> wrote:
>>>> On Tue, Feb 24, 2015 at 9:04 AM, Peter Crosthwaite
>>>> <peter.crosthwaite@xilinx.com> wrote:
>>>>> Hi Peter and all,
>>>>>
>>>>> Xilinx's next gen SoC has been announced. This series adds a SoC and
>>>>> machine model.
>>>>>
>>>>> 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 generic machine model that exposes just the RAW SoC itself. The
>>>>> only external device modelled is DDR RAM, as driven by the -m option.
>>>>> The standard bootloader and PSCI support is used.
>>>>>
>>>>> Regards,
>>>>> Peter
>>>>>
>>>>>
>>>>> Peter Crosthwaite (15):
>>>>>   target-arm: cpu64: Factor out ARM cortex init
>>>>>   target-arm: cpu64: Add support for cortex-a53
>>>>>   arm: Introduce Xilinx Zynq MPSoC
>>>>>   arm: xlnx-zynq-mp: Add GIC
>>>>>   arm: xlnx-zynq-mp: Connect CPU Timers to GIC
>>>>>   net: cadence_gem: Clean up variable names
>>>>>   net: cadence_gem: Split state struct and type into header
>>>>>   arm: xilinx-zynq-mp: Add GEM support
>>>>>   char: cadence_uart: Clean up variable names
>>>>>   char: cadence_uart: Split state struct and type into header
>>>>>   arm: xilinx-zynq-mp: Add UART support
>>>>>   arm: Add xilinx-zynq-mp-generic machine
>>>>>   arm: xilinx-zynq-mp-generic: Add external RAM
>>>>>   arm: xilinx-zynq-mp-generic: Add bootloading
>>>>>   arm: xlnx-zynq-mp: Add PSCI setup
>>>>>
>>>>>  default-configs/aarch64-softmmu.mak |   2 +-
>>>>>  hw/arm/Makefile.objs                |   1 +
>>>>>  hw/arm/xlnx-zynq-mp-generic.c       |  67 +++++++++++++++
>>>>>  hw/arm/xlnx-zynq-mp.c               | 167 ++++++++++++++++++++++++++++++++++++
>>>>
>>>> These two files need to be renamed. You can' tell what each one is
>>>> from the title.
>>
>> Just re-read this, sorry about the 'need to be'.
>>
>>>>
>>>
>>> So the unqualified "zynqmp" should imply the SoC. This currently
>>> applies to allwinner a10 and digic so Im trying to follow that
>>> convention. When we start using specific boards, the zynqmp will be
>>
>> I see your point, but Allwinner only have one file in the hw/arm/ folder
>> that starts with 'allwinner', making it much clearer what it is. Digic and
>> Exynos also have the '_boards' suffix on their boards, which separates
>> the SoC from the board/boards.
>>
>> With that logic the 'xlnx-zynq-mp-generic.c' probably should have a
>> 'board' in there somewhere.
>>
>>> dropped from the name completely. We could instead use the specific
>>> board name ep108 but I wanted that to be additive. The board would be
>>> called:
>>>
>>> hw/arm/xlnx-ep108.c
>>>
>>> by current conventions. One alternative would be just go out with
>>> ep108 board only and no generic machine if you think that works
>>> better?
>>
>> That naming is clearer, but only if the user knows what the EP108 is.
>> If they are just looking to 'boot Xilinx's ZynqMP chip' then it might be a
>> bit confusing. In saying that, I don't think it's a big issue and I would
>> be fine with having a 'xlnx-ep108.c' board
>>
>
> Thinking we just go with EP108? That is what Michal has done with the
> Linux kernel.

I think that is probably the best option. Especially if it is what we are doing
with the Linux kernel.

>
>>>
>>>> It should be something like 'xilinx_zynqmp_soc.c' and
>>>> 'xilinx_zynqmp_machine.c' (or maybe just 'xilinx_zynqmp.c').
>>>>
>>>
>>> I'm trying to start using the truncated vendor name xlnx instead of
>>> xilinx going forward (there is already in-tree precedent) as the 3
>>> saved chars does save on a few 80 char line wraps. This is also the
>>> convention used in device tree compatible strings.
>>
>> Ok, makes sense. Is it possible to change the current 'xilinx_zynq.c'
>> file then, that way everything matches? I guess that could break current
>> implementations though (epically if the actual machine name changes).
>>
>
> We could make this change to all the variable, fn and file names for
> consistency, but we would not be able to change the machine name (or
> any other user visible strings).

We should probably start to do (it probably can wait till once this set is
accepted). That way all it will be as consistent as possible.

Thanks,

Alistair

>
> Regards,
> Peter
>
>> Thanks,
>>
>> Alistair
>>
>>>
>>> Regards,
>>> Peter
>>>
>>>> That way it also matches the same style as the current Zynq machine.
>>>>
>>>> Thanks,
>>>>
>>>> Alistair
>>>>
>>>>>  hw/char/cadence_uart.c              | 113 ++++++++++--------------
>>>>>  hw/net/cadence_gem.c                |  95 ++++++--------------
>>>>>  include/hw/arm/xlnx-zynq-mp.h       |  29 +++++++
>>>>>  include/hw/char/cadence_uart.h      |  35 ++++++++
>>>>>  include/hw/net/cadence_gem.h        |  49 +++++++++++
>>>>>  target-arm/cpu64.c                  |  47 +++++++---
>>>>>  10 files changed, 456 insertions(+), 149 deletions(-)
>>>>>  create mode 100644 hw/arm/xlnx-zynq-mp-generic.c
>>>>>  create mode 100644 hw/arm/xlnx-zynq-mp.c
>>>>>  create mode 100644 include/hw/arm/xlnx-zynq-mp.h
>>>>>  create mode 100644 include/hw/char/cadence_uart.h
>>>>>  create mode 100644 include/hw/net/cadence_gem.h
>>>>>
>>>>> --
>>>>> 2.3.0.1.g27a12f1
>>>>>
>>>>>
>>>>
>>>
>>
>