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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] False-positive error logging and initialization failure when a device has 0 dynamic capacity partitions configured.
- [Medium] The validation check `is_power_of_2(blk_size)` truncates the 64-bit block size on 32-bit architectures, allowing invalid block sizes to pass validation.
--

--- Patch [2]: [PATCH v12 2/8] cxl/mem: Read dynamic capacity configuration from the device ---
commit 7a0233410b5ae133da2a756fa11cf61b2145cace
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 the DCD configuration information from a CXL device. It reads
and validates all partitions but configures only the first one.

> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c

[ ... ]

> +static int cxl_dc_check(struct device *dev, struct cxl_dc_partition_info *part_array,
> +			u8 index, struct cxl_dc_partition *dev_part)
> +{
> +	u64 blk_size = le64_to_cpu(dev_part->block_size);

[ ... ]

> +	if (blk_size == 0 || blk_size % CXL_DCD_BLOCK_LINE_SIZE ||
> +	    !is_power_of_2(blk_size)) {

[Severity: Medium]
Could passing the 64-bit blk_size to is_power_of_2() cause silent truncation
on 32-bit architectures?

is_power_of_2() takes an unsigned long, which might allow an invalid 64-bit
block size that is not a power of 2 (but whose lower 32 bits form a power of 2)
to bypass the validation check.

> +		dev_err(dev, "DC partition %d invalid block size %llu\n",
> +			index, blk_size);
> +		return -EINVAL;
> +	}

[ ... ]

> +int cxl_dev_dc_identify(struct cxl_mailbox *mbox,
> +			struct cxl_dc_partition_info *dc_info)
> +{

[ ... ]

> +	do {
> +		int rc, i, j;
> +
> +		rc = cxl_get_dc_config(mbox, start_partition, partition_count,
> +				       dc_resp, dc_resp_size);
> +		if (rc < 0) {
> +			dev_err(dev, "Failed to get DC config: %d\n", rc);
> +			return rc;
> +		}
> +
> +		if (rc == 0) {
> +			dev_err(dev,
> +				"Device reported %u partitions available but returned none at index %u\n",
> +				dc_resp->avail_partition_count, start_partition);
> +			return -EIO;
> +		}

[Severity: Medium]
Does this code incorrectly fail initialization when a valid CXL device has 0
dynamic capacity partitions configured?

Since this loop is implemented as a do-while loop, it will always execute at
least once. If a device has 0 available partitions, it will return 0
partitions (rc == 0), which causes this path to log an error and abort
initialization with -EIO.

> +
> +		num_partitions += rc;

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