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

[Dave Jiang <dave.jiang@intel.com>]

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);
>