e1000: fix init values for command register

Submitted by Michael S. Tsirkin on Dec. 9, 2009, 8:35 p.m.

Details

Message ID 20091209203537.GA7836@redhat.com
State New
Headers show

Commit Message

Michael S. Tsirkin Dec. 9, 2009, 8:35 p.m.
Command register for e1000 was initialized to
values out of spec: all of bus master,
io, memory and interrupt disable bits were set.

This breaks the device now that we actually respect
the interrupt disable bit, unless the guest
happens to clear it. Fix, and make the device
more spec compliant, by not touching
the default.

There are implications for migration
from old qemu as well, will be addressed
separately.

Reported-by: Luiz Capitulino <lcapitulino@redhat.com>
Tested-by: Luiz Capitulino <lcapitulino@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/e1000.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

Comments

Luiz Capitulino Dec. 10, 2009, 1:03 p.m.
On Wed, 9 Dec 2009 22:35:37 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> Command register for e1000 was initialized to
> values out of spec: all of bus master,
> io, memory and interrupt disable bits were set.
> 
> This breaks the device now that we actually respect
> the interrupt disable bit, unless the guest
> happens to clear it. Fix, and make the device
> more spec compliant, by not touching
> the default.
> 
> There are implications for migration
> from old qemu as well, will be addressed
> separately.

 FWIW, I've tested the others network cards (only for the slirp case)
and they worked fine.
Michael S. Tsirkin Dec. 10, 2009, 6:14 p.m.
On Wed, Dec 09, 2009 at 10:35:37PM +0200, Michael S. Tsirkin wrote:
> Command register for e1000 was initialized to
> values out of spec: all of bus master,
> io, memory and interrupt disable bits were set.
> 
> This breaks the device now that we actually respect
> the interrupt disable bit, unless the guest
> happens to clear it. Fix, and make the device
> more spec compliant, by not touching
> the default.
> 
> There are implications for migration
> from old qemu as well, will be addressed
> separately.
> 
> Reported-by: Luiz Capitulino <lcapitulino@redhat.com>
> Tested-by: Luiz Capitulino <lcapitulino@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

This patch is also appropriate for 0.12, it will make migration 0.12 to
master less painful.
What's the process to get it there?
Should I create pci-for-0.12 branch on my tree?


> ---
>  hw/e1000.c |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/e1000.c b/hw/e1000.c
> index 8566fe3..ad7a267 100644
> --- a/hw/e1000.c
> +++ b/hw/e1000.c
> @@ -1089,7 +1089,6 @@ static int pci_e1000_init(PCIDevice *pci_dev)
>  
>      pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_INTEL);
>      pci_config_set_device_id(pci_conf, E1000_DEVID);
> -    *(uint16_t *)(pci_conf+0x04) = cpu_to_le16(0x0407);
>      *(uint16_t *)(pci_conf+0x06) = cpu_to_le16(0x0010);
>      pci_conf[0x08] = 0x03;
>      pci_config_set_class(pci_conf, PCI_CLASS_NETWORK_ETHERNET);
> -- 
> 1.6.6.rc1.43.gf55cc

Patch hide | download patch | download mbox

diff --git a/hw/e1000.c b/hw/e1000.c
index 8566fe3..ad7a267 100644
--- a/hw/e1000.c
+++ b/hw/e1000.c
@@ -1089,7 +1089,6 @@  static int pci_e1000_init(PCIDevice *pci_dev)
 
     pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_INTEL);
     pci_config_set_device_id(pci_conf, E1000_DEVID);
-    *(uint16_t *)(pci_conf+0x04) = cpu_to_le16(0x0407);
     *(uint16_t *)(pci_conf+0x06) = cpu_to_le16(0x0010);
     pci_conf[0x08] = 0x03;
     pci_config_set_class(pci_conf, PCI_CLASS_NETWORK_ETHERNET);