amd-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] drm/amd/display: Fix kernel panic by breakpoint
@ 2020-10-23  7:46 Takashi Iwai
  2020-10-23  7:46 ` [PATCH 1/3] drm/amd/display: Fix kernel panic by dal_gpio_open() error Takashi Iwai
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Takashi Iwai @ 2020-10-23  7:46 UTC (permalink / raw)
  To: dri-devel; +Cc: linux-kernel, amd-gfx

Hi,

the amdgpu driver's ASSERT_CRITICAL() macro calls the
kgdb_breakpoing() even if no debug option is set, and this leads to a
kernel panic on distro kernels.  The first two patches are the
oneliner fixes for those, while the last one is the cleanup of those
debug macros.


Takashi

===

Takashi Iwai (3):
  drm/amd/display: Fix kernel panic by dal_gpio_open() error
  drm/amd/display: Don't invoke kgdb_breakpoint() unconditionally
  drm/amd/display: Clean up debug macros

 drivers/gpu/drm/amd/display/Kconfig             |  1 +
 drivers/gpu/drm/amd/display/dc/gpio/gpio_base.c |  4 +--
 drivers/gpu/drm/amd/display/dc/os_types.h       | 33 +++++++++----------------
 3 files changed, 15 insertions(+), 23 deletions(-)

-- 
2.16.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/3] drm/amd/display: Fix kernel panic by dal_gpio_open() error
  2020-10-23  7:46 [PATCH 0/3] drm/amd/display: Fix kernel panic by breakpoint Takashi Iwai
@ 2020-10-23  7:46 ` Takashi Iwai
  2020-10-23  7:46 ` [PATCH 2/3] drm/amd/display: Don't invoke kgdb_breakpoint() unconditionally Takashi Iwai
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Takashi Iwai @ 2020-10-23  7:46 UTC (permalink / raw)
  To: dri-devel; +Cc: linux-kernel, amd-gfx

Currently both error code paths handled in dal_gpio_open_ex() issues
ASSERT_CRITICAL(), and this leads to a kernel panic unnecessarily if
CONFIG_KGDB is enabled.  Since basically both are non-critical errors
and can be recovered, drop those assert calls and use a safer one,
BREAK_TO_DEBUGGER(), for allowing the debugging, instead.

BugLink: https://bugzilla.opensuse.org/show_bug.cgi?id=1177973
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/gpu/drm/amd/display/dc/gpio/gpio_base.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/gpio/gpio_base.c b/drivers/gpu/drm/amd/display/dc/gpio/gpio_base.c
index f67c18375bfd..dac427b68fd7 100644
--- a/drivers/gpu/drm/amd/display/dc/gpio/gpio_base.c
+++ b/drivers/gpu/drm/amd/display/dc/gpio/gpio_base.c
@@ -63,13 +63,13 @@ enum gpio_result dal_gpio_open_ex(
 	enum gpio_mode mode)
 {
 	if (gpio->pin) {
-		ASSERT_CRITICAL(false);
+		BREAK_TO_DEBUGGER();
 		return GPIO_RESULT_ALREADY_OPENED;
 	}
 
 	// No action if allocation failed during gpio construct
 	if (!gpio->hw_container.ddc) {
-		ASSERT_CRITICAL(false);
+		BREAK_TO_DEBUGGER();
 		return GPIO_RESULT_NON_SPECIFIC_ERROR;
 	}
 	gpio->mode = mode;
-- 
2.16.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/3] drm/amd/display: Don't invoke kgdb_breakpoint() unconditionally
  2020-10-23  7:46 [PATCH 0/3] drm/amd/display: Fix kernel panic by breakpoint Takashi Iwai
  2020-10-23  7:46 ` [PATCH 1/3] drm/amd/display: Fix kernel panic by dal_gpio_open() error Takashi Iwai
@ 2020-10-23  7:46 ` Takashi Iwai
  2020-10-23  7:46 ` [PATCH 3/3] drm/amd/display: Clean up debug macros Takashi Iwai
  2020-10-23 23:16 ` [PATCH 0/3] drm/amd/display: Fix kernel panic by breakpoint Luben Tuikov
  3 siblings, 0 replies; 8+ messages in thread
From: Takashi Iwai @ 2020-10-23  7:46 UTC (permalink / raw)
  To: dri-devel; +Cc: linux-kernel, amd-gfx

ASSERT_CRITICAL() invokes kgdb_breakpoint() whenever either
CONFIG_KGDB or CONFIG_HAVE_KGDB is set.  This, however, may lead to a
kernel panic when no kdb stuff is attached, since the
kgdb_breakpoint() call issues INT3.  It's nothing but a surprise for
normal end-users.

For avoiding the pitfall, make the kgdb_breakpoint() call only when
CONFIG_DEBUG_KERNEL_DC is set.

https://bugzilla.opensuse.org/show_bug.cgi?id=1177973
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/gpu/drm/amd/display/dc/os_types.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/os_types.h b/drivers/gpu/drm/amd/display/dc/os_types.h
index 330acaaed79a..32758b245754 100644
--- a/drivers/gpu/drm/amd/display/dc/os_types.h
+++ b/drivers/gpu/drm/amd/display/dc/os_types.h
@@ -94,7 +94,7 @@
  * general debug capabilities
  *
  */
-#if defined(CONFIG_HAVE_KGDB) || defined(CONFIG_KGDB)
+#if defined(CONFIG_DEBUG_KERNEL_DC) && (defined(CONFIG_HAVE_KGDB) || defined(CONFIG_KGDB))
 #define ASSERT_CRITICAL(expr) do {	\
 	if (WARN_ON(!(expr))) { \
 		kgdb_breakpoint(); \
-- 
2.16.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/3] drm/amd/display: Clean up debug macros
  2020-10-23  7:46 [PATCH 0/3] drm/amd/display: Fix kernel panic by breakpoint Takashi Iwai
  2020-10-23  7:46 ` [PATCH 1/3] drm/amd/display: Fix kernel panic by dal_gpio_open() error Takashi Iwai
  2020-10-23  7:46 ` [PATCH 2/3] drm/amd/display: Don't invoke kgdb_breakpoint() unconditionally Takashi Iwai
@ 2020-10-23  7:46 ` Takashi Iwai
  2020-10-23 23:16 ` [PATCH 0/3] drm/amd/display: Fix kernel panic by breakpoint Luben Tuikov
  3 siblings, 0 replies; 8+ messages in thread
From: Takashi Iwai @ 2020-10-23  7:46 UTC (permalink / raw)
  To: dri-devel; +Cc: linux-kernel, amd-gfx

This patch simplifies the ASSERT*() and BREAK_TO_DEBUGGER() macros:
- Move the dependency check of CONFIG_KGDB into Kconfig
- Unify the kgdb_breakpoint() call
- Drop the non-existing CONFIG_HAVE_KGDB

Also align the behavior of ASSERT() macro in both cases with and
without CONFIG_DEBUG_KERNEL_DC.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/gpu/drm/amd/display/Kconfig       |  1 +
 drivers/gpu/drm/amd/display/dc/os_types.h | 33 +++++++++++--------------------
 2 files changed, 13 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/Kconfig b/drivers/gpu/drm/amd/display/Kconfig
index f24abf428534..60dfdd432aba 100644
--- a/drivers/gpu/drm/amd/display/Kconfig
+++ b/drivers/gpu/drm/amd/display/Kconfig
@@ -42,6 +42,7 @@ config DRM_AMD_DC_SI
 config DEBUG_KERNEL_DC
 	bool "Enable kgdb break in DC"
 	depends on DRM_AMD_DC
+	depends on KGDB
 	help
 	  Choose this option if you want to hit kdgb_break in assert.
 
diff --git a/drivers/gpu/drm/amd/display/dc/os_types.h b/drivers/gpu/drm/amd/display/dc/os_types.h
index 32758b245754..95cb56929e79 100644
--- a/drivers/gpu/drm/amd/display/dc/os_types.h
+++ b/drivers/gpu/drm/amd/display/dc/os_types.h
@@ -94,36 +94,27 @@
  * general debug capabilities
  *
  */
-#if defined(CONFIG_DEBUG_KERNEL_DC) && (defined(CONFIG_HAVE_KGDB) || defined(CONFIG_KGDB))
-#define ASSERT_CRITICAL(expr) do {	\
-	if (WARN_ON(!(expr))) { \
-		kgdb_breakpoint(); \
-	} \
-} while (0)
+#ifdef CONFIG_DEBUG_KERNEL_DC
+#define dc_breakpoint()		kgdb_breakpoint()
 #else
-#define ASSERT_CRITICAL(expr) do {	\
-	if (WARN_ON(!(expr))) { \
-		; \
-	} \
-} while (0)
+#define dc_breakpoint()		do {} while (0)
 #endif
 
-#if defined(CONFIG_DEBUG_KERNEL_DC)
-#define ASSERT(expr) ASSERT_CRITICAL(expr)
+#define ASSERT_CRITICAL(expr) do {		\
+		if (WARN_ON(!(expr)))		\
+			dc_breakpoint();	\
+	} while (0)
 
-#else
-#define ASSERT(expr) WARN_ON_ONCE(!(expr))
-#endif
+#define ASSERT(expr) do {			\
+		if (WARN_ON_ONCE(!(expr)))	\
+			dc_breakpoint();	\
+	} while (0)
 
-#if defined(CONFIG_DEBUG_KERNEL_DC) && (defined(CONFIG_HAVE_KGDB) || defined(CONFIG_KGDB))
 #define BREAK_TO_DEBUGGER() \
 	do { \
 		DRM_DEBUG_DRIVER("%s():%d\n", __func__, __LINE__); \
-		kgdb_breakpoint(); \
+		dc_breakpoint(); \
 	} while (0)
-#else
-#define BREAK_TO_DEBUGGER() DRM_DEBUG_DRIVER("%s():%d\n", __func__, __LINE__)
-#endif
 
 #define DC_ERR(...)  do { \
 	dm_error(__VA_ARGS__); \
-- 
2.16.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/3] drm/amd/display: Fix kernel panic by breakpoint
  2020-10-23  7:46 [PATCH 0/3] drm/amd/display: Fix kernel panic by breakpoint Takashi Iwai
                   ` (2 preceding siblings ...)
  2020-10-23  7:46 ` [PATCH 3/3] drm/amd/display: Clean up debug macros Takashi Iwai
@ 2020-10-23 23:16 ` Luben Tuikov
  2020-10-26 19:34   ` Alex Deucher
  3 siblings, 1 reply; 8+ messages in thread
From: Luben Tuikov @ 2020-10-23 23:16 UTC (permalink / raw)
  To: Takashi Iwai, dri-devel; +Cc: linux-kernel, amd-gfx

On 2020-10-23 03:46, Takashi Iwai wrote:
> Hi,
> 
> the amdgpu driver's ASSERT_CRITICAL() macro calls the
> kgdb_breakpoing() even if no debug option is set, and this leads to a
> kernel panic on distro kernels.  The first two patches are the
> oneliner fixes for those, while the last one is the cleanup of those
> debug macros.

This looks like good work and solid. Hopefully it gets picked up.

Regards,
Luben

> 
> 
> Takashi
> 
> ===
> 
> Takashi Iwai (3):
>   drm/amd/display: Fix kernel panic by dal_gpio_open() error
>   drm/amd/display: Don't invoke kgdb_breakpoint() unconditionally
>   drm/amd/display: Clean up debug macros
> 
>  drivers/gpu/drm/amd/display/Kconfig             |  1 +
>  drivers/gpu/drm/amd/display/dc/gpio/gpio_base.c |  4 +--
>  drivers/gpu/drm/amd/display/dc/os_types.h       | 33 +++++++++----------------
>  3 files changed, 15 insertions(+), 23 deletions(-)
> 

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/3] drm/amd/display: Fix kernel panic by breakpoint
  2020-10-23 23:16 ` [PATCH 0/3] drm/amd/display: Fix kernel panic by breakpoint Luben Tuikov
@ 2020-10-26 19:34   ` Alex Deucher
  2020-10-26 20:22     ` Kazlauskas, Nicholas
  0 siblings, 1 reply; 8+ messages in thread
From: Alex Deucher @ 2020-10-26 19:34 UTC (permalink / raw)
  To: Luben Tuikov
  Cc: Takashi Iwai, amd-gfx list, LKML, Maling list - DRI developers

Yes, looks good to me as well.  Series is:
Acked-by: Alex Deucher <alexander.deucher@amd.com>
I'll give the display guys a few more days to look this over, but if
there are no objections, I'll apply them.

Thanks!

Alex

On Fri, Oct 23, 2020 at 7:16 PM Luben Tuikov <luben.tuikov@amd.com> wrote:
>
> On 2020-10-23 03:46, Takashi Iwai wrote:
> > Hi,
> >
> > the amdgpu driver's ASSERT_CRITICAL() macro calls the
> > kgdb_breakpoing() even if no debug option is set, and this leads to a
> > kernel panic on distro kernels.  The first two patches are the
> > oneliner fixes for those, while the last one is the cleanup of those
> > debug macros.
>
> This looks like good work and solid. Hopefully it gets picked up.
>
> Regards,
> Luben
>
> >
> >
> > Takashi
> >
> > ===
> >
> > Takashi Iwai (3):
> >   drm/amd/display: Fix kernel panic by dal_gpio_open() error
> >   drm/amd/display: Don't invoke kgdb_breakpoint() unconditionally
> >   drm/amd/display: Clean up debug macros
> >
> >  drivers/gpu/drm/amd/display/Kconfig             |  1 +
> >  drivers/gpu/drm/amd/display/dc/gpio/gpio_base.c |  4 +--
> >  drivers/gpu/drm/amd/display/dc/os_types.h       | 33 +++++++++----------------
> >  3 files changed, 15 insertions(+), 23 deletions(-)
> >
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/3] drm/amd/display: Fix kernel panic by breakpoint
  2020-10-26 19:34   ` Alex Deucher
@ 2020-10-26 20:22     ` Kazlauskas, Nicholas
  2020-10-26 20:33       ` Alex Deucher
  0 siblings, 1 reply; 8+ messages in thread
From: Kazlauskas, Nicholas @ 2020-10-26 20:22 UTC (permalink / raw)
  To: Alex Deucher, Luben Tuikov
  Cc: Takashi Iwai, Maling list - DRI developers, LKML, amd-gfx list

Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>

Looks fine to me. Feel free to apply.

Regards,
Nicholas Kazlauskas

On 2020-10-26 3:34 p.m., Alex Deucher wrote:
> Yes, looks good to me as well.  Series is:
> Acked-by: Alex Deucher <alexander.deucher@amd.com>
> I'll give the display guys a few more days to look this over, but if
> there are no objections, I'll apply them.
> 
> Thanks!
> 
> Alex
> 
> On Fri, Oct 23, 2020 at 7:16 PM Luben Tuikov <luben.tuikov@amd.com> wrote:
>>
>> On 2020-10-23 03:46, Takashi Iwai wrote:
>>> Hi,
>>>
>>> the amdgpu driver's ASSERT_CRITICAL() macro calls the
>>> kgdb_breakpoing() even if no debug option is set, and this leads to a
>>> kernel panic on distro kernels.  The first two patches are the
>>> oneliner fixes for those, while the last one is the cleanup of those
>>> debug macros.
>>
>> This looks like good work and solid. Hopefully it gets picked up.
>>
>> Regards,
>> Luben
>>
>>>
>>>
>>> Takashi
>>>
>>> ===
>>>
>>> Takashi Iwai (3):
>>>    drm/amd/display: Fix kernel panic by dal_gpio_open() error
>>>    drm/amd/display: Don't invoke kgdb_breakpoint() unconditionally
>>>    drm/amd/display: Clean up debug macros
>>>
>>>   drivers/gpu/drm/amd/display/Kconfig             |  1 +
>>>   drivers/gpu/drm/amd/display/dc/gpio/gpio_base.c |  4 +--
>>>   drivers/gpu/drm/amd/display/dc/os_types.h       | 33 +++++++++----------------
>>>   3 files changed, 15 insertions(+), 23 deletions(-)
>>>
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> 

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/3] drm/amd/display: Fix kernel panic by breakpoint
  2020-10-26 20:22     ` Kazlauskas, Nicholas
@ 2020-10-26 20:33       ` Alex Deucher
  0 siblings, 0 replies; 8+ messages in thread
From: Alex Deucher @ 2020-10-26 20:33 UTC (permalink / raw)
  To: Kazlauskas, Nicholas
  Cc: Takashi Iwai, Maling list - DRI developers, Luben Tuikov, LKML,
	amd-gfx list

Applied.  Thanks!

Alex

On Mon, Oct 26, 2020 at 4:22 PM Kazlauskas, Nicholas
<nicholas.kazlauskas@amd.com> wrote:
>
> Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
>
> Looks fine to me. Feel free to apply.
>
> Regards,
> Nicholas Kazlauskas
>
> On 2020-10-26 3:34 p.m., Alex Deucher wrote:
> > Yes, looks good to me as well.  Series is:
> > Acked-by: Alex Deucher <alexander.deucher@amd.com>
> > I'll give the display guys a few more days to look this over, but if
> > there are no objections, I'll apply them.
> >
> > Thanks!
> >
> > Alex
> >
> > On Fri, Oct 23, 2020 at 7:16 PM Luben Tuikov <luben.tuikov@amd.com> wrote:
> >>
> >> On 2020-10-23 03:46, Takashi Iwai wrote:
> >>> Hi,
> >>>
> >>> the amdgpu driver's ASSERT_CRITICAL() macro calls the
> >>> kgdb_breakpoing() even if no debug option is set, and this leads to a
> >>> kernel panic on distro kernels.  The first two patches are the
> >>> oneliner fixes for those, while the last one is the cleanup of those
> >>> debug macros.
> >>
> >> This looks like good work and solid. Hopefully it gets picked up.
> >>
> >> Regards,
> >> Luben
> >>
> >>>
> >>>
> >>> Takashi
> >>>
> >>> ===
> >>>
> >>> Takashi Iwai (3):
> >>>    drm/amd/display: Fix kernel panic by dal_gpio_open() error
> >>>    drm/amd/display: Don't invoke kgdb_breakpoint() unconditionally
> >>>    drm/amd/display: Clean up debug macros
> >>>
> >>>   drivers/gpu/drm/amd/display/Kconfig             |  1 +
> >>>   drivers/gpu/drm/amd/display/dc/gpio/gpio_base.c |  4 +--
> >>>   drivers/gpu/drm/amd/display/dc/os_types.h       | 33 +++++++++----------------
> >>>   3 files changed, 15 insertions(+), 23 deletions(-)
> >>>
> >>
> >> _______________________________________________
> >> amd-gfx mailing list
> >> amd-gfx@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> >
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-10-26 20:33 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-23  7:46 [PATCH 0/3] drm/amd/display: Fix kernel panic by breakpoint Takashi Iwai
2020-10-23  7:46 ` [PATCH 1/3] drm/amd/display: Fix kernel panic by dal_gpio_open() error Takashi Iwai
2020-10-23  7:46 ` [PATCH 2/3] drm/amd/display: Don't invoke kgdb_breakpoint() unconditionally Takashi Iwai
2020-10-23  7:46 ` [PATCH 3/3] drm/amd/display: Clean up debug macros Takashi Iwai
2020-10-23 23:16 ` [PATCH 0/3] drm/amd/display: Fix kernel panic by breakpoint Luben Tuikov
2020-10-26 19:34   ` Alex Deucher
2020-10-26 20:22     ` Kazlauskas, Nicholas
2020-10-26 20:33       ` Alex Deucher

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).