Re: [PATCH v29 4/5] sfc: obtain and map cxl range using devm_cxl_probe_mem

["Dan Williams (nvidia)" <djbw@kernel.org>]

alejandro.lucero-palau@ wrote:
> From: Alejandro Lucero <alucerop@amd.com>
> 
> Use core API for safely obtain the CXL range linked to an HDM committed
> by the BIOS. Map such a range for being used as the ctpio buffer.
> 
> A potential user space action through sysfs unbinding or core cxl
> modules remove will trigger sfc driver device detachment, with that case
> not racing with this mapping as this is done during driver probe and
> therefore protected with device lock against those user space actions.
> 
> Signed-off-by: Alejandro Lucero <alucerop@amd.com>
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> Acked-by: Edward Cree <ecree.xilinx@gmail.com>
> ---
>  drivers/net/ethernet/sfc/efx.c     |  2 ++
>  drivers/net/ethernet/sfc/efx_cxl.c | 23 +++++++++++++++++++++++
>  drivers/net/ethernet/sfc/efx_cxl.h |  3 +++
>  3 files changed, 28 insertions(+)
> 
> diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
> index 61cbb6cfc360..3806cd3dd7f4 100644
> --- a/drivers/net/ethernet/sfc/efx.c
> +++ b/drivers/net/ethernet/sfc/efx.c
> @@ -984,6 +984,7 @@ static void efx_pci_remove(struct pci_dev *pci_dev)
>  	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");
>  
> @@ -1242,6 +1243,7 @@ static int efx_pci_probe(struct pci_dev *pci_dev,
>  	return 0;
>  
>   fail3:
> +	efx_cxl_exit(probe_data);
>  	efx_fini_io(efx);
>   fail2:
>  	efx_fini_struct(efx);
> diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c
> index 18b535b3ea40..3e7c950f83e9 100644
> --- a/drivers/net/ethernet/sfc/efx_cxl.c
> +++ b/drivers/net/ethernet/sfc/efx_cxl.c
> @@ -18,6 +18,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 range cxl_pio_range;
>  	struct efx_cxl *cxl;
>  	u16 dvsec;
>  	int rc;
> @@ -73,9 +74,31 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
>  		return -ENODEV;
>  	}
>  
> +	cxl->cxlmd = devm_cxl_probe_mem(&cxl->cxlds, &cxl_pio_range);
> +	if (IS_ERR(cxl->cxlmd)) {
> +		pci_err(pci_dev, "CXL accel memdev creation failed\n");
> +		return PTR_ERR(cxl->cxlmd);
> +	}
> +
> +	cxl->ctpio_cxl = ioremap_wc(cxl_pio_range.start,
> +				    range_len(&cxl_pio_range));
> +	if (!cxl->ctpio_cxl) {
> +		pci_err(pci_dev, "CXL ioremap region (%pra) failed\n",
> +			&cxl_pio_range);
> +		return -ENOMEM;
> +	}
> +
>  	probe_data->cxl = cxl;
>  
>  	return 0;
>  }
>  
> +void efx_cxl_exit(struct efx_probe_data *probe_data)
> +{

If you are going to have an explicit efx_cxl_exit() then I would also
add an explicit unregistration of the memdev. This would also fix the
Sashiko report about pci_disable_device() running while the cxl_memdev
is still registered. Unfortunately, mixing devm and explicit unwind is
always fraught.

Let me know if this passes your testing, and I can send it out as a
standalone patch. You could also use it to unwind if the ioremap()
fails.

-- 8< --
From 47940f7d6de6dd1039d0e445eb944ca96ad5eb3a Mon Sep 17 00:00:00 2001
From: Dan Williams <djbw@kernel.org>
Date: Wed, 24 Jun 2026 11:47:24 -0700
Subject: [PATCH] cxl: Add devm_cxl_remove_mem()

Undo devm_cxl_probe_mem() without causing the host driver to unload.

The expectation of devm_cxl_probe_mem() is that upon attach the caller
depends on the CXL topology not being torn down. If it is then it is
escalated to a 'remove' event on the caller. However, if the caller wants
to cancel their interest in CXL operation, they should be able to do so
without that side effect.

This is useful for setup scenarios where CXL successfully attaches, but
some other event precludes CXL operation.

Cc: Alejandro Lucero <alucerop@amd.com>
Signed-off-by: Dan Williams <djbw@kernel.org>
---
 include/cxl/cxl.h         |  1 +
 drivers/cxl/core/memdev.c | 25 +++++++++++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h
index 016c74fb747c..cff2922b0d4a 100644
--- a/include/cxl/cxl.h
+++ b/include/cxl/cxl.h
@@ -226,4 +226,5 @@ struct cxl_dev_state *_devm_cxl_dev_state_create(struct device *dev,
 
 struct cxl_memdev *devm_cxl_probe_mem(struct cxl_dev_state *cxlds,
 				      struct range *range);
+void devm_cxl_remove_mem(struct cxl_memdev *cxlmd);
 #endif /* __CXL_CXL_H__ */
diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
index 33a3d2e7b13a..aa35876f0067 100644
--- a/drivers/cxl/core/memdev.c
+++ b/drivers/cxl/core/memdev.c
@@ -1185,6 +1185,31 @@ struct cxl_memdev *__devm_cxl_add_memdev(struct cxl_dev_state *cxlds,
 }
 EXPORT_SYMBOL_FOR_MODULES(__devm_cxl_add_memdev, "cxl_mem");
 
+static struct cxl_attach_region *cancel_probe_mem_teardown(struct cxl_memdev *cxlmd)
+{
+	struct cxl_attach_region *attach;
+
+	guard(device)(&cxlmd->dev);
+	attach = container_of(cxlmd->attach, typeof(*attach), attach);
+	cxlmd->attach = NULL;
+	return attach;
+}
+
+void devm_cxl_remove_mem(struct cxl_memdev *cxlmd)
+{
+	struct cxl_attach_region *attach;
+	struct cxl_dev_state *cxlds;
+
+	if (IS_ERR(cxlmd))
+		return;
+
+	attach = cancel_probe_mem_teardown(cxlmd);
+	cxlds = cxlmd->cxlds;
+	devm_release_action(cxlds->dev, cxl_memdev_unregister, cxlmd);
+	devm_kfree(cxlds->dev, attach);
+}
+EXPORT_SYMBOL_NS_GPL(devm_cxl_remove_mem, "CXL");
+
 static void sanitize_teardown_notifier(void *data)
 {
 	struct cxl_memdev_state *mds = data;
-- 
2.54.0