[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aXFk2pr7mFw4eqMK@gourry-fedora-PF4VCD3F>
Date: Wed, 21 Jan 2026 18:44:26 -0500
From: Gregory Price <gourry@...rry.net>
To: Ira Weiny <ira.weiny@...el.com>
Cc: linux-cxl@...r.kernel.org, linux-kernel@...r.kernel.org,
kernel-team@...a.com, dave@...olabs.net,
jonathan.cameron@...wei.com, dave.jiang@...el.com,
alison.schofield@...el.com, vishal.l.verma@...el.com,
dan.j.williams@...el.com
Subject: Re: [PATCH v2 1/3] drivers/cxl: introduce cxl_region_driver field
for cxl_region
On Wed, Jan 21, 2026 at 04:27:23PM -0600, Ira Weiny wrote:
> > enum cxl_region_driver {
> > CXL_REGION_DRIVER_NONE,
> > CXL_REGION_DRIVER_DAX,
> > CXL_REGION_DRIVER_PMEM
> > };
>
> The problem I see with this series is that this is not actually telling
> the user which driver is being used. Only which device is being created.
> Then it is assumed the proper driver attaches.
>
I'm not fully following this comment, apologies.
> >
> > $cat regionN/region_driver
> > [none,dax,pmem]
>
> I feel like this will be more useful when CXL RAM regions can be driven by
> different drivers. I forget right now the rest of your other series. But
> I wonder if the actual driver in drivers/dax/cxl.c (the dax driver) should
> be setting this field?
>
the code in drivers/dax/cxl.c gets called by region code at probe() time
So by the point you get to there, you've already made the decision to
engage the dax_region glue - i.e. this is actually needed to be set
prior to `echo region0 > cxl/drivers/bind`
so the workflow looks like this
- echo region0 > decoder0.0/create_ram_region
- /* do all the decoder programming and commit */
- echo [dax,sysram,...] > region0/region_driver
- echo region0 > cxl/drivers/region/bind
-> region probe() creates dax_region
-> dax_region probe()
-> dax probe()
so region probe() then sees you selected DAX and goes down the dax
rabbit hole.
> Also escaping my memory ATM, is why one can't relate the dax_region to the
> cxl_region in user space already?
>
you can, it's linked in `region0/dax_region` after probe()
> > Auto-regions will either be static sysram (BIOS-onlined) and has no
> > region controller associated with it - or if the SP bit was set a
> > DAX device will be created. This will be discovered at probe time.
>
> This seems like it adds information. Since these BIOS controlled regions
> kind of get 'lost'.
>
It actually just formally encodes: "Auto-regions have only two
reasonable end-points:
auto-SysRAM region: no driver control (it's already online)
EFI_MEMORY_SP : DAX
This is what the current implicit behavior is.
> :-/
>
> I'm not opposed to this idea but I'm worried that this adds to the already
> very implicit nature of the CXL subsystem.
>
Point of clarification:
"All RAM regions will be defaulted to CXL_REGION_DRIVER_DAX"
|
V
"All Auto-regions in a RAM partition will be defaulted to
CXL_REGION_DRIVER_DAX"
Auto (BIOS-configured) regions should be considered evil for exactly this
reason. They make the software model much more difficult to discern.
If a user wants to change an auto-decoder region, they would have to:
1) unbind daxdev
2) unbind dax_region
3) unbind region0
4) echo 'new_driver' > region0/region_driver
5) echo region0 > cxl/drivers/region/bind
Unless we want to change the existing default pattern for auto-regions
to just not auto-probe, which would break existing systems.
> > + /* NONE type is not a valid selection for manually probed regions */
> > + if (sysfs_streq(buf, "dax"))
> > + cxlr->driver = CXL_REGION_DRIVER_DAX;
>
> How does this work? Doesn't this have to create a dax_region device for
> the driver to attach to? That is the equal to CXL_REGION_DRIVER_DAX being
> set at region probe time.
>
This gets read at probe() time essentially to generate dax_region
This is essentially how it "already works" - the information is just
obfuscated by the default nature of:
pmem region? -> nvdimm glue
ram region? -> dax glue
> Hindsight: It seems like this driver should have been called the CXL
> partition driver. As it is triggered by the partitions being found... I
> think. I'm probably really confused right now.
I was trying to say this on the recent dax and CXL calls - the current
region driver (region.c) is really the PARTITION driver, and what i'm
trying to break out is the ACTUAL region driver pieces.
This is why the original series called this a region_controller, because
both region and driver here is just overloaded :[
~Gregory
Powered by blists - more mailing lists