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

[Amos Kong]

On Fri, Feb 06, 2015 at 01:46:36PM +0000, Stefan Hajnoczi wrote:
> On Wed, Jan 07, 2015 at 04:08:29PM +0000, Stefan Hajnoczi wrote:
> > On Thu, Dec 18, 2014 at 05:22:19PM +0800, Amos Kong 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);
> > >          qemu_flush_queued_packets(qemu_get_queue(s->nic));
> > >          start_xmit(s);
> > >      }
> > 
> > This is weird.
> > 
> > Are you sure there's not some guest behavior missing like the NIC option
> > ROM leaving bus mastering enabled after the BIOS/EFI has booted, causing
> > Windows debug to work on physical machines?

QEMU emulated e1000 is too old, it's not in the official support
list of window network debug.

> > Before we merge a hack like this we should understand the problem 100%.
> 
> Any new insights into what is going on here?

It's not good to unconditionaly enable BM, we should enable/disable it
according to other event/status change.
 
> Stefan

-- 
                        Amos.

signature.asc
Description: Digital signature