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

[Leon Romanovsky]

On Thu, Aug 13, 2026 at 09:50:07PM -0600, Logan Gunthorpe wrote:
> 
> 
> 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.

We can, and will. I just wanted to minimize the amount of churn
during the RFC stage of this work.

> 
> 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.

Will do

> 
> 
> > @@ -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.

Right now, there is no ACPI support, and I chose to prioritize
existing systems that already have some form of support over an ACPI
solution that does not yet exist.

But you are right: once the HMAT extension is ratified, we
need to use ACPI first.

> 
> --
> 
> 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 a lot.

> 
> Thanks,
> 
> Logan