linux-kernel - Re: [PATCH] drm/client: Send hotplug event after registering a client

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: <325dad0e-38ff-9f60-efc9-0fd711d63267@linaro.org>
Date:   Tue, 11 Jul 2023 00:11:50 +0300
From:   Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To:     Thomas Zimmermann <tzimmermann@...e.de>, javierm@...hat.com,
        noralf@...nnes.org
Cc:     dri-devel@...ts.freedesktop.org,
        Moritz Duge <MoritzDuge@...ahilft.de>,
        Torsten Krah <krah.tm@...il.com>,
        Paul Schyska <pschyska@...il.com>,
        Daniel Vetter <daniel.vetter@...ll.ch>,
        David Airlie <airlied@...il.com>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Maxime Ripard <mripard@...nel.org>,
        Russell King <linux@...linux.org.uk>,
        Inki Dae <inki.dae@...sung.com>,
        Seung-Woo Kim <sw0312.kim@...sung.com>,
        Kyungmin Park <kyungmin.park@...sung.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        Patrik Jakobsson <patrik.r.jakobsson@...il.com>,
        Rob Clark <robdclark@...il.com>,
        Abhinav Kumar <quic_abhinavk@...cinc.com>,
        Tomi Valkeinen <tomi.valkeinen@...asonboard.com>,
        Alex Deucher <alexander.deucher@....com>,
        Christian König <christian.koenig@....com>,
        "Pan, Xinhui" <Xinhui.Pan@....com>,
        Thierry Reding <thierry.reding@...il.com>,
        Mikko Perttunen <mperttunen@...dia.com>,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-samsung-soc@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        freedreno@...ts.freedesktop.org, amd-gfx@...ts.freedesktop.org,
        linux-tegra@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH] drm/client: Send hotplug event after registering a client

On 10/07/2023 12:10, Thomas Zimmermann wrote:
> Generate a hotplug event after registering a client to allow the
> client to configure its display. Remove the hotplug calls from the
> existing clients for fbdev emulation. This change fixes a concurrency
> bug between registering a client and receiving events from the DRM
> core. The bug is present in the fbdev emulation of all drivers.
> 
> The fbdev emulation currently generates a hotplug event before
> registering the client to the device. For each new output, the DRM
> core sends an additional hotplug event to each registered client.
> 
> If the DRM core detects first output between sending the artificial
> hotplug and registering the device, the output's hotplug event gets
> lost. If this is the first output, the fbdev console display remains
> dark. This has been observed with amdgpu and fbdev-generic.
> 
> Fix this by adding hotplug generation directly to the client's
> register helper drm_client_register(). Registering the client and
> receiving events are serialized by struct drm_device.clientlist_mutex.
> So an output is either configured by the initial hotplug event, or
> the client has already been registered.
> 
> The bug was originally added in commit 6e3f17ee73f7 ("drm/fb-helper:
> generic: Call drm_client_add() after setup is done"), in which adding
> a client and receiving a hotplug event switched order. It was hidden,
> as most hardware and drivers have at least on static output configured.
> Other drivers didn't use the internal DRM client or still had struct
> drm_mode_config_funcs.output_poll_changed set. That callback handled
> hotplug events as well. After not setting the callback in amdgpu in
> commit 0e3172bac3f4 ("drm/amdgpu: Don't set struct
> drm_driver.output_poll_changed"), amdgpu did not show a framebuffer
> console if output events got lost. The bug got copy-pasted from
> fbdev-generic into the other fbdev emulation.
> 
> Reported-by: Moritz Duge <MoritzDuge@...ahilft.de>
> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/2649
> Fixes: 6e3f17ee73f7 ("drm/fb-helper: generic: Call drm_client_add() after setup is done")
> Fixes: 8ab59da26bc0 ("drm/fb-helper: Move generic fbdev emulation into separate source file")
> Fixes: b79fe9abd58b ("drm/fbdev-dma: Implement fbdev emulation for GEM DMA helpers")
> Fixes: 63c381552f69 ("drm/armada: Implement fbdev emulation as in-kernel client")
> Fixes: 49953b70e7d3 ("drm/exynos: Implement fbdev emulation as in-kernel client")
> Fixes: 8f1aaccb04b7 ("drm/gma500: Implement client-based fbdev emulation")
> Fixes: 940b869c2f2f ("drm/msm: Implement fbdev emulation as in-kernel client")
> Fixes: 9e69bcd88e45 ("drm/omapdrm: Implement fbdev emulation as in-kernel client")
> Fixes: e317a69fe891 ("drm/radeon: Implement client-based fbdev emulation")
> Fixes: 71ec16f45ef8 ("drm/tegra: Implement fbdev emulation as in-kernel client")
> Signed-off-by: Thomas Zimmermann <tzimmermann@...e.de>
> Tested-by: Moritz Duge <MoritzDuge@...ahilft.de>
> Tested-by: Torsten Krah <krah.tm@...il.com>
> Tested-by: Paul Schyska <pschyska@...il.com>
> Cc: Daniel Vetter <daniel.vetter@...ll.ch>
> Cc: David Airlie <airlied@...il.com>
> Cc: Noralf Trønnes <noralf@...nnes.org>
> Cc: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>
> Cc: Maxime Ripard <mripard@...nel.org>
> Cc: Javier Martinez Canillas <javierm@...hat.com>
> Cc: Russell King <linux@...linux.org.uk>
> Cc: Inki Dae <inki.dae@...sung.com>
> Cc: Seung-Woo Kim <sw0312.kim@...sung.com>
> Cc: Kyungmin Park <kyungmin.park@...sung.com>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
> Cc: Patrik Jakobsson <patrik.r.jakobsson@...il.com>
> Cc: Rob Clark <robdclark@...il.com>
> Cc: Abhinav Kumar <quic_abhinavk@...cinc.com>
> Cc: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
> Cc: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
> Cc: Alex Deucher <alexander.deucher@....com>
> Cc: "Christian König" <christian.koenig@....com>
> Cc: "Pan, Xinhui" <Xinhui.Pan@....com>
> Cc: Thierry Reding <thierry.reding@...il.com>
> Cc: Mikko Perttunen <mperttunen@...dia.com>
> Cc: dri-devel@...ts.freedesktop.org
> Cc: linux-kernel@...r.kernel.org
> Cc: linux-arm-kernel@...ts.infradead.org
> Cc: linux-samsung-soc@...r.kernel.org
> Cc: linux-arm-msm@...r.kernel.org
> Cc: freedreno@...ts.freedesktop.org
> Cc: amd-gfx@...ts.freedesktop.org
> Cc: linux-tegra@...r.kernel.org
> Cc: dri-devel@...ts.freedesktop.org
> Cc: <stable@...r.kernel.org> # v5.2+
> ---
>   drivers/gpu/drm/armada/armada_fbdev.c     |  4 ----
>   drivers/gpu/drm/drm_client.c              | 21 +++++++++++++++++++++
>   drivers/gpu/drm/drm_fbdev_dma.c           |  4 ----
>   drivers/gpu/drm/drm_fbdev_generic.c       |  4 ----
>   drivers/gpu/drm/exynos/exynos_drm_fbdev.c |  4 ----
>   drivers/gpu/drm/gma500/fbdev.c            |  4 ----
>   drivers/gpu/drm/msm/msm_fbdev.c           |  4 ----

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org> # msm

>   drivers/gpu/drm/omapdrm/omap_fbdev.c      |  4 ----
>   drivers/gpu/drm/radeon/radeon_fbdev.c     |  4 ----
>   drivers/gpu/drm/tegra/fbdev.c             |  4 ----
>   10 files changed, 21 insertions(+), 36 deletions(-)

BTW: As you have been clearing this area. I see that significant amount 
of DRM drivers use exactly the same code for msm_fbdev_client_funcs and 
for the significant part of foo_fbdev_setup(). Do you have any plans for 
moving that into a library / generic code? If not, I can take a look at 
crafting the patch.

-- 
With best wishes
Dmitry