subject:"Re\: \[PATCH 5\/5\] drm\/amdgpu\: replace iova debugfs file with iomem"

Re: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

2018-02-09 Thread Tom St Denis


On 09/02/18 09:56 AM, Christian König wrote:

Am 09.02.2018 um 15:51 schrieb Tom St Denis:

On 09/02/18 09:12 AM, Christian König wrote:
No, there is simply no need to initialize the system domain. What are 
the values of p->mapping and adev->mman.bdev.dev_mapping when they 
don't match? Maybe we are allocating memory before initializing 
adev->mman.bdev.dev_mapping.


In my test setup I'm running test 3 from libdrm (suite 1) with a pause 
before the unmap/free call.  So the IB should still be mapped.  Indeed 
the VM PTE decoding has the V bit set.


Or do you have more than one GPU in the system? E.g. APU+dGPU? Could 
it be that you read through the wrong device?


I found the issue:

while (size) {
    phys_addr_t addr = *pos & PAGE_MASK;
    loff_t off = *pos & ~PAGE_MASK;
    size_t bytes = PAGE_SIZE - off;

"bytes" should be limited by the 'size' parameter passed in.  What is 
happening instead is it's reading the entire PTB until it hits a V=0 
page and then returns an error and in the process is doing "fun 
things" to the user mode application (by copying more data than I 
asked for).


Ah, obvious problem.

Do you want to fix it or should I take a look? You wanted to add write 
support as well anyway IIRC.


Yup, I can tackle this this afternoon.

I'll take your read only patch and make it do both read/write (and fix 
the minor error).


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

Re: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

2018-02-09 Thread Christian König


Am 09.02.2018 um 15:51 schrieb Tom St Denis:

On 09/02/18 09:12 AM, Christian König wrote:
No, there is simply no need to initialize the system domain. What are 
the values of p->mapping and adev->mman.bdev.dev_mapping when they 
don't match? Maybe we are allocating memory before initializing 
adev->mman.bdev.dev_mapping.


In my test setup I'm running test 3 from libdrm (suite 1) with a pause 
before the unmap/free call.  So the IB should still be mapped.  Indeed 
the VM PTE decoding has the V bit set.


Or do you have more than one GPU in the system? E.g. APU+dGPU? Could 
it be that you read through the wrong device?


I found the issue:

while (size) {
    phys_addr_t addr = *pos & PAGE_MASK;
    loff_t off = *pos & ~PAGE_MASK;
    size_t bytes = PAGE_SIZE - off;

"bytes" should be limited by the 'size' parameter passed in.  What is 
happening instead is it's reading the entire PTB until it hits a V=0 
page and then returns an error and in the process is doing "fun 
things" to the user mode application (by copying more data than I 
asked for).


Ah, obvious problem.

Do you want to fix it or should I take a look? You wanted to add write 
support as well anyway IIRC.


I've just pushed the first three patches from that series to 
amd-staging-drm-next.


Thanks for testing,
Christian.




Tom


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

Re: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

2018-02-09 Thread Tom St Denis


On 09/02/18 09:12 AM, Christian König wrote:
No, there is simply no need to initialize the system domain. What are 
the values of p->mapping and adev->mman.bdev.dev_mapping when they don't 
match? Maybe we are allocating memory before initializing 
adev->mman.bdev.dev_mapping.


In my test setup I'm running test 3 from libdrm (suite 1) with a pause 
before the unmap/free call.  So the IB should still be mapped.  Indeed 
the VM PTE decoding has the V bit set.


Or do you have more than one GPU in the system? E.g. APU+dGPU? Could it 
be that you read through the wrong device?


I found the issue:

while (size) {
phys_addr_t addr = *pos & PAGE_MASK;
loff_t off = *pos & ~PAGE_MASK;
size_t bytes = PAGE_SIZE - off;

"bytes" should be limited by the 'size' parameter passed in.  What is 
happening instead is it's reading the entire PTB until it hits a V=0 
page and then returns an error and in the process is doing "fun things" 
to the user mode application (by copying more data than I asked for).



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

Re: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

2018-02-09 Thread Christian König


Am 09.02.2018 um 15:02 schrieb Tom St Denis:

On 09/02/18 08:56 AM, Christian König wrote:

Am 09.02.2018 um 14:32 schrieb Tom St Denis:

On 02/02/18 02:09 PM, Christian König wrote:

[SNIP]
+    if (p->mapping != adev->mman.bdev.dev_mapping)
+    return -EPERM;


This comparison fails for both IOMMU and non-IOMMU devices in my 
carrizo+polaris10 box.


The address being read from is what the VM decodes to (checked with 
strace).


Have you applied the whole series? That patches before this one are 
necessary to initialize p->mapping when there isn't any userspace 
mapping for the page.



Yes, I have the entire 5 pages applied to a temp branch based on the 
tip of drm-next


$ git log --oneline HEAD~10..
405bc1dc85db (HEAD -> iomem) wip
a06d7a6f29e4 drm/amdgpu: replace iova debugfs file with iomem
d324c21f2c5e drm/ttm: set page mapping during allocation
9f440ee91c58 drm/radeon: remove extra TT unpopulated check
f55d505b0387 drm/amdgpu: remove extra TT unpopulated check
37d705119ea8 drm/ttm: add ttm_tt_populate wrapper
53af6035d04b (origin/amd-staging-drm-next, amd-staging-drm-next) 
drm/radeon: only enable swiotlb path when need v2


(the wip is me adding printks to see which error path is taken).

I don't see an init call for adev->mman.bdev.man[TTM_PL_SYSTEM] 
anywhere.  Maybe that's related?


No, there is simply no need to initialize the system domain. What are 
the values of p->mapping and adev->mman.bdev.dev_mapping when they don't 
match? Maybe we are allocating memory before initializing 
adev->mman.bdev.dev_mapping.


Or do you have more than one GPU in the system? E.g. APU+dGPU? Could it 
be that you read through the wrong device?


Christian.



Tom


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

Re: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

2018-02-09 Thread Tom St Denis


On 09/02/18 08:56 AM, Christian König wrote:

Am 09.02.2018 um 14:32 schrieb Tom St Denis:

On 02/02/18 02:09 PM, Christian König wrote:

[SNIP]
+    if (p->mapping != adev->mman.bdev.dev_mapping)
+    return -EPERM;


This comparison fails for both IOMMU and non-IOMMU devices in my 
carrizo+polaris10 box.


The address being read from is what the VM decodes to (checked with 
strace).


Have you applied the whole series? That patches before this one are 
necessary to initialize p->mapping when there isn't any userspace 
mapping for the page.



Yes, I have the entire 5 pages applied to a temp branch based on the tip 
of drm-next


$ git log --oneline HEAD~10..
405bc1dc85db (HEAD -> iomem) wip
a06d7a6f29e4 drm/amdgpu: replace iova debugfs file with iomem
d324c21f2c5e drm/ttm: set page mapping during allocation
9f440ee91c58 drm/radeon: remove extra TT unpopulated check
f55d505b0387 drm/amdgpu: remove extra TT unpopulated check
37d705119ea8 drm/ttm: add ttm_tt_populate wrapper
53af6035d04b (origin/amd-staging-drm-next, amd-staging-drm-next) 
drm/radeon: only enable swiotlb path when need v2


(the wip is me adding printks to see which error path is taken).

I don't see an init call for adev->mman.bdev.man[TTM_PL_SYSTEM] 
anywhere.  Maybe that's related?


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

Re: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

2018-02-09 Thread Christian König


Am 09.02.2018 um 14:32 schrieb Tom St Denis:

On 02/02/18 02:09 PM, Christian König wrote:

[SNIP]
+    if (p->mapping != adev->mman.bdev.dev_mapping)
+    return -EPERM;


This comparison fails for both IOMMU and non-IOMMU devices in my 
carrizo+polaris10 box.


The address being read from is what the VM decodes to (checked with 
strace).


Have you applied the whole series? That patches before this one are 
necessary to initialize p->mapping when there isn't any userspace 
mapping for the page.


Christian.



Tom


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

Re: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

2018-02-09 Thread Tom St Denis


On 02/02/18 02:09 PM, Christian König wrote:

This allows access to pages allocated through the driver with optional
IOMMU mapping.

Signed-off-by: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 57 -
  1 file changed, 35 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 648c449aaa79..795ceaeb82d5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1929,38 +1929,51 @@ static const struct file_operations amdgpu_ttm_gtt_fops 
= {
  
  #endif
  
-static ssize_t amdgpu_iova_to_phys_read(struct file *f, char __user *buf,

-  size_t size, loff_t *pos)
+static ssize_t amdgpu_iomem_read(struct file *f, char __user *buf,
+size_t size, loff_t *pos)
  {
struct amdgpu_device *adev = file_inode(f)->i_private;
-   int r;
-   uint64_t phys;
struct iommu_domain *dom;
+   ssize_t result = 0;
+   int r;
  
-	// always return 8 bytes

-   if (size != 8)
-   return -EINVAL;
+   dom = iommu_get_domain_for_dev(adev->dev);
  
-	// only accept page addresses

-   if (*pos & 0xFFF)
-   return -EINVAL;
+   while (size) {
+   phys_addr_t addr = *pos & PAGE_MASK;
+   loff_t off = *pos & ~PAGE_MASK;
+   size_t bytes = PAGE_SIZE - off;
+   unsigned long pfn;
+   struct page *p;
+   void *ptr;
  
-	dom = iommu_get_domain_for_dev(adev->dev);

-   if (dom)
-   phys = iommu_iova_to_phys(dom, *pos);
-   else
-   phys = *pos;
+   addr = dom ? iommu_iova_to_phys(dom, addr) : addr;
  
-	r = copy_to_user(buf, , 8);

-   if (r)
-   return -EFAULT;
+   pfn = addr >> PAGE_SHIFT;
+   if (!pfn_valid(pfn))
+   return -EPERM;
+
+   p = pfn_to_page(pfn);
+   if (p->mapping != adev->mman.bdev.dev_mapping)
+   return -EPERM;


This comparison fails for both IOMMU and non-IOMMU devices in my 
carrizo+polaris10 box.


The address being read from is what the VM decodes to (checked with strace).

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

Re: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

2018-02-05 Thread Tom St Denis

Attached is a patch for umr{master} which should in theory support both 
iova and iomem.


I can add the write method if you want since ya it should be fairly 
simple to copy/pasta that up.


Cheers,
Tom

On 05/02/18 07:07 AM, Christian König wrote:

Well adding write support is trivial.

What I'm more concerned about is if setting page->mapping during 
allocation of the page could have any negative effect?


I of hand don't see any since the page isn't reclaimable directly 
anyway, but I'm not 100% sure of that.


Christian.

Am 05.02.2018 um 12:49 schrieb Tom St Denis:
Another thing that occurred to me is this will break write access to 
GPU bound memory.  Previously we relied on iova to translate the 
address and then /dev/mem or /dev/fmem to read/write it.  But since 
this is literally a read only method obviously there's no write support.


Tom


On 04/02/18 09:56 PM, He, Roger wrote:

Patch1 & 2 & 4,   Reviewed-by: Roger He 
Patch 5:  Acked-by: Roger He 

-Original Message-
From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On 
Behalf Of Christian K?nig

Sent: Saturday, February 03, 2018 3:10 AM
To: amd-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org
Subject: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

This allows access to pages allocated through the driver with 
optional IOMMU mapping.


Signed-off-by: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 57 
-

  1 file changed, 35 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c

index 648c449aaa79..795ceaeb82d5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1929,38 +1929,51 @@ static const struct file_operations 
amdgpu_ttm_gtt_fops = {

    #endif
  -static ssize_t amdgpu_iova_to_phys_read(struct file *f, char 
__user *buf,

-   size_t size, loff_t *pos)
+static ssize_t amdgpu_iomem_read(struct file *f, char __user *buf,
+ size_t size, loff_t *pos)
  {
  struct amdgpu_device *adev = file_inode(f)->i_private;
-    int r;
-    uint64_t phys;
  struct iommu_domain *dom;
+    ssize_t result = 0;
+    int r;
  -    // always return 8 bytes
-    if (size != 8)
-    return -EINVAL;
+    dom = iommu_get_domain_for_dev(adev->dev);
  -    // only accept page addresses
-    if (*pos & 0xFFF)
-    return -EINVAL;
+    while (size) {
+    phys_addr_t addr = *pos & PAGE_MASK;
+    loff_t off = *pos & ~PAGE_MASK;
+    size_t bytes = PAGE_SIZE - off;
+    unsigned long pfn;
+    struct page *p;
+    void *ptr;
  -    dom = iommu_get_domain_for_dev(adev->dev);
-    if (dom)
-    phys = iommu_iova_to_phys(dom, *pos);
-    else
-    phys = *pos;
+    addr = dom ? iommu_iova_to_phys(dom, addr) : addr;
  -    r = copy_to_user(buf, , 8);
-    if (r)
-    return -EFAULT;
+    pfn = addr >> PAGE_SHIFT;
+    if (!pfn_valid(pfn))
+    return -EPERM;
+
+    p = pfn_to_page(pfn);
+    if (p->mapping != adev->mman.bdev.dev_mapping)
+    return -EPERM;
+
+    ptr = kmap(p);
+    r = copy_to_user(buf, ptr, bytes);
+    kunmap(p);
+    if (r)
+    return -EFAULT;
  -    return 8;
+    size -= bytes;
+    *pos += bytes;
+    result += bytes;
+    }
+
+    return result;
  }
  -static const struct file_operations amdgpu_ttm_iova_fops = {
+static const struct file_operations amdgpu_ttm_iomem_fops = {
  .owner = THIS_MODULE,
-    .read = amdgpu_iova_to_phys_read,
+    .read = amdgpu_iomem_read,
  .llseek = default_llseek
  };
  @@ -1973,7 +1986,7 @@ static const struct {  #ifdef 
CONFIG_DRM_AMDGPU_GART_DEBUGFS

  { "amdgpu_gtt", _ttm_gtt_fops, TTM_PL_TT }, #endif
-    { "amdgpu_iova", _ttm_iova_fops, TTM_PL_SYSTEM },
+    { "amdgpu_iomem", _ttm_iomem_fops, TTM_PL_SYSTEM },
  };
    #endif
--
2.14.1

___
dri-devel mailing list
dri-de...@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







>From 67703a62763dfb2107bd503c5ae76414a50c50a4 Mon Sep 17 00:00:00 2001
From: Tom St Denis 
Date: Mon, 5 Feb 2018 08:53:40 -0500
Subject: [PATCH umr] add support for new iomem debugfs entry

Signed-off-by: Tom St Denis 
---
 src/lib/close_asic.c |  1 +
 src/lib/discover.c   |  3 +++
 src/lib/read_vram.c  | 29 +++--
 src/umr.h|  3 ++-
 4 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/src/lib/close_asic.c b/src/lib/close_asic.c
index 782b1a0d029b..6b220cd578e9 100644
--- a/src/lib/close_asic.c
+++ b/src/lib/close_asic.c
@@

Re: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

2018-02-05 Thread Christian König


Well adding write support is trivial.

What I'm more concerned about is if setting page->mapping during 
allocation of the page could have any negative effect?


I of hand don't see any since the page isn't reclaimable directly 
anyway, but I'm not 100% sure of that.


Christian.

Am 05.02.2018 um 12:49 schrieb Tom St Denis:
Another thing that occurred to me is this will break write access to 
GPU bound memory.  Previously we relied on iova to translate the 
address and then /dev/mem or /dev/fmem to read/write it.  But since 
this is literally a read only method obviously there's no write support.


Tom


On 04/02/18 09:56 PM, He, Roger wrote:

Patch1 & 2 & 4,   Reviewed-by: Roger He 
Patch 5:  Acked-by: Roger He 

-Original Message-
From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On 
Behalf Of Christian K?nig

Sent: Saturday, February 03, 2018 3:10 AM
To: amd-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org
Subject: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

This allows access to pages allocated through the driver with 
optional IOMMU mapping.


Signed-off-by: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 57 
-

  1 file changed, 35 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c

index 648c449aaa79..795ceaeb82d5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1929,38 +1929,51 @@ static const struct file_operations 
amdgpu_ttm_gtt_fops = {

    #endif
  -static ssize_t amdgpu_iova_to_phys_read(struct file *f, char 
__user *buf,

-   size_t size, loff_t *pos)
+static ssize_t amdgpu_iomem_read(struct file *f, char __user *buf,
+ size_t size, loff_t *pos)
  {
  struct amdgpu_device *adev = file_inode(f)->i_private;
-    int r;
-    uint64_t phys;
  struct iommu_domain *dom;
+    ssize_t result = 0;
+    int r;
  -    // always return 8 bytes
-    if (size != 8)
-    return -EINVAL;
+    dom = iommu_get_domain_for_dev(adev->dev);
  -    // only accept page addresses
-    if (*pos & 0xFFF)
-    return -EINVAL;
+    while (size) {
+    phys_addr_t addr = *pos & PAGE_MASK;
+    loff_t off = *pos & ~PAGE_MASK;
+    size_t bytes = PAGE_SIZE - off;
+    unsigned long pfn;
+    struct page *p;
+    void *ptr;
  -    dom = iommu_get_domain_for_dev(adev->dev);
-    if (dom)
-    phys = iommu_iova_to_phys(dom, *pos);
-    else
-    phys = *pos;
+    addr = dom ? iommu_iova_to_phys(dom, addr) : addr;
  -    r = copy_to_user(buf, , 8);
-    if (r)
-    return -EFAULT;
+    pfn = addr >> PAGE_SHIFT;
+    if (!pfn_valid(pfn))
+    return -EPERM;
+
+    p = pfn_to_page(pfn);
+    if (p->mapping != adev->mman.bdev.dev_mapping)
+    return -EPERM;
+
+    ptr = kmap(p);
+    r = copy_to_user(buf, ptr, bytes);
+    kunmap(p);
+    if (r)
+    return -EFAULT;
  -    return 8;
+    size -= bytes;
+    *pos += bytes;
+    result += bytes;
+    }
+
+    return result;
  }
  -static const struct file_operations amdgpu_ttm_iova_fops = {
+static const struct file_operations amdgpu_ttm_iomem_fops = {
  .owner = THIS_MODULE,
-    .read = amdgpu_iova_to_phys_read,
+    .read = amdgpu_iomem_read,
  .llseek = default_llseek
  };
  @@ -1973,7 +1986,7 @@ static const struct {  #ifdef 
CONFIG_DRM_AMDGPU_GART_DEBUGFS

  { "amdgpu_gtt", _ttm_gtt_fops, TTM_PL_TT }, #endif
-    { "amdgpu_iova", _ttm_iova_fops, TTM_PL_SYSTEM },
+    { "amdgpu_iomem", _ttm_iomem_fops, TTM_PL_SYSTEM },
  };
    #endif
--
2.14.1

___
dri-devel mailing list
dri-de...@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





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

Re: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

2018-02-05 Thread Tom St Denis

Another thing that occurred to me is this will break write access to GPU 
bound memory.  Previously we relied on iova to translate the address and 
then /dev/mem or /dev/fmem to read/write it.  But since this is 
literally a read only method obviously there's no write support.


Tom


On 04/02/18 09:56 PM, He, Roger wrote:

Patch1 & 2 & 4,   Reviewed-by: Roger He 
Patch 5:  Acked-by: Roger He 

-Original Message-
From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf Of 
Christian K?nig
Sent: Saturday, February 03, 2018 3:10 AM
To: amd-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org
Subject: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

This allows access to pages allocated through the driver with optional IOMMU 
mapping.

Signed-off-by: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 57 -
  1 file changed, 35 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 648c449aaa79..795ceaeb82d5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1929,38 +1929,51 @@ static const struct file_operations amdgpu_ttm_gtt_fops 
= {
  
  #endif
  
-static ssize_t amdgpu_iova_to_phys_read(struct file *f, char __user *buf,

-  size_t size, loff_t *pos)
+static ssize_t amdgpu_iomem_read(struct file *f, char __user *buf,
+size_t size, loff_t *pos)
  {
struct amdgpu_device *adev = file_inode(f)->i_private;
-   int r;
-   uint64_t phys;
struct iommu_domain *dom;
+   ssize_t result = 0;
+   int r;
  
-	// always return 8 bytes

-   if (size != 8)
-   return -EINVAL;
+   dom = iommu_get_domain_for_dev(adev->dev);
  
-	// only accept page addresses

-   if (*pos & 0xFFF)
-   return -EINVAL;
+   while (size) {
+   phys_addr_t addr = *pos & PAGE_MASK;
+   loff_t off = *pos & ~PAGE_MASK;
+   size_t bytes = PAGE_SIZE - off;
+   unsigned long pfn;
+   struct page *p;
+   void *ptr;
  
-	dom = iommu_get_domain_for_dev(adev->dev);

-   if (dom)
-   phys = iommu_iova_to_phys(dom, *pos);
-   else
-   phys = *pos;
+   addr = dom ? iommu_iova_to_phys(dom, addr) : addr;
  
-	r = copy_to_user(buf, , 8);

-   if (r)
-   return -EFAULT;
+   pfn = addr >> PAGE_SHIFT;
+   if (!pfn_valid(pfn))
+   return -EPERM;
+
+   p = pfn_to_page(pfn);
+   if (p->mapping != adev->mman.bdev.dev_mapping)
+   return -EPERM;
+
+   ptr = kmap(p);
+   r = copy_to_user(buf, ptr, bytes);
+   kunmap(p);
+   if (r)
+   return -EFAULT;
  
-	return 8;

+   size -= bytes;
+   *pos += bytes;
+   result += bytes;
+   }
+
+   return result;
  }
  
-static const struct file_operations amdgpu_ttm_iova_fops = {

+static const struct file_operations amdgpu_ttm_iomem_fops = {
.owner = THIS_MODULE,
-   .read = amdgpu_iova_to_phys_read,
+   .read = amdgpu_iomem_read,
.llseek = default_llseek
  };
  
@@ -1973,7 +1986,7 @@ static const struct {  #ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS

{ "amdgpu_gtt", _ttm_gtt_fops, TTM_PL_TT },  #endif
-   { "amdgpu_iova", _ttm_iova_fops, TTM_PL_SYSTEM },
+   { "amdgpu_iomem", _ttm_iomem_fops, TTM_PL_SYSTEM },
  };
  
  #endif

--
2.14.1

___
dri-devel mailing list
dri-de...@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



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

RE: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

2018-02-04 Thread He, Roger

Patch1 & 2 & 4,   Reviewed-by: Roger He 
Patch 5:  Acked-by: Roger He 

-Original Message-
From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf Of 
Christian K?nig
Sent: Saturday, February 03, 2018 3:10 AM
To: amd-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org
Subject: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

This allows access to pages allocated through the driver with optional IOMMU 
mapping.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 57 -
 1 file changed, 35 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 648c449aaa79..795ceaeb82d5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1929,38 +1929,51 @@ static const struct file_operations amdgpu_ttm_gtt_fops 
= {
 
 #endif
 
-static ssize_t amdgpu_iova_to_phys_read(struct file *f, char __user *buf,
-  size_t size, loff_t *pos)
+static ssize_t amdgpu_iomem_read(struct file *f, char __user *buf,
+size_t size, loff_t *pos)
 {
struct amdgpu_device *adev = file_inode(f)->i_private;
-   int r;
-   uint64_t phys;
struct iommu_domain *dom;
+   ssize_t result = 0;
+   int r;
 
-   // always return 8 bytes
-   if (size != 8)
-   return -EINVAL;
+   dom = iommu_get_domain_for_dev(adev->dev);
 
-   // only accept page addresses
-   if (*pos & 0xFFF)
-   return -EINVAL;
+   while (size) {
+   phys_addr_t addr = *pos & PAGE_MASK;
+   loff_t off = *pos & ~PAGE_MASK;
+   size_t bytes = PAGE_SIZE - off;
+   unsigned long pfn;
+   struct page *p;
+   void *ptr;
 
-   dom = iommu_get_domain_for_dev(adev->dev);
-   if (dom)
-   phys = iommu_iova_to_phys(dom, *pos);
-   else
-   phys = *pos;
+   addr = dom ? iommu_iova_to_phys(dom, addr) : addr;
 
-   r = copy_to_user(buf, , 8);
-   if (r)
-   return -EFAULT;
+   pfn = addr >> PAGE_SHIFT;
+   if (!pfn_valid(pfn))
+   return -EPERM;
+
+   p = pfn_to_page(pfn);
+   if (p->mapping != adev->mman.bdev.dev_mapping)
+   return -EPERM;
+
+   ptr = kmap(p);
+   r = copy_to_user(buf, ptr, bytes);
+   kunmap(p);
+   if (r)
+   return -EFAULT;
 
-   return 8;
+   size -= bytes;
+   *pos += bytes;
+   result += bytes;
+   }
+
+   return result;
 }
 
-static const struct file_operations amdgpu_ttm_iova_fops = {
+static const struct file_operations amdgpu_ttm_iomem_fops = {
.owner = THIS_MODULE,
-   .read = amdgpu_iova_to_phys_read,
+   .read = amdgpu_iomem_read,
.llseek = default_llseek
 };
 
@@ -1973,7 +1986,7 @@ static const struct {  #ifdef 
CONFIG_DRM_AMDGPU_GART_DEBUGFS
{ "amdgpu_gtt", _ttm_gtt_fops, TTM_PL_TT },  #endif
-   { "amdgpu_iova", _ttm_iova_fops, TTM_PL_SYSTEM },
+   { "amdgpu_iomem", _ttm_iomem_fops, TTM_PL_SYSTEM },
 };
 
 #endif
--
2.14.1

___
dri-devel mailing list
dri-de...@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

Re: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

2018-02-02 Thread StDenis, Tom

I haven't tried the patch but just like to point out this breaks umr :-)  I'll 
have to craft something on Monday to support this and iova in parallel until 
the iova kernels are realistically EOL'ed.

On the other hand I support this idea since it eliminates the need for an fmem 
hack.  So much appreciated.

Cheers,
Tom


From: amd-gfx  on behalf of Christian 
König 
Sent: Friday, February 2, 2018 14:09
To: amd-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org
Subject: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

This allows access to pages allocated through the driver with optional
IOMMU mapping.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 57 -
 1 file changed, 35 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 648c449aaa79..795ceaeb82d5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1929,38 +1929,51 @@ static const struct file_operations amdgpu_ttm_gtt_fops 
= {

 #endif

-static ssize_t amdgpu_iova_to_phys_read(struct file *f, char __user *buf,
-  size_t size, loff_t *pos)
+static ssize_t amdgpu_iomem_read(struct file *f, char __user *buf,
+size_t size, loff_t *pos)
 {
struct amdgpu_device *adev = file_inode(f)->i_private;
-   int r;
-   uint64_t phys;
struct iommu_domain *dom;
+   ssize_t result = 0;
+   int r;

-   // always return 8 bytes
-   if (size != 8)
-   return -EINVAL;
+   dom = iommu_get_domain_for_dev(adev->dev);

-   // only accept page addresses
-   if (*pos & 0xFFF)
-   return -EINVAL;
+   while (size) {
+   phys_addr_t addr = *pos & PAGE_MASK;
+   loff_t off = *pos & ~PAGE_MASK;
+   size_t bytes = PAGE_SIZE - off;
+   unsigned long pfn;
+   struct page *p;
+   void *ptr;

-   dom = iommu_get_domain_for_dev(adev->dev);
-   if (dom)
-   phys = iommu_iova_to_phys(dom, *pos);
-   else
-   phys = *pos;
+   addr = dom ? iommu_iova_to_phys(dom, addr) : addr;

-   r = copy_to_user(buf, , 8);
-   if (r)
-   return -EFAULT;
+   pfn = addr >> PAGE_SHIFT;
+   if (!pfn_valid(pfn))
+   return -EPERM;
+
+   p = pfn_to_page(pfn);
+   if (p->mapping != adev->mman.bdev.dev_mapping)
+   return -EPERM;
+
+   ptr = kmap(p);
+   r = copy_to_user(buf, ptr, bytes);
+   kunmap(p);
+   if (r)
+   return -EFAULT;

-   return 8;
+   size -= bytes;
+   *pos += bytes;
+   result += bytes;
+   }
+
+   return result;
 }

-static const struct file_operations amdgpu_ttm_iova_fops = {
+static const struct file_operations amdgpu_ttm_iomem_fops = {
.owner = THIS_MODULE,
-   .read = amdgpu_iova_to_phys_read,
+   .read = amdgpu_iomem_read,
.llseek = default_llseek
 };

@@ -1973,7 +1986,7 @@ static const struct {
 #ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS
{ "amdgpu_gtt", _ttm_gtt_fops, TTM_PL_TT },
 #endif
-   { "amdgpu_iova", _ttm_iova_fops, TTM_PL_SYSTEM },
+   { "amdgpu_iomem", _ttm_iomem_fops, TTM_PL_SYSTEM },
 };

 #endif
--
2.14.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 5/5] drm/amdgpu: replace iova debugfs file with iomem

Re: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

Re: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

Re: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

Re: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

Re: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

Re: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

Re: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

Re: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

Re: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

RE: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

Re: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

12 matches

Site Navigation

Mail list logo

Footer information