Re: [PATCH v2 05/29] vfio: export PCI helpers needed for vfio-user

[Cédric Le Goater]

On 5/20/25 17:03, John Levon wrote:

The vfio-user code will need to re-use various parts of the vfio PCI
code. Export them in hw/vfio/pci.h, and rename them to the vfio_pci_*
namespace.

Signed-off-by: John Levon <john.le...@nutanix.com>
---
  hw/vfio/pci.h        | 14 ++++++++++++
  hw/vfio/pci.c        | 54 ++++++++++++++++++++++----------------------
  hw/vfio/trace-events |  6 ++---
  3 files changed, 44 insertions(+), 30 deletions(-)

diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
index 5ce0fb916f..61fa385ddb 100644
--- a/hw/vfio/pci.h
+++ b/hw/vfio/pci.h
@@ -248,4 +248,18 @@ void vfio_display_finalize(VFIOPCIDevice *vdev);

extern const VMStateDescription vfio_display_vmstate; +void vfio_pci_bars_exit(VFIOPCIDevice *vdev);

+bool vfio_pci_add_capabilities(VFIOPCIDevice *vdev, Error **errp);
+bool vfio_pci_config_setup(VFIOPCIDevice *vdev, Error **errp);
+Object *vfio_pci_get_object(VFIODevice *vbasedev);
+bool vfio_pci_interrupt_setup(VFIOPCIDevice *vdev, Error **errp);
+void vfio_pci_intx_eoi(VFIODevice *vbasedev);
+int vfio_pci_load_config(VFIODevice *vbasedev, QEMUFile *f);
+void vfio_pci_put_device(VFIOPCIDevice *vdev);
+bool vfio_pci_populate_device(VFIOPCIDevice *vdev, Error **errp);
+void vfio_pci_register_err_notifier(VFIOPCIDevice *vdev);
+void vfio_pci_register_req_notifier(VFIOPCIDevice *vdev);
+int vfio_pci_save_config(VFIODevice *vbasedev, QEMUFile *f, Error **errp);
+void vfio_pci_teardown_msi(VFIOPCIDevice *vdev);
+

The rename is a good thing. There are more to be done.

One comment,

These routines :

  Object *vfio_pci_get_object(VFIODevice *vbasedev);
  void vfio_pci_intx_eoi(VFIODevice *vbasedev);
  int vfio_pci_load_config(VFIODevice *vbasedev, QEMUFile *f);
  int vfio_pci_save_config(VFIODevice *vbasedev, QEMUFile *f, Error **errp);

are exported to simply define the VFIODeviceOps vfio_user_pci_ops struct
(and they don't take a VFIOPCIDevice as first argument). I wonder if we
could find a way to keep them private.

Also, does vfio-user support migration ? if not then, we only need to
export vfio_pci_intx_eoi()


Thanks,

C.

  #endif /* HW_VFIO_VFIO_PCI_H */
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index b1250d85bf..5159198bdb 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -103,7 +103,7 @@ static void vfio_intx_interrupt(void *opaque)
      }
  }

-static void vfio_intx_eoi(VFIODevice *vbasedev)

+void vfio_pci_intx_eoi(VFIODevice *vbasedev)
  {
      VFIOPCIDevice *vdev = container_of(vbasedev, VFIOPCIDevice, vbasedev);

@@ -111,7 +111,7 @@ static void vfio_intx_eoi(VFIODevice *vbasedev)

          return;
      }

- trace_vfio_intx_eoi(vbasedev->name);

+    trace_vfio_pci_intx_eoi(vbasedev->name);

vdev->intx.pending = false;

      pci_irq_deassert(&vdev->pdev);
@@ -236,7 +236,7 @@ static void vfio_intx_update(VFIOPCIDevice *vdev, 
PCIINTxRoute *route)
      }

/* Re-enable the interrupt in cased we missed an EOI */

-    vfio_intx_eoi(&vdev->vbasedev);
+    vfio_pci_intx_eoi(&vdev->vbasedev);
  }

static void vfio_intx_routing_notifier(PCIDevice *pdev)

@@ -1743,7 +1743,7 @@ static bool vfio_msix_setup(VFIOPCIDevice *vdev, int pos, 
Error **errp)
      return true;
  }

-static void vfio_teardown_msi(VFIOPCIDevice *vdev)

+void vfio_pci_teardown_msi(VFIOPCIDevice *vdev)
  {
      msi_uninit(&vdev->pdev);

@@ -1839,7 +1839,7 @@ static void vfio_bars_register(VFIOPCIDevice *vdev)

      }
  }

-static void vfio_bars_exit(VFIOPCIDevice *vdev)

+void vfio_pci_bars_exit(VFIOPCIDevice *vdev)
  {
      int i;

@@ -2430,7 +2430,7 @@ static void vfio_add_ext_cap(VFIOPCIDevice *vdev)

      g_free(config);
  }

-static bool vfio_add_capabilities(VFIOPCIDevice *vdev, Error **errp)

+bool vfio_pci_add_capabilities(VFIOPCIDevice *vdev, Error **errp)
  {
      PCIDevice *pdev = &vdev->pdev;

@@ -2599,7 +2599,7 @@ static void vfio_pci_compute_needs_reset(VFIODevice *vbasedev)

      }
  }

-static Object *vfio_pci_get_object(VFIODevice *vbasedev)

+Object *vfio_pci_get_object(VFIODevice *vbasedev)
  {
      VFIOPCIDevice *vdev = container_of(vbasedev, VFIOPCIDevice, vbasedev);

@@ -2655,7 +2655,7 @@ static const VMStateDescription vmstate_vfio_pci_config = {

      }
  };

-static int vfio_pci_save_config(VFIODevice *vbasedev, QEMUFile *f, Error **errp)

+int vfio_pci_save_config(VFIODevice *vbasedev, QEMUFile *f, Error **errp)
  {
      VFIOPCIDevice *vdev = container_of(vbasedev, VFIOPCIDevice, vbasedev);

@@ -2663,7 +2663,7 @@ static int vfio_pci_save_config(VFIODevice *vbasedev, QEMUFile *f, Error **errp)

                                         errp);
  }

-static int vfio_pci_load_config(VFIODevice *vbasedev, QEMUFile *f)

+int vfio_pci_load_config(VFIODevice *vbasedev, QEMUFile *f)
  {
      VFIOPCIDevice *vdev = container_of(vbasedev, VFIOPCIDevice, vbasedev);
      PCIDevice *pdev = &vdev->pdev;
@@ -2706,7 +2706,7 @@ static int vfio_pci_load_config(VFIODevice *vbasedev, 
QEMUFile *f)
  static VFIODeviceOps vfio_pci_ops = {
      .vfio_compute_needs_reset = vfio_pci_compute_needs_reset,
      .vfio_hot_reset_multi = vfio_pci_hot_reset_multi,
-    .vfio_eoi = vfio_intx_eoi,
+    .vfio_eoi = vfio_pci_intx_eoi,
      .vfio_get_object = vfio_pci_get_object,
      .vfio_save_config = vfio_pci_save_config,
      .vfio_load_config = vfio_pci_load_config,
@@ -2777,7 +2777,7 @@ bool vfio_populate_vga(VFIOPCIDevice *vdev, Error **errp)
      return true;
  }

-static bool vfio_populate_device(VFIOPCIDevice *vdev, Error **errp)

+bool vfio_pci_populate_device(VFIOPCIDevice *vdev, Error **errp)
  {
      VFIODevice *vbasedev = &vdev->vbasedev;
      struct vfio_region_info *reg_info = NULL;
@@ -2823,7 +2823,7 @@ static bool vfio_populate_device(VFIOPCIDevice *vdev, 
Error **errp)
          return false;
      }

- trace_vfio_populate_device_config(vdev->vbasedev.name,

+    trace_vfio_pci_populate_device_config(vdev->vbasedev.name,
                                        (unsigned long)reg_info->size,
                                        (unsigned long)reg_info->offset,
                                        (unsigned long)reg_info->flags);
@@ -2845,7 +2845,7 @@ static bool vfio_populate_device(VFIOPCIDevice *vdev, 
Error **errp)
      ret = vfio_device_get_irq_info(vbasedev, VFIO_PCI_ERR_IRQ_INDEX, 
&irq_info);
      if (ret) {
          /* This can fail for an old kernel or legacy PCI dev */
-        trace_vfio_populate_device_get_irq_info_failure(strerror(-ret));
+        trace_vfio_pci_populate_device_get_irq_info_failure(strerror(-ret));
      } else if (irq_info.count == 1) {
          vdev->pci_aer = true;
      } else {
@@ -2857,7 +2857,7 @@ static bool vfio_populate_device(VFIOPCIDevice *vdev, 
Error **errp)
      return true;
  }

-static void vfio_pci_put_device(VFIOPCIDevice *vdev)

+void vfio_pci_put_device(VFIOPCIDevice *vdev)
  {
      vfio_display_finalize(vdev);
      vfio_bars_finalize(vdev);
@@ -2905,7 +2905,7 @@ static void vfio_err_notifier_handler(void *opaque)
   * and continue after disabling error recovery support for the
   * device.
   */
-static void vfio_register_err_notifier(VFIOPCIDevice *vdev)
+void vfio_pci_register_err_notifier(VFIOPCIDevice *vdev)
  {
      Error *err = NULL;
      int32_t fd;
@@ -2964,7 +2964,7 @@ static void vfio_req_notifier_handler(void *opaque)
      }
  }

-static void vfio_register_req_notifier(VFIOPCIDevice *vdev)

+void vfio_pci_register_req_notifier(VFIOPCIDevice *vdev)
  {
      struct vfio_irq_info irq_info;
      Error *err = NULL;
@@ -3018,7 +3018,7 @@ static void vfio_unregister_req_notifier(VFIOPCIDevice 
*vdev)
      vdev->req_enabled = false;
  }

-static bool vfio_pci_config_setup(VFIOPCIDevice *vdev, Error **errp)

+bool vfio_pci_config_setup(VFIOPCIDevice *vdev, Error **errp)
  {
      PCIDevice *pdev = &vdev->pdev;
      VFIODevice *vbasedev = &vdev->vbasedev;
@@ -3124,7 +3124,7 @@ static bool vfio_pci_config_setup(VFIOPCIDevice *vdev, 
Error **errp)
      return true;
  }

-static bool vfio_interrupt_setup(VFIOPCIDevice *vdev, Error **errp)

+bool vfio_pci_interrupt_setup(VFIOPCIDevice *vdev, Error **errp)
  {
      PCIDevice *pdev = &vdev->pdev;

@@ -3214,7 +3214,7 @@ static void vfio_pci_realize(PCIDevice *pdev, Error **errp)

          goto error;
      }

- if (!vfio_populate_device(vdev, errp)) {

+    if (!vfio_pci_populate_device(vdev, errp)) {
          goto error;
      }

@@ -3228,7 +3228,7 @@ static void vfio_pci_realize(PCIDevice *pdev, Error **errp)

          goto out_teardown;
      }

- if (!vfio_add_capabilities(vdev, errp)) {

+    if (!vfio_pci_add_capabilities(vdev, errp)) {
          goto out_unset_idev;
      }

@@ -3244,7 +3244,7 @@ static void vfio_pci_realize(PCIDevice *pdev, Error **errp)

          vfio_bar_quirk_setup(vdev, i);
      }

- if (!vfio_interrupt_setup(vdev, errp)) {

+    if (!vfio_pci_interrupt_setup(vdev, errp)) {
          goto out_unset_idev;
      }

@@ -3288,8 +3288,8 @@ static void vfio_pci_realize(PCIDevice *pdev, Error **errp)

          }
      }

- vfio_register_err_notifier(vdev);

-    vfio_register_req_notifier(vdev);
+    vfio_pci_register_err_notifier(vdev);
+    vfio_pci_register_req_notifier(vdev);
      vfio_setup_resetfn_quirk(vdev);

return;

@@ -3310,8 +3310,8 @@ out_unset_idev:
          pci_device_unset_iommu_device(pdev);
      }
  out_teardown:
-    vfio_teardown_msi(vdev);
-    vfio_bars_exit(vdev);
+    vfio_pci_teardown_msi(vdev);
+    vfio_pci_bars_exit(vdev);
  error:
      error_prepend(errp, VFIO_MSG_PREFIX, vbasedev->name);
  }
@@ -3338,9 +3338,9 @@ static void vfio_exitfn(PCIDevice *pdev)
      if (vdev->intx.mmap_timer) {
          timer_free(vdev->intx.mmap_timer);
      }
-    vfio_teardown_msi(vdev);
+    vfio_pci_teardown_msi(vdev);
      vfio_pci_disable_rp_atomics(vdev);
-    vfio_bars_exit(vdev);
+    vfio_pci_bars_exit(vdev);
      vfio_migration_exit(vbasedev);
      if (!vbasedev->mdev) {
          pci_device_unset_iommu_device(pdev);
diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
index e90ec9bff8..f06236f37b 100644
--- a/hw/vfio/trace-events
+++ b/hw/vfio/trace-events
@@ -2,7 +2,7 @@

# pci.c

  vfio_intx_interrupt(const char *name, char line) " (%s) Pin %c"
-vfio_intx_eoi(const char *name) " (%s) EOI"
+vfio_pci_intx_eoi(const char *name) " (%s) EOI"
  vfio_intx_enable_kvm(const char *name) " (%s) KVM INTx accel enabled"
  vfio_intx_disable_kvm(const char *name) " (%s) KVM INTx accel disabled"
  vfio_intx_update(const char *name, int new_irq, int target_irq) " (%s) IRQ moved %d 
-> %d"
@@ -35,8 +35,8 @@ vfio_pci_hot_reset(const char *name, const char *type) " (%s) 
%s"
  vfio_pci_hot_reset_has_dep_devices(const char *name) "%s: hot reset dependent 
devices:"
  vfio_pci_hot_reset_dep_devices(int domain, int bus, int slot, int function, int 
group_id) "\t%04x:%02x:%02x.%x group %d"
  vfio_pci_hot_reset_result(const char *name, const char *result) "%s hot reset: 
%s"
-vfio_populate_device_config(const char *name, unsigned long size, unsigned long offset, 
unsigned long flags) "Device '%s' config: size: 0x%lx, offset: 0x%lx, flags: 
0x%lx"
-vfio_populate_device_get_irq_info_failure(const char *errstr) 
"VFIO_DEVICE_GET_IRQ_INFO failure: %s"
+vfio_pci_populate_device_config(const char *name, unsigned long size, unsigned long 
offset, unsigned long flags) "Device '%s' config: size: 0x%lx, offset: 0x%lx, flags: 
0x%lx"
+vfio_pci_populate_device_get_irq_info_failure(const char *errstr) 
"VFIO_DEVICE_GET_IRQ_INFO failure: %s"
  vfio_mdev(const char *name, bool is_mdev) " (%s) is_mdev %d"
  vfio_add_ext_cap_dropped(const char *name, uint16_t cap, uint16_t offset) "%s 
0x%x@0x%x"
  vfio_pci_reset(const char *name) " (%s)"