Re: [PATCH v1 03/14] cxl/region: Add dynamic DAX region support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] cxl/region: missing cxled_dcd_enabled() check in cxl_region_attach()
--

commit fbb76db124dfcb143d519163ab21665f41cdcfa6
Author: Ira Weiny <iweiny@kernel.org>

cxl/region: Add dynamic DAX region support

This commit introduces dynamic DAX region support for CXL dynamic capacity
devices (DCD), tracking shareable and read-only partition properties on
CXL_PARTMODE_DYNAMIC_RAM regions and creating DAX regions with
IORESOURCE_DAX_DCD and a zero-sized seed device.

> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index f4a9839c73d2e..fc9139ce358fe 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c

[ ... ]

> @@ -2080,6 +2087,20 @@ static int cxl_region_attach(struct cxl_region *cxlr,
>  		return -EINVAL;
>  	}
>  
> +	/* A DC region's flavor is fixed; every member partition must match */
> +	if (cxlr->mode == CXL_PARTMODE_DYNAMIC_RAM) {
> +		struct cxl_dpa_partition *part = &cxlds->part[cxled->part];
> +
> +		if (part->shareable != cxlr->shareable ||
> +		    part->read_only != cxlr->read_only) {
> +			dev_dbg(&cxlr->dev, "%s: partition is %s, region is %s\n",
> +				dev_name(&cxled->cxld.dev),
> +				cxl_dc_mode_name(part->shareable, part->read_only),
> +				cxl_dc_mode_name(cxlr->shareable, cxlr->read_only));
> +			return -EINVAL;
> +		}
> +	}
> +
>  	/* all full of members, or interleave config not established? */
>  	if (p->state > CXL_CONFIG_INTERLEAVE_ACTIVE) {
>  		dev_dbg(&cxlr->dev, "region already active\n");
> @@ -2411,6 +2432,7 @@ static size_t store_targetN(struct cxl_region *cxlr, const char *buf, int pos,
>  	if (sysfs_streq(buf, "\n"))
>  		rc = detach_target(cxlr, pos);
>  	else {
> +		struct cxl_endpoint_decoder *cxled;
>  		struct device *dev;
>  
>  		dev = bus_find_device_by_name(&cxl_bus_type, NULL, buf);
> @@ -2422,8 +2444,14 @@ static size_t store_targetN(struct cxl_region *cxlr, const char *buf, int pos,
>  			goto out;
>  		}
>  
> -		rc = attach_target(cxlr, to_cxl_endpoint_decoder(dev), pos,
> -				   TASK_INTERRUPTIBLE);
> +		cxled = to_cxl_endpoint_decoder(dev);
> +		if (cxlr->mode == CXL_PARTMODE_DYNAMIC_RAM &&
> +		    !cxled_dcd_enabled(cxled)) {
> +			dev_dbg(dev, "DCD not enabled\n");
> +			rc = -ENXIO;
> +			goto out;
> +		}
> +		rc = attach_target(cxlr, cxled, pos, TASK_INTERRUPTIBLE);
>  out:
>  		put_device(dev);
>  	}

[Severity: Medium]
Should the cxled_dcd_enabled(cxled) check live inside the
cxlr->mode == CXL_PARTMODE_DYNAMIC_RAM block in cxl_region_attach() instead of
store_targetN()?

In the auto-discovery path, cxl_add_to_region() looks up an existing region
before calling construct_region():

drivers/cxl/core/region.c:cxl_add_to_region() {
	...
	struct cxl_region *cxlr __free(put_cxl_region) =
		cxl_find_region_by_range(cxlrd, &ctx.hpa_range);
	if (!cxlr)
		cxlr = construct_region(cxlrd, &ctx);

	rc = PTR_ERR_OR_ZERO(cxlr);
	if (rc)
		return rc;

	attach_target(cxlr, cxled, -1, TASK_UNINTERRUPTIBLE);
	...
}

When cxl_find_region_by_range() returns an existing region, construct_region()
is skipped and cxl_add_to_region() calls
attach_target() -> __attach_target() -> cxl_region_attach() directly.

For example, if an auto-discovered DC region has its endpoint detached (where
__cxl_decoder_detach() lowers p->state to CXL_CONFIG_INTERLEAVE_ACTIVE while
leaving cxlr registered under cxlrd) and DCD is not enabled when the endpoint
port rebinds, or if a second endpoint with DCD disabled matches a region
created by an earlier endpoint during multi-way auto-discovery,
cxl_add_to_region() will find the existing cxlr and attach and commit cxled
without checking cxled_dcd_enabled(cxled).

Checking !cxled_dcd_enabled(cxled) in cxl_region_attach() (alongside
construct_region()) would cover all target attachment paths under
cxl_rwsem.region and cxl_rwsem.dpa and avoid duplicating the check in
store_targetN().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104230445.66625-1-anisa.su@samsung.com?part=3