Patchwork [V3] e1000: drop check_rxov, always treat RX ring with RDH == RDT as empty

login
register
mail settings
Submitter Dmitry Fleytman
Date Oct. 19, 2012, 5:56 a.m.
Message ID <1350626215-24558-2-git-send-email-dmitry@daynix.com>
Download mbox | patch
Permalink /patch/192563/
State New
Headers show

Comments

Dmitry Fleytman - Oct. 19, 2012, 5:56 a.m.
Real HW always treats RX ring with RDH == RDT as empty.
Emulation is supposed to behave the same.

Reported-by: Chris Webb <chris.webb@elastichosts.com>
Reported-by: Richard Davies <richard.davies@elastichosts.com>
Signed-off-by: Dmitry Fleytman <dmitry@daynix.com>
---
 hw/e1000.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)
Stefan Hajnoczi - Oct. 19, 2012, 7:52 a.m.
On Fri, Oct 19, 2012 at 07:56:55AM +0200, Dmitry Fleytman wrote:
> Real HW always treats RX ring with RDH == RDT as empty.
> Emulation is supposed to behave the same.
> 
> Reported-by: Chris Webb <chris.webb@elastichosts.com>
> Reported-by: Richard Davies <richard.davies@elastichosts.com>
> Signed-off-by: Dmitry Fleytman <dmitry@daynix.com>
> ---
>  hw/e1000.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)

Applied to the net tree:
http://github.com/stefanha/qemu/commits/net

Thanks for your efforts in squashing this bug!  Glad it was possible to
drop check_rxov.

Stefan
Dmitry Fleytman - Oct. 19, 2012, 8:46 a.m.
Thanks Stefan,

It was my very first idea to drop check_rxov and solve the problem,
however for some reason I was sure that it required to emulate real HW behavior.

I'm glad we clarified this.

Regards,
Dmitry Fleytman

On Fri, Oct 19, 2012 at 9:52 AM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Fri, Oct 19, 2012 at 07:56:55AM +0200, Dmitry Fleytman wrote:
>> Real HW always treats RX ring with RDH == RDT as empty.
>> Emulation is supposed to behave the same.
>>
>> Reported-by: Chris Webb <chris.webb@elastichosts.com>
>> Reported-by: Richard Davies <richard.davies@elastichosts.com>
>> Signed-off-by: Dmitry Fleytman <dmitry@daynix.com>
>> ---
>>  hw/e1000.c | 7 ++-----
>>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> Applied to the net tree:
> http://github.com/stefanha/qemu/commits/net
>
> Thanks for your efforts in squashing this bug!  Glad it was possible to
> drop check_rxov.
>
> Stefan

Patch

diff --git a/hw/e1000.c b/hw/e1000.c
index 63fee10..ab39d47 100644
--- a/hw/e1000.c
+++ b/hw/e1000.c
@@ -92,7 +92,6 @@  typedef struct E1000State_st {
 
     uint32_t rxbuf_size;
     uint32_t rxbuf_min_shift;
-    int check_rxov;
     struct e1000_tx {
         unsigned char header[256];
         unsigned char vlan_header[4];
@@ -741,11 +740,11 @@  static bool e1000_has_rxbufs(E1000State *s, size_t total_size)
     int bufs;
     /* Fast-path short packets */
     if (total_size <= s->rxbuf_size) {
-        return s->mac_reg[RDH] != s->mac_reg[RDT] || !s->check_rxov;
+        return s->mac_reg[RDH] != s->mac_reg[RDT];
     }
     if (s->mac_reg[RDH] < s->mac_reg[RDT]) {
         bufs = s->mac_reg[RDT] - s->mac_reg[RDH];
-    } else if (s->mac_reg[RDH] > s->mac_reg[RDT] || !s->check_rxov) {
+    } else if (s->mac_reg[RDH] > s->mac_reg[RDT]) {
         bufs = s->mac_reg[RDLEN] /  sizeof(struct e1000_rx_desc) +
             s->mac_reg[RDT] - s->mac_reg[RDH];
     } else {
@@ -848,7 +847,6 @@  e1000_receive(NetClientState *nc, const uint8_t *buf, size_t size)
 
         if (++s->mac_reg[RDH] * sizeof(desc) >= s->mac_reg[RDLEN])
             s->mac_reg[RDH] = 0;
-        s->check_rxov = 1;
         /* see comment in start_xmit; same here */
         if (s->mac_reg[RDH] == rdh_start) {
             DBGOUT(RXERR, "RDH wraparound @%x, RDT %x, RDLEN %x\n",
@@ -925,7 +923,6 @@  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 (e1000_has_rxbufs(s, 1)) {
         qemu_flush_queued_packets(&s->nic->nc);