Re: [PATCH v10 02/12] cxl: Pass decoder settings to HDM commit helpers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] New pre-checks in `setup_hw_decoder()` permanently wedge the decoder after an error and cause spurious failures during quick reset-recommit cycles.
--

--- Patch [2]: [PATCH v10 02/12] cxl: Pass decoder settings to HDM commit helpers ---
commit e130015805d1a3746154432a62193b6dcb3f71f9
Author: Srirangan Madhavan <smadhavan@nvidia.com>

cxl: Pass decoder settings to HDM commit helpers

Introduce struct cxl_decoder_settings as a plain snapshot of HDM decoder
programming state and pass it to the shared commit helpers. Split the helper
entry points into cxl_commit_start() and cxl_commit_wait() so hdm.c keeps
the existing DPA-lock and commit policy flow while later reset restore code
can reuse the register programming sequence.

> diff --git a/drivers/cxl/core/resource.c b/drivers/cxl/core/resource.c
> index dd8bed3d3ff0c..dd5e0cc82da49 100644
> --- a/drivers/cxl/core/resource.c
> +++ b/drivers/cxl/core/resource.c

[ ... ]

> +static int setup_hw_decoder(struct cxl_decoder_settings *settings,
> +			    void __iomem *hdm)
>  {
> -	int id = cxld->id;
> +	int id = settings->id;
> +	u64 target_or_skip;
>  	u64 base, size;
>  	u32 ctrl;
>  
> -	/* common decoder settings */
> -	ctrl = readl(hdm + CXL_HDM_DECODER0_CTRL_OFFSET(cxld->id));
> -	cxld_set_interleave(cxld, &ctrl);
> -	cxld_set_type(cxld, &ctrl);
> -	base = cxld->hpa_range.start;
> -	size = range_len(&cxld->hpa_range);
> +	ctrl = readl(hdm + CXL_HDM_DECODER0_CTRL_OFFSET(id));
> +	if (ctrl & CXL_HDM_DECODER0_CTRL_COMMITTED)
> +		return -EBUSY;
> +	if (ctrl & CXL_HDM_DECODER0_CTRL_COMMIT)
> +		return -ETIMEDOUT;
> +	if (ctrl & CXL_HDM_DECODER0_CTRL_COMMIT_ERROR)
> +		return -EIO;

[Severity: High]
Will these new pre-checks permanently wedge the decoder if a previous commit
attempt timed out or encountered an error?

If a commit times out, cxld_await_commit() returns -ETIMEDOUT leaving the
CXL_HDM_DECODER0_CTRL_COMMIT bit set. Because CXL_DECODER_F_ENABLE is not set
on failure, cxl_decoder_reset() refuses to run, so the COMMIT bit is never
cleared.

When a subsequent commit attempt occurs, won't it immediately return
-ETIMEDOUT from these checks without attempting to clear the error?

Similarly, for quick reset-recommit cycles, cxl_decoder_reset() clears
the COMMIT bit but does not wait for the hardware to clear COMMITTED.
Could this leave the hardware in a transient state that triggers the newly
added -EBUSY check?

> +	cxld_set_interleave(settings, &ctrl);
> +	cxld_set_type(settings, &ctrl);
> +	base = settings->hpa_range.start;
> +	size = range_len(&settings->hpa_range);

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804192958.1823952-1-smadhavan@nvidia.com?part=2