Re: [PATCH v3] cxl/test: Enforce PMD alignment for volatile mock regions

[Alison Schofield <alison.schofield@intel.com>]

On Wed, May 27, 2026 at 12:08:07PM +0800, Richard Cheng wrote:
> 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:

snip

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

Sounds good, and right.  Thanks for the explanation.


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

I don't think we need the backtrace of warn, how about this:

if (!IS_ALIGNED(mock_auto_region_size, PMD_SIZE)) {
    pr_err_once("mock_auto_region_size %d must be PMD-aligned\n",
                mock_auto_region_size);
    return -EINVAL;
}

> 
> Best regards,
> Richard Cheng.
> 
> > > +
> > >  	cxl_acpi_test();
> > >  	cxl_core_test();
> > >  	cxl_mem_test();
> > > 
> > > base-commit: 6779b50faa562e6cca1aa6a4649a4d764c6c7e28
> > > -- 
> > > 2.43.0
> > >