All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/4] FTGMAC100 nic model for the Aspeed SoCs
@ 2017-04-01 12:57 Cédric Le Goater
  2017-04-01 12:57 ` [Qemu-devel] [PATCH 1/4] net: add FTGMAC100 support Cédric Le Goater
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Cédric Le Goater @ 2017-04-01 12:57 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Jason Wang, Dmitry Fleytman, Samuel Thibault, Jan Kiszka,
	qemu-devel, qemu-arm, Cédric Le Goater

Hi,

The Aspeed SoCs AST2400 and AST2500 have two FTGMAC100 ethernet
controllers. This serie proposes a model for this device and a way to
customize the bit definitions which are slightly different from the
Faraday definitions.

The last patch adds a fake NC-SI (Network Controller Sideband
Interface) backend to pretend a NIC is being managed. This is only
usable with the slirp stack.

The model has been tested on the 'palmetto' and the 'romulus' machine
using different implementations of the Linux driver and with U-Boot.
It has been stressed with iperf.

Thanks,

C. 

Cédric Le Goater (4):
  net: add FTGMAC100 support
  net/ftgmac100: add a 'aspeed' property
  aspeed: add a FTGMAC100 nic
  slirp: add a fake NC-SI backend

 default-configs/arm-softmmu.mak |   1 +
 hw/arm/aspeed_soc.c             |  21 +
 hw/net/Makefile.objs            |   1 +
 hw/net/ftgmac100.c              | 990 ++++++++++++++++++++++++++++++++++++++++
 include/hw/arm/aspeed_soc.h     |   2 +
 include/hw/net/ftgmac100.h      |  62 +++
 include/hw/net/mii.h            |   6 +
 include/net/eth.h               |   1 +
 slirp/Makefile.objs             |   2 +-
 slirp/ncsi-pkt.h                | 418 +++++++++++++++++
 slirp/ncsi.c                    |  78 ++++
 slirp/slirp.c                   |   4 +
 slirp/slirp.h                   |   3 +
 13 files changed, 1588 insertions(+), 1 deletion(-)
 create mode 100644 hw/net/ftgmac100.c
 create mode 100644 include/hw/net/ftgmac100.h
 create mode 100644 slirp/ncsi-pkt.h
 create mode 100644 slirp/ncsi.c

-- 
2.7.4

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH 1/4] net: add FTGMAC100 support
  2017-04-01 12:57 [Qemu-devel] [PATCH 0/4] FTGMAC100 nic model for the Aspeed SoCs Cédric Le Goater
@ 2017-04-01 12:57 ` Cédric Le Goater
  2017-04-10 13:43   ` Peter Maydell
  2017-04-01 12:57 ` [Qemu-devel] [PATCH 2/4] net/ftgmac100: add a 'aspeed' property Cédric Le Goater
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Cédric Le Goater @ 2017-04-01 12:57 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Jason Wang, Dmitry Fleytman, Samuel Thibault, Jan Kiszka,
	qemu-devel, qemu-arm, Cédric Le Goater

The FTGMAC100 device is an Ethernet controller with DMA function that
can be found on Aspeed SoCs (which include NCSI).

It is fully compliant with IEEE 802.3 specification for 10/100 Mbps
Ethernet and IEEE 802.3z specification for 1000 Mbps Ethernet and
includes Reduced Media Independent Interface (RMII) and Reduced
Gigabit Media Independent Interface (RGMII) interfaces. It adopts an
AHB bus interface and integrates a link list DMA engine with direct
M-Bus accesses for transmitting and receiving packets. It has
independent TX/RX fifos, supports half and full duplex (1000 Mbps mode
only supports full duplex), flow control for full duplex and
backpressure for half duplex.

The FTGMAC100 also implements IP, TCP, UDP checksum offloads and
supports IEEE 802.1Q VLAN tag insertion and removal. It offers
high-priority transmit queue for QoS and CoS applications

This model is complete enough to satisfy two different Linux drivers
and a U-Boot driver. Not supported features are :

 - IEEE 802.1Q VLAN
 - High Priority Transmit Queue
 - Wake-On-LAN functions

The code is based on the Coldfire Fast Ethernet Controller model.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 default-configs/arm-softmmu.mak |   1 +
 hw/net/Makefile.objs            |   1 +
 hw/net/ftgmac100.c              | 977 ++++++++++++++++++++++++++++++++++++++++
 include/hw/net/ftgmac100.h      |  58 +++
 include/hw/net/mii.h            |   6 +
 5 files changed, 1043 insertions(+)
 create mode 100644 hw/net/ftgmac100.c
 create mode 100644 include/hw/net/ftgmac100.h

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 15a53598d1c3..acc4c6c86297 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -30,6 +30,7 @@ CONFIG_LAN9118=y
 CONFIG_SMC91C111=y
 CONFIG_ALLWINNER_EMAC=y
 CONFIG_IMX_FEC=y
+CONFIG_FTGMAC100=y
 CONFIG_DS1338=y
 CONFIG_RX8900=y
 CONFIG_PFLASH_CFI01=y
diff --git a/hw/net/Makefile.objs b/hw/net/Makefile.objs
index 6a95d92d37f8..5ddaffe63a46 100644
--- a/hw/net/Makefile.objs
+++ b/hw/net/Makefile.objs
@@ -26,6 +26,7 @@ common-obj-$(CONFIG_IMX_FEC) += imx_fec.o
 common-obj-$(CONFIG_CADENCE) += cadence_gem.o
 common-obj-$(CONFIG_STELLARIS_ENET) += stellaris_enet.o
 common-obj-$(CONFIG_LANCE) += lance.o
+common-obj-$(CONFIG_FTGMAC100) += ftgmac100.o
 
 obj-$(CONFIG_ETRAXFS) += etraxfs_eth.o
 obj-$(CONFIG_COLDFIRE) += mcf_fec.o
diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c
new file mode 100644
index 000000000000..331e87391962
--- /dev/null
+++ b/hw/net/ftgmac100.c
@@ -0,0 +1,977 @@
+/*
+ * Faraday FTGMAC100 Gigabit Ethernet
+ *
+ * Copyright (C) 2016 IBM Corp.
+ *
+ * Based on Coldfire Fast Ethernet Controller emulation.
+ *
+ * Copyright (c) 2007 CodeSourcery.
+ *
+ * This code is licensed under the GPL version 2 or later. See the
+ * COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/net/ftgmac100.h"
+#include "sysemu/dma.h"
+#include "qemu/log.h"
+#include "net/checksum.h"
+#include "net/eth.h"
+#include "hw/net/mii.h"
+
+/* For crc32 */
+#include <zlib.h>
+
+/*
+ * FTGMAC100 registers
+ */
+#define FTGMAC100_ISR             0x00
+#define FTGMAC100_IER             0x04
+#define FTGMAC100_MAC_MADR        0x08
+#define FTGMAC100_MAC_LADR        0x0c
+#define FTGMAC100_MATH0           0x10
+#define FTGMAC100_MATH1           0x14
+#define FTGMAC100_NPTXPD          0x18
+#define FTGMAC100_RXPD            0x1C
+#define FTGMAC100_NPTXR_BADR      0x20
+#define FTGMAC100_RXR_BADR        0x24
+#define FTGMAC100_HPTXPD          0x28 /* TODO */
+#define FTGMAC100_HPTXR_BADR      0x2c /* TODO */
+#define FTGMAC100_ITC             0x30
+#define FTGMAC100_APTC            0x34
+#define FTGMAC100_DBLAC           0x38
+#define FTGMAC100_REVR            0x40
+#define FTGMAC100_FEAR1           0x44
+#define FTGMAC100_RBSR            0x4c
+#define FTGMAC100_TPAFCR          0x48
+
+#define FTGMAC100_MACCR           0x50
+#define FTGMAC100_MACSR           0x54 /* TODO */
+#define FTGMAC100_PHYCR           0x60
+#define FTGMAC100_PHYDATA         0x64
+#define FTGMAC100_FCR             0x68
+
+/*
+ * Interrupt status register & interrupt enable register
+ */
+#define FTGMAC100_INT_RPKT_BUF    (1 << 0)
+#define FTGMAC100_INT_RPKT_FIFO   (1 << 1)
+#define FTGMAC100_INT_NO_RXBUF    (1 << 2)
+#define FTGMAC100_INT_RPKT_LOST   (1 << 3)
+#define FTGMAC100_INT_XPKT_ETH    (1 << 4)
+#define FTGMAC100_INT_XPKT_FIFO   (1 << 5)
+#define FTGMAC100_INT_NO_NPTXBUF  (1 << 6)
+#define FTGMAC100_INT_XPKT_LOST   (1 << 7)
+#define FTGMAC100_INT_AHB_ERR     (1 << 8)
+#define FTGMAC100_INT_PHYSTS_CHG  (1 << 9)
+#define FTGMAC100_INT_NO_HPTXBUF  (1 << 10)
+
+/*
+ * Automatic polling timer control register
+ */
+#define FTGMAC100_APTC_RXPOLL_CNT(x)        ((x) & 0xf)
+#define FTGMAC100_APTC_RXPOLL_TIME_SEL      (1 << 4)
+#define FTGMAC100_APTC_TXPOLL_CNT(x)        (((x) >> 8) & 0xf)
+#define FTGMAC100_APTC_TXPOLL_TIME_SEL      (1 << 12)
+
+/*
+ * PHY control register
+ */
+#define FTGMAC100_PHYCR_MIIRD               (1 << 26)
+#define FTGMAC100_PHYCR_MIIWR               (1 << 27)
+
+#define FTGMAC100_PHYCR_DEV(v)              (((v) >> 16) & 0x1f)
+#define FTGMAC100_PHYCR_REG(v)              (((v) >> 21) & 0x1f)
+
+/*
+ * PHY data register
+ */
+#define FTGMAC100_PHYDATA_MIIWDATA(x)       ((x) & 0xffff)
+#define FTGMAC100_PHYDATA_MIIRDATA(phydata) (((phydata) >> 16) & 0xffff)
+
+/*
+ * Feature Register
+ */
+#define FTGMAC100_REVR_NEW_MDIO_INTERFACE   (1 << 31)
+
+/*
+ * MAC control register
+ */
+#define FTGMAC100_MACCR_TXDMA_EN         (1 << 0)
+#define FTGMAC100_MACCR_RXDMA_EN         (1 << 1)
+#define FTGMAC100_MACCR_TXMAC_EN         (1 << 2)
+#define FTGMAC100_MACCR_RXMAC_EN         (1 << 3)
+#define FTGMAC100_MACCR_RM_VLAN          (1 << 4)
+#define FTGMAC100_MACCR_HPTXR_EN         (1 << 5)
+#define FTGMAC100_MACCR_LOOP_EN          (1 << 6)
+#define FTGMAC100_MACCR_ENRX_IN_HALFTX   (1 << 7)
+#define FTGMAC100_MACCR_FULLDUP          (1 << 8)
+#define FTGMAC100_MACCR_GIGA_MODE        (1 << 9)
+#define FTGMAC100_MACCR_CRC_APD          (1 << 10) /* not needed */
+#define FTGMAC100_MACCR_RX_RUNT          (1 << 12)
+#define FTGMAC100_MACCR_JUMBO_LF         (1 << 13)
+#define FTGMAC100_MACCR_RX_ALL           (1 << 14)
+#define FTGMAC100_MACCR_HT_MULTI_EN      (1 << 15)
+#define FTGMAC100_MACCR_RX_MULTIPKT      (1 << 16)
+#define FTGMAC100_MACCR_RX_BROADPKT      (1 << 17)
+#define FTGMAC100_MACCR_DISCARD_CRCERR   (1 << 18)
+#define FTGMAC100_MACCR_FAST_MODE        (1 << 19)
+#define FTGMAC100_MACCR_SW_RST           (1 << 31)
+
+/*
+ * Transmit descriptor, aligned to 16 bytes
+ */
+struct ftgmac100_txdes {
+        unsigned int        txdes0;
+        unsigned int        txdes1;
+        unsigned int        txdes2;      /* not used by HW */
+        unsigned int        txdes3;      /* TXBUF_BADR */
+} __attribute__ ((aligned(16)));
+
+#define FTGMAC100_TXDES0_TXBUF_SIZE(x)   ((x) & 0x3fff)
+#define FTGMAC100_TXDES0_EDOTR           (1 << 15)
+#define FTGMAC100_TXDES0_CRC_ERR         (1 << 19)
+#define FTGMAC100_TXDES0_LTS             (1 << 28)
+#define FTGMAC100_TXDES0_FTS             (1 << 29)
+#define FTGMAC100_TXDES0_TXDMA_OWN       (1 << 31)
+
+#define FTGMAC100_TXDES1_VLANTAG_CI(x)   ((x) & 0xffff)
+#define FTGMAC100_TXDES1_INS_VLANTAG     (1 << 16)
+#define FTGMAC100_TXDES1_TCP_CHKSUM      (1 << 17)
+#define FTGMAC100_TXDES1_UDP_CHKSUM      (1 << 18)
+#define FTGMAC100_TXDES1_IP_CHKSUM       (1 << 19)
+#define FTGMAC100_TXDES1_LLC             (1 << 22)
+#define FTGMAC100_TXDES1_TX2FIC          (1 << 30)
+#define FTGMAC100_TXDES1_TXIC            (1 << 31)
+
+/*
+ * Receive descriptor, aligned to 16 bytes
+ */
+struct ftgmac100_rxdes {
+        unsigned int        rxdes0;
+        unsigned int        rxdes1;
+        unsigned int        rxdes2;      /* not used by HW */
+        unsigned int        rxdes3;      /* RXBUF_BADR */
+} __attribute__ ((aligned(16)));
+
+#define FTGMAC100_RXDES0_VDBC            0x3fff
+#define FTGMAC100_RXDES0_EDORR           (1 << 15)
+#define FTGMAC100_RXDES0_MULTICAST       (1 << 16)
+#define FTGMAC100_RXDES0_BROADCAST       (1 << 17)
+#define FTGMAC100_RXDES0_RX_ERR          (1 << 18)
+#define FTGMAC100_RXDES0_CRC_ERR         (1 << 19)
+#define FTGMAC100_RXDES0_FTL             (1 << 20)
+#define FTGMAC100_RXDES0_RUNT            (1 << 21)
+#define FTGMAC100_RXDES0_RX_ODD_NB       (1 << 22)
+#define FTGMAC100_RXDES0_FIFO_FULL       (1 << 23)
+#define FTGMAC100_RXDES0_PAUSE_OPCODE    (1 << 24)
+#define FTGMAC100_RXDES0_PAUSE_FRAME     (1 << 25)
+#define FTGMAC100_RXDES0_LRS             (1 << 28)
+#define FTGMAC100_RXDES0_FRS             (1 << 29)
+#define FTGMAC100_RXDES0_RXPKT_RDY       (1 << 31)
+
+#define FTGMAC100_RXDES1_VLANTAG_CI      0xffff
+#define FTGMAC100_RXDES1_PROT_MASK       (0x3 << 20)
+#define FTGMAC100_RXDES1_PROT_NONIP      (0x0 << 20)
+#define FTGMAC100_RXDES1_PROT_IP         (0x1 << 20)
+#define FTGMAC100_RXDES1_PROT_TCPIP      (0x2 << 20)
+#define FTGMAC100_RXDES1_PROT_UDPIP      (0x3 << 20)
+#define FTGMAC100_RXDES1_LLC             (1 << 22)
+#define FTGMAC100_RXDES1_DF              (1 << 23)
+#define FTGMAC100_RXDES1_VLANTAG_AVAIL   (1 << 24)
+#define FTGMAC100_RXDES1_TCP_CHKSUM_ERR  (1 << 25)
+#define FTGMAC100_RXDES1_UDP_CHKSUM_ERR  (1 << 26)
+#define FTGMAC100_RXDES1_IP_CHKSUM_ERR   (1 << 27)
+
+/*
+ * PHY values (to be defined elsewhere ...)
+ */
+#define PHY_INT_ENERGYON            (1 << 7)
+#define PHY_INT_AUTONEG_COMPLETE    (1 << 6)
+#define PHY_INT_FAULT               (1 << 5)
+#define PHY_INT_DOWN                (1 << 4)
+#define PHY_INT_AUTONEG_LP          (1 << 3)
+#define PHY_INT_PARFAULT            (1 << 2)
+#define PHY_INT_AUTONEG_PAGE        (1 << 1)
+
+/* Common Buffer Descriptor  */
+typedef struct {
+    uint32_t        des0;
+    uint32_t        des1;
+    uint32_t        des2;        /* not used by HW */
+    uint32_t        des3;        /* TXBUF_BADR */
+} Ftgmac100Desc  __attribute__ ((aligned(16)));
+
+/* max frame size is :
+ *
+ *   9216 for Jumbo frames (+ 4 for VLAN)
+ *   1518 for other frames (+ 4 for VLAN)
+ */
+#define FTGMAC100_MAX_FRAME_SIZE(s)                             \
+    ((s->maccr & FTGMAC100_MACCR_JUMBO_LF ? 9216 : 1518) + 4)
+
+static void ftgmac100_update_irq(Ftgmac100State *s);
+
+/*
+ * The MII phy could raise a GPIO to the processor which in turn
+ * could be handled as an interrpt by the OS.
+ * For now we don't handle any GPIO/interrupt line, so the OS will
+ * have to poll for the PHY status.
+ */
+static void phy_update_irq(Ftgmac100State *s)
+{
+    ftgmac100_update_irq(s);
+}
+
+static void phy_update_link(Ftgmac100State *s)
+{
+    /* Autonegotiation status mirrors link status.  */
+    if (qemu_get_queue(s->nic)->link_down) {
+        s->phy_status &= ~0x0024;
+        s->phy_int |= PHY_INT_DOWN;
+    } else {
+        s->phy_status |= 0x0024;
+        s->phy_int |= PHY_INT_ENERGYON;
+        s->phy_int |= PHY_INT_AUTONEG_COMPLETE;
+    }
+    phy_update_irq(s);
+}
+
+static void ftgmac100_set_link(NetClientState *nc)
+{
+    phy_update_link(FTGMAC100(qemu_get_nic_opaque(nc)));
+}
+
+static void phy_reset(Ftgmac100State *s)
+{
+    s->phy_status = 0x796d;
+    s->phy_control = 0x1140;
+    s->phy_advertise = 0x0de1;
+    s->phy_int_mask = 0;
+    s->phy_int = 0;
+    phy_update_link(s);
+}
+
+static uint32_t do_phy_read(Ftgmac100State *s, int reg)
+{
+    uint32_t val;
+
+    switch (reg) {
+    case MII_BMCR: /* Basic Control */
+        val = s->phy_control;
+        break;
+    case MII_BMSR: /* Basic Status */
+        val = s->phy_status;
+        break;
+    case MII_PHYID1: /* ID1 */
+        val = RTL8211E_PHYID1;
+        break;
+    case MII_PHYID2: /* ID2 */
+        val = RTL8211E_PHYID2;
+        break;
+    case MII_ANAR: /* Auto-neg advertisement */
+        val = s->phy_advertise;
+        break;
+    case MII_ANLPAR: /* Auto-neg Link Partner Ability */
+        val = 0x45e1;
+        break;
+    case MII_ANER: /* Auto-neg Expansion */
+        val = 1;
+        break;
+    case MII_CTRL1000: /* 1000BASE-T control  */
+        val = 0x300;
+        break;
+    case MII_STAT1000: /* 1000BASE-T status  */
+        val = 0x800;
+        break;
+    case 29:    /* Interrupt source.  */
+        val = s->phy_int;
+        s->phy_int = 0;
+        phy_update_irq(s);
+        break;
+    case 30:    /* Interrupt mask */
+        val = s->phy_int_mask;
+        break;
+    case MII_LBREMR:
+    case MII_REC:
+    case 27:
+    case 31:
+        qemu_log_mask(LOG_UNIMP, "%s: reg %d not implemented\n",
+                      __func__, reg);
+        val = 0;
+        break;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad address at offset %d\n",
+                      __func__, reg);
+        val = 0;
+        break;
+    }
+
+    return val;
+}
+
+static void do_phy_write(Ftgmac100State *s, int reg, uint32_t val)
+{
+    switch (reg) {
+    case MII_BMCR:     /* Basic Control */
+        if (val & 0x8000) {
+            phy_reset(s);
+        } else {
+            s->phy_control = val & 0x7980;
+            /* Complete autonegotiation immediately.  */
+            if (val & 0x1000) {
+                s->phy_status |= 0x0020;
+            }
+        }
+        break;
+    case MII_ANAR:     /* Auto-neg advertisement */
+        s->phy_advertise = (val & 0x2d7f) | 0x80;
+        break;
+    case 30:    /* Interrupt mask */
+        s->phy_int_mask = val & 0xff;
+        phy_update_irq(s);
+        break;
+    case MII_LBREMR:
+    case MII_REC:
+    case 27:
+    case 31:
+        qemu_log_mask(LOG_UNIMP, "%s: reg %d not implemented\n",
+                      __func__, reg);
+        break;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad address at offset %d\n",
+                      __func__, reg);
+        break;
+    }
+}
+
+static void ftgmac100_read_bd(Ftgmac100Desc *bd, dma_addr_t addr)
+{
+    dma_memory_read(&address_space_memory, addr, bd, sizeof(*bd));
+    bd->des0 = le32_to_cpu(bd->des0);
+    bd->des1 = le32_to_cpu(bd->des1);
+    bd->des2 = le32_to_cpu(bd->des2);
+    bd->des3 = le32_to_cpu(bd->des3);
+}
+
+static void ftgmac100_write_bd(Ftgmac100Desc *bd, dma_addr_t addr)
+{
+    Ftgmac100Desc lebd;
+    lebd.des0 = cpu_to_le32(bd->des0);
+    lebd.des1 = cpu_to_le32(bd->des1);
+    lebd.des2 = cpu_to_le32(bd->des2);
+    lebd.des3 = cpu_to_le32(bd->des3);
+    dma_memory_write(&address_space_memory, addr, &lebd, sizeof(lebd));
+}
+
+static void ftgmac100_update_irq(Ftgmac100State *s)
+{
+    uint32_t active;
+    uint32_t changed;
+
+    active = s->isr & s->ier;
+    changed = active ^ s->irq_state;
+    if (changed) {
+        qemu_set_irq(s->irq, active);
+    }
+    s->irq_state = active;
+}
+
+/* Locate a possible first descriptor to transmit. When Linux resets
+ * the device, the indexes of ring buffers are cleared but the dma
+ * buffers are not, so we need to find a starting point.
+ */
+static uint32_t ftgmac100_find_txdes(Ftgmac100State *s, uint32_t addr)
+{
+    Ftgmac100Desc bd;
+
+    while (1) {
+        ftgmac100_read_bd(&bd, addr);
+        if (bd.des0 & (FTGMAC100_TXDES0_FTS | FTGMAC100_TXDES0_EDOTR)) {
+            break;
+        }
+        addr += sizeof(Ftgmac100Desc);
+    }
+    return addr;
+}
+
+static void ftgmac100_do_tx(Ftgmac100State *s, uint32_t tx_ring,
+                            uint32_t tx_descriptor)
+{
+    int frame_size = 0;
+    uint8_t frame[FTGMAC100_MAX_FRAME_SIZE(s)];
+    uint8_t *ptr = frame;
+    uint32_t addr;
+    uint32_t flags = 0;
+
+    addr = ftgmac100_find_txdes(s, tx_descriptor);
+
+    while (1) {
+        Ftgmac100Desc bd;
+        int len;
+
+        ftgmac100_read_bd(&bd, addr);
+        if ((bd.des0 & FTGMAC100_TXDES0_TXDMA_OWN) == 0) {
+            /* Run out of descriptors to transmit.  */
+            s->isr |= FTGMAC100_INT_NO_NPTXBUF;
+            break;
+        }
+
+        /* record transmit flags as they are valid only on the first
+         * segment */
+        if (bd.des0 & FTGMAC100_TXDES0_FTS) {
+            flags = bd.des1;
+        }
+
+        len = bd.des0 & 0x3FFF;
+        if (frame_size + len > FTGMAC100_MAX_FRAME_SIZE(s)) {
+            qemu_log_mask(LOG_GUEST_ERROR, "%s: frame too big : %d bytes\n",
+                          __func__, len);
+            len = FTGMAC100_MAX_FRAME_SIZE(s) - frame_size;
+        }
+        dma_memory_read(&address_space_memory, bd.des3, ptr, len);
+        ptr += len;
+        frame_size += len;
+        if (bd.des0 & FTGMAC100_TXDES0_LTS) {
+            if (flags & FTGMAC100_TXDES1_IP_CHKSUM) {
+                net_checksum_calculate(frame, frame_size);
+            }
+            /* Last buffer in frame.  */
+            qemu_send_packet(qemu_get_queue(s->nic), frame, frame_size);
+            ptr = frame;
+            frame_size = 0;
+            if (flags & FTGMAC100_TXDES1_TXIC) {
+                s->isr |= FTGMAC100_INT_XPKT_ETH;
+            }
+        }
+
+        if (flags & FTGMAC100_TXDES1_TX2FIC) {
+            s->isr |= FTGMAC100_INT_XPKT_FIFO;
+        }
+        bd.des0 &= ~FTGMAC100_TXDES0_TXDMA_OWN;
+
+        /* Write back the modified descriptor.  */
+        ftgmac100_write_bd(&bd, addr);
+        /* Advance to the next descriptor.  */
+        if (bd.des0 & FTGMAC100_TXDES0_EDOTR) {
+            addr = tx_ring;
+        } else {
+            addr += sizeof(Ftgmac100Desc);
+        }
+    }
+
+    s->tx_descriptor = addr;
+
+    ftgmac100_update_irq(s);
+}
+
+static int ftgmac100_can_receive(NetClientState *nc)
+{
+    Ftgmac100State *s = FTGMAC100(qemu_get_nic_opaque(nc));
+    Ftgmac100Desc bd;
+
+    if ((s->maccr & (FTGMAC100_MACCR_RXDMA_EN | FTGMAC100_MACCR_RXMAC_EN))
+         != (FTGMAC100_MACCR_RXDMA_EN | FTGMAC100_MACCR_RXMAC_EN)) {
+        return 0;
+    }
+
+    ftgmac100_read_bd(&bd, s->rx_descriptor);
+    return !(bd.des0 & FTGMAC100_RXDES0_RXPKT_RDY);
+}
+
+/*
+ * This is purely informative. The HW can poll the RW (and RX) ring
+ * buffers for available descriptors but we don't need to trigger a
+ * timer for that in qemu.
+ */
+static uint32_t ftgmac100_rxpoll(Ftgmac100State *s)
+{
+    /* Polling times :
+     *
+     * Speed      TIME_SEL=0    TIME_SEL=1
+     *
+     *    10         51.2 ms      819.2 ms
+     *   100         5.12 ms      81.92 ms
+     *  1000        1.024 ms     16.384 ms
+     */
+    static const int div[] = { 20, 200, 1000 };
+
+    uint32_t cnt = 1024 * FTGMAC100_APTC_RXPOLL_CNT(s->aptcr);
+    uint32_t speed = (s->maccr & FTGMAC100_MACCR_FAST_MODE) ? 1 : 0;
+    uint32_t period;
+
+    if (s->aptcr & FTGMAC100_APTC_RXPOLL_TIME_SEL) {
+        cnt <<= 4;
+    }
+
+    if (s->maccr & FTGMAC100_MACCR_GIGA_MODE) {
+        speed = 2;
+    }
+
+    period = cnt / div[speed];
+
+    return period;
+}
+
+static void ftgmac100_reset(DeviceState *d)
+{
+    Ftgmac100State *s = FTGMAC100(d);
+
+    /* Reset the FTGMAC100 */
+    s->isr = 0;
+    s->ier = 0;
+    s->rx_enabled = 0;
+    s->rx_ring = 0;
+    s->rbsr = 0x640;
+    s->rx_descriptor = 0;
+    s->tx_ring = 0;
+    s->tx_descriptor = 0;
+    s->math[0] = 0;
+    s->math[1] = 0;
+    s->itc = 0;
+    s->aptcr = 1;
+    s->dblac = 0x00022f00;
+    s->revr = 0;
+    s->fear1 = 0;
+    s->tpafcr = 0xf1;
+
+    s->maccr = 0;
+    s->phycr = 0;
+    s->phydata = 0;
+    s->fcr = 0x400;
+
+    /* and the PHY */
+    phy_reset(s);
+
+    ftgmac100_update_irq(s);
+}
+
+static uint64_t ftgmac100_read(void *opaque, hwaddr addr, unsigned size)
+{
+    Ftgmac100State *s = FTGMAC100(opaque);
+
+    switch (addr & 0xff) {
+    case FTGMAC100_ISR:
+        return s->isr;
+    case FTGMAC100_IER:
+        return s->ier;
+    case FTGMAC100_MAC_MADR:
+        return (s->conf.macaddr.a[0] << 8)  | s->conf.macaddr.a[1];
+    case FTGMAC100_MAC_LADR:
+        return (s->conf.macaddr.a[2] << 24) | (s->conf.macaddr.a[3] << 16) |
+               (s->conf.macaddr.a[4] << 8)  |  s->conf.macaddr.a[5];
+    case FTGMAC100_MATH0:
+        return s->math[0];
+    case FTGMAC100_MATH1:
+        return s->math[1];
+    case FTGMAC100_ITC:
+        return s->itc;
+    case FTGMAC100_DBLAC:
+        return s->dblac;
+    case FTGMAC100_REVR:
+        return s->revr;
+    case FTGMAC100_FEAR1:
+        return s->fear1;
+    case FTGMAC100_TPAFCR:
+        return s->tpafcr;
+    case FTGMAC100_FCR:
+        return s->fcr;
+    case FTGMAC100_MACCR:
+        return s->maccr;
+    case FTGMAC100_PHYCR:
+        return s->phycr;
+    case FTGMAC100_PHYDATA:
+        return s->phydata;
+
+    case FTGMAC100_MACSR: /* MAC Status Register (MACSR) */
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad address at offset 0x%"
+                      HWADDR_PRIx "\n", __func__, addr);
+        return 0;
+    }
+}
+
+static void ftgmac100_write(void *opaque, hwaddr addr,
+                          uint64_t value, unsigned size)
+{
+    Ftgmac100State *s = FTGMAC100(opaque);
+    int reg;
+
+    switch (addr & 0xff) {
+    case FTGMAC100_ISR: /* Interrupt status */
+        s->isr &= ~value;
+        break;
+    case FTGMAC100_IER: /* Interrupt control */
+        s->ier = value;
+        break;
+    case FTGMAC100_MAC_MADR: /* MAC */
+        s->conf.macaddr.a[0] = value >> 8;
+        s->conf.macaddr.a[1] = value;
+        break;
+    case FTGMAC100_MAC_LADR:
+        s->conf.macaddr.a[2] = value >> 24;
+        s->conf.macaddr.a[3] = value >> 16;
+        s->conf.macaddr.a[4] = value >> 8;
+        s->conf.macaddr.a[5] = value;
+        break;
+    case FTGMAC100_MATH0: /* Multicast Address Hash Table 0 */
+        s->math[0] = value;
+        break;
+    case FTGMAC100_MATH1: /* Multicast Address Hash Table 1 */
+        s->math[1] = value;
+        break;
+    case FTGMAC100_ITC: /* TODO: Interrupt Timer Control */
+        s->itc = value;
+        break;
+    case FTGMAC100_RXR_BADR: /* Ring buffer address */
+        s->rx_ring = value;
+        s->rx_descriptor = s->rx_ring;
+        break;
+
+    case FTGMAC100_RBSR: /* DMA buffer size */
+        s->rbsr = value;
+        break;
+
+    case FTGMAC100_NPTXR_BADR: /* Transmit buffer address */
+        s->tx_ring = value;
+        s->tx_descriptor = s->tx_ring;
+        break;
+
+    case FTGMAC100_NPTXPD: /* Trigger transmit */
+        if ((s->maccr & (FTGMAC100_MACCR_TXDMA_EN | FTGMAC100_MACCR_TXMAC_EN))
+            == (FTGMAC100_MACCR_TXDMA_EN | FTGMAC100_MACCR_TXMAC_EN)) {
+            /* TODO: high priority tx ring */
+            ftgmac100_do_tx(s, s->tx_ring, s->tx_descriptor);
+        }
+        if (ftgmac100_can_receive(qemu_get_queue(s->nic))) {
+            qemu_flush_queued_packets(qemu_get_queue(s->nic));
+        }
+        break;
+
+    case FTGMAC100_RXPD: /* Receive Poll Demand Register */
+        if (ftgmac100_can_receive(qemu_get_queue(s->nic))) {
+            qemu_flush_queued_packets(qemu_get_queue(s->nic));
+        }
+        break;
+
+    case FTGMAC100_APTC: /* Automatic polling */
+        s->aptcr = value;
+
+        if (FTGMAC100_APTC_RXPOLL_CNT(s->aptcr)) {
+            ftgmac100_rxpoll(s);
+        }
+
+        if (FTGMAC100_APTC_TXPOLL_CNT(s->aptcr)) {
+            qemu_log_mask(LOG_UNIMP, "%s: no transmit polling\n", __func__);
+        }
+        break;
+
+    case FTGMAC100_MACCR: /* MAC Device control */
+        s->maccr = value;
+        if (value & FTGMAC100_MACCR_SW_RST) {
+            ftgmac100_reset(DEVICE(s));
+        }
+
+        if (ftgmac100_can_receive(qemu_get_queue(s->nic))) {
+            qemu_flush_queued_packets(qemu_get_queue(s->nic));
+        }
+        break;
+
+    case FTGMAC100_PHYCR:  /* PHY Device control */
+        reg = FTGMAC100_PHYCR_REG(value);
+        s->phycr = value;
+        if (value & FTGMAC100_PHYCR_MIIWR) {
+            do_phy_write(s, reg, s->phydata & 0xffff);
+            s->phycr &= ~FTGMAC100_PHYCR_MIIWR;
+        } else {
+            s->phydata = do_phy_read(s, reg) << 16;
+            s->phycr &= ~FTGMAC100_PHYCR_MIIRD;
+        }
+        break;
+    case FTGMAC100_PHYDATA:
+        s->phydata = value & 0xffff;
+        break;
+    case FTGMAC100_DBLAC: /* DMA Burst Length and Arbitration Control */
+        s->dblac = value;
+        break;
+    case FTGMAC100_REVR:  /* Feature Register */
+        /* TODO: Only Old MDIO interface is supported */
+        s->revr = value & ~FTGMAC100_REVR_NEW_MDIO_INTERFACE;
+        break;
+    case FTGMAC100_FEAR1: /* Feature Register 1 */
+        s->fear1 = value;
+        break;
+    case FTGMAC100_TPAFCR: /* Transmit Priority Arbitration and FIFO Control */
+        s->tpafcr = value;
+        break;
+    case FTGMAC100_FCR: /* Flow Control  */
+        s->fcr  = value;
+        break;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad address at offset 0x%"
+                      HWADDR_PRIx "\n", __func__, addr);
+        break;
+    }
+
+    ftgmac100_update_irq(s);
+}
+
+static int ftgmac100_filter(Ftgmac100State *s, const uint8_t *buf, size_t len)
+{
+    unsigned mcast_idx;
+
+    if (s->maccr & FTGMAC100_MACCR_RX_ALL) {
+        return 1;
+    }
+
+    switch (get_eth_packet_type(PKT_GET_ETH_HDR(buf))) {
+    case ETH_PKT_BCAST:
+        if (!(s->maccr & FTGMAC100_MACCR_RX_BROADPKT)) {
+            return 0;
+        }
+        break;
+    case ETH_PKT_MCAST:
+        if (!(s->maccr & FTGMAC100_MACCR_RX_MULTIPKT)) {
+            if (!(s->maccr & FTGMAC100_MACCR_HT_MULTI_EN)) {
+                return 0;
+            }
+
+            /* TODO: this does not seem to work for ftgmac100 */
+            mcast_idx = compute_mcast_idx(buf);
+            if (!(s->math[mcast_idx / 32] & (1 << (mcast_idx % 32)))) {
+                return 0;
+            }
+        }
+        break;
+    case ETH_PKT_UCAST:
+        if (memcmp(s->conf.macaddr.a, buf, 6)) {
+            return 0;
+        }
+        break;
+    }
+
+    return 1;
+}
+
+static ssize_t ftgmac100_receive(NetClientState *nc, const uint8_t *buf,
+                                 size_t len)
+{
+    Ftgmac100State *s = FTGMAC100(qemu_get_nic_opaque(nc));
+    Ftgmac100Desc bd;
+    uint32_t flags = 0;
+    uint32_t addr;
+    uint32_t crc;
+    uint32_t buf_addr;
+    uint8_t *crc_ptr;
+    unsigned int buf_len;
+    size_t size = len;
+    uint32_t first = FTGMAC100_RXDES0_FRS;
+
+    if ((s->maccr & (FTGMAC100_MACCR_RXDMA_EN | FTGMAC100_MACCR_RXMAC_EN))
+         != (FTGMAC100_MACCR_RXDMA_EN | FTGMAC100_MACCR_RXMAC_EN)) {
+        return -1;
+    }
+
+    /* TODO : Pad to minimum Ethernet frame length */
+    /* handle small packets.  */
+    if (size < 10) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: dropped frame of %zd bytes\n",
+                      __func__, size);
+        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;
+    }
+
+    /* 4 bytes for the CRC.  */
+    size += 4;
+    crc = cpu_to_be32(crc32(~0, buf, size));
+    crc_ptr = (uint8_t *) &crc;
+
+    /* Huge frames are truncted.  */
+    if (size > FTGMAC100_MAX_FRAME_SIZE(s)) {
+        size = FTGMAC100_MAX_FRAME_SIZE(s);
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: frame too big : %zd bytes\n",
+                      __func__, size);
+        flags |= FTGMAC100_RXDES0_FTL;
+    }
+
+    switch (get_eth_packet_type(PKT_GET_ETH_HDR(buf))) {
+    case ETH_PKT_BCAST:
+        flags |= FTGMAC100_RXDES0_BROADCAST;
+        break;
+    case ETH_PKT_MCAST:
+        flags |= FTGMAC100_RXDES0_MULTICAST;
+        break;
+    case ETH_PKT_UCAST:
+        break;
+    }
+
+    addr = s->rx_descriptor;
+    while (size > 0) {
+        if (!ftgmac100_can_receive(nc)) {
+            qemu_log_mask(LOG_GUEST_ERROR, "%s: Unexpected packet\n", __func__);
+            return -1;
+        }
+
+        ftgmac100_read_bd(&bd, addr);
+        if (bd.des0 & FTGMAC100_RXDES0_RXPKT_RDY) {
+            /* No descriptors available.  Bail out.  */
+            qemu_log_mask(LOG_GUEST_ERROR, "%s: Lost end of frame\n",
+                          __func__);
+            s->isr |= FTGMAC100_INT_NO_RXBUF;
+            break;
+        }
+        buf_len = (size <= s->rbsr) ? size : s->rbsr;
+        bd.des0 |= buf_len & 0x3fff;
+        size -= buf_len;
+
+        /* The last 4 bytes are the CRC.  */
+        if (size < 4) {
+            buf_len += size - 4;
+        }
+        buf_addr = bd.des3;
+        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,
+                             crc_ptr, 4 - size);
+            crc_ptr += 4 - size;
+        }
+
+        bd.des0 |= first | FTGMAC100_RXDES0_RXPKT_RDY;
+        first = 0;
+        if (size == 0) {
+            /* Last buffer in frame.  */
+            bd.des0 |= flags | FTGMAC100_RXDES0_LRS;
+            s->isr |= FTGMAC100_INT_RPKT_BUF;
+        } else {
+            s->isr |= FTGMAC100_INT_RPKT_FIFO;
+        }
+        ftgmac100_write_bd(&bd, addr);
+        if (bd.des0 & FTGMAC100_RXDES0_EDORR) {
+            addr = s->rx_ring;
+        } else {
+            addr += sizeof(Ftgmac100Desc);
+        }
+    }
+    s->rx_descriptor = addr;
+
+    ftgmac100_update_irq(s);
+    return len;
+}
+
+static const MemoryRegionOps ftgmac100_ops = {
+    .read = ftgmac100_read,
+    .write = ftgmac100_write,
+    .valid.min_access_size = 4,
+    .valid.max_access_size = 4,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+};
+
+static void ftgmac100_cleanup(NetClientState *nc)
+{
+    Ftgmac100State *s = FTGMAC100(qemu_get_nic_opaque(nc));
+
+    s->nic = NULL;
+}
+
+static NetClientInfo net_ftgmac100_info = {
+    .type = NET_CLIENT_DRIVER_NIC,
+    .size = sizeof(NICState),
+    .can_receive = ftgmac100_can_receive,
+    .receive = ftgmac100_receive,
+    .cleanup = ftgmac100_cleanup,
+    .link_status_changed = ftgmac100_set_link,
+};
+
+static void ftgmac100_realize(DeviceState *dev, Error **errp)
+{
+    Ftgmac100State *s = FTGMAC100(dev);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+
+    memory_region_init_io(&s->iomem, OBJECT(dev), &ftgmac100_ops, s,
+                          TYPE_FTGMAC100, 0x2000);
+    sysbus_init_mmio(sbd, &s->iomem);
+    sysbus_init_irq(sbd, &s->irq);
+    qemu_macaddr_default_if_unset(&s->conf.macaddr);
+
+    s->conf.peers.ncs[0] = nd_table[0].netdev;
+
+    s->nic = qemu_new_nic(&net_ftgmac100_info, &s->conf,
+                          object_get_typename(OBJECT(dev)), DEVICE(dev)->id,
+                          s);
+    qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a);
+}
+
+static const VMStateDescription vmstate_ftgmac100 = {
+    .name = TYPE_FTGMAC100,
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(irq_state, Ftgmac100State),
+        VMSTATE_UINT32(isr, Ftgmac100State),
+        VMSTATE_UINT32(ier, Ftgmac100State),
+        VMSTATE_UINT32(rx_enabled, Ftgmac100State),
+        VMSTATE_UINT32(rx_ring, Ftgmac100State),
+        VMSTATE_UINT32(rbsr, Ftgmac100State),
+        VMSTATE_UINT32(tx_ring, Ftgmac100State),
+        VMSTATE_UINT32(rx_descriptor, Ftgmac100State),
+        VMSTATE_UINT32(tx_descriptor, Ftgmac100State),
+        VMSTATE_UINT32_ARRAY(math, Ftgmac100State, 2),
+        VMSTATE_UINT32(itc, Ftgmac100State),
+        VMSTATE_UINT32(aptcr, Ftgmac100State),
+        VMSTATE_UINT32(dblac, Ftgmac100State),
+        VMSTATE_UINT32(revr, Ftgmac100State),
+        VMSTATE_UINT32(fear1, Ftgmac100State),
+        VMSTATE_UINT32(tpafcr, Ftgmac100State),
+        VMSTATE_UINT32(maccr, Ftgmac100State),
+        VMSTATE_UINT32(phycr, Ftgmac100State),
+        VMSTATE_UINT32(phydata, Ftgmac100State),
+        VMSTATE_UINT32(fcr, Ftgmac100State),
+        VMSTATE_UINT32(phy_status, Ftgmac100State),
+        VMSTATE_UINT32(phy_control, Ftgmac100State),
+        VMSTATE_UINT32(phy_advertise, Ftgmac100State),
+        VMSTATE_UINT32(phy_int, Ftgmac100State),
+        VMSTATE_UINT32(phy_int_mask, Ftgmac100State),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static Property ftgmac100_properties[] = {
+    DEFINE_NIC_PROPERTIES(Ftgmac100State, conf),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void ftgmac100_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->vmsd = &vmstate_ftgmac100;
+    dc->reset = ftgmac100_reset;
+    dc->props = ftgmac100_properties;
+    set_bit(DEVICE_CATEGORY_NETWORK, dc->categories);
+    dc->realize = ftgmac100_realize;
+    dc->desc = "Faraday FTGMAC100 Gigabit Ethernet emulation";
+}
+
+static const TypeInfo ftgmac100_info = {
+    .name = TYPE_FTGMAC100,
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(Ftgmac100State),
+    .class_init = ftgmac100_class_init,
+};
+
+static void ftgmac100_register_types(void)
+{
+    type_register_static(&ftgmac100_info);
+}
+
+type_init(ftgmac100_register_types)
diff --git a/include/hw/net/ftgmac100.h b/include/hw/net/ftgmac100.h
new file mode 100644
index 000000000000..3bb4fe1a3ece
--- /dev/null
+++ b/include/hw/net/ftgmac100.h
@@ -0,0 +1,58 @@
+/*
+ * Faraday FTGMAC100 Gigabit Ethernet
+ *
+ * Copyright (C) 2016 IBM Corp.
+ *
+ * This code is licensed under the GPL version 2 or later. See the
+ * COPYING file in the top-level directory.
+ */
+
+#ifndef FTGMAC100_H
+#define FTGMAC100_H
+
+#define TYPE_FTGMAC100 "ftgmac100"
+#define FTGMAC100(obj) OBJECT_CHECK(Ftgmac100State, (obj), TYPE_FTGMAC100)
+
+#include "hw/sysbus.h"
+#include "net/net.h"
+
+typedef struct Ftgmac100State {
+    /*< private >*/
+    SysBusDevice parent_obj;
+
+    /*< public >*/
+    NICState *nic;
+    NICConf conf;
+    qemu_irq irq;
+    MemoryRegion iomem;
+
+    uint32_t irq_state;
+    uint32_t isr;
+    uint32_t ier;
+    uint32_t rx_enabled;
+    uint32_t rx_ring;
+    uint32_t rx_descriptor;
+    uint32_t tx_ring;
+    uint32_t tx_descriptor;
+    uint32_t math[2];
+    uint32_t rbsr;
+    uint32_t itc;
+    uint32_t aptcr;
+    uint32_t dblac;
+    uint32_t revr;
+    uint32_t fear1;
+    uint32_t tpafcr;
+    uint32_t maccr;
+    uint32_t phycr;
+    uint32_t phydata;
+    uint32_t fcr;
+
+
+    uint32_t phy_status;
+    uint32_t phy_control;
+    uint32_t phy_advertise;
+    uint32_t phy_int;
+    uint32_t phy_int_mask;
+} Ftgmac100State;
+
+#endif
diff --git a/include/hw/net/mii.h b/include/hw/net/mii.h
index 9fdd7bbe75f1..f820acdb4a19 100644
--- a/include/hw/net/mii.h
+++ b/include/hw/net/mii.h
@@ -29,6 +29,8 @@
 #define MII_ANAR            4
 #define MII_ANLPAR          5
 #define MII_ANER            6
+#define MII_CTRL1000        9
+#define MII_STAT1000        10
 #define MII_NSR             16
 #define MII_LBREMR          17
 #define MII_REC             18
@@ -69,6 +71,10 @@
 #define RTL8201CP_PHYID1    0x0000
 #define RTL8201CP_PHYID2    0x8201
 
+/* RealTek 8211E */
+#define RTL8211E_PHYID1    0x001c
+#define RTL8211E_PHYID2    0xc915
+
 /* National Semiconductor DP83848 */
 #define DP83848_PHYID1      0x2000
 #define DP83848_PHYID2      0x5c90
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH 2/4] net/ftgmac100: add a 'aspeed' property
  2017-04-01 12:57 [Qemu-devel] [PATCH 0/4] FTGMAC100 nic model for the Aspeed SoCs Cédric Le Goater
  2017-04-01 12:57 ` [Qemu-devel] [PATCH 1/4] net: add FTGMAC100 support Cédric Le Goater
@ 2017-04-01 12:57 ` Cédric Le Goater
  2017-04-01 12:57 ` [Qemu-devel] [PATCH 3/4] aspeed: add a FTGMAC100 nic Cédric Le Goater
  2017-04-01 12:57 ` [Qemu-devel] [PATCH 4/4] slirp: add a fake NC-SI backend Cédric Le Goater
  3 siblings, 0 replies; 8+ messages in thread
From: Cédric Le Goater @ 2017-04-01 12:57 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Jason Wang, Dmitry Fleytman, Samuel Thibault, Jan Kiszka,
	qemu-devel, qemu-arm, Cédric Le Goater

The Aspeed SoCs have a different definition of the end of the ring
buffer bit. Add a property to specify which set of bits should be used
by the NIC.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/net/ftgmac100.c         | 19 ++++++++++++++++---
 include/hw/net/ftgmac100.h |  4 ++++
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c
index 331e87391962..8867c435ba9d 100644
--- a/hw/net/ftgmac100.c
+++ b/hw/net/ftgmac100.c
@@ -133,6 +133,7 @@ struct ftgmac100_txdes {
 #define FTGMAC100_TXDES0_CRC_ERR         (1 << 19)
 #define FTGMAC100_TXDES0_LTS             (1 << 28)
 #define FTGMAC100_TXDES0_FTS             (1 << 29)
+#define FTGMAC100_TXDES0_EDOTR_ASPEED    (1 << 30)
 #define FTGMAC100_TXDES0_TXDMA_OWN       (1 << 31)
 
 #define FTGMAC100_TXDES1_VLANTAG_CI(x)   ((x) & 0xffff)
@@ -168,6 +169,7 @@ struct ftgmac100_rxdes {
 #define FTGMAC100_RXDES0_PAUSE_FRAME     (1 << 25)
 #define FTGMAC100_RXDES0_LRS             (1 << 28)
 #define FTGMAC100_RXDES0_FRS             (1 << 29)
+#define FTGMAC100_RXDES0_EDORR_ASPEED    (1 << 30)
 #define FTGMAC100_RXDES0_RXPKT_RDY       (1 << 31)
 
 #define FTGMAC100_RXDES1_VLANTAG_CI      0xffff
@@ -387,7 +389,7 @@ static uint32_t ftgmac100_find_txdes(Ftgmac100State *s, uint32_t addr)
 
     while (1) {
         ftgmac100_read_bd(&bd, addr);
-        if (bd.des0 & (FTGMAC100_TXDES0_FTS | FTGMAC100_TXDES0_EDOTR)) {
+        if (bd.des0 & (FTGMAC100_TXDES0_FTS | s->txdes0_edotr)) {
             break;
         }
         addr += sizeof(Ftgmac100Desc);
@@ -453,7 +455,7 @@ static void ftgmac100_do_tx(Ftgmac100State *s, uint32_t tx_ring,
         /* Write back the modified descriptor.  */
         ftgmac100_write_bd(&bd, addr);
         /* Advance to the next descriptor.  */
-        if (bd.des0 & FTGMAC100_TXDES0_EDOTR) {
+        if (bd.des0 & s->txdes0_edotr) {
             addr = tx_ring;
         } else {
             addr += sizeof(Ftgmac100Desc);
@@ -856,7 +858,7 @@ static ssize_t ftgmac100_receive(NetClientState *nc, const uint8_t *buf,
             s->isr |= FTGMAC100_INT_RPKT_FIFO;
         }
         ftgmac100_write_bd(&bd, addr);
-        if (bd.des0 & FTGMAC100_RXDES0_EDORR) {
+        if (bd.des0 & s->rxdes0_edorr) {
             addr = s->rx_ring;
         } else {
             addr += sizeof(Ftgmac100Desc);
@@ -897,6 +899,14 @@ static void ftgmac100_realize(DeviceState *dev, Error **errp)
     Ftgmac100State *s = FTGMAC100(dev);
     SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
 
+    if (s->aspeed) {
+        s->txdes0_edotr = FTGMAC100_TXDES0_EDOTR_ASPEED;
+        s->rxdes0_edorr = FTGMAC100_RXDES0_EDORR_ASPEED;
+    } else {
+        s->txdes0_edotr = FTGMAC100_TXDES0_EDOTR;
+        s->rxdes0_edorr = FTGMAC100_RXDES0_EDORR;
+    }
+
     memory_region_init_io(&s->iomem, OBJECT(dev), &ftgmac100_ops, s,
                           TYPE_FTGMAC100, 0x2000);
     sysbus_init_mmio(sbd, &s->iomem);
@@ -941,11 +951,14 @@ static const VMStateDescription vmstate_ftgmac100 = {
         VMSTATE_UINT32(phy_advertise, Ftgmac100State),
         VMSTATE_UINT32(phy_int, Ftgmac100State),
         VMSTATE_UINT32(phy_int_mask, Ftgmac100State),
+        VMSTATE_UINT32(txdes0_edotr, Ftgmac100State),
+        VMSTATE_UINT32(rxdes0_edorr, Ftgmac100State),
         VMSTATE_END_OF_LIST()
     }
 };
 
 static Property ftgmac100_properties[] = {
+    DEFINE_PROP_BOOL("aspeed", Ftgmac100State, aspeed, false),
     DEFINE_NIC_PROPERTIES(Ftgmac100State, conf),
     DEFINE_PROP_END_OF_LIST(),
 };
diff --git a/include/hw/net/ftgmac100.h b/include/hw/net/ftgmac100.h
index 3bb4fe1a3ece..feb387bf0e2d 100644
--- a/include/hw/net/ftgmac100.h
+++ b/include/hw/net/ftgmac100.h
@@ -53,6 +53,10 @@ typedef struct Ftgmac100State {
     uint32_t phy_advertise;
     uint32_t phy_int;
     uint32_t phy_int_mask;
+
+    bool aspeed;
+    uint32_t txdes0_edotr;
+    uint32_t rxdes0_edorr;
 } Ftgmac100State;
 
 #endif
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH 3/4] aspeed: add a FTGMAC100 nic
  2017-04-01 12:57 [Qemu-devel] [PATCH 0/4] FTGMAC100 nic model for the Aspeed SoCs Cédric Le Goater
  2017-04-01 12:57 ` [Qemu-devel] [PATCH 1/4] net: add FTGMAC100 support Cédric Le Goater
  2017-04-01 12:57 ` [Qemu-devel] [PATCH 2/4] net/ftgmac100: add a 'aspeed' property Cédric Le Goater
@ 2017-04-01 12:57 ` Cédric Le Goater
  2017-04-01 12:57 ` [Qemu-devel] [PATCH 4/4] slirp: add a fake NC-SI backend Cédric Le Goater
  3 siblings, 0 replies; 8+ messages in thread
From: Cédric Le Goater @ 2017-04-01 12:57 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Jason Wang, Dmitry Fleytman, Samuel Thibault, Jan Kiszka,
	qemu-devel, qemu-arm, Cédric Le Goater

There is a second NIC but we do not use it for the moment. We use the
'aspeed' property to tune the definition of the end of ring buffer bit
for the Aspeed SoCs.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/arm/aspeed_soc.c         | 21 +++++++++++++++++++++
 include/hw/arm/aspeed_soc.h |  2 ++
 2 files changed, 23 insertions(+)

diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index 712ae9d6c54d..af0964cf6add 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -19,6 +19,7 @@
 #include "hw/char/serial.h"
 #include "qemu/log.h"
 #include "hw/i2c/aspeed_i2c.h"
+#include "net/net.h"
 
 #define ASPEED_SOC_UART_5_BASE      0x00184000
 #define ASPEED_SOC_IOMEM_SIZE       0x00200000
@@ -33,6 +34,8 @@
 #define ASPEED_SOC_TIMER_BASE       0x1E782000
 #define ASPEED_SOC_WDT_BASE         0x1E785000
 #define ASPEED_SOC_I2C_BASE         0x1E78A000
+#define ASPEED_SOC_ETH1_BASE        0x1E660000
+#define ASPEED_SOC_ETH2_BASE        0x1E680000
 
 static const int uart_irqs[] = { 9, 32, 33, 34, 10 };
 static const int timer_irqs[] = { 16, 17, 18, 35, 36, 37, 38, 39, };
@@ -177,6 +180,10 @@ static void aspeed_soc_init(Object *obj)
     qdev_set_parent_bus(DEVICE(&s->wdt), sysbus_get_default());
     object_property_add_const_link(OBJECT(&s->wdt), "scu", OBJECT(&s->scu),
                                    NULL);
+
+    object_initialize(&s->ftgmac100, sizeof(s->ftgmac100), TYPE_FTGMAC100);
+    object_property_add_child(obj, "ftgmac100", OBJECT(&s->ftgmac100), NULL);
+    qdev_set_parent_bus(DEVICE(&s->ftgmac100), sysbus_get_default());
 }
 
 static void aspeed_soc_realize(DeviceState *dev, Error **errp)
@@ -304,6 +311,20 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
         return;
     }
     sysbus_mmio_map(SYS_BUS_DEVICE(&s->wdt), 0, ASPEED_SOC_WDT_BASE);
+
+    /* Net */
+    qdev_set_nic_properties(DEVICE(&s->ftgmac100), &nd_table[0]);
+    object_property_set_bool(OBJECT(&s->ftgmac100), true, "aspeed", &err);
+    object_property_set_bool(OBJECT(&s->ftgmac100), true, "realized",
+                             &local_err);
+    error_propagate(&err, local_err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+    sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100), 0, ASPEED_SOC_ETH1_BASE);
+    sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100), 0,
+                       qdev_get_gpio_in(DEVICE(&s->vic), 2));
 }
 
 static void aspeed_soc_class_init(ObjectClass *oc, void *data)
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index dbec0c159885..1fd0dc690be1 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -20,6 +20,7 @@
 #include "hw/i2c/aspeed_i2c.h"
 #include "hw/ssi/aspeed_smc.h"
 #include "hw/watchdog/wdt_aspeed.h"
+#include "hw/net/ftgmac100.h"
 
 #define ASPEED_SPIS_NUM  2
 
@@ -39,6 +40,7 @@ typedef struct AspeedSoCState {
     AspeedSMCState spi[ASPEED_SPIS_NUM];
     AspeedSDMCState sdmc;
     AspeedWDTState wdt;
+    Ftgmac100State ftgmac100;
 } AspeedSoCState;
 
 #define TYPE_ASPEED_SOC "aspeed-soc"
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH 4/4] slirp: add a fake NC-SI backend
  2017-04-01 12:57 [Qemu-devel] [PATCH 0/4] FTGMAC100 nic model for the Aspeed SoCs Cédric Le Goater
                   ` (2 preceding siblings ...)
  2017-04-01 12:57 ` [Qemu-devel] [PATCH 3/4] aspeed: add a FTGMAC100 nic Cédric Le Goater
@ 2017-04-01 12:57 ` Cédric Le Goater
  3 siblings, 0 replies; 8+ messages in thread
From: Cédric Le Goater @ 2017-04-01 12:57 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Jason Wang, Dmitry Fleytman, Samuel Thibault, Jan Kiszka,
	qemu-devel, qemu-arm, Cédric Le Goater

NC-SI (Network Controller Sideband Interface) enables a BMC to manage
a set of NICs on a system. This model takes the simplest approach and
reverses the NC-SI packets to pretend a NIC is present and exercise
the Linux driver.

The NCSI header file <ncsi-pkt.h> comes from mainline Linux and was
untabified.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/net/eth.h   |   1 +
 slirp/Makefile.objs |   2 +-
 slirp/ncsi-pkt.h    | 418 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 slirp/ncsi.c        |  78 ++++++++++
 slirp/slirp.c       |   4 +
 slirp/slirp.h       |   3 +
 6 files changed, 505 insertions(+), 1 deletion(-)
 create mode 100644 slirp/ncsi-pkt.h
 create mode 100644 slirp/ncsi.c

diff --git a/include/net/eth.h b/include/net/eth.h
index afeb45be34e1..09054a506d6e 100644
--- a/include/net/eth.h
+++ b/include/net/eth.h
@@ -209,6 +209,7 @@ struct tcp_hdr {
 #define ETH_P_IPV6                (0x86dd)
 #define ETH_P_VLAN                (0x8100)
 #define ETH_P_DVLAN               (0x88a8)
+#define ETH_P_NCSI                (0x88f8)
 #define ETH_P_UNKNOWN             (0xffff)
 #define VLAN_VID_MASK             0x0fff
 #define IP_HEADER_VERSION_4       (4)
diff --git a/slirp/Makefile.objs b/slirp/Makefile.objs
index 1baa1f1c7cfb..28049b03cddc 100644
--- a/slirp/Makefile.objs
+++ b/slirp/Makefile.objs
@@ -2,4 +2,4 @@ common-obj-y = cksum.o if.o ip_icmp.o ip6_icmp.o ip6_input.o ip6_output.o \
                ip_input.o ip_output.o dnssearch.o dhcpv6.o
 common-obj-y += slirp.o mbuf.o misc.o sbuf.o socket.o tcp_input.o tcp_output.o
 common-obj-y += tcp_subr.o tcp_timer.o udp.o udp6.o bootp.o tftp.o arp_table.o \
-                ndp_table.o
+                ndp_table.o ncsi.o
diff --git a/slirp/ncsi-pkt.h b/slirp/ncsi-pkt.h
new file mode 100644
index 000000000000..cb786bba463e
--- /dev/null
+++ b/slirp/ncsi-pkt.h
@@ -0,0 +1,418 @@
+/*
+ * Copyright Gavin Shan, IBM Corporation 2016.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef NCSI_PKT_H
+#define NCSI_PKT_H
+
+#define __be32 uint32_t
+#define __be16 uint16_t
+
+struct ncsi_pkt_hdr {
+        unsigned char mc_id;        /* Management controller ID */
+        unsigned char revision;     /* NCSI version - 0x01      */
+        unsigned char reserved;     /* Reserved                 */
+        unsigned char id;           /* Packet sequence number   */
+        unsigned char type;         /* Packet type              */
+        unsigned char channel;      /* Network controller ID    */
+        __be16        length;       /* Payload length           */
+        __be32        reserved1[2]; /* Reserved                 */
+};
+
+struct ncsi_cmd_pkt_hdr {
+        struct ncsi_pkt_hdr common; /* Common NCSI packet header */
+};
+
+struct ncsi_rsp_pkt_hdr {
+        struct ncsi_pkt_hdr common; /* Common NCSI packet header */
+        __be16              code;   /* Response code             */
+        __be16              reason; /* Response reason           */
+};
+
+struct ncsi_aen_pkt_hdr {
+        struct ncsi_pkt_hdr common;       /* Common NCSI packet header */
+        unsigned char       reserved2[3]; /* Reserved                  */
+        unsigned char       type;         /* AEN packet type           */
+};
+
+/* NCSI common command packet */
+struct ncsi_cmd_pkt {
+        struct ncsi_cmd_pkt_hdr cmd;      /* Command header */
+        __be32                  checksum; /* Checksum       */
+        unsigned char           pad[26];
+};
+
+struct ncsi_rsp_pkt {
+        struct ncsi_rsp_pkt_hdr rsp;      /* Response header */
+        __be32                  checksum; /* Checksum        */
+        unsigned char           pad[22];
+};
+
+/* Select Package */
+struct ncsi_cmd_sp_pkt {
+        struct ncsi_cmd_pkt_hdr cmd;            /* Command header */
+        unsigned char           reserved[3];    /* Reserved       */
+        unsigned char           hw_arbitration; /* HW arbitration */
+        __be32                  checksum;       /* Checksum       */
+        unsigned char           pad[22];
+};
+
+/* Disable Channel */
+struct ncsi_cmd_dc_pkt {
+        struct ncsi_cmd_pkt_hdr cmd;         /* Command header  */
+        unsigned char           reserved[3]; /* Reserved        */
+        unsigned char           ald;         /* Allow link down */
+        __be32                  checksum;    /* Checksum        */
+        unsigned char           pad[22];
+};
+
+/* Reset Channel */
+struct ncsi_cmd_rc_pkt {
+        struct ncsi_cmd_pkt_hdr cmd;      /* Command header */
+        __be32                  reserved; /* Reserved       */
+        __be32                  checksum; /* Checksum       */
+        unsigned char           pad[22];
+};
+
+/* AEN Enable */
+struct ncsi_cmd_ae_pkt {
+        struct ncsi_cmd_pkt_hdr cmd;         /* Command header   */
+        unsigned char           reserved[3]; /* Reserved         */
+        unsigned char           mc_id;       /* MC ID            */
+        __be32                  mode;        /* AEN working mode */
+        __be32                  checksum;    /* Checksum         */
+        unsigned char           pad[18];
+};
+
+/* Set Link */
+struct ncsi_cmd_sl_pkt {
+        struct ncsi_cmd_pkt_hdr cmd;      /* Command header    */
+        __be32                  mode;     /* Link working mode */
+        __be32                  oem_mode; /* OEM link mode     */
+        __be32                  checksum; /* Checksum          */
+        unsigned char           pad[18];
+};
+
+/* Set VLAN Filter */
+struct ncsi_cmd_svf_pkt {
+        struct ncsi_cmd_pkt_hdr cmd;       /* Command header    */
+        __be16                  reserved;  /* Reserved          */
+        __be16                  vlan;      /* VLAN ID           */
+        __be16                  reserved1; /* Reserved          */
+        unsigned char           index;     /* VLAN table index  */
+        unsigned char           enable;    /* Enable or disable */
+        __be32                  checksum;  /* Checksum          */
+        unsigned char           pad[14];
+};
+
+/* Enable VLAN */
+struct ncsi_cmd_ev_pkt {
+        struct ncsi_cmd_pkt_hdr cmd;         /* Command header   */
+        unsigned char           reserved[3]; /* Reserved         */
+        unsigned char           mode;        /* VLAN filter mode */
+        __be32                  checksum;    /* Checksum         */
+        unsigned char           pad[22];
+};
+
+/* Set MAC Address */
+struct ncsi_cmd_sma_pkt {
+        struct ncsi_cmd_pkt_hdr cmd;      /* Command header          */
+        unsigned char           mac[6];   /* MAC address             */
+        unsigned char           index;    /* MAC table index         */
+        unsigned char           at_e;     /* Addr type and operation */
+        __be32                  checksum; /* Checksum                */
+        unsigned char           pad[18];
+};
+
+/* Enable Broadcast Filter */
+struct ncsi_cmd_ebf_pkt {
+        struct ncsi_cmd_pkt_hdr cmd;      /* Command header */
+        __be32                  mode;     /* Filter mode    */
+        __be32                  checksum; /* Checksum       */
+        unsigned char           pad[22];
+};
+
+/* Enable Global Multicast Filter */
+struct ncsi_cmd_egmf_pkt {
+        struct ncsi_cmd_pkt_hdr cmd;      /* Command header */
+        __be32                  mode;     /* Global MC mode */
+        __be32                  checksum; /* Checksum       */
+        unsigned char           pad[22];
+};
+
+/* Set NCSI Flow Control */
+struct ncsi_cmd_snfc_pkt {
+        struct ncsi_cmd_pkt_hdr cmd;         /* Command header    */
+        unsigned char           reserved[3]; /* Reserved          */
+        unsigned char           mode;        /* Flow control mode */
+        __be32                  checksum;    /* Checksum          */
+        unsigned char           pad[22];
+};
+
+/* Get Link Status */
+struct ncsi_rsp_gls_pkt {
+        struct ncsi_rsp_pkt_hdr rsp;        /* Response header   */
+        __be32                  status;     /* Link status       */
+        __be32                  other;      /* Other indications */
+        __be32                  oem_status; /* OEM link status   */
+        __be32                  checksum;
+        unsigned char           pad[10];
+};
+
+/* Get Version ID */
+struct ncsi_rsp_gvi_pkt {
+        struct ncsi_rsp_pkt_hdr rsp;          /* Response header */
+        __be32                  ncsi_version; /* NCSI version    */
+        unsigned char           reserved[3];  /* Reserved        */
+        unsigned char           alpha2;       /* NCSI version    */
+        unsigned char           fw_name[12];  /* f/w name string */
+        __be32                  fw_version;   /* f/w version     */
+        __be16                  pci_ids[4];   /* PCI IDs         */
+        __be32                  mf_id;        /* Manufacture ID  */
+        __be32                  checksum;
+};
+
+/* Get Capabilities */
+struct ncsi_rsp_gc_pkt {
+        struct ncsi_rsp_pkt_hdr rsp;         /* Response header   */
+        __be32                  cap;         /* Capabilities      */
+        __be32                  bc_cap;      /* Broadcast cap     */
+        __be32                  mc_cap;      /* Multicast cap     */
+        __be32                  buf_cap;     /* Buffering cap     */
+        __be32                  aen_cap;     /* AEN cap           */
+        unsigned char           vlan_cnt;    /* VLAN filter count */
+        unsigned char           mixed_cnt;   /* Mix filter count  */
+        unsigned char           mc_cnt;      /* MC filter count   */
+        unsigned char           uc_cnt;      /* UC filter count   */
+        unsigned char           reserved[2]; /* Reserved          */
+        unsigned char           vlan_mode;   /* VLAN mode         */
+        unsigned char           channel_cnt; /* Channel count     */
+        __be32                  checksum;    /* Checksum          */
+};
+
+/* Get Parameters */
+struct ncsi_rsp_gp_pkt {
+        struct ncsi_rsp_pkt_hdr rsp;          /* Response header       */
+        unsigned char           mac_cnt;      /* Number of MAC addr    */
+        unsigned char           reserved[2];  /* Reserved              */
+        unsigned char           mac_enable;   /* MAC addr enable flags */
+        unsigned char           vlan_cnt;     /* VLAN tag count        */
+        unsigned char           reserved1;    /* Reserved              */
+        __be16                  vlan_enable;  /* VLAN tag enable flags */
+        __be32                  link_mode;    /* Link setting          */
+        __be32                  bc_mode;      /* BC filter mode        */
+        __be32                  valid_modes;  /* Valid mode parameters */
+        unsigned char           vlan_mode;    /* VLAN mode             */
+        unsigned char           fc_mode;      /* Flow control mode     */
+        unsigned char           reserved2[2]; /* Reserved              */
+        __be32                  aen_mode;     /* AEN mode              */
+        unsigned char           mac[6];       /* Supported MAC addr    */
+        __be16                  vlan;         /* Supported VLAN tags   */
+        __be32                  checksum;     /* Checksum              */
+};
+
+/* Get Controller Packet Statistics */
+struct ncsi_rsp_gcps_pkt {
+        struct ncsi_rsp_pkt_hdr rsp;            /* Response header            */
+        __be32                  cnt_hi;         /* Counter cleared            */
+        __be32                  cnt_lo;         /* Counter cleared            */
+        __be32                  rx_bytes;       /* Rx bytes                   */
+        __be32                  tx_bytes;       /* Tx bytes                   */
+        __be32                  rx_uc_pkts;     /* Rx UC packets              */
+        __be32                  rx_mc_pkts;     /* Rx MC packets              */
+        __be32                  rx_bc_pkts;     /* Rx BC packets              */
+        __be32                  tx_uc_pkts;     /* Tx UC packets              */
+        __be32                  tx_mc_pkts;     /* Tx MC packets              */
+        __be32                  tx_bc_pkts;     /* Tx BC packets              */
+        __be32                  fcs_err;        /* FCS errors                 */
+        __be32                  align_err;      /* Alignment errors           */
+        __be32                  false_carrier;  /* False carrier detection    */
+        __be32                  runt_pkts;      /* Rx runt packets            */
+        __be32                  jabber_pkts;    /* Rx jabber packets          */
+        __be32                  rx_pause_xon;   /* Rx pause XON frames        */
+        __be32                  rx_pause_xoff;  /* Rx XOFF frames             */
+        __be32                  tx_pause_xon;   /* Tx XON frames              */
+        __be32                  tx_pause_xoff;  /* Tx XOFF frames             */
+        __be32                  tx_s_collision; /* Single collision frames    */
+        __be32                  tx_m_collision; /* Multiple collision frames  */
+        __be32                  l_collision;    /* Late collision frames      */
+        __be32                  e_collision;    /* Excessive collision frames */
+        __be32                  rx_ctl_frames;  /* Rx control frames          */
+        __be32                  rx_64_frames;   /* Rx 64-bytes frames         */
+        __be32                  rx_127_frames;  /* Rx 65-127 bytes frames     */
+        __be32                  rx_255_frames;  /* Rx 128-255 bytes frames    */
+        __be32                  rx_511_frames;  /* Rx 256-511 bytes frames    */
+        __be32                  rx_1023_frames; /* Rx 512-1023 bytes frames   */
+        __be32                  rx_1522_frames; /* Rx 1024-1522 bytes frames  */
+        __be32                  rx_9022_frames; /* Rx 1523-9022 bytes frames  */
+        __be32                  tx_64_frames;   /* Tx 64-bytes frames         */
+        __be32                  tx_127_frames;  /* Tx 65-127 bytes frames     */
+        __be32                  tx_255_frames;  /* Tx 128-255 bytes frames    */
+        __be32                  tx_511_frames;  /* Tx 256-511 bytes frames    */
+        __be32                  tx_1023_frames; /* Tx 512-1023 bytes frames   */
+        __be32                  tx_1522_frames; /* Tx 1024-1522 bytes frames  */
+        __be32                  tx_9022_frames; /* Tx 1523-9022 bytes frames  */
+        __be32                  rx_valid_bytes; /* Rx valid bytes             */
+        __be32                  rx_runt_pkts;   /* Rx error runt packets      */
+        __be32                  rx_jabber_pkts; /* Rx error jabber packets    */
+        __be32                  checksum;       /* Checksum                   */
+};
+
+/* Get NCSI Statistics */
+struct ncsi_rsp_gns_pkt {
+        struct ncsi_rsp_pkt_hdr rsp;           /* Response header         */
+        __be32                  rx_cmds;       /* Rx NCSI commands        */
+        __be32                  dropped_cmds;  /* Dropped commands        */
+        __be32                  cmd_type_errs; /* Command type errors     */
+        __be32                  cmd_csum_errs; /* Command checksum errors */
+        __be32                  rx_pkts;       /* Rx NCSI packets         */
+        __be32                  tx_pkts;       /* Tx NCSI packets         */
+        __be32                  tx_aen_pkts;   /* Tx AEN packets          */
+        __be32                  checksum;      /* Checksum                */
+};
+
+/* Get NCSI Pass-through Statistics */
+struct ncsi_rsp_gnpts_pkt {
+        struct ncsi_rsp_pkt_hdr rsp;            /* Response header     */
+        __be32                  tx_pkts;        /* Tx packets          */
+        __be32                  tx_dropped;     /* Tx dropped packets  */
+        __be32                  tx_channel_err; /* Tx channel errors   */
+        __be32                  tx_us_err;      /* Tx undersize errors */
+        __be32                  rx_pkts;        /* Rx packets          */
+        __be32                  rx_dropped;     /* Rx dropped packets  */
+        __be32                  rx_channel_err; /* Rx channel errors   */
+        __be32                  rx_us_err;      /* Rx undersize errors */
+        __be32                  rx_os_err;      /* Rx oversize errors  */
+        __be32                  checksum;       /* Checksum            */
+};
+
+/* Get package status */
+struct ncsi_rsp_gps_pkt {
+        struct ncsi_rsp_pkt_hdr rsp;      /* Response header             */
+        __be32                  status;   /* Hardware arbitration status */
+        __be32                  checksum;
+};
+
+/* Get package UUID */
+struct ncsi_rsp_gpuuid_pkt {
+        struct ncsi_rsp_pkt_hdr rsp;      /* Response header */
+        unsigned char           uuid[16]; /* UUID            */
+        __be32                  checksum;
+};
+
+/* AEN: Link State Change */
+struct ncsi_aen_lsc_pkt {
+        struct ncsi_aen_pkt_hdr aen;        /* AEN header      */
+        __be32                  status;     /* Link status     */
+        __be32                  oem_status; /* OEM link status */
+        __be32                  checksum;   /* Checksum        */
+        unsigned char           pad[14];
+};
+
+/* AEN: Configuration Required */
+struct ncsi_aen_cr_pkt {
+        struct ncsi_aen_pkt_hdr aen;      /* AEN header */
+        __be32                  checksum; /* Checksum   */
+        unsigned char           pad[22];
+};
+
+/* AEN: Host Network Controller Driver Status Change */
+struct ncsi_aen_hncdsc_pkt {
+        struct ncsi_aen_pkt_hdr aen;      /* AEN header */
+        __be32                  status;   /* Status     */
+        __be32                  checksum; /* Checksum   */
+        unsigned char           pad[18];
+};
+
+/* NCSI packet revision */
+#define NCSI_PKT_REVISION       0x01
+
+/* NCSI packet commands */
+#define NCSI_PKT_CMD_CIS        0x00 /* Clear Initial State              */
+#define NCSI_PKT_CMD_SP         0x01 /* Select Package                   */
+#define NCSI_PKT_CMD_DP         0x02 /* Deselect Package                 */
+#define NCSI_PKT_CMD_EC         0x03 /* Enable Channel                   */
+#define NCSI_PKT_CMD_DC         0x04 /* Disable Channel                  */
+#define NCSI_PKT_CMD_RC         0x05 /* Reset Channel                    */
+#define NCSI_PKT_CMD_ECNT       0x06 /* Enable Channel Network Tx        */
+#define NCSI_PKT_CMD_DCNT       0x07 /* Disable Channel Network Tx       */
+#define NCSI_PKT_CMD_AE         0x08 /* AEN Enable                       */
+#define NCSI_PKT_CMD_SL         0x09 /* Set Link                         */
+#define NCSI_PKT_CMD_GLS        0x0a /* Get Link                         */
+#define NCSI_PKT_CMD_SVF        0x0b /* Set VLAN Filter                  */
+#define NCSI_PKT_CMD_EV         0x0c /* Enable VLAN                      */
+#define NCSI_PKT_CMD_DV         0x0d /* Disable VLAN                     */
+#define NCSI_PKT_CMD_SMA        0x0e /* Set MAC address                  */
+#define NCSI_PKT_CMD_EBF        0x10 /* Enable Broadcast Filter          */
+#define NCSI_PKT_CMD_DBF        0x11 /* Disable Broadcast Filter         */
+#define NCSI_PKT_CMD_EGMF       0x12 /* Enable Global Multicast Filter   */
+#define NCSI_PKT_CMD_DGMF       0x13 /* Disable Global Multicast Filter  */
+#define NCSI_PKT_CMD_SNFC       0x14 /* Set NCSI Flow Control            */
+#define NCSI_PKT_CMD_GVI        0x15 /* Get Version ID                   */
+#define NCSI_PKT_CMD_GC         0x16 /* Get Capabilities                 */
+#define NCSI_PKT_CMD_GP         0x17 /* Get Parameters                   */
+#define NCSI_PKT_CMD_GCPS       0x18 /* Get Controller Packet Statistics */
+#define NCSI_PKT_CMD_GNS        0x19 /* Get NCSI Statistics              */
+#define NCSI_PKT_CMD_GNPTS      0x1a /* Get NCSI Pass-throu Statistics   */
+#define NCSI_PKT_CMD_GPS        0x1b /* Get package status               */
+#define NCSI_PKT_CMD_OEM        0x50 /* OEM                              */
+#define NCSI_PKT_CMD_PLDM       0x51 /* PLDM request over NCSI over RBT  */
+#define NCSI_PKT_CMD_GPUUID     0x52 /* Get package UUID                 */
+
+/* NCSI packet responses */
+#define NCSI_PKT_RSP_CIS        (NCSI_PKT_CMD_CIS    + 0x80)
+#define NCSI_PKT_RSP_SP         (NCSI_PKT_CMD_SP     + 0x80)
+#define NCSI_PKT_RSP_DP         (NCSI_PKT_CMD_DP     + 0x80)
+#define NCSI_PKT_RSP_EC         (NCSI_PKT_CMD_EC     + 0x80)
+#define NCSI_PKT_RSP_DC         (NCSI_PKT_CMD_DC     + 0x80)
+#define NCSI_PKT_RSP_RC         (NCSI_PKT_CMD_RC     + 0x80)
+#define NCSI_PKT_RSP_ECNT       (NCSI_PKT_CMD_ECNT   + 0x80)
+#define NCSI_PKT_RSP_DCNT       (NCSI_PKT_CMD_DCNT   + 0x80)
+#define NCSI_PKT_RSP_AE         (NCSI_PKT_CMD_AE     + 0x80)
+#define NCSI_PKT_RSP_SL         (NCSI_PKT_CMD_SL     + 0x80)
+#define NCSI_PKT_RSP_GLS        (NCSI_PKT_CMD_GLS    + 0x80)
+#define NCSI_PKT_RSP_SVF        (NCSI_PKT_CMD_SVF    + 0x80)
+#define NCSI_PKT_RSP_EV         (NCSI_PKT_CMD_EV     + 0x80)
+#define NCSI_PKT_RSP_DV         (NCSI_PKT_CMD_DV     + 0x80)
+#define NCSI_PKT_RSP_SMA        (NCSI_PKT_CMD_SMA    + 0x80)
+#define NCSI_PKT_RSP_EBF        (NCSI_PKT_CMD_EBF    + 0x80)
+#define NCSI_PKT_RSP_DBF        (NCSI_PKT_CMD_DBF    + 0x80)
+#define NCSI_PKT_RSP_EGMF       (NCSI_PKT_CMD_EGMF   + 0x80)
+#define NCSI_PKT_RSP_DGMF       (NCSI_PKT_CMD_DGMF   + 0x80)
+#define NCSI_PKT_RSP_SNFC       (NCSI_PKT_CMD_SNFC   + 0x80)
+#define NCSI_PKT_RSP_GVI        (NCSI_PKT_CMD_GVI    + 0x80)
+#define NCSI_PKT_RSP_GC         (NCSI_PKT_CMD_GC     + 0x80)
+#define NCSI_PKT_RSP_GP         (NCSI_PKT_CMD_GP     + 0x80)
+#define NCSI_PKT_RSP_GCPS       (NCSI_PKT_CMD_GCPS   + 0x80)
+#define NCSI_PKT_RSP_GNS        (NCSI_PKT_CMD_GNS    + 0x80)
+#define NCSI_PKT_RSP_GNPTS      (NCSI_PKT_CMD_GNPTS  + 0x80)
+#define NCSI_PKT_RSP_GPS        (NCSI_PKT_CMD_GPS    + 0x80)
+#define NCSI_PKT_RSP_OEM        (NCSI_PKT_CMD_OEM    + 0x80)
+#define NCSI_PKT_RSP_PLDM       (NCSI_PKT_CMD_PLDM   + 0x80)
+#define NCSI_PKT_RSP_GPUUID     (NCSI_PKT_CMD_GPUUID + 0x80)
+
+/* NCSI response code/reason */
+#define NCSI_PKT_RSP_C_COMPLETED        0x0000 /* Command Completed        */
+#define NCSI_PKT_RSP_C_FAILED           0x0001 /* Command Failed           */
+#define NCSI_PKT_RSP_C_UNAVAILABLE      0x0002 /* Command Unavailable      */
+#define NCSI_PKT_RSP_C_UNSUPPORTED      0x0003 /* Command Unsupported      */
+#define NCSI_PKT_RSP_R_NO_ERROR         0x0000 /* No Error                 */
+#define NCSI_PKT_RSP_R_INTERFACE        0x0001 /* Interface not ready      */
+#define NCSI_PKT_RSP_R_PARAM            0x0002 /* Invalid Parameter        */
+#define NCSI_PKT_RSP_R_CHANNEL          0x0003 /* Channel not Ready        */
+#define NCSI_PKT_RSP_R_PACKAGE          0x0004 /* Package not Ready        */
+#define NCSI_PKT_RSP_R_LENGTH           0x0005 /* Invalid payload length   */
+#define NCSI_PKT_RSP_R_UNKNOWN          0x7fff /* Command type unsupported */
+
+/* NCSI AEN packet type */
+#define NCSI_PKT_AEN            0xFF /* AEN Packet               */
+#define NCSI_PKT_AEN_LSC        0x00 /* Link status change       */
+#define NCSI_PKT_AEN_CR         0x01 /* Configuration required   */
+#define NCSI_PKT_AEN_HNCDSC     0x02 /* HNC driver status change */
+
+#endif /* NCSI_PKT_H */
diff --git a/slirp/ncsi.c b/slirp/ncsi.c
new file mode 100644
index 000000000000..77302ec5e5a5
--- /dev/null
+++ b/slirp/ncsi.c
@@ -0,0 +1,78 @@
+/*
+ * NC-SI (Network Controller Sideband Interface) "echo" model
+ *
+ * Copyright (C) 2016 IBM Corp.
+ *
+ * This code is licensed under the GPL version 2 or later. See the
+ * COPYING file in the top-level directory.
+ */
+#include "qemu/osdep.h"
+#include "slirp.h"
+
+#include "ncsi-pkt.h"
+
+/*
+ * ncsi header + checksum + max payload (NCSI_PKT_CMD_GVI)
+ */
+#define NCSI_LEN (sizeof(struct ncsi_pkt_hdr) + 4 + 36)
+
+void ncsi_input(Slirp *slirp, const uint8_t *pkt, int pkt_len)
+{
+    struct ncsi_pkt_hdr *nh = (struct ncsi_pkt_hdr *)(pkt + ETH_HLEN);
+    uint8_t ncsi_reply[ETH_HLEN + NCSI_LEN];
+    struct ethhdr *reh = (struct ethhdr *)ncsi_reply;
+    struct ncsi_rsp_pkt_hdr *rnh = (struct ncsi_rsp_pkt_hdr *)
+        (ncsi_reply + ETH_HLEN);
+
+    memset(ncsi_reply, 0, sizeof(ncsi_reply));
+
+    memset(reh->h_dest, 0xff, ETH_ALEN);
+    memset(reh->h_source, 0xff, ETH_ALEN);
+    reh->h_proto = htons(ETH_P_NCSI);
+
+    rnh->common.mc_id        = nh->mc_id;
+    rnh->common.revision     = NCSI_PKT_REVISION;
+    rnh->common.reserved     = 0;
+    rnh->common.id           = nh->id;
+    rnh->common.type         = nh->type + 0x80;
+    rnh->common.channel      = nh->channel;
+    rnh->common.length       = htons(4);
+    rnh->common.reserved1[0] = 0;
+    rnh->common.reserved1[1] = 0;
+    rnh->code         = htons(NCSI_PKT_RSP_C_COMPLETED);
+    rnh->reason       = htons(NCSI_PKT_RSP_R_NO_ERROR);
+
+    switch (nh->type) {
+    case NCSI_PKT_CMD_SMA:
+        rnh->common.length = htons(4);
+        break;
+    case NCSI_PKT_CMD_GVI:
+        rnh->common.length = htons(36);
+        break;
+    case NCSI_PKT_CMD_GC: {
+        struct ncsi_rsp_gc_pkt *rsp = (struct ncsi_rsp_gc_pkt *) rnh;
+
+        rnh->common.length = htons(32);
+        rsp->cap = htonl(~0);
+        rsp->bc_cap = htonl(~0);
+        rsp->mc_cap = htonl(~0);
+        rsp->buf_cap = htonl(~0);
+        rsp->aen_cap = htonl(~0);
+        rsp->vlan_mode = 0xff;
+        rsp->uc_cnt = 2;
+        break;
+    }
+
+    case NCSI_PKT_CMD_GLS: {
+        struct ncsi_rsp_gls_pkt *rsp = (struct ncsi_rsp_gls_pkt *) rnh;
+
+        rnh->common.length = htons(16);
+        rsp->status = htonl(0x1);
+        break;
+    }
+    default:
+        break;
+    }
+
+    slirp_output(slirp->opaque, ncsi_reply, sizeof(ncsi_reply));
+}
diff --git a/slirp/slirp.c b/slirp/slirp.c
index 5a94b06f5eb4..9a50918346b1 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -870,6 +870,10 @@ void slirp_input(Slirp *slirp, const uint8_t *pkt, int pkt_len)
         }
         break;
 
+    case ETH_P_NCSI:
+        ncsi_input(slirp, pkt, pkt_len);
+        break;
+
     default:
         break;
     }
diff --git a/slirp/slirp.h b/slirp/slirp.h
index 3877f667f027..5af4f482b55f 100644
--- a/slirp/slirp.h
+++ b/slirp/slirp.h
@@ -231,6 +231,9 @@ extern Slirp *slirp_instance;
 
 void if_start(Slirp *);
 
+/* ncsi.c */
+void ncsi_input(Slirp *slirp, const uint8_t *pkt, int pkt_len);
+
 #ifndef _WIN32
 #include <netdb.h>
 #endif
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH 1/4] net: add FTGMAC100 support
  2017-04-01 12:57 ` [Qemu-devel] [PATCH 1/4] net: add FTGMAC100 support Cédric Le Goater
@ 2017-04-10 13:43   ` Peter Maydell
  2017-04-12 17:29     ` Cédric Le Goater
  0 siblings, 1 reply; 8+ messages in thread
From: Peter Maydell @ 2017-04-10 13:43 UTC (permalink / raw)
  To: Cédric Le Goater
  Cc: Jason Wang, Dmitry Fleytman, Samuel Thibault, Jan Kiszka,
	QEMU Developers, qemu-arm

On 1 April 2017 at 13:57, Cédric Le Goater <clg@kaod.org> wrote:
> The FTGMAC100 device is an Ethernet controller with DMA function that
> can be found on Aspeed SoCs (which include NCSI).
>
> It is fully compliant with IEEE 802.3 specification for 10/100 Mbps
> Ethernet and IEEE 802.3z specification for 1000 Mbps Ethernet and
> includes Reduced Media Independent Interface (RMII) and Reduced
> Gigabit Media Independent Interface (RGMII) interfaces. It adopts an
> AHB bus interface and integrates a link list DMA engine with direct
> M-Bus accesses for transmitting and receiving packets. It has
> independent TX/RX fifos, supports half and full duplex (1000 Mbps mode
> only supports full duplex), flow control for full duplex and
> backpressure for half duplex.
>
> The FTGMAC100 also implements IP, TCP, UDP checksum offloads and
> supports IEEE 802.1Q VLAN tag insertion and removal. It offers
> high-priority transmit queue for QoS and CoS applications
>
> This model is complete enough to satisfy two different Linux drivers
> and a U-Boot driver. Not supported features are :
>
>  - IEEE 802.1Q VLAN
>  - High Priority Transmit Queue
>  - Wake-On-LAN functions
>
> The code is based on the Coldfire Fast Ethernet Controller model.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  default-configs/arm-softmmu.mak |   1 +
>  hw/net/Makefile.objs            |   1 +
>  hw/net/ftgmac100.c              | 977 ++++++++++++++++++++++++++++++++++++++++
>  include/hw/net/ftgmac100.h      |  58 +++
>  include/hw/net/mii.h            |   6 +
>  5 files changed, 1043 insertions(+)
>  create mode 100644 hw/net/ftgmac100.c
>  create mode 100644 include/hw/net/ftgmac100.h
>
> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
> index 15a53598d1c3..acc4c6c86297 100644
> --- a/default-configs/arm-softmmu.mak
> +++ b/default-configs/arm-softmmu.mak
> @@ -30,6 +30,7 @@ CONFIG_LAN9118=y
>  CONFIG_SMC91C111=y
>  CONFIG_ALLWINNER_EMAC=y
>  CONFIG_IMX_FEC=y
> +CONFIG_FTGMAC100=y
>  CONFIG_DS1338=y
>  CONFIG_RX8900=y
>  CONFIG_PFLASH_CFI01=y
> diff --git a/hw/net/Makefile.objs b/hw/net/Makefile.objs
> index 6a95d92d37f8..5ddaffe63a46 100644
> --- a/hw/net/Makefile.objs
> +++ b/hw/net/Makefile.objs
> @@ -26,6 +26,7 @@ common-obj-$(CONFIG_IMX_FEC) += imx_fec.o
>  common-obj-$(CONFIG_CADENCE) += cadence_gem.o
>  common-obj-$(CONFIG_STELLARIS_ENET) += stellaris_enet.o
>  common-obj-$(CONFIG_LANCE) += lance.o
> +common-obj-$(CONFIG_FTGMAC100) += ftgmac100.o
>
>  obj-$(CONFIG_ETRAXFS) += etraxfs_eth.o
>  obj-$(CONFIG_COLDFIRE) += mcf_fec.o
> diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c
> new file mode 100644
> index 000000000000..331e87391962
> --- /dev/null
> +++ b/hw/net/ftgmac100.c
> @@ -0,0 +1,977 @@
> +/*
> + * Faraday FTGMAC100 Gigabit Ethernet
> + *
> + * Copyright (C) 2016 IBM Corp.
> + *
> + * Based on Coldfire Fast Ethernet Controller emulation.
> + *
> + * Copyright (c) 2007 CodeSourcery.
> + *
> + * This code is licensed under the GPL version 2 or later. See the
> + * COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/net/ftgmac100.h"
> +#include "sysemu/dma.h"
> +#include "qemu/log.h"
> +#include "net/checksum.h"
> +#include "net/eth.h"
> +#include "hw/net/mii.h"
> +
> +/* For crc32 */
> +#include <zlib.h>
> +
> +/*
> + * FTGMAC100 registers
> + */
> +#define FTGMAC100_ISR             0x00
> +#define FTGMAC100_IER             0x04
> +#define FTGMAC100_MAC_MADR        0x08
> +#define FTGMAC100_MAC_LADR        0x0c
> +#define FTGMAC100_MATH0           0x10
> +#define FTGMAC100_MATH1           0x14
> +#define FTGMAC100_NPTXPD          0x18
> +#define FTGMAC100_RXPD            0x1C
> +#define FTGMAC100_NPTXR_BADR      0x20
> +#define FTGMAC100_RXR_BADR        0x24
> +#define FTGMAC100_HPTXPD          0x28 /* TODO */
> +#define FTGMAC100_HPTXR_BADR      0x2c /* TODO */

What's TODO about a #define ?
Maybe better to have TODO comment and LOG_UNIMP at the register/read
write point?

> +#define FTGMAC100_ITC             0x30
> +#define FTGMAC100_APTC            0x34
> +#define FTGMAC100_DBLAC           0x38
> +#define FTGMAC100_REVR            0x40
> +#define FTGMAC100_FEAR1           0x44
> +#define FTGMAC100_RBSR            0x4c
> +#define FTGMAC100_TPAFCR          0x48
> +
> +#define FTGMAC100_MACCR           0x50
> +#define FTGMAC100_MACSR           0x54 /* TODO */
> +#define FTGMAC100_PHYCR           0x60
> +#define FTGMAC100_PHYDATA         0x64
> +#define FTGMAC100_FCR             0x68
> +
> +/*
> + * Interrupt status register & interrupt enable register
> + */
> +#define FTGMAC100_INT_RPKT_BUF    (1 << 0)
> +#define FTGMAC100_INT_RPKT_FIFO   (1 << 1)
> +#define FTGMAC100_INT_NO_RXBUF    (1 << 2)
> +#define FTGMAC100_INT_RPKT_LOST   (1 << 3)
> +#define FTGMAC100_INT_XPKT_ETH    (1 << 4)
> +#define FTGMAC100_INT_XPKT_FIFO   (1 << 5)
> +#define FTGMAC100_INT_NO_NPTXBUF  (1 << 6)
> +#define FTGMAC100_INT_XPKT_LOST   (1 << 7)
> +#define FTGMAC100_INT_AHB_ERR     (1 << 8)
> +#define FTGMAC100_INT_PHYSTS_CHG  (1 << 9)
> +#define FTGMAC100_INT_NO_HPTXBUF  (1 << 10)
> +
> +/*
> + * Automatic polling timer control register
> + */
> +#define FTGMAC100_APTC_RXPOLL_CNT(x)        ((x) & 0xf)
> +#define FTGMAC100_APTC_RXPOLL_TIME_SEL      (1 << 4)
> +#define FTGMAC100_APTC_TXPOLL_CNT(x)        (((x) >> 8) & 0xf)
> +#define FTGMAC100_APTC_TXPOLL_TIME_SEL      (1 << 12)
> +
> +/*
> + * PHY control register
> + */
> +#define FTGMAC100_PHYCR_MIIRD               (1 << 26)
> +#define FTGMAC100_PHYCR_MIIWR               (1 << 27)
> +
> +#define FTGMAC100_PHYCR_DEV(v)              (((v) >> 16) & 0x1f)
> +#define FTGMAC100_PHYCR_REG(v)              (((v) >> 21) & 0x1f)
> +
> +/*
> + * PHY data register
> + */
> +#define FTGMAC100_PHYDATA_MIIWDATA(x)       ((x) & 0xffff)
> +#define FTGMAC100_PHYDATA_MIIRDATA(phydata) (((phydata) >> 16) & 0xffff)

There's some inconsistency here between 'x' and 'v' and 'phydata'
for macro parameter names.

> +
> +/*
> + * Feature Register
> + */
> +#define FTGMAC100_REVR_NEW_MDIO_INTERFACE   (1 << 31)
> +
> +/*
> + * MAC control register
> + */
> +#define FTGMAC100_MACCR_TXDMA_EN         (1 << 0)
> +#define FTGMAC100_MACCR_RXDMA_EN         (1 << 1)
> +#define FTGMAC100_MACCR_TXMAC_EN         (1 << 2)
> +#define FTGMAC100_MACCR_RXMAC_EN         (1 << 3)
> +#define FTGMAC100_MACCR_RM_VLAN          (1 << 4)
> +#define FTGMAC100_MACCR_HPTXR_EN         (1 << 5)
> +#define FTGMAC100_MACCR_LOOP_EN          (1 << 6)
> +#define FTGMAC100_MACCR_ENRX_IN_HALFTX   (1 << 7)
> +#define FTGMAC100_MACCR_FULLDUP          (1 << 8)
> +#define FTGMAC100_MACCR_GIGA_MODE        (1 << 9)
> +#define FTGMAC100_MACCR_CRC_APD          (1 << 10) /* not needed */
> +#define FTGMAC100_MACCR_RX_RUNT          (1 << 12)
> +#define FTGMAC100_MACCR_JUMBO_LF         (1 << 13)
> +#define FTGMAC100_MACCR_RX_ALL           (1 << 14)
> +#define FTGMAC100_MACCR_HT_MULTI_EN      (1 << 15)
> +#define FTGMAC100_MACCR_RX_MULTIPKT      (1 << 16)
> +#define FTGMAC100_MACCR_RX_BROADPKT      (1 << 17)
> +#define FTGMAC100_MACCR_DISCARD_CRCERR   (1 << 18)
> +#define FTGMAC100_MACCR_FAST_MODE        (1 << 19)
> +#define FTGMAC100_MACCR_SW_RST           (1 << 31)
> +
> +/*
> + * Transmit descriptor, aligned to 16 bytes
> + */
> +struct ftgmac100_txdes {

Coding standard says camelcase for struct names (and typedefs).

> +        unsigned int        txdes0;
> +        unsigned int        txdes1;
> +        unsigned int        txdes2;      /* not used by HW */
> +        unsigned int        txdes3;      /* TXBUF_BADR */
> +} __attribute__ ((aligned(16)));

Why does this need to be attribute aligned ? Typically
you don't want to be using C structs for things actually
in guest memory IMHO. (I think your uses of it are all


If the fields have to be exactly 32 bits then uint32_t is
a clearer way to write them.

> +
> +#define FTGMAC100_TXDES0_TXBUF_SIZE(x)   ((x) & 0x3fff)
> +#define FTGMAC100_TXDES0_EDOTR           (1 << 15)
> +#define FTGMAC100_TXDES0_CRC_ERR         (1 << 19)
> +#define FTGMAC100_TXDES0_LTS             (1 << 28)
> +#define FTGMAC100_TXDES0_FTS             (1 << 29)
> +#define FTGMAC100_TXDES0_TXDMA_OWN       (1 << 31)
> +
> +#define FTGMAC100_TXDES1_VLANTAG_CI(x)   ((x) & 0xffff)
> +#define FTGMAC100_TXDES1_INS_VLANTAG     (1 << 16)
> +#define FTGMAC100_TXDES1_TCP_CHKSUM      (1 << 17)
> +#define FTGMAC100_TXDES1_UDP_CHKSUM      (1 << 18)
> +#define FTGMAC100_TXDES1_IP_CHKSUM       (1 << 19)
> +#define FTGMAC100_TXDES1_LLC             (1 << 22)
> +#define FTGMAC100_TXDES1_TX2FIC          (1 << 30)
> +#define FTGMAC100_TXDES1_TXIC            (1 << 31)
> +
> +/*
> + * Receive descriptor, aligned to 16 bytes
> + */
> +struct ftgmac100_rxdes {
> +        unsigned int        rxdes0;
> +        unsigned int        rxdes1;
> +        unsigned int        rxdes2;      /* not used by HW */
> +        unsigned int        rxdes3;      /* RXBUF_BADR */
> +} __attribute__ ((aligned(16)));
> +
> +#define FTGMAC100_RXDES0_VDBC            0x3fff
> +#define FTGMAC100_RXDES0_EDORR           (1 << 15)
> +#define FTGMAC100_RXDES0_MULTICAST       (1 << 16)
> +#define FTGMAC100_RXDES0_BROADCAST       (1 << 17)
> +#define FTGMAC100_RXDES0_RX_ERR          (1 << 18)
> +#define FTGMAC100_RXDES0_CRC_ERR         (1 << 19)
> +#define FTGMAC100_RXDES0_FTL             (1 << 20)
> +#define FTGMAC100_RXDES0_RUNT            (1 << 21)
> +#define FTGMAC100_RXDES0_RX_ODD_NB       (1 << 22)
> +#define FTGMAC100_RXDES0_FIFO_FULL       (1 << 23)
> +#define FTGMAC100_RXDES0_PAUSE_OPCODE    (1 << 24)
> +#define FTGMAC100_RXDES0_PAUSE_FRAME     (1 << 25)
> +#define FTGMAC100_RXDES0_LRS             (1 << 28)
> +#define FTGMAC100_RXDES0_FRS             (1 << 29)
> +#define FTGMAC100_RXDES0_RXPKT_RDY       (1 << 31)
> +
> +#define FTGMAC100_RXDES1_VLANTAG_CI      0xffff
> +#define FTGMAC100_RXDES1_PROT_MASK       (0x3 << 20)
> +#define FTGMAC100_RXDES1_PROT_NONIP      (0x0 << 20)
> +#define FTGMAC100_RXDES1_PROT_IP         (0x1 << 20)
> +#define FTGMAC100_RXDES1_PROT_TCPIP      (0x2 << 20)
> +#define FTGMAC100_RXDES1_PROT_UDPIP      (0x3 << 20)
> +#define FTGMAC100_RXDES1_LLC             (1 << 22)
> +#define FTGMAC100_RXDES1_DF              (1 << 23)
> +#define FTGMAC100_RXDES1_VLANTAG_AVAIL   (1 << 24)
> +#define FTGMAC100_RXDES1_TCP_CHKSUM_ERR  (1 << 25)
> +#define FTGMAC100_RXDES1_UDP_CHKSUM_ERR  (1 << 26)
> +#define FTGMAC100_RXDES1_IP_CHKSUM_ERR   (1 << 27)
> +
> +/*
> + * PHY values (to be defined elsewhere ...)
> + */
> +#define PHY_INT_ENERGYON            (1 << 7)
> +#define PHY_INT_AUTONEG_COMPLETE    (1 << 6)
> +#define PHY_INT_FAULT               (1 << 5)
> +#define PHY_INT_DOWN                (1 << 4)
> +#define PHY_INT_AUTONEG_LP          (1 << 3)
> +#define PHY_INT_PARFAULT            (1 << 2)
> +#define PHY_INT_AUTONEG_PAGE        (1 << 1)
> +
> +/* Common Buffer Descriptor  */
> +typedef struct {
> +    uint32_t        des0;
> +    uint32_t        des1;
> +    uint32_t        des2;        /* not used by HW */
> +    uint32_t        des3;        /* TXBUF_BADR */
> +} Ftgmac100Desc  __attribute__ ((aligned(16)));
> +
> +/* max frame size is :
> + *
> + *   9216 for Jumbo frames (+ 4 for VLAN)
> + *   1518 for other frames (+ 4 for VLAN)
> + */
> +#define FTGMAC100_MAX_FRAME_SIZE(s)                             \
> +    ((s->maccr & FTGMAC100_MACCR_JUMBO_LF ? 9216 : 1518) + 4)

Is there a reason this has to be a macro rather than a function?

> +
> +static void ftgmac100_update_irq(Ftgmac100State *s);

You could just put the function definition here, right?

> +
> +/*
> + * The MII phy could raise a GPIO to the processor which in turn
> + * could be handled as an interrpt by the OS.
> + * For now we don't handle any GPIO/interrupt line, so the OS will
> + * have to poll for the PHY status.
> + */
> +static void phy_update_irq(Ftgmac100State *s)
> +{
> +    ftgmac100_update_irq(s);
> +}
> +
> +static void phy_update_link(Ftgmac100State *s)
> +{
> +    /* Autonegotiation status mirrors link status.  */
> +    if (qemu_get_queue(s->nic)->link_down) {
> +        s->phy_status &= ~0x0024;

Is there a useful symbolic constant or constants we could use
instead of this 0x24 ?

> +        s->phy_int |= PHY_INT_DOWN;
> +    } else {
> +        s->phy_status |= 0x0024;
> +        s->phy_int |= PHY_INT_ENERGYON;
> +        s->phy_int |= PHY_INT_AUTONEG_COMPLETE;
> +    }
> +    phy_update_irq(s);

This will end up doing a qemu_set_irq() from a device
reset function, which we don't recommend.

> +}
> +
> +static void ftgmac100_set_link(NetClientState *nc)
> +{
> +    phy_update_link(FTGMAC100(qemu_get_nic_opaque(nc)));
> +}
> +
> +static void phy_reset(Ftgmac100State *s)

I suspect there should be more capital letters in this
struct name -- we use camelcase but where there's an
acronym we leave it uppercase usually. So probably
FTGMAC100State.

> +{
> +    s->phy_status = 0x796d;
> +    s->phy_control = 0x1140;
> +    s->phy_advertise = 0x0de1;
> +    s->phy_int_mask = 0;
> +    s->phy_int = 0;
> +    phy_update_link(s);
> +}
> +
> +static uint32_t do_phy_read(Ftgmac100State *s, int reg)
> +{
> +    uint32_t val;
> +
> +    switch (reg) {
> +    case MII_BMCR: /* Basic Control */
> +        val = s->phy_control;
> +        break;
> +    case MII_BMSR: /* Basic Status */
> +        val = s->phy_status;
> +        break;
> +    case MII_PHYID1: /* ID1 */
> +        val = RTL8211E_PHYID1;
> +        break;
> +    case MII_PHYID2: /* ID2 */
> +        val = RTL8211E_PHYID2;
> +        break;
> +    case MII_ANAR: /* Auto-neg advertisement */
> +        val = s->phy_advertise;
> +        break;
> +    case MII_ANLPAR: /* Auto-neg Link Partner Ability */
> +        val = 0x45e1;
> +        break;
> +    case MII_ANER: /* Auto-neg Expansion */
> +        val = 1;
> +        break;
> +    case MII_CTRL1000: /* 1000BASE-T control  */
> +        val = 0x300;
> +        break;
> +    case MII_STAT1000: /* 1000BASE-T status  */
> +        val = 0x800;
> +        break;
> +    case 29:    /* Interrupt source.  */
> +        val = s->phy_int;
> +        s->phy_int = 0;
> +        phy_update_irq(s);
> +        break;
> +    case 30:    /* Interrupt mask */
> +        val = s->phy_int_mask;
> +        break;
> +    case MII_LBREMR:
> +    case MII_REC:
> +    case 27:
> +    case 31:

Why do only some of the cases here have symbolic names? Can we
define names for the others?

> +        qemu_log_mask(LOG_UNIMP, "%s: reg %d not implemented\n",
> +                      __func__, reg);
> +        val = 0;
> +        break;
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad address at offset %d\n",
> +                      __func__, reg);
> +        val = 0;
> +        break;
> +    }
> +
> +    return val;
> +}
> +
> +static void do_phy_write(Ftgmac100State *s, int reg, uint32_t val)
> +{
> +    switch (reg) {
> +    case MII_BMCR:     /* Basic Control */
> +        if (val & 0x8000) {
> +            phy_reset(s);
> +        } else {
> +            s->phy_control = val & 0x7980;
> +            /* Complete autonegotiation immediately.  */
> +            if (val & 0x1000) {
> +                s->phy_status |= 0x0020;
> +            }
> +        }
> +        break;
> +    case MII_ANAR:     /* Auto-neg advertisement */
> +        s->phy_advertise = (val & 0x2d7f) | 0x80;
> +        break;
> +    case 30:    /* Interrupt mask */
> +        s->phy_int_mask = val & 0xff;
> +        phy_update_irq(s);
> +        break;
> +    case MII_LBREMR:
> +    case MII_REC:
> +    case 27:
> +    case 31:
> +        qemu_log_mask(LOG_UNIMP, "%s: reg %d not implemented\n",
> +                      __func__, reg);
> +        break;
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad address at offset %d\n",
> +                      __func__, reg);
> +        break;
> +    }
> +}
> +
> +static void ftgmac100_read_bd(Ftgmac100Desc *bd, dma_addr_t addr)
> +{
> +    dma_memory_read(&address_space_memory, addr, bd, sizeof(*bd));

What should we do if the memory access fails? (The answer is probably
not "carry on regardless" since in that case we'll start doing
operations on bd->des* fields which are uninitialized memory.)

> +    bd->des0 = le32_to_cpu(bd->des0);
> +    bd->des1 = le32_to_cpu(bd->des1);
> +    bd->des2 = le32_to_cpu(bd->des2);
> +    bd->des3 = le32_to_cpu(bd->des3);

Note that since you've copied the structure out of guest memory
you don't care about the alignment of the Ftgmac100Desc struct in
host memory.

> +}
> +
> +static void ftgmac100_write_bd(Ftgmac100Desc *bd, dma_addr_t addr)
> +{
> +    Ftgmac100Desc lebd;
> +    lebd.des0 = cpu_to_le32(bd->des0);
> +    lebd.des1 = cpu_to_le32(bd->des1);
> +    lebd.des2 = cpu_to_le32(bd->des2);
> +    lebd.des3 = cpu_to_le32(bd->des3);
> +    dma_memory_write(&address_space_memory, addr, &lebd, sizeof(lebd));
> +}
> +
> +static void ftgmac100_update_irq(Ftgmac100State *s)
> +{
> +    uint32_t active;
> +    uint32_t changed;
> +
> +    active = s->isr & s->ier;
> +    changed = active ^ s->irq_state;
> +    if (changed) {
> +        qemu_set_irq(s->irq, active);
> +    }
> +    s->irq_state = active;

We don't generally track current outgoing irq state in devices
(it is awkward for reset and migration). Instead we just always
call qemu_set_irq() and let the destination end decide whether
anything has changed.

> +}
> +
> +/* Locate a possible first descriptor to transmit. When Linux resets
> + * the device, the indexes of ring buffers are cleared but the dma
> + * buffers are not, so we need to find a starting point.
> + */

Behaviour of Linux is an odd thing to reference here -- we care about
what the hardware does, not what the guest does. Is the datasheet
unclear here?

> +static uint32_t ftgmac100_find_txdes(Ftgmac100State *s, uint32_t addr)
> +{
> +    Ftgmac100Desc bd;
> +
> +    while (1) {
> +        ftgmac100_read_bd(&bd, addr);
> +        if (bd.des0 & (FTGMAC100_TXDES0_FTS | FTGMAC100_TXDES0_EDOTR)) {
> +            break;
> +        }
> +        addr += sizeof(Ftgmac100Desc);

This will step forwards through all of the guest address space
if the guest has helpfully populated it with bogus descriptor
structures. Is that really what the hardware does? I would have
expected it to go round in a ring.

We should probably also not allow the guest to let us go round
in an infinite loop, but I'm not sure what the current recommended
approach for that in QEMU is. Jason may know.

> +    }
> +    return addr;
> +}
> +
> +static void ftgmac100_do_tx(Ftgmac100State *s, uint32_t tx_ring,
> +                            uint32_t tx_descriptor)
> +{
> +    int frame_size = 0;
> +    uint8_t frame[FTGMAC100_MAX_FRAME_SIZE(s)];

Ah, this is why it was a macro.

9K is perhaps a bit big to put on the stack?

> +    uint8_t *ptr = frame;
> +    uint32_t addr;
> +    uint32_t flags = 0;
> +
> +    addr = ftgmac100_find_txdes(s, tx_descriptor);
> +
> +    while (1) {
> +        Ftgmac100Desc bd;
> +        int len;
> +
> +        ftgmac100_read_bd(&bd, addr);
> +        if ((bd.des0 & FTGMAC100_TXDES0_TXDMA_OWN) == 0) {
> +            /* Run out of descriptors to transmit.  */
> +            s->isr |= FTGMAC100_INT_NO_NPTXBUF;
> +            break;
> +        }
> +
> +        /* record transmit flags as they are valid only on the first
> +         * segment */
> +        if (bd.des0 & FTGMAC100_TXDES0_FTS) {
> +            flags = bd.des1;
> +        }
> +
> +        len = bd.des0 & 0x3FFF;
> +        if (frame_size + len > FTGMAC100_MAX_FRAME_SIZE(s)) {
> +            qemu_log_mask(LOG_GUEST_ERROR, "%s: frame too big : %d bytes\n",
> +                          __func__, len);
> +            len = FTGMAC100_MAX_FRAME_SIZE(s) - frame_size;
> +        }
> +        dma_memory_read(&address_space_memory, bd.des3, ptr, len);
> +        ptr += len;
> +        frame_size += len;
> +        if (bd.des0 & FTGMAC100_TXDES0_LTS) {
> +            if (flags & FTGMAC100_TXDES1_IP_CHKSUM) {
> +                net_checksum_calculate(frame, frame_size);
> +            }
> +            /* Last buffer in frame.  */
> +            qemu_send_packet(qemu_get_queue(s->nic), frame, frame_size);
> +            ptr = frame;
> +            frame_size = 0;
> +            if (flags & FTGMAC100_TXDES1_TXIC) {
> +                s->isr |= FTGMAC100_INT_XPKT_ETH;
> +            }
> +        }
> +
> +        if (flags & FTGMAC100_TXDES1_TX2FIC) {
> +            s->isr |= FTGMAC100_INT_XPKT_FIFO;
> +        }
> +        bd.des0 &= ~FTGMAC100_TXDES0_TXDMA_OWN;
> +
> +        /* Write back the modified descriptor.  */
> +        ftgmac100_write_bd(&bd, addr);
> +        /* Advance to the next descriptor.  */
> +        if (bd.des0 & FTGMAC100_TXDES0_EDOTR) {
> +            addr = tx_ring;
> +        } else {
> +            addr += sizeof(Ftgmac100Desc);
> +        }
> +    }
> +
> +    s->tx_descriptor = addr;
> +
> +    ftgmac100_update_irq(s);
> +}
> +
> +static int ftgmac100_can_receive(NetClientState *nc)
> +{
> +    Ftgmac100State *s = FTGMAC100(qemu_get_nic_opaque(nc));
> +    Ftgmac100Desc bd;
> +
> +    if ((s->maccr & (FTGMAC100_MACCR_RXDMA_EN | FTGMAC100_MACCR_RXMAC_EN))
> +         != (FTGMAC100_MACCR_RXDMA_EN | FTGMAC100_MACCR_RXMAC_EN)) {
> +        return 0;
> +    }
> +
> +    ftgmac100_read_bd(&bd, s->rx_descriptor);
> +    return !(bd.des0 & FTGMAC100_RXDES0_RXPKT_RDY);
> +}
> +
> +/*
> + * This is purely informative. The HW can poll the RW (and RX) ring
> + * buffers for available descriptors but we don't need to trigger a
> + * timer for that in qemu.
> + */
> +static uint32_t ftgmac100_rxpoll(Ftgmac100State *s)
> +{
> +    /* Polling times :
> +     *
> +     * Speed      TIME_SEL=0    TIME_SEL=1
> +     *
> +     *    10         51.2 ms      819.2 ms
> +     *   100         5.12 ms      81.92 ms
> +     *  1000        1.024 ms     16.384 ms
> +     */
> +    static const int div[] = { 20, 200, 1000 };
> +
> +    uint32_t cnt = 1024 * FTGMAC100_APTC_RXPOLL_CNT(s->aptcr);
> +    uint32_t speed = (s->maccr & FTGMAC100_MACCR_FAST_MODE) ? 1 : 0;
> +    uint32_t period;
> +
> +    if (s->aptcr & FTGMAC100_APTC_RXPOLL_TIME_SEL) {
> +        cnt <<= 4;
> +    }
> +
> +    if (s->maccr & FTGMAC100_MACCR_GIGA_MODE) {
> +        speed = 2;
> +    }
> +
> +    period = cnt / div[speed];
> +
> +    return period;
> +}
> +
> +static void ftgmac100_reset(DeviceState *d)
> +{
> +    Ftgmac100State *s = FTGMAC100(d);
> +
> +    /* Reset the FTGMAC100 */
> +    s->isr = 0;
> +    s->ier = 0;
> +    s->rx_enabled = 0;
> +    s->rx_ring = 0;
> +    s->rbsr = 0x640;
> +    s->rx_descriptor = 0;
> +    s->tx_ring = 0;
> +    s->tx_descriptor = 0;
> +    s->math[0] = 0;
> +    s->math[1] = 0;
> +    s->itc = 0;
> +    s->aptcr = 1;
> +    s->dblac = 0x00022f00;
> +    s->revr = 0;
> +    s->fear1 = 0;
> +    s->tpafcr = 0xf1;
> +
> +    s->maccr = 0;
> +    s->phycr = 0;
> +    s->phydata = 0;
> +    s->fcr = 0x400;
> +
> +    /* and the PHY */
> +    phy_reset(s);
> +
> +    ftgmac100_update_irq(s);

Again, updating irqs in reset functions is not recommended.

> +}
> +
> +static uint64_t ftgmac100_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +    Ftgmac100State *s = FTGMAC100(opaque);
> +
> +    switch (addr & 0xff) {
> +    case FTGMAC100_ISR:
> +        return s->isr;
> +    case FTGMAC100_IER:
> +        return s->ier;
> +    case FTGMAC100_MAC_MADR:
> +        return (s->conf.macaddr.a[0] << 8)  | s->conf.macaddr.a[1];
> +    case FTGMAC100_MAC_LADR:
> +        return (s->conf.macaddr.a[2] << 24) | (s->conf.macaddr.a[3] << 16) |
> +               (s->conf.macaddr.a[4] << 8)  |  s->conf.macaddr.a[5];

This will sign extend the high bit of macaddr.a[2] into bits 63..32 of
the return value, which probably isn't what you intended and will
make Coverity unhapppy.

> +    case FTGMAC100_MATH0:
> +        return s->math[0];
> +    case FTGMAC100_MATH1:
> +        return s->math[1];
> +    case FTGMAC100_ITC:
> +        return s->itc;
> +    case FTGMAC100_DBLAC:
> +        return s->dblac;
> +    case FTGMAC100_REVR:
> +        return s->revr;
> +    case FTGMAC100_FEAR1:
> +        return s->fear1;
> +    case FTGMAC100_TPAFCR:
> +        return s->tpafcr;
> +    case FTGMAC100_FCR:
> +        return s->fcr;
> +    case FTGMAC100_MACCR:
> +        return s->maccr;
> +    case FTGMAC100_PHYCR:
> +        return s->phycr;
> +    case FTGMAC100_PHYDATA:
> +        return s->phydata;
> +
> +    case FTGMAC100_MACSR: /* MAC Status Register (MACSR) */
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad address at offset 0x%"
> +                      HWADDR_PRIx "\n", __func__, addr);
> +        return 0;
> +    }
> +}
> +
> +static void ftgmac100_write(void *opaque, hwaddr addr,
> +                          uint64_t value, unsigned size)
> +{
> +    Ftgmac100State *s = FTGMAC100(opaque);
> +    int reg;
> +
> +    switch (addr & 0xff) {
> +    case FTGMAC100_ISR: /* Interrupt status */
> +        s->isr &= ~value;
> +        break;
> +    case FTGMAC100_IER: /* Interrupt control */
> +        s->ier = value;
> +        break;
> +    case FTGMAC100_MAC_MADR: /* MAC */
> +        s->conf.macaddr.a[0] = value >> 8;
> +        s->conf.macaddr.a[1] = value;
> +        break;
> +    case FTGMAC100_MAC_LADR:
> +        s->conf.macaddr.a[2] = value >> 24;
> +        s->conf.macaddr.a[3] = value >> 16;
> +        s->conf.macaddr.a[4] = value >> 8;
> +        s->conf.macaddr.a[5] = value;
> +        break;
> +    case FTGMAC100_MATH0: /* Multicast Address Hash Table 0 */
> +        s->math[0] = value;
> +        break;
> +    case FTGMAC100_MATH1: /* Multicast Address Hash Table 1 */
> +        s->math[1] = value;
> +        break;
> +    case FTGMAC100_ITC: /* TODO: Interrupt Timer Control */
> +        s->itc = value;
> +        break;
> +    case FTGMAC100_RXR_BADR: /* Ring buffer address */
> +        s->rx_ring = value;
> +        s->rx_descriptor = s->rx_ring;
> +        break;
> +
> +    case FTGMAC100_RBSR: /* DMA buffer size */
> +        s->rbsr = value;
> +        break;
> +
> +    case FTGMAC100_NPTXR_BADR: /* Transmit buffer address */
> +        s->tx_ring = value;
> +        s->tx_descriptor = s->tx_ring;
> +        break;
> +
> +    case FTGMAC100_NPTXPD: /* Trigger transmit */
> +        if ((s->maccr & (FTGMAC100_MACCR_TXDMA_EN | FTGMAC100_MACCR_TXMAC_EN))
> +            == (FTGMAC100_MACCR_TXDMA_EN | FTGMAC100_MACCR_TXMAC_EN)) {
> +            /* TODO: high priority tx ring */
> +            ftgmac100_do_tx(s, s->tx_ring, s->tx_descriptor);
> +        }
> +        if (ftgmac100_can_receive(qemu_get_queue(s->nic))) {
> +            qemu_flush_queued_packets(qemu_get_queue(s->nic));
> +        }
> +        break;
> +
> +    case FTGMAC100_RXPD: /* Receive Poll Demand Register */
> +        if (ftgmac100_can_receive(qemu_get_queue(s->nic))) {
> +            qemu_flush_queued_packets(qemu_get_queue(s->nic));
> +        }
> +        break;
> +
> +    case FTGMAC100_APTC: /* Automatic polling */
> +        s->aptcr = value;
> +
> +        if (FTGMAC100_APTC_RXPOLL_CNT(s->aptcr)) {
> +            ftgmac100_rxpoll(s);
> +        }
> +
> +        if (FTGMAC100_APTC_TXPOLL_CNT(s->aptcr)) {
> +            qemu_log_mask(LOG_UNIMP, "%s: no transmit polling\n", __func__);
> +        }
> +        break;
> +
> +    case FTGMAC100_MACCR: /* MAC Device control */
> +        s->maccr = value;
> +        if (value & FTGMAC100_MACCR_SW_RST) {
> +            ftgmac100_reset(DEVICE(s));
> +        }
> +
> +        if (ftgmac100_can_receive(qemu_get_queue(s->nic))) {
> +            qemu_flush_queued_packets(qemu_get_queue(s->nic));
> +        }
> +        break;
> +
> +    case FTGMAC100_PHYCR:  /* PHY Device control */
> +        reg = FTGMAC100_PHYCR_REG(value);
> +        s->phycr = value;
> +        if (value & FTGMAC100_PHYCR_MIIWR) {
> +            do_phy_write(s, reg, s->phydata & 0xffff);
> +            s->phycr &= ~FTGMAC100_PHYCR_MIIWR;
> +        } else {
> +            s->phydata = do_phy_read(s, reg) << 16;
> +            s->phycr &= ~FTGMAC100_PHYCR_MIIRD;
> +        }
> +        break;
> +    case FTGMAC100_PHYDATA:
> +        s->phydata = value & 0xffff;
> +        break;
> +    case FTGMAC100_DBLAC: /* DMA Burst Length and Arbitration Control */
> +        s->dblac = value;
> +        break;
> +    case FTGMAC100_REVR:  /* Feature Register */
> +        /* TODO: Only Old MDIO interface is supported */
> +        s->revr = value & ~FTGMAC100_REVR_NEW_MDIO_INTERFACE;
> +        break;
> +    case FTGMAC100_FEAR1: /* Feature Register 1 */
> +        s->fear1 = value;
> +        break;
> +    case FTGMAC100_TPAFCR: /* Transmit Priority Arbitration and FIFO Control */
> +        s->tpafcr = value;
> +        break;
> +    case FTGMAC100_FCR: /* Flow Control  */
> +        s->fcr  = value;
> +        break;
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad address at offset 0x%"
> +                      HWADDR_PRIx "\n", __func__, addr);
> +        break;
> +    }
> +
> +    ftgmac100_update_irq(s);
> +}
> +
> +static int ftgmac100_filter(Ftgmac100State *s, const uint8_t *buf, size_t len)
> +{
> +    unsigned mcast_idx;
> +
> +    if (s->maccr & FTGMAC100_MACCR_RX_ALL) {
> +        return 1;
> +    }
> +
> +    switch (get_eth_packet_type(PKT_GET_ETH_HDR(buf))) {
> +    case ETH_PKT_BCAST:
> +        if (!(s->maccr & FTGMAC100_MACCR_RX_BROADPKT)) {
> +            return 0;
> +        }
> +        break;
> +    case ETH_PKT_MCAST:
> +        if (!(s->maccr & FTGMAC100_MACCR_RX_MULTIPKT)) {
> +            if (!(s->maccr & FTGMAC100_MACCR_HT_MULTI_EN)) {
> +                return 0;
> +            }
> +
> +            /* TODO: this does not seem to work for ftgmac100 */
> +            mcast_idx = compute_mcast_idx(buf);
> +            if (!(s->math[mcast_idx / 32] & (1 << (mcast_idx % 32)))) {
> +                return 0;
> +            }
> +        }
> +        break;
> +    case ETH_PKT_UCAST:
> +        if (memcmp(s->conf.macaddr.a, buf, 6)) {
> +            return 0;
> +        }
> +        break;
> +    }
> +
> +    return 1;
> +}
> +
> +static ssize_t ftgmac100_receive(NetClientState *nc, const uint8_t *buf,
> +                                 size_t len)
> +{
> +    Ftgmac100State *s = FTGMAC100(qemu_get_nic_opaque(nc));
> +    Ftgmac100Desc bd;
> +    uint32_t flags = 0;
> +    uint32_t addr;
> +    uint32_t crc;
> +    uint32_t buf_addr;
> +    uint8_t *crc_ptr;
> +    unsigned int buf_len;
> +    size_t size = len;
> +    uint32_t first = FTGMAC100_RXDES0_FRS;
> +
> +    if ((s->maccr & (FTGMAC100_MACCR_RXDMA_EN | FTGMAC100_MACCR_RXMAC_EN))
> +         != (FTGMAC100_MACCR_RXDMA_EN | FTGMAC100_MACCR_RXMAC_EN)) {
> +        return -1;
> +    }
> +
> +    /* TODO : Pad to minimum Ethernet frame length */
> +    /* handle small packets.  */
> +    if (size < 10) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: dropped frame of %zd bytes\n",
> +                      __func__, size);
> +        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;
> +    }
> +
> +    /* 4 bytes for the CRC.  */
> +    size += 4;
> +    crc = cpu_to_be32(crc32(~0, buf, size));
> +    crc_ptr = (uint8_t *) &crc;
> +
> +    /* Huge frames are truncted.  */

"truncated"

> +    if (size > FTGMAC100_MAX_FRAME_SIZE(s)) {
> +        size = FTGMAC100_MAX_FRAME_SIZE(s);
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: frame too big : %zd bytes\n",
> +                      __func__, size);
> +        flags |= FTGMAC100_RXDES0_FTL;
> +    }
> +
> +    switch (get_eth_packet_type(PKT_GET_ETH_HDR(buf))) {
> +    case ETH_PKT_BCAST:
> +        flags |= FTGMAC100_RXDES0_BROADCAST;
> +        break;
> +    case ETH_PKT_MCAST:
> +        flags |= FTGMAC100_RXDES0_MULTICAST;
> +        break;
> +    case ETH_PKT_UCAST:
> +        break;
> +    }
> +
> +    addr = s->rx_descriptor;
> +    while (size > 0) {
> +        if (!ftgmac100_can_receive(nc)) {
> +            qemu_log_mask(LOG_GUEST_ERROR, "%s: Unexpected packet\n", __func__);
> +            return -1;
> +        }
> +
> +        ftgmac100_read_bd(&bd, addr);
> +        if (bd.des0 & FTGMAC100_RXDES0_RXPKT_RDY) {
> +            /* No descriptors available.  Bail out.  */
> +            qemu_log_mask(LOG_GUEST_ERROR, "%s: Lost end of frame\n",
> +                          __func__);
> +            s->isr |= FTGMAC100_INT_NO_RXBUF;
> +            break;
> +        }
> +        buf_len = (size <= s->rbsr) ? size : s->rbsr;
> +        bd.des0 |= buf_len & 0x3fff;
> +        size -= buf_len;
> +
> +        /* The last 4 bytes are the CRC.  */
> +        if (size < 4) {
> +            buf_len += size - 4;
> +        }
> +        buf_addr = bd.des3;
> +        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,
> +                             crc_ptr, 4 - size);
> +            crc_ptr += 4 - size;
> +        }
> +
> +        bd.des0 |= first | FTGMAC100_RXDES0_RXPKT_RDY;
> +        first = 0;
> +        if (size == 0) {
> +            /* Last buffer in frame.  */
> +            bd.des0 |= flags | FTGMAC100_RXDES0_LRS;
> +            s->isr |= FTGMAC100_INT_RPKT_BUF;
> +        } else {
> +            s->isr |= FTGMAC100_INT_RPKT_FIFO;
> +        }
> +        ftgmac100_write_bd(&bd, addr);
> +        if (bd.des0 & FTGMAC100_RXDES0_EDORR) {
> +            addr = s->rx_ring;
> +        } else {
> +            addr += sizeof(Ftgmac100Desc);
> +        }
> +    }
> +    s->rx_descriptor = addr;
> +
> +    ftgmac100_update_irq(s);
> +    return len;
> +}
> +
> +static const MemoryRegionOps ftgmac100_ops = {
> +    .read = ftgmac100_read,
> +    .write = ftgmac100_write,
> +    .valid.min_access_size = 4,
> +    .valid.max_access_size = 4,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
> +};
> +
> +static void ftgmac100_cleanup(NetClientState *nc)
> +{
> +    Ftgmac100State *s = FTGMAC100(qemu_get_nic_opaque(nc));
> +
> +    s->nic = NULL;
> +}
> +
> +static NetClientInfo net_ftgmac100_info = {
> +    .type = NET_CLIENT_DRIVER_NIC,
> +    .size = sizeof(NICState),
> +    .can_receive = ftgmac100_can_receive,
> +    .receive = ftgmac100_receive,
> +    .cleanup = ftgmac100_cleanup,
> +    .link_status_changed = ftgmac100_set_link,
> +};
> +
> +static void ftgmac100_realize(DeviceState *dev, Error **errp)
> +{
> +    Ftgmac100State *s = FTGMAC100(dev);
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> +
> +    memory_region_init_io(&s->iomem, OBJECT(dev), &ftgmac100_ops, s,
> +                          TYPE_FTGMAC100, 0x2000);
> +    sysbus_init_mmio(sbd, &s->iomem);
> +    sysbus_init_irq(sbd, &s->irq);
> +    qemu_macaddr_default_if_unset(&s->conf.macaddr);
> +
> +    s->conf.peers.ncs[0] = nd_table[0].netdev;
> +
> +    s->nic = qemu_new_nic(&net_ftgmac100_info, &s->conf,
> +                          object_get_typename(OBJECT(dev)), DEVICE(dev)->id,
> +                          s);
> +    qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a);
> +}
> +
> +static const VMStateDescription vmstate_ftgmac100 = {
> +    .name = TYPE_FTGMAC100,
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT32(irq_state, Ftgmac100State),
> +        VMSTATE_UINT32(isr, Ftgmac100State),
> +        VMSTATE_UINT32(ier, Ftgmac100State),
> +        VMSTATE_UINT32(rx_enabled, Ftgmac100State),
> +        VMSTATE_UINT32(rx_ring, Ftgmac100State),
> +        VMSTATE_UINT32(rbsr, Ftgmac100State),
> +        VMSTATE_UINT32(tx_ring, Ftgmac100State),
> +        VMSTATE_UINT32(rx_descriptor, Ftgmac100State),
> +        VMSTATE_UINT32(tx_descriptor, Ftgmac100State),
> +        VMSTATE_UINT32_ARRAY(math, Ftgmac100State, 2),
> +        VMSTATE_UINT32(itc, Ftgmac100State),
> +        VMSTATE_UINT32(aptcr, Ftgmac100State),
> +        VMSTATE_UINT32(dblac, Ftgmac100State),
> +        VMSTATE_UINT32(revr, Ftgmac100State),
> +        VMSTATE_UINT32(fear1, Ftgmac100State),
> +        VMSTATE_UINT32(tpafcr, Ftgmac100State),
> +        VMSTATE_UINT32(maccr, Ftgmac100State),
> +        VMSTATE_UINT32(phycr, Ftgmac100State),
> +        VMSTATE_UINT32(phydata, Ftgmac100State),
> +        VMSTATE_UINT32(fcr, Ftgmac100State),
> +        VMSTATE_UINT32(phy_status, Ftgmac100State),
> +        VMSTATE_UINT32(phy_control, Ftgmac100State),
> +        VMSTATE_UINT32(phy_advertise, Ftgmac100State),
> +        VMSTATE_UINT32(phy_int, Ftgmac100State),
> +        VMSTATE_UINT32(phy_int_mask, Ftgmac100State),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static Property ftgmac100_properties[] = {
> +    DEFINE_NIC_PROPERTIES(Ftgmac100State, conf),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void ftgmac100_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->vmsd = &vmstate_ftgmac100;
> +    dc->reset = ftgmac100_reset;
> +    dc->props = ftgmac100_properties;
> +    set_bit(DEVICE_CATEGORY_NETWORK, dc->categories);
> +    dc->realize = ftgmac100_realize;
> +    dc->desc = "Faraday FTGMAC100 Gigabit Ethernet emulation";
> +}
> +
> +static const TypeInfo ftgmac100_info = {
> +    .name = TYPE_FTGMAC100,
> +    .parent = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(Ftgmac100State),
> +    .class_init = ftgmac100_class_init,
> +};
> +
> +static void ftgmac100_register_types(void)
> +{
> +    type_register_static(&ftgmac100_info);
> +}
> +
> +type_init(ftgmac100_register_types)
> diff --git a/include/hw/net/ftgmac100.h b/include/hw/net/ftgmac100.h
> new file mode 100644
> index 000000000000..3bb4fe1a3ece
> --- /dev/null
> +++ b/include/hw/net/ftgmac100.h
> @@ -0,0 +1,58 @@
> +/*
> + * Faraday FTGMAC100 Gigabit Ethernet
> + *
> + * Copyright (C) 2016 IBM Corp.
> + *
> + * This code is licensed under the GPL version 2 or later. See the
> + * COPYING file in the top-level directory.
> + */
> +
> +#ifndef FTGMAC100_H
> +#define FTGMAC100_H
> +
> +#define TYPE_FTGMAC100 "ftgmac100"
> +#define FTGMAC100(obj) OBJECT_CHECK(Ftgmac100State, (obj), TYPE_FTGMAC100)
> +
> +#include "hw/sysbus.h"
> +#include "net/net.h"
> +
> +typedef struct Ftgmac100State {
> +    /*< private >*/
> +    SysBusDevice parent_obj;
> +
> +    /*< public >*/
> +    NICState *nic;
> +    NICConf conf;
> +    qemu_irq irq;
> +    MemoryRegion iomem;
> +
> +    uint32_t irq_state;
> +    uint32_t isr;
> +    uint32_t ier;
> +    uint32_t rx_enabled;
> +    uint32_t rx_ring;
> +    uint32_t rx_descriptor;
> +    uint32_t tx_ring;
> +    uint32_t tx_descriptor;
> +    uint32_t math[2];
> +    uint32_t rbsr;
> +    uint32_t itc;
> +    uint32_t aptcr;
> +    uint32_t dblac;
> +    uint32_t revr;
> +    uint32_t fear1;
> +    uint32_t tpafcr;
> +    uint32_t maccr;
> +    uint32_t phycr;
> +    uint32_t phydata;
> +    uint32_t fcr;
> +
> +
> +    uint32_t phy_status;
> +    uint32_t phy_control;
> +    uint32_t phy_advertise;
> +    uint32_t phy_int;
> +    uint32_t phy_int_mask;
> +} Ftgmac100State;
> +
> +#endif
> diff --git a/include/hw/net/mii.h b/include/hw/net/mii.h
> index 9fdd7bbe75f1..f820acdb4a19 100644
> --- a/include/hw/net/mii.h
> +++ b/include/hw/net/mii.h
> @@ -29,6 +29,8 @@
>  #define MII_ANAR            4
>  #define MII_ANLPAR          5
>  #define MII_ANER            6
> +#define MII_CTRL1000        9
> +#define MII_STAT1000        10
>  #define MII_NSR             16
>  #define MII_LBREMR          17
>  #define MII_REC             18
> @@ -69,6 +71,10 @@
>  #define RTL8201CP_PHYID1    0x0000
>  #define RTL8201CP_PHYID2    0x8201
>
> +/* RealTek 8211E */
> +#define RTL8211E_PHYID1    0x001c
> +#define RTL8211E_PHYID2    0xc915
> +
>  /* National Semiconductor DP83848 */
>  #define DP83848_PHYID1      0x2000
>  #define DP83848_PHYID2      0x5c90
> --
> 2.7.4
>

thanks
-- PMM

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH 1/4] net: add FTGMAC100 support
  2017-04-10 13:43   ` Peter Maydell
@ 2017-04-12 17:29     ` Cédric Le Goater
  2017-04-12 17:37       ` Peter Maydell
  0 siblings, 1 reply; 8+ messages in thread
From: Cédric Le Goater @ 2017-04-12 17:29 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Jason Wang, Dmitry Fleytman, Samuel Thibault, Jan Kiszka,
	QEMU Developers, qemu-arm

On 04/10/2017 03:43 PM, Peter Maydell wrote:
> On 1 April 2017 at 13:57, Cédric Le Goater <clg@kaod.org> wrote:
>> The FTGMAC100 device is an Ethernet controller with DMA function that
>> can be found on Aspeed SoCs (which include NCSI).
>>
>> It is fully compliant with IEEE 802.3 specification for 10/100 Mbps
>> Ethernet and IEEE 802.3z specification for 1000 Mbps Ethernet and
>> includes Reduced Media Independent Interface (RMII) and Reduced
>> Gigabit Media Independent Interface (RGMII) interfaces. It adopts an
>> AHB bus interface and integrates a link list DMA engine with direct
>> M-Bus accesses for transmitting and receiving packets. It has
>> independent TX/RX fifos, supports half and full duplex (1000 Mbps mode
>> only supports full duplex), flow control for full duplex and
>> backpressure for half duplex.
>>
>> The FTGMAC100 also implements IP, TCP, UDP checksum offloads and
>> supports IEEE 802.1Q VLAN tag insertion and removal. It offers
>> high-priority transmit queue for QoS and CoS applications
>>
>> This model is complete enough to satisfy two different Linux drivers
>> and a U-Boot driver. Not supported features are :
>>
>>  - IEEE 802.1Q VLAN
>>  - High Priority Transmit Queue
>>  - Wake-On-LAN functions
>>
>> The code is based on the Coldfire Fast Ethernet Controller model.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>  default-configs/arm-softmmu.mak |   1 +
>>  hw/net/Makefile.objs            |   1 +
>>  hw/net/ftgmac100.c              | 977 ++++++++++++++++++++++++++++++++++++++++
>>  include/hw/net/ftgmac100.h      |  58 +++
>>  include/hw/net/mii.h            |   6 +
>>  5 files changed, 1043 insertions(+)
>>  create mode 100644 hw/net/ftgmac100.c
>>  create mode 100644 include/hw/net/ftgmac100.h
>>
>> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
>> index 15a53598d1c3..acc4c6c86297 100644
>> --- a/default-configs/arm-softmmu.mak
>> +++ b/default-configs/arm-softmmu.mak
>> @@ -30,6 +30,7 @@ CONFIG_LAN9118=y
>>  CONFIG_SMC91C111=y
>>  CONFIG_ALLWINNER_EMAC=y
>>  CONFIG_IMX_FEC=y
>> +CONFIG_FTGMAC100=y
>>  CONFIG_DS1338=y
>>  CONFIG_RX8900=y
>>  CONFIG_PFLASH_CFI01=y
>> diff --git a/hw/net/Makefile.objs b/hw/net/Makefile.objs
>> index 6a95d92d37f8..5ddaffe63a46 100644
>> --- a/hw/net/Makefile.objs
>> +++ b/hw/net/Makefile.objs
>> @@ -26,6 +26,7 @@ common-obj-$(CONFIG_IMX_FEC) += imx_fec.o
>>  common-obj-$(CONFIG_CADENCE) += cadence_gem.o
>>  common-obj-$(CONFIG_STELLARIS_ENET) += stellaris_enet.o
>>  common-obj-$(CONFIG_LANCE) += lance.o
>> +common-obj-$(CONFIG_FTGMAC100) += ftgmac100.o
>>
>>  obj-$(CONFIG_ETRAXFS) += etraxfs_eth.o
>>  obj-$(CONFIG_COLDFIRE) += mcf_fec.o
>> diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c
>> new file mode 100644
>> index 000000000000..331e87391962
>> --- /dev/null
>> +++ b/hw/net/ftgmac100.c
>> @@ -0,0 +1,977 @@
>> +/*
>> + * Faraday FTGMAC100 Gigabit Ethernet
>> + *
>> + * Copyright (C) 2016 IBM Corp.
>> + *
>> + * Based on Coldfire Fast Ethernet Controller emulation.
>> + *
>> + * Copyright (c) 2007 CodeSourcery.
>> + *
>> + * This code is licensed under the GPL version 2 or later. See the
>> + * COPYING file in the top-level directory.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "hw/net/ftgmac100.h"
>> +#include "sysemu/dma.h"
>> +#include "qemu/log.h"
>> +#include "net/checksum.h"
>> +#include "net/eth.h"
>> +#include "hw/net/mii.h"
>> +
>> +/* For crc32 */
>> +#include <zlib.h>
>> +
>> +/*
>> + * FTGMAC100 registers
>> + */
>> +#define FTGMAC100_ISR             0x00
>> +#define FTGMAC100_IER             0x04
>> +#define FTGMAC100_MAC_MADR        0x08
>> +#define FTGMAC100_MAC_LADR        0x0c
>> +#define FTGMAC100_MATH0           0x10
>> +#define FTGMAC100_MATH1           0x14
>> +#define FTGMAC100_NPTXPD          0x18
>> +#define FTGMAC100_RXPD            0x1C
>> +#define FTGMAC100_NPTXR_BADR      0x20
>> +#define FTGMAC100_RXR_BADR        0x24
>> +#define FTGMAC100_HPTXPD          0x28 /* TODO */
>> +#define FTGMAC100_HPTXR_BADR      0x2c /* TODO */
> 
> What's TODO about a #define ?
> Maybe better to have TODO comment and LOG_UNIMP at the register/read
> write point?

yes. I have changed that. 

> 
>> +#define FTGMAC100_ITC             0x30
>> +#define FTGMAC100_APTC            0x34
>> +#define FTGMAC100_DBLAC           0x38
>> +#define FTGMAC100_REVR            0x40
>> +#define FTGMAC100_FEAR1           0x44
>> +#define FTGMAC100_RBSR            0x4c
>> +#define FTGMAC100_TPAFCR          0x48
>> +
>> +#define FTGMAC100_MACCR           0x50
>> +#define FTGMAC100_MACSR           0x54 /* TODO */
>> +#define FTGMAC100_PHYCR           0x60
>> +#define FTGMAC100_PHYDATA         0x64
>> +#define FTGMAC100_FCR             0x68
>> +
>> +/*
>> + * Interrupt status register & interrupt enable register
>> + */
>> +#define FTGMAC100_INT_RPKT_BUF    (1 << 0)
>> +#define FTGMAC100_INT_RPKT_FIFO   (1 << 1)
>> +#define FTGMAC100_INT_NO_RXBUF    (1 << 2)
>> +#define FTGMAC100_INT_RPKT_LOST   (1 << 3)
>> +#define FTGMAC100_INT_XPKT_ETH    (1 << 4)
>> +#define FTGMAC100_INT_XPKT_FIFO   (1 << 5)
>> +#define FTGMAC100_INT_NO_NPTXBUF  (1 << 6)
>> +#define FTGMAC100_INT_XPKT_LOST   (1 << 7)
>> +#define FTGMAC100_INT_AHB_ERR     (1 << 8)
>> +#define FTGMAC100_INT_PHYSTS_CHG  (1 << 9)
>> +#define FTGMAC100_INT_NO_HPTXBUF  (1 << 10)
>> +
>> +/*
>> + * Automatic polling timer control register
>> + */
>> +#define FTGMAC100_APTC_RXPOLL_CNT(x)        ((x) & 0xf)
>> +#define FTGMAC100_APTC_RXPOLL_TIME_SEL      (1 << 4)
>> +#define FTGMAC100_APTC_TXPOLL_CNT(x)        (((x) >> 8) & 0xf)
>> +#define FTGMAC100_APTC_TXPOLL_TIME_SEL      (1 << 12)
>> +
>> +/*
>> + * PHY control register
>> + */
>> +#define FTGMAC100_PHYCR_MIIRD               (1 << 26)
>> +#define FTGMAC100_PHYCR_MIIWR               (1 << 27)
>> +
>> +#define FTGMAC100_PHYCR_DEV(v)              (((v) >> 16) & 0x1f)
>> +#define FTGMAC100_PHYCR_REG(v)              (((v) >> 21) & 0x1f)
>> +
>> +/*
>> + * PHY data register
>> + */
>> +#define FTGMAC100_PHYDATA_MIIWDATA(x)       ((x) & 0xffff)
>> +#define FTGMAC100_PHYDATA_MIIRDATA(phydata) (((phydata) >> 16) & 0xffff)
> 
> There's some inconsistency here between 'x' and 'v' and 'phydata'
> for macro parameter names.

Fixed.

>> +
>> +/*
>> + * Feature Register
>> + */
>> +#define FTGMAC100_REVR_NEW_MDIO_INTERFACE   (1 << 31)
>> +
>> +/*
>> + * MAC control register
>> + */
>> +#define FTGMAC100_MACCR_TXDMA_EN         (1 << 0)
>> +#define FTGMAC100_MACCR_RXDMA_EN         (1 << 1)
>> +#define FTGMAC100_MACCR_TXMAC_EN         (1 << 2)
>> +#define FTGMAC100_MACCR_RXMAC_EN         (1 << 3)
>> +#define FTGMAC100_MACCR_RM_VLAN          (1 << 4)
>> +#define FTGMAC100_MACCR_HPTXR_EN         (1 << 5)
>> +#define FTGMAC100_MACCR_LOOP_EN          (1 << 6)
>> +#define FTGMAC100_MACCR_ENRX_IN_HALFTX   (1 << 7)
>> +#define FTGMAC100_MACCR_FULLDUP          (1 << 8)
>> +#define FTGMAC100_MACCR_GIGA_MODE        (1 << 9)
>> +#define FTGMAC100_MACCR_CRC_APD          (1 << 10) /* not needed */
>> +#define FTGMAC100_MACCR_RX_RUNT          (1 << 12)
>> +#define FTGMAC100_MACCR_JUMBO_LF         (1 << 13)
>> +#define FTGMAC100_MACCR_RX_ALL           (1 << 14)
>> +#define FTGMAC100_MACCR_HT_MULTI_EN      (1 << 15)
>> +#define FTGMAC100_MACCR_RX_MULTIPKT      (1 << 16)
>> +#define FTGMAC100_MACCR_RX_BROADPKT      (1 << 17)
>> +#define FTGMAC100_MACCR_DISCARD_CRCERR   (1 << 18)
>> +#define FTGMAC100_MACCR_FAST_MODE        (1 << 19)
>> +#define FTGMAC100_MACCR_SW_RST           (1 << 31)
>> +
>> +/*
>> + * Transmit descriptor, aligned to 16 bytes
>> + */
>> +struct ftgmac100_txdes {
> 
> Coding standard says camelcase for struct names (and typedefs).

Indeed.
 
>> +        unsigned int        txdes0;
>> +        unsigned int        txdes1;
>> +        unsigned int        txdes2;      /* not used by HW */
>> +        unsigned int        txdes3;      /* TXBUF_BADR */
>> +} __attribute__ ((aligned(16)));
> 
> Why does this need to be attribute aligned ? Typically
> you don't want to be using C structs for things actually
> in guest memory IMHO. (I think your uses of it are all
> 

I have removed these struct definitions. They came from Linux and
were unused.

> If the fields have to be exactly 32 bits then uint32_t is
> a clearer way to write them.

yes.
 
>> +
>> +#define FTGMAC100_TXDES0_TXBUF_SIZE(x)   ((x) & 0x3fff)
>> +#define FTGMAC100_TXDES0_EDOTR           (1 << 15)
>> +#define FTGMAC100_TXDES0_CRC_ERR         (1 << 19)
>> +#define FTGMAC100_TXDES0_LTS             (1 << 28)
>> +#define FTGMAC100_TXDES0_FTS             (1 << 29)
>> +#define FTGMAC100_TXDES0_TXDMA_OWN       (1 << 31)
>> +
>> +#define FTGMAC100_TXDES1_VLANTAG_CI(x)   ((x) & 0xffff)
>> +#define FTGMAC100_TXDES1_INS_VLANTAG     (1 << 16)
>> +#define FTGMAC100_TXDES1_TCP_CHKSUM      (1 << 17)
>> +#define FTGMAC100_TXDES1_UDP_CHKSUM      (1 << 18)
>> +#define FTGMAC100_TXDES1_IP_CHKSUM       (1 << 19)
>> +#define FTGMAC100_TXDES1_LLC             (1 << 22)
>> +#define FTGMAC100_TXDES1_TX2FIC          (1 << 30)
>> +#define FTGMAC100_TXDES1_TXIC            (1 << 31)
>> +
>> +/*
>> + * Receive descriptor, aligned to 16 bytes
>> + */
>> +struct ftgmac100_rxdes {
>> +        unsigned int        rxdes0;
>> +        unsigned int        rxdes1;
>> +        unsigned int        rxdes2;      /* not used by HW */
>> +        unsigned int        rxdes3;      /* RXBUF_BADR */
>> +} __attribute__ ((aligned(16)));
>> +
>> +#define FTGMAC100_RXDES0_VDBC            0x3fff
>> +#define FTGMAC100_RXDES0_EDORR           (1 << 15)
>> +#define FTGMAC100_RXDES0_MULTICAST       (1 << 16)
>> +#define FTGMAC100_RXDES0_BROADCAST       (1 << 17)
>> +#define FTGMAC100_RXDES0_RX_ERR          (1 << 18)
>> +#define FTGMAC100_RXDES0_CRC_ERR         (1 << 19)
>> +#define FTGMAC100_RXDES0_FTL             (1 << 20)
>> +#define FTGMAC100_RXDES0_RUNT            (1 << 21)
>> +#define FTGMAC100_RXDES0_RX_ODD_NB       (1 << 22)
>> +#define FTGMAC100_RXDES0_FIFO_FULL       (1 << 23)
>> +#define FTGMAC100_RXDES0_PAUSE_OPCODE    (1 << 24)
>> +#define FTGMAC100_RXDES0_PAUSE_FRAME     (1 << 25)
>> +#define FTGMAC100_RXDES0_LRS             (1 << 28)
>> +#define FTGMAC100_RXDES0_FRS             (1 << 29)
>> +#define FTGMAC100_RXDES0_RXPKT_RDY       (1 << 31)
>> +
>> +#define FTGMAC100_RXDES1_VLANTAG_CI      0xffff
>> +#define FTGMAC100_RXDES1_PROT_MASK       (0x3 << 20)
>> +#define FTGMAC100_RXDES1_PROT_NONIP      (0x0 << 20)
>> +#define FTGMAC100_RXDES1_PROT_IP         (0x1 << 20)
>> +#define FTGMAC100_RXDES1_PROT_TCPIP      (0x2 << 20)
>> +#define FTGMAC100_RXDES1_PROT_UDPIP      (0x3 << 20)
>> +#define FTGMAC100_RXDES1_LLC             (1 << 22)
>> +#define FTGMAC100_RXDES1_DF              (1 << 23)
>> +#define FTGMAC100_RXDES1_VLANTAG_AVAIL   (1 << 24)
>> +#define FTGMAC100_RXDES1_TCP_CHKSUM_ERR  (1 << 25)
>> +#define FTGMAC100_RXDES1_UDP_CHKSUM_ERR  (1 << 26)
>> +#define FTGMAC100_RXDES1_IP_CHKSUM_ERR   (1 << 27)
>> +
>> +/*
>> + * PHY values (to be defined elsewhere ...)
>> + */
>> +#define PHY_INT_ENERGYON            (1 << 7)
>> +#define PHY_INT_AUTONEG_COMPLETE    (1 << 6)
>> +#define PHY_INT_FAULT               (1 << 5)
>> +#define PHY_INT_DOWN                (1 << 4)
>> +#define PHY_INT_AUTONEG_LP          (1 << 3)
>> +#define PHY_INT_PARFAULT            (1 << 2)
>> +#define PHY_INT_AUTONEG_PAGE        (1 << 1)
>> +
>> +/* Common Buffer Descriptor  */
>> +typedef struct {
>> +    uint32_t        des0;
>> +    uint32_t        des1;
>> +    uint32_t        des2;        /* not used by HW */
>> +    uint32_t        des3;        /* TXBUF_BADR */
>> +} Ftgmac100Desc  __attribute__ ((aligned(16)));
>> +
>> +/* max frame size is :
>> + *
>> + *   9216 for Jumbo frames (+ 4 for VLAN)
>> + *   1518 for other frames (+ 4 for VLAN)
>> + */
>> +#define FTGMAC100_MAX_FRAME_SIZE(s)                             \
>> +    ((s->maccr & FTGMAC100_MACCR_JUMBO_LF ? 9216 : 1518) + 4)
> 
> Is there a reason this has to be a macro rather than a function?

This macro is now a fixed value of 10240 and the frame buffer is 
allocated in the realize fnction of the FTGMAC100State Object
 
>> +
>> +static void ftgmac100_update_irq(Ftgmac100State *s);
> 
> You could just put the function definition here, right?

yes.

>> +
>> +/*
>> + * The MII phy could raise a GPIO to the processor which in turn
>> + * could be handled as an interrpt by the OS.
>> + * For now we don't handle any GPIO/interrupt line, so the OS will
>> + * have to poll for the PHY status.
>> + */
>> +static void phy_update_irq(Ftgmac100State *s)
>> +{
>> +    ftgmac100_update_irq(s);
>> +}
>> +
>> +static void phy_update_link(Ftgmac100State *s)
>> +{
>> +    /* Autonegotiation status mirrors link status.  */
>> +    if (qemu_get_queue(s->nic)->link_down) {
>> +        s->phy_status &= ~0x0024;
> 
> Is there a useful symbolic constant or constants we could use
> instead of this 0x24 ?

sigh. I will need to check the specs ...

>> +        s->phy_int |= PHY_INT_DOWN;
>> +    } else {
>> +        s->phy_status |= 0x0024;
>> +        s->phy_int |= PHY_INT_ENERGYON;
>> +        s->phy_int |= PHY_INT_AUTONEG_COMPLETE;
>> +    }
>> +    phy_update_irq(s);
> 
> This will end up doing a qemu_set_irq() from a device
> reset function, which we don't recommend.

OK.

>> +}
>> +
>> +static void ftgmac100_set_link(NetClientState *nc)
>> +{
>> +    phy_update_link(FTGMAC100(qemu_get_nic_opaque(nc)));
>> +}
>> +
>> +static void phy_reset(Ftgmac100State *s)
> 
> I suspect there should be more capital letters in this
> struct name -- we use camelcase but where there's an
> acronym we leave it uppercase usually. So probably
> FTGMAC100State.

I have used FTGMAC100State in the next version.
>> +{
>> +    s->phy_status = 0x796d;
>> +    s->phy_control = 0x1140;
>> +    s->phy_advertise = 0x0de1;
>> +    s->phy_int_mask = 0;
>> +    s->phy_int = 0;
>> +    phy_update_link(s);
>> +}
>> +
>> +static uint32_t do_phy_read(Ftgmac100State *s, int reg)
>> +{
>> +    uint32_t val;
>> +
>> +    switch (reg) {
>> +    case MII_BMCR: /* Basic Control */
>> +        val = s->phy_control;
>> +        break;
>> +    case MII_BMSR: /* Basic Status */
>> +        val = s->phy_status;
>> +        break;
>> +    case MII_PHYID1: /* ID1 */
>> +        val = RTL8211E_PHYID1;
>> +        break;
>> +    case MII_PHYID2: /* ID2 */
>> +        val = RTL8211E_PHYID2;
>> +        break;
>> +    case MII_ANAR: /* Auto-neg advertisement */
>> +        val = s->phy_advertise;
>> +        break;
>> +    case MII_ANLPAR: /* Auto-neg Link Partner Ability */
>> +        val = 0x45e1;
>> +        break;
>> +    case MII_ANER: /* Auto-neg Expansion */
>> +        val = 1;
>> +        break;
>> +    case MII_CTRL1000: /* 1000BASE-T control  */
>> +        val = 0x300;
>> +        break;
>> +    case MII_STAT1000: /* 1000BASE-T status  */
>> +        val = 0x800;
>> +        break;
>> +    case 29:    /* Interrupt source.  */
>> +        val = s->phy_int;
>> +        s->phy_int = 0;
>> +        phy_update_irq(s);
>> +        break;
>> +    case 30:    /* Interrupt mask */
>> +        val = s->phy_int_mask;
>> +        break;
>> +    case MII_LBREMR:
>> +    case MII_REC:
>> +    case 27:
>> +    case 31:
> 
> Why do only some of the cases here have symbolic names? Can we
> define names for the others?

Well, yes. I will try to. 

As this model is pretending using a rtl8211e PHY chip, we should know 
which register is which and improve the naming. 

>> +        qemu_log_mask(LOG_UNIMP, "%s: reg %d not implemented\n",
>> +                      __func__, reg);
>> +        val = 0;
>> +        break;
>> +    default:
>> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad address at offset %d\n",
>> +                      __func__, reg);
>> +        val = 0;
>> +        break;
>> +    }
>> +
>> +    return val;
>> +}
>> +
>> +static void do_phy_write(Ftgmac100State *s, int reg, uint32_t val)
>> +{
>> +    switch (reg) {
>> +    case MII_BMCR:     /* Basic Control */
>> +        if (val & 0x8000) {
>> +            phy_reset(s);
>> +        } else {
>> +            s->phy_control = val & 0x7980;
>> +            /* Complete autonegotiation immediately.  */
>> +            if (val & 0x1000) {
>> +                s->phy_status |= 0x0020;
>> +            }
>> +        }
>> +        break;
>> +    case MII_ANAR:     /* Auto-neg advertisement */
>> +        s->phy_advertise = (val & 0x2d7f) | 0x80;
>> +        break;
>> +    case 30:    /* Interrupt mask */
>> +        s->phy_int_mask = val & 0xff;
>> +        phy_update_irq(s);
>> +        break;
>> +    case MII_LBREMR:
>> +    case MII_REC:
>> +    case 27:
>> +    case 31:
>> +        qemu_log_mask(LOG_UNIMP, "%s: reg %d not implemented\n",
>> +                      __func__, reg);
>> +        break;
>> +    default:
>> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad address at offset %d\n",
>> +                      __func__, reg);
>> +        break;
>> +    }
>> +}
>> +
>> +static void ftgmac100_read_bd(Ftgmac100Desc *bd, dma_addr_t addr)
>> +{
>> +    dma_memory_read(&address_space_memory, addr, bd, sizeof(*bd));
> 
> What should we do if the memory access fails? (The answer is probably
> not "carry on regardless" since in that case we'll start doing
> operations on bd->des* fields which are uninitialized memory.)

yes. I fixed that for reads.

>> +    bd->des0 = le32_to_cpu(bd->des0);
>> +    bd->des1 = le32_to_cpu(bd->des1);
>> +    bd->des2 = le32_to_cpu(bd->des2);
>> +    bd->des3 = le32_to_cpu(bd->des3);
> 
> Note that since you've copied the structure out of guest memory
> you don't care about the alignment of the Ftgmac100Desc struct in
> host memory.

yep. I have removed the alignment.
 
>> +}
>> +
>> +static void ftgmac100_write_bd(Ftgmac100Desc *bd, dma_addr_t addr)
>> +{
>> +    Ftgmac100Desc lebd;
>> +    lebd.des0 = cpu_to_le32(bd->des0);
>> +    lebd.des1 = cpu_to_le32(bd->des1);
>> +    lebd.des2 = cpu_to_le32(bd->des2);
>> +    lebd.des3 = cpu_to_le32(bd->des3);
>> +    dma_memory_write(&address_space_memory, addr, &lebd, sizeof(lebd));
>> +}
>> +
>> +static void ftgmac100_update_irq(Ftgmac100State *s)
>> +{
>> +    uint32_t active;
>> +    uint32_t changed;
>> +
>> +    active = s->isr & s->ier;
>> +    changed = active ^ s->irq_state;
>> +    if (changed) {
>> +        qemu_set_irq(s->irq, active);
>> +    }
>> +    s->irq_state = active;
> 
> We don't generally track current outgoing irq state in devices
> (it is awkward for reset and migration). Instead we just always
> call qemu_set_irq() and let the destination end decide whether
> anything has changed.

ok. 

>> +}
>> +
>> +/* Locate a possible first descriptor to transmit. When Linux resets
>> + * the device, the indexes of ring buffers are cleared but the dma
>> + * buffers are not, so we need to find a starting point.
>> + */
> 
> Behaviour of Linux is an odd thing to reference here -- we care about
> what the hardware does, not what the guest does. Is the datasheet
> unclear here?

No. This is useless crap from my early beginnings. I just removed it,
and all is fine. Sorry for the noise.
 
>> +static uint32_t ftgmac100_find_txdes(Ftgmac100State *s, uint32_t addr)
>> +{
>> +    Ftgmac100Desc bd;
>> +
>> +    while (1) {
>> +        ftgmac100_read_bd(&bd, addr);
>> +        if (bd.des0 & (FTGMAC100_TXDES0_FTS | FTGMAC100_TXDES0_EDOTR)) {
>> +            break;
>> +        }
>> +        addr += sizeof(Ftgmac100Desc);
> 
> This will step forwards through all of the guest address space
> if the guest has helpfully populated it with bogus descriptor
> structures. Is that really what the hardware does? I would have
> expected it to go round in a ring.
> 
> We should probably also not allow the guest to let us go round
> in an infinite loop, but I'm not sure what the current recommended
> approach for that in QEMU is. Jason may know.
>
>> +    }
>> +    return addr;
>> +}
>> +
>> +static void ftgmac100_do_tx(Ftgmac100State *s, uint32_t tx_ring,
>> +                            uint32_t tx_descriptor)
>> +{
>> +    int frame_size = 0;
>> +    uint8_t frame[FTGMAC100_MAX_FRAME_SIZE(s)];
> 
> Ah, this is why it was a macro.
> 
> 9K is perhaps a bit big to put on the stack?

So as a replacement, I have used an allocated frame in the realize 
function. I hope this is ok.
 
>> +    uint8_t *ptr = frame;
>> +    uint32_t addr;
>> +    uint32_t flags = 0;
>> +
>> +    addr = ftgmac100_find_txdes(s, tx_descriptor);
>> +
>> +    while (1) {
>> +        Ftgmac100Desc bd;
>> +        int len;
>> +
>> +        ftgmac100_read_bd(&bd, addr);
>> +        if ((bd.des0 & FTGMAC100_TXDES0_TXDMA_OWN) == 0) {
>> +            /* Run out of descriptors to transmit.  */
>> +            s->isr |= FTGMAC100_INT_NO_NPTXBUF;
>> +            break;
>> +        }
>> +
>> +        /* record transmit flags as they are valid only on the first
>> +         * segment */
>> +        if (bd.des0 & FTGMAC100_TXDES0_FTS) {
>> +            flags = bd.des1;
>> +        }
>> +
>> +        len = bd.des0 & 0x3FFF;
>> +        if (frame_size + len > FTGMAC100_MAX_FRAME_SIZE(s)) {
>> +            qemu_log_mask(LOG_GUEST_ERROR, "%s: frame too big : %d bytes\n",
>> +                          __func__, len);
>> +            len = FTGMAC100_MAX_FRAME_SIZE(s) - frame_size;
>> +        }
>> +        dma_memory_read(&address_space_memory, bd.des3, ptr, len);
>> +        ptr += len;
>> +        frame_size += len;
>> +        if (bd.des0 & FTGMAC100_TXDES0_LTS) {
>> +            if (flags & FTGMAC100_TXDES1_IP_CHKSUM) {
>> +                net_checksum_calculate(frame, frame_size);
>> +            }
>> +            /* Last buffer in frame.  */
>> +            qemu_send_packet(qemu_get_queue(s->nic), frame, frame_size);
>> +            ptr = frame;
>> +            frame_size = 0;
>> +            if (flags & FTGMAC100_TXDES1_TXIC) {
>> +                s->isr |= FTGMAC100_INT_XPKT_ETH;
>> +            }
>> +        }
>> +
>> +        if (flags & FTGMAC100_TXDES1_TX2FIC) {
>> +            s->isr |= FTGMAC100_INT_XPKT_FIFO;
>> +        }
>> +        bd.des0 &= ~FTGMAC100_TXDES0_TXDMA_OWN;
>> +
>> +        /* Write back the modified descriptor.  */
>> +        ftgmac100_write_bd(&bd, addr);
>> +        /* Advance to the next descriptor.  */
>> +        if (bd.des0 & FTGMAC100_TXDES0_EDOTR) {
>> +            addr = tx_ring;
>> +        } else {
>> +            addr += sizeof(Ftgmac100Desc);
>> +        }
>> +    }
>> +
>> +    s->tx_descriptor = addr;
>> +
>> +    ftgmac100_update_irq(s);
>> +}
>> +
>> +static int ftgmac100_can_receive(NetClientState *nc)
>> +{
>> +    Ftgmac100State *s = FTGMAC100(qemu_get_nic_opaque(nc));
>> +    Ftgmac100Desc bd;
>> +
>> +    if ((s->maccr & (FTGMAC100_MACCR_RXDMA_EN | FTGMAC100_MACCR_RXMAC_EN))
>> +         != (FTGMAC100_MACCR_RXDMA_EN | FTGMAC100_MACCR_RXMAC_EN)) {
>> +        return 0;
>> +    }
>> +
>> +    ftgmac100_read_bd(&bd, s->rx_descriptor);
>> +    return !(bd.des0 & FTGMAC100_RXDES0_RXPKT_RDY);
>> +}
>> +
>> +/*
>> + * This is purely informative. The HW can poll the RW (and RX) ring
>> + * buffers for available descriptors but we don't need to trigger a
>> + * timer for that in qemu.
>> + */
>> +static uint32_t ftgmac100_rxpoll(Ftgmac100State *s)
>> +{
>> +    /* Polling times :
>> +     *
>> +     * Speed      TIME_SEL=0    TIME_SEL=1
>> +     *
>> +     *    10         51.2 ms      819.2 ms
>> +     *   100         5.12 ms      81.92 ms
>> +     *  1000        1.024 ms     16.384 ms
>> +     */
>> +    static const int div[] = { 20, 200, 1000 };
>> +
>> +    uint32_t cnt = 1024 * FTGMAC100_APTC_RXPOLL_CNT(s->aptcr);
>> +    uint32_t speed = (s->maccr & FTGMAC100_MACCR_FAST_MODE) ? 1 : 0;
>> +    uint32_t period;
>> +
>> +    if (s->aptcr & FTGMAC100_APTC_RXPOLL_TIME_SEL) {
>> +        cnt <<= 4;
>> +    }
>> +
>> +    if (s->maccr & FTGMAC100_MACCR_GIGA_MODE) {
>> +        speed = 2;
>> +    }
>> +
>> +    period = cnt / div[speed];
>> +
>> +    return period;
>> +}
>> +
>> +static void ftgmac100_reset(DeviceState *d)
>> +{
>> +    Ftgmac100State *s = FTGMAC100(d);
>> +
>> +    /* Reset the FTGMAC100 */
>> +    s->isr = 0;
>> +    s->ier = 0;
>> +    s->rx_enabled = 0;
>> +    s->rx_ring = 0;
>> +    s->rbsr = 0x640;
>> +    s->rx_descriptor = 0;
>> +    s->tx_ring = 0;
>> +    s->tx_descriptor = 0;
>> +    s->math[0] = 0;
>> +    s->math[1] = 0;
>> +    s->itc = 0;
>> +    s->aptcr = 1;
>> +    s->dblac = 0x00022f00;
>> +    s->revr = 0;
>> +    s->fear1 = 0;
>> +    s->tpafcr = 0xf1;
>> +
>> +    s->maccr = 0;
>> +    s->phycr = 0;
>> +    s->phydata = 0;
>> +    s->fcr = 0x400;
>> +
>> +    /* and the PHY */
>> +    phy_reset(s);
>> +
>> +    ftgmac100_update_irq(s);
> 
> Again, updating irqs in reset functions is not recommended.

OK.

>> +}
>> +
>> +static uint64_t ftgmac100_read(void *opaque, hwaddr addr, unsigned size)
>> +{
>> +    Ftgmac100State *s = FTGMAC100(opaque);
>> +
>> +    switch (addr & 0xff) {
>> +    case FTGMAC100_ISR:
>> +        return s->isr;
>> +    case FTGMAC100_IER:
>> +        return s->ier;
>> +    case FTGMAC100_MAC_MADR:
>> +        return (s->conf.macaddr.a[0] << 8)  | s->conf.macaddr.a[1];
>> +    case FTGMAC100_MAC_LADR:
>> +        return (s->conf.macaddr.a[2] << 24) | (s->conf.macaddr.a[3] << 16) |
>> +               (s->conf.macaddr.a[4] << 8)  |  s->conf.macaddr.a[5];
> 
> This will sign extend the high bit of macaddr.a[2] into bits 63..32 of
> the return value, which probably isn't what you intended and will
> make Coverity unhapppy.

indeed. What would you recommand ? simply :

	((uint64_t) s->conf.macaddr.a[2] << 24) | ...



>> +    case FTGMAC100_MATH0:
>> +        return s->math[0];
>> +    case FTGMAC100_MATH1:
>> +        return s->math[1];
>> +    case FTGMAC100_ITC:
>> +        return s->itc;
>> +    case FTGMAC100_DBLAC:
>> +        return s->dblac;
>> +    case FTGMAC100_REVR:
>> +        return s->revr;
>> +    case FTGMAC100_FEAR1:
>> +        return s->fear1;
>> +    case FTGMAC100_TPAFCR:
>> +        return s->tpafcr;
>> +    case FTGMAC100_FCR:
>> +        return s->fcr;
>> +    case FTGMAC100_MACCR:
>> +        return s->maccr;
>> +    case FTGMAC100_PHYCR:
>> +        return s->phycr;
>> +    case FTGMAC100_PHYDATA:
>> +        return s->phydata;
>> +
>> +    case FTGMAC100_MACSR: /* MAC Status Register (MACSR) */
>> +    default:
>> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad address at offset 0x%"
>> +                      HWADDR_PRIx "\n", __func__, addr);
>> +        return 0;
>> +    }
>> +}
>> +
>> +static void ftgmac100_write(void *opaque, hwaddr addr,
>> +                          uint64_t value, unsigned size)
>> +{
>> +    Ftgmac100State *s = FTGMAC100(opaque);
>> +    int reg;
>> +
>> +    switch (addr & 0xff) {
>> +    case FTGMAC100_ISR: /* Interrupt status */
>> +        s->isr &= ~value;
>> +        break;
>> +    case FTGMAC100_IER: /* Interrupt control */
>> +        s->ier = value;
>> +        break;
>> +    case FTGMAC100_MAC_MADR: /* MAC */
>> +        s->conf.macaddr.a[0] = value >> 8;
>> +        s->conf.macaddr.a[1] = value;
>> +        break;
>> +    case FTGMAC100_MAC_LADR:
>> +        s->conf.macaddr.a[2] = value >> 24;
>> +        s->conf.macaddr.a[3] = value >> 16;
>> +        s->conf.macaddr.a[4] = value >> 8;
>> +        s->conf.macaddr.a[5] = value;
>> +        break;
>> +    case FTGMAC100_MATH0: /* Multicast Address Hash Table 0 */
>> +        s->math[0] = value;
>> +        break;
>> +    case FTGMAC100_MATH1: /* Multicast Address Hash Table 1 */
>> +        s->math[1] = value;
>> +        break;
>> +    case FTGMAC100_ITC: /* TODO: Interrupt Timer Control */
>> +        s->itc = value;
>> +        break;
>> +    case FTGMAC100_RXR_BADR: /* Ring buffer address */
>> +        s->rx_ring = value;
>> +        s->rx_descriptor = s->rx_ring;
>> +        break;
>> +
>> +    case FTGMAC100_RBSR: /* DMA buffer size */
>> +        s->rbsr = value;
>> +        break;
>> +
>> +    case FTGMAC100_NPTXR_BADR: /* Transmit buffer address */
>> +        s->tx_ring = value;
>> +        s->tx_descriptor = s->tx_ring;
>> +        break;
>> +
>> +    case FTGMAC100_NPTXPD: /* Trigger transmit */
>> +        if ((s->maccr & (FTGMAC100_MACCR_TXDMA_EN | FTGMAC100_MACCR_TXMAC_EN))
>> +            == (FTGMAC100_MACCR_TXDMA_EN | FTGMAC100_MACCR_TXMAC_EN)) {
>> +            /* TODO: high priority tx ring */
>> +            ftgmac100_do_tx(s, s->tx_ring, s->tx_descriptor);
>> +        }
>> +        if (ftgmac100_can_receive(qemu_get_queue(s->nic))) {
>> +            qemu_flush_queued_packets(qemu_get_queue(s->nic));
>> +        }
>> +        break;
>> +
>> +    case FTGMAC100_RXPD: /* Receive Poll Demand Register */
>> +        if (ftgmac100_can_receive(qemu_get_queue(s->nic))) {
>> +            qemu_flush_queued_packets(qemu_get_queue(s->nic));
>> +        }
>> +        break;
>> +
>> +    case FTGMAC100_APTC: /* Automatic polling */
>> +        s->aptcr = value;
>> +
>> +        if (FTGMAC100_APTC_RXPOLL_CNT(s->aptcr)) {
>> +            ftgmac100_rxpoll(s);
>> +        }
>> +
>> +        if (FTGMAC100_APTC_TXPOLL_CNT(s->aptcr)) {
>> +            qemu_log_mask(LOG_UNIMP, "%s: no transmit polling\n", __func__);
>> +        }
>> +        break;
>> +
>> +    case FTGMAC100_MACCR: /* MAC Device control */
>> +        s->maccr = value;
>> +        if (value & FTGMAC100_MACCR_SW_RST) {
>> +            ftgmac100_reset(DEVICE(s));
>> +        }
>> +
>> +        if (ftgmac100_can_receive(qemu_get_queue(s->nic))) {
>> +            qemu_flush_queued_packets(qemu_get_queue(s->nic));
>> +        }
>> +        break;
>> +
>> +    case FTGMAC100_PHYCR:  /* PHY Device control */
>> +        reg = FTGMAC100_PHYCR_REG(value);
>> +        s->phycr = value;
>> +        if (value & FTGMAC100_PHYCR_MIIWR) {
>> +            do_phy_write(s, reg, s->phydata & 0xffff);
>> +            s->phycr &= ~FTGMAC100_PHYCR_MIIWR;
>> +        } else {
>> +            s->phydata = do_phy_read(s, reg) << 16;
>> +            s->phycr &= ~FTGMAC100_PHYCR_MIIRD;
>> +        }
>> +        break;
>> +    case FTGMAC100_PHYDATA:
>> +        s->phydata = value & 0xffff;
>> +        break;
>> +    case FTGMAC100_DBLAC: /* DMA Burst Length and Arbitration Control */
>> +        s->dblac = value;
>> +        break;
>> +    case FTGMAC100_REVR:  /* Feature Register */
>> +        /* TODO: Only Old MDIO interface is supported */
>> +        s->revr = value & ~FTGMAC100_REVR_NEW_MDIO_INTERFACE;
>> +        break;
>> +    case FTGMAC100_FEAR1: /* Feature Register 1 */
>> +        s->fear1 = value;
>> +        break;
>> +    case FTGMAC100_TPAFCR: /* Transmit Priority Arbitration and FIFO Control */
>> +        s->tpafcr = value;
>> +        break;
>> +    case FTGMAC100_FCR: /* Flow Control  */
>> +        s->fcr  = value;
>> +        break;
>> +    default:
>> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad address at offset 0x%"
>> +                      HWADDR_PRIx "\n", __func__, addr);
>> +        break;
>> +    }
>> +
>> +    ftgmac100_update_irq(s);
>> +}
>> +
>> +static int ftgmac100_filter(Ftgmac100State *s, const uint8_t *buf, size_t len)
>> +{
>> +    unsigned mcast_idx;
>> +
>> +    if (s->maccr & FTGMAC100_MACCR_RX_ALL) {
>> +        return 1;
>> +    }
>> +
>> +    switch (get_eth_packet_type(PKT_GET_ETH_HDR(buf))) {
>> +    case ETH_PKT_BCAST:
>> +        if (!(s->maccr & FTGMAC100_MACCR_RX_BROADPKT)) {
>> +            return 0;
>> +        }
>> +        break;
>> +    case ETH_PKT_MCAST:
>> +        if (!(s->maccr & FTGMAC100_MACCR_RX_MULTIPKT)) {
>> +            if (!(s->maccr & FTGMAC100_MACCR_HT_MULTI_EN)) {
>> +                return 0;
>> +            }
>> +
>> +            /* TODO: this does not seem to work for ftgmac100 */
>> +            mcast_idx = compute_mcast_idx(buf);
>> +            if (!(s->math[mcast_idx / 32] & (1 << (mcast_idx % 32)))) {
>> +                return 0;
>> +            }
>> +        }
>> +        break;
>> +    case ETH_PKT_UCAST:
>> +        if (memcmp(s->conf.macaddr.a, buf, 6)) {
>> +            return 0;
>> +        }
>> +        break;
>> +    }
>> +
>> +    return 1;
>> +}
>> +
>> +static ssize_t ftgmac100_receive(NetClientState *nc, const uint8_t *buf,
>> +                                 size_t len)
>> +{
>> +    Ftgmac100State *s = FTGMAC100(qemu_get_nic_opaque(nc));
>> +    Ftgmac100Desc bd;
>> +    uint32_t flags = 0;
>> +    uint32_t addr;
>> +    uint32_t crc;
>> +    uint32_t buf_addr;
>> +    uint8_t *crc_ptr;
>> +    unsigned int buf_len;
>> +    size_t size = len;
>> +    uint32_t first = FTGMAC100_RXDES0_FRS;
>> +
>> +    if ((s->maccr & (FTGMAC100_MACCR_RXDMA_EN | FTGMAC100_MACCR_RXMAC_EN))
>> +         != (FTGMAC100_MACCR_RXDMA_EN | FTGMAC100_MACCR_RXMAC_EN)) {
>> +        return -1;
>> +    }
>> +
>> +    /* TODO : Pad to minimum Ethernet frame length */
>> +    /* handle small packets.  */
>> +    if (size < 10) {
>> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: dropped frame of %zd bytes\n",
>> +                      __func__, size);
>> +        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;
>> +    }
>> +
>> +    /* 4 bytes for the CRC.  */
>> +    size += 4;
>> +    crc = cpu_to_be32(crc32(~0, buf, size));
>> +    crc_ptr = (uint8_t *) &crc;
>> +
>> +    /* Huge frames are truncted.  */
> 
> "truncated"
> 
>> +    if (size > FTGMAC100_MAX_FRAME_SIZE(s)) {
>> +        size = FTGMAC100_MAX_FRAME_SIZE(s);
>> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: frame too big : %zd bytes\n",
>> +                      __func__, size);
>> +        flags |= FTGMAC100_RXDES0_FTL;
>> +    }
>> +
>> +    switch (get_eth_packet_type(PKT_GET_ETH_HDR(buf))) {
>> +    case ETH_PKT_BCAST:
>> +        flags |= FTGMAC100_RXDES0_BROADCAST;
>> +        break;
>> +    case ETH_PKT_MCAST:
>> +        flags |= FTGMAC100_RXDES0_MULTICAST;
>> +        break;
>> +    case ETH_PKT_UCAST:
>> +        break;
>> +    }
>> +
>> +    addr = s->rx_descriptor;
>> +    while (size > 0) {
>> +        if (!ftgmac100_can_receive(nc)) {
>> +            qemu_log_mask(LOG_GUEST_ERROR, "%s: Unexpected packet\n", __func__);
>> +            return -1;
>> +        }
>> +
>> +        ftgmac100_read_bd(&bd, addr);
>> +        if (bd.des0 & FTGMAC100_RXDES0_RXPKT_RDY) {
>> +            /* No descriptors available.  Bail out.  */
>> +            qemu_log_mask(LOG_GUEST_ERROR, "%s: Lost end of frame\n",
>> +                          __func__);
>> +            s->isr |= FTGMAC100_INT_NO_RXBUF;
>> +            break;
>> +        }
>> +        buf_len = (size <= s->rbsr) ? size : s->rbsr;
>> +        bd.des0 |= buf_len & 0x3fff;
>> +        size -= buf_len;
>> +
>> +        /* The last 4 bytes are the CRC.  */
>> +        if (size < 4) {
>> +            buf_len += size - 4;
>> +        }
>> +        buf_addr = bd.des3;
>> +        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,
>> +                             crc_ptr, 4 - size);
>> +            crc_ptr += 4 - size;
>> +        }
>> +
>> +        bd.des0 |= first | FTGMAC100_RXDES0_RXPKT_RDY;
>> +        first = 0;
>> +        if (size == 0) {
>> +            /* Last buffer in frame.  */
>> +            bd.des0 |= flags | FTGMAC100_RXDES0_LRS;
>> +            s->isr |= FTGMAC100_INT_RPKT_BUF;
>> +        } else {
>> +            s->isr |= FTGMAC100_INT_RPKT_FIFO;
>> +        }
>> +        ftgmac100_write_bd(&bd, addr);
>> +        if (bd.des0 & FTGMAC100_RXDES0_EDORR) {
>> +            addr = s->rx_ring;
>> +        } else {
>> +            addr += sizeof(Ftgmac100Desc);
>> +        }
>> +    }
>> +    s->rx_descriptor = addr;
>> +
>> +    ftgmac100_update_irq(s);
>> +    return len;
>> +}
>> +
>> +static const MemoryRegionOps ftgmac100_ops = {
>> +    .read = ftgmac100_read,
>> +    .write = ftgmac100_write,
>> +    .valid.min_access_size = 4,
>> +    .valid.max_access_size = 4,
>> +    .endianness = DEVICE_LITTLE_ENDIAN,
>> +};
>> +
>> +static void ftgmac100_cleanup(NetClientState *nc)
>> +{
>> +    Ftgmac100State *s = FTGMAC100(qemu_get_nic_opaque(nc));
>> +
>> +    s->nic = NULL;
>> +}
>> +
>> +static NetClientInfo net_ftgmac100_info = {
>> +    .type = NET_CLIENT_DRIVER_NIC,
>> +    .size = sizeof(NICState),
>> +    .can_receive = ftgmac100_can_receive,
>> +    .receive = ftgmac100_receive,
>> +    .cleanup = ftgmac100_cleanup,
>> +    .link_status_changed = ftgmac100_set_link,
>> +};
>> +
>> +static void ftgmac100_realize(DeviceState *dev, Error **errp)
>> +{
>> +    Ftgmac100State *s = FTGMAC100(dev);
>> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>> +
>> +    memory_region_init_io(&s->iomem, OBJECT(dev), &ftgmac100_ops, s,
>> +                          TYPE_FTGMAC100, 0x2000);
>> +    sysbus_init_mmio(sbd, &s->iomem);
>> +    sysbus_init_irq(sbd, &s->irq);
>> +    qemu_macaddr_default_if_unset(&s->conf.macaddr);
>> +
>> +    s->conf.peers.ncs[0] = nd_table[0].netdev;
>> +
>> +    s->nic = qemu_new_nic(&net_ftgmac100_info, &s->conf,
>> +                          object_get_typename(OBJECT(dev)), DEVICE(dev)->id,
>> +                          s);
>> +    qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a);
>> +}
>> +
>> +static const VMStateDescription vmstate_ftgmac100 = {
>> +    .name = TYPE_FTGMAC100,
>> +    .version_id = 1,
>> +    .minimum_version_id = 1,
>> +    .fields = (VMStateField[]) {
>> +        VMSTATE_UINT32(irq_state, Ftgmac100State),
>> +        VMSTATE_UINT32(isr, Ftgmac100State),
>> +        VMSTATE_UINT32(ier, Ftgmac100State),
>> +        VMSTATE_UINT32(rx_enabled, Ftgmac100State),
>> +        VMSTATE_UINT32(rx_ring, Ftgmac100State),
>> +        VMSTATE_UINT32(rbsr, Ftgmac100State),
>> +        VMSTATE_UINT32(tx_ring, Ftgmac100State),
>> +        VMSTATE_UINT32(rx_descriptor, Ftgmac100State),
>> +        VMSTATE_UINT32(tx_descriptor, Ftgmac100State),
>> +        VMSTATE_UINT32_ARRAY(math, Ftgmac100State, 2),
>> +        VMSTATE_UINT32(itc, Ftgmac100State),
>> +        VMSTATE_UINT32(aptcr, Ftgmac100State),
>> +        VMSTATE_UINT32(dblac, Ftgmac100State),
>> +        VMSTATE_UINT32(revr, Ftgmac100State),
>> +        VMSTATE_UINT32(fear1, Ftgmac100State),
>> +        VMSTATE_UINT32(tpafcr, Ftgmac100State),
>> +        VMSTATE_UINT32(maccr, Ftgmac100State),
>> +        VMSTATE_UINT32(phycr, Ftgmac100State),
>> +        VMSTATE_UINT32(phydata, Ftgmac100State),
>> +        VMSTATE_UINT32(fcr, Ftgmac100State),
>> +        VMSTATE_UINT32(phy_status, Ftgmac100State),
>> +        VMSTATE_UINT32(phy_control, Ftgmac100State),
>> +        VMSTATE_UINT32(phy_advertise, Ftgmac100State),
>> +        VMSTATE_UINT32(phy_int, Ftgmac100State),
>> +        VMSTATE_UINT32(phy_int_mask, Ftgmac100State),
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
>> +
>> +static Property ftgmac100_properties[] = {
>> +    DEFINE_NIC_PROPERTIES(Ftgmac100State, conf),
>> +    DEFINE_PROP_END_OF_LIST(),
>> +};
>> +
>> +static void ftgmac100_class_init(ObjectClass *klass, void *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +
>> +    dc->vmsd = &vmstate_ftgmac100;
>> +    dc->reset = ftgmac100_reset;
>> +    dc->props = ftgmac100_properties;
>> +    set_bit(DEVICE_CATEGORY_NETWORK, dc->categories);
>> +    dc->realize = ftgmac100_realize;
>> +    dc->desc = "Faraday FTGMAC100 Gigabit Ethernet emulation";
>> +}
>> +
>> +static const TypeInfo ftgmac100_info = {
>> +    .name = TYPE_FTGMAC100,
>> +    .parent = TYPE_SYS_BUS_DEVICE,
>> +    .instance_size = sizeof(Ftgmac100State),
>> +    .class_init = ftgmac100_class_init,
>> +};
>> +
>> +static void ftgmac100_register_types(void)
>> +{
>> +    type_register_static(&ftgmac100_info);
>> +}
>> +
>> +type_init(ftgmac100_register_types)
>> diff --git a/include/hw/net/ftgmac100.h b/include/hw/net/ftgmac100.h
>> new file mode 100644
>> index 000000000000..3bb4fe1a3ece
>> --- /dev/null
>> +++ b/include/hw/net/ftgmac100.h
>> @@ -0,0 +1,58 @@
>> +/*
>> + * Faraday FTGMAC100 Gigabit Ethernet
>> + *
>> + * Copyright (C) 2016 IBM Corp.
>> + *
>> + * This code is licensed under the GPL version 2 or later. See the
>> + * COPYING file in the top-level directory.
>> + */
>> +
>> +#ifndef FTGMAC100_H
>> +#define FTGMAC100_H
>> +
>> +#define TYPE_FTGMAC100 "ftgmac100"
>> +#define FTGMAC100(obj) OBJECT_CHECK(Ftgmac100State, (obj), TYPE_FTGMAC100)
>> +
>> +#include "hw/sysbus.h"
>> +#include "net/net.h"
>> +
>> +typedef struct Ftgmac100State {
>> +    /*< private >*/
>> +    SysBusDevice parent_obj;
>> +
>> +    /*< public >*/
>> +    NICState *nic;
>> +    NICConf conf;
>> +    qemu_irq irq;
>> +    MemoryRegion iomem;
>> +
>> +    uint32_t irq_state;
>> +    uint32_t isr;
>> +    uint32_t ier;
>> +    uint32_t rx_enabled;
>> +    uint32_t rx_ring;
>> +    uint32_t rx_descriptor;
>> +    uint32_t tx_ring;
>> +    uint32_t tx_descriptor;
>> +    uint32_t math[2];
>> +    uint32_t rbsr;
>> +    uint32_t itc;
>> +    uint32_t aptcr;
>> +    uint32_t dblac;
>> +    uint32_t revr;
>> +    uint32_t fear1;
>> +    uint32_t tpafcr;
>> +    uint32_t maccr;
>> +    uint32_t phycr;
>> +    uint32_t phydata;
>> +    uint32_t fcr;
>> +
>> +
>> +    uint32_t phy_status;
>> +    uint32_t phy_control;
>> +    uint32_t phy_advertise;
>> +    uint32_t phy_int;
>> +    uint32_t phy_int_mask;
>> +} Ftgmac100State;
>> +
>> +#endif
>> diff --git a/include/hw/net/mii.h b/include/hw/net/mii.h
>> index 9fdd7bbe75f1..f820acdb4a19 100644
>> --- a/include/hw/net/mii.h
>> +++ b/include/hw/net/mii.h
>> @@ -29,6 +29,8 @@
>>  #define MII_ANAR            4
>>  #define MII_ANLPAR          5
>>  #define MII_ANER            6
>> +#define MII_CTRL1000        9
>> +#define MII_STAT1000        10
>>  #define MII_NSR             16
>>  #define MII_LBREMR          17
>>  #define MII_REC             18
>> @@ -69,6 +71,10 @@
>>  #define RTL8201CP_PHYID1    0x0000
>>  #define RTL8201CP_PHYID2    0x8201
>>
>> +/* RealTek 8211E */
>> +#define RTL8211E_PHYID1    0x001c
>> +#define RTL8211E_PHYID2    0xc915
>> +
>>  /* National Semiconductor DP83848 */
>>  #define DP83848_PHYID1      0x2000
>>  #define DP83848_PHYID2      0x5c90
>> --
>> 2.7.4
>>

Thanks for the review,

C. 

> thanks
> -- PMM
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH 1/4] net: add FTGMAC100 support
  2017-04-12 17:29     ` Cédric Le Goater
@ 2017-04-12 17:37       ` Peter Maydell
  0 siblings, 0 replies; 8+ messages in thread
From: Peter Maydell @ 2017-04-12 17:37 UTC (permalink / raw)
  To: Cédric Le Goater
  Cc: Jason Wang, Dmitry Fleytman, Samuel Thibault, Jan Kiszka,
	QEMU Developers, qemu-arm

On 12 April 2017 at 18:29, Cédric Le Goater <clg@kaod.org> wrote:
> On 04/10/2017 03:43 PM, Peter Maydell wrote:
>> On 1 April 2017 at 13:57, Cédric Le Goater <clg@kaod.org> wrote:
>>> +    case FTGMAC100_MAC_LADR:
>>> +        return (s->conf.macaddr.a[2] << 24) | (s->conf.macaddr.a[3] << 16) |
>>> +               (s->conf.macaddr.a[4] << 8)  |  s->conf.macaddr.a[5];
>>
>> This will sign extend the high bit of macaddr.a[2] into bits 63..32 of
>> the return value, which probably isn't what you intended and will
>> make Coverity unhapppy.
>
> indeed. What would you recommand ? simply :
>
>         ((uint64_t) s->conf.macaddr.a[2] << 24) | ...

Cast to uint32_t is sufficient. We just need to avoid ending
up with a signed 32 bit value...

thanks
-- PMM

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-04-12 17:37 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-01 12:57 [Qemu-devel] [PATCH 0/4] FTGMAC100 nic model for the Aspeed SoCs Cédric Le Goater
2017-04-01 12:57 ` [Qemu-devel] [PATCH 1/4] net: add FTGMAC100 support Cédric Le Goater
2017-04-10 13:43   ` Peter Maydell
2017-04-12 17:29     ` Cédric Le Goater
2017-04-12 17:37       ` Peter Maydell
2017-04-01 12:57 ` [Qemu-devel] [PATCH 2/4] net/ftgmac100: add a 'aspeed' property Cédric Le Goater
2017-04-01 12:57 ` [Qemu-devel] [PATCH 3/4] aspeed: add a FTGMAC100 nic Cédric Le Goater
2017-04-01 12:57 ` [Qemu-devel] [PATCH 4/4] slirp: add a fake NC-SI backend Cédric Le Goater

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.