We are configuring the error signaling on the vast majority of devices and
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. 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.
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>
---
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.0On 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?
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.
> ---
> 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
>
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
> >
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
>>
© 2016 - 2026 Red Hat, Inc.