On 2024/02/12 17:35, Philippe Mathieu-Daudé wrote:
Hi,QDev base class doesn't expect UNREALIZE to fail, and this handler is only recommended for hot-plug devices: /** * qdev_unrealize: Unrealize a device * @dev: device to unrealize * * Warning: most devices in QEMU do not expect to be unrealized. Only * devices which are hot-unpluggable should be unrealized (as part of * the unplugging process); all other devices are expected to last for * the life of the simulation and should not be unrealized and freed. */ void qdev_unrealize(DeviceState *dev) { object_property_set_bool(OBJECT(dev), "realized", false, &error_abort); ^^^^^^^^^^^^ } static void device_unparent(Object *obj) { DeviceState *dev = DEVICE(obj); BusState *bus; if (dev->realized) { qdev_unrealize(dev); } while (dev->num_child_bus) { bus = QLIST_FIRST(&dev->child_bus); object_unparent(OBJECT(bus)); } if (dev->parent_bus) { bus_remove_child(dev->parent_bus, dev); object_unref(OBJECT(dev->parent_bus)); dev->parent_bus = NULL; } } Now apparently some devices expect failures, see commit 7c0fa8dff8 ("pcie: Add support for Single Root I/O Virtualization (SR/IOV)"): static void unregister_vfs(PCIDevice *dev) { uint16_t num_vfs = dev->exp.sriov_pf.num_vfs; uint16_t i; for (i = 0; i < num_vfs; i++) { Error *err = NULL; PCIDevice *vf = dev->exp.sriov_pf.vf[i]; if (!object_property_set_bool(OBJECT(vf), "realized", false, &err)) { ^^^^ error_reportf_err(err, "Failed to unplug: "); } object_unparent(OBJECT(vf)); object_unref(OBJECT(vf)); } ... } (Note the failure path only emits a warning). So instead of calling the QDev unrealize layer, this function is calling the lower layer, QOM, bypassing the class handlers, leading to further cleanups such commit 08f6328480 ("pcie: Release references of virtual functions") or recent patch https://lore.kernel.org/qemu-devel/20240210-reuse-v2-6-24ba2a502...@daynix.com/. I couldn't find any explicit possible failure in: pci_qdev_unrealize() do_pci_unregister_device() pci_bus_unrealize() so, what is the failure unregister_vfs() is trying to recover from?
When unrealizing, device_set_realized() is only used to report that the device is not hotpluggable.
I understand if a device is in a odd state, the kernel could reject an unplug request. If so, how to deal with that cleanly?
The guest kernel requests to unregister VFs so QEMU shouldn't ask it if they should be unplugged again. I think that's why unrealize_vfs() doesn't use qdev_unplug().
In any case, with "[PATCH v2 5/6] pcie_sriov: Reuse SR-IOV VF device instances", the runtime realization/unrealization code is replaced with PCI-specific device enablement code, which derives from PCI power down logic. While the patch was written to deal with realization errors, it also eliminates the need to unrealize VFs at runtime.
Regards, Akihiko Odaki
