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

[Jason Wang <jasowang@redhat.com>]

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>