On Tue, Aug 25, 2026 at 04:26:26AM +0800, Fabio M. De Francesco wrote: > CXL r4.0 sec 8.1.5.1 lists Downstream Port Containment among the > conditions that reset the Downstream Component's bus number. DPC > recovery needs to wait for the link to come back up, but if Access > Control Services Source Validation bit is enables PM Initialization will > fail to complete. > > Have dpc_reset_link() reset the secondary bus of a CXL Downstream Port. > Split pci_bridge_secondary_bus_reset() so the reset takes an action > saying what to do with the CXL regions routed through the Port, and pass > the action that unbinds them without offlining their memory. Offlining > migrates the memory that the memdev back and on a contained link are not > addressable. Don't release the memdev driver in cxl_error_detected(). >
Hi Fabio, Do we have any mechanism to prevent CPU or DMA devices from accessing the System RAM while the link and HDM decoder are unavailable ? Unbinding the region only tops region managment, it doesn't stop CPU or DMA memory traffic that's still online. Unbinding also removes DAX/kmem device while its memory is online. dev_dax_kmem_remove() can't remove that memory in this state and will leave the resource reserved until reboot, so the region may not rebind afterward. Also while DPC link is down, cxl_sbr_save_hdm-state() can't safely read downstream HDM register through MMIO, I am not sure whether it's correct to save HDM state here. Best regards, Richard Cheng. > Signed-off-by: Fabio M. De Francesco <[email protected]> > --- > drivers/cxl/core/dport_sbr.c | 57 ++++++++++++++++++++++++++++++++- > drivers/cxl/core/ras.c | 10 ++++++ > drivers/pci/pci.c | 62 ++++++++++++++++++++++++++---------- > drivers/pci/pci.h | 15 +++++++++ > drivers/pci/pcie/dpc.c | 46 +++++++++++++++++++++++++- > include/linux/aer.h | 9 ++++++ > include/linux/pci.h | 3 ++ > 7 files changed, 183 insertions(+), 19 deletions(-) > > diff --git a/drivers/cxl/core/dport_sbr.c b/drivers/cxl/core/dport_sbr.c > index 2b6f840e22a7..823b63012f45 100644 > --- a/drivers/cxl/core/dport_sbr.c > +++ b/drivers/cxl/core/dport_sbr.c > @@ -9,6 +9,30 @@ > #include <cxl.h> > #include "core.h" > > +/* > + * cxl_region_unbind - take a region out of service ahead of a reset > + * @cxlr: region routed through the CXL Downstream Port being reset > + * > + * Unbind the region driver, which tears down everything built on the region: > + * the dax region device, its dax device and the driver bound to it. An SBR > + * zeroes the downstream bus number, so a region left bound would decode to a > + * device in reset. > + * > + * The memory the region hosts is left as it is. A caller that reaches a live > + * device offlines it first; see cxl_region_disable(). > + * > + * Context: process context. Driver unbind sleeps, so this cannot run in > atomic > + * context. > + */ > +static void cxl_region_unbind(struct cxl_region *cxlr) > +{ > + struct cxl_region_params *p = &cxlr->params; > + > + device_release_driver(&cxlr->dev); > + dev_dbg(&cxlr->dev, "%s: region unbound before reset, HPA %pr\n", > + __func__, p->res); > +} > + > /* > * cxl_region_disable - make a region inactive ahead of a Secondary Bus Reset > * @cxlr: region routed through the CXL Downstream Port being reset > @@ -58,7 +82,7 @@ static int cxl_region_disable(struct cxl_region *cxlr) > return rc; > } > > - device_release_driver(&cxlr->dev); > + cxl_region_unbind(cxlr); > dev_dbg(&cxlr->dev, "%s: System RAM offline, region disabled before > reset, HPA %pr\n", > __func__, p->res); > > @@ -284,6 +308,36 @@ static int cxl_sbr_disable_regions(struct pci_dev > *dport_pci) > return rc; > } > > +/* > + * Unbind the regions routed through the Downstream Port being reset, leaving > + * their memory online. Used on the DPC recovery path, where dpc_reset_link() > + * clears DPC Trigger Status and enters the reset without waiting for the > link, > + * so the device may still be unreachable and the page migration that an > offline > + * performs would have no device to read from. > + * > + * Unbinding cannot fail, so unlike cxl_sbr_disable_regions() this never > aborts > + * the reset. The memory stays online across the reset with no region > decoding > + * it; cxl_sbr_enable_regions() reprograms the decoders on the way out. > + */ > +static void cxl_sbr_unbind_regions(struct pci_dev *dport_pci) > +{ > + struct cxl_region *cxlr; > + struct xarray regions; > + unsigned long index; > + > + if (cxl_sbr_save_hdm_state(dport_pci)) > + pci_warn(dport_pci, "HDM state not saved, decode will not be > restored\n"); > + > + xa_init(®ions); > + > + cxl_sbr_collect_regions(dport_pci, ®ions); > + > + xa_for_each(®ions, index, cxlr) > + cxl_region_unbind(cxlr); > + > + cxl_sbr_put_regions(®ions); > +} > + > /* > * Re-enable the regions disabled by cxl_sbr_disable_regions(). Restore the > HDM > * decode first: a region cannot serve memory through decoders that are not > @@ -315,5 +369,6 @@ static void cxl_sbr_enable_regions(struct pci_dev > *dport_pci) > > const struct pci_cxl_sbr_region_ops cxl_sbr_region_ops = { > .disable_regions = cxl_sbr_disable_regions, > + .unbind_regions = cxl_sbr_unbind_regions, > .enable_regions = cxl_sbr_enable_regions, > }; > diff --git a/drivers/cxl/core/ras.c b/drivers/cxl/core/ras.c > index 99fb00949c2f..568a906f3b50 100644 > --- a/drivers/cxl/core/ras.c > +++ b/drivers/cxl/core/ras.c > @@ -323,6 +323,16 @@ pci_ers_result_t cxl_error_detected(struct pci_dev *pdev, > } > return PCI_ERS_RESULT_CAN_RECOVER; > case pci_channel_io_frozen: > + /* > + * A Port on the path in DPC means dpc_reset_link() is about to > + * reset the link, and that path takes the CXL regions out of > + * service and restores the HDM decode itself. Keep the memdev > + * driver bound so the endpoint and its decoders are still there > + * to restore. > + */ > + if (pci_dpc_containment_active(pdev)) > + return PCI_ERS_RESULT_NEED_RESET; > + > dev_warn(&pdev->dev, > "%s: frozen state error detected, disable CXL.mem\n", > dev_name(dev)); > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index f3781d1e8f6e..eedd516f8484 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -4864,24 +4864,37 @@ struct cxl_sbr_ctx { > u16 command; > }; > > -static bool is_cxl_dport(struct pci_dev *dev) > +bool is_cxl_dport(struct pci_dev *dev) > { > return pcie_is_cxl(dev) && pcie_downstream_port(dev); > } > > -static u16 cxl_port_dvsec(struct pci_dev *dev) > +u16 cxl_port_dvsec(struct pci_dev *dev) > { > return pci_find_dvsec_capability(dev, PCI_VENDOR_ID_CXL, > PCI_DVSEC_CXL_PORT); > } > > static int cxl_sbr_prepare(struct pci_dev *bridge, u16 dvsec, > - struct cxl_sbr_ctx *ctx) > + struct cxl_sbr_ctx *ctx, > + enum cxl_sbr_region_action action) > { > int rc; > > - /* Abort before touching hardware if the regions cannot be disabled. */ > - if (cxl_sbr_region_ops) { > + /* > + * CXL_SBR_UNBIND: the link is already down, so offlining the regions' > + * memory would take the reads that page migration performs as a machine > + * check. Per PCIe r7.0 sec 2.9.3 the Port answers a Non-Posted Request > + * with an Unsupported Request or Completer Abort completion while it is > + * in DPC. Unbinding never fails, so the reset always goes ahead. > + * > + * CXL_SBR_OFFLINE_AND_UNBIND: the device is reachable, so offline the > + * memory first and abort the reset before touching hardware if that > + * fails. > + */ > + if (cxl_sbr_region_ops && action == CXL_SBR_UNBIND) { > + cxl_sbr_region_ops->unbind_regions(bridge); > + } else if (cxl_sbr_region_ops) { > rc = cxl_sbr_region_ops->disable_regions(bridge); > if (rc) > return rc; > @@ -5003,19 +5016,17 @@ static void cxl_sbr_complete(struct pci_dev *bridge, > u16 dvsec, > cxl_sbr_region_ops->enable_regions(bridge); > } > > -/** > - * pci_bridge_secondary_bus_reset - Reset the secondary bus on a PCI bridge. > - * @dev: Bridge device > - * > - * Use the bridge control register to assert reset on the secondary bus. > - * Devices on the secondary bus are left in power-on state. > +/* > + * __pci_bridge_secondary_bus_reset - assert Secondary Bus Reset on a bridge > + * @dev: bridge device > + * @action: what to do with the CXL regions reached through @dev > * > - * When @dev is a CXL Downstream Port, clear ACS Source Validation and Bus > - * Master Enable across the reset, per the workaround in CXL r4.0 sec > 8.1.5.1, > - * so that Port Power Management Initialization completes at link-up. The > - * bits stay cleared until the secondary bus is back, then are restored. > + * See pci_bridge_secondary_bus_reset(). Pass CXL_SBR_UNBIND when the link is > + * already down, which leaves the regions' memory online because offlining it > + * needs a reachable device. > */ > -int pci_bridge_secondary_bus_reset(struct pci_dev *dev) > +int __pci_bridge_secondary_bus_reset(struct pci_dev *dev, > + enum cxl_sbr_region_action action) > { > struct cxl_sbr_ctx ctx = {}; > u16 dvsec = 0; > @@ -5028,7 +5039,7 @@ int pci_bridge_secondary_bus_reset(struct pci_dev *dev) > if (is_cxl_dport(dev)) > dvsec = cxl_port_dvsec(dev); > if (dvsec) { > - rc = cxl_sbr_prepare(dev, dvsec, &ctx); > + rc = cxl_sbr_prepare(dev, dvsec, &ctx, action); > if (rc) > return rc; > } > @@ -5042,6 +5053,23 @@ int pci_bridge_secondary_bus_reset(struct pci_dev *dev) > > return rc; > } > + > +/** > + * pci_bridge_secondary_bus_reset - Reset the secondary bus on a PCI bridge. > + * @dev: Bridge device > + * > + * Use the bridge control register to assert reset on the secondary bus. > + * Devices on the secondary bus are left in power-on state. > + * > + * When @dev is a CXL Downstream Port, clear ACS Source Validation and Bus > + * Master Enable across the reset, per the workaround in CXL r4.0 sec > 8.1.5.1, > + * so that Port Power Management Initialization completes at link-up. The > + * bits stay cleared until the secondary bus is back, then are restored. > + */ > +int pci_bridge_secondary_bus_reset(struct pci_dev *dev) > +{ > + return __pci_bridge_secondary_bus_reset(dev, > CXL_SBR_OFFLINE_AND_UNBIND); > +} > EXPORT_SYMBOL_GPL(pci_bridge_secondary_bus_reset); > > static int pci_parent_bus_reset(struct pci_dev *dev, bool probe) > diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h > index 4469e1a77f3c..b6d873b077ed 100644 > --- a/drivers/pci/pci.h > +++ b/drivers/pci/pci.h > @@ -233,6 +233,21 @@ int pci_mmap_fits(struct pci_dev *pdev, int resno, > struct vm_area_struct *vmai, > bool pci_reset_supported(struct pci_dev *dev); > void pci_init_reset_methods(struct pci_dev *dev); > int pci_bridge_secondary_bus_reset(struct pci_dev *dev); > + > +/* > + * What to do with the CXL regions reached through a Downstream Port before > it > + * is reset. Offlining their memory needs a reachable device, so a Port whose > + * link is already down only unbinds and leaves the memory online. > + */ > +enum cxl_sbr_region_action { > + CXL_SBR_OFFLINE_AND_UNBIND, > + CXL_SBR_UNBIND, > +}; > + > +int __pci_bridge_secondary_bus_reset(struct pci_dev *dev, > + enum cxl_sbr_region_action action); > +bool is_cxl_dport(struct pci_dev *dev); > +u16 cxl_port_dvsec(struct pci_dev *dev); > int pci_bus_error_reset(struct pci_dev *dev); > int pci_try_reset_bridge(struct pci_dev *bridge); > > diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c > index 2b779bd1d861..ad74086bf82c 100644 > --- a/drivers/pci/pcie/dpc.c > +++ b/drivers/pci/pcie/dpc.c > @@ -127,6 +127,44 @@ bool pci_dpc_recovered(struct pci_dev *pdev) > } > #endif /* CONFIG_HOTPLUG_PCI_PCIE */ > > +/** > + * pci_dpc_containment_active - whether a Port above @pdev is contained by > DPC > + * @pdev: PCI device below the Port > + * > + * Per PCIe r7.0 sec 2.9.3 the Port's LTSSM stays in the Disabled state while > + * DPC Trigger Status is set, and dpc_reset_link() clears that bit only after > + * pcie_do_recovery() has broadcast error_detected. A ->error_detected() > + * callback can therefore use this to tell a DPC containment from any other > + * frozen-channel error, and to know that the link is about to be reset. > + * > + * The Port that triggered is on the path to @pdev, because the broadcast > walks > + * that Port's subordinate bus, so test every bridge above @pdev. > + * > + * Return: true if a Port on the path to @pdev has DPC Trigger Status set. > + */ > +bool pci_dpc_containment_active(struct pci_dev *pdev) > +{ > + struct pci_dev *bridge; > + > + for (bridge = pci_upstream_bridge(pdev); bridge; > + bridge = pci_upstream_bridge(bridge)) { > + u16 status; > + > + if (!bridge->dpc_cap) > + continue; > + > + pci_read_config_word(bridge, > + bridge->dpc_cap + PCI_EXP_DPC_STATUS, > + &status); > + if (!PCI_POSSIBLE_ERROR(status) && > + (status & PCI_EXP_DPC_STATUS_TRIGGER)) > + return true; > + } > + > + return false; > +} > +EXPORT_SYMBOL_GPL(pci_dpc_containment_active); > + > static int dpc_wait_rp_inactive(struct pci_dev *pdev) > { > unsigned long timeout = jiffies + HZ; > @@ -149,6 +187,7 @@ pci_ers_result_t dpc_reset_link(struct pci_dev *pdev) > { > pci_ers_result_t ret; > u16 cap; > + int rc; > > set_bit(PCI_DPC_RECOVERING, &pdev->priv_flags); > > @@ -174,7 +213,12 @@ pci_ers_result_t dpc_reset_link(struct pci_dev *pdev) > pci_write_config_word(pdev, cap + PCI_EXP_DPC_STATUS, > PCI_EXP_DPC_STATUS_TRIGGER); > > - if (pci_bridge_wait_for_secondary_bus(pdev, "DPC")) { > + if (is_cxl_dport(pdev) && cxl_port_dvsec(pdev)) > + rc = __pci_bridge_secondary_bus_reset(pdev, CXL_SBR_UNBIND); > + else > + rc = pci_bridge_wait_for_secondary_bus(pdev, "DPC"); > + > + if (rc) { > clear_bit(PCI_DPC_RECOVERED, &pdev->priv_flags); > ret = PCI_ERS_RESULT_DISCONNECT; > } else { > diff --git a/include/linux/aer.h b/include/linux/aer.h > index df0f5c382286..7ac029f01c25 100644 > --- a/include/linux/aer.h > +++ b/include/linux/aer.h > @@ -66,6 +66,15 @@ static inline int pcie_aer_is_native(struct pci_dev *dev) > { return 0; } > static inline void pci_aer_unmask_internal_errors(struct pci_dev *dev) { } > #endif > > +#if defined(CONFIG_PCIE_DPC) > +bool pci_dpc_containment_active(struct pci_dev *pdev); > +#else > +static inline bool pci_dpc_containment_active(struct pci_dev *pdev) > +{ > + return false; > +} > +#endif > + > void pci_print_aer(struct pci_dev *dev, int aer_severity, > struct aer_capability_regs *aer); > int cper_severity_to_aer(int cper_severity); > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 2feb0e355305..0d9832ce6f3d 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -1609,10 +1609,13 @@ int pci_bridge_secondary_bus_reset(struct pci_dev > *dev); > /** > * struct pci_cxl_sbr_region_ops - CXL region callbacks for a bus reset > * @disable_regions: disable the regions below @dport, 0 or errno > + * @unbind_regions: unbind the drivers of the regions below @dport, leaving > + * their memory online, for a link already contained > * @enable_regions: re-enable the regions below @dport > */ > struct pci_cxl_sbr_region_ops { > int (*disable_regions)(struct pci_dev *dport); > + void (*unbind_regions)(struct pci_dev *dport); > void (*enable_regions)(struct pci_dev *dport); > }; > > -- > 2.55.0 > >
