Re: [PATCH v1 6/6] vfio: Allow error notification and recovery for ISM device

[Farhan Ali <alifm@linux.ibm.com>]

On 8/15/2025 1:48 PM, Alex Williamson wrote:
> On Thu, 14 Aug 2025 14:02:05 -0700
> Farhan Ali <alifm@linux.ibm.com> wrote:
>
>> On 8/14/2025 1:48 PM, Bjorn Helgaas wrote:
>>> On Wed, Aug 13, 2025 at 10:08:20AM -0700, Farhan Ali wrote:
>>>> VFIO allows error recovery and notification for devices that
>>>> are PCIe (and thus AER) capable. But for PCI devices on IBM
>>>> s390 error recovery involves platform firmware and
>>>> notification to operating system is done by architecture
>>>> specific way. The Internal Shared Memory(ISM) device is a legacy
>>>> PCI device (so not PCIe capable), but can still be recovered
>>>> when notified of an error.
>>> "PCIe (and thus AER) capable" reads as though AER is required for all
>>> PCIe devices, but AER is optional.
>>>
>>> I don't know the details of VFIO and why it tests for PCIe instead of
>>> AER.  Maybe AER is not relevant here and you don't need to mention
>>> AER above at all?
>> The original change that introduced this commit dad9f89 "VFIO-AER:
>> Vfio-pci driver changes for supporting AER" was adding the support for
>> AER for vfio. My assumption is the author thought if the device is AER
>> capable the pcie check should be sufficient? I can remove the AER
>> references in commit message. Thanks Farhan
> I've looked back through discussions when this went in and can't find
> any specific reasoning about why we chose pci_is_pcie() here.  Maybe
> we were trying to avoid setting up an error signal on devices that
> cannot have AER, but then why didn't we check specifically for AER.
> Maybe some version used PCIe specific calls in the handler that we
> didn't want to check runtime, but I don't spot such a dependency now.
>
> Possibly we should just remove the check.  We're configuring the error
> signaling on the vast majority of devices, it's extremely rare that it
> fires anyway, reporting it on a device where it cannot trigger seems
> relatively negligible and avoids extra ugly code.  Thanks,
>
> Alex

Okay will remove the check and re-word the commit message.

Thanks
Farhan