[PATCH 02/18] gdbstub: Move phy_memory_mode to GDBSystemState

Ilya Leoshkevich posted 18 patches 4 weeks, 1 day ago
[PATCH 02/18] gdbstub: Move phy_memory_mode to GDBSystemState
Posted by Ilya Leoshkevich 4 weeks, 1 day ago
Follow the convention that all the pieces of the global stub state must
be inside a single struct.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 gdbstub/system.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/gdbstub/system.c b/gdbstub/system.c
index 1ad87fe7fdf..5ce357c6c2b 100644
--- a/gdbstub/system.c
+++ b/gdbstub/system.c
@@ -35,6 +35,7 @@
 typedef struct {
     CharBackend chr;
     Chardev *mon_chr;
+    int phy_memory_mode;
 } GDBSystemState;
 
 GDBSystemState gdbserver_system_state;
@@ -445,14 +446,12 @@ void gdb_qemu_exit(int code)
 /*
  * Memory access
  */
-static int phy_memory_mode;
-
 int gdb_target_memory_rw_debug(CPUState *cpu, hwaddr addr,
                                uint8_t *buf, int len, bool is_write)
 {
     CPUClass *cc;
 
-    if (phy_memory_mode) {
+    if (gdbserver_system_state.phy_memory_mode) {
         if (is_write) {
             cpu_physical_memory_write(addr, buf, len);
         } else {
@@ -491,7 +490,8 @@ bool gdb_can_reverse(void)
 void gdb_handle_query_qemu_phy_mem_mode(GArray *params,
                                         void *ctx)
 {
-    g_string_printf(gdbserver_state.str_buf, "%d", phy_memory_mode);
+    g_string_printf(gdbserver_state.str_buf, "%d",
+                    gdbserver_system_state.phy_memory_mode);
     gdb_put_strbuf();
 }
 
@@ -503,9 +503,9 @@ void gdb_handle_set_qemu_phy_mem_mode(GArray *params, void *ctx)
     }
 
     if (!gdb_get_cmd_param(params, 0)->val_ul) {
-        phy_memory_mode = 0;
+        gdbserver_system_state.phy_memory_mode = 0;
     } else {
-        phy_memory_mode = 1;
+        gdbserver_system_state.phy_memory_mode = 1;
     }
     gdb_put_packet("OK");
 }
-- 
2.46.0
Re: [PATCH 02/18] gdbstub: Move phy_memory_mode to GDBSystemState
Posted by Richard Henderson 2 weeks, 3 days ago
On 9/23/24 09:12, Ilya Leoshkevich wrote:
> Follow the convention that all the pieces of the global stub state must
> be inside a single struct.
> 
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>   gdbstub/system.c | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/gdbstub/system.c b/gdbstub/system.c
> index 1ad87fe7fdf..5ce357c6c2b 100644
> --- a/gdbstub/system.c
> +++ b/gdbstub/system.c
> @@ -35,6 +35,7 @@
>   typedef struct {
>       CharBackend chr;
>       Chardev *mon_chr;
> +    int phy_memory_mode;
>   } GDBSystemState;

While you're at it, this should be a bool.

Anyway,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

>   
>   GDBSystemState gdbserver_system_state;
> @@ -445,14 +446,12 @@ void gdb_qemu_exit(int code)
>   /*
>    * Memory access
>    */
> -static int phy_memory_mode;
> -
>   int gdb_target_memory_rw_debug(CPUState *cpu, hwaddr addr,
>                                  uint8_t *buf, int len, bool is_write)
>   {
>       CPUClass *cc;
>   
> -    if (phy_memory_mode) {
> +    if (gdbserver_system_state.phy_memory_mode) {
>           if (is_write) {
>               cpu_physical_memory_write(addr, buf, len);
>           } else {
> @@ -491,7 +490,8 @@ bool gdb_can_reverse(void)
>   void gdb_handle_query_qemu_phy_mem_mode(GArray *params,
>                                           void *ctx)
>   {
> -    g_string_printf(gdbserver_state.str_buf, "%d", phy_memory_mode);
> +    g_string_printf(gdbserver_state.str_buf, "%d",
> +                    gdbserver_system_state.phy_memory_mode);
>       gdb_put_strbuf();
>   }
>   
> @@ -503,9 +503,9 @@ void gdb_handle_set_qemu_phy_mem_mode(GArray *params, void *ctx)
>       }
>   
>       if (!gdb_get_cmd_param(params, 0)->val_ul) {
> -        phy_memory_mode = 0;
> +        gdbserver_system_state.phy_memory_mode = 0;
>       } else {
> -        phy_memory_mode = 1;
> +        gdbserver_system_state.phy_memory_mode = 1;
>       }
>       gdb_put_packet("OK");
>   }