| From 714eaebb94c9b5ae4d8eaaa92dcb4b91ab022340 Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Tue, 9 Nov 2021 10:04:18 -0800 |
| Subject: drm/msm/dp: Avoid unpowered AUX xfers that caused crashes |
| |
| From: Douglas Anderson <dianders@chromium.org> |
| |
| [ Upstream commit d03fcc1de0863b1188ceb867cfa84a578fdc96bc ] |
| |
| 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 <dianders@chromium.org> |
| Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com> |
| Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com> |
| Link: https://lore.kernel.org/r/20211109100403.1.I4e23470d681f7efe37e2e7f1a6466e15e9bb1d72@changeid |
| Signed-off-by: Rob Clark <robdclark@chromium.org> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| drivers/gpu/drm/msm/dp/dp_aux.c | 17 +++++++++++++++++ |
| 1 file changed, 17 insertions(+) |
| |
| diff --git a/drivers/gpu/drm/msm/dp/dp_aux.c b/drivers/gpu/drm/msm/dp/dp_aux.c |
| index eb40d8413bca9..6d36f63c33388 100644 |
| --- a/drivers/gpu/drm/msm/dp/dp_aux.c |
| +++ b/drivers/gpu/drm/msm/dp/dp_aux.c |
| @@ -33,6 +33,7 @@ struct dp_aux_private { |
| bool read; |
| bool no_send_addr; |
| bool no_send_stop; |
| + bool initted; |
| u32 offset; |
| u32 segment; |
| |
| @@ -331,6 +332,10 @@ static ssize_t dp_aux_transfer(struct drm_dp_aux *dp_aux, |
| } |
| |
| mutex_lock(&aux->mutex); |
| + if (!aux->initted) { |
| + ret = -EIO; |
| + goto exit; |
| + } |
| |
| dp_aux_update_offset_and_segment(aux, msg); |
| dp_aux_transfer_helper(aux, msg, true); |
| @@ -380,6 +385,8 @@ static ssize_t dp_aux_transfer(struct drm_dp_aux *dp_aux, |
| } |
| |
| aux->cmd_busy = false; |
| + |
| +exit: |
| mutex_unlock(&aux->mutex); |
| |
| return ret; |
| @@ -431,8 +438,13 @@ void dp_aux_init(struct drm_dp_aux *dp_aux) |
| |
| aux = container_of(dp_aux, struct dp_aux_private, dp_aux); |
| |
| + mutex_lock(&aux->mutex); |
| + |
| dp_catalog_aux_enable(aux->catalog, true); |
| aux->retry_cnt = 0; |
| + aux->initted = true; |
| + |
| + mutex_unlock(&aux->mutex); |
| } |
| |
| void dp_aux_deinit(struct drm_dp_aux *dp_aux) |
| @@ -441,7 +453,12 @@ void dp_aux_deinit(struct drm_dp_aux *dp_aux) |
| |
| aux = container_of(dp_aux, struct dp_aux_private, dp_aux); |
| |
| + mutex_lock(&aux->mutex); |
| + |
| + aux->initted = false; |
| dp_catalog_aux_enable(aux->catalog, false); |
| + |
| + mutex_unlock(&aux->mutex); |
| } |
| |
| int dp_aux_register(struct drm_dp_aux *dp_aux) |
| -- |
| 2.33.0 |
| |