[Qemu-devel] [PATCH v2 0/4] net: ftgmac100 enhancements

[Qemu-devel] [PATCH v2 0/4] net: ftgmac100 enhancements

[Cédric Le Goater]

Hello,

Here is a couple of enhancements and fixes for the ftgmac100 NIC used
on the Aspeed SoC. It includes VLAN and multicast support.

Thanks,

Cédric.

Cédric Le Goater (4):
  ftgmac100: compute maximum frame size depending on the protocol
  ftgmac100: add IEEE 802.1Q VLAN support
  ftgmac100: fix multicast hash routine
  ftgmac100: remove check on runt messages

 include/hw/net/ftgmac100.h |  7 ++++-
 hw/net/ftgmac100.c         | 64 +++++++++++++++++++++++++++++++---------------
 2 files changed, 50 insertions(+), 21 deletions(-)

-- 
2.13.6

[Qemu-devel] [PATCH v2 1/4] ftgmac100: compute maximum frame size depending on the protocol

[Cédric Le Goater]

The maximum frame size includes the CRC and depends if a VLAN tag is
inserted or not. Adjust the frame size limit in the transmit handler
using on the FTGMAC100State buffer size and in the receive handler use
the packet protocol.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/net/ftgmac100.h |  7 ++++++-
 hw/net/ftgmac100.c         | 23 ++++++++++++-----------
 2 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/include/hw/net/ftgmac100.h b/include/hw/net/ftgmac100.h
index d9bc589fbf72..94cfe0533297 100644
--- a/include/hw/net/ftgmac100.h
+++ b/include/hw/net/ftgmac100.h
@@ -16,6 +16,11 @@
 #include "hw/sysbus.h"
 #include "net/net.h"
 
+/*
+ * Max frame size for the receiving buffer
+ */
+#define FTGMAC100_MAX_FRAME_SIZE    9220
+
 typedef struct FTGMAC100State {
     /*< private >*/
     SysBusDevice parent_obj;
@@ -26,7 +31,7 @@ typedef struct FTGMAC100State {
     qemu_irq irq;
     MemoryRegion iomem;
 
-    uint8_t *frame;
+    uint8_t frame[FTGMAC100_MAX_FRAME_SIZE];
 
     uint32_t irq_state;
     uint32_t isr;
diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c
index 3300e8ef4a80..425ac36cff87 100644
--- a/hw/net/ftgmac100.c
+++ b/hw/net/ftgmac100.c
@@ -207,16 +207,18 @@ typedef struct {
 /*
  * Max frame size for the receiving buffer
  */
-#define FTGMAC100_MAX_FRAME_SIZE    10240
+#define FTGMAC100_MAX_FRAME_SIZE    9220
 
 /* Limits depending on the type of the frame
  *
  *   9216 for Jumbo frames (+ 4 for VLAN)
  *   1518 for other frames (+ 4 for VLAN)
  */
-static int ftgmac100_max_frame_size(FTGMAC100State *s)
+static int ftgmac100_max_frame_size(FTGMAC100State *s, uint16_t proto)
 {
-    return (s->maccr & FTGMAC100_MACCR_JUMBO_LF ? 9216 : 1518) + 4;
+    int max = (s->maccr & FTGMAC100_MACCR_JUMBO_LF ? 9216 : 1518);
+
+    return max + (proto == ETH_P_VLAN ? 4 : 0);
 }
 
 static void ftgmac100_update_irq(FTGMAC100State *s)
@@ -408,7 +410,6 @@ static void ftgmac100_do_tx(FTGMAC100State *s, uint32_t tx_ring,
     uint8_t *ptr = s->frame;
     uint32_t addr = tx_descriptor;
     uint32_t flags = 0;
-    int max_frame_size = ftgmac100_max_frame_size(s);
 
     while (1) {
         FTGMAC100Desc bd;
@@ -427,11 +428,12 @@ static void ftgmac100_do_tx(FTGMAC100State *s, uint32_t tx_ring,
             flags = bd.des1;
         }
 
-        len = bd.des0 & 0x3FFF;
-        if (frame_size + len > max_frame_size) {
+        len = FTGMAC100_TXDES0_TXBUF_SIZE(bd.des0);
+        if (frame_size + len > sizeof(s->frame)) {
             qemu_log_mask(LOG_GUEST_ERROR, "%s: frame too big : %d bytes\n",
                           __func__, len);
-            len = max_frame_size - frame_size;
+            s->isr |= FTGMAC100_INT_XPKT_LOST;
+            len =  sizeof(s->frame) - frame_size;
         }
 
         if (dma_memory_read(&address_space_memory, bd.des3, ptr, len)) {
@@ -788,7 +790,8 @@ static ssize_t ftgmac100_receive(NetClientState *nc, const uint8_t *buf,
     uint32_t buf_len;
     size_t size = len;
     uint32_t first = FTGMAC100_RXDES0_FRS;
-    int max_frame_size = ftgmac100_max_frame_size(s);
+    uint16_t proto = be16_to_cpu(PKT_GET_ETH_HDR(buf)->h_proto);
+    int max_frame_size = ftgmac100_max_frame_size(s, proto);
 
     if ((s->maccr & (FTGMAC100_MACCR_RXDMA_EN | FTGMAC100_MACCR_RXMAC_EN))
          != (FTGMAC100_MACCR_RXDMA_EN | FTGMAC100_MACCR_RXMAC_EN)) {
@@ -820,9 +823,9 @@ static ssize_t ftgmac100_receive(NetClientState *nc, const uint8_t *buf,
 
     /* Huge frames are truncated.  */
     if (size > max_frame_size) {
-        size = max_frame_size;
         qemu_log_mask(LOG_GUEST_ERROR, "%s: frame too big : %zd bytes\n",
                       __func__, size);
+        size = max_frame_size;
         flags |= FTGMAC100_RXDES0_FTL;
     }
 
@@ -940,8 +943,6 @@ static void ftgmac100_realize(DeviceState *dev, Error **errp)
                           object_get_typename(OBJECT(dev)), DEVICE(dev)->id,
                           s);
     qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a);
-
-    s->frame = g_malloc(FTGMAC100_MAX_FRAME_SIZE);
 }
 
 static const VMStateDescription vmstate_ftgmac100 = {
-- 
2.13.6

[Qemu-devel] [PATCH v2 2/4] ftgmac100: add IEEE 802.1Q VLAN support

[Cédric Le Goater]

The ftgmac100 NIC supports VLAN tag insertion and the MAC engine also
has a control to remove VLAN tags from received packets.

The VLAN control bits and VLAN tag information are contained in the
second word of the transmit and receive descriptors. The Insert VLAN
bit and the VLAN Tag available bit are only valid in the first segment
of the packet.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---

 Changes since v1:

 - replaced byte shifting by store/load helpers
 
 hw/net/ftgmac100.c | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c
index 425ac36cff87..abf80655f28f 100644
--- a/hw/net/ftgmac100.c
+++ b/hw/net/ftgmac100.c
@@ -443,6 +443,22 @@ static void ftgmac100_do_tx(FTGMAC100State *s, uint32_t tx_ring,
             break;
         }
 
+        /* Check for VLAN */
+        if (bd.des0 & FTGMAC100_TXDES0_FTS &&
+            bd.des1 & FTGMAC100_TXDES1_INS_VLANTAG &&
+            be16_to_cpu(PKT_GET_ETH_HDR(ptr)->h_proto) != ETH_P_VLAN) {
+            if (frame_size + len + 4 > sizeof(s->frame)) {
+                qemu_log_mask(LOG_GUEST_ERROR, "%s: frame too big : %d bytes\n",
+                              __func__, len);
+                s->isr |= FTGMAC100_INT_XPKT_LOST;
+                len =  sizeof(s->frame) - frame_size - 4;
+            }
+            memmove(ptr + 16, ptr + 12, len - 12);
+            stw_be_p(ptr + 12, ETH_P_VLAN);
+            stw_be_p(ptr + 14, bd.des1);
+            len += 4;
+        }
+
         ptr += len;
         frame_size += len;
         if (bd.des0 & FTGMAC100_TXDES0_LTS) {
@@ -864,7 +880,20 @@ static ssize_t ftgmac100_receive(NetClientState *nc, const uint8_t *buf,
             buf_len += size - 4;
         }
         buf_addr = bd.des3;
-        dma_memory_write(&address_space_memory, buf_addr, buf, buf_len);
+        if (first && proto == ETH_P_VLAN && buf_len >= 18) {
+            bd.des1 = lduw_be_p(buf + 14) | FTGMAC100_RXDES1_VLANTAG_AVAIL;
+
+            if (s->maccr & FTGMAC100_MACCR_RM_VLAN) {
+                dma_memory_write(&address_space_memory, buf_addr, buf, 12);
+                dma_memory_write(&address_space_memory, buf_addr + 12, buf + 16,
+                                 buf_len - 16);
+            } else {
+                dma_memory_write(&address_space_memory, buf_addr, buf, buf_len);
+            }
+        } else {
+            bd.des1 = 0;
+            dma_memory_write(&address_space_memory, buf_addr, buf, buf_len);
+        }
         buf += buf_len;
         if (size < 4) {
             dma_memory_write(&address_space_memory, buf_addr + buf_len,
-- 
2.13.6

[Qemu-devel] [PATCH v2 3/4] ftgmac100: fix multicast hash routine

[Cédric Le Goater]

Based on the multicast hash calculation of the FTGMAC100 Linux driver.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/net/ftgmac100.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c
index abf80655f28f..8a7f274dc118 100644
--- a/hw/net/ftgmac100.c
+++ b/hw/net/ftgmac100.c
@@ -776,8 +776,8 @@ static int ftgmac100_filter(FTGMAC100State *s, const uint8_t *buf, size_t len)
                 return 0;
             }
 
-            /* TODO: this does not seem to work for ftgmac100 */
-            mcast_idx = net_crc32(buf, ETH_ALEN) >> 26;
+            mcast_idx = net_crc32_le(buf, ETH_ALEN);
+            mcast_idx = (~(mcast_idx >> 2)) & 0x3f;
             if (!(s->math[mcast_idx / 32] & (1 << (mcast_idx % 32)))) {
                 return 0;
             }
-- 
2.13.6

[Qemu-devel] [PATCH v2 4/4] ftgmac100: remove check on runt messages

[Cédric Le Goater]

This is a ethernet wire limitation not needed in emulation. It breaks
U-Boot n/w stack also.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/net/ftgmac100.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c
index 8a7f274dc118..909c1182eebe 100644
--- a/hw/net/ftgmac100.c
+++ b/hw/net/ftgmac100.c
@@ -822,12 +822,6 @@ static ssize_t ftgmac100_receive(NetClientState *nc, const uint8_t *buf,
         return size;
     }
 
-    if (size < 64 && !(s->maccr & FTGMAC100_MACCR_RX_RUNT)) {
-        qemu_log_mask(LOG_GUEST_ERROR, "%s: dropped runt frame of %zd bytes\n",
-                      __func__, size);
-        return size;
-    }
-
     if (!ftgmac100_filter(s, buf, size)) {
         return size;
     }
-- 
2.13.6

Re: [Qemu-devel] [PATCH v2 2/4] ftgmac100: add IEEE 802.1Q VLAN support

[Peter Maydell]

On 30 May 2018 at 07:17, Cédric Le Goater <clg@kaod.org> wrote:
> The ftgmac100 NIC supports VLAN tag insertion and the MAC engine also
> has a control to remove VLAN tags from received packets.
>
> The VLAN control bits and VLAN tag information are contained in the
> second word of the transmit and receive descriptors. The Insert VLAN
> bit and the VLAN Tag available bit are only valid in the first segment
> of the packet.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> +        /* Check for VLAN */
> +        if (bd.des0 & FTGMAC100_TXDES0_FTS &&
> +            bd.des1 & FTGMAC100_TXDES1_INS_VLANTAG &&
> +            be16_to_cpu(PKT_GET_ETH_HDR(ptr)->h_proto) != ETH_P_VLAN) {

This is kind of bogus because there's no guarantee that h_proto here
is sufficiently aligned to do a halfword load. However this is
a problem common to all the PKT_GET_* macros so we should probably
attack it separately to this series.

Getting the ethernet h_proto field seems common enough that
something like

static inline uint16_t eth_get_proto(uint8_t *pkt)
{
    return lduw_be_p(&PKT_GET_ETH_HDR(pkt)->h_proto);
}

would be handy in eth.h.


Anyway, I've applied this series to target-arm.next.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 2/4] ftgmac100: add IEEE 802.1Q VLAN support

[Cédric Le Goater]

On 06/07/2018 02:41 PM, Peter Maydell wrote:
> On 30 May 2018 at 07:17, Cédric Le Goater <clg@kaod.org> wrote:
>> The ftgmac100 NIC supports VLAN tag insertion and the MAC engine also
>> has a control to remove VLAN tags from received packets.
>>
>> The VLAN control bits and VLAN tag information are contained in the
>> second word of the transmit and receive descriptors. The Insert VLAN
>> bit and the VLAN Tag available bit are only valid in the first segment
>> of the packet.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
>> +        /* Check for VLAN */
>> +        if (bd.des0 & FTGMAC100_TXDES0_FTS &&
>> +            bd.des1 & FTGMAC100_TXDES1_INS_VLANTAG &&
>> +            be16_to_cpu(PKT_GET_ETH_HDR(ptr)->h_proto) != ETH_P_VLAN) {
> 
> This is kind of bogus because there's no guarantee that h_proto here
> is sufficiently aligned to do a halfword load. However this is
> a problem common to all the PKT_GET_* macros so we should probably
> attack it separately to this series.
> 
> Getting the ethernet h_proto field seems common enough that
> something like
> 
> static inline uint16_t eth_get_proto(uint8_t *pkt)
> {
>     return lduw_be_p(&PKT_GET_ETH_HDR(pkt)->h_proto);
> }
> 
> would be handy in eth.h.

yes. There are a few candidates :

hw/net/virtio-net.c:        int vid = lduw_be_p(ptr + 14) & 0xfff;
hw/net/e1000e_core.c:        uint16_t vid = lduw_be_p(buf + 14);
hw/net/rtl8139.c:            lduw_be_p(&buf[ETH_ALEN * 2]) == ETH_P_VLAN) {
hw/net/e1000x_common.c:    uint16_t eth_proto = lduw_be_p(buf + 12);
hw/net/e1000.c:        uint16_t vid = lduw_be_p(buf + 14);
hw/net/e1000.c:        vlan_special = cpu_to_le16(lduw_be_p(filter_buf + 14));

Thanks,

C. 

> Anyway, I've applied this series to target-arm.next.
> 
> thanks
> -- PMM
>