Re: [PATCH v3 1/3] cxl/hdm: Reject switch decoder interleave ways that overflow targets

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

Pre-existing issues:
- [High] The `target_list` union is not endian-safe, causing corrupted target mapping on big-endian architectures.
--

--- Patch [1]: [PATCH v3 1/3] cxl/hdm: Reject switch decoder interleave ways that overflow targets ---
commit 4abc7b385aec9c1d79e37f3577a9009b393c2333
Author: Alison Schofield <[email protected]>

cxl/hdm: Reject switch decoder interleave ways that overflow targets

This patch validates switch decoder enumeration to ensure the interleave
ways encoding fits the available targets. It prevents overrunning the
target arrays during enumeration and handles uncommitted decoders by ignoring
stale interleave ways.

Link: https://sashiko.dev/#/patchset/[email protected]?part=1

> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -1084,6 +1086,37 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,

[ ... ]

> +		if (cxld->interleave_ways > cxlsd->nr_targets) {
> +			dev_err(&port->dev,
> +				"decoder%d.%d: interleave ways: %d exceeds targets: %d\n",
> +				port->id, cxld->id, cxld->interleave_ways,
> +				cxlsd->nr_targets);
> +			return -ENXIO;
> +		}
> +
>  		lo = readl(hdm + CXL_HDM_DECODER0_TL_LOW(which));
>  		hi = readl(hdm + CXL_HDM_DECODER0_TL_HIGH(which));
>  		target_list.value = (hi << 32) + lo;

[Severity: High]
This is a pre-existing issue, but does this union overlay create an implicit
memory dependency on the CPU's byte order?

In init_hdm_decoder(), the target mapping extracts bytes via the target_id
array based on memory layout:

    target_list.value = (hi << 32) + lo;
    for (i = 0; i < cxld->interleave_ways; i++)
        cxld->target_map[i] = target_list.target_id[i];

On big-endian architectures, byte 0 of the 64-bit integer corresponds to the
most significant byte (target 7) instead of the least significant byte
(target 0).

Does this completely reverse the target mapping, potentially routing memory
transactions to the wrong CXL targets and leading to memory corruption?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1