From 8117f948f12bc559edf40916e7693512c8c9a50b Mon Sep 17 00:00:00 2001 From: Douglas Anderson Date: Thu, 1 Jun 2023 14:31:50 -0700 Subject: kgdb: Provide a stub kgdb_nmicallback() if !CONFIG_KGDB To save architectures from needing to wrap the call in #ifdefs, add a stub no-op version of kgdb_nmicallback(), which returns 1 if it didn't handle anything. Reviewed-by: Daniel Thompson Signed-off-by: Douglas Anderson Link: https://lore.kernel.org/r/20230601143109.v9.6.Ia3aeac89bb6751b682237e76e5ba594318e4b1aa@changeid Signed-off-by: Daniel Thompson --- include/linux/kgdb.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h index 258cdde8d356..76e891ee9e37 100644 --- a/include/linux/kgdb.h +++ b/include/linux/kgdb.h @@ -365,5 +365,6 @@ extern void kgdb_free_init_mem(void); #define dbg_late_init() static inline void kgdb_panic(const char *msg) {} static inline void kgdb_free_init_mem(void) { } +static inline int kgdb_nmicallback(int cpu, void *regs) { return 1; } #endif /* ! CONFIG_KGDB */ #endif /* _KGDB_H_ */ -- cgit v1.2.3 From 0914e4d3cda853471a72b2c26a516c7619658b87 Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Wed, 17 May 2023 14:47:53 +0200 Subject: kdb: include kdb_private.h for function prototypes The kdb_kbd_cleanup_state() is called from another file through the kdb_private.h file, but that is not included before the definition, causing a W=1 warning: kernel/debug/kdb/kdb_keyboard.c:198:6: error: no previous prototype for 'kdb_kbd_cleanup_state' [-Werror=missing-prototypes] Signed-off-by: Arnd Bergmann Link: https://lore.kernel.org/r/20230517124802.929751-1-arnd@kernel.org Signed-off-by: Daniel Thompson --- kernel/debug/kdb/kdb_keyboard.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/kernel/debug/kdb/kdb_keyboard.c b/kernel/debug/kdb/kdb_keyboard.c index f87c750d3eb3..3c2987f46f6e 100644 --- a/kernel/debug/kdb/kdb_keyboard.c +++ b/kernel/debug/kdb/kdb_keyboard.c @@ -13,6 +13,8 @@ #include #include +#include "kdb_private.h" + /* Keyboard Controller Registers on normal PCs. */ #define KBD_STATUS_REG 0x64 /* Status register (R) */ -- cgit v1.2.3 From 1ed0555850cdaa3113a32891d273f7a859565264 Mon Sep 17 00:00:00 2001 From: Douglas Anderson Date: Wed, 28 Jun 2023 12:56:17 -0700 Subject: kdb: Handle LF in the command parser The main kdb command parser only handles CR (ASCII 13 AKA '\r') today, but not LF (ASCII 10 AKA '\n'). That means that the kdb command parser can handle terminals that send just CR or that send CR+LF but can't handle terminals that send just LF. The fact that kdb didn't handle LF in the command parser tripped up a tool I tried to use with it. Specifically, I was trying to send a command to my device to resume it from kdb using a ChromeOS tool like: dut-control cpu_uart_cmd:"g" That tool only terminates lines with LF, not CR+LF. Arguably the ChromeOS tool should be fixed. After all, officially kdb seems to be designed such that CR+LF is the official line ending transmitted over the wire and that internally a line ending is just '\n' (LF). Some evidence: * uart_poll_put_char(), which is used by kdb, notices a '\n' and converts it to '\r\n'. * kdb functions specifically use '\r' to get a carriage return without a newline. You can see this in the pager where kdb will write a '\r' and then write over the pager prompt. However, all that being said there's no real harm in accepting LF as a command terminator in the kdb parser and doing so seems like it would improve compatibility. After this, I'd expect that things would work OK-ish with a remote terminal that used any of CR, CR+LF, or LF as a line ending. Someone using CR as a line ending might get some ugliness where kdb wasn't able to overwrite the last line, but basic commands would work. Someone using just LF as a line ending would probably also work OK. A few other notes: - It can be noted that "bash" running on an "agetty" handles LF as a line termination with no complaints. - Historically, kdb's "pager" actually handled either CR or LF fine. A very quick inspection would make one think that kdb's pager actually could have paged down two lines instead of one for anyone using CR+LF, but this is generally avoided because of kdb_input_flush(). - Conceivably one could argue that some of this special case logic belongs in uart_poll_get_char() since uart_poll_put_char() handles the '\n' => '\r\n' conversion. I would argue that perhaps we should eventually do the opposite and move the '\n' => '\r\n' out of uart_poll_put_char(). Having that conversion at such a low level could interfere if we ever want to transfer binary data. In addition, if we truly made uart_poll_get_char() the inverse of uart_poll_put_char() it would convert back to '\n' and (ironically) kdb's parser currently only looks for '\r' to find the end of a command. Signed-off-by: Douglas Anderson Link: https://lore.kernel.org/r/20230628125612.1.I5cc6c3d916195f5bcfdf5b75d823f2037707f5dc@changeid Signed-off-by: Daniel Thompson --- kernel/debug/kdb/kdb_io.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/kernel/debug/kdb/kdb_io.c b/kernel/debug/kdb/kdb_io.c index 5c7e9ba7cd6b..813cb6cf72d6 100644 --- a/kernel/debug/kdb/kdb_io.c +++ b/kernel/debug/kdb/kdb_io.c @@ -131,6 +131,7 @@ char kdb_getchar(void) int escape_delay = 0; get_char_func *f, *f_prev = NULL; int key; + static bool last_char_was_cr; for (f = &kdb_poll_funcs[0]; ; ++f) { if (*f == NULL) { @@ -149,6 +150,18 @@ char kdb_getchar(void) continue; } + /* + * The caller expects that newlines are either CR or LF. However + * some terminals send _both_ CR and LF. Avoid having to handle + * this in the caller by stripping the LF if we saw a CR right + * before. + */ + if (last_char_was_cr && key == '\n') { + last_char_was_cr = false; + continue; + } + last_char_was_cr = (key == '\r'); + /* * When the first character is received (or we get a change * input source) we set ourselves up to handle an escape @@ -244,7 +257,8 @@ poll_again: *cp = tmp; } break; - case 13: /* enter */ + case 10: /* linefeed */ + case 13: /* carriage return */ *lastchar++ = '\n'; *lastchar++ = '\0'; if (!KDB_STATE(KGDB_TRANS)) { -- cgit v1.2.3 From b6464883f45ae6412de33e53587974fd86ba811e Mon Sep 17 00:00:00 2001 From: Daniel Thompson Date: Fri, 30 Jun 2023 21:12:06 +0100 Subject: kdb: move kdb_send_sig() declaration to a better header file kdb_send_sig() is defined in the signal code and called from kdb, but the declaration is part of the kdb internal code. Move the declaration to the shared header to avoid the warning: kernel/signal.c:4789:6: error: no previous prototype for 'kdb_send_sig' [-Werror=missing-prototypes] Reported-by: Arnd Bergmann Closes: https://lore.kernel.org/lkml/20230517125423.930967-1-arnd@kernel.org/ Signed-off-by: Daniel Thompson Link: https://lore.kernel.org/r/20230630201206.2396930-1-daniel.thompson@linaro.org --- include/linux/kdb.h | 2 ++ kernel/debug/kdb/kdb_private.h | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/include/linux/kdb.h b/include/linux/kdb.h index 07dfb6a20a1c..f6c2ddb16b95 100644 --- a/include/linux/kdb.h +++ b/include/linux/kdb.h @@ -196,6 +196,8 @@ int kdb_process_cpu(const struct task_struct *p) return cpu; } +extern void kdb_send_sig(struct task_struct *p, int sig); + #ifdef CONFIG_KALLSYMS extern const char *kdb_walk_kallsyms(loff_t *pos); #else /* ! CONFIG_KALLSYMS */ diff --git a/kernel/debug/kdb/kdb_private.h b/kernel/debug/kdb/kdb_private.h index 1f8c519a5f81..548fd4059bf9 100644 --- a/kernel/debug/kdb/kdb_private.h +++ b/kernel/debug/kdb/kdb_private.h @@ -194,7 +194,6 @@ extern char kdb_task_state_char (const struct task_struct *); extern bool kdb_task_state(const struct task_struct *p, const char *mask); extern void kdb_ps_suppressed(void); extern void kdb_ps1(const struct task_struct *p); -extern void kdb_send_sig(struct task_struct *p, int sig); extern char kdb_getchar(void); extern char *kdb_getstr(char *, size_t, const char *); extern void kdb_gdb_state_pass(char *buf); -- cgit v1.2.3