Re: [PATCH v4 2/4] cxl/hdm: Only interpret interleave settings on committed decoders

在 2026/7/21 09:59, Alison Schofield 写道:
> Decoder enumeration reads the interleave ways and granularity fields
> from the CTRL register unconditionally, then decodes and validates
> them for every decoder. On an uncommitted decoder these fields are
> stale. They hold whatever was last latched and are reprogrammed when
> the decoder is committed. Interpreting them serves no purpose, and
> rejecting enumeration on a reserved encoding in those stale bits can
> fail an otherwise usable decoder.
>
> Decode and validate interleave ways and granularity only when the
> decoder is committed. On an uncommitted decoder the stale register
> values are not interpreted. The interleave fields are set when the
> decoder is programmed at commit time.
>
>
> Reported-by: Li Ming <[email protected]>
> Closes: https://lore.kernel.org/all/[email protected]/
> Signed-off-by: Alison Schofield <[email protected]>
> ---
>   drivers/cxl/core/hdm.c | 33 ++++++++++++++++++---------------
>   1 file changed, 18 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index d81df45d8005..1f995191baf5 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -1064,21 +1064,24 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
>   			writel(ctrl, hdm + CXL_HDM_DECODER0_CTRL_OFFSET(which));
>   		}
>   	}
> -	rc = eiw_to_ways(FIELD_GET(CXL_HDM_DECODER0_CTRL_IW_MASK, ctrl),
> -			  &cxld->interleave_ways);
> -	if (rc) {
> -		dev_warn(&port->dev,
> -			 "decoder%d.%d: Invalid interleave ways (ctrl: %#x)\n",
> -			 port->id, cxld->id, ctrl);
> -		return rc;
> -	}
> -	rc = eig_to_granularity(FIELD_GET(CXL_HDM_DECODER0_CTRL_IG_MASK, ctrl),
> -				 &cxld->interleave_granularity);
> -	if (rc) {
> -		dev_warn(&port->dev,
> -			 "decoder%d.%d: Invalid interleave granularity (ctrl: %#x)\n",
> -			 port->id, cxld->id, ctrl);
> -		return rc;
> +	/* Interleave settings are only valid on a committed decoder */
> +	if (committed) {
> +		rc = eiw_to_ways(FIELD_GET(CXL_HDM_DECODER0_CTRL_IW_MASK, ctrl),
> +				 &cxld->interleave_ways);
> +		if (rc) {
> +			dev_warn(&port->dev,
> +				 "decoder%d.%d: Invalid interleave ways (ctrl: %#x)\n",
> +				 port->id, cxld->id, ctrl);
> +			return rc;
> +		}
> +		rc = eig_to_granularity(FIELD_GET(CXL_HDM_DECODER0_CTRL_IG_MASK, ctrl),
> +					&cxld->interleave_granularity);
> +		if (rc) {
> +			dev_warn(&port->dev,
> +				 "decoder%d.%d: Invalid interleave granularity (ctrl: %#x)\n",
> +				 port->id, cxld->id, ctrl);
> +			return rc;
> +		}

There is a "if (committed) {}" block before these changes, maybe we can 
combine them into one.

Besides, there is a piece of codes related to a committed decoder 
handling in the end of init_hdm_decoder(), maybe we should put all 
committed decoder handling codes into one "if {}" block?

>   	}
>   
>   	dev_dbg(&port->dev, "decoder%d.%d: range: %#llx-%#llx iw: %d ig: %d\n",