Re: [PATCH v19 11/22] cxl: Define a driver interface for HPA free space enumeration

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

On 10/9/25 21:55, Cheatham, Benjamin wrote:
> On 10/6/2025 5:01 AM, alejandro.lucero-palau@amd.com wrote:
>> From: Alejandro Lucero <alucerop@amd.com>
>>
>> CXL region creation involves allocating capacity from Device Physical Address
>> (DPA) and assigning it to decode a given Host Physical Address (HPA). Before
>> determining how much DPA to allocate the amount of available HPA must be
>> determined. Also, not all HPA is created equal, some HPA targets RAM, some
>> targets PMEM, some is prepared for device-memory flows like HDM-D and HDM-DB,
>> and some is HDM-H (host-only).
>>
>> In order to support Type2 CXL devices, wrap all of those concerns into
>> an API that retrieves a root decoder (platform CXL window) that fits the
>> specified constraints and the capacity available for a new region.
>>
>> Add a complementary function for releasing the reference to such root
>> decoder.
>>
>> Based on https://lore.kernel.org/linux-cxl/168592159290.1948938.13522227102445462976.stgit@dwillia2-xfh.jf.intel.com/
>>
>> Signed-off-by: Alejandro Lucero <alucerop@amd.com>
>> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>> ---
> Hey Alejandro,


Hi Ben,


> I've been testing this on my setup and noticed a few issues when BIOS sets up the HDM decoders. It came down to 2 issues:
> 	1) Enabling "Specific Purpose Memory" added a Soft Reserve resource below the CXL window resource and broke
> 	this patch (more below)


Maybe we should talk (first) about this internally as it is about AMD 
BIOS (I guess). I have been talking with the BIOS team about this 
EFI_MEMORY_SP vs EFI_RESERVED_MEMORY, and I'm afraid the discussion is 
not over yet :-).


> 	2) The endpoint decoder was already set up which broke DPA allocation and then CXL region creation (see my response
> 	to patch 18/22 for fix and explanation)


Yes, if the BIOS configures the device HDM decoder, the current patchset 
does not do the right thing. As I said in the cover letter, my 
expectation at the time, and hopefully in the future as well, although 
I'm not sure about it, was the BIOS not doing so. Most of the 
implementation is based on QEMU, so I found this problem when dealing 
with a real system with a Type2 aware BIOS ... . I was tempted to 
include support for this case, but I did not do so for several reasons:


1) I want to think the patchset is close to being accepted and changes 
at this state could delay it further. After more than a year, and 
because this patchset is about "initial Type2 support", I think it is 
better to do so in a follow-up work, and when there is a client 
requiring it.

2) Because that conversation with BIOS guys, I prefer to be sure what to 
do, as there are other things we need to clarify and in my opinion, far 
more important than current Type2 support.

3) CXL is a fast moving part of the kernel and I bet we will find 
another case which the current patchset is not dealing with properly. In 
fact there is another report of devices with the BAR with CXL 
information being also used by the driver for other purposes and 
existing a problem when mapping the CXL registers after the driver did 
map the whole BAR.


So, I think the current patchset where most of the API for Type2 drivers 
is implemented should go as soon as possible, which will facilitate 
those follow-up works for the case you describe and the other one about 
BAR mappings. If not, even if retirement is still far away for me, I'll 
be concerned about the impending future of this work ... but of course, 
this is my suggestion, so let's see other opinions about it.


Thank you.


>
> The fix I did for 1 is a bit hacky but it's essentially checking none of the resources below the CXL window are onlined as
> system memory. It's roughly equivalent to what's being done in the CXL_PARTMODE_RAM case of cxl_region_probe(), but
> I'm restricting the resources to "Soft Reserved" to be safe.
>
> The diff for 2 is pretty big. If you don't want to take it at this point I can send it as a follow up. In that case I'd definitely
> add that auto regions won't work in at least the cover letter (and in the description of 18/22 as well?).
>
> ---
>
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index acaca64764bf..2d60131edff3 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -784,6 +784,19 @@ static int find_max_hpa(struct device *dev, void *data)
>          lockdep_assert_held_read(&cxl_rwsem.region);
>          res = cxlrd->res->child;
>
> +       /*
> +        * BIOS may have marked the CXL window as soft reserved. Make sure it's
> +        * free to use.
> +        */
> +       while (res && resource_size(res) == resource_size(cxlrd->res)) {
> +               if ((res->flags & IORESOURCE_BUSY) ||
> +                   (res->flags & IORESOURCE_SYSRAM) ||
> +                   strcmp(res->name, "Soft Reserved") != 0)
> +                       return 0;
> +
> +               res = res->child;
> +       }
> +
>          /* With no resource child the whole parent resource is available */
>          if (!res)
>                  max = resource_size(cxlrd->res);