Re: [PATCH v2 00/13] Make SBR work for CXL Downstream Ports

[Richard Cheng <icheng@nvidia.com>]

On Tue, Aug 25, 2026 at 04:26:15AM +0800, Fabio M. De Francesco wrote:
> A Secondary Bus Reset (SBR), a Link Down or a Downstream Port
> Containment (DPC) event on a CXL Downstream Port clears the Downstream
> Component's captured Bus Number. CXL r4.0 sec 8.1.5.1 notes that, if the
> Component then sources its Power Management (PM) Initialization traffic
> with Requester Bus = 0, a Port with Access Control Services (ACS) Source
> Validation (SV) enabled may reject the Component's CREDIT_RTN IP2PM
> message, PM Initialization may fail to complete, and that "another
> Secondary Bus Reset alone will not facilitate recovery". PCIe r7.0 sec
> 6.12.1.1 makes the rejection an error that is reported as ACS Violation.
> 
> The Implementation Note under sec 8.1.5.1 gives the sequence that avoids
> it: save and clear Bus Master Enable and ACS SV, generate the SBR, wait
> for Port PM Initialization Complete, restore both bits, continue device
> re-initialization. This series takes a step further to a preventive
> measure that avoids the ACS Violation in the first place, not after
> failure.
> 
> Patches 1-5 restore HDM Decoder registers below a reset CXL Downstream
> Port.
> 
> Patches 6-8 disable and re-enable the CXL Regions around the reset:
> offline memory, invalidate CPU caches for its range, collect the set of
> regions the CXL DPort which is SBR andunbind the driver.
> 
> Patch 9 disable and re-enable CXL regions across a Downstream Port reset
> and registers the CXL region callbacks for a Downstream Port bus. We
> want the reset to be called only when the CXL driver is loaded.
> 
> Patch 10 calls the CXL operations from pci_bridge_secondary_bus_reset().
> 
> Patch 11 issues the same sequence from dpc_reset_link().
> 
> Patches 12 and 13 gives root a sysfs attribute that conrols whether the
> SBR is allowed to be unmasked and prevents kernel from unmasking SBR on
> its own.
> 
> What user space finds after an SBR of a CXL Downstream Port: the dax
> device returns under the same name, the region's HPA range, interleave
> geometry and decoder programming are restored, and the media content is
> unchanged for PMEM. System RAM comes back registered but offline.
>

Hi Fabio,

I think overall the recovery sequence makes sense, but I have some concerns
about how the state of one reset is represented.

In your current design, if I'm not misunderstanding, the state of one reset is split
across several places:

- PCI keeps ACS, BME, and Port Control in its local cxl_sbr_ctx
- CXL keeps HDM state in a global xarray keyed by the Downstream Port
- disable_regions() collects a region set, operates on it, and then discards it
- enable_regions() collects the region set again after the reset

There's no single owner that remembers exactly which regions and memory blocks were
changed by the corresponded reset. The set collected after the reset should be the
same set that was disabled, but I don't get where does this guaranteed ?
Collection or decoder restoration may fail, but enable_regions() can't report failure
and may still reattach regions.

IMHO, maybe have an opaque per-reset context, something like

"""
prepare(dport, mode) -> context
reset
restore(context)
"""

The context should hold
- Ref to the exact affected reegions, ports, EP
- The original bound state of each region
- The original online type of each memory block changed by preparation
- Cached HDM Global Control and decoder state
- The current phase and which preparation steps completed

I think PCI core doesn't need to know the contents of this context, it only pass it
to CXL core. This can allow the recovery path to dinstinguish 3 outcomes
- Preparation failed before SBR
- SBR and restoration succeeded
- SBR was issued but restoration failed

This can also avoid the need to recollect topology after reset or store operation state
in a global xarray.
Separate resets would naturally have separate state.

Does this model fit the indended flow ?

Best regards,
Richard Cheng.

 
> Fabio M. De Francesco (13):
>   cxl/pci: Make the HDM and Mem_Enable writes callable from cxl_core
>   cxl/hdm: Add cxl_decoder_recommit() to restore one HDM decoder
>   cxl/hdm: Add cxl_endpoint_enable_hdm_decode() to restore CXL.mem
>     decode
>   cxl/hdm: Reprogram the HDM Decoders below a CXL Port
>   cxl/core: Restore the HDM decoders below a reset Downstream Port
>   drivers/base/memory: Add cxl_offline_memory() to offline a physical
>     range
>   cxl/core: Add region disable and enable for a Downstream Port reset
>   cxl/core: Collect the CXL regions routed through a Downstream Port
>   PCI/CXL: Register CXL region callbacks for a Downstream Port bus reset
>   PCI/CXL: Clear ACS SV across an SBR of a CXL Downstream Port
>   PCI/DPC: Issue the CXL recovery SBR from DPC without offlining memory
>   PCI/CXL: Add cxl_unmask_sbr to permit an SBR of a CXL Downstream Port
>   PCI/CXL: Refuse an SBR of a CXL Downstream Port without the unmask
>     consent
> 
>  Documentation/ABI/testing/sysfs-bus-pci |  16 +
>  drivers/base/memory.c                   |  36 +++
>  drivers/cxl/core/Makefile               |   2 +-
>  drivers/cxl/core/core.h                 |  28 ++
>  drivers/cxl/core/dport_sbr.c            | 374 ++++++++++++++++++++++++
>  drivers/cxl/core/hdm.c                  | 341 +++++++++++++++++++++
>  drivers/cxl/core/pci.c                  |  19 +-
>  drivers/cxl/core/port.c                 |   4 +-
>  drivers/cxl/core/ras.c                  |  10 +
>  drivers/cxl/core/region.c               |   6 +-
>  drivers/pci/pci-sysfs.c                 |  31 ++
>  drivers/pci/pci.c                       | 254 +++++++++++++++-
>  drivers/pci/pci.h                       |  15 +
>  drivers/pci/pcie/dpc.c                  |  46 ++-
>  include/linux/aer.h                     |   9 +
>  include/linux/memory.h                  |   5 +
>  include/linux/pci.h                     |  16 +
>  include/uapi/linux/pci_regs.h           |   2 +
>  tools/testing/cxl/Kbuild                |   2 +-
>  19 files changed, 1193 insertions(+), 23 deletions(-)
>  create mode 100644 drivers/cxl/core/dport_sbr.c
> 
> -- 
> 2.55.0
> 
>