Re: [PATCH v24 10/11] cxl: Avoid dax creation for accelerators

Alison Schofield
2026-03-30T22:36:23Z

On Mon, Mar 30, 2026 at 03:26:41PM +0100, Alejandro Lucero Palau wrote:
>
> On 3/25/26 02:59, Alison Schofield wrote:
> > On Mon, Mar 23, 2026 at 11:31:16AM +0000, alejandro.lucero-palau@amd.com wrote:
> > > From: Alejandro Lucero <alucerop@amd.com>
> > >
> > > By definition a type2 cxl device will use the host managed memory for
> > > specific functionality, therefore it should not be available to other
> > > uses.
> > Hi Alejandro,
> >
> > I'm wondering if we're skipping too much, or perhaps just needs
> > a clarifying comment?
> >
> > Commit message says 'Avoid dax creation...', that's specific.
> >
> > Commit log says 'should not be available to other uses', that's less specific
>
>
> Hi Allison,
>
>
> Would you prefer something like "should not be available to other uses like
> DAX".?

The point really is that this skips 3 things, not just DAX creation.
So, address each of those things in the commit message, or narrow
what is skipped to be only the DAX creation.

see below...

>
>
> > follow me ...
> >
> > > Signed-off-by: Alejandro Lucero <alucerop@amd.com>
> > > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > > Reviewed-by: Davidlohr Bueso <daves@stgolabs.net>
> > > Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> > > Reviewed-by: Ben Cheatham <benjamin.cheatham@amd.com>
> > > ---
> > > drivers/cxl/core/region.c | 7 +++++++
> > > 1 file changed, 7 insertions(+)
> > >
> > > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> > > index 8bb53a095290..2f9bdb4f1f4f 100644
> > > --- a/drivers/cxl/core/region.c
> > > +++ b/drivers/cxl/core/region.c
> > > @@ -4263,6 +4263,13 @@ static int cxl_region_probe(struct device *dev)
> > > if (rc)
> > > return rc;
> > > + /*
> > > + * HDM-D[B] (device-memory) regions have accelerator specific usage.
> > > + * Skip device-dax registration.
> > > + */
> > > + if (cxlr->type == CXL_DECODER_DEVMEM)
> > > + return 0;
> > > +
> > And above says 'Skip device-dax' specific again, and is indeed skipped.
> >
> > This escape is not surgically placed at the point where dax is added.
> > We are skipping a few other things before dax registration actually
> > happens. Why skip memory notifiers, shutdown_notifiers, poison setup?
>
>
> posion setup crashes with a type2 and I'm not sure if the memory notifiers
> make sense for Type2.
>

wrt - poison set crashes with a type2:

Let's not leave a known crash to be tripped upon in the future.

The crash is probably on a dereference on cxl_memdev_state, mds, which
you guarded against in Patch 4/11 in other paths. I think this needs
similar check added to cxl_memdev_has_poison_cmd(). With that, I expect
the cxl_region_setup_poison() in this path here. Success meaning it looked
to see if it's member devices supported poison, they do not, so no poison
set up.

I'm not familiar w the notifiers.

>
> If it is really needed, I would prefer to support it as a follow up.
>
>
> Thank you
>
>
> >
> > > /*
> > > * From this point on any path that changes the region's state away from
> > > * CXL_CONFIG_COMMIT is also responsible for releasing the driver.
> > > --
> > > 2.34.1
> > >
> > >