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

[Jocelyn Falempe]

On 13/08/2023 04:20, nerdopolis wrote:
> 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"?

Thanks for taking time to test it. Yes it's possible to show the panic 
reason, as it's a parameter in the panic callback.
> 
> 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

That's a good idea. It's also probably possible to re-use the tux boot 
logo, but I didn't try it yet.

But currently, my priority is to see if it can get accepted, and if it 
can work with a wide range of drivers.

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


Best Regards,

-- 

Jocelyn
> 
> 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
>>
> 
> 
> 
>