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

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Incomplete isolation check fix for Intel RCiEPs allowing Direct Translated P2P.
--

--- Patch [8]: [PATCH v4 08/18] PCI: Account for Direct Translated P2P in ACS isolation checks ---
commit 9a6d73ba8f155a4921ede8386e7c653f5cc4bd35
Author: Leon Romanovsky <[email protected]>

PCI: Account for Direct Translated P2P in ACS isolation checks

This commit fixes ACS isolation checks by accounting for Direct Translated
P2P bypassing Request Redirect. It introduces a scope parameter to allow
callers to specify whether peer-to-peer Requests must be isolated.

> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c

[ ... ]

> -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
>  	 * addresses.  Refer to Intel VT-d specification, r3.1, sec 3.16,
>  	 * "Root-Complex Peer to Peer Considerations".
>  	 */
>  	if (pci_pcie_type(dev) != PCI_EXP_TYPE_RC_END)
>  		return -ENOTTY;
>  
>  	return pci_acs_ctrl_enabled(acs_flags,
>  		PCI_ACS_SV | PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_UF);
>  }

[Severity: High]
This isn't a bug introduced by this patch, but should pci_quirk_rciep_acs()
account for the new scope parameter when evaluating PCI_ACS_RR?

The comment in pci_quirk_rciep_acs() notes that Intel RCiEPs allow P2P on
translated addresses, which means Request Redirect does not actually isolate
them from each other for translated requests.

If a caller passes PCI_ACS_SCOPE_ALL, does unconditionally returning success
here allow a malicious VM assigned a multifunction Intel RCiEP to forge
translated requests (AT=Translated) to perform peer-to-peer DMA to another
function, bypassing the IOMMU?

Since this patch introduces pci_acs_rr_ineffective() to handle this exact
scenario for standard ACS capabilities and the Intel SPT PCH quirk, would
it make sense to apply similar scope checking here?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=8