linux-kernel - Re: [GIT PULL] PCI changes for v5.1

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]

Date:   Mon, 18 Mar 2019 05:33:14 +0100
From:   Lukas Wunner <lukas@...ner.de>
To:     Alex G <mr.nuke.me@...il.com>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Bjorn Helgaas <helgaas@...nel.org>, linux-pci@...r.kernel.org,
        Linux List Kernel Mailing <linux-kernel@...r.kernel.org>,
        Lorenzo Pieralisi <lorenzo.pieralisi@....com>
Subject: Re: [GIT PULL] PCI changes for v5.1

On Sun, Mar 17, 2019 at 07:22:17PM -0500, Alex G wrote:
> On 3/17/19 4:18 PM, Linus Torvalds wrote:
> > On Fri, Mar 8, 2019 at 9:31 AM Bjorn Helgaas <helgaas@...nel.org> wrote:
> > > 
> > >    - Report PCIe links that become degraded at run-time (Alexandru Gagniuc)
> > 
> > Gaah. Only now as I'm about to do the rc1 release am I looking at new
> > runtime warnings, and noticing that this causes
> > 
> >    genirq: Threaded irq requested with handler=NULL and !ONESHOT for irq 16
> >    pcie_bw_notification: probe of 0000:00:1b.0:pcie010 failed with error -22
> > 
> > because you can't have a NULL handler for a level-triggered interrupt
> > (you need something to shut the interrupt off so that it doesn't keep
> > screaming!).
> 
> Thanks for the catch. I did not see the error on my test machines. I'll take
> a look tomorrow, and update through Bjorn. Seems like an easy fix.

My apologies for not spotting this during review.

This can't be fixed by setting the IRQF_ONESHOT flag because the IRQ is
always shared with pciehp, dpc and other port services, which do not set
the flag, so you'd get a flags mismatch error in __setup_irq().

The solution is thus to acknowledge the interrupt in
pcie_bw_notification_handler() and move the portion which may sleep
to a separate function which is used as IRQ thread.  In other words,
move the down_read() / up_read() portion to a separate function and
return IRQ_WAKE_THREAD instead of IRQ_HANDLED.

The reason why it didn't show up on your test machines is likely that
they're using MSI on all PCIe ports, whereas Linus' machines appear
to possess at least one PCIe port which uses legacy INTx interrupts.
(MSI sets IRQCHIP_ONESHOT_SAFE for the irq_chip.)

Thanks,

Lukas