Re: [PATCH v5 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:

New issues:
- [High] cxl_dpa_free() incorrectly bypasses teardown for zero-sized decoders, permanently locking hdm_end and breaking the LIFO DPA teardown sequence.

Pre-existing issues:
- [High] Pre-existing bug in cxl_get_poison_unmapped: prematurely breaking out of the partition loop skips poison scanning for unmapped partitions.
--

commit 66fbe5db5d8f3f27534b45af8c4ef71b88ab7f3e
Author: Richard Cheng <icheng@nvidia.com>

cxl/hdm: Allow zero sized HDM decoders

Treat empty decoders as first class instead of special casing them, back
them with a kmalloc'd resource, since the resource tree can't represent
an empty range, and keep the skip and hdm_end accounting intact. Guard
the paths an empty decoder can't serve, e.g. region attach, DPA free, and
poison queries.

> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index 0c80b76a5f9b4..ccbab2e21f060 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
[ ... ]
> @@ -545,7 +569,7 @@ int cxl_dpa_free(struct cxl_endpoint_decoder *cxled)
>  	struct device *dev = &cxled->cxld.dev;
>  
>  	guard(rwsem_write)(&cxl_rwsem.dpa);
> -	if (!cxled->dpa_res)
> +	if (cxled_empty(cxled))
>  		return 0;

[Severity: High]
Does this early return prevent decrementing port->hdm_end for zero-sized
decoders?

If an unlocked zero-sized decoder is freed by userspace (for example, by
writing 0 to dpa_size in sysfs), cxled_empty() returns true because
resource_size(dpa_res) is 0. The function then returns 0 early, skipping
devm_cxl_dpa_release().

Because hdm_end is decremented only inside __cxl_dpa_release(), bypassing it
leaves port->hdm_end permanently elevated. Would subsequent attempts to free
lower-indexed decoders fail the cxled->cxld.id != port->hdm_end check with
-EBUSY, preventing further dynamic DPA reconfiguration?

>  	if (cxled->cxld.region) {
>  		dev_dbg(dev, "decoder assigned to: %s\n",
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index e50dc716d4e82..eeecb588821a4 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
[ ... ]
> @@ -2953,9 +2960,17 @@ int cxl_get_poison_by_endpoint(struct cxl_port *port)
>  	};
>  
>  	rc = device_for_each_child(&port->dev, &ctx, poison_by_decoder);
> -	if (rc == 1)
> +	if (rc == 1) {
> +		/*
> +		 * No decoder with a sized DPA reservation was walked
> +		 * (every committed decoder is zero-size): scan all
> +		 * partitions in full.
> +		 */
> +		if (ctx.part < 0)
> +			ctx.part = 0;
>  		rc = cxl_get_poison_unmapped(to_cxl_memdev(port->uport_dev),
>  					     &ctx);

[Severity: High]
This is a pre-existing issue, but does cxl_get_poison_unmapped() prematurely
break out of its partition loop when encountering a fully mapped partition?

Looking at cxl_get_poison_unmapped() in drivers/cxl/core/region.c:

for (int i = ctx->part; i < cxlds->nr_partitions; i++) {
        ...
        length = res->end - offset + 1;
        if (!length)
                break;
        rc = cxl_mem_get_poison(...);

If it processes a partition that is fully mapped, length becomes 0, and the
break statement completely terminates the outer loop over the remaining
partitions instead of moving to the next one. Should this be a continue
instead of a break so that poison in subsequent unmapped partitions isn't
silently ignored?

> +	}
>  
>  	return rc;
>  }

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