[PATCH] gdbstub: synchronize state for individual 'p' register reads

Boden Randin posted 1 patch 6 days, 16 hours ago
gdbstub/gdbstub.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] gdbstub: synchronize state for individual 'p' register reads
Posted by Boden Randin 6 days, 16 hours ago
'p' does not call cpu_synchronize_state like 'g' does.

Accelerators may keep guest CPU state out of sync with QEMU while a
vCPU is running. After a vCPU is stopped, an individual 'p' register
read can therefore observe stale state until something updates the state.

Signed-off-by: Boden Randin <bodenrandin1@gmail.com>
---

Notes:
    I found this while investigating why LLDB was showing the RIP truncated to 32 bits under WHPX. LLDB appears to use individual p register reads by default, while GDB primarily uses g, which is why I noticed the issue much more consistently with LLDB.
    
    This is my first open-source contribution, so please let me know if I missed anything in the patch or submission process.

 gdbstub/gdbstub.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 0a328b0dd4..31a845cfa7 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -1246,7 +1246,7 @@ static void handle_get_reg(GArray *params, void *user_ctx)
         gdb_put_packet("E14");
         return;
     }
-
+    cpu_synchronize_state(gdbserver_state.g_cpu);
     reg_size = gdb_read_register(gdbserver_state.g_cpu,
                                  gdbserver_state.mem_buf,
                                  gdb_get_cmd_param(params, 0)->val_ull);
-- 
2.53.0.windows.1
Re: [PATCH] gdbstub: synchronize state for individual 'p' register reads
Posted by Philippe Mathieu-Daudé 4 days, 14 hours ago
On 20/9/26 04:36, Boden Randin wrote:
> 'p' does not call cpu_synchronize_state like 'g' does.
> 
> Accelerators may keep guest CPU state out of sync with QEMU while a
> vCPU is running. After a vCPU is stopped, an individual 'p' register
> read can therefore observe stale state until something updates the state.

Yeah, I mentioned that to Alex few months ago and have a similar
fix but put my work on hold to avoid clash with Richard series in
the same area, waiting his work to be merged:
https://lore.kernel.org/qemu-devel/20260710205401.836304-1-richard.henderson@linaro.org/

> 
> Signed-off-by: Boden Randin <bodenrandin1@gmail.com>
> ---
> 
> Notes:
>      I found this while investigating why LLDB was showing the RIP truncated to 32 bits under WHPX. LLDB appears to use individual p register reads by default, while GDB primarily uses g, which is why I noticed the issue much more consistently with LLDB.
>      
>      This is my first open-source contribution, so please let me know if I missed anything in the patch or submission process.

Well done!

> 
>   gdbstub/gdbstub.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
> index 0a328b0dd4..31a845cfa7 100644
> --- a/gdbstub/gdbstub.c
> +++ b/gdbstub/gdbstub.c
> @@ -1246,7 +1246,7 @@ static void handle_get_reg(GArray *params, void *user_ctx)
>           gdb_put_packet("E14");
>           return;
>       }
> -
> +    cpu_synchronize_state(gdbserver_state.g_cpu);
>       reg_size = gdb_read_register(gdbserver_state.g_cpu,
>                                    gdbserver_state.mem_buf,
>                                    gdb_get_cmd_param(params, 0)->val_ull);

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>