[Qemu-devel] [PATCH 1/2] Ignore RX tail kicks when RX disabled.

[Dmitry Fleytman]

Device RX initization from driver's side consists of following steps:
  1. Initialize head and tail of RX ring to 0
  2. Enable Rx (set bit in RCTL register)
  3. Allocate buffers, fill descriptors
  4. Write ring tail

Forth operation signals hardware that RX buffers available
and it may start packets indication.

Current implementation treats first operation (write 0 to ring tail)
as signal of buffers availability and starts data transfers as soon
as RX enable indicaton arrives.

This is not correct because there is a chance that ring is still
empty (third action not performed yet) and then memory corruption
occures.

Device has to ignore RX tail kicks unless RX enabled.

Reported-by: Chris Webb <address@hidden>
Reported-by: Richard Davies <address@hidden>
Signed-off-by: Dmitry Fleytman <address@hidden>
---
 hw/e1000.c | 29 +++++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/hw/e1000.c b/hw/e1000.c
index 63fee10..606bf3a 100644
--- a/hw/e1000.c
+++ b/hw/e1000.c
@@ -267,6 +267,7 @@ static void e1000_reset(void *opaque)
 {
     E1000State *d = opaque;
 
+    d->check_rxov = 1;
     qemu_del_timer(d->autoneg_timer);
     memset(d->phy_reg, 0, sizeof d->phy_reg);
     memmove(d->phy_reg, phy_reg_init, sizeof phy_reg_init);
@@ -285,6 +286,10 @@ set_ctrl(E1000State *s, int index, uint32_t val)
 {
     /* RST is self clearing */
     s->mac_reg[CTRL] = val & ~E1000_CTRL_RST;
+
+    if (val & E1000_CTRL_RST) {
+        s->check_rxov = 1;
+    }
 }
 
 static void
@@ -754,12 +759,18 @@ static bool e1000_has_rxbufs(E1000State *s, size_t 
total_size)
     return total_size <= bufs * s->rxbuf_size;
 }
 
+static inline bool
+is_receive_enabled(E1000State *s)
+{
+    return s->mac_reg[RCTL] & E1000_RCTL_EN;
+}
+
 static int
 e1000_can_receive(NetClientState *nc)
 {
     E1000State *s = DO_UPCAST(NICState, nc, nc)->opaque;
 
-    return (s->mac_reg[RCTL] & E1000_RCTL_EN) && e1000_has_rxbufs(s, 1);
+    return is_receive_enabled(s) && e1000_has_rxbufs(s, 1);
 }
 
 static uint64_t rx_desc_base(E1000State *s)
@@ -785,8 +796,9 @@ e1000_receive(NetClientState *nc, const uint8_t *buf, 
size_t size)
     size_t desc_size;
     size_t total_size;
 
-    if (!(s->mac_reg[RCTL] & E1000_RCTL_EN))
+    if (!is_receive_enabled(s)) {
         return -1;
+    }
 
     /* Pad to minimum Ethernet frame length */
     if (size < sizeof(min_buf)) {
@@ -925,8 +937,12 @@ mac_writereg(E1000State *s, int index, uint32_t val)
 static void
 set_rdt(E1000State *s, int index, uint32_t val)
 {
-    s->check_rxov = 0;
     s->mac_reg[index] = val & 0xffff;
+
+    if (is_receive_enabled(s)) {
+        s->check_rxov = 0;
+    }
+
     if (e1000_has_rxbufs(s, 1)) {
         qemu_flush_queued_packets(&s->nic->nc);
     }
@@ -1065,7 +1081,12 @@ static void e1000_io_write(void *opaque, 
target_phys_addr_t addr,
 {
     E1000State *s = opaque;
 
-    (void)s;
+    switch (addr) {
+    case E1000_CTRL_DUP:
+        if (val & E1000_CTRL_RST) {
+            s->check_rxov = 1;
+        }
+    }
 }
 
 static const MemoryRegionOps e1000_io_ops = {
-- 
1.7.11.4