* [PATCH v5 01/10] cxl/region: Add decoder check to check_commit_order()
2025-07-03 23:27 [PATCH v5 00/10] cxl: Delay HB port and switch dport probing until endpoint dev probe Dave Jiang
@ 2025-07-03 23:27 ` Dave Jiang
2025-07-03 23:27 ` [PATCH v5 02/10] cxl: Add helper to detect top of CXL device topology Dave Jiang
` (8 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: Dave Jiang @ 2025-07-03 23:27 UTC (permalink / raw)
To: linux-cxl
Cc: dave, jonathan.cameron, alison.schofield, vishal.l.verma,
ira.weiny, dan.j.williams, Gregory Price, Li Ming,
Jonathan Cameron
check_commit_order() attempts to convert a device to a decoder without
making sure the device is a decoder. So far this has been working due
to pure luck. Issue discovered while doing deferred dport probing when
child ports are now in the midst of decoders due to ordering change
of child port additions. Add a check before attempting to do decoder
conversion.
Reviewed-by: Gregory Price <gourry@gourry.net>
Reviewed-by: Li Ming <ming.li@zohomail.com>
Reviewed-by: Alison Schofield <alison.schofield@intel.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
drivers/cxl/core/region.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 6e5e1460068d..b94fda6f2e4c 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -787,7 +787,12 @@ static size_t show_targetN(struct cxl_region *cxlr, char *buf, int pos)
static int check_commit_order(struct device *dev, void *data)
{
- struct cxl_decoder *cxld = to_cxl_decoder(dev);
+ struct cxl_decoder *cxld;
+
+ if (!is_switch_decoder(dev))
+ return 0;
+
+ cxld = to_cxl_decoder(dev);
/*
* if port->commit_end is not the only free decoder, then out of
--
2.50.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v5 02/10] cxl: Add helper to detect top of CXL device topology
2025-07-03 23:27 [PATCH v5 00/10] cxl: Delay HB port and switch dport probing until endpoint dev probe Dave Jiang
2025-07-03 23:27 ` [PATCH v5 01/10] cxl/region: Add decoder check to check_commit_order() Dave Jiang
@ 2025-07-03 23:27 ` Dave Jiang
2025-07-03 23:27 ` [PATCH v5 03/10] cxl: Add helper to reap dport Dave Jiang
` (7 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: Dave Jiang @ 2025-07-03 23:27 UTC (permalink / raw)
To: linux-cxl
Cc: dave, jonathan.cameron, alison.schofield, vishal.l.verma,
ira.weiny, dan.j.williams, Jonathan Cameron, Li Ming
Add a helper to replace the open code detection of CXL device hierarchy
root. The helper will be used for delayed hostbridge port creation later
on.
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Li Ming <ming.li@zohomail.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
drivers/cxl/core/port.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index eb46c6764d20..a49491c14d19 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -39,6 +39,15 @@ DECLARE_RWSEM(cxl_region_rwsem);
static DEFINE_IDA(cxl_port_ida);
static DEFINE_XARRAY(cxl_root_buses);
+/*
+ * The terminal device in PCI is NULL and @platform_bus
+ * for platform devices (for cxl_test)
+ */
+static bool is_cxl_hierarchy_head(struct device *dev)
+{
+ return (!dev || dev == &platform_bus);
+}
+
int cxl_num_decoders_committed(struct cxl_port *port)
{
lockdep_assert_held(&cxl_region_rwsem);
@@ -1635,11 +1644,7 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
struct device *uport_dev;
struct cxl_dport *dport;
- /*
- * The terminal "grandparent" in PCI is NULL and @platform_bus
- * for platform devices
- */
- if (!dport_dev || dport_dev == &platform_bus)
+ if (is_cxl_hierarchy_head(dport_dev))
return 0;
uport_dev = dport_dev->parent;
--
2.50.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v5 03/10] cxl: Add helper to reap dport
2025-07-03 23:27 [PATCH v5 00/10] cxl: Delay HB port and switch dport probing until endpoint dev probe Dave Jiang
2025-07-03 23:27 ` [PATCH v5 01/10] cxl/region: Add decoder check to check_commit_order() Dave Jiang
2025-07-03 23:27 ` [PATCH v5 02/10] cxl: Add helper to detect top of CXL device topology Dave Jiang
@ 2025-07-03 23:27 ` Dave Jiang
2025-07-03 23:27 ` [PATCH v5 04/10] cxl: Defer dport allocation for switch ports Dave Jiang
` (6 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: Dave Jiang @ 2025-07-03 23:27 UTC (permalink / raw)
To: linux-cxl
Cc: dave, jonathan.cameron, alison.schofield, vishal.l.verma,
ira.weiny, dan.j.williams
Refactor the code in reap_dports() out to provide a helper function that
reaps a single dport. This will be used later in the cleanup path for
allocating a dport.
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
drivers/cxl/core/port.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index a49491c14d19..9691da831224 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -1447,6 +1447,13 @@ static void delete_switch_port(struct cxl_port *port)
devm_release_action(port->dev.parent, unregister_port, port);
}
+static void reap_dport(struct cxl_port *port, struct cxl_dport *dport)
+{
+ devm_release_action(&port->dev, cxl_dport_unlink, dport);
+ devm_release_action(&port->dev, cxl_dport_remove, dport);
+ devm_kfree(&port->dev, dport);
+}
+
static void reap_dports(struct cxl_port *port)
{
struct cxl_dport *dport;
@@ -1454,11 +1461,8 @@ static void reap_dports(struct cxl_port *port)
device_lock_assert(&port->dev);
- xa_for_each(&port->dports, index, dport) {
- devm_release_action(&port->dev, cxl_dport_unlink, dport);
- devm_release_action(&port->dev, cxl_dport_remove, dport);
- devm_kfree(&port->dev, dport);
- }
+ xa_for_each(&port->dports, index, dport)
+ reap_dport(port, dport);
}
struct detach_ctx {
--
2.50.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v5 04/10] cxl: Defer dport allocation for switch ports
2025-07-03 23:27 [PATCH v5 00/10] cxl: Delay HB port and switch dport probing until endpoint dev probe Dave Jiang
` (2 preceding siblings ...)
2025-07-03 23:27 ` [PATCH v5 03/10] cxl: Add helper to reap dport Dave Jiang
@ 2025-07-03 23:27 ` Dave Jiang
2025-07-04 9:52 ` Jonathan Cameron
2025-07-08 6:21 ` Li Ming
2025-07-03 23:27 ` [PATCH v5 05/10] cxl/test: Add cxl_test support for cxl_port_update_total_ports() Dave Jiang
` (5 subsequent siblings)
9 siblings, 2 replies; 20+ messages in thread
From: Dave Jiang @ 2025-07-03 23:27 UTC (permalink / raw)
To: linux-cxl
Cc: dave, jonathan.cameron, alison.schofield, vishal.l.verma,
ira.weiny, dan.j.williams
The current implementation enumerates the dports during the cxl_port
driver probe. Without an endpoint connected, the dport may not be
active during port probe. This scheme may prevent a valid hardware
dport id to be retrieved and MMIO registers to be read when an endpoint
is hot-plugged. Move the dport allocation and setup to behind memdev
probe so the endpoint is guaranteed to be connected.
In the original enumeration behavior, there are 3 phases (or 2 if no CXL
switches) for port creation. cxl_acpi() creates a Root Port (RP) from the
ACPI0017.N device. Through that it enumerate downstream ports composed
of ACPI0016.N devices through add_host_bridge_dport(). Once done, it
use add_host_bridge_uport() to create the ports that enumerates the PCI
RPs as the dports of these ports. Every time a port is created, the port
driver is attached and drv->probe() is called and
devm_cxl_port_enumerate_dports() is envoked to enumerate and probe
the dports.
The second phase is if there are any CXL switches. When the pci endpoint
device driver (cxl_pci) calls probe, it will add a mem device and triggers
the cxl_mem->probe(). cxl_mem->probe() calls devm_cxl_enumerate_ports()
and attempts to discovery and create all the ports represent CXL switches.
During this phase, a port is created per switch and the attached dports
are also enumerated and probed.
The last phase is creating endpoint port which happens for all endpoint
devices.
In this commit, the port create and its dport probing in cxl_acpi is not
changed. That will be handled in a different patch later on. The behavior
change is only for CXL switch ports. Only the dport that is part of the
path for an endpoint device to the RP will be probed. This happens
naturally by the code walking up the device hierarchy and identifying the
upstream device and the downstream device.
There are two points where the interception of dport creation happens
during the devm_cxl_enumerate_ports() path. The first location is right
before the function calls add_port_attach_ep() where it does the dport
allocation for the RP. Once the dport is allocated, the iteration path
is reset to the beginning to try again. The second location happens
in add_port_attach_ep() after the location where either the port is
discovered or allocated new if it does not exist.
Locking of port device during __cxl_port_add_dport() protects modifications
against the port and its dports while multiple endpoints can be probing at
the same time and the same port is being modified concurrently.
While the decoders are allocated during the port driver probe,
The decoders must also be updated since previously it's all done when all
the dports are setup and now every time a dport is setup per endpoint, the
switch target listing need to be updated with new dport. A
guard(rwsem_write) is used to update decoder targets. This is similar to
when decoder_populate_target() is called and the decoder programming
must be protected.
Link: https://lore.kernel.org/linux-cxl/20250305100123.3077031-1-rrichter@amd.com/
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v5:
- Change cxl_port_get_total_dports() to cxl_port_update_total_dports(). (Jonathan)
- Return dport for the devm_cxl_port_add_dport() and friends. (Jonathan)
---
drivers/cxl/core/core.h | 2 +
drivers/cxl/core/pci.c | 88 +++++++++++++++++++
drivers/cxl/core/port.c | 184 ++++++++++++++++++++++++++++++++++++++--
drivers/cxl/cxl.h | 5 ++
drivers/cxl/port.c | 8 +-
5 files changed, 273 insertions(+), 14 deletions(-)
diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
index 29b61828a847..8cfead6f3b08 100644
--- a/drivers/cxl/core/core.h
+++ b/drivers/cxl/core/core.h
@@ -122,6 +122,8 @@ void cxl_ras_exit(void);
int cxl_gpf_port_setup(struct cxl_dport *dport);
int cxl_acpi_get_extended_linear_cache_size(struct resource *backing_res,
int nid, resource_size_t *size);
+struct cxl_dport *devm_cxl_add_dport_by_dev(struct cxl_port *port,
+ struct device *dport_dev);
#ifdef CONFIG_CXL_FEATURES
struct cxl_feat_entry *
diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
index b50551601c2e..336451b9144a 100644
--- a/drivers/cxl/core/pci.c
+++ b/drivers/cxl/core/pci.c
@@ -24,6 +24,44 @@ static unsigned short media_ready_timeout = 60;
module_param(media_ready_timeout, ushort, 0644);
MODULE_PARM_DESC(media_ready_timeout, "seconds to wait for media ready");
+/**
+ * devm_cxl_add_dport_by_dev - allocate a dport by dport device
+ * @port: cxl_port that hosts the dport
+ * @dport_dev: 'struct device' of the dport
+ *
+ * Returns the allocate dport on success or ERR_PTR() of -errno on error
+ */
+struct cxl_dport *devm_cxl_add_dport_by_dev(struct cxl_port *port,
+ struct device *dport_dev)
+{
+ struct cxl_register_map map;
+ struct pci_dev *pdev;
+ u32 lnkcap, port_num;
+ int type;
+ int rc;
+
+ if (!dev_is_pci(dport_dev))
+ return ERR_PTR(-EINVAL);
+
+ device_lock_assert(&port->dev);
+
+ pdev = to_pci_dev(dport_dev);
+ type = pci_pcie_type(pdev);
+ if (type != PCI_EXP_TYPE_DOWNSTREAM && type != PCI_EXP_TYPE_ROOT_PORT)
+ return ERR_PTR(-EINVAL);
+
+ if (pci_read_config_dword(pdev, pci_pcie_cap(pdev) + PCI_EXP_LNKCAP,
+ &lnkcap))
+ return ERR_PTR(-ENXIO);
+
+ rc = cxl_find_regblock(pdev, CXL_REGLOC_RBI_COMPONENT, &map);
+ if (rc)
+ dev_dbg(&port->dev, "failed to find component registers\n");
+
+ port_num = FIELD_GET(PCI_EXP_LNKCAP_PN, lnkcap);
+ return devm_cxl_add_dport(port, &pdev->dev, port_num, map.resource);
+}
+
struct cxl_walk_context {
struct pci_bus *bus;
struct cxl_port *port;
@@ -1169,3 +1207,53 @@ int cxl_gpf_port_setup(struct cxl_dport *dport)
return 0;
}
+
+static int match_dport(struct pci_dev *pdev, void *data)
+{
+ struct cxl_walk_context *ctx = data;
+ int type = pci_pcie_type(pdev);
+
+ if (pdev->bus != ctx->bus)
+ return 0;
+ if (!pci_is_pcie(pdev))
+ return 0;
+ if (type != ctx->type)
+ return 0;
+
+ ctx->count++;
+ return 0;
+}
+
+int cxl_port_update_total_dports(struct cxl_port *port)
+{
+ struct pci_bus *bus = cxl_port_to_pci_bus(port);
+ struct cxl_walk_context ctx;
+ int type;
+
+ if (!bus) {
+ dev_err(&port->dev, "No PCI bus found for port %s\n",
+ dev_name(&port->dev));
+ return -ENXIO;
+ }
+
+ if (pci_is_root_bus(bus))
+ type = PCI_EXP_TYPE_ROOT_PORT;
+ else
+ type = PCI_EXP_TYPE_DOWNSTREAM;
+
+ ctx = (struct cxl_walk_context) {
+ .bus = bus,
+ .type = type,
+ };
+ pci_walk_bus(bus, match_dport, &ctx);
+
+ port->total_dports = ctx.count;
+ if (port->total_dports == 0) {
+ dev_warn(&port->dev, "No dports found for port %s on bus %s\n",
+ dev_name(&port->dev), bus->name);
+ return -ENXIO;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_NS_GPL(cxl_port_update_total_dports, "CXL");
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 9691da831224..ffd39c5b7222 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -1551,6 +1551,128 @@ static resource_size_t find_component_registers(struct device *dev)
return map.resource;
}
+static int match_port_by_uport(struct device *dev, const void *data)
+{
+ const struct device *uport_dev = data;
+ struct cxl_port *port;
+
+ if (!is_cxl_port(dev))
+ return 0;
+
+ port = to_cxl_port(dev);
+ return uport_dev == port->uport_dev;
+}
+
+/*
+ * Function takes a device reference on the port device. Caller should do a
+ * put_device() when done.
+ */
+static struct cxl_port *find_cxl_port_by_uport(struct device *uport_dev)
+{
+ struct device *dev;
+
+ dev = bus_find_device(&cxl_bus_type, NULL, uport_dev, match_port_by_uport);
+ if (dev)
+ return to_cxl_port(dev);
+ return NULL;
+}
+
+static int update_switch_decoder(struct device *dev, void *data)
+{
+ struct cxl_dport *dport = data;
+ struct cxl_switch_decoder *cxlsd;
+ struct cxl_decoder *cxld;
+ int i;
+
+ if (!is_switch_decoder(dev))
+ return 0;
+
+ cxlsd = to_cxl_switch_decoder(dev);
+ cxld = &cxlsd->cxld;
+ guard(rwsem_write)(&cxl_region_rwsem);
+ for (i = 0; i < cxld->interleave_ways; i++) {
+ if (cxlsd->target_map[i] == dport->port_id) {
+ cxlsd->target[i] = dport;
+ return 0;
+ }
+ }
+
+ dev_dbg(dev, "Updating decoder target_map with %s and none found\n",
+ dev_name(dport->dport_dev));
+
+ return 0;
+}
+
+static int update_decoders_with_dport(struct cxl_port *port, struct cxl_dport *dport)
+{
+ device_lock_assert(&port->dev);
+ return device_for_each_child(&port->dev, dport, update_switch_decoder);
+}
+
+static int cxl_port_setup_with_dport(struct cxl_port *port,
+ struct cxl_dport *dport)
+{
+ device_lock_assert(&port->dev);
+
+ cxl_switch_parse_cdat(port);
+
+ return update_decoders_with_dport(port, dport);
+}
+
+static struct cxl_dport *devm_cxl_port_add_dport(struct cxl_port *port,
+ struct device *dport_dev)
+{
+ struct cxl_dport *dport;
+ int rc;
+
+ device_lock_assert(&port->dev);
+
+ /* Port driver not attached yet, wait for cxl_acpi reprobe */
+ if (!port->dev.driver)
+ return ERR_PTR(-ENODEV);
+
+ dport = cxl_find_dport_by_dev(port, dport_dev);
+ if (dport)
+ return dport;
+
+ dport = devm_cxl_add_dport_by_dev(port, dport_dev);
+ if (IS_ERR(dport))
+ return dport;
+
+ rc = cxl_port_setup_with_dport(port, dport);
+ if (rc) {
+ reap_dport(port, dport);
+ return ERR_PTR(rc);
+ }
+
+ return dport;
+}
+
+static struct cxl_dport *devm_cxl_add_dport_by_uport(struct device *uport_dev,
+ struct device *dport_dev)
+{
+ struct cxl_port *port __free(put_cxl_port) =
+ find_cxl_port_by_uport(uport_dev);
+
+ if (!port)
+ return ERR_PTR(-ENODEV);
+
+ guard(device)(&port->dev);
+ return devm_cxl_port_add_dport(port, dport_dev);
+}
+
+static struct cxl_dport *devm_cxl_add_dport_for_rp(struct device *uport_dev,
+ struct device *dport_dev)
+{
+ struct device *dparent = grandparent(dport_dev);
+
+ /* Only go down this path if we are at the root port */
+ if (!is_cxl_hierarchy_head(dparent))
+ return NULL;
+
+ return devm_cxl_add_dport_by_uport(uport_dev, dport_dev);
+}
+
static int add_port_attach_ep(struct cxl_memdev *cxlmd,
struct device *uport_dev,
struct device *dport_dev)
@@ -1584,6 +1706,8 @@ static int add_port_attach_ep(struct cxl_memdev *cxlmd,
*/
struct cxl_port *port __free(put_cxl_port) = NULL;
scoped_guard(device, &parent_port->dev) {
+ struct cxl_dport *new_dport;
+
if (!parent_port->dev.driver) {
dev_warn(&cxlmd->dev,
"port %s:%s disabled, failed to enumerate CXL.mem\n",
@@ -1592,6 +1716,8 @@ static int add_port_attach_ep(struct cxl_memdev *cxlmd,
}
port = find_cxl_port_at(parent_port, dport_dev, &dport);
+ if (!port)
+ port = find_cxl_port_by_uport(uport_dev);
if (!port) {
component_reg_phys = find_component_registers(uport_dev);
port = devm_cxl_add_port(&parent_port->dev, uport_dev,
@@ -1599,11 +1725,27 @@ static int add_port_attach_ep(struct cxl_memdev *cxlmd,
if (IS_ERR(port))
return PTR_ERR(port);
- /* retry find to pick up the new dport information */
- port = find_cxl_port_at(parent_port, dport_dev, &dport);
- if (!port)
- return -ENXIO;
+ /*
+ * The port holds a device reference via find_cxl_port_at()
+ * if the port is valid. But if the port is newly created
+ * via devm_cxl_add_port(), no reference is held. Therefore
+ * the driver needs to get a device reference here.
+ */
+ get_device(&port->dev);
}
+
+ guard(device)(&port->dev);
+ new_dport = devm_cxl_port_add_dport(port, dport_dev);
+ if (IS_ERR(new_dport)) {
+ if (PTR_ERR(new_dport) != -EEXIST)
+ return PTR_ERR(new_dport);
+
+ new_dport = cxl_find_dport_by_dev(port, dport_dev);
+ if (!new_dport)
+ return -ENODEV;
+ }
+
+ dport = new_dport;
}
dev_dbg(&cxlmd->dev, "add to new port %s:%s\n",
@@ -1620,11 +1762,13 @@ static int add_port_attach_ep(struct cxl_memdev *cxlmd,
return rc;
}
+#define CXL_ITER_LEVEL_SWITCH 1
+
int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
{
struct device *dev = &cxlmd->dev;
struct device *iter;
- int rc;
+ int rc, i;
/*
* Skip intermediate port enumeration in the RCH case, there
@@ -1643,7 +1787,7 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
* attempt fails.
*/
retry:
- for (iter = dev; iter; iter = grandparent(iter)) {
+ for (i = 0, iter = dev; iter; i++, iter = grandparent(iter)) {
struct device *dport_dev = grandparent(iter);
struct device *uport_dev;
struct cxl_dport *dport;
@@ -1686,9 +1830,28 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
if (!dev_is_cxl_root_child(&port->dev))
continue;
+ /*
+ * This is a corner case where the rootport is setup but
+ * the switch dport is not. It needs to go back to the
+ * beginning to setup the switch port.
+ */
+ if (i >= CXL_ITER_LEVEL_SWITCH) {
+ struct cxl_port *pport __free(put_cxl_port) =
+ cxl_mem_find_port(cxlmd, &dport);
+ if (!port)
+ goto retry;
+ }
+
return 0;
}
+ dport = devm_cxl_add_dport_for_rp(uport_dev, dport_dev);
+ /* Added a dport, restart enumeration */
+ if (!IS_ERR_OR_NULL(dport))
+ goto retry;
+ if (IS_ERR(dport))
+ return PTR_ERR(dport);
+
rc = add_port_attach_ep(cxlmd, uport_dev, dport_dev);
/* port missing, try to add parent */
if (rc == -EAGAIN)
@@ -1727,16 +1890,19 @@ static int decoder_populate_targets(struct cxl_switch_decoder *cxlsd,
return 0;
device_lock_assert(&port->dev);
+ memcpy(cxlsd->target_map, target_map, sizeof(cxlsd->target_map));
if (xa_empty(&port->dports))
- return -EINVAL;
+ return 0;
guard(rwsem_write)(&cxl_region_rwsem);
for (i = 0; i < cxlsd->cxld.interleave_ways; i++) {
struct cxl_dport *dport = find_dport(port, target_map[i]);
- if (!dport)
- return -ENXIO;
+ if (!dport) {
+ /* dport may be activated later */
+ continue;
+ }
cxlsd->target[i] = dport;
}
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 3f1695c96abc..de7883747555 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -403,6 +403,7 @@ struct cxl_endpoint_decoder {
* struct cxl_switch_decoder - Switch specific CXL HDM Decoder
* @cxld: base cxl_decoder object
* @nr_targets: number of elements in @target
+ * @target_map: map of target dport ids to interleave positions
* @target: active ordered target list in current decoder configuration
*
* The 'switch' decoder type represents the decoder instances of cxl_port's that
@@ -414,6 +415,7 @@ struct cxl_endpoint_decoder {
struct cxl_switch_decoder {
struct cxl_decoder cxld;
int nr_targets;
+ int target_map[CXL_DECODER_MAX_INTERLEAVE];
struct cxl_dport *target[];
};
@@ -584,6 +586,7 @@ struct cxl_dax_region {
* @parent_dport: dport that points to this port in the parent
* @decoder_ida: allocator for decoder ids
* @reg_map: component and ras register mapping parameters
+ * @total_dports: total possible dports in this port
* @nr_dports: number of entries in @dports
* @hdm_end: track last allocated HDM decoder instance for allocation ordering
* @commit_end: cursor to track highest committed decoder for commit ordering
@@ -604,6 +607,7 @@ struct cxl_port {
struct cxl_dport *parent_dport;
struct ida decoder_ida;
struct cxl_register_map reg_map;
+ int total_dports;
int nr_dports;
int hdm_end;
int commit_end;
@@ -902,6 +906,7 @@ void cxl_coordinates_combine(struct access_coordinate *out,
struct access_coordinate *c2);
bool cxl_endpoint_decoder_reset_detected(struct cxl_port *port);
+int cxl_port_update_total_dports(struct cxl_port *port);
/*
* Unit test builds overrides this to __weak, find the 'strong' version
diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
index fe4b593331da..ee7dcd7c4f24 100644
--- a/drivers/cxl/port.c
+++ b/drivers/cxl/port.c
@@ -65,12 +65,10 @@ static int cxl_switch_port_probe(struct cxl_port *port)
/* Cache the data early to ensure is_visible() works */
read_cdat_data(port);
- rc = devm_cxl_port_enumerate_dports(port);
- if (rc < 0)
+ rc = cxl_port_update_total_dports(port);
+ if (rc)
return rc;
- cxl_switch_parse_cdat(port);
-
cxlhdm = devm_cxl_setup_hdm(port, NULL);
if (!IS_ERR(cxlhdm))
return devm_cxl_enumerate_decoders(cxlhdm, NULL);
@@ -80,7 +78,7 @@ static int cxl_switch_port_probe(struct cxl_port *port)
return PTR_ERR(cxlhdm);
}
- if (rc == 1) {
+ if (port->total_dports == 1) {
dev_dbg(&port->dev, "Fallback to passthrough decoder\n");
return devm_cxl_add_passthrough_decoder(port);
}
--
2.50.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v5 04/10] cxl: Defer dport allocation for switch ports
2025-07-03 23:27 ` [PATCH v5 04/10] cxl: Defer dport allocation for switch ports Dave Jiang
@ 2025-07-04 9:52 ` Jonathan Cameron
2025-07-07 17:59 ` Dave Jiang
2025-07-07 22:48 ` Dave Jiang
2025-07-08 6:21 ` Li Ming
1 sibling, 2 replies; 20+ messages in thread
From: Jonathan Cameron @ 2025-07-04 9:52 UTC (permalink / raw)
To: Dave Jiang
Cc: linux-cxl, dave, alison.schofield, vishal.l.verma, ira.weiny,
dan.j.williams
Hi Dave,
> ---
> v5:
> - Change cxl_port_get_total_dports() to cxl_port_update_total_dports(). (Jonathan)
> - Return dport for the devm_cxl_port_add_dport() and friends. (Jonathan)
I'm not following how the code around this works. See below.
If I'm reading it right and it should just be a return ERR_PTR(-EEXIST)
where noted, with that fixed:
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index 9691da831224..ffd39c5b7222 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -1551,6 +1551,128 @@ static resource_size_t find_component_registers(struct device *dev)
> +
> +static struct cxl_dport *devm_cxl_port_add_dport(struct cxl_port *port,
> + struct device *dport_dev)
> +{
> + struct cxl_dport *dport;
> + int rc;
> +
> + device_lock_assert(&port->dev);
> +
> + /* Port driver not attached yet, wait for cxl_acpi reprobe */
> + if (!port->dev.driver)
> + return ERR_PTR(-ENODEV);
> +
> + dport = cxl_find_dport_by_dev(port, dport_dev);
> + if (dport)
> + return dport;
I was expecting this to return -EEXIST.
Only going to be visible in the devm_cxl_add_dport_for_rp()
though so maybe test isn't hitting that?
> +
> + dport = devm_cxl_add_dport_by_dev(port, dport_dev);
> + if (IS_ERR(dport))
> + return dport;
> +
> + rc = cxl_port_setup_with_dport(port, dport);
> + if (rc) {
> + reap_dport(port, dport);
> + return ERR_PTR(rc);
> + }
> +
> + return dport;
> +}
> +
> +static struct cxl_dport *devm_cxl_add_dport_by_uport(struct device *uport_dev,
> + struct device *dport_dev)
> +{
> + struct cxl_port *port __free(put_cxl_port) =
> + find_cxl_port_by_uport(uport_dev);
> +
> + if (!port)
> + return ERR_PTR(-ENODEV);
> +
> + guard(device)(&port->dev);
> + return devm_cxl_port_add_dport(port, dport_dev);
> +}
> +
> +static struct cxl_dport *devm_cxl_add_dport_for_rp(struct device *uport_dev,
> + struct device *dport_dev)
> +{
> + struct device *dparent = grandparent(dport_dev);
> +
> + /* Only go down this path if we are at the root port */
> + if (!is_cxl_hierarchy_head(dparent))
> + return NULL;
> +
> + return devm_cxl_add_dport_by_uport(uport_dev, dport_dev);
> +}
> +
> static int add_port_attach_ep(struct cxl_memdev *cxlmd,
> struct device *uport_dev,
> struct device *dport_dev)
> @@ -1584,6 +1706,8 @@ static int add_port_attach_ep(struct cxl_memdev *cxlmd,
> */
> struct cxl_port *port __free(put_cxl_port) = NULL;
> scoped_guard(device, &parent_port->dev) {
> + struct cxl_dport *new_dport;
> +
> if (!parent_port->dev.driver) {
> dev_warn(&cxlmd->dev,
> "port %s:%s disabled, failed to enumerate CXL.mem\n",
> @@ -1592,6 +1716,8 @@ static int add_port_attach_ep(struct cxl_memdev *cxlmd,
> }
>
> port = find_cxl_port_at(parent_port, dport_dev, &dport);
> + if (!port)
> + port = find_cxl_port_by_uport(uport_dev);
> if (!port) {
> component_reg_phys = find_component_registers(uport_dev);
> port = devm_cxl_add_port(&parent_port->dev, uport_dev,
> @@ -1599,11 +1725,27 @@ static int add_port_attach_ep(struct cxl_memdev *cxlmd,
> if (IS_ERR(port))
> return PTR_ERR(port);
>
> - /* retry find to pick up the new dport information */
> - port = find_cxl_port_at(parent_port, dport_dev, &dport);
> - if (!port)
> - return -ENXIO;
> + /*
> + * The port holds a device reference via find_cxl_port_at()
> + * if the port is valid. But if the port is newly created
> + * via devm_cxl_add_port(), no reference is held. Therefore
> + * the driver needs to get a device reference here.
> + */
> + get_device(&port->dev);
> }
> +
> + guard(device)(&port->dev);
> + new_dport = devm_cxl_port_add_dport(port, dport_dev);
> + if (IS_ERR(new_dport)) {
> + if (PTR_ERR(new_dport) != -EEXIST)
> + return PTR_ERR(new_dport);
I think we never get here currently due to lack of returning -EEXIST.
> +
> + new_dport = cxl_find_dport_by_dev(port, dport_dev);
> + if (!new_dport)
> + return -ENODEV;
> + }
> +
> + dport = new_dport;
> }
>
> dev_dbg(&cxlmd->dev, "add to new port %s:%s\n",
> @@ -1620,11 +1762,13 @@ static int add_port_attach_ep(struct cxl_memdev *cxlmd,
> return rc;
> }
>
> +#define CXL_ITER_LEVEL_SWITCH 1
> +
> int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
> {
> struct device *dev = &cxlmd->dev;
> struct device *iter;
> - int rc;
> + int rc, i;
>
> /*
> * Skip intermediate port enumeration in the RCH case, there
> @@ -1643,7 +1787,7 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
> * attempt fails.
> */
> retry:
> - for (iter = dev; iter; iter = grandparent(iter)) {
> + for (i = 0, iter = dev; iter; i++, iter = grandparent(iter)) {
> struct device *dport_dev = grandparent(iter);
> struct device *uport_dev;
> struct cxl_dport *dport;
> @@ -1686,9 +1830,28 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
> if (!dev_is_cxl_root_child(&port->dev))
> continue;
>
> + /*
> + * This is a corner case where the rootport is setup but
> + * the switch dport is not. It needs to go back to the
> + * beginning to setup the switch port.
> + */
> + if (i >= CXL_ITER_LEVEL_SWITCH) {
> + struct cxl_port *pport __free(put_cxl_port) =
> + cxl_mem_find_port(cxlmd, &dport);
> + if (!port)
> + goto retry;
> + }
> +
> return 0;
> }
>
> + dport = devm_cxl_add_dport_for_rp(uport_dev, dport_dev);
> + /* Added a dport, restart enumeration */
> + if (!IS_ERR_OR_NULL(dport))
> + goto retry;
As above. I think if this finds a already existing dport we loop for ever.
> + if (IS_ERR(dport))
> + return PTR_ERR(dport);
> +
> rc = add_port_attach_ep(cxlmd, uport_dev, dport_dev);
> /* port missing, try to add parent */
> if (rc == -EAGAIN)
}
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v5 04/10] cxl: Defer dport allocation for switch ports
2025-07-04 9:52 ` Jonathan Cameron
@ 2025-07-07 17:59 ` Dave Jiang
2025-07-07 22:48 ` Dave Jiang
1 sibling, 0 replies; 20+ messages in thread
From: Dave Jiang @ 2025-07-07 17:59 UTC (permalink / raw)
To: Jonathan Cameron
Cc: linux-cxl, dave, alison.schofield, vishal.l.verma, ira.weiny,
dan.j.williams
On 7/4/25 2:52 AM, Jonathan Cameron wrote:
>
> Hi Dave,
>
>> ---
>> v5:
>> - Change cxl_port_get_total_dports() to cxl_port_update_total_dports(). (Jonathan)
>> - Return dport for the devm_cxl_port_add_dport() and friends. (Jonathan)
> I'm not following how the code around this works. See below.
>
> If I'm reading it right and it should just be a return ERR_PTR(-EEXIST)
> where noted, with that fixed:
Yes will get that fixed. Thanks!
DJ
>
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
>
>
>> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
>> index 9691da831224..ffd39c5b7222 100644
>> --- a/drivers/cxl/core/port.c
>> +++ b/drivers/cxl/core/port.c
>> @@ -1551,6 +1551,128 @@ static resource_size_t find_component_registers(struct device *dev)
>
>> +
>> +static struct cxl_dport *devm_cxl_port_add_dport(struct cxl_port *port,
>> + struct device *dport_dev)
>> +{
>> + struct cxl_dport *dport;
>> + int rc;
>> +
>> + device_lock_assert(&port->dev);
>> +
>> + /* Port driver not attached yet, wait for cxl_acpi reprobe */
>> + if (!port->dev.driver)
>> + return ERR_PTR(-ENODEV);
>> +
>> + dport = cxl_find_dport_by_dev(port, dport_dev);
>> + if (dport)
>> + return dport;
>
> I was expecting this to return -EEXIST.
> Only going to be visible in the devm_cxl_add_dport_for_rp()
> though so maybe test isn't hitting that?
>
>> +
>> + dport = devm_cxl_add_dport_by_dev(port, dport_dev);
>> + if (IS_ERR(dport))
>> + return dport;
>> +
>> + rc = cxl_port_setup_with_dport(port, dport);
>> + if (rc) {
>> + reap_dport(port, dport);
>> + return ERR_PTR(rc);
>> + }
>> +
>> + return dport;
>> +}
>> +
>> +static struct cxl_dport *devm_cxl_add_dport_by_uport(struct device *uport_dev,
>> + struct device *dport_dev)
>> +{
>> + struct cxl_port *port __free(put_cxl_port) =
>> + find_cxl_port_by_uport(uport_dev);
>> +
>> + if (!port)
>> + return ERR_PTR(-ENODEV);
>> +
>> + guard(device)(&port->dev);
>> + return devm_cxl_port_add_dport(port, dport_dev);
>> +}
>> +
>> +static struct cxl_dport *devm_cxl_add_dport_for_rp(struct device *uport_dev,
>> + struct device *dport_dev)
>> +{
>> + struct device *dparent = grandparent(dport_dev);
>> +
>> + /* Only go down this path if we are at the root port */
>> + if (!is_cxl_hierarchy_head(dparent))
>> + return NULL;
>> +
>> + return devm_cxl_add_dport_by_uport(uport_dev, dport_dev);
>> +}
>> +
>> static int add_port_attach_ep(struct cxl_memdev *cxlmd,
>> struct device *uport_dev,
>> struct device *dport_dev)
>> @@ -1584,6 +1706,8 @@ static int add_port_attach_ep(struct cxl_memdev *cxlmd,
>> */
>> struct cxl_port *port __free(put_cxl_port) = NULL;
>> scoped_guard(device, &parent_port->dev) {
>> + struct cxl_dport *new_dport;
>> +
>> if (!parent_port->dev.driver) {
>> dev_warn(&cxlmd->dev,
>> "port %s:%s disabled, failed to enumerate CXL.mem\n",
>> @@ -1592,6 +1716,8 @@ static int add_port_attach_ep(struct cxl_memdev *cxlmd,
>> }
>>
>> port = find_cxl_port_at(parent_port, dport_dev, &dport);
>> + if (!port)
>> + port = find_cxl_port_by_uport(uport_dev);
>> if (!port) {
>> component_reg_phys = find_component_registers(uport_dev);
>> port = devm_cxl_add_port(&parent_port->dev, uport_dev,
>> @@ -1599,11 +1725,27 @@ static int add_port_attach_ep(struct cxl_memdev *cxlmd,
>> if (IS_ERR(port))
>> return PTR_ERR(port);
>>
>> - /* retry find to pick up the new dport information */
>> - port = find_cxl_port_at(parent_port, dport_dev, &dport);
>> - if (!port)
>> - return -ENXIO;
>> + /*
>> + * The port holds a device reference via find_cxl_port_at()
>> + * if the port is valid. But if the port is newly created
>> + * via devm_cxl_add_port(), no reference is held. Therefore
>> + * the driver needs to get a device reference here.
>> + */
>> + get_device(&port->dev);
>> }
>> +
>> + guard(device)(&port->dev);
>> + new_dport = devm_cxl_port_add_dport(port, dport_dev);
>> + if (IS_ERR(new_dport)) {
>> + if (PTR_ERR(new_dport) != -EEXIST)
>> + return PTR_ERR(new_dport);
>
> I think we never get here currently due to lack of returning -EEXIST.
>
>> +
>> + new_dport = cxl_find_dport_by_dev(port, dport_dev);
>> + if (!new_dport)
>> + return -ENODEV;
>> + }
>> +
>> + dport = new_dport;
>> }
>>
>> dev_dbg(&cxlmd->dev, "add to new port %s:%s\n",
>> @@ -1620,11 +1762,13 @@ static int add_port_attach_ep(struct cxl_memdev *cxlmd,
>> return rc;
>> }
>>
>> +#define CXL_ITER_LEVEL_SWITCH 1
>> +
>> int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
>> {
>> struct device *dev = &cxlmd->dev;
>> struct device *iter;
>> - int rc;
>> + int rc, i;
>>
>> /*
>> * Skip intermediate port enumeration in the RCH case, there
>> @@ -1643,7 +1787,7 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
>> * attempt fails.
>> */
>> retry:
>> - for (iter = dev; iter; iter = grandparent(iter)) {
>> + for (i = 0, iter = dev; iter; i++, iter = grandparent(iter)) {
>> struct device *dport_dev = grandparent(iter);
>> struct device *uport_dev;
>> struct cxl_dport *dport;
>> @@ -1686,9 +1830,28 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
>> if (!dev_is_cxl_root_child(&port->dev))
>> continue;
>>
>> + /*
>> + * This is a corner case where the rootport is setup but
>> + * the switch dport is not. It needs to go back to the
>> + * beginning to setup the switch port.
>> + */
>> + if (i >= CXL_ITER_LEVEL_SWITCH) {
>> + struct cxl_port *pport __free(put_cxl_port) =
>> + cxl_mem_find_port(cxlmd, &dport);
>> + if (!port)
>> + goto retry;
>> + }
>> +
>> return 0;
>> }
>>
>> + dport = devm_cxl_add_dport_for_rp(uport_dev, dport_dev);
>> + /* Added a dport, restart enumeration */
>> + if (!IS_ERR_OR_NULL(dport))
>> + goto retry;
>
> As above. I think if this finds a already existing dport we loop for ever.
>
>> + if (IS_ERR(dport))
>> + return PTR_ERR(dport);
>> +
>> rc = add_port_attach_ep(cxlmd, uport_dev, dport_dev);
>> /* port missing, try to add parent */
>> if (rc == -EAGAIN)
> }
>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v5 04/10] cxl: Defer dport allocation for switch ports
2025-07-04 9:52 ` Jonathan Cameron
2025-07-07 17:59 ` Dave Jiang
@ 2025-07-07 22:48 ` Dave Jiang
1 sibling, 0 replies; 20+ messages in thread
From: Dave Jiang @ 2025-07-07 22:48 UTC (permalink / raw)
To: Jonathan Cameron
Cc: linux-cxl, dave, alison.schofield, vishal.l.verma, ira.weiny,
dan.j.williams
On 7/4/25 2:52 AM, Jonathan Cameron wrote:
>
> Hi Dave,
>
>> ---
>> v5:
>> - Change cxl_port_get_total_dports() to cxl_port_update_total_dports(). (Jonathan)
>> - Return dport for the devm_cxl_port_add_dport() and friends. (Jonathan)
> I'm not following how the code around this works. See below.
>
> If I'm reading it right and it should just be a return ERR_PTR(-EEXIST)
> where noted, with that fixed:
>
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
>
>
>> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
>> index 9691da831224..ffd39c5b7222 100644
>> --- a/drivers/cxl/core/port.c
>> +++ b/drivers/cxl/core/port.c
>> @@ -1551,6 +1551,128 @@ static resource_size_t find_component_registers(struct device *dev)
>
>> +
>> +static struct cxl_dport *devm_cxl_port_add_dport(struct cxl_port *port,
>> + struct device *dport_dev)
>> +{
>> + struct cxl_dport *dport;
>> + int rc;
>> +
>> + device_lock_assert(&port->dev);
>> +
>> + /* Port driver not attached yet, wait for cxl_acpi reprobe */
>> + if (!port->dev.driver)
>> + return ERR_PTR(-ENODEV);
>> +
>> + dport = cxl_find_dport_by_dev(port, dport_dev);
>> + if (dport)
>> + return dport;
>
> I was expecting this to return -EEXIST.
> Only going to be visible in the devm_cxl_add_dport_for_rp()
> though so maybe test isn't hitting that?
>
>> +
>> + dport = devm_cxl_add_dport_by_dev(port, dport_dev);
>> + if (IS_ERR(dport))
>> + return dport;
>> +
>> + rc = cxl_port_setup_with_dport(port, dport);
>> + if (rc) {
>> + reap_dport(port, dport);
>> + return ERR_PTR(rc);
>> + }
>> +
>> + return dport;
>> +}
>> +
>> +static struct cxl_dport *devm_cxl_add_dport_by_uport(struct device *uport_dev,
>> + struct device *dport_dev)
>> +{
>> + struct cxl_port *port __free(put_cxl_port) =
>> + find_cxl_port_by_uport(uport_dev);
>> +
>> + if (!port)
>> + return ERR_PTR(-ENODEV);
>> +
>> + guard(device)(&port->dev);
>> + return devm_cxl_port_add_dport(port, dport_dev);
>> +}
>> +
>> +static struct cxl_dport *devm_cxl_add_dport_for_rp(struct device *uport_dev,
>> + struct device *dport_dev)
>> +{
>> + struct device *dparent = grandparent(dport_dev);
>> +
>> + /* Only go down this path if we are at the root port */
>> + if (!is_cxl_hierarchy_head(dparent))
>> + return NULL;
>> +
>> + return devm_cxl_add_dport_by_uport(uport_dev, dport_dev);
>> +}
>> +
>> static int add_port_attach_ep(struct cxl_memdev *cxlmd,
>> struct device *uport_dev,
>> struct device *dport_dev)
>> @@ -1584,6 +1706,8 @@ static int add_port_attach_ep(struct cxl_memdev *cxlmd,
>> */
>> struct cxl_port *port __free(put_cxl_port) = NULL;
>> scoped_guard(device, &parent_port->dev) {
>> + struct cxl_dport *new_dport;
>> +
>> if (!parent_port->dev.driver) {
>> dev_warn(&cxlmd->dev,
>> "port %s:%s disabled, failed to enumerate CXL.mem\n",
>> @@ -1592,6 +1716,8 @@ static int add_port_attach_ep(struct cxl_memdev *cxlmd,
>> }
>>
>> port = find_cxl_port_at(parent_port, dport_dev, &dport);
>> + if (!port)
>> + port = find_cxl_port_by_uport(uport_dev);
>> if (!port) {
>> component_reg_phys = find_component_registers(uport_dev);
>> port = devm_cxl_add_port(&parent_port->dev, uport_dev,
>> @@ -1599,11 +1725,27 @@ static int add_port_attach_ep(struct cxl_memdev *cxlmd,
>> if (IS_ERR(port))
>> return PTR_ERR(port);
>>
>> - /* retry find to pick up the new dport information */
>> - port = find_cxl_port_at(parent_port, dport_dev, &dport);
>> - if (!port)
>> - return -ENXIO;
>> + /*
>> + * The port holds a device reference via find_cxl_port_at()
>> + * if the port is valid. But if the port is newly created
>> + * via devm_cxl_add_port(), no reference is held. Therefore
>> + * the driver needs to get a device reference here.
>> + */
>> + get_device(&port->dev);
>> }
>> +
>> + guard(device)(&port->dev);
>> + new_dport = devm_cxl_port_add_dport(port, dport_dev);
>> + if (IS_ERR(new_dport)) {
>> + if (PTR_ERR(new_dport) != -EEXIST)
>> + return PTR_ERR(new_dport);
>
> I think we never get here currently due to lack of returning -EEXIST.
>
>> +
>> + new_dport = cxl_find_dport_by_dev(port, dport_dev);
>> + if (!new_dport)
>> + return -ENODEV;
>> + }
>> +
>> + dport = new_dport;
>> }
>>
>> dev_dbg(&cxlmd->dev, "add to new port %s:%s\n",
>> @@ -1620,11 +1762,13 @@ static int add_port_attach_ep(struct cxl_memdev *cxlmd,
>> return rc;
>> }
>>
>> +#define CXL_ITER_LEVEL_SWITCH 1
>> +
>> int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
>> {
>> struct device *dev = &cxlmd->dev;
>> struct device *iter;
>> - int rc;
>> + int rc, i;
>>
>> /*
>> * Skip intermediate port enumeration in the RCH case, there
>> @@ -1643,7 +1787,7 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
>> * attempt fails.
>> */
>> retry:
>> - for (iter = dev; iter; iter = grandparent(iter)) {
>> + for (i = 0, iter = dev; iter; i++, iter = grandparent(iter)) {
>> struct device *dport_dev = grandparent(iter);
>> struct device *uport_dev;
>> struct cxl_dport *dport;
>> @@ -1686,9 +1830,28 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
>> if (!dev_is_cxl_root_child(&port->dev))
>> continue;
>>
>> + /*
>> + * This is a corner case where the rootport is setup but
>> + * the switch dport is not. It needs to go back to the
>> + * beginning to setup the switch port.
>> + */
>> + if (i >= CXL_ITER_LEVEL_SWITCH) {
>> + struct cxl_port *pport __free(put_cxl_port) =
>> + cxl_mem_find_port(cxlmd, &dport);
>> + if (!port)
>> + goto retry;
>> + }
>> +
>> return 0;
>> }
>>
>> + dport = devm_cxl_add_dport_for_rp(uport_dev, dport_dev);
>> + /* Added a dport, restart enumeration */
>> + if (!IS_ERR_OR_NULL(dport))
>> + goto retry;
>
> As above. I think if this finds a already existing dport we loop for ever.
Actually it is the opposite. If we found an existing dport, we will want to start the hierarchy walk from the beginning. If we allow it to drop through, things don't work properly during testing. It ends up being something like:
+ dport = devm_cxl_add_dport_for_rp(uport_dev, dport_dev);
+ /* Added a dport, restart enumeration */
+ if (!IS_ERR_OR_NULL(dport) || PTR_ERR(dport) == -EEXIST)
+ goto retry;
+ if (IS_ERR(dport))
+ return PTR_ERR(dport);
DJ
>
>> + if (IS_ERR(dport))
>> + return PTR_ERR(dport);
>> +
>> rc = add_port_attach_ep(cxlmd, uport_dev, dport_dev);
>> /* port missing, try to add parent */
>> if (rc == -EAGAIN)
> }
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 04/10] cxl: Defer dport allocation for switch ports
2025-07-03 23:27 ` [PATCH v5 04/10] cxl: Defer dport allocation for switch ports Dave Jiang
2025-07-04 9:52 ` Jonathan Cameron
@ 2025-07-08 6:21 ` Li Ming
2025-07-08 21:47 ` Dave Jiang
1 sibling, 1 reply; 20+ messages in thread
From: Li Ming @ 2025-07-08 6:21 UTC (permalink / raw)
To: Dave Jiang, linux-cxl
Cc: dave, jonathan.cameron, alison.schofield, vishal.l.verma,
ira.weiny, dan.j.williams
On 7/4/2025 7:27 AM, Dave Jiang wrote:
> The current implementation enumerates the dports during the cxl_port
> driver probe. Without an endpoint connected, the dport may not be
> active during port probe. This scheme may prevent a valid hardware
> dport id to be retrieved and MMIO registers to be read when an endpoint
> is hot-plugged. Move the dport allocation and setup to behind memdev
> probe so the endpoint is guaranteed to be connected.
>
> In the original enumeration behavior, there are 3 phases (or 2 if no CXL
> switches) for port creation. cxl_acpi() creates a Root Port (RP) from the
> ACPI0017.N device. Through that it enumerate downstream ports composed
> of ACPI0016.N devices through add_host_bridge_dport(). Once done, it
> use add_host_bridge_uport() to create the ports that enumerates the PCI
> RPs as the dports of these ports. Every time a port is created, the port
> driver is attached and drv->probe() is called and
> devm_cxl_port_enumerate_dports() is envoked to enumerate and probe
> the dports.
>
> The second phase is if there are any CXL switches. When the pci endpoint
> device driver (cxl_pci) calls probe, it will add a mem device and triggers
> the cxl_mem->probe(). cxl_mem->probe() calls devm_cxl_enumerate_ports()
> and attempts to discovery and create all the ports represent CXL switches.
> During this phase, a port is created per switch and the attached dports
> are also enumerated and probed.
>
> The last phase is creating endpoint port which happens for all endpoint
> devices.
>
> In this commit, the port create and its dport probing in cxl_acpi is not
> changed. That will be handled in a different patch later on. The behavior
> change is only for CXL switch ports. Only the dport that is part of the
> path for an endpoint device to the RP will be probed. This happens
> naturally by the code walking up the device hierarchy and identifying the
> upstream device and the downstream device.
>
> There are two points where the interception of dport creation happens
> during the devm_cxl_enumerate_ports() path. The first location is right
> before the function calls add_port_attach_ep() where it does the dport
> allocation for the RP. Once the dport is allocated, the iteration path
> is reset to the beginning to try again. The second location happens
> in add_port_attach_ep() after the location where either the port is
> discovered or allocated new if it does not exist.
>
> Locking of port device during __cxl_port_add_dport() protects modifications
> against the port and its dports while multiple endpoints can be probing at
> the same time and the same port is being modified concurrently.
>
> While the decoders are allocated during the port driver probe,
> The decoders must also be updated since previously it's all done when all
> the dports are setup and now every time a dport is setup per endpoint, the
> switch target listing need to be updated with new dport. A
> guard(rwsem_write) is used to update decoder targets. This is similar to
> when decoder_populate_target() is called and the decoder programming
> must be protected.
>
> Link: https://lore.kernel.org/linux-cxl/20250305100123.3077031-1-rrichter@amd.com/
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
> v5:
> - Change cxl_port_get_total_dports() to cxl_port_update_total_dports(). (Jonathan)
> - Return dport for the devm_cxl_port_add_dport() and friends. (Jonathan)
> ---
> drivers/cxl/core/core.h | 2 +
> drivers/cxl/core/pci.c | 88 +++++++++++++++++++
> drivers/cxl/core/port.c | 184 ++++++++++++++++++++++++++++++++++++++--
> drivers/cxl/cxl.h | 5 ++
> drivers/cxl/port.c | 8 +-
> 5 files changed, 273 insertions(+), 14 deletions(-)
>
[snip]
> +#define CXL_ITER_LEVEL_SWITCH 1
> +
> int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
> {
> struct device *dev = &cxlmd->dev;
> struct device *iter;
> - int rc;
> + int rc, i;
>
> /*
> * Skip intermediate port enumeration in the RCH case, there
> @@ -1643,7 +1787,7 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
> * attempt fails.
> */
> retry:
> - for (iter = dev; iter; iter = grandparent(iter)) {
> + for (i = 0, iter = dev; iter; i++, iter = grandparent(iter)) {
> struct device *dport_dev = grandparent(iter);
> struct device *uport_dev;
> struct cxl_dport *dport;
> @@ -1686,9 +1830,28 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
> if (!dev_is_cxl_root_child(&port->dev))
> continue;
>
> + /*
> + * This is a corner case where the rootport is setup but
> + * the switch dport is not. It needs to go back to the
> + * beginning to setup the switch port.
> + */
> + if (i >= CXL_ITER_LEVEL_SWITCH) {
> + struct cxl_port *pport __free(put_cxl_port) =
> + cxl_mem_find_port(cxlmd, &dport);
> + if (!port)
> + goto retry;
should be "if (!pport)" here?
> + }
> +
> return 0;
> }
>
> + dport = devm_cxl_add_dport_for_rp(uport_dev, dport_dev);
> + /* Added a dport, restart enumeration */
> + if (!IS_ERR_OR_NULL(dport))
> + goto retry;
> + if (IS_ERR(dport))
> + return PTR_ERR(dport);
> +
This part is a little bit complicated. My understanding is that devm_cxl_add_dport_for_rp() is invoked only for uport_dev == cxl host bridge case, and below add_port_attach_ep() is invoked only for uport_dev == cxl switch USP case, is it?
If yes, maybe it is better to use a if {} else {} block here? like
if (uport_dev is host bridge) {
devm_cxl_add_dport_for_rp();
} else {
add_switch_port_attach_ep();
}
Ming
[snip]
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v5 04/10] cxl: Defer dport allocation for switch ports
2025-07-08 6:21 ` Li Ming
@ 2025-07-08 21:47 ` Dave Jiang
0 siblings, 0 replies; 20+ messages in thread
From: Dave Jiang @ 2025-07-08 21:47 UTC (permalink / raw)
To: Li Ming, linux-cxl
Cc: dave, jonathan.cameron, alison.schofield, vishal.l.verma,
ira.weiny, dan.j.williams
On 7/7/25 11:21 PM, Li Ming wrote:
> On 7/4/2025 7:27 AM, Dave Jiang wrote:
>> The current implementation enumerates the dports during the cxl_port
>> driver probe. Without an endpoint connected, the dport may not be
>> active during port probe. This scheme may prevent a valid hardware
>> dport id to be retrieved and MMIO registers to be read when an endpoint
>> is hot-plugged. Move the dport allocation and setup to behind memdev
>> probe so the endpoint is guaranteed to be connected.
>>
>> In the original enumeration behavior, there are 3 phases (or 2 if no CXL
>> switches) for port creation. cxl_acpi() creates a Root Port (RP) from the
>> ACPI0017.N device. Through that it enumerate downstream ports composed
>> of ACPI0016.N devices through add_host_bridge_dport(). Once done, it
>> use add_host_bridge_uport() to create the ports that enumerates the PCI
>> RPs as the dports of these ports. Every time a port is created, the port
>> driver is attached and drv->probe() is called and
>> devm_cxl_port_enumerate_dports() is envoked to enumerate and probe
>> the dports.
>>
>> The second phase is if there are any CXL switches. When the pci endpoint
>> device driver (cxl_pci) calls probe, it will add a mem device and triggers
>> the cxl_mem->probe(). cxl_mem->probe() calls devm_cxl_enumerate_ports()
>> and attempts to discovery and create all the ports represent CXL switches.
>> During this phase, a port is created per switch and the attached dports
>> are also enumerated and probed.
>>
>> The last phase is creating endpoint port which happens for all endpoint
>> devices.
>>
>> In this commit, the port create and its dport probing in cxl_acpi is not
>> changed. That will be handled in a different patch later on. The behavior
>> change is only for CXL switch ports. Only the dport that is part of the
>> path for an endpoint device to the RP will be probed. This happens
>> naturally by the code walking up the device hierarchy and identifying the
>> upstream device and the downstream device.
>>
>> There are two points where the interception of dport creation happens
>> during the devm_cxl_enumerate_ports() path. The first location is right
>> before the function calls add_port_attach_ep() where it does the dport
>> allocation for the RP. Once the dport is allocated, the iteration path
>> is reset to the beginning to try again. The second location happens
>> in add_port_attach_ep() after the location where either the port is
>> discovered or allocated new if it does not exist.
>>
>> Locking of port device during __cxl_port_add_dport() protects modifications
>> against the port and its dports while multiple endpoints can be probing at
>> the same time and the same port is being modified concurrently.
>>
>> While the decoders are allocated during the port driver probe,
>> The decoders must also be updated since previously it's all done when all
>> the dports are setup and now every time a dport is setup per endpoint, the
>> switch target listing need to be updated with new dport. A
>> guard(rwsem_write) is used to update decoder targets. This is similar to
>> when decoder_populate_target() is called and the decoder programming
>> must be protected.
>>
>> Link: https://lore.kernel.org/linux-cxl/20250305100123.3077031-1-rrichter@amd.com/
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>> ---
>> v5:
>> - Change cxl_port_get_total_dports() to cxl_port_update_total_dports(). (Jonathan)
>> - Return dport for the devm_cxl_port_add_dport() and friends. (Jonathan)
>> ---
>> drivers/cxl/core/core.h | 2 +
>> drivers/cxl/core/pci.c | 88 +++++++++++++++++++
>> drivers/cxl/core/port.c | 184 ++++++++++++++++++++++++++++++++++++++--
>> drivers/cxl/cxl.h | 5 ++
>> drivers/cxl/port.c | 8 +-
>> 5 files changed, 273 insertions(+), 14 deletions(-)
>>
> [snip]
>> +#define CXL_ITER_LEVEL_SWITCH 1
>> +
>> int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
>> {
>> struct device *dev = &cxlmd->dev;
>> struct device *iter;
>> - int rc;
>> + int rc, i;
>>
>> /*
>> * Skip intermediate port enumeration in the RCH case, there
>> @@ -1643,7 +1787,7 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
>> * attempt fails.
>> */
>> retry:
>> - for (iter = dev; iter; iter = grandparent(iter)) {
>> + for (i = 0, iter = dev; iter; i++, iter = grandparent(iter)) {
>> struct device *dport_dev = grandparent(iter);
>> struct device *uport_dev;
>> struct cxl_dport *dport;
>> @@ -1686,9 +1830,28 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
>> if (!dev_is_cxl_root_child(&port->dev))
>> continue;
>>
>> + /*
>> + * This is a corner case where the rootport is setup but
>> + * the switch dport is not. It needs to go back to the
>> + * beginning to setup the switch port.
>> + */
>> + if (i >= CXL_ITER_LEVEL_SWITCH) {
>> + struct cxl_port *pport __free(put_cxl_port) =
>> + cxl_mem_find_port(cxlmd, &dport);
>> + if (!port)
>> + goto retry;
> should be "if (!pport)" here?
Yes. Great catch!
>> + }
>> +
>> return 0;
>> }
>>
>> + dport = devm_cxl_add_dport_for_rp(uport_dev, dport_dev);
>> + /* Added a dport, restart enumeration */
>> + if (!IS_ERR_OR_NULL(dport))
>> + goto retry;
>> + if (IS_ERR(dport))
>> + return PTR_ERR(dport);
>> +
>
> This part is a little bit complicated. My understanding is that devm_cxl_add_dport_for_rp() is invoked only for uport_dev == cxl host bridge case, and below add_port_attach_ep() is invoked only for uport_dev == cxl switch USP case, is it?
>
> If yes, maybe it is better to use a if {} else {} block here? like
>
> if (uport_dev is host bridge) {
>
> devm_cxl_add_dport_for_rp();
>
> } else {
>
> add_switch_port_attach_ep();
>
> }
Yes I'll change to that. I can also drop devm_cxl_add_dport_for_rp() and directly call devm_cxl_add_dport_by_uport().
DJ
>
>
> Ming
>
> [snip]
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v5 05/10] cxl/test: Add cxl_test support for cxl_port_update_total_ports()
2025-07-03 23:27 [PATCH v5 00/10] cxl: Delay HB port and switch dport probing until endpoint dev probe Dave Jiang
` (3 preceding siblings ...)
2025-07-03 23:27 ` [PATCH v5 04/10] cxl: Defer dport allocation for switch ports Dave Jiang
@ 2025-07-03 23:27 ` Dave Jiang
2025-07-03 23:27 ` [PATCH v5 06/10] cxl/test: Add mock version of devm_cxl_add_dport_by_dev() Dave Jiang
` (4 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: Dave Jiang @ 2025-07-03 23:27 UTC (permalink / raw)
To: linux-cxl
Cc: dave, jonathan.cameron, alison.schofield, vishal.l.verma,
ira.weiny, dan.j.williams
In the delayed dport allocation scheme, the total number of dports
need to be discovered during port probe. Add the mock function
that does it for cxl_test.
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v5:
- Update to support cxl_port_update_total_dports()
- Consolidate common code for getting the port array (Jonathan)
---
tools/testing/cxl/Kbuild | 1 +
tools/testing/cxl/test/cxl.c | 53 +++++++++++++++++++++++++++++++++--
tools/testing/cxl/test/mock.c | 15 ++++++++++
tools/testing/cxl/test/mock.h | 1 +
4 files changed, 68 insertions(+), 2 deletions(-)
diff --git a/tools/testing/cxl/Kbuild b/tools/testing/cxl/Kbuild
index 31a2d73c963f..b9fbaaca999e 100644
--- a/tools/testing/cxl/Kbuild
+++ b/tools/testing/cxl/Kbuild
@@ -5,6 +5,7 @@ ldflags-y += --wrap=acpi_evaluate_integer
ldflags-y += --wrap=acpi_pci_find_root
ldflags-y += --wrap=nvdimm_bus_register
ldflags-y += --wrap=devm_cxl_port_enumerate_dports
+ldflags-y += --wrap=cxl_port_update_total_dports
ldflags-y += --wrap=devm_cxl_setup_hdm
ldflags-y += --wrap=devm_cxl_add_passthrough_decoder
ldflags-y += --wrap=devm_cxl_enumerate_decoders
diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
index 8a5815ca870d..6fd1eaca6cbe 100644
--- a/tools/testing/cxl/test/cxl.c
+++ b/tools/testing/cxl/test/cxl.c
@@ -920,10 +920,12 @@ static int mock_cxl_enumerate_decoders(struct cxl_hdm *cxlhdm,
return 0;
}
-static int mock_cxl_port_enumerate_dports(struct cxl_port *port)
+static int get_port_array(struct cxl_port *port,
+ struct platform_device ***port_array,
+ int *port_array_size)
{
struct platform_device **array;
- int i, array_size;
+ int array_size;
if (port->depth == 1) {
if (is_multi_bridge(port->uport_dev)) {
@@ -957,6 +959,52 @@ static int mock_cxl_port_enumerate_dports(struct cxl_port *port)
return -ENXIO;
}
+ *port_array = array;
+ *port_array_size = array_size;
+
+ return 0;
+}
+
+static int mock_cxl_port_update_total_dports(struct cxl_port *port)
+{
+ struct platform_device **array;
+ int rc, i, array_size;
+ int dports = 0;
+
+ rc = get_port_array(port, &array, &array_size);
+ if (rc)
+ return rc;
+
+ for (i = 0; i < array_size; i++) {
+ struct platform_device *pdev = array[i];
+
+ if (pdev->dev.parent != port->uport_dev) {
+ dev_dbg(&port->dev, "%s: mismatch parent %s\n",
+ dev_name(port->uport_dev),
+ dev_name(pdev->dev.parent));
+ continue;
+ }
+
+ dports++;
+ }
+
+ if (!dports)
+ return -ENXIO;
+
+ port->total_dports = dports;
+
+ return 0;
+}
+
+static int mock_cxl_port_enumerate_dports(struct cxl_port *port)
+{
+ struct platform_device **array;
+ int rc, i, array_size;
+
+ rc = get_port_array(port, &array, &array_size);
+ if (rc)
+ return rc;
+
for (i = 0; i < array_size; i++) {
struct platform_device *pdev = array[i];
struct cxl_dport *dport;
@@ -1035,6 +1083,7 @@ static struct cxl_mock_ops cxl_mock_ops = {
.acpi_evaluate_integer = mock_acpi_evaluate_integer,
.acpi_pci_find_root = mock_acpi_pci_find_root,
.devm_cxl_port_enumerate_dports = mock_cxl_port_enumerate_dports,
+ .cxl_port_update_total_dports = mock_cxl_port_update_total_dports,
.devm_cxl_setup_hdm = mock_cxl_setup_hdm,
.devm_cxl_add_passthrough_decoder = mock_cxl_add_passthrough_decoder,
.devm_cxl_enumerate_decoders = mock_cxl_enumerate_decoders,
diff --git a/tools/testing/cxl/test/mock.c b/tools/testing/cxl/test/mock.c
index 1989ae020df3..6272c0691275 100644
--- a/tools/testing/cxl/test/mock.c
+++ b/tools/testing/cxl/test/mock.c
@@ -196,6 +196,21 @@ int __wrap_devm_cxl_port_enumerate_dports(struct cxl_port *port)
}
EXPORT_SYMBOL_NS_GPL(__wrap_devm_cxl_port_enumerate_dports, "CXL");
+int __wrap_cxl_port_update_total_dports(struct cxl_port *port)
+{
+ int rc, index;
+ struct cxl_mock_ops *ops = get_cxl_mock_ops(&index);
+
+ if (ops && ops->is_mock_port(port->uport_dev))
+ rc = ops->cxl_port_update_total_dports(port);
+ else
+ rc = cxl_port_update_total_dports(port);
+ put_cxl_mock_ops(index);
+
+ return rc;
+}
+EXPORT_SYMBOL_NS_GPL(__wrap_cxl_port_update_total_dports, "CXL");
+
int __wrap_cxl_await_media_ready(struct cxl_dev_state *cxlds)
{
int rc, index;
diff --git a/tools/testing/cxl/test/mock.h b/tools/testing/cxl/test/mock.h
index d1b0271d2822..bf2a3e5581ba 100644
--- a/tools/testing/cxl/test/mock.h
+++ b/tools/testing/cxl/test/mock.h
@@ -20,6 +20,7 @@ struct cxl_mock_ops {
bool (*is_mock_port)(struct device *dev);
bool (*is_mock_dev)(struct device *dev);
int (*devm_cxl_port_enumerate_dports)(struct cxl_port *port);
+ int (*cxl_port_update_total_dports)(struct cxl_port *port);
struct cxl_hdm *(*devm_cxl_setup_hdm)(
struct cxl_port *port, struct cxl_endpoint_dvsec_info *info);
int (*devm_cxl_add_passthrough_decoder)(struct cxl_port *port);
--
2.50.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v5 06/10] cxl/test: Add mock version of devm_cxl_add_dport_by_dev()
2025-07-03 23:27 [PATCH v5 00/10] cxl: Delay HB port and switch dport probing until endpoint dev probe Dave Jiang
` (4 preceding siblings ...)
2025-07-03 23:27 ` [PATCH v5 05/10] cxl/test: Add cxl_test support for cxl_port_update_total_ports() Dave Jiang
@ 2025-07-03 23:27 ` Dave Jiang
2025-07-04 9:57 ` Jonathan Cameron
2025-07-03 23:27 ` [PATCH v5 07/10] cxl: Change sslbis handler to only handle single dport Dave Jiang
` (3 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: Dave Jiang @ 2025-07-03 23:27 UTC (permalink / raw)
To: linux-cxl
Cc: dave, jonathan.cameron, alison.schofield, vishal.l.verma,
ira.weiny, dan.j.williams
devm_cxl_add_dport_by_dev() outside of cxl_test is done through PCI
hierarchy. However with cxl_test, it needs to be done through the
platform device hierarchy. Add the mock function for
devm_cxl_add_dport_by_dev().
When cxl_core calls a cxl_core exported function and that function is
mocked by cxl_test, the call chain causes a circular dependency issue. Dan
provided a workaround to avoid this issue. Apply the method to changes from
the late dport allocation changes in order to enable cxl-test.
In cxl_core they are defined with "__" added in front of the function. A
macro is used to define the original function names for when non-test
version of the kernel is built. A bit of macros and typedefs are used to
allow mocking of those functions in cxl_test.
Co-developed-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v5:
- Use get_port_array() function
---
drivers/cxl/core/core.h | 2 --
drivers/cxl/core/pci.c | 7 ++++---
drivers/cxl/cxl.h | 19 +++++++++++++++++
tools/testing/cxl/Kbuild | 1 +
tools/testing/cxl/cxl_core_exports.c | 12 +++++++++++
tools/testing/cxl/exports.h | 10 +++++++++
tools/testing/cxl/test/cxl.c | 31 ++++++++++++++++++++++++++++
tools/testing/cxl/test/mock.c | 23 +++++++++++++++++++++
tools/testing/cxl/test/mock.h | 2 ++
9 files changed, 102 insertions(+), 5 deletions(-)
create mode 100644 tools/testing/cxl/exports.h
diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
index 8cfead6f3b08..29b61828a847 100644
--- a/drivers/cxl/core/core.h
+++ b/drivers/cxl/core/core.h
@@ -122,8 +122,6 @@ void cxl_ras_exit(void);
int cxl_gpf_port_setup(struct cxl_dport *dport);
int cxl_acpi_get_extended_linear_cache_size(struct resource *backing_res,
int nid, resource_size_t *size);
-struct cxl_dport *devm_cxl_add_dport_by_dev(struct cxl_port *port,
- struct device *dport_dev);
#ifdef CONFIG_CXL_FEATURES
struct cxl_feat_entry *
diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
index 336451b9144a..5594b0fcddb0 100644
--- a/drivers/cxl/core/pci.c
+++ b/drivers/cxl/core/pci.c
@@ -25,14 +25,14 @@ module_param(media_ready_timeout, ushort, 0644);
MODULE_PARM_DESC(media_ready_timeout, "seconds to wait for media ready");
/**
- * devm_cxl_add_dport_by_dev - allocate a dport by dport device
+ * __devm_cxl_add_dport_by_dev - allocate a dport by dport device
* @port: cxl_port that hosts the dport
* @dport_dev: 'struct device' of the dport
*
* Returns the allocate dport on success or ERR_PTR() of -errno on error
*/
-struct cxl_dport *devm_cxl_add_dport_by_dev(struct cxl_port *port,
- struct device *dport_dev)
+struct cxl_dport *__devm_cxl_add_dport_by_dev(struct cxl_port *port,
+ struct device *dport_dev)
{
struct cxl_register_map map;
struct pci_dev *pdev;
@@ -61,6 +61,7 @@ struct cxl_dport *devm_cxl_add_dport_by_dev(struct cxl_port *port,
port_num = FIELD_GET(PCI_EXP_LNKCAP_PN, lnkcap);
return devm_cxl_add_dport(port, &pdev->dev, port_num, map.resource);
}
+EXPORT_SYMBOL_NS_GPL(__devm_cxl_add_dport_by_dev, "CXL");
struct cxl_walk_context {
struct pci_bus *bus;
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index de7883747555..340073265657 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -907,6 +907,10 @@ void cxl_coordinates_combine(struct access_coordinate *out,
bool cxl_endpoint_decoder_reset_detected(struct cxl_port *port);
int cxl_port_update_total_dports(struct cxl_port *port);
+struct cxl_dport *devm_cxl_add_dport_by_dev(struct cxl_port *port,
+ struct device *dport_dev);
+struct cxl_dport *__devm_cxl_add_dport_by_dev(struct cxl_port *port,
+ struct device *dport_dev);
/*
* Unit test builds overrides this to __weak, find the 'strong' version
@@ -928,4 +932,19 @@ static inline struct rw_semaphore *rwsem_read_intr_acquire(struct rw_semaphore *
DEFINE_FREE(rwsem_read_release, struct rw_semaphore *, if (_T) up_read(_T))
+/*
+ * Declaration for functions that are mocked by cxl_test that are called by
+ * cxl_core. The respective functions are defined as __foo() and called by
+ * cxl_core as foo(). The macros below ensures that those functions would
+ * exist as foo(). See tools/testing/cxl/cxl_core_exports.c and
+ * tools/testing/cxl/exports.h for setting up the mock functions. The dance
+ * is done to avoid a circular dependency where cxl_core calls a function that
+ * ends up being a mock function and goes to * cxl_test where it calls a
+ * cxl_core function.
+ */
+#ifndef CXL_TEST_ENABLE
+#define DECLARE_TESTABLE(x) __##x
+#define devm_cxl_add_dport_by_dev DECLARE_TESTABLE(devm_cxl_add_dport_by_dev)
+#endif
+
#endif /* __CXL_H__ */
diff --git a/tools/testing/cxl/Kbuild b/tools/testing/cxl/Kbuild
index b9fbaaca999e..d8c5f7df4866 100644
--- a/tools/testing/cxl/Kbuild
+++ b/tools/testing/cxl/Kbuild
@@ -22,6 +22,7 @@ CXL_SRC := $(DRIVERS)/cxl
CXL_CORE_SRC := $(DRIVERS)/cxl/core
ccflags-y := -I$(srctree)/drivers/cxl/
ccflags-y += -D__mock=__weak
+ccflags-y += -DCXL_TEST_ENABLE=1
ccflags-y += -DTRACE_INCLUDE_PATH=$(CXL_CORE_SRC) -I$(srctree)/drivers/cxl/core/
obj-m += cxl_acpi.o
diff --git a/tools/testing/cxl/cxl_core_exports.c b/tools/testing/cxl/cxl_core_exports.c
index f088792a8925..0d18abc1f5a3 100644
--- a/tools/testing/cxl/cxl_core_exports.c
+++ b/tools/testing/cxl/cxl_core_exports.c
@@ -2,6 +2,18 @@
/* Copyright(c) 2022 Intel Corporation. All rights reserved. */
#include "cxl.h"
+#include "exports.h"
/* Exporting of cxl_core symbols that are only used by cxl_test */
EXPORT_SYMBOL_NS_GPL(cxl_num_decoders_committed, "CXL");
+
+cxl_add_dport_by_dev_fn _devm_cxl_add_dport_by_dev =
+ __devm_cxl_add_dport_by_dev;
+EXPORT_SYMBOL_NS_GPL(_devm_cxl_add_dport_by_dev, "CXL");
+
+struct cxl_dport *devm_cxl_add_dport_by_dev(struct cxl_port *port,
+ struct device *dport_dev)
+{
+ return _devm_cxl_add_dport_by_dev(port, dport_dev);
+}
+EXPORT_SYMBOL_NS_GPL(devm_cxl_add_dport_by_dev, "CXL");
diff --git a/tools/testing/cxl/exports.h b/tools/testing/cxl/exports.h
new file mode 100644
index 000000000000..9261ce6f1197
--- /dev/null
+++ b/tools/testing/cxl/exports.h
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright(c) 2025 Intel Corporation */
+#ifndef __MOCK_CXL_EXPORTS_H_
+#define __MOCK_CXL_EXPORTS_H_
+
+typedef struct cxl_dport *(*cxl_add_dport_by_dev_fn)(struct cxl_port *port,
+ struct device *dport_dev);
+extern cxl_add_dport_by_dev_fn _devm_cxl_add_dport_by_dev;
+
+#endif
diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
index 6fd1eaca6cbe..3e9c877fcff2 100644
--- a/tools/testing/cxl/test/cxl.c
+++ b/tools/testing/cxl/test/cxl.c
@@ -1026,6 +1026,36 @@ static int mock_cxl_port_enumerate_dports(struct cxl_port *port)
return 0;
}
+static struct cxl_dport *mock_cxl_add_dport_by_dev(struct cxl_port *port,
+ struct device *dport_dev)
+{
+ struct platform_device **array;
+ int rc, i, array_size;
+
+ rc = get_port_array(port, &array, &array_size);
+ if (rc)
+ return ERR_PTR(rc);
+
+ for (i = 0; i < array_size; i++) {
+ struct platform_device *pdev = array[i];
+
+ if (pdev->dev.parent != port->uport_dev) {
+ dev_dbg(&port->dev, "%s: mismatch parent %s\n",
+ dev_name(port->uport_dev),
+ dev_name(pdev->dev.parent));
+ continue;
+ }
+
+ if (&pdev->dev != dport_dev)
+ continue;
+
+ return devm_cxl_add_dport(port, &pdev->dev, pdev->id,
+ CXL_RESOURCE_NONE);
+ }
+
+ return ERR_PTR(-ENODEV);
+}
+
/*
* Faking the cxl_dpa_perf for the memdev when appropriate.
*/
@@ -1088,6 +1118,7 @@ static struct cxl_mock_ops cxl_mock_ops = {
.devm_cxl_add_passthrough_decoder = mock_cxl_add_passthrough_decoder,
.devm_cxl_enumerate_decoders = mock_cxl_enumerate_decoders,
.cxl_endpoint_parse_cdat = mock_cxl_endpoint_parse_cdat,
+ .devm_cxl_add_dport_by_dev = mock_cxl_add_dport_by_dev,
.list = LIST_HEAD_INIT(cxl_mock_ops.list),
};
diff --git a/tools/testing/cxl/test/mock.c b/tools/testing/cxl/test/mock.c
index 6272c0691275..3c7190b7d5a4 100644
--- a/tools/testing/cxl/test/mock.c
+++ b/tools/testing/cxl/test/mock.c
@@ -10,12 +10,18 @@
#include <cxlmem.h>
#include <cxlpci.h>
#include "mock.h"
+#include "../exports.h"
static LIST_HEAD(mock);
+static struct cxl_dport *
+redirect_devm_cxl_add_dport_by_dev(struct cxl_port *port,
+ struct device *dport_dev);
+
void register_cxl_mock_ops(struct cxl_mock_ops *ops)
{
list_add_rcu(&ops->list, &mock);
+ _devm_cxl_add_dport_by_dev = redirect_devm_cxl_add_dport_by_dev;
}
EXPORT_SYMBOL_GPL(register_cxl_mock_ops);
@@ -23,6 +29,7 @@ DEFINE_STATIC_SRCU(cxl_mock_srcu);
void unregister_cxl_mock_ops(struct cxl_mock_ops *ops)
{
+ _devm_cxl_add_dport_by_dev = __devm_cxl_add_dport_by_dev;
list_del_rcu(&ops->list);
synchronize_srcu(&cxl_mock_srcu);
}
@@ -326,6 +333,22 @@ void __wrap_cxl_dport_init_ras_reporting(struct cxl_dport *dport, struct device
}
EXPORT_SYMBOL_NS_GPL(__wrap_cxl_dport_init_ras_reporting, "CXL");
+struct cxl_dport *redirect_devm_cxl_add_dport_by_dev(struct cxl_port *port,
+ struct device *dport_dev)
+{
+ int index;
+ struct cxl_mock_ops *ops = get_cxl_mock_ops(&index);
+ struct cxl_dport *dport;
+
+ if (ops && ops->is_mock_port(port->uport_dev))
+ dport = ops->devm_cxl_add_dport_by_dev(port, dport_dev);
+ else
+ dport = devm_cxl_add_dport_by_dev(port, dport_dev);
+ put_cxl_mock_ops(index);
+
+ return dport;
+}
+
MODULE_LICENSE("GPL v2");
MODULE_DESCRIPTION("cxl_test: emulation module");
MODULE_IMPORT_NS("ACPI");
diff --git a/tools/testing/cxl/test/mock.h b/tools/testing/cxl/test/mock.h
index bf2a3e5581ba..0a440b9f544c 100644
--- a/tools/testing/cxl/test/mock.h
+++ b/tools/testing/cxl/test/mock.h
@@ -27,6 +27,8 @@ struct cxl_mock_ops {
int (*devm_cxl_enumerate_decoders)(
struct cxl_hdm *hdm, struct cxl_endpoint_dvsec_info *info);
void (*cxl_endpoint_parse_cdat)(struct cxl_port *port);
+ struct cxl_dport *(*devm_cxl_add_dport_by_dev)(
+ struct cxl_port *port, struct device *dport_dev);
};
void register_cxl_mock_ops(struct cxl_mock_ops *ops);
--
2.50.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v5 06/10] cxl/test: Add mock version of devm_cxl_add_dport_by_dev()
2025-07-03 23:27 ` [PATCH v5 06/10] cxl/test: Add mock version of devm_cxl_add_dport_by_dev() Dave Jiang
@ 2025-07-04 9:57 ` Jonathan Cameron
0 siblings, 0 replies; 20+ messages in thread
From: Jonathan Cameron @ 2025-07-04 9:57 UTC (permalink / raw)
To: Dave Jiang
Cc: linux-cxl, dave, alison.schofield, vishal.l.verma, ira.weiny,
dan.j.williams
On Thu, 3 Jul 2025 16:27:06 -0700
Dave Jiang <dave.jiang@intel.com> wrote:
> devm_cxl_add_dport_by_dev() outside of cxl_test is done through PCI
> hierarchy. However with cxl_test, it needs to be done through the
> platform device hierarchy. Add the mock function for
> devm_cxl_add_dport_by_dev().
>
> When cxl_core calls a cxl_core exported function and that function is
> mocked by cxl_test, the call chain causes a circular dependency issue. Dan
> provided a workaround to avoid this issue. Apply the method to changes from
> the late dport allocation changes in order to enable cxl-test.
>
> In cxl_core they are defined with "__" added in front of the function. A
> macro is used to define the original function names for when non-test
> version of the kernel is built. A bit of macros and typedefs are used to
> allow mocking of those functions in cxl_test.
>
> Co-developed-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
I'm never totally confident on cxl_test stuff but fwiw looks find to me.
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v5 07/10] cxl: Change sslbis handler to only handle single dport
2025-07-03 23:27 [PATCH v5 00/10] cxl: Delay HB port and switch dport probing until endpoint dev probe Dave Jiang
` (5 preceding siblings ...)
2025-07-03 23:27 ` [PATCH v5 06/10] cxl/test: Add mock version of devm_cxl_add_dport_by_dev() Dave Jiang
@ 2025-07-03 23:27 ` Dave Jiang
2025-07-03 23:27 ` [PATCH v5 08/10] cxl: Create an xarray to tie a host bridge to the cxl_root Dave Jiang
` (2 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: Dave Jiang @ 2025-07-03 23:27 UTC (permalink / raw)
To: linux-cxl
Cc: dave, jonathan.cameron, alison.schofield, vishal.l.verma,
ira.weiny, dan.j.williams, Gregory Price, Jonathan Cameron,
Li Ming
While cxl_switch_parse_cdat() is harmless to be run multiple times, it is
not efficient in the current scheme where one dport is being updated at
a time by the memdev probe path. Change the input parameter to the
specific dport being updated to pick up the SSLBIS information for just
that dport.
Reviewed-by: Gregory Price <gourry@gourry.net>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Li Ming <ming.li@zohomail.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
drivers/cxl/core/cdat.c | 23 ++++++++++-------------
drivers/cxl/core/port.c | 2 +-
drivers/cxl/cxl.h | 2 +-
3 files changed, 12 insertions(+), 15 deletions(-)
diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c
index 0ccef2f2a26a..58c21a2f22c1 100644
--- a/drivers/cxl/core/cdat.c
+++ b/drivers/cxl/core/cdat.c
@@ -440,8 +440,8 @@ static int cdat_sslbis_handler(union acpi_subtable_headers *header, void *arg,
} *tbl = (struct acpi_cdat_sslbis_table *)header;
int size = sizeof(header->cdat) + sizeof(tbl->sslbis_header);
struct acpi_cdat_sslbis *sslbis;
- struct cxl_port *port = arg;
- struct device *dev = &port->dev;
+ struct cxl_dport *dport = arg;
+ struct device *dev = &dport->port->dev;
int remain, entries, i;
u16 len;
@@ -467,8 +467,6 @@ static int cdat_sslbis_handler(union acpi_subtable_headers *header, void *arg,
u16 y = le16_to_cpu((__force __le16)tbl->entries[i].porty_id);
__le64 le_base;
__le16 le_val;
- struct cxl_dport *dport;
- unsigned long index;
u16 dsp_id;
u64 val;
@@ -499,28 +497,27 @@ static int cdat_sslbis_handler(union acpi_subtable_headers *header, void *arg,
val = cdat_normalize(le16_to_cpu(le_val), le64_to_cpu(le_base),
sslbis->data_type);
- xa_for_each(&port->dports, index, dport) {
- if (dsp_id == ACPI_CDAT_SSLBIS_ANY_PORT ||
- dsp_id == dport->port_id) {
- cxl_access_coordinate_set(dport->coord,
- sslbis->data_type,
- val);
- }
+ if (dsp_id == ACPI_CDAT_SSLBIS_ANY_PORT ||
+ dsp_id == dport->port_id) {
+ cxl_access_coordinate_set(dport->coord,
+ sslbis->data_type, val);
+ return 0;
}
}
return 0;
}
-void cxl_switch_parse_cdat(struct cxl_port *port)
+void cxl_switch_parse_cdat(struct cxl_dport *dport)
{
+ struct cxl_port *port = dport->port;
int rc;
if (!port->cdat.table)
return;
rc = cdat_table_parse(ACPI_CDAT_TYPE_SSLBIS, cdat_sslbis_handler,
- port, port->cdat.table, port->cdat.length);
+ dport, port->cdat.table, port->cdat.length);
rc = cdat_table_parse_output(rc);
if (rc)
dev_dbg(&port->dev, "Failed to parse SSLBIS: %d\n", rc);
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index ffd39c5b7222..1e57e7e9f0cc 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -1614,7 +1614,7 @@ static int cxl_port_setup_with_dport(struct cxl_port *port,
{
device_lock_assert(&port->dev);
- cxl_switch_parse_cdat(port);
+ cxl_switch_parse_cdat(dport);
return update_decoders_with_dport(port, dport);
}
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 340073265657..cc5e0858824b 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -891,7 +891,7 @@ static inline u64 cxl_port_get_spa_cache_alias(struct cxl_port *endpoint,
#endif
void cxl_endpoint_parse_cdat(struct cxl_port *port);
-void cxl_switch_parse_cdat(struct cxl_port *port);
+void cxl_switch_parse_cdat(struct cxl_dport *dport);
int cxl_endpoint_get_perf_coordinates(struct cxl_port *port,
struct access_coordinate *coord);
--
2.50.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v5 08/10] cxl: Create an xarray to tie a host bridge to the cxl_root
2025-07-03 23:27 [PATCH v5 00/10] cxl: Delay HB port and switch dport probing until endpoint dev probe Dave Jiang
` (6 preceding siblings ...)
2025-07-03 23:27 ` [PATCH v5 07/10] cxl: Change sslbis handler to only handle single dport Dave Jiang
@ 2025-07-03 23:27 ` Dave Jiang
2025-07-03 23:27 ` [PATCH v5 09/10] cxl: Move enumeration of hostbridge ports to the memdev probe path Dave Jiang
2025-07-03 23:27 ` [PATCH v5 10/10] cxl: Remove devm_cxl_port_enumerate_dports() that is no longer used Dave Jiang
9 siblings, 0 replies; 20+ messages in thread
From: Dave Jiang @ 2025-07-03 23:27 UTC (permalink / raw)
To: linux-cxl
Cc: dave, jonathan.cameron, alison.schofield, vishal.l.verma,
ira.weiny, dan.j.williams
Add helper functions to setup association of a host bridge device to a
related cxl_root. Functions are in preparation to support the moving
of host bridge ports creation from cxl_acpi to cxl_memdev probe path.
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
drivers/cxl/core/port.c | 53 +++++++++++++++++++++++++++++++++++++++++
drivers/cxl/cxl.h | 4 ++++
2 files changed, 57 insertions(+)
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 1e57e7e9f0cc..1089fce24293 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -38,6 +38,7 @@ DECLARE_RWSEM(cxl_region_rwsem);
static DEFINE_IDA(cxl_port_ida);
static DEFINE_XARRAY(cxl_root_buses);
+static DEFINE_XARRAY(cxl_root_ports);
/*
* The terminal device in PCI is NULL and @platform_bus
@@ -1014,6 +1015,58 @@ int devm_cxl_register_pci_bus(struct device *host, struct device *uport_dev,
}
EXPORT_SYMBOL_NS_GPL(devm_cxl_register_pci_bus, "CXL");
+/**
+ * cxl_hb_uport_dev_to_root - Retrieve cxl_root tied to the host bridge device
+ * @hb_uport_dev: host bridge upstream port device
+ *
+ * Return cxl_root on success or NULL on failure
+ *
+ * A reference is taken on the port device. Caller needs to call put_device()
+ * when done.
+ */
+struct cxl_root *cxl_hb_uport_dev_to_root(struct device *hb_uport_dev)
+{
+ struct cxl_root *root;
+
+ root = xa_load(&cxl_root_ports, (unsigned long)hb_uport_dev);
+ if (!root)
+ return NULL;
+
+ get_device(&root->port.dev);
+
+ return root;
+}
+EXPORT_SYMBOL_NS_GPL(cxl_hb_uport_dev_to_root, "CXL");
+
+static void unregister_hb_uport_root_ports(void *hb_uport_dev)
+{
+ xa_erase(&cxl_root_ports, (unsigned long)hb_uport_dev);
+}
+
+/**
+ * devm_cxl_register_hb_uport_root_port - Tie a hostbridge device to a root port
+ * @host: device that hosts the memory for the xarray entries
+ * @hb_uport_dev: host bridge device that serves as the xarray index
+ * @root: cxl_root that serves as the xarray entry data
+ *
+ * Return 0 on success or -errno on failure.
+ */
+int devm_cxl_register_hb_uport_root_port(struct device *host,
+ struct device *hb_uport_dev,
+ struct cxl_root *root)
+{
+ int rc;
+
+ rc = xa_insert(&cxl_root_ports, (unsigned long)hb_uport_dev, root,
+ GFP_KERNEL);
+ if (rc)
+ return rc;
+
+ return devm_add_action_or_reset(host, unregister_hb_uport_root_ports,
+ hb_uport_dev);
+}
+EXPORT_SYMBOL_NS_GPL(devm_cxl_register_hb_uport_root_port, "CXL");
+
static bool dev_is_cxl_root_child(struct device *dev)
{
struct cxl_port *port, *parent;
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index cc5e0858824b..f88a7ab8d94b 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -734,6 +734,10 @@ struct pci_bus;
int devm_cxl_register_pci_bus(struct device *host, struct device *uport_dev,
struct pci_bus *bus);
struct pci_bus *cxl_port_to_pci_bus(struct cxl_port *port);
+int devm_cxl_register_hb_uport_root_port(struct device *host,
+ struct device *hb_uport_dev,
+ struct cxl_root *root);
+struct cxl_root *cxl_hb_uport_dev_to_root(struct device *uport_dev);
struct cxl_port *devm_cxl_add_port(struct device *host,
struct device *uport_dev,
resource_size_t component_reg_phys,
--
2.50.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v5 09/10] cxl: Move enumeration of hostbridge ports to the memdev probe path
2025-07-03 23:27 [PATCH v5 00/10] cxl: Delay HB port and switch dport probing until endpoint dev probe Dave Jiang
` (7 preceding siblings ...)
2025-07-03 23:27 ` [PATCH v5 08/10] cxl: Create an xarray to tie a host bridge to the cxl_root Dave Jiang
@ 2025-07-03 23:27 ` Dave Jiang
2025-07-04 10:07 ` Jonathan Cameron
2025-07-03 23:27 ` [PATCH v5 10/10] cxl: Remove devm_cxl_port_enumerate_dports() that is no longer used Dave Jiang
9 siblings, 1 reply; 20+ messages in thread
From: Dave Jiang @ 2025-07-03 23:27 UTC (permalink / raw)
To: linux-cxl
Cc: dave, jonathan.cameron, alison.schofield, vishal.l.verma,
ira.weiny, dan.j.williams
Current enuemration scheme in cxl_acpi module creates the ports under the
root port by enumerating the hostbridges after the dports under the root
port is created. However error messages "cxl portN: Couldn't locate the
CXL.cache and CXL.mem capability array header" is observed when certain
platform has PCIe hotplug option turned on in BIOS. If the cxl_acpi module
probe is running before the CXL link between the endpoint device and the
RP is established, then the platform may not have exposed DVSEC ID 3 and/or
DVSEC ID 7 blocks which will trigger the error message. This behavior
is defined by the spec and not a hardware quirk.
Setup an association in cxl_port to tie the host bridge device to the
associated cxl_root. The cxl_root provides a callback that's setup
by the cxl_acpi probe function in order to create a port per host bridge
that was previously done during cxl_acpi probe. Add the calling of the
callback in devm_cxl_enumerate_ports(). The observed behavior is that
ports that are not connected to endpoint device(s) are no longer
enumerated. This should also remove any excessive noise of port probe
failing on those inactive ports.
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v5:
- Updated with new devm_cxl_port_add_dport() and friends.
---
drivers/cxl/acpi.c | 137 ++++++++++++++++++++++++----------------
drivers/cxl/core/port.c | 94 ++++++++++++++++++++++++---
drivers/cxl/cxl.h | 2 +
3 files changed, 170 insertions(+), 63 deletions(-)
diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
index a1a99ec3f12c..badaa99ab33a 100644
--- a/drivers/cxl/acpi.c
+++ b/drivers/cxl/acpi.c
@@ -296,8 +296,80 @@ static int cxl_acpi_qos_class(struct cxl_root *cxl_root,
return cxl_acpi_evaluate_qtg_dsm(handle, coord, entries, qos_class);
}
+/* Note, @dev is used by mock_acpi_table_parse_cedt() */
+struct cxl_chbs_context {
+ struct device *dev;
+ unsigned long long uid;
+ resource_size_t base;
+ u32 cxl_version;
+ int nr_versions;
+ u32 saved_version;
+};
+
+static int cxl_get_chbs(struct device *dev, struct acpi_device *hb,
+ struct cxl_chbs_context *ctx);
+
+/*
+ * A host bridge is a dport to a CFMWS decoder and it is a uport to the
+ * dport (PCIe Root Ports) in the host bridge.
+ */
+static int cxl_acpi_setup_hostbridge_uport(struct cxl_root *cxl_root,
+ struct device *bridge_dev)
+{
+ struct cxl_port *root_port = &cxl_root->port;
+ struct device *host = root_port->dev.parent;
+ struct acpi_device *hb = ACPI_COMPANION(bridge_dev);
+ resource_size_t component_reg_phys;
+ struct acpi_pci_root *pci_root;
+ struct cxl_chbs_context ctx;
+ struct cxl_dport *dport;
+ struct cxl_port *port;
+ int rc;
+
+ device_lock_assert(&cxl_root->port.dev);
+ pci_root = acpi_pci_find_root(hb->handle);
+ dport = cxl_find_dport_by_dev(root_port, bridge_dev);
+ if (!dport) {
+ dev_dbg(host, "Host bridge expected and not found\n");
+ return -ENODEV;
+ }
+
+ if (dport->rch) {
+ dev_info(bridge_dev, "host supports CXL (restricted)\n");
+ return 0;
+ }
+
+ rc = cxl_get_chbs(&hb->dev, hb, &ctx);
+ if (rc)
+ return rc;
+
+ if (ctx.cxl_version == ACPI_CEDT_CHBS_VERSION_CXL11) {
+ dev_warn(bridge_dev,
+ "CXL CHBS version mismatch, skip port registration\n");
+ return 0;
+ }
+
+ component_reg_phys = ctx.base;
+ if (component_reg_phys != CXL_RESOURCE_NONE)
+ dev_dbg(&hb->dev, "CHBRC found for UID %lld: %pa\n",
+ ctx.uid, &component_reg_phys);
+
+ rc = devm_cxl_register_pci_bus(host, bridge_dev, pci_root->bus);
+ if (rc && rc != -EBUSY)
+ return rc;
+
+ port = devm_cxl_add_port(host, bridge_dev, component_reg_phys, dport);
+ if (IS_ERR(port))
+ return PTR_ERR(port);
+
+ dev_info(bridge_dev, "host supports CXL\n");
+
+ return 0;
+}
+
static const struct cxl_root_ops acpi_root_ops = {
.qos_class = cxl_acpi_qos_class,
+ .setup_hostbridge_uport = cxl_acpi_setup_hostbridge_uport,
};
static void del_cxl_resource(struct resource *res)
@@ -466,16 +538,6 @@ __mock struct acpi_device *to_cxl_host_bridge(struct device *host,
return NULL;
}
-/* Note, @dev is used by mock_acpi_table_parse_cedt() */
-struct cxl_chbs_context {
- struct device *dev;
- unsigned long long uid;
- resource_size_t base;
- u32 cxl_version;
- int nr_versions;
- u32 saved_version;
-};
-
static int cxl_get_chbs_iter(union acpi_subtable_headers *header, void *arg,
const unsigned long end)
{
@@ -598,7 +660,7 @@ static int add_host_bridge_dport(struct device *match, void *arg)
/*
* In RCH mode, bind the component regs base to the dport. In
* VH mode it will be bound to the CXL host bridge's port
- * object later in add_host_bridge_uport().
+ * object later in cxl_acpi_setup_hostbridge_uport().
*/
if (ctx.cxl_version == ACPI_CEDT_CHBS_VERSION_CXL11) {
dev_dbg(match, "RCRB found for UID %lld: %pa\n", ctx.uid,
@@ -620,22 +682,14 @@ static int add_host_bridge_dport(struct device *match, void *arg)
return 0;
}
-/*
- * A host bridge is a dport to a CFMWS decode and it is a uport to the
- * dport (PCIe Root Ports) in the host bridge.
- */
-static int add_host_bridge_uport(struct device *match, void *arg)
+static int set_cxl_root_to_hostbridge(struct device *match, void *arg)
{
struct cxl_port *root_port = arg;
struct device *host = root_port->dev.parent;
struct acpi_device *hb = to_cxl_host_bridge(host, match);
- struct acpi_pci_root *pci_root;
struct cxl_dport *dport;
- struct cxl_port *port;
+ struct acpi_pci_root *pci_root;
struct device *bridge;
- struct cxl_chbs_context ctx;
- resource_size_t component_reg_phys;
- int rc;
if (!hb)
return 0;
@@ -648,37 +702,12 @@ static int add_host_bridge_uport(struct device *match, void *arg)
return 0;
}
- if (dport->rch) {
- dev_info(bridge, "host supports CXL (restricted)\n");
+ if (dport->rch)
return 0;
- }
- rc = cxl_get_chbs(match, hb, &ctx);
- if (rc)
- return rc;
-
- if (ctx.cxl_version == ACPI_CEDT_CHBS_VERSION_CXL11) {
- dev_warn(bridge,
- "CXL CHBS version mismatch, skip port registration\n");
- return 0;
- }
-
- component_reg_phys = ctx.base;
- if (component_reg_phys != CXL_RESOURCE_NONE)
- dev_dbg(match, "CHBCR found for UID %lld: %pa\n",
- ctx.uid, &component_reg_phys);
-
- rc = devm_cxl_register_pci_bus(host, bridge, pci_root->bus);
- if (rc)
- return rc;
-
- port = devm_cxl_add_port(host, bridge, component_reg_phys, dport);
- if (IS_ERR(port))
- return PTR_ERR(port);
-
- dev_info(bridge, "host supports CXL\n");
-
- return 0;
+ bridge = pci_root->bus->bridge;
+ return devm_cxl_register_hb_uport_root_port(host, bridge,
+ to_cxl_root(root_port));
}
static int add_root_nvdimm_bridge(struct device *match, void *data)
@@ -852,6 +881,7 @@ static int cxl_acpi_probe(struct platform_device *pdev)
return PTR_ERR(cxl_root);
root_port = &cxl_root->port;
+ /* Root level scanned with host-bridge as dports */
rc = bus_for_each_dev(adev->dev.bus, NULL, root_port,
add_host_bridge_dport);
if (rc < 0)
@@ -880,12 +910,9 @@ static int cxl_acpi_probe(struct platform_device *pdev)
*/
device_for_each_child(&root_port->dev, cxl_res, pair_cxl_resource);
- /*
- * Root level scanned with host-bridge as dports, now scan host-bridges
- * for their role as CXL uports to their CXL-capable PCIe Root Ports.
- */
+ /* Scan host-bridges and point the cxl root port struct to it */
rc = bus_for_each_dev(adev->dev.bus, NULL, root_port,
- add_host_bridge_uport);
+ set_cxl_root_to_hostbridge);
if (rc < 0)
return rc;
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 1089fce24293..c624f4801b68 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -1789,14 +1789,8 @@ static int add_port_attach_ep(struct cxl_memdev *cxlmd,
guard(device)(&port->dev);
new_dport = devm_cxl_port_add_dport(port, dport_dev);
- if (IS_ERR(new_dport)) {
- if (PTR_ERR(new_dport) != -EEXIST)
- return PTR_ERR(new_dport);
-
- new_dport = cxl_find_dport_by_dev(port, dport_dev);
- if (!new_dport)
- return -ENODEV;
- }
+ if (IS_ERR(new_dport))
+ return PTR_ERR(new_dport);
dport = new_dport;
}
@@ -1817,6 +1811,86 @@ static int add_port_attach_ep(struct cxl_memdev *cxlmd,
#define CXL_ITER_LEVEL_SWITCH 1
+static int get_hostbridge_port_devices(struct cxl_memdev *cxlmd,
+ struct device **hb_uport_dev,
+ struct device **hb_dport_dev)
+{
+ struct device *dev = &cxlmd->dev;
+ struct device *iter;
+
+ for (iter = dev; iter; iter = grandparent(iter)) {
+ struct device *dport_dev = grandparent(iter);
+ struct device *uport_dev = dport_dev->parent;
+
+ if (is_cxl_hierarchy_head(uport_dev->parent)) {
+ *hb_uport_dev = uport_dev;
+ *hb_dport_dev = dport_dev;
+ return 0;
+ }
+ }
+
+ return -ENODEV;
+}
+
+static int cxl_hostbridge_port_setup(struct cxl_memdev *cxlmd)
+{
+ struct device *hb_uport_dev, *hb_dport_dev;
+ struct cxl_dport *dport;
+ int rc;
+
+ rc = get_hostbridge_port_devices(cxlmd, &hb_uport_dev, &hb_dport_dev);
+ if (rc)
+ return -ENODEV;
+
+ struct cxl_root *cxl_root __free(put_cxl_root) =
+ cxl_hb_uport_dev_to_root(hb_uport_dev);
+ if (!cxl_root)
+ return -ENODEV;
+
+ guard(device)(&cxl_root->port.dev);
+ struct cxl_port *port __free(put_cxl_port) =
+ find_cxl_port(hb_dport_dev, &dport);
+ if (!port)
+ port = find_cxl_port_by_uport(hb_uport_dev);
+
+ /* Port already established, add the associated dport if needed. */
+ if (port) {
+ if (dport)
+ return 0;
+
+ guard(device)(&port->dev);
+ dport = devm_cxl_port_add_dport(port, hb_dport_dev);
+ if (IS_ERR(dport)) {
+ dev_dbg(&cxlmd->dev,
+ "failed to add dport %s to port %s: %ld\n",
+ dev_name(hb_dport_dev), dev_name(&port->dev),
+ PTR_ERR(dport));
+ return PTR_ERR(dport);
+ }
+ return 0;
+ }
+
+ /* No port found, setup a port via the root port ops */
+ if (!cxl_root->ops || !cxl_root->ops->setup_hostbridge_uport)
+ return -EOPNOTSUPP;
+
+ rc = cxl_root->ops->setup_hostbridge_uport(cxl_root, hb_uport_dev);
+ if (rc)
+ return rc;
+
+ /* Add the dport that goes with the newly created port */
+ dport = devm_cxl_add_dport_by_uport(hb_uport_dev, hb_dport_dev);
+ if (IS_ERR(dport)) {
+ dev_dbg(&cxlmd->dev,
+ "failed to add dport %s to port %s: %ld\n",
+ dev_name(hb_dport_dev), dev_name(&cxl_root->port.dev),
+ PTR_ERR(dport));
+ return PTR_ERR(dport);
+ }
+
+ return 0;
+}
+
int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
{
struct device *dev = &cxlmd->dev;
@@ -1830,6 +1904,10 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
if (cxlmd->cxlds->rcd)
return 0;
+ rc = cxl_hostbridge_port_setup(cxlmd);
+ if (rc)
+ return rc;
+
rc = devm_add_action_or_reset(&cxlmd->dev, cxl_detach_ep, cxlmd);
if (rc)
return rc;
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index f88a7ab8d94b..f9972fb4990f 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -642,6 +642,8 @@ struct cxl_root_ops {
int (*qos_class)(struct cxl_root *cxl_root,
struct access_coordinate *coord, int entries,
int *qos_class);
+ int (*setup_hostbridge_uport)(struct cxl_root *cxl_root,
+ struct device *bridge_dev);
};
static inline struct cxl_dport *
--
2.50.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v5 09/10] cxl: Move enumeration of hostbridge ports to the memdev probe path
2025-07-03 23:27 ` [PATCH v5 09/10] cxl: Move enumeration of hostbridge ports to the memdev probe path Dave Jiang
@ 2025-07-04 10:07 ` Jonathan Cameron
2025-07-07 23:39 ` Dave Jiang
0 siblings, 1 reply; 20+ messages in thread
From: Jonathan Cameron @ 2025-07-04 10:07 UTC (permalink / raw)
To: Dave Jiang
Cc: linux-cxl, dave, alison.schofield, vishal.l.verma, ira.weiny,
dan.j.williams
On Thu, 3 Jul 2025 16:27:09 -0700
Dave Jiang <dave.jiang@intel.com> wrote:
> Current enuemration scheme in cxl_acpi module creates the ports under the
> root port by enumerating the hostbridges after the dports under the root
> port is created. However error messages "cxl portN: Couldn't locate the
> CXL.cache and CXL.mem capability array header" is observed when certain
> platform has PCIe hotplug option turned on in BIOS. If the cxl_acpi module
> probe is running before the CXL link between the endpoint device and the
> RP is established, then the platform may not have exposed DVSEC ID 3 and/or
> DVSEC ID 7 blocks which will trigger the error message. This behavior
> is defined by the spec and not a hardware quirk.
>
> Setup an association in cxl_port to tie the host bridge device to the
> associated cxl_root. The cxl_root provides a callback that's setup
> by the cxl_acpi probe function in order to create a port per host bridge
> that was previously done during cxl_acpi probe. Add the calling of the
> callback in devm_cxl_enumerate_ports(). The observed behavior is that
> ports that are not connected to endpoint device(s) are no longer
> enumerated. This should also remove any excessive noise of port probe
> failing on those inactive ports.
>
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
> v5:
> - Updated with new devm_cxl_port_add_dport() and friends.
Hi Dave,
One of the bits of code that I was confused by in the patch that
added devm_cxl_port_add_dport() goes away in here. I'm not
sure why.
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index 1089fce24293..c624f4801b68 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -1789,14 +1789,8 @@ static int add_port_attach_ep(struct cxl_memdev *cxlmd,
>
> guard(device)(&port->dev);
> new_dport = devm_cxl_port_add_dport(port, dport_dev);
> - if (IS_ERR(new_dport)) {
> - if (PTR_ERR(new_dport) != -EEXIST)
> - return PTR_ERR(new_dport);
Interesting. Is this in the wrong patch?
> -
> - new_dport = cxl_find_dport_by_dev(port, dport_dev);
> - if (!new_dport)
> - return -ENODEV;
> - }
> + if (IS_ERR(new_dport))
> + return PTR_ERR(new_dport);
>
> dport = new_dport;
> }
> +static int cxl_hostbridge_port_setup(struct cxl_memdev *cxlmd)
> +{
> + struct device *hb_uport_dev, *hb_dport_dev;
> + struct cxl_dport *dport;
> + int rc;
> +
> + rc = get_hostbridge_port_devices(cxlmd, &hb_uport_dev, &hb_dport_dev);
> + if (rc)
> + return -ENODEV;
> +
> + struct cxl_root *cxl_root __free(put_cxl_root) =
> + cxl_hb_uport_dev_to_root(hb_uport_dev);
> + if (!cxl_root)
> + return -ENODEV;
> +
> + guard(device)(&cxl_root->port.dev);
> + struct cxl_port *port __free(put_cxl_port) =
> + find_cxl_port(hb_dport_dev, &dport);
> + if (!port)
> + port = find_cxl_port_by_uport(hb_uport_dev);
> +
> + /* Port already established, add the associated dport if needed. */
> + if (port) {
> + if (dport)
> + return 0;
The sequence I badly explained on v4 still bothers me here as I think
it might be possible to hit that if (dport) with dport never initialized.
Even if it isn't, I'd be surprised if the compiler / static analysis can
see that there is no such path. So if we think it can't happen, just initialize
dport at declaration. If it can happen, I think it being null is still the
right answer, but maybe I'm missing something.
> +
> + guard(device)(&port->dev);
> + dport = devm_cxl_port_add_dport(port, hb_dport_dev);
> + if (IS_ERR(dport)) {
> + dev_dbg(&cxlmd->dev,
> + "failed to add dport %s to port %s: %ld\n",
> + dev_name(hb_dport_dev), dev_name(&port->dev),
> + PTR_ERR(dport));
> + return PTR_ERR(dport);
> + }
> + return 0;
> + }
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v5 09/10] cxl: Move enumeration of hostbridge ports to the memdev probe path
2025-07-04 10:07 ` Jonathan Cameron
@ 2025-07-07 23:39 ` Dave Jiang
0 siblings, 0 replies; 20+ messages in thread
From: Dave Jiang @ 2025-07-07 23:39 UTC (permalink / raw)
To: Jonathan Cameron
Cc: linux-cxl, dave, alison.schofield, vishal.l.verma, ira.weiny,
dan.j.williams
On 7/4/25 3:07 AM, Jonathan Cameron wrote:
> On Thu, 3 Jul 2025 16:27:09 -0700
> Dave Jiang <dave.jiang@intel.com> wrote:
>
>> Current enuemration scheme in cxl_acpi module creates the ports under the
>> root port by enumerating the hostbridges after the dports under the root
>> port is created. However error messages "cxl portN: Couldn't locate the
>> CXL.cache and CXL.mem capability array header" is observed when certain
>> platform has PCIe hotplug option turned on in BIOS. If the cxl_acpi module
>> probe is running before the CXL link between the endpoint device and the
>> RP is established, then the platform may not have exposed DVSEC ID 3 and/or
>> DVSEC ID 7 blocks which will trigger the error message. This behavior
>> is defined by the spec and not a hardware quirk.
>>
>> Setup an association in cxl_port to tie the host bridge device to the
>> associated cxl_root. The cxl_root provides a callback that's setup
>> by the cxl_acpi probe function in order to create a port per host bridge
>> that was previously done during cxl_acpi probe. Add the calling of the
>> callback in devm_cxl_enumerate_ports(). The observed behavior is that
>> ports that are not connected to endpoint device(s) are no longer
>> enumerated. This should also remove any excessive noise of port probe
>> failing on those inactive ports.
>>
>> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>> ---
>> v5:
>> - Updated with new devm_cxl_port_add_dport() and friends.
> Hi Dave,
>
> One of the bits of code that I was confused by in the patch that
> added devm_cxl_port_add_dport() goes away in here. I'm not
> sure why.
>
>> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
>> index 1089fce24293..c624f4801b68 100644
>> --- a/drivers/cxl/core/port.c
>> +++ b/drivers/cxl/core/port.c
>> @@ -1789,14 +1789,8 @@ static int add_port_attach_ep(struct cxl_memdev *cxlmd,
>>
>> guard(device)(&port->dev);
>> new_dport = devm_cxl_port_add_dport(port, dport_dev);
>> - if (IS_ERR(new_dport)) {
>> - if (PTR_ERR(new_dport) != -EEXIST)
>> - return PTR_ERR(new_dport);
>
> Interesting. Is this in the wrong patch?
Shouldn't have been deleted to begin with.
DJ
>
>> -
>> - new_dport = cxl_find_dport_by_dev(port, dport_dev);
>> - if (!new_dport)
>> - return -ENODEV;
>> - }
>> + if (IS_ERR(new_dport))
>> + return PTR_ERR(new_dport);
>>
>> dport = new_dport;
>> }
>
>> +static int cxl_hostbridge_port_setup(struct cxl_memdev *cxlmd)
>> +{
>> + struct device *hb_uport_dev, *hb_dport_dev;
>> + struct cxl_dport *dport;
>> + int rc;
>> +
>> + rc = get_hostbridge_port_devices(cxlmd, &hb_uport_dev, &hb_dport_dev);
>> + if (rc)
>> + return -ENODEV;
>> +
>> + struct cxl_root *cxl_root __free(put_cxl_root) =
>> + cxl_hb_uport_dev_to_root(hb_uport_dev);
>> + if (!cxl_root)
>> + return -ENODEV;
>> +
>> + guard(device)(&cxl_root->port.dev);
>> + struct cxl_port *port __free(put_cxl_port) =
>> + find_cxl_port(hb_dport_dev, &dport);
>> + if (!port)
>> + port = find_cxl_port_by_uport(hb_uport_dev);
>> +
>> + /* Port already established, add the associated dport if needed. */
>> + if (port) {
>> + if (dport)
>> + return 0;
>
> The sequence I badly explained on v4 still bothers me here as I think
> it might be possible to hit that if (dport) with dport never initialized.
> Even if it isn't, I'd be surprised if the compiler / static analysis can
> see that there is no such path. So if we think it can't happen, just initialize
> dport at declaration. If it can happen, I think it being null is still the
> right answer, but maybe I'm missing something.
>
>
>> +
>> + guard(device)(&port->dev);
>> + dport = devm_cxl_port_add_dport(port, hb_dport_dev);
>> + if (IS_ERR(dport)) {
>> + dev_dbg(&cxlmd->dev,
>> + "failed to add dport %s to port %s: %ld\n",
>> + dev_name(hb_dport_dev), dev_name(&port->dev),
>> + PTR_ERR(dport));
>> + return PTR_ERR(dport);
>> + }
>> + return 0;
>> + }
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v5 10/10] cxl: Remove devm_cxl_port_enumerate_dports() that is no longer used
2025-07-03 23:27 [PATCH v5 00/10] cxl: Delay HB port and switch dport probing until endpoint dev probe Dave Jiang
` (8 preceding siblings ...)
2025-07-03 23:27 ` [PATCH v5 09/10] cxl: Move enumeration of hostbridge ports to the memdev probe path Dave Jiang
@ 2025-07-03 23:27 ` Dave Jiang
2025-07-04 10:09 ` Jonathan Cameron
9 siblings, 1 reply; 20+ messages in thread
From: Dave Jiang @ 2025-07-03 23:27 UTC (permalink / raw)
To: linux-cxl
Cc: dave, jonathan.cameron, alison.schofield, vishal.l.verma,
ira.weiny, dan.j.williams
Delete function devm_cxl_port_enumerate_dports() which is no longer being
used anywhere. Also remove cxl_test support.
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
drivers/cxl/core/pci.c | 87 ++++-------------------------------
drivers/cxl/cxlpci.h | 1 -
tools/testing/cxl/Kbuild | 1 -
tools/testing/cxl/test/cxl.c | 31 -------------
tools/testing/cxl/test/mock.c | 15 ------
tools/testing/cxl/test/mock.h | 1 -
6 files changed, 8 insertions(+), 128 deletions(-)
diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
index 5594b0fcddb0..717e25131db7 100644
--- a/drivers/cxl/core/pci.c
+++ b/drivers/cxl/core/pci.c
@@ -63,85 +63,6 @@ struct cxl_dport *__devm_cxl_add_dport_by_dev(struct cxl_port *port,
}
EXPORT_SYMBOL_NS_GPL(__devm_cxl_add_dport_by_dev, "CXL");
-struct cxl_walk_context {
- struct pci_bus *bus;
- struct cxl_port *port;
- int type;
- int error;
- int count;
-};
-
-static int match_add_dports(struct pci_dev *pdev, void *data)
-{
- struct cxl_walk_context *ctx = data;
- struct cxl_port *port = ctx->port;
- int type = pci_pcie_type(pdev);
- struct cxl_register_map map;
- struct cxl_dport *dport;
- u32 lnkcap, port_num;
- int rc;
-
- if (pdev->bus != ctx->bus)
- return 0;
- if (!pci_is_pcie(pdev))
- return 0;
- if (type != ctx->type)
- return 0;
- if (pci_read_config_dword(pdev, pci_pcie_cap(pdev) + PCI_EXP_LNKCAP,
- &lnkcap))
- return 0;
-
- rc = cxl_find_regblock(pdev, CXL_REGLOC_RBI_COMPONENT, &map);
- if (rc)
- dev_dbg(&port->dev, "failed to find component registers\n");
-
- port_num = FIELD_GET(PCI_EXP_LNKCAP_PN, lnkcap);
- dport = devm_cxl_add_dport(port, &pdev->dev, port_num, map.resource);
- if (IS_ERR(dport)) {
- ctx->error = PTR_ERR(dport);
- return PTR_ERR(dport);
- }
- ctx->count++;
-
- return 0;
-}
-
-/**
- * devm_cxl_port_enumerate_dports - enumerate downstream ports of the upstream port
- * @port: cxl_port whose ->uport_dev is the upstream of dports to be enumerated
- *
- * Returns a positive number of dports enumerated or a negative error
- * code.
- */
-int devm_cxl_port_enumerate_dports(struct cxl_port *port)
-{
- struct pci_bus *bus = cxl_port_to_pci_bus(port);
- struct cxl_walk_context ctx;
- int type;
-
- if (!bus)
- return -ENXIO;
-
- if (pci_is_root_bus(bus))
- type = PCI_EXP_TYPE_ROOT_PORT;
- else
- type = PCI_EXP_TYPE_DOWNSTREAM;
-
- ctx = (struct cxl_walk_context) {
- .port = port,
- .bus = bus,
- .type = type,
- };
- pci_walk_bus(bus, match_add_dports, &ctx);
-
- if (ctx.count == 0)
- return -ENODEV;
- if (ctx.error)
- return ctx.error;
- return ctx.count;
-}
-EXPORT_SYMBOL_NS_GPL(devm_cxl_port_enumerate_dports, "CXL");
-
static int cxl_dvsec_mem_range_valid(struct cxl_dev_state *cxlds, int id)
{
struct pci_dev *pdev = to_pci_dev(cxlds->dev);
@@ -1209,6 +1130,14 @@ int cxl_gpf_port_setup(struct cxl_dport *dport)
return 0;
}
+struct cxl_walk_context {
+ struct pci_bus *bus;
+ struct cxl_port *port;
+ int type;
+ int error;
+ int count;
+};
+
static int match_dport(struct pci_dev *pdev, void *data)
{
struct cxl_walk_context *ctx = data;
diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h
index 54e219b0049e..7827a183128d 100644
--- a/drivers/cxl/cxlpci.h
+++ b/drivers/cxl/cxlpci.h
@@ -127,7 +127,6 @@ static inline bool cxl_pci_flit_256(struct pci_dev *pdev)
return lnksta2 & PCI_EXP_LNKSTA2_FLIT;
}
-int devm_cxl_port_enumerate_dports(struct cxl_port *port);
struct cxl_dev_state;
int cxl_hdm_decode_init(struct cxl_dev_state *cxlds, struct cxl_hdm *cxlhdm,
struct cxl_endpoint_dvsec_info *info);
diff --git a/tools/testing/cxl/Kbuild b/tools/testing/cxl/Kbuild
index d8c5f7df4866..4e3597d4ee9c 100644
--- a/tools/testing/cxl/Kbuild
+++ b/tools/testing/cxl/Kbuild
@@ -4,7 +4,6 @@ ldflags-y += --wrap=is_acpi_device_node
ldflags-y += --wrap=acpi_evaluate_integer
ldflags-y += --wrap=acpi_pci_find_root
ldflags-y += --wrap=nvdimm_bus_register
-ldflags-y += --wrap=devm_cxl_port_enumerate_dports
ldflags-y += --wrap=cxl_port_update_total_dports
ldflags-y += --wrap=devm_cxl_setup_hdm
ldflags-y += --wrap=devm_cxl_add_passthrough_decoder
diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
index 3e9c877fcff2..d52ae0be4941 100644
--- a/tools/testing/cxl/test/cxl.c
+++ b/tools/testing/cxl/test/cxl.c
@@ -996,36 +996,6 @@ static int mock_cxl_port_update_total_dports(struct cxl_port *port)
return 0;
}
-static int mock_cxl_port_enumerate_dports(struct cxl_port *port)
-{
- struct platform_device **array;
- int rc, i, array_size;
-
- rc = get_port_array(port, &array, &array_size);
- if (rc)
- return rc;
-
- for (i = 0; i < array_size; i++) {
- struct platform_device *pdev = array[i];
- struct cxl_dport *dport;
-
- if (pdev->dev.parent != port->uport_dev) {
- dev_dbg(&port->dev, "%s: mismatch parent %s\n",
- dev_name(port->uport_dev),
- dev_name(pdev->dev.parent));
- continue;
- }
-
- dport = devm_cxl_add_dport(port, &pdev->dev, pdev->id,
- CXL_RESOURCE_NONE);
-
- if (IS_ERR(dport))
- return PTR_ERR(dport);
- }
-
- return 0;
-}
-
static struct cxl_dport *mock_cxl_add_dport_by_dev(struct cxl_port *port,
struct device *dport_dev)
{
@@ -1112,7 +1082,6 @@ static struct cxl_mock_ops cxl_mock_ops = {
.acpi_table_parse_cedt = mock_acpi_table_parse_cedt,
.acpi_evaluate_integer = mock_acpi_evaluate_integer,
.acpi_pci_find_root = mock_acpi_pci_find_root,
- .devm_cxl_port_enumerate_dports = mock_cxl_port_enumerate_dports,
.cxl_port_update_total_dports = mock_cxl_port_update_total_dports,
.devm_cxl_setup_hdm = mock_cxl_setup_hdm,
.devm_cxl_add_passthrough_decoder = mock_cxl_add_passthrough_decoder,
diff --git a/tools/testing/cxl/test/mock.c b/tools/testing/cxl/test/mock.c
index 3c7190b7d5a4..8bb90d732332 100644
--- a/tools/testing/cxl/test/mock.c
+++ b/tools/testing/cxl/test/mock.c
@@ -188,21 +188,6 @@ int __wrap_devm_cxl_enumerate_decoders(struct cxl_hdm *cxlhdm,
}
EXPORT_SYMBOL_NS_GPL(__wrap_devm_cxl_enumerate_decoders, "CXL");
-int __wrap_devm_cxl_port_enumerate_dports(struct cxl_port *port)
-{
- int rc, index;
- struct cxl_mock_ops *ops = get_cxl_mock_ops(&index);
-
- if (ops && ops->is_mock_port(port->uport_dev))
- rc = ops->devm_cxl_port_enumerate_dports(port);
- else
- rc = devm_cxl_port_enumerate_dports(port);
- put_cxl_mock_ops(index);
-
- return rc;
-}
-EXPORT_SYMBOL_NS_GPL(__wrap_devm_cxl_port_enumerate_dports, "CXL");
-
int __wrap_cxl_port_update_total_dports(struct cxl_port *port)
{
int rc, index;
diff --git a/tools/testing/cxl/test/mock.h b/tools/testing/cxl/test/mock.h
index 0a440b9f544c..4d262a38dd51 100644
--- a/tools/testing/cxl/test/mock.h
+++ b/tools/testing/cxl/test/mock.h
@@ -19,7 +19,6 @@ struct cxl_mock_ops {
bool (*is_mock_bus)(struct pci_bus *bus);
bool (*is_mock_port)(struct device *dev);
bool (*is_mock_dev)(struct device *dev);
- int (*devm_cxl_port_enumerate_dports)(struct cxl_port *port);
int (*cxl_port_update_total_dports)(struct cxl_port *port);
struct cxl_hdm *(*devm_cxl_setup_hdm)(
struct cxl_port *port, struct cxl_endpoint_dvsec_info *info);
--
2.50.0
^ permalink raw reply related [flat|nested] 20+ messages in thread