linux-kernel - Re: [PATCH 2/2] cxl/pci: Check link status and only enable active dports

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <67cb5c53405df_17b58e294bf@iweiny-mobl.notmuch>
Date: Fri, 7 Mar 2025 14:51:31 -0600
From: Ira Weiny <ira.weiny@...el.com>
To: Robert Richter <rrichter@....com>, Ira Weiny <ira.weiny@...el.com>
CC: Alison Schofield <alison.schofield@...el.com>, Vishal Verma
	<vishal.l.verma@...el.com>, Dan Williams <dan.j.williams@...el.com>,
	"Jonathan Cameron" <Jonathan.Cameron@...wei.com>, Dave Jiang
	<dave.jiang@...el.com>, Davidlohr Bueso <dave@...olabs.net>,
	<linux-cxl@...r.kernel.org>, <linux-kernel@...r.kernel.org>, Gregory Price
	<gourry@...rry.net>, "Terry Bowman" <terry.bowman@....com>
Subject: Re: [PATCH 2/2] cxl/pci: Check link status and only enable active
 dports

Robert Richter wrote:
> On 05.03.25 09:19:59, Ira Weiny wrote:
> > Robert Richter wrote:
> > > When downstream ports are enumerated, some of them may not be
> > > connected to a corresponding endpoint or upstream switch port. The
> > > dport is inactive and its link status is down then. For permanently
> > > disabled ports a HwInit configuration mechanism (set by hardware or
> > > firmware) may assign a (further unused) default port number. The port
> > > number may be set to the same value accross other inactive links.
> > > Those duplicate port numbers cause the downstream port enumeration to
> > > fail including the root or switch port initialization
> > > (cxl_switch_port_probe()) and all its active downstream ports.
> > > 
> > > Prevent a port initialization failure by checking the link status and
> > > only enabling active dports. If a dport is inactive, there is no
> > > matching component (endpoint or switch) connected to and thus, it must
> > > not be enumerated and added to the kernel's CXL device hierarchy.
> > > There is no device that will connect to an inactive dport.
> > 
> > This makes much more sense.
> > 
> > Wouldn't it be better to use this patch and leave the old error
> > messages/failures on duplicate port ids?  IOW drop patch 1?
> 
> The link check only works if Data Link Layer Link Active Reporting is
> supported. That is why there is the patch 1. That patch also handles
> cases where dups are seen that are unrelated to the link status.

Isn't it a bug in the hardware to have 2 active dports with the same ID?

> 
> > What happens if a link is having issues (perhaps going up and down) and
> > RAS events fire for this dport?  Does the lack of a dport object cause
> > issues?
> 
> Once a dport was enumerated it exists as long as the parent exists.

Yes but do we care enough about this unused memory being held until the
destruction of the device object?

Currently I think it all get's torn down which means we are not keeping
memory around.

Ira