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

Message-ID: <ZzdF1zrgQNNRlkgP@wunner.de>
Date: Fri, 15 Nov 2024 14:00:07 +0100
From: Lukas Wunner <lukas@...ner.de>
To: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
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,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] PCI/PME+pciehp: Request IRQF_ONESHOT because bwctrl
 shares IRQ

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.

Thanks,

Lukas