Re: [PATCH 12/15] cxl/cache: Add snoop filter creation and set up

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] cxl/cache: devres leak on dport hot-remove due to devm group mismatch
--

commit a51df73f15bcf70227ccbfa7fe083fcb3c6ed6e3
Author: Ben Cheatham <Benjamin.Cheatham@amd.com>

cxl/cache: Add snoop filter creation and set up

This commit adds the capability to track the system's CXL snoop filters and
probes them during dport addition. Snoop filter capacity allocation will be
handled in a later patch.

> diff --git a/drivers/cxl/core/cache.c b/drivers/cxl/core/cache.c
> --- a/drivers/cxl/core/cache.c
> +++ b/drivers/cxl/core/cache.c
[ ... ]
> +int cxl_dport_probe_snoop_filter(struct cxl_dport *dport)
> +{
[ ... ]
> +	rc = cxl_map_component_regs(&dport->reg_map, &dport->regs.component,
> +				    BIT(CXL_CM_CAP_CAP_ID_SNOOP));
[ ... ]
> diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
> --- a/drivers/cxl/port.c
> +++ b/drivers/cxl/port.c
> @@ -301,6 +301,16 @@ static struct cxl_dport *cxl_port_add_dport(struct cxl_port *port,
>  	/* New dport added, update the decoder targets */
>  	cxl_port_update_decoder_targets(port, dport);
>  
> +	/* 
> +	 * cxl_cachedevs won't probe if this fails, but it's not an error for
> +	 * cxl_memdevs
> +	 */
> +	rc = cxl_dport_probe_snoop_filter(dport);

[Severity: Medium]
Does this code leak devres memory and virtual address space on dport
hot-remove?

This function call happens outside of the dport devres group, which was
already closed when __devm_cxl_add_dport() returned (and the temporary
port_dr_group is removed earlier in this function).

When cxl_dport_probe_snoop_filter() is called, it triggers a devm
allocation:

cxl_dport_probe_snoop_filter()
  -> cxl_map_component_regs()
    -> devm_cxl_iomap_block()

Because there is no active devres group specifically scoped to the dport
at this point, will this devm allocation attach directly to the parent
port instead? If so, the allocation won't be freed when the dport is
hot-removed, leading to a resource leak.

> +	if (rc)
> +		dev_info(dport->dport_dev,
> +			 "Failed to find or create a CXL snoop filter: %d\n",
> +			 rc);
> +
>  	dev_dbg(&port->dev, "dport%d:%s added\n", dport->port_id,
>  		dev_name(dport_dev));
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923173342.5584-1-Benjamin.Cheatham@amd.com?part=12