Re: [PATCH v14 2/8] cxl/mem: Read dynamic capacity configuration from the device

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] cxl/mem: cxl_configure_dcd() incorrectly treats Total Capacity as static capacity boundary
--

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

cxl/mem: Read dynamic capacity configuration from the device

This commit implements the Dynamic Capacity (DC) mailbox commands to read 
and store CXL DCD configuration information. It validates the first 
partition and ensures there are no gaps or overlaps before configuring it.

> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index b3419df586b9c..15df8db314d90 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -643,6 +643,54 @@ int cxl_mem_dpa_fetch(struct cxl_memdev_state *mds, struct cxl_dpa_info *info)

[ ... ]

> +int cxl_configure_dcd(struct cxl_memdev_state *mds, struct cxl_dpa_info *info)
> +{
> +	struct cxl_dc_partition_info dc_info = { };
> +	struct device *dev = mds->cxlds.dev;
> +	u64 total;
> +	int rc;
> +
> +	if (!mds->cxlds.media_ready)
> +		return 0;
> +
> +	rc = cxl_dev_dc_identify(&mds->cxlds.cxl_mbox, &dc_info);
> +	if (rc) {
> +		dev_warn(dev,
> +			 "Failed to read Dynamic Capacity config: %d\n", rc);
> +		return rc;
> +	}
> +
> +	if (dc_info.start < info->size) {
> +		dev_err(dev,
> +			"DC partition 0 base %#llx overlaps static capacity ending at %#llx\n",
> +			dc_info.start, info->size);
> +		return -EINVAL;
> +	}

[Severity: High]
Does info->size here represent the total capacity (volatile + persistent +
dynamic) rather than just the static capacity?

Since cxl_mem_dpa_fetch() initializes info->size to mds->total_bytes 
(which is the total capacity), comparing the dynamic partition's base 
against this will evaluate to true for spec-compliant devices, 
unconditionally rejecting them as overlaps.

> +
> +	/* A gap between static capacity and the DC partition is not supported */
> +	if (dc_info.start > info->size) {
> +		dev_warn(dev,
> +			 "DC partition 0 base %#llx leaves a gap from static capacity ending at %#llx\n",
> +			 dc_info.start, info->size);
> +		return -EOPNOTSUPP;
> +	}

[Severity: High]
If info->size includes dynamic capacity, would enforcing that the DCD 
partition starts exactly at info->size be physically impossible without 
overlapping the unprovisioned space at the end of the DPA?

> +
> +	if (check_add_overflow(info->size, dc_info.size, &total)) {
> +		dev_err(dev,
> +			"DC partition 0 size %#llx overflows DPA space ending at %#llx\n",
> +			dc_info.size, info->size);
> +		return -EINVAL;
> +	}
> +
> +	info->size = total;

[Severity: High]
Could this lead to double-counting the dynamic capacity?

Since info->size already includes the dynamic capacity per the CXL spec, 
adding dc_info.size here would inflate the value. 

If a non-compliant device bypassed the overlap checks, this inflated size 
could also cause the strict contiguity check in cxl_dpa_setup() to fail 
later during device probe:

drivers/cxl/core/hdm.c:cxl_dpa_setup() {
    ...
    /* Require ordered + contiguous partitions */
    if (i) {
        const struct cxl_dpa_part_info *prev = &info->part[i - 1];

        if (prev->range.end + 1 != part->range.start)
            return -EINVAL;
    }
    ...
}

> +	dev_dbg(dev, "Adding dynamic ram partition 1; %#llx size %#llx\n",
> +		dc_info.start, dc_info.size);
> +	add_part(info, dc_info.start, dc_info.size, CXL_PARTMODE_DYNAMIC_RAM_1);
> +
> +	return 0;
> +}

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