linux-kernel - Re: [PATCH 1/1] PCI/PME+pciehp: Request IRQF

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: <ca3008f1-d4ba-a68e-5a3c-a9e2e075eaa0@linux.intel.com>
Date: Fri, 15 Nov 2024 18:27:34 +0200 (EET)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Lukas Wunner <lukas@...ner.de>
cc: Stefan Wahren <wahrenst@....net>, 
    Florian Fainelli <florian.fainelli@...adcom.com>, 
    Thomas Gleixner <tglx@...utronix.de>, Bjorn Helgaas <bhelgaas@...gle.com>, 
    linux-pci@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/1] PCI/PME+pciehp: Request IRQF_ONESHOT because bwctrl
 shares IRQ

On Fri, 15 Nov 2024, Lukas Wunner wrote:

> On Thu, Nov 14, 2024 at 04:20:34PM +0200, Ilpo Järvinen wrote:
> > --- a/drivers/pci/hotplug/pciehp_hpc.c
> > +++ b/drivers/pci/hotplug/pciehp_hpc.c
> > @@ -68,7 +68,8 @@ static inline int pciehp_request_irq(struct controller *ctrl)
> >  
> >  	/* Installs the interrupt handler */
> >  	retval = request_threaded_irq(irq, pciehp_isr, pciehp_ist,
> > -				      IRQF_SHARED, "pciehp", ctrl);
> > +				      IRQF_SHARED | IRQF_ONESHOT,
> > +				      "pciehp", ctrl);
> >  	if (retval)
> >  		ctrl_err(ctrl, "Cannot get irq %d for the hotplug controller\n",
> >  			 irq);
> 
> I don't think this will work.  The IRQ thread pciehp_ist() may write
> to the Slot Control register and await a Command Completed event,
> e.g. when turning Slot Power on/off, changing LEDs, etc.
> 
> What happens then is, the hardware sets the Command Completed bit in
> the Slot Status register and signals an interrupt.  The hardirq handler
> pciehp_isr() reads the Slot Status register, acknowledges the
> Command Completed event, sets "ctrl->cmd_busy = 0" and wakes up the
> waiting IRQ thread.
> 
> In other words, pciehp does need the interrupt to stay enabled while
> the IRQ thread is running so that the hardirq handler can receive
> Command Completed interrupts.
> 
> Note that DPC also does not use IRQF_ONESHOT, so you'd have to change
> that as well in this patch.  The Raspberry Pi happens to not support
> DPC, so Stefan didn't see an error related to it.
> 
> I'm afraid you need to amend bwctrl to work without IRQF_ONESHOT rather
> than changing all the others.

That isn't complicated. The current irq thread handler is simple enough 
that it will just work as hardirq handler without any changes.

-- 
 i.