[PATCH] drm/amd/amdgpu: add support for iova_to_phys to replace TTM trace (v5)

[Tom St Denis]

On 19/09/17 11:17 AM, Deucher, Alexander wrote:
>> -----Original Message-----
>> From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On Behalf
>> Of Tom St Denis
>> Sent: Tuesday, September 19, 2017 8:13 AM
>> To: amd-gfx at lists.freedesktop.org
>> Cc: StDenis, Tom
>> Subject: [PATCH] drm/amd/amdgpu: add support for iova_to_phys to
>> replace TTM trace (v5)
>>
>> Signed-off-by: Tom St Denis <tom.stdenis at amd.com>
>>
>> (v2): Add domain to iova debugfs
>> (v3): Add true read/write methods to access system memory of pages
>>        mapped to the device
>> (v4): Move get_domain call out of loop and return on error
>> (v5): Just use kmap/kunmap
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 99
>> +++++++++++++++++++++++++++++++++
>>   1 file changed, 99 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> index 50d20903de4f..c7f8e081a772 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> @@ -43,6 +43,7 @@
>>   #include <linux/swap.h>
>>   #include <linux/pagemap.h>
>>   #include <linux/debugfs.h>
>> +#include <linux/iommu.h>
> 
> Just to double check, do you need any compile guards for the IOMMU disabled case?

I honestly don't know.  Is there an easy way to setup a cross build that 
won't automagically enable the iommu in the configuration?

(also, if the iommu team co-operated from the get go...).

Looking at the cavium thunder ethernet source (I grepped for 
iova_to_phys) it seems the return of dev_to_domain might be NULL if 
IOMMU is disabled in which case these read/writes will return -EFAULT...

ideally the iommu_iova_to_phys() would be an identity function if the 
domain is NULL.  Which it doesn't.  It'll OOPS if domain==NULL.

So the fix might be to skip the iova_to_phys call if domain==NULL and 
act on the address as a physical address.

On my carrizo with IOMMU enabled I can read the gfx ring buffer directly 
with --vm-read for both the Carrizo and Polaris10 (the Carrizo doesn't 
do mapping, the Polaris10 does).  I can try a boot with iommu disabled 
and see what happens.

Tom


> 
> Alex
> 
>>   #include "amdgpu.h"
>>   #include "amdgpu_trace.h"
>>   #include "bif/bif_4_1_d.h"
>> @@ -1809,7 +1810,104 @@ 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)
>> +{
>> +	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;
>> +}
>> +
>> +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;
>> +
>> +	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_from_user(ptr, buf, n);
>> +		kunmap(pfn_to_page(PFN_DOWN(phys)));
>> +		if (r)
>> +			return -EFAULT;
>> +
>> +		*pos += n;
>> +		size -= n;
>> +		result += n;
>> +	}
>> +
>> +	return result;
>> +}
>> +
>> +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
>> +};
>>
>>   static const struct {
>>   	char *name;
>> @@ -1820,6 +1918,7 @@ static const struct {
>>   #ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS
>>   	{ "amdgpu_gtt", &amdgpu_ttm_gtt_fops, TTM_PL_TT },
>>   #endif
>> +	{ "amdgpu_iova", &amdgpu_ttm_iova_fops, TTM_PL_SYSTEM },
>>   };
>>
>>   #endif
>> --
>> 2.12.0
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx