Re: [PATCH v11 9/9] vfio: Remove the pcie check for VFIO_PCI_ERR_IRQ_INDEX

Alex Williamson
2026-03-25T17:50:29Z

On Tue, 24 Mar 2026 16:26:02 -0500
Bjorn Helgaas <helgaas@kernel.org> wrote:

> On Mon, Mar 16, 2026 at 12:15:44PM -0700, Farhan Ali wrote:
> > We are configuring the error signaling on the vast majority of devices and
>
> Who is "we"? If a function configures error signaling, can you
> mention the name?
>
> > it's extremely rare that it fires anyway. This allows userspace to
> > be notified on errors for legacy PCI devices. The Internal Shared
> > Memory (ISM) device on s390 is one such device.
>
> This commit log talks about things that could be done, but doesn't
> actually say what the patch does or what makes it safe and effective,
> and I'm not VFIO-literate enough for it to be clear.
>
> These pci_is_pcie() tests were added by dad9f8972e04 ("VFIO-AER:
> Vfio-pci driver changes for supporting AER"), so I suppose the
> dad9f8972e04 assumption was that AER was the only error reporting
> mechanism, and AER only exists on PCIe devices?

Yes, that's the conclusion we came to in previous discussions that
Farhan notes in their reply.

> But s390 can report errors for conventional PCI devices, and you want
> VFIO to support that as well?
>
> Obviously this change needs to be safe for all arches, not just s390.
> I suppose it's safe to report the VFIO_PCI_ERR_IRQ_INDEX info
> everywhere; it's just that it will never be used except on s390? And
> I guess powerpc, which can get to vfio_pci_core_aer_err_detected() via
> eeh_report_failure().
>
> It looks like vfio_pci_driver provides vfio_pci_core_err_handlers
> whether the device is conventional PCI or PCIe, and s390 can already
> call vfio_pci_core_aer_err_detected() (the .error_detected() hook) via
> zpci_event_notify_error_detected(), so this patch makes it possible
> for the guest (QEMU, etc) to learn about it?
>
> > For PCI devices on IBM s390 error
> > recovery involves platform firmware and notification to operating system
> > is done by architecture specific way. So the ISM device can still be
> > recovered when notified of an error.
>
> I guess this error recovery part would be done by the guest ISM
> driver, triggered when when something like QEMU receives the eventfd
> signal from vfio_pci_core_aer_err_detected()?
>
> > Reviewed-by: Julian Ruess <julianr@linux.ibm.com>
> > Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>
> > Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
>
> I don't maintain VFIO, so I'm just kibbitzing here. Hopefully Alex
> will chime in.

It's the previous patch about restoring open state of the device on
.reset_done that gives me more anxiety than just reporting that
non-PCIe (non-AER) devices can report errors. At worst here, I think
userspace might be wiring an interrupt on a conventional device that
cannot fire, whereas most PCIe device have AER. The number of
conventional device in use with vfio-pci is probably not enough to
worry about though.

For completeness, I'll note that QEMU sets a "pci_aer" flag based on
whether this error IRQ is exposed, where I think we had intended this
might interact with emulated AER. However, it never made it that far
and just registers a handler that stops the VM.

Farhan, this and the previous patch should use "vfio/pci:" as their
title prefix. Thanks,

Alex

> > ---
> > drivers/vfio/pci/vfio_pci_core.c | 8 ++------
> > drivers/vfio/pci/vfio_pci_intrs.c | 3 +--
> > 2 files changed, 3 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> > index f1bd1266b88f..cfd9a51cd194 100644
> > --- a/drivers/vfio/pci/vfio_pci_core.c
> > +++ b/drivers/vfio/pci/vfio_pci_core.c
> > @@ -786,8 +786,7 @@ static int vfio_pci_get_irq_count(struct vfio_pci_core_device *vdev, int irq_typ
> > return (flags & PCI_MSIX_FLAGS_QSIZE) + 1;
> > }
> > } else if (irq_type == VFIO_PCI_ERR_IRQ_INDEX) {
> > - if (pci_is_pcie(vdev->pdev))
> > - return 1;
> > + return 1;
> > } else if (irq_type == VFIO_PCI_REQ_IRQ_INDEX) {
> > return 1;
> > }
> > @@ -1163,11 +1162,8 @@ static int vfio_pci_ioctl_get_irq_info(struct vfio_pci_core_device *vdev,
> > switch (info.index) {
> > case VFIO_PCI_INTX_IRQ_INDEX ... VFIO_PCI_MSIX_IRQ_INDEX:
> > case VFIO_PCI_REQ_IRQ_INDEX:
> > - break;
> > case VFIO_PCI_ERR_IRQ_INDEX:
> > - if (pci_is_pcie(vdev->pdev))
> > - break;
> > - fallthrough;
> > + break;
> > default:
> > return -EINVAL;
> > }
> > diff --git a/drivers/vfio/pci/vfio_pci_intrs.c b/drivers/vfio/pci/vfio_pci_intrs.c
> > index 33944d4d9dc4..64f80f64ff57 100644
> > --- a/drivers/vfio/pci/vfio_pci_intrs.c
> > +++ b/drivers/vfio/pci/vfio_pci_intrs.c
> > @@ -859,8 +859,7 @@ int vfio_pci_set_irqs_ioctl(struct vfio_pci_core_device *vdev, uint32_t flags,
> > case VFIO_PCI_ERR_IRQ_INDEX:
> > switch (flags & VFIO_IRQ_SET_ACTION_TYPE_MASK) {
> > case VFIO_IRQ_SET_ACTION_TRIGGER:
> > - if (pci_is_pcie(vdev->pdev))
> > - func = vfio_pci_set_err_trigger;
> > + func = vfio_pci_set_err_trigger;
> > break;
> > }
> > break;
> > --
> > 2.43.0
> >