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

2019-04-23 Thread Christian König

Am 19.04.19 um 19:13 schrieb Alex Deucher:

On Thu, Apr 18, 2019 at 8:09 AM 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 
---
  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)

We should also add:
(vendor == PCI_VENDOR_ID_AMD && device == 0x1576) // Carrizo
(vendor == PCI_VENDOR_ID_AMD && device == 0x1536) // Kaveri/kabini/mullins


Well not sure about Carrizo, but at least on Kaveri this does NOT work 
in general.


The root complex there can't route between different PCI devices!

An exception is the APU on Kaveri, here the BAR is actually backed by 
system memory which is invisible to the kernel.


We need to handle this special case inside the driver itself.

Christian.



Alex


+   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

___
amd-gfx mailing list
amd-...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

2019-04-19 Thread Alex Deucher
On Thu, Apr 18, 2019 at 8:09 AM 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 
> ---
>  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)

We should also add:
(vendor == PCI_VENDOR_ID_AMD && device == 0x1576) // Carrizo
(vendor == PCI_VENDOR_ID_AMD && device == 0x1536) // Kaveri/kabini/mullins

Alex

> +   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
>
> ___
> amd-gfx mailing list
> amd-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

2019-04-18 Thread Christian König
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 
---
 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

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel