[RFC PATCH 0/3] cxl: Auto-create a region for Type-2 memdev attach

[RFC PATCH 0/3] cxl: Auto-create a region for Type-2 memdev attach

[Richard Cheng]

A Type-2 accelerator driver calls devm_cxl_probe_mem() to register its
memdev and get an HPA range, but today only if FW already committed a
region. Real accelerators can have usable memory with no committed decoder,
so get nothing.

If no FW region is mapped, the core picks the device's unused manual
DEVMEM decoder and a compatible x1 Type-2 RAM root, allocates the full
volatile DPA and HPA, commits the decoder, and returns the range. Unbind
resets and removes it. Strict first cut with single decoder, IW=1, minimum
granularity, first-compatible root.

The design intent is that the provider F_LOCKs its region against userspace
but must reset its own software region on detach. A plain flag would also
allow reset on generic kill/delete paths, so we thread a reset context
through teardown and commit rollback. devm_cxl_probe_mem() may now commit
decoders.

Testing result is in the following.
- Built clean with clang/LLVM on arm64
- cxl_test, type2_test=1. accel0 takes the unchanged attach path. accel1
  drives auto_create -> a committed 512 MB RAM region. The test asserts
  the 512 MB HPA range. committed state and 256 byte granularity confirmed
  via sysfs.
- Unbind tears the region down with no orphaned decoder, rebind re-creates
  a fresh committed region.
- Mock test only. Real accelerators whose FW commits a decoder take the
  attach path, and vfio-cxl binds only FW-committed devices, so auto-create
  has no real-HW caller yet.

Best regards,
Richard Cheng.

Richard Cheng (3):
  cxl/region: Reset software-created regions on memdev detach
  cxl/region: Auto-create a region for memdev attach
  cxl/test: Exercise Type-2 automatic region creation

 drivers/cxl/core/region.c      | 422 +++++++++++++++++++++++++++++----
 tools/testing/cxl/test/accel.c |   7 +
 tools/testing/cxl/test/cxl.c   |  61 ++++-
 3 files changed, 439 insertions(+), 51 deletions(-)


base-commit: 1c6b4ceafc3b994871c29340e0c1ddb0af5800e7
-- 
2.43.0

[RFC PATCH 1/3] cxl/region: Reset software-created regions on memdev detach

[Richard Cheng]

A memdev attach provider locks its region to prevent userspace
disposition. Future software-created Type-2 regions inherit that lock,
causing the existing reset path to treat them like FW-owned or HW-locked
regions and leave their decoder programming behind when the provider
detaches.

Pass a managed-detach context through region teardown. In that context,
allow reset only when the region is non-AUTO, owned by a memdev attach
provider, and has no HW-locked decoder in its path. This narrowly
permits the provider to reset its own software-created region without
changing generic teardown behavior.

Generic teardown remains unchanged, while FW-discovered AUTO regions and
genuinely locked decoders remain protected.

Signed-off-by: Richard Cheng <icheng@nvidia.com>
---
 drivers/cxl/core/region.c | 112 +++++++++++++++++++++++++++++++-------
 1 file changed, 91 insertions(+), 21 deletions(-)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 1e211542b6b6..7fcaddc61180 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -39,6 +39,7 @@
 static nodemask_t nodemask_region_seen = NODE_MASK_NONE;
 
 static struct cxl_region *to_cxl_region(struct device *dev);
+static bool cxl_region_has_memdev_attach(struct cxl_region *cxlr);
 
 #define __ACCESS_ATTR_RO(_level, _name) {				\
 	.attr	= { .name = __stringify(_name), .mode = 0444 },		\
@@ -222,6 +223,45 @@ static struct cxl_region_ref *cxl_rr_load(struct cxl_port *port,
 	return xa_load(&port->regions, (unsigned long)cxlr);
 }
 
+static bool cxl_region_has_locked_decoder(struct cxl_region *cxlr)
+{
+	struct cxl_region_params *p = &cxlr->params;
+	int i;
+
+	lockdep_assert_held_write(&cxl_rwsem.region);
+
+	if (cxlr->cxlrd->cxlsd.cxld.flags & CXL_DECODER_F_LOCK)
+		return true;
+
+	for (i = 0; i < p->interleave_ways; i++) {
+		struct cxl_endpoint_decoder *cxled = p->targets[i];
+		struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
+		struct cxl_port *iter = cxled_to_port(cxled);
+		struct cxl_dev_state *cxlds = cxlmd->cxlds;
+		struct cxl_ep *ep;
+
+		if (cxled->cxld.flags & CXL_DECODER_F_LOCK)
+			return true;
+
+		if (cxlds->rcd)
+			continue;
+
+		while (!is_cxl_root(to_cxl_port(iter->dev.parent)))
+			iter = to_cxl_port(iter->dev.parent);
+
+		for (ep = cxl_ep_load(iter, cxlmd); iter;
+		     iter = ep->next, ep = cxl_ep_load(iter, cxlmd)) {
+			struct cxl_region_ref *cxl_rr;
+
+			cxl_rr = cxl_rr_load(iter, cxlr);
+			if (cxl_rr->decoder->flags & CXL_DECODER_F_LOCK)
+				return true;
+		}
+	}
+
+	return false;
+}
+
 static int cxl_region_invalidate_memregion(struct cxl_region *cxlr)
 {
 	if (!cpu_cache_has_invalidate_memregion()) {
@@ -243,12 +283,27 @@ static int cxl_region_invalidate_memregion(struct cxl_region *cxlr)
 	return 0;
 }
 
-static void cxl_region_decode_reset(struct cxl_region *cxlr, int count)
+enum cxl_region_reset_context {
+	CXL_REGION_RESET_DEFAULT,
+	CXL_REGION_RESET_MANAGED_DETACH,
+};
+
+static void cxl_region_decode_reset(struct cxl_region *cxlr, int count,
+				    enum cxl_region_reset_context context)
 {
 	struct cxl_region_params *p = &cxlr->params;
 	int i;
 
-	if (test_bit(CXL_REGION_F_LOCK, &cxlr->flags))
+	/*
+	 * An attach provider locks a region against userspace disposition, but
+	 * a non-auto attach-owned region is software-owned and needs reset on
+	 * managed detach, unless its decoder path has a genuine hardware lock.
+	 */
+	if (test_bit(CXL_REGION_F_LOCK, &cxlr->flags) &&
+	    (context != CXL_REGION_RESET_MANAGED_DETACH ||
+	     test_bit(CXL_REGION_F_AUTO, &cxlr->flags) ||
+	     !cxl_region_has_memdev_attach(cxlr) ||
+	     cxl_region_has_locked_decoder(cxlr)))
 		return;
 
 	/*
@@ -350,7 +405,7 @@ static int cxl_region_decode_commit(struct cxl_region *cxlr)
 
 err:
 	/* undo the targets that were successfully committed */
-	cxl_region_decode_reset(cxlr, i);
+	cxl_region_decode_reset(cxlr, i, CXL_REGION_RESET_DEFAULT);
 	return rc;
 }
 
@@ -449,7 +504,8 @@ static ssize_t commit_store(struct device *dev, struct device_attribute *attr,
 	 * thread already handled this reset.
 	 */
 	if (p->state == CXL_CONFIG_RESET_PENDING) {
-		cxl_region_decode_reset(cxlr, p->interleave_ways);
+		cxl_region_decode_reset(cxlr, p->interleave_ways,
+					CXL_REGION_RESET_DEFAULT);
 		p->state = CXL_CONFIG_ACTIVE;
 	}
 
@@ -2268,7 +2324,8 @@ static void cxl_cancel_auto_attach(struct cxl_endpoint_decoder *cxled)
 static struct cxl_region *
 __cxl_decoder_detach(struct cxl_region *cxlr,
 		     struct cxl_endpoint_decoder *cxled, int pos,
-		     enum cxl_detach_mode mode)
+		     enum cxl_detach_mode mode,
+		     enum cxl_region_reset_context context)
 {
 	struct cxl_region_params *p;
 
@@ -2299,7 +2356,8 @@ __cxl_decoder_detach(struct cxl_region *cxlr,
 		cxled->part = -1;
 
 	if (p->state > CXL_CONFIG_ACTIVE) {
-		cxl_region_decode_reset(cxlr, p->interleave_ways);
+		cxl_region_decode_reset(cxlr, p->interleave_ways,
+					context);
 		p->state = CXL_CONFIG_ACTIVE;
 	}
 
@@ -2340,23 +2398,25 @@ __cxl_decoder_detach(struct cxl_region *cxlr,
  *
  * When the detachment finds a region release the region driver.
  */
-int cxl_decoder_detach(struct cxl_region *cxlr,
-		       struct cxl_endpoint_decoder *cxled, int pos,
-		       enum cxl_detach_mode mode)
+static int cxl_decoder_detach_context(
+	struct cxl_region *cxlr, struct cxl_endpoint_decoder *cxled, int pos,
+	enum cxl_detach_mode mode, enum cxl_region_reset_context context)
 {
 	struct cxl_region *detach;
 
 	/* when the decoder is being destroyed lock unconditionally */
 	if (mode == DETACH_INVALIDATE) {
 		guard(rwsem_write)(&cxl_rwsem.region);
-		detach = __cxl_decoder_detach(cxlr, cxled, pos, mode);
+		detach = __cxl_decoder_detach(cxlr, cxled, pos, mode,
+					      context);
 	} else {
 		int rc;
 
 		ACQUIRE(rwsem_write_kill, rwsem)(&cxl_rwsem.region);
 		if ((rc = ACQUIRE_ERR(rwsem_write_kill, &rwsem)))
 			return rc;
-		detach = __cxl_decoder_detach(cxlr, cxled, pos, mode);
+		detach = __cxl_decoder_detach(cxlr, cxled, pos, mode,
+					      context);
 	}
 
 	if (detach) {
@@ -2366,6 +2426,14 @@ int cxl_decoder_detach(struct cxl_region *cxlr,
 	return 0;
 }
 
+int cxl_decoder_detach(struct cxl_region *cxlr,
+		       struct cxl_endpoint_decoder *cxled, int pos,
+		       enum cxl_detach_mode mode)
+{
+	return cxl_decoder_detach_context(cxlr, cxled, pos, mode,
+					  CXL_REGION_RESET_DEFAULT);
+}
+
 static int __attach_target(struct cxl_region *cxlr,
 			   struct cxl_endpoint_decoder *cxled, int pos,
 			   unsigned int state)
@@ -2398,9 +2466,10 @@ static int attach_target(struct cxl_region *cxlr,
 	return rc;
 }
 
-static int detach_target(struct cxl_region *cxlr, int pos)
+static int detach_target(struct cxl_region *cxlr, int pos,
+			 enum cxl_region_reset_context context)
 {
-	return cxl_decoder_detach(cxlr, NULL, pos, DETACH_ONLY);
+	return cxl_decoder_detach_context(cxlr, NULL, pos, DETACH_ONLY, context);
 }
 
 static size_t store_targetN(struct cxl_region *cxlr, const char *buf, int pos,
@@ -2409,7 +2478,7 @@ static size_t store_targetN(struct cxl_region *cxlr, const char *buf, int pos,
 	int rc;
 
 	if (sysfs_streq(buf, "\n"))
-		rc = detach_target(cxlr, pos);
+		rc = detach_target(cxlr, pos, CXL_REGION_RESET_DEFAULT);
 	else {
 		struct device *dev;
 
@@ -2559,7 +2628,8 @@ static struct cxl_region *to_cxl_region(struct device *dev)
 	return container_of(dev, struct cxl_region, dev);
 }
 
-static void unregister_region(struct cxl_region *cxlr)
+static void unregister_region(struct cxl_region *cxlr,
+			      enum cxl_region_reset_context context)
 {
 	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr->dev.parent);
 	struct cxl_region_params *p = &cxlr->params;
@@ -2574,7 +2644,7 @@ static void unregister_region(struct cxl_region *cxlr)
 	 * region parameters.
 	 */
 	for (i = 0; i < p->interleave_ways; i++)
-		detach_target(cxlr, i);
+		detach_target(cxlr, i, context);
 
 	cxlr->hpa_range = DEFINE_RANGE(0, -1);
 
@@ -2589,7 +2659,7 @@ static void endpoint_unregister_region(void *_cxlr)
 
 	guard(mutex)(&cxlrd->regions_lock);
 	if (xa_load(&cxlrd->regions, cxlr->id))
-		unregister_region(cxlr);
+		unregister_region(cxlr, CXL_REGION_RESET_MANAGED_DETACH);
 	put_device(&cxlr->dev);
 }
 
@@ -2717,7 +2787,7 @@ void kill_regions(struct cxl_root_decoder *cxlrd)
 	/* no more region creation */
 	cxlrd->dead = true;
 	xa_for_each(&cxlrd->regions, index, cxlr)
-		unregister_region(cxlr);
+		unregister_region(cxlr, CXL_REGION_RESET_DEFAULT);
 }
 
 /**
@@ -2760,7 +2830,7 @@ static struct cxl_region *devm_cxl_add_region(struct cxl_root_decoder *cxlrd,
 
 	rc = xa_insert(&cxlrd->regions, cxlr->id, cxlr, GFP_KERNEL);
 	if (rc) {
-		unregister_region(cxlr);
+		unregister_region(cxlr, CXL_REGION_RESET_DEFAULT);
 		return ERR_PTR(rc);
 	}
 
@@ -2893,7 +2963,7 @@ static ssize_t delete_region_store(struct device *dev,
 	if (!cxlr || !sysfs_streq(buf, dev_name(&cxlr->dev)))
 		return -ENODEV;
 
-	unregister_region(cxlr);
+	unregister_region(cxlr, CXL_REGION_RESET_DEFAULT);
 
 	return len;
 }
@@ -3781,7 +3851,7 @@ static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd,
 
 	rc = __construct_region(cxlr, ctx);
 	if (rc) {
-		unregister_region(cxlr);
+		unregister_region(cxlr, CXL_REGION_RESET_DEFAULT);
 		return ERR_PTR(rc);
 	}
 
-- 
2.43.0

[RFC PATCH 2/3] cxl/region: Auto-create a region for memdev attach

[Richard Cheng]

devm_cxl_probe_mem() currently fails when FW has not committed a region,
even when a Type-2 accelerator has usable CXL.mem capacity.

When no mapped decoder exists, select a pristine manual DEVMEM decoder
and the first compatible unlocked Type-2 RAM root decoder. Create a
non-AUTO, single-target region, allocate HPA and the full volatile DPA
partition, attach and commit the decoder path, then return the resulting
HPA range.

Use provider-managed reset for partial-commit rollback and unwind
region, HPA, DPA, and partition state in reverse order on failure.
Preserve the existing FW-precommitted path.

This support is limited to decoder 0, IW=1, and first-compatible root
selection.

Signed-off-by: Richard Cheng <icheng@nvidia.com>
---
 drivers/cxl/core/region.c | 312 ++++++++++++++++++++++++++++++++++----
 1 file changed, 284 insertions(+), 28 deletions(-)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 7fcaddc61180..4ceabdfdd3b6 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -363,7 +363,8 @@ static int commit_decoder(struct cxl_decoder *cxld)
 	return 0;
 }
 
-static int cxl_region_decode_commit(struct cxl_region *cxlr)
+static int cxl_region_decode_commit(
+	struct cxl_region *cxlr, enum cxl_region_reset_context context)
 {
 	struct cxl_region_params *p = &cxlr->params;
 	int i, rc = 0;
@@ -405,7 +406,7 @@ static int cxl_region_decode_commit(struct cxl_region *cxlr)
 
 err:
 	/* undo the targets that were successfully committed */
-	cxl_region_decode_reset(cxlr, i, CXL_REGION_RESET_DEFAULT);
+	cxl_region_decode_reset(cxlr, i, context);
 	return rc;
 }
 
@@ -427,7 +428,8 @@ static int queue_reset(struct cxl_region *cxlr)
 	return 0;
 }
 
-static int __commit(struct cxl_region *cxlr)
+static int __commit_context(struct cxl_region *cxlr,
+			    enum cxl_region_reset_context context)
 {
 	struct cxl_region_params *p = &cxlr->params;
 	int rc;
@@ -452,7 +454,7 @@ static int __commit(struct cxl_region *cxlr)
 	if (rc)
 		return rc;
 
-	rc = cxl_region_decode_commit(cxlr);
+	rc = cxl_region_decode_commit(cxlr, context);
 	if (rc)
 		return rc;
 
@@ -461,6 +463,11 @@ static int __commit(struct cxl_region *cxlr)
 	return 0;
 }
 
+static int __commit(struct cxl_region *cxlr)
+{
+	return __commit_context(cxlr, CXL_REGION_RESET_DEFAULT);
+}
+
 static ssize_t commit_store(struct device *dev, struct device_attribute *attr,
 			    const char *buf, size_t len)
 {
@@ -4177,45 +4184,271 @@ static int first_mapped_decoder(struct device *dev, const void *data)
 	return 0;
 }
 
+static int first_attach_decoder(struct device *dev, const void *data)
+{
+	struct cxl_port *endpoint = (struct cxl_port *)data;
+	struct cxl_endpoint_decoder *cxled;
+	struct cxl_decoder *cxld;
+
+	if (!is_endpoint_decoder(dev))
+		return 0;
+
+	cxled = to_cxl_endpoint_decoder(dev);
+	cxld = &cxled->cxld;
+	if (cxld->id != 0 || cxled->state != CXL_DECODER_STATE_MANUAL ||
+	    cxld->target_type != CXL_DECODER_DEVMEM || cxld->region ||
+	    cxled->dpa_res ||
+	    (cxld->flags & (CXL_DECODER_F_ENABLE | CXL_DECODER_F_LOCK)))
+		return 0;
+
+	if (endpoint->hdm_end != -1 || cxl_num_decoders_committed(endpoint))
+		return 0;
+
+	return 1;
+}
+
+static int first_attach_root_decoder(struct device *dev, const void *data)
+{
+	struct cxl_port *endpoint = (struct cxl_port *)data;
+	unsigned long required = CXL_DECODER_F_TYPE2 | CXL_DECODER_F_RAM |
+				 CXL_DECODER_F_ENABLE;
+	struct cxl_root_decoder *cxlrd;
+	struct cxl_switch_decoder *cxlsd;
+	struct cxl_decoder *cxld;
+	struct cxl_dport *dport;
+
+	if (!is_root_decoder(dev) || !device_is_registered(dev))
+		return 0;
+
+	cxlrd = to_cxl_root_decoder(dev);
+	cxlsd = &cxlrd->cxlsd;
+	cxld = &cxlsd->cxld;
+	if (cxlrd->dead || !cxlrd->res ||
+	    (cxld->flags & required) != required ||
+	    (cxld->flags & CXL_DECODER_F_LOCK) ||
+	    cxld->interleave_ways != 1 || cxlsd->nr_targets < 1)
+		return 0;
+
+	dport = cxl_find_dport_by_dev(cxlrd_to_port(cxlrd),
+				       endpoint->host_bridge);
+	return dport && cxlsd->target[0] == dport;
+}
+
+static struct cxl_root_decoder *
+find_attach_root_decoder(struct cxl_endpoint_decoder *cxled)
+{
+	struct cxl_port *endpoint = cxled_to_port(cxled);
+	struct cxl_root *root __free(put_cxl_root) = find_cxl_root(endpoint);
+	struct device *dev;
+
+	if (!root)
+		return ERR_PTR(-ENXIO);
+
+	/* First compatible x1 Type-2 window is strict v1 policy. */
+	dev = device_find_child(&root->port.dev, endpoint,
+				first_attach_root_decoder);
+	if (!dev)
+		return ERR_PTR(-ENXIO);
+
+	return to_cxl_root_decoder(dev);
+}
+
+static void restore_attach_decoder_part(struct cxl_endpoint_decoder *cxled,
+					int old_part)
+{
+	guard(rwsem_write)(&cxl_rwsem.dpa);
+	cxled->part = old_part;
+}
+
+static int select_attach_ram(struct cxl_endpoint_decoder *cxled,
+			     int *old_part, resource_size_t *size)
+{
+	struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
+	struct cxl_dev_state *cxlds = cxlmd->cxlds;
+	struct resource *res;
+	int part, rc;
+
+	scoped_guard(rwsem_read, &cxl_rwsem.dpa)
+		*old_part = cxled->part;
+
+	rc = cxl_dpa_set_part(cxled, CXL_PARTMODE_RAM);
+	if (rc)
+		return rc;
+
+	guard(rwsem_read)(&cxl_rwsem.dpa);
+	part = cxled->part;
+	if (part < 0 || part >= cxlds->nr_partitions)
+		return -ENXIO;
+
+	res = &cxlds->part[part].res;
+	if (res->child)
+		return -EBUSY;
+
+	*size = resource_size(res);
+	if (!*size || !IS_ALIGNED(*size, SZ_256M))
+		return -EINVAL;
+
+	return 0;
+}
+
+static struct cxl_region *
+create_attach_region(struct cxl_endpoint_decoder *cxled,
+		     struct cxl_root_decoder *cxlrd, resource_size_t size)
+{
+	struct cxl_region *cxlr;
+	int rc;
+
+	guard(mutex)(&cxlrd->regions_lock);
+	do {
+		cxlr = __create_region(cxlrd, CXL_PARTMODE_RAM,
+				       atomic_read(&cxlrd->region_id),
+				       CXL_DECODER_DEVMEM);
+	} while (IS_ERR(cxlr) && PTR_ERR(cxlr) == -EBUSY);
+	if (IS_ERR(cxlr))
+		return cxlr;
+
+	scoped_guard(rwsem_write, &cxl_rwsem.region) {
+		/* Single-target IW=1 is strict v1 policy. */
+		rc = set_interleave_ways(cxlr, 1);
+		if (!rc)
+			rc = set_interleave_granularity(
+				cxlr, CXL_DECODER_MIN_GRANULARITY);
+		if (!rc)
+			rc = alloc_hpa(cxlr, size);
+	}
+	if (rc)
+		goto err_unregister;
+
+	rc = cxl_dpa_alloc(cxled, size);
+	if (rc)
+		goto err_unregister;
+
+	rc = attach_target(cxlr, cxled, 0, TASK_UNINTERRUPTIBLE);
+	if (rc)
+		goto err_unregister;
+
+	rc = __commit_context(cxlr, CXL_REGION_RESET_MANAGED_DETACH);
+	if (rc)
+		goto err_unregister;
+
+	rc = device_attach(&cxlr->dev);
+	if (rc <= 0) {
+		if (!rc)
+			rc = -ENXIO;
+		goto err_unregister;
+	}
+
+	get_device(&cxlr->dev);
+	return cxlr;
+
+err_unregister:
+	unregister_region(cxlr, CXL_REGION_RESET_MANAGED_DETACH);
+	return ERR_PTR(rc);
+}
+
+static void cleanup_attach_dpa(struct cxl_endpoint_decoder *cxled,
+			       int old_part, int setup_rc)
+{
+	int rc;
+
+	rc = cxl_dpa_free(cxled);
+	if (rc)
+		dev_err(&cxled->cxld.dev,
+			"failed to clean up DPA after attach error %d: %d\n",
+			setup_rc, rc);
+	restore_attach_decoder_part(cxled, old_part);
+}
+
+static int create_memdev_attach_region(struct cxl_memdev *cxlmd,
+				       struct cxl_attach_region *attach)
+{
+	struct cxl_port *endpoint = cxlmd->endpoint;
+	struct device *decoder_dev __free(put_device) = NULL;
+	struct cxl_endpoint_decoder *cxled;
+	struct cxl_root_decoder *cxlrd;
+	struct cxl_region *cxlr;
+	struct range hpa_range;
+	resource_size_t size;
+	int old_part, rc;
+
+	scoped_guard(rwsem_read, &cxl_rwsem.region) {
+		guard(rwsem_read)(&cxl_rwsem.dpa);
+		decoder_dev = device_find_child(&endpoint->dev, endpoint,
+						first_attach_decoder);
+	}
+	if (!decoder_dev) {
+		dev_dbg(cxlmd->cxlds->dev,
+			"no free manual DEVMEM decoder to auto-create a region for %s\n",
+			dev_name(&cxlmd->dev));
+		return -ENXIO;
+	}
+	cxled = to_cxl_endpoint_decoder(decoder_dev);
+
+	rc = select_attach_ram(cxled, &old_part, &size);
+	if (rc) {
+		restore_attach_decoder_part(cxled, old_part);
+		return rc;
+	}
+
+	cxlrd = find_attach_root_decoder(cxled);
+	if (IS_ERR(cxlrd)) {
+		rc = PTR_ERR(cxlrd);
+		dev_dbg(cxlmd->cxlds->dev,
+			"no compatible Type-2 root decoder to auto-create a region for %s: %d\n",
+			dev_name(&cxlmd->dev), rc);
+		goto err_cleanup_dpa;
+	}
+
+	cxlr = create_attach_region(cxled, cxlrd, size);
+	put_device(&cxlrd->cxlsd.cxld.dev);
+	if (IS_ERR(cxlr)) {
+		rc = PTR_ERR(cxlr);
+		goto err_cleanup_dpa;
+	}
+
+	hpa_range = (struct range) {
+		.start = cxlr->params.res->start,
+		.end = cxlr->params.res->end,
+	};
+	rc = devm_add_action_or_reset(&endpoint->dev,
+				      endpoint_unregister_region, cxlr);
+	if (rc)
+		goto err_cleanup_dpa;
+
+	attach->hpa_range = hpa_range;
+	return 0;
+
+err_cleanup_dpa:
+	cleanup_attach_dpa(cxled, old_part, rc);
+	return rc;
+}
+
 /*
- * Runs in cxl_mem_probe context after successful endpoint probe, assumes the
- * simple case of single mapped decoder per memdev.
+ * Attach to a firmware-precommitted region already mapped to the endpoint.
+ * Return 0 on success, -ENODEV when no region is present (the caller then
+ * auto-creates one), or a negative errno for a present-but-unusable region.
  */
-int cxl_memdev_attach_region(struct cxl_memdev *cxlmd)
+static int find_committed_attach_region(struct cxl_memdev *cxlmd,
+					struct cxl_attach_region *attach)
 {
-	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 */
-	guard(device)(&endpoint->dev);
-	if (!endpoint->dev.driver)
-		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
-	 */
 	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;
-	}
+	if (!dev)
+		return -ENODEV;
 
 	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_dbg(cxlmd->cxlds->dev, "region %s not committed for memdev %s\n",
 			dev_name(&cxlr->dev), dev_name(&cxlmd->dev));
 		return -ENXIO;
 	}
@@ -4226,10 +4459,10 @@ int cxl_memdev_attach_region(struct cxl_memdev *cxlmd)
 		return -ENXIO;
 	}
 
-	/* Only teardown regions that pass validation, ignore the rest */
+	/* Only teardown regions that pass validation. */
 	get_device(&cxlr->dev);
-	rc = devm_add_action_or_reset(&endpoint->dev,
-				      endpoint_unregister_region, cxlr);
+	rc = devm_add_action_or_reset(&endpoint->dev, endpoint_unregister_region,
+				      cxlr);
 	if (rc)
 		return rc;
 
@@ -4239,6 +4472,29 @@ int cxl_memdev_attach_region(struct cxl_memdev *cxlmd)
 	};
 	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)
+{
+	struct cxl_attach_region *attach =
+		container_of(cxlmd->attach, typeof(*attach), attach);
+	struct cxl_port *endpoint = cxlmd->endpoint;
+	int rc;
+
+	/* hold endpoint lock to setup autoremove of the region */
+	guard(device)(&endpoint->dev);
+	if (!endpoint->dev.driver)
+		return -ENXIO;
+
+	rc = find_committed_attach_region(cxlmd, attach);
+	if (rc != -ENODEV)
+		return rc;
+
+	return create_memdev_attach_region(cxlmd, attach);
+}
 EXPORT_SYMBOL_FOR_MODULES(cxl_memdev_attach_region, "cxl_mem");
 
 /*
-- 
2.43.0

[RFC PATCH 3/3] cxl/test: Exercise Type-2 automatic region creation

[Richard Cheng]

Add a second mock Type-2 accelerator with an independent single-target
CFMWS and an uncommitted manual DEVMEM decoder. Keep the existing
FW-precomitted accelerator unchanged.

Verify that devm_cxl_probe_mem() creates the missing region and returns
a valid 512 MB HPA range. Preserve the manual decoder config after reset
so the fallback remains available across unbind and rebind.

Signed-off-by: Richard Cheng <icheng@nvidia.com>
---
 tools/testing/cxl/test/accel.c |  7 ++++
 tools/testing/cxl/test/cxl.c   | 61 ++++++++++++++++++++++++++++++++--
 2 files changed, 65 insertions(+), 3 deletions(-)

diff --git a/tools/testing/cxl/test/accel.c b/tools/testing/cxl/test/accel.c
index 8e6f4687ca02..7e5d76a7f8c4 100644
--- a/tools/testing/cxl/test/accel.c
+++ b/tools/testing/cxl/test/accel.c
@@ -38,6 +38,13 @@ static int cxl_mock_accel_probe(struct platform_device *pdev)
 	cxlmd = devm_cxl_probe_mem(cxlds, &mock_range);
 	if (IS_ERR(cxlmd))
 		return PTR_ERR(cxlmd);
+	if (mock_range.start > mock_range.end ||
+	    range_len(&mock_range) != SZ_512M) {
+		dev_err(dev,
+			"accelerator%d returned invalid HPA range %pra (expected 512 MiB)\n",
+			pdev->id, &mock_range);
+		return -ERANGE;
+	}
 	cxl_accel->cxlmd = cxlmd;
 
 	dev_dbg(dev, "Probed mock accelerator with range %pra\n", &mock_range);
diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
index 8ab2ce1262f3..305a0c3705da 100644
--- a/tools/testing/cxl/test/cxl.c
+++ b/tools/testing/cxl/test/cxl.c
@@ -28,7 +28,7 @@ static bool type2_test;
 #define NR_CXL_SWITCH_PORTS 2
 #define NR_CXL_PORT_DECODERS 8
 #define NR_BRIDGES (NR_CXL_HOST_BRIDGES + NR_CXL_SINGLE_HOST + NR_CXL_RCH)
-#define NR_CXL_TYPE2_ACCEL 1
+#define NR_CXL_TYPE2_ACCEL 2
 
 #define MOCK_AUTO_REGION_SIZE_DEFAULT SZ_512M
 static int mock_auto_region_size = MOCK_AUTO_REGION_SIZE_DEFAULT;
@@ -493,7 +493,16 @@ static void cfmws_elc_update(struct acpi_cedt_cfmws *window, int index)
 
 static void update_type2_cfmws(void)
 {
+	struct acpi_cedt_cfmws *window = &mock_cedt.cfmws1.cfmws;
+
 	memcpy(&mock_cedt.cfmws0.cfmws, &type2_cfmws0, sizeof(type2_cfmws0));
+
+	window->header.length = sizeof(*window) +
+				sizeof(mock_cedt.cfmws1.target[0]);
+	window->interleave_ways = 0;
+	window->restrictions = ACPI_CEDT_CFMWS_RESTRICT_DEVMEM |
+			       ACPI_CEDT_CFMWS_RESTRICT_VOLATILE;
+	mock_cedt.cfmws1.target[0] = 1;
 }
 
 static int populate_cedt(void)
@@ -814,6 +823,32 @@ static int mock_decoder_commit(struct cxl_decoder *cxld);
 static void mock_decoder_reset(struct cxl_decoder *cxld);
 static void init_disabled_mock_decoder(struct cxl_decoder *cxld);
 
+static bool is_type2_manual_decoder(struct cxl_decoder *cxld,
+				    struct platform_device *pdev)
+{
+	return type2_test && is_endpoint_decoder(&cxld->dev) && pdev &&
+		pdev->id == 1 && !strcmp(pdev->name, "cxl_type2_accel") &&
+		cxld->id == 0;
+}
+
+static void init_type2_manual_decoder(struct cxl_endpoint_decoder *cxled)
+{
+	struct cxl_decoder *cxld = &cxled->cxld;
+
+	cxld->hpa_range = (struct range) {
+		.start = 0,
+		.end = -1,
+	};
+	cxld->interleave_ways = 1;
+	cxld->interleave_granularity = CXL_DECODER_MIN_GRANULARITY;
+	cxld->target_type = CXL_DECODER_DEVMEM;
+	cxld->flags = 0;
+	cxled->state = CXL_DECODER_STATE_MANUAL;
+	cxled->skip = 0;
+	cxld->commit = mock_decoder_commit;
+	cxld->reset = mock_decoder_reset;
+}
+
 static void cxld_copy(struct cxl_decoder *a, struct cxl_decoder *b)
 {
 	a->id = b->id;
@@ -1089,6 +1124,7 @@ enum cxld_init_type {
 	MOCK_DECODER_INIT_SAVED,
 	MOCK_DECODER_INIT_TYPE3_AUTO,
 	MOCK_DECODER_INIT_TYPE2_AUTO,
+	MOCK_DECODER_INIT_TYPE2_MANUAL,
 };
 
 static enum cxld_init_type get_decoder_init_type(struct cxl_decoder *cxld,
@@ -1104,6 +1140,8 @@ static enum cxld_init_type get_decoder_init_type(struct cxl_decoder *cxld,
 	}
 
 	*td = NULL;
+	if (is_type2_manual_decoder(cxld, pdev))
+		return MOCK_DECODER_INIT_TYPE2_MANUAL;
 
 	/*
 	 * The first decoder on the first 2 devices on the first switch
@@ -1121,7 +1159,9 @@ static enum cxld_init_type get_decoder_init_type(struct cxl_decoder *cxld,
 			    MOCK_DECODER_INIT_TYPE3_AUTO;
 }
 
-static bool mock_decoder_handle_saved(struct cxl_decoder *cxld, struct cxl_test_decoder *td)
+static bool mock_decoder_handle_saved(struct cxl_decoder *cxld,
+				      struct cxl_test_decoder *td,
+				      struct platform_device *pdev)
 {
 	bool enabled;
 
@@ -1133,6 +1173,11 @@ static bool mock_decoder_handle_saved(struct cxl_decoder *cxld, struct cxl_test_
 	if (enabled)
 		return !cxld_registry_restore(cxld, td);
 
+	if (is_type2_manual_decoder(cxld, pdev)) {
+		init_type2_manual_decoder(to_cxl_endpoint_decoder(&cxld->dev));
+		return false;
+	}
+
 	init_disabled_mock_decoder(cxld);
 	return false;
 }
@@ -1209,6 +1254,13 @@ static void mock_init_hdm_type2_cxled(struct cxl_endpoint_decoder *cxled,
 	put_device(dev);
 }
 
+static void mock_init_hdm_type2_manual(struct cxl_endpoint_decoder *cxled)
+{
+	init_type2_manual_decoder(cxled);
+
+	WARN_ON_ONCE(!cxld_registry_new(&cxled->cxld));
+}
+
 static void mock_init_hdm_type3_cxled(struct cxl_endpoint_decoder *cxled,
 				      struct cxl_port *port,
 				      struct platform_device *pdev,
@@ -1361,7 +1413,7 @@ static bool mock_init_hdm_decoder(struct cxl_decoder *cxld)
 	case MOCK_DECODER_INIT_SAVED:
 		if (WARN_ON(!td))
 			return false;
-		return mock_decoder_handle_saved(cxld, td);
+		return mock_decoder_handle_saved(cxld, td, pdev);
 	case MOCK_DECODER_INIT_DEFAULT:
 		/*
 		 * The default path picks up all the decoders that are not
@@ -1375,6 +1427,9 @@ static bool mock_init_hdm_decoder(struct cxl_decoder *cxld)
 	case MOCK_DECODER_INIT_TYPE2_AUTO:
 		mock_init_hdm_type2_cxled(cxled, port);
 		return false;
+	case MOCK_DECODER_INIT_TYPE2_MANUAL:
+		mock_init_hdm_type2_manual(cxled);
+		return false;
 	default:
 		return false;
 	}
-- 
2.43.0