linux-kernel - Re: [PATCH v2] PCI: pciehp: Clear LBMS on hot-remove to prevent link speed reduction

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: <cf5f3b03-4c70-7a35-056e-5d94fc26f697@amd.com>
Date: Tue, 18 Jun 2024 14:23:21 -0700
From: Smita Koralahalli <Smita.KoralahalliChannabasappa@....com>
To: Bjorn Helgaas <helgaas@...nel.org>
Cc: linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
 Mahesh J Salgaonkar <mahesh@...ux.ibm.com>, Lukas Wunner <lukas@...ner.de>,
 Yazen Ghannam <yazen.ghannam@....com>,
 Ilpo Jarvinen <ilpo.jarvinen@...ux.intel.com>,
 Bowman Terry <terry.bowman@....com>, Hagan Billy <billy.hagan@....com>,
 Simon Guinot <simon.guinot@...gate.com>,
 "Maciej W . Rozycki" <macro@...am.me.uk>,
 Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@...ux.intel.com>
Subject: Re: [PATCH v2] PCI: pciehp: Clear LBMS on hot-remove to prevent link
 speed reduction

On 6/18/2024 11:51 AM, Smita Koralahalli wrote:

[snip]

>>>> But IIUC LBMS is set by hardware but never cleared by hardware, so if
>>>> we remove a device and power off the slot, it doesn't seem like LBMS
>>>> could be telling us anything useful (what could we do in response to
>>>> LBMS when the slot is empty?), so it makes sense to me to clear it.
>>>>
>>>> It seems like pciehp_unconfigure_device() does sort of PCI core and
>>>> driver-related things and possibly could be something shared by all
>>>> hotplug drivers, while remove_board() does things more specific to the
>>>> hotplug model (pciehp, shpchp, etc).
>>>>
>>>>   From that perspective, clearing LBMS might fit better in
>>>> remove_board().  In that case, I wonder whether it should be done
>>>> after turning off slot power?  This patch clears is *before* turning
>>>> off the power, so I wonder if hardware could possibly set it again
>>>> before the poweroff?

While clearing LBMS in remove_board() here:

if (POWER_CTRL(ctrl)) {
	pciehp_power_off_slot(ctrl);
+	pcie_capability_write_word(ctrl->pcie->port, PCI_EXP_LNKSTA,
				   PCI_EXP_LNKSTA_LBMS);

	/*
	 * After turning power off, we must wait for at least 1 second
	 * before taking any action that relies on power having been
	 * removed from the slot/adapter.
	 */
	msleep(1000);

	/* Ignore link or presence changes caused by power off */
	atomic_and(~(PCI_EXP_SLTSTA_DLLSC | PCI_EXP_SLTSTA_PDC),
		   &ctrl->pending_events);
}

This can happen too right? I.e Just after the slot poweroff and before 
LBMS clearing the PDC/PDSC could be fired. Then 
pciehp_handle_presence_or_link_change() would hit case "OFF_STATE" and 
proceed with pciehp_enable_slot() ....pcie_failed_link_retrain() and 
ultimately link speed drops..

So, I added clearing just before turning off the slot.. Let me know if 
I'm thinking it right.

Thanks
Smita
>>>
>>> Yeah by talking to HW people I realized that HW could interfere possibly
>>> anytime to set LBMS when the slot power is on. Will change it to 
>>> include in
>>> remove_board().
>>>

[snip]