Re: [Qemu-devel] [PATCH 2/2] e1000: unconditionally enable bus mastering

[Amos Kong]

On Thu, Dec 18, 2014 at 09:57:29AM +0008, Jason Wang wrote:
> 
> 
> On Thu, Dec 18, 2014 at 5:22 PM, Amos Kong <address@hidden> wrote:
> >After enabled network debug of e1000 in Win2012-64r2 guest,
> >Bus mastering of e1000 can't be enabled by e1000 driver. It
> >caused guest can't get IP address.
> >
> >  # bcdedit /debug on
> >  # bcdedit /dbgsettings net hostip:192.168.122.100 port:50000
> >  (We can use non-existed IP here, it's just used to pass the
> >   setup, not really use it)
> >
> >If we disable debug function, e1000 driver can enable bus
> >mastering bit successfully, guest network is fine.
> >
> >This patch changed e1000 backend to enalbe bus mastering
> >unconditionally as a workaround.
> >
> >Signed-off-by: Amos Kong <address@hidden>
> >---
> > hw/net/e1000.c | 11 +++++++++--
> > 1 file changed, 9 insertions(+), 2 deletions(-)
> >
> >diff --git a/hw/net/e1000.c b/hw/net/e1000.c
> >index ec9224b..82829ae 100644
> >--- a/hw/net/e1000.c
> >+++ b/hw/net/e1000.c
> >@@ -1544,8 +1544,15 @@ static void e1000_write_config(PCIDevice *pci_dev,
> >uint32_t address,
> >     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)) {
> >+    if (range_covers_byte(address, len, PCI_COMMAND)) {
> >+        /*
> >+         * Some guest (eg: Win2012-64r2) doesn't enable bus mastering
> >+         * correctly, it caused guest network down. So we unconditionally
> >+         * enable PCI bus mastering and BM memory region for e1000 as
> >+         * a workaround.
> >+         */
> >+        pci_dev->config[PCI_COMMAND] |= PCI_COMMAND_MASTER;
> >+        memory_region_set_enabled(&pci_dev->bus_master_enable_region,
> >true);
> 
> So BM is still set even if guest want to clear it?

BM will be cleared many times in boot stage, enabling it once doesn't
work. Curently we honestly clear the bit, then BM isn't enabled.
But it's might be too aggressive/crazy to enable BM forever.

> >
> >         qemu_flush_queued_packets(qemu_get_queue(s->nic));
> >         start_xmit(s);
> >     }

-- 
                        Amos.

signature.asc
Description: Digital signature