Re: [PATCH v4 2/2] selftests/pcie_lmr: Add tests for the Lane Margining debugfs interface

[Priyank Rathod]

On Tue, 6 Oct 2026, sashiko-bot@xxxxxxxxxx wrote:
> > +	up=$([ "$rx" = 6 ] && echo "$dev" || echo "$partner")
> > +	after="$(link_regs "$dev") $(link_regs "$partner")"
> > +	if [ -z "${before// }" ]; then
> > +		ktap_test_skip "$dev: setpci not found"
> > +	elif [ -n "$up" ] && [ ! -d "/sys/bus/pci/devices/$up/link" ]; then
> > +		ktap_test_skip "$dev: no OS ASPM control on link"
> 
> [Severity: Medium]
> Is it possible that the logic for identifying the upstream port is reversed?
> 
> In active_tests(), the variable up is assigned using the receiver type.
> If rx=6, dev is the Endpoint device, so up becomes the Endpoint.
> If rx=1, dev is the Downstream Port, meaning partner is the Endpoint,
> so up still evaluates to the Endpoint device.
> 
> Because up always evaluates to the Endpoint device instead of the
> Downstream Port, the check for the sysfs link directory will always fail,
> as that directory is only present on the Downstream Port. Will this cause
> the test to unconditionally skip the final LNKCTL restoration checks?

For human reviewers: this is a false positive.

In PCIe terminology, the Upstream Port ('up', receiver 6) is the child
device below the Downstream Port (an Endpoint or Switch Upstream Port).
In drivers/pci/pcie/aspm.c, aspm_ctrl_attrs_are_visible() looks up the
link via pcie_aspm_get_link(pdev), which reads
pci_upstream_bridge(pdev)->link_state.  So the sysfs 'link/' directory
is attached to the child device below the Downstream Port (receiver 6),
not to the Downstream Port itself.

To avoid any confusion over the variable name 'up', I'll send v5 with
'up' renamed to 'child' and a comment explaining the sysfs 'link/'
lookup.

Thanks,
Priyank