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

[Dan Williams (nvidia)]

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>

[..]
> 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.
 */

> +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