Re: [PATCH v23 08/22] cxl/hdm: Add support for getting region from committed decoder

[Alejandro Lucero Palau <alucerop@amd.com>]

On 3/9/26 22:49, PJ Waskiewicz wrote:
> On Thu, 2026-02-12 at 09:16 +0000, Alejandro Lucero Palau wrote:
>> On 2/11/26 22:11, Cheatham, Benjamin wrote:
>>> On 2/1/2026 9:54 AM, 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
>>>> 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>
>>>> ---
>>>>    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 6e516c69b2d2..a172ce4e9b19 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;
>>> Is this the way you're supposed to check if a decoder is committed?
>>> The doc comment for @hdm_end in
>>> struct cxl_port says it's just the last allocated decoder. If
>>> allocated decoders are always committed then
>>> I'm fine with this, otherwise I think you'd want to a register read
>>> or something to find the commit state.
>>
>> Hi Ben,
>>
>>
>> Yes, I think you are right. This works in my tests and it is safe
>> because I check the region does exist before using it. But the error
>> inside sfc should then not be fatal for cxl sfc initialization and
>> fallback to the other cxl initialization possibility.
>>
> So I'm running into this situation I think.
>
> When you're testing, are you surviving a reload of the driver?  Right
> now, I can load and successfully create the region0 device.  However,
> following the same teardown path in SFC, I cannot reload my driver
> afterwards and map the region.  I get:
>
> cxl_port endpoint5: failed to attach decoder5 to region0: -6  (ENXIO)
> <driver> 0000:c1:00.0: CXL found committed decoder without a region
> <driver> 0000:c1:00.0: CXL init failed
>
> I'd be surprised if SFC in its current patch state would survive this
> same insmod/rmmod/insmod test.


Yes, I can load and unload the sfc driver and always getting the HDM 
decoder committed, what was the purpose behind v23.


I wonder if you applied the patch from smita's series:


https://lore.kernel.org/linux-cxl/20260210064501.157591-1-Smita.KoralahalliChannabasappa@amd.com/T/#mdad81d3817def8baace77ead9e2e305e775cf51d


If you did, then not sure what could be happening. Could you post here 
what you see at /sys/bus/cxl/devices after loading your driver, and 
after unloading it?


>
> -PJ