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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250815144855.51f2ac24.alex.williamson@redhat.com>
Date: Fri, 15 Aug 2025 14:48:55 -0600
From: Alex Williamson <alex.williamson@...hat.com>
To: Farhan Ali <alifm@...ux.ibm.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 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