linux-kernel - Re: [PATCH v2 3/3] drm/panic: Add a drm_panic/draw

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] [day] [month] [year] [list]
Message-ID: <20250923-smart-curly-muskrat-9acae7@penduick>
Date: Tue, 23 Sep 2025 12:15:26 +0200
From: Maxime Ripard <mripard@...nel.org>
To: Jocelyn Falempe <jfalempe@...hat.com>
Cc: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, 
	Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>, 
	Simona Vetter <simona@...ll.ch>, Javier Martinez Canillas <javierm@...hat.com>, 
	linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH v2 3/3] drm/panic: Add a drm_panic/draw_test in debugfs

On Thu, Sep 11, 2025 at 02:00:42PM +0200, Jocelyn Falempe wrote:
> On 10/09/2025 12:49, Maxime Ripard wrote:
> > On Mon, Sep 08, 2025 at 11:00:31AM +0200, Jocelyn Falempe wrote:
> > > This adds a new drm_panic/draw_test file in debugfs.
> > > This file allows to test the panic screen rendering at different
> > > resolution and pixel format.
> > > It's useful only for kernel developers that want to create or
> > > customize a panic screen.
> > > 
> > > If you want to check the result at 1024x768 using XRGB8888:
> > > 
> > > cd /sys/kernel/debug/drm_panic/
> > > exec 3<> draw_test
> > > echo 1024x768:XR24 >&3
> > > cat <&3 > ~/panic_screen.raw
> > > exec 3<&-
> > > 
> > > Signed-off-by: Jocelyn Falempe <jfalempe@...hat.com>
> > 
> > I see what you meant in your previous version, and I misunderstood what
> > you were saying, sorry.
> > 
> > > v2:
> > >   * Use debugfs instead of sending the framebuffer through the kunit logs. (Thomas Zimmermann).
> > > 
> > >   drivers/gpu/drm/Kconfig     |   2 +
> > >   drivers/gpu/drm/drm_panic.c | 117 ++++++++++++++++++++++++++++++++++++
> > >   2 files changed, 119 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> > > index f7ea8e895c0c..0d3146070d9c 100644
> > > --- a/drivers/gpu/drm/Kconfig
> > > +++ b/drivers/gpu/drm/Kconfig
> > > @@ -83,6 +83,8 @@ config DRM_PANIC_DEBUG
> > >   	  Add dri/[device]/drm_panic_plane_x in the kernel debugfs, to force the
> > >   	  panic handler to write the panic message to this plane scanout buffer.
> > >   	  This is unsafe and should not be enabled on a production build.
> > > +	  Also adds a drm_panic/draw_test file in debugfs, to easily test the
> > > +	  panic screen rendering.
> > >   	  If in doubt, say "N".
> > >   config DRM_PANIC_SCREEN
> > > diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
> > > index d89812ff1935..0c01d6067eab 100644
> > > --- a/drivers/gpu/drm/drm_panic.c
> > > +++ b/drivers/gpu/drm/drm_panic.c
> > > @@ -873,6 +873,7 @@ static void drm_panic(struct kmsg_dumper *dumper, struct kmsg_dump_detail *detai
> > >    */
> > >   #ifdef CONFIG_DRM_PANIC_DEBUG
> > >   #include <linux/debugfs.h>
> > > +#include <linux/vmalloc.h>
> > >   static ssize_t debugfs_trigger_write(struct file *file, const char __user *user_buf,
> > >   				     size_t count, loff_t *ppos)
> > > @@ -901,8 +902,122 @@ static void debugfs_register_plane(struct drm_plane *plane, int index)
> > >   	debugfs_create_file(fname, 0200, plane->dev->debugfs_root,
> > >   			    plane, &dbg_drm_panic_ops);
> > >   }
> > > +
> > > +/*
> > > + * Draw test interface
> > > + * This can be used to check the panic screen at any resolution/pixel format.
> > > + * The framebuffer memory is freed when the file is closed, so use this sh
> > > + * script to write the parameters and read the result without closing the file.
> > > + * cd /sys/kernel/debug/drm_panic/
> > > + * exec 3<> draw_test
> > > + * echo 1024x768:XR24 >&3
> > > + * cat <&3 > ~/panic_screen.raw
> > > + * exec 3<&-
> > > + */
> > 
> > This should be documented properly, and I'm also kind of wondering how
> > that would fit in the larger testing ecosystem.
> > 
> > Ie, how can someone that just starts contributing to Linux, or is
> > setting up a CI platform, can have that test running.
> > 
> > kunit is great for that, kselftests to some extent too, but I'm not sure
> > an ad-hoc interface is.
> 
> It's a bit harder to setup, but also allows to do some useful things.

I'm not saying that the original test has no use. I'm saying a test that
can't be discovered and run automatically is a lot less useful, and even
more so when it's using an ad-hoc interface.

Because that means that CI, all the other devs, etc. probably won't know
anything about it and you'll end up being the only one running the test.

That's why I've been insisting on a standard solution, because that
would solve that problem.

I still believe that your use-case is legit, and the test can be useful,
but it needs to be somewhat standard. Getting the opinion from the kunit
maintainers would be a great first step for example.

> I've written a small GUI application which displays the contents of the
> debugfs drm_panic/draw_test file in a window.
> The displayed content is automatically refreshed whenever the window is
> resized, making it easy to inspect the DRM panic output at any screen
> resolution.
> https://gitlab.com/kdj0c/panicviewer

And I'm sure that part is useful to you, I wonder if it's something that
should be upstream.

Maxime

Download attachment "signature.asc" of type "application/pgp-signature" (274 bytes)