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

Fabio M. De Francesco
2025-03-29T10:16:16Z

[-- Attachment #1: Type: text/plain, Size: 5375 bytes --]

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.

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 488 bytes --]