Skip to contentSkip to conversation

null mac address

Qemu-devel New William Dauchy 4 participants 16 messages Feb 24, 2011 – Mar 13, 2011

lore
Share & tools
lore
#0 lore raw

Hi,

I got some troubles hot plugging network pci devices. An attach works as expected but the mac address is still set to "00:00:00:00:00:00" on the guest machine. I have to reboot the guest to get the correct mac address.
I first tried through libvirt with:

# virsh attach-interface dom0 network default --mac 52:54:00:f6:84:ba

and then through qemu monitor to make sure that it wasn't a libvirt issue:
device_add rtl8139
or
device_add rtl8139,mac=01:02:03:04:05:06

Always the same result on the guest. A device info on qemu give the correct result, that is to say, with a correct mac address.
I went through rtl8139.c and saw that the mac address is set in rtl8139_reset. This function was called in pci_rtl8139_init but removed since c169998802505c244b8bcad562633f29de7d74a4 commit, because it doesn't make sense to call it when the virtual machine is shutdown.
I'm now wondering where I am supposed to call this reset function when live attaching a pci device. I think it could fix the mac address issue.
I will be very pleased to receive some tips to create a patch for this issue.

Regards,

Signature · 1 line
William
#1 lore raw

At 02/24/2011 10:40 PM, William Dauchy Write:

Quoted 14 lines

Hi,

I got some troubles hot plugging network pci devices. An attach works as expected but the mac address is still set to "00:00:00:00:00:00" on the guest machine. I have to reboot the guest to get the correct mac address.
I first tried through libvirt with:

# virsh attach-interface dom0 network default --mac 52:54:00:f6:84:ba

and then through qemu monitor to make sure that it wasn't a libvirt issue:
device_add rtl8139
or
device_add rtl8139,mac=01:02:03:04:05:06

Quoted 10 lines

Always the same result on the guest. A device info on qemu give the correct result, that is to say, with a correct mac address.
I went through rtl8139.c and saw that the mac address is set in rtl8139_reset. This function was called in pci_rtl8139_init but removed since c169998802505c244b8bcad562633f29de7d74a4 commit, because it doesn't make sense to call it when the virtual machine is shutdown.
I'm now wondering where I am supposed to call this reset function when live attaching a pci device. I think it could fix the mac address issue.
I will be very pleased to receive some tips to create a patch for this issue.

I got the same troubles, but I don't notice commit c1699988, and I sent a patch(call reset function in init) some days before. So I think this patch is wrong.

I think the following method can solve this problem:
move eeprom init from reset function into init function, as it is read only, and does not need init again in reset function.

I will test this method. If it's OK, I will send a new patch.

Thnaks
Wen Congyang

Quoted 2 lines

Regards,

#2 lore raw
In reply to William Dauchy

At 02/24/2011 10:40 PM, William Dauchy Write:

Quoted 14 lines

Hi,

I got some troubles hot plugging network pci devices. An attach works as expected but the mac address is still set to "00:00:00:00:00:00" on the guest machine. I have to reboot the guest to get the correct mac address.
I first tried through libvirt with:

# virsh attach-interface dom0 network default --mac 52:54:00:f6:84:ba

and then through qemu monitor to make sure that it wasn't a libvirt issue:
device_add rtl8139
or
device_add rtl8139,mac=01:02:03:04:05:06

Quoted 10 lines

Always the same result on the guest. A device info on qemu give the correct result, that is to say, with a correct mac address.
I went through rtl8139.c and saw that the mac address is set in rtl8139_reset. This function was called in pci_rtl8139_init but removed since c169998802505c244b8bcad562633f29de7d74a4 commit, because it doesn't make sense to call it when the virtual machine is shutdown.
I'm now wondering where I am supposed to call this reset function when live attaching a pci device. I think it could fix the mac address issue.
I will be very pleased to receive some tips to create a patch for this issue.

Please try the following patch.

Thanks
Wen Congyang

Quoted 1 line

From efa0632f563a69dc299daaf4b235c1a0521d6e02 Mon Sep 17 00:00:00 2001

From: Wen Congyang <[email protected]>
Date: Fri, 25 Feb 2011 09:56:27 +0800
Subject: [PATCH] move eeprom init from reset function to init function

---
 hw/pcnet-pci.c |   12 ++++++++++++
 hw/pcnet.c     |   13 -------------
 hw/rtl8139.c   |   24 ++++++++++++------------
 3 files changed, 24 insertions(+), 25 deletions(-)
hw/pcnet-pci.c +12 -0
index 339a401..d7c4fc3 100644
@@ -270,6 +270,8 @@ static int pci_pcnet_init(PCIDevice *pci_dev)
     PCIPCNetState *d = DO_UPCAST(PCIPCNetState, pci_dev, pci_dev);
     PCNetState *s = &d->state;
     uint8_t *pci_conf;
+    int i;
+    uint16_t checksum;
 
 #if 0
     printf("sizeof(RMD)=%d, sizeof(TMD)=%d\n",
@@ -292,6 +294,16 @@ static int pci_pcnet_init(PCIDevice *pci_dev)
     pci_conf[PCI_MIN_GNT] = 0x06;
     pci_conf[PCI_MAX_LAT] = 0xff;
 
+    /* Initialize the PROM */
+
+    memcpy(s->prom, s->conf.macaddr.a, 6);
+    s->prom[12] = s->prom[13] = 0x00;
+    s->prom[14] = s->prom[15] = 0x57;
+
+    for (i = 0,checksum = 0; i < 16; i++)
+        checksum += s->prom[i];
+    *(uint16_t *)&s->prom[12] = cpu_to_le16(checksum);
+
     /* Handler for memory-mapped I/O */
     s->mmio_index =
       cpu_register_io_memory(pcnet_mmio_read, pcnet_mmio_write, &d->state,
hw/pcnet.c +0 -13
index db52dc5..283828a 100644
@@ -1557,19 +1557,6 @@ uint32_t pcnet_bcr_readw(PCNetState *s, uint32_t rap)
 void pcnet_h_reset(void *opaque)
 {
     PCNetState *s = opaque;
-    int i;
-    uint16_t checksum;
-
-    /* Initialize the PROM */
-
-    memcpy(s->prom, s->conf.macaddr.a, 6);
-    s->prom[12] = s->prom[13] = 0x00;
-    s->prom[14] = s->prom[15] = 0x57;
-
-    for (i = 0,checksum = 0; i < 16; i++)
-        checksum += s->prom[i];
-    *(uint16_t *)&s->prom[12] = cpu_to_le16(checksum);
-
 
     s->bcr[BCR_MSRDA] = 0x0005;
     s->bcr[BCR_MSWRA] = 0x0005;
hw/rtl8139.c +12 -12
index a22530c..580c83e 100644
@@ -1189,18 +1189,6 @@ static void rtl8139_reset(DeviceState *d)
 
     rtl8139_update_irq(s);
 
-    /* prepare eeprom */
-    s->eeprom.contents[0] = 0x8129;
-#if 1
-    // PCI vendor and device ID should be mirrored here
-    s->eeprom.contents[1] = PCI_VENDOR_ID_REALTEK;
-    s->eeprom.contents[2] = PCI_DEVICE_ID_REALTEK_8139;
-#endif
-
-    s->eeprom.contents[7] = s->conf.macaddr.a[0] | s->conf.macaddr.a[1] << 8;
-    s->eeprom.contents[8] = s->conf.macaddr.a[2] | s->conf.macaddr.a[3] << 8;
-    s->eeprom.contents[9] = s->conf.macaddr.a[4] | s->conf.macaddr.a[5] << 8;
-
     /* mark all status registers as owned by host */
     for (i = 0; i < 4; ++i)
     {
@@ -3392,6 +3380,18 @@ static int pci_rtl8139_init(PCIDevice *dev)
 
     qemu_macaddr_default_if_unset(&s->conf.macaddr);
 
+    /* prepare eeprom */
+    s->eeprom.contents[0] = 0x8129;
+#if 1
+    // PCI vendor and device ID should be mirrored here
+    s->eeprom.contents[1] = PCI_VENDOR_ID_REALTEK;
+    s->eeprom.contents[2] = PCI_DEVICE_ID_REALTEK_8139;
+#endif
+
+    s->eeprom.contents[7] = s->conf.macaddr.a[0] | s->conf.macaddr.a[1] << 8;
+    s->eeprom.contents[8] = s->conf.macaddr.a[2] | s->conf.macaddr.a[3] << 8;
+    s->eeprom.contents[9] = s->conf.macaddr.a[4] | s->conf.macaddr.a[5] << 8;
+
     s->nic = qemu_new_nic(&net_rtl8139_info, &s->conf,
                           dev->qdev.info->name, dev->qdev.id, s);
     qemu_format_nic_info_str(&s->nic->nc, s->conf.macaddr.a);
Signature · 1 line
1.7.1
Quoted 2 lines

Regards,

#3 lore raw
Quoted 56 lines · Wen Congyang

At 02/24/2011 10:40 PM, William Dauchy Write:

Quoted 14 lines

Hi,

I got some troubles hot plugging network pci devices. An attach works as expected but the mac address is still set to "00:00:00:00:00:00" on the guest machine. I have to reboot the guest to get the correct mac address.
I first tried through libvirt with:

# virsh attach-interface dom0 network default --mac 52:54:00:f6:84:ba

and then through qemu monitor to make sure that it wasn't a libvirt issue:
device_add rtl8139
or
device_add rtl8139,mac=01:02:03:04:05:06

Quoted 10 lines

Always the same result on the guest. A device info on qemu give the correct result, that is to say, with a correct mac address.
I went through rtl8139.c and saw that the mac address is set in rtl8139_reset. This function was called in pci_rtl8139_init but removed since c169998802505c244b8bcad562633f29de7d74a4 commit, because it doesn't make sense to call it when the virtual machine is shutdown.
I'm now wondering where I am supposed to call this reset function when live attaching a pci device. I think it could fix the mac address issue.
I will be very pleased to receive some tips to create a patch for this issue.

Please try the following patch.

Thanks
Wen Congyang

From efa0632f563a69dc299daaf4b235c1a0521d6e02 Mon Sep 17 00:00:00 2001
From: Wen Congyang <[email protected]>
Date: Fri, 25 Feb 2011 09:56:27 +0800
Subject: [PATCH] move eeprom init from reset function to init function

---
 hw/pcnet-pci.c |   12 ++++++++++++
 hw/pcnet.c     |   13 -------------
 hw/rtl8139.c   |   24 ++++++++++++------------

 3 files changed, 24 insertions(+), 25 deletions(-)
hw/pcnet-pci.c +2 -0
index 339a401..d7c4fc3 100644
@@ -270,6 +270,8 @@ static int pci_pcnet_init(PCIDevice *pci_dev)
     PCIPCNetState *d = DO_UPCAST(PCIPCNetState, pci_dev, pci_dev);
     PCNetState *s = &d->state;
     uint8_t *pci_conf;
+    int i;
+    uint16_t checksum;
 
  #if 0
     printf("sizeof(RMD)=%d, sizeof(TMD)=%d\n",

hw/pcnet-pci.c static int pci_pcnet_init(PCIDevice *pci_dev) · line 303

Quoted 10 lines
hw/pcnet-pci.c +7 -0
@@ -292,6 +294,16 @@ static int pci_pcnet_init(PCIDevice *pci_dev)
     pci_conf[PCI_MIN_GNT] = 0x06;
     pci_conf[PCI_MAX_LAT] = 0xff;
 
+    /* Initialize the PROM */
+
+    memcpy(s->prom, s->conf.macaddr.a, 6);
+    s->prom[12] = s->prom[13] = 0x00;
+    s->prom[14] = s->prom[15] = 0x57;
+
+    for (i = 0,checksum = 0; i < 16; i++)

Please add braces to fix the CODING_STYLE problem while moving.

hw/pcnet-pci.c static int pci_pcnet_init(PCIDevice *pci_dev) · line 305

Quoted 2 lines
hw/pcnet-pci.c +2 -0
+        checksum += s->prom[i];
+    *(uint16_t *)&s->prom[12] = cpu_to_le16(checksum);

This is not the right place, since lance.c uses the common part of pcnet.c. Please put the lines instead to pcnet_common_init().

hw/pcnet-pci.c static int pci_pcnet_init(PCIDevice *pci_dev) · line 306

Quoted 1 line
hw/pcnet-pci.c +1 -0
+    // PCI vendor and device ID should be mirrored here

Also here it would be nice to convert C99 comments to C89 while moving.

#4 lore raw

PATCH v2 1/1 move eeprom init from reset function to init function

When we hot plug network pci devices, the mac address is still set to "00:00:00:00:00:00" on the guest machine. The reason is that the kernel does not reset the device and we init eeprom in reset function.

move eeprom init from reset function into init function, as it is read only, and does not need init again in reset function.

---
 hw/pcnet.c   |   27 ++++++++++++++-------------
 hw/rtl8139.c |   24 ++++++++++++------------
 2 files changed, 26 insertions(+), 25 deletions(-)
hw/pcnet.c +14 -13
index db52dc5..4499031 100644
@@ -1557,19 +1557,6 @@ uint32_t pcnet_bcr_readw(PCNetState *s, uint32_t rap)
 void pcnet_h_reset(void *opaque)
 {
     PCNetState *s = opaque;
-    int i;
-    uint16_t checksum;
-
-    /* Initialize the PROM */
-
-    memcpy(s->prom, s->conf.macaddr.a, 6);
-    s->prom[12] = s->prom[13] = 0x00;
-    s->prom[14] = s->prom[15] = 0x57;
-
-    for (i = 0,checksum = 0; i < 16; i++)
-        checksum += s->prom[i];
-    *(uint16_t *)&s->prom[12] = cpu_to_le16(checksum);
-
 
     s->bcr[BCR_MSRDA] = 0x0005;
     s->bcr[BCR_MSWRA] = 0x0005;
@@ -1736,6 +1723,20 @@ void pcnet_common_cleanup(PCNetState *d)
 
 int pcnet_common_init(DeviceState *dev, PCNetState *s, NetClientInfo *info)
 {
+    int i;
+    uint16_t checksum;
+
+    /* Initialize the PROM */
+
+    memcpy(s->prom, s->conf.macaddr.a, 6);
+    s->prom[12] = s->prom[13] = 0x00;
+    s->prom[14] = s->prom[15] = 0x57;
+
+    for (i = 0, checksum = 0; i < 16; i++) {
+        checksum += s->prom[i];
+    }
+    *(uint16_t *)&s->prom[12] = cpu_to_le16(checksum);
+
     s->poll_timer = qemu_new_timer(vm_clock, pcnet_poll_timer, s);
 
     qemu_macaddr_default_if_unset(&s->conf.macaddr);
hw/rtl8139.c +12 -12
index a22530c..8356d5a 100644
@@ -1189,18 +1189,6 @@ static void rtl8139_reset(DeviceState *d)
 
     rtl8139_update_irq(s);
 
-    /* prepare eeprom */
-    s->eeprom.contents[0] = 0x8129;
-#if 1
-    // PCI vendor and device ID should be mirrored here
-    s->eeprom.contents[1] = PCI_VENDOR_ID_REALTEK;
-    s->eeprom.contents[2] = PCI_DEVICE_ID_REALTEK_8139;
-#endif
-
-    s->eeprom.contents[7] = s->conf.macaddr.a[0] | s->conf.macaddr.a[1] << 8;
-    s->eeprom.contents[8] = s->conf.macaddr.a[2] | s->conf.macaddr.a[3] << 8;
-    s->eeprom.contents[9] = s->conf.macaddr.a[4] | s->conf.macaddr.a[5] << 8;
-
     /* mark all status registers as owned by host */
     for (i = 0; i < 4; ++i)
     {
@@ -3392,6 +3380,18 @@ static int pci_rtl8139_init(PCIDevice *dev)
 
     qemu_macaddr_default_if_unset(&s->conf.macaddr);
 
+    /* prepare eeprom */
+    s->eeprom.contents[0] = 0x8129;
+#if 1
+    /* PCI vendor and device ID should be mirrored here */
+    s->eeprom.contents[1] = PCI_VENDOR_ID_REALTEK;
+    s->eeprom.contents[2] = PCI_DEVICE_ID_REALTEK_8139;
+#endif
+
+    s->eeprom.contents[7] = s->conf.macaddr.a[0] | s->conf.macaddr.a[1] << 8;
+    s->eeprom.contents[8] = s->conf.macaddr.a[2] | s->conf.macaddr.a[3] << 8;
+    s->eeprom.contents[9] = s->conf.macaddr.a[4] | s->conf.macaddr.a[5] << 8;
+
     s->nic = qemu_new_nic(&net_rtl8139_info, &s->conf,
                           dev->qdev.info->name, dev->qdev.id, s);
     qemu_format_nic_info_str(&s->nic->nc, s->conf.macaddr.a);
Signature · 1 line
1.7.1
#5 lore raw

PATCH 1/1 moving eeprom initialization

In reply to Blue Swirl

The initialization should not be only on reset but also when initializing the device.
It resolves a bug when hot plugging a pci network device: the mac address was always null.

---
 hw/pcnet.c   |   27 ++++++++++++++-------------
 hw/rtl8139.c |   24 ++++++++++++------------
 2 files changed, 26 insertions(+), 25 deletions(-)
hw/pcnet.c +14 -13
index db52dc5..4e30e9c 100644
@@ -1557,19 +1557,6 @@ uint32_t pcnet_bcr_readw(PCNetState *s, uint32_t rap)
 void pcnet_h_reset(void *opaque)
 {
     PCNetState *s = opaque;
-    int i;
-    uint16_t checksum;
-
-    /* Initialize the PROM */
-
-    memcpy(s->prom, s->conf.macaddr.a, 6);
-    s->prom[12] = s->prom[13] = 0x00;
-    s->prom[14] = s->prom[15] = 0x57;
-
-    for (i = 0,checksum = 0; i < 16; i++)
-        checksum += s->prom[i];
-    *(uint16_t *)&s->prom[12] = cpu_to_le16(checksum);
-
 
     s->bcr[BCR_MSRDA] = 0x0005;
     s->bcr[BCR_MSWRA] = 0x0005;
@@ -1736,6 +1723,9 @@ void pcnet_common_cleanup(PCNetState *d)
 
 int pcnet_common_init(DeviceState *dev, PCNetState *s, NetClientInfo *info)
 {
+    int i;
+    uint16_t checksum;
+
     s->poll_timer = qemu_new_timer(vm_clock, pcnet_poll_timer, s);
 
     qemu_macaddr_default_if_unset(&s->conf.macaddr);
@@ -1744,5 +1734,16 @@ int pcnet_common_init(DeviceState *dev, PCNetState *s, NetClientInfo *info)
 
     add_boot_device_path(s->conf.bootindex, dev, "/ethernet-phy@0");
 
+    /* Initialize the PROM */
+
+    memcpy(s->prom, s->conf.macaddr.a, 6);
+    s->prom[12] = s->prom[13] = 0x00;
+    s->prom[14] = s->prom[15] = 0x57;
+
+    for (i = 0, checksum = 0; i < 16; i++) {
+        checksum += s->prom[i];
+    }
+    *(uint16_t *)&s->prom[12] = cpu_to_le16(checksum);
+
     return 0;
 }
hw/rtl8139.c +12 -12
index a22530c..8356d5a 100644
@@ -1189,18 +1189,6 @@ static void rtl8139_reset(DeviceState *d)
 
     rtl8139_update_irq(s);
 
-    /* prepare eeprom */
-    s->eeprom.contents[0] = 0x8129;
-#if 1
-    // PCI vendor and device ID should be mirrored here
-    s->eeprom.contents[1] = PCI_VENDOR_ID_REALTEK;
-    s->eeprom.contents[2] = PCI_DEVICE_ID_REALTEK_8139;
-#endif
-
-    s->eeprom.contents[7] = s->conf.macaddr.a[0] | s->conf.macaddr.a[1] << 8;
-    s->eeprom.contents[8] = s->conf.macaddr.a[2] | s->conf.macaddr.a[3] << 8;
-    s->eeprom.contents[9] = s->conf.macaddr.a[4] | s->conf.macaddr.a[5] << 8;
-
     /* mark all status registers as owned by host */
     for (i = 0; i < 4; ++i)
     {
@@ -3392,6 +3380,18 @@ static int pci_rtl8139_init(PCIDevice *dev)
 
     qemu_macaddr_default_if_unset(&s->conf.macaddr);
 
+    /* prepare eeprom */
+    s->eeprom.contents[0] = 0x8129;
+#if 1
+    /* PCI vendor and device ID should be mirrored here */
+    s->eeprom.contents[1] = PCI_VENDOR_ID_REALTEK;
+    s->eeprom.contents[2] = PCI_DEVICE_ID_REALTEK_8139;
+#endif
+
+    s->eeprom.contents[7] = s->conf.macaddr.a[0] | s->conf.macaddr.a[1] << 8;
+    s->eeprom.contents[8] = s->conf.macaddr.a[2] | s->conf.macaddr.a[3] << 8;
+    s->eeprom.contents[9] = s->conf.macaddr.a[4] | s->conf.macaddr.a[5] << 8;
+
     s->nic = qemu_new_nic(&net_rtl8139_info, &s->conf,
                           dev->qdev.info->name, dev->qdev.id, s);
     qemu_format_nic_info_str(&s->nic->nc, s->conf.macaddr.a);
Signature · 1 line
William
#6 lore raw

[Qemu-devel] Re: [PATCH] moving eeprom initialization

Quoted 103 lines · William Dauchy

The initialization should not be only on reset but also when initializing the device.
It resolves a bug when hot plugging a pci network device: the mac address was always null.
---
 hw/pcnet.c   |   27 ++++++++++++++-------------
 hw/rtl8139.c |   24 ++++++++++++------------

 2 files changed, 26 insertions(+), 25 deletions(-)
hw/pcnet.c +14 -13
index db52dc5..4e30e9c 100644
@@ -1557,19 +1557,6 @@ uint32_t pcnet_bcr_readw(PCNetState *s, uint32_t rap)
  void pcnet_h_reset(void *opaque)
  {
     PCNetState *s = opaque;
-    int i;
-    uint16_t checksum;
-
-    /* Initialize the PROM */
-
-    memcpy(s->prom, s->conf.macaddr.a, 6);
-    s->prom[12] = s->prom[13] = 0x00;
-    s->prom[14] = s->prom[15] = 0x57;
-
-    for (i = 0,checksum = 0; i < 16; i++)
-        checksum += s->prom[i];
-    *(uint16_t *)&s->prom[12] = cpu_to_le16(checksum);
-
 
     s->bcr[BCR_MSRDA] = 0x0005;
     s->bcr[BCR_MSWRA] = 0x0005;
@@ -1736,6 +1723,9 @@ void pcnet_common_cleanup(PCNetState *d)
 
  int pcnet_common_init(DeviceState *dev, PCNetState *s, NetClientInfo *info)
  {
+    int i;
+    uint16_t checksum;
+
     s->poll_timer = qemu_new_timer(vm_clock, pcnet_poll_timer, s);
 
     qemu_macaddr_default_if_unset(&s->conf.macaddr);
@@ -1744,5 +1734,16 @@ int pcnet_common_init(DeviceState *dev, PCNetState *s, NetClientInfo *info)
 
     add_boot_device_path(s->conf.bootindex, dev, "/ethernet-phy@0");
 
+    /* Initialize the PROM */
+
+    memcpy(s->prom, s->conf.macaddr.a, 6);
+    s->prom[12] = s->prom[13] = 0x00;
+    s->prom[14] = s->prom[15] = 0x57;
+
+    for (i = 0, checksum = 0; i < 16; i++) {
+        checksum += s->prom[i];
+    }
+    *(uint16_t *)&s->prom[12] = cpu_to_le16(checksum);
+
     return 0;
  }
hw/rtl8139.c +12 -13
index a22530c..8356d5a 100644
@@ -1189,18 +1189,6 @@ static void rtl8139_reset(DeviceState *d)
 
     rtl8139_update_irq(s);
 
-    /* prepare eeprom */
-    s->eeprom.contents[0] = 0x8129;
-#if 1
-    // PCI vendor and device ID should be mirrored here
-    s->eeprom.contents[1] = PCI_VENDOR_ID_REALTEK;
-    s->eeprom.contents[2] = PCI_DEVICE_ID_REALTEK_8139;
-#endif
-
-    s->eeprom.contents[7] = s->conf.macaddr.a[0] | s->conf.macaddr.a[1] << 8;
-    s->eeprom.contents[8] = s->conf.macaddr.a[2] | s->conf.macaddr.a[3] << 8;
-    s->eeprom.contents[9] = s->conf.macaddr.a[4] | s->conf.macaddr.a[5] << 8;
-
     /* mark all status registers as owned by host */
     for (i = 0; i < 4; ++i)
     {
@@ -3392,6 +3380,18 @@ static int pci_rtl8139_init(PCIDevice *dev)
 
     qemu_macaddr_default_if_unset(&s->conf.macaddr);
 
+    /* prepare eeprom */
+    s->eeprom.contents[0] = 0x8129;
+#if 1
+    /* PCI vendor and device ID should be mirrored here */
+    s->eeprom.contents[1] = PCI_VENDOR_ID_REALTEK;
+    s->eeprom.contents[2] = PCI_DEVICE_ID_REALTEK_8139;
+#endif
+
+    s->eeprom.contents[7] = s->conf.macaddr.a[0] | s->conf.macaddr.a[1] << 8;
+    s->eeprom.contents[8] = s->conf.macaddr.a[2] | s->conf.macaddr.a[3] << 8;
+    s->eeprom.contents[9] = s->conf.macaddr.a[4] | s->conf.macaddr.a[5] << 8;
+
     s->nic = qemu_new_nic(&net_rtl8139_info, &s->conf,
                           dev->qdev.info->name, dev->qdev.id, s);
     qemu_format_nic_info_str(&s->nic->nc, s->conf.macaddr.a);
--
Quoted 1 line

William

hm I just noticed your correction here
http://lists.gnu.org/archive/html/qemu-devel/2011-02/msg03232.html ; sorry
Anyway, I tested my version and it works fine.

Regards,

Signature · 1 line
William
#7 lore raw

Re: [Qemu-devel] Re: [PATCH] moving eeprom initialization

Hello,

Your patch should be based on fixes for correct EEPROM initialization, see for details: http://www.mail-archive.com/[email protected]/msg56414.html

Ciao,
Gerhard

--
http://www.wiesinger.com/

Quoted 118 lines · William Dauchy
Quoted 103 lines · William Dauchy

The initialization should not be only on reset but also when initializing the device.
It resolves a bug when hot plugging a pci network device: the mac address was always null.
---
 hw/pcnet.c   |   27 ++++++++++++++-------------
 hw/rtl8139.c |   24 ++++++++++++------------

 2 files changed, 26 insertions(+), 25 deletions(-)
hw/pcnet.c +14 -13
index db52dc5..4e30e9c 100644
@@ -1557,19 +1557,6 @@ uint32_t pcnet_bcr_readw(PCNetState *s, uint32_t rap)
  void pcnet_h_reset(void *opaque)
  {
     PCNetState *s = opaque;
-    int i;
-    uint16_t checksum;
-
-    /* Initialize the PROM */
-
-    memcpy(s->prom, s->conf.macaddr.a, 6);
-    s->prom[12] = s->prom[13] = 0x00;
-    s->prom[14] = s->prom[15] = 0x57;
-
-    for (i = 0,checksum = 0; i < 16; i++)
-        checksum += s->prom[i];
-    *(uint16_t *)&s->prom[12] = cpu_to_le16(checksum);
-
 
     s->bcr[BCR_MSRDA] = 0x0005;
     s->bcr[BCR_MSWRA] = 0x0005;
@@ -1736,6 +1723,9 @@ void pcnet_common_cleanup(PCNetState *d)
 
  int pcnet_common_init(DeviceState *dev, PCNetState *s, NetClientInfo *info)
  {
+    int i;
+    uint16_t checksum;
+
     s->poll_timer = qemu_new_timer(vm_clock, pcnet_poll_timer, s);
 
     qemu_macaddr_default_if_unset(&s->conf.macaddr);
@@ -1744,5 +1734,16 @@ int pcnet_common_init(DeviceState *dev, PCNetState *s, NetClientInfo *info)
 
     add_boot_device_path(s->conf.bootindex, dev, "/ethernet-phy@0");
 
+    /* Initialize the PROM */
+
+    memcpy(s->prom, s->conf.macaddr.a, 6);
+    s->prom[12] = s->prom[13] = 0x00;
+    s->prom[14] = s->prom[15] = 0x57;
+
+    for (i = 0, checksum = 0; i < 16; i++) {
+        checksum += s->prom[i];
+    }
+    *(uint16_t *)&s->prom[12] = cpu_to_le16(checksum);
+
     return 0;
  }
hw/rtl8139.c +12 -13
index a22530c..8356d5a 100644
@@ -1189,18 +1189,6 @@ static void rtl8139_reset(DeviceState *d)
 
     rtl8139_update_irq(s);
 
-    /* prepare eeprom */
-    s->eeprom.contents[0] = 0x8129;
-#if 1
-    // PCI vendor and device ID should be mirrored here
-    s->eeprom.contents[1] = PCI_VENDOR_ID_REALTEK;
-    s->eeprom.contents[2] = PCI_DEVICE_ID_REALTEK_8139;
-#endif
-
-    s->eeprom.contents[7] = s->conf.macaddr.a[0] | s->conf.macaddr.a[1] << 8;
-    s->eeprom.contents[8] = s->conf.macaddr.a[2] | s->conf.macaddr.a[3] << 8;
-    s->eeprom.contents[9] = s->conf.macaddr.a[4] | s->conf.macaddr.a[5] << 8;
-
     /* mark all status registers as owned by host */
     for (i = 0; i < 4; ++i)
     {
@@ -3392,6 +3380,18 @@ static int pci_rtl8139_init(PCIDevice *dev)
 
     qemu_macaddr_default_if_unset(&s->conf.macaddr);
 
+    /* prepare eeprom */
+    s->eeprom.contents[0] = 0x8129;
+#if 1
+    /* PCI vendor and device ID should be mirrored here */
+    s->eeprom.contents[1] = PCI_VENDOR_ID_REALTEK;
+    s->eeprom.contents[2] = PCI_DEVICE_ID_REALTEK_8139;
+#endif
+
+    s->eeprom.contents[7] = s->conf.macaddr.a[0] | s->conf.macaddr.a[1] << 8;
+    s->eeprom.contents[8] = s->conf.macaddr.a[2] | s->conf.macaddr.a[3] << 8;
+    s->eeprom.contents[9] = s->conf.macaddr.a[4] | s->conf.macaddr.a[5] << 8;
+
     s->nic = qemu_new_nic(&net_rtl8139_info, &s->conf,
                           dev->qdev.info->name, dev->qdev.id, s);
     qemu_format_nic_info_str(&s->nic->nc, s->conf.macaddr.a);
--
Quoted 1 line

William

hm I just noticed your correction here
http://lists.gnu.org/archive/html/qemu-devel/2011-02/msg03232.html ; sorry
Anyway, I tested my version and it works fine.

Regards,

Signature · 1 line
William
#9 lore raw

[Qemu-devel] Re: [PATCH] moving eeprom initialization

In reply to William Dauchy
Quoted 4 lines · William Dauchy

The initialization should not be only on reset but also when initializing the device.
It resolves a bug when hot plugging a pci network device: the mac address was always null.

Missing Signed-off-by: line.

#10 lore raw

PATCH 1/1 moving eeprom initialization

The initialization should not be only on reset but also when initializing the device.
It resolves a bug when hot plugging a pci network device: the mac address was always null.

---
 hw/pcnet.c   |   27 ++++++++++++++-------------
 hw/rtl8139.c |   24 ++++++++++++------------
 2 files changed, 26 insertions(+), 25 deletions(-)
hw/pcnet.c +14 -13
index db52dc5..4e30e9c 100644
@@ -1557,19 +1557,6 @@ uint32_t pcnet_bcr_readw(PCNetState *s, uint32_t rap)
 void pcnet_h_reset(void *opaque)
 {
     PCNetState *s = opaque;
-    int i;
-    uint16_t checksum;
-
-    /* Initialize the PROM */
-
-    memcpy(s->prom, s->conf.macaddr.a, 6);
-    s->prom[12] = s->prom[13] = 0x00;
-    s->prom[14] = s->prom[15] = 0x57;
-
-    for (i = 0,checksum = 0; i < 16; i++)
-        checksum += s->prom[i];
-    *(uint16_t *)&s->prom[12] = cpu_to_le16(checksum);
-
 
     s->bcr[BCR_MSRDA] = 0x0005;
     s->bcr[BCR_MSWRA] = 0x0005;
@@ -1736,6 +1723,9 @@ void pcnet_common_cleanup(PCNetState *d)
 
 int pcnet_common_init(DeviceState *dev, PCNetState *s, NetClientInfo *info)
 {
+    int i;
+    uint16_t checksum;
+
     s->poll_timer = qemu_new_timer(vm_clock, pcnet_poll_timer, s);
 
     qemu_macaddr_default_if_unset(&s->conf.macaddr);
@@ -1744,5 +1734,16 @@ int pcnet_common_init(DeviceState *dev, PCNetState *s, NetClientInfo *info)
 
     add_boot_device_path(s->conf.bootindex, dev, "/ethernet-phy@0");
 
+    /* Initialize the PROM */
+
+    memcpy(s->prom, s->conf.macaddr.a, 6);
+    s->prom[12] = s->prom[13] = 0x00;
+    s->prom[14] = s->prom[15] = 0x57;
+
+    for (i = 0, checksum = 0; i < 16; i++) {
+        checksum += s->prom[i];
+    }
+    *(uint16_t *)&s->prom[12] = cpu_to_le16(checksum);
+
     return 0;
 }
hw/rtl8139.c +12 -12
index a22530c..8356d5a 100644
@@ -1189,18 +1189,6 @@ static void rtl8139_reset(DeviceState *d)
 
     rtl8139_update_irq(s);
 
-    /* prepare eeprom */
-    s->eeprom.contents[0] = 0x8129;
-#if 1
-    // PCI vendor and device ID should be mirrored here
-    s->eeprom.contents[1] = PCI_VENDOR_ID_REALTEK;
-    s->eeprom.contents[2] = PCI_DEVICE_ID_REALTEK_8139;
-#endif
-
-    s->eeprom.contents[7] = s->conf.macaddr.a[0] | s->conf.macaddr.a[1] << 8;
-    s->eeprom.contents[8] = s->conf.macaddr.a[2] | s->conf.macaddr.a[3] << 8;
-    s->eeprom.contents[9] = s->conf.macaddr.a[4] | s->conf.macaddr.a[5] << 8;
-
     /* mark all status registers as owned by host */
     for (i = 0; i < 4; ++i)
     {
@@ -3392,6 +3380,18 @@ static int pci_rtl8139_init(PCIDevice *dev)
 
     qemu_macaddr_default_if_unset(&s->conf.macaddr);
 
+    /* prepare eeprom */
+    s->eeprom.contents[0] = 0x8129;
+#if 1
+    /* PCI vendor and device ID should be mirrored here */
+    s->eeprom.contents[1] = PCI_VENDOR_ID_REALTEK;
+    s->eeprom.contents[2] = PCI_DEVICE_ID_REALTEK_8139;
+#endif
+
+    s->eeprom.contents[7] = s->conf.macaddr.a[0] | s->conf.macaddr.a[1] << 8;
+    s->eeprom.contents[8] = s->conf.macaddr.a[2] | s->conf.macaddr.a[3] << 8;
+    s->eeprom.contents[9] = s->conf.macaddr.a[4] | s->conf.macaddr.a[5] << 8;
+
     s->nic = qemu_new_nic(&net_rtl8139_info, &s->conf,
                           dev->qdev.info->name, dev->qdev.id, s);
     qemu_format_nic_info_str(&s->nic->nc, s->conf.macaddr.a);
Signature · 1 line
1.7.2.3
#11 lore raw

Re: [Qemu-devel] [PATCH] moving eeprom initialization

Hello,

AMD fix is now upstream. Please merge it.

Thnx.

Ciao,
Gerhard

--
http://www.wiesinger.com/

Quoted 110 lines · William Dauchy

The initialization should not be only on reset but also when initializing the device.
It resolves a bug when hot plugging a pci network device: the mac address was always null.

---

hw/pcnet.c   |   27 ++++++++++++++-------------
hw/rtl8139.c |   24 ++++++++++++------------
2 files changed, 26 insertions(+), 25 deletions(-)
hw/pcnet.c +14 -13
index db52dc5..4e30e9c 100644
@@ -1557,19 +1557,6 @@ uint32_t pcnet_bcr_readw(PCNetState *s, uint32_t rap)
 void pcnet_h_reset(void *opaque)
 {
    PCNetState *s = opaque;
-    int i;
-    uint16_t checksum;
-
-    /* Initialize the PROM */
-
-    memcpy(s->prom, s->conf.macaddr.a, 6);
-    s->prom[12] = s->prom[13] = 0x00;
-    s->prom[14] = s->prom[15] = 0x57;
-
-    for (i = 0,checksum = 0; i < 16; i++)
-        checksum += s->prom[i];
-    *(uint16_t *)&s->prom[12] = cpu_to_le16(checksum);
-
 
    s->bcr[BCR_MSRDA] = 0x0005;
    s->bcr[BCR_MSWRA] = 0x0005;
@@ -1736,6 +1723,9 @@ void pcnet_common_cleanup(PCNetState *d)
 
 int pcnet_common_init(DeviceState *dev, PCNetState *s, NetClientInfo *info)
 {
+    int i;
+    uint16_t checksum;
+
    s->poll_timer = qemu_new_timer(vm_clock, pcnet_poll_timer, s);
 
    qemu_macaddr_default_if_unset(&s->conf.macaddr);
@@ -1744,5 +1734,16 @@ int pcnet_common_init(DeviceState *dev, PCNetState *s, NetClientInfo *info)
 
    add_boot_device_path(s->conf.bootindex, dev, "/ethernet-phy@0");
 
+    /* Initialize the PROM */
+
+    memcpy(s->prom, s->conf.macaddr.a, 6);
+    s->prom[12] = s->prom[13] = 0x00;
+    s->prom[14] = s->prom[15] = 0x57;
+
+    for (i = 0, checksum = 0; i < 16; i++) {
+        checksum += s->prom[i];
+    }
+    *(uint16_t *)&s->prom[12] = cpu_to_le16(checksum);
+
    return 0;
 }
hw/rtl8139.c +12 -12
index a22530c..8356d5a 100644
@@ -1189,18 +1189,6 @@ static void rtl8139_reset(DeviceState *d)
 
    rtl8139_update_irq(s);
 
-    /* prepare eeprom */
-    s->eeprom.contents[0] = 0x8129;
-#if 1
-    // PCI vendor and device ID should be mirrored here
-    s->eeprom.contents[1] = PCI_VENDOR_ID_REALTEK;
-    s->eeprom.contents[2] = PCI_DEVICE_ID_REALTEK_8139;
-#endif
-
-    s->eeprom.contents[7] = s->conf.macaddr.a[0] | s->conf.macaddr.a[1] << 8;
-    s->eeprom.contents[8] = s->conf.macaddr.a[2] | s->conf.macaddr.a[3] << 8;
-    s->eeprom.contents[9] = s->conf.macaddr.a[4] | s->conf.macaddr.a[5] << 8;
-
    /* mark all status registers as owned by host */
    for (i = 0; i < 4; ++i)
    {
@@ -3392,6 +3380,18 @@ static int pci_rtl8139_init(PCIDevice *dev)
 
    qemu_macaddr_default_if_unset(&s->conf.macaddr);
 
+    /* prepare eeprom */
+    s->eeprom.contents[0] = 0x8129;
+#if 1
+    /* PCI vendor and device ID should be mirrored here */
+    s->eeprom.contents[1] = PCI_VENDOR_ID_REALTEK;
+    s->eeprom.contents[2] = PCI_DEVICE_ID_REALTEK_8139;
+#endif
+
+    s->eeprom.contents[7] = s->conf.macaddr.a[0] | s->conf.macaddr.a[1] << 8;
+    s->eeprom.contents[8] = s->conf.macaddr.a[2] | s->conf.macaddr.a[3] << 8;
+    s->eeprom.contents[9] = s->conf.macaddr.a[4] | s->conf.macaddr.a[5] << 8;
+
    s->nic = qemu_new_nic(&net_rtl8139_info, &s->conf,
                          dev->qdev.info->name, dev->qdev.id, s);
    qemu_format_nic_info_str(&s->nic->nc, s->conf.macaddr.a);
Signature · 1 line
1.7.2.3
#12 lore raw

Re: [Qemu-devel] [PATCH] moving eeprom initialization

Quoted 1 line · Gerhard Wiesinger

AMD fix is now upstream. Please merge it.

oh sorry. did not see; will do.

Signature · 1 line
William
#13 lore raw

PATCH 1/1 moving eeprom initialization

In reply to Blue Swirl

The initialization should not be only on reset but also when initializing the device.
It resolves a bug when hot plugging a pci network device: the mac address was always null.

---
 hw/pcnet.c   |   59 +++++++++++++++++++++++++++++----------------------------
 hw/rtl8139.c |   23 ++++++++++-----------
 2 files changed, 41 insertions(+), 41 deletions(-)
hw/pcnet.c +30 -29
index 6dfdcc4..d3d5661 100644
@@ -1557,35 +1557,6 @@ uint32_t pcnet_bcr_readw(PCNetState *s, uint32_t rap)
 void pcnet_h_reset(void *opaque)
 {
     PCNetState *s = opaque;
-    int i;
-    uint16_t checksum;
-
-    /* Initialize the PROM */
-
-    /*
-      Datasheet: http://pdfdata.datasheetsite.com/web/24528/AM79C970A.pdf
-      page 95
-    */
-    memcpy(s->prom, s->conf.macaddr.a, 6);
-    /* Reserved Location: must be 00h */
-    s->prom[6] = s->prom[7] = 0x00;
-    /* Reserved Location: must be 00h */
-    s->prom[8] = 0x00;
-    /* Hardware ID: must be 11h if compatibility to AMD drivers is desired */
-    s->prom[9] = 0x11;
-    /* User programmable space, init with 0 */
-    s->prom[10] = s->prom[11] = 0x00;
-    /* LSByte of two-byte checksum, which is the sum of bytes 00h-0Bh
-       and bytes 0Eh and 0Fh, must therefore be initialized with 0! */
-    s->prom[12] = s->prom[13] = 0x00;
-    /* Must be ASCII W (57h) if compatibility to AMD
-       driver software is desired */
-    s->prom[14] = s->prom[15] = 0x57;
-
-    for (i = 0,checksum = 0; i < 16; i++)
-        checksum += s->prom[i];
-    *(uint16_t *)&s->prom[12] = cpu_to_le16(checksum);
-
 
     s->bcr[BCR_MSRDA] = 0x0005;
     s->bcr[BCR_MSWRA] = 0x0005;
@@ -1752,6 +1723,9 @@ void pcnet_common_cleanup(PCNetState *d)
 
 int pcnet_common_init(DeviceState *dev, PCNetState *s, NetClientInfo *info)
 {
+    int i;
+    uint16_t checksum;
+
     s->poll_timer = qemu_new_timer(vm_clock, pcnet_poll_timer, s);
 
     qemu_macaddr_default_if_unset(&s->conf.macaddr);
@@ -1760,5 +1734,32 @@ int pcnet_common_init(DeviceState *dev, PCNetState *s, NetClientInfo *info)
 
     add_boot_device_path(s->conf.bootindex, dev, "/ethernet-phy@0");
 
+    /* Initialize the PROM */
+
+    /*
+      Datasheet: http://pdfdata.datasheetsite.com/web/24528/AM79C970A.pdf
+      page 95
+    */
+    memcpy(s->prom, s->conf.macaddr.a, 6);
+    /* Reserved Location: must be 00h */
+    s->prom[6] = s->prom[7] = 0x00;
+    /* Reserved Location: must be 00h */
+    s->prom[8] = 0x00;
+    /* Hardware ID: must be 11h if compatibility to AMD drivers is desired */
+    s->prom[9] = 0x11;
+    /* User programmable space, init with 0 */
+    s->prom[10] = s->prom[11] = 0x00;
+    /* LSByte of two-byte checksum, which is the sum of bytes 00h-0Bh
+       and bytes 0Eh and 0Fh, must therefore be initialized with 0! */
+    s->prom[12] = s->prom[13] = 0x00;
+    /* Must be ASCII W (57h) if compatibility to AMD
+       driver software is desired */
+    s->prom[14] = s->prom[15] = 0x57;
+
+    for (i = 0, checksum = 0; i < 16; i++) {
+        checksum += s->prom[i];
+    }
+    *(uint16_t *)&s->prom[12] = cpu_to_le16(checksum);
+
     return 0;
 }
hw/rtl8139.c +11 -12
index a22530c..7a87522 100644
@@ -1189,18 +1189,6 @@ static void rtl8139_reset(DeviceState *d)
 
     rtl8139_update_irq(s);
 
-    /* prepare eeprom */
-    s->eeprom.contents[0] = 0x8129;
-#if 1
-    // PCI vendor and device ID should be mirrored here
-    s->eeprom.contents[1] = PCI_VENDOR_ID_REALTEK;
-    s->eeprom.contents[2] = PCI_DEVICE_ID_REALTEK_8139;
-#endif
-
-    s->eeprom.contents[7] = s->conf.macaddr.a[0] | s->conf.macaddr.a[1] << 8;
-    s->eeprom.contents[8] = s->conf.macaddr.a[2] | s->conf.macaddr.a[3] << 8;
-    s->eeprom.contents[9] = s->conf.macaddr.a[4] | s->conf.macaddr.a[5] << 8;
-
     /* mark all status registers as owned by host */
     for (i = 0; i < 4; ++i)
     {
@@ -3392,6 +3380,17 @@ static int pci_rtl8139_init(PCIDevice *dev)
 
     qemu_macaddr_default_if_unset(&s->conf.macaddr);
 
+    /* prepare eeprom */
+    s->eeprom.contents[0] = 0x8129;
+#if 1
+    /* PCI vendor and device ID should be mirrored here */
+    s->eeprom.contents[1] = PCI_VENDOR_ID_REALTEK;
+    s->eeprom.contents[2] = PCI_DEVICE_ID_REALTEK_8139;
+#endif
+    s->eeprom.contents[7] = s->conf.macaddr.a[0] | s->conf.macaddr.a[1] << 8;
+    s->eeprom.contents[8] = s->conf.macaddr.a[2] | s->conf.macaddr.a[3] << 8;
+    s->eeprom.contents[9] = s->conf.macaddr.a[4] | s->conf.macaddr.a[5] << 8;
+
     s->nic = qemu_new_nic(&net_rtl8139_info, &s->conf,
                           dev->qdev.info->name, dev->qdev.id, s);
     qemu_format_nic_info_str(&s->nic->nc, s->conf.macaddr.a);
Signature · 1 line
1.7.2.3
#14 lore raw

[Qemu-devel] Re: [PATCH] moving eeprom initialization

At 03/07/2011 05:27 AM, William Dauchy Write:

Quoted 7 lines

The initialization should not be only on reset but also when initializing the device.
It resolves a bug when hot plugging a pci network device: the mac address was always null.

Thanks for merging AMD fix.
This patch works fine to me.

Quoted 129 lines
---
 hw/pcnet.c   |   59 +++++++++++++++++++++++++++++----------------------------
 hw/rtl8139.c |   23 ++++++++++-----------
 2 files changed, 41 insertions(+), 41 deletions(-)
hw/pcnet.c +30 -29
index 6dfdcc4..d3d5661 100644
@@ -1557,35 +1557,6 @@ uint32_t pcnet_bcr_readw(PCNetState *s, uint32_t rap)
 void pcnet_h_reset(void *opaque)
 {
     PCNetState *s = opaque;
-    int i;
-    uint16_t checksum;
-
-    /* Initialize the PROM */
-
-    /*
-      Datasheet: http://pdfdata.datasheetsite.com/web/24528/AM79C970A.pdf
-      page 95
-    */
-    memcpy(s->prom, s->conf.macaddr.a, 6);
-    /* Reserved Location: must be 00h */
-    s->prom[6] = s->prom[7] = 0x00;
-    /* Reserved Location: must be 00h */
-    s->prom[8] = 0x00;
-    /* Hardware ID: must be 11h if compatibility to AMD drivers is desired */
-    s->prom[9] = 0x11;
-    /* User programmable space, init with 0 */
-    s->prom[10] = s->prom[11] = 0x00;
-    /* LSByte of two-byte checksum, which is the sum of bytes 00h-0Bh
-       and bytes 0Eh and 0Fh, must therefore be initialized with 0! */
-    s->prom[12] = s->prom[13] = 0x00;
-    /* Must be ASCII W (57h) if compatibility to AMD
-       driver software is desired */
-    s->prom[14] = s->prom[15] = 0x57;
-
-    for (i = 0,checksum = 0; i < 16; i++)
-        checksum += s->prom[i];
-    *(uint16_t *)&s->prom[12] = cpu_to_le16(checksum);
-
 
     s->bcr[BCR_MSRDA] = 0x0005;
     s->bcr[BCR_MSWRA] = 0x0005;
@@ -1752,6 +1723,9 @@ void pcnet_common_cleanup(PCNetState *d)
 
 int pcnet_common_init(DeviceState *dev, PCNetState *s, NetClientInfo *info)
 {
+    int i;
+    uint16_t checksum;
+
     s->poll_timer = qemu_new_timer(vm_clock, pcnet_poll_timer, s);
 
     qemu_macaddr_default_if_unset(&s->conf.macaddr);
@@ -1760,5 +1734,32 @@ int pcnet_common_init(DeviceState *dev, PCNetState *s, NetClientInfo *info)
 
     add_boot_device_path(s->conf.bootindex, dev, "/ethernet-phy@0");
 
+    /* Initialize the PROM */
+
+    /*
+      Datasheet: http://pdfdata.datasheetsite.com/web/24528/AM79C970A.pdf
+      page 95
+    */
+    memcpy(s->prom, s->conf.macaddr.a, 6);
+    /* Reserved Location: must be 00h */
+    s->prom[6] = s->prom[7] = 0x00;
+    /* Reserved Location: must be 00h */
+    s->prom[8] = 0x00;
+    /* Hardware ID: must be 11h if compatibility to AMD drivers is desired */
+    s->prom[9] = 0x11;
+    /* User programmable space, init with 0 */
+    s->prom[10] = s->prom[11] = 0x00;
+    /* LSByte of two-byte checksum, which is the sum of bytes 00h-0Bh
+       and bytes 0Eh and 0Fh, must therefore be initialized with 0! */
+    s->prom[12] = s->prom[13] = 0x00;
+    /* Must be ASCII W (57h) if compatibility to AMD
+       driver software is desired */
+    s->prom[14] = s->prom[15] = 0x57;
+
+    for (i = 0, checksum = 0; i < 16; i++) {
+        checksum += s->prom[i];
+    }
+    *(uint16_t *)&s->prom[12] = cpu_to_le16(checksum);
+
     return 0;
 }
hw/rtl8139.c +11 -12
index a22530c..7a87522 100644
@@ -1189,18 +1189,6 @@ static void rtl8139_reset(DeviceState *d)
 
     rtl8139_update_irq(s);
 
-    /* prepare eeprom */
-    s->eeprom.contents[0] = 0x8129;
-#if 1
-    // PCI vendor and device ID should be mirrored here
-    s->eeprom.contents[1] = PCI_VENDOR_ID_REALTEK;
-    s->eeprom.contents[2] = PCI_DEVICE_ID_REALTEK_8139;
-#endif
-
-    s->eeprom.contents[7] = s->conf.macaddr.a[0] | s->conf.macaddr.a[1] << 8;
-    s->eeprom.contents[8] = s->conf.macaddr.a[2] | s->conf.macaddr.a[3] << 8;
-    s->eeprom.contents[9] = s->conf.macaddr.a[4] | s->conf.macaddr.a[5] << 8;
-
     /* mark all status registers as owned by host */
     for (i = 0; i < 4; ++i)
     {
@@ -3392,6 +3380,17 @@ static int pci_rtl8139_init(PCIDevice *dev)
 
     qemu_macaddr_default_if_unset(&s->conf.macaddr);
 
+    /* prepare eeprom */
+    s->eeprom.contents[0] = 0x8129;
+#if 1
+    /* PCI vendor and device ID should be mirrored here */
+    s->eeprom.contents[1] = PCI_VENDOR_ID_REALTEK;
+    s->eeprom.contents[2] = PCI_DEVICE_ID_REALTEK_8139;
+#endif
+    s->eeprom.contents[7] = s->conf.macaddr.a[0] | s->conf.macaddr.a[1] << 8;
+    s->eeprom.contents[8] = s->conf.macaddr.a[2] | s->conf.macaddr.a[3] << 8;
+    s->eeprom.contents[9] = s->conf.macaddr.a[4] | s->conf.macaddr.a[5] << 8;
+
     s->nic = qemu_new_nic(&net_rtl8139_info, &s->conf,
                           dev->qdev.info->name, dev->qdev.id, s);
     qemu_format_nic_info_str(&s->nic->nc, s->conf.macaddr.a);
Applied
#15 lore raw

[Qemu-devel] Re: [PATCH] moving eeprom initialization

In reply to William Dauchy

Thanks, applied.

Quoted 140 lines · William Dauchy

The initialization should not be only on reset but also when initializing the device.
It resolves a bug when hot plugging a pci network device: the mac address was always null.

---
 hw/pcnet.c   |   59 +++++++++++++++++++++++++++++---------------------------- hw/rtl8139.c |   23 ++++++++++-----------

 2 files changed, 41 insertions(+), 41 deletions(-)
hw/pcnet.c +30 -29
index 6dfdcc4..d3d5661 100644
@@ -1557,35 +1557,6 @@ uint32_t pcnet_bcr_readw(PCNetState *s, uint32_t rap)
  void pcnet_h_reset(void *opaque)
  {
     PCNetState *s = opaque;
-    int i;
-    uint16_t checksum;
-
-    /* Initialize the PROM */
-
-    /*
-      Datasheet: http://pdfdata.datasheetsite.com/web/24528/AM79C970A.pdf
-      page 95
-    */
-    memcpy(s->prom, s->conf.macaddr.a, 6);
-    /* Reserved Location: must be 00h */
-    s->prom[6] = s->prom[7] = 0x00;
-    /* Reserved Location: must be 00h */
-    s->prom[8] = 0x00;
-    /* Hardware ID: must be 11h if compatibility to AMD drivers is desired */
-    s->prom[9] = 0x11;
-    /* User programmable space, init with 0 */
-    s->prom[10] = s->prom[11] = 0x00;
-    /* LSByte of two-byte checksum, which is the sum of bytes 00h-0Bh
-       and bytes 0Eh and 0Fh, must therefore be initialized with 0! */
-    s->prom[12] = s->prom[13] = 0x00;
-    /* Must be ASCII W (57h) if compatibility to AMD
-       driver software is desired */
-    s->prom[14] = s->prom[15] = 0x57;
-
-    for (i = 0,checksum = 0; i < 16; i++)
-        checksum += s->prom[i];
-    *(uint16_t *)&s->prom[12] = cpu_to_le16(checksum);
-
 
     s->bcr[BCR_MSRDA] = 0x0005;
     s->bcr[BCR_MSWRA] = 0x0005;
@@ -1752,6 +1723,9 @@ void pcnet_common_cleanup(PCNetState *d)
 
  int pcnet_common_init(DeviceState *dev, PCNetState *s, NetClientInfo *info)
  {
+    int i;
+    uint16_t checksum;
+
     s->poll_timer = qemu_new_timer(vm_clock, pcnet_poll_timer, s);
 
     qemu_macaddr_default_if_unset(&s->conf.macaddr);
@@ -1760,5 +1734,32 @@ int pcnet_common_init(DeviceState *dev, PCNetState *s, NetClientInfo *info)
 
     add_boot_device_path(s->conf.bootindex, dev, "/ethernet-phy@0");
 
+    /* Initialize the PROM */
+
+    /*
+      Datasheet: http://pdfdata.datasheetsite.com/web/24528/AM79C970A.pdf
+      page 95
+    */
+    memcpy(s->prom, s->conf.macaddr.a, 6);
+    /* Reserved Location: must be 00h */
+    s->prom[6] = s->prom[7] = 0x00;
+    /* Reserved Location: must be 00h */
+    s->prom[8] = 0x00;
+    /* Hardware ID: must be 11h if compatibility to AMD drivers is desired */
+    s->prom[9] = 0x11;
+    /* User programmable space, init with 0 */
+    s->prom[10] = s->prom[11] = 0x00;
+    /* LSByte of two-byte checksum, which is the sum of bytes 00h-0Bh
+       and bytes 0Eh and 0Fh, must therefore be initialized with 0! */
+    s->prom[12] = s->prom[13] = 0x00;
+    /* Must be ASCII W (57h) if compatibility to AMD
+       driver software is desired */
+    s->prom[14] = s->prom[15] = 0x57;
+
+    for (i = 0, checksum = 0; i < 16; i++) {
+        checksum += s->prom[i];
+    }
+    *(uint16_t *)&s->prom[12] = cpu_to_le16(checksum);
+
     return 0;
  }
hw/rtl8139.c +11 -13
index a22530c..7a87522 100644
@@ -1189,18 +1189,6 @@ static void rtl8139_reset(DeviceState *d)
 
     rtl8139_update_irq(s);
 
-    /* prepare eeprom */
-    s->eeprom.contents[0] = 0x8129;
-#if 1
-    // PCI vendor and device ID should be mirrored here
-    s->eeprom.contents[1] = PCI_VENDOR_ID_REALTEK;
-    s->eeprom.contents[2] = PCI_DEVICE_ID_REALTEK_8139;
-#endif
-
-    s->eeprom.contents[7] = s->conf.macaddr.a[0] | s->conf.macaddr.a[1] << 8;
-    s->eeprom.contents[8] = s->conf.macaddr.a[2] | s->conf.macaddr.a[3] << 8;
-    s->eeprom.contents[9] = s->conf.macaddr.a[4] | s->conf.macaddr.a[5] << 8;
-
     /* mark all status registers as owned by host */
     for (i = 0; i < 4; ++i)
     {
@@ -3392,6 +3380,17 @@ static int pci_rtl8139_init(PCIDevice *dev)
 
     qemu_macaddr_default_if_unset(&s->conf.macaddr);
 
+    /* prepare eeprom */
+    s->eeprom.contents[0] = 0x8129;
+#if 1
+    /* PCI vendor and device ID should be mirrored here */
+    s->eeprom.contents[1] = PCI_VENDOR_ID_REALTEK;
+    s->eeprom.contents[2] = PCI_DEVICE_ID_REALTEK_8139;
+#endif
+    s->eeprom.contents[7] = s->conf.macaddr.a[0] | s->conf.macaddr.a[1] << 8;
+    s->eeprom.contents[8] = s->conf.macaddr.a[2] | s->conf.macaddr.a[3] << 8;
+    s->eeprom.contents[9] = s->conf.macaddr.a[4] | s->conf.macaddr.a[5] << 8;
+
     s->nic = qemu_new_nic(&net_rtl8139_info, &s->conf,
                           dev->qdev.info->name, dev->qdev.id, s);
     qemu_format_nic_info_str(&s->nic->nc, s->conf.macaddr.a);
--

1.7.2.3