Re: [PATCH v8 05/11] cxl: Defer dport allocation for switch ports

[Robert Richter <rrichter@amd.com>]

On 22.08.25 08:52:39, Dave Jiang wrote:
> 
> 
> On 8/22/25 2:59 AM, Robert Richter wrote:
> > On 20.08.25 08:20:04, Dave Jiang wrote:
> >> On 8/20/25 5:41 AM, Robert Richter wrote:
> >>> Hi Dave,
> >>>
> >>> see my comments below.
> >>>
> >>> On 14.08.25 15:21:45, Dave Jiang wrote:
> >>
> >> <--snip-->
> >>  
> >>>> +	if (IS_ERR(new_dport))
> >>>> +		return new_dport;
> >>>> +
> >>>> +	cxl_switch_parse_cdat(port);
> >>>> +
> >>>> +	/*
> >>>> +	 * First instance of dport appearing, need to setup the port, including
> >>>> +	 * allocating decoders.
> >>>> +	 */
> >>>> +	if (port->nr_dports == 1) {
> >>>> +		rc = cxl_switch_port_setup(port);
> >>>
> >>> Can't this be done with port creation? I don't see a reason doing this
> >>> late at this point.
> >>
> > 
> >> The main reason we are doing this is to move the port register
> >> probing until we know the CXL link is established. Otherwise when
> >> cxl_acpi does probe and calls add_host_bridge_uport(), that
> >> devm_cxl_add_port() can trigger errors if the platform BIOS enables
> >> PCI hotplug support on Intel platforms. The error messages "cxl
> >> portN: Couldn't locate the CXL.cache and CXL.mem capability array
> >> header" is observed. Essentially we can be trying to map registers
> >> while DVSEC ID 3 and/or 7 has not appeared yet. And in turn because
> >> that got pushed out, so did the decoder enumeration.
> > 
> > The code suggests the Component Registers of the CXL Host Bridge are
> > not yet ready. Is this delayed after the first Root Port is connected
> > to a CXL Endpoint/Switch? PCIe DVSEC ID 3 and 7
> > (CXL_DVSEC_PORT_EXTENSIONS, CXL_DVSEC_PCIE_FLEXBUS_PORT) are part of
> > the pcie config space, which is enumerated not before a CXL endpoint
> > becomes active. I haven't found a spec refs here. Please explain.
> 

> So the behavior is observed when PCIe hotplug support is turned on
> in BIOS for the Intel platform. A CXL device is plugged in to a RP
> without CXL switches. The thinking is that the CXL link is not fully
> established at the time when cxl_acpi_probe() is running and the
> ports are being added. And the only way to 100% be sure the link is
> established is when we are enumerating the memdev just like the
> dports. Not sure what spec ref are you looking for. Table 8-2
> indicates that those 2 DVSECs are mandatory for CXL root ports. Lack
> of presence means either the RP isn't CXL or the CXL link isn't
> established yet. I would assume this would also be true if a CXL
> memdev is hot-plugged into a slot post boot.

But add_host_bridge_uport() only creates ports for the host bridge
(ACPI0016) devices and enumerates their component registers (CHBCR).
The root ports are being added already late as those are part of the
pci hierarchy. The root ports are discovered in
devm_cxl_enumerate_ports() not earlier than the mem_dev is probed.
devm_cxl_add_memdev() is called once the endpoint is probed and the
CXL link is up.

That is, function cxl_port_get_or_add_dport() in add_port_attach_ep
should only add the dport. Then, a retry in the enumeration loop will
be triggered and the cxl_port for the root port is added. No explicit
call of cxl_switch_port_setup() should be needed, it can be done
during cxl_port_probe(). All done late after the endpoint was found.

-Robert