[PATCH v8 3/8] drm/panic: Add debugfs entry to test without triggering panic.

[Jocelyn Falempe]

On 29/02/2024 12:21, Daniel Vetter wrote:
> On Tue, Feb 27, 2024 at 11:04:14AM +0100, Jocelyn Falempe wrote:
>> Add a debugfs file, so you can test drm_panic without freezing
>> your machine. This is unsafe, and should be enabled only for
>> developer or tester.
>>
>> to display the drm_panic screen, just run:
>> echo 1 > /sys/kernel/debug/drm_panic/trigger
>>
>> Signed-off-by: Jocelyn Falempe <jfalempe at redhat.com>
>> ---
>>   drivers/gpu/drm/Kconfig     |  9 +++++++
>>   drivers/gpu/drm/drm_panic.c | 47 +++++++++++++++++++++++++++++++++++++
>>   2 files changed, 56 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
>> index c17d8a8f6877..8dcea29f595c 100644
>> --- a/drivers/gpu/drm/Kconfig
>> +++ b/drivers/gpu/drm/Kconfig
>> @@ -125,6 +125,15 @@ config DRM_PANIC_BACKGROUND_COLOR
>>   	depends on DRM_PANIC
>>   	default 0x000000
>>   
>> +config DRM_PANIC_DEBUG
>> +	bool "Add a debug fs entry to trigger drm_panic"
>> +	depends on DRM_PANIC && DEBUG_FS
>> +	help
>> +	  Add drm_panic/trigger in the kernel debugfs, to force the panic
>> +	  handler to write the panic message to the scanout buffer. This is
>> +	  unsafe and should not be enabled on a production build.
>> +	  If in doubt, say "N".
>> +
>>   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 c9f386476ef9..c5d3f725c5f5 100644
>> --- a/drivers/gpu/drm/drm_panic.c
>> +++ b/drivers/gpu/drm/drm_panic.c
>> @@ -398,3 +398,50 @@ void drm_panic_unregister(struct drm_plane *plane)
>>   }
>>   EXPORT_SYMBOL(drm_panic_unregister);
>>   
>> +
>> +/*
>> + * DEBUG, This is currently unsafe.
>> + * Also it will call all panic_notifier, since there is no way to filter and
>> + * only call the drm_panic notifier.
>> + */
>> +#ifdef CONFIG_DRM_PANIC_DEBUG
>> +#include <linux/debugfs.h>
>> +
>> +static struct dentry *debug_dir;
>> +static struct dentry *debug_trigger;
>> +
>> +static ssize_t dbgfs_trigger_write(struct file *file, const char __user *user_buf,
>> +				   size_t count, loff_t *ppos)
>> +{
>> +	bool run;
>> +
>> +	if (kstrtobool_from_user(user_buf, count, &run) == 0 && run)
>> +		atomic_notifier_call_chain(&panic_notifier_list, 0, "Test drm panic from debugfs");
> 
> Since this is just the general panic notifier it feels very misplaced in
> the drm subsystem. I think moving that code into the core panic code makes
> a lot more sense, then we'd also have all the right people on Cc: to
> figure out how we can best recreate the correct calling context (like nmi
> context or whatever) for best case simulation of panic code. John Ogness
> definitely needs to see this and ack, wherever we put it.

I'm not sure it makes sense to test all panic notifiers at once.

So maybe I can write an atomic_notifier_call_chain_with_filter(), and 
filter on the callback address, so it will only call the drm_panic 
handlers ?

-- 

Jocelyn

> -Sima
> 
>> +	return count;
>> +}
>> +
>> +static const struct file_operations dbg_drm_panic_ops = {
>> +	.owner = THIS_MODULE,
>> +	.write = dbgfs_trigger_write,
>> +};
>> +
>> +static int __init debugfs_start(void)
>> +{
>> +	debug_dir = debugfs_create_dir("drm_panic", NULL);
>> +
>> +	if (IS_ERR(debug_dir))
>> +		return PTR_ERR(debug_dir);
>> +	debug_trigger = debugfs_create_file("trigger", 0200, debug_dir,
>> +					    NULL, &dbg_drm_panic_ops);
>> +	return 0;
>> +}
>> +
>> +static void __exit debugfs_end(void)
>> +{
>> +	debugfs_remove_recursive(debug_dir);
>> +}
>> +
>> +module_init(debugfs_start);
>> +module_exit(debugfs_end);
>> +
>> +#endif
>> -- 
>> 2.43.0
>>
>