[Qemu-devel] [PATCH v2 0/6] e1000: Various fixes and registers' implementation

[Qemu-devel] [PATCH v2 0/6] e1000: Various fixes and registers' implementation

[Leonid Bloch]

This series fixes issues with packet/octet counting in e1000's Statistic
registers, fixes a bug in the packet address filtering procedure, and
implements many MAC registers that were absent before, some Statistic
counters among them. Additionally, several cosmetic changes are made.

Differences from v1:
--------------------
* Wording of several commit messages corrected.
* For trivially implemented Diagnostic registers, a debug message is
  added on read/write attempts, alerting of incomplete implementation.
* Following testing on a physical device, only the lower 16 bits can now
  be read from AIT, and only the lower 4 - from FFMT*.
* The grow_8reg_if_not_full function is rewritten.
* inc_tx_bcast_or_mcast_count and increase_size_stats are now called
  from within e1000_send_packet, to avoid code duplication.

The majority of these changes result from Jason Wang's review - thank
you, Jason!

Leonid Bloch (6):
  e1000: Cosmetic and alignment fixes
  e1000: Trivial implementation of various MAC registers
  e1000: Fixing the received/transmitted packets' counters
  e1000: Fixing the received/transmitted octets' counters
  e1000: Fixing the packet address filtering procedure
  e1000: Implementing various counters

 hw/net/e1000.c      | 352 +++++++++++++++++++++++++++++++++++++++-------------
 hw/net/e1000_regs.h |   8 +-
 2 files changed, 275 insertions(+), 85 deletions(-)

-- 
2.4.3

[Qemu-devel] [PATCH v2 1/6] e1000: Cosmetic and alignment fixes

[Leonid Bloch]

This fixes some alignment and cosmetic issues. The changes are made
in order that the following patches in this series will look like
integral parts of the code surrounding them, while conforming to the
coding style. Although some changes in unrelated areas are also made.

Signed-off-by: Leonid Bloch <leonid.bloch@ravellosystems.com>
Signed-off-by: Dmitry Fleytman <dmitry.fleytman@ravellosystems.com>
---
 hw/net/e1000.c      | 135 +++++++++++++++++++++++++++-------------------------
 hw/net/e1000_regs.h |   2 +-
 2 files changed, 71 insertions(+), 66 deletions(-)

diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 09c9e9d..232edf1 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -41,20 +41,20 @@
 
 #ifdef E1000_DEBUG
 enum {
-    DEBUG_GENERAL,	DEBUG_IO,	DEBUG_MMIO,	DEBUG_INTERRUPT,
-    DEBUG_RX,		DEBUG_TX,	DEBUG_MDIC,	DEBUG_EEPROM,
-    DEBUG_UNKNOWN,	DEBUG_TXSUM,	DEBUG_TXERR,	DEBUG_RXERR,
+    DEBUG_GENERAL,      DEBUG_IO,       DEBUG_MMIO,     DEBUG_INTERRUPT,
+    DEBUG_RX,           DEBUG_TX,       DEBUG_MDIC,     DEBUG_EEPROM,
+    DEBUG_UNKNOWN,      DEBUG_TXSUM,    DEBUG_TXERR,    DEBUG_RXERR,
     DEBUG_RXFILTER,     DEBUG_PHY,      DEBUG_NOTYET,
 };
-#define DBGBIT(x)	(1<<DEBUG_##x)
+#define DBGBIT(x)    (1<<DEBUG_##x)
 static int debugflags = DBGBIT(TXERR) | DBGBIT(GENERAL);
 
-#define	DBGOUT(what, fmt, ...) do { \
+#define DBGOUT(what, fmt, ...) do { \
     if (debugflags & DBGBIT(what)) \
         fprintf(stderr, "e1000: " fmt, ## __VA_ARGS__); \
     } while (0)
 #else
-#define	DBGOUT(what, fmt, ...) do {} while (0)
+#define DBGOUT(what, fmt, ...) do {} while (0)
 #endif
 
 #define IOPORT_SIZE       0x40
@@ -118,7 +118,7 @@ typedef struct E1000State_st {
     } tx;
 
     struct {
-        uint32_t val_in;	// shifted in from guest driver
+        uint32_t val_in;    /* shifted in from guest driver */
         uint16_t bitnum_in;
         uint16_t bitnum_out;
         uint16_t reading;
@@ -155,19 +155,19 @@ typedef struct E1000BaseClass {
 #define E1000_DEVICE_GET_CLASS(obj) \
     OBJECT_GET_CLASS(E1000BaseClass, (obj), TYPE_E1000_BASE)
 
-#define	defreg(x)	x = (E1000_##x>>2)
+#define defreg(x)    x = (E1000_##x>>2)
 enum {
-    defreg(CTRL),	defreg(EECD),	defreg(EERD),	defreg(GPRC),
-    defreg(GPTC),	defreg(ICR),	defreg(ICS),	defreg(IMC),
-    defreg(IMS),	defreg(LEDCTL),	defreg(MANC),	defreg(MDIC),
-    defreg(MPC),	defreg(PBA),	defreg(RCTL),	defreg(RDBAH),
-    defreg(RDBAL),	defreg(RDH),	defreg(RDLEN),	defreg(RDT),
-    defreg(STATUS),	defreg(SWSM),	defreg(TCTL),	defreg(TDBAH),
-    defreg(TDBAL),	defreg(TDH),	defreg(TDLEN),	defreg(TDT),
-    defreg(TORH),	defreg(TORL),	defreg(TOTH),	defreg(TOTL),
-    defreg(TPR),	defreg(TPT),	defreg(TXDCTL),	defreg(WUFC),
-    defreg(RA),		defreg(MTA),	defreg(CRCERRS),defreg(VFTA),
-    defreg(VET),        defreg(RDTR),   defreg(RADV),   defreg(TADV),
+    defreg(CTRL),    defreg(EECD),    defreg(EERD),    defreg(GPRC),
+    defreg(GPTC),    defreg(ICR),     defreg(ICS),     defreg(IMC),
+    defreg(IMS),     defreg(LEDCTL),  defreg(MANC),    defreg(MDIC),
+    defreg(MPC),     defreg(PBA),     defreg(RCTL),    defreg(RDBAH),
+    defreg(RDBAL),   defreg(RDH),     defreg(RDLEN),   defreg(RDT),
+    defreg(STATUS),  defreg(SWSM),    defreg(TCTL),    defreg(TDBAH),
+    defreg(TDBAL),   defreg(TDH),     defreg(TDLEN),   defreg(TDT),
+    defreg(TORH),    defreg(TORL),    defreg(TOTH),    defreg(TOTL),
+    defreg(TPR),     defreg(TPT),     defreg(TXDCTL),  defreg(WUFC),
+    defreg(RA),      defreg(MTA),     defreg(CRCERRS), defreg(VFTA),
+    defreg(VET),     defreg(RDTR),    defreg(RADV),    defreg(TADV),
     defreg(ITR),
 };
 
@@ -226,12 +226,12 @@ enum { NPHYWRITEOPS = ARRAY_SIZE(phyreg_writeops) };
 
 enum { PHY_R = 1, PHY_W = 2, PHY_RW = PHY_R | PHY_W };
 static const char phy_regcap[0x20] = {
-    [PHY_STATUS] = PHY_R,	[M88E1000_EXT_PHY_SPEC_CTRL] = PHY_RW,
-    [PHY_ID1] = PHY_R,		[M88E1000_PHY_SPEC_CTRL] = PHY_RW,
-    [PHY_CTRL] = PHY_RW,	[PHY_1000T_CTRL] = PHY_RW,
-    [PHY_LP_ABILITY] = PHY_R,	[PHY_1000T_STATUS] = PHY_R,
-    [PHY_AUTONEG_ADV] = PHY_RW,	[M88E1000_RX_ERR_CNTR] = PHY_R,
-    [PHY_ID2] = PHY_R,		[M88E1000_PHY_SPEC_STATUS] = PHY_R,
+    [PHY_STATUS] = PHY_R,       [M88E1000_EXT_PHY_SPEC_CTRL] = PHY_RW,
+    [PHY_ID1] = PHY_R,          [M88E1000_PHY_SPEC_CTRL] = PHY_RW,
+    [PHY_CTRL] = PHY_RW,        [PHY_1000T_CTRL] = PHY_RW,
+    [PHY_LP_ABILITY] = PHY_R,   [PHY_1000T_STATUS] = PHY_R,
+    [PHY_AUTONEG_ADV] = PHY_RW, [M88E1000_RX_ERR_CNTR] = PHY_R,
+    [PHY_ID2] = PHY_R,          [M88E1000_PHY_SPEC_STATUS] = PHY_R,
     [PHY_AUTONEG_EXP] = PHY_R,
 };
 
@@ -510,17 +510,17 @@ set_eecd(E1000State *s, int index, uint32_t val)
 
     s->eecd_state.old_eecd = val & (E1000_EECD_SK | E1000_EECD_CS |
             E1000_EECD_DI|E1000_EECD_FWE_MASK|E1000_EECD_REQ);
-    if (!(E1000_EECD_CS & val))			// CS inactive; nothing to do
-	return;
-    if (E1000_EECD_CS & (val ^ oldval)) {	// CS rise edge; reset state
-	s->eecd_state.val_in = 0;
-	s->eecd_state.bitnum_in = 0;
-	s->eecd_state.bitnum_out = 0;
-	s->eecd_state.reading = 0;
+    if (!(E1000_EECD_CS & val))              /* CS inactive; nothing to do */
+        return;
+    if (E1000_EECD_CS & (val ^ oldval)) {    /* CS rise edge; reset state */
+        s->eecd_state.val_in = 0;
+        s->eecd_state.bitnum_in = 0;
+        s->eecd_state.bitnum_out = 0;
+        s->eecd_state.reading = 0;
     }
-    if (!(E1000_EECD_SK & (val ^ oldval)))	// no clock edge
+    if (!(E1000_EECD_SK & (val ^ oldval)))   /* no clock edge */
         return;
-    if (!(E1000_EECD_SK & val)) {		// falling edge
+    if (!(E1000_EECD_SK & val)) {            /* falling edge */
         s->eecd_state.bitnum_out++;
         return;
     }
@@ -621,11 +621,11 @@ xmit_seg(E1000State *s)
         css = tp->ipcss;
         DBGOUT(TXSUM, "frames %d size %d ipcss %d\n",
                frames, tp->size, css);
-        if (tp->ip) {		// IPv4
+        if (tp->ip) {    /* IPv4 */
             stw_be_p(tp->data+css+2, tp->size - css);
             stw_be_p(tp->data+css+4,
                           be16_to_cpup((uint16_t *)(tp->data+css+4))+frames);
-        } else			// IPv6
+        } else           /* IPv6 */
             stw_be_p(tp->data+css+4, tp->size - css);
         css = tp->tucss;
         len = tp->size - css;
@@ -634,8 +634,8 @@ xmit_seg(E1000State *s)
             sofar = frames * tp->mss;
             stl_be_p(tp->data+css+4, ldl_be_p(tp->data+css+4)+sofar); /* seq */
             if (tp->paylen - sofar > tp->mss)
-                tp->data[css + 13] &= ~9;		// PSH, FIN
-        } else	// UDP
+                tp->data[css + 13] &= ~9;    /* PSH, FIN */
+        } else    /* UDP */
             stw_be_p(tp->data+css+4, len);
         if (tp->sum_needed & E1000_TXD_POPTS_TXSM) {
             unsigned int phsum;
@@ -657,8 +657,10 @@ xmit_seg(E1000State *s)
         memmove(tp->data, tp->data + 4, 8);
         memcpy(tp->data + 8, tp->vlan_header, 4);
         e1000_send_packet(s, tp->vlan, tp->size + 4);
-    } else
+    } else {
         e1000_send_packet(s, tp->data, tp->size);
+    }
+
     s->mac_reg[TPT]++;
     s->mac_reg[GPTC]++;
     n = s->mac_reg[TOTL];
@@ -679,7 +681,7 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp)
     struct e1000_tx *tp = &s->tx;
 
     s->mit_ide |= (txd_lower & E1000_TXD_CMD_IDE);
-    if (dtype == E1000_TXD_CMD_DEXT) {	// context descriptor
+    if (dtype == E1000_TXD_CMD_DEXT) {    /* context descriptor */
         op = le32_to_cpu(xp->cmd_and_length);
         tp->ipcss = xp->lower_setup.ip_fields.ipcss;
         tp->ipcso = xp->lower_setup.ip_fields.ipcso;
@@ -694,7 +696,7 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp)
         tp->tcp = (op & E1000_TXD_CMD_TCP) ? 1 : 0;
         tp->tse = (op & E1000_TXD_CMD_TSE) ? 1 : 0;
         tp->tso_frames = 0;
-        if (tp->tucso == 0) {	// this is probably wrong
+        if (tp->tucso == 0) {    /* this is probably wrong */
             DBGOUT(TXSUM, "TCP/UDP: cso 0!\n");
             tp->tucso = tp->tucss + (tp->tcp ? 16 : 6);
         }
@@ -718,7 +720,7 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp)
         stw_be_p(tp->vlan_header + 2,
                       le16_to_cpu(dp->upper.fields.special));
     }
-        
+
     addr = le64_to_cpu(dp->buffer_addr);
     if (tp->tse && tp->cptse) {
         msh = tp->hdr_len + tp->mss;
@@ -1206,20 +1208,22 @@ set_ims(E1000State *s, int index, uint32_t val)
     set_ics(s, 0, 0);
 }
 
-#define getreg(x)	[x] = mac_readreg
+#define getreg(x)    [x] = mac_readreg
 static uint32_t (*macreg_readops[])(E1000State *, int) = {
-    getreg(PBA),	getreg(RCTL),	getreg(TDH),	getreg(TXDCTL),
-    getreg(WUFC),	getreg(TDT),	getreg(CTRL),	getreg(LEDCTL),
-    getreg(MANC),	getreg(MDIC),	getreg(SWSM),	getreg(STATUS),
-    getreg(TORL),	getreg(TOTL),	getreg(IMS),	getreg(TCTL),
-    getreg(RDH),	getreg(RDT),	getreg(VET),	getreg(ICS),
-    getreg(TDBAL),	getreg(TDBAH),	getreg(RDBAH),	getreg(RDBAL),
-    getreg(TDLEN),      getreg(RDLEN),  getreg(RDTR),   getreg(RADV),
-    getreg(TADV),       getreg(ITR),
-
-    [TOTH] = mac_read_clr8,	[TORH] = mac_read_clr8,	[GPRC] = mac_read_clr4,
-    [GPTC] = mac_read_clr4,	[TPR] = mac_read_clr4,	[TPT] = mac_read_clr4,
-    [ICR] = mac_icr_read,	[EECD] = get_eecd,	[EERD] = flash_eerd_read,
+    getreg(PBA),      getreg(RCTL),     getreg(TDH),      getreg(TXDCTL),
+    getreg(WUFC),     getreg(TDT),      getreg(CTRL),     getreg(LEDCTL),
+    getreg(MANC),     getreg(MDIC),     getreg(SWSM),     getreg(STATUS),
+    getreg(TORL),     getreg(TOTL),     getreg(IMS),      getreg(TCTL),
+    getreg(RDH),      getreg(RDT),      getreg(VET),      getreg(ICS),
+    getreg(TDBAL),    getreg(TDBAH),    getreg(RDBAH),    getreg(RDBAL),
+    getreg(TDLEN),    getreg(RDLEN),    getreg(RDTR),     getreg(RADV),
+    getreg(TADV),     getreg(ITR),
+
+    [TOTH] = mac_read_clr8,       [TORH] = mac_read_clr8,
+    [GPRC] = mac_read_clr4,       [GPTC] = mac_read_clr4,
+    [TPT] = mac_read_clr4,        [TPR] = mac_read_clr4,
+    [ICR] = mac_icr_read,         [EECD] = get_eecd,
+    [EERD] = flash_eerd_read,
     [CRCERRS ... MPC] = &mac_readreg,
     [RA ... RA+31] = &mac_readreg,
     [MTA ... MTA+127] = &mac_readreg,
@@ -1227,17 +1231,18 @@ static uint32_t (*macreg_readops[])(E1000State *, int) = {
 };
 enum { NREADOPS = ARRAY_SIZE(macreg_readops) };
 
-#define putreg(x)	[x] = mac_writereg
+#define putreg(x)    [x] = mac_writereg
 static void (*macreg_writeops[])(E1000State *, int, uint32_t) = {
-    putreg(PBA),	putreg(EERD),	putreg(SWSM),	putreg(WUFC),
-    putreg(TDBAL),	putreg(TDBAH),	putreg(TXDCTL),	putreg(RDBAH),
-    putreg(RDBAL),	putreg(LEDCTL), putreg(VET),
-    [TDLEN] = set_dlen,	[RDLEN] = set_dlen,	[TCTL] = set_tctl,
-    [TDT] = set_tctl,	[MDIC] = set_mdic,	[ICS] = set_ics,
-    [TDH] = set_16bit,	[RDH] = set_16bit,	[RDT] = set_rdt,
-    [IMC] = set_imc,	[IMS] = set_ims,	[ICR] = set_icr,
-    [EECD] = set_eecd,	[RCTL] = set_rx_control, [CTRL] = set_ctrl,
-    [RDTR] = set_16bit, [RADV] = set_16bit,     [TADV] = set_16bit,
+    putreg(PBA),      putreg(EERD),     putreg(SWSM),     putreg(WUFC),
+    putreg(TDBAL),    putreg(TDBAH),    putreg(TXDCTL),   putreg(RDBAH),
+    putreg(RDBAL),    putreg(LEDCTL),   putreg(VET),
+
+    [TDLEN] = set_dlen, [RDLEN] = set_dlen,      [TCTL] = set_tctl,
+    [TDT] = set_tctl,   [MDIC] = set_mdic,       [ICS] = set_ics,
+    [TDH] = set_16bit,  [RDH] = set_16bit,       [RDT] = set_rdt,
+    [IMC] = set_imc,    [IMS] = set_ims,         [ICR] = set_icr,
+    [EECD] = set_eecd,  [RCTL] = set_rx_control, [CTRL] = set_ctrl,
+    [RDTR] = set_16bit, [RADV] = set_16bit,      [TADV] = set_16bit,
     [ITR] = set_16bit,
     [RA ... RA+31] = &mac_writereg,
     [MTA ... MTA+127] = &mac_writereg,
diff --git a/hw/net/e1000_regs.h b/hw/net/e1000_regs.h
index 60b96aa..afd81cc 100644
--- a/hw/net/e1000_regs.h
+++ b/hw/net/e1000_regs.h
@@ -158,7 +158,7 @@
 #define E1000_PHY_CTRL     0x00F10  /* PHY Control Register in CSR */
 #define FEXTNVM_SW_CONFIG  0x0001
 #define E1000_PBA      0x01000  /* Packet Buffer Allocation - RW */
-#define E1000_PBS      0x01008  /* Packet Buffer Size */
+#define E1000_PBS      0x01008  /* Packet Buffer Size - RW */
 #define E1000_EEMNGCTL 0x01010  /* MNG EEprom Control */
 #define E1000_FLASH_UPDATES 1000
 #define E1000_EEARBC   0x01024  /* EEPROM Auto Read Bus Control */
-- 
2.4.3

[Qemu-devel] [PATCH v2 3/6] e1000: Fixing the received/transmitted packets' counters

[Leonid Bloch]

According to Intel's specs, these counters (as all the other Statistic
registers) stick at 0xffffffff when the maximum value is reached.
Previously, they would reset after the max. value.

Signed-off-by: Leonid Bloch <leonid.bloch@ravellosystems.com>
Signed-off-by: Dmitry Fleytman <dmitry.fleytman@ravellosystems.com>
---
 hw/net/e1000.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index bf43ab7..74d5e94 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -575,6 +575,14 @@ putsum(uint8_t *data, uint32_t n, uint32_t sloc, uint32_t css, uint32_t cse)
     }
 }
 
+static inline void
+inc_reg_if_not_full(E1000State *s, int index)
+{
+    if (s->mac_reg[index] != 0xffffffff) {
+        s->mac_reg[index]++;
+    }
+}
+
 static inline int
 vlan_enabled(E1000State *s)
 {
@@ -671,8 +679,8 @@ xmit_seg(E1000State *s)
         e1000_send_packet(s, tp->data, tp->size);
     }
 
-    s->mac_reg[TPT]++;
-    s->mac_reg[GPTC]++;
+    inc_reg_if_not_full(s, TPT);
+    s->mac_reg[GPTC] = s->mac_reg[TPT];
     n = s->mac_reg[TOTL];
     if ((s->mac_reg[TOTL] += s->tx.size) < n)
         s->mac_reg[TOTH]++;
@@ -1085,8 +1093,8 @@ e1000_receive_iov(NetClientState *nc, const struct iovec *iov, int iovcnt)
         }
     } while (desc_offset < total_size);
 
-    s->mac_reg[GPRC]++;
-    s->mac_reg[TPR]++;
+    inc_reg_if_not_full(s, TPR);
+    s->mac_reg[GPRC] = s->mac_reg[TPR];
     /* TOR - Total Octets Received:
      * This register includes bytes received in a packet from the <Destination
      * Address> field through the <CRC> field, inclusively.
-- 
2.4.3

[Qemu-devel] [PATCH v2 4/6] e1000: Fixing the received/transmitted octets' counters

[Leonid Bloch]

Previously, these 64-bit registers did not stick at their maximal
values when (and if) they reached them, as they should do, according to
the specs.

This patch introduces a function that takes care of such registers,
avoiding code duplication, making the relevant parts more compatible
with the QEMU coding style, while ensuring that in the unlikely case
of reaching the maximal value, the counter will stick there, as it
supposed to.

Signed-off-by: Leonid Bloch <leonid.bloch@ravellosystems.com>
Signed-off-by: Dmitry Fleytman <dmitry.fleytman@ravellosystems.com>
---
 hw/net/e1000.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 74d5e94..660bd4d 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -583,6 +583,20 @@ inc_reg_if_not_full(E1000State *s, int index)
     }
 }
 
+static void
+grow_8reg_if_not_full(E1000State *s, int index, int size)
+{
+    uint64_t sum = s->mac_reg[index] | (uint64_t)s->mac_reg[index+1] << 32;
+
+    if (sum + size < sum) {
+        sum = ~0ULL;
+    } else {
+        sum += size;
+    }
+    s->mac_reg[index] = sum;
+    s->mac_reg[index+1] = sum >> 32;
+}
+
 static inline int
 vlan_enabled(E1000State *s)
 {
@@ -632,7 +646,7 @@ static void
 xmit_seg(E1000State *s)
 {
     uint16_t len, *sp;
-    unsigned int frames = s->tx.tso_frames, css, sofar, n;
+    unsigned int frames = s->tx.tso_frames, css, sofar;
     struct e1000_tx *tp = &s->tx;
 
     if (tp->tse && tp->cptse) {
@@ -680,10 +694,8 @@ xmit_seg(E1000State *s)
     }
 
     inc_reg_if_not_full(s, TPT);
+    grow_8reg_if_not_full(s, TOTL, s->tx.size);
     s->mac_reg[GPTC] = s->mac_reg[TPT];
-    n = s->mac_reg[TOTL];
-    if ((s->mac_reg[TOTL] += s->tx.size) < n)
-        s->mac_reg[TOTH]++;
 }
 
 static void
@@ -1098,11 +1110,9 @@ e1000_receive_iov(NetClientState *nc, const struct iovec *iov, int iovcnt)
     /* TOR - Total Octets Received:
      * This register includes bytes received in a packet from the <Destination
      * Address> field through the <CRC> field, inclusively.
+     * Always include FCS length (4) in size.
      */
-    n = s->mac_reg[TORL] + size + /* Always include FCS length. */ 4;
-    if (n < s->mac_reg[TORL])
-        s->mac_reg[TORH]++;
-    s->mac_reg[TORL] = n;
+    grow_8reg_if_not_full(s, TORL, size+4);
 
     n = E1000_ICS_RXT0;
     if ((rdt = s->mac_reg[RDT]) < s->mac_reg[RDH])
-- 
2.4.3

[Qemu-devel] [PATCH v2 5/6] e1000: Fixing the packet address filtering procedure

[Leonid Bloch]

Previously, if promiscuous unicast was enabled, a packet was received
straight away, even if it was a multicast or a broadcast packet. This
patch fixes that behavior, while making the filtering procedure a bit
more human-readable.

Signed-off-by: Leonid Bloch <leonid.bloch@ravellosystems.com>
Signed-off-by: Dmitry Fleytman <dmitry.fleytman@ravellosystems.com>
---
 hw/net/e1000.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 660bd4d..a649ab8 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -866,6 +866,7 @@ receive_filter(E1000State *s, const uint8_t *buf, int size)
     static const uint8_t bcast[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
     static const int mta_shift[] = {4, 3, 2, 0};
     uint32_t f, rctl = s->mac_reg[RCTL], ra[2], *rp;
+    int isbcast = !memcmp(buf, bcast, sizeof bcast), ismcast = (buf[0] & 1);
 
     if (is_vlan_packet(s, buf) && vlan_rx_filter_enabled(s)) {
         uint16_t vid = be16_to_cpup((uint16_t *)(buf + 14));
@@ -875,14 +876,17 @@ receive_filter(E1000State *s, const uint8_t *buf, int size)
             return 0;
     }
 
-    if (rctl & E1000_RCTL_UPE)			// promiscuous
+    if (!isbcast && !ismcast && (rctl & E1000_RCTL_UPE)) { /* promiscuous ucast */
         return 1;
+    }
 
-    if ((buf[0] & 1) && (rctl & E1000_RCTL_MPE))	// promiscuous mcast
+    if (ismcast && (rctl & E1000_RCTL_MPE)) {          /* promiscuous mcast */
         return 1;
+    }
 
-    if ((rctl & E1000_RCTL_BAM) && !memcmp(buf, bcast, sizeof bcast))
+    if (isbcast && (rctl & E1000_RCTL_BAM)) {          /* broadcast enabled */
         return 1;
+    }
 
     for (rp = s->mac_reg + RA; rp < s->mac_reg + RA + 32; rp += 2) {
         if (!(rp[1] & E1000_RAH_AV))
-- 
2.4.3

[Qemu-devel] [PATCH v2 6/6] e1000: Implementing various counters

[Leonid Bloch]

This implements the following Statistic registers (various counters)
according to Intel's specs:

TSCTC  GOTCL  GOTCH  GORCL  GORCH  MPRC   BPRC   RUC    ROC
BPTC   MPTC   PTC... PRC...

Signed-off-by: Leonid Bloch <leonid.bloch@ravellosystems.com>
Signed-off-by: Dmitry Fleytman <dmitry.fleytman@ravellosystems.com>
---
 hw/net/e1000.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 73 insertions(+), 5 deletions(-)

diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index a649ab8..ea1f0d5 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -37,6 +37,8 @@
 
 #include "e1000_regs.h"
 
+static const uint8_t bcast[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
+
 #define E1000_DEBUG
 
 #ifdef E1000_DEBUG
@@ -178,7 +180,13 @@ enum {
     defreg(DC),      defreg(TNCRS),   defreg(SEC),     defreg(CEXTERR),
     defreg(RLEC),    defreg(XONRXC),  defreg(XONTXC),  defreg(XOFFRXC),
     defreg(XOFFTXC), defreg(RFC),     defreg(RJC),     defreg(RNBC),
-    defreg(TSCTFC),  defreg(MGTPRC),  defreg(MGTPDC),  defreg(MGTPTC)
+    defreg(TSCTFC),  defreg(MGTPRC),  defreg(MGTPDC),  defreg(MGTPTC),
+    defreg(RUC),     defreg(ROC),     defreg(GORCL),   defreg(GORCH),
+    defreg(GOTCL),   defreg(GOTCH),   defreg(BPRC),    defreg(MPRC),
+    defreg(TSCTC),   defreg(PRC64),   defreg(PRC127),  defreg(PRC255),
+    defreg(PRC511),  defreg(PRC1023), defreg(PRC1522), defreg(PTC64),
+    defreg(PTC127),  defreg(PTC255),  defreg(PTC511),  defreg(PTC1023),
+    defreg(PTC1522), defreg(MPTC),    defreg(BPTC)
 };
 
 static void
@@ -583,6 +591,16 @@ inc_reg_if_not_full(E1000State *s, int index)
     }
 }
 
+static inline void
+inc_tx_bcast_or_mcast_count(E1000State *s, const unsigned char *arr)
+{
+    if (!memcmp(arr, bcast, sizeof bcast)) {
+        inc_reg_if_not_full(s, BPTC);
+    } else if (arr[0] & 1) {
+        inc_reg_if_not_full(s, MPTC);
+    }
+}
+
 static void
 grow_8reg_if_not_full(E1000State *s, int index, int size)
 {
@@ -597,6 +615,24 @@ grow_8reg_if_not_full(E1000State *s, int index, int size)
     s->mac_reg[index+1] = sum >> 32;
 }
 
+static void
+increase_size_stats(E1000State *s, const int *size_regs, int size)
+{
+    if (size > 1023) {
+        inc_reg_if_not_full(s, size_regs[5]);
+    } else if (size > 511) {
+        inc_reg_if_not_full(s, size_regs[4]);
+    } else if (size > 255) {
+        inc_reg_if_not_full(s, size_regs[3]);
+    } else if (size > 127) {
+        inc_reg_if_not_full(s, size_regs[2]);
+    } else if (size > 64) {
+        inc_reg_if_not_full(s, size_regs[1]);
+    } else if (size == 64) {
+        inc_reg_if_not_full(s, size_regs[0]);
+    }
+}
+
 static inline int
 vlan_enabled(E1000State *s)
 {
@@ -634,12 +670,17 @@ fcs_len(E1000State *s)
 static void
 e1000_send_packet(E1000State *s, const uint8_t *buf, int size)
 {
+    static const int PTCregs[6] = { PTC64, PTC127, PTC255, PTC511,
+                                    PTC1023, PTC1522 };
+
     NetClientState *nc = qemu_get_queue(s->nic);
     if (s->phy_reg[PHY_CTRL] & MII_CR_LOOPBACK) {
         nc->info->receive(nc, buf, size);
     } else {
         qemu_send_packet(nc, buf, size);
     }
+    inc_tx_bcast_or_mcast_count(s, buf);
+    increase_size_stats(s, PTCregs, size);
 }
 
 static void
@@ -665,8 +706,11 @@ xmit_seg(E1000State *s)
         if (tp->tcp) {
             sofar = frames * tp->mss;
             stl_be_p(tp->data+css+4, ldl_be_p(tp->data+css+4)+sofar); /* seq */
-            if (tp->paylen - sofar > tp->mss)
+            if (tp->paylen - sofar > tp->mss) {
                 tp->data[css + 13] &= ~9;    /* PSH, FIN */
+            } else if (frames) {
+                inc_reg_if_not_full(s, TSCTC);
+            }
         } else    /* UDP */
             stw_be_p(tp->data+css+4, len);
         if (tp->sum_needed & E1000_TXD_POPTS_TXSM) {
@@ -696,6 +740,8 @@ xmit_seg(E1000State *s)
     inc_reg_if_not_full(s, TPT);
     grow_8reg_if_not_full(s, TOTL, s->tx.size);
     s->mac_reg[GPTC] = s->mac_reg[TPT];
+    s->mac_reg[GOTCL] = s->mac_reg[TOTL];
+    s->mac_reg[GOTCH] = s->mac_reg[TOTH];
 }
 
 static void
@@ -863,7 +909,6 @@ start_xmit(E1000State *s)
 static int
 receive_filter(E1000State *s, const uint8_t *buf, int size)
 {
-    static const uint8_t bcast[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
     static const int mta_shift[] = {4, 3, 2, 0};
     uint32_t f, rctl = s->mac_reg[RCTL], ra[2], *rp;
     int isbcast = !memcmp(buf, bcast, sizeof bcast), ismcast = (buf[0] & 1);
@@ -881,10 +926,12 @@ receive_filter(E1000State *s, const uint8_t *buf, int size)
     }
 
     if (ismcast && (rctl & E1000_RCTL_MPE)) {          /* promiscuous mcast */
+        inc_reg_if_not_full(s, MPRC);
         return 1;
     }
 
     if (isbcast && (rctl & E1000_RCTL_BAM)) {          /* broadcast enabled */
+        inc_reg_if_not_full(s, BPRC);
         return 1;
     }
 
@@ -906,8 +953,10 @@ receive_filter(E1000State *s, const uint8_t *buf, int size)
 
     f = mta_shift[(rctl >> E1000_RCTL_MO_SHIFT) & 3];
     f = (((buf[5] << 8) | buf[4]) >> f) & 0xfff;
-    if (s->mac_reg[MTA + (f >> 5)] & (1 << (f & 0x1f)))
+    if (s->mac_reg[MTA + (f >> 5)] & (1 << (f & 0x1f))) {
+        inc_reg_if_not_full(s, MPRC);
         return 1;
+    }
     DBGOUT(RXFILTER,
            "dropping, inexact filter mismatch: %02x:%02x:%02x:%02x:%02x:%02x MO %d MTA[%d] %x\n",
            buf[0], buf[1], buf[2], buf[3], buf[4], buf[5],
@@ -996,6 +1045,8 @@ e1000_receive_iov(NetClientState *nc, const struct iovec *iov, int iovcnt)
     size_t desc_offset;
     size_t desc_size;
     size_t total_size;
+    static const int PRCregs[6] = { PRC64, PRC127, PRC255, PRC511,
+                                    PRC1023, PRC1522 };
 
     if (!(s->mac_reg[STATUS] & E1000_STATUS_LU)) {
         return -1;
@@ -1009,6 +1060,7 @@ e1000_receive_iov(NetClientState *nc, const struct iovec *iov, int iovcnt)
     if (size < sizeof(min_buf)) {
         iov_to_buf(iov, iovcnt, 0, min_buf, size);
         memset(&min_buf[size], 0, sizeof(min_buf) - size);
+        inc_reg_if_not_full(s, RUC);
         min_iov.iov_base = filter_buf = min_buf;
         min_iov.iov_len = size = sizeof(min_buf);
         iovcnt = 1;
@@ -1024,6 +1076,7 @@ e1000_receive_iov(NetClientState *nc, const struct iovec *iov, int iovcnt)
         (size > MAXIMUM_ETHERNET_VLAN_SIZE
         && !(s->mac_reg[RCTL] & E1000_RCTL_LPE)))
         && !(s->mac_reg[RCTL] & E1000_RCTL_SBP)) {
+        inc_reg_if_not_full(s, ROC);
         return size;
     }
 
@@ -1109,6 +1162,7 @@ e1000_receive_iov(NetClientState *nc, const struct iovec *iov, int iovcnt)
         }
     } while (desc_offset < total_size);
 
+    increase_size_stats(s, PRCregs, total_size);
     inc_reg_if_not_full(s, TPR);
     s->mac_reg[GPRC] = s->mac_reg[TPR];
     /* TOR - Total Octets Received:
@@ -1117,6 +1171,8 @@ e1000_receive_iov(NetClientState *nc, const struct iovec *iov, int iovcnt)
      * Always include FCS length (4) in size.
      */
     grow_8reg_if_not_full(s, TORL, size+4);
+    s->mac_reg[GORCL] = s->mac_reg[TORL];
+    s->mac_reg[GORCH] = s->mac_reg[TORH];
 
     n = E1000_ICS_RXT0;
     if ((rdt = s->mac_reg[RDT]) < s->mac_reg[RDH])
@@ -1305,11 +1361,23 @@ static uint32_t (*macreg_readops[])(E1000State *, int) = {
     getreg(TNCRS),    getreg(SEC),      getreg(CEXTERR),  getreg(RLEC),
     getreg(XONRXC),   getreg(XONTXC),   getreg(XOFFRXC),  getreg(XOFFTXC),
     getreg(RFC),      getreg(RJC),      getreg(RNBC),     getreg(TSCTFC),
-    getreg(MGTPRC),   getreg(MGTPDC),   getreg(MGTPTC),
+    getreg(MGTPRC),   getreg(MGTPDC),   getreg(MGTPTC),   getreg(GORCL),
+    getreg(GOTCL),
 
     [TOTH] = mac_read_clr8,       [TORH] = mac_read_clr8,
+    [GOTCH] = mac_read_clr8,      [GORCH] = mac_read_clr8,
+    [PRC64] = mac_read_clr4,      [PRC127] = mac_read_clr4,
+    [PRC255] = mac_read_clr4,     [PRC511] = mac_read_clr4,
+    [PRC1023] = mac_read_clr4,    [PRC1522] = mac_read_clr4,
+    [PTC64] = mac_read_clr4,      [PTC127] = mac_read_clr4,
+    [PTC255] = mac_read_clr4,     [PTC511] = mac_read_clr4,
+    [PTC1023] = mac_read_clr4,    [PTC1522] = mac_read_clr4,
     [GPRC] = mac_read_clr4,       [GPTC] = mac_read_clr4,
     [TPT] = mac_read_clr4,        [TPR] = mac_read_clr4,
+    [RUC] = mac_read_clr4,        [ROC] = mac_read_clr4,
+    [BPRC] = mac_read_clr4,       [MPRC] = mac_read_clr4,
+    [TSCTC] = mac_read_clr4,      [BPTC] = mac_read_clr4,
+    [MPTC] = mac_read_clr4,
     [ICR] = mac_icr_read,         [EECD] = get_eecd,
     [EERD] = flash_eerd_read,
     [RDFH] = mac_low13_read_prt,  [RDFT] = mac_low13_read_prt,
-- 
2.4.3

Re: [Qemu-devel] [PATCH v2 0/6] e1000: Various fixes and registers' implementation

[Jason Wang]

On 10/27/2015 09:20 PM, Leonid Bloch wrote:
> This series fixes issues with packet/octet counting in e1000's Statistic
> registers, fixes a bug in the packet address filtering procedure, and
> implements many MAC registers that were absent before, some Statistic
> counters among them. Additionally, several cosmetic changes are made.
>
> Differences from v1:
> --------------------
> * Wording of several commit messages corrected.
> * For trivially implemented Diagnostic registers, a debug message is
>   added on read/write attempts, alerting of incomplete implementation.
> * Following testing on a physical device, only the lower 16 bits can now
>   be read from AIT, and only the lower 4 - from FFMT*.
> * The grow_8reg_if_not_full function is rewritten.
> * inc_tx_bcast_or_mcast_count and increase_size_stats are now called
>   from within e1000_send_packet, to avoid code duplication.
>
> The majority of these changes result from Jason Wang's review - thank
> you, Jason!
>
> Leonid Bloch (6):
>   e1000: Cosmetic and alignment fixes
>   e1000: Trivial implementation of various MAC registers
>   e1000: Fixing the received/transmitted packets' counters
>   e1000: Fixing the received/transmitted octets' counters
>   e1000: Fixing the packet address filtering procedure
>   e1000: Implementing various counters
>
>  hw/net/e1000.c      | 352 +++++++++++++++++++++++++++++++++++++++-------------
>  hw/net/e1000_regs.h |   8 +-
>  2 files changed, 275 insertions(+), 85 deletions(-)
>

Applied in https://github.com/jasowang/qemu/commits/net

Thanks

Re: [Qemu-devel] [PATCH v2 0/6] e1000: Various fixes and registers' implementation

[Dmitry Fleytman]

[-- Attachment #1: Type: text/plain, Size: 1993 bytes --]


-----------------------------------
Daynix Computing LTD
Dmitry Fleytman, CTO
Email: dmitry@daynix.com <mailto:yan@daynix.com>
Phone: +972-54-2819481
Web: www.daynix.com <http://www.daynix.com/>


> On 28 Oct 2015, at 07:41 AM, Jason Wang <jasowang@redhat.com> wrote:
> 
> 
> 
> On 10/27/2015 09:20 PM, Leonid Bloch wrote:
>> This series fixes issues with packet/octet counting in e1000's Statistic
>> registers, fixes a bug in the packet address filtering procedure, and
>> implements many MAC registers that were absent before, some Statistic
>> counters among them. Additionally, several cosmetic changes are made.
>> 
>> Differences from v1:
>> --------------------
>> * Wording of several commit messages corrected.
>> * For trivially implemented Diagnostic registers, a debug message is
>>  added on read/write attempts, alerting of incomplete implementation.
>> * Following testing on a physical device, only the lower 16 bits can now
>>  be read from AIT, and only the lower 4 - from FFMT*.
>> * The grow_8reg_if_not_full function is rewritten.
>> * inc_tx_bcast_or_mcast_count and increase_size_stats are now called
>>  from within e1000_send_packet, to avoid code duplication.
>> 
>> The majority of these changes result from Jason Wang's review - thank
>> you, Jason!
>> 
>> Leonid Bloch (6):
>>  e1000: Cosmetic and alignment fixes
>>  e1000: Trivial implementation of various MAC registers
>>  e1000: Fixing the received/transmitted packets' counters
>>  e1000: Fixing the received/transmitted octets' counters
>>  e1000: Fixing the packet address filtering procedure
>>  e1000: Implementing various counters
>> 
>> hw/net/e1000.c      | 352 +++++++++++++++++++++++++++++++++++++++-------------
>> hw/net/e1000_regs.h |   8 +-
>> 2 files changed, 275 insertions(+), 85 deletions(-)
>> 
> 
> Applied in https://github.com/jasowang/qemu/commits/net <https://github.com/jasowang/qemu/commits/net>


Thanks, Jason!

> 
> Thanks


[-- Attachment #2: Type: text/html, Size: 7001 bytes --]

Re: [Qemu-devel] [PATCH v2 0/6] e1000: Various fixes and registers' implementation

[Jason Wang]

On 10/28/2015 01:41 PM, Jason Wang wrote:
>
> On 10/27/2015 09:20 PM, Leonid Bloch wrote:
>> This series fixes issues with packet/octet counting in e1000's Statistic
>> registers, fixes a bug in the packet address filtering procedure, and
>> implements many MAC registers that were absent before, some Statistic
>> counters among them. Additionally, several cosmetic changes are made.
>>
>> Differences from v1:
>> --------------------
>> * Wording of several commit messages corrected.
>> * For trivially implemented Diagnostic registers, a debug message is
>>   added on read/write attempts, alerting of incomplete implementation.
>> * Following testing on a physical device, only the lower 16 bits can now
>>   be read from AIT, and only the lower 4 - from FFMT*.
>> * The grow_8reg_if_not_full function is rewritten.
>> * inc_tx_bcast_or_mcast_count and increase_size_stats are now called
>>   from within e1000_send_packet, to avoid code duplication.
>>
>> The majority of these changes result from Jason Wang's review - thank
>> you, Jason!
>>
>> Leonid Bloch (6):
>>   e1000: Cosmetic and alignment fixes
>>   e1000: Trivial implementation of various MAC registers
>>   e1000: Fixing the received/transmitted packets' counters
>>   e1000: Fixing the received/transmitted octets' counters
>>   e1000: Fixing the packet address filtering procedure
>>   e1000: Implementing various counters
>>
>>  hw/net/e1000.c      | 352 +++++++++++++++++++++++++++++++++++++++-------------
>>  hw/net/e1000_regs.h |   8 +-
>>  2 files changed, 275 insertions(+), 85 deletions(-)
>>
> Applied in https://github.com/jasowang/qemu/commits/net
>
> Thanks
>

Actually one thing is missed in this series - migration. The value of
new implemented registers will be lost after migration. This needs to be
fixed.

So need a v3, sorry.