Message ID | 06d8cd0be786fb6786d42c9251b37094bff813a0.1639549843.git.jag.raman@oracle.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | vfio-user server in QEMU | expand |
On Wed, Dec 15, 2021 at 10:35:30AM -0500, Jagannathan Raman wrote: > @@ -150,6 +157,38 @@ static void vfu_object_init_ctx(VfuObject *o, Error **errp) > + o->pci_dev = PCI_DEVICE(dev); ... > @@ -190,6 +229,8 @@ static void vfu_object_finalize(Object *obj) > > o->device = NULL; > > + o->pci_dev = NULL; We need to consider how this interacts with device_del hot unplug. o->pci_dev is a pointer and we don't hold a refcount, so I think o->pci_dev could disappear at any moment. A pair of object_ref/unref(OBJECT(o->pci_dev)) calls would not be enough because device_del will still unrealize the device that's in use by the vfio-user server. I suggest adding a check to qdev_unplug() that prevents unplug when the device is in use by the vfio-user server. That's similar to the code in that function for preventing unplug during migration. One way to do that is by adding a new API: /* * Register an Error that is raised when unplug is attempted on a * device. If another blocker has already been registered then that * Error may be raised during unplug instead. * * qdev_del_unplug_blocker() must be called to remove this blocker. */ void qdev_add_unplug_blocker(DeviceState *dev, Error *blocker); /* * Deregister an Error that was previously registered with * qdev_add_unplug_blocker(). */ void qdev_del_unplug_blocker(DeviceState *dev, Error *blocker); The vfio-user server then needs to add an Error *unplug_blocker field to VfuObject and call qdev_add/del_unplug_blocker() on the PCI device. From a user perspective this means that device_del fails with "Device currently in use by vfio-user server '%s'". Stefan
> On Dec 16, 2021, at 5:39 AM, Stefan Hajnoczi <stefanha@redhat.com> wrote: > > On Wed, Dec 15, 2021 at 10:35:30AM -0500, Jagannathan Raman wrote: >> @@ -150,6 +157,38 @@ static void vfu_object_init_ctx(VfuObject *o, Error **errp) >> + o->pci_dev = PCI_DEVICE(dev); > ... >> @@ -190,6 +229,8 @@ static void vfu_object_finalize(Object *obj) >> >> o->device = NULL; >> >> + o->pci_dev = NULL; > > We need to consider how this interacts with device_del hot unplug. > o->pci_dev is a pointer and we don't hold a refcount, so I think > o->pci_dev could disappear at any moment. > > A pair of object_ref/unref(OBJECT(o->pci_dev)) calls would not be enough > because device_del will still unrealize the device that's in use by the > vfio-user server. > > I suggest adding a check to qdev_unplug() that prevents unplug when the > device is in use by the vfio-user server. That's similar to the code in > that function for preventing unplug during migration. > > One way to do that is by adding a new API: > > /* > * Register an Error that is raised when unplug is attempted on a > * device. If another blocker has already been registered then that > * Error may be raised during unplug instead. > * > * qdev_del_unplug_blocker() must be called to remove this blocker. > */ > void qdev_add_unplug_blocker(DeviceState *dev, Error *blocker); > > /* > * Deregister an Error that was previously registered with > * qdev_add_unplug_blocker(). > */ > void qdev_del_unplug_blocker(DeviceState *dev, Error *blocker); > > The vfio-user server then needs to add an Error *unplug_blocker field to > VfuObject and call qdev_add/del_unplug_blocker() on the PCI device. > > From a user perspective this means that device_del fails with "Device > currently in use by vfio-user server '%s'". OK sounds good, will add the above unplug blocker API for PCI devices. Thank you! -- Jag > > Stefan
diff --git a/hw/remote/vfio-user-obj.c b/hw/remote/vfio-user-obj.c index f439b81787..bcbea59bf1 100644 --- a/hw/remote/vfio-user-obj.c +++ b/hw/remote/vfio-user-obj.c @@ -44,6 +44,8 @@ #include "qemu/notify.h" #include "sysemu/sysemu.h" #include "libvfio-user.h" +#include "hw/qdev-core.h" +#include "hw/pci/pci.h" #define TYPE_VFU_OBJECT "x-vfio-user-server" OBJECT_DECLARE_TYPE(VfuObject, VfuObjectClass, VFU_OBJECT) @@ -69,6 +71,8 @@ struct VfuObject { Notifier machine_done; vfu_ctx_t *vfu_ctx; + + PCIDevice *pci_dev; }; static void vfu_object_init_ctx(VfuObject *o, Error **errp); @@ -133,6 +137,9 @@ static void vfu_object_machine_done(Notifier *notifier, void *data) static void vfu_object_init_ctx(VfuObject *o, Error **errp) { ERRP_GUARD(); + DeviceState *dev = NULL; + vfu_pci_type_t pci_type = VFU_PCI_TYPE_CONVENTIONAL; + int ret; if (o->vfu_ctx || !o->socket || !o->device || !phase_check(PHASE_MACHINE_READY)) { @@ -150,6 +157,38 @@ static void vfu_object_init_ctx(VfuObject *o, Error **errp) error_setg(errp, "vfu: Failed to create context - %s", strerror(errno)); return; } + + dev = qdev_find_recursive(sysbus_get_default(), o->device); + if (dev == NULL) { + error_setg(errp, "vfu: Device %s not found", o->device); + goto fail; + } + + if (!object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { + error_setg(errp, "vfu: %s not a PCI device", o->device); + goto fail; + } + + o->pci_dev = PCI_DEVICE(dev); + + if (pci_is_express(o->pci_dev)) { + pci_type = VFU_PCI_TYPE_EXPRESS; + } + + ret = vfu_pci_init(o->vfu_ctx, pci_type, PCI_HEADER_TYPE_NORMAL, 0); + if (ret < 0) { + error_setg(errp, + "vfu: Failed to attach PCI device %s to context - %s", + o->device, strerror(errno)); + goto fail; + } + + return; + +fail: + vfu_destroy_ctx(o->vfu_ctx); + o->vfu_ctx = NULL; + o->pci_dev = NULL; } static void vfu_object_init(Object *obj) @@ -190,6 +229,8 @@ static void vfu_object_finalize(Object *obj) o->device = NULL; + o->pci_dev = NULL; + if (!k->nr_devs && !k->daemon) { qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN); }