[<prev] [next>] [day] [month] [year] [list]
Message-Id: <20250928101433.424778-1-ming.li@zohomail.com>
Date: Sun, 28 Sep 2025 18:14:33 +0800
From: Li Ming <ming.li@...omail.com>
To: dave@...olabs.net,
jonathan.cameron@...wei.com,
dave.jiang@...el.com,
alison.schofield@...el.com,
vishal.l.verma@...el.com,
ira.weiny@...el.com,
dan.j.williams@...el.com,
shiju.jose@...wei.com
Cc: linux-cxl@...r.kernel.org,
linux-kernel@...r.kernel.org,
Li Ming <ming.li@...omail.com>
Subject: [PATCH v2 1/1] cxl/port: Avoid missing port component registers setup
port->nr_dports is used to represent how many dports added to the cxl
port, it will increase in add_dport() when a new dport is being added to
the cxl port, but it will not be reduced when a dport is removed from
the cxl port.
Currently, when the first dport is added to a cxl port, it will trigger
component registers setup on the cxl port, the implementation is using
port->nr_dports to confirm if the dport is the first dport.
A corner case here is that adding dport could fail after port->nr_dports
updating and before checking port->nr_dports for component registers
setup. If the failure happens during the first dport attaching, it will
cause that CXL subsystem has not chance to execute component registers
setup for the cxl port. the failure flow like below:
port->nr_dports = 0
dport 1 adding to the port:
add_dport() # port->nr_dports: 1
failed on devm_add_action_or_reset() or sysfs_create_link()
return error # port->nr_dports: 1
dport 2 adding to the port:
add_dport() # port->nr_dports: 2
no failure
skip component registers setup because of port->nr_dports is 2
The solution here is that moving component registers setup closer to
add_dport(), so if add_dport() is executed correctly for the first
dport, component registers setup on the port will be executed
immediately after that.
Signed-off-by: Li Ming <ming.li@...omail.com>
---
v2:
- remove dport from port->dports in case of component registers setup
failed.
base-commit: 46037455cbb748c5e85071c95f2244e81986eb58 cxl/next
---
drivers/cxl/core/port.c | 26 ++++++++++++++------------
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index d5f71eb1ade8..8128fd2b5b31 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -1182,6 +1182,20 @@ __devm_cxl_add_dport(struct cxl_port *port, struct device *dport_dev,
if (rc)
return ERR_PTR(rc);
+ /*
+ * Setup port register if this is the first dport showed up. Having
+ * a dport also means that there is at least 1 active link.
+ */
+ if (port->nr_dports == 1 &&
+ port->component_reg_phys != CXL_RESOURCE_NONE) {
+ rc = cxl_port_setup_regs(port, port->component_reg_phys);
+ if (rc) {
+ xa_erase(&port->dports, (unsigned long)dport->dport_dev);
+ return ERR_PTR(rc);
+ }
+ port->component_reg_phys = CXL_RESOURCE_NONE;
+ }
+
get_device(dport_dev);
rc = devm_add_action_or_reset(host, cxl_dport_remove, dport);
if (rc)
@@ -1200,18 +1214,6 @@ __devm_cxl_add_dport(struct cxl_port *port, struct device *dport_dev,
cxl_debugfs_create_dport_dir(dport);
- /*
- * Setup port register if this is the first dport showed up. Having
- * a dport also means that there is at least 1 active link.
- */
- if (port->nr_dports == 1 &&
- port->component_reg_phys != CXL_RESOURCE_NONE) {
- rc = cxl_port_setup_regs(port, port->component_reg_phys);
- if (rc)
- return ERR_PTR(rc);
- port->component_reg_phys = CXL_RESOURCE_NONE;
- }
-
return dport;
}
--
2.34.1
Powered by blists - more mailing lists