Re: [PATCH] PCI/P2PDMA: start with a whitelist for root complexes

[Bjorn Helgaas <helgaas@kernel.org>]

On Thu, Apr 18, 2019 at 01:58:59PM +0200, Christian König wrote:
> A lot of root complexes can still do P2P even when PCI devices
> don't share a common upstream bridge.
> 
> Start adding a whitelist and allow P2P if both participants are
> attached to known good root complex.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>

I applied this with Logan's reviewed-by to pci/peer-to-peer for v5.2,
thanks!

This should be easy to build on in the future as we discover new
hardware that supports this and as we find out any quirks in the way
they do it.

> ---
>  drivers/pci/p2pdma.c | 38 +++++++++++++++++++++++++++++++++++---
>  1 file changed, 35 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
> index c52298d76e64..212baaa7f93b 100644
> --- a/drivers/pci/p2pdma.c
> +++ b/drivers/pci/p2pdma.c
> @@ -274,6 +274,31 @@ static void seq_buf_print_bus_devfn(struct seq_buf *buf, struct pci_dev *pdev)
>  	seq_buf_printf(buf, "%s;", pci_name(pdev));
>  }
>  
> +/*
> + * If we can't find a common upstream bridge take a look at the root complex and
> + * compare it to a whitelist of known good hardware.
> + */
> +static bool root_complex_whitelist(struct pci_dev *dev)
> +{
> +	struct pci_host_bridge *host = pci_find_host_bridge(dev->bus);
> +	struct pci_dev *root = pci_get_slot(host->bus, PCI_DEVFN(0, 0));
> +	unsigned short vendor, device;
> +
> +	if (!root)
> +		return false;
> +
> +	vendor = root->vendor;
> +	device = root->device;
> +	pci_dev_put(root);
> +
> +	/* AMD ZEN host bridges can do peer to peer */
> +	if (vendor == PCI_VENDOR_ID_AMD && device == 0x1450)
> +		return true;
> +
> +	/* TODO: Extend that to a proper whitelist */
> +	return false;
> +}
> +
>  /*
>   * Find the distance through the nearest common upstream bridge between
>   * two PCI devices.
> @@ -317,13 +342,13 @@ static void seq_buf_print_bus_devfn(struct seq_buf *buf, struct pci_dev *pdev)
>   * In this case, a list of all infringing bridge addresses will be
>   * populated in acs_list (assuming it's non-null) for printk purposes.
>   */
> -static int upstream_bridge_distance(struct pci_dev *a,
> -				    struct pci_dev *b,
> +static int upstream_bridge_distance(struct pci_dev *provider,
> +				    struct pci_dev *client,
>  				    struct seq_buf *acs_list)
>  {
> +	struct pci_dev *a = provider, *b = client, *bb;
>  	int dist_a = 0;
>  	int dist_b = 0;
> -	struct pci_dev *bb = NULL;
>  	int acs_cnt = 0;
>  
>  	/*
> @@ -354,6 +379,13 @@ static int upstream_bridge_distance(struct pci_dev *a,
>  		dist_a++;
>  	}
>  
> +	/* Allow the connection if both devices are on a whitelisted root
> +	 * complex, but add an arbitary large value to the distance.
> +	 */
> +	if (root_complex_whitelist(provider) &&
> +	    root_complex_whitelist(client))
> +		return 0x1000 + dist_a + dist_b;
> +
>  	return -1;
>  
>  check_b_path_acs:
> -- 
> 2.17.1
>