[Qemu-devel] [PATCH 0/2] ignore bus master for e1000

[Qemu-devel] [PATCH 0/2] ignore bus master for e1000

[Amos Kong]

Win2012-64r2 guest doesn't set bus mastering correctly,
it caused guest network down, this patch ignored it for
e1000 nic for workarounding the guest issue.

Patch 1 is an update version of:
http://lists.gnu.org/archive/html/qemu-devel/2014-12/msg00048.html
transmit packets are also defered in latest version.

Amos Kong (1):
  e1000: unconditionally enable bus mastering

Michael S. Tsirkin (1):
  e1000: defer packets until BM enabled

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

-- 
2.1.0

[Qemu-devel] [PATCH 1/2] e1000: defer packets until BM enabled

[Amos Kong]

From: "Michael S. Tsirkin" <mst@redhat.com>

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. This patch
defer transmit packets, only start transmit when BM
is enabled.

Tested-by: Gabriel Somlo <somlo@cmu.edu>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Amos Kong <akong@redhat.com>
---
 hw/net/e1000.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index e33a4da..ec9224b 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"
 
@@ -790,6 +791,11 @@ start_xmit(E1000State *s)
     struct e1000_tx_desc desc;
     uint32_t tdh_start = s->mac_reg[TDH], cause = E1000_ICS_TXQE;
 
+    if (!(d->config[PCI_COMMAND] & PCI_COMMAND_MASTER)) {
+        DBGOUT(TX, "BM disabled\n");
+        return;
+    }
+
     if (!(s->mac_reg[TCTL] & E1000_TCTL_EN)) {
         DBGOUT(TX, "tx disabled\n");
         return;
@@ -923,7 +929,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 +1537,21 @@ 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));
+        start_xmit(s);
+    }
+}
+
+
 static int pci_e1000_init(PCIDevice *pci_dev)
 {
     DeviceState *dev = DEVICE(pci_dev);
@@ -1539,6 +1562,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 */
-- 
2.1.0

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

[Amos Kong]

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 <akong@redhat.com>
---
 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);
     }
-- 
2.1.0

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

[Jason Wang]

On Thu, Dec 18, 2014 at 5:22 PM, Amos Kong <akong@redhat.com> 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 <akong@redhat.com>
> ---
>  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?
> 
>          qemu_flush_queued_packets(qemu_get_queue(s->nic));
>          start_xmit(s);
>      }
> -- 
> 2.1.0
> 
> 

Re: [Qemu-devel] [PATCH 0/2] ignore bus master for e1000

[Jason Wang]

On Thu, Dec 18, 2014 at 5:22 PM, Amos Kong <akong@redhat.com> wrote:
> Win2012-64r2 guest doesn't set bus mastering correctly,
> it caused guest network down, this patch ignored it for
> e1000 nic for workarounding the guest issue.
> 
> Patch 1 is an update version of:
> http://lists.gnu.org/archive/html/qemu-devel/2014-12/msg00048.html
> transmit packets are also defered in latest version.
> 
> Amos Kong (1):
>   e1000: unconditionally enable bus mastering
> 
> Michael S. Tsirkin (1):
>   e1000: defer packets until BM enabled
> 
>  hw/net/e1000.c | 34 +++++++++++++++++++++++++++++++++-
>  1 file changed, 33 insertions(+), 1 deletion(-)
> 
> -- 
> 2.1.0

Another question unrelated to this series:
Does 82540EM still supported by Win2k12?
The following link shows that 82540EM were not supported in Win2k12.
https://downloadcenter.intel.com/SearchResult.aspx?lang=eng&ProdId=983

The oldest 825x card supported by 2k12 seems 82574.
https://downloadcenter.intel.com/Detail_Desc.aspx?DwnldID=21694&lang=eng&ProdId=3189

If yes, workaround a buggy driver may be endless work in the future.

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

[Amos Kong]

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

On Thu, Dec 18, 2014 at 09:57:29AM +0008, Jason Wang wrote:
> 
> 
> On Thu, Dec 18, 2014 at 5:22 PM, Amos Kong <akong@redhat.com> 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 <akong@redhat.com>
> >---
> > 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.

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

Re: [Qemu-devel] [PATCH 0/2] ignore bus master for e1000

[Denis V. Lunev]

On 18/12/14 13:05, Jason Wang wrote:
>
>
> On Thu, Dec 18, 2014 at 5:22 PM, Amos Kong <akong@redhat.com> wrote:
>> Win2012-64r2 guest doesn't set bus mastering correctly,
>> it caused guest network down, this patch ignored it for
>> e1000 nic for workarounding the guest issue.
>>
>> Patch 1 is an update version of:
>> http://lists.gnu.org/archive/html/qemu-devel/2014-12/msg00048.html
>> transmit packets are also defered in latest version.
>>
>> Amos Kong (1):
>>   e1000: unconditionally enable bus mastering
>>
>> Michael S. Tsirkin (1):
>>   e1000: defer packets until BM enabled
>>
>>  hw/net/e1000.c | 34 +++++++++++++++++++++++++++++++++-
>>  1 file changed, 33 insertions(+), 1 deletion(-)
>>
>> --
>> 2.1.0
>
> Another question unrelated to this series:
> Does 82540EM still supported by Win2k12?
> The following link shows that 82540EM were not supported in Win2k12.
> https://downloadcenter.intel.com/SearchResult.aspx?lang=eng&ProdId=983
>
> The oldest 825x card supported by 2k12 seems 82574.
> https://downloadcenter.intel.com/Detail_Desc.aspx?DwnldID=21694&lang=eng&ProdId=3189
>
>
> If yes, workaround a buggy driver may be endless work in the future.
>
>

w2k12r2 datacenter edition founds QEMU e1000 card just fine
(checked by observation, very basic browser test).

Though I know some troubles with Parallels emulation of 82574
in w2k12 on driver size. The driver is really old and not
supported.

Thus from my opinion switching to this device might not be
a good choice.

Re: [Qemu-devel] [PATCH 0/2] ignore bus master for e1000

[Denis V. Lunev]

On 18/12/14 14:01, Denis V. Lunev wrote:
> On 18/12/14 13:05, Jason Wang wrote:
>>
>>
>> On Thu, Dec 18, 2014 at 5:22 PM, Amos Kong <akong@redhat.com> wrote:
>>> Win2012-64r2 guest doesn't set bus mastering correctly,
>>> it caused guest network down, this patch ignored it for
>>> e1000 nic for workarounding the guest issue.
>>>
>>> Patch 1 is an update version of:
>>> http://lists.gnu.org/archive/html/qemu-devel/2014-12/msg00048.html
>>> transmit packets are also defered in latest version.
>>>
>>> Amos Kong (1):
>>>   e1000: unconditionally enable bus mastering
>>>
>>> Michael S. Tsirkin (1):
>>>   e1000: defer packets until BM enabled
>>>
>>>  hw/net/e1000.c | 34 +++++++++++++++++++++++++++++++++-
>>>  1 file changed, 33 insertions(+), 1 deletion(-)
>>>
>>> --
>>> 2.1.0
>>
>> Another question unrelated to this series:
>> Does 82540EM still supported by Win2k12?
>> The following link shows that 82540EM were not supported in Win2k12.
>> https://downloadcenter.intel.com/SearchResult.aspx?lang=eng&ProdId=983
>>
>> The oldest 825x card supported by 2k12 seems 82574.
>> https://downloadcenter.intel.com/Detail_Desc.aspx?DwnldID=21694&lang=eng&ProdId=3189
>>
>>
>>
>> If yes, workaround a buggy driver may be endless work in the future.
>>
>>
>
> w2k12r2 datacenter edition founds QEMU e1000 card just fine
> (checked by observation, very basic browser test).
>
> Though I know some troubles with Parallels emulation of 82574
> in w2k12 on driver size. The driver is really old and not
> supported.
>
> Thus from my opinion switching to this device might not be
> a good choice.

correction. Mistaken a bit. Sorry for this.

1) 82540EM and 82545EM are at least working. There are some
    troubles in driver under the load. Driver is the same.
2) 82574 is known to be slower in Parallels/VMware.

Re: [Qemu-devel] [PATCH 0/2] ignore bus master for e1000

[Amos Kong]

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

On Thu, Dec 18, 2014 at 10:13:07AM +0008, Jason Wang wrote:
> 
> 
> On Thu, Dec 18, 2014 at 5:22 PM, Amos Kong <akong@redhat.com> wrote:
> >Win2012-64r2 guest doesn't set bus mastering correctly,
> >it caused guest network down, this patch ignored it for
> >e1000 nic for workarounding the guest issue.
> >
> >Patch 1 is an update version of:
> >http://lists.gnu.org/archive/html/qemu-devel/2014-12/msg00048.html
> >transmit packets are also defered in latest version.
> >
> >Amos Kong (1):
> >  e1000: unconditionally enable bus mastering
> >
> >Michael S. Tsirkin (1):
> >  e1000: defer packets until BM enabled
> >
> > hw/net/e1000.c | 34 +++++++++++++++++++++++++++++++++-
> > 1 file changed, 33 insertions(+), 1 deletion(-)
> >
> >-- 
> >2.1.0
> 
> Another question unrelated to this series:
> Does 82540EM still supported by Win2k12?

82540EM isn't in the support list. Some users want to use emulated
e1000 to debug Windows guests. I will try other emulated nics in QEMU.

> The following link shows that 82540EM were not supported in Win2k12.
> https://downloadcenter.intel.com/SearchResult.aspx?lang=eng&ProdId=983
> 
> The oldest 825x card supported by 2k12 seems 82574.
> https://downloadcenter.intel.com/Detail_Desc.aspx?DwnldID=21694&lang=eng&ProdId=3189
 
> If yes, workaround a buggy driver may be endless work in the future.

I think so.

-- 
			Amos.

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

Re: [Qemu-devel] [PATCH 0/2] ignore bus master for e1000

[Jason Wang]

On Thu, Dec 18, 2014 at 7:11 PM, Denis V. Lunev 
<den-lists@parallels.com> wrote:
> On 18/12/14 14:01, Denis V. Lunev wrote:
>> On 18/12/14 13:05, Jason Wang wrote:
>>> 
>>> 
>>> On Thu, Dec 18, 2014 at 5:22 PM, Amos Kong <akong@redhat.com> wrote:
>>>> Win2012-64r2 guest doesn't set bus mastering correctly,
>>>> it caused guest network down, this patch ignored it for
>>>> e1000 nic for workarounding the guest issue.
>>>> 
>>>> Patch 1 is an update version of:
>>>> http://lists.gnu.org/archive/html/qemu-devel/2014-12/msg00048.html
>>>> transmit packets are also defered in latest version.
>>>> 
>>>> Amos Kong (1):
>>>>   e1000: unconditionally enable bus mastering
>>>> 
>>>> Michael S. Tsirkin (1):
>>>>   e1000: defer packets until BM enabled
>>>> 
>>>>  hw/net/e1000.c | 34 +++++++++++++++++++++++++++++++++-
>>>>  1 file changed, 33 insertions(+), 1 deletion(-)
>>>> 
>>>> --
>>>> 2.1.0
>>> 
>>> Another question unrelated to this series:
>>> Does 82540EM still supported by Win2k12?
>>> The following link shows that 82540EM were not supported in Win2k12.
>>> https://downloadcenter.intel.com/SearchResult.aspx?lang=eng&ProdId=983
>>> 
>>> The oldest 825x card supported by 2k12 seems 82574.
>>> https://downloadcenter.intel.com/Detail_Desc.aspx?DwnldID=21694&lang=eng&ProdId=3189
>>> 
>>> 
>>> 
>>> If yes, workaround a buggy driver may be endless work in the future.
>>> 
>>> 
>> 
>> w2k12r2 datacenter edition founds QEMU e1000 card just fine
>> (checked by observation, very basic browser test).
>> 
>> Though I know some troubles with Parallels emulation of 82574
>> in w2k12 on driver size. The driver is really old and not
>> supported.
>> 
>> Thus from my opinion switching to this device might not be
>> a good choice.
> 
> correction. Mistaken a bit. Sorry for this.
> 
> 1) 82540EM and 82545EM are at least working. There are some
>    troubles in driver under the load. Driver is the same.

If you mean the troubles happen on real hardware.
We'd really not encourage the user to use e1000 in 2k12 guest.
> 
> 2) 82574 is known to be slower in Parallels/VMware.

Re: [Qemu-devel] [PATCH 0/2] ignore bus master for e1000

[Jason Wang]

On Fri, Dec 19, 2014 at 11:09 AM, Amos Kong <akong@redhat.com> wrote:
> On Thu, Dec 18, 2014 at 10:13:07AM +0008, Jason Wang wrote:
>>  
>>  
>>  On Thu, Dec 18, 2014 at 5:22 PM, Amos Kong <akong@redhat.com> wrote:
>>  >Win2012-64r2 guest doesn't set bus mastering correctly,
>>  >it caused guest network down, this patch ignored it for
>>  >e1000 nic for workarounding the guest issue.
>>  >
>>  >Patch 1 is an update version of:
>>  >http://lists.gnu.org/archive/html/qemu-devel/2014-12/msg00048.html
>>  >transmit packets are also defered in latest version.
>>  >
>>  >Amos Kong (1):
>>  >  e1000: unconditionally enable bus mastering
>>  >
>>  >Michael S. Tsirkin (1):
>>  >  e1000: defer packets until BM enabled
>>  >
>>  > hw/net/e1000.c | 34 +++++++++++++++++++++++++++++++++-
>>  > 1 file changed, 33 insertions(+), 1 deletion(-)
>>  >
>>  >-- 
>>  >2.1.0
>>  
>>  Another question unrelated to this series:
>>  Does 82540EM still supported by Win2k12?
> 
> 82540EM isn't in the support list. Some users want to use emulated
> e1000 to debug Windows guests. I will try other emulated nics in QEMU.

Right, but I suspect the issue may be still. But let's see.
> 
>>  The following link shows that 82540EM were not supported in Win2k12.
>>  
>> https://downloadcenter.intel.com/SearchResult.aspx?lang=eng&ProdId=983
>>  
>>  The oldest 825x card supported by 2k12 seems 82574.
>>  
>> https://downloadcenter.intel.com/Detail_Desc.aspx?DwnldID=21694&lang=eng&ProdId=3189
>  
>>  If yes, workaround a buggy driver may be endless work in the future.
> 
> I think so.
> 
> -- 
> 			Amos.

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

[Stefan Hajnoczi]

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

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 <akong@redhat.com>
> ---
>  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?

Before we merge a hack like this we should understand the problem 100%.

Stefan

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

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

[Stefan Hajnoczi]

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

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 <akong@redhat.com>
> > ---
> >  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?
> 
> Before we merge a hack like this we should understand the problem 100%.

Any new insights into what is going on here?

Stefan

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

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

[Amos Kong]

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

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 <akong@redhat.com>
> > > ---
> > >  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.

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