[PATCH 4/4 v3] cxl/test: Simulate an x86 Low Memory Hole for tests

Fabio M. De Francesco posted 4 patches 9 months, 1 week ago
[PATCH 4/4 v3] cxl/test: Simulate an x86 Low Memory Hole for tests
Posted by Fabio M. De Francesco 9 months, 1 week ago
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.

Since the auto-created region of cxl-test uses mock_cfmws[0], whose 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 if
it was run on the mock environment created by cxl-tests.

Therefore, save the mock_cfmsw[0] range base_hpa and reuse it to match CXL
Root Decoders and Regions with Endpoint Decoders when the driver is run on
mock devices.

Since the auto-created region of cxl-test uses mock_cfmws[0], the
LMH path in the CXL Driver will be exercised every time the cxl-test
module is loaded. Executing unit test: cxl-topology.sh, confirms the
region created successfully with a LMH.

Cc: Alison Schofield <alison.schofield@intel.com>
Cc: Dan Williams <dan.j.williams@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/lmh.c               | 35 ++++++++++++++++++++++++----
 drivers/cxl/core/lmh.h               |  2 ++
 tools/testing/cxl/cxl_core_exports.c |  2 ++
 tools/testing/cxl/test/cxl.c         | 10 ++++++++
 4 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/drivers/cxl/core/lmh.c b/drivers/cxl/core/lmh.c
index 2e32f867eb94..9c55670c1c84 100644
--- a/drivers/cxl/core/lmh.c
+++ b/drivers/cxl/core/lmh.c
@@ -1,11 +1,28 @@
 // SPDX-License-Identifier: GPL-2.0-only
 
 #include <linux/range.h>
+#include <linux/pci.h>
+
 #include "lmh.h"
 
 /* Start of CFMWS range that end before x86 Low Memory Holes */
 #define LMH_CFMWS_RANGE_START 0x0ULL
 
+static u64 mock_cfmws0_range_start = ULLONG_MAX;
+
+void set_mock_cfmws0_range_start(const u64 start)
+{
+	mock_cfmws0_range_start = start;
+}
+
+static u64 get_cfmws_range_start(const struct device *dev)
+{
+	if (dev_is_pci(dev))
+		return LMH_CFMWS_RANGE_START;
+
+	return mock_cfmws0_range_start;
+}
+
 /*
  * Match CXL Root and Endpoint Decoders by comparing SPA and HPA ranges.
  *
@@ -19,14 +36,19 @@ bool arch_match_spa(const struct cxl_root_decoder *cxlrd,
 		    const struct cxl_endpoint_decoder *cxled)
 {
 	const struct range *r1, *r2;
+	u64 cfmws_range_start;
 	int niw;
 
+	cfmws_range_start = get_cfmws_range_start(&cxled->cxld.dev);
+	if (cfmws_range_start == ULLONG_MAX)
+		return false;
+
 	r1 = &cxlrd->cxlsd.cxld.hpa_range;
 	r2 = &cxled->cxld.hpa_range;
 	niw = cxled->cxld.interleave_ways;
 
-	if (r1->start == LMH_CFMWS_RANGE_START && r1->start == r2->start &&
-	    r1->end < (LMH_CFMWS_RANGE_START + SZ_4G) && r1->end < r2->end &&
+	if (r1->start == cfmws_range_start && r1->start == r2->start &&
+	    r1->end < (cfmws_range_start + SZ_4G) && r1->end < r2->end &&
 	    IS_ALIGNED(range_len(r2), niw * SZ_256M))
 		return true;
 
@@ -40,9 +62,14 @@ bool arch_match_region(const struct cxl_region_params *p,
 	const struct range *r = &cxld->hpa_range;
 	const struct resource *res = p->res;
 	int niw = cxld->interleave_ways;
+	u64 cfmws_range_start;
+
+	cfmws_range_start = get_cfmws_range_start(&cxld->dev);
+	if (cfmws_range_start == ULLONG_MAX)
+		return false;
 
-	if (res->start == LMH_CFMWS_RANGE_START && res->start == r->start &&
-	    res->end < (LMH_CFMWS_RANGE_START + SZ_4G) && res->end < r->end &&
+	if (res->start == cfmws_range_start && res->start == r->start &&
+	    res->end < (cfmws_range_start + SZ_4G) && res->end < r->end &&
 	    IS_ALIGNED(range_len(r), niw * SZ_256M))
 		return true;
 
diff --git a/drivers/cxl/core/lmh.h b/drivers/cxl/core/lmh.h
index 16746ceac1ed..b6337120ee17 100644
--- a/drivers/cxl/core/lmh.h
+++ b/drivers/cxl/core/lmh.h
@@ -2,6 +2,8 @@
 
 #include "cxl.h"
 
+void set_mock_cfmws0_range_start(u64 start);
+
 #ifdef CONFIG_CXL_ARCH_LOW_MEMORY_HOLE
 bool arch_match_spa(const struct cxl_root_decoder *cxlrd,
 		    const struct cxl_endpoint_decoder *cxled);
diff --git a/tools/testing/cxl/cxl_core_exports.c b/tools/testing/cxl/cxl_core_exports.c
index f088792a8925..7b20f9fcf0d7 100644
--- a/tools/testing/cxl/cxl_core_exports.c
+++ b/tools/testing/cxl/cxl_core_exports.c
@@ -2,6 +2,8 @@
 /* Copyright(c) 2022 Intel Corporation. All rights reserved. */
 
 #include "cxl.h"
+#include "lmh.h"
 
 /* Exporting of cxl_core symbols that are only used by cxl_test */
 EXPORT_SYMBOL_NS_GPL(cxl_num_decoders_committed, "CXL");
+EXPORT_SYMBOL_NS_GPL(set_mock_cfmws0_range_start, "CXL");
diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
index 1c3336095923..8c69ce0a272f 100644
--- a/tools/testing/cxl/test/cxl.c
+++ b/tools/testing/cxl/test/cxl.c
@@ -9,6 +9,7 @@
 #include <linux/pci.h>
 #include <linux/mm.h>
 #include <cxlmem.h>
+#include <core/lmh.h>
 
 #include "../watermark.h"
 #include "mock.h"
@@ -212,7 +213,11 @@ static struct {
 			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
 					ACPI_CEDT_CFMWS_RESTRICT_VOLATILE,
 			.qtg_id = FAKE_QTG_ID,
+#if defined(CONFIG_CXL_ARCH_LOW_MEMORY_HOLE)
+			.window_size = SZ_256M * 3UL,
+#else
 			.window_size = SZ_256M * 4UL,
+#endif
 		},
 		.target = { 0 },
 	},
@@ -454,6 +459,7 @@ static int populate_cedt(void)
 			return -ENOMEM;
 		window->base_hpa = res->range.start;
 	}
+	set_mock_cfmws0_range_start(mock_cfmws[0]->base_hpa);
 
 	return 0;
 }
@@ -744,7 +750,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_ARCH_LOW_MEMORY_HOLE)
+	const int size = SZ_1G;
+#else
 	const int size = SZ_512M;
+#endif
 	struct cxl_memdev *cxlmd;
 	struct cxl_dport *dport;
 	struct device *dev;
-- 
2.48.1
Re: [PATCH 4/4 v3] cxl/test: Simulate an x86 Low Memory Hole for tests
Posted by Dan Williams 8 months, 3 weeks ago
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.
> 
> Since the auto-created region of cxl-test uses mock_cfmws[0], whose 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 if
> it was run on the mock environment created by cxl-tests.
> 
> Therefore, save the mock_cfmsw[0] range base_hpa and reuse it to match CXL
> Root Decoders and Regions with Endpoint Decoders when the driver is run on
> mock devices.
> 
> Since the auto-created region of cxl-test uses mock_cfmws[0], the
> LMH path in the CXL Driver will be exercised every time the cxl-test
> module is loaded. Executing unit test: cxl-topology.sh, confirms the
> region created successfully with a LMH.
> 
> Cc: Alison Schofield <alison.schofield@intel.com>
> Cc: Dan Williams <dan.j.williams@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/lmh.c               | 35 ++++++++++++++++++++++++----
>  drivers/cxl/core/lmh.h               |  2 ++
>  tools/testing/cxl/cxl_core_exports.c |  2 ++
>  tools/testing/cxl/test/cxl.c         | 10 ++++++++
>  4 files changed, 45 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/cxl/core/lmh.c b/drivers/cxl/core/lmh.c
> index 2e32f867eb94..9c55670c1c84 100644
> --- a/drivers/cxl/core/lmh.c
> +++ b/drivers/cxl/core/lmh.c
> @@ -1,11 +1,28 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  
>  #include <linux/range.h>
> +#include <linux/pci.h>
> +
>  #include "lmh.h"
>  
>  /* Start of CFMWS range that end before x86 Low Memory Holes */
>  #define LMH_CFMWS_RANGE_START 0x0ULL
>  
> +static u64 mock_cfmws0_range_start = ULLONG_MAX;
> +
> +void set_mock_cfmws0_range_start(const u64 start)
> +{
> +	mock_cfmws0_range_start = start;
> +}
> +
> +static u64 get_cfmws_range_start(const struct device *dev)
> +{
> +	if (dev_is_pci(dev))
> +		return LMH_CFMWS_RANGE_START;
> +
> +	return mock_cfmws0_range_start;
> +}
> +

cxl_test should never result in "mock" infrastructure appearing outside
of tools/testing/cxl/

>  /*
>   * Match CXL Root and Endpoint Decoders by comparing SPA and HPA ranges.
>   *
> @@ -19,14 +36,19 @@ bool arch_match_spa(const struct cxl_root_decoder *cxlrd,
>  		    const struct cxl_endpoint_decoder *cxled)
>  {
>  	const struct range *r1, *r2;
> +	u64 cfmws_range_start;
>  	int niw;
>  
> +	cfmws_range_start = get_cfmws_range_start(&cxled->cxld.dev);
> +	if (cfmws_range_start == ULLONG_MAX)
> +		return false;
> +
>  	r1 = &cxlrd->cxlsd.cxld.hpa_range;
>  	r2 = &cxled->cxld.hpa_range;
>  	niw = cxled->cxld.interleave_ways;
>  
> -	if (r1->start == LMH_CFMWS_RANGE_START && r1->start == r2->start &&
> -	    r1->end < (LMH_CFMWS_RANGE_START + SZ_4G) && r1->end < r2->end &&
> +	if (r1->start == cfmws_range_start && r1->start == r2->start &&
> +	    r1->end < (cfmws_range_start + SZ_4G) && r1->end < r2->end &&
>  	    IS_ALIGNED(range_len(r2), niw * SZ_256M))
>  		return true;
>  
> @@ -40,9 +62,14 @@ bool arch_match_region(const struct cxl_region_params *p,
>  	const struct range *r = &cxld->hpa_range;
>  	const struct resource *res = p->res;
>  	int niw = cxld->interleave_ways;
> +	u64 cfmws_range_start;
> +
> +	cfmws_range_start = get_cfmws_range_start(&cxld->dev);
> +	if (cfmws_range_start == ULLONG_MAX)
> +		return false;
>  
> -	if (res->start == LMH_CFMWS_RANGE_START && res->start == r->start &&
> -	    res->end < (LMH_CFMWS_RANGE_START + SZ_4G) && res->end < r->end &&
> +	if (res->start == cfmws_range_start && res->start == r->start &&
> +	    res->end < (cfmws_range_start + SZ_4G) && res->end < r->end &&
>  	    IS_ALIGNED(range_len(r), niw * SZ_256M))
>  		return true;

Someone should be able to read the straight line CXL driver code and
never know that an alternate implementation exists for changing these
details.

So, the mock interface for this stuff should intercept at the
arch_match_spa() and arch_match_region() level.

To me that looks like mark these implementations with the __mock
attribute, similar to to_cxl_host_bridge(). Then define strong versions
in tools/testing/cxl/mock_lmh.c.

The strong versions would apply memory hole semantics to both windows
starting at zero and whatever cxl_test window you choose.
Re: [PATCH 4/4 v3] cxl/test: Simulate an x86 Low Memory Hole for tests
Posted by Dan Williams 8 months, 2 weeks ago
Dan Williams wrote:
> 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.
> > 
> > Since the auto-created region of cxl-test uses mock_cfmws[0], whose 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 if
> > it was run on the mock environment created by cxl-tests.
> > 
> > Therefore, save the mock_cfmsw[0] range base_hpa and reuse it to match CXL
> > Root Decoders and Regions with Endpoint Decoders when the driver is run on
> > mock devices.
> > 
> > Since the auto-created region of cxl-test uses mock_cfmws[0], the
> > LMH path in the CXL Driver will be exercised every time the cxl-test
> > module is loaded. Executing unit test: cxl-topology.sh, confirms the
> > region created successfully with a LMH.
> > 
> > Cc: Alison Schofield <alison.schofield@intel.com>
> > Cc: Dan Williams <dan.j.williams@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/lmh.c               | 35 ++++++++++++++++++++++++----
> >  drivers/cxl/core/lmh.h               |  2 ++
> >  tools/testing/cxl/cxl_core_exports.c |  2 ++
> >  tools/testing/cxl/test/cxl.c         | 10 ++++++++
> >  4 files changed, 45 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/cxl/core/lmh.c b/drivers/cxl/core/lmh.c
> > index 2e32f867eb94..9c55670c1c84 100644
> > --- a/drivers/cxl/core/lmh.c
> > +++ b/drivers/cxl/core/lmh.c
[..]
> 
> Someone should be able to read the straight line CXL driver code and
> never know that an alternate implementation exists for changing these
> details.
> 
> So, the mock interface for this stuff should intercept at the
> arch_match_spa() and arch_match_region() level.
> 
> To me that looks like mark these implementations with the __mock
> attribute, similar to to_cxl_host_bridge(). Then define strong versions
> in tools/testing/cxl/mock_lmh.c.
> 
> The strong versions would apply memory hole semantics to both windows
> starting at zero and whatever cxl_test window you choose.

In the interests of moving this forward and because cxl_test, while
useful, is a problem I created, here is a rough mockup of this proposal.

The observation that this hole detection logic is self contained and
suitable to be duplicated in the test code.

The other proposal in this mockup is that the names of these
augmentation functions be changed from "arch" to "platform" as this hole
is a total system / platform quirk not limited to a CPU arch.

Lastly, while I did not make that change I think lmh.c should probably
be renamed platform.c on the expectation that all future platform
quirkiness can land in that file. CONFIG_CXL_PLATFORM_QUIRKS can be
selected in the x86 case, but something tells me every arch that ships
CXL will eventually select CONFIG_CXL_PLATFORM_QUIRKS which might make
the config option moot in the future.

This builds and loads CXL test, and I see the mock versions of these
platform helpers being called.

-- 8< --
diff --git a/drivers/cxl/core/lmh.c b/drivers/cxl/core/lmh.c
index 9c55670c1c84..1bfe516aacf3 100644
--- a/drivers/cxl/core/lmh.c
+++ b/drivers/cxl/core/lmh.c
@@ -3,26 +3,12 @@
 #include <linux/range.h>
 #include <linux/pci.h>
 
+#include "cxlmem.h"
 #include "lmh.h"
 
 /* Start of CFMWS range that end before x86 Low Memory Holes */
 #define LMH_CFMWS_RANGE_START 0x0ULL
 
-static u64 mock_cfmws0_range_start = ULLONG_MAX;
-
-void set_mock_cfmws0_range_start(const u64 start)
-{
-	mock_cfmws0_range_start = start;
-}
-
-static u64 get_cfmws_range_start(const struct device *dev)
-{
-	if (dev_is_pci(dev))
-		return LMH_CFMWS_RANGE_START;
-
-	return mock_cfmws0_range_start;
-}
-
 /*
  * Match CXL Root and Endpoint Decoders by comparing SPA and HPA ranges.
  *
@@ -32,52 +18,48 @@ static u64 get_cfmws_range_start(const struct device *dev)
  * also larger than that of the matching root decoder. If there are LMH's,
  * the root decoder range end is always less than SZ_4G.
  */
-bool arch_match_spa(const struct cxl_root_decoder *cxlrd,
-		    const struct cxl_endpoint_decoder *cxled)
+__weak bool
+platform_root_decoder_contains(const struct cxl_root_decoder *cxlrd,
+			       const struct cxl_endpoint_decoder *cxled)
 {
 	const struct range *r1, *r2;
-	u64 cfmws_range_start;
 	int niw;
 
-	cfmws_range_start = get_cfmws_range_start(&cxled->cxld.dev);
-	if (cfmws_range_start == ULLONG_MAX)
-		return false;
-
 	r1 = &cxlrd->cxlsd.cxld.hpa_range;
 	r2 = &cxled->cxld.hpa_range;
 	niw = cxled->cxld.interleave_ways;
 
-	if (r1->start == cfmws_range_start && r1->start == r2->start &&
-	    r1->end < (cfmws_range_start + SZ_4G) && r1->end < r2->end &&
+	if (r1->start == LMH_CFMWS_RANGE_START && r1->start == r2->start &&
+	    r1->end < (LMH_CFMWS_RANGE_START + SZ_4G) && r1->end < r2->end &&
 	    IS_ALIGNED(range_len(r2), niw * SZ_256M))
 		return true;
 
 	return false;
 }
 
-/* Similar to arch_match_spa(), it matches regions and decoders */
-bool arch_match_region(const struct cxl_region_params *p,
-		       const struct cxl_decoder *cxld)
+__weak bool platform_region_contains(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 niw = cxld->interleave_ways;
-	u64 cfmws_range_start;
-
-	cfmws_range_start = get_cfmws_range_start(&cxld->dev);
-	if (cfmws_range_start == ULLONG_MAX)
-		return false;
 
-	if (res->start == cfmws_range_start && res->start == r->start &&
-	    res->end < (cfmws_range_start + SZ_4G) && res->end < r->end &&
+	if (res->start == LMH_CFMWS_RANGE_START && res->start == r->start &&
+	    res->end < (LMH_CFMWS_RANGE_START + SZ_4G) && res->end < r->end &&
 	    IS_ALIGNED(range_len(r), niw * SZ_256M))
 		return true;
 
 	return false;
 }
 
-void arch_adjust_region_resource(struct resource *res,
-				 struct cxl_root_decoder *cxlrd)
+void platform_region_adjust(const struct cxl_root_decoder *cxlrd,
+			    const struct cxl_endpoint_decoder *cxled,
+			    struct resource *res)
 {
-	res->end = cxlrd->res->end;
+	if (platform_root_decoder_contains(cxlrd, cxled)) {
+		res->end = cxlrd->res->end;
+		dev_dbg(cxled_to_memdev(cxled)->dev.parent,
+			"(LMH) has been adjusted (%s: %pr)\n",
+			dev_name(&cxled->cxld.dev), res);
+	}
 }
diff --git a/drivers/cxl/core/lmh.h b/drivers/cxl/core/lmh.h
index b6337120ee17..34f88b9e8290 100644
--- a/drivers/cxl/core/lmh.h
+++ b/drivers/cxl/core/lmh.h
@@ -2,30 +2,31 @@
 
 #include "cxl.h"
 
-void set_mock_cfmws0_range_start(u64 start);
-
 #ifdef CONFIG_CXL_ARCH_LOW_MEMORY_HOLE
-bool arch_match_spa(const struct cxl_root_decoder *cxlrd,
-		    const struct cxl_endpoint_decoder *cxled);
-bool arch_match_region(const struct cxl_region_params *p,
-		       const struct cxl_decoder *cxld);
-void arch_adjust_region_resource(struct resource *res,
-				 struct cxl_root_decoder *cxlrd);
+bool platform_root_decoder_contains(const struct cxl_root_decoder *cxlrd,
+				    const struct cxl_endpoint_decoder *cxled);
+bool platform_region_contains(const struct cxl_region_params *p,
+			      const struct cxl_decoder *cxld);
+void platform_region_adjust(const struct cxl_root_decoder *cxlrd,
+			    const struct cxl_endpoint_decoder *cxled,
+			    struct resource *res);
 #else
-static bool arch_match_spa(struct cxl_root_decoder *cxlrd,
-			   struct cxl_endpoint_decoder *cxled)
+static bool platform_root_decoder_contains(struct cxl_root_decoder *cxlrd,
+					   struct cxl_endpoint_decoder *cxled)
 {
 	return false;
 }
 
-static bool arch_match_region(struct cxl_region_params *p,
-			      struct cxl_decoder *cxld)
+static bool platform_region_contains(struct cxl_region_params *p,
+				     struct cxl_decoder *cxld)
 {
 	return false;
 }
 
-static void arch_adjust_region_resource(struct resource *res,
-					struct cxl_root_decoder *cxlrd)
+static inline void
+platform_region_adjust(const struct cxl_root_decoder *cxlrd,
+		       const struct cxl_endpoint_decoder *cxled,
+		       struct resource *res)
 {
 }
 #endif /* CXL_ARCH_LOW_MEMORY_HOLE */
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 9eb23ecedecf..7822e726ea8c 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -851,7 +851,7 @@ static bool region_res_match_cxl_range(const struct cxl_region_params *p,
 		return true;
 
 	cxld = container_of(range, struct cxl_decoder, hpa_range);
-	if (arch_match_region(p, cxld))
+	if (platform_region_contains(p, cxld))
 		return true;
 
 	return false;
@@ -1784,7 +1784,7 @@ static int match_switch_decoder_by_range(struct device *dev,
 		if (range_contains(r1, r2))
 			return 1;
 		cxlrd = to_cxl_root_decoder(dev);
-		if (arch_match_spa(cxlrd, cxled))
+		if (platform_root_decoder_contains(cxlrd, cxled))
 			return 1;
 	}
 	return (r1->start == r2->start && r1->end == r2->end);
@@ -1994,7 +1994,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) && !arch_match_spa(cxlrd, cxled)) {
+	    resource_size(p->res) && !platform_root_decoder_contains(cxlrd, cxled)) {
 		dev_dbg(&cxlr->dev,
 			"%s:%s-size-%#llx * ways-%d + cache-%#llx != region-size-%#llx\n",
 			dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev),
@@ -3231,7 +3231,7 @@ static int match_root_decoder_by_range(struct device *dev,
 
 	if (range_contains(r1, r2))
 		return true;
-	if (arch_match_spa(cxlrd, cxled))
+	if (platform_root_decoder_contains(cxlrd, cxled))
 		return true;
 
 	return false;
@@ -3254,7 +3254,7 @@ static int match_region_by_range(struct device *dev, const void *data)
 	if (p->res) {
 		if (p->res->start == r->start && p->res->end == r->end)
 			return 1;
-		if (arch_match_region(p, &cxled->cxld))
+		if (platform_region_contains(p, &cxled->cxld))
 			return 1;
 	}
 
@@ -3348,16 +3348,7 @@ static int __construct_region(struct cxl_region *cxlr,
 	 * Trim the HPA retrieved from hardware to fit the SPA mapped by the
 	 * platform
 	 */
-	if (arch_match_spa(cxlrd, cxled)) {
-		dev_dbg(cxlmd->dev.parent, "(LMH) Resource (%s: %pr)\n",
-			dev_name(&cxled->cxld.dev), res);
-
-		arch_adjust_region_resource(res, cxlrd);
-
-		dev_dbg(cxlmd->dev.parent,
-			"(LMH) has been adjusted (%s: %pr)\n",
-			dev_name(&cxled->cxld.dev), res);
-	}
+	platform_region_adjust(cxlrd, cxled, res);
 
 	rc = insert_resource(cxlrd->res, res);
 	if (rc) {
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index 3ec6b906371b..1b3a97ef3f11 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -74,7 +74,7 @@ static inline struct cxl_port *cxlrd_to_port(struct cxl_root_decoder *cxlrd)
 }
 
 static inline struct cxl_memdev *
-cxled_to_memdev(struct cxl_endpoint_decoder *cxled)
+cxled_to_memdev(const struct cxl_endpoint_decoder *cxled)
 {
 	struct cxl_port *port = to_cxl_port(cxled->cxld.dev.parent);
 
diff --git a/tools/testing/cxl/Kbuild b/tools/testing/cxl/Kbuild
index 3b3c24b1496e..9d24a7fd2536 100644
--- a/tools/testing/cxl/Kbuild
+++ b/tools/testing/cxl/Kbuild
@@ -65,6 +65,7 @@ cxl_core-y += $(CXL_CORE_SRC)/acpi.o
 cxl_core-y += $(CXL_CORE_SRC)/ras.o
 cxl_core-$(CONFIG_TRACING) += $(CXL_CORE_SRC)/trace.o
 cxl_core-$(CONFIG_CXL_ARCH_LOW_MEMORY_HOLE) += $(CXL_CORE_SRC)/lmh.o
+cxl_core-$(CONFIG_CXL_ARCH_LOW_MEMORY_HOLE) += mock_lmh.o
 cxl_core-$(CONFIG_CXL_REGION) += $(CXL_CORE_SRC)/region.o
 cxl_core-$(CONFIG_CXL_FEATURES) += $(CXL_CORE_SRC)/features.o
 cxl_core-$(CONFIG_CXL_MCE) += $(CXL_CORE_SRC)/mce.o
diff --git a/tools/testing/cxl/cxl_core_exports.c b/tools/testing/cxl/cxl_core_exports.c
index 7b20f9fcf0d7..f088792a8925 100644
--- a/tools/testing/cxl/cxl_core_exports.c
+++ b/tools/testing/cxl/cxl_core_exports.c
@@ -2,8 +2,6 @@
 /* Copyright(c) 2022 Intel Corporation. All rights reserved. */
 
 #include "cxl.h"
-#include "lmh.h"
 
 /* Exporting of cxl_core symbols that are only used by cxl_test */
 EXPORT_SYMBOL_NS_GPL(cxl_num_decoders_committed, "CXL");
-EXPORT_SYMBOL_NS_GPL(set_mock_cfmws0_range_start, "CXL");
diff --git a/tools/testing/cxl/mock_lmh.c b/tools/testing/cxl/mock_lmh.c
new file mode 100644
index 000000000000..b849166ba86a
--- /dev/null
+++ b/tools/testing/cxl/mock_lmh.c
@@ -0,0 +1,96 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/module.h>
+#include <linux/range.h>
+#include <linux/pci.h>
+
+#include <cxlmem.h>
+#include <lmh.h>
+#include "test/mock.h"
+
+static bool is_mock_dev(struct device *dev)
+{
+	struct cxl_mock_ops *(*get_ops_fn)(int *index);
+	struct cxl_mock_ops *ops = NULL;
+	void (*put_ops_fn)(int index);
+	bool is_mock = false;
+	int index;
+
+	get_ops_fn = symbol_get(get_cxl_mock_ops);
+	if (!get_ops_fn)
+		return false;
+	put_ops_fn = symbol_get(put_cxl_mock_ops);
+	if (!put_ops_fn)
+		goto out;
+
+	ops = get_ops_fn(&index);
+	if (ops)
+		is_mock = ops->is_mock_dev(dev);
+	put_ops_fn(index);
+
+out:
+	symbol_put(get_cxl_mock_ops);
+
+	return is_mock;
+}
+
+/* Start of CFMWS range that end before x86 Low Memory Holes */
+#define LMH_CFMWS_RANGE_START 0x0ULL
+
+static bool
+real_platform_root_decoder_contains(const struct cxl_root_decoder *cxlrd,
+				    const struct cxl_endpoint_decoder *cxled)
+{
+	const struct range *r1, *r2;
+	int niw;
+
+	r1 = &cxlrd->cxlsd.cxld.hpa_range;
+	r2 = &cxled->cxld.hpa_range;
+	niw = cxled->cxld.interleave_ways;
+
+	if (r1->start == LMH_CFMWS_RANGE_START && r1->start == r2->start &&
+	    r1->end < (LMH_CFMWS_RANGE_START + SZ_4G) && r1->end < r2->end &&
+	    IS_ALIGNED(range_len(r2), niw * SZ_256M))
+		return true;
+
+	return false;
+}
+
+bool platform_root_decoder_contains(const struct cxl_root_decoder *cxlrd,
+				    const struct cxl_endpoint_decoder *cxled)
+{
+	if (is_mock_dev(cxled_to_memdev(cxled)->dev.parent)) {
+		/* cxl_test_platform_root_decoder_contains(...) */
+		return false;
+	}
+
+	return real_platform_root_decoder_contains(cxlrd, cxled);
+}
+
+static bool real_platform_region_contains(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 niw = cxld->interleave_ways;
+
+	if (res->start == LMH_CFMWS_RANGE_START && res->start == r->start &&
+	    res->end < (LMH_CFMWS_RANGE_START + SZ_4G) && res->end < r->end &&
+	    IS_ALIGNED(range_len(r), niw * SZ_256M))
+		return true;
+
+	return false;
+}
+
+bool platform_region_contains(const struct cxl_region_params *p,
+			      const struct cxl_decoder *cxld)
+{
+	struct cxl_port *port = to_cxl_port(cxld->dev.parent);
+
+	if (is_mock_dev(port->uport_dev)) {
+		/* cxl_test_platform_root_decoder_contains(...) */
+		return false;
+	}
+
+	return real_platform_region_contains(p, cxld);
+}
diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
index 8c69ce0a272f..f54a648d2268 100644
--- a/tools/testing/cxl/test/cxl.c
+++ b/tools/testing/cxl/test/cxl.c
@@ -459,7 +459,6 @@ static int populate_cedt(void)
 			return -ENOMEM;
 		window->base_hpa = res->range.start;
 	}
-	set_mock_cfmws0_range_start(mock_cfmws[0]->base_hpa);
 
 	return 0;
 }
Re: [PATCH 4/4 v3] cxl/test: Simulate an x86 Low Memory Hole for tests
Posted by Fabio M. De Francesco 8 months, 3 weeks ago
On Saturday, March 29, 2025 12:40:47 AM Central European Standard Time Dan Williams wrote:
> 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.
> > 
> > Since the auto-created region of cxl-test uses mock_cfmws[0], whose 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 if
> > it was run on the mock environment created by cxl-tests.
> > 
> > Therefore, save the mock_cfmsw[0] range base_hpa and reuse it to match CXL
> > Root Decoders and Regions with Endpoint Decoders when the driver is run on
> > mock devices.
> > 
> > Since the auto-created region of cxl-test uses mock_cfmws[0], the
> > LMH path in the CXL Driver will be exercised every time the cxl-test
> > module is loaded. Executing unit test: cxl-topology.sh, confirms the
> > region created successfully with a LMH.
> > 
> > Cc: Alison Schofield <alison.schofield@intel.com>
> > Cc: Dan Williams <dan.j.williams@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/lmh.c               | 35 ++++++++++++++++++++++++----
> >  drivers/cxl/core/lmh.h               |  2 ++
> >  tools/testing/cxl/cxl_core_exports.c |  2 ++
> >  tools/testing/cxl/test/cxl.c         | 10 ++++++++
> >  4 files changed, 45 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/cxl/core/lmh.c b/drivers/cxl/core/lmh.c
> > index 2e32f867eb94..9c55670c1c84 100644
> > --- a/drivers/cxl/core/lmh.c
> > +++ b/drivers/cxl/core/lmh.c
> > @@ -1,11 +1,28 @@
> >  // SPDX-License-Identifier: GPL-2.0-only
> >  
> >  #include <linux/range.h>
> > +#include <linux/pci.h>
> > +
> >  #include "lmh.h"
> >  
> >  /* Start of CFMWS range that end before x86 Low Memory Holes */
> >  #define LMH_CFMWS_RANGE_START 0x0ULL
> >  
> > +static u64 mock_cfmws0_range_start = ULLONG_MAX;
> > +
> > +void set_mock_cfmws0_range_start(const u64 start)
> > +{
> > +	mock_cfmws0_range_start = start;
> > +}
> > +
> > +static u64 get_cfmws_range_start(const struct device *dev)
> > +{
> > +	if (dev_is_pci(dev))
> > +		return LMH_CFMWS_RANGE_START;
> > +
> > +	return mock_cfmws0_range_start;
> > +}
> > +
> 
> cxl_test should never result in "mock" infrastructure appearing outside
> of tools/testing/cxl/
> 
> >  /*
> >   * Match CXL Root and Endpoint Decoders by comparing SPA and HPA ranges.
> >   *
> > @@ -19,14 +36,19 @@ bool arch_match_spa(const struct cxl_root_decoder *cxlrd,
> >  		    const struct cxl_endpoint_decoder *cxled)
> >  {
> >  	const struct range *r1, *r2;
> > +	u64 cfmws_range_start;
> >  	int niw;
> >  
> > +	cfmws_range_start = get_cfmws_range_start(&cxled->cxld.dev);
> > +	if (cfmws_range_start == ULLONG_MAX)
> > +		return false;
> > +
> >  	r1 = &cxlrd->cxlsd.cxld.hpa_range;
> >  	r2 = &cxled->cxld.hpa_range;
> >  	niw = cxled->cxld.interleave_ways;
> >  
> > -	if (r1->start == LMH_CFMWS_RANGE_START && r1->start == r2->start &&
> > -	    r1->end < (LMH_CFMWS_RANGE_START + SZ_4G) && r1->end < r2->end &&
> > +	if (r1->start == cfmws_range_start && r1->start == r2->start &&
> > +	    r1->end < (cfmws_range_start + SZ_4G) && r1->end < r2->end &&
> >  	    IS_ALIGNED(range_len(r2), niw * SZ_256M))
> >  		return true;
> >  
> > @@ -40,9 +62,14 @@ bool arch_match_region(const struct cxl_region_params *p,
> >  	const struct range *r = &cxld->hpa_range;
> >  	const struct resource *res = p->res;
> >  	int niw = cxld->interleave_ways;
> > +	u64 cfmws_range_start;
> > +
> > +	cfmws_range_start = get_cfmws_range_start(&cxld->dev);
> > +	if (cfmws_range_start == ULLONG_MAX)
> > +		return false;
> >  
> > -	if (res->start == LMH_CFMWS_RANGE_START && res->start == r->start &&
> > -	    res->end < (LMH_CFMWS_RANGE_START + SZ_4G) && res->end < r->end &&
> > +	if (res->start == cfmws_range_start && res->start == r->start &&
> > +	    res->end < (cfmws_range_start + SZ_4G) && res->end < r->end &&
> >  	    IS_ALIGNED(range_len(r), niw * SZ_256M))
> >  		return true;
> 
> Someone should be able to read the straight line CXL driver code and
> never know that an alternate implementation exists for changing these
> details.
> 
> So, the mock interface for this stuff should intercept at the
> arch_match_spa() and arch_match_region() level.
> 
> To me that looks like mark these implementations with the __mock
> attribute, similar to to_cxl_host_bridge(). Then define strong versions
> in tools/testing/cxl/mock_lmh.c.
> 
> The strong versions would apply memory hole semantics to both windows
> starting at zero and whatever cxl_test window you choose.
> 
I thought the same and wanted to use the strong/weak mechanism, but then 
I noticed that the strong version (in tools/testing/cxl/mock_lmh.c) was never
called. I think it never happens because of the weak version is called from 
cxl_core. I think that all functions called from cxl_core can't be override
from cxl_test. 

Is that deduction unfounded? Am I missing something?

Thanks,

Fabio

P.S.: Please notice that to_cxl_host_bridge() is never used in cxl_core.
Re: [PATCH 4/4 v3] cxl/test: Simulate an x86 Low Memory Hole for tests
Posted by Fabio M. De Francesco 8 months, 3 weeks ago
On Saturday, March 29, 2025 11:16:09 AM Central European Standard Time Fabio M. De Francesco wrote:
> On Saturday, March 29, 2025 12:40:47 AM Central European Standard Time Dan Williams wrote:
> > 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.
> > > 
> > > Since the auto-created region of cxl-test uses mock_cfmws[0], whose 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 if
> > > it was run on the mock environment created by cxl-tests.
> > > 
> > > Therefore, save the mock_cfmsw[0] range base_hpa and reuse it to match CXL
> > > Root Decoders and Regions with Endpoint Decoders when the driver is run on
> > > mock devices.
> > > 
> > > Since the auto-created region of cxl-test uses mock_cfmws[0], the
> > > LMH path in the CXL Driver will be exercised every time the cxl-test
> > > module is loaded. Executing unit test: cxl-topology.sh, confirms the
> > > region created successfully with a LMH.
> > > 
> > > Cc: Alison Schofield <alison.schofield@intel.com>
> > > Cc: Dan Williams <dan.j.williams@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/lmh.c               | 35 ++++++++++++++++++++++++----
> > >  drivers/cxl/core/lmh.h               |  2 ++
> > >  tools/testing/cxl/cxl_core_exports.c |  2 ++
> > >  tools/testing/cxl/test/cxl.c         | 10 ++++++++
> > >  4 files changed, 45 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/cxl/core/lmh.c b/drivers/cxl/core/lmh.c
> > > index 2e32f867eb94..9c55670c1c84 100644
> > > --- a/drivers/cxl/core/lmh.c
> > > +++ b/drivers/cxl/core/lmh.c
> > > @@ -1,11 +1,28 @@
> > >  // SPDX-License-Identifier: GPL-2.0-only
> > >  
> > >  #include <linux/range.h>
> > > +#include <linux/pci.h>
> > > +
> > >  #include "lmh.h"
> > >  
> > >  /* Start of CFMWS range that end before x86 Low Memory Holes */
> > >  #define LMH_CFMWS_RANGE_START 0x0ULL
> > >  
> > > +static u64 mock_cfmws0_range_start = ULLONG_MAX;
> > > +
> > > +void set_mock_cfmws0_range_start(const u64 start)
> > > +{
> > > +	mock_cfmws0_range_start = start;
> > > +}
> > > +
> > > +static u64 get_cfmws_range_start(const struct device *dev)
> > > +{
> > > +	if (dev_is_pci(dev))
> > > +		return LMH_CFMWS_RANGE_START;
> > > +
> > > +	return mock_cfmws0_range_start;
> > > +}
> > > +
> > 
> > cxl_test should never result in "mock" infrastructure appearing outside
> > of tools/testing/cxl/
> > 
> > >  /*
> > >   * Match CXL Root and Endpoint Decoders by comparing SPA and HPA ranges.
> > >   *
> > > @@ -19,14 +36,19 @@ bool arch_match_spa(const struct cxl_root_decoder *cxlrd,
> > >  		    const struct cxl_endpoint_decoder *cxled)
> > >  {
> > >  	const struct range *r1, *r2;
> > > +	u64 cfmws_range_start;
> > >  	int niw;
> > >  
> > > +	cfmws_range_start = get_cfmws_range_start(&cxled->cxld.dev);
> > > +	if (cfmws_range_start == ULLONG_MAX)
> > > +		return false;
> > > +
> > >  	r1 = &cxlrd->cxlsd.cxld.hpa_range;
> > >  	r2 = &cxled->cxld.hpa_range;
> > >  	niw = cxled->cxld.interleave_ways;
> > >  
> > > -	if (r1->start == LMH_CFMWS_RANGE_START && r1->start == r2->start &&
> > > -	    r1->end < (LMH_CFMWS_RANGE_START + SZ_4G) && r1->end < r2->end &&
> > > +	if (r1->start == cfmws_range_start && r1->start == r2->start &&
> > > +	    r1->end < (cfmws_range_start + SZ_4G) && r1->end < r2->end &&
> > >  	    IS_ALIGNED(range_len(r2), niw * SZ_256M))
> > >  		return true;
> > >  
> > > @@ -40,9 +62,14 @@ bool arch_match_region(const struct cxl_region_params *p,
> > >  	const struct range *r = &cxld->hpa_range;
> > >  	const struct resource *res = p->res;
> > >  	int niw = cxld->interleave_ways;
> > > +	u64 cfmws_range_start;
> > > +
> > > +	cfmws_range_start = get_cfmws_range_start(&cxld->dev);
> > > +	if (cfmws_range_start == ULLONG_MAX)
> > > +		return false;
> > >  
> > > -	if (res->start == LMH_CFMWS_RANGE_START && res->start == r->start &&
> > > -	    res->end < (LMH_CFMWS_RANGE_START + SZ_4G) && res->end < r->end &&
> > > +	if (res->start == cfmws_range_start && res->start == r->start &&
> > > +	    res->end < (cfmws_range_start + SZ_4G) && res->end < r->end &&
> > >  	    IS_ALIGNED(range_len(r), niw * SZ_256M))
> > >  		return true;
> > 
> > Someone should be able to read the straight line CXL driver code and
> > never know that an alternate implementation exists for changing these
> > details.
> > 
> > So, the mock interface for this stuff should intercept at the
> > arch_match_spa() and arch_match_region() level.
> > 
> > To me that looks like mark these implementations with the __mock
> > attribute, similar to to_cxl_host_bridge(). Then define strong versions
> > in tools/testing/cxl/mock_lmh.c.
> > 
> > The strong versions would apply memory hole semantics to both windows
> > starting at zero and whatever cxl_test window you choose.
> > 
> I thought the same and wanted to use the strong/weak mechanism, but then 
> I noticed that the strong version (in tools/testing/cxl/mock_lmh.c) was never
> called. I think it never happens because of the weak version is called from 
> cxl_core. I think that all functions called from cxl_core can't be override
> from cxl_test. 
> 
> Is that deduction unfounded? Am I missing something?
> 
> Thanks,
> 
> Fabio
> 
> P.S.: Please notice that to_cxl_host_bridge() is never used in cxl_core.
> 
I mistakenly thought you were suggesting something like the wrap approach
that is not possible if the caller of the wrapped function is internal to 
the CXL core.[1]

Fabio

[1] https://lore.kernel.org/all/6711b7c0c0b53_3ee2294a6@dwillia2-xfh.jf.intel.com.notmuch/

 
Re: [PATCH 4/4 v3] cxl/test: Simulate an x86 Low Memory Hole for tests
Posted by Robert Richter 9 months ago
On 14.03.25 12:36:33, 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.
> 
> Since the auto-created region of cxl-test uses mock_cfmws[0], whose 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 if
> it was run on the mock environment created by cxl-tests.
> 
> Therefore, save the mock_cfmsw[0] range base_hpa and reuse it to match CXL
> Root Decoders and Regions with Endpoint Decoders when the driver is run on
> mock devices.
> 
> Since the auto-created region of cxl-test uses mock_cfmws[0], the
> LMH path in the CXL Driver will be exercised every time the cxl-test
> module is loaded. Executing unit test: cxl-topology.sh, confirms the
> region created successfully with a LMH.
> 
> Cc: Alison Schofield <alison.schofield@intel.com>
> Cc: Dan Williams <dan.j.williams@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/lmh.c               | 35 ++++++++++++++++++++++++----
>  drivers/cxl/core/lmh.h               |  2 ++

Can you take a look to move all those changes to testing/? This
indicates the interface of your mock functions need improvement.

-Robert

>  tools/testing/cxl/cxl_core_exports.c |  2 ++
>  tools/testing/cxl/test/cxl.c         | 10 ++++++++
>  4 files changed, 45 insertions(+), 4 deletions(-)
Re: [PATCH 4/4 v3] cxl/test: Simulate an x86 Low Memory Hole for tests
Posted by Fabio M. De Francesco 8 months, 3 weeks ago
On Friday, March 21, 2025 11:42:09 AM Central European Standard Time Robert Richter wrote:
> On 14.03.25 12:36:33, 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.
> > 
> > Since the auto-created region of cxl-test uses mock_cfmws[0], whose 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 if
> > it was run on the mock environment created by cxl-tests.
> > 
> > Therefore, save the mock_cfmsw[0] range base_hpa and reuse it to match CXL
> > Root Decoders and Regions with Endpoint Decoders when the driver is run on
> > mock devices.
> > 
> > Since the auto-created region of cxl-test uses mock_cfmws[0], the
> > LMH path in the CXL Driver will be exercised every time the cxl-test
> > module is loaded. Executing unit test: cxl-topology.sh, confirms the
> > region created successfully with a LMH.
> > 
> > Cc: Alison Schofield <alison.schofield@intel.com>
> > Cc: Dan Williams <dan.j.williams@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/lmh.c               | 35 ++++++++++++++++++++++++----
> >  drivers/cxl/core/lmh.h               |  2 ++
> 
> Can you take a look to move all those changes to testing/? This
> indicates the interface of your mock functions need improvement.
> 
> -Robert
> 
> >  tools/testing/cxl/cxl_core_exports.c |  2 ++
> >  tools/testing/cxl/test/cxl.c         | 10 ++++++++
> >  4 files changed, 45 insertions(+), 4 deletions(-)
> 
Your comment is not very detailed but I think you are suggesting to 
override the lmh functions in testing. If that is the case, I don't think 
that any function which is exported from and also called by cxl-core 
can be override by testing with the strong/weak mechanism. 

But I'm not an expert of linker related topics and not even sure that I 
understood what you suggested.

Would you please elaborate more?

Thanks,

Fabio
Re: [PATCH 4/4 v3] cxl/test: Simulate an x86 Low Memory Hole for tests
Posted by Robert Richter 8 months, 3 weeks ago
On 26.03.25 17:58:50, Fabio M. De Francesco wrote:
> On Friday, March 21, 2025 11:42:09 AM Central European Standard Time Robert Richter wrote:
> > On 14.03.25 12:36:33, 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.
> > > 
> > > Since the auto-created region of cxl-test uses mock_cfmws[0], whose 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 if
> > > it was run on the mock environment created by cxl-tests.
> > > 
> > > Therefore, save the mock_cfmsw[0] range base_hpa and reuse it to match CXL
> > > Root Decoders and Regions with Endpoint Decoders when the driver is run on
> > > mock devices.
> > > 
> > > Since the auto-created region of cxl-test uses mock_cfmws[0], the
> > > LMH path in the CXL Driver will be exercised every time the cxl-test
> > > module is loaded. Executing unit test: cxl-topology.sh, confirms the
> > > region created successfully with a LMH.
> > > 
> > > Cc: Alison Schofield <alison.schofield@intel.com>
> > > Cc: Dan Williams <dan.j.williams@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/lmh.c               | 35 ++++++++++++++++++++++++----
> > >  drivers/cxl/core/lmh.h               |  2 ++
> > 
> > Can you take a look to move all those changes to testing/? This
> > indicates the interface of your mock functions need improvement.
> > 
> > -Robert
> > 
> > >  tools/testing/cxl/cxl_core_exports.c |  2 ++
> > >  tools/testing/cxl/test/cxl.c         | 10 ++++++++
> > >  4 files changed, 45 insertions(+), 4 deletions(-)
> > 
> Your comment is not very detailed but I think you are suggesting to 
> override the lmh functions in testing. If that is the case, I don't think 
> that any function which is exported from and also called by cxl-core 
> can be override by testing with the strong/weak mechanism. 

I think you need to modify cxl_test to create a LMH hole (register a
mock bridge and root port with the necessary parameters, etc.) and
then run cxl_core unmodified.

-Robert

> 
> But I'm not an expert of linker related topics and not even sure that I 
> understood what you suggested.
> 
> Would you please elaborate more?
> 
> Thanks,
> 
> Fabio


Re: [PATCH 4/4 v3] cxl/test: Simulate an x86 Low Memory Hole for tests
Posted by Ira Weiny 9 months ago
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.
> 
> Since the auto-created region of cxl-test uses mock_cfmws[0], whose 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 if
> it was run on the mock environment created by cxl-tests.
> 
> Therefore, save the mock_cfmsw[0] range base_hpa and reuse it to match CXL
> Root Decoders and Regions with Endpoint Decoders when the driver is run on
> mock devices.
> 
> Since the auto-created region of cxl-test uses mock_cfmws[0], the
> LMH path in the CXL Driver will be exercised every time the cxl-test
> module is loaded. Executing unit test: cxl-topology.sh, confirms the
> region created successfully with a LMH.
> 
> Cc: Alison Schofield <alison.schofield@intel.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Ira Weiny <ira.weiny@intel.com>

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

[snip]