Re: [PATCH 2/2] drm/amdgpu: Implement ttm_bo_driver.access_vram callback

[Felix Kuehling <felix.kuehling-5C7GfCeVMHo@public.gmane.org>]

On 17-07-14 06:08 AM, Christian König wrote:
> Am 13.07.2017 um 23:08 schrieb Felix Kuehling:
>> Allows gdb to access contents of user mode mapped VRAM BOs.
>>
>> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 59
>> +++++++++++++++++++++++++++++++++
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h |  2 ++
>>   2 files changed, 61 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> index ff5614b..d65551d 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> @@ -1115,6 +1115,64 @@ static bool
>> amdgpu_ttm_bo_eviction_valuable(struct ttm_buffer_object *bo,
>>       return ttm_bo_eviction_valuable(bo, place);
>>   }
>>   +static int amdgpu_ttm_access_vram(struct ttm_buffer_object *bo,
>> +                  unsigned long offset,
>> +                  void *buf, int len, int write)
>> +{
>> +    struct amdgpu_bo *abo = container_of(bo, struct amdgpu_bo, tbo);
>> +    struct amdgpu_device *adev = amdgpu_ttm_adev(abo->tbo.bdev);
>> +    struct drm_mm_node *nodes = abo->tbo.mem.mm_node;
>> +    uint32_t value = 0;
>> +    int result = 0;
>> +    uint64_t pos;
>> +    unsigned long flags;
>> +
>> +    while (offset >= (nodes->size << PAGE_SHIFT)) {
>> +        offset -= nodes->size << PAGE_SHIFT;
>> +        ++nodes;
>> +    }
>> +    pos = (nodes->start << PAGE_SHIFT) + offset;
>
> This silently assumes that a read would never cross a node boundary,
> doesn't it?

It doesn't. See below ...

>
> Christian.
>
>> +
>> +    while (len && pos < adev->mc.mc_vram_size) {
>> +        uint64_t aligned_pos = pos & ~(uint64_t)3;
>> +        uint32_t bytes = 4 - (pos & 3);
>> +        uint32_t shift = (pos & 3) * 8;
>> +        uint32_t mask = 0xffffffff << shift;
>> +
>> +        if (len < bytes) {
>> +            mask &= 0xffffffff >> (bytes - len) * 8;
>> +            bytes = len;
>> +        }
>> +
>> +        spin_lock_irqsave(&adev->mmio_idx_lock, flags);
>> +        WREG32(mmMM_INDEX, ((uint32_t)aligned_pos) | 0x80000000);
>> +        WREG32(mmMM_INDEX_HI, aligned_pos >> 31);
>> +        if (!write || mask != 0xffffffff)
>> +            value = RREG32(mmMM_DATA);
>> +        if (write) {
>> +            value &= ~mask;
>> +            value |= (*(uint32_t *)buf << shift) & mask;
>> +            WREG32(mmMM_DATA, value);
>> +        }
>> +        spin_unlock_irqrestore(&adev->mmio_idx_lock, flags);
>> +        if (!write) {
>> +            value = (value & mask) >> shift;
>> +            memcpy(buf, &value, bytes);
>> +        }
>> +
>> +        result += bytes;
>> +        buf = (uint8_t *)buf + bytes;
>> +        pos += bytes;
>> +        len -= bytes;
>> +        if (pos >= (nodes->start + nodes->size) << PAGE_SHIFT) {
>> +            ++nodes;
>> +            pos = (nodes->start << PAGE_SHIFT);

... Here I handle crossing a node boundary. Yes, I actually added this
case to my kfdtest unit test and made sure it works, along with all odd
alignments that the code above handles.

Regards,
  Felix

>> +        }
>> +    }
>> +
>> +    return result;
>> +}
>> +
>>   static struct ttm_bo_driver amdgpu_bo_driver = {
>>       .ttm_tt_create = &amdgpu_ttm_tt_create,
>>       .ttm_tt_populate = &amdgpu_ttm_tt_populate,
>> @@ -1130,6 +1188,7 @@ static bool
>> amdgpu_ttm_bo_eviction_valuable(struct ttm_buffer_object *bo,
>>       .io_mem_reserve = &amdgpu_ttm_io_mem_reserve,
>>       .io_mem_free = &amdgpu_ttm_io_mem_free,
>>       .io_mem_pfn = amdgpu_ttm_io_mem_pfn,
>> +    .access_vram = &amdgpu_ttm_access_vram
>>   };
>>     int amdgpu_ttm_init(struct amdgpu_device *adev)
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>> index f137c24..a22e430 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>> @@ -78,6 +78,8 @@ int amdgpu_fill_buffer(struct amdgpu_bo *bo,
>>               struct dma_fence **fence);
>>     int amdgpu_mmap(struct file *filp, struct vm_area_struct *vma);
>> +int amdgpu_bo_mmap(struct file *filp, struct vm_area_struct *vma,
>> +           struct ttm_bo_device *bdev);
>>   bool amdgpu_ttm_is_bound(struct ttm_tt *ttm);
>>   int amdgpu_ttm_bind(struct ttm_buffer_object *bo, struct
>> ttm_mem_reg *bo_mem);
>>   int amdgpu_ttm_recover_gart(struct amdgpu_device *adev);
>
>

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