Re: [PATCH 8/9] cxl/core: Add dax_kmem_region and sysram_region drivers

Cheatham, Benjamin
2026-02-02T17:02:47Z

On 1/30/2026 4:12 PM, Gregory Price wrote:
> On Fri, Jan 30, 2026 at 03:27:12PM -0600, Cheatham, Benjamin wrote:
>> On 1/29/2026 3:04 PM, Gregory Price wrote:
>>> In the current kmem driver binding process, the only way for users
>>> to define hotplug policy is via a build-time option, or by not
>>> onlining memory by default and setting each individual memory block
>>> online after hotplug occurs. We can solve this with a configuration
>>> step between region-probe and dax-probe.
>>>
>>> Add the infrastructure for a two-stage driver binding for kmem-mode
>>> dax regions. The cxl_dax_kmem_region driver probes cxl_sysram_region
>>> devices and creates cxl_dax_region with dax_driver=kmem.
>>>
>>> This creates an interposition step where users can configure policy.
>>>
>>> Device hierarchy:
>>> region0 -> sysram_region0 -> dax_region0 -> dax0.0
>>
>> This technically comes up in the devdax_region driver patch first, but I noticed it here
>> so this is where I'm putting it:
>>
>> I like the idea here, but the implementation is all off. Firstly, devm_cxl_add_sysram_region()
>> is never called outside of sysram_region_driver::probe(), so I'm not sure how they ever get
>> added to the system (same with devdax regions).
>>
>> Second, there's this weird pattern of adding sub-region (sysram, devdax, etc.) devices being added
>> inside of the sub-region driver probe. I would expect the devices are added then the probe function
>> is called.
>
> I originally tried doing with region0/region_driver, but that design
> pattern is also confusing - and it creates differently bad patterns.
>
> echo region0 > decoder0.0/create_ram_region -> creates region0
>
> # Current pattern
> echo region > driver/region/probe /* auto-region behavior */
>
> # region_driver attribute pattern
> echo "sysram" > region0/region_driver
> echo region0 > driver/region/probe /* uses sysram region driver */
>
> https://lore.kernel.org/linux-cxl/20260113202138.3021093-1-gourry@gourry.net/
>
> Ira pointed out that this design makes the "implicit" design of the
> driver worse. The user doesn't actually know what driver is being used
> under the hood - it just knows something is being used.
>
> This at least makes it explicit which driver is being used - and splits
> the uses-case logic up into discrete drivers (dax users don't have to
> worry about sysram users breaking their stuff).
>
> If it makes more sense, you could swap the ordering of the names
>
> echo region0 > region/bind
> echo region0 > region_sysram/bind
> echo region0 > region_daxdev/bind
> echo region0 > region_dax_kmem/bind
> echo region0 > region_pony/bind
>
> ---
>
> The underlying issue is that region::probe() is trying to be a
> god-function for every possible use case, and hiding the use case
> behind an attribute vs a driver is not good.
>
> (also the default behavior for region::probe() in an otherwise
> unconfigured region is required for backwards compatibility)

Ok, that makes sense. I think I just got lost in the sauce while looking at this last
week and this explanation helped a lot.>
>> What I think should be going on here (and correct me if I'm wrong) is:
>> 1. a cxl_region device is added to the system
>> 2. cxl_region::probe() is called on said device (one in cxl/core/region.c)
>> 3. Said probe function figures out the device is a dax_region or whatever else and creates that type of region device
>> (i.e. cxl_region::probe() -> device_add(&cxl_sysram_device))
>> 4. if the device's dax driver type is DAXDRV_DEVICE_TYPE it gets sent to the daxdev_region driver
>> 5a. if the device's dax driver type is DAXDRV_KMEM_TYPE it gets sent to the sysram_region driver which holds it until
>> the online_type is set
>> 5b. Once the online_type is set, the device is forwarded to the dax_kmem_region driver? Not sure on this part
>>
>> What seems to be happening is that the cxl_region is added, all of these region drivers try
>> to bind to it since they all use the same device id (CXL_DEVICE_REGION) and the correct one is
>> figured out by magic? I'm somewhat confused at this point :/.
>>
>
> For auto-regions:
> region_probe() eats it and you get the default behavior.
>
> For non-auto regions:
> create_x_region generates an un-configured region and fails to probe
> until the user commits it and probes it.

I think this was the source of my misunderstanding. I was trying to understand how it
works for auto regions when it's never meant to apply to them.

Sorry if this is a stupid question, but what stops auto regions from binding to the
sysram/dax region drivers? They all bind to region devices, so I assume there's something
keeping them from binding before the core region driver gets a chance.

Thanks,
Ben
>
> auto-regions are evil and should be discouraged.
>
> ~Gregory