Re: [PATCH v2 3/3] drm/panic: Add a kmsg panic screen
On 07/06/2024 11:16, Javier Martinez Canillas wrote:
Jocelyn Falempe writes:
Add a kmsg option, which will display the last lines of kmsg,
and should be similar to fbcon.
Add a drm.panic_screen module parameter, so you can choose between
the different panic screens available.
two options currently, but more will be added later:
* "user": a short message telling the user to reboot the machine.
* "kmsg": fill the screen with the last lines of kmsg.
You can even change it at runtime by writing to
/sys/module/drm/parameters/panic_screen
Great!
v2:
* use module parameter instead of Kconfig choice
(Javier Martinez Canillas)
Signed-off-by: Jocelyn Falempe
---
drivers/gpu/drm/Kconfig | 11
drivers/gpu/drm/drm_panic.c | 108
2 files changed, 109 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 9703429de6b9..944815cee080 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -137,6 +137,17 @@ config DRM_PANIC_DEBUG
This is unsafe and should not be enabled on a production build.
If in doubt, say "N".
+config DRM_PANIC_SCREEN
+ string "Panic screen formater"
+ default "user"
+ depends on DRM_PANIC
+ help
+ This option enable to choose what will be displayed when a kernel
+ panic occurs. You can choose between "user", a short message telling
+ the user to reboot the system, or "kmsg" which will display the last
+ lines of kmsg.
Maybe I would mention here that this is only about the default, but that
can be changed using the "drm.panic_screen=" kernel cmdline parameter or
writting to the /sys/module/drm/parameters/panic_screen sysfs entry.
[...]
Done
+static void draw_panic_static_kmsg(struct drm_scanout_buffer *sb)
+{
+ u32 fg_color = convert_from_xrgb(CONFIG_DRM_PANIC_FOREGROUND_COLOR,
sb->format->format);
+ u32 bg_color = convert_from_xrgb(CONFIG_DRM_PANIC_BACKGROUND_COLOR,
sb->format->format);
+ const struct font_desc *font = get_default_font(sb->width, sb->height,
NULL, NULL);
Dan reported that get_default_font() can return NULL
+ struct drm_rect r_screen = DRM_RECT_INIT(0, 0, sb->width, sb->height);
+ struct kmsg_dump_iter iter;
+ char kmsg_buf[512];
+ size_t kmsg_len;
+ struct drm_panic_line line;
+ int yoffset = sb->height - font->height - (sb->height % font->height) /
2;
+
+ if (!font)
+ return;
+
... so you have to calculate yoffset after checking if the font is not NULL.
Yes I fixed that too.
with that fixed:
Reviewed-by: Javier Martinez Canillas
Thanks a lot.
I just pushed this series to drm-misc-next.
Best regards,
--
Jocelyn
Re: [PATCH v2 3/3] drm/panic: Add a kmsg panic screen
Jocelyn Falempe writes:
> Add a kmsg option, which will display the last lines of kmsg,
> and should be similar to fbcon.
> Add a drm.panic_screen module parameter, so you can choose between
> the different panic screens available.
> two options currently, but more will be added later:
> * "user": a short message telling the user to reboot the machine.
> * "kmsg": fill the screen with the last lines of kmsg.
>
> You can even change it at runtime by writing to
> /sys/module/drm/parameters/panic_screen
>
Great!
> v2:
> * use module parameter instead of Kconfig choice
>(Javier Martinez Canillas)
>
> Signed-off-by: Jocelyn Falempe
> ---
> drivers/gpu/drm/Kconfig | 11
> drivers/gpu/drm/drm_panic.c | 108
> 2 files changed, 109 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 9703429de6b9..944815cee080 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -137,6 +137,17 @@ config DRM_PANIC_DEBUG
> This is unsafe and should not be enabled on a production build.
> If in doubt, say "N".
>
> +config DRM_PANIC_SCREEN
> + string "Panic screen formater"
> + default "user"
> + depends on DRM_PANIC
> + help
> + This option enable to choose what will be displayed when a kernel
> + panic occurs. You can choose between "user", a short message telling
> + the user to reboot the system, or "kmsg" which will display the last
> + lines of kmsg.
Maybe I would mention here that this is only about the default, but that
can be changed using the "drm.panic_screen=" kernel cmdline parameter or
writting to the /sys/module/drm/parameters/panic_screen sysfs entry.
[...]
> +static void draw_panic_static_kmsg(struct drm_scanout_buffer *sb)
> +{
> + u32 fg_color = convert_from_xrgb(CONFIG_DRM_PANIC_FOREGROUND_COLOR,
> sb->format->format);
> + u32 bg_color = convert_from_xrgb(CONFIG_DRM_PANIC_BACKGROUND_COLOR,
> sb->format->format);
> + const struct font_desc *font = get_default_font(sb->width, sb->height,
> NULL, NULL);
Dan reported that get_default_font() can return NULL
> + struct drm_rect r_screen = DRM_RECT_INIT(0, 0, sb->width, sb->height);
> + struct kmsg_dump_iter iter;
> + char kmsg_buf[512];
> + size_t kmsg_len;
> + struct drm_panic_line line;
> + int yoffset = sb->height - font->height - (sb->height % font->height) /
> 2;
> +
> + if (!font)
> + return;
> +
... so you have to calculate yoffset after checking if the font is not NULL.
with that fixed:
Reviewed-by: Javier Martinez Canillas
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
Re: [PATCH v2 3/3] drm/panic: Add a kmsg panic screen
Hi Jocelyn,
kernel test robot noticed the following build warnings:
url:
https://github.com/intel-lab-lkp/linux/commits/Jocelyn-Falempe/drm-panic-only-draw-the-foreground-color-in-drm_panic_blit/20240603-181247
base: 86266829ea755f737762ebda614c59b136c8feac
patch link:
https://lore.kernel.org/r/20240603095343.39588-4-jfalempe%40redhat.com
patch subject: [PATCH v2 3/3] drm/panic: Add a kmsg panic screen
config: i386-randconfig-141-20240604
(https://download.01.org/0day-ci/archive/20240604/202406041051.kuvqtzcd-...@intel.com/config)
compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot
| Reported-by: Dan Carpenter
| Closes: https://lore.kernel.org/r/202406041051.kuvqtzcd-...@intel.com/
smatch warnings:
drivers/gpu/drm/drm_panic.c:531 draw_panic_static_kmsg() warn: variable
dereferenced before check 'font' (see line 529)
vim +/font +531 drivers/gpu/drm/drm_panic.c
c259bba1e69ff2 Jocelyn Falempe 2024-06-03 519 static void
draw_panic_static_kmsg(struct drm_scanout_buffer *sb)
c259bba1e69ff2 Jocelyn Falempe 2024-06-03 520 {
c259bba1e69ff2 Jocelyn Falempe 2024-06-03 521 u32 fg_color =
convert_from_xrgb(CONFIG_DRM_PANIC_FOREGROUND_COLOR, sb->format->format);
c259bba1e69ff2 Jocelyn Falempe 2024-06-03 522 u32 bg_color =
convert_from_xrgb(CONFIG_DRM_PANIC_BACKGROUND_COLOR, sb->format->format);
c259bba1e69ff2 Jocelyn Falempe 2024-06-03 523 const struct font_desc
*font = get_default_font(sb->width, sb->height, NULL, NULL);
c259bba1e69ff2 Jocelyn Falempe 2024-06-03 524 struct drm_rect
r_screen = DRM_RECT_INIT(0, 0, sb->width, sb->height);
c259bba1e69ff2 Jocelyn Falempe 2024-06-03 525 struct kmsg_dump_iter
iter;
c259bba1e69ff2 Jocelyn Falempe 2024-06-03 526 char kmsg_buf[512];
c259bba1e69ff2 Jocelyn Falempe 2024-06-03 527 size_t kmsg_len;
c259bba1e69ff2 Jocelyn Falempe 2024-06-03 528 struct drm_panic_line
line;
c259bba1e69ff2 Jocelyn Falempe 2024-06-03 @529 int yoffset =
sb->height - font->height - (sb->height % font->height) / 2;
Unchecked dereferences
c259bba1e69ff2 Jocelyn Falempe 2024-06-03 530
c259bba1e69ff2 Jocelyn Falempe 2024-06-03 @531 if (!font)
Checked too late
c259bba1e69ff2 Jocelyn Falempe 2024-06-03 532 return;
c259bba1e69ff2 Jocelyn Falempe 2024-06-03 533
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
[PATCH v2 3/3] drm/panic: Add a kmsg panic screen
Add a kmsg option, which will display the last lines of kmsg,
and should be similar to fbcon.
Add a drm.panic_screen module parameter, so you can choose between
the different panic screens available.
two options currently, but more will be added later:
* "user": a short message telling the user to reboot the machine.
* "kmsg": fill the screen with the last lines of kmsg.
You can even change it at runtime by writing to
/sys/module/drm/parameters/panic_screen
v2:
* use module parameter instead of Kconfig choice
(Javier Martinez Canillas)
Signed-off-by: Jocelyn Falempe
---
drivers/gpu/drm/Kconfig | 11
drivers/gpu/drm/drm_panic.c | 108
2 files changed, 109 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 9703429de6b9..944815cee080 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -137,6 +137,17 @@ config DRM_PANIC_DEBUG
This is unsafe and should not be enabled on a production build.
If in doubt, say "N".
+config DRM_PANIC_SCREEN
+ string "Panic screen formater"
+ default "user"
+ depends on DRM_PANIC
+ help
+ This option enable to choose what will be displayed when a kernel
+ panic occurs. You can choose between "user", a short message telling
+ the user to reboot the system, or "kmsg" which will display the last
+ lines of kmsg.
+ Default is "user"
+
config DRM_DEBUG_DP_MST_TOPOLOGY_REFS
bool "Enable refcount backtrace history in the DP MST helpers"
depends on STACKTRACE_SUPPORT
diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
index 5dc9e98108ed..2e11273a8ad6 100644
--- a/drivers/gpu/drm/drm_panic.c
+++ b/drivers/gpu/drm/drm_panic.c
@@ -12,6 +12,7 @@
#include
#include
#include
+#include
#include
#include
@@ -27,6 +28,12 @@ MODULE_AUTHOR("Jocelyn Falempe");
MODULE_DESCRIPTION("DRM panic handler");
MODULE_LICENSE("GPL");
+static char drm_panic_screen[16] = CONFIG_DRM_PANIC_SCREEN;
+module_param_string(panic_screen, drm_panic_screen, sizeof(drm_panic_screen),
0644);
+MODULE_PARM_DESC(panic_screen,
+"Choose what will be displayed by drm_panic, 'user' or 'kmsg'
[default="
+CONFIG_DRM_PANIC_SCREEN "]");
+
/**
* DOC: overview
*
@@ -437,24 +444,18 @@ static void draw_txt_rectangle(struct drm_scanout_buffer
*sb,
}
}
-/*
- * Draw the panic message at the center of the screen
- */
-static void draw_panic_static(struct drm_scanout_buffer *sb)
+static void draw_panic_static_user(struct drm_scanout_buffer *sb)
{
size_t msg_lines = ARRAY_SIZE(panic_msg);
size_t logo_lines = ARRAY_SIZE(logo);
- u32 fg_color = CONFIG_DRM_PANIC_FOREGROUND_COLOR;
- u32 bg_color = CONFIG_DRM_PANIC_BACKGROUND_COLOR;
+ u32 fg_color = convert_from_xrgb(CONFIG_DRM_PANIC_FOREGROUND_COLOR,
sb->format->format);
+ u32 bg_color = convert_from_xrgb(CONFIG_DRM_PANIC_BACKGROUND_COLOR,
sb->format->format);
const struct font_desc *font = get_default_font(sb->width, sb->height,
NULL, NULL);
struct drm_rect r_screen, r_logo, r_msg;
if (!font)
return;
- fg_color = convert_from_xrgb(fg_color, sb->format->format);
- bg_color = convert_from_xrgb(bg_color, sb->format->format);
-
r_screen = DRM_RECT_INIT(0, 0, sb->width, sb->height);
r_logo = DRM_RECT_INIT(0, 0,
@@ -477,6 +478,84 @@ static void draw_panic_static(struct drm_scanout_buffer
*sb)
draw_txt_rectangle(sb, font, panic_msg, msg_lines, true, &r_msg,
fg_color);
}
+
+/*
+ * Draw one line of kmsg, and handle wrapping if it won't fit in the screen
width.
+ * Return the y-offset of the next line.
+ */
+static int draw_line_with_wrap(struct drm_scanout_buffer *sb, const struct
font_desc *font,
+ struct drm_panic_line *line, int yoffset, u32
fg_color)
+{
+ int chars_per_row = sb->width / font->width;
+ struct drm_rect r_txt = DRM_RECT_INIT(0, yoffset, sb->width,
sb->height);
+ struct drm_panic_line line_wrap;
+
+ if (line->len > chars_per_row) {
+ line_wrap.len = line->len % chars_per_row;
+ line_wrap.txt = line->txt + line->len - line_wrap.len;
+ draw_txt_rectangle(sb, font, &line_wrap, 1, false, &r_txt,
fg_color);
+ r_txt.y1 -= font->height;
+ if (r_txt.y1 < 0)
+ return r_txt.y1;
+ while (line_wrap.txt > line->txt) {
+ line_wrap.txt -= chars_per_row;
+ line_wrap.len = chars_per_row;
+ draw_txt_rectangle(sb, font, &line_wrap, 1, false,
&r_txt, fg_color);
+ r_txt.y1 -= font->height;
+ if (r_txt.y1 < 0)
+ return r_txt.y1;
+ }
+ } else
