[v3] Maintainer updates for May (testing, plugins, virtio-gpu) - pre-PR

[PATCH v3 19/20] gdbstub: Implement qGDBServerVersion packet

Posted by Alex Bennée 7 months ago

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
+    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",
-- 
2.39.5

Re: [PATCH v3 19/20] gdbstub: Implement qGDBServerVersion packet

Posted by Philippe Mathieu-Daudé 6 months, 3 weeks ago

On 21/5/25 17: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

g_string_printf() isn't really justified, we usually call
g_string_append().

> +    gdb_put_strbuf();
> +}

Re: [PATCH v3 19/20] gdbstub: Implement qGDBServerVersion packet

Posted by Alex Bennée 6 months, 3 weeks ago

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> On 21/5/25 17: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
>
> g_string_printf() isn't really justified, we usually call
> g_string_append().

How is that meant to work with a format string?

>
>> +    gdb_put_strbuf();
>> +}

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH v3 19/20] gdbstub: Implement qGDBServerVersion packet

Posted by Akihiko Odaki 7 months ago

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;

Re: [PATCH v3 19/20] gdbstub: Implement qGDBServerVersion packet

Posted by Alex Bennée 7 months ago

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.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH v3 19/20] gdbstub: Implement qGDBServerVersion packet

Posted by Akihiko Odaki 7 months ago

On 2025/05/22 19:05, Alex Bennée wrote:
> 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.
> 

My suggestion is to pass just "qemu" as the name. The example with 
"qemu-mode" and "qemu-target" is a demonstration of how additional 
information should be provided, and I don't think we need to provide the 
mode and target information now. We can think of details when adding 
such information.

Regards,
Akihiko Odaki