Re: [PATCH v9 02/18] PCI/P2PDMA: Derive routing from directional ACS controls

[Leon Romanovsky]

On Tue, Oct 06, 2026 at 03:49:47PM -0500, Bjorn Helgaas wrote:
> On Thu, Oct 01, 2026 at 02:55:10PM +0300, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@xxxxxxxxxx>
> > 
> > pci_bridge_has_acs_redir() treats Request and Completion Redirect as
> > interchangeable. On asymmetric fabrics, a control for only the reverse TLP
> > direction can unnecessarily force P2PDMA through the host bridge.
> 
> Does "the reverse TLP direction" refer to Completions?

In general, the P2P code treats TLPs flowing from device A to device B the
same as TLPs flowing from device B to device A.

However, in the context of this commit message, yes: completions flow in the
opposite direction from the device's perspective.

> 
> > Evaluate Request Redirect for client Requests and Completion Redirect for
> > provider read Completions. Continue treating enabled Egress Control
> > conservatively as a Request redirect.
> 
> Completion Redirect is intended to avoid ordering rule violations
> between Completions and Requests when Requests are redirected (PCIe
> r7.0, sec 6.12.1.1).  I assume this patch preserves the ordering rule,
> but does the commit log need to say something about that?  I don't
> know enough about P2P DMA for it to be obvious to me.

I don't think so, i didn't change anything related to ordering.

> 
> Not really a question for this series, but p2pdma.c and p2pdma.rst
> refer to "clients" and "providers", neither of which are mentioned in
> the PCIe spec.  In this case it sounds like a client is a Requester
> and a provider is a Completer in spec terms.  Is that always the case?
> If so, "client" and "provider" in this paragraph are not adding any
> information.
> 
> If "client" is not the same concept as "Requester" and "provider" not
> the same as "Completer", maybe p2pdma.rst could explain the
> difference?

Client vs. provider are actual target vs. initiator. They express the
device role in the flow.

Thanks

> 
> > Fixes: 52916982af48 ("PCI/P2PDMA: Support peer-to-peer memory")
> > Reviewed-by: Logan Gunthorpe <logang@xxxxxxxxxxxx>
> > Tested-by: Tushar Dave <tdave@xxxxxxxxxx>
> > Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxx>
> > ---
> >  drivers/pci/p2pdma.c | 75 ++++++++++++++++++++++++++++++++++++++++------------
> >  1 file changed, 58 insertions(+), 17 deletions(-)
> > 
> > diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
> > index 4e4d2df17a45..12612b82d80d 100644
> > --- a/drivers/pci/p2pdma.c
> > +++ b/drivers/pci/p2pdma.c
> > @@ -21,6 +21,8 @@
> >  #include <linux/seq_buf.h>
> >  #include <linux/xarray.h>
> >  
> > +#include "pci.h"
> > +
> >  struct pci_p2pdma {
> >  	struct gen_pool *pool;
> >  	bool p2pmem_published;
> > @@ -490,26 +492,56 @@ static struct pci_dev *find_parent_pci_dev(struct device *dev)
> >  	return NULL;
> >  }
> >  
> > +enum pci_acs_p2pdma_state {
> > +	PCI_ACS_P2PDMA_DIRECT,
> > +	PCI_ACS_P2PDMA_REDIRECT,
> > +};
> > +
> >  /*
> > - * 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.
> > + * Decide how a peer-to-peer Request at an ACS-capable ingress port routes,
> > + * from that port's ACS Control register.
> > + *
> > + * Linux does not read the Egress Control Vector, so Egress Control is treated
> > + * conservatively as a redirect. Per PCIe r7.0 Table 6-11 the outcomes it
> > + * selects are a direct route and an ACS Violation, and neither one lets peer
> > + * bus addressing be assumed.
> >   */
> > -static int pci_bridge_has_acs_redir(struct pci_dev *pdev)
> > +static enum pci_acs_p2pdma_state
> > +pci_acs_p2pdma_request(u16 ctrl)
> >  {
> > -	int pos;
> > -	u16 ctrl;
> > +	return ctrl & (PCI_ACS_RR | PCI_ACS_EC) ?
> > +		PCI_ACS_P2PDMA_REDIRECT : PCI_ACS_P2PDMA_DIRECT;
> > +}
> >  
> > -	pos = pdev->acs_cap;
> > -	if (!pos)
> > -		return 0;
> > +/*
> > + * 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.
> > + */
> > +static enum pci_acs_p2pdma_state
> > +pci_acs_p2pdma_completion(u16 ctrl)
> > +{
> > +	return ctrl & PCI_ACS_CR ? PCI_ACS_P2PDMA_REDIRECT :
> > +				   PCI_ACS_P2PDMA_DIRECT;
> > +}
> >  
> > -	pci_read_config_word(pdev, pos + PCI_ACS_CTRL, &ctrl);
> > +/*
> > + * Read @pdev's ACS Control register. A device without an ACS capability has
> > + * no peer-to-peer controls at all, which routes the same as having them all
> > + * clear. Returns false when the register is present but cannot be read; @ctrl
> > + * is then meaningless.
> > + */
> > +static bool pci_acs_p2pdma_ctrl(struct pci_dev *pdev, u16 *ctrl)
> > +{
> > +	int pos;
> >  
> > -	if (ctrl & (PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_EC))
> > -		return 1;
> > +	pos = pdev->acs_cap;
> > +	if (!pos) {
> > +		*ctrl = 0;
> > +		return true;
> > +	}
> >  
> > -	return 0;
> > +	return !pci_read_config_word(pdev, pos + PCI_ACS_CTRL, ctrl);
> >  }
> >  
> >  static void seq_buf_print_bus_devfn(struct seq_buf *buf, struct pci_dev *pdev)
> > @@ -698,6 +730,10 @@ static unsigned long map_types_idx(struct pci_dev *client)
> >   * then to Device B. The mapping type returned depends on the ACS
> >   * redirection setting of the ports along the path.
> >   *
> > + * The client initiates Requests to provider memory. Check Request Redirect
> > + * on the client path and Completion Redirect for read Completions on the
> > + * provider path.
> > + *
> >   * If ACS redirect is set on any port in the path, traffic between the
> >   * devices will go through the host bridge, so return
> >   * PCI_P2PDMA_MAP_THRU_HOST_BRIDGE; otherwise return
> > @@ -721,6 +757,7 @@ calc_map_type_and_dist(struct pci_dev *provider, struct pci_dev *client,
> >  	int dist_a = 0;
> >  	int dist_b = 0;
> >  	char buf[128];
> > +	u16 ctrl;
> >  
> >  	seq_buf_init(&acs_list, buf, sizeof(buf));
> >  
> > @@ -732,7 +769,9 @@ calc_map_type_and_dist(struct pci_dev *provider, struct pci_dev *client,
> >  	while (a) {
> >  		dist_b = 0;
> >  
> > -		if (pci_bridge_has_acs_redir(a)) {
> > +		if (!pci_acs_p2pdma_ctrl(a, &ctrl) ||
> > +		    pci_acs_p2pdma_completion(ctrl) ==
> > +			    PCI_ACS_P2PDMA_REDIRECT) {
> >  			seq_buf_print_bus_devfn(&acs_list, a);
> >  			acs_cnt++;
> >  		}
> > @@ -761,7 +800,9 @@ calc_map_type_and_dist(struct pci_dev *provider, struct pci_dev *client,
> >  		if (a == bb)
> >  			break;
> >  
> > -		if (pci_bridge_has_acs_redir(bb)) {
> > +		if (!pci_acs_p2pdma_ctrl(bb, &ctrl) ||
> > +		    pci_acs_p2pdma_request(ctrl) ==
> > +			    PCI_ACS_P2PDMA_REDIRECT) {
> >  			seq_buf_print_bus_devfn(&acs_list, bb);
> >  			acs_cnt++;
> >  		}
> > @@ -1109,10 +1150,10 @@ EXPORT_SYMBOL_GPL(pci_p2pmem_publish);
> >  /**
> >   * pci_p2pdma_map_type - Determine the mapping type for P2PDMA transfers
> >   * @provider: P2PDMA provider structure
> > - * @dev: Target device for the transfer
> > + * @dev: Client device that initiates the transfer
> >   *
> >   * Determines how peer-to-peer DMA transfers should be mapped between
> > - * the provider and the target device. The mapping type indicates whether
> > + * the provider and the client device. The mapping type indicates whether
> >   * the transfer can be done directly through PCI switches or must go
> >   * through the host bridge.
> >   */
> > 
> > -- 
> > 2.55.0
> >