[PULL 00/28 for 5.0] testing and gdbstub updates

Alex Bennée posted 28 patches 4 years, 1 month ago
Only 9 patches received!
There is a newer version of this series
configure                                    |   9 +
include/exec/gdbstub.h                       |  62 +-
include/hw/core/cpu.h                        |   2 +-
target/alpha/cpu.h                           |   2 +-
target/arm/cpu.h                             |  31 +-
target/cris/cpu.h                            |   4 +-
target/hppa/cpu.h                            |   2 +-
target/i386/cpu.h                            |   2 +-
target/lm32/cpu.h                            |   2 +-
target/m68k/cpu.h                            |   2 +-
target/microblaze/cpu.h                      |   2 +-
target/mips/internal.h                       |   2 +-
target/openrisc/cpu.h                        |   2 +-
target/ppc/cpu.h                             |   4 +-
target/riscv/cpu.h                           |   2 +-
target/s390x/internal.h                      |   2 +-
target/sh4/cpu.h                             |   2 +-
target/sparc/cpu.h                           |   2 +-
target/xtensa/cpu.h                          |   2 +-
gdbstub.c                                    | 936 +++++++++++++--------------
hw/core/cpu.c                                |   2 +-
target/alpha/gdbstub.c                       |   2 +-
target/arm/cpu.c                             |   7 +-
target/arm/gdbstub.c                         | 173 ++++-
target/arm/gdbstub64.c                       |   2 +-
target/arm/helper.c                          | 186 +++++-
target/cris/gdbstub.c                        |   4 +-
target/hppa/gdbstub.c                        |   2 +-
target/i386/gdbstub.c                        |  29 +-
target/lm32/gdbstub.c                        |   2 +-
target/m68k/gdbstub.c                        |   2 +-
target/m68k/helper.c                         |  33 +-
target/microblaze/gdbstub.c                  |   2 +-
target/mips/gdbstub.c                        |   2 +-
target/nios2/cpu.c                           |   2 +-
target/openrisc/gdbstub.c                    |   2 +-
target/ppc/gdbstub.c                         |  48 +-
target/ppc/translate_init.inc.c              |  54 +-
target/riscv/gdbstub.c                       |  20 +-
target/s390x/gdbstub.c                       |  30 +-
target/sh4/gdbstub.c                         |   2 +-
target/sparc/gdbstub.c                       |   2 +-
target/xtensa/gdbstub.c                      |   2 +-
tests/tcg/aarch64/sve-ioctls.c               |  70 ++
tests/tcg/aarch64/sysregs.c                  | 172 +++++
.travis.yml                                  |   1 +
tests/.gitignore                             |   1 +
tests/docker/dockerfiles/debian-amd64.docker |   6 +-
tests/docker/dockerfiles/debian10.docker     |   3 +
tests/docker/dockerfiles/debian9.docker      |   3 +
tests/guest-debug/run-test.py                |  57 ++
tests/tcg/aarch64/Makefile.target            |  32 +
tests/tcg/aarch64/gdbstub/test-sve-ioctl.py  |  85 +++
tests/tcg/aarch64/gdbstub/test-sve.py        |  84 +++
54 files changed, 1497 insertions(+), 701 deletions(-)
create mode 100644 tests/tcg/aarch64/sve-ioctls.c
create mode 100644 tests/tcg/aarch64/sysregs.c
create mode 100755 tests/guest-debug/run-test.py
create mode 100644 tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
create mode 100644 tests/tcg/aarch64/gdbstub/test-sve.py
[PULL 00/28 for 5.0] testing and gdbstub updates
Posted by Alex Bennée 4 years, 1 month ago
The following changes since commit 6fb1603aa24d9212493e4819d7b685be9c9aad7a:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20200317' into staging (2020-03-17 14:44:50 +0000)

are available in the Git repository at:

  https://github.com/stsquad/qemu.git tags/pull-testing-and-gdbstub-170320-1

for you to fetch changes up to 3bc2609d478779be600fd66744eb4e3730ec5e33:

  gdbstub: Fix single-step issue by confirming 'vContSupported+' feature to gdb (2020-03-17 17:38:52 +0000)

----------------------------------------------------------------
Testing and gdbstub updates:

  - docker updates for VirGL
  - re-factor gdbstub for static GDBState
  - re-factor gdbstub for dynamic arrays
  - add SVE support to arm gdbstub
  - add some guest debug tests to check-tcg
  - add aarch64 userspace register tests
  - remove packet size limit to gdbstub
  - simplify gdbstub monitor code
  - report vContSupported in gdbstub to use proper single-step

----------------------------------------------------------------
Alex Bennée (20):
      gdbstub: make GDBState static and have common init function
      gdbstub: stop passing GDBState * around and use global
      gdbstub: move str_buf to GDBState and use GString
      gdbstub: move mem_buf to GDBState and use GByteArray
      gdbstub: add helper for 128 bit registers
      target/arm: use gdb_get_reg helpers
      target/m68k: use gdb_get_reg helpers
      target/i386: use gdb_get_reg helpers
      gdbstub: extend GByteArray to read register helpers
      target/arm: prepare for multiple dynamic XMLs
      target/arm: explicitly encode regnum in our XML
      target/arm: default SVE length to 64 bytes for linux-user
      target/arm: generate xml description of our SVE registers
      target/arm: don't bother with id_aa64pfr0_read for USER_ONLY
      tests/tcg/aarch64: userspace system register test
      configure: allow user to specify what gdb to use
      tests/guest-debug: add a simple test runner
      tests/tcg/aarch64: add a gdbstub testcase for SVE registers
      tests/tcg/aarch64: add SVE iotcl test
      tests/tcg/aarch64: add test-sve-ioctl guest-debug test

Changbin Du (1):
      gdbstub: Fix single-step issue by confirming 'vContSupported+' feature to gdb

Damien Hedde (2):
      gdbstub: change GDBState.last_packet to GByteArray
      gdbstub: do not split gdb_monitor_write payload

Philippe Mathieu-Daudé (5):
      tests/docker: Install tools to cross-debug and build Linux kernels
      tests/docker: Update VirGL git repository URL
      tests/docker: Remove obsolete VirGL --with-glx configure option
      tests/docker: Update VirGL to v0.8.0
      travis.yml: Set G_MESSAGES_DEBUG do report GLib errors

 configure                                    |   9 +
 include/exec/gdbstub.h                       |  62 +-
 include/hw/core/cpu.h                        |   2 +-
 target/alpha/cpu.h                           |   2 +-
 target/arm/cpu.h                             |  31 +-
 target/cris/cpu.h                            |   4 +-
 target/hppa/cpu.h                            |   2 +-
 target/i386/cpu.h                            |   2 +-
 target/lm32/cpu.h                            |   2 +-
 target/m68k/cpu.h                            |   2 +-
 target/microblaze/cpu.h                      |   2 +-
 target/mips/internal.h                       |   2 +-
 target/openrisc/cpu.h                        |   2 +-
 target/ppc/cpu.h                             |   4 +-
 target/riscv/cpu.h                           |   2 +-
 target/s390x/internal.h                      |   2 +-
 target/sh4/cpu.h                             |   2 +-
 target/sparc/cpu.h                           |   2 +-
 target/xtensa/cpu.h                          |   2 +-
 gdbstub.c                                    | 936 +++++++++++++--------------
 hw/core/cpu.c                                |   2 +-
 target/alpha/gdbstub.c                       |   2 +-
 target/arm/cpu.c                             |   7 +-
 target/arm/gdbstub.c                         | 173 ++++-
 target/arm/gdbstub64.c                       |   2 +-
 target/arm/helper.c                          | 186 +++++-
 target/cris/gdbstub.c                        |   4 +-
 target/hppa/gdbstub.c                        |   2 +-
 target/i386/gdbstub.c                        |  29 +-
 target/lm32/gdbstub.c                        |   2 +-
 target/m68k/gdbstub.c                        |   2 +-
 target/m68k/helper.c                         |  33 +-
 target/microblaze/gdbstub.c                  |   2 +-
 target/mips/gdbstub.c                        |   2 +-
 target/nios2/cpu.c                           |   2 +-
 target/openrisc/gdbstub.c                    |   2 +-
 target/ppc/gdbstub.c                         |  48 +-
 target/ppc/translate_init.inc.c              |  54 +-
 target/riscv/gdbstub.c                       |  20 +-
 target/s390x/gdbstub.c                       |  30 +-
 target/sh4/gdbstub.c                         |   2 +-
 target/sparc/gdbstub.c                       |   2 +-
 target/xtensa/gdbstub.c                      |   2 +-
 tests/tcg/aarch64/sve-ioctls.c               |  70 ++
 tests/tcg/aarch64/sysregs.c                  | 172 +++++
 .travis.yml                                  |   1 +
 tests/.gitignore                             |   1 +
 tests/docker/dockerfiles/debian-amd64.docker |   6 +-
 tests/docker/dockerfiles/debian10.docker     |   3 +
 tests/docker/dockerfiles/debian9.docker      |   3 +
 tests/guest-debug/run-test.py                |  57 ++
 tests/tcg/aarch64/Makefile.target            |  32 +
 tests/tcg/aarch64/gdbstub/test-sve-ioctl.py  |  85 +++
 tests/tcg/aarch64/gdbstub/test-sve.py        |  84 +++
 54 files changed, 1497 insertions(+), 701 deletions(-)
 create mode 100644 tests/tcg/aarch64/sve-ioctls.c
 create mode 100644 tests/tcg/aarch64/sysregs.c
 create mode 100755 tests/guest-debug/run-test.py
 create mode 100644 tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
 create mode 100644 tests/tcg/aarch64/gdbstub/test-sve.py

-- 
2.20.1


Re: [PULL 00/28 for 5.0] testing and gdbstub updates
Posted by Peter Maydell 4 years, 1 month ago
On Tue, 17 Mar 2020 at 17:50, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> The following changes since commit 6fb1603aa24d9212493e4819d7b685be9c9aad7a:
>
>   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20200317' into staging (2020-03-17 14:44:50 +0000)
>
> are available in the Git repository at:
>
>   https://github.com/stsquad/qemu.git tags/pull-testing-and-gdbstub-170320-1
>
> for you to fetch changes up to 3bc2609d478779be600fd66744eb4e3730ec5e33:
>
>   gdbstub: Fix single-step issue by confirming 'vContSupported+' feature to gdb (2020-03-17 17:38:52 +0000)
>
> ----------------------------------------------------------------
> Testing and gdbstub updates:
>
>   - docker updates for VirGL
>   - re-factor gdbstub for static GDBState
>   - re-factor gdbstub for dynamic arrays
>   - add SVE support to arm gdbstub
>   - add some guest debug tests to check-tcg
>   - add aarch64 userspace register tests
>   - remove packet size limit to gdbstub
>   - simplify gdbstub monitor code
>   - report vContSupported in gdbstub to use proper single-step
>
> ----------------------------------------------------------------


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.0
for any user-visible changes.

-- PMM

[PULL 02/28] tests/docker: Update VirGL git repository URL
Posted by Alex Bennée 4 years, 1 month ago
From: Philippe Mathieu-Daudé <philmd@redhat.com>

freedesktop.org is moving to a GitLab instance,
use the new url.

- https://www.fooishbar.org/blog/gitlab-fdo-introduction/
- https://gitlab.freedesktop.org/freedesktop/freedesktop/-/wikis/home

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20200212202709.12665-2-philmd@redhat.com>
Message-Id: <20200316172155.971-3-alex.bennee@linaro.org>

diff --git a/tests/docker/dockerfiles/debian-amd64.docker b/tests/docker/dockerfiles/debian-amd64.docker
index 3b860af1068..b67fad54cb7 100644
--- a/tests/docker/dockerfiles/debian-amd64.docker
+++ b/tests/docker/dockerfiles/debian-amd64.docker
@@ -27,7 +27,7 @@ RUN apt update && \
         libegl1-mesa-dev \
         libepoxy-dev \
         libgbm-dev
-RUN git clone https://anongit.freedesktop.org/git/virglrenderer.git /usr/src/virglrenderer && \
+RUN git clone https://gitlab.freedesktop.org/virgl/virglrenderer.git /usr/src/virglrenderer && \
     cd /usr/src/virglrenderer && git checkout virglrenderer-0.7.0
 RUN cd /usr/src/virglrenderer && ./autogen.sh && ./configure --with-glx --disable-tests && make install
 
-- 
2.20.1


[PULL 03/28] tests/docker: Remove obsolete VirGL --with-glx configure option
Posted by Alex Bennée 4 years, 1 month ago
From: Philippe Mathieu-Daudé <philmd@redhat.com>

The GLX configure option has been removed in 71c75f201d [*].
We missed that when updating to v0.7.0 in commit fab3220f97.

This silents:

  configure: creating ./config.status
  config.status: creating virglrenderer.pc
  ...
  configure: WARNING: unrecognized options: --with-glx

[*] https://gitlab.freedesktop.org/virgl/virglrenderer/commit/71c75f201d

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20200212202709.12665-3-philmd@redhat.com>
Message-Id: <20200316172155.971-4-alex.bennee@linaro.org>

diff --git a/tests/docker/dockerfiles/debian-amd64.docker b/tests/docker/dockerfiles/debian-amd64.docker
index b67fad54cb7..a1d40a56fa1 100644
--- a/tests/docker/dockerfiles/debian-amd64.docker
+++ b/tests/docker/dockerfiles/debian-amd64.docker
@@ -29,7 +29,7 @@ RUN apt update && \
         libgbm-dev
 RUN git clone https://gitlab.freedesktop.org/virgl/virglrenderer.git /usr/src/virglrenderer && \
     cd /usr/src/virglrenderer && git checkout virglrenderer-0.7.0
-RUN cd /usr/src/virglrenderer && ./autogen.sh && ./configure --with-glx --disable-tests && make install
+RUN cd /usr/src/virglrenderer && ./autogen.sh && ./configure --disable-tests && make install
 
 # netmap
 RUN apt update && \
-- 
2.20.1


[PULL 04/28] tests/docker: Update VirGL to v0.8.0
Posted by Alex Bennée 4 years, 1 month ago
From: Philippe Mathieu-Daudé <philmd@redhat.com>

Building the qemu:debian-amd64 fails when building VirGL:

  make[2]: Entering directory '/usr/src/virglrenderer/src/gallium/auxiliary'
    CC       cso_cache/cso_cache.lo
    CC       cso_cache/cso_hash.lo
    CC       os/os_misc.lo
    CC       util/u_debug.lo
    CC       util/u_debug_describe.lo
    CC       util/u_format.lo
    GEN      util/u_format_table.c
  Traceback (most recent call last):
    File "./util/u_format_table.py", line 168, in <module>
      main()
    File "./util/u_format_table.py", line 164, in main
      write_format_table(formats)
    File "./util/u_format_table.py", line 132, in write_format_table
      print("   %s,\t/* is_array */" % (bool_map(format.is_array()),))
    File "/usr/src/virglrenderer/src/gallium/auxiliary/util/u_format_parse.py", line 164, in is_array
      return self.array_element() != None
    File "/usr/src/virglrenderer/src/gallium/auxiliary/util/u_format_parse.py", line 73, in __eq__
      return self.type == other.type and self.norm == other.norm and self.pure == other.pure and self.size == other.size
  AttributeError: 'NoneType' object has no attribute 'type'
  make[2]: Leaving directory '/usr/src/virglrenderer/src/gallium/auxiliary'
  make[2]: *** [Makefile:906: util/u_format_table.c] Error 1
  make[1]: *** [Makefile:631: install-recursive] Error 1

VirGL commits a8962eda1..a613dcc82 fix this problem.
Update to VirGL 0.8.0 which contains them.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20200212202709.12665-4-philmd@redhat.com>
Message-Id: <20200316172155.971-5-alex.bennee@linaro.org>

diff --git a/tests/docker/dockerfiles/debian-amd64.docker b/tests/docker/dockerfiles/debian-amd64.docker
index a1d40a56fa1..c70c916343e 100644
--- a/tests/docker/dockerfiles/debian-amd64.docker
+++ b/tests/docker/dockerfiles/debian-amd64.docker
@@ -28,7 +28,7 @@ RUN apt update && \
         libepoxy-dev \
         libgbm-dev
 RUN git clone https://gitlab.freedesktop.org/virgl/virglrenderer.git /usr/src/virglrenderer && \
-    cd /usr/src/virglrenderer && git checkout virglrenderer-0.7.0
+    cd /usr/src/virglrenderer && git checkout virglrenderer-0.8.0
 RUN cd /usr/src/virglrenderer && ./autogen.sh && ./configure --disable-tests && make install
 
 # netmap
-- 
2.20.1


[PULL 05/28] travis.yml: Set G_MESSAGES_DEBUG do report GLib errors
Posted by Alex Bennée 4 years, 1 month ago
From: Philippe Mathieu-Daudé <philmd@redhat.com>

Since commit f5852efa293 we can display GLib errors with the QEMU
error reporting API. Set it to the 'error' level, as this helps
understanding failures from QEMU calls to GLib on Travis-CI.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20200316101544.22361-1-philmd@redhat.com>
Message-Id: <20200316172155.971-6-alex.bennee@linaro.org>

diff --git a/.travis.yml b/.travis.yml
index b92798ac3b9..ccf68aa9abc 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -79,6 +79,7 @@ env:
     - MAIN_SOFTMMU_TARGETS="aarch64-softmmu,mips64-softmmu,ppc64-softmmu,riscv64-softmmu,s390x-softmmu,x86_64-softmmu"
     - CCACHE_SLOPPINESS="include_file_ctime,include_file_mtime"
     - CCACHE_MAXSIZE=1G
+    - G_MESSAGES_DEBUG=error
 
 
 git:
-- 
2.20.1


[PULL 06/28] gdbstub: make GDBState static and have common init function
Posted by Alex Bennée 4 years, 1 month ago
Instead of allocating make this entirely static. We shall reduce the
size of the structure in later commits and dynamically allocate parts
of it. We introduce an init and reset helper function to keep all the
manipulation in one place.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Damien Hedde <damien.hedde@greensocs.com>

Message-Id: <20200316172155.971-7-alex.bennee@linaro.org>

diff --git a/gdbstub.c b/gdbstub.c
index 22a2d630cdc..57d6e50ddfc 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -342,6 +342,7 @@ enum RSState {
     RS_CHKSUM2,
 };
 typedef struct GDBState {
+    bool init;       /* have we been initialised? */
     CPUState *c_cpu; /* current CPU for step/continue ops */
     CPUState *g_cpu; /* current CPU for other ops */
     CPUState *query_cpu; /* for q{f|s}ThreadInfo */
@@ -372,7 +373,23 @@ typedef struct GDBState {
  */
 static int sstep_flags = SSTEP_ENABLE|SSTEP_NOIRQ|SSTEP_NOTIMER;
 
-static GDBState *gdbserver_state;
+static GDBState gdbserver_state;
+
+static void init_gdbserver_state(void)
+{
+    g_assert(!gdbserver_state.init);
+    memset(&gdbserver_state, 0, sizeof(GDBState));
+    gdbserver_state.init = true;
+}
+
+#ifndef CONFIG_USER_ONLY
+static void reset_gdbserver_state(void)
+{
+    g_free(gdbserver_state.processes);
+    gdbserver_state.processes = NULL;
+    gdbserver_state.process_num = 0;
+}
+#endif
 
 bool gdb_has_xml;
 
@@ -425,8 +442,8 @@ int use_gdb_syscalls(void)
     /* -semihosting-config target=auto */
     /* On the first call check if gdb is connected and remember. */
     if (gdb_syscall_mode == GDB_SYS_UNKNOWN) {
-        gdb_syscall_mode = (gdbserver_state ? GDB_SYS_ENABLED
-                                            : GDB_SYS_DISABLED);
+        gdb_syscall_mode = gdbserver_state.init ?
+            GDB_SYS_ENABLED : GDB_SYS_DISABLED;
     }
     return gdb_syscall_mode == GDB_SYS_ENABLED;
 }
@@ -984,7 +1001,7 @@ static int gdb_breakpoint_insert(int type, target_ulong addr, target_ulong len)
     int err = 0;
 
     if (kvm_enabled()) {
-        return kvm_insert_breakpoint(gdbserver_state->c_cpu, addr, len, type);
+        return kvm_insert_breakpoint(gdbserver_state.c_cpu, addr, len, type);
     }
 
     switch (type) {
@@ -1021,7 +1038,7 @@ static int gdb_breakpoint_remove(int type, target_ulong addr, target_ulong len)
     int err = 0;
 
     if (kvm_enabled()) {
-        return kvm_remove_breakpoint(gdbserver_state->c_cpu, addr, len, type);
+        return kvm_remove_breakpoint(gdbserver_state.c_cpu, addr, len, type);
     }
 
     switch (type) {
@@ -1074,7 +1091,7 @@ static void gdb_breakpoint_remove_all(void)
     CPUState *cpu;
 
     if (kvm_enabled()) {
-        kvm_remove_all_breakpoints(gdbserver_state->c_cpu);
+        kvm_remove_all_breakpoints(gdbserver_state.c_cpu);
         return;
     }
 
@@ -2601,7 +2618,7 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
 
 void gdb_set_stop_cpu(CPUState *cpu)
 {
-    GDBProcess *p = gdb_get_cpu_process(gdbserver_state, cpu);
+    GDBProcess *p = gdb_get_cpu_process(&gdbserver_state, cpu);
 
     if (!p->attached) {
         /*
@@ -2611,14 +2628,14 @@ void gdb_set_stop_cpu(CPUState *cpu)
         return;
     }
 
-    gdbserver_state->c_cpu = cpu;
-    gdbserver_state->g_cpu = cpu;
+    gdbserver_state.c_cpu = cpu;
+    gdbserver_state.g_cpu = cpu;
 }
 
 #ifndef CONFIG_USER_ONLY
 static void gdb_vm_state_change(void *opaque, int running, RunState state)
 {
-    GDBState *s = gdbserver_state;
+    GDBState *s = &gdbserver_state;
     CPUState *cpu = s->c_cpu;
     char buf[256];
     char thread_id[16];
@@ -2722,17 +2739,16 @@ void gdb_do_syscallv(gdb_syscall_complete_cb cb, const char *fmt, va_list va)
     char *p_end;
     target_ulong addr;
     uint64_t i64;
-    GDBState *s;
 
-    s = gdbserver_state;
-    if (!s)
+    if (!gdbserver_state.init)
         return;
-    s->current_syscall_cb = cb;
+
+    gdbserver_state.current_syscall_cb = cb;
 #ifndef CONFIG_USER_ONLY
     vm_stop(RUN_STATE_DEBUG);
 #endif
-    p = s->syscall_buf;
-    p_end = &s->syscall_buf[sizeof(s->syscall_buf)];
+    p = &gdbserver_state.syscall_buf[0];
+    p_end = &gdbserver_state.syscall_buf[sizeof(gdbserver_state.syscall_buf)];
     *(p++) = 'F';
     while (*fmt) {
         if (*fmt == '%') {
@@ -2765,14 +2781,14 @@ void gdb_do_syscallv(gdb_syscall_complete_cb cb, const char *fmt, va_list va)
     }
     *p = 0;
 #ifdef CONFIG_USER_ONLY
-    put_packet(s, s->syscall_buf);
+    put_packet(&gdbserver_state, gdbserver_state.syscall_buf);
     /* Return control to gdb for it to process the syscall request.
      * Since the protocol requires that gdb hands control back to us
      * using a "here are the results" F packet, we don't need to check
      * gdb_handlesig's return value (which is the signal to deliver if
      * execution was resumed via a continue packet).
      */
-    gdb_handlesig(s->c_cpu, 0);
+    gdb_handlesig(gdbserver_state.c_cpu, 0);
 #else
     /* In this case wait to send the syscall packet until notification that
        the CPU has stopped.  This must be done because if the packet is sent
@@ -2780,7 +2796,7 @@ void gdb_do_syscallv(gdb_syscall_complete_cb cb, const char *fmt, va_list va)
        is still in the running state, which can cause packets to be dropped
        and state transition 'T' packets to be sent while the syscall is still
        being processed.  */
-    qemu_cpu_kick(s->c_cpu);
+    qemu_cpu_kick(gdbserver_state.c_cpu);
 #endif
 }
 
@@ -2941,15 +2957,13 @@ static void gdb_read_byte(GDBState *s, uint8_t ch)
 /* Tell the remote gdb that the process has exited.  */
 void gdb_exit(CPUArchState *env, int code)
 {
-  GDBState *s;
   char buf[4];
 
-  s = gdbserver_state;
-  if (!s) {
+  if (!gdbserver_state.init) {
       return;
   }
 #ifdef CONFIG_USER_ONLY
-  if (gdbserver_fd < 0 || s->fd < 0) {
+  if (gdbserver_fd < 0 || gdbserver_state.fd < 0) {
       return;
   }
 #endif
@@ -2957,10 +2971,10 @@ void gdb_exit(CPUArchState *env, int code)
   trace_gdbstub_op_exiting((uint8_t)code);
 
   snprintf(buf, sizeof(buf), "W%02x", (uint8_t)code);
-  put_packet(s, buf);
+  put_packet(&gdbserver_state, buf);
 
 #ifndef CONFIG_USER_ONLY
-  qemu_chr_fe_deinit(&s->chr, true);
+  qemu_chr_fe_deinit(&gdbserver_state.chr, true);
 #endif
 }
 
@@ -2993,12 +3007,10 @@ static void create_default_process(GDBState *s)
 int
 gdb_handlesig(CPUState *cpu, int sig)
 {
-    GDBState *s;
     char buf[256];
     int n;
 
-    s = gdbserver_state;
-    if (gdbserver_fd < 0 || s->fd < 0) {
+    if (gdbserver_fd < 0 || gdbserver_state.fd < 0) {
         return sig;
     }
 
@@ -3008,58 +3020,55 @@ gdb_handlesig(CPUState *cpu, int sig)
 
     if (sig != 0) {
         snprintf(buf, sizeof(buf), "S%02x", target_signal_to_gdb(sig));
-        put_packet(s, buf);
+        put_packet(&gdbserver_state, buf);
     }
     /* put_packet() might have detected that the peer terminated the
        connection.  */
-    if (s->fd < 0) {
+    if (gdbserver_state.fd < 0) {
         return sig;
     }
 
     sig = 0;
-    s->state = RS_IDLE;
-    s->running_state = 0;
-    while (s->running_state == 0) {
-        n = read(s->fd, buf, 256);
+    gdbserver_state.state = RS_IDLE;
+    gdbserver_state.running_state = 0;
+    while (gdbserver_state.running_state == 0) {
+        n = read(gdbserver_state.fd, buf, 256);
         if (n > 0) {
             int i;
 
             for (i = 0; i < n; i++) {
-                gdb_read_byte(s, buf[i]);
+                gdb_read_byte(&gdbserver_state, buf[i]);
             }
         } else {
             /* XXX: Connection closed.  Should probably wait for another
                connection before continuing.  */
             if (n == 0) {
-                close(s->fd);
+                close(gdbserver_state.fd);
             }
-            s->fd = -1;
+            gdbserver_state.fd = -1;
             return sig;
         }
     }
-    sig = s->signal;
-    s->signal = 0;
+    sig = gdbserver_state.signal;
+    gdbserver_state.signal = 0;
     return sig;
 }
 
 /* Tell the remote gdb that the process has exited due to SIG.  */
 void gdb_signalled(CPUArchState *env, int sig)
 {
-    GDBState *s;
     char buf[4];
 
-    s = gdbserver_state;
-    if (gdbserver_fd < 0 || s->fd < 0) {
+    if (gdbserver_fd < 0 || gdbserver_state.fd < 0) {
         return;
     }
 
     snprintf(buf, sizeof(buf), "X%02x", target_signal_to_gdb(sig));
-    put_packet(s, buf);
+    put_packet(&gdbserver_state, buf);
 }
 
 static bool gdb_accept(void)
 {
-    GDBState *s;
     struct sockaddr_in sockaddr;
     socklen_t len;
     int fd;
@@ -3083,15 +3092,13 @@ static bool gdb_accept(void)
         return false;
     }
 
-    s = g_malloc0(sizeof(GDBState));
-    create_default_process(s);
-    s->processes[0].attached = true;
-    s->c_cpu = gdb_first_attached_cpu(s);
-    s->g_cpu = s->c_cpu;
-    s->fd = fd;
+    init_gdbserver_state();
+    create_default_process(&gdbserver_state);
+    gdbserver_state.processes[0].attached = true;
+    gdbserver_state.c_cpu = gdb_first_attached_cpu(&gdbserver_state);
+    gdbserver_state.g_cpu = gdbserver_state.c_cpu;
+    gdbserver_state.fd = fd;
     gdb_has_xml = false;
-
-    gdbserver_state = s;
     return true;
 }
 
@@ -3144,13 +3151,11 @@ int gdbserver_start(int port)
 /* Disable gdb stub for child processes.  */
 void gdbserver_fork(CPUState *cpu)
 {
-    GDBState *s = gdbserver_state;
-
-    if (gdbserver_fd < 0 || s->fd < 0) {
+    if (gdbserver_fd < 0 || gdbserver_state.fd < 0) {
         return;
     }
-    close(s->fd);
-    s->fd = -1;
+    close(gdbserver_state.fd);
+    gdbserver_state.fd = -1;
     cpu_breakpoint_remove_all(cpu, BP_GDB);
     cpu_watchpoint_remove_all(cpu, BP_GDB);
 }
@@ -3167,7 +3172,7 @@ static void gdb_chr_receive(void *opaque, const uint8_t *buf, int size)
     int i;
 
     for (i = 0; i < size; i++) {
-        gdb_read_byte(gdbserver_state, buf[i]);
+        gdb_read_byte(&gdbserver_state, buf[i]);
     }
 }
 
@@ -3210,13 +3215,13 @@ static int gdb_monitor_write(Chardev *chr, const uint8_t *buf, int len)
     const char *p = (const char *)buf;
     int max_sz;
 
-    max_sz = (sizeof(gdbserver_state->last_packet) - 2) / 2;
+    max_sz = (sizeof(gdbserver_state.last_packet) - 2) / 2;
     for (;;) {
         if (len <= max_sz) {
-            gdb_monitor_output(gdbserver_state, p, len);
+            gdb_monitor_output(&gdbserver_state, p, len);
             break;
         }
-        gdb_monitor_output(gdbserver_state, p, max_sz);
+        gdb_monitor_output(&gdbserver_state, p, max_sz);
         p += max_sz;
         len -= max_sz;
     }
@@ -3308,18 +3313,10 @@ static void create_processes(GDBState *s)
     create_default_process(s);
 }
 
-static void cleanup_processes(GDBState *s)
-{
-    g_free(s->processes);
-    s->process_num = 0;
-    s->processes = NULL;
-}
-
 int gdbserver_start(const char *device)
 {
     trace_gdbstub_op_start(device);
 
-    GDBState *s;
     char gdbstub_device_name[128];
     Chardev *chr = NULL;
     Chardev *mon_chr;
@@ -3357,10 +3354,8 @@ int gdbserver_start(const char *device)
             return -1;
     }
 
-    s = gdbserver_state;
-    if (!s) {
-        s = g_malloc0(sizeof(GDBState));
-        gdbserver_state = s;
+    if (!gdbserver_state.init) {
+        init_gdbserver_state();
 
         qemu_add_vm_change_state_handler(gdb_vm_state_change, NULL);
 
@@ -3369,31 +3364,30 @@ int gdbserver_start(const char *device)
                                    NULL, NULL, &error_abort);
         monitor_init_hmp(mon_chr, false, &error_abort);
     } else {
-        qemu_chr_fe_deinit(&s->chr, true);
-        mon_chr = s->mon_chr;
-        cleanup_processes(s);
-        memset(s, 0, sizeof(GDBState));
-        s->mon_chr = mon_chr;
+        qemu_chr_fe_deinit(&gdbserver_state.chr, true);
+        mon_chr = gdbserver_state.mon_chr;
+        reset_gdbserver_state();
     }
 
-    create_processes(s);
+    create_processes(&gdbserver_state);
 
     if (chr) {
-        qemu_chr_fe_init(&s->chr, chr, &error_abort);
-        qemu_chr_fe_set_handlers(&s->chr, gdb_chr_can_receive, gdb_chr_receive,
-                                 gdb_chr_event, NULL, s, NULL, true);
+        qemu_chr_fe_init(&gdbserver_state.chr, chr, &error_abort);
+        qemu_chr_fe_set_handlers(&gdbserver_state.chr, gdb_chr_can_receive,
+                                 gdb_chr_receive, gdb_chr_event,
+                                 NULL, &gdbserver_state, NULL, true);
     }
-    s->state = chr ? RS_IDLE : RS_INACTIVE;
-    s->mon_chr = mon_chr;
-    s->current_syscall_cb = NULL;
+    gdbserver_state.state = chr ? RS_IDLE : RS_INACTIVE;
+    gdbserver_state.mon_chr = mon_chr;
+    gdbserver_state.current_syscall_cb = NULL;
 
     return 0;
 }
 
 void gdbserver_cleanup(void)
 {
-    if (gdbserver_state) {
-        put_packet(gdbserver_state, "W00");
+    if (gdbserver_state.init) {
+        put_packet(&gdbserver_state, "W00");
     }
 }
 
-- 
2.20.1


[PULL 07/28] gdbstub: stop passing GDBState * around and use global
Posted by Alex Bennée 4 years, 1 month ago
We only have one GDBState which should be allocated at the time we
process any commands. This will make further clean-up a bit easier.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Damien Hedde <damien.hedde@greensocs.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Message-Id: <20200316172155.971-8-alex.bennee@linaro.org>

diff --git a/gdbstub.c b/gdbstub.c
index 57d6e50ddfc..7243a2f7af9 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -397,21 +397,21 @@ bool gdb_has_xml;
 /* XXX: This is not thread safe.  Do we care?  */
 static int gdbserver_fd = -1;
 
-static int get_char(GDBState *s)
+static int get_char(void)
 {
     uint8_t ch;
     int ret;
 
     for(;;) {
-        ret = qemu_recv(s->fd, &ch, 1, 0);
+        ret = qemu_recv(gdbserver_state.fd, &ch, 1, 0);
         if (ret < 0) {
             if (errno == ECONNRESET)
-                s->fd = -1;
+                gdbserver_state.fd = -1;
             if (errno != EINTR)
                 return -1;
         } else if (ret == 0) {
-            close(s->fd);
-            s->fd = -1;
+            close(gdbserver_state.fd);
+            gdbserver_state.fd = -1;
             return -1;
         } else {
             break;
@@ -449,11 +449,11 @@ int use_gdb_syscalls(void)
 }
 
 /* Resume execution.  */
-static inline void gdb_continue(GDBState *s)
+static inline void gdb_continue(void)
 {
 
 #ifdef CONFIG_USER_ONLY
-    s->running_state = 1;
+    gdbserver_state.running_state = 1;
     trace_gdbstub_op_continue();
 #else
     if (!runstate_needs_reset()) {
@@ -467,7 +467,7 @@ static inline void gdb_continue(GDBState *s)
  * Resume execution, per CPU actions. For user-mode emulation it's
  * equivalent to gdb_continue.
  */
-static int gdb_continue_partial(GDBState *s, char *newstates)
+static int gdb_continue_partial(char *newstates)
 {
     CPUState *cpu;
     int res = 0;
@@ -482,7 +482,7 @@ static int gdb_continue_partial(GDBState *s, char *newstates)
             cpu_single_step(cpu, sstep_flags);
         }
     }
-    s->running_state = 1;
+    gdbserver_state.running_state = 1;
 #else
     int flag = 0;
 
@@ -520,13 +520,13 @@ static int gdb_continue_partial(GDBState *s, char *newstates)
     return res;
 }
 
-static void put_buffer(GDBState *s, const uint8_t *buf, int len)
+static void put_buffer(const uint8_t *buf, int len)
 {
 #ifdef CONFIG_USER_ONLY
     int ret;
 
     while (len > 0) {
-        ret = send(s->fd, buf, len, 0);
+        ret = send(gdbserver_state.fd, buf, len, 0);
         if (ret < 0) {
             if (errno != EINTR)
                 return;
@@ -538,7 +538,7 @@ static void put_buffer(GDBState *s, const uint8_t *buf, int len)
 #else
     /* XXX this blocks entire thread. Rewrite to use
      * qemu_chr_fe_write and background I/O callbacks */
-    qemu_chr_fe_write_all(&s->chr, buf, len);
+    qemu_chr_fe_write_all(&gdbserver_state.chr, buf, len);
 #endif
 }
 
@@ -620,17 +620,18 @@ static void hexdump(const char *buf, int len,
 }
 
 /* return -1 if error, 0 if OK */
-static int put_packet_binary(GDBState *s, const char *buf, int len, bool dump)
+static int put_packet_binary(const char *buf, int len, bool dump)
 {
     int csum, i;
     uint8_t *p;
+    uint8_t *ps = &gdbserver_state.last_packet[0];
 
     if (dump && trace_event_get_state_backends(TRACE_GDBSTUB_IO_BINARYREPLY)) {
         hexdump(buf, len, trace_gdbstub_io_binaryreply);
     }
 
     for(;;) {
-        p = s->last_packet;
+        p = ps;
         *(p++) = '$';
         memcpy(p, buf, len);
         p += len;
@@ -642,11 +643,11 @@ static int put_packet_binary(GDBState *s, const char *buf, int len, bool dump)
         *(p++) = tohex((csum >> 4) & 0xf);
         *(p++) = tohex((csum) & 0xf);
 
-        s->last_packet_len = p - s->last_packet;
-        put_buffer(s, (uint8_t *)s->last_packet, s->last_packet_len);
+        gdbserver_state.last_packet_len = p - ps;
+        put_buffer(ps, gdbserver_state.last_packet_len);
 
 #ifdef CONFIG_USER_ONLY
-        i = get_char(s);
+        i = get_char();
         if (i < 0)
             return -1;
         if (i == '+')
@@ -659,11 +660,11 @@ static int put_packet_binary(GDBState *s, const char *buf, int len, bool dump)
 }
 
 /* return -1 if error, 0 if OK */
-static int put_packet(GDBState *s, const char *buf)
+static int put_packet(const char *buf)
 {
     trace_gdbstub_io_reply(buf);
 
-    return put_packet_binary(s, buf, strlen(buf), false);
+    return put_packet_binary(buf, strlen(buf), false);
 }
 
 /* Encode data using the encoding for 'x' packets.  */
@@ -687,37 +688,38 @@ static int memtox(char *buf, const char *mem, int len)
     return p - buf;
 }
 
-static uint32_t gdb_get_cpu_pid(const GDBState *s, CPUState *cpu)
+static uint32_t gdb_get_cpu_pid(CPUState *cpu)
 {
     /* TODO: In user mode, we should use the task state PID */
     if (cpu->cluster_index == UNASSIGNED_CLUSTER_INDEX) {
         /* Return the default process' PID */
-        return s->processes[s->process_num - 1].pid;
+        int index = gdbserver_state.process_num - 1;
+        return gdbserver_state.processes[index].pid;
     }
     return cpu->cluster_index + 1;
 }
 
-static GDBProcess *gdb_get_process(const GDBState *s, uint32_t pid)
+static GDBProcess *gdb_get_process(uint32_t pid)
 {
     int i;
 
     if (!pid) {
         /* 0 means any process, we take the first one */
-        return &s->processes[0];
+        return &gdbserver_state.processes[0];
     }
 
-    for (i = 0; i < s->process_num; i++) {
-        if (s->processes[i].pid == pid) {
-            return &s->processes[i];
+    for (i = 0; i < gdbserver_state.process_num; i++) {
+        if (gdbserver_state.processes[i].pid == pid) {
+            return &gdbserver_state.processes[i];
         }
     }
 
     return NULL;
 }
 
-static GDBProcess *gdb_get_cpu_process(const GDBState *s, CPUState *cpu)
+static GDBProcess *gdb_get_cpu_process(CPUState *cpu)
 {
-    return gdb_get_process(s, gdb_get_cpu_pid(s, cpu));
+    return gdb_get_process(gdb_get_cpu_pid(cpu));
 }
 
 static CPUState *find_cpu(uint32_t thread_id)
@@ -733,13 +735,12 @@ static CPUState *find_cpu(uint32_t thread_id)
     return NULL;
 }
 
-static CPUState *get_first_cpu_in_process(const GDBState *s,
-                                          GDBProcess *process)
+static CPUState *get_first_cpu_in_process(GDBProcess *process)
 {
     CPUState *cpu;
 
     CPU_FOREACH(cpu) {
-        if (gdb_get_cpu_pid(s, cpu) == process->pid) {
+        if (gdb_get_cpu_pid(cpu) == process->pid) {
             return cpu;
         }
     }
@@ -747,13 +748,13 @@ static CPUState *get_first_cpu_in_process(const GDBState *s,
     return NULL;
 }
 
-static CPUState *gdb_next_cpu_in_process(const GDBState *s, CPUState *cpu)
+static CPUState *gdb_next_cpu_in_process(CPUState *cpu)
 {
-    uint32_t pid = gdb_get_cpu_pid(s, cpu);
+    uint32_t pid = gdb_get_cpu_pid(cpu);
     cpu = CPU_NEXT(cpu);
 
     while (cpu) {
-        if (gdb_get_cpu_pid(s, cpu) == pid) {
+        if (gdb_get_cpu_pid(cpu) == pid) {
             break;
         }
 
@@ -764,12 +765,12 @@ static CPUState *gdb_next_cpu_in_process(const GDBState *s, CPUState *cpu)
 }
 
 /* Return the cpu following @cpu, while ignoring unattached processes. */
-static CPUState *gdb_next_attached_cpu(const GDBState *s, CPUState *cpu)
+static CPUState *gdb_next_attached_cpu(CPUState *cpu)
 {
     cpu = CPU_NEXT(cpu);
 
     while (cpu) {
-        if (gdb_get_cpu_process(s, cpu)->attached) {
+        if (gdb_get_cpu_process(cpu)->attached) {
             break;
         }
 
@@ -780,29 +781,29 @@ static CPUState *gdb_next_attached_cpu(const GDBState *s, CPUState *cpu)
 }
 
 /* Return the first attached cpu */
-static CPUState *gdb_first_attached_cpu(const GDBState *s)
+static CPUState *gdb_first_attached_cpu(void)
 {
     CPUState *cpu = first_cpu;
-    GDBProcess *process = gdb_get_cpu_process(s, cpu);
+    GDBProcess *process = gdb_get_cpu_process(cpu);
 
     if (!process->attached) {
-        return gdb_next_attached_cpu(s, cpu);
+        return gdb_next_attached_cpu(cpu);
     }
 
     return cpu;
 }
 
-static CPUState *gdb_get_cpu(const GDBState *s, uint32_t pid, uint32_t tid)
+static CPUState *gdb_get_cpu(uint32_t pid, uint32_t tid)
 {
     GDBProcess *process;
     CPUState *cpu;
 
     if (!pid && !tid) {
         /* 0 means any process/thread, we take the first attached one */
-        return gdb_first_attached_cpu(s);
+        return gdb_first_attached_cpu();
     } else if (pid && !tid) {
         /* any thread in a specific process */
-        process = gdb_get_process(s, pid);
+        process = gdb_get_process(pid);
 
         if (process == NULL) {
             return NULL;
@@ -812,7 +813,7 @@ static CPUState *gdb_get_cpu(const GDBState *s, uint32_t pid, uint32_t tid)
             return NULL;
         }
 
-        return get_first_cpu_in_process(s, process);
+        return get_first_cpu_in_process(process);
     } else {
         /* a specific thread */
         cpu = find_cpu(tid);
@@ -821,7 +822,7 @@ static CPUState *gdb_get_cpu(const GDBState *s, uint32_t pid, uint32_t tid)
             return NULL;
         }
 
-        process = gdb_get_cpu_process(s, cpu);
+        process = gdb_get_cpu_process(cpu);
 
         if (pid && process->pid != pid) {
             return NULL;
@@ -835,13 +836,13 @@ static CPUState *gdb_get_cpu(const GDBState *s, uint32_t pid, uint32_t tid)
     }
 }
 
-static const char *get_feature_xml(const GDBState *s, const char *p,
-                                   const char **newp, GDBProcess *process)
+static const char *get_feature_xml(const char *p, const char **newp,
+                                   GDBProcess *process)
 {
     size_t len;
     int i;
     const char *name;
-    CPUState *cpu = get_first_cpu_in_process(s, process);
+    CPUState *cpu = get_first_cpu_in_process(process);
     CPUClass *cc = CPU_GET_CLASS(cpu);
 
     len = 0;
@@ -1076,13 +1077,13 @@ static inline void gdb_cpu_breakpoint_remove_all(CPUState *cpu)
 #endif
 }
 
-static void gdb_process_breakpoint_remove_all(const GDBState *s, GDBProcess *p)
+static void gdb_process_breakpoint_remove_all(GDBProcess *p)
 {
-    CPUState *cpu = get_first_cpu_in_process(s, p);
+    CPUState *cpu = get_first_cpu_in_process(p);
 
     while (cpu) {
         gdb_cpu_breakpoint_remove_all(cpu);
-        cpu = gdb_next_cpu_in_process(s, cpu);
+        cpu = gdb_next_cpu_in_process(cpu);
     }
 }
 
@@ -1100,20 +1101,19 @@ static void gdb_breakpoint_remove_all(void)
     }
 }
 
-static void gdb_set_cpu_pc(GDBState *s, target_ulong pc)
+static void gdb_set_cpu_pc(target_ulong pc)
 {
-    CPUState *cpu = s->c_cpu;
+    CPUState *cpu = gdbserver_state.c_cpu;
 
     cpu_synchronize_state(cpu);
     cpu_set_pc(cpu, pc);
 }
 
-static char *gdb_fmt_thread_id(const GDBState *s, CPUState *cpu,
-                           char *buf, size_t buf_size)
+static char *gdb_fmt_thread_id(CPUState *cpu, char *buf, size_t buf_size)
 {
-    if (s->multiprocess) {
+    if (gdbserver_state.multiprocess) {
         snprintf(buf, buf_size, "p%02x.%02x",
-                 gdb_get_cpu_pid(s, cpu), cpu_gdb_index(cpu));
+                 gdb_get_cpu_pid(cpu), cpu_gdb_index(cpu));
     } else {
         snprintf(buf, buf_size, "%02x", cpu_gdb_index(cpu));
     }
@@ -1180,7 +1180,7 @@ static GDBThreadIdKind read_thread_id(const char *buf, const char **end_buf,
  * returns -ENOTSUP if a command is unsupported, -EINVAL or -ERANGE if there is
  *         a format error, 0 on success.
  */
-static int gdb_handle_vcont(GDBState *s, const char *p)
+static int gdb_handle_vcont(const char *p)
 {
     int res, signal = 0;
     char cur_action;
@@ -1255,36 +1255,36 @@ static int gdb_handle_vcont(GDBState *s, const char *p)
             goto out;
 
         case GDB_ALL_PROCESSES:
-            cpu = gdb_first_attached_cpu(s);
+            cpu = gdb_first_attached_cpu();
             while (cpu) {
                 if (newstates[cpu->cpu_index] == 1) {
                     newstates[cpu->cpu_index] = cur_action;
                 }
 
-                cpu = gdb_next_attached_cpu(s, cpu);
+                cpu = gdb_next_attached_cpu(cpu);
             }
             break;
 
         case GDB_ALL_THREADS:
-            process = gdb_get_process(s, pid);
+            process = gdb_get_process(pid);
 
             if (!process->attached) {
                 res = -EINVAL;
                 goto out;
             }
 
-            cpu = get_first_cpu_in_process(s, process);
+            cpu = get_first_cpu_in_process(process);
             while (cpu) {
                 if (newstates[cpu->cpu_index] == 1) {
                     newstates[cpu->cpu_index] = cur_action;
                 }
 
-                cpu = gdb_next_cpu_in_process(s, cpu);
+                cpu = gdb_next_cpu_in_process(cpu);
             }
             break;
 
         case GDB_ONE_THREAD:
-            cpu = gdb_get_cpu(s, pid, tid);
+            cpu = gdb_get_cpu(pid, tid);
 
             /* invalid CPU/thread specified */
             if (!cpu) {
@@ -1299,8 +1299,8 @@ static int gdb_handle_vcont(GDBState *s, const char *p)
             break;
         }
     }
-    s->signal = signal;
-    gdb_continue_partial(s, newstates);
+    gdbserver_state.signal = signal;
+    gdb_continue_partial(newstates);
 
 out:
     g_free(newstates);
@@ -1409,7 +1409,6 @@ static int cmd_parse_params(const char *data, const char *schema,
 }
 
 typedef struct GdbCmdContext {
-    GDBState *s;
     GdbCmdVariant *params;
     int num_params;
     uint8_t mem_buf[MAX_PACKET_LENGTH];
@@ -1453,7 +1452,7 @@ static inline int startswith(const char *string, const char *pattern)
   return !strncmp(string, pattern, strlen(pattern));
 }
 
-static int process_string_cmd(GDBState *s, void *user_ctx, const char *data,
+static int process_string_cmd(void *user_ctx, const char *data,
                               const GdbCmdParseEntry *cmds, int num_cmds)
 {
     int i, schema_len, max_num_params = 0;
@@ -1490,7 +1489,6 @@ static int process_string_cmd(GDBState *s, void *user_ctx, const char *data,
             return -1;
         }
 
-        gdb_ctx.s = s;
         cmd->handler(&gdb_ctx, user_ctx);
         return 0;
     }
@@ -1498,8 +1496,7 @@ static int process_string_cmd(GDBState *s, void *user_ctx, const char *data,
     return -1;
 }
 
-static void run_cmd_parser(GDBState *s, const char *data,
-                           const GdbCmdParseEntry *cmd)
+static void run_cmd_parser(const char *data, const GdbCmdParseEntry *cmd)
 {
     if (!data) {
         return;
@@ -1507,44 +1504,43 @@ static void run_cmd_parser(GDBState *s, const char *data,
 
     /* In case there was an error during the command parsing we must
     * send a NULL packet to indicate the command is not supported */
-    if (process_string_cmd(s, NULL, data, cmd, 1)) {
-        put_packet(s, "");
+    if (process_string_cmd(NULL, data, cmd, 1)) {
+        put_packet("");
     }
 }
 
 static void handle_detach(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
     GDBProcess *process;
-    GDBState *s = gdb_ctx->s;
     uint32_t pid = 1;
 
-    if (s->multiprocess) {
+    if (gdbserver_state.multiprocess) {
         if (!gdb_ctx->num_params) {
-            put_packet(s, "E22");
+            put_packet("E22");
             return;
         }
 
         pid = gdb_ctx->params[0].val_ul;
     }
 
-    process = gdb_get_process(s, pid);
-    gdb_process_breakpoint_remove_all(s, process);
+    process = gdb_get_process(pid);
+    gdb_process_breakpoint_remove_all(process);
     process->attached = false;
 
-    if (pid == gdb_get_cpu_pid(s, s->c_cpu)) {
-        s->c_cpu = gdb_first_attached_cpu(s);
+    if (pid == gdb_get_cpu_pid(gdbserver_state.c_cpu)) {
+        gdbserver_state.c_cpu = gdb_first_attached_cpu();
     }
 
-    if (pid == gdb_get_cpu_pid(s, s->g_cpu)) {
-        s->g_cpu = gdb_first_attached_cpu(s);
+    if (pid == gdb_get_cpu_pid(gdbserver_state.g_cpu)) {
+        gdbserver_state.g_cpu = gdb_first_attached_cpu();
     }
 
-    if (!s->c_cpu) {
+    if (!gdbserver_state.c_cpu) {
         /* No more process attached */
         gdb_syscall_mode = GDB_SYS_DISABLED;
-        gdb_continue(s);
+        gdb_continue();
     }
-    put_packet(s, "OK");
+    put_packet("OK");
 }
 
 static void handle_thread_alive(GdbCmdContext *gdb_ctx, void *user_ctx)
@@ -1552,33 +1548,33 @@ static void handle_thread_alive(GdbCmdContext *gdb_ctx, void *user_ctx)
     CPUState *cpu;
 
     if (!gdb_ctx->num_params) {
-        put_packet(gdb_ctx->s, "E22");
+        put_packet("E22");
         return;
     }
 
     if (gdb_ctx->params[0].thread_id.kind == GDB_READ_THREAD_ERR) {
-        put_packet(gdb_ctx->s, "E22");
+        put_packet("E22");
         return;
     }
 
-    cpu = gdb_get_cpu(gdb_ctx->s, gdb_ctx->params[0].thread_id.pid,
+    cpu = gdb_get_cpu(gdb_ctx->params[0].thread_id.pid,
                       gdb_ctx->params[0].thread_id.tid);
     if (!cpu) {
-        put_packet(gdb_ctx->s, "E22");
+        put_packet("E22");
         return;
     }
 
-    put_packet(gdb_ctx->s, "OK");
+    put_packet("OK");
 }
 
 static void handle_continue(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
     if (gdb_ctx->num_params) {
-        gdb_set_cpu_pc(gdb_ctx->s, gdb_ctx->params[0].val_ull);
+        gdb_set_cpu_pc(gdb_ctx->params[0].val_ull);
     }
 
-    gdb_ctx->s->signal = 0;
-    gdb_continue(gdb_ctx->s);
+    gdbserver_state.signal = 0;
+    gdb_continue();
 }
 
 static void handle_cont_with_sig(GdbCmdContext *gdb_ctx, void *user_ctx)
@@ -1593,11 +1589,11 @@ static void handle_cont_with_sig(GdbCmdContext *gdb_ctx, void *user_ctx)
         signal = gdb_ctx->params[0].val_ul;
     }
 
-    gdb_ctx->s->signal = gdb_signal_to_target(signal);
-    if (gdb_ctx->s->signal == -1) {
-        gdb_ctx->s->signal = 0;
+    gdbserver_state.signal = gdb_signal_to_target(signal);
+    if (gdbserver_state.signal == -1) {
+        gdbserver_state.signal = 0;
     }
-    gdb_continue(gdb_ctx->s);
+    gdb_continue();
 }
 
 static void handle_set_thread(GdbCmdContext *gdb_ctx, void *user_ctx)
@@ -1605,24 +1601,24 @@ static void handle_set_thread(GdbCmdContext *gdb_ctx, void *user_ctx)
     CPUState *cpu;
 
     if (gdb_ctx->num_params != 2) {
-        put_packet(gdb_ctx->s, "E22");
+        put_packet("E22");
         return;
     }
 
     if (gdb_ctx->params[1].thread_id.kind == GDB_READ_THREAD_ERR) {
-        put_packet(gdb_ctx->s, "E22");
+        put_packet("E22");
         return;
     }
 
     if (gdb_ctx->params[1].thread_id.kind != GDB_ONE_THREAD) {
-        put_packet(gdb_ctx->s, "OK");
+        put_packet("OK");
         return;
     }
 
-    cpu = gdb_get_cpu(gdb_ctx->s, gdb_ctx->params[1].thread_id.pid,
+    cpu = gdb_get_cpu(gdb_ctx->params[1].thread_id.pid,
                       gdb_ctx->params[1].thread_id.tid);
     if (!cpu) {
-        put_packet(gdb_ctx->s, "E22");
+        put_packet("E22");
         return;
     }
 
@@ -1632,15 +1628,15 @@ static void handle_set_thread(GdbCmdContext *gdb_ctx, void *user_ctx)
      */
     switch (gdb_ctx->params[0].opcode) {
     case 'c':
-        gdb_ctx->s->c_cpu = cpu;
-        put_packet(gdb_ctx->s, "OK");
+        gdbserver_state.c_cpu = cpu;
+        put_packet("OK");
         break;
     case 'g':
-        gdb_ctx->s->g_cpu = cpu;
-        put_packet(gdb_ctx->s, "OK");
+        gdbserver_state.g_cpu = cpu;
+        put_packet("OK");
         break;
     default:
-        put_packet(gdb_ctx->s, "E22");
+        put_packet("E22");
         break;
     }
 }
@@ -1650,7 +1646,7 @@ static void handle_insert_bp(GdbCmdContext *gdb_ctx, void *user_ctx)
     int res;
 
     if (gdb_ctx->num_params != 3) {
-        put_packet(gdb_ctx->s, "E22");
+        put_packet("E22");
         return;
     }
 
@@ -1658,14 +1654,14 @@ static void handle_insert_bp(GdbCmdContext *gdb_ctx, void *user_ctx)
                                 gdb_ctx->params[1].val_ull,
                                 gdb_ctx->params[2].val_ull);
     if (res >= 0) {
-        put_packet(gdb_ctx->s, "OK");
+        put_packet("OK");
         return;
     } else if (res == -ENOSYS) {
-        put_packet(gdb_ctx->s, "");
+        put_packet("");
         return;
     }
 
-    put_packet(gdb_ctx->s, "E22");
+    put_packet("E22");
 }
 
 static void handle_remove_bp(GdbCmdContext *gdb_ctx, void *user_ctx)
@@ -1673,7 +1669,7 @@ static void handle_remove_bp(GdbCmdContext *gdb_ctx, void *user_ctx)
     int res;
 
     if (gdb_ctx->num_params != 3) {
-        put_packet(gdb_ctx->s, "E22");
+        put_packet("E22");
         return;
     }
 
@@ -1681,14 +1677,14 @@ static void handle_remove_bp(GdbCmdContext *gdb_ctx, void *user_ctx)
                                 gdb_ctx->params[1].val_ull,
                                 gdb_ctx->params[2].val_ull);
     if (res >= 0) {
-        put_packet(gdb_ctx->s, "OK");
+        put_packet("OK");
         return;
     } else if (res == -ENOSYS) {
-        put_packet(gdb_ctx->s, "");
+        put_packet("");
         return;
     }
 
-    put_packet(gdb_ctx->s, "E22");
+    put_packet("E22");
 }
 
 /*
@@ -1707,20 +1703,20 @@ static void handle_set_reg(GdbCmdContext *gdb_ctx, void *user_ctx)
     int reg_size;
 
     if (!gdb_has_xml) {
-        put_packet(gdb_ctx->s, "");
+        put_packet("");
         return;
     }
 
     if (gdb_ctx->num_params != 2) {
-        put_packet(gdb_ctx->s, "E22");
+        put_packet("E22");
         return;
     }
 
     reg_size = strlen(gdb_ctx->params[1].data) / 2;
     hextomem(gdb_ctx->mem_buf, gdb_ctx->params[1].data, reg_size);
-    gdb_write_register(gdb_ctx->s->g_cpu, gdb_ctx->mem_buf,
+    gdb_write_register(gdbserver_state.g_cpu, gdb_ctx->mem_buf,
                        gdb_ctx->params[0].val_ull);
-    put_packet(gdb_ctx->s, "OK");
+    put_packet("OK");
 }
 
 static void handle_get_reg(GdbCmdContext *gdb_ctx, void *user_ctx)
@@ -1728,73 +1724,73 @@ static void handle_get_reg(GdbCmdContext *gdb_ctx, void *user_ctx)
     int reg_size;
 
     if (!gdb_has_xml) {
-        put_packet(gdb_ctx->s, "");
+        put_packet("");
         return;
     }
 
     if (!gdb_ctx->num_params) {
-        put_packet(gdb_ctx->s, "E14");
+        put_packet("E14");
         return;
     }
 
-    reg_size = gdb_read_register(gdb_ctx->s->g_cpu, gdb_ctx->mem_buf,
+    reg_size = gdb_read_register(gdbserver_state.g_cpu, gdb_ctx->mem_buf,
                                  gdb_ctx->params[0].val_ull);
     if (!reg_size) {
-        put_packet(gdb_ctx->s, "E14");
+        put_packet("E14");
         return;
     }
 
     memtohex(gdb_ctx->str_buf, gdb_ctx->mem_buf, reg_size);
-    put_packet(gdb_ctx->s, gdb_ctx->str_buf);
+    put_packet(gdb_ctx->str_buf);
 }
 
 static void handle_write_mem(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
     if (gdb_ctx->num_params != 3) {
-        put_packet(gdb_ctx->s, "E22");
+        put_packet("E22");
         return;
     }
 
     /* hextomem() reads 2*len bytes */
     if (gdb_ctx->params[1].val_ull > strlen(gdb_ctx->params[2].data) / 2) {
-        put_packet(gdb_ctx->s, "E22");
+        put_packet("E22");
         return;
     }
 
     hextomem(gdb_ctx->mem_buf, gdb_ctx->params[2].data,
              gdb_ctx->params[1].val_ull);
-    if (target_memory_rw_debug(gdb_ctx->s->g_cpu, gdb_ctx->params[0].val_ull,
+    if (target_memory_rw_debug(gdbserver_state.g_cpu, gdb_ctx->params[0].val_ull,
                                gdb_ctx->mem_buf,
                                gdb_ctx->params[1].val_ull, true)) {
-        put_packet(gdb_ctx->s, "E14");
+        put_packet("E14");
         return;
     }
 
-    put_packet(gdb_ctx->s, "OK");
+    put_packet("OK");
 }
 
 static void handle_read_mem(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
     if (gdb_ctx->num_params != 2) {
-        put_packet(gdb_ctx->s, "E22");
+        put_packet("E22");
         return;
     }
 
     /* memtohex() doubles the required space */
     if (gdb_ctx->params[1].val_ull > MAX_PACKET_LENGTH / 2) {
-        put_packet(gdb_ctx->s, "E22");
+        put_packet("E22");
         return;
     }
 
-    if (target_memory_rw_debug(gdb_ctx->s->g_cpu, gdb_ctx->params[0].val_ull,
+    if (target_memory_rw_debug(gdbserver_state.g_cpu, gdb_ctx->params[0].val_ull,
                                gdb_ctx->mem_buf,
                                gdb_ctx->params[1].val_ull, false)) {
-        put_packet(gdb_ctx->s, "E14");
+        put_packet("E14");
         return;
     }
 
     memtohex(gdb_ctx->str_buf, gdb_ctx->mem_buf, gdb_ctx->params[1].val_ull);
-    put_packet(gdb_ctx->s, gdb_ctx->str_buf);
+    put_packet(gdb_ctx->str_buf);
 }
 
 static void handle_write_all_regs(GdbCmdContext *gdb_ctx, void *user_ctx)
@@ -1807,37 +1803,37 @@ static void handle_write_all_regs(GdbCmdContext *gdb_ctx, void *user_ctx)
         return;
     }
 
-    cpu_synchronize_state(gdb_ctx->s->g_cpu);
+    cpu_synchronize_state(gdbserver_state.g_cpu);
     registers = gdb_ctx->mem_buf;
     len = strlen(gdb_ctx->params[0].data) / 2;
     hextomem(registers, gdb_ctx->params[0].data, len);
-    for (addr = 0; addr < gdb_ctx->s->g_cpu->gdb_num_g_regs && len > 0;
+    for (addr = 0; addr < gdbserver_state.g_cpu->gdb_num_g_regs && len > 0;
          addr++) {
-        reg_size = gdb_write_register(gdb_ctx->s->g_cpu, registers, addr);
+        reg_size = gdb_write_register(gdbserver_state.g_cpu, registers, addr);
         len -= reg_size;
         registers += reg_size;
     }
-    put_packet(gdb_ctx->s, "OK");
+    put_packet("OK");
 }
 
 static void handle_read_all_regs(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
     target_ulong addr, len;
 
-    cpu_synchronize_state(gdb_ctx->s->g_cpu);
+    cpu_synchronize_state(gdbserver_state.g_cpu);
     len = 0;
-    for (addr = 0; addr < gdb_ctx->s->g_cpu->gdb_num_g_regs; addr++) {
-        len += gdb_read_register(gdb_ctx->s->g_cpu, gdb_ctx->mem_buf + len,
+    for (addr = 0; addr < gdbserver_state.g_cpu->gdb_num_g_regs; addr++) {
+        len += gdb_read_register(gdbserver_state.g_cpu, gdb_ctx->mem_buf + len,
                                  addr);
     }
 
     memtohex(gdb_ctx->str_buf, gdb_ctx->mem_buf, len);
-    put_packet(gdb_ctx->s, gdb_ctx->str_buf);
+    put_packet(gdb_ctx->str_buf);
 }
 
 static void handle_file_io(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
-    if (gdb_ctx->num_params >= 1 && gdb_ctx->s->current_syscall_cb) {
+    if (gdb_ctx->num_params >= 1 && gdbserver_state.current_syscall_cb) {
         target_ulong ret, err;
 
         ret = (target_ulong)gdb_ctx->params[0].val_ull;
@@ -1846,31 +1842,31 @@ static void handle_file_io(GdbCmdContext *gdb_ctx, void *user_ctx)
         } else {
             err = 0;
         }
-        gdb_ctx->s->current_syscall_cb(gdb_ctx->s->c_cpu, ret, err);
-        gdb_ctx->s->current_syscall_cb = NULL;
+        gdbserver_state.current_syscall_cb(gdbserver_state.c_cpu, ret, err);
+        gdbserver_state.current_syscall_cb = NULL;
     }
 
     if (gdb_ctx->num_params >= 3 && gdb_ctx->params[2].opcode == (uint8_t)'C') {
-        put_packet(gdb_ctx->s, "T02");
+        put_packet("T02");
         return;
     }
 
-    gdb_continue(gdb_ctx->s);
+    gdb_continue();
 }
 
 static void handle_step(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
     if (gdb_ctx->num_params) {
-        gdb_set_cpu_pc(gdb_ctx->s, (target_ulong)gdb_ctx->params[0].val_ull);
+        gdb_set_cpu_pc((target_ulong)gdb_ctx->params[0].val_ull);
     }
 
-    cpu_single_step(gdb_ctx->s->c_cpu, sstep_flags);
-    gdb_continue(gdb_ctx->s);
+    cpu_single_step(gdbserver_state.c_cpu, sstep_flags);
+    gdb_continue();
 }
 
 static void handle_v_cont_query(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
-    put_packet(gdb_ctx->s, "vCont;c;C;s;S");
+    put_packet("vCont;c;C;s;S");
 }
 
 static void handle_v_cont(GdbCmdContext *gdb_ctx, void *user_ctx)
@@ -1881,11 +1877,11 @@ static void handle_v_cont(GdbCmdContext *gdb_ctx, void *user_ctx)
         return;
     }
 
-    res = gdb_handle_vcont(gdb_ctx->s, gdb_ctx->params[0].data);
+    res = gdb_handle_vcont(gdb_ctx->params[0].data);
     if ((res == -EINVAL) || (res == -ERANGE)) {
-        put_packet(gdb_ctx->s, "E22");
+        put_packet("E22");
     } else if (res) {
-        put_packet(gdb_ctx->s, "");
+        put_packet("");
     }
 }
 
@@ -1900,31 +1896,31 @@ static void handle_v_attach(GdbCmdContext *gdb_ctx, void *user_ctx)
         goto cleanup;
     }
 
-    process = gdb_get_process(gdb_ctx->s, gdb_ctx->params[0].val_ul);
+    process = gdb_get_process(gdb_ctx->params[0].val_ul);
     if (!process) {
         goto cleanup;
     }
 
-    cpu = get_first_cpu_in_process(gdb_ctx->s, process);
+    cpu = get_first_cpu_in_process(process);
     if (!cpu) {
         goto cleanup;
     }
 
     process->attached = true;
-    gdb_ctx->s->g_cpu = cpu;
-    gdb_ctx->s->c_cpu = cpu;
+    gdbserver_state.g_cpu = cpu;
+    gdbserver_state.c_cpu = cpu;
 
-    gdb_fmt_thread_id(gdb_ctx->s, cpu, thread_id, sizeof(thread_id));
+    gdb_fmt_thread_id(cpu, thread_id, sizeof(thread_id));
     snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "T%02xthread:%s;",
              GDB_SIGNAL_TRAP, thread_id);
 cleanup:
-    put_packet(gdb_ctx->s, gdb_ctx->str_buf);
+    put_packet(gdb_ctx->str_buf);
 }
 
 static void handle_v_kill(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
     /* Kill the target */
-    put_packet(gdb_ctx->s, "OK");
+    put_packet("OK");
     error_report("QEMU: Terminated via GDBstub");
     exit(0);
 }
@@ -1961,10 +1957,10 @@ static void handle_v_commands(GdbCmdContext *gdb_ctx, void *user_ctx)
         return;
     }
 
-    if (process_string_cmd(gdb_ctx->s, NULL, gdb_ctx->params[0].data,
+    if (process_string_cmd(NULL, gdb_ctx->params[0].data,
                            gdb_v_commands_table,
                            ARRAY_SIZE(gdb_v_commands_table))) {
-        put_packet(gdb_ctx->s, "");
+        put_packet("");
     }
 }
 
@@ -1973,7 +1969,7 @@ static void handle_query_qemu_sstepbits(GdbCmdContext *gdb_ctx, void *user_ctx)
     snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf),
              "ENABLE=%x,NOIRQ=%x,NOTIMER=%x", SSTEP_ENABLE,
              SSTEP_NOIRQ, SSTEP_NOTIMER);
-    put_packet(gdb_ctx->s, gdb_ctx->str_buf);
+    put_packet(gdb_ctx->str_buf);
 }
 
 static void handle_set_qemu_sstep(GdbCmdContext *gdb_ctx, void *user_ctx)
@@ -1983,13 +1979,13 @@ static void handle_set_qemu_sstep(GdbCmdContext *gdb_ctx, void *user_ctx)
     }
 
     sstep_flags = gdb_ctx->params[0].val_ul;
-    put_packet(gdb_ctx->s, "OK");
+    put_packet("OK");
 }
 
 static void handle_query_qemu_sstep(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
     snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "0x%x", sstep_flags);
-    put_packet(gdb_ctx->s, gdb_ctx->str_buf);
+    put_packet(gdb_ctx->str_buf);
 }
 
 static void handle_query_curr_tid(GdbCmdContext *gdb_ctx, void *user_ctx)
@@ -2003,33 +1999,32 @@ static void handle_query_curr_tid(GdbCmdContext *gdb_ctx, void *user_ctx)
      * the first thread of the current process (gdb returns the
      * first thread).
      */
-    process = gdb_get_cpu_process(gdb_ctx->s, gdb_ctx->s->g_cpu);
-    cpu = get_first_cpu_in_process(gdb_ctx->s, process);
-    gdb_fmt_thread_id(gdb_ctx->s, cpu, thread_id, sizeof(thread_id));
+    process = gdb_get_cpu_process(gdbserver_state.g_cpu);
+    cpu = get_first_cpu_in_process(process);
+    gdb_fmt_thread_id(cpu, thread_id, sizeof(thread_id));
     snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "QC%s", thread_id);
-    put_packet(gdb_ctx->s, gdb_ctx->str_buf);
+    put_packet(gdb_ctx->str_buf);
 }
 
 static void handle_query_threads(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
     char thread_id[16];
 
-    if (!gdb_ctx->s->query_cpu) {
-        put_packet(gdb_ctx->s, "l");
+    if (!gdbserver_state.query_cpu) {
+        put_packet("l");
         return;
     }
 
-    gdb_fmt_thread_id(gdb_ctx->s, gdb_ctx->s->query_cpu, thread_id,
+    gdb_fmt_thread_id(gdbserver_state.query_cpu, thread_id,
                       sizeof(thread_id));
     snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "m%s", thread_id);
-    put_packet(gdb_ctx->s, gdb_ctx->str_buf);
-    gdb_ctx->s->query_cpu =
-        gdb_next_attached_cpu(gdb_ctx->s, gdb_ctx->s->query_cpu);
+    put_packet(gdb_ctx->str_buf);
+    gdbserver_state.query_cpu = gdb_next_attached_cpu(gdbserver_state.query_cpu);
 }
 
 static void handle_query_first_threads(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
-    gdb_ctx->s->query_cpu = gdb_first_attached_cpu(gdb_ctx->s);
+    gdbserver_state.query_cpu = gdb_first_attached_cpu();
     handle_query_threads(gdb_ctx, user_ctx);
 }
 
@@ -2040,11 +2035,11 @@ static void handle_query_thread_extra(GdbCmdContext *gdb_ctx, void *user_ctx)
 
     if (!gdb_ctx->num_params ||
         gdb_ctx->params[0].thread_id.kind == GDB_READ_THREAD_ERR) {
-        put_packet(gdb_ctx->s, "E22");
+        put_packet("E22");
         return;
     }
 
-    cpu = gdb_get_cpu(gdb_ctx->s, gdb_ctx->params[0].thread_id.pid,
+    cpu = gdb_get_cpu(gdb_ctx->params[0].thread_id.pid,
                       gdb_ctx->params[0].thread_id.tid);
     if (!cpu) {
         return;
@@ -2052,7 +2047,7 @@ static void handle_query_thread_extra(GdbCmdContext *gdb_ctx, void *user_ctx)
 
     cpu_synchronize_state(cpu);
 
-    if (gdb_ctx->s->multiprocess && (gdb_ctx->s->process_num > 1)) {
+    if (gdbserver_state.multiprocess && (gdbserver_state.process_num > 1)) {
         /* Print the CPU model and name in multiprocess mode */
         ObjectClass *oc = object_get_class(OBJECT(cpu));
         const char *cpu_model = object_class_get_name(oc);
@@ -2069,7 +2064,7 @@ static void handle_query_thread_extra(GdbCmdContext *gdb_ctx, void *user_ctx)
     }
     trace_gdbstub_op_extra_info((char *)gdb_ctx->mem_buf);
     memtohex(gdb_ctx->str_buf, gdb_ctx->mem_buf, len);
-    put_packet(gdb_ctx->s, gdb_ctx->str_buf);
+    put_packet(gdb_ctx->str_buf);
 }
 
 #ifdef CONFIG_USER_ONLY
@@ -2077,14 +2072,14 @@ static void handle_query_offsets(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
     TaskState *ts;
 
-    ts = gdb_ctx->s->c_cpu->opaque;
+    ts = gdbserver_state.c_cpu->opaque;
     snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf),
              "Text=" TARGET_ABI_FMT_lx ";Data=" TARGET_ABI_FMT_lx
              ";Bss=" TARGET_ABI_FMT_lx,
              ts->info->code_offset,
              ts->info->data_offset,
              ts->info->data_offset);
-    put_packet(gdb_ctx->s, gdb_ctx->str_buf);
+    put_packet(gdb_ctx->str_buf);
 }
 #else
 static void handle_query_rcmd(GdbCmdContext *gdb_ctx, void *user_ctx)
@@ -2092,21 +2087,21 @@ static void handle_query_rcmd(GdbCmdContext *gdb_ctx, void *user_ctx)
     int len;
 
     if (!gdb_ctx->num_params) {
-        put_packet(gdb_ctx->s, "E22");
+        put_packet("E22");
         return;
     }
 
     len = strlen(gdb_ctx->params[0].data);
     if (len % 2) {
-        put_packet(gdb_ctx->s, "E01");
+        put_packet("E01");
         return;
     }
 
     len = len / 2;
     hextomem(gdb_ctx->mem_buf, gdb_ctx->params[0].data, len);
     gdb_ctx->mem_buf[len++] = 0;
-    qemu_chr_be_write(gdb_ctx->s->mon_chr, gdb_ctx->mem_buf, len);
-    put_packet(gdb_ctx->s, "OK");
+    qemu_chr_be_write(gdbserver_state.mon_chr, gdb_ctx->mem_buf, len);
+    put_packet("OK");
 
 }
 #endif
@@ -2125,11 +2120,11 @@ static void handle_query_supported(GdbCmdContext *gdb_ctx, void *user_ctx)
 
     if (gdb_ctx->num_params &&
         strstr(gdb_ctx->params[0].data, "multiprocess+")) {
-        gdb_ctx->s->multiprocess = true;
+        gdbserver_state.multiprocess = true;
     }
 
     pstrcat(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), ";multiprocess+");
-    put_packet(gdb_ctx->s, gdb_ctx->str_buf);
+    put_packet(gdb_ctx->str_buf);
 }
 
 static void handle_query_xfer_features(GdbCmdContext *gdb_ctx, void *user_ctx)
@@ -2141,22 +2136,22 @@ static void handle_query_xfer_features(GdbCmdContext *gdb_ctx, void *user_ctx)
     const char *p;
 
     if (gdb_ctx->num_params < 3) {
-        put_packet(gdb_ctx->s, "E22");
+        put_packet("E22");
         return;
     }
 
-    process = gdb_get_cpu_process(gdb_ctx->s, gdb_ctx->s->g_cpu);
-    cc = CPU_GET_CLASS(gdb_ctx->s->g_cpu);
+    process = gdb_get_cpu_process(gdbserver_state.g_cpu);
+    cc = CPU_GET_CLASS(gdbserver_state.g_cpu);
     if (!cc->gdb_core_xml_file) {
-        put_packet(gdb_ctx->s, "");
+        put_packet("");
         return;
     }
 
     gdb_has_xml = true;
     p = gdb_ctx->params[0].data;
-    xml = get_feature_xml(gdb_ctx->s, p, &p, process);
+    xml = get_feature_xml(p, &p, process);
     if (!xml) {
-        put_packet(gdb_ctx->s, "E00");
+        put_packet("E00");
         return;
     }
 
@@ -2164,7 +2159,7 @@ static void handle_query_xfer_features(GdbCmdContext *gdb_ctx, void *user_ctx)
     len = gdb_ctx->params[2].val_ul;
     total_len = strlen(xml);
     if (addr > total_len) {
-        put_packet(gdb_ctx->s, "E00");
+        put_packet("E00");
         return;
     }
 
@@ -2180,12 +2175,12 @@ static void handle_query_xfer_features(GdbCmdContext *gdb_ctx, void *user_ctx)
         len = memtox(gdb_ctx->str_buf + 1, xml + addr, total_len - addr);
     }
 
-    put_packet_binary(gdb_ctx->s, gdb_ctx->str_buf, len + 1, true);
+    put_packet_binary(gdb_ctx->str_buf, len + 1, true);
 }
 
 static void handle_query_attached(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
-    put_packet(gdb_ctx->s, GDB_ATTACHED);
+    put_packet(GDB_ATTACHED);
 }
 
 static void handle_query_qemu_supported(GdbCmdContext *gdb_ctx, void *user_ctx)
@@ -2194,7 +2189,7 @@ static void handle_query_qemu_supported(GdbCmdContext *gdb_ctx, void *user_ctx)
 #ifndef CONFIG_USER_ONLY
     pstrcat(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), ";PhyMemMode");
 #endif
-    put_packet(gdb_ctx->s, gdb_ctx->str_buf);
+    put_packet(gdb_ctx->str_buf);
 }
 
 #ifndef CONFIG_USER_ONLY
@@ -2202,13 +2197,13 @@ static void handle_query_qemu_phy_mem_mode(GdbCmdContext *gdb_ctx,
                                            void *user_ctx)
 {
     snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "%d", phy_memory_mode);
-    put_packet(gdb_ctx->s, gdb_ctx->str_buf);
+    put_packet(gdb_ctx->str_buf);
 }
 
 static void handle_set_qemu_phy_mem_mode(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
     if (!gdb_ctx->num_params) {
-        put_packet(gdb_ctx->s, "E22");
+        put_packet("E22");
         return;
     }
 
@@ -2217,7 +2212,7 @@ static void handle_set_qemu_phy_mem_mode(GdbCmdContext *gdb_ctx, void *user_ctx)
     } else {
         phy_memory_mode = 1;
     }
-    put_packet(gdb_ctx->s, "OK");
+    put_packet("OK");
 }
 #endif
 
@@ -2333,16 +2328,16 @@ static void handle_gen_query(GdbCmdContext *gdb_ctx, void *user_ctx)
         return;
     }
 
-    if (!process_string_cmd(gdb_ctx->s, NULL, gdb_ctx->params[0].data,
+    if (!process_string_cmd(NULL, gdb_ctx->params[0].data,
                             gdb_gen_query_set_common_table,
                             ARRAY_SIZE(gdb_gen_query_set_common_table))) {
         return;
     }
 
-    if (process_string_cmd(gdb_ctx->s, NULL, gdb_ctx->params[0].data,
+    if (process_string_cmd(NULL, gdb_ctx->params[0].data,
                            gdb_gen_query_table,
                            ARRAY_SIZE(gdb_gen_query_table))) {
-        put_packet(gdb_ctx->s, "");
+        put_packet("");
     }
 }
 
@@ -2352,16 +2347,16 @@ static void handle_gen_set(GdbCmdContext *gdb_ctx, void *user_ctx)
         return;
     }
 
-    if (!process_string_cmd(gdb_ctx->s, NULL, gdb_ctx->params[0].data,
+    if (!process_string_cmd(NULL, gdb_ctx->params[0].data,
                             gdb_gen_query_set_common_table,
                             ARRAY_SIZE(gdb_gen_query_set_common_table))) {
         return;
     }
 
-    if (process_string_cmd(gdb_ctx->s, NULL, gdb_ctx->params[0].data,
+    if (process_string_cmd(NULL, gdb_ctx->params[0].data,
                            gdb_gen_set_table,
                            ARRAY_SIZE(gdb_gen_set_table))) {
-        put_packet(gdb_ctx->s, "");
+        put_packet("");
     }
 }
 
@@ -2369,11 +2364,11 @@ static void handle_target_halt(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
     char thread_id[16];
 
-    gdb_fmt_thread_id(gdb_ctx->s, gdb_ctx->s->c_cpu, thread_id,
+    gdb_fmt_thread_id(gdbserver_state.c_cpu, thread_id,
                       sizeof(thread_id));
     snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "T%02xthread:%s;",
              GDB_SIGNAL_TRAP, thread_id);
-    put_packet(gdb_ctx->s, gdb_ctx->str_buf);
+    put_packet(gdb_ctx->str_buf);
     /*
      * Remove all the breakpoints when this query is issued,
      * because gdb is doing an initial connect and the state
@@ -2382,7 +2377,7 @@ static void handle_target_halt(GdbCmdContext *gdb_ctx, void *user_ctx)
     gdb_breakpoint_remove_all();
 }
 
-static int gdb_handle_packet(GDBState *s, const char *line_buf)
+static int gdb_handle_packet(const char *line_buf)
 {
     const GdbCmdParseEntry *cmd_parser = NULL;
 
@@ -2390,7 +2385,7 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
 
     switch (line_buf[0]) {
     case '!':
-        put_packet(s, "OK");
+        put_packet("OK");
         break;
     case '?':
         {
@@ -2605,12 +2600,12 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
         break;
     default:
         /* put empty packet */
-        put_packet(s, "");
+        put_packet("");
         break;
     }
 
     if (cmd_parser) {
-        run_cmd_parser(s, line_buf, cmd_parser);
+        run_cmd_parser(line_buf, cmd_parser);
     }
 
     return RS_IDLE;
@@ -2618,7 +2613,7 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
 
 void gdb_set_stop_cpu(CPUState *cpu)
 {
-    GDBProcess *p = gdb_get_cpu_process(&gdbserver_state, cpu);
+    GDBProcess *p = gdb_get_cpu_process(cpu);
 
     if (!p->attached) {
         /*
@@ -2635,19 +2630,18 @@ void gdb_set_stop_cpu(CPUState *cpu)
 #ifndef CONFIG_USER_ONLY
 static void gdb_vm_state_change(void *opaque, int running, RunState state)
 {
-    GDBState *s = &gdbserver_state;
-    CPUState *cpu = s->c_cpu;
+    CPUState *cpu = gdbserver_state.c_cpu;
     char buf[256];
     char thread_id[16];
     const char *type;
     int ret;
 
-    if (running || s->state == RS_INACTIVE) {
+    if (running || gdbserver_state.state == RS_INACTIVE) {
         return;
     }
     /* Is there a GDB syscall waiting to be sent?  */
-    if (s->current_syscall_cb) {
-        put_packet(s, s->syscall_buf);
+    if (gdbserver_state.current_syscall_cb) {
+        put_packet(gdbserver_state.syscall_buf);
         return;
     }
 
@@ -2656,7 +2650,7 @@ static void gdb_vm_state_change(void *opaque, int running, RunState state)
         return;
     }
 
-    gdb_fmt_thread_id(s, cpu, thread_id, sizeof(thread_id));
+    gdb_fmt_thread_id(cpu, thread_id, sizeof(thread_id));
 
     switch (state) {
     case RUN_STATE_DEBUG:
@@ -2721,7 +2715,7 @@ static void gdb_vm_state_change(void *opaque, int running, RunState state)
     snprintf(buf, sizeof(buf), "T%02xthread:%s;", ret, thread_id);
 
 send_packet:
-    put_packet(s, buf);
+    put_packet(buf);
 
     /* disable single step if it was enabled */
     cpu_single_step(cpu, 0);
@@ -2740,8 +2734,9 @@ void gdb_do_syscallv(gdb_syscall_complete_cb cb, const char *fmt, va_list va)
     target_ulong addr;
     uint64_t i64;
 
-    if (!gdbserver_state.init)
+    if (!gdbserver_state.init) {
         return;
+    }
 
     gdbserver_state.current_syscall_cb = cb;
 #ifndef CONFIG_USER_ONLY
@@ -2781,7 +2776,7 @@ void gdb_do_syscallv(gdb_syscall_complete_cb cb, const char *fmt, va_list va)
     }
     *p = 0;
 #ifdef CONFIG_USER_ONLY
-    put_packet(&gdbserver_state, gdbserver_state.syscall_buf);
+    put_packet(gdbserver_state.syscall_buf);
     /* Return control to gdb for it to process the syscall request.
      * Since the protocol requires that gdb hands control back to us
      * using a "here are the results" F packet, we don't need to check
@@ -2809,17 +2804,17 @@ void gdb_do_syscall(gdb_syscall_complete_cb cb, const char *fmt, ...)
     va_end(va);
 }
 
-static void gdb_read_byte(GDBState *s, uint8_t ch)
+static void gdb_read_byte(uint8_t ch)
 {
     uint8_t reply;
 
 #ifndef CONFIG_USER_ONLY
-    if (s->last_packet_len) {
+    if (gdbserver_state.last_packet_len) {
         /* Waiting for a response to the last packet.  If we see the start
            of a new command then abandon the previous response.  */
         if (ch == '-') {
             trace_gdbstub_err_got_nack();
-            put_buffer(s, (uint8_t *)s->last_packet, s->last_packet_len);
+            put_buffer((uint8_t *)gdbserver_state.last_packet, gdbserver_state.last_packet_len);
         } else if (ch == '+') {
             trace_gdbstub_io_got_ack();
         } else {
@@ -2827,7 +2822,7 @@ static void gdb_read_byte(GDBState *s, uint8_t ch)
         }
 
         if (ch == '+' || ch == '$')
-            s->last_packet_len = 0;
+            gdbserver_state.last_packet_len = 0;
         if (ch != '$')
             return;
     }
@@ -2838,13 +2833,13 @@ static void gdb_read_byte(GDBState *s, uint8_t ch)
     } else
 #endif
     {
-        switch(s->state) {
+        switch(gdbserver_state.state) {
         case RS_IDLE:
             if (ch == '$') {
                 /* start of command packet */
-                s->line_buf_index = 0;
-                s->line_sum = 0;
-                s->state = RS_GETLINE;
+                gdbserver_state.line_buf_index = 0;
+                gdbserver_state.line_sum = 0;
+                gdbserver_state.state = RS_GETLINE;
             } else {
                 trace_gdbstub_err_garbage(ch);
             }
@@ -2852,37 +2847,37 @@ static void gdb_read_byte(GDBState *s, uint8_t ch)
         case RS_GETLINE:
             if (ch == '}') {
                 /* start escape sequence */
-                s->state = RS_GETLINE_ESC;
-                s->line_sum += ch;
+                gdbserver_state.state = RS_GETLINE_ESC;
+                gdbserver_state.line_sum += ch;
             } else if (ch == '*') {
                 /* start run length encoding sequence */
-                s->state = RS_GETLINE_RLE;
-                s->line_sum += ch;
+                gdbserver_state.state = RS_GETLINE_RLE;
+                gdbserver_state.line_sum += ch;
             } else if (ch == '#') {
                 /* end of command, start of checksum*/
-                s->state = RS_CHKSUM1;
-            } else if (s->line_buf_index >= sizeof(s->line_buf) - 1) {
+                gdbserver_state.state = RS_CHKSUM1;
+            } else if (gdbserver_state.line_buf_index >= sizeof(gdbserver_state.line_buf) - 1) {
                 trace_gdbstub_err_overrun();
-                s->state = RS_IDLE;
+                gdbserver_state.state = RS_IDLE;
             } else {
                 /* unescaped command character */
-                s->line_buf[s->line_buf_index++] = ch;
-                s->line_sum += ch;
+                gdbserver_state.line_buf[gdbserver_state.line_buf_index++] = ch;
+                gdbserver_state.line_sum += ch;
             }
             break;
         case RS_GETLINE_ESC:
             if (ch == '#') {
                 /* unexpected end of command in escape sequence */
-                s->state = RS_CHKSUM1;
-            } else if (s->line_buf_index >= sizeof(s->line_buf) - 1) {
+                gdbserver_state.state = RS_CHKSUM1;
+            } else if (gdbserver_state.line_buf_index >= sizeof(gdbserver_state.line_buf) - 1) {
                 /* command buffer overrun */
                 trace_gdbstub_err_overrun();
-                s->state = RS_IDLE;
+                gdbserver_state.state = RS_IDLE;
             } else {
                 /* parse escaped character and leave escape state */
-                s->line_buf[s->line_buf_index++] = ch ^ 0x20;
-                s->line_sum += ch;
-                s->state = RS_GETLINE;
+                gdbserver_state.line_buf[gdbserver_state.line_buf_index++] = ch ^ 0x20;
+                gdbserver_state.line_sum += ch;
+                gdbserver_state.state = RS_GETLINE;
             }
             break;
         case RS_GETLINE_RLE:
@@ -2893,25 +2888,25 @@ static void gdb_read_byte(GDBState *s, uint8_t ch)
             if (ch < ' ' || ch == '#' || ch == '$' || ch > 126) {
                 /* invalid RLE count encoding */
                 trace_gdbstub_err_invalid_repeat(ch);
-                s->state = RS_GETLINE;
+                gdbserver_state.state = RS_GETLINE;
             } else {
                 /* decode repeat length */
                 int repeat = ch - ' ' + 3;
-                if (s->line_buf_index + repeat >= sizeof(s->line_buf) - 1) {
+                if (gdbserver_state.line_buf_index + repeat >= sizeof(gdbserver_state.line_buf) - 1) {
                     /* that many repeats would overrun the command buffer */
                     trace_gdbstub_err_overrun();
-                    s->state = RS_IDLE;
-                } else if (s->line_buf_index < 1) {
+                    gdbserver_state.state = RS_IDLE;
+                } else if (gdbserver_state.line_buf_index < 1) {
                     /* got a repeat but we have nothing to repeat */
                     trace_gdbstub_err_invalid_rle();
-                    s->state = RS_GETLINE;
+                    gdbserver_state.state = RS_GETLINE;
                 } else {
                     /* repeat the last character */
-                    memset(s->line_buf + s->line_buf_index,
-                           s->line_buf[s->line_buf_index - 1], repeat);
-                    s->line_buf_index += repeat;
-                    s->line_sum += ch;
-                    s->state = RS_GETLINE;
+                    memset(gdbserver_state.line_buf + gdbserver_state.line_buf_index,
+                           gdbserver_state.line_buf[gdbserver_state.line_buf_index - 1], repeat);
+                    gdbserver_state.line_buf_index += repeat;
+                    gdbserver_state.line_sum += ch;
+                    gdbserver_state.state = RS_GETLINE;
                 }
             }
             break;
@@ -2919,33 +2914,33 @@ static void gdb_read_byte(GDBState *s, uint8_t ch)
             /* get high hex digit of checksum */
             if (!isxdigit(ch)) {
                 trace_gdbstub_err_checksum_invalid(ch);
-                s->state = RS_GETLINE;
+                gdbserver_state.state = RS_GETLINE;
                 break;
             }
-            s->line_buf[s->line_buf_index] = '\0';
-            s->line_csum = fromhex(ch) << 4;
-            s->state = RS_CHKSUM2;
+            gdbserver_state.line_buf[gdbserver_state.line_buf_index] = '\0';
+            gdbserver_state.line_csum = fromhex(ch) << 4;
+            gdbserver_state.state = RS_CHKSUM2;
             break;
         case RS_CHKSUM2:
             /* get low hex digit of checksum */
             if (!isxdigit(ch)) {
                 trace_gdbstub_err_checksum_invalid(ch);
-                s->state = RS_GETLINE;
+                gdbserver_state.state = RS_GETLINE;
                 break;
             }
-            s->line_csum |= fromhex(ch);
+            gdbserver_state.line_csum |= fromhex(ch);
 
-            if (s->line_csum != (s->line_sum & 0xff)) {
-                trace_gdbstub_err_checksum_incorrect(s->line_sum, s->line_csum);
+            if (gdbserver_state.line_csum != (gdbserver_state.line_sum & 0xff)) {
+                trace_gdbstub_err_checksum_incorrect(gdbserver_state.line_sum, gdbserver_state.line_csum);
                 /* send NAK reply */
                 reply = '-';
-                put_buffer(s, &reply, 1);
-                s->state = RS_IDLE;
+                put_buffer(&reply, 1);
+                gdbserver_state.state = RS_IDLE;
             } else {
                 /* send ACK reply */
                 reply = '+';
-                put_buffer(s, &reply, 1);
-                s->state = gdb_handle_packet(s, s->line_buf);
+                put_buffer(&reply, 1);
+                gdbserver_state.state = gdb_handle_packet(gdbserver_state.line_buf);
             }
             break;
         default:
@@ -2971,7 +2966,7 @@ void gdb_exit(CPUArchState *env, int code)
   trace_gdbstub_op_exiting((uint8_t)code);
 
   snprintf(buf, sizeof(buf), "W%02x", (uint8_t)code);
-  put_packet(&gdbserver_state, buf);
+  put_packet(buf);
 
 #ifndef CONFIG_USER_ONLY
   qemu_chr_fe_deinit(&gdbserver_state.chr, true);
@@ -2988,7 +2983,7 @@ static void create_default_process(GDBState *s)
     GDBProcess *process;
     int max_pid = 0;
 
-    if (s->process_num) {
+    if (gdbserver_state.process_num) {
         max_pid = s->processes[s->process_num - 1].pid;
     }
 
@@ -3020,7 +3015,7 @@ gdb_handlesig(CPUState *cpu, int sig)
 
     if (sig != 0) {
         snprintf(buf, sizeof(buf), "S%02x", target_signal_to_gdb(sig));
-        put_packet(&gdbserver_state, buf);
+        put_packet(buf);
     }
     /* put_packet() might have detected that the peer terminated the
        connection.  */
@@ -3037,7 +3032,7 @@ gdb_handlesig(CPUState *cpu, int sig)
             int i;
 
             for (i = 0; i < n; i++) {
-                gdb_read_byte(&gdbserver_state, buf[i]);
+                gdb_read_byte(buf[i]);
             }
         } else {
             /* XXX: Connection closed.  Should probably wait for another
@@ -3064,7 +3059,7 @@ void gdb_signalled(CPUArchState *env, int sig)
     }
 
     snprintf(buf, sizeof(buf), "X%02x", target_signal_to_gdb(sig));
-    put_packet(&gdbserver_state, buf);
+    put_packet(buf);
 }
 
 static bool gdb_accept(void)
@@ -3095,7 +3090,7 @@ static bool gdb_accept(void)
     init_gdbserver_state();
     create_default_process(&gdbserver_state);
     gdbserver_state.processes[0].attached = true;
-    gdbserver_state.c_cpu = gdb_first_attached_cpu(&gdbserver_state);
+    gdbserver_state.c_cpu = gdb_first_attached_cpu();
     gdbserver_state.g_cpu = gdbserver_state.c_cpu;
     gdbserver_state.fd = fd;
     gdb_has_xml = false;
@@ -3172,7 +3167,7 @@ static void gdb_chr_receive(void *opaque, const uint8_t *buf, int size)
     int i;
 
     for (i = 0; i < size; i++) {
-        gdb_read_byte(&gdbserver_state, buf[i]);
+        gdb_read_byte(buf[i]);
     }
 }
 
@@ -3188,7 +3183,7 @@ static void gdb_chr_event(void *opaque, QEMUChrEvent event)
             s->processes[i].attached = !i;
         }
 
-        s->c_cpu = gdb_first_attached_cpu(s);
+        s->c_cpu = gdb_first_attached_cpu();
         s->g_cpu = s->c_cpu;
 
         vm_stop(RUN_STATE_PAUSED);
@@ -3199,7 +3194,7 @@ static void gdb_chr_event(void *opaque, QEMUChrEvent event)
     }
 }
 
-static void gdb_monitor_output(GDBState *s, const char *msg, int len)
+static void gdb_monitor_output(const char *msg, int len)
 {
     char buf[MAX_PACKET_LENGTH];
 
@@ -3207,7 +3202,7 @@ static void gdb_monitor_output(GDBState *s, const char *msg, int len)
     if (len > (MAX_PACKET_LENGTH/2) - 1)
         len = (MAX_PACKET_LENGTH/2) - 1;
     memtohex(buf + 1, (uint8_t *)msg, len);
-    put_packet(s, buf);
+    put_packet(buf);
 }
 
 static int gdb_monitor_write(Chardev *chr, const uint8_t *buf, int len)
@@ -3218,10 +3213,10 @@ static int gdb_monitor_write(Chardev *chr, const uint8_t *buf, int len)
     max_sz = (sizeof(gdbserver_state.last_packet) - 2) / 2;
     for (;;) {
         if (len <= max_sz) {
-            gdb_monitor_output(&gdbserver_state, p, len);
+            gdb_monitor_output(p, len);
             break;
         }
-        gdb_monitor_output(&gdbserver_state, p, max_sz);
+        gdb_monitor_output(p, max_sz);
         p += max_sz;
         len -= max_sz;
     }
@@ -3305,9 +3300,9 @@ static void create_processes(GDBState *s)
 {
     object_child_foreach(object_get_root(), find_cpu_clusters, s);
 
-    if (s->processes) {
+    if (gdbserver_state.processes) {
         /* Sort by PID */
-        qsort(s->processes, s->process_num, sizeof(s->processes[0]), pid_order);
+        qsort(gdbserver_state.processes, gdbserver_state.process_num, sizeof(gdbserver_state.processes[0]), pid_order);
     }
 
     create_default_process(s);
@@ -3387,7 +3382,7 @@ int gdbserver_start(const char *device)
 void gdbserver_cleanup(void)
 {
     if (gdbserver_state.init) {
-        put_packet(&gdbserver_state, "W00");
+        put_packet("W00");
     }
 }
 
-- 
2.20.1


[PULL 08/28] gdbstub: move str_buf to GDBState and use GString
Posted by Alex Bennée 4 years, 1 month ago
Rather than having a static buffer replace str_buf with a GString
which we know can grow on demand. Convert the internal functions to
take a GString instead of a char * and length.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Damien Hedde <damien.hedde@greensocs.com>
Tested-by: Damien Hedde <damien.hedde@greensocs.com>
Message-Id: <20200316172155.971-9-alex.bennee@linaro.org>

diff --git a/gdbstub.c b/gdbstub.c
index 7243a2f7af9..f6b42825445 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -366,6 +366,7 @@ typedef struct GDBState {
     int process_num;
     char syscall_buf[256];
     gdb_syscall_complete_cb current_syscall_cb;
+    GString *str_buf;
 } GDBState;
 
 /* By default use no IRQs and no timers while single stepping so as to
@@ -380,6 +381,7 @@ static void init_gdbserver_state(void)
     g_assert(!gdbserver_state.init);
     memset(&gdbserver_state, 0, sizeof(GDBState));
     gdbserver_state.init = true;
+    gdbserver_state.str_buf = g_string_new(NULL);
 }
 
 #ifndef CONFIG_USER_ONLY
@@ -563,17 +565,15 @@ static inline int tohex(int v)
 }
 
 /* writes 2*len+1 bytes in buf */
-static void memtohex(char *buf, const uint8_t *mem, int len)
+static void memtohex(GString *buf, const uint8_t *mem, int len)
 {
     int i, c;
-    char *q;
-    q = buf;
     for(i = 0; i < len; i++) {
         c = mem[i];
-        *q++ = tohex(c >> 4);
-        *q++ = tohex(c & 0xf);
+        g_string_append_c(buf, tohex(c >> 4));
+        g_string_append_c(buf, tohex(c & 0xf));
     }
-    *q = '\0';
+    g_string_append_c(buf, '\0');
 }
 
 static void hextomem(uint8_t *mem, const char *buf, int len)
@@ -667,25 +667,28 @@ static int put_packet(const char *buf)
     return put_packet_binary(buf, strlen(buf), false);
 }
 
+static void put_strbuf(void)
+{
+    put_packet(gdbserver_state.str_buf->str);
+}
+
 /* Encode data using the encoding for 'x' packets.  */
-static int memtox(char *buf, const char *mem, int len)
+static void memtox(GString *buf, const char *mem, int len)
 {
-    char *p = buf;
     char c;
 
     while (len--) {
         c = *(mem++);
         switch (c) {
         case '#': case '$': case '*': case '}':
-            *(p++) = '}';
-            *(p++) = c ^ 0x20;
+            g_string_append_c(buf, '}');
+            g_string_append_c(buf, c ^ 0x20);
             break;
         default:
-            *(p++) = c;
+            g_string_append_c(buf, c);
             break;
         }
     }
-    return p - buf;
 }
 
 static uint32_t gdb_get_cpu_pid(CPUState *cpu)
@@ -1109,16 +1112,14 @@ static void gdb_set_cpu_pc(target_ulong pc)
     cpu_set_pc(cpu, pc);
 }
 
-static char *gdb_fmt_thread_id(CPUState *cpu, char *buf, size_t buf_size)
+static void gdb_append_thread_id(CPUState *cpu, GString *buf)
 {
     if (gdbserver_state.multiprocess) {
-        snprintf(buf, buf_size, "p%02x.%02x",
-                 gdb_get_cpu_pid(cpu), cpu_gdb_index(cpu));
+        g_string_append_printf(buf, "p%02x.%02x",
+                               gdb_get_cpu_pid(cpu), cpu_gdb_index(cpu));
     } else {
-        snprintf(buf, buf_size, "%02x", cpu_gdb_index(cpu));
+        g_string_append_printf(buf, "%02x", cpu_gdb_index(cpu));
     }
-
-    return buf;
 }
 
 typedef enum GDBThreadIdKind {
@@ -1412,7 +1413,6 @@ typedef struct GdbCmdContext {
     GdbCmdVariant *params;
     int num_params;
     uint8_t mem_buf[MAX_PACKET_LENGTH];
-    char str_buf[MAX_PACKET_LENGTH + 1];
 } GdbCmdContext;
 
 typedef void (*GdbCmdHandler)(GdbCmdContext *gdb_ctx, void *user_ctx);
@@ -1502,6 +1502,8 @@ static void run_cmd_parser(const char *data, const GdbCmdParseEntry *cmd)
         return;
     }
 
+    g_string_set_size(gdbserver_state.str_buf, 0);
+
     /* In case there was an error during the command parsing we must
     * send a NULL packet to indicate the command is not supported */
     if (process_string_cmd(NULL, data, cmd, 1)) {
@@ -1740,8 +1742,8 @@ static void handle_get_reg(GdbCmdContext *gdb_ctx, void *user_ctx)
         return;
     }
 
-    memtohex(gdb_ctx->str_buf, gdb_ctx->mem_buf, reg_size);
-    put_packet(gdb_ctx->str_buf);
+    memtohex(gdbserver_state.str_buf, gdb_ctx->mem_buf, reg_size);
+    put_strbuf();
 }
 
 static void handle_write_mem(GdbCmdContext *gdb_ctx, void *user_ctx)
@@ -1789,8 +1791,8 @@ static void handle_read_mem(GdbCmdContext *gdb_ctx, void *user_ctx)
         return;
     }
 
-    memtohex(gdb_ctx->str_buf, gdb_ctx->mem_buf, gdb_ctx->params[1].val_ull);
-    put_packet(gdb_ctx->str_buf);
+    memtohex(gdbserver_state.str_buf, gdb_ctx->mem_buf, gdb_ctx->params[1].val_ull);
+    put_strbuf();
 }
 
 static void handle_write_all_regs(GdbCmdContext *gdb_ctx, void *user_ctx)
@@ -1827,8 +1829,8 @@ static void handle_read_all_regs(GdbCmdContext *gdb_ctx, void *user_ctx)
                                  addr);
     }
 
-    memtohex(gdb_ctx->str_buf, gdb_ctx->mem_buf, len);
-    put_packet(gdb_ctx->str_buf);
+    memtohex(gdbserver_state.str_buf, gdb_ctx->mem_buf, len);
+    put_strbuf();
 }
 
 static void handle_file_io(GdbCmdContext *gdb_ctx, void *user_ctx)
@@ -1889,9 +1891,8 @@ static void handle_v_attach(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
     GDBProcess *process;
     CPUState *cpu;
-    char thread_id[16];
 
-    pstrcpy(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "E22");
+    g_string_assign(gdbserver_state.str_buf, "E22");
     if (!gdb_ctx->num_params) {
         goto cleanup;
     }
@@ -1910,11 +1911,11 @@ static void handle_v_attach(GdbCmdContext *gdb_ctx, void *user_ctx)
     gdbserver_state.g_cpu = cpu;
     gdbserver_state.c_cpu = cpu;
 
-    gdb_fmt_thread_id(cpu, thread_id, sizeof(thread_id));
-    snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "T%02xthread:%s;",
-             GDB_SIGNAL_TRAP, thread_id);
+    g_string_printf(gdbserver_state.str_buf, "T%02xthread:", GDB_SIGNAL_TRAP);
+    gdb_append_thread_id(cpu, gdbserver_state.str_buf);
+    g_string_append_c(gdbserver_state.str_buf, ';');
 cleanup:
-    put_packet(gdb_ctx->str_buf);
+    put_strbuf();
 }
 
 static void handle_v_kill(GdbCmdContext *gdb_ctx, void *user_ctx)
@@ -1966,10 +1967,9 @@ static void handle_v_commands(GdbCmdContext *gdb_ctx, void *user_ctx)
 
 static void handle_query_qemu_sstepbits(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
-    snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf),
-             "ENABLE=%x,NOIRQ=%x,NOTIMER=%x", SSTEP_ENABLE,
-             SSTEP_NOIRQ, SSTEP_NOTIMER);
-    put_packet(gdb_ctx->str_buf);
+    g_string_printf(gdbserver_state.str_buf, "ENABLE=%x,NOIRQ=%x,NOTIMER=%x",
+                    SSTEP_ENABLE, SSTEP_NOIRQ, SSTEP_NOTIMER);
+    put_strbuf();
 }
 
 static void handle_set_qemu_sstep(GdbCmdContext *gdb_ctx, void *user_ctx)
@@ -1984,15 +1984,14 @@ static void handle_set_qemu_sstep(GdbCmdContext *gdb_ctx, void *user_ctx)
 
 static void handle_query_qemu_sstep(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
-    snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "0x%x", sstep_flags);
-    put_packet(gdb_ctx->str_buf);
+    g_string_printf(gdbserver_state.str_buf, "0x%x", sstep_flags);
+    put_strbuf();
 }
 
 static void handle_query_curr_tid(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
     CPUState *cpu;
     GDBProcess *process;
-    char thread_id[16];
 
     /*
      * "Current thread" remains vague in the spec, so always return
@@ -2001,24 +2000,21 @@ static void handle_query_curr_tid(GdbCmdContext *gdb_ctx, void *user_ctx)
      */
     process = gdb_get_cpu_process(gdbserver_state.g_cpu);
     cpu = get_first_cpu_in_process(process);
-    gdb_fmt_thread_id(cpu, thread_id, sizeof(thread_id));
-    snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "QC%s", thread_id);
-    put_packet(gdb_ctx->str_buf);
+    g_string_assign(gdbserver_state.str_buf, "QC");
+    gdb_append_thread_id(cpu, gdbserver_state.str_buf);
+    put_strbuf();
 }
 
 static void handle_query_threads(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
-    char thread_id[16];
-
     if (!gdbserver_state.query_cpu) {
         put_packet("l");
         return;
     }
 
-    gdb_fmt_thread_id(gdbserver_state.query_cpu, thread_id,
-                      sizeof(thread_id));
-    snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "m%s", thread_id);
-    put_packet(gdb_ctx->str_buf);
+    g_string_assign(gdbserver_state.str_buf, "m");
+    gdb_append_thread_id(gdbserver_state.query_cpu, gdbserver_state.str_buf);
+    put_strbuf();
     gdbserver_state.query_cpu = gdb_next_attached_cpu(gdbserver_state.query_cpu);
 }
 
@@ -2030,8 +2026,8 @@ static void handle_query_first_threads(GdbCmdContext *gdb_ctx, void *user_ctx)
 
 static void handle_query_thread_extra(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
+    g_autoptr(GString) rs = g_string_new(NULL);
     CPUState *cpu;
-    int len;
 
     if (!gdb_ctx->num_params ||
         gdb_ctx->params[0].thread_id.kind == GDB_READ_THREAD_ERR) {
@@ -2051,20 +2047,17 @@ static void handle_query_thread_extra(GdbCmdContext *gdb_ctx, void *user_ctx)
         /* Print the CPU model and name in multiprocess mode */
         ObjectClass *oc = object_get_class(OBJECT(cpu));
         const char *cpu_model = object_class_get_name(oc);
-        char *cpu_name = object_get_canonical_path_component(OBJECT(cpu));
-        len = snprintf((char *)gdb_ctx->mem_buf, sizeof(gdb_ctx->str_buf) / 2,
-                       "%s %s [%s]", cpu_model, cpu_name,
-                       cpu->halted ? "halted " : "running");
-        g_free(cpu_name);
+        g_autofree char *cpu_name;
+        cpu_name  = object_get_canonical_path_component(OBJECT(cpu));
+        g_string_printf(rs, "%s %s [%s]", cpu_model, cpu_name,
+                        cpu->halted ? "halted " : "running");
     } else {
-        /* memtohex() doubles the required space */
-        len = snprintf((char *)gdb_ctx->mem_buf, sizeof(gdb_ctx->str_buf) / 2,
-                        "CPU#%d [%s]", cpu->cpu_index,
+        g_string_printf(rs, "CPU#%d [%s]", cpu->cpu_index,
                         cpu->halted ? "halted " : "running");
     }
-    trace_gdbstub_op_extra_info((char *)gdb_ctx->mem_buf);
-    memtohex(gdb_ctx->str_buf, gdb_ctx->mem_buf, len);
-    put_packet(gdb_ctx->str_buf);
+    trace_gdbstub_op_extra_info(rs->str);
+    memtohex(gdbserver_state.str_buf, (uint8_t *)rs->str, rs->len);
+    put_strbuf();
 }
 
 #ifdef CONFIG_USER_ONLY
@@ -2073,13 +2066,14 @@ static void handle_query_offsets(GdbCmdContext *gdb_ctx, void *user_ctx)
     TaskState *ts;
 
     ts = gdbserver_state.c_cpu->opaque;
-    snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf),
-             "Text=" TARGET_ABI_FMT_lx ";Data=" TARGET_ABI_FMT_lx
-             ";Bss=" TARGET_ABI_FMT_lx,
-             ts->info->code_offset,
-             ts->info->data_offset,
-             ts->info->data_offset);
-    put_packet(gdb_ctx->str_buf);
+    g_string_printf(gdbserver_state.str_buf,
+                    "Text=" TARGET_ABI_FMT_lx
+                    ";Data=" TARGET_ABI_FMT_lx
+                    ";Bss=" TARGET_ABI_FMT_lx,
+                    ts->info->code_offset,
+                    ts->info->data_offset,
+                    ts->info->data_offset);
+    put_strbuf();
 }
 #else
 static void handle_query_rcmd(GdbCmdContext *gdb_ctx, void *user_ctx)
@@ -2110,12 +2104,10 @@ static void handle_query_supported(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
     CPUClass *cc;
 
-    snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "PacketSize=%x",
-             MAX_PACKET_LENGTH);
+    g_string_printf(gdbserver_state.str_buf, "PacketSize=%x", MAX_PACKET_LENGTH);
     cc = CPU_GET_CLASS(first_cpu);
     if (cc->gdb_core_xml_file) {
-        pstrcat(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf),
-                ";qXfer:features:read+");
+        g_string_append(gdbserver_state.str_buf, ";qXfer:features:read+");
     }
 
     if (gdb_ctx->num_params &&
@@ -2123,8 +2115,8 @@ static void handle_query_supported(GdbCmdContext *gdb_ctx, void *user_ctx)
         gdbserver_state.multiprocess = true;
     }
 
-    pstrcat(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), ";multiprocess+");
-    put_packet(gdb_ctx->str_buf);
+    g_string_append(gdbserver_state.str_buf, ";multiprocess+");
+    put_strbuf();
 }
 
 static void handle_query_xfer_features(GdbCmdContext *gdb_ctx, void *user_ctx)
@@ -2168,14 +2160,15 @@ static void handle_query_xfer_features(GdbCmdContext *gdb_ctx, void *user_ctx)
     }
 
     if (len < total_len - addr) {
-        gdb_ctx->str_buf[0] = 'm';
-        len = memtox(gdb_ctx->str_buf + 1, xml + addr, len);
+        g_string_assign(gdbserver_state.str_buf, "m");
+        memtox(gdbserver_state.str_buf, xml + addr, len);
     } else {
-        gdb_ctx->str_buf[0] = 'l';
-        len = memtox(gdb_ctx->str_buf + 1, xml + addr, total_len - addr);
+        g_string_assign(gdbserver_state.str_buf, "l");
+        memtox(gdbserver_state.str_buf, xml + addr, total_len - addr);
     }
 
-    put_packet_binary(gdb_ctx->str_buf, len + 1, true);
+    put_packet_binary(gdbserver_state.str_buf->str,
+                      gdbserver_state.str_buf->len, true);
 }
 
 static void handle_query_attached(GdbCmdContext *gdb_ctx, void *user_ctx)
@@ -2185,19 +2178,19 @@ static void handle_query_attached(GdbCmdContext *gdb_ctx, void *user_ctx)
 
 static void handle_query_qemu_supported(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
-    snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "sstepbits;sstep");
+    g_string_printf(gdbserver_state.str_buf, "sstepbits;sstep");
 #ifndef CONFIG_USER_ONLY
-    pstrcat(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), ";PhyMemMode");
+    g_string_append(gdbserver_state.str_buf, ";PhyMemMode");
 #endif
-    put_packet(gdb_ctx->str_buf);
+    put_strbuf();
 }
 
 #ifndef CONFIG_USER_ONLY
 static void handle_query_qemu_phy_mem_mode(GdbCmdContext *gdb_ctx,
                                            void *user_ctx)
 {
-    snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "%d", phy_memory_mode);
-    put_packet(gdb_ctx->str_buf);
+    g_string_printf(gdbserver_state.str_buf, "%d", phy_memory_mode);
+    put_strbuf();
 }
 
 static void handle_set_qemu_phy_mem_mode(GdbCmdContext *gdb_ctx, void *user_ctx)
@@ -2362,13 +2355,10 @@ static void handle_gen_set(GdbCmdContext *gdb_ctx, void *user_ctx)
 
 static void handle_target_halt(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
-    char thread_id[16];
-
-    gdb_fmt_thread_id(gdbserver_state.c_cpu, thread_id,
-                      sizeof(thread_id));
-    snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "T%02xthread:%s;",
-             GDB_SIGNAL_TRAP, thread_id);
-    put_packet(gdb_ctx->str_buf);
+    g_string_printf(gdbserver_state.str_buf, "T%02xthread:", GDB_SIGNAL_TRAP);
+    gdb_append_thread_id(gdbserver_state.c_cpu, gdbserver_state.str_buf);
+    g_string_append_c(gdbserver_state.str_buf, ';');
+    put_strbuf();
     /*
      * Remove all the breakpoints when this query is issued,
      * because gdb is doing an initial connect and the state
@@ -2631,8 +2621,8 @@ void gdb_set_stop_cpu(CPUState *cpu)
 static void gdb_vm_state_change(void *opaque, int running, RunState state)
 {
     CPUState *cpu = gdbserver_state.c_cpu;
-    char buf[256];
-    char thread_id[16];
+    g_autoptr(GString) buf = g_string_new(NULL);
+    g_autoptr(GString) tid = g_string_new(NULL);
     const char *type;
     int ret;
 
@@ -2650,7 +2640,7 @@ static void gdb_vm_state_change(void *opaque, int running, RunState state)
         return;
     }
 
-    gdb_fmt_thread_id(cpu, thread_id, sizeof(thread_id));
+    gdb_append_thread_id(cpu, tid);
 
     switch (state) {
     case RUN_STATE_DEBUG:
@@ -2668,10 +2658,9 @@ static void gdb_vm_state_change(void *opaque, int running, RunState state)
             }
             trace_gdbstub_hit_watchpoint(type, cpu_gdb_index(cpu),
                     (target_ulong)cpu->watchpoint_hit->vaddr);
-            snprintf(buf, sizeof(buf),
-                     "T%02xthread:%s;%swatch:" TARGET_FMT_lx ";",
-                     GDB_SIGNAL_TRAP, thread_id, type,
-                     (target_ulong)cpu->watchpoint_hit->vaddr);
+            g_string_printf(buf, "T%02xthread:%s;%swatch:" TARGET_FMT_lx ";",
+                            GDB_SIGNAL_TRAP, tid->str, type,
+                            (target_ulong)cpu->watchpoint_hit->vaddr);
             cpu->watchpoint_hit = NULL;
             goto send_packet;
         } else {
@@ -2712,10 +2701,10 @@ static void gdb_vm_state_change(void *opaque, int running, RunState state)
         break;
     }
     gdb_set_stop_cpu(cpu);
-    snprintf(buf, sizeof(buf), "T%02xthread:%s;", ret, thread_id);
+    g_string_printf(buf, "T%02xthread:%s;", ret, tid->str);
 
 send_packet:
-    put_packet(buf);
+    put_packet(buf->str);
 
     /* disable single step if it was enabled */
     cpu_single_step(cpu, 0);
@@ -3196,13 +3185,9 @@ static void gdb_chr_event(void *opaque, QEMUChrEvent event)
 
 static void gdb_monitor_output(const char *msg, int len)
 {
-    char buf[MAX_PACKET_LENGTH];
-
-    buf[0] = 'O';
-    if (len > (MAX_PACKET_LENGTH/2) - 1)
-        len = (MAX_PACKET_LENGTH/2) - 1;
-    memtohex(buf + 1, (uint8_t *)msg, len);
-    put_packet(buf);
+    g_autoptr(GString) buf = g_string_new("O");
+    memtohex(buf, (uint8_t *)msg, len);
+    put_packet(buf->str);
 }
 
 static int gdb_monitor_write(Chardev *chr, const uint8_t *buf, int len)
-- 
2.20.1


[PULL 09/28] gdbstub: move mem_buf to GDBState and use GByteArray
Posted by Alex Bennée 4 years, 1 month ago
This is in preparation for further re-factoring of the register API
with the rest of the code. Theoretically the read register function
could overwrite the MAX_PACKET_LENGTH buffer although currently all
registers are well within the size range.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Damien Hedde <damien.hedde@greensocs.com>
Tested-by: Damien Hedde <damien.hedde@greensocs.com>

Message-Id: <20200316172155.971-10-alex.bennee@linaro.org>

diff --git a/gdbstub.c b/gdbstub.c
index f6b42825445..db537a712cc 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -367,6 +367,7 @@ typedef struct GDBState {
     char syscall_buf[256];
     gdb_syscall_complete_cb current_syscall_cb;
     GString *str_buf;
+    GByteArray *mem_buf;
 } GDBState;
 
 /* By default use no IRQs and no timers while single stepping so as to
@@ -382,6 +383,7 @@ static void init_gdbserver_state(void)
     memset(&gdbserver_state, 0, sizeof(GDBState));
     gdbserver_state.init = true;
     gdbserver_state.str_buf = g_string_new(NULL);
+    gdbserver_state.mem_buf = g_byte_array_sized_new(MAX_PACKET_LENGTH);
 }
 
 #ifndef CONFIG_USER_ONLY
@@ -576,12 +578,13 @@ static void memtohex(GString *buf, const uint8_t *mem, int len)
     g_string_append_c(buf, '\0');
 }
 
-static void hextomem(uint8_t *mem, const char *buf, int len)
+static void hextomem(GByteArray *mem, const char *buf, int len)
 {
     int i;
 
     for(i = 0; i < len; i++) {
-        mem[i] = (fromhex(buf[0]) << 4) | fromhex(buf[1]);
+        guint8 byte = fromhex(buf[0]) << 4 | fromhex(buf[1]);
+        g_byte_array_append(mem, &byte, 1);
         buf += 2;
     }
 }
@@ -1412,7 +1415,6 @@ static int cmd_parse_params(const char *data, const char *schema,
 typedef struct GdbCmdContext {
     GdbCmdVariant *params;
     int num_params;
-    uint8_t mem_buf[MAX_PACKET_LENGTH];
 } GdbCmdContext;
 
 typedef void (*GdbCmdHandler)(GdbCmdContext *gdb_ctx, void *user_ctx);
@@ -1503,6 +1505,7 @@ static void run_cmd_parser(const char *data, const GdbCmdParseEntry *cmd)
     }
 
     g_string_set_size(gdbserver_state.str_buf, 0);
+    g_byte_array_set_size(gdbserver_state.mem_buf, 0);
 
     /* In case there was an error during the command parsing we must
     * send a NULL packet to indicate the command is not supported */
@@ -1715,8 +1718,8 @@ static void handle_set_reg(GdbCmdContext *gdb_ctx, void *user_ctx)
     }
 
     reg_size = strlen(gdb_ctx->params[1].data) / 2;
-    hextomem(gdb_ctx->mem_buf, gdb_ctx->params[1].data, reg_size);
-    gdb_write_register(gdbserver_state.g_cpu, gdb_ctx->mem_buf,
+    hextomem(gdbserver_state.mem_buf, gdb_ctx->params[1].data, reg_size);
+    gdb_write_register(gdbserver_state.g_cpu, gdbserver_state.mem_buf->data,
                        gdb_ctx->params[0].val_ull);
     put_packet("OK");
 }
@@ -1735,14 +1738,17 @@ static void handle_get_reg(GdbCmdContext *gdb_ctx, void *user_ctx)
         return;
     }
 
-    reg_size = gdb_read_register(gdbserver_state.g_cpu, gdb_ctx->mem_buf,
+    reg_size = gdb_read_register(gdbserver_state.g_cpu,
+                                 gdbserver_state.mem_buf->data,
                                  gdb_ctx->params[0].val_ull);
     if (!reg_size) {
         put_packet("E14");
         return;
+    } else {
+        g_byte_array_set_size(gdbserver_state.mem_buf, reg_size);
     }
 
-    memtohex(gdbserver_state.str_buf, gdb_ctx->mem_buf, reg_size);
+    memtohex(gdbserver_state.str_buf, gdbserver_state.mem_buf->data, reg_size);
     put_strbuf();
 }
 
@@ -1759,11 +1765,11 @@ static void handle_write_mem(GdbCmdContext *gdb_ctx, void *user_ctx)
         return;
     }
 
-    hextomem(gdb_ctx->mem_buf, gdb_ctx->params[2].data,
+    hextomem(gdbserver_state.mem_buf, gdb_ctx->params[2].data,
              gdb_ctx->params[1].val_ull);
     if (target_memory_rw_debug(gdbserver_state.g_cpu, gdb_ctx->params[0].val_ull,
-                               gdb_ctx->mem_buf,
-                               gdb_ctx->params[1].val_ull, true)) {
+                               gdbserver_state.mem_buf->data,
+                               gdbserver_state.mem_buf->len, true)) {
         put_packet("E14");
         return;
     }
@@ -1784,14 +1790,17 @@ static void handle_read_mem(GdbCmdContext *gdb_ctx, void *user_ctx)
         return;
     }
 
+    g_byte_array_set_size(gdbserver_state.mem_buf, gdb_ctx->params[1].val_ull);
+
     if (target_memory_rw_debug(gdbserver_state.g_cpu, gdb_ctx->params[0].val_ull,
-                               gdb_ctx->mem_buf,
-                               gdb_ctx->params[1].val_ull, false)) {
+                               gdbserver_state.mem_buf->data,
+                               gdbserver_state.mem_buf->len, false)) {
         put_packet("E14");
         return;
     }
 
-    memtohex(gdbserver_state.str_buf, gdb_ctx->mem_buf, gdb_ctx->params[1].val_ull);
+    memtohex(gdbserver_state.str_buf, gdbserver_state.mem_buf->data,
+             gdbserver_state.mem_buf->len);
     put_strbuf();
 }
 
@@ -1806,9 +1815,9 @@ static void handle_write_all_regs(GdbCmdContext *gdb_ctx, void *user_ctx)
     }
 
     cpu_synchronize_state(gdbserver_state.g_cpu);
-    registers = gdb_ctx->mem_buf;
     len = strlen(gdb_ctx->params[0].data) / 2;
-    hextomem(registers, gdb_ctx->params[0].data, len);
+    hextomem(gdbserver_state.mem_buf, gdb_ctx->params[0].data, len);
+    registers = gdbserver_state.mem_buf->data;
     for (addr = 0; addr < gdbserver_state.g_cpu->gdb_num_g_regs && len > 0;
          addr++) {
         reg_size = gdb_write_register(gdbserver_state.g_cpu, registers, addr);
@@ -1825,11 +1834,14 @@ static void handle_read_all_regs(GdbCmdContext *gdb_ctx, void *user_ctx)
     cpu_synchronize_state(gdbserver_state.g_cpu);
     len = 0;
     for (addr = 0; addr < gdbserver_state.g_cpu->gdb_num_g_regs; addr++) {
-        len += gdb_read_register(gdbserver_state.g_cpu, gdb_ctx->mem_buf + len,
+        len += gdb_read_register(gdbserver_state.g_cpu,
+                                 gdbserver_state.mem_buf->data + len,
                                  addr);
     }
+    /* FIXME: This is after the fact sizing */
+    g_byte_array_set_size(gdbserver_state.mem_buf, len);
 
-    memtohex(gdbserver_state.str_buf, gdb_ctx->mem_buf, len);
+    memtohex(gdbserver_state.str_buf, gdbserver_state.mem_buf->data, len);
     put_strbuf();
 }
 
@@ -2078,6 +2090,7 @@ static void handle_query_offsets(GdbCmdContext *gdb_ctx, void *user_ctx)
 #else
 static void handle_query_rcmd(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
+    const guint8 zero = 0;
     int len;
 
     if (!gdb_ctx->num_params) {
@@ -2091,12 +2104,13 @@ static void handle_query_rcmd(GdbCmdContext *gdb_ctx, void *user_ctx)
         return;
     }
 
+    g_assert(gdbserver_state.mem_buf->len == 0);
     len = len / 2;
-    hextomem(gdb_ctx->mem_buf, gdb_ctx->params[0].data, len);
-    gdb_ctx->mem_buf[len++] = 0;
-    qemu_chr_be_write(gdbserver_state.mon_chr, gdb_ctx->mem_buf, len);
+    hextomem(gdbserver_state.mem_buf, gdb_ctx->params[0].data, len);
+    g_byte_array_append(gdbserver_state.mem_buf, &zero, 1);
+    qemu_chr_be_write(gdbserver_state.mon_chr, gdbserver_state.mem_buf->data,
+                      gdbserver_state.mem_buf->len);
     put_packet("OK");
-
 }
 #endif
 
-- 
2.20.1


[PULL 10/28] gdbstub: add helper for 128 bit registers
Posted by Alex Bennée 4 years, 1 month ago
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20200316172155.971-11-alex.bennee@linaro.org>

diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index 08363969c14..59e366ba3af 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -102,6 +102,19 @@ static inline int gdb_get_reg64(uint8_t *mem_buf, uint64_t val)
     return 8;
 }
 
+static inline int gdb_get_reg128(uint8_t *mem_buf, uint64_t val_hi,
+                                 uint64_t val_lo)
+{
+#ifdef TARGET_WORDS_BIGENDIAN
+    stq_p(mem_buf, val_hi);
+    stq_p(mem_buf + 8, val_lo);
+#else
+    stq_p(mem_buf, val_lo);
+    stq_p(mem_buf + 8, val_hi);
+#endif
+    return 16;
+}
+
 #if TARGET_LONG_BITS == 64
 #define gdb_get_regl(buf, val) gdb_get_reg64(buf, val)
 #define ldtul_p(addr) ldq_p(addr)
-- 
2.20.1


[PULL 11/28] target/arm: use gdb_get_reg helpers
Posted by Alex Bennée 4 years, 1 month ago
This is cleaner than poking memory directly and will make later
clean-ups easier.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Message-Id: <20200316172155.971-12-alex.bennee@linaro.org>

diff --git a/target/arm/helper.c b/target/arm/helper.c
index b61ee73d18a..69104fb351d 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -106,21 +106,17 @@ static int aarch64_fpu_gdb_get_reg(CPUARMState *env, uint8_t *buf, int reg)
 {
     switch (reg) {
     case 0 ... 31:
-        /* 128 bit FP register */
-        {
-            uint64_t *q = aa64_vfp_qreg(env, reg);
-            stq_le_p(buf, q[0]);
-            stq_le_p(buf + 8, q[1]);
-            return 16;
-        }
+    {
+        /* 128 bit FP register - quads are in LE order */
+        uint64_t *q = aa64_vfp_qreg(env, reg);
+        return gdb_get_reg128(buf, q[1], q[0]);
+    }
     case 32:
         /* FPSR */
-        stl_p(buf, vfp_get_fpsr(env));
-        return 4;
+        return gdb_get_reg32(buf, vfp_get_fpsr(env));
     case 33:
         /* FPCR */
-        stl_p(buf, vfp_get_fpcr(env));
-        return 4;
+        return gdb_get_reg32(buf,vfp_get_fpcr(env));
     default:
         return 0;
     }
-- 
2.20.1


[PULL 12/28] target/m68k: use gdb_get_reg helpers
Posted by Alex Bennée 4 years, 1 month ago
This is cleaner than poking memory directly and will make later
clean-ups easier.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20200316172155.971-13-alex.bennee@linaro.org>

diff --git a/target/m68k/helper.c b/target/m68k/helper.c
index baf7729af00..c23b70f854d 100644
--- a/target/m68k/helper.c
+++ b/target/m68k/helper.c
@@ -72,19 +72,15 @@ static int cf_fpu_gdb_get_reg(CPUM68KState *env, uint8_t *mem_buf, int n)
 {
     if (n < 8) {
         float_status s;
-        stfq_p(mem_buf, floatx80_to_float64(env->fregs[n].d, &s));
-        return 8;
+        return gdb_get_reg64(mem_buf, floatx80_to_float64(env->fregs[n].d, &s));
     }
     switch (n) {
     case 8: /* fpcontrol */
-        stl_be_p(mem_buf, env->fpcr);
-        return 4;
+        return gdb_get_reg32(mem_buf, env->fpcr);
     case 9: /* fpstatus */
-        stl_be_p(mem_buf, env->fpsr);
-        return 4;
+        return gdb_get_reg32(mem_buf, env->fpsr);
     case 10: /* fpiar, not implemented */
-        memset(mem_buf, 0, 4);
-        return 4;
+        return gdb_get_reg32(mem_buf, 0);
     }
     return 0;
 }
@@ -112,21 +108,18 @@ static int cf_fpu_gdb_set_reg(CPUM68KState *env, uint8_t *mem_buf, int n)
 static int m68k_fpu_gdb_get_reg(CPUM68KState *env, uint8_t *mem_buf, int n)
 {
     if (n < 8) {
-        stw_be_p(mem_buf, env->fregs[n].l.upper);
-        memset(mem_buf + 2, 0, 2);
-        stq_be_p(mem_buf + 4, env->fregs[n].l.lower);
-        return 12;
+        int len = gdb_get_reg16(mem_buf, env->fregs[n].l.upper);
+        len += gdb_get_reg16(mem_buf + len, 0);
+        len += gdb_get_reg64(mem_buf + len, env->fregs[n].l.lower);
+        return len;
     }
     switch (n) {
     case 8: /* fpcontrol */
-        stl_be_p(mem_buf, env->fpcr);
-        return 4;
+        return gdb_get_reg32(mem_buf, env->fpcr);
     case 9: /* fpstatus */
-        stl_be_p(mem_buf, env->fpsr);
-        return 4;
+        return gdb_get_reg32(mem_buf, env->fpsr);
     case 10: /* fpiar, not implemented */
-        memset(mem_buf, 0, 4);
-        return 4;
+        return gdb_get_reg32(mem_buf, 0);
     }
     return 0;
 }
-- 
2.20.1


[PULL 13/28] target/i386: use gdb_get_reg helpers
Posted by Alex Bennée 4 years, 1 month ago
This is cleaner than poking memory directly and will make later
clean-ups easier.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20200316172155.971-14-alex.bennee@linaro.org>

diff --git a/target/i386/gdbstub.c b/target/i386/gdbstub.c
index 572ead641ca..e4d8cb66c00 100644
--- a/target/i386/gdbstub.c
+++ b/target/i386/gdbstub.c
@@ -98,26 +98,22 @@ int x86_cpu_gdb_read_register(CPUState *cs, uint8_t *mem_buf, int n)
                 return gdb_get_reg64(mem_buf,
                                      env->regs[gpr_map[n]] & 0xffffffffUL);
             } else {
-                memset(mem_buf, 0, sizeof(target_ulong));
-                return sizeof(target_ulong);
+                return gdb_get_regl(mem_buf, 0);
             }
         } else {
             return gdb_get_reg32(mem_buf, env->regs[gpr_map32[n]]);
         }
     } else if (n >= IDX_FP_REGS && n < IDX_FP_REGS + 8) {
-#ifdef USE_X86LDOUBLE
-        /* FIXME: byteswap float values - after fixing fpregs layout. */
-        memcpy(mem_buf, &env->fpregs[n - IDX_FP_REGS], 10);
-#else
-        memset(mem_buf, 0, 10);
-#endif
-        return 10;
+        floatx80 *fp = (floatx80 *) &env->fpregs[n - IDX_FP_REGS];
+        int len = gdb_get_reg64(mem_buf, cpu_to_le64(fp->low));
+        len += gdb_get_reg16(mem_buf + len, cpu_to_le16(fp->high));
+        return len;
     } else if (n >= IDX_XMM_REGS && n < IDX_XMM_REGS + CPU_NB_REGS) {
         n -= IDX_XMM_REGS;
         if (n < CPU_NB_REGS32 || TARGET_LONG_BITS == 64) {
-            stq_p(mem_buf, env->xmm_regs[n].ZMM_Q(0));
-            stq_p(mem_buf + 8, env->xmm_regs[n].ZMM_Q(1));
-            return 16;
+            return gdb_get_reg128(mem_buf,
+                                  env->xmm_regs[n].ZMM_Q(0),
+                                  env->xmm_regs[n].ZMM_Q(1));
         }
     } else {
         switch (n) {
@@ -290,10 +286,9 @@ int x86_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
             return 4;
         }
     } else if (n >= IDX_FP_REGS && n < IDX_FP_REGS + 8) {
-#ifdef USE_X86LDOUBLE
-        /* FIXME: byteswap float values - after fixing fpregs layout. */
-        memcpy(&env->fpregs[n - IDX_FP_REGS], mem_buf, 10);
-#endif
+        floatx80 *fp = (floatx80 *) &env->fpregs[n - IDX_FP_REGS];
+        fp->low = le64_to_cpu(* (uint64_t *) mem_buf);
+        fp->high = le16_to_cpu(* (uint16_t *) (mem_buf + 8));
         return 10;
     } else if (n >= IDX_XMM_REGS && n < IDX_XMM_REGS + CPU_NB_REGS) {
         n -= IDX_XMM_REGS;
-- 
2.20.1


[PULL 17/28] target/arm: default SVE length to 64 bytes for linux-user
Posted by Alex Bennée 4 years, 1 month ago
The Linux kernel chooses the default of 64 bytes for SVE registers on
the basis that it is the largest size on known hardware that won't
grow the signal frame. We still honour the sve-max-vq property and
userspace can expand the number of lanes by calling PR_SVE_SET_VL.

This should not make any difference to SVE enabled software as the SVE
is of course vector length agnostic.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Message-Id: <20200316172155.971-18-alex.bennee@linaro.org>

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 3623ecefbd9..0909ce86a12 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -195,9 +195,10 @@ static void arm_cpu_reset(CPUState *s)
         env->cp15.cpacr_el1 = deposit64(env->cp15.cpacr_el1, 20, 2, 3);
         /* and to the SVE instructions */
         env->cp15.cpacr_el1 = deposit64(env->cp15.cpacr_el1, 16, 2, 3);
-        /* with maximum vector length */
-        env->vfp.zcr_el[1] = cpu_isar_feature(aa64_sve, cpu) ?
-                             cpu->sve_max_vq - 1 : 0;
+        /* with reasonable vector length */
+        if (cpu_isar_feature(aa64_sve, cpu)) {
+            env->vfp.zcr_el[1] = MIN(cpu->sve_max_vq - 1, 3);
+        }
         /*
          * Enable TBI0 and TBI1.  While the real kernel only enables TBI0,
          * turning on both here will produce smaller code and otherwise
-- 
2.20.1


[PULL 19/28] target/arm: don't bother with id_aa64pfr0_read for USER_ONLY
Posted by Alex Bennée 4 years, 1 month ago
For system emulation we need to check the state of the GIC before we
report the value. However this isn't relevant to exporting of the
value to linux-user and indeed breaks the exported value as set by
modify_arm_cp_regs.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20200316172155.971-20-alex.bennee@linaro.org>

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 7e560ea7db6..d2ec2c53510 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6697,6 +6697,7 @@ static uint64_t id_pfr1_read(CPUARMState *env, const ARMCPRegInfo *ri)
     return pfr1;
 }
 
+#ifndef CONFIG_USER_ONLY
 static uint64_t id_aa64pfr0_read(CPUARMState *env, const ARMCPRegInfo *ri)
 {
     ARMCPU *cpu = env_archcpu(env);
@@ -6707,6 +6708,7 @@ static uint64_t id_aa64pfr0_read(CPUARMState *env, const ARMCPRegInfo *ri)
     }
     return pfr0;
 }
+#endif
 
 /* Shared logic between LORID and the rest of the LOR* registers.
  * Secure state has already been delt with.
@@ -7280,16 +7282,24 @@ void register_cp_regs_for_features(ARMCPU *cpu)
          * define new registers here.
          */
         ARMCPRegInfo v8_idregs[] = {
-            /* ID_AA64PFR0_EL1 is not a plain ARM_CP_CONST because we don't
-             * know the right value for the GIC field until after we
-             * define these regs.
+            /*
+             * ID_AA64PFR0_EL1 is not a plain ARM_CP_CONST in system
+             * emulation because we don't know the right value for the
+             * GIC field until after we define these regs.
              */
             { .name = "ID_AA64PFR0_EL1", .state = ARM_CP_STATE_AA64,
               .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 4, .opc2 = 0,
-              .access = PL1_R, .type = ARM_CP_NO_RAW,
+              .access = PL1_R,
+#ifdef CONFIG_USER_ONLY
+              .type = ARM_CP_CONST,
+              .resetvalue = cpu->isar.id_aa64pfr0
+#else
+              .type = ARM_CP_NO_RAW,
               .accessfn = access_aa64_tid3,
               .readfn = id_aa64pfr0_read,
-              .writefn = arm_cp_write_ignore },
+              .writefn = arm_cp_write_ignore
+#endif
+            },
             { .name = "ID_AA64PFR1_EL1", .state = ARM_CP_STATE_AA64,
               .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 4, .opc2 = 1,
               .access = PL1_R, .type = ARM_CP_CONST,
-- 
2.20.1


[PULL 21/28] configure: allow user to specify what gdb to use
Posted by Alex Bennée 4 years, 1 month ago
This is useful, especially when testing relatively new gdbstub
features that might not be in distro packages yet.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20200316172155.971-22-alex.bennee@linaro.org>

diff --git a/configure b/configure
index eb49bb6680c..057994bce69 100755
--- a/configure
+++ b/configure
@@ -303,6 +303,7 @@ libs_qga=""
 debug_info="yes"
 stack_protector=""
 use_containers="yes"
+gdb_bin=$(command -v "gdb")
 
 if test -e "$source_path/.git"
 then
@@ -1588,6 +1589,8 @@ for opt do
   ;;
   --disable-fuzzing) fuzzing=no
   ;;
+  --gdb=*) gdb_bin="$optarg"
+  ;;
   *)
       echo "ERROR: unknown option $opt"
       echo "Try '$0 --help' for more information"
@@ -1773,6 +1776,7 @@ Advanced options (experts only):
   --enable-plugins
                            enable plugins via shared library loading
   --disable-containers     don't use containers for cross-building
+  --gdb=GDB-path           gdb to use for gdbstub tests [$gdb_bin]
 
 Optional features, enabled with --enable-FEATURE and
 disabled with --disable-FEATURE, default is enabled if available:
@@ -6734,6 +6738,7 @@ echo "libudev           $libudev"
 echo "default devices   $default_devices"
 echo "plugin support    $plugins"
 echo "fuzzing support   $fuzzing"
+echo "gdb               $gdb_bin"
 
 if test "$supported_cpu" = "no"; then
     echo
@@ -7608,6 +7613,10 @@ if test "$plugins" = "yes" ; then
     fi
 fi
 
+if test -n "$gdb_bin" ; then
+    echo "HAVE_GDB_BIN=$gdb_bin" >> $config_host_mak
+fi
+
 if test "$tcg_interpreter" = "yes"; then
   QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/tci $QEMU_INCLUDES"
 elif test "$ARCH" = "sparc64" ; then
-- 
2.20.1


[PULL 22/28] tests/guest-debug: add a simple test runner
Posted by Alex Bennée 4 years, 1 month ago
The test runners job is to start QEMU with guest debug enabled and
then spawn a gdb process running a test script that exercises the
functionality it wants to test.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20200316172155.971-23-alex.bennee@linaro.org>

diff --git a/tests/guest-debug/run-test.py b/tests/guest-debug/run-test.py
new file mode 100755
index 00000000000..8c49ee2f225
--- /dev/null
+++ b/tests/guest-debug/run-test.py
@@ -0,0 +1,57 @@
+#!/usr/bin/env python3
+#
+# Run a gdbstub test case
+#
+# Copyright (c) 2019 Linaro
+#
+# Author: Alex Bennée <alex.bennee@linaro.org>
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or later.
+# See the COPYING file in the top-level directory.
+#
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+import argparse
+import subprocess
+import shutil
+import shlex
+
+def get_args():
+    parser = argparse.ArgumentParser(description="A gdbstub test runner")
+    parser.add_argument("--qemu", help="Qemu binary for test",
+                        required=True)
+    parser.add_argument("--qargs", help="Qemu arguments for test")
+    parser.add_argument("--binary", help="Binary to debug",
+                        required=True)
+    parser.add_argument("--test", help="GDB test script",
+                        required=True)
+    parser.add_argument("--gdb", help="The gdb binary to use", default=None)
+
+    return parser.parse_args()
+
+if __name__ == '__main__':
+    args = get_args()
+
+    # Search for a gdb we can use
+    if not args.gdb:
+        args.gdb = shutil.which("gdb-multiarch")
+    if not args.gdb:
+        args.gdb = shutil.which("gdb")
+    if not args.gdb:
+        print("We need gdb to run the test")
+        exit(-1)
+
+    # Launch QEMU with binary
+    if "system" in args.qemu:
+        cmd = "%s %s %s -s -S" % (args.qemu, args.qargs, args.binary)
+    else:
+        cmd = "%s %s -g 1234 %s" % (args.qemu, args.qargs, args.binary)
+
+    inferior = subprocess.Popen(shlex.split(cmd))
+
+    # Now launch gdb with our test and collect the result
+    gdb_cmd = "%s %s -ex 'target remote localhost:1234' -x %s" % (args.gdb, args.binary, args.test)
+
+    result = subprocess.call(gdb_cmd, shell=True);
+
+    exit(result)
-- 
2.20.1


[PULL 26/28] gdbstub: change GDBState.last_packet to GByteArray
Posted by Alex Bennée 4 years, 1 month ago
From: Damien Hedde <damien.hedde@greensocs.com>

Remove the packet size upper limit by using a GByteArray
instead of a statically allocated array for last_packet.
Thus we can now send big packets.

Also remove the last_packet_len field and use last_packet->len
instead.

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20191211160514.58373-2-damien.hedde@greensocs.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20200316172155.971-27-alex.bennee@linaro.org>

diff --git a/gdbstub.c b/gdbstub.c
index 0bcfc47a7c5..a60ef5125eb 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -351,8 +351,7 @@ typedef struct GDBState {
     int line_buf_index;
     int line_sum; /* running checksum */
     int line_csum; /* checksum at the end of the packet */
-    uint8_t last_packet[MAX_PACKET_LENGTH + 4];
-    int last_packet_len;
+    GByteArray *last_packet;
     int signal;
 #ifdef CONFIG_USER_ONLY
     int fd;
@@ -384,6 +383,7 @@ static void init_gdbserver_state(void)
     gdbserver_state.init = true;
     gdbserver_state.str_buf = g_string_new(NULL);
     gdbserver_state.mem_buf = g_byte_array_sized_new(MAX_PACKET_LENGTH);
+    gdbserver_state.last_packet = g_byte_array_sized_new(MAX_PACKET_LENGTH + 4);
 }
 
 #ifndef CONFIG_USER_ONLY
@@ -626,28 +626,29 @@ static void hexdump(const char *buf, int len,
 static int put_packet_binary(const char *buf, int len, bool dump)
 {
     int csum, i;
-    uint8_t *p;
-    uint8_t *ps = &gdbserver_state.last_packet[0];
+    uint8_t footer[3];
 
     if (dump && trace_event_get_state_backends(TRACE_GDBSTUB_IO_BINARYREPLY)) {
         hexdump(buf, len, trace_gdbstub_io_binaryreply);
     }
 
     for(;;) {
-        p = ps;
-        *(p++) = '$';
-        memcpy(p, buf, len);
-        p += len;
+        g_byte_array_set_size(gdbserver_state.last_packet, 0);
+        g_byte_array_append(gdbserver_state.last_packet,
+                            (const uint8_t *) "$", 1);
+        g_byte_array_append(gdbserver_state.last_packet,
+                            (const uint8_t *) buf, len);
         csum = 0;
         for(i = 0; i < len; i++) {
             csum += buf[i];
         }
-        *(p++) = '#';
-        *(p++) = tohex((csum >> 4) & 0xf);
-        *(p++) = tohex((csum) & 0xf);
+        footer[0] = '#';
+        footer[1] = tohex((csum >> 4) & 0xf);
+        footer[2] = tohex((csum) & 0xf);
+        g_byte_array_append(gdbserver_state.last_packet, footer, 3);
 
-        gdbserver_state.last_packet_len = p - ps;
-        put_buffer(ps, gdbserver_state.last_packet_len);
+        put_buffer(gdbserver_state.last_packet->data,
+                   gdbserver_state.last_packet->len);
 
 #ifdef CONFIG_USER_ONLY
         i = get_char();
@@ -2812,20 +2813,22 @@ static void gdb_read_byte(uint8_t ch)
     uint8_t reply;
 
 #ifndef CONFIG_USER_ONLY
-    if (gdbserver_state.last_packet_len) {
+    if (gdbserver_state.last_packet->len) {
         /* Waiting for a response to the last packet.  If we see the start
            of a new command then abandon the previous response.  */
         if (ch == '-') {
             trace_gdbstub_err_got_nack();
-            put_buffer((uint8_t *)gdbserver_state.last_packet, gdbserver_state.last_packet_len);
+            put_buffer(gdbserver_state.last_packet->data,
+                       gdbserver_state.last_packet->len);
         } else if (ch == '+') {
             trace_gdbstub_io_got_ack();
         } else {
             trace_gdbstub_io_got_unexpected(ch);
         }
 
-        if (ch == '+' || ch == '$')
-            gdbserver_state.last_packet_len = 0;
+        if (ch == '+' || ch == '$') {
+            g_byte_array_set_size(gdbserver_state.last_packet, 0);
+        }
         if (ch != '$')
             return;
     }
@@ -3209,7 +3212,7 @@ static int gdb_monitor_write(Chardev *chr, const uint8_t *buf, int len)
     const char *p = (const char *)buf;
     int max_sz;
 
-    max_sz = (sizeof(gdbserver_state.last_packet) - 2) / 2;
+    max_sz = (MAX_PACKET_LENGTH / 2) + 1;
     for (;;) {
         if (len <= max_sz) {
             gdb_monitor_output(p, len);
-- 
2.20.1


[PULL 27/28] gdbstub: do not split gdb_monitor_write payload
Posted by Alex Bennée 4 years, 1 month ago
From: Damien Hedde <damien.hedde@greensocs.com>

Since we can now send packets of arbitrary length:
simplify gdb_monitor_write() and send the whole payload
in one packet.

Suggested-by: Luc Michel <luc.michel@greensocs.com>
Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20191211160514.58373-3-damien.hedde@greensocs.com>
Message-Id: <20200316172155.971-28-alex.bennee@linaro.org>

diff --git a/gdbstub.c b/gdbstub.c
index a60ef5125eb..9ae148cd1ff 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -3200,28 +3200,11 @@ static void gdb_chr_event(void *opaque, QEMUChrEvent event)
     }
 }
 
-static void gdb_monitor_output(const char *msg, int len)
-{
-    g_autoptr(GString) buf = g_string_new("O");
-    memtohex(buf, (uint8_t *)msg, len);
-    put_packet(buf->str);
-}
-
 static int gdb_monitor_write(Chardev *chr, const uint8_t *buf, int len)
 {
-    const char *p = (const char *)buf;
-    int max_sz;
-
-    max_sz = (MAX_PACKET_LENGTH / 2) + 1;
-    for (;;) {
-        if (len <= max_sz) {
-            gdb_monitor_output(p, len);
-            break;
-        }
-        gdb_monitor_output(p, max_sz);
-        p += max_sz;
-        len -= max_sz;
-    }
+    g_autoptr(GString) hex_buf = g_string_new("O");
+    memtohex(hex_buf, buf, len);
+    put_packet(hex_buf->str);
     return len;
 }
 
-- 
2.20.1


[PULL 28/28] gdbstub: Fix single-step issue by confirming 'vContSupported+' feature to gdb
Posted by Alex Bennée 4 years, 1 month ago
From: Changbin Du <changbin.du@gmail.com>

Recently when debugging an arm32 system on qemu, I found sometimes the
single-step command (stepi) is not working. This can be reproduced by
below steps:
 1) start qemu-system-arm -s -S .. and wait for gdb connection.
 2) start gdb and connect to qemu. In my case, gdb gets a wrong value
    (0x60) for PC, which is an another bug.
 3) After connected, type 'stepi' and expect it will stop at next ins.

But, it has never stopped. This because:
 1) We doesn't report ‘vContSupported’ feature to gdb explicitly and gdb
    think we do not support it. In this case, gdb use a software breakpoint
    to emulate single-step.
 2) Since gdb gets a wrong initial value of PC, then gdb inserts a
    breakpoint to wrong place (PC+4).

Not only for the arm target, Philippe has also encountered this on MIPS.
Probably gdb has different assumption for different architectures.

Since we do support ‘vContSupported’ query command, so let's tell gdb that
we support it.

Before this change, gdb send below 'Z0' packet to implement single-step:
gdb_handle_packet: Z0,4,4

After this change, gdb send "vCont;s.." which is expected:
gdb_handle_packet: vCont?
put_packet: vCont;c;C;s;S
gdb_handle_packet: vCont;s:p1.1;c:p1.-1

Signed-off-by: Changbin Du <changbin.du@gmail.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20200221002559.6768-1-changbin.du@gmail.com>
[AJB: fix for static gdbstub]
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Luc Michel <luc.michel@greensocs.com>
Message-Id: <20200316172155.971-29-alex.bennee@linaro.org>

diff --git a/gdbstub.c b/gdbstub.c
index 9ae148cd1ff..013fb1ac0f1 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -2130,7 +2130,7 @@ static void handle_query_supported(GdbCmdContext *gdb_ctx, void *user_ctx)
         gdbserver_state.multiprocess = true;
     }
 
-    g_string_append(gdbserver_state.str_buf, ";multiprocess+");
+    g_string_append(gdbserver_state.str_buf, ";vContSupported+;multiprocess+");
     put_strbuf();
 }
 
-- 
2.20.1