[PATCH 0/6] cxl: Initialization reworks in support Soft Reserve Recovery and Accelerator Memory

[PATCH 0/6] cxl: Initialization reworks in support Soft Reserve Recovery and Accelerator Memory

[Dan Williams]

The CXL subsystem is modular. That modularity is a benefit for
separation of concerns and testing. It is generally appropriate for this
class of devices that support hotplug and can dynamically add a CXL
personality alongside their PCI personality. However, a cost of modules
is ambiguity about when devices (cxl_memdevs, cxl_ports, cxl_regions)
have had a chance to attach to their corresponding drivers on
@cxl_bus_type.

This problem of not being able to reliably determine when a device has
had a chance to attach to its driver vs still waiting for the module to
load, is a common problem for the "Soft Reserve Recovery" [1], and
"Accelerator Memory" [2] enabling efforts.

For "Soft Reserve Recovery" it wants to use wait_for_device_probe() as a
sync point for when CXL devices present at boot have had a chance to
attach to the cxl_pci driver (generic CXL memory expansion class
driver). That breaks down if wait_for_device_probe() only flushes PCI
device probe, but not the cxl_mem_probe() of the cxl_memdev that
cxl_pci_probe() creates.

For "Accelerator Memory", the driver is not cxl_pci, but any potential
PCI driver that wants to use the devm_cxl_add_memdev() ABI to attach to
the CXL memory domain. Those drivers want to know if the CXL link is
live end-to-end (from endpoint, through switches, to the host bridge)
and CXL memory operations are enabled. If not, a CXL accelerator may be
able to fall back to PCI-only operation. Similar to the "Soft Reserve
Memory" it needs to know that the CXL subsystem had a chance to probe
the ancestor topology of the device and let that driver make a
synchronous decision about CXL operation.

In support of those efforts:

* Clean up some resource lifetime issues in the current code
* Move some object creation symbols (devm_cxl_add_memdev() and
  devm_cxl_add_endpoint()) into the cxl_mem.ko and cxl_port.ko objects.
  Implicitly guarantee that cxl_mem_driver and cxl_port_driver have been
  registered prior to any device objects being registered. This is
  preferred over explicit open-coded request_module().
* Use scoped-based-cleanup before adding more resource management in
  devm_cxl_add_memdev()
* Give an accelerator the opportunity to run setup operations in
  cxl_mem_probe() so it can further probe if the CXL configuration matches
  its needs.

Some of these previously appeared on a branch as an RFC [3] and left
"Soft Reserve Recovery" and "Accelerator Memory" to jockey for ordering.
Instead, create a shared topic branch for both of those efforts to
import. The main changes since that RFC are fixing a bug and reducing
the amount of refactoring (which contributed to hiding the bug).

[1]: http://lore.kernel.org/20251120031925.87762-1-Smita.KoralahalliChannabasappa@amd.com
[2]: http://lore.kernel.org/20251119192236.2527305-1-alejandro.lucero-palau@amd.com
[3]: https://git.kernel.org/pub/scm/linux/kernel/git/cxl/cxl.git/log/?h=for-6.18/cxl-probe-order

Dan Williams (6):
  cxl/mem: Fix devm_cxl_memdev_edac_release() confusion
  cxl/mem: Arrange for always-synchronous memdev attach
  cxl/port: Arrange for always synchronous endpoint attach
  cxl/mem: Convert devm_cxl_add_memdev() to scope-based-cleanup
  cxl/mem: Drop @host argument to devm_cxl_add_memdev()
  cxl/mem: Introduce a memdev creation ->probe() operation

 drivers/cxl/Kconfig          |   2 +-
 drivers/cxl/cxl.h            |   2 +
 drivers/cxl/cxlmem.h         |  17 ++++--
 drivers/cxl/core/edac.c      |  64 ++++++++++++---------
 drivers/cxl/core/memdev.c    | 104 ++++++++++++++++++++++++-----------
 drivers/cxl/mem.c            |  69 +++++++++--------------
 drivers/cxl/pci.c            |   2 +-
 drivers/cxl/port.c           |  40 ++++++++++++++
 tools/testing/cxl/test/mem.c |   2 +-
 9 files changed, 192 insertions(+), 110 deletions(-)


base-commit: ea5514e300568cbe8f19431c3e424d4791db8291
-- 
2.51.1

[PATCH 1/6] cxl/mem: Fix devm_cxl_memdev_edac_release() confusion

[Dan Williams]

A device release method is only for undoing allocations on the path to
preparing the device for device_add(). In contrast, devm allocations are
post device_add(), are acquired during / after ->probe() and are released
synchronous with ->remove().

So, a "devm" helper in a "release" method is a clear anti-pattern.

Move this devm release action where it belongs, an action created at edac
object creation time. Otherwise, this leaks resources until
cxl_memdev_release() time which may be long after these xarray and error
record caches have gone idle.

Note, this also fixes up the type of @cxlmd->err_rec_array which needlessly
dropped type-safety.

Fixes: 0b5ccb0de1e2 ("cxl/edac: Support for finding memory operation attributes from the current boot")
Cc: Dave Jiang <dave.jiang@intel.com>
Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Shiju Jose <shiju.jose@huawei.com>
Cc: Alison Schofield <alison.schofield@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/cxl/cxlmem.h      |  5 +--
 drivers/cxl/core/edac.c   | 64 ++++++++++++++++++++++-----------------
 drivers/cxl/core/memdev.c |  1 -
 3 files changed, 38 insertions(+), 32 deletions(-)

diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index 434031a0c1f7..c12ab4fc9512 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -63,7 +63,7 @@ struct cxl_memdev {
 	int depth;
 	u8 scrub_cycle;
 	int scrub_region_id;
-	void *err_rec_array;
+	struct cxl_mem_err_rec *err_rec_array;
 };
 
 static inline struct cxl_memdev *to_cxl_memdev(struct device *dev)
@@ -877,7 +877,6 @@ int devm_cxl_memdev_edac_register(struct cxl_memdev *cxlmd);
 int devm_cxl_region_edac_register(struct cxl_region *cxlr);
 int cxl_store_rec_gen_media(struct cxl_memdev *cxlmd, union cxl_event *evt);
 int cxl_store_rec_dram(struct cxl_memdev *cxlmd, union cxl_event *evt);
-void devm_cxl_memdev_edac_release(struct cxl_memdev *cxlmd);
 #else
 static inline int devm_cxl_memdev_edac_register(struct cxl_memdev *cxlmd)
 { return 0; }
@@ -889,8 +888,6 @@ static inline int cxl_store_rec_gen_media(struct cxl_memdev *cxlmd,
 static inline int cxl_store_rec_dram(struct cxl_memdev *cxlmd,
 				     union cxl_event *evt)
 { return 0; }
-static inline void devm_cxl_memdev_edac_release(struct cxl_memdev *cxlmd)
-{ return; }
 #endif
 
 #ifdef CONFIG_CXL_SUSPEND
diff --git a/drivers/cxl/core/edac.c b/drivers/cxl/core/edac.c
index 79994ca9bc9f..81160260e26b 100644
--- a/drivers/cxl/core/edac.c
+++ b/drivers/cxl/core/edac.c
@@ -1988,6 +1988,40 @@ static int cxl_memdev_soft_ppr_init(struct cxl_memdev *cxlmd,
 	return 0;
 }
 
+static void err_rec_free(void *_cxlmd)
+{
+	struct cxl_memdev *cxlmd = _cxlmd;
+	struct cxl_mem_err_rec *array_rec = cxlmd->err_rec_array;
+	struct cxl_event_gen_media *rec_gen_media;
+	struct cxl_event_dram *rec_dram;
+	unsigned long index;
+
+	cxlmd->err_rec_array = NULL;
+	xa_for_each(&array_rec->rec_dram, index, rec_dram)
+		kfree(rec_dram);
+	xa_destroy(&array_rec->rec_dram);
+
+	xa_for_each(&array_rec->rec_gen_media, index, rec_gen_media)
+		kfree(rec_gen_media);
+	xa_destroy(&array_rec->rec_gen_media);
+	kfree(array_rec);
+}
+
+static int devm_cxl_memdev_setup_err_rec(struct cxl_memdev *cxlmd)
+{
+	struct cxl_mem_err_rec *array_rec =
+		kzalloc(sizeof(*array_rec), GFP_KERNEL);
+
+	if (!array_rec)
+		return -ENOMEM;
+
+	xa_init(&array_rec->rec_gen_media);
+	xa_init(&array_rec->rec_dram);
+	cxlmd->err_rec_array = array_rec;
+
+	return devm_add_action_or_reset(&cxlmd->dev, err_rec_free, cxlmd);
+}
+
 int devm_cxl_memdev_edac_register(struct cxl_memdev *cxlmd)
 {
 	struct edac_dev_feature ras_features[CXL_NR_EDAC_DEV_FEATURES];
@@ -2038,15 +2072,9 @@ int devm_cxl_memdev_edac_register(struct cxl_memdev *cxlmd)
 		}
 
 		if (repair_inst) {
-			struct cxl_mem_err_rec *array_rec =
-				devm_kzalloc(&cxlmd->dev, sizeof(*array_rec),
-					     GFP_KERNEL);
-			if (!array_rec)
-				return -ENOMEM;
-
-			xa_init(&array_rec->rec_gen_media);
-			xa_init(&array_rec->rec_dram);
-			cxlmd->err_rec_array = array_rec;
+			rc = devm_cxl_memdev_setup_err_rec(cxlmd);
+			if (rc)
+				return rc;
 		}
 	}
 
@@ -2088,22 +2116,4 @@ int devm_cxl_region_edac_register(struct cxl_region *cxlr)
 }
 EXPORT_SYMBOL_NS_GPL(devm_cxl_region_edac_register, "CXL");
 
-void devm_cxl_memdev_edac_release(struct cxl_memdev *cxlmd)
-{
-	struct cxl_mem_err_rec *array_rec = cxlmd->err_rec_array;
-	struct cxl_event_gen_media *rec_gen_media;
-	struct cxl_event_dram *rec_dram;
-	unsigned long index;
-
-	if (!IS_ENABLED(CONFIG_CXL_EDAC_MEM_REPAIR) || !array_rec)
-		return;
-
-	xa_for_each(&array_rec->rec_dram, index, rec_dram)
-		kfree(rec_dram);
-	xa_destroy(&array_rec->rec_dram);
 
-	xa_for_each(&array_rec->rec_gen_media, index, rec_gen_media)
-		kfree(rec_gen_media);
-	xa_destroy(&array_rec->rec_gen_media);
-}
-EXPORT_SYMBOL_NS_GPL(devm_cxl_memdev_edac_release, "CXL");
diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
index e370d733e440..4dff7f44d908 100644
--- a/drivers/cxl/core/memdev.c
+++ b/drivers/cxl/core/memdev.c
@@ -27,7 +27,6 @@ static void cxl_memdev_release(struct device *dev)
 	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
 
 	ida_free(&cxl_memdev_ida, cxlmd->id);
-	devm_cxl_memdev_edac_release(cxlmd);
 	kfree(cxlmd);
 }
 
-- 
2.51.1

Re: [PATCH 1/6] cxl/mem: Fix devm_cxl_memdev_edac_release() confusion

[Dave Jiang]

On 12/3/25 7:21 PM, Dan Williams wrote:
> A device release method is only for undoing allocations on the path to
> preparing the device for device_add(). In contrast, devm allocations are
> post device_add(), are acquired during / after ->probe() and are released
> synchronous with ->remove().
> 
> So, a "devm" helper in a "release" method is a clear anti-pattern.
> 
> Move this devm release action where it belongs, an action created at edac
> object creation time. Otherwise, this leaks resources until
> cxl_memdev_release() time which may be long after these xarray and error
> record caches have gone idle.
> 
> Note, this also fixes up the type of @cxlmd->err_rec_array which needlessly
> dropped type-safety.
> 
> Fixes: 0b5ccb0de1e2 ("cxl/edac: Support for finding memory operation attributes from the current boot")
> Cc: Dave Jiang <dave.jiang@intel.com>
> Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Shiju Jose <shiju.jose@huawei.com>
> Cc: Alison Schofield <alison.schofield@intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

This can go into fixes right? Does not look like a dependency for the rest of the series.

> ---
>  drivers/cxl/cxlmem.h      |  5 +--
>  drivers/cxl/core/edac.c   | 64 ++++++++++++++++++++++-----------------
>  drivers/cxl/core/memdev.c |  1 -
>  3 files changed, 38 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index 434031a0c1f7..c12ab4fc9512 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -63,7 +63,7 @@ struct cxl_memdev {
>  	int depth;
>  	u8 scrub_cycle;
>  	int scrub_region_id;
> -	void *err_rec_array;
> +	struct cxl_mem_err_rec *err_rec_array;
>  };
>  
>  static inline struct cxl_memdev *to_cxl_memdev(struct device *dev)
> @@ -877,7 +877,6 @@ int devm_cxl_memdev_edac_register(struct cxl_memdev *cxlmd);
>  int devm_cxl_region_edac_register(struct cxl_region *cxlr);
>  int cxl_store_rec_gen_media(struct cxl_memdev *cxlmd, union cxl_event *evt);
>  int cxl_store_rec_dram(struct cxl_memdev *cxlmd, union cxl_event *evt);
> -void devm_cxl_memdev_edac_release(struct cxl_memdev *cxlmd);
>  #else
>  static inline int devm_cxl_memdev_edac_register(struct cxl_memdev *cxlmd)
>  { return 0; }
> @@ -889,8 +888,6 @@ static inline int cxl_store_rec_gen_media(struct cxl_memdev *cxlmd,
>  static inline int cxl_store_rec_dram(struct cxl_memdev *cxlmd,
>  				     union cxl_event *evt)
>  { return 0; }
> -static inline void devm_cxl_memdev_edac_release(struct cxl_memdev *cxlmd)
> -{ return; }
>  #endif
>  
>  #ifdef CONFIG_CXL_SUSPEND
> diff --git a/drivers/cxl/core/edac.c b/drivers/cxl/core/edac.c
> index 79994ca9bc9f..81160260e26b 100644
> --- a/drivers/cxl/core/edac.c
> +++ b/drivers/cxl/core/edac.c
> @@ -1988,6 +1988,40 @@ static int cxl_memdev_soft_ppr_init(struct cxl_memdev *cxlmd,
>  	return 0;
>  }
>  
> +static void err_rec_free(void *_cxlmd)
> +{
> +	struct cxl_memdev *cxlmd = _cxlmd;
> +	struct cxl_mem_err_rec *array_rec = cxlmd->err_rec_array;
> +	struct cxl_event_gen_media *rec_gen_media;
> +	struct cxl_event_dram *rec_dram;
> +	unsigned long index;
> +
> +	cxlmd->err_rec_array = NULL;
> +	xa_for_each(&array_rec->rec_dram, index, rec_dram)
> +		kfree(rec_dram);
> +	xa_destroy(&array_rec->rec_dram);
> +
> +	xa_for_each(&array_rec->rec_gen_media, index, rec_gen_media)
> +		kfree(rec_gen_media);
> +	xa_destroy(&array_rec->rec_gen_media);
> +	kfree(array_rec);
> +}
> +
> +static int devm_cxl_memdev_setup_err_rec(struct cxl_memdev *cxlmd)
> +{
> +	struct cxl_mem_err_rec *array_rec =
> +		kzalloc(sizeof(*array_rec), GFP_KERNEL);
> +
> +	if (!array_rec)
> +		return -ENOMEM;
> +
> +	xa_init(&array_rec->rec_gen_media);
> +	xa_init(&array_rec->rec_dram);
> +	cxlmd->err_rec_array = array_rec;
> +
> +	return devm_add_action_or_reset(&cxlmd->dev, err_rec_free, cxlmd);
> +}
> +
>  int devm_cxl_memdev_edac_register(struct cxl_memdev *cxlmd)
>  {
>  	struct edac_dev_feature ras_features[CXL_NR_EDAC_DEV_FEATURES];
> @@ -2038,15 +2072,9 @@ int devm_cxl_memdev_edac_register(struct cxl_memdev *cxlmd)
>  		}
>  
>  		if (repair_inst) {
> -			struct cxl_mem_err_rec *array_rec =
> -				devm_kzalloc(&cxlmd->dev, sizeof(*array_rec),
> -					     GFP_KERNEL);
> -			if (!array_rec)
> -				return -ENOMEM;
> -
> -			xa_init(&array_rec->rec_gen_media);
> -			xa_init(&array_rec->rec_dram);
> -			cxlmd->err_rec_array = array_rec;
> +			rc = devm_cxl_memdev_setup_err_rec(cxlmd);
> +			if (rc)
> +				return rc;
>  		}
>  	}
>  
> @@ -2088,22 +2116,4 @@ int devm_cxl_region_edac_register(struct cxl_region *cxlr)
>  }
>  EXPORT_SYMBOL_NS_GPL(devm_cxl_region_edac_register, "CXL");
>  
> -void devm_cxl_memdev_edac_release(struct cxl_memdev *cxlmd)
> -{
> -	struct cxl_mem_err_rec *array_rec = cxlmd->err_rec_array;
> -	struct cxl_event_gen_media *rec_gen_media;
> -	struct cxl_event_dram *rec_dram;
> -	unsigned long index;
> -
> -	if (!IS_ENABLED(CONFIG_CXL_EDAC_MEM_REPAIR) || !array_rec)
> -		return;
> -
> -	xa_for_each(&array_rec->rec_dram, index, rec_dram)
> -		kfree(rec_dram);
> -	xa_destroy(&array_rec->rec_dram);
>  
> -	xa_for_each(&array_rec->rec_gen_media, index, rec_gen_media)
> -		kfree(rec_gen_media);
> -	xa_destroy(&array_rec->rec_gen_media);
> -}
> -EXPORT_SYMBOL_NS_GPL(devm_cxl_memdev_edac_release, "CXL");
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index e370d733e440..4dff7f44d908 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -27,7 +27,6 @@ static void cxl_memdev_release(struct device *dev)
>  	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
>  
>  	ida_free(&cxl_memdev_ida, cxlmd->id);
> -	devm_cxl_memdev_edac_release(cxlmd);
>  	kfree(cxlmd);
>  }
>  

Re: [PATCH 1/6] cxl/mem: Fix devm_cxl_memdev_edac_release() confusion

[dan.j.williams]

Dave Jiang wrote:
> 
> 
> On 12/3/25 7:21 PM, Dan Williams wrote:
> > A device release method is only for undoing allocations on the path to
> > preparing the device for device_add(). In contrast, devm allocations are
> > post device_add(), are acquired during / after ->probe() and are released
> > synchronous with ->remove().
> > 
> > So, a "devm" helper in a "release" method is a clear anti-pattern.
> > 
> > Move this devm release action where it belongs, an action created at edac
> > object creation time. Otherwise, this leaks resources until
> > cxl_memdev_release() time which may be long after these xarray and error
> > record caches have gone idle.
> > 
> > Note, this also fixes up the type of @cxlmd->err_rec_array which needlessly
> > dropped type-safety.
> > 
> > Fixes: 0b5ccb0de1e2 ("cxl/edac: Support for finding memory operation attributes from the current boot")
> > Cc: Dave Jiang <dave.jiang@intel.com>
> > Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > Cc: Shiju Jose <shiju.jose@huawei.com>
> > Cc: Alison Schofield <alison.schofield@intel.com>
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> 
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> 
> This can go into fixes right? Does not look like a dependency for the rest of the series.

It could go into fixes, but until someone reports a practical problem
the urgency is low. The impacts of the leak are minor especially on
typical builds where CONFIG_DEBUG_KOBJECT_RELEASE is disabled. The main
motivation to push it along is to interdict any further attempts to make
the "release" vs "devm" mistake.

Re: [PATCH 1/6] cxl/mem: Fix devm_cxl_memdev_edac_release() confusion

[Cheatham, Benjamin]

On 12/3/2025 8:21 PM, Dan Williams wrote:
> A device release method is only for undoing allocations on the path to
> preparing the device for device_add(). In contrast, devm allocations are
> post device_add(), are acquired during / after ->probe() and are released
> synchronous with ->remove().
> 
> So, a "devm" helper in a "release" method is a clear anti-pattern.
> 
> Move this devm release action where it belongs, an action created at edac
> object creation time. Otherwise, this leaks resources until
> cxl_memdev_release() time which may be long after these xarray and error
> record caches have gone idle.
> 
> Note, this also fixes up the type of @cxlmd->err_rec_array which needlessly
> dropped type-safety.
> 
> Fixes: 0b5ccb0de1e2 ("cxl/edac: Support for finding memory operation attributes from the current boot")
> Cc: Dave Jiang <dave.jiang@intel.com>
> Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Shiju Jose <shiju.jose@huawei.com>
> Cc: Alison Schofield <alison.schofield@intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---

Reviewed-by: Ben Cheatham <benjamin.cheatham@amd.com>

Re: [PATCH 1/6] cxl/mem: Fix devm_cxl_memdev_edac_release() confusion

[Alison Schofield]

On Wed, Dec 03, 2025 at 06:21:31PM -0800, Dan Williams wrote:
> A device release method is only for undoing allocations on the path to
> preparing the device for device_add(). In contrast, devm allocations are
> post device_add(), are acquired during / after ->probe() and are released
> synchronous with ->remove().
> 
> So, a "devm" helper in a "release" method is a clear anti-pattern.
> 
> Move this devm release action where it belongs, an action created at edac
> object creation time. Otherwise, this leaks resources until
> cxl_memdev_release() time which may be long after these xarray and error
> record caches have gone idle.
> 
> Note, this also fixes up the type of @cxlmd->err_rec_array which needlessly
> dropped type-safety.
> 
> Fixes: 0b5ccb0de1e2 ("cxl/edac: Support for finding memory operation attributes from the current boot")
> Cc: Dave Jiang <dave.jiang@intel.com>
> Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Shiju Jose <shiju.jose@huawei.com>
> Cc: Alison Schofield <alison.schofield@intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Reviewed-by: Alison Schofield <alison.schofield@intel.com>

Re: [PATCH 1/6] cxl/mem: Fix devm_cxl_memdev_edac_release() confusion

[Alejandro Lucero Palau]

On 12/4/25 02:21, Dan Williams wrote:
> A device release method is only for undoing allocations on the path to
> preparing the device for device_add(). In contrast, devm allocations are


Should this not be referencing to device_del() instead?


> post device_add(), are acquired during / after ->probe() and are released
> synchronous with ->remove().
>
> So, a "devm" helper in a "release" method is a clear anti-pattern.
>
> Move this devm release action where it belongs, an action created at edac
> object creation time. Otherwise, this leaks resources until
> cxl_memdev_release() time which may be long after these xarray and error
> record caches have gone idle.
>
> Note, this also fixes up the type of @cxlmd->err_rec_array which needlessly
> dropped type-safety.
>
> Fixes: 0b5ccb0de1e2 ("cxl/edac: Support for finding memory operation attributes from the current boot")
> Cc: Dave Jiang <dave.jiang@intel.com>
> Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Shiju Jose <shiju.jose@huawei.com>
> Cc: Alison Schofield <alison.schofield@intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>   drivers/cxl/cxlmem.h      |  5 +--
>   drivers/cxl/core/edac.c   | 64 ++++++++++++++++++++++-----------------
>   drivers/cxl/core/memdev.c |  1 -
>   3 files changed, 38 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index 434031a0c1f7..c12ab4fc9512 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -63,7 +63,7 @@ struct cxl_memdev {
>   	int depth;
>   	u8 scrub_cycle;
>   	int scrub_region_id;
> -	void *err_rec_array;
> +	struct cxl_mem_err_rec *err_rec_array;
>   };
>   
>   static inline struct cxl_memdev *to_cxl_memdev(struct device *dev)
> @@ -877,7 +877,6 @@ int devm_cxl_memdev_edac_register(struct cxl_memdev *cxlmd);
>   int devm_cxl_region_edac_register(struct cxl_region *cxlr);
>   int cxl_store_rec_gen_media(struct cxl_memdev *cxlmd, union cxl_event *evt);
>   int cxl_store_rec_dram(struct cxl_memdev *cxlmd, union cxl_event *evt);
> -void devm_cxl_memdev_edac_release(struct cxl_memdev *cxlmd);
>   #else
>   static inline int devm_cxl_memdev_edac_register(struct cxl_memdev *cxlmd)
>   { return 0; }
> @@ -889,8 +888,6 @@ static inline int cxl_store_rec_gen_media(struct cxl_memdev *cxlmd,
>   static inline int cxl_store_rec_dram(struct cxl_memdev *cxlmd,
>   				     union cxl_event *evt)
>   { return 0; }
> -static inline void devm_cxl_memdev_edac_release(struct cxl_memdev *cxlmd)
> -{ return; }
>   #endif
>   
>   #ifdef CONFIG_CXL_SUSPEND
> diff --git a/drivers/cxl/core/edac.c b/drivers/cxl/core/edac.c
> index 79994ca9bc9f..81160260e26b 100644
> --- a/drivers/cxl/core/edac.c
> +++ b/drivers/cxl/core/edac.c
> @@ -1988,6 +1988,40 @@ static int cxl_memdev_soft_ppr_init(struct cxl_memdev *cxlmd,
>   	return 0;
>   }
>   
> +static void err_rec_free(void *_cxlmd)
> +{
> +	struct cxl_memdev *cxlmd = _cxlmd;
> +	struct cxl_mem_err_rec *array_rec = cxlmd->err_rec_array;
> +	struct cxl_event_gen_media *rec_gen_media;
> +	struct cxl_event_dram *rec_dram;
> +	unsigned long index;
> +
> +	cxlmd->err_rec_array = NULL;
> +	xa_for_each(&array_rec->rec_dram, index, rec_dram)
> +		kfree(rec_dram);
> +	xa_destroy(&array_rec->rec_dram);
> +
> +	xa_for_each(&array_rec->rec_gen_media, index, rec_gen_media)
> +		kfree(rec_gen_media);
> +	xa_destroy(&array_rec->rec_gen_media);
> +	kfree(array_rec);
> +}
> +
> +static int devm_cxl_memdev_setup_err_rec(struct cxl_memdev *cxlmd)
> +{
> +	struct cxl_mem_err_rec *array_rec =
> +		kzalloc(sizeof(*array_rec), GFP_KERNEL);
> +
> +	if (!array_rec)
> +		return -ENOMEM;
> +
> +	xa_init(&array_rec->rec_gen_media);
> +	xa_init(&array_rec->rec_dram);
> +	cxlmd->err_rec_array = array_rec;
> +
> +	return devm_add_action_or_reset(&cxlmd->dev, err_rec_free, cxlmd);
> +}
> +
>   int devm_cxl_memdev_edac_register(struct cxl_memdev *cxlmd)
>   {
>   	struct edac_dev_feature ras_features[CXL_NR_EDAC_DEV_FEATURES];
> @@ -2038,15 +2072,9 @@ int devm_cxl_memdev_edac_register(struct cxl_memdev *cxlmd)
>   		}
>   
>   		if (repair_inst) {
> -			struct cxl_mem_err_rec *array_rec =
> -				devm_kzalloc(&cxlmd->dev, sizeof(*array_rec),
> -					     GFP_KERNEL);
> -			if (!array_rec)
> -				return -ENOMEM;
> -
> -			xa_init(&array_rec->rec_gen_media);
> -			xa_init(&array_rec->rec_dram);
> -			cxlmd->err_rec_array = array_rec;
> +			rc = devm_cxl_memdev_setup_err_rec(cxlmd);
> +			if (rc)
> +				return rc;
>   		}
>   	}
>   
> @@ -2088,22 +2116,4 @@ int devm_cxl_region_edac_register(struct cxl_region *cxlr)
>   }
>   EXPORT_SYMBOL_NS_GPL(devm_cxl_region_edac_register, "CXL");
>   
> -void devm_cxl_memdev_edac_release(struct cxl_memdev *cxlmd)
> -{
> -	struct cxl_mem_err_rec *array_rec = cxlmd->err_rec_array;
> -	struct cxl_event_gen_media *rec_gen_media;
> -	struct cxl_event_dram *rec_dram;
> -	unsigned long index;
> -
> -	if (!IS_ENABLED(CONFIG_CXL_EDAC_MEM_REPAIR) || !array_rec)
> -		return;
> -
> -	xa_for_each(&array_rec->rec_dram, index, rec_dram)
> -		kfree(rec_dram);
> -	xa_destroy(&array_rec->rec_dram);
>   
> -	xa_for_each(&array_rec->rec_gen_media, index, rec_gen_media)
> -		kfree(rec_gen_media);
> -	xa_destroy(&array_rec->rec_gen_media);
> -}
> -EXPORT_SYMBOL_NS_GPL(devm_cxl_memdev_edac_release, "CXL");
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index e370d733e440..4dff7f44d908 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -27,7 +27,6 @@ static void cxl_memdev_release(struct device *dev)
>   	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
>   
>   	ida_free(&cxl_memdev_ida, cxlmd->id);
> -	devm_cxl_memdev_edac_release(cxlmd);
>   	kfree(cxlmd);
>   }
>   

Re: [PATCH 1/6] cxl/mem: Fix devm_cxl_memdev_edac_release() confusion

[dan.j.williams]

Alejandro Lucero Palau wrote:
> 
> On 12/4/25 02:21, Dan Williams wrote:
> > A device release method is only for undoing allocations on the path to
> > preparing the device for device_add(). In contrast, devm allocations are
> 
> 
> Should this not be referencing to device_del() instead?

No, device_del() is not involved in what this patch is addressing. Prior
to device_add() the container of that device is allocated and acquires
some resources. device_add() and device_del() are only registering /
unregistering the device with the device-core. The release method undoes
any resource acquisition done *prior* to device_add().

RE: [PATCH 1/6] cxl/mem: Fix devm_cxl_memdev_edac_release() confusion

[Shiju Jose]

>-----Original Message-----
>From: Dan Williams <dan.j.williams@intel.com>
>Sent: 04 December 2025 02:22
>To: dave.jiang@intel.com
>Cc: linux-cxl@vger.kernel.org; linux-kernel@vger.kernel.org;
>Smita.KoralahalliChannabasappa@amd.com; alison.schofield@intel.com;
>terry.bowman@amd.com; alejandro.lucero-palau@amd.com; linux-
>pci@vger.kernel.org; Jonathan Cameron <jonathan.cameron@huawei.com>;
>Shiju Jose <shiju.jose@huawei.com>
>Subject: [PATCH 1/6] cxl/mem: Fix devm_cxl_memdev_edac_release() confusion
>
>A device release method is only for undoing allocations on the path to preparing
>the device for device_add(). In contrast, devm allocations are post device_add(),
>are acquired during / after ->probe() and are released synchronous with -
>>remove().
>
>So, a "devm" helper in a "release" method is a clear anti-pattern.
>
>Move this devm release action where it belongs, an action created at edac object
>creation time. Otherwise, this leaks resources until
>cxl_memdev_release() time which may be long after these xarray and error
>record caches have gone idle.
>
>Note, this also fixes up the type of @cxlmd->err_rec_array which needlessly
>dropped type-safety.
>
>Fixes: 0b5ccb0de1e2 ("cxl/edac: Support for finding memory operation
>attributes from the current boot")
>Cc: Dave Jiang <dave.jiang@intel.com>
>Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>Cc: Shiju Jose <shiju.jose@huawei.com>
>Cc: Alison Schofield <alison.schofield@intel.com>
>Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Tested-by: Shiju Jose <shiju.jose@huawei.com>
Reviewed-by: Shiju Jose <shiju.jose@huawei.com>
>---
> drivers/cxl/cxlmem.h      |  5 +--
> drivers/cxl/core/edac.c   | 64 ++++++++++++++++++++++-----------------
> drivers/cxl/core/memdev.c |  1 -
> 3 files changed, 38 insertions(+), 32 deletions(-)
>
>diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h index
>434031a0c1f7..c12ab4fc9512 100644
>--- a/drivers/cxl/cxlmem.h
>+++ b/drivers/cxl/cxlmem.h
>@@ -63,7 +63,7 @@ struct cxl_memdev {
> 	int depth;
> 	u8 scrub_cycle;
> 	int scrub_region_id;
>-	void *err_rec_array;
>+	struct cxl_mem_err_rec *err_rec_array;
> };
>
> static inline struct cxl_memdev *to_cxl_memdev(struct device *dev) @@ -877,7
>+877,6 @@ int devm_cxl_memdev_edac_register(struct cxl_memdev *cxlmd);
>int devm_cxl_region_edac_register(struct cxl_region *cxlr);  int
>cxl_store_rec_gen_media(struct cxl_memdev *cxlmd, union cxl_event *evt);  int
>cxl_store_rec_dram(struct cxl_memdev *cxlmd, union cxl_event *evt); -void
>devm_cxl_memdev_edac_release(struct cxl_memdev *cxlmd);  #else  static
>inline int devm_cxl_memdev_edac_register(struct cxl_memdev *cxlmd)  { return
>0; } @@ -889,8 +888,6 @@ static inline int cxl_store_rec_gen_media(struct
>cxl_memdev *cxlmd,  static inline int cxl_store_rec_dram(struct cxl_memdev
>*cxlmd,
> 				     union cxl_event *evt)
> { return 0; }
>-static inline void devm_cxl_memdev_edac_release(struct cxl_memdev *cxlmd) -
>{ return; }  #endif
>
> #ifdef CONFIG_CXL_SUSPEND
>diff --git a/drivers/cxl/core/edac.c b/drivers/cxl/core/edac.c index
>79994ca9bc9f..81160260e26b 100644
>--- a/drivers/cxl/core/edac.c
>+++ b/drivers/cxl/core/edac.c
>@@ -1988,6 +1988,40 @@ static int cxl_memdev_soft_ppr_init(struct
>cxl_memdev *cxlmd,
> 	return 0;
> }
>
>+static void err_rec_free(void *_cxlmd)
>+{
>+	struct cxl_memdev *cxlmd = _cxlmd;
>+	struct cxl_mem_err_rec *array_rec = cxlmd->err_rec_array;
>+	struct cxl_event_gen_media *rec_gen_media;
>+	struct cxl_event_dram *rec_dram;
>+	unsigned long index;
>+
>+	cxlmd->err_rec_array = NULL;
>+	xa_for_each(&array_rec->rec_dram, index, rec_dram)
>+		kfree(rec_dram);
>+	xa_destroy(&array_rec->rec_dram);
>+
>+	xa_for_each(&array_rec->rec_gen_media, index, rec_gen_media)
>+		kfree(rec_gen_media);
>+	xa_destroy(&array_rec->rec_gen_media);
>+	kfree(array_rec);
>+}
>+
>+static int devm_cxl_memdev_setup_err_rec(struct cxl_memdev *cxlmd) {
>+	struct cxl_mem_err_rec *array_rec =
>+		kzalloc(sizeof(*array_rec), GFP_KERNEL);
>+
>+	if (!array_rec)
>+		return -ENOMEM;
>+
>+	xa_init(&array_rec->rec_gen_media);
>+	xa_init(&array_rec->rec_dram);
>+	cxlmd->err_rec_array = array_rec;
>+
>+	return devm_add_action_or_reset(&cxlmd->dev, err_rec_free, cxlmd); }
>+
> int devm_cxl_memdev_edac_register(struct cxl_memdev *cxlmd)  {
> 	struct edac_dev_feature ras_features[CXL_NR_EDAC_DEV_FEATURES];
>@@ -2038,15 +2072,9 @@ int devm_cxl_memdev_edac_register(struct
>cxl_memdev *cxlmd)
> 		}
>
> 		if (repair_inst) {
>-			struct cxl_mem_err_rec *array_rec =
>-				devm_kzalloc(&cxlmd->dev, sizeof(*array_rec),
>-					     GFP_KERNEL);
>-			if (!array_rec)
>-				return -ENOMEM;
>-
>-			xa_init(&array_rec->rec_gen_media);
>-			xa_init(&array_rec->rec_dram);
>-			cxlmd->err_rec_array = array_rec;
>+			rc = devm_cxl_memdev_setup_err_rec(cxlmd);
>+			if (rc)
>+				return rc;
> 		}
> 	}
>
>@@ -2088,22 +2116,4 @@ int devm_cxl_region_edac_register(struct
>cxl_region *cxlr)  }  EXPORT_SYMBOL_NS_GPL(devm_cxl_region_edac_register,
>"CXL");
>
>-void devm_cxl_memdev_edac_release(struct cxl_memdev *cxlmd) -{
>-	struct cxl_mem_err_rec *array_rec = cxlmd->err_rec_array;
>-	struct cxl_event_gen_media *rec_gen_media;
>-	struct cxl_event_dram *rec_dram;
>-	unsigned long index;
>-
>-	if (!IS_ENABLED(CONFIG_CXL_EDAC_MEM_REPAIR) || !array_rec)
>-		return;
>-
>-	xa_for_each(&array_rec->rec_dram, index, rec_dram)
>-		kfree(rec_dram);
>-	xa_destroy(&array_rec->rec_dram);
>
>-	xa_for_each(&array_rec->rec_gen_media, index, rec_gen_media)
>-		kfree(rec_gen_media);
>-	xa_destroy(&array_rec->rec_gen_media);
>-}
>-EXPORT_SYMBOL_NS_GPL(devm_cxl_memdev_edac_release, "CXL"); diff --git
>a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c index
>e370d733e440..4dff7f44d908 100644
>--- a/drivers/cxl/core/memdev.c
>+++ b/drivers/cxl/core/memdev.c
>@@ -27,7 +27,6 @@ static void cxl_memdev_release(struct device *dev)
> 	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
>
> 	ida_free(&cxl_memdev_ida, cxlmd->id);
>-	devm_cxl_memdev_edac_release(cxlmd);
> 	kfree(cxlmd);
> }
>
>--
>2.51.1
>

Re: [PATCH 1/6] cxl/mem: Fix devm_cxl_memdev_edac_release() confusion

[Jonathan Cameron]

On Wed,  3 Dec 2025 18:21:31 -0800
Dan Williams <dan.j.williams@intel.com> wrote:

> A device release method is only for undoing allocations on the path to
> preparing the device for device_add(). In contrast, devm allocations are
> post device_add(), are acquired during / after ->probe() and are released
> synchronous with ->remove().
> 
> So, a "devm" helper in a "release" method is a clear anti-pattern.
> 
> Move this devm release action where it belongs, an action created at edac
> object creation time. Otherwise, this leaks resources until
> cxl_memdev_release() time which may be long after these xarray and error
> record caches have gone idle.
 
> Note, this also fixes up the type of @cxlmd->err_rec_array which needlessly
> dropped type-safety.
> 
> Fixes: 0b5ccb0de1e2 ("cxl/edac: Support for finding memory operation attributes from the current boot")
> Cc: Dave Jiang <dave.jiang@intel.com>
> Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Shiju Jose <shiju.jose@huawei.com>
> Cc: Alison Schofield <alison.schofield@intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

This change seems fine to me. The tear down occurs after unregistering the
edac device and as the repair is via sysfs in there we shouldn't have any
left over calls in flight.  Was an odd code pattern. Oops to missing this
in earlier review.

Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>

[PATCH 2/6] cxl/mem: Arrange for always-synchronous memdev attach

[Dan Williams]

In preparation for CXL accelerator drivers that have a hard dependency on
CXL capability initialization, arrange for cxl_mem_probe() to always run
synchronous with the device_add() of cxl_memdev instances. I.e.
cxl_mem_driver registration is always complete before the first memdev
creation event.

At present, cxl_pci does not care about the attach state of the cxl_memdev
because all generic memory expansion functionality can be handled by the
cxl_core. For accelerators, however, that driver needs to perform driver
specific initialization if CXL is available, or execute a fallback to PCIe
only operation.

This synchronous attach guarantee is also needed for Soft Reserve Recovery,
which is an effort that needs to assert that devices have had a chance to
attach before making a go / no-go decision on proceeding with CXL subsystem
initialization.

By moving devm_cxl_add_memdev() to cxl_mem.ko it removes async module
loading as one reason that a memdev may not be attached upon return from
devm_cxl_add_memdev().

Cc: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
Cc: Alejandro Lucero <alucerop@amd.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/cxl/Kconfig       |  2 +-
 drivers/cxl/cxlmem.h      |  2 ++
 drivers/cxl/core/memdev.c | 10 +++++++---
 drivers/cxl/mem.c         | 17 +++++++++++++++++
 4 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig
index 48b7314afdb8..f1361ed6a0d4 100644
--- a/drivers/cxl/Kconfig
+++ b/drivers/cxl/Kconfig
@@ -22,6 +22,7 @@ if CXL_BUS
 config CXL_PCI
 	tristate "PCI manageability"
 	default CXL_BUS
+	select CXL_MEM
 	help
 	  The CXL specification defines a "CXL memory device" sub-class in the
 	  PCI "memory controller" base class of devices. Device's identified by
@@ -89,7 +90,6 @@ config CXL_PMEM
 
 config CXL_MEM
 	tristate "CXL: Memory Expansion"
-	depends on CXL_PCI
 	default CXL_BUS
 	help
 	  The CXL.mem protocol allows a device to act as a provider of "System
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index c12ab4fc9512..012e68acad34 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -95,6 +95,8 @@ static inline bool is_cxl_endpoint(struct cxl_port *port)
 	return is_cxl_memdev(port->uport_dev);
 }
 
+struct cxl_memdev *__devm_cxl_add_memdev(struct device *host,
+					 struct cxl_dev_state *cxlds);
 struct cxl_memdev *devm_cxl_add_memdev(struct device *host,
 				       struct cxl_dev_state *cxlds);
 int devm_cxl_sanitize_setup_notifier(struct device *host,
diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
index 4dff7f44d908..7a4153e1c6a7 100644
--- a/drivers/cxl/core/memdev.c
+++ b/drivers/cxl/core/memdev.c
@@ -1050,8 +1050,12 @@ static const struct file_operations cxl_memdev_fops = {
 	.llseek = noop_llseek,
 };
 
-struct cxl_memdev *devm_cxl_add_memdev(struct device *host,
-				       struct cxl_dev_state *cxlds)
+/*
+ * Core helper for devm_cxl_add_memdev() that wants to both create a device and
+ * assert to the caller that upon return cxl_mem::probe() has been invoked.
+ */
+struct cxl_memdev *__devm_cxl_add_memdev(struct device *host,
+					 struct cxl_dev_state *cxlds)
 {
 	struct cxl_memdev *cxlmd;
 	struct device *dev;
@@ -1093,7 +1097,7 @@ struct cxl_memdev *devm_cxl_add_memdev(struct device *host,
 	put_device(dev);
 	return ERR_PTR(rc);
 }
-EXPORT_SYMBOL_NS_GPL(devm_cxl_add_memdev, "CXL");
+EXPORT_SYMBOL_FOR_MODULES(__devm_cxl_add_memdev, "cxl_mem");
 
 static void sanitize_teardown_notifier(void *data)
 {
diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
index 6e6777b7bafb..55883797ab2d 100644
--- a/drivers/cxl/mem.c
+++ b/drivers/cxl/mem.c
@@ -201,6 +201,22 @@ static int cxl_mem_probe(struct device *dev)
 	return devm_add_action_or_reset(dev, enable_suspend, NULL);
 }
 
+/**
+ * devm_cxl_add_memdev - Add a CXL memory device
+ * @host: devres alloc/release context and parent for the memdev
+ * @cxlds: CXL device state to associate with the memdev
+ *
+ * Upon return the device will have had a chance to attach to the
+ * cxl_mem driver, but may fail if the CXL topology is not ready
+ * (hardware CXL link down, or software platform CXL root not attached)
+ */
+struct cxl_memdev *devm_cxl_add_memdev(struct device *host,
+				       struct cxl_dev_state *cxlds)
+{
+	return __devm_cxl_add_memdev(host, cxlds);
+}
+EXPORT_SYMBOL_NS_GPL(devm_cxl_add_memdev, "CXL");
+
 static ssize_t trigger_poison_list_store(struct device *dev,
 					 struct device_attribute *attr,
 					 const char *buf, size_t len)
@@ -248,6 +264,7 @@ static struct cxl_driver cxl_mem_driver = {
 	.probe = cxl_mem_probe,
 	.id = CXL_DEVICE_MEMORY_EXPANDER,
 	.drv = {
+		.probe_type = PROBE_FORCE_SYNCHRONOUS,
 		.dev_groups = cxl_mem_groups,
 	},
 };
-- 
2.51.1

Re: [PATCH 2/6] cxl/mem: Arrange for always-synchronous memdev attach

[Dave Jiang]

On 12/3/25 7:21 PM, Dan Williams wrote:
> In preparation for CXL accelerator drivers that have a hard dependency on
> CXL capability initialization, arrange for cxl_mem_probe() to always run
> synchronous with the device_add() of cxl_memdev instances. I.e.
> cxl_mem_driver registration is always complete before the first memdev
> creation event.
> 
> At present, cxl_pci does not care about the attach state of the cxl_memdev
> because all generic memory expansion functionality can be handled by the
> cxl_core. For accelerators, however, that driver needs to perform driver
> specific initialization if CXL is available, or execute a fallback to PCIe
> only operation.
> 
> This synchronous attach guarantee is also needed for Soft Reserve Recovery,
> which is an effort that needs to assert that devices have had a chance to
> attach before making a go / no-go decision on proceeding with CXL subsystem
> initialization.
> 
> By moving devm_cxl_add_memdev() to cxl_mem.ko it removes async module
> loading as one reason that a memdev may not be attached upon return from
> devm_cxl_add_memdev().
> 
> Cc: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
> Cc: Alejandro Lucero <alucerop@amd.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/cxl/Kconfig       |  2 +-
>  drivers/cxl/cxlmem.h      |  2 ++
>  drivers/cxl/core/memdev.c | 10 +++++++---
>  drivers/cxl/mem.c         | 17 +++++++++++++++++
>  4 files changed, 27 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig
> index 48b7314afdb8..f1361ed6a0d4 100644
> --- a/drivers/cxl/Kconfig
> +++ b/drivers/cxl/Kconfig
> @@ -22,6 +22,7 @@ if CXL_BUS
>  config CXL_PCI
>  	tristate "PCI manageability"
>  	default CXL_BUS
> +	select CXL_MEM
>  	help
>  	  The CXL specification defines a "CXL memory device" sub-class in the
>  	  PCI "memory controller" base class of devices. Device's identified by
> @@ -89,7 +90,6 @@ config CXL_PMEM
>  
>  config CXL_MEM
>  	tristate "CXL: Memory Expansion"
> -	depends on CXL_PCI
>  	default CXL_BUS
>  	help
>  	  The CXL.mem protocol allows a device to act as a provider of "System
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index c12ab4fc9512..012e68acad34 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -95,6 +95,8 @@ static inline bool is_cxl_endpoint(struct cxl_port *port)
>  	return is_cxl_memdev(port->uport_dev);
>  }
>  
> +struct cxl_memdev *__devm_cxl_add_memdev(struct device *host,
> +					 struct cxl_dev_state *cxlds);
>  struct cxl_memdev *devm_cxl_add_memdev(struct device *host,
>  				       struct cxl_dev_state *cxlds);
>  int devm_cxl_sanitize_setup_notifier(struct device *host,
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index 4dff7f44d908..7a4153e1c6a7 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -1050,8 +1050,12 @@ static const struct file_operations cxl_memdev_fops = {
>  	.llseek = noop_llseek,
>  };
>  
> -struct cxl_memdev *devm_cxl_add_memdev(struct device *host,
> -				       struct cxl_dev_state *cxlds)
> +/*
> + * Core helper for devm_cxl_add_memdev() that wants to both create a device and
> + * assert to the caller that upon return cxl_mem::probe() has been invoked.
> + */
> +struct cxl_memdev *__devm_cxl_add_memdev(struct device *host,
> +					 struct cxl_dev_state *cxlds)
>  {
>  	struct cxl_memdev *cxlmd;
>  	struct device *dev;
> @@ -1093,7 +1097,7 @@ struct cxl_memdev *devm_cxl_add_memdev(struct device *host,
>  	put_device(dev);
>  	return ERR_PTR(rc);
>  }
> -EXPORT_SYMBOL_NS_GPL(devm_cxl_add_memdev, "CXL");
> +EXPORT_SYMBOL_FOR_MODULES(__devm_cxl_add_memdev, "cxl_mem");
>  
>  static void sanitize_teardown_notifier(void *data)
>  {
> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
> index 6e6777b7bafb..55883797ab2d 100644
> --- a/drivers/cxl/mem.c
> +++ b/drivers/cxl/mem.c
> @@ -201,6 +201,22 @@ static int cxl_mem_probe(struct device *dev)
>  	return devm_add_action_or_reset(dev, enable_suspend, NULL);
>  }
>  
> +/**
> + * devm_cxl_add_memdev - Add a CXL memory device
> + * @host: devres alloc/release context and parent for the memdev
> + * @cxlds: CXL device state to associate with the memdev
> + *
> + * Upon return the device will have had a chance to attach to the
> + * cxl_mem driver, but may fail if the CXL topology is not ready
> + * (hardware CXL link down, or software platform CXL root not attached)
> + */
> +struct cxl_memdev *devm_cxl_add_memdev(struct device *host,
> +				       struct cxl_dev_state *cxlds)
> +{
> +	return __devm_cxl_add_memdev(host, cxlds);
> +}
> +EXPORT_SYMBOL_NS_GPL(devm_cxl_add_memdev, "CXL");
> +
>  static ssize_t trigger_poison_list_store(struct device *dev,
>  					 struct device_attribute *attr,
>  					 const char *buf, size_t len)
> @@ -248,6 +264,7 @@ static struct cxl_driver cxl_mem_driver = {
>  	.probe = cxl_mem_probe,
>  	.id = CXL_DEVICE_MEMORY_EXPANDER,
>  	.drv = {
> +		.probe_type = PROBE_FORCE_SYNCHRONOUS,
>  		.dev_groups = cxl_mem_groups,
>  	},
>  };

Re: [PATCH 2/6] cxl/mem: Arrange for always-synchronous memdev attach

[Cheatham, Benjamin]

On 12/3/2025 8:21 PM, Dan Williams wrote:
> In preparation for CXL accelerator drivers that have a hard dependency on
> CXL capability initialization, arrange for cxl_mem_probe() to always run
> synchronous with the device_add() of cxl_memdev instances. I.e.
> cxl_mem_driver registration is always complete before the first memdev
> creation event.
> 
> At present, cxl_pci does not care about the attach state of the cxl_memdev
> because all generic memory expansion functionality can be handled by the
> cxl_core. For accelerators, however, that driver needs to perform driver
> specific initialization if CXL is available, or execute a fallback to PCIe
> only operation.
> 
> This synchronous attach guarantee is also needed for Soft Reserve Recovery,
> which is an effort that needs to assert that devices have had a chance to
> attach before making a go / no-go decision on proceeding with CXL subsystem
> initialization.
> 
> By moving devm_cxl_add_memdev() to cxl_mem.ko it removes async module
> loading as one reason that a memdev may not be attached upon return from
> devm_cxl_add_memdev().
> 
> Cc: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
> Cc: Alejandro Lucero <alucerop@amd.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---

Reviewed-by: Ben Cheatham <benjamin.cheatham@amd.com>

Re: [PATCH 2/6] cxl/mem: Arrange for always-synchronous memdev attach

[Alison Schofield]

On Wed, Dec 03, 2025 at 06:21:32PM -0800, Dan Williams wrote:
> In preparation for CXL accelerator drivers that have a hard dependency on
> CXL capability initialization, arrange for cxl_mem_probe() to always run
> synchronous with the device_add() of cxl_memdev instances. I.e.
> cxl_mem_driver registration is always complete before the first memdev
> creation event.
> 
> At present, cxl_pci does not care about the attach state of the cxl_memdev
> because all generic memory expansion functionality can be handled by the
> cxl_core. For accelerators, however, that driver needs to perform driver
> specific initialization if CXL is available, or execute a fallback to PCIe
> only operation.
> 
> This synchronous attach guarantee is also needed for Soft Reserve Recovery,
> which is an effort that needs to assert that devices have had a chance to
> attach before making a go / no-go decision on proceeding with CXL subsystem
> initialization.
> 
> By moving devm_cxl_add_memdev() to cxl_mem.ko it removes async module
> loading as one reason that a memdev may not be attached upon return from
> devm_cxl_add_memdev().
> 
> Cc: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
> Cc: Alejandro Lucero <alucerop@amd.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

I did test w this set as a whole, but only giving the Tested-by Tag
to ones exercised in my testing

Tested-by: Alison Schofield <alison.schofield@intel.com>
Reviewed-by: Alison Schofield <alison.schofield@intel.com>

Re: [PATCH 2/6] cxl/mem: Arrange for always-synchronous memdev attach

[Jonathan Cameron]

On Wed,  3 Dec 2025 18:21:32 -0800
Dan Williams <dan.j.williams@intel.com> wrote:

> In preparation for CXL accelerator drivers that have a hard dependency on
> CXL capability initialization, arrange for cxl_mem_probe() to always run
> synchronous with the device_add() of cxl_memdev instances. I.e.
> cxl_mem_driver registration is always complete before the first memdev
> creation event.
> 
> At present, cxl_pci does not care about the attach state of the cxl_memdev
> because all generic memory expansion functionality can be handled by the
> cxl_core. For accelerators, however, that driver needs to perform driver
> specific initialization if CXL is available, or execute a fallback to PCIe
> only operation.
> 
> This synchronous attach guarantee is also needed for Soft Reserve Recovery,
> which is an effort that needs to assert that devices have had a chance to
> attach before making a go / no-go decision on proceeding with CXL subsystem
> initialization.
> 
> By moving devm_cxl_add_memdev() to cxl_mem.ko it removes async module
> loading as one reason that a memdev may not be attached upon return from
> devm_cxl_add_memdev().
> 
> Cc: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
> Cc: Alejandro Lucero <alucerop@amd.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Seems fine to me as well. Even independent of the reasons that
drove this patch set I'm keen on the simpler mental model that synchronous
probing allows.

Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>

[PATCH 3/6] cxl/port: Arrange for always synchronous endpoint attach

[Dan Williams]

Make it so that upon return from devm_cxl_add_endpoint() that
cxl_mem_probe() can assume that the endpoint has had a chance to complete
cxl_port_probe().  I.e. cxl_port module loading has completed prior to
device registration.

Delete the MODULE_SOFTDEP() as it is not sufficient for this purpose, but a
hard link-time dependency is reliable. Specifically MODULE_SOFTDEP() does
not guarantee that the module loading has completed prior to the completion
of the current module's init.

Cc: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
Cc: Alejandro Lucero <alucerop@amd.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/cxl/cxl.h  |  2 ++
 drivers/cxl/mem.c  | 43 -------------------------------------------
 drivers/cxl/port.c | 40 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 42 insertions(+), 43 deletions(-)

diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index ba17fa86d249..c796c3db36e0 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -780,6 +780,8 @@ struct cxl_port *devm_cxl_add_port(struct device *host,
 				   struct cxl_dport *parent_dport);
 struct cxl_root *devm_cxl_add_root(struct device *host,
 				   const struct cxl_root_ops *ops);
+int devm_cxl_add_endpoint(struct device *host, struct cxl_memdev *cxlmd,
+			  struct cxl_dport *parent_dport);
 struct cxl_root *find_cxl_root(struct cxl_port *port);
 
 DEFINE_FREE(put_cxl_root, struct cxl_root *, if (_T) put_device(&_T->port.dev))
diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
index 55883797ab2d..d62931526fd4 100644
--- a/drivers/cxl/mem.c
+++ b/drivers/cxl/mem.c
@@ -45,44 +45,6 @@ static int cxl_mem_dpa_show(struct seq_file *file, void *data)
 	return 0;
 }
 
-static int devm_cxl_add_endpoint(struct device *host, struct cxl_memdev *cxlmd,
-				 struct cxl_dport *parent_dport)
-{
-	struct cxl_port *parent_port = parent_dport->port;
-	struct cxl_port *endpoint, *iter, *down;
-	int rc;
-
-	/*
-	 * Now that the path to the root is established record all the
-	 * intervening ports in the chain.
-	 */
-	for (iter = parent_port, down = NULL; !is_cxl_root(iter);
-	     down = iter, iter = to_cxl_port(iter->dev.parent)) {
-		struct cxl_ep *ep;
-
-		ep = cxl_ep_load(iter, cxlmd);
-		ep->next = down;
-	}
-
-	/* Note: endpoint port component registers are derived from @cxlds */
-	endpoint = devm_cxl_add_port(host, &cxlmd->dev, CXL_RESOURCE_NONE,
-				     parent_dport);
-	if (IS_ERR(endpoint))
-		return PTR_ERR(endpoint);
-
-	rc = cxl_endpoint_autoremove(cxlmd, endpoint);
-	if (rc)
-		return rc;
-
-	if (!endpoint->dev.driver) {
-		dev_err(&cxlmd->dev, "%s failed probe\n",
-			dev_name(&endpoint->dev));
-		return -ENXIO;
-	}
-
-	return 0;
-}
-
 static int cxl_debugfs_poison_inject(void *data, u64 dpa)
 {
 	struct cxl_memdev *cxlmd = data;
@@ -275,8 +237,3 @@ MODULE_DESCRIPTION("CXL: Memory Expansion");
 MODULE_LICENSE("GPL v2");
 MODULE_IMPORT_NS("CXL");
 MODULE_ALIAS_CXL(CXL_DEVICE_MEMORY_EXPANDER);
-/*
- * create_endpoint() wants to validate port driver attach immediately after
- * endpoint registration.
- */
-MODULE_SOFTDEP("pre: cxl_port");
diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
index 51c8f2f84717..7937e7e53797 100644
--- a/drivers/cxl/port.c
+++ b/drivers/cxl/port.c
@@ -156,10 +156,50 @@ static struct cxl_driver cxl_port_driver = {
 	.probe = cxl_port_probe,
 	.id = CXL_DEVICE_PORT,
 	.drv = {
+		.probe_type = PROBE_FORCE_SYNCHRONOUS,
 		.dev_groups = cxl_port_attribute_groups,
 	},
 };
 
+int devm_cxl_add_endpoint(struct device *host, struct cxl_memdev *cxlmd,
+			  struct cxl_dport *parent_dport)
+{
+	struct cxl_port *parent_port = parent_dport->port;
+	struct cxl_port *endpoint, *iter, *down;
+	int rc;
+
+	/*
+	 * Now that the path to the root is established record all the
+	 * intervening ports in the chain.
+	 */
+	for (iter = parent_port, down = NULL; !is_cxl_root(iter);
+	     down = iter, iter = to_cxl_port(iter->dev.parent)) {
+		struct cxl_ep *ep;
+
+		ep = cxl_ep_load(iter, cxlmd);
+		ep->next = down;
+	}
+
+	/* Note: endpoint port component registers are derived from @cxlds */
+	endpoint = devm_cxl_add_port(host, &cxlmd->dev, CXL_RESOURCE_NONE,
+				     parent_dport);
+	if (IS_ERR(endpoint))
+		return PTR_ERR(endpoint);
+
+	rc = cxl_endpoint_autoremove(cxlmd, endpoint);
+	if (rc)
+		return rc;
+
+	if (!endpoint->dev.driver) {
+		dev_err(&cxlmd->dev, "%s failed probe\n",
+			dev_name(&endpoint->dev));
+		return -ENXIO;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_FOR_MODULES(devm_cxl_add_endpoint, "cxl_mem");
+
 static int __init cxl_port_init(void)
 {
 	return cxl_driver_register(&cxl_port_driver);
-- 
2.51.1

Re: [PATCH 3/6] cxl/port: Arrange for always synchronous endpoint attach

[Dave Jiang]

On 12/3/25 7:21 PM, Dan Williams wrote:
> Make it so that upon return from devm_cxl_add_endpoint() that
> cxl_mem_probe() can assume that the endpoint has had a chance to complete
> cxl_port_probe().  I.e. cxl_port module loading has completed prior to
> device registration.
> 
> Delete the MODULE_SOFTDEP() as it is not sufficient for this purpose, but a
> hard link-time dependency is reliable. Specifically MODULE_SOFTDEP() does
> not guarantee that the module loading has completed prior to the completion
> of the current module's init.
> 
> Cc: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
> Cc: Alejandro Lucero <alucerop@amd.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/cxl/cxl.h  |  2 ++
>  drivers/cxl/mem.c  | 43 -------------------------------------------
>  drivers/cxl/port.c | 40 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 42 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index ba17fa86d249..c796c3db36e0 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -780,6 +780,8 @@ struct cxl_port *devm_cxl_add_port(struct device *host,
>  				   struct cxl_dport *parent_dport);
>  struct cxl_root *devm_cxl_add_root(struct device *host,
>  				   const struct cxl_root_ops *ops);
> +int devm_cxl_add_endpoint(struct device *host, struct cxl_memdev *cxlmd,
> +			  struct cxl_dport *parent_dport);
>  struct cxl_root *find_cxl_root(struct cxl_port *port);
>  
>  DEFINE_FREE(put_cxl_root, struct cxl_root *, if (_T) put_device(&_T->port.dev))
> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
> index 55883797ab2d..d62931526fd4 100644
> --- a/drivers/cxl/mem.c
> +++ b/drivers/cxl/mem.c
> @@ -45,44 +45,6 @@ static int cxl_mem_dpa_show(struct seq_file *file, void *data)
>  	return 0;
>  }
>  
> -static int devm_cxl_add_endpoint(struct device *host, struct cxl_memdev *cxlmd,
> -				 struct cxl_dport *parent_dport)
> -{
> -	struct cxl_port *parent_port = parent_dport->port;
> -	struct cxl_port *endpoint, *iter, *down;
> -	int rc;
> -
> -	/*
> -	 * Now that the path to the root is established record all the
> -	 * intervening ports in the chain.
> -	 */
> -	for (iter = parent_port, down = NULL; !is_cxl_root(iter);
> -	     down = iter, iter = to_cxl_port(iter->dev.parent)) {
> -		struct cxl_ep *ep;
> -
> -		ep = cxl_ep_load(iter, cxlmd);
> -		ep->next = down;
> -	}
> -
> -	/* Note: endpoint port component registers are derived from @cxlds */
> -	endpoint = devm_cxl_add_port(host, &cxlmd->dev, CXL_RESOURCE_NONE,
> -				     parent_dport);
> -	if (IS_ERR(endpoint))
> -		return PTR_ERR(endpoint);
> -
> -	rc = cxl_endpoint_autoremove(cxlmd, endpoint);
> -	if (rc)
> -		return rc;
> -
> -	if (!endpoint->dev.driver) {
> -		dev_err(&cxlmd->dev, "%s failed probe\n",
> -			dev_name(&endpoint->dev));
> -		return -ENXIO;
> -	}
> -
> -	return 0;
> -}
> -
>  static int cxl_debugfs_poison_inject(void *data, u64 dpa)
>  {
>  	struct cxl_memdev *cxlmd = data;
> @@ -275,8 +237,3 @@ MODULE_DESCRIPTION("CXL: Memory Expansion");
>  MODULE_LICENSE("GPL v2");
>  MODULE_IMPORT_NS("CXL");
>  MODULE_ALIAS_CXL(CXL_DEVICE_MEMORY_EXPANDER);
> -/*
> - * create_endpoint() wants to validate port driver attach immediately after
> - * endpoint registration.
> - */
> -MODULE_SOFTDEP("pre: cxl_port");
> diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
> index 51c8f2f84717..7937e7e53797 100644
> --- a/drivers/cxl/port.c
> +++ b/drivers/cxl/port.c
> @@ -156,10 +156,50 @@ static struct cxl_driver cxl_port_driver = {
>  	.probe = cxl_port_probe,
>  	.id = CXL_DEVICE_PORT,
>  	.drv = {
> +		.probe_type = PROBE_FORCE_SYNCHRONOUS,
>  		.dev_groups = cxl_port_attribute_groups,
>  	},
>  };
>  
> +int devm_cxl_add_endpoint(struct device *host, struct cxl_memdev *cxlmd,
> +			  struct cxl_dport *parent_dport)
> +{
> +	struct cxl_port *parent_port = parent_dport->port;
> +	struct cxl_port *endpoint, *iter, *down;
> +	int rc;
> +
> +	/*
> +	 * Now that the path to the root is established record all the
> +	 * intervening ports in the chain.
> +	 */
> +	for (iter = parent_port, down = NULL; !is_cxl_root(iter);
> +	     down = iter, iter = to_cxl_port(iter->dev.parent)) {
> +		struct cxl_ep *ep;
> +
> +		ep = cxl_ep_load(iter, cxlmd);
> +		ep->next = down;
> +	}
> +
> +	/* Note: endpoint port component registers are derived from @cxlds */
> +	endpoint = devm_cxl_add_port(host, &cxlmd->dev, CXL_RESOURCE_NONE,
> +				     parent_dport);
> +	if (IS_ERR(endpoint))
> +		return PTR_ERR(endpoint);
> +
> +	rc = cxl_endpoint_autoremove(cxlmd, endpoint);
> +	if (rc)
> +		return rc;
> +
> +	if (!endpoint->dev.driver) {
> +		dev_err(&cxlmd->dev, "%s failed probe\n",
> +			dev_name(&endpoint->dev));
> +		return -ENXIO;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_FOR_MODULES(devm_cxl_add_endpoint, "cxl_mem");
> +
>  static int __init cxl_port_init(void)
>  {
>  	return cxl_driver_register(&cxl_port_driver);

Re: [PATCH 3/6] cxl/port: Arrange for always synchronous endpoint attach

[Cheatham, Benjamin]

On 12/3/2025 8:21 PM, Dan Williams wrote:
> Make it so that upon return from devm_cxl_add_endpoint() that
> cxl_mem_probe() can assume that the endpoint has had a chance to complete
> cxl_port_probe().  I.e. cxl_port module loading has completed prior to
> device registration.
> 
> Delete the MODULE_SOFTDEP() as it is not sufficient for this purpose, but a
> hard link-time dependency is reliable. Specifically MODULE_SOFTDEP() does
> not guarantee that the module loading has completed prior to the completion
> of the current module's init.
> 
> Cc: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
> Cc: Alejandro Lucero <alucerop@amd.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---

Reviewed-by: Ben Cheatham <benjamin.cheatham@amd.com>

Re: [PATCH 3/6] cxl/port: Arrange for always synchronous endpoint attach

[Alison Schofield]

On Wed, Dec 03, 2025 at 06:21:33PM -0800, Dan Williams wrote:
> Make it so that upon return from devm_cxl_add_endpoint() that
> cxl_mem_probe() can assume that the endpoint has had a chance to complete
> cxl_port_probe().  I.e. cxl_port module loading has completed prior to
> device registration.
> 
> Delete the MODULE_SOFTDEP() as it is not sufficient for this purpose, but a
> hard link-time dependency is reliable. Specifically MODULE_SOFTDEP() does
> not guarantee that the module loading has completed prior to the completion
> of the current module's init.
> 
> Cc: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
> Cc: Alejandro Lucero <alucerop@amd.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Tested-by: Alison Schofield <alison.schofield@intel.com>
Reviewed-by: Alison Schofield <alison.schofield@intel.com>

Re: [PATCH 3/6] cxl/port: Arrange for always synchronous endpoint attach

[Jonathan Cameron]

On Wed,  3 Dec 2025 18:21:33 -0800
Dan Williams <dan.j.williams@intel.com> wrote:

> Make it so that upon return from devm_cxl_add_endpoint() that
> cxl_mem_probe() can assume that the endpoint has had a chance to complete
> cxl_port_probe().  I.e. cxl_port module loading has completed prior to
> device registration.
> 
> Delete the MODULE_SOFTDEP() as it is not sufficient for this purpose, but a
> hard link-time dependency is reliable. Specifically MODULE_SOFTDEP() does
> not guarantee that the module loading has completed prior to the completion
> of the current module's init.
> 
> Cc: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
> Cc: Alejandro Lucero <alucerop@amd.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>

[PATCH 4/6] cxl/mem: Convert devm_cxl_add_memdev() to scope-based-cleanup

[Dan Williams]

In preparation for adding more setup steps, convert the current
implementation to scope-based cleanup.

The cxl_memdev_shutdown() is only required after cdev_device_add(). With
that moved to a helper function it precludes the need to add
scope-based-handler for that cleanup if devm_add_action_or_reset() fails.

Cc: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
Cc: Alejandro Lucero <alucerop@amd.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/cxl/core/memdev.c | 58 +++++++++++++++++++++++----------------
 1 file changed, 34 insertions(+), 24 deletions(-)

diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
index 7a4153e1c6a7..d0efc9ceda2a 100644
--- a/drivers/cxl/core/memdev.c
+++ b/drivers/cxl/core/memdev.c
@@ -1050,6 +1050,33 @@ static const struct file_operations cxl_memdev_fops = {
 	.llseek = noop_llseek,
 };
 
+/*
+ * Activate ioctl operations, no cxl_memdev_rwsem manipulation needed as this is
+ * ordered with cdev_add() publishing the device.
+ */
+static int cxlmd_add(struct cxl_memdev *cxlmd, struct cxl_dev_state *cxlds)
+{
+	int rc;
+
+	cxlmd->cxlds = cxlds;
+	cxlds->cxlmd = cxlmd;
+
+	rc = cdev_device_add(&cxlmd->cdev, &cxlmd->dev);
+	if (rc) {
+		/*
+		 * The cdev was briefly live, shutdown any ioctl operations that
+		 * saw that state.
+		 */
+		cxl_memdev_shutdown(&cxlmd->dev);
+		return rc;
+	}
+
+	return 0;
+}
+
+DEFINE_FREE(put_cxlmd, struct cxl_memdev *,
+	    if (!IS_ERR_OR_NULL(_T)) put_device(&_T->dev);)
+
 /*
  * Core helper for devm_cxl_add_memdev() that wants to both create a device and
  * assert to the caller that upon return cxl_mem::probe() has been invoked.
@@ -1057,45 +1084,28 @@ static const struct file_operations cxl_memdev_fops = {
 struct cxl_memdev *__devm_cxl_add_memdev(struct device *host,
 					 struct cxl_dev_state *cxlds)
 {
-	struct cxl_memdev *cxlmd;
 	struct device *dev;
-	struct cdev *cdev;
 	int rc;
 
-	cxlmd = cxl_memdev_alloc(cxlds, &cxl_memdev_fops);
+	struct cxl_memdev *cxlmd __free(put_cxlmd) =
+		cxl_memdev_alloc(cxlds, &cxl_memdev_fops);
 	if (IS_ERR(cxlmd))
 		return cxlmd;
 
 	dev = &cxlmd->dev;
 	rc = dev_set_name(dev, "mem%d", cxlmd->id);
 	if (rc)
-		goto err;
-
-	/*
-	 * Activate ioctl operations, no cxl_memdev_rwsem manipulation
-	 * needed as this is ordered with cdev_add() publishing the device.
-	 */
-	cxlmd->cxlds = cxlds;
-	cxlds->cxlmd = cxlmd;
+		return ERR_PTR(rc);
 
-	cdev = &cxlmd->cdev;
-	rc = cdev_device_add(cdev, dev);
+	rc = cxlmd_add(cxlmd, cxlds);
 	if (rc)
-		goto err;
+		return ERR_PTR(rc);
 
-	rc = devm_add_action_or_reset(host, cxl_memdev_unregister, cxlmd);
+	rc = devm_add_action_or_reset(host, cxl_memdev_unregister,
+				      no_free_ptr(cxlmd));
 	if (rc)
 		return ERR_PTR(rc);
 	return cxlmd;
-
-err:
-	/*
-	 * The cdev was briefly live, shutdown any ioctl operations that
-	 * saw that state.
-	 */
-	cxl_memdev_shutdown(dev);
-	put_device(dev);
-	return ERR_PTR(rc);
 }
 EXPORT_SYMBOL_FOR_MODULES(__devm_cxl_add_memdev, "cxl_mem");
 
-- 
2.51.1

Re: [PATCH 4/6] cxl/mem: Convert devm_cxl_add_memdev() to scope-based-cleanup

[Dave Jiang]

On 12/3/25 7:21 PM, Dan Williams wrote:
> In preparation for adding more setup steps, convert the current
> implementation to scope-based cleanup.
> 
> The cxl_memdev_shutdown() is only required after cdev_device_add(). With
> that moved to a helper function it precludes the need to add
> scope-based-handler for that cleanup if devm_add_action_or_reset() fails.
> 
> Cc: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
> Cc: Alejandro Lucero <alucerop@amd.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/cxl/core/memdev.c | 58 +++++++++++++++++++++++----------------
>  1 file changed, 34 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index 7a4153e1c6a7..d0efc9ceda2a 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -1050,6 +1050,33 @@ static const struct file_operations cxl_memdev_fops = {
>  	.llseek = noop_llseek,
>  };
>  
> +/*
> + * Activate ioctl operations, no cxl_memdev_rwsem manipulation needed as this is
> + * ordered with cdev_add() publishing the device.
> + */
> +static int cxlmd_add(struct cxl_memdev *cxlmd, struct cxl_dev_state *cxlds)
> +{
> +	int rc;
> +
> +	cxlmd->cxlds = cxlds;
> +	cxlds->cxlmd = cxlmd;
> +
> +	rc = cdev_device_add(&cxlmd->cdev, &cxlmd->dev);
> +	if (rc) {
> +		/*
> +		 * The cdev was briefly live, shutdown any ioctl operations that
> +		 * saw that state.
> +		 */
> +		cxl_memdev_shutdown(&cxlmd->dev);
> +		return rc;
> +	}
> +
> +	return 0;
> +}
> +
> +DEFINE_FREE(put_cxlmd, struct cxl_memdev *,
> +	    if (!IS_ERR_OR_NULL(_T)) put_device(&_T->dev);)
> +
>  /*
>   * Core helper for devm_cxl_add_memdev() that wants to both create a device and
>   * assert to the caller that upon return cxl_mem::probe() has been invoked.
> @@ -1057,45 +1084,28 @@ static const struct file_operations cxl_memdev_fops = {
>  struct cxl_memdev *__devm_cxl_add_memdev(struct device *host,
>  					 struct cxl_dev_state *cxlds)
>  {
> -	struct cxl_memdev *cxlmd;
>  	struct device *dev;
> -	struct cdev *cdev;
>  	int rc;
>  
> -	cxlmd = cxl_memdev_alloc(cxlds, &cxl_memdev_fops);
> +	struct cxl_memdev *cxlmd __free(put_cxlmd) =
> +		cxl_memdev_alloc(cxlds, &cxl_memdev_fops);
>  	if (IS_ERR(cxlmd))
>  		return cxlmd;
>  
>  	dev = &cxlmd->dev;
>  	rc = dev_set_name(dev, "mem%d", cxlmd->id);
>  	if (rc)
> -		goto err;
> -
> -	/*
> -	 * Activate ioctl operations, no cxl_memdev_rwsem manipulation
> -	 * needed as this is ordered with cdev_add() publishing the device.
> -	 */
> -	cxlmd->cxlds = cxlds;
> -	cxlds->cxlmd = cxlmd;
> +		return ERR_PTR(rc);
>  
> -	cdev = &cxlmd->cdev;
> -	rc = cdev_device_add(cdev, dev);
> +	rc = cxlmd_add(cxlmd, cxlds);
>  	if (rc)
> -		goto err;
> +		return ERR_PTR(rc);
>  
> -	rc = devm_add_action_or_reset(host, cxl_memdev_unregister, cxlmd);
> +	rc = devm_add_action_or_reset(host, cxl_memdev_unregister,
> +				      no_free_ptr(cxlmd));
>  	if (rc)
>  		return ERR_PTR(rc);
>  	return cxlmd;
> -
> -err:
> -	/*
> -	 * The cdev was briefly live, shutdown any ioctl operations that
> -	 * saw that state.
> -	 */
> -	cxl_memdev_shutdown(dev);
> -	put_device(dev);
> -	return ERR_PTR(rc);
>  }
>  EXPORT_SYMBOL_FOR_MODULES(__devm_cxl_add_memdev, "cxl_mem");
>  

Re: [PATCH 4/6] cxl/mem: Convert devm_cxl_add_memdev() to scope-based-cleanup

[Cheatham, Benjamin]

[snip]

> +}
> +
> +DEFINE_FREE(put_cxlmd, struct cxl_memdev *,
> +	    if (!IS_ERR_OR_NULL(_T)) put_device(&_T->dev);)
> +
>  /*
>   * Core helper for devm_cxl_add_memdev() that wants to both create a device and
>   * assert to the caller that upon return cxl_mem::probe() has been invoked.
> @@ -1057,45 +1084,28 @@ static const struct file_operations cxl_memdev_fops = {
>  struct cxl_memdev *__devm_cxl_add_memdev(struct device *host,
>  					 struct cxl_dev_state *cxlds)
>  {
> -	struct cxl_memdev *cxlmd;
>  	struct device *dev;
> -	struct cdev *cdev;
>  	int rc;
>  
> -	cxlmd = cxl_memdev_alloc(cxlds, &cxl_memdev_fops);
> +	struct cxl_memdev *cxlmd __free(put_cxlmd) =
> +		cxl_memdev_alloc(cxlds, &cxl_memdev_fops);
>  	if (IS_ERR(cxlmd))
>  		return cxlmd;
>  
>  	dev = &cxlmd->dev;
>  	rc = dev_set_name(dev, "mem%d", cxlmd->id);
>  	if (rc)
> -		goto err;
> -
> -	/*
> -	 * Activate ioctl operations, no cxl_memdev_rwsem manipulation
> -	 * needed as this is ordered with cdev_add() publishing the device.
> -	 */
> -	cxlmd->cxlds = cxlds;
> -	cxlds->cxlmd = cxlmd;
> +		return ERR_PTR(rc);
>  
> -	cdev = &cxlmd->cdev;
> -	rc = cdev_device_add(cdev, dev);
> +	rc = cxlmd_add(cxlmd, cxlds);
>  	if (rc)
> -		goto err;
> +		return ERR_PTR(rc);
>  
> -	rc = devm_add_action_or_reset(host, cxl_memdev_unregister, cxlmd);
> +	rc = devm_add_action_or_reset(host, cxl_memdev_unregister,
> +				      no_free_ptr(cxlmd));
>  	if (rc)
>  		return ERR_PTR(rc);
>  	return cxlmd;

Isn't cxlmd zeroed out by no_free_ptr() above? I think what needs to happen here is:

	rc = devm_add_action_or_reset(host, cxl_memdev_unregister, cxlmd);
	if (rc) {
		no_free_ptr(cxlmd);
		return ERR_PTR(rc);
	}

	return_ptr(cxlmd);

Looking ahead, this gets nullified in patch 6/6 so I guess it's only an issue if
someone is in the middle of a bisect (or doesn't pick up patch 6/6 for some reason).

> -
> -err:
> -	/*
> -	 * The cdev was briefly live, shutdown any ioctl operations that
> -	 * saw that state.
> -	 */
> -	cxl_memdev_shutdown(dev);
> -	put_device(dev);
> -	return ERR_PTR(rc);
>  }
>  EXPORT_SYMBOL_FOR_MODULES(__devm_cxl_add_memdev, "cxl_mem");
>  

Re: [PATCH 4/6] cxl/mem: Convert devm_cxl_add_memdev() to scope-based-cleanup

[dan.j.williams]

Cheatham, Benjamin wrote:
[..]
> > -	rc = devm_add_action_or_reset(host, cxl_memdev_unregister, cxlmd);
> > +	rc = devm_add_action_or_reset(host, cxl_memdev_unregister,
> > +				      no_free_ptr(cxlmd));
> >  	if (rc)
> >  		return ERR_PTR(rc);
> >  	return cxlmd;
> 
> Isn't cxlmd zeroed out by no_free_ptr() above? I think what needs to happen here is:
> 
> 	rc = devm_add_action_or_reset(host, cxl_memdev_unregister, cxlmd);
> 	if (rc) {
> 		no_free_ptr(cxlmd);
> 		return ERR_PTR(rc);
> 	}
> 
> 	return_ptr(cxlmd);
> 
> Looking ahead, this gets nullified in patch 6/6 so I guess it's only an issue if
> someone is in the middle of a bisect (or doesn't pick up patch 6/6 for some reason).

Good catch! ...and no, I should not leave that bisect problem in the set.

Will pull the cxl_memdev_autoremove() change forward for that.

Re: [PATCH 4/6] cxl/mem: Convert devm_cxl_add_memdev() to scope-based-cleanup

[Alison Schofield]

On Wed, Dec 03, 2025 at 06:21:34PM -0800, Dan Williams wrote:
> In preparation for adding more setup steps, convert the current
> implementation to scope-based cleanup.
> 
> The cxl_memdev_shutdown() is only required after cdev_device_add(). With
> that moved to a helper function it precludes the need to add
> scope-based-handler for that cleanup if devm_add_action_or_reset() fails.
> 
> Cc: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
> Cc: Alejandro Lucero <alucerop@amd.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Reviewed-by: Alison Schofield <alison.schofield@intel.com>

[PATCH 5/6] cxl/mem: Drop @host argument to devm_cxl_add_memdev()

[Dan Williams]

In all cases the device that created the 'struct cxl_dev_state' instance is
also the device to host the devm cleanup of devm_cxl_add_memdev(). This
simplifies the function prototype, and limits a degree of freedom of the
API.

Cc: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
Cc: Alejandro Lucero <alucerop@amd.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/cxl/cxlmem.h         | 6 ++----
 drivers/cxl/core/memdev.c    | 5 ++---
 drivers/cxl/mem.c            | 9 +++++----
 drivers/cxl/pci.c            | 2 +-
 tools/testing/cxl/test/mem.c | 2 +-
 5 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index 012e68acad34..9db31c7993c4 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -95,10 +95,8 @@ static inline bool is_cxl_endpoint(struct cxl_port *port)
 	return is_cxl_memdev(port->uport_dev);
 }
 
-struct cxl_memdev *__devm_cxl_add_memdev(struct device *host,
-					 struct cxl_dev_state *cxlds);
-struct cxl_memdev *devm_cxl_add_memdev(struct device *host,
-				       struct cxl_dev_state *cxlds);
+struct cxl_memdev *__devm_cxl_add_memdev(struct cxl_dev_state *cxlds);
+struct cxl_memdev *devm_cxl_add_memdev(struct cxl_dev_state *cxlds);
 int devm_cxl_sanitize_setup_notifier(struct device *host,
 				     struct cxl_memdev *cxlmd);
 struct cxl_memdev_state;
diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
index d0efc9ceda2a..7d85ba25835a 100644
--- a/drivers/cxl/core/memdev.c
+++ b/drivers/cxl/core/memdev.c
@@ -1081,8 +1081,7 @@ DEFINE_FREE(put_cxlmd, struct cxl_memdev *,
  * Core helper for devm_cxl_add_memdev() that wants to both create a device and
  * assert to the caller that upon return cxl_mem::probe() has been invoked.
  */
-struct cxl_memdev *__devm_cxl_add_memdev(struct device *host,
-					 struct cxl_dev_state *cxlds)
+struct cxl_memdev *__devm_cxl_add_memdev(struct cxl_dev_state *cxlds)
 {
 	struct device *dev;
 	int rc;
@@ -1101,7 +1100,7 @@ struct cxl_memdev *__devm_cxl_add_memdev(struct device *host,
 	if (rc)
 		return ERR_PTR(rc);
 
-	rc = devm_add_action_or_reset(host, cxl_memdev_unregister,
+	rc = devm_add_action_or_reset(cxlds->dev, cxl_memdev_unregister,
 				      no_free_ptr(cxlmd));
 	if (rc)
 		return ERR_PTR(rc);
diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
index d62931526fd4..677996c65272 100644
--- a/drivers/cxl/mem.c
+++ b/drivers/cxl/mem.c
@@ -165,17 +165,18 @@ static int cxl_mem_probe(struct device *dev)
 
 /**
  * devm_cxl_add_memdev - Add a CXL memory device
- * @host: devres alloc/release context and parent for the memdev
  * @cxlds: CXL device state to associate with the memdev
  *
  * Upon return the device will have had a chance to attach to the
  * cxl_mem driver, but may fail if the CXL topology is not ready
  * (hardware CXL link down, or software platform CXL root not attached)
+ *
+ * The parent of the resulting device and the devm context for allocations is
+ * @cxlds->dev.
  */
-struct cxl_memdev *devm_cxl_add_memdev(struct device *host,
-				       struct cxl_dev_state *cxlds)
+struct cxl_memdev *devm_cxl_add_memdev(struct cxl_dev_state *cxlds)
 {
-	return __devm_cxl_add_memdev(host, cxlds);
+	return __devm_cxl_add_memdev(cxlds);
 }
 EXPORT_SYMBOL_NS_GPL(devm_cxl_add_memdev, "CXL");
 
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index 0be4e508affe..1c6fc5334806 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -1006,7 +1006,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (rc)
 		dev_dbg(&pdev->dev, "No CXL Features discovered\n");
 
-	cxlmd = devm_cxl_add_memdev(&pdev->dev, cxlds);
+	cxlmd = devm_cxl_add_memdev(cxlds);
 	if (IS_ERR(cxlmd))
 		return PTR_ERR(cxlmd);
 
diff --git a/tools/testing/cxl/test/mem.c b/tools/testing/cxl/test/mem.c
index 176dcde570cd..8a22b7601627 100644
--- a/tools/testing/cxl/test/mem.c
+++ b/tools/testing/cxl/test/mem.c
@@ -1767,7 +1767,7 @@ static int cxl_mock_mem_probe(struct platform_device *pdev)
 
 	cxl_mock_add_event_logs(&mdata->mes);
 
-	cxlmd = devm_cxl_add_memdev(&pdev->dev, cxlds);
+	cxlmd = devm_cxl_add_memdev(cxlds);
 	if (IS_ERR(cxlmd))
 		return PTR_ERR(cxlmd);
 
-- 
2.51.1

Re: [PATCH 5/6] cxl/mem: Drop @host argument to devm_cxl_add_memdev()

[Cheatham, Benjamin]

On 12/3/2025 8:21 PM, Dan Williams wrote:
> In all cases the device that created the 'struct cxl_dev_state' instance is
> also the device to host the devm cleanup of devm_cxl_add_memdev(). This
> simplifies the function prototype, and limits a degree of freedom of the
> API.
> 
> Cc: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
> Cc: Alejandro Lucero <alucerop@amd.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---

Reviewed-by: Ben Cheatham <benjamin.cheatham@amd.com>

Re: [PATCH 5/6] cxl/mem: Drop @host argument to devm_cxl_add_memdev()

[Dave Jiang]

On 12/3/25 7:21 PM, Dan Williams wrote:
> In all cases the device that created the 'struct cxl_dev_state' instance is
> also the device to host the devm cleanup of devm_cxl_add_memdev(). This
> simplifies the function prototype, and limits a degree of freedom of the
> API.
> 
> Cc: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
> Cc: Alejandro Lucero <alucerop@amd.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/cxl/cxlmem.h         | 6 ++----
>  drivers/cxl/core/memdev.c    | 5 ++---
>  drivers/cxl/mem.c            | 9 +++++----
>  drivers/cxl/pci.c            | 2 +-
>  tools/testing/cxl/test/mem.c | 2 +-
>  5 files changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index 012e68acad34..9db31c7993c4 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -95,10 +95,8 @@ static inline bool is_cxl_endpoint(struct cxl_port *port)
>  	return is_cxl_memdev(port->uport_dev);
>  }
>  
> -struct cxl_memdev *__devm_cxl_add_memdev(struct device *host,
> -					 struct cxl_dev_state *cxlds);
> -struct cxl_memdev *devm_cxl_add_memdev(struct device *host,
> -				       struct cxl_dev_state *cxlds);
> +struct cxl_memdev *__devm_cxl_add_memdev(struct cxl_dev_state *cxlds);
> +struct cxl_memdev *devm_cxl_add_memdev(struct cxl_dev_state *cxlds);
>  int devm_cxl_sanitize_setup_notifier(struct device *host,
>  				     struct cxl_memdev *cxlmd);
>  struct cxl_memdev_state;
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index d0efc9ceda2a..7d85ba25835a 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -1081,8 +1081,7 @@ DEFINE_FREE(put_cxlmd, struct cxl_memdev *,
>   * Core helper for devm_cxl_add_memdev() that wants to both create a device and
>   * assert to the caller that upon return cxl_mem::probe() has been invoked.
>   */
> -struct cxl_memdev *__devm_cxl_add_memdev(struct device *host,
> -					 struct cxl_dev_state *cxlds)
> +struct cxl_memdev *__devm_cxl_add_memdev(struct cxl_dev_state *cxlds)
>  {
>  	struct device *dev;
>  	int rc;
> @@ -1101,7 +1100,7 @@ struct cxl_memdev *__devm_cxl_add_memdev(struct device *host,
>  	if (rc)
>  		return ERR_PTR(rc);
>  
> -	rc = devm_add_action_or_reset(host, cxl_memdev_unregister,
> +	rc = devm_add_action_or_reset(cxlds->dev, cxl_memdev_unregister,
>  				      no_free_ptr(cxlmd));
>  	if (rc)
>  		return ERR_PTR(rc);
> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
> index d62931526fd4..677996c65272 100644
> --- a/drivers/cxl/mem.c
> +++ b/drivers/cxl/mem.c
> @@ -165,17 +165,18 @@ static int cxl_mem_probe(struct device *dev)
>  
>  /**
>   * devm_cxl_add_memdev - Add a CXL memory device
> - * @host: devres alloc/release context and parent for the memdev
>   * @cxlds: CXL device state to associate with the memdev
>   *
>   * Upon return the device will have had a chance to attach to the
>   * cxl_mem driver, but may fail if the CXL topology is not ready
>   * (hardware CXL link down, or software platform CXL root not attached)
> + *
> + * The parent of the resulting device and the devm context for allocations is
> + * @cxlds->dev.
>   */
> -struct cxl_memdev *devm_cxl_add_memdev(struct device *host,
> -				       struct cxl_dev_state *cxlds)
> +struct cxl_memdev *devm_cxl_add_memdev(struct cxl_dev_state *cxlds)
>  {
> -	return __devm_cxl_add_memdev(host, cxlds);
> +	return __devm_cxl_add_memdev(cxlds);
>  }
>  EXPORT_SYMBOL_NS_GPL(devm_cxl_add_memdev, "CXL");
>  
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index 0be4e508affe..1c6fc5334806 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -1006,7 +1006,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	if (rc)
>  		dev_dbg(&pdev->dev, "No CXL Features discovered\n");
>  
> -	cxlmd = devm_cxl_add_memdev(&pdev->dev, cxlds);
> +	cxlmd = devm_cxl_add_memdev(cxlds);
>  	if (IS_ERR(cxlmd))
>  		return PTR_ERR(cxlmd);
>  
> diff --git a/tools/testing/cxl/test/mem.c b/tools/testing/cxl/test/mem.c
> index 176dcde570cd..8a22b7601627 100644
> --- a/tools/testing/cxl/test/mem.c
> +++ b/tools/testing/cxl/test/mem.c
> @@ -1767,7 +1767,7 @@ static int cxl_mock_mem_probe(struct platform_device *pdev)
>  
>  	cxl_mock_add_event_logs(&mdata->mes);
>  
> -	cxlmd = devm_cxl_add_memdev(&pdev->dev, cxlds);
> +	cxlmd = devm_cxl_add_memdev(cxlds);
>  	if (IS_ERR(cxlmd))
>  		return PTR_ERR(cxlmd);
>  

Re: [PATCH 5/6] cxl/mem: Drop @host argument to devm_cxl_add_memdev()

[Alison Schofield]

On Wed, Dec 03, 2025 at 06:21:35PM -0800, Dan Williams wrote:
> In all cases the device that created the 'struct cxl_dev_state' instance is
> also the device to host the devm cleanup of devm_cxl_add_memdev(). This
> simplifies the function prototype, and limits a degree of freedom of the
> API.
> 
> Cc: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
> Cc: Alejandro Lucero <alucerop@amd.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Reviewed-by: Alison Schofield <alison.schofield@intel.com>

Re: [PATCH 5/6] cxl/mem: Drop @host argument to devm_cxl_add_memdev()

[Jonathan Cameron]

On Wed,  3 Dec 2025 18:21:35 -0800
Dan Williams <dan.j.williams@intel.com> wrote:

> In all cases the device that created the 'struct cxl_dev_state' instance is
> also the device to host the devm cleanup of devm_cxl_add_memdev(). This
> simplifies the function prototype, and limits a degree of freedom of the
> API.
> 
> Cc: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
> Cc: Alejandro Lucero <alucerop@amd.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Fair enough. I'm not hugely keen on a devm_ first parameter not
being the dev, but constraining that interface is probably worth
the small amount of thought this requires.

Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>

[PATCH 6/6] cxl/mem: Introduce a memdev creation ->probe() operation

[Dan Williams]

Allow for a driver to pass a routine to be called in cxl_mem_probe()
context. This ability is inspired by and mirrors the semantics of
faux_device_create(). It allows for the caller to run CXL-topology
attach-dependent logic on behalf of the caller.

This capability is needed for CXL accelerator device drivers that need to
make decisions about enabling CXL dependent functionality in the device, or
falling back to PCIe-only operation.

The probe callback runs after the port topology is successfully attached
for the given memdev.

Cc: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
Cc: Alejandro Lucero <alucerop@amd.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/cxl/cxlmem.h         | 12 ++++++++--
 drivers/cxl/core/memdev.c    | 46 +++++++++++++++++++++++++++++-------
 drivers/cxl/mem.c            | 12 ++++++++--
 drivers/cxl/pci.c            |  2 +-
 tools/testing/cxl/test/mem.c |  2 +-
 5 files changed, 60 insertions(+), 14 deletions(-)

diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index 9db31c7993c4..ed759f88b21f 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -34,6 +34,10 @@
 	(FIELD_GET(CXLMDEV_RESET_NEEDED_MASK, status) !=                       \
 	 CXLMDEV_RESET_NEEDED_NOT)
 
+struct cxl_memdev_ops {
+	int (*probe)(struct cxl_memdev *cxlmd);
+};
+
 /**
  * struct cxl_memdev - CXL bus object representing a Type-3 Memory Device
  * @dev: driver core device object
@@ -43,6 +47,7 @@
  * @cxl_nvb: coordinate removal of @cxl_nvd if present
  * @cxl_nvd: optional bridge to an nvdimm if the device supports pmem
  * @endpoint: connection to the CXL port topology for this memory device
+ * @ops: incremental caller specific probe routine
  * @id: id number of this memdev instance.
  * @depth: endpoint port depth
  * @scrub_cycle: current scrub cycle set for this device
@@ -59,6 +64,7 @@ struct cxl_memdev {
 	struct cxl_nvdimm_bridge *cxl_nvb;
 	struct cxl_nvdimm *cxl_nvd;
 	struct cxl_port *endpoint;
+	const struct cxl_memdev_ops *ops;
 	int id;
 	int depth;
 	u8 scrub_cycle;
@@ -95,8 +101,10 @@ static inline bool is_cxl_endpoint(struct cxl_port *port)
 	return is_cxl_memdev(port->uport_dev);
 }
 
-struct cxl_memdev *__devm_cxl_add_memdev(struct cxl_dev_state *cxlds);
-struct cxl_memdev *devm_cxl_add_memdev(struct cxl_dev_state *cxlds);
+struct cxl_memdev *__devm_cxl_add_memdev(struct cxl_dev_state *cxlds,
+					 const struct cxl_memdev_ops *ops);
+struct cxl_memdev *devm_cxl_add_memdev(struct cxl_dev_state *cxlds,
+				       const struct cxl_memdev_ops *ops);
 int devm_cxl_sanitize_setup_notifier(struct device *host,
 				     struct cxl_memdev *cxlmd);
 struct cxl_memdev_state;
diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
index 7d85ba25835a..41f1964d8cbf 100644
--- a/drivers/cxl/core/memdev.c
+++ b/drivers/cxl/core/memdev.c
@@ -648,7 +648,8 @@ static void detach_memdev(struct work_struct *work)
 static struct lock_class_key cxl_memdev_key;
 
 static struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds,
-					   const struct file_operations *fops)
+					   const struct file_operations *fops,
+					   const struct cxl_memdev_ops *ops)
 {
 	struct cxl_memdev *cxlmd;
 	struct device *dev;
@@ -664,6 +665,8 @@ static struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds,
 		goto err;
 	cxlmd->id = rc;
 	cxlmd->depth = -1;
+	cxlmd->ops = ops;
+	cxlmd->endpoint = ERR_PTR(-ENXIO);
 
 	dev = &cxlmd->dev;
 	device_initialize(dev);
@@ -1077,17 +1080,48 @@ static int cxlmd_add(struct cxl_memdev *cxlmd, struct cxl_dev_state *cxlds)
 DEFINE_FREE(put_cxlmd, struct cxl_memdev *,
 	    if (!IS_ERR_OR_NULL(_T)) put_device(&_T->dev);)
 
+static struct cxl_memdev *cxl_memdev_autoremove(struct cxl_memdev *cxlmd)
+{
+	struct cxl_memdev *ret = cxlmd;
+	int rc;
+
+	/*
+	 * If ops is provided fail if the driver is not attached upon
+	 * return. The ->endpoint ERR_PTR may have a more precise error
+	 * code to convey. Note that failure here could be the result of
+	 * a race to teardown the CXL port topology. I.e.
+	 * cxl_mem_probe() could have succeeded and then cxl_mem unbound
+	 * before the lock is acquired.
+	 */
+	guard(device)(&cxlmd->dev);
+	if (cxlmd->ops && !cxlmd->dev.driver) {
+		ret = ERR_PTR(-ENXIO);
+		if (IS_ERR(cxlmd->endpoint))
+			ret = ERR_CAST(cxlmd->endpoint);
+		cxl_memdev_unregister(cxlmd);
+		return ret;
+	}
+
+	rc = devm_add_action_or_reset(cxlmd->cxlds->dev, cxl_memdev_unregister,
+				      cxlmd);
+	if (rc)
+		return ERR_PTR(rc);
+
+	return ret;
+}
+
 /*
  * Core helper for devm_cxl_add_memdev() that wants to both create a device and
  * assert to the caller that upon return cxl_mem::probe() has been invoked.
  */
-struct cxl_memdev *__devm_cxl_add_memdev(struct cxl_dev_state *cxlds)
+struct cxl_memdev *__devm_cxl_add_memdev(struct cxl_dev_state *cxlds,
+					 const struct cxl_memdev_ops *ops)
 {
 	struct device *dev;
 	int rc;
 
 	struct cxl_memdev *cxlmd __free(put_cxlmd) =
-		cxl_memdev_alloc(cxlds, &cxl_memdev_fops);
+		cxl_memdev_alloc(cxlds, &cxl_memdev_fops, ops);
 	if (IS_ERR(cxlmd))
 		return cxlmd;
 
@@ -1100,11 +1134,7 @@ struct cxl_memdev *__devm_cxl_add_memdev(struct cxl_dev_state *cxlds)
 	if (rc)
 		return ERR_PTR(rc);
 
-	rc = devm_add_action_or_reset(cxlds->dev, cxl_memdev_unregister,
-				      no_free_ptr(cxlmd));
-	if (rc)
-		return ERR_PTR(rc);
-	return cxlmd;
+	return cxl_memdev_autoremove(no_free_ptr(cxlmd));
 }
 EXPORT_SYMBOL_FOR_MODULES(__devm_cxl_add_memdev, "cxl_mem");
 
diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
index 677996c65272..d61f121c6f97 100644
--- a/drivers/cxl/mem.c
+++ b/drivers/cxl/mem.c
@@ -142,6 +142,12 @@ static int cxl_mem_probe(struct device *dev)
 			return rc;
 	}
 
+	if (cxlmd->ops) {
+		rc = cxlmd->ops->probe(cxlmd);
+		if (rc)
+			return rc;
+	}
+
 	rc = devm_cxl_memdev_edac_register(cxlmd);
 	if (rc)
 		dev_dbg(dev, "CXL memdev EDAC registration failed rc=%d\n", rc);
@@ -166,6 +172,7 @@ static int cxl_mem_probe(struct device *dev)
 /**
  * devm_cxl_add_memdev - Add a CXL memory device
  * @cxlds: CXL device state to associate with the memdev
+ * @ops: optional operations to run in cxl_mem::{probe,remove}() context
  *
  * Upon return the device will have had a chance to attach to the
  * cxl_mem driver, but may fail if the CXL topology is not ready
@@ -174,9 +181,10 @@ static int cxl_mem_probe(struct device *dev)
  * The parent of the resulting device and the devm context for allocations is
  * @cxlds->dev.
  */
-struct cxl_memdev *devm_cxl_add_memdev(struct cxl_dev_state *cxlds)
+struct cxl_memdev *devm_cxl_add_memdev(struct cxl_dev_state *cxlds,
+				       const struct cxl_memdev_ops *ops)
 {
-	return __devm_cxl_add_memdev(cxlds);
+	return __devm_cxl_add_memdev(cxlds, ops);
 }
 EXPORT_SYMBOL_NS_GPL(devm_cxl_add_memdev, "CXL");
 
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index 1c6fc5334806..549368a9c868 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -1006,7 +1006,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (rc)
 		dev_dbg(&pdev->dev, "No CXL Features discovered\n");
 
-	cxlmd = devm_cxl_add_memdev(cxlds);
+	cxlmd = devm_cxl_add_memdev(cxlds, NULL);
 	if (IS_ERR(cxlmd))
 		return PTR_ERR(cxlmd);
 
diff --git a/tools/testing/cxl/test/mem.c b/tools/testing/cxl/test/mem.c
index 8a22b7601627..cb87e8c0e63c 100644
--- a/tools/testing/cxl/test/mem.c
+++ b/tools/testing/cxl/test/mem.c
@@ -1767,7 +1767,7 @@ static int cxl_mock_mem_probe(struct platform_device *pdev)
 
 	cxl_mock_add_event_logs(&mdata->mes);
 
-	cxlmd = devm_cxl_add_memdev(cxlds);
+	cxlmd = devm_cxl_add_memdev(cxlds, NULL);
 	if (IS_ERR(cxlmd))
 		return PTR_ERR(cxlmd);
 
-- 
2.51.1

Re: [PATCH 6/6] cxl/mem: Introduce a memdev creation ->probe() operation

[Cheatham, Benjamin]

[snip]

> +static struct cxl_memdev *cxl_memdev_autoremove(struct cxl_memdev *cxlmd)
> +{
> +	struct cxl_memdev *ret = cxlmd;
> +	int rc;
> +
> +	/*
> +	 * If ops is provided fail if the driver is not attached upon
> +	 * return. The ->endpoint ERR_PTR may have a more precise error
> +	 * code to convey. Note that failure here could be the result of
> +	 * a race to teardown the CXL port topology. I.e.
> +	 * cxl_mem_probe() could have succeeded and then cxl_mem unbound
> +	 * before the lock is acquired.
> +	 */
> +	guard(device)(&cxlmd->dev);
> +	if (cxlmd->ops && !cxlmd->dev.driver) {
> +		ret = ERR_PTR(-ENXIO);
> +		if (IS_ERR(cxlmd->endpoint))
> +			ret = ERR_CAST(cxlmd->endpoint);
> +		cxl_memdev_unregister(cxlmd);
> +		return ret;
> +	}

Two minor gripes here:

1. The cxlmd->endpoint portion of this is unused. I think the idea is since this is prep work for
Alejandro's set to just put in here, but I would argue it should be introduced in his set since that's
where it's actually used.

2. I don't particularly like drivers having to provide a cxlmd->ops to automatically unregister the device on
cxl_mem probe failure. It's probably not likely, but it possible that a driver wouldn't have any extra set up
to do for CXL.mem but still needs to fallback to PCIe mode if it can't be properly set up.

I don't want to nit-pick without alternatives, so my first thought was a flag passed into devm_cxl_add_memdev()
(and eventually this function) that dictates whether failure to attach to cxl_mem is a failure condition. Then
that flag replaces the cxlmd->ops check. It may not be worth adding that degree of versatility before anyone
wants it though, so I'm fine with the above approach if you feel it's the way to go.

Thanks,
Ben

Re: [PATCH 6/6] cxl/mem: Introduce a memdev creation ->probe() operation

[dan.j.williams]

Cheatham, Benjamin wrote:
> [snip]
> 
> > +static struct cxl_memdev *cxl_memdev_autoremove(struct cxl_memdev *cxlmd)
> > +{
> > +	struct cxl_memdev *ret = cxlmd;
> > +	int rc;
> > +
> > +	/*
> > +	 * If ops is provided fail if the driver is not attached upon
> > +	 * return. The ->endpoint ERR_PTR may have a more precise error
> > +	 * code to convey. Note that failure here could be the result of
> > +	 * a race to teardown the CXL port topology. I.e.
> > +	 * cxl_mem_probe() could have succeeded and then cxl_mem unbound
> > +	 * before the lock is acquired.
> > +	 */
> > +	guard(device)(&cxlmd->dev);
> > +	if (cxlmd->ops && !cxlmd->dev.driver) {
> > +		ret = ERR_PTR(-ENXIO);
> > +		if (IS_ERR(cxlmd->endpoint))
> > +			ret = ERR_CAST(cxlmd->endpoint);
> > +		cxl_memdev_unregister(cxlmd);
> > +		return ret;
> > +	}
> 
> Two minor gripes here:
> 
> 1. The cxlmd->endpoint portion of this is unused. I think the idea is since this is prep work for
> Alejandro's set to just put in here, but I would argue it should be introduced in his set since that's
> where it's actually used.

Fair enough, a follow-on changes can add that back.

> 2. I don't particularly like drivers having to provide a cxlmd->ops to
> automatically unregister the device on cxl_mem probe failure. It's
> probably not likely, but it possible that a driver wouldn't have any
> extra set up to do for CXL.mem but still needs to fallback to PCIe
> mode if it can't be properly set up.
> 
> I don't want to nit-pick without alternatives, so my first thought was
> a flag passed into devm_cxl_add_memdev() (and eventually this
> function) that dictates whether failure to attach to cxl_mem is a
> failure condition. Then that flag replaces the cxlmd->ops check. It
> may not be worth adding that degree of versatility before anyone wants
> it though, so I'm fine with the above approach if you feel it's the
> way to go.

So the problem is that cxl_pci definitely does not care about immediate
cxl_memdev attachment the way that a Type-2 driver probably should. The
way I would handle the case of drivers that only want probe failures and
nothing else is to just provide a common default @ops implementation
that those drivers can use to get that behavior.

That is functionally equivalent to a new devm_cxl_add_memdev() flag, and
is something that can come later when such a driver arrives.

Re: [PATCH 6/6] cxl/mem: Introduce a memdev creation ->probe() operation

[dan.j.williams]

dan.j.williams@ wrote:
[..]
> That is functionally equivalent to a new devm_cxl_add_memdev() flag, and
> is something that can come later when such a driver arrives.

Here are the fixups collected for a v2, I added some documentation of
the expectations around @ops:

1:  aa399f4e13c5 = 1:  1368388728fd cxl/mem: Fix devm_cxl_memdev_edac_release() confusion
2:  120ac819e8cb = 2:  f56b7c7ec1cb cxl/mem: Arrange for always-synchronous memdev attach
3:  bcb0109994a3 = 3:  1fe83e925d59 cxl/port: Arrange for always synchronous endpoint attach
4:  74c426d1dd5d ! 4:  9bd03230d6f3 cxl/mem: Convert devm_cxl_add_memdev() to scope-based-cleanup
    @@ drivers/cxl/core/memdev.c: static const struct file_operations cxl_memdev_fops =
     +
     +DEFINE_FREE(put_cxlmd, struct cxl_memdev *,
     +	    if (!IS_ERR_OR_NULL(_T)) put_device(&_T->dev);)
    ++
    ++static struct cxl_memdev *cxl_memdev_autoremove(struct cxl_memdev *cxlmd)
    ++{
    ++	int rc;
    ++
    ++	rc = devm_add_action_or_reset(cxlmd->cxlds->dev, cxl_memdev_unregister,
    ++				      cxlmd);
    ++	if (rc)
    ++		return ERR_PTR(rc);
    ++
    ++	return cxlmd;
    ++}
     +
      /*
       * Core helper for devm_cxl_add_memdev() that wants to both create a device and
    @@ drivers/cxl/core/memdev.c: static const struct file_operations cxl_memdev_fops =
     -	 */
     -	cxlmd->cxlds = cxlds;
     -	cxlds->cxlmd = cxlmd;
    -+		return ERR_PTR(rc);
    - 
    +-
     -	cdev = &cxlmd->cdev;
     -	rc = cdev_device_add(cdev, dev);
    -+	rc = cxlmd_add(cxlmd, cxlds);
    - 	if (rc)
    +-	if (rc)
     -		goto err;
     +		return ERR_PTR(rc);
      
     -	rc = devm_add_action_or_reset(host, cxl_memdev_unregister, cxlmd);
    -+	rc = devm_add_action_or_reset(host, cxl_memdev_unregister,
    -+				      no_free_ptr(cxlmd));
    ++	rc = cxlmd_add(cxlmd, cxlds);
      	if (rc)
      		return ERR_PTR(rc);
    - 	return cxlmd;
    --
    +-	return cxlmd;
    + 
     -err:
     -	/*
     -	 * The cdev was briefly live, shutdown any ioctl operations that
    @@ drivers/cxl/core/memdev.c: static const struct file_operations cxl_memdev_fops =
     -	cxl_memdev_shutdown(dev);
     -	put_device(dev);
     -	return ERR_PTR(rc);
    ++	return cxl_memdev_autoremove(no_free_ptr(cxlmd));
      }
      EXPORT_SYMBOL_FOR_MODULES(__devm_cxl_add_memdev, "cxl_mem");
      
5:  b48acca13cdb ! 5:  e636831b1ff9 cxl/mem: Drop @host argument to devm_cxl_add_memdev()
    @@ drivers/cxl/cxlmem.h: static inline bool is_cxl_endpoint(struct cxl_port *port)
      struct cxl_memdev_state;
     
      ## drivers/cxl/core/memdev.c ##
    -@@ drivers/cxl/core/memdev.c: DEFINE_FREE(put_cxlmd, struct cxl_memdev *,
    +@@ drivers/cxl/core/memdev.c: static struct cxl_memdev *cxl_memdev_autoremove(struct cxl_memdev *cxlmd)
       * Core helper for devm_cxl_add_memdev() that wants to both create a device and
       * assert to the caller that upon return cxl_mem::probe() has been invoked.
       */
    @@ drivers/cxl/core/memdev.c: DEFINE_FREE(put_cxlmd, struct cxl_memdev *,
      {
      	struct device *dev;
      	int rc;
    -@@ drivers/cxl/core/memdev.c: struct cxl_memdev *__devm_cxl_add_memdev(struct device *host,
    - 	if (rc)
    - 		return ERR_PTR(rc);
    - 
    --	rc = devm_add_action_or_reset(host, cxl_memdev_unregister,
    -+	rc = devm_add_action_or_reset(cxlds->dev, cxl_memdev_unregister,
    - 				      no_free_ptr(cxlmd));
    - 	if (rc)
    - 		return ERR_PTR(rc);
     
      ## drivers/cxl/mem.c ##
     @@ drivers/cxl/mem.c: static int cxl_mem_probe(struct device *dev)
6:  f7e58dea4878 ! 6:  13c07d702c92 cxl/mem: Introduce a memdev creation ->probe() operation
    @@ drivers/cxl/core/memdev.c: static struct cxl_memdev *cxl_memdev_alloc(struct cxl
      
      	dev = &cxlmd->dev;
      	device_initialize(dev);
    -@@ drivers/cxl/core/memdev.c: static int cxlmd_add(struct cxl_memdev *cxlmd, struct cxl_dev_state *cxlds)
    - DEFINE_FREE(put_cxlmd, struct cxl_memdev *,
    - 	    if (!IS_ERR_OR_NULL(_T)) put_device(&_T->dev);)
    +@@ drivers/cxl/core/memdev.c: static struct cxl_memdev *cxl_memdev_autoremove(struct cxl_memdev *cxlmd)
    + {
    + 	int rc;
      
    -+static struct cxl_memdev *cxl_memdev_autoremove(struct cxl_memdev *cxlmd)
    -+{
    -+	struct cxl_memdev *ret = cxlmd;
    -+	int rc;
    -+
     +	/*
    -+	 * If ops is provided fail if the driver is not attached upon
    -+	 * return. The ->endpoint ERR_PTR may have a more precise error
    -+	 * code to convey. Note that failure here could be the result of
    -+	 * a race to teardown the CXL port topology. I.e.
    -+	 * cxl_mem_probe() could have succeeded and then cxl_mem unbound
    -+	 * before the lock is acquired.
    ++	 * If ops is provided fail if the driver is not attached upon return.
    ++	 * Note that failure here could be the result of a race to teardown the
    ++	 * CXL port topology. I.e. cxl_mem_probe() could have succeeded and then
    ++	 * cxl_mem unbound before the lock is acquired.
     +	 */
     +	guard(device)(&cxlmd->dev);
     +	if (cxlmd->ops && !cxlmd->dev.driver) {
    -+		ret = ERR_PTR(-ENXIO);
    -+		if (IS_ERR(cxlmd->endpoint))
    -+			ret = ERR_CAST(cxlmd->endpoint);
     +		cxl_memdev_unregister(cxlmd);
    -+		return ret;
    ++		return ERR_PTR(-ENXIO);
     +	}
     +
    -+	rc = devm_add_action_or_reset(cxlmd->cxlds->dev, cxl_memdev_unregister,
    -+				      cxlmd);
    -+	if (rc)
    -+		return ERR_PTR(rc);
    -+
    -+	return ret;
    -+}
    -+
    - /*
    + 	rc = devm_add_action_or_reset(cxlmd->cxlds->dev, cxl_memdev_unregister,
    + 				      cxlmd);
    + 	if (rc)
    +@@ drivers/cxl/core/memdev.c: static struct cxl_memdev *cxl_memdev_autoremove(struct cxl_memdev *cxlmd)
       * Core helper for devm_cxl_add_memdev() that wants to both create a device and
       * assert to the caller that upon return cxl_mem::probe() has been invoked.
       */
    @@ drivers/cxl/core/memdev.c: static int cxlmd_add(struct cxl_memdev *cxlmd, struct
      	if (IS_ERR(cxlmd))
      		return cxlmd;
      
    -@@ drivers/cxl/core/memdev.c: struct cxl_memdev *__devm_cxl_add_memdev(struct cxl_dev_state *cxlds)
    - 	if (rc)
    - 		return ERR_PTR(rc);
    - 
    --	rc = devm_add_action_or_reset(cxlds->dev, cxl_memdev_unregister,
    --				      no_free_ptr(cxlmd));
    --	if (rc)
    --		return ERR_PTR(rc);
    --	return cxlmd;
    -+	return cxl_memdev_autoremove(no_free_ptr(cxlmd));
    - }
    - EXPORT_SYMBOL_FOR_MODULES(__devm_cxl_add_memdev, "cxl_mem");
    - 
     
      ## drivers/cxl/mem.c ##
     @@ drivers/cxl/mem.c: static int cxl_mem_probe(struct device *dev)
    @@ drivers/cxl/mem.c: static int cxl_mem_probe(struct device *dev)
      /**
       * devm_cxl_add_memdev - Add a CXL memory device
       * @cxlds: CXL device state to associate with the memdev
    -+ * @ops: optional operations to run in cxl_mem::{probe,remove}() context
    ++ * @ops: optional operations to run in cxl_mem_probe() context
       *
       * Upon return the device will have had a chance to attach to the
    -  * cxl_mem driver, but may fail if the CXL topology is not ready
    -@@ drivers/cxl/mem.c: static int cxl_mem_probe(struct device *dev)
    +- * cxl_mem driver, but may fail if the CXL topology is not ready
    +- * (hardware CXL link down, or software platform CXL root not attached)
    ++ * cxl_mem driver, but may fail to attach if the CXL topology is not ready
    ++ * (hardware CXL link down, or software platform CXL root not attached).
    ++ *
    ++ * When @ops is NULL it indicates the caller wants the memdev to remain
    ++ * registered even if it does not immediately attach to the CXL hierarchy. When
    ++ * @ops is provided a cxl_mem_probe() failure leads to failure of this routine.
    +  *
       * The parent of the resulting device and the devm context for allocations is
       * @cxlds->dev.
       */

Re: [PATCH 6/6] cxl/mem: Introduce a memdev creation ->probe() operation

[Cheatham, Benjamin]

On 12/4/2025 4:02 PM, dan.j.williams@intel.com wrote:
> dan.j.williams@ wrote:
> [..]
>> That is functionally equivalent to a new devm_cxl_add_memdev() flag, and
>> is something that can come later when such a driver arrives.
> 
> Here are the fixups collected for a v2, I added some documentation of
> the expectations around @ops:
> 

Changes look good to me, feel free to add my reviewed-by for 4/6 & 6/6 when
you send v2.

Thanks,
Ben

Re: [PATCH 6/6] cxl/mem: Introduce a memdev creation ->probe() operation

[Dave Jiang]

On 12/3/25 7:21 PM, Dan Williams wrote:
> Allow for a driver to pass a routine to be called in cxl_mem_probe()
> context. This ability is inspired by and mirrors the semantics of
> faux_device_create(). It allows for the caller to run CXL-topology
> attach-dependent logic on behalf of the caller.
> 
> This capability is needed for CXL accelerator device drivers that need to
> make decisions about enabling CXL dependent functionality in the device, or
> falling back to PCIe-only operation.
> 
> The probe callback runs after the port topology is successfully attached
> for the given memdev.
> 
> Cc: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
> Cc: Alejandro Lucero <alucerop@amd.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/cxl/cxlmem.h         | 12 ++++++++--
>  drivers/cxl/core/memdev.c    | 46 +++++++++++++++++++++++++++++-------
>  drivers/cxl/mem.c            | 12 ++++++++--
>  drivers/cxl/pci.c            |  2 +-
>  tools/testing/cxl/test/mem.c |  2 +-
>  5 files changed, 60 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index 9db31c7993c4..ed759f88b21f 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -34,6 +34,10 @@
>  	(FIELD_GET(CXLMDEV_RESET_NEEDED_MASK, status) !=                       \
>  	 CXLMDEV_RESET_NEEDED_NOT)
>  
> +struct cxl_memdev_ops {
> +	int (*probe)(struct cxl_memdev *cxlmd);
> +};
> +
>  /**
>   * struct cxl_memdev - CXL bus object representing a Type-3 Memory Device
>   * @dev: driver core device object
> @@ -43,6 +47,7 @@
>   * @cxl_nvb: coordinate removal of @cxl_nvd if present
>   * @cxl_nvd: optional bridge to an nvdimm if the device supports pmem
>   * @endpoint: connection to the CXL port topology for this memory device
> + * @ops: incremental caller specific probe routine
>   * @id: id number of this memdev instance.
>   * @depth: endpoint port depth
>   * @scrub_cycle: current scrub cycle set for this device
> @@ -59,6 +64,7 @@ struct cxl_memdev {
>  	struct cxl_nvdimm_bridge *cxl_nvb;
>  	struct cxl_nvdimm *cxl_nvd;
>  	struct cxl_port *endpoint;
> +	const struct cxl_memdev_ops *ops;
>  	int id;
>  	int depth;
>  	u8 scrub_cycle;
> @@ -95,8 +101,10 @@ static inline bool is_cxl_endpoint(struct cxl_port *port)
>  	return is_cxl_memdev(port->uport_dev);
>  }
>  
> -struct cxl_memdev *__devm_cxl_add_memdev(struct cxl_dev_state *cxlds);
> -struct cxl_memdev *devm_cxl_add_memdev(struct cxl_dev_state *cxlds);
> +struct cxl_memdev *__devm_cxl_add_memdev(struct cxl_dev_state *cxlds,
> +					 const struct cxl_memdev_ops *ops);
> +struct cxl_memdev *devm_cxl_add_memdev(struct cxl_dev_state *cxlds,
> +				       const struct cxl_memdev_ops *ops);
>  int devm_cxl_sanitize_setup_notifier(struct device *host,
>  				     struct cxl_memdev *cxlmd);
>  struct cxl_memdev_state;
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index 7d85ba25835a..41f1964d8cbf 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -648,7 +648,8 @@ static void detach_memdev(struct work_struct *work)
>  static struct lock_class_key cxl_memdev_key;
>  
>  static struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds,
> -					   const struct file_operations *fops)
> +					   const struct file_operations *fops,
> +					   const struct cxl_memdev_ops *ops)
>  {
>  	struct cxl_memdev *cxlmd;
>  	struct device *dev;
> @@ -664,6 +665,8 @@ static struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds,
>  		goto err;
>  	cxlmd->id = rc;
>  	cxlmd->depth = -1;
> +	cxlmd->ops = ops;
> +	cxlmd->endpoint = ERR_PTR(-ENXIO);
>  
>  	dev = &cxlmd->dev;
>  	device_initialize(dev);
> @@ -1077,17 +1080,48 @@ static int cxlmd_add(struct cxl_memdev *cxlmd, struct cxl_dev_state *cxlds)
>  DEFINE_FREE(put_cxlmd, struct cxl_memdev *,
>  	    if (!IS_ERR_OR_NULL(_T)) put_device(&_T->dev);)
>  
> +static struct cxl_memdev *cxl_memdev_autoremove(struct cxl_memdev *cxlmd)
> +{
> +	struct cxl_memdev *ret = cxlmd;
> +	int rc;
> +
> +	/*
> +	 * If ops is provided fail if the driver is not attached upon
> +	 * return. The ->endpoint ERR_PTR may have a more precise error
> +	 * code to convey. Note that failure here could be the result of
> +	 * a race to teardown the CXL port topology. I.e.
> +	 * cxl_mem_probe() could have succeeded and then cxl_mem unbound
> +	 * before the lock is acquired.
> +	 */
> +	guard(device)(&cxlmd->dev);
> +	if (cxlmd->ops && !cxlmd->dev.driver) {
> +		ret = ERR_PTR(-ENXIO);
> +		if (IS_ERR(cxlmd->endpoint))
> +			ret = ERR_CAST(cxlmd->endpoint);
> +		cxl_memdev_unregister(cxlmd);
> +		return ret;
> +	}
> +
> +	rc = devm_add_action_or_reset(cxlmd->cxlds->dev, cxl_memdev_unregister,
> +				      cxlmd);
> +	if (rc)
> +		return ERR_PTR(rc);
> +
> +	return ret;
> +}
> +
>  /*
>   * Core helper for devm_cxl_add_memdev() that wants to both create a device and
>   * assert to the caller that upon return cxl_mem::probe() has been invoked.
>   */
> -struct cxl_memdev *__devm_cxl_add_memdev(struct cxl_dev_state *cxlds)
> +struct cxl_memdev *__devm_cxl_add_memdev(struct cxl_dev_state *cxlds,
> +					 const struct cxl_memdev_ops *ops)
>  {
>  	struct device *dev;
>  	int rc;
>  
>  	struct cxl_memdev *cxlmd __free(put_cxlmd) =
> -		cxl_memdev_alloc(cxlds, &cxl_memdev_fops);
> +		cxl_memdev_alloc(cxlds, &cxl_memdev_fops, ops);
>  	if (IS_ERR(cxlmd))
>  		return cxlmd;
>  
> @@ -1100,11 +1134,7 @@ struct cxl_memdev *__devm_cxl_add_memdev(struct cxl_dev_state *cxlds)
>  	if (rc)
>  		return ERR_PTR(rc);
>  
> -	rc = devm_add_action_or_reset(cxlds->dev, cxl_memdev_unregister,
> -				      no_free_ptr(cxlmd));
> -	if (rc)
> -		return ERR_PTR(rc);
> -	return cxlmd;
> +	return cxl_memdev_autoremove(no_free_ptr(cxlmd));
>  }
>  EXPORT_SYMBOL_FOR_MODULES(__devm_cxl_add_memdev, "cxl_mem");
>  
> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
> index 677996c65272..d61f121c6f97 100644
> --- a/drivers/cxl/mem.c
> +++ b/drivers/cxl/mem.c
> @@ -142,6 +142,12 @@ static int cxl_mem_probe(struct device *dev)
>  			return rc;
>  	}
>  
> +	if (cxlmd->ops) {
> +		rc = cxlmd->ops->probe(cxlmd);
> +		if (rc)
> +			return rc;
> +	}
> +
>  	rc = devm_cxl_memdev_edac_register(cxlmd);
>  	if (rc)
>  		dev_dbg(dev, "CXL memdev EDAC registration failed rc=%d\n", rc);
> @@ -166,6 +172,7 @@ static int cxl_mem_probe(struct device *dev)
>  /**
>   * devm_cxl_add_memdev - Add a CXL memory device
>   * @cxlds: CXL device state to associate with the memdev
> + * @ops: optional operations to run in cxl_mem::{probe,remove}() context
>   *
>   * Upon return the device will have had a chance to attach to the
>   * cxl_mem driver, but may fail if the CXL topology is not ready
> @@ -174,9 +181,10 @@ static int cxl_mem_probe(struct device *dev)
>   * The parent of the resulting device and the devm context for allocations is
>   * @cxlds->dev.
>   */
> -struct cxl_memdev *devm_cxl_add_memdev(struct cxl_dev_state *cxlds)
> +struct cxl_memdev *devm_cxl_add_memdev(struct cxl_dev_state *cxlds,
> +				       const struct cxl_memdev_ops *ops)
>  {
> -	return __devm_cxl_add_memdev(cxlds);
> +	return __devm_cxl_add_memdev(cxlds, ops);
>  }
>  EXPORT_SYMBOL_NS_GPL(devm_cxl_add_memdev, "CXL");
>  
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index 1c6fc5334806..549368a9c868 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -1006,7 +1006,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	if (rc)
>  		dev_dbg(&pdev->dev, "No CXL Features discovered\n");
>  
> -	cxlmd = devm_cxl_add_memdev(cxlds);
> +	cxlmd = devm_cxl_add_memdev(cxlds, NULL);
>  	if (IS_ERR(cxlmd))
>  		return PTR_ERR(cxlmd);
>  
> diff --git a/tools/testing/cxl/test/mem.c b/tools/testing/cxl/test/mem.c
> index 8a22b7601627..cb87e8c0e63c 100644
> --- a/tools/testing/cxl/test/mem.c
> +++ b/tools/testing/cxl/test/mem.c
> @@ -1767,7 +1767,7 @@ static int cxl_mock_mem_probe(struct platform_device *pdev)
>  
>  	cxl_mock_add_event_logs(&mdata->mes);
>  
> -	cxlmd = devm_cxl_add_memdev(cxlds);
> +	cxlmd = devm_cxl_add_memdev(cxlds, NULL);
>  	if (IS_ERR(cxlmd))
>  		return PTR_ERR(cxlmd);
>  

Re: [PATCH 0/6] cxl: Initialization reworks in support Soft Reserve Recovery and Accelerator Memory

[Alejandro Lucero Palau]

On 12/4/25 02:21, Dan Williams wrote:
> The CXL subsystem is modular. That modularity is a benefit for
> separation of concerns and testing. It is generally appropriate for this
> class of devices that support hotplug and can dynamically add a CXL
> personality alongside their PCI personality. However, a cost of modules
> is ambiguity about when devices (cxl_memdevs, cxl_ports, cxl_regions)
> have had a chance to attach to their corresponding drivers on
> @cxl_bus_type.
>
> This problem of not being able to reliably determine when a device has
> had a chance to attach to its driver vs still waiting for the module to
> load, is a common problem for the "Soft Reserve Recovery" [1], and
> "Accelerator Memory" [2] enabling efforts.
>
> For "Soft Reserve Recovery" it wants to use wait_for_device_probe() as a
> sync point for when CXL devices present at boot have had a chance to
> attach to the cxl_pci driver (generic CXL memory expansion class
> driver). That breaks down if wait_for_device_probe() only flushes PCI
> device probe, but not the cxl_mem_probe() of the cxl_memdev that
> cxl_pci_probe() creates.
>
> For "Accelerator Memory", the driver is not cxl_pci, but any potential
> PCI driver that wants to use the devm_cxl_add_memdev() ABI to attach to
> the CXL memory domain. Those drivers want to know if the CXL link is
> live end-to-end (from endpoint, through switches, to the host bridge)
> and CXL memory operations are enabled. If not, a CXL accelerator may be
> able to fall back to PCI-only operation. Similar to the "Soft Reserve
> Memory" it needs to know that the CXL subsystem had a chance to probe
> the ancestor topology of the device and let that driver make a
> synchronous decision about CXL operation.


IMO, this is not the problem with accelerators, because this can not be 
dynamically done, or not easily. The HW will support CXL or PCI, and if 
CXL mem is not enabled by the firmware, likely due to a 
negotiation/linking problem, the driver can keep going with CXL.io. Of 
course, this is from my experience with sfc driver/hardware. Note sfc 
driver added the check for CXL availability based on Terry's v13.


But this is useful for solving the problem of module removal which can 
leave the type2 driver without the base for doing any unwinding. Once a 
type2 uses code from those other cxl modules explicitly, the problem is 
avoided. You seem to have forgotten about this problem, what I think it 
is worth to describe.


>
> In support of those efforts:
>
> * Clean up some resource lifetime issues in the current code
> * Move some object creation symbols (devm_cxl_add_memdev() and
>    devm_cxl_add_endpoint()) into the cxl_mem.ko and cxl_port.ko objects.
>    Implicitly guarantee that cxl_mem_driver and cxl_port_driver have been
>    registered prior to any device objects being registered. This is
>    preferred over explicit open-coded request_module().
> * Use scoped-based-cleanup before adding more resource management in
>    devm_cxl_add_memdev()
> * Give an accelerator the opportunity to run setup operations in
>    cxl_mem_probe() so it can further probe if the CXL configuration matches
>    its needs.
>
> Some of these previously appeared on a branch as an RFC [3] and left
> "Soft Reserve Recovery" and "Accelerator Memory" to jockey for ordering.
> Instead, create a shared topic branch for both of those efforts to
> import. The main changes since that RFC are fixing a bug and reducing
> the amount of refactoring (which contributed to hiding the bug).
>
> [1]: http://lore.kernel.org/20251120031925.87762-1-Smita.KoralahalliChannabasappa@amd.com
> [2]: http://lore.kernel.org/20251119192236.2527305-1-alejandro.lucero-palau@amd.com
> [3]: https://git.kernel.org/pub/scm/linux/kernel/git/cxl/cxl.git/log/?h=for-6.18/cxl-probe-order
>
> Dan Williams (6):
>    cxl/mem: Fix devm_cxl_memdev_edac_release() confusion
>    cxl/mem: Arrange for always-synchronous memdev attach
>    cxl/port: Arrange for always synchronous endpoint attach
>    cxl/mem: Convert devm_cxl_add_memdev() to scope-based-cleanup
>    cxl/mem: Drop @host argument to devm_cxl_add_memdev()
>    cxl/mem: Introduce a memdev creation ->probe() operation
>
>   drivers/cxl/Kconfig          |   2 +-
>   drivers/cxl/cxl.h            |   2 +
>   drivers/cxl/cxlmem.h         |  17 ++++--
>   drivers/cxl/core/edac.c      |  64 ++++++++++++---------
>   drivers/cxl/core/memdev.c    | 104 ++++++++++++++++++++++++-----------
>   drivers/cxl/mem.c            |  69 +++++++++--------------
>   drivers/cxl/pci.c            |   2 +-
>   drivers/cxl/port.c           |  40 ++++++++++++++
>   tools/testing/cxl/test/mem.c |   2 +-
>   9 files changed, 192 insertions(+), 110 deletions(-)
>
>
> base-commit: ea5514e300568cbe8f19431c3e424d4791db8291

Re: [PATCH 0/6] cxl: Initialization reworks in support Soft Reserve Recovery and Accelerator Memory

[dan.j.williams]

Alejandro Lucero Palau wrote:
[..]
> > For "Accelerator Memory", the driver is not cxl_pci, but any potential
> > PCI driver that wants to use the devm_cxl_add_memdev() ABI to attach to
> > the CXL memory domain. Those drivers want to know if the CXL link is
> > live end-to-end (from endpoint, through switches, to the host bridge)
> > and CXL memory operations are enabled. If not, a CXL accelerator may be
> > able to fall back to PCI-only operation. Similar to the "Soft Reserve
> > Memory" it needs to know that the CXL subsystem had a chance to probe
> > the ancestor topology of the device and let that driver make a
> > synchronous decision about CXL operation.
> 
> 
> IMO, this is not the problem with accelerators, because this can not be 
> dynamically done, or not easily.

Hmm, what do you mean can not be dynamically done? The observation is
that a CXL card and its driver have no idea if the card is going to be
plugged into a PCIe only slot.

At runtime the driver only finds out the CXL is not there from the
result of devm_cxl_add_memdev().

> The HW will support CXL or PCI, and if 
> CXL mem is not enabled by the firmware, likely due to a 
> negotiation/linking problem, the driver can keep going with CXL.io.

Right, I think we are in violent agreement.

> Of course, this is from my experience with sfc driver/hardware. Note
> sfc driver added the check for CXL availability based on Terry's v13.

Note that Terry's check for CXL availabilty is purely a hardware
detection, there are still software reasons why cxl_acpi and cxl_mem
can prevent devm_cxl_add_memdev() success.

> But this is useful for solving the problem of module removal which can 
> leave the type2 driver without the base for doing any unwinding. Once a 
> type2 uses code from those other cxl modules explicitly, the problem is 
> avoided. You seem to have forgotten about this problem, what I think it 
> is worth to describe.

What problem exactly? If it needs to be captured in these changelogs or
code comments, let me know.

Re: [PATCH 0/6] cxl: Initialization reworks in support Soft Reserve Recovery and Accelerator Memory

[Alejandro Lucero Palau]

On 12/5/25 21:17, dan.j.williams@intel.com wrote:
> Alejandro Lucero Palau wrote:
> [..]
>>> For "Accelerator Memory", the driver is not cxl_pci, but any potential
>>> PCI driver that wants to use the devm_cxl_add_memdev() ABI to attach to
>>> the CXL memory domain. Those drivers want to know if the CXL link is
>>> live end-to-end (from endpoint, through switches, to the host bridge)
>>> and CXL memory operations are enabled. If not, a CXL accelerator may be
>>> able to fall back to PCI-only operation. Similar to the "Soft Reserve
>>> Memory" it needs to know that the CXL subsystem had a chance to probe
>>> the ancestor topology of the device and let that driver make a
>>> synchronous decision about CXL operation.
>>
>> IMO, this is not the problem with accelerators, because this can not be
>> dynamically done, or not easily.
> Hmm, what do you mean can not be dynamically done? The observation is
> that a CXL card and its driver have no idea if the card is going to be
> plugged into a PCIe only slot.


Right.


>
> At runtime the driver only finds out the CXL is not there from the
> result of devm_cxl_add_memdev().


If there is no CXL properly initialized, what also implies a PCI-only 
slot, the driver can know looking at the CXL.mem and CXL.cache status in 
the CXL control register. That is what sfc driver does now using Terry's 
patchset instead of only checking CXL DVSEC and trying further CXL 
initialization using the CXL core API for Type2. Neither call to create 
cxl dev state nor memdev is needed to figure out. Of course, those calls 
can point to another kind of problem, but the driver can find out 
without using them.


>> The HW will support CXL or PCI, and if
>> CXL mem is not enabled by the firmware, likely due to a
>> negotiation/linking problem, the driver can keep going with CXL.io.
> Right, I think we are in violent agreement.
>
>> Of course, this is from my experience with sfc driver/hardware. Note
>> sfc driver added the check for CXL availability based on Terry's v13.
> Note that Terry's check for CXL availabilty is purely a hardware
> detection, there are still software reasons why cxl_acpi and cxl_mem
> can prevent devm_cxl_add_memdev() success.
>
>> But this is useful for solving the problem of module removal which can
>> leave the type2 driver without the base for doing any unwinding. Once a
>> type2 uses code from those other cxl modules explicitly, the problem is
>> avoided. You seem to have forgotten about this problem, what I think it
>> is worth to describe.
> What problem exactly? If it needs to be captured in these changelogs or
> code comments, let me know.


It is a surprise you not remembering this ...


v17 tried to fix this problem which was pointed out in v16 by you in 
several patches.


v17:

https://lore.kernel.org/linux-cxl/6887b72724173_11968100cb@dwillia2-mobl4.notmuch/


Next my reply to another comment from you trying to clarify/enumerate 
different problems which were getting intertwined creating confusion (at 
least to me). Sadly none did comment further, likely none read my 
explanation ... even if I asked for it with another email and 
specifically in one community meeting:

https://lore.kernel.org/linux-cxl/836d06d6-a36f-4ba3-b7c9-ba8687ba2190@amd.com/


Next discussion about trying to solve the modules removal adding a 
callback by the driver which you did not like:

https://lore.kernel.org/linux-cxl/6892325deccdb_55f09100fb@dwillia2-xfh.jf.intel.com.notmuch/


Here, v16, you stated specifically about cxl kernel modules removed 
while a Type2 driver is using cxl:

https://lore.kernel.org/linux-cxl/682e2a0b9b15b_1626e10088@dwillia2-xfh.jf.intel.com.notmuch/

https://lore.kernel.org/linux-cxl/682e300371a0_1626e1003@dwillia2-xfh.jf.intel.com.notmuch/

https://lore.kernel.org/linux-cxl/682e3f3343977_1626e100b0@dwillia2-xfh.jf.intel.com.notmuch/

Re: [PATCH 0/6] cxl: Initialization reworks in support Soft Reserve Recovery and Accelerator Memory

[dan.j.williams]

Alejandro Lucero Palau wrote:
[..]
> If there is no CXL properly initialized, what also implies a PCI-only 
> slot, the driver can know looking at the CXL.mem and CXL.cache status in 
> the CXL control register. That is what sfc driver does now using Terry's 
> patchset instead of only checking CXL DVSEC and trying further CXL 
> initialization using the CXL core API for Type2. Neither call to create 
> cxl dev state nor memdev is needed to figure out. Of course, those calls 
> can point to another kind of problem, but the driver can find out 
> without using them.

It can, but I am not sure why a driver would want to open code a partial
answer to that question and not just rely on the CXL core to do the full
determination?

> >> The HW will support CXL or PCI, and if
> >> CXL mem is not enabled by the firmware, likely due to a
> >> negotiation/linking problem, the driver can keep going with CXL.io.
> > Right, I think we are in violent agreement.
> >
> >> Of course, this is from my experience with sfc driver/hardware. Note
> >> sfc driver added the check for CXL availability based on Terry's v13.
> > Note that Terry's check for CXL availabilty is purely a hardware
> > detection, there are still software reasons why cxl_acpi and cxl_mem
> > can prevent devm_cxl_add_memdev() success.
> >
> >> But this is useful for solving the problem of module removal which can
> >> leave the type2 driver without the base for doing any unwinding. Once a
> >> type2 uses code from those other cxl modules explicitly, the problem is
> >> avoided. You seem to have forgotten about this problem, what I think it
> >> is worth to describe.
> > What problem exactly? If it needs to be captured in these changelogs or
> > code comments, let me know.
> 
> 
> It is a surprise you not remembering this ...

I did not immediately recognize that this statement: "problem of module
removal which can leave the type2 driver without the base for doing any
unwinding". This set is about init time fixes so talking about removal
through me for a loop.

Thanks for the additional context below.

> v17 tried to fix this problem which was pointed out in v16 by you in 
> several patches.
> 
> 
> v17:
> 
> https://lore.kernel.org/linux-cxl/6887b72724173_11968100cb@dwillia2-mobl4.notmuch/
> 
> Next my reply to another comment from you trying to clarify/enumerate 
> different problems which were getting intertwined creating confusion (at 
> least to me). Sadly none did comment further, likely none read my 
> explanation ... even if I asked for it with another email and 
> specifically in one community meeting:
> 
> https://lore.kernel.org/linux-cxl/836d06d6-a36f-4ba3-b7c9-ba8687ba2190@amd.com/

So this also is about the init race, not removal, right?

This is why I think Smita's patches are a precursor to Type-2 because
both need that sync-point to when that platform CXL initialization has
completed.

> Next discussion about trying to solve the modules removal adding a 
> callback by the driver which you did not like:
> 
> https://lore.kernel.org/linux-cxl/6892325deccdb_55f09100fb@dwillia2-xfh.jf.intel.com.notmuch/
> 

A proposal that implements what I talk about there is something like
this:

diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
index 51a07cd85c7b..a4cb6d0f0da7 100644
--- a/drivers/cxl/core/memdev.c
+++ b/drivers/cxl/core/memdev.c
@@ -641,6 +641,16 @@ static void detach_memdev(struct work_struct *work)
 	struct cxl_memdev *cxlmd;
 
 	cxlmd = container_of(work, typeof(*cxlmd), detach_work);
+
+	/*
+	 * Default to detaching the memdev, but in the case of memdev ops the
+	 * memdev creator may want to detach the parent device as well.
+	 */
+	if (cxlmd->ops && cxlmd->ops->detach) {
+		cxlmd->ops->detach(cxlmd);
+		return;
+	}
+
 	device_release_driver(&cxlmd->dev);
 	put_device(&cxlmd->dev);
 }

Where that detach implementation is something like:

void accelerator_driver_detach(struct cxl_memdev *cxlmd)
{
	device_release_driver(cxlmd->dev.parent);
	/* the above also detaches the cxlmd via devm action */
	put_device(&cxlmd->dev);
}

What I am not sure about is whether the accelerator driver needs the
ability to do anything besides shutdown when the CXL hierarchy is torn
down. I.e. no ->detach() callback, just make it the rule that when @ops
are specified devm_cxl_add_memdev() failure is a permanent failure at
registration time and CXL hierarchy removal also takes down the
accelerator driver.

Re: [PATCH 0/6] cxl: Initialization reworks in support Soft Reserve Recovery and Accelerator Memory

[Alejandro Lucero Palau]

On 12/4/25 02:21, Dan Williams wrote:
> The CXL subsystem is modular. That modularity is a benefit for
> separation of concerns and testing. It is generally appropriate for this
> class of devices that support hotplug and can dynamically add a CXL
> personality alongside their PCI personality. However, a cost of modules
> is ambiguity about when devices (cxl_memdevs, cxl_ports, cxl_regions)
> have had a chance to attach to their corresponding drivers on
> @cxl_bus_type.
>
> This problem of not being able to reliably determine when a device has
> had a chance to attach to its driver vs still waiting for the module to
> load, is a common problem for the "Soft Reserve Recovery" [1], and
> "Accelerator Memory" [2] enabling efforts.
>
> For "Soft Reserve Recovery" it wants to use wait_for_device_probe() as a
> sync point for when CXL devices present at boot have had a chance to
> attach to the cxl_pci driver (generic CXL memory expansion class
> driver). That breaks down if wait_for_device_probe() only flushes PCI
> device probe, but not the cxl_mem_probe() of the cxl_memdev that
> cxl_pci_probe() creates.
>
> For "Accelerator Memory", the driver is not cxl_pci, but any potential
> PCI driver that wants to use the devm_cxl_add_memdev() ABI to attach to
> the CXL memory domain. Those drivers want to know if the CXL link is
> live end-to-end (from endpoint, through switches, to the host bridge)
> and CXL memory operations are enabled. If not, a CXL accelerator may be
> able to fall back to PCI-only operation. Similar to the "Soft Reserve
> Memory" it needs to know that the CXL subsystem had a chance to probe
> the ancestor topology of the device and let that driver make a
> synchronous decision about CXL operation.
>
> In support of those efforts:
>
> * Clean up some resource lifetime issues in the current code
> * Move some object creation symbols (devm_cxl_add_memdev() and
>    devm_cxl_add_endpoint()) into the cxl_mem.ko and cxl_port.ko objects.
>    Implicitly guarantee that cxl_mem_driver and cxl_port_driver have been
>    registered prior to any device objects being registered. This is
>    preferred over explicit open-coded request_module().
> * Use scoped-based-cleanup before adding more resource management in
>    devm_cxl_add_memdev()
> * Give an accelerator the opportunity to run setup operations in
>    cxl_mem_probe() so it can further probe if the CXL configuration matches
>    its needs.
>
> Some of these previously appeared on a branch as an RFC [3] and left
> "Soft Reserve Recovery" and "Accelerator Memory" to jockey for ordering.
> Instead, create a shared topic branch for both of those efforts to
> import. The main changes since that RFC are fixing a bug and reducing
> the amount of refactoring (which contributed to hiding the bug).
>
> [1]: http://lore.kernel.org/20251120031925.87762-1-Smita.KoralahalliChannabasappa@amd.com
> [2]: http://lore.kernel.org/20251119192236.2527305-1-alejandro.lucero-palau@amd.com
> [3]: https://git.kernel.org/pub/scm/linux/kernel/git/cxl/cxl.git/log/?h=for-6.18/cxl-probe-order
>
> Dan Williams (6):
>    cxl/mem: Fix devm_cxl_memdev_edac_release() confusion
>    cxl/mem: Arrange for always-synchronous memdev attach
>    cxl/port: Arrange for always synchronous endpoint attach
>    cxl/mem: Convert devm_cxl_add_memdev() to scope-based-cleanup
>    cxl/mem: Drop @host argument to devm_cxl_add_memdev()
>    cxl/mem: Introduce a memdev creation ->probe() operation
>
>   drivers/cxl/Kconfig          |   2 +-
>   drivers/cxl/cxl.h            |   2 +
>   drivers/cxl/cxlmem.h         |  17 ++++--
>   drivers/cxl/core/edac.c      |  64 ++++++++++++---------
>   drivers/cxl/core/memdev.c    | 104 ++++++++++++++++++++++++-----------
>   drivers/cxl/mem.c            |  69 +++++++++--------------
>   drivers/cxl/pci.c            |   2 +-
>   drivers/cxl/port.c           |  40 ++++++++++++++
>   tools/testing/cxl/test/mem.c |   2 +-
>   9 files changed, 192 insertions(+), 110 deletions(-)
>
>
> base-commit: ea5514e300568cbe8f19431c3e424d4791db8291


I have tested this series with Type2 and it works as expected:


1) type2 fails to be installed if cxl_mem not loaded.

2) cxl_mem, cxl_port and cxl_core not removable as type2 driver 
installed depends on them.

3) cxl_acpi is still possible to remove ...


FWIW:


Tested-by: Alejandro Lucero <alucerop@amd.com>

Re: [PATCH 0/6] cxl: Initialization reworks in support Soft Reserve Recovery and Accelerator Memory

[dan.j.williams]

Alejandro Lucero Palau wrote:
[..]
> > Dan Williams (6):
> >    cxl/mem: Fix devm_cxl_memdev_edac_release() confusion
> >    cxl/mem: Arrange for always-synchronous memdev attach
> >    cxl/port: Arrange for always synchronous endpoint attach
> >    cxl/mem: Convert devm_cxl_add_memdev() to scope-based-cleanup
> >    cxl/mem: Drop @host argument to devm_cxl_add_memdev()
> >    cxl/mem: Introduce a memdev creation ->probe() operation
> >
> >   drivers/cxl/Kconfig          |   2 +-
> >   drivers/cxl/cxl.h            |   2 +
> >   drivers/cxl/cxlmem.h         |  17 ++++--
> >   drivers/cxl/core/edac.c      |  64 ++++++++++++---------
> >   drivers/cxl/core/memdev.c    | 104 ++++++++++++++++++++++++-----------
> >   drivers/cxl/mem.c            |  69 +++++++++--------------
> >   drivers/cxl/pci.c            |   2 +-
> >   drivers/cxl/port.c           |  40 ++++++++++++++
> >   tools/testing/cxl/test/mem.c |   2 +-
> >   9 files changed, 192 insertions(+), 110 deletions(-)
> >
> >
> > base-commit: ea5514e300568cbe8f19431c3e424d4791db8291
> 
> 
> I have tested this series with Type2 and it works as expected:
> 
> 
> 1) type2 fails to be installed if cxl_mem not loaded.
> 
> 2) cxl_mem, cxl_port and cxl_core not removable as type2 driver 
> installed depends on them.
> 
> 3) cxl_acpi is still possible to remove ...
> 
> 
> FWIW:
> 
> 
> Tested-by: Alejandro Lucero <alucerop@amd.com>

Thanks.

Do note that module removability does not affect the unbind path. So you
can still do all the same "remove" violence via:

"echo $device > /sys/bus/cxl/driver/$driver/unbind"

...as would be caused by:

"modprobe -r $driver"

...i.e. nothing blocks unbind even if the module is pinned.