mbox series

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

Message ID 20230915083307.1185571-1-jfalempe@redhat.com (mailing list archive)
Headers show
Series drm/panic: Add a drm panic handler | expand

Message

Jocelyn Falempe Sept. 15, 2023, 8:28 a.m. UTC
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 a new get_scanout_buffer() api

To test it, make sure you're using the simpledrm driver, and trigger a panic:
echo c > /proc/sysrq-trigger

v2
 * Use get_scanout_buffer() instead of the drm client API. (Thomas Zimmermann)
 * Add the panic reason to the panic message (Nerdopolis)
 * Add an exclamation mark (Nerdopolis)
 
I didn't reuse the fbdev functions yet, that would need some fbdev refactoring, because they rely on struct fb_info, and struct vc_data (for font/console). But I still plan to at least try it for v3.

A few more though:
 1) what about gpu with multiple monitor connected ?
maybe get_scanout_buffer() could return a list of scanout buffers ?
 2) I think for some GPU drivers, there might need a flush_scanout_buffer() function, that should be called after the scanout buffer has been filled ?

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      | 270 +++++++++++++++++++++++++++++++
 drivers/gpu/drm/tiny/simpledrm.c |  17 ++
 include/drm/drm_drv.h            |  14 ++
 include/drm/drm_panic.h          |  41 +++++
 7 files changed, 357 insertions(+)
 create mode 100644 drivers/gpu/drm/drm_panic.c
 create mode 100644 include/drm/drm_panic.h


base-commit: 2dde18cd1d8fac735875f2e4987f11817cc0bc2c

Comments

nerdopolis Sept. 16, 2023, 1:09 p.m. UTC | #1
On Friday, September 15, 2023 4:28:20 AM 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 a new get_scanout_buffer() api
> 
> To test it, make sure you're using the simpledrm driver, and trigger a panic:
> echo c > /proc/sysrq-trigger
> 
This seems to work pretty good! With this one, I don't need to have Weston (or another display server) running for it to work this time.
The panic reason works, which is pretty sweet.

FYI: I do get a hunk that fails to apply in simpledrm_remove in drivers/gpu/drm/tiny/simpledrm.c
Seems to be a change in a recentish commit
https://github.com/torvalds/linux/commit/84e6da7ad5537826343636b846530ec2167d4a19

Thanks!
> v2
>  * Use get_scanout_buffer() instead of the drm client API. (Thomas Zimmermann)
>  * Add the panic reason to the panic message (Nerdopolis)
>  * Add an exclamation mark (Nerdopolis)
>  
> I didn't reuse the fbdev functions yet, that would need some fbdev refactoring, because they rely on struct fb_info, and struct vc_data (for font/console). But I still plan to at least try it for v3.
> 
> A few more though:
>  1) what about gpu with multiple monitor connected ?
> maybe get_scanout_buffer() could return a list of scanout buffers ?
>  2) I think for some GPU drivers, there might need a flush_scanout_buffer() function, that should be called after the scanout buffer has been filled ?
> 
> 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      | 270 +++++++++++++++++++++++++++++++
>  drivers/gpu/drm/tiny/simpledrm.c |  17 ++
>  include/drm/drm_drv.h            |  14 ++
>  include/drm/drm_panic.h          |  41 +++++
>  7 files changed, 357 insertions(+)
>  create mode 100644 drivers/gpu/drm/drm_panic.c
>  create mode 100644 include/drm/drm_panic.h
> 
> 
> base-commit: 2dde18cd1d8fac735875f2e4987f11817cc0bc2c
>
Jocelyn Falempe Sept. 18, 2023, 9:32 a.m. UTC | #2
On 16/09/2023 15:09, nerdopolis wrote:
> On Friday, September 15, 2023 4:28:20 AM 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 a new get_scanout_buffer() api
>>
>> To test it, make sure you're using the simpledrm driver, and trigger a panic:
>> echo c > /proc/sysrq-trigger
>>
> This seems to work pretty good! With this one, I don't need to have Weston (or another display server) running for it to work this time.
> The panic reason works, which is pretty sweet.

Thanks for testing, that's really appreciated.
> 
> FYI: I do get a hunk that fails to apply in simpledrm_remove in drivers/gpu/drm/tiny/simpledrm.c
> Seems to be a change in a recentish commit
> https://github.com/torvalds/linux/commit/84e6da7ad5537826343636b846530ec2167d4a19

Thanks for the head-up, when doing this RFC, I'm based on latest 
released version v6.5, to avoid having to rebase too often.
When it's closer to merging, I will rebase to drm-misc-next.

Best regards,
Noralf Trønnes Sept. 18, 2023, 11:19 p.m. UTC | #3
Hi,

On 9/15/23 10:28, 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 a new get_scanout_buffer() api
> 

There's a panic handling entry in Documentation/gpu/todo.rst pointing to
some work done in this area.

Noralf.

> To test it, make sure you're using the simpledrm driver, and trigger a panic:
> echo c > /proc/sysrq-trigger
> 
> v2
>  * Use get_scanout_buffer() instead of the drm client API. (Thomas Zimmermann)
>  * Add the panic reason to the panic message (Nerdopolis)
>  * Add an exclamation mark (Nerdopolis)
>  
> I didn't reuse the fbdev functions yet, that would need some fbdev refactoring, because they rely on struct fb_info, and struct vc_data (for font/console). But I still plan to at least try it for v3.
> 
> A few more though:
>  1) what about gpu with multiple monitor connected ?
> maybe get_scanout_buffer() could return a list of scanout buffers ?
>  2) I think for some GPU drivers, there might need a flush_scanout_buffer() function, that should be called after the scanout buffer has been filled ?
> 
> 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      | 270 +++++++++++++++++++++++++++++++
>  drivers/gpu/drm/tiny/simpledrm.c |  17 ++
>  include/drm/drm_drv.h            |  14 ++
>  include/drm/drm_panic.h          |  41 +++++
>  7 files changed, 357 insertions(+)
>  create mode 100644 drivers/gpu/drm/drm_panic.c
>  create mode 100644 include/drm/drm_panic.h
> 
> 
> base-commit: 2dde18cd1d8fac735875f2e4987f11817cc0bc2c
Jocelyn Falempe Sept. 19, 2023, 7:40 a.m. UTC | #4
On 19/09/2023 01:19, Noralf Trønnes wrote:
> Hi,
> 
> On 9/15/23 10:28, 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 a new get_scanout_buffer() api
>>
> 
> There's a panic handling entry in Documentation/gpu/todo.rst pointing to
> some work done in this area.

Thanks a lot for this pointer, I wasn't aware of this previous drm panic 
attempt. I hope this one will go a bit further ;)
Also I wasn't sure about what is allowed or not in the panic handler, 
and this doc makes things really clear.

Regarding the fbcon interactions, I prevent this at build time, I rely 
on userspace consoles, and hope they will progressively replace fbcon.

My real question is how did I missed that ? My google-fu failed me this 
time.

Best regards,