On Tue, 3 Nov 2026 at 18:57, Stefan Weil <[email protected]> wrote: > > e100_nic_realize() registers a system reset handler via > qemu_register_reset(nic_reset, s), but pci_nic_uninit() never > unregisters it. After the device is hot-unplugged, the LegacyReset > object stored in the root reset container still holds the raw pointer > to the freed EEPRO100State, so the next system reset invokes > nic_reset() on freed memory. > > Instead, define a reset phase on the device itself: assign > phases.hold in eepro100_class_init() to a wrapper around nic_reset(). > The device is already part of the resettable tree via its parent bus > (TYPE_DEVICE implements the Resettable interface), so no explicit > registration is needed, and the reset state is automatically dropped > when the device is unplugged. This follows the approach used by the > e1000, e1000e and igb emulations. > > Reported-by: Hui Peng <[email protected]> > Suggested-by: Peter Maydell <[email protected]> > Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4058 > Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4510 > Signed-off-by: Stefan Weil <[email protected]> > --- > > Note that I used AI assistance to create and test the patch. > I am not shure whether this is acceptable to small and rather mechnical > changes like in this case. > > The Reported-by for issue 4058 was not added because I did not > find the required full name and email of the reporter.
Looking at the git logs it looks like we tend to handle gitlab bug reporters as Reported-by: MyeonghunPak <https://gitlab.com/mhun512> > hw/net/eepro100.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c > index aaa3eb60f8a..e1cd8346941 100644 > --- a/hw/net/eepro100.c > +++ b/hw/net/eepro100.c > @@ -617,6 +617,13 @@ static void nic_reset(void *opaque) > nic_selective_reset(s); > } > > +static void eepro100_reset_hold(Object *obj, ResetType type) > +{ > + EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, PCI_DEVICE(obj)); Why the DO_UPCAST macro here rather than a standard QOM cast? > + > + nic_reset(s); > +} > + > #if defined(DEBUG_EEPRO100) > static const char * const e100_reg[PCI_IO_SIZE / 4] = { > "Command/Status", > @@ -1881,8 +1888,6 @@ static void e100_nic_realize(PCIDevice *pci_dev, Error > **errp) > qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a); > TRACE(OTHER, logout("%s\n", qemu_get_queue(s->nic)->info_str)); > > - qemu_register_reset(nic_reset, s); > - > s->vmstate = g_memdup(&vmstate_eepro100, sizeof(vmstate_eepro100)); > s->vmstate->name = qemu_get_queue(s->nic)->model; > vmstate_register_any(VMSTATE_IF(&pci_dev->qdev), s->vmstate, s); There's also a call to nic_reset() lurking in the realize function, which should be removed. > @@ -2064,6 +2069,7 @@ static void eepro100_class_init(ObjectClass *klass, > const void *data) > { > DeviceClass *dc = DEVICE_CLASS(klass); > PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); > + ResettableClass *rc = RESETTABLE_CLASS(klass); > E100PCIDeviceInfo *info; > > info = eepro100_get_class_by_name(object_class_get_name(klass)); > @@ -2080,6 +2086,7 @@ static void eepro100_class_init(ObjectClass *klass, > const void *data) > k->revision = info->revision; > k->subsystem_vendor_id = info->subsystem_vendor_id; > k->subsystem_id = info->subsystem_id; > + rc->phases.hold = eepro100_reset_hold; > } thanks -- PMM
