[PATCH 0/3] gdbstub: add support for switchable endianness

[PATCH 0/3] gdbstub: add support for switchable endianness

[Changbin Du]

To resolve the issue to debug switchable targets, this serias introduces
basic infrastructure for gdbstub and enable support for ARM and RISC-V
targets.

For example, now there is no problem to debug an big-enadian aarch64 target
on x86 host.

  $ qemu-system-aarch64 -gdb tcp::1234,endianness=big ...

Changbin Du (3):
  gdbstub: add basic infrastructure to support switchable endianness
  arm: gdbstub: add support for switchable endianness
  riscv: gdbstub: add support for switchable endianness

 configs/targets/aarch64-softmmu.mak |  1 +
 configs/targets/arm-softmmu.mak     |  1 +
 configs/targets/riscv32-softmmu.mak |  1 +
 configs/targets/riscv64-softmmu.mak |  1 +
 gdbstub.c                           | 11 +++++
 include/exec/gdbstub.h              | 72 +++++++++++++++++++++--------
 qemu-options.hx                     |  7 ++-
 softmmu/vl.c                        | 50 +++++++++++++++++++-
 target/arm/gdbstub.c                |  2 +-
 target/arm/gdbstub64.c              |  2 +-
 target/riscv/gdbstub.c              | 12 ++---
 11 files changed, 131 insertions(+), 29 deletions(-)

-- 
2.32.0

[PATCH 0/3] gdbstub: add support for switchable endianness

[Changbin Du]

To resolve the issue to debug switchable targets, this serias introduces
basic infrastructure for gdbstub and enable support for ARM and RISC-V
targets.

For example, now there is no problem to debug an big-enadian aarch64 target
on x86 host.

  $ qemu-system-aarch64 -gdb tcp::1234,endianness=big ...

Changbin Du (3):
  gdbstub: add basic infrastructure to support switchable endianness
  arm: gdbstub: add support for switchable endianness
  riscv: gdbstub: add support for switchable endianness

 configs/targets/aarch64-softmmu.mak |  1 +
 configs/targets/arm-softmmu.mak     |  1 +
 configs/targets/riscv32-softmmu.mak |  1 +
 configs/targets/riscv64-softmmu.mak |  1 +
 gdbstub.c                           | 11 +++++
 include/exec/gdbstub.h              | 72 +++++++++++++++++++++--------
 qemu-options.hx                     |  7 ++-
 softmmu/vl.c                        | 50 +++++++++++++++++++-
 target/arm/gdbstub.c                |  2 +-
 target/arm/gdbstub64.c              |  2 +-
 target/riscv/gdbstub.c              | 12 ++---
 11 files changed, 131 insertions(+), 29 deletions(-)

-- 
2.32.0

[PATCH 1/3] gdbstub: add basic infrastructure to support switchable endianness

[Changbin Du]

Some architectures (e.g. ARM versions 3 and above, RISC-V, PowerPC, Alpha,
MIPS, IA-64...) allow switchable endianness. Now our emulation code can
handle both endianness well, but the gdbstub can only handle one of them.

For example, it is problematic to debug a ARM big endian guest on x86 host.
This because the GDB remote protocol transfers values in target byte order
but qemu always take it as little endian on x86 host.

To support switchable endianness targets, this patch introduces:
  - a new sub-option 'endianness' for '-gdb'.
  - common interfaces to swap byte order according to host and target
    byte order.
  - a new configuration option TARGET_SWICHABLE_ENDIANNESS.

For example, to debug a arm64 big endian target, you could start qemu as
below:

  $ qemu-system-aarch64 -gdb tcp::1234,endianness=big ...

Latter we will add switchable endianness support for ARM and RISC-V
targets. For other switchable targets them can be supported in future.

Signed-off-by: Changbin Du <changbin.du@gmail.com>
---
 gdbstub.c              | 11 +++++++
 include/exec/gdbstub.h | 72 +++++++++++++++++++++++++++++++-----------
 qemu-options.hx        |  7 ++--
 softmmu/vl.c           | 50 ++++++++++++++++++++++++++++-
 4 files changed, 119 insertions(+), 21 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 52bde5bdc9..ec67d6a299 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -62,6 +62,17 @@
 static int phy_memory_mode;
 #endif
 
+#ifdef HOST_WORDS_BIGENDIAN
+const bool gdb_host_bigendian = true;
+#else
+const bool gdb_host_bigendian = false;
+#endif
+#ifdef TARGET_WORDS_BIGENDIAN
+bool gdb_target_bigendian = true;
+#else
+bool gdb_target_bigendian = false;
+#endif
+
 static inline int target_memory_rw_debug(CPUState *cpu, target_ulong addr,
                                          uint8_t *buf, int len, bool is_write)
 {
diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index a024a0350d..2c6f90fc28 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -84,9 +84,17 @@ void gdb_register_coprocessor(CPUState *cpu,
                               gdb_get_reg_cb get_reg, gdb_set_reg_cb set_reg,
                               int num_regs, const char *xml, int g_pos);
 
+extern const bool gdb_host_bigendian;
+extern bool gdb_target_bigendian;
+
+/* The GDB remote protocol transfers values in target byte order. */
+static inline bool gdb_bswap_needed(void)
+{
+    return gdb_host_bigendian != gdb_target_bigendian;
+}
+
 /*
- * The GDB remote protocol transfers values in target byte order. As
- * the gdbstub may be batching up several register values we always
+ * As the gdbstub may be batching up several register values we always
  * append to the array.
  */
 
@@ -98,21 +106,21 @@ static inline int gdb_get_reg8(GByteArray *buf, uint8_t val)
 
 static inline int gdb_get_reg16(GByteArray *buf, uint16_t val)
 {
-    uint16_t to_word = tswap16(val);
+    uint16_t to_word = gdb_bswap_needed() ? bswap16(val) : val;
     g_byte_array_append(buf, (uint8_t *) &to_word, 2);
     return 2;
 }
 
 static inline int gdb_get_reg32(GByteArray *buf, uint32_t val)
 {
-    uint32_t to_long = tswap32(val);
+    uint32_t to_long = gdb_bswap_needed() ? bswap32(val) : val;
     g_byte_array_append(buf, (uint8_t *) &to_long, 4);
     return 4;
 }
 
 static inline int gdb_get_reg64(GByteArray *buf, uint64_t val)
 {
-    uint64_t to_quad = tswap64(val);
+    uint64_t to_quad = gdb_bswap_needed() ? bswap64(val) : val;
     g_byte_array_append(buf, (uint8_t *) &to_quad, 8);
     return 8;
 }
@@ -121,17 +129,20 @@ static inline int gdb_get_reg128(GByteArray *buf, uint64_t val_hi,
                                  uint64_t val_lo)
 {
     uint64_t to_quad;
-#ifdef TARGET_WORDS_BIGENDIAN
-    to_quad = tswap64(val_hi);
-    g_byte_array_append(buf, (uint8_t *) &to_quad, 8);
-    to_quad = tswap64(val_lo);
-    g_byte_array_append(buf, (uint8_t *) &to_quad, 8);
-#else
-    to_quad = tswap64(val_lo);
-    g_byte_array_append(buf, (uint8_t *) &to_quad, 8);
-    to_quad = tswap64(val_hi);
-    g_byte_array_append(buf, (uint8_t *) &to_quad, 8);
-#endif
+
+    if (gdb_bswap_needed()) {
+        if (gdb_target_bigendian) {
+            to_quad = bswap64(val_hi);
+            g_byte_array_append(buf, (uint8_t *) &to_quad, 8);
+            to_quad = bswap64(val_lo);
+            g_byte_array_append(buf, (uint8_t *) &to_quad, 8);
+        } else {
+            to_quad = bswap64(val_lo);
+            g_byte_array_append(buf, (uint8_t *) &to_quad, 8);
+            to_quad = bswap64(val_hi);
+            g_byte_array_append(buf, (uint8_t *) &to_quad, 8);
+        }
+    }
     return 16;
 }
 
@@ -157,13 +168,38 @@ static inline uint8_t * gdb_get_reg_ptr(GByteArray *buf, int len)
     return buf->data + buf->len - len;
 }
 
+static inline uint8_t gdb_read_reg8(uint8_t *mem_buf)
+{
+    return *mem_buf;
+}
+
+static inline uint16_t gdb_read_reg16(uint8_t *mem_buf)
+{
+    uint16_t val = lduw_p(mem_buf);
+    return gdb_bswap_needed() ? bswap16(val) : val;
+}
+
+static inline uint32_t gdb_read_reg32(uint8_t *mem_buf)
+{
+    uint32_t val = ldl_p(mem_buf);
+    return gdb_bswap_needed() ? bswap32(val) : val;
+}
+
+static inline uint64_t gdb_read_reg64(uint8_t *mem_buf)
+{
+    uint64_t val = ldq_p(mem_buf);
+    return gdb_bswap_needed() ? bswap64(val) : val;
+}
+
 #if TARGET_LONG_BITS == 64
 #define gdb_get_regl(buf, val) gdb_get_reg64(buf, val)
-#define ldtul_p(addr) ldq_p(addr)
+#define gdb_read_regl(mem_buf) gdb_read_reg64(mem_buf)
 #else
 #define gdb_get_regl(buf, val) gdb_get_reg32(buf, val)
-#define ldtul_p(addr) ldl_p(addr)
+#define gdb_read_regl(mem_buf) gdb_read_reg32(mem_buf)
 #endif
+/* ldtul_p is deprecated */
+#define ldtul_p(mem_buf)       gdb_read_regl(mem_buf)
 
 #endif
 
diff --git a/qemu-options.hx b/qemu-options.hx
index 83aa59a920..779b861331 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3954,9 +3954,12 @@ SRST
 ERST
 
 DEF("gdb", HAS_ARG, QEMU_OPTION_gdb, \
-    "-gdb dev        accept gdb connection on 'dev'. (QEMU defaults to starting\n"
+    "-gdb [dev=]device[,endianness=default|little|big]\n"
+    "                accept gdb connection on 'dev'. (QEMU defaults to starting\n"
     "                the guest without waiting for gdb to connect; use -S too\n"
-    "                if you want it to not start execution.)\n",
+    "                if you want it to not start execution.) The 'endianness'\n"
+    "                specifies the endianness mode of the target which supports\n"
+    "                switchable endianness.\n",
     QEMU_ARCH_ALL)
 SRST
 ``-gdb dev``
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 5ca11e7469..dc57d518ab 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -1405,6 +1405,48 @@ static void qemu_create_default_devices(void)
     }
 }
 
+static QemuOptsList qemu_gdb_opts = {
+    .name = "gdb",
+    .head = QTAILQ_HEAD_INITIALIZER(qemu_gdb_opts.head),
+    .implied_opt_name = "dev",
+    .desc = {
+        {
+            .name = "dev",
+            .type = QEMU_OPT_STRING,
+        },
+        {
+            .name = "endianness",
+            .type = QEMU_OPT_STRING,
+        },
+        { /* end of list */ }
+    },
+};
+
+static void configure_gdb(QemuOpts *opts)
+{
+    const char *dev = qemu_opt_get(opts, "dev");
+    const char *endianness = qemu_opt_get(opts, "endianness");
+
+    if (dev) {
+        add_device_config(DEV_GDB, dev);
+    }
+
+    if (endianness && strcmp(endianness, "default")) {
+#ifdef TARGET_SWICHABLE_ENDIANNESS
+        if (!strcmp(endianness, "little")) {
+            gdb_target_bigendian = false;
+        } else if (!strcmp(endianness, "big")) {
+            gdb_target_bigendian = true;
+        } else {
+            error_report("unknown endianness %s", endianness);
+        }
+#else
+        error_report("endianness is not switchable for current target");
+        exit(1);
+#endif
+    }
+}
+
 static int serial_parse(const char *devname)
 {
     int index = num_serial_hds;
@@ -2761,6 +2803,7 @@ void qemu_init(int argc, char **argv, char **envp)
     qemu_add_opts(&qemu_semihosting_config_opts);
     qemu_add_opts(&qemu_fw_cfg_opts);
     qemu_add_opts(&qemu_action_opts);
+    qemu_add_opts(&qemu_gdb_opts);
     module_call_init(MODULE_INIT_OPTS);
 
     error_init(argv[0]);
@@ -3014,7 +3057,12 @@ void qemu_init(int argc, char **argv, char **envp)
                 add_device_config(DEV_GDB, "tcp::" DEFAULT_GDBSTUB_PORT);
                 break;
             case QEMU_OPTION_gdb:
-                add_device_config(DEV_GDB, optarg);
+                opts = qemu_opts_parse_noisily(qemu_find_opts("gdb"),
+                                               optarg, true);
+                if (!opts) {
+                    exit(EXIT_FAILURE);
+                }
+                configure_gdb(opts);
                 break;
             case QEMU_OPTION_L:
                 if (is_help_option(optarg)) {
-- 
2.32.0

[PATCH 1/3] gdbstub: add basic infrastructure to support switchable endianness

[Changbin Du]

Some architectures (e.g. ARM versions 3 and above, RISC-V, PowerPC, Alpha,
MIPS, IA-64...) allow switchable endianness. Now our emulation code can
handle both endianness well, but the gdbstub can only handle one of them.

For example, it is problematic to debug a ARM big endian guest on x86 host.
This because the GDB remote protocol transfers values in target byte order
but qemu always take it as little endian on x86 host.

To support switchable endianness targets, this patch introduces:
  - a new sub-option 'endianness' for '-gdb'.
  - common interfaces to swap byte order according to host and target
    byte order.
  - a new configuration option TARGET_SWICHABLE_ENDIANNESS.

For example, to debug a arm64 big endian target, you could start qemu as
below:

  $ qemu-system-aarch64 -gdb tcp::1234,endianness=big ...

Latter we will add switchable endianness support for ARM and RISC-V
targets. For other switchable targets them can be supported in future.

Signed-off-by: Changbin Du <changbin.du@gmail.com>
---
 gdbstub.c              | 11 +++++++
 include/exec/gdbstub.h | 72 +++++++++++++++++++++++++++++++-----------
 qemu-options.hx        |  7 ++--
 softmmu/vl.c           | 50 ++++++++++++++++++++++++++++-
 4 files changed, 119 insertions(+), 21 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 52bde5bdc9..ec67d6a299 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -62,6 +62,17 @@
 static int phy_memory_mode;
 #endif
 
+#ifdef HOST_WORDS_BIGENDIAN
+const bool gdb_host_bigendian = true;
+#else
+const bool gdb_host_bigendian = false;
+#endif
+#ifdef TARGET_WORDS_BIGENDIAN
+bool gdb_target_bigendian = true;
+#else
+bool gdb_target_bigendian = false;
+#endif
+
 static inline int target_memory_rw_debug(CPUState *cpu, target_ulong addr,
                                          uint8_t *buf, int len, bool is_write)
 {
diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index a024a0350d..2c6f90fc28 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -84,9 +84,17 @@ void gdb_register_coprocessor(CPUState *cpu,
                               gdb_get_reg_cb get_reg, gdb_set_reg_cb set_reg,
                               int num_regs, const char *xml, int g_pos);
 
+extern const bool gdb_host_bigendian;
+extern bool gdb_target_bigendian;
+
+/* The GDB remote protocol transfers values in target byte order. */
+static inline bool gdb_bswap_needed(void)
+{
+    return gdb_host_bigendian != gdb_target_bigendian;
+}
+
 /*
- * The GDB remote protocol transfers values in target byte order. As
- * the gdbstub may be batching up several register values we always
+ * As the gdbstub may be batching up several register values we always
  * append to the array.
  */
 
@@ -98,21 +106,21 @@ static inline int gdb_get_reg8(GByteArray *buf, uint8_t val)
 
 static inline int gdb_get_reg16(GByteArray *buf, uint16_t val)
 {
-    uint16_t to_word = tswap16(val);
+    uint16_t to_word = gdb_bswap_needed() ? bswap16(val) : val;
     g_byte_array_append(buf, (uint8_t *) &to_word, 2);
     return 2;
 }
 
 static inline int gdb_get_reg32(GByteArray *buf, uint32_t val)
 {
-    uint32_t to_long = tswap32(val);
+    uint32_t to_long = gdb_bswap_needed() ? bswap32(val) : val;
     g_byte_array_append(buf, (uint8_t *) &to_long, 4);
     return 4;
 }
 
 static inline int gdb_get_reg64(GByteArray *buf, uint64_t val)
 {
-    uint64_t to_quad = tswap64(val);
+    uint64_t to_quad = gdb_bswap_needed() ? bswap64(val) : val;
     g_byte_array_append(buf, (uint8_t *) &to_quad, 8);
     return 8;
 }
@@ -121,17 +129,20 @@ static inline int gdb_get_reg128(GByteArray *buf, uint64_t val_hi,
                                  uint64_t val_lo)
 {
     uint64_t to_quad;
-#ifdef TARGET_WORDS_BIGENDIAN
-    to_quad = tswap64(val_hi);
-    g_byte_array_append(buf, (uint8_t *) &to_quad, 8);
-    to_quad = tswap64(val_lo);
-    g_byte_array_append(buf, (uint8_t *) &to_quad, 8);
-#else
-    to_quad = tswap64(val_lo);
-    g_byte_array_append(buf, (uint8_t *) &to_quad, 8);
-    to_quad = tswap64(val_hi);
-    g_byte_array_append(buf, (uint8_t *) &to_quad, 8);
-#endif
+
+    if (gdb_bswap_needed()) {
+        if (gdb_target_bigendian) {
+            to_quad = bswap64(val_hi);
+            g_byte_array_append(buf, (uint8_t *) &to_quad, 8);
+            to_quad = bswap64(val_lo);
+            g_byte_array_append(buf, (uint8_t *) &to_quad, 8);
+        } else {
+            to_quad = bswap64(val_lo);
+            g_byte_array_append(buf, (uint8_t *) &to_quad, 8);
+            to_quad = bswap64(val_hi);
+            g_byte_array_append(buf, (uint8_t *) &to_quad, 8);
+        }
+    }
     return 16;
 }
 
@@ -157,13 +168,38 @@ static inline uint8_t * gdb_get_reg_ptr(GByteArray *buf, int len)
     return buf->data + buf->len - len;
 }
 
+static inline uint8_t gdb_read_reg8(uint8_t *mem_buf)
+{
+    return *mem_buf;
+}
+
+static inline uint16_t gdb_read_reg16(uint8_t *mem_buf)
+{
+    uint16_t val = lduw_p(mem_buf);
+    return gdb_bswap_needed() ? bswap16(val) : val;
+}
+
+static inline uint32_t gdb_read_reg32(uint8_t *mem_buf)
+{
+    uint32_t val = ldl_p(mem_buf);
+    return gdb_bswap_needed() ? bswap32(val) : val;
+}
+
+static inline uint64_t gdb_read_reg64(uint8_t *mem_buf)
+{
+    uint64_t val = ldq_p(mem_buf);
+    return gdb_bswap_needed() ? bswap64(val) : val;
+}
+
 #if TARGET_LONG_BITS == 64
 #define gdb_get_regl(buf, val) gdb_get_reg64(buf, val)
-#define ldtul_p(addr) ldq_p(addr)
+#define gdb_read_regl(mem_buf) gdb_read_reg64(mem_buf)
 #else
 #define gdb_get_regl(buf, val) gdb_get_reg32(buf, val)
-#define ldtul_p(addr) ldl_p(addr)
+#define gdb_read_regl(mem_buf) gdb_read_reg32(mem_buf)
 #endif
+/* ldtul_p is deprecated */
+#define ldtul_p(mem_buf)       gdb_read_regl(mem_buf)
 
 #endif
 
diff --git a/qemu-options.hx b/qemu-options.hx
index 83aa59a920..779b861331 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3954,9 +3954,12 @@ SRST
 ERST
 
 DEF("gdb", HAS_ARG, QEMU_OPTION_gdb, \
-    "-gdb dev        accept gdb connection on 'dev'. (QEMU defaults to starting\n"
+    "-gdb [dev=]device[,endianness=default|little|big]\n"
+    "                accept gdb connection on 'dev'. (QEMU defaults to starting\n"
     "                the guest without waiting for gdb to connect; use -S too\n"
-    "                if you want it to not start execution.)\n",
+    "                if you want it to not start execution.) The 'endianness'\n"
+    "                specifies the endianness mode of the target which supports\n"
+    "                switchable endianness.\n",
     QEMU_ARCH_ALL)
 SRST
 ``-gdb dev``
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 5ca11e7469..dc57d518ab 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -1405,6 +1405,48 @@ static void qemu_create_default_devices(void)
     }
 }
 
+static QemuOptsList qemu_gdb_opts = {
+    .name = "gdb",
+    .head = QTAILQ_HEAD_INITIALIZER(qemu_gdb_opts.head),
+    .implied_opt_name = "dev",
+    .desc = {
+        {
+            .name = "dev",
+            .type = QEMU_OPT_STRING,
+        },
+        {
+            .name = "endianness",
+            .type = QEMU_OPT_STRING,
+        },
+        { /* end of list */ }
+    },
+};
+
+static void configure_gdb(QemuOpts *opts)
+{
+    const char *dev = qemu_opt_get(opts, "dev");
+    const char *endianness = qemu_opt_get(opts, "endianness");
+
+    if (dev) {
+        add_device_config(DEV_GDB, dev);
+    }
+
+    if (endianness && strcmp(endianness, "default")) {
+#ifdef TARGET_SWICHABLE_ENDIANNESS
+        if (!strcmp(endianness, "little")) {
+            gdb_target_bigendian = false;
+        } else if (!strcmp(endianness, "big")) {
+            gdb_target_bigendian = true;
+        } else {
+            error_report("unknown endianness %s", endianness);
+        }
+#else
+        error_report("endianness is not switchable for current target");
+        exit(1);
+#endif
+    }
+}
+
 static int serial_parse(const char *devname)
 {
     int index = num_serial_hds;
@@ -2761,6 +2803,7 @@ void qemu_init(int argc, char **argv, char **envp)
     qemu_add_opts(&qemu_semihosting_config_opts);
     qemu_add_opts(&qemu_fw_cfg_opts);
     qemu_add_opts(&qemu_action_opts);
+    qemu_add_opts(&qemu_gdb_opts);
     module_call_init(MODULE_INIT_OPTS);
 
     error_init(argv[0]);
@@ -3014,7 +3057,12 @@ void qemu_init(int argc, char **argv, char **envp)
                 add_device_config(DEV_GDB, "tcp::" DEFAULT_GDBSTUB_PORT);
                 break;
             case QEMU_OPTION_gdb:
-                add_device_config(DEV_GDB, optarg);
+                opts = qemu_opts_parse_noisily(qemu_find_opts("gdb"),
+                                               optarg, true);
+                if (!opts) {
+                    exit(EXIT_FAILURE);
+                }
+                configure_gdb(opts);
                 break;
             case QEMU_OPTION_L:
                 if (is_help_option(optarg)) {
-- 
2.32.0

[PATCH 2/3] arm: gdbstub: add support for switchable endianness

[Changbin Du]

Apply new gdbstub interfaces we added previously to support both little
and big endian guest debugging for ARM. And enable the
TARGET_SWICHABLE_ENDIANNESS option.

Signed-off-by: Changbin Du <changbin.du@gmail.com>
---
 configs/targets/aarch64-softmmu.mak | 1 +
 configs/targets/arm-softmmu.mak     | 1 +
 target/arm/gdbstub.c                | 2 +-
 target/arm/gdbstub64.c              | 2 +-
 4 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/configs/targets/aarch64-softmmu.mak b/configs/targets/aarch64-softmmu.mak
index 7703127674..14e7f166a7 100644
--- a/configs/targets/aarch64-softmmu.mak
+++ b/configs/targets/aarch64-softmmu.mak
@@ -3,3 +3,4 @@ TARGET_BASE_ARCH=arm
 TARGET_SUPPORTS_MTTCG=y
 TARGET_XML_FILES= gdb-xml/aarch64-core.xml gdb-xml/aarch64-fpu.xml gdb-xml/arm-core.xml gdb-xml/arm-vfp.xml gdb-xml/arm-vfp3.xml gdb-xml/arm-neon.xml gdb-xml/arm-m-profile.xml
 TARGET_NEED_FDT=y
+TARGET_SWICHABLE_ENDIANNESS=y
diff --git a/configs/targets/arm-softmmu.mak b/configs/targets/arm-softmmu.mak
index 84a98f4818..5f40e858f9 100644
--- a/configs/targets/arm-softmmu.mak
+++ b/configs/targets/arm-softmmu.mak
@@ -2,3 +2,4 @@ TARGET_ARCH=arm
 TARGET_SUPPORTS_MTTCG=y
 TARGET_XML_FILES= gdb-xml/arm-core.xml gdb-xml/arm-vfp.xml gdb-xml/arm-vfp3.xml gdb-xml/arm-neon.xml gdb-xml/arm-m-profile.xml
 TARGET_NEED_FDT=y
+TARGET_SWICHABLE_ENDIANNESS=y
diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index 826601b341..188e82d938 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -74,7 +74,7 @@ int arm_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
     CPUARMState *env = &cpu->env;
     uint32_t tmp;
 
-    tmp = ldl_p(mem_buf);
+    tmp = gdb_read_reg32(mem_buf);
 
     /* Mask out low bit of PC to workaround gdb bugs.  This will probably
        cause problems if we ever implement the Jazelle DBX extensions.  */
diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
index 251539ef79..5358ad31b4 100644
--- a/target/arm/gdbstub64.c
+++ b/target/arm/gdbstub64.c
@@ -47,7 +47,7 @@ int aarch64_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
     CPUARMState *env = &cpu->env;
     uint64_t tmp;
 
-    tmp = ldq_p(mem_buf);
+    tmp = gdb_read_reg64(mem_buf);
 
     if (n < 31) {
         /* Core integer register.  */
-- 
2.32.0

[PATCH 2/3] arm: gdbstub: add support for switchable endianness

[Changbin Du]

Apply new gdbstub interfaces we added previously to support both little
and big endian guest debugging for ARM. And enable the
TARGET_SWICHABLE_ENDIANNESS option.

Signed-off-by: Changbin Du <changbin.du@gmail.com>
---
 configs/targets/aarch64-softmmu.mak | 1 +
 configs/targets/arm-softmmu.mak     | 1 +
 target/arm/gdbstub.c                | 2 +-
 target/arm/gdbstub64.c              | 2 +-
 4 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/configs/targets/aarch64-softmmu.mak b/configs/targets/aarch64-softmmu.mak
index 7703127674..14e7f166a7 100644
--- a/configs/targets/aarch64-softmmu.mak
+++ b/configs/targets/aarch64-softmmu.mak
@@ -3,3 +3,4 @@ TARGET_BASE_ARCH=arm
 TARGET_SUPPORTS_MTTCG=y
 TARGET_XML_FILES= gdb-xml/aarch64-core.xml gdb-xml/aarch64-fpu.xml gdb-xml/arm-core.xml gdb-xml/arm-vfp.xml gdb-xml/arm-vfp3.xml gdb-xml/arm-neon.xml gdb-xml/arm-m-profile.xml
 TARGET_NEED_FDT=y
+TARGET_SWICHABLE_ENDIANNESS=y
diff --git a/configs/targets/arm-softmmu.mak b/configs/targets/arm-softmmu.mak
index 84a98f4818..5f40e858f9 100644
--- a/configs/targets/arm-softmmu.mak
+++ b/configs/targets/arm-softmmu.mak
@@ -2,3 +2,4 @@ TARGET_ARCH=arm
 TARGET_SUPPORTS_MTTCG=y
 TARGET_XML_FILES= gdb-xml/arm-core.xml gdb-xml/arm-vfp.xml gdb-xml/arm-vfp3.xml gdb-xml/arm-neon.xml gdb-xml/arm-m-profile.xml
 TARGET_NEED_FDT=y
+TARGET_SWICHABLE_ENDIANNESS=y
diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index 826601b341..188e82d938 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -74,7 +74,7 @@ int arm_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
     CPUARMState *env = &cpu->env;
     uint32_t tmp;
 
-    tmp = ldl_p(mem_buf);
+    tmp = gdb_read_reg32(mem_buf);
 
     /* Mask out low bit of PC to workaround gdb bugs.  This will probably
        cause problems if we ever implement the Jazelle DBX extensions.  */
diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
index 251539ef79..5358ad31b4 100644
--- a/target/arm/gdbstub64.c
+++ b/target/arm/gdbstub64.c
@@ -47,7 +47,7 @@ int aarch64_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
     CPUARMState *env = &cpu->env;
     uint64_t tmp;
 
-    tmp = ldq_p(mem_buf);
+    tmp = gdb_read_reg64(mem_buf);
 
     if (n < 31) {
         /* Core integer register.  */
-- 
2.32.0

[PATCH 3/3] riscv: gdbstub: add support for switchable endianness

[Changbin Du]

Apply new gdbstub interfaces we added previously to support both little
and big endian guest debugging for RISC-V. And enable the
TARGET_SWICHABLE_ENDIANNESS option.

Signed-off-by: Changbin Du <changbin.du@gmail.com>
---
 configs/targets/riscv32-softmmu.mak |  1 +
 configs/targets/riscv64-softmmu.mak |  1 +
 target/riscv/gdbstub.c              | 12 ++++++------
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/configs/targets/riscv32-softmmu.mak b/configs/targets/riscv32-softmmu.mak
index d8b71cddcd..7f02e67c72 100644
--- a/configs/targets/riscv32-softmmu.mak
+++ b/configs/targets/riscv32-softmmu.mak
@@ -3,3 +3,4 @@ TARGET_BASE_ARCH=riscv
 TARGET_SUPPORTS_MTTCG=y
 TARGET_XML_FILES= gdb-xml/riscv-32bit-cpu.xml gdb-xml/riscv-32bit-fpu.xml gdb-xml/riscv-64bit-fpu.xml gdb-xml/riscv-32bit-virtual.xml
 TARGET_NEED_FDT=y
+TARGET_SWICHABLE_ENDIANNESS=y
diff --git a/configs/targets/riscv64-softmmu.mak b/configs/targets/riscv64-softmmu.mak
index 7c0e7eeb42..c3e812495c 100644
--- a/configs/targets/riscv64-softmmu.mak
+++ b/configs/targets/riscv64-softmmu.mak
@@ -3,3 +3,4 @@ TARGET_BASE_ARCH=riscv
 TARGET_SUPPORTS_MTTCG=y
 TARGET_XML_FILES= gdb-xml/riscv-64bit-cpu.xml gdb-xml/riscv-32bit-fpu.xml gdb-xml/riscv-64bit-fpu.xml gdb-xml/riscv-64bit-virtual.xml
 TARGET_NEED_FDT=y
+TARGET_SWICHABLE_ENDIANNESS=y
diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index a7a9c0b1fe..d639cea859 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -42,10 +42,10 @@ int riscv_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
         /* discard writes to x0 */
         return sizeof(target_ulong);
     } else if (n < 32) {
-        env->gpr[n] = ldtul_p(mem_buf);
+        env->gpr[n] = gdb_read_regl(mem_buf);
         return sizeof(target_ulong);
     } else if (n == 32) {
-        env->pc = ldtul_p(mem_buf);
+        env->pc = gdb_read_regl(mem_buf);
         return sizeof(target_ulong);
     }
     return 0;
@@ -81,11 +81,11 @@ static int riscv_gdb_get_fpu(CPURISCVState *env, GByteArray *buf, int n)
 static int riscv_gdb_set_fpu(CPURISCVState *env, uint8_t *mem_buf, int n)
 {
     if (n < 32) {
-        env->fpr[n] = ldq_p(mem_buf); /* always 64-bit */
+        env->fpr[n] = gdb_read_reg64(mem_buf); /* always 64-bit */
         return sizeof(uint64_t);
     /* there is hole between ft11 and fflags in fpu.xml */
     } else if (n < 36 && n > 32) {
-        target_ulong val = ldtul_p(mem_buf);
+        target_ulong val = gdb_read_regl(mem_buf);
         int result;
         /*
          * CSR_FFLAGS is at index 1 in csr_register, and gdb says it is FP
@@ -118,7 +118,7 @@ static int riscv_gdb_get_csr(CPURISCVState *env, GByteArray *buf, int n)
 static int riscv_gdb_set_csr(CPURISCVState *env, uint8_t *mem_buf, int n)
 {
     if (n < CSR_TABLE_SIZE) {
-        target_ulong val = ldtul_p(mem_buf);
+        target_ulong val = gdb_read_regl(mem_buf);
         int result;
 
         result = riscv_csrrw_debug(env, n, NULL, val, -1);
@@ -145,7 +145,7 @@ static int riscv_gdb_set_virtual(CPURISCVState *cs, uint8_t *mem_buf, int n)
 {
     if (n == 0) {
 #ifndef CONFIG_USER_ONLY
-        cs->priv = ldtul_p(mem_buf) & 0x3;
+        cs->priv = gdb_read_regl(mem_buf) & 0x3;
         if (cs->priv == PRV_H) {
             cs->priv = PRV_S;
         }
-- 
2.32.0

[PATCH 3/3] riscv: gdbstub: add support for switchable endianness

[Changbin Du]

Apply new gdbstub interfaces we added previously to support both little
and big endian guest debugging for RISC-V. And enable the
TARGET_SWICHABLE_ENDIANNESS option.

Signed-off-by: Changbin Du <changbin.du@gmail.com>
---
 configs/targets/riscv32-softmmu.mak |  1 +
 configs/targets/riscv64-softmmu.mak |  1 +
 target/riscv/gdbstub.c              | 12 ++++++------
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/configs/targets/riscv32-softmmu.mak b/configs/targets/riscv32-softmmu.mak
index d8b71cddcd..7f02e67c72 100644
--- a/configs/targets/riscv32-softmmu.mak
+++ b/configs/targets/riscv32-softmmu.mak
@@ -3,3 +3,4 @@ TARGET_BASE_ARCH=riscv
 TARGET_SUPPORTS_MTTCG=y
 TARGET_XML_FILES= gdb-xml/riscv-32bit-cpu.xml gdb-xml/riscv-32bit-fpu.xml gdb-xml/riscv-64bit-fpu.xml gdb-xml/riscv-32bit-virtual.xml
 TARGET_NEED_FDT=y
+TARGET_SWICHABLE_ENDIANNESS=y
diff --git a/configs/targets/riscv64-softmmu.mak b/configs/targets/riscv64-softmmu.mak
index 7c0e7eeb42..c3e812495c 100644
--- a/configs/targets/riscv64-softmmu.mak
+++ b/configs/targets/riscv64-softmmu.mak
@@ -3,3 +3,4 @@ TARGET_BASE_ARCH=riscv
 TARGET_SUPPORTS_MTTCG=y
 TARGET_XML_FILES= gdb-xml/riscv-64bit-cpu.xml gdb-xml/riscv-32bit-fpu.xml gdb-xml/riscv-64bit-fpu.xml gdb-xml/riscv-64bit-virtual.xml
 TARGET_NEED_FDT=y
+TARGET_SWICHABLE_ENDIANNESS=y
diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index a7a9c0b1fe..d639cea859 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -42,10 +42,10 @@ int riscv_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
         /* discard writes to x0 */
         return sizeof(target_ulong);
     } else if (n < 32) {
-        env->gpr[n] = ldtul_p(mem_buf);
+        env->gpr[n] = gdb_read_regl(mem_buf);
         return sizeof(target_ulong);
     } else if (n == 32) {
-        env->pc = ldtul_p(mem_buf);
+        env->pc = gdb_read_regl(mem_buf);
         return sizeof(target_ulong);
     }
     return 0;
@@ -81,11 +81,11 @@ static int riscv_gdb_get_fpu(CPURISCVState *env, GByteArray *buf, int n)
 static int riscv_gdb_set_fpu(CPURISCVState *env, uint8_t *mem_buf, int n)
 {
     if (n < 32) {
-        env->fpr[n] = ldq_p(mem_buf); /* always 64-bit */
+        env->fpr[n] = gdb_read_reg64(mem_buf); /* always 64-bit */
         return sizeof(uint64_t);
     /* there is hole between ft11 and fflags in fpu.xml */
     } else if (n < 36 && n > 32) {
-        target_ulong val = ldtul_p(mem_buf);
+        target_ulong val = gdb_read_regl(mem_buf);
         int result;
         /*
          * CSR_FFLAGS is at index 1 in csr_register, and gdb says it is FP
@@ -118,7 +118,7 @@ static int riscv_gdb_get_csr(CPURISCVState *env, GByteArray *buf, int n)
 static int riscv_gdb_set_csr(CPURISCVState *env, uint8_t *mem_buf, int n)
 {
     if (n < CSR_TABLE_SIZE) {
-        target_ulong val = ldtul_p(mem_buf);
+        target_ulong val = gdb_read_regl(mem_buf);
         int result;
 
         result = riscv_csrrw_debug(env, n, NULL, val, -1);
@@ -145,7 +145,7 @@ static int riscv_gdb_set_virtual(CPURISCVState *cs, uint8_t *mem_buf, int n)
 {
     if (n == 0) {
 #ifndef CONFIG_USER_ONLY
-        cs->priv = ldtul_p(mem_buf) & 0x3;
+        cs->priv = gdb_read_regl(mem_buf) & 0x3;
         if (cs->priv == PRV_H) {
             cs->priv = PRV_S;
         }
-- 
2.32.0

Re: [PATCH 0/3] gdbstub: add support for switchable endianness

[Philippe Mathieu-Daudé]

On 8/23/21 4:20 PM, Changbin Du wrote:
> To resolve the issue to debug switchable targets, this serias introduces
> basic infrastructure for gdbstub and enable support for ARM and RISC-V
> targets.
> 
> For example, now there is no problem to debug an big-enadian aarch64 target
> on x86 host.
> 
>   $ qemu-system-aarch64 -gdb tcp::1234,endianness=big ...

I don't understand why you need all that.
Maybe you aren't using gdb-multiarch?

You can install it or start it via QEMU Debian Docker image:

$ docker run -it --rm -v /tmp:/tmp -u $UID --network=host \
  registry.gitlab.com/qemu-project/qemu/qemu/debian10 \
  gdb-multiarch -q \
    --ex 'set architecture aarch64' \
    --ex 'set endian big'
The target architecture is assumed to be aarch64
The target is assumed to be big endian
(gdb) target remote 172.17.0.1:1234
(gdb)

Re: [PATCH 0/3] gdbstub: add support for switchable endianness

[Philippe Mathieu-Daudé]

On 8/23/21 4:20 PM, Changbin Du wrote:
> To resolve the issue to debug switchable targets, this serias introduces
> basic infrastructure for gdbstub and enable support for ARM and RISC-V
> targets.
> 
> For example, now there is no problem to debug an big-enadian aarch64 target
> on x86 host.
> 
>   $ qemu-system-aarch64 -gdb tcp::1234,endianness=big ...

I don't understand why you need all that.
Maybe you aren't using gdb-multiarch?

You can install it or start it via QEMU Debian Docker image:

$ docker run -it --rm -v /tmp:/tmp -u $UID --network=host \
  registry.gitlab.com/qemu-project/qemu/qemu/debian10 \
  gdb-multiarch -q \
    --ex 'set architecture aarch64' \
    --ex 'set endian big'
The target architecture is assumed to be aarch64
The target is assumed to be big endian
(gdb) target remote 172.17.0.1:1234
(gdb)

Re: [PATCH 0/3] gdbstub: add support for switchable endianness

[Peter Maydell]

On Mon, 23 Aug 2021 at 15:20, Changbin Du <changbin.du@gmail.com> wrote:
>
> To resolve the issue to debug switchable targets, this serias introduces
> basic infrastructure for gdbstub and enable support for ARM and RISC-V
> targets.

As I understand it, fixing this problem requires support
from gdb, not merely changes to QEMU. You would need to be
able to have QEMU tell gdb "the guest's endianness is now
little/big" and have gdb cope with that change.

> For example, now there is no problem to debug an big-enadian aarch64 target
> on x86 host.
>
>   $ qemu-system-aarch64 -gdb tcp::1234,endianness=big ...

I don't feel like this is the right approach. QEMU already
knows the endianness of the guest at any point.

thanks
-- PMM

Re: [PATCH 0/3] gdbstub: add support for switchable endianness

[Peter Maydell]

On Mon, 23 Aug 2021 at 15:20, Changbin Du <changbin.du@gmail.com> wrote:
>
> To resolve the issue to debug switchable targets, this serias introduces
> basic infrastructure for gdbstub and enable support for ARM and RISC-V
> targets.

As I understand it, fixing this problem requires support
from gdb, not merely changes to QEMU. You would need to be
able to have QEMU tell gdb "the guest's endianness is now
little/big" and have gdb cope with that change.

> For example, now there is no problem to debug an big-enadian aarch64 target
> on x86 host.
>
>   $ qemu-system-aarch64 -gdb tcp::1234,endianness=big ...

I don't feel like this is the right approach. QEMU already
knows the endianness of the guest at any point.

thanks
-- PMM

Re: [PATCH 0/3] gdbstub: add support for switchable endianness

[Peter Maydell]

On Mon, 23 Aug 2021 at 16:21, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> On 8/23/21 4:20 PM, Changbin Du wrote:
> > To resolve the issue to debug switchable targets, this serias introduces
> > basic infrastructure for gdbstub and enable support for ARM and RISC-V
> > targets.
> >
> > For example, now there is no problem to debug an big-enadian aarch64 target
> > on x86 host.
> >
> >   $ qemu-system-aarch64 -gdb tcp::1234,endianness=big ...
>
> I don't understand why you need all that.
> Maybe you aren't using gdb-multiarch?
>
> You can install it or start it via QEMU Debian Docker image:
>
> $ docker run -it --rm -v /tmp:/tmp -u $UID --network=host \
>   registry.gitlab.com/qemu-project/qemu/qemu/debian10 \
>   gdb-multiarch -q \
>     --ex 'set architecture aarch64' \
>     --ex 'set endian big'
> The target architecture is assumed to be aarch64
> The target is assumed to be big endian
> (gdb) target remote 172.17.0.1:1234

I don't think that will help, because an AArch64 CPU (at least
in the boards we model) will always start up in little-endian,
and our gdbstub will always transfer register data etc in
little-endian order, because gdb cannot cope with a target that
isn't always the same endianness. Fixing this requires gdb
changes to be more capable of handling dynamic target changes
(this would also help with eg debugging across 32<->64 bit switches);
as I understand it that gdb work would be pretty significant,
and at least for aarch64 pretty much nobody cares about
big-endian, so nobody's got round to doing it yet.

Our target/ppc/gdbstub.c code takes a different tack: it
always sends register data in the same order the CPU is
currently in, which has a different set of cases when it
goes wrong.

thanks
-- PMM

Re: [PATCH 0/3] gdbstub: add support for switchable endianness

[Peter Maydell]

On Mon, 23 Aug 2021 at 16:21, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> On 8/23/21 4:20 PM, Changbin Du wrote:
> > To resolve the issue to debug switchable targets, this serias introduces
> > basic infrastructure for gdbstub and enable support for ARM and RISC-V
> > targets.
> >
> > For example, now there is no problem to debug an big-enadian aarch64 target
> > on x86 host.
> >
> >   $ qemu-system-aarch64 -gdb tcp::1234,endianness=big ...
>
> I don't understand why you need all that.
> Maybe you aren't using gdb-multiarch?
>
> You can install it or start it via QEMU Debian Docker image:
>
> $ docker run -it --rm -v /tmp:/tmp -u $UID --network=host \
>   registry.gitlab.com/qemu-project/qemu/qemu/debian10 \
>   gdb-multiarch -q \
>     --ex 'set architecture aarch64' \
>     --ex 'set endian big'
> The target architecture is assumed to be aarch64
> The target is assumed to be big endian
> (gdb) target remote 172.17.0.1:1234

I don't think that will help, because an AArch64 CPU (at least
in the boards we model) will always start up in little-endian,
and our gdbstub will always transfer register data etc in
little-endian order, because gdb cannot cope with a target that
isn't always the same endianness. Fixing this requires gdb
changes to be more capable of handling dynamic target changes
(this would also help with eg debugging across 32<->64 bit switches);
as I understand it that gdb work would be pretty significant,
and at least for aarch64 pretty much nobody cares about
big-endian, so nobody's got round to doing it yet.

Our target/ppc/gdbstub.c code takes a different tack: it
always sends register data in the same order the CPU is
currently in, which has a different set of cases when it
goes wrong.

thanks
-- PMM

Re: [PATCH 0/3] gdbstub: add support for switchable endianness

[Philippe Mathieu-Daudé]

On 8/23/21 5:30 PM, Peter Maydell wrote:
> On Mon, 23 Aug 2021 at 16:21, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>
>> On 8/23/21 4:20 PM, Changbin Du wrote:
>>> To resolve the issue to debug switchable targets, this serias introduces
>>> basic infrastructure for gdbstub and enable support for ARM and RISC-V
>>> targets.
>>>
>>> For example, now there is no problem to debug an big-enadian aarch64 target
>>> on x86 host.
>>>
>>>   $ qemu-system-aarch64 -gdb tcp::1234,endianness=big ...
>>
>> I don't understand why you need all that.
>> Maybe you aren't using gdb-multiarch?
>>
>> You can install it or start it via QEMU Debian Docker image:
>>
>> $ docker run -it --rm -v /tmp:/tmp -u $UID --network=host \
>>   registry.gitlab.com/qemu-project/qemu/qemu/debian10 \
>>   gdb-multiarch -q \
>>     --ex 'set architecture aarch64' \
>>     --ex 'set endian big'
>> The target architecture is assumed to be aarch64
>> The target is assumed to be big endian
>> (gdb) target remote 172.17.0.1:1234
> 
> I don't think that will help, because an AArch64 CPU (at least
> in the boards we model) will always start up in little-endian,
> and our gdbstub will always transfer register data etc in
> little-endian order, because gdb cannot cope with a target that
> isn't always the same endianness. Fixing this requires gdb
> changes to be more capable of handling dynamic target changes
> (this would also help with eg debugging across 32<->64 bit switches);
> as I understand it that gdb work would be pretty significant,
> and at least for aarch64 pretty much nobody cares about
> big-endian, so nobody's got round to doing it yet.
> 
> Our target/ppc/gdbstub.c code takes a different tack: it
> always sends register data in the same order the CPU is
> currently in, which has a different set of cases when it
> goes wrong.

I remember having tested the 'setend be' instruction (from
https://github.com/pcrost/arm-be-test) 3 years ago using
it. Connected as little endian, set breakpoint, on BP hit
disconnect, set big-endian, reconnect, keep single-stepping
(in the same gdb session).

I doubt anything changed since.

Re: [PATCH 0/3] gdbstub: add support for switchable endianness

[Philippe Mathieu-Daudé]

On 8/23/21 5:30 PM, Peter Maydell wrote:
> On Mon, 23 Aug 2021 at 16:21, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>
>> On 8/23/21 4:20 PM, Changbin Du wrote:
>>> To resolve the issue to debug switchable targets, this serias introduces
>>> basic infrastructure for gdbstub and enable support for ARM and RISC-V
>>> targets.
>>>
>>> For example, now there is no problem to debug an big-enadian aarch64 target
>>> on x86 host.
>>>
>>>   $ qemu-system-aarch64 -gdb tcp::1234,endianness=big ...
>>
>> I don't understand why you need all that.
>> Maybe you aren't using gdb-multiarch?
>>
>> You can install it or start it via QEMU Debian Docker image:
>>
>> $ docker run -it --rm -v /tmp:/tmp -u $UID --network=host \
>>   registry.gitlab.com/qemu-project/qemu/qemu/debian10 \
>>   gdb-multiarch -q \
>>     --ex 'set architecture aarch64' \
>>     --ex 'set endian big'
>> The target architecture is assumed to be aarch64
>> The target is assumed to be big endian
>> (gdb) target remote 172.17.0.1:1234
> 
> I don't think that will help, because an AArch64 CPU (at least
> in the boards we model) will always start up in little-endian,
> and our gdbstub will always transfer register data etc in
> little-endian order, because gdb cannot cope with a target that
> isn't always the same endianness. Fixing this requires gdb
> changes to be more capable of handling dynamic target changes
> (this would also help with eg debugging across 32<->64 bit switches);
> as I understand it that gdb work would be pretty significant,
> and at least for aarch64 pretty much nobody cares about
> big-endian, so nobody's got round to doing it yet.
> 
> Our target/ppc/gdbstub.c code takes a different tack: it
> always sends register data in the same order the CPU is
> currently in, which has a different set of cases when it
> goes wrong.

I remember having tested the 'setend be' instruction (from
https://github.com/pcrost/arm-be-test) 3 years ago using
it. Connected as little endian, set breakpoint, on BP hit
disconnect, set big-endian, reconnect, keep single-stepping
(in the same gdb session).

I doubt anything changed since.

Re: [PATCH 0/3] gdbstub: add support for switchable endianness

[Changbin Du]

On Mon, Aug 23, 2021 at 05:21:07PM +0200, Philippe Mathieu-Daudé wrote:
> On 8/23/21 4:20 PM, Changbin Du wrote:
> > To resolve the issue to debug switchable targets, this serias introduces
> > basic infrastructure for gdbstub and enable support for ARM and RISC-V
> > targets.
> > 
> > For example, now there is no problem to debug an big-enadian aarch64 target
> > on x86 host.
> > 
> >   $ qemu-system-aarch64 -gdb tcp::1234,endianness=big ...
> 
> I don't understand why you need all that.
> Maybe you aren't using gdb-multiarch?
>
Nope, my gdb support all architectures.

> You can install it or start it via QEMU Debian Docker image:
> 
> $ docker run -it --rm -v /tmp:/tmp -u $UID --network=host \
>   registry.gitlab.com/qemu-project/qemu/qemu/debian10 \
>   gdb-multiarch -q \
>     --ex 'set architecture aarch64' \
>     --ex 'set endian big'
> The target architecture is assumed to be aarch64
> The target is assumed to be big endian
> (gdb) target remote 172.17.0.1:1234
> (gdb)
>
The gdb has no problem to read endianness and arch info from elf. The problem
is how qemu gdbstub handles the byte order it received.

Now let's try to debug a big-enadian aarch64 linux kernel.

1) start qemu with '-gdb tcp::1234'

$ gdb-multiarch vmlinux
(gdb) target remote :1234
Remote debugging using :1234
0x0000004000000000 in ?? ()
=> 0x0000004000000000:	Cannot access memory at address 0x4000000000
(gdb) ni
Cannot access memory at address 0x4000000000
(gdb) show architecture 
The target architecture is set to "auto" (currently "aarch64").
(gdb) show endian 
The target endianness is set automatically (currently big endian).

You see it an't work, not to mention adding breakpoints.

2) start qemu with '-gdb tcp::1234,endianness=big'

$ gdb-multiarch vmlinux
(gdb) target remote :1234
Remote debugging using :1234
0x0000000040000000 in ?? ()
=> 0x0000000040000000:	c0 00 00 58	ldr	x0, 0x40000018
(gdb) ni
0x0000000040000004 in ?? ()
=> 0x0000000040000004:	e1 03 1f aa	mov	x1, xzr
(gdb) b start_kernel
Breakpoint 1 at 0xffff800011130ee8 (2 locations)
(gdb) c
Continuing.

Thread 1 hit Breakpoint 1, 0xffff800011130ee8 in start_kernel ()
=> 0xffff800011130ee8 <start_kernel+0>:	5f 24 03 d5	bti	c
(gdb) bt
#0  0xffff800011130ee8 in start_kernel ()
#1  0xffff8000111303c8 in __primary_switched () at arch/arm64/kernel/head.S:467
Backtrace stopped: previous frame identical to this frame (corrupt stack?)

okay, now it works fine.

-- 
Cheers,
Changbin Du

Re: [PATCH 0/3] gdbstub: add support for switchable endianness

[Changbin Du]

On Mon, Aug 23, 2021 at 05:21:07PM +0200, Philippe Mathieu-Daudé wrote:
> On 8/23/21 4:20 PM, Changbin Du wrote:
> > To resolve the issue to debug switchable targets, this serias introduces
> > basic infrastructure for gdbstub and enable support for ARM and RISC-V
> > targets.
> > 
> > For example, now there is no problem to debug an big-enadian aarch64 target
> > on x86 host.
> > 
> >   $ qemu-system-aarch64 -gdb tcp::1234,endianness=big ...
> 
> I don't understand why you need all that.
> Maybe you aren't using gdb-multiarch?
>
Nope, my gdb support all architectures.

> You can install it or start it via QEMU Debian Docker image:
> 
> $ docker run -it --rm -v /tmp:/tmp -u $UID --network=host \
>   registry.gitlab.com/qemu-project/qemu/qemu/debian10 \
>   gdb-multiarch -q \
>     --ex 'set architecture aarch64' \
>     --ex 'set endian big'
> The target architecture is assumed to be aarch64
> The target is assumed to be big endian
> (gdb) target remote 172.17.0.1:1234
> (gdb)
>
The gdb has no problem to read endianness and arch info from elf. The problem
is how qemu gdbstub handles the byte order it received.

Now let's try to debug a big-enadian aarch64 linux kernel.

1) start qemu with '-gdb tcp::1234'

$ gdb-multiarch vmlinux
(gdb) target remote :1234
Remote debugging using :1234
0x0000004000000000 in ?? ()
=> 0x0000004000000000:	Cannot access memory at address 0x4000000000
(gdb) ni
Cannot access memory at address 0x4000000000
(gdb) show architecture 
The target architecture is set to "auto" (currently "aarch64").
(gdb) show endian 
The target endianness is set automatically (currently big endian).

You see it an't work, not to mention adding breakpoints.

2) start qemu with '-gdb tcp::1234,endianness=big'

$ gdb-multiarch vmlinux
(gdb) target remote :1234
Remote debugging using :1234
0x0000000040000000 in ?? ()
=> 0x0000000040000000:	c0 00 00 58	ldr	x0, 0x40000018
(gdb) ni
0x0000000040000004 in ?? ()
=> 0x0000000040000004:	e1 03 1f aa	mov	x1, xzr
(gdb) b start_kernel
Breakpoint 1 at 0xffff800011130ee8 (2 locations)
(gdb) c
Continuing.

Thread 1 hit Breakpoint 1, 0xffff800011130ee8 in start_kernel ()
=> 0xffff800011130ee8 <start_kernel+0>:	5f 24 03 d5	bti	c
(gdb) bt
#0  0xffff800011130ee8 in start_kernel ()
#1  0xffff8000111303c8 in __primary_switched () at arch/arm64/kernel/head.S:467
Backtrace stopped: previous frame identical to this frame (corrupt stack?)

okay, now it works fine.

-- 
Cheers,
Changbin Du

Re: [PATCH 0/3] gdbstub: add support for switchable endianness

[Changbin Du]

On Mon, Aug 23, 2021 at 04:30:05PM +0100, Peter Maydell wrote:
> On Mon, 23 Aug 2021 at 16:21, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> >
> > On 8/23/21 4:20 PM, Changbin Du wrote:
> > > To resolve the issue to debug switchable targets, this serias introduces
> > > basic infrastructure for gdbstub and enable support for ARM and RISC-V
> > > targets.
> > >
> > > For example, now there is no problem to debug an big-enadian aarch64 target
> > > on x86 host.
> > >
> > >   $ qemu-system-aarch64 -gdb tcp::1234,endianness=big ...
> >
> > I don't understand why you need all that.
> > Maybe you aren't using gdb-multiarch?
> >
> > You can install it or start it via QEMU Debian Docker image:
> >
> > $ docker run -it --rm -v /tmp:/tmp -u $UID --network=host \
> >   registry.gitlab.com/qemu-project/qemu/qemu/debian10 \
> >   gdb-multiarch -q \
> >     --ex 'set architecture aarch64' \
> >     --ex 'set endian big'
> > The target architecture is assumed to be aarch64
> > The target is assumed to be big endian
> > (gdb) target remote 172.17.0.1:1234
> 
> I don't think that will help, because an AArch64 CPU (at least
> in the boards we model) will always start up in little-endian,
> and our gdbstub will always transfer register data etc in
> little-endian order, because gdb cannot cope with a target that
> isn't always the same endianness. Fixing this requires gdb
Yes, that's the problem.

> changes to be more capable of handling dynamic target changes
> (this would also help with eg debugging across 32<->64 bit switches);
> as I understand it that gdb work would be pretty significant,
> and at least for aarch64 pretty much nobody cares about
> big-endian, so nobody's got round to doing it yet.
> 
Mostly we do not care dynamic target changes because nearly all OS will setup
endianness mode by its first instruction. And dynamic changes for gdb is hard
since the byte order of debugging info in elf is fixed. And currently the GDB
remote protocol does not support querying endianness info from remote.

So usually we needn't change byte order during a debug session, but we just want
the qemu gdbstub can send data in and handle data it received in right byte order.
This patch does this work with the help of users via the option 'endianness='.

> Our target/ppc/gdbstub.c code takes a different tack: it
> always sends register data in the same order the CPU is
> currently in, which has a different set of cases when it
> goes wrong.
>
Yes, I tried to do this before. But as I said above GDB unable to handle dynamic
target changing. Maybe we can take this way as 'endianness=default'? Anyway,
this requires each target provides a interface to determine the current byte
order.

> thanks
> -- PMM

-- 
Cheers,
Changbin Du

Re: [PATCH 0/3] gdbstub: add support for switchable endianness

[Changbin Du]

On Mon, Aug 23, 2021 at 04:30:05PM +0100, Peter Maydell wrote:
> On Mon, 23 Aug 2021 at 16:21, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> >
> > On 8/23/21 4:20 PM, Changbin Du wrote:
> > > To resolve the issue to debug switchable targets, this serias introduces
> > > basic infrastructure for gdbstub and enable support for ARM and RISC-V
> > > targets.
> > >
> > > For example, now there is no problem to debug an big-enadian aarch64 target
> > > on x86 host.
> > >
> > >   $ qemu-system-aarch64 -gdb tcp::1234,endianness=big ...
> >
> > I don't understand why you need all that.
> > Maybe you aren't using gdb-multiarch?
> >
> > You can install it or start it via QEMU Debian Docker image:
> >
> > $ docker run -it --rm -v /tmp:/tmp -u $UID --network=host \
> >   registry.gitlab.com/qemu-project/qemu/qemu/debian10 \
> >   gdb-multiarch -q \
> >     --ex 'set architecture aarch64' \
> >     --ex 'set endian big'
> > The target architecture is assumed to be aarch64
> > The target is assumed to be big endian
> > (gdb) target remote 172.17.0.1:1234
> 
> I don't think that will help, because an AArch64 CPU (at least
> in the boards we model) will always start up in little-endian,
> and our gdbstub will always transfer register data etc in
> little-endian order, because gdb cannot cope with a target that
> isn't always the same endianness. Fixing this requires gdb
Yes, that's the problem.

> changes to be more capable of handling dynamic target changes
> (this would also help with eg debugging across 32<->64 bit switches);
> as I understand it that gdb work would be pretty significant,
> and at least for aarch64 pretty much nobody cares about
> big-endian, so nobody's got round to doing it yet.
> 
Mostly we do not care dynamic target changes because nearly all OS will setup
endianness mode by its first instruction. And dynamic changes for gdb is hard
since the byte order of debugging info in elf is fixed. And currently the GDB
remote protocol does not support querying endianness info from remote.

So usually we needn't change byte order during a debug session, but we just want
the qemu gdbstub can send data in and handle data it received in right byte order.
This patch does this work with the help of users via the option 'endianness='.

> Our target/ppc/gdbstub.c code takes a different tack: it
> always sends register data in the same order the CPU is
> currently in, which has a different set of cases when it
> goes wrong.
>
Yes, I tried to do this before. But as I said above GDB unable to handle dynamic
target changing. Maybe we can take this way as 'endianness=default'? Anyway,
this requires each target provides a interface to determine the current byte
order.

> thanks
> -- PMM

-- 
Cheers,
Changbin Du

Re: [PATCH 0/3] gdbstub: add support for switchable endianness

[Peter Maydell]

On Tue, 24 Aug 2021 at 00:05, Changbin Du <changbin.du@gmail.com> wrote:
>
> On Mon, Aug 23, 2021 at 04:30:05PM +0100, Peter Maydell wrote:
> > changes to be more capable of handling dynamic target changes
> > (this would also help with eg debugging across 32<->64 bit switches);
> > as I understand it that gdb work would be pretty significant,
> > and at least for aarch64 pretty much nobody cares about
> > big-endian, so nobody's got round to doing it yet.
> >
> Mostly we do not care dynamic target changes because nearly all OS will setup
> endianness mode by its first instruction. And dynamic changes for gdb is hard
> since the byte order of debugging info in elf is fixed. And currently the GDB
> remote protocol does not support querying endianness info from remote.
>
> So usually we needn't change byte order during a debug session, but we just want
> the qemu gdbstub can send data in and handle data it received in right byte order.
> This patch does this work with the help of users via the option 'endianness='.

I'm not a huge fan of putting in workarounds that deal with the
problem for specific cases and require users to tweak options settings,
rather than solving the problem in a more general way that would
let it all Just Work for all cases.

-- PMM

Re: [PATCH 0/3] gdbstub: add support for switchable endianness

[Peter Maydell]

On Tue, 24 Aug 2021 at 00:05, Changbin Du <changbin.du@gmail.com> wrote:
>
> On Mon, Aug 23, 2021 at 04:30:05PM +0100, Peter Maydell wrote:
> > changes to be more capable of handling dynamic target changes
> > (this would also help with eg debugging across 32<->64 bit switches);
> > as I understand it that gdb work would be pretty significant,
> > and at least for aarch64 pretty much nobody cares about
> > big-endian, so nobody's got round to doing it yet.
> >
> Mostly we do not care dynamic target changes because nearly all OS will setup
> endianness mode by its first instruction. And dynamic changes for gdb is hard
> since the byte order of debugging info in elf is fixed. And currently the GDB
> remote protocol does not support querying endianness info from remote.
>
> So usually we needn't change byte order during a debug session, but we just want
> the qemu gdbstub can send data in and handle data it received in right byte order.
> This patch does this work with the help of users via the option 'endianness='.

I'm not a huge fan of putting in workarounds that deal with the
problem for specific cases and require users to tweak options settings,
rather than solving the problem in a more general way that would
let it all Just Work for all cases.

-- PMM

Re: [PATCH 0/3] gdbstub: add support for switchable endianness

[Changbin Du]

On Tue, Aug 24, 2021 at 10:11:14AM +0100, Peter Maydell wrote:
> On Tue, 24 Aug 2021 at 00:05, Changbin Du <changbin.du@gmail.com> wrote:
> >
> > On Mon, Aug 23, 2021 at 04:30:05PM +0100, Peter Maydell wrote:
> > > changes to be more capable of handling dynamic target changes
> > > (this would also help with eg debugging across 32<->64 bit switches);
> > > as I understand it that gdb work would be pretty significant,
> > > and at least for aarch64 pretty much nobody cares about
> > > big-endian, so nobody's got round to doing it yet.
> > >
> > Mostly we do not care dynamic target changes because nearly all OS will setup
> > endianness mode by its first instruction. And dynamic changes for gdb is hard
> > since the byte order of debugging info in elf is fixed. And currently the GDB
> > remote protocol does not support querying endianness info from remote.
> >
> > So usually we needn't change byte order during a debug session, but we just want
> > the qemu gdbstub can send data in and handle data it received in right byte order.
> > This patch does this work with the help of users via the option 'endianness='.
> 
> I'm not a huge fan of putting in workarounds that deal with the
> problem for specific cases and require users to tweak options settings,
> rather than solving the problem in a more general way that would
> let it all Just Work for all cases.
>
Probably we can add a new callback 'gdb_get_endianness' for CPUClass, and use
this callback to determine if bswap is needed every time we read/write cpu
registers. What's your thought?

> -- PMM

-- 
Cheers,
Changbin Du

Re: [PATCH 0/3] gdbstub: add support for switchable endianness

[Changbin Du]

On Tue, Aug 24, 2021 at 10:11:14AM +0100, Peter Maydell wrote:
> On Tue, 24 Aug 2021 at 00:05, Changbin Du <changbin.du@gmail.com> wrote:
> >
> > On Mon, Aug 23, 2021 at 04:30:05PM +0100, Peter Maydell wrote:
> > > changes to be more capable of handling dynamic target changes
> > > (this would also help with eg debugging across 32<->64 bit switches);
> > > as I understand it that gdb work would be pretty significant,
> > > and at least for aarch64 pretty much nobody cares about
> > > big-endian, so nobody's got round to doing it yet.
> > >
> > Mostly we do not care dynamic target changes because nearly all OS will setup
> > endianness mode by its first instruction. And dynamic changes for gdb is hard
> > since the byte order of debugging info in elf is fixed. And currently the GDB
> > remote protocol does not support querying endianness info from remote.
> >
> > So usually we needn't change byte order during a debug session, but we just want
> > the qemu gdbstub can send data in and handle data it received in right byte order.
> > This patch does this work with the help of users via the option 'endianness='.
> 
> I'm not a huge fan of putting in workarounds that deal with the
> problem for specific cases and require users to tweak options settings,
> rather than solving the problem in a more general way that would
> let it all Just Work for all cases.
>
Probably we can add a new callback 'gdb_get_endianness' for CPUClass, and use
this callback to determine if bswap is needed every time we read/write cpu
registers. What's your thought?

> -- PMM

-- 
Cheers,
Changbin Du

Re: [PATCH 0/3] gdbstub: add support for switchable endianness

[Peter Maydell]

On Fri, 27 Aug 2021 at 15:49, Changbin Du <changbin.du@gmail.com> wrote:
>
> On Tue, Aug 24, 2021 at 10:11:14AM +0100, Peter Maydell wrote:
> > On Tue, 24 Aug 2021 at 00:05, Changbin Du <changbin.du@gmail.com> wrote:
> > >
> > > On Mon, Aug 23, 2021 at 04:30:05PM +0100, Peter Maydell wrote:
> > > > changes to be more capable of handling dynamic target changes
> > > > (this would also help with eg debugging across 32<->64 bit switches);
> > > > as I understand it that gdb work would be pretty significant,
> > > > and at least for aarch64 pretty much nobody cares about
> > > > big-endian, so nobody's got round to doing it yet.
> > > >
> > > Mostly we do not care dynamic target changes because nearly all OS will setup
> > > endianness mode by its first instruction. And dynamic changes for gdb is hard
> > > since the byte order of debugging info in elf is fixed. And currently the GDB
> > > remote protocol does not support querying endianness info from remote.
> > >
> > > So usually we needn't change byte order during a debug session, but we just want
> > > the qemu gdbstub can send data in and handle data it received in right byte order.
> > > This patch does this work with the help of users via the option 'endianness='.
> >
> > I'm not a huge fan of putting in workarounds that deal with the
> > problem for specific cases and require users to tweak options settings,
> > rather than solving the problem in a more general way that would
> > let it all Just Work for all cases.
> >
> Probably we can add a new callback 'gdb_get_endianness' for CPUClass, and use
> this callback to determine if bswap is needed every time we read/write cpu
> registers. What's your thought?

I think that you need to start by talking to the gdb folks about
how debugging a dynamic endianness target should work. Fixing
this probably goes something like:
 * agree on design for how dynamic endianness, 32-64 mode changes,
   etc, should be handled by gdb
 * make gdb changes
 * document required gdbstub protocol enhancements (ie how the stub
   tells gdb about endianness changes, whether this changes how we
   send register information, memory data, etc)
 * implement those changes in QEMU

You seem to be trying to start with the final step, not the first one :-)

-- PMM

Re: [PATCH 0/3] gdbstub: add support for switchable endianness

[Peter Maydell]

On Fri, 27 Aug 2021 at 15:49, Changbin Du <changbin.du@gmail.com> wrote:
>
> On Tue, Aug 24, 2021 at 10:11:14AM +0100, Peter Maydell wrote:
> > On Tue, 24 Aug 2021 at 00:05, Changbin Du <changbin.du@gmail.com> wrote:
> > >
> > > On Mon, Aug 23, 2021 at 04:30:05PM +0100, Peter Maydell wrote:
> > > > changes to be more capable of handling dynamic target changes
> > > > (this would also help with eg debugging across 32<->64 bit switches);
> > > > as I understand it that gdb work would be pretty significant,
> > > > and at least for aarch64 pretty much nobody cares about
> > > > big-endian, so nobody's got round to doing it yet.
> > > >
> > > Mostly we do not care dynamic target changes because nearly all OS will setup
> > > endianness mode by its first instruction. And dynamic changes for gdb is hard
> > > since the byte order of debugging info in elf is fixed. And currently the GDB
> > > remote protocol does not support querying endianness info from remote.
> > >
> > > So usually we needn't change byte order during a debug session, but we just want
> > > the qemu gdbstub can send data in and handle data it received in right byte order.
> > > This patch does this work with the help of users via the option 'endianness='.
> >
> > I'm not a huge fan of putting in workarounds that deal with the
> > problem for specific cases and require users to tweak options settings,
> > rather than solving the problem in a more general way that would
> > let it all Just Work for all cases.
> >
> Probably we can add a new callback 'gdb_get_endianness' for CPUClass, and use
> this callback to determine if bswap is needed every time we read/write cpu
> registers. What's your thought?

I think that you need to start by talking to the gdb folks about
how debugging a dynamic endianness target should work. Fixing
this probably goes something like:
 * agree on design for how dynamic endianness, 32-64 mode changes,
   etc, should be handled by gdb
 * make gdb changes
 * document required gdbstub protocol enhancements (ie how the stub
   tells gdb about endianness changes, whether this changes how we
   send register information, memory data, etc)
 * implement those changes in QEMU

You seem to be trying to start with the final step, not the first one :-)

-- PMM