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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ffc2fc08-2e95-4b35-840c-be8f5511340f@linux.ibm.com>
Date: Fri, 15 Aug 2025 14:36:26 -0700
From: Farhan Ali <alifm@...ux.ibm.com>
To: Alex Williamson <alex.williamson@...hat.com>
Cc: Bjorn Helgaas <helgaas@...nel.org>, linux-s390@...r.kernel.org,
        kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
        schnelle@...ux.ibm.com, mjrosato@...ux.ibm.com
Subject: Re: [PATCH v1 6/6] vfio: Allow error notification and recovery for
 ISM device


On 8/15/2025 1:48 PM, Alex Williamson wrote:
> On Thu, 14 Aug 2025 14:02:05 -0700
> Farhan Ali <alifm@...ux.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