DMA-buf P2P

DMA-buf P2P

[Christian König]

Hi guys,

as promised this is the patch set which enables P2P buffer sharing with DMA-buf.

Basic idea is that importers can set a flag noting that they can deal with and sgt which doesn't contains pages.

This in turn is the signal to the exporter that we don't need to move a buffer to system memory any more when a remote device wants to access it.

Please review and/or comment,
Christian.


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

[PATCH 6/6] drm/amdgpu: add support for exporting VRAM using DMA-buf v2

[Christian König]

We should be able to do this now after checking all the prerequisites.

v2: fix entrie count in the sgt

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c    | 46 ++++++++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h      |  9 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 96 ++++++++++++++++++++
 3 files changed, 142 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
index a290ae830b11..55bb39281c5d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
@@ -318,22 +318,45 @@ amdgpu_gem_map_dma_buf(struct dma_buf_attachment *attach,
 	}
 
 	if (attach->invalidate) {
-		/* move buffer into GTT */
+		/* move buffer into GTT or VRAM */
 		struct ttm_operation_ctx ctx = { false, false };
+		unsigned domains = AMDGPU_GEM_DOMAIN_GTT;
 
-		amdgpu_bo_placement_from_domain(bo, AMDGPU_GEM_DOMAIN_GTT);
+		if (bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM &&
+		    attach->peer2peer) {
+			bo->flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
+			domains |= AMDGPU_GEM_DOMAIN_VRAM;
+		}
+		amdgpu_bo_placement_from_domain(bo, domains);
 		r = ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);
 		if (r)
 			return ERR_PTR(r);
 	}
 
-	sgt = drm_prime_pages_to_sg(bo->tbo.ttm->pages, bo->tbo.num_pages);
-	if (IS_ERR(sgt))
-		return sgt;
+	switch (bo->tbo.mem.mem_type) {
+	case TTM_PL_TT:
+		sgt = drm_prime_pages_to_sg(bo->tbo.ttm->pages,
+					    bo->tbo.num_pages);
+		if (IS_ERR(sgt))
+			return sgt;
+
+		if (!dma_map_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir,
+				      DMA_ATTR_SKIP_CPU_SYNC)) {
+			r = -EINVAL;
+			goto error_free;
+		}
+		break;
 
-	if (!dma_map_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir,
-			      DMA_ATTR_SKIP_CPU_SYNC))
+	case TTM_PL_VRAM:
+		r = amdgpu_vram_mgr_alloc_sgt(adev, &bo->tbo.mem, attach->dev,
+					      dir, &sgt);
+		if (r)
+			goto error_free;
+		break;
+	default:
+		r = -EINVAL;
 		goto error_free;
+	}
 
 	if (attach->dev->driver != adev->dev->driver)
 		bo->prime_shared_count++;
@@ -343,7 +366,7 @@ amdgpu_gem_map_dma_buf(struct dma_buf_attachment *attach,
 error_free:
 	sg_free_table(sgt);
 	kfree(sgt);
-	return ERR_PTR(-ENOMEM);
+	return ERR_PTR(r);
 }
 
 /**
@@ -367,10 +390,15 @@ static void amdgpu_gem_unmap_dma_buf(struct dma_buf_attachment *attach,
 	if (attach->dev->driver != adev->dev->driver && bo->prime_shared_count)
 		bo->prime_shared_count--;
 
-	if (sgt) {
+	if (!sgt)
+		return;
+
+	if (sgt->sgl->page_link) {
 		dma_unmap_sg(attach->dev, sgt->sgl, sgt->nents, dir);
 		sg_free_table(sgt);
 		kfree(sgt);
+	} else {
+		amdgpu_vram_mgr_free_sgt(adev, attach->dev, dir, sgt);
 	}
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
index c2b7669004ba..0b4cdbe867e7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
@@ -72,6 +72,15 @@ uint64_t amdgpu_gtt_mgr_usage(struct ttm_mem_type_manager *man);
 int amdgpu_gtt_mgr_recover(struct ttm_mem_type_manager *man);
 
 u64 amdgpu_vram_mgr_bo_visible_size(struct amdgpu_bo *bo);
+int amdgpu_vram_mgr_alloc_sgt(struct amdgpu_device *adev,
+			      struct ttm_mem_reg *mem,
+			      struct device *dev,
+			      enum dma_data_direction dir,
+			      struct sg_table **sgt);
+void amdgpu_vram_mgr_free_sgt(struct amdgpu_device *adev,
+			      struct device *dev,
+			      enum dma_data_direction dir,
+			      struct sg_table *sgt);
 uint64_t amdgpu_vram_mgr_usage(struct ttm_mem_type_manager *man);
 uint64_t amdgpu_vram_mgr_vis_usage(struct ttm_mem_type_manager *man);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
index ec9ea3fdbb4a..520cea4dbdab 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
@@ -399,6 +399,102 @@ static void amdgpu_vram_mgr_del(struct ttm_mem_type_manager *man,
 	mem->mm_node = NULL;
 }
 
+/**
+ * amdgpu_vram_mgr_alloc_sgt - allocate and fill a sg table
+ *
+ * @adev: amdgpu device pointer
+ * @mem: TTM memory object
+ * @dev: the other device
+ * @dir: dma direction
+ * @sgt: resulting sg table
+ *
+ * Allocate and fill a sg table from a VRAM allocation.
+ */
+int amdgpu_vram_mgr_alloc_sgt(struct amdgpu_device *adev,
+			      struct ttm_mem_reg *mem,
+			      struct device *dev,
+			      enum dma_data_direction dir,
+			      struct sg_table **sgt)
+{
+	struct drm_mm_node *node;
+	struct scatterlist *sg;
+	int num_entries = 0;
+	unsigned int pages;
+	int i, r;
+
+	*sgt = kmalloc(sizeof(*sg), GFP_KERNEL);
+	if (!*sgt)
+		return -ENOMEM;
+
+	for (pages = mem->num_pages, node = mem->mm_node;
+	     pages; pages -= node->size, ++node)
+		++num_entries;
+
+	r = sg_alloc_table(*sgt, num_entries, GFP_KERNEL);
+	if (r)
+		goto error_free;
+
+	for_each_sg((*sgt)->sgl, sg, num_entries, i)
+		sg->length = 0;
+
+	node = mem->mm_node;
+	for_each_sg((*sgt)->sgl, sg, num_entries, i) {
+		phys_addr_t phys = (node->start << PAGE_SHIFT) +
+			adev->gmc.aper_base;
+		size_t size = node->size << PAGE_SHIFT;
+		dma_addr_t addr;
+
+		++node;
+		addr = dma_map_resource(dev, phys, size, dir,
+					DMA_ATTR_SKIP_CPU_SYNC);
+		r = dma_mapping_error(dev, addr);
+		if (r)
+			goto error_unmap;
+
+		sg_set_dma_addr(sg, addr, size, 0);
+	}
+	return 0;
+
+error_unmap:
+	for_each_sg((*sgt)->sgl, sg, num_entries, i) {
+		if (!sg->length)
+			continue;
+
+		dma_unmap_resource(dev, sg->dma_address,
+				   sg->length, dir,
+				   DMA_ATTR_SKIP_CPU_SYNC);
+	}
+	sg_free_table(*sgt);
+
+error_free:
+	kfree(*sgt);
+	return r;
+}
+
+/**
+ * amdgpu_vram_mgr_alloc_sgt - allocate and fill a sg table
+ *
+ * @adev: amdgpu device pointer
+ * @sgt: sg table to free
+ *
+ * Free a previously allocate sg table.
+ */
+void amdgpu_vram_mgr_free_sgt(struct amdgpu_device *adev,
+			      struct device *dev,
+			      enum dma_data_direction dir,
+			      struct sg_table *sgt)
+{
+	struct scatterlist *sg;
+	int i;
+
+	for_each_sg(sgt->sgl, sg, sgt->nents, i)
+		dma_unmap_resource(dev, sg->dma_address,
+				   sg->length, dir,
+				   DMA_ATTR_SKIP_CPU_SYNC);
+	sg_free_table(sgt);
+	kfree(sgt);
+}
+
 /**
  * amdgpu_vram_mgr_usage - how many bytes are used in this domain
  *
-- 
2.17.1

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

Re: [PATCH 6/6] drm/amdgpu: add support for exporting VRAM using DMA-buf v2

[Alex Deucher]

On Thu, Apr 18, 2019 at 8:09 AM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> We should be able to do this now after checking all the prerequisites.
>
> v2: fix entrie count in the sgt
>
> Signed-off-by: Christian König <christian.koenig@amd.com>

Series is:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c    | 46 ++++++++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h      |  9 ++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 96 ++++++++++++++++++++
>  3 files changed, 142 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
> index a290ae830b11..55bb39281c5d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
> @@ -318,22 +318,45 @@ amdgpu_gem_map_dma_buf(struct dma_buf_attachment *attach,
>         }
>
>         if (attach->invalidate) {
> -               /* move buffer into GTT */
> +               /* move buffer into GTT or VRAM */
>                 struct ttm_operation_ctx ctx = { false, false };
> +               unsigned domains = AMDGPU_GEM_DOMAIN_GTT;
>
> -               amdgpu_bo_placement_from_domain(bo, AMDGPU_GEM_DOMAIN_GTT);
> +               if (bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM &&
> +                   attach->peer2peer) {
> +                       bo->flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
> +                       domains |= AMDGPU_GEM_DOMAIN_VRAM;
> +               }
> +               amdgpu_bo_placement_from_domain(bo, domains);
>                 r = ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);
>                 if (r)
>                         return ERR_PTR(r);
>         }
>
> -       sgt = drm_prime_pages_to_sg(bo->tbo.ttm->pages, bo->tbo.num_pages);
> -       if (IS_ERR(sgt))
> -               return sgt;
> +       switch (bo->tbo.mem.mem_type) {
> +       case TTM_PL_TT:
> +               sgt = drm_prime_pages_to_sg(bo->tbo.ttm->pages,
> +                                           bo->tbo.num_pages);
> +               if (IS_ERR(sgt))
> +                       return sgt;
> +
> +               if (!dma_map_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir,
> +                                     DMA_ATTR_SKIP_CPU_SYNC)) {
> +                       r = -EINVAL;
> +                       goto error_free;
> +               }
> +               break;
>
> -       if (!dma_map_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir,
> -                             DMA_ATTR_SKIP_CPU_SYNC))
> +       case TTM_PL_VRAM:
> +               r = amdgpu_vram_mgr_alloc_sgt(adev, &bo->tbo.mem, attach->dev,
> +                                             dir, &sgt);
> +               if (r)
> +                       goto error_free;
> +               break;
> +       default:
> +               r = -EINVAL;
>                 goto error_free;
> +       }
>
>         if (attach->dev->driver != adev->dev->driver)
>                 bo->prime_shared_count++;
> @@ -343,7 +366,7 @@ amdgpu_gem_map_dma_buf(struct dma_buf_attachment *attach,
>  error_free:
>         sg_free_table(sgt);
>         kfree(sgt);
> -       return ERR_PTR(-ENOMEM);
> +       return ERR_PTR(r);
>  }
>
>  /**
> @@ -367,10 +390,15 @@ static void amdgpu_gem_unmap_dma_buf(struct dma_buf_attachment *attach,
>         if (attach->dev->driver != adev->dev->driver && bo->prime_shared_count)
>                 bo->prime_shared_count--;
>
> -       if (sgt) {
> +       if (!sgt)
> +               return;
> +
> +       if (sgt->sgl->page_link) {
>                 dma_unmap_sg(attach->dev, sgt->sgl, sgt->nents, dir);
>                 sg_free_table(sgt);
>                 kfree(sgt);
> +       } else {
> +               amdgpu_vram_mgr_free_sgt(adev, attach->dev, dir, sgt);
>         }
>  }
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> index c2b7669004ba..0b4cdbe867e7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> @@ -72,6 +72,15 @@ uint64_t amdgpu_gtt_mgr_usage(struct ttm_mem_type_manager *man);
>  int amdgpu_gtt_mgr_recover(struct ttm_mem_type_manager *man);
>
>  u64 amdgpu_vram_mgr_bo_visible_size(struct amdgpu_bo *bo);
> +int amdgpu_vram_mgr_alloc_sgt(struct amdgpu_device *adev,
> +                             struct ttm_mem_reg *mem,
> +                             struct device *dev,
> +                             enum dma_data_direction dir,
> +                             struct sg_table **sgt);
> +void amdgpu_vram_mgr_free_sgt(struct amdgpu_device *adev,
> +                             struct device *dev,
> +                             enum dma_data_direction dir,
> +                             struct sg_table *sgt);
>  uint64_t amdgpu_vram_mgr_usage(struct ttm_mem_type_manager *man);
>  uint64_t amdgpu_vram_mgr_vis_usage(struct ttm_mem_type_manager *man);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> index ec9ea3fdbb4a..520cea4dbdab 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> @@ -399,6 +399,102 @@ static void amdgpu_vram_mgr_del(struct ttm_mem_type_manager *man,
>         mem->mm_node = NULL;
>  }
>
> +/**
> + * amdgpu_vram_mgr_alloc_sgt - allocate and fill a sg table
> + *
> + * @adev: amdgpu device pointer
> + * @mem: TTM memory object
> + * @dev: the other device
> + * @dir: dma direction
> + * @sgt: resulting sg table
> + *
> + * Allocate and fill a sg table from a VRAM allocation.
> + */
> +int amdgpu_vram_mgr_alloc_sgt(struct amdgpu_device *adev,
> +                             struct ttm_mem_reg *mem,
> +                             struct device *dev,
> +                             enum dma_data_direction dir,
> +                             struct sg_table **sgt)
> +{
> +       struct drm_mm_node *node;
> +       struct scatterlist *sg;
> +       int num_entries = 0;
> +       unsigned int pages;
> +       int i, r;
> +
> +       *sgt = kmalloc(sizeof(*sg), GFP_KERNEL);
> +       if (!*sgt)
> +               return -ENOMEM;
> +
> +       for (pages = mem->num_pages, node = mem->mm_node;
> +            pages; pages -= node->size, ++node)
> +               ++num_entries;
> +
> +       r = sg_alloc_table(*sgt, num_entries, GFP_KERNEL);
> +       if (r)
> +               goto error_free;
> +
> +       for_each_sg((*sgt)->sgl, sg, num_entries, i)
> +               sg->length = 0;
> +
> +       node = mem->mm_node;
> +       for_each_sg((*sgt)->sgl, sg, num_entries, i) {
> +               phys_addr_t phys = (node->start << PAGE_SHIFT) +
> +                       adev->gmc.aper_base;
> +               size_t size = node->size << PAGE_SHIFT;
> +               dma_addr_t addr;
> +
> +               ++node;
> +               addr = dma_map_resource(dev, phys, size, dir,
> +                                       DMA_ATTR_SKIP_CPU_SYNC);
> +               r = dma_mapping_error(dev, addr);
> +               if (r)
> +                       goto error_unmap;
> +
> +               sg_set_dma_addr(sg, addr, size, 0);
> +       }
> +       return 0;
> +
> +error_unmap:
> +       for_each_sg((*sgt)->sgl, sg, num_entries, i) {
> +               if (!sg->length)
> +                       continue;
> +
> +               dma_unmap_resource(dev, sg->dma_address,
> +                                  sg->length, dir,
> +                                  DMA_ATTR_SKIP_CPU_SYNC);
> +       }
> +       sg_free_table(*sgt);
> +
> +error_free:
> +       kfree(*sgt);
> +       return r;
> +}
> +
> +/**
> + * amdgpu_vram_mgr_alloc_sgt - allocate and fill a sg table
> + *
> + * @adev: amdgpu device pointer
> + * @sgt: sg table to free
> + *
> + * Free a previously allocate sg table.
> + */
> +void amdgpu_vram_mgr_free_sgt(struct amdgpu_device *adev,
> +                             struct device *dev,
> +                             enum dma_data_direction dir,
> +                             struct sg_table *sgt)
> +{
> +       struct scatterlist *sg;
> +       int i;
> +
> +       for_each_sg(sgt->sgl, sg, sgt->nents, i)
> +               dma_unmap_resource(dev, sg->dma_address,
> +                                  sg->length, dir,
> +                                  DMA_ATTR_SKIP_CPU_SYNC);
> +       sg_free_table(sgt);
> +       kfree(sgt);
> +}
> +
>  /**
>   * amdgpu_vram_mgr_usage - how many bytes are used in this domain
>   *
> --
> 2.17.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 1/6] lib/scatterlist: add sg_set_dma_addr() helper

[Christian König]

Use this function to set an sg entry to point to device resources mapped
using dma_map_resource(). The page pointer is set to NULL and only the DMA
address, length and offset values are valid.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 include/linux/scatterlist.h | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index b96f0d0b5b8f..d72f76768767 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -145,6 +145,29 @@ static inline void sg_set_buf(struct scatterlist *sg, const void *buf,
 	sg_set_page(sg, virt_to_page(buf), buflen, offset_in_page(buf));
 }
 
+/**
+ * sg_set_dma_addr - Set sg entry to point at specified dma address
+ * @sg:		 SG entry
+ * @address:	 DMA address to set
+ * @len:	 Length of data
+ * @offset:	 Offset into page
+ *
+ * Description:
+ *   Use this function to set an sg entry to point to device resources mapped
+ *   using dma_map_resource(). The page pointer is set to NULL and only the DMA
+ *   address, length and offset values are valid.
+ *
+ **/
+static inline void sg_set_dma_addr(struct scatterlist *sg, dma_addr_t address,
+				   unsigned int len, unsigned int offset)
+{
+	sg_set_page(sg, NULL, len, offset);
+	sg->dma_address = address;
+#ifdef CONFIG_NEED_SG_DMA_LENGTH
+	sg->dma_length = len;
+#endif
+}
+
 /*
  * Loop over each sg element, following the pointer to a new list if necessary
  */
-- 
2.17.1

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

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

[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 <christian.koenig@amd.com>
---
 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

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

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

[Alex Deucher]

On Thu, Apr 18, 2019 at 8:09 AM Christian König
<ckoenig.leichtzumerken@gmail.com> 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>
> ---
>  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-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[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
> <ckoenig.leichtzumerken@gmail.com> 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>
>> ---
>>   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-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[PATCH 3/6] dma-buf: add peer2peer flag

[Christian König]

Add a peer2peer flag noting that the importer can deal with device
resources which are not backed by pages.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/dma-buf/dma-buf.c | 1 +
 include/linux/dma-buf.h   | 4 ++++
 2 files changed, 5 insertions(+)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index f23ff8355505..3ab49b492017 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -563,6 +563,7 @@ struct dma_buf_attachment *dma_buf_attach(const struct dma_buf_attach_info *info
 
 	attach->dev = info->dev;
 	attach->dmabuf = dmabuf;
+	attach->peer2peer = info->peer2peer;
 	attach->importer_priv = info->importer_priv;
 	attach->invalidate = info->invalidate;
 
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
index a615b74e5894..d2746bdfe92c 100644
--- a/include/linux/dma-buf.h
+++ b/include/linux/dma-buf.h
@@ -346,6 +346,7 @@ struct dma_buf {
  * @dmabuf: buffer for this attachment.
  * @dev: device attached to the buffer.
  * @node: list of dma_buf_attachment.
+ * @peer2peer: true if the importer can handle peer resources without pages.
  * @priv: exporter specific attachment data.
  * @importer_priv: importer specific attachment data.
  *
@@ -362,6 +363,7 @@ struct dma_buf_attachment {
 	struct dma_buf *dmabuf;
 	struct device *dev;
 	struct list_head node;
+	bool peer2peer;
 	void *priv;
 	struct sg_table *sgt;
 	void *importer_priv;
@@ -427,6 +429,7 @@ struct dma_buf_export_info {
  * struct dma_buf_attach_info - holds information needed to attach to a dma_buf
  * @dmabuf:		the exported dma_buf
  * @dev:		the device which wants to import the attachment
+ * @peer2peer:		true if the importer can handle peer resources without pages
  * @importer_priv:	private data of importer to this attachment
  * @invalidate:		callback to use for invalidating mappings
  *
@@ -436,6 +439,7 @@ struct dma_buf_export_info {
 struct dma_buf_attach_info {
 	struct dma_buf *dmabuf;
 	struct device *dev;
+	bool peer2peer;
 	void *importer_priv;
 	void (*invalidate)(struct dma_buf_attachment *attach);
 };
-- 
2.17.1

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

Re: [PATCH 3/6] dma-buf: add peer2peer flag

[Daniel Vetter]

On Thu, Apr 18, 2019 at 02:09:25PM +0200, Christian König wrote:
> Add a peer2peer flag noting that the importer can deal with device
> resources which are not backed by pages.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>

Should we add something like "Such importers should also support dynamic
dma_buf attachments." to the kerneldoc?

Either way lgtm

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/dma-buf/dma-buf.c | 1 +
>  include/linux/dma-buf.h   | 4 ++++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index f23ff8355505..3ab49b492017 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -563,6 +563,7 @@ struct dma_buf_attachment *dma_buf_attach(const struct dma_buf_attach_info *info
>  
>  	attach->dev = info->dev;
>  	attach->dmabuf = dmabuf;
> +	attach->peer2peer = info->peer2peer;
>  	attach->importer_priv = info->importer_priv;
>  	attach->invalidate = info->invalidate;
>  
> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> index a615b74e5894..d2746bdfe92c 100644
> --- a/include/linux/dma-buf.h
> +++ b/include/linux/dma-buf.h
> @@ -346,6 +346,7 @@ struct dma_buf {
>   * @dmabuf: buffer for this attachment.
>   * @dev: device attached to the buffer.
>   * @node: list of dma_buf_attachment.
> + * @peer2peer: true if the importer can handle peer resources without pages.
>   * @priv: exporter specific attachment data.
>   * @importer_priv: importer specific attachment data.
>   *
> @@ -362,6 +363,7 @@ struct dma_buf_attachment {
>  	struct dma_buf *dmabuf;
>  	struct device *dev;
>  	struct list_head node;
> +	bool peer2peer;
>  	void *priv;
>  	struct sg_table *sgt;
>  	void *importer_priv;
> @@ -427,6 +429,7 @@ struct dma_buf_export_info {
>   * struct dma_buf_attach_info - holds information needed to attach to a dma_buf
>   * @dmabuf:		the exported dma_buf
>   * @dev:		the device which wants to import the attachment
> + * @peer2peer:		true if the importer can handle peer resources without pages
>   * @importer_priv:	private data of importer to this attachment
>   * @invalidate:		callback to use for invalidating mappings
>   *
> @@ -436,6 +439,7 @@ struct dma_buf_export_info {
>  struct dma_buf_attach_info {
>  	struct dma_buf *dmabuf;
>  	struct device *dev;
> +	bool peer2peer;
>  	void *importer_priv;
>  	void (*invalidate)(struct dma_buf_attachment *attach);
>  };
> -- 
> 2.17.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/6] dma-buf: add peer2peer flag

[Daniel Vetter]

On Tue, Apr 23, 2019 at 10:16:50AM +0200, Daniel Vetter wrote:
> On Thu, Apr 18, 2019 at 02:09:25PM +0200, Christian König wrote:
> > Add a peer2peer flag noting that the importer can deal with device
> > resources which are not backed by pages.
> > 
> > Signed-off-by: Christian König <christian.koenig@amd.com>
> 
> Should we add something like "Such importers should also support dynamic
> dma_buf attachments." to the kerneldoc?
> 
> Either way lgtm
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> >  drivers/dma-buf/dma-buf.c | 1 +
> >  include/linux/dma-buf.h   | 4 ++++
> >  2 files changed, 5 insertions(+)
> > 
> > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> > index f23ff8355505..3ab49b492017 100644
> > --- a/drivers/dma-buf/dma-buf.c
> > +++ b/drivers/dma-buf/dma-buf.c
> > @@ -563,6 +563,7 @@ struct dma_buf_attachment *dma_buf_attach(const struct dma_buf_attach_info *info
> >  
> >  	attach->dev = info->dev;
> >  	attach->dmabuf = dmabuf;
> > +	attach->peer2peer = info->peer2peer;
> >  	attach->importer_priv = info->importer_priv;
> >  	attach->invalidate = info->invalidate;
> >  
> > diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> > index a615b74e5894..d2746bdfe92c 100644
> > --- a/include/linux/dma-buf.h
> > +++ b/include/linux/dma-buf.h
> > @@ -346,6 +346,7 @@ struct dma_buf {
> >   * @dmabuf: buffer for this attachment.
> >   * @dev: device attached to the buffer.
> >   * @node: list of dma_buf_attachment.
> > + * @peer2peer: true if the importer can handle peer resources without pages.
> >   * @priv: exporter specific attachment data.
> >   * @importer_priv: importer specific attachment data.
> >   *
> > @@ -362,6 +363,7 @@ struct dma_buf_attachment {
> >  	struct dma_buf *dmabuf;
> >  	struct device *dev;
> >  	struct list_head node;
> > +	bool peer2peer;
> >  	void *priv;
> >  	struct sg_table *sgt;
> >  	void *importer_priv;
> > @@ -427,6 +429,7 @@ struct dma_buf_export_info {
> >   * struct dma_buf_attach_info - holds information needed to attach to a dma_buf
> >   * @dmabuf:		the exported dma_buf
> >   * @dev:		the device which wants to import the attachment
> > + * @peer2peer:		true if the importer can handle peer resources without pages

And a bikeshed: inline kerneldoc style for this would be good (I think I
made that comment on the earlier series too), to avoid too much of a
pressure towards too terse kerneldoc. @invalidate definitely needs a lot
more text, and I think peer2peer would also benefit from a few pointers
(e.g. which sg table helpers you can use and which you can't because the
pages pointer is NULL).
-Daniel

> >   * @importer_priv:	private data of importer to this attachment
> >   * @invalidate:		callback to use for invalidating mappings
> >   *
> > @@ -436,6 +439,7 @@ struct dma_buf_export_info {
> >  struct dma_buf_attach_info {
> >  	struct dma_buf *dmabuf;
> >  	struct device *dev;
> > +	bool peer2peer;
> >  	void *importer_priv;
> >  	void (*invalidate)(struct dma_buf_attachment *attach);
> >  };
> > -- 
> > 2.17.1
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 4/6] drm/amdgpu: note that we can handle peer2peer DMA-buf

[Christian König]

Importing should work out of the box.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
index 30634396719b..af103b7e21e8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
@@ -519,6 +519,7 @@ struct drm_gem_object *amdgpu_gem_prime_import(struct drm_device *dev,
 	if (IS_ERR(obj))
 		return obj;
 
+	attach_info.peer2peer = true;
 	attach_info.importer_priv = obj;
 	attach = dma_buf_attach(&attach_info);
 	if (IS_ERR(attach)) {
-- 
2.17.1

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

[PATCH 5/6] drm/amdgpu: add checks if DMA-buf P2P is supported

[Christian König]

Check if we can do peer2peer on the PCIe bus.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
index af103b7e21e8..a290ae830b11 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
@@ -39,6 +39,7 @@
 #include <drm/amdgpu_drm.h>
 #include <linux/dma-buf.h>
 #include <linux/dma-fence-array.h>
+#include <linux/pci-p2pdma.h>
 
 /**
  * amdgpu_gem_prime_vmap - &dma_buf_ops.vmap implementation
@@ -254,13 +255,27 @@ static int amdgpu_gem_dma_buf_attach(struct dma_buf *dma_buf,
 {
 	struct drm_gem_object *obj = dma_buf->priv;
 	struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
+	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
+
+	if (!attach->peer2peer)
+		goto no_peer2peer;
+
+	if (!dev_is_pci(attach->dev))
+		goto no_peer2peer;
+
+	if (pci_p2pdma_distance_many(adev->pdev, &attach->dev, 1, true) < 0)
+		goto no_peer2peer;
+
+	return 0;
+
+no_peer2peer:
+	attach->peer2peer = false;
 
 	/* Make sure the buffer is pinned when userspace didn't set GTT as
 	 * preferred domain. This avoid ping/pong situations with scan out BOs.
 	 */
 	if (!(bo->preferred_domains & AMDGPU_GEM_DOMAIN_GTT))
 		attach->invalidate = NULL;
-
 	return 0;
 }
 
-- 
2.17.1

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

RE: DMA-buf P2P

[Zhou, David(ChunMing)]

Which test are you using? Can share?

-David

> -----Original Message-----
> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
> Christian K?nig
> Sent: Thursday, April 18, 2019 8:09 PM
> To: dri-devel@lists.freedesktop.org; amd-gfx@lists.freedesktop.org
> Subject: DMA-buf P2P
> 
> Hi guys,
> 
> as promised this is the patch set which enables P2P buffer sharing with DMA-
> buf.
> 
> Basic idea is that importers can set a flag noting that they can deal with and
> sgt which doesn't contains pages.
> 
> This in turn is the signal to the exporter that we don't need to move a buffer
> to system memory any more when a remote device wants to access it.
> 
> Please review and/or comment,
> Christian.
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx