| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 21 Jun 2024 14:56:55 +0200
From: Jocelyn Falempe <jfalempe@...hat.com>
To: Javier Martinez Canillas <javierm@...hat.com>,
linux-kernel@...r.kernel.org
Cc: Maxime Ripard <mripard@...nel.org>, Daniel Vetter <daniel@...ll.ch>,
David Airlie <airlied@...il.com>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Thomas Zimmermann <tzimmermann@...e.de>, dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH] drm/ssd130x: Add drm_panic support
On 21/06/2024 00:22, Javier Martinez Canillas wrote:
> Add support for the drm_panic infrastructure, which allows to display
> a user friendly message on the screen when a Linux kernel panic occurs.
>
> The display controller doesn't scanout the framebuffer, but instead the
> pixels are sent to the device using a transport bus. For this reason, a
> .panic_flush handler is needed to flush the panic image to the display.
Thanks for this patch, that's really cool that drm_panic can work on
this device too.
>
> Signed-off-by: Javier Martinez Canillas <javierm@...hat.com>
> ---
>
> drivers/gpu/drm/solomon/ssd130x.c | 64 +++++++++++++++++++++++++++++++
> 1 file changed, 64 insertions(+)
>
> diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd130x.c
> index 6f51bcf774e2..0bac97bd39b9 100644
> --- a/drivers/gpu/drm/solomon/ssd130x.c
> +++ b/drivers/gpu/drm/solomon/ssd130x.c
> @@ -32,6 +32,7 @@
> #include <drm/drm_managed.h>
> #include <drm/drm_modes.h>
> #include <drm/drm_rect.h>
> +#include <drm/drm_panic.h>
> #include <drm/drm_probe_helper.h>
>
> #include "ssd130x.h"
> @@ -1386,6 +1387,63 @@ static void ssd133x_primary_plane_atomic_disable(struct drm_plane *plane,
> drm_dev_exit(idx);
> }
>
> +static int ssd130x_primary_plane_helper_get_scanout_buffer(struct drm_plane *plane,
> + struct drm_scanout_buffer *sb)
> +{
> + struct drm_plane_state *plane_state = plane->state;
> + struct drm_shadow_plane_state *shadow_plane_state;
> +
> + if (!plane_state || !plane_state->fb || !plane_state->crtc)
> + return -EINVAL;
> +
> + shadow_plane_state = to_drm_shadow_plane_state(plane_state);
> +
> + sb->format = plane->state->fb->format;
> + sb->width = plane->state->fb->width;
> + sb->height = plane->state->fb->height;
> + sb->pitch[0] = plane->state->fb->pitches[0];
> + sb->map[0] = shadow_plane_state->data[0];
> +
> + return 0;
> +}
> +
> +static void ssd130x_primary_plane_helper_panic_flush(struct drm_plane *plane)
> +{
> + struct drm_plane_state *plane_state = plane->state;
> + struct ssd130x_plane_state *ssd130x_plane_state = to_ssd130x_plane_state(plane_state);
> + struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(plane_state);
> + struct drm_crtc *crtc = plane_state->crtc;
> + struct ssd130x_crtc_state *ssd130x_crtc_state = to_ssd130x_crtc_state(crtc->state);
> +
> + ssd130x_fb_blit_rect(plane_state->fb, &shadow_plane_state->data[0], &plane_state->dst,
> + ssd130x_plane_state->buffer, ssd130x_crtc_state->data_array,
> + &shadow_plane_state->fmtcnv_state);
ssd130x_fb_blit_rect() will call regmap->write(), which involve mutex
and might sleep. And if the mutex is taken when the panic occurs, it
might deadlock the panic handling.
One solution would be to configure the regmap with config->fast_io and
config->use_raw_spinlock, and check that the lock is available with
try_lock(map->raw_spin_lock)
But that means it will waste cpu cycle with busy waiting for normal
operation, which is not good.
So for this particular device, I think it's ok, because it's unlikely
you'll run kdump or other kernel panic handlers.
But I would like to know what others think about it, and if it's
acceptable or not.
--
Jocelyn
> +}
> +
> +static void ssd132x_primary_plane_helper_panic_flush(struct drm_plane *plane)
> +{
> + struct drm_plane_state *plane_state = plane->state;
> + struct ssd130x_plane_state *ssd130x_plane_state = to_ssd130x_plane_state(plane_state);
> + struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(plane_state);
> + struct drm_crtc *crtc = plane_state->crtc;
> + struct ssd130x_crtc_state *ssd130x_crtc_state = to_ssd130x_crtc_state(crtc->state);
> +
> + ssd132x_fb_blit_rect(plane_state->fb, &shadow_plane_state->data[0], &plane_state->dst,
> + ssd130x_plane_state->buffer, ssd130x_crtc_state->data_array,
> + &shadow_plane_state->fmtcnv_state);
> +}
> +
> +static void ssd133x_primary_plane_helper_panic_flush(struct drm_plane *plane)
> +{
> + struct drm_plane_state *plane_state = plane->state;
> + struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(plane_state);
> + struct drm_crtc *crtc = plane_state->crtc;
> + struct ssd130x_crtc_state *ssd130x_crtc_state = to_ssd130x_crtc_state(crtc->state);
> +
> + ssd133x_fb_blit_rect(plane_state->fb, &shadow_plane_state->data[0], &plane_state->dst,
> + ssd130x_crtc_state->data_array, &shadow_plane_state->fmtcnv_state);
> +}
> +
> /* Called during init to allocate the plane's atomic state. */
> static void ssd130x_primary_plane_reset(struct drm_plane *plane)
> {
> @@ -1442,18 +1500,24 @@ static const struct drm_plane_helper_funcs ssd130x_primary_plane_helper_funcs[]
> .atomic_check = ssd130x_primary_plane_atomic_check,
> .atomic_update = ssd130x_primary_plane_atomic_update,
> .atomic_disable = ssd130x_primary_plane_atomic_disable,
> + .get_scanout_buffer = ssd130x_primary_plane_helper_get_scanout_buffer,
> + .panic_flush = ssd130x_primary_plane_helper_panic_flush,
> },
> [SSD132X_FAMILY] = {
> DRM_GEM_SHADOW_PLANE_HELPER_FUNCS,
> .atomic_check = ssd132x_primary_plane_atomic_check,
> .atomic_update = ssd132x_primary_plane_atomic_update,
> .atomic_disable = ssd132x_primary_plane_atomic_disable,
> + .get_scanout_buffer = ssd130x_primary_plane_helper_get_scanout_buffer,
> + .panic_flush = ssd132x_primary_plane_helper_panic_flush,
> },
> [SSD133X_FAMILY] = {
> DRM_GEM_SHADOW_PLANE_HELPER_FUNCS,
> .atomic_check = ssd133x_primary_plane_atomic_check,
> .atomic_update = ssd133x_primary_plane_atomic_update,
> .atomic_disable = ssd133x_primary_plane_atomic_disable,
> + .get_scanout_buffer = ssd130x_primary_plane_helper_get_scanout_buffer,
> + .panic_flush = ssd133x_primary_plane_helper_panic_flush,
> }
> };
>
Powered by blists - more mailing lists