Re: [PATCH V3 21/42] vfio/pci: export MSI functions

[Cédric Le Goater]

On 5/16/25 19:58, Steven Sistare wrote:

On 5/16/2025 4:31 AM, Cédric Le Goater wrote:

On 5/12/25 17:32, Steve Sistare wrote:

Export various MSI functions, for use by CPR in subsequent patches.
No functional change.

Signed-off-by: Steve Sistare <steven.sist...@oracle.com>

Please rename this routines with a 'vfio_pci' prefix.

Are you sure?  That makes sense for:
   vfio_vector_init -> vfio_pci_vector_init

but the rest already have msi or intx in the name which unambiguously
means pci.  Adding pci_ seems unecessarily verbose:

We are slowly defining an API for an internal VFIO library. I prefer
to ensure the interface is clean by changing the names of external
services to reflect the namespace they belong to.

All routines are implemented in hw/vfio/pci.c and most take a
VFIOPCIDevice as first argument.

+void vfio_msi_interrupt(void *opaque);
+void vfio_add_kvm_msi_virq(VFIOPCIDevice *vdev, VFIOMSIVector *vector,
+                           int vector_n, bool msix);
+int vfio_msix_vector_use(PCIDevice *pdev, unsigned int nr, MSIMessage msg);
+void vfio_msix_vector_release(PCIDevice *pdev, unsigned int nr);

vfio_msi_interrupt(), vfio_msix_vector_use() and
vfio_msix_vector_release() are rather low level routines.
I think we need a wrapper to avoid exposing them.


Thanks,

C.

+bool vfio_msix_present(void *opaque, int version_id);
+void vfio_prepare_kvm_msi_virq_batch(VFIOPCIDevice *vdev);
+void vfio_commit_kvm_msi_virq_batch(VFIOPCIDevice *vdev);
+bool vfio_intx_enable(VFIOPCIDevice *vdev, Error **errp);

- Steve

---
  hw/vfio/pci.c | 21 ++++++++++-----------
  hw/vfio/pci.h | 12 ++++++++++++
  2 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index d2b08a3..1bca415 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -279,7 +279,7 @@ static void vfio_irqchip_change(Notifier *notify, void 
*data)
      vfio_intx_update(vdev, &vdev->intx.route);
  }
-static bool vfio_intx_enable(VFIOPCIDevice *vdev, Error **errp)
+bool vfio_intx_enable(VFIOPCIDevice *vdev, Error **errp)
  {
      uint8_t pin = vfio_pci_read_config(&vdev->pdev, PCI_INTERRUPT_PIN, 1);
      Error *err = NULL;
@@ -353,7 +353,7 @@ static void vfio_intx_disable(VFIOPCIDevice *vdev)
  /*
   * MSI/X
   */
-static void vfio_msi_interrupt(void *opaque)
+void vfio_msi_interrupt(void *opaque)
  {
      VFIOMSIVector *vector = opaque;
      VFIOPCIDevice *vdev = vector->vdev;
@@ -474,8 +474,8 @@ static int vfio_enable_vectors(VFIOPCIDevice *vdev, bool 
msix)
      return ret;
  }
-static void vfio_add_kvm_msi_virq(VFIOPCIDevice *vdev, VFIOMSIVector *vector,
-                                  int vector_n, bool msix)
+void vfio_add_kvm_msi_virq(VFIOPCIDevice *vdev, VFIOMSIVector *vector,
+                           int vector_n, bool msix)
  {
      if ((msix && vdev->no_kvm_msix) || (!msix && vdev->no_kvm_msi)) {
          return;
@@ -529,7 +529,7 @@ static void vfio_update_kvm_msi_virq(VFIOMSIVector *vector, 
MSIMessage msg,
      kvm_irqchip_commit_routes(kvm_state);
  }
-static void vfio_vector_init(VFIOPCIDevice *vdev, int nr)
+void vfio_vector_init(VFIOPCIDevice *vdev, int nr)
  {
      VFIOMSIVector *vector = &vdev->msi_vectors[nr];
      PCIDevice *pdev = &vdev->pdev;
@@ -641,13 +641,12 @@ static int vfio_msix_vector_do_use(PCIDevice *pdev, 
unsigned int nr,
      return 0;
  }
-static int vfio_msix_vector_use(PCIDevice *pdev,
-                                unsigned int nr, MSIMessage msg)
+int vfio_msix_vector_use(PCIDevice *pdev, unsigned int nr, MSIMessage msg)
  {
      return vfio_msix_vector_do_use(pdev, nr, &msg, vfio_msi_interrupt);
  }
-static void vfio_msix_vector_release(PCIDevice *pdev, unsigned int nr)
+void vfio_msix_vector_release(PCIDevice *pdev, unsigned int nr)
  {
      VFIOPCIDevice *vdev = VFIO_PCI_BASE(pdev);
      VFIOMSIVector *vector = &vdev->msi_vectors[nr];
@@ -674,14 +673,14 @@ static void vfio_msix_vector_release(PCIDevice *pdev, 
unsigned int nr)
      }
  }
-static void vfio_prepare_kvm_msi_virq_batch(VFIOPCIDevice *vdev)
+void vfio_prepare_kvm_msi_virq_batch(VFIOPCIDevice *vdev)
  {
      assert(!vdev->defer_kvm_irq_routing);
      vdev->defer_kvm_irq_routing = true;
      vfio_route_change = kvm_irqchip_begin_route_changes(kvm_state);
  }
-static void vfio_commit_kvm_msi_virq_batch(VFIOPCIDevice *vdev)
+void vfio_commit_kvm_msi_virq_batch(VFIOPCIDevice *vdev)
  {
      int i;
@@ -2632,7 +2631,7 @@ static Object *vfio_pci_get_object(VFIODevice *vbasedev)
      return OBJECT(vdev);
  }
-static bool vfio_msix_present(void *opaque, int version_id)
+bool vfio_msix_present(void *opaque, int version_id)
  {
      PCIDevice *pdev = opaque;
diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
index 5ce0fb9..c892054 100644
--- a/hw/vfio/pci.h
+++ b/hw/vfio/pci.h
@@ -210,6 +210,18 @@ static inline bool vfio_is_vga(VFIOPCIDevice *vdev)
      return class == PCI_CLASS_DISPLAY_VGA;
  }
+/* MSI/MSI-X/INTx */
+void vfio_vector_init(VFIOPCIDevice *vdev, int nr);
+void vfio_msi_interrupt(void *opaque);
+void vfio_add_kvm_msi_virq(VFIOPCIDevice *vdev, VFIOMSIVector *vector,
+                           int vector_n, bool msix);
+int vfio_msix_vector_use(PCIDevice *pdev, unsigned int nr, MSIMessage msg);
+void vfio_msix_vector_release(PCIDevice *pdev, unsigned int nr);
+bool vfio_msix_present(void *opaque, int version_id);
+void vfio_prepare_kvm_msi_virq_batch(VFIOPCIDevice *vdev);
+void vfio_commit_kvm_msi_virq_batch(VFIOPCIDevice *vdev);
+bool vfio_intx_enable(VFIOPCIDevice *vdev, Error **errp);
+
  uint32_t vfio_pci_read_config(PCIDevice *pdev, uint32_t addr, int len);
  void vfio_pci_write_config(PCIDevice *pdev,
                             uint32_t addr, uint32_t val, int len);