Re: [RFC][PATCH 0/2] drm/panic: Add a drm panic handler

From: nerdopolis <bluescreen_avenger@verizon.net>
To: dri-devel@lists.freedesktop.org, tzimmermann@suse.de,
	airlied@redhat.com,  maarten.lankhorst@linux.intel.com,
	mripard@kernel.org, daniel@ffwll.ch, javierm@redhat.com,
	Jocelyn Falempe <jfalempe@redhat.com>
Subject: Re: [RFC][PATCH 0/2] drm/panic: Add a drm panic handler
Date: Sat, 12 Aug 2023 22:20:57 -0400	[thread overview]
Message-ID: <3182767.5fSG56mABF@nerdopolis2> (raw)
In-Reply-To: <20230809192514.158062-1-jfalempe@redhat.com>

On Wednesday, August 9, 2023 3:17:27 PM EDT Jocelyn Falempe wrote:
> This introduces a new drm panic handler, which displays a message when a panic occurs.
> So when fbcon is disabled, you can still see a kernel panic.
> 
> This is one of the missing feature, when disabling VT/fbcon in the kernel:
> https://www.reddit.com/r/linux/comments/10eccv9/config_vtn_in_2023/
> Fbcon can be replaced by a userspace kms console, but the panic screen must be done in the kernel.
> 
> This is a proof of concept, and works only with simpledrm, using the drm_client API.
> This implementation with the drm client API, allocates new framebuffers, and looks a bit too complex to run in a panic handler.
> Maybe we should add an API to "steal" the current framebuffer instead, because in a panic handler user-space is already stopped.
> 
> To test it, make sure you're using the simpledrm driver, and trigger a panic:
> echo c > /proc/sysrq-trigger
> 
> There is one thing I don't know how to do, is to unregister the drm_panic when the graphic driver is unloaded.
> drm_client_register() says it will automatically unregister on driver unload. But then I don't know how to remove it from my linked list, and free the drm_client_dev struct.
> 
> This is a first draft, so let me know what do you think about it.
Hi, 

Oh wow, that's my post. I'm sorry about the late reply, I only saw this late yesterday, and I wanted to test it first.
I had to edit my test QEMU script a bit to use TianoCore for virtual UEFI boot as there is no gfxmode=keep for SimpleDRM to work otherwise when specifying -kernel to qemu AFAIK

I tested it, and it works! That's pretty cool, although is it possible to show the message, such as "attempted to kill init"?

I like the little ASCII Tux. Maybe an ASCII /!\ or [X] on the belly would make it more obvious to users that it is an error condition. 
Especially for non-English speaking users

I will tweak my script a bit so I can test it more quickly in the future too.

Thanks!
> 
> Best regards,
> 
> 
> 
> 
> Jocelyn Falempe (2):
>   drm/panic: Add a drm panic handler
>   drm/simpledrm: Add drm_panic support
> 
>  drivers/gpu/drm/Kconfig          |  11 ++
>  drivers/gpu/drm/Makefile         |   1 +
>  drivers/gpu/drm/drm_drv.c        |   3 +
>  drivers/gpu/drm/drm_panic.c      | 286 +++++++++++++++++++++++++++++++
>  drivers/gpu/drm/tiny/simpledrm.c |   2 +
>  include/drm/drm_panic.h          |  26 +++
>  6 files changed, 329 insertions(+)
>  create mode 100644 drivers/gpu/drm/drm_panic.c
>  create mode 100644 include/drm/drm_panic.h
> 
> 
> base-commit: 6995e2de6891c724bfeb2db33d7b87775f913ad1
> 