[v4,06/14] vfio-user: find and init PCI device

Commit Message

Jag Raman Dec. 15, 2021, 3:35 p.m. UTC

Find the PCI device with specified id. Initialize the device context
with the QEMU PCI device

Signed-off-by: Elena Ufimtseva <[e-mail]>
Signed-off-by: John G Johnson <[e-mail]>
Signed-off-by: Jagannathan Raman <[e-mail]>
---
 hw/remote/vfio-user-obj.c | 41 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

Comments

Stefan Hajnoczi Dec. 16, 2021, 10:39 a.m. UTC | #1

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

Jag Raman Dec. 17, 2021, 3:12 a.m. UTC | #2

> On Dec 16, 2021, at 5:39 AM, Stefan Hajnoczi <[e-mail]> 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

Patch

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);
     }