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

Farhan Ali
2026-03-24T22:30:28Z

Hi Bjorn,

On 3/24/2026 2:26 PM, Bjorn Helgaas 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?

Thanks for taking a look. By 'We' it would refer to userspace/QEMU that
set up error notification for a vfio device. I took the message verbatim
from Alex's suggestion[1].

>> 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?

We don't have the context anymore why PCIe device check was added. Alex
had suggested to remove the check entirely[1].

> 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?

Yes, with this patch want to notify the userspace/QEMU about
conventional PCI devices. I think it should be safe to report
VFIO_PCI_ERR_IRQ_INDEX even for conventional PCI devices for other
architectures. AFAIK other architectures would use the AER error
handling mechanism to report the error and call
vfio_pci_core_aer_err_detected().

>> 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()?

Yes, we wanted to be able to signal QEMU on an error, so that QEMU can
appropriately notify the guest about the error. Today the ISM driver
doesn't register error recovery callbacks. However we can still recover
from the guest by writing to 'reset' sysfs file.

[1]
https://lore.kernel.org/all/20250815144855.51f2ac24.alex.williamson@redhat.com/

>
>> 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.
>
>> ---
>> 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
>>