Re: [PATCH v2] vfio: pci: Check INTx availability before enabling them

[Alex Williamson <alex.williamson@redhat.com>]

On Tue, 18 Mar 2025 17:52:53 +0000
Shivaprasad G Bhat <sbhat@linux.ibm.com> wrote:

> Currently, the PCI_INTERRUPT_PIN alone is checked before enabling
> the INTx. It is also necessary to have the IRQ Lines assigned for
> the INTx to work.
> 
> The problem was observed on Power10 systems which primarily use
> MSI-X, and LSI lines are not connected on all devices under a
> PCIe switch. In this configuration where the PIN is non-zero
> but the LINE was 0xff, the VFIO_DEVICE_SET_IRQS was failing as
> it was trying to map the irqfd for the LSI of the device.
> 
> So the patch queries the INTx availability with
> VFIO_DEVICE_GET_IRQ_INFO ioctl, and enables only if available.

The kernel should just virtualize the PIN register to report INTx isn't
supported, as we already do when INTx is broken, or for platforms that
don't support INTx, and as proposed for INTx set to IRQ_NOTCONNECTED.
No QEMU changes necessary.  Thanks,

Alex

> 
> Signed-off-by: Shivaprasad G Bhat <sbhat@linux.ibm.com>
> ---
> Changelog:
> v1: https://lore.kernel.org/qemu-devel/173834353589.1880.3587671276264097972.stgit@linux.ibm.com/
> - Split the fix into two parts as suggested. Kernel part posted here [1]
> - Changed to use the irq_info for checking the intx availability.
> 
> [1]: https://lore.kernel.org/all/174231895238.2295.12586708771396482526.stgit@linux.ibm.com/
> 
>  hw/vfio/pci.c |   24 +++++++++++++++++++++---
>  1 file changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 7f1532fbed..54de6e72f8 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -261,6 +261,25 @@ static void vfio_irqchip_change(Notifier *notify, void *data)
>      vfio_intx_update(vdev, &vdev->intx.route);
>  }
> 
> +static bool vfio_check_intx_available(VFIOPCIDevice *vdev)
> +{
> +    uint8_t pin = vfio_pci_read_config(&vdev->pdev, PCI_INTERRUPT_PIN, 1);
> +    struct vfio_irq_info irq_info = { .argsz = sizeof(irq_info),
> +                                      .index = VFIO_PCI_INTX_IRQ_INDEX};
> +
> +    if (!pin) {
> +        return false;
> +    }
> +
> +    if (ioctl(vdev->vbasedev.fd,
> +              VFIO_DEVICE_GET_IRQ_INFO, &irq_info) < 0) {
> +        warn_report("VFIO_DEVICE_GET_IRQ_INFO failed to query INTx");
> +        return false;
> +    }
> +
> +    return (irq_info.count != 0);
> +}
> +
>  static bool vfio_intx_enable(VFIOPCIDevice *vdev, Error **errp)
>  {
>      uint8_t pin = vfio_pci_read_config(&vdev->pdev, PCI_INTERRUPT_PIN, 1);
> @@ -268,8 +287,7 @@ static bool vfio_intx_enable(VFIOPCIDevice *vdev, Error **errp)
>      int32_t fd;
>      int ret;
> 
> -
> -    if (!pin) {
> +    if (!vfio_check_intx_available(vdev)) {
>          return true;
>      }
> 
> @@ -3151,7 +3169,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
>                 vdev->msi_cap_size);
>      }
> 
> -    if (vfio_pci_read_config(&vdev->pdev, PCI_INTERRUPT_PIN, 1)) {
> +    if (vfio_check_intx_available(vdev)) {
>          vdev->intx.mmap_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL,
>                                                    vfio_intx_mmap_enable, vdev);
>          pci_device_set_intx_routing_notifier(&vdev->pdev,
> 
>