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

Alejandro Lucero Palau
2026-03-24T21:20:32Z

On 3/24/26 16:48, Jonathan Cameron wrote:
> 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
>

:-)

I'll fix it.

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

Uhmm, yes, that works but because I check such a decoder is linked to a
region inside the sfc driver.

Benjamin Cheatham pointed out the weakness of this and I did a bad job
trying to improve it

.

I think I should check here the decoder state and if a region was auto
discovered based in such a decoder instead than by the driver.

I will fix it.

Thanks.

>> +}
>> +
>> +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");
>