linux-kernel - Re: [PATCH 3/4] PCI/hotplug: reset_lock is not required synchronizing with irq thread

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: <Z9Ppr63yDUhOF1Xo@wunner.de>
Date: Fri, 14 Mar 2025 09:32:47 +0100
From: Lukas Wunner <lukas@...ner.de>
To: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Cc: Bjorn Helgaas <bhelgaas@...gle.com>, linux-pci@...r.kernel.org,
	Guenter Roeck <groeck@...iper.net>,
	Mika Westerberg <mika.westerberg@...ux.intel.com>,
	"Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
	Rajat Jain <rajatxjain@...il.com>,
	Joel Mathew Thomas <proxy0@...amail.com>,
	linux-kernel@...r.kernel.org,
	Jonathan Cameron <Jonathan.Cameron@...wei.com>
Subject: Re: [PATCH 3/4] PCI/hotplug: reset_lock is not required
 synchronizing with irq thread

On Thu, Mar 13, 2025 at 04:23:32PM +0200, Ilpo Järvinen wrote:
> Disabling HPIE (Hot-Plug Interrupt Enable) and synchronizing with irq
> handling in pciehp_reset_slot() is enough to ensure no pending events
> are processed during the slot reset. Thus, there is no need to take
> reset_lock in the IRQ thread.
[...]
> --- a/drivers/pci/hotplug/pciehp_hpc.c
> +++ b/drivers/pci/hotplug/pciehp_hpc.c
> @@ -748,12 +748,10 @@ static irqreturn_t pciehp_ist(int irq, void *dev_id)
>  	 * Disable requests have higher priority than Presence Detect Changed
>  	 * or Data Link Layer State Changed events.
>  	 */
> -	down_read_nested(&ctrl->reset_lock, ctrl->depth);
>  	if (events & DISABLE_SLOT)
>  		pciehp_handle_disable_request(ctrl);
>  	else if (events & (PCI_EXP_SLTSTA_PDC | PCI_EXP_SLTSTA_DLLSC))
>  		pciehp_handle_presence_or_link_change(ctrl, events);
> -	up_read(&ctrl->reset_lock);
>  
>  	ret = IRQ_HANDLED;
>  out:

The release and re-acquisition of reset_lock in
pciehp_configure_device() and pciehp_unconfigure_device()
needs to be removed as well if the above hunk is applied.

But please wait a little while before respinning so that I can
think through the whole series.

Thanks,

Lukas