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

[Richard Cheng <icheng@nvidia.com>]

On Mon, Jun 15, 2026 at 05:35:41PM +0800, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] devm mappings for dport BI registers leak on hotplug due to being called outside the dport devres group in cxl_port_add_dport.
> --
> 
> commit a836080fdf13fa334055f7f13a95491b66ef793f
> 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 the BI Decoder of a downstream port
> at dport-creation time and a port's own BI capability during port probe.
> 
> > diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
> > index ada51948d52fa..ca8d8c14b7876 100644
> > --- a/drivers/cxl/port.c
> > +++ b/drivers/cxl/port.c
> 
> [ ... ]
> 
> > @@ -261,6 +321,17 @@ static struct cxl_dport *cxl_port_add_dport(struct cxl_port *port,
> >  	/* This group was only needed for early exit above */
> >  	devres_remove_group(&port->dev, no_free_ptr(port_dr_group));
> >  
> > +	if (dev_is_pci(dport_dev)) {
> > +		switch (pci_pcie_type(to_pci_dev(dport_dev))) {
> > +		case PCI_EXP_TYPE_ROOT_PORT:
> > +		case PCI_EXP_TYPE_DOWNSTREAM:
> > +			cxl_dport_map_bi(dport);
> 
> [Severity: High]
> Does this leak devm mappings if the dport is hot-removed?
> 
> Looking at cxl_port_add_dport(), devm_cxl_add_dport_by_dev() creates a
> devres group for the dport on the parent port's device. However,
> cxl_dport_map_bi() is called after devm_cxl_add_dport_by_dev() returns,
> meaning the devres group has already been closed.
> 
> Because cxl_dport_map_bi() calls devm_cxl_iomap_block() against the parent
> port device, these devm allocations bypass the dport group and attach
> directly to the parent port device.
> 
> If the dport is hot-removed, the dport devres group is released, but the BI
> capability mappings appear to be left behind. If the dport is subsequently
> re-added, won't devm_request_mem_region() fail with -EBUSY due to the
> physical address region conflict, permanently breaking BI capabilities?
>

Hi Davidlohr,

I think sashiko-bot's is right on this part, this part runs after devm_cxl_add_dport_by_dev()
has already closed the dport's devres group, so the BI request_mem_region()/ioremap() land on
the port's devres list outside that group. On runtime dport removal the group release won't free
them, and re-adding the dport then fails the BI region request with -EBUSY.

The dport RAS mapping is done inside __devm_cxl_add_dport() while the group is still open for
this reason.
I would suggest mapping BI the same way which is to move cxl_dport_map_bi() into __devm_cxl_add_dport()
next to the RAS setup, and set map->host like RAS does.

What do you think ?

Best regards,
Richard Cheng.
 
> > +			break;
> > +		default:
> > +			break;
> > +		}
> > +	}
> > +
> >  	cxl_switch_parse_cdat(dport);
> >  
> >  	/* New dport added, update the decoder targets */
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260615145529.13848-1-dave@stgolabs.net?part=1
>