Stephen Brennan <stephen.s.brennan@xxxxxxxxxx> writes: > Amal Raj T <tjarlama@xxxxxxxxx> writes: > >> From: Amal Raj T <amalrajt@xxxxxxxx> >> >> Add a new query `linux.vmcoreinfo` to kgdb that returns >> vmcoreinfo to the client using the mem2ebin encoding. >> Maximum size of output buffer is set to 2x the maximum >> size of VMCOREINFO_BYTES (kgdb_mem2ebin() requires 1x >> for the temporary copy plus another 1x (max) for the >> escaped data). >> >> Link: https://github.com/osandov/drgn/wiki/GDB-Remote-Protocol-proposal:-linux.vmcoreinfo-query-packet >> --- >> kernel/debug/gdbstub.c | 18 ++++++++++++++---- >> lib/Kconfig.kgdb | 1 + >> 2 files changed, 15 insertions(+), 4 deletions(-) >> >> diff --git a/kernel/debug/gdbstub.c b/kernel/debug/gdbstub.c >> index f88e21d5502a..f2c80bd368e2 100644 >> --- a/kernel/debug/gdbstub.c >> +++ b/kernel/debug/gdbstub.c >> @@ -30,20 +30,21 @@ >> #include <linux/kgdb.h> >> #include <linux/kdb.h> >> #include <linux/serial_core.h> >> +#include <linux/string.h> >> #include <linux/reboot.h> >> #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 * 2, BUFMAX)]; > > Looking at the code added to gdb_cmd_query(), the actual maximum size of > the response is VMCOREINFO_BYTES * 2 + 1, to account for the 'Q' > character. This is a large buffer, for most architectures it would be > 8193 bytes, compared to the current 2048 or 4096. > > The more I look at this, the more concerned I am that we're going about > this wrong. GDB has packet types which allow reading uninterpreted bytes > of OS-related data, and they allow specifying a requested offset and > length: > > https://sourceware.org/gdb/current/onlinedocs/gdb.html/General-Query-Packets.html#qXfer-read > > This would ensure that we could break up the data into multiple smaller > packets, which may go with the protocol design better. I can't find any > documented maximum packet size, but in the GDB remote.c code, I see it > being initialized as small as 400. > > I'm aware that I come up with the "qlinux.vmcoreinfo" idea so I'm the > one who should be chastised for missing this potential limitation if it > does exist. I think I'll need to go into the GDB code and play around > with this more, and also share this idea with the binutils people who I > probably should have consulted in the first place. After checking in with some helpful members of the GDB mailing list[1], it sounds like the answer here is to use the qXfer command linked above. Essentially, the format would be like this: Request: qXfer:vmcoreinfo:read::OFFSET,LENGTH Reply: 'm [DATA]' - DATA read from the target OFFSET. More data may be present. The amount of data may be less than LENGTH. 'l [DATA]' - DATA read from the target offset. There is no more data to be read. The amount of data may be less than LENGTH. 'l' - There is no data at that offset The DATA would still be encoded in the same escaped binary format implemented in patch 1. The useful difference here is that we would not need to expand remcom_out_buffer. We simply reply with as much data as requested, or as much as we can fit into the output buffer, whichever is smaller. The client will use multiple requests until the entire data is read. So patch 1 would need to be updated to be aware of remaining space in the buffer, and it would need to do partial reads. Since the escaping scheme rarely needs to escape characters (especially for vmcoreinfo), we can optimistically fill the whole buffer (not just half) and then we could determine the amount of escapes and do the escaping & shifting all at once. Something like this: int kgdb_mem2ebin(char *mem, char *buf, int count, int cap) { int err, i, esc = 0; err = copy_from_kernel_nofault(buf, mem, min(count, cap)); if (err) return err; /* Some characters need to be escaped, but it's uncommon. * Count the number of escaped characters and then perform * the escaping in reverse. */ for (i = 0; i < count && i + esc < cap; i++) { if (buf[i] == '}' || buf[i] == '#' || buf[i] == '*' || buf[i] == '#') { if (i + esc + 1 == cap) { /* Skip characters that need escaping when we only have * one byte of capacity remaining. */ buf[i] = '\0'; break; } else { esc += 1; } } } count = i; i -= 1; while (esc > 0) { char c = buf[i]; if (c == '}' || c == '#' || c == '*' || c == '#') { buf[i + esc] = c ^ 0x20; esc -= 1; buf[i + esc] = '}'; } else { buf[i + esc] = c; } i -= 1; } return count; } [1]: https://inbox.sourceware.org/gdb/87y0zds39y.fsf@xxxxxxxxxx/T/#t
