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

[Richard Cheng]

On Tue, Jun 23, 2026 at 01:13:40PM +0800, Dan Williams (nvidia) wrote:
> 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.
> > 
> > Suggested-by: Dan Williams <djbw@xxxxxxxxxx>
> > Signed-off-by: Vishal Aslot <vaslot@xxxxxxxxxx>
> > Signed-off-by: Richard Cheng <icheng@xxxxxxxxxx>
> 
> Looks good, you added the cxled_empty() helper and dropped the dev_dbg()
> announcing the arrival of committed empty decoders like we chatted
> about.
> 
> The comment on cxled_empty() could use come adjustment, but maybe Dave
> can fix that up on applying.
> 
> Reviewed-by: Dan Williams <djbw@xxxxxxxxxx>
>

Hi Dan,

I'll send v6 to tweak these comments for easier work for Dave.
 
> [..]
> > diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> > index 1297594beaec..5231345ff78e 100644
> > --- a/drivers/cxl/cxl.h
> > +++ b/drivers/cxl/cxl.h
> > @@ -324,6 +324,15 @@ struct cxl_endpoint_decoder {
> >  	int pos;
> >  };
> >  
> > +/*
> > + * Some BIOS use locked empty decoders to preclude HDM decode aliasing
> > + * for TSP operation. Use cxled_empty() to handle that common case.
> > + */
> 
> Hmm, that is not the "common case".
> 
> /*
>  * The common case is decoders with no reservation, but also handle
>  * decoders with a zero-sized reservation that firmware may install for
>  * security lockdown purposes.
>  */
> 

No problem, amend it.

--Richard

> > +static inline bool cxled_empty(struct cxl_endpoint_decoder *cxled)
> > +{
> > +	return !cxled->dpa_res || !resource_size(cxled->dpa_res);
> > +}
> > +
> >  /**
> >   * struct cxl_switch_decoder - Switch specific CXL HDM Decoder
> >   * @cxld: base cxl_decoder object