From: Dave Jiang <dave.jiang@intel.com>
To: linux-cxl@vger.kernel.org
Cc: Dan Williams <dan.j.williams@intel.com>,
Dave Jiang <dave.jiang@intel.com>,
dave@stgolabs.net, jonathan.cameron@huawei.com,
alison.schofield@intel.com, ira.weiny@intel.com,
rrichter@amd.com, ming.li@zohomail.com
Subject: [PATCH v2 05/10] cxl: Defer hardware dport->port_id assignment and registers probing
Date: Tue, 6 May 2025 17:43:05 -0700 [thread overview]
Message-ID: <20250507004310.3536991-6-dave.jiang@intel.com> (raw)
In-Reply-To: <20250507004310.3536991-1-dave.jiang@intel.com>
Current implementation only enumerates the dports during the port 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 hw
dport id assignment and the register probing to behind memdev probe so the
endpoint is guaranteed to be connected.
The detection of duplicate dport for add_dport() is removed. The port_id
is not read from the hw at this point any longer. The port->id will always
be unique since it's retrieved from an ida. The dup detection thus become
irrelevant.
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.
Link: https://lore.kernel.org/linux-cxl/20250305100123.3077031-1-rrichter@amd.com/
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v2:
- Fix spelling error in commit message. (Ming)
- Move probing to the specific dport. (Ming)
- Add decoder update.
- Return errors when setup dports. (Jonathan)
- Add driver check before setup dport. (Dan)
- Add comment on why location to do dport update. (Dan)
---
drivers/cxl/core/core.h | 4 ++
drivers/cxl/core/pci.c | 60 ++++++++++++----
drivers/cxl/core/port.c | 152 +++++++++++++++++++++++++++++++++++-----
drivers/cxl/cxl.h | 4 ++
drivers/cxl/port.c | 20 +-----
5 files changed, 192 insertions(+), 48 deletions(-)
diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
index 17b692eb3257..8b1d9a8ab852 100644
--- a/drivers/cxl/core/core.h
+++ b/drivers/cxl/core/core.h
@@ -134,4 +134,8 @@ int cxl_set_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
u16 *return_code);
#endif
+int cxl_dport_setup(struct cxl_dport *dport);
+int cxl_dport_setup_regs(struct device *host, struct cxl_dport *dport,
+ resource_size_t component_reg_phys);
+
#endif /* __CXL_CORE_H__ */
diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
index 3b84b43ab194..16bd842904e5 100644
--- a/drivers/cxl/core/pci.c
+++ b/drivers/cxl/core/pci.c
@@ -24,6 +24,52 @@ 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");
+int cxl_dport_setup(struct cxl_dport *dport)
+{
+ struct device *dport_dev = dport->dport_dev;
+ struct cxl_port *port = dport->port;
+ struct cxl_register_map map;
+ struct pci_dev *pdev;
+ u32 lnkcap, port_num;
+ int rc;
+
+ if (!dev_is_pci(dport_dev))
+ return 0;
+
+ /*
+ * dport->port_id is valid means that dport has been probed and is
+ * setup.
+ */
+ if (dport->port_num != CXL_DPORT_NUM_INVALID)
+ return 0;
+
+ pdev = to_pci_dev(dport_dev);
+ if (pci_read_config_dword(pdev, pci_pcie_cap(pdev) + PCI_EXP_LNKCAP,
+ &lnkcap))
+ return -ENODEV;
+
+ rc = cxl_find_regblock(pdev, CXL_REGLOC_RBI_COMPONENT, &map);
+ if (rc) {
+ dev_dbg(&port->dev, "failed to find component registers\n");
+ return -ENODEV;
+ }
+
+ port_num = FIELD_GET(PCI_EXP_LNKCAP_PN, lnkcap);
+ rc = cxl_dport_setup_regs(&port->dev, dport, map.resource);
+ if (rc)
+ return rc;
+
+ if (map.resource != CXL_RESOURCE_NONE) {
+ dev_dbg(dport->dport_dev,
+ "Component Register found for dport: %pa\n",
+ &map.resource);
+ }
+
+ dport->port_num = port_num;
+
+ return 0;
+}
+
struct cxl_walk_context {
struct pci_bus *bus;
struct cxl_port *port;
@@ -37,10 +83,7 @@ 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;
@@ -48,22 +91,13 @@ static int match_add_dports(struct pci_dev *pdev, void *data)
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, map.resource);
+ dport = devm_cxl_add_dport(port, &pdev->dev, CXL_RESOURCE_NONE);
if (IS_ERR(dport)) {
ctx->error = PTR_ERR(dport);
return PTR_ERR(dport);
}
- dport->port_num = port_num;
ctx->count++;
return 0;
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 70173d23139c..04e18a102d26 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -782,8 +782,8 @@ static int cxl_port_setup_regs(struct cxl_port *port,
component_reg_phys);
}
-static int cxl_dport_setup_regs(struct device *host, struct cxl_dport *dport,
- resource_size_t component_reg_phys)
+int cxl_dport_setup_regs(struct device *host, struct cxl_dport *dport,
+ resource_size_t component_reg_phys)
{
int rc;
@@ -1004,7 +1004,7 @@ int devm_cxl_register_pci_bus(struct device *host, struct device *uport_dev,
}
EXPORT_SYMBOL_NS_GPL(devm_cxl_register_pci_bus, "CXL");
-static bool dev_is_cxl_root_child(struct device *dev)
+bool dev_is_cxl_root_child(struct device *dev)
{
struct cxl_port *port, *parent;
@@ -1021,6 +1021,7 @@ static bool dev_is_cxl_root_child(struct device *dev)
return false;
}
+EXPORT_SYMBOL_NS_GPL(dev_is_cxl_root_child, "CXL");
struct cxl_root *find_cxl_root(struct cxl_port *port)
{
@@ -1059,19 +1060,9 @@ static struct cxl_dport *find_dport_by_num(struct cxl_port *port, int port_num)
static int add_dport(struct cxl_port *port, struct cxl_dport *dport)
{
- struct cxl_dport *dup;
int rc;
device_lock_assert(&port->dev);
- dup = find_dport_by_num(port, dport->port_num);
- if (dup) {
- dev_err(&port->dev,
- "unable to add dport%d-%s non-unique port num (%s)\n",
- dport->port_num, dev_name(dport->dport_dev),
- dev_name(dup->dport_dev));
- return -EBUSY;
- }
-
rc = xa_insert(&port->dports, (unsigned long)dport->dport_dev, dport,
GFP_KERNEL);
if (rc)
@@ -1643,6 +1634,118 @@ static int add_port_attach_ep(struct cxl_memdev *cxlmd,
return rc;
}
+static int port_probe_dport(struct cxl_port *port, struct device *dport_dev,
+ struct cxl_dport **probed_dport)
+{
+ struct cxl_dport *dport;
+ unsigned long index;
+ int rc;
+
+ device_lock_assert(&port->dev);
+ xa_for_each(&port->dports, index, dport) {
+ if (dport->dport_dev == dport_dev) {
+ rc = cxl_dport_setup(dport);
+ if (rc)
+ return rc;
+ *probed_dport = dport;
+ return 0;
+ }
+ }
+
+ *probed_dport = NULL;
+
+ return -ENODEV;
+}
+
+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_num) {
+ 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);
+}
+
+int devm_cxl_port_setup_decoders(struct cxl_port *port)
+{
+ struct cxl_dport *dport;
+ struct cxl_hdm *cxlhdm;
+ unsigned long index;
+ int dports = 0;
+
+ cxlhdm = devm_cxl_setup_hdm(port, NULL);
+ if (!IS_ERR(cxlhdm))
+ return devm_cxl_enumerate_decoders(cxlhdm, NULL);
+
+ if (PTR_ERR(cxlhdm) != -ENODEV) {
+ dev_err(&port->dev, "Failed to map HDM decoder capability\n");
+ return PTR_ERR(cxlhdm);
+ }
+
+ xa_for_each(&port->dports, index, dport)
+ dports++;
+
+ if (dports == 1) {
+ dev_dbg(&port->dev, "Fallback to passthrough decoder\n");
+ return devm_cxl_add_passthrough_decoder(port);
+ }
+
+ dev_err(&port->dev, "HDM decoder capability not found\n");
+ return -ENXIO;
+}
+EXPORT_SYMBOL_NS_GPL(devm_cxl_port_setup_decoders, "CXL");
+
+static int cxl_switch_port_dport_setup(struct cxl_port *port,
+ struct device *dport_dev)
+{
+ struct cxl_dport *dport;
+ int rc;
+
+ device_lock_assert(&port->dev);
+
+ if (!port->dev.driver)
+ return -ENODEV;
+
+ rc = port_probe_dport(port, dport_dev, &dport);
+ if (rc)
+ return rc;
+
+ cxl_switch_parse_cdat(port);
+
+ /* Make sure that no decoders have been allocated before proceeding. */
+ if (ida_is_empty(&port->decoder_ida))
+ rc = devm_cxl_port_setup_decoders(port);
+ else
+ rc = update_decoders_with_dport(port, dport);
+ if (rc)
+ return rc;
+
+ return 0;
+}
+
int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
{
struct device *dev = &cxlmd->dev;
@@ -1695,6 +1798,19 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
"found already registered port %s:%s\n",
dev_name(&port->dev),
dev_name(port->uport_dev));
+
+ /*
+ * Attempt to do single pass dport setup by checking here
+ * instead of doing it during port creation. Otherwise
+ * it still needs to check here for dports that are
+ * being probed with a port already created.
+ */
+ scoped_guard(device, &port->dev) {
+ rc = cxl_switch_port_dport_setup(port, dport_dev);
+ if (rc)
+ return rc;
+ }
+
rc = cxl_add_ep(dport, &cxlmd->dev);
/*
@@ -1760,10 +1876,14 @@ static int decoder_populate_targets(struct cxl_switch_decoder *cxlsd,
guard(rwsem_write)(&cxl_region_rwsem);
for (i = 0; i < cxlsd->cxld.interleave_ways; i++) {
- struct cxl_dport *dport = find_dport_by_num(port, target_map[i]);
+ struct cxl_dport *dport;
- if (!dport)
- return -ENXIO;
+ cxlsd->target_map[i] = target_map[i];
+ dport = find_dport_by_num(port, target_map[i]);
+ 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 4ba3bbe9600b..6b85d5f23be0 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -404,6 +404,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: array of expected dport port_id mirror the target
* @target: active ordered target list in current decoder configuration
*
* The 'switch' decoder type represents the decoder instances of cxl_port's that
@@ -415,6 +416,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[];
};
@@ -906,6 +908,8 @@ void cxl_coordinates_combine(struct access_coordinate *out,
struct access_coordinate *c2);
bool cxl_endpoint_decoder_reset_detected(struct cxl_port *port);
+int devm_cxl_port_setup_decoders(struct cxl_port *port);
+bool dev_is_cxl_root_child(struct device *dev);
/*
* Unit test builds overrides this to __weak, find the 'strong' version
diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
index a35fc5552845..4d840a6ef802 100644
--- a/drivers/cxl/port.c
+++ b/drivers/cxl/port.c
@@ -59,7 +59,6 @@ static int discover_region(struct device *dev, void *root)
static int cxl_switch_port_probe(struct cxl_port *port)
{
- struct cxl_hdm *cxlhdm;
int rc;
/* Cache the data early to ensure is_visible() works */
@@ -69,24 +68,7 @@ static int cxl_switch_port_probe(struct cxl_port *port)
if (rc < 0)
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);
-
- if (PTR_ERR(cxlhdm) != -ENODEV) {
- dev_err(&port->dev, "Failed to map HDM decoder capability\n");
- return PTR_ERR(cxlhdm);
- }
-
- if (rc == 1) {
- dev_dbg(&port->dev, "Fallback to passthrough decoder\n");
- return devm_cxl_add_passthrough_decoder(port);
- }
-
- dev_err(&port->dev, "HDM decoder capability not found\n");
- return -ENXIO;
+ return 0;
}
static int cxl_endpoint_port_probe(struct cxl_port *port)
--
2.49.0
next prev parent reply other threads:[~2025-05-07 0:43 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-05-07 0:43 [PATCH v2 00/10] cxl: Delay HB port and switch dport probing until endpoint dev probe Dave Jiang
2025-05-07 0:43 ` [PATCH v2 01/10] cxl/region: Add decoder check to check_commit_order() Dave Jiang
2025-05-08 19:54 ` Alison Schofield
2025-05-09 0:55 ` Li Ming
2025-05-13 4:46 ` Gregory Price
2025-05-20 11:14 ` Jonathan Cameron
2025-05-20 16:13 ` Dave Jiang
2025-05-07 0:43 ` [PATCH v2 02/10] cxl: Saperate out CXL dport->id vs actual dport hardware id Dave Jiang
2025-05-08 20:08 ` Alison Schofield
2025-05-15 16:35 ` Dave Jiang
2025-05-09 0:51 ` Li Ming
2025-05-15 16:33 ` Dave Jiang
2025-05-09 9:14 ` Alejandro Lucero Palau
2025-05-15 16:35 ` Dave Jiang
2025-05-13 5:04 ` Gregory Price
2025-05-15 16:38 ` Dave Jiang
2025-05-20 11:19 ` Jonathan Cameron
2025-05-07 0:43 ` [PATCH v2 03/10] cxl: Rename find_dport() to provide better function intent Dave Jiang
2025-05-09 0:55 ` Li Ming
2025-05-09 9:20 ` Alejandro Lucero Palau
2025-05-15 17:04 ` Dave Jiang
2025-05-19 16:33 ` Dave Jiang
2025-05-20 11:21 ` Jonathan Cameron
2025-05-13 5:07 ` Gregory Price
2025-05-07 0:43 ` [PATCH v2 04/10] cxl: Remove adding of port_num via devm_cxl_add_dport() Dave Jiang
2025-05-09 0:56 ` Li Ming
2025-05-13 5:13 ` Gregory Price
2025-05-20 11:23 ` Jonathan Cameron
2025-05-07 0:43 ` Dave Jiang [this message]
2025-05-08 4:50 ` [PATCH v2 05/10] cxl: Defer hardware dport->port_id assignment and registers probing Li Ming
2025-05-13 15:43 ` Gregory Price
2025-05-15 22:03 ` Dave Jiang
2025-05-20 11:26 ` Jonathan Cameron
2025-05-20 16:33 ` Dave Jiang
2025-05-20 12:27 ` Jonathan Cameron
2025-05-07 0:43 ` [PATCH v2 06/10] cxl/test: Add workaround for cxl_test for cxl_core calling mocked functions Dave Jiang
2025-05-20 12:31 ` Jonathan Cameron
2025-05-07 0:43 ` [PATCH v2 07/10] cxl: Change sslbis handler to only handle single dport Dave Jiang
2025-05-13 15:48 ` Gregory Price
2025-05-20 12:32 ` Jonathan Cameron
2025-05-20 21:53 ` Dave Jiang
2025-05-07 0:43 ` [PATCH v2 08/10] cxl: Add helper to detect top of CXL device topology Dave Jiang
2025-05-13 15:49 ` Gregory Price
2025-05-13 16:12 ` Dave Jiang
2025-05-15 17:03 ` Gregory Price
2025-05-16 15:47 ` Dave Jiang
2025-05-20 12:34 ` Jonathan Cameron
2025-05-20 21:55 ` Dave Jiang
2025-05-07 0:43 ` [PATCH v2 09/10] cxl: Create an xarray to tie a host bridge to the cxl_root Dave Jiang
2025-05-13 16:01 ` Gregory Price
2025-05-20 12:53 ` Jonathan Cameron
2025-05-07 0:43 ` [PATCH v2 10/10] cxl: Move enumeration of hostbridge ports to the memdev probe path Dave Jiang
2025-05-20 13:11 ` Jonathan Cameron
2025-05-20 21:59 ` Dave Jiang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250507004310.3536991-6-dave.jiang@intel.com \
--to=dave.jiang@intel.com \
--cc=alison.schofield@intel.com \
--cc=dan.j.williams@intel.com \
--cc=dave@stgolabs.net \
--cc=ira.weiny@intel.com \
--cc=jonathan.cameron@huawei.com \
--cc=linux-cxl@vger.kernel.org \
--cc=ming.li@zohomail.com \
--cc=rrichter@amd.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox