[PATCH 0/7] cxl: Add CXL type2 accelerator support for cxl_test

[PATCH 0/7] cxl: Add CXL type2 accelerator support for cxl_test

[Dave Jiang]

The series is based on v7.0-rc7 with Alejandro's type2 series [1] and
with Dan's follow on series [2].

Series adds a 'type2_test' module parameter where it setup a mock type2
hierarchy with a mock type2 accelerator device directly under a root
port that has an auto region setup.

It also includes a CXL fix that was encountered when running tests with
the new code.

[
  {
    "memdevs":[
      {
        "memdev":"mem0",
        "ram_size":536870912,
        "ram_qos_class":42,
        "host":"cxl_type2_accel.0",
        "poison_injectable":false
      }
    ]
  },
  {
    "regions":[
      {
        "region":"region0",
        "resource":70300293136384,
        "size":536870912,
        "type":"ram",
        "interleave_ways":1,
        "interleave_granularity":4096,
        "decode_state":"commit"
      }
    ]
  }
]

    "root decoders":[
      {
        "decoder":"decoder0.0",
        "resource":70300293136384,
        "size":1073741824,
        "interleave_ways":1,
        "accelmem_capable":true,
        "qos_class":42,
        "nr_targets":1
      },

[1]: https://lore.kernel.org/linux-cxl/20260330143827.1278677-1-alejandro.lucero-palau@amd.com/T/#t
[2]: https://lore.kernel.org/linux-cxl/20260403210050.1058650-1-dan.j.williams@intel.com/T/#t


Dave Jiang (7):
  cxl/test: Refactor mock_init_hdm_decoder() to prep for type2 decoder
  cxl/test: Add type2 support for mock CFMWS0
  cxl/test: Refactor platform device enumerations
  cxl/test: Add hierarchy enumeration support for type2 device
  cxl/test: Fixup hdm init for auto region to support type2
  cxl/test: Add cxl_test accelerator driver
  cxl: Fix double unregistration of CXL regions for type2 devices

 drivers/cxl/core/region.c      |   3 +-
 tools/testing/cxl/test/Kbuild  |   2 +
 tools/testing/cxl/test/accel.c |  71 ++++
 tools/testing/cxl/test/cxl.c   | 717 ++++++++++++++++++++++++---------
 4 files changed, 605 insertions(+), 188 deletions(-)
 create mode 100644 tools/testing/cxl/test/accel.c


base-commit: 6fa897f2d9a917c4bd7b911b230fcfad05a952c2
-- 
2.53.0

[PATCH 1/7] cxl/test: Refactor mock_init_hdm_decoder() to prep for type2 decoder

[Dave Jiang]

Split mock_init_hdm_decoder() into different types of initialization.
Currently the code deals with default initialization, saved decoders,
and type3 auto region initialization. In the future, a type2 auto
initialization will also be added. This refactor will make it cleaner
to add additional init types.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 tools/testing/cxl/test/cxl.c | 184 ++++++++++++++++++++++-------------
 1 file changed, 115 insertions(+), 69 deletions(-)

diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
index cd47fdd7ccb5..c26c37cef12c 100644
--- a/tools/testing/cxl/test/cxl.c
+++ b/tools/testing/cxl/test/cxl.c
@@ -1052,74 +1052,25 @@ static int first_decoder(struct device *dev, const void *data)
 	return 0;
 }
 
-/*
- * Initialize a decoder during HDM enumeration.
- *
- * If a saved registry entry exists:
- *   - enabled decoders are restored from the saved programming
- *   - disabled decoders are initialized in a clean disabled state
- *
- * If no registry entry exists the decoder follows the normal mock
- * initialization path, including the special auto-region setup for
- * the first endpoints under host-bridge0.
- *
- * Returns true if decoder state was restored from the registry. In
- * that case the saved decode configuration (including target mapping)
- * has already been applied and the map_targets() is skipped.
- */
-static bool mock_init_hdm_decoder(struct cxl_decoder *cxld)
+enum cxld_init_type {
+	MOCK_DECODER_INIT_DEFAULT,
+	MOCK_DECODER_INIT_SAVED,
+	MOCK_DECODER_INIT_TYPE3_AUTO,
+};
+
+static enum cxld_init_type get_decoder_init_type(struct cxl_decoder *cxld,
+						 struct platform_device *pdev,
+						 bool hb0,
+						 struct cxl_test_decoder **td)
 {
-	struct acpi_cedt_cfmws *window = mock_cfmws[0];
-	struct platform_device *pdev = NULL;
-	struct cxl_endpoint_decoder *cxled;
-	struct cxl_switch_decoder *cxlsd;
-	struct cxl_port *port, *iter;
-	struct cxl_test_decoder *td;
-	struct cxl_memdev *cxlmd;
-	struct cxl_dport *dport;
-	struct device *dev;
-	bool hb0 = false;
-	u64 base;
-	int i;
+	struct cxl_test_decoder *found_td = cxld_registry_find(cxld);
 
-	if (is_endpoint_decoder(&cxld->dev)) {
-		cxled = to_cxl_endpoint_decoder(&cxld->dev);
-		cxlmd = cxled_to_memdev(cxled);
-		WARN_ON(!dev_is_platform(cxlmd->dev.parent));
-		pdev = to_platform_device(cxlmd->dev.parent);
-
-		/* check is endpoint is attach to host-bridge0 */
-		port = cxled_to_port(cxled);
-		do {
-			if (port->uport_dev == &cxl_host_bridge[0]->dev) {
-				hb0 = true;
-				break;
-			}
-			if (is_cxl_port(port->dev.parent))
-				port = to_cxl_port(port->dev.parent);
-			else
-				port = NULL;
-		} while (port);
-		port = cxled_to_port(cxled);
-	} else {
-		port = to_cxl_port(cxld->dev.parent);
+	if (found_td) {
+		*td = found_td;
+		return MOCK_DECODER_INIT_SAVED;
 	}
 
-	td = cxld_registry_find(cxld);
-	if (td) {
-		bool enabled;
-
-		if (is_switch_decoder(&cxld->dev))
-			enabled = td->cxlsd.cxld.flags & CXL_DECODER_F_ENABLE;
-		else
-			enabled = td->cxled.cxld.flags & CXL_DECODER_F_ENABLE;
-
-		if (enabled)
-			return !cxld_registry_restore(cxld, td);
-
-		init_disabled_mock_decoder(cxld);
-		return false;
-	}
+	*td = NULL;
 
 	/*
 	 * The first decoder on the first 2 devices on the first switch
@@ -1130,10 +1081,40 @@ static bool mock_init_hdm_decoder(struct cxl_decoder *cxld)
 	 * See 'cxl list -BMPu -m cxl_mem.0,cxl_mem.4'
 	 */
 	if (!is_endpoint_decoder(&cxld->dev) || !hb0 || pdev->id % 4 ||
-	    pdev->id > 4 || cxld->id > 0) {
-		default_mock_decoder(cxld);
-		return false;
-	}
+	    pdev->id > 4 || cxld->id > 0)
+		return MOCK_DECODER_INIT_DEFAULT;
+
+	return MOCK_DECODER_INIT_TYPE3_AUTO;
+}
+
+static bool mock_decoder_handle_saved(struct cxl_decoder *cxld, struct cxl_test_decoder *td)
+{
+	bool enabled;
+
+	if (is_switch_decoder(&cxld->dev))
+		enabled = td->cxlsd.cxld.flags & CXL_DECODER_F_ENABLE;
+	else
+		enabled = td->cxled.cxld.flags & CXL_DECODER_F_ENABLE;
+
+	if (enabled)
+		return !cxld_registry_restore(cxld, td);
+
+	init_disabled_mock_decoder(cxld);
+	return false;
+}
+
+static void mock_init_hdm_type3_cxled(struct cxl_endpoint_decoder *cxled,
+				      struct cxl_port *port,
+				      struct platform_device *pdev)
+{
+	struct acpi_cedt_cfmws *window = mock_cfmws[0];
+	struct cxl_decoder *cxld = &cxled->cxld;
+	struct cxl_switch_decoder *cxlsd;
+	struct cxl_dport *dport;
+	struct cxl_port *iter;
+	struct device *dev;
+	u64 base;
+	int i;
 
 	base = window->base_hpa;
 	if (extended_linear_cache)
@@ -1208,8 +1189,73 @@ static bool mock_init_hdm_decoder(struct cxl_decoder *cxld)
 		cxld_registry_update(cxld);
 		put_device(dev);
 	}
+}
 
-	return false;
+/*
+ * Initialize a decoder during HDM enumeration.
+ *
+ * If a saved registry entry exists:
+ *   - enabled decoders are restored from the saved programming
+ *   - disabled decoders are initialized in a clean disabled state
+ *
+ * If no registry entry exists the decoder follows the normal mock
+ * initialization path, including the special auto-region setup for
+ * the first endpoints under host-bridge0.
+ *
+ * Returns true if decoder state was restored from the registry. In
+ * that case the saved decode configuration (including target mapping)
+ * has already been applied and the map_targets() is skipped.
+ */
+static bool mock_init_hdm_decoder(struct cxl_decoder *cxld)
+{
+	struct cxl_endpoint_decoder *cxled = NULL;
+	struct platform_device *pdev = NULL;
+	struct cxl_test_decoder *td;
+	struct cxl_memdev *cxlmd;
+	struct cxl_port *port;
+	bool hb0 = false;
+
+	if (is_endpoint_decoder(&cxld->dev)) {
+		cxled = to_cxl_endpoint_decoder(&cxld->dev);
+		cxlmd = cxled_to_memdev(cxled);
+		WARN_ON(!dev_is_platform(cxlmd->dev.parent));
+		pdev = to_platform_device(cxlmd->dev.parent);
+
+		/* check is endpoint is attach to host-bridge0 */
+		port = cxled_to_port(cxled);
+		do {
+			if (port->uport_dev == &cxl_host_bridge[0]->dev) {
+				hb0 = true;
+				break;
+			}
+			if (is_cxl_port(port->dev.parent))
+				port = to_cxl_port(port->dev.parent);
+			else
+				port = NULL;
+		} while (port);
+		port = cxled_to_port(cxled);
+	} else {
+		port = to_cxl_port(cxld->dev.parent);
+	}
+
+	switch (get_decoder_init_type(cxld, pdev, hb0, &td)) {
+	case MOCK_DECODER_INIT_SAVED:
+		if (!td)
+			return false;
+		return mock_decoder_handle_saved(cxld, td);
+	case MOCK_DECODER_INIT_DEFAULT:
+		/*
+		 * The default path picks up all the decoders that are not
+		 * endpoint.
+		 */
+		default_mock_decoder(cxld);
+		return false;
+	case MOCK_DECODER_INIT_TYPE3_AUTO:
+		mock_init_hdm_type3_cxled(cxled, port, pdev);
+		return false;
+	default:
+		return false;
+	}
 }
 
 static int mock_cxl_enumerate_decoders(struct cxl_hdm *cxlhdm,
-- 
2.53.0

[PATCH 2/7] cxl/test: Add type2 support for mock CFMWS0

[Dave Jiang]

Add a module parameter 'type2_test' for triggering type2 test support
in cxl_test. Setup the CFMWS0 configuration to be type2 when 'type2_test'
is set.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 tools/testing/cxl/test/cxl.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
index c26c37cef12c..903223013e41 100644
--- a/tools/testing/cxl/test/cxl.c
+++ b/tools/testing/cxl/test/cxl.c
@@ -16,6 +16,7 @@
 
 static int interleave_arithmetic;
 static bool extended_linear_cache;
+static bool type2_test;
 
 #define FAKE_QTG_ID	42
 
@@ -383,6 +384,19 @@ static struct {
 	},
 };
 
+static struct acpi_cedt_cfmws type2_cfmws0 = {
+	.header = {
+		.type = ACPI_CEDT_TYPE_CFMWS,
+		.length = sizeof(mock_cedt.cfmws0),
+	},
+	.interleave_ways = 0,
+	.granularity = 4,
+	.restrictions = ACPI_CEDT_CFMWS_RESTRICT_DEVMEM |
+			ACPI_CEDT_CFMWS_RESTRICT_VOLATILE,
+	.qtg_id = FAKE_QTG_ID,
+	.window_size = SZ_256M * 4,
+};
+
 struct acpi_cedt_cfmws *mock_cfmws[] = {
 	[0] = &mock_cedt.cfmws0.cfmws,
 	[1] = &mock_cedt.cfmws1.cfmws,
@@ -471,6 +485,11 @@ static void cfmws_elc_update(struct acpi_cedt_cfmws *window, int index)
 	window->window_size = mock_auto_region_size * 2;
 }
 
+static void update_type2_cfmws(void)
+{
+	memcpy(&mock_cedt.cfmws0.cfmws, &type2_cfmws0, sizeof(type2_cfmws0));
+}
+
 static int populate_cedt(void)
 {
 	struct cxl_mock_res *res;
@@ -492,10 +511,14 @@ static int populate_cedt(void)
 		chbs->length = size;
 	}
 
+	if (type2_test)
+		update_type2_cfmws();
+
 	for (i = cfmws_start; i <= cfmws_end; i++) {
 		struct acpi_cedt_cfmws *window = mock_cfmws[i];
 
-		cfmws_elc_update(window, i);
+		if (i == 0 && !type2_test)
+			cfmws_elc_update(window, i);
 		res = alloc_mock_res(window->window_size, SZ_256M);
 		if (!res)
 			return -ENOMEM;
@@ -2029,6 +2052,8 @@ module_param(interleave_arithmetic, int, 0444);
 MODULE_PARM_DESC(interleave_arithmetic, "Modulo:0, XOR:1");
 module_param(extended_linear_cache, bool, 0444);
 MODULE_PARM_DESC(extended_linear_cache, "Enable extended linear cache support");
+module_param(type2_test, bool, 0444);
+MODULE_PARM_DESC(type2_test, "Enable type 2 support testing");
 module_init(cxl_test_init);
 module_exit(cxl_test_exit);
 MODULE_LICENSE("GPL v2");
-- 
2.53.0

[PATCH 3/7] cxl/test: Refactor platform device enumerations

[Dave Jiang]

Split all the host bridges, rootports, upstream and downstream ports
enumerations to separate helper functions. This should make adding
type2 hierarchy easier later on.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 tools/testing/cxl/test/cxl.c | 327 +++++++++++++++++++++++------------
 1 file changed, 217 insertions(+), 110 deletions(-)

diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
index 903223013e41..5fd73f0634c6 100644
--- a/tools/testing/cxl/test/cxl.c
+++ b/tools/testing/cxl/test/cxl.c
@@ -1833,10 +1833,207 @@ static struct attribute *cxl_acpi_attrs[] = {
 };
 ATTRIBUTE_GROUPS(cxl_acpi);
 
+static void host_bridges_remove(void)
+{
+	int i;
+
+	for (i = ARRAY_SIZE(cxl_host_bridge) - 1; i >= 0; i--) {
+		struct platform_device *pdev = cxl_host_bridge[i];
+
+		if (!pdev)
+			break;
+
+		sysfs_remove_link(&pdev->dev.kobj, "physical_node");
+		platform_device_unregister(cxl_host_bridge[i]);
+	}
+}
+
+static int host_bridges_populate(void)
+{
+	int rc = 0;
+
+	for (int i = 0; i < ARRAY_SIZE(cxl_host_bridge); i++) {
+		struct acpi_device *adev = &host_bridge[i];
+		struct platform_device *pdev;
+
+		pdev = platform_device_alloc("cxl_host_bridge", i);
+		if (!pdev)
+			goto err_bridge;
+
+		mock_companion(adev, &pdev->dev);
+		rc = platform_device_add(pdev);
+		if (rc) {
+			platform_device_put(pdev);
+			goto err_bridge;
+		}
+
+		cxl_host_bridge[i] = pdev;
+		mock_pci_bus[i].bridge = &pdev->dev;
+		rc = sysfs_create_link(&pdev->dev.kobj, &pdev->dev.kobj,
+				       "physical_node");
+		if (rc)
+			goto err_bridge;
+	}
+
+	return 0;
+
+err_bridge:
+	host_bridges_remove();
+	return rc;
+}
+
+static void cxl_rootports_remove(void)
+{
+	for (int i = ARRAY_SIZE(cxl_root_port) - 1; i >= 0; i--) {
+		struct platform_device *pdev = cxl_root_port[i];
+
+		if (!pdev)
+			break;
+
+		platform_device_unregister(pdev);
+	}
+}
+
+static int cxl_rootports_populate(void)
+{
+	int rc = 0;
+
+	for (int i = 0; i < ARRAY_SIZE(cxl_root_port); i++) {
+		struct platform_device *bridge =
+			cxl_host_bridge[i % ARRAY_SIZE(cxl_host_bridge)];
+		struct platform_device *pdev;
+
+		pdev = platform_device_alloc("cxl_root_port", i);
+		if (!pdev)
+			goto err_port;
+
+		pdev->dev.parent = &bridge->dev;
+
+		rc = platform_device_add(pdev);
+		if (rc) {
+			platform_device_put(pdev);
+			goto err_port;
+		}
+		cxl_root_port[i] = pdev;
+	}
+
+	return 0;
+
+err_port:
+	cxl_rootports_remove();
+	return rc;
+}
+
+static void cxl_usps_remove(void)
+{
+	for (int i = ARRAY_SIZE(cxl_switch_dport) - 1; i >= 0; i--) {
+		struct platform_device *pdev = cxl_switch_dport[i];
+
+		if (!pdev)
+			break;
+
+		platform_device_unregister(cxl_switch_dport[i]);
+	}
+}
+
+static int cxl_usps_populate(void)
+{
+	int rc = 0;
+
+	for (int i = 0; i < ARRAY_SIZE(cxl_switch_uport); i++) {
+		struct platform_device *root_port = cxl_root_port[i];
+		struct platform_device *pdev;
+
+		pdev = platform_device_alloc("cxl_switch_uport", i);
+		if (!pdev)
+			goto err_uport;
+
+		pdev->dev.parent = &root_port->dev;
+
+		rc = platform_device_add(pdev);
+		if (rc) {
+			platform_device_put(pdev);
+			goto err_uport;
+		}
+		cxl_switch_uport[i] = pdev;
+	}
+
+	return 0;
+
+err_uport:
+	cxl_usps_remove();
+	return rc;
+}
+
+static void cxl_dsps_remove(void)
+{
+	for (int i = ARRAY_SIZE(cxl_switch_uport) - 1; i >= 0; i--) {
+		struct platform_device *pdev = cxl_switch_uport[i];
+
+		if (!pdev)
+			break;
+
+		platform_device_unregister(cxl_switch_uport[i]);
+	}
+}
+
+static int cxl_dsps_populate(void)
+{
+	int rc = 0;
+
+	for (int i = 0; i < ARRAY_SIZE(cxl_switch_dport); i++) {
+		struct platform_device *uport =
+			cxl_switch_uport[i % ARRAY_SIZE(cxl_switch_uport)];
+		struct platform_device *pdev;
+
+		pdev = platform_device_alloc("cxl_switch_dport", i);
+		if (!pdev)
+			goto err_dport;
+		pdev->dev.parent = &uport->dev;
+
+		rc = platform_device_add(pdev);
+		if (rc) {
+			platform_device_put(pdev);
+			goto err_dport;
+		}
+		cxl_switch_dport[i] = pdev;
+	}
+
+	return 0;
+
+err_dport:
+	cxl_dsps_remove();
+	return rc;
+}
+
+static void cxl_switches_remove(void)
+{
+	cxl_usps_remove();
+	cxl_dsps_remove();
+}
+
+static int cxl_switches_populate(void)
+{
+	int rc;
+
+	BUILD_BUG_ON(ARRAY_SIZE(cxl_switch_uport) != ARRAY_SIZE(cxl_root_port));
+	rc = cxl_usps_populate();
+	if (rc)
+		return rc;
+
+	rc = cxl_dsps_populate();
+	if (rc) {
+		cxl_usps_remove();
+		return rc;
+	}
+
+	return 0;
+}
+
 static __init int cxl_test_init(void)
 {
-	int rc, i;
 	struct range mappable;
+	int rc;
 
 	cxl_acpi_test();
 	cxl_core_test();
@@ -1872,86 +2069,21 @@ static __init int cxl_test_init(void)
 	if (rc)
 		goto err_populate;
 
-	for (i = 0; i < ARRAY_SIZE(cxl_host_bridge); i++) {
-		struct acpi_device *adev = &host_bridge[i];
-		struct platform_device *pdev;
+	rc = host_bridges_populate();
+	if (rc)
+		goto err_populate;
 
-		pdev = platform_device_alloc("cxl_host_bridge", i);
-		if (!pdev)
-			goto err_bridge;
+	rc = cxl_rootports_populate();
+	if (rc)
+		goto err_host_bridges;
 
-		mock_companion(adev, &pdev->dev);
-		rc = platform_device_add(pdev);
-		if (rc) {
-			platform_device_put(pdev);
-			goto err_bridge;
-		}
-
-		cxl_host_bridge[i] = pdev;
-		mock_pci_bus[i].bridge = &pdev->dev;
-		rc = sysfs_create_link(&pdev->dev.kobj, &pdev->dev.kobj,
-				       "physical_node");
-		if (rc)
-			goto err_bridge;
-	}
-
-	for (i = 0; i < ARRAY_SIZE(cxl_root_port); i++) {
-		struct platform_device *bridge =
-			cxl_host_bridge[i % ARRAY_SIZE(cxl_host_bridge)];
-		struct platform_device *pdev;
-
-		pdev = platform_device_alloc("cxl_root_port", i);
-		if (!pdev)
-			goto err_port;
-		pdev->dev.parent = &bridge->dev;
-
-		rc = platform_device_add(pdev);
-		if (rc) {
-			platform_device_put(pdev);
-			goto err_port;
-		}
-		cxl_root_port[i] = pdev;
-	}
-
-	BUILD_BUG_ON(ARRAY_SIZE(cxl_switch_uport) != ARRAY_SIZE(cxl_root_port));
-	for (i = 0; i < ARRAY_SIZE(cxl_switch_uport); i++) {
-		struct platform_device *root_port = cxl_root_port[i];
-		struct platform_device *pdev;
-
-		pdev = platform_device_alloc("cxl_switch_uport", i);
-		if (!pdev)
-			goto err_uport;
-		pdev->dev.parent = &root_port->dev;
-
-		rc = platform_device_add(pdev);
-		if (rc) {
-			platform_device_put(pdev);
-			goto err_uport;
-		}
-		cxl_switch_uport[i] = pdev;
-	}
-
-	for (i = 0; i < ARRAY_SIZE(cxl_switch_dport); i++) {
-		struct platform_device *uport =
-			cxl_switch_uport[i % ARRAY_SIZE(cxl_switch_uport)];
-		struct platform_device *pdev;
-
-		pdev = platform_device_alloc("cxl_switch_dport", i);
-		if (!pdev)
-			goto err_dport;
-		pdev->dev.parent = &uport->dev;
-
-		rc = platform_device_add(pdev);
-		if (rc) {
-			platform_device_put(pdev);
-			goto err_dport;
-		}
-		cxl_switch_dport[i] = pdev;
-	}
+	rc = cxl_switches_populate();
+	if (rc)
+		goto err_root_ports;
 
 	rc = cxl_single_topo_init();
 	if (rc)
-		goto err_dport;
+		goto err_switches;
 
 	rc = cxl_rch_topo_init();
 	if (rc)
@@ -1981,24 +2113,12 @@ static __init int cxl_test_init(void)
 	cxl_rch_topo_exit();
 err_single:
 	cxl_single_topo_exit();
-err_dport:
-	for (i = ARRAY_SIZE(cxl_switch_dport) - 1; i >= 0; i--)
-		platform_device_unregister(cxl_switch_dport[i]);
-err_uport:
-	for (i = ARRAY_SIZE(cxl_switch_uport) - 1; i >= 0; i--)
-		platform_device_unregister(cxl_switch_uport[i]);
-err_port:
-	for (i = ARRAY_SIZE(cxl_root_port) - 1; i >= 0; i--)
-		platform_device_unregister(cxl_root_port[i]);
-err_bridge:
-	for (i = ARRAY_SIZE(cxl_host_bridge) - 1; i >= 0; i--) {
-		struct platform_device *pdev = cxl_host_bridge[i];
-
-		if (!pdev)
-			continue;
-		sysfs_remove_link(&pdev->dev.kobj, "physical_node");
-		platform_device_unregister(cxl_host_bridge[i]);
-	}
+err_switches:
+	cxl_switches_remove();
+err_root_ports:
+	cxl_rootports_remove();
+err_host_bridges:
+	host_bridges_remove();
 err_populate:
 	depopulate_all_mock_resources();
 err_gen_pool_add:
@@ -2021,26 +2141,13 @@ static void free_decoder_registry(void)
 
 static __exit void cxl_test_exit(void)
 {
-	int i;
-
 	cxl_mem_exit();
 	platform_device_unregister(cxl_acpi);
 	cxl_rch_topo_exit();
 	cxl_single_topo_exit();
-	for (i = ARRAY_SIZE(cxl_switch_dport) - 1; i >= 0; i--)
-		platform_device_unregister(cxl_switch_dport[i]);
-	for (i = ARRAY_SIZE(cxl_switch_uport) - 1; i >= 0; i--)
-		platform_device_unregister(cxl_switch_uport[i]);
-	for (i = ARRAY_SIZE(cxl_root_port) - 1; i >= 0; i--)
-		platform_device_unregister(cxl_root_port[i]);
-	for (i = ARRAY_SIZE(cxl_host_bridge) - 1; i >= 0; i--) {
-		struct platform_device *pdev = cxl_host_bridge[i];
-
-		if (!pdev)
-			continue;
-		sysfs_remove_link(&pdev->dev.kobj, "physical_node");
-		platform_device_unregister(cxl_host_bridge[i]);
-	}
+	cxl_switches_remove();
+	cxl_rootports_remove();
+	host_bridges_remove();
 	depopulate_all_mock_resources();
 	gen_pool_destroy(cxl_mock_pool);
 	unregister_cxl_mock_ops(&cxl_mock_ops);
-- 
2.53.0

[PATCH 4/7] cxl/test: Add hierarchy enumeration support for type2 device

[Dave Jiang]

Add enumeration of type2 device hierarchy in cxl-test. The type2 device
is setup to be directly attached to a root port instead of rp -> switch
-> device that type3 hierarchy is setup..

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 tools/testing/cxl/test/cxl.c | 110 ++++++++++++++++++++++++++++++++---
 1 file changed, 103 insertions(+), 7 deletions(-)

diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
index 5fd73f0634c6..59a265ad23e0 100644
--- a/tools/testing/cxl/test/cxl.c
+++ b/tools/testing/cxl/test/cxl.c
@@ -1539,6 +1539,9 @@ static __init int cxl_rch_topo_init(void)
 {
 	int rc, i;
 
+	if (type2_test)
+		return 0;
+
 	for (i = 0; i < ARRAY_SIZE(cxl_rch); i++) {
 		int idx = NR_CXL_HOST_BRIDGES + NR_CXL_SINGLE_HOST + i;
 		struct acpi_device *adev = &host_bridge[idx];
@@ -1582,6 +1585,9 @@ static void cxl_rch_topo_exit(void)
 {
 	int i;
 
+	if (type2_test)
+		return;
+
 	for (i = ARRAY_SIZE(cxl_rch) - 1; i >= 0; i--) {
 		struct platform_device *pdev = cxl_rch[i];
 
@@ -1596,6 +1602,9 @@ static __init int cxl_single_topo_init(void)
 {
 	int i, rc;
 
+	if (type2_test)
+		return 0;
+
 	for (i = 0; i < ARRAY_SIZE(cxl_hb_single); i++) {
 		struct acpi_device *adev =
 			&host_bridge[NR_CXL_HOST_BRIDGES + i];
@@ -1705,6 +1714,9 @@ static void cxl_single_topo_exit(void)
 {
 	int i;
 
+	if (type2_test)
+		return;
+
 	for (i = ARRAY_SIZE(cxl_swd_single) - 1; i >= 0; i--)
 		platform_device_unregister(cxl_swd_single[i]);
 	for (i = ARRAY_SIZE(cxl_swu_single) - 1; i >= 0; i--)
@@ -1721,19 +1733,90 @@ static void cxl_single_topo_exit(void)
 	}
 }
 
+static void cxl_type3_mem_exit(void)
+{
+	struct platform_device *pdev;
+	int i;
+
+	for (i = ARRAY_SIZE(cxl_rcd) - 1; i >= 0; i--) {
+		pdev = cxl_rcd[i];
+		if (!pdev)
+			break;
+		platform_device_unregister(cxl_rcd[i]);
+	}
+
+	for (i = ARRAY_SIZE(cxl_mem_single) - 1; i >= 0; i--) {
+		pdev = cxl_mem_single[i];
+		if (!pdev)
+			break;
+		platform_device_unregister(cxl_mem_single[i]);
+	}
+
+	for (i = ARRAY_SIZE(cxl_mem) - 1; i >= 0; i--) {
+		pdev = cxl_mem[i];
+		if (!pdev)
+			break;
+		platform_device_unregister(pdev);
+	}
+}
+
+static void cxl_type2_mem_exit(void)
+{
+	for (int i = NR_CXL_ROOT_PORTS - 1; i >= 0; i--) {
+		struct platform_device *pdev = cxl_mem[i];
+
+		if (!pdev)
+			break;
+		platform_device_unregister(pdev);
+	}
+}
+
 static void cxl_mem_exit(void)
 {
-	int i;
+	if (type2_test) {
+		cxl_type2_mem_exit();
+		return;
+	}
 
-	for (i = ARRAY_SIZE(cxl_rcd) - 1; i >= 0; i--)
-		platform_device_unregister(cxl_rcd[i]);
-	for (i = ARRAY_SIZE(cxl_mem_single) - 1; i >= 0; i--)
-		platform_device_unregister(cxl_mem_single[i]);
-	for (i = ARRAY_SIZE(cxl_mem) - 1; i >= 0; i--)
+	cxl_type3_mem_exit();
+}
+
+static int cxl_type2_mem_init(void)
+{
+	int i, rc;
+
+	for (i = 0; i < NR_CXL_ROOT_PORTS; i++) {
+		struct platform_device *dport = cxl_root_port[i];
+		struct platform_device *pdev;
+
+		pdev = platform_device_alloc("cxl_type2_accel", i);
+		if (!pdev)
+			goto err_mem;
+		pdev->dev.parent = &dport->dev;
+		set_dev_node(&pdev->dev, i % 2);
+
+		rc = platform_device_add(pdev);
+		if (rc) {
+			platform_device_put(pdev);
+			goto err_mem;
+		}
+		cxl_mem[i] = pdev;
+	}
+
+	return 0;
+
+err_mem:
+	for (i = NR_CXL_ROOT_PORTS - 1; i >= 0; i--) {
+		struct platform_device *pdev = cxl_mem[i];
+
+		if (!pdev)
+			break;
 		platform_device_unregister(cxl_mem[i]);
+	}
+	return rc;
 }
 
-static int cxl_mem_init(void)
+static int cxl_type3_mem_init(void)
 {
 	int i, rc;
 
@@ -1806,6 +1889,13 @@ static int cxl_mem_init(void)
 	return rc;
 }
 
+static int cxl_mem_init(void)
+{
+	if (type2_test)
+		return cxl_type2_mem_init();
+	return cxl_type3_mem_init();
+}
+
 static ssize_t
 decoder_reset_preserve_registry_show(struct device *dev,
 				     struct device_attribute *attr, char *buf)
@@ -2008,6 +2098,9 @@ static int cxl_dsps_populate(void)
 
 static void cxl_switches_remove(void)
 {
+	if (type2_test)
+		return;
+
 	cxl_usps_remove();
 	cxl_dsps_remove();
 }
@@ -2016,6 +2109,9 @@ static int cxl_switches_populate(void)
 {
 	int rc;
 
+	if (type2_test)
+		return 0;
+
 	BUILD_BUG_ON(ARRAY_SIZE(cxl_switch_uport) != ARRAY_SIZE(cxl_root_port));
 	rc = cxl_usps_populate();
 	if (rc)
-- 
2.53.0

[PATCH 5/7] cxl/test: Fixup hdm init for auto region to support type2

[Dave Jiang]

Add support to setup initialization of decoders in order to support
type2 auto region.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 tools/testing/cxl/test/cxl.c | 71 +++++++++++++++++++++++++++++++++++-
 1 file changed, 70 insertions(+), 1 deletion(-)

diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
index 59a265ad23e0..8e489d22ab5e 100644
--- a/tools/testing/cxl/test/cxl.c
+++ b/tools/testing/cxl/test/cxl.c
@@ -1079,6 +1079,7 @@ enum cxld_init_type {
 	MOCK_DECODER_INIT_DEFAULT,
 	MOCK_DECODER_INIT_SAVED,
 	MOCK_DECODER_INIT_TYPE3_AUTO,
+	MOCK_DECODER_INIT_TYPE2_AUTO,
 };
 
 static enum cxld_init_type get_decoder_init_type(struct cxl_decoder *cxld,
@@ -1107,7 +1108,8 @@ static enum cxld_init_type get_decoder_init_type(struct cxl_decoder *cxld,
 	    pdev->id > 4 || cxld->id > 0)
 		return MOCK_DECODER_INIT_DEFAULT;
 
-	return MOCK_DECODER_INIT_TYPE3_AUTO;
+	return type2_test ? MOCK_DECODER_INIT_TYPE2_AUTO :
+			    MOCK_DECODER_INIT_TYPE3_AUTO;
 }
 
 static bool mock_decoder_handle_saved(struct cxl_decoder *cxld, struct cxl_test_decoder *td)
@@ -1126,6 +1128,70 @@ static bool mock_decoder_handle_saved(struct cxl_decoder *cxld, struct cxl_test_
 	return false;
 }
 
+static void mock_init_hdm_type2_cxled(struct cxl_endpoint_decoder *cxled,
+				      struct cxl_port *port,
+				      struct platform_device *pdev)
+{
+	struct acpi_cedt_cfmws *window = mock_cfmws[0];
+	struct cxl_decoder *cxld = &cxled->cxld;
+	struct cxl_switch_decoder *cxlsd;
+	struct cxl_dport *dport;
+	struct cxl_port *root_port;
+	struct device *dev;
+	u64 base;
+
+	base = window->base_hpa;
+	cxld->hpa_range = (struct range) {
+		.start = base,
+		.end = base + mock_auto_region_size - 1,
+	};
+
+	cxld->interleave_ways = 1;
+	eig_to_granularity(window->granularity, &cxld->interleave_granularity);
+	cxld->target_type = CXL_DECODER_DEVMEM;
+	cxld->flags = CXL_DECODER_F_ENABLE;
+	cxled->state = CXL_DECODER_STATE_AUTO;
+	port->commit_end = cxld->id;
+	devm_cxl_dpa_reserve(cxled, 0,
+			     mock_auto_region_size / cxld->interleave_ways, 0);
+	cxld->commit = mock_decoder_commit;
+	cxld->reset = mock_decoder_reset;
+
+	WARN_ON_ONCE(!cxld_registry_new(cxld));
+	/*
+	 * Now that endpoint decoder is set up, walk up the hierarchy
+	 * and setup the root port decoder targeting @cxlmd.
+	 */
+	dport = port->parent_dport;
+	root_port = dport->port;
+	dev = device_find_child(&root_port->dev, NULL, first_decoder);
+	/*
+	 * Ancestor ports are guaranteed to be enumerated before
+	 * @port, and all ports have at least one decoder.
+	 */
+	if (WARN_ON(!dev))
+		return;
+
+	cxlsd = to_cxl_switch_decoder(dev);
+	cxlsd->target[0] = dport;
+	cxlsd->cxld.target_map[0] = dport->port_id;
+	cxld = &cxlsd->cxld;
+	cxld->target_type = CXL_DECODER_DEVMEM;
+	cxld->flags = CXL_DECODER_F_ENABLE;
+	root_port->commit_end = 0;
+	cxld->interleave_ways = 1;
+	cxld->interleave_granularity = 4096;
+	cxld->hpa_range = (struct range) {
+		.start = base,
+		.end = base + mock_auto_region_size - 1,
+	};
+	cxld->commit = mock_decoder_commit;
+	cxld->reset = mock_decoder_reset;
+
+	cxld_registry_update(cxld);
+	put_device(dev);
+}
+
 static void mock_init_hdm_type3_cxled(struct cxl_endpoint_decoder *cxled,
 				      struct cxl_port *port,
 				      struct platform_device *pdev)
@@ -1276,6 +1342,9 @@ static bool mock_init_hdm_decoder(struct cxl_decoder *cxld)
 	case MOCK_DECODER_INIT_TYPE3_AUTO:
 		mock_init_hdm_type3_cxled(cxled, port, pdev);
 		return false;
+	case MOCK_DECODER_INIT_TYPE2_AUTO:
+		mock_init_hdm_type2_cxled(cxled, port, pdev);
+		return false;
 	default:
 		return false;
 	}
-- 
2.53.0

[PATCH 6/7] cxl/test: Add cxl_test accelerator driver

[Dave Jiang]

Add a type2 accelerator mock driver for the platform device that
simulates a CXL type2 device. The driver exercises the same
minimal API calls that a real CXL type2 driver would utilize.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 tools/testing/cxl/test/Kbuild  |  2 +
 tools/testing/cxl/test/accel.c | 71 ++++++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+)
 create mode 100644 tools/testing/cxl/test/accel.c

diff --git a/tools/testing/cxl/test/Kbuild b/tools/testing/cxl/test/Kbuild
index af50972c8b6d..83d88628df17 100644
--- a/tools/testing/cxl/test/Kbuild
+++ b/tools/testing/cxl/test/Kbuild
@@ -5,9 +5,11 @@ obj-m += cxl_test.o
 obj-m += cxl_mock.o
 obj-m += cxl_mock_mem.o
 obj-m += cxl_translate.o
+obj-m += cxl_mock_accel.o
 
 cxl_test-y := cxl.o
 cxl_mock-y := mock.o
 cxl_mock_mem-y := mem.o
+cxl_mock_accel-y := accel.o
 
 KBUILD_CFLAGS := $(filter-out -Wmissing-prototypes -Wmissing-declarations, $(KBUILD_CFLAGS))
diff --git a/tools/testing/cxl/test/accel.c b/tools/testing/cxl/test/accel.c
new file mode 100644
index 000000000000..f21fdd57034e
--- /dev/null
+++ b/tools/testing/cxl/test/accel.c
@@ -0,0 +1,71 @@
+// SPDX-License-Identifier: GPL-2.0-only
+// Copyright(c) 2026 Intel Corporation. All rights reserved.
+
+#include <linux/platform_device.h>
+#include <linux/mod_devicetable.h>
+#include <linux/vmalloc.h>
+#include <linux/module.h>
+#include <linux/delay.h>
+#include <linux/sizes.h>
+#include <linux/bits.h>
+#include <cxl/mailbox.h>
+#include <crypto/sha2.h>
+#include <cxlmem.h>
+
+struct mock_cxl_accel {
+	struct cxl_dev_state cxlds;
+	struct cxl_memdev *cxlmd;
+};
+
+static int cxl_mock_accel_probe(struct platform_device *pdev)
+{
+	struct cxl_attach_region attach = {
+		.attach = {
+			.probe = cxl_memdev_attach_region,
+		}
+	};
+	struct mock_cxl_accel *cxl_accel;
+	struct cxl_dev_state *cxlds;
+	struct cxl_memdev *cxlmd;
+	int rc;
+
+	cxl_accel = devm_cxl_dev_state_create(&pdev->dev, CXL_DEVTYPE_DEVMEM,
+					      pdev->dev.id + 1, 0,
+					      struct mock_cxl_accel, cxlds,
+					      false);
+	if (!cxl_accel)
+		return -ENODEV;
+
+	cxlds = &cxl_accel->cxlds;
+	cxlds->media_ready = true;
+	rc = cxl_set_capacity(cxlds, SZ_512M);
+	if (rc)
+		return rc;
+
+	cxlmd = devm_cxl_add_memdev(cxlds, &attach.attach);
+	if (IS_ERR(cxlmd))
+		return PTR_ERR(cxlmd);
+	cxl_accel->cxlmd = cxlmd;
+
+	return 0;
+}
+
+static const struct platform_device_id cxl_mock_accel_ids[] = {
+	{ .name = "cxl_type2_accel", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(platform, cxl_mock_accel_ids);
+
+static struct platform_driver cxl_mock_accel_driver = {
+	.probe = cxl_mock_accel_probe,
+	.id_table = cxl_mock_accel_ids,
+	.driver = {
+		.name = KBUILD_MODNAME,
+		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
+	},
+};
+
+module_platform_driver(cxl_mock_accel_driver);
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("cxl_test: accelerator device mock module");
+MODULE_IMPORT_NS("CXL");
-- 
2.53.0

[PATCH 7/7] cxl: Fix double unregistration of CXL regions for type2 devices

[Dave Jiang]

For a type2 device cxl_memdev_attach_region() is provided as the callback
to devm_cxl_add_memdev() call. In cxl_memdev_attach_region() a devm
action cxl_endpoint_region_autoremove() is registered to remove the
region when the device is removed. And this action calls
unregister_region() on trigger.

When the endpoint port driver is probed, discover_region() is called
to register a region. The call tree becomes discover_region() ->
cxl_add_to_region() -> construct_region() -> __create_region() ->
devm_cxl_add_region(). In devm_cxl_add_region(), unregister_region()
is also added as a devm action.

When removing the cxl_test module, the platform devices are removed
and that triggers the endpoint driver removal and calling of
unregister_region(). And then when the mock type2 device driver is being
removed, the devm action of unregister_region() is called again, which
causes a kernel warning about double unregistration of the same region.
Add a check to see if the devm action is already removed before calling
unregister_region() for cxl_endpoint_region_autoremove().

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

---
- Need to add a fixes tag when the patch commit that introduces
  cxl_endpoint_region_autoremove() is stable.
---
 drivers/cxl/core/region.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 61a3efdbf3c9..1a7c0895bb88 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -4296,7 +4296,8 @@ static void cxl_endpoint_region_autoremove(void *_cxlr)
 	struct cxl_root_decoder *cxlrd = cxlr->cxlrd;
 	struct cxl_port *port = cxlrd_to_port(cxlrd);
 
-	devm_release_action(port->uport_dev, unregister_region, cxlr);
+	if (!devm_remove_action_nowarn(port->uport_dev, unregister_region, cxlr))
+		unregister_region(cxlr);
 }
 
 /*
-- 
2.53.0

Re: [PATCH 7/7] cxl: Fix double unregistration of CXL regions for type2 devices

[Alejandro Lucero Palau]

On 4/23/26 00:02, Dave Jiang wrote:
> For a type2 device cxl_memdev_attach_region() is provided as the callback
> to devm_cxl_add_memdev() call. In cxl_memdev_attach_region() a devm
> action cxl_endpoint_region_autoremove() is registered to remove the
> region when the device is removed. And this action calls
> unregister_region() on trigger.
>
> When the endpoint port driver is probed, discover_region() is called
> to register a region. The call tree becomes discover_region() ->
> cxl_add_to_region() -> construct_region() -> __create_region() ->
> devm_cxl_add_region(). In devm_cxl_add_region(), unregister_region()
> is also added as a devm action.
>
> When removing the cxl_test module, the platform devices are removed
> and that triggers the endpoint driver removal and calling of
> unregister_region(). And then when the mock type2 device driver is being
> removed, the devm action of unregister_region() is called again, which
> causes a kernel warning about double unregistration of the same region.
> Add a check to see if the devm action is already removed before calling
> unregister_region() for cxl_endpoint_region_autoremove().
>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>
> ---
> - Need to add a fixes tag when the patch commit that introduces
>    cxl_endpoint_region_autoremove() is stable.
> ---
>   drivers/cxl/core/region.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 61a3efdbf3c9..1a7c0895bb88 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -4296,7 +4296,8 @@ static void cxl_endpoint_region_autoremove(void *_cxlr)
>   	struct cxl_root_decoder *cxlrd = cxlr->cxlrd;
>   	struct cxl_port *port = cxlrd_to_port(cxlrd);
>   
> -	devm_release_action(port->uport_dev, unregister_region, cxlr);
> +	if (!devm_remove_action_nowarn(port->uport_dev, unregister_region, cxlr))
> +		unregister_region(cxlr);
>   }
>   
>   /*


Hi Dave,


Yes, I saw that problem and reported to Dan:


https://lore.kernel.org/linux-cxl/20260403210050.1058650-1-dan.j.williams@intel.com/T/#m467dc88199645865a05b5fef858a67f3a608895b


I thought about adding similar check in my impending v26 but I ended up 
changing the linking of the created region from the root port to the 
endpoint device only. This avoids the problem you report for the cases 
of removing cxl_acpi before accelerator driver or cxl_mem unbinding the 
accelerator memdev.


You will see that later today within v26.


Thank you.

Re: [PATCH 0/7] cxl: Add CXL type2 accelerator support for cxl_test

[Alejandro Lucero Palau]

On 4/23/26 00:02, Dave Jiang wrote:
> The series is based on v7.0-rc7 with Alejandro's type2 series [1] and
> with Dan's follow on series [2].


FWIW, in v26 I am extending Dan's approach as it is not enough for 
safely allowing an accelerator driver to work with the autodiscovered 
region for the cases behind, cxl_acpi removal or cxl_mem ubinding 
accelerator memdev.


Thank you,

Alejandro.


> Series adds a 'type2_test' module parameter where it setup a mock type2
> hierarchy with a mock type2 accelerator device directly under a root
> port that has an auto region setup.
>
> It also includes a CXL fix that was encountered when running tests with
> the new code.
>
> [
>    {
>      "memdevs":[
>        {
>          "memdev":"mem0",
>          "ram_size":536870912,
>          "ram_qos_class":42,
>          "host":"cxl_type2_accel.0",
>          "poison_injectable":false
>        }
>      ]
>    },
>    {
>      "regions":[
>        {
>          "region":"region0",
>          "resource":70300293136384,
>          "size":536870912,
>          "type":"ram",
>          "interleave_ways":1,
>          "interleave_granularity":4096,
>          "decode_state":"commit"
>        }
>      ]
>    }
> ]
>
>      "root decoders":[
>        {
>          "decoder":"decoder0.0",
>          "resource":70300293136384,
>          "size":1073741824,
>          "interleave_ways":1,
>          "accelmem_capable":true,
>          "qos_class":42,
>          "nr_targets":1
>        },
>
> [1]: https://lore.kernel.org/linux-cxl/20260330143827.1278677-1-alejandro.lucero-palau@amd.com/T/#t
> [2]: https://lore.kernel.org/linux-cxl/20260403210050.1058650-1-dan.j.williams@intel.com/T/#t
>
>
> Dave Jiang (7):
>    cxl/test: Refactor mock_init_hdm_decoder() to prep for type2 decoder
>    cxl/test: Add type2 support for mock CFMWS0
>    cxl/test: Refactor platform device enumerations
>    cxl/test: Add hierarchy enumeration support for type2 device
>    cxl/test: Fixup hdm init for auto region to support type2
>    cxl/test: Add cxl_test accelerator driver
>    cxl: Fix double unregistration of CXL regions for type2 devices
>
>   drivers/cxl/core/region.c      |   3 +-
>   tools/testing/cxl/test/Kbuild  |   2 +
>   tools/testing/cxl/test/accel.c |  71 ++++
>   tools/testing/cxl/test/cxl.c   | 717 ++++++++++++++++++++++++---------
>   4 files changed, 605 insertions(+), 188 deletions(-)
>   create mode 100644 tools/testing/cxl/test/accel.c
>
>
> base-commit: 6fa897f2d9a917c4bd7b911b230fcfad05a952c2

Re: [PATCH 7/7] cxl: Fix double unregistration of CXL regions for type2 devices

[Dave Jiang]

On 4/23/26 12:10 AM, Alejandro Lucero Palau wrote:
> 
> On 4/23/26 00:02, Dave Jiang wrote:
>> For a type2 device cxl_memdev_attach_region() is provided as the callback
>> to devm_cxl_add_memdev() call. In cxl_memdev_attach_region() a devm
>> action cxl_endpoint_region_autoremove() is registered to remove the
>> region when the device is removed. And this action calls
>> unregister_region() on trigger.
>>
>> When the endpoint port driver is probed, discover_region() is called
>> to register a region. The call tree becomes discover_region() ->
>> cxl_add_to_region() -> construct_region() -> __create_region() ->
>> devm_cxl_add_region(). In devm_cxl_add_region(), unregister_region()
>> is also added as a devm action.
>>
>> When removing the cxl_test module, the platform devices are removed
>> and that triggers the endpoint driver removal and calling of
>> unregister_region(). And then when the mock type2 device driver is being
>> removed, the devm action of unregister_region() is called again, which
>> causes a kernel warning about double unregistration of the same region.
>> Add a check to see if the devm action is already removed before calling
>> unregister_region() for cxl_endpoint_region_autoremove().
>>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>>
>> ---
>> - Need to add a fixes tag when the patch commit that introduces
>>    cxl_endpoint_region_autoremove() is stable.
>> ---
>>   drivers/cxl/core/region.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
>> index 61a3efdbf3c9..1a7c0895bb88 100644
>> --- a/drivers/cxl/core/region.c
>> +++ b/drivers/cxl/core/region.c
>> @@ -4296,7 +4296,8 @@ static void cxl_endpoint_region_autoremove(void *_cxlr)
>>       struct cxl_root_decoder *cxlrd = cxlr->cxlrd;
>>       struct cxl_port *port = cxlrd_to_port(cxlrd);
>>   -    devm_release_action(port->uport_dev, unregister_region, cxlr);
>> +    if (!devm_remove_action_nowarn(port->uport_dev, unregister_region, cxlr))
>> +        unregister_region(cxlr);
>>   }
>>     /*
> 
> 
> Hi Dave,
> 
> 
> Yes, I saw that problem and reported to Dan:
> 
> 
> https://lore.kernel.org/linux-cxl/20260403210050.1058650-1-dan.j.williams@intel.com/T/#m467dc88199645865a05b5fef858a67f3a608895b
> 
> 
> I thought about adding similar check in my impending v26 but I ended up changing the linking of the created region from the root port to the endpoint device only. This avoids the problem you report for the cases of removing cxl_acpi before accelerator driver or cxl_mem unbinding the accelerator memdev.
> 
> 
> You will see that later today within v26.
> 

Thanks. This series will adapt to whatever the final resulting version we merge upstream.
> 
> Thank you.
> 

Re: [PATCH 7/7] cxl: Fix double unregistration of CXL regions for type2 devices

[Dan Williams (nvidia)]

Dave Jiang wrote:
> For a type2 device cxl_memdev_attach_region() is provided as the callback
> to devm_cxl_add_memdev() call. In cxl_memdev_attach_region() a devm
> action cxl_endpoint_region_autoremove() is registered to remove the
> region when the device is removed. And this action calls
> unregister_region() on trigger.
> 
> When the endpoint port driver is probed, discover_region() is called
> to register a region. The call tree becomes discover_region() ->
> cxl_add_to_region() -> construct_region() -> __create_region() ->
> devm_cxl_add_region(). In devm_cxl_add_region(), unregister_region()
> is also added as a devm action.
> 
> When removing the cxl_test module, the platform devices are removed
> and that triggers the endpoint driver removal and calling of
> unregister_region(). And then when the mock type2 device driver is being
> removed, the devm action of unregister_region() is called again, which
> causes a kernel warning about double unregistration of the same region.
> Add a check to see if the devm action is already removed before calling
> unregister_region() for cxl_endpoint_region_autoremove().
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> 
> ---
> - Need to add a fixes tag when the patch commit that introduces
>   cxl_endpoint_region_autoremove() is stable.
> ---
>  drivers/cxl/core/region.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 61a3efdbf3c9..1a7c0895bb88 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -4296,7 +4296,8 @@ static void cxl_endpoint_region_autoremove(void *_cxlr)
>  	struct cxl_root_decoder *cxlrd = cxlr->cxlrd;
>  	struct cxl_port *port = cxlrd_to_port(cxlrd);
>  
> -	devm_release_action(port->uport_dev, unregister_region, cxlr);
> +	if (!devm_remove_action_nowarn(port->uport_dev, unregister_region, cxlr))
> +		unregister_region(cxlr);

Good find, but no, it's always a mistake to use
devm_remove_action_nowarn() outside of Rust.

I am drafting a replacement fix for the issue Sungwoo reported as the
approach there would also apply here. The main observation is that we
need to separate the remove action lifetime from the region lifetime.