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

On Fri, Aug 21, 2026 at 07:54:03PM +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] ACS Egress Control (EC) is incorrectly bypassed for Completion TLPs, violating the PCIe specification and allowing unworkable P2P DMA paths to be mapped as direct.
> --
> 
> --- Patch [11]: [PATCH v4 11/18] PCI/P2PDMA: Derive peer-to-peer routing from ACS control bits ---
> commit e4c8e3c20c328ff7022188129cc2873b1613420b
> Author: Leon Romanovsky <[email protected]>
> 
> PCI/P2PDMA: Derive peer-to-peer routing from ACS control bits
> 
> This commit modifies the P2PDMA ACS redirection check to separate the logic
> for Request and Completion TLPs. It evaluates Request Redirect for client
> requests and Completion Redirect for provider read completions.
> 
> > diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
> > index a364008bbf504..1eda7b2135d70 100644
> > --- a/drivers/pci/p2pdma.c
> > +++ b/drivers/pci/p2pdma.c
> 
> [ ... ]
> 
> > @@ -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;
> 
> [Severity: High]
> According to PCIe Base Specification 6.12.4, ACS Egress Control applies to
> all TLPs, including both Requests and Completions.

No, it is wrong.
"PCIe 7.0 explicitly states in §6.12.1.1 and §6.12.1.2: “Completions are never affected
by ACS P2P Egress Control.” Section 6.12.3 says Completion routing is determined exclusively
by Completion Redirect. The EC Control bit description in §7.7.12.3 also refers only
to peer-to-peer Requests."



> 
> Does this logic incorrectly bypass Egress Control (PCI_ACS_EC) for Completion
> TLPs by only evaluating Completion Redirect (PCI_ACS_CR)?
> 
> If a PCIe switch has ACS Egress Control configured to isolate P2P traffic,
> will the hardware drop P2P Read Completions as ACS violations while the
> kernel incorrectly maps the path as direct? This could lead to Completion
> Timeouts and potential runtime failures when drivers attempt P2P DMA reads.
> 
> > -	return 0;
> > +	return ctrl & (PCI_ACS_RR | PCI_ACS_EC) ?
> > +		PCI_ACS_P2PDMA_REDIRECT : PCI_ACS_P2PDMA_DIRECT;
> >  }
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=11
>