From: Dave Jiang <dave.jiang@intel.com>
To: "Fabio M. De Francesco" <fabio.m.de.francesco@linux.intel.com>,
linux-cxl@vger.kernel.org
Cc: Davidlohr Bueso <dave@stgolabs.net>,
Jonathan Cameron <jonathan.cameron@huawei.com>,
Alison Schofield <alison.schofield@intel.com>,
Vishal Verma <vishal.l.verma@intel.com>,
Ira Weiny <ira.weiny@intel.com>,
Dan Williams <dan.j.williams@intel.com>,
Robert Richter <rrichter@amd.com>,
ming.li@zohomail.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 4/4 v4] cxl/test: Simulate an x86 Low Memory Hole for tests
Date: Mon, 8 Sep 2025 11:26:02 -0700 [thread overview]
Message-ID: <25a2be9a-a702-4db2-9db7-6f76348d89fd@intel.com> (raw)
In-Reply-To: <20250724142144.776992-5-fabio.m.de.francesco@linux.intel.com>
On 7/24/25 7:20 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. Executing unit test: cxl-topology.sh, confirms the region created
> successfully with a LMH.
>
> 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.
>
> 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.
>
> 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.c | 9 +-
> tools/testing/cxl/Kbuild | 1 +
> tools/testing/cxl/mock_platform.c | 137 ++++++++++++++++++++++++++++++
> tools/testing/cxl/test/cxl.c | 10 +++
> tools/testing/cxl/test/mock.h | 1 +
> 5 files changed, 154 insertions(+), 4 deletions(-)
> create mode 100644 tools/testing/cxl/mock_platform.c
>
> diff --git a/drivers/cxl/core/platform.c b/drivers/cxl/core/platform.c
> index 8202750742d0..ba1dafece495 100644
> --- a/drivers/cxl/core/platform.c
> +++ b/drivers/cxl/core/platform.c
> @@ -17,8 +17,9 @@
> * 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 platform_root_decoder_contains(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)
Can you take a look at this and see if something similar can be used instead of what you are doing here? Would prefer not introduce a third type of emulation.
https://lore.kernel.org/linux-cxl/20250829180928.842707-8-dave.jiang@intel.com/T/#u
> {
> const struct range *r1, *r2;
> int niw;
> @@ -39,8 +40,8 @@ bool platform_root_decoder_contains(const struct cxl_root_decoder *cxlrd,
> * Similar to platform_root_decoder_contains(), it matches regions and
> * decoders
> */
> -bool platform_region_contains(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;
> diff --git a/tools/testing/cxl/Kbuild b/tools/testing/cxl/Kbuild
> index 77e392c4b541..64c5c8c34805 100644
> --- a/tools/testing/cxl/Kbuild
> +++ b/tools/testing/cxl/Kbuild
> @@ -66,6 +66,7 @@ cxl_core-y += $(CXL_CORE_SRC)/acpi.o
> cxl_core-$(CONFIG_TRACING) += $(CXL_CORE_SRC)/trace.o
> cxl_core-$(CONFIG_CXL_REGION) += $(CXL_CORE_SRC)/region.o
> cxl_core-$(CONFIG_CXL_PLATFORM_QUIRKS) += $(CXL_CORE_SRC)/platform.o
> +cxl_core-$(CONFIG_CXL_PLATFORM_QUIRKS) += mock_platform.o
> cxl_core-$(CONFIG_CXL_MCE) += $(CXL_CORE_SRC)/mce.o
> cxl_core-$(CONFIG_CXL_FEATURES) += $(CXL_CORE_SRC)/features.o
> cxl_core-$(CONFIG_CXL_EDAC_MEM_FEATURES) += $(CXL_CORE_SRC)/edac.o
> diff --git a/tools/testing/cxl/mock_platform.c b/tools/testing/cxl/mock_platform.c
> new file mode 100644
> index 000000000000..1775c64b3c7c
> --- /dev/null
> +++ b/tools/testing/cxl/mock_platform.c
> @@ -0,0 +1,137 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/completion.h>
> +#include <linux/module.h>
> +#include <linux/range.h>
> +#include <linux/pci.h>
> +
> +#include <cxlmem.h>
> +#include <platform.h>
> +#include "test/mock.h"
> +
> +static u64 mock_cfmws0_range_start;
> +
> +void set_mock_cfmws0_range_start(u64 start)
> +{
> + mock_cfmws0_range_start = start;
> +}
> +EXPORT_SYMBOL_NS_GPL(set_mock_cfmws0_range_start, "CXL");
> +
> +static bool is_mock_port(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_port(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;
cxlsd_range
cxled_range
Makes it easier to read vs r1 and r2 and knowing what's being compared.
> + 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))
Can this check be turned into a helper function? Also, maybe line it up with one check per line. Otherwise it looks rather confusing. Comment applies to rest of the patch.
DJ
> + return true;
> +
> + return false;
> +}
> +
> +static bool
> +cxl_test_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 == mock_cfmws0_range_start && r1->start == r2->start &&
> + r1->end < (mock_cfmws0_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)
> +{
> + struct cxl_port *port = to_cxl_port(cxled->cxld.dev.parent);
> +
> + if (is_mock_port(port->uport_dev))
> + return cxl_test_platform_root_decoder_contains(cxlrd, cxled);
> +
> + 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;
> +}
> +
> +static bool cxl_test_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 == 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), 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_port(port->uport_dev))
> + return cxl_test_platform_region_contains(p, cxld);
> +
> + return real_platform_region_contains(p, cxld);
> +}
> diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
> index 8a5815ca870d..a411c055d390 100644
> --- a/tools/testing/cxl/test/cxl.c
> +++ b/tools/testing/cxl/test/cxl.c
> @@ -212,7 +212,11 @@ static struct {
> .restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
> 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 },
> },
> @@ -453,6 +457,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;
> @@ -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_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;
> diff --git a/tools/testing/cxl/test/mock.h b/tools/testing/cxl/test/mock.h
> index d1b0271d2822..792eabbd0f18 100644
> --- a/tools/testing/cxl/test/mock.h
> +++ b/tools/testing/cxl/test/mock.h
> @@ -32,3 +32,4 @@ void register_cxl_mock_ops(struct cxl_mock_ops *ops);
> void unregister_cxl_mock_ops(struct cxl_mock_ops *ops);
> struct cxl_mock_ops *get_cxl_mock_ops(int *index);
> void put_cxl_mock_ops(int index);
> +void set_mock_cfmws0_range_start(u64 start);
next prev parent reply other threads:[~2025-09-08 18:26 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-07-24 14:20 [PATCH 0/4 v4] cxl/core: Enable Region creation/attach on x86 with LMH Fabio M. De Francesco
2025-07-24 14:20 ` [PATCH 1/4 v4] cxl/core: Change match_*_by_range() signatures Fabio M. De Francesco
2025-08-01 20:04 ` Cheatham, Benjamin
2025-08-13 12:27 ` Jonathan Cameron
2025-09-05 0:31 ` Dave Jiang
2025-07-24 14:20 ` [PATCH 2/4 v4] cxl/core: Add helpers to detect Low Memory Holes on x86 Fabio M. De Francesco
2025-08-01 20:04 ` Cheatham, Benjamin
2025-08-21 13:25 ` Fabio M. De Francesco
2025-09-18 14:23 ` Fabio M. De Francesco
2025-09-05 22:13 ` Dave Jiang
2025-07-24 14:20 ` [PATCH 3/4 v4] cxl/core: Enable Region creation on x86 with LMH Fabio M. De Francesco
2025-08-01 20:04 ` Cheatham, Benjamin
2025-08-21 15:15 ` Fabio M. De Francesco
2025-09-05 23:14 ` Dave Jiang
2025-07-24 14:20 ` [PATCH 4/4 v4] cxl/test: Simulate an x86 Low Memory Hole for tests Fabio M. De Francesco
2025-08-20 21:08 ` Alison Schofield
2025-09-08 18:26 ` Dave Jiang [this message]
2025-09-05 0:07 ` [PATCH 0/4 v4] cxl/core: Enable Region creation/attach on x86 with LMH Dave Jiang
2025-09-06 0:03 ` Dave Jiang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=25a2be9a-a702-4db2-9db7-6f76348d89fd@intel.com \
--to=dave.jiang@intel.com \
--cc=alison.schofield@intel.com \
--cc=dan.j.williams@intel.com \
--cc=dave@stgolabs.net \
--cc=fabio.m.de.francesco@linux.intel.com \
--cc=ira.weiny@intel.com \
--cc=jonathan.cameron@huawei.com \
--cc=linux-cxl@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=ming.li@zohomail.com \
--cc=rrichter@amd.com \
--cc=vishal.l.verma@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.