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

Smita Koralahalli
2024-06-18T21:23:26Z

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]