Am 03.10.19 um 21:44 schrieb Yang, Philip:
user_pages array should always be freed after validation regardless if
user pages are changed after bo is created because with HMM change parse
bo always allocate user pages array to get user pages for userptr bo.
Don't need to get user pages while creating uerptr bo because user pages
will only be used while validating after parsing userptr bo.
Bugzilla: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1844962
v2: remove unused local variable and amend commit
Signed-off-by: Philip Yang <Philip.Yang@xxxxxxx>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 4 +---
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 23 +----------------------
2 files changed, 2 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 49b767b7238f..961186e7113e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -474,7 +474,6 @@ static int amdgpu_cs_list_validate(struct amdgpu_cs_parser *p,
list_for_each_entry(lobj, validated, tv.head) {
struct amdgpu_bo *bo = ttm_to_amdgpu_bo(lobj->tv.bo);
- bool binding_userptr = false;
struct mm_struct *usermm;
usermm = amdgpu_ttm_tt_get_usermm(bo->tbo.ttm);
@@ -491,14 +490,13 @@ static int amdgpu_cs_list_validate(struct amdgpu_cs_parser *p,
amdgpu_ttm_tt_set_user_pages(bo->tbo.ttm,
lobj->user_pages);
- binding_userptr = true;
}
r = amdgpu_cs_validate(p, bo);
if (r)
return r;
- if (binding_userptr) {
+ if (lobj->user_pages) {
kvfree(lobj->user_pages);
lobj->user_pages = NULL;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index a828e3d0bfbd..3ccd61d69964 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -283,7 +283,6 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void *data,
int amdgpu_gem_userptr_ioctl(struct drm_device *dev, void *data,
struct drm_file *filp)
{
- struct ttm_operation_ctx ctx = { true, false };
struct amdgpu_device *adev = dev->dev_private;
struct drm_amdgpu_gem_userptr *args = data;
struct drm_gem_object *gobj;
@@ -326,32 +325,12 @@ int amdgpu_gem_userptr_ioctl(struct drm_device *dev, void *data,
goto release_object;
}
- if (args->flags & AMDGPU_GEM_USERPTR_VALIDATE) {
We can't drop that handling here, it is mandatory to detect an
application bug where an application tries to user an userptr with a VMA
which is not anonymous memory.
This must be detected and rejected as invalid here.
I suggest that we allocate a local pages array similar to how we do it
during CS and release that after the function is done.
Regards,
Christian.
- r = amdgpu_ttm_tt_get_user_pages(bo, bo->tbo.ttm->pages);
- if (r)
- goto release_object;
-
- r = amdgpu_bo_reserve(bo, true);
- if (r)
- goto user_pages_done;
-
- amdgpu_bo_placement_from_domain(bo, AMDGPU_GEM_DOMAIN_GTT);
- r = ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);
- amdgpu_bo_unreserve(bo);
- if (r)
- goto user_pages_done;
- }
-
r = drm_gem_handle_create(filp, gobj, &handle);
if (r)
- goto user_pages_done;
+ goto release_object;
args->handle = handle;
-user_pages_done:
- if (args->flags & AMDGPU_GEM_USERPTR_VALIDATE)
- amdgpu_ttm_tt_get_user_pages_done(bo->tbo.ttm);
-
release_object:
drm_gem_object_put_unlocked(gobj);
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
