Re: [PATCH v3 1/9] cxl/region: Factor port target calculations

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The target count check `iw > 8` incorrectly blocks 16-way interleaving for switch decoders, contradicting its own documentation.
--

--- Patch [1]: [PATCH v3 1/9] cxl/region: Factor port target calculations ---
commit a455145f1f8fa170cab75f1f658966ee9cf551ab
Author: Alison Schofield <alison.schofield@intel.com>

cxl/region: Factor port target calculations

This commit refactors calculations for interleave fan-out, port decoder
granularity, and endpoint distance into dedicated helpers. It also removes
parent decoder validation from this path.

> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 1e211542b6b64..1082db7b2cca9 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c

[ ... ]

>  static int cxl_port_setup_targets(struct cxl_port *port,
>  				  struct cxl_region *cxlr,
>  				  struct cxl_endpoint_decoder *cxled)
>  {

[ ... ]

>  	/*
>  	 * While root level decoders support x3, x6, x12, switch level
>  	 * decoders only support powers of 2 up to x16.
>  	 */
> -	if (!is_power_of_2(cxl_rr->nr_targets)) {
> +	if (!is_power_of_2(iw)) {
>  		dev_dbg(&cxlr->dev, "%s:%s: invalid target count %d\n",
> -			dev_name(port->uport_dev), dev_name(&port->dev),
> -			cxl_rr->nr_targets);
> +			dev_name(port->uport_dev), dev_name(&port->dev), iw);
>  		return -EINVAL;
>  	}

[ ... ]

> +	if (iw > 8 || iw > cxlsd->nr_targets) {
> +		dev_dbg(&cxlr->dev,
> +			"%s:%s:%s: ways: %d overflows targets: %d\n",
> +			dev_name(port->uport_dev), dev_name(&port->dev),
> +			dev_name(&cxld->dev), iw, cxlsd->nr_targets);
> +		return -ENXIO;
> +	}

[Severity: High]
This is a pre-existing issue, but does this hardcoded limit incorrectly
prevent 16-way interleaving for switch decoders?

The comment above this validation states that switch level decoders support
powers of 2 up to x16. However, if iw is 16, wouldn't the iw > 8 condition
evaluate to true and unconditionally fail the region setup by returning
-ENXIO?

Could this limit be updated to allow iw up to 16 to match the hardware
capabilities and documentation?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785444498.git.alison.schofield@intel.com?part=1