[PATCH v25 00/11] Type2 device basic support

[PATCH v25 00/11] Type2 device basic support

[alejandro.lucero-palau]

From: Alejandro Lucero <alucerop@amd.com>

This patchset should be aplied on the for-7.1/cxl-type2-support branch.

For the sake of having the impending Type2 support for known drivers
needing it (sfc and Jump Trading) this version does only support the
case of HDM Type2 device committed by the BIOS, keeping the
decoders untouched when driver is unload, which requires a patch from v6
Smita's series:

https://lore.kernel.org/linux-cxl/20260210064501.157591-1-Smita.KoralahalliChannabasappa@amd.com/T/#mdad81d3817def8baace77ead9e2e305e775cf51d ?

or a BIOS locking those decoders.

changes in v25:
  - minor fix in patch1 and patch2 (Jonathan)
  - patch6: get region instead of decoder and region
		  make check based on decoder state (Benjamin)
  - patch8: decouple unregister_region and cxl_unregister_region
    	    and use devm action release instead. (Gregory)
  - patch9: add comment when calling cxl_unregister_region (Jonathan)
  - patch 10: fix commit (Allison)

	

Alejandro Lucero (11):
  sfc: add cxl support
  cxl/sfc: Map cxl regs
  cxl/sfc: Initialize dpa without a mailbox
  cxl: Prepare memdev creation for type2
  sfc: create type2 cxl memdev
  cxl/hdm: Add support for getting region from committed decoder
  cxl: Add function for obtaining region range
  cxl: Export function for unwinding cxl by accelerators
  sfc: obtain decoder and region if committed by firmware
  cxl: Avoid dax creation for accelerators
  sfc: support pio mapping based on cxl

 drivers/cxl/core/core.h               |   2 +
 drivers/cxl/core/hdm.c                |  48 ++++++++++
 drivers/cxl/core/mbox.c               |  51 +----------
 drivers/cxl/core/memdev.c             |  81 ++++++++++++++++-
 drivers/cxl/core/pci.c                |   1 +
 drivers/cxl/core/port.c               |   1 +
 drivers/cxl/core/region.c             |  38 ++++++++
 drivers/cxl/core/regs.c               |   1 +
 drivers/cxl/cxlmem.h                  |   6 --
 drivers/cxl/cxlpci.h                  |  12 ---
 drivers/cxl/mem.c                     |  45 +++++++---
 drivers/cxl/pci.c                     |   1 +
 drivers/net/ethernet/sfc/Kconfig      |   9 ++
 drivers/net/ethernet/sfc/Makefile     |   1 +
 drivers/net/ethernet/sfc/ef10.c       |  53 +++++++++--
 drivers/net/ethernet/sfc/efx.c        |  15 +++-
 drivers/net/ethernet/sfc/efx_cxl.c    | 124 ++++++++++++++++++++++++++
 drivers/net/ethernet/sfc/efx_cxl.h    |  40 +++++++++
 drivers/net/ethernet/sfc/net_driver.h |  12 +++
 drivers/net/ethernet/sfc/nic.h        |   3 +
 include/cxl/cxl.h                     |  13 +++
 include/cxl/pci.h                     |  21 +++++
 22 files changed, 489 insertions(+), 89 deletions(-)
 create mode 100644 drivers/net/ethernet/sfc/efx_cxl.c
 create mode 100644 drivers/net/ethernet/sfc/efx_cxl.h
 create mode 100644 include/cxl/pci.h


base-commit: 64584273dfb8a1e5fc7d78094ba22a93c204b44e
-- 
2.34.1

[PATCH v25 01/11] sfc: add cxl support

[alejandro.lucero-palau]

From: Alejandro Lucero <alucerop@amd.com>

Add CXL initialization based on new CXL API for accel drivers and make
it dependent on kernel CXL configuration.

Signed-off-by: Alejandro Lucero <alucerop@amd.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Acked-by: Edward Cree <ecree.xilinx@gmail.com>
Reviewed-by: Alison Schofield <alison.schofield@intel.com>
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/net/ethernet/sfc/Kconfig      |  9 +++++
 drivers/net/ethernet/sfc/Makefile     |  1 +
 drivers/net/ethernet/sfc/efx.c        | 15 ++++++-
 drivers/net/ethernet/sfc/efx_cxl.c    | 56 +++++++++++++++++++++++++++
 drivers/net/ethernet/sfc/efx_cxl.h    | 40 +++++++++++++++++++
 drivers/net/ethernet/sfc/net_driver.h | 10 +++++
 6 files changed, 130 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/ethernet/sfc/efx_cxl.c
 create mode 100644 drivers/net/ethernet/sfc/efx_cxl.h

diff --git a/drivers/net/ethernet/sfc/Kconfig b/drivers/net/ethernet/sfc/Kconfig
index c4c43434f314..979f2801e2a8 100644
--- a/drivers/net/ethernet/sfc/Kconfig
+++ b/drivers/net/ethernet/sfc/Kconfig
@@ -66,6 +66,15 @@ config SFC_MCDI_LOGGING
 	  Driver-Interface) commands and responses, allowing debugging of
 	  driver/firmware interaction.  The tracing is actually enabled by
 	  a sysfs file 'mcdi_logging' under the PCI device.
+config SFC_CXL
+	bool "Solarflare SFC9100-family CXL support"
+	depends on SFC && CXL_BUS >= SFC
+	default SFC
+	help
+	  This enables SFC CXL support if the kernel is configuring CXL for
+	  using CTPIO with CXL.mem. The SFC device with CXL support and
+	  with a CXL-aware firmware can be used for minimizing latencies
+	  when sending through CTPIO.
 
 source "drivers/net/ethernet/sfc/falcon/Kconfig"
 source "drivers/net/ethernet/sfc/siena/Kconfig"
diff --git a/drivers/net/ethernet/sfc/Makefile b/drivers/net/ethernet/sfc/Makefile
index d99039ec468d..bb0f1891cde6 100644
--- a/drivers/net/ethernet/sfc/Makefile
+++ b/drivers/net/ethernet/sfc/Makefile
@@ -13,6 +13,7 @@ sfc-$(CONFIG_SFC_SRIOV)	+= sriov.o ef10_sriov.o ef100_sriov.o ef100_rep.o \
                            mae.o tc.o tc_bindings.o tc_counters.o \
                            tc_encap_actions.o tc_conntrack.o
 
+sfc-$(CONFIG_SFC_CXL)	+= efx_cxl.o
 obj-$(CONFIG_SFC)	+= sfc.o
 
 obj-$(CONFIG_SFC_FALCON) += falcon/
diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index 8f136a11d396..c1e6e31f07cf 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -34,6 +34,7 @@
 #include "selftest.h"
 #include "sriov.h"
 #include "efx_devlink.h"
+#include "efx_cxl.h"
 
 #include "mcdi_port_common.h"
 #include "mcdi_pcol.h"
@@ -981,12 +982,15 @@ static void efx_pci_remove(struct pci_dev *pci_dev)
 	efx_pci_remove_main(efx);
 
 	efx_fini_io(efx);
+
+	probe_data = container_of(efx, struct efx_probe_data, efx);
+	efx_cxl_exit(probe_data);
+
 	pci_dbg(efx->pci_dev, "shutdown successful\n");
 
 	efx_fini_devlink_and_unlock(efx);
 	efx_fini_struct(efx);
 	free_netdev(efx->net_dev);
-	probe_data = container_of(efx, struct efx_probe_data, efx);
 	kfree(probe_data);
 };
 
@@ -1190,6 +1194,15 @@ static int efx_pci_probe(struct pci_dev *pci_dev,
 	if (rc)
 		goto fail2;
 
+	/* A successful cxl initialization implies a CXL region created to be
+	 * used for PIO buffers. If there is no CXL support, or initialization
+	 * fails, efx_cxl_pio_initialised will be false and legacy PIO buffers
+	 * defined at specific PCI BAR regions will be used.
+	 */
+	rc = efx_cxl_init(probe_data);
+	if (rc)
+		pci_err(pci_dev, "CXL initialization failed with error %d\n", rc);
+
 	rc = efx_pci_probe_post_io(efx);
 	if (rc) {
 		/* On failure, retry once immediately.
diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c
new file mode 100644
index 000000000000..f66db976fde9
--- /dev/null
+++ b/drivers/net/ethernet/sfc/efx_cxl.c
@@ -0,0 +1,56 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/****************************************************************************
+ *
+ * Driver for AMD network controllers and boards
+ * Copyright (C) 2025, Advanced Micro Devices, Inc.
+ */
+
+#include <linux/pci.h>
+
+#include "net_driver.h"
+#include "efx_cxl.h"
+
+#define EFX_CTPIO_BUFFER_SIZE	SZ_256M
+
+int efx_cxl_init(struct efx_probe_data *probe_data)
+{
+	struct efx_nic *efx = &probe_data->efx;
+	struct pci_dev *pci_dev = efx->pci_dev;
+	struct efx_cxl *cxl;
+	u16 dvsec;
+
+	probe_data->cxl_pio_initialised = false;
+
+	/* Is the device configured with and using CXL? */
+	if (!pcie_is_cxl(pci_dev))
+		return 0;
+
+	dvsec = pci_find_dvsec_capability(pci_dev, PCI_VENDOR_ID_CXL,
+					  PCI_DVSEC_CXL_DEVICE);
+	if (!dvsec) {
+		pci_info(pci_dev, "CXL_DVSEC_PCIE_DEVICE capability not found\n");
+		return 0;
+	}
+
+	pci_dbg(pci_dev, "CXL_DVSEC_PCIE_DEVICE capability found\n");
+
+	/* Create a cxl_dev_state embedded in the cxl struct using cxl core api
+	 * specifying no mbox available.
+	 */
+	cxl = devm_cxl_dev_state_create(&pci_dev->dev, CXL_DEVTYPE_DEVMEM,
+					pci_dev->dev.id, dvsec, struct efx_cxl,
+					cxlds, false);
+
+	if (!cxl)
+		return -ENOMEM;
+
+	probe_data->cxl = cxl;
+
+	return 0;
+}
+
+void efx_cxl_exit(struct efx_probe_data *probe_data)
+{
+}
+
+MODULE_IMPORT_NS("CXL");
diff --git a/drivers/net/ethernet/sfc/efx_cxl.h b/drivers/net/ethernet/sfc/efx_cxl.h
new file mode 100644
index 000000000000..961639cef692
--- /dev/null
+++ b/drivers/net/ethernet/sfc/efx_cxl.h
@@ -0,0 +1,40 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/****************************************************************************
+ * Driver for AMD network controllers and boards
+ * Copyright (C) 2025, Advanced Micro Devices, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation, incorporated herein by reference.
+ */
+
+#ifndef EFX_CXL_H
+#define EFX_CXL_H
+
+#ifdef CONFIG_SFC_CXL
+
+#include <cxl/cxl.h>
+
+struct cxl_root_decoder;
+struct cxl_port;
+struct cxl_endpoint_decoder;
+struct cxl_region;
+struct efx_probe_data;
+
+struct efx_cxl {
+	struct cxl_dev_state cxlds;
+	struct cxl_memdev *cxlmd;
+	struct cxl_root_decoder *cxlrd;
+	struct cxl_port *endpoint;
+	struct cxl_endpoint_decoder *cxled;
+	struct cxl_region *efx_region;
+	void __iomem *ctpio_cxl;
+};
+
+int efx_cxl_init(struct efx_probe_data *probe_data);
+void efx_cxl_exit(struct efx_probe_data *probe_data);
+#else
+static inline int efx_cxl_init(struct efx_probe_data *probe_data) { return 0; }
+static inline void efx_cxl_exit(struct efx_probe_data *probe_data) {}
+#endif
+#endif
diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
index b98c259f672d..3964b2c56609 100644
--- a/drivers/net/ethernet/sfc/net_driver.h
+++ b/drivers/net/ethernet/sfc/net_driver.h
@@ -1197,14 +1197,24 @@ struct efx_nic {
 	atomic_t n_rx_noskb_drops;
 };
 
+#ifdef CONFIG_SFC_CXL
+struct efx_cxl;
+#endif
+
 /**
  * struct efx_probe_data - State after hardware probe
  * @pci_dev: The PCI device
  * @efx: Efx NIC details
+ * @cxl: details of related cxl objects
+ * @cxl_pio_initialised: cxl initialization outcome.
  */
 struct efx_probe_data {
 	struct pci_dev *pci_dev;
 	struct efx_nic efx;
+#ifdef CONFIG_SFC_CXL
+	struct efx_cxl *cxl;
+	bool cxl_pio_initialised;
+#endif
 };
 
 static inline struct efx_nic *efx_netdev_priv(struct net_device *dev)
-- 
2.34.1

Re: [PATCH v25 01/11] sfc: add cxl support

[Dan Williams]

Hi Alejandro, similar to the approach taken with the CXL port error
handling series [1], I propose that additional changes to this
implementation be handled incrementally.

There may still be something that should be rebased out of the history,
but we have passed the point where fixes on top of stable commits starts
to be a better working model.

[1]: http://lore.kernel.org/69c98caef1348_178904100e0@dwillia2-mobl4.notmuch

One warning below...

alejandro.lucero-palau@ wrote:
> From: Alejandro Lucero <alucerop@amd.com>
> 
> Add CXL initialization based on new CXL API for accel drivers and make
> it dependent on kernel CXL configuration.
> 
> Signed-off-by: Alejandro Lucero <alucerop@amd.com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Acked-by: Edward Cree <ecree.xilinx@gmail.com>
> Reviewed-by: Alison Schofield <alison.schofield@intel.com>
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
[..]
> +	/* Create a cxl_dev_state embedded in the cxl struct using cxl core api
> +	 * specifying no mbox available.
> +	 */
> +	cxl = devm_cxl_dev_state_create(&pci_dev->dev, CXL_DEVTYPE_DEVMEM,
> +					pci_dev->dev.id, dvsec, struct efx_cxl,
> +					cxlds, false);
> +
> +	if (!cxl)
> +		return -ENOMEM;
> +
> +	probe_data->cxl = cxl;
> +
> +	return 0;
> +}
> +
> +void efx_cxl_exit(struct efx_probe_data *probe_data)
> +{
> +}

Not a driver I maintain so feel free to disregard, but if you are going
to have an exit handler, then you also want an explicit
devm_cxl_dev_state_destroy() API.

As a reviewer I do not want to always remember that some of this
driver's resources are cleaned up in the ->remove() callback and others
are cleaned up post ->remove() by the devres core. It is either 100%
explicit release or 100% implicit release if I have my druthers.

[PATCH v25 02/11] cxl/sfc: Map cxl regs

[alejandro.lucero-palau]

From: Alejandro Lucero <alucerop@amd.com>

Export cxl core functions for a Type2 driver being able to discover and
map the device registers.

Use it in sfc driver cxl initialization.

Signed-off-by: Alejandro Lucero <alucerop@amd.com>
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Ben Cheatham <benjamin.cheatham@amd.com>
---
 drivers/cxl/core/pci.c             |  1 +
 drivers/cxl/core/port.c            |  1 +
 drivers/cxl/core/regs.c            |  1 +
 drivers/cxl/cxlpci.h               | 12 ------------
 drivers/cxl/pci.c                  |  1 +
 drivers/net/ethernet/sfc/efx_cxl.c | 26 ++++++++++++++++++++++++++
 include/cxl/pci.h                  | 21 +++++++++++++++++++++
 7 files changed, 51 insertions(+), 12 deletions(-)
 create mode 100644 include/cxl/pci.h

diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
index d1f487b3d809..2bcd683aa286 100644
--- a/drivers/cxl/core/pci.c
+++ b/drivers/cxl/core/pci.c
@@ -6,6 +6,7 @@
 #include <linux/delay.h>
 #include <linux/pci.h>
 #include <linux/pci-doe.h>
+#include <cxl/pci.h>
 #include <linux/aer.h>
 #include <cxlpci.h>
 #include <cxlmem.h>
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 0c5957d1d329..a53b87de70e5 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -11,6 +11,7 @@
 #include <linux/idr.h>
 #include <linux/node.h>
 #include <cxl/einj.h>
+#include <cxl/pci.h>
 #include <cxlmem.h>
 #include <cxlpci.h>
 #include <cxl.h>
diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c
index 93710cf4f0a6..20c2d9fbcfe7 100644
--- a/drivers/cxl/core/regs.c
+++ b/drivers/cxl/core/regs.c
@@ -4,6 +4,7 @@
 #include <linux/device.h>
 #include <linux/slab.h>
 #include <linux/pci.h>
+#include <cxl/pci.h>
 #include <cxlmem.h>
 #include <cxlpci.h>
 #include <pmu.h>
diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h
index b826eb53cf7b..110ec9c44f09 100644
--- a/drivers/cxl/cxlpci.h
+++ b/drivers/cxl/cxlpci.h
@@ -13,16 +13,6 @@
  */
 #define CXL_PCI_DEFAULT_MAX_VECTORS 16
 
-/* Register Block Identifier (RBI) */
-enum cxl_regloc_type {
-	CXL_REGLOC_RBI_EMPTY = 0,
-	CXL_REGLOC_RBI_COMPONENT,
-	CXL_REGLOC_RBI_VIRT,
-	CXL_REGLOC_RBI_MEMDEV,
-	CXL_REGLOC_RBI_PMU,
-	CXL_REGLOC_RBI_TYPES
-};
-
 /*
  * Table Access DOE, CDAT Read Entry Response
  *
@@ -112,6 +102,4 @@ static inline void devm_cxl_port_ras_setup(struct cxl_port *port)
 }
 #endif
 
-int cxl_pci_setup_regs(struct pci_dev *pdev, enum cxl_regloc_type type,
-		       struct cxl_register_map *map);
 #endif /* __CXL_PCI_H__ */
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index adc7c4bcb03a..5cad118e7101 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -11,6 +11,7 @@
 #include <linux/pci.h>
 #include <linux/aer.h>
 #include <linux/io.h>
+#include <cxl/pci.h>
 #include <cxl/mailbox.h>
 #include "cxlmem.h"
 #include "cxlpci.h"
diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c
index f66db976fde9..6937616ec362 100644
--- a/drivers/net/ethernet/sfc/efx_cxl.c
+++ b/drivers/net/ethernet/sfc/efx_cxl.c
@@ -7,6 +7,8 @@
 
 #include <linux/pci.h>
 
+#include <cxl/cxl.h>
+#include <cxl/pci.h>
 #include "net_driver.h"
 #include "efx_cxl.h"
 
@@ -18,6 +20,7 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
 	struct pci_dev *pci_dev = efx->pci_dev;
 	struct efx_cxl *cxl;
 	u16 dvsec;
+	int rc;
 
 	probe_data->cxl_pio_initialised = false;
 
@@ -44,6 +47,29 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
 	if (!cxl)
 		return -ENOMEM;
 
+	rc = cxl_pci_setup_regs(pci_dev, CXL_REGLOC_RBI_COMPONENT,
+				&cxl->cxlds.reg_map);
+	if (rc) {
+		pci_err(pci_dev, "No component registers\n");
+		return rc;
+	}
+
+	if (!cxl->cxlds.reg_map.component_map.hdm_decoder.valid) {
+		pci_err(pci_dev, "Expected HDM component register not found\n");
+		return -ENODEV;
+	}
+
+	if (!cxl->cxlds.reg_map.component_map.ras.valid) {
+		pci_err(pci_dev, "Expected RAS component register not found\n");
+		return -ENODEV;
+	}
+
+	/* Set media ready explicitly as there are neither mailbox for checking
+	 * this state nor the CXL register involved, both not mandatory for
+	 * type2.
+	 */
+	cxl->cxlds.media_ready = true;
+
 	probe_data->cxl = cxl;
 
 	return 0;
diff --git a/include/cxl/pci.h b/include/cxl/pci.h
new file mode 100644
index 000000000000..a172439f08c6
--- /dev/null
+++ b/include/cxl/pci.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* Copyright(c) 2020 Intel Corporation. All rights reserved. */
+
+#ifndef __CXL_CXL_PCI_H__
+#define __CXL_CXL_PCI_H__
+
+/* Register Block Identifier (RBI) */
+enum cxl_regloc_type {
+	CXL_REGLOC_RBI_EMPTY = 0,
+	CXL_REGLOC_RBI_COMPONENT,
+	CXL_REGLOC_RBI_VIRT,
+	CXL_REGLOC_RBI_MEMDEV,
+	CXL_REGLOC_RBI_PMU,
+	CXL_REGLOC_RBI_TYPES
+};
+
+struct cxl_register_map;
+
+int cxl_pci_setup_regs(struct pci_dev *pdev, enum cxl_regloc_type type,
+		       struct cxl_register_map *map);
+#endif
-- 
2.34.1

[PATCH v25 03/11] cxl/sfc: Initialize dpa without a mailbox

[alejandro.lucero-palau]

From: Alejandro Lucero <alucerop@amd.com>

Type3 relies on mailbox CXL_MBOX_OP_IDENTIFY command for initializing
memdev state params which end up being used for DPA initialization.

Allow a Type2 driver to initialize DPA simply by giving the size of its
volatile hardware partition.

Move related functions to memdev.

Add sfc driver as the client.

Signed-off-by: Alejandro Lucero <alucerop@amd.com>
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Ben Cheatham <benjamin.cheatham@amd.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/cxl/core/core.h            |  2 +
 drivers/cxl/core/mbox.c            | 51 +----------------------
 drivers/cxl/core/memdev.c          | 66 ++++++++++++++++++++++++++++++
 drivers/net/ethernet/sfc/efx_cxl.c |  5 +++
 include/cxl/cxl.h                  |  2 +
 5 files changed, 76 insertions(+), 50 deletions(-)

diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
index 5539e941782f..77c684744771 100644
--- a/drivers/cxl/core/core.h
+++ b/drivers/cxl/core/core.h
@@ -97,6 +97,8 @@ void __iomem *devm_cxl_iomap_block(struct device *dev, resource_size_t addr,
 struct dentry *cxl_debugfs_create_dir(const char *dir);
 int cxl_dpa_set_part(struct cxl_endpoint_decoder *cxled,
 		     enum cxl_partition_mode mode);
+struct cxl_memdev_state;
+int cxl_mem_get_partition_info(struct cxl_memdev_state *mds);
 int cxl_dpa_alloc(struct cxl_endpoint_decoder *cxled, u64 size);
 int cxl_dpa_free(struct cxl_endpoint_decoder *cxled);
 resource_size_t cxl_dpa_size(struct cxl_endpoint_decoder *cxled);
diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
index 451ff2287b44..cf37676068c2 100644
--- a/drivers/cxl/core/mbox.c
+++ b/drivers/cxl/core/mbox.c
@@ -1151,7 +1151,7 @@ EXPORT_SYMBOL_NS_GPL(cxl_mem_get_event_records, "CXL");
  *
  * See CXL @8.2.9.5.2.1 Get Partition Info
  */
-static int cxl_mem_get_partition_info(struct cxl_memdev_state *mds)
+int cxl_mem_get_partition_info(struct cxl_memdev_state *mds)
 {
 	struct cxl_mailbox *cxl_mbox = &mds->cxlds.cxl_mbox;
 	struct cxl_mbox_get_partition_info pi;
@@ -1307,55 +1307,6 @@ int cxl_mem_sanitize(struct cxl_memdev *cxlmd, u16 cmd)
 	return -EBUSY;
 }
 
-static void add_part(struct cxl_dpa_info *info, u64 start, u64 size, enum cxl_partition_mode mode)
-{
-	int i = info->nr_partitions;
-
-	if (size == 0)
-		return;
-
-	info->part[i].range = (struct range) {
-		.start = start,
-		.end = start + size - 1,
-	};
-	info->part[i].mode = mode;
-	info->nr_partitions++;
-}
-
-int cxl_mem_dpa_fetch(struct cxl_memdev_state *mds, struct cxl_dpa_info *info)
-{
-	struct cxl_dev_state *cxlds = &mds->cxlds;
-	struct device *dev = cxlds->dev;
-	int rc;
-
-	if (!cxlds->media_ready) {
-		info->size = 0;
-		return 0;
-	}
-
-	info->size = mds->total_bytes;
-
-	if (mds->partition_align_bytes == 0) {
-		add_part(info, 0, mds->volatile_only_bytes, CXL_PARTMODE_RAM);
-		add_part(info, mds->volatile_only_bytes,
-			 mds->persistent_only_bytes, CXL_PARTMODE_PMEM);
-		return 0;
-	}
-
-	rc = cxl_mem_get_partition_info(mds);
-	if (rc) {
-		dev_err(dev, "Failed to query partition information\n");
-		return rc;
-	}
-
-	add_part(info, 0, mds->active_volatile_bytes, CXL_PARTMODE_RAM);
-	add_part(info, mds->active_volatile_bytes, mds->active_persistent_bytes,
-		 CXL_PARTMODE_PMEM);
-
-	return 0;
-}
-EXPORT_SYMBOL_NS_GPL(cxl_mem_dpa_fetch, "CXL");
-
 int cxl_get_dirty_count(struct cxl_memdev_state *mds, u32 *count)
 {
 	struct cxl_mailbox *cxl_mbox = &mds->cxlds.cxl_mbox;
diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
index 99e422594885..166ec6b7d041 100644
--- a/drivers/cxl/core/memdev.c
+++ b/drivers/cxl/core/memdev.c
@@ -582,6 +582,72 @@ bool is_cxl_memdev(const struct device *dev)
 }
 EXPORT_SYMBOL_NS_GPL(is_cxl_memdev, "CXL");
 
+static void add_part(struct cxl_dpa_info *info, u64 start, u64 size, enum cxl_partition_mode mode)
+{
+	int i = info->nr_partitions;
+
+	if (size == 0)
+		return;
+
+	info->part[i].range = (struct range) {
+		.start = start,
+		.end = start + size - 1,
+	};
+	info->part[i].mode = mode;
+	info->nr_partitions++;
+}
+
+int cxl_mem_dpa_fetch(struct cxl_memdev_state *mds, struct cxl_dpa_info *info)
+{
+	struct cxl_dev_state *cxlds = &mds->cxlds;
+	struct device *dev = cxlds->dev;
+	int rc;
+
+	if (!cxlds->media_ready) {
+		info->size = 0;
+		return 0;
+	}
+
+	info->size = mds->total_bytes;
+
+	if (mds->partition_align_bytes == 0) {
+		add_part(info, 0, mds->volatile_only_bytes, CXL_PARTMODE_RAM);
+		add_part(info, mds->volatile_only_bytes,
+			 mds->persistent_only_bytes, CXL_PARTMODE_PMEM);
+		return 0;
+	}
+
+	rc = cxl_mem_get_partition_info(mds);
+	if (rc) {
+		dev_err(dev, "Failed to query partition information\n");
+		return rc;
+	}
+
+	add_part(info, 0, mds->active_volatile_bytes, CXL_PARTMODE_RAM);
+	add_part(info, mds->active_volatile_bytes, mds->active_persistent_bytes,
+		 CXL_PARTMODE_PMEM);
+
+	return 0;
+}
+EXPORT_SYMBOL_NS_GPL(cxl_mem_dpa_fetch, "CXL");
+
+/**
+ * cxl_set_capacity: initialize dpa by a driver without a mailbox.
+ *
+ * @cxlds: pointer to cxl_dev_state
+ * @capacity: device volatile memory size
+ */
+int cxl_set_capacity(struct cxl_dev_state *cxlds, u64 capacity)
+{
+	struct cxl_dpa_info range_info = {
+		.size = capacity,
+	};
+
+	add_part(&range_info, 0, capacity, CXL_PARTMODE_RAM);
+	return cxl_dpa_setup(cxlds, &range_info);
+}
+EXPORT_SYMBOL_NS_GPL(cxl_set_capacity, "CXL");
+
 /**
  * set_exclusive_cxl_commands() - atomically disable user cxl commands
  * @mds: The device state to operate on
diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c
index 6937616ec362..6619084a77d8 100644
--- a/drivers/net/ethernet/sfc/efx_cxl.c
+++ b/drivers/net/ethernet/sfc/efx_cxl.c
@@ -70,6 +70,11 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
 	 */
 	cxl->cxlds.media_ready = true;
 
+	if (cxl_set_capacity(&cxl->cxlds, EFX_CTPIO_BUFFER_SIZE)) {
+		pci_err(pci_dev, "dpa capacity setup failed\n");
+		return -ENODEV;
+	}
+
 	probe_data->cxl = cxl;
 
 	return 0;
diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h
index fa7269154620..1346771edc4e 100644
--- a/include/cxl/cxl.h
+++ b/include/cxl/cxl.h
@@ -223,4 +223,6 @@ struct cxl_dev_state *_devm_cxl_dev_state_create(struct device *dev,
 		(drv_struct *)_devm_cxl_dev_state_create(parent, type, serial, dvsec,	\
 						      sizeof(drv_struct), mbox);	\
 	})
+
+int cxl_set_capacity(struct cxl_dev_state *cxlds, u64 capacity);
 #endif /* __CXL_CXL_H__ */
-- 
2.34.1

[PATCH v25 04/11] cxl: Prepare memdev creation for type2

[alejandro.lucero-palau]

From: Alejandro Lucero <alucerop@amd.com>

Current cxl core is relying on a CXL_DEVTYPE_CLASSMEM type device when
creating a memdev leading to problems when obtaining cxl_memdev_state
references from a CXL_DEVTYPE_DEVMEM type.

Modify check for obtaining cxl_memdev_state adding CXL_DEVTYPE_DEVMEM
support.

Make devm_cxl_add_memdev accessible from an accel driver.

Signed-off-by: Alejandro Lucero <alucerop@amd.com>
Reviewed-by: Ben Cheatham <benjamin.cheatham@amd.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Alison Schofield <alison.schofield@intel.com>
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/cxl/core/memdev.c | 15 +++++++++++--
 drivers/cxl/cxlmem.h      |  6 ------
 drivers/cxl/mem.c         | 45 +++++++++++++++++++++++++++++----------
 include/cxl/cxl.h         |  6 ++++++
 4 files changed, 53 insertions(+), 19 deletions(-)

diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
index 166ec6b7d041..759b43364ed7 100644
--- a/drivers/cxl/core/memdev.c
+++ b/drivers/cxl/core/memdev.c
@@ -7,6 +7,7 @@
 #include <linux/slab.h>
 #include <linux/idr.h>
 #include <linux/pci.h>
+#include <cxl/cxl.h>
 #include <cxlmem.h>
 #include "trace.h"
 #include "core.h"
@@ -576,9 +577,16 @@ static const struct device_type cxl_memdev_type = {
 	.groups = cxl_memdev_attribute_groups,
 };
 
+static const struct device_type cxl_accel_memdev_type = {
+	.name = "cxl_accel_memdev",
+	.release = cxl_memdev_release,
+	.devnode = cxl_memdev_devnode,
+};
+
 bool is_cxl_memdev(const struct device *dev)
 {
-	return dev->type == &cxl_memdev_type;
+	return (dev->type == &cxl_memdev_type ||
+		dev->type == &cxl_accel_memdev_type);
 }
 EXPORT_SYMBOL_NS_GPL(is_cxl_memdev, "CXL");
 
@@ -773,7 +781,10 @@ static struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds,
 	dev->parent = cxlds->dev;
 	dev->bus = &cxl_bus_type;
 	dev->devt = MKDEV(cxl_mem_major, cxlmd->id);
-	dev->type = &cxl_memdev_type;
+	if (cxlds->type == CXL_DEVTYPE_DEVMEM)
+		dev->type = &cxl_accel_memdev_type;
+	else
+		dev->type = &cxl_memdev_type;
 	device_set_pm_not_required(dev);
 	INIT_WORK(&cxlmd->detach_work, detach_memdev);
 
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index 281546de426e..c98db6f18aa2 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -34,10 +34,6 @@
 	(FIELD_GET(CXLMDEV_RESET_NEEDED_MASK, status) !=                       \
 	 CXLMDEV_RESET_NEEDED_NOT)
 
-struct cxl_memdev_attach {
-	int (*probe)(struct cxl_memdev *cxlmd);
-};
-
 /**
  * struct cxl_memdev - CXL bus object representing a Type-3 Memory Device
  * @dev: driver core device object
@@ -103,8 +99,6 @@ static inline bool is_cxl_endpoint(struct cxl_port *port)
 
 struct cxl_memdev *__devm_cxl_add_memdev(struct cxl_dev_state *cxlds,
 					 const struct cxl_memdev_attach *attach);
-struct cxl_memdev *devm_cxl_add_memdev(struct cxl_dev_state *cxlds,
-				       const struct cxl_memdev_attach *attach);
 int devm_cxl_sanitize_setup_notifier(struct device *host,
 				     struct cxl_memdev *cxlmd);
 struct cxl_memdev_state;
diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
index fcffe24dcb42..ff858318091f 100644
--- a/drivers/cxl/mem.c
+++ b/drivers/cxl/mem.c
@@ -65,6 +65,26 @@ static int cxl_debugfs_poison_clear(void *data, u64 dpa)
 DEFINE_DEBUGFS_ATTRIBUTE(cxl_poison_clear_fops, NULL,
 			 cxl_debugfs_poison_clear, "%llx\n");
 
+static void cxl_memdev_poison_enable(struct cxl_memdev_state *mds,
+				     struct cxl_memdev *cxlmd,
+				     struct dentry *dentry)
+{
+	/*
+	 * Avoid poison debugfs for DEVMEM aka accelerators as they rely on
+	 * cxl_memdev_state.
+	 */
+	if (!mds)
+		return;
+
+	if (test_bit(CXL_POISON_ENABLED_INJECT, mds->poison.enabled_cmds))
+		debugfs_create_file("inject_poison", 0200, dentry, cxlmd,
+				    &cxl_poison_inject_fops);
+
+	if (test_bit(CXL_POISON_ENABLED_CLEAR, mds->poison.enabled_cmds))
+		debugfs_create_file("clear_poison", 0200, dentry, cxlmd,
+				    &cxl_poison_clear_fops);
+}
+
 static int cxl_mem_probe(struct device *dev)
 {
 	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
@@ -92,12 +112,7 @@ static int cxl_mem_probe(struct device *dev)
 	dentry = cxl_debugfs_create_dir(dev_name(dev));
 	debugfs_create_devm_seqfile(dev, "dpamem", dentry, cxl_mem_dpa_show);
 
-	if (test_bit(CXL_POISON_ENABLED_INJECT, mds->poison.enabled_cmds))
-		debugfs_create_file("inject_poison", 0200, dentry, cxlmd,
-				    &cxl_poison_inject_fops);
-	if (test_bit(CXL_POISON_ENABLED_CLEAR, mds->poison.enabled_cmds))
-		debugfs_create_file("clear_poison", 0200, dentry, cxlmd,
-				    &cxl_poison_clear_fops);
+	cxl_memdev_poison_enable(mds, cxlmd, dentry);
 
 	rc = devm_add_action_or_reset(dev, remove_debugfs, dentry);
 	if (rc)
@@ -206,16 +221,24 @@ static ssize_t trigger_poison_list_store(struct device *dev,
 }
 static DEVICE_ATTR_WO(trigger_poison_list);
 
-static umode_t cxl_mem_visible(struct kobject *kobj, struct attribute *a, int n)
+static bool cxl_poison_attr_visible(struct kobject *kobj, struct attribute *a)
 {
 	struct device *dev = kobj_to_dev(kobj);
 	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
 	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlmd->cxlds);
 
-	if (a == &dev_attr_trigger_poison_list.attr)
-		if (!test_bit(CXL_POISON_ENABLED_LIST,
-			      mds->poison.enabled_cmds))
-			return 0;
+	if (!mds ||
+	    !test_bit(CXL_POISON_ENABLED_LIST, mds->poison.enabled_cmds))
+		return false;
+
+	return true;
+}
+
+static umode_t cxl_mem_visible(struct kobject *kobj, struct attribute *a, int n)
+{
+	if (a == &dev_attr_trigger_poison_list.attr &&
+	    !cxl_poison_attr_visible(kobj, a))
+		return 0;
 
 	return a->mode;
 }
diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h
index 1346771edc4e..10a9b8fa2f6b 100644
--- a/include/cxl/cxl.h
+++ b/include/cxl/cxl.h
@@ -149,6 +149,10 @@ struct cxl_dpa_partition {
 
 #define CXL_NR_PARTITIONS_MAX 2
 
+struct cxl_memdev_attach {
+	int (*probe)(struct cxl_memdev *cxlmd);
+};
+
 /**
  * struct cxl_dev_state - The driver device state
  *
@@ -225,4 +229,6 @@ struct cxl_dev_state *_devm_cxl_dev_state_create(struct device *dev,
 	})
 
 int cxl_set_capacity(struct cxl_dev_state *cxlds, u64 capacity);
+struct cxl_memdev *devm_cxl_add_memdev(struct cxl_dev_state *cxlds,
+				       const struct cxl_memdev_attach *attach);
 #endif /* __CXL_CXL_H__ */
-- 
2.34.1

Re: [PATCH v25 04/11] cxl: Prepare memdev creation for type2

[Dan Williams]

alejandro.lucero-palau@ wrote:
> From: Alejandro Lucero <alucerop@amd.com>
> 
> Current cxl core is relying on a CXL_DEVTYPE_CLASSMEM type device when
> creating a memdev leading to problems when obtaining cxl_memdev_state
> references from a CXL_DEVTYPE_DEVMEM type.
> 
> Modify check for obtaining cxl_memdev_state adding CXL_DEVTYPE_DEVMEM
> support.
> 
> Make devm_cxl_add_memdev accessible from an accel driver.
> 
[..]
> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
> index fcffe24dcb42..ff858318091f 100644
> --- a/drivers/cxl/mem.c
> +++ b/drivers/cxl/mem.c
> @@ -65,6 +65,26 @@ static int cxl_debugfs_poison_clear(void *data, u64 dpa)
>  DEFINE_DEBUGFS_ATTRIBUTE(cxl_poison_clear_fops, NULL,
>  			 cxl_debugfs_poison_clear, "%llx\n");
>  
> +static void cxl_memdev_poison_enable(struct cxl_memdev_state *mds,
> +				     struct cxl_memdev *cxlmd,
> +				     struct dentry *dentry)
> +{
> +	/*
> +	 * Avoid poison debugfs for DEVMEM aka accelerators as they rely on
> +	 * cxl_memdev_state.
> +	 */

There is nothing stopping an accelerator from having a CXL mailbox, so
should probably circle back to either make this comment more generic, or
just delete it.

[PATCH v25 05/11] sfc: create type2 cxl memdev

[alejandro.lucero-palau]

From: Alejandro Lucero <alucerop@amd.com>

Use cxl API for creating a cxl memory device using the type2
cxl_dev_state struct.

Signed-off-by: Alejandro Lucero <alucerop@amd.com>
Reviewed-by: Martin Habets <habetsm.xilinx@gmail.com>
Reviewed-by: Fan Ni <fan.ni@samsung.com>
Acked-by: Edward Cree <ecree.xilinx@gmail.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/net/ethernet/sfc/efx_cxl.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c
index 6619084a77d8..63e6f277ae9f 100644
--- a/drivers/net/ethernet/sfc/efx_cxl.c
+++ b/drivers/net/ethernet/sfc/efx_cxl.c
@@ -75,6 +75,12 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
 		return -ENODEV;
 	}
 
+	cxl->cxlmd = devm_cxl_add_memdev(&cxl->cxlds, NULL);
+	if (IS_ERR(cxl->cxlmd)) {
+		pci_err(pci_dev, "CXL accel memdev creation failed");
+		return PTR_ERR(cxl->cxlmd);
+	}
+
 	probe_data->cxl = cxl;
 
 	return 0;
-- 
2.34.1

Re: [PATCH v25 05/11] sfc: create type2 cxl memdev

[kernel test robot]

Hi,

kernel test robot noticed the following build errors:

[auto build test ERROR on 64584273dfb8a1e5fc7d78094ba22a93c204b44e]

url:    https://github.com/intel-lab-lkp/linux/commits/alejandro-lucero-palau-amd-com/sfc-add-cxl-support/20260331-055820
base:   64584273dfb8a1e5fc7d78094ba22a93c204b44e
patch link:    https://lore.kernel.org/r/20260330143827.1278677-6-alejandro.lucero-palau%40amd.com
patch subject: [PATCH v25 05/11] sfc: create type2 cxl memdev
config: x86_64-randconfig-003-20260331 (https://download.01.org/0day-ci/archive/20260401/202604010047.zovyb3Z0-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260401/202604010047.zovyb3Z0-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202604010047.zovyb3Z0-lkp@intel.com/

All errors (new ones prefixed by >>):

   ld: vmlinux.o: in function `efx_cxl_init':
>> drivers/net/ethernet/sfc/efx_cxl.c:78:(.text+0xd33fcd): undefined reference to `devm_cxl_add_memdev'


vim +78 drivers/net/ethernet/sfc/efx_cxl.c

    16	
    17	int efx_cxl_init(struct efx_probe_data *probe_data)
    18	{
    19		struct efx_nic *efx = &probe_data->efx;
    20		struct pci_dev *pci_dev = efx->pci_dev;
    21		struct efx_cxl *cxl;
    22		u16 dvsec;
    23		int rc;
    24	
    25		probe_data->cxl_pio_initialised = false;
    26	
    27		/* Is the device configured with and using CXL? */
    28		if (!pcie_is_cxl(pci_dev))
    29			return 0;
    30	
    31		dvsec = pci_find_dvsec_capability(pci_dev, PCI_VENDOR_ID_CXL,
    32						  PCI_DVSEC_CXL_DEVICE);
    33		if (!dvsec) {
    34			pci_info(pci_dev, "CXL_DVSEC_PCIE_DEVICE capability not found\n");
    35			return 0;
    36		}
    37	
    38		pci_dbg(pci_dev, "CXL_DVSEC_PCIE_DEVICE capability found\n");
    39	
    40		/* Create a cxl_dev_state embedded in the cxl struct using cxl core api
    41		 * specifying no mbox available.
    42		 */
    43		cxl = devm_cxl_dev_state_create(&pci_dev->dev, CXL_DEVTYPE_DEVMEM,
    44						pci_dev->dev.id, dvsec, struct efx_cxl,
    45						cxlds, false);
    46	
    47		if (!cxl)
    48			return -ENOMEM;
    49	
    50		rc = cxl_pci_setup_regs(pci_dev, CXL_REGLOC_RBI_COMPONENT,
    51					&cxl->cxlds.reg_map);
    52		if (rc) {
    53			pci_err(pci_dev, "No component registers\n");
    54			return rc;
    55		}
    56	
    57		if (!cxl->cxlds.reg_map.component_map.hdm_decoder.valid) {
    58			pci_err(pci_dev, "Expected HDM component register not found\n");
    59			return -ENODEV;
    60		}
    61	
    62		if (!cxl->cxlds.reg_map.component_map.ras.valid) {
    63			pci_err(pci_dev, "Expected RAS component register not found\n");
    64			return -ENODEV;
    65		}
    66	
    67		/* Set media ready explicitly as there are neither mailbox for checking
    68		 * this state nor the CXL register involved, both not mandatory for
    69		 * type2.
    70		 */
    71		cxl->cxlds.media_ready = true;
    72	
    73		if (cxl_set_capacity(&cxl->cxlds, EFX_CTPIO_BUFFER_SIZE)) {
    74			pci_err(pci_dev, "dpa capacity setup failed\n");
    75			return -ENODEV;
    76		}
    77	
  > 78		cxl->cxlmd = devm_cxl_add_memdev(&cxl->cxlds, NULL);
    79		if (IS_ERR(cxl->cxlmd)) {
    80			pci_err(pci_dev, "CXL accel memdev creation failed");
    81			return PTR_ERR(cxl->cxlmd);
    82		}
    83	
    84		probe_data->cxl = cxl;
    85	
    86		return 0;
    87	}
    88	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v25 05/11] sfc: create type2 cxl memdev

[Dan Williams]

alejandro.lucero-palau@ wrote:
> From: Alejandro Lucero <alucerop@amd.com>
> 
> Use cxl API for creating a cxl memory device using the type2
> cxl_dev_state struct.
> 
> Signed-off-by: Alejandro Lucero <alucerop@amd.com>
> Reviewed-by: Martin Habets <habetsm.xilinx@gmail.com>
> Reviewed-by: Fan Ni <fan.ni@samsung.com>
> Acked-by: Edward Cree <ecree.xilinx@gmail.com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/net/ethernet/sfc/efx_cxl.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c
> index 6619084a77d8..63e6f277ae9f 100644
> --- a/drivers/net/ethernet/sfc/efx_cxl.c
> +++ b/drivers/net/ethernet/sfc/efx_cxl.c
> @@ -75,6 +75,12 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
>  		return -ENODEV;
>  	}
>  
> +	cxl->cxlmd = devm_cxl_add_memdev(&cxl->cxlds, NULL);

Did this forget about:

29317f8dc6ed cxl/mem: Introduce cxl_memdev_attach for CXL-dependent operation

?

tl;dr: see the replacement patch below for this and the next several
patches. Untested! I will write a cxl_test for it tomorrow. For now,
take it as a "something like this".

---
...longer story.

The difference between general memory expansion and other CXL memory is
that the driver has a use case for the memory. For general memory
expansion the memdev *is* the use case. A NULL attach parameter means
there is value in continuing if the memdev is not attached to the CXL
domain or not mapped in any region.

A non-NULL @attach addresses all the caller's requirements at attach
time. It also arranges by default to trigger ->remove() if anything in
the CXL topology is disturbed while the region is in use. Anything from
hotplug, to modprobe -r cxl_acpi, or cxl disable-port.

For efx a finer grained failure mode can be built on top of this by
adding a remove action that something like efx_tx_may_pio() could use to
dynamically fallback away from CXL if something goes wrong.

Until then though the CXL subsystem, because CXL allows for dynamic
memory reassignment, goes to great lengths to ensure that it can all be
dynamically torn down. So consumers that register relative to a cxl_port
topology need to be prepared for that port hierarchy to be torn down and
evacuate their usage of any regions.

The rest of the patches in this set are racy because none of the results
are stable (locks dropped on return) and any teardown is silent (no
notification of teardown).

All of this wants to stay local to the cxl_core and should not be
anything that accelerator drivers need to worry about. Accelerator
drivers just register, get a physical address range to map, and off they
go.

The meat of this patch is cxl_memdev_attach_region(). The rest of is
protection against userspace messing up the region configuration.

---
diff --git a/drivers/net/ethernet/sfc/efx_cxl.h b/drivers/net/ethernet/sfc/efx_cxl.h
index 961639cef692..bfd8633a1e01 100644
--- a/drivers/net/ethernet/sfc/efx_cxl.h
+++ b/drivers/net/ethernet/sfc/efx_cxl.h
@@ -24,10 +24,7 @@ struct efx_probe_data;
 struct efx_cxl {
 	struct cxl_dev_state cxlds;
 	struct cxl_memdev *cxlmd;
-	struct cxl_root_decoder *cxlrd;
-	struct cxl_port *endpoint;
-	struct cxl_endpoint_decoder *cxled;
-	struct cxl_region *efx_region;
+	struct cxl_attach_region attach;
 	void __iomem *ctpio_cxl;
 };
 
diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h
index 10a9b8fa2f6b..1698d15ec1ca 100644
--- a/include/cxl/cxl.h
+++ b/include/cxl/cxl.h
@@ -153,6 +153,22 @@ struct cxl_memdev_attach {
 	int (*probe)(struct cxl_memdev *cxlmd);
 };
 
+/**
+ * struct cxl_attach_region - coordinate mapping a region at memdev registration
+ * @attach: common core attachment descriptor
+ * @region: 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.
+ */
+struct cxl_attach_region {
+	struct cxl_memdev_attach attach;
+	struct range region;
+};
+
+int cxl_memdev_attach_region(struct cxl_memdev *cxlmd);
+
 /**
  * struct cxl_dev_state - The driver device state
  *
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 3edb5703d6de..5d60e6c0a89e 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -406,6 +406,40 @@ static int __commit(struct cxl_region *cxlr)
 	return 0;
 }
 
+/*
+ * When a region's memdevs specify an @attach method the attach provider is
+ * responsible for dispositioning the region for both probe and userspace
+ * management
+ */
+static bool cxl_region_has_memdev_attach(struct cxl_region *cxlr)
+{
+	struct cxl_region_params *p = &cxlr->params;
+
+	for (int i = 0; i < p->nr_targets; i++) {
+		struct cxl_endpoint_decoder *cxled = p->targets[i];
+		struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
+
+		if (cxlmd->attach)
+			return true;
+	}
+
+	return false;
+}
+
+static int check_region_mutable(struct cxl_region *cxlr)
+{
+	int rc;
+
+	ACQUIRE(rwsem_read_intr, rwsem)(&cxl_rwsem.region);
+	if ((rc = ACQUIRE_ERR(rwsem_read_intr, &rwsem)))
+		return rc;
+
+	if (cxl_region_has_memdev_attach(cxlr))
+		return -EBUSY;
+
+	return 0;
+}
+
 static ssize_t commit_store(struct device *dev, struct device_attribute *attr,
 			    const char *buf, size_t len)
 {
@@ -418,6 +452,10 @@ static ssize_t commit_store(struct device *dev, struct device_attribute *attr,
 	if (rc)
 		return rc;
 
+	rc = check_region_mutable(cxlr);
+	if (rc)
+		return rc;
+
 	if (commit) {
 		rc = __commit(cxlr);
 		if (rc)
@@ -2768,17 +2806,23 @@ static ssize_t delete_region_store(struct device *dev,
 {
 	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(dev);
 	struct cxl_port *port = to_cxl_port(dev->parent);
-	struct cxl_region *cxlr;
+	int rc;
 
-	cxlr = cxl_find_region_by_name(cxlrd, buf);
+	struct cxl_region *cxlr __free(put_cxl_region) =
+		cxl_find_region_by_name(cxlrd, buf);
 	if (IS_ERR(cxlr))
 		return PTR_ERR(cxlr);
 
+	rc = check_region_mutable(cxlr);
+	if (rc)
+		return rc;
+
 	devm_release_action(port->uport_dev, unregister_region, cxlr);
-	put_device(&cxlr->dev);
 
 	return len;
 }
+
+
 DEVICE_ATTR_WO(delete_region);
 
 static void cxl_pmem_region_release(struct device *dev)
@@ -4223,6 +4267,88 @@ static int cxl_region_can_probe(struct cxl_region *cxlr)
 	return 0;
 }
 
+static int first_mapped_decoder(struct device *dev, const void *data)
+{
+	struct cxl_endpoint_decoder *cxled;
+
+	if (!is_endpoint_decoder(dev))
+		return 0;
+
+	cxled = to_cxl_endpoint_decoder(dev);
+	if (cxled->cxld.region)
+		return 1;
+
+	return 0;
+}
+
+/*
+ * As this is running in endpoint port remove context it does not race cxl_root
+ * destruction since port topologies are always removed depth first.
+ */
+static void cxl_endpoint_region_autoremove(void *_cxlr)
+{
+	struct cxl_region *cxlr = _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);
+}
+
+/*
+ * 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;
+	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;
+	}
+
+	cxled = to_cxl_endpoint_decoder(dev);
+	cxlr = cxled->cxld.region;
+	rc = devm_add_action_or_reset(&endpoint->dev,
+				      cxl_endpoint_region_autoremove, cxlr);
+	if (rc)
+		return 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));
+		return -ENXIO;
+	}
+
+	attach->region = (struct range) {
+		.start = cxlr->params.res->start,
+		.end = cxlr->params.res->end,
+	};
+	return 0;
+}
+EXPORT_SYMBOL_NS_GPL(cxl_memdev_attach_region, "CXL");
+
 static int cxl_region_probe(struct device *dev)
 {
 	struct cxl_region *cxlr = to_cxl_region(dev);
@@ -4254,6 +4380,9 @@ static int cxl_region_probe(struct device *dev)
 	if (rc)
 		return rc;
 
+	if (cxl_region_has_memdev_attach(cxlr))
+		return 0;
+
 	switch (cxlr->mode) {
 	case CXL_PARTMODE_PMEM:
 		rc = devm_cxl_region_edac_register(cxlr);
diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c
index 6619084a77d8..a388f3120dd4 100644
--- a/drivers/net/ethernet/sfc/efx_cxl.c
+++ b/drivers/net/ethernet/sfc/efx_cxl.c
@@ -75,6 +75,25 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
 		return -ENODEV;
 	}
 
+	cxl->attach = (struct cxl_attach_region) {
+		.attach = {
+			.probe = cxl_memdev_attach_region,
+		},
+	};
+	cxl->cxlmd = devm_cxl_add_memdev(&cxl->cxlds, &cxl->attach.attach);
+	if (IS_ERR(cxl->cxlmd)) {
+		pci_err(pci_dev, "CXL accel memdev creation failed");
+		return PTR_ERR(cxl->cxlmd);
+	}
+
+	cxl->ctpio_cxl = ioremap(cxl->attach.region.start,
+				 range_len(&cxl->attach.region));
+	if (!cxl->ctpio_cxl) {
+		pci_err(pci_dev, "CXL ioremap region (%pra) failed", &range);
+		return -ENOMEM;
+	}
+
+
 	probe_data->cxl = cxl;
 
 	return 0;
@@ -82,6 +101,10 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
 
 void efx_cxl_exit(struct efx_probe_data *probe_data)
 {
+	if (!probe_data->cxl)
+		return;
+
+	iounmap(probe_data->cxl->ctpio_cxl);
 }
 
 MODULE_IMPORT_NS("CXL");

Re: [PATCH v25 05/11] sfc: create type2 cxl memdev

[Alejandro Lucero Palau]

On 4/1/26 06:17, Dan Williams wrote:
> alejandro.lucero-palau@ wrote:
>> From: Alejandro Lucero <alucerop@amd.com>
>>
>> Use cxl API for creating a cxl memory device using the type2
>> cxl_dev_state struct.
>>
>> Signed-off-by: Alejandro Lucero <alucerop@amd.com>
>> Reviewed-by: Martin Habets <habetsm.xilinx@gmail.com>
>> Reviewed-by: Fan Ni <fan.ni@samsung.com>
>> Acked-by: Edward Cree <ecree.xilinx@gmail.com>
>> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
>> ---
>>   drivers/net/ethernet/sfc/efx_cxl.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c
>> index 6619084a77d8..63e6f277ae9f 100644
>> --- a/drivers/net/ethernet/sfc/efx_cxl.c
>> +++ b/drivers/net/ethernet/sfc/efx_cxl.c
>> @@ -75,6 +75,12 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
>>   		return -ENODEV;
>>   	}
>>   
>> +	cxl->cxlmd = devm_cxl_add_memdev(&cxl->cxlds, NULL);
> Did this forget about:
>
> 29317f8dc6ed cxl/mem: Introduce cxl_memdev_attach for CXL-dependent operation
>
> ?


I did not. The idea behind attach does not make sense for sfc, so I do 
not use that option. I did discuss this with you at LPC and based on 
previous comments which had not replies (case 1 in here):


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


 From our conversation at LPC, there is no reason for a CXL device 
connected to the CXL Root Bridge not having its memdev properly 
initialized. I did ask you specifically about this, and you mentioned 
links on CXL switches not ready, but as I said then and repeat now, this 
is not what sfc driver expects at all. Only a port from a CXL Root 
Bridge makes sense due to the latencies involved with more complex CXL 
topologies.


So, from that list I wrote in that old thread where I tried to summarize 
the problems and clarify the confusion behind different concerns, the 
only issue is someone removing cxl_acpi. I do not think that should be 
possible at all, so something should be added for other modules 
depending on it avoiding its removal. I would say any CXL port created 
is (in X86) dependent on cxl_acpi, so something at port creation 
invoking a exported cxl_acpi function could do it, like a cxl_acpi_users 
counter. This could also help for visibility to user space about its usage.


Therefore, I do not like the changes you propose here. But, if this 
proposal is the only way this can go through in time for 7.1, I will 
resign myself to it. It is not clear I'll be working on CXL in the near 
future, so any progress even without my agreement is better than nothing.


> tl;dr: see the replacement patch below for this and the next several
> patches. Untested! I will write a cxl_test for it tomorrow. For now,
> take it as a "something like this".
>
> ---
> ...longer story.
>
> The difference between general memory expansion and other CXL memory is
> that the driver has a use case for the memory. For general memory
> expansion the memdev *is* the use case. A NULL attach parameter means
> there is value in continuing if the memdev is not attached to the CXL
> domain or not mapped in any region.
>
> A non-NULL @attach addresses all the caller's requirements at attach
> time. It also arranges by default to trigger ->remove() if anything in
> the CXL topology is disturbed while the region is in use. Anything from
> hotplug, to modprobe -r cxl_acpi, or cxl disable-port.
>
> For efx a finer grained failure mode can be built on top of this by
> adding a remove action that something like efx_tx_may_pio() could use to
> dynamically fallback away from CXL if something goes wrong.
>
> Until then though the CXL subsystem, because CXL allows for dynamic
> memory reassignment, goes to great lengths to ensure that it can all be
> dynamically torn down. So consumers that register relative to a cxl_port
> topology need to be prepared for that port hierarchy to be torn down and
> evacuate their usage of any regions.
>
> The rest of the patches in this set are racy because none of the results
> are stable (locks dropped on return) and any teardown is silent (no
> notification of teardown).
>
> All of this wants to stay local to the cxl_core and should not be
> anything that accelerator drivers need to worry about. Accelerator
> drivers just register, get a physical address range to map, and off they
> go.
>
> The meat of this patch is cxl_memdev_attach_region(). The rest of is
> protection against userspace messing up the region configuration.
>
> ---
> diff --git a/drivers/net/ethernet/sfc/efx_cxl.h b/drivers/net/ethernet/sfc/efx_cxl.h
> index 961639cef692..bfd8633a1e01 100644
> --- a/drivers/net/ethernet/sfc/efx_cxl.h
> +++ b/drivers/net/ethernet/sfc/efx_cxl.h
> @@ -24,10 +24,7 @@ struct efx_probe_data;
>   struct efx_cxl {
>   	struct cxl_dev_state cxlds;
>   	struct cxl_memdev *cxlmd;
> -	struct cxl_root_decoder *cxlrd;
> -	struct cxl_port *endpoint;
> -	struct cxl_endpoint_decoder *cxled;
> -	struct cxl_region *efx_region;
> +	struct cxl_attach_region attach;
>   	void __iomem *ctpio_cxl;
>   };
>   
> diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h
> index 10a9b8fa2f6b..1698d15ec1ca 100644
> --- a/include/cxl/cxl.h
> +++ b/include/cxl/cxl.h
> @@ -153,6 +153,22 @@ struct cxl_memdev_attach {
>   	int (*probe)(struct cxl_memdev *cxlmd);
>   };
>   
> +/**
> + * struct cxl_attach_region - coordinate mapping a region at memdev registration
> + * @attach: common core attachment descriptor
> + * @region: 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.
> + */
> +struct cxl_attach_region {
> +	struct cxl_memdev_attach attach;
> +	struct range region;
> +};
> +
> +int cxl_memdev_attach_region(struct cxl_memdev *cxlmd);
> +
>   /**
>    * struct cxl_dev_state - The driver device state
>    *
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 3edb5703d6de..5d60e6c0a89e 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -406,6 +406,40 @@ static int __commit(struct cxl_region *cxlr)
>   	return 0;
>   }
>   
> +/*
> + * When a region's memdevs specify an @attach method the attach provider is
> + * responsible for dispositioning the region for both probe and userspace
> + * management
> + */
> +static bool cxl_region_has_memdev_attach(struct cxl_region *cxlr)
> +{
> +	struct cxl_region_params *p = &cxlr->params;
> +
> +	for (int i = 0; i < p->nr_targets; i++) {
> +		struct cxl_endpoint_decoder *cxled = p->targets[i];
> +		struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
> +
> +		if (cxlmd->attach)
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
> +static int check_region_mutable(struct cxl_region *cxlr)
> +{
> +	int rc;
> +
> +	ACQUIRE(rwsem_read_intr, rwsem)(&cxl_rwsem.region);
> +	if ((rc = ACQUIRE_ERR(rwsem_read_intr, &rwsem)))
> +		return rc;
> +
> +	if (cxl_region_has_memdev_attach(cxlr))
> +		return -EBUSY;
> +
> +	return 0;
> +}
> +
>   static ssize_t commit_store(struct device *dev, struct device_attribute *attr,
>   			    const char *buf, size_t len)
>   {
> @@ -418,6 +452,10 @@ static ssize_t commit_store(struct device *dev, struct device_attribute *attr,
>   	if (rc)
>   		return rc;
>   
> +	rc = check_region_mutable(cxlr);
> +	if (rc)
> +		return rc;
> +
>   	if (commit) {
>   		rc = __commit(cxlr);
>   		if (rc)
> @@ -2768,17 +2806,23 @@ static ssize_t delete_region_store(struct device *dev,
>   {
>   	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(dev);
>   	struct cxl_port *port = to_cxl_port(dev->parent);
> -	struct cxl_region *cxlr;
> +	int rc;
>   
> -	cxlr = cxl_find_region_by_name(cxlrd, buf);
> +	struct cxl_region *cxlr __free(put_cxl_region) =
> +		cxl_find_region_by_name(cxlrd, buf);
>   	if (IS_ERR(cxlr))
>   		return PTR_ERR(cxlr);
>   
> +	rc = check_region_mutable(cxlr);
> +	if (rc)
> +		return rc;
> +
>   	devm_release_action(port->uport_dev, unregister_region, cxlr);
> -	put_device(&cxlr->dev);
>   
>   	return len;
>   }
> +
> +
>   DEVICE_ATTR_WO(delete_region);
>   
>   static void cxl_pmem_region_release(struct device *dev)
> @@ -4223,6 +4267,88 @@ static int cxl_region_can_probe(struct cxl_region *cxlr)
>   	return 0;
>   }
>   
> +static int first_mapped_decoder(struct device *dev, const void *data)
> +{
> +	struct cxl_endpoint_decoder *cxled;
> +
> +	if (!is_endpoint_decoder(dev))
> +		return 0;
> +
> +	cxled = to_cxl_endpoint_decoder(dev);
> +	if (cxled->cxld.region)
> +		return 1;
> +
> +	return 0;
> +}
> +
> +/*
> + * As this is running in endpoint port remove context it does not race cxl_root
> + * destruction since port topologies are always removed depth first.
> + */
> +static void cxl_endpoint_region_autoremove(void *_cxlr)
> +{
> +	struct cxl_region *cxlr = _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);
> +}
> +
> +/*
> + * 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;
> +	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;
> +	}
> +
> +	cxled = to_cxl_endpoint_decoder(dev);
> +	cxlr = cxled->cxld.region;
> +	rc = devm_add_action_or_reset(&endpoint->dev,
> +				      cxl_endpoint_region_autoremove, cxlr);
> +	if (rc)
> +		return 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));
> +		return -ENXIO;
> +	}
> +
> +	attach->region = (struct range) {
> +		.start = cxlr->params.res->start,
> +		.end = cxlr->params.res->end,
> +	};
> +	return 0;
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_memdev_attach_region, "CXL");
> +
>   static int cxl_region_probe(struct device *dev)
>   {
>   	struct cxl_region *cxlr = to_cxl_region(dev);
> @@ -4254,6 +4380,9 @@ static int cxl_region_probe(struct device *dev)
>   	if (rc)
>   		return rc;
>   
> +	if (cxl_region_has_memdev_attach(cxlr))
> +		return 0;
> +
>   	switch (cxlr->mode) {
>   	case CXL_PARTMODE_PMEM:
>   		rc = devm_cxl_region_edac_register(cxlr);
> diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c
> index 6619084a77d8..a388f3120dd4 100644
> --- a/drivers/net/ethernet/sfc/efx_cxl.c
> +++ b/drivers/net/ethernet/sfc/efx_cxl.c
> @@ -75,6 +75,25 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
>   		return -ENODEV;
>   	}
>   
> +	cxl->attach = (struct cxl_attach_region) {
> +		.attach = {
> +			.probe = cxl_memdev_attach_region,
> +		},
> +	};
> +	cxl->cxlmd = devm_cxl_add_memdev(&cxl->cxlds, &cxl->attach.attach);
> +	if (IS_ERR(cxl->cxlmd)) {
> +		pci_err(pci_dev, "CXL accel memdev creation failed");
> +		return PTR_ERR(cxl->cxlmd);
> +	}
> +
> +	cxl->ctpio_cxl = ioremap(cxl->attach.region.start,
> +				 range_len(&cxl->attach.region));
> +	if (!cxl->ctpio_cxl) {
> +		pci_err(pci_dev, "CXL ioremap region (%pra) failed", &range);
> +		return -ENOMEM;
> +	}
> +
> +
>   	probe_data->cxl = cxl;
>   
>   	return 0;
> @@ -82,6 +101,10 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
>   
>   void efx_cxl_exit(struct efx_probe_data *probe_data)
>   {
> +	if (!probe_data->cxl)
> +		return;
> +
> +	iounmap(probe_data->cxl->ctpio_cxl);
>   }
>   
>   MODULE_IMPORT_NS("CXL");

Re: [PATCH v25 05/11] sfc: create type2 cxl memdev

[Dan Williams]

Alejandro Lucero Palau wrote:
> 
> On 4/1/26 06:17, Dan Williams wrote:
> > alejandro.lucero-palau@ wrote:
> >> From: Alejandro Lucero <alucerop@amd.com>
> >>
> >> Use cxl API for creating a cxl memory device using the type2
> >> cxl_dev_state struct.
> >>
> >> Signed-off-by: Alejandro Lucero <alucerop@amd.com>
> >> Reviewed-by: Martin Habets <habetsm.xilinx@gmail.com>
> >> Reviewed-by: Fan Ni <fan.ni@samsung.com>
> >> Acked-by: Edward Cree <ecree.xilinx@gmail.com>
> >> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> >> ---
> >>   drivers/net/ethernet/sfc/efx_cxl.c | 6 ++++++
> >>   1 file changed, 6 insertions(+)
> >>
> >> diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c
> >> index 6619084a77d8..63e6f277ae9f 100644
> >> --- a/drivers/net/ethernet/sfc/efx_cxl.c
> >> +++ b/drivers/net/ethernet/sfc/efx_cxl.c
> >> @@ -75,6 +75,12 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
> >>   		return -ENODEV;
> >>   	}
> >>   
> >> +	cxl->cxlmd = devm_cxl_add_memdev(&cxl->cxlds, NULL);
> > Did this forget about:
> >
> > 29317f8dc6ed cxl/mem: Introduce cxl_memdev_attach for CXL-dependent operation
> >
> > ?
> 
> 
> I did not. The idea behind attach does not make sense for sfc, so I do 
> not use that option. I did discuss this with you at LPC and based on 
> previous comments which had not replies (case 1 in here):
> 
> 
> https://lore.kernel.org/linux-cxl/836d06d6-a36f-4ba3-b7c9-ba8687ba2190@amd.com/
> 
> 
>  From our conversation at LPC, there is no reason for a CXL device 
> connected to the CXL Root Bridge not having its memdev properly 
> initialized.

The cxl_core can not make assumptions about attachment topology. It has
always been the case that the cxl_core needs to be prepared to handle
everything that the firmware did not. Error handling and kexec also lead to
situations where the boot configuration gets invalidated and needs
recovery.

> I did ask you specifically about this, and you mentioned links on CXL
> switches not ready, but as I said then and repeat now, this is not
> what sfc driver expects at all. Only a port from a CXL Root Bridge
> makes sense due to the latencies involved with more complex CXL
> topologies.
> 
> 
> So, from that list I wrote in that old thread where I tried to summarize 
> the problems and clarify the confusion behind different concerns, the 
> only issue is someone removing cxl_acpi. I do not think that should be 
> possible at all,

...but it *is* possible to remove cxl_acpi, it *is* possible to invoke
'cxl disable-port'. The fact that it is possible contributes to
complexity, but it also supports flexibility and is a building block for
error handling / recovery.

> so something should be added for other modules depending on it
> avoiding its removal. I would say any CXL port created is (in X86)
> dependent on cxl_acpi, so something at port creation invoking a
> exported cxl_acpi function could do it, like a cxl_acpi_users counter.
> This could also help for visibility to user space about its usage.

There are two choices, fail removal or handle removal. There are some
degrees of freedom that can be pared back, but outright blocking removal
is not one of them.

> Therefore, I do not like the changes you propose here. But, if this 
> proposal is the only way

It is not a matter of like or dislike, it is a matter of functional
correctness relative to the state of the subsystem today. Simply put,
one proposal handles the locking and lifetime concerns, the other does
not.

Re: [PATCH v25 05/11] sfc: create type2 cxl memdev

[Alejandro Lucero Palau]

On 4/1/26 22:53, Dan Williams wrote:
> Alejandro Lucero Palau wrote:
>> On 4/1/26 06:17, Dan Williams wrote:
>>> alejandro.lucero-palau@ wrote:
>>>> From: Alejandro Lucero <alucerop@amd.com>
>>>>
>>>> Use cxl API for creating a cxl memory device using the type2
>>>> cxl_dev_state struct.
>>>>
>>>> Signed-off-by: Alejandro Lucero <alucerop@amd.com>
>>>> Reviewed-by: Martin Habets <habetsm.xilinx@gmail.com>
>>>> Reviewed-by: Fan Ni <fan.ni@samsung.com>
>>>> Acked-by: Edward Cree <ecree.xilinx@gmail.com>
>>>> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>>>> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
>>>> ---
>>>>    drivers/net/ethernet/sfc/efx_cxl.c | 6 ++++++
>>>>    1 file changed, 6 insertions(+)
>>>>
>>>> diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c
>>>> index 6619084a77d8..63e6f277ae9f 100644
>>>> --- a/drivers/net/ethernet/sfc/efx_cxl.c
>>>> +++ b/drivers/net/ethernet/sfc/efx_cxl.c
>>>> @@ -75,6 +75,12 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
>>>>    		return -ENODEV;
>>>>    	}
>>>>    
>>>> +	cxl->cxlmd = devm_cxl_add_memdev(&cxl->cxlds, NULL);
>>> Did this forget about:
>>>
>>> 29317f8dc6ed cxl/mem: Introduce cxl_memdev_attach for CXL-dependent operation
>>>
>>> ?
>>
>> I did not. The idea behind attach does not make sense for sfc, so I do
>> not use that option. I did discuss this with you at LPC and based on
>> previous comments which had not replies (case 1 in here):
>>
>>
>> https://lore.kernel.org/linux-cxl/836d06d6-a36f-4ba3-b7c9-ba8687ba2190@amd.com/
>>
>>
>>   From our conversation at LPC, there is no reason for a CXL device
>> connected to the CXL Root Bridge not having its memdev properly
>> initialized.
> The cxl_core can not make assumptions about attachment topology. It has
> always been the case that the cxl_core needs to be prepared to handle
> everything that the firmware did not.


This is too much generic as an argument. If you want to convince me, I 
would need a real case where this can be a problem.

The current assumption and the only option supported with this last 
version is the BIOS *is* configuring the device HDM and there should be 
nothing to do from the cxl core in that regard:


1) BIOS configured and committed the device HDM: CXL.mem is used.

2) BIOS did not configure the HDM: CXL.mem is not used.


Any problem with the memdev initialization *requiring* your attach 
option is helpless ... if there exists a reason for it being helpful 
because the memdev initialization triggering the port and hdm 
initilization was not completely done, please explain why that could 
happen in the scenario I am presenting with an accelerator connected to 
a CXL Host Bridge port and the BIOS configuring the HDM based on the CFMWS.


> Error handling and kexec also lead to
> situations where the boot configuration gets invalidated and needs
> recovery.


Error handling based on RAS is a follow-up work. CXL reset is not 
supported ... and AFAIK, it is not supported by Type3 either or it has 
not been for a good bunch of time. If so, I do not understand why this 
should stop this patchset.


About kexec, it happens without a shutdown ... so if the system does it 
without proper handling, nothing we can do. If it is properly handled, 
the cxl core initilaization will happen ... is it handling the case of 
finding HDMs already configured (kexec will leave them untouched, won't 
it?)? From a type2 point of view and using this patchset, if kexec was 
executed after removing first the sfc driver, there should not be any 
problem. If I am missing something about this, please explain it in 
detail or give any reference I can look at. FWIW, I did use kexec a lot 
in embedded systems and had to deal with some hardware needing special 
treatment before triggering it, so happy to learn more about the 
implications with CXL.


>
>> I did ask you specifically about this, and you mentioned links on CXL
>> switches not ready, but as I said then and repeat now, this is not
>> what sfc driver expects at all. Only a port from a CXL Root Bridge
>> makes sense due to the latencies involved with more complex CXL
>> topologies.
>>
>>
>> So, from that list I wrote in that old thread where I tried to summarize
>> the problems and clarify the confusion behind different concerns, the
>> only issue is someone removing cxl_acpi. I do not think that should be
>> possible at all,
> ...but it *is* possible to remove cxl_acpi, it *is* possible to invoke
> 'cxl disable-port'. The fact that it is possible contributes to
> complexity, but it also supports flexibility and is a building block for
> error handling / recovery.


It is possible but, should it? When should it be allowed with drivers 
relying on its functionality?


>> so something should be added for other modules depending on it
>> avoiding its removal. I would say any CXL port created is (in X86)
>> dependent on cxl_acpi, so something at port creation invoking a
>> exported cxl_acpi function could do it, like a cxl_acpi_users counter.
>> This could also help for visibility to user space about its usage.
> There are two choices, fail removal or handle removal. There are some
> degrees of freedom that can be pared back, but outright blocking removal
> is not one of them.


Why not?


>> Therefore, I do not like the changes you propose here. But, if this
>> proposal is the only way
> It is not a matter of like or dislike, it is a matter of functional
> correctness relative to the state of the subsystem today. Simply put,
> one proposal handles the locking and lifetime concerns, the other does
> not.

Re: [PATCH v25 05/11] sfc: create type2 cxl memdev

[Dan Williams]

Alejandro Lucero Palau wrote:
[..]
> > ...but it *is* possible to remove cxl_acpi, it *is* possible to invoke
> > 'cxl disable-port'. The fact that it is possible contributes to
> > complexity, but it also supports flexibility and is a building block for
> > error handling / recovery.
> 
> 
> It is possible but, should it? When should it be allowed with drivers 
> relying on its functionality?

That is a good question, but not for this set. The immediate goal is
acquire a region safely, and that means complying with current locking
and lifetime rules. We can always evolve the core over time, but for the
present "just give me a region and call me back if something invalidates
it" is the status quo.

[PATCH v25 06/11] cxl/hdm: Add support for getting region from committed decoder

[alejandro.lucero-palau]

From: Alejandro Lucero <alucerop@amd.com>

A Type2 device configured by the BIOS can have its HDM committed and
a cxl region linked by auto discovery when the device memdev is created.

Add a function for a Type2 driver to obtain such a region.

Signed-off-by: Alejandro Lucero <alucerop@amd.com>
---
 drivers/cxl/core/hdm.c | 48 ++++++++++++++++++++++++++++++++++++++++++
 include/cxl/cxl.h      |  2 ++
 2 files changed, 50 insertions(+)

diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
index c222e98ae736..ce5feb4b26ae 100644
--- a/drivers/cxl/core/hdm.c
+++ b/drivers/cxl/core/hdm.c
@@ -686,6 +686,54 @@ int cxl_dpa_alloc(struct cxl_endpoint_decoder *cxled, u64 size)
 	return devm_add_action_or_reset(&port->dev, cxl_dpa_release, cxled);
 }
 
+static int find_committed_endpoint_decoder(struct device *dev, const void *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;
+
+	return (cxld->flags & CXL_DECODER_F_ENABLE);
+}
+
+/**
+ * cxl_get_region_from_committed_decoder - obtain a pointer to a region
+ * @cxlmd: CXL memdev from an endpoint device
+ *
+ * An accelerator decoder can be set up by the firmware/BIOS and the auto
+ * discovery region creation triggered by the memdev object initialization.
+ * Using this function the related driver can obtain such a region.
+ *
+ * Only one committed HDM is expected, returning the first one found.
+ *
+ * Return pointer to a region or NULL
+ */
+struct cxl_region *cxl_get_region_from_committed_decoder(struct cxl_memdev *cxlmd)
+{
+	struct cxl_port *endpoint = cxlmd->endpoint;
+	struct cxl_endpoint_decoder *cxled;
+
+	if (!endpoint)
+		return NULL;
+
+	guard(rwsem_read)(&cxl_rwsem.dpa);
+	struct device *cxled_dev __free(put_device) =
+		device_find_child(&endpoint->dev, NULL,
+				  find_committed_endpoint_decoder);
+
+	if (!cxled_dev)
+		return NULL;
+
+	cxled = to_cxl_endpoint_decoder(cxled_dev);
+
+	return cxled->cxld.region;
+}
+EXPORT_SYMBOL_NS_GPL(cxl_get_region_from_committed_decoder, "CXL");
+
 static void cxld_set_interleave(struct cxl_decoder *cxld, u32 *ctrl)
 {
 	u16 eig;
diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h
index 10a9b8fa2f6b..05c6ff2404b0 100644
--- a/include/cxl/cxl.h
+++ b/include/cxl/cxl.h
@@ -231,4 +231,6 @@ struct cxl_dev_state *_devm_cxl_dev_state_create(struct device *dev,
 int cxl_set_capacity(struct cxl_dev_state *cxlds, u64 capacity);
 struct cxl_memdev *devm_cxl_add_memdev(struct cxl_dev_state *cxlds,
 				       const struct cxl_memdev_attach *attach);
+struct cxl_region;
+struct cxl_region *cxl_get_region_from_committed_decoder(struct cxl_memdev *cxlmd);
 #endif /* __CXL_CXL_H__ */
-- 
2.34.1

Re: [PATCH v25 06/11] cxl/hdm: Add support for getting region from committed decoder

[Dan Williams]

alejandro.lucero-palau@ wrote:
> From: Alejandro Lucero <alucerop@amd.com>
> 
> A Type2 device configured by the BIOS can have its HDM committed and
> a cxl region linked by auto discovery when the device memdev is created.
> 
> Add a function for a Type2 driver to obtain such a region.
> 
> Signed-off-by: Alejandro Lucero <alucerop@amd.com>
[..]
> +/**
> + * cxl_get_region_from_committed_decoder - obtain a pointer to a region
> + * @cxlmd: CXL memdev from an endpoint device
> + *
> + * An accelerator decoder can be set up by the firmware/BIOS and the auto
> + * discovery region creation triggered by the memdev object initialization.
> + * Using this function the related driver can obtain such a region.
> + *
> + * Only one committed HDM is expected, returning the first one found.
> + *
> + * Return pointer to a region or NULL
> + */
> +struct cxl_region *cxl_get_region_from_committed_decoder(struct cxl_memdev *cxlmd)
> +{
> +	struct cxl_port *endpoint = cxlmd->endpoint;
> +	struct cxl_endpoint_decoder *cxled;
> +
> +	if (!endpoint)
> +		return NULL;
> +
> +	guard(rwsem_read)(&cxl_rwsem.dpa);
> +	struct device *cxled_dev __free(put_device) =
> +		device_find_child(&endpoint->dev, NULL,
> +				  find_committed_endpoint_decoder);
> +
> +	if (!cxled_dev)
> +		return NULL;
> +
> +	cxled = to_cxl_endpoint_decoder(cxled_dev);
> +
> +	return cxled->cxld.region;
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_get_region_from_committed_decoder, "CXL");

As soon as this function returns there is no guarantee that the region
is still valid.

[PATCH v25 07/11] cxl: Add function for obtaining region range

[alejandro.lucero-palau]

From: Alejandro Lucero <alucerop@amd.com>

A CXL region struct contains the physical address to work with.

Type2 drivers can create a CXL region but have not access to the
related struct as it is defined as private by the kernel CXL core.
Add a function for getting the cxl region range to be used for mapping
such memory range by a Type2 driver.

Signed-off-by: Alejandro Lucero <alucerop@amd.com>
Reviewed-by: Zhi Wang <zhiw@nvidia.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Ben Cheatham <benjamin.cheatham@amd.com>
---
 drivers/cxl/core/region.c | 23 +++++++++++++++++++++++
 include/cxl/cxl.h         |  2 ++
 2 files changed, 25 insertions(+)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 3edb5703d6de..9c3ba911d865 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -2655,6 +2655,29 @@ static struct cxl_region *devm_cxl_add_region(struct cxl_root_decoder *cxlrd,
 	return ERR_PTR(rc);
 }
 
+/**
+ * cxl_get_region_range - obtain range linked to a CXL region
+ *
+ * @region: a pointer to struct cxl_region
+ * @range: a pointer to a struct range to be set
+ *
+ * Returns 0 or error.
+ */
+int cxl_get_region_range(struct cxl_region *region, struct range *range)
+{
+	if (WARN_ON_ONCE(!region))
+		return -ENODEV;
+
+	if (!region->params.res)
+		return -ENOSPC;
+
+	range->start = region->params.res->start;
+	range->end = region->params.res->end;
+
+	return 0;
+}
+EXPORT_SYMBOL_NS_GPL(cxl_get_region_range, "CXL");
+
 static ssize_t __create_region_show(struct cxl_root_decoder *cxlrd, char *buf)
 {
 	return sysfs_emit(buf, "region%u\n", atomic_read(&cxlrd->region_id));
diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h
index 05c6ff2404b0..aa3cf8509b41 100644
--- a/include/cxl/cxl.h
+++ b/include/cxl/cxl.h
@@ -233,4 +233,6 @@ struct cxl_memdev *devm_cxl_add_memdev(struct cxl_dev_state *cxlds,
 				       const struct cxl_memdev_attach *attach);
 struct cxl_region;
 struct cxl_region *cxl_get_region_from_committed_decoder(struct cxl_memdev *cxlmd);
+struct range;
+int cxl_get_region_range(struct cxl_region *region, struct range *range);
 #endif /* __CXL_CXL_H__ */
-- 
2.34.1

Re: [PATCH v25 07/11] cxl: Add function for obtaining region range

[Dan Williams]

alejandro.lucero-palau@ wrote:
> From: Alejandro Lucero <alucerop@amd.com>
> 
> A CXL region struct contains the physical address to work with.
> 
> Type2 drivers can create a CXL region but have not access to the
> related struct as it is defined as private by the kernel CXL core.
> Add a function for getting the cxl region range to be used for mapping
> such memory range by a Type2 driver.
> 
> Signed-off-by: Alejandro Lucero <alucerop@amd.com>
> Reviewed-by: Zhi Wang <zhiw@nvidia.com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> Reviewed-by: Ben Cheatham <benjamin.cheatham@amd.com>
> ---
>  drivers/cxl/core/region.c | 23 +++++++++++++++++++++++
>  include/cxl/cxl.h         |  2 ++
>  2 files changed, 25 insertions(+)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 3edb5703d6de..9c3ba911d865 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -2655,6 +2655,29 @@ static struct cxl_region *devm_cxl_add_region(struct cxl_root_decoder *cxlrd,
>  	return ERR_PTR(rc);
>  }
>  
> +/**
> + * cxl_get_region_range - obtain range linked to a CXL region
> + *
> + * @region: a pointer to struct cxl_region
> + * @range: a pointer to a struct range to be set
> + *
> + * Returns 0 or error.
> + */
> +int cxl_get_region_range(struct cxl_region *region, struct range *range)
> +{
> +	if (WARN_ON_ONCE(!region))
> +		return -ENODEV;
> +
> +	if (!region->params.res)
> +		return -ENOSPC;
> +
> +	range->start = region->params.res->start;
> +	range->end = region->params.res->end;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_get_region_range, "CXL");

Missing locks to keep the range valid until this context can be hooked
up an invalidation path like "unload driver on region destruction", or
other protection against user triggered destruction while the memory is
in use.

[PATCH v25 08/11] cxl: Export function for unwinding cxl by accelerators

[alejandro.lucero-palau]

From: Alejandro Lucero <alucerop@amd.com>

Add cxl_unregister_region() to the accelerator driver API
for a clean exit.

Signed-off-by: Alejandro Lucero <alucerop@amd.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/cxl/core/region.c | 8 ++++++++
 include/cxl/cxl.h         | 1 +
 2 files changed, 9 insertions(+)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 9c3ba911d865..b3a6d08be461 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -2490,6 +2490,14 @@ static void unregister_region(void *_cxlr)
 	put_device(&cxlr->dev);
 }
 
+void cxl_unregister_region(struct cxl_region *cxlr)
+{
+	struct cxl_port *port = to_cxl_port(cxlr->cxlrd->cxlsd.cxld.dev.parent);
+
+	devm_release_action(port->uport_dev, unregister_region, cxlr);
+}
+EXPORT_SYMBOL_NS_GPL(cxl_unregister_region, "CXL");
+
 static struct lock_class_key cxl_region_key;
 
 static struct cxl_region *cxl_region_alloc(struct cxl_root_decoder *cxlrd, int id)
diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h
index aa3cf8509b41..e046c59904a5 100644
--- a/include/cxl/cxl.h
+++ b/include/cxl/cxl.h
@@ -235,4 +235,5 @@ struct cxl_region;
 struct cxl_region *cxl_get_region_from_committed_decoder(struct cxl_memdev *cxlmd);
 struct range;
 int cxl_get_region_range(struct cxl_region *region, struct range *range);
+void cxl_unregister_region(struct cxl_region *cxlr);
 #endif /* __CXL_CXL_H__ */
-- 
2.34.1

Re: [PATCH v25 08/11] cxl: Export function for unwinding cxl by accelerators

[Dan Williams]

alejandro.lucero-palau@ wrote:
> From: Alejandro Lucero <alucerop@amd.com>
> 
> Add cxl_unregister_region() to the accelerator driver API
> for a clean exit.
> 
> Signed-off-by: Alejandro Lucero <alucerop@amd.com>
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/cxl/core/region.c | 8 ++++++++
>  include/cxl/cxl.h         | 1 +
>  2 files changed, 9 insertions(+)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 9c3ba911d865..b3a6d08be461 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -2490,6 +2490,14 @@ static void unregister_region(void *_cxlr)
>  	put_device(&cxlr->dev);
>  }
>  
> +void cxl_unregister_region(struct cxl_region *cxlr)
> +{
> +	struct cxl_port *port = to_cxl_port(cxlr->cxlrd->cxlsd.cxld.dev.parent);
> +
> +	devm_release_action(port->uport_dev, unregister_region, cxlr);
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_unregister_region, "CXL");

devm_release_action() must only run from port->uport_dev attach context,
I.e. a context known to not race ->remove() / devres_release_all().

[PATCH v25 09/11] sfc: obtain decoder and region if committed by firmware

[alejandro.lucero-palau]

From: Alejandro Lucero <alucerop@amd.com>

Check if device HDM is already committed during firmware/BIOS
initialization.

A CXL region should exist if so after memdev allocation/initialization.
Get HPA from region and map it.

Signed-off-by: Alejandro Lucero <alucerop@amd.com>
Acked-by: Edward Cree <ecree.xilinx@gmail.com>
---
 drivers/net/ethernet/sfc/efx_cxl.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c
index 63e6f277ae9f..f3bcae22d1dd 100644
--- a/drivers/net/ethernet/sfc/efx_cxl.c
+++ b/drivers/net/ethernet/sfc/efx_cxl.c
@@ -19,6 +19,7 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
 	struct efx_nic *efx = &probe_data->efx;
 	struct pci_dev *pci_dev = efx->pci_dev;
 	struct efx_cxl *cxl;
+	struct range range;
 	u16 dvsec;
 	int rc;
 
@@ -81,6 +82,24 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
 		return PTR_ERR(cxl->cxlmd);
 	}
 
+	cxl->efx_region = cxl_get_region_from_committed_decoder(cxl->cxlmd);
+	if (!cxl->efx_region)
+		return -ENODEV;
+
+	rc = cxl_get_region_range(cxl->efx_region, &range);
+	if (rc) {
+		pci_err(pci_dev,
+			"CXL getting regions params from a committed decoder failed");
+		return rc;
+	}
+
+	cxl->ctpio_cxl = ioremap(range.start, range_len(&range));
+	if (!cxl->ctpio_cxl) {
+		pci_err(pci_dev, "CXL ioremap region (%pra) failed", &range);
+		return -ENOMEM;
+	}
+
+
 	probe_data->cxl = cxl;
 
 	return 0;
@@ -88,6 +107,17 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
 
 void efx_cxl_exit(struct efx_probe_data *probe_data)
 {
+	if (!probe_data->cxl)
+		return;
+
+	iounmap(probe_data->cxl->ctpio_cxl);
+
+	/* If the sfc cxl initialization was successful, it implies the
+	 * endpoint decoder had an auto discover region which is the one
+	 * we used and we need to remove now. Otherwise the region will
+	 * be around until the root port is removed.
+	 */
+	cxl_unregister_region(probe_data->cxl->efx_region);
 }
 
 MODULE_IMPORT_NS("CXL");
-- 
2.34.1

Re: [PATCH v25 09/11] sfc: obtain decoder and region if committed by firmware

[kernel test robot]

Hi,

kernel test robot noticed the following build errors:

[auto build test ERROR on 64584273dfb8a1e5fc7d78094ba22a93c204b44e]

url:    https://github.com/intel-lab-lkp/linux/commits/alejandro-lucero-palau-amd-com/sfc-add-cxl-support/20260331-055820
base:   64584273dfb8a1e5fc7d78094ba22a93c204b44e
patch link:    https://lore.kernel.org/r/20260330143827.1278677-10-alejandro.lucero-palau%40amd.com
patch subject: [PATCH v25 09/11] sfc: obtain decoder and region if committed by firmware
config: sparc-randconfig-002-20260331 (https://download.01.org/0day-ci/archive/20260401/202604010036.h2OVjfQU-lkp@intel.com/config)
compiler: sparc-linux-gcc (GCC) 15.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260401/202604010036.h2OVjfQU-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202604010036.h2OVjfQU-lkp@intel.com/

All errors (new ones prefixed by >>):

   sparc-linux-ld: drivers/net/ethernet/sfc/efx_cxl.o: in function `efx_cxl_init':
>> drivers/net/ethernet/sfc/efx_cxl.c:89:(.text+0x198): undefined reference to `cxl_get_region_range'
   sparc-linux-ld: drivers/net/ethernet/sfc/efx_cxl.o: in function `efx_cxl_exit':
>> drivers/net/ethernet/sfc/efx_cxl.c:120:(.text+0x234): undefined reference to `cxl_unregister_region'


vim +89 drivers/net/ethernet/sfc/efx_cxl.c

    16	
    17	int efx_cxl_init(struct efx_probe_data *probe_data)
    18	{
    19		struct efx_nic *efx = &probe_data->efx;
    20		struct pci_dev *pci_dev = efx->pci_dev;
    21		struct efx_cxl *cxl;
    22		struct range range;
    23		u16 dvsec;
    24		int rc;
    25	
    26		probe_data->cxl_pio_initialised = false;
    27	
    28		/* Is the device configured with and using CXL? */
    29		if (!pcie_is_cxl(pci_dev))
    30			return 0;
    31	
    32		dvsec = pci_find_dvsec_capability(pci_dev, PCI_VENDOR_ID_CXL,
    33						  PCI_DVSEC_CXL_DEVICE);
    34		if (!dvsec) {
    35			pci_info(pci_dev, "CXL_DVSEC_PCIE_DEVICE capability not found\n");
    36			return 0;
    37		}
    38	
    39		pci_dbg(pci_dev, "CXL_DVSEC_PCIE_DEVICE capability found\n");
    40	
    41		/* Create a cxl_dev_state embedded in the cxl struct using cxl core api
    42		 * specifying no mbox available.
    43		 */
    44		cxl = devm_cxl_dev_state_create(&pci_dev->dev, CXL_DEVTYPE_DEVMEM,
    45						pci_dev->dev.id, dvsec, struct efx_cxl,
    46						cxlds, false);
    47	
    48		if (!cxl)
    49			return -ENOMEM;
    50	
    51		rc = cxl_pci_setup_regs(pci_dev, CXL_REGLOC_RBI_COMPONENT,
    52					&cxl->cxlds.reg_map);
    53		if (rc) {
    54			pci_err(pci_dev, "No component registers\n");
    55			return rc;
    56		}
    57	
    58		if (!cxl->cxlds.reg_map.component_map.hdm_decoder.valid) {
    59			pci_err(pci_dev, "Expected HDM component register not found\n");
    60			return -ENODEV;
    61		}
    62	
    63		if (!cxl->cxlds.reg_map.component_map.ras.valid) {
    64			pci_err(pci_dev, "Expected RAS component register not found\n");
    65			return -ENODEV;
    66		}
    67	
    68		/* Set media ready explicitly as there are neither mailbox for checking
    69		 * this state nor the CXL register involved, both not mandatory for
    70		 * type2.
    71		 */
    72		cxl->cxlds.media_ready = true;
    73	
    74		if (cxl_set_capacity(&cxl->cxlds, EFX_CTPIO_BUFFER_SIZE)) {
    75			pci_err(pci_dev, "dpa capacity setup failed\n");
    76			return -ENODEV;
    77		}
    78	
    79		cxl->cxlmd = devm_cxl_add_memdev(&cxl->cxlds, NULL);
    80		if (IS_ERR(cxl->cxlmd)) {
    81			pci_err(pci_dev, "CXL accel memdev creation failed");
    82			return PTR_ERR(cxl->cxlmd);
    83		}
    84	
    85		cxl->efx_region = cxl_get_region_from_committed_decoder(cxl->cxlmd);
    86		if (!cxl->efx_region)
    87			return -ENODEV;
    88	
  > 89		rc = cxl_get_region_range(cxl->efx_region, &range);
    90		if (rc) {
    91			pci_err(pci_dev,
    92				"CXL getting regions params from a committed decoder failed");
    93			return rc;
    94		}
    95	
    96		cxl->ctpio_cxl = ioremap(range.start, range_len(&range));
    97		if (!cxl->ctpio_cxl) {
    98			pci_err(pci_dev, "CXL ioremap region (%pra) failed", &range);
    99			return -ENOMEM;
   100		}
   101	
   102	
   103		probe_data->cxl = cxl;
   104	
   105		return 0;
   106	}
   107	
   108	void efx_cxl_exit(struct efx_probe_data *probe_data)
   109	{
   110		if (!probe_data->cxl)
   111			return;
   112	
   113		iounmap(probe_data->cxl->ctpio_cxl);
   114	
   115		/* If the sfc cxl initialization was successful, it implies the
   116		 * endpoint decoder had an auto discover region which is the one
   117		 * we used and we need to remove now. Otherwise the region will
   118		 * be around until the root port is removed.
   119		 */
 > 120		cxl_unregister_region(probe_data->cxl->efx_region);
   121	}
   122	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH v25 10/11] cxl: Avoid dax creation for accelerators

[alejandro.lucero-palau]

From: Alejandro Lucero <alucerop@amd.com>

By definition a type2 cxl device will use the host managed memory for
specific functionality, therefore it should not be available to other
uses like DAX.

Signed-off-by: Alejandro Lucero <alucerop@amd.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Davidlohr Bueso <daves@stgolabs.net>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Ben Cheatham <benjamin.cheatham@amd.com>
---
 drivers/cxl/core/region.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index b3a6d08be461..6a89a1817199 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -4264,6 +4264,13 @@ static int cxl_region_probe(struct device *dev)
 	if (rc)
 		return rc;
 
+	/*
+	 * HDM-D[B] (device-memory) regions have accelerator specific usage.
+	 * Skip device-dax registration.
+	 */
+	if (cxlr->type == CXL_DECODER_DEVMEM)
+		return 0;
+
 	/*
 	 * From this point on any path that changes the region's state away from
 	 * CXL_CONFIG_COMMIT is also responsible for releasing the driver.
-- 
2.34.1

Re: [PATCH v25 10/11] cxl: Avoid dax creation for accelerators

[Dan Williams]

alejandro.lucero-palau@ wrote:
> From: Alejandro Lucero <alucerop@amd.com>
> 
> By definition a type2 cxl device will use the host managed memory for
> specific functionality, therefore it should not be available to other
> uses like DAX.
> 
> Signed-off-by: Alejandro Lucero <alucerop@amd.com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Reviewed-by: Davidlohr Bueso <daves@stgolabs.net>
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> Reviewed-by: Ben Cheatham <benjamin.cheatham@amd.com>
> ---
>  drivers/cxl/core/region.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index b3a6d08be461..6a89a1817199 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -4264,6 +4264,13 @@ static int cxl_region_probe(struct device *dev)
>  	if (rc)
>  		return rc;
>  
> +	/*
> +	 * HDM-D[B] (device-memory) regions have accelerator specific usage.
> +	 * Skip device-dax registration.
> +	 */
> +	if (cxlr->type == CXL_DECODER_DEVMEM)
> +		return 0;
> +

It is possible for a general memory expander to also be
CXL_DECODER_DEVMEM. Imagine a memory expander that could back
invalidate on decoder change. See my reply to patch5 for the changes to
cxl_region_probe() when an accelerator driver wants to own mapping a
resulting region.

Also note that cxl_region_has_memdev_attach() arranges for memory
notifiers and poison to be setup. This is for accelerators that may use
devm_memremap_pages() and accelerators that may implement a full CXL
mailbox for whatever reason. A CXL mailbox remains the only generic
mechanism for determining the DPA layout of a device.

[PATCH v25 11/11] sfc: support pio mapping based on cxl

[alejandro.lucero-palau]

From: Alejandro Lucero <alucerop@amd.com>

A PIO buffer is a region of device memory to which the driver can write a
packet for TX, with the device handling the transmit doorbell without
requiring a DMA for getting the packet data, which helps reducing latency
in certain exchanges. With CXL mem protocol this latency can be lowered
further.

With a device supporting CXL and successfully initialised, use the cxl
region to map the memory range and use this mapping for PIO buffers.

Add the disabling of those CXL-based PIO buffers if the callback for
potential cxl endpoint removal by the CXL code happens.

Signed-off-by: Alejandro Lucero <alucerop@amd.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/net/ethernet/sfc/ef10.c       | 53 +++++++++++++++++++++++----
 drivers/net/ethernet/sfc/efx_cxl.c    |  1 +
 drivers/net/ethernet/sfc/net_driver.h |  2 +
 drivers/net/ethernet/sfc/nic.h        |  3 ++
 4 files changed, 52 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/sfc/ef10.c b/drivers/net/ethernet/sfc/ef10.c
index 7e04f115bbaa..ae422bac329e 100644
--- a/drivers/net/ethernet/sfc/ef10.c
+++ b/drivers/net/ethernet/sfc/ef10.c
@@ -24,6 +24,7 @@
 #include <linux/wait.h>
 #include <linux/workqueue.h>
 #include <net/udp_tunnel.h>
+#include "efx_cxl.h"
 
 /* Hardware control for EF10 architecture including 'Huntington'. */
 
@@ -106,7 +107,7 @@ static int efx_ef10_get_vf_index(struct efx_nic *efx)
 
 static int efx_ef10_init_datapath_caps(struct efx_nic *efx)
 {
-	MCDI_DECLARE_BUF(outbuf, MC_CMD_GET_CAPABILITIES_V4_OUT_LEN);
+	MCDI_DECLARE_BUF(outbuf, MC_CMD_GET_CAPABILITIES_V7_OUT_LEN);
 	struct efx_ef10_nic_data *nic_data = efx->nic_data;
 	size_t outlen;
 	int rc;
@@ -177,6 +178,12 @@ static int efx_ef10_init_datapath_caps(struct efx_nic *efx)
 			  efx->num_mac_stats);
 	}
 
+	if (outlen < MC_CMD_GET_CAPABILITIES_V7_OUT_LEN)
+		nic_data->datapath_caps3 = 0;
+	else
+		nic_data->datapath_caps3 = MCDI_DWORD(outbuf,
+						      GET_CAPABILITIES_V7_OUT_FLAGS3);
+
 	return 0;
 }
 
@@ -919,6 +926,9 @@ static void efx_ef10_forget_old_piobufs(struct efx_nic *efx)
 static void efx_ef10_remove(struct efx_nic *efx)
 {
 	struct efx_ef10_nic_data *nic_data = efx->nic_data;
+#ifdef CONFIG_SFC_CXL
+	struct efx_probe_data *probe_data;
+#endif
 	int rc;
 
 #ifdef CONFIG_SFC_SRIOV
@@ -949,7 +959,12 @@ static void efx_ef10_remove(struct efx_nic *efx)
 
 	efx_mcdi_rx_free_indir_table(efx);
 
+#ifdef CONFIG_SFC_CXL
+	probe_data = container_of(efx, struct efx_probe_data, efx);
+	if (nic_data->wc_membase && !probe_data->cxl_pio_in_use)
+#else
 	if (nic_data->wc_membase)
+#endif
 		iounmap(nic_data->wc_membase);
 
 	rc = efx_mcdi_free_vis(efx);
@@ -1140,6 +1155,9 @@ static int efx_ef10_dimension_resources(struct efx_nic *efx)
 	unsigned int channel_vis, pio_write_vi_base, max_vis;
 	struct efx_ef10_nic_data *nic_data = efx->nic_data;
 	unsigned int uc_mem_map_size, wc_mem_map_size;
+#ifdef CONFIG_SFC_CXL
+	struct efx_probe_data *probe_data;
+#endif
 	void __iomem *membase;
 	int rc;
 
@@ -1263,8 +1281,28 @@ static int efx_ef10_dimension_resources(struct efx_nic *efx)
 	iounmap(efx->membase);
 	efx->membase = membase;
 
-	/* Set up the WC mapping if needed */
-	if (wc_mem_map_size) {
+	if (!wc_mem_map_size)
+		goto skip_pio;
+
+	/* Set up the WC mapping */
+
+#ifdef CONFIG_SFC_CXL
+	probe_data = container_of(efx, struct efx_probe_data, efx);
+	if ((nic_data->datapath_caps3 &
+	    (1 << MC_CMD_GET_CAPABILITIES_V7_OUT_CXL_CONFIG_ENABLE_LBN)) &&
+	    probe_data->cxl_pio_initialised) {
+		/* Using PIO through CXL mapping? */
+		nic_data->pio_write_base = probe_data->cxl->ctpio_cxl +
+					   (pio_write_vi_base * efx->vi_stride +
+					    ER_DZ_TX_PIOBUF - uc_mem_map_size);
+
+		nic_data->pio_write_vi_base = pio_write_vi_base;
+
+		probe_data->cxl_pio_in_use = true;
+	} else
+#endif
+	{
+		/* Using legacy PIO BAR mapping */
 		nic_data->wc_membase = ioremap_wc(efx->membase_phys +
 						  uc_mem_map_size,
 						  wc_mem_map_size);
@@ -1279,12 +1317,13 @@ static int efx_ef10_dimension_resources(struct efx_nic *efx)
 			nic_data->wc_membase +
 			(pio_write_vi_base * efx->vi_stride + ER_DZ_TX_PIOBUF -
 			 uc_mem_map_size);
-
-		rc = efx_ef10_link_piobufs(efx);
-		if (rc)
-			efx_ef10_free_piobufs(efx);
 	}
 
+	rc = efx_ef10_link_piobufs(efx);
+	if (rc)
+		efx_ef10_free_piobufs(efx);
+
+skip_pio:
 	netif_dbg(efx, probe, efx->net_dev,
 		  "memory BAR at %pa (virtual %p+%x UC, %p+%x WC)\n",
 		  &efx->membase_phys, efx->membase, uc_mem_map_size,
diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c
index f3bcae22d1dd..c918febfc651 100644
--- a/drivers/net/ethernet/sfc/efx_cxl.c
+++ b/drivers/net/ethernet/sfc/efx_cxl.c
@@ -101,6 +101,7 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
 
 
 	probe_data->cxl = cxl;
+	probe_data->cxl_pio_initialised = true;
 
 	return 0;
 }
diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
index 3964b2c56609..bea4eecdf842 100644
--- a/drivers/net/ethernet/sfc/net_driver.h
+++ b/drivers/net/ethernet/sfc/net_driver.h
@@ -1207,6 +1207,7 @@ struct efx_cxl;
  * @efx: Efx NIC details
  * @cxl: details of related cxl objects
  * @cxl_pio_initialised: cxl initialization outcome.
+ * @cxl_pio_in_use: PIO using CXL mapping
  */
 struct efx_probe_data {
 	struct pci_dev *pci_dev;
@@ -1214,6 +1215,7 @@ struct efx_probe_data {
 #ifdef CONFIG_SFC_CXL
 	struct efx_cxl *cxl;
 	bool cxl_pio_initialised;
+	bool cxl_pio_in_use;
 #endif
 };
 
diff --git a/drivers/net/ethernet/sfc/nic.h b/drivers/net/ethernet/sfc/nic.h
index ec3b2df43b68..7480f9995dfb 100644
--- a/drivers/net/ethernet/sfc/nic.h
+++ b/drivers/net/ethernet/sfc/nic.h
@@ -152,6 +152,8 @@ enum {
  *	%MC_CMD_GET_CAPABILITIES response)
  * @datapath_caps2: Further Capabilities of datapath firmware (FLAGS2 field of
  * %MC_CMD_GET_CAPABILITIES response)
+ * @datapath_caps3: Further Capabilities of datapath firmware (FLAGS3 field of
+ * %MC_CMD_GET_CAPABILITIES response)
  * @rx_dpcpu_fw_id: Firmware ID of the RxDPCPU
  * @tx_dpcpu_fw_id: Firmware ID of the TxDPCPU
  * @must_probe_vswitching: Flag: vswitching has yet to be setup after MC reboot
@@ -187,6 +189,7 @@ struct efx_ef10_nic_data {
 	bool must_check_datapath_caps;
 	u32 datapath_caps;
 	u32 datapath_caps2;
+	u32 datapath_caps3;
 	unsigned int rx_dpcpu_fw_id;
 	unsigned int tx_dpcpu_fw_id;
 	bool must_probe_vswitching;
-- 
2.34.1