On 04/10/2017 03:43 PM, Peter Maydell wrote: > On 1 April 2017 at 13:57, Cédric Le Goater <c...@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 <c...@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 >