From: Richard Cheng <icheng@nvidia.com>
To: Alison Schofield <alison.schofield@intel.com>
Cc: dave@stgolabs.ne, jonathan.cameron@huawei.com,
dave.jiang@intel.com, vishal.l.verma@intel.com,
ira.weiny@intel.com, dan.j.williams@intel.com,
fabio.m.de.francesco@linux.intel.com, rrichter@amd.com,
ming.li@zohomail.com, linux-cxl@vger.kernel.org,
linux-kernel@vger.kernel.org, newtonl@nvidia.com,
kristinc@nvidia.com, kaihengf@nvidia.com, kobak@nvidia.com
Subject: Re: [PATCH v3] cxl/test: Enforce PMD alignment for volatile mock regions
Date: Wed, 27 May 2026 12:08:07 +0800 [thread overview]
Message-ID: <ahZsCOIoIt9Ejom_@MWDK4CY14F> (raw)
In-Reply-To: <ahYq-pJIfNWJr4TR@aschofie-mobl2.lan>
On Tue, May 26, 2026 at 04:21:30PM +0800, Alison Schofield wrote:
> On Fri, May 22, 2026 at 01:44:57PM +0800, Richard Cheng wrote:
> > cxl_test allocates synthetic CFMWS HPA windows from a gen_pool with
> > SZ_256M alignment. On arm64 with CONFIG_ARM64_64K_PAGES=y and
> > CONFIG_PGTABLE_LEVELS=3, PMD_SIZE is 512M, so every CXL region carved
> > from a volatile window inherits a non-PMD-aligned start, and
> > cxl_dax_region_probe() -> alloc_dax_region() fails:
> >
> > """
> > cxl_dax_region dax_region1: probe with driver cxl_dax_region failed
> > with error -12
> > """
> >
> > Enforce that every volatile mock CFMWS is PMD-aligned in both start and
> > size
> >
> > Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> > Acked-by: Kai-Heng Feng <kaihengf@nvidia.com>
> > Signed-off-by: Richard Cheng <icheng@nvidia.com>
> > ---
> > Changelog:
> >
> > v2->v3:
> > - Correct hunk headers
> > - Add base commit SHA
> >
> > v1->v2:
> > - Reframe from "fix start alignment" to "enforce PMD invariant for
> > volatile CFMWS"
> > - Bump cfmws5.window_size to max(SZ_256M, PMD_SIZE)
> > - Gate populate_cedt() alignment on
> > ACPI_CEDT_CFMWS_RESTRICT_VOLATILE so PMEM windows stay at SZ_256M
> > - Add BUILD_BUG_ON for MOCK_AUTO_REGION_SIZE_DEFAULT v.s. PMD_SIZE
> > in cxl_test_init()
> >
> > Best regards,
> > Richard Cheng.
> > ---
> > tools/testing/cxl/test/cxl.c | 9 +++++++--
> > 1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
> > index 418669927fb0..7a1105f9a0bb 100644
> > --- a/tools/testing/cxl/test/cxl.c
> > +++ b/tools/testing/cxl/test/cxl.c
> > @@ -318,7 +318,7 @@ static struct {
> > .restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
> > ACPI_CEDT_CFMWS_RESTRICT_VOLATILE,
> > .qtg_id = FAKE_QTG_ID,
> > - .window_size = SZ_256M,
> > + .window_size = SZ_256M > PMD_SIZE ? SZ_256M : PMD_SIZE,
>
> Why not this: max_t(resource_size_t, SZ_256M, PMD_SIZE)
>
Hi Alison,
Thanks for the feedback, but I doubt this will work.
"max_t()" expands to a statement expression, which isn't a constant expression so it can't be used in this file-scope static initializer.
I did the change and compile, the error is
"""
include/linux/minmax.h:86:9: error: braced-group within expression
allowed only inside a function
86 | ({ type ux = (x); type uy = (y); __cmp(op, ux, uy); })
| ^
test/cxl.c:295:40: note: in expansion of macro 'max_t'
295 | .window_size = max_t(resource_size_t, SZ_256M, PMD_SIZE),
"""
So I don't think this approach works, I would like to stick to ternary form if that's ok for you.
>
> > },
> > .target = { 3 },
> > },
> > @@ -495,9 +495,12 @@ static int populate_cedt(void)
> >
> > for (i = cfmws_start; i <= cfmws_end; i++) {
> > struct acpi_cedt_cfmws *window = mock_cfmws[i];
> > + int align = SZ_256M;
> >
> > cfmws_elc_update(window, i);
> > - res = alloc_mock_res(window->window_size, SZ_256M);
> > + if (window->restrictions & ACPI_CEDT_CFMWS_RESTRICT_VOLATILE)
> > + align = max_t(int, SZ_256M, PMD_SIZE);
> > + res = alloc_mock_res(window->window_size, align);
> > if (!res)
> > return -ENOMEM;
> > window->base_hpa = res->range.start;
> > @@ -1826,6 +1829,8 @@ static __init int cxl_test_init(void)
> > int rc, i;
> > struct range mappable;
> >
> > + BUILD_BUG_ON(!IS_ALIGNED(MOCK_AUTO_REGION_SIZE_DEFAULT, PMD_SIZE));
>
> The rule we need to enforce is tha the actual auto region size is
> PMD aligned, not the default definition. So I think this needs to
> be a runtime check of IS_ALIGNED(mock_auto_region_size, PMD_SIZE).
>
> -- Alison
>
Thanks for the catch, I'll fix this and send a v4 to make it runtime check.
Do you prefer WARN_ON_ONCE() to be ammend or no? I mean
"""
if (!IS_ALIGNED(mock_auto_region_size, PMD_SIZE))
return -EINVAL;
"""
or
"""
if (WARN_ON_ONCE(!IS_ALIGNED(mock_auto_region_size, PMD_SIZE)))
return -EINVAL;
"""
Which one do you prefer ? or any other idea ? please let me know and I'll make the change.
Best regards,
Richard Cheng.
> > +
> > cxl_acpi_test();
> > cxl_core_test();
> > cxl_mem_test();
> >
> > base-commit: 6779b50faa562e6cca1aa6a4649a4d764c6c7e28
> > --
> > 2.43.0
> >
next prev parent reply other threads:[~2026-05-27 4:08 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-22 5:44 Richard Cheng
2026-05-26 23:21 ` Alison Schofield
2026-05-27 4:08 ` Richard Cheng [this message]
2026-05-27 5:48 ` Alison Schofield
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=ahZsCOIoIt9Ejom_@MWDK4CY14F \
--to=icheng@nvidia.com \
--cc=alison.schofield@intel.com \
--cc=dan.j.williams@intel.com \
--cc=dave.jiang@intel.com \
--cc=dave@stgolabs.ne \
--cc=fabio.m.de.francesco@linux.intel.com \
--cc=ira.weiny@intel.com \
--cc=jonathan.cameron@huawei.com \
--cc=kaihengf@nvidia.com \
--cc=kobak@nvidia.com \
--cc=kristinc@nvidia.com \
--cc=linux-cxl@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=ming.li@zohomail.com \
--cc=newtonl@nvidia.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®