Re: [PATCH 1/5] cxl: Add BI register probing and port initialization

[Richard Cheng <icheng@nvidia.com>]

On Fri, Jun 05, 2026 at 01:33:07AM +0800, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Silent failure to map and configure CXL BI Route Table due to probe sequence mismatch
> --
> 
> commit 34d3cc24691a25107f1dbbcc8e03fc18ffcae164
> Author: Davidlohr Bueso <dave@stgolabs.net>
> 
> cxl: Add BI register probing and port initialization
> 
> This commit adds register probing for BI Route Table and BI Decoder capability
> structures. It introduces helpers to map these structures at dport-creation
> time and port probe time.
> 
> > diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
> > index ada51948d52fa..2dd712c5d312a 100644
> > --- a/drivers/cxl/port.c
> > +++ b/drivers/cxl/port.c
> [ ... ]
> > @@ -58,6 +58,62 @@ static int discover_region(struct device *dev, void *unused)
> [ ... ]
> > +static void cxl_port_map_bi(struct cxl_port *port)
> > +{
> > +	struct cxl_register_map *map = &port->reg_map;
> [ ... ]
> > +	/* map this port's own BI capability */
> > +	if (is_cxl_endpoint(port)) {
> > +		if (!map->component_map.bi_decoder.valid) {
> > +			dev_dbg(&port->dev, "BI Decoder registers not found\n");
> > +			return;
> > +		}
> > +		cap_id = CXL_CM_CAP_CAP_ID_BI_DECODER;
> > +	} else {
> > +		if (!map->component_map.bi_rt.valid) {
> > +			dev_dbg(&port->dev, "BI RT registers not found\n");
> > +			return;
> > +		}
> 
> [Severity: High]
> Does this silently fail to map the BI Route Table registers for switch
> ports?
> 
> When called from cxl_switch_port_probe(), the component registers have not
> been discovered yet. The registers are actually probed and marked valid when
> the first downstream port is added, which triggers cxl_port_add_dport() to call
> cxl_port_setup_regs().
> 
> Because the valid flag is always false here during switch port probe, the map
> function returns early and permanently leaves port->regs.bi_rt unmapped. Will
> this bypass the required hardware commit protocol for the switch upstream port
> and break Back-Invalidate coherency for downstream endpoints?
> 
> > +		cap_id = CXL_CM_CAP_CAP_ID_BI_RT;
> > +	}
> > +
> > +	map->host = &port->dev;
> > +	if (cxl_map_component_regs(map, &port->regs, BIT(cap_id)))
> > +		dev_dbg(&port->dev, "Failed to map BI capability 0x%x\n",
> > +			cap_id);
> > +}
> > +
> >  static int cxl_switch_port_probe(struct cxl_port *port)
> >  {
> >  	/* Reset nr_dports for rebind of driver */
> [ ... ]
> > @@ -66,6 +122,8 @@ static int cxl_switch_port_probe(struct cxl_port *port)
> >  	/* Cache the data early to ensure is_visible() works */
> >  	read_cdat_data(port);
> >  
> > +	cxl_port_map_bi(port);
> > +
> >  	return 0;
> >  }
>

Hi Davidlohr,

cxl_port_map_bi() is called here before any dport exists.
Component register discovery happends later in cxl_port_add_dport() and then
cxl_port_setup_regs() on the first dport.

So for switch USP, bi_rt.valid is false at probe time, mapping is skipped
and never retried. I saw from your cover letter's dmesg

"""
cxl_port port2: BI RT registers not found   ← probe-time map_bi
...
cxl_port port2: found BI RT capability      ← later, on first dport
"""

Endpoints and RP/DSP paths are fine, but switch BI RT mapping might
be the gap.

Let me know what you think, please correct me if I'm wrong, thanks


Best regards,
Richard Cheng.
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260605003329.2584012-1-dave@stgolabs.net?part=1
>