Re: [PATCH RFC 4/7] PCI/P2PDMA: Prefer providers with better HMAT performance

[Logan Gunthorpe]

On 2026-08-12 1:47 p.m., Leon Romanovsky wrote:
> @@ -820,21 +844,29 @@ static unsigned long map_types_idx(struct pci_dev *client)
>   * PCI_P2PDMA_MAP_THRU_HOST_BRIDGE. Otherwise, return
>   * PCI_P2PDMA_MAP_BUS_ADDR.
>   *
> - * Any two devices that have a data path that goes through the host bridge
> - * will consult a whitelist. If the host bridge is in the whitelist, return
> - * PCI_P2PDMA_MAP_THRU_HOST_BRIDGE with the distance set to the number of
> - * ports per above. If the device is not in the whitelist, return
> - * PCI_P2PDMA_MAP_NOT_SUPPORTED.
> + * Any two devices that have a data path through a host bridge require
> + * platform support from the CPU, the host bridge whitelist, or a reachable
> + * ordered HMAT path. Return PCI_P2PDMA_MAP_NOT_SUPPORTED when none of those
> + * sources permits the path.
>   */
>  VISIBLE_IF_KUNIT enum pci_p2pdma_map_type
>  calc_map_type_and_dist(struct pci_dev *provider, struct pci_dev *client,
>  		int *dist, bool verbose)
> +{
> +	return __calc_map_type_and_dist(provider, client, dist, verbose, NULL);
> +}

This patch is a bit hard to follow compared to the earlier ones in this
series.

Why do we need to have a different variant of the function that excludes
coord and is exported only for KUNIT? Why can't we just export the
original function as is instead of creating the double underscore
variant? Personally, I've been trying to avoid creating double
underscore functions and naming functions more appropriately. But this
one seems weird to me.

Seems like some of these details would be better split into another
patch justifying them as this change seems more like prep changes for
the KUNIT work that follows instead of what the patch is meant to do:
enabling the HMAT stuff.


> @@ -941,12 +976,22 @@ calc_map_type_and_dist(struct pci_dev *provider, struct pci_dev *client,
>  	}
>  
>  map_through_host_bridge:
> -	if (!cpu_supports_p2pdma() &&
> -	    !host_bridge_hmat_p2p(provider, client) &&
> -	    !host_bridge_whitelist(provider, client, verbose)) {
> -		if (verbose)
> +	host_bridge_allowed = cpu_supports_p2pdma() ||
> +			      host_bridge_whitelist(provider, client,
> +						    false);
> +	/*
> +	 * The coordinates are only used to rank providers, which happens in
> +	 * process context. Skip the firmware lookup on the mapping path once
> +	 * the CPU or the whitelist has already permitted the path.
> +	 */

This feels backwards to me. If ACPI is kind enough to include
information on P2PDMA support then I feel like we should use it
exclusively. Not prioritize the old janky whitelists.

--

In general this series looks really nice. And I'm so glad someone is
finally adding this stuff to ACPI so that we can move away from the
annoying white list.

Thanks,

Logan