Re: [PATCH v12 10/19] PCI/P2PDMA: Reject Translated Requests blocked by Translation Blocking

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] PCI/P2PDMA: missing Intel SPT PCH ACS Control offset check for PCI_ACS_TB
--

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

PCI/P2PDMA: Reject Translated Requests blocked by Translation Blocking

This commit teaches P2PDMA to check ACS Translation Blocking across the
ports traversed by a peer-to-peer route and reject Translated Requests
when any port along the path blocks them.

> diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
> index 785e4668a2af5..fc4dea28c8606 100644
> --- a/drivers/pci/p2pdma.c
> +++ b/drivers/pci/p2pdma.c
[ ... ]
> @@ -678,6 +691,39 @@ pci_acs_p2pdma_no_upstream_forwarding(struct pci_dev *common)
>  	return NULL;
>  }
>  
> +/*
> + * Report whether any port between @client and @common rejects Translated
> + * addresses. If @common is NULL, every port up to the host bridge is walked.
> + * A port whose ACS Control cannot be read counts as blocking, which withdraws
> + * only the Translated classes because an Untranslated Request is routed by the
> + * redirect controls instead.
> + */
> +static bool pci_p2pdma_path_blocks_translation(struct pci_dev *client,
> +					       struct pci_dev *common)
> +{
> +	struct pci_dev *pdev;
> +	u16 ctrl;
> +
> +	/*
> +	 * @common is @client itself when the provider is the client or sits
> +	 * below it. The Request never travels upstream then, so no port sees
> +	 * it and none can reject its Address Type.
> +	 */
> +	if (client == common)
> +		return false;
> +
> +	for (pdev = pci_upstream_bridge(client); pdev && pdev != common;
> +	     pdev = pci_upstream_bridge(pdev)) {
> +		if (!pci_acs_p2pdma_ctrl(pdev, &ctrl))
> +			return true;
> +
> +		if (ctrl & PCI_ACS_TB)
> +			return true;
> +	}

[Severity: Medium]
When calc_map_type_and_dist() calls pci_p2pdma_path_blocks_translation()
with common set to NULL (either when path.no_common_bridge is true or
when checking path.tb_above_divergence from b_child up to the root bus),
this loop walks all the way up to the Root Port.

On Intel Sunrise Point, Union Point, and 7th/8th Gen Mobile PCH Root
Ports matched by pci_quirk_intel_spt_pch_acs_match(), pdev->acs_cap is
non-zero, but due to hardware errata the ACS Capability and Control
registers are 32-bit dwords rather than 16-bit words. As a result,
pci_quirk_enable_intel_spt_pch_acs() programs PCI_ACS_TB into
pos + INTEL_SPT_ACS_CTRL (offset 8):

drivers/pci/quirks.c:pci_quirk_enable_intel_spt_pch_acs() {
	...
	pci_read_config_dword(dev, pos + PCI_ACS_CAP, &cap);
	pci_read_config_dword(dev, pos + INTEL_SPT_ACS_CTRL, &ctrl);
	...
	if (pci_ats_disabled() || dev->external_facing || dev->untrusted)
		ctrl |= (cap & PCI_ACS_TB);

	pci_write_config_dword(dev, pos + INTEL_SPT_ACS_CTRL, ctrl);
	...
}

However, pci_acs_p2pdma_ctrl() reads the 16-bit word at pos + PCI_ACS_CTRL
(offset 6), which on these Root Ports is the upper 16 bits of the 32-bit
ACS Capability register (reading as 0):

drivers/pci/p2pdma.c:pci_acs_p2pdma_ctrl() {
	...
	if (pci_read_config_word(pdev, pos + PCI_ACS_CTRL, ctrl) ||
	    PCI_POSSIBLE_ERROR(*ctrl))
		return false;

	*ctrl = pci_acs_p2pdma_quirk_redirect(pdev, *ctrl);
	return true;
}

Unlike pci_acs_p2pdma_no_upstream_forwarding(), which consults
pci_dev_specific_acs_enabled() when walking to the Root Port,
pci_p2pdma_path_blocks_translation() only inspects ctrl from
pci_acs_p2pdma_ctrl().

Won't pci_p2pdma_path_blocks_translation() miss PCI_ACS_TB on these
Intel PCH Root Ports and allow calc_map_type_and_dist() to return
PCI_P2PDMA_MAP_THRU_HOST_BRIDGE instead of
PCI_P2PDMA_MAP_NOT_SUPPORTED for Translated Requests that the Root Port
will reject?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103-fix-p2p-acs-v4-0-v12-0-912101f93517@nvidia.com?part=10