Skip to content

Commit d03fcc1

Browse files
diandersrobclark
authored andcommitted
drm/msm/dp: Avoid unpowered AUX xfers that caused crashes
If you happened to try to access `/dev/drm_dp_aux` devices provided by the MSM DP AUX driver too early at bootup you could go boom. Let's avoid that by only allowing AUX transfers when the controller is powered up. Specifically the crash that was seen (on Chrome OS 5.4 tree with relevant backports): Kernel panic - not syncing: Asynchronous SError Interrupt CPU: 0 PID: 3131 Comm: fwupd Not tainted 5.4.144-16620-g28af11b73efb #1 Hardware name: Google Lazor (rev3+) with KB Backlight (DT) Call trace: dump_backtrace+0x0/0x14c show_stack+0x20/0x2c dump_stack+0xac/0x124 panic+0x150/0x390 nmi_panic+0x80/0x94 arm64_serror_panic+0x78/0x84 do_serror+0x0/0x118 do_serror+0xa4/0x118 el1_error+0xbc/0x160 dp_catalog_aux_write_data+0x1c/0x3c dp_aux_cmd_fifo_tx+0xf0/0x1b0 dp_aux_transfer+0x1b0/0x2bc drm_dp_dpcd_access+0x8c/0x11c drm_dp_dpcd_read+0x64/0x10c auxdev_read_iter+0xd4/0x1c4 I did a little bit of tracing and found that: * We register the AUX device very early at bootup. * Power isn't actually turned on for my system until hpd_event_thread() -> dp_display_host_init() -> dp_power_init() * You can see that dp_power_init() calls dp_aux_init() which is where we start allowing AUX channel requests to go through. In general this patch is a bit of a bandaid but at least it gets us out of the current state where userspace acting at the wrong time can fully crash the system. * I think the more proper fix (which requires quite a bit more changes) is to power stuff on while an AUX transfer is happening. This is like the solution we did for ti-sn65dsi86. This might be required for us to move to populating the panel via the DP-AUX bus. * Another fix considered was to dynamically register / unregister. I tried that at <https://crrev.com/c/3169431/3> but it got ugly. Currently there's a bug where the pm_runtime() state isn't tracked properly and that causes us to just keep registering more and more. Signed-off-by: Douglas Anderson <[email protected]> Reviewed-by: Kuogee Hsieh <[email protected]> Reviewed-by: Abhinav Kumar <[email protected]> Link: https://lore.kernel.org/r/20211109100403.1.I4e23470d681f7efe37e2e7f1a6466e15e9bb1d72@changeid Signed-off-by: Rob Clark <[email protected]>
1 parent cd92cc1 commit d03fcc1

File tree

1 file changed

+17
-0
lines changed

1 file changed

+17
-0
lines changed

drivers/gpu/drm/msm/dp/dp_aux.c

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ struct dp_aux_private {
3333
bool read;
3434
bool no_send_addr;
3535
bool no_send_stop;
36+
bool initted;
3637
u32 offset;
3738
u32 segment;
3839

@@ -331,6 +332,10 @@ static ssize_t dp_aux_transfer(struct drm_dp_aux *dp_aux,
331332
}
332333

333334
mutex_lock(&aux->mutex);
335+
if (!aux->initted) {
336+
ret = -EIO;
337+
goto exit;
338+
}
334339

335340
dp_aux_update_offset_and_segment(aux, msg);
336341
dp_aux_transfer_helper(aux, msg, true);
@@ -380,6 +385,8 @@ static ssize_t dp_aux_transfer(struct drm_dp_aux *dp_aux,
380385
}
381386

382387
aux->cmd_busy = false;
388+
389+
exit:
383390
mutex_unlock(&aux->mutex);
384391

385392
return ret;
@@ -431,8 +438,13 @@ void dp_aux_init(struct drm_dp_aux *dp_aux)
431438

432439
aux = container_of(dp_aux, struct dp_aux_private, dp_aux);
433440

441+
mutex_lock(&aux->mutex);
442+
434443
dp_catalog_aux_enable(aux->catalog, true);
435444
aux->retry_cnt = 0;
445+
aux->initted = true;
446+
447+
mutex_unlock(&aux->mutex);
436448
}
437449

438450
void dp_aux_deinit(struct drm_dp_aux *dp_aux)
@@ -441,7 +453,12 @@ void dp_aux_deinit(struct drm_dp_aux *dp_aux)
441453

442454
aux = container_of(dp_aux, struct dp_aux_private, dp_aux);
443455

456+
mutex_lock(&aux->mutex);
457+
458+
aux->initted = false;
444459
dp_catalog_aux_enable(aux->catalog, false);
460+
461+
mutex_unlock(&aux->mutex);
445462
}
446463

447464
int dp_aux_register(struct drm_dp_aux *dp_aux)

0 commit comments

Comments
 (0)