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

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 d=
evice 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 pas=
s validation.
--

--- Patch [2]: [PATCH v12 2/8] cxl/mem: Read dynamic capacity configuration=
 from the device ---
commit 7a0233410b5ae133da2a756fa11cf61b2145cace
Author: Ira Weiny <[email protected]>

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 =3D le64_to_cpu(dev_part->block_size);

[ ... ]

> +	if (blk_size =3D=3D 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 o=
f 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 =3D 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 =3D=3D 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 =3D=3D 0), which causes this path to log an error and abort
initialization with -EIO.

> +
> +		num_partitions +=3D rc;

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260731084901.1512=
[email protected]?part=3D2