| Message ID | 20250521164250.135776-20-alex.bennee@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Maintainer updates for May (testing, plugins, virtio-gpu) - pre-PR | expand |
On 2025/05/22 1:42, Alex Bennée wrote: > From: Dominik 'Disconnect3d' Czarnota <dominik.b.czarnota@gmail.com> > > This commit adds support for the `qGDBServerVersion` packet to the qemu > gdbstub which could be used by clients to detect the QEMU version > (and, e.g., use a workaround for known bugs). > > This packet is not documented/standarized by GDB but it was implemented > by LLDB gdbstub [0] and is helpful for projects like Pwndbg [1]. > > This has been implemented by Patryk, who I included in Co-authored-by > and who asked me to send the patch. > > [0] https://lldb.llvm.org/resources/lldbgdbremote.html#qgdbserverversion > [1] https://github.com/pwndbg/pwndbg/issues/2648 > > Co-authored-by: Patryk 'patryk4815' Sondej <patryk.sondej@gmail.com> > Signed-off-by: Dominik 'Disconnect3d' Czarnota <dominik.b.czarnota@gmail.com> > Message-Id: <20250403191340.53343-1-dominik.b.czarnota@gmail.com> > [AJB: fix include, checkpatch linewrap] > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > --- > gdbstub/gdbstub.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c > index 6023c80d25..def0b7e877 100644 > --- a/gdbstub/gdbstub.c > +++ b/gdbstub/gdbstub.c > @@ -28,6 +28,7 @@ > #include "qemu/cutils.h" > #include "qemu/module.h" > #include "qemu/error-report.h" > +#include "qemu/target-info.h" > #include "trace.h" > #include "exec/gdbstub.h" > #include "gdbstub/commands.h" > @@ -1597,6 +1598,18 @@ static void handle_query_threads(GArray *params, void *user_ctx) > gdbserver_state.query_cpu = gdb_next_attached_cpu(gdbserver_state.query_cpu); > } > > +static void handle_query_gdb_server_version(GArray *params, void *user_ctx) > +{ > +#if defined(CONFIG_USER_ONLY) > + g_string_printf(gdbserver_state.str_buf, "name:qemu-%s;version:%s;", > + target_name(), QEMU_VERSION); > +#else > + g_string_printf(gdbserver_state.str_buf, "name:qemu-system-%s;version:%s;", > + target_name(), QEMU_VERSION); > +#endif I sugguest passing "qemu" as the name property. I guess LLDB developers chose to explicitly have the key-value pair syntax so that users can have one unified logic for parsing and avoid the mess of HTTP's User-Agent; there is a proposal for Web that introduces key-value pairs in a similar manner: https://developer.chrome.com/docs/privacy-security/user-agent-client-hints If we embed more information like to the name property, users will need to have an additional logic to know if it's QEMU or to know other information. Instead, we can emit: name:qemu;version:10.0.50; and we can use something like follows if we want to tell more: name:qemu;version:10.0.50;qemu-mode:system;qemu-target:hexagon;
Akihiko Odaki <akihiko.odaki@daynix.com> writes: > On 2025/05/22 1:42, Alex Bennée wrote: >> From: Dominik 'Disconnect3d' Czarnota <dominik.b.czarnota@gmail.com> >> This commit adds support for the `qGDBServerVersion` packet to the >> qemu >> gdbstub which could be used by clients to detect the QEMU version >> (and, e.g., use a workaround for known bugs). >> This packet is not documented/standarized by GDB but it was >> implemented >> by LLDB gdbstub [0] and is helpful for projects like Pwndbg [1]. >> This has been implemented by Patryk, who I included in >> Co-authored-by >> and who asked me to send the patch. >> [0] >> https://lldb.llvm.org/resources/lldbgdbremote.html#qgdbserverversion >> [1] https://github.com/pwndbg/pwndbg/issues/2648 >> Co-authored-by: Patryk 'patryk4815' Sondej <patryk.sondej@gmail.com> >> Signed-off-by: Dominik 'Disconnect3d' Czarnota <dominik.b.czarnota@gmail.com> >> Message-Id: <20250403191340.53343-1-dominik.b.czarnota@gmail.com> >> [AJB: fix include, checkpatch linewrap] >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >> --- >> gdbstub/gdbstub.c | 17 +++++++++++++++++ >> 1 file changed, 17 insertions(+) >> diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c >> index 6023c80d25..def0b7e877 100644 >> --- a/gdbstub/gdbstub.c >> +++ b/gdbstub/gdbstub.c >> @@ -28,6 +28,7 @@ >> #include "qemu/cutils.h" >> #include "qemu/module.h" >> #include "qemu/error-report.h" >> +#include "qemu/target-info.h" >> #include "trace.h" >> #include "exec/gdbstub.h" >> #include "gdbstub/commands.h" >> @@ -1597,6 +1598,18 @@ static void handle_query_threads(GArray *params, void *user_ctx) >> gdbserver_state.query_cpu = gdb_next_attached_cpu(gdbserver_state.query_cpu); >> } >> +static void handle_query_gdb_server_version(GArray *params, void >> *user_ctx) >> +{ >> +#if defined(CONFIG_USER_ONLY) >> + g_string_printf(gdbserver_state.str_buf, "name:qemu-%s;version:%s;", >> + target_name(), QEMU_VERSION); >> +#else >> + g_string_printf(gdbserver_state.str_buf, "name:qemu-system-%s;version:%s;", >> + target_name(), QEMU_VERSION); >> +#endif > > I sugguest passing "qemu" as the name property. > > I guess LLDB developers chose to explicitly have the key-value pair > syntax so that users can have one unified logic for parsing and avoid > the mess of HTTP's User-Agent; there is a proposal for Web that > introduces key-value pairs in a similar manner: > https://developer.chrome.com/docs/privacy-security/user-agent-client-hints > > If we embed more information like to the name property, users will > need to have an additional logic to know if it's QEMU or to know other > information. Instead, we can emit: > name:qemu;version:10.0.50; > > and we can use something like follows if we want to tell more: > name:qemu;version:10.0.50;qemu-mode:system;qemu-target:hexagon; I think we are getting into bike shedding territory here. GDB does need a decent way to communicate about supported targets and when it comes up with one we shall implement it. But I don't think we need to give too much thought to this custom response for now.
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c index 6023c80d25..def0b7e877 100644 --- a/gdbstub/gdbstub.c +++ b/gdbstub/gdbstub.c @@ -28,6 +28,7 @@ #include "qemu/cutils.h" #include "qemu/module.h" #include "qemu/error-report.h" +#include "qemu/target-info.h" #include "trace.h" #include "exec/gdbstub.h" #include "gdbstub/commands.h" @@ -1597,6 +1598,18 @@ static void handle_query_threads(GArray *params, void *user_ctx) gdbserver_state.query_cpu = gdb_next_attached_cpu(gdbserver_state.query_cpu); } +static void handle_query_gdb_server_version(GArray *params, void *user_ctx) +{ +#if defined(CONFIG_USER_ONLY) + g_string_printf(gdbserver_state.str_buf, "name:qemu-%s;version:%s;", + target_name(), QEMU_VERSION); +#else + g_string_printf(gdbserver_state.str_buf, "name:qemu-system-%s;version:%s;", + target_name(), QEMU_VERSION); +#endif + gdb_put_strbuf(); +} + static void handle_query_first_threads(GArray *params, void *user_ctx) { gdbserver_state.query_cpu = gdb_first_attached_cpu(); @@ -1842,6 +1855,10 @@ static const GdbCmdParseEntry gdb_gen_query_table[] = { .handler = handle_query_threads, .cmd = "sThreadInfo", }, + { + .handler = handle_query_gdb_server_version, + .cmd = "GDBServerVersion", + }, { .handler = handle_query_first_threads, .cmd = "fThreadInfo",