From 019560d23450a320f4060ff235563a5277ea8033 Mon Sep 17 00:00:00 2001 From: Dave Jiang Date: Thu, 1 Oct 2026 08:47:42 -0700 Subject: [PATCH 1/9] cxl/port: Clear cached dport pointers when a dport is removed Switch decoders cache dport pointers in cxlsd->target[], and nothing clears them when a dport is freed. Readers then dereference freed memory. KASAN caught it under a cxl_test load/unload loop with concurrent sysfs reads (abbreviated). Clear the matching slots from cxl_dport_remove(), walking the port's decoders the way cxl_port_update_decoder_targets() does on the add side. Scan all nr_targets slots, the target[] allocation size, and clear every hit. Fixes: 8330671c57c7 ("cxl: Add helper to delete dport") Cc: stable@vger.kernel.org Assisted-by: LLM Reviewed-by: Jonathan Cameron Reviewed-by: Li Ming Reviewed-by: Gregory Price (Meta) Reviewed-by: Alison Schofield Link: https://patch.msgid.link/20261001154744.1095902-2-dave.jiang@intel.com Signed-off-by: Dave Jiang --- drivers/cxl/core/port.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c index 95c0be57ed53..ecad766737aa 100644 --- a/drivers/cxl/core/port.c +++ b/drivers/cxl/core/port.c @@ -1097,11 +1097,40 @@ static void cond_cxl_root_unlock(struct cxl_port *port) device_unlock(&port->dev); } +static int clear_decoder_target(struct device *dev, void *data) +{ + struct cxl_dport *dport = data; + struct cxl_switch_decoder *cxlsd; + + if (!is_switch_decoder(dev)) + return 0; + + cxlsd = to_cxl_switch_decoder(dev); + guard(rwsem_write)(&cxl_rwsem.region); + + /* + * A dport can occupy more than one position of an interleave, so + * scan the whole target list rather than stopping at the first hit. + */ + for (int i = 0; i < cxlsd->nr_targets; i++) { + if (cxlsd->target[i] != dport) + continue; + cxlsd->target[i] = NULL; + dev_dbg(dev, "dport%d removed from target list, index %d\n", + dport->port_id, i); + } + + return 0; +} + static void cxl_dport_remove(void *data) { struct cxl_dport *dport = data; struct cxl_port *port = dport->port; + /* counterpart of cxl_port_update_decoder_targets() */ + device_for_each_child(&port->dev, dport, clear_decoder_target); + port->nr_dports--; xa_erase(&port->dports, (unsigned long) dport->dport_dev); put_device(dport->dport_dev); From b72a03ebd9586d59992ecaa6ceed11ea96835734 Mon Sep 17 00:00:00 2001 From: Dave Jiang Date: Thu, 1 Oct 2026 08:47:43 -0700 Subject: [PATCH 2/9] cxl/core: Hold the dport host lock across dport lookup and use Callers of cxl_pci_find_port() and cxl_mem_find_port() can be handed a 'struct cxl_dport' that is already freed, and all four dereference it without the lock that would prevent that. A dport is kzalloc()'d into a devres group on its host device, so it dies when that host unbinds. The host is &port->dev for a switch port and port->uport_dev for the root port. Both helpers return the dport through an output parameter. The caller's port reference pins struct cxl_port, not the dport allocation, and the lookup itself is a bare xa_load() under no lock. Add cxl_port_dport_host() to name the device that pins a dport, and cxl_pci_find_dport() / cxl_mem_find_dport() to look one up with that device's lock held and asserted. Convert the four callers to use them and bail if the dport is gone. cxl_port_add_dport() already follows this rule. cxl_mem_probe() chose its endpoint devm host from dport->rch, which meant dereferencing the dport before taking any lock. Use cxl_port_dport_host(): rch dports only ever land on the root port, so it selects the same device without needing the dport. With every caller converted, the dport output parameter of cxl_pci_find_port() and cxl_mem_find_port() is unused. Drop it. Fixes: 733b57f262b0 ("cxl/pci: Early setup RCH dport component registers from RCRB") Assisted-by: LLM Reviewed-by: Li Ming Reviewed-by: Alison Schofield Link: https://patch.msgid.link/20261001154744.1095902-3-dave.jiang@intel.com Signed-off-by: Dave Jiang --- drivers/cxl/core/core.h | 6 +--- drivers/cxl/core/pci.c | 8 ++++- drivers/cxl/core/port.c | 62 ++++++++++++++++++++++++++------------ drivers/cxl/core/ras_rch.c | 12 +++++++- drivers/cxl/cxl.h | 22 +++++++++++--- drivers/cxl/mem.c | 14 ++++++--- drivers/cxl/pci.c | 11 ++++--- 7 files changed, 95 insertions(+), 40 deletions(-) diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h index bb380ec6daeb..79918fde1575 100644 --- a/drivers/cxl/core/core.h +++ b/drivers/cxl/core/core.h @@ -178,11 +178,7 @@ static inline struct device *port_to_host(struct cxl_port *port) static inline struct device *dport_to_host(struct cxl_dport *dport) { - struct cxl_port *port = dport->port; - - if (is_cxl_root(port)) - return port->uport_dev; - return &port->dev; + return cxl_port_dport_host(dport->port); } #ifdef CONFIG_CXL_RAS void cxl_ras_init(void); diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c index 43b9b7afff29..4af165a436d2 100644 --- a/drivers/cxl/core/pci.c +++ b/drivers/cxl/core/pci.c @@ -743,10 +743,16 @@ int cxl_pci_setup_regs(struct pci_dev *pdev, enum cxl_regloc_type type, if (rc && type == CXL_REGLOC_RBI_COMPONENT && is_cxl_restricted(pdev)) { struct cxl_dport *dport; struct cxl_port *port __free(put_cxl_port) = - cxl_pci_find_port(pdev, &dport); + cxl_pci_find_port(pdev); if (!port) return -EPROBE_DEFER; + guard(device)(cxl_port_dport_host(port)); + + dport = cxl_pci_find_dport(pdev, port); + if (!dport) + return -EPROBE_DEFER; + rc = cxl_rcrb_get_comp_regs(pdev, map, dport); if (rc) return rc; diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c index ecad766737aa..979dffdccef0 100644 --- a/drivers/cxl/core/port.c +++ b/drivers/cxl/core/port.c @@ -1395,13 +1395,11 @@ static int cxl_add_ep(struct cxl_dport *dport, struct device *ep_dev) struct cxl_find_port_ctx { const struct device *dport_dev; const struct cxl_port *parent_port; - struct cxl_dport **dport; }; static int match_port_by_dport(struct device *dev, const void *data) { const struct cxl_find_port_ctx *ctx = data; - struct cxl_dport *dport; struct cxl_port *port; if (!is_cxl_port(dev)) @@ -1410,10 +1408,7 @@ static int match_port_by_dport(struct device *dev, const void *data) return 0; port = to_cxl_port(dev); - dport = cxl_find_dport_by_dev(port, ctx->dport_dev); - if (ctx->dport) - *ctx->dport = dport; - return dport != NULL; + return !!cxl_find_dport_by_dev(port, ctx->dport_dev); } static struct cxl_port *__find_cxl_port_by_dport(struct cxl_find_port_ctx *ctx) @@ -1432,22 +1427,21 @@ static struct cxl_port *__find_cxl_port_by_dport(struct cxl_find_port_ctx *ctx) /** * find_cxl_port_by_dport - find a cxl_port by one of its targets * @dport_dev: device representing the dport target - * @dport: optional output of the 'struct cxl_dport' companion of the @dport_dev * * Return a 'struct cxl_port' with an elevated reference if found. Use * __free(put_cxl_port) to release. + * + * The port reference does not pin the dport, which is a devm allocation of + * cxl_port_dport_host(). Use cxl_pci_find_dport() or cxl_mem_find_dport() + * under that device's lock to get it. */ -static struct cxl_port *find_cxl_port_by_dport(struct device *dport_dev, - struct cxl_dport **dport) +static struct cxl_port *find_cxl_port_by_dport(struct device *dport_dev) { struct cxl_find_port_ctx ctx = { .dport_dev = dport_dev, - .dport = dport, }; - struct cxl_port *port; - port = __find_cxl_port_by_dport(&ctx); - return port; + return __find_cxl_port_by_dport(&ctx); } /* @@ -1937,20 +1931,50 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd) } EXPORT_SYMBOL_NS_GPL(devm_cxl_enumerate_ports, "CXL"); -struct cxl_port *cxl_pci_find_port(struct pci_dev *pdev, - struct cxl_dport **dport) +struct cxl_port *cxl_pci_find_port(struct pci_dev *pdev) { - return find_cxl_port_by_dport(pdev->dev.parent, dport); + return find_cxl_port_by_dport(pdev->dev.parent); } EXPORT_SYMBOL_NS_GPL(cxl_pci_find_port, "CXL"); -struct cxl_port *cxl_mem_find_port(struct cxl_memdev *cxlmd, - struct cxl_dport **dport) +struct cxl_port *cxl_mem_find_port(struct cxl_memdev *cxlmd) { - return find_cxl_port_by_dport(grandparent(&cxlmd->dev), dport); + return find_cxl_port_by_dport(grandparent(&cxlmd->dev)); } EXPORT_SYMBOL_NS_GPL(cxl_mem_find_port, "CXL"); +/** + * cxl_pci_find_dport - find the dport of @port that @pdev is below + * @pdev: PCI device below the dport + * @port: port to search, typically from cxl_pci_find_port() + * + * Context: Caller must hold the cxl_port_dport_host() lock of @port, and + * must not use the result after dropping it. + */ +struct cxl_dport *cxl_pci_find_dport(struct pci_dev *pdev, + struct cxl_port *port) +{ + device_lock_assert(cxl_port_dport_host(port)); + return cxl_find_dport_by_dev(port, pdev->dev.parent); +} +EXPORT_SYMBOL_NS_GPL(cxl_pci_find_dport, "CXL"); + +/** + * cxl_mem_find_dport - find the dport of @port that @cxlmd is below + * @cxlmd: memdev below the dport + * @port: port to search, typically from cxl_mem_find_port() + * + * Context: Caller must hold the cxl_port_dport_host() lock of @port, and + * must not use the result after dropping it. + */ +struct cxl_dport *cxl_mem_find_dport(struct cxl_memdev *cxlmd, + struct cxl_port *port) +{ + device_lock_assert(cxl_port_dport_host(port)); + return cxl_find_dport_by_dev(port, grandparent(&cxlmd->dev)); +} +EXPORT_SYMBOL_NS_GPL(cxl_mem_find_dport, "CXL"); + static int decoder_populate_targets(struct cxl_switch_decoder *cxlsd, struct cxl_port *port) { diff --git a/drivers/cxl/core/ras_rch.c b/drivers/cxl/core/ras_rch.c index e0e01aa5eba6..3c6aae26deed 100644 --- a/drivers/cxl/core/ras_rch.c +++ b/drivers/cxl/core/ras_rch.c @@ -118,10 +118,20 @@ void cxl_handle_rdport_errors(struct cxl_dev_state *cxlds) int severity; struct cxl_port *port __free(put_cxl_port) = - cxl_pci_find_port(pdev, &dport); + cxl_pci_find_port(pdev); if (!port) return; + /* + * The dport may have been freed since the lookup above, so take the + * lock that pins it and find it again. + */ + guard(device)(cxl_port_dport_host(port)); + + dport = cxl_pci_find_dport(pdev, port); + if (!dport) + return; + if (!cxl_rch_get_aer_info(dport->regs.dport_aer, &aer_regs)) return; diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h index cab8ce39f465..cd636d014872 100644 --- a/drivers/cxl/cxl.h +++ b/drivers/cxl/cxl.h @@ -701,6 +701,18 @@ static inline bool is_cxl_root(struct cxl_port *port) return port->uport_dev == port->dev.parent; } +/* + * Return the owner device of the devm allocations of @port's dports. They + * are freed when that device unbinds, so hold device_lock() on the returned + * device across any lookup and use of a 'struct cxl_dport'. + */ +static inline struct device *cxl_port_dport_host(struct cxl_port *port) +{ + if (is_cxl_root(port)) + return port->uport_dev; + return &port->dev; +} + /* Address translation functions exported to cxl_translate test module only */ int cxl_validate_translation_params(u8 eiw, u16 eig, int pos); u64 cxl_calculate_hpa_offset(u64 dpa_offset, int pos, u8 eiw, u16 eig); @@ -747,10 +759,12 @@ DEFINE_FREE(put_cxl_dax_region, struct cxl_dax_region *, if (!IS_ERR_OR_NULL(_T) int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd); void cxl_bus_rescan(void); void cxl_bus_drain(void); -struct cxl_port *cxl_pci_find_port(struct pci_dev *pdev, - struct cxl_dport **dport); -struct cxl_port *cxl_mem_find_port(struct cxl_memdev *cxlmd, - struct cxl_dport **dport); +struct cxl_dport *cxl_pci_find_dport(struct pci_dev *pdev, + struct cxl_port *port); +struct cxl_dport *cxl_mem_find_dport(struct cxl_memdev *cxlmd, + struct cxl_port *port); +struct cxl_port *cxl_pci_find_port(struct pci_dev *pdev); +struct cxl_port *cxl_mem_find_port(struct cxl_memdev *cxlmd); bool schedule_cxl_memdev_detach(struct cxl_memdev *cxlmd); struct cxl_dport *devm_cxl_add_dport(struct cxl_port *port, diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c index 3959ec963026..493c4071245d 100644 --- a/drivers/cxl/mem.c +++ b/drivers/cxl/mem.c @@ -139,7 +139,7 @@ static int cxl_mem_probe(struct device *dev) return rc; struct cxl_port *parent_port __free(put_cxl_port) = - cxl_mem_find_port(cxlmd, &dport); + cxl_mem_find_port(cxlmd); if (!parent_port) { dev_err(dev, "CXL port topology not found\n"); return -ENXIO; @@ -154,10 +154,7 @@ static int cxl_mem_probe(struct device *dev) } } - if (dport->rch) - endpoint_parent = parent_port->uport_dev; - else - endpoint_parent = &parent_port->dev; + endpoint_parent = cxl_port_dport_host(parent_port); scoped_guard(device, endpoint_parent) { if (!endpoint_parent->driver) { @@ -166,6 +163,13 @@ static int cxl_mem_probe(struct device *dev) return -ENXIO; } + /* @dport is only valid while @endpoint_parent stays bound */ + dport = cxl_mem_find_dport(cxlmd, parent_port); + if (!dport) { + dev_err(dev, "CXL topology dport not found\n"); + return -ENXIO; + } + rc = devm_cxl_add_endpoint(endpoint_parent, cxlmd, dport); if (rc) return rc; diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c index 8b560cae91f2..44ae1a20bbbf 100644 --- a/drivers/cxl/pci.c +++ b/drivers/cxl/pci.c @@ -709,22 +709,23 @@ static ssize_t rcd_pcie_cap_emit(struct device *dev, u16 offset, char *buf, size struct device *root_dev; struct cxl_dport *dport; struct cxl_port *root __free(put_cxl_port) = - cxl_mem_find_port(cxlmd, &dport); + cxl_mem_find_port(cxlmd); if (!root) return -ENXIO; - root_dev = root->uport_dev; + root_dev = cxl_port_dport_host(root); if (!root_dev) return -ENXIO; - if (!dport->regs.rcd_pcie_cap) - return -ENXIO; - guard(device)(root_dev); if (!root_dev->driver) return -ENXIO; + dport = cxl_mem_find_dport(cxlmd, root); + if (!dport || !dport->regs.rcd_pcie_cap) + return -ENXIO; + switch (width) { case 2: return sysfs_emit(buf, "%#x\n", From bcda00c78d075340fe30cb6c980507fac711e7d1 Mon Sep 17 00:00:00 2001 From: Dave Jiang Date: Thu, 1 Oct 2026 08:47:44 -0700 Subject: [PATCH 3/9] cxl/port: Attach endpoints to a dport under the lock that pins it devm_cxl_enumerate_ports() and add_port_attach_ep() look up or create a dport under its port's device lock, drop the lock, then pass the dport to cxl_add_ep(). add_ep() reads dport->port before taking the lock, and cxl_gpf_port_setup() reads and writes the dport with no lock held. A concurrent cxl_detach_ep() for another memdev below the same port can free the dport in that window: it calls del_dports() under the port lock when the last endpoint leaves. The enumerating memdev then dereferences freed memory. Attach the endpoint and set up GPF while the port lock from the lookup is still held. Make add_ep() assert that lock instead of taking it. Under the lock, a dport found in port->dports is still live, since removal erases it from the xarray before freeing it. [ dj: Added kdoc update from Alison. ] Fixes: de516b40116e ("cxl/port: Record dport in endpoint references") Reported-by: Li Ming Closes: https://lore.kernel.org/linux-cxl/0c68324c-cfc7-46b2-9344-24ae40e88923@zohomail.com/ Assisted-by: LLM Reviewed-by: Li Ming Reviewed-by: Alison Schofield Link: https://patch.msgid.link/20261001154744.1095902-4-dave.jiang@intel.com Signed-off-by: Dave Jiang --- drivers/cxl/core/port.c | 117 ++++++++++++++++++++++------------------ 1 file changed, 64 insertions(+), 53 deletions(-) diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c index 979dffdccef0..a7bfe320c802 100644 --- a/drivers/cxl/core/port.c +++ b/drivers/cxl/core/port.c @@ -1357,7 +1357,7 @@ static int add_ep(struct cxl_ep *new) { struct cxl_port *port = new->dport->port; - guard(device)(&port->dev); + device_lock_assert(&port->dev); if (port->dead) return -ENXIO; @@ -1373,6 +1373,11 @@ static int add_ep(struct cxl_ep *new) * Intermediate CXL ports are scanned based on the arrival of endpoints. * When those endpoints depart the port can be destroyed once all * endpoints that care about that port have been removed. + * + * Context: Caller must hold the device lock of @dport->port, which pins + * @dport, across the lookup of @dport and this call. @dport->port + * is never the CXL root port, whose dports are pinned by its + * uport_dev instead. */ static int cxl_add_ep(struct cxl_dport *dport, struct device *ep_dev) { @@ -1703,13 +1708,15 @@ static struct cxl_dport *probe_dport(struct cxl_port *port, return drv->add_dport(port, dport_dev); } -static struct cxl_dport *devm_cxl_create_port(struct device *ep_dev, - struct cxl_port *parent_port, - struct cxl_dport *parent_dport, - struct device *uport_dev, - struct device *dport_dev) +static int devm_cxl_create_port(struct device *ep_dev, + struct cxl_port *parent_port, + struct cxl_dport *parent_dport, + struct device *uport_dev, + struct device *dport_dev) { resource_size_t component_reg_phys; + struct cxl_dport *dport; + int rc; device_lock_assert(&parent_port->dev); if (!parent_port->dev.driver) { @@ -1726,7 +1733,7 @@ static struct cxl_dport *devm_cxl_create_port(struct device *ep_dev, port = devm_cxl_add_port(&parent_port->dev, uport_dev, component_reg_phys, parent_dport); if (IS_ERR(port)) - return ERR_CAST(port); + return PTR_ERR(port); /* * retry to make sure a port is found. a port device @@ -1734,7 +1741,7 @@ static struct cxl_dport *devm_cxl_create_port(struct device *ep_dev, */ port = find_cxl_port_by_uport(uport_dev); if (!port) - return ERR_PTR(-ENODEV); + return -ENODEV; dev_dbg(ep_dev, "created port %s:%s\n", dev_name(&port->dev), dev_name(port->uport_dev)); @@ -1743,11 +1750,25 @@ static struct cxl_dport *devm_cxl_create_port(struct device *ep_dev, * Port was created before right before this function is * called. Signal the caller to deal with it. */ - return ERR_PTR(-EAGAIN); + return -EAGAIN; } + /* @dport is only valid while @port's lock is held */ guard(device)(&port->dev); - return probe_dport(port, dport_dev); + dport = probe_dport(port, dport_dev); + if (IS_ERR(dport)) + return PTR_ERR(dport); + + rc = cxl_add_ep(dport, ep_dev); + if (rc == -EBUSY) { + /* + * "can't" happen, but this error code means + * something to the caller, so translate it. + */ + rc = -ENXIO; + } + + return rc; } static int add_port_attach_ep(struct cxl_memdev *cxlmd, @@ -1755,7 +1776,7 @@ static int add_port_attach_ep(struct cxl_memdev *cxlmd, struct device *dport_dev) { struct device *dparent = grandparent(dport_dev); - struct cxl_dport *dport, *parent_dport; + struct cxl_dport *parent_dport; int rc; if (is_cxl_host_bridge(dparent)) { @@ -1784,33 +1805,23 @@ static int add_port_attach_ep(struct cxl_memdev *cxlmd, return PTR_ERR(parent_dport); } - dport = devm_cxl_create_port(&cxlmd->dev, parent_port, - parent_dport, uport_dev, - dport_dev); - if (IS_ERR(dport)) { - /* Port or dport already exists, restart iteration */ - if (PTR_ERR(dport) == -EAGAIN || PTR_ERR(dport) == -EBUSY) - return 0; - return PTR_ERR(dport); - } + rc = devm_cxl_create_port(&cxlmd->dev, parent_port, + parent_dport, uport_dev, dport_dev); } - rc = cxl_add_ep(dport, &cxlmd->dev); - if (rc == -EBUSY) { - /* - * "can't" happen, but this error code means - * something to the caller, so translate it. - */ - rc = -ENXIO; - } + /* Port or dport already exists, restart iteration */ + if (rc == -EAGAIN || rc == -EBUSY) + return 0; return rc; } -static struct cxl_dport *find_or_add_dport(struct cxl_port *port, - struct device *dport_dev) +static int find_or_add_dport_attach_ep(struct cxl_memdev *cxlmd, + struct cxl_port *port, + struct device *dport_dev) { struct cxl_dport *dport; + int rc; /* * The port is already visible in CXL hierarchy, but it may still @@ -1826,13 +1837,28 @@ static struct cxl_dport *find_or_add_dport(struct cxl_port *port, if (!dport) { dport = probe_dport(port, dport_dev); if (IS_ERR(dport)) - return dport; + return PTR_ERR(dport); /* New dport added, restart iteration */ - return ERR_PTR(-EAGAIN); + return -EAGAIN; } - return dport; + /* @dport is only valid while @port's lock is held */ + rc = cxl_add_ep(dport, &cxlmd->dev); + + /* + * If the endpoint already exists in the port's list, + * that's ok, it was added on a previous pass. + * Otherwise, retry in add_port_attach_ep() after taking + * the parent_port lock as the current port may be being + * reaped. + */ + if (rc && rc != -EBUSY) + return rc; + + cxl_gpf_port_setup(dport); + + return 0; } int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd) @@ -1861,7 +1887,6 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd) for (iter = dev; iter; iter = grandparent(iter)) { struct device *dport_dev = grandparent(iter); struct device *uport_dev; - struct cxl_dport *dport; if (is_cxl_host_bridge(dport_dev)) return 0; @@ -1888,27 +1913,13 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd) * RP port enumerated by cxl_acpi without dport will * have the dport added here. */ - dport = find_or_add_dport(port, dport_dev); - if (IS_ERR(dport)) { - if (PTR_ERR(dport) == -EAGAIN) - goto retry; - return PTR_ERR(dport); - } - - rc = cxl_add_ep(dport, &cxlmd->dev); - - /* - * If the endpoint already exists in the port's list, - * that's ok, it was added on a previous pass. - * Otherwise, retry in add_port_attach_ep() after taking - * the parent_port lock as the current port may be being - * reaped. - */ - if (rc && rc != -EBUSY) + rc = find_or_add_dport_attach_ep(cxlmd, port, + dport_dev); + if (rc == -EAGAIN) + goto retry; + if (rc) return rc; - cxl_gpf_port_setup(dport); - /* Any more ports to add between this one and the root? */ if (!dev_is_cxl_root_child(&port->dev)) continue; From d2654fa0d841fa36ffa76577f83a2810d643ae19 Mon Sep 17 00:00:00 2001 From: Guixin Liu Date: Thu, 8 Oct 2026 11:46:12 +0800 Subject: [PATCH 4/9] cxl/pci: Use the Device GPF DVSEC for restricted endpoints cxl_gpf_get_dvsec() picks between the Port and the Device GPF DVSEC from the PCIe type of the given device, and only a plain endpoint counts as a device. A restricted CXL device is an endpoint too, but it falls into the port case and is probed for the Port GPF DVSEC instead of the Device GPF DVSEC. On an RCD, dirty shutdown tracking therefore never arms: the DVSEC lookup in cxl_nvdimm_arm_dirty_shutdown_tracking() fails, the dirty shutdown count stays invalid, and the warning complains about a missing Port GPF DVSEC on a device. Treat restricted endpoints as devices. Fixes: 36aace15d9bd ("cxl/pci: Drop the parameter is_port of cxl_gpf_get_dvsec()") Cc: stable@vger.kernel.org Assisted-by: LLM Signed-off-by: Guixin Liu Reviewed-by: Jonathan Cameron Reviewed-by: Dave Jiang Reviewed-by: Alison Schofield Link: https://patch.msgid.link/20261008034619.28059-2-kanie@linux.alibaba.com Signed-off-by: Dave Jiang --- drivers/cxl/core/pci.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c index 4af165a436d2..5667b4c5a6f3 100644 --- a/drivers/cxl/core/pci.c +++ b/drivers/cxl/core/pci.c @@ -819,7 +819,8 @@ u16 cxl_gpf_get_dvsec(struct device *dev) return 0; pdev = to_pci_dev(dev); - if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ENDPOINT) + if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ENDPOINT || + is_cxl_restricted(pdev)) is_port = false; dvsec = pci_find_dvsec_capability(pdev, PCI_VENDOR_ID_CXL, From ec2e30a682945616d194d7999cf96606b7ee5a88 Mon Sep 17 00:00:00 2001 From: Guixin Liu Date: Thu, 8 Oct 2026 11:46:13 +0800 Subject: [PATCH 5/9] cxl/pci: Program the Port GPF timeouts before caching the DVSEC cxl_gpf_port_setup() caches the Port GPF DVSEC offset in the dport before programming the phase timeouts, and ignores the return value of both update_gpf_port_dvsec() calls: the function always reports success. A failing config write is therefore not only silent, it is also permanent: the timeouts stay at the hardware defaults instead of the maximum flush window the kernel is asking for, and the cached offset keeps any later endpoint attach from retrying the update. Propagate the errors and cache the offset only after both phases are programmed, so the next endpoint attach retries the setup. Fixes: 6af941db6a60 ("cxl/pci: Update Port GPF timeout only when the first EP attaching") Cc: stable@vger.kernel.org Assisted-by: LLM Signed-off-by: Guixin Liu Reviewed-by: Jonathan Cameron Reviewed-by: Dave Jiang Reviewed-by: Alison Schofield Link: https://patch.msgid.link/20261008034619.28059-3-kanie@linux.alibaba.com Signed-off-by: Dave Jiang --- drivers/cxl/core/pci.c | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c index 5667b4c5a6f3..2be3ba63e2d0 100644 --- a/drivers/cxl/core/pci.c +++ b/drivers/cxl/core/pci.c @@ -855,7 +855,7 @@ static int update_gpf_port_dvsec(struct pci_dev *pdev, int dvsec, int phase) rc = pci_read_config_word(pdev, dvsec + offset, &ctrl); if (rc) - return rc; + return pcibios_err_to_errno(rc); if (FIELD_GET(base, ctrl) == GPF_TIMEOUT_BASE_MAX && FIELD_GET(scale, ctrl) == GPF_TIMEOUT_SCALE_MAX) @@ -865,11 +865,17 @@ static int update_gpf_port_dvsec(struct pci_dev *pdev, int dvsec, int phase) ctrl |= FIELD_PREP(scale, GPF_TIMEOUT_SCALE_MAX); rc = pci_write_config_word(pdev, dvsec + offset, ctrl); - if (!rc) - pci_dbg(pdev, "Port GPF phase %d timeout: %d0 secs\n", - phase, GPF_TIMEOUT_BASE_MAX); + if (rc) { + rc = pcibios_err_to_errno(rc); + pci_warn(pdev, "Port GPF phase %d timeout write failed: %d\n", + phase, rc); + return rc; + } - return rc; + pci_dbg(pdev, "Port GPF phase %d timeout: %d0 secs\n", + phase, GPF_TIMEOUT_BASE_MAX); + + return 0; } int cxl_gpf_port_setup(struct cxl_dport *dport) @@ -879,16 +885,22 @@ int cxl_gpf_port_setup(struct cxl_dport *dport) if (!dport->gpf_dvsec) { struct pci_dev *pdev; - int dvsec; + int dvsec, rc; dvsec = cxl_gpf_get_dvsec(dport->dport_dev); if (!dvsec) return -EINVAL; - dport->gpf_dvsec = dvsec; pdev = to_pci_dev(dport->dport_dev); - update_gpf_port_dvsec(pdev, dport->gpf_dvsec, 1); - update_gpf_port_dvsec(pdev, dport->gpf_dvsec, 2); + rc = update_gpf_port_dvsec(pdev, dvsec, 1); + if (rc) + return rc; + + rc = update_gpf_port_dvsec(pdev, dvsec, 2); + if (rc) + return rc; + + dport->gpf_dvsec = dvsec; } return 0; From b5d14009bdb1973d54a6ea111a660b3dae115be5 Mon Sep 17 00:00:00 2001 From: Guixin Liu Date: Thu, 8 Oct 2026 11:46:14 +0800 Subject: [PATCH 6/9] cxl/pci: Update only the Port GPF timeout fields update_gpf_port_dvsec() rebuilds the phase control register from just the timeout base and scale, so the rest of the register is cleared by the write. The other bits are reserved today and nothing is lost, but the full-register rewrite would clobber any field the spec defines there later. Read-modify-write the register and change only the two timeout fields. Fixes: a52b6a2c1c99 ("cxl/pci: Support Global Persistent Flush (GPF)") Assisted-by: LLM Signed-off-by: Guixin Liu Reviewed-by: Dave Jiang Reviewed-by: Alison Schofield Link: https://patch.msgid.link/20261008034619.28059-4-kanie@linux.alibaba.com Signed-off-by: Dave Jiang --- drivers/cxl/core/pci.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c index 2be3ba63e2d0..713a75e68bb9 100644 --- a/drivers/cxl/core/pci.c +++ b/drivers/cxl/core/pci.c @@ -861,8 +861,8 @@ static int update_gpf_port_dvsec(struct pci_dev *pdev, int dvsec, int phase) FIELD_GET(scale, ctrl) == GPF_TIMEOUT_SCALE_MAX) return 0; - ctrl = FIELD_PREP(base, GPF_TIMEOUT_BASE_MAX); - ctrl |= FIELD_PREP(scale, GPF_TIMEOUT_SCALE_MAX); + FIELD_MODIFY(base, &ctrl, GPF_TIMEOUT_BASE_MAX); + FIELD_MODIFY(scale, &ctrl, GPF_TIMEOUT_SCALE_MAX); rc = pci_write_config_word(pdev, dvsec + offset, ctrl); if (rc) { From 16440d6e4bf1175cda83438d9f9a8ae84bfcf0ee Mon Sep 17 00:00:00 2001 From: Koichiro Den Date: Fri, 9 Oct 2026 13:54:50 +0900 Subject: [PATCH 7/9] cxl/port: Convert PCIe link latency to nanoseconds Commit 51293c565cf4 ("cxl: Fix incorrect region perf data calculation") converted CDAT latencies to nanoseconds and removed the conversion in cxl_region_perf_data_calculate(), but left PCIe link latencies in picoseconds. This inflates the FlitLatency contribution to region and node latencies by a factor of 1000, exposing incorrect values in sysfs. The resulting abstract distances can incorrectly reverse the memory tier ordering of nodes. Convert the FlitLatency contribution to nanoseconds before adding it to the access coordinates, rounding up as cdat_normalize() does. Fixes: 51293c565cf4 ("cxl: Fix incorrect region perf data calculation") Cc: stable@vger.kernel.org Signed-off-by: Koichiro Den Reviewed-by: Alison Schofield Reviewed-by: Dave Jiang Link: https://patch.msgid.link/20261009045452.1107921-2-den@valinux.co.jp Signed-off-by: Dave Jiang --- drivers/cxl/core/port.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c index a7bfe320c802..376d1d720709 100644 --- a/drivers/cxl/core/port.c +++ b/drivers/cxl/core/port.c @@ -2392,6 +2392,9 @@ EXPORT_SYMBOL_NS_GPL(schedule_cxl_memdev_detach, "CXL"); static void add_latency(struct access_coordinate *c, long latency) { + /* Convert link latency from picoseconds to nanoseconds. */ + latency = DIV_ROUND_UP(latency, 1000); + for (int i = 0; i < ACCESS_COORDINATE_MAX; i++) { c[i].write_latency += latency; c[i].read_latency += latency; From db4503d4575cdd2fad7e8a13a8a8ffcbc86d152d Mon Sep 17 00:00:00 2001 From: Koichiro Den Date: Fri, 9 Oct 2026 13:54:51 +0900 Subject: [PATCH 8/9] cxl: Account for link width in latency calculation cxl_pci_get_latency() derives bandwidth from the per-lane speed returned by pcie_link_speed_mbps(), ignoring the negotiated link width. This overestimates the FlitLatency contribution for multi-lane links. Sections 2.11.3 and 2.11.4 of the Intel CXL Memory Device Software Guide describe the link bandwidth as the product of the negotiated speed and width. Replace pcie_link_speed_mbps() with pcie_link_bandwidth_mbps() and migrate both CXL callers (the only users of the API). The new helper obtains speed and width from a single Link Status read. The CXL bandwidth calculation no longer needs a separate width read and multiplication. While at it, make error handling more robust by converting PCI config read errors to negative errno values and checking for zero bandwidth before calculating latency. Fixes: 4d07a05397c8 ("cxl: Calculate and store PCI link latency for the downstream ports") Cc: stable@vger.kernel.org Signed-off-by: Koichiro Den Reviewed-by: Alison Schofield Acked-by: Bjorn Helgaas # pci/pci.c Reviewed-by: Dave Jiang Link: https://patch.msgid.link/20261009045452.1107921-3-den@valinux.co.jp Signed-off-by: Dave Jiang --- drivers/cxl/core/pci.c | 20 +++++++------------- drivers/pci/pci.c | 23 ++++++++++++++++++----- include/linux/pci.h | 2 +- 3 files changed, 26 insertions(+), 19 deletions(-) diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c index 713a75e68bb9..125abb36df7f 100644 --- a/drivers/cxl/core/pci.c +++ b/drivers/cxl/core/pci.c @@ -660,8 +660,8 @@ long cxl_pci_get_latency(struct pci_dev *pdev) { long bw; - bw = pcie_link_speed_mbps(pdev); - if (bw < 0) + bw = pcie_link_bandwidth_mbps(pdev); + if (bw <= 0) return 0; bw /= BITS_PER_BYTE; @@ -771,18 +771,12 @@ EXPORT_SYMBOL_NS_GPL(cxl_pci_setup_regs, "CXL"); int cxl_pci_get_bandwidth(struct pci_dev *pdev, struct access_coordinate *c) { - int speed, bw; - u16 lnksta; - u32 width; + int bw; - speed = pcie_link_speed_mbps(pdev); - if (speed < 0) - return speed; - speed /= BITS_PER_BYTE; - - pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnksta); - width = FIELD_GET(PCI_EXP_LNKSTA_NLW, lnksta); - bw = speed * width; + bw = pcie_link_bandwidth_mbps(pdev); + if (bw < 0) + return bw; + bw /= BITS_PER_BYTE; for (int i = 0; i < ACCESS_COORDINATE_MAX; i++) { c[i].read_bandwidth = bw; diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index b2879a6be5f8..aa3d180de56e 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -5980,18 +5980,31 @@ static enum pci_bus_speed to_pcie_link_speed(u16 lnksta) return pcie_link_speed[FIELD_GET(PCI_EXP_LNKSTA_CLS, lnksta)]; } -int pcie_link_speed_mbps(struct pci_dev *pdev) +/** + * pcie_link_bandwidth_mbps - get the current bandwidth of a single PCIe link + * @pdev: PCI device to query + * + * The bandwidth includes all negotiated lanes and does not account for + * encoding overhead. A negotiated width of zero gives zero bandwidth. + * + * Return: Bandwidth in Mb/s, or a negative errno on failure. + */ +int pcie_link_bandwidth_mbps(struct pci_dev *pdev) { u16 lnksta; - int err; + int speed, err; err = pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnksta); if (err) - return err; + return pcibios_err_to_errno(err); - return pcie_dev_speed_mbps(to_pcie_link_speed(lnksta)); + speed = pcie_dev_speed_mbps(to_pcie_link_speed(lnksta)); + if (speed < 0) + return speed; + + return speed * FIELD_GET(PCI_EXP_LNKSTA_NLW, lnksta); } -EXPORT_SYMBOL(pcie_link_speed_mbps); +EXPORT_SYMBOL(pcie_link_bandwidth_mbps); /** * pcie_bandwidth_available - determine minimum link settings of a PCIe diff --git a/include/linux/pci.h b/include/linux/pci.h index d31a8d107b1e..bbfb0b98ff46 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1477,7 +1477,7 @@ int pcie_set_mps(struct pci_dev *dev, int mps); u32 pcie_bandwidth_available(struct pci_dev *dev, struct pci_dev **limiting_dev, enum pci_bus_speed *speed, enum pcie_link_width *width); -int pcie_link_speed_mbps(struct pci_dev *pdev); +int pcie_link_bandwidth_mbps(struct pci_dev *pdev); void pcie_print_link_status(struct pci_dev *dev); int pcie_reset_flr(struct pci_dev *dev, bool probe); int pcie_flr(struct pci_dev *dev); From 2658c66a5efb5f30f5ced708a5de831845b43f6d Mon Sep 17 00:00:00 2001 From: Koichiro Den Date: Fri, 9 Oct 2026 13:54:52 +0900 Subject: [PATCH 9/9] cxl/acpi: Convert QTG _DSM latencies to picoseconds The QTG _DSM expects read and write latencies in picoseconds, as specified in CXL 3.0, Table 9-30. The code base originally kept the latencies passed to the QTG _DSM in picoseconds in effect. Commit 51293c565cf4 ("cxl: Fix incorrect region perf data calculation") normalized CDAT and host bridge latencies to nanoseconds, but did not add a conversion back to picoseconds when constructing the _DSM input. Firmware can therefore select QTG IDs using understated latencies. Convert the latencies to picoseconds when constructing the _DSM input package. Multiply in 64 bits and reject latencies that cannot fit in the DWORD inputs defined by the specification. Fixes: 51293c565cf4 ("cxl: Fix incorrect region perf data calculation") Cc: stable@vger.kernel.org Signed-off-by: Koichiro Den Reviewed-by: Alison Schofield Reviewed-by: Dave Jiang Link: https://patch.msgid.link/20261009045452.1107921-4-den@valinux.co.jp Signed-off-by: Dave Jiang --- drivers/cxl/acpi.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c index fb09a5ff48c1..b7ce6b59c680 100644 --- a/drivers/cxl/acpi.c +++ b/drivers/cxl/acpi.c @@ -231,9 +231,10 @@ cxl_acpi_evaluate_qtg_dsm(acpi_handle handle, struct access_coordinate *coord, int entries, int *qos_class) { union acpi_object *out_obj, *out_buf, *obj; + /* QTG _DSM expects latencies in picoseconds. */ union acpi_object in_array[4] = { - [0].integer = { ACPI_TYPE_INTEGER, coord->read_latency }, - [1].integer = { ACPI_TYPE_INTEGER, coord->write_latency }, + [0].integer = { ACPI_TYPE_INTEGER, coord->read_latency * 1000ULL }, + [1].integer = { ACPI_TYPE_INTEGER, coord->write_latency * 1000ULL }, [2].integer = { ACPI_TYPE_INTEGER, coord->read_bandwidth }, [3].integer = { ACPI_TYPE_INTEGER, coord->write_bandwidth }, }; @@ -251,6 +252,11 @@ cxl_acpi_evaluate_qtg_dsm(acpi_handle handle, struct access_coordinate *coord, if (!entries) return -EINVAL; + /* Latency inputs must fit in the DWORD fields defined by the spec. */ + if (coord->read_latency > U32_MAX / 1000 || + coord->write_latency > U32_MAX / 1000) + return -ERANGE; + out_obj = acpi_evaluate_dsm(handle, &acpi_cxl_qtg_id_guid, 1, 1, &in_obj); if (!out_obj) return -ENXIO;