[Qemu-devel] [PATCH arm-devs v2 00/14] Cadence GEM Bugfixes and missing features

[Qemu-devel] [PATCH arm-devs v2 00/14] Cadence GEM Bugfixes and missing features

[Peter Crosthwaite]

Hi Peter,

This series updated cadence GEM (ethernet in Zynq) with a number of
bugfixes and enhancements. Most of this has come out of some stress
testing we have done internally.

I have also testing briefly using mainline Linux GEM driver.

root@zynq:~# wget
http://people.debian.org/~aurel32/qemu/arm/debian_lenny_arm_staandard.qcow2
Connecting to proxy.xilinx.com:8080 (149.199.33.100:8080)
debian_lenny_arm_sta  18% |*****                          | 29898k
0:00:21 ETArandom: nonblocking pool is initialized
debian_lenny_arm_sta 100% |*******************************|   157M
0:00:00 ETA
root@zynq:~# md5sum debian_lenny_arm_standard.qcow2
7be70fc105525803fae5e095f5b4ecb5  debian_lenny_arm_standard.qcow2

Regards,
Peter

Changed since v1:
Addressed PMM review


Edgar E. Iglesias (1):
  net/cadence_gem: Update DMA rx descriptors as we process them

Peter Crosthwaite (13):
  net/cadence_gem: Implement mac level loopback mode
  net/cadence_gem: Don't assert against 0 buffer address
  net/cadence_gem: simplify rx buf descriptor walking
  net/cadence_gem: Prefetch rx descriptors ASAP
  net/cadence_gem: Implement RX descriptor match mode flags
  net/cadence_gem: Implement SAR match bit in rx desc
  net/cadence_gem: Implement SAR (de)activation
  net/cadence_gem: Add missing VMSTATE_END_OF_LIST
  net/cadence_gem: Fix rx multi-fragment packets
  net/cadence_gem: Fix small packet FCS stripping
  net/cadence_gem: Fix register w1c logic
  net/cadence_gem: Improve can_receive debug printfery
  net/cadence_gem: Don't rx packets when no rx buffer available

 hw/net/cadence_gem.c | 278 ++++++++++++++++++++++++++++++++-------------------
 1 file changed, 177 insertions(+), 101 deletions(-)

-- 
1.8.4.4

[Qemu-devel] [PATCH arm-devs v2 01/14] net/cadence_gem: Implement mac level loopback mode

[Peter Crosthwaite]

Cadence GEM has a MAC level loopback mode. Implement. Use the same basic
operation as the already implemented PHY loopback.

Reported-by: Deepika Dhamija <deepika@xilinx.com>
Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---

 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 4a355bb..a31801d 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -893,7 +893,7 @@ static void gem_transmit(GemState *s)
             gem_transmit_updatestats(s, tx_packet, total_bytes);
 
             /* Send the packet somewhere */
-            if (s->phy_loop) {
+            if (s->phy_loop || (s->regs[GEM_NWCTRL] & GEM_NWCTRL_LOCALLOOP)) {
                 gem_receive(qemu_get_queue(s->nic), tx_packet, total_bytes);
             } else {
                 qemu_send_packet(qemu_get_queue(s->nic), tx_packet,
-- 
1.8.4.4

[Qemu-devel] [PATCH arm-devs v2 02/14] net/cadence_gem: Update DMA rx descriptors as we process them

[Peter Crosthwaite]

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

We were updating the ownership bit of all descriptors if packets
get split and written through several descriptors.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---

 hw/net/cadence_gem.c | 33 ++++++++++++++++-----------------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index a31801d..b84ee60 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -592,6 +592,7 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
     unsigned   rxbuf_offset;
     uint8_t    rxbuf[2048];
     uint8_t   *rxbuf_ptr;
+    bool first_desc = true;
 
     s = qemu_get_nic_opaque(nc);
 
@@ -701,6 +702,21 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
                                   rxbuf_ptr, MIN(bytes_to_copy, rxbufsize));
         bytes_to_copy -= MIN(bytes_to_copy, rxbufsize);
         rxbuf_ptr += MIN(bytes_to_copy, rxbufsize);
+
+        /* Update the descriptor.  */
+        if (first_desc) {
+            rx_desc_set_sof(desc);
+            first_desc = false;
+        }
+        if (bytes_to_copy == 0) {
+            rx_desc_set_eof(desc);
+            rx_desc_set_length(desc, size);
+        }
+        rx_desc_set_ownership(desc);
+        /* Descriptor write-back.  */
+        cpu_physical_memory_write(packet_desc_addr,
+                                  (uint8_t *)&desc[0], sizeof(desc));
+
         if (bytes_to_copy == 0) {
             break;
         }
@@ -716,12 +732,6 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
     DB_PRINT("set length: %ld, EOF on descriptor 0x%x\n", size,
             (unsigned)packet_desc_addr);
 
-    /* Update last descriptor with EOF and total length */
-    rx_desc_set_eof(desc);
-    rx_desc_set_length(desc, size);
-    cpu_physical_memory_write(packet_desc_addr,
-                              (uint8_t *)&desc[0], sizeof(desc));
-
     /* Advance RX packet descriptor Q */
     last_desc_addr = packet_desc_addr;
     packet_desc_addr = s->rx_desc_addr;
@@ -734,20 +744,9 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
         s->rx_desc_addr += 8;
     }
 
-    DB_PRINT("set SOF, OWN on descriptor 0x%08x\n", (unsigned)packet_desc_addr);
-
     /* Count it */
     gem_receive_updatestats(s, buf, size);
 
-    /* Update first descriptor (which could also be the last) */
-    /* read descriptor */
-    cpu_physical_memory_read(packet_desc_addr,
-                             (uint8_t *)&desc[0], sizeof(desc));
-    rx_desc_set_sof(desc);
-    rx_desc_set_ownership(desc);
-    cpu_physical_memory_write(packet_desc_addr,
-                              (uint8_t *)&desc[0], sizeof(desc));
-
     s->regs[GEM_RXSTATUS] |= GEM_RXSTATUS_FRMRCVD;
     s->regs[GEM_ISR] |= GEM_INT_RXCMPL & ~(s->regs[GEM_IMR]);
 
-- 
1.8.4.4

[Qemu-devel] [PATCH arm-devs v2 03/14] net/cadence_gem: Don't assert against 0 buffer address

[Peter Crosthwaite]

This has no real hardware analog and asserting correctness of DMA
addresses is not a perhiperal level problem. Delete.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
changed since v1:
Delete the check completely

 hw/net/cadence_gem.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index b84ee60..b0f3dba 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -688,15 +688,6 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
         DB_PRINT("copy %d bytes to 0x%x\n", MIN(bytes_to_copy, rxbufsize),
                 rx_desc_get_buffer(desc));
 
-        /*
-         * Let's have QEMU lend a helping hand.
-         */
-        if (rx_desc_get_buffer(desc) == 0) {
-            DB_PRINT("Invalid RX buffer (NULL) for descriptor 0x%x\n",
-                     (unsigned)packet_desc_addr);
-            break;
-        }
-
         /* Copy packet data to emulated DMA buffer */
         cpu_physical_memory_write(rx_desc_get_buffer(desc) + rxbuf_offset,
                                   rxbuf_ptr, MIN(bytes_to_copy, rxbufsize));
-- 
1.8.4.4

[Qemu-devel] [PATCH arm-devs v2 04/14] net/cadence_gem: simplify rx buf descriptor walking

[Peter Crosthwaite]

There was a replication of the rx descriptor address walking logic.
Reorder the flow control to remove. This refactoring also obsoletes
the local variables packet_desc_addr and last_desc_addr.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 hw/net/cadence_gem.c | 37 +++++++++----------------------------
 1 file changed, 9 insertions(+), 28 deletions(-)

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index b0f3dba..69ad87e 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -586,7 +586,6 @@ static int gem_mac_address_filter(GemState *s, const uint8_t *packet)
 static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
 {
     unsigned    desc[2];
-    hwaddr packet_desc_addr, last_desc_addr;
     GemState *s;
     unsigned   rxbufsize, bytes_to_copy;
     unsigned   rxbuf_offset;
@@ -667,17 +666,16 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
 
     DB_PRINT("config bufsize: %d packet size: %ld\n", rxbufsize, size);
 
-    packet_desc_addr = s->rx_desc_addr;
-    while (1) {
-        DB_PRINT("read descriptor 0x%x\n", (unsigned)packet_desc_addr);
+    while (bytes_to_copy) {
+        DB_PRINT("read descriptor 0x%x\n", (unsigned)s->rx_desc_addr);
         /* read current descriptor */
-        cpu_physical_memory_read(packet_desc_addr,
+        cpu_physical_memory_read(s->rx_desc_addr,
                                  (uint8_t *)&desc[0], sizeof(desc));
 
         /* Descriptor owned by software ? */
         if (rx_desc_get_ownership(desc) == 1) {
             DB_PRINT("descriptor 0x%x owned by sw.\n",
-                     (unsigned)packet_desc_addr);
+                     (unsigned)s->rx_desc_addr);
             s->regs[GEM_RXSTATUS] |= GEM_RXSTATUS_NOBUF;
             s->regs[GEM_ISR] |= GEM_INT_RXUSED & ~(s->regs[GEM_IMR]);
             /* Handle interrupt consequences */
@@ -705,36 +703,19 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
         }
         rx_desc_set_ownership(desc);
         /* Descriptor write-back.  */
-        cpu_physical_memory_write(packet_desc_addr,
+        cpu_physical_memory_write(s->rx_desc_addr,
                                   (uint8_t *)&desc[0], sizeof(desc));
 
-        if (bytes_to_copy == 0) {
-            break;
-        }
-
         /* Next descriptor */
         if (rx_desc_get_wrap(desc)) {
-            packet_desc_addr = s->regs[GEM_RXQBASE];
+            DB_PRINT("wrapping RX descriptor list\n");
+            s->rx_desc_addr = s->regs[GEM_RXQBASE];
         } else {
-            packet_desc_addr += 8;
+            DB_PRINT("incrementing RX descriptor list\n");
+            s->rx_desc_addr += 8;
         }
     }
 
-    DB_PRINT("set length: %ld, EOF on descriptor 0x%x\n", size,
-            (unsigned)packet_desc_addr);
-
-    /* Advance RX packet descriptor Q */
-    last_desc_addr = packet_desc_addr;
-    packet_desc_addr = s->rx_desc_addr;
-    s->rx_desc_addr = last_desc_addr;
-    if (rx_desc_get_wrap(desc)) {
-        s->rx_desc_addr = s->regs[GEM_RXQBASE];
-        DB_PRINT("wrapping RX descriptor list\n");
-    } else {
-        DB_PRINT("incrementing RX descriptor list\n");
-        s->rx_desc_addr += 8;
-    }
-
     /* Count it */
     gem_receive_updatestats(s, buf, size);
 
-- 
1.8.4.4

[Qemu-devel] [PATCH arm-devs v2 05/14] net/cadence_gem: Prefetch rx descriptors ASAP

[Peter Crosthwaite]

The real hardware prefetches rx buffer descriptors ASAP and
potentially throws relevant interrupts following the fetch
even in the absence of a received packet.

Reported-by: Deepika Dhamija <deepika@xilinx.com>
Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
changed since v1:
Fixed function indentation (PMM review).

 hw/net/cadence_gem.c | 62 +++++++++++++++++++++++++++++-----------------------
 1 file changed, 35 insertions(+), 27 deletions(-)

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 69ad87e..6734a9d 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -346,6 +346,8 @@ typedef struct GemState {
     uint32_t rx_desc_addr;
     uint32_t tx_desc_addr;
 
+    unsigned rx_desc[2];
+
 } GemState;
 
 /* The broadcast MAC address: 0xFFFFFFFFFFFF */
@@ -579,13 +581,30 @@ static int gem_mac_address_filter(GemState *s, const uint8_t *packet)
     return GEM_RX_REJECT;
 }
 
+static void gem_get_rx_desc(GemState *s)
+{
+    DB_PRINT("read descriptor 0x%x\n", (unsigned)s->rx_desc_addr);
+    /* read current descriptor */
+    cpu_physical_memory_read(s->rx_desc_addr,
+                             (uint8_t *)s->rx_desc, sizeof(s->rx_desc));
+
+    /* Descriptor owned by software ? */
+    if (rx_desc_get_ownership(s->rx_desc) == 1) {
+        DB_PRINT("descriptor 0x%x owned by sw.\n",
+                 (unsigned)s->rx_desc_addr);
+        s->regs[GEM_RXSTATUS] |= GEM_RXSTATUS_NOBUF;
+        s->regs[GEM_ISR] |= GEM_INT_RXUSED & ~(s->regs[GEM_IMR]);
+        /* Handle interrupt consequences */
+        gem_update_int_status(s);
+    }
+}
+
 /*
  * gem_receive:
  * Fit a packet handed to us by QEMU into the receive descriptor ring.
  */
 static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
 {
-    unsigned    desc[2];
     GemState *s;
     unsigned   rxbufsize, bytes_to_copy;
     unsigned   rxbuf_offset;
@@ -595,11 +614,6 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
 
     s = qemu_get_nic_opaque(nc);
 
-    /* Do nothing if receive is not enabled. */
-    if (!gem_can_receive(nc)) {
-        return -1;
-    }
-
     /* Is this destination MAC address "for us" ? */
     if (gem_mac_address_filter(s, buf) == GEM_RX_REJECT) {
         return -1;
@@ -667,53 +681,44 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
     DB_PRINT("config bufsize: %d packet size: %ld\n", rxbufsize, size);
 
     while (bytes_to_copy) {
-        DB_PRINT("read descriptor 0x%x\n", (unsigned)s->rx_desc_addr);
-        /* read current descriptor */
-        cpu_physical_memory_read(s->rx_desc_addr,
-                                 (uint8_t *)&desc[0], sizeof(desc));
-
-        /* Descriptor owned by software ? */
-        if (rx_desc_get_ownership(desc) == 1) {
-            DB_PRINT("descriptor 0x%x owned by sw.\n",
-                     (unsigned)s->rx_desc_addr);
-            s->regs[GEM_RXSTATUS] |= GEM_RXSTATUS_NOBUF;
-            s->regs[GEM_ISR] |= GEM_INT_RXUSED & ~(s->regs[GEM_IMR]);
-            /* Handle interrupt consequences */
-            gem_update_int_status(s);
+        /* Do nothing if receive is not enabled. */
+        if (!gem_can_receive(nc)) {
+            assert(!first_desc);
             return -1;
         }
 
         DB_PRINT("copy %d bytes to 0x%x\n", MIN(bytes_to_copy, rxbufsize),
-                rx_desc_get_buffer(desc));
+                rx_desc_get_buffer(s->rx_desc));
 
         /* Copy packet data to emulated DMA buffer */
-        cpu_physical_memory_write(rx_desc_get_buffer(desc) + rxbuf_offset,
+        cpu_physical_memory_write(rx_desc_get_buffer(s->rx_desc) + rxbuf_offset,
                                   rxbuf_ptr, MIN(bytes_to_copy, rxbufsize));
         bytes_to_copy -= MIN(bytes_to_copy, rxbufsize);
         rxbuf_ptr += MIN(bytes_to_copy, rxbufsize);
 
         /* Update the descriptor.  */
         if (first_desc) {
-            rx_desc_set_sof(desc);
+            rx_desc_set_sof(s->rx_desc);
             first_desc = false;
         }
         if (bytes_to_copy == 0) {
-            rx_desc_set_eof(desc);
-            rx_desc_set_length(desc, size);
+            rx_desc_set_eof(s->rx_desc);
+            rx_desc_set_length(s->rx_desc, size);
         }
-        rx_desc_set_ownership(desc);
+        rx_desc_set_ownership(s->rx_desc);
         /* Descriptor write-back.  */
         cpu_physical_memory_write(s->rx_desc_addr,
-                                  (uint8_t *)&desc[0], sizeof(desc));
+                                  (uint8_t *)s->rx_desc, sizeof(s->rx_desc));
 
         /* Next descriptor */
-        if (rx_desc_get_wrap(desc)) {
+        if (rx_desc_get_wrap(s->rx_desc)) {
             DB_PRINT("wrapping RX descriptor list\n");
             s->rx_desc_addr = s->regs[GEM_RXQBASE];
         } else {
             DB_PRINT("incrementing RX descriptor list\n");
             s->rx_desc_addr += 8;
         }
+        gem_get_rx_desc(s);
     }
 
     /* Count it */
@@ -1053,6 +1058,9 @@ static void gem_write(void *opaque, hwaddr offset, uint64_t val,
     /* Handle register write side effects */
     switch (offset) {
     case GEM_NWCTRL:
+        if (val & GEM_NWCTRL_RXENA) {
+            gem_get_rx_desc(s);
+        }
         if (val & GEM_NWCTRL_TXSTART) {
             gem_transmit(s);
         }
-- 
1.8.4.4

[Qemu-devel] [PATCH arm-devs v2 06/14] net/cadence_gem: Implement RX descriptor match mode flags

[Peter Crosthwaite]

The various Rx packet address matching mode flags were not being set in
the rx descriptor. Implement.

Reported-by: Deepika Dhamija <deepika@xilinx.com>
Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
changed since v1:
s/PROM_ACCEPT/PROMISCUOUS_ACCEPT to be more self documenting
Add full return code comment on accept function (PMM review)

 hw/net/cadence_gem.c | 82 +++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 69 insertions(+), 13 deletions(-)

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 6734a9d..dceafb5 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -222,8 +222,13 @@
 #define PHY_REG_INT_ST_ENERGY   0x0010
 
 /***********************************************************************/
-#define GEM_RX_REJECT  1
-#define GEM_RX_ACCEPT  0
+#define GEM_RX_REJECT                   (-1)
+#define GEM_RX_PROMISCUOUS_ACCEPT       (-2)
+#define GEM_RX_BROADCAST_ACCEPT         (-3)
+#define GEM_RX_MULTICAST_HASH_ACCEPT    (-4)
+#define GEM_RX_UNICAST_HASH_ACCEPT      (-5)
+
+#define GEM_RX_SAR_ACCEPT               0
 
 /***********************************************************************/
 
@@ -236,6 +241,12 @@
 #define DESC_0_RX_WRAP 0x00000002
 #define DESC_0_RX_OWNERSHIP 0x00000001
 
+#define R_DESC_1_RX_SAR_SHIFT           25
+#define R_DESC_1_RX_SAR_LENGTH          2
+#define R_DESC_1_RX_UNICAST_HASH        (1 << 29)
+#define R_DESC_1_RX_MULTICAST_HASH      (1 << 30)
+#define R_DESC_1_RX_BROADCAST           (1 << 31)
+
 #define DESC_1_RX_SOF 0x00004000
 #define DESC_1_RX_EOF 0x00008000
 
@@ -315,6 +326,27 @@ static inline void rx_desc_set_length(unsigned *desc, unsigned len)
     desc[1] |= len;
 }
 
+static inline void rx_desc_set_broadcast(unsigned *desc)
+{
+    desc[1] |= R_DESC_1_RX_BROADCAST;
+}
+
+static inline void rx_desc_set_unicast_hash(unsigned *desc)
+{
+    desc[1] |= R_DESC_1_RX_UNICAST_HASH;
+}
+
+static inline void rx_desc_set_multicast_hash(unsigned *desc)
+{
+    desc[1] |= R_DESC_1_RX_MULTICAST_HASH;
+}
+
+static inline void rx_desc_set_sar(unsigned *desc, int sar_idx)
+{
+    desc[1] = deposit32(desc[1], R_DESC_1_RX_SAR_SHIFT, R_DESC_1_RX_SAR_LENGTH,
+                        sar_idx);
+}
+
 #define TYPE_CADENCE_GEM "cadence_gem"
 #define GEM(obj) OBJECT_CHECK(GemState, (obj), TYPE_CADENCE_GEM)
 
@@ -529,7 +561,10 @@ static unsigned calc_mac_hash(const uint8_t *mac)
  * Accept or reject this destination address?
  * Returns:
  * GEM_RX_REJECT: reject
- * GEM_RX_ACCEPT: accept
+ * >= 0: Specific address accept (which matched SAR is returned)
+ * others for various other modes of accept:
+ * GEM_RM_PROMISCUOUS_ACCEPT, GEM_RX_BROADCAST_ACCEPT,
+ * GEM_RX_MULTICAST_HASH_ACCEPT or GEM_RX_UNICAST_HASH_ACCEPT
  */
 static int gem_mac_address_filter(GemState *s, const uint8_t *packet)
 {
@@ -538,7 +573,7 @@ static int gem_mac_address_filter(GemState *s, const uint8_t *packet)
 
     /* Promiscuous mode? */
     if (s->regs[GEM_NWCFG] & GEM_NWCFG_PROMISC) {
-        return GEM_RX_ACCEPT;
+        return GEM_RX_PROMISCUOUS_ACCEPT;
     }
 
     if (!memcmp(packet, broadcast_addr, 6)) {
@@ -546,7 +581,7 @@ static int gem_mac_address_filter(GemState *s, const uint8_t *packet)
         if (s->regs[GEM_NWCFG] & GEM_NWCFG_BCAST_REJ) {
             return GEM_RX_REJECT;
         }
-        return GEM_RX_ACCEPT;
+        return GEM_RX_BROADCAST_ACCEPT;
     }
 
     /* Accept packets -w- hash match? */
@@ -557,24 +592,24 @@ static int gem_mac_address_filter(GemState *s, const uint8_t *packet)
         hash_index = calc_mac_hash(packet);
         if (hash_index < 32) {
             if (s->regs[GEM_HASHLO] & (1<<hash_index)) {
-                return GEM_RX_ACCEPT;
+                return packet[0] == 0x01 ? GEM_RX_MULTICAST_HASH_ACCEPT :
+                                           GEM_RX_UNICAST_HASH_ACCEPT;
             }
         } else {
             hash_index -= 32;
             if (s->regs[GEM_HASHHI] & (1<<hash_index)) {
-                return GEM_RX_ACCEPT;
+                return packet[0] == 0x01 ? GEM_RX_MULTICAST_HASH_ACCEPT :
+                                           GEM_RX_UNICAST_HASH_ACCEPT;
             }
         }
     }
 
     /* Check all 4 specific addresses */
     gem_spaddr = (uint8_t *)&(s->regs[GEM_SPADDR1LO]);
-    for (i = 0; i < 4; i++) {
-        if (!memcmp(packet, gem_spaddr, 6)) {
-            return GEM_RX_ACCEPT;
+    for (i = 3; i >= 0; i--) {
+        if (!memcmp(packet, gem_spaddr + 8 * i, 6)) {
+            return GEM_RX_SAR_ACCEPT + i;
         }
-
-        gem_spaddr += 8;
     }
 
     /* No address match; reject the packet */
@@ -611,11 +646,13 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
     uint8_t    rxbuf[2048];
     uint8_t   *rxbuf_ptr;
     bool first_desc = true;
+    int maf;
 
     s = qemu_get_nic_opaque(nc);
 
     /* Is this destination MAC address "for us" ? */
-    if (gem_mac_address_filter(s, buf) == GEM_RX_REJECT) {
+    maf = gem_mac_address_filter(s, buf);
+    if (maf == GEM_RX_REJECT) {
         return -1;
     }
 
@@ -706,6 +743,25 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
             rx_desc_set_length(s->rx_desc, size);
         }
         rx_desc_set_ownership(s->rx_desc);
+
+        switch (maf) {
+        case GEM_RX_PROMISCUOUS_ACCEPT:
+            break;
+        case GEM_RX_BROADCAST_ACCEPT:
+            rx_desc_set_broadcast(s->rx_desc);
+            break;
+        case GEM_RX_UNICAST_HASH_ACCEPT:
+            rx_desc_set_unicast_hash(s->rx_desc);
+            break;
+        case GEM_RX_MULTICAST_HASH_ACCEPT:
+            rx_desc_set_multicast_hash(s->rx_desc);
+            break;
+        case GEM_RX_REJECT:
+            abort();
+        default: /* SAR */
+            rx_desc_set_sar(s->rx_desc, maf);
+        }
+
         /* Descriptor write-back.  */
         cpu_physical_memory_write(s->rx_desc_addr,
                                   (uint8_t *)s->rx_desc, sizeof(s->rx_desc));
-- 
1.8.4.4

[Qemu-devel] [PATCH arm-devs v2 07/14] net/cadence_gem: Implement SAR match bit in rx desc

[Peter Crosthwaite]

Bit 27 of the RX buffer desc word 1 should be set when the packet was
accepted due to specific address register match. Implement.

This feature is absent from the Xilinx documentation (UG585) but the
behaviour is tested as accurate on real hardware.

Reported-by: Deepika Dhamija <deepika@xilinx.com>
Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---

 hw/net/cadence_gem.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index dceafb5..58d9b63 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -243,6 +243,7 @@
 
 #define R_DESC_1_RX_SAR_SHIFT           25
 #define R_DESC_1_RX_SAR_LENGTH          2
+#define R_DESC_1_RX_SAR_MATCH           (1 << 27)
 #define R_DESC_1_RX_UNICAST_HASH        (1 << 29)
 #define R_DESC_1_RX_MULTICAST_HASH      (1 << 30)
 #define R_DESC_1_RX_BROADCAST           (1 << 31)
@@ -345,6 +346,7 @@ static inline void rx_desc_set_sar(unsigned *desc, int sar_idx)
 {
     desc[1] = deposit32(desc[1], R_DESC_1_RX_SAR_SHIFT, R_DESC_1_RX_SAR_LENGTH,
                         sar_idx);
+    desc[1] |= R_DESC_1_RX_SAR_MATCH;
 }
 
 #define TYPE_CADENCE_GEM "cadence_gem"
-- 
1.8.4.4

[Qemu-devel] [PATCH arm-devs v2 08/14] net/cadence_gem: Implement SAR (de)activation

[Peter Crosthwaite]

The Specific address registers can be enabled or disabled by software.
QEMU was assuming they were always enabled. Implement the
disable/enable feature. SARs are disabled by writing to the lower half
register. They are re-enabled by then writing the upper half.

Reported-by: Deepika Dhamija <deepika@xilinx.com>
Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
change since v1:
commit msg typo s/where/were (PMM review)
Add VMSD for sar_active (PMM review)
s/++i/i++ (PMM review)

 hw/net/cadence_gem.c | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 58d9b63..07e6fe7 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -382,6 +382,7 @@ typedef struct GemState {
 
     unsigned rx_desc[2];
 
+    bool sar_active[4];
 } GemState;
 
 /* The broadcast MAC address: 0xFFFFFFFFFFFF */
@@ -609,7 +610,7 @@ static int gem_mac_address_filter(GemState *s, const uint8_t *packet)
     /* Check all 4 specific addresses */
     gem_spaddr = (uint8_t *)&(s->regs[GEM_SPADDR1LO]);
     for (i = 3; i >= 0; i--) {
-        if (!memcmp(packet, gem_spaddr + 8 * i, 6)) {
+        if (s->sar_active[i] && !memcmp(packet, gem_spaddr + 8 * i, 6)) {
             return GEM_RX_SAR_ACCEPT + i;
         }
     }
@@ -983,6 +984,7 @@ static void gem_phy_reset(GemState *s)
 
 static void gem_reset(DeviceState *d)
 {
+    int i;
     GemState *s = GEM(d);
 
     DB_PRINT("\n");
@@ -1002,6 +1004,10 @@ static void gem_reset(DeviceState *d)
     s->regs[GEM_DESCONF5] = 0x002f2145;
     s->regs[GEM_DESCONF6] = 0x00000200;
 
+    for (i = 0; i < 4; i++) {
+        s->sar_active[i] = false;
+    }
+
     gem_phy_reset(s);
 
     gem_update_int_status(s);
@@ -1151,6 +1157,18 @@ static void gem_write(void *opaque, hwaddr offset, uint64_t val,
         s->regs[GEM_IMR] |= val;
         gem_update_int_status(s);
         break;
+    case GEM_SPADDR1LO:
+    case GEM_SPADDR2LO:
+    case GEM_SPADDR3LO:
+    case GEM_SPADDR4LO:
+        s->sar_active[(offset - GEM_SPADDR1LO) / 2] = false;
+        break;
+    case GEM_SPADDR1HI:
+    case GEM_SPADDR2HI:
+    case GEM_SPADDR3HI:
+    case GEM_SPADDR4HI:
+        s->sar_active[(offset - GEM_SPADDR1HI) / 2] = true;
+        break;
     case GEM_PHYMNTNC:
         if (val & GEM_PHYMNTNC_OP_W) {
             uint32_t phy_addr, reg_num;
@@ -1218,15 +1236,16 @@ static int gem_init(SysBusDevice *sbd)
 
 static const VMStateDescription vmstate_cadence_gem = {
     .name = "cadence_gem",
-    .version_id = 1,
-    .minimum_version_id = 1,
-    .minimum_version_id_old = 1,
+    .version_id = 2,
+    .minimum_version_id = 2,
+    .minimum_version_id_old = 2,
     .fields      = (VMStateField[]) {
         VMSTATE_UINT32_ARRAY(regs, GemState, GEM_MAXREG),
         VMSTATE_UINT16_ARRAY(phy_regs, GemState, 32),
         VMSTATE_UINT8(phy_loop, GemState),
         VMSTATE_UINT32(rx_desc_addr, GemState),
         VMSTATE_UINT32(tx_desc_addr, GemState),
+        VMSTATE_BOOL_ARRAY(sar_active, GemState, 4),
     }
 };
 
-- 
1.8.4.4

[Qemu-devel] [PATCH arm-devs v2 09/14] net/cadence_gem: Add missing VMSTATE_END_OF_LIST

[Peter Crosthwaite]

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 hw/net/cadence_gem.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 07e6fe7..8cfad04 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -1246,6 +1246,7 @@ static const VMStateDescription vmstate_cadence_gem = {
         VMSTATE_UINT32(rx_desc_addr, GemState),
         VMSTATE_UINT32(tx_desc_addr, GemState),
         VMSTATE_BOOL_ARRAY(sar_active, GemState, 4),
+        VMSTATE_END_OF_LIST(),
     }
 };
 
-- 
1.8.4.4

[Qemu-devel] [PATCH arm-devs v2 10/14] net/cadence_gem: Fix rx multi-fragment packets

[Peter Crosthwaite]

Bytes_to_copy was being updated before its final use where it
advances the rx buffer pointer. This was causing total mayhem,
where packet data for any subsequent fragments was being fetched
from the wrong place.

Reported-by: Deepika Dhamija <deepika@xilinx.com>
Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---

 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 8cfad04..2afafdf 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -733,8 +733,8 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
         /* Copy packet data to emulated DMA buffer */
         cpu_physical_memory_write(rx_desc_get_buffer(s->rx_desc) + rxbuf_offset,
                                   rxbuf_ptr, MIN(bytes_to_copy, rxbufsize));
-        bytes_to_copy -= MIN(bytes_to_copy, rxbufsize);
         rxbuf_ptr += MIN(bytes_to_copy, rxbufsize);
+        bytes_to_copy -= MIN(bytes_to_copy, rxbufsize);
 
         /* Update the descriptor.  */
         if (first_desc) {
-- 
1.8.4.4

[Qemu-devel] [PATCH arm-devs v2 11/14] net/cadence_gem: Fix small packet FCS stripping

[Peter Crosthwaite]

The minimum packet size is 64, however this is before FCS stripping
occurs. So when FCS stripping the minimum packet size is 60. Fix.

Reported-by: Deepika Dhamija <deepika@xilinx.com>
Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
changed since v1: Setup bytes_to_copy before minimum size padding.

 hw/net/cadence_gem.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 2afafdf..1619507 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -687,6 +687,14 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
                  GEM_DMACFG_RBUFSZ_S) * GEM_DMACFG_RBUFSZ_MUL;
     bytes_to_copy = size;
 
+    /* Pad to minimum length. Assume FCS field is stripped, logic
+     * below will increment it to the real minimum of 64 when
+     * not FCS stripping
+     */
+    if (size < 60) {
+        size = 60;
+    }
+
     /* Strip of FCS field ? (usually yes) */
     if (s->regs[GEM_NWCFG] & GEM_NWCFG_STRIP_FCS) {
         rxbuf_ptr = (void *)buf;
@@ -713,11 +721,6 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
         size += 4;
     }
 
-    /* Pad to minimum length */
-    if (size < 64) {
-        size = 64;
-    }
-
     DB_PRINT("config bufsize: %d packet size: %ld\n", rxbufsize, size);
 
     while (bytes_to_copy) {
-- 
1.8.4.4

[Qemu-devel] [PATCH arm-devs v2 12/14] net/cadence_gem: Fix register w1c logic

[Peter Crosthwaite]

This write-1-clear logic was incorrect. It was always clearing w1c
bits regardless of whether the written value was 1 or not. i.e. it
was implementing a write-anything-to-clear strategy.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---

 hw/net/cadence_gem.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 1619507..f2c734e 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -1112,15 +1112,14 @@ static void gem_write(void *opaque, hwaddr offset, uint64_t val,
 
     /* Squash bits which are read only in write value */
     val &= ~(s->regs_ro[offset]);
-    /* Preserve (only) bits which are read only in register */
-    readonly = s->regs[offset];
-    readonly &= s->regs_ro[offset];
-
-    /* Squash bits which are write 1 to clear */
-    val &= ~(s->regs_w1c[offset] & val);
+    /* Preserve (only) bits which are read only and wtc in register */
+    readonly = s->regs[offset] & (s->regs_ro[offset] | s->regs_w1c[offset]);
 
     /* Copy register write to backing store */
-    s->regs[offset] = val | readonly;
+    s->regs[offset] = (val & ~s->regs_w1c[offset]) | readonly;
+
+    /* do w1c */
+    s->regs[offset] &= ~(s->regs_w1c[offset] & val);
 
     /* Handle register write side effects */
     switch (offset) {
-- 
1.8.4.4

[Qemu-devel] [PATCH arm-devs v2 13/14] net/cadence_gem: Improve can_receive debug printfery

[Peter Crosthwaite]

Currently this just floods indicating that can_receive has been called
by the net framework. Instead, save the result of the most recent
can_receive callback as state and only print a message if the result
changes (indicating some sort of actual state change in GEM). Make said
debug message more meaningful as well.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
changed since v1:
Fixed "cant" typo (PMM review)

 hw/net/cadence_gem.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index f2c734e..f6e38ca 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -380,6 +380,8 @@ typedef struct GemState {
     uint32_t rx_desc_addr;
     uint32_t tx_desc_addr;
 
+    uint8_t can_rx_state; /* Debug only */
+
     unsigned rx_desc[2];
 
     bool sar_active[4];
@@ -452,13 +454,19 @@ static int gem_can_receive(NetClientState *nc)
 
     s = qemu_get_nic_opaque(nc);
 
-    DB_PRINT("\n");
-
     /* Do nothing if receive is not enabled. */
     if (!(s->regs[GEM_NWCTRL] & GEM_NWCTRL_RXENA)) {
+        if (s->can_rx_state != 1) {
+            s->can_rx_state = 1;
+            DB_PRINT("can't receive - no enable\n");
+        }
         return 0;
     }
 
+    if (s->can_rx_state != 0) {
+        s->can_rx_state = 0;
+        DB_PRINT("can receive 0x%x\n", s->rx_desc_addr);
+    }
     return 1;
 }
 
-- 
1.8.4.4

[Qemu-devel] [PATCH arm-devs v2 14/14] net/cadence_gem: Don't rx packets when no rx buffer available

[Peter Crosthwaite]

Return false from can_receive() when no valid buffer descriptor is
available. Ensures against mass packet droppage in some applications.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
changed since v1:
Typos ("dont" "cant" "recieve") (PMM review)

 hw/net/cadence_gem.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index f6e38ca..92dc2f2 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -463,6 +463,15 @@ static int gem_can_receive(NetClientState *nc)
         return 0;
     }
 
+    if (rx_desc_get_ownership(s->rx_desc) == 1) {
+        if (s->can_rx_state != 2) {
+            s->can_rx_state = 2;
+            DB_PRINT("can't receive - busy buffer descriptor 0x%x\n",
+                     s->rx_desc_addr);
+        }
+        return 0;
+    }
+
     if (s->can_rx_state != 0) {
         s->can_rx_state = 0;
         DB_PRINT("can receive 0x%x\n", s->rx_desc_addr);
@@ -1142,7 +1151,7 @@ static void gem_write(void *opaque, hwaddr offset, uint64_t val,
             /* Reset to start of Q when transmit disabled. */
             s->tx_desc_addr = s->regs[GEM_TXQBASE];
         }
-        if (val & GEM_NWCTRL_RXENA) {
+        if (gem_can_receive(qemu_get_queue(s->nic))) {
             qemu_flush_queued_packets(qemu_get_queue(s->nic));
         }
         break;
-- 
1.8.4.4

Re: [Qemu-devel] [PATCH arm-devs v2 00/14] Cadence GEM Bugfixes and missing features

[Peter Maydell]

On 4 December 2013 05:53, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> Hi Peter,
>
> This series updated cadence GEM (ethernet in Zynq) with a number of
> bugfixes and enhancements. Most of this has come out of some stress
> testing we have done internally.
>
> I have also testing briefly using mainline Linux GEM driver.

Thanks; applied all to target-arm.next.

-- PMM