Re: [PATCH v3 00/17] PCI/P2PDMA: Fix ACS egress control handling
Leon Romanovsky <[email protected]>
| Newsgroups | org.kernel.vger.linux-doc,dev.linux.lists.iommu,org.kernel.vger.linux-kernel,org.kernel.vger.linux-pci |
|---|---|
| Message-ID | <20260818083532.GE66924@unreal> |
On Tue, Aug 11, 2026 at 12:30:42PM +0300, Leon Romanovsky 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. > > 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 also forces > host-bridge routing. > > 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 <[email protected]> > --- > 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/[email protected] > > Changes in v2: > - Added Logan's ROB tags > - Added commas in Documentation patch > - Link to v1: https://patch.msgid.link/[email protected] > > --- > Leon Romanovsky (17): > 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 > > Documentation/admin-guide/kernel-parameters.txt | 9 +- > Documentation/driver-api/pci/p2pdma.rst | 15 + > drivers/iommu/iommu.c | 8 +- > drivers/pci/Kconfig | 15 + > drivers/pci/Makefile | 1 + > drivers/pci/ats.c | 11 +- > drivers/pci/p2pdma.c | 218 +++++-- > drivers/pci/pci.c | 115 +++- > drivers/pci/pci.h | 63 +- > drivers/pci/pci_acs_test.c | 801 ++++++++++++++++++++++++ > drivers/pci/quirks.c | 62 +- > include/linux/pci.h | 30 +- > 12 files changed, 1259 insertions(+), 89 deletions(-) Bjorn, Is there anything you need from me to merge this series after the merge window closes? If so, I would prefer to spend these two weeks fixing or reworking the code. Thanks, > --- > base-commit: 43598807f71ac1c9164f26004acf2496d4038daf > change-id: 20260713-fix-p2p-acs-725f8dd7b0e8 > > Best regards, > -- > Leon Romanovsky <[email protected]> > >