linux-kernel - Re: [PATCH v3 05/10] s390/pci: Restore IRQ unconditionally for the zPCI 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: <5735f0b6c543f1ca1a8d404dca3b6c42ae05d71c.camel@linux.ibm.com>
Date: Tue, 16 Sep 2025 12:59:32 +0200
From: Niklas Schnelle <schnelle@...ux.ibm.com>
To: Farhan Ali <alifm@...ux.ibm.com>, linux-s390@...r.kernel.org,
        kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-pci@...r.kernel.org
Cc: alex.williamson@...hat.com, helgaas@...nel.org, mjrosato@...ux.ibm.com
Subject: Re: [PATCH v3 05/10] s390/pci: Restore IRQ unconditionally for the
 zPCI device

On Mon, 2025-09-15 at 10:42 -0700, Farhan Ali wrote:
> On 9/15/2025 1:39 AM, Niklas Schnelle wrote:
> > On Thu, 2025-09-11 at 11:33 -0700, Farhan Ali wrote:
> > > Commit c1e18c17bda6 ("s390/pci: add zpci_set_irq()/zpci_clear_irq()"),
> > > introduced the zpci_set_irq() and zpci_clear_irq(), to be used while
> > > resetting a zPCI device.
> > > 
> > > Commit da995d538d3a ("s390/pci: implement reset_slot for hotplug slot"),
> > > mentions zpci_clear_irq() being called in the path for zpci_hot_reset_device().
> > > But that is not the case anymore and these functions are not called
> > > outside of this file.
> > If you're doing another version I think you could add a bit more
> > information on why this still works for existing recovery based on my
> > investigation in
> > https://lore.kernel.org/lkml/052ebdbb6f2d38025ca4345ee51e4857e19bb0e4.camel@linux.ibm.com/
> > 
> > Even if you don't add more explanations, I'd tend to just drop the
> > above paragraph as it doesn't seem relevant and sounds like
> > zpci_hot_reset_device() doesn't clear IRQs. As explained in the linked
> > mail there really is no need to call zpci_clear_irq() in
> > zpci_hot_reset_device() as the CLP disable does disable IRQs. It's
> > really only the state tracking that can get screwed up but is also fine
> > for drivers which end up doing the tear down.
> 
> I referenced commit da995d538d3a as that commit introduced the 
> arch_restore_msi_irqs and describes the reasoning as to why we need it. 
> It also mentions about zpci_clear_irq being called by 
> zpci_hot_reset_device. IMHO the message was confusing as it took me my 
> down the path of trying to identify any commit that changed the behavior 
> since da995d538d3a. But that wasn't the case and it was an error in the 
> commit message. I want to keep a reference here to at least clarify that.

Ok that makes sense, maybe you could add asentence like:
"… these functions are not called outside of this file. Instead
zpci_hot_reset_device() relies on zpci_disable_device() also clearing
the IRQs, but misses to reset the zdev->irqs_registered flag.