Skip to content

Commit

Permalink
drm/i915/gt: Use i915_vm_put on ppgtt_create error paths
Browse files Browse the repository at this point in the history
commit 20e377e upstream.

Now that the scratch page and page directories have a reference back to
the i915_address_space, we cannot do an immediate free of the ppgtt upon
error as those buffer objects will perform a later i915_vm_put in their
deferred frees.

The downside is that by replacing the onion unwind along the error
paths, the ppgtt cleanup must handle a partially constructed vm. This
includes ensuring that the vm->cleanup is set prior to the error path.

Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/6900
Signed-off-by: Chris Wilson <chris.p.wilson@intel.com>
Fixes: 4d8151a ("drm/i915: Don't free shared locks while shared")
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: <stable@vger.kernel.org> # v5.14+
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220926153333.102195-1-matthew.auld@intel.com
(cherry picked from commit c286558)
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
ickle-intel authored and gregkh committed Oct 21, 2022
1 parent 7d80473 commit 61fb775
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 36 deletions.
16 changes: 8 additions & 8 deletions drivers/gpu/drm/i915/gt/gen6_ppgtt.c
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,7 @@ static int gen6_ppgtt_init_scratch(struct gen6_ppgtt *ppgtt)
i915_gem_object_put(vm->scratch[1]);
err_scratch0:
i915_gem_object_put(vm->scratch[0]);
vm->scratch[0] = NULL;
return ret;
}

Expand All @@ -268,9 +269,10 @@ static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
gen6_ppgtt_free_pd(ppgtt);
free_scratch(vm);

mutex_destroy(&ppgtt->flush);
if (ppgtt->base.pd)
free_pd(&ppgtt->base.vm, ppgtt->base.pd);

free_pd(&ppgtt->base.vm, ppgtt->base.pd);
mutex_destroy(&ppgtt->flush);
}

static void pd_vma_bind(struct i915_address_space *vm,
Expand Down Expand Up @@ -449,19 +451,17 @@ struct i915_ppgtt *gen6_ppgtt_create(struct intel_gt *gt)

err = gen6_ppgtt_init_scratch(ppgtt);
if (err)
goto err_free;
goto err_put;

ppgtt->base.pd = gen6_alloc_top_pd(ppgtt);
if (IS_ERR(ppgtt->base.pd)) {
err = PTR_ERR(ppgtt->base.pd);
goto err_scratch;
goto err_put;
}

return &ppgtt->base;

err_scratch:
free_scratch(&ppgtt->base.vm);
err_free:
kfree(ppgtt);
err_put:
i915_vm_put(&ppgtt->base.vm);
return ERR_PTR(err);
}
58 changes: 30 additions & 28 deletions drivers/gpu/drm/i915/gt/gen8_ppgtt.c
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,10 @@ static void gen8_ppgtt_cleanup(struct i915_address_space *vm)
if (intel_vgpu_active(vm->i915))
gen8_ppgtt_notify_vgt(ppgtt, false);

__gen8_ppgtt_cleanup(vm, ppgtt->pd, gen8_pd_top_count(vm), vm->top);
if (ppgtt->pd)
__gen8_ppgtt_cleanup(vm, ppgtt->pd,
gen8_pd_top_count(vm), vm->top);

free_scratch(vm);
}

Expand Down Expand Up @@ -803,8 +806,10 @@ static int gen8_init_scratch(struct i915_address_space *vm)
struct drm_i915_gem_object *obj;

obj = vm->alloc_pt_dma(vm, I915_GTT_PAGE_SIZE_4K);
if (IS_ERR(obj))
if (IS_ERR(obj)) {
ret = PTR_ERR(obj);
goto free_scratch;
}

ret = map_pt_dma(vm, obj);
if (ret) {
Expand All @@ -823,7 +828,8 @@ static int gen8_init_scratch(struct i915_address_space *vm)
free_scratch:
while (i--)
i915_gem_object_put(vm->scratch[i]);
return -ENOMEM;
vm->scratch[0] = NULL;
return ret;
}

static int gen8_preallocate_top_level_pdp(struct i915_ppgtt *ppgtt)
Expand Down Expand Up @@ -901,6 +907,7 @@ gen8_alloc_top_pd(struct i915_address_space *vm)
struct i915_ppgtt *gen8_ppgtt_create(struct intel_gt *gt,
unsigned long lmem_pt_obj_flags)
{
struct i915_page_directory *pd;
struct i915_ppgtt *ppgtt;
int err;

Expand Down Expand Up @@ -946,21 +953,7 @@ struct i915_ppgtt *gen8_ppgtt_create(struct intel_gt *gt,
ppgtt->vm.alloc_scratch_dma = alloc_pt_dma;
}

err = gen8_init_scratch(&ppgtt->vm);
if (err)
goto err_free;

ppgtt->pd = gen8_alloc_top_pd(&ppgtt->vm);
if (IS_ERR(ppgtt->pd)) {
err = PTR_ERR(ppgtt->pd);
goto err_free_scratch;
}

if (!i915_vm_is_4lvl(&ppgtt->vm)) {
err = gen8_preallocate_top_level_pdp(ppgtt);
if (err)
goto err_free_pd;
}
ppgtt->vm.pte_encode = gen8_pte_encode;

ppgtt->vm.bind_async_flags = I915_VMA_LOCAL_BIND;
ppgtt->vm.insert_entries = gen8_ppgtt_insert;
Expand All @@ -971,22 +964,31 @@ struct i915_ppgtt *gen8_ppgtt_create(struct intel_gt *gt,
ppgtt->vm.allocate_va_range = gen8_ppgtt_alloc;
ppgtt->vm.clear_range = gen8_ppgtt_clear;
ppgtt->vm.foreach = gen8_ppgtt_foreach;
ppgtt->vm.cleanup = gen8_ppgtt_cleanup;

ppgtt->vm.pte_encode = gen8_pte_encode;
err = gen8_init_scratch(&ppgtt->vm);
if (err)
goto err_put;

pd = gen8_alloc_top_pd(&ppgtt->vm);
if (IS_ERR(pd)) {
err = PTR_ERR(pd);
goto err_put;
}
ppgtt->pd = pd;

if (!i915_vm_is_4lvl(&ppgtt->vm)) {
err = gen8_preallocate_top_level_pdp(ppgtt);
if (err)
goto err_put;
}

if (intel_vgpu_active(gt->i915))
gen8_ppgtt_notify_vgt(ppgtt, true);

ppgtt->vm.cleanup = gen8_ppgtt_cleanup;

return ppgtt;

err_free_pd:
__gen8_ppgtt_cleanup(&ppgtt->vm, ppgtt->pd,
gen8_pd_top_count(&ppgtt->vm), ppgtt->vm.top);
err_free_scratch:
free_scratch(&ppgtt->vm);
err_free:
kfree(ppgtt);
err_put:
i915_vm_put(&ppgtt->vm);
return ERR_PTR(err);
}
3 changes: 3 additions & 0 deletions drivers/gpu/drm/i915/gt/intel_gtt.c
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,9 @@ void free_scratch(struct i915_address_space *vm)
{
int i;

if (!vm->scratch[0])
return;

for (i = 0; i <= vm->top; i++)
i915_gem_object_put(vm->scratch[i]);
}
Expand Down

0 comments on commit 61fb775

Please sign in to comment.