Re: [PATCH v24 06/11] cxl/hdm: Add support for getting region from committed decoder

Jonathan Cameron
2026-03-24T16:48:18Z

On Mon, 23 Mar 2026 11:31:12 +0000
alejandro.lucero-palau@amd.com wrote:

> From: Alejandro Lucero <alucerop@amd.com>
>
> A Type2 device configured by the BIOS can already have its HDM
> committed. Add a cxl_get_committed_decoder() function for cheking
Spell check commit messages. checking if there is one

> so after memdev creation. A CXL region should have been created
> during memdev initialization, therefore a Type2 driver can ask for
> such a region for working with the HPA. If the HDM is not committed,
> a Type2 driver will create the region after obtaining proper HPA
> and DPA space.
>
> Signed-off-by: Alejandro Lucero <alucerop@amd.com>

I'm not following the logic below. Perhaps some comments would help
me understand why the condition is sufficient.

> ---
> drivers/cxl/core/hdm.c | 39 +++++++++++++++++++++++++++++++++++++++
> include/cxl/cxl.h | 3 +++
> 2 files changed, 42 insertions(+)
>
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index c222e98ae736..006be88efaa5 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -686,6 +686,45 @@ int cxl_dpa_alloc(struct cxl_endpoint_decoder *cxled, u64 size)
> return devm_add_action_or_reset(&port->dev, cxl_dpa_release, cxled);
> }
>
> +static int find_committed_endpoint_decoder(struct device *dev, const void *data)
> +{
> + struct cxl_endpoint_decoder *cxled;
> + struct cxl_port *port;
> +
> + if (!is_endpoint_decoder(dev))
> + return 0;
> +
> + cxled = to_cxl_endpoint_decoder(dev);
> + port = cxled_to_port(cxled);
> +
> + return cxled->cxld.id == port->hdm_end;

This condition is non obvious enough that to me it maybe needs a comment on
why it works.

> +}
> +
> +struct cxl_endpoint_decoder *cxl_get_committed_decoder(struct cxl_memdev *cxlmd,
> + struct cxl_region **cxlr)
> +{
> + struct cxl_port *endpoint = cxlmd->endpoint;
> + struct cxl_endpoint_decoder *cxled;
> + struct device *cxled_dev;
> +
> + if (!endpoint)
> + return NULL;
> +
> + guard(rwsem_read)(&cxl_rwsem.dpa);

struct device *cxled_dev __free(device) =
device_find_child(&...)

or something along those lines to avoid need for manual put below.

> + cxled_dev = device_find_child(&endpoint->dev, NULL,
> + find_committed_endpoint_decoder);
> +
> + if (!cxled_dev)
> + return NULL;
> +
> + cxled = to_cxl_endpoint_decoder(cxled_dev);
> + *cxlr = cxled->cxld.region;
> +
> + put_device(cxled_dev);

> + return cxled;
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_get_committed_decoder, "CXL");