| Message ID | 1378211609-16121-18-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Sep 03, 2013 at 02:33:08PM +0200, Paolo Bonzini wrote: > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > hw/net/e1000.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/hw/net/e1000.c b/hw/net/e1000.c > index f5ebed4..55fb062 100644 > --- a/hw/net/e1000.c > +++ b/hw/net/e1000.c > @@ -1310,9 +1310,9 @@ e1000_cleanup(NetClientState *nc) > } > > static void > -pci_e1000_uninit(PCIDevice *dev) > +pci_e1000_instance_finalize(Object *obj) > { > - E1000State *d = E1000(dev); > + E1000State *d = E1000(obj); > > timer_del(d->autoneg_timer); > timer_free(d->autoneg_timer); So this looks wrong. This cancels timers after pci device has been destroyed, so meanwhile timers can run and send interrupts. > @@ -1394,7 +1394,6 @@ static void e1000_class_init(ObjectClass *klass, void *data) > PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); > > k->init = pci_e1000_init; > - k->exit = pci_e1000_uninit; > k->romfile = "efi-e1000.rom"; > k->vendor_id = PCI_VENDOR_ID_INTEL; > k->device_id = E1000_DEVID; > @@ -1412,6 +1411,7 @@ static const TypeInfo e1000_info = { > .parent = TYPE_PCI_DEVICE, > .instance_size = sizeof(E1000State), > .class_init = e1000_class_init, > + .instance_finalize = pci_e1000_instance_finalize, > }; > > static void e1000_register_types(void) > -- > 1.8.3.1 >
Il 17/09/2013 11:27, Michael S. Tsirkin ha scritto: >> > static void >> > -pci_e1000_uninit(PCIDevice *dev) >> > +pci_e1000_instance_finalize(Object *obj) >> > { >> > - E1000State *d = E1000(dev); >> > + E1000State *d = E1000(obj); >> > >> > timer_del(d->autoneg_timer); >> > timer_free(d->autoneg_timer); > So this looks wrong. > This cancels timers after pci device has been destroyed, > so meanwhile timers can run and send interrupts. There are definitely cases where the timer deals with pending I/O and has to run after the device has been removed from guest access. This is _not_ yet the point of destruction; the connection to the host backend still exists in particular (it is only dropped by object_property_del_all, which is called right after instance_finalize). It should not be a problem for a device to raise an interrupt after pci_do_unregister_device; it should go nowhere. If it is passed to the guest, it's a bug that we have to fix. Paolo
diff --git a/hw/net/e1000.c b/hw/net/e1000.c index f5ebed4..55fb062 100644 --- a/hw/net/e1000.c +++ b/hw/net/e1000.c @@ -1310,9 +1310,9 @@ e1000_cleanup(NetClientState *nc) } static void -pci_e1000_uninit(PCIDevice *dev) +pci_e1000_instance_finalize(Object *obj) { - E1000State *d = E1000(dev); + E1000State *d = E1000(obj); timer_del(d->autoneg_timer); timer_free(d->autoneg_timer); @@ -1394,7 +1394,6 @@ static void e1000_class_init(ObjectClass *klass, void *data) PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); k->init = pci_e1000_init; - k->exit = pci_e1000_uninit; k->romfile = "efi-e1000.rom"; k->vendor_id = PCI_VENDOR_ID_INTEL; k->device_id = E1000_DEVID; @@ -1412,6 +1411,7 @@ static const TypeInfo e1000_info = { .parent = TYPE_PCI_DEVICE, .instance_size = sizeof(E1000State), .class_init = e1000_class_init, + .instance_finalize = pci_e1000_instance_finalize, }; static void e1000_register_types(void)
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- hw/net/e1000.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)