Re: [PATCH v4 3/4] cxl/hdm: Make switch decoder target parsing endian-safe

On Mon, 20 Jul 2026 18:59:39 -0700
Alison Schofield <[email protected]> wrote:

> Switch decoder target IDs are stored one per byte, starting with
> target 0 in the least significant byte. The current code accesses
> those bytes in memory order, which only matches register order on
> little-endian hosts.
> 
> On big-endian hosts, the target IDs are reversed and the decoder is
> associated with the wrong downstream ports.
> 
> Extract each target ID from the register value with an explicit shift
> and mask so the target mapping is independent of host endianness.
> 
> Fixes: d17d0540a0db ("cxl/core/hdm: Add CXL standard decoder enumeration to the core")
> Reviewed-by: Richard Cheng <[email protected]>
> Reviewed-by: Li Ming <[email protected]>
> Signed-off-by: Alison Schofield <[email protected]>

Suggestion for explicitly handling the endian issue inline.
I'm also fine if you stick with what you have here.

Reviewed-by: Jonathan Cameron <[email protected]>

> ---
>  drivers/cxl/core/hdm.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index 1f995191baf5..077aface1a2a 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -976,14 +976,11 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
>  {
>  	struct cxl_endpoint_decoder *cxled = NULL;
>  	u64 size, base, skip, dpa_size, lo, hi;
> +	u64 target_list;
>  	bool committed;
>  	u32 remainder;
>  	int i, rc;
>  	u32 ctrl;

Maybe alternative approach along the lines of:

> -	union {
> -		u64 value;
		struct {
			__le32 lo;
			__le32 hi;
		};
> -		unsigned char target_id[8];
> -	} target_list;
>  
>  	if (should_emulate_decoders(info))
>  		return cxl_setup_hdm_decoder_from_dvsec(port, cxld, dpa_base,
> @@ -1122,9 +1119,9 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
>  
>  		lo = readl(hdm + CXL_HDM_DECODER0_TL_LOW(which));
>  		hi = readl(hdm + CXL_HDM_DECODER0_TL_HIGH(which));

		target_list.lo = cpu_to_le32(readl());
		target_list.hi = cpu_to_le32(readl());

or just have a byte array and

		put_unaligned_le32(&target_id[0], readl());
		put_unaligned_le32(&target_id[4], readl());

> -		target_list.value = (hi << 32) + lo;
> +		target_list = (hi << 32) + lo;
>  		for (i = 0; i < cxld->interleave_ways; i++)
> -			cxld->target_map[i] = target_list.target_id[i];
> +			cxld->target_map[i] = (target_list >> (i * 8)) & 0xff;
>  
>  		return 0;
>  	}