[PATCH v2 0/2] Fix port enumeration failure

[PATCH v2 0/2] Fix port enumeration failure

[Li Ming]

I ran CXL mock testing with next branch, I usually hit the following
call trace.

 Oops: general protection fault, probably for non-canonical address 0xdffffc0000000092: 0000 [#1] SMP KASAN NOPTI
 KASAN: null-ptr-deref in range [0x0000000000000490-0x0000000000000497]
 CPU: 3 UID: 0 PID: 42 Comm: kworker/u16:1 Tainted: G           O      J 6.19.0-rc5-cxl+ #4 PREEMPT(voluntary)
 Tainted: [O]=OOT_MODULE, [J]=FWCTL
 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.17.0-0-gb52ca86e094d-prebuilt.qemu.org 04/01/2014
 Workqueue: async async_run_entry_fn
 RIP: 0010:cxl_dpa_to_region+0x105/0x1f0 [cxl_core]
 Call Trace:
  <TASK>
  cxl_event_trace_record+0xd1/0xa70 [cxl_core]
  __cxl_event_trace_record+0x12f/0x1e0 [cxl_core]
  cxl_mem_get_records_log+0x261/0x500 [cxl_core]
  cxl_mem_get_event_records+0x7c/0xc0 [cxl_core]
  cxl_mock_mem_probe+0xd38/0x1c60 [cxl_mock_mem]
  platform_probe+0x9d/0x130
  really_probe+0x1c8/0x960
  driver_probe_device+0x45/0x120
  __device_attach_driver+0x15d/0x280
  bus_for_each_drv+0x100/0x180
  __device_attach_async_helper+0x199/0x250
  async_run_entry_fn+0x95/0x430
  process_one_work+0x7db/0x1940

After detailed debugging, I identified adding dport failure leads to the
problem.
What I observed is when two memdev were trying to enumerate a same port,
the first memdev was responsible for port creation and bind it to the
cxl port driver. However, there is a small window between the point
where the new port becomes visible(after being added to the device list
of cxl bus) and when it is bound to the port driver. During this window,
the second memdev may discover the port and acquire its lock while
attempting to add its dport, which blocks bus_probe_device() inside
device_add(). As a result, the second memdev observes the port as
unbound and fails to add its dport. The second memdev->endpoint would
not be updated because of that, then trigger above trace.

The solution is to fix this race by holding the host lock of the target
port during dport addition, preventing premature access before driver
binding completed.

base-commit: 63fbf275fa9f18f7020fb8acf54fa107e51d0f23 cxl/next

Changes from V1:
- Remove the patch of initializing memdev->endpoint to NULL. (Dan)
- Fixes typo errors. (Jonathan)
- Introduce a helper called to_port_host().
- unregister_port() cleanup.

Li Ming (2):
  cxl/port: Hold port host lock while dport adding.
  cxl/port: unregister_port() cleanup

 drivers/cxl/core/port.c | 47 +++++++++++++++++++++++++----------------
 1 file changed, 29 insertions(+), 18 deletions(-)

-- 
2.43.0

[PATCH v2 1/2] cxl/port: Hold port host lock while dport adding.

[Li Ming]

CXL testing environment can trigger following trace
 Oops: general protection fault, probably for non-canonical address 0xdffffc0000000092: 0000 [#1] SMP KASAN NOPTI
 KASAN: null-ptr-deref in range [0x0000000000000490-0x0000000000000497]
 RIP: 0010:cxl_dpa_to_region+0x105/0x1f0 [cxl_core]
 Call Trace:
  <TASK>
  cxl_event_trace_record+0xd1/0xa70 [cxl_core]
  __cxl_event_trace_record+0x12f/0x1e0 [cxl_core]
  cxl_mem_get_records_log+0x261/0x500 [cxl_core]
  cxl_mem_get_event_records+0x7c/0xc0 [cxl_core]
  cxl_mock_mem_probe+0xd38/0x1c60 [cxl_mock_mem]
  platform_probe+0x9d/0x130
  really_probe+0x1c8/0x960
  __driver_probe_device+0x187/0x3e0
  driver_probe_device+0x45/0x120
  __device_attach_driver+0x15d/0x280

When CXL subsystem adds a cxl port to a hierarchy, there is a small
window where the new port becomes visible before it is bound to a
driver. This happens because device_add() adds a device to bus device
list before bus_probe_device() binds it to a driver.
So if two cxl memdevs are trying to add a dport to a same port via
devm_cxl_enumerate_ports(), the second cxl memdev may observe the port
and attempt to add a dport, but fails because the port has not yet been
attached to cxl port driver. That causes the memdev->endpoint can not be
updated.

The sequence is like:

CPU 0					CPU 1
devm_cxl_enumerate_ports()
  # port not found, add it
  add_port_attach_ep()
    # hold the parent port lock
    # to add the new port
    devm_cxl_create_port()
      device_add()
	# Add dev to bus devs list
	bus_add_device()
					devm_cxl_enumerate_ports()
					  # found the port
					  find_cxl_port_by_uport()
					  # hold port lock to add a dport
					  device_lock(the port)
					  find_or_add_dport()
					    cxl_port_add_dport()
					      return -ENXIO because port->dev.driver is NULL
					  device_unlock(the port)
	bus_probe_device()
	  # hold the port lock
	  # for attaching
	  device_lock(the port)
	    attaching the new port
	  device_unlock(the port)

To fix this race, require that dport addition holds the host lock
of the target port(the host of CXL root and all cxl host bridge ports is
the platform firmware device, the host of all other ports is their
parent port). The CXL subsystem already requires holding the host lock
while attaching a new port. Therefore, successfully acquiring the host
lock guarantees that port attaching has completed.

Fixes: 4f06d81e7c6a ("cxl: Defer dport allocation for switch ports")
Signed-off-by: Li Ming <ming.li@zohomail.com>
---
 drivers/cxl/core/port.c | 40 ++++++++++++++++++++++++++++++++--------
 1 file changed, 32 insertions(+), 8 deletions(-)

diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 5c82e6f32572..6f0d9fe439db 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -612,6 +612,23 @@ struct cxl_port *parent_port_of(struct cxl_port *port)
 	return port->parent_dport->port;
 }
 
+static struct device *to_port_host(struct cxl_port *port)
+{
+	struct cxl_port *parent = parent_port_of(port);
+
+	/*
+	 * The host of CXL root port and the first level of ports is
+	 * the platform firmware device, the host of all other ports
+	 * is their parent port.
+	 */
+	if (!parent)
+		return port->uport_dev;
+	else if (is_cxl_root(parent))
+		return parent->uport_dev;
+	else
+		return &parent->dev;
+}
+
 static void unregister_port(void *_port)
 {
 	struct cxl_port *port = _port;
@@ -1790,7 +1807,16 @@ static struct cxl_dport *find_or_add_dport(struct cxl_port *port,
 {
 	struct cxl_dport *dport;
 
-	device_lock_assert(&port->dev);
+	/*
+	 * The port is already visible in CXL hierarchy, but it may still
+	 * be in the process of binding to the CXL port driver at this point.
+	 *
+	 * port creation and driver binding are protected by the port's host
+	 * lock, so acquire the host lock here to ensure the port has completed
+	 * driver binding before proceeding with dport addition.
+	 */
+	guard(device)(to_port_host(port));
+	guard(device)(&port->dev);
 	dport = cxl_find_dport_by_dev(port, dport_dev);
 	if (!dport) {
 		dport = probe_dport(port, dport_dev);
@@ -1857,13 +1883,11 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
 			 * RP port enumerated by cxl_acpi without dport will
 			 * have the dport added here.
 			 */
-			scoped_guard(device, &port->dev) {
-				dport = find_or_add_dport(port, dport_dev);
-				if (IS_ERR(dport)) {
-					if (PTR_ERR(dport) == -EAGAIN)
-						goto retry;
-					return PTR_ERR(dport);
-				}
+			dport = find_or_add_dport(port, dport_dev);
+			if (IS_ERR(dport)) {
+				if (PTR_ERR(dport) == -EAGAIN)
+					goto retry;
+				return PTR_ERR(dport);
 			}
 
 			rc = cxl_add_ep(dport, &cxlmd->dev);
-- 
2.43.0

[PATCH v2 2/2] cxl/port: unregister_port() cleanup

[Li Ming]

to_port_host() has been introduced, use to_port_host() in
unregister_port() to get the host device of the target port.

Signed-off-by: Li Ming <ming.li@zohomail.com>
---
 drivers/cxl/core/port.c | 16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 6f0d9fe439db..5754cffe6b8e 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -632,22 +632,8 @@ static struct device *to_port_host(struct cxl_port *port)
 static void unregister_port(void *_port)
 {
 	struct cxl_port *port = _port;
-	struct cxl_port *parent = parent_port_of(port);
-	struct device *lock_dev;
-
-	/*
-	 * CXL root port's and the first level of ports are unregistered
-	 * under the platform firmware device lock, all other ports are
-	 * unregistered while holding their parent port lock.
-	 */
-	if (!parent)
-		lock_dev = port->uport_dev;
-	else if (is_cxl_root(parent))
-		lock_dev = parent->uport_dev;
-	else
-		lock_dev = &parent->dev;
 
-	device_lock_assert(lock_dev);
+	device_lock_assert(to_port_host(port));
 	port->dead = true;
 	device_unregister(&port->dev);
 }
-- 
2.43.0

Re: [PATCH v2 1/2] cxl/port: Hold port host lock while dport adding.

[dan.j.williams]

Li Ming wrote:
> CXL testing environment can trigger following trace
>  Oops: general protection fault, probably for non-canonical address 0xdffffc0000000092: 0000 [#1] SMP KASAN NOPTI
>  KASAN: null-ptr-deref in range [0x0000000000000490-0x0000000000000497]
>  RIP: 0010:cxl_dpa_to_region+0x105/0x1f0 [cxl_core]
>  Call Trace:
>   <TASK>
>   cxl_event_trace_record+0xd1/0xa70 [cxl_core]
>   __cxl_event_trace_record+0x12f/0x1e0 [cxl_core]
>   cxl_mem_get_records_log+0x261/0x500 [cxl_core]
>   cxl_mem_get_event_records+0x7c/0xc0 [cxl_core]
>   cxl_mock_mem_probe+0xd38/0x1c60 [cxl_mock_mem]
>   platform_probe+0x9d/0x130
>   really_probe+0x1c8/0x960
>   __driver_probe_device+0x187/0x3e0
>   driver_probe_device+0x45/0x120
>   __device_attach_driver+0x15d/0x280
> 
> When CXL subsystem adds a cxl port to a hierarchy, there is a small
> window where the new port becomes visible before it is bound to a
> driver. This happens because device_add() adds a device to bus device
> list before bus_probe_device() binds it to a driver.
> So if two cxl memdevs are trying to add a dport to a same port via
> devm_cxl_enumerate_ports(), the second cxl memdev may observe the port
> and attempt to add a dport, but fails because the port has not yet been
> attached to cxl port driver. That causes the memdev->endpoint can not be
> updated.
> 
> The sequence is like:
> 
> CPU 0					CPU 1
> devm_cxl_enumerate_ports()
>   # port not found, add it
>   add_port_attach_ep()
>     # hold the parent port lock
>     # to add the new port
>     devm_cxl_create_port()
>       device_add()
> 	# Add dev to bus devs list
> 	bus_add_device()
> 					devm_cxl_enumerate_ports()
> 					  # found the port
> 					  find_cxl_port_by_uport()
> 					  # hold port lock to add a dport
> 					  device_lock(the port)
> 					  find_or_add_dport()
> 					    cxl_port_add_dport()
> 					      return -ENXIO because port->dev.driver is NULL
> 					  device_unlock(the port)
> 	bus_probe_device()
> 	  # hold the port lock
> 	  # for attaching
> 	  device_lock(the port)
> 	    attaching the new port
> 	  device_unlock(the port)
> 
> To fix this race, require that dport addition holds the host lock
> of the target port(the host of CXL root and all cxl host bridge ports is
> the platform firmware device, the host of all other ports is their
> parent port). The CXL subsystem already requires holding the host lock
> while attaching a new port. Therefore, successfully acquiring the host
> lock guarantees that port attaching has completed.
> 
> Fixes: 4f06d81e7c6a ("cxl: Defer dport allocation for switch ports")
> Signed-off-by: Li Ming <ming.li@zohomail.com>
> ---
>  drivers/cxl/core/port.c | 40 ++++++++++++++++++++++++++++++++--------
>  1 file changed, 32 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index 5c82e6f32572..6f0d9fe439db 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -612,6 +612,23 @@ struct cxl_port *parent_port_of(struct cxl_port *port)
>  	return port->parent_dport->port;
>  }
>  
> +static struct device *to_port_host(struct cxl_port *port)
> +{
> +	struct cxl_port *parent = parent_port_of(port);
> +
> +	/*
> +	 * The host of CXL root port and the first level of ports is
> +	 * the platform firmware device, the host of all other ports
> +	 * is their parent port.
> +	 */
> +	if (!parent)
> +		return port->uport_dev;

This helper looks good and this case is theoretically correct, but I
assume that find_or_add_dport() never hits this case, right?

How about move the introduction of this helper to its own lead-in patch
and use it to replace the open coded version of this pattern in
unregister_port(), __devm_cxl_add_dport(), and endpoint_host().

I do notice you caught the one in unregister_port(), but there are more.

With that change you can add:

Reviewed-by: Dan Williams <dan.j.williams@intel.com>

Re: [PATCH v2 1/2] cxl/port: Hold port host lock while dport adding.

[Li Ming]

在 2026/2/8 01:28, dan.j.williams@intel.com 写道:
> Li Ming wrote:
[...]
>>
>> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
>> index 5c82e6f32572..6f0d9fe439db 100644
>> --- a/drivers/cxl/core/port.c
>> +++ b/drivers/cxl/core/port.c
>> @@ -612,6 +612,23 @@ struct cxl_port *parent_port_of(struct cxl_port *port)
>>   	return port->parent_dport->port;
>>   }
>>   
>> +static struct device *to_port_host(struct cxl_port *port)
>> +{
>> +	struct cxl_port *parent = parent_port_of(port);
>> +
>> +	/*
>> +	 * The host of CXL root port and the first level of ports is
>> +	 * the platform firmware device, the host of all other ports
>> +	 * is their parent port.
>> +	 */
>> +	if (!parent)
>> +		return port->uport_dev;
> This helper looks good and this case is theoretically correct, but I
> assume that find_or_add_dport() never hits this case, right?

Right.  This case is only for finding cxl root's host, 
find_or_add_dport() is

only used for cxl host bridge ports and cxl switch ports.

> How about move the introduction of this helper to its own lead-in patch
> and use it to replace the open coded version of this pattern in
> unregister_port(), __devm_cxl_add_dport(), and endpoint_host().

Sure, will do it in next version, thanks for that.


Ming

> I do notice you caught the one in unregister_port(), but there are more.
>
> With that change you can add:
>
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
>

Re: [PATCH v2 1/2] cxl/port: Hold port host lock while dport adding.

[Li Ming]

在 2026/2/8 01:28, dan.j.williams@intel.com 写道:
> Li Ming wrote:
[...]
> +static struct device *to_port_host(struct cxl_port *port)
> +{
> +	struct cxl_port *parent = parent_port_of(port);
> +
> +	/*
> +	 * The host of CXL root port and the first level of ports is
> +	 * the platform firmware device, the host of all other ports
> +	 * is their parent port.
> +	 */
> +	if (!parent)
> +		return port->uport_dev;
> This helper looks good and this case is theoretically correct, but I
> assume that find_or_add_dport() never hits this case, right?
>
> How about move the introduction of this helper to its own lead-in patch
> and use it to replace the open coded version of this pattern in
> unregister_port(), __devm_cxl_add_dport(), and endpoint_host().

BTW, after taking a look into __devm_cxl_add_dport(), this helper is not 
suitable for this case,

the host in __devm_cxl_add_dport() means the new dport's host, it is the 
device of the cxl port which

the cxl port belongs(or the uport_dev of the cxl port if the port is cxl 
root).

So I will update unregister_port() and endpoint_host(), and check if 
there are other

places needed to be cleanup.


Ming

Re: [PATCH v2 0/2] Fix port enumeration failure

[Alison Schofield]

On Sat, Feb 07, 2026 at 09:35:10PM +0800, Li Ming wrote:
> I ran CXL mock testing with next branch, I usually hit the following
> call trace.

Hi Ming,
I started hitting this after you first posted, however it takes me a
long time to hit it. Do you have a method to reproduce that is better
than running on repeat as I do? Thanks!
-- Alison

snip

Re: [PATCH v2 0/2] Fix port enumeration failure

[Li Ming]

在 2026/2/10 11:04, Alison Schofield 写道:
> On Sat, Feb 07, 2026 at 09:35:10PM +0800, Li Ming wrote:
>> I ran CXL mock testing with next branch, I usually hit the following
>> call trace.
> Hi Ming,
> I started hitting this after you first posted, however it takes me a
> long time to hit it. Do you have a method to reproduce that is better
> than running on repeat as I do? Thanks!
> -- Alison
>
> snip

Hi Alison,

My environment is a VM with 2 CPUs, maybe you can try it.

I use a script to run "meson test --suite=cxl" again and again in the VM.

I can typically reproduce it within 30 minutes, I have also encountered 
cases where it fails to reproduce even after one or two hours.


Ming

Re: [PATCH v2 0/2] Fix port enumeration failure

[Dave Jiang]

On 2/10/26 4:56 AM, Li Ming wrote:
> 
> 在 2026/2/10 11:04, Alison Schofield 写道:
>> On Sat, Feb 07, 2026 at 09:35:10PM +0800, Li Ming wrote:
>>> I ran CXL mock testing with next branch, I usually hit the following
>>> call trace.
>> Hi Ming,
>> I started hitting this after you first posted, however it takes me a
>> long time to hit it. Do you have a method to reproduce that is better
>> than running on repeat as I do? Thanks!
>> -- Alison
>>
>> snip
> 
> Hi Alison,
> 
> My environment is a VM with 2 CPUs, maybe you can try it.
> 
> I use a script to run "meson test --suite=cxl" again and again in the VM.

The other day Alison showed me the --repeat=N parameter for meson test. You can use that instead of script unless it does something special. 

> 
> I can typically reproduce it within 30 minutes, I have also encountered cases where it fails to reproduce even after one or two hours.
> 
> 
> Ming
> 

Re: [PATCH v2 0/2] Fix port enumeration failure

[Li Ming]

在 2026/2/10 23:00, Dave Jiang 写道:
>
> On 2/10/26 4:56 AM, Li Ming wrote:
>> 在 2026/2/10 11:04, Alison Schofield 写道:
>>> On Sat, Feb 07, 2026 at 09:35:10PM +0800, Li Ming wrote:
>>>> I ran CXL mock testing with next branch, I usually hit the following
>>>> call trace.
>>> Hi Ming,
>>> I started hitting this after you first posted, however it takes me a
>>> long time to hit it. Do you have a method to reproduce that is better
>>> than running on repeat as I do? Thanks!
>>> -- Alison
>>>
>>> snip
>> Hi Alison,
>>
>> My environment is a VM with 2 CPUs, maybe you can try it.
>>
>> I use a script to run "meson test --suite=cxl" again and again in the VM.
> The other day Alison showed me the --repeat=N parameter for meson test. You can use that instead of script unless it does something special.
>
Good to know that, thanks. I tried it and it can also hit the issue.


Ming