[RFC PATCH v2 1/3] cxl/mem: Separate provider registration from region attachment

[Richard Cheng <icheng@nvidia.com>]

devm_cxl_probe_mem() requires a committed region when registering a
provider-owned memdev. If FW has not created a region, registration
fails before the provider can use the endpoint topology to provision
one.

Make the attachment probe callback optional while retaining a non-NULL
descriptor to preserve provider ownership and handling of CXL link loss.
Install endpoint cleanup for FW-discovered provider regions even when
the provider never requests attachment.

This establishes the registration and attachment APIs needed for
explicit region provisioning without introducing region creation or
allocation policy.

Signed-off-by: Richard Cheng <icheng@nvidia.com>
---
Changelog:

v1 -> v2:
- Rework the patch into a registration/attachment refactor, following
  Alejandro's request to separate region provisioning from memdev
registration.
- Add devm_cxl_register_mem() to establish a provider-owned memdev and
  its endpoint without requiring a comitted region.
- Add devm_cxl_attach_mem_region() to obtain the HPA range of an
  existing committed, single-target region after registration.
- Make the region probe callback optional while preserving provider
  ownership, synchronous endpoint setup, and hdling of CXL link loss.
---
 drivers/cxl/core/region.c | 146 ++++++++++++++++++++++++++------------
 drivers/cxl/cxlmem.h      |  22 ++++--
 drivers/cxl/mem.c         |  69 +++++++++++++++++-
 include/cxl/cxl.h         |   2 +
 4 files changed, 188 insertions(+), 51 deletions(-)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 5ef0ca0694ff..7a64a730587d 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -4111,67 +4111,123 @@ static int first_mapped_decoder(struct device *dev, const void *data)
 	return 0;
 }
 
-/*
- * Runs in cxl_mem_probe context after successful endpoint probe, assumes the
- * simple case of single mapped decoder per memdev.
- */
-int cxl_memdev_attach_region(struct cxl_memdev *cxlmd)
+static int unregister_memdev_region(struct device *dev, void *data)
 {
-	struct cxl_attach_region *attach =
-		container_of(cxlmd->attach, typeof(*attach), attach);
-	struct cxl_port *endpoint = cxlmd->endpoint;
 	struct cxl_endpoint_decoder *cxled;
 	struct cxl_region *cxlr;
-	int rc;
 
-	/* hold endpoint lock to setup autoremove of the region */
+	if (!is_endpoint_decoder(dev))
+		return 0;
+
+	cxled = to_cxl_endpoint_decoder(dev);
+	scoped_guard(rwsem_read, &cxl_rwsem.region) {
+		cxlr = cxled->cxld.region;
+		if (!cxlr)
+			return 0;
+		get_device(&cxlr->dev);
+	}
+
+	/* Unregistration needs the region write lock. */
+	endpoint_unregister_region(cxlr);
+	return 0;
+}
+
+static void endpoint_unregister_regions(void *data)
+{
+	struct cxl_port *endpoint = data;
+
+	device_for_each_child(&endpoint->dev, NULL, unregister_memdev_region);
+}
+
+int cxl_memdev_setup_region_cleanup(struct cxl_memdev *cxlmd)
+{
+	struct cxl_port *endpoint = cxlmd->endpoint;
+
+	device_lock_assert(&cxlmd->dev);
+	if (IS_ERR_OR_NULL(endpoint))
+		return -ENXIO;
+
 	guard(device)(&endpoint->dev);
-	if (!endpoint->dev.driver)
+	if (!endpoint->dev.driver || endpoint->dead)
 		return -ENXIO;
-	guard(rwsem_read)(&cxl_rwsem.region);
-	guard(rwsem_read)(&cxl_rwsem.dpa);
 
 	/*
-	 * TODO auto-instantiate a region, for now assume this will find an
-	 * auto-region
+	 * Endpoint probe may discover provider-owned firmware regions even if
+	 * the provider never requests their HPA range. Run before decoder
+	 * teardown so those regions are unregistered, not just detached.
 	 */
-	struct device *dev __free(put_device) =
-		device_find_child(&endpoint->dev, NULL, first_mapped_decoder);
-
-	if (!dev) {
-		dev_dbg(cxlmd->cxlds->dev, "no region found for memdev %s\n",
-			dev_name(&cxlmd->dev));
-		return -ENXIO;
-	}
+	return devm_add_action_or_reset(&endpoint->dev,
+				      endpoint_unregister_regions, endpoint);
+}
+EXPORT_SYMBOL_FOR_MODULES(cxl_memdev_setup_region_cleanup, "cxl_mem");
 
-	cxled = to_cxl_endpoint_decoder(dev);
-	cxlr = cxled->cxld.region;
+/* Caller holds the memdev lock; attach to a single mapped decoder. */
+int cxl_memdev_attach_region(struct cxl_memdev *cxlmd, struct range *hpa_range)
+{
+	struct cxl_port *endpoint = cxlmd->endpoint;
+	struct cxl_endpoint_decoder *cxled;
+	struct cxl_region *cxlr;
+	int rc;
 
-	if (cxlr->params.state < CXL_CONFIG_COMMIT) {
-		dev_dbg(cxlmd->cxlds->dev,
-			"region %s not committed for memdev %s\n",
-			dev_name(&cxlr->dev), dev_name(&cxlmd->dev));
+	device_lock_assert(&cxlmd->dev);
+	if (IS_ERR_OR_NULL(endpoint))
 		return -ENXIO;
-	}
 
-	if (cxlr->params.nr_targets > 1) {
-		dev_dbg(cxlmd->cxlds->dev,
-			"Only attach to local non-interleaved region\n");
+	/* hold endpoint lock to setup autoremove of the region */
+	guard(device)(&endpoint->dev);
+	if (!endpoint->dev.driver || endpoint->dead)
 		return -ENXIO;
-	}
 
-	/* Only teardown regions that pass validation, ignore the rest */
-	get_device(&cxlr->dev);
-	rc = devm_add_action_or_reset(&endpoint->dev,
-				      endpoint_unregister_region, cxlr);
-	if (rc)
-		return rc;
+	scoped_guard(rwsem_read, &cxl_rwsem.region) {
+		guard(rwsem_read)(&cxl_rwsem.dpa);
 
-	attach->hpa_range = (struct range) {
-		.start = cxlr->params.res->start,
-		.end = cxlr->params.res->end,
-	};
-	return 0;
+		struct device *dev __free(put_device) =
+			device_find_child(&endpoint->dev, NULL,
+					  first_mapped_decoder);
+		if (!dev) {
+			dev_dbg(cxlmd->cxlds->dev,
+				"no region found for memdev %s\n",
+				dev_name(&cxlmd->dev));
+			return -ENXIO;
+		}
+
+		cxled = to_cxl_endpoint_decoder(dev);
+		cxlr = cxled->cxld.region;
+		if (cxlr->params.state < CXL_CONFIG_COMMIT) {
+			dev_dbg(cxlmd->cxlds->dev,
+				"region %s not committed for memdev %s\n",
+				dev_name(&cxlr->dev), dev_name(&cxlmd->dev));
+			return -ENXIO;
+		}
+
+		if (cxlr->params.nr_targets > 1) {
+			dev_dbg(cxlmd->cxlds->dev,
+				"Only attach to local non-interleaved region\n");
+			return -ENXIO;
+		}
+		if (!cxlr->params.res)
+			return -ENXIO;
+
+		/* Only teardown regions that pass validation, ignore the rest. */
+		if (!devm_is_action_added(&endpoint->dev,
+					  endpoint_unregister_region, cxlr)) {
+			get_device(&cxlr->dev);
+			rc = devm_add_action(&endpoint->dev,
+					     endpoint_unregister_region, cxlr);
+			if (rc)
+				break;
+		}
+
+		*hpa_range = (struct range) {
+			.start = cxlr->params.res->start,
+			.end = cxlr->params.res->end,
+		};
+		return 0;
+	}
+
+	/* devm_add_action() failed; teardown needs the region write lock. */
+	endpoint_unregister_region(cxlr);
+	return rc;
 }
 EXPORT_SYMBOL_FOR_MODULES(cxl_memdev_attach_region, "cxl_mem");
 
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index c401e3a1af06..8c050bc308bd 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -97,6 +97,13 @@ static inline bool is_cxl_endpoint(struct cxl_port *port)
 	return is_cxl_memdev(port->uport_dev);
 }
 
+/**
+ * struct cxl_memdev_attach - provider ownership and CXL link requirements
+ * @probe: optional region probe callback, called with the memdev locked
+ *
+ * A non-NULL descriptor requires successful synchronous endpoint setup and
+ * preserves provider ownership even when no region probe is requested.
+ */
 struct cxl_memdev_attach {
 	int (*probe)(struct cxl_memdev *cxlmd);
 };
@@ -107,8 +114,8 @@ struct cxl_memdev_attach {
  * @hpa_range: physical address range of the region
  *
  * For the common simple case of a CXL device with private (non-general purpose
- * / "accelerator") memory, enumerate firmware instantiated region, or
- * instantiate a region for the device's capacity. Destroy the region on detach.
+ * / "accelerator") memory, enumerate a firmware-instantiated region and
+ * report its range. Destroy the region on detach.
  */
 struct cxl_attach_region {
 	struct cxl_memdev_attach attach;
@@ -116,12 +123,19 @@ struct cxl_attach_region {
 };
 
 #ifdef CONFIG_CXL_REGION
-int cxl_memdev_attach_region(struct cxl_memdev *cxlmd);
+int cxl_memdev_attach_region(struct cxl_memdev *cxlmd, struct range *hpa_range);
+int cxl_memdev_setup_region_cleanup(struct cxl_memdev *cxlmd);
 #else
-static inline int cxl_memdev_attach_region(struct cxl_memdev *cxlmd)
+static inline int cxl_memdev_attach_region(struct cxl_memdev *cxlmd,
+					   struct range *hpa_range)
 {
 	return -EOPNOTSUPP;
 }
+
+static inline int cxl_memdev_setup_region_cleanup(struct cxl_memdev *cxlmd)
+{
+	return 0;
+}
 #endif
 
 struct cxl_memdev *devm_cxl_add_classdev(struct cxl_dev_state *cxlds);
diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
index 3959ec963026..aa08d88ab104 100644
--- a/drivers/cxl/mem.c
+++ b/drivers/cxl/mem.c
@@ -172,7 +172,10 @@ static int cxl_mem_probe(struct device *dev)
 	}
 
 	if (cxlmd->attach) {
-		rc = cxlmd->attach->probe(cxlmd);
+		if (cxlmd->attach->probe)
+			rc = cxlmd->attach->probe(cxlmd);
+		else
+			rc = cxl_memdev_setup_region_cleanup(cxlmd);
 		if (rc)
 			return rc;
 	}
@@ -215,6 +218,68 @@ struct cxl_memdev *devm_cxl_add_classdev(struct cxl_dev_state *cxlds)
 }
 EXPORT_SYMBOL_NS_GPL(devm_cxl_add_classdev, "CXL");
 
+/**
+ * devm_cxl_register_mem - Register a provider-owned CXL memory device
+ * @cxlds: CXL device state to associate with the memdev
+ *
+ * Establish the CXL port topology and endpoint synchronously, without requiring
+ * a committed region. The provider retains ownership of its memory, including
+ * any firmware-discovered regions, and must detach if the CXL link is lost.
+ *
+ * The parent of the resulting device and the devm context for allocations is
+ * @cxlds->dev. Returns the registered memdev or an ERR_PTR() on failure.
+ */
+struct cxl_memdev *devm_cxl_register_mem(struct cxl_dev_state *cxlds)
+{
+	struct cxl_memdev_attach *attach;
+
+	attach = devm_kzalloc(cxlds->dev, sizeof(*attach), GFP_KERNEL);
+	if (!attach)
+		return ERR_PTR(-ENOMEM);
+
+	return __devm_cxl_add_memdev(cxlds, attach);
+}
+EXPORT_SYMBOL_NS_GPL(devm_cxl_register_mem, "CXL");
+
+/**
+ * devm_cxl_attach_mem_region - Attach a registered memdev to its region
+ * @cxlmd: provider-owned memdev returned by devm_cxl_register_mem()
+ * @hpa_range: CXL.mem physical address range result
+ *
+ * Attach to an existing committed, single-target region. This does not create
+ * or program a region. Repeated attachment to the same region returns the same
+ * range without adding another cleanup action. Failure leaves the memdev
+ * registered so that the provider can decide how to proceed.
+ *
+ * The region is removed when the endpoint detaches. Returns zero on success or
+ * a negative errno; @hpa_range is empty on failure.
+ */
+int devm_cxl_attach_mem_region(struct cxl_memdev *cxlmd,
+			       struct range *hpa_range)
+{
+	if (!hpa_range)
+		return -EINVAL;
+	*hpa_range = DEFINE_RANGE(0, -1);
+
+	if (!cxlmd->attach)
+		return -EINVAL;
+
+	guard(device)(&cxlmd->dev);
+	if (!cxlmd->dev.driver || !cxlmd->cxlds)
+		return -ENXIO;
+
+	return cxl_memdev_attach_region(cxlmd, hpa_range);
+}
+EXPORT_SYMBOL_NS_GPL(devm_cxl_attach_mem_region, "CXL");
+
+static int cxl_probe_mem_region(struct cxl_memdev *cxlmd)
+{
+	struct cxl_attach_region *attach =
+		container_of(cxlmd->attach, typeof(*attach), attach);
+
+	return cxl_memdev_attach_region(cxlmd, &attach->hpa_range);
+}
+
 /**
  * devm_cxl_probe_mem - Add a CXL memory device and probe its region
  * @cxlds: CXL device state to associate with the memdev
@@ -242,7 +307,7 @@ struct cxl_memdev *devm_cxl_probe_mem(struct cxl_dev_state *cxlds,
 
 	*attach = (struct cxl_attach_region) {
 		.attach = {
-			   .probe = cxl_memdev_attach_region,
+			   .probe = cxl_probe_mem_region,
 		},
 		.hpa_range = { 0, -1 },
 	};
diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h
index 802b143de83d..3019e3ea5f09 100644
--- a/include/cxl/cxl.h
+++ b/include/cxl/cxl.h
@@ -224,6 +224,8 @@ struct cxl_dev_state *_devm_cxl_dev_state_create(struct device *dev,
 						      sizeof(drv_struct), mbox);	\
 	})
 
+struct cxl_memdev *devm_cxl_register_mem(struct cxl_dev_state *cxlds);
+int devm_cxl_attach_mem_region(struct cxl_memdev *cxlmd, struct range *hpa_range);
 struct cxl_memdev *devm_cxl_probe_mem(struct cxl_dev_state *cxlds,
 				      struct range *range);
 
-- 
2.43.0