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

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

On 12/18/25 15:03, Jonathan Cameron wrote:
> On Thu, 18 Dec 2025 11:52:58 +0000
> Alejandro Lucero Palau <alucerop@amd.com> wrote:
>
>> Hi Jonathan,
>>
>>
>> On 12/15/25 13:50, Jonathan Cameron wrote:
>>> On Fri, 5 Dec 2025 11:52:34 +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
>>> checking if this is so after memdev creation.
>>>   
>>>> 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>
>>> Hi Alejandro,
>>>
>>> I'm in two minds about this.  In general there are devices that have
>>> been configured by the BIOS because they are already in use. I'm not sure
>>> the driver you are working with here is necessarily set up to survive
>>> that sort of live setup without interrupting data flows.
>>
>> This is not mainly about my driver/device but something PJ and Dan agree
>> on support along this type2 patchset.
>>
>> You can see the v21 discussions, but basically PJ can not have his
>> driver using the committed decoders from BIOS. So this change addresses
>> that situation which my driver/device can also benefit from as current
>> BIOS available is committing decoders regardless of UEFI flags like
>> EFI_RESERVED_TYPE.
>>
>>
>> Neither in my case nor in PJ case the device will be in use before
>> kernel is executing, although PJ should confirm this.
> There was some discussion in that thread of whether the decoders are locked.
> If they aren't (and if the device is not in use, or some other hard constraint
> isn't requiring it, in my view they definitely shouldn't be!) I'd at least
> like to consider the option of a 'cleanup pass' to tear them down and give
> the driver a clean slate to build on. Kind of similar to what we do in
> making PCI reeumerate in the kernel if we really don't like what the bios did.


I do not mind to support that option, but could we do it as a follow-up?


> Might not be possible if there is another higher numbered decoder in use
> though :(
>
>>
>>> If it is fair enough to support this, otherwise my inclination is tear
>>> down whatever the bios did and start again (unless locked - in which
>>> case go grumble at your BIOS folk). Reasoning being that we then only
>>> have to handle the equivalent of the hotplug flow in both cases rather
>>> than having to handle 2.
>>
>> Well, the automatic discovery region used for Type3 can be reused for
>> Type2 in this scenario, so we do not need to tear down what the BIOS
>> did. However, the argument is what we should do when the driver exits
>> which the current functionality added with the patchset being tearing
>> down the device and CXL bridge decoders. Dan seems to be keen on not
>> doing this tear down even if the HDMs are not locked.
> That's the question that makes this interesting.  What is reasoning for
> leaving bios stuff around in type 2 cases? I'd definitely like 'a way'
> to blow it away even if another option keeps it in place.
> A bios configures for what it can see at boot not necessarily what shows
> up later.  Similar cases exist in PCI such as resizeable BARs.
> The OS knows a lot more about the workload than the bios ever does and
> may choose to reconfigure because of hotplugged devices.


The main reason seems to be an assumption from BIOSes that only 
advertise CFMWS is there exists a CXL.mem enabled ... with the CXL Host 
Bridge CFMWS being equal to the total CXL.mem advertises by those 
devices discovered. This is something I have been talking about in 
discord and internally because I think that creates problems with 
hotplugging and future FAM support, or maybe current DCD.


One case, theoretical but I think quite possible, is a device requiring 
the CXL.mem not using the full capacity in all modes, likely because 
that device memory used for other purposes and kept hidden from the 
host. So the one knowing what to do should be the driver and dependent 
on the device and likely some other data maybe even configurable from 
user space.


So yes, I agree with you that the kernel should be able to do things far 
better than the BIOS ...



>>
>> What I can say is I have tested this patchset with an AMD system and
>> with the BIOS committing the HDM decoders for my device, and the first
>> time the driver loads it gets the region from the automatic discovery
>> while creating memdev, and the driver does tear down the HDMs when
>> exiting. Subsequent driver loads do the HDM configuration as this
>> patchset had been doing from day one. So all works as expected.
>>
>>
>> I'm inclined to leave the functionality as it is now, and your
>> suggestion or Dan's one for keeping the HDMs, as they were configured by
>> the BIOS, when driver exits should require, IMO, a good reason behind it.
> I'd definitely not make the assumption that BIOS' always do things for
> good reasons. They do things because someone once thought there was
> a good reason - or some other OS relied on them doing some part of setup.
>

100% agreement again.

>>
>>> There are also the TSP / encrypted link cases where we need to be careful.
>>> I have no idea if that applies here.
>>
>> I would say, let's wait until this support is completed, but as far as I
>> know, this is not a requirement for current Type2 clients (sfc and jump
>> trading).
> Dealing with this later works for me.  As long as it fails cleanly all good.


Great.

Thanks!


> Jonathan
>