diff mbox series

[v4,2/3] Manually revert "drm/i915: Fix a VMA UAF for multi-gt platform"

Message ID 20240122141007.401490-7-janusz.krzysztofik@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: Fix VMA UAF on destroy against deactivate race | expand

Commit Message

Janusz Krzysztofik Jan. 22, 2024, 2:04 p.m. UTC 
This reverts changes introduced by commit f56fe3e91787, obsoleted by
"drm/i915/vma: Fix UAF on destroy against retire race".

Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 17 -----------------
 1 file changed, 17 deletions(-)

Comments

Rodrigo Vivi Jan. 22, 2024, 9:13 p.m. UTC | #1 
On Mon, Jan 22, 2024 at 03:04:43PM +0100, Janusz Krzysztofik wrote:
> This reverts changes introduced by commit f56fe3e91787, obsoleted by
> "drm/i915/vma: Fix UAF on destroy against retire race".

I believe the good chunk of the first commit message should be moved
here instead.

But why did you do a 'manually revert'? revert itself didn't apply?
maybe you could avoid the word 'revert' and use something like
Remove extra multi-gt pm-references... or something like that.

> 
> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 17 -----------------
>  1 file changed, 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> index d3a771afb083e..cedca8fd8754d 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> @@ -2686,7 +2686,6 @@ static int
>  eb_select_engine(struct i915_execbuffer *eb)
>  {
>  	struct intel_context *ce, *child;
> -	struct intel_gt *gt;
>  	unsigned int idx;
>  	int err;
>  
> @@ -2710,17 +2709,10 @@ eb_select_engine(struct i915_execbuffer *eb)
>  		}
>  	}
>  	eb->num_batches = ce->parallel.number_children + 1;
> -	gt = ce->engine->gt;
>  
>  	for_each_child(ce, child)
>  		intel_context_get(child);
>  	eb->wakeref = intel_gt_pm_get(ce->engine->gt);
> -	/*
> -	 * Keep GT0 active on MTL so that i915_vma_parked() doesn't
> -	 * free VMAs while execbuf ioctl is validating VMAs.
> -	 */
> -	if (gt->info.id)
> -		eb->wakeref_gt0 = intel_gt_pm_get(to_gt(gt->i915));
>  
>  	if (!test_bit(CONTEXT_ALLOC_BIT, &ce->flags)) {
>  		err = intel_context_alloc_state(ce);
> @@ -2759,9 +2751,6 @@ eb_select_engine(struct i915_execbuffer *eb)
>  	return err;
>  
>  err:
> -	if (gt->info.id)
> -		intel_gt_pm_put(to_gt(gt->i915), eb->wakeref_gt0);
> -
>  	intel_gt_pm_put(ce->engine->gt, eb->wakeref);
>  	for_each_child(ce, child)
>  		intel_context_put(child);
> @@ -2775,12 +2764,6 @@ eb_put_engine(struct i915_execbuffer *eb)
>  	struct intel_context *child;
>  
>  	i915_vm_put(eb->context->vm);
> -	/*
> -	 * This works in conjunction with eb_select_engine() to prevent
> -	 * i915_vma_parked() from interfering while execbuf validates vmas.
> -	 */
> -	if (eb->gt->info.id)
> -		intel_gt_pm_put(to_gt(eb->gt->i915), eb->wakeref_gt0);
>  	intel_gt_pm_put(eb->context->engine->gt, eb->wakeref);
>  	for_each_child(eb->context, child)
>  		intel_context_put(child);
> -- 
> 2.43.0
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index d3a771afb083e..cedca8fd8754d 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -2686,7 +2686,6 @@  static int
 eb_select_engine(struct i915_execbuffer *eb)
 {
 	struct intel_context *ce, *child;
-	struct intel_gt *gt;
 	unsigned int idx;
 	int err;
 
@@ -2710,17 +2709,10 @@  eb_select_engine(struct i915_execbuffer *eb)
 		}
 	}
 	eb->num_batches = ce->parallel.number_children + 1;
-	gt = ce->engine->gt;
 
 	for_each_child(ce, child)
 		intel_context_get(child);
 	eb->wakeref = intel_gt_pm_get(ce->engine->gt);
-	/*
-	 * Keep GT0 active on MTL so that i915_vma_parked() doesn't
-	 * free VMAs while execbuf ioctl is validating VMAs.
-	 */
-	if (gt->info.id)
-		eb->wakeref_gt0 = intel_gt_pm_get(to_gt(gt->i915));
 
 	if (!test_bit(CONTEXT_ALLOC_BIT, &ce->flags)) {
 		err = intel_context_alloc_state(ce);
@@ -2759,9 +2751,6 @@  eb_select_engine(struct i915_execbuffer *eb)
 	return err;
 
 err:
-	if (gt->info.id)
-		intel_gt_pm_put(to_gt(gt->i915), eb->wakeref_gt0);
-
 	intel_gt_pm_put(ce->engine->gt, eb->wakeref);
 	for_each_child(ce, child)
 		intel_context_put(child);
@@ -2775,12 +2764,6 @@  eb_put_engine(struct i915_execbuffer *eb)
 	struct intel_context *child;
 
 	i915_vm_put(eb->context->vm);
-	/*
-	 * This works in conjunction with eb_select_engine() to prevent
-	 * i915_vma_parked() from interfering while execbuf validates vmas.
-	 */
-	if (eb->gt->info.id)
-		intel_gt_pm_put(to_gt(eb->gt->i915), eb->wakeref_gt0);
 	intel_gt_pm_put(eb->context->engine->gt, eb->wakeref);
 	for_each_child(eb->context, child)
 		intel_context_put(child);