[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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] driver core: AB-BA deadlock in device_link_add and device_release_driver_internal
- [High] driver core: fw_devlink broken by rejecting unbound suppliers in device_link_add
- [Low] driver core: mixed goto cleanup and scoped_guard violates cleanup guidelines
- [Low] driver core: dead code in PM runtime check due to device_is_bound
--

commit 1f69a7873c55c75da1b5f2ed9898c41c813921cc
Author: Alejandro Lucero <alucerop@amd.com>

driver core: Rely on supplier driver binding at link creation

This commit modifies device link creation to rely on supplier driver binding.
It evaluates device_is_bound() for the supplier upfront and conditionally
checks PM initialization based on the DL_FLAG_PM_RUNTIME flag.

> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 4c0c373998a19..eb6d87e35d76c 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) {

[Severity: High]
Can this lead to an AB-BA deadlock between device_links_write_lock() and
the supplier device lock?

The device_link_add() function holds device_links_write_lock() before
acquiring the supplier's device lock via scoped_guard(device, supplier).

However, during device unbinding, device_release_driver_internal() takes the
device lock via __device_driver_lock() first. It then calls
__device_release_driver() which invokes device_links_driver_cleanup(),
attempting to acquire device_links_write_lock().

Could this result in a lock inversion?

> +		if (!device_is_bound(supplier)) {

[Severity: High]
Does rejecting unbound suppliers break fw_devlink's dependency tracking?

The fw_devlink_create_devlink() routine actively tries to create dormant
links to supplier devices before they are probed or bound to a driver. This
ensures proper probe ordering.

Rejecting these links unconditionally because device_is_bound() requires a
driver to be attached appears to break deferred probing guarantees.

Should we permit links when the supplier is not bound yet?

> +			link = NULL;
> +			goto out;

[Severity: Low]
Does jumping out of a scope-based cleanup block violate the cleanup
guidelines?

The kernel subsystem cleanup guidelines state that the usage of goto and
cleanup helpers should never be mixed in the same function.

Using a goto to jump out of a scoped_guard() block to a manual cleanup label
mixes the two paradigms.

Could this be converted completely to scope-based cleanup, or would it be
better to avoid using scoped_guard() here?

> +		}
> +	}
> +	/*
> +	 * 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)) ||

[Severity: Low]
Is the !device_pm_initialized(supplier) condition reachable anymore?

The device_is_bound() check earlier in the function fundamentally
guarantees that a driver is attached. This is only possible if the
device has already been registered.

Since device_pm_initialized() directly evaluates to device_is_registered(),
!device_pm_initialized(supplier) will evaluate to false under all possible
code paths passing the new bound check.

Is this check now dead code?

> +	    (!(flags & DL_FLAG_SYNC_STATE_ONLY) &&
> +	     device_is_dependent(consumer, supplier))) {
>  		link = NULL;
>  		goto out;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260921191239.4249-1-alucerop@amd.com?part=1

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

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

[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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] cxl: Use-After-Free of attach->cxlr when endpoint unbinds
--

commit 0680840dffb6451e74369681a082d7e4373586f8
Author: Alejandro Lucero <alucerop@amd.com>

cxl/region: Add region reference in memdev attach

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.

> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 27e63e6dab7c8..78ca7ebc3e557 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;

[Severity: High]
Could this result in a use-after-free if the endpoint is unbound?

Looking at cxl_memdev_attach_region(), attach is dynamically allocated on the
parent PCI device (cxlds->dev), but the lifetime of cxlr is exclusively tied
to endpoint->dev via the devm action endpoint_unregister_region.

If the CXL endpoint port (endpoint->dev) driver is unbound independently of the
PCI device (e.g., via sysfs or error recovery), the devm action runs and drops
the final reference, freeing cxlr.

However, attach remains alive on the still-bound PCI device, leaving
attach->cxlr pointing to freed memory. A subsequent consumer calling
cxl_get_pf0_memdev() could then access attach->cxlr->dev and trigger a
use-after-free.

Should an additional reference to cxlr be taken here, or the pointer cleared
during endpoint teardown?

>  	attach->hpa_range = (struct range) {
>  		.start = cxlr->params.res->start,
>  		.end = cxlr->params.res->end,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260921191239.4249-1-alucerop@amd.com?part=2

[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

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 3/4] cxl/memdev: Add support for multi PF devices

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] cxl: UAF / memory corruption due to omitted no_free_ptr() in cxl_get_pf0_memdev()
- [High] cxl: NULL pointer dereference when cxlmd->attach is NULL
- [High] cxl: data race and NULL pointer dereference due to missing memory barriers
--

commit 8b8a34ac3eece08d08d13bded379e20360255e65
Author: Alejandro Lucero <alucerop@amd.com>

cxl/memdev: Add support for multi PF devices

This commit adds 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,
keeping the CXL memory use safe.

> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index b3419df586b9c..67be02faa7e14 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,
> +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);

[Severity: High]
If cxlmd->attach is NULL (e.g. for a standard class memory device initialized
via devm_cxl_add_memdev), does container_of() evaluate to NULL here since the
member is at offset 0?

> +
> +	/*
> +	 * 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)

[Severity: High]
Could this lead to a NULL pointer dereference when accessing
attach->hpa_range.end if attach evaluated to NULL above?

Additionally, since attach->hpa_range.end is checked locklessly to determine
if initialization is complete, could CPU or compiler reordering cause the
subsequent read of attach->cxlr to see an uninitialized or NULL value?

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

[Severity: High]
If attach->cxlr is read as NULL due to missing memory barriers or acquire
semantics, could this dereference lead to a kernel panic?

> +		return ERR_PTR(-ENODEV);
> +
> +	range->start = attach->hpa_range.start;
> +	range->end = attach->hpa_range.end;
> +
> +	return to_cxl_memdev(mem_dev);

[Severity: High]
Will the device reference acquired by bus_find_device() be unconditionally
dropped here at scope exit due to the __free(put_device) cleanup attribute?

Should this return to_cxl_memdev(no_free_ptr(mem_dev)) to inhibit the
cleanup and safely transfer ownership of the acquired reference to the caller?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260921191239.4249-1-alucerop@amd.com?part=3

[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 4/4] sfc: add multipf support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] sfc: inverted error check for cxl_map() causes UAF on success
- [Medium] pci: reference leak of pf0_pci_dev in efx_cxl_init()
- [Medium] sfc: memory leak of non-PF0 cxl struct in efx_cxl_exit()
--

commit d0979ed10895deac33baab224f2c0e771350412b
Author: Alejandro Lucero <alucerop@amd.com>

    sfc: add multipf support
    
    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.

> diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c
> index 348d7404cd7aa..bed8d9c591853 100644
> --- a/drivers/net/ethernet/sfc/efx_cxl.c
> +++ b/drivers/net/ethernet/sfc/efx_cxl.c
> @@ -21,8 +46,48 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
[ ... ]
> +	/* 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));

[Severity: Medium]
Does this code leak the reference to the parent PCI device structure?

pci_get_slot() increments the reference count of the returned PCI device.
However, pci_dev_put() is never called to release this reference in any of the
subsequent return paths within the non-PF0 branch of efx_cxl_init().

> +		/* 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;
[ ... ]
> +		cxl = kzalloc_obj(*cxl, GFP_KERNEL);
> +		if (!cxl)
> +			return -ENOMEM;

[Severity: Medium]
Does the non-PF0 cxl struct memory get leaked during device removal or probe
failure?

The cxl object is allocated manually here using kzalloc_obj(), whereas for PF0
devices it uses devm_cxl_dev_state_create() which is automatically freed via
device resource management.

Since the unified teardown function efx_cxl_exit() only unmaps the I/O memory
and doesn't call kfree(probe_data->cxl), wouldn't this manual allocation be
leaked?

> +
> +		cxl->cxlmd = cxlmd;
> +
> +		if (!cxl_map(probe_data, cxl, (u64)devfn, cxl_pio_range)) {
> +			kfree(cxl);
> +			return -ENOMEM;
> +		}

[Severity: High]
Is the error check for cxl_map() inverted here?

cxl_map() returns 0 on success and sets probe_data->cxl:

static int cxl_map(struct efx_probe_data *probe_data, struct efx_cxl *cxl, ...)
{
        ...
        probe_data->cxl = cxl;
        ...
        return 0;
}

Because of the inverted check, a success (0) evaluates to true, triggering
this error path which calls kfree(cxl) and returns -ENOMEM.

When the device probe eventually fails, the teardown path efx_cxl_exit() will
see that probe_data->cxl is non-NULL and attempt to dereference it to unmap
I/O memory:

void efx_cxl_exit(struct efx_probe_data *probe_data)
{
        if (!probe_data->cxl)
                return;

        iounmap(probe_data->cxl->ctpio_cxl);
}

Could this lead to a Use-After-Free crash on every successful mapping of a
non-PF0 device?

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

[Severity: High]
Does this similarly abort initialization on success for the PF0 path due to the
inverted error check on cxl_map()?

> -	}
> -
> -	probe_data->cxl_pio_initialised = true;
> -	probe_data->cxl = cxl;
>  
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260921191239.4249-1-alucerop@amd.com?part=4

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

[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.