Re: [PATCH 1/2] drm: Implement vm_operations_struct.access

Am 13.07.2017 um 23:08 schrieb Felix Kuehling:

Allows gdb to access contents of user mode mapped BOs. VRAM access requires the driver to implement a new callback in ttm_bo_driver.

One more comment additionally to what Michel already wrote below, apart from that it looks good to me.

Signed-off-by: Felix Kuehling Felix.Kuehling@amd.com

drivers/gpu/drm/ttm/ttm_bo_vm.c | 66 ++++++++++++++++++++++++++++++++++++++++- include/drm/ttm/ttm_bo_driver.h | 12 ++++++++ 2 files changed, 77 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c index 9f53df9..0ef2eb9 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c @@ -294,10 +294,74 @@ static void ttm_bo_vm_close(struct vm_area_struct *vma) vma->vm_private_data = NULL; }

+static int ttm_bo_vm_access_kmap(struct ttm_buffer_object *bo,

• 		 unsigned long offset,
  

• 		 void *buf, int len, int write)
  

+{

• unsigned long first_page = offset >> PAGE_SHIFT;
• unsigned long last_page = (offset + len - 1) >> PAGE_SHIFT;
• unsigned long num_pages = last_page - first_page + 1;
• struct ttm_bo_kmap_obj map;
• void *ptr;
• bool is_iomem;
• int ret;
• ret = ttm_bo_kmap(bo, first_page, num_pages, &map);
• if (ret)

• return ret;
  

• offset -= first_page << PAGE_SHIFT;
• ptr = (uint8_t *)ttm_kmap_obj_virtual(&map, &is_iomem) + offset;
• WARN_ON(is_iomem);
• if (write)

• memcpy(ptr, buf, len);
  

• else

• memcpy(buf, ptr, len);
  

• ttm_bo_kunmap(&map);
• return len;

+}

+static int ttm_bo_vm_access(struct vm_area_struct *vma, unsigned long addr,

• 	    void *buf, int len, int write)
  

+{

• unsigned long offset = (addr) - vma->vm_start;
• struct ttm_buffer_object *bo = vma->vm_private_data;
• int ret;
• if (len < 1 || (offset + len) >> PAGE_SHIFT > bo->num_pages)

• return -EIO;
  

• ret = ttm_bo_reserve(bo, true, false, NULL);
• if (ret)

• return ret;
  

• switch(bo->mem.mem_type) {
• case TTM_PL_SYSTEM:

When the BO is in the system domain you need to add this as well I think:

if (unlikely(bo->ttm->page_flags & TTM_PAGE_FLAG_SWAPPED)) { ret = ttm_tt_swapin(bo->ttm); if (unlikely(ret != 0)) return ret; }

Regards, Christian.

• case TTM_PL_TT:

• ret = ttm_bo_vm_access_kmap(bo, offset, buf, len, write);
  

• break;
  

• case TTM_PL_VRAM:

• if (bo->bdev->driver->access_vram)
  

• 	ret = bo->bdev->driver->access_vram(
  

• 		bo, offset, buf, len, write);
  

• else
  

• 	ret = -EIO;
  

• break;
  

• default:

• ret = -EIO;
  

• }
• ttm_bo_unreserve(bo);
• return ret;

+}

• static const struct vm_operations_struct ttm_bo_vm_ops = { .fault = ttm_bo_vm_fault, .open = ttm_bo_vm_open,

• .close = ttm_bo_vm_close

• .close = ttm_bo_vm_close,

• .access = ttm_bo_vm_access };

  static struct ttm_buffer_object *ttm_bo_vm_lookup(struct ttm_bo_device *bdev,

diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h index 6bbd34d..6ce5094 100644 --- a/include/drm/ttm/ttm_bo_driver.h +++ b/include/drm/ttm/ttm_bo_driver.h @@ -471,6 +471,18 @@ struct ttm_bo_driver { */ unsigned long (*io_mem_pfn)(struct ttm_buffer_object *bo, unsigned long page_offset);

• /**

• * Read/write VRAM buffers for ptrace access
  

• *
  

• * @bo: the BO to access
  

• * @offset: the offset from the start of the BO
  

• * @buf: pointer to source/destination buffer
  

• * @len: number of bytes to copy
  

• * @write: whether to read (0) from or write (non-0) to BO
  

• */
  

• int (*access_vram)(struct ttm_buffer_object *bo, unsigned long offset,

• 	   void *buf, int len, int write);
  

  };

  /**