Re: [PATCH v5 1/2] PCI/LMR: Add Lane Margining at the Receiver debugfs interface

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] pci: pci_lmr_reboot_notify skips lane reset on trylock failure
--

commit 1dd16e08c517390b6bc52a453ae7eeb35b21f5e3
Author: Priyank Rathod <rathodpriyank@google.com>

PCI/LMR: Add Lane Margining at the Receiver debugfs interface

This commit introduces a debugfs interface for PCIe Lane Margining at the
Receiver, enabling user-space to configure sampling point offsets and read
error counts. It also adds logic to disable ASPM and hardware autonomous
speed/width during an active margining session.

> diff --git a/drivers/pci/pcie/margin.c b/drivers/pci/pcie/margin.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/pci/pcie/margin.c

[ ... ]

> +/* Return margined lanes to normal settings before reboot or kexec */
> +static int pci_lmr_reboot_notify(struct notifier_block *nb,
> +				 unsigned long action, void *data)
> +{
> +	struct pci_lmr_port *port;
> +
> +	guard(mutex)(&pci_lmr_mutex);
> +	list_for_each_entry(port, &pci_lmr_ports, node) {
> +		if (port->state != LMR_ACTIVE)
> +			continue;
> +		if (!mutex_trylock(&port->lock))
> +			continue;

[Severity: Medium]
If this mutex_trylock() fails, does this code skip the hardware restore steps?

If a system reboot or kexec is initiated while a user-space tool is actively
margining a PCIe link, the margining thread may be holding port->lock
(which can block for up to 100ms waiting for hardware responses).

When this trylock fails, the notifier silently continues, skipping both the
lane reset (pci_lmr_reset_lanes) and the restoration of hardware autonomous
width/speed settings (pci_lmr_autonomous_restore).

Because kexec does not assert PERST# to fully reset the PCIe link, could the
new kernel inherit a link with shifted margins and disabled autonomous
management, potentially leading to link instability and missing devices?

> +		if (port->state == LMR_ACTIVE) {
> +			if (!pci_dev_is_disconnected(port->dev))
> +				pci_lmr_reset_lanes(port);
> +			pci_lmr_autonomous_restore(port);
> +		}
> +		mutex_unlock(&port->lock);
> +	}
> +
> +	return NOTIFY_DONE;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261006-pcie-link-endpoints-v5-0-c1e73235746b@google.com?part=1