drm/i915: drop the __i915_active_call pointer packing

matt-auld · matt-auld · commit c3b147604f27 · 2021-05-05T11:36:23.000+01:00
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: ca419f4 ("drm/i915: Fix crash in auto_retire") References: d8e44e4 ("drm/i915/overlay: Fix active retire callback alignment") References: fd5f262 ("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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -343,18 +343,15 @@ active_instance(struct i915_active *ref, u64 idx)
 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
@@ -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
@@ -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
@@ -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
@@ -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;
 }

Original file line number	Diff line number	Diff line change
`@@ -326,7 +326,6 @@ void intel_context_unpin(struct intel_context *ce)`
`326`	`326`	`intel_context_put(ce);`
`327`	`327`	`}`
`328`	`328`
`329`		`-__i915_active_call`
`330`	`329`	`static void __intel_context_retire(struct i915_active *active)`
`331`	`330`	`{`
`332`	`331`	`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)`
`385`	`384`	`mutex_init(&ce->pin_mutex);`
`386`	`385`
`387`	`386`	`i915_active_init(&ce->active,`
`388`		`- __intel_context_active, __intel_context_retire);`
	`387`	`+ __intel_context_active, __intel_context_retire, 0);`
`389`	`388`	`}`
`390`	`389`
`391`	`390`	`void intel_context_fini(struct intel_context *ce)`
Original file line number	Diff line number	Diff line change
`@@ -98,7 +98,6 @@ static void pool_free_work(struct work_struct *wrk)`
`98`	`98`	`round_jiffies_up_relative(HZ));`
`99`	`99`	`}`
`100`	`100`
`101`		`-__i915_active_call`
`102`	`101`	`static void pool_retire(struct i915_active *ref)`
`103`	`102`	`{`
`104`	`103`	`struct intel_gt_buffer_pool_node *node =`
`@@ -154,7 +153,7 @@ node_create(struct intel_gt_buffer_pool *pool, size_t sz,`
`154`	`153`	`node->age = 0;`
`155`	`154`	`node->pool = pool;`
`156`	`155`	`node->pinned = false;`
`157`		`- i915_active_init(&node->active, NULL, pool_retire);`
	`156`	`+ i915_active_init(&node->active, NULL, pool_retire, 0);`
`158`	`157`
`159`	`158`	`obj = i915_gem_object_create_internal(gt->i915, sz);`
`160`	`159`	`if (IS_ERR(obj)) {`
Original file line number	Diff line number	Diff line change
`@@ -32,7 +32,6 @@ static struct i915_vma hwsp_alloc(struct intel_gt gt)`
`32`	`32`	`return vma;`
`33`	`33`	`}`
`34`	`34`
`35`		`-__i915_active_call`
`36`	`35`	`static void __timeline_retire(struct i915_active *active)`
`37`	`36`	`{`
`38`	`37`	`struct intel_timeline *tl =`
`@@ -104,7 +103,8 @@ static int intel_timeline_init(struct intel_timeline *timeline,`
`104`	`103`	`INIT_LIST_HEAD(&timeline->requests);`
`105`	`104`
`106`	`105`	`i915_syncmap_init(&timeline->sync);`
`107`		`- i915_active_init(&timeline->active, __timeline_active, __timeline_retire);`
	`106`	`+ i915_active_init(&timeline->active, __timeline_active,`
	`107`	`+ __timeline_retire, 0);`
`108`	`108`
`109`	`109`	`return 0;`
`110`	`110`	`}`
Original file line number	Diff line number	Diff line change
`@@ -55,7 +55,7 @@ static struct intel_ring mock_ring(struct intel_engine_cs engine)`
`55`	`55`	`kfree(ring);`
`56`	`56`	`return NULL;`
`57`	`57`	`}`
`58`		`- i915_active_init(&ring->vma->active, NULL, NULL);`
	`58`	`+ i915_active_init(&ring->vma->active, NULL, NULL, 0);`
`59`	`59`	`__set_bit(I915_VMA_GGTT_BIT, __i915_vma_flags(ring->vma));`
`60`	`60`	`__set_bit(DRM_MM_NODE_ALLOCATED_BIT, &ring->vma->node.flags);`
`61`	`61`	`ring->vma->node.size = sz;`
Original file line number	Diff line number	Diff line change
`@@ -63,7 +63,7 @@ static void pulse_put(struct pulse *p)`
`63`	`63`	`kref_put(&p->kref, pulse_free);`
`64`	`64`	`}`
`65`	`65`
`66`		`-__i915_active_call static void pulse_retire(struct i915_active *active)`
	`66`	`+static void pulse_retire(struct i915_active *active)`
`67`	`67`	`{`
`68`	`68`	`pulse_put(container_of(active, struct pulse, active));`
`69`	`69`	`}`
`@@ -77,7 +77,7 @@ static struct pulse *pulse_create(void)`
`77`	`77`	`return p;`
`78`	`78`
`79`	`79`	`kref_init(&p->kref);`
`80`		`- i915_active_init(&p->active, pulse_active, pulse_retire);`
	`80`	`+ i915_active_init(&p->active, pulse_active, pulse_retire, 0);`
`81`	`81`
`82`	`82`	`return p;`
`83`	`83`	`}`