On Thu, Oct 01, 2026 at 02:55:12PM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <[email protected]>
>
> ACS redirect controls choose between peer and upstream routes only at the
> path divergence. Applying them below that point rejects valid nested
> topologies because traffic already has only an upstream route.
I guess the point here is that prior to this patch,
calc_map_type_and_dist() returned PCI_P2PDMA_MAP_NOT_SUPPORTED in a
case where it didn't need to? Can you include an example to make this
concrete?
It looks like in v7.3, we only return PCI_P2PDMA_MAP_NOT_SUPPORTED if
a TLP has to go through a host bridge. Do we mistakenly assume that
if a bridge has PCI_ACS_RR set, a Request must go all the way to the
host bridge, even if a bridge closer to the root does not have
PCI_ACS_RR set?
> Evaluate Request controls on the client-side divergence port and Completion
> Redirect on the provider-side port and reject an unreadable ACS Control
> register.
>
> Fixes: 52916982af48 ("PCI/P2PDMA: Support peer-to-peer memory")
> Reviewed-by: Logan Gunthorpe <[email protected]>
> Tested-by: Tushar Dave <[email protected]>
> Signed-off-by: Leon Romanovsky <[email protected]>
> ---
> drivers/pci/p2pdma.c | 101
> +++++++++++++++++++++++++++++----------------
> include/linux/pci-p2pdma.h | 8 ++--
> 2 files changed, 70 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
> index 12612b82d80d..550e6c7346ef 100644
> --- a/drivers/pci/p2pdma.c
> +++ b/drivers/pci/p2pdma.c
> @@ -493,6 +493,7 @@ static struct pci_dev *find_parent_pci_dev(struct device
> *dev)
> }
>
> enum pci_acs_p2pdma_state {
> + PCI_ACS_P2PDMA_NOT_SUPPORTED,
> PCI_ACS_P2PDMA_DIRECT,
> PCI_ACS_P2PDMA_REDIRECT,
> };
> @@ -730,13 +731,13 @@ 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.
> + * The client initiates Requests to provider memory. At the path divergence,
> + * check Request Redirect and Egress Control on the client-side port, and
> + * Completion Redirect for read Completions on the provider-side port.
> *
> - * 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
> + * If ACS redirects traffic at either divergence port, return
> + * PCI_P2PDMA_MAP_THRU_HOST_BRIDGE. If the ACS Control register cannot be
> + * read, return PCI_P2PDMA_MAP_NOT_SUPPORTED. Otherwise, return
> * PCI_P2PDMA_MAP_BUS_ADDR.
> *
> * Any two devices that have a data path that goes through the host bridge
> @@ -750,10 +751,13 @@ calc_map_type_and_dist(struct pci_dev *provider, struct
> pci_dev *client,
> int *dist, bool verbose)
> {
> enum pci_p2pdma_map_type map_type = PCI_P2PDMA_MAP_THRU_HOST_BRIDGE;
> + enum pci_acs_p2pdma_state state = PCI_ACS_P2PDMA_NOT_SUPPORTED;
> struct pci_dev *a = provider, *b = client, *bb;
> + struct pci_dev *a_child = NULL, *b_child = NULL;
> + struct pci_dev *acs_unreadable = NULL;
> struct pci_p2pdma *p2pdma;
> struct seq_buf acs_list;
> - int acs_cnt = 0;
> + int acs_redirect_cnt = 0;
> int dist_a = 0;
> int dist_b = 0;
> char buf[128];
> @@ -768,51 +772,67 @@ calc_map_type_and_dist(struct pci_dev *provider, struct
> pci_dev *client,
> */
> while (a) {
> dist_b = 0;
> -
> - 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++;
> - }
> -
> + b_child = NULL;
> bb = b;
>
> while (bb) {
> if (a == bb)
> - goto check_b_path_acs;
> + goto check_paths_acs;
>
> + b_child = bb;
> bb = pci_upstream_bridge(bb);
> dist_b++;
> }
>
> + a_child = a;
> a = pci_upstream_bridge(a);
> dist_a++;
> }
>
> + /*
> + * The paths share no upstream bridge, so there is no direct path for
> + * ACS to gate: PCI_P2PDMA_MAP_BUS_ADDR is not reachable here and the
> + * request can only get to the peer through the host bridge.
> + */
> *dist = dist_a + dist_b;
> goto map_through_host_bridge;
>
> -check_b_path_acs:
> - bb = b;
> -
> - while (bb) {
> - if (a == bb)
> - break;
> +check_paths_acs:
> + *dist = dist_a + dist_b;
>
> - 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++;
> + /*
> + * ACS P2P routing controls apply where a TLP can route toward the peer
> + * or upstream. Below that divergence, its only route toward the other
> + * branch is upstream, so redirect controls do not affect the path.
> + */
> + if (a_child && b_child) {
> + if (pci_acs_p2pdma_ctrl(a_child, &ctrl))
> + state = pci_acs_p2pdma_completion(ctrl);
> + if (state != PCI_ACS_P2PDMA_DIRECT) {
> + seq_buf_print_bus_devfn(&acs_list, a_child);
> + if (state == PCI_ACS_P2PDMA_REDIRECT)
> + acs_redirect_cnt++;
> + else if (!acs_unreadable)
> + acs_unreadable = a_child;
> }
>
> - bb = pci_upstream_bridge(bb);
> + state = PCI_ACS_P2PDMA_NOT_SUPPORTED;
> + if (pci_acs_p2pdma_ctrl(b_child, &ctrl))
> + state = pci_acs_p2pdma_request(ctrl);
> + if (state != PCI_ACS_P2PDMA_DIRECT) {
> + seq_buf_print_bus_devfn(&acs_list, b_child);
> + if (state == PCI_ACS_P2PDMA_REDIRECT)
> + acs_redirect_cnt++;
> + else if (!acs_unreadable)
> + acs_unreadable = b_child;
> + }
> }
>
> - *dist = dist_a + dist_b;
> -
> - if (!acs_cnt) {
> + /*
> + * Below a shared upstream bridge, a path whose divergence ports do not
> + * redirect routes the request directly.
> + */
> + if (!acs_unreadable && !acs_redirect_cnt) {
> map_type = PCI_P2PDMA_MAP_BUS_ADDR;
> goto done;
> }
> @@ -821,10 +841,21 @@ calc_map_type_and_dist(struct pci_dev *provider, struct
> pci_dev *client,
> /* Drop the final semicolon; the list is not empty here. */
> if (!seq_buf_has_overflowed(&acs_list))
> acs_list.buffer[acs_list.len - 1] = '\0';
> - pci_warn(client, "ACS redirect is set between the client and
> provider (%s)\n",
> - pci_name(provider));
> - pci_warn(client, "to disable ACS redirect for this path, add
> the kernel parameter: pci=disable_acs_redir=%s\n",
> - seq_buf_str(&acs_list));
> + if (acs_unreadable)
> + pci_warn(client, "ACS Control is unreadable for
> provider %s at %s\n",
> + pci_name(provider), pci_name(acs_unreadable));
> + else {
> + pci_warn(client, "ACS redirect is set between the
> client and provider (%s)\n",
> + pci_name(provider));
> + pci_warn(client, "to disable ACS controls for this
> path, add the kernel parameter: pci=disable_acs_redir=%s\n",
> + seq_buf_str(&acs_list));
> + }
> + }
> +
> + /* An unreadable control does not establish an upstream redirect. */
> + if (acs_unreadable) {
> + map_type = PCI_P2PDMA_MAP_NOT_SUPPORTED;
> + goto done;
> }
>
> map_through_host_bridge:
> diff --git a/include/linux/pci-p2pdma.h b/include/linux/pci-p2pdma.h
> index 873de20a2247..dd17501ba1b6 100644
> --- a/include/linux/pci-p2pdma.h
> +++ b/include/linux/pci-p2pdma.h
> @@ -42,10 +42,10 @@ enum pci_p2pdma_map_type {
> PCI_P2PDMA_MAP_NONE,
>
> /*
> - * PCI_P2PDMA_MAP_NOT_SUPPORTED: Indicates the transaction will
> - * traverse the host bridge and the host bridge is not in the
> - * allowlist. DMA Mapping routines should return an error when
> - * this is returned.
> + * PCI_P2PDMA_MAP_NOT_SUPPORTED: Indicates no safe mapping is available,
> + * for example because ACS blocks the direct path or the required host
> + * bridge is not in the allowlist. DMA Mapping routines should return an
> + * error when this is returned.
> */
> PCI_P2PDMA_MAP_NOT_SUPPORTED,
>
>
> --
> 2.55.0
>
