linux-kernel - Re: [PATCH v2 3/3] drm/msm/dp: check main link status before start aux read

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: <161852681935.46595.9941294298184495830@swboyd.mtv.corp.google.com>
Date:   Thu, 15 Apr 2021 15:46:59 -0700
From:   Stephen Boyd <swboyd@...omium.org>
To:     khsieh@...eaurora.org
Cc:     robdclark@...il.com, sean@...rly.run, abhinavk@...eaurora.org,
        aravindh@...eaurora.org, airlied@...ux.ie, daniel@...ll.ch,
        linux-arm-msm@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        freedreno@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 3/3] drm/msm/dp: check main link status before start aux read

Quoting khsieh@...eaurora.org (2021-04-15 15:02:40)
> On 2021-04-15 13:06, Stephen Boyd wrote:
> > 
> > Is it really necessary to have this patch at all? I think there are
> > bigger problems with suspend/resume of the DP driver in relation to the
> > kthread stopping. I hope that the aux channel would start NAKing
> > transfers once the cable is disconnected too, so that we don't need to
> > do an extra check for each aux transfer.
> 
> I am working on duplicate this problem, but it is not happen on me yet 
> so far.
>  From kernel dump, i can see it crash at dp_irq_hdp_handle() after 
> suspended.
> dp_irq_hpd_handle and dp_pm_suspend() are serialized by event_mutex.
> 
> After suspend, ahb clock is disabled.
> Hence next dp_catalog_link_is_connected() crash at acess dp ctrl 
> registers.
> 
> 
> aux channel does not do NAKing immediately if unplugged. Therefore 
> aux_transfer will wait until timeout (HZ/4).
> worst, drm_dp_dpcd_access() will retry 32 times before return dpcd 
> read/write failed.
> This patch try to eliminate the time spinning on waiting for timeout 32 
> times.
> 

Would be useful to have that level of detail in the commit text.

Maybe when the cable is disconnected the DP phy should be shutdown and
some bit in the phy could effectively "cut off" the aux channel and then
NAKs would start coming through here in the DP controller I/O register
space?