Re: [PATCH v3 1/2] cxl/hdm: Allow zero sized HDM decoders

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Unbalanced lifecycle operation: manually advancing `port->hdm_end` for a zero-size decoder permanently breaks DPA resource freeing for all preceding decoders.
- [High] The patch modifies the port's DPA watermark (`port->hdm_end`) for zero-sized decoders without acquiring the required `cxl_rwsem.dpa` lock.
--

commit 9ef90cd8aa01ad12f98b0af61f61ae8863b14582
Author: Richard Cheng <icheng@nvidia.com>

cxl/hdm: Allow zero sized HDM decoders

This patch permits committing an HDM decoder with a size of 0. It enumerates
the decoder into the topology and manually advances the port's DPA watermark
to pass in-order checks without reserving actual DPA space.

> 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
[ ... ]
> @@ -1096,6 +1100,15 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
>  	if (!committed)
>  		return 0;
>  
> +	/*
> +	 * A committed zero-size decoder reserves no DPA, but still advance
> +	 * the port's DPA watermark.
> +	 */
> +	if (size == 0) {
> +		port->hdm_end = cxld->id;

[Severity: High]
Does modifying port->hdm_end here require the cxl_rwsem.dpa lock?

It appears other reads and writes to port->hdm_end are protected by the DPA
read-write semaphore. Since devm_cxl_enumerate_decoders() iteratively adds
decoders and exposes them to sysfs before the enumeration loop finishes,
could a concurrent sysfs operation on a previously added decoder race with
this assignment?

[Severity: High]
Could advancing port->hdm_end without registering a devres cleanup action
break DPA freeing for earlier decoders?

Normally devm_cxl_dpa_reserve() registers a cleanup that manages teardown.
If we advance port->hdm_end here but cxl_dpa_free() exits early when it sees
no DPA resource:

cxl_dpa_free() {
        if (!cxled->dpa_res)
                return 0;
        ...
        if (cxled->cxld.id != port->hdm_end) {
                return -EBUSY;
        }
        ...
}

Will port->hdm_end remain permanently trapped at this zero-size decoder's
ID, preventing the strict LIFO un-provisioning of any preceding valid
decoders and breaking manual sysfs teardown?

> +		return 0;
> +	}
> +
>  	dpa_size = div_u64_rem(size, cxld->interleave_ways, &remainder);

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