drm/i915: drop the __i915_active_call pointer packing

We use some of the lower bits of the retire function pointer for
potential flags, which is quite thorny, since the caller needs to
remember to give the function the correct alignment with
__i915_active_call, otherwise we might incorrectly unpack the pointer
and jump to some garbage address later. Instead of all this let's just
pass the flags along as a separate parameter.

Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Suggested-by: Daniel Vetter <daniel@ffwll.ch>
References: ca419f407b43 ("drm/i915: Fix crash in auto_retire")
References: d8e44e4dd221 ("drm/i915/overlay: Fix active retire callback alignment")
References: fd5f262db118 ("drm/i915/selftests: Fix active retire callback alignment")
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/20210504164136.96456-1-matthew.auld@intel.com

diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.c b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
index 6fc6965b613304304e9589624fad4c4175905e9d..2a25f7bf7e5c148679b909116b8d03445d391ec3 100644 (file)
--- a/drivers/gpu/drm/i915/display/intel_frontbuffer.c
+++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
@@ -206,7 +206,6 @@ static int frontbuffer_active(struct i915_active *ref)
        return 0;
 }
 
-__i915_active_call
 static void frontbuffer_retire(struct i915_active *ref)
 {
        struct intel_frontbuffer *front =
@@ -261,7 +260,8 @@ intel_frontbuffer_get(struct drm_i915_gem_object *obj)
        atomic_set(&front->bits, 0);
        i915_active_init(&front->write,
                         frontbuffer_active,
-                        i915_active_may_sleep(frontbuffer_retire));
+                        frontbuffer_retire,
+                        I915_ACTIVE_RETIRE_SLEEPS);
 
        spin_lock(&i915->fb_tracking.lock);
        if (rcu_access_pointer(obj->frontbuffer)) {

diff --git a/drivers/gpu/drm/i915/display/intel_overlay.c b/drivers/gpu/drm/i915/display/intel_overlay.c
index 8fa4313569a9f0debd2ccb244939bc0d45fe59f7..addc83a92315a0d543d64cf970f2ae1c3389203a 100644 (file)
--- a/drivers/gpu/drm/i915/display/intel_overlay.c
+++ b/drivers/gpu/drm/i915/display/intel_overlay.c
@@ -383,8 +383,7 @@ static void intel_overlay_off_tail(struct intel_overlay *overlay)
                i830_overlay_clock_gating(dev_priv, true);
 }
 
-__i915_active_call static void
-intel_overlay_last_flip_retire(struct i915_active *active)
+static void intel_overlay_last_flip_retire(struct i915_active *active)
 {
        struct intel_overlay *overlay =
                container_of(active, typeof(*overlay), last_flip);
@@ -1399,7 +1398,7 @@ void intel_overlay_setup(struct drm_i915_private *dev_priv)
        overlay->saturation = 146;
 
        i915_active_init(&overlay->last_flip,
-                        NULL, intel_overlay_last_flip_retire);
+                        NULL, intel_overlay_last_flip_retire, 0);
 
        ret = get_registers(overlay, OVERLAY_NEEDS_PHYSICAL(dev_priv));
        if (ret)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index fd8ee52e17a472bc61ba2a20f341125036afb97c..188dee13e017d9b81168079d5667e7ac36a4f641 100644 (file)
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -1046,7 +1046,6 @@ struct context_barrier_task {
        void *data;
 };
 
-__i915_active_call
 static void cb_retire(struct i915_active *base)
 {
        struct context_barrier_task *cb = container_of(base, typeof(*cb), base);
@@ -1080,7 +1079,7 @@ static int context_barrier_task(struct i915_gem_context *ctx,
        if (!cb)
                return -ENOMEM;
 
-       i915_active_init(&cb->base, NULL, cb_retire);
+       i915_active_init(&cb->base, NULL, cb_retire, 0);
        err = i915_active_acquire(&cb->base);
        if (err) {
                kfree(cb);

diff --git a/drivers/gpu/drm/i915/gt/gen6_ppgtt.c b/drivers/gpu/drm/i915/gt/gen6_ppgtt.c
index 21b1085769be28e899c0aaf41b206b9e23a8d929..1aee5e6b1b23f1663f5b78e7c384f6c23ecbc2ea 100644 (file)
--- a/drivers/gpu/drm/i915/gt/gen6_ppgtt.c
+++ b/drivers/gpu/drm/i915/gt/gen6_ppgtt.c
@@ -343,7 +343,7 @@ static struct i915_vma *pd_vma_create(struct gen6_ppgtt *ppgtt, int size)
        if (!vma)
                return ERR_PTR(-ENOMEM);
 
-       i915_active_init(&vma->active, NULL, NULL);
+       i915_active_init(&vma->active, NULL, NULL, 0);
 
        kref_init(&vma->ref);
        mutex_init(&vma->pages_mutex);

diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
index 17cf2640b082b10a62d092bcf83061a73a6e5ac0..4033184f13b9fe9474443dd5f4bf30e3631a8330 100644 (file)
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -326,7 +326,6 @@ void intel_context_unpin(struct intel_context *ce)
        intel_context_put(ce);
 }
 
-__i915_active_call
 static void __intel_context_retire(struct i915_active *active)
 {
        struct intel_context *ce = container_of(active, typeof(*ce), active);
@@ -385,7 +384,7 @@ intel_context_init(struct intel_context *ce, struct intel_engine_cs *engine)
        mutex_init(&ce->pin_mutex);
 
        i915_active_init(&ce->active,
-                        __intel_context_active, __intel_context_retire);
+                        __intel_context_active, __intel_context_retire, 0);
 }
 
 void intel_context_fini(struct intel_context *ce)

diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c
index 0fa6c38893f72e11d4f24f9b79253ab8bb1da20e..7bf84cd21543a4884dd60069c98c3be01f374136 100644 (file)
--- a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c
@@ -867,7 +867,7 @@ void intel_ggtt_init_fences(struct i915_ggtt *ggtt)
        for (i = 0; i < num_fences; i++) {
                struct i915_fence_reg *fence = &ggtt->fence_regs[i];
 
-               i915_active_init(&fence->active, NULL, NULL);
+               i915_active_init(&fence->active, NULL, NULL, 0);
                fence->ggtt = ggtt;
                fence->id = i;
                list_add_tail(&fence->link, &ggtt->fence_list);

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_buffer_pool.c b/drivers/gpu/drm/i915/gt/intel_gt_buffer_pool.c
index c594681075989285eb100df706aee6bec1680190..aa0a59c5b614707cef04e8fcea924f40e9cf9ce8 100644 (file)
--- a/drivers/gpu/drm/i915/gt/intel_gt_buffer_pool.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_buffer_pool.c
@@ -98,7 +98,6 @@ static void pool_free_work(struct work_struct *wrk)
                                      round_jiffies_up_relative(HZ));
 }
 
-__i915_active_call
 static void pool_retire(struct i915_active *ref)
 {
        struct intel_gt_buffer_pool_node *node =
@@ -154,7 +153,7 @@ node_create(struct intel_gt_buffer_pool *pool, size_t sz,
        node->age = 0;
        node->pool = pool;
        node->pinned = false;
-       i915_active_init(&node->active, NULL, pool_retire);
+       i915_active_init(&node->active, NULL, pool_retire, 0);
 
        obj = i915_gem_object_create_internal(gt->i915, sz);
        if (IS_ERR(obj)) {

diff --git a/drivers/gpu/drm/i915/gt/intel_timeline.c b/drivers/gpu/drm/i915/gt/intel_timeline.c
index f19cf6d2fa85cf2d4db57a6e339fcafcb216480d..c4a126c8caef871ab14b2c78ad19d044636c66cc 100644 (file)
--- a/drivers/gpu/drm/i915/gt/intel_timeline.c
+++ b/drivers/gpu/drm/i915/gt/intel_timeline.c
@@ -32,7 +32,6 @@ static struct i915_vma *hwsp_alloc(struct intel_gt *gt)
        return vma;
 }
 
-__i915_active_call
 static void __timeline_retire(struct i915_active *active)
 {
        struct intel_timeline *tl =
@@ -104,7 +103,8 @@ static int intel_timeline_init(struct intel_timeline *timeline,
        INIT_LIST_HEAD(&timeline->requests);
 
        i915_syncmap_init(&timeline->sync);
-       i915_active_init(&timeline->active, __timeline_active, __timeline_retire);
+       i915_active_init(&timeline->active, __timeline_active,
+                        __timeline_retire, 0);
 
        return 0;
 }

diff --git a/drivers/gpu/drm/i915/gt/mock_engine.c b/drivers/gpu/drm/i915/gt/mock_engine.c
index e1ba03b93ffa0fa9107296eace077d6f1b30ca00..32589c6625e16019356f8c4e85c9ea077511913d 100644 (file)
--- a/drivers/gpu/drm/i915/gt/mock_engine.c
+++ b/drivers/gpu/drm/i915/gt/mock_engine.c
@@ -55,7 +55,7 @@ static struct intel_ring *mock_ring(struct intel_engine_cs *engine)
                kfree(ring);
                return NULL;
        }
-       i915_active_init(&ring->vma->active, NULL, NULL);
+       i915_active_init(&ring->vma->active, NULL, NULL, 0);
        __set_bit(I915_VMA_GGTT_BIT, __i915_vma_flags(ring->vma));
        __set_bit(DRM_MM_NODE_ALLOCATED_BIT, &ring->vma->node.flags);
        ring->vma->node.size = sz;

diff --git a/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c b/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
index fcde223e26ffd7a7cdfb8141c51ec222b109a31a..4896e4ccad50eea0d63d3194e605f02a278097b9 100644 (file)
--- a/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
+++ b/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
@@ -63,7 +63,7 @@ static void pulse_put(struct pulse *p)
        kref_put(&p->kref, pulse_free);
 }
 
-__i915_active_call static void pulse_retire(struct i915_active *active)
+static void pulse_retire(struct i915_active *active)
 {
        pulse_put(container_of(active, struct pulse, active));
 }
@@ -77,7 +77,7 @@ static struct pulse *pulse_create(void)
                return p;
 
        kref_init(&p->kref);
-       i915_active_init(&p->active, pulse_active, pulse_retire);
+       i915_active_init(&p->active, pulse_active, pulse_retire, 0);
 
        return p;
 }

diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
index aa573b078ae75c83d9e81c60f54579a66c1dfe2a..b1aa1c482c32264f43bdd9ad7f855dc7b434ba7e 100644 (file)
--- a/drivers/gpu/drm/i915/i915_active.c
+++ b/drivers/gpu/drm/i915/i915_active.c
@@ -343,18 +343,15 @@ out:
 void __i915_active_init(struct i915_active *ref,
                        int (*active)(struct i915_active *ref),
                        void (*retire)(struct i915_active *ref),
+                       unsigned long flags,
                        struct lock_class_key *mkey,
                        struct lock_class_key *wkey)
 {
-       unsigned long bits;
-
        debug_active_init(ref);
 
-       ref->flags = 0;
+       ref->flags = flags;
        ref->active = active;
-       ref->retire = ptr_unpack_bits(retire, &bits, 2);
-       if (bits & I915_ACTIVE_MAY_SLEEP)
-               ref->flags |= I915_ACTIVE_RETIRE_SLEEPS;
+       ref->retire = retire;
 
        spin_lock_init(&ref->tree_lock);
        ref->tree = RB_ROOT;
@@ -1156,8 +1153,7 @@ static int auto_active(struct i915_active *ref)
        return 0;
 }
 
-__i915_active_call static void
-auto_retire(struct i915_active *ref)
+static void auto_retire(struct i915_active *ref)
 {
        i915_active_put(ref);
 }
@@ -1171,7 +1167,7 @@ struct i915_active *i915_active_create(void)
                return NULL;
 
        kref_init(&aa->ref);
-       i915_active_init(&aa->base, auto_active, auto_retire);
+       i915_active_init(&aa->base, auto_active, auto_retire, 0);
 
        return &aa->base;
 }

diff --git a/drivers/gpu/drm/i915/i915_active.h b/drivers/gpu/drm/i915/i915_active.h
index fb165d3f01cf378ed8fbcde6181682199fdc68f8..d0feda68b874f9516d8997c67c2570e3be66495f 100644 (file)
--- a/drivers/gpu/drm/i915/i915_active.h
+++ b/drivers/gpu/drm/i915/i915_active.h
@@ -152,15 +152,16 @@ i915_active_fence_isset(const struct i915_active_fence *active)
 void __i915_active_init(struct i915_active *ref,
                        int (*active)(struct i915_active *ref),
                        void (*retire)(struct i915_active *ref),
+                       unsigned long flags,
                        struct lock_class_key *mkey,
                        struct lock_class_key *wkey);
 
 /* Specialise each class of i915_active to avoid impossible lockdep cycles. */
-#define i915_active_init(ref, active, retire) do {             \
-       static struct lock_class_key __mkey;                            \
-       static struct lock_class_key __wkey;                            \
-                                                                       \
-       __i915_active_init(ref, active, retire, &__mkey, &__wkey);      \
+#define i915_active_init(ref, active, retire, flags) do {                      \
+       static struct lock_class_key __mkey;                                    \
+       static struct lock_class_key __wkey;                                    \
+                                                                               \
+       __i915_active_init(ref, active, retire, flags, &__mkey, &__wkey);       \
 } while (0)
 
 struct dma_fence *

diff --git a/drivers/gpu/drm/i915/i915_active_types.h b/drivers/gpu/drm/i915/i915_active_types.h
index 6360c3e4b7657b0b9a8795469aa845c6c0494b12..c149f348a972ea38ce8f3bcde0a1a58f9b9acb31 100644 (file)
--- a/drivers/gpu/drm/i915/i915_active_types.h
+++ b/drivers/gpu/drm/i915/i915_active_types.h
@@ -24,11 +24,6 @@ struct i915_active_fence {
 
 struct active_node;
 
-#define I915_ACTIVE_MAY_SLEEP BIT(0)
-
-#define __i915_active_call __aligned(4)
-#define i915_active_may_sleep(fn) ptr_pack_bits(&(fn), I915_ACTIVE_MAY_SLEEP, 2)
-
 struct i915_active {
        atomic_t count;
        struct mutex mutex;

diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 468317e3b4778c9d84d51ae1afdbcbb224a95404..a6cd0fa628477cf4e09ab70a2c8fa7ce50e14893 100644 (file)
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -94,7 +94,6 @@ static int __i915_vma_active(struct i915_active *ref)
        return i915_vma_tryget(active_to_vma(ref)) ? 0 : -ENOENT;
 }
 
-__i915_active_call
 static void __i915_vma_retire(struct i915_active *ref)
 {
        i915_vma_put(active_to_vma(ref));
@@ -125,7 +124,7 @@ vma_create(struct drm_i915_gem_object *obj,
        vma->size = obj->base.size;
        vma->display_alignment = I915_GTT_MIN_ALIGNMENT;
 
-       i915_active_init(&vma->active, __i915_vma_active, __i915_vma_retire);
+       i915_active_init(&vma->active, __i915_vma_active, __i915_vma_retire, 0);
 
        /* Declare ourselves safe for use inside shrinkers */
        if (IS_ENABLED(CONFIG_LOCKDEP)) {

diff --git a/drivers/gpu/drm/i915/selftests/i915_active.c b/drivers/gpu/drm/i915/selftests/i915_active.c
index 1aa52b5cc4886898e37ae47f3a13daad204c679f..61bf4560d8af3a278a3973c755c6ce9163d89ea9 100644 (file)
--- a/drivers/gpu/drm/i915/selftests/i915_active.c
+++ b/drivers/gpu/drm/i915/selftests/i915_active.c
@@ -51,7 +51,7 @@ static int __live_active(struct i915_active *base)
        return 0;
 }
 
-__i915_active_call static void __live_retire(struct i915_active *base)
+static void __live_retire(struct i915_active *base)
 {
        struct live_active *active = container_of(base, typeof(*active), base);
 
@@ -68,7 +68,7 @@ static struct live_active *__live_alloc(struct drm_i915_private *i915)
                return NULL;
 
        kref_init(&active->ref);
-       i915_active_init(&active->base, __live_active, __live_retire);
+       i915_active_init(&active->base, __live_active, __live_retire, 0);
 
        return active;
 }