Re: [PATCH v3 0/9] cxl: Delay HB port and switch dport probing until endpoint dev probe

[Robert Richter <rrichter@amd.com>]

Dave,

thanks for your series.

On 21.05.25 11:34:34, Dave Jiang wrote:
> v3:
> - Main changes revolve around improving naming of hostbridge uport and dport (Gregory)
> - See specific patches for detailed change log
> 
> This series attempts to delay the setup of dports and Host Bridge (HB) register
> until when the endpoint device (memdev) is being probed. At this point,
> the CXL link is established and all the devices along the CXL link path up to
> the Root Port (RP) should be active.
> 
> And hopefully this help a bit with Robert's issue raised in the "Inactive
> downstream port handling" series [1]. Testing would be appreicated. Thank you!
> 
> [1]: https://lore.kernel.org/linux-cxl/67c8a0cc23ec_24b64294f6@dwillia2-xfh.jf.intel.com.notmuch/
> 
> Dave Jiang (9):
>   cxl/region: Add decoder check to check_commit_order()
>   cxl: Add helper to detect top of CXL device topology
>   cxl: Separate out CXL dport->id vs actual dport hardware id
>   cxl: Remove adding of port_num via devm_cxl_add_dport()
>   cxl: Defer hardware dport->port_id assignment and registers probing
>   cxl/test: Add workaround for cxl_test for cxl_core calling mocked
>     functions
>   cxl: Change sslbis handler to only handle single dport
>   cxl: Create an xarray to tie a host bridge to the cxl_root
>   cxl: Move enumeration of hostbridge ports to the memdev probe path

I have tested your series and this solves the enumeration issue with
ports where the link of its downstream ports is down and that have
duplicate port ids. For the whole series you can add:

Tested-by: Robert Richter <rrichter@amd.com>

However, some observations:

The port_num can no longer retrieved using sysfs. Previously, the X in
dportX could be used to identify the port number (former port_id)
which was identical to the numbers in the sysfs target_list entries.
This is esp. useful to reconstruct the decoder tree and map pci child
devices and its decoders to a parent decoder including its positions
in the target list. Maybe create a targetX symlink from the decoder
device to the child decoders of it?

Due to the different initialization order there is an odd port
numbering now showing up with unexpected, out-of-order sequence
numbers, e.g.:

/sys/bus/cxl/devices/port1/endpoint2
/sys/bus/cxl/devices/port1/endpoint3
/sys/bus/cxl/devices/port1/endpoint6
/sys/bus/cxl/devices/port4/endpoint5
/sys/bus/cxl/devices/port4/endpoint7
/sys/bus/cxl/devices/port4/endpoint10
/sys/bus/cxl/devices/port4/endpoint11
/sys/bus/cxl/devices/port8/endpoint9
/sys/bus/cxl/devices/port12/endpoint13
/sys/bus/cxl/devices/port12/endpoint14

Still, this is correct, but maybe we could force a specific order
during initialization, such as per-port initialization which should
result in a defined order? Note the shared numbering of ports and
endpoints is also confusing, maybe that could be changed here too?

Going to review your patches.

Thanks for your patches.

-Robert