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

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

[Jocelyn Falempe]

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.

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

[PATCH 1/2] drm/panic: Add a drm panic handler

[Jocelyn Falempe]

This module displays a user friendly message when a kernel panic
occurs. It currently doesn't contain any debug information,
but that can be added later.

Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
---
 drivers/gpu/drm/Kconfig     |  11 ++
 drivers/gpu/drm/Makefile    |   1 +
 drivers/gpu/drm/drm_drv.c   |   3 +
 drivers/gpu/drm/drm_panic.c | 286 ++++++++++++++++++++++++++++++++++++
 include/drm/drm_panic.h     |  26 ++++
 5 files changed, 327 insertions(+)
 create mode 100644 drivers/gpu/drm/drm_panic.c
 create mode 100644 include/drm/drm_panic.h

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index ba3fb04bb691..cc6771351d6d 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -98,6 +98,17 @@ config DRM_KMS_HELPER
 	help
 	  CRTC helpers for KMS drivers.
 
+config DRM_PANIC
+	bool "Display a user-friendly message when a kernel panic occurs"
+	depends on DRM && !FRAMEBUFFER_CONSOLE
+	select FONT_SUPPORT
+	default y if DRM_SIMPLEDRM
+	help
+	  Enable a drm panic handler, which will display a user-friendly message
+	  when a kernel panic occurs. It's useful when using a user-space
+	  console instead of fbcon.
+	  It will only work if your graphic driver supports this feature.
+
 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/Makefile b/drivers/gpu/drm/Makefile
index a33257d2bc7f..c73dbac9c01e 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -72,6 +72,7 @@ drm-$(CONFIG_DRM_PRIVACY_SCREEN) += \
 	drm_privacy_screen_x86.o
 drm-$(CONFIG_DRM_ACCEL) += ../../accel/drm_accel.o
 obj-$(CONFIG_DRM)	+= drm.o
+drm-$(CONFIG_DRM_PANIC) += drm_panic.o
 
 obj-$(CONFIG_DRM_PANEL_ORIENTATION_QUIRKS) += drm_panel_orientation_quirks.o
 
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index cee0cc522ed9..0f2d96edd86d 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -45,6 +45,7 @@
 #include <drm/drm_mode_object.h>
 #include <drm/drm_print.h>
 #include <drm/drm_privacy_screen_machine.h>
+#include <drm/drm_panic.h>
 
 #include "drm_crtc_internal.h"
 #include "drm_internal.h"
@@ -1065,6 +1066,7 @@ static void drm_core_exit(void)
 	unregister_chrdev(DRM_MAJOR, "drm");
 	debugfs_remove(drm_debugfs_root);
 	drm_sysfs_destroy();
+	drm_panic_exit();
 	idr_destroy(&drm_minors_idr);
 	drm_connector_ida_destroy();
 }
@@ -1076,6 +1078,7 @@ static int __init drm_core_init(void)
 	drm_connector_ida_init();
 	idr_init(&drm_minors_idr);
 	drm_memcpy_init_early();
+	drm_panic_init();
 
 	ret = drm_sysfs_init();
 	if (ret < 0) {
diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
new file mode 100644
index 000000000000..538ca5daaebf
--- /dev/null
+++ b/drivers/gpu/drm/drm_panic.c
@@ -0,0 +1,286 @@
+// SPDX-License-Identifier: GPL-2.0 or MIT
+/*
+ * Copyright (c) 2023 Jocelyn Falempe <jfalempe@redhat.com>
+ * inspired by the drm_log driver from David Herrmann <dh.herrmann@gmail.com>
+ * Tux Ascii art taken from cowsay written by Tony Monroe
+ */
+
+#include <linux/font.h>
+#include <linux/iosys-map.h>
+#include <linux/kdebug.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/panic_notifier.h>
+#include <linux/types.h>
+#include <linux/slab.h>
+
+#include <drm/drm_client.h>
+#include <drm/drm_fourcc.h>
+#include <drm/drm_framebuffer.h>
+#include <drm/drm_print.h>
+
+MODULE_AUTHOR("Jocelyn Falempe");
+MODULE_DESCRIPTION("DRM PANIC");
+MODULE_LICENSE("GPL");
+
+/**
+ * This module displays a user friendly message on screen when a kernel panic
+ * occurs.
+ * It will display only one frame, so just clear it, and draw white pixels for
+ * the characters. Performance optimizations are low priority as the machine is
+ * already in an unusable state.
+ */
+
+/**
+ * List of drm clients
+ */
+struct dpanic_drm_client {
+	struct list_head head;
+	struct drm_client_dev client;
+};
+
+struct dpanic_line {
+	u32 len;
+	const char *txt;
+};
+
+#define PANIC_LINE(s) {.len = sizeof(s), .txt = s}
+
+const struct dpanic_line panic_msg[] = {
+	PANIC_LINE("KERNEL PANIC !"),
+	PANIC_LINE(""),
+	PANIC_LINE("Please reboot your computer.")
+};
+
+const struct dpanic_line logo[] = {
+	PANIC_LINE("     .--."),
+	PANIC_LINE("    |o_o |"),
+	PANIC_LINE("    |:_/ |"),
+	PANIC_LINE("   //   \\ \\"),
+	PANIC_LINE("  (|     | )"),
+	PANIC_LINE(" /'\\_   _/`\\"),
+	PANIC_LINE(" \\___)=(___/"),
+};
+
+static LIST_HEAD(dpanic_clients);
+static DEFINE_MUTEX(dpanic_lock);
+
+#define IOSYS_WRITE8(offset, val) iosys_map_wr(screen_base, offset, u8, val)
+/*
+ * Only handle DRM_FORMAT_XRGB8888 for testing
+ */
+static inline void dpanic_draw_px(struct iosys_map *screen_base, size_t offset, u32 pixel_format,
+				  u8 r, u8 g, u8 b)
+{
+	switch (pixel_format) {
+	case DRM_FORMAT_XRGB8888:
+		IOSYS_WRITE8(offset++, b);
+		IOSYS_WRITE8(offset++, g);
+		IOSYS_WRITE8(offset++, r);
+		IOSYS_WRITE8(offset++, 0xff);
+		break;
+	default:
+		pr_err("Format not supported\n");
+		break;
+	/* TODO other format */
+	}
+}
+
+/* Draw a single character at given destination */
+static void dpanic_draw_char(char ch, size_t x, size_t y,
+			     struct drm_framebuffer *fb,
+			     struct iosys_map *map,
+			     const struct font_desc *font)
+{
+	size_t src_width, src_height, src_stride, i, dst_off;
+	const u8 *src;
+
+	src_width = font->width;
+	src_height = font->height;
+	src_stride = DIV_ROUND_UP(src_width, 8);
+
+	dst_off = x * font->width * fb->format->cpp[0] + y * font->height * fb->pitches[0];
+
+	src = font->data;
+	src += ch * src_height * src_stride;
+
+	while (src_height--) {
+		for (i = 0; i < src_width; ++i) {
+			/* only draw white pixels */
+			if (src[i / 8] & (0x80 >> (i % 8)))
+				dpanic_draw_px(map, dst_off + i * fb->format->cpp[0],
+					       fb->format->format, 0xff, 0xff, 0xff);
+		}
+		src += src_stride;
+		dst_off += fb->pitches[0];
+	}
+}
+
+static void dpanic_draw_line_centered(const struct dpanic_line *line, size_t y,
+				      struct drm_framebuffer *fb,
+				      struct iosys_map *map,
+				      const struct font_desc *font)
+{
+	size_t chars_per_line = fb->width / font->width;
+	size_t skip_left, x;
+
+	if (line->len > chars_per_line)
+		return;
+
+	skip_left = (chars_per_line - line->len) / 2;
+
+	for (x = 0; x < line->len; x++)
+		dpanic_draw_char(line->txt[x], skip_left + x, y, fb, map, font);
+}
+
+/*
+ * Draw the Tux logo at the upper left corner
+ */
+static void dpanic_draw_logo(struct drm_framebuffer *fb,
+			     struct iosys_map *map,
+			     const struct font_desc *font)
+{
+	size_t chars_per_line = fb->width / font->width;
+	size_t x, y;
+
+	for (y = 0; y < ARRAY_SIZE(logo); y++)
+		for (x = 0; x < logo[y].len && x < chars_per_line; x++)
+			dpanic_draw_char(logo[y].txt[x], x, y, fb, map, font);
+}
+
+/*
+ * Draw the panic message at the center of the screen
+ */
+static void dpanic_static_draw(struct drm_client_buffer *buffer)
+{
+	size_t y, lines, skip_top;
+	size_t len = ARRAY_SIZE(panic_msg);
+	struct iosys_map map;
+	struct drm_framebuffer *fb = buffer->fb;
+	const struct font_desc *font = get_default_font(fb->width, fb->height, 0x8080, 0x8080);
+
+	if (!font)
+		return;
+
+	if (drm_client_buffer_vmap(buffer, &map)) {
+		pr_err("VMAP failed\n");
+		return;
+	}
+	lines = fb->height / font->height;
+	skip_top = (lines - len) / 2;
+
+	/* clear screen */
+	iosys_map_memset(&map, 0, 0, fb->height * fb->pitches[0]);
+
+	for (y = 0; y < len; y++)
+		dpanic_draw_line_centered(&panic_msg[y], y + skip_top, fb, &map, font);
+
+	if (skip_top >= ARRAY_SIZE(logo))
+		dpanic_draw_logo(fb, &map, font);
+
+	drm_client_framebuffer_flush(buffer, NULL);
+}
+
+#define MAX_MODESET 8
+
+static void drm_panic_client(struct drm_client_dev *client)
+{
+	struct drm_client_buffer *buffer[MAX_MODESET];
+	int ret, n_modeset, i;
+	struct drm_mode_set *mode_set;
+
+	ret = drm_client_modeset_probe(client, 0, 0);
+	if (ret)
+		return;
+
+	n_modeset = 0;
+	drm_client_for_each_modeset(mode_set, client) {
+		struct drm_plane *primary = mode_set->crtc->primary;
+		struct drm_framebuffer *fb;
+
+		if (primary->state && primary->state->fb)
+			fb = primary->state->fb;
+		else if (primary->fb)
+			fb = primary->fb;
+		else
+			continue;
+
+		pr_debug("DRM Panic, FB width %d, height %d\n", fb->width, fb->height);
+		buffer[n_modeset] = drm_client_framebuffer_create(client, fb->width,
+								  fb->height,
+								  fb->format->format);
+
+		if (IS_ERR(buffer[n_modeset])) {
+			pr_err("DRM Panic can't allocate buffer\n");
+			continue;
+		}
+		mode_set->fb = buffer[n_modeset]->fb;
+		n_modeset++;
+		if (n_modeset == MAX_MODESET)
+			break;
+	}
+	ret = drm_client_modeset_commit_locked(client);
+	if (ret)
+		return;
+
+	for (i = 0; i < n_modeset; i++)
+		dpanic_static_draw(buffer[i]);
+}
+
+static int drm_panic(struct notifier_block *this, unsigned long event,
+		     void *ptr)
+{
+	struct dpanic_drm_client *dpanic_client;
+
+	list_for_each_entry(dpanic_client, &dpanic_clients, head) {
+		drm_panic_client(&dpanic_client->client);
+	}
+	return NOTIFY_OK;
+}
+
+struct notifier_block drm_panic_notifier = {
+	.notifier_call = drm_panic,
+};
+
+void drm_panic_init_client(struct drm_device *dev)
+{
+	struct dpanic_drm_client *new;
+	int ret;
+
+	new = kzalloc(sizeof(*new), GFP_KERNEL);
+	if (!new)
+		return;
+
+	ret = drm_client_init(dev, &new->client, "drm_panic", NULL);
+	if (ret < 0) {
+		kfree(new);
+		return;
+	}
+	drm_client_register(&new->client);
+	list_add_tail(&new->head, &dpanic_clients);
+
+	drm_info(dev, "Registered with drm panic\n");
+}
+EXPORT_SYMBOL(drm_panic_init_client);
+
+/**
+ * drm_panic_init() - Initialize drm-panic subsystem
+ *
+ * register the panic notifier
+ */
+void drm_panic_init(void)
+{
+	/* register panic handler */
+	atomic_notifier_chain_register(&panic_notifier_list,
+				       &drm_panic_notifier);
+}
+
+/**
+ * drm_log_exit() - Shutdown drm-panic subsystem
+ */
+void drm_panic_exit(void)
+{
+	atomic_notifier_chain_unregister(&panic_notifier_list,
+					 &drm_panic_notifier);
+}
diff --git a/include/drm/drm_panic.h b/include/drm/drm_panic.h
new file mode 100644
index 000000000000..a943b82324e3
--- /dev/null
+++ b/include/drm/drm_panic.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0 or MIT */
+#ifndef __DRM_PANIC_H__
+#define __DRM_PANIC_H__
+
+/*
+ * Copyright (c) 2023 Jocelyn Falempe <jfalempe@redhat.com>
+ */
+
+#include <linux/module.h>
+#include <linux/types.h>
+#include <linux/iosys-map.h>
+
+#ifdef CONFIG_DRM_PANIC
+
+void drm_panic_init(void);
+void drm_panic_exit(void);
+
+void drm_panic_init_client(struct drm_device *dev);
+#else
+
+static inline void drm_panic_init(void) {}
+static inline void drm_panic_exit(void) {}
+static inline void drm_panic_init_client(struct drm_device *dev) {}
+#endif
+
+#endif /* __DRM_LOG_H__ */
-- 
2.41.0

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

[kernel test robot]

Hi Jocelyn,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 6995e2de6891c724bfeb2db33d7b87775f913ad1]

url:    https://github.com/intel-lab-lkp/linux/commits/Jocelyn-Falempe/drm-panic-Add-a-drm-panic-handler/20230810-032733
base:   6995e2de6891c724bfeb2db33d7b87775f913ad1
patch link:    https://lore.kernel.org/r/20230809192514.158062-2-jfalempe%40redhat.com
patch subject: [PATCH 1/2] drm/panic: Add a drm panic handler
config: nios2-randconfig-r011-20230809 (https://download.01.org/0day-ci/archive/20230810/202308101307.hbEyAamN-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230810/202308101307.hbEyAamN-lkp@intel.com/reproduce)

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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202308101307.hbEyAamN-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/gpu/drm/drm_panic.c:246:6: warning: no previous prototype for 'drm_panic_init_client' [-Wmissing-prototypes]
     246 | void drm_panic_init_client(struct drm_device *dev)
         |      ^~~~~~~~~~~~~~~~~~~~~
>> drivers/gpu/drm/drm_panic.c:272:6: warning: no previous prototype for 'drm_panic_init' [-Wmissing-prototypes]
     272 | void drm_panic_init(void)
         |      ^~~~~~~~~~~~~~
>> drivers/gpu/drm/drm_panic.c:282:6: warning: no previous prototype for 'drm_panic_exit' [-Wmissing-prototypes]
     282 | void drm_panic_exit(void)
         |      ^~~~~~~~~~~~~~
--
>> drivers/gpu/drm/drm_panic.c:28: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
    * This module displays a user friendly message on screen when a kernel panic
   drivers/gpu/drm/drm_panic.c:36: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
    * List of drm clients
>> drivers/gpu/drm/drm_panic.c:283: warning: expecting prototype for drm_log_exit(). Prototype was for drm_panic_exit() instead


vim +/drm_panic_init_client +246 drivers/gpu/drm/drm_panic.c

   245	
 > 246	void drm_panic_init_client(struct drm_device *dev)
   247	{
   248		struct dpanic_drm_client *new;
   249		int ret;
   250	
   251		new = kzalloc(sizeof(*new), GFP_KERNEL);
   252		if (!new)
   253			return;
   254	
   255		ret = drm_client_init(dev, &new->client, "drm_panic", NULL);
   256		if (ret < 0) {
   257			kfree(new);
   258			return;
   259		}
   260		drm_client_register(&new->client);
   261		list_add_tail(&new->head, &dpanic_clients);
   262	
   263		drm_info(dev, "Registered with drm panic\n");
   264	}
   265	EXPORT_SYMBOL(drm_panic_init_client);
   266	
   267	/**
   268	 * drm_panic_init() - Initialize drm-panic subsystem
   269	 *
   270	 * register the panic notifier
   271	 */
 > 272	void drm_panic_init(void)
   273	{
   274		/* register panic handler */
   275		atomic_notifier_chain_register(&panic_notifier_list,
   276					       &drm_panic_notifier);
   277	}
   278	
   279	/**
   280	 * drm_log_exit() - Shutdown drm-panic subsystem
   281	 */
 > 282	void drm_panic_exit(void)
 > 283	{

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

[kernel test robot]

Hi Jocelyn,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 6995e2de6891c724bfeb2db33d7b87775f913ad1]

url:    https://github.com/intel-lab-lkp/linux/commits/Jocelyn-Falempe/drm-panic-Add-a-drm-panic-handler/20230810-032733
base:   6995e2de6891c724bfeb2db33d7b87775f913ad1
patch link:    https://lore.kernel.org/r/20230809192514.158062-2-jfalempe%40redhat.com
patch subject: [PATCH 1/2] drm/panic: Add a drm panic handler
config: nios2-randconfig-r011-20230809 (https://download.01.org/0day-ci/archive/20230810/202308101307.hbEyAamN-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230810/202308101307.hbEyAamN-lkp@intel.com/reproduce)

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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202308101307.hbEyAamN-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/gpu/drm/drm_panic.c:246:6: warning: no previous prototype for 'drm_panic_init_client' [-Wmissing-prototypes]
     246 | void drm_panic_init_client(struct drm_device *dev)
         |      ^~~~~~~~~~~~~~~~~~~~~
>> drivers/gpu/drm/drm_panic.c:272:6: warning: no previous prototype for 'drm_panic_init' [-Wmissing-prototypes]
     272 | void drm_panic_init(void)
         |      ^~~~~~~~~~~~~~
>> drivers/gpu/drm/drm_panic.c:282:6: warning: no previous prototype for 'drm_panic_exit' [-Wmissing-prototypes]
     282 | void drm_panic_exit(void)
         |      ^~~~~~~~~~~~~~
--
>> drivers/gpu/drm/drm_panic.c:28: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
    * This module displays a user friendly message on screen when a kernel panic
   drivers/gpu/drm/drm_panic.c:36: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
    * List of drm clients
>> drivers/gpu/drm/drm_panic.c:283: warning: expecting prototype for drm_log_exit(). Prototype was for drm_panic_exit() instead


vim +/drm_panic_init_client +246 drivers/gpu/drm/drm_panic.c

   245	
 > 246	void drm_panic_init_client(struct drm_device *dev)
   247	{
   248		struct dpanic_drm_client *new;
   249		int ret;
   250	
   251		new = kzalloc(sizeof(*new), GFP_KERNEL);
   252		if (!new)
   253			return;
   254	
   255		ret = drm_client_init(dev, &new->client, "drm_panic", NULL);
   256		if (ret < 0) {
   257			kfree(new);
   258			return;
   259		}
   260		drm_client_register(&new->client);
   261		list_add_tail(&new->head, &dpanic_clients);
   262	
   263		drm_info(dev, "Registered with drm panic\n");
   264	}
   265	EXPORT_SYMBOL(drm_panic_init_client);
   266	
   267	/**
   268	 * drm_panic_init() - Initialize drm-panic subsystem
   269	 *
   270	 * register the panic notifier
   271	 */
 > 272	void drm_panic_init(void)
   273	{
   274		/* register panic handler */
   275		atomic_notifier_chain_register(&panic_notifier_list,
   276					       &drm_panic_notifier);
   277	}
   278	
   279	/**
   280	 * drm_log_exit() - Shutdown drm-panic subsystem
   281	 */
 > 282	void drm_panic_exit(void)
 > 283	{

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

[kernel test robot]

Hi Jocelyn,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 6995e2de6891c724bfeb2db33d7b87775f913ad1]

url:    https://github.com/intel-lab-lkp/linux/commits/Jocelyn-Falempe/drm-panic-Add-a-drm-panic-handler/20230810-032733
base:   6995e2de6891c724bfeb2db33d7b87775f913ad1
patch link:    https://lore.kernel.org/r/20230809192514.158062-2-jfalempe%40redhat.com
patch subject: [PATCH 1/2] drm/panic: Add a drm panic handler
config: s390-randconfig-r044-20230809 (https://download.01.org/0day-ci/archive/20230810/202308101423.GSvLgbbe-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce: (https://download.01.org/0day-ci/archive/20230810/202308101423.GSvLgbbe-lkp@intel.com/reproduce)

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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202308101423.GSvLgbbe-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/gpu/drm/drm_panic.c:9:
   In file included from include/linux/iosys-map.h:10:
   In file included from include/linux/io.h:13:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     547 |         val = __raw_readb(PCI_IOBASE + addr);
         |                           ~~~~~~~~~~ ^
   include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     560 |         val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:37:59: note: expanded from macro '__le16_to_cpu'
      37 | #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
         |                                                           ^
   include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
     102 | #define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
         |                                                      ^
   In file included from drivers/gpu/drm/drm_panic.c:9:
   In file included from include/linux/iosys-map.h:10:
   In file included from include/linux/io.h:13:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     573 |         val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:35:59: note: expanded from macro '__le32_to_cpu'
      35 | #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
         |                                                           ^
   include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32'
     115 | #define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
         |                                                      ^
   In file included from drivers/gpu/drm/drm_panic.c:9:
   In file included from include/linux/iosys-map.h:10:
   In file included from include/linux/io.h:13:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     584 |         __raw_writeb(value, PCI_IOBASE + addr);
         |                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     594 |         __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     604 |         __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:692:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     692 |         readsb(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:700:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     700 |         readsw(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:708:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     708 |         readsl(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:717:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     717 |         writesb(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
   include/asm-generic/io.h:726:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     726 |         writesw(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
   include/asm-generic/io.h:735:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     735 |         writesl(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
>> drivers/gpu/drm/drm_panic.c:246:6: warning: no previous prototype for function 'drm_panic_init_client' [-Wmissing-prototypes]
     246 | void drm_panic_init_client(struct drm_device *dev)
         |      ^
   drivers/gpu/drm/drm_panic.c:246:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
     246 | void drm_panic_init_client(struct drm_device *dev)
         | ^
         | static 
>> drivers/gpu/drm/drm_panic.c:272:6: warning: no previous prototype for function 'drm_panic_init' [-Wmissing-prototypes]
     272 | void drm_panic_init(void)
         |      ^
   drivers/gpu/drm/drm_panic.c:272:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
     272 | void drm_panic_init(void)
         | ^
         | static 
>> drivers/gpu/drm/drm_panic.c:282:6: warning: no previous prototype for function 'drm_panic_exit' [-Wmissing-prototypes]
     282 | void drm_panic_exit(void)
         |      ^
   drivers/gpu/drm/drm_panic.c:282:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
     282 | void drm_panic_exit(void)
         | ^
         | static 
   15 warnings generated.
--
>> drivers/gpu/drm/drm_panic.c:28: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
    * This module displays a user friendly message on screen when a kernel panic
   drivers/gpu/drm/drm_panic.c:36: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
    * List of drm clients
>> drivers/gpu/drm/drm_panic.c:283: warning: expecting prototype for drm_log_exit(). Prototype was for drm_panic_exit() instead


vim +/drm_panic_init_client +246 drivers/gpu/drm/drm_panic.c

   245	
 > 246	void drm_panic_init_client(struct drm_device *dev)
   247	{
   248		struct dpanic_drm_client *new;
   249		int ret;
   250	
   251		new = kzalloc(sizeof(*new), GFP_KERNEL);
   252		if (!new)
   253			return;
   254	
   255		ret = drm_client_init(dev, &new->client, "drm_panic", NULL);
   256		if (ret < 0) {
   257			kfree(new);
   258			return;
   259		}
   260		drm_client_register(&new->client);
   261		list_add_tail(&new->head, &dpanic_clients);
   262	
   263		drm_info(dev, "Registered with drm panic\n");
   264	}
   265	EXPORT_SYMBOL(drm_panic_init_client);
   266	
   267	/**
   268	 * drm_panic_init() - Initialize drm-panic subsystem
   269	 *
   270	 * register the panic notifier
   271	 */
 > 272	void drm_panic_init(void)
   273	{
   274		/* register panic handler */
   275		atomic_notifier_chain_register(&panic_notifier_list,
   276					       &drm_panic_notifier);
   277	}
   278	
   279	/**
   280	 * drm_log_exit() - Shutdown drm-panic subsystem
   281	 */
 > 282	void drm_panic_exit(void)
 > 283	{

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

[kernel test robot]

Hi Jocelyn,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 6995e2de6891c724bfeb2db33d7b87775f913ad1]

url:    https://github.com/intel-lab-lkp/linux/commits/Jocelyn-Falempe/drm-panic-Add-a-drm-panic-handler/20230810-032733
base:   6995e2de6891c724bfeb2db33d7b87775f913ad1
patch link:    https://lore.kernel.org/r/20230809192514.158062-2-jfalempe%40redhat.com
patch subject: [PATCH 1/2] drm/panic: Add a drm panic handler
config: s390-randconfig-r044-20230809 (https://download.01.org/0day-ci/archive/20230810/202308101423.GSvLgbbe-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce: (https://download.01.org/0day-ci/archive/20230810/202308101423.GSvLgbbe-lkp@intel.com/reproduce)

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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202308101423.GSvLgbbe-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/gpu/drm/drm_panic.c:9:
   In file included from include/linux/iosys-map.h:10:
   In file included from include/linux/io.h:13:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     547 |         val = __raw_readb(PCI_IOBASE + addr);
         |                           ~~~~~~~~~~ ^
   include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     560 |         val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:37:59: note: expanded from macro '__le16_to_cpu'
      37 | #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
         |                                                           ^
   include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
     102 | #define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
         |                                                      ^
   In file included from drivers/gpu/drm/drm_panic.c:9:
   In file included from include/linux/iosys-map.h:10:
   In file included from include/linux/io.h:13:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     573 |         val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:35:59: note: expanded from macro '__le32_to_cpu'
      35 | #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
         |                                                           ^
   include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32'
     115 | #define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
         |                                                      ^
   In file included from drivers/gpu/drm/drm_panic.c:9:
   In file included from include/linux/iosys-map.h:10:
   In file included from include/linux/io.h:13:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     584 |         __raw_writeb(value, PCI_IOBASE + addr);
         |                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     594 |         __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     604 |         __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:692:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     692 |         readsb(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:700:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     700 |         readsw(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:708:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     708 |         readsl(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:717:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     717 |         writesb(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
   include/asm-generic/io.h:726:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     726 |         writesw(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
   include/asm-generic/io.h:735:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     735 |         writesl(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
>> drivers/gpu/drm/drm_panic.c:246:6: warning: no previous prototype for function 'drm_panic_init_client' [-Wmissing-prototypes]
     246 | void drm_panic_init_client(struct drm_device *dev)
         |      ^
   drivers/gpu/drm/drm_panic.c:246:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
     246 | void drm_panic_init_client(struct drm_device *dev)
         | ^
         | static 
>> drivers/gpu/drm/drm_panic.c:272:6: warning: no previous prototype for function 'drm_panic_init' [-Wmissing-prototypes]
     272 | void drm_panic_init(void)
         |      ^
   drivers/gpu/drm/drm_panic.c:272:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
     272 | void drm_panic_init(void)
         | ^
         | static 
>> drivers/gpu/drm/drm_panic.c:282:6: warning: no previous prototype for function 'drm_panic_exit' [-Wmissing-prototypes]
     282 | void drm_panic_exit(void)
         |      ^
   drivers/gpu/drm/drm_panic.c:282:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
     282 | void drm_panic_exit(void)
         | ^
         | static 
   15 warnings generated.
--
>> drivers/gpu/drm/drm_panic.c:28: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
    * This module displays a user friendly message on screen when a kernel panic
   drivers/gpu/drm/drm_panic.c:36: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
    * List of drm clients
>> drivers/gpu/drm/drm_panic.c:283: warning: expecting prototype for drm_log_exit(). Prototype was for drm_panic_exit() instead


vim +/drm_panic_init_client +246 drivers/gpu/drm/drm_panic.c

   245	
 > 246	void drm_panic_init_client(struct drm_device *dev)
   247	{
   248		struct dpanic_drm_client *new;
   249		int ret;
   250	
   251		new = kzalloc(sizeof(*new), GFP_KERNEL);
   252		if (!new)
   253			return;
   254	
   255		ret = drm_client_init(dev, &new->client, "drm_panic", NULL);
   256		if (ret < 0) {
   257			kfree(new);
   258			return;
   259		}
   260		drm_client_register(&new->client);
   261		list_add_tail(&new->head, &dpanic_clients);
   262	
   263		drm_info(dev, "Registered with drm panic\n");
   264	}
   265	EXPORT_SYMBOL(drm_panic_init_client);
   266	
   267	/**
   268	 * drm_panic_init() - Initialize drm-panic subsystem
   269	 *
   270	 * register the panic notifier
   271	 */
 > 272	void drm_panic_init(void)
   273	{
   274		/* register panic handler */
   275		atomic_notifier_chain_register(&panic_notifier_list,
   276					       &drm_panic_notifier);
   277	}
   278	
   279	/**
   280	 * drm_log_exit() - Shutdown drm-panic subsystem
   281	 */
 > 282	void drm_panic_exit(void)
 > 283	{

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH 2/2] drm/simpledrm: Add drm_panic support

[Jocelyn Falempe]

Add support for the drm_panic module, which displays a user-friendly
message to the screen when a kernel panic occurs.

Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
---
 drivers/gpu/drm/tiny/simpledrm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
index 25e11ef11c4c..4e356fb5a062 100644
--- a/drivers/gpu/drm/tiny/simpledrm.c
+++ b/drivers/gpu/drm/tiny/simpledrm.c
@@ -25,6 +25,7 @@
 #include <drm/drm_modeset_helper_vtables.h>
 #include <drm/drm_plane_helper.h>
 #include <drm/drm_probe_helper.h>
+#include <drm/drm_panic.h>
 
 #define DRIVER_NAME	"simpledrm"
 #define DRIVER_DESC	"DRM driver for simple-framebuffer platform devices"
@@ -884,6 +885,7 @@ static int simpledrm_probe(struct platform_device *pdev)
 		color_mode = sdev->format->depth; // can be 15 or 16
 
 	drm_fbdev_generic_setup(dev, color_mode);
+	drm_panic_init_client(dev);
 
 	return 0;
 }
-- 
2.41.0

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

[nerdopolis]

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
> 

Re: [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
>>
> 
> 
> 
> 

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

[nerdopolis]

On Monday, August 21, 2023 9:34:52 AM EDT Jocelyn Falempe wrote:
> 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.
> 
Alright, that makes sense. I don't have a huge variety of hardware, but I have a udl (displaylink 2) device that can be tested once ready
> > 
> > 
> > I will tweak my script a bit so I can test it more quickly in the future too.
> 
> 
> Best Regards,
> 
> > 
> > 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
> >>
> > 
> > 
> > 
> > 
> 
> 
> 

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

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 4793 bytes --]

Hi Jocelyn,

thanks for moving this effort forward. It's much appreciated. I looked 
through the patches and tried the patchset on my test machine.

Am 09.08.23 um 21:17 schrieb Jocelyn Falempe:
> 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.

Yes, that was also my first thought. I'd use an extra callback in struct 
drm_driver, like this:

struct drm_driver {
   int (*get_scanout_buffer)(/* return HW scanout */)
}

The scanout buffer would be described by kernel virtual address address, 
resolution, color format and scanline pitch. And that's what the panic 
handler uses.

Any driver implementing this interface would support the panic handler. 
If there's a concurrent display update, we'd have to synchronize.

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

The penguin was cute. :)

This only works if the display is already running. I had to start Gnome 
to set a display mode. Then let the panic handler take over the output.

But with simpledrm, we could even display a message without an output, 
as the framebuffer is always there.

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

Unregistering wouldn't be necessary with this proposed 
get_scanout_buffer. In the case of a panic, just remain silent if 
there's no driver that provides such a callback.

> 
> This is a first draft, so let me know what do you think about it.

One thing that will need serious work is the raw output. The current 
blitting for XRGB8888 is really just a quick-and-dirty hack.

I think we should try to reuse fbdev's blitting code, if possible. The 
fbdev core, helpers and console come with all the features we need. We 
really only need to make them work without the struct fb_info, which is 
a full fbdev device.

In struct fb_ops, there are callbacks for modifying the framebuffer. [1] 
They are used by fbcon foir drawing. But they operate on fb_info.

For a while I've been thinking about using something like a drawable to 
provide some abstractions:

struct drawable {
	/* store buffer parameters here */
	...

	struct drawable_funcs *funcs;
};

struct drawable_funcs {
	/* have function pointers similar to struct fb_ops */
	fill_rect()
	copy_area()
	image_blit()
};

We cannot rewrite all the existing fbdev drivers. To make this work with 
fbdev, we'd need adapter code that converts from drawable to fb_info and 
forwards to the existing helpers in fb_ops.

But for DRM's panic output, drawable_funcs would have to point to the 
scanout buffer and compatible callback funcs, for which we have 
implementations in fbdev.

We might be able to create console-like output that is independent from 
the fb_info. Hence, we could possible reuse a good chunk of the current 
panic output.

Best regards
Thomas

[1] https://elixir.bootlin.com/linux/v6.5.1/source/include/linux/fb.h#L273

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

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

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

[Jocelyn Falempe]

On 04/09/2023 16:29, Thomas Zimmermann wrote:
> Hi Jocelyn,
> 
> thanks for moving this effort forward. It's much appreciated. I looked 
> through the patches and tried the patchset on my test machine.

Thanks for taking the time to review and test it.
> 
> Am 09.08.23 um 21:17 schrieb Jocelyn Falempe:
>> 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.
> 
> Yes, that was also my first thought. I'd use an extra callback in struct 
> drm_driver, like this:
> 
> struct drm_driver {
>    int (*get_scanout_buffer)(/* return HW scanout */)
> }
> 
> The scanout buffer would be described by kernel virtual address address, 
> resolution, color format and scanline pitch. And that's what the panic 
> handler uses.

Thanks, I will try this solution. It shouldn't be too hard for simpledrm.
> 
> Any driver implementing this interface would support the panic handler. 
> If there's a concurrent display update, we'd have to synchronize.

Normally in the panic handler, the kernel is already single-threaded, so 
there won't be another task doing things in parallel. (But the GPU might 
still be busy doing other stuff, if we're in the middle of a game).
I think this drm_panic should be a "best effort", we can't guarantee the 
user will see the panic screen in all panic situations.

> 
>>
>> To test it, make sure you're using the simpledrm driver, and trigger a 
>> panic:
>> echo c > /proc/sysrq-trigger
> 
> The penguin was cute. :)
> 
> This only works if the display is already running. I had to start Gnome 
> to set a display mode. Then let the panic handler take over the output.

oh, I didn't expect this limitation. I will try to test that too. It 
might also get fixed by using the get_scanout_buffer() above.
> 
> But with simpledrm, we could even display a message without an output, 
> as the framebuffer is always there.
> 
>>
>> 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.
> 
> Unregistering wouldn't be necessary with this proposed 
> get_scanout_buffer. In the case of a panic, just remain silent if 
> there's no driver that provides such a callback.

Is there a way to loop over all drm_devices ?
otherwise drm_panic may still call get_scanout_buffer() on a freed 
device, which would be problematic.
> 
>>
>> This is a first draft, so let me know what do you think about it.
> 
> One thing that will need serious work is the raw output. The current 
> blitting for XRGB8888 is really just a quick-and-dirty hack.
> 
> I think we should try to reuse fbdev's blitting code, if possible. The 
> fbdev core, helpers and console come with all the features we need. We 
> really only need to make them work without the struct fb_info, which is 
> a full fbdev device.

I'm a bit reluctant to re-use the fbdev code, for a few reasons:
  * I want drm_panic to work if CONFIG_FB and CONFIG_DRM_FBDEV_EMULATION 
are not set.
  * drm_panic only needs two things, to clear the framebuffer, and then 
draw white pixels where needed. As the frame is static, and the amount 
of white pixels is low, that should be good enough. So copy_area() or 
image_blit() are not that useful.
  * For this particular use-case, performances are not a priority, it 
doesn't matter if it takes 10us or 100ms to draw the panic screen, as it 
will stay the same until the user reboots.
  * Some aggressive optimizations might cause issues in a panic handler, 
like if you use workqueue/tasklet.

On the other hand, writing the code for all supported formats is a bit 
tedious. drm_log [1] did it in ~300 lines, which should keep code 
duplication low.

> 
> In struct fb_ops, there are callbacks for modifying the framebuffer. [1] 
> They are used by fbcon foir drawing. But they operate on fb_info.
> 
> For a while I've been thinking about using something like a drawable to 
> provide some abstractions:
> 
> struct drawable {
>      /* store buffer parameters here */
>      ...
> 
>      struct drawable_funcs *funcs;
> };
> 
> struct drawable_funcs {
>      /* have function pointers similar to struct fb_ops */
>      fill_rect()
>      copy_area()
>      image_blit()
> };
> 
> We cannot rewrite all the existing fbdev drivers. To make this work with 
> fbdev, we'd need adapter code that converts from drawable to fb_info and 
> forwards to the existing helpers in fb_ops.
> 
> But for DRM's panic output, drawable_funcs would have to point to the 
> scanout buffer and compatible callback funcs, for which we have 
> implementations in fbdev.
> 
> We might be able to create console-like output that is independent from 
> the fb_info. Hence, we could possible reuse a good chunk of the current 
> panic output.

I think that was the goal of drm_log, but this can be done better in 
userspace, for example there is work ongoing to make plymouth display 
them during the boot [2].

For the panic screen, only the kernel can do it. I also think the 
current fbcon/kernel log is good for developer, but too technical for 
most end-user.

Best regards,

-- 

Jocelyn

[1] 
https://lore.kernel.org/all/1394131242-29567-1-git-send-email-dh.herrmann@gmail.com/

[2] https://gitlab.freedesktop.org/plymouth/plymouth/-/merge_requests/224

> 
> Best regards
> Thomas
> 
> [1] https://elixir.bootlin.com/linux/v6.5.1/source/include/linux/fb.h#L273
> 
>>
>> 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
> 

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

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 7725 bytes --]

Hi Jocelyn

Am 05.09.23 um 16:46 schrieb Jocelyn Falempe:
> On 04/09/2023 16:29, Thomas Zimmermann wrote:
>> Hi Jocelyn,
>>
>> thanks for moving this effort forward. It's much appreciated. I looked 
>> through the patches and tried the patchset on my test machine.
> 
> Thanks for taking the time to review and test it.
>>
>> Am 09.08.23 um 21:17 schrieb Jocelyn Falempe:
>>> 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.
>>
>> Yes, that was also my first thought. I'd use an extra callback in 
>> struct drm_driver, like this:
>>
>> struct drm_driver {
>>    int (*get_scanout_buffer)(/* return HW scanout */)
>> }
>>
>> The scanout buffer would be described by kernel virtual address 
>> address, resolution, color format and scanline pitch. And that's what 
>> the panic handler uses.
> 
> Thanks, I will try this solution. It shouldn't be too hard for simpledrm.
>>
>> Any driver implementing this interface would support the panic 
>> handler. If there's a concurrent display update, we'd have to 
>> synchronize.
> 
> Normally in the panic handler, the kernel is already single-threaded, so 
> there won't be another task doing things in parallel. (But the GPU might 
> still be busy doing other stuff, if we're in the middle of a game).
> I think this drm_panic should be a "best effort", we can't guarantee the 
> user will see the panic screen in all panic situations.
> 
>>
>>>
>>> To test it, make sure you're using the simpledrm driver, and trigger 
>>> a panic:
>>> echo c > /proc/sysrq-trigger
>>
>> The penguin was cute. :)
>>
>> This only works if the display is already running. I had to start 
>> Gnome to set a display mode. Then let the panic handler take over the 
>> output.
> 
> oh, I didn't expect this limitation. I will try to test that too. It 
> might also get fixed by using the get_scanout_buffer() above.

I guess it depends. We'd need a working display mode, or we'd require 
get_scanout_buffer() to set it up for us. IDK how much that is possible 
in the case of a kernel panic.

Apart from that, we should use whatever has been programmed into 
hardware. No need for DRM clients or GEM buffers, etc.

>>
>> But with simpledrm, we could even display a message without an output, 
>> as the framebuffer is always there.
>>
>>>
>>> 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.
>>
>> Unregistering wouldn't be necessary with this proposed 
>> get_scanout_buffer. In the case of a panic, just remain silent if 
>> there's no driver that provides such a callback.
> 
> Is there a way to loop over all drm_devices ?
> otherwise drm_panic may still call get_scanout_buffer() on a freed 
> device, which would be problematic.

I don't see such a list. It could be added as part of the registering 
the device.

>>
>>>
>>> This is a first draft, so let me know what do you think about it.
>>
>> One thing that will need serious work is the raw output. The current 
>> blitting for XRGB8888 is really just a quick-and-dirty hack.
>>
>> I think we should try to reuse fbdev's blitting code, if possible. The 
>> fbdev core, helpers and console come with all the features we need. We 
>> really only need to make them work without the struct fb_info, which 
>> is a full fbdev device.
> 
> I'm a bit reluctant to re-use the fbdev code, for a few reasons:
>   * I want drm_panic to work if CONFIG_FB and CONFIG_DRM_FBDEV_EMULATION 
> are not set.

The code would live in video/ and be independend from fbdev. It's quite 
a bit of refactoring, but might be worth it in the long term.

>   * drm_panic only needs two things, to clear the framebuffer, and then 
> draw white pixels where needed. As the frame is static, and the amount 
> of white pixels is low, that should be good enough. So copy_area() or 
> image_blit() are not that useful.

It's still a lot of color formats. And there's panel rotation to take 
into account. Fbcon has support for this already and knows how to get 
glyphs onto the screen. I'd expect that all this gets useful if you want 
to display a stack trace.

>   * For this particular use-case, performances are not a priority, it 
> doesn't matter if it takes 10us or 100ms to draw the panic screen, as it 
> will stay the same until the user reboots.
>   * Some aggressive optimizations might cause issues in a panic handler, 
> like if you use workqueue/tasklet.
> 
> On the other hand, writing the code for all supported formats is a bit 
> tedious. drm_log [1] did it in ~300 lines, which should keep code 
> duplication low.

I'm concerned that we'll now add yet another set format-conversion 
helpers. We already have the fbdev drawing helpers and DRM's 
format-conversion helpers. IMHO we should try to reuse more of the 
existing code. I assume that we need some extensive prototyping first to 
see how to do this correctly.

Best regards
Thomas

> 
>>
>> In struct fb_ops, there are callbacks for modifying the framebuffer. 
>> [1] They are used by fbcon foir drawing. But they operate on fb_info.
>>
>> For a while I've been thinking about using something like a drawable 
>> to provide some abstractions:
>>
>> struct drawable {
>>      /* store buffer parameters here */
>>      ...
>>
>>      struct drawable_funcs *funcs;
>> };
>>
>> struct drawable_funcs {
>>      /* have function pointers similar to struct fb_ops */
>>      fill_rect()
>>      copy_area()
>>      image_blit()
>> };
>>
>> We cannot rewrite all the existing fbdev drivers. To make this work 
>> with fbdev, we'd need adapter code that converts from drawable to 
>> fb_info and forwards to the existing helpers in fb_ops.
>>
>> But for DRM's panic output, drawable_funcs would have to point to the 
>> scanout buffer and compatible callback funcs, for which we have 
>> implementations in fbdev.
>>
>> We might be able to create console-like output that is independent 
>> from the fb_info. Hence, we could possible reuse a good chunk of the 
>> current panic output.
> 
> I think that was the goal of drm_log, but this can be done better in 
> userspace, for example there is work ongoing to make plymouth display 
> them during the boot [2].
> 
> For the panic screen, only the kernel can do it. I also think the 
> current fbcon/kernel log is good for developer, but too technical for 
> most end-user.
> 
> Best regards,
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

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

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

Hello Jocelyn and Thomas,

> Hi Jocelyn
>
> Am 05.09.23 um 16:46 schrieb Jocelyn Falempe:

[...]

>> 
>> I'm a bit reluctant to re-use the fbdev code, for a few reasons:
>>   * I want drm_panic to work if CONFIG_FB and CONFIG_DRM_FBDEV_EMULATION 
>> are not set.
>
> The code would live in video/ and be independend from fbdev. It's quite 
> a bit of refactoring, but might be worth it in the long term.
>

FWIW, I agree with Thomas here. The drawing/blitting logic seems to be
useful regardless of fbdev and moving it to video/ makes sense to me,
instead of adding yet another infrastructure to do the same that will
only be used for the DRM panic handler.

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat