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

[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 <Hongbo.He at amd.com>
>> Patch 5:  Acked-by: Roger He <Hongbo.He at amd.com>
>>
>> -----Original Message-----
>> From: dri-devel [mailto:dri-devel-bounces at lists.freedesktop.org] On 
>> Behalf Of Christian K?nig
>> Sent: Saturday, February 03, 2018 3:10 AM
>> To: amd-gfx at lists.freedesktop.org; dri-devel at 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 <christian.koenig at amd.com>
>> ---
>>   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, &phys, 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", &amdgpu_ttm_gtt_fops, TTM_PL_TT }, #endif
>> -    { "amdgpu_iova", &amdgpu_ttm_iova_fops, TTM_PL_SYSTEM },
>> +    { "amdgpu_iomem", &amdgpu_ttm_iomem_fops, TTM_PL_SYSTEM },
>>   };
>>     #endif
>> -- 
>> 2.14.1
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>
>