[PATCH v2] bus/pci: fix automatic interrupt type selection

[PATCH v2] bus/pci: fix automatic interrupt type selection

[Pekka Riikonen]

Check if kernel returns 0 interrupt vectors and try another interrupt
type in that case.  Failing to check the vector count can select an
interrupt type that's unusable.

Signed-off-by: Pekka Riikonen <priikone@iki.fi>
---
  drivers/bus/pci/linux/pci_vfio.c | 4 ++++
  1 file changed, 4 insertions(+)

diff --git a/drivers/bus/pci/linux/pci_vfio.c 
b/drivers/bus/pci/linux/pci_vfio.c
index 5317170231..02e7d7e40b 100644
--- a/drivers/bus/pci/linux/pci_vfio.c
+++ b/drivers/bus/pci/linux/pci_vfio.c
@@ -237,6 +237,10 @@ pci_vfio_setup_interrupts(struct rte_pci_device 
*dev, int vfio_dev_fd)
  				continue;
  		}

+		/* If no vectors, try another one */
+		if (irq.count == 0)
+			continue;
+
  		/* Reallocate the efds and elist fields of intr_handle based
  		 * on PCI device MSIX size.
  		 */
-- 
2.34.1

Re: [PATCH v2] bus/pci: fix automatic interrupt type selection

[Stephen Hemminger]

On Fri, 20 Jun 2025 15:50:27 +0300
Pekka Riikonen <priikone@iki.fi> wrote:

> Check if kernel returns 0 interrupt vectors and try another interrupt
> type in that case.  Failing to check the vector count can select an
> interrupt type that's unusable.
> 
> Signed-off-by: Pekka Riikonen <priikone@iki.fi>

Patch looks good, automated review had some feedback.

1. Should add a Fixes: and Cc: stable@dpdk.org

2. There is a pre-existing bugs in the file as well

Error 1: Missing handling when interrupt type was explicitly requested and count is 0

Confidence: ~85%

The existing VFIO_IRQ_INFO_EVENTFD check (lines 223–228) has two branches: if the user explicitly requested this interrupt type (intr_mode != RTE_INTR_MODE_NONE), it returns an error; otherwise it continues to the next type. The new irq.count == 0 check does not mirror this pattern — it unconditionally does continue.

This means if a user explicitly requested e.g. MSI-X via command line (--vfio-intr=msix) but the device reports 0 vectors for MSI-X, the code will silently fall through and try MSI or INTx instead of reporting an error. This is inconsistent with the existing behavior for the eventfd flag check, and arguably a bug: the user asked for a specific interrupt type that doesn't work, and should be told.

Suggested fix:
c

		/* If no vectors, try another type (or fail if explicitly requested) */
		if (irq.count == 0) {
			if (intr_mode != RTE_INTR_MODE_NONE) {
				PCI_LOG(ERR, "Interrupt vector has no entries!");
				return -1;
			}
			continue;
		}

Error 2: Resource leak — eventfd fd leaked on later error paths (pre-existing)

Confidence: ~90%

This is a pre-existing issue, not introduced by this patch, but worth noting since the patch touches this function. At line 240, eventfd() creates a file descriptor. If rte_intr_fd_set() at line 247 fails, the function returns -1 without closing fd. This is a file descriptor leak on that error path.

Similarly, the rte_intr_event_list_update failure path at line 236–237 is fine (no fd allocated yet), but the rte_intr_fd_set failure is a real leak.

This is pre-existing and not caused by this patch, so it's informational rather than something blocking this patch.

Re: [PATCH v2] bus/pci: fix automatic interrupt type selection

[Kevin Traynor]

On 20/06/2025 13:50, Pekka Riikonen wrote:
> Check if kernel returns 0 interrupt vectors and try another interrupt
> type in that case.  Failing to check the vector count can select an
> interrupt type that's unusable.
> 
> Signed-off-by: Pekka Riikonen <priikone@iki.fi>
> ---
>   drivers/bus/pci/linux/pci_vfio.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 

Hi Pekka,

just one comment below about logging.

> diff --git a/drivers/bus/pci/linux/pci_vfio.c 
> b/drivers/bus/pci/linux/pci_vfio.c
> index 5317170231..02e7d7e40b 100644
> --- a/drivers/bus/pci/linux/pci_vfio.c
> +++ b/drivers/bus/pci/linux/pci_vfio.c
> @@ -237,6 +237,10 @@ pci_vfio_setup_interrupts(struct rte_pci_device 
> *dev, int vfio_dev_fd)
>   				continue;
>   		}
> 
> +		/* If no vectors, try another one */
> +		if (irq.count == 0)
> +			continue;
> +

This looks functionally correct. It will try the next interrupt type and
in the case where a user has selected a specific interrupt type with the
command line --vfio-intr, then it will return -1 at the end.

It would be nice to be consistent with the eventfd check just above and
add an error log in the case where the user specified the interrupt type.

something like below. what do you think ?

+               /* If no vectors, fail if there is an explicit
+                * interrupt type, otherwise continue */
+               if (irq.count == 0) {
+                       if (intr_mode != RTE_INTR_MODE_NONE) {
+                               PCI_LOG(ERR, "Interrupt has no vectors!");
+                               return -1;
+                       } else
+                               continue;
+               }


>   		/* Reallocate the efds and elist fields of intr_handle based
>   		 * on PCI device MSIX size.
>   		 */