History

Merge tag 'drm-intel-fixes-2023-02-02' of git://anongit.freedesktop.org/drm/drm-intel into drm-fixes

- Fixes for potential use-after-free and double-free (Rob) - GuC locking and refcount fixes (John) - Display's reference clock value fix (Chaitanya) Signed-off-by: Dave Airlie <airlied@redhat.com>

Dave Airlie committed 3 years ago

Commit 74d6c8e2 parents abf301e 47a2bd911 files · +112 −76Browse files

Unified · Split

11 files
  1. drivers/gpu/drm/i915/display/intel_cdclk.c +1 −1
  2. drivers/gpu/drm/i915/gem/i915_gem_context.c +11 −3
  3. drivers/gpu/drm/i915/gem/i915_gem_tiling.c +5 −4
  4. drivers/gpu/drm/i915/gt/intel_context.c +3 −1
  5. drivers/gpu/drm/i915/gt/intel_context.h +1 −2
  6. drivers/gpu/drm/i915/gt/intel_engine.h +2 −2
  7. drivers/gpu/drm/i915/gt/intel_engine_cs.c +39 −35
  8. drivers/gpu/drm/i915/gt/intel_execlists_submission.c +27 −0
  9. drivers/gpu/drm/i915/gt/intel_execlists_submission.h +4 −0
  10. drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +13 −1
  11. drivers/gpu/drm/i915/i915_gpu_error.c +6 −27
drivers/gpu/drm/i915/display/intel_cdclk.c+1−1
@@ -1319,7 +1319,7 @@ static const struct intel_cdclk_vals adlp_cdclk_table[] = { · 1318 unmodified lines
13191319 { .refclk = 24000, .cdclk = 192000, .divider = 2, .ratio = 16 },
13201320 { .refclk = 24000, .cdclk = 312000, .divider = 2, .ratio = 26 },
13211321 { .refclk = 24000, .cdclk = 552000, .divider = 2, .ratio = 46 },
1322− { .refclk = 24400, .cdclk = 648000, .divider = 2, .ratio = 54 },
1322+ { .refclk = 24000, .cdclk = 648000, .divider = 2, .ratio = 54 },
13231323
13241324 { .refclk = 38400, .cdclk = 179200, .divider = 3, .ratio = 14 },
13251325 { .refclk = 38400, .cdclk = 192000, .divider = 2, .ratio = 10 },
drivers/gpu/drm/i915/gem/i915_gem_context.c+11−3
@@ -1861,11 +1861,19 @@ static int get_ppgtt(struct drm_i915_file_private *file_priv, · 1860 unmodified lines
18611861 vm = ctx->vm;
18621862 GEM_BUG_ON(!vm);
18631863
1864+ /*
1865+ * Get a reference for the allocated handle. Once the handle is
1866+ * visible in the vm_xa table, userspace could try to close it
1867+ * from under our feet, so we need to hold the extra reference
1868+ * first.
1869+ */
1870+ i915_vm_get(vm);
1871+
18641872 err = xa_alloc(&file_priv->vm_xa, &id, vm, xa_limit_32b, GFP_KERNEL);
1865− if (err)
1873+ if (err) {
1874+ i915_vm_put(vm);
18661875 return err;
1867−
1868− i915_vm_get(vm);
1876+ }
18691877
18701878 GEM_BUG_ON(id == 0); /* reserved for invalid/unassigned ppgtt */
18711879 args->value = id;
drivers/gpu/drm/i915/gem/i915_gem_tiling.c+5−4
@@ -305,10 +305,6 @@ i915_gem_object_set_tiling(struct drm_i915_gem_object *obj, · 304 unmodified lines
305305 spin_unlock(&obj->vma.lock);
306306
307307 obj->tiling_and_stride = tiling | stride;
308− i915_gem_object_unlock(obj);
309−
310− /* Force the fence to be reacquired for GTT access */
311− i915_gem_object_release_mmap_gtt(obj);
312308
313309 /* Try to preallocate memory required to save swizzling on put-pages */
314310 if (i915_gem_object_needs_bit17_swizzle(obj)) {
@@ -321,6 +317,11 @@ i915_gem_object_set_tiling(struct drm_i915_gem_object *obj, · 6 unmodified lines
321317 obj->bit_17 = NULL;
322318 }
323319
320+ i915_gem_object_unlock(obj);
321+
322+ /* Force the fence to be reacquired for GTT access */
323+ i915_gem_object_release_mmap_gtt(obj);
324+
324325 return 0;
325326 }
326327
drivers/gpu/drm/i915/gt/intel_context.c+3−1
@@ -528,7 +528,7 @@ retry: · 527 unmodified lines
528528 return rq;
529529 }
530530
531−struct i915_request *intel_context_find_active_request(struct intel_context *ce)
531+struct i915_request *intel_context_get_active_request(struct intel_context *ce)
532532 {
533533 struct intel_context *parent = intel_context_to_parent(ce);
534534 struct i915_request *rq, *active = NULL;
@@ -552,6 +552,8 @@ struct i915_request *intel_context_find_active_request(struct intel_context *ce) · 17 unmodified lines
552552
553553 active = rq;
554554 }
555+ if (active)
556+ active = i915_request_get_rcu(active);
555557 spin_unlock_irqrestore(&parent->guc_state.lock, flags);
556558
557559 return active;
drivers/gpu/drm/i915/gt/intel_context.h+1−2
@@ -268,8 +268,7 @@ int intel_context_prepare_remote_request(struct intel_context *ce, · 267 unmodified lines
268268
269269 struct i915_request *intel_context_create_request(struct intel_context *ce);
270270
271−struct i915_request *
272−intel_context_find_active_request(struct intel_context *ce);
271+struct i915_request *intel_context_get_active_request(struct intel_context *ce);
273272
274273 static inline bool intel_context_is_barrier(const struct intel_context *ce)
275274 {
drivers/gpu/drm/i915/gt/intel_engine.h+2−2
@@ -248,8 +248,8 @@ void intel_engine_dump_active_requests(struct list_head *requests, · 247 unmodified lines
248248 ktime_t intel_engine_get_busy_time(struct intel_engine_cs *engine,
249249 ktime_t *now);
250250
251−struct i915_request *
252−intel_engine_execlist_find_hung_request(struct intel_engine_cs *engine);
251+void intel_engine_get_hung_entity(struct intel_engine_cs *engine,
252+ struct intel_context **ce, struct i915_request **rq);
253253
254254 u32 intel_engine_context_size(struct intel_gt *gt, u8 class);
255255 struct intel_context *
drivers/gpu/drm/i915/gt/intel_engine_cs.c+39−35
@@ -2094,17 +2094,6 @@ static void print_request_ring(struct drm_printer *m, struct i915_request *rq) · 2093 unmodified lines
20942094 }
20952095 }
20962096
2097−static unsigned long list_count(struct list_head *list)
2098−{
2099− struct list_head *pos;
2100− unsigned long count = 0;
2101−
2102− list_for_each(pos, list)
2103− count++;
2104−
2105− return count;
2106−}
2107−
21082097 static unsigned long read_ul(void *p, size_t x)
21092098 {
21102099 return *(unsigned long *)(p + x);
@@ -2196,11 +2185,11 @@ void intel_engine_dump_active_requests(struct list_head *requests, · 85 unmodified lines
21962185 }
21972186 }
21982187
2199−static void engine_dump_active_requests(struct intel_engine_cs *engine, struct drm_printer *m)
2188+static void engine_dump_active_requests(struct intel_engine_cs *engine,
2189+ struct drm_printer *m)
22002190 {
2191+ struct intel_context *hung_ce = NULL;
22012192 struct i915_request *hung_rq = NULL;
2202− struct intel_context *ce;
2203− bool guc;
22042193
22052194 /*
22062195 * No need for an engine->irq_seqno_barrier() before the seqno reads.
@@ -2209,27 +2198,22 @@ static void engine_dump_active_requests(struct intel_engine_cs *engine, struct d · 2 unmodified lines
22092198 * But the intention here is just to report an instantaneous snapshot
22102199 * so that's fine.
22112200 */
2212− lockdep_assert_held(&engine->sched_engine->lock);
2201+ intel_engine_get_hung_entity(engine, &hung_ce, &hung_rq);
22132202
22142203 drm_printf(m, "\tRequests:\n");
22152204
2216− guc = intel_uc_uses_guc_submission(&engine->gt->uc);
2217− if (guc) {
2218− ce = intel_engine_get_hung_context(engine);
2219− if (ce)
2220− hung_rq = intel_context_find_active_request(ce);
2221− } else {
2222− hung_rq = intel_engine_execlist_find_hung_request(engine);
2223− }
2224−
22252205 if (hung_rq)
22262206 engine_dump_request(hung_rq, m, "\t\thung");
2207+ else if (hung_ce)
2208+ drm_printf(m, "\t\tGot hung ce but no hung rq!\n");
22272209
2228− if (guc)
2210+ if (intel_uc_uses_guc_submission(&engine->gt->uc))
22292211 intel_guc_dump_active_requests(engine, hung_rq, m);
22302212 else
2231− intel_engine_dump_active_requests(&engine->sched_engine->requests,
2232− hung_rq, m);
2213+ intel_execlists_dump_active_requests(engine, hung_rq, m);
2214+
2215+ if (hung_rq)
2216+ i915_request_put(hung_rq);
22332217 }
22342218
22352219 void intel_engine_dump(struct intel_engine_cs *engine,
@@ -2239,7 +2223,6 @@ void intel_engine_dump(struct intel_engine_cs *engine, · 3 unmodified lines
22392223 struct i915_gpu_error * const error = &engine->i915->gpu_error;
22402224 struct i915_request *rq;
22412225 intel_wakeref_t wakeref;
2242− unsigned long flags;
22432226 ktime_t dummy;
22442227
22452228 if (header) {
@@ -2276,13 +2259,8 @@ void intel_engine_dump(struct intel_engine_cs *engine, · 30 unmodified lines
22762259 i915_reset_count(error));
22772260 print_properties(engine, m);
22782261
2279− spin_lock_irqsave(&engine->sched_engine->lock, flags);
22802262 engine_dump_active_requests(engine, m);
22812263
2282− drm_printf(m, "\tOn hold?: %lu\n",
2283− list_count(&engine->sched_engine->hold));
2284− spin_unlock_irqrestore(&engine->sched_engine->lock, flags);
2285−
22862264 drm_printf(m, "\tMMIO base: 0x%08x\n", engine->mmio_base);
22872265 wakeref = intel_runtime_pm_get_if_in_use(engine->uncore->rpm);
22882266 if (wakeref) {
@@ -2328,8 +2306,7 @@ intel_engine_create_virtual(struct intel_engine_cs **siblings, · 39 unmodified lines
23282306 return siblings[0]->cops->create_virtual(siblings, count, flags);
23292307 }
23302308
2331−struct i915_request *
2332−intel_engine_execlist_find_hung_request(struct intel_engine_cs *engine)
2309+static struct i915_request *engine_execlist_find_hung_request(struct intel_engine_cs *engine)
23332310 {
23342311 struct i915_request *request, *active = NULL;
23352312
@@ -2381,6 +2358,33 @@ intel_engine_execlist_find_hung_request(struct intel_engine_cs *engine) · 45 unmodified lines
23812358 return active;
23822359 }
23832360
2361+void intel_engine_get_hung_entity(struct intel_engine_cs *engine,
2362+ struct intel_context **ce, struct i915_request **rq)
2363+{
2364+ unsigned long flags;
2365+
2366+ *ce = intel_engine_get_hung_context(engine);
2367+ if (*ce) {
2368+ intel_engine_clear_hung_context(engine);
2369+
2370+ *rq = intel_context_get_active_request(*ce);
2371+ return;
2372+ }
2373+
2374+ /*
2375+ * Getting here with GuC enabled means it is a forced error capture
2376+ * with no actual hang. So, no need to attempt the execlist search.
2377+ */
2378+ if (intel_uc_uses_guc_submission(&engine->gt->uc))
2379+ return;
2380+
2381+ spin_lock_irqsave(&engine->sched_engine->lock, flags);
2382+ *rq = engine_execlist_find_hung_request(engine);
2383+ if (*rq)
2384+ *rq = i915_request_get_rcu(*rq);
2385+ spin_unlock_irqrestore(&engine->sched_engine->lock, flags);
2386+}
2387+
23842388 void xehp_enable_ccs_engines(struct intel_engine_cs *engine)
23852389 {
23862390 /*
drivers/gpu/drm/i915/gt/intel_execlists_submission.c+27−0
@@ -4148,6 +4148,33 @@ void intel_execlists_show_requests(struct intel_engine_cs *engine, · 4147 unmodified lines
41484148 spin_unlock_irqrestore(&sched_engine->lock, flags);
41494149 }
41504150
4151+static unsigned long list_count(struct list_head *list)
4152+{
4153+ struct list_head *pos;
4154+ unsigned long count = 0;
4155+
4156+ list_for_each(pos, list)
4157+ count++;
4158+
4159+ return count;
4160+}
4161+
4162+void intel_execlists_dump_active_requests(struct intel_engine_cs *engine,
4163+ struct i915_request *hung_rq,
4164+ struct drm_printer *m)
4165+{
4166+ unsigned long flags;
4167+
4168+ spin_lock_irqsave(&engine->sched_engine->lock, flags);
4169+
4170+ intel_engine_dump_active_requests(&engine->sched_engine->requests, hung_rq, m);
4171+
4172+ drm_printf(m, "\tOn hold?: %lu\n",
4173+ list_count(&engine->sched_engine->hold));
4174+
4175+ spin_unlock_irqrestore(&engine->sched_engine->lock, flags);
4176+}
4177+
41514178 #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
41524179 #include "selftest_execlists.c"
41534180 #endif
drivers/gpu/drm/i915/gt/intel_execlists_submission.h+4−0
@@ -32,6 +32,10 @@ void intel_execlists_show_requests(struct intel_engine_cs *engine, · 31 unmodified lines
3232 int indent),
3333 unsigned int max);
3434
35+void intel_execlists_dump_active_requests(struct intel_engine_cs *engine,
36+ struct i915_request *hung_rq,
37+ struct drm_printer *m);
38+
3539 bool
3640 intel_engine_in_execlists_submission_mode(const struct intel_engine_cs *engine);
3741
drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c+13−1
@@ -1702,7 +1702,7 @@ static void __guc_reset_context(struct intel_context *ce, intel_engine_mask_t st · 1701 unmodified lines
17021702 goto next_context;
17031703
17041704 guilty = false;
1705− rq = intel_context_find_active_request(ce);
1705+ rq = intel_context_get_active_request(ce);
17061706 if (!rq) {
17071707 head = ce->ring->tail;
17081708 goto out_replay;
@@ -1715,6 +1715,7 @@ static void __guc_reset_context(struct intel_context *ce, intel_engine_mask_t st · 6 unmodified lines
17151715 head = intel_ring_wrap(ce->ring, rq->head);
17161716
17171717 __i915_request_reset(rq, guilty);
1718+ i915_request_put(rq);
17181719 out_replay:
17191720 guc_reset_state(ce, head, guilty);
17201721 next_context:
@@ -4817,6 +4818,8 @@ void intel_guc_find_hung_context(struct intel_engine_cs *engine) · 3096 unmodified lines
48174818
48184819 xa_lock_irqsave(&guc->context_lookup, flags);
48194820 xa_for_each(&guc->context_lookup, index, ce) {
4821+ bool found;
4822+
48204823 if (!kref_get_unless_zero(&ce->ref))
48214824 continue;
48224825
@@ -4833,10 +4836,18 @@ void intel_guc_find_hung_context(struct intel_engine_cs *engine) · 10 unmodified lines
48334836 goto next;
48344837 }
48354838
4839+ found = false;
4840+ spin_lock(&ce->guc_state.lock);
48364841 list_for_each_entry(rq, &ce->guc_state.requests, sched.link) {
48374842 if (i915_test_request_state(rq) != I915_REQUEST_ACTIVE)
48384843 continue;
48394844
4845+ found = true;
4846+ break;
4847+ }
4848+ spin_unlock(&ce->guc_state.lock);
4849+
4850+ if (found) {
48404851 intel_engine_set_hung_context(engine, ce);
48414852
48424853 /* Can only cope with one hang at a time... */
@@ -4844,6 +4855,7 @@ void intel_guc_find_hung_context(struct intel_engine_cs *engine) · 1 unmodified line
48444855 xa_lock(&guc->context_lookup);
48454856 goto done;
48464857 }
4858+
48474859 next:
48484860 intel_context_put(ce);
48494861 xa_lock(&guc->context_lookup);
drivers/gpu/drm/i915/i915_gpu_error.c+6−27
@@ -1596,43 +1596,20 @@ capture_engine(struct intel_engine_cs *engine, · 1595 unmodified lines
15961596 {
15971597 struct intel_engine_capture_vma *capture = NULL;
15981598 struct intel_engine_coredump *ee;
1599− struct intel_context *ce;
1599+ struct intel_context *ce = NULL;
16001600 struct i915_request *rq = NULL;
1601− unsigned long flags;
16021601
16031602 ee = intel_engine_coredump_alloc(engine, ALLOW_FAIL, dump_flags);
16041603 if (!ee)
16051604 return NULL;
16061605
1607− ce = intel_engine_get_hung_context(engine);
1608− if (ce) {
1609− intel_engine_clear_hung_context(engine);
1610− rq = intel_context_find_active_request(ce);
1611− if (!rq || !i915_request_started(rq))
1612− goto no_request_capture;
1613− } else {
1614− /*
1615− * Getting here with GuC enabled means it is a forced error capture
1616− * with no actual hang. So, no need to attempt the execlist search.
1617− */
1618− if (!intel_uc_uses_guc_submission(&engine->gt->uc)) {
1619− spin_lock_irqsave(&engine->sched_engine->lock, flags);
1620− rq = intel_engine_execlist_find_hung_request(engine);
1621− spin_unlock_irqrestore(&engine->sched_engine->lock,
1622− flags);
1623− }
1624− }
1625− if (rq)
1626− rq = i915_request_get_rcu(rq);
1627−
1628− if (!rq)
1606+ intel_engine_get_hung_entity(engine, &ce, &rq);
1607+ if (!rq || !i915_request_started(rq))
16291608 goto no_request_capture;
16301609
16311610 capture = intel_engine_coredump_add_request(ee, rq, ATOMIC_MAYFAIL);
1632− if (!capture) {
1633− i915_request_put(rq);
1611+ if (!capture)
16341612 goto no_request_capture;
1635− }
16361613 if (dump_flags & CORE_DUMP_FLAG_IS_GUC_CAPTURE)
16371614 intel_guc_capture_get_matching_node(engine->gt, ee, ce);
16381615
@@ -1642,6 +1619,8 @@ capture_engine(struct intel_engine_cs *engine, · 3 unmodified lines
16421619 return ee;
16431620
16441621 no_request_capture:
1622+ if (rq)
1623+ i915_request_put(rq);
16451624 kfree(ee);
16461625 return NULL;
16471626 }