Re: [PATCH v1 3/4] cxl/memdev: Add support for multi PF devices

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

On 9/21/26 12:12 PM, alucerop@amd.com wrote:
> From: Alejandro Lucero <alucerop@amd.com>
> 
> A PCI device can present multiple Physical Functions(PFs) but the CXL
> specs restrict to the first one, PF0, the discovery and management of
> CXL capabilities accessed through a PF0 BAR. Other non-PF0 PFs need to
> obtain the CXL.mem range to work with somehow.
> 
> Add a device link between the cxl region a PF0 memdev is attached to and
> the non-PF0 wanting to use the CXL region. A CXL region release will
> trigger such a PF to be released from its driver first.
> 
> PF0 being unbound from its driver triggers memdev and region release
> leading to non-PF0s being unbound first keeping the CXL memory use safe.
> 
> Signed-off-by: Alejandro Lucero <alucerop@amd.com>
> ---
>  drivers/cxl/core/memdev.c | 66 +++++++++++++++++++++++++++++++++++++++
>  include/cxl/cxl.h         |  1 +
>  2 files changed, 67 insertions(+)
> 
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index b3419df586b9..67be02faa7e1 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -802,6 +802,72 @@ static struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds,
>  	return ERR_PTR(rc);
>  }
>  
> +static int match_memdev_by_parent_device(struct device *dev, const void *data)
> +{
> +	const struct device *pf_dev = data;
> +	struct cxl_memdev *cxlmd;
> +
> +	if (!is_cxl_memdev(dev))
> +		return 0;
> +
> +	cxlmd = to_cxl_memdev(dev);
> +	return (cxlmd->cxlds->dev == pf_dev);
> +}
> +
> +/**
> + * cxl_get_pf0_memdev - register a device link with the region PF0 memdev is
> + * attached to. The region release will imply the link consumer to be unbound
> + * from its driver first.
> + *
> + * @pf0: device to use for finding target memdev.
> + * @pfx: device to link to PF0's memdev region, the link consumer.
> + * @range: to be set with the PF0's memdev range.
> + *
> + * Return: PF0 memdev pointer or error.
> + */
> +struct cxl_memdev *cxl_get_pf0_memdev(struct device *pf0, struct device *pfx,

cxl_link_to_pf0_region() may be a better name? cxl_get_pf0_memdev() hides the intention of linking.

> +				      struct range *range)
> +{
> +	struct cxl_attach_region *attach;
> +	struct cxl_memdev *cxlmd;
> +	struct device *mem_dev __free(put_device) =
> +		bus_find_device(&cxl_bus_type, NULL, pf0,
> +				match_memdev_by_parent_device);
> +
> +	if (!mem_dev)
> +		return ERR_PTR(-ENODEV);
> +
> +	cxlmd = to_cxl_memdev(mem_dev);
> +
> +	/*
> +	 * we got the cxl_memdev and the implicit get_device in bus_find_device
> +	 * makes the next steps safe.
> +	 */
> +	attach = container_of(cxlmd->attach, struct cxl_attach_region, attach);
> +
> +	/*
> +	 * The cxlmd object does exist and it can be found in the cxl bus after
> +	 * creation but before attach probe setting the proper HPA range. If so,
> +	 * the caller will need to try later.
> +	 */
> +	if (attach->hpa_range.end == -1)

CXL_RESOURCE_NONE instead of -1?

> +		return ERR_PTR(-EPROBE_DEFER);
> +
> +	/*
> +	 * Create the device link between the region and the consumer device.
> +	 * AUTOREMOVE_CONSUMER means the link implicitly to be removed if the
> +	 * consumer unbinds first with no consequences for the supplier.
> +	 */
> +	if (!device_link_add(pfx, &attach->cxlr->dev, DL_FLAG_AUTOREMOVE_CONSUMER))
> +		return ERR_PTR(-ENODEV);
> +
> +	range->start = attach->hpa_range.start;
> +	range->end = attach->hpa_range.end;
> +
> +	return to_cxl_memdev(mem_dev);

Should we bother returning cxl_memdev? Does the SFC driver consumer it at all?

DJ

> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_get_pf0_memdev, "CXL");
> +
>  static long __cxl_memdev_ioctl(struct cxl_memdev *cxlmd, unsigned int cmd,
>  			       unsigned long arg)
>  {
> diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h
> index 802b143de83d..e3b1e5be95f8 100644
> --- a/include/cxl/cxl.h
> +++ b/include/cxl/cxl.h
> @@ -228,4 +228,5 @@ struct cxl_memdev *devm_cxl_probe_mem(struct cxl_dev_state *cxlds,
>  				      struct range *range);
>  
>  int cxl_set_capacity(struct cxl_dev_state *cxlds, u64 capacity);
> +struct cxl_memdev *cxl_get_pf0_memdev(struct device *pf0, struct device *pfx, struct range *range);
>  #endif /* __CXL_CXL_H__ */