Skip to contentSkip to conversation

drm/amd/amdgpu: Partial revert of iova debugfs

PATCH In review Tom St Denis 2 participants 2 messages Sep 20, 2017 1 message not in the archive

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 drm/amd/amdgpu: Partial revert of iova debugfs

We discovered that on some devices even with iommu enabled you can access all of system memory through the iommu translation.

Therefore, we revert the read method to the translation only service and drop the write method completely.

---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 90 +++++----------------------------
 1 file changed, 13 insertions(+), 77 deletions(-)
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +13 -77
index 0e5f78f3a97e..ce435dbbb398 100644
@@ -1784,98 +1784,34 @@ static ssize_t amdgpu_iova_to_phys_read(struct file *f, char __user *buf,
 				   size_t size, loff_t *pos)
 {
 	struct amdgpu_device *adev = file_inode(f)->i_private;
-	ssize_t result, n;
 	int r;
 	uint64_t phys;
-	void *ptr;
 	struct iommu_domain *dom;
 
-	dom = iommu_get_domain_for_dev(adev->dev);
-	if (!dom)
-		return -EFAULT;
-
-	result = 0;
-	while (size) {
-		// get physical address and map
-		phys = iommu_iova_to_phys(dom, *pos);
-
-		// copy upto one page
-		if (size > PAGE_SIZE)
-			n = PAGE_SIZE;
-		else
-			n = size;
-
-		// to end of the page
-		if (((*pos & (PAGE_SIZE - 1)) + n) >= PAGE_SIZE)
-			n = PAGE_SIZE - (*pos & (PAGE_SIZE - 1));
-
-		ptr = kmap(pfn_to_page(PFN_DOWN(phys)));
-		if (!ptr)
-			return -EFAULT;
-
-		r = copy_to_user(buf, ptr, n);
-		kunmap(pfn_to_page(PFN_DOWN(phys)));
-		if (r)
-			return -EFAULT;
-
-		*pos += n;
-		size -= n;
-		result += n;
-	}
-
-	return result;
-}
+	// always return 8 bytes
+	if (size != 8)
+		return -EINVAL;
 
-static ssize_t amdgpu_iova_to_phys_write(struct file *f, const char __user *buf,
-				   size_t size, loff_t *pos)
-{
-	struct amdgpu_device *adev = file_inode(f)->i_private;
-	ssize_t result, n;
-	int r;
-	uint64_t phys;
-	void *ptr;
-	struct iommu_domain *dom;
+	// only accept page addresses
+	if (*pos & 0xFFF)
+		return -EINVAL;
 
 	dom = iommu_get_domain_for_dev(adev->dev);
-	if (!dom)
-		return -EFAULT;
-
-	result = 0;
-	while (size) {
-		// get physical address and map
+	if (dom)
 		phys = iommu_iova_to_phys(dom, *pos);
+	else
+		phys = *pos;
 
-		// copy upto one page
-		if (size > PAGE_SIZE)
-			n = PAGE_SIZE;
-		else
-			n = size;
-
-		// to end of the page
-		if (((*pos & (PAGE_SIZE - 1)) + n) >= PAGE_SIZE)
-			n = PAGE_SIZE - (*pos & (PAGE_SIZE - 1));
-
-		ptr = kmap(pfn_to_page(PFN_DOWN(phys)));
-		if (!ptr)
-			return -EFAULT;
-
-		r = copy_from_user(ptr, buf, n);
-		kunmap(pfn_to_page(PFN_DOWN(phys)));
-		if (r)
-			return -EFAULT;
-
-		*pos += n;
-		size -= n;
-		result += n;
-	}
+	r = copy_to_user(buf, &phys, 8);
+	if (r)
+		return -EFAULT;
 
-	return result;
+	return 8;
 }
 
 static const struct file_operations amdgpu_ttm_iova_fops = {
 	.owner = THIS_MODULE,
 	.read = amdgpu_iova_to_phys_read,
-	.write = amdgpu_iova_to_phys_write,
 	.llseek = default_llseek
 };
 
Signature · 6 lines
2.12.0

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

A message that is not in the archive (sent off-list or not yet received): [email protected]

Reviewed-by Christan König
#2 lore raw

Am 20.09.2017 um 13:42 schrieb Tom St Denis:

Quoted 7 lines

We discovered that on some devices even with iommu enabled you can access all of system memory through the iommu translation.

Therefore, we revert the read method to the translation only service and drop the write method completely.

Quoted 110 lines
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 90 +++++----------------------------
  1 file changed, 13 insertions(+), 77 deletions(-)
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +12 -76
index 0e5f78f3a97e..ce435dbbb398 100644
@@ -1784,98 +1784,34 @@ static ssize_t amdgpu_iova_to_phys_read(struct file *f, char __user *buf,
  				   size_t size, loff_t *pos)
  {
  	struct amdgpu_device *adev = file_inode(f)->i_private;
-	ssize_t result, n;
  	int r;
  	uint64_t phys;
-	void *ptr;
  	struct iommu_domain *dom;
 
-	dom = iommu_get_domain_for_dev(adev->dev);
-	if (!dom)
-		return -EFAULT;
-
-	result = 0;
-	while (size) {
-		// get physical address and map
-		phys = iommu_iova_to_phys(dom, *pos);
-
-		// copy upto one page
-		if (size > PAGE_SIZE)
-			n = PAGE_SIZE;
-		else
-			n = size;
-
-		// to end of the page
-		if (((*pos & (PAGE_SIZE - 1)) + n) >= PAGE_SIZE)
-			n = PAGE_SIZE - (*pos & (PAGE_SIZE - 1));
-
-		ptr = kmap(pfn_to_page(PFN_DOWN(phys)));
-		if (!ptr)
-			return -EFAULT;
-
-		r = copy_to_user(buf, ptr, n);
-		kunmap(pfn_to_page(PFN_DOWN(phys)));
-		if (r)
-			return -EFAULT;
-
-		*pos += n;
-		size -= n;
-		result += n;
-	}
-
-	return result;
-}
+	// always return 8 bytes
+	if (size != 8)
+		return -EINVAL;
 
-static ssize_t amdgpu_iova_to_phys_write(struct file *f, const char __user *buf,
-				   size_t size, loff_t *pos)
-{
-	struct amdgpu_device *adev = file_inode(f)->i_private;
-	ssize_t result, n;
-	int r;
-	uint64_t phys;
-	void *ptr;
-	struct iommu_domain *dom;
+	// only accept page addresses
+	if (*pos & 0xFFF)
+		return -EINVAL;
 
  	dom = iommu_get_domain_for_dev(adev->dev);
-	if (!dom)
-		return -EFAULT;
-
-	result = 0;
-	while (size) {
-		// get physical address and map
+	if (dom)
  		phys = iommu_iova_to_phys(dom, *pos);
+	else
+		phys = *pos;
 
-		// copy upto one page
-		if (size > PAGE_SIZE)
-			n = PAGE_SIZE;
-		else
-			n = size;
-
-		// to end of the page
-		if (((*pos & (PAGE_SIZE - 1)) + n) >= PAGE_SIZE)
-			n = PAGE_SIZE - (*pos & (PAGE_SIZE - 1));
-
-		ptr = kmap(pfn_to_page(PFN_DOWN(phys)));
-		if (!ptr)
-			return -EFAULT;
-
-		r = copy_from_user(ptr, buf, n);
-		kunmap(pfn_to_page(PFN_DOWN(phys)));
-		if (r)
-			return -EFAULT;
-
-		*pos += n;
-		size -= n;
-		result += n;
-	}
+	r = copy_to_user(buf, &phys, 8);
+	if (r)
+		return -EFAULT;
 
-	return result;

drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c static ssize_t amdgpu_iova_to_phys_read(struct file *f, char __user *buf, · line 1817

Quoted 10 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +1 -1
+	return 8;
  }
 
  static const struct file_operations amdgpu_ttm_iova_fops = {
  	.owner = THIS_MODULE,
  	.read = amdgpu_iova_to_phys_read,
-	.write = amdgpu_iova_to_phys_write,
  	.llseek = default_llseek
  };
 

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx