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(-)
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;
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(); \
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__); \
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(-)
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
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
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
dri-devel@lists.freedesktop.org