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

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

On 12/3/25 7:21 PM, Dan Williams wrote:
> Allow for a driver to pass a routine to be called in cxl_mem_probe()
> context. This ability is inspired by and mirrors the semantics of
> faux_device_create(). It 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.
> 
> Cc: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
> Cc: Alejandro Lucero <alucerop@amd.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

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

> ---
>  drivers/cxl/cxlmem.h         | 12 ++++++++--
>  drivers/cxl/core/memdev.c    | 46 +++++++++++++++++++++++++++++-------
>  drivers/cxl/mem.c            | 12 ++++++++--
>  drivers/cxl/pci.c            |  2 +-
>  tools/testing/cxl/test/mem.c |  2 +-
>  5 files changed, 60 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index 9db31c7993c4..ed759f88b21f 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;
> @@ -95,8 +101,10 @@ static inline bool is_cxl_endpoint(struct cxl_port *port)
>  	return is_cxl_memdev(port->uport_dev);
>  }
>  
> -struct cxl_memdev *__devm_cxl_add_memdev(struct cxl_dev_state *cxlds);
> -struct cxl_memdev *devm_cxl_add_memdev(struct cxl_dev_state *cxlds);
> +struct cxl_memdev *__devm_cxl_add_memdev(struct cxl_dev_state *cxlds,
> +					 const struct cxl_memdev_ops *ops);
> +struct cxl_memdev *devm_cxl_add_memdev(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/core/memdev.c b/drivers/cxl/core/memdev.c
> index 7d85ba25835a..41f1964d8cbf 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -648,7 +648,8 @@ static void detach_memdev(struct work_struct *work)
>  static struct lock_class_key cxl_memdev_key;
>  
>  static struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds,
> -					   const struct file_operations *fops)
> +					   const struct file_operations *fops,
> +					   const struct cxl_memdev_ops *ops)
>  {
>  	struct cxl_memdev *cxlmd;
>  	struct device *dev;
> @@ -664,6 +665,8 @@ static struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds,
>  		goto err;
>  	cxlmd->id = rc;
>  	cxlmd->depth = -1;
> +	cxlmd->ops = ops;
> +	cxlmd->endpoint = ERR_PTR(-ENXIO);
>  
>  	dev = &cxlmd->dev;
>  	device_initialize(dev);
> @@ -1077,17 +1080,48 @@ static int cxlmd_add(struct cxl_memdev *cxlmd, struct cxl_dev_state *cxlds)
>  DEFINE_FREE(put_cxlmd, struct cxl_memdev *,
>  	    if (!IS_ERR_OR_NULL(_T)) put_device(&_T->dev);)
>  
> +static struct cxl_memdev *cxl_memdev_autoremove(struct cxl_memdev *cxlmd)
> +{
> +	struct cxl_memdev *ret = cxlmd;
> +	int 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 (cxlmd->ops && !cxlmd->dev.driver) {
> +		ret = ERR_PTR(-ENXIO);
> +		if (IS_ERR(cxlmd->endpoint))
> +			ret = ERR_CAST(cxlmd->endpoint);
> +		cxl_memdev_unregister(cxlmd);
> +		return ret;
> +	}
> +
> +	rc = devm_add_action_or_reset(cxlmd->cxlds->dev, cxl_memdev_unregister,
> +				      cxlmd);
> +	if (rc)
> +		return ERR_PTR(rc);
> +
> +	return ret;
> +}
> +
>  /*
>   * Core helper for devm_cxl_add_memdev() that wants to both create a device and
>   * assert to the caller that upon return cxl_mem::probe() has been invoked.
>   */
> -struct cxl_memdev *__devm_cxl_add_memdev(struct cxl_dev_state *cxlds)
> +struct cxl_memdev *__devm_cxl_add_memdev(struct cxl_dev_state *cxlds,
> +					 const struct cxl_memdev_ops *ops)
>  {
>  	struct device *dev;
>  	int rc;
>  
>  	struct cxl_memdev *cxlmd __free(put_cxlmd) =
> -		cxl_memdev_alloc(cxlds, &cxl_memdev_fops);
> +		cxl_memdev_alloc(cxlds, &cxl_memdev_fops, ops);
>  	if (IS_ERR(cxlmd))
>  		return cxlmd;
>  
> @@ -1100,11 +1134,7 @@ struct cxl_memdev *__devm_cxl_add_memdev(struct cxl_dev_state *cxlds)
>  	if (rc)
>  		return ERR_PTR(rc);
>  
> -	rc = devm_add_action_or_reset(cxlds->dev, cxl_memdev_unregister,
> -				      no_free_ptr(cxlmd));
> -	if (rc)
> -		return ERR_PTR(rc);
> -	return cxlmd;
> +	return cxl_memdev_autoremove(no_free_ptr(cxlmd));
>  }
>  EXPORT_SYMBOL_FOR_MODULES(__devm_cxl_add_memdev, "cxl_mem");
>  
> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
> index 677996c65272..d61f121c6f97 100644
> --- a/drivers/cxl/mem.c
> +++ b/drivers/cxl/mem.c
> @@ -142,6 +142,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);
> @@ -166,6 +172,7 @@ static int cxl_mem_probe(struct device *dev)
>  /**
>   * devm_cxl_add_memdev - Add a CXL memory device
>   * @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
> @@ -174,9 +181,10 @@ static int cxl_mem_probe(struct device *dev)
>   * The parent of the resulting device and the devm context for allocations is
>   * @cxlds->dev.
>   */
> -struct cxl_memdev *devm_cxl_add_memdev(struct cxl_dev_state *cxlds)
> +struct cxl_memdev *devm_cxl_add_memdev(struct cxl_dev_state *cxlds,
> +				       const struct cxl_memdev_ops *ops)
>  {
> -	return __devm_cxl_add_memdev(cxlds);
> +	return __devm_cxl_add_memdev(cxlds, ops);
>  }
>  EXPORT_SYMBOL_NS_GPL(devm_cxl_add_memdev, "CXL");
>  
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index 1c6fc5334806..549368a9c868 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -1006,7 +1006,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(cxlds);
> +	cxlmd = devm_cxl_add_memdev(cxlds, NULL);
>  	if (IS_ERR(cxlmd))
>  		return PTR_ERR(cxlmd);
>  
> diff --git a/tools/testing/cxl/test/mem.c b/tools/testing/cxl/test/mem.c
> index 8a22b7601627..cb87e8c0e63c 100644
> --- a/tools/testing/cxl/test/mem.c
> +++ b/tools/testing/cxl/test/mem.c
> @@ -1767,7 +1767,7 @@ static int cxl_mock_mem_probe(struct platform_device *pdev)
>  
>  	cxl_mock_add_event_logs(&mdata->mes);
>  
> -	cxlmd = devm_cxl_add_memdev(cxlds);
> +	cxlmd = devm_cxl_add_memdev(cxlds, NULL);
>  	if (IS_ERR(cxlmd))
>  		return PTR_ERR(cxlmd);
>