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

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

[Leon Romanovsky]

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>

[PATCH v4 01/18] PCI/P2PDMA: Do not tear down the allocate attribute on registration failure

[Leon Romanovsky]

From: Leon Romanovsky <leonro@nvidia.com>

pci_p2pdma_add_resource() installs pci_p2pdma_unmap_mappings() as a devres
action with the devres allocated p2p_pgmap as its data, and only then adds
the range to the pool:

	error = devm_add_action_or_reset(&pdev->dev, pci_p2pdma_unmap_mappings,
					 p2p_pgmap);
	if (error)
		goto pages_free;

	p2pdma = rcu_dereference_protected(pdev->p2pdma, 1);
	error = gen_pool_add_owner(p2pdma->pool, ...);
	if (error)
		goto pages_free;

The action removes the allocate attribute for the whole device, which
tears down existing userspace mappings of every BAR already registered on
it. Both failures here get that wrong, in opposite ways.

devm_add_action_or_reset() runs the action when it cannot allocate its
devres node, so an -ENOMEM while registering a second BAR unmaps the
first one. Use devm_add_action() and let the error path unwind only what
this call created.

gen_pool_add_owner() allocates a chunk and can also fail with -ENOMEM.
There the action is registered, and the error path frees p2p_pgmap with
devm_kfree() while leaving the action pointing at it. On unbind devres
runs the action and pci_p2pdma_unmap_mappings() dereferences
p2p_pgmap->mem->owner->kobj, which is freed memory. Give that failure its
own label and drop the action with devm_remove_action(), which removes it
without running it.

Tested-by: Tushar Dave <tdave@nvidia.com>
Fixes: 7e9c7ef83d78 ("PCI/P2PDMA: Allow userspace VMA allocations through sysfs")
Fixes: f58ef9d1d135 ("PCI/P2PDMA: Separate the mmap() support from the core logic")
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 drivers/pci/p2pdma.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
index b2d5266f8653..dc7aaa990fed 100644
--- a/drivers/pci/p2pdma.c
+++ b/drivers/pci/p2pdma.c
@@ -440,8 +440,8 @@ int pci_p2pdma_add_resource(struct pci_dev *pdev, int bar, size_t size,
 		goto pgmap_free;
 	}
 
-	error = devm_add_action_or_reset(&pdev->dev, pci_p2pdma_unmap_mappings,
-					 p2p_pgmap);
+	error = devm_add_action(&pdev->dev, pci_p2pdma_unmap_mappings,
+				p2p_pgmap);
 	if (error)
 		goto pages_free;
 
@@ -451,13 +451,15 @@ int pci_p2pdma_add_resource(struct pci_dev *pdev, int bar, size_t size,
 			range_len(&pgmap->range), dev_to_node(&pdev->dev),
 			&pgmap->ref);
 	if (error)
-		goto pages_free;
+		goto mappings_remove;
 
 	pci_info(pdev, "added peer-to-peer DMA memory %#llx-%#llx\n",
 		 pgmap->range.start, pgmap->range.end);
 
 	return 0;
 
+mappings_remove:
+	devm_remove_action(&pdev->dev, pci_p2pdma_unmap_mappings, p2p_pgmap);
 pages_free:
 	devm_memunmap_pages(&pdev->dev, pgmap);
 pgmap_free:

-- 
2.55.0

Re: [PATCH v4 01/18] PCI/P2PDMA: Do not tear down the allocate attribute on registration failure

[Logan Gunthorpe]

On 2026-08-21 13:38, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@nvidia.com>
> 
> pci_p2pdma_add_resource() installs pci_p2pdma_unmap_mappings() as a devres
> action with the devres allocated p2p_pgmap as its data, and only then adds
> the range to the pool:
> 
> 	error = devm_add_action_or_reset(&pdev->dev, pci_p2pdma_unmap_mappings,
> 					 p2p_pgmap);
> 	if (error)
> 		goto pages_free;
> 
> 	p2pdma = rcu_dereference_protected(pdev->p2pdma, 1);
> 	error = gen_pool_add_owner(p2pdma->pool, ...);
> 	if (error)
> 		goto pages_free;
> 
> The action removes the allocate attribute for the whole device, which
> tears down existing userspace mappings of every BAR already registered on
> it. Both failures here get that wrong, in opposite ways.
> 
> devm_add_action_or_reset() runs the action when it cannot allocate its
> devres node, so an -ENOMEM while registering a second BAR unmaps the
> first one. Use devm_add_action() and let the error path unwind only what
> this call created.
> 
> gen_pool_add_owner() allocates a chunk and can also fail with -ENOMEM.
> There the action is registered, and the error path frees p2p_pgmap with
> devm_kfree() while leaving the action pointing at it. On unbind devres
> runs the action and pci_p2pdma_unmap_mappings() dereferences
> p2p_pgmap->mem->owner->kobj, which is freed memory. Give that failure its
> own label and drop the action with devm_remove_action(), which removes it
> without running it.
> 
> Tested-by: Tushar Dave <tdave@nvidia.com>
> Fixes: 7e9c7ef83d78 ("PCI/P2PDMA: Allow userspace VMA allocations through sysfs")
> Fixes: f58ef9d1d135 ("PCI/P2PDMA: Separate the mmap() support from the core logic")
> Signed-off-by: Leon Romanovsky <leonro@nvidia.com>

Makes sense to me:

Reviewed-by: Logan Gunthorpe <logang@deltatee.com>

Re: [PATCH v4 01/18] PCI/P2PDMA: Do not tear down the allocate attribute on registration failure

[Jason Gunthorpe]

> [ ... 18 lines skipped ... ]
> devm_add_action_or_reset() runs the action when it cannot allocate its
> devres node, so an -ENOMEM while registering a second BAR unmaps the
> first one. Use devm_add_action() and let the error path unwind only what
> this call created.

It is so weird we have multiple devres's removing the single shared
sysfs function.. I guess the secondary ones are all NOPs? Oh well,
this is an improvement:

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

-- 
Jason

[PATCH v4 02/18] PCI/P2PDMA: Wait for RCU readers before freeing state

[Leon Romanovsky]

From: Leon Romanovsky <leonro@nvidia.com>

pci_p2pmem_find_many() scans all PCI devices without locking or
protection against driver unbind, including devices with poolless
P2PDMA state. pci_has_p2pmem() may observe pdev->p2pdma just before
driver unbind clears it, while pci_p2pdma_release() skips the grace
period when no pool is present. This allows devres to free the object
while it is still in use.

Clear the pointer with RCU_INIT_POINTER() and always wait for pre-existing
RCU readers before returning. The same grace period continues to protect
gen_pool users for pool-backed providers.

Tested-by: Tushar Dave <tdave@nvidia.com>
Cc: Alex Williamson <alex@shazbot.org>
Cc: Matt Evans <matt@ozlabs.org>
Fixes: 372d6d1b8ae3 ("PCI/P2PDMA: Refactor to separate core P2P functionality from memory allocation")
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 drivers/pci/p2pdma.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
index dc7aaa990fed..e8e8c7d81d22 100644
--- a/drivers/pci/p2pdma.c
+++ b/drivers/pci/p2pdma.c
@@ -236,9 +236,8 @@ static void pci_p2pdma_release(void *data)
 		return;
 
 	/* Flush and disable pci_alloc_p2p_mem() */
-	pdev->p2pdma = NULL;
-	if (p2pdma->pool)
-		synchronize_rcu();
+	RCU_INIT_POINTER(pdev->p2pdma, NULL);
+	synchronize_rcu();
 	xa_destroy(&p2pdma->map_types);
 
 	if (!p2pdma->pool)

-- 
2.55.0

Re: [PATCH v4 02/18] PCI/P2PDMA: Wait for RCU readers before freeing state

[Logan Gunthorpe]

On 2026-08-21 13:38, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@nvidia.com>
> 
> pci_p2pmem_find_many() scans all PCI devices without locking or
> protection against driver unbind, including devices with poolless
> P2PDMA state. pci_has_p2pmem() may observe pdev->p2pdma just before
> driver unbind clears it, while pci_p2pdma_release() skips the grace
> period when no pool is present. This allows devres to free the object
> while it is still in use.
> 
> Clear the pointer with RCU_INIT_POINTER() and always wait for pre-existing
> RCU readers before returning. The same grace period continues to protect
> gen_pool users for pool-backed providers.
> 
> Tested-by: Tushar Dave <tdave@nvidia.com>
> Cc: Alex Williamson <alex@shazbot.org>
> Cc: Matt Evans <matt@ozlabs.org>
> Fixes: 372d6d1b8ae3 ("PCI/P2PDMA: Refactor to separate core P2P functionality from memory allocation")
> Signed-off-by: Leon Romanovsky <leonro@nvidia.com>

Sounds fine:

Reviewed-by: Logan Gunthorpe <logang@deltatee.com>

Re: [PATCH v4 02/18] PCI/P2PDMA: Wait for RCU readers before freeing state

[Jason Gunthorpe]

On Fri, 21 Aug 2026 22:38:33 +0300, Leon Romanovsky <leon@kernel.org> wrote:
> pci_p2pmem_find_many() scans all PCI devices without locking or
> protection against driver unbind, including devices with poolless
> P2PDMA state. pci_has_p2pmem() may observe pdev->p2pdma just before
> driver unbind clears it, while pci_p2pdma_release() skips the grace
> period when no pool is present. This allows devres to free the object
> while it is still in use.
> 
> [...]

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

-- 
Jason

[PATCH v4 03/18] PCI/P2PDMA: Restrict the p2pmem search to pool backed providers

[Leon Romanovsky]

From: Leon Romanovsky <leonro@nvidia.com>

pci_p2pmem_find_many() exists to pick a provider that the caller will
then allocate from with pci_alloc_p2pmem(), which goes straight to the
gen_pool:

	ret = (void *)gen_pool_alloc_owner(p2pdma->pool, size, (void **) &ref);

pci_has_p2pmem() does not ask for that pool, only for the published flag.
The two used to be equivalent, because a provider could only exist by way
of pci_p2pdma_add_resource(), which always creates the pool.

pcim_p2pdma_init() broke that. It registers a provider for the DMABUF
path and never creates a pool, so pdev->p2pdma is set while
p2pdma->pool stays NULL. Nothing publishes such a provider today, so the
search cannot return one yet, but the flag alone no longer says what the
caller needs.

Ask for the pool as well, so the search covers the providers its result
is used for. A later patch documents the pdev->p2pdma lifetime and RCU
rules.

Tested-by: Tushar Dave <tdave@nvidia.com>
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 drivers/pci/p2pdma.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
index e8e8c7d81d22..6618ef170ce1 100644
--- a/drivers/pci/p2pdma.c
+++ b/drivers/pci/p2pdma.c
@@ -863,7 +863,12 @@ static bool pci_has_p2pmem(struct pci_dev *pdev)
 
 	rcu_read_lock();
 	p2pdma = rcu_dereference(pdev->p2pdma);
-	res = p2pdma && p2pdma->p2pmem_published;
+	/*
+	 * The callers hand the result to pci_alloc_p2pmem(), so only a
+	 * provider backed by a pool is of any use here. pcim_p2pdma_init()
+	 * creates providers without one.
+	 */
+	res = p2pdma && p2pdma->pool && p2pdma->p2pmem_published;
 	rcu_read_unlock();
 
 	return res;

-- 
2.55.0

Re: [PATCH v4 03/18] PCI/P2PDMA: Restrict the p2pmem search to pool backed providers

[Logan Gunthorpe]

On 2026-08-21 13:38, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@nvidia.com>
> 
> pci_p2pmem_find_many() exists to pick a provider that the caller will
> then allocate from with pci_alloc_p2pmem(), which goes straight to the
> gen_pool:
> 
> 	ret = (void *)gen_pool_alloc_owner(p2pdma->pool, size, (void **) &ref);
> 
> pci_has_p2pmem() does not ask for that pool, only for the published flag.
> The two used to be equivalent, because a provider could only exist by way
> of pci_p2pdma_add_resource(), which always creates the pool.
> 
> pcim_p2pdma_init() broke that. It registers a provider for the DMABUF
> path and never creates a pool, so pdev->p2pdma is set while
> p2pdma->pool stays NULL. Nothing publishes such a provider today, so the
> search cannot return one yet, but the flag alone no longer says what the
> caller needs.
> 
> Ask for the pool as well, so the search covers the providers its result
> is used for. A later patch documents the pdev->p2pdma lifetime and RCU
> rules.
> 
> Tested-by: Tushar Dave <tdave@nvidia.com>
> Signed-off-by: Leon Romanovsky <leonro@nvidia.com>

Makes sense to me:

Reviewed-by: Logan Gunthorpe <logang@deltatee.com>

Re: [PATCH v4 03/18] PCI/P2PDMA: Restrict the p2pmem search to pool backed providers

[Jason Gunthorpe]

On Fri, 21 Aug 2026 22:38:34 +0300, Leon Romanovsky <leon@kernel.org> wrote:
> pci_p2pmem_find_many() exists to pick a provider that the caller will
> then allocate from with pci_alloc_p2pmem(), which goes straight to the
> gen_pool:
> 
> 	ret = (void *)gen_pool_alloc_owner(p2pdma->pool, size, (void **) &ref);
> 
> pci_has_p2pmem() does not ask for that pool, only for the published flag.
> The two used to be equivalent, because a provider could only exist by way
> of pci_p2pdma_add_resource(), which always creates the pool.
> 
> [...]

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

-- 
Jason

[PATCH v4 04/18] PCI/P2PDMA: Safely terminate ACS redirect lists

[Leon Romanovsky]

From: Leon Romanovsky <leonro@nvidia.com>

seq_buf marks an overflow by setting len to size + 1. The ACS diagnostic
path unconditionally writes a terminator to buffer[len - 1], so a path
with enough ACS ports to fill the 128-byte buffer writes one byte beyond
the buffer when verbose diagnostics are requested.

Use seq_buf_str() to terminate truncated output safely and remove the final
semicolon only when the buffer did not overflow.

Tested-by: Tushar Dave <tdave@nvidia.com>
Fixes: 52916982af48 ("PCI/P2PDMA: Support peer-to-peer memory")
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 drivers/pci/p2pdma.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
index 6618ef170ce1..a77ef9deb3c6 100644
--- a/drivers/pci/p2pdma.c
+++ b/drivers/pci/p2pdma.c
@@ -767,11 +767,13 @@ calc_map_type_and_dist(struct pci_dev *provider, struct pci_dev *client,
 	}
 
 	if (verbose) {
-		acs_list.buffer[acs_list.len-1] = 0; /* drop final semicolon */
+		/* Drop the final semicolon; the list is not empty here. */
+		if (!seq_buf_has_overflowed(&acs_list))
+			acs_list.buffer[acs_list.len - 1] = '\0';
 		pci_warn(client, "ACS redirect is set between the client and provider (%s)\n",
 			 pci_name(provider));
 		pci_warn(client, "to disable ACS redirect for this path, add the kernel parameter: pci=disable_acs_redir=%s\n",
-			 acs_list.buffer);
+			 seq_buf_str(&acs_list));
 	}
 	acs_redirects = true;
 

-- 
2.55.0

Re: [PATCH v4 04/18] PCI/P2PDMA: Safely terminate ACS redirect lists

[Jason Gunthorpe]

On Fri, 21 Aug 2026 22:38:35 +0300, Leon Romanovsky <leon@kernel.org> wrote:
> seq_buf marks an overflow by setting len to size + 1. The ACS diagnostic
> path unconditionally writes a terminator to buffer[len - 1], so a path
> with enough ACS ports to fill the 128-byte buffer writes one byte beyond
> the buffer when verbose diagnostics are requested.
> 
> Use seq_buf_str() to terminate truncated output safely and remove the final
> semicolon only when the buffer did not overflow.
> 
> [...]

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

-- 
Jason

[PATCH v4 05/18] PCI/P2PDMA: Document the pdev->p2pdma lifetime and RCU rules

[Leon Romanovsky]

From: Leon Romanovsky <leonro@nvidia.com>

pdev->p2pdma has two lifetime models. Provider-based entry points are
quiesced by their driver before remove completes. pci_p2pmem_find_many()
and the p2pmem sysfs attributes can race with unbind and therefore rely
on the teardown grace period.

Document publication, teardown, and how the grace period protects both
the struct pci_p2pdma object and its optional gen_pool.

Tested-by: Tushar Dave <tdave@nvidia.com>
Cc: Alex Williamson <alex@shazbot.org>
Cc: Matt Evans <matt@ozlabs.org>
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 drivers/pci/p2pdma.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 53 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
index a77ef9deb3c6..49bc8cf06240 100644
--- a/drivers/pci/p2pdma.c
+++ b/drivers/pci/p2pdma.c
@@ -21,6 +21,39 @@
 #include <linux/seq_buf.h>
 #include <linux/xarray.h>
 
+/*
+ * Lifetime and RCU usage
+ *
+ * Within one driver bind, pdev->p2pdma is published exactly once, by
+ * pcim_p2pdma_init(), and cleared exactly once, by the pci_p2pdma_release()
+ * devres action that the same function installs. It is never re-pointed at a
+ * second struct pci_p2pdma, so a reader that observes a non-NULL pointer
+ * always observes the same, fully initialised object. That object is devres
+ * memory allocated before the action is installed, so devres frees it only
+ * after pci_p2pdma_release() has returned.
+ *
+ * Most exported entry points reach pdev->p2pdma through a struct pci_dev or a
+ * struct p2pdma_provider owned by the provider driver, and
+ * pcim_p2pdma_provider() requires callers to drop those references before the
+ * driver's remove() completes. Those cannot run concurrently with
+ * pci_p2pdma_release(), and their rcu_dereference() calls are simply how an
+ * __rcu pointer is read.
+ *
+ * pci_p2pmem_find_many() and the p2pmem sysfs attributes are the exceptions.
+ * The first walks every PCI device, so it can reach a provider whose driver is
+ * unbinding: pci_get_device() pins the struct pci_dev, not the driver. The
+ * second is reachable from userspace until sysfs_remove_group() runs at the end
+ * of the release. pci_has_p2pmem() must dereference the object to determine
+ * whether it owns a gen_pool, so even a poolless object must remain alive until
+ * that RCU reader exits. The sysfs group is created with the pool.
+ *
+ * The grace period in pci_p2pdma_release() first protects the struct
+ * pci_p2pdma itself from being freed while pci_has_p2pmem() is using it. For a
+ * pool-backed provider it also fences the gen_pool: gen_pool_alloc_owner()
+ * walks pool->chunks under RCU and gen_pool_destroy() frees those chunks
+ * without waiting for a grace period of its own, so pci_alloc_p2pmem() and
+ * p2pmem_alloc_mmap() hold rcu_read_lock() across the allocation.
+ */
 struct pci_p2pdma {
 	struct gen_pool *pool;
 	bool p2pmem_published;
@@ -235,9 +268,19 @@ static void pci_p2pdma_release(void *data)
 	if (!p2pdma)
 		return;
 
-	/* Flush and disable pci_alloc_p2p_mem() */
+	/*
+	 * Stop new RCU readers and wait for readers that observed p2pdma before
+	 * allowing devres to free it. This is required even without a pool,
+	 * because pci_has_p2pmem() dereferences every non-NULL p2pdma it finds.
+	 * For a pool-backed provider this also fences gen_pool_destroy().
+	 */
 	RCU_INIT_POINTER(pdev->p2pdma, NULL);
 	synchronize_rcu();
+
+	/*
+	 * The grace period also ensures no RCU reader can still be accessing
+	 * map_types here.
+	 */
 	xa_destroy(&p2pdma->map_types);
 
 	if (!p2pdma->pool)
@@ -255,6 +298,9 @@ static void pci_p2pdma_release(void *data)
  * for a PCI device. It allocates and sets up the necessary data
  * structures to support P2PDMA operations, including mapping type
  * tracking.
+ *
+ * The state is published once per driver bind and torn down by a devres
+ * action on unbind. Repeated calls for the same device are a no-op.
  */
 int pcim_p2pdma_init(struct pci_dev *pdev)
 {
@@ -786,6 +832,12 @@ calc_map_type_and_dist(struct pci_dev *provider, struct pci_dev *client,
 		map_type = PCI_P2PDMA_MAP_NOT_SUPPORTED;
 	}
 done:
+	/*
+	 * pci_p2pmem_find_many() reaches this with a provider whose driver may
+	 * be unbinding, so the store runs under RCU: pci_p2pdma_release()
+	 * clears the pointer and waits for readers before destroying
+	 * map_types. See "Lifetime and RCU usage" above.
+	 */
 	rcu_read_lock();
 	p2pdma = rcu_dereference(provider->p2pdma);
 	if (p2pdma)

-- 
2.55.0

I

[Logan Gunthorpe]

On 2026-08-21 13:38, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@nvidia.com>
> 
> pdev->p2pdma has two lifetime models. Provider-based entry points are
> quiesced by their driver before remove completes. pci_p2pmem_find_many()
> and the p2pmem sysfs attributes can race with unbind and therefore rely
> on the teardown grace period.
> 
> Document publication, teardown, and how the grace period protects both
> the struct pci_p2pdma object and its optional gen_pool.
> 
> Tested-by: Tushar Dave <tdave@nvidia.com>
> Cc: Alex Williamson <alex@shazbot.org>
> Cc: Matt Evans <matt@ozlabs.org>
> Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
> ---
>  drivers/pci/p2pdma.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 53 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
> index a77ef9deb3c6..49bc8cf06240 100644
> --- a/drivers/pci/p2pdma.c
> +++ b/drivers/pci/p2pdma.c
> @@ -21,6 +21,39 @@
>  #include <linux/seq_buf.h>
>  #include <linux/xarray.h>
>  
> +/*
> + * Lifetime and RCU usage
> + *
> + * Within one driver bind, pdev->p2pdma is published exactly once, by

Is published the right verb here? Seems like we use published for
different purposes in p2pdma and calling pcim_p2pdma_init() publishing
reads strangely.

> + * pcim_p2pdma_init(), and cleared exactly once, by the pci_p2pdma_release()
> + * devres action that the same function installs. It is never re-pointed at a

"It is never re-pointed at" is some strange wording. I had to read it a
few times to understand what it is saying.

> + * second struct pci_p2pdma, so a reader that observes a non-NULL pointer
> + * always observes the same, fully initialised object. That object is devres
> + * memory allocated before the action is installed, so devres frees it only
> + * after pci_p2pdma_release() has returned.

I don't quite follow the point of this paragraph. It's like it's
building to some kind of gotcha, but all it seems to be saying is is the
life cycle of pdev->p2pdma is the same as the lifecycle of pdev.

> + * Most exported entry points reach pdev->p2pdma through a struct pci_dev or a
> + * struct p2pdma_provider owned by the provider driver, and
> + * pcim_p2pdma_provider() requires callers to drop those references before the
> + * driver's remove() completes. Those cannot run concurrently with
> + * pci_p2pdma_release(), and their rcu_dereference() calls are simply how an
> + * __rcu pointer is read.
> + *
> + * pci_p2pmem_find_many() and the p2pmem sysfs attributes are the exceptions.
> + * The first walks every PCI device, so it can reach a provider whose driver is
> + * unbinding: pci_get_device() pins the struct pci_dev, not the driver. The
> + * second is reachable from userspace until sysfs_remove_group() runs at the end
> + * of the release. pci_has_p2pmem() must dereference the object to determine
> + * whether it owns a gen_pool, so even a poolless object must remain alive until

Maybe a hyphen with pool-less or maybe better to use plain language: "so
even a device without a pool must remain alive..."

> + * that RCU reader exits. The sysfs group is created with the pool.
> + *
> + * The grace period in pci_p2pdma_release() first protects the struct
> + * pci_p2pdma itself from being freed while pci_has_p2pmem() is using it. For a
> + * pool-backed provider it also fences the gen_pool: gen_pool_alloc_owner()
> + * walks pool->chunks under RCU and gen_pool_destroy() frees those chunks
> + * without waiting for a grace period of its own, so pci_alloc_p2pmem() and
> + * p2pmem_alloc_mmap() hold rcu_read_lock() across the allocation.
> + */
>  struct pci_p2pdma {
>  	struct gen_pool *pool;
>  	bool p2pmem_published;
> @@ -235,9 +268,19 @@ static void pci_p2pdma_release(void *data)
>  	if (!p2pdma)
>  		return;
>  
> -	/* Flush and disable pci_alloc_p2p_mem() */
> +	/*
> +	 * Stop new RCU readers and wait for readers that observed p2pdma before
> +	 * allowing devres to free it. This is required even without a pool,
> +	 * because pci_has_p2pmem() dereferences every non-NULL p2pdma it finds.
> +	 * For a pool-backed provider this also fences gen_pool_destroy().
> +	 */
>  	RCU_INIT_POINTER(pdev->p2pdma, NULL);
>  	synchronize_rcu();
> +
> +	/*
> +	 * The grace period also ensures no RCU reader can still be accessing
> +	 * map_types here.
> +	 */
>  	xa_destroy(&p2pdma->map_types);
>  
>  	if (!p2pdma->pool)
> @@ -255,6 +298,9 @@ static void pci_p2pdma_release(void *data)
>   * for a PCI device. It allocates and sets up the necessary data
>   * structures to support P2PDMA operations, including mapping type
>   * tracking.
> + *
> + * The state is published once per driver bind and torn down by a devres

I still find the use of "published" here a bit odd and I'm not sure what
"state" it is referring to.

> + * action on unbind. Repeated calls for the same device are a no-op.
>   */
>  int pcim_p2pdma_init(struct pci_dev *pdev)
>  {
> @@ -786,6 +832,12 @@ calc_map_type_and_dist(struct pci_dev *provider, struct pci_dev *client,
>  		map_type = PCI_P2PDMA_MAP_NOT_SUPPORTED;
>  	}
>  done:
> +	/*
> +	 * pci_p2pmem_find_many() reaches this with a provider whose driver may
> +	 * be unbinding, so the store runs under RCU: pci_p2pdma_release()
> +	 * clears the pointer and waits for readers before destroying
> +	 * map_types. See "Lifetime and RCU usage" above.
> +	 */

I don't know, but this seems like we're just describing basic RCU usage
here. I'm not sure I personally find much value in the comment.

>  	rcu_read_lock();
>  	p2pdma = rcu_dereference(provider->p2pdma);
>  	if (p2pdma)
> 

Thanks,

Logan

[PATCH v4 06/18] PCI/P2PDMA: Gate the host bridge whitelist warning on verbose

[Leon Romanovsky]

From: Leon Romanovsky <leonro@nvidia.com>

calc_map_type_and_dist() prints every other diagnostic under its verbose
argument, but reaches the "Host bridge not in P2PDMA whitelist" warning
through host_bridge_whitelist(), which it hands acs_redirects instead.
A caller that asked for a silent answer still gets the warning whenever
any port on the path has an ACS redirect bit set, the CPU is not
whitelisted by cpu_supports_p2pdma(), and the host bridge is not in
pci_p2pdma_whitelist[].

pci_p2pmem_find_many() is such a caller. It sweeps every device with
published p2pmem and asks for the distance to each client with
verbose=false, and pci_p2pdma_distance_many() recomputes rather than
consulting the map_types cache, so the warning repeats on every sweep.

The argument was never meant to say "ACS redirects were found". When
commit cf201bfe8cdc ("PCI/P2PDMA: Warn if host bridge not in whitelist")
added it, acs_redirects was a bool pointer that the quiet entry point
passed as NULL:

	if (verbose)
		map = calc_map_type_and_dist_warn(provider, pci_client,
						  &distance);
	else
		map = calc_map_type_and_dist(provider, pci_client,
					     &distance, NULL, NULL);

so the argument was true on exactly the path that commit describes.
Folding the two entry points into one verbose flag turned the pointer
into a value and left the call site alone, silently narrowing the
warning to paths that carry an ACS redirect.

Pass verbose. This also restores the warning for a verbose caller that
takes the host bridge route with no ACS redirect on the path, which
until now was told it could not use peer-to-peer DMA without being told
which vendor and device would have to be added to the whitelist.

Tested-by: Tushar Dave <tdave@nvidia.com>
Fixes: d1b8dc09dd71 ("PCI/P2PDMA: Simplify distance calculation")
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 drivers/pci/p2pdma.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
index 49bc8cf06240..a364008bbf50 100644
--- a/drivers/pci/p2pdma.c
+++ b/drivers/pci/p2pdma.c
@@ -750,7 +750,6 @@ calc_map_type_and_dist(struct pci_dev *provider, struct pci_dev *client,
 {
 	enum pci_p2pdma_map_type map_type = PCI_P2PDMA_MAP_THRU_HOST_BRIDGE;
 	struct pci_dev *a = provider, *b = client, *bb;
-	bool acs_redirects = false;
 	struct pci_p2pdma *p2pdma;
 	struct seq_buf acs_list;
 	int acs_cnt = 0;
@@ -821,11 +820,10 @@ calc_map_type_and_dist(struct pci_dev *provider, struct pci_dev *client,
 		pci_warn(client, "to disable ACS redirect for this path, add the kernel parameter: pci=disable_acs_redir=%s\n",
 			 seq_buf_str(&acs_list));
 	}
-	acs_redirects = true;
 
 map_through_host_bridge:
 	if (!cpu_supports_p2pdma() &&
-	    !host_bridge_whitelist(provider, client, acs_redirects)) {
+	    !host_bridge_whitelist(provider, client, verbose)) {
 		if (verbose)
 			pci_warn(client, "cannot be used for peer-to-peer DMA as the client and provider (%s) do not share an upstream bridge or whitelisted host bridge\n",
 				 pci_name(provider));

-- 
2.55.0

Re: [PATCH v4 06/18] PCI/P2PDMA: Gate the host bridge whitelist warning on verbose

[Logan Gunthorpe]

On 2026-08-21 13:38, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@nvidia.com>
> 
> calc_map_type_and_dist() prints every other diagnostic under its verbose
> argument, but reaches the "Host bridge not in P2PDMA whitelist" warning
> through host_bridge_whitelist(), which it hands acs_redirects instead.
> A caller that asked for a silent answer still gets the warning whenever
> any port on the path has an ACS redirect bit set, the CPU is not
> whitelisted by cpu_supports_p2pdma(), and the host bridge is not in
> pci_p2pdma_whitelist[].
> 
> pci_p2pmem_find_many() is such a caller. It sweeps every device with
> published p2pmem and asks for the distance to each client with
> verbose=false, and pci_p2pdma_distance_many() recomputes rather than
> consulting the map_types cache, so the warning repeats on every sweep.
> 
> The argument was never meant to say "ACS redirects were found". When
> commit cf201bfe8cdc ("PCI/P2PDMA: Warn if host bridge not in whitelist")
> added it, acs_redirects was a bool pointer that the quiet entry point
> passed as NULL:
> 
> 	if (verbose)
> 		map = calc_map_type_and_dist_warn(provider, pci_client,
> 						  &distance);
> 	else
> 		map = calc_map_type_and_dist(provider, pci_client,
> 					     &distance, NULL, NULL);
> 
> so the argument was true on exactly the path that commit describes.
> Folding the two entry points into one verbose flag turned the pointer
> into a value and left the call site alone, silently narrowing the
> warning to paths that carry an ACS redirect.
> 
> Pass verbose. This also restores the warning for a verbose caller that
> takes the host bridge route with no ACS redirect on the path, which
> until now was told it could not use peer-to-peer DMA without being told
> which vendor and device would have to be added to the whitelist.
> 
> Tested-by: Tushar Dave <tdave@nvidia.com>
> Fixes: d1b8dc09dd71 ("PCI/P2PDMA: Simplify distance calculation")
> Signed-off-by: Leon Romanovsky <leonro@nvidia.com>

Took me a bit of effort to understand this history, but I think the end
result makes more sense than what is currently there.

Reviewed-by: Logan Gunthorpe <logang@deltatee.com>

Re: [PATCH v4 06/18] PCI/P2PDMA: Gate the host bridge whitelist warning on verbose

[Jason Gunthorpe]

On Fri, 21 Aug 2026 22:38:37 +0300, Leon Romanovsky <leon@kernel.org> wrote:
> calc_map_type_and_dist() prints every other diagnostic under its verbose
> argument, but reaches the "Host bridge not in P2PDMA whitelist" warning
> through host_bridge_whitelist(), which it hands acs_redirects instead.
> A caller that asked for a silent answer still gets the warning whenever
> any port on the path has an ACS redirect bit set, the CPU is not
> whitelisted by cpu_supports_p2pdma(), and the host bridge is not in
> pci_p2pdma_whitelist[].
> 
> [...]

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

-- 
Jason

[PATCH v4 07/18] PCI/P2PDMA: Document the Address Type assumption

[Leon Romanovsky]

From: Leon Romanovsky <leonro@nvidia.com>

P2PDMA selects a mapping from the ACS controls that govern Requests
carrying an Untranslated address. PCIe r7.0, sec 6.12.3 routes a
Translated Request directly to the peer when ACS Direct Translated P2P is
enabled, regardless of P2P Request Redirect and P2P Egress Control.
Translation Blocking takes precedence and prevents that direct route.

Document this assumption because an ATS capable client can otherwise
reach the peer directly whichever mapping P2PDMA selects.

Tested-by: Tushar Dave <tdave@nvidia.com>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 Documentation/driver-api/pci/p2pdma.rst | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/driver-api/pci/p2pdma.rst b/Documentation/driver-api/pci/p2pdma.rst
index d3f406cca694..a7fd426c3685 100644
--- a/Documentation/driver-api/pci/p2pdma.rst
+++ b/Documentation/driver-api/pci/p2pdma.rst
@@ -15,6 +15,13 @@ then based on the ACS settings the transaction can route entirely within
 the PCIe hierarchy and never reach the root port. The kernel will evaluate
 the PCIe topology and always permit P2P in these well-defined cases.
 
+This evaluation covers the ACS controls that govern Requests carrying an
+Untranslated address. Unless ACS Translation Blocking is enabled, a Port
+with ACS Direct Translated P2P enabled routes a Request carrying a Translated
+address directly to the peer regardless of those controls. An ATS capable
+client may therefore reach the peer on the direct path whichever mapping the
+kernel selects.
+
 However, if the P2P transaction reaches the host bridge then it might have to
 hairpin back out the same root port, be routed inside the CPU SOC to another
 PCIe root port, or routed internally to the SOC.

-- 
2.55.0

Re: [PATCH v4 07/18] PCI/P2PDMA: Document the Address Type assumption

[Jason Gunthorpe]

> [ ... 21 lines skipped ... ]
> +This evaluation covers the ACS controls that govern Requests carrying an
> +Untranslated address. Unless ACS Translation Blocking is enabled, a Port
> +with ACS Direct Translated P2P enabled routes a Request carrying a Translated
> +address directly to the peer regardless of those controls. An ATS capable
> +client may therefore reach the peer on the direct path whichever mapping the
> +kernel selects.

My note on the cover letter applies here.. A user of the p2p subsystem
kind of has to issue untranslated strict ordered TLPs otherwise the
ACS analysis is not correct.

A translated address routing to the host bridge for a peer device
still requires support in the host bridge to hairpin back out.

So I would prefer this is clarified to that that exactly, using it
with ATS translated TLPs is not defined.

-- 
Jason

[PATCH v4 08/18] PCI: Account for Direct Translated P2P in ACS isolation checks

[Leon Romanovsky]

From: Leon Romanovsky <leonro@nvidia.com>

PCIe r7.0, sec 6.12.3:

  peer-to-peer Memory Requests whose Address Type (AT) field indicates a
  Translated address must be routed to the peer Port/Function without
  redirection, regardless of ACS P2P Request Redirect and ACS P2P Egress
  Control settings.

Request Redirect therefore does not isolate devices below a Port with
ACS Direct Translated P2P enabled.

Sec 6.12.1.1 makes such a Request an ACS Violation once Translation
Blocking is enabled, and that error "must take precedence over ... ACS
P2P control mechanisms". Report isolation only in that case. Without
Translation Blocking, devices below such a Port now share an IOMMU
group.

This only holds for a caller that needs Request Redirect to isolate
peers. pci_enable_pasid() asks for Request Redirect for a different
reason: a Request carrying a PASID is routed by address alone (sec
2.2.10.4), so it has to be redirected Upstream to reach the translation
agent. Direct Translated P2P says nothing about that, because a
Translated Request already carries an address the agent produced for
that PASID (sec 10.1.3).

Give pci_acs_enabled() and pci_acs_path_enabled() a scope so each caller
states which Requests its answer has to cover, and apply the rule above
only for PCI_ACS_SCOPE_ALL.

pci_acs_flags_enabled() and the Intel SPT PCH quirk both need the rule,
so it lives in pci_acs_rr_ineffective().

Tested-by: Tushar Dave <tdave@nvidia.com>
Fixes: ad805758c0eb ("PCI: add ACS validation utility")
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 drivers/iommu/iommu.c |  8 +++++---
 drivers/pci/ats.c     | 11 +++++++++-
 drivers/pci/pci.c     | 26 +++++++++++++++--------
 drivers/pci/pci.h     | 30 +++++++++++++++++++++++++--
 drivers/pci/quirks.c  | 57 ++++++++++++++++++++++++++++++++++-----------------
 include/linux/pci.h   | 30 +++++++++++++++++++++++----
 6 files changed, 124 insertions(+), 38 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index e8f13dcebbde..6ab32d714ce5 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1502,13 +1502,14 @@ static struct iommu_group *get_pci_function_alias_group(struct pci_dev *pdev,
 	struct pci_dev *tmp = NULL;
 	struct iommu_group *group;
 
-	if (!pdev->multifunction || pci_acs_enabled(pdev, REQ_ACS_FLAGS))
+	if (!pdev->multifunction ||
+	    pci_acs_enabled(pdev, REQ_ACS_FLAGS, PCI_ACS_SCOPE_ALL))
 		return NULL;
 
 	for_each_pci_dev(tmp) {
 		if (tmp == pdev || tmp->bus != pdev->bus ||
 		    PCI_SLOT(tmp->devfn) != PCI_SLOT(pdev->devfn) ||
-		    pci_acs_enabled(tmp, REQ_ACS_FLAGS))
+		    pci_acs_enabled(tmp, REQ_ACS_FLAGS, PCI_ACS_SCOPE_ALL))
 			continue;
 
 		group = get_pci_alias_group(tmp, devfns);
@@ -1652,7 +1653,8 @@ struct iommu_group *pci_device_group(struct device *dev)
 		if (!bus->self)
 			continue;
 
-		if (pci_acs_path_enabled(bus->self, NULL, REQ_ACS_FLAGS))
+		if (pci_acs_path_enabled(bus->self, NULL, REQ_ACS_FLAGS,
+					 PCI_ACS_SCOPE_ALL))
 			break;
 
 		pdev = bus->self;
diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
index 96efa00d9743..35c3949f39e0 100644
--- a/drivers/pci/ats.c
+++ b/drivers/pci/ats.c
@@ -463,7 +463,16 @@ int pci_enable_pasid(struct pci_dev *pdev, int features)
 	if (!pasid)
 		return -EINVAL;
 
-	if (!pci_acs_path_enabled(pdev, NULL, PCI_ACS_RR | PCI_ACS_UF))
+	/*
+	 * A Request carrying a PASID is routed by address alone (PCIe r7.0,
+	 * sec 2.2.10.4), so it has to be redirected Upstream to reach the
+	 * translation agent. Only Untranslated Requests are at stake here:
+	 * a Translated Request already carries an address the agent produced
+	 * for this PASID (sec 10.1.3), so ACS Direct Translated P2P routing it
+	 * to a peer is not a way around the agent.
+	 */
+	if (!pci_acs_path_enabled(pdev, NULL, PCI_ACS_RR | PCI_ACS_UF,
+				  PCI_ACS_SCOPE_UNTRANSLATED))
 		return -EINVAL;
 
 	pci_read_config_word(pdev, pasid + PCI_PASID_CAP, &supported);
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 77b17b13ee61..492bb26a99de 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3545,7 +3545,8 @@ void pci_configure_ari(struct pci_dev *dev)
 	}
 }
 
-static bool pci_acs_flags_enabled(struct pci_dev *pdev, u16 acs_flags)
+static bool pci_acs_flags_enabled(struct pci_dev *pdev, u16 acs_flags,
+				  enum pci_acs_scope scope)
 {
 	int pos;
 	u16 ctrl;
@@ -3554,6 +3555,11 @@ static bool pci_acs_flags_enabled(struct pci_dev *pdev, u16 acs_flags)
 	if (!pos)
 		return false;
 
+	pci_read_config_word(pdev, pos + PCI_ACS_CTRL, &ctrl);
+
+	if (pci_acs_rr_ineffective(ctrl, acs_flags, scope))
+		return false;
+
 	/*
 	 * Except for egress control, capabilities are either required
 	 * or only required if controllable.  Features missing from the
@@ -3561,7 +3567,6 @@ static bool pci_acs_flags_enabled(struct pci_dev *pdev, u16 acs_flags)
 	 */
 	acs_flags &= (pdev->acs_capabilities | PCI_ACS_EC);
 
-	pci_read_config_word(pdev, pos + PCI_ACS_CTRL, &ctrl);
 	return (ctrl & acs_flags) == acs_flags;
 }
 
@@ -3569,6 +3574,7 @@ static bool pci_acs_flags_enabled(struct pci_dev *pdev, u16 acs_flags)
  * pci_acs_enabled - test ACS against required flags for a given device
  * @pdev: device to test
  * @acs_flags: required PCI ACS flags
+ * @scope: which peer-to-peer Requests the answer has to cover
  *
  * Return true if the device supports the provided flags.  Automatically
  * filters out flags that are not implemented on multifunction devices.
@@ -3581,11 +3587,12 @@ static bool pci_acs_flags_enabled(struct pci_dev *pdev, u16 acs_flags)
  * it much easier for callers of this function to ignore the actual type
  * or topology of the device when testing ACS support.
  */
-bool pci_acs_enabled(struct pci_dev *pdev, u16 acs_flags)
+bool pci_acs_enabled(struct pci_dev *pdev, u16 acs_flags,
+		     enum pci_acs_scope scope)
 {
 	int ret;
 
-	ret = pci_dev_specific_acs_enabled(pdev, acs_flags);
+	ret = pci_dev_specific_acs_enabled(pdev, acs_flags, scope);
 	if (ret >= 0)
 		return ret > 0;
 
@@ -3620,7 +3627,7 @@ bool pci_acs_enabled(struct pci_dev *pdev, u16 acs_flags)
 	 */
 	case PCI_EXP_TYPE_DOWNSTREAM:
 	case PCI_EXP_TYPE_ROOT_PORT:
-		return pci_acs_flags_enabled(pdev, acs_flags);
+		return pci_acs_flags_enabled(pdev, acs_flags, scope);
 	/*
 	 * PCIe 3.0, 6.12.1.2 specifies ACS capabilities that should be
 	 * implemented by the remaining PCIe types to indicate peer-to-peer
@@ -3635,7 +3642,7 @@ bool pci_acs_enabled(struct pci_dev *pdev, u16 acs_flags)
 		if (!pdev->multifunction)
 			break;
 
-		return pci_acs_flags_enabled(pdev, acs_flags);
+		return pci_acs_flags_enabled(pdev, acs_flags, scope);
 	}
 
 	/*
@@ -3650,19 +3657,20 @@ bool pci_acs_enabled(struct pci_dev *pdev, u16 acs_flags)
  * @start: starting downstream device
  * @end: ending upstream device or NULL to search to the root bus
  * @acs_flags: required flags
+ * @scope: which peer-to-peer Requests the answer has to cover
  *
  * Walk up a device tree from start to end testing PCI ACS support.  If
  * any step along the way does not support the required flags, return false.
  */
-bool pci_acs_path_enabled(struct pci_dev *start,
-			  struct pci_dev *end, u16 acs_flags)
+bool pci_acs_path_enabled(struct pci_dev *start, struct pci_dev *end,
+			  u16 acs_flags, enum pci_acs_scope scope)
 {
 	struct pci_dev *pdev, *parent = start;
 
 	do {
 		pdev = parent;
 
-		if (!pci_acs_enabled(pdev, acs_flags))
+		if (!pci_acs_enabled(pdev, acs_flags, scope))
 			return false;
 
 		if (pci_is_root_bus(pdev->bus))
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 4469e1a77f3c..6230adb39166 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -1045,15 +1045,41 @@ resource_size_t pci_min_window_alignment(struct pci_bus *bus,
 
 void pci_acs_init(struct pci_dev *dev);
 void pci_enable_acs(struct pci_dev *dev);
+
+/*
+ * PCIe r7.0, sec 6.12.3: ACS P2P Request Redirect does not by itself keep a
+ * peer-to-peer Request off the direct path to its target.
+ *
+ * Direct Translated P2P routes a Request carrying a Translated address to the
+ * peer regardless of Request Redirect, so Request Redirect does not isolate
+ * unless Translation Blocking rejects the Request first (sec 6.12.1.1).  It
+ * says nothing about an Untranslated Request, so a caller asking only about
+ * those is unaffected.
+ *
+ * @ctrl is the ACS Control register, @acs_flags the controls the caller asked
+ * for, and @scope the Requests its answer has to cover.
+ */
+static inline bool pci_acs_rr_ineffective(u32 ctrl, u16 acs_flags,
+					  enum pci_acs_scope scope)
+{
+	if (!(acs_flags & PCI_ACS_RR))
+		return false;
+
+	return scope == PCI_ACS_SCOPE_ALL &&
+	       (ctrl & PCI_ACS_DT) && !(ctrl & PCI_ACS_TB);
+}
+
 #ifdef CONFIG_PCI_QUIRKS
-int pci_dev_specific_acs_enabled(struct pci_dev *dev, u16 acs_flags);
+int pci_dev_specific_acs_enabled(struct pci_dev *dev, u16 acs_flags,
+				 enum pci_acs_scope scope);
 int pci_dev_specific_enable_acs(struct pci_dev *dev);
 int pci_dev_specific_disable_acs_redir(struct pci_dev *dev);
 void pci_disable_broken_acs_cap(struct pci_dev *pdev);
 int pcie_failed_link_retrain(struct pci_dev *dev);
 #else
 static inline int pci_dev_specific_acs_enabled(struct pci_dev *dev,
-					       u16 acs_flags)
+					       u16 acs_flags,
+					       enum pci_acs_scope scope)
 {
 	return -ENOTTY;
 }
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index b09f27f7846f..8b50cd0e5114 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -4707,7 +4707,8 @@ static int pci_acs_ctrl_enabled(u16 acs_ctrl_req, u16 acs_ctrl_ena)
  * 1022:780f [AMD] FCH PCI Bridge
  * 1022:7809 [AMD] FCH USB OHCI Controller
  */
-static int pci_quirk_amd_sb_acs(struct pci_dev *dev, u16 acs_flags)
+static int pci_quirk_amd_sb_acs(struct pci_dev *dev, u16 acs_flags,
+				enum pci_acs_scope scope)
 {
 #ifdef CONFIG_ACPI
 	struct acpi_table_header *header = NULL;
@@ -4752,7 +4753,8 @@ static bool pci_quirk_cavium_acs_match(struct pci_dev *dev)
 	}
 }
 
-static int pci_quirk_cavium_acs(struct pci_dev *dev, u16 acs_flags)
+static int pci_quirk_cavium_acs(struct pci_dev *dev, u16 acs_flags,
+				enum pci_acs_scope scope)
 {
 	if (!pci_quirk_cavium_acs_match(dev))
 		return -ENOTTY;
@@ -4769,7 +4771,8 @@ static int pci_quirk_cavium_acs(struct pci_dev *dev, u16 acs_flags)
 		PCI_ACS_SV | PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_UF);
 }
 
-static int pci_quirk_xgene_acs(struct pci_dev *dev, u16 acs_flags)
+static int pci_quirk_xgene_acs(struct pci_dev *dev, u16 acs_flags,
+			       enum pci_acs_scope scope)
 {
 	/*
 	 * X-Gene Root Ports matching this quirk do not allow peer-to-peer
@@ -4785,7 +4788,8 @@ static int pci_quirk_xgene_acs(struct pci_dev *dev, u16 acs_flags)
  * But the implementation could block peer-to-peer transactions between them
  * and provide ACS-like functionality.
  */
-static int pci_quirk_zhaoxin_pcie_ports_acs(struct pci_dev *dev, u16 acs_flags)
+static int pci_quirk_zhaoxin_pcie_ports_acs(struct pci_dev *dev, u16 acs_flags,
+					    enum pci_acs_scope scope)
 {
 	if (!pci_is_pcie(dev) ||
 	    ((pci_pcie_type(dev) != PCI_EXP_TYPE_ROOT_PORT) &&
@@ -4856,7 +4860,8 @@ static bool pci_quirk_intel_pch_acs_match(struct pci_dev *dev)
 	return false;
 }
 
-static int pci_quirk_intel_pch_acs(struct pci_dev *dev, u16 acs_flags)
+static int pci_quirk_intel_pch_acs(struct pci_dev *dev, u16 acs_flags,
+				   enum pci_acs_scope scope)
 {
 	if (!pci_quirk_intel_pch_acs_match(dev))
 		return -ENOTTY;
@@ -4878,7 +4883,8 @@ static int pci_quirk_intel_pch_acs(struct pci_dev *dev, u16 acs_flags)
  * Port to pass traffic to another Root Port.  All PCIe transactions are
  * terminated inside the Root Port.
  */
-static int pci_quirk_qcom_rp_acs(struct pci_dev *dev, u16 acs_flags)
+static int pci_quirk_qcom_rp_acs(struct pci_dev *dev, u16 acs_flags,
+				 enum pci_acs_scope scope)
 {
 	return pci_acs_ctrl_enabled(acs_flags,
 		PCI_ACS_SV | PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_UF);
@@ -4890,13 +4896,15 @@ static int pci_quirk_qcom_rp_acs(struct pci_dev *dev, u16 acs_flags)
  * and validate bus numbers in requests, but does not provide an ACS
  * capability.
  */
-static int pci_quirk_nxp_rp_acs(struct pci_dev *dev, u16 acs_flags)
+static int pci_quirk_nxp_rp_acs(struct pci_dev *dev, u16 acs_flags,
+				enum pci_acs_scope scope)
 {
 	return pci_acs_ctrl_enabled(acs_flags,
 		PCI_ACS_SV | PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_UF);
 }
 
-static int pci_quirk_al_acs(struct pci_dev *dev, u16 acs_flags)
+static int pci_quirk_al_acs(struct pci_dev *dev, u16 acs_flags,
+			    enum pci_acs_scope scope)
 {
 	if (pci_pcie_type(dev) != PCI_EXP_TYPE_ROOT_PORT)
 		return -ENOTTY;
@@ -4976,7 +4984,8 @@ static bool pci_quirk_intel_spt_pch_acs_match(struct pci_dev *dev)
 
 #define INTEL_SPT_ACS_CTRL (PCI_ACS_CAP + 4)
 
-static int pci_quirk_intel_spt_pch_acs(struct pci_dev *dev, u16 acs_flags)
+static int pci_quirk_intel_spt_pch_acs(struct pci_dev *dev, u16 acs_flags,
+				       enum pci_acs_scope scope)
 {
 	int pos;
 	u32 cap, ctrl;
@@ -4990,14 +4999,18 @@ static int pci_quirk_intel_spt_pch_acs(struct pci_dev *dev, u16 acs_flags)
 
 	/* see pci_acs_flags_enabled() */
 	pci_read_config_dword(dev, pos + PCI_ACS_CAP, &cap);
-	acs_flags &= (cap | PCI_ACS_EC);
-
 	pci_read_config_dword(dev, pos + INTEL_SPT_ACS_CTRL, &ctrl);
 
+	if (pci_acs_rr_ineffective(ctrl, acs_flags, scope))
+		return 0;
+
+	acs_flags &= (cap | PCI_ACS_EC);
+
 	return pci_acs_ctrl_enabled(acs_flags, ctrl);
 }
 
-static int pci_quirk_mf_endpoint_acs(struct pci_dev *dev, u16 acs_flags)
+static int pci_quirk_mf_endpoint_acs(struct pci_dev *dev, u16 acs_flags,
+				     enum pci_acs_scope scope)
 {
 	/*
 	 * SV, TB, and UF are not relevant to multifunction endpoints.
@@ -5013,7 +5026,8 @@ static int pci_quirk_mf_endpoint_acs(struct pci_dev *dev, u16 acs_flags)
 		PCI_ACS_CR | PCI_ACS_UF | PCI_ACS_DT);
 }
 
-static int pci_quirk_rciep_acs(struct pci_dev *dev, u16 acs_flags)
+static int pci_quirk_rciep_acs(struct pci_dev *dev, u16 acs_flags,
+			       enum pci_acs_scope scope)
 {
 	/*
 	 * Intel RCiEP's are required to allow p2p only on translated
@@ -5027,7 +5041,8 @@ static int pci_quirk_rciep_acs(struct pci_dev *dev, u16 acs_flags)
 		PCI_ACS_SV | PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_UF);
 }
 
-static int pci_quirk_brcm_acs(struct pci_dev *dev, u16 acs_flags)
+static int pci_quirk_brcm_acs(struct pci_dev *dev, u16 acs_flags,
+			      enum pci_acs_scope scope)
 {
 	/*
 	 * iProc PAXB Root Ports don't advertise an ACS capability, but
@@ -5039,7 +5054,8 @@ static int pci_quirk_brcm_acs(struct pci_dev *dev, u16 acs_flags)
 		PCI_ACS_SV | PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_UF);
 }
 
-static int pci_quirk_loongson_acs(struct pci_dev *dev, u16 acs_flags)
+static int pci_quirk_loongson_acs(struct pci_dev *dev, u16 acs_flags,
+				  enum pci_acs_scope scope)
 {
 	/*
 	 * Loongson PCIe Root Ports don't advertise an ACS capability, but
@@ -5060,7 +5076,8 @@ static int pci_quirk_loongson_acs(struct pci_dev *dev, u16 acs_flags)
  * RP1000/RP2000 10G NICs(sp).
  * FF5xxx 40G/25G/10G NICs(aml).
  */
-static int  pci_quirk_wangxun_nic_acs(struct pci_dev *dev, u16 acs_flags)
+static int  pci_quirk_wangxun_nic_acs(struct pci_dev *dev, u16 acs_flags,
+				      enum pci_acs_scope scope)
 {
 	switch (dev->device) {
 	case 0x0100 ... 0x010F: /* EM */
@@ -5077,7 +5094,8 @@ static int  pci_quirk_wangxun_nic_acs(struct pci_dev *dev, u16 acs_flags)
 static const struct pci_dev_acs_enabled {
 	u16 vendor;
 	u16 device;
-	int (*acs_enabled)(struct pci_dev *dev, u16 acs_flags);
+	int (*acs_enabled)(struct pci_dev *dev, u16 acs_flags,
+			   enum pci_acs_scope scope);
 } pci_dev_acs_enabled[] = {
 	{ PCI_VENDOR_ID_ATI, 0x4385, pci_quirk_amd_sb_acs },
 	{ PCI_VENDOR_ID_ATI, 0x439c, pci_quirk_amd_sb_acs },
@@ -5256,7 +5274,8 @@ static const struct pci_dev_acs_enabled {
  *   0:		Device does not provide all the desired controls
  *   >0:	Device provides all the controls in @acs_flags
  */
-int pci_dev_specific_acs_enabled(struct pci_dev *dev, u16 acs_flags)
+int pci_dev_specific_acs_enabled(struct pci_dev *dev, u16 acs_flags,
+				 enum pci_acs_scope scope)
 {
 	const struct pci_dev_acs_enabled *i;
 	int ret;
@@ -5272,7 +5291,7 @@ int pci_dev_specific_acs_enabled(struct pci_dev *dev, u16 acs_flags)
 		     i->vendor == (u16)PCI_ANY_ID) &&
 		    (i->device == dev->device ||
 		     i->device == (u16)PCI_ANY_ID)) {
-			ret = i->acs_enabled(dev, acs_flags);
+			ret = i->acs_enabled(dev, acs_flags, scope);
 			if (ret >= 0)
 				return ret;
 		}
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 64b308b6e61c..867c0f0970bd 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -277,6 +277,26 @@ enum pci_bus_flags {
 	PCI_BUS_FLAGS_NO_EXTCFG	= (__force pci_bus_flags_t) 8,
 };
 
+/**
+ * enum pci_acs_scope - which peer-to-peer Requests an ACS check must cover
+ * @PCI_ACS_SCOPE_ALL: every peer-to-peer Request, including one carrying a
+ *	Translated address.  ACS Direct Translated P2P routes those to the peer
+ *	regardless of P2P Request Redirect (PCIe r7.0, sec 6.12.3), so it
+ *	defeats isolation unless ACS Translation Blocking rejects them first
+ *	(sec 6.12.1.1).
+ * @PCI_ACS_SCOPE_UNTRANSLATED: only Requests carrying an Untranslated address.
+ *	ACS Direct Translated P2P does not apply to those, so it says nothing
+ *	about whether they reach the Root Complex.
+ *
+ * A caller proving that peers cannot reach each other wants
+ * %PCI_ACS_SCOPE_ALL.  A caller that only needs Untranslated Requests routed
+ * Upstream, such as pci_enable_pasid(), wants %PCI_ACS_SCOPE_UNTRANSLATED.
+ */
+enum pci_acs_scope {
+	PCI_ACS_SCOPE_ALL,
+	PCI_ACS_SCOPE_UNTRANSLATED,
+};
+
 /* Values from Link Status register, PCIe r3.1, sec 7.8.8 */
 enum pcie_link_width {
 	PCIE_LNK_WIDTH_RESRV	= 0x00,
@@ -2213,7 +2233,8 @@ static inline struct pci_dev *pci_dev_get(struct pci_dev *dev) { return NULL; }
 
 #define dev_is_pci(d) (false)
 #define dev_is_pf(d) (false)
-static inline bool pci_acs_enabled(struct pci_dev *pdev, u16 acs_flags)
+static inline bool pci_acs_enabled(struct pci_dev *pdev, u16 acs_flags,
+				   enum pci_acs_scope scope)
 { return false; }
 static inline int pci_irqd_intx_xlate(struct irq_domain *d,
 				      struct device_node *node,
@@ -2710,9 +2731,10 @@ static inline bool pci_dev_is_disconnected(const struct pci_dev *dev)
 }
 
 void pci_request_acs(void);
-bool pci_acs_enabled(struct pci_dev *pdev, u16 acs_flags);
-bool pci_acs_path_enabled(struct pci_dev *start,
-			  struct pci_dev *end, u16 acs_flags);
+bool pci_acs_enabled(struct pci_dev *pdev, u16 acs_flags,
+		     enum pci_acs_scope scope);
+bool pci_acs_path_enabled(struct pci_dev *start, struct pci_dev *end,
+			  u16 acs_flags, enum pci_acs_scope scope);
 int pci_enable_atomic_ops_to_root(struct pci_dev *dev, u32 cap_mask);
 
 #define PCI_VPD_LRDT			0x80	/* Large Resource Data Type */

-- 
2.55.0

Re: [PATCH v4 08/18] PCI: Account for Direct Translated P2P in ACS isolation checks

[Jason Gunthorpe]

> PCIe r7.0, sec 6.12.3:
> 
>   peer-to-peer Memory Requests whose Address Type (AT) field indicates a
>   Translated address must be routed to the peer Port/Function without
>   redirection, regardless of ACS P2P Request Redirect and ACS P2P Egress
>   Control settings.
> 
> Request Redirect therefore does not isolate devices below a Port with
> ACS Direct Translated P2P enabled.

No, this whole patch misunderstand what "isolate" means in the context
of iommu groups.

Translated requests are *always* isolated becuase the translated
address always comes from the isolating iommu. We trust them just
fine.

-- 
Jason

Re: [PATCH v4 08/18] PCI: Account for Direct Translated P2P in ACS isolation checks

[Logan Gunthorpe]

On 2026-08-21 13:38, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@nvidia.com>
> 
> PCIe r7.0, sec 6.12.3:
> 
>   peer-to-peer Memory Requests whose Address Type (AT) field indicates a
>   Translated address must be routed to the peer Port/Function without
>   redirection, regardless of ACS P2P Request Redirect and ACS P2P Egress
>   Control settings.
> 
> Request Redirect therefore does not isolate devices below a Port with
> ACS Direct Translated P2P enabled.
> 
> Sec 6.12.1.1 makes such a Request an ACS Violation once Translation
> Blocking is enabled, and that error "must take precedence over ... ACS
> P2P control mechanisms". Report isolation only in that case. Without
> Translation Blocking, devices below such a Port now share an IOMMU
> group.
> 
> This only holds for a caller that needs Request Redirect to isolate
> peers. pci_enable_pasid() asks for Request Redirect for a different
> reason: a Request carrying a PASID is routed by address alone (sec
> 2.2.10.4), so it has to be redirected Upstream to reach the translation
> agent. Direct Translated P2P says nothing about that, because a
> Translated Request already carries an address the agent produced for
> that PASID (sec 10.1.3).
> 
> Give pci_acs_enabled() and pci_acs_path_enabled() a scope so each caller
> states which Requests its answer has to cover, and apply the rule above
> only for PCI_ACS_SCOPE_ALL.
> 
> pci_acs_flags_enabled() and the Intel SPT PCH quirk both need the rule,
> so it lives in pci_acs_rr_ineffective().
> 
> Tested-by: Tushar Dave <tdave@nvidia.com>
> Fixes: ad805758c0eb ("PCI: add ACS validation utility")
> Signed-off-by: Leon Romanovsky <leonro@nvidia.com>

Looks, ok to me:

Reviewed-by: Logan Gunthorpe <logang@deltatee.com>

[PATCH v4 09/18] PCI: Add ACS egress control vector accessor

[Leon Romanovsky]

From: Leon Romanovsky <leonro@nvidia.com>

Whether ACS P2P Egress Control routes, redirects, or blocks a peer-to-peer
request depends on the Egress Control Vector bit for the target port, not
on the enable bit alone (PCIe r7.0, sec 6.12.3).

Provide a helper to read that bit for a peer Root or Switch Downstream
Port. Report an unreadable or uncovered vector as an error rather than as
a clear bit, so callers do not mistake it for permission to route
directly.

Each bit corresponds to a Port Number within one Switch or Root Complex
(sec 7.7.12.4), so both ports have to number their ports in the same
place. Downstream Ports of one Switch share its internal bus, and Root
Ports of one Root Complex share a root bus, so require a shared bus and
reject anything else. A target numbered elsewhere has no bit in this
vector and would select an unrelated one.

Tested-by: Tushar Dave <tdave@nvidia.com>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 drivers/pci/pci.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/pci.h |  1 +
 2 files changed, 69 insertions(+)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 492bb26a99de..8d165c9534ff 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3545,6 +3545,74 @@ void pci_configure_ari(struct pci_dev *dev)
 	}
 }
 
+/*
+ * PCIe r7.0, sec 7.7.12: only for Root Ports and Switch Downstream Ports does
+ * each Egress Control Vector bit correspond to a Port Number.  Elsewhere the
+ * vector is indexed by Function or Function Group Number, so a Link
+ * Capabilities Port Number must not be used to select a bit.
+ *
+ * pcie_downstream_port() is too permissive here because it also accepts a
+ * PCI/PCI-X to PCIe Bridge.
+ */
+static bool pci_acs_egress_vector_port(const struct pci_dev *dev)
+{
+	int type = pci_pcie_type(dev);
+
+	return type == PCI_EXP_TYPE_ROOT_PORT ||
+	       type == PCI_EXP_TYPE_DOWNSTREAM;
+}
+
+/**
+ * pci_acs_egress_ctrl_is_set - Read an ACS Egress Control Vector bit
+ * @pdev: ingress Root or Switch Downstream Port
+ * @target: target Root or Switch Downstream Port
+ *
+ * Return: 1 if @pdev's Egress Control Vector bit for @target is set, 0 if
+ * it is clear, or a negative errno if the bit cannot be determined.
+ */
+int pci_acs_egress_ctrl_is_set(struct pci_dev *pdev, struct pci_dev *target)
+{
+	unsigned int vector_size;
+	u32 lnkcap, vector;
+	u8 target_port;
+	int ret;
+
+	if (!(pdev->acs_capabilities & PCI_ACS_EC) ||
+	    !pci_acs_egress_vector_port(pdev) ||
+	    !pci_acs_egress_vector_port(target))
+		return -EOPNOTSUPP;
+
+	/*
+	 * Each vector bit corresponds to a Port Number within one Switch or
+	 * Root Complex (PCIe r7.0, sec 7.7.12.4). Downstream Ports of one
+	 * Switch share its internal bus and Root Ports of one Root Complex
+	 * share a root bus, so anything else numbers its ports elsewhere and
+	 * would index an unrelated bit here.
+	 */
+	if (pdev->bus != target->bus)
+		return -EOPNOTSUPP;
+
+	ret = pcie_capability_read_dword(target, PCI_EXP_LNKCAP, &lnkcap);
+	if (ret)
+		return pcibios_err_to_errno(ret);
+
+	target_port = FIELD_GET(PCI_EXP_LNKCAP_PN, lnkcap);
+	vector_size = pdev->acs_capabilities >> 8;
+
+	/* An Egress Control Vector Size of 0 encodes 256 bits. */
+	if (vector_size && target_port >= vector_size)
+		return -ERANGE;
+
+	ret = pci_read_config_dword(pdev,
+				    pdev->acs_cap + PCI_ACS_EGRESS_CTL_V +
+				    (target_port / 32) * sizeof(vector),
+				    &vector);
+	if (ret)
+		return pcibios_err_to_errno(ret);
+
+	return !!(vector & BIT(target_port % 32));
+}
+
 static bool pci_acs_flags_enabled(struct pci_dev *pdev, u16 acs_flags,
 				  enum pci_acs_scope scope)
 {
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 6230adb39166..d3ea9b2bb7fc 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -1069,6 +1069,7 @@ static inline bool pci_acs_rr_ineffective(u32 ctrl, u16 acs_flags,
 	       (ctrl & PCI_ACS_DT) && !(ctrl & PCI_ACS_TB);
 }
 
+int pci_acs_egress_ctrl_is_set(struct pci_dev *pdev, struct pci_dev *target);
 #ifdef CONFIG_PCI_QUIRKS
 int pci_dev_specific_acs_enabled(struct pci_dev *dev, u16 acs_flags,
 				 enum pci_acs_scope scope);

-- 
2.55.0

[PATCH v4 10/18] PCI: Account for ACS egress control in isolation checks

[Leon Romanovsky]

From: Leon Romanovsky <leonro@nvidia.com>

pci_acs_enabled() treats P2P Request Redirect as effective whenever its
control bit is set. PCIe r7.0, sec 6.12.3, table 6-11 lets an enabled
Egress Control Vector override it: a clear vector bit routes the request
directly.

IOMMU grouping uses this check to prove peer requests cannot bypass the
IOMMU, but cannot know every applicable vector bit, so Request Redirect
gives no such guarantee while Egress Control is enabled.

Report Request Redirect as ineffective there, merging the devices into
one IOMMU group, and report no isolation when the register cannot be
read. Apply the same rule to the Intel SPT PCH quirk.

Unlike Direct Translated P2P this holds for an Untranslated Request too,
so it applies in both scopes. pci_enable_pasid() therefore fails on a
path where a port has Egress Control enabled, because Request Redirect
no longer shows that a Request carrying a PASID reaches the translation
agent.

Tested-by: Tushar Dave <tdave@nvidia.com>
Fixes: ad805758c0eb ("PCI: add ACS validation utility")
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 drivers/pci/pci.c    |  3 ++-
 drivers/pci/pci.h    | 13 +++++++++++--
 drivers/pci/quirks.c |  7 +++++--
 3 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 8d165c9534ff..a633f473590f 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3623,7 +3623,8 @@ static bool pci_acs_flags_enabled(struct pci_dev *pdev, u16 acs_flags,
 	if (!pos)
 		return false;
 
-	pci_read_config_word(pdev, pos + PCI_ACS_CTRL, &ctrl);
+	if (pci_read_config_word(pdev, pos + PCI_ACS_CTRL, &ctrl))
+		return false;
 
 	if (pci_acs_rr_ineffective(ctrl, acs_flags, scope))
 		return false;
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index d3ea9b2bb7fc..32394e349766 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -1056,6 +1056,12 @@ void pci_enable_acs(struct pci_dev *dev);
  * says nothing about an Untranslated Request, so a caller asking only about
  * those is unaffected.
  *
+ * Egress Control can override Request Redirect for any peer Request,
+ * Untranslated ones included, so it applies in either scope.  This
+ * target-independent test cannot prove that every applicable Egress Control
+ * Vector bit is set, so Request Redirect does not guarantee that the Request
+ * leaves the direct path while Egress Control is enabled.
+ *
  * @ctrl is the ACS Control register, @acs_flags the controls the caller asked
  * for, and @scope the Requests its answer has to cover.
  */
@@ -1065,8 +1071,11 @@ static inline bool pci_acs_rr_ineffective(u32 ctrl, u16 acs_flags,
 	if (!(acs_flags & PCI_ACS_RR))
 		return false;
 
-	return scope == PCI_ACS_SCOPE_ALL &&
-	       (ctrl & PCI_ACS_DT) && !(ctrl & PCI_ACS_TB);
+	if (scope == PCI_ACS_SCOPE_ALL &&
+	    (ctrl & PCI_ACS_DT) && !(ctrl & PCI_ACS_TB))
+		return true;
+
+	return ctrl & PCI_ACS_EC;
 }
 
 int pci_acs_egress_ctrl_is_set(struct pci_dev *pdev, struct pci_dev *target);
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 8b50cd0e5114..cee6be63cadd 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -4998,8 +4998,11 @@ static int pci_quirk_intel_spt_pch_acs(struct pci_dev *dev, u16 acs_flags,
 		return -ENOTTY;
 
 	/* see pci_acs_flags_enabled() */
-	pci_read_config_dword(dev, pos + PCI_ACS_CAP, &cap);
-	pci_read_config_dword(dev, pos + INTEL_SPT_ACS_CTRL, &ctrl);
+	if (pci_read_config_dword(dev, pos + PCI_ACS_CAP, &cap))
+		return 0;
+
+	if (pci_read_config_dword(dev, pos + INTEL_SPT_ACS_CTRL, &ctrl))
+		return 0;
 
 	if (pci_acs_rr_ineffective(ctrl, acs_flags, scope))
 		return 0;

-- 
2.55.0

Re: [PATCH v4 10/18] PCI: Account for ACS egress control in isolation checks

[Jason Gunthorpe]

> pci_acs_enabled() treats P2P Request Redirect as effective whenever its
> control bit is set. PCIe r7.0, sec 6.12.3, table 6-11 lets an enabled
> Egress Control Vector override it: a clear vector bit routes the request
> directly.

This may well all be correct, but I'd prefer not to touch grouping in
this series working on p2pdma.. The grouping logic is already not
doing a good job with the spec and we can't change it because it
creates regression.

-- 
Jason

[PATCH v4 11/18] PCI/P2PDMA: Derive peer-to-peer routing from ACS control bits

[Leon Romanovsky]

From: Leon Romanovsky <leonro@nvidia.com>

pci_bridge_has_acs_redir() treats Request Redirect and Completion Redirect
as interchangeable for a P2PDMA transaction. On asymmetric fabrics this
sends an otherwise direct mapping through the host bridge.

The client issues Requests to provider memory, while the provider emits
read Completions. Evaluate Request Redirect for the former and Completion
Redirect for the latter. Leave Egress Control conservative until the target
vector is available.

Tested-by: Tushar Dave <tdave@nvidia.com>
Fixes: 52916982af48 ("PCI/P2PDMA: Support peer-to-peer memory")
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 drivers/pci/p2pdma.c | 49 ++++++++++++++++++++++++++++++++++---------------
 1 file changed, 34 insertions(+), 15 deletions(-)

diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
index a364008bbf50..1eda7b2135d7 100644
--- a/drivers/pci/p2pdma.c
+++ b/drivers/pci/p2pdma.c
@@ -21,6 +21,8 @@
 #include <linux/seq_buf.h>
 #include <linux/xarray.h>
 
+#include "pci.h"
+
 /*
  * Lifetime and RCU usage
  *
@@ -536,26 +538,35 @@ static struct pci_dev *find_parent_pci_dev(struct device *dev)
 	return NULL;
 }
 
-/*
- * Check if a PCI bridge has its ACS redirection bits set to redirect P2P
- * TLPs upstream via ACS. Returns 1 if the packets will be redirected
- * upstream, 0 otherwise.
- */
-static int pci_bridge_has_acs_redir(struct pci_dev *pdev)
+enum pci_acs_p2pdma_state {
+	PCI_ACS_P2PDMA_DIRECT,
+	PCI_ACS_P2PDMA_REDIRECT,
+};
+
+enum pci_acs_p2pdma_tlp {
+	PCI_ACS_P2PDMA_TLP_REQUEST,
+	PCI_ACS_P2PDMA_TLP_COMPLETION,
+};
+
+static enum pci_acs_p2pdma_state
+pci_acs_p2pdma_state(struct pci_dev *pdev, enum pci_acs_p2pdma_tlp tlp)
 {
 	int pos;
 	u16 ctrl;
 
 	pos = pdev->acs_cap;
 	if (!pos)
-		return 0;
+		return PCI_ACS_P2PDMA_DIRECT;
 
-	pci_read_config_word(pdev, pos + PCI_ACS_CTRL, &ctrl);
+	if (pci_read_config_word(pdev, pos + PCI_ACS_CTRL, &ctrl))
+		return PCI_ACS_P2PDMA_REDIRECT;
 
-	if (ctrl & (PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_EC))
-		return 1;
+	if (tlp == PCI_ACS_P2PDMA_TLP_COMPLETION)
+		return ctrl & PCI_ACS_CR ? PCI_ACS_P2PDMA_REDIRECT :
+					   PCI_ACS_P2PDMA_DIRECT;
 
-	return 0;
+	return ctrl & (PCI_ACS_RR | PCI_ACS_EC) ?
+		PCI_ACS_P2PDMA_REDIRECT : PCI_ACS_P2PDMA_DIRECT;
 }
 
 static void seq_buf_print_bus_devfn(struct seq_buf *buf, struct pci_dev *pdev)
@@ -733,6 +744,10 @@ static unsigned long map_types_idx(struct pci_dev *client)
  * then to Device B. The mapping type returned depends on the ACS
  * redirection setting of the ports along the path.
  *
+ * The client initiates Requests to provider memory. Check Request Redirect
+ * on the client path and Completion Redirect for read Completions on the
+ * provider path.
+ *
  * If ACS redirect is set on any port in the path, traffic between the
  * devices will go through the host bridge, so return
  * PCI_P2PDMA_MAP_THRU_HOST_BRIDGE; otherwise return
@@ -767,7 +782,9 @@ calc_map_type_and_dist(struct pci_dev *provider, struct pci_dev *client,
 	while (a) {
 		dist_b = 0;
 
-		if (pci_bridge_has_acs_redir(a)) {
+		if (pci_acs_p2pdma_state(a,
+					 PCI_ACS_P2PDMA_TLP_COMPLETION) ==
+		    PCI_ACS_P2PDMA_REDIRECT) {
 			seq_buf_print_bus_devfn(&acs_list, a);
 			acs_cnt++;
 		}
@@ -796,7 +813,9 @@ calc_map_type_and_dist(struct pci_dev *provider, struct pci_dev *client,
 		if (a == bb)
 			break;
 
-		if (pci_bridge_has_acs_redir(bb)) {
+		if (pci_acs_p2pdma_state(bb,
+					 PCI_ACS_P2PDMA_TLP_REQUEST) ==
+		    PCI_ACS_P2PDMA_REDIRECT) {
 			seq_buf_print_bus_devfn(&acs_list, bb);
 			acs_cnt++;
 		}
@@ -1150,10 +1169,10 @@ EXPORT_SYMBOL_GPL(pci_p2pmem_publish);
 /**
  * pci_p2pdma_map_type - Determine the mapping type for P2PDMA transfers
  * @provider: P2PDMA provider structure
- * @dev: Target device for the transfer
+ * @dev: Client device that initiates the transfer
  *
  * Determines how peer-to-peer DMA transfers should be mapped between
- * the provider and the target device. The mapping type indicates whether
+ * the provider and the client device. The mapping type indicates whether
  * the transfer can be done directly through PCI switches or must go
  * through the host bridge.
  */

-- 
2.55.0

[PATCH v4 12/18] PCI/P2PDMA: Honor ACS egress control vectors

[Leon Romanovsky]

From: Leon Romanovsky <leonro@nvidia.com>

An enabled Egress Control bit does not by itself redirect a peer Request.
The result depends on the vector bit for the target port; a set bit without
Request Redirect is an ACS Violation, while a clear bit permits direct
routing.

Evaluate controls only at the path divergence: Request Redirect and Egress
Control on the client-side port, and Completion Redirect on the
provider-side port. Below it, traffic toward the other branch already
routes upstream.

Reject unreadable vectors and ACS Violations. Neither condition redirects
the transaction upstream, so a host-bridge mapping cannot make the path
usable.

Tested-by: Tushar Dave <tdave@nvidia.com>
Fixes: 52916982af48 ("PCI/P2PDMA: Support peer-to-peer memory")
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 drivers/pci/p2pdma.c       | 127 +++++++++++++++++++++++++++++++--------------
 include/linux/pci-p2pdma.h |   8 +--
 2 files changed, 91 insertions(+), 44 deletions(-)

diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
index 1eda7b2135d7..bcbbdbd8ee47 100644
--- a/drivers/pci/p2pdma.c
+++ b/drivers/pci/p2pdma.c
@@ -541,6 +541,7 @@ static struct pci_dev *find_parent_pci_dev(struct device *dev)
 enum pci_acs_p2pdma_state {
 	PCI_ACS_P2PDMA_DIRECT,
 	PCI_ACS_P2PDMA_REDIRECT,
+	PCI_ACS_P2PDMA_NOT_SUPPORTED,
 };
 
 enum pci_acs_p2pdma_tlp {
@@ -549,9 +550,10 @@ enum pci_acs_p2pdma_tlp {
 };
 
 static enum pci_acs_p2pdma_state
-pci_acs_p2pdma_state(struct pci_dev *pdev, enum pci_acs_p2pdma_tlp tlp)
+pci_acs_p2pdma_state(struct pci_dev *pdev, struct pci_dev *target,
+		     enum pci_acs_p2pdma_tlp tlp)
 {
-	int pos;
+	int pos, ret;
 	u16 ctrl;
 
 	pos = pdev->acs_cap;
@@ -559,14 +561,31 @@ pci_acs_p2pdma_state(struct pci_dev *pdev, enum pci_acs_p2pdma_tlp tlp)
 		return PCI_ACS_P2PDMA_DIRECT;
 
 	if (pci_read_config_word(pdev, pos + PCI_ACS_CTRL, &ctrl))
-		return PCI_ACS_P2PDMA_REDIRECT;
+		return PCI_ACS_P2PDMA_NOT_SUPPORTED;
 
 	if (tlp == PCI_ACS_P2PDMA_TLP_COMPLETION)
 		return ctrl & PCI_ACS_CR ? PCI_ACS_P2PDMA_REDIRECT :
 					   PCI_ACS_P2PDMA_DIRECT;
 
-	return ctrl & (PCI_ACS_RR | PCI_ACS_EC) ?
-		PCI_ACS_P2PDMA_REDIRECT : PCI_ACS_P2PDMA_DIRECT;
+	/* EC applies only to Requests where the target is known. */
+	if (!target || !(ctrl & PCI_ACS_EC))
+		return ctrl & PCI_ACS_RR ?
+			PCI_ACS_P2PDMA_REDIRECT : PCI_ACS_P2PDMA_DIRECT;
+
+	/*
+	 * PCIe r7.0, sec 6.12.3, table 6-11: a set Egress Control Vector
+	 * bit redirects the request only when Request Redirect is set.  With
+	 * Request Redirect clear, the request is handled as an ACS Violation.
+	 * A clear vector bit permits the Request to route directly.
+	 */
+	ret = pci_acs_egress_ctrl_is_set(pdev, target);
+	if (ret < 0)
+		return PCI_ACS_P2PDMA_NOT_SUPPORTED;
+	if (ret)
+		return ctrl & PCI_ACS_RR ? PCI_ACS_P2PDMA_REDIRECT :
+						PCI_ACS_P2PDMA_NOT_SUPPORTED;
+
+	return PCI_ACS_P2PDMA_DIRECT;
 }
 
 static void seq_buf_print_bus_devfn(struct seq_buf *buf, struct pci_dev *pdev)
@@ -744,14 +763,14 @@ static unsigned long map_types_idx(struct pci_dev *client)
  * then to Device B. The mapping type returned depends on the ACS
  * redirection setting of the ports along the path.
  *
- * The client initiates Requests to provider memory. Check Request Redirect
- * on the client path and Completion Redirect for read Completions on the
- * provider path.
+ * The client initiates Requests to provider memory. At the path divergence,
+ * check Request Redirect and Egress Control on the client-side port, and
+ * Completion Redirect for read Completions on the provider-side port.
  *
- * If ACS redirect is set on any port in the path, traffic between the
- * devices will go through the host bridge, so return
- * PCI_P2PDMA_MAP_THRU_HOST_BRIDGE; otherwise return
- * PCI_P2PDMA_MAP_BUS_ADDR.
+ * If ACS redirects traffic at either divergence port, return
+ * PCI_P2PDMA_MAP_THRU_HOST_BRIDGE. If ACS blocks the direct path or leaves
+ * its routing indeterminate, return PCI_P2PDMA_MAP_NOT_SUPPORTED. Otherwise,
+ * return PCI_P2PDMA_MAP_BUS_ADDR.
  *
  * Any two devices that have a data path that goes through the host bridge
  * will consult a whitelist. If the host bridge is in the whitelist, return
@@ -765,9 +784,12 @@ calc_map_type_and_dist(struct pci_dev *provider, struct pci_dev *client,
 {
 	enum pci_p2pdma_map_type map_type = PCI_P2PDMA_MAP_THRU_HOST_BRIDGE;
 	struct pci_dev *a = provider, *b = client, *bb;
+	struct pci_dev *a_child = NULL, *b_child = NULL;
+	struct pci_dev *acs_unsupported = NULL;
+	enum pci_acs_p2pdma_state state;
 	struct pci_p2pdma *p2pdma;
 	struct seq_buf acs_list;
-	int acs_cnt = 0;
+	int acs_redirect_cnt = 0;
 	int dist_a = 0;
 	int dist_b = 0;
 	char buf[128];
@@ -781,51 +803,66 @@ calc_map_type_and_dist(struct pci_dev *provider, struct pci_dev *client,
 	 */
 	while (a) {
 		dist_b = 0;
-
-		if (pci_acs_p2pdma_state(a,
-					 PCI_ACS_P2PDMA_TLP_COMPLETION) ==
-		    PCI_ACS_P2PDMA_REDIRECT) {
-			seq_buf_print_bus_devfn(&acs_list, a);
-			acs_cnt++;
-		}
-
+		b_child = NULL;
 		bb = b;
 
 		while (bb) {
 			if (a == bb)
-				goto check_b_path_acs;
+				goto check_paths_acs;
 
+			b_child = bb;
 			bb = pci_upstream_bridge(bb);
 			dist_b++;
 		}
 
+		a_child = a;
 		a = pci_upstream_bridge(a);
 		dist_a++;
 	}
 
+	/*
+	 * The paths share no upstream bridge, so there is no direct path for
+	 * ACS to gate: PCI_P2PDMA_MAP_BUS_ADDR is not reachable here and the
+	 * request can only get to the peer through the host bridge.
+	 */
 	*dist = dist_a + dist_b;
 	goto map_through_host_bridge;
 
-check_b_path_acs:
-	bb = b;
-
-	while (bb) {
-		if (a == bb)
-			break;
+check_paths_acs:
+	*dist = dist_a + dist_b;
 
-		if (pci_acs_p2pdma_state(bb,
-					 PCI_ACS_P2PDMA_TLP_REQUEST) ==
-		    PCI_ACS_P2PDMA_REDIRECT) {
-			seq_buf_print_bus_devfn(&acs_list, bb);
-			acs_cnt++;
+	/*
+	 * ACS P2P routing controls apply where a TLP can route toward the peer
+	 * or upstream. Below that divergence, its only route toward the other
+	 * branch is upstream, so redirect controls do not affect the path.
+	 */
+	if (a_child && b_child) {
+		state = pci_acs_p2pdma_state(a_child, NULL,
+					     PCI_ACS_P2PDMA_TLP_COMPLETION);
+		if (state != PCI_ACS_P2PDMA_DIRECT) {
+			seq_buf_print_bus_devfn(&acs_list, a_child);
+			if (state == PCI_ACS_P2PDMA_REDIRECT)
+				acs_redirect_cnt++;
+			else if (!acs_unsupported)
+				acs_unsupported = a_child;
 		}
 
-		bb = pci_upstream_bridge(bb);
+		state = pci_acs_p2pdma_state(b_child, a_child,
+					     PCI_ACS_P2PDMA_TLP_REQUEST);
+		if (state != PCI_ACS_P2PDMA_DIRECT) {
+			seq_buf_print_bus_devfn(&acs_list, b_child);
+			if (state == PCI_ACS_P2PDMA_REDIRECT)
+				acs_redirect_cnt++;
+			else if (!acs_unsupported)
+				acs_unsupported = b_child;
+		}
 	}
 
-	*dist = dist_a + dist_b;
-
-	if (!acs_cnt) {
+	/*
+	 * Below a shared upstream bridge, a path that neither divergence port
+	 * redirects or blocks routes the request directly.
+	 */
+	if (!acs_unsupported && !acs_redirect_cnt) {
 		map_type = PCI_P2PDMA_MAP_BUS_ADDR;
 		goto done;
 	}
@@ -834,12 +871,22 @@ calc_map_type_and_dist(struct pci_dev *provider, struct pci_dev *client,
 		/* Drop the final semicolon; the list is not empty here. */
 		if (!seq_buf_has_overflowed(&acs_list))
 			acs_list.buffer[acs_list.len - 1] = '\0';
-		pci_warn(client, "ACS redirect is set between the client and provider (%s)\n",
-			 pci_name(provider));
-		pci_warn(client, "to disable ACS redirect for this path, add the kernel parameter: pci=disable_acs_redir=%s\n",
+		if (acs_unsupported)
+			pci_warn(client, "ACS leaves no usable direct P2P path to provider %s at %s\n",
+				 pci_name(provider), pci_name(acs_unsupported));
+		else
+			pci_warn(client, "ACS redirect is set between the client and provider (%s)\n",
+				 pci_name(provider));
+		pci_warn(client, "to disable ACS controls for this path, add the kernel parameter: pci=disable_acs_redir=%s\n",
 			 seq_buf_str(&acs_list));
 	}
 
+	/* A blocked or indeterminate direct path is not an upstream redirect. */
+	if (acs_unsupported) {
+		map_type = PCI_P2PDMA_MAP_NOT_SUPPORTED;
+		goto done;
+	}
+
 map_through_host_bridge:
 	if (!cpu_supports_p2pdma() &&
 	    !host_bridge_whitelist(provider, client, verbose)) {
diff --git a/include/linux/pci-p2pdma.h b/include/linux/pci-p2pdma.h
index 873de20a2247..dd17501ba1b6 100644
--- a/include/linux/pci-p2pdma.h
+++ b/include/linux/pci-p2pdma.h
@@ -42,10 +42,10 @@ enum pci_p2pdma_map_type {
 	PCI_P2PDMA_MAP_NONE,
 
 	/*
-	 * PCI_P2PDMA_MAP_NOT_SUPPORTED: Indicates the transaction will
-	 * traverse the host bridge and the host bridge is not in the
-	 * allowlist. DMA Mapping routines should return an error when
-	 * this is returned.
+	 * PCI_P2PDMA_MAP_NOT_SUPPORTED: Indicates no safe mapping is available,
+	 * for example because ACS blocks the direct path or the required host
+	 * bridge is not in the allowlist. DMA Mapping routines should return an
+	 * error when this is returned.
 	 */
 	PCI_P2PDMA_MAP_NOT_SUPPORTED,
 

-- 
2.55.0

[PATCH v4 13/18] PCI/P2PDMA: Document ACS egress control handling

[Leon Romanovsky]

From: Leon Romanovsky <leonro@nvidia.com>

ACS controls are directional and affect only a peer-versus-upstream routing
decision, but the P2PDMA documentation describes them as path-wide. This
can make valid asymmetric or nested configurations appear unsupported.

Document the fixed transaction roles and the two ports at the path
divergence. Request Redirect and Egress Control apply on the client-side
port, while Completion Redirect applies on the provider-side port. Controls
below the divergence cannot change traffic whose only route is upstream.

Distinguish redirects, which establish a host route, from ACS Violations
and unreadable routing state, which must be rejected.

Tested-by: Tushar Dave <tdave@nvidia.com>
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 Documentation/admin-guide/kernel-parameters.txt |  9 +++++----
 Documentation/driver-api/pci/p2pdma.rst         | 17 +++++++++++++++++
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index b5493a7f8f22..5c3ed4fd439c 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5226,10 +5226,11 @@ Kernel parameters
 		disable_acs_redir=<pci_dev>[; ...]
 				Specify one or more PCI devices (in the format
 				specified above) separated by semicolons.
-				Each device specified will have the PCI ACS
-				redirect capabilities forced off which will
-				allow P2P traffic between devices through
-				bridges without forcing it upstream. Note:
+				Each device specified will have the PCI ACS P2P
+				Request Redirect, Completion Redirect, and Egress
+				Control features forced off. This may allow P2P
+				traffic through bridges that would otherwise be
+				redirected upstream or blocked. Note:
 				this removes isolation between devices and
 				may put more devices in an IOMMU group.
 		config_acs=
diff --git a/Documentation/driver-api/pci/p2pdma.rst b/Documentation/driver-api/pci/p2pdma.rst
index a7fd426c3685..4e21942eab65 100644
--- a/Documentation/driver-api/pci/p2pdma.rst
+++ b/Documentation/driver-api/pci/p2pdma.rst
@@ -15,6 +15,23 @@ then based on the ACS settings the transaction can route entirely within
 the PCIe hierarchy and never reach the root port. The kernel will evaluate
 the PCIe topology and always permit P2P in these well-defined cases.
 
+ACS P2P Egress Control does not, by itself, force a transaction upstream. A
+clear Egress Control Vector bit for the peer port permits direct routing; a
+set bit redirects the request upstream when P2P Request Redirect is enabled.
+When Request Redirect is disabled, a set vector bit causes an ACS Violation
+instead.
+
+The client remains the PCIe requester when it reads or writes provider memory.
+Where the paths diverge, the kernel therefore evaluates P2P Request Redirect
+and Egress Control on the client-side port, and P2P Completion Redirect on the
+provider-side port for completions from a read. Below the divergence, the route
+toward the other branch is already upstream, so those P2P redirect controls do
+not affect it. Redirect controls for the reverse transaction directions do not
+affect the mapping. P2P DMA is routed through the host bridge when either
+applicable port redirects. If an applicable port blocks the direct path or its
+routing cannot be determined, P2P DMA is rejected because neither condition
+establishes an upstream route.
+
 This evaluation covers the ACS controls that govern Requests carrying an
 Untranslated address. Unless ACS Translation Blocking is enabled, a Port
 with ACS Direct Translated P2P enabled routes a Request carrying a Translated

-- 
2.55.0

[PATCH v4 14/18] PCI/P2PDMA: Extract pure ACS routing decision helpers

[Leon Romanovsky]

From: Leon Romanovsky <leonro@nvidia.com>

The ACS routing decision and Egress Control Vector size calculation are
mixed with config-space I/O, which makes their branch-heavy behavior
difficult to exercise independently.

Extract pci_acs_p2pdma_decision() with explicit Request or Completion type,
target availability, and vector state inputs. Extract
pci_acs_egress_port_valid() for the encoded vector-size boundary. Keep
config access in their callers and expose only the pure helpers under
CONFIG_KUNIT.

Tested-by: Tushar Dave <tdave@nvidia.com>
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 drivers/pci/p2pdma.c | 70 ++++++++++++++++++++++++++++------------------------
 drivers/pci/pci.c    | 26 ++++++++++++++++---
 drivers/pci/pci.h    | 23 +++++++++++++++++
 3 files changed, 83 insertions(+), 36 deletions(-)

diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
index bcbbdbd8ee47..632f2d8c51e0 100644
--- a/drivers/pci/p2pdma.c
+++ b/drivers/pci/p2pdma.c
@@ -538,22 +538,45 @@ static struct pci_dev *find_parent_pci_dev(struct device *dev)
 	return NULL;
 }
 
-enum pci_acs_p2pdma_state {
-	PCI_ACS_P2PDMA_DIRECT,
-	PCI_ACS_P2PDMA_REDIRECT,
-	PCI_ACS_P2PDMA_NOT_SUPPORTED,
-};
+/*
+ * PCIe r7.0, sec 6.12.3, table 6-11: decide how a peer-to-peer TLP at an
+ * ACS-capable ingress port routes, given its ACS Control register @ctrl and
+ * whether it is a Request or Completion (@tlp).
+ *
+ * Egress Control applies only to Requests where the target is known (the path
+ * divergence). There, a set vector bit redirects the Request only when Request
+ * Redirect is set; with Request Redirect clear it is an ACS Violation. A clear
+ * vector bit permits the Request to route directly. @has_target and @egress
+ * describe that target and its Egress Control Vector bit. Completion routing
+ * depends only on Completion Redirect.
+ */
+VISIBLE_IF_KUNIT enum pci_acs_p2pdma_state
+pci_acs_p2pdma_decision(u16 ctrl, enum pci_acs_p2pdma_tlp tlp,
+			bool has_target, int egress)
+{
+	if (tlp == PCI_ACS_P2PDMA_TLP_COMPLETION)
+		return ctrl & PCI_ACS_CR ? PCI_ACS_P2PDMA_REDIRECT :
+					   PCI_ACS_P2PDMA_DIRECT;
 
-enum pci_acs_p2pdma_tlp {
-	PCI_ACS_P2PDMA_TLP_REQUEST,
-	PCI_ACS_P2PDMA_TLP_COMPLETION,
-};
+	if (!has_target || !(ctrl & PCI_ACS_EC))
+		return ctrl & PCI_ACS_RR ?
+			PCI_ACS_P2PDMA_REDIRECT : PCI_ACS_P2PDMA_DIRECT;
+
+	if (egress < 0)
+		return PCI_ACS_P2PDMA_NOT_SUPPORTED;
+	if (egress)
+		return ctrl & PCI_ACS_RR ? PCI_ACS_P2PDMA_REDIRECT :
+					   PCI_ACS_P2PDMA_NOT_SUPPORTED;
+
+	return PCI_ACS_P2PDMA_DIRECT;
+}
+EXPORT_SYMBOL_IF_KUNIT(pci_acs_p2pdma_decision);
 
 static enum pci_acs_p2pdma_state
 pci_acs_p2pdma_state(struct pci_dev *pdev, struct pci_dev *target,
 		     enum pci_acs_p2pdma_tlp tlp)
 {
-	int pos, ret;
+	int pos, egress = 0;
 	u16 ctrl;
 
 	pos = pdev->acs_cap;
@@ -563,29 +586,12 @@ pci_acs_p2pdma_state(struct pci_dev *pdev, struct pci_dev *target,
 	if (pci_read_config_word(pdev, pos + PCI_ACS_CTRL, &ctrl))
 		return PCI_ACS_P2PDMA_NOT_SUPPORTED;
 
-	if (tlp == PCI_ACS_P2PDMA_TLP_COMPLETION)
-		return ctrl & PCI_ACS_CR ? PCI_ACS_P2PDMA_REDIRECT :
-					   PCI_ACS_P2PDMA_DIRECT;
-
-	/* EC applies only to Requests where the target is known. */
-	if (!target || !(ctrl & PCI_ACS_EC))
-		return ctrl & PCI_ACS_RR ?
-			PCI_ACS_P2PDMA_REDIRECT : PCI_ACS_P2PDMA_DIRECT;
+	/* Egress Control applies only to Requests with a known target. */
+	if (tlp == PCI_ACS_P2PDMA_TLP_REQUEST && target &&
+	    (ctrl & PCI_ACS_EC))
+		egress = pci_acs_egress_ctrl_is_set(pdev, target);
 
-	/*
-	 * PCIe r7.0, sec 6.12.3, table 6-11: a set Egress Control Vector
-	 * bit redirects the request only when Request Redirect is set.  With
-	 * Request Redirect clear, the request is handled as an ACS Violation.
-	 * A clear vector bit permits the Request to route directly.
-	 */
-	ret = pci_acs_egress_ctrl_is_set(pdev, target);
-	if (ret < 0)
-		return PCI_ACS_P2PDMA_NOT_SUPPORTED;
-	if (ret)
-		return ctrl & PCI_ACS_RR ? PCI_ACS_P2PDMA_REDIRECT :
-						PCI_ACS_P2PDMA_NOT_SUPPORTED;
-
-	return PCI_ACS_P2PDMA_DIRECT;
+	return pci_acs_p2pdma_decision(ctrl, tlp, !!target, egress);
 }
 
 static void seq_buf_print_bus_devfn(struct seq_buf *buf, struct pci_dev *pdev)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index a633f473590f..d900fdb6f37d 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3562,6 +3562,26 @@ static bool pci_acs_egress_vector_port(const struct pci_dev *dev)
 	       type == PCI_EXP_TYPE_DOWNSTREAM;
 }
 
+/**
+ * pci_acs_egress_port_valid - Is a target port within the Egress Control Vector
+ * @acs_caps: the ingress port's ACS Capability register
+ * @target_port: the target Downstream Port number
+ *
+ * The Egress Control Vector Size occupies bits 15:8 of the ACS Capability
+ * register (PCIe r7.0, sec 7.7.12).  A size of 0 encodes 256 bits, so
+ * every port number is addressable.
+ *
+ * Return: %true if @target_port has a bit in the Egress Control Vector.
+ */
+VISIBLE_IF_KUNIT
+bool pci_acs_egress_port_valid(u16 acs_caps, u8 target_port)
+{
+	unsigned int vector_size = acs_caps >> 8;
+
+	return !vector_size || target_port < vector_size;
+}
+EXPORT_SYMBOL_IF_KUNIT(pci_acs_egress_port_valid);
+
 /**
  * pci_acs_egress_ctrl_is_set - Read an ACS Egress Control Vector bit
  * @pdev: ingress Root or Switch Downstream Port
@@ -3572,7 +3592,6 @@ static bool pci_acs_egress_vector_port(const struct pci_dev *dev)
  */
 int pci_acs_egress_ctrl_is_set(struct pci_dev *pdev, struct pci_dev *target)
 {
-	unsigned int vector_size;
 	u32 lnkcap, vector;
 	u8 target_port;
 	int ret;
@@ -3597,10 +3616,8 @@ int pci_acs_egress_ctrl_is_set(struct pci_dev *pdev, struct pci_dev *target)
 		return pcibios_err_to_errno(ret);
 
 	target_port = FIELD_GET(PCI_EXP_LNKCAP_PN, lnkcap);
-	vector_size = pdev->acs_capabilities >> 8;
 
-	/* An Egress Control Vector Size of 0 encodes 256 bits. */
-	if (vector_size && target_port >= vector_size)
+	if (!pci_acs_egress_port_valid(pdev->acs_capabilities, target_port))
 		return -ERANGE;
 
 	ret = pci_read_config_dword(pdev,
@@ -3612,6 +3629,7 @@ int pci_acs_egress_ctrl_is_set(struct pci_dev *pdev, struct pci_dev *target)
 
 	return !!(vector & BIT(target_port % 32));
 }
+EXPORT_SYMBOL_IF_KUNIT(pci_acs_egress_ctrl_is_set);
 
 static bool pci_acs_flags_enabled(struct pci_dev *pdev, u16 acs_flags,
 				  enum pci_acs_scope scope)
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 32394e349766..4593fb43f96c 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -2,6 +2,7 @@
 #ifndef DRIVERS_PCI_H
 #define DRIVERS_PCI_H
 
+#include <kunit/visibility.h>
 #include <linux/align.h>
 #include <linux/bitfield.h>
 #include <linux/pci.h>
@@ -1079,6 +1080,28 @@ static inline bool pci_acs_rr_ineffective(u32 ctrl, u16 acs_flags,
 }
 
 int pci_acs_egress_ctrl_is_set(struct pci_dev *pdev, struct pci_dev *target);
+
+/*
+ * Peer-to-peer routing decision for an ACS-capable ingress port, per
+ * PCIe r7.0, sec 6.12.3, table 6-11.
+ */
+enum pci_acs_p2pdma_state {
+	PCI_ACS_P2PDMA_DIRECT,		/* peer-to-peer permitted directly */
+	PCI_ACS_P2PDMA_REDIRECT,	/* redirected upstream to host bridge */
+	PCI_ACS_P2PDMA_NOT_SUPPORTED,	/* no usable peer-to-peer route */
+};
+
+enum pci_acs_p2pdma_tlp {
+	PCI_ACS_P2PDMA_TLP_REQUEST,
+	PCI_ACS_P2PDMA_TLP_COMPLETION,
+};
+
+#if IS_ENABLED(CONFIG_KUNIT)
+bool pci_acs_egress_port_valid(u16 acs_caps, u8 target_port);
+enum pci_acs_p2pdma_state
+pci_acs_p2pdma_decision(u16 ctrl, enum pci_acs_p2pdma_tlp tlp,
+			bool has_target, int egress);
+#endif
 #ifdef CONFIG_PCI_QUIRKS
 int pci_dev_specific_acs_enabled(struct pci_dev *dev, u16 acs_flags,
 				 enum pci_acs_scope scope);

-- 
2.55.0

[PATCH v4 15/18] PCI/P2PDMA: Add KUnit tests for ACS routing decisions

[Leon Romanovsky]

From: Leon Romanovsky <leonro@nvidia.com>

The ACS routing decision combines TLP type, redirect controls, target
availability, and the Egress Control Vector. Hardware cannot reliably
provide every combination needed to cover this matrix.

Add KUnit cases for Request and Completion routing, including
reverse-direction controls that must be ignored and every Egress Control
outcome. Cover vector-size boundaries and config-space access with fake
pci_ops so the result does not depend on available PCIe hardware.

Tested-by: Tushar Dave <tdave@nvidia.com>
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 drivers/pci/Kconfig        |  15 ++
 drivers/pci/Makefile       |   1 +
 drivers/pci/pci_acs_test.c | 444 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 460 insertions(+)

diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
index 0c7408509ba2..30ad7f407c6f 100644
--- a/drivers/pci/Kconfig
+++ b/drivers/pci/Kconfig
@@ -226,6 +226,21 @@ config PCI_P2PDMA
 
 	  If unsure, say N.
 
+config PCI_ACS_KUNIT_TEST
+	tristate "KUnit tests for PCI ACS P2P routing" if !KUNIT_ALL_TESTS
+	depends on PCI_P2PDMA && KUNIT
+	default KUNIT_ALL_TESTS
+	help
+	  Enable KUnit tests for the PCI ACS peer-to-peer routing decision
+	  logic (PCIe ACS Egress Control, table 6-11), including the code
+	  paths that require an ACS Egress Control Vector and so cannot be
+	  exercised on typical peer-to-peer hardware.
+
+	  For more information on KUnit and unit tests in general, refer to
+	  the KUnit documentation in Documentation/dev-tools/kunit/.
+
+	  If unsure, say N.
+
 config PCI_LABEL
 	def_bool y if (DMI || ACPI)
 	select NLS
diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
index 41ebc3b9a518..6305d128d3df 100644
--- a/drivers/pci/Makefile
+++ b/drivers/pci/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_PCI_STUB)		+= pci-stub.o
 obj-$(CONFIG_PCI_PF_STUB)	+= pci-pf-stub.o
 obj-$(CONFIG_PCI_ECAM)		+= ecam.o
 obj-$(CONFIG_PCI_P2PDMA)	+= p2pdma.o
+obj-$(CONFIG_PCI_ACS_KUNIT_TEST) += pci_acs_test.o
 obj-$(CONFIG_XEN_PCIDEV_FRONTEND) += xen-pcifront.o
 obj-$(CONFIG_VGA_ARB)		+= vgaarb.o
 obj-$(CONFIG_PCI_DOE)		+= doe.o
diff --git a/drivers/pci/pci_acs_test.c b/drivers/pci/pci_acs_test.c
new file mode 100644
index 000000000000..aec7d7636f70
--- /dev/null
+++ b/drivers/pci/pci_acs_test.c
@@ -0,0 +1,444 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit tests for PCI ACS peer-to-peer routing decision logic.
+ *
+ * These exercise the pure helpers factored out of the ACS Egress Control
+ * handling (PCIe r7.0, sec 6.12.3, table 6-11).  They cover the code paths
+ * that require an ACS Egress Control Vector, which cannot be reached on the
+ * peer-to-peer hardware commonly available for testing.
+ */
+#include <kunit/test.h>
+
+#include <linux/pci.h>
+#include <linux/pci_regs.h>
+
+#include "pci.h"
+
+/* pci_acs_p2pdma_decision(): the table 6-11 truth table. */
+
+struct acs_decision_case {
+	const char *desc;
+	u16 ctrl;
+	enum pci_acs_p2pdma_tlp tlp;
+	bool has_target;
+	int egress;
+	enum pci_acs_p2pdma_state expect;
+};
+
+/* Shorthands to keep the table below readable. */
+#define ACS_DIRECT	PCI_ACS_P2PDMA_DIRECT
+#define ACS_REDIR	PCI_ACS_P2PDMA_REDIRECT
+#define ACS_NO_P2P	PCI_ACS_P2PDMA_NOT_SUPPORTED
+#define ACS_REQ		PCI_ACS_P2PDMA_TLP_REQUEST
+#define ACS_CPL		PCI_ACS_P2PDMA_TLP_COMPLETION
+
+static const struct acs_decision_case acs_decision_cases[] = {
+	/* Completion routing depends only on Completion Redirect. */
+	{ "cpl/none", 0, ACS_CPL, false, 0, ACS_DIRECT },
+	{ "cpl/rr", PCI_ACS_RR, ACS_CPL, false, 0, ACS_DIRECT },
+	{ "cpl/cr", PCI_ACS_CR, ACS_CPL, false, 0, ACS_REDIR },
+	{ "cpl/rr_cr", PCI_ACS_RR | PCI_ACS_CR, ACS_CPL, false, 0,
+	  ACS_REDIR },
+	{ "cpl/ec_target", PCI_ACS_EC | PCI_ACS_RR, ACS_CPL, true, 1,
+	  ACS_DIRECT },
+	{ "cpl/ec_error", PCI_ACS_EC | PCI_ACS_CR, ACS_CPL, true, -ERANGE,
+	  ACS_REDIR },
+
+	/* No Request target known: Egress Control is ignored and RR decides. */
+	{ "req/no_target/none", 0, ACS_REQ, false, 0, ACS_DIRECT },
+	{ "req/no_target/rr", PCI_ACS_RR, ACS_REQ, false, 0, ACS_REDIR },
+	{ "req/no_target/cr", PCI_ACS_CR, ACS_REQ, false, 0, ACS_DIRECT },
+	{ "req/no_target/rr_cr", PCI_ACS_RR | PCI_ACS_CR, ACS_REQ, false, 0,
+	  ACS_REDIR },
+	{ "req/no_target/ec_only", PCI_ACS_EC, ACS_REQ, false, 0,
+	  ACS_DIRECT },
+
+	/* Request target known but EC clear: RR decides, egress is ignored. */
+	{ "req/ec_clear/none", 0, ACS_REQ, true, 0, ACS_DIRECT },
+	{ "req/ec_clear/rr", PCI_ACS_RR, ACS_REQ, true, 0, ACS_REDIR },
+	{ "req/ec_clear/cr", PCI_ACS_CR, ACS_REQ, true, 0, ACS_DIRECT },
+	{ "req/ec_clear/rr_cr", PCI_ACS_RR | PCI_ACS_CR, ACS_REQ, true, 0,
+	  ACS_REDIR },
+
+	/* EC set but vector unreadable: never a usable P2P route. */
+	{ "req/ec/eopnotsupp", PCI_ACS_EC | PCI_ACS_RR, ACS_REQ, true,
+	  -EOPNOTSUPP, ACS_NO_P2P },
+	{ "req/ec/erange", PCI_ACS_EC | PCI_ACS_CR, ACS_REQ, true, -ERANGE,
+	  ACS_NO_P2P },
+
+	/* EC set, vector bit set: redirect iff RR, else ACS Violation. */
+	{ "req/ec/vec_set/none", PCI_ACS_EC, ACS_REQ, true, 1, ACS_NO_P2P },
+	{ "req/ec/vec_set/cr", PCI_ACS_EC | PCI_ACS_CR, ACS_REQ, true, 1,
+	  ACS_NO_P2P },
+	{ "req/ec/vec_set/rr", PCI_ACS_EC | PCI_ACS_RR, ACS_REQ, true, 1,
+	  ACS_REDIR },
+	{ "req/ec/vec_set/rr_cr", PCI_ACS_EC | PCI_ACS_RR | PCI_ACS_CR,
+	  ACS_REQ, true, 1, ACS_REDIR },
+
+	/* EC set, vector bit clear: the Request routes directly. */
+	{ "req/ec/vec_clear/none", PCI_ACS_EC, ACS_REQ, true, 0,
+	  ACS_DIRECT },
+	{ "req/ec/vec_clear/rr", PCI_ACS_EC | PCI_ACS_RR, ACS_REQ, true, 0,
+	  ACS_DIRECT },
+	{ "req/ec/vec_clear/cr", PCI_ACS_EC | PCI_ACS_CR, ACS_REQ, true, 0,
+	  ACS_DIRECT },
+	{ "req/ec/vec_clear/rr_cr", PCI_ACS_EC | PCI_ACS_RR | PCI_ACS_CR,
+	  ACS_REQ, true, 0, ACS_DIRECT },
+};
+
+#undef ACS_DIRECT
+#undef ACS_REDIR
+#undef ACS_NO_P2P
+#undef ACS_REQ
+#undef ACS_CPL
+
+static void acs_decision_desc(const struct acs_decision_case *c, char *desc)
+{
+	strscpy(desc, c->desc, KUNIT_PARAM_DESC_SIZE);
+}
+
+KUNIT_ARRAY_PARAM(acs_decision, acs_decision_cases, acs_decision_desc);
+
+static void pci_acs_p2pdma_decision_test(struct kunit *test)
+{
+	const struct acs_decision_case *c = test->param_value;
+
+	KUNIT_EXPECT_EQ(test,
+			pci_acs_p2pdma_decision(c->ctrl, c->tlp,
+						c->has_target, c->egress),
+			c->expect);
+}
+
+/* pci_acs_egress_port_valid(): the Egress Control Vector Size rule. */
+
+struct egress_valid_case {
+	const char *desc;
+	u16 acs_caps;
+	u8 target_port;
+	bool expect;
+};
+
+static const struct egress_valid_case egress_valid_cases[] = {
+	/* A Vector Size of 0 encodes 256 bits, so every port is addressable. */
+	{ "size0/port0",	0x0000,	  0, true },
+	{ "size0/port255",	0x0000,	255, true },
+	/* Vector Size N (bits 15:8): ports [0, N) are addressable. */
+	{ "size1/port0",	0x0100,	  0, true },
+	{ "size1/port1",	0x0100,	  1, false },
+	{ "size8/port7",	0x0800,	  7, true },
+	{ "size8/port8",	0x0800,	  8, false },
+	{ "size255/port254",	0xff00,	254, true },
+	{ "size255/port255",	0xff00,	255, false },
+};
+
+static void egress_valid_desc(const struct egress_valid_case *c, char *desc)
+{
+	strscpy(desc, c->desc, KUNIT_PARAM_DESC_SIZE);
+}
+
+KUNIT_ARRAY_PARAM(egress_valid, egress_valid_cases, egress_valid_desc);
+
+static void pci_acs_egress_port_valid_test(struct kunit *test)
+{
+	const struct egress_valid_case *c = test->param_value;
+
+	KUNIT_EXPECT_EQ(test,
+			pci_acs_egress_port_valid(c->acs_caps, c->target_port),
+			c->expect);
+}
+
+/*
+ * pci_acs_egress_ctrl_is_set(): drive the config-space reads with a fake pci_ops
+ * so the Egress Control Vector lookup is exercised without real hardware --
+ * the target Port Number from LNKCAP, the vector DWORD at target_port/32, and
+ * the bit at target_port%32.
+ */
+
+/* PCIe Capabilities register value: device/port @type, capability version 2. */
+#define ACS_TEST_PCIE_FLAGS(type)	(((type) << 4) | 0x2)
+#define ACS_DOWNSTREAM			ACS_TEST_PCIE_FLAGS(PCI_EXP_TYPE_DOWNSTREAM)
+#define ACS_ENDPOINT			ACS_TEST_PCIE_FLAGS(PCI_EXP_TYPE_ENDPOINT)
+#define ACS_ROOT_PORT			ACS_TEST_PCIE_FLAGS(PCI_EXP_TYPE_ROOT_PORT)
+#define ACS_PCIE_BRIDGE			ACS_TEST_PCIE_FLAGS(PCI_EXP_TYPE_PCIE_BRIDGE)
+
+struct acs_fake_cfg {
+	unsigned int	pdev_devfn;
+	unsigned int	target_devfn;
+	u16		pdev_acs_cap;
+	u8		target_pcie_cap;
+	u8		target_port;
+	u32		egress_vector[8];	/* full 256-bit vector */
+};
+
+static int acs_fake_cfg_read(struct pci_bus *bus, unsigned int devfn,
+			     int where, int size, u32 *val)
+{
+	struct acs_fake_cfg *cfg = bus->sysdata;
+
+	*val = 0;
+	if (size != 4)
+		return PCIBIOS_SUCCESSFUL;
+
+	if (devfn == cfg->target_devfn &&
+	    where == cfg->target_pcie_cap + PCI_EXP_LNKCAP) {
+		*val = FIELD_PREP(PCI_EXP_LNKCAP_PN, cfg->target_port);
+	} else if (devfn == cfg->pdev_devfn) {
+		int base = cfg->pdev_acs_cap + PCI_ACS_EGRESS_CTL_V;
+
+		if (where >= base &&
+		    where < base + (int)sizeof(cfg->egress_vector))
+			*val = cfg->egress_vector[(where - base) / 4];
+	}
+	return PCIBIOS_SUCCESSFUL;
+}
+
+static int acs_fake_cfg_write(struct pci_bus *bus, unsigned int devfn,
+			      int where, int size, u32 val)
+{
+	return PCIBIOS_SUCCESSFUL;
+}
+
+static struct pci_ops acs_fake_ops = {
+	.read	= acs_fake_cfg_read,
+	.write	= acs_fake_cfg_write,
+};
+
+static struct acs_fake_cfg acs_base_cfg(void)
+{
+	return (struct acs_fake_cfg){
+		.pdev_devfn	 = PCI_DEVFN(0, 0),
+		.target_devfn	 = PCI_DEVFN(1, 0),
+		.pdev_acs_cap	 = 0x100,
+		.target_pcie_cap = 0x40,
+	};
+}
+
+static int acs_egress_ctrl_set(struct kunit *test, struct acs_fake_cfg *cfg,
+			       u16 pdev_acs_caps, u16 pdev_flags, u16 target_flags)
+{
+	struct pci_bus *bus = kunit_kzalloc(test, sizeof(*bus), GFP_KERNEL);
+	struct pci_dev *pdev = kunit_kzalloc(test, sizeof(*pdev), GFP_KERNEL);
+	struct pci_dev *target = kunit_kzalloc(test, sizeof(*target), GFP_KERNEL);
+
+	KUNIT_ASSERT_NOT_NULL(test, bus);
+	KUNIT_ASSERT_NOT_NULL(test, pdev);
+	KUNIT_ASSERT_NOT_NULL(test, target);
+
+	bus->ops = &acs_fake_ops;
+	bus->sysdata = cfg;
+
+	pdev->bus = bus;
+	pdev->devfn = cfg->pdev_devfn;
+	pdev->acs_cap = cfg->pdev_acs_cap;
+	pdev->acs_capabilities = pdev_acs_caps;
+	pdev->pcie_cap = 0x40;
+	pdev->pcie_flags_reg = pdev_flags;
+
+	target->bus = bus;
+	target->devfn = cfg->target_devfn;
+	target->pcie_cap = cfg->target_pcie_cap;
+	target->pcie_flags_reg = target_flags;
+
+	return pci_acs_egress_ctrl_is_set(pdev, target);
+}
+
+static void acs_egress_vector_bit_set_test(struct kunit *test)
+{
+	struct acs_fake_cfg cfg = acs_base_cfg();
+
+	cfg.target_port = 5;
+	cfg.egress_vector[0] = BIT(5);
+
+	KUNIT_EXPECT_EQ(test,
+			acs_egress_ctrl_set(test, &cfg, PCI_ACS_EC | (32 << 8),
+					    ACS_DOWNSTREAM, ACS_DOWNSTREAM),
+			1);
+}
+
+static void acs_egress_vector_bit_clear_test(struct kunit *test)
+{
+	struct acs_fake_cfg cfg = acs_base_cfg();
+
+	cfg.target_port = 5;	/* vector left all-zero */
+
+	KUNIT_EXPECT_EQ(test,
+			acs_egress_ctrl_set(test, &cfg, PCI_ACS_EC | (32 << 8),
+					    ACS_DOWNSTREAM, ACS_DOWNSTREAM),
+			0);
+}
+
+static void acs_egress_high_port_index_test(struct kunit *test)
+{
+	struct acs_fake_cfg cfg = acs_base_cfg();
+
+	/* Port 40 lives in vector DWORD 1, bit 8: exercises target_port/32. */
+	cfg.target_port = 40;
+	cfg.egress_vector[1] = BIT(40 % 32);
+
+	KUNIT_EXPECT_EQ(test,
+			acs_egress_ctrl_set(test, &cfg, PCI_ACS_EC | (64 << 8),
+					    ACS_DOWNSTREAM, ACS_DOWNSTREAM),
+			1);
+}
+
+static void acs_egress_port_out_of_range_test(struct kunit *test)
+{
+	struct acs_fake_cfg cfg = acs_base_cfg();
+
+	/* Vector Size 8, port 40 is beyond it. */
+	cfg.target_port = 40;
+
+	KUNIT_EXPECT_EQ(test,
+			acs_egress_ctrl_set(test, &cfg, PCI_ACS_EC | (8 << 8),
+					    ACS_DOWNSTREAM, ACS_DOWNSTREAM),
+			-ERANGE);
+}
+
+static void acs_egress_no_ec_cap_test(struct kunit *test)
+{
+	struct acs_fake_cfg cfg = acs_base_cfg();
+
+	cfg.target_port = 5;
+
+	/* acs_capabilities without PCI_ACS_EC: unsupported. */
+	KUNIT_EXPECT_EQ(test,
+			acs_egress_ctrl_set(test, &cfg, 32 << 8,
+					    ACS_DOWNSTREAM, ACS_DOWNSTREAM),
+			-EOPNOTSUPP);
+}
+
+static void acs_egress_pdev_not_downstream_test(struct kunit *test)
+{
+	struct acs_fake_cfg cfg = acs_base_cfg();
+
+	cfg.target_port = 5;
+
+	KUNIT_EXPECT_EQ(test,
+			acs_egress_ctrl_set(test, &cfg, PCI_ACS_EC | (32 << 8),
+					    ACS_ENDPOINT, ACS_DOWNSTREAM),
+			-EOPNOTSUPP);
+}
+
+static void acs_egress_target_not_downstream_test(struct kunit *test)
+{
+	struct acs_fake_cfg cfg = acs_base_cfg();
+
+	cfg.target_port = 5;
+
+	KUNIT_EXPECT_EQ(test,
+			acs_egress_ctrl_set(test, &cfg, PCI_ACS_EC | (32 << 8),
+					    ACS_DOWNSTREAM, ACS_ENDPOINT),
+			-EOPNOTSUPP);
+}
+
+/*
+ * The vector is indexed by Port Number only for Root Ports and Switch
+ * Downstream Ports, so a PCI/PCI-X to PCIe Bridge must not be indexed by its
+ * Link Capabilities Port Number.
+ */
+static void acs_egress_pdev_pcie_bridge_test(struct kunit *test)
+{
+	struct acs_fake_cfg cfg = acs_base_cfg();
+
+	cfg.target_port = 5;
+
+	KUNIT_EXPECT_EQ(test,
+			acs_egress_ctrl_set(test, &cfg, PCI_ACS_EC | (32 << 8),
+					    ACS_PCIE_BRIDGE, ACS_DOWNSTREAM),
+			-EOPNOTSUPP);
+}
+
+static void acs_egress_target_pcie_bridge_test(struct kunit *test)
+{
+	struct acs_fake_cfg cfg = acs_base_cfg();
+
+	cfg.target_port = 5;
+
+	KUNIT_EXPECT_EQ(test,
+			acs_egress_ctrl_set(test, &cfg, PCI_ACS_EC | (32 << 8),
+					    ACS_DOWNSTREAM, ACS_PCIE_BRIDGE),
+			-EOPNOTSUPP);
+}
+
+/*
+ * Each vector bit is a Port Number within one Switch or Root Complex (PCIe
+ * r7.0, sec 7.7.12.4), so a target that does not share the ingress port's bus
+ * has no bit here even when the bit at its Port Number is set.
+ */
+static void acs_egress_target_other_bus_test(struct kunit *test)
+{
+	struct acs_fake_cfg cfg = acs_base_cfg();
+	struct pci_bus *bus = kunit_kzalloc(test, sizeof(*bus), GFP_KERNEL);
+	struct pci_bus *other = kunit_kzalloc(test, sizeof(*other), GFP_KERNEL);
+	struct pci_dev *pdev = kunit_kzalloc(test, sizeof(*pdev), GFP_KERNEL);
+	struct pci_dev *target = kunit_kzalloc(test, sizeof(*target), GFP_KERNEL);
+
+	KUNIT_ASSERT_NOT_NULL(test, bus);
+	KUNIT_ASSERT_NOT_NULL(test, other);
+	KUNIT_ASSERT_NOT_NULL(test, pdev);
+	KUNIT_ASSERT_NOT_NULL(test, target);
+
+	cfg.target_port = 5;
+	cfg.egress_vector[0] = BIT(5);
+
+	bus->ops = &acs_fake_ops;
+	bus->sysdata = &cfg;
+	other->ops = &acs_fake_ops;
+	other->sysdata = &cfg;
+
+	pdev->bus = bus;
+	pdev->devfn = cfg.pdev_devfn;
+	pdev->acs_cap = cfg.pdev_acs_cap;
+	pdev->acs_capabilities = PCI_ACS_EC | (32 << 8);
+	pdev->pcie_cap = 0x40;
+	pdev->pcie_flags_reg = ACS_DOWNSTREAM;
+
+	target->bus = other;
+	target->devfn = cfg.target_devfn;
+	target->pcie_cap = cfg.target_pcie_cap;
+	target->pcie_flags_reg = ACS_DOWNSTREAM;
+
+	KUNIT_EXPECT_EQ(test, pci_acs_egress_ctrl_is_set(pdev, target),
+			-EOPNOTSUPP);
+}
+
+/* A Root Port is a valid ingress and egress port for the vector. */
+static void acs_egress_root_port_test(struct kunit *test)
+{
+	struct acs_fake_cfg cfg = acs_base_cfg();
+
+	cfg.target_port = 5;
+	cfg.egress_vector[0] = BIT(5);
+
+	KUNIT_EXPECT_EQ(test,
+			acs_egress_ctrl_set(test, &cfg, PCI_ACS_EC | (32 << 8),
+					    ACS_ROOT_PORT, ACS_ROOT_PORT),
+			1);
+}
+
+static struct kunit_case pci_acs_test_cases[] = {
+	KUNIT_CASE_PARAM(pci_acs_p2pdma_decision_test, acs_decision_gen_params),
+	KUNIT_CASE_PARAM(pci_acs_egress_port_valid_test, egress_valid_gen_params),
+	KUNIT_CASE(acs_egress_vector_bit_set_test),
+	KUNIT_CASE(acs_egress_vector_bit_clear_test),
+	KUNIT_CASE(acs_egress_high_port_index_test),
+	KUNIT_CASE(acs_egress_port_out_of_range_test),
+	KUNIT_CASE(acs_egress_no_ec_cap_test),
+	KUNIT_CASE(acs_egress_pdev_not_downstream_test),
+	KUNIT_CASE(acs_egress_target_not_downstream_test),
+	KUNIT_CASE(acs_egress_pdev_pcie_bridge_test),
+	KUNIT_CASE(acs_egress_target_pcie_bridge_test),
+	KUNIT_CASE(acs_egress_target_other_bus_test),
+	KUNIT_CASE(acs_egress_root_port_test),
+	{}
+};
+
+static struct kunit_suite pci_acs_test_suite = {
+	.name = "pci_acs",
+	.test_cases = pci_acs_test_cases,
+};
+kunit_test_suite(pci_acs_test_suite);
+
+MODULE_IMPORT_NS("EXPORTED_FOR_KUNIT_TESTING");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("KUnit tests for PCI ACS peer-to-peer routing decisions");

-- 
2.55.0

[PATCH v4 16/18] PCI/P2PDMA: Add KUnit coverage for the ACS P2P routing walk

[Leon Romanovsky]

From: Leon Romanovsky <leonro@nvidia.com>

calc_map_type_and_dist() must apply Request controls to the client-side
divergence port and Completion controls to the provider-side port. Pure
helper tests cannot verify that topology assignment.

Build a nested fake PCIe switch fabric and exercise the full walk. Cover
direct routing, Request and Completion redirects, Egress Control outcomes,
and the asymmetric provider-RR/client-CR configuration. Also verify that
redirect bits below the divergence do not affect the direct path and an ACS
Violation without a redirect is rejected.

Tested-by: Tushar Dave <tdave@nvidia.com>
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 drivers/pci/p2pdma.c       |   3 +-
 drivers/pci/pci.h          |   4 +
 drivers/pci/pci_acs_test.c | 295 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 301 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
index 632f2d8c51e0..e54901d21d8a 100644
--- a/drivers/pci/p2pdma.c
+++ b/drivers/pci/p2pdma.c
@@ -784,7 +784,7 @@ static unsigned long map_types_idx(struct pci_dev *client)
  * ports per above. If the device is not in the whitelist, return
  * PCI_P2PDMA_MAP_NOT_SUPPORTED.
  */
-static enum pci_p2pdma_map_type
+VISIBLE_IF_KUNIT enum pci_p2pdma_map_type
 calc_map_type_and_dist(struct pci_dev *provider, struct pci_dev *client,
 		int *dist, bool verbose)
 {
@@ -916,6 +916,7 @@ calc_map_type_and_dist(struct pci_dev *provider, struct pci_dev *client,
 	rcu_read_unlock();
 	return map_type;
 }
+EXPORT_SYMBOL_IF_KUNIT(calc_map_type_and_dist);
 
 /**
  * pci_p2pdma_distance_many - Determine the cumulative distance between
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 4593fb43f96c..4d0f4478158f 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -6,6 +6,7 @@
 #include <linux/align.h>
 #include <linux/bitfield.h>
 #include <linux/pci.h>
+#include <linux/pci-p2pdma.h>
 #include <trace/events/pci.h>
 
 struct pcie_tlp_log;
@@ -1101,6 +1102,9 @@ bool pci_acs_egress_port_valid(u16 acs_caps, u8 target_port);
 enum pci_acs_p2pdma_state
 pci_acs_p2pdma_decision(u16 ctrl, enum pci_acs_p2pdma_tlp tlp,
 			bool has_target, int egress);
+enum pci_p2pdma_map_type calc_map_type_and_dist(struct pci_dev *provider,
+						struct pci_dev *client,
+						int *dist, bool verbose);
 #endif
 #ifdef CONFIG_PCI_QUIRKS
 int pci_dev_specific_acs_enabled(struct pci_dev *dev, u16 acs_flags,
diff --git a/drivers/pci/pci_acs_test.c b/drivers/pci/pci_acs_test.c
index aec7d7636f70..dd1054bb4f09 100644
--- a/drivers/pci/pci_acs_test.c
+++ b/drivers/pci/pci_acs_test.c
@@ -10,6 +10,7 @@
 #include <kunit/test.h>
 
 #include <linux/pci.h>
+#include <linux/pci-p2pdma.h>
 #include <linux/pci_regs.h>
 
 #include "pci.h"
@@ -416,6 +417,292 @@ static void acs_egress_root_port_test(struct kunit *test)
 			1);
 }
 
+/*
+ * calc_map_type_and_dist(): drive the full provider->client hierarchy walk
+ * over a fabricated PCIe fabric matching the canonical "two devices behind one
+ * switch" tree:
+ *
+ *   host bridge / root bus
+ *     Root Port
+ *       Switch Upstream Port
+ *         Switch Downstream Port 0
+ *           Nested Switch -- provider
+ *         Switch Downstream Port 1
+ *           Nested Switch -- client
+ *
+ * A fake pci_ops answers the ACS Control, Egress Control Vector and LNKCAP
+ * reads for the downstream ports, so the ACS Egress Control evaluated at the
+ * path divergence (Downstream Port 1 targeting Downstream Port 0) decides the
+ * mapping without any real hardware.
+ */
+
+struct acs_dn_cfg {
+	u16	acs_ctrl;		/* ACS Control register value */
+	u8	port;			/* this port's LNKCAP Port Number */
+	u32	egress[8];		/* Egress Control Vector (256 bits) */
+};
+
+struct acs_fabric {
+	struct pci_dev		*provider;
+	struct pci_dev		*client;
+	struct pci_dev		*dn0;	/* Downstream Port 0 (provider side) */
+	struct pci_dev		*dn1;	/* Downstream Port 1 (client side) */
+	struct pci_dev		*provider_leaf;
+	struct pci_dev		*client_leaf;
+	struct acs_dn_cfg	dn0_cfg;
+	struct acs_dn_cfg	dn1_cfg;
+	struct acs_dn_cfg	provider_leaf_cfg;
+	struct acs_dn_cfg	client_leaf_cfg;
+};
+
+static void acs_dn_read(struct pci_dev *dn, struct acs_dn_cfg *c,
+			int where, int size, u32 *val)
+{
+	int vec = dn->acs_cap + PCI_ACS_EGRESS_CTL_V;
+
+	if (size == 4 && where == dn->pcie_cap + PCI_EXP_LNKCAP)
+		*val = FIELD_PREP(PCI_EXP_LNKCAP_PN, c->port);
+	else if (dn->acs_cap && size == 2 && where == dn->acs_cap + PCI_ACS_CTRL)
+		*val = c->acs_ctrl;
+	else if (dn->acs_cap && size == 4 &&
+		 where >= vec && where < vec + (int)sizeof(c->egress))
+		*val = c->egress[(where - vec) / 4];
+}
+
+static int acs_fabric_read(struct pci_bus *bus, unsigned int devfn,
+			   int where, int size, u32 *val)
+{
+	struct acs_fabric *f = bus->sysdata;
+
+	*val = 0;
+	if (bus == f->dn0->bus && devfn == f->dn0->devfn)
+		acs_dn_read(f->dn0, &f->dn0_cfg, where, size, val);
+	else if (bus == f->dn1->bus && devfn == f->dn1->devfn)
+		acs_dn_read(f->dn1, &f->dn1_cfg, where, size, val);
+	else if (bus == f->provider_leaf->bus &&
+		 devfn == f->provider_leaf->devfn)
+		acs_dn_read(f->provider_leaf, &f->provider_leaf_cfg, where,
+			    size, val);
+	else if (bus == f->client_leaf->bus &&
+		 devfn == f->client_leaf->devfn)
+		acs_dn_read(f->client_leaf, &f->client_leaf_cfg, where, size,
+			    val);
+	return PCIBIOS_SUCCESSFUL;
+}
+
+static int acs_fabric_write(struct pci_bus *bus, unsigned int devfn,
+			    int where, int size, u32 val)
+{
+	return PCIBIOS_SUCCESSFUL;
+}
+
+static struct pci_ops acs_fabric_ops = {
+	.read	= acs_fabric_read,
+	.write	= acs_fabric_write,
+};
+
+static struct pci_bus *acs_add_bus(struct kunit *test, struct pci_bus *parent,
+				   struct pci_dev *self, u8 nr, void *sysdata)
+{
+	struct pci_bus *bus = kunit_kzalloc(test, sizeof(*bus), GFP_KERNEL);
+
+	KUNIT_ASSERT_NOT_NULL(test, bus);
+	bus->parent = parent;
+	bus->self = self;
+	bus->number = nr;
+	bus->ops = &acs_fabric_ops;
+	bus->sysdata = sysdata;
+	INIT_LIST_HEAD(&bus->devices);
+	return bus;
+}
+
+static struct pci_dev *acs_add_dev(struct kunit *test, struct pci_bus *bus,
+				   unsigned int devfn, int pcie_type)
+{
+	struct pci_dev *dev = kunit_kzalloc(test, sizeof(*dev), GFP_KERNEL);
+
+	KUNIT_ASSERT_NOT_NULL(test, dev);
+	dev->bus = bus;
+	dev->devfn = devfn;
+	dev->pcie_cap = 0x40;
+	dev->pcie_flags_reg = ACS_TEST_PCIE_FLAGS(pcie_type);
+	list_add_tail(&dev->bus_list, &bus->devices);
+	return dev;
+}
+
+static void acs_build_fabric(struct kunit *test, struct acs_fabric *f)
+{
+	struct pci_bus *bus0, *bus1, *bus2, *bus3, *bus4, *bus5, *bus6;
+	struct pci_bus *bus7, *bus8;
+	struct pci_dev *rootport, *swup, *provider_swup, *client_swup;
+	struct pci_host_bridge *host;
+
+	host = kunit_kzalloc(test, sizeof(*host), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, host);
+
+	bus0 = acs_add_bus(test, NULL, NULL, 0, f);		/* root bus */
+	/* The Root Port doubles as the whitelisted host-bridge device. */
+	rootport = acs_add_dev(test, bus0, PCI_DEVFN(0, 0),
+			       PCI_EXP_TYPE_ROOT_PORT);
+	rootport->vendor = PCI_VENDOR_ID_GOOGLE;
+	rootport->device = 0x1234;
+	host->bus = bus0;
+	bus0->bridge = &host->dev;
+
+	bus1 = acs_add_bus(test, bus0, rootport, 1, f);
+	swup = acs_add_dev(test, bus1, PCI_DEVFN(0, 0), PCI_EXP_TYPE_UPSTREAM);
+
+	bus2 = acs_add_bus(test, bus1, swup, 2, f);
+	f->dn0 = acs_add_dev(test, bus2, PCI_DEVFN(0, 0), PCI_EXP_TYPE_DOWNSTREAM);
+	f->dn1 = acs_add_dev(test, bus2, PCI_DEVFN(1, 0), PCI_EXP_TYPE_DOWNSTREAM);
+
+	bus3 = acs_add_bus(test, bus2, f->dn0, 3, f);
+	provider_swup = acs_add_dev(test, bus3, PCI_DEVFN(0, 0),
+				    PCI_EXP_TYPE_UPSTREAM);
+	bus5 = acs_add_bus(test, bus3, provider_swup, 5, f);
+	f->provider_leaf = acs_add_dev(test, bus5, PCI_DEVFN(0, 0),
+				       PCI_EXP_TYPE_DOWNSTREAM);
+	bus7 = acs_add_bus(test, bus5, f->provider_leaf, 7, f);
+	f->provider = acs_add_dev(test, bus7, PCI_DEVFN(0, 0),
+				  PCI_EXP_TYPE_ENDPOINT);
+
+	bus4 = acs_add_bus(test, bus2, f->dn1, 4, f);
+	client_swup = acs_add_dev(test, bus4, PCI_DEVFN(0, 0),
+				  PCI_EXP_TYPE_UPSTREAM);
+	bus6 = acs_add_bus(test, bus4, client_swup, 6, f);
+	f->client_leaf = acs_add_dev(test, bus6, PCI_DEVFN(0, 0),
+				     PCI_EXP_TYPE_DOWNSTREAM);
+	bus8 = acs_add_bus(test, bus6, f->client_leaf, 8, f);
+	f->client = acs_add_dev(test, bus8, PCI_DEVFN(0, 0),
+				PCI_EXP_TYPE_ENDPOINT);
+}
+
+static enum pci_p2pdma_map_type acs_walk_map(struct acs_fabric *f)
+{
+	int dist;
+
+	return calc_map_type_and_dist(f->provider, f->client, &dist, false);
+}
+
+static void acs_walk_bus_addr_test(struct kunit *test)
+{
+	struct acs_fabric f = {};
+
+	acs_build_fabric(test, &f);
+	/* No ACS on the path: peer-to-peer is allowed directly. */
+	KUNIT_EXPECT_EQ(test, acs_walk_map(&f), PCI_P2PDMA_MAP_BUS_ADDR);
+}
+
+static void acs_walk_ec_violation_test(struct kunit *test)
+{
+	struct acs_fabric f = {};
+
+	acs_build_fabric(test, &f);
+	/*
+	 * Client Downstream Port 1 has Egress Control enabled with the vector bit
+	 * for provider Downstream Port 0 set and Request Redirect clear: an ACS
+	 * Violation. The direct path is unusable, and no redirect establishes an
+	 * upstream route.
+	 */
+	f.dn1->acs_cap = 0x100;
+	f.dn1->acs_capabilities = PCI_ACS_EC | (64 << 8);
+	f.dn1_cfg.acs_ctrl = PCI_ACS_EC;
+	f.dn0_cfg.port = 5;
+	f.dn1_cfg.egress[0] = BIT(5);
+
+	KUNIT_EXPECT_EQ(test, acs_walk_map(&f),
+			PCI_P2PDMA_MAP_NOT_SUPPORTED);
+}
+
+static void acs_walk_ec_vector_clear_test(struct kunit *test)
+{
+	struct acs_fabric f = {};
+
+	acs_build_fabric(test, &f);
+	/* Egress Control enabled but the provider vector bit is clear. */
+	f.dn1->acs_cap = 0x100;
+	f.dn1->acs_capabilities = PCI_ACS_EC | (64 << 8);
+	f.dn1_cfg.acs_ctrl = PCI_ACS_EC;
+	f.dn0_cfg.port = 5;		/* egress vector left all-zero */
+
+	KUNIT_EXPECT_EQ(test, acs_walk_map(&f), PCI_P2PDMA_MAP_BUS_ADDR);
+}
+
+static void acs_walk_request_redirect_test(struct kunit *test)
+{
+	struct acs_fabric f = {};
+
+	acs_build_fabric(test, &f);
+	/* Client Request Redirect sends traffic through the host bridge. */
+	f.dn1->acs_cap = 0x100;
+	f.dn1->acs_capabilities = PCI_ACS_RR;
+	f.dn1_cfg.acs_ctrl = PCI_ACS_RR;
+
+	/* The Google root port is whitelisted, so the host-bridge path is OK. */
+	KUNIT_EXPECT_EQ(test, acs_walk_map(&f),
+			PCI_P2PDMA_MAP_THRU_HOST_BRIDGE);
+}
+
+static void acs_walk_completion_redirect_test(struct kunit *test)
+{
+	struct acs_fabric f = {};
+
+	acs_build_fabric(test, &f);
+	/* Provider Completion Redirect sends traffic through the host bridge. */
+	f.dn0->acs_cap = 0x100;
+	f.dn0->acs_capabilities = PCI_ACS_CR;
+	f.dn0_cfg.acs_ctrl = PCI_ACS_CR;
+
+	/* The Google root port is whitelisted, so the host-bridge path is OK. */
+	KUNIT_EXPECT_EQ(test, acs_walk_map(&f),
+			PCI_P2PDMA_MAP_THRU_HOST_BRIDGE);
+}
+
+static void acs_walk_asymmetric_direct_test(struct kunit *test)
+{
+	struct acs_fabric f = {};
+
+	acs_build_fabric(test, &f);
+	/* Provider RR/EC and client CR act on the reverse transaction paths. */
+	f.dn0->acs_cap = 0x100;
+	f.dn0->acs_capabilities = PCI_ACS_RR | PCI_ACS_EC | (64 << 8);
+	f.dn0_cfg.acs_ctrl = PCI_ACS_RR | PCI_ACS_EC;
+	f.dn1_cfg.port = 6;
+	f.dn0_cfg.egress[0] = BIT(6);
+	f.dn1->acs_cap = 0x100;
+	f.dn1->acs_capabilities = PCI_ACS_CR | PCI_ACS_EC | (64 << 8);
+	f.dn1_cfg.acs_ctrl = PCI_ACS_CR | PCI_ACS_EC;
+	f.dn0_cfg.port = 5;		/* client vector left all-zero */
+
+	KUNIT_EXPECT_EQ(test, acs_walk_map(&f), PCI_P2PDMA_MAP_BUS_ADDR);
+}
+
+static void acs_walk_nested_completion_redirect_test(struct kunit *test)
+{
+	struct acs_fabric f = {};
+
+	acs_build_fabric(test, &f);
+	/* The completion already routes upstream at this nested port. */
+	f.provider_leaf->acs_cap = 0x100;
+	f.provider_leaf->acs_capabilities = PCI_ACS_CR;
+	f.provider_leaf_cfg.acs_ctrl = PCI_ACS_CR;
+
+	KUNIT_EXPECT_EQ(test, acs_walk_map(&f), PCI_P2PDMA_MAP_BUS_ADDR);
+}
+
+static void acs_walk_nested_request_redirect_test(struct kunit *test)
+{
+	struct acs_fabric f = {};
+
+	acs_build_fabric(test, &f);
+	/* The request already routes upstream at this nested port. */
+	f.client_leaf->acs_cap = 0x100;
+	f.client_leaf->acs_capabilities = PCI_ACS_RR;
+	f.client_leaf_cfg.acs_ctrl = PCI_ACS_RR;
+
+	KUNIT_EXPECT_EQ(test, acs_walk_map(&f), PCI_P2PDMA_MAP_BUS_ADDR);
+}
+
 static struct kunit_case pci_acs_test_cases[] = {
 	KUNIT_CASE_PARAM(pci_acs_p2pdma_decision_test, acs_decision_gen_params),
 	KUNIT_CASE_PARAM(pci_acs_egress_port_valid_test, egress_valid_gen_params),
@@ -430,6 +717,14 @@ static struct kunit_case pci_acs_test_cases[] = {
 	KUNIT_CASE(acs_egress_target_pcie_bridge_test),
 	KUNIT_CASE(acs_egress_target_other_bus_test),
 	KUNIT_CASE(acs_egress_root_port_test),
+	KUNIT_CASE(acs_walk_bus_addr_test),
+	KUNIT_CASE(acs_walk_ec_violation_test),
+	KUNIT_CASE(acs_walk_ec_vector_clear_test),
+	KUNIT_CASE(acs_walk_request_redirect_test),
+	KUNIT_CASE(acs_walk_completion_redirect_test),
+	KUNIT_CASE(acs_walk_asymmetric_direct_test),
+	KUNIT_CASE(acs_walk_nested_completion_redirect_test),
+	KUNIT_CASE(acs_walk_nested_request_redirect_test),
 	{}
 };
 

-- 
2.55.0

[PATCH v4 17/18] PCI: Add KUnit coverage for ACS isolation checks

[Leon Romanovsky]

From: Leon Romanovsky <leonro@nvidia.com>

Direct Translated P2P and Egress Control both let a peer request reach
the peer without Request Redirect, and whether Request Redirect still
isolates depends further on Translation Blocking and on the flags the
caller requests. Firmware owns these bits, so the combinations are not
reachable on a given machine.

Drive pci_acs_flags_enabled() with a fake pci_ops supplying the ACS
Control register and check each combination, in both scopes: an
Untranslated-only caller such as pci_enable_pasid() is unaffected by
Direct Translated P2P but still loses Request Redirect to Egress
Control. Cover the two ways the check gives up as well: a device with no
ACS capability, and a control register that cannot be read.

The function is exposed under CONFIG_KUNIT via VISIBLE_IF_KUNIT.

Tested-by: Tushar Dave <tdave@nvidia.com>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
Assisted-by: Claude:claude-opus-5
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 drivers/pci/pci.c          |   6 +-
 drivers/pci/pci.h          |   2 +
 drivers/pci/pci_acs_test.c | 181 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 187 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index d900fdb6f37d..7ee1fca60a12 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3631,8 +3631,9 @@ int pci_acs_egress_ctrl_is_set(struct pci_dev *pdev, struct pci_dev *target)
 }
 EXPORT_SYMBOL_IF_KUNIT(pci_acs_egress_ctrl_is_set);
 
-static bool pci_acs_flags_enabled(struct pci_dev *pdev, u16 acs_flags,
-				  enum pci_acs_scope scope)
+VISIBLE_IF_KUNIT
+bool pci_acs_flags_enabled(struct pci_dev *pdev, u16 acs_flags,
+			   enum pci_acs_scope scope)
 {
 	int pos;
 	u16 ctrl;
@@ -3656,6 +3657,7 @@ static bool pci_acs_flags_enabled(struct pci_dev *pdev, u16 acs_flags,
 
 	return (ctrl & acs_flags) == acs_flags;
 }
+EXPORT_SYMBOL_IF_KUNIT(pci_acs_flags_enabled);
 
 /**
  * pci_acs_enabled - test ACS against required flags for a given device
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 4d0f4478158f..9921bc3cabb9 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -1098,6 +1098,8 @@ enum pci_acs_p2pdma_tlp {
 };
 
 #if IS_ENABLED(CONFIG_KUNIT)
+bool pci_acs_flags_enabled(struct pci_dev *pdev, u16 acs_flags,
+			   enum pci_acs_scope scope);
 bool pci_acs_egress_port_valid(u16 acs_caps, u8 target_port);
 enum pci_acs_p2pdma_state
 pci_acs_p2pdma_decision(u16 ctrl, enum pci_acs_p2pdma_tlp tlp,
diff --git a/drivers/pci/pci_acs_test.c b/drivers/pci/pci_acs_test.c
index dd1054bb4f09..806bcb6ae778 100644
--- a/drivers/pci/pci_acs_test.c
+++ b/drivers/pci/pci_acs_test.c
@@ -148,6 +148,184 @@ static void pci_acs_egress_port_valid_test(struct kunit *test)
 			c->expect);
 }
 
+/*
+ * pci_acs_flags_enabled(): Direct Translated P2P and Egress Control both let a
+ * peer request reach the peer without Request Redirect, so neither may report
+ * isolation.  Translation Blocking rejects a Translated Request before it is
+ * routed, which restores the Request Redirect guarantee.  A fake pci_ops
+ * supplies the ACS Control register.
+ */
+
+/* Flags an IOMMU asks for; see REQ_ACS_FLAGS in drivers/iommu/iommu.c. */
+#define ACS_REQ_FLAGS	(PCI_ACS_SV | PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_UF)
+#define ACS_ALL_CAPS	(PCI_ACS_SV | PCI_ACS_TB | PCI_ACS_RR | PCI_ACS_CR | \
+			 PCI_ACS_UF | PCI_ACS_EC | PCI_ACS_DT)
+#define ACS_TEST_CAP	0x100
+/* Flags pci_enable_pasid() asks for; see drivers/pci/ats.c. */
+#define ACS_PASID_FLAGS	(PCI_ACS_RR | PCI_ACS_UF)
+
+struct acs_ctrl_cfg {
+	unsigned int	devfn;
+	u16		cap;		/* offset the ACS capability answers at */
+	u16		ctrl;
+	bool		fail_read;
+};
+
+static int acs_ctrl_read(struct pci_bus *bus, unsigned int devfn,
+			 int where, int size, u32 *val)
+{
+	struct acs_ctrl_cfg *cfg = bus->sysdata;
+
+	*val = 0;
+	if (cfg->fail_read)
+		return PCIBIOS_DEVICE_NOT_FOUND;
+
+	if (devfn == cfg->devfn && size == 2 &&
+	    where == cfg->cap + PCI_ACS_CTRL)
+		*val = cfg->ctrl;
+	return PCIBIOS_SUCCESSFUL;
+}
+
+static int acs_ctrl_write(struct pci_bus *bus, unsigned int devfn,
+			  int where, int size, u32 val)
+{
+	return PCIBIOS_SUCCESSFUL;
+}
+
+static struct pci_ops acs_ctrl_ops = {
+	.read	= acs_ctrl_read,
+	.write	= acs_ctrl_write,
+};
+
+struct acs_isolation_case {
+	const char *desc;
+	u16	ctrl;			/* ACS Control register */
+	u16	req;			/* flags the caller asks for */
+	enum pci_acs_scope scope;	/* Requests the answer must cover */
+	bool	expect;			/* isolation reported? */
+};
+
+static const struct acs_isolation_case acs_isolation_cases[] = {
+	{ "plain_rr", ACS_REQ_FLAGS, ACS_REQ_FLAGS, PCI_ACS_SCOPE_ALL, true },
+	/* Direct Translated P2P bypasses Request Redirect ... */
+	{ "dt", ACS_REQ_FLAGS | PCI_ACS_DT, ACS_REQ_FLAGS,
+	  PCI_ACS_SCOPE_ALL, false },
+	/* ... unless Translation Blocking rejects the Translated Request. */
+	{ "dt_tb", ACS_REQ_FLAGS | PCI_ACS_DT | PCI_ACS_TB, ACS_REQ_FLAGS,
+	  PCI_ACS_SCOPE_ALL, true },
+	{ "tb_only", ACS_REQ_FLAGS | PCI_ACS_TB, ACS_REQ_FLAGS,
+	  PCI_ACS_SCOPE_ALL, true },
+	/* Egress Control can override Request Redirect as well. */
+	{ "ec", ACS_REQ_FLAGS | PCI_ACS_EC, ACS_REQ_FLAGS,
+	  PCI_ACS_SCOPE_ALL, false },
+	{ "ec_dt_tb", ACS_REQ_FLAGS | PCI_ACS_EC | PCI_ACS_DT | PCI_ACS_TB,
+	  ACS_REQ_FLAGS, PCI_ACS_SCOPE_ALL, false },
+	/* Without Request Redirect requested, neither bit is consulted. */
+	{ "no_rr_dt", PCI_ACS_SV | PCI_ACS_CR | PCI_ACS_UF | PCI_ACS_DT,
+	  PCI_ACS_SV | PCI_ACS_CR | PCI_ACS_UF, PCI_ACS_SCOPE_ALL, true },
+	/* A control bit the caller asked for is simply missing. */
+	{ "rr_not_enabled", PCI_ACS_SV | PCI_ACS_CR | PCI_ACS_UF, ACS_REQ_FLAGS,
+	  PCI_ACS_SCOPE_ALL, false },
+
+	/*
+	 * An Untranslated-only caller such as pci_enable_pasid() is not
+	 * affected by Direct Translated P2P, but is still affected by Egress
+	 * Control, which acts on Untranslated peer Requests too.
+	 */
+	{ "untrans/plain_rr", ACS_PASID_FLAGS, ACS_PASID_FLAGS,
+	  PCI_ACS_SCOPE_UNTRANSLATED, true },
+	{ "untrans/dt", ACS_PASID_FLAGS | PCI_ACS_DT, ACS_PASID_FLAGS,
+	  PCI_ACS_SCOPE_UNTRANSLATED, true },
+	{ "untrans/dt_tb", ACS_PASID_FLAGS | PCI_ACS_DT | PCI_ACS_TB,
+	  ACS_PASID_FLAGS, PCI_ACS_SCOPE_UNTRANSLATED, true },
+	{ "untrans/ec", ACS_PASID_FLAGS | PCI_ACS_EC, ACS_PASID_FLAGS,
+	  PCI_ACS_SCOPE_UNTRANSLATED, false },
+	{ "untrans/rr_not_enabled", PCI_ACS_UF, ACS_PASID_FLAGS,
+	  PCI_ACS_SCOPE_UNTRANSLATED, false },
+};
+
+static void acs_isolation_desc(const struct acs_isolation_case *c, char *desc)
+{
+	strscpy(desc, c->desc, KUNIT_PARAM_DESC_SIZE);
+}
+
+KUNIT_ARRAY_PARAM(acs_isolation, acs_isolation_cases, acs_isolation_desc);
+
+static void pci_acs_flags_enabled_test(struct kunit *test)
+{
+	const struct acs_isolation_case *c = test->param_value;
+	struct acs_ctrl_cfg cfg = { .devfn = PCI_DEVFN(0, 0),
+				    .cap = ACS_TEST_CAP, .ctrl = c->ctrl };
+	struct pci_bus *bus = kunit_kzalloc(test, sizeof(*bus), GFP_KERNEL);
+	struct pci_dev *pdev = kunit_kzalloc(test, sizeof(*pdev), GFP_KERNEL);
+
+	KUNIT_ASSERT_NOT_NULL(test, bus);
+	KUNIT_ASSERT_NOT_NULL(test, pdev);
+
+	bus->ops = &acs_ctrl_ops;
+	bus->sysdata = &cfg;
+
+	pdev->bus = bus;
+	pdev->devfn = cfg.devfn;
+	pdev->acs_cap = ACS_TEST_CAP;
+	pdev->acs_capabilities = ACS_ALL_CAPS;
+
+	KUNIT_EXPECT_EQ(test, pci_acs_flags_enabled(pdev, c->req, c->scope),
+			c->expect);
+}
+
+static bool acs_isolated(struct kunit *test, struct acs_ctrl_cfg *cfg,
+			 u16 acs_cap, u16 acs_flags)
+{
+	struct pci_bus *bus = kunit_kzalloc(test, sizeof(*bus), GFP_KERNEL);
+	struct pci_dev *pdev = kunit_kzalloc(test, sizeof(*pdev), GFP_KERNEL);
+
+	KUNIT_ASSERT_NOT_NULL(test, bus);
+	KUNIT_ASSERT_NOT_NULL(test, pdev);
+
+	bus->ops = &acs_ctrl_ops;
+	bus->sysdata = cfg;
+
+	pdev->bus = bus;
+	pdev->devfn = cfg->devfn;
+	pdev->acs_cap = acs_cap;
+	pdev->acs_capabilities = ACS_ALL_CAPS;
+
+	return pci_acs_flags_enabled(pdev, acs_flags, PCI_ACS_SCOPE_ALL);
+}
+
+/*
+ * Without an ACS capability there is no control register to consult.  The
+ * fake answers at offset 0 here, so dropping the acs_cap guard would read an
+ * isolating control word rather than nothing.
+ */
+static void pci_acs_flags_no_cap_test(struct kunit *test)
+{
+	struct acs_ctrl_cfg cfg = { .devfn = PCI_DEVFN(0, 0), .cap = 0,
+				    .ctrl = ACS_REQ_FLAGS };
+
+	KUNIT_EXPECT_FALSE(test, acs_isolated(test, &cfg, 0, ACS_REQ_FLAGS));
+}
+
+/*
+ * An unreadable ACS Control register reads back as all ones, which satisfies
+ * any requested control.  Ask without Request Redirect, so that neither
+ * pci_acs_rr_ineffective() nor the control word itself can deny isolation and
+ * the read failure is the only thing left that can.
+ */
+static void pci_acs_flags_read_fails_test(struct kunit *test)
+{
+	u16 no_rr = ACS_REQ_FLAGS & ~PCI_ACS_RR;
+	struct acs_ctrl_cfg cfg = { .devfn = PCI_DEVFN(0, 0),
+				    .cap = ACS_TEST_CAP,
+				    .ctrl = ACS_REQ_FLAGS };
+
+	KUNIT_EXPECT_TRUE(test, acs_isolated(test, &cfg, ACS_TEST_CAP, no_rr));
+
+	cfg.fail_read = true;
+	KUNIT_EXPECT_FALSE(test, acs_isolated(test, &cfg, ACS_TEST_CAP, no_rr));
+}
+
 /*
  * pci_acs_egress_ctrl_is_set(): drive the config-space reads with a fake pci_ops
  * so the Egress Control Vector lookup is exercised without real hardware --
@@ -706,6 +884,9 @@ static void acs_walk_nested_request_redirect_test(struct kunit *test)
 static struct kunit_case pci_acs_test_cases[] = {
 	KUNIT_CASE_PARAM(pci_acs_p2pdma_decision_test, acs_decision_gen_params),
 	KUNIT_CASE_PARAM(pci_acs_egress_port_valid_test, egress_valid_gen_params),
+	KUNIT_CASE_PARAM(pci_acs_flags_enabled_test, acs_isolation_gen_params),
+	KUNIT_CASE(pci_acs_flags_no_cap_test),
+	KUNIT_CASE(pci_acs_flags_read_fails_test),
 	KUNIT_CASE(acs_egress_vector_bit_set_test),
 	KUNIT_CASE(acs_egress_vector_bit_clear_test),
 	KUNIT_CASE(acs_egress_high_port_index_test),

-- 
2.55.0

[PATCH v4 18/18] PCI/P2PDMA: Log detailed ACS routing diagnostics

[Leon Romanovsky]

From: Leon Romanovsky <leonro@nvidia.com>

When a platform rejects a P2PDMA mapping, existing warnings identify only
the final ACS blocker and host bridge result. They do not expose the
complete topology, live ACS settings, divergence ports, cache state, or
intermediate decisions needed to distinguish routing and policy failures.

Emit info-level diagnostics for verbose calculations and cache lookups.
Report both paths, bridge bus ranges, raw and decoded ACS controls, Egress
Control vectors, selected Request and Completion states, host fallback, and
the final mapping. Keep non-verbose distance and KUnit paths quiet.

Tested-by: Tushar Dave <tdave@nvidia.com>
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 drivers/pci/p2pdma.c | 273 ++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 259 insertions(+), 14 deletions(-)

diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
index e54901d21d8a..b79eb4ce09f4 100644
--- a/drivers/pci/p2pdma.c
+++ b/drivers/pci/p2pdma.c
@@ -572,26 +572,200 @@ pci_acs_p2pdma_decision(u16 ctrl, enum pci_acs_p2pdma_tlp tlp,
 }
 EXPORT_SYMBOL_IF_KUNIT(pci_acs_p2pdma_decision);
 
+static const char *pci_acs_p2pdma_state_name(enum pci_acs_p2pdma_state state)
+{
+	switch (state) {
+	case PCI_ACS_P2PDMA_DIRECT:
+		return "direct";
+	case PCI_ACS_P2PDMA_REDIRECT:
+		return "redirect";
+	case PCI_ACS_P2PDMA_NOT_SUPPORTED:
+		return "not-supported";
+	}
+
+	return "invalid";
+}
+
+static const char *pci_p2pdma_map_type_name(enum pci_p2pdma_map_type type)
+{
+	switch (type) {
+	case PCI_P2PDMA_MAP_UNKNOWN:
+		return "unknown";
+	case PCI_P2PDMA_MAP_NONE:
+		return "none";
+	case PCI_P2PDMA_MAP_NOT_SUPPORTED:
+		return "not-supported";
+	case PCI_P2PDMA_MAP_BUS_ADDR:
+		return "bus-address";
+	case PCI_P2PDMA_MAP_THRU_HOST_BRIDGE:
+		return "through-host-bridge";
+	}
+
+	return "invalid";
+}
+
+static const char *pci_acs_p2pdma_tlp_name(enum pci_acs_p2pdma_tlp tlp)
+{
+	return tlp == PCI_ACS_P2PDMA_TLP_REQUEST ? "request" : "completion";
+}
+
+static int pci_p2pdma_port_number(struct pci_dev *pdev)
+{
+	u32 lnkcap;
+	int ret;
+
+	if (!pci_is_pcie(pdev))
+		return -EOPNOTSUPP;
+
+	ret = pcie_capability_read_dword(pdev, PCI_EXP_LNKCAP, &lnkcap);
+	if (ret)
+		return pcibios_err_to_errno(ret);
+
+	return FIELD_GET(PCI_EXP_LNKCAP_PN, lnkcap);
+}
+
+static void pci_p2pdma_log_egress_vector(struct pci_dev *pdev,
+					 int target_port)
+{
+	unsigned int offset;
+	u32 vector, mask;
+	int ret;
+
+	if (target_port < 0 || target_port > U8_MAX)
+		return;
+
+	offset = pdev->acs_cap + PCI_ACS_EGRESS_CTL_V +
+		 (target_port / 32) * sizeof(vector);
+	mask = BIT(target_port % 32);
+	ret = pci_read_config_dword(pdev, offset, &vector);
+	if (ret) {
+		pci_info(pdev,
+			 "P2PDMA ACS debug: Egress Control Vector read at %#x failed: %#x\n",
+			 offset, ret);
+		return;
+	}
+
+	pci_info(pdev,
+		 "P2PDMA ACS debug: Egress Control Vector offset=%#x value=%#010x bit=%d mask=%#010x set=%u\n",
+		 offset, vector, target_port, mask, !!(vector & mask));
+}
+
 static enum pci_acs_p2pdma_state
 pci_acs_p2pdma_state(struct pci_dev *pdev, struct pci_dev *target,
-		     enum pci_acs_p2pdma_tlp tlp)
+		     enum pci_acs_p2pdma_tlp tlp, bool verbose)
 {
-	int pos, egress = 0;
+	enum pci_acs_p2pdma_state state;
+	bool egress_checked = false;
+	int pos, ret, egress = 0;
+	int target_port = -1;
 	u16 ctrl;
 
 	pos = pdev->acs_cap;
-	if (!pos)
+	if (!pos) {
+		if (verbose)
+			pci_info(pdev,
+				 "P2PDMA ACS debug: %s target=%s has no ACS capability; result=direct\n",
+				 pci_acs_p2pdma_tlp_name(tlp),
+				 target ? pci_name(target) : "<none>");
 		return PCI_ACS_P2PDMA_DIRECT;
+	}
 
-	if (pci_read_config_word(pdev, pos + PCI_ACS_CTRL, &ctrl))
+	ret = pci_read_config_word(pdev, pos + PCI_ACS_CTRL, &ctrl);
+	if (ret) {
+		if (verbose)
+			pci_info(pdev,
+				 "P2PDMA ACS debug: %s target=%s ACS Control read failed at %#x: %#x; result=not-supported\n",
+				 pci_acs_p2pdma_tlp_name(tlp),
+				 target ? pci_name(target) : "<none>",
+				 pos + PCI_ACS_CTRL, ret);
 		return PCI_ACS_P2PDMA_NOT_SUPPORTED;
+	}
 
 	/* Egress Control applies only to Requests with a known target. */
 	if (tlp == PCI_ACS_P2PDMA_TLP_REQUEST && target &&
-	    (ctrl & PCI_ACS_EC))
+	    (ctrl & PCI_ACS_EC)) {
+		egress_checked = true;
 		egress = pci_acs_egress_ctrl_is_set(pdev, target);
+	}
+
+	state = pci_acs_p2pdma_decision(ctrl, tlp, !!target, egress);
+	if (!verbose)
+		return state;
+
+	if (target)
+		target_port = pci_p2pdma_port_number(target);
+
+	pci_info(pdev,
+		 "P2PDMA ACS debug: %s target=%s target-port=%d same-bus=%u cap=%#x caps=%#06x ctrl=%#06x result=%s\n",
+		 pci_acs_p2pdma_tlp_name(tlp),
+		 target ? pci_name(target) : "<none>", target_port,
+		 target && pdev->bus == target->bus, pos,
+		 pdev->acs_capabilities, ctrl, pci_acs_p2pdma_state_name(state));
+	pci_info(pdev,
+		 "P2PDMA ACS debug: control bits SV=%u TB=%u RR=%u CR=%u UF=%u EC=%u DT=%u\n",
+		 !!(ctrl & PCI_ACS_SV), !!(ctrl & PCI_ACS_TB),
+		 !!(ctrl & PCI_ACS_RR), !!(ctrl & PCI_ACS_CR),
+		 !!(ctrl & PCI_ACS_UF), !!(ctrl & PCI_ACS_EC),
+		 !!(ctrl & PCI_ACS_DT));
+	if (egress_checked)
+		pci_info(pdev,
+			 "P2PDMA ACS debug: Egress Control target=%s port=%d vector-size=%u lookup=%d\n",
+			 pci_name(target), target_port,
+			 pdev->acs_capabilities >> 8, egress);
+	else
+		pci_info(pdev,
+			 "P2PDMA ACS debug: Egress Control vector not consulted for this TLP\n");
+	if (egress_checked && egress >= 0)
+		pci_p2pdma_log_egress_vector(pdev, target_port);
+
+	return state;
+}
+
+static void pci_p2pdma_log_path(const char *name, struct pci_dev *start,
+				struct pci_dev *common)
+{
+	struct pci_dev *pdev, *upstream;
+	int hop = 0, ret, type;
+	u16 ctrl;
+
+	for (pdev = start; pdev; pdev = upstream, hop++) {
+		upstream = pci_upstream_bridge(pdev);
+		type = pci_is_pcie(pdev) ? pci_pcie_type(pdev) : -1;
+		pci_info(pdev,
+			 "P2PDMA ACS debug: %s path hop=%d common=%u pcie=%u type=%d class=%#08x vendor=%04x device=%04x upstream=%s\n",
+			 name, hop, pdev == common, pci_is_pcie(pdev), type,
+			 pdev->class, pdev->vendor, pdev->device,
+			 upstream ? pci_name(upstream) : "<none>");
+
+		if (pdev->subordinate)
+			pci_info(pdev,
+				 "P2PDMA ACS debug: bridge bus range=%02llx-%02llx\n",
+				 (unsigned long long)pdev->subordinate->busn_res.start,
+				 (unsigned long long)pdev->subordinate->busn_res.end);
+
+		if (!pdev->acs_cap) {
+			pci_info(pdev,
+				 "P2PDMA ACS debug: ACS capability absent\n");
+			continue;
+		}
+
+		ret = pci_read_config_word(pdev, pdev->acs_cap + PCI_ACS_CTRL,
+					   &ctrl);
+		if (ret) {
+			pci_info(pdev,
+				 "P2PDMA ACS debug: ACS cap=%#x caps=%#06x Control read failed: %#x\n",
+				 pdev->acs_cap, pdev->acs_capabilities, ret);
+			continue;
+		}
 
-	return pci_acs_p2pdma_decision(ctrl, tlp, !!target, egress);
+		pci_info(pdev,
+			 "P2PDMA ACS debug: ACS cap=%#x caps=%#06x ctrl=%#06x SV=%u TB=%u RR=%u CR=%u UF=%u EC=%u DT=%u\n",
+			 pdev->acs_cap, pdev->acs_capabilities, ctrl,
+			 !!(ctrl & PCI_ACS_SV), !!(ctrl & PCI_ACS_TB),
+			 !!(ctrl & PCI_ACS_RR), !!(ctrl & PCI_ACS_CR),
+			 !!(ctrl & PCI_ACS_UF), !!(ctrl & PCI_ACS_EC),
+			 !!(ctrl & PCI_ACS_DT));
+	}
 }
 
 static void seq_buf_print_bus_devfn(struct seq_buf *buf, struct pci_dev *pdev)
@@ -792,15 +966,23 @@ calc_map_type_and_dist(struct pci_dev *provider, struct pci_dev *client,
 	struct pci_dev *a = provider, *b = client, *bb;
 	struct pci_dev *a_child = NULL, *b_child = NULL;
 	struct pci_dev *acs_unsupported = NULL;
+	struct pci_host_bridge *provider_host, *client_host;
 	enum pci_acs_p2pdma_state state;
 	struct pci_p2pdma *p2pdma;
 	struct seq_buf acs_list;
+	bool cpu_p2pdma, host_whitelisted = false;
+	bool cache_store = false;
 	int acs_redirect_cnt = 0;
 	int dist_a = 0;
 	int dist_b = 0;
 	char buf[128];
 
 	seq_buf_init(&acs_list, buf, sizeof(buf));
+	if (verbose)
+		pci_info(client,
+			 "P2PDMA ACS debug: begin provider=%s client=%s cache-index=%#lx\n",
+			 pci_name(provider), pci_name(client),
+			 map_types_idx(client));
 
 	/*
 	 * Note, we don't need to take references to devices returned by
@@ -832,10 +1014,27 @@ calc_map_type_and_dist(struct pci_dev *provider, struct pci_dev *client,
 	 * request can only get to the peer through the host bridge.
 	 */
 	*dist = dist_a + dist_b;
+	if (verbose) {
+		pci_info(client,
+			 "P2PDMA ACS debug: no common upstream bridge provider-distance=%d client-distance=%d total=%d\n",
+			 dist_a, dist_b, *dist);
+		pci_p2pdma_log_path("provider", provider, NULL);
+		pci_p2pdma_log_path("client", client, NULL);
+	}
 	goto map_through_host_bridge;
 
 check_paths_acs:
 	*dist = dist_a + dist_b;
+	if (verbose) {
+		pci_info(client,
+			 "P2PDMA ACS debug: common=%s provider-divergence=%s client-divergence=%s provider-distance=%d client-distance=%d total=%d\n",
+			 pci_name(a),
+			 a_child ? pci_name(a_child) : "<none>",
+			 b_child ? pci_name(b_child) : "<none>",
+			 dist_a, dist_b, *dist);
+		pci_p2pdma_log_path("provider", provider, a);
+		pci_p2pdma_log_path("client", client, a);
+	}
 
 	/*
 	 * ACS P2P routing controls apply where a TLP can route toward the peer
@@ -844,7 +1043,8 @@ calc_map_type_and_dist(struct pci_dev *provider, struct pci_dev *client,
 	 */
 	if (a_child && b_child) {
 		state = pci_acs_p2pdma_state(a_child, NULL,
-					     PCI_ACS_P2PDMA_TLP_COMPLETION);
+					     PCI_ACS_P2PDMA_TLP_COMPLETION,
+					     verbose);
 		if (state != PCI_ACS_P2PDMA_DIRECT) {
 			seq_buf_print_bus_devfn(&acs_list, a_child);
 			if (state == PCI_ACS_P2PDMA_REDIRECT)
@@ -854,7 +1054,8 @@ calc_map_type_and_dist(struct pci_dev *provider, struct pci_dev *client,
 		}
 
 		state = pci_acs_p2pdma_state(b_child, a_child,
-					     PCI_ACS_P2PDMA_TLP_REQUEST);
+					     PCI_ACS_P2PDMA_TLP_REQUEST,
+					     verbose);
 		if (state != PCI_ACS_P2PDMA_DIRECT) {
 			seq_buf_print_bus_devfn(&acs_list, b_child);
 			if (state == PCI_ACS_P2PDMA_REDIRECT)
@@ -862,6 +1063,9 @@ calc_map_type_and_dist(struct pci_dev *provider, struct pci_dev *client,
 			else if (!acs_unsupported)
 				acs_unsupported = b_child;
 		}
+	} else if (verbose) {
+		pci_info(client,
+			 "P2PDMA ACS debug: peer divergence is incomplete; no ACS peer-routing controls evaluated\n");
 	}
 
 	/*
@@ -894,8 +1098,25 @@ calc_map_type_and_dist(struct pci_dev *provider, struct pci_dev *client,
 	}
 
 map_through_host_bridge:
-	if (!cpu_supports_p2pdma() &&
-	    !host_bridge_whitelist(provider, client, verbose)) {
+	cpu_p2pdma = cpu_supports_p2pdma();
+	if (!cpu_p2pdma)
+		host_whitelisted = host_bridge_whitelist(provider, client,
+							  verbose);
+
+	if (verbose) {
+		provider_host = pci_find_host_bridge(provider->bus);
+		client_host = pci_find_host_bridge(client->bus);
+		pci_info(client,
+			 "P2PDMA ACS debug: host fallback cpu-support=%u whitelist=%s provider-host=%s client-host=%s same-host=%u\n",
+			 cpu_p2pdma,
+			 cpu_p2pdma ? "not-consulted" :
+					(host_whitelisted ? "yes" : "no"),
+			 provider_host ? dev_name(&provider_host->dev) : "<none>",
+			 client_host ? dev_name(&client_host->dev) : "<none>",
+			 provider_host && provider_host == client_host);
+	}
+
+	if (!cpu_p2pdma && !host_whitelisted) {
 		if (verbose)
 			pci_warn(client, "cannot be used for peer-to-peer DMA as the client and provider (%s) do not share an upstream bridge or whitelisted host bridge\n",
 				 pci_name(provider));
@@ -910,10 +1131,19 @@ calc_map_type_and_dist(struct pci_dev *provider, struct pci_dev *client,
 	 */
 	rcu_read_lock();
 	p2pdma = rcu_dereference(provider->p2pdma);
-	if (p2pdma)
+	if (p2pdma) {
 		xa_store(&p2pdma->map_types, map_types_idx(client),
 			 xa_mk_value(map_type), GFP_ATOMIC);
+		cache_store = true;
+	}
 	rcu_read_unlock();
+	if (verbose)
+		pci_info(client,
+			 "P2PDMA ACS debug: final provider=%s result=%s(%d) distance=%d redirects=%d unsupported=%s cache-store=%u index=%#lx\n",
+			 pci_name(provider), pci_p2pdma_map_type_name(map_type),
+			 map_type, *dist, acs_redirect_cnt,
+			 acs_unsupported ? pci_name(acs_unsupported) : "<none>",
+			 cache_store, map_types_idx(client));
 	return map_type;
 }
 EXPORT_SYMBOL_IF_KUNIT(calc_map_type_and_dist);
@@ -1237,23 +1467,38 @@ enum pci_p2pdma_map_type pci_p2pdma_map_type(struct p2pdma_provider *provider,
 	struct pci_dev *pdev = to_pci_dev(provider->owner);
 	struct pci_dev *client;
 	struct pci_p2pdma *p2pdma;
+	unsigned long cache_index;
+	bool provider_state;
 	int dist;
 
-	if (!pdev->p2pdma)
+	if (!pdev->p2pdma) {
+		pci_info(pdev,
+			 "P2PDMA ACS debug: map lookup rejected; provider state is absent\n");
 		return PCI_P2PDMA_MAP_NOT_SUPPORTED;
+	}
 
-	if (!dev_is_pci(dev))
+	if (!dev_is_pci(dev)) {
+		dev_info(dev,
+			 "P2PDMA ACS debug: provider=%s map lookup rejected; client is not PCI\n",
+			 pci_name(pdev));
 		return PCI_P2PDMA_MAP_NOT_SUPPORTED;
+	}
 
 	client = to_pci_dev(dev);
+	cache_index = map_types_idx(client);
 
 	rcu_read_lock();
 	p2pdma = rcu_dereference(pdev->p2pdma);
 
 	if (p2pdma)
 		type = xa_to_value(xa_load(&p2pdma->map_types,
-					   map_types_idx(client)));
+					   cache_index));
+	provider_state = !!p2pdma;
 	rcu_read_unlock();
+	pci_info(client,
+		 "P2PDMA ACS debug: map lookup provider=%s index=%#lx cached=%s(%d) provider-state=%u\n",
+		 pci_name(pdev), cache_index, pci_p2pdma_map_type_name(type),
+		 type, provider_state);
 
 	if (type == PCI_P2PDMA_MAP_UNKNOWN)
 		return calc_map_type_and_dist(pdev, client, &dist, true);

-- 
2.55.0

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

[Jason Gunthorpe]

> 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