Re: [PATCH 3/4 v5] cxl/core: Enable Region creation on x86 with LMH

[Alison Schofield <alison.schofield@intel.com>]

On Mon, Oct 06, 2025 at 05:58:06PM +0200, Fabio M. De Francesco wrote:
> The CXL Fixed Memory Window Structure (CFMWS) describes zero or more
> Host Physical Address (HPA) windows that are associated with each CXL
> Host Bridge. Each window represents a contiguous HPA that may be
> interleaved with one or more targets (CXL v3.2 - 9.18.1.3).
> 
> The Low Memory Hole (LMH) of x86 is a range of addresses of physical low
> memory to which systems cannot send transactions. In some case the size
> of that hole is not compatible with the constraint that the CFMWS size
> shall be multiple of Interleave Ways * 256 MB. (CXL v3.2 - Table 9-22).
> 
> On those systems, the BIOS publishes CFMWS which communicate the active
> System Physical Address (SPA) ranges that map to a subset of the Host
> Physical Address (HPA) ranges. The SPA range trims out the hole, and the
> capacity in the endpoint is lost with no SPA to map to CXL HPA in that
> hole.
> 
> In the early stages of CXL regions construction and attach on platforms
> that have Low Memory Holes, cxl_add_to_region() fails and returns an
> error for it can't find any CFMWS range that matches a given endpoint
> decoder.
> 
> Detect an LMH by comparing root decoder and endpoint decoder range.
> Match root decoders HPA range and constructed region with the
> corresponding endpoint decoders. Construct CXL region with the end of
> its HPA ranges end adjusted to the matching SPA and adjust the DPA
> resource end of the hardware decoders to fit the region.  Allow the
> attach target process to complete by allowing regions and decoders to
> bypass the constraints that don't hold when an LMH is present.[1]
> 
> [1] commit 7a81173f3 ("cxl: Documentation/driver-api/cxl: Describe the x86 Low Memory Hole solution")
> 
> Cc: Alison Schofield <alison.schofield@intel.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Dave Jiang <dave.jiang@intel.com>
> Cc: Ira Weiny <ira.weiny@intel.com>
> Signed-off-by: Fabio M. De Francesco <fabio.m.de.francesco@linux.intel.com>
> ---
>  drivers/cxl/core/region.c | 47 ++++++++++++++++++++++++++++++++-------
>  tools/testing/cxl/Kbuild  |  1 +
>  2 files changed, 40 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 43a854036202..9a499bfca23d 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -14,6 +14,7 @@
>  #include <linux/string_choices.h>
>  #include <cxlmem.h>
>  #include <cxl.h>
> +#include "platform_quirks.h"
>  #include "core.h"
>  

snip

> @@ -3479,6 +3498,12 @@ static int __construct_region(struct cxl_region *cxlr,
>  	*res = DEFINE_RES_MEM_NAMED(hpa->start, range_len(hpa),
>  				    dev_name(&cxlr->dev));
>  
> +	/*
> +	 * Trim the HPA retrieved from hardware to fit the SPA mapped by the
> +	 * platform
> +	 */
> +	platform_res_adjust(res, cxled, cxlrd);
> +

Noted this a bit in other patch, not so sure about that comment.
But anyway, do we really want to say what it is doing or let it be
a mystery of the quirks. I'm really not clear on where we are going
with these quirks and the naming of the helper functions.

If you split into 2 helpers, you can try something like:
	*res = platform_adjust_region_resource(...);

And then later, do the endpoint adjust. See below:


>  	rc = cxl_extended_linear_cache_resize(cxlr, res);
>  	if (rc && rc != -EOPNOTSUPP) {
>  		/*
> @@ -3588,6 +3613,12 @@ int cxl_add_to_region(struct cxl_endpoint_decoder *cxled)
>  		cxl_find_region_by_range(cxlrd, cxled);
>  	if (!cxlr)
>  		cxlr = construct_region(cxlrd, cxled);
> +	else
> +		/*
> +		 * Adjust the Endpoint Decoder's dpa_res to fit the Region which
> +		 * it has to be attached to
> +		 */
> +		platform_res_adjust(NULL, cxled, cxlrd);

Following from above, would it work to skip the else, and knowing
that the region resource was adjusted in construct_region(), only
do this here for every cxled that attaches.

	cxled = platform_adjust_endpoint_resource(...)

snip to end.