mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Jocelyn Falempe <jfalempe@redhat.com>
To: Nahida Buer <me@nahida.im>
Cc: dri-devel@lists.freedesktop.org,
	Javier Martinez Canillas <javierm@redhat.com>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Maxime Ripard <mripard@kernel.org>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	David Airlie <airlied@gmail.com>, Simona Vetter <simona@ffwll.ch>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] drm/panic: allow overriding panic QR code URL
Date: Wed, 7 Oct 2026 15:17:48 +0200	[thread overview]
Message-ID: <24ad6178-2f3b-4f84-91b5-c1f4fb92a21b@redhat.com> (raw)
In-Reply-To: <CAEEzyciisJgZgx4PZQbEEkUUKRMpiAsmmsW9JfQzwCCDx4xZKA@mail.gmail.com>

On 10/3/26 01:03, Nahida Buer wrote:
> Thanks for the review.
> 
> My motivation was slightly different from disabling the URL when the
> configured endpoint is untrusted or unavailable. I wanted to keep the
> URL QR mode while allowing the same kernel image to use a different
> reporting endpoint depending on the deployment.
> 
> It also allows enabling the compressed URL mode without rebuilding the
> kernel. With an empty CONFIG_DRM_PANIC_SCREEN_QR_CODE_URL, the QR code
> contains the raw kmsg. Supplying drm.panic_qr_url= at boot instead
> switches it to the URL format, where the kmsg is compressed with zlib.
> 
> This could be useful for a distribution or a locally built kernel image
> used on multiple systems: the image can keep a neutral/default
> configuration, while individual systems can opt into a specific panic
> report collector and compressed QR payload at boot time.
> 
> panic_screen=user or panic_screen=kmsg only disables the URL QR mode;
> it does not provide a way to keep that mode while changing or enabling
> the reporting endpoint.
> 
> The parameter is read-only after initialization, so it only selects the
> endpoint at boot/module load time.
> 
> If keeping the URL fixed at build time is intentional policy for DRM
> panic, though, I'm fine with dropping the patch.

After thinking more about this, I prefer to keep the URL fixed at build 
time.
I hope it won't refrain you from contributing again.

Best regards,

-- 

Jocelyn
> 
> Jocelyn Falempe <jfalempe@redhat.com> 于2026年10月3日周六 06:56写道:
>>
>> On 10/3/26 00:20, 纳西妲 · Nahida wrote:
>>> The base URL used for the panic QR code is currently fixed at build time
>>> by CONFIG_DRM_PANIC_SCREEN_QR_CODE_URL. Changing the panic report
>>> endpoint therefore requires rebuilding the kernel.
>>>
>>> Add a drm.panic_qr_url module parameter to allow overriding the URL at
>>> boot or module load time, while keeping the Kconfig value as the
>>> default.
>>>
>>> Keep the parameter read-only after initialization so the panic path can
>>> access it without additional synchronization.
>>>
>>> Expose the configured URL to the DRM panic helper through an internal
>>> getter so the helper can use the runtime value when generating the QR
>>> code.
>>
>> The code looks good, but I think it shouldn't be allowed to change this
>> setting at boot time.
>> You can already set the drm_panic screen to "user" or "kmsg" in the
>> kernel command line if you don't trust the default URL, or if the URL is
>> no more reachable.
>> It introduces some complexity, and I don't really see the benefit.
>>
>> Best regards,
>>
>> --
>>
>> Jocelyn>
>>> Assisted-by: LLM
>>> Signed-off-by: 纳西妲 · Nahida <me@nahida.im>
>>> ---
>>> v2:
>>> - Grow the formatted URL buffer to accommodate the maximum 255-byte
>>>     runtime base URL.
>>> - Reject unexpectedly truncated formatted URLs.
>>> - Test the maximum-length runtime URL.
>>>
>>> v1: https://lore.kernel.org/r/20261002203140.121165-1-me@nahida.im
>>>
>>> Tested on x86_64:
>>> - Full modular build with DRM=m and DRM_KMS_HELPER=m.
>>> - QEMU/virtio-gpu runtime override with a non-empty Kconfig default URL.
>>> - QEMU/virtio-gpu runtime fallback to the Kconfig default URL.
>>> - QEMU/virtio-gpu runtime override with an empty Kconfig default URL.
>>> - QEMU/virtio-gpu raw kmsg QR fallback with no configured URL.
>>> - QEMU/virtio-gpu runtime override with a 255-byte base URL.
>>>
>>>    drivers/gpu/drm/Kconfig              |  9 +++++----
>>>    drivers/gpu/drm/drm_panic.c          |  9 +++++++++
>>>    drivers/gpu/drm/drm_panic_helper.c   | 17 +++++++++++------
>>>    drivers/gpu/drm/drm_panic_internal.h |  4 ++++
>>>    4 files changed, 29 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
>>> index 61111d50b..3d9d421f0 100644
>>> --- a/drivers/gpu/drm/Kconfig
>>> +++ b/drivers/gpu/drm/Kconfig
>>> @@ -111,10 +111,11 @@ config DRM_PANIC_SCREEN_QR_CODE_URL
>>>        string "Base URL of the QR code in the panic screen"
>>>        depends on DRM_PANIC_SCREEN_QR_CODE
>>>        help
>>> -       This option sets the base URL to report the kernel panic. If it's set
>>> -       the QR code will contain the URL and the kmsg compressed with zlib as
>>> -       a URL parameter. If it's empty, the QR code will contain the kmsg as
>>> -       uncompressed text only.
>>> +       This option sets the default base URL to report the kernel panic. If
>>> +       it's set the QR code will contain the URL and the kmsg compressed with
>>> +       zlib as a URL parameter. If it's empty, the QR code will contain the
>>> +       kmsg as uncompressed text only. It can be overridden with the
>>> +       drm.panic_qr_url=<url> kernel parameter.
>>>          There is a demo code in javascript, to decode and uncompress the kmsg
>>>          data from the URL parameter at https://github.com/kdj0c/panic_report
>>>
>>> diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
>>> index 9fe97c4ed..463544f71 100644
>>> --- a/drivers/gpu/drm/drm_panic.c
>>> +++ b/drivers/gpu/drm/drm_panic.c
>>> @@ -49,6 +49,15 @@ MODULE_LICENSE("GPL");
>>>    static uint panic_qr_version = CONFIG_DRM_PANIC_SCREEN_QR_VERSION;
>>>    module_param(panic_qr_version, uint, 0644);
>>>    MODULE_PARM_DESC(panic_qr_version, "maximum version (size) of the QR code");
>>> +static char panic_qr_url[256] = CONFIG_DRM_PANIC_SCREEN_QR_CODE_URL;
>>> +module_param_string(panic_qr_url, panic_qr_url, sizeof(panic_qr_url), 0444);
>>> +MODULE_PARM_DESC(panic_qr_url, "base URL of the QR code in the panic screen");
>>> +
>>> +const char *drm_panic_get_qr_url(void)
>>> +{
>>> +     return panic_qr_url;
>>> +}
>>> +EXPORT_SYMBOL_GPL(drm_panic_get_qr_url);
>>>    #endif
>>>
>>>    static enum drm_panic_type drm_panic_type = -1;
>>> diff --git a/drivers/gpu/drm/drm_panic_helper.c b/drivers/gpu/drm/drm_panic_helper.c
>>> index 8913c3d22..b0ea0b4e8 100644
>>> --- a/drivers/gpu/drm/drm_panic_helper.c
>>> +++ b/drivers/gpu/drm/drm_panic_helper.c
>>> @@ -609,17 +609,20 @@ static void drm_panic_helper_qr_exit(void)
>>>        stream.workspace = NULL;
>>>    }
>>>
>>> -static int drm_panic_helper_get_qr_code_url(u8 **qr_image, unsigned int qr_version)
>>> +static int drm_panic_helper_get_qr_code_url(u8 **qr_image, unsigned int qr_version,
>>> +                                         const char *qr_url)
>>>    {
>>>        struct kmsg_dump_iter iter;
>>> -     char url[256];
>>> +     char url[512];
>>>        size_t kmsg_len, max_kmsg_size;
>>>        char *kmsg;
>>>        int max_qr_data_size, url_len;
>>>
>>>        url_len = snprintf(url, sizeof(url), "%s?a=%s&v=%s&z=",
>>> -                        CONFIG_DRM_PANIC_SCREEN_QR_CODE_URL,
>>> -                        utsname()->machine, utsname()->release);
>>> +                        qr_url, utsname()->machine, utsname()->release);
>>> +
>>> +     if (url_len >= (int)sizeof(url))
>>> +             return -E2BIG;
>>>
>>>        max_qr_data_size = drm_panic_helper_qr_max_data_size(qr_version, url_len);
>>>        max_kmsg_size = min(MAX_ZLIB_RATIO * max_qr_data_size, QR_BUFFER1_SIZE);
>>> @@ -686,8 +689,10 @@ static int drm_panic_helper_get_qr_code_raw(u8 **qr_image, unsigned int qr_versi
>>>
>>>    static int drm_panic_helper_get_qr_code(u8 **qr_image, unsigned int qr_version)
>>>    {
>>> -     if (strlen(CONFIG_DRM_PANIC_SCREEN_QR_CODE_URL) > 0)
>>> -             return drm_panic_helper_get_qr_code_url(qr_image, qr_version);
>>> +     const char *qr_url = drm_panic_get_qr_url();
>>> +
>>> +     if (qr_url[0])
>>> +             return drm_panic_helper_get_qr_code_url(qr_image, qr_version, qr_url);
>>>        else
>>>                return drm_panic_helper_get_qr_code_raw(qr_image, qr_version);
>>>    }
>>> diff --git a/drivers/gpu/drm/drm_panic_internal.h b/drivers/gpu/drm/drm_panic_internal.h
>>> index fe6d514a0..f33552265 100644
>>> --- a/drivers/gpu/drm/drm_panic_internal.h
>>> +++ b/drivers/gpu/drm/drm_panic_internal.h
>>> @@ -14,6 +14,10 @@ struct drm_device;
>>>
>>>    #ifdef CONFIG_DRM_PANIC
>>>
>>> +#if IS_ENABLED(CONFIG_DRM_PANIC_SCREEN_QR_CODE)
>>> +const char *drm_panic_get_qr_url(void);
>>> +#endif
>>> +
>>>    /**
>>>     * drm_panic_lock - protect panic printing relevant state
>>>     * @dev: struct drm_device
>>>
>>> base-commit: 54e61ea6c492d69a1717018826d8340810c812eb
>>
> 


      reply	other threads:[~2026-10-07 13:17 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-02 20:31 [PATCH] " 纳西妲 · Nahida
2026-10-02 22:20 ` [PATCH v2] " 纳西妲 · Nahida
2026-10-02 22:56   ` Jocelyn Falempe
2026-10-02 23:03     ` Nahida Buer
2026-10-07 13:17       ` Jocelyn Falempe [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=24ad6178-2f3b-4f84-91b5-c1f4fb92a21b@redhat.com \
    --to=jfalempe@redhat.com \
    --cc=airlied@gmail.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=javierm@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=me@nahida.im \
    --cc=mripard@kernel.org \
    --cc=simona@ffwll.ch \
    --cc=tzimmermann@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®