| Message ID | 20241211153955.33518-4-tjarlama@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add a new command in kgdb for vmcoreinfo | expand |
Hi, On Wed, Dec 11, 2024 at 7:40 AM Amal Raj T <tjarlama@gmail.com> wrote: > > From: Amal Raj T <amalrajt@meta.com> > > Add a new query `linux.vmcoreinfo` to kgdb that returns > vmcoreinfo to the client using the mem2ebin encoding. Can you add more documentation about `linux.vmcoreinfo`? Is there a GDB patch that goes along with this that does something with it? I assume that GDB patch would need the same magic escaping sequence you've come up with? How does one end up having a VM core for kgdb to serve up? Does it automatically get generated before we enter kdb due to a crash now that we select VMCORE_INFO, or is there some other mechanism? > Maximum size of output buffer is set to 3X the maximum > size of VMCOREINFO_BYTES (kgdb_mem2ebin() requires 1x > for the temporary copy plus up to 2x for the escaped > data). Can you explain the 3x more? Specifically, it looks like the temporary copy doesn't take up any extra space, unless I read your code wrong (always possible). Let's assume mem is '}}' and count is 2. When the function starts you end up copying mem to the end of the buffer. Thus, buffer will be '??}}' The first time through the loop, you'll end filling in the escaped first character and the buffer will be '}]}}' The second time through the loop you'll have '}]}]'. ...so I think you need 2x the escaped data (or 2x +1 unless you remove the '\0' termination from your earlier patch). Oh, I guess you actually also need 1 extra byte for the 'Q' in your response? > Signed-off-by: Amal Raj T <amalrajt@meta.com> > --- > kernel/debug/gdbstub.c | 10 +++++++++- > lib/Kconfig.kgdb | 1 + > 2 files changed, 10 insertions(+), 1 deletion(-) > > diff --git a/kernel/debug/gdbstub.c b/kernel/debug/gdbstub.c > index 6198d2eb49c4..5bec444fc6d3 100644 > --- a/kernel/debug/gdbstub.c > +++ b/kernel/debug/gdbstub.c > @@ -34,13 +34,14 @@ > #include <linux/uaccess.h> > #include <asm/cacheflush.h> > #include <linux/unaligned.h> > +#include <linux/vmcore_info.h> > #include "debug_core.h" > > #define KGDB_MAX_THREAD_QUERY 17 > > /* Our I/O buffers. */ > static char remcom_in_buffer[BUFMAX]; > -static char remcom_out_buffer[BUFMAX]; > +static char remcom_out_buffer[MAX(VMCOREINFO_BYTES*3, BUFMAX)]; > static int gdbstub_use_prev_in_buf; > static int gdbstub_prev_in_buf_pos; > > @@ -768,6 +769,13 @@ static void gdb_cmd_query(struct kgdb_state *ks) > *(--ptr) = '\0'; > break; > > + case 'l': > + if (memcmp(remcom_in_buffer + 1, "linux.vmcoreinfo", 16) == 0) { Is there termination in the `remcom_in_buffer`? If so, I'd rather see a string comparison function used. Otherwise `linux.vmcoreinfoWithExtraStuffAtTheEnd` will also match.
diff --git a/kernel/debug/gdbstub.c b/kernel/debug/gdbstub.c index 6198d2eb49c4..5bec444fc6d3 100644 --- a/kernel/debug/gdbstub.c +++ b/kernel/debug/gdbstub.c @@ -34,13 +34,14 @@ #include <linux/uaccess.h> #include <asm/cacheflush.h> #include <linux/unaligned.h> +#include <linux/vmcore_info.h> #include "debug_core.h" #define KGDB_MAX_THREAD_QUERY 17 /* Our I/O buffers. */ static char remcom_in_buffer[BUFMAX]; -static char remcom_out_buffer[BUFMAX]; +static char remcom_out_buffer[MAX(VMCOREINFO_BYTES*3, BUFMAX)]; static int gdbstub_use_prev_in_buf; static int gdbstub_prev_in_buf_pos; @@ -768,6 +769,13 @@ static void gdb_cmd_query(struct kgdb_state *ks) *(--ptr) = '\0'; break; + case 'l': + if (memcmp(remcom_in_buffer + 1, "linux.vmcoreinfo", 16) == 0) { + remcom_out_buffer[0] = 'Q'; + kgdb_mem2ebin(vmcoreinfo_data, remcom_out_buffer + 1, vmcoreinfo_size); + } + break; + case 'C': /* Current thread id */ strcpy(remcom_out_buffer, "QC"); diff --git a/lib/Kconfig.kgdb b/lib/Kconfig.kgdb index 537e1b3f5734..012529eee79e 100644 --- a/lib/Kconfig.kgdb +++ b/lib/Kconfig.kgdb @@ -12,6 +12,7 @@ menuconfig KGDB bool "KGDB: kernel debugger" depends on HAVE_ARCH_KGDB depends on DEBUG_KERNEL + select VMCORE_INFO help If you say Y here, it will be possible to remotely debug the kernel using gdb. It is recommended but not required, that