[Qemu-devel] [PATCH v2] e1000: defer packets until BM enabled

[Qemu-devel] [PATCH v2] e1000: defer packets until BM enabled

[Michael S. Tsirkin]

Some guests seem to set BM for e1000 after
enabling RX.
If packets arrive in the window, device is wedged.
Probably works by luck on real hardware, work around
this by making can_receive depend on BM.

Tested-by: Gabriel Somlo <somlo@cmu.edu>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

Amos - you were the one reporting the failures, could
you pls confirm this patch fixes the issues for you?

 hw/net/e1000.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index e33a4da..89c5788 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -33,6 +33,7 @@
 #include "sysemu/sysemu.h"
 #include "sysemu/dma.h"
 #include "qemu/iov.h"
+#include "qemu/range.h"
 
 #include "e1000_regs.h"
 
@@ -923,7 +924,9 @@ e1000_can_receive(NetClientState *nc)
     E1000State *s = qemu_get_nic_opaque(nc);
 
     return (s->mac_reg[STATUS] & E1000_STATUS_LU) &&
-        (s->mac_reg[RCTL] & E1000_RCTL_EN) && e1000_has_rxbufs(s, 1);
+        (s->mac_reg[RCTL] & E1000_RCTL_EN) &&
+        (s->parent_obj.config[PCI_COMMAND] & PCI_COMMAND_MASTER) &&
+        e1000_has_rxbufs(s, 1);
 }
 
 static uint64_t rx_desc_base(E1000State *s)
@@ -1529,6 +1532,20 @@ static NetClientInfo net_e1000_info = {
     .link_status_changed = e1000_set_link_status,
 };
 
+static void e1000_write_config(PCIDevice *pci_dev, uint32_t address,
+                                uint32_t val, int len)
+{
+    E1000State *s = E1000(pci_dev);
+
+    pci_default_write_config(pci_dev, address, val, len);
+
+    if (range_covers_byte(address, len, PCI_COMMAND) &&
+        (pci_dev->config[PCI_COMMAND] & PCI_COMMAND_MASTER)) {
+        qemu_flush_queued_packets(qemu_get_queue(s->nic));
+    }
+}
+
+
 static int pci_e1000_init(PCIDevice *pci_dev)
 {
     DeviceState *dev = DEVICE(pci_dev);
@@ -1539,6 +1556,8 @@ static int pci_e1000_init(PCIDevice *pci_dev)
     int i;
     uint8_t *macaddr;
 
+    pci_dev->config_write = e1000_write_config;
+
     pci_conf = pci_dev->config;
 
     /* TODO: RST# value should be 0, PCI spec 6.2.4 */
-- 
MST

Re: [Qemu-devel] [PATCH v2] e1000: defer packets until BM enabled

[Jason Wang]

On Tue, Dec 2, 2014 at 2:06 AM, Michael S. Tsirkin <mst@redhat.com> 
wrote:
> Some guests seem to set BM for e1000 after
> enabling RX.
> If packets arrive in the window, device is wedged.
> Probably works by luck on real hardware, work around
> this by making can_receive depend on BM.
> 
> Tested-by: Gabriel Somlo <somlo@cmu.edu>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> 
> Amos - you were the one reporting the failures, could
> you pls confirm this patch fixes the issues for you?
> 
>  hw/net/e1000.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
> index e33a4da..89c5788 100644
> --- a/hw/net/e1000.c
> +++ b/hw/net/e1000.c
> @@ -33,6 +33,7 @@
>  #include "sysemu/sysemu.h"
>  #include "sysemu/dma.h"
>  #include "qemu/iov.h"
> +#include "qemu/range.h"
>  
>  #include "e1000_regs.h"
>  
> @@ -923,7 +924,9 @@ e1000_can_receive(NetClientState *nc)
>      E1000State *s = qemu_get_nic_opaque(nc);
>  
>      return (s->mac_reg[STATUS] & E1000_STATUS_LU) &&
> -        (s->mac_reg[RCTL] & E1000_RCTL_EN) && e1000_has_rxbufs(s, 1);
> +        (s->mac_reg[RCTL] & E1000_RCTL_EN) &&
> +        (s->parent_obj.config[PCI_COMMAND] & PCI_COMMAND_MASTER) &&
> +        e1000_has_rxbufs(s, 1);
>  }
>  
>  static uint64_t rx_desc_base(E1000State *s)
> @@ -1529,6 +1532,20 @@ static NetClientInfo net_e1000_info = {
>      .link_status_changed = e1000_set_link_status,
>  };
>  
> +static void e1000_write_config(PCIDevice *pci_dev, uint32_t address,
> +                                uint32_t val, int len)
> +{
> +    E1000State *s = E1000(pci_dev);
> +
> +    pci_default_write_config(pci_dev, address, val, len);
> +
> +    if (range_covers_byte(address, len, PCI_COMMAND) &&
> +        (pci_dev->config[PCI_COMMAND] & PCI_COMMAND_MASTER)) {
> +        qemu_flush_queued_packets(qemu_get_queue(s->nic));
> +    }
> +}
> +
> +
>  static int pci_e1000_init(PCIDevice *pci_dev)
>  {
>      DeviceState *dev = DEVICE(pci_dev);
> @@ -1539,6 +1556,8 @@ static int pci_e1000_init(PCIDevice *pci_dev)
>      int i;
>      uint8_t *macaddr;
>  
> +    pci_dev->config_write = e1000_write_config;
> +
>      pci_conf = pci_dev->config;
>  
>      /* TODO: RST# value should be 0, PCI spec 6.2.4 */
> -- 
> MST

Reviewed-by: Jason Wang <jasowang@redhat.com>

Re: [Qemu-devel] [PATCH v2] e1000: defer packets until BM enabled

[Stefan Hajnoczi]

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

On Mon, Dec 01, 2014 at 08:06:52PM +0200, Michael S. Tsirkin wrote:
> Some guests seem to set BM for e1000 after
> enabling RX.
> If packets arrive in the window, device is wedged.
> Probably works by luck on real hardware, work around
> this by making can_receive depend on BM.
> 
> Tested-by: Gabriel Somlo <somlo@cmu.edu>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> 
> Amos - you were the one reporting the failures, could
> you pls confirm this patch fixes the issues for you?
> 
>  hw/net/e1000.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)

Thanks, applied to my net tree:
https://github.com/stefanha/qemu/commits/net

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]