* [PATCH] drm/panic: depends on !VT_CONSOLE
@ 2024-06-13 15:40 Jocelyn Falempe
2024-06-16 12:43 ` Javier Martinez Canillas
2024-07-16 2:38 ` nerdopolis
0 siblings, 2 replies; 9+ messages in thread
From: Jocelyn Falempe @ 2024-06-13 15:40 UTC (permalink / raw)
To: dri-devel, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Daniel Vetter, Javier Martinez Canillas,
Geert Uytterhoeven
Cc: Jocelyn Falempe, Daniel Vetter
The race condition between fbcon and drm_panic can only occurs if
VT_CONSOLE is set. So update drm_panic dependency accordingly.
This will make it easier for Linux distributions to enable drm_panic
by disabling VT_CONSOLE, and keeping fbcon terminal.
The only drawback is that fbcon won't display the boot kmsg, so it
should rely on userspace to do that.
At least plymouth already handle this case with
https://gitlab.freedesktop.org/plymouth/plymouth/-/merge_requests/224
Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
---
drivers/gpu/drm/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index a9df94291622..f5c989aed7e9 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -107,7 +107,7 @@ config DRM_KMS_HELPER
config DRM_PANIC
bool "Display a user-friendly message when a kernel panic occurs"
- depends on DRM && !FRAMEBUFFER_CONSOLE
+ depends on DRM && !(FRAMEBUFFER_CONSOLE && VT_CONSOLE)
select DRM_KMS_HELPER
select FONT_SUPPORT
help
base-commit: 2bae076f3e35234e42bd7c90acd8caae8368ba90
--
2.45.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/panic: depends on !VT_CONSOLE
2024-06-13 15:40 [PATCH] drm/panic: depends on !VT_CONSOLE Jocelyn Falempe
@ 2024-06-16 12:43 ` Javier Martinez Canillas
2024-06-17 8:16 ` Jocelyn Falempe
2024-07-16 2:38 ` nerdopolis
1 sibling, 1 reply; 9+ messages in thread
From: Javier Martinez Canillas @ 2024-06-16 12:43 UTC (permalink / raw)
To: Jocelyn Falempe, dri-devel, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter,
Geert Uytterhoeven
Cc: Jocelyn Falempe, Daniel Vetter
Jocelyn Falempe <jfalempe@redhat.com> writes:
Hello Jocelyn,
> The race condition between fbcon and drm_panic can only occurs if
> VT_CONSOLE is set. So update drm_panic dependency accordingly.
> This will make it easier for Linux distributions to enable drm_panic
> by disabling VT_CONSOLE, and keeping fbcon terminal.
> The only drawback is that fbcon won't display the boot kmsg, so it
> should rely on userspace to do that.
> At least plymouth already handle this case with
> https://gitlab.freedesktop.org/plymouth/plymouth/-/merge_requests/224
>
> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
> ---
> drivers/gpu/drm/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index a9df94291622..f5c989aed7e9 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -107,7 +107,7 @@ config DRM_KMS_HELPER
>
> config DRM_PANIC
> bool "Display a user-friendly message when a kernel panic occurs"
> - depends on DRM && !FRAMEBUFFER_CONSOLE
> + depends on DRM && !(FRAMEBUFFER_CONSOLE && VT_CONSOLE)
I thought the idea was to only make it depend on !VT_CONSOLE, so that
distros could also enable fbcon / VT but prevent the race condition to
happen due the VT not being a system console for the kernel to print
messages ?
In other words, my understanding from the discussion with Sima was that
this should be instead:
+ depends on DRM && !VT_CONSOLE
I've tested that and at least I see that a framebuffer console is present
and `echo c > /proc/sysrq-trigger` triggers the DRM panic handler message
(but don't know if the race exists and is just that I was not hitting it).
If my understanding is correct and should only be a depends on !VT_CONSOLE
then feel free to add my:
Tested-by: Javier Martinez Canillas <javierm@redhat.com>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/panic: depends on !VT_CONSOLE
2024-06-16 12:43 ` Javier Martinez Canillas
@ 2024-06-17 8:16 ` Jocelyn Falempe
2024-06-17 8:25 ` Geert Uytterhoeven
2024-06-17 8:27 ` Javier Martinez Canillas
0 siblings, 2 replies; 9+ messages in thread
From: Jocelyn Falempe @ 2024-06-17 8:16 UTC (permalink / raw)
To: Javier Martinez Canillas, dri-devel, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
Geert Uytterhoeven
Cc: Daniel Vetter
On 16/06/2024 14:43, Javier Martinez Canillas wrote:
> Jocelyn Falempe <jfalempe@redhat.com> writes:
>
> Hello Jocelyn,
>
>> The race condition between fbcon and drm_panic can only occurs if
>> VT_CONSOLE is set. So update drm_panic dependency accordingly.
>> This will make it easier for Linux distributions to enable drm_panic
>> by disabling VT_CONSOLE, and keeping fbcon terminal.
>> The only drawback is that fbcon won't display the boot kmsg, so it
>> should rely on userspace to do that.
>> At least plymouth already handle this case with
>> https://gitlab.freedesktop.org/plymouth/plymouth/-/merge_requests/224
>>
>> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
>> ---
>> drivers/gpu/drm/Kconfig | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
>> index a9df94291622..f5c989aed7e9 100644
>> --- a/drivers/gpu/drm/Kconfig
>> +++ b/drivers/gpu/drm/Kconfig
>> @@ -107,7 +107,7 @@ config DRM_KMS_HELPER
>>
>> config DRM_PANIC
>> bool "Display a user-friendly message when a kernel panic occurs"
>> - depends on DRM && !FRAMEBUFFER_CONSOLE
>> + depends on DRM && !(FRAMEBUFFER_CONSOLE && VT_CONSOLE)
>
> I thought the idea was to only make it depend on !VT_CONSOLE, so that
> distros could also enable fbcon / VT but prevent the race condition to
> happen due the VT not being a system console for the kernel to print
> messages ?
Yes, but when writing the patch, I thought that if you have VT_CONSOLE=y
and FRAMEBUFFER_CONSOLE=n, then there won't be any race condition, and
drm_panic can be enabled safely.
I don't know if that really matters, and if VT_CONSOLE has any usage
apart from fbcon.
>
> In other words, my understanding from the discussion with Sima was that
> this should be instead:
>
> + depends on DRM && !VT_CONSOLE
>
> I've tested that and at least I see that a framebuffer console is present
> and `echo c > /proc/sysrq-trigger` triggers the DRM panic handler message
> (but don't know if the race exists and is just that I was not hitting it).
>
> If my understanding is correct and should only be a depends on !VT_CONSOLE
> then feel free to add my:
>
> Tested-by: Javier Martinez Canillas <javierm@redhat.com>
>
Best regards,
--
Jocelyn
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/panic: depends on !VT_CONSOLE
2024-06-17 8:16 ` Jocelyn Falempe
@ 2024-06-17 8:25 ` Geert Uytterhoeven
2024-06-17 9:20 ` Jocelyn Falempe
2024-06-17 8:27 ` Javier Martinez Canillas
1 sibling, 1 reply; 9+ messages in thread
From: Geert Uytterhoeven @ 2024-06-17 8:25 UTC (permalink / raw)
To: Jocelyn Falempe
Cc: Javier Martinez Canillas, dri-devel, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
Geert Uytterhoeven, Daniel Vetter
Hi Jocelyn,
On Mon, Jun 17, 2024 at 10:16 AM Jocelyn Falempe <jfalempe@redhat.com> wrote:
> On 16/06/2024 14:43, Javier Martinez Canillas wrote:
> > Jocelyn Falempe <jfalempe@redhat.com> writes:
> >> The race condition between fbcon and drm_panic can only occurs if
> >> VT_CONSOLE is set. So update drm_panic dependency accordingly.
> >> This will make it easier for Linux distributions to enable drm_panic
> >> by disabling VT_CONSOLE, and keeping fbcon terminal.
> >> The only drawback is that fbcon won't display the boot kmsg, so it
> >> should rely on userspace to do that.
> >> At least plymouth already handle this case with
> >> https://gitlab.freedesktop.org/plymouth/plymouth/-/merge_requests/224
> >>
> >> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> >> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
> >> ---
> >> drivers/gpu/drm/Kconfig | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> >> index a9df94291622..f5c989aed7e9 100644
> >> --- a/drivers/gpu/drm/Kconfig
> >> +++ b/drivers/gpu/drm/Kconfig
> >> @@ -107,7 +107,7 @@ config DRM_KMS_HELPER
> >>
> >> config DRM_PANIC
> >> bool "Display a user-friendly message when a kernel panic occurs"
> >> - depends on DRM && !FRAMEBUFFER_CONSOLE
> >> + depends on DRM && !(FRAMEBUFFER_CONSOLE && VT_CONSOLE)
> >
> > I thought the idea was to only make it depend on !VT_CONSOLE, so that
> > distros could also enable fbcon / VT but prevent the race condition to
> > happen due the VT not being a system console for the kernel to print
> > messages ?
>
> Yes, but when writing the patch, I thought that if you have VT_CONSOLE=y
> and FRAMEBUFFER_CONSOLE=n, then there won't be any race condition, and
> drm_panic can be enabled safely.
> I don't know if that really matters, and if VT_CONSOLE has any usage
> apart from fbcon.
It is used for any kind of virtual terminal, so also for vgacon.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/panic: depends on !VT_CONSOLE
2024-06-17 8:16 ` Jocelyn Falempe
2024-06-17 8:25 ` Geert Uytterhoeven
@ 2024-06-17 8:27 ` Javier Martinez Canillas
1 sibling, 0 replies; 9+ messages in thread
From: Javier Martinez Canillas @ 2024-06-17 8:27 UTC (permalink / raw)
To: Jocelyn Falempe, dri-devel, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter,
Geert Uytterhoeven
Cc: Daniel Vetter
Jocelyn Falempe <jfalempe@redhat.com> writes:
> On 16/06/2024 14:43, Javier Martinez Canillas wrote:
>> Jocelyn Falempe <jfalempe@redhat.com> writes:
>>
>> Hello Jocelyn,
>>
>>> The race condition between fbcon and drm_panic can only occurs if
>>> VT_CONSOLE is set. So update drm_panic dependency accordingly.
>>> This will make it easier for Linux distributions to enable drm_panic
>>> by disabling VT_CONSOLE, and keeping fbcon terminal.
>>> The only drawback is that fbcon won't display the boot kmsg, so it
>>> should rely on userspace to do that.
>>> At least plymouth already handle this case with
>>> https://gitlab.freedesktop.org/plymouth/plymouth/-/merge_requests/224
>>>
>>> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
>>> ---
>>> drivers/gpu/drm/Kconfig | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
>>> index a9df94291622..f5c989aed7e9 100644
>>> --- a/drivers/gpu/drm/Kconfig
>>> +++ b/drivers/gpu/drm/Kconfig
>>> @@ -107,7 +107,7 @@ config DRM_KMS_HELPER
>>>
>>> config DRM_PANIC
>>> bool "Display a user-friendly message when a kernel panic occurs"
>>> - depends on DRM && !FRAMEBUFFER_CONSOLE
>>> + depends on DRM && !(FRAMEBUFFER_CONSOLE && VT_CONSOLE)
>>
>> I thought the idea was to only make it depend on !VT_CONSOLE, so that
>> distros could also enable fbcon / VT but prevent the race condition to
>> happen due the VT not being a system console for the kernel to print
>> messages ?
>
> Yes, but when writing the patch, I thought that if you have VT_CONSOLE=y
> and FRAMEBUFFER_CONSOLE=n, then there won't be any race condition, and
> drm_panic can be enabled safely.
Ah, I see what you mean.
> I don't know if that really matters, and if VT_CONSOLE has any usage
> apart from fbcon.
>
Right. I don't know...
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/panic: depends on !VT_CONSOLE
2024-06-17 8:25 ` Geert Uytterhoeven
@ 2024-06-17 9:20 ` Jocelyn Falempe
2024-06-17 9:27 ` Javier Martinez Canillas
0 siblings, 1 reply; 9+ messages in thread
From: Jocelyn Falempe @ 2024-06-17 9:20 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Javier Martinez Canillas, dri-devel, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
Geert Uytterhoeven, Daniel Vetter
On 17/06/2024 10:25, Geert Uytterhoeven wrote:
> Hi Jocelyn,
>
> On Mon, Jun 17, 2024 at 10:16 AM Jocelyn Falempe <jfalempe@redhat.com> wrote:
>> On 16/06/2024 14:43, Javier Martinez Canillas wrote:
>>> Jocelyn Falempe <jfalempe@redhat.com> writes:
>>>> The race condition between fbcon and drm_panic can only occurs if
>>>> VT_CONSOLE is set. So update drm_panic dependency accordingly.
>>>> This will make it easier for Linux distributions to enable drm_panic
>>>> by disabling VT_CONSOLE, and keeping fbcon terminal.
>>>> The only drawback is that fbcon won't display the boot kmsg, so it
>>>> should rely on userspace to do that.
>>>> At least plymouth already handle this case with
>>>> https://gitlab.freedesktop.org/plymouth/plymouth/-/merge_requests/224
>>>>
>>>> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
>>>> ---
>>>> drivers/gpu/drm/Kconfig | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
>>>> index a9df94291622..f5c989aed7e9 100644
>>>> --- a/drivers/gpu/drm/Kconfig
>>>> +++ b/drivers/gpu/drm/Kconfig
>>>> @@ -107,7 +107,7 @@ config DRM_KMS_HELPER
>>>>
>>>> config DRM_PANIC
>>>> bool "Display a user-friendly message when a kernel panic occurs"
>>>> - depends on DRM && !FRAMEBUFFER_CONSOLE
>>>> + depends on DRM && !(FRAMEBUFFER_CONSOLE && VT_CONSOLE)
>>>
>>> I thought the idea was to only make it depend on !VT_CONSOLE, so that
>>> distros could also enable fbcon / VT but prevent the race condition to
>>> happen due the VT not being a system console for the kernel to print
>>> messages ?
>>
>> Yes, but when writing the patch, I thought that if you have VT_CONSOLE=y
>> and FRAMEBUFFER_CONSOLE=n, then there won't be any race condition, and
>> drm_panic can be enabled safely.
>> I don't know if that really matters, and if VT_CONSOLE has any usage
>> apart from fbcon.
>
> It is used for any kind of virtual terminal, so also for vgacon.
Ah thanks, but I think vgacon is safe to use with drm_panic.
The problem with fbcon, is that it can also uses the fbdev emulation
from the drm driver, and in this case, drm_panic will write to the same
framebuffer as fbcon during a panic, leading to corrupted output.
This can't occur with other graphical console, so !(FRAMEBUFFER_CONSOLE
&& VT_CONSOLE) is probably good.
--
Jocelyn
>
> Gr{oetje,eeting}s,
>
> Geert
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/panic: depends on !VT_CONSOLE
2024-06-17 9:20 ` Jocelyn Falempe
@ 2024-06-17 9:27 ` Javier Martinez Canillas
2024-06-18 15:22 ` Jocelyn Falempe
0 siblings, 1 reply; 9+ messages in thread
From: Javier Martinez Canillas @ 2024-06-17 9:27 UTC (permalink / raw)
To: Jocelyn Falempe, Geert Uytterhoeven
Cc: dri-devel, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Daniel Vetter, Geert Uytterhoeven, Daniel Vetter
Jocelyn Falempe <jfalempe@redhat.com> writes:
> On 17/06/2024 10:25, Geert Uytterhoeven wrote:
>> Hi Jocelyn,
>>
>> On Mon, Jun 17, 2024 at 10:16 AM Jocelyn Falempe <jfalempe@redhat.com> wrote:
>>> On 16/06/2024 14:43, Javier Martinez Canillas wrote:
>>>> Jocelyn Falempe <jfalempe@redhat.com> writes:
>>>>> The race condition between fbcon and drm_panic can only occurs if
>>>>> VT_CONSOLE is set. So update drm_panic dependency accordingly.
>>>>> This will make it easier for Linux distributions to enable drm_panic
>>>>> by disabling VT_CONSOLE, and keeping fbcon terminal.
>>>>> The only drawback is that fbcon won't display the boot kmsg, so it
>>>>> should rely on userspace to do that.
>>>>> At least plymouth already handle this case with
>>>>> https://gitlab.freedesktop.org/plymouth/plymouth/-/merge_requests/224
>>>>>
>>>>> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>>> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
>>>>> ---
>>>>> drivers/gpu/drm/Kconfig | 2 +-
>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
>>>>> index a9df94291622..f5c989aed7e9 100644
>>>>> --- a/drivers/gpu/drm/Kconfig
>>>>> +++ b/drivers/gpu/drm/Kconfig
>>>>> @@ -107,7 +107,7 @@ config DRM_KMS_HELPER
>>>>>
>>>>> config DRM_PANIC
>>>>> bool "Display a user-friendly message when a kernel panic occurs"
>>>>> - depends on DRM && !FRAMEBUFFER_CONSOLE
>>>>> + depends on DRM && !(FRAMEBUFFER_CONSOLE && VT_CONSOLE)
>>>>
>>>> I thought the idea was to only make it depend on !VT_CONSOLE, so that
>>>> distros could also enable fbcon / VT but prevent the race condition to
>>>> happen due the VT not being a system console for the kernel to print
>>>> messages ?
>>>
>>> Yes, but when writing the patch, I thought that if you have VT_CONSOLE=y
>>> and FRAMEBUFFER_CONSOLE=n, then there won't be any race condition, and
>>> drm_panic can be enabled safely.
>>> I don't know if that really matters, and if VT_CONSOLE has any usage
>>> apart from fbcon.
>>
>> It is used for any kind of virtual terminal, so also for vgacon.
>
> Ah thanks, but I think vgacon is safe to use with drm_panic.
>
> The problem with fbcon, is that it can also uses the fbdev emulation
> from the drm driver, and in this case, drm_panic will write to the same
> framebuffer as fbcon during a panic, leading to corrupted output.
> This can't occur with other graphical console, so !(FRAMEBUFFER_CONSOLE
> && VT_CONSOLE) is probably good.
>
Yeah, I agre that !(FRAMEBUFFER_CONSOLE && VT_CONSOLE) as you have in your
patch is what makes sense. !VT_CONSOLE alone as I argued in my first email
isn't correct since as you pointed out, it shouldn't affect other consoles
besides fbcon.
So please feel free to also add:
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/panic: depends on !VT_CONSOLE
2024-06-17 9:27 ` Javier Martinez Canillas
@ 2024-06-18 15:22 ` Jocelyn Falempe
0 siblings, 0 replies; 9+ messages in thread
From: Jocelyn Falempe @ 2024-06-18 15:22 UTC (permalink / raw)
To: Javier Martinez Canillas, Geert Uytterhoeven
Cc: dri-devel, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Daniel Vetter, Geert Uytterhoeven, Daniel Vetter
On 17/06/2024 11:27, Javier Martinez Canillas wrote:
> Jocelyn Falempe <jfalempe@redhat.com> writes:
>
>> On 17/06/2024 10:25, Geert Uytterhoeven wrote:
>>> Hi Jocelyn,
>>>
>>> On Mon, Jun 17, 2024 at 10:16 AM Jocelyn Falempe <jfalempe@redhat.com> wrote:
>>>> On 16/06/2024 14:43, Javier Martinez Canillas wrote:
>>>>> Jocelyn Falempe <jfalempe@redhat.com> writes:
>>>>>> The race condition between fbcon and drm_panic can only occurs if
>>>>>> VT_CONSOLE is set. So update drm_panic dependency accordingly.
>>>>>> This will make it easier for Linux distributions to enable drm_panic
>>>>>> by disabling VT_CONSOLE, and keeping fbcon terminal.
>>>>>> The only drawback is that fbcon won't display the boot kmsg, so it
>>>>>> should rely on userspace to do that.
>>>>>> At least plymouth already handle this case with
>>>>>> https://gitlab.freedesktop.org/plymouth/plymouth/-/merge_requests/224
>>>>>>
>>>>>> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>>>> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
>>>>>> ---
>>>>>> drivers/gpu/drm/Kconfig | 2 +-
>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
>>>>>> index a9df94291622..f5c989aed7e9 100644
>>>>>> --- a/drivers/gpu/drm/Kconfig
>>>>>> +++ b/drivers/gpu/drm/Kconfig
>>>>>> @@ -107,7 +107,7 @@ config DRM_KMS_HELPER
>>>>>>
>>>>>> config DRM_PANIC
>>>>>> bool "Display a user-friendly message when a kernel panic occurs"
>>>>>> - depends on DRM && !FRAMEBUFFER_CONSOLE
>>>>>> + depends on DRM && !(FRAMEBUFFER_CONSOLE && VT_CONSOLE)
>>>>>
>>>>> I thought the idea was to only make it depend on !VT_CONSOLE, so that
>>>>> distros could also enable fbcon / VT but prevent the race condition to
>>>>> happen due the VT not being a system console for the kernel to print
>>>>> messages ?
>>>>
>>>> Yes, but when writing the patch, I thought that if you have VT_CONSOLE=y
>>>> and FRAMEBUFFER_CONSOLE=n, then there won't be any race condition, and
>>>> drm_panic can be enabled safely.
>>>> I don't know if that really matters, and if VT_CONSOLE has any usage
>>>> apart from fbcon.
>>>
>>> It is used for any kind of virtual terminal, so also for vgacon.
>>
>> Ah thanks, but I think vgacon is safe to use with drm_panic.
>>
>> The problem with fbcon, is that it can also uses the fbdev emulation
>> from the drm driver, and in this case, drm_panic will write to the same
>> framebuffer as fbcon during a panic, leading to corrupted output.
>> This can't occur with other graphical console, so !(FRAMEBUFFER_CONSOLE
>> && VT_CONSOLE) is probably good.
>>
>
> Yeah, I agre that !(FRAMEBUFFER_CONSOLE && VT_CONSOLE) as you have in your
> patch is what makes sense. !VT_CONSOLE alone as I argued in my first email
> isn't correct since as you pointed out, it shouldn't affect other consoles
> besides fbcon.
>
> So please feel free to also add:
>
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
>
Thanks all for your comments and reviews.
I just pushed it to drm-misc-next.
--
Jocelyn
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/panic: depends on !VT_CONSOLE
2024-06-13 15:40 [PATCH] drm/panic: depends on !VT_CONSOLE Jocelyn Falempe
2024-06-16 12:43 ` Javier Martinez Canillas
@ 2024-07-16 2:38 ` nerdopolis
1 sibling, 0 replies; 9+ messages in thread
From: nerdopolis @ 2024-07-16 2:38 UTC (permalink / raw)
To: dri-devel, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Daniel Vetter, Javier Martinez Canillas,
Geert Uytterhoeven
Cc: Jocelyn Falempe, Daniel Vetter
On Thursday, June 13, 2024 11:40:16 AM EDT Jocelyn Falempe wrote:
> The race condition between fbcon and drm_panic can only occurs if
> VT_CONSOLE is set. So update drm_panic dependency accordingly.
> This will make it easier for Linux distributions to enable drm_panic
> by disabling VT_CONSOLE, and keeping fbcon terminal.
> The only drawback is that fbcon won't display the boot kmsg, so it
> should rely on userspace to do that.
> At least plymouth already handle this case with
> https://gitlab.freedesktop.org/plymouth/plymouth/-/merge_requests/224
Hi
An FYI, seeing this and https://fedoraproject.org/wiki/Changes/EnableDrmPanic ,
there is a slight issue with CONFIG_VT_CONSOLE=n in relation to systemd that
was recently discovered outside of virt-manager VMs, (or computers that have no
devices connected to the first serial port)
This was seen on a full CONFIG_VT=n kernel, but it also happens on kernels
with just CONFIG_VT_CONSOLE disabled.
When /dev/console is /dev/ttyS0 (as that is the new default without the VT
console, and /dev/ttyS0 is not connected to anything, isatty()'s TCGETS ioctl
on /dev/console results in an input output error, causing isatty() to return
false. Because of this, systemd's status logging then rejects /dev/console as
not being a terminal device, and rejects writing status output to it, resulting
in a decrease of verbosity available to Plymouth.
It at first did not show up in my testing, because all my virt-manager and qemu
VMs had serial devices attached, which would cause isatty() to return true on
/dev/ttyS0 backed /dev/console, but now I have raised this with linux-serial.
Thanks
>
> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
> ---
> drivers/gpu/drm/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index a9df94291622..f5c989aed7e9 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -107,7 +107,7 @@ config DRM_KMS_HELPER
>
> config DRM_PANIC
> bool "Display a user-friendly message when a kernel panic occurs"
> - depends on DRM && !FRAMEBUFFER_CONSOLE
> + depends on DRM && !(FRAMEBUFFER_CONSOLE && VT_CONSOLE)
> select DRM_KMS_HELPER
> select FONT_SUPPORT
> help
>
> base-commit: 2bae076f3e35234e42bd7c90acd8caae8368ba90
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-07-16 2:38 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-13 15:40 [PATCH] drm/panic: depends on !VT_CONSOLE Jocelyn Falempe
2024-06-16 12:43 ` Javier Martinez Canillas
2024-06-17 8:16 ` Jocelyn Falempe
2024-06-17 8:25 ` Geert Uytterhoeven
2024-06-17 9:20 ` Jocelyn Falempe
2024-06-17 9:27 ` Javier Martinez Canillas
2024-06-18 15:22 ` Jocelyn Falempe
2024-06-17 8:27 ` Javier Martinez Canillas
2024-07-16 2:38 ` nerdopolis
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.