linux-kernel - Re: [PATCH v7] drm/msm/dp: Always clear mask bits to disable interrupts at dp_ctrl_reset_irq

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <089aaf55-084b-9a29-629c-3096930fac3b@quicinc.com>
Date:   Thu, 19 May 2022 10:50:48 -0700
From:   Kuogee Hsieh <quic_khsieh@...cinc.com>
To:     <robdclark@...il.com>, <sean@...rly.run>, <swboyd@...omium.org>,
        <dianders@...omium.org>, <vkoul@...nel.org>, <daniel@...ll.ch>,
        <airlied@...ux.ie>, <agross@...nel.org>,
        <dmitry.baryshkov@...aro.org>, <bjorn.andersson@...aro.org>
CC:     <quic_abhinavk@...cinc.com>, <quic_aravindh@...cinc.com>,
        <quic_sbillaka@...cinc.com>, <freedreno@...ts.freedesktop.org>,
        <dri-devel@...ts.freedesktop.org>, <linux-arm-msm@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v7] drm/msm/dp: Always clear mask bits to disable
 interrupts at dp_ctrl_reset_irq_ctrl()


On 5/17/2022 9:21 AM, Kuogee Hsieh wrote:

Is anyone has comments on this patch?
> dp_catalog_ctrl_reset() will software reset DP controller. But it will
> not reset programmable registers to default value. DP driver still have
> to clear mask bits to interrupt status registers to disable interrupts
> after software reset of controller.
>
> At current implementation, dp_ctrl_reset_irq_ctrl() will software reset dp
> controller but did not call dp_catalog_ctrl_enable_irq(false) to clear hpd
> related interrupt mask bits to disable hpd related interrupts due to it
> mistakenly think hpd related interrupt mask bits will be cleared by software
> reset of dp controller automatically. This mistake may cause system to crash
> during suspending procedure due to unexpected irq fired and trigger event
> thread to access dp controller registers with controller clocks are disabled.
>
> This patch fixes system crash during suspending problem by removing "enable"
> flag condition checking at dp_ctrl_reset_irq_ctrl() so that hpd related
> interrupt mask bits are cleared to prevent unexpected from happening.
>
> Changes in v2:
> -- add more details commit text
>
> Changes in v3:
> -- add synchrons_irq()
> -- add atomic_t suspended
>
> Changes in v4:
> -- correct Fixes's commit ID
> -- remove synchrons_irq()
>
> Changes in v5:
> -- revise commit text
>
> Changes in v6:
> -- add event_lock to protect "suspended"
>
> Changes in v7:
> -- delete "suspended" flag
>
> Fixes: 989ebe7bc446 ("drm/msm/dp: do not initialize phy until plugin interrupt received")
> Signed-off-by: Kuogee Hsieh <quic_khsieh@...cinc.com>
> ---
>   drivers/gpu/drm/msm/dp/dp_ctrl.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> index 5356856..5ddb4e8 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> @@ -1380,8 +1380,13 @@ void dp_ctrl_reset_irq_ctrl(struct dp_ctrl *dp_ctrl, bool enable)
>   
>   	dp_catalog_ctrl_reset(ctrl->catalog);
>   
> -	if (enable)
> -		dp_catalog_ctrl_enable_irq(ctrl->catalog, enable);
> +	/*
> +	 * all dp controller programmable registers will not
> +	 * be reset to default value after DP_SW_RESET
> +	 * therefore interrupt mask bits have to be updated
> +	 * to enable/disable interrupts
> +	 */
> +	dp_catalog_ctrl_enable_irq(ctrl->catalog, enable);
>   }
>   
>   void dp_ctrl_phy_init(struct dp_ctrl *dp_ctrl)