lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20260129191039.kodgkum445ws5rhs@offworld>
Date: Thu, 29 Jan 2026 11:10:39 -0800
From: Davidlohr Bueso <dave@...olabs.net>
To: Gregory Price <gourry@...rry.net>
Cc: linux-cxl@...r.kernel.org, linux-kernel@...r.kernel.org,
	kernel-team@...a.com, jonathan.cameron@...wei.com,
	dave.jiang@...el.com, alison.schofield@...el.com,
	vishal.l.verma@...el.com, ira.weiny@...el.com,
	dan.j.williams@...el.com, anisa.su@...sung.com,
	dongjoo.seo1@...sung.com, s.neeraj@...sung.com
Subject: Re: [PATCH v2 1/3] drivers/cxl: introduce cxl_region_driver field
 for cxl_region

On Tue, 13 Jan 2026, 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".

I think your overall idea of these region driver backends to isolate use
cases makes a lot of sense and helps untangle things. This series is a good
step and nicely moves ad-hoc complexity out of region.c.

>
>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.

s/CXL_CONTROL_DAX/CXL_REGION_DRIVER_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: Davidlohr Bueso <dave@...olabs.net>

>---
> 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
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ