[Qemu-devel] null mac address

[Qemu-devel] null mac address

[William Dauchy]

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,
-- 
William

Re: [Qemu-devel] null mac address

[Wen Congyang]

At 02/24/2011 10:40 PM, William Dauchy Write:
> 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.

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

> 
> Regards,

Re: [Qemu-devel] null mac address

[Wen Congyang]

At 02/24/2011 10:40 PM, William Dauchy Write:
> 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.

Please try the following patch.

Thanks
Wen Congyang

>From efa0632f563a69dc299daaf4b235c1a0521d6e02 Mon Sep 17 00:00:00 2001
From: Wen Congyang <wency@cn.fujitsu.com>
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(-)

diff --git a/hw/pcnet-pci.c b/hw/pcnet-pci.c
index 339a401..d7c4fc3 100644
--- a/hw/pcnet-pci.c
+++ b/hw/pcnet-pci.c
@@ -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,
diff --git a/hw/pcnet.c b/hw/pcnet.c
index db52dc5..283828a 100644
--- a/hw/pcnet.c
+++ b/hw/pcnet.c
@@ -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;
diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index a22530c..580c83e 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -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);
-- 
1.7.1


> 
> Regards,

Re: [Qemu-devel] null mac address

[Blue Swirl]

On Fri, Feb 25, 2011 at 4:55 AM, Wen Congyang <wency@cn.fujitsu.com> wrote:
> At 02/24/2011 10:40 PM, William Dauchy Write:
>> 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.
>
> Please try the following patch.
>
> Thanks
> Wen Congyang
>
> From efa0632f563a69dc299daaf4b235c1a0521d6e02 Mon Sep 17 00:00:00 2001
> From: Wen Congyang <wency@cn.fujitsu.com>
> 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(-)
>
> diff --git a/hw/pcnet-pci.c b/hw/pcnet-pci.c
> index 339a401..d7c4fc3 100644
> --- a/hw/pcnet-pci.c
> +++ b/hw/pcnet-pci.c
> @@ -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++)

Please add braces to fix the CODING_STYLE problem while moving.

> +        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().

> +    // PCI vendor and device ID should be mirrored here

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

[Qemu-devel] [PATCH v2] move eeprom init from reset function to init function

[Wen Congyang]

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.

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>

---
 hw/pcnet.c   |   27 ++++++++++++++-------------
 hw/rtl8139.c |   24 ++++++++++++------------
 2 files changed, 26 insertions(+), 25 deletions(-)

diff --git a/hw/pcnet.c b/hw/pcnet.c
index db52dc5..4499031 100644
--- a/hw/pcnet.c
+++ b/hw/pcnet.c
@@ -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);
diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index a22530c..8356d5a 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -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);
-- 
1.7.1

[Qemu-devel] [PATCH] moving eeprom initialization

[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(-)

diff --git a/hw/pcnet.c b/hw/pcnet.c
index db52dc5..4e30e9c 100644
--- a/hw/pcnet.c
+++ b/hw/pcnet.c
@@ -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;
 }
diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index a22530c..8356d5a 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -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);
-- 
William

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

[William Dauchy]

On Wed, Mar 2, 2011 at 2:36 PM, William Dauchy <wdauchy@gmail.com> wrote:
> 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(-)
>
> diff --git a/hw/pcnet.c b/hw/pcnet.c
> index db52dc5..4e30e9c 100644
> --- a/hw/pcnet.c
> +++ b/hw/pcnet.c
> @@ -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;
>  }
> diff --git a/hw/rtl8139.c b/hw/rtl8139.c
> index a22530c..8356d5a 100644
> --- a/hw/rtl8139.c
> +++ b/hw/rtl8139.c
> @@ -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);
> --
> 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,

-- 
William

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

[Gerhard Wiesinger]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 4305 bytes --]

Hello,

Your patch should be based on fixes for correct EEPROM initialization, 
see for details: 
http://www.mail-archive.com/qemu-devel@nongnu.org/msg56414.html

Ciao,
Gerhard

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


On Wed, 2 Mar 2011, William Dauchy wrote:

> On Wed, Mar 2, 2011 at 2:36 PM, William Dauchy <wdauchy@gmail.com> wrote:
>> 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(-)
>>
>> diff --git a/hw/pcnet.c b/hw/pcnet.c
>> index db52dc5..4e30e9c 100644
>> --- a/hw/pcnet.c
>> +++ b/hw/pcnet.c
>> @@ -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;
>>  }
>> diff --git a/hw/rtl8139.c b/hw/rtl8139.c
>> index a22530c..8356d5a 100644
>> --- a/hw/rtl8139.c
>> +++ b/hw/rtl8139.c
>> @@ -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);
>> --
>> 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,
>
> -- 
> William
>
>
>

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

[William Dauchy]

On Wed, Mar 2, 2011 at 7:28 PM, Gerhard Wiesinger <lists@wiesinger.com> wrote:
> Your patch should be based on fixes for correct EEPROM initialization, see
> for details: http://www.mail-archive.com/qemu-devel@nongnu.org/msg56414.html

This patch is not yet integrated upstream. I will correct it if needed.

-- 
William

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

[Blue Swirl]

On Wed, Mar 2, 2011 at 3:36 PM, William Dauchy <wdauchy@gmail.com> wrote:
> 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.

[Qemu-devel] [PATCH] moving eeprom initialization

[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.

Signed-off-by: William Dauchy <wdauchy@gmail.com>
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
 hw/pcnet.c   |   27 ++++++++++++++-------------
 hw/rtl8139.c |   24 ++++++++++++------------
 2 files changed, 26 insertions(+), 25 deletions(-)

diff --git a/hw/pcnet.c b/hw/pcnet.c
index db52dc5..4e30e9c 100644
--- a/hw/pcnet.c
+++ b/hw/pcnet.c
@@ -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;
 }
diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index a22530c..8356d5a 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -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);
-- 
1.7.2.3

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

[Gerhard Wiesinger]

Hello,

AMD fix is now upstream. Please merge it.

Thnx.

Ciao,
Gerhard

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


On Sun, 6 Mar 2011, William Dauchy wrote:

> 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.
>
> Signed-off-by: William Dauchy <wdauchy@gmail.com>
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> ---
> hw/pcnet.c   |   27 ++++++++++++++-------------
> hw/rtl8139.c |   24 ++++++++++++------------
> 2 files changed, 26 insertions(+), 25 deletions(-)
>
> diff --git a/hw/pcnet.c b/hw/pcnet.c
> index db52dc5..4e30e9c 100644
> --- a/hw/pcnet.c
> +++ b/hw/pcnet.c
> @@ -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;
> }
> diff --git a/hw/rtl8139.c b/hw/rtl8139.c
> index a22530c..8356d5a 100644
> --- a/hw/rtl8139.c
> +++ b/hw/rtl8139.c
> @@ -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);
> -- 
> 1.7.2.3
>
>
>

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

[William Dauchy]

On Sun, Mar 6, 2011 at 9:38 PM, Gerhard Wiesinger <lists@wiesinger.com> wrote:
> AMD fix is now upstream. Please merge it.

oh sorry. did not see; will do.

-- 
William

[Qemu-devel] [PATCH] moving eeprom initialization

[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.

Signed-off-by: William Dauchy <wdauchy@gmail.com>
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
 hw/pcnet.c   |   59 +++++++++++++++++++++++++++++----------------------------
 hw/rtl8139.c |   23 ++++++++++-----------
 2 files changed, 41 insertions(+), 41 deletions(-)

diff --git a/hw/pcnet.c b/hw/pcnet.c
index 6dfdcc4..d3d5661 100644
--- a/hw/pcnet.c
+++ b/hw/pcnet.c
@@ -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;
 }
diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index a22530c..7a87522 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -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

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

[Wen Congyang]

At 03/07/2011 05:27 AM, William Dauchy Write:
> 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.
> 
> Signed-off-by: William Dauchy <wdauchy@gmail.com>
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>

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

> ---
>  hw/pcnet.c   |   59 +++++++++++++++++++++++++++++----------------------------
>  hw/rtl8139.c |   23 ++++++++++-----------
>  2 files changed, 41 insertions(+), 41 deletions(-)
> 
> diff --git a/hw/pcnet.c b/hw/pcnet.c
> index 6dfdcc4..d3d5661 100644
> --- a/hw/pcnet.c
> +++ b/hw/pcnet.c
> @@ -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;
>  }
> diff --git a/hw/rtl8139.c b/hw/rtl8139.c
> index a22530c..7a87522 100644
> --- a/hw/rtl8139.c
> +++ b/hw/rtl8139.c
> @@ -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);

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

[Blue Swirl]

Thanks, applied.

On Sun, Mar 6, 2011 at 11:27 PM, William Dauchy <wdauchy@gmail.com> wrote:
> 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.
>
> Signed-off-by: William Dauchy <wdauchy@gmail.com>
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> ---
>  hw/pcnet.c   |   59 +++++++++++++++++++++++++++++----------------------------
>  hw/rtl8139.c |   23 ++++++++++-----------
>  2 files changed, 41 insertions(+), 41 deletions(-)
>
> diff --git a/hw/pcnet.c b/hw/pcnet.c
> index 6dfdcc4..d3d5661 100644
> --- a/hw/pcnet.c
> +++ b/hw/pcnet.c
> @@ -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;
>  }
> diff --git a/hw/rtl8139.c b/hw/rtl8139.c
> index a22530c..7a87522 100644
> --- a/hw/rtl8139.c
> +++ b/hw/rtl8139.c
> @@ -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
>
>