Re: [PATCH v20 03/22] cxl/mem: Introduce a memdev creation ->probe() operation

Dave Jiang
2025-11-13T23:02:18Z

On 11/10/25 8:36 AM, alejandro.lucero-palau@amd.com wrote:
> From: Dan Williams <dan.j.williams@intel.com>
>
> Allow for a driver to pass a routine to be called in cxl_mem_probe()
> context. This ability mirrors the semantics of faux_device_create() and
> allows for the caller to run CXL-topology-attach dependent logic on behalf
> of the caller.
>
> This capability is needed for CXL accelerator device drivers that need to
> make decisions about enabling CXL dependent functionality in the device, or
> falling back to PCIe-only operation.
>
> The probe callback runs after the port topology is successfully attached
> for the given memdev.
>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

With your sign off tag added,
Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
> drivers/cxl/core/memdev.c | 5 ++++-
> drivers/cxl/core/pci_drv.c | 2 +-
> drivers/cxl/cxlmem.h | 9 ++++++++-
> drivers/cxl/mem.c | 27 +++++++++++++++++++++++++--
> drivers/cxl/private.h | 3 ++-
> tools/testing/cxl/test/mem.c | 2 +-
> 6 files changed, 41 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index 14b4601faf66..45b5714651d0 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -1035,7 +1035,8 @@ static const struct file_operations cxl_memdev_fops = {
> .llseek = noop_llseek,
> };
>
> -struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds)
> +struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds,
> + const struct cxl_memdev_ops *ops)
> {
> struct cxl_memdev *cxlmd __free(kfree) =
> kzalloc(sizeof(*cxlmd), GFP_KERNEL);
> @@ -1052,6 +1053,8 @@ struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds)
> return ERR_PTR(rc);
> cxlmd->id = rc;
> cxlmd->depth = -1;
> + cxlmd->ops = ops;
> + cxlmd->endpoint = ERR_PTR(-ENXIO);
> cxlmd->cxlds = cxlds;
> cxlds->cxlmd = cxlmd;
>
> diff --git a/drivers/cxl/core/pci_drv.c b/drivers/cxl/core/pci_drv.c
> index bc3c959f7eb6..f43590062efd 100644
> --- a/drivers/cxl/core/pci_drv.c
> +++ b/drivers/cxl/core/pci_drv.c
> @@ -1007,7 +1007,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> if (rc)
> dev_dbg(&pdev->dev, "No CXL Features discovered\n");
>
> - cxlmd = devm_cxl_add_memdev(&pdev->dev, cxlds);
> + cxlmd = devm_cxl_add_memdev(&pdev->dev, cxlds, NULL);
> if (IS_ERR(cxlmd))
> return PTR_ERR(cxlmd);
>
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index 434031a0c1f7..e55f52a5598d 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -34,6 +34,10 @@
> (FIELD_GET(CXLMDEV_RESET_NEEDED_MASK, status) != \
> CXLMDEV_RESET_NEEDED_NOT)
>
> +struct cxl_memdev_ops {
> + int (*probe)(struct cxl_memdev *cxlmd);
> +};
> +
> /**
> * struct cxl_memdev - CXL bus object representing a Type-3 Memory Device
> * @dev: driver core device object
> @@ -43,6 +47,7 @@
> * @cxl_nvb: coordinate removal of @cxl_nvd if present
> * @cxl_nvd: optional bridge to an nvdimm if the device supports pmem
> * @endpoint: connection to the CXL port topology for this memory device
> + * @ops: incremental caller specific probe routine
> * @id: id number of this memdev instance.
> * @depth: endpoint port depth
> * @scrub_cycle: current scrub cycle set for this device
> @@ -59,6 +64,7 @@ struct cxl_memdev {
> struct cxl_nvdimm_bridge *cxl_nvb;
> struct cxl_nvdimm *cxl_nvd;
> struct cxl_port *endpoint;
> + const struct cxl_memdev_ops *ops;
> int id;
> int depth;
> u8 scrub_cycle;
> @@ -96,7 +102,8 @@ static inline bool is_cxl_endpoint(struct cxl_port *port)
> }
>
> struct cxl_memdev *devm_cxl_add_memdev(struct device *host,
> - struct cxl_dev_state *cxlds);
> + struct cxl_dev_state *cxlds,
> + const struct cxl_memdev_ops *ops);
> 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 01a8e808196e..ebe17fb6bb82 100644
> --- a/drivers/cxl/mem.c
> +++ b/drivers/cxl/mem.c
> @@ -144,6 +144,12 @@ static int cxl_mem_probe(struct device *dev)
> return rc;
> }
>
> + if (cxlmd->ops) {
> + rc = cxlmd->ops->probe(cxlmd);
> + if (rc)
> + return rc;
> + }
> +
> rc = devm_cxl_memdev_edac_register(cxlmd);
> if (rc)
> dev_dbg(dev, "CXL memdev EDAC registration failed rc=%d\n", rc);
> @@ -178,15 +184,17 @@ DEFINE_FREE(cxlmd_free, struct cxl_memdev *, __cxlmd_free(_T))
> * devm_cxl_add_memdev - Add a CXL memory device
> * @host: devres alloc/release context and parent for the memdev
> * @cxlds: CXL device state to associate with the memdev
> + * @ops: optional operations to run in cxl_mem::{probe,remove}() context
> *
> * Upon return the device will have had a chance to attach to the
> * cxl_mem driver, but may fail if the CXL topology is not ready
> * (hardware CXL link down, or software platform CXL root not attached)
> */
> struct cxl_memdev *devm_cxl_add_memdev(struct device *host,
> - struct cxl_dev_state *cxlds)
> + struct cxl_dev_state *cxlds,
> + const struct cxl_memdev_ops *ops)
> {
> - struct cxl_memdev *cxlmd __free(cxlmd_free) = cxl_memdev_alloc(cxlds);
> + struct cxl_memdev *cxlmd __free(cxlmd_free) = cxl_memdev_alloc(cxlds, ops);
> int rc;
>
> if (IS_ERR(cxlmd))
> @@ -200,6 +208,21 @@ struct cxl_memdev *devm_cxl_add_memdev(struct device *host,
> if (rc)
> return ERR_PTR(rc);
>
> + /*
> + * If ops is provided fail if the driver is not attached upon
> + * return. The ->endpoint ERR_PTR may have a more precise error
> + * code to convey. Note that failure here could be the result of
> + * a race to teardown the CXL port topology. I.e.
> + * cxl_mem_probe() could have succeeded and then cxl_mem unbound
> + * before the lock is acquired.
> + */
> + guard(device)(&cxlmd->dev);
> + if (ops && !cxlmd->dev.driver) {
> + if (IS_ERR(cxlmd->endpoint))
> + return ERR_CAST(cxlmd->endpoint);
> + return ERR_PTR(-ENXIO);
> + }
> +
> return no_free_ptr(cxlmd);
> }
> EXPORT_SYMBOL_NS_GPL(devm_cxl_add_memdev, "CXL");
> diff --git a/drivers/cxl/private.h b/drivers/cxl/private.h
> index f8d1ff64f534..7c04797a3a28 100644
> --- a/drivers/cxl/private.h
> +++ b/drivers/cxl/private.h
> @@ -8,7 +8,8 @@
>
> #ifndef __CXL_PRIVATE_H__
> #define __CXL_PRIVATE_H__
> -struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds);
> +struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds,
> + const struct cxl_memdev_ops *ops);
> int devm_cxl_memdev_add_or_reset(struct device *host, struct cxl_memdev *cxlmd);
> int devm_cxl_add_endpoint(struct device *host, struct cxl_memdev *cxlmd,
> struct cxl_dport *parent_dport);
> diff --git a/tools/testing/cxl/test/mem.c b/tools/testing/cxl/test/mem.c
> index d533481672b7..33d06ec5a4b9 100644
> --- a/tools/testing/cxl/test/mem.c
> +++ b/tools/testing/cxl/test/mem.c
> @@ -1768,7 +1768,7 @@ static int cxl_mock_mem_probe(struct platform_device *pdev)
>
> cxl_mock_add_event_logs(&mdata->mes);
>
> - cxlmd = devm_cxl_add_memdev(&pdev->dev, cxlds);
> + cxlmd = devm_cxl_add_memdev(&pdev->dev, cxlds, NULL);
> if (IS_ERR(cxlmd))
> return PTR_ERR(cxlmd);
>