mbox series

[v2,00/14] gdbstub refactor and SVE support

Message ID 20191130084602.10818-1-alex.bennee@linaro.org
Headers show
Series gdbstub refactor and SVE support | expand

Message

Alex Bennée Nov. 30, 2019, 8:45 a.m. UTC
Hi,

So this is the first fully working version of the SVE gdbstub
implementation. As before the first set of patches are mostly
re-factoring of the gdbstub register interface. I include a new patch
to default the SVE size to a more reasonable number for linux-user to
mirror what the Linux kernel actually does. Finally the XML that QEMU
generates is closer to that of GDB in terms of the nesting of the
union and keeping all registers intact. If you enable the maximum VQ
you still end up with pages of text if you examine the vector
registers.

Finally I've added a bit more test harness for running gdbstub tests
and included a basic smoke test for SVE itself. It's still not plumbed
into the main test harness yet though as there are complications
caused by the available version of gdb. However I hope I can hide
these in the run-test.py script later and just skip tests that we know
won't work.

Alex Bennée (14):
  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
  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
  tests/guest-debug: add a simple test runner
  tests/tcg: add a gdbstub testcase for SVE registers

 include/exec/gdbstub.h                |  49 +-
 include/hw/core/cpu.h                 |   2 +-
 target/alpha/cpu.h                    |   2 +-
 target/arm/cpu.h                      |  34 +-
 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                             | 892 +++++++++++++-------------
 hw/core/cpu.c                         |   2 +-
 target/alpha/gdbstub.c                |   2 +-
 target/arm/cpu64.c                    |   3 +
 target/arm/gdbstub.c                  | 169 ++++-
 target/arm/gdbstub64.c                |   2 +-
 target/arm/helper.c                   | 151 ++++-
 target/cris/gdbstub.c                 |   4 +-
 target/hppa/gdbstub.c                 |   2 +-
 target/i386/gdbstub.c                 |   2 +-
 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                |  18 +-
 target/s390x/gdbstub.c                |  30 +-
 target/sh4/gdbstub.c                  |   2 +-
 target/sparc/gdbstub.c                |   2 +-
 target/xtensa/gdbstub.c               |   2 +-
 tests/.gitignore                      |   1 +
 tests/guest-debug/run-test.py         |  57 ++
 tests/tcg/aarch64/gdbstub/test-sve.py |  75 +++
 45 files changed, 1038 insertions(+), 644 deletions(-)
 create mode 100755 tests/guest-debug/run-test.py
 create mode 100644 tests/tcg/aarch64/gdbstub/test-sve.py

Comments

no-reply@patchew.org Nov. 30, 2019, 9:33 a.m. UTC | #1
Patchew URL: https://patchew.org/QEMU/20191130084602.10818-1-alex.bennee@linaro.org/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [PATCH  v2 00/14] gdbstub refactor and SVE support
Type: series
Message-id: 20191130084602.10818-1-alex.bennee@linaro.org

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
b3409ec tests/tcg: add a gdbstub testcase for SVE registers
d39b296 tests/guest-debug: add a simple test runner
0a36c06 target/arm: generate xml description of our SVE registers
0536f07 target/arm: default SVE length to 64 bytes for linux-user
24fc7f5 target/arm: explicitly encode regnum in our XML
41a25c6 target/arm: prepare for multiple dynamic XMLs
7c1e4cd gdbstub: extend GByteArray to read register helpers
0918a59 target/m68k: use gdb_get_reg helpers
fd549cb target/arm: use gdb_get_reg helpers
588590d gdbstub: add helper for 128 bit registers
7368743 gdbstub: move mem_buf to GDBState and use GByteArray
9fd02d7 gdbstub: move str_buf to GDBState and use GString
27a63ee gdbstub: stop passing GDBState * around and use global
8bf5457 gdbstub: make GDBState static and have common init function

=== OUTPUT BEGIN ===
1/14 Checking commit 8bf54575f2b0 (gdbstub: make GDBState static and have common init function)
ERROR: braces {} are necessary for all arms of this statement
#128: FILE: gdbstub.c:2743:
+    if (!gdbserver_state.init)
[...]

ERROR: suspect code indent for conditional statements (2, 6)
#178: FILE: gdbstub.c:2962:
+  if (!gdbserver_state.init) {
       return;

ERROR: suspect code indent for conditional statements (2, 6)
#183: FILE: gdbstub.c:2966:
+  if (gdbserver_fd < 0 || gdbserver_state.fd < 0) {
       return;

total: 3 errors, 0 warnings, 384 lines checked

Patch 1/14 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

2/14 Checking commit 27a63ee5f12d (gdbstub: stop passing GDBState * around and use global)
WARNING: line over 80 characters
#742: FILE: gdbstub.c:1763:
+    if (target_memory_rw_debug(gdbserver_state.g_cpu, gdb_ctx->params[0].val_ull,

WARNING: line over 80 characters
#770: FILE: gdbstub.c:1786:
+    if (target_memory_rw_debug(gdbserver_state.g_cpu, gdb_ctx->params[0].val_ull,

WARNING: line over 80 characters
#997: FILE: gdbstub.c:2023:
+    gdbserver_state.query_cpu = gdb_next_attached_cpu(gdbserver_state.query_cpu);

ERROR: line over 90 characters
#1326: FILE: gdbstub.c:2817:
+            put_buffer((uint8_t *)gdbserver_state.last_packet, gdbserver_state.last_packet_len);

ERROR: space required before the open parenthesis '('
#1344: FILE: gdbstub.c:2836:
+        switch(gdbserver_state.state) {

ERROR: line over 90 characters
#1376: FILE: gdbstub.c:2859:
+            } else if (gdbserver_state.line_buf_index >= sizeof(gdbserver_state.line_buf) - 1) {

ERROR: line over 90 characters
#1394: FILE: gdbstub.c:2872:
+            } else if (gdbserver_state.line_buf_index >= sizeof(gdbserver_state.line_buf) - 1) {

WARNING: line over 80 characters
#1404: FILE: gdbstub.c:2878:
+                gdbserver_state.line_buf[gdbserver_state.line_buf_index++] = ch ^ 0x20;

ERROR: line over 90 characters
#1420: FILE: gdbstub.c:2895:
+                if (gdbserver_state.line_buf_index + repeat >= sizeof(gdbserver_state.line_buf) - 1) {

WARNING: line over 80 characters
#1438: FILE: gdbstub.c:2905:
+                    memset(gdbserver_state.line_buf + gdbserver_state.line_buf_index,

ERROR: line over 90 characters
#1439: FILE: gdbstub.c:2906:
+                           gdbserver_state.line_buf[gdbserver_state.line_buf_index - 1], repeat);

WARNING: line over 80 characters
#1474: FILE: gdbstub.c:2933:
+            if (gdbserver_state.line_csum != (gdbserver_state.line_sum & 0xff)) {

ERROR: line over 90 characters
#1475: FILE: gdbstub.c:2934:
+                trace_gdbstub_err_checksum_incorrect(gdbserver_state.line_sum, gdbserver_state.line_csum);

WARNING: line over 80 characters
#1488: FILE: gdbstub.c:2943:
+                gdbserver_state.state = gdb_handle_packet(s, gdbserver_state.line_buf);

ERROR: line over 90 characters
#1563: FILE: gdbstub.c:3305:
+        qsort(gdbserver_state.processes, gdbserver_state.process_num, sizeof(gdbserver_state.processes[0]), pid_order);

total: 8 errors, 7 warnings, 1478 lines checked

Patch 2/14 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

3/14 Checking commit 9fd02d7eba1e (gdbstub: move str_buf to GDBState and use GString)
WARNING: line over 80 characters
#149: FILE: gdbstub.c:1795:
+    memtohex(gdbserver_state.str_buf, gdb_ctx->mem_buf, gdb_ctx->params[1].val_ull);

WARNING: line over 80 characters
#322: FILE: gdbstub.c:2108:
+    g_string_printf(gdbserver_state.str_buf, "PacketSize=%x", MAX_PACKET_LENGTH);

total: 0 errors, 2 warnings, 422 lines checked

Patch 3/14 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
4/14 Checking commit 7368743b4a92 (gdbstub: move mem_buf to GDBState and use GByteArray)
5/14 Checking commit 588590d6a4ea (gdbstub: add helper for 128 bit registers)
6/14 Checking commit fd549cb833af (target/arm: use gdb_get_reg helpers)
ERROR: space required after that ',' (ctx:VxV)
#43: FILE: target/arm/helper.c:118:
+        return gdb_get_reg32(buf,vfp_get_fpcr(env));
                                 ^

total: 1 errors, 0 warnings, 28 lines checked

Patch 6/14 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

7/14 Checking commit 0918a59e7cc0 (target/m68k: use gdb_get_reg helpers)
8/14 Checking commit 7c1e4cda755c (gdbstub: extend GByteArray to read register helpers)
9/14 Checking commit 41a25c63b5c7 (target/arm: prepare for multiple dynamic XMLs)
ERROR: line over 90 characters
#127: FILE: target/arm/gdbstub.c:159:
+    cpu->dyn_sysreg_xml.data.cpregs.keys = g_new(uint32_t, g_hash_table_size(cpu->cp_regs));

total: 1 errors, 0 warnings, 136 lines checked

Patch 9/14 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

10/14 Checking commit 24fc7f5cae91 (target/arm: explicitly encode regnum in our XML)
11/14 Checking commit 0536f077e43f (target/arm: default SVE length to 64 bytes for linux-user)
12/14 Checking commit 0a36c0686afb (target/arm: generate xml description of our SVE registers)
ERROR: spaces required around that '/' (ctx:VxV)
#113: FILE: target/arm/gdbstub.c:229:
+    for (bits = 128; bits >= 8; bits = bits/2) {
                                            ^

WARNING: line over 80 characters
#118: FILE: target/arm/gdbstub.c:234:
+                g_string_append_printf(s, "<field name=\"%c\" type=\"vq%d%c%c\"/>",

ERROR: spaces required around that '/' (ctx:VxV)
#128: FILE: target/arm/gdbstub.c:244:
+    for (bits = 128; bits >= 8; bits = bits/2) {
                                            ^

WARNING: line over 80 characters
#319: FILE: target/arm/helper.c:7081:
+                                     arm_gen_dynamic_svereg_xml(cs, cs->gdb_num_regs),

total: 2 errors, 2 warnings, 306 lines checked

Patch 12/14 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

13/14 Checking commit d39b29603288 (tests/guest-debug: add a simple test runner)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#15: 
new file mode 100755

ERROR: line over 90 characters
#72: FILE: tests/guest-debug/run-test.py:53:
+    gdb_cmd = "%s %s -ex 'target remote localhost:1234' -x %s" % (args.gdb, args.binary, args.test)

total: 1 errors, 1 warnings, 57 lines checked

Patch 13/14 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

14/14 Checking commit b3409ec3f919 (tests/tcg: add a gdbstub testcase for SVE registers)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#35: 
new file mode 100644

total: 0 errors, 1 warnings, 82 lines checked

Patch 14/14 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20191130084602.10818-1-alex.bennee@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com