[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <21bdac12-135a-4420-a1df-98b41269d74f@intel.com>
Date: Tue, 13 Jan 2026 14:16:58 -0700
From: Dave Jiang <dave.jiang@...el.com>
To: Gregory Price <gourry@...rry.net>, linux-cxl@...r.kernel.org
Cc: linux-kernel@...r.kernel.org, kernel-team@...a.com, dave@...olabs.net,
jonathan.cameron@...wei.com, alison.schofield@...el.com,
vishal.l.verma@...el.com, ira.weiny@...el.com, dan.j.williams@...el.com
Subject: Re: [PATCH v2 1/3] drivers/cxl: introduce cxl_region_driver field for
cxl_region
On 1/13/26 1:21 PM, 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@...rry.net>
Reviewed-by: Dave Jiang <dave.jiang@...el.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;
Powered by blists - more mailing lists