| Message ID | 20100802211121.5497.36512.stgit@localhost6.localdomain6 |
|---|---|
| State | New |
| Headers | show |
rtl8139 has the same problem, except there's a much bigger pile of code in rtl8139_reset()? I think maybe we need to revisit this wholesale remove of reset calls from init functions, unless I'm missing how hotplug is supposed to work. Thanks, Alex On Mon, 2010-08-02 at 15:15 -0600, Alex Williamson wrote: > When we removed the call to e1000_reset() back in cset c1699988, we > left some register state uninitialized. When we hotplug the device, > we don't go through a reset cycle, which means a hot added e1000 is > useless until the VM reboots. Duplicate the bits we need from > e1000_reset(). > > Signed-off-by: Alex Williamson <alex.williamson@redhat.com> > --- > > 0.13 candidate? > > hw/e1000.c | 4 ++++ > 1 files changed, 4 insertions(+), 0 deletions(-) > > diff --git a/hw/e1000.c b/hw/e1000.c > index 80b78bc..eb323d2 100644 > --- a/hw/e1000.c > +++ b/hw/e1000.c > @@ -1129,6 +1129,10 @@ static int pci_e1000_init(PCIDevice *pci_dev) > checksum = (uint16_t) EEPROM_SUM - checksum; > d->eeprom_data[EEPROM_CHECKSUM_REG] = checksum; > > + memmove(d->phy_reg, phy_reg_init, sizeof phy_reg_init); > + memmove(d->mac_reg, mac_reg_init, sizeof mac_reg_init); > + d->rxbuf_min_shift = 1; > + > d->nic = qemu_new_nic(&net_e1000_info, &d->conf, > d->dev.qdev.info->name, d->dev.qdev.id, d); > >
On Mon, Aug 02, 2010 at 03:15:17PM -0600, Alex Williamson wrote: > When we hotplug the device, > we don't go through a reset cycle, which means a hot added e1000 is > useless until the VM reboots. I do guess, however, that this is true for any device, right? Wouldn't it be better to just call the newly added reset function at hotplug? One way to do that, would be to store a value indicated qemu has already started. If you add a reset handler after that, the function is called before being placed on the list.
On Tue, 2010-08-03 at 08:17 -0400, Glauber Costa wrote: > On Mon, Aug 02, 2010 at 03:15:17PM -0600, Alex Williamson wrote: > > When we hotplug the device, > > we don't go through a reset cycle, which means a hot added e1000 is > > useless until the VM reboots. > > I do guess, however, that this is true for any device, right? > > Wouldn't it be better to just call the newly added reset function at > hotplug? One way to do that, would be to store a value indicated qemu > has already started. If you add a reset handler after that, the function > is called before being placed on the list. Yeah, that sounds like a better idea. We can actually do it quite easily from qdev_init using the hotplugged flag on the DeviceState. Drop this e1000 specific one, I'll send a new patch in a minute. Thanks, Alex
diff --git a/hw/e1000.c b/hw/e1000.c index 80b78bc..eb323d2 100644 --- a/hw/e1000.c +++ b/hw/e1000.c @@ -1129,6 +1129,10 @@ static int pci_e1000_init(PCIDevice *pci_dev) checksum = (uint16_t) EEPROM_SUM - checksum; d->eeprom_data[EEPROM_CHECKSUM_REG] = checksum; + memmove(d->phy_reg, phy_reg_init, sizeof phy_reg_init); + memmove(d->mac_reg, mac_reg_init, sizeof mac_reg_init); + d->rxbuf_min_shift = 1; + d->nic = qemu_new_nic(&net_e1000_info, &d->conf, d->dev.qdev.info->name, d->dev.qdev.id, d);
When we removed the call to e1000_reset() back in cset c1699988, we left some register state uninitialized. When we hotplug the device, we don't go through a reset cycle, which means a hot added e1000 is useless until the VM reboots. Duplicate the bits we need from e1000_reset(). Signed-off-by: Alex Williamson <alex.williamson@redhat.com> --- 0.13 candidate? hw/e1000.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)