[PATCH v4 00/18] PCI/P2PDMA: Fix ACS egress control handling

Leon Romanovsky posted 18 patches 1 month, 1 week ago
Documentation/admin-guide/kernel-parameters.txt |   9 +-
Documentation/driver-api/pci/p2pdma.rst         |  24 +
drivers/iommu/iommu.c                           |   8 +-
drivers/pci/Kconfig                             |  15 +
drivers/pci/Makefile                            |   1 +
drivers/pci/ats.c                               |  11 +-
drivers/pci/p2pdma.c                            | 496 +++++++++++--
drivers/pci/pci.c                               | 115 ++-
drivers/pci/pci.h                               |  69 +-
drivers/pci/pci_acs_test.c                      | 920 ++++++++++++++++++++++++
drivers/pci/quirks.c                            |  62 +-
include/linux/pci-p2pdma.h                      |   8 +-
include/linux/pci.h                             |  30 +-
13 files changed, 1661 insertions(+), 107 deletions(-)
[PATCH v4 00/18] PCI/P2PDMA: Fix ACS egress control handling
Posted by Leon Romanovsky 1 month, 1 week ago
PCI P2PDMA treats any enabled ACS P2P Egress Control bit as an upstream
redirect. PCIe r7.0, sec 6.12.3, table 6-11 says the Egress Control
Vector bit for the target port decides instead: a clear bit routes a peer
request directly, regardless of P2P Request Redirect. Firmware can
therefore enable Egress Control with a permissive vector while Linux
incorrectly rejects a valid direct P2P path.

Table 6-11, where E is ACS P2P Egress Control Enable, R is ACS P2P
Request Redirect Enable and V the Egress Control Vector bit for the
target port:

  E  R  V  Required Handling for Peer-to-Peer Requests
  -  -  -  ------------------------------------------
  0  0  x  Route directly to peer-to-peer target
  0  1  x  Redirect Upstream
  1  0  1  Handle as an ACS Violation
  1  0  0  Route directly to peer-to-peer target
  1  1  1  Redirect Upstream
  1  1  0  Route directly to peer-to-peer target

P2P Completion Redirect lies outside this table and forces host-bridge
routing when set at the provider-side path divergence.

The same interaction affects target-independent ACS isolation checks.
Request Redirect does not guarantee that peer requests are forwarded
upstream while Egress Control is enabled because a clear vector bit
overrides it. Such checks cannot identify every potential target, so
treat Request Redirect as ineffective while Egress Control is enabled,
which merges the affected devices into one IOMMU group.

ACS Direct Translated P2P routes a Request carrying a Translated address
to the peer regardless of Request Redirect and Egress Control, so it
voids the same guarantee unless Translation Blocking rejects the Request
first.

That last rule holds only for a caller that needs Request Redirect to
isolate peers. pci_enable_pasid() asks for it so that a Request carrying
a PASID reaches the translation agent (sec 2.2.10.4), and a Translated
Request already carries an address the agent produced for that PASID
(sec 10.1.3). pci_acs_enabled() and pci_acs_path_enabled() therefore
take a scope, and Direct Translated P2P applies only to
PCI_ACS_SCOPE_ALL.

A pre-existing gap comes first. The routing analysis covers only Requests
carrying an Untranslated address; ACS Direct Translated P2P overrides
those controls, so that scope is now written down rather than implied.

It is nearly impossible to test all possible combinations due to limited
hardware availability, so I added KUnit coverage for ACS routing
decisions, isolation checks, Egress Control Vector lookups, and
provider-to-client path traversal over a fabricated PCIe fabric.

Disclaimer:
All patches were prepared with AI assistance, with a significant
difference between the code changes and the KUnit tests. The code
changes were thoroughly reviewed and rewritten.

In contrast, the KUnit patches were produced entirely by AI with
minimal human interaction, and multiple AI tools (Claude, Codex,
and Gemini) with frontier models were used to verify that the tests
comply with the PCI specification.

Thanks

Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
Changes in v4:
- Added debug prints (we can drop it) patch which is very useful for automatic
  root cause analysis. Just feed the output of these prints, together
  with topology and kernel boot command line to your favorite LLM and it
  will give you reliable RCA why ACS didn't work.
- Reject ACS Violations and unreadable routing state instead of treating
  them as host-bridge redirects
- Added Tested-by tags from Tushar Dave
- Added support to asymmetric ACS routing
- Limited redirect checks to the two ports at the path divergence
- Added standalone ACS routing diagnostics for hardware retesting
- Link to v3: https://patch.msgid.link/20260811-fix-p2p-acs-v3-0-efc488ee7c03@nvidia.com

Changes in v3:
- Fixed pci_p2pdma_add_resource() error unwinding
- Made pdev->p2pdma teardown wait unconditionally for RCU readers
- Restricted pci_p2pmem_find_many() to pool-backed providers
- Documented the pdev->p2pdma lifetime and RCU rules
- Fixed calc_map_type_and_dist() handling of the verbose argument
- Required the ACS port and target to share a bus before indexing the
  Egress Control Vector
- Gave pci_acs_enabled() and pci_acs_path_enabled() a scope, so the ACS
  Direct Translated P2P rule no longer stops pci_enable_pasid() from
  enabling PASID
- Dropped "Report ACS ports when the paths share no upstream bridge":
  the mapping type cannot change without a shared upstream bridge, so
  the pci=disable_acs_redir= hint was not actionable there and the ACS
  walk only cost config space reads
- Folded the Request Redirect rule into pci_acs_rr_ineffective(), so
  pci_acs_flags_enabled() and the Intel SPT PCH quirk share one copy
- Renamed pci_acs_egress_ctrl_set() to pci_acs_egress_ctrl_is_set(), it
  reads the bit rather than setting it
- Reworded the blocked-path warning: ACS may also leave the direct route
  indeterminate rather than blocked
- Added KUnit coverage for the shared-bus guard, a device with no ACS
  capability and an unreadable ACS Control register
- Added the missing Fixes: tags, a second one on the
  pci_p2pdma_add_resource() unwinding fix (the dangling devres action
  dates to f58ef9d1d135) and one on the Egress Control isolation change
- Link to v2: https://patch.msgid.link/20260806-fix-p2p-acs-v2-0-0cec14812965@nvidia.com

Changes in v2:
- Added Logan's ROB tags
- Added commas in Documentation patch
- Link to v1: https://patch.msgid.link/20260802-fix-p2p-acs-v1-0-a7c5eb64fff6@nvidia.com

---
Leon Romanovsky (18):
      PCI/P2PDMA: Do not tear down the allocate attribute on registration failure
      PCI/P2PDMA: Wait for RCU readers before freeing state
      PCI/P2PDMA: Restrict the p2pmem search to pool backed providers
      PCI/P2PDMA: Safely terminate ACS redirect lists
      PCI/P2PDMA: Document the pdev->p2pdma lifetime and RCU rules
      PCI/P2PDMA: Gate the host bridge whitelist warning on verbose
      PCI/P2PDMA: Document the Address Type assumption
      PCI: Account for Direct Translated P2P in ACS isolation checks
      PCI: Add ACS egress control vector accessor
      PCI: Account for ACS egress control in isolation checks
      PCI/P2PDMA: Derive peer-to-peer routing from ACS control bits
      PCI/P2PDMA: Honor ACS egress control vectors
      PCI/P2PDMA: Document ACS egress control handling
      PCI/P2PDMA: Extract pure ACS routing decision helpers
      PCI/P2PDMA: Add KUnit tests for ACS routing decisions
      PCI/P2PDMA: Add KUnit coverage for the ACS P2P routing walk
      PCI: Add KUnit coverage for ACS isolation checks
      PCI/P2PDMA: Log detailed ACS routing diagnostics

 Documentation/admin-guide/kernel-parameters.txt |   9 +-
 Documentation/driver-api/pci/p2pdma.rst         |  24 +
 drivers/iommu/iommu.c                           |   8 +-
 drivers/pci/Kconfig                             |  15 +
 drivers/pci/Makefile                            |   1 +
 drivers/pci/ats.c                               |  11 +-
 drivers/pci/p2pdma.c                            | 496 +++++++++++--
 drivers/pci/pci.c                               | 115 ++-
 drivers/pci/pci.h                               |  69 +-
 drivers/pci/pci_acs_test.c                      | 920 ++++++++++++++++++++++++
 drivers/pci/quirks.c                            |  62 +-
 include/linux/pci-p2pdma.h                      |   8 +-
 include/linux/pci.h                             |  30 +-
 13 files changed, 1661 insertions(+), 107 deletions(-)
---
base-commit: 43598807f71ac1c9164f26004acf2496d4038daf
change-id: 20260821-fix-p2p-acs-v4-0-e72455e3a261

Best regards,
--  
Leon Romanovsky <leonro@nvidia.com>
Re: [PATCH v4 00/18] PCI/P2PDMA: Fix ACS egress control handling
Posted by Jason Gunthorpe 1 month ago
> PCI P2PDMA treats any enabled ACS P2P Egress Control bit as an upstream
> redirect. PCIe r7.0, sec 6.12.3, table 6-11 says the Egress Control
> Vector bit for the target port decides instead: a clear bit routes a peer
> request directly, regardless of P2P Request Redirect. Firmware can
> therefore enable Egress Control with a permissive vector while Linux
> incorrectly rejects a valid direct P2P path.

I've never seen anyone use the egress control vector and broadly Linux
doesn't support it. The ACS command line shouldn't enable "P2P Egress
Control Enable" for this reason.

It is not a bad thing to accommodate the egress vector when improving
the ACS logic, but the main stream usage is the interaction of the
other bits along with ATS & RO in the TLP. See the comment I left a
long time ago:

https://elixir.bootlin.com/linux/v7.2/source/drivers/infiniband/hw/mlx5/mlx5_ib.h#L1649

So it would be nicer to read in the commit message how the mainstream
stuff is fixed up and just a little bit about egress control.

> [ ... 36 lines skipped ... ]
> A pre-existing gap comes first. The routing analysis covers only Requests
> carrying an Untranslated address; ACS Direct Translated P2P overrides
> those controls, so that scope is now written down rather than implied.

What I talked about with Thomas is we probably need the P2P subsystem
to know what kind of TLP the driver intends to put here when doing the
evaluation: strict order, relaxed order and translated all have
different possible routing options, and real system configure things
so each one takes a different path :\

Currently I think the P2P subsystem is assuming strict order
non-translated TLPs when it makes its calculations. Which is fine, but
as we go toward enhancing this each of the different paths should be
kept seperate.

I don't know how the driver facing API should work, but at least real
devices have options to use ATS or not, use RO or not, and can make
use of information from the P2P subsytem to make the right choice.

Further, when we get to things like an ACPI description of this stuff,
it would be nice to still discover these differences as well.

-- 
Jason
Re: [PATCH v4 00/18] PCI/P2PDMA: Fix ACS egress control handling
Posted by Leon Romanovsky 4 weeks, 1 day ago
On Mon, Aug 24, 2026 at 08:22:50PM -0300, Jason Gunthorpe wrote:
> > PCI P2PDMA treats any enabled ACS P2P Egress Control bit as an upstream
> > redirect. PCIe r7.0, sec 6.12.3, table 6-11 says the Egress Control
> > Vector bit for the target port decides instead: a clear bit routes a peer
> > request directly, regardless of P2P Request Redirect. Firmware can
> > therefore enable Egress Control with a permissive vector while Linux
> > incorrectly rejects a valid direct P2P path.
> 
> I've never seen anyone use the egress control vector and broadly Linux
> doesn't support it. The ACS command line shouldn't enable "P2P Egress
> Control Enable" for this reason.

I tried to follow the PCI specification as closely as possible here, but
of course I always welcome the idea of eliminating one of the paths.

> 
> It is not a bad thing to accommodate the egress vector when improving
> the ACS logic, but the main stream usage is the interaction of the
> other bits along with ATS & RO in the TLP. See the comment I left a
> long time ago:
> 
> https://elixir.bootlin.com/linux/v7.2/source/drivers/infiniband/hw/mlx5/mlx5_ib.h#L1649
> 
> So it would be nicer to read in the commit message how the mainstream
> stuff is fixed up and just a little bit about egress control.

I will split the series into bug-fix patches and code improvements.
This should also help describe the purpose of the series more clearly.

> 
> > [ ... 36 lines skipped ... ]
> > A pre-existing gap comes first. The routing analysis covers only Requests
> > carrying an Untranslated address; ACS Direct Translated P2P overrides
> > those controls, so that scope is now written down rather than implied.
> 
> What I talked about with Thomas is we probably need the P2P subsystem
> to know what kind of TLP the driver intends to put here when doing the
> evaluation: strict order, relaxed order and translated all have
> different possible routing options, and real system configure things
> so each one takes a different path :\
> 
> Currently I think the P2P subsystem is assuming strict order
> non-translated TLPs when it makes its calculations. Which is fine, but
> as we go toward enhancing this each of the different paths should be
> kept seperate.
> 
> I don't know how the driver facing API should work, but at least real
> devices have options to use ATS or not, use RO or not, and can make
> use of information from the P2P subsytem to make the right choice.

Do you see a function like mlx5_umem_needs_ats() being implemented as part of
the PCI P2P logic?
https://lore.kernel.org/all/4-v1-bd147097458e+ede-umem_dmabuf_jgg@nvidia.com/

> 
> Further, when we get to things like an ACPI description of this stuff,
> it would be nice to still discover these differences as well.
> 
> -- 
> Jason
Re: [PATCH v4 00/18] PCI/P2PDMA: Fix ACS egress control handling
Posted by Jason Gunthorpe 3 weeks, 6 days ago
On Sun, Aug 30, 2026 at 12:02:23PM +0300, Leon Romanovsky wrote:
> > I don't know how the driver facing API should work, but at least real
> > devices have options to use ATS or not, use RO or not, and can make
> > use of information from the P2P subsytem to make the right choice.
> 
> Do you see a function like mlx5_umem_needs_ats() being implemented as part of
> the PCI P2P logic?
> https://lore.kernel.org/all/4-v1-bd147097458e+ede-umem_dmabuf_jgg@nvidia.com/

Maybe someday, virtualization makes it hard since the information
needed gets wiped away, but lets start by imagining bare metal working
right.

Ideally	if the P2P route requires ATS then the driver should know and
it should setup ATS. Similarly if it doesn't then it shouldn't. The
only way to know is to evaluate the paths through the ACS controls to
figure what works.

Jason