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 <[email protected]> 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/[email protected]?part=10
