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.1 - 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 cases the size
of that hole is not compatible with the CXL hardware decoder constraint
that the size is always aligned to 256M * Interleave Ways.

On those systems, 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 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
with Low Memory Holes, cxl_add_to_region() fails and returns an error
because it can't find any CXL Window that matches a given CXL Endpoint
Decoder.

Detect a Low Memory Hole by comparing Root Decoders and Endpoint Decoders
ranges with the use of arch_match_{spa,region}() helpers.

Match Root Decoders and CXL Regions with corresponding CXL Endpoint
Decoders. Currently a Low Memory Holes would prevent the matching functions
to return true.

Construct CXL Regions with HPA range's end adjusted to the matching SPA.

Allow the attach target process to complete by allowing Regions to not
fit with alignment constraints (i.e., alignment to NIW * 256M rule).

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 | 53 +++++++++++++++++++++++++++++++++------
 tools/testing/cxl/Kbuild  |  1 +
 2 files changed, 46 insertions(+), 8 deletions(-)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index f607e7f97184..b7fdf9c4393d 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -12,6 +12,7 @@
 #include <linux/memory-tiers.h>
 #include <cxlmem.h>
 #include <cxl.h>
+#include "platform.h"
 #include "core.h"
 
 /**
@@ -834,6 +835,8 @@ static int match_free_decoder(struct device *dev, const void *data)
 static bool region_res_match_cxl_range(const struct cxl_region_params *p,
 				       struct range *range)
 {
+	struct cxl_decoder *cxld;
+
 	if (!p->res)
 		return false;
 
@@ -842,8 +845,15 @@ static bool region_res_match_cxl_range(const struct cxl_region_params *p,
 	 * to be fronted by the DRAM range in current known implementation.
 	 * This assumption will be made until a variant implementation exists.
 	 */
-	return p->res->start + p->cache_size == range->start &&
-		p->res->end == range->end;
+	if (p->res->start + p->cache_size == range->start &&
+	    p->res->end == range->end)
+		return true;
+
+	cxld = container_of(range, struct cxl_decoder, hpa_range);
+	if (platform_region_contains(p, cxld))
+		return true;
+
+	return false;
 }
 
 static int match_auto_decoder(struct device *dev, const void *data)
@@ -1763,6 +1773,7 @@ static int match_switch_and_ep_decoders(struct device *dev, const void *data)
 {
 	const struct cxl_endpoint_decoder *cxled = data;
 	struct cxl_switch_decoder *cxlsd;
+	struct cxl_root_decoder *cxlrd;
 	const struct range *r1, *r2;
 
 	if (!is_switch_decoder(dev))
@@ -1772,8 +1783,13 @@ static int match_switch_and_ep_decoders(struct device *dev, const void *data)
 	r1 = &cxlsd->cxld.hpa_range;
 	r2 = &cxled->cxld.hpa_range;
 
-	if (is_root_decoder(dev))
-		return range_contains(r1, r2);
+	if (is_root_decoder(dev)) {
+		if (range_contains(r1, r2))
+			return 1;
+		cxlrd = to_cxl_root_decoder(dev);
+		if (platform_root_decoder_contains(cxlrd, cxled))
+			return 1;
+	}
 	return (r1->start == r2->start && r1->end == r2->end);
 }
 
@@ -1990,7 +2006,7 @@ static int cxl_region_attach(struct cxl_region *cxlr,
 	}
 
 	if (resource_size(cxled->dpa_res) * p->interleave_ways + p->cache_size !=
-	    resource_size(p->res)) {
+	    resource_size(p->res) && !platform_root_decoder_contains(cxlrd, cxled)) {
 		dev_dbg(&cxlr->dev,
 			"%s:%s-size-%#llx * ways-%d + cache-%#llx != region-size-%#llx\n",
 			dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev),
@@ -3230,7 +3246,12 @@ static int match_root_and_ep_decoders(struct device *dev, const void *data)
 	r1 = &cxlrd->cxlsd.cxld.hpa_range;
 	r2 = &cxled->cxld.hpa_range;
 
-	return range_contains(r1, r2);
+	if (range_contains(r1, r2))
+		return true;
+	if (platform_root_decoder_contains(cxlrd, cxled))
+		return true;
+
+	return false;
 }
 
 static struct cxl_decoder *
@@ -3277,8 +3298,12 @@ static int match_region_and_ep_decoder(struct device *dev, const void *data)
 	p = &cxlr->params;
 
 	guard(rwsem_read)(&cxl_region_rwsem);
-	if (p->res && p->res->start == r->start && p->res->end == r->end)
-		return 1;
+	if (p->res) {
+		if (p->res->start == r->start && p->res->end == r->end)
+			return 1;
+		if (platform_region_contains(p, &cxled->cxld))
+			return 1;
+	}
 
 	return 0;
 }
@@ -3355,6 +3380,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);
+
 	rc = cxl_extended_linear_cache_resize(cxlr, res);
 	if (rc && rc != -EOPNOTSUPP) {
 		/*
@@ -3464,6 +3495,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);
 	mutex_unlock(&cxlrd->range_lock);
 
 	rc = PTR_ERR_OR_ZERO(cxlr);
diff --git a/tools/testing/cxl/Kbuild b/tools/testing/cxl/Kbuild
index 31a2d73c963f..77e392c4b541 100644
--- a/tools/testing/cxl/Kbuild
+++ b/tools/testing/cxl/Kbuild
@@ -65,6 +65,7 @@ cxl_core-y += $(CXL_CORE_SRC)/ras.o
 cxl_core-y += $(CXL_CORE_SRC)/acpi.o
 cxl_core-$(CONFIG_TRACING) += $(CXL_CORE_SRC)/trace.o
 cxl_core-$(CONFIG_CXL_REGION) += $(CXL_CORE_SRC)/region.o
+cxl_core-$(CONFIG_CXL_PLATFORM_QUIRKS) += $(CXL_CORE_SRC)/platform.o
 cxl_core-$(CONFIG_CXL_MCE) += $(CXL_CORE_SRC)/mce.o
 cxl_core-$(CONFIG_CXL_FEATURES) += $(CXL_CORE_SRC)/features.o
 cxl_core-$(CONFIG_CXL_EDAC_MEM_FEATURES) += $(CXL_CORE_SRC)/edac.o
-- 
2.50.1

On 7/24/2025 9:20 AM, 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.1 - 9.18.1.3).

Update to 3.2 spec? Sorry I forgot to mention it earlier, but you'll probably
want to do this for the whole series.

> 
> The Low Memory Hole (LMH) of x86 is a range of addresses of physical low
> memory to which systems cannot send transactions. In some cases the size
> of that hole is not compatible with the CXL hardware decoder constraint
> that the size is always aligned to 256M * Interleave Ways.

Spec ref here.

> 
> On those systems, 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 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
> with Low Memory Holes, cxl_add_to_region() fails and returns an error
> because it can't find any CXL Window that matches a given CXL Endpoint
> Decoder.
> 
> Detect a Low Memory Hole by comparing Root Decoders and Endpoint Decoders
> ranges with the use of arch_match_{spa,region}() helpers.
> 
> Match Root Decoders and CXL Regions with corresponding CXL Endpoint
> Decoders. Currently a Low Memory Holes would prevent the matching functions
> to return true.
> 
> Construct CXL Regions with HPA range's end adjusted to the matching SPA.
> 
> Allow the attach target process to complete by allowing Regions to not
> fit with alignment constraints (i.e., alignment to NIW * 256M rule).
> 
> 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 | 53 +++++++++++++++++++++++++++++++++------
>  tools/testing/cxl/Kbuild  |  1 +
>  2 files changed, 46 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index f607e7f97184..b7fdf9c4393d 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -12,6 +12,7 @@
>  #include <linux/memory-tiers.h>
>  #include <cxlmem.h>
>  #include <cxl.h>
> +#include "platform.h"
>  #include "core.h"
>  
>  /**
> @@ -834,6 +835,8 @@ static int match_free_decoder(struct device *dev, const void *data)
>  static bool region_res_match_cxl_range(const struct cxl_region_params *p,
>  				       struct range *range)
>  {
> +	struct cxl_decoder *cxld;
> +
>  	if (!p->res)
>  		return false;
>  
> @@ -842,8 +845,15 @@ static bool region_res_match_cxl_range(const struct cxl_region_params *p,
>  	 * to be fronted by the DRAM range in current known implementation.
>  	 * This assumption will be made until a variant implementation exists.
>  	 */
> -	return p->res->start + p->cache_size == range->start &&
> -		p->res->end == range->end;
> +	if (p->res->start + p->cache_size == range->start &&
> +	    p->res->end == range->end)
> +		return true;
> +
> +	cxld = container_of(range, struct cxl_decoder, hpa_range);
> +	if (platform_region_contains(p, cxld))
> +		return true;
> +
> +	return false;

Can just return result of platform_region_contains() here.

>  }
>  
>  static int match_auto_decoder(struct device *dev, const void *data)
> @@ -1763,6 +1773,7 @@ static int match_switch_and_ep_decoders(struct device *dev, const void *data)
>  {
>  	const struct cxl_endpoint_decoder *cxled = data;
>  	struct cxl_switch_decoder *cxlsd;
> +	struct cxl_root_decoder *cxlrd;
>  	const struct range *r1, *r2;
>  
>  	if (!is_switch_decoder(dev))
> @@ -1772,8 +1783,13 @@ static int match_switch_and_ep_decoders(struct device *dev, const void *data)
>  	r1 = &cxlsd->cxld.hpa_range;
>  	r2 = &cxled->cxld.hpa_range;
>  
> -	if (is_root_decoder(dev))
> -		return range_contains(r1, r2);
> +	if (is_root_decoder(dev)) {
> +		if (range_contains(r1, r2))
> +			return 1;
> +		cxlrd = to_cxl_root_decoder(dev);
> +		if (platform_root_decoder_contains(cxlrd, cxled))
> +			return 1;
> +	}

I don't think it's possible, but the way this is written allows falling through
to the return statement below. This if statement should probably be: 

	if (is_root_decoder(dev)) {
		cxlrd = to_cxl_root_decoder(dev);
		return range_contains(r1, r2) || platform_root_decoder_contains(cxlrd, cxled));
	}

>  	return (r1->start == r2->start && r1->end == r2->end);
>  }
>  
> @@ -1990,7 +2006,7 @@ static int cxl_region_attach(struct cxl_region *cxlr,
>  	}
>  
>  	if (resource_size(cxled->dpa_res) * p->interleave_ways + p->cache_size !=
> -	    resource_size(p->res)) {
> +	    resource_size(p->res) && !platform_root_decoder_contains(cxlrd, cxled)) {
>  		dev_dbg(&cxlr->dev,
>  			"%s:%s-size-%#llx * ways-%d + cache-%#llx != region-size-%#llx\n",
>  			dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev),
> @@ -3230,7 +3246,12 @@ static int match_root_and_ep_decoders(struct device *dev, const void *data)
>  	r1 = &cxlrd->cxlsd.cxld.hpa_range;
>  	r2 = &cxled->cxld.hpa_range;
>  
> -	return range_contains(r1, r2);
> +	if (range_contains(r1, r2))
> +		return true;
> +	if (platform_root_decoder_contains(cxlrd, cxled))
> +		return true;
> +
> +	return false;

Can just return the || of the two above if statements.

>  }
>  
>  static struct cxl_decoder *
> @@ -3277,8 +3298,12 @@ static int match_region_and_ep_decoder(struct device *dev, const void *data)
>  	p = &cxlr->params;
>  
>  	guard(rwsem_read)(&cxl_region_rwsem);
> -	if (p->res && p->res->start == r->start && p->res->end == r->end)
> -		return 1;
> +	if (p->res) {
> +		if (p->res->start == r->start && p->res->end == r->end)
> +			return 1;
> +		if (platform_region_contains(p, &cxled->cxld))
> +			return 1;
> +	}

Same thing here.

>  
>  	return 0;
>  }
> @@ -3355,6 +3380,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);
> +
>  	rc = cxl_extended_linear_cache_resize(cxlr, res);
>  	if (rc && rc != -EOPNOTSUPP) {
>  		/*
> @@ -3464,6 +3495,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);

I'm 50/50 on whether these comments are unnecessary. The routine is pretty well documented
and also has an explanatory comment above the definition in platform.c. I think you
can probably remove them, but I'll defer to your/someone else's judgement here.

Thanks,
Ben

On Friday, August 1, 2025 10:04:18 PM Central European Summer Time Cheatham, Benjamin wrote:
> On 7/24/2025 9:20 AM, 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.1 - 9.18.1.3).
> 
> Update to 3.2 spec? Sorry I forgot to mention it earlier, but you'll probably
> want to do this for the whole series.
> 
Sure, thanks.
> > 
> > The Low Memory Hole (LMH) of x86 is a range of addresses of physical low
> > memory to which systems cannot send transactions. In some cases the size
> > of that hole is not compatible with the CXL hardware decoder constraint
> > that the size is always aligned to 256M * Interleave Ways.
> 
> Spec ref here.
> 
Okay.
> > 
> > On those systems, 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 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
> > with Low Memory Holes, cxl_add_to_region() fails and returns an error
> > because it can't find any CXL Window that matches a given CXL Endpoint
> > Decoder.
> > 
> > Detect a Low Memory Hole by comparing Root Decoders and Endpoint Decoders
> > ranges with the use of arch_match_{spa,region}() helpers.
> > 
> > Match Root Decoders and CXL Regions with corresponding CXL Endpoint
> > Decoders. Currently a Low Memory Holes would prevent the matching functions
> > to return true.
> > 
> > Construct CXL Regions with HPA range's end adjusted to the matching SPA.
> > 
> > Allow the attach target process to complete by allowing Regions to not
> > fit with alignment constraints (i.e., alignment to NIW * 256M rule).
> > 
> > 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 | 53 +++++++++++++++++++++++++++++++++------
> >  tools/testing/cxl/Kbuild  |  1 +
> >  2 files changed, 46 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> > index f607e7f97184..b7fdf9c4393d 100644
> > --- a/drivers/cxl/core/region.c
> > +++ b/drivers/cxl/core/region.c
> > @@ -12,6 +12,7 @@
> >  #include <linux/memory-tiers.h>
> >  #include <cxlmem.h>
> >  #include <cxl.h>
> > +#include "platform.h"
> >  #include "core.h"
> >  
> >  /**
> > @@ -834,6 +835,8 @@ static int match_free_decoder(struct device *dev, const void *data)
> >  static bool region_res_match_cxl_range(const struct cxl_region_params *p,
> >  				       struct range *range)
> >  {
> > +	struct cxl_decoder *cxld;
> > +
> >  	if (!p->res)
> >  		return false;
> >  
> > @@ -842,8 +845,15 @@ static bool region_res_match_cxl_range(const struct cxl_region_params *p,
> >  	 * to be fronted by the DRAM range in current known implementation.
> >  	 * This assumption will be made until a variant implementation exists.
> >  	 */
> > -	return p->res->start + p->cache_size == range->start &&
> > -		p->res->end == range->end;
> > +	if (p->res->start + p->cache_size == range->start &&
> > +	    p->res->end == range->end)
> > +		return true;
> > +
> > +	cxld = container_of(range, struct cxl_decoder, hpa_range);
> > +	if (platform_region_contains(p, cxld))
> > +		return true;
> > +
> > +	return false;
> 
> Can just return result of platform_region_contains() here.
>
Okay.
> 
> >  }
> >  
> >  static int match_auto_decoder(struct device *dev, const void *data)
> > @@ -1763,6 +1773,7 @@ static int match_switch_and_ep_decoders(struct device *dev, const void *data)
> >  {
> >  	const struct cxl_endpoint_decoder *cxled = data;
> >  	struct cxl_switch_decoder *cxlsd;
> > +	struct cxl_root_decoder *cxlrd;
> >  	const struct range *r1, *r2;
> >  
> >  	if (!is_switch_decoder(dev))
> > @@ -1772,8 +1783,13 @@ static int match_switch_and_ep_decoders(struct device *dev, const void *data)
> >  	r1 = &cxlsd->cxld.hpa_range;
> >  	r2 = &cxled->cxld.hpa_range;
> >  
> > -	if (is_root_decoder(dev))
> > -		return range_contains(r1, r2);
> > +	if (is_root_decoder(dev)) {
> > +		if (range_contains(r1, r2))
> > +			return 1;
> > +		cxlrd = to_cxl_root_decoder(dev);
> > +		if (platform_root_decoder_contains(cxlrd, cxled))
> > +			return 1;
> > +	}
> 
> I don't think it's possible, but the way this is written allows falling through
> to the return statement below.
>
It's possible. It can happen and it's harmless yet unnecessary.

> This if statement should probably be: 
> 
> 	if (is_root_decoder(dev)) {
> 		cxlrd = to_cxl_root_decoder(dev);
> 		return range_contains(r1, r2) || platform_root_decoder_contains(cxlrd, cxled));
> 	}
> 
> >  	return (r1->start == r2->start && r1->end == r2->end);
> >  }
> >  
And since it's unnecessary, I agree with you and rewrite it.

I just wanted, out of my personal preference, to highlight that calls
of platform_root_decoder_contains() happen only to allow the driver 
to succeed also on a less common case, that is on platforms with LMH.  
>
> > @@ -1990,7 +2006,7 @@ static int cxl_region_attach(struct cxl_region *cxlr,
> >  	}
> >  
> >  	if (resource_size(cxled->dpa_res) * p->interleave_ways + p->cache_size !=
> > -	    resource_size(p->res)) {
> > +	    resource_size(p->res) && !platform_root_decoder_contains(cxlrd, cxled)) {
> >  		dev_dbg(&cxlr->dev,
> >  			"%s:%s-size-%#llx * ways-%d + cache-%#llx != region-size-%#llx\n",
> >  			dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev),
> > @@ -3230,7 +3246,12 @@ static int match_root_and_ep_decoders(struct device *dev, const void *data)
> >  	r1 = &cxlrd->cxlsd.cxld.hpa_range;
> >  	r2 = &cxled->cxld.hpa_range;
> >  
> > -	return range_contains(r1, r2);
> > +	if (range_contains(r1, r2))
> > +		return true;
> > +	if (platform_root_decoder_contains(cxlrd, cxled))
> > +		return true;
> > +
> > +	return false;
> 
> Can just return the || of the two above if statements.
> 
Sure, as it is going to be with the other function.
> >  }
> >  
> >  static struct cxl_decoder *
> > @@ -3277,8 +3298,12 @@ static int match_region_and_ep_decoder(struct device *dev, const void *data)
> >  	p = &cxlr->params;
> >  
> >  	guard(rwsem_read)(&cxl_region_rwsem);
> > -	if (p->res && p->res->start == r->start && p->res->end == r->end)
> > -		return 1;
> > +	if (p->res) {
> > +		if (p->res->start == r->start && p->res->end == r->end)
> > +			return 1;
> > +		if (platform_region_contains(p, &cxled->cxld))
> > +			return 1;
> > +	}
> 
> Same thing here.
> 
> >  
> >  	return 0;
> >  }
> > @@ -3355,6 +3380,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);
> > +
> >  	rc = cxl_extended_linear_cache_resize(cxlr, res);
> >  	if (rc && rc != -EOPNOTSUPP) {
> >  		/*
> > @@ -3464,6 +3495,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);
> 
> I'm 50/50 on whether these comments are unnecessary. The routine is pretty well documented
> and also has an explanatory comment above the definition in platform.c. I think you
> can probably remove them, but I'll defer to your/someone else's judgement here.
> 
I'll think about it. Anyway, I wanted to clarify that the two call sites 
serve two different purposes (i.e., for already constructed regions we may 
still need to adjust dpa_res).

Thanks,

Fabio
>
> Thanks,
> Ben
> 

On 7/24/25 7:20 AM, 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.1 - 9.18.1.3).

Should bump it to the latest spec available (r3.2)

> 
> The Low Memory Hole (LMH) of x86 is a range of addresses of physical low
> memory to which systems cannot send transactions. In some cases the size

s/cases/cases, /

> of that hole is not compatible with the CXL hardware decoder constraint
> that the size is always aligned to 256M * Interleave Ways.
> 
> On those systems, BIOS publishes CFMWS which communicate the active System

s/BIOS/the BIOS/

> Physical Address (SPA) ranges that map to a subset of the Host Physical
> Address (HPA) ranges. The SPA range trims out the hole, and capacity in

s/capacity/the capacity/

> the endpoint is lost with no SPA to map to CXL HPA in that hole.
s/with/because there is/

> 
> In the early stages of CXL Regions construction and attach on platforms
s/Regions/region/

> with Low Memory Holes, cxl_add_to_region() fails and returns an error
s/Low Memory Holes/LMH/

> because it can't find any CXL Window that matches a given CXL Endpoint
s/CXL Window/CFMWS range/ I think?

> Decoder.
s/Endpoint Decoder/endpoint decoder HPA range/


> 
> Detect a Low Memory Hole by comparing Root Decoders and Endpoint Decoders
s/Low Memory Hole/LMH/
s/Root Decoders/root decoder HPA range/
s/Endpoint Decoders/endpoint decoder HPA range/

> ranges with the use of arch_match_{spa,region}() helpers.
> 
> Match Root Decoders and CXL Regions with corresponding CXL Endpoint
> Decoders. Currently a Low Memory Holes would prevent the matching functions
> to return true.
s/Root Decoders/root decoder HPA range/
s/Regions/region/
s/Endpoint Decoders/endpoint decoder HPA range/
s/Low Memory Hole/LMH/

> 
> Construct CXL Regions with HPA range's end adjusted to the matching SPA.

Construct CXL region with the end of its HPA range adjusted to the matching SPA.

> 
> Allow the attach target process to complete by allowing Regions to not
s/Regions/regions/

> fit with alignment constraints (i.e., alignment to NIW * 256M rule).

s/to not fit with/to bypass the/ ?

> 
> 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 | 53 +++++++++++++++++++++++++++++++++------
>  tools/testing/cxl/Kbuild  |  1 +
>  2 files changed, 46 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index f607e7f97184..b7fdf9c4393d 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -12,6 +12,7 @@
>  #include <linux/memory-tiers.h>
>  #include <cxlmem.h>
>  #include <cxl.h>
> +#include "platform.h"
>  #include "core.h"
>  
>  /**
> @@ -834,6 +835,8 @@ static int match_free_decoder(struct device *dev, const void *data)
>  static bool region_res_match_cxl_range(const struct cxl_region_params *p,
>  				       struct range *range)
>  {
> +	struct cxl_decoder *cxld;
> +
>  	if (!p->res)
>  		return false;
>  
> @@ -842,8 +845,15 @@ static bool region_res_match_cxl_range(const struct cxl_region_params *p,
>  	 * to be fronted by the DRAM range in current known implementation.
>  	 * This assumption will be made until a variant implementation exists.
>  	 */
> -	return p->res->start + p->cache_size == range->start &&
> -		p->res->end == range->end;
> +	if (p->res->start + p->cache_size == range->start &&
> +	    p->res->end == range->end)
> +		return true;

Have you thought through the scenario where extended linear cache exists and covers LMH? You may need to extend the math for platform_region_contains() maybe?

> +
> +	cxld = container_of(range, struct cxl_decoder, hpa_range);
> +	if (platform_region_contains(p, cxld))
> +		return true;
> +
> +	return false;

return platform_region_contains(p, cxld);

>  }
>  
>  static int match_auto_decoder(struct device *dev, const void *data)
> @@ -1763,6 +1773,7 @@ static int match_switch_and_ep_decoders(struct device *dev, const void *data)
>  {
>  	const struct cxl_endpoint_decoder *cxled = data;
>  	struct cxl_switch_decoder *cxlsd;
> +	struct cxl_root_decoder *cxlrd;
>  	const struct range *r1, *r2;
>  
>  	if (!is_switch_decoder(dev))
> @@ -1772,8 +1783,13 @@ static int match_switch_and_ep_decoders(struct device *dev, const void *data)
>  	r1 = &cxlsd->cxld.hpa_range;
>  	r2 = &cxled->cxld.hpa_range;
>  
> -	if (is_root_decoder(dev))
> -		return range_contains(r1, r2);
> +	if (is_root_decoder(dev)) {
> +		if (range_contains(r1, r2))
> +			return 1;
> +		cxlrd = to_cxl_root_decoder(dev);
> +		if (platform_root_decoder_contains(cxlrd, cxled))
> +			return 1;
> +	}
>  	return (r1->start == r2->start && r1->end == r2->end);
>  }
>  
> @@ -1990,7 +2006,7 @@ static int cxl_region_attach(struct cxl_region *cxlr,
>  	}
>  
>  	if (resource_size(cxled->dpa_res) * p->interleave_ways + p->cache_size !=
> -	    resource_size(p->res)) {
> +	    resource_size(p->res) && !platform_root_decoder_contains(cxlrd, cxled)) {
>  		dev_dbg(&cxlr->dev,
>  			"%s:%s-size-%#llx * ways-%d + cache-%#llx != region-size-%#llx\n",
>  			dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev),
> @@ -3230,7 +3246,12 @@ static int match_root_and_ep_decoders(struct device *dev, const void *data)
>  	r1 = &cxlrd->cxlsd.cxld.hpa_range;
>  	r2 = &cxled->cxld.hpa_range;
>  
> -	return range_contains(r1, r2);
> +	if (range_contains(r1, r2))
> +		return true;
> +	if (platform_root_decoder_contains(cxlrd, cxled))
> +		return true;
> +
> +	return false;

return range_contais(...) || platform_root_decoder_contains(...);

DJ

>  }
>  
>  static struct cxl_decoder *
> @@ -3277,8 +3298,12 @@ static int match_region_and_ep_decoder(struct device *dev, const void *data)
>  	p = &cxlr->params;
>  
>  	guard(rwsem_read)(&cxl_region_rwsem);
> -	if (p->res && p->res->start == r->start && p->res->end == r->end)
> -		return 1;
> +	if (p->res) {
> +		if (p->res->start == r->start && p->res->end == r->end)
> +			return 1;
> +		if (platform_region_contains(p, &cxled->cxld))
> +			return 1;
> +	}
>  
>  	return 0;
>  }
> @@ -3355,6 +3380,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);
> +
>  	rc = cxl_extended_linear_cache_resize(cxlr, res);
>  	if (rc && rc != -EOPNOTSUPP) {
>  		/*
> @@ -3464,6 +3495,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);
>  	mutex_unlock(&cxlrd->range_lock);
>  
>  	rc = PTR_ERR_OR_ZERO(cxlr);
> diff --git a/tools/testing/cxl/Kbuild b/tools/testing/cxl/Kbuild
> index 31a2d73c963f..77e392c4b541 100644
> --- a/tools/testing/cxl/Kbuild
> +++ b/tools/testing/cxl/Kbuild
> @@ -65,6 +65,7 @@ cxl_core-y += $(CXL_CORE_SRC)/ras.o
>  cxl_core-y += $(CXL_CORE_SRC)/acpi.o
>  cxl_core-$(CONFIG_TRACING) += $(CXL_CORE_SRC)/trace.o
>  cxl_core-$(CONFIG_CXL_REGION) += $(CXL_CORE_SRC)/region.o
> +cxl_core-$(CONFIG_CXL_PLATFORM_QUIRKS) += $(CXL_CORE_SRC)/platform.o
>  cxl_core-$(CONFIG_CXL_MCE) += $(CXL_CORE_SRC)/mce.o
>  cxl_core-$(CONFIG_CXL_FEATURES) += $(CXL_CORE_SRC)/features.o
>  cxl_core-$(CONFIG_CXL_EDAC_MEM_FEATURES) += $(CXL_CORE_SRC)/edac.o