Re: [PATCH v11 09/19] PCI/P2PDMA: Route Relaxed Ordering Completions directly

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] pci/p2pdma: false positive warning if Relaxed Ordering route succeeds
--

commit e6e15dbbdb2b5d6dee258b1197f71025a221015f
Author: Leon Romanovsky <leon@kernel.org>

PCI/P2PDMA: Route Relaxed Ordering Completions directly

ACS P2P Completion Redirect leaves Completions carrying Relaxed Ordering
alone, per PCIe r7.0 sec 6.12.1.1. A Completion copies that attribute from
its Request, so a client setting it on Non-Posted Requests need not use the
host bridge solely because Completion Redirect is enabled.

Route Completions directly for PCI_P2PDMA_TLP_RELAXED_CPL unless a device
quirk redirects them or the Completer lacks an internal route to its
sibling. Neither restriction promises a Relaxed Ordering exception. No
caller selects these classes yet.

> diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
> index 9d416329aa4b8..785e4668a2af5 100644
> --- a/drivers/pci/p2pdma.c
> +++ b/drivers/pci/p2pdma.c
> @@ -580,11 +580,15 @@ pci_acs_p2pdma_request(u16 ctrl, unsigned int tlp_flags)
>  /*
>   * Decide how a peer-to-peer Completion at an ACS-capable ingress port routes.
>   * PCIe r7.0 sec 6.12.1.1: no ACS control other than P2P Completion Redirect
> - * affects a Completion.
> + * affects a Completion, and that one leaves Completions carrying the Relaxed
> + * Ordering attribute alone.
>   */
>  static enum pci_acs_p2pdma_state
>  pci_acs_p2pdma_completion(u16 ctrl, unsigned int tlp_flags)
>  {
> +	if (tlp_flags & PCI_P2PDMA_TLP_RELAXED_CPL)
> +		return PCI_ACS_P2PDMA_DIRECT;
> +

[Severity: Low]
Now that Relaxed Ordering completions can route directly while strict ordering
might still redirect, does this cause a false positive warning in
calc_map_type_and_dist() in drivers/pci/p2pdma.c?

calc_map_type_and_dist() iterates over all PCI_P2PDMA_TLP_CLASSES and sets
host_fallback = true if any class requires the host bridge:

	for (flags = 0; flags < PCI_P2PDMA_TLP_CLASSES; flags++) {
		map_type[flags] = pci_p2pdma_route(&path, flags);
		if (map_type[flags] == PCI_P2PDMA_MAP_THRU_HOST_BRIDGE)
			host_fallback = true;
	}

Later, if the host bridge does not support P2PDMA, it emits an unconditional
warning for the device:

	if (!cpu_p2pdma && !host_whitelisted) {
		if (verbose)
			pci_warn(client, "cannot be used for peer-to-peer DMA...");
	}

If a caller successfully requests a Relaxed Ordering route that maps directly,
but the strict ordering route required the host bridge, will this
unconditional warning mislead users into thinking P2P DMA is completely
broken for their setup?

>  	return ctrl & PCI_ACS_CR ? PCI_ACS_P2PDMA_REDIRECT :
>  				   PCI_ACS_P2PDMA_DIRECT;
>  }

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013-fix-p2p-acs-v4-0-v11-0-72cf8c96e617@nvidia.com?part=9