Skip to content

Commit 1f33dc0

Browse files
jkrzyszt-intelAndi Shyti
authored andcommitted
drm/i915: Remove extra multi-gt pm-references
There was an attempt to fix an issue of illegal attempts to free a still active i915 VMA object when parking a GT believed to be idle, reported by CI on 2-GT Meteor Lake. As a solution, an extra wakeref for a Primary GT was acquired from i915_gem_do_execbuffer() -- see commit f56fe3e ("drm/i915: Fix a VMA UAF for multi-gt platform"). However, that fix occurred insufficient -- the issue was still reported by CI. That wakeref was released on exit from i915_gem_do_execbuffer(), then potentially before completion of the request and deactivation of its associated VMAs. Moreover, CI reports indicated that single-GT platforms also suffered sporadically from the same race. Since the issue has now been fixed by a preceding patch "drm/i915/vma: Fix UAF on destroy against retire race", drop the no longer useful changes introduced by that insufficient fix. v3: Also drop the no longer used .wakeref_gt0 field from struct i915_execbuffer. v2: Avoid the word "revert" in commit message (Rodrigo), - update commit description reusing relevant chunks dropped from the description of the proper fix (Rodrigo). Signed-off-by: Janusz Krzysztofik <[email protected]> Cc: Nirmoy Das <[email protected]> Cc: Rodrigo Vivi <[email protected]> Reviewed-by: Nirmoy Das <[email protected]> Signed-off-by: Andi Shyti <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
1 parent f3c71b2 commit 1f33dc0

File tree

1 file changed

+0
-18
lines changed

1 file changed

+0
-18
lines changed

drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,6 @@ struct i915_execbuffer {
254254
struct intel_context *context; /* logical state for the request */
255255
struct i915_gem_context *gem_context; /** caller's context */
256256
intel_wakeref_t wakeref;
257-
intel_wakeref_t wakeref_gt0;
258257

259258
/** our requests to build */
260259
struct i915_request *requests[MAX_ENGINE_INSTANCE + 1];
@@ -2685,7 +2684,6 @@ static int
26852684
eb_select_engine(struct i915_execbuffer *eb)
26862685
{
26872686
struct intel_context *ce, *child;
2688-
struct intel_gt *gt;
26892687
unsigned int idx;
26902688
int err;
26912689

@@ -2709,17 +2707,10 @@ eb_select_engine(struct i915_execbuffer *eb)
27092707
}
27102708
}
27112709
eb->num_batches = ce->parallel.number_children + 1;
2712-
gt = ce->engine->gt;
27132710

27142711
for_each_child(ce, child)
27152712
intel_context_get(child);
27162713
eb->wakeref = intel_gt_pm_get(ce->engine->gt);
2717-
/*
2718-
* Keep GT0 active on MTL so that i915_vma_parked() doesn't
2719-
* free VMAs while execbuf ioctl is validating VMAs.
2720-
*/
2721-
if (gt->info.id)
2722-
eb->wakeref_gt0 = intel_gt_pm_get(to_gt(gt->i915));
27232714

27242715
if (!test_bit(CONTEXT_ALLOC_BIT, &ce->flags)) {
27252716
err = intel_context_alloc_state(ce);
@@ -2758,9 +2749,6 @@ eb_select_engine(struct i915_execbuffer *eb)
27582749
return err;
27592750

27602751
err:
2761-
if (gt->info.id)
2762-
intel_gt_pm_put(to_gt(gt->i915), eb->wakeref_gt0);
2763-
27642752
intel_gt_pm_put(ce->engine->gt, eb->wakeref);
27652753
for_each_child(ce, child)
27662754
intel_context_put(child);
@@ -2774,12 +2762,6 @@ eb_put_engine(struct i915_execbuffer *eb)
27742762
struct intel_context *child;
27752763

27762764
i915_vm_put(eb->context->vm);
2777-
/*
2778-
* This works in conjunction with eb_select_engine() to prevent
2779-
* i915_vma_parked() from interfering while execbuf validates vmas.
2780-
*/
2781-
if (eb->gt->info.id)
2782-
intel_gt_pm_put(to_gt(eb->gt->i915), eb->wakeref_gt0);
27832765
intel_gt_pm_put(eb->context->engine->gt, eb->wakeref);
27842766
for_each_child(eb->context, child)
27852767
intel_context_put(child);

0 commit comments

Comments
 (0)