Re: [PATCH v2 1/3] drivers/cxl: introduce cxl_region_driver field for cxl_region

["Fabio M. De Francesco" <fabio.m.de.francesco@linux.intel.com>]

On Tuesday, January 13, 2026 9:21:36 PM Central European Standard Time Gregory Price wrote:
> The CXL driver presently has 3 modes of managing a cxl_region:
>   - no specific driver (bios-onlined SystemRAM)
>   - dax_region (all other RAM regions, for now)
>   - pmem_region (all PMEM regions)
> 
> Formalize these into specific "region drivers".
> 
> enum cxl_region_driver {
> 	CXL_REGION_DRIVER_NONE,
> 	CXL_REGION_DRIVER_DAX,
> 	CXL_REGION_DRIVER_PMEM
> };
> 
> $cat regionN/region_driver
> [none,dax,pmem]
> 
> The intent is to clarify how to to add additional drivers (sysram,
> dynamic_capacity, etc) in the future, and to allow switching the
> driver selection via a sysfs entry `regionN/region_driver`.
> 
> All RAM regions will be defaulted to CXL_CONTROL_DAX.
> 
> 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.
> 
> Signed-off-by: Gregory Price <gourry@gourry.net>
> ---

Reviewed-by: Fabio M. De Francesco <fabio.m.de.francesco@linux.intel.com>

>  drivers/cxl/core/region.c | 113 ++++++++++++++++++++++++++++++--------
>  drivers/cxl/cxl.h         |   8 +++
>  2 files changed, 98 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index ae899f68551f..f8262d2169ea 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -626,6 +626,57 @@ static ssize_t mode_show(struct device *dev, struct device_attribute *attr,
>  }
>  static DEVICE_ATTR_RO(mode);
>  
> +static ssize_t region_driver_show(struct device *dev,
> +				  struct device_attribute *attr,
> +				  char *buf)
> +{
> +	struct cxl_region *cxlr = to_cxl_region(dev);
> +	const char *desc;
> +
> +	switch (cxlr->driver) {
> +	case CXL_REGION_DRIVER_NONE:
> +		desc = "none";
> +		break;
> +	case CXL_REGION_DRIVER_DAX:
> +		desc = "dax";
> +		break;
> +	case CXL_REGION_DRIVER_PMEM:
> +		desc = "pmem";
> +		break;
> +	}
> +
> +	return sysfs_emit(buf, "%s\n", desc);
> +}
> +
> +static ssize_t region_driver_store(struct device *dev,
> +				   struct device_attribute *attr,
> +				   const char *buf, size_t len)
> +{
> +	struct cxl_region *cxlr = to_cxl_region(dev);
> +	struct cxl_region_params *p = &cxlr->params;
> +	int rc;
> +
> +	ACQUIRE(rwsem_write_kill, rwsem)(&cxl_rwsem.region);
> +	if ((rc = ACQUIRE_ERR(rwsem_write_kill, &rwsem)))
> +		return rc;
> +
> +	if (p->state >= CXL_CONFIG_COMMIT)
> +		return -EBUSY;
> +
> +	/* PMEM drivers cannot be changed */
> +	if (cxlr->mode == CXL_PARTMODE_PMEM)
> +		return -EBUSY;
> +
> +	/* NONE type is not a valid selection for manually probed regions */
> +	if (sysfs_streq(buf, "dax"))
> +		cxlr->driver = CXL_REGION_DRIVER_DAX;
> +	else
> +		return -EINVAL;
> +
> +	return len;
> +}
> +static DEVICE_ATTR_RW(region_driver);
> +
>  static int alloc_hpa(struct cxl_region *cxlr, resource_size_t size)
>  {
>  	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr->dev.parent);
> @@ -772,6 +823,7 @@ static struct attribute *cxl_region_attrs[] = {
>  	&dev_attr_size.attr,
>  	&dev_attr_mode.attr,
>  	&dev_attr_extended_linear_cache_size.attr,
> +	&dev_attr_region_driver.attr,
>  	NULL,
>  };
>  
> @@ -2599,6 +2651,16 @@ static struct cxl_region *devm_cxl_add_region(struct cxl_root_decoder *cxlrd,
>  	cxlr->mode = mode;
>  	cxlr->type = type;
>  
> +	/*
> +	 * PMEM regions only have 1 driver: pmem_region
> +	 * RAM regions default to DAX, but if the memory is already onlined by
> +	 * BIOS as 'System-RAM', the DAX driver will be dropped during probe.
> +	 */
> +	if (mode == CXL_PARTMODE_PMEM)
> +		cxlr->driver = CXL_REGION_DRIVER_PMEM;
> +	else
> +		cxlr->driver = CXL_REGION_DRIVER_DAX;
> +
>  	dev = &cxlr->dev;
>  	rc = dev_set_name(dev, "region%d", id);
>  	if (rc)
> @@ -3951,33 +4013,38 @@ static int cxl_region_probe(struct device *dev)
>  			return rc;
>  	}
>  
> -	switch (cxlr->mode) {
> -	case CXL_PARTMODE_PMEM:
> -		rc = devm_cxl_region_edac_register(cxlr);
> -		if (rc)
> -			dev_dbg(&cxlr->dev, "CXL EDAC registration for region_id=%d failed\n",
> -				cxlr->id);
> +	if (cxlr->mode > CXL_PARTMODE_PMEM) {
> +		dev_dbg(&cxlr->dev, "unsupported region mode: %d\n",
> +			cxlr->mode);
> +		return -ENXIO;
> +	}
>  
> -		return devm_cxl_add_pmem_region(cxlr);
> -	case CXL_PARTMODE_RAM:
> -		rc = devm_cxl_region_edac_register(cxlr);
> -		if (rc)
> -			dev_dbg(&cxlr->dev, "CXL EDAC registration for region_id=%d failed\n",
> -				cxlr->id);
> +	/*
> +	 * The region can not be managed by CXL if any portion of
> +	 * 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) {
> +		cxlr->driver = CXL_REGION_DRIVER_NONE;
> +		return 0;
> +	}
>  
> -		/*
> -		 * The region can not be manged by CXL if any portion of
> -		 * 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;
> +	rc = devm_cxl_region_edac_register(cxlr);
> +	dev_dbg(&cxlr->dev, "CXL EDAC registration for region_id=%d %s\n",
> +			cxlr->id, rc ? "failed" : "succeeded");
> +
> +	switch (cxlr->driver) {
> +	case CXL_REGION_DRIVER_NONE:
> +		return 0;
> +	case CXL_REGION_DRIVER_DAX:
>  		return devm_cxl_add_dax_region(cxlr);
> +	case CXL_REGION_DRIVER_PMEM:
> +		return devm_cxl_add_pmem_region(cxlr);
>  	default:
> -		dev_dbg(&cxlr->dev, "unsupported region mode: %d\n",
> -			cxlr->mode);
> +		dev_dbg(&cxlr->dev, "unsupported region driver: %d\n",
> +			cxlr->driver);
>  		return -ENXIO;
>  	}
>  }
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index ba17fa86d249..e8256099de29 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -502,6 +502,13 @@ enum cxl_partition_mode {
>  	CXL_PARTMODE_PMEM,
>  };
>  
> +
> +enum cxl_region_driver {
> +	CXL_REGION_DRIVER_NONE,
> +	CXL_REGION_DRIVER_DAX,
> +	CXL_REGION_DRIVER_PMEM,
> +};
> +
>  /*
>   * Indicate whether this region has been assembled by autodetection or
>   * userspace assembly. Prevent endpoint decoders outside of automatic
> @@ -543,6 +550,7 @@ struct cxl_region {
>  	struct device dev;
>  	int id;
>  	enum cxl_partition_mode mode;
> +	enum cxl_region_driver driver;
>  	enum cxl_decoder_type type;
>  	struct cxl_nvdimm_bridge *cxl_nvb;
>  	struct cxl_pmem_region *cxlr_pmem;
> -- 
> 2.52.0
> 
> 
>