[PATCH v1 0/4] Type2 multipf support

[PATCH v1 0/4] Type2 multipf support

[alucerop]

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.

This patchset adds support for getting the CXL HPA range other non-PF0s
can use based on what PF0 did initialize. This implies CXL for those PFs
can only be used if PF0 is bound and successfully initialised CXL. This
needs to cover the potential race between PFs probing where non-PF0s
will be deferred if the expected CXL is not ready yet. Moreover, those
PFs can not keep using such CXL memory if PF0 is unbound what can
happen in different scenarios.

This first version after the RFC uses device links as proposed by
Richard Cheng which simplifies the support avoiding specific handling of
CXL memdev siblings as the RFC did. Using device links needs a change to
this core kernel functionality for allowing suppliers without power
management initialised covered in patch 1. The second patch adds a new
field to cxl_attach_region struct for facilitating the device link using
the cxl region device where the Type2 memdev is attached to.

As stated with the RFC:

the final Type2 basic support was possible once Dan Williams and
I reached an agreement on how to solve the potential unwinding spenarios
linked to a cxl memdev object. The actions triggering this unwinding are:

 - User space unbinding the cxl mem device from the cxl mem driver.
 - User space removing cxl_acpi module.
 - User space unbinding Type2/accelerator pci device from its driver.
 - User space removing Type2/accelerator driver.

The last two trigger the unwinding from the Type2/accelerator driver
exit path, while the first two start the unwinding which in turn invoke
the Type2 driver release from its pci device.

In any case, the decission was to release the Type2 driver always
instead of a degraded functionality if CXL.mem is only part of the full
functionality. This needs to be extended to other non-PF0 PFs, so all
the scenarios listed above ending up releasing those other PFs as well
from their drivers.

I have tested this patchset  with real hardware advertising two PFs and
under all the scenarios listed, but stressing this requires another
framework, likely under qemu or adding a new cxl test set.

The base is vanilla 7.3-rc4.

Alejandro Lucero (4):
  driver core: Rely on supplier driver binding at link creation
  cxl/region: Add region reference in memdev attach
  cxl/memdev: Add support for multi PF devices
  sfc: add multipf support

 drivers/base/core.c                | 27 +++++++----
 drivers/cxl/core/memdev.c          | 66 ++++++++++++++++++++++++++
 drivers/cxl/core/region.c          |  1 +
 drivers/cxl/cxlmem.h               |  2 +
 drivers/net/ethernet/sfc/efx_cxl.c | 75 ++++++++++++++++++++++++++----
 include/cxl/cxl.h                  |  1 +
 6 files changed, 155 insertions(+), 17 deletions(-)


base-commit: 93f51579e7df248780214094418f205253383cc5
-- 
2.34.1

[PATCH v1 1/4] driver core: Rely on supplier driver binding at link creation

[alucerop]

From: Alejandro Lucero <alucerop@amd.com>

Differentiate between driver binding from PM dependency when a device
link is created. Rely on the device being bound to a driver for validating
the supplier as some device drivers could not support PM.

Check for supplier's PM state only if consumer specifies PM_RUNTIME flag.

Signed-off-by: Alejandro Lucero <alucerop@amd.com>
---
 drivers/base/core.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 4c0c373998a1..eb6d87e35d76 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -834,15 +834,26 @@ struct device_link *device_link_add(struct device *consumer,
 	device_pm_lock();
 
 	/*
-	 * If the supplier has not been fully registered yet or there is a
-	 * reverse (non-SYNC_STATE_ONLY) dependency between the consumer and
-	 * the supplier already in the graph, return NULL. If the link is a
-	 * SYNC_STATE_ONLY link, we don't check for reverse dependencies
-	 * because it only affects sync_state() callbacks.
+	 * If the supplier has not been fully registered yet with a driver
+	 * return NULL.
 	 */
-	if (!device_pm_initialized(supplier)
-	    || (!(flags & DL_FLAG_SYNC_STATE_ONLY) &&
-		  device_is_dependent(consumer, supplier))) {
+	scoped_guard(device, supplier) {
+		if (!device_is_bound(supplier)) {
+			link = NULL;
+			goto out;
+		}
+	}
+	/*
+	 * If consumer asks for PM to use the link and the supplier has not
+	 * PM initialized, or if there is a reverse (non-SYNC_STATE_ONLY)
+	 * dependency between the consumer and the supplier already in the
+	 * graph, return NULL. If the link is a SYNC_STATE_ONLY link, we
+	 * don't check for reverse dependencies because it only affects
+	 * sync_state() callbacks.
+	 */
+	if (((flags & DL_FLAG_PM_RUNTIME) && !device_pm_initialized(supplier)) ||
+	    (!(flags & DL_FLAG_SYNC_STATE_ONLY) &&
+	     device_is_dependent(consumer, supplier))) {
 		link = NULL;
 		goto out;
 	}
-- 
2.34.1

[PATCH v1 2/4] cxl/region: Add region reference in memdev attach

[alucerop]

From: Alejandro Lucero <alucerop@amd.com>

Use a new field in cxl_attach_region struct for easily link it with the
region the memdev is attached to.

This facilitates device links creation where such a region is the supplier
with non-PF0 physical functions wanting to use the CXL region being the
consumers.

Signed-off-by: Alejandro Lucero <alucerop@amd.com>
---
 drivers/cxl/core/region.c | 1 +
 drivers/cxl/cxlmem.h      | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 27e63e6dab7c..78ca7ebc3e55 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -4132,6 +4132,7 @@ int cxl_memdev_attach_region(struct cxl_memdev *cxlmd)
 	if (rc)
 		return rc;
 
+	attach->cxlr = cxlr;
 	attach->hpa_range = (struct range) {
 		.start = cxlr->params.res->start,
 		.end = cxlr->params.res->end,
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index c401e3a1af06..c598561b8e5f 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -104,6 +104,7 @@ struct cxl_memdev_attach {
 /**
  * struct cxl_attach_region - coordinate mapping a region at memdev registration
  * @attach: common core attachment descriptor
+ * @cxlr: cxl region the memdev is attached to.
  * @hpa_range: physical address range of the region
  *
  * For the common simple case of a CXL device with private (non-general purpose
@@ -112,6 +113,7 @@ struct cxl_memdev_attach {
  */
 struct cxl_attach_region {
 	struct cxl_memdev_attach attach;
+	struct cxl_region *cxlr;
 	struct range hpa_range;
 };
 
-- 
2.34.1

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

[alucerop]

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,
+				      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)
+		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);
+}
+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__ */
-- 
2.34.1

[PATCH v1 4/4] sfc: add multipf support

[alucerop]

From: Alejandro Lucero <alucerop@amd.com>

Use CXL core accelerator API for registering non-PF0 PFs to the memdev
linked to the PF0, along with its complementary unregister.

Adapt the ioremap call per PF to be an offset based on the PF function
index and a hardcoded per PF CXL.mem slot size.

Signed-off-by: Alejandro Lucero <alucerop@amd.com>
---
 drivers/net/ethernet/sfc/efx_cxl.c | 75 ++++++++++++++++++++++++++----
 1 file changed, 66 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c
index 348d7404cd7a..bed8d9c59185 100644
--- a/drivers/net/ethernet/sfc/efx_cxl.c
+++ b/drivers/net/ethernet/sfc/efx_cxl.c
@@ -13,6 +13,31 @@
 #include "efx_cxl.h"
 
 #define EFX_CTPIO_BUFFER_SIZE	SZ_256M
+#define EFX_CTPIO_BUFFER_PER_PF_SIZE	SZ_8M
+
+static int cxl_map(struct efx_probe_data *probe_data, struct efx_cxl *cxl,
+		   u64 devfn, struct range cxl_pio_range)
+{
+	struct efx_nic *efx = &probe_data->efx;
+	struct pci_dev *pci_dev = efx->pci_dev;
+	u64 cxl_pio_pf_start;
+
+	cxl_pio_pf_start = cxl_pio_range.start + devfn *
+			   EFX_CTPIO_BUFFER_PER_PF_SIZE;
+
+	cxl->ctpio_cxl = ioremap_wc(cxl_pio_pf_start,
+				    EFX_CTPIO_BUFFER_PER_PF_SIZE);
+	if (!cxl->ctpio_cxl) {
+		pci_err(pci_dev, "CXL ioremap region (%pra) failed\n",
+			&cxl_pio_range);
+		return -ENOMEM;
+	}
+
+	probe_data->cxl = cxl;
+	probe_data->cxl_pio_initialised = true;
+
+	return 0;
+}
 
 int efx_cxl_init(struct efx_probe_data *probe_data)
 {
@@ -21,8 +46,48 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
 	struct range cxl_pio_range;
 	struct efx_cxl *cxl;
 	u16 dvsec;
+	u8 devfn;
 	int rc;
 
+	if (efx->type->is_vf)
+		return 0;
+
+	/* are we PF0? */
+	devfn = PCI_FUNC(pci_dev->devfn);
+	if (devfn != 0) {
+		struct pci_dev *pf0_pci_dev;
+		struct cxl_memdev *cxlmd;
+
+		pf0_pci_dev = pci_get_slot(pci_dev->bus,
+					   PCI_DEVFN(PCI_SLOT(pci_dev->devfn),
+						     0));
+		/* This should not happen! */
+		if (!pf0_pci_dev)
+			return 0;
+
+		/* Is the PF0 device configured with and using CXL? */
+		if (!pcie_is_cxl(pf0_pci_dev))
+			return 0;
+
+		cxlmd = cxl_get_pf0_memdev(&pf0_pci_dev->dev, &pci_dev->dev,
+					   &cxl_pio_range);
+
+		if (IS_ERR(cxlmd))
+			return -EPROBE_DEFER;
+
+		cxl = kzalloc_obj(*cxl, GFP_KERNEL);
+		if (!cxl)
+			return -ENOMEM;
+
+		cxl->cxlmd = cxlmd;
+
+		if (!cxl_map(probe_data, cxl, (u64)devfn, cxl_pio_range)) {
+			kfree(cxl);
+			return -ENOMEM;
+		}
+		return 0;
+	}
+
 	/* Is the device configured with and using CXL? */
 	if (!pcie_is_cxl(pci_dev))
 		return 0;
@@ -80,16 +145,8 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
 		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);
+	if (!cxl_map(probe_data, cxl, 0, cxl_pio_range))
 		return -ENOMEM;
-	}
-
-	probe_data->cxl_pio_initialised = true;
-	probe_data->cxl = cxl;
 
 	return 0;
 }
-- 
2.34.1

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

[Dave Jiang]

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__ */

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

[Lucero Palau, Alejandro]

On 22/09/2026 00:07, Dave Jiang wrote:
>
> 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.


Uhmm. Not sure. It does hide the linking, but the main point is to get 
the CXL HPA range to work with, an in kernel parlance to get versus put 
is what I had in mind. The device link is how safely the PF can use the 
CXL memory. Noting now that the function description forgot to say about 
the HPA range ...


>> +				      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?


OK.


>
>> +		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?


It does not consume the pointer but it uses the memdev indirectly ... 
this supports my previous comment about "getting" the memdev, but you 
are right, the pointer does not need to be given.


Maybe to return the HPA instead, but the call needs to support 
EPROBE_DEFER, so returning an int would work. What do you think?


Thanks,

Alejandro.


> 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__ */

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

[Dave Jiang]

On 9/22/26 7:07 AM, Lucero Palau, Alejandro wrote:
> 
> On 22/09/2026 00:07, Dave Jiang wrote:
>>
>> 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.
> 
> 
> Uhmm. Not sure. It does hide the linking, but the main point is to get the CXL HPA range to work with, an in kernel parlance to get versus put is what I had in mind. The device link is how safely the PF can use the CXL memory. Noting now that the function description forgot to say about the HPA range ...

Ok just bike shedding here. cxl_link_and_retrieve_pf0_region()?

> 
> 
>>> +                      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?
> 
> 
> OK.
> 
> 
>>
>>> +        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?
> 
> 
> It does not consume the pointer but it uses the memdev indirectly ... this supports my previous comment about "getting" the memdev, but you are right, the pointer does not need to be given.
> 
> 
> Maybe to return the HPA instead, but the call needs to support EPROBE_DEFER, so returning an int would work. What do you think?

Yeah returning an int would work. Standard errno/success return.

DJ

> 
> 
> Thanks,
> 
> Alejandro.
> 
> 
>> 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__ */

Re: [PATCH v1 1/4] driver core: Rely on supplier driver binding at link creation

[Maxime Chevallier]

Hi,

On 9/21/26 21:12, alucerop@amd.com wrote:
> From: Alejandro Lucero <alucerop@amd.com>
> 
> Differentiate between driver binding from PM dependency when a device
> link is created. Rely on the device being bound to a driver for validating
> the supplier as some device drivers could not support PM.
> 
> Check for supplier's PM state only if consumer specifies PM_RUNTIME flag.

Hmmm this patch seems to have broken pretty much all the boards I'm running that
boot with DT. I'm getting logs such as :

platform 16100000.serial: Failed to create device link (0x124) with supplier
  16000400.clock-controller

Some are just hanging at "Starting kernel..." from u-boot.

Then boards go silent as the uart dies, and I'm using that uart to access the
device's console :(

Found with a WIP stmmac runner for netdev CI.

Unfortunately I don't have much logs to share, as this just prevents the boards
from booting :( With this patch reverted, all boards boot fine

Maxime

> 
> Signed-off-by: Alejandro Lucero <alucerop@amd.com>
> ---
>  drivers/base/core.c | 27 +++++++++++++++++++--------
>  1 file changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 4c0c373998a1..eb6d87e35d76 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -834,15 +834,26 @@ struct device_link *device_link_add(struct device *consumer,
>  	device_pm_lock();
>  
>  	/*
> -	 * If the supplier has not been fully registered yet or there is a
> -	 * reverse (non-SYNC_STATE_ONLY) dependency between the consumer and
> -	 * the supplier already in the graph, return NULL. If the link is a
> -	 * SYNC_STATE_ONLY link, we don't check for reverse dependencies
> -	 * because it only affects sync_state() callbacks.
> +	 * If the supplier has not been fully registered yet with a driver
> +	 * return NULL.
>  	 */
> -	if (!device_pm_initialized(supplier)
> -	    || (!(flags & DL_FLAG_SYNC_STATE_ONLY) &&
> -		  device_is_dependent(consumer, supplier))) {
> +	scoped_guard(device, supplier) {
> +		if (!device_is_bound(supplier)) {
> +			link = NULL;
> +			goto out;
> +		}
> +	}
> +	/*
> +	 * If consumer asks for PM to use the link and the supplier has not
> +	 * PM initialized, or if there is a reverse (non-SYNC_STATE_ONLY)
> +	 * dependency between the consumer and the supplier already in the
> +	 * graph, return NULL. If the link is a SYNC_STATE_ONLY link, we
> +	 * don't check for reverse dependencies because it only affects
> +	 * sync_state() callbacks.
> +	 */
> +	if (((flags & DL_FLAG_PM_RUNTIME) && !device_pm_initialized(supplier)) ||
> +	    (!(flags & DL_FLAG_SYNC_STATE_ONLY) &&
> +	     device_is_dependent(consumer, supplier))) {
>  		link = NULL;
>  		goto out;
>  	}

Re: [PATCH v1 1/4] driver core: Rely on supplier driver binding at link creation

[Lucero Palau, Alejandro]

On 22/09/2026 22:39, Maxime Chevallier wrote:
> Hi,
>
> On 9/21/26 21:12, alucerop@amd.com wrote:
>> From: Alejandro Lucero <alucerop@amd.com>
>>
>> Differentiate between driver binding from PM dependency when a device
>> link is created. Rely on the device being bound to a driver for validating
>> the supplier as some device drivers could not support PM.
>>
>> Check for supplier's PM state only if consumer specifies PM_RUNTIME flag.


Hi Maxime,


> Hmmm this patch seems to have broken pretty much all the boards I'm running that
> boot with DT. I'm getting logs such as :
>
> platform 16100000.serial: Failed to create device link (0x124) with supplier
>    16000400.clock-controller
>
> Some are just hanging at "Starting kernel..." from u-boot.
>
> Then boards go silent as the uart dies, and I'm using that uart to access the
> device's console :(
>
> Found with a WIP stmmac runner for netdev CI.
>
> Unfortunately I don't have much logs to share, as this just prevents the boards
> from booting :( With this patch reverted, all boards boot fine


It is obvious I did not understand well the implications ...


I think the supplier device lock could be the problem behind the hanging 
and how the supplier initialization is checked now based on the driver 
bound behind the console issue ...


I have a couple of embedded boards to play with so I will try to 
reproduce the problem for getting more info. FWIW, all was fine with the 
server I tested this, no device link errors at all.


Thank you for testing it!



>
> Maxime
>
>> Signed-off-by: Alejandro Lucero <alucerop@amd.com>
>> ---
>>   drivers/base/core.c | 27 +++++++++++++++++++--------
>>   1 file changed, 19 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/base/core.c b/drivers/base/core.c
>> index 4c0c373998a1..eb6d87e35d76 100644
>> --- a/drivers/base/core.c
>> +++ b/drivers/base/core.c
>> @@ -834,15 +834,26 @@ struct device_link *device_link_add(struct device *consumer,
>>   	device_pm_lock();
>>   
>>   	/*
>> -	 * If the supplier has not been fully registered yet or there is a
>> -	 * reverse (non-SYNC_STATE_ONLY) dependency between the consumer and
>> -	 * the supplier already in the graph, return NULL. If the link is a
>> -	 * SYNC_STATE_ONLY link, we don't check for reverse dependencies
>> -	 * because it only affects sync_state() callbacks.
>> +	 * If the supplier has not been fully registered yet with a driver
>> +	 * return NULL.
>>   	 */
>> -	if (!device_pm_initialized(supplier)
>> -	    || (!(flags & DL_FLAG_SYNC_STATE_ONLY) &&
>> -		  device_is_dependent(consumer, supplier))) {
>> +	scoped_guard(device, supplier) {
>> +		if (!device_is_bound(supplier)) {
>> +			link = NULL;
>> +			goto out;
>> +		}
>> +	}
>> +	/*
>> +	 * If consumer asks for PM to use the link and the supplier has not
>> +	 * PM initialized, or if there is a reverse (non-SYNC_STATE_ONLY)
>> +	 * dependency between the consumer and the supplier already in the
>> +	 * graph, return NULL. If the link is a SYNC_STATE_ONLY link, we
>> +	 * don't check for reverse dependencies because it only affects
>> +	 * sync_state() callbacks.
>> +	 */
>> +	if (((flags & DL_FLAG_PM_RUNTIME) && !device_pm_initialized(supplier)) ||
>> +	    (!(flags & DL_FLAG_SYNC_STATE_ONLY) &&
>> +	     device_is_dependent(consumer, supplier))) {
>>   		link = NULL;
>>   		goto out;
>>   	}

Re: [PATCH v1 1/4] driver core: Rely on supplier driver binding at link creation

[Lucero Palau, Alejandro]

On 23/09/2026 09:49, Lucero Palau, Alejandro wrote:
>
> On 22/09/2026 22:39, Maxime Chevallier wrote:
>> Hi,
>>
>> On 9/21/26 21:12, alucerop@amd.com wrote:
>>> From: Alejandro Lucero <alucerop@amd.com>
>>>
>>> Differentiate between driver binding from PM dependency when a device
>>> link is created. Rely on the device being bound to a driver for 
>>> validating
>>> the supplier as some device drivers could not support PM.
>>>
>>> Check for supplier's PM state only if consumer specifies PM_RUNTIME 
>>> flag.
>
>
> Hi Maxime,
>
>
>> Hmmm this patch seems to have broken pretty much all the boards I'm 
>> running that
>> boot with DT. I'm getting logs such as :
>>
>> platform 16100000.serial: Failed to create device link (0x124) with 
>> supplier
>>    16000400.clock-controller
>>
>> Some are just hanging at "Starting kernel..." from u-boot.
>>
>> Then boards go silent as the uart dies, and I'm using that uart to 
>> access the
>> device's console :(
>>
>> Found with a WIP stmmac runner for netdev CI.
>>
>> Unfortunately I don't have much logs to share, as this just prevents 
>> the boards
>> from booting :( With this patch reverted, all boards boot fine
>
>
> It is obvious I did not understand well the implications ...
>
>
> I think the supplier device lock could be the problem behind the 
> hanging and how the supplier initialization is checked now based on 
> the driver bound behind the console issue ...
>
>
> I have a couple of embedded boards to play with so I will try to 
> reproduce the problem for getting more info. FWIW, all was fine with 
> the server I tested this, no device link errors at all.
>

I have to amend my words ... the server had not exactly the same code 
:-)  I have to copy things in and out due to security measures and 
sometimes is hard to have all synced.


I think the sashiko reports could be a good start to fixing this.


Apologies for the any inconvenient.

Thanks,

Alejandro.


>
> Thank you for testing it!
>
>
>
>>
>> Maxime
>>
>>> Signed-off-by: Alejandro Lucero <alucerop@amd.com>
>>> ---
>>>   drivers/base/core.c | 27 +++++++++++++++++++--------
>>>   1 file changed, 19 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/base/core.c b/drivers/base/core.c
>>> index 4c0c373998a1..eb6d87e35d76 100644
>>> --- a/drivers/base/core.c
>>> +++ b/drivers/base/core.c
>>> @@ -834,15 +834,26 @@ struct device_link *device_link_add(struct 
>>> device *consumer,
>>>       device_pm_lock();
>>>         /*
>>> -     * If the supplier has not been fully registered yet or there is a
>>> -     * reverse (non-SYNC_STATE_ONLY) dependency between the 
>>> consumer and
>>> -     * the supplier already in the graph, return NULL. If the link 
>>> is a
>>> -     * SYNC_STATE_ONLY link, we don't check for reverse dependencies
>>> -     * because it only affects sync_state() callbacks.
>>> +     * If the supplier has not been fully registered yet with a driver
>>> +     * return NULL.
>>>        */
>>> -    if (!device_pm_initialized(supplier)
>>> -        || (!(flags & DL_FLAG_SYNC_STATE_ONLY) &&
>>> -          device_is_dependent(consumer, supplier))) {
>>> +    scoped_guard(device, supplier) {
>>> +        if (!device_is_bound(supplier)) {
>>> +            link = NULL;
>>> +            goto out;
>>> +        }
>>> +    }
>>> +    /*
>>> +     * If consumer asks for PM to use the link and the supplier has 
>>> not
>>> +     * PM initialized, or if there is a reverse (non-SYNC_STATE_ONLY)
>>> +     * dependency between the consumer and the supplier already in the
>>> +     * graph, return NULL. If the link is a SYNC_STATE_ONLY link, we
>>> +     * don't check for reverse dependencies because it only affects
>>> +     * sync_state() callbacks.
>>> +     */
>>> +    if (((flags & DL_FLAG_PM_RUNTIME) && 
>>> !device_pm_initialized(supplier)) ||
>>> +        (!(flags & DL_FLAG_SYNC_STATE_ONLY) &&
>>> +         device_is_dependent(consumer, supplier))) {
>>>           link = NULL;
>>>           goto out;
>>>       }

[syzbot ci] Re: Type2 multipf support

[syzbot ci]

syzbot ci has tested the following series

[v1] Type2 multipf support
https://lore.kernel.org/all/20260921191239.4249-1-alucerop@amd.com
* [PATCH v1 1/4] driver core: Rely on supplier driver binding at link creation
* [PATCH v1 2/4] cxl/region: Add region reference in memdev attach
* [PATCH v1 3/4] cxl/memdev: Add support for multi PF devices
* [PATCH v1 4/4] sfc: add multipf support

and found the following issue:
WARNING: synchronous SCSI scan failed without making any progress, switching to async

Full report is available here:
https://ci.syzbot.org/series/cce74032-73a7-4071-a29e-65a7ec02c350

***

WARNING: synchronous SCSI scan failed without making any progress, switching to async

tree:      axboe
URL:       https://kernel.googlesource.com/pub/scm/linux/kernel/git/axboe/linux.git
base:      93f51579e7df248780214094418f205253383cc5
arch:      amd64
compiler:  Debian clang version 22.1.8 (++20260613092233+e80beda6e255-1~exp1~20260613092250.77), Debian LLD 22.1.8
config:    https://ci.syzbot.org/builds/50697200-bc23-49c1-b943-c9c91049cf06/config

ata1: Failed to create link to scsi device 0:0:0:0
scsi_alloc_sdev: Allocation failure during SCSI scanning, some SCSI devices might not be configured
ata1: Failed to create link to scsi device 0:0:0:0
scsi_alloc_sdev: Allocation failure during SCSI scanning, some SCSI devices might not be configured
ata1: Failed to create link to scsi device 0:0:0:0
scsi_alloc_sdev: Allocation failure during SCSI scanning, some SCSI devices might not be configured
ata1: Failed to create link to scsi device 0:0:0:0
scsi_alloc_sdev: Allocation failure during SCSI scanning, some SCSI devices might not be configured
ata1: Failed to create link to scsi device 0:0:0:0
scsi_alloc_sdev: Allocation failure during SCSI scanning, some SCSI devices might not be configured
ata1: Failed to create link to scsi device 0:0:0:0
scsi_alloc_sdev: Allocation failure during SCSI scanning, some SCSI devices might not be configured
ata1: WARNING: synchronous SCSI scan failed without making any progress, switching to async


***

If these findings have caused you to resend the series or submit a
separate fix, please add the following tag to your commit message:
  Tested-by: syzbot@syzkaller.appspotmail.com

---
This report is generated by a bot. It may contain errors.
syzbot ci engineers can be reached at syzkaller@googlegroups.com.

To test a fix for this bug, please reply with `#syz test`
(on a separate line) and attach the patch to the email.

Notes:
- The patch will be applied on top of the tested series (as an
  incremental fix).
- To test a new version of the whole series, please send it directly
  to syzbot@lists.linux.dev.
- Arguments like custom git repos and branches are not supported.

Re: [PATCH v1 4/4] sfc: add multipf support

[Jonathan Cameron]

On Mon, 21 Sep 2026 20:12:39 +0100
<alucerop@amd.com> wrote:

> From: Alejandro Lucero <alucerop@amd.com>
> 
> Use CXL core accelerator API for registering non-PF0 PFs to the memdev
> linked to the PF0, along with its complementary unregister.
> 
> Adapt the ioremap call per PF to be an offset based on the PF function
> index and a hardcoded per PF CXL.mem slot size.

I was wondering how you'd know what memory belonged to which one!
Simple solutions work best I suppose :)

> 
> Signed-off-by: Alejandro Lucero <alucerop@amd.com>
> ---
>  drivers/net/ethernet/sfc/efx_cxl.c | 75 ++++++++++++++++++++++++++----
>  1 file changed, 66 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c
> index 348d7404cd7a..bed8d9c59185 100644
> --- a/drivers/net/ethernet/sfc/efx_cxl.c
> +++ b/drivers/net/ethernet/sfc/efx_cxl.c
> @@ -13,6 +13,31 @@
>  #include "efx_cxl.h"
>  
>  #define EFX_CTPIO_BUFFER_SIZE	SZ_256M
> +#define EFX_CTPIO_BUFFER_PER_PF_SIZE	SZ_8M
> +
> +static int cxl_map(struct efx_probe_data *probe_data, struct efx_cxl *cxl,
> +		   u64 devfn, struct range cxl_pio_range)
> +{
> +	struct efx_nic *efx = &probe_data->efx;
> +	struct pci_dev *pci_dev = efx->pci_dev;
> +	u64 cxl_pio_pf_start;
> +
> +	cxl_pio_pf_start = cxl_pio_range.start + devfn *
> +			   EFX_CTPIO_BUFFER_PER_PF_SIZE;
Wrap as per operator precedence as easier to read.

			   cxl_pio_range.start +
			   devfn * EFX_CTPIO_BUFFER_PER_SIZE;

> +
> +	cxl->ctpio_cxl = ioremap_wc(cxl_pio_pf_start,
> +				    EFX_CTPIO_BUFFER_PER_PF_SIZE);
> +	if (!cxl->ctpio_cxl) {
> +		pci_err(pci_dev, "CXL ioremap region (%pra) failed\n",
> +			&cxl_pio_range);
> +		return -ENOMEM;
> +	}
> +
> +	probe_data->cxl = cxl;
> +	probe_data->cxl_pio_initialised = true;

'map' is carry quite a lot here that isn't really about mapping anything.
Maybe think a bit more on the naming?

> +
> +	return 0;
> +}
>  
>  int efx_cxl_init(struct efx_probe_data *probe_data)
>  {
> @@ -21,8 +46,48 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
>  	struct range cxl_pio_range;
>  	struct efx_cxl *cxl;
>  	u16 dvsec;
> +	u8 devfn;
>  	int rc;
>  
> +	if (efx->type->is_vf)
> +		return 0;
> +
> +	/* are we PF0? */

First things we ask seems to be Are we not PF0? 

> +	devfn = PCI_FUNC(pci_dev->devfn);
> +	if (devfn != 0) {

I'd factor this lot out as a helper to slightly improve readability.
Perhaps factor out both paths and then have an if else.


> +		struct pci_dev *pf0_pci_dev;
> +		struct cxl_memdev *cxlmd;
> +
> +		pf0_pci_dev = pci_get_slot(pci_dev->bus,
> +					   PCI_DEVFN(PCI_SLOT(pci_dev->devfn),
> +						     0));
> +		/* This should not happen! */
> +		if (!pf0_pci_dev)
> +			return 0;
> +
> +		/* Is the PF0 device configured with and using CXL? */
> +		if (!pcie_is_cxl(pf0_pci_dev))
> +			return 0;
> +
> +		cxlmd = cxl_get_pf0_memdev(&pf0_pci_dev->dev, &pci_dev->dev,
> +					   &cxl_pio_range);
> +
> +		if (IS_ERR(cxlmd))
> +			return -EPROBE_DEFER;
> +
> +		cxl = kzalloc_obj(*cxl, GFP_KERNEL);
> +		if (!cxl)
> +			return -ENOMEM;
> +
> +		cxl->cxlmd = cxlmd;
> +
> +		if (!cxl_map(probe_data, cxl, (u64)devfn, cxl_pio_range)) {
> +			kfree(cxl);
> +			return -ENOMEM;

ENOMEM for a map failure? Seems a little odd but if there is precedence
fair enough.

> +		}
> +		return 0;
> +	}
> +
>  	/* Is the device configured with and using CXL? */
>  	if (!pcie_is_cxl(pci_dev))
>  		return 0;
> @@ -80,16 +145,8 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
>  		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);
> +	if (!cxl_map(probe_data, cxl, 0, cxl_pio_range))
>  		return -ENOMEM;
> -	}
> -
> -	probe_data->cxl_pio_initialised = true;
> -	probe_data->cxl = cxl;
>  
>  	return 0;
>  }

Re: [PATCH v1 1/4] driver core: Rely on supplier driver binding at link creation

[Lucero Palau, Alejandro]

On 23/09/2026 10:58, Lucero Palau, Alejandro wrote:
>
> On 23/09/2026 09:49, Lucero Palau, Alejandro wrote:
>>
>> On 22/09/2026 22:39, Maxime Chevallier wrote:
>>> Hi,
>>>
>>> On 9/21/26 21:12, alucerop@amd.com wrote:
>>>> From: Alejandro Lucero <alucerop@amd.com>
>>>>
>>>> Differentiate between driver binding from PM dependency when a device
>>>> link is created. Rely on the device being bound to a driver for 
>>>> validating
>>>> the supplier as some device drivers could not support PM.
>>>>
>>>> Check for supplier's PM state only if consumer specifies PM_RUNTIME 
>>>> flag.
>>
>>
>> Hi Maxime,
>>
>>
>>> Hmmm this patch seems to have broken pretty much all the boards I'm 
>>> running that
>>> boot with DT. I'm getting logs such as :
>>>
>>> platform 16100000.serial: Failed to create device link (0x124) with 
>>> supplier
>>>    16000400.clock-controller
>>>
>>> Some are just hanging at "Starting kernel..." from u-boot.
>>>
>>> Then boards go silent as the uart dies, and I'm using that uart to 
>>> access the
>>> device's console :(
>>>
>>> Found with a WIP stmmac runner for netdev CI.
>>>
>>> Unfortunately I don't have much logs to share, as this just prevents 
>>> the boards
>>> from booting :( With this patch reverted, all boards boot fine
>>
>>
>> It is obvious I did not understand well the implications ...
>>
>>
>> I think the supplier device lock could be the problem behind the 
>> hanging and how the supplier initialization is checked now based on 
>> the driver bound behind the console issue ...
>>
>>
>> I have a couple of embedded boards to play with so I will try to 
>> reproduce the problem for getting more info. FWIW, all was fine with 
>> the server I tested this, no device link errors at all.
>>
>
> I have to amend my words ... the server had not exactly the same code 
> :-)  I have to copy things in and out due to security measures and 
> sometimes is hard to have all synced.
>
>
> I think the sashiko reports could be a good start to fixing this.
>

FWIW, the problem seems to be the scoped guard. I had a plain 
device_lock for using device_is_bound as required then device_unlock 
after it, but moved to scoped_guard blindly ... .


I think for my impending multipf support I only need something like this:


--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -761,7 +761,7 @@ struct device_link *device_link_add(struct device 
*consumer,
          * SYNC_STATE_ONLY link, we don't check for reverse dependencies
          * because it only affects sync_state() callbacks.
          */
-       if (!device_pm_initialized(supplier)
+       if ((!device_pm_not_required(supplier) && 
!device_pm_initialized(supplier))
             || (!(flags & DL_FLAG_SYNC_STATE_ONLY) &&
                   device_is_dependent(consumer, supplier))) {
                 link = NULL;

but when I studied the code I thought checking for "the supplier has not 
been fully registered yet" was not being achieved if the registration 
meant driver binding, what honestly confuses me because checking for 
device_pm_initialized implies looking at the dev->power.in_dpm_list what 
seems to only happen at device_add time ... so I wonder how the 
device_link_add could use a supplier device without device_add completed.


Anyway, I will go with the simpler change posted above in v2, and keep 
studying the other potential problem not directly connected to my 
multipf support.


>
> Apologies for the any inconvenient.
>
> Thanks,
>
> Alejandro.
>
>
>>
>> Thank you for testing it!
>>
>>
>>
>>>
>>> Maxime
>>>
>>>> Signed-off-by: Alejandro Lucero <alucerop@amd.com>
>>>> ---
>>>>   drivers/base/core.c | 27 +++++++++++++++++++--------
>>>>   1 file changed, 19 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/drivers/base/core.c b/drivers/base/core.c
>>>> index 4c0c373998a1..eb6d87e35d76 100644
>>>> --- a/drivers/base/core.c
>>>> +++ b/drivers/base/core.c
>>>> @@ -834,15 +834,26 @@ struct device_link *device_link_add(struct 
>>>> device *consumer,
>>>>       device_pm_lock();
>>>>         /*
>>>> -     * If the supplier has not been fully registered yet or there 
>>>> is a
>>>> -     * reverse (non-SYNC_STATE_ONLY) dependency between the 
>>>> consumer and
>>>> -     * the supplier already in the graph, return NULL. If the link 
>>>> is a
>>>> -     * SYNC_STATE_ONLY link, we don't check for reverse dependencies
>>>> -     * because it only affects sync_state() callbacks.
>>>> +     * If the supplier has not been fully registered yet with a 
>>>> driver
>>>> +     * return NULL.
>>>>        */
>>>> -    if (!device_pm_initialized(supplier)
>>>> -        || (!(flags & DL_FLAG_SYNC_STATE_ONLY) &&
>>>> -          device_is_dependent(consumer, supplier))) {
>>>> +    scoped_guard(device, supplier) {
>>>> +        if (!device_is_bound(supplier)) {
>>>> +            link = NULL;
>>>> +            goto out;
>>>> +        }
>>>> +    }
>>>> +    /*
>>>> +     * If consumer asks for PM to use the link and the supplier 
>>>> has not
>>>> +     * PM initialized, or if there is a reverse (non-SYNC_STATE_ONLY)
>>>> +     * dependency between the consumer and the supplier already in 
>>>> the
>>>> +     * graph, return NULL. If the link is a SYNC_STATE_ONLY link, we
>>>> +     * don't check for reverse dependencies because it only affects
>>>> +     * sync_state() callbacks.
>>>> +     */
>>>> +    if (((flags & DL_FLAG_PM_RUNTIME) && 
>>>> !device_pm_initialized(supplier)) ||
>>>> +        (!(flags & DL_FLAG_SYNC_STATE_ONLY) &&
>>>> +         device_is_dependent(consumer, supplier))) {
>>>>           link = NULL;
>>>>           goto out;
>>>>       }

Re: [PATCH v1 4/4] sfc: add multipf support

[Lucero Palau, Alejandro]

On 24/09/2026 02:15, Jonathan Cameron wrote:
> On Mon, 21 Sep 2026 20:12:39 +0100
> <alucerop@amd.com> wrote:
>
>> From: Alejandro Lucero <alucerop@amd.com>
>>
>> Use CXL core accelerator API for registering non-PF0 PFs to the memdev
>> linked to the PF0, along with its complementary unregister.
>>
>> Adapt the ioremap call per PF to be an offset based on the PF function
>> index and a hardcoded per PF CXL.mem slot size.
> I was wondering how you'd know what memory belonged to which one!
> Simple solutions work best I suppose :)


Hi Jonathan,


Yes, I think nowadays it is simple. I'm afraid if CXL usage increases 
this will require some request to the firmware ... which could depend on 
previous setting requests to that same firmware through fwctl.


>
>> Signed-off-by: Alejandro Lucero <alucerop@amd.com>
>> ---
>>   drivers/net/ethernet/sfc/efx_cxl.c | 75 ++++++++++++++++++++++++++----
>>   1 file changed, 66 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c
>> index 348d7404cd7a..bed8d9c59185 100644
>> --- a/drivers/net/ethernet/sfc/efx_cxl.c
>> +++ b/drivers/net/ethernet/sfc/efx_cxl.c
>> @@ -13,6 +13,31 @@
>>   #include "efx_cxl.h"
>>   
>>   #define EFX_CTPIO_BUFFER_SIZE	SZ_256M
>> +#define EFX_CTPIO_BUFFER_PER_PF_SIZE	SZ_8M
>> +
>> +static int cxl_map(struct efx_probe_data *probe_data, struct efx_cxl *cxl,
>> +		   u64 devfn, struct range cxl_pio_range)
>> +{
>> +	struct efx_nic *efx = &probe_data->efx;
>> +	struct pci_dev *pci_dev = efx->pci_dev;
>> +	u64 cxl_pio_pf_start;
>> +
>> +	cxl_pio_pf_start = cxl_pio_range.start + devfn *
>> +			   EFX_CTPIO_BUFFER_PER_PF_SIZE;
> Wrap as per operator precedence as easier to read.
>
> 			   cxl_pio_range.start +
> 			   devfn * EFX_CTPIO_BUFFER_PER_SIZE;


OK


>> +
>> +	cxl->ctpio_cxl = ioremap_wc(cxl_pio_pf_start,
>> +				    EFX_CTPIO_BUFFER_PER_PF_SIZE);
>> +	if (!cxl->ctpio_cxl) {
>> +		pci_err(pci_dev, "CXL ioremap region (%pra) failed\n",
>> +			&cxl_pio_range);
>> +		return -ENOMEM;
>> +	}
>> +
>> +	probe_data->cxl = cxl;
>> +	probe_data->cxl_pio_initialised = true;
> 'map' is carry quite a lot here that isn't really about mapping anything.
> Maybe think a bit more on the naming?


Not sure I understand your complain as ioremap is being invoked here. 
Maybe cxl_iomap or sfc_cxl_iomap as this is a static/local function 
would address your concern?


>> +
>> +	return 0;
>> +}
>>   
>>   int efx_cxl_init(struct efx_probe_data *probe_data)
>>   {
>> @@ -21,8 +46,48 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
>>   	struct range cxl_pio_range;
>>   	struct efx_cxl *cxl;
>>   	u16 dvsec;
>> +	u8 devfn;
>>   	int rc;
>>   
>> +	if (efx->type->is_vf)
>> +		return 0;
>> +
>> +	/* are we PF0? */
> First things we ask seems to be Are we not PF0?


Yeah. I will change that.


>> +	devfn = PCI_FUNC(pci_dev->devfn);
>> +	if (devfn != 0) {
> I'd factor this lot out as a helper to slightly improve readability.
> Perhaps factor out both paths and then have an if else.


Yes, I think this makes sense.


>
>> +		struct pci_dev *pf0_pci_dev;
>> +		struct cxl_memdev *cxlmd;
>> +
>> +		pf0_pci_dev = pci_get_slot(pci_dev->bus,
>> +					   PCI_DEVFN(PCI_SLOT(pci_dev->devfn),
>> +						     0));
>> +		/* This should not happen! */
>> +		if (!pf0_pci_dev)
>> +			return 0;
>> +
>> +		/* Is the PF0 device configured with and using CXL? */
>> +		if (!pcie_is_cxl(pf0_pci_dev))
>> +			return 0;
>> +
>> +		cxlmd = cxl_get_pf0_memdev(&pf0_pci_dev->dev, &pci_dev->dev,
>> +					   &cxl_pio_range);
>> +
>> +		if (IS_ERR(cxlmd))
>> +			return -EPROBE_DEFER;
>> +
>> +		cxl = kzalloc_obj(*cxl, GFP_KERNEL);
>> +		if (!cxl)
>> +			return -ENOMEM;
>> +
>> +		cxl->cxlmd = cxlmd;
>> +
>> +		if (!cxl_map(probe_data, cxl, (u64)devfn, cxl_pio_range)) {
>> +			kfree(cxl);
>> +			return -ENOMEM;
> ENOMEM for a map failure? Seems a little odd but if there is precedence
> fair enough.


Confused here. I can see ENOMEM being a common error if ioremap fails 
through the kernel. Maybe this related to your previous concern about 
the function naming, but cxl_map can only fail in one way and that being 
not different to an ioremap failure.


Thanks,

Alejandro


>> +		}
>> +		return 0;
>> +	}
>> +
>>   	/* Is the device configured with and using CXL? */
>>   	if (!pcie_is_cxl(pci_dev))
>>   		return 0;
>> @@ -80,16 +145,8 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
>>   		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);
>> +	if (!cxl_map(probe_data, cxl, 0, cxl_pio_range))
>>   		return -ENOMEM;
>> -	}
>> -
>> -	probe_data->cxl_pio_initialised = true;
>> -	probe_data->cxl = cxl;
>>   
>>   	return 0;
>>   }

Re: [PATCH v1 4/4] sfc: add multipf support

[Jonathan Cameron]

On Fri, 25 Sep 2026 12:16:56 +0100
"Lucero Palau, Alejandro" <alejandro.lucero-palau@amd.com> wrote:

> On 24/09/2026 02:15, Jonathan Cameron wrote:
> > On Mon, 21 Sep 2026 20:12:39 +0100
> > <alucerop@amd.com> wrote:
> >  
> >> From: Alejandro Lucero <alucerop@amd.com>
> >>
> >> Use CXL core accelerator API for registering non-PF0 PFs to the memdev
> >> linked to the PF0, along with its complementary unregister.
> >>
> >> Adapt the ioremap call per PF to be an offset based on the PF function
> >> index and a hardcoded per PF CXL.mem slot size.  
> > I was wondering how you'd know what memory belonged to which one!
> > Simple solutions work best I suppose :)  
> 
> 
> Hi Jonathan,
> 
> 
> Yes, I think nowadays it is simple. I'm afraid if CXL usage increases 
> this will require some request to the firmware ... which could depend on 
> previous setting requests to that same firmware through fwctl.

Ultimately I'd kind of expect either an allocation mechanism where
we tell the device which portion of memory it has (nice if that was
shared architecture rather than a per device thing), or a way
to discover if in practice it is fixed (like here).

> 
> 
> >> +
> >> +	cxl->ctpio_cxl = ioremap_wc(cxl_pio_pf_start,
> >> +				    EFX_CTPIO_BUFFER_PER_PF_SIZE);
> >> +	if (!cxl->ctpio_cxl) {
> >> +		pci_err(pci_dev, "CXL ioremap region (%pra) failed\n",
> >> +			&cxl_pio_range);
> >> +		return -ENOMEM;
> >> +	}
> >> +
> >> +	probe_data->cxl = cxl;
> >> +	probe_data->cxl_pio_initialised = true;  
> > 'map' is carry quite a lot here that isn't really about mapping anything.
> > Maybe think a bit more on the naming?  
> 
> 
> Not sure I understand your complain as ioremap is being invoked here. 
> Maybe cxl_iomap or sfc_cxl_iomap as this is a static/local function 
> would address your concern?

The cxl_pio_initialized doesn't have anything to do with mapping as such.

> 
> 
> >> +
> >> +	return 0;
> >> +}
>
> >> +
> >> +		if (!cxl_map(probe_data, cxl, (u64)devfn, cxl_pio_range)) {
> >> +			kfree(cxl);
> >> +			return -ENOMEM;  
> > ENOMEM for a map failure? Seems a little odd but if there is precedence
> > fair enough.  
> 
> 
> Confused here. I can see ENOMEM being a common error if ioremap fails 
> through the kernel. Maybe this related to your previous concern about 
> the function naming, but cxl_map can only fail in one way and that being 
> not different to an ioremap failure.

Ok. If it's common choice than fine to stick with that.

> 
> 
> Thanks,
> 
> Alejandro