[Qemu-devel] [PATCH v1 00/12] arm: Add first models of Xilinx Versal SoC

[Qemu-devel] [PATCH v1 00/12] arm: Add first models of Xilinx Versal SoC

[Edgar E. Iglesias]

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

This patch series adds initial support for Xilinx's Versal SoC.
Xilinx is introducing Versal, an adaptive compute acceleration platform
(ACAP), built on 7nm FinFET process technology. Versal ACAPs combine Scalar
Processing Engines, Adaptable Hardware Engines, and Intelligent Engines with
leading-edge memory and interfacing technologies to deliver powerful
heterogeneous acceleration for any application. The Versal AI Core series has
five devices, offering 128 to 400 AI Engines. The series includes dual-core Arm
Cortex-A72 application processors, dual-core Arm Cortex-R5 real-time
processors, 256KB of on-chip memory with ECC, more than 1,900 DSP engines
optimized for high-precision floating point with low latency.

More info can be found here:
https://www.xilinx.com/news/press/2018/xilinx-unveils-versal-the-first-in-a-new-category-of-platforms-delivering-rapid-innovation-with-software-programmability-and-scalable-ai-inference.html


In QEMU we'd like to have a virtual developer board with the Versal SoC
and a selected set of peripherals under the control of QEMU.
We'd like to gradually extend this board as QEMU gains more support
for Versal hardware components. QEMU will generate a device-tree
describing only the components it supports and includes in the virtual
dev board.

Before adding Versal support, this series starts with a few fixes to the
GEM that I ran into when running recent kernels on the Versal and ZynqMP
models.

I also noticed a problem with HVC insns not being enabled when using
QEMU's PSCI implementation on CPU's with EL2 and EL3 enabled. This causes
problems for Linux/KVM guests, also fixed in this series.

Best regards,
Edgar

Edgar E. Iglesias (12):
  net: cadence_gem: Disable TSU feature bit
  net: cadence_gem: Announce availability of priority queues
  net: cadence_gem: Use uint32_t for 32bit descriptor words
  net: cadence_gem: Add macro with max number of descriptor words
  net: cadence_gem: Add support for extended descriptors
  net: cadence_gem: Add support for selecting the DMA MemoryRegion
  net: cadence_gem: Implement support for 64bit descriptor addresses
  net: cadence_gem: Announce 64bit addressing support
  target-arm: powerctl: Enable HVC when starting CPUs to EL2
  target/arm: Add the Cortex-A72
  hw/arm: versal: Add a model of Xilinx Versal SoC
  hw/arm: versal: Add a virtual Xilinx Versal board

 default-configs/aarch64-softmmu.mak |   1 +
 hw/arm/Makefile.objs                |   1 +
 hw/arm/xlnx-versal-virt.c           | 494 ++++++++++++++++++++++++++++++++++++
 hw/arm/xlnx-versal.c                | 339 +++++++++++++++++++++++++
 hw/net/cadence_gem.c                | 196 ++++++++++----
 include/hw/arm/xlnx-versal.h        | 122 +++++++++
 include/hw/net/cadence_gem.h        |   7 +-
 target/arm/arm-powerctl.c           |  11 +
 target/arm/cpu64.c                  |  59 +++++
 9 files changed, 1175 insertions(+), 55 deletions(-)
 create mode 100644 hw/arm/xlnx-versal-virt.c
 create mode 100644 hw/arm/xlnx-versal.c
 create mode 100644 include/hw/arm/xlnx-versal.h

-- 
2.7.4

[Qemu-devel] [PATCH v1 01/12] net: cadence_gem: Disable TSU feature bit

[Edgar E. Iglesias]

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

Disable the Timestamping Unit feature bit since QEMU does not
yet support it. This allows guest SW to correctly probe for
its existance.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 hw/net/cadence_gem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 0fa4b0d..e560b7a 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -1228,7 +1228,7 @@ static void gem_reset(DeviceState *d)
     s->regs[GEM_MODID] = s->revision;
     s->regs[GEM_DESCONF] = 0x02500111;
     s->regs[GEM_DESCONF2] = 0x2ab13fff;
-    s->regs[GEM_DESCONF5] = 0x002f2145;
+    s->regs[GEM_DESCONF5] = 0x002f2045;
     s->regs[GEM_DESCONF6] = 0x00000200;
 
     /* Set MAC address */
-- 
2.7.4

[Qemu-devel] [PATCH v1 02/12] net: cadence_gem: Announce availability of priority queues

[Edgar E. Iglesias]

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

Announce the availability of the various priority queues.
This fixes an issue where guest kernels would miss to
configure secondary queues due to inproper feature bits.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 hw/net/cadence_gem.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index e560b7a..901c173 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -1213,6 +1213,7 @@ static void gem_reset(DeviceState *d)
     int i;
     CadenceGEMState *s = CADENCE_GEM(d);
     const uint8_t *a;
+    uint32_t queues_mask;
 
     DB_PRINT("\n");
 
@@ -1229,7 +1230,10 @@ static void gem_reset(DeviceState *d)
     s->regs[GEM_DESCONF] = 0x02500111;
     s->regs[GEM_DESCONF2] = 0x2ab13fff;
     s->regs[GEM_DESCONF5] = 0x002f2045;
-    s->regs[GEM_DESCONF6] = 0x00000200;
+    s->regs[GEM_DESCONF6] = 0x0;
+
+    queues_mask = MAKE_64BIT_MASK(1, s->num_priority_queues - 1);
+    s->regs[GEM_DESCONF6] |= queues_mask;
 
     /* Set MAC address */
     a = &s->conf.macaddr.a[0];
-- 
2.7.4

[Qemu-devel] [PATCH v1 03/12] net: cadence_gem: Use uint32_t for 32bit descriptor words

[Edgar E. Iglesias]

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

Use uint32_t instead of unsigned to describe 32bit descriptor words.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 hw/net/cadence_gem.c         | 42 +++++++++++++++++++++---------------------
 include/hw/net/cadence_gem.h |  2 +-
 2 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 901c173..31f3fe0 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -302,42 +302,42 @@
 
 #define GEM_MODID_VALUE 0x00020118
 
-static inline unsigned tx_desc_get_buffer(unsigned *desc)
+static inline unsigned tx_desc_get_buffer(uint32_t *desc)
 {
     return desc[0];
 }
 
-static inline unsigned tx_desc_get_used(unsigned *desc)
+static inline unsigned tx_desc_get_used(uint32_t *desc)
 {
     return (desc[1] & DESC_1_USED) ? 1 : 0;
 }
 
-static inline void tx_desc_set_used(unsigned *desc)
+static inline void tx_desc_set_used(uint32_t *desc)
 {
     desc[1] |= DESC_1_USED;
 }
 
-static inline unsigned tx_desc_get_wrap(unsigned *desc)
+static inline unsigned tx_desc_get_wrap(uint32_t *desc)
 {
     return (desc[1] & DESC_1_TX_WRAP) ? 1 : 0;
 }
 
-static inline unsigned tx_desc_get_last(unsigned *desc)
+static inline unsigned tx_desc_get_last(uint32_t *desc)
 {
     return (desc[1] & DESC_1_TX_LAST) ? 1 : 0;
 }
 
-static inline void tx_desc_set_last(unsigned *desc)
+static inline void tx_desc_set_last(uint32_t *desc)
 {
     desc[1] |= DESC_1_TX_LAST;
 }
 
-static inline unsigned tx_desc_get_length(unsigned *desc)
+static inline unsigned tx_desc_get_length(uint32_t *desc)
 {
     return desc[1] & DESC_1_LENGTH;
 }
 
-static inline void print_gem_tx_desc(unsigned *desc, uint8_t queue)
+static inline void print_gem_tx_desc(uint32_t *desc, uint8_t queue)
 {
     DB_PRINT("TXDESC (queue %" PRId8 "):\n", queue);
     DB_PRINT("bufaddr: 0x%08x\n", *desc);
@@ -347,58 +347,58 @@ static inline void print_gem_tx_desc(unsigned *desc, uint8_t queue)
     DB_PRINT("length:  %d\n", tx_desc_get_length(desc));
 }
 
-static inline unsigned rx_desc_get_buffer(unsigned *desc)
+static inline unsigned rx_desc_get_buffer(uint32_t *desc)
 {
     return desc[0] & ~0x3UL;
 }
 
-static inline unsigned rx_desc_get_wrap(unsigned *desc)
+static inline unsigned rx_desc_get_wrap(uint32_t *desc)
 {
     return desc[0] & DESC_0_RX_WRAP ? 1 : 0;
 }
 
-static inline unsigned rx_desc_get_ownership(unsigned *desc)
+static inline unsigned rx_desc_get_ownership(uint32_t *desc)
 {
     return desc[0] & DESC_0_RX_OWNERSHIP ? 1 : 0;
 }
 
-static inline void rx_desc_set_ownership(unsigned *desc)
+static inline void rx_desc_set_ownership(uint32_t *desc)
 {
     desc[0] |= DESC_0_RX_OWNERSHIP;
 }
 
-static inline void rx_desc_set_sof(unsigned *desc)
+static inline void rx_desc_set_sof(uint32_t *desc)
 {
     desc[1] |= DESC_1_RX_SOF;
 }
 
-static inline void rx_desc_set_eof(unsigned *desc)
+static inline void rx_desc_set_eof(uint32_t *desc)
 {
     desc[1] |= DESC_1_RX_EOF;
 }
 
-static inline void rx_desc_set_length(unsigned *desc, unsigned len)
+static inline void rx_desc_set_length(uint32_t *desc, unsigned len)
 {
     desc[1] &= ~DESC_1_LENGTH;
     desc[1] |= len;
 }
 
-static inline void rx_desc_set_broadcast(unsigned *desc)
+static inline void rx_desc_set_broadcast(uint32_t *desc)
 {
     desc[1] |= R_DESC_1_RX_BROADCAST;
 }
 
-static inline void rx_desc_set_unicast_hash(unsigned *desc)
+static inline void rx_desc_set_unicast_hash(uint32_t *desc)
 {
     desc[1] |= R_DESC_1_RX_UNICAST_HASH;
 }
 
-static inline void rx_desc_set_multicast_hash(unsigned *desc)
+static inline void rx_desc_set_multicast_hash(uint32_t *desc)
 {
     desc[1] |= R_DESC_1_RX_MULTICAST_HASH;
 }
 
-static inline void rx_desc_set_sar(unsigned *desc, int sar_idx)
+static inline void rx_desc_set_sar(uint32_t *desc, int sar_idx)
 {
     desc[1] = deposit32(desc[1], R_DESC_1_RX_SAR_SHIFT, R_DESC_1_RX_SAR_LENGTH,
                         sar_idx);
@@ -1042,7 +1042,7 @@ static void gem_transmit_updatestats(CadenceGEMState *s, const uint8_t *packet,
  */
 static void gem_transmit(CadenceGEMState *s)
 {
-    unsigned    desc[2];
+    uint32_t desc[2];
     hwaddr packet_desc_addr;
     uint8_t     tx_packet[2048];
     uint8_t     *p;
@@ -1108,7 +1108,7 @@ static void gem_transmit(CadenceGEMState *s)
 
             /* Last descriptor for this packet; hand the whole thing off */
             if (tx_desc_get_last(desc)) {
-                unsigned    desc_first[2];
+                uint32_t desc_first[2];
 
                 /* Modify the 1st descriptor of this packet to be owned by
                  * the processor.
diff --git a/include/hw/net/cadence_gem.h b/include/hw/net/cadence_gem.h
index 35de622..633d564 100644
--- a/include/hw/net/cadence_gem.h
+++ b/include/hw/net/cadence_gem.h
@@ -74,7 +74,7 @@ typedef struct CadenceGEMState {
 
     uint8_t can_rx_state; /* Debug only */
 
-    unsigned rx_desc[MAX_PRIORITY_QUEUES][2];
+    uint32_t rx_desc[MAX_PRIORITY_QUEUES][2];
 
     bool sar_active[4];
 } CadenceGEMState;
-- 
2.7.4

[Qemu-devel] [PATCH v1 04/12] net: cadence_gem: Add macro with max number of descriptor words

[Edgar E. Iglesias]

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

Add macro with max number of DMA descriptor words.
No functional change.

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

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 31f3fe0..4d769b0 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -1042,7 +1042,7 @@ static void gem_transmit_updatestats(CadenceGEMState *s, const uint8_t *packet,
  */
 static void gem_transmit(CadenceGEMState *s)
 {
-    uint32_t desc[2];
+    uint32_t desc[DESC_MAX_NUM_WORDS];
     hwaddr packet_desc_addr;
     uint8_t     tx_packet[2048];
     uint8_t     *p;
@@ -1108,7 +1108,7 @@ static void gem_transmit(CadenceGEMState *s)
 
             /* Last descriptor for this packet; hand the whole thing off */
             if (tx_desc_get_last(desc)) {
-                uint32_t desc_first[2];
+                uint32_t desc_first[DESC_MAX_NUM_WORDS];
 
                 /* Modify the 1st descriptor of this packet to be owned by
                  * the processor.
diff --git a/include/hw/net/cadence_gem.h b/include/hw/net/cadence_gem.h
index 633d564..b33ef65 100644
--- a/include/hw/net/cadence_gem.h
+++ b/include/hw/net/cadence_gem.h
@@ -32,6 +32,9 @@
 
 #define CADENCE_GEM_MAXREG        (0x00000800 / 4) /* Last valid GEM address */
 
+/* Max number of words in a DMA descriptor.  */
+#define DESC_MAX_NUM_WORDS              2
+
 #define MAX_PRIORITY_QUEUES             8
 #define MAX_TYPE1_SCREENERS             16
 #define MAX_TYPE2_SCREENERS             16
@@ -74,7 +77,7 @@ typedef struct CadenceGEMState {
 
     uint8_t can_rx_state; /* Debug only */
 
-    uint32_t rx_desc[MAX_PRIORITY_QUEUES][2];
+    uint32_t rx_desc[MAX_PRIORITY_QUEUES][DESC_MAX_NUM_WORDS];
 
     bool sar_active[4];
 } CadenceGEMState;
-- 
2.7.4

[Qemu-devel] [PATCH v1 05/12] net: cadence_gem: Add support for extended descriptors

[Edgar E. Iglesias]

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

Add support for extended descriptors with optional 64bit
addressing and timestamping. QEMU will not yet provide
timestamps (always leaving the valid timestamp bit as zero).

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 hw/net/cadence_gem.c         | 69 ++++++++++++++++++++++++++++++++------------
 include/hw/net/cadence_gem.h |  2 +-
 2 files changed, 52 insertions(+), 19 deletions(-)

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 4d769b0..759c1d7 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -207,6 +207,9 @@
 #define GEM_NWCFG_BCAST_REJ    0x00000020 /* Reject broadcast packets */
 #define GEM_NWCFG_PROMISC      0x00000010 /* Accept all packets */
 
+#define GEM_DMACFG_ADDR_64B    (1U << 30)
+#define GEM_DMACFG_TX_BD_EXT   (1U << 29)
+#define GEM_DMACFG_RX_BD_EXT   (1U << 28)
 #define GEM_DMACFG_RBUFSZ_M    0x00FF0000 /* DMA RX Buffer Size mask */
 #define GEM_DMACFG_RBUFSZ_S    16         /* DMA RX Buffer Size shift */
 #define GEM_DMACFG_RBUFSZ_MUL  64         /* DMA RX Buffer Size multiplier */
@@ -302,9 +305,14 @@
 
 #define GEM_MODID_VALUE 0x00020118
 
-static inline unsigned tx_desc_get_buffer(uint32_t *desc)
+static inline uint64_t tx_desc_get_buffer(CadenceGEMState *s, uint32_t *desc)
 {
-    return desc[0];
+    uint64_t ret = desc[0];
+
+    if (s->regs[GEM_DMACFG] & GEM_DMACFG_ADDR_64B) {
+        ret |= (uint64_t)desc[2] << 32;
+    }
+    return ret;
 }
 
 static inline unsigned tx_desc_get_used(uint32_t *desc)
@@ -347,9 +355,30 @@ static inline void print_gem_tx_desc(uint32_t *desc, uint8_t queue)
     DB_PRINT("length:  %d\n", tx_desc_get_length(desc));
 }
 
-static inline unsigned rx_desc_get_buffer(uint32_t *desc)
+static inline uint64_t rx_desc_get_buffer(CadenceGEMState *s, uint32_t *desc)
 {
-    return desc[0] & ~0x3UL;
+    uint64_t ret = desc[0] & ~0x3UL;
+
+    if (s->regs[GEM_DMACFG] & GEM_DMACFG_ADDR_64B) {
+        ret |= (uint64_t)desc[2] << 32;
+    }
+    return ret;
+}
+
+static inline int gem_get_desc_len(CadenceGEMState *s, bool rx_n_tx)
+{
+    int ret = 2;
+
+    if (s->regs[GEM_DMACFG] & GEM_DMACFG_ADDR_64B) {
+        ret += 2;
+    }
+    if (s->regs[GEM_DMACFG] & (rx_n_tx ? GEM_DMACFG_RX_BD_EXT
+                                       : GEM_DMACFG_TX_BD_EXT)) {
+        ret += 2;
+    }
+
+    assert(ret <= DESC_MAX_NUM_WORDS);
+    return ret;
 }
 
 static inline unsigned rx_desc_get_wrap(uint32_t *desc)
@@ -419,7 +448,7 @@ static void gem_init_register_masks(CadenceGEMState *s)
     memset(&s->regs_ro[0], 0, sizeof(s->regs_ro));
     s->regs_ro[GEM_NWCTRL]   = 0xFFF80000;
     s->regs_ro[GEM_NWSTATUS] = 0xFFFFFFFF;
-    s->regs_ro[GEM_DMACFG]   = 0xFE00F000;
+    s->regs_ro[GEM_DMACFG]   = 0x8E00F000;
     s->regs_ro[GEM_TXSTATUS] = 0xFFFFFE08;
     s->regs_ro[GEM_RXQBASE]  = 0x00000003;
     s->regs_ro[GEM_TXQBASE]  = 0x00000003;
@@ -807,7 +836,8 @@ static void gem_get_rx_desc(CadenceGEMState *s, int q)
     DB_PRINT("read descriptor 0x%x\n", (unsigned)s->rx_desc_addr[q]);
     /* read current descriptor */
     cpu_physical_memory_read(s->rx_desc_addr[q],
-                             (uint8_t *)s->rx_desc[q], sizeof(s->rx_desc[q]));
+                             (uint8_t *)s->rx_desc[q],
+                             sizeof(uint32_t) * gem_get_desc_len(s, true));
 
     /* Descriptor owned by software ? */
     if (rx_desc_get_ownership(s->rx_desc[q]) == 1) {
@@ -926,9 +956,10 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
                 rx_desc_get_buffer(s->rx_desc[q]));
 
         /* Copy packet data to emulated DMA buffer */
-        cpu_physical_memory_write(rx_desc_get_buffer(s->rx_desc[q]) +
-                                                                 rxbuf_offset,
-                                  rxbuf_ptr, MIN(bytes_to_copy, rxbufsize));
+        cpu_physical_memory_write(rx_desc_get_buffer(s, s->rx_desc[q]) +
+                                                                  rxbuf_offset,
+                                  rxbuf_ptr,
+                                  MIN(bytes_to_copy, rxbufsize));
         rxbuf_ptr += MIN(bytes_to_copy, rxbufsize);
         bytes_to_copy -= MIN(bytes_to_copy, rxbufsize);
 
@@ -964,7 +995,7 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
         /* Descriptor write-back.  */
         cpu_physical_memory_write(s->rx_desc_addr[q],
                                   (uint8_t *)s->rx_desc[q],
-                                  sizeof(s->rx_desc[q]));
+                                  sizeof(uint32_t) * gem_get_desc_len(s, true));
 
         /* Next descriptor */
         if (rx_desc_get_wrap(s->rx_desc[q])) {
@@ -972,7 +1003,7 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
             s->rx_desc_addr[q] = s->regs[GEM_RXQBASE];
         } else {
             DB_PRINT("incrementing RX descriptor list\n");
-            s->rx_desc_addr[q] += 8;
+            s->rx_desc_addr[q] += 4 * gem_get_desc_len(s, true);
         }
 
         gem_get_rx_desc(s, q);
@@ -1069,7 +1100,8 @@ static void gem_transmit(CadenceGEMState *s)
 
         DB_PRINT("read descriptor 0x%" HWADDR_PRIx "\n", packet_desc_addr);
         cpu_physical_memory_read(packet_desc_addr,
-                                 (uint8_t *)desc, sizeof(desc));
+                                 (uint8_t *)desc,
+                                 sizeof(uint32_t) * gem_get_desc_len(s, false));
         /* Handle all descriptors owned by hardware */
         while (tx_desc_get_used(desc) == 0) {
 
@@ -1082,7 +1114,7 @@ static void gem_transmit(CadenceGEMState *s)
             /* The real hardware would eat this (and possibly crash).
              * For QEMU let's lend a helping hand.
              */
-            if ((tx_desc_get_buffer(desc) == 0) ||
+            if ((tx_desc_get_buffer(s, desc) == 0) ||
                 (tx_desc_get_length(desc) == 0)) {
                 DB_PRINT("Invalid TX descriptor @ 0x%x\n",
                          (unsigned)packet_desc_addr);
@@ -1101,7 +1133,7 @@ static void gem_transmit(CadenceGEMState *s)
             /* Gather this fragment of the packet from "dma memory" to our
              * contig buffer.
              */
-            cpu_physical_memory_read(tx_desc_get_buffer(desc), p,
+            cpu_physical_memory_read(tx_desc_get_buffer(s, desc), p,
                                      tx_desc_get_length(desc));
             p += tx_desc_get_length(desc);
             total_bytes += tx_desc_get_length(desc);
@@ -1124,7 +1156,8 @@ static void gem_transmit(CadenceGEMState *s)
                 if (tx_desc_get_wrap(desc)) {
                     s->tx_desc_addr[q] = s->regs[GEM_TXQBASE];
                 } else {
-                    s->tx_desc_addr[q] = packet_desc_addr + 8;
+                    s->tx_desc_addr[q] = packet_desc_addr +
+                                         4 * gem_get_desc_len(s, false);
                 }
                 DB_PRINT("TX descriptor next: 0x%08x\n", s->tx_desc_addr[q]);
 
@@ -1168,11 +1201,11 @@ static void gem_transmit(CadenceGEMState *s)
                 tx_desc_set_last(desc);
                 packet_desc_addr = s->regs[GEM_TXQBASE];
             } else {
-                packet_desc_addr += 8;
+                packet_desc_addr += 4 * gem_get_desc_len(s, false);
             }
             DB_PRINT("read descriptor 0x%" HWADDR_PRIx "\n", packet_desc_addr);
-            cpu_physical_memory_read(packet_desc_addr,
-                                     (uint8_t *)desc, sizeof(desc));
+            cpu_physical_memory_read(packet_desc_addr, (uint8_t *)desc,
+                                sizeof(uint32_t) * gem_get_desc_len(s, false));
         }
 
         if (tx_desc_get_used(desc)) {
diff --git a/include/hw/net/cadence_gem.h b/include/hw/net/cadence_gem.h
index b33ef65..00dbf4f 100644
--- a/include/hw/net/cadence_gem.h
+++ b/include/hw/net/cadence_gem.h
@@ -33,7 +33,7 @@
 #define CADENCE_GEM_MAXREG        (0x00000800 / 4) /* Last valid GEM address */
 
 /* Max number of words in a DMA descriptor.  */
-#define DESC_MAX_NUM_WORDS              2
+#define DESC_MAX_NUM_WORDS              6
 
 #define MAX_PRIORITY_QUEUES             8
 #define MAX_TYPE1_SCREENERS             16
-- 
2.7.4

[Qemu-devel] [PATCH v1 06/12] net: cadence_gem: Add support for selecting the DMA MemoryRegion

[Edgar E. Iglesias]

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

Add support for selecting the Memory Region that the GEM
will do DMA to.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 hw/net/cadence_gem.c         | 63 ++++++++++++++++++++++++++++----------------
 include/hw/net/cadence_gem.h |  2 ++
 2 files changed, 43 insertions(+), 22 deletions(-)

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 759c1d7..ab02515 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -28,6 +28,7 @@
 #include "hw/net/cadence_gem.h"
 #include "qapi/error.h"
 #include "qemu/log.h"
+#include "sysemu/dma.h"
 #include "net/checksum.h"
 
 #ifdef CADENCE_GEM_ERR_DEBUG
@@ -835,9 +836,9 @@ static void gem_get_rx_desc(CadenceGEMState *s, int q)
 {
     DB_PRINT("read descriptor 0x%x\n", (unsigned)s->rx_desc_addr[q]);
     /* read current descriptor */
-    cpu_physical_memory_read(s->rx_desc_addr[q],
-                             (uint8_t *)s->rx_desc[q],
-                             sizeof(uint32_t) * gem_get_desc_len(s, true));
+    address_space_read(s->dma_as, s->rx_desc_addr[q], MEMTXATTRS_UNSPECIFIED,
+                       (uint8_t *)s->rx_desc[q],
+                       sizeof(uint32_t) * gem_get_desc_len(s, true));
 
     /* Descriptor owned by software ? */
     if (rx_desc_get_ownership(s->rx_desc[q]) == 1) {
@@ -956,10 +957,10 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
                 rx_desc_get_buffer(s->rx_desc[q]));
 
         /* Copy packet data to emulated DMA buffer */
-        cpu_physical_memory_write(rx_desc_get_buffer(s, s->rx_desc[q]) +
+        address_space_write(s->dma_as, rx_desc_get_buffer(s, s->rx_desc[q]) +
                                                                   rxbuf_offset,
-                                  rxbuf_ptr,
-                                  MIN(bytes_to_copy, rxbufsize));
+                            MEMTXATTRS_UNSPECIFIED, rxbuf_ptr,
+                            MIN(bytes_to_copy, rxbufsize));
         rxbuf_ptr += MIN(bytes_to_copy, rxbufsize);
         bytes_to_copy -= MIN(bytes_to_copy, rxbufsize);
 
@@ -993,9 +994,10 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
         }
 
         /* Descriptor write-back.  */
-        cpu_physical_memory_write(s->rx_desc_addr[q],
-                                  (uint8_t *)s->rx_desc[q],
-                                  sizeof(uint32_t) * gem_get_desc_len(s, true));
+        address_space_write(s->dma_as, s->rx_desc_addr[q],
+                            MEMTXATTRS_UNSPECIFIED,
+                            (uint8_t *)s->rx_desc[q],
+                            sizeof(uint32_t) * gem_get_desc_len(s, true));
 
         /* Next descriptor */
         if (rx_desc_get_wrap(s->rx_desc[q])) {
@@ -1099,9 +1101,9 @@ static void gem_transmit(CadenceGEMState *s)
         packet_desc_addr = s->tx_desc_addr[q];
 
         DB_PRINT("read descriptor 0x%" HWADDR_PRIx "\n", packet_desc_addr);
-        cpu_physical_memory_read(packet_desc_addr,
-                                 (uint8_t *)desc,
-                                 sizeof(uint32_t) * gem_get_desc_len(s, false));
+        address_space_read(s->dma_as, packet_desc_addr,
+                           MEMTXATTRS_UNSPECIFIED, (uint8_t *)desc,
+                           sizeof(uint32_t) * gem_get_desc_len(s, false));
         /* Handle all descriptors owned by hardware */
         while (tx_desc_get_used(desc) == 0) {
 
@@ -1133,8 +1135,9 @@ static void gem_transmit(CadenceGEMState *s)
             /* Gather this fragment of the packet from "dma memory" to our
              * contig buffer.
              */
-            cpu_physical_memory_read(tx_desc_get_buffer(s, desc), p,
-                                     tx_desc_get_length(desc));
+            address_space_read(s->dma_as, tx_desc_get_buffer(s, desc),
+                               MEMTXATTRS_UNSPECIFIED,
+                               p, tx_desc_get_length(desc));
             p += tx_desc_get_length(desc);
             total_bytes += tx_desc_get_length(desc);
 
@@ -1145,13 +1148,15 @@ static void gem_transmit(CadenceGEMState *s)
                 /* Modify the 1st descriptor of this packet to be owned by
                  * the processor.
                  */
-                cpu_physical_memory_read(s->tx_desc_addr[q],
-                                         (uint8_t *)desc_first,
-                                         sizeof(desc_first));
+                address_space_read(s->dma_as, s->tx_desc_addr[q],
+                                   MEMTXATTRS_UNSPECIFIED,
+                                   (uint8_t *)desc_first,
+                                   sizeof(desc_first));
                 tx_desc_set_used(desc_first);
-                cpu_physical_memory_write(s->tx_desc_addr[q],
-                                          (uint8_t *)desc_first,
-                                          sizeof(desc_first));
+                address_space_write(s->dma_as, s->tx_desc_addr[q],
+                                  MEMTXATTRS_UNSPECIFIED,
+                                  (uint8_t *)desc_first,
+                                   sizeof(desc_first));
                 /* Advance the hardware current descriptor past this packet */
                 if (tx_desc_get_wrap(desc)) {
                     s->tx_desc_addr[q] = s->regs[GEM_TXQBASE];
@@ -1204,8 +1209,9 @@ static void gem_transmit(CadenceGEMState *s)
                 packet_desc_addr += 4 * gem_get_desc_len(s, false);
             }
             DB_PRINT("read descriptor 0x%" HWADDR_PRIx "\n", packet_desc_addr);
-            cpu_physical_memory_read(packet_desc_addr, (uint8_t *)desc,
-                                sizeof(uint32_t) * gem_get_desc_len(s, false));
+            address_space_read(s->dma_as, packet_desc_addr,
+                              MEMTXATTRS_UNSPECIFIED, (uint8_t *)desc,
+                              sizeof(uint32_t) * gem_get_desc_len(s, false));
         }
 
         if (tx_desc_get_used(desc)) {
@@ -1500,6 +1506,13 @@ static void gem_realize(DeviceState *dev, Error **errp)
     CadenceGEMState *s = CADENCE_GEM(dev);
     int i;
 
+    if (s->dma_mr) {
+        s->dma_as = g_malloc0(sizeof(AddressSpace));
+        address_space_init(s->dma_as, s->dma_mr, NULL);
+    } else {
+        s->dma_as = &address_space_memory;
+    }
+
     if (s->num_priority_queues == 0 ||
         s->num_priority_queues > MAX_PRIORITY_QUEUES) {
         error_setg(errp, "Invalid num-priority-queues value: %" PRIx8,
@@ -1537,6 +1550,12 @@ static void gem_init(Object *obj)
                           "enet", sizeof(s->regs));
 
     sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->iomem);
+
+    object_property_add_link(obj, "dma", TYPE_MEMORY_REGION,
+                             (Object **)&s->dma_mr,
+                             qdev_prop_allow_set_link_before_realize,
+                             OBJ_PROP_LINK_STRONG,
+                             &error_abort);
 }
 
 static const VMStateDescription vmstate_cadence_gem = {
diff --git a/include/hw/net/cadence_gem.h b/include/hw/net/cadence_gem.h
index 00dbf4f..c8f0751 100644
--- a/include/hw/net/cadence_gem.h
+++ b/include/hw/net/cadence_gem.h
@@ -45,6 +45,8 @@ typedef struct CadenceGEMState {
 
     /*< public >*/
     MemoryRegion iomem;
+    MemoryRegion *dma_mr;
+    AddressSpace *dma_as;
     NICState *nic;
     NICConf conf;
     qemu_irq irq[MAX_PRIORITY_QUEUES];
-- 
2.7.4

[Qemu-devel] [PATCH v1 07/12] net: cadence_gem: Implement support for 64bit descriptor addresses

[Edgar E. Iglesias]

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

Implement support for 64bit descriptor addresses.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 hw/net/cadence_gem.c | 47 +++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 39 insertions(+), 8 deletions(-)

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index ab02515..f93cd8e 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -153,6 +153,9 @@
 #define GEM_RECEIVE_Q1_PTR              (0x00000480 / 4)
 #define GEM_RECEIVE_Q7_PTR              (GEM_RECEIVE_Q1_PTR + 6)
 
+#define GEM_TBQPH                       (0x000004C8 / 4)
+#define GEM_RBQPH                       (0x000004D4 / 4)
+
 #define GEM_INT_Q1_ENABLE               (0x00000600 / 4)
 #define GEM_INT_Q7_ENABLE               (GEM_INT_Q1_ENABLE + 6)
 
@@ -832,18 +835,42 @@ static int get_queue_from_screen(CadenceGEMState *s, uint8_t *rxbuf_ptr,
     return 0;
 }
 
+static hwaddr gem_get_desc_addr(CadenceGEMState *s, bool tx, int q)
+{
+    hwaddr desc_addr = 0;
+
+    if (s->regs[GEM_DMACFG] & GEM_DMACFG_ADDR_64B) {
+        desc_addr = s->regs[tx ? GEM_TBQPH : GEM_RBQPH];
+    }
+    desc_addr <<= 32;
+    desc_addr |= tx ? s->tx_desc_addr[q] : s->rx_desc_addr[q];
+    return desc_addr;
+}
+
+static hwaddr gem_get_tx_desc_addr(CadenceGEMState *s, int q)
+{
+    return gem_get_desc_addr(s, true, q);
+}
+
+static hwaddr gem_get_rx_desc_addr(CadenceGEMState *s, int q)
+{
+    return gem_get_desc_addr(s, false, q);
+}
+
 static void gem_get_rx_desc(CadenceGEMState *s, int q)
 {
-    DB_PRINT("read descriptor 0x%x\n", (unsigned)s->rx_desc_addr[q]);
+    hwaddr desc_addr = gem_get_rx_desc_addr(s, q);
+
+    DB_PRINT("read descriptor 0x%" HWADDR_PRIx "\n", desc_addr);
+
     /* read current descriptor */
-    address_space_read(s->dma_as, s->rx_desc_addr[q], MEMTXATTRS_UNSPECIFIED,
+    address_space_read(s->dma_as, desc_addr, MEMTXATTRS_UNSPECIFIED,
                        (uint8_t *)s->rx_desc[q],
                        sizeof(uint32_t) * gem_get_desc_len(s, true));
 
     /* Descriptor owned by software ? */
     if (rx_desc_get_ownership(s->rx_desc[q]) == 1) {
-        DB_PRINT("descriptor 0x%x owned by sw.\n",
-                 (unsigned)s->rx_desc_addr[q]);
+        DB_PRINT("descriptor 0x%" HWADDR_PRIx " owned by sw.\n", desc_addr);
         s->regs[GEM_RXSTATUS] |= GEM_RXSTATUS_NOBUF;
         s->regs[GEM_ISR] |= GEM_INT_RXUSED & ~(s->regs[GEM_IMR]);
         /* Handle interrupt consequences */
@@ -947,6 +974,8 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
     q = get_queue_from_screen(s, rxbuf_ptr, rxbufsize);
 
     while (bytes_to_copy) {
+        hwaddr desc_addr;
+
         /* Do nothing if receive is not enabled. */
         if (!gem_can_receive(nc)) {
             assert(!first_desc);
@@ -994,7 +1023,8 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
         }
 
         /* Descriptor write-back.  */
-        address_space_write(s->dma_as, s->rx_desc_addr[q],
+        desc_addr = gem_get_rx_desc_addr(s, q);
+        address_space_write(s->dma_as, desc_addr,
                             MEMTXATTRS_UNSPECIFIED,
                             (uint8_t *)s->rx_desc[q],
                             sizeof(uint32_t) * gem_get_desc_len(s, true));
@@ -1098,7 +1128,7 @@ static void gem_transmit(CadenceGEMState *s)
 
     for (q = s->num_priority_queues - 1; q >= 0; q--) {
         /* read current descriptor */
-        packet_desc_addr = s->tx_desc_addr[q];
+        packet_desc_addr = gem_get_tx_desc_addr(s, q);
 
         DB_PRINT("read descriptor 0x%" HWADDR_PRIx "\n", packet_desc_addr);
         address_space_read(s->dma_as, packet_desc_addr,
@@ -1144,16 +1174,17 @@ static void gem_transmit(CadenceGEMState *s)
             /* Last descriptor for this packet; hand the whole thing off */
             if (tx_desc_get_last(desc)) {
                 uint32_t desc_first[DESC_MAX_NUM_WORDS];
+                hwaddr desc_addr = gem_get_tx_desc_addr(s, q);
 
                 /* Modify the 1st descriptor of this packet to be owned by
                  * the processor.
                  */
-                address_space_read(s->dma_as, s->tx_desc_addr[q],
+                address_space_read(s->dma_as, desc_addr,
                                    MEMTXATTRS_UNSPECIFIED,
                                    (uint8_t *)desc_first,
                                    sizeof(desc_first));
                 tx_desc_set_used(desc_first);
-                address_space_write(s->dma_as, s->tx_desc_addr[q],
+                address_space_write(s->dma_as, desc_addr,
                                   MEMTXATTRS_UNSPECIFIED,
                                   (uint8_t *)desc_first,
                                    sizeof(desc_first));
-- 
2.7.4

[Qemu-devel] [PATCH v1 08/12] net: cadence_gem: Announce 64bit addressing support

[Edgar E. Iglesias]

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

Announce 64bit addressing support.

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

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index f93cd8e..fc81fb5 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -142,6 +142,7 @@
 #define GEM_DESCONF4      (0x0000028C/4)
 #define GEM_DESCONF5      (0x00000290/4)
 #define GEM_DESCONF6      (0x00000294/4)
+#define GEM_DESCONF6_64B_MASK (1U << 23)
 #define GEM_DESCONF7      (0x00000298/4)
 
 #define GEM_INT_Q1_STATUS               (0x00000400 / 4)
@@ -1300,7 +1301,7 @@ static void gem_reset(DeviceState *d)
     s->regs[GEM_DESCONF] = 0x02500111;
     s->regs[GEM_DESCONF2] = 0x2ab13fff;
     s->regs[GEM_DESCONF5] = 0x002f2045;
-    s->regs[GEM_DESCONF6] = 0x0;
+    s->regs[GEM_DESCONF6] = GEM_DESCONF6_64B_MASK;
 
     queues_mask = MAKE_64BIT_MASK(1, s->num_priority_queues - 1);
     s->regs[GEM_DESCONF6] |= queues_mask;
-- 
2.7.4

[Qemu-devel] [PATCH v1 09/12] target-arm: powerctl: Enable HVC when starting CPUs to EL2

[Edgar E. Iglesias]

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

When QEMU provides the equivalent of the EL3 firmware, we
need to enable HVCs in scr_el3 when turning on CPUs that
target EL2.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target/arm/arm-powerctl.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/target/arm/arm-powerctl.c b/target/arm/arm-powerctl.c
index ce55eeb..54f2974 100644
--- a/target/arm/arm-powerctl.c
+++ b/target/arm/arm-powerctl.c
@@ -63,6 +63,7 @@ static void arm_set_cpu_on_async_work(CPUState *target_cpu_state,
     struct CpuOnInfo *info = (struct CpuOnInfo *) data.host_ptr;
 
     /* Initialize the cpu we are turning on */
+    qemu_log("CPU%d reset\n", target_cpu_state->cpu_index);
     cpu_reset(target_cpu_state);
     target_cpu_state->halted = 0;
 
@@ -103,6 +104,16 @@ static void arm_set_cpu_on_async_work(CPUState *target_cpu_state,
     } else {
         /* Processor is not in secure mode */
         target_cpu->env.cp15.scr_el3 |= SCR_NS;
+
+        /*
+         * If QEMU is providing the equivalent of EL3 firmware, then we need
+         * to make sure a CPU targeting EL2 comes out of reset with a
+         * functional HVC insn.
+         */
+        if (arm_feature(&target_cpu->env, ARM_FEATURE_EL3)
+            && info->target_el == 2) {
+            target_cpu->env.cp15.scr_el3 |= SCR_HCE;
+        }
     }
 
     /* We check if the started CPU is now at the correct level */
-- 
2.7.4

[Qemu-devel] [PATCH v1 10/12] target/arm: Add the Cortex-A72

[Edgar E. Iglesias]

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

Add the ARM Cortex-A72.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target/arm/cpu64.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 800bff7..02e500b 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -218,6 +218,64 @@ static void aarch64_a53_initfn(Object *obj)
     define_arm_cp_regs(cpu, cortex_a57_a53_cp_reginfo);
 }
 
+static void aarch64_a72_initfn(Object *obj)
+{
+    ARMCPU *cpu = ARM_CPU(obj);
+
+    cpu->dtb_compatible = "arm,cortex-a72";
+    set_feature(&cpu->env, ARM_FEATURE_V8);
+    set_feature(&cpu->env, ARM_FEATURE_VFP4);
+    set_feature(&cpu->env, ARM_FEATURE_NEON);
+    set_feature(&cpu->env, ARM_FEATURE_GENERIC_TIMER);
+    set_feature(&cpu->env, ARM_FEATURE_AARCH64);
+    set_feature(&cpu->env, ARM_FEATURE_CBAR_RO);
+    set_feature(&cpu->env, ARM_FEATURE_V8_AES);
+    set_feature(&cpu->env, ARM_FEATURE_V8_SHA1);
+    set_feature(&cpu->env, ARM_FEATURE_V8_SHA256);
+    set_feature(&cpu->env, ARM_FEATURE_V8_PMULL);
+    set_feature(&cpu->env, ARM_FEATURE_CRC);
+    set_feature(&cpu->env, ARM_FEATURE_EL2);
+    set_feature(&cpu->env, ARM_FEATURE_EL3);
+    cpu->midr = 0x410fd083;
+    cpu->revidr = 0x00000000;
+    cpu->reset_fpsid = 0x41034080;
+    cpu->mvfr0 = 0x10110222;
+    cpu->mvfr1 = 0x12111111;
+    cpu->mvfr2 = 0x00000043;
+    cpu->ctr = 0x8444c004;
+    cpu->reset_sctlr = 0x00c50838;
+    cpu->id_pfr0 = 0x00000131;
+    cpu->id_pfr1 = 0x00011011;
+    cpu->id_dfr0 = 0x03010066;
+    cpu->id_afr0 = 0x00000000;
+    cpu->id_mmfr0 = 0x10201105;
+    cpu->id_mmfr1 = 0x40000000;
+    cpu->id_mmfr2 = 0x01260000;
+    cpu->id_mmfr3 = 0x02102211;
+    cpu->id_isar0 = 0x02101110;
+    cpu->id_isar1 = 0x13112111;
+    cpu->id_isar2 = 0x21232042;
+    cpu->id_isar3 = 0x01112131;
+    cpu->id_isar4 = 0x00011142;
+    cpu->id_isar5 = 0x00011121;
+    cpu->id_aa64pfr0 = 0x00002222;
+    cpu->id_aa64dfr0 = 0x10305106;
+    cpu->pmceid0 = 0x00000000;
+    cpu->pmceid1 = 0x00000000;
+    cpu->id_aa64isar0 = 0x00011120;
+    cpu->id_aa64mmfr0 = 0x00001124;
+    cpu->dbgdidr = 0x3516d000;
+    cpu->clidr = 0x0a200023;
+    cpu->ccsidr[0] = 0x701fe00a; /* 32KB L1 dcache */
+    cpu->ccsidr[1] = 0x201fe012; /* 48KB L1 icache */
+    cpu->ccsidr[2] = 0x707fe07a; /* 1MB L2 cache */
+    cpu->dcz_blocksize = 4; /* 64 bytes */
+    cpu->gic_num_lrs = 4;
+    cpu->gic_vpribits = 5;
+    cpu->gic_vprebits = 5;
+    define_arm_cp_regs(cpu, cortex_a57_a53_cp_reginfo);
+}
+
 static void cpu_max_get_sve_vq(Object *obj, Visitor *v, const char *name,
                                void *opaque, Error **errp)
 {
@@ -293,6 +351,7 @@ typedef struct ARMCPUInfo {
 static const ARMCPUInfo aarch64_cpus[] = {
     { .name = "cortex-a57",         .initfn = aarch64_a57_initfn },
     { .name = "cortex-a53",         .initfn = aarch64_a53_initfn },
+    { .name = "cortex-a72",         .initfn = aarch64_a72_initfn },
     { .name = "max",                .initfn = aarch64_max_initfn },
     { .name = NULL }
 };
-- 
2.7.4

[Qemu-devel] [PATCH v1 11/12] hw/arm: versal: Add a model of Xilinx Versal SoC

[Edgar E. Iglesias]

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

Add a model of Xilinx Versal SoC.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 default-configs/aarch64-softmmu.mak |   1 +
 hw/arm/Makefile.objs                |   1 +
 hw/arm/xlnx-versal.c                | 339 ++++++++++++++++++++++++++++++++++++
 include/hw/arm/xlnx-versal.h        | 122 +++++++++++++
 4 files changed, 463 insertions(+)
 create mode 100644 hw/arm/xlnx-versal.c
 create mode 100644 include/hw/arm/xlnx-versal.h

diff --git a/default-configs/aarch64-softmmu.mak b/default-configs/aarch64-softmmu.mak
index 6f790f0..4ea9add 100644
--- a/default-configs/aarch64-softmmu.mak
+++ b/default-configs/aarch64-softmmu.mak
@@ -8,4 +8,5 @@ CONFIG_DDC=y
 CONFIG_DPCD=y
 CONFIG_XLNX_ZYNQMP=y
 CONFIG_XLNX_ZYNQMP_ARM=y
+CONFIG_XLNX_VERSAL=y
 CONFIG_ARM_SMMUV3=y
diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index 5f88062..ec21d9b 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -26,6 +26,7 @@ obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10.o cubieboard.o
 obj-$(CONFIG_RASPI) += bcm2835_peripherals.o bcm2836.o raspi.o
 obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o
 obj-$(CONFIG_XLNX_ZYNQMP_ARM) += xlnx-zynqmp.o xlnx-zcu102.o
+obj-$(CONFIG_XLNX_VERSAL) += xlnx-versal.o
 obj-$(CONFIG_FSL_IMX25) += fsl-imx25.o imx25_pdk.o
 obj-$(CONFIG_FSL_IMX31) += fsl-imx31.o kzm.o
 obj-$(CONFIG_FSL_IMX6) += fsl-imx6.o sabrelite.o
diff --git a/hw/arm/xlnx-versal.c b/hw/arm/xlnx-versal.c
new file mode 100644
index 0000000..c12fc85
--- /dev/null
+++ b/hw/arm/xlnx-versal.c
@@ -0,0 +1,339 @@
+/*
+ * Xilinx Versal SoC model.
+ *
+ * Copyright (c) 2018 Xilinx Inc.
+ * Written by Edgar E. Iglesias
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 or
+ * (at your option) any later version.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qemu-common.h"
+#include "qemu/log.h"
+#include "hw/sysbus.h"
+#include "net/net.h"
+#include "sysemu/sysemu.h"
+#include "sysemu/kvm.h"
+#include "hw/arm/arm.h"
+#include "kvm_arm.h"
+#include "hw/misc/unimp.h"
+#include "hw/intc/arm_gicv3_common.h"
+#include "hw/arm/xlnx-versal.h"
+
+#define XLNX_VERSAL_ACPU_TYPE "cortex-a72" "-" TYPE_ARM_CPU
+#define GEM_REVISION        0x40070106
+
+static void versal_create_apu_cpus(Versal *s, Error **errp)
+{
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(s->fpd.apu.cpu); i++) {
+        Object *obj;
+        char *name;
+
+        obj = object_new(XLNX_VERSAL_ACPU_TYPE);
+        if (!obj) {
+            /* Secondary CPUs start in PSCI powered-down state */
+            error_setg(errp, "Unable to create apu.cpu[%d] of type %s",
+                       i, XLNX_VERSAL_ACPU_TYPE);
+            return;
+        }
+
+        name = g_strdup_printf("apu-cpu[%d]", i);
+        object_property_add_child(OBJECT(s), name, obj, &error_fatal);
+        g_free(name);
+
+        object_property_set_int(obj, s->cfg.psci_conduit,
+                                "psci-conduit", &error_abort);
+        if (i) {
+            object_property_set_bool(obj, true,
+                                     "start-powered-off", &error_abort);
+        }
+
+        object_property_set_int(obj, ARRAY_SIZE(s->fpd.apu.cpu),
+                                "core-count", &error_abort);
+        object_property_set_link(obj, OBJECT(&s->fpd.apu.mr), "memory",
+                                 &error_abort);
+        object_property_set_bool(obj, true, "realized", &error_fatal);
+        s->fpd.apu.cpu[i] = ARM_CPU(obj);
+    }
+}
+
+static void versal_create_apu_gic(Versal *s, qemu_irq *pic, Error **errp)
+{
+    static const uint64_t addrs[] = {
+        MM_GIC_APU_DIST_MAIN,
+        MM_GIC_APU_REDIST_0
+    };
+    SysBusDevice *gicbusdev;
+    DeviceState *gicdev;
+    int nr_apu_cpus = ARRAY_SIZE(s->fpd.apu.cpu);
+    int i;
+
+    sysbus_init_child_obj(OBJECT(s), "apu-gic",
+                          &s->fpd.apu.gic, sizeof(s->fpd.apu.gic),
+                          gicv3_class_name());
+    gicbusdev = SYS_BUS_DEVICE(&s->fpd.apu.gic);
+    gicdev = DEVICE(&s->fpd.apu.gic);
+    qdev_prop_set_uint32(gicdev, "revision", 3);
+    qdev_prop_set_uint32(gicdev, "num-cpu", 2);
+    qdev_prop_set_uint32(gicdev, "num-irq", XLNX_VERSAL_NR_IRQS + 32);
+    qdev_prop_set_uint32(gicdev, "len-redist-region-count", 1);
+    qdev_prop_set_uint32(gicdev, "redist-region-count[0]", 2);
+    if (!kvm_irqchip_in_kernel()) {
+        qdev_prop_set_bit(gicdev, "has-security-extensions", true);
+    }
+
+    object_property_set_bool(OBJECT(&s->fpd.apu.gic), true, "realized", errp);
+
+    for (i = 0; i < ARRAY_SIZE(addrs); i++) {
+        MemoryRegion *mr;
+
+        mr = sysbus_mmio_get_region(gicbusdev, i);
+        memory_region_add_subregion(&s->fpd.apu.mr, addrs[i], mr);
+    }
+
+    for (i = 0; i < nr_apu_cpus; i++) {
+        DeviceState *cpudev = DEVICE(s->fpd.apu.cpu[i]);
+        int ppibase = XLNX_VERSAL_NR_IRQS + i * GIC_INTERNAL + GIC_NR_SGIS;
+        qemu_irq maint_irq;
+        int ti;
+        /* Mapping from the output timer irq lines from the CPU to the
+         * GIC PPI inputs we use for the virt board.
+         */
+        const int timer_irq[] = {
+            [GTIMER_PHYS] = VERSAL_TIMER_NS_EL1_IRQ,
+            [GTIMER_VIRT] = VERSAL_TIMER_VIRT_IRQ,
+            [GTIMER_HYP]  = VERSAL_TIMER_NS_EL2_IRQ,
+            [GTIMER_SEC]  = VERSAL_TIMER_S_EL1_IRQ,
+        };
+
+        for (ti = 0; ti < ARRAY_SIZE(timer_irq); ti++) {
+            qdev_connect_gpio_out(cpudev, ti,
+                                  qdev_get_gpio_in(gicdev,
+                                                   ppibase + timer_irq[ti]));
+        }
+        maint_irq = qdev_get_gpio_in(gicdev,
+                                        ppibase + VERSAL_GIC_MAINT_IRQ);
+        qdev_connect_gpio_out_named(cpudev, "gicv3-maintenance-interrupt",
+                                    0, maint_irq);
+        sysbus_connect_irq(gicbusdev, i, qdev_get_gpio_in(cpudev, ARM_CPU_IRQ));
+        sysbus_connect_irq(gicbusdev, i + nr_apu_cpus,
+                           qdev_get_gpio_in(cpudev, ARM_CPU_FIQ));
+        sysbus_connect_irq(gicbusdev, i + 2 * nr_apu_cpus,
+                           qdev_get_gpio_in(cpudev, ARM_CPU_VIRQ));
+        sysbus_connect_irq(gicbusdev, i + 3 * nr_apu_cpus,
+                           qdev_get_gpio_in(cpudev, ARM_CPU_VFIQ));
+    }
+
+    for (i = 0; i < XLNX_VERSAL_NR_IRQS; i++) {
+        pic[i] = qdev_get_gpio_in(gicdev, i);
+    }
+}
+
+static void versal_create_uarts(Versal *s, qemu_irq *pic)
+{
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(s->lpd.iou.uart); i++) {
+        static const int irqs[] = { VERSAL_UART0_IRQ_0, VERSAL_UART1_IRQ_0};
+        static const uint64_t addrs[] = { MM_UART0, MM_UART1 };
+        char *name = g_strdup_printf("uart%d", i);
+        DeviceState *dev;
+        MemoryRegion *mr;
+
+        dev = qdev_create(NULL, "pl011");
+        s->lpd.iou.uart[i] = SYS_BUS_DEVICE(dev);
+        qdev_prop_set_chr(dev, "chardev", serial_hd(i));
+        object_property_add_child(OBJECT(s), name, OBJECT(dev), &error_fatal);
+        qdev_init_nofail(dev);
+
+        mr = sysbus_mmio_get_region(s->lpd.iou.uart[i], 0);
+        memory_region_add_subregion(&s->mr_ps, addrs[i], mr);
+
+        sysbus_connect_irq(s->lpd.iou.uart[i], 0, pic[irqs[i]]);
+        g_free(name);
+    }
+}
+
+static void versal_create_gems(Versal *s, qemu_irq *pic)
+{
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(s->lpd.iou.gem); i++) {
+        static const int irqs[] = { VERSAL_GEM0_IRQ_0, VERSAL_GEM1_IRQ_0};
+        static const uint64_t addrs[] = { MM_GEM0, MM_GEM1 };
+        char *name = g_strdup_printf("gem%d", i);
+        NICInfo *nd = &nd_table[i];
+        DeviceState *dev;
+        MemoryRegion *mr;
+
+        dev = qdev_create(NULL, "cadence_gem");
+        s->lpd.iou.gem[i] = SYS_BUS_DEVICE(dev);
+        object_property_add_child(OBJECT(s), name, OBJECT(dev), &error_fatal);
+        if (nd->used) {
+            qemu_check_nic_model(nd, "cadence_gem");
+            qdev_set_nic_properties(dev, nd);
+        }
+        object_property_set_int(OBJECT(s->lpd.iou.gem[i]),
+                                2, "num-priority-queues",
+                                &error_abort);
+        object_property_set_link(OBJECT(s->lpd.iou.gem[i]),
+                                 OBJECT(&s->mr_ps), "dma",
+                                 &error_abort);
+        qdev_init_nofail(dev);
+
+        mr = sysbus_mmio_get_region(s->lpd.iou.gem[i], 0);
+        memory_region_add_subregion(&s->mr_ps, addrs[i], mr);
+
+        sysbus_connect_irq(s->lpd.iou.gem[i], 0, pic[irqs[i]]);
+        g_free(name);
+    }
+}
+
+/* This takes the board allocated linear DDR memory and creates aliases
+ * for each split DDR range/apperture on the Versal address map.
+ */
+static void versal_map_ddr(Versal *s)
+{
+    uint64_t size = memory_region_size(s->cfg.mr_ddr);
+    /* Describes the various split DDR access regions.  */
+    static const struct {
+        uint64_t base;
+        uint64_t size;
+    } addr_ranges[] = {
+        { MM_TOP_DDR, MM_TOP_DDR_SIZE },
+        { MM_TOP_DDR_2, MM_TOP_DDR_2_SIZE },
+        { MM_TOP_DDR_3, MM_TOP_DDR_3_SIZE },
+        { MM_TOP_DDR_4, MM_TOP_DDR_4_SIZE }
+    };
+    uint64_t offset = 0;
+    int i;
+
+    assert(ARRAY_SIZE(addr_ranges) == ARRAY_SIZE(s->noc.mr_ddr_ranges));
+    for (i = 0; i < ARRAY_SIZE(addr_ranges) && size; i++) {
+        char *name;
+        uint64_t mapsize;
+
+        mapsize = size < addr_ranges[i].size ? size : addr_ranges[i].size;
+        name = g_strdup_printf("noc-ddr-range%d", i);
+        /* Create the MR alias.  */
+        memory_region_init_alias(&s->noc.mr_ddr_ranges[i], OBJECT(s),
+                                 name, s->cfg.mr_ddr,
+                                 offset, mapsize);
+
+        /* Map it onto the NoC MR.  */
+        memory_region_add_subregion(&s->mr_ps, addr_ranges[i].base,
+                                    &s->noc.mr_ddr_ranges[i]);
+        offset += mapsize;
+        size -= mapsize;
+        g_free(name);
+    }
+}
+
+static void versal_unimp_area(Versal *s, const char *name,
+                                MemoryRegion *mr,
+                                hwaddr base, hwaddr size)
+{
+    DeviceState *dev = qdev_create(NULL, TYPE_UNIMPLEMENTED_DEVICE);
+    MemoryRegion *mr_dev;
+
+    qdev_prop_set_string(dev, "name", name);
+    qdev_prop_set_uint64(dev, "size", size);
+    object_property_add_child(OBJECT(s), name, OBJECT(dev), &error_fatal);
+    qdev_init_nofail(dev);
+
+    mr_dev = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0);
+    memory_region_add_subregion(mr, base, mr_dev);
+}
+
+static void versal_unimp(Versal *s)
+{
+    versal_unimp_area(s, "psm", &s->mr_ps,
+                        MM_PSM_START, MM_PSM_END - MM_PSM_START);
+    versal_unimp_area(s, "crl", &s->mr_ps,
+                        MM_CRL, MM_CRL_SIZE);
+    versal_unimp_area(s, "crf", &s->mr_ps,
+                        MM_FPD_CRF, MM_FPD_CRF_SIZE);
+    versal_unimp_area(s, "iou-scntr", &s->mr_ps,
+                        MM_IOU_SCNTR, MM_IOU_SCNTR_SIZE);
+    versal_unimp_area(s, "iou-scntr-seucre", &s->mr_ps,
+                        MM_IOU_SCNTRS, MM_IOU_SCNTRS_SIZE);
+}
+
+static void versal_realize(DeviceState *dev, Error **errp)
+{
+    Versal *s = XLNX_VERSAL(dev);
+    qemu_irq pic[XLNX_VERSAL_NR_IRQS];
+
+    versal_create_apu_cpus(s, errp);
+    versal_create_apu_gic(s, pic, errp);
+    versal_create_uarts(s, pic);
+    versal_create_gems(s, pic);
+    versal_map_ddr(s);
+    versal_unimp(s);
+
+    /* Create the OCM.  */
+    memory_region_init_ram(&s->lpd.mr_ocm, OBJECT(s), "ocm",
+                           MM_OCM_SIZE, &error_fatal);
+
+    memory_region_add_subregion_overlap(&s->mr_ps, MM_OCM, &s->lpd.mr_ocm, 0);
+    memory_region_add_subregion_overlap(&s->fpd.apu.mr, 0, &s->mr_ps, 0);
+}
+
+static void versal_init(Object *obj)
+{
+    Versal *s = XLNX_VERSAL(obj);
+
+    memory_region_init(&s->fpd.apu.mr, obj, "mr-apu", UINT64_MAX);
+    memory_region_init(&s->mr_ps, obj, "mr-ps-switch", UINT64_MAX);
+}
+
+static const VMStateDescription versal_vmstate = {
+    .name = "xlnx-ve",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        /* FIXME.  */
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static Property versal_properties[] = {
+    DEFINE_PROP_LINK("ddr", Versal, cfg.mr_ddr, TYPE_MEMORY_REGION,
+                     MemoryRegion *),
+    DEFINE_PROP_UINT32("psci-conduit", Versal, cfg.psci_conduit, 0),
+    DEFINE_PROP_END_OF_LIST()
+};
+
+static void versal_reset(DeviceState *dev)
+{
+}
+
+static void versal_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = versal_realize;
+    dc->vmsd = &versal_vmstate;
+    dc->props = versal_properties;
+    dc->reset = versal_reset;
+}
+
+static const TypeInfo versal_info = {
+    .name = TYPE_XLNX_VERSAL,
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(Versal),
+    .instance_init = versal_init,
+    .class_init = versal_class_init,
+};
+
+static void versal_register_types(void)
+{
+    type_register_static(&versal_info);
+}
+
+type_init(versal_register_types);
diff --git a/include/hw/arm/xlnx-versal.h b/include/hw/arm/xlnx-versal.h
new file mode 100644
index 0000000..9da621e
--- /dev/null
+++ b/include/hw/arm/xlnx-versal.h
@@ -0,0 +1,122 @@
+/*
+ * Model of the Xilinx Versal
+ *
+ * Copyright (c) 2018 Xilinx Inc.
+ * Written by Edgar E. Iglesias
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 or
+ * (at your option) any later version.
+ */
+
+#ifndef XLNX_VERSAL_H
+#define XLNX_VERSAL_H
+
+#include "hw/sysbus.h"
+#include "hw/arm/arm.h"
+#include "hw/intc/arm_gicv3.h"
+
+#define TYPE_XLNX_VERSAL "xlnx-versal"
+#define XLNX_VERSAL(obj) OBJECT_CHECK(Versal, (obj), TYPE_XLNX_VERSAL)
+
+#define XLNX_VERSAL_NR_ACPUS   2
+#define XLNX_VERSAL_NR_UARTS   2
+#define XLNX_VERSAL_NR_GEMS    2
+#define XLNX_VERSAL_NR_IRQS    256
+
+typedef struct Versal {
+    /*< private >*/
+    SysBusDevice parent_obj;
+
+    /*< public >*/
+    struct {
+        struct {
+            MemoryRegion mr;
+            ARMCPU *cpu[XLNX_VERSAL_NR_ACPUS];
+            GICv3State gic;
+        } apu;
+    } fpd;
+
+    MemoryRegion mr_ps;
+
+    struct {
+        /* 4 ranges to access DDR.  */
+        MemoryRegion mr_ddr_ranges[4];
+    } noc;
+
+    struct {
+        MemoryRegion mr_ocm;
+
+        struct {
+            SysBusDevice *uart[XLNX_VERSAL_NR_UARTS];
+            SysBusDevice *gem[XLNX_VERSAL_NR_GEMS];
+        } iou;
+    } lpd;
+
+    struct {
+        MemoryRegion *mr_ddr;
+        uint32_t psci_conduit;
+    } cfg;
+} Versal;
+
+/* Memory-map and IRQ definitions. Copied a subset from
+ * auto-generated files.  */
+
+#define VERSAL_GIC_MAINT_IRQ        9
+#define VERSAL_TIMER_VIRT_IRQ       11
+#define VERSAL_TIMER_S_EL1_IRQ      13
+#define VERSAL_TIMER_NS_EL1_IRQ     14
+#define VERSAL_TIMER_NS_EL2_IRQ     10
+
+#define VERSAL_UART0_IRQ_0         18
+#define VERSAL_UART1_IRQ_0         19
+#define VERSAL_GEM0_IRQ_0          56
+#define VERSAL_GEM0_WAKE_IRQ_0     57
+#define VERSAL_GEM1_IRQ_0          58
+#define VERSAL_GEM1_WAKE_IRQ_0     59
+
+/* Architecturally eserved IRQs suitable for virtualization.  */
+#define VERSAL_RSVD_HIGH_IRQ_FIRST 160
+#define VERSAL_RSVD_HIGH_IRQ_LAST  255
+
+#define MM_TOP_RSVD                 0xa0000000U
+#define MM_TOP_RSVD_SIZE            0x4000000
+#define MM_GIC_APU_DIST_MAIN        0xf9000000U
+#define MM_GIC_APU_DIST_MAIN_SIZE   0x10000
+#define MM_GIC_APU_REDIST_0         0xf9080000U
+#define MM_GIC_APU_REDIST_0_SIZE    0x80000
+
+#define MM_UART0                    0xff000000U
+#define MM_UART0_SIZE               0x10000
+#define MM_UART1                    0xff010000U
+#define MM_UART1_SIZE               0x10000
+
+#define MM_GEM0                     0xff0c0000U
+#define MM_GEM0_SIZE                0x10000
+#define MM_GEM1                     0xff0d0000U
+#define MM_GEM1_SIZE                0x10000
+
+#define MM_OCM                      0xfffc0000U
+#define MM_OCM_SIZE                 0x40000
+
+#define MM_TOP_DDR                  0x0
+#define MM_TOP_DDR_SIZE             0x80000000U
+#define MM_TOP_DDR_2                0x800000000ULL
+#define MM_TOP_DDR_2_SIZE           0x800000000ULL
+#define MM_TOP_DDR_3                0xc000000000ULL
+#define MM_TOP_DDR_3_SIZE           0x4000000000ULL
+#define MM_TOP_DDR_4                0x10000000000ULL
+#define MM_TOP_DDR_4_SIZE           0xb780000000ULL
+
+#define MM_PSM_START                0xffc80000U
+#define MM_PSM_END                  0xffcf0000U
+
+#define MM_CRL                      0xff5e0000U
+#define MM_CRL_SIZE                 0x300000
+#define MM_IOU_SCNTR                0xff130000U
+#define MM_IOU_SCNTR_SIZE           0x10000
+#define MM_IOU_SCNTRS               0xff140000U
+#define MM_IOU_SCNTRS_SIZE          0x10000
+#define MM_FPD_CRF                  0xfd1a0000U
+#define MM_FPD_CRF_SIZE             0x140000
+#endif
-- 
2.7.4

[Qemu-devel] [PATCH v1 12/12] hw/arm: versal: Add a virtual Xilinx Versal board

[Edgar E. Iglesias]

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

Add a virtual Xilinx Versal board.

This board is based on the Xilinx Versal SoC. The exact
details of what peripherals are attached to this board
will remain in control of QEMU. QEMU will generate an
FDT on the fly for Linux and other software to auto-discover
peripherals.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 hw/arm/Makefile.objs      |   2 +-
 hw/arm/xlnx-versal-virt.c | 494 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 495 insertions(+), 1 deletion(-)
 create mode 100644 hw/arm/xlnx-versal-virt.c

diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index ec21d9b..50c7b4a 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -26,7 +26,7 @@ obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10.o cubieboard.o
 obj-$(CONFIG_RASPI) += bcm2835_peripherals.o bcm2836.o raspi.o
 obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o
 obj-$(CONFIG_XLNX_ZYNQMP_ARM) += xlnx-zynqmp.o xlnx-zcu102.o
-obj-$(CONFIG_XLNX_VERSAL) += xlnx-versal.o
+obj-$(CONFIG_XLNX_VERSAL) += xlnx-versal.o xlnx-versal-virt.o
 obj-$(CONFIG_FSL_IMX25) += fsl-imx25.o imx25_pdk.o
 obj-$(CONFIG_FSL_IMX31) += fsl-imx31.o kzm.o
 obj-$(CONFIG_FSL_IMX6) += fsl-imx6.o sabrelite.o
diff --git a/hw/arm/xlnx-versal-virt.c b/hw/arm/xlnx-versal-virt.c
new file mode 100644
index 0000000..1ae125b
--- /dev/null
+++ b/hw/arm/xlnx-versal-virt.c
@@ -0,0 +1,494 @@
+/*
+ * Xilinx Versal Virtual board.
+ *
+ * Copyright (c) 2018 Xilinx Inc.
+ * Written by Edgar E. Iglesias
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 or
+ * (at your option) any later version.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "qemu/error-report.h"
+#include "qapi/error.h"
+#include "sysemu/device_tree.h"
+#include "exec/address-spaces.h"
+#include "hw/boards.h"
+#include "hw/sysbus.h"
+#include "hw/arm/sysbus-fdt.h"
+#include "hw/arm/fdt.h"
+#include "cpu.h"
+#include "hw/arm/xlnx-versal.h"
+
+#define TYPE_XLNX_VERSAL_VIRT_MACHINE MACHINE_TYPE_NAME("xlnx-versal-virt")
+#define XLNX_VERSAL_VIRT_MACHINE(obj) \
+    OBJECT_CHECK(VersalVirt, (obj), TYPE_XLNX_VERSAL_VIRT_MACHINE)
+
+typedef struct VersalVirt {
+    MachineState parent_obj;
+
+    Versal soc;
+    MemoryRegion mr_ddr;
+
+    void *fdt;
+    int fdt_size;
+    struct {
+        uint32_t gic;
+        uint32_t ethernet_phy[2];
+        uint32_t clk_125Mhz;
+        uint32_t clk_25Mhz;
+    } phandle;
+    struct arm_boot_info binfo;
+
+    struct {
+        bool secure;
+    } cfg;
+} VersalVirt;
+
+static void fdt_create(VersalVirt *s)
+{
+    MachineClass *mc = MACHINE_GET_CLASS(s);
+    int i;
+
+    s->fdt = create_device_tree(&s->fdt_size);
+    if (!s->fdt) {
+        error_report("create_device_tree() failed");
+        exit(1);
+    }
+
+    /* Allocate all phandles.  */
+    s->phandle.gic = qemu_fdt_alloc_phandle(s->fdt);
+    for (i = 0; i < ARRAY_SIZE(s->phandle.ethernet_phy); i++) {
+        s->phandle.ethernet_phy[i] = qemu_fdt_alloc_phandle(s->fdt);
+    }
+    s->phandle.clk_25Mhz = qemu_fdt_alloc_phandle(s->fdt);
+    s->phandle.clk_125Mhz = qemu_fdt_alloc_phandle(s->fdt);
+
+    /* Create /chosen node for load_dtb.  */
+    qemu_fdt_add_subnode(s->fdt, "/chosen");
+
+    /* Header */
+    qemu_fdt_setprop_cell(s->fdt, "/", "interrupt-parent", s->phandle.gic);
+    qemu_fdt_setprop_cell(s->fdt, "/", "#size-cells", 0x2);
+    qemu_fdt_setprop_cell(s->fdt, "/", "#address-cells", 0x2);
+    qemu_fdt_setprop_string(s->fdt, "/", "model", mc->desc);
+    qemu_fdt_setprop_string(s->fdt, "/", "compatible", "xlnx-versal-virt");
+}
+
+static void fdt_add_clk_node(VersalVirt *s, const char *name,
+                             unsigned int freq_hz, uint32_t phandle)
+{
+    qemu_fdt_add_subnode(s->fdt, name);
+    qemu_fdt_setprop_cell(s->fdt, name, "phandle", phandle);
+    qemu_fdt_setprop_cell(s->fdt, name, "clock-frequency", freq_hz);
+    qemu_fdt_setprop_cell(s->fdt, name, "#clock-cells", 0x0);
+    qemu_fdt_setprop_string(s->fdt, name, "compatible", "fixed-clock");
+    qemu_fdt_setprop(s->fdt, name, "u-boot,dm-pre-reloc", NULL, 0);
+}
+
+static void fdt_add_cpu_nodes(VersalVirt *s, uint32_t psci_conduit)
+{
+    int i;
+
+    qemu_fdt_add_subnode(s->fdt, "/cpus");
+    qemu_fdt_setprop_cell(s->fdt, "/cpus", "#size-cells", 0x0);
+    qemu_fdt_setprop_cell(s->fdt, "/cpus", "#address-cells", 1);
+
+    for (i = XLNX_VERSAL_NR_ACPUS - 1; i >= 0; i--) {
+        char *name = g_strdup_printf("/cpus/cpu@%d", i);
+        ARMCPU *armcpu = ARM_CPU(qemu_get_cpu(i));
+
+        qemu_fdt_add_subnode(s->fdt, name);
+        qemu_fdt_setprop_cell(s->fdt, name, "reg", armcpu->mp_affinity);
+        if (psci_conduit != QEMU_PSCI_CONDUIT_DISABLED) {
+            qemu_fdt_setprop_string(s->fdt, name, "enable-method", "psci");
+        }
+        qemu_fdt_setprop_string(s->fdt, name, "device_type", "cpu");
+        qemu_fdt_setprop_string(s->fdt, name, "compatible",
+                                armcpu->dtb_compatible);
+        g_free(name);
+    }
+}
+
+static void fdt_add_gic_nodes(VersalVirt *s)
+{
+    char *nodename;
+
+    nodename = g_strdup_printf("/gic@%x", MM_GIC_APU_DIST_MAIN);
+    qemu_fdt_add_subnode(s->fdt, nodename);
+    qemu_fdt_setprop_cell(s->fdt, nodename, "phandle", s->phandle.gic);
+    qemu_fdt_setprop_cells(s->fdt, nodename, "interrupts",
+                           GIC_FDT_IRQ_TYPE_PPI, VERSAL_GIC_MAINT_IRQ,
+                           GIC_FDT_IRQ_FLAGS_LEVEL_HI);
+    qemu_fdt_setprop(s->fdt, nodename, "interrupt-controller", NULL, 0);
+    qemu_fdt_setprop_sized_cells(s->fdt, nodename, "reg",
+                                 2, MM_GIC_APU_DIST_MAIN,
+                                 2, MM_GIC_APU_DIST_MAIN_SIZE,
+                                 2, MM_GIC_APU_REDIST_0,
+                                 2, MM_GIC_APU_REDIST_0_SIZE);
+    qemu_fdt_setprop_cell(s->fdt, nodename, "#interrupt-cells", 3);
+    qemu_fdt_setprop_string(s->fdt, nodename, "compatible", "arm,gic-v3");
+}
+
+static void fdt_add_timer_nodes(VersalVirt *s)
+{
+    const char compat[] = "arm,armv8-timer";
+    uint32_t irqflags = GIC_FDT_IRQ_FLAGS_LEVEL_HI;
+
+    qemu_fdt_add_subnode(s->fdt, "/timer");
+    qemu_fdt_setprop_cells(s->fdt, "/timer", "interrupts",
+            GIC_FDT_IRQ_TYPE_PPI, VERSAL_TIMER_S_EL1_IRQ, irqflags,
+            GIC_FDT_IRQ_TYPE_PPI, VERSAL_TIMER_NS_EL1_IRQ, irqflags,
+            GIC_FDT_IRQ_TYPE_PPI, VERSAL_TIMER_VIRT_IRQ, irqflags,
+            GIC_FDT_IRQ_TYPE_PPI, VERSAL_TIMER_NS_EL2_IRQ, irqflags);
+    qemu_fdt_setprop(s->fdt, "/timer", "compatible",
+                     compat, sizeof(compat));
+}
+
+static void fdt_add_uart_nodes(VersalVirt *s)
+{
+    uint64_t addrs[] = { MM_UART1, MM_UART0 };
+    unsigned int irqs[] = { VERSAL_UART1_IRQ_0, VERSAL_UART0_IRQ_0 };
+    const char compat[] = "arm,pl011\0arm,sbsa-uart";
+    const char clocknames[] = "uartclk\0apb_pclk";
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(addrs); i++) {
+        char *name = g_strdup_printf("/uart@%" PRIx64, addrs[i]);
+        qemu_fdt_add_subnode(s->fdt, name);
+        qemu_fdt_setprop_cell(s->fdt, name, "current-speed", 115200);
+        qemu_fdt_setprop_cells(s->fdt, name, "clocks",
+                               s->phandle.clk_125Mhz, s->phandle.clk_125Mhz);
+        qemu_fdt_setprop(s->fdt, name, "clock-names",
+                         clocknames, sizeof(clocknames));
+
+        qemu_fdt_setprop_cells(s->fdt, name, "interrupts",
+                               GIC_FDT_IRQ_TYPE_SPI, irqs[i],
+                               GIC_FDT_IRQ_FLAGS_LEVEL_HI);
+        qemu_fdt_setprop_sized_cells(s->fdt, name, "reg",
+                                     2, addrs[i], 2, 0x1000);
+        qemu_fdt_setprop(s->fdt, name, "compatible",
+                         compat, sizeof(compat));
+        qemu_fdt_setprop(s->fdt, name, "u-boot,dm-pre-reloc", NULL, 0);
+
+        if (addrs[i] == MM_UART0) {
+            /* Select UART0.  */
+            qemu_fdt_setprop_string(s->fdt, "/chosen", "stdout-path", name);
+        }
+        g_free(name);
+    }
+}
+
+static void fdt_add_fixed_link_nodes(VersalVirt *s, char *gemname,
+                                     uint32_t phandle)
+{
+    char *name = g_strdup_printf("%s/fixed-link", gemname);
+
+    qemu_fdt_add_subnode(s->fdt, name);
+    qemu_fdt_setprop_cell(s->fdt, name, "phandle", phandle);
+    qemu_fdt_setprop_cells(s->fdt, name, "full-duplex");
+    qemu_fdt_setprop_cell(s->fdt, name, "speed", 1000);
+    g_free(name);
+}
+
+static void fdt_add_gem_nodes(VersalVirt *s)
+{
+    uint64_t addrs[] = { MM_GEM1, MM_GEM0 };
+    unsigned int irqs[] = { VERSAL_GEM1_IRQ_0, VERSAL_GEM0_IRQ_0 };
+    const char clocknames[] = "pclk\0hclk\0tx_clk\0rx_clk";
+    const char compat_gem[] = "cdns,zynqmp-gem\0cdns,gem";
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(addrs); i++) {
+        char *name = g_strdup_printf("/ethernet@%" PRIx64, addrs[i]);
+        qemu_fdt_add_subnode(s->fdt, name);
+
+        fdt_add_fixed_link_nodes(s, name, s->phandle.ethernet_phy[i]);
+        qemu_fdt_setprop_string(s->fdt, name, "phy-mode", "rgmii-id");
+        qemu_fdt_setprop_cell(s->fdt, name, "phy-handle",
+                              s->phandle.ethernet_phy[i]);
+        qemu_fdt_setprop_cells(s->fdt, name, "clocks",
+                               s->phandle.clk_25Mhz, s->phandle.clk_25Mhz,
+                               s->phandle.clk_25Mhz, s->phandle.clk_25Mhz);
+        qemu_fdt_setprop(s->fdt, name, "clock-names",
+                         clocknames, sizeof(clocknames));
+        qemu_fdt_setprop_cells(s->fdt, name, "interrupts",
+                               GIC_FDT_IRQ_TYPE_SPI, irqs[i],
+                               GIC_FDT_IRQ_FLAGS_LEVEL_HI,
+                               GIC_FDT_IRQ_TYPE_SPI, irqs[i],
+                               GIC_FDT_IRQ_FLAGS_LEVEL_HI);
+        qemu_fdt_setprop_sized_cells(s->fdt, name, "reg",
+                                     2, addrs[i], 2, 0x1000);
+        qemu_fdt_setprop(s->fdt, name, "compatible",
+                         compat_gem, sizeof(compat_gem));
+        qemu_fdt_setprop_cell(s->fdt, name, "#address-cells", 1);
+        qemu_fdt_setprop_cell(s->fdt, name, "#size-cells", 0);
+        g_free(name);
+    }
+}
+
+static void fdt_nop_memory_nodes(void *fdt, Error **errp)
+{
+    Error *err = NULL;
+    char **node_path;
+    int n = 0;
+
+    node_path = qemu_fdt_node_unit_path(fdt, "memory", &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+    while (node_path[n]) {
+        if (g_str_has_prefix(node_path[n], "/memory")) {
+            qemu_fdt_nop_node(fdt, node_path[n]);
+        }
+        n++;
+    }
+    g_strfreev(node_path);
+}
+
+static void fdt_add_memory_nodes(VersalVirt *s, void *fdt, uint64_t ram_size)
+{
+    /* Describes the various split DDR access regions.  */
+    static const struct {
+        uint64_t base;
+        uint64_t size;
+    } addr_ranges[] = {
+        { MM_TOP_DDR, MM_TOP_DDR_SIZE },
+        { MM_TOP_DDR_2, MM_TOP_DDR_2_SIZE },
+        { MM_TOP_DDR_3, MM_TOP_DDR_3_SIZE },
+        { MM_TOP_DDR_4, MM_TOP_DDR_4_SIZE }
+    };
+    uint64_t mem_reg_prop[8] = {0};
+    uint64_t size = ram_size;
+    Error *err = NULL;
+    char *name;
+    int i;
+
+    fdt_nop_memory_nodes(fdt, &err);
+    if (err) {
+        error_report_err(err);
+        return;
+    }
+
+    name = g_strdup_printf("/memory@%x", MM_TOP_DDR);
+    for (i = 0; i < ARRAY_SIZE(addr_ranges) && size; i++) {
+        uint64_t mapsize;
+
+        mapsize = size < addr_ranges[i].size ? size : addr_ranges[i].size;
+
+        mem_reg_prop[i * 2] = addr_ranges[i].base;
+        mem_reg_prop[i * 2 + 1] = mapsize;
+        size -= mapsize;
+    }
+    qemu_fdt_add_subnode(fdt, name);
+    qemu_fdt_setprop_string(fdt, name, "device_type", "memory");
+
+    switch (i) {
+    case 1:
+        qemu_fdt_setprop_sized_cells(fdt, name, "reg",
+                                     2, mem_reg_prop[0],
+                                     2, mem_reg_prop[1]);
+        break;
+    case 2:
+        qemu_fdt_setprop_sized_cells(fdt, name, "reg",
+                                     2, mem_reg_prop[0],
+                                     2, mem_reg_prop[1],
+                                     2, mem_reg_prop[2],
+                                     2, mem_reg_prop[3]);
+        break;
+    case 3:
+        qemu_fdt_setprop_sized_cells(fdt, name, "reg",
+                                     2, mem_reg_prop[0],
+                                     2, mem_reg_prop[1],
+                                     2, mem_reg_prop[2],
+                                     2, mem_reg_prop[3],
+                                     2, mem_reg_prop[4],
+                                     2, mem_reg_prop[5]);
+        break;
+    case 4:
+        qemu_fdt_setprop_sized_cells(fdt, name, "reg",
+                                     2, mem_reg_prop[0],
+                                     2, mem_reg_prop[1],
+                                     2, mem_reg_prop[2],
+                                     2, mem_reg_prop[3],
+                                     2, mem_reg_prop[4],
+                                     2, mem_reg_prop[5],
+                                     2, mem_reg_prop[6],
+                                     2, mem_reg_prop[7]);
+        break;
+    default:
+        g_assert_not_reached();
+    }
+    g_free(name);
+}
+
+static void versal_virt_modify_dtb(const struct arm_boot_info *binfo,
+                                    void *fdt)
+{
+    VersalVirt *s = container_of(binfo, VersalVirt, binfo);
+
+    fdt_add_memory_nodes(s, fdt, binfo->ram_size);
+}
+
+static void *versal_virt_get_dtb(const struct arm_boot_info *binfo,
+                                  int *fdt_size)
+{
+    const VersalVirt *board = container_of(binfo, VersalVirt, binfo);
+
+    *fdt_size = board->fdt_size;
+    return board->fdt;
+}
+
+#define NUM_VIRTIO_TRANSPORT 32
+static void create_virtio_regions(VersalVirt *s)
+{
+    int virtio_mmio_size = 0x200;
+    int i;
+
+    for (i = 0; i < NUM_VIRTIO_TRANSPORT; i++) {
+        char *name = g_strdup_printf("virtio%d", i);;
+        hwaddr base = MM_TOP_RSVD + i * virtio_mmio_size;
+        int irq = VERSAL_RSVD_HIGH_IRQ_FIRST + i;
+        MemoryRegion *mr;
+        DeviceState *dev;
+        qemu_irq pic_irq;
+
+        pic_irq = qdev_get_gpio_in(DEVICE(&s->soc.fpd.apu.gic), irq);
+        dev = qdev_create(NULL, "virtio-mmio");
+        object_property_add_child(OBJECT(&s->soc), name, OBJECT(dev),
+                                  &error_fatal);
+        qdev_init_nofail(dev);
+        sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic_irq);
+        mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0);
+        memory_region_add_subregion(&s->soc.mr_ps, base, mr);
+        sysbus_create_simple("virtio-mmio", base, pic_irq);
+    }
+
+    for (i = 0; i < NUM_VIRTIO_TRANSPORT; i++) {
+        hwaddr base = MM_TOP_RSVD + i * virtio_mmio_size;
+        int irq = VERSAL_RSVD_HIGH_IRQ_FIRST + i;
+        char *name = g_strdup_printf("/virtio_mmio@%" PRIx64, base);
+
+        qemu_fdt_add_subnode(s->fdt, name);
+        qemu_fdt_setprop(s->fdt, name, "dma-coherent", NULL, 0);
+        qemu_fdt_setprop_cells(s->fdt, name, "interrupts",
+                               GIC_FDT_IRQ_TYPE_SPI, irq,
+                               GIC_FDT_IRQ_FLAGS_EDGE_LO_HI);
+        qemu_fdt_setprop_sized_cells(s->fdt, name, "reg",
+                                     2, base, 2, virtio_mmio_size);
+        qemu_fdt_setprop_string(s->fdt, name, "compatible", "virtio,mmio");
+        g_free(name);
+    }
+}
+
+static void versal_virt_init(MachineState *machine)
+{
+    VersalVirt *s = XLNX_VERSAL_VIRT_MACHINE(machine);
+    int psci_conduit = QEMU_PSCI_CONDUIT_DISABLED;
+
+    /*
+     * If the user provides an Operating System to be loaded, we expect them
+     * to use the -kernel command line option.
+     *
+     * Users can load firmware or boot-loaders with the -device loader options.
+     *
+     * When loading an OS, we generate a dtb and let arm_load_kernel() select
+     * where it gets loaded. This dtb will be passed to the kernel in x0.
+     *
+     * If there's no -kernel option, we generate a DTB and place it at 0x1000
+     * for the bootloaders or firmware to pick up.
+     *
+     * If users want to provide their own DTB, they can use the -dtb option.
+     * These dtb's will have their memory nodes modified to match QEMU's
+     * selected ram_size option before they get passed to the kernel or fw.
+     *
+     * When loading an OS, we turn on QEMU's PSCI implementation with SMC
+     * as the PSCI conduit. When there's no -kernel, we assume the user
+     * provides EL3 firmware to handle PSCI.
+     */
+    if (machine->kernel_filename) {
+        psci_conduit = QEMU_PSCI_CONDUIT_SMC;
+    }
+
+    memory_region_allocate_system_memory(&s->mr_ddr, NULL, "ddr",
+                                         machine->ram_size);
+
+    sysbus_init_child_obj(OBJECT(machine), "xlnx-ve", &s->soc,
+                          sizeof(s->soc), TYPE_XLNX_VERSAL);
+    object_property_set_link(OBJECT(&s->soc), OBJECT(&s->mr_ddr),
+                             "ddr", &error_abort);
+    object_property_set_int(OBJECT(&s->soc), psci_conduit,
+                            "psci-conduit", &error_abort);
+    object_property_set_bool(OBJECT(&s->soc), true, "realized", &error_fatal);
+
+    fdt_create(s);
+    create_virtio_regions(s);
+    fdt_add_gem_nodes(s);
+    fdt_add_uart_nodes(s);
+    fdt_add_gic_nodes(s);
+    fdt_add_timer_nodes(s);
+    fdt_add_cpu_nodes(s, psci_conduit);
+    fdt_add_clk_node(s, "/clk125", 125000000, s->phandle.clk_125Mhz);
+    fdt_add_clk_node(s, "/clk25", 25000000, s->phandle.clk_25Mhz);
+
+    /* Make the APU cpu address space visible to virtio and other
+     * modules unaware of muliple address-spaces.  */
+    memory_region_add_subregion_overlap(get_system_memory(),
+                                        0, &s->soc.fpd.apu.mr, 0);
+
+    s->binfo.ram_size = machine->ram_size;
+    s->binfo.kernel_filename = machine->kernel_filename;
+    s->binfo.kernel_cmdline = machine->kernel_cmdline;
+    s->binfo.initrd_filename = machine->initrd_filename;
+    s->binfo.loader_start = 0x0;
+    s->binfo.get_dtb = versal_virt_get_dtb;
+    s->binfo.modify_dtb = versal_virt_modify_dtb;
+    if (machine->kernel_filename) {
+        arm_load_kernel(s->soc.fpd.apu.cpu[0], &s->binfo);
+    } else {
+        AddressSpace *as = arm_boot_address_space(s->soc.fpd.apu.cpu[0],
+                                                  &s->binfo);
+        /* Some boot-loaders (e.g u-boot) don't like blobs at address 0 (NULL).
+         * Offset things by 4K.  */
+        s->binfo.loader_start = 0x1000;
+        s->binfo.dtb_limit = 0x1000000;
+        if (arm_load_dtb(s->binfo.loader_start,
+                         &s->binfo, s->binfo.dtb_limit, as) < 0) {
+            exit(EXIT_FAILURE);
+        }
+    }
+}
+
+static void versal_virt_machine_instance_init(Object *obj)
+{
+}
+
+static void versal_virt_machine_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+
+    mc->desc = "Xilinx Versal Virtual development board";
+    mc->init = versal_virt_init;
+    mc->max_cpus = XLNX_VERSAL_NR_ACPUS;
+    mc->default_cpus = XLNX_VERSAL_NR_ACPUS;
+    mc->no_cdrom = true;
+}
+
+static const TypeInfo versal_virt_machine_init_typeinfo = {
+    .name       = TYPE_XLNX_VERSAL_VIRT_MACHINE,
+    .parent     = TYPE_MACHINE,
+    .class_init = versal_virt_machine_class_init,
+    .instance_init = versal_virt_machine_instance_init,
+    .instance_size = sizeof(VersalVirt),
+};
+
+static void versal_virt_machine_init_register_types(void)
+{
+    type_register_static(&versal_virt_machine_init_typeinfo);
+}
+
+type_init(versal_virt_machine_init_register_types)
+
-- 
2.7.4

Re: [Qemu-devel] [PATCH v1 01/12] net: cadence_gem: Disable TSU feature bit

[Alistair Francis]

On Wed, Oct 3, 2018 at 8:11 AM Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
>
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Disable the Timestamping Unit feature bit since QEMU does not
> yet support it. This allows guest SW to correctly probe for
> its existance.
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

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

Alistair

> ---
>  hw/net/cadence_gem.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> index 0fa4b0d..e560b7a 100644
> --- a/hw/net/cadence_gem.c
> +++ b/hw/net/cadence_gem.c
> @@ -1228,7 +1228,7 @@ static void gem_reset(DeviceState *d)
>      s->regs[GEM_MODID] = s->revision;
>      s->regs[GEM_DESCONF] = 0x02500111;
>      s->regs[GEM_DESCONF2] = 0x2ab13fff;
> -    s->regs[GEM_DESCONF5] = 0x002f2145;
> +    s->regs[GEM_DESCONF5] = 0x002f2045;
>      s->regs[GEM_DESCONF6] = 0x00000200;
>
>      /* Set MAC address */
> --
> 2.7.4
>
>

Re: [Qemu-devel] [PATCH v1 02/12] net: cadence_gem: Announce availability of priority queues

[Alistair]

On 10/03/2018 08:07 AM, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> 
> Announce the availability of the various priority queues.
> This fixes an issue where guest kernels would miss to
> configure secondary queues due to inproper feature bits.
> 
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

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

Alistair

> ---
>   hw/net/cadence_gem.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> index e560b7a..901c173 100644
> --- a/hw/net/cadence_gem.c
> +++ b/hw/net/cadence_gem.c
> @@ -1213,6 +1213,7 @@ static void gem_reset(DeviceState *d)
>       int i;
>       CadenceGEMState *s = CADENCE_GEM(d);
>       const uint8_t *a;
> +    uint32_t queues_mask;
>   
>       DB_PRINT("\n");
>   
> @@ -1229,7 +1230,10 @@ static void gem_reset(DeviceState *d)
>       s->regs[GEM_DESCONF] = 0x02500111;
>       s->regs[GEM_DESCONF2] = 0x2ab13fff;
>       s->regs[GEM_DESCONF5] = 0x002f2045;
> -    s->regs[GEM_DESCONF6] = 0x00000200;
> +    s->regs[GEM_DESCONF6] = 0x0;
> +
> +    queues_mask = MAKE_64BIT_MASK(1, s->num_priority_queues - 1);
> +    s->regs[GEM_DESCONF6] |= queues_mask;
>   
>       /* Set MAC address */
>       a = &s->conf.macaddr.a[0];
> 

Re: [Qemu-devel] [PATCH v1 03/12] net: cadence_gem: Use uint32_t for 32bit descriptor words

[Alistair]

On 10/03/2018 08:07 AM, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> 
> Use uint32_t instead of unsigned to describe 32bit descriptor words.
> 
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

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

Alistair

> ---
>   hw/net/cadence_gem.c         | 42 +++++++++++++++++++++---------------------
>   include/hw/net/cadence_gem.h |  2 +-
>   2 files changed, 22 insertions(+), 22 deletions(-)
> 
> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> index 901c173..31f3fe0 100644
> --- a/hw/net/cadence_gem.c
> +++ b/hw/net/cadence_gem.c
> @@ -302,42 +302,42 @@
>   
>   #define GEM_MODID_VALUE 0x00020118
>   
> -static inline unsigned tx_desc_get_buffer(unsigned *desc)
> +static inline unsigned tx_desc_get_buffer(uint32_t *desc)
>   {
>       return desc[0];
>   }
>   
> -static inline unsigned tx_desc_get_used(unsigned *desc)
> +static inline unsigned tx_desc_get_used(uint32_t *desc)
>   {
>       return (desc[1] & DESC_1_USED) ? 1 : 0;
>   }
>   
> -static inline void tx_desc_set_used(unsigned *desc)
> +static inline void tx_desc_set_used(uint32_t *desc)
>   {
>       desc[1] |= DESC_1_USED;
>   }
>   
> -static inline unsigned tx_desc_get_wrap(unsigned *desc)
> +static inline unsigned tx_desc_get_wrap(uint32_t *desc)
>   {
>       return (desc[1] & DESC_1_TX_WRAP) ? 1 : 0;
>   }
>   
> -static inline unsigned tx_desc_get_last(unsigned *desc)
> +static inline unsigned tx_desc_get_last(uint32_t *desc)
>   {
>       return (desc[1] & DESC_1_TX_LAST) ? 1 : 0;
>   }
>   
> -static inline void tx_desc_set_last(unsigned *desc)
> +static inline void tx_desc_set_last(uint32_t *desc)
>   {
>       desc[1] |= DESC_1_TX_LAST;
>   }
>   
> -static inline unsigned tx_desc_get_length(unsigned *desc)
> +static inline unsigned tx_desc_get_length(uint32_t *desc)
>   {
>       return desc[1] & DESC_1_LENGTH;
>   }
>   
> -static inline void print_gem_tx_desc(unsigned *desc, uint8_t queue)
> +static inline void print_gem_tx_desc(uint32_t *desc, uint8_t queue)
>   {
>       DB_PRINT("TXDESC (queue %" PRId8 "):\n", queue);
>       DB_PRINT("bufaddr: 0x%08x\n", *desc);
> @@ -347,58 +347,58 @@ static inline void print_gem_tx_desc(unsigned *desc, uint8_t queue)
>       DB_PRINT("length:  %d\n", tx_desc_get_length(desc));
>   }
>   
> -static inline unsigned rx_desc_get_buffer(unsigned *desc)
> +static inline unsigned rx_desc_get_buffer(uint32_t *desc)
>   {
>       return desc[0] & ~0x3UL;
>   }
>   
> -static inline unsigned rx_desc_get_wrap(unsigned *desc)
> +static inline unsigned rx_desc_get_wrap(uint32_t *desc)
>   {
>       return desc[0] & DESC_0_RX_WRAP ? 1 : 0;
>   }
>   
> -static inline unsigned rx_desc_get_ownership(unsigned *desc)
> +static inline unsigned rx_desc_get_ownership(uint32_t *desc)
>   {
>       return desc[0] & DESC_0_RX_OWNERSHIP ? 1 : 0;
>   }
>   
> -static inline void rx_desc_set_ownership(unsigned *desc)
> +static inline void rx_desc_set_ownership(uint32_t *desc)
>   {
>       desc[0] |= DESC_0_RX_OWNERSHIP;
>   }
>   
> -static inline void rx_desc_set_sof(unsigned *desc)
> +static inline void rx_desc_set_sof(uint32_t *desc)
>   {
>       desc[1] |= DESC_1_RX_SOF;
>   }
>   
> -static inline void rx_desc_set_eof(unsigned *desc)
> +static inline void rx_desc_set_eof(uint32_t *desc)
>   {
>       desc[1] |= DESC_1_RX_EOF;
>   }
>   
> -static inline void rx_desc_set_length(unsigned *desc, unsigned len)
> +static inline void rx_desc_set_length(uint32_t *desc, unsigned len)
>   {
>       desc[1] &= ~DESC_1_LENGTH;
>       desc[1] |= len;
>   }
>   
> -static inline void rx_desc_set_broadcast(unsigned *desc)
> +static inline void rx_desc_set_broadcast(uint32_t *desc)
>   {
>       desc[1] |= R_DESC_1_RX_BROADCAST;
>   }
>   
> -static inline void rx_desc_set_unicast_hash(unsigned *desc)
> +static inline void rx_desc_set_unicast_hash(uint32_t *desc)
>   {
>       desc[1] |= R_DESC_1_RX_UNICAST_HASH;
>   }
>   
> -static inline void rx_desc_set_multicast_hash(unsigned *desc)
> +static inline void rx_desc_set_multicast_hash(uint32_t *desc)
>   {
>       desc[1] |= R_DESC_1_RX_MULTICAST_HASH;
>   }
>   
> -static inline void rx_desc_set_sar(unsigned *desc, int sar_idx)
> +static inline void rx_desc_set_sar(uint32_t *desc, int sar_idx)
>   {
>       desc[1] = deposit32(desc[1], R_DESC_1_RX_SAR_SHIFT, R_DESC_1_RX_SAR_LENGTH,
>                           sar_idx);
> @@ -1042,7 +1042,7 @@ static void gem_transmit_updatestats(CadenceGEMState *s, const uint8_t *packet,
>    */
>   static void gem_transmit(CadenceGEMState *s)
>   {
> -    unsigned    desc[2];
> +    uint32_t desc[2];
>       hwaddr packet_desc_addr;
>       uint8_t     tx_packet[2048];
>       uint8_t     *p;
> @@ -1108,7 +1108,7 @@ static void gem_transmit(CadenceGEMState *s)
>   
>               /* Last descriptor for this packet; hand the whole thing off */
>               if (tx_desc_get_last(desc)) {
> -                unsigned    desc_first[2];
> +                uint32_t desc_first[2];
>   
>                   /* Modify the 1st descriptor of this packet to be owned by
>                    * the processor.
> diff --git a/include/hw/net/cadence_gem.h b/include/hw/net/cadence_gem.h
> index 35de622..633d564 100644
> --- a/include/hw/net/cadence_gem.h
> +++ b/include/hw/net/cadence_gem.h
> @@ -74,7 +74,7 @@ typedef struct CadenceGEMState {
>   
>       uint8_t can_rx_state; /* Debug only */
>   
> -    unsigned rx_desc[MAX_PRIORITY_QUEUES][2];
> +    uint32_t rx_desc[MAX_PRIORITY_QUEUES][2];
>   
>       bool sar_active[4];
>   } CadenceGEMState;
> 

Re: [Qemu-devel] [PATCH v1 04/12] net: cadence_gem: Add macro with max number of descriptor words

[Alistair]

On 10/03/2018 08:07 AM, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> 
> Add macro with max number of DMA descriptor words.
> No functional change.
> 
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

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

Alistair

> ---
>   hw/net/cadence_gem.c         | 4 ++--
>   include/hw/net/cadence_gem.h | 5 ++++-
>   2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> index 31f3fe0..4d769b0 100644
> --- a/hw/net/cadence_gem.c
> +++ b/hw/net/cadence_gem.c
> @@ -1042,7 +1042,7 @@ static void gem_transmit_updatestats(CadenceGEMState *s, const uint8_t *packet,
>    */
>   static void gem_transmit(CadenceGEMState *s)
>   {
> -    uint32_t desc[2];
> +    uint32_t desc[DESC_MAX_NUM_WORDS];
>       hwaddr packet_desc_addr;
>       uint8_t     tx_packet[2048];
>       uint8_t     *p;
> @@ -1108,7 +1108,7 @@ static void gem_transmit(CadenceGEMState *s)
>   
>               /* Last descriptor for this packet; hand the whole thing off */
>               if (tx_desc_get_last(desc)) {
> -                uint32_t desc_first[2];
> +                uint32_t desc_first[DESC_MAX_NUM_WORDS];
>   
>                   /* Modify the 1st descriptor of this packet to be owned by
>                    * the processor.
> diff --git a/include/hw/net/cadence_gem.h b/include/hw/net/cadence_gem.h
> index 633d564..b33ef65 100644
> --- a/include/hw/net/cadence_gem.h
> +++ b/include/hw/net/cadence_gem.h
> @@ -32,6 +32,9 @@
>   
>   #define CADENCE_GEM_MAXREG        (0x00000800 / 4) /* Last valid GEM address */
>   
> +/* Max number of words in a DMA descriptor.  */
> +#define DESC_MAX_NUM_WORDS              2
> +
>   #define MAX_PRIORITY_QUEUES             8
>   #define MAX_TYPE1_SCREENERS             16
>   #define MAX_TYPE2_SCREENERS             16
> @@ -74,7 +77,7 @@ typedef struct CadenceGEMState {
>   
>       uint8_t can_rx_state; /* Debug only */
>   
> -    uint32_t rx_desc[MAX_PRIORITY_QUEUES][2];
> +    uint32_t rx_desc[MAX_PRIORITY_QUEUES][DESC_MAX_NUM_WORDS];
>   
>       bool sar_active[4];
>   } CadenceGEMState;
> 

Re: [Qemu-devel] [PATCH v1 05/12] net: cadence_gem: Add support for extended descriptors

[Alistair]

On 10/03/2018 08:07 AM, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> 
> Add support for extended descriptors with optional 64bit
> addressing and timestamping. QEMU will not yet provide
> timestamps (always leaving the valid timestamp bit as zero).
> 
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

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

Alistair

> ---
>   hw/net/cadence_gem.c         | 69 ++++++++++++++++++++++++++++++++------------
>   include/hw/net/cadence_gem.h |  2 +-
>   2 files changed, 52 insertions(+), 19 deletions(-)
> 
> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> index 4d769b0..759c1d7 100644
> --- a/hw/net/cadence_gem.c
> +++ b/hw/net/cadence_gem.c
> @@ -207,6 +207,9 @@
>   #define GEM_NWCFG_BCAST_REJ    0x00000020 /* Reject broadcast packets */
>   #define GEM_NWCFG_PROMISC      0x00000010 /* Accept all packets */
>   
> +#define GEM_DMACFG_ADDR_64B    (1U << 30)
> +#define GEM_DMACFG_TX_BD_EXT   (1U << 29)
> +#define GEM_DMACFG_RX_BD_EXT   (1U << 28)
>   #define GEM_DMACFG_RBUFSZ_M    0x00FF0000 /* DMA RX Buffer Size mask */
>   #define GEM_DMACFG_RBUFSZ_S    16         /* DMA RX Buffer Size shift */
>   #define GEM_DMACFG_RBUFSZ_MUL  64         /* DMA RX Buffer Size multiplier */
> @@ -302,9 +305,14 @@
>   
>   #define GEM_MODID_VALUE 0x00020118
>   
> -static inline unsigned tx_desc_get_buffer(uint32_t *desc)
> +static inline uint64_t tx_desc_get_buffer(CadenceGEMState *s, uint32_t *desc)
>   {
> -    return desc[0];
> +    uint64_t ret = desc[0];
> +
> +    if (s->regs[GEM_DMACFG] & GEM_DMACFG_ADDR_64B) {
> +        ret |= (uint64_t)desc[2] << 32;
> +    }
> +    return ret;
>   }
>   
>   static inline unsigned tx_desc_get_used(uint32_t *desc)
> @@ -347,9 +355,30 @@ static inline void print_gem_tx_desc(uint32_t *desc, uint8_t queue)
>       DB_PRINT("length:  %d\n", tx_desc_get_length(desc));
>   }
>   
> -static inline unsigned rx_desc_get_buffer(uint32_t *desc)
> +static inline uint64_t rx_desc_get_buffer(CadenceGEMState *s, uint32_t *desc)
>   {
> -    return desc[0] & ~0x3UL;
> +    uint64_t ret = desc[0] & ~0x3UL;
> +
> +    if (s->regs[GEM_DMACFG] & GEM_DMACFG_ADDR_64B) {
> +        ret |= (uint64_t)desc[2] << 32;
> +    }
> +    return ret;
> +}
> +
> +static inline int gem_get_desc_len(CadenceGEMState *s, bool rx_n_tx)
> +{
> +    int ret = 2;
> +
> +    if (s->regs[GEM_DMACFG] & GEM_DMACFG_ADDR_64B) {
> +        ret += 2;
> +    }
> +    if (s->regs[GEM_DMACFG] & (rx_n_tx ? GEM_DMACFG_RX_BD_EXT
> +                                       : GEM_DMACFG_TX_BD_EXT)) {
> +        ret += 2;
> +    }
> +
> +    assert(ret <= DESC_MAX_NUM_WORDS);
> +    return ret;
>   }
>   
>   static inline unsigned rx_desc_get_wrap(uint32_t *desc)
> @@ -419,7 +448,7 @@ static void gem_init_register_masks(CadenceGEMState *s)
>       memset(&s->regs_ro[0], 0, sizeof(s->regs_ro));
>       s->regs_ro[GEM_NWCTRL]   = 0xFFF80000;
>       s->regs_ro[GEM_NWSTATUS] = 0xFFFFFFFF;
> -    s->regs_ro[GEM_DMACFG]   = 0xFE00F000;
> +    s->regs_ro[GEM_DMACFG]   = 0x8E00F000;
>       s->regs_ro[GEM_TXSTATUS] = 0xFFFFFE08;
>       s->regs_ro[GEM_RXQBASE]  = 0x00000003;
>       s->regs_ro[GEM_TXQBASE]  = 0x00000003;
> @@ -807,7 +836,8 @@ static void gem_get_rx_desc(CadenceGEMState *s, int q)
>       DB_PRINT("read descriptor 0x%x\n", (unsigned)s->rx_desc_addr[q]);
>       /* read current descriptor */
>       cpu_physical_memory_read(s->rx_desc_addr[q],
> -                             (uint8_t *)s->rx_desc[q], sizeof(s->rx_desc[q]));
> +                             (uint8_t *)s->rx_desc[q],
> +                             sizeof(uint32_t) * gem_get_desc_len(s, true));
>   
>       /* Descriptor owned by software ? */
>       if (rx_desc_get_ownership(s->rx_desc[q]) == 1) {
> @@ -926,9 +956,10 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
>                   rx_desc_get_buffer(s->rx_desc[q]));
>   
>           /* Copy packet data to emulated DMA buffer */
> -        cpu_physical_memory_write(rx_desc_get_buffer(s->rx_desc[q]) +
> -                                                                 rxbuf_offset,
> -                                  rxbuf_ptr, MIN(bytes_to_copy, rxbufsize));
> +        cpu_physical_memory_write(rx_desc_get_buffer(s, s->rx_desc[q]) +
> +                                                                  rxbuf_offset,
> +                                  rxbuf_ptr,
> +                                  MIN(bytes_to_copy, rxbufsize));
>           rxbuf_ptr += MIN(bytes_to_copy, rxbufsize);
>           bytes_to_copy -= MIN(bytes_to_copy, rxbufsize);
>   
> @@ -964,7 +995,7 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
>           /* Descriptor write-back.  */
>           cpu_physical_memory_write(s->rx_desc_addr[q],
>                                     (uint8_t *)s->rx_desc[q],
> -                                  sizeof(s->rx_desc[q]));
> +                                  sizeof(uint32_t) * gem_get_desc_len(s, true));
>   
>           /* Next descriptor */
>           if (rx_desc_get_wrap(s->rx_desc[q])) {
> @@ -972,7 +1003,7 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
>               s->rx_desc_addr[q] = s->regs[GEM_RXQBASE];
>           } else {
>               DB_PRINT("incrementing RX descriptor list\n");
> -            s->rx_desc_addr[q] += 8;
> +            s->rx_desc_addr[q] += 4 * gem_get_desc_len(s, true);
>           }
>   
>           gem_get_rx_desc(s, q);
> @@ -1069,7 +1100,8 @@ static void gem_transmit(CadenceGEMState *s)
>   
>           DB_PRINT("read descriptor 0x%" HWADDR_PRIx "\n", packet_desc_addr);
>           cpu_physical_memory_read(packet_desc_addr,
> -                                 (uint8_t *)desc, sizeof(desc));
> +                                 (uint8_t *)desc,
> +                                 sizeof(uint32_t) * gem_get_desc_len(s, false));
>           /* Handle all descriptors owned by hardware */
>           while (tx_desc_get_used(desc) == 0) {
>   
> @@ -1082,7 +1114,7 @@ static void gem_transmit(CadenceGEMState *s)
>               /* The real hardware would eat this (and possibly crash).
>                * For QEMU let's lend a helping hand.
>                */
> -            if ((tx_desc_get_buffer(desc) == 0) ||
> +            if ((tx_desc_get_buffer(s, desc) == 0) ||
>                   (tx_desc_get_length(desc) == 0)) {
>                   DB_PRINT("Invalid TX descriptor @ 0x%x\n",
>                            (unsigned)packet_desc_addr);
> @@ -1101,7 +1133,7 @@ static void gem_transmit(CadenceGEMState *s)
>               /* Gather this fragment of the packet from "dma memory" to our
>                * contig buffer.
>                */
> -            cpu_physical_memory_read(tx_desc_get_buffer(desc), p,
> +            cpu_physical_memory_read(tx_desc_get_buffer(s, desc), p,
>                                        tx_desc_get_length(desc));
>               p += tx_desc_get_length(desc);
>               total_bytes += tx_desc_get_length(desc);
> @@ -1124,7 +1156,8 @@ static void gem_transmit(CadenceGEMState *s)
>                   if (tx_desc_get_wrap(desc)) {
>                       s->tx_desc_addr[q] = s->regs[GEM_TXQBASE];
>                   } else {
> -                    s->tx_desc_addr[q] = packet_desc_addr + 8;
> +                    s->tx_desc_addr[q] = packet_desc_addr +
> +                                         4 * gem_get_desc_len(s, false);
>                   }
>                   DB_PRINT("TX descriptor next: 0x%08x\n", s->tx_desc_addr[q]);
>   
> @@ -1168,11 +1201,11 @@ static void gem_transmit(CadenceGEMState *s)
>                   tx_desc_set_last(desc);
>                   packet_desc_addr = s->regs[GEM_TXQBASE];
>               } else {
> -                packet_desc_addr += 8;
> +                packet_desc_addr += 4 * gem_get_desc_len(s, false);
>               }
>               DB_PRINT("read descriptor 0x%" HWADDR_PRIx "\n", packet_desc_addr);
> -            cpu_physical_memory_read(packet_desc_addr,
> -                                     (uint8_t *)desc, sizeof(desc));
> +            cpu_physical_memory_read(packet_desc_addr, (uint8_t *)desc,
> +                                sizeof(uint32_t) * gem_get_desc_len(s, false));
>           }
>   
>           if (tx_desc_get_used(desc)) {
> diff --git a/include/hw/net/cadence_gem.h b/include/hw/net/cadence_gem.h
> index b33ef65..00dbf4f 100644
> --- a/include/hw/net/cadence_gem.h
> +++ b/include/hw/net/cadence_gem.h
> @@ -33,7 +33,7 @@
>   #define CADENCE_GEM_MAXREG        (0x00000800 / 4) /* Last valid GEM address */
>   
>   /* Max number of words in a DMA descriptor.  */
> -#define DESC_MAX_NUM_WORDS              2
> +#define DESC_MAX_NUM_WORDS              6
>   
>   #define MAX_PRIORITY_QUEUES             8
>   #define MAX_TYPE1_SCREENERS             16
> 

Re: [Qemu-devel] [PATCH v1 08/12] net: cadence_gem: Announce 64bit addressing support

[Alistair]

On 10/03/2018 08:07 AM, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> 
> Announce 64bit addressing support.
> 
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

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

Alistair

> ---
>   hw/net/cadence_gem.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> index f93cd8e..fc81fb5 100644
> --- a/hw/net/cadence_gem.c
> +++ b/hw/net/cadence_gem.c
> @@ -142,6 +142,7 @@
>   #define GEM_DESCONF4      (0x0000028C/4)
>   #define GEM_DESCONF5      (0x00000290/4)
>   #define GEM_DESCONF6      (0x00000294/4)
> +#define GEM_DESCONF6_64B_MASK (1U << 23)
>   #define GEM_DESCONF7      (0x00000298/4)
>   
>   #define GEM_INT_Q1_STATUS               (0x00000400 / 4)
> @@ -1300,7 +1301,7 @@ static void gem_reset(DeviceState *d)
>       s->regs[GEM_DESCONF] = 0x02500111;
>       s->regs[GEM_DESCONF2] = 0x2ab13fff;
>       s->regs[GEM_DESCONF5] = 0x002f2045;
> -    s->regs[GEM_DESCONF6] = 0x0;
> +    s->regs[GEM_DESCONF6] = GEM_DESCONF6_64B_MASK;
>   
>       queues_mask = MAKE_64BIT_MASK(1, s->num_priority_queues - 1);
>       s->regs[GEM_DESCONF6] |= queues_mask;
> 

Re: [Qemu-devel] [PATCH v1 06/12] net: cadence_gem: Add support for selecting the DMA MemoryRegion

[Alistair]

On 10/03/2018 08:07 AM, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> 
> Add support for selecting the Memory Region that the GEM
> will do DMA to.
> 
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

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

Alistair

> ---
>   hw/net/cadence_gem.c         | 63 ++++++++++++++++++++++++++++----------------
>   include/hw/net/cadence_gem.h |  2 ++
>   2 files changed, 43 insertions(+), 22 deletions(-)
> 
> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> index 759c1d7..ab02515 100644
> --- a/hw/net/cadence_gem.c
> +++ b/hw/net/cadence_gem.c
> @@ -28,6 +28,7 @@
>   #include "hw/net/cadence_gem.h"
>   #include "qapi/error.h"
>   #include "qemu/log.h"
> +#include "sysemu/dma.h"
>   #include "net/checksum.h"
>   
>   #ifdef CADENCE_GEM_ERR_DEBUG
> @@ -835,9 +836,9 @@ static void gem_get_rx_desc(CadenceGEMState *s, int q)
>   {
>       DB_PRINT("read descriptor 0x%x\n", (unsigned)s->rx_desc_addr[q]);
>       /* read current descriptor */
> -    cpu_physical_memory_read(s->rx_desc_addr[q],
> -                             (uint8_t *)s->rx_desc[q],
> -                             sizeof(uint32_t) * gem_get_desc_len(s, true));
> +    address_space_read(s->dma_as, s->rx_desc_addr[q], MEMTXATTRS_UNSPECIFIED,
> +                       (uint8_t *)s->rx_desc[q],
> +                       sizeof(uint32_t) * gem_get_desc_len(s, true));
>   
>       /* Descriptor owned by software ? */
>       if (rx_desc_get_ownership(s->rx_desc[q]) == 1) {
> @@ -956,10 +957,10 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
>                   rx_desc_get_buffer(s->rx_desc[q]));
>   
>           /* Copy packet data to emulated DMA buffer */
> -        cpu_physical_memory_write(rx_desc_get_buffer(s, s->rx_desc[q]) +
> +        address_space_write(s->dma_as, rx_desc_get_buffer(s, s->rx_desc[q]) +
>                                                                     rxbuf_offset,
> -                                  rxbuf_ptr,
> -                                  MIN(bytes_to_copy, rxbufsize));
> +                            MEMTXATTRS_UNSPECIFIED, rxbuf_ptr,
> +                            MIN(bytes_to_copy, rxbufsize));
>           rxbuf_ptr += MIN(bytes_to_copy, rxbufsize);
>           bytes_to_copy -= MIN(bytes_to_copy, rxbufsize);
>   
> @@ -993,9 +994,10 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
>           }
>   
>           /* Descriptor write-back.  */
> -        cpu_physical_memory_write(s->rx_desc_addr[q],
> -                                  (uint8_t *)s->rx_desc[q],
> -                                  sizeof(uint32_t) * gem_get_desc_len(s, true));
> +        address_space_write(s->dma_as, s->rx_desc_addr[q],
> +                            MEMTXATTRS_UNSPECIFIED,
> +                            (uint8_t *)s->rx_desc[q],
> +                            sizeof(uint32_t) * gem_get_desc_len(s, true));
>   
>           /* Next descriptor */
>           if (rx_desc_get_wrap(s->rx_desc[q])) {
> @@ -1099,9 +1101,9 @@ static void gem_transmit(CadenceGEMState *s)
>           packet_desc_addr = s->tx_desc_addr[q];
>   
>           DB_PRINT("read descriptor 0x%" HWADDR_PRIx "\n", packet_desc_addr);
> -        cpu_physical_memory_read(packet_desc_addr,
> -                                 (uint8_t *)desc,
> -                                 sizeof(uint32_t) * gem_get_desc_len(s, false));
> +        address_space_read(s->dma_as, packet_desc_addr,
> +                           MEMTXATTRS_UNSPECIFIED, (uint8_t *)desc,
> +                           sizeof(uint32_t) * gem_get_desc_len(s, false));
>           /* Handle all descriptors owned by hardware */
>           while (tx_desc_get_used(desc) == 0) {
>   
> @@ -1133,8 +1135,9 @@ static void gem_transmit(CadenceGEMState *s)
>               /* Gather this fragment of the packet from "dma memory" to our
>                * contig buffer.
>                */
> -            cpu_physical_memory_read(tx_desc_get_buffer(s, desc), p,
> -                                     tx_desc_get_length(desc));
> +            address_space_read(s->dma_as, tx_desc_get_buffer(s, desc),
> +                               MEMTXATTRS_UNSPECIFIED,
> +                               p, tx_desc_get_length(desc));
>               p += tx_desc_get_length(desc);
>               total_bytes += tx_desc_get_length(desc);
>   
> @@ -1145,13 +1148,15 @@ static void gem_transmit(CadenceGEMState *s)
>                   /* Modify the 1st descriptor of this packet to be owned by
>                    * the processor.
>                    */
> -                cpu_physical_memory_read(s->tx_desc_addr[q],
> -                                         (uint8_t *)desc_first,
> -                                         sizeof(desc_first));
> +                address_space_read(s->dma_as, s->tx_desc_addr[q],
> +                                   MEMTXATTRS_UNSPECIFIED,
> +                                   (uint8_t *)desc_first,
> +                                   sizeof(desc_first));
>                   tx_desc_set_used(desc_first);
> -                cpu_physical_memory_write(s->tx_desc_addr[q],
> -                                          (uint8_t *)desc_first,
> -                                          sizeof(desc_first));
> +                address_space_write(s->dma_as, s->tx_desc_addr[q],
> +                                  MEMTXATTRS_UNSPECIFIED,
> +                                  (uint8_t *)desc_first,
> +                                   sizeof(desc_first));
>                   /* Advance the hardware current descriptor past this packet */
>                   if (tx_desc_get_wrap(desc)) {
>                       s->tx_desc_addr[q] = s->regs[GEM_TXQBASE];
> @@ -1204,8 +1209,9 @@ static void gem_transmit(CadenceGEMState *s)
>                   packet_desc_addr += 4 * gem_get_desc_len(s, false);
>               }
>               DB_PRINT("read descriptor 0x%" HWADDR_PRIx "\n", packet_desc_addr);
> -            cpu_physical_memory_read(packet_desc_addr, (uint8_t *)desc,
> -                                sizeof(uint32_t) * gem_get_desc_len(s, false));
> +            address_space_read(s->dma_as, packet_desc_addr,
> +                              MEMTXATTRS_UNSPECIFIED, (uint8_t *)desc,
> +                              sizeof(uint32_t) * gem_get_desc_len(s, false));
>           }
>   
>           if (tx_desc_get_used(desc)) {
> @@ -1500,6 +1506,13 @@ static void gem_realize(DeviceState *dev, Error **errp)
>       CadenceGEMState *s = CADENCE_GEM(dev);
>       int i;
>   
> +    if (s->dma_mr) {
> +        s->dma_as = g_malloc0(sizeof(AddressSpace));
> +        address_space_init(s->dma_as, s->dma_mr, NULL);
> +    } else {
> +        s->dma_as = &address_space_memory;
> +    }
> +
>       if (s->num_priority_queues == 0 ||
>           s->num_priority_queues > MAX_PRIORITY_QUEUES) {
>           error_setg(errp, "Invalid num-priority-queues value: %" PRIx8,
> @@ -1537,6 +1550,12 @@ static void gem_init(Object *obj)
>                             "enet", sizeof(s->regs));
>   
>       sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->iomem);
> +
> +    object_property_add_link(obj, "dma", TYPE_MEMORY_REGION,
> +                             (Object **)&s->dma_mr,
> +                             qdev_prop_allow_set_link_before_realize,
> +                             OBJ_PROP_LINK_STRONG,
> +                             &error_abort);
>   }
>   
>   static const VMStateDescription vmstate_cadence_gem = {
> diff --git a/include/hw/net/cadence_gem.h b/include/hw/net/cadence_gem.h
> index 00dbf4f..c8f0751 100644
> --- a/include/hw/net/cadence_gem.h
> +++ b/include/hw/net/cadence_gem.h
> @@ -45,6 +45,8 @@ typedef struct CadenceGEMState {
>   
>       /*< public >*/
>       MemoryRegion iomem;
> +    MemoryRegion *dma_mr;
> +    AddressSpace *dma_as;
>       NICState *nic;
>       NICConf conf;
>       qemu_irq irq[MAX_PRIORITY_QUEUES];
> 

Re: [Qemu-devel] [PATCH v1 03/12] net: cadence_gem: Use uint32_t for 32bit descriptor words

[Philippe Mathieu-Daudé]

On 03/10/2018 17:07, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> 
> Use uint32_t instead of unsigned to describe 32bit descriptor words.
> 
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  hw/net/cadence_gem.c         | 42 +++++++++++++++++++++---------------------
>  include/hw/net/cadence_gem.h |  2 +-
>  2 files changed, 22 insertions(+), 22 deletions(-)
> 
> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> index 901c173..31f3fe0 100644
> --- a/hw/net/cadence_gem.c
> +++ b/hw/net/cadence_gem.c
> @@ -302,42 +302,42 @@
>  
>  #define GEM_MODID_VALUE 0x00020118
>  
> -static inline unsigned tx_desc_get_buffer(unsigned *desc)
> +static inline unsigned tx_desc_get_buffer(uint32_t *desc)
>  {
>      return desc[0];
>  }
>  
> -static inline unsigned tx_desc_get_used(unsigned *desc)
> +static inline unsigned tx_desc_get_used(uint32_t *desc)
>  {
>      return (desc[1] & DESC_1_USED) ? 1 : 0;
>  }
>  
> -static inline void tx_desc_set_used(unsigned *desc)
> +static inline void tx_desc_set_used(uint32_t *desc)
>  {
>      desc[1] |= DESC_1_USED;
>  }
>  
> -static inline unsigned tx_desc_get_wrap(unsigned *desc)
> +static inline unsigned tx_desc_get_wrap(uint32_t *desc)
>  {
>      return (desc[1] & DESC_1_TX_WRAP) ? 1 : 0;
>  }
>  
> -static inline unsigned tx_desc_get_last(unsigned *desc)
> +static inline unsigned tx_desc_get_last(uint32_t *desc)
>  {
>      return (desc[1] & DESC_1_TX_LAST) ? 1 : 0;
>  }
>  
> -static inline void tx_desc_set_last(unsigned *desc)
> +static inline void tx_desc_set_last(uint32_t *desc)
>  {
>      desc[1] |= DESC_1_TX_LAST;
>  }
>  
> -static inline unsigned tx_desc_get_length(unsigned *desc)
> +static inline unsigned tx_desc_get_length(uint32_t *desc)
>  {
>      return desc[1] & DESC_1_LENGTH;
>  }
>  
> -static inline void print_gem_tx_desc(unsigned *desc, uint8_t queue)
> +static inline void print_gem_tx_desc(uint32_t *desc, uint8_t queue)
>  {
>      DB_PRINT("TXDESC (queue %" PRId8 "):\n", queue);
>      DB_PRINT("bufaddr: 0x%08x\n", *desc);
> @@ -347,58 +347,58 @@ static inline void print_gem_tx_desc(unsigned *desc, uint8_t queue)
>      DB_PRINT("length:  %d\n", tx_desc_get_length(desc));
>  }
>  
> -static inline unsigned rx_desc_get_buffer(unsigned *desc)
> +static inline unsigned rx_desc_get_buffer(uint32_t *desc)
>  {
>      return desc[0] & ~0x3UL;
>  }
>  
> -static inline unsigned rx_desc_get_wrap(unsigned *desc)
> +static inline unsigned rx_desc_get_wrap(uint32_t *desc)
>  {
>      return desc[0] & DESC_0_RX_WRAP ? 1 : 0;
>  }
>  
> -static inline unsigned rx_desc_get_ownership(unsigned *desc)
> +static inline unsigned rx_desc_get_ownership(uint32_t *desc)
>  {
>      return desc[0] & DESC_0_RX_OWNERSHIP ? 1 : 0;
>  }
>  
> -static inline void rx_desc_set_ownership(unsigned *desc)
> +static inline void rx_desc_set_ownership(uint32_t *desc)
>  {
>      desc[0] |= DESC_0_RX_OWNERSHIP;
>  }
>  
> -static inline void rx_desc_set_sof(unsigned *desc)
> +static inline void rx_desc_set_sof(uint32_t *desc)
>  {
>      desc[1] |= DESC_1_RX_SOF;
>  }
>  
> -static inline void rx_desc_set_eof(unsigned *desc)
> +static inline void rx_desc_set_eof(uint32_t *desc)
>  {
>      desc[1] |= DESC_1_RX_EOF;
>  }
>  
> -static inline void rx_desc_set_length(unsigned *desc, unsigned len)
> +static inline void rx_desc_set_length(uint32_t *desc, unsigned len)
>  {
>      desc[1] &= ~DESC_1_LENGTH;
>      desc[1] |= len;
>  }
>  
> -static inline void rx_desc_set_broadcast(unsigned *desc)
> +static inline void rx_desc_set_broadcast(uint32_t *desc)
>  {
>      desc[1] |= R_DESC_1_RX_BROADCAST;
>  }
>  
> -static inline void rx_desc_set_unicast_hash(unsigned *desc)
> +static inline void rx_desc_set_unicast_hash(uint32_t *desc)
>  {
>      desc[1] |= R_DESC_1_RX_UNICAST_HASH;
>  }
>  
> -static inline void rx_desc_set_multicast_hash(unsigned *desc)
> +static inline void rx_desc_set_multicast_hash(uint32_t *desc)
>  {
>      desc[1] |= R_DESC_1_RX_MULTICAST_HASH;
>  }
>  
> -static inline void rx_desc_set_sar(unsigned *desc, int sar_idx)
> +static inline void rx_desc_set_sar(uint32_t *desc, int sar_idx)
>  {
>      desc[1] = deposit32(desc[1], R_DESC_1_RX_SAR_SHIFT, R_DESC_1_RX_SAR_LENGTH,
>                          sar_idx);
> @@ -1042,7 +1042,7 @@ static void gem_transmit_updatestats(CadenceGEMState *s, const uint8_t *packet,
>   */
>  static void gem_transmit(CadenceGEMState *s)
>  {
> -    unsigned    desc[2];
> +    uint32_t desc[2];
>      hwaddr packet_desc_addr;
>      uint8_t     tx_packet[2048];
>      uint8_t     *p;
> @@ -1108,7 +1108,7 @@ static void gem_transmit(CadenceGEMState *s)
>  
>              /* Last descriptor for this packet; hand the whole thing off */
>              if (tx_desc_get_last(desc)) {
> -                unsigned    desc_first[2];
> +                uint32_t desc_first[2];
>  
>                  /* Modify the 1st descriptor of this packet to be owned by
>                   * the processor.
> diff --git a/include/hw/net/cadence_gem.h b/include/hw/net/cadence_gem.h
> index 35de622..633d564 100644
> --- a/include/hw/net/cadence_gem.h
> +++ b/include/hw/net/cadence_gem.h
> @@ -74,7 +74,7 @@ typedef struct CadenceGEMState {
>  
>      uint8_t can_rx_state; /* Debug only */
>  
> -    unsigned rx_desc[MAX_PRIORITY_QUEUES][2];
> +    uint32_t rx_desc[MAX_PRIORITY_QUEUES][2];
>  
>      bool sar_active[4];
>  } CadenceGEMState;
> 

Re: [Qemu-devel] [PATCH v1 04/12] net: cadence_gem: Add macro with max number of descriptor words

[Philippe Mathieu-Daudé]

On 03/10/2018 17:07, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> 
> Add macro with max number of DMA descriptor words.
> No functional change.
> 
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  hw/net/cadence_gem.c         | 4 ++--
>  include/hw/net/cadence_gem.h | 5 ++++-
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> index 31f3fe0..4d769b0 100644
> --- a/hw/net/cadence_gem.c
> +++ b/hw/net/cadence_gem.c
> @@ -1042,7 +1042,7 @@ static void gem_transmit_updatestats(CadenceGEMState *s, const uint8_t *packet,
>   */
>  static void gem_transmit(CadenceGEMState *s)
>  {
> -    uint32_t desc[2];
> +    uint32_t desc[DESC_MAX_NUM_WORDS];
>      hwaddr packet_desc_addr;
>      uint8_t     tx_packet[2048];
>      uint8_t     *p;
> @@ -1108,7 +1108,7 @@ static void gem_transmit(CadenceGEMState *s)
>  
>              /* Last descriptor for this packet; hand the whole thing off */
>              if (tx_desc_get_last(desc)) {
> -                uint32_t desc_first[2];
> +                uint32_t desc_first[DESC_MAX_NUM_WORDS];
>  
>                  /* Modify the 1st descriptor of this packet to be owned by
>                   * the processor.
> diff --git a/include/hw/net/cadence_gem.h b/include/hw/net/cadence_gem.h
> index 633d564..b33ef65 100644
> --- a/include/hw/net/cadence_gem.h
> +++ b/include/hw/net/cadence_gem.h
> @@ -32,6 +32,9 @@
>  
>  #define CADENCE_GEM_MAXREG        (0x00000800 / 4) /* Last valid GEM address */
>  
> +/* Max number of words in a DMA descriptor.  */
> +#define DESC_MAX_NUM_WORDS              2
> +
>  #define MAX_PRIORITY_QUEUES             8
>  #define MAX_TYPE1_SCREENERS             16
>  #define MAX_TYPE2_SCREENERS             16
> @@ -74,7 +77,7 @@ typedef struct CadenceGEMState {
>  
>      uint8_t can_rx_state; /* Debug only */
>  
> -    uint32_t rx_desc[MAX_PRIORITY_QUEUES][2];
> +    uint32_t rx_desc[MAX_PRIORITY_QUEUES][DESC_MAX_NUM_WORDS];
>  
>      bool sar_active[4];
>  } CadenceGEMState;
> 

Re: [Qemu-devel] [PATCH v1 07/12] net: cadence_gem: Implement support for 64bit descriptor addresses

[Alistair]

On 10/03/2018 08:07 AM, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> 
> Implement support for 64bit descriptor addresses.
> 
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

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

Alistair

> ---
>   hw/net/cadence_gem.c | 47 +++++++++++++++++++++++++++++++++++++++--------
>   1 file changed, 39 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> index ab02515..f93cd8e 100644
> --- a/hw/net/cadence_gem.c
> +++ b/hw/net/cadence_gem.c
> @@ -153,6 +153,9 @@
>   #define GEM_RECEIVE_Q1_PTR              (0x00000480 / 4)
>   #define GEM_RECEIVE_Q7_PTR              (GEM_RECEIVE_Q1_PTR + 6)
>   
> +#define GEM_TBQPH                       (0x000004C8 / 4)
> +#define GEM_RBQPH                       (0x000004D4 / 4)
> +
>   #define GEM_INT_Q1_ENABLE               (0x00000600 / 4)
>   #define GEM_INT_Q7_ENABLE               (GEM_INT_Q1_ENABLE + 6)
>   
> @@ -832,18 +835,42 @@ static int get_queue_from_screen(CadenceGEMState *s, uint8_t *rxbuf_ptr,
>       return 0;
>   }
>   
> +static hwaddr gem_get_desc_addr(CadenceGEMState *s, bool tx, int q)
> +{
> +    hwaddr desc_addr = 0;
> +
> +    if (s->regs[GEM_DMACFG] & GEM_DMACFG_ADDR_64B) {
> +        desc_addr = s->regs[tx ? GEM_TBQPH : GEM_RBQPH];
> +    }
> +    desc_addr <<= 32;
> +    desc_addr |= tx ? s->tx_desc_addr[q] : s->rx_desc_addr[q];
> +    return desc_addr;
> +}
> +
> +static hwaddr gem_get_tx_desc_addr(CadenceGEMState *s, int q)
> +{
> +    return gem_get_desc_addr(s, true, q);
> +}
> +
> +static hwaddr gem_get_rx_desc_addr(CadenceGEMState *s, int q)
> +{
> +    return gem_get_desc_addr(s, false, q);
> +}
> +
>   static void gem_get_rx_desc(CadenceGEMState *s, int q)
>   {
> -    DB_PRINT("read descriptor 0x%x\n", (unsigned)s->rx_desc_addr[q]);
> +    hwaddr desc_addr = gem_get_rx_desc_addr(s, q);
> +
> +    DB_PRINT("read descriptor 0x%" HWADDR_PRIx "\n", desc_addr);
> +
>       /* read current descriptor */
> -    address_space_read(s->dma_as, s->rx_desc_addr[q], MEMTXATTRS_UNSPECIFIED,
> +    address_space_read(s->dma_as, desc_addr, MEMTXATTRS_UNSPECIFIED,
>                          (uint8_t *)s->rx_desc[q],
>                          sizeof(uint32_t) * gem_get_desc_len(s, true));
>   
>       /* Descriptor owned by software ? */
>       if (rx_desc_get_ownership(s->rx_desc[q]) == 1) {
> -        DB_PRINT("descriptor 0x%x owned by sw.\n",
> -                 (unsigned)s->rx_desc_addr[q]);
> +        DB_PRINT("descriptor 0x%" HWADDR_PRIx " owned by sw.\n", desc_addr);
>           s->regs[GEM_RXSTATUS] |= GEM_RXSTATUS_NOBUF;
>           s->regs[GEM_ISR] |= GEM_INT_RXUSED & ~(s->regs[GEM_IMR]);
>           /* Handle interrupt consequences */
> @@ -947,6 +974,8 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
>       q = get_queue_from_screen(s, rxbuf_ptr, rxbufsize);
>   
>       while (bytes_to_copy) {
> +        hwaddr desc_addr;
> +
>           /* Do nothing if receive is not enabled. */
>           if (!gem_can_receive(nc)) {
>               assert(!first_desc);
> @@ -994,7 +1023,8 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
>           }
>   
>           /* Descriptor write-back.  */
> -        address_space_write(s->dma_as, s->rx_desc_addr[q],
> +        desc_addr = gem_get_rx_desc_addr(s, q);
> +        address_space_write(s->dma_as, desc_addr,
>                               MEMTXATTRS_UNSPECIFIED,
>                               (uint8_t *)s->rx_desc[q],
>                               sizeof(uint32_t) * gem_get_desc_len(s, true));
> @@ -1098,7 +1128,7 @@ static void gem_transmit(CadenceGEMState *s)
>   
>       for (q = s->num_priority_queues - 1; q >= 0; q--) {
>           /* read current descriptor */
> -        packet_desc_addr = s->tx_desc_addr[q];
> +        packet_desc_addr = gem_get_tx_desc_addr(s, q);
>   
>           DB_PRINT("read descriptor 0x%" HWADDR_PRIx "\n", packet_desc_addr);
>           address_space_read(s->dma_as, packet_desc_addr,
> @@ -1144,16 +1174,17 @@ static void gem_transmit(CadenceGEMState *s)
>               /* Last descriptor for this packet; hand the whole thing off */
>               if (tx_desc_get_last(desc)) {
>                   uint32_t desc_first[DESC_MAX_NUM_WORDS];
> +                hwaddr desc_addr = gem_get_tx_desc_addr(s, q);
>   
>                   /* Modify the 1st descriptor of this packet to be owned by
>                    * the processor.
>                    */
> -                address_space_read(s->dma_as, s->tx_desc_addr[q],
> +                address_space_read(s->dma_as, desc_addr,
>                                      MEMTXATTRS_UNSPECIFIED,
>                                      (uint8_t *)desc_first,
>                                      sizeof(desc_first));
>                   tx_desc_set_used(desc_first);
> -                address_space_write(s->dma_as, s->tx_desc_addr[q],
> +                address_space_write(s->dma_as, desc_addr,
>                                     MEMTXATTRS_UNSPECIFIED,
>                                     (uint8_t *)desc_first,
>                                      sizeof(desc_first));
> 

Re: [Qemu-devel] [PATCH v1 06/12] net: cadence_gem: Add support for selecting the DMA MemoryRegion

[Philippe Mathieu-Daudé]

Hi Edgar,

On 03/10/2018 17:07, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> 
> Add support for selecting the Memory Region that the GEM
> will do DMA to.
> 
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>  hw/net/cadence_gem.c         | 63 ++++++++++++++++++++++++++++----------------
>  include/hw/net/cadence_gem.h |  2 ++
>  2 files changed, 43 insertions(+), 22 deletions(-)
> 
> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> index 759c1d7..ab02515 100644
> --- a/hw/net/cadence_gem.c
> +++ b/hw/net/cadence_gem.c
> @@ -28,6 +28,7 @@
>  #include "hw/net/cadence_gem.h"
>  #include "qapi/error.h"
>  #include "qemu/log.h"
> +#include "sysemu/dma.h"
>  #include "net/checksum.h"
>  
>  #ifdef CADENCE_GEM_ERR_DEBUG
> @@ -835,9 +836,9 @@ static void gem_get_rx_desc(CadenceGEMState *s, int q)
>  {
>      DB_PRINT("read descriptor 0x%x\n", (unsigned)s->rx_desc_addr[q]);
>      /* read current descriptor */
> -    cpu_physical_memory_read(s->rx_desc_addr[q],
> -                             (uint8_t *)s->rx_desc[q],
> -                             sizeof(uint32_t) * gem_get_desc_len(s, true));
> +    address_space_read(s->dma_as, s->rx_desc_addr[q], MEMTXATTRS_UNSPECIFIED,
> +                       (uint8_t *)s->rx_desc[q],
> +                       sizeof(uint32_t) * gem_get_desc_len(s, true));
>  
>      /* Descriptor owned by software ? */
>      if (rx_desc_get_ownership(s->rx_desc[q]) == 1) {
> @@ -956,10 +957,10 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
>                  rx_desc_get_buffer(s->rx_desc[q]));
>  
>          /* Copy packet data to emulated DMA buffer */
> -        cpu_physical_memory_write(rx_desc_get_buffer(s, s->rx_desc[q]) +
> +        address_space_write(s->dma_as, rx_desc_get_buffer(s, s->rx_desc[q]) +
>                                                                    rxbuf_offset,
> -                                  rxbuf_ptr,
> -                                  MIN(bytes_to_copy, rxbufsize));
> +                            MEMTXATTRS_UNSPECIFIED, rxbuf_ptr,
> +                            MIN(bytes_to_copy, rxbufsize));
>          rxbuf_ptr += MIN(bytes_to_copy, rxbufsize);
>          bytes_to_copy -= MIN(bytes_to_copy, rxbufsize);
>  
> @@ -993,9 +994,10 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
>          }
>  
>          /* Descriptor write-back.  */
> -        cpu_physical_memory_write(s->rx_desc_addr[q],
> -                                  (uint8_t *)s->rx_desc[q],
> -                                  sizeof(uint32_t) * gem_get_desc_len(s, true));
> +        address_space_write(s->dma_as, s->rx_desc_addr[q],
> +                            MEMTXATTRS_UNSPECIFIED,
> +                            (uint8_t *)s->rx_desc[q],
> +                            sizeof(uint32_t) * gem_get_desc_len(s, true));
>  
>          /* Next descriptor */
>          if (rx_desc_get_wrap(s->rx_desc[q])) {
> @@ -1099,9 +1101,9 @@ static void gem_transmit(CadenceGEMState *s)
>          packet_desc_addr = s->tx_desc_addr[q];
>  
>          DB_PRINT("read descriptor 0x%" HWADDR_PRIx "\n", packet_desc_addr);
> -        cpu_physical_memory_read(packet_desc_addr,
> -                                 (uint8_t *)desc,
> -                                 sizeof(uint32_t) * gem_get_desc_len(s, false));
> +        address_space_read(s->dma_as, packet_desc_addr,
> +                           MEMTXATTRS_UNSPECIFIED, (uint8_t *)desc,
> +                           sizeof(uint32_t) * gem_get_desc_len(s, false));
>          /* Handle all descriptors owned by hardware */
>          while (tx_desc_get_used(desc) == 0) {
>  
> @@ -1133,8 +1135,9 @@ static void gem_transmit(CadenceGEMState *s)
>              /* Gather this fragment of the packet from "dma memory" to our
>               * contig buffer.
>               */
> -            cpu_physical_memory_read(tx_desc_get_buffer(s, desc), p,
> -                                     tx_desc_get_length(desc));
> +            address_space_read(s->dma_as, tx_desc_get_buffer(s, desc),
> +                               MEMTXATTRS_UNSPECIFIED,
> +                               p, tx_desc_get_length(desc));
>              p += tx_desc_get_length(desc);
>              total_bytes += tx_desc_get_length(desc);
>  
> @@ -1145,13 +1148,15 @@ static void gem_transmit(CadenceGEMState *s)
>                  /* Modify the 1st descriptor of this packet to be owned by
>                   * the processor.
>                   */
> -                cpu_physical_memory_read(s->tx_desc_addr[q],
> -                                         (uint8_t *)desc_first,
> -                                         sizeof(desc_first));
> +                address_space_read(s->dma_as, s->tx_desc_addr[q],
> +                                   MEMTXATTRS_UNSPECIFIED,
> +                                   (uint8_t *)desc_first,
> +                                   sizeof(desc_first));
>                  tx_desc_set_used(desc_first);
> -                cpu_physical_memory_write(s->tx_desc_addr[q],
> -                                          (uint8_t *)desc_first,
> -                                          sizeof(desc_first));
> +                address_space_write(s->dma_as, s->tx_desc_addr[q],
> +                                  MEMTXATTRS_UNSPECIFIED,
> +                                  (uint8_t *)desc_first,
> +                                   sizeof(desc_first));
>                  /* Advance the hardware current descriptor past this packet */
>                  if (tx_desc_get_wrap(desc)) {
>                      s->tx_desc_addr[q] = s->regs[GEM_TXQBASE];
> @@ -1204,8 +1209,9 @@ static void gem_transmit(CadenceGEMState *s)
>                  packet_desc_addr += 4 * gem_get_desc_len(s, false);
>              }
>              DB_PRINT("read descriptor 0x%" HWADDR_PRIx "\n", packet_desc_addr);
> -            cpu_physical_memory_read(packet_desc_addr, (uint8_t *)desc,
> -                                sizeof(uint32_t) * gem_get_desc_len(s, false));
> +            address_space_read(s->dma_as, packet_desc_addr,
> +                              MEMTXATTRS_UNSPECIFIED, (uint8_t *)desc,
> +                              sizeof(uint32_t) * gem_get_desc_len(s, false));
>          }
>  
>          if (tx_desc_get_used(desc)) {
> @@ -1500,6 +1506,13 @@ static void gem_realize(DeviceState *dev, Error **errp)
>      CadenceGEMState *s = CADENCE_GEM(dev);
>      int i;
>  
> +    if (s->dma_mr) {
> +        s->dma_as = g_malloc0(sizeof(AddressSpace));
> +        address_space_init(s->dma_as, s->dma_mr, NULL);
> +    } else {
> +        s->dma_as = &address_space_memory;
> +    }

This is not the first time I see this if() block.

Should we consider refactor it?

Anyway:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>


> +
>      if (s->num_priority_queues == 0 ||
>          s->num_priority_queues > MAX_PRIORITY_QUEUES) {
>          error_setg(errp, "Invalid num-priority-queues value: %" PRIx8,
> @@ -1537,6 +1550,12 @@ static void gem_init(Object *obj)
>                            "enet", sizeof(s->regs));
>  
>      sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->iomem);
> +
> +    object_property_add_link(obj, "dma", TYPE_MEMORY_REGION,
> +                             (Object **)&s->dma_mr,
> +                             qdev_prop_allow_set_link_before_realize,
> +                             OBJ_PROP_LINK_STRONG,
> +                             &error_abort);
>  }
>  
>  static const VMStateDescription vmstate_cadence_gem = {
> diff --git a/include/hw/net/cadence_gem.h b/include/hw/net/cadence_gem.h
> index 00dbf4f..c8f0751 100644
> --- a/include/hw/net/cadence_gem.h
> +++ b/include/hw/net/cadence_gem.h
> @@ -45,6 +45,8 @@ typedef struct CadenceGEMState {
>  
>      /*< public >*/
>      MemoryRegion iomem;
> +    MemoryRegion *dma_mr;
> +    AddressSpace *dma_as;
>      NICState *nic;
>      NICConf conf;
>      qemu_irq irq[MAX_PRIORITY_QUEUES];
> 

Re: [Qemu-devel] [PATCH v1 11/12] hw/arm: versal: Add a model of Xilinx Versal SoC

[Philippe Mathieu-Daudé]

On 03/10/2018 17:07, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> 
> Add a model of Xilinx Versal SoC.
> 
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>  default-configs/aarch64-softmmu.mak |   1 +
>  hw/arm/Makefile.objs                |   1 +
>  hw/arm/xlnx-versal.c                | 339 ++++++++++++++++++++++++++++++++++++
>  include/hw/arm/xlnx-versal.h        | 122 +++++++++++++
>  4 files changed, 463 insertions(+)
>  create mode 100644 hw/arm/xlnx-versal.c
>  create mode 100644 include/hw/arm/xlnx-versal.h
> 
> diff --git a/default-configs/aarch64-softmmu.mak b/default-configs/aarch64-softmmu.mak
> index 6f790f0..4ea9add 100644
> --- a/default-configs/aarch64-softmmu.mak
> +++ b/default-configs/aarch64-softmmu.mak
> @@ -8,4 +8,5 @@ CONFIG_DDC=y
>  CONFIG_DPCD=y
>  CONFIG_XLNX_ZYNQMP=y
>  CONFIG_XLNX_ZYNQMP_ARM=y
> +CONFIG_XLNX_VERSAL=y
>  CONFIG_ARM_SMMUV3=y
> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
> index 5f88062..ec21d9b 100644
> --- a/hw/arm/Makefile.objs
> +++ b/hw/arm/Makefile.objs
> @@ -26,6 +26,7 @@ obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10.o cubieboard.o
>  obj-$(CONFIG_RASPI) += bcm2835_peripherals.o bcm2836.o raspi.o
>  obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o
>  obj-$(CONFIG_XLNX_ZYNQMP_ARM) += xlnx-zynqmp.o xlnx-zcu102.o
> +obj-$(CONFIG_XLNX_VERSAL) += xlnx-versal.o
>  obj-$(CONFIG_FSL_IMX25) += fsl-imx25.o imx25_pdk.o
>  obj-$(CONFIG_FSL_IMX31) += fsl-imx31.o kzm.o
>  obj-$(CONFIG_FSL_IMX6) += fsl-imx6.o sabrelite.o
> diff --git a/hw/arm/xlnx-versal.c b/hw/arm/xlnx-versal.c
> new file mode 100644
> index 0000000..c12fc85
> --- /dev/null
> +++ b/hw/arm/xlnx-versal.c
> @@ -0,0 +1,339 @@
> +/*
> + * Xilinx Versal SoC model.
> + *
> + * Copyright (c) 2018 Xilinx Inc.
> + * Written by Edgar E. Iglesias
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 or
> + * (at your option) any later version.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "qemu-common.h"
> +#include "qemu/log.h"
> +#include "hw/sysbus.h"
> +#include "net/net.h"
> +#include "sysemu/sysemu.h"
> +#include "sysemu/kvm.h"
> +#include "hw/arm/arm.h"
> +#include "kvm_arm.h"
> +#include "hw/misc/unimp.h"
> +#include "hw/intc/arm_gicv3_common.h"
> +#include "hw/arm/xlnx-versal.h"
> +
> +#define XLNX_VERSAL_ACPU_TYPE "cortex-a72" "-" TYPE_ARM_CPU
> +#define GEM_REVISION        0x40070106
> +
> +static void versal_create_apu_cpus(Versal *s, Error **errp)
> +{
> +    int i;
> +
> +    for (i = 0; i < ARRAY_SIZE(s->fpd.apu.cpu); i++) {
> +        Object *obj;
> +        char *name;
> +
> +        obj = object_new(XLNX_VERSAL_ACPU_TYPE);
> +        if (!obj) {
> +            /* Secondary CPUs start in PSCI powered-down state */
> +            error_setg(errp, "Unable to create apu.cpu[%d] of type %s",
> +                       i, XLNX_VERSAL_ACPU_TYPE);
> +            return;
> +        }
> +
> +        name = g_strdup_printf("apu-cpu[%d]", i);
> +        object_property_add_child(OBJECT(s), name, obj, &error_fatal);
> +        g_free(name);
> +
> +        object_property_set_int(obj, s->cfg.psci_conduit,
> +                                "psci-conduit", &error_abort);
> +        if (i) {
> +            object_property_set_bool(obj, true,
> +                                     "start-powered-off", &error_abort);
> +        }
> +
> +        object_property_set_int(obj, ARRAY_SIZE(s->fpd.apu.cpu),
> +                                "core-count", &error_abort);
> +        object_property_set_link(obj, OBJECT(&s->fpd.apu.mr), "memory",
> +                                 &error_abort);
> +        object_property_set_bool(obj, true, "realized", &error_fatal);
> +        s->fpd.apu.cpu[i] = ARM_CPU(obj);
> +    }
> +}
> +
> +static void versal_create_apu_gic(Versal *s, qemu_irq *pic, Error **errp)
> +{
> +    static const uint64_t addrs[] = {
> +        MM_GIC_APU_DIST_MAIN,
> +        MM_GIC_APU_REDIST_0
> +    };
> +    SysBusDevice *gicbusdev;
> +    DeviceState *gicdev;
> +    int nr_apu_cpus = ARRAY_SIZE(s->fpd.apu.cpu);
> +    int i;
> +
> +    sysbus_init_child_obj(OBJECT(s), "apu-gic",
> +                          &s->fpd.apu.gic, sizeof(s->fpd.apu.gic),
> +                          gicv3_class_name());
> +    gicbusdev = SYS_BUS_DEVICE(&s->fpd.apu.gic);
> +    gicdev = DEVICE(&s->fpd.apu.gic);
> +    qdev_prop_set_uint32(gicdev, "revision", 3);
> +    qdev_prop_set_uint32(gicdev, "num-cpu", 2);
> +    qdev_prop_set_uint32(gicdev, "num-irq", XLNX_VERSAL_NR_IRQS + 32);
> +    qdev_prop_set_uint32(gicdev, "len-redist-region-count", 1);
> +    qdev_prop_set_uint32(gicdev, "redist-region-count[0]", 2);
> +    if (!kvm_irqchip_in_kernel()) {
> +        qdev_prop_set_bit(gicdev, "has-security-extensions", true);
> +    }
> +
> +    object_property_set_bool(OBJECT(&s->fpd.apu.gic), true, "realized", errp);
> +
> +    for (i = 0; i < ARRAY_SIZE(addrs); i++) {
> +        MemoryRegion *mr;
> +
> +        mr = sysbus_mmio_get_region(gicbusdev, i);
> +        memory_region_add_subregion(&s->fpd.apu.mr, addrs[i], mr);
> +    }
> +
> +    for (i = 0; i < nr_apu_cpus; i++) {
> +        DeviceState *cpudev = DEVICE(s->fpd.apu.cpu[i]);
> +        int ppibase = XLNX_VERSAL_NR_IRQS + i * GIC_INTERNAL + GIC_NR_SGIS;
> +        qemu_irq maint_irq;
> +        int ti;
> +        /* Mapping from the output timer irq lines from the CPU to the
> +         * GIC PPI inputs we use for the virt board.
> +         */
> +        const int timer_irq[] = {
> +            [GTIMER_PHYS] = VERSAL_TIMER_NS_EL1_IRQ,
> +            [GTIMER_VIRT] = VERSAL_TIMER_VIRT_IRQ,
> +            [GTIMER_HYP]  = VERSAL_TIMER_NS_EL2_IRQ,
> +            [GTIMER_SEC]  = VERSAL_TIMER_S_EL1_IRQ,
> +        };
> +
> +        for (ti = 0; ti < ARRAY_SIZE(timer_irq); ti++) {
> +            qdev_connect_gpio_out(cpudev, ti,
> +                                  qdev_get_gpio_in(gicdev,
> +                                                   ppibase + timer_irq[ti]));
> +        }
> +        maint_irq = qdev_get_gpio_in(gicdev,
> +                                        ppibase + VERSAL_GIC_MAINT_IRQ);
> +        qdev_connect_gpio_out_named(cpudev, "gicv3-maintenance-interrupt",
> +                                    0, maint_irq);
> +        sysbus_connect_irq(gicbusdev, i, qdev_get_gpio_in(cpudev, ARM_CPU_IRQ));
> +        sysbus_connect_irq(gicbusdev, i + nr_apu_cpus,
> +                           qdev_get_gpio_in(cpudev, ARM_CPU_FIQ));
> +        sysbus_connect_irq(gicbusdev, i + 2 * nr_apu_cpus,
> +                           qdev_get_gpio_in(cpudev, ARM_CPU_VIRQ));
> +        sysbus_connect_irq(gicbusdev, i + 3 * nr_apu_cpus,
> +                           qdev_get_gpio_in(cpudev, ARM_CPU_VFIQ));
> +    }
> +
> +    for (i = 0; i < XLNX_VERSAL_NR_IRQS; i++) {
> +        pic[i] = qdev_get_gpio_in(gicdev, i);
> +    }
> +}
> +
> +static void versal_create_uarts(Versal *s, qemu_irq *pic)
> +{
> +    int i;
> +
> +    for (i = 0; i < ARRAY_SIZE(s->lpd.iou.uart); i++) {
> +        static const int irqs[] = { VERSAL_UART0_IRQ_0, VERSAL_UART1_IRQ_0};
> +        static const uint64_t addrs[] = { MM_UART0, MM_UART1 };
> +        char *name = g_strdup_printf("uart%d", i);
> +        DeviceState *dev;
> +        MemoryRegion *mr;
> +
> +        dev = qdev_create(NULL, "pl011");
> +        s->lpd.iou.uart[i] = SYS_BUS_DEVICE(dev);
> +        qdev_prop_set_chr(dev, "chardev", serial_hd(i));
> +        object_property_add_child(OBJECT(s), name, OBJECT(dev), &error_fatal);
> +        qdev_init_nofail(dev);
> +
> +        mr = sysbus_mmio_get_region(s->lpd.iou.uart[i], 0);
> +        memory_region_add_subregion(&s->mr_ps, addrs[i], mr);
> +
> +        sysbus_connect_irq(s->lpd.iou.uart[i], 0, pic[irqs[i]]);
> +        g_free(name);
> +    }
> +}
> +
> +static void versal_create_gems(Versal *s, qemu_irq *pic)
> +{
> +    int i;
> +
> +    for (i = 0; i < ARRAY_SIZE(s->lpd.iou.gem); i++) {
> +        static const int irqs[] = { VERSAL_GEM0_IRQ_0, VERSAL_GEM1_IRQ_0};
> +        static const uint64_t addrs[] = { MM_GEM0, MM_GEM1 };
> +        char *name = g_strdup_printf("gem%d", i);
> +        NICInfo *nd = &nd_table[i];
> +        DeviceState *dev;
> +        MemoryRegion *mr;
> +
> +        dev = qdev_create(NULL, "cadence_gem");
> +        s->lpd.iou.gem[i] = SYS_BUS_DEVICE(dev);
> +        object_property_add_child(OBJECT(s), name, OBJECT(dev), &error_fatal);
> +        if (nd->used) {
> +            qemu_check_nic_model(nd, "cadence_gem");
> +            qdev_set_nic_properties(dev, nd);
> +        }
> +        object_property_set_int(OBJECT(s->lpd.iou.gem[i]),
> +                                2, "num-priority-queues",
> +                                &error_abort);
> +        object_property_set_link(OBJECT(s->lpd.iou.gem[i]),
> +                                 OBJECT(&s->mr_ps), "dma",
> +                                 &error_abort);
> +        qdev_init_nofail(dev);
> +
> +        mr = sysbus_mmio_get_region(s->lpd.iou.gem[i], 0);
> +        memory_region_add_subregion(&s->mr_ps, addrs[i], mr);
> +
> +        sysbus_connect_irq(s->lpd.iou.gem[i], 0, pic[irqs[i]]);
> +        g_free(name);
> +    }
> +}
> +
> +/* This takes the board allocated linear DDR memory and creates aliases
> + * for each split DDR range/apperture on the Versal address map.
> + */
> +static void versal_map_ddr(Versal *s)
> +{
> +    uint64_t size = memory_region_size(s->cfg.mr_ddr);
> +    /* Describes the various split DDR access regions.  */
> +    static const struct {
> +        uint64_t base;
> +        uint64_t size;
> +    } addr_ranges[] = {
> +        { MM_TOP_DDR, MM_TOP_DDR_SIZE },
> +        { MM_TOP_DDR_2, MM_TOP_DDR_2_SIZE },
> +        { MM_TOP_DDR_3, MM_TOP_DDR_3_SIZE },
> +        { MM_TOP_DDR_4, MM_TOP_DDR_4_SIZE }
> +    };
> +    uint64_t offset = 0;
> +    int i;
> +
> +    assert(ARRAY_SIZE(addr_ranges) == ARRAY_SIZE(s->noc.mr_ddr_ranges));
> +    for (i = 0; i < ARRAY_SIZE(addr_ranges) && size; i++) {
> +        char *name;
> +        uint64_t mapsize;
> +
> +        mapsize = size < addr_ranges[i].size ? size : addr_ranges[i].size;
> +        name = g_strdup_printf("noc-ddr-range%d", i);
> +        /* Create the MR alias.  */
> +        memory_region_init_alias(&s->noc.mr_ddr_ranges[i], OBJECT(s),
> +                                 name, s->cfg.mr_ddr,
> +                                 offset, mapsize);
> +
> +        /* Map it onto the NoC MR.  */
> +        memory_region_add_subregion(&s->mr_ps, addr_ranges[i].base,
> +                                    &s->noc.mr_ddr_ranges[i]);
> +        offset += mapsize;
> +        size -= mapsize;
> +        g_free(name);
> +    }
> +}
> +
> +static void versal_unimp_area(Versal *s, const char *name,
> +                                MemoryRegion *mr,
> +                                hwaddr base, hwaddr size)
> +{
> +    DeviceState *dev = qdev_create(NULL, TYPE_UNIMPLEMENTED_DEVICE);
> +    MemoryRegion *mr_dev;
> +
> +    qdev_prop_set_string(dev, "name", name);
> +    qdev_prop_set_uint64(dev, "size", size);
> +    object_property_add_child(OBJECT(s), name, OBJECT(dev), &error_fatal);
> +    qdev_init_nofail(dev);
> +
> +    mr_dev = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0);
> +    memory_region_add_subregion(mr, base, mr_dev);
> +}
> +
> +static void versal_unimp(Versal *s)
> +{
> +    versal_unimp_area(s, "psm", &s->mr_ps,
> +                        MM_PSM_START, MM_PSM_END - MM_PSM_START);
> +    versal_unimp_area(s, "crl", &s->mr_ps,
> +                        MM_CRL, MM_CRL_SIZE);
> +    versal_unimp_area(s, "crf", &s->mr_ps,
> +                        MM_FPD_CRF, MM_FPD_CRF_SIZE);
> +    versal_unimp_area(s, "iou-scntr", &s->mr_ps,
> +                        MM_IOU_SCNTR, MM_IOU_SCNTR_SIZE);
> +    versal_unimp_area(s, "iou-scntr-seucre", &s->mr_ps,
> +                        MM_IOU_SCNTRS, MM_IOU_SCNTRS_SIZE);
> +}
> +
> +static void versal_realize(DeviceState *dev, Error **errp)
> +{
> +    Versal *s = XLNX_VERSAL(dev);
> +    qemu_irq pic[XLNX_VERSAL_NR_IRQS];
> +
> +    versal_create_apu_cpus(s, errp);
> +    versal_create_apu_gic(s, pic, errp);
> +    versal_create_uarts(s, pic);
> +    versal_create_gems(s, pic);
> +    versal_map_ddr(s);
> +    versal_unimp(s);
> +
> +    /* Create the OCM.  */
> +    memory_region_init_ram(&s->lpd.mr_ocm, OBJECT(s), "ocm",
> +                           MM_OCM_SIZE, &error_fatal);
> +
> +    memory_region_add_subregion_overlap(&s->mr_ps, MM_OCM, &s->lpd.mr_ocm, 0);
> +    memory_region_add_subregion_overlap(&s->fpd.apu.mr, 0, &s->mr_ps, 0);
> +}
> +
> +static void versal_init(Object *obj)
> +{
> +    Versal *s = XLNX_VERSAL(obj);
> +
> +    memory_region_init(&s->fpd.apu.mr, obj, "mr-apu", UINT64_MAX);
> +    memory_region_init(&s->mr_ps, obj, "mr-ps-switch", UINT64_MAX);
> +}
> +
> +static const VMStateDescription versal_vmstate = {
> +    .name = "xlnx-ve",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        /* FIXME.  */
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static Property versal_properties[] = {
> +    DEFINE_PROP_LINK("ddr", Versal, cfg.mr_ddr, TYPE_MEMORY_REGION,
> +                     MemoryRegion *),
> +    DEFINE_PROP_UINT32("psci-conduit", Versal, cfg.psci_conduit, 0),
> +    DEFINE_PROP_END_OF_LIST()
> +};
> +
> +static void versal_reset(DeviceState *dev)
> +{
> +}
> +
> +static void versal_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = versal_realize;
> +    dc->vmsd = &versal_vmstate;
> +    dc->props = versal_properties;
> +    dc->reset = versal_reset;
> +}
> +
> +static const TypeInfo versal_info = {
> +    .name = TYPE_XLNX_VERSAL,
> +    .parent = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(Versal),
> +    .instance_init = versal_init,
> +    .class_init = versal_class_init,
> +};
> +
> +static void versal_register_types(void)
> +{
> +    type_register_static(&versal_info);
> +}
> +
> +type_init(versal_register_types);
> diff --git a/include/hw/arm/xlnx-versal.h b/include/hw/arm/xlnx-versal.h
> new file mode 100644
> index 0000000..9da621e
> --- /dev/null
> +++ b/include/hw/arm/xlnx-versal.h
> @@ -0,0 +1,122 @@
> +/*
> + * Model of the Xilinx Versal
> + *
> + * Copyright (c) 2018 Xilinx Inc.
> + * Written by Edgar E. Iglesias
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 or
> + * (at your option) any later version.
> + */
> +
> +#ifndef XLNX_VERSAL_H
> +#define XLNX_VERSAL_H
> +
> +#include "hw/sysbus.h"
> +#include "hw/arm/arm.h"
> +#include "hw/intc/arm_gicv3.h"
> +
> +#define TYPE_XLNX_VERSAL "xlnx-versal"
> +#define XLNX_VERSAL(obj) OBJECT_CHECK(Versal, (obj), TYPE_XLNX_VERSAL)
> +
> +#define XLNX_VERSAL_NR_ACPUS   2
> +#define XLNX_VERSAL_NR_UARTS   2
> +#define XLNX_VERSAL_NR_GEMS    2
> +#define XLNX_VERSAL_NR_IRQS    256
> +
> +typedef struct Versal {
> +    /*< private >*/
> +    SysBusDevice parent_obj;
> +
> +    /*< public >*/
> +    struct {
> +        struct {
> +            MemoryRegion mr;
> +            ARMCPU *cpu[XLNX_VERSAL_NR_ACPUS];
> +            GICv3State gic;
> +        } apu;
> +    } fpd;
> +
> +    MemoryRegion mr_ps;
> +
> +    struct {
> +        /* 4 ranges to access DDR.  */
> +        MemoryRegion mr_ddr_ranges[4];
> +    } noc;
> +
> +    struct {
> +        MemoryRegion mr_ocm;
> +
> +        struct {
> +            SysBusDevice *uart[XLNX_VERSAL_NR_UARTS];
> +            SysBusDevice *gem[XLNX_VERSAL_NR_GEMS];
> +        } iou;
> +    } lpd;

Amazing how a so complex SoC is modelled that clear...

You won my "clearest SoC model of the year" prize.

> +
> +    struct {
> +        MemoryRegion *mr_ddr;
> +        uint32_t psci_conduit;
> +    } cfg;
> +} Versal;
> +
> +/* Memory-map and IRQ definitions. Copied a subset from
> + * auto-generated files.  */
> +
> +#define VERSAL_GIC_MAINT_IRQ        9
> +#define VERSAL_TIMER_VIRT_IRQ       11
> +#define VERSAL_TIMER_S_EL1_IRQ      13
> +#define VERSAL_TIMER_NS_EL1_IRQ     14
> +#define VERSAL_TIMER_NS_EL2_IRQ     10
> +
> +#define VERSAL_UART0_IRQ_0         18
> +#define VERSAL_UART1_IRQ_0         19
> +#define VERSAL_GEM0_IRQ_0          56
> +#define VERSAL_GEM0_WAKE_IRQ_0     57
> +#define VERSAL_GEM1_IRQ_0          58
> +#define VERSAL_GEM1_WAKE_IRQ_0     59
> +
> +/* Architecturally eserved IRQs suitable for virtualization.  */
> +#define VERSAL_RSVD_HIGH_IRQ_FIRST 160
> +#define VERSAL_RSVD_HIGH_IRQ_LAST  255
> +
> +#define MM_TOP_RSVD                 0xa0000000U
> +#define MM_TOP_RSVD_SIZE            0x4000000
> +#define MM_GIC_APU_DIST_MAIN        0xf9000000U
> +#define MM_GIC_APU_DIST_MAIN_SIZE   0x10000
> +#define MM_GIC_APU_REDIST_0         0xf9080000U
> +#define MM_GIC_APU_REDIST_0_SIZE    0x80000
> +
> +#define MM_UART0                    0xff000000U
> +#define MM_UART0_SIZE               0x10000
> +#define MM_UART1                    0xff010000U
> +#define MM_UART1_SIZE               0x10000
> +
> +#define MM_GEM0                     0xff0c0000U
> +#define MM_GEM0_SIZE                0x10000
> +#define MM_GEM1                     0xff0d0000U
> +#define MM_GEM1_SIZE                0x10000
> +
> +#define MM_OCM                      0xfffc0000U
> +#define MM_OCM_SIZE                 0x40000
> +
> +#define MM_TOP_DDR                  0x0
> +#define MM_TOP_DDR_SIZE             0x80000000U
> +#define MM_TOP_DDR_2                0x800000000ULL
> +#define MM_TOP_DDR_2_SIZE           0x800000000ULL
> +#define MM_TOP_DDR_3                0xc000000000ULL
> +#define MM_TOP_DDR_3_SIZE           0x4000000000ULL
> +#define MM_TOP_DDR_4                0x10000000000ULL
> +#define MM_TOP_DDR_4_SIZE           0xb780000000ULL
> +
> +#define MM_PSM_START                0xffc80000U
> +#define MM_PSM_END                  0xffcf0000U
> +
> +#define MM_CRL                      0xff5e0000U
> +#define MM_CRL_SIZE                 0x300000
> +#define MM_IOU_SCNTR                0xff130000U
> +#define MM_IOU_SCNTR_SIZE           0x10000
> +#define MM_IOU_SCNTRS               0xff140000U
> +#define MM_IOU_SCNTRS_SIZE          0x10000
> +#define MM_FPD_CRF                  0xfd1a0000U
> +#define MM_FPD_CRF_SIZE             0x140000
> +#endif
> 

Re: [Qemu-devel] [PATCH v1 06/12] net: cadence_gem: Add support for selecting the DMA MemoryRegion

[Peter Maydell]

On 3 October 2018 at 16:07, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Add support for selecting the Memory Region that the GEM
> will do DMA to.
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---


> @@ -1500,6 +1506,13 @@ static void gem_realize(DeviceState *dev, Error **errp)
>      CadenceGEMState *s = CADENCE_GEM(dev);
>      int i;
>
> +    if (s->dma_mr) {
> +        s->dma_as = g_malloc0(sizeof(AddressSpace));
> +        address_space_init(s->dma_as, s->dma_mr, NULL);

Why not just have the CadenceGEMState embed the AddressSpace

    AddressSpace dma_as;

rather than doing a separate memory allocation here?

> +    } else {
> +        s->dma_as = &address_space_memory;
> +    }

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 06/12] net: cadence_gem: Add support for selecting the DMA MemoryRegion

[Peter Maydell]

On 6 October 2018 at 00:14, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> Hi Edgar,
>
> On 03/10/2018 17:07, Edgar E. Iglesias wrote:
>> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>>
>> Add support for selecting the Memory Region that the GEM
>> will do DMA to.

>> @@ -1500,6 +1506,13 @@ static void gem_realize(DeviceState *dev, Error **errp)
>>      CadenceGEMState *s = CADENCE_GEM(dev);
>>      int i;
>>
>> +    if (s->dma_mr) {
>> +        s->dma_as = g_malloc0(sizeof(AddressSpace));
>> +        address_space_init(s->dma_as, s->dma_mr, NULL);
>> +    } else {
>> +        s->dma_as = &address_space_memory;
>> +    }
>
> This is not the first time I see this if() block.
>
> Should we consider refactor it?

Given that there are only three users of the device in the tree,
I think the nicest cleanup would be to make those callers pass
in a suitable MemoryRegion (which could just be the system memory
MR), and make the "dma" property of the device mandatory to set.
We can do that as followup, though.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 06/12] net: cadence_gem: Add support for selecting the DMA MemoryRegion

[Peter Maydell]

On 3 October 2018 at 16:07, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Add support for selecting the Memory Region that the GEM
> will do DMA to.
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>  hw/net/cadence_gem.c         | 63 ++++++++++++++++++++++++++++----------------
>  include/hw/net/cadence_gem.h |  2 ++
>  2 files changed, 43 insertions(+), 22 deletions(-)
>
> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> index 759c1d7..ab02515 100644
> --- a/hw/net/cadence_gem.c
> +++ b/hw/net/cadence_gem.c
> @@ -28,6 +28,7 @@
>  #include "hw/net/cadence_gem.h"
>  #include "qapi/error.h"
>  #include "qemu/log.h"
> +#include "sysemu/dma.h"
>  #include "net/checksum.h"
>
>  #ifdef CADENCE_GEM_ERR_DEBUG
> @@ -835,9 +836,9 @@ static void gem_get_rx_desc(CadenceGEMState *s, int q)
>  {
>      DB_PRINT("read descriptor 0x%x\n", (unsigned)s->rx_desc_addr[q]);
>      /* read current descriptor */
> -    cpu_physical_memory_read(s->rx_desc_addr[q],
> -                             (uint8_t *)s->rx_desc[q],
> -                             sizeof(uint32_t) * gem_get_desc_len(s, true));
> +    address_space_read(s->dma_as, s->rx_desc_addr[q], MEMTXATTRS_UNSPECIFIED,
> +                       (uint8_t *)s->rx_desc[q],
> +                       sizeof(uint32_t) * gem_get_desc_len(s, true));

At some point you might want to add support for handling "descriptor
read/write failed", incidentally: address_space_read/write return a
MemTxResult which you can check for != MEMTX_OK.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 09/12] target-arm: powerctl: Enable HVC when starting CPUs to EL2

[Peter Maydell]

On 3 October 2018 at 16:07, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> When QEMU provides the equivalent of the EL3 firmware, we
> need to enable HVCs in scr_el3 when turning on CPUs that
> target EL2.
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>  target/arm/arm-powerctl.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/target/arm/arm-powerctl.c b/target/arm/arm-powerctl.c
> index ce55eeb..54f2974 100644
> --- a/target/arm/arm-powerctl.c
> +++ b/target/arm/arm-powerctl.c
> @@ -63,6 +63,7 @@ static void arm_set_cpu_on_async_work(CPUState *target_cpu_state,
>      struct CpuOnInfo *info = (struct CpuOnInfo *) data.host_ptr;
>
>      /* Initialize the cpu we are turning on */
> +    qemu_log("CPU%d reset\n", target_cpu_state->cpu_index);

qemu_log logging should always be masked to a particular
log kind (either via qemu_log_mask() or by explicit check on
the log level). If this was accidentally left-in debug, you could
just drop it :-)

>      cpu_reset(target_cpu_state);
>      target_cpu_state->halted = 0;
>
> @@ -103,6 +104,16 @@ static void arm_set_cpu_on_async_work(CPUState *target_cpu_state,
>      } else {
>          /* Processor is not in secure mode */
>          target_cpu->env.cp15.scr_el3 |= SCR_NS;
> +
> +        /*
> +         * If QEMU is providing the equivalent of EL3 firmware, then we need
> +         * to make sure a CPU targeting EL2 comes out of reset with a
> +         * functional HVC insn.
> +         */
> +        if (arm_feature(&target_cpu->env, ARM_FEATURE_EL3)
> +            && info->target_el == 2) {
> +            target_cpu->env.cp15.scr_el3 |= SCR_HCE;
> +        }
>      }

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
This is definitely required for PSCI and I think it makes sense
in other cases that end up in this powerctl code with EL3 set.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 10/12] target/arm: Add the Cortex-A72

[Peter Maydell]

On 3 October 2018 at 16:07, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Add the ARM Cortex-A72.
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>  target/arm/cpu64.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 59 insertions(+)
>
> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> index 800bff7..02e500b 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -218,6 +218,64 @@ static void aarch64_a53_initfn(Object *obj)
>      define_arm_cp_regs(cpu, cortex_a57_a53_cp_reginfo);
>  }
>
> +static void aarch64_a72_initfn(Object *obj)
> +{
> +    ARMCPU *cpu = ARM_CPU(obj);
> +
> +    cpu->dtb_compatible = "arm,cortex-a72";
> +    set_feature(&cpu->env, ARM_FEATURE_V8);
> +    set_feature(&cpu->env, ARM_FEATURE_VFP4);
> +    set_feature(&cpu->env, ARM_FEATURE_NEON);
> +    set_feature(&cpu->env, ARM_FEATURE_GENERIC_TIMER);
> +    set_feature(&cpu->env, ARM_FEATURE_AARCH64);
> +    set_feature(&cpu->env, ARM_FEATURE_CBAR_RO);
> +    set_feature(&cpu->env, ARM_FEATURE_V8_AES);
> +    set_feature(&cpu->env, ARM_FEATURE_V8_SHA1);
> +    set_feature(&cpu->env, ARM_FEATURE_V8_SHA256);
> +    set_feature(&cpu->env, ARM_FEATURE_V8_PMULL);
> +    set_feature(&cpu->env, ARM_FEATURE_CRC);
> +    set_feature(&cpu->env, ARM_FEATURE_EL2);
> +    set_feature(&cpu->env, ARM_FEATURE_EL3);

No ARM_FEATURE_PMU ?

> +    cpu->midr = 0x410fd083;
> +    cpu->revidr = 0x00000000;
> +    cpu->reset_fpsid = 0x41034080;
> +    cpu->mvfr0 = 0x10110222;
> +    cpu->mvfr1 = 0x12111111;
> +    cpu->mvfr2 = 0x00000043;
> +    cpu->ctr = 0x8444c004;
> +    cpu->reset_sctlr = 0x00c50838;

Do you happen to have the hardware to hand to check what the
top 4 bits of the reset value of SCTLR_ELx are? I think they
should be 0x3 -- the Arm ARM says that [29:28] are RES1 (as
does the A72 TRM, though its top level summary table lists
0x00c50838 as the reset value for some of the SCTLR_ELx.)

QEMU may have the wrong value for A53/A57 here too, I suspect.

> +    cpu->id_pfr0 = 0x00000131;
> +    cpu->id_pfr1 = 0x00011011;
> +    cpu->id_dfr0 = 0x03010066;
> +    cpu->id_afr0 = 0x00000000;
> +    cpu->id_mmfr0 = 0x10201105;
> +    cpu->id_mmfr1 = 0x40000000;
> +    cpu->id_mmfr2 = 0x01260000;
> +    cpu->id_mmfr3 = 0x02102211;
> +    cpu->id_isar0 = 0x02101110;
> +    cpu->id_isar1 = 0x13112111;
> +    cpu->id_isar2 = 0x21232042;
> +    cpu->id_isar3 = 0x01112131;
> +    cpu->id_isar4 = 0x00011142;
> +    cpu->id_isar5 = 0x00011121;
> +    cpu->id_aa64pfr0 = 0x00002222;
> +    cpu->id_aa64dfr0 = 0x10305106;
> +    cpu->pmceid0 = 0x00000000;
> +    cpu->pmceid1 = 0x00000000;
> +    cpu->id_aa64isar0 = 0x00011120;
> +    cpu->id_aa64mmfr0 = 0x00001124;
> +    cpu->dbgdidr = 0x3516d000;
> +    cpu->clidr = 0x0a200023;
> +    cpu->ccsidr[0] = 0x701fe00a; /* 32KB L1 dcache */
> +    cpu->ccsidr[1] = 0x201fe012; /* 48KB L1 icache */
> +    cpu->ccsidr[2] = 0x707fe07a; /* 1MB L2 cache */
> +    cpu->dcz_blocksize = 4; /* 64 bytes */
> +    cpu->gic_num_lrs = 4;
> +    cpu->gic_vpribits = 5;
> +    cpu->gic_vprebits = 5;
> +    define_arm_cp_regs(cpu, cortex_a57_a53_cp_reginfo);

The impdef registers do seem to be basically the same as
the A53/A57. The function name is now a bit inaccurate,
though, but I don't have a good idea for a better name.

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 11/12] hw/arm: versal: Add a model of Xilinx Versal SoC

[Peter Maydell]

On 3 October 2018 at 16:07, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Add a model of Xilinx Versal SoC.
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>  default-configs/aarch64-softmmu.mak |   1 +
>  hw/arm/Makefile.objs                |   1 +
>  hw/arm/xlnx-versal.c                | 339 ++++++++++++++++++++++++++++++++++++
>  include/hw/arm/xlnx-versal.h        | 122 +++++++++++++
>  4 files changed, 463 insertions(+)
>  create mode 100644 hw/arm/xlnx-versal.c
>  create mode 100644 include/hw/arm/xlnx-versal.h
>


> +#define XLNX_VERSAL_ACPU_TYPE "cortex-a72" "-" TYPE_ARM_CPU

ARM_CPU_TYPE_NAME("cortex-a72") is preferable to hand-assembling
the type name like this.

> +#define GEM_REVISION        0x40070106
> +

> +    for (i = 0; i < nr_apu_cpus; i++) {
> +        DeviceState *cpudev = DEVICE(s->fpd.apu.cpu[i]);
> +        int ppibase = XLNX_VERSAL_NR_IRQS + i * GIC_INTERNAL + GIC_NR_SGIS;
> +        qemu_irq maint_irq;
> +        int ti;
> +        /* Mapping from the output timer irq lines from the CPU to the
> +         * GIC PPI inputs we use for the virt board.
> +         */

This isn't the virt board :-) -- cut-n-pasted comment ?

> +        const int timer_irq[] = {
> +            [GTIMER_PHYS] = VERSAL_TIMER_NS_EL1_IRQ,
> +            [GTIMER_VIRT] = VERSAL_TIMER_VIRT_IRQ,
> +            [GTIMER_HYP]  = VERSAL_TIMER_NS_EL2_IRQ,
> +            [GTIMER_SEC]  = VERSAL_TIMER_S_EL1_IRQ,
> +        };
> +
> +        for (ti = 0; ti < ARRAY_SIZE(timer_irq); ti++) {
> +            qdev_connect_gpio_out(cpudev, ti,
> +                                  qdev_get_gpio_in(gicdev,
> +                                                   ppibase + timer_irq[ti]));
> +        }
> +        maint_irq = qdev_get_gpio_in(gicdev,
> +                                        ppibase + VERSAL_GIC_MAINT_IRQ);
> +        qdev_connect_gpio_out_named(cpudev, "gicv3-maintenance-interrupt",
> +                                    0, maint_irq);
> +        sysbus_connect_irq(gicbusdev, i, qdev_get_gpio_in(cpudev, ARM_CPU_IRQ));
> +        sysbus_connect_irq(gicbusdev, i + nr_apu_cpus,
> +                           qdev_get_gpio_in(cpudev, ARM_CPU_FIQ));
> +        sysbus_connect_irq(gicbusdev, i + 2 * nr_apu_cpus,
> +                           qdev_get_gpio_in(cpudev, ARM_CPU_VIRQ));
> +        sysbus_connect_irq(gicbusdev, i + 3 * nr_apu_cpus,
> +                           qdev_get_gpio_in(cpudev, ARM_CPU_VFIQ));
> +    }
> +
> +    for (i = 0; i < XLNX_VERSAL_NR_IRQS; i++) {
> +        pic[i] = qdev_get_gpio_in(gicdev, i);
> +    }

> +/* This takes the board allocated linear DDR memory and creates aliases
> + * for each split DDR range/apperture on the Versal address map.

"aperture"



> +static void versal_realize(DeviceState *dev, Error **errp)
> +{
> +    Versal *s = XLNX_VERSAL(dev);
> +    qemu_irq pic[XLNX_VERSAL_NR_IRQS];
> +
> +    versal_create_apu_cpus(s, errp);
> +    versal_create_apu_gic(s, pic, errp);
> +    versal_create_uarts(s, pic);
> +    versal_create_gems(s, pic);
> +    versal_map_ddr(s);
> +    versal_unimp(s);
> +
> +    /* Create the OCM.  */
> +    memory_region_init_ram(&s->lpd.mr_ocm, OBJECT(s), "ocm",
> +                           MM_OCM_SIZE, &error_fatal);

What's an OCM? Is it really memory, or is this a stub for something?

> +
> +    memory_region_add_subregion_overlap(&s->mr_ps, MM_OCM, &s->lpd.mr_ocm, 0);
> +    memory_region_add_subregion_overlap(&s->fpd.apu.mr, 0, &s->mr_ps, 0);
> +}
> +
> +static void versal_init(Object *obj)
> +{
> +    Versal *s = XLNX_VERSAL(obj);
> +
> +    memory_region_init(&s->fpd.apu.mr, obj, "mr-apu", UINT64_MAX);
> +    memory_region_init(&s->mr_ps, obj, "mr-ps-switch", UINT64_MAX);
> +}
> +
> +static const VMStateDescription versal_vmstate = {
> +    .name = "xlnx-ve",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        /* FIXME.  */

Stray FIXME comment -- I think the answer may be "the
SoC object has no state of its own so needs neither a
vmsd nor a reset method" ? (If so and if you drop them,
do put a comment in the class init about why they're not
provided. Some day we may have a mechanism for a device
to explicitly say "I need no vmstate" so we can assert if
none is provided.)

> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static Property versal_properties[] = {
> +    DEFINE_PROP_LINK("ddr", Versal, cfg.mr_ddr, TYPE_MEMORY_REGION,
> +                     MemoryRegion *),
> +    DEFINE_PROP_UINT32("psci-conduit", Versal, cfg.psci_conduit, 0),
> +    DEFINE_PROP_END_OF_LIST()
> +};
> +
> +static void versal_reset(DeviceState *dev)
> +{
> +}
> +
> +static void versal_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = versal_realize;
> +    dc->vmsd = &versal_vmstate;
> +    dc->props = versal_properties;
> +    dc->reset = versal_reset;
> +}

Looks good otherwise.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 00/12] arm: Add first models of Xilinx Versal SoC

[Peter Maydell]

On 3 October 2018 at 16:07, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> In QEMU we'd like to have a virtual developer board with the Versal SoC
> and a selected set of peripherals under the control of QEMU.
> We'd like to gradually extend this board as QEMU gains more support
> for Versal hardware components. QEMU will generate a device-tree
> describing only the components it supports and includes in the virtual
> dev board.

So, the SoC implementation and the GEM and HVC bugfix patches
here are straightforward. What I'm less sure about is the "virtual"
nature of the board model. What do we gain doing this rather than
just modelling some particular Versal dev board?

At the moment we have a fairly clear distinction:
 * most machine models are models of real hardware, and the
   real hardware is the litmus test for how things are supposed
   to work (and, like real hardware, the user provides the DTB)
 * the "virt" board is special, because it is purely virtual and
   contains only a few specific devices, so it can run Linux guests

This would seem to be an odd hybrid, with an SoC that's a model
of real hardware but also some virtual "QEMU controls what's
present and creates the dtb" aspects.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 06/12] net: cadence_gem: Add support for selecting the DMA MemoryRegion

[Edgar E. Iglesias]

On Mon, Oct 08, 2018 at 01:24:51PM +0100, Peter Maydell wrote:
> On 3 October 2018 at 16:07, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >
> > Add support for selecting the Memory Region that the GEM
> > will do DMA to.
> >
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> > ---
> 
> 
> > @@ -1500,6 +1506,13 @@ static void gem_realize(DeviceState *dev, Error **errp)
> >      CadenceGEMState *s = CADENCE_GEM(dev);
> >      int i;
> >
> > +    if (s->dma_mr) {
> > +        s->dma_as = g_malloc0(sizeof(AddressSpace));
> > +        address_space_init(s->dma_as, s->dma_mr, NULL);
> 
> Why not just have the CadenceGEMState embed the AddressSpace
> 
>     AddressSpace dma_as;
> 
> rather than doing a separate memory allocation here?

No reason not to, I copied this from a pattern in our code and didn't reflect too much about the allocation.
I'll change it for next version.

Cheers,
Edgar


> 
> > +    } else {
> > +        s->dma_as = &address_space_memory;
> > +    }
> 
> thanks
> -- PMM

Re: [Qemu-devel] [PATCH v1 06/12] net: cadence_gem: Add support for selecting the DMA MemoryRegion

[Edgar E. Iglesias]

On Mon, Oct 08, 2018 at 01:30:20PM +0100, Peter Maydell wrote:
> On 3 October 2018 at 16:07, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >
> > Add support for selecting the Memory Region that the GEM
> > will do DMA to.
> >
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> > ---
> >  hw/net/cadence_gem.c         | 63 ++++++++++++++++++++++++++++----------------
> >  include/hw/net/cadence_gem.h |  2 ++
> >  2 files changed, 43 insertions(+), 22 deletions(-)
> >
> > diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> > index 759c1d7..ab02515 100644
> > --- a/hw/net/cadence_gem.c
> > +++ b/hw/net/cadence_gem.c
> > @@ -28,6 +28,7 @@
> >  #include "hw/net/cadence_gem.h"
> >  #include "qapi/error.h"
> >  #include "qemu/log.h"
> > +#include "sysemu/dma.h"
> >  #include "net/checksum.h"
> >
> >  #ifdef CADENCE_GEM_ERR_DEBUG
> > @@ -835,9 +836,9 @@ static void gem_get_rx_desc(CadenceGEMState *s, int q)
> >  {
> >      DB_PRINT("read descriptor 0x%x\n", (unsigned)s->rx_desc_addr[q]);
> >      /* read current descriptor */
> > -    cpu_physical_memory_read(s->rx_desc_addr[q],
> > -                             (uint8_t *)s->rx_desc[q],
> > -                             sizeof(uint32_t) * gem_get_desc_len(s, true));
> > +    address_space_read(s->dma_as, s->rx_desc_addr[q], MEMTXATTRS_UNSPECIFIED,
> > +                       (uint8_t *)s->rx_desc[q],
> > +                       sizeof(uint32_t) * gem_get_desc_len(s, true));
> 
> At some point you might want to add support for handling "descriptor
> read/write failed", incidentally: address_space_read/write return a
> MemTxResult which you can check for != MEMTX_OK.

Yes, the GEM can report those errors to SW so that's a nice feature to follow up with.

Thanks,
Edgar

Re: [Qemu-devel] [PATCH v1 09/12] target-arm: powerctl: Enable HVC when starting CPUs to EL2

[Edgar E. Iglesias]

On Mon, Oct 08, 2018 at 01:41:36PM +0100, Peter Maydell wrote:
> On 3 October 2018 at 16:07, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >
> > When QEMU provides the equivalent of the EL3 firmware, we
> > need to enable HVCs in scr_el3 when turning on CPUs that
> > target EL2.
> >
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> > ---
> >  target/arm/arm-powerctl.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/target/arm/arm-powerctl.c b/target/arm/arm-powerctl.c
> > index ce55eeb..54f2974 100644
> > --- a/target/arm/arm-powerctl.c
> > +++ b/target/arm/arm-powerctl.c
> > @@ -63,6 +63,7 @@ static void arm_set_cpu_on_async_work(CPUState *target_cpu_state,
> >      struct CpuOnInfo *info = (struct CpuOnInfo *) data.host_ptr;
> >
> >      /* Initialize the cpu we are turning on */
> > +    qemu_log("CPU%d reset\n", target_cpu_state->cpu_index);
> 
> qemu_log logging should always be masked to a particular
> log kind (either via qemu_log_mask() or by explicit check on
> the log level). If this was accidentally left-in debug, you could
> just drop it :-)

Hah, yes this is an accidental leftover...


> 
> >      cpu_reset(target_cpu_state);
> >      target_cpu_state->halted = 0;
> >
> > @@ -103,6 +104,16 @@ static void arm_set_cpu_on_async_work(CPUState *target_cpu_state,
> >      } else {
> >          /* Processor is not in secure mode */
> >          target_cpu->env.cp15.scr_el3 |= SCR_NS;
> > +
> > +        /*
> > +         * If QEMU is providing the equivalent of EL3 firmware, then we need
> > +         * to make sure a CPU targeting EL2 comes out of reset with a
> > +         * functional HVC insn.
> > +         */
> > +        if (arm_feature(&target_cpu->env, ARM_FEATURE_EL3)
> > +            && info->target_el == 2) {
> > +            target_cpu->env.cp15.scr_el3 |= SCR_HCE;
> > +        }
> >      }
> 
> Otherwise
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> This is definitely required for PSCI and I think it makes sense
> in other cases that end up in this powerctl code with EL3 set.

Thanks,
Edgar

Re: [Qemu-devel] [PATCH v1 10/12] target/arm: Add the Cortex-A72

[Edgar E. Iglesias]

On Mon, Oct 08, 2018 at 02:10:29PM +0100, Peter Maydell wrote:
> On 3 October 2018 at 16:07, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >
> > Add the ARM Cortex-A72.
> >
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> > ---
> >  target/arm/cpu64.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 59 insertions(+)
> >
> > diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> > index 800bff7..02e500b 100644
> > --- a/target/arm/cpu64.c
> > +++ b/target/arm/cpu64.c
> > @@ -218,6 +218,64 @@ static void aarch64_a53_initfn(Object *obj)
> >      define_arm_cp_regs(cpu, cortex_a57_a53_cp_reginfo);
> >  }
> >
> > +static void aarch64_a72_initfn(Object *obj)
> > +{
> > +    ARMCPU *cpu = ARM_CPU(obj);
> > +
> > +    cpu->dtb_compatible = "arm,cortex-a72";
> > +    set_feature(&cpu->env, ARM_FEATURE_V8);
> > +    set_feature(&cpu->env, ARM_FEATURE_VFP4);
> > +    set_feature(&cpu->env, ARM_FEATURE_NEON);
> > +    set_feature(&cpu->env, ARM_FEATURE_GENERIC_TIMER);
> > +    set_feature(&cpu->env, ARM_FEATURE_AARCH64);
> > +    set_feature(&cpu->env, ARM_FEATURE_CBAR_RO);
> > +    set_feature(&cpu->env, ARM_FEATURE_V8_AES);
> > +    set_feature(&cpu->env, ARM_FEATURE_V8_SHA1);
> > +    set_feature(&cpu->env, ARM_FEATURE_V8_SHA256);
> > +    set_feature(&cpu->env, ARM_FEATURE_V8_PMULL);
> > +    set_feature(&cpu->env, ARM_FEATURE_CRC);
> > +    set_feature(&cpu->env, ARM_FEATURE_EL2);
> > +    set_feature(&cpu->env, ARM_FEATURE_EL3);
> 
> No ARM_FEATURE_PMU ?

Will add in v2.


> 
> > +    cpu->midr = 0x410fd083;
> > +    cpu->revidr = 0x00000000;
> > +    cpu->reset_fpsid = 0x41034080;
> > +    cpu->mvfr0 = 0x10110222;
> > +    cpu->mvfr1 = 0x12111111;
> > +    cpu->mvfr2 = 0x00000043;
> > +    cpu->ctr = 0x8444c004;
> > +    cpu->reset_sctlr = 0x00c50838;
> 
> Do you happen to have the hardware to hand to check what the
> top 4 bits of the reset value of SCTLR_ELx are? I think they
> should be 0x3 -- the Arm ARM says that [29:28] are RES1 (as
> does the A72 TRM, though its top level summary table lists
> 0x00c50838 as the reset value for some of the SCTLR_ELx.)
> 
> QEMU may have the wrong value for A53/A57 here too, I suspect.

I don't have access to the A72s at the moment but looking at logs
it seems to be 0x00c50838 for both the A53 and A72.
Looking at "Table 4-118 SCTLR bit assignments" in the A72 TRM,
bits [30:28] seem to have been allocated. Bit 30 depends on
configuration inputs to the core and [29:28] seem to be hard-coded
to zero.

On ZynqMP and Versal, bit 30 (CFGTE) seems to be  runtime configurable
prior to releasing the A53s/A72s from reset. I haven't tried it though.

I ran a test on the ZynqMP a53s and got the folloing right after reset,
for EL1, EL2 and EL3:
sctlr c52838 30c50838 c52838

The 0x2000 mask is due to V-hivecs being set.
SCLTR_EL2 seems to get the RES1 bits set to 1...

Perhaps we should just leave it as is for now?


> 
> > +    cpu->id_pfr0 = 0x00000131;
> > +    cpu->id_pfr1 = 0x00011011;
> > +    cpu->id_dfr0 = 0x03010066;
> > +    cpu->id_afr0 = 0x00000000;
> > +    cpu->id_mmfr0 = 0x10201105;
> > +    cpu->id_mmfr1 = 0x40000000;
> > +    cpu->id_mmfr2 = 0x01260000;
> > +    cpu->id_mmfr3 = 0x02102211;
> > +    cpu->id_isar0 = 0x02101110;
> > +    cpu->id_isar1 = 0x13112111;
> > +    cpu->id_isar2 = 0x21232042;
> > +    cpu->id_isar3 = 0x01112131;
> > +    cpu->id_isar4 = 0x00011142;
> > +    cpu->id_isar5 = 0x00011121;
> > +    cpu->id_aa64pfr0 = 0x00002222;
> > +    cpu->id_aa64dfr0 = 0x10305106;
> > +    cpu->pmceid0 = 0x00000000;
> > +    cpu->pmceid1 = 0x00000000;
> > +    cpu->id_aa64isar0 = 0x00011120;
> > +    cpu->id_aa64mmfr0 = 0x00001124;
> > +    cpu->dbgdidr = 0x3516d000;
> > +    cpu->clidr = 0x0a200023;
> > +    cpu->ccsidr[0] = 0x701fe00a; /* 32KB L1 dcache */
> > +    cpu->ccsidr[1] = 0x201fe012; /* 48KB L1 icache */
> > +    cpu->ccsidr[2] = 0x707fe07a; /* 1MB L2 cache */
> > +    cpu->dcz_blocksize = 4; /* 64 bytes */
> > +    cpu->gic_num_lrs = 4;
> > +    cpu->gic_vpribits = 5;
> > +    cpu->gic_vprebits = 5;
> > +    define_arm_cp_regs(cpu, cortex_a57_a53_cp_reginfo);
> 
> The impdef registers do seem to be basically the same as
> the A53/A57. The function name is now a bit inaccurate,
> though, but I don't have a good idea for a better name.
> 
> Otherwise
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> 
> thanks
> -- PMM

Re: [Qemu-devel] [PATCH v1 11/12] hw/arm: versal: Add a model of Xilinx Versal SoC

[Edgar E. Iglesias]

On Mon, Oct 08, 2018 at 02:19:09PM +0100, Peter Maydell wrote:
> On 3 October 2018 at 16:07, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >
> > Add a model of Xilinx Versal SoC.
> >
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> > ---
> >  default-configs/aarch64-softmmu.mak |   1 +
> >  hw/arm/Makefile.objs                |   1 +
> >  hw/arm/xlnx-versal.c                | 339 ++++++++++++++++++++++++++++++++++++
> >  include/hw/arm/xlnx-versal.h        | 122 +++++++++++++
> >  4 files changed, 463 insertions(+)
> >  create mode 100644 hw/arm/xlnx-versal.c
> >  create mode 100644 include/hw/arm/xlnx-versal.h
> >
> 
> 
> > +#define XLNX_VERSAL_ACPU_TYPE "cortex-a72" "-" TYPE_ARM_CPU
> 
> ARM_CPU_TYPE_NAME("cortex-a72") is preferable to hand-assembling
> the type name like this.

Fixed for v2.


> 
> > +#define GEM_REVISION        0x40070106
> > +
> 
> > +    for (i = 0; i < nr_apu_cpus; i++) {
> > +        DeviceState *cpudev = DEVICE(s->fpd.apu.cpu[i]);
> > +        int ppibase = XLNX_VERSAL_NR_IRQS + i * GIC_INTERNAL + GIC_NR_SGIS;
> > +        qemu_irq maint_irq;
> > +        int ti;
> > +        /* Mapping from the output timer irq lines from the CPU to the
> > +         * GIC PPI inputs we use for the virt board.
> > +         */
> 
> This isn't the virt board :-) -- cut-n-pasted comment ?

Fixed for v2.


> 
> > +        const int timer_irq[] = {
> > +            [GTIMER_PHYS] = VERSAL_TIMER_NS_EL1_IRQ,
> > +            [GTIMER_VIRT] = VERSAL_TIMER_VIRT_IRQ,
> > +            [GTIMER_HYP]  = VERSAL_TIMER_NS_EL2_IRQ,
> > +            [GTIMER_SEC]  = VERSAL_TIMER_S_EL1_IRQ,
> > +        };
> > +
> > +        for (ti = 0; ti < ARRAY_SIZE(timer_irq); ti++) {
> > +            qdev_connect_gpio_out(cpudev, ti,
> > +                                  qdev_get_gpio_in(gicdev,
> > +                                                   ppibase + timer_irq[ti]));
> > +        }
> > +        maint_irq = qdev_get_gpio_in(gicdev,
> > +                                        ppibase + VERSAL_GIC_MAINT_IRQ);
> > +        qdev_connect_gpio_out_named(cpudev, "gicv3-maintenance-interrupt",
> > +                                    0, maint_irq);
> > +        sysbus_connect_irq(gicbusdev, i, qdev_get_gpio_in(cpudev, ARM_CPU_IRQ));
> > +        sysbus_connect_irq(gicbusdev, i + nr_apu_cpus,
> > +                           qdev_get_gpio_in(cpudev, ARM_CPU_FIQ));
> > +        sysbus_connect_irq(gicbusdev, i + 2 * nr_apu_cpus,
> > +                           qdev_get_gpio_in(cpudev, ARM_CPU_VIRQ));
> > +        sysbus_connect_irq(gicbusdev, i + 3 * nr_apu_cpus,
> > +                           qdev_get_gpio_in(cpudev, ARM_CPU_VFIQ));
> > +    }
> > +
> > +    for (i = 0; i < XLNX_VERSAL_NR_IRQS; i++) {
> > +        pic[i] = qdev_get_gpio_in(gicdev, i);
> > +    }
> 
> > +/* This takes the board allocated linear DDR memory and creates aliases
> > + * for each split DDR range/apperture on the Versal address map.
> 
> "aperture"

Fixed


> 
> 
> 
> > +static void versal_realize(DeviceState *dev, Error **errp)
> > +{
> > +    Versal *s = XLNX_VERSAL(dev);
> > +    qemu_irq pic[XLNX_VERSAL_NR_IRQS];
> > +
> > +    versal_create_apu_cpus(s, errp);
> > +    versal_create_apu_gic(s, pic, errp);
> > +    versal_create_uarts(s, pic);
> > +    versal_create_gems(s, pic);
> > +    versal_map_ddr(s);
> > +    versal_unimp(s);
> > +
> > +    /* Create the OCM.  */
> > +    memory_region_init_ram(&s->lpd.mr_ocm, OBJECT(s), "ocm",
> > +                           MM_OCM_SIZE, &error_fatal);
> 
> What's an OCM? Is it really memory, or is this a stub for something?


I've changed the comment to spell out that it's an On Chip Memory.


> 
> > +
> > +    memory_region_add_subregion_overlap(&s->mr_ps, MM_OCM, &s->lpd.mr_ocm, 0);
> > +    memory_region_add_subregion_overlap(&s->fpd.apu.mr, 0, &s->mr_ps, 0);
> > +}
> > +
> > +static void versal_init(Object *obj)
> > +{
> > +    Versal *s = XLNX_VERSAL(obj);
> > +
> > +    memory_region_init(&s->fpd.apu.mr, obj, "mr-apu", UINT64_MAX);
> > +    memory_region_init(&s->mr_ps, obj, "mr-ps-switch", UINT64_MAX);
> > +}
> > +
> > +static const VMStateDescription versal_vmstate = {
> > +    .name = "xlnx-ve",
> > +    .version_id = 1,
> > +    .minimum_version_id = 1,
> > +    .fields = (VMStateField[]) {
> > +        /* FIXME.  */
> 
> Stray FIXME comment -- I think the answer may be "the
> SoC object has no state of its own so needs neither a
> vmsd nor a reset method" ? (If so and if you drop them,
> do put a comment in the class init about why they're not
> provided. Some day we may have a mechanism for a device
> to explicitly say "I need no vmstate" so we can assert if
> none is provided.)

Thanks, I'll do that.


> 
> > +        VMSTATE_END_OF_LIST()
> > +    }
> > +};
> > +
> > +static Property versal_properties[] = {
> > +    DEFINE_PROP_LINK("ddr", Versal, cfg.mr_ddr, TYPE_MEMORY_REGION,
> > +                     MemoryRegion *),
> > +    DEFINE_PROP_UINT32("psci-conduit", Versal, cfg.psci_conduit, 0),
> > +    DEFINE_PROP_END_OF_LIST()
> > +};
> > +
> > +static void versal_reset(DeviceState *dev)
> > +{
> > +}
> > +
> > +static void versal_class_init(ObjectClass *klass, void *data)
> > +{
> > +    DeviceClass *dc = DEVICE_CLASS(klass);
> > +
> > +    dc->realize = versal_realize;
> > +    dc->vmsd = &versal_vmstate;
> > +    dc->props = versal_properties;
> > +    dc->reset = versal_reset;
> > +}
> 
> Looks good otherwise.


Thanks,
Edgar

Re: [Qemu-devel] [PATCH v1 10/12] target/arm: Add the Cortex-A72

[Peter Maydell]

On 8 October 2018 at 22:34, Edgar E. Iglesias <edgar.iglesias@xilinx.com> wrote:
> On Mon, Oct 08, 2018 at 02:10:29PM +0100, Peter Maydell wrote:
>> On 3 October 2018 at 16:07, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
>> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>> >
>> > Add the ARM Cortex-A72.
>> >
>> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

>> > +    cpu->midr = 0x410fd083;
>> > +    cpu->revidr = 0x00000000;
>> > +    cpu->reset_fpsid = 0x41034080;
>> > +    cpu->mvfr0 = 0x10110222;
>> > +    cpu->mvfr1 = 0x12111111;
>> > +    cpu->mvfr2 = 0x00000043;
>> > +    cpu->ctr = 0x8444c004;
>> > +    cpu->reset_sctlr = 0x00c50838;
>>
>> Do you happen to have the hardware to hand to check what the
>> top 4 bits of the reset value of SCTLR_ELx are? I think they
>> should be 0x3 -- the Arm ARM says that [29:28] are RES1 (as
>> does the A72 TRM, though its top level summary table lists
>> 0x00c50838 as the reset value for some of the SCTLR_ELx.)
>>
>> QEMU may have the wrong value for A53/A57 here too, I suspect.
>
> I don't have access to the A72s at the moment but looking at logs
> it seems to be 0x00c50838 for both the A53 and A72.
> Looking at "Table 4-118 SCTLR bit assignments" in the A72 TRM,
> bits [30:28] seem to have been allocated. Bit 30 depends on
> configuration inputs to the core and [29:28] seem to be hard-coded
> to zero.

Ah, this is a 32-bit view vs 64-bit view thing. In 32-bit,
SCTLR[28] is TRE (TEX remap enable), and SCTLR[29] is AFE
(access flag enable), and both are resets-to-zero.
HSCTLR[28] and [29] are both reserved, RES1.
In 64-bit, SCTLR_EL1[29:28] are RES1 in ARMv8.1 and v8.0, and
have new meanings assigned in v8.2 and v8.3.
SCTLR_EL2[29:28] and SCTLR_EL3[29:28] are reserved, RES1.

For QEMU at the moment we don't deal with this, and so we
have only the one reset value, cpu->reset_sctlr, which we use
for both the SCTLR_EL1 and SCTLR_EL3 resets. Our HSCTLR/SCTLR_EL2
resets to zero, and we don't allow for the 64-bit and 32-bit views
not necessarily being the same value.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 00/12] arm: Add first models of Xilinx Versal SoC

[Edgar E. Iglesias]

On Mon, Oct 08, 2018 at 03:08:14PM +0100, Peter Maydell wrote:
> On 3 October 2018 at 16:07, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > In QEMU we'd like to have a virtual developer board with the Versal SoC
> > and a selected set of peripherals under the control of QEMU.
> > We'd like to gradually extend this board as QEMU gains more support
> > for Versal hardware components. QEMU will generate a device-tree
> > describing only the components it supports and includes in the virtual
> > dev board.
> 
> So, the SoC implementation and the GEM and HVC bugfix patches
> here are straightforward. What I'm less sure about is the "virtual"
> nature of the board model. What do we gain doing this rather than
> just modelling some particular Versal dev board?
> 
> At the moment we have a fairly clear distinction:
>  * most machine models are models of real hardware, and the
>    real hardware is the litmus test for how things are supposed
>    to work (and, like real hardware, the user provides the DTB)
>  * the "virt" board is special, because it is purely virtual and
>    contains only a few specific devices, so it can run Linux guests
> 
> This would seem to be an odd hybrid, with an SoC that's a model
> of real hardware but also some virtual "QEMU controls what's
> present and creates the dtb" aspects.


Hi Peter,

This is a good question.

There're a few issues we see that we think this approach will help to solve
or at least mitigate.

The Versal architecture will be a family of chips (same as ZynqMP). Each
tapeout and board comes with a set of physical limitations. I.e due to
size/power/heat and other constraints not everything that the architecture
has can be available everywhere. For example a specific board may only
enable one GEM.

We'd like to have a QEMU board that doesn't get limited by these physical
constrains but instead, with time, can grow to the full potential even
though there may not be tapeouts nor boards at this given point in time
that have it all.

Another problem that we've seen with the ZynqMP is that users struggle
to run stuff on QEMU because we don't support the full set of devices
needed to run everything that you can run on a real board. So we need
to create "QEMU" device-trees that contain the subset that QEMU supports.
These dtbs then float around and mixing and matching versions becomes a problem.
If QEMU generates the a device tree on the fly, we hope to avoid that
problem. Note that this board still accepts a dtb on the command-line if
users really want to pass one.


At the moment, Versal only exists in FPGA based emulation boards.
These implementations are flexible in that different configurations can
be re-programmed onto the FPGAs at any time.
>From my perspective, the name of the board is not so important. We can
call it Xilinx Versal QEMU Developer board, Xilinx Versal Emulation Board,
or what ever but it would be nice to keep the characteristics of an
phsyically unconstrained board and the auto-generated device-tree.

I hope that clarifies the intent.

Thanks and Best regards,
Edgar

Re: [Qemu-devel] [PATCH v1 10/12] target/arm: Add the Cortex-A72

[Edgar E. Iglesias]

On Tue, Oct 09, 2018 at 10:30:01AM +0100, Peter Maydell wrote:
> On 8 October 2018 at 22:34, Edgar E. Iglesias <edgar.iglesias@xilinx.com> wrote:
> > On Mon, Oct 08, 2018 at 02:10:29PM +0100, Peter Maydell wrote:
> >> On 3 October 2018 at 16:07, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> >> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >> >
> >> > Add the ARM Cortex-A72.
> >> >
> >> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> 
> >> > +    cpu->midr = 0x410fd083;
> >> > +    cpu->revidr = 0x00000000;
> >> > +    cpu->reset_fpsid = 0x41034080;
> >> > +    cpu->mvfr0 = 0x10110222;
> >> > +    cpu->mvfr1 = 0x12111111;
> >> > +    cpu->mvfr2 = 0x00000043;
> >> > +    cpu->ctr = 0x8444c004;
> >> > +    cpu->reset_sctlr = 0x00c50838;
> >>
> >> Do you happen to have the hardware to hand to check what the
> >> top 4 bits of the reset value of SCTLR_ELx are? I think they
> >> should be 0x3 -- the Arm ARM says that [29:28] are RES1 (as
> >> does the A72 TRM, though its top level summary table lists
> >> 0x00c50838 as the reset value for some of the SCTLR_ELx.)
> >>
> >> QEMU may have the wrong value for A53/A57 here too, I suspect.
> >
> > I don't have access to the A72s at the moment but looking at logs
> > it seems to be 0x00c50838 for both the A53 and A72.
> > Looking at "Table 4-118 SCTLR bit assignments" in the A72 TRM,
> > bits [30:28] seem to have been allocated. Bit 30 depends on
> > configuration inputs to the core and [29:28] seem to be hard-coded
> > to zero.
> 
> Ah, this is a 32-bit view vs 64-bit view thing. In 32-bit,
> SCTLR[28] is TRE (TEX remap enable), and SCTLR[29] is AFE
> (access flag enable), and both are resets-to-zero.
> HSCTLR[28] and [29] are both reserved, RES1.
> In 64-bit, SCTLR_EL1[29:28] are RES1 in ARMv8.1 and v8.0, and
> have new meanings assigned in v8.2 and v8.3.
> SCTLR_EL2[29:28] and SCTLR_EL3[29:28] are reserved, RES1.
> 
> For QEMU at the moment we don't deal with this, and so we
> have only the one reset value, cpu->reset_sctlr, which we use
> for both the SCTLR_EL1 and SCTLR_EL3 resets. Our HSCTLR/SCTLR_EL2
> resets to zero, and we don't allow for the 64-bit and 32-bit views
> not necessarily being the same value.

Aha, I see. I'll leave as is then and we can fix the 64 bit stuff later I guess.

Another A72 related thing I wanted to check with you. A month or two ago I was
looking at an issue with Linux running very slowly on our models.
Something that popped up was that Linux was running a couple of spectre related
"workarounds" and "hardening" sequences on the QEMU A72s.

There are a couple of bits in the ID_AARCH64_PFR0 register that
Linux checks before enabling the sequences but I never found any
documentation of them in the specs. Bits 56 and 60.

In Linux these are refered to as:
ID_AA64PFR0_CSV2_SHIFT
ID_AA64PFR0_CSV3_SHIFT

This is what we have in our tree:

    cpu->gic_vprebits = 5;
    define_arm_cp_regs(cpu, cortex_a57_a53_cp_reginfo);

    /* Xilinx FIXUPs.  */
    /* These indicate the BP hardening and KPTI aren't needed.  */
    cpu->id_aa64pfr0 |= (uint64_t)1 << 56; /* BP.  */
    cpu->id_aa64pfr0 |= (uint64_t)1 << 60; /* KPTI.  */
}

Do you know what these are?
Should we be setting these in QEMU?

Cheers,
Edgar

Re: [Qemu-devel] [PATCH v1 10/12] target/arm: Add the Cortex-A72

[Laurent Desnogues]

Hello,

On Tue, Oct 9, 2018 at 3:19 PM Edgar E. Iglesias
<edgar.iglesias@xilinx.com> wrote:
>
> Another A72 related thing I wanted to check with you. A month or two ago I was
> looking at an issue with Linux running very slowly on our models.
> Something that popped up was that Linux was running a couple of spectre related
> "workarounds" and "hardening" sequences on the QEMU A72s.
>
> There are a couple of bits in the ID_AARCH64_PFR0 register that
> Linux checks before enabling the sequences but I never found any
> documentation of them in the specs. Bits 56 and 60.
>
> In Linux these are refered to as:
> ID_AA64PFR0_CSV2_SHIFT
> ID_AA64PFR0_CSV3_SHIFT
>
> This is what we have in our tree:
>
>     cpu->gic_vprebits = 5;
>     define_arm_cp_regs(cpu, cortex_a57_a53_cp_reginfo);
>
>     /* Xilinx FIXUPs.  */
>     /* These indicate the BP hardening and KPTI aren't needed.  */
>     cpu->id_aa64pfr0 |= (uint64_t)1 << 56; /* BP.  */
>     cpu->id_aa64pfr0 |= (uint64_t)1 << 60; /* KPTI.  */
> }
>
> Do you know what these are?
> Should we be setting these in QEMU?

These fields are publicly documented in the system register specification:

https://developer.arm.com/products/architecture/cpu-architecture/a-profile/exploration-tools

These are ARMv8.5 fields, I don't think these should be set by default
for Cortex-A72. Of course nothing prevents you from defining a
specific CPU with these fields set to boot faster :-)  Or perhaps add
a property to override the default value of these registers?


Laurent

Re: [Qemu-devel] [PATCH v1 10/12] target/arm: Add the Cortex-A72

[Edgar E. Iglesias]

On Tue, Oct 09, 2018 at 03:40:13PM +0200, Laurent Desnogues wrote:
> Hello,
> 
> On Tue, Oct 9, 2018 at 3:19 PM Edgar E. Iglesias
> <edgar.iglesias@xilinx.com> wrote:
> >
> > Another A72 related thing I wanted to check with you. A month or two ago I was
> > looking at an issue with Linux running very slowly on our models.
> > Something that popped up was that Linux was running a couple of spectre related
> > "workarounds" and "hardening" sequences on the QEMU A72s.
> >
> > There are a couple of bits in the ID_AARCH64_PFR0 register that
> > Linux checks before enabling the sequences but I never found any
> > documentation of them in the specs. Bits 56 and 60.
> >
> > In Linux these are refered to as:
> > ID_AA64PFR0_CSV2_SHIFT
> > ID_AA64PFR0_CSV3_SHIFT
> >
> > This is what we have in our tree:
> >
> >     cpu->gic_vprebits = 5;
> >     define_arm_cp_regs(cpu, cortex_a57_a53_cp_reginfo);
> >
> >     /* Xilinx FIXUPs.  */
> >     /* These indicate the BP hardening and KPTI aren't needed.  */
> >     cpu->id_aa64pfr0 |= (uint64_t)1 << 56; /* BP.  */
> >     cpu->id_aa64pfr0 |= (uint64_t)1 << 60; /* KPTI.  */
> > }
> >
> > Do you know what these are?
> > Should we be setting these in QEMU?
> 
> These fields are publicly documented in the system register specification:
> 
> https://developer.arm.com/products/architecture/cpu-architecture/a-profile/exploration-tools
> 
> These are ARMv8.5 fields, I don't think these should be set by default
> for Cortex-A72. Of course nothing prevents you from defining a
> specific CPU with these fields set to boot faster :-)  Or perhaps add
> a property to override the default value of these registers?

Thanks, I had no idea these documents existed!

Cheers,
Edgar