linux-doc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4 v5] cxl/core: Enable Region creation/attach on x86 with LMH
@ 2025-10-06 15:58 Fabio M. De Francesco
  2025-10-06 15:58 ` [PATCH 1/4 v5] cxl/core: Change match_*_by_range() signatures Fabio M. De Francesco
                   ` (3 more replies)
  0 siblings, 4 replies; 24+ messages in thread
From: Fabio M. De Francesco @ 2025-10-06 15:58 UTC (permalink / raw)
  To: linux-cxl
  Cc: Davidlohr Bueso, Jonathan Cameron, Dave Jiang, Alison Schofield,
	Vishal Verma, Ira Weiny, Dan Williams, Jonathan Corbet, linux-doc,
	linux-kernel, Gregory Price, Robert Richter, Cheatham Benjamin,
	Fabio M. De Francesco

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. 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, the driver fails and returns an error because it
expects that the CXL Endpoint Decoder range is a subset of the Root
Decoder's (SPA >= HPA). On x86 with LMH's, it happens that SPA < HPA.

Therefore, detect x86 Low Memory Holes, match CXL Root and Endpoint
Decoders or already made CXL Regions and Decoders to allow the
construction of new CXL Regions and the attachment of Endpoint Decoders,
even if SPA < HPA. If needed because of LMH's, adjust the Endpoint
Decoder range end to match Root Decoder's.

- Patch 1/4 changes the calling conventions of three match_*_by_range()
  helpers in preparation of 3/4.
- Patch 2/4 Introduces helpers to detect LMH's and also one to adjust
  the HPA range end for CXL Regions construction.
- Patch 3/4 enables CXL Regions construction and Endpoint Decoders
  attachment by matching Root Decoders or Regions with Endpoint
  Decoders, adjusting Endpoint Decoders HPA range end, and relaxing
  constraints while Endpoints decoders' attachment.
- Patch 4/4 simulates a LMH for the CXL tests on patched CXL driver,
  rewrite the two functions of 2/4 to test the construction of a region
  and the attachment of the decoders in the test environment.

Many thanks to Alison, Dan, Dave and Ira for their help.

Commenting on v1, Alison wrote a couple of observations on what users
will see. I suggest anyone interested to see how this series affect
users to take a look at her observations.[0] Thank you!

Changes for v5:

  Patch 1/4:
	Add Reviewed-by tag. (Jonathan) 
	Use Reverse Christmas Tree notation. (Benjamin)
	Rename three match_*(). (Dave)

  Patch 2/4:
	Rewrite two paragraphs on the commit message for better clarity
	and flow. (Dave)
	Fix grammar and syntax errors in the commit message. (Benjamin, Dave)
	List the conditions under which the platform*() helpers match root
	decoders or regions with intermediate switch or endpoint decoders.
	(Dave)
	Rename platform.c to platform_quirks.c. (Dave)
	Rename local variables for root and endpoint decoders. (Dave)
	Have one conditional per line. (Dave)
	Reword comments in platform_res_adjust(). (Benjamin)
	Make "inline" the platform*() functions declarations in #else block.
	(Benjamin)
	Make kdocs from regular comments on functions. (Benjamin)
	Rename platform_*_contains() functions to platform_cxlrd_matches_cxled()
	and platform_region_matches_cxld() to better reflect their semantics.
	Reference a commit to CXL documentation.

  Patch 3/4:
	Update commit message to Spec 3.2 (Benjamin)
	Return result of platform_region_contains() directly (Benjamin, Dave)
	Make a logical OR of two if statements (Benjamin, Dave)
	Fix grammar errors and improve readability of the commit message (Dave)

  Patch 4/4:
	Base the LMH simulation on the redirect/mock mechanism (Dave)
	Rename a few local variables (Dave)

Changes for v4:

  Re-base on top of 
	"cxl: Address translation support, part 1: Cleanups and refactoring";[1] 
  Drop no more necessary 2/4;
  Drop collected tags because of major changes throughout the series.

  1/3 - Adjust Endpoint Decoders dpa_res->end (Alison) [2] 
  3/3 - Use weak/strong mechanism (Dan) [3]

v4 - https://lore.kernel.org/linux-cxl/20250724142144.776992-1-fabio.m.de.francesco@linux.intel.com/
v3 - https://lore.kernel.org/linux-cxl/20250314113708.759808-1-fabio.m.de.francesco@linux.intel.com/
v2 - https://lore.kernel.org/linux-cxl/20250114203432.31861-1-fabio.m.de.francesco@linux.intel.com/
v1 - https://lore.kernel.org/all/20241122155226.2068287-1-fabio.m.de.francesco@linux.intel.com/

[0] - https://lore.kernel.org/all/Z0Tzif55CcHuujJ-@aschofie-mobl2.lan/
[1] - https://lore.kernel.org/linux-cxl/20250509150700.2817697-1-rrichter@amd.com/
[2] - https://lore.kernel.org/linux-cxl/Z9tzZkn1rqd2Uk_6@aschofie-mobl2.lan/
[3] - https://lore.kernel.org/linux-cxl/67ee07cd4f8ec_1c2c6294d5@dwillia2-xfh.jf.intel.com.notmuch/

Fabio M. De Francesco (4):
  cxl/core: Change match_*_by_range() signatures
  cxl/core: Add helpers to detect Low Memory Holes on x86
  cxl/core: Enable Region creation on x86 with LMH
  cxl/test: Simulate an x86 Low Memory Hole for tests

 drivers/cxl/Kconfig                  |   4 +
 drivers/cxl/core/Makefile            |   1 +
 drivers/cxl/core/platform_quirks.c   | 104 +++++++++++++++++++++++++
 drivers/cxl/core/platform_quirks.h   |  49 ++++++++++++
 drivers/cxl/core/region.c            | 109 ++++++++++++++++++---------
 tools/testing/cxl/Kbuild             |   1 +
 tools/testing/cxl/cxl_core_exports.c |  23 ++++++
 tools/testing/cxl/exports.h          |   7 ++
 tools/testing/cxl/test/cxl.c         |  54 +++++++++++++
 tools/testing/cxl/test/mock.c        |  48 ++++++++++++
 tools/testing/cxl/test/mock.h        |   4 +
 11 files changed, 369 insertions(+), 35 deletions(-)
 create mode 100644 drivers/cxl/core/platform_quirks.c
 create mode 100644 drivers/cxl/core/platform_quirks.h

base-commit: 46037455cbb748c5e85071c95f2244e81986eb58
-- 
2.50.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 1/4 v5] cxl/core: Change match_*_by_range() signatures
  2025-10-06 15:58 [PATCH 0/4 v5] cxl/core: Enable Region creation/attach on x86 with LMH Fabio M. De Francesco
@ 2025-10-06 15:58 ` Fabio M. De Francesco
  2025-10-06 17:35   ` Gregory Price
  2025-10-06 23:30   ` Dave Jiang
  2025-10-06 15:58 ` [PATCH 2/4 v5] cxl/core: Add helpers to detect Low Memory Holes on x86 Fabio M. De Francesco
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 24+ messages in thread
From: Fabio M. De Francesco @ 2025-10-06 15:58 UTC (permalink / raw)
  To: linux-cxl
  Cc: Davidlohr Bueso, Jonathan Cameron, Dave Jiang, Alison Schofield,
	Vishal Verma, Ira Weiny, Dan Williams, Jonathan Corbet, linux-doc,
	linux-kernel, Gregory Price, Robert Richter, Cheatham Benjamin,
	Fabio M. De Francesco

Replace struct range parameter with struct cxl_endpoint_decoder of
which range is a member in the match_*_by_range() functions and rename
them according to their semantics.

This is in preparation for expanding these helpers to perform arch
specific Root Decoders and Region matchings with
cxl_endpoint_decoder(s).

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>
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
Signed-off-by: Fabio M. De Francesco <fabio.m.de.francesco@linux.intel.com>
---
 drivers/cxl/core/region.c | 62 ++++++++++++++++++++++-----------------
 1 file changed, 35 insertions(+), 27 deletions(-)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index e14c1d305b22..43a854036202 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -1766,27 +1766,29 @@ static int cmp_interleave_pos(const void *a, const void *b)
 	return cxled_a->pos - cxled_b->pos;
 }
 
-static int match_switch_decoder_by_range(struct device *dev,
-					 const void *data)
+static int match_cxlsd_to_cxled_by_range(struct device *dev, const void *data)
 {
+	const struct cxl_endpoint_decoder *cxled = data;
 	struct cxl_switch_decoder *cxlsd;
-	const struct range *r1, *r2 = data;
-
+	const struct range *r1, *r2;
 
 	if (!is_switch_decoder(dev))
 		return 0;
 
 	cxlsd = to_cxl_switch_decoder(dev);
 	r1 = &cxlsd->cxld.hpa_range;
+	r2 = &cxled->cxld.hpa_range;
 
 	if (is_root_decoder(dev))
 		return range_contains(r1, r2);
 	return (r1->start == r2->start && r1->end == r2->end);
 }
 
-static int find_pos_and_ways(struct cxl_port *port, struct range *range,
-			     int *pos, int *ways)
+static int find_pos_and_ways(struct cxl_port *port,
+			     struct cxl_endpoint_decoder *cxled, int *pos,
+			     int *ways)
 {
+	struct range *range = &cxled->cxld.hpa_range;
 	struct cxl_switch_decoder *cxlsd;
 	struct cxl_port *parent;
 	struct device *dev;
@@ -1796,8 +1798,8 @@ static int find_pos_and_ways(struct cxl_port *port, struct range *range,
 	if (!parent)
 		return rc;
 
-	dev = device_find_child(&parent->dev, range,
-				match_switch_decoder_by_range);
+	dev = device_find_child(&parent->dev, cxled,
+				match_cxlsd_to_cxled_by_range);
 	if (!dev) {
 		dev_err(port->uport_dev,
 			"failed to find decoder mapping %#llx-%#llx\n",
@@ -1883,7 +1885,7 @@ static int cxl_calc_interleave_pos(struct cxl_endpoint_decoder *cxled)
 		if (is_cxl_root(iter))
 			break;
 
-		rc = find_pos_and_ways(iter, range, &parent_pos, &parent_ways);
+		rc = find_pos_and_ways(iter, cxled, &parent_pos, &parent_ways);
 		if (rc)
 			return rc;
 
@@ -3342,24 +3344,30 @@ static int devm_cxl_add_dax_region(struct cxl_region *cxlr)
 	return rc;
 }
 
-static int match_decoder_by_range(struct device *dev, const void *data)
+static int match_cxlrd_to_cxled_by_range(struct device *dev, const void *data)
 {
-	const struct range *r1, *r2 = data;
-	struct cxl_decoder *cxld;
+	const struct cxl_endpoint_decoder *cxled = data;
+	struct cxl_root_decoder *cxlrd;
+	const struct range *r1, *r2;
 
-	if (!is_switch_decoder(dev))
+	if (!is_root_decoder(dev))
 		return 0;
 
-	cxld = to_cxl_decoder(dev);
-	r1 = &cxld->hpa_range;
+	cxlrd = to_cxl_root_decoder(dev);
+	r1 = &cxlrd->cxlsd.cxld.hpa_range;
+	r2 = &cxled->cxld.hpa_range;
+
 	return range_contains(r1, r2);
 }
 
 static struct cxl_decoder *
-cxl_port_find_switch_decoder(struct cxl_port *port, struct range *hpa)
+cxl_port_find_root_decoder(struct cxl_port *port,
+			   struct cxl_endpoint_decoder *cxled)
 {
-	struct device *cxld_dev = device_find_child(&port->dev, hpa,
-						    match_decoder_by_range);
+	struct device *cxld_dev;
+
+	cxld_dev = device_find_child(&port->dev, cxled,
+				     match_cxlrd_to_cxled_by_range);
 
 	return cxld_dev ? to_cxl_decoder(cxld_dev) : NULL;
 }
@@ -3371,9 +3379,8 @@ cxl_find_root_decoder(struct cxl_endpoint_decoder *cxled)
 	struct cxl_port *port = cxled_to_port(cxled);
 	struct cxl_root *cxl_root __free(put_cxl_root) = find_cxl_root(port);
 	struct cxl_decoder *root, *cxld = &cxled->cxld;
-	struct range *hpa = &cxld->hpa_range;
 
-	root = cxl_port_find_switch_decoder(&cxl_root->port, hpa);
+	root = cxl_port_find_root_decoder(&cxl_root->port, cxled);
 	if (!root) {
 		dev_err(cxlmd->dev.parent,
 			"%s:%s no CXL window for range %#llx:%#llx\n",
@@ -3385,11 +3392,12 @@ cxl_find_root_decoder(struct cxl_endpoint_decoder *cxled)
 	return to_cxl_root_decoder(&root->dev);
 }
 
-static int match_region_by_range(struct device *dev, const void *data)
+static int match_region_to_cxled_by_range(struct device *dev, const void *data)
 {
+	const struct cxl_endpoint_decoder *cxled = data;
+	const struct range *r = &cxled->cxld.hpa_range;
 	struct cxl_region_params *p;
 	struct cxl_region *cxlr;
-	const struct range *r = data;
 
 	if (!is_cxl_region(dev))
 		return 0;
@@ -3547,12 +3555,13 @@ static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd,
 }
 
 static struct cxl_region *
-cxl_find_region_by_range(struct cxl_root_decoder *cxlrd, struct range *hpa)
+cxl_find_region_by_range(struct cxl_root_decoder *cxlrd,
+			 struct cxl_endpoint_decoder *cxled)
 {
 	struct device *region_dev;
 
-	region_dev = device_find_child(&cxlrd->cxlsd.cxld.dev, hpa,
-				       match_region_by_range);
+	region_dev = device_find_child(&cxlrd->cxlsd.cxld.dev, cxled,
+				       match_region_to_cxled_by_range);
 	if (!region_dev)
 		return NULL;
 
@@ -3561,7 +3570,6 @@ cxl_find_region_by_range(struct cxl_root_decoder *cxlrd, struct range *hpa)
 
 int cxl_add_to_region(struct cxl_endpoint_decoder *cxled)
 {
-	struct range *hpa = &cxled->cxld.hpa_range;
 	struct cxl_region_params *p;
 	bool attach = false;
 	int rc;
@@ -3577,7 +3585,7 @@ int cxl_add_to_region(struct cxl_endpoint_decoder *cxled)
 	 */
 	mutex_lock(&cxlrd->range_lock);
 	struct cxl_region *cxlr __free(put_cxl_region) =
-		cxl_find_region_by_range(cxlrd, hpa);
+		cxl_find_region_by_range(cxlrd, cxled);
 	if (!cxlr)
 		cxlr = construct_region(cxlrd, cxled);
 	mutex_unlock(&cxlrd->range_lock);
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 2/4 v5] cxl/core: Add helpers to detect Low Memory Holes on x86
  2025-10-06 15:58 [PATCH 0/4 v5] cxl/core: Enable Region creation/attach on x86 with LMH Fabio M. De Francesco
  2025-10-06 15:58 ` [PATCH 1/4 v5] cxl/core: Change match_*_by_range() signatures Fabio M. De Francesco
@ 2025-10-06 15:58 ` Fabio M. De Francesco
  2025-10-06 17:40   ` Gregory Price
                     ` (5 more replies)
  2025-10-06 15:58 ` [PATCH 3/4 v5] cxl/core: Enable Region creation on x86 with LMH Fabio M. De Francesco
  2025-10-06 15:58 ` [PATCH 4/4 v5] cxl/test: Simulate an x86 Low Memory Hole for tests Fabio M. De Francesco
  3 siblings, 6 replies; 24+ messages in thread
From: Fabio M. De Francesco @ 2025-10-06 15:58 UTC (permalink / raw)
  To: linux-cxl
  Cc: Davidlohr Bueso, Jonathan Cameron, Dave Jiang, Alison Schofield,
	Vishal Verma, Ira Weiny, Dan Williams, Jonathan Corbet, linux-doc,
	linux-kernel, Gregory Price, Robert Richter, Cheatham Benjamin,
	Fabio M. De Francesco

On a x86 platform with a low memory hole (LHM), the BIOS may publish
CFMWS that describes a system physical address (SPA) range that
typically is only a subset of the corresponding CXL intermediate switch
and endpoint decoder's host physical address (HPA) ranges. The CFMWS
range never intersects the LHM and so the driver instantiates a root
decoder whose HPA range size doesn't fully contain the matching switch
and endpoint decoders' HPA ranges.[1]

To construct regions and attach decoders, the driver needs to match root
decoders and regions with endpoint decoders. The process fails and
returns errors because the driver is not designed to deal with SPA
ranges which are smaller than the corresponding hardware decoders HPA
ranges.

Introduce two functions that indirectly detect the presence of x86 LMH
and allow the matching between a root decoder or an already constructed
region with a corresponding intermediate switch or endpoint decoder to
enable the construction of a region and the subsequent attachment of the
same decoders to that region.

These functions return true when SPA/HPA misalignments due to LMH's are
detected under specific conditions:

- Both the SPA and HPA ranges must start at LMH_CFMWS_RANGE_START (i.e.,
  0x0 on x86 with LMH's).
- The SPA range's size is less than HPA's.
- The SPA range's size is less than 4G.
- The HPA range's size is aligned to the NIW * 256M rule.

Also introduce a function that adjusts the range end of a region to be
constructed and the DPA range's end of the endpoint decoders that will
be later attached to that region.

[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/Kconfig                |  4 ++
 drivers/cxl/core/Makefile          |  1 +
 drivers/cxl/core/platform_quirks.c | 99 ++++++++++++++++++++++++++++++
 drivers/cxl/core/platform_quirks.h | 33 ++++++++++
 4 files changed, 137 insertions(+)
 create mode 100644 drivers/cxl/core/platform_quirks.c
 create mode 100644 drivers/cxl/core/platform_quirks.h

diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig
index 48b7314afdb8..03c0583bc9a3 100644
--- a/drivers/cxl/Kconfig
+++ b/drivers/cxl/Kconfig
@@ -211,6 +211,10 @@ config CXL_REGION
 
 	  If unsure say 'y'
 
+config CXL_PLATFORM_QUIRKS
+	def_bool y
+	depends on CXL_REGION
+
 config CXL_REGION_INVALIDATION_TEST
 	bool "CXL: Region Cache Management Bypass (TEST)"
 	depends on CXL_REGION
diff --git a/drivers/cxl/core/Makefile b/drivers/cxl/core/Makefile
index 5ad8fef210b5..1684e46b8709 100644
--- a/drivers/cxl/core/Makefile
+++ b/drivers/cxl/core/Makefile
@@ -17,6 +17,7 @@ cxl_core-y += cdat.o
 cxl_core-y += ras.o
 cxl_core-$(CONFIG_TRACING) += trace.o
 cxl_core-$(CONFIG_CXL_REGION) += region.o
+cxl_core-$(CONFIG_CXL_PLATFORM_QUIRKS) += platform_quirks.o
 cxl_core-$(CONFIG_CXL_MCE) += mce.o
 cxl_core-$(CONFIG_CXL_FEATURES) += features.o
 cxl_core-$(CONFIG_CXL_EDAC_MEM_FEATURES) += edac.o
diff --git a/drivers/cxl/core/platform_quirks.c b/drivers/cxl/core/platform_quirks.c
new file mode 100644
index 000000000000..7e76e392b1ae
--- /dev/null
+++ b/drivers/cxl/core/platform_quirks.c
@@ -0,0 +1,99 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/range.h>
+#include "platform_quirks.h"
+#include "cxlmem.h"
+#include "core.h"
+
+/* Start of CFMWS range that end before x86 Low Memory Holes */
+#define LMH_CFMWS_RANGE_START 0x0ULL
+
+/**
+ * platform_cxlrd_matches_cxled() - Platform quirk to match CXL Root and
+ * Endpoint Decoders. It allows matching on platforms with LMH's.
+ * @cxlrd: The Root Decoder against which @cxled is tested for matching.
+ * @cxled: The Endpoint Decoder to be tested for matching @cxlrd.
+ *
+ * platform_cxlrd_matches_cxled() is typically called from the
+ * match_*_by_range() functions in region.c. It checks if an endpoint decoder
+ * matches a given root decoder and returns true to allow the driver to succeed
+ * in the construction of regions where it would otherwise fail for the presence
+ * of a Low Memory Hole (see Documentation/driver-api/cxl/conventions.rst).
+ *
+ * In x86 platforms with LMH's, the CFMWS ranges never intersect the LMH, the
+ * endpoint decoder's HPA range size is always guaranteed aligned to NIW*256MB
+ * and also typically larger than the matching root decoder's, and the root
+ * decoder's range end is at an address that is necessarily less than SZ_4G
+ * (i.e., the Hole is in Low Memory - this function doesn't deal with other
+ * kinds of holes).
+ *
+ * Return: true if an endpoint matches a root decoder, else false.
+ */
+bool platform_cxlrd_matches_cxled(const struct cxl_root_decoder *cxlrd,
+				  const struct cxl_endpoint_decoder *cxled)
+{
+	const struct range *rd_r, *sd_r;
+	int align;
+
+	rd_r = &cxlrd->cxlsd.cxld.hpa_range;
+	sd_r = &cxled->cxld.hpa_range;
+	align = cxled->cxld.interleave_ways * SZ_256M;
+
+	if (rd_r->start == LMH_CFMWS_RANGE_START &&
+	    rd_r->start == sd_r->start && rd_r->end < sd_r->end &&
+	    rd_r->end < (LMH_CFMWS_RANGE_START + SZ_4G) &&
+	    IS_ALIGNED(range_len(sd_r), align))
+		return true;
+
+	return false;
+}
+
+/**
+ * platform_region_matches_cxld() - Platform quirk to match a CXL Region and a
+ * Switch or Endpoint Decoder. It allows matching on platforms with LMH's.
+ * @p: Region Params against which @cxled is matched.
+ * @cxld: Switch or Endpoint Decoder to be tested for matching @p.
+ *
+ * Similar to platform_cxlrd_matches_cxled(), it matches regions and
+ * decoders on platforms with LMH's.
+ *
+ * Return: true if a Decoder matches a Region, else false.
+ */
+bool platform_region_matches_cxld(const struct cxl_region_params *p,
+				  const struct cxl_decoder *cxld)
+{
+	const struct range *r = &cxld->hpa_range;
+	const struct resource *res = p->res;
+	int align = cxld->interleave_ways * SZ_256M;
+
+	if (res->start == LMH_CFMWS_RANGE_START && res->start == r->start &&
+	    res->end < r->end && res->end < (LMH_CFMWS_RANGE_START + SZ_4G) &&
+	    IS_ALIGNED(range_len(r), align))
+		return true;
+
+	return false;
+}
+
+void platform_res_adjust(struct resource *res,
+			 struct cxl_endpoint_decoder *cxled,
+			 const struct cxl_root_decoder *cxlrd)
+{
+	if (!platform_cxlrd_matches_cxled(cxlrd, cxled))
+		return;
+
+	guard(rwsem_write)(&cxl_rwsem.dpa);
+	dev_dbg(cxled_to_memdev(cxled)->dev.parent,
+		"Low Memory Hole detected. Resources were (%s: %pr, %pr)\n",
+		dev_name(&cxled->cxld.dev), res, cxled->dpa_res);
+	if (res) {
+		/* Trim region resource overlap with LMH */
+		res->end = cxlrd->res->end;
+	}
+	/* Match endpoint decoder's DPA resource to root decoder's */
+	cxled->dpa_res->end =
+		cxled->dpa_res->start +
+		resource_size(cxlrd->res) / cxled->cxld.interleave_ways - 1;
+	dev_info(cxled_to_memdev(cxled)->dev.parent,
+		 "Resources have been adjusted for LMH (%s: %pr, %pr)\n",
+		 dev_name(&cxled->cxld.dev), res, cxled->dpa_res);
+}
diff --git a/drivers/cxl/core/platform_quirks.h b/drivers/cxl/core/platform_quirks.h
new file mode 100644
index 000000000000..a15592b4e90e
--- /dev/null
+++ b/drivers/cxl/core/platform_quirks.h
@@ -0,0 +1,33 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include "cxl.h"
+
+#ifdef CONFIG_CXL_PLATFORM_QUIRKS
+bool platform_cxlrd_matches_cxled(const struct cxl_root_decoder *cxlrd,
+				  const struct cxl_endpoint_decoder *cxled);
+bool platform_region_matches_cxld(const struct cxl_region_params *p,
+				  const struct cxl_decoder *cxld);
+void platform_res_adjust(struct resource *res,
+			 struct cxl_endpoint_decoder *cxled,
+			 const struct cxl_root_decoder *cxlrd);
+#else
+static inline bool
+platform_root_decoder_contains(const struct cxl_root_decoder *cxlrd,
+			       const struct cxl_endpoint_decoder *cxled)
+{
+	return false;
+}
+
+static inline bool
+platform_region_matches_cxld(const struct cxl_region_params *p,
+			     const struct cxl_decoder *cxld)
+{
+	return false;
+}
+
+inline void platform_res_adjust(struct resource *res,
+				struct cxl_endpoint_decoder *cxled,
+				const struct cxl_root_decoder *cxlrd)
+{
+}
+#endif /* CONFIG_CXL_PLATFORM_QUIRKS */
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 3/4 v5] cxl/core: Enable Region creation on x86 with LMH
  2025-10-06 15:58 [PATCH 0/4 v5] cxl/core: Enable Region creation/attach on x86 with LMH Fabio M. De Francesco
  2025-10-06 15:58 ` [PATCH 1/4 v5] cxl/core: Change match_*_by_range() signatures Fabio M. De Francesco
  2025-10-06 15:58 ` [PATCH 2/4 v5] cxl/core: Add helpers to detect Low Memory Holes on x86 Fabio M. De Francesco
@ 2025-10-06 15:58 ` Fabio M. De Francesco
  2025-10-06 17:46   ` Gregory Price
  2025-10-09  3:29   ` Alison Schofield
  2025-10-06 15:58 ` [PATCH 4/4 v5] cxl/test: Simulate an x86 Low Memory Hole for tests Fabio M. De Francesco
  3 siblings, 2 replies; 24+ messages in thread
From: Fabio M. De Francesco @ 2025-10-06 15:58 UTC (permalink / raw)
  To: linux-cxl
  Cc: Davidlohr Bueso, Jonathan Cameron, Dave Jiang, Alison Schofield,
	Vishal Verma, Ira Weiny, Dan Williams, Jonathan Corbet, linux-doc,
	linux-kernel, Gregory Price, Robert Richter, Cheatham Benjamin,
	Fabio M. De Francesco

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"
 
 /**
@@ -841,6 +842,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;
 
@@ -849,8 +852,13 @@ 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);
+
+	return platform_region_matches_cxld(p, cxld);
 }
 
 static int match_auto_decoder(struct device *dev, const void *data)
@@ -1770,6 +1778,7 @@ static int match_cxlsd_to_cxled_by_range(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))
@@ -1779,8 +1788,13 @@ static int match_cxlsd_to_cxled_by_range(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_cxlrd_matches_cxled(cxlrd, cxled))
+			return 1;
+	}
 	return (r1->start == r2->start && r1->end == r2->end);
 }
 
@@ -1997,7 +2011,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_cxlrd_matches_cxled(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),
@@ -3357,7 +3371,8 @@ static int match_cxlrd_to_cxled_by_range(struct device *dev, const void *data)
 	r1 = &cxlrd->cxlsd.cxld.hpa_range;
 	r2 = &cxled->cxld.hpa_range;
 
-	return range_contains(r1, r2);
+	return (range_contains(r1, r2)) ||
+		(platform_cxlrd_matches_cxled(cxlrd, cxled));
 }
 
 static struct cxl_decoder *
@@ -3406,8 +3421,12 @@ static int match_region_to_cxled_by_range(struct device *dev, const void *data)
 	p = &cxlr->params;
 
 	guard(rwsem_read)(&cxl_rwsem.region);
-	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_matches_cxld(p, &cxled->cxld))
+			return 1;
+	}
 
 	return 0;
 }
@@ -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);
+
 	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);
 	mutex_unlock(&cxlrd->range_lock);
 
 	rc = PTR_ERR_OR_ZERO(cxlr);
diff --git a/tools/testing/cxl/Kbuild b/tools/testing/cxl/Kbuild
index 0d5ce4b74b9f..205f4c813468 100644
--- a/tools/testing/cxl/Kbuild
+++ b/tools/testing/cxl/Kbuild
@@ -61,6 +61,7 @@ cxl_core-y += $(CXL_CORE_SRC)/cdat.o
 cxl_core-y += $(CXL_CORE_SRC)/ras.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_quirks.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


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 4/4 v5] cxl/test: Simulate an x86 Low Memory Hole for tests
  2025-10-06 15:58 [PATCH 0/4 v5] cxl/core: Enable Region creation/attach on x86 with LMH Fabio M. De Francesco
                   ` (2 preceding siblings ...)
  2025-10-06 15:58 ` [PATCH 3/4 v5] cxl/core: Enable Region creation on x86 with LMH Fabio M. De Francesco
@ 2025-10-06 15:58 ` Fabio M. De Francesco
  2025-10-07 20:37   ` Dave Jiang
  2025-10-09  3:52   ` Alison Schofield
  3 siblings, 2 replies; 24+ messages in thread
From: Fabio M. De Francesco @ 2025-10-06 15:58 UTC (permalink / raw)
  To: linux-cxl
  Cc: Davidlohr Bueso, Jonathan Cameron, Dave Jiang, Alison Schofield,
	Vishal Verma, Ira Weiny, Dan Williams, Jonathan Corbet, linux-doc,
	linux-kernel, Gregory Price, Robert Richter, Cheatham Benjamin,
	Fabio M. De Francesco

Simulate an x86 Low Memory Hole for the CXL tests by changing the first
mock CFMWS range size to 768MB and the CXL Endpoint Decoder HPA range
sizes to 1GB. The auto-created region of cxl-test uses mock_cfmws[0],
therefore the LMH path in the CXL Driver will be exercised every time
the cxl-test module is loaded.

Since mock_cfmws[0] range base address is typically different from the
one published by the BIOS on real hardware, the driver would fail to
create and attach CXL Regions when it's run on the mock environment
created by cxl-tests. Furthermore, cxl-topology.sh, cxl-events.sh, and
cxl-sanitize.sh, would also fail.

To make the above-mentioned tests succeed again, add two "mock" versions
of platform_*() that check the HPA range start of mock_cfmws[0] instead
of LMH_CFMWS_RANGE_START. When cxl_core calls a cxl_core exported
function and that function is mocked by cxl_test, the call chain causes
a circular dependency issue. Then add also two "redirect" versions of
platform_*() to work out the circular dependency issue.

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/platform_quirks.c   | 23 +++++++-----
 drivers/cxl/core/platform_quirks.h   | 20 +++++++++--
 tools/testing/cxl/cxl_core_exports.c | 23 ++++++++++++
 tools/testing/cxl/exports.h          |  7 ++++
 tools/testing/cxl/test/cxl.c         | 54 ++++++++++++++++++++++++++++
 tools/testing/cxl/test/mock.c        | 48 +++++++++++++++++++++++++
 tools/testing/cxl/test/mock.h        |  4 +++
 7 files changed, 168 insertions(+), 11 deletions(-)

diff --git a/drivers/cxl/core/platform_quirks.c b/drivers/cxl/core/platform_quirks.c
index 7e76e392b1ae..aecd376f2766 100644
--- a/drivers/cxl/core/platform_quirks.c
+++ b/drivers/cxl/core/platform_quirks.c
@@ -1,20 +1,23 @@
-// SPDX-License-Identifier: GPL-2.0-only
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* Copyright(c) 2025 Intel Corporation. */
 
 #include <linux/range.h>
+#include <cxlmem.h>
+#include <cxl.h>
+
 #include "platform_quirks.h"
-#include "cxlmem.h"
 #include "core.h"
 
 /* Start of CFMWS range that end before x86 Low Memory Holes */
 #define LMH_CFMWS_RANGE_START 0x0ULL
 
 /**
- * platform_cxlrd_matches_cxled() - Platform quirk to match CXL Root and
+ * __platform_cxlrd_matches_cxled() - Platform quirk to match CXL Root and
  * Endpoint Decoders. It allows matching on platforms with LMH's.
  * @cxlrd: The Root Decoder against which @cxled is tested for matching.
  * @cxled: The Endpoint Decoder to be tested for matching @cxlrd.
  *
- * platform_cxlrd_matches_cxled() is typically called from the
+ * __platform_cxlrd_matches_cxled() is typically called from the
  * match_*_by_range() functions in region.c. It checks if an endpoint decoder
  * matches a given root decoder and returns true to allow the driver to succeed
  * in the construction of regions where it would otherwise fail for the presence
@@ -29,8 +32,8 @@
  *
  * Return: true if an endpoint matches a root decoder, else false.
  */
-bool platform_cxlrd_matches_cxled(const struct cxl_root_decoder *cxlrd,
-				  const struct cxl_endpoint_decoder *cxled)
+bool __platform_cxlrd_matches_cxled(const struct cxl_root_decoder *cxlrd,
+				    const struct cxl_endpoint_decoder *cxled)
 {
 	const struct range *rd_r, *sd_r;
 	int align;
@@ -47,9 +50,10 @@ bool platform_cxlrd_matches_cxled(const struct cxl_root_decoder *cxlrd,
 
 	return false;
 }
+EXPORT_SYMBOL_NS_GPL(__platform_cxlrd_matches_cxled, "CXL");
 
 /**
- * platform_region_matches_cxld() - Platform quirk to match a CXL Region and a
+ * __platform_region_matches_cxld() - Platform quirk to match a CXL Region and a
  * Switch or Endpoint Decoder. It allows matching on platforms with LMH's.
  * @p: Region Params against which @cxled is matched.
  * @cxld: Switch or Endpoint Decoder to be tested for matching @p.
@@ -59,8 +63,8 @@ bool platform_cxlrd_matches_cxled(const struct cxl_root_decoder *cxlrd,
  *
  * Return: true if a Decoder matches a Region, else false.
  */
-bool platform_region_matches_cxld(const struct cxl_region_params *p,
-				  const struct cxl_decoder *cxld)
+bool __platform_region_matches_cxld(const struct cxl_region_params *p,
+				    const struct cxl_decoder *cxld)
 {
 	const struct range *r = &cxld->hpa_range;
 	const struct resource *res = p->res;
@@ -73,6 +77,7 @@ bool platform_region_matches_cxld(const struct cxl_region_params *p,
 
 	return false;
 }
+EXPORT_SYMBOL_NS_GPL(__platform_region_matches_cxld, "CXL");
 
 void platform_res_adjust(struct resource *res,
 			 struct cxl_endpoint_decoder *cxled,
diff --git a/drivers/cxl/core/platform_quirks.h b/drivers/cxl/core/platform_quirks.h
index a15592b4e90e..bdea00365dad 100644
--- a/drivers/cxl/core/platform_quirks.h
+++ b/drivers/cxl/core/platform_quirks.h
@@ -1,4 +1,8 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
+/* Copyright(c) 2025 Intel Corporation. */
+
+#ifndef __PLATFORM_QUIRKS_H__
+#define __PLATFORM_QUIRKS_H__
 
 #include "cxl.h"
 
@@ -7,13 +11,17 @@ bool platform_cxlrd_matches_cxled(const struct cxl_root_decoder *cxlrd,
 				  const struct cxl_endpoint_decoder *cxled);
 bool platform_region_matches_cxld(const struct cxl_region_params *p,
 				  const struct cxl_decoder *cxld);
+bool __platform_cxlrd_matches_cxled(const struct cxl_root_decoder *cxlrd,
+				    const struct cxl_endpoint_decoder *cxled);
+bool __platform_region_matches_cxld(const struct cxl_region_params *p,
+				    const struct cxl_decoder *cxld);
 void platform_res_adjust(struct resource *res,
 			 struct cxl_endpoint_decoder *cxled,
 			 const struct cxl_root_decoder *cxlrd);
 #else
 static inline bool
-platform_root_decoder_contains(const struct cxl_root_decoder *cxlrd,
-			       const struct cxl_endpoint_decoder *cxled)
+platform_cxlrd_matches_cxled(const struct cxl_root_decoder *cxlrd,
+			     const struct cxl_endpoint_decoder *cxled)
 {
 	return false;
 }
@@ -31,3 +39,11 @@ inline void platform_res_adjust(struct resource *res,
 {
 }
 #endif /* CONFIG_CXL_PLATFORM_QUIRKS */
+
+#ifndef CXL_TEST_ENABLE
+#define DECLARE_TESTABLE(x) __##x
+#define platform_cxlrd_matches_cxled DECLARE_TESTABLE(platform_cxlrd_matches_cxled)
+#define platform_region_matches_cxld DECLARE_TESTABLE(platform_region_matches_cxld)
+#endif
+
+#endif /* __PLATFORM_QUIRKS_H__ */
diff --git a/tools/testing/cxl/cxl_core_exports.c b/tools/testing/cxl/cxl_core_exports.c
index 6754de35598d..a9e37156d126 100644
--- a/tools/testing/cxl/cxl_core_exports.c
+++ b/tools/testing/cxl/cxl_core_exports.c
@@ -3,6 +3,7 @@
 
 #include "cxl.h"
 #include "exports.h"
+#include "platform_quirks.h"
 
 /* Exporting of cxl_core symbols that are only used by cxl_test */
 EXPORT_SYMBOL_NS_GPL(cxl_num_decoders_committed, "CXL");
@@ -27,3 +28,25 @@ int devm_cxl_switch_port_decoders_setup(struct cxl_port *port)
 	return _devm_cxl_switch_port_decoders_setup(port);
 }
 EXPORT_SYMBOL_NS_GPL(devm_cxl_switch_port_decoders_setup, "CXL");
+
+platform_cxlrd_matches_cxled_fn _platform_cxlrd_matches_cxled =
+	__platform_cxlrd_matches_cxled;
+EXPORT_SYMBOL_NS_GPL(_platform_cxlrd_matches_cxled, "CXL");
+
+bool platform_cxlrd_matches_cxled(const struct cxl_root_decoder *cxlrd,
+				  const struct cxl_endpoint_decoder *cxled)
+{
+	return _platform_cxlrd_matches_cxled(cxlrd, cxled);
+}
+EXPORT_SYMBOL_NS_GPL(platform_cxlrd_matches_cxled, "CXL");
+
+platform_region_matches_cxld_fn _platform_region_matches_cxld =
+	__platform_region_matches_cxld;
+EXPORT_SYMBOL_NS_GPL(_platform_region_matches_cxld, "CXL");
+
+bool platform_region_matches_cxld(const struct cxl_region_params *p,
+				  const struct cxl_decoder *cxld)
+{
+	return _platform_region_matches_cxld(p, cxld);
+}
+EXPORT_SYMBOL_NS_GPL(platform_region_matches_cxld, "CXL");
diff --git a/tools/testing/cxl/exports.h b/tools/testing/cxl/exports.h
index 7ebee7c0bd67..e0e4c58dadf2 100644
--- a/tools/testing/cxl/exports.h
+++ b/tools/testing/cxl/exports.h
@@ -10,4 +10,11 @@ extern cxl_add_dport_by_dev_fn _devm_cxl_add_dport_by_dev;
 typedef int(*cxl_switch_decoders_setup_fn)(struct cxl_port *port);
 extern cxl_switch_decoders_setup_fn _devm_cxl_switch_port_decoders_setup;
 
+typedef bool(*platform_cxlrd_matches_cxled_fn)(const struct cxl_root_decoder *cxlrd,
+					       const struct cxl_endpoint_decoder *cxled);
+extern platform_cxlrd_matches_cxled_fn _platform_cxlrd_matches_cxled;
+
+typedef bool(*platform_region_matches_cxld_fn)(const struct cxl_region_params *p,
+					       const struct cxl_decoder *cxld);
+extern platform_region_matches_cxld_fn _platform_region_matches_cxld;
 #endif
diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
index 2d135ca533d0..ada431b180f4 100644
--- a/tools/testing/cxl/test/cxl.c
+++ b/tools/testing/cxl/test/cxl.c
@@ -9,6 +9,8 @@
 #include <linux/acpi.h>
 #include <linux/pci.h>
 #include <linux/mm.h>
+
+#include <platform_quirks.h>
 #include <cxlmem.h>
 
 #include "../watermark.h"
@@ -213,7 +215,11 @@ static struct {
 			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
 					ACPI_CEDT_CFMWS_RESTRICT_VOLATILE,
 			.qtg_id = FAKE_QTG_ID,
+#if defined(CONFIG_CXL_PLATFORM_QUIRKS)
+			.window_size = SZ_256M * 3UL,
+#else
 			.window_size = SZ_256M * 4UL,
+#endif
 		},
 		.target = { 0 },
 	},
@@ -426,6 +432,13 @@ static struct cxl_mock_res *alloc_mock_res(resource_size_t size, int align)
 	return res;
 }
 
+static u64 mock_cfmws0_range_start;
+
+static void set_mock_cfmws0_range_start(u64 start)
+{
+	mock_cfmws0_range_start = start;
+}
+
 static int populate_cedt(void)
 {
 	struct cxl_mock_res *res;
@@ -454,6 +467,8 @@ static int populate_cedt(void)
 		if (!res)
 			return -ENOMEM;
 		window->base_hpa = res->range.start;
+		if (i == 0)
+			set_mock_cfmws0_range_start(res->range.start);
 	}
 
 	return 0;
@@ -738,7 +753,11 @@ static void mock_init_hdm_decoder(struct cxl_decoder *cxld)
 	struct cxl_endpoint_decoder *cxled;
 	struct cxl_switch_decoder *cxlsd;
 	struct cxl_port *port, *iter;
+#if defined(CONFIG_CXL_PLATFORM_QUIRKS)
+	const int size = SZ_1G;
+#else
 	const int size = SZ_512M;
+#endif
 	struct cxl_memdev *cxlmd;
 	struct cxl_dport *dport;
 	struct device *dev;
@@ -1103,6 +1122,39 @@ static void mock_cxl_endpoint_parse_cdat(struct cxl_port *port)
 	cxl_endpoint_get_perf_coordinates(port, ep_c);
 }
 
+static bool
+mock_platform_cxlrd_matches_cxled(const struct cxl_root_decoder *cxlrd,
+				  const struct cxl_endpoint_decoder *cxled)
+{
+	const struct range *rd_r, *ed_r;
+	int align;
+
+	rd_r = &cxlrd->cxlsd.cxld.hpa_range;
+	ed_r = &cxled->cxld.hpa_range;
+	align = cxled->cxld.interleave_ways * SZ_256M;
+
+	return rd_r->start == mock_cfmws0_range_start &&
+	       rd_r->start == ed_r->start &&
+	       rd_r->end < (mock_cfmws0_range_start + SZ_4G) &&
+	       rd_r->end < ed_r->end &&
+	       IS_ALIGNED(range_len(ed_r), align);
+}
+
+static bool
+mock_platform_region_matches_cxld(const struct cxl_region_params *p,
+				  const struct cxl_decoder *cxld)
+{
+	const struct range *r = &cxld->hpa_range;
+	const struct resource *res = p->res;
+	int align = cxld->interleave_ways * SZ_256M;
+
+	return res->start == mock_cfmws0_range_start &&
+	       res->start == r->start &&
+	       res->end < (mock_cfmws0_range_start + SZ_4G) &&
+	       res->end < r->end &&
+	       IS_ALIGNED(range_len(r), align);
+}
+
 static struct cxl_mock_ops cxl_mock_ops = {
 	.is_mock_adev = is_mock_adev,
 	.is_mock_bridge = is_mock_bridge,
@@ -1117,6 +1169,8 @@ static struct cxl_mock_ops cxl_mock_ops = {
 	.devm_cxl_port_enumerate_dports = mock_cxl_port_enumerate_dports,
 	.cxl_endpoint_parse_cdat = mock_cxl_endpoint_parse_cdat,
 	.devm_cxl_add_dport_by_dev = mock_cxl_add_dport_by_dev,
+	.platform_cxlrd_matches_cxled = mock_platform_cxlrd_matches_cxled,
+	.platform_region_matches_cxld = mock_platform_region_matches_cxld,
 	.list = LIST_HEAD_INIT(cxl_mock_ops.list),
 };
 
diff --git a/tools/testing/cxl/test/mock.c b/tools/testing/cxl/test/mock.c
index 995269a75cbd..45d8fe67e3e9 100644
--- a/tools/testing/cxl/test/mock.c
+++ b/tools/testing/cxl/test/mock.c
@@ -7,6 +7,8 @@
 #include <linux/export.h>
 #include <linux/acpi.h>
 #include <linux/pci.h>
+
+#include <platform_quirks.h>
 #include <cxlmem.h>
 #include <cxlpci.h>
 #include "mock.h"
@@ -18,6 +20,12 @@ static struct cxl_dport *
 redirect_devm_cxl_add_dport_by_dev(struct cxl_port *port,
 				   struct device *dport_dev);
 static int redirect_devm_cxl_switch_port_decoders_setup(struct cxl_port *port);
+static bool
+redirect_platform_cxlrd_matches_cxled(const struct cxl_root_decoder *cxlrd,
+				      const struct cxl_endpoint_decoder *cxled);
+static bool
+redirect_platform_region_matches_cxld(const struct cxl_region_params *p,
+				      const struct cxl_decoder *cxld);
 
 void register_cxl_mock_ops(struct cxl_mock_ops *ops)
 {
@@ -25,6 +33,8 @@ void register_cxl_mock_ops(struct cxl_mock_ops *ops)
 	_devm_cxl_add_dport_by_dev = redirect_devm_cxl_add_dport_by_dev;
 	_devm_cxl_switch_port_decoders_setup =
 		redirect_devm_cxl_switch_port_decoders_setup;
+	_platform_cxlrd_matches_cxled = redirect_platform_cxlrd_matches_cxled;
+	_platform_region_matches_cxld = redirect_platform_region_matches_cxld;
 }
 EXPORT_SYMBOL_GPL(register_cxl_mock_ops);
 
@@ -35,6 +45,8 @@ void unregister_cxl_mock_ops(struct cxl_mock_ops *ops)
 	_devm_cxl_switch_port_decoders_setup =
 		__devm_cxl_switch_port_decoders_setup;
 	_devm_cxl_add_dport_by_dev = __devm_cxl_add_dport_by_dev;
+	_platform_cxlrd_matches_cxled = __platform_cxlrd_matches_cxled;
+	_platform_region_matches_cxld = __platform_region_matches_cxld;
 	list_del_rcu(&ops->list);
 	synchronize_srcu(&cxl_mock_srcu);
 }
@@ -285,6 +297,42 @@ struct cxl_dport *redirect_devm_cxl_add_dport_by_dev(struct cxl_port *port,
 	return dport;
 }
 
+static bool
+redirect_platform_cxlrd_matches_cxled(const struct cxl_root_decoder *cxlrd,
+				      const struct cxl_endpoint_decoder *cxled)
+{
+	int index;
+	bool match;
+	struct cxl_mock_ops *ops = get_cxl_mock_ops(&index);
+	struct cxl_port *port = to_cxl_port(cxled->cxld.dev.parent);
+
+	if (ops && ops->is_mock_port(port->uport_dev))
+		match = ops->platform_cxlrd_matches_cxled(cxlrd, cxled);
+	else
+		match = __platform_cxlrd_matches_cxled(cxlrd, cxled);
+	put_cxl_mock_ops(index);
+
+	return match;
+}
+
+static bool
+redirect_platform_region_matches_cxld(const struct cxl_region_params *p,
+				      const struct cxl_decoder *cxld)
+{
+	int index;
+	bool match;
+	struct cxl_mock_ops *ops = get_cxl_mock_ops(&index);
+	struct cxl_port *port = to_cxl_port(cxld->dev.parent);
+
+	if (ops && ops->is_mock_port(port->uport_dev))
+		match = ops->platform_region_matches_cxld(p, cxld);
+	else
+		match = __platform_region_matches_cxld(p, cxld);
+	put_cxl_mock_ops(index);
+
+	return match;
+}
+
 MODULE_LICENSE("GPL v2");
 MODULE_DESCRIPTION("cxl_test: emulation module");
 MODULE_IMPORT_NS("ACPI");
diff --git a/tools/testing/cxl/test/mock.h b/tools/testing/cxl/test/mock.h
index 4ed932e76aae..803d7cc0197c 100644
--- a/tools/testing/cxl/test/mock.h
+++ b/tools/testing/cxl/test/mock.h
@@ -25,6 +25,10 @@ struct cxl_mock_ops {
 	void (*cxl_endpoint_parse_cdat)(struct cxl_port *port);
 	struct cxl_dport *(*devm_cxl_add_dport_by_dev)(struct cxl_port *port,
 						       struct device *dport_dev);
+	bool (*platform_cxlrd_matches_cxled)(const struct cxl_root_decoder *cxlrd,
+					     const struct cxl_endpoint_decoder *cxled);
+	bool (*platform_region_matches_cxld)(const struct cxl_region_params *p,
+					     const struct cxl_decoder *cxld);
 };
 
 void register_cxl_mock_ops(struct cxl_mock_ops *ops);
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/4 v5] cxl/core: Change match_*_by_range() signatures
  2025-10-06 15:58 ` [PATCH 1/4 v5] cxl/core: Change match_*_by_range() signatures Fabio M. De Francesco
@ 2025-10-06 17:35   ` Gregory Price
  2025-10-06 23:30   ` Dave Jiang
  1 sibling, 0 replies; 24+ messages in thread
From: Gregory Price @ 2025-10-06 17:35 UTC (permalink / raw)
  To: Fabio M. De Francesco
  Cc: linux-cxl, Davidlohr Bueso, Jonathan Cameron, Dave Jiang,
	Alison Schofield, Vishal Verma, Ira Weiny, Dan Williams,
	Jonathan Corbet, linux-doc, linux-kernel, Robert Richter,
	Cheatham Benjamin

On Mon, Oct 06, 2025 at 05:58:04PM +0200, Fabio M. De Francesco wrote:
> Replace struct range parameter with struct cxl_endpoint_decoder of
> which range is a member in the match_*_by_range() functions and rename
> them according to their semantics.
> 
> This is in preparation for expanding these helpers to perform arch
> specific Root Decoders and Region matchings with
> cxl_endpoint_decoder(s).
> 
> 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>
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>

This may end up conflicting with the AMD ATL patch set, as that set also
I believe made some changes to this area.  But somewhat irrelevant for
the short term.

Reviewed-by: Gregory Price <gourry@gourry.net>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/4 v5] cxl/core: Add helpers to detect Low Memory Holes on x86
  2025-10-06 15:58 ` [PATCH 2/4 v5] cxl/core: Add helpers to detect Low Memory Holes on x86 Fabio M. De Francesco
@ 2025-10-06 17:40   ` Gregory Price
  2025-10-07  0:00   ` Dave Jiang
                     ` (4 subsequent siblings)
  5 siblings, 0 replies; 24+ messages in thread
From: Gregory Price @ 2025-10-06 17:40 UTC (permalink / raw)
  To: Fabio M. De Francesco
  Cc: linux-cxl, Davidlohr Bueso, Jonathan Cameron, Dave Jiang,
	Alison Schofield, Vishal Verma, Ira Weiny, Dan Williams,
	Jonathan Corbet, linux-doc, linux-kernel, Robert Richter,
	Cheatham Benjamin

On Mon, Oct 06, 2025 at 05:58:05PM +0200, Fabio M. De Francesco wrote:
> On a x86 platform with a low memory hole (LHM), the BIOS may publish
                                        ^^^ LMH ^^^

> CFMWS that describes a system physical address (SPA) range that
> typically is only a subset of the corresponding CXL intermediate switch
> and endpoint decoder's host physical address (HPA) ranges. The CFMWS
> range never intersects the LHM and so the driver instantiates a root
> decoder whose HPA range size doesn't fully contain the matching switch
> and endpoint decoders' HPA ranges.[1]
> 
> To construct regions and attach decoders, the driver needs to match root
> decoders and regions with endpoint decoders. The process fails and
> returns errors because the driver is not designed to deal with SPA
> ranges which are smaller than the corresponding hardware decoders HPA
> ranges.
> 
> Introduce two functions that indirectly detect the presence of x86 LMH
> and allow the matching between a root decoder or an already constructed
> region with a corresponding intermediate switch or endpoint decoder to
> enable the construction of a region and the subsequent attachment of the
> same decoders to that region.
> 
> These functions return true when SPA/HPA misalignments due to LMH's are
> detected under specific conditions:
> 
> - Both the SPA and HPA ranges must start at LMH_CFMWS_RANGE_START (i.e.,
>   0x0 on x86 with LMH's).
> - The SPA range's size is less than HPA's.
> - The SPA range's size is less than 4G.
> - The HPA range's size is aligned to the NIW * 256M rule.
> 
> Also introduce a function that adjusts the range end of a region to be
> constructed and the DPA range's end of the endpoint decoders that will
> be later attached to that region.
> 
> [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>

lgmt

Reviewed-by: Gregory Price <gourry@gourry.net>


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/4 v5] cxl/core: Enable Region creation on x86 with LMH
  2025-10-06 15:58 ` [PATCH 3/4 v5] cxl/core: Enable Region creation on x86 with LMH Fabio M. De Francesco
@ 2025-10-06 17:46   ` Gregory Price
  2025-10-07 20:25     ` Dave Jiang
  2025-10-09  3:29   ` Alison Schofield
  1 sibling, 1 reply; 24+ messages in thread
From: Gregory Price @ 2025-10-06 17:46 UTC (permalink / raw)
  To: Fabio M. De Francesco
  Cc: linux-cxl, Davidlohr Bueso, Jonathan Cameron, Dave Jiang,
	Alison Schofield, Vishal Verma, Ira Weiny, Dan Williams,
	Jonathan Corbet, linux-doc, linux-kernel, Robert Richter,
	Cheatham Benjamin

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).
> 
...
> 
> 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>

Couple inlines but just nits

Reviewed-by: Gregory Price <gourry@gourry.net>

> @@ -1770,6 +1778,7 @@ static int match_cxlsd_to_cxled_by_range(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))
> @@ -1779,8 +1788,13 @@ static int match_cxlsd_to_cxled_by_range(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_cxlrd_matches_cxled(cxlrd, cxled))
> +			return 1;
> +	}

Is there any concern for longer term maintainability if addition
match_*() functions are added?  Or is this upkeep just the unfortunate
maintenance cost of supportering the quirk?

>  
>  static struct cxl_decoder *
> @@ -3406,8 +3421,12 @@ static int match_region_to_cxled_by_range(struct device *dev, const void *data)
>  	p = &cxlr->params;
>  
>  	guard(rwsem_read)(&cxl_rwsem.region);
> -	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_matches_cxld(p, &cxled->cxld))
> +			return 1;
> +	}


if (!p->res)
	return 0;
if (p->res->start == r->start && p->res->end == r->end)
	return 1;
if (platform_region_matches_cxld(p, &cxled->cxld))
	return 1;
return 0;

?

I like flat, but I also dislike not-logic.  Style choice here, unless
others have a strong feeling this is fine.

~Gregory

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/4 v5] cxl/core: Change match_*_by_range() signatures
  2025-10-06 15:58 ` [PATCH 1/4 v5] cxl/core: Change match_*_by_range() signatures Fabio M. De Francesco
  2025-10-06 17:35   ` Gregory Price
@ 2025-10-06 23:30   ` Dave Jiang
  1 sibling, 0 replies; 24+ messages in thread
From: Dave Jiang @ 2025-10-06 23:30 UTC (permalink / raw)
  To: Fabio M. De Francesco, linux-cxl
  Cc: Davidlohr Bueso, Jonathan Cameron, Alison Schofield, Vishal Verma,
	Ira Weiny, Dan Williams, Jonathan Corbet, linux-doc, linux-kernel,
	Gregory Price, Robert Richter, Cheatham Benjamin



On 10/6/25 8:58 AM, Fabio M. De Francesco wrote:
> Replace struct range parameter with struct cxl_endpoint_decoder of
> which range is a member in the match_*_by_range() functions and rename
> them according to their semantics.
> 
> This is in preparation for expanding these helpers to perform arch
> specific Root Decoders and Region matchings with
> cxl_endpoint_decoder(s).
> 
> 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>
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> Signed-off-by: Fabio M. De Francesco <fabio.m.de.francesco@linux.intel.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/cxl/core/region.c | 62 ++++++++++++++++++++++-----------------
>  1 file changed, 35 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index e14c1d305b22..43a854036202 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -1766,27 +1766,29 @@ static int cmp_interleave_pos(const void *a, const void *b)
>  	return cxled_a->pos - cxled_b->pos;
>  }
>  
> -static int match_switch_decoder_by_range(struct device *dev,
> -					 const void *data)
> +static int match_cxlsd_to_cxled_by_range(struct device *dev, const void *data)
>  {
> +	const struct cxl_endpoint_decoder *cxled = data;
>  	struct cxl_switch_decoder *cxlsd;
> -	const struct range *r1, *r2 = data;
> -
> +	const struct range *r1, *r2;
>  
>  	if (!is_switch_decoder(dev))
>  		return 0;
>  
>  	cxlsd = to_cxl_switch_decoder(dev);
>  	r1 = &cxlsd->cxld.hpa_range;
> +	r2 = &cxled->cxld.hpa_range;
>  
>  	if (is_root_decoder(dev))
>  		return range_contains(r1, r2);
>  	return (r1->start == r2->start && r1->end == r2->end);
>  }
>  
> -static int find_pos_and_ways(struct cxl_port *port, struct range *range,
> -			     int *pos, int *ways)
> +static int find_pos_and_ways(struct cxl_port *port,
> +			     struct cxl_endpoint_decoder *cxled, int *pos,
> +			     int *ways)
>  {
> +	struct range *range = &cxled->cxld.hpa_range;
>  	struct cxl_switch_decoder *cxlsd;
>  	struct cxl_port *parent;
>  	struct device *dev;
> @@ -1796,8 +1798,8 @@ static int find_pos_and_ways(struct cxl_port *port, struct range *range,
>  	if (!parent)
>  		return rc;
>  
> -	dev = device_find_child(&parent->dev, range,
> -				match_switch_decoder_by_range);
> +	dev = device_find_child(&parent->dev, cxled,
> +				match_cxlsd_to_cxled_by_range);
>  	if (!dev) {
>  		dev_err(port->uport_dev,
>  			"failed to find decoder mapping %#llx-%#llx\n",
> @@ -1883,7 +1885,7 @@ static int cxl_calc_interleave_pos(struct cxl_endpoint_decoder *cxled)
>  		if (is_cxl_root(iter))
>  			break;
>  
> -		rc = find_pos_and_ways(iter, range, &parent_pos, &parent_ways);
> +		rc = find_pos_and_ways(iter, cxled, &parent_pos, &parent_ways);
>  		if (rc)
>  			return rc;
>  
> @@ -3342,24 +3344,30 @@ static int devm_cxl_add_dax_region(struct cxl_region *cxlr)
>  	return rc;
>  }
>  
> -static int match_decoder_by_range(struct device *dev, const void *data)
> +static int match_cxlrd_to_cxled_by_range(struct device *dev, const void *data)
>  {
> -	const struct range *r1, *r2 = data;
> -	struct cxl_decoder *cxld;
> +	const struct cxl_endpoint_decoder *cxled = data;
> +	struct cxl_root_decoder *cxlrd;
> +	const struct range *r1, *r2;
>  
> -	if (!is_switch_decoder(dev))
> +	if (!is_root_decoder(dev))
>  		return 0;
>  
> -	cxld = to_cxl_decoder(dev);
> -	r1 = &cxld->hpa_range;
> +	cxlrd = to_cxl_root_decoder(dev);
> +	r1 = &cxlrd->cxlsd.cxld.hpa_range;
> +	r2 = &cxled->cxld.hpa_range;
> +
>  	return range_contains(r1, r2);
>  }
>  
>  static struct cxl_decoder *
> -cxl_port_find_switch_decoder(struct cxl_port *port, struct range *hpa)
> +cxl_port_find_root_decoder(struct cxl_port *port,
> +			   struct cxl_endpoint_decoder *cxled)
>  {
> -	struct device *cxld_dev = device_find_child(&port->dev, hpa,
> -						    match_decoder_by_range);
> +	struct device *cxld_dev;
> +
> +	cxld_dev = device_find_child(&port->dev, cxled,
> +				     match_cxlrd_to_cxled_by_range);
>  
>  	return cxld_dev ? to_cxl_decoder(cxld_dev) : NULL;
>  }
> @@ -3371,9 +3379,8 @@ cxl_find_root_decoder(struct cxl_endpoint_decoder *cxled)
>  	struct cxl_port *port = cxled_to_port(cxled);
>  	struct cxl_root *cxl_root __free(put_cxl_root) = find_cxl_root(port);
>  	struct cxl_decoder *root, *cxld = &cxled->cxld;
> -	struct range *hpa = &cxld->hpa_range;
>  
> -	root = cxl_port_find_switch_decoder(&cxl_root->port, hpa);
> +	root = cxl_port_find_root_decoder(&cxl_root->port, cxled);
>  	if (!root) {
>  		dev_err(cxlmd->dev.parent,
>  			"%s:%s no CXL window for range %#llx:%#llx\n",
> @@ -3385,11 +3392,12 @@ cxl_find_root_decoder(struct cxl_endpoint_decoder *cxled)
>  	return to_cxl_root_decoder(&root->dev);
>  }
>  
> -static int match_region_by_range(struct device *dev, const void *data)
> +static int match_region_to_cxled_by_range(struct device *dev, const void *data)
>  {
> +	const struct cxl_endpoint_decoder *cxled = data;
> +	const struct range *r = &cxled->cxld.hpa_range;
>  	struct cxl_region_params *p;
>  	struct cxl_region *cxlr;
> -	const struct range *r = data;
>  
>  	if (!is_cxl_region(dev))
>  		return 0;
> @@ -3547,12 +3555,13 @@ static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd,
>  }
>  
>  static struct cxl_region *
> -cxl_find_region_by_range(struct cxl_root_decoder *cxlrd, struct range *hpa)
> +cxl_find_region_by_range(struct cxl_root_decoder *cxlrd,
> +			 struct cxl_endpoint_decoder *cxled)
>  {
>  	struct device *region_dev;
>  
> -	region_dev = device_find_child(&cxlrd->cxlsd.cxld.dev, hpa,
> -				       match_region_by_range);
> +	region_dev = device_find_child(&cxlrd->cxlsd.cxld.dev, cxled,
> +				       match_region_to_cxled_by_range);
>  	if (!region_dev)
>  		return NULL;
>  
> @@ -3561,7 +3570,6 @@ cxl_find_region_by_range(struct cxl_root_decoder *cxlrd, struct range *hpa)
>  
>  int cxl_add_to_region(struct cxl_endpoint_decoder *cxled)
>  {
> -	struct range *hpa = &cxled->cxld.hpa_range;
>  	struct cxl_region_params *p;
>  	bool attach = false;
>  	int rc;
> @@ -3577,7 +3585,7 @@ int cxl_add_to_region(struct cxl_endpoint_decoder *cxled)
>  	 */
>  	mutex_lock(&cxlrd->range_lock);
>  	struct cxl_region *cxlr __free(put_cxl_region) =
> -		cxl_find_region_by_range(cxlrd, hpa);
> +		cxl_find_region_by_range(cxlrd, cxled);
>  	if (!cxlr)
>  		cxlr = construct_region(cxlrd, cxled);
>  	mutex_unlock(&cxlrd->range_lock);


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/4 v5] cxl/core: Add helpers to detect Low Memory Holes on x86
  2025-10-06 15:58 ` [PATCH 2/4 v5] cxl/core: Add helpers to detect Low Memory Holes on x86 Fabio M. De Francesco
  2025-10-06 17:40   ` Gregory Price
@ 2025-10-07  0:00   ` Dave Jiang
  2025-10-09  3:16   ` Alison Schofield
                     ` (3 subsequent siblings)
  5 siblings, 0 replies; 24+ messages in thread
From: Dave Jiang @ 2025-10-07  0:00 UTC (permalink / raw)
  To: Fabio M. De Francesco, linux-cxl
  Cc: Davidlohr Bueso, Jonathan Cameron, Alison Schofield, Vishal Verma,
	Ira Weiny, Dan Williams, Jonathan Corbet, linux-doc, linux-kernel,
	Gregory Price, Robert Richter, Cheatham Benjamin



On 10/6/25 8:58 AM, Fabio M. De Francesco wrote:
> On a x86 platform with a low memory hole (LHM), the BIOS may publish
> CFMWS that describes a system physical address (SPA) range that
> typically is only a subset of the corresponding CXL intermediate switch
> and endpoint decoder's host physical address (HPA) ranges. The CFMWS
> range never intersects the LHM and so the driver instantiates a root
> decoder whose HPA range size doesn't fully contain the matching switch
> and endpoint decoders' HPA ranges.[1]
> 
> To construct regions and attach decoders, the driver needs to match root
> decoders and regions with endpoint decoders. The process fails and
> returns errors because the driver is not designed to deal with SPA
> ranges which are smaller than the corresponding hardware decoders HPA
> ranges.
> 
> Introduce two functions that indirectly detect the presence of x86 LMH
> and allow the matching between a root decoder or an already constructed
> region with a corresponding intermediate switch or endpoint decoder to
> enable the construction of a region and the subsequent attachment of the
> same decoders to that region.
> 
> These functions return true when SPA/HPA misalignments due to LMH's are
> detected under specific conditions:
> 
> - Both the SPA and HPA ranges must start at LMH_CFMWS_RANGE_START (i.e.,
>   0x0 on x86 with LMH's).
> - The SPA range's size is less than HPA's.
> - The SPA range's size is less than 4G.
> - The HPA range's size is aligned to the NIW * 256M rule.
> 
> Also introduce a function that adjusts the range end of a region to be
> constructed and the DPA range's end of the endpoint decoders that will
> be later attached to that region.
> 
> [1] commit 7a81173f3 ("cxl: Documentation/driver-api/cxl: Describe the x86 Low Memory Hole solution")

I don't think this is the right hash.
c5dca38633da ("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/Kconfig                |  4 ++
>  drivers/cxl/core/Makefile          |  1 +
>  drivers/cxl/core/platform_quirks.c | 99 ++++++++++++++++++++++++++++++
>  drivers/cxl/core/platform_quirks.h | 33 ++++++++++
>  4 files changed, 137 insertions(+)
>  create mode 100644 drivers/cxl/core/platform_quirks.c
>  create mode 100644 drivers/cxl/core/platform_quirks.h
> 
> diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig
> index 48b7314afdb8..03c0583bc9a3 100644
> --- a/drivers/cxl/Kconfig
> +++ b/drivers/cxl/Kconfig
> @@ -211,6 +211,10 @@ config CXL_REGION
>  
>  	  If unsure say 'y'
>  
> +config CXL_PLATFORM_QUIRKS
> +	def_bool y
> +	depends on CXL_REGION
> +
>  config CXL_REGION_INVALIDATION_TEST
>  	bool "CXL: Region Cache Management Bypass (TEST)"
>  	depends on CXL_REGION
> diff --git a/drivers/cxl/core/Makefile b/drivers/cxl/core/Makefile
> index 5ad8fef210b5..1684e46b8709 100644
> --- a/drivers/cxl/core/Makefile
> +++ b/drivers/cxl/core/Makefile
> @@ -17,6 +17,7 @@ cxl_core-y += cdat.o
>  cxl_core-y += ras.o
>  cxl_core-$(CONFIG_TRACING) += trace.o
>  cxl_core-$(CONFIG_CXL_REGION) += region.o
> +cxl_core-$(CONFIG_CXL_PLATFORM_QUIRKS) += platform_quirks.o
>  cxl_core-$(CONFIG_CXL_MCE) += mce.o
>  cxl_core-$(CONFIG_CXL_FEATURES) += features.o
>  cxl_core-$(CONFIG_CXL_EDAC_MEM_FEATURES) += edac.o
> diff --git a/drivers/cxl/core/platform_quirks.c b/drivers/cxl/core/platform_quirks.c
> new file mode 100644
> index 000000000000..7e76e392b1ae
> --- /dev/null
> +++ b/drivers/cxl/core/platform_quirks.c
> @@ -0,0 +1,99 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/range.h>
> +#include "platform_quirks.h"
> +#include "cxlmem.h"
> +#include "core.h"
> +
> +/* Start of CFMWS range that end before x86 Low Memory Holes */
> +#define LMH_CFMWS_RANGE_START 0x0ULL
> +
> +/**
> + * platform_cxlrd_matches_cxled() - Platform quirk to match CXL Root and
> + * Endpoint Decoders. It allows matching on platforms with LMH's.
> + * @cxlrd: The Root Decoder against which @cxled is tested for matching.
> + * @cxled: The Endpoint Decoder to be tested for matching @cxlrd.
> + *
> + * platform_cxlrd_matches_cxled() is typically called from the
> + * match_*_by_range() functions in region.c. It checks if an endpoint decoder
> + * matches a given root decoder and returns true to allow the driver to succeed
> + * in the construction of regions where it would otherwise fail for the presence
> + * of a Low Memory Hole (see Documentation/driver-api/cxl/conventions.rst).
> + *
> + * In x86 platforms with LMH's, the CFMWS ranges never intersect the LMH, the
> + * endpoint decoder's HPA range size is always guaranteed aligned to NIW*256MB
> + * and also typically larger than the matching root decoder's, and the root
> + * decoder's range end is at an address that is necessarily less than SZ_4G
> + * (i.e., the Hole is in Low Memory - this function doesn't deal with other
> + * kinds of holes).
> + *
> + * Return: true if an endpoint matches a root decoder, else false.
> + */
> +bool platform_cxlrd_matches_cxled(const struct cxl_root_decoder *cxlrd,
> +				  const struct cxl_endpoint_decoder *cxled)
> +{
> +	const struct range *rd_r, *sd_r;
> +	int align;
> +
> +	rd_r = &cxlrd->cxlsd.cxld.hpa_range;
> +	sd_r = &cxled->cxld.hpa_range;
> +	align = cxled->cxld.interleave_ways * SZ_256M;
> +
> +	if (rd_r->start == LMH_CFMWS_RANGE_START &&
> +	    rd_r->start == sd_r->start && rd_r->end < sd_r->end &&

Break this into 2 lines to make it more readable

> +	    rd_r->end < (LMH_CFMWS_RANGE_START + SZ_4G) &&
> +	    IS_ALIGNED(range_len(sd_r), align))
> +		return true;
> +
> +	return false;
> +}
> +
> +/**
> + * platform_region_matches_cxld() - Platform quirk to match a CXL Region and a
> + * Switch or Endpoint Decoder. It allows matching on platforms with LMH's.
> + * @p: Region Params against which @cxled is matched.
> + * @cxld: Switch or Endpoint Decoder to be tested for matching @p.
> + *
> + * Similar to platform_cxlrd_matches_cxled(), it matches regions and
> + * decoders on platforms with LMH's.
> + *
> + * Return: true if a Decoder matches a Region, else false.
> + */
> +bool platform_region_matches_cxld(const struct cxl_region_params *p,
> +				  const struct cxl_decoder *cxld)
> +{
> +	const struct range *r = &cxld->hpa_range;
> +	const struct resource *res = p->res;
> +	int align = cxld->interleave_ways * SZ_256M;
> +
> +	if (res->start == LMH_CFMWS_RANGE_START && res->start == r->start &&
> +	    res->end < r->end && res->end < (LMH_CFMWS_RANGE_START + SZ_4G) &&

Break the first and this line into 2 lines each with a single compare to make it more readable

> +	    IS_ALIGNED(range_len(r), align))
> +		return true;
> +
> +	return false;
> +}
> +
> +void platform_res_adjust(struct resource *res,
> +			 struct cxl_endpoint_decoder *cxled,
> +			 const struct cxl_root_decoder *cxlrd)
> +{
> +	if (!platform_cxlrd_matches_cxled(cxlrd, cxled))
> +		return;
> +
> +	guard(rwsem_write)(&cxl_rwsem.dpa);
> +	dev_dbg(cxled_to_memdev(cxled)->dev.parent,
> +		"Low Memory Hole detected. Resources were (%s: %pr, %pr)\n",
> +		dev_name(&cxled->cxld.dev), res, cxled->dpa_res);
> +	if (res) {
> +		/* Trim region resource overlap with LMH */
> +		res->end = cxlrd->res->end;
> +	}
> +	/* Match endpoint decoder's DPA resource to root decoder's */
> +	cxled->dpa_res->end =
> +		cxled->dpa_res->start +

this can be on the same line as the first

DJ

> +		resource_size(cxlrd->res) / cxled->cxld.interleave_ways - 1;
> +	dev_info(cxled_to_memdev(cxled)->dev.parent,
> +		 "Resources have been adjusted for LMH (%s: %pr, %pr)\n",
> +		 dev_name(&cxled->cxld.dev), res, cxled->dpa_res);
> +}
> diff --git a/drivers/cxl/core/platform_quirks.h b/drivers/cxl/core/platform_quirks.h
> new file mode 100644
> index 000000000000..a15592b4e90e
> --- /dev/null
> +++ b/drivers/cxl/core/platform_quirks.h
> @@ -0,0 +1,33 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#include "cxl.h"
> +
> +#ifdef CONFIG_CXL_PLATFORM_QUIRKS
> +bool platform_cxlrd_matches_cxled(const struct cxl_root_decoder *cxlrd,
> +				  const struct cxl_endpoint_decoder *cxled);
> +bool platform_region_matches_cxld(const struct cxl_region_params *p,
> +				  const struct cxl_decoder *cxld);
> +void platform_res_adjust(struct resource *res,
> +			 struct cxl_endpoint_decoder *cxled,
> +			 const struct cxl_root_decoder *cxlrd);
> +#else
> +static inline bool
> +platform_root_decoder_contains(const struct cxl_root_decoder *cxlrd,
> +			       const struct cxl_endpoint_decoder *cxled)
> +{
> +	return false;
> +}
> +
> +static inline bool
> +platform_region_matches_cxld(const struct cxl_region_params *p,
> +			     const struct cxl_decoder *cxld)
> +{
> +	return false;
> +}
> +
> +inline void platform_res_adjust(struct resource *res,
> +				struct cxl_endpoint_decoder *cxled,
> +				const struct cxl_root_decoder *cxlrd)
> +{
> +}
> +#endif /* CONFIG_CXL_PLATFORM_QUIRKS */


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/4 v5] cxl/core: Enable Region creation on x86 with LMH
  2025-10-06 17:46   ` Gregory Price
@ 2025-10-07 20:25     ` Dave Jiang
  2025-10-09 14:30       ` Gregory Price
  0 siblings, 1 reply; 24+ messages in thread
From: Dave Jiang @ 2025-10-07 20:25 UTC (permalink / raw)
  To: Gregory Price, Fabio M. De Francesco
  Cc: linux-cxl, Davidlohr Bueso, Jonathan Cameron, Alison Schofield,
	Vishal Verma, Ira Weiny, Dan Williams, Jonathan Corbet, linux-doc,
	linux-kernel, Robert Richter, Cheatham Benjamin



On 10/6/25 10:46 AM, Gregory Price wrote:
> 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).
>>
> ...
>>
>> 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>
> 
> Couple inlines but just nits
> 
> Reviewed-by: Gregory Price <gourry@gourry.net>
> 
>> @@ -1770,6 +1778,7 @@ static int match_cxlsd_to_cxled_by_range(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))
>> @@ -1779,8 +1788,13 @@ static int match_cxlsd_to_cxled_by_range(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_cxlrd_matches_cxled(cxlrd, cxled))
>> +			return 1;
>> +	}
> 
> Is there any concern for longer term maintainability if addition
> match_*() functions are added?  Or is this upkeep just the unfortunate
> maintenance cost of supportering the quirk?

Suggestions welcome. Would be nice if we have cleaner ways of dealing with this.

> 
>>  
>>  static struct cxl_decoder *
>> @@ -3406,8 +3421,12 @@ static int match_region_to_cxled_by_range(struct device *dev, const void *data)
>>  	p = &cxlr->params;
>>  
>>  	guard(rwsem_read)(&cxl_rwsem.region);
>> -	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_matches_cxld(p, &cxled->cxld))
>> +			return 1;
>> +	}
> 
> 
> if (!p->res)
> 	return 0;
> if (p->res->start == r->start && p->res->end == r->end)
> 	return 1;
> if (platform_region_matches_cxld(p, &cxled->cxld))
> 	return 1;
> return 0;
> 
> ?
> 
> I like flat, but I also dislike not-logic.  Style choice here, unless
> others have a strong feeling this is fine.

More flat is definitely the preferred way. With the changes, the last one we can actually do:

return platform_region_matches_cxld(p, &cxled->cxld));


> 
> ~Gregory


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 4/4 v5] cxl/test: Simulate an x86 Low Memory Hole for tests
  2025-10-06 15:58 ` [PATCH 4/4 v5] cxl/test: Simulate an x86 Low Memory Hole for tests Fabio M. De Francesco
@ 2025-10-07 20:37   ` Dave Jiang
  2025-10-09  3:34     ` Alison Schofield
  2025-10-09  3:52   ` Alison Schofield
  1 sibling, 1 reply; 24+ messages in thread
From: Dave Jiang @ 2025-10-07 20:37 UTC (permalink / raw)
  To: Fabio M. De Francesco, linux-cxl
  Cc: Davidlohr Bueso, Jonathan Cameron, Alison Schofield, Vishal Verma,
	Ira Weiny, Dan Williams, Jonathan Corbet, linux-doc, linux-kernel,
	Gregory Price, Robert Richter, Cheatham Benjamin



On 10/6/25 8:58 AM, Fabio M. De Francesco wrote:
> Simulate an x86 Low Memory Hole for the CXL tests by changing the first
> mock CFMWS range size to 768MB and the CXL Endpoint Decoder HPA range
> sizes to 1GB. The auto-created region of cxl-test uses mock_cfmws[0],
> therefore the LMH path in the CXL Driver will be exercised every time
> the cxl-test module is loaded.
> 
> Since mock_cfmws[0] range base address is typically different from the
> one published by the BIOS on real hardware, the driver would fail to
> create and attach CXL Regions when it's run on the mock environment
> created by cxl-tests. Furthermore, cxl-topology.sh, cxl-events.sh, and
> cxl-sanitize.sh, would also fail.
> 
> To make the above-mentioned tests succeed again, add two "mock" versions
> of platform_*() that check the HPA range start of mock_cfmws[0] instead
> of LMH_CFMWS_RANGE_START. When cxl_core calls a cxl_core exported
> function and that function is mocked by cxl_test, the call chain causes
> a circular dependency issue. Then add also two "redirect" versions of
> platform_*() to work out the circular dependency issue.
> 
> 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>

Just a nit below. Otherwise looks ok
Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/cxl/core/platform_quirks.c   | 23 +++++++-----
>  drivers/cxl/core/platform_quirks.h   | 20 +++++++++--
>  tools/testing/cxl/cxl_core_exports.c | 23 ++++++++++++
>  tools/testing/cxl/exports.h          |  7 ++++
>  tools/testing/cxl/test/cxl.c         | 54 ++++++++++++++++++++++++++++
>  tools/testing/cxl/test/mock.c        | 48 +++++++++++++++++++++++++
>  tools/testing/cxl/test/mock.h        |  4 +++
>  7 files changed, 168 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/cxl/core/platform_quirks.c b/drivers/cxl/core/platform_quirks.c
> index 7e76e392b1ae..aecd376f2766 100644
> --- a/drivers/cxl/core/platform_quirks.c
> +++ b/drivers/cxl/core/platform_quirks.c
> @@ -1,20 +1,23 @@
> -// SPDX-License-Identifier: GPL-2.0-only
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/* Copyright(c) 2025 Intel Corporation. */

This change should've happened when the file was first introduced earlier.

DJ

>  
>  #include <linux/range.h>
> +#include <cxlmem.h>
> +#include <cxl.h>
> +
>  #include "platform_quirks.h"
> -#include "cxlmem.h"
>  #include "core.h"
>  
>  /* Start of CFMWS range that end before x86 Low Memory Holes */
>  #define LMH_CFMWS_RANGE_START 0x0ULL
>  
>  /**
> - * platform_cxlrd_matches_cxled() - Platform quirk to match CXL Root and
> + * __platform_cxlrd_matches_cxled() - Platform quirk to match CXL Root and
>   * Endpoint Decoders. It allows matching on platforms with LMH's.
>   * @cxlrd: The Root Decoder against which @cxled is tested for matching.
>   * @cxled: The Endpoint Decoder to be tested for matching @cxlrd.
>   *
> - * platform_cxlrd_matches_cxled() is typically called from the
> + * __platform_cxlrd_matches_cxled() is typically called from the
>   * match_*_by_range() functions in region.c. It checks if an endpoint decoder
>   * matches a given root decoder and returns true to allow the driver to succeed
>   * in the construction of regions where it would otherwise fail for the presence
> @@ -29,8 +32,8 @@
>   *
>   * Return: true if an endpoint matches a root decoder, else false.
>   */
> -bool platform_cxlrd_matches_cxled(const struct cxl_root_decoder *cxlrd,
> -				  const struct cxl_endpoint_decoder *cxled)
> +bool __platform_cxlrd_matches_cxled(const struct cxl_root_decoder *cxlrd,
> +				    const struct cxl_endpoint_decoder *cxled)
>  {
>  	const struct range *rd_r, *sd_r;
>  	int align;
> @@ -47,9 +50,10 @@ bool platform_cxlrd_matches_cxled(const struct cxl_root_decoder *cxlrd,
>  
>  	return false;
>  }
> +EXPORT_SYMBOL_NS_GPL(__platform_cxlrd_matches_cxled, "CXL");
>  
>  /**
> - * platform_region_matches_cxld() - Platform quirk to match a CXL Region and a
> + * __platform_region_matches_cxld() - Platform quirk to match a CXL Region and a
>   * Switch or Endpoint Decoder. It allows matching on platforms with LMH's.
>   * @p: Region Params against which @cxled is matched.
>   * @cxld: Switch or Endpoint Decoder to be tested for matching @p.
> @@ -59,8 +63,8 @@ bool platform_cxlrd_matches_cxled(const struct cxl_root_decoder *cxlrd,
>   *
>   * Return: true if a Decoder matches a Region, else false.
>   */
> -bool platform_region_matches_cxld(const struct cxl_region_params *p,
> -				  const struct cxl_decoder *cxld)
> +bool __platform_region_matches_cxld(const struct cxl_region_params *p,
> +				    const struct cxl_decoder *cxld)
>  {
>  	const struct range *r = &cxld->hpa_range;
>  	const struct resource *res = p->res;
> @@ -73,6 +77,7 @@ bool platform_region_matches_cxld(const struct cxl_region_params *p,
>  
>  	return false;
>  }
> +EXPORT_SYMBOL_NS_GPL(__platform_region_matches_cxld, "CXL");
>  
>  void platform_res_adjust(struct resource *res,
>  			 struct cxl_endpoint_decoder *cxled,
> diff --git a/drivers/cxl/core/platform_quirks.h b/drivers/cxl/core/platform_quirks.h
> index a15592b4e90e..bdea00365dad 100644
> --- a/drivers/cxl/core/platform_quirks.h
> +++ b/drivers/cxl/core/platform_quirks.h
> @@ -1,4 +1,8 @@
>  /* SPDX-License-Identifier: GPL-2.0-only */
> +/* Copyright(c) 2025 Intel Corporation. */
> +
> +#ifndef __PLATFORM_QUIRKS_H__
> +#define __PLATFORM_QUIRKS_H__
>  
>  #include "cxl.h"
>  
> @@ -7,13 +11,17 @@ bool platform_cxlrd_matches_cxled(const struct cxl_root_decoder *cxlrd,
>  				  const struct cxl_endpoint_decoder *cxled);
>  bool platform_region_matches_cxld(const struct cxl_region_params *p,
>  				  const struct cxl_decoder *cxld);
> +bool __platform_cxlrd_matches_cxled(const struct cxl_root_decoder *cxlrd,
> +				    const struct cxl_endpoint_decoder *cxled);
> +bool __platform_region_matches_cxld(const struct cxl_region_params *p,
> +				    const struct cxl_decoder *cxld);
>  void platform_res_adjust(struct resource *res,
>  			 struct cxl_endpoint_decoder *cxled,
>  			 const struct cxl_root_decoder *cxlrd);
>  #else
>  static inline bool
> -platform_root_decoder_contains(const struct cxl_root_decoder *cxlrd,
> -			       const struct cxl_endpoint_decoder *cxled)
> +platform_cxlrd_matches_cxled(const struct cxl_root_decoder *cxlrd,
> +			     const struct cxl_endpoint_decoder *cxled)
>  {
>  	return false;
>  }
> @@ -31,3 +39,11 @@ inline void platform_res_adjust(struct resource *res,
>  {
>  }
>  #endif /* CONFIG_CXL_PLATFORM_QUIRKS */
> +
> +#ifndef CXL_TEST_ENABLE
> +#define DECLARE_TESTABLE(x) __##x
> +#define platform_cxlrd_matches_cxled DECLARE_TESTABLE(platform_cxlrd_matches_cxled)
> +#define platform_region_matches_cxld DECLARE_TESTABLE(platform_region_matches_cxld)
> +#endif
> +
> +#endif /* __PLATFORM_QUIRKS_H__ */
> diff --git a/tools/testing/cxl/cxl_core_exports.c b/tools/testing/cxl/cxl_core_exports.c
> index 6754de35598d..a9e37156d126 100644
> --- a/tools/testing/cxl/cxl_core_exports.c
> +++ b/tools/testing/cxl/cxl_core_exports.c
> @@ -3,6 +3,7 @@
>  
>  #include "cxl.h"
>  #include "exports.h"
> +#include "platform_quirks.h"
>  
>  /* Exporting of cxl_core symbols that are only used by cxl_test */
>  EXPORT_SYMBOL_NS_GPL(cxl_num_decoders_committed, "CXL");
> @@ -27,3 +28,25 @@ int devm_cxl_switch_port_decoders_setup(struct cxl_port *port)
>  	return _devm_cxl_switch_port_decoders_setup(port);
>  }
>  EXPORT_SYMBOL_NS_GPL(devm_cxl_switch_port_decoders_setup, "CXL");
> +
> +platform_cxlrd_matches_cxled_fn _platform_cxlrd_matches_cxled =
> +	__platform_cxlrd_matches_cxled;
> +EXPORT_SYMBOL_NS_GPL(_platform_cxlrd_matches_cxled, "CXL");
> +
> +bool platform_cxlrd_matches_cxled(const struct cxl_root_decoder *cxlrd,
> +				  const struct cxl_endpoint_decoder *cxled)
> +{
> +	return _platform_cxlrd_matches_cxled(cxlrd, cxled);
> +}
> +EXPORT_SYMBOL_NS_GPL(platform_cxlrd_matches_cxled, "CXL");
> +
> +platform_region_matches_cxld_fn _platform_region_matches_cxld =
> +	__platform_region_matches_cxld;
> +EXPORT_SYMBOL_NS_GPL(_platform_region_matches_cxld, "CXL");
> +
> +bool platform_region_matches_cxld(const struct cxl_region_params *p,
> +				  const struct cxl_decoder *cxld)
> +{
> +	return _platform_region_matches_cxld(p, cxld);
> +}
> +EXPORT_SYMBOL_NS_GPL(platform_region_matches_cxld, "CXL");
> diff --git a/tools/testing/cxl/exports.h b/tools/testing/cxl/exports.h
> index 7ebee7c0bd67..e0e4c58dadf2 100644
> --- a/tools/testing/cxl/exports.h
> +++ b/tools/testing/cxl/exports.h
> @@ -10,4 +10,11 @@ extern cxl_add_dport_by_dev_fn _devm_cxl_add_dport_by_dev;
>  typedef int(*cxl_switch_decoders_setup_fn)(struct cxl_port *port);
>  extern cxl_switch_decoders_setup_fn _devm_cxl_switch_port_decoders_setup;
>  
> +typedef bool(*platform_cxlrd_matches_cxled_fn)(const struct cxl_root_decoder *cxlrd,
> +					       const struct cxl_endpoint_decoder *cxled);
> +extern platform_cxlrd_matches_cxled_fn _platform_cxlrd_matches_cxled;
> +
> +typedef bool(*platform_region_matches_cxld_fn)(const struct cxl_region_params *p,
> +					       const struct cxl_decoder *cxld);
> +extern platform_region_matches_cxld_fn _platform_region_matches_cxld;
>  #endif
> diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
> index 2d135ca533d0..ada431b180f4 100644
> --- a/tools/testing/cxl/test/cxl.c
> +++ b/tools/testing/cxl/test/cxl.c
> @@ -9,6 +9,8 @@
>  #include <linux/acpi.h>
>  #include <linux/pci.h>
>  #include <linux/mm.h>
> +
> +#include <platform_quirks.h>
>  #include <cxlmem.h>
>  
>  #include "../watermark.h"
> @@ -213,7 +215,11 @@ static struct {
>  			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
>  					ACPI_CEDT_CFMWS_RESTRICT_VOLATILE,
>  			.qtg_id = FAKE_QTG_ID,
> +#if defined(CONFIG_CXL_PLATFORM_QUIRKS)
> +			.window_size = SZ_256M * 3UL,
> +#else
>  			.window_size = SZ_256M * 4UL,
> +#endif
>  		},
>  		.target = { 0 },
>  	},
> @@ -426,6 +432,13 @@ static struct cxl_mock_res *alloc_mock_res(resource_size_t size, int align)
>  	return res;
>  }
>  
> +static u64 mock_cfmws0_range_start;
> +
> +static void set_mock_cfmws0_range_start(u64 start)
> +{
> +	mock_cfmws0_range_start = start;
> +}
> +
>  static int populate_cedt(void)
>  {
>  	struct cxl_mock_res *res;
> @@ -454,6 +467,8 @@ static int populate_cedt(void)
>  		if (!res)
>  			return -ENOMEM;
>  		window->base_hpa = res->range.start;
> +		if (i == 0)
> +			set_mock_cfmws0_range_start(res->range.start);
>  	}
>  
>  	return 0;
> @@ -738,7 +753,11 @@ static void mock_init_hdm_decoder(struct cxl_decoder *cxld)
>  	struct cxl_endpoint_decoder *cxled;
>  	struct cxl_switch_decoder *cxlsd;
>  	struct cxl_port *port, *iter;
> +#if defined(CONFIG_CXL_PLATFORM_QUIRKS)
> +	const int size = SZ_1G;
> +#else
>  	const int size = SZ_512M;
> +#endif
>  	struct cxl_memdev *cxlmd;
>  	struct cxl_dport *dport;
>  	struct device *dev;
> @@ -1103,6 +1122,39 @@ static void mock_cxl_endpoint_parse_cdat(struct cxl_port *port)
>  	cxl_endpoint_get_perf_coordinates(port, ep_c);
>  }
>  
> +static bool
> +mock_platform_cxlrd_matches_cxled(const struct cxl_root_decoder *cxlrd,
> +				  const struct cxl_endpoint_decoder *cxled)
> +{
> +	const struct range *rd_r, *ed_r;
> +	int align;
> +
> +	rd_r = &cxlrd->cxlsd.cxld.hpa_range;
> +	ed_r = &cxled->cxld.hpa_range;
> +	align = cxled->cxld.interleave_ways * SZ_256M;
> +
> +	return rd_r->start == mock_cfmws0_range_start &&
> +	       rd_r->start == ed_r->start &&
> +	       rd_r->end < (mock_cfmws0_range_start + SZ_4G) &&
> +	       rd_r->end < ed_r->end &&
> +	       IS_ALIGNED(range_len(ed_r), align);
> +}
> +
> +static bool
> +mock_platform_region_matches_cxld(const struct cxl_region_params *p,
> +				  const struct cxl_decoder *cxld)
> +{
> +	const struct range *r = &cxld->hpa_range;
> +	const struct resource *res = p->res;
> +	int align = cxld->interleave_ways * SZ_256M;
> +
> +	return res->start == mock_cfmws0_range_start &&
> +	       res->start == r->start &&
> +	       res->end < (mock_cfmws0_range_start + SZ_4G) &&
> +	       res->end < r->end &&
> +	       IS_ALIGNED(range_len(r), align);
> +}
> +
>  static struct cxl_mock_ops cxl_mock_ops = {
>  	.is_mock_adev = is_mock_adev,
>  	.is_mock_bridge = is_mock_bridge,
> @@ -1117,6 +1169,8 @@ static struct cxl_mock_ops cxl_mock_ops = {
>  	.devm_cxl_port_enumerate_dports = mock_cxl_port_enumerate_dports,
>  	.cxl_endpoint_parse_cdat = mock_cxl_endpoint_parse_cdat,
>  	.devm_cxl_add_dport_by_dev = mock_cxl_add_dport_by_dev,
> +	.platform_cxlrd_matches_cxled = mock_platform_cxlrd_matches_cxled,
> +	.platform_region_matches_cxld = mock_platform_region_matches_cxld,
>  	.list = LIST_HEAD_INIT(cxl_mock_ops.list),
>  };
>  
> diff --git a/tools/testing/cxl/test/mock.c b/tools/testing/cxl/test/mock.c
> index 995269a75cbd..45d8fe67e3e9 100644
> --- a/tools/testing/cxl/test/mock.c
> +++ b/tools/testing/cxl/test/mock.c
> @@ -7,6 +7,8 @@
>  #include <linux/export.h>
>  #include <linux/acpi.h>
>  #include <linux/pci.h>
> +
> +#include <platform_quirks.h>
>  #include <cxlmem.h>
>  #include <cxlpci.h>
>  #include "mock.h"
> @@ -18,6 +20,12 @@ static struct cxl_dport *
>  redirect_devm_cxl_add_dport_by_dev(struct cxl_port *port,
>  				   struct device *dport_dev);
>  static int redirect_devm_cxl_switch_port_decoders_setup(struct cxl_port *port);
> +static bool
> +redirect_platform_cxlrd_matches_cxled(const struct cxl_root_decoder *cxlrd,
> +				      const struct cxl_endpoint_decoder *cxled);
> +static bool
> +redirect_platform_region_matches_cxld(const struct cxl_region_params *p,
> +				      const struct cxl_decoder *cxld);
>  
>  void register_cxl_mock_ops(struct cxl_mock_ops *ops)
>  {
> @@ -25,6 +33,8 @@ void register_cxl_mock_ops(struct cxl_mock_ops *ops)
>  	_devm_cxl_add_dport_by_dev = redirect_devm_cxl_add_dport_by_dev;
>  	_devm_cxl_switch_port_decoders_setup =
>  		redirect_devm_cxl_switch_port_decoders_setup;
> +	_platform_cxlrd_matches_cxled = redirect_platform_cxlrd_matches_cxled;
> +	_platform_region_matches_cxld = redirect_platform_region_matches_cxld;
>  }
>  EXPORT_SYMBOL_GPL(register_cxl_mock_ops);
>  
> @@ -35,6 +45,8 @@ void unregister_cxl_mock_ops(struct cxl_mock_ops *ops)
>  	_devm_cxl_switch_port_decoders_setup =
>  		__devm_cxl_switch_port_decoders_setup;
>  	_devm_cxl_add_dport_by_dev = __devm_cxl_add_dport_by_dev;
> +	_platform_cxlrd_matches_cxled = __platform_cxlrd_matches_cxled;
> +	_platform_region_matches_cxld = __platform_region_matches_cxld;
>  	list_del_rcu(&ops->list);
>  	synchronize_srcu(&cxl_mock_srcu);
>  }
> @@ -285,6 +297,42 @@ struct cxl_dport *redirect_devm_cxl_add_dport_by_dev(struct cxl_port *port,
>  	return dport;
>  }
>  
> +static bool
> +redirect_platform_cxlrd_matches_cxled(const struct cxl_root_decoder *cxlrd,
> +				      const struct cxl_endpoint_decoder *cxled)
> +{
> +	int index;
> +	bool match;
> +	struct cxl_mock_ops *ops = get_cxl_mock_ops(&index);
> +	struct cxl_port *port = to_cxl_port(cxled->cxld.dev.parent);
> +
> +	if (ops && ops->is_mock_port(port->uport_dev))
> +		match = ops->platform_cxlrd_matches_cxled(cxlrd, cxled);
> +	else
> +		match = __platform_cxlrd_matches_cxled(cxlrd, cxled);
> +	put_cxl_mock_ops(index);
> +
> +	return match;
> +}
> +
> +static bool
> +redirect_platform_region_matches_cxld(const struct cxl_region_params *p,
> +				      const struct cxl_decoder *cxld)
> +{
> +	int index;
> +	bool match;
> +	struct cxl_mock_ops *ops = get_cxl_mock_ops(&index);
> +	struct cxl_port *port = to_cxl_port(cxld->dev.parent);
> +
> +	if (ops && ops->is_mock_port(port->uport_dev))
> +		match = ops->platform_region_matches_cxld(p, cxld);
> +	else
> +		match = __platform_region_matches_cxld(p, cxld);
> +	put_cxl_mock_ops(index);
> +
> +	return match;
> +}
> +
>  MODULE_LICENSE("GPL v2");
>  MODULE_DESCRIPTION("cxl_test: emulation module");
>  MODULE_IMPORT_NS("ACPI");
> diff --git a/tools/testing/cxl/test/mock.h b/tools/testing/cxl/test/mock.h
> index 4ed932e76aae..803d7cc0197c 100644
> --- a/tools/testing/cxl/test/mock.h
> +++ b/tools/testing/cxl/test/mock.h
> @@ -25,6 +25,10 @@ struct cxl_mock_ops {
>  	void (*cxl_endpoint_parse_cdat)(struct cxl_port *port);
>  	struct cxl_dport *(*devm_cxl_add_dport_by_dev)(struct cxl_port *port,
>  						       struct device *dport_dev);
> +	bool (*platform_cxlrd_matches_cxled)(const struct cxl_root_decoder *cxlrd,
> +					     const struct cxl_endpoint_decoder *cxled);
> +	bool (*platform_region_matches_cxld)(const struct cxl_region_params *p,
> +					     const struct cxl_decoder *cxld);
>  };
>  
>  void register_cxl_mock_ops(struct cxl_mock_ops *ops);


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/4 v5] cxl/core: Add helpers to detect Low Memory Holes on x86
  2025-10-06 15:58 ` [PATCH 2/4 v5] cxl/core: Add helpers to detect Low Memory Holes on x86 Fabio M. De Francesco
  2025-10-06 17:40   ` Gregory Price
  2025-10-07  0:00   ` Dave Jiang
@ 2025-10-09  3:16   ` Alison Schofield
  2025-11-05 18:02     ` Fabio M. De Francesco
  2025-10-10  7:38   ` kernel test robot
                     ` (2 subsequent siblings)
  5 siblings, 1 reply; 24+ messages in thread
From: Alison Schofield @ 2025-10-09  3:16 UTC (permalink / raw)
  To: Fabio M. De Francesco
  Cc: linux-cxl, Davidlohr Bueso, Jonathan Cameron, Dave Jiang,
	Vishal Verma, Ira Weiny, Dan Williams, Jonathan Corbet, linux-doc,
	linux-kernel, Gregory Price, Robert Richter, Cheatham Benjamin

On Mon, Oct 06, 2025 at 05:58:05PM +0200, Fabio M. De Francesco wrote:
> On a x86 platform with a low memory hole (LHM), the BIOS may publish
> CFMWS that describes a system physical address (SPA) range that
> typically is only a subset of the corresponding CXL intermediate switch
> and endpoint decoder's host physical address (HPA) ranges. The CFMWS
> range never intersects the LHM and so the driver instantiates a root
> decoder whose HPA range size doesn't fully contain the matching switch
> and endpoint decoders' HPA ranges.[1]
> 
> To construct regions and attach decoders, the driver needs to match root
> decoders and regions with endpoint decoders. The process fails and
> returns errors because the driver is not designed to deal with SPA
> ranges which are smaller than the corresponding hardware decoders HPA
> ranges.
> 
> Introduce two functions that indirectly detect the presence of x86 LMH
> and allow the matching between a root decoder or an already constructed
> region with a corresponding intermediate switch or endpoint decoder to
> enable the construction of a region and the subsequent attachment of the
> same decoders to that region.
> 
> These functions return true when SPA/HPA misalignments due to LMH's are
> detected under specific conditions:
> 
> - Both the SPA and HPA ranges must start at LMH_CFMWS_RANGE_START (i.e.,
>   0x0 on x86 with LMH's).
> - The SPA range's size is less than HPA's.
> - The SPA range's size is less than 4G.
> - The HPA range's size is aligned to the NIW * 256M rule.
> 
> Also introduce a function that adjusts the range end of a region to be
> constructed and the DPA range's end of the endpoint decoders that will
> be later attached to that region.

Hi Fabio,

Your getting some fresh eyes on some of this with my review.
The adjustment of resources is what caught my eye, and I looked at
platform_res_adjust() in this patch and it's usage in the next patch.


> 
> [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/Kconfig                |  4 ++
>  drivers/cxl/core/Makefile          |  1 +
>  drivers/cxl/core/platform_quirks.c | 99 ++++++++++++++++++++++++++++++
>  drivers/cxl/core/platform_quirks.h | 33 ++++++++++
>  4 files changed, 137 insertions(+)
>  create mode 100644 drivers/cxl/core/platform_quirks.c
>  create mode 100644 drivers/cxl/core/platform_quirks.h
> 
> diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig
> index 48b7314afdb8..03c0583bc9a3 100644
> --- a/drivers/cxl/Kconfig
> +++ b/drivers/cxl/Kconfig
> @@ -211,6 +211,10 @@ config CXL_REGION
>  
>  	  If unsure say 'y'
>  
> +config CXL_PLATFORM_QUIRKS
> +	def_bool y
> +	depends on CXL_REGION
> +

Why no help text for the new CONFIG option?
That text will probably answer my next question: why do we have the
option?

snip


I have comments for the callsites of platform_res_adjust() in the next
patch, but I'll pull some of that back into this patch to keep it all
in one, more logical, place.

There are 2 callsites, and one passes in NULL for 'res' because
at that site we know that the regions struct res has been adjusted.
I felt that was subtle, and that it may be better to just pass in
the 'res' all the time and let the function adjust if needed,
ignore if not needed.

The name platform_res_adjust() suggested that the 'res' as in the
region 'res' was getting adjusted. This is adjusting multiple resources
- the region resource and the endpoint decoder dpa resource. If it's
meant to be kind of opaque, that's ok, but by using _res_ it sure sounds
like it's adjusting the the region resource (when viewed from the call site).

I might have done this in 2 helpers for crispness:
res = platform_adjust_region_resource()
cxled = platform_adjust_endpoint_decoder()

Then you could adjust the region resource once when the region
is constructed, and the endpoint regions every time in 
cxl_add_to_region().

If you are settled with one adjust routine, perhaps just a 
rename to platform_adjust_resources() will make it sound as
broad as it is.


> +void platform_res_adjust(struct resource *res,
> +			 struct cxl_endpoint_decoder *cxled,
> +			 const struct cxl_root_decoder *cxlrd)
> +{
> +	if (!platform_cxlrd_matches_cxled(cxlrd, cxled))
> +		return;
> +
> +	guard(rwsem_write)(&cxl_rwsem.dpa);
> +	dev_dbg(cxled_to_memdev(cxled)->dev.parent,
> +		"Low Memory Hole detected. Resources were (%s: %pr, %pr)\n",
> +		dev_name(&cxled->cxld.dev), res, cxled->dpa_res);
> +	if (res) {
> +		/* Trim region resource overlap with LMH */
> +		res->end = cxlrd->res->end;
> +	}

Prefer dev_info so always appears.
Prefer to see the region name.
I'm guessing the dev_dbg() above and the dev_info() below are written
with the idea that we want the before view only in dev_dbg() and the
after view only in dev_info().

Looks like this now:
[] cxl_core:platform_res_adjust:90: cxl_mock_mem cxl_mem.0: Low Memory Hole detected. Resources were (decoder12.0: [mem 0x3ff010000000-0x3ff04fffffff flags 0x200], [mem 0x00000000-0x1fffffff flags 0x80000200])
[] cxl_mock_mem cxl_mem.0: Resources have been adjusted for LMH (decoder12.0: [mem 0x3ff010000000-0x3ff03fffffff flags 0x200], [mem 0x00000000-0x17ffffff flags 0x80000200])
[] cxl_core:platform_res_adjust:90: cxl_mock_mem cxl_mem.4: Low Memory Hole detected. Resources were (decoder13.0: (null), [mem 0x00000000-0x1fffffff flags 0x80000200])
[] cxl_mock_mem cxl_mem.4: Resources have been adjusted for LMH (decoder13.0: (null), [mem 0x00000000-0x17ffffff flags 0x80000200])

I'll suggest this to emit explicitly what is changing:
[] cxl region0: LMH Low memory hole trims region resource [mem 0x3ff010000000-0x3ff04fffffff flags 0x200] to [mem 0x3ff010000000-0x3ff03fffffff flags 0x200])
[] cxl decoder13.0: LMH Low memory hole trims DPA resource [mem 0x00000000-0x1fffffff flags 0x80000200] to [mem 0x00000000-0x17ffffff flags 0x80000200])
[] cxl decoder17.0: LMH Low memory hole trims DPA resource [mem 0x00000000-0x1fffffff flags 0x80000200] to [mem 0x00000000-0x17ffffff flags 0x80000200])


> +	/* Match endpoint decoder's DPA resource to root decoder's */
A 'Match' would be if the the endpoint and root decoder resource were
same. This is more of adjustment or recalculation of the DPA length.

> +	cxled->dpa_res->end =
> +		cxled->dpa_res->start +
> +		resource_size(cxlrd->res) / cxled->cxld.interleave_ways - 1;

I'm cautious about the use of division and suggest this as the more
bullet-proof kernel style:

	slice = div_u64(resource_size(cxlrd->res), cxled->cxld.interleave_ways);
	cxled->dpa_res->end = cxled->dpa_res->start + slice - 1;



> +	dev_info(cxled_to_memdev(cxled)->dev.parent,
> +		 "Resources have been adjusted for LMH (%s: %pr, %pr)\n",
> +		 dev_name(&cxled->cxld.dev), res, cxled->dpa_res);
> +}

Here's the diff showing how I emmited the that messaging above. I really
wanted to have that region name to emit. This was done keeping the
adjust in one function, but maybe you'll choose to split :)


---
 drivers/cxl/core/platform_quirks.c | 32 ++++++++++++++++++------------
 drivers/cxl/core/platform_quirks.h |  6 ++++--
 drivers/cxl/core/region.c          | 15 ++++++++------
 3 files changed, 32 insertions(+), 21 deletions(-)

diff --git a/drivers/cxl/core/platform_quirks.c b/drivers/cxl/core/platform_quirks.c
index aecd376f2766..aa25770c088a 100644
--- a/drivers/cxl/core/platform_quirks.c
+++ b/drivers/cxl/core/platform_quirks.c
@@ -81,24 +81,30 @@ EXPORT_SYMBOL_NS_GPL(__platform_region_matches_cxld, "CXL");
 
 void platform_res_adjust(struct resource *res,
 			 struct cxl_endpoint_decoder *cxled,
-			 const struct cxl_root_decoder *cxlrd)
+			 const struct cxl_root_decoder *cxlrd,
+			 const struct device *region_dev)
 {
+	struct resource dpa_res_orig = *cxled->dpa_res;
+	u64 slice;
+
 	if (!platform_cxlrd_matches_cxled(cxlrd, cxled))
 		return;
 
 	guard(rwsem_write)(&cxl_rwsem.dpa);
-	dev_dbg(cxled_to_memdev(cxled)->dev.parent,
-		"Low Memory Hole detected. Resources were (%s: %pr, %pr)\n",
-		dev_name(&cxled->cxld.dev), res, cxled->dpa_res);
-	if (res) {
-		/* Trim region resource overlap with LMH */
+
+	/* Region resource will need a trim at first endpoint attach only */
+	if ((res) && (res->end != cxlrd->res->end)) {
+		dev_info(region_dev,
+			 "LMH Low memory hole trims region resource %pr to %pr)\n",
+			 res, cxlrd->res);
 		res->end = cxlrd->res->end;
 	}
-	/* Match endpoint decoder's DPA resource to root decoder's */
-	cxled->dpa_res->end =
-		cxled->dpa_res->start +
-		resource_size(cxlrd->res) / cxled->cxld.interleave_ways - 1;
-	dev_info(cxled_to_memdev(cxled)->dev.parent,
-		 "Resources have been adjusted for LMH (%s: %pr, %pr)\n",
-		 dev_name(&cxled->cxld.dev), res, cxled->dpa_res);
+
+	/* Adjust the endpoint decoder DPA resource end */
+	slice = div_u64(resource_size(cxlrd->res), cxled->cxld.interleave_ways);
+	cxled->dpa_res->end = cxled->dpa_res->start + slice - 1;
+
+	dev_info(&cxled->cxld.dev,
+		 "LMH Low memory hole trims DPA resource %pr to %pr)\n",
+		 &dpa_res_orig, cxled->dpa_res);
 }
diff --git a/drivers/cxl/core/platform_quirks.h b/drivers/cxl/core/platform_quirks.h
index bdea00365dad..55647711cdb4 100644
--- a/drivers/cxl/core/platform_quirks.h
+++ b/drivers/cxl/core/platform_quirks.h
@@ -17,7 +17,8 @@ bool __platform_region_matches_cxld(const struct cxl_region_params *p,
 				    const struct cxl_decoder *cxld);
 void platform_res_adjust(struct resource *res,
 			 struct cxl_endpoint_decoder *cxled,
-			 const struct cxl_root_decoder *cxlrd);
+			 const struct cxl_root_decoder *cxlrd,
+			 const struct device *region_dev);
 #else
 static inline bool
 platform_cxlrd_matches_cxled(const struct cxl_root_decoder *cxlrd,
@@ -35,7 +36,8 @@ platform_region_matches_cxld(const struct cxl_region_params *p,
 
 inline void platform_res_adjust(struct resource *res,
 				struct cxl_endpoint_decoder *cxled,
-				const struct cxl_root_decoder *cxlrd)
+				const struct cxl_root_decoder *cxlrd,
+				const struct device *region_dev);
 {
 }
 #endif /* CONFIG_CXL_PLATFORM_QUIRKS */
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 9a499bfca23d..d4298a61b912 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -3502,7 +3502,7 @@ static int __construct_region(struct cxl_region *cxlr,
 	 * Trim the HPA retrieved from hardware to fit the SPA mapped by the
 	 * platform
 	 */
-	platform_res_adjust(res, cxled, cxlrd);
+	platform_res_adjust(res, cxled, cxlrd, &cxlr->dev);
 
 	rc = cxl_extended_linear_cache_resize(cxlr, res);
 	if (rc && rc != -EOPNOTSUPP) {
@@ -3611,14 +3611,17 @@ int cxl_add_to_region(struct cxl_endpoint_decoder *cxled)
 	mutex_lock(&cxlrd->range_lock);
 	struct cxl_region *cxlr __free(put_cxl_region) =
 		cxl_find_region_by_range(cxlrd, cxled);
-	if (!cxlr)
+	if (!cxlr) {
 		cxlr = construct_region(cxlrd, cxled);
-	else
+	} else {
 		/*
-		 * Adjust the Endpoint Decoder's dpa_res to fit the Region which
-		 * it has to be attached to
+		 * Platform adjustments are done in construct_region()
+		 * for first target, and here for additional targets.
 		 */
-		platform_res_adjust(NULL, cxled, cxlrd);
+		p = &cxlr->params;
+		platform_res_adjust(p->res, cxled, cxlrd, &cxlr->dev);
+	}
+
 	mutex_unlock(&cxlrd->range_lock);
 
 	rc = PTR_ERR_OR_ZERO(cxlr);
-- 
2.37.3

> 

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/4 v5] cxl/core: Enable Region creation on x86 with LMH
  2025-10-06 15:58 ` [PATCH 3/4 v5] cxl/core: Enable Region creation on x86 with LMH Fabio M. De Francesco
  2025-10-06 17:46   ` Gregory Price
@ 2025-10-09  3:29   ` Alison Schofield
  1 sibling, 0 replies; 24+ messages in thread
From: Alison Schofield @ 2025-10-09  3:29 UTC (permalink / raw)
  To: Fabio M. De Francesco
  Cc: linux-cxl, Davidlohr Bueso, Jonathan Cameron, Dave Jiang,
	Vishal Verma, Ira Weiny, Dan Williams, Jonathan Corbet, linux-doc,
	linux-kernel, Gregory Price, Robert Richter, Cheatham Benjamin

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.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 4/4 v5] cxl/test: Simulate an x86 Low Memory Hole for tests
  2025-10-07 20:37   ` Dave Jiang
@ 2025-10-09  3:34     ` Alison Schofield
  0 siblings, 0 replies; 24+ messages in thread
From: Alison Schofield @ 2025-10-09  3:34 UTC (permalink / raw)
  To: Dave Jiang
  Cc: Fabio M. De Francesco, linux-cxl, Davidlohr Bueso,
	Jonathan Cameron, Vishal Verma, Ira Weiny, Dan Williams,
	Jonathan Corbet, linux-doc, linux-kernel, Gregory Price,
	Robert Richter, Cheatham Benjamin

On Tue, Oct 07, 2025 at 01:37:29PM -0700, Dave Jiang wrote:
> 
snip
> 
> > diff --git a/drivers/cxl/core/platform_quirks.c b/drivers/cxl/core/platform_quirks.c
> > index 7e76e392b1ae..aecd376f2766 100644
> > --- a/drivers/cxl/core/platform_quirks.c
> > +++ b/drivers/cxl/core/platform_quirks.c
> > @@ -1,20 +1,23 @@
> > -// SPDX-License-Identifier: GPL-2.0-only
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/* Copyright(c) 2025 Intel Corporation. */
> 
> This change should've happened when the file was first introduced earlier.
> 
> DJ

It's also a checkpatch WARNING. Need to keep the // style.
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 4/4 v5] cxl/test: Simulate an x86 Low Memory Hole for tests
  2025-10-06 15:58 ` [PATCH 4/4 v5] cxl/test: Simulate an x86 Low Memory Hole for tests Fabio M. De Francesco
  2025-10-07 20:37   ` Dave Jiang
@ 2025-10-09  3:52   ` Alison Schofield
  1 sibling, 0 replies; 24+ messages in thread
From: Alison Schofield @ 2025-10-09  3:52 UTC (permalink / raw)
  To: Fabio M. De Francesco
  Cc: linux-cxl, Davidlohr Bueso, Jonathan Cameron, Dave Jiang,
	Vishal Verma, Ira Weiny, Dan Williams, Jonathan Corbet, linux-doc,
	linux-kernel, Gregory Price, Robert Richter, Cheatham Benjamin

On Mon, Oct 06, 2025 at 05:58:07PM +0200, Fabio M. De Francesco wrote:
> Simulate an x86 Low Memory Hole for the CXL tests by changing the first
> mock CFMWS range size to 768MB and the CXL Endpoint Decoder HPA range
> sizes to 1GB. The auto-created region of cxl-test uses mock_cfmws[0],
> therefore the LMH path in the CXL Driver will be exercised every time
> the cxl-test module is loaded.
> 
> Since mock_cfmws[0] range base address is typically different from the
> one published by the BIOS on real hardware, the driver would fail to
> create and attach CXL Regions when it's run on the mock environment
> created by cxl-tests. Furthermore, cxl-topology.sh, cxl-events.sh, and
> cxl-sanitize.sh, would also fail.
> 
> To make the above-mentioned tests succeed again, add two "mock" versions
> of platform_*() that check the HPA range start of mock_cfmws[0] instead
> of LMH_CFMWS_RANGE_START. When cxl_core calls a cxl_core exported
> function and that function is mocked by cxl_test, the call chain causes
> a circular dependency issue. Then add also two "redirect" versions of
> platform_*() to work out the circular dependency issue.

This is some amazing mockness here !!!!

Please make it easier to review and maintain by making these 2 mock
functions match their core counterparts exactly with only one exception:

s/LMH_CFMWS_RANGE_START/mock_cfmws0_range_start

Doable, right?

>  
> +static bool
> +mock_platform_cxlrd_matches_cxled(const struct cxl_root_decoder *cxlrd,
> +				  const struct cxl_endpoint_decoder *cxled)
> +{
> +	const struct range *rd_r, *ed_r;
> +	int align;
> +
> +	rd_r = &cxlrd->cxlsd.cxld.hpa_range;
> +	ed_r = &cxled->cxld.hpa_range;
> +	align = cxled->cxld.interleave_ways * SZ_256M;
> +
> +	return rd_r->start == mock_cfmws0_range_start &&
> +	       rd_r->start == ed_r->start &&
> +	       rd_r->end < (mock_cfmws0_range_start + SZ_4G) &&
> +	       rd_r->end < ed_r->end &&
> +	       IS_ALIGNED(range_len(ed_r), align);
> +}
> +
> +static bool
> +mock_platform_region_matches_cxld(const struct cxl_region_params *p,
> +				  const struct cxl_decoder *cxld)
> +{
> +	const struct range *r = &cxld->hpa_range;
> +	const struct resource *res = p->res;
> +	int align = cxld->interleave_ways * SZ_256M;
> +
> +	return res->start == mock_cfmws0_range_start &&
> +	       res->start == r->start &&
> +	       res->end < (mock_cfmws0_range_start + SZ_4G) &&
> +	       res->end < r->end &&
> +	       IS_ALIGNED(range_len(r), align);
> +}
> +
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/4 v5] cxl/core: Enable Region creation on x86 with LMH
  2025-10-07 20:25     ` Dave Jiang
@ 2025-10-09 14:30       ` Gregory Price
  0 siblings, 0 replies; 24+ messages in thread
From: Gregory Price @ 2025-10-09 14:30 UTC (permalink / raw)
  To: Dave Jiang
  Cc: Fabio M. De Francesco, linux-cxl, Davidlohr Bueso,
	Jonathan Cameron, Alison Schofield, Vishal Verma, Ira Weiny,
	Dan Williams, Jonathan Corbet, linux-doc, linux-kernel,
	Robert Richter, Cheatham Benjamin

On Tue, Oct 07, 2025 at 01:25:11PM -0700, Dave Jiang wrote:
> On 10/6/25 10:46 AM, Gregory Price wrote:
> >> @@ -1779,8 +1788,13 @@ static int match_cxlsd_to_cxled_by_range(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_cxlrd_matches_cxled(cxlrd, cxled))
> >> +			return 1;
> >> +	}
> > 
> > Is there any concern for longer term maintainability if addition
> > match_*() functions are added?  Or is this upkeep just the unfortunate
> > maintenance cost of supportering the quirk?
> 
> Suggestions welcome. Would be nice if we have cleaner ways of dealing with this.
>

Had a bit of a think about it, but nothing immediately pops out
that doesn't just end with more obfuscation.  It is what it is.

~Gregory

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/4 v5] cxl/core: Add helpers to detect Low Memory Holes on x86
  2025-10-06 15:58 ` [PATCH 2/4 v5] cxl/core: Add helpers to detect Low Memory Holes on x86 Fabio M. De Francesco
                     ` (2 preceding siblings ...)
  2025-10-09  3:16   ` Alison Schofield
@ 2025-10-10  7:38   ` kernel test robot
  2025-11-05 18:11     ` Fabio M. De Francesco
  2025-10-10 14:49   ` kernel test robot
  2025-10-28 15:58   ` Jonathan Cameron
  5 siblings, 1 reply; 24+ messages in thread
From: kernel test robot @ 2025-10-10  7:38 UTC (permalink / raw)
  To: Fabio M. De Francesco, linux-cxl
  Cc: oe-kbuild-all, Davidlohr Bueso, Jonathan Cameron, Dave Jiang,
	Alison Schofield, Vishal Verma, Ira Weiny, Dan Williams,
	Jonathan Corbet, linux-doc, linux-kernel, Gregory Price,
	Robert Richter, Cheatham Benjamin, Fabio M. De Francesco

Hi Fabio,

kernel test robot noticed the following build errors:

[auto build test ERROR on 46037455cbb748c5e85071c95f2244e81986eb58]

url:    https://github.com/intel-lab-lkp/linux/commits/Fabio-M-De-Francesco/cxl-core-Change-match_-_by_range-signatures/20251010-111627
base:   46037455cbb748c5e85071c95f2244e81986eb58
patch link:    https://lore.kernel.org/r/20251006155836.791418-3-fabio.m.de.francesco%40linux.intel.com
patch subject: [PATCH 2/4 v5] cxl/core: Add helpers to detect Low Memory Holes on x86
config: i386-buildonly-randconfig-005-20251010 (https://download.01.org/0day-ci/archive/20251010/202510101555.zofjGvZF-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251010/202510101555.zofjGvZF-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510101555.zofjGvZF-lkp@intel.com/

All errors (new ones prefixed by >>):

   ld: drivers/cxl/core/platform_quirks.o: in function `platform_res_adjust':
>> drivers/cxl/core/platform_quirks.c:95:(.text+0x1f1): undefined reference to `__udivdi3'


vim +95 drivers/cxl/core/platform_quirks.c

    76	
    77	void platform_res_adjust(struct resource *res,
    78				 struct cxl_endpoint_decoder *cxled,
    79				 const struct cxl_root_decoder *cxlrd)
    80	{
    81		if (!platform_cxlrd_matches_cxled(cxlrd, cxled))
    82			return;
    83	
    84		guard(rwsem_write)(&cxl_rwsem.dpa);
    85		dev_dbg(cxled_to_memdev(cxled)->dev.parent,
    86			"Low Memory Hole detected. Resources were (%s: %pr, %pr)\n",
    87			dev_name(&cxled->cxld.dev), res, cxled->dpa_res);
    88		if (res) {
    89			/* Trim region resource overlap with LMH */
    90			res->end = cxlrd->res->end;
    91		}
    92		/* Match endpoint decoder's DPA resource to root decoder's */
    93		cxled->dpa_res->end =
    94			cxled->dpa_res->start +
  > 95			resource_size(cxlrd->res) / cxled->cxld.interleave_ways - 1;

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/4 v5] cxl/core: Add helpers to detect Low Memory Holes on x86
  2025-10-06 15:58 ` [PATCH 2/4 v5] cxl/core: Add helpers to detect Low Memory Holes on x86 Fabio M. De Francesco
                     ` (3 preceding siblings ...)
  2025-10-10  7:38   ` kernel test robot
@ 2025-10-10 14:49   ` kernel test robot
  2025-11-05 18:20     ` Fabio M. De Francesco
  2025-10-28 15:58   ` Jonathan Cameron
  5 siblings, 1 reply; 24+ messages in thread
From: kernel test robot @ 2025-10-10 14:49 UTC (permalink / raw)
  To: Fabio M. De Francesco, linux-cxl
  Cc: llvm, oe-kbuild-all, Davidlohr Bueso, Jonathan Cameron,
	Dave Jiang, Alison Schofield, Vishal Verma, Ira Weiny,
	Dan Williams, Jonathan Corbet, linux-doc, linux-kernel,
	Gregory Price, Robert Richter, Cheatham Benjamin,
	Fabio M. De Francesco

Hi Fabio,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 46037455cbb748c5e85071c95f2244e81986eb58]

url:    https://github.com/intel-lab-lkp/linux/commits/Fabio-M-De-Francesco/cxl-core-Change-match_-_by_range-signatures/20251010-111627
base:   46037455cbb748c5e85071c95f2244e81986eb58
patch link:    https://lore.kernel.org/r/20251006155836.791418-3-fabio.m.de.francesco%40linux.intel.com
patch subject: [PATCH 2/4 v5] cxl/core: Add helpers to detect Low Memory Holes on x86
config: i386-randconfig-011-20251010 (https://download.01.org/0day-ci/archive/20251010/202510102229.iFcGqbMH-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251010/202510102229.iFcGqbMH-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510102229.iFcGqbMH-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/cxl/core/platform_quirks.c:70:36: warning: result of comparison of constant 4294967296 with expression of type 'const resource_size_t' (aka 'const unsigned int') is always true [-Wtautological-constant-out-of-range-compare]
      70 |             res->end < r->end && res->end < (LMH_CFMWS_RANGE_START + SZ_4G) &&
         |                                  ~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   1 warning generated.


vim +70 drivers/cxl/core/platform_quirks.c

    50	
    51	/**
    52	 * platform_region_matches_cxld() - Platform quirk to match a CXL Region and a
    53	 * Switch or Endpoint Decoder. It allows matching on platforms with LMH's.
    54	 * @p: Region Params against which @cxled is matched.
    55	 * @cxld: Switch or Endpoint Decoder to be tested for matching @p.
    56	 *
    57	 * Similar to platform_cxlrd_matches_cxled(), it matches regions and
    58	 * decoders on platforms with LMH's.
    59	 *
    60	 * Return: true if a Decoder matches a Region, else false.
    61	 */
    62	bool platform_region_matches_cxld(const struct cxl_region_params *p,
    63					  const struct cxl_decoder *cxld)
    64	{
    65		const struct range *r = &cxld->hpa_range;
    66		const struct resource *res = p->res;
    67		int align = cxld->interleave_ways * SZ_256M;
    68	
    69		if (res->start == LMH_CFMWS_RANGE_START && res->start == r->start &&
  > 70		    res->end < r->end && res->end < (LMH_CFMWS_RANGE_START + SZ_4G) &&
    71		    IS_ALIGNED(range_len(r), align))
    72			return true;
    73	
    74		return false;
    75	}
    76	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/4 v5] cxl/core: Add helpers to detect Low Memory Holes on x86
  2025-10-06 15:58 ` [PATCH 2/4 v5] cxl/core: Add helpers to detect Low Memory Holes on x86 Fabio M. De Francesco
                     ` (4 preceding siblings ...)
  2025-10-10 14:49   ` kernel test robot
@ 2025-10-28 15:58   ` Jonathan Cameron
  5 siblings, 0 replies; 24+ messages in thread
From: Jonathan Cameron @ 2025-10-28 15:58 UTC (permalink / raw)
  To: Fabio M. De Francesco
  Cc: linux-cxl, Davidlohr Bueso, Dave Jiang, Alison Schofield,
	Vishal Verma, Ira Weiny, Dan Williams, Jonathan Corbet, linux-doc,
	linux-kernel, Gregory Price, Robert Richter, Cheatham Benjamin

On Mon,  6 Oct 2025 17:58:05 +0200
"Fabio M. De Francesco" <fabio.m.de.francesco@linux.intel.com> wrote:

> On a x86 platform with a low memory hole (LHM), the BIOS may publish
> CFMWS that describes a system physical address (SPA) range that
> typically is only a subset of the corresponding CXL intermediate switch
> and endpoint decoder's host physical address (HPA) ranges. The CFMWS
> range never intersects the LHM and so the driver instantiates a root
> decoder whose HPA range size doesn't fully contain the matching switch
> and endpoint decoders' HPA ranges.[1]
> 
> To construct regions and attach decoders, the driver needs to match root
> decoders and regions with endpoint decoders. The process fails and
> returns errors because the driver is not designed to deal with SPA
> ranges which are smaller than the corresponding hardware decoders HPA
> ranges.
> 
> Introduce two functions that indirectly detect the presence of x86 LMH
> and allow the matching between a root decoder or an already constructed
> region with a corresponding intermediate switch or endpoint decoder to
> enable the construction of a region and the subsequent attachment of the
> same decoders to that region.
> 
> These functions return true when SPA/HPA misalignments due to LMH's are
> detected under specific conditions:
> 
> - Both the SPA and HPA ranges must start at LMH_CFMWS_RANGE_START (i.e.,
>   0x0 on x86 with LMH's).
> - The SPA range's size is less than HPA's.
> - The SPA range's size is less than 4G.
> - The HPA range's size is aligned to the NIW * 256M rule.
> 
> Also introduce a function that adjusts the range end of a region to be
> constructed and the DPA range's end of the endpoint decoders that will
> be later attached to that region.
> 
> [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>
A few minor comments from me.

> diff --git a/drivers/cxl/core/platform_quirks.c b/drivers/cxl/core/platform_quirks.c
> new file mode 100644
> index 000000000000..7e76e392b1ae
> --- /dev/null
> +++ b/drivers/cxl/core/platform_quirks.c
> @@ -0,0 +1,99 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/range.h>
> +#include "platform_quirks.h"
> +#include "cxlmem.h"
> +#include "core.h"
> +
> +/* Start of CFMWS range that end before x86 Low Memory Holes */
> +#define LMH_CFMWS_RANGE_START 0x0ULL
> +
> +/**
> + * platform_cxlrd_matches_cxled() - Platform quirk to match CXL Root and
> + * Endpoint Decoders. It allows matching on platforms with LMH's.
> + * @cxlrd: The Root Decoder against which @cxled is tested for matching.
> + * @cxled: The Endpoint Decoder to be tested for matching @cxlrd.
> + *
> + * platform_cxlrd_matches_cxled() is typically called from the
> + * match_*_by_range() functions in region.c. It checks if an endpoint decoder
> + * matches a given root decoder and returns true to allow the driver to succeed
> + * in the construction of regions where it would otherwise fail for the presence
> + * of a Low Memory Hole (see Documentation/driver-api/cxl/conventions.rst).
> + *
> + * In x86 platforms with LMH's, the CFMWS ranges never intersect the LMH, the
> + * endpoint decoder's HPA range size is always guaranteed aligned to NIW*256MB
> + * and also typically larger than the matching root decoder's, and the root
> + * decoder's range end is at an address that is necessarily less than SZ_4G
> + * (i.e., the Hole is in Low Memory - this function doesn't deal with other
> + * kinds of holes).
> + *
> + * Return: true if an endpoint matches a root decoder, else false.
> + */
> +bool platform_cxlrd_matches_cxled(const struct cxl_root_decoder *cxlrd,
> +				  const struct cxl_endpoint_decoder *cxled)
> +{
> +	const struct range *rd_r, *sd_r;
> +	int align;
> +
> +	rd_r = &cxlrd->cxlsd.cxld.hpa_range;
> +	sd_r = &cxled->cxld.hpa_range;
> +	align = cxled->cxld.interleave_ways * SZ_256M;
> +
> +	if (rd_r->start == LMH_CFMWS_RANGE_START &&
> +	    rd_r->start == sd_r->start && rd_r->end < sd_r->end &&
> +	    rd_r->end < (LMH_CFMWS_RANGE_START + SZ_4G) &&
> +	    IS_ALIGNED(range_len(sd_r), align))
> +		return true;
> +
> +	return false;
Similar to below.  Simply returning the boolean statement can be simpler

	return rd_r->start == LMH_CFMWS_RANGE_START &&
	       rd_r->start == sd_r->start &&
	       rd_r->end < sd_r->end &&
	       rd_r->end < (LMH_CFMWS_RANGE_START + SZ_4G) &&
	       IS_ALIGNED(range_len(sd_r), align);


> +}
> +
> +/**
> + * platform_region_matches_cxld() - Platform quirk to match a CXL Region and a
> + * Switch or Endpoint Decoder. It allows matching on platforms with LMH's.
> + * @p: Region Params against which @cxled is matched.
> + * @cxld: Switch or Endpoint Decoder to be tested for matching @p.
> + *
> + * Similar to platform_cxlrd_matches_cxled(), it matches regions and
> + * decoders on platforms with LMH's.
> + *
> + * Return: true if a Decoder matches a Region, else false.
> + */
> +bool platform_region_matches_cxld(const struct cxl_region_params *p,
> +				  const struct cxl_decoder *cxld)
> +{
> +	const struct range *r = &cxld->hpa_range;
> +	const struct resource *res = p->res;
> +	int align = cxld->interleave_ways * SZ_256M;
> +
> +	if (res->start == LMH_CFMWS_RANGE_START && res->start == r->start &&
> +	    res->end < r->end && res->end < (LMH_CFMWS_RANGE_START + SZ_4G) &&
> +	    IS_ALIGNED(range_len(r), align))
> +		return true;

Dave commented on line break up here, but I'd go further.

	return res->start == LMH_CFMWS_RANGE_START &&
	       res->start == r->start &&
	       res->end < r->end &&
	       res->end < (LMH_CFMWS_RANGE_START + SZ_4G) &&
	       IS_ALIGNED(range_len(r), align);

> +
> +	return false;
> +}

> diff --git a/drivers/cxl/core/platform_quirks.h b/drivers/cxl/core/platform_quirks.h
> new file mode 100644
> index 000000000000..a15592b4e90e
> --- /dev/null
> +++ b/drivers/cxl/core/platform_quirks.h
> @@ -0,0 +1,33 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#include "cxl.h"
> +
> +#ifdef CONFIG_CXL_PLATFORM_QUIRKS
> +bool platform_cxlrd_matches_cxled(const struct cxl_root_decoder *cxlrd,
> +				  const struct cxl_endpoint_decoder *cxled);
> +bool platform_region_matches_cxld(const struct cxl_region_params *p,
> +				  const struct cxl_decoder *cxld);
> +void platform_res_adjust(struct resource *res,
> +			 struct cxl_endpoint_decoder *cxled,
> +			 const struct cxl_root_decoder *cxlrd);
> +#else
> +static inline bool
> +platform_root_decoder_contains(const struct cxl_root_decoder *cxlrd,
> +			       const struct cxl_endpoint_decoder *cxled)
> +{
> +	return false;
> +}
> +
> +static inline bool
> +platform_region_matches_cxld(const struct cxl_region_params *p,
> +			     const struct cxl_decoder *cxld)
> +{
> +	return false;
> +}
> +
> +inline void platform_res_adjust(struct resource *res,
static 
> +				struct cxl_endpoint_decoder *cxled,
> +				const struct cxl_root_decoder *cxlrd)
> +{
> +}
> +#endif /* CONFIG_CXL_PLATFORM_QUIRKS */


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/4 v5] cxl/core: Add helpers to detect Low Memory Holes on x86
  2025-10-09  3:16   ` Alison Schofield
@ 2025-11-05 18:02     ` Fabio M. De Francesco
  0 siblings, 0 replies; 24+ messages in thread
From: Fabio M. De Francesco @ 2025-11-05 18:02 UTC (permalink / raw)
  To: Alison Schofield
  Cc: linux-cxl, Davidlohr Bueso, Jonathan Cameron, Dave Jiang,
	Vishal Verma, Ira Weiny, Dan Williams, Jonathan Corbet, linux-doc,
	linux-kernel, Gregory Price, Robert Richter, Cheatham Benjamin

On Thursday, October 9, 2025 5:16:05 AM Central European Standard Time Alison Schofield wrote:
> On Mon, Oct 06, 2025 at 05:58:05PM +0200, Fabio M. De Francesco wrote:
> > On a x86 platform with a low memory hole (LHM), the BIOS may publish
> > CFMWS that describes a system physical address (SPA) range that
> > typically is only a subset of the corresponding CXL intermediate switch
> > and endpoint decoder's host physical address (HPA) ranges. The CFMWS
> > range never intersects the LHM and so the driver instantiates a root
> > decoder whose HPA range size doesn't fully contain the matching switch
> > and endpoint decoders' HPA ranges.[1]
> > 
> > To construct regions and attach decoders, the driver needs to match root
> > decoders and regions with endpoint decoders. The process fails and
> > returns errors because the driver is not designed to deal with SPA
> > ranges which are smaller than the corresponding hardware decoders HPA
> > ranges.
> > 
> > Introduce two functions that indirectly detect the presence of x86 LMH
> > and allow the matching between a root decoder or an already constructed
> > region with a corresponding intermediate switch or endpoint decoder to
> > enable the construction of a region and the subsequent attachment of the
> > same decoders to that region.
> > 
> > These functions return true when SPA/HPA misalignments due to LMH's are
> > detected under specific conditions:
> > 
> > - Both the SPA and HPA ranges must start at LMH_CFMWS_RANGE_START (i.e.,
> >   0x0 on x86 with LMH's).
> > - The SPA range's size is less than HPA's.
> > - The SPA range's size is less than 4G.
> > - The HPA range's size is aligned to the NIW * 256M rule.
> > 
> > Also introduce a function that adjusts the range end of a region to be
> > constructed and the DPA range's end of the endpoint decoders that will
> > be later attached to that region.
> 
> Hi Fabio,
> 
> Your getting some fresh eyes on some of this with my review.
> The adjustment of resources is what caught my eye, and I looked at
> platform_res_adjust() in this patch and it's usage in the next patch.
> 
Hi Alison,

Thanks for looking at this version with fresh eyes. 

As always, your reviews catch details that others miss and go well beyond 
what's typical - providing working code and demonstrating the results is 
very helpful.
> > 
> > [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/Kconfig                |  4 ++
> >  drivers/cxl/core/Makefile          |  1 +
> >  drivers/cxl/core/platform_quirks.c | 99 ++++++++++++++++++++++++++++++
> >  drivers/cxl/core/platform_quirks.h | 33 ++++++++++
> >  4 files changed, 137 insertions(+)
> >  create mode 100644 drivers/cxl/core/platform_quirks.c
> >  create mode 100644 drivers/cxl/core/platform_quirks.h
> > 
> > diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig
> > index 48b7314afdb8..03c0583bc9a3 100644
> > --- a/drivers/cxl/Kconfig
> > +++ b/drivers/cxl/Kconfig
> > @@ -211,6 +211,10 @@ config CXL_REGION
> >  
> >  	  If unsure say 'y'
> >  
> > +config CXL_PLATFORM_QUIRKS
> > +	def_bool y
> > +	depends on CXL_REGION
> > +
> 
> Why no help text for the new CONFIG option?
>
Good catch. Below I'll show you the help text I'm thinking to add.  
>
> That text will probably answer my next question: why do we have the
> option?
>
For now, PLATFORM_QUIRKS enables only region creation and endpoint
attachment support on x86 with LMH. In the future, it's intended to be
selected by other platforms that need quirks.[1]

I think we should allow users to choose whether to enable it - they can
leave it disabled, or select it if they know it's needed or are unsure
about potential platform issues. The overhead from running quirks should
be minimal.

All that said, how about this?

diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig
index 03c0583bc9a3..5ab8d5c23187 100644
--- a/drivers/cxl/Kconfig
+++ b/drivers/cxl/Kconfig
@@ -212,8 +212,15 @@ config CXL_REGION
          If unsure say 'y'
 
 config CXL_PLATFORM_QUIRKS
-       def_bool y
+       bool "CXL: Region Platform Quirks"
        depends on CXL_REGION
+       help
+         Enable support for the following platform quirks:
+
+               - Region creation / Endpoint Decoders attach in x86 with Low
+                 Memory Holes (Documentation/driver-api/cxl/conventions.rst).
+
+         If unsure say 'y'
 
 config CXL_REGION_INVALIDATION_TEST
        bool "CXL: Region Cache Management Bypass (TEST)"

> 
> I have comments for the callsites of platform_res_adjust() in the next
> patch, but I'll pull some of that back into this patch to keep it all
> in one, more logical, place.
> 
> There are 2 callsites, and one passes in NULL for 'res' because
> at that site we know that the regions struct res has been adjusted.
> I felt that was subtle, and that it may be better to just pass in
> the 'res' all the time and let the function adjust if needed,
> ignore if not needed.
>
I'll implement your suggestion in v6.
>  
> The name platform_res_adjust() suggested that the 'res' as in the
> region 'res' was getting adjusted. This is adjusting multiple resources
> - the region resource and the endpoint decoder dpa resource. If it's
> meant to be kind of opaque, that's ok, but by using _res_ it sure sounds
> like it's adjusting the the region resource (when viewed from the call site).
> 
> I might have done this in 2 helpers for crispness:
> res = platform_adjust_region_resource()
> cxled = platform_adjust_endpoint_decoder()
> 
> Then you could adjust the region resource once when the region
> is constructed, and the endpoint regions every time in 
> cxl_add_to_region().
> 
> If you are settled with one adjust routine, perhaps just a 
> rename to platform_adjust_resources() will make it sound as
> broad as it is.
>
I prefer not to introduce more than one platform_adjust_*(), so I'll rename 
platform_res_adjust() to platform_adjust_resources().
> 
> > +void platform_res_adjust(struct resource *res,
> > +			 struct cxl_endpoint_decoder *cxled,
> > +			 const struct cxl_root_decoder *cxlrd)
> > +{
> > +	if (!platform_cxlrd_matches_cxled(cxlrd, cxled))
> > +		return;
> > +
> > +	guard(rwsem_write)(&cxl_rwsem.dpa);
> > +	dev_dbg(cxled_to_memdev(cxled)->dev.parent,
> > +		"Low Memory Hole detected. Resources were (%s: %pr, %pr)\n",
> > +		dev_name(&cxled->cxld.dev), res, cxled->dpa_res);
> > +	if (res) {
> > +		/* Trim region resource overlap with LMH */
> > +		res->end = cxlrd->res->end;
> > +	}
> 
> Prefer dev_info so always appears.
> Prefer to see the region name.
>
Okay to both suggestions.
>
> I'm guessing the dev_dbg() above and the dev_info() below are written
> with the idea that we want the before view only in dev_dbg() and the
> after view only in dev_info().
> 
> Looks like this now:
> [] cxl_core:platform_res_adjust:90: cxl_mock_mem cxl_mem.0: Low Memory Hole detected. Resources were (decoder12.0: [mem 0x3ff010000000-0x3ff04fffffff flags 0x200], [mem 0x00000000-0x1fffffff flags 0x80000200])
> [] cxl_mock_mem cxl_mem.0: Resources have been adjusted for LMH (decoder12.0: [mem 0x3ff010000000-0x3ff03fffffff flags 0x200], [mem 0x00000000-0x17ffffff flags 0x80000200])
> [] cxl_core:platform_res_adjust:90: cxl_mock_mem cxl_mem.4: Low Memory Hole detected. Resources were (decoder13.0: (null), [mem 0x00000000-0x1fffffff flags 0x80000200])
> [] cxl_mock_mem cxl_mem.4: Resources have been adjusted for LMH (decoder13.0: (null), [mem 0x00000000-0x17ffffff flags 0x80000200])
> 
> I'll suggest this to emit explicitly what is changing:
> [] cxl region0: LMH Low memory hole trims region resource [mem 0x3ff010000000-0x3ff04fffffff flags 0x200] to [mem 0x3ff010000000-0x3ff03fffffff flags 0x200])
> [] cxl decoder13.0: LMH Low memory hole trims DPA resource [mem 0x00000000-0x1fffffff flags 0x80000200] to [mem 0x00000000-0x17ffffff flags 0x80000200])
> [] cxl decoder17.0: LMH Low memory hole trims DPA resource [mem 0x00000000-0x1fffffff flags 0x80000200] to [mem 0x00000000-0x17ffffff flags 0x80000200])
>
I'll make platform_adjust_resources() output the messages you just showed. 
> 
> > +	/* Match endpoint decoder's DPA resource to root decoder's */
> A 'Match' would be if the the endpoint and root decoder resource were
> same. This is more of adjustment or recalculation of the DPA length.
> 
> > +	cxled->dpa_res->end =
> > +		cxled->dpa_res->start +
> > +		resource_size(cxlrd->res) / cxled->cxld.interleave_ways - 1;
> 
> I'm cautious about the use of division and suggest this as the more
> bullet-proof kernel style:
> 
> 	slice = div_u64(resource_size(cxlrd->res), cxled->cxld.interleave_ways);
> 	cxled->dpa_res->end = cxled->dpa_res->start + slice - 1;
> 
div_u64() is the safer choice.  
> 
> > +	dev_info(cxled_to_memdev(cxled)->dev.parent,
> > +		 "Resources have been adjusted for LMH (%s: %pr, %pr)\n",
> > +		 dev_name(&cxled->cxld.dev), res, cxled->dpa_res);
> > +}
> 
> Here's the diff showing how I emmited the that messaging above. I really
> wanted to have that region name to emit. This was done keeping the
> adjust in one function, but maybe you'll choose to split :)
>
Thank you,

Fabio

[1] https://lore.kernel.org/linux-cxl/67ee07cd4f8ec_1c2c6294d5@dwillia2-xfh.jf.intel.com.notmuch/
> 
> ---
>  drivers/cxl/core/platform_quirks.c | 32 ++++++++++++++++++------------
>  drivers/cxl/core/platform_quirks.h |  6 ++++--
>  drivers/cxl/core/region.c          | 15 ++++++++------
>  3 files changed, 32 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/cxl/core/platform_quirks.c b/drivers/cxl/core/platform_quirks.c
> index aecd376f2766..aa25770c088a 100644
> --- a/drivers/cxl/core/platform_quirks.c
> +++ b/drivers/cxl/core/platform_quirks.c
> @@ -81,24 +81,30 @@ EXPORT_SYMBOL_NS_GPL(__platform_region_matches_cxld, "CXL");
>  
>  void platform_res_adjust(struct resource *res,
>  			 struct cxl_endpoint_decoder *cxled,
> -			 const struct cxl_root_decoder *cxlrd)
> +			 const struct cxl_root_decoder *cxlrd,
> +			 const struct device *region_dev)
>  {
> +	struct resource dpa_res_orig = *cxled->dpa_res;
> +	u64 slice;
> +
>  	if (!platform_cxlrd_matches_cxled(cxlrd, cxled))
>  		return;
>  
>  	guard(rwsem_write)(&cxl_rwsem.dpa);
> -	dev_dbg(cxled_to_memdev(cxled)->dev.parent,
> -		"Low Memory Hole detected. Resources were (%s: %pr, %pr)\n",
> -		dev_name(&cxled->cxld.dev), res, cxled->dpa_res);
> -	if (res) {
> -		/* Trim region resource overlap with LMH */
> +
> +	/* Region resource will need a trim at first endpoint attach only */
> +	if ((res) && (res->end != cxlrd->res->end)) {
> +		dev_info(region_dev,
> +			 "LMH Low memory hole trims region resource %pr to %pr)\n",
> +			 res, cxlrd->res);
>  		res->end = cxlrd->res->end;
>  	}
> -	/* Match endpoint decoder's DPA resource to root decoder's */
> -	cxled->dpa_res->end =
> -		cxled->dpa_res->start +
> -		resource_size(cxlrd->res) / cxled->cxld.interleave_ways - 1;
> -	dev_info(cxled_to_memdev(cxled)->dev.parent,
> -		 "Resources have been adjusted for LMH (%s: %pr, %pr)\n",
> -		 dev_name(&cxled->cxld.dev), res, cxled->dpa_res);
> +
> +	/* Adjust the endpoint decoder DPA resource end */
> +	slice = div_u64(resource_size(cxlrd->res), cxled->cxld.interleave_ways);
> +	cxled->dpa_res->end = cxled->dpa_res->start + slice - 1;
> +
> +	dev_info(&cxled->cxld.dev,
> +		 "LMH Low memory hole trims DPA resource %pr to %pr)\n",
> +		 &dpa_res_orig, cxled->dpa_res);
>  }
> diff --git a/drivers/cxl/core/platform_quirks.h b/drivers/cxl/core/platform_quirks.h
> index bdea00365dad..55647711cdb4 100644
> --- a/drivers/cxl/core/platform_quirks.h
> +++ b/drivers/cxl/core/platform_quirks.h
> @@ -17,7 +17,8 @@ bool __platform_region_matches_cxld(const struct cxl_region_params *p,
>  				    const struct cxl_decoder *cxld);
>  void platform_res_adjust(struct resource *res,
>  			 struct cxl_endpoint_decoder *cxled,
> -			 const struct cxl_root_decoder *cxlrd);
> +			 const struct cxl_root_decoder *cxlrd,
> +			 const struct device *region_dev);
>  #else
>  static inline bool
>  platform_cxlrd_matches_cxled(const struct cxl_root_decoder *cxlrd,
> @@ -35,7 +36,8 @@ platform_region_matches_cxld(const struct cxl_region_params *p,
>  
>  inline void platform_res_adjust(struct resource *res,
>  				struct cxl_endpoint_decoder *cxled,
> -				const struct cxl_root_decoder *cxlrd)
> +				const struct cxl_root_decoder *cxlrd,
> +				const struct device *region_dev);
>  {
>  }
>  #endif /* CONFIG_CXL_PLATFORM_QUIRKS */
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 9a499bfca23d..d4298a61b912 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -3502,7 +3502,7 @@ static int __construct_region(struct cxl_region *cxlr,
>  	 * Trim the HPA retrieved from hardware to fit the SPA mapped by the
>  	 * platform
>  	 */
> -	platform_res_adjust(res, cxled, cxlrd);
> +	platform_res_adjust(res, cxled, cxlrd, &cxlr->dev);
>  
>  	rc = cxl_extended_linear_cache_resize(cxlr, res);
>  	if (rc && rc != -EOPNOTSUPP) {
> @@ -3611,14 +3611,17 @@ int cxl_add_to_region(struct cxl_endpoint_decoder *cxled)
>  	mutex_lock(&cxlrd->range_lock);
>  	struct cxl_region *cxlr __free(put_cxl_region) =
>  		cxl_find_region_by_range(cxlrd, cxled);
> -	if (!cxlr)
> +	if (!cxlr) {
>  		cxlr = construct_region(cxlrd, cxled);
> -	else
> +	} else {
>  		/*
> -		 * Adjust the Endpoint Decoder's dpa_res to fit the Region which
> -		 * it has to be attached to
> +		 * Platform adjustments are done in construct_region()
> +		 * for first target, and here for additional targets.
>  		 */
> -		platform_res_adjust(NULL, cxled, cxlrd);
> +		p = &cxlr->params;
> +		platform_res_adjust(p->res, cxled, cxlrd, &cxlr->dev);
> +	}
> +
>  	mutex_unlock(&cxlrd->range_lock);
>  
>  	rc = PTR_ERR_OR_ZERO(cxlr);
> -- 
> 2.37.3
> 
> > 
> 





^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/4 v5] cxl/core: Add helpers to detect Low Memory Holes on x86
  2025-10-10  7:38   ` kernel test robot
@ 2025-11-05 18:11     ` Fabio M. De Francesco
  0 siblings, 0 replies; 24+ messages in thread
From: Fabio M. De Francesco @ 2025-11-05 18:11 UTC (permalink / raw)
  To: linux-cxl, kernel test robot
  Cc: oe-kbuild-all, Davidlohr Bueso, Jonathan Cameron, Dave Jiang,
	Alison Schofield, Vishal Verma, Ira Weiny, Dan Williams,
	Jonathan Corbet, linux-doc, linux-kernel, Gregory Price,
	Robert Richter, Cheatham Benjamin

On Friday, October 10, 2025 9:38:02 AM Central European Standard Time kernel test robot wrote:
> Hi Fabio,
> 
> kernel test robot noticed the following build errors:
> 
> [auto build test ERROR on 46037455cbb748c5e85071c95f2244e81986eb58]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Fabio-M-De-Francesco/cxl-core-Change-match_-_by_range-signatures/20251010-111627
> base:   46037455cbb748c5e85071c95f2244e81986eb58
> patch link:    https://lore.kernel.org/r/20251006155836.791418-3-fabio.m.de.francesco%40linux.intel.com
> patch subject: [PATCH 2/4 v5] cxl/core: Add helpers to detect Low Memory Holes on x86
> config: i386-buildonly-randconfig-005-20251010 (https://download.01.org/0day-ci/archive/20251010/202510101555.zofjGvZF-lkp@intel.com/config)
> compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251010/202510101555.zofjGvZF-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202510101555.zofjGvZF-lkp@intel.com/
> 
> All errors (new ones prefixed by >>):
> 
>    ld: drivers/cxl/core/platform_quirks.o: in function `platform_res_adjust':
> >> drivers/cxl/core/platform_quirks.c:95:(.text+0x1f1): undefined reference to `__udivdi3'
> 
> 
> vim +95 drivers/cxl/core/platform_quirks.c
> 
>     76	
>     77	void platform_res_adjust(struct resource *res,
>     78				 struct cxl_endpoint_decoder *cxled,
>     79				 const struct cxl_root_decoder *cxlrd)
>     80	{
>     81		if (!platform_cxlrd_matches_cxled(cxlrd, cxled))
>     82			return;
>     83	
>     84		guard(rwsem_write)(&cxl_rwsem.dpa);
>     85		dev_dbg(cxled_to_memdev(cxled)->dev.parent,
>     86			"Low Memory Hole detected. Resources were (%s: %pr, %pr)\n",
>     87			dev_name(&cxled->cxld.dev), res, cxled->dpa_res);
>     88		if (res) {
>     89			/* Trim region resource overlap with LMH */
>     90			res->end = cxlrd->res->end;
>     91		}
>     92		/* Match endpoint decoder's DPA resource to root decoder's */
>     93		cxled->dpa_res->end =
>     94			cxled->dpa_res->start +
>   > 95			resource_size(cxlrd->res) / cxled->cxld.interleave_ways - 1;
> 
I think that the use of udiv_64() (per Alison suggestion) will fix this 
error.

Fabio
>
> -- 
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
> 
> 





^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/4 v5] cxl/core: Add helpers to detect Low Memory Holes on x86
  2025-10-10 14:49   ` kernel test robot
@ 2025-11-05 18:20     ` Fabio M. De Francesco
  2025-11-05 18:51       ` Dave Jiang
  0 siblings, 1 reply; 24+ messages in thread
From: Fabio M. De Francesco @ 2025-11-05 18:20 UTC (permalink / raw)
  To: linux-cxl, kernel test robot
  Cc: llvm, oe-kbuild-all, Davidlohr Bueso, Jonathan Cameron,
	Dave Jiang, Alison Schofield, Vishal Verma, Ira Weiny,
	Dan Williams, Jonathan Corbet, linux-doc, linux-kernel,
	Gregory Price, Robert Richter, Cheatham Benjamin

On Friday, October 10, 2025 4:49:01 PM Central European Standard Time kernel test robot wrote:
> Hi Fabio,
> 
> kernel test robot noticed the following build warnings:
> 
> [auto build test WARNING on 46037455cbb748c5e85071c95f2244e81986eb58]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Fabio-M-De-Francesco/cxl-core-Change-match_-_by_range-signatures/20251010-111627
> base:   46037455cbb748c5e85071c95f2244e81986eb58
> patch link:    https://lore.kernel.org/r/20251006155836.791418-3-fabio.m.de.francesco%40linux.intel.com
> patch subject: [PATCH 2/4 v5] cxl/core: Add helpers to detect Low Memory Holes on x86
> config: i386-randconfig-011-20251010 (https://download.01.org/0day-ci/archive/20251010/202510102229.iFcGqbMH-lkp@intel.com/config)
> compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251010/202510102229.iFcGqbMH-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202510102229.iFcGqbMH-lkp@intel.com/
> 
> All warnings (new ones prefixed by >>):
> 
> >> drivers/cxl/core/platform_quirks.c:70:36: warning: result of comparison of constant 4294967296 with expression of type 'const resource_size_t' (aka 'const unsigned int') is always true [-Wtautological-constant-out-of-range-compare]
>       70 |             res->end < r->end && res->end < (LMH_CFMWS_RANGE_START + SZ_4G) &&
>          |                                  ~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    1 warning generated.
> 
Unavoidable warning wherever res->end is an u32 variable:

/* include/linux/types.h */

#ifdef CONFIG_PHYS_ADDR_T_64BIT
typedef u64 phys_addr_t;
#else
typedef u32 phys_addr_t;
#endif

typedef phys_addr_t resource_size_t;

/* include/linux/ioport.h */

struct resource {
	resource_size_t start;
	resource_size_t end;
};

I think we should ignore this report.

Fabio
> 
> vim +70 drivers/cxl/core/platform_quirks.c
> 
>     50	
>     51	/**
>     52	 * platform_region_matches_cxld() - Platform quirk to match a CXL Region and a
>     53	 * Switch or Endpoint Decoder. It allows matching on platforms with LMH's.
>     54	 * @p: Region Params against which @cxled is matched.
>     55	 * @cxld: Switch or Endpoint Decoder to be tested for matching @p.
>     56	 *
>     57	 * Similar to platform_cxlrd_matches_cxled(), it matches regions and
>     58	 * decoders on platforms with LMH's.
>     59	 *
>     60	 * Return: true if a Decoder matches a Region, else false.
>     61	 */
>     62	bool platform_region_matches_cxld(const struct cxl_region_params *p,
>     63					  const struct cxl_decoder *cxld)
>     64	{
>     65		const struct range *r = &cxld->hpa_range;
>     66		const struct resource *res = p->res;
>     67		int align = cxld->interleave_ways * SZ_256M;
>     68	
>     69		if (res->start == LMH_CFMWS_RANGE_START && res->start == r->start &&
>   > 70		    res->end < r->end && res->end < (LMH_CFMWS_RANGE_START + SZ_4G) &&
>     71		    IS_ALIGNED(range_len(r), align))
>     72			return true;
>     73	
>     74		return false;
>     75	}
>     76	
> 
> -- 
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
> 
> 





^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/4 v5] cxl/core: Add helpers to detect Low Memory Holes on x86
  2025-11-05 18:20     ` Fabio M. De Francesco
@ 2025-11-05 18:51       ` Dave Jiang
  0 siblings, 0 replies; 24+ messages in thread
From: Dave Jiang @ 2025-11-05 18:51 UTC (permalink / raw)
  To: Fabio M. De Francesco, linux-cxl, kernel test robot
  Cc: llvm, oe-kbuild-all, Davidlohr Bueso, Jonathan Cameron,
	Alison Schofield, Vishal Verma, Ira Weiny, Dan Williams,
	Jonathan Corbet, linux-doc, linux-kernel, Gregory Price,
	Robert Richter, Cheatham Benjamin



On 11/5/25 11:20 AM, Fabio M. De Francesco wrote:
> On Friday, October 10, 2025 4:49:01 PM Central European Standard Time kernel test robot wrote:
>> Hi Fabio,
>>
>> kernel test robot noticed the following build warnings:
>>
>> [auto build test WARNING on 46037455cbb748c5e85071c95f2244e81986eb58]
>>
>> url:    https://github.com/intel-lab-lkp/linux/commits/Fabio-M-De-Francesco/cxl-core-Change-match_-_by_range-signatures/20251010-111627
>> base:   46037455cbb748c5e85071c95f2244e81986eb58
>> patch link:    https://lore.kernel.org/r/20251006155836.791418-3-fabio.m.de.francesco%40linux.intel.com
>> patch subject: [PATCH 2/4 v5] cxl/core: Add helpers to detect Low Memory Holes on x86
>> config: i386-randconfig-011-20251010 (https://download.01.org/0day-ci/archive/20251010/202510102229.iFcGqbMH-lkp@intel.com/config)
>> compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
>> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251010/202510102229.iFcGqbMH-lkp@intel.com/reproduce)
>>
>> If you fix the issue in a separate patch/commit (i.e. not just a new version of
>> the same patch/commit), kindly add following tags
>> | Reported-by: kernel test robot <lkp@intel.com>
>> | Closes: https://lore.kernel.org/oe-kbuild-all/202510102229.iFcGqbMH-lkp@intel.com/
>>
>> All warnings (new ones prefixed by >>):
>>
>>>> drivers/cxl/core/platform_quirks.c:70:36: warning: result of comparison of constant 4294967296 with expression of type 'const resource_size_t' (aka 'const unsigned int') is always true [-Wtautological-constant-out-of-range-compare]
>>       70 |             res->end < r->end && res->end < (LMH_CFMWS_RANGE_START + SZ_4G) &&
>>          |                                  ~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>    1 warning generated.
>>
> Unavoidable warning wherever res->end is an u32 variable:
> 
> /* include/linux/types.h */
> 
> #ifdef CONFIG_PHYS_ADDR_T_64BIT
> typedef u64 phys_addr_t;
> #else
> typedef u32 phys_addr_t;
> #endif
> 
> typedef phys_addr_t resource_size_t;
> 
> /* include/linux/ioport.h */
> 
> struct resource {
> 	resource_size_t start;
> 	resource_size_t end;
> };
> 
> I think we should ignore this report.

Maybe try casting res->end to (u64) and see if it shuts the warning up?

DJ

> 
> Fabio
>>
>> vim +70 drivers/cxl/core/platform_quirks.c
>>
>>     50	
>>     51	/**
>>     52	 * platform_region_matches_cxld() - Platform quirk to match a CXL Region and a
>>     53	 * Switch or Endpoint Decoder. It allows matching on platforms with LMH's.
>>     54	 * @p: Region Params against which @cxled is matched.
>>     55	 * @cxld: Switch or Endpoint Decoder to be tested for matching @p.
>>     56	 *
>>     57	 * Similar to platform_cxlrd_matches_cxled(), it matches regions and
>>     58	 * decoders on platforms with LMH's.
>>     59	 *
>>     60	 * Return: true if a Decoder matches a Region, else false.
>>     61	 */
>>     62	bool platform_region_matches_cxld(const struct cxl_region_params *p,
>>     63					  const struct cxl_decoder *cxld)
>>     64	{
>>     65		const struct range *r = &cxld->hpa_range;
>>     66		const struct resource *res = p->res;
>>     67		int align = cxld->interleave_ways * SZ_256M;
>>     68	
>>     69		if (res->start == LMH_CFMWS_RANGE_START && res->start == r->start &&
>>   > 70		    res->end < r->end && res->end < (LMH_CFMWS_RANGE_START + SZ_4G) &&
>>     71		    IS_ALIGNED(range_len(r), align))
>>     72			return true;
>>     73	
>>     74		return false;
>>     75	}
>>     76	
>>
>> -- 
>> 0-DAY CI Kernel Test Service
>> https://github.com/intel/lkp-tests/wiki
>>
>>
> 
> 
> 
> 


^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2025-11-05 18:51 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-06 15:58 [PATCH 0/4 v5] cxl/core: Enable Region creation/attach on x86 with LMH Fabio M. De Francesco
2025-10-06 15:58 ` [PATCH 1/4 v5] cxl/core: Change match_*_by_range() signatures Fabio M. De Francesco
2025-10-06 17:35   ` Gregory Price
2025-10-06 23:30   ` Dave Jiang
2025-10-06 15:58 ` [PATCH 2/4 v5] cxl/core: Add helpers to detect Low Memory Holes on x86 Fabio M. De Francesco
2025-10-06 17:40   ` Gregory Price
2025-10-07  0:00   ` Dave Jiang
2025-10-09  3:16   ` Alison Schofield
2025-11-05 18:02     ` Fabio M. De Francesco
2025-10-10  7:38   ` kernel test robot
2025-11-05 18:11     ` Fabio M. De Francesco
2025-10-10 14:49   ` kernel test robot
2025-11-05 18:20     ` Fabio M. De Francesco
2025-11-05 18:51       ` Dave Jiang
2025-10-28 15:58   ` Jonathan Cameron
2025-10-06 15:58 ` [PATCH 3/4 v5] cxl/core: Enable Region creation on x86 with LMH Fabio M. De Francesco
2025-10-06 17:46   ` Gregory Price
2025-10-07 20:25     ` Dave Jiang
2025-10-09 14:30       ` Gregory Price
2025-10-09  3:29   ` Alison Schofield
2025-10-06 15:58 ` [PATCH 4/4 v5] cxl/test: Simulate an x86 Low Memory Hole for tests Fabio M. De Francesco
2025-10-07 20:37   ` Dave Jiang
2025-10-09  3:34     ` Alison Schofield
2025-10-09  3:52   ` Alison Schofield

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).