Re: [PATCH v9 2/3] cxl/hdm: Allow zero sized HDM decoders

[Alison Schofield <alison.schofield@intel.com>]

On Wed, Aug 05, 2026 at 01:55:23PM +0800, Richard Cheng wrote:
> CXL r4.0 §8.2.4.20.12 ("Committing Decoder Programming") and §14.13.10
> ("CXL HDM Decoder Zero Size Commit") permit committing an HDM decoder
> with size 0. BIOS may commit and lock such decoders so the OS cannot
> program regions through them, this is a design choice rather than a spec
> requirement.
> 
> The kernel rejected these with -ENXIO during port enumeration and
> aborted the whole port, so affected systems showed nothing under "cxl
> list".
> 
> 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.

Hi Richard,

I applied this set to 7.3-rc1 and retested w the new test case in
cxl-topology.sh - all good.

I do have some comments though - see below 

snip...
I'm snipping past a bunch of Reviewed-by tags a bit sheepishly.
Maybe you'll quickly dispute my feedback?

> 
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index 0c80b76a5f9b..3b3cfa0f2507 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -240,6 +240,18 @@ static resource_size_t __adjust_skip(struct cxl_dev_state *cxlds,
>  }
>  #define release_skip(c, b, l) __adjust_skip((c), (b), (l), NULL)
>  
> +static void cxl_dpa_release_region(struct resource *parent,
> +				   struct resource *res)
> +{
> +	/* zero sized decoders are not tracked in the resource tree */
> +	if (resource_size(res) == 0) {
> +		kfree(res);
> +		return;
> +	}
> +
> +	__release_region(parent, res->start, resource_size(res));
> +}
> +
>  /*
>   * Must be called in a context that synchronizes against this decoder's
>   * port ->remove() callback (like an endpoint decoder sysfs attribute)
> @@ -256,7 +268,7 @@ static void __cxl_dpa_release(struct cxl_endpoint_decoder *cxled)
>  
>  	/* save @skip_start, before @res is released */
>  	skip_start = res->start - cxled->skip;
> -	__release_region(&cxlds->dpa_res, res->start, resource_size(res));
> +	cxl_dpa_release_region(&cxlds->dpa_res, res);
>  	if (cxled->skip)
>  		release_skip(cxlds, skip_start, cxled->skip);
>  	cxled->skip = 0;
> @@ -336,6 +348,26 @@ static int request_skip(struct cxl_dev_state *cxlds,
>  	return -EBUSY;
>  }
>  
> +static struct resource *cxl_dpa_request_region(struct resource *parent,
> +					       resource_size_t start,
> +					       resource_size_t n,
> +					       const char *name)
> +{
> +	if (!n) {
> +		struct resource *res;
> +
> +		res = kmalloc_obj(*res);
> +		if (!res)
> +			return NULL;
> +
> +		*res = DEFINE_RES_NAMED(start, 0, name, IORESOURCE_MEM);
> +
> +		return res;
> +	}
> +
> +	return __request_region(parent, start, n, name, 0);
> +}
> +

2 failures are now collapsing into one -EBUSY 
1) __request_region() returning NULL is a resource conflict.
2) kmalloc_obj() returning NULL is -ENOMEM

Can this preserve the -ENONMEM?  Maybe using ERR_PTR() or handle the
zero size alloc in __cxl_dpa_reserve()?


snip
>  
> @@ -545,7 +572,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;
>  	if (cxled->cxld.region) {
>  		dev_dbg(dev, "decoder assigned to: %s\n",


Is cxled_empty() the right test above?

The old check only returns early when there is no DPA resource.
cxled_empty() also returns true for a zero-sized decoder that does
have a resource, so this can return success before reaching the
CXL_DECODER_F_ENABLE check.

For a committed zero-sized decoder, that means echo 0 > dpa_size can
report success even though the decoder can't actually be freed.

Can the original !cxled->dpa_res check should remain, or the empty
case be handled after the existing state checks.

This also means I don't think "DPA free" belongs in the commit message's
list of paths that need to be guarded.

-- Alison

snip