Re: [PATCH 1/6] drivers/cxl: add cxl_memctrl_mode and region->memctrl

[Gregory Price <gourry@gourry.net>]

On Mon, Jan 12, 2026 at 03:10:29PM -0600, Cheatham, Benjamin wrote:
> On 1/12/2026 10:35 AM, Gregory Price wrote:
> > The CXL driver presently hands policy management over to DAX subsystem
> > for sysram regions, which makes building policy around the entire region
> > clunky and at time difficult (e.g. multiple actions to offline and
> > hot-unplug memory reliably).
> > 
> > To support multiple backend controllers for memory regions (for example
> > dax vs direct hotplug), implement a memctrl field in cxl_region allows
> > switching uncomitted regions between different "memory controllers".
> > 
> > CXL_CONTROL_NONE: No selected controller, probe will fail.
> > CXL_CONTROL_AUTO: If memory is already online as SysRAM, no controller
> >                   otherwise register a dax_region
> 
> I think you can streamline this to get rid of CXL_CONTROL_AUTO. If BIOS set up
> the memory you can just set the mode to CXL_CONTROL_NONE, otherwise use CXL_CONTROL_DAX.
> This patch would be a bit less complicated at the very least, and I don't think it
> would require much reworking of later patches.
>

I suppose if it's an auto-region (CXL_REGION_F_AUTO) we can default to
DAX if the SP bit is set (memory is soft-reserved).

I was hoping to quietly convert our systems from the dax driver to the
sysram driver without too much hubub, but this would require our old
systems to implement a udev or chef thing to online the memory after
boot.  Otherwise I can't ditch the MHP_ auto-online flags to
auto-online.

But yeah I won't fight this too much, just need to think about it.  I
can always ditch it and bring auto back later if there's something that
breaks unexpectedly.

> > +	switch (cxlr->memctrl) {
> > +	case CXL_MEMCTRL_AUTO:
> > +		/*
> > +		 * The region can not be manged by CXL if any portion of
> 
> s/manged/managed. May as well fix it since it's being moved.

ack.

> > +		 * it is already online as 'System RAM'
> > +		 */
> > +		if (walk_iomem_res_desc(IORES_DESC_NONE,
> > +					IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY,
> > +					p->res->start, p->res->end, cxlr,
> > +					is_system_ram) > 0)
> > +			return 0;
> > +		return devm_cxl_add_dax_region(cxlr);
> 
> If you take my suggestion at the top about removing MEMCTRL_AUTO, this case become
> the MEMCTRL_DAX case.
>

ack.

> > +	default:
> > +		desc = "";
> 
> Nit: I would prefer this say "none" instead of being blank to match the code.

reasonable

> > @@ -3579,6 +3558,9 @@ static int __construct_region(struct cxl_region *cxlr,
> >  
> >  	set_bit(CXL_REGION_F_AUTO, &cxlr->flags);
> >  
> > +	/* Auto-regions will either be static sysram (onlined by BIOS) or DAX */
> > +	cxlr->memctrl = CXL_MEMCTRL_AUTO;
> 
> And this would be set to CXL_MEM_CTRL_DAX instead.
> 

Mmmm, hm.  I guess if the region is already online, we'll revert this to
none at probe time, so yeah that should work.

There may be some kind of management we want to add to auto-regions
onlined by BIOS (e.g. no SP bit), but we can add AUTO in if we ever
discover that use-case and argue for it then.

Thanks,
~Gregory