qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] target/arm: Always build Aarch64 gdbstub helpers
@ 2024-06-19 12:49 Philippe Mathieu-Daudé
  2024-06-19 12:49 ` [PATCH 1/2] target/arm: Merge gdbstub64.c within gdbstub.c Philippe Mathieu-Daudé
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-06-19 12:49 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Gustavo Romero, qemu-arm, Anton Johansson,
	Philippe =?unknown-8bit?q?Mathieu-Daud=C3=A9?=

Merge gdbstub64.c in gdbstub.c and remove uses of
target specific TARGET_AARCH64 definition.
Small step toward single ARM/Aarch64 binary.

Philippe Mathieu-Daudé (2):
  target/arm: Merge gdbstub64.c within gdbstub.c
  target/arm: Always build Aarch64 gdbstub helpers

 target/arm/cpu.h       |   8 +-
 target/arm/internals.h |   2 -
 target/arm/gdbstub.c   | 363 +++++++++++++++++++++++++++++++++++++-
 target/arm/gdbstub64.c | 383 -----------------------------------------
 target/arm/meson.build |   1 -
 5 files changed, 364 insertions(+), 393 deletions(-)
 delete mode 100644 target/arm/gdbstub64.c

-- 
2.41.0



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/2] target/arm: Merge gdbstub64.c within gdbstub.c
  2024-06-19 12:49 [PATCH 0/2] target/arm: Always build Aarch64 gdbstub helpers Philippe Mathieu-Daudé
@ 2024-06-19 12:49 ` Philippe Mathieu-Daudé
  2024-06-19 12:49 ` [PATCH 2/2] target/arm: Always build Aarch64 gdbstub helpers Philippe Mathieu-Daudé
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-06-19 12:49 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Gustavo Romero, qemu-arm, Anton Johansson,
	Philippe Mathieu-Daudé

Simple code movement adding #ifdef'ry on TARGET_AARCH64.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/arm/gdbstub.c   | 363 ++++++++++++++++++++++++++++++++++++++
 target/arm/gdbstub64.c | 383 -----------------------------------------
 target/arm/meson.build |   1 -
 3 files changed, 363 insertions(+), 384 deletions(-)
 delete mode 100644 target/arm/gdbstub64.c

diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index a3bb73cfa7..508b3d8116 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -18,6 +18,7 @@
  * License along with this library; if not, see <http://www.gnu.org/licenses/>.
  */
 #include "qemu/osdep.h"
+#include "qemu/log.h"
 #include "cpu.h"
 #include "exec/gdbstub.h"
 #include "gdbstub/helpers.h"
@@ -310,6 +311,368 @@ static GDBFeature *arm_gen_dynamic_sysreg_feature(CPUState *cs, int base_reg)
     return &cpu->dyn_sysreg_feature.desc;
 }
 
+#ifdef TARGET_AARCH64
+int aarch64_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
+{
+    ARMCPU *cpu = ARM_CPU(cs);
+    CPUARMState *env = &cpu->env;
+
+    if (n < 31) {
+        /* Core integer register.  */
+        return gdb_get_reg64(mem_buf, env->xregs[n]);
+    }
+    switch (n) {
+    case 31:
+        return gdb_get_reg64(mem_buf, env->xregs[31]);
+    case 32:
+        return gdb_get_reg64(mem_buf, env->pc);
+    case 33:
+        return gdb_get_reg32(mem_buf, pstate_read(env));
+    }
+    /* Unknown register.  */
+    return 0;
+}
+
+int aarch64_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
+{
+    ARMCPU *cpu = ARM_CPU(cs);
+    CPUARMState *env = &cpu->env;
+    uint64_t tmp;
+
+    tmp = ldq_p(mem_buf);
+
+    if (n < 31) {
+        /* Core integer register.  */
+        env->xregs[n] = tmp;
+        return 8;
+    }
+    switch (n) {
+    case 31:
+        env->xregs[31] = tmp;
+        return 8;
+    case 32:
+        env->pc = tmp;
+        return 8;
+    case 33:
+        /* CPSR */
+        pstate_write(env, tmp);
+        return 4;
+    }
+    /* Unknown register.  */
+    return 0;
+}
+
+int aarch64_gdb_get_fpu_reg(CPUState *cs, GByteArray *buf, int reg)
+{
+    ARMCPU *cpu = ARM_CPU(cs);
+    CPUARMState *env = &cpu->env;
+
+    switch (reg) {
+    case 0 ... 31:
+    {
+        /* 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 */
+        return gdb_get_reg32(buf, vfp_get_fpsr(env));
+    case 33:
+        /* FPCR */
+        return gdb_get_reg32(buf, vfp_get_fpcr(env));
+    default:
+        return 0;
+    }
+}
+
+int aarch64_gdb_set_fpu_reg(CPUState *cs, uint8_t *buf, int reg)
+{
+    ARMCPU *cpu = ARM_CPU(cs);
+    CPUARMState *env = &cpu->env;
+
+    switch (reg) {
+    case 0 ... 31:
+        /* 128 bit FP register */
+        {
+            uint64_t *q = aa64_vfp_qreg(env, reg);
+            q[0] = ldq_le_p(buf);
+            q[1] = ldq_le_p(buf + 8);
+            return 16;
+        }
+    case 32:
+        /* FPSR */
+        vfp_set_fpsr(env, ldl_p(buf));
+        return 4;
+    case 33:
+        /* FPCR */
+        vfp_set_fpcr(env, ldl_p(buf));
+        return 4;
+    default:
+        return 0;
+    }
+}
+
+int aarch64_gdb_get_sve_reg(CPUState *cs, GByteArray *buf, int reg)
+{
+    ARMCPU *cpu = ARM_CPU(cs);
+    CPUARMState *env = &cpu->env;
+
+    switch (reg) {
+    /* The first 32 registers are the zregs */
+    case 0 ... 31:
+    {
+        int vq, len = 0;
+        for (vq = 0; vq < cpu->sve_max_vq; vq++) {
+            len += gdb_get_reg128(buf,
+                                  env->vfp.zregs[reg].d[vq * 2 + 1],
+                                  env->vfp.zregs[reg].d[vq * 2]);
+        }
+        return len;
+    }
+    case 32:
+        return gdb_get_reg32(buf, vfp_get_fpsr(env));
+    case 33:
+        return gdb_get_reg32(buf, vfp_get_fpcr(env));
+    /* then 16 predicates and the ffr */
+    case 34 ... 50:
+    {
+        int preg = reg - 34;
+        int vq, len = 0;
+        for (vq = 0; vq < cpu->sve_max_vq; vq = vq + 4) {
+            len += gdb_get_reg64(buf, env->vfp.pregs[preg].p[vq / 4]);
+        }
+        return len;
+    }
+    case 51:
+    {
+        /*
+         * We report in Vector Granules (VG) which is 64bit in a Z reg
+         * while the ZCR works in Vector Quads (VQ) which is 128bit chunks.
+         */
+        int vq = sve_vqm1_for_el(env, arm_current_el(env)) + 1;
+        return gdb_get_reg64(buf, vq * 2);
+    }
+    default:
+        /* gdbstub asked for something out our range */
+        qemu_log_mask(LOG_UNIMP, "%s: out of range register %d", __func__, reg);
+        break;
+    }
+
+    return 0;
+}
+
+int aarch64_gdb_set_sve_reg(CPUState *cs, uint8_t *buf, int reg)
+{
+    ARMCPU *cpu = ARM_CPU(cs);
+    CPUARMState *env = &cpu->env;
+
+    /* The first 32 registers are the zregs */
+    switch (reg) {
+    /* The first 32 registers are the zregs */
+    case 0 ... 31:
+    {
+        int vq, len = 0;
+        uint64_t *p = (uint64_t *) buf;
+        for (vq = 0; vq < cpu->sve_max_vq; vq++) {
+            env->vfp.zregs[reg].d[vq * 2 + 1] = *p++;
+            env->vfp.zregs[reg].d[vq * 2] = *p++;
+            len += 16;
+        }
+        return len;
+    }
+    case 32:
+        vfp_set_fpsr(env, *(uint32_t *)buf);
+        return 4;
+    case 33:
+        vfp_set_fpcr(env, *(uint32_t *)buf);
+        return 4;
+    case 34 ... 50:
+    {
+        int preg = reg - 34;
+        int vq, len = 0;
+        uint64_t *p = (uint64_t *) buf;
+        for (vq = 0; vq < cpu->sve_max_vq; vq = vq + 4) {
+            env->vfp.pregs[preg].p[vq / 4] = *p++;
+            len += 8;
+        }
+        return len;
+    }
+    case 51:
+        /* cannot set vg via gdbstub */
+        return 0;
+    default:
+        /* gdbstub asked for something out our range */
+        break;
+    }
+
+    return 0;
+}
+
+int aarch64_gdb_get_pauth_reg(CPUState *cs, GByteArray *buf, int reg)
+{
+    ARMCPU *cpu = ARM_CPU(cs);
+    CPUARMState *env = &cpu->env;
+
+    switch (reg) {
+    case 0: /* pauth_dmask */
+    case 1: /* pauth_cmask */
+    case 2: /* pauth_dmask_high */
+    case 3: /* pauth_cmask_high */
+        /*
+         * Note that older versions of this feature only contained
+         * pauth_{d,c}mask, for use with Linux user processes, and
+         * thus exclusively in the low half of the address space.
+         *
+         * To support system mode, and to debug kernels, two new regs
+         * were added to cover the high half of the address space.
+         * For the purpose of pauth_ptr_mask, we can use any well-formed
+         * address within the address space half -- here, 0 and -1.
+         */
+        {
+            bool is_data = !(reg & 1);
+            bool is_high = reg & 2;
+            ARMMMUIdx mmu_idx = arm_stage1_mmu_idx(env);
+            ARMVAParameters param;
+
+            param = aa64_va_parameters(env, -is_high, mmu_idx, is_data, false);
+            return gdb_get_reg64(buf, pauth_ptr_mask(param));
+        }
+    default:
+        return 0;
+    }
+}
+
+int aarch64_gdb_set_pauth_reg(CPUState *cs, uint8_t *buf, int reg)
+{
+    /* All pseudo registers are read-only. */
+    return 0;
+}
+
+static void output_vector_union_type(GDBFeatureBuilder *builder, int reg_width,
+                                     const char *name)
+{
+    struct TypeSize {
+        const char *gdb_type;
+        short size;
+        char sz, suffix;
+    };
+
+    static const struct TypeSize vec_lanes[] = {
+        /* quads */
+        { "uint128", 128, 'q', 'u' },
+        { "int128", 128, 'q', 's' },
+        /* 64 bit */
+        { "ieee_double", 64, 'd', 'f' },
+        { "uint64", 64, 'd', 'u' },
+        { "int64", 64, 'd', 's' },
+        /* 32 bit */
+        { "ieee_single", 32, 's', 'f' },
+        { "uint32", 32, 's', 'u' },
+        { "int32", 32, 's', 's' },
+        /* 16 bit */
+        { "ieee_half", 16, 'h', 'f' },
+        { "uint16", 16, 'h', 'u' },
+        { "int16", 16, 'h', 's' },
+        /* bytes */
+        { "uint8", 8, 'b', 'u' },
+        { "int8", 8, 'b', 's' },
+    };
+
+    static const char suf[] = { 'b', 'h', 's', 'd', 'q' };
+    int i, j;
+
+    /* First define types and totals in a whole VL */
+    for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
+        gdb_feature_builder_append_tag(
+            builder, "<vector id=\"%s%c%c\" type=\"%s\" count=\"%d\"/>",
+            name, vec_lanes[i].sz, vec_lanes[i].suffix,
+            vec_lanes[i].gdb_type, reg_width / vec_lanes[i].size);
+    }
+
+    /*
+     * Now define a union for each size group containing unsigned and
+     * signed and potentially float versions of each size from 128 to
+     * 8 bits.
+     */
+    for (i = 0; i < ARRAY_SIZE(suf); i++) {
+        int bits = 8 << i;
+
+        gdb_feature_builder_append_tag(builder, "<union id=\"%sn%c\">",
+                                       name, suf[i]);
+        for (j = 0; j < ARRAY_SIZE(vec_lanes); j++) {
+            if (vec_lanes[j].size == bits) {
+                gdb_feature_builder_append_tag(
+                    builder, "<field name=\"%c\" type=\"%s%c%c\"/>",
+                    vec_lanes[j].suffix, name,
+                    vec_lanes[j].sz, vec_lanes[j].suffix);
+            }
+        }
+        gdb_feature_builder_append_tag(builder, "</union>");
+    }
+
+    /* And now the final union of unions */
+    gdb_feature_builder_append_tag(builder, "<union id=\"%s\">", name);
+    for (i = ARRAY_SIZE(suf) - 1; i >= 0; i--) {
+        gdb_feature_builder_append_tag(builder,
+                                       "<field name=\"%c\" type=\"%sn%c\"/>",
+                                       suf[i], name, suf[i]);
+    }
+    gdb_feature_builder_append_tag(builder, "</union>");
+}
+
+GDBFeature *arm_gen_dynamic_svereg_feature(CPUState *cs, int base_reg)
+{
+    ARMCPU *cpu = ARM_CPU(cs);
+    int reg_width = cpu->sve_max_vq * 128;
+    int pred_width = cpu->sve_max_vq * 16;
+    GDBFeatureBuilder builder;
+    char *name;
+    int reg = 0;
+    int i;
+
+    gdb_feature_builder_init(&builder, &cpu->dyn_svereg_feature.desc,
+                             "org.gnu.gdb.aarch64.sve", "sve-registers.xml",
+                             base_reg);
+
+    /* Create the vector union type. */
+    output_vector_union_type(&builder, reg_width, "svev");
+
+    /* Create the predicate vector type. */
+    gdb_feature_builder_append_tag(
+        &builder, "<vector id=\"svep\" type=\"uint8\" count=\"%d\"/>",
+        pred_width / 8);
+
+    /* Define the vector registers. */
+    for (i = 0; i < 32; i++) {
+        name = g_strdup_printf("z%d", i);
+        gdb_feature_builder_append_reg(&builder, name, reg_width, reg++,
+                                       "svev", NULL);
+    }
+
+    /* fpscr & status registers */
+    gdb_feature_builder_append_reg(&builder, "fpsr", 32, reg++,
+                                   "int", "float");
+    gdb_feature_builder_append_reg(&builder, "fpcr", 32, reg++,
+                                   "int", "float");
+
+    /* Define the predicate registers. */
+    for (i = 0; i < 16; i++) {
+        name = g_strdup_printf("p%d", i);
+        gdb_feature_builder_append_reg(&builder, name, pred_width, reg++,
+                                       "svep", NULL);
+    }
+    gdb_feature_builder_append_reg(&builder, "ffr", pred_width, reg++,
+                                   "svep", "vector");
+
+    /* Define the vector length pseudo-register. */
+    gdb_feature_builder_append_reg(&builder, "vg", 64, reg++, "int", NULL);
+
+    gdb_feature_builder_end(&builder);
+
+    return &cpu->dyn_svereg_feature.desc;
+}
+#endif
+
 #ifdef CONFIG_TCG
 typedef enum {
     M_SYSREG_MSP,
diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
deleted file mode 100644
index caa31ff3fa..0000000000
--- a/target/arm/gdbstub64.c
+++ /dev/null
@@ -1,383 +0,0 @@
-/*
- * ARM gdb server stub: AArch64 specific functions.
- *
- * Copyright (c) 2013 SUSE LINUX Products GmbH
- *
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2.1 of the License, or (at your option) any later version.
- *
- * This library is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this library; if not, see <http://www.gnu.org/licenses/>.
- */
-#include "qemu/osdep.h"
-#include "qemu/log.h"
-#include "cpu.h"
-#include "internals.h"
-#include "gdbstub/helpers.h"
-
-int aarch64_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
-{
-    ARMCPU *cpu = ARM_CPU(cs);
-    CPUARMState *env = &cpu->env;
-
-    if (n < 31) {
-        /* Core integer register.  */
-        return gdb_get_reg64(mem_buf, env->xregs[n]);
-    }
-    switch (n) {
-    case 31:
-        return gdb_get_reg64(mem_buf, env->xregs[31]);
-    case 32:
-        return gdb_get_reg64(mem_buf, env->pc);
-    case 33:
-        return gdb_get_reg32(mem_buf, pstate_read(env));
-    }
-    /* Unknown register.  */
-    return 0;
-}
-
-int aarch64_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
-{
-    ARMCPU *cpu = ARM_CPU(cs);
-    CPUARMState *env = &cpu->env;
-    uint64_t tmp;
-
-    tmp = ldq_p(mem_buf);
-
-    if (n < 31) {
-        /* Core integer register.  */
-        env->xregs[n] = tmp;
-        return 8;
-    }
-    switch (n) {
-    case 31:
-        env->xregs[31] = tmp;
-        return 8;
-    case 32:
-        env->pc = tmp;
-        return 8;
-    case 33:
-        /* CPSR */
-        pstate_write(env, tmp);
-        return 4;
-    }
-    /* Unknown register.  */
-    return 0;
-}
-
-int aarch64_gdb_get_fpu_reg(CPUState *cs, GByteArray *buf, int reg)
-{
-    ARMCPU *cpu = ARM_CPU(cs);
-    CPUARMState *env = &cpu->env;
-
-    switch (reg) {
-    case 0 ... 31:
-    {
-        /* 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 */
-        return gdb_get_reg32(buf, vfp_get_fpsr(env));
-    case 33:
-        /* FPCR */
-        return gdb_get_reg32(buf, vfp_get_fpcr(env));
-    default:
-        return 0;
-    }
-}
-
-int aarch64_gdb_set_fpu_reg(CPUState *cs, uint8_t *buf, int reg)
-{
-    ARMCPU *cpu = ARM_CPU(cs);
-    CPUARMState *env = &cpu->env;
-
-    switch (reg) {
-    case 0 ... 31:
-        /* 128 bit FP register */
-        {
-            uint64_t *q = aa64_vfp_qreg(env, reg);
-            q[0] = ldq_le_p(buf);
-            q[1] = ldq_le_p(buf + 8);
-            return 16;
-        }
-    case 32:
-        /* FPSR */
-        vfp_set_fpsr(env, ldl_p(buf));
-        return 4;
-    case 33:
-        /* FPCR */
-        vfp_set_fpcr(env, ldl_p(buf));
-        return 4;
-    default:
-        return 0;
-    }
-}
-
-int aarch64_gdb_get_sve_reg(CPUState *cs, GByteArray *buf, int reg)
-{
-    ARMCPU *cpu = ARM_CPU(cs);
-    CPUARMState *env = &cpu->env;
-
-    switch (reg) {
-    /* The first 32 registers are the zregs */
-    case 0 ... 31:
-    {
-        int vq, len = 0;
-        for (vq = 0; vq < cpu->sve_max_vq; vq++) {
-            len += gdb_get_reg128(buf,
-                                  env->vfp.zregs[reg].d[vq * 2 + 1],
-                                  env->vfp.zregs[reg].d[vq * 2]);
-        }
-        return len;
-    }
-    case 32:
-        return gdb_get_reg32(buf, vfp_get_fpsr(env));
-    case 33:
-        return gdb_get_reg32(buf, vfp_get_fpcr(env));
-    /* then 16 predicates and the ffr */
-    case 34 ... 50:
-    {
-        int preg = reg - 34;
-        int vq, len = 0;
-        for (vq = 0; vq < cpu->sve_max_vq; vq = vq + 4) {
-            len += gdb_get_reg64(buf, env->vfp.pregs[preg].p[vq / 4]);
-        }
-        return len;
-    }
-    case 51:
-    {
-        /*
-         * We report in Vector Granules (VG) which is 64bit in a Z reg
-         * while the ZCR works in Vector Quads (VQ) which is 128bit chunks.
-         */
-        int vq = sve_vqm1_for_el(env, arm_current_el(env)) + 1;
-        return gdb_get_reg64(buf, vq * 2);
-    }
-    default:
-        /* gdbstub asked for something out our range */
-        qemu_log_mask(LOG_UNIMP, "%s: out of range register %d", __func__, reg);
-        break;
-    }
-
-    return 0;
-}
-
-int aarch64_gdb_set_sve_reg(CPUState *cs, uint8_t *buf, int reg)
-{
-    ARMCPU *cpu = ARM_CPU(cs);
-    CPUARMState *env = &cpu->env;
-
-    /* The first 32 registers are the zregs */
-    switch (reg) {
-    /* The first 32 registers are the zregs */
-    case 0 ... 31:
-    {
-        int vq, len = 0;
-        uint64_t *p = (uint64_t *) buf;
-        for (vq = 0; vq < cpu->sve_max_vq; vq++) {
-            env->vfp.zregs[reg].d[vq * 2 + 1] = *p++;
-            env->vfp.zregs[reg].d[vq * 2] = *p++;
-            len += 16;
-        }
-        return len;
-    }
-    case 32:
-        vfp_set_fpsr(env, *(uint32_t *)buf);
-        return 4;
-    case 33:
-        vfp_set_fpcr(env, *(uint32_t *)buf);
-        return 4;
-    case 34 ... 50:
-    {
-        int preg = reg - 34;
-        int vq, len = 0;
-        uint64_t *p = (uint64_t *) buf;
-        for (vq = 0; vq < cpu->sve_max_vq; vq = vq + 4) {
-            env->vfp.pregs[preg].p[vq / 4] = *p++;
-            len += 8;
-        }
-        return len;
-    }
-    case 51:
-        /* cannot set vg via gdbstub */
-        return 0;
-    default:
-        /* gdbstub asked for something out our range */
-        break;
-    }
-
-    return 0;
-}
-
-int aarch64_gdb_get_pauth_reg(CPUState *cs, GByteArray *buf, int reg)
-{
-    ARMCPU *cpu = ARM_CPU(cs);
-    CPUARMState *env = &cpu->env;
-
-    switch (reg) {
-    case 0: /* pauth_dmask */
-    case 1: /* pauth_cmask */
-    case 2: /* pauth_dmask_high */
-    case 3: /* pauth_cmask_high */
-        /*
-         * Note that older versions of this feature only contained
-         * pauth_{d,c}mask, for use with Linux user processes, and
-         * thus exclusively in the low half of the address space.
-         *
-         * To support system mode, and to debug kernels, two new regs
-         * were added to cover the high half of the address space.
-         * For the purpose of pauth_ptr_mask, we can use any well-formed
-         * address within the address space half -- here, 0 and -1.
-         */
-        {
-            bool is_data = !(reg & 1);
-            bool is_high = reg & 2;
-            ARMMMUIdx mmu_idx = arm_stage1_mmu_idx(env);
-            ARMVAParameters param;
-
-            param = aa64_va_parameters(env, -is_high, mmu_idx, is_data, false);
-            return gdb_get_reg64(buf, pauth_ptr_mask(param));
-        }
-    default:
-        return 0;
-    }
-}
-
-int aarch64_gdb_set_pauth_reg(CPUState *cs, uint8_t *buf, int reg)
-{
-    /* All pseudo registers are read-only. */
-    return 0;
-}
-
-static void output_vector_union_type(GDBFeatureBuilder *builder, int reg_width,
-                                     const char *name)
-{
-    struct TypeSize {
-        const char *gdb_type;
-        short size;
-        char sz, suffix;
-    };
-
-    static const struct TypeSize vec_lanes[] = {
-        /* quads */
-        { "uint128", 128, 'q', 'u' },
-        { "int128", 128, 'q', 's' },
-        /* 64 bit */
-        { "ieee_double", 64, 'd', 'f' },
-        { "uint64", 64, 'd', 'u' },
-        { "int64", 64, 'd', 's' },
-        /* 32 bit */
-        { "ieee_single", 32, 's', 'f' },
-        { "uint32", 32, 's', 'u' },
-        { "int32", 32, 's', 's' },
-        /* 16 bit */
-        { "ieee_half", 16, 'h', 'f' },
-        { "uint16", 16, 'h', 'u' },
-        { "int16", 16, 'h', 's' },
-        /* bytes */
-        { "uint8", 8, 'b', 'u' },
-        { "int8", 8, 'b', 's' },
-    };
-
-    static const char suf[] = { 'b', 'h', 's', 'd', 'q' };
-    int i, j;
-
-    /* First define types and totals in a whole VL */
-    for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
-        gdb_feature_builder_append_tag(
-            builder, "<vector id=\"%s%c%c\" type=\"%s\" count=\"%d\"/>",
-            name, vec_lanes[i].sz, vec_lanes[i].suffix,
-            vec_lanes[i].gdb_type, reg_width / vec_lanes[i].size);
-    }
-
-    /*
-     * Now define a union for each size group containing unsigned and
-     * signed and potentially float versions of each size from 128 to
-     * 8 bits.
-     */
-    for (i = 0; i < ARRAY_SIZE(suf); i++) {
-        int bits = 8 << i;
-
-        gdb_feature_builder_append_tag(builder, "<union id=\"%sn%c\">",
-                                       name, suf[i]);
-        for (j = 0; j < ARRAY_SIZE(vec_lanes); j++) {
-            if (vec_lanes[j].size == bits) {
-                gdb_feature_builder_append_tag(
-                    builder, "<field name=\"%c\" type=\"%s%c%c\"/>",
-                    vec_lanes[j].suffix, name,
-                    vec_lanes[j].sz, vec_lanes[j].suffix);
-            }
-        }
-        gdb_feature_builder_append_tag(builder, "</union>");
-    }
-
-    /* And now the final union of unions */
-    gdb_feature_builder_append_tag(builder, "<union id=\"%s\">", name);
-    for (i = ARRAY_SIZE(suf) - 1; i >= 0; i--) {
-        gdb_feature_builder_append_tag(builder,
-                                       "<field name=\"%c\" type=\"%sn%c\"/>",
-                                       suf[i], name, suf[i]);
-    }
-    gdb_feature_builder_append_tag(builder, "</union>");
-}
-
-GDBFeature *arm_gen_dynamic_svereg_feature(CPUState *cs, int base_reg)
-{
-    ARMCPU *cpu = ARM_CPU(cs);
-    int reg_width = cpu->sve_max_vq * 128;
-    int pred_width = cpu->sve_max_vq * 16;
-    GDBFeatureBuilder builder;
-    char *name;
-    int reg = 0;
-    int i;
-
-    gdb_feature_builder_init(&builder, &cpu->dyn_svereg_feature.desc,
-                             "org.gnu.gdb.aarch64.sve", "sve-registers.xml",
-                             base_reg);
-
-    /* Create the vector union type. */
-    output_vector_union_type(&builder, reg_width, "svev");
-
-    /* Create the predicate vector type. */
-    gdb_feature_builder_append_tag(
-        &builder, "<vector id=\"svep\" type=\"uint8\" count=\"%d\"/>",
-        pred_width / 8);
-
-    /* Define the vector registers. */
-    for (i = 0; i < 32; i++) {
-        name = g_strdup_printf("z%d", i);
-        gdb_feature_builder_append_reg(&builder, name, reg_width, reg++,
-                                       "svev", NULL);
-    }
-
-    /* fpscr & status registers */
-    gdb_feature_builder_append_reg(&builder, "fpsr", 32, reg++,
-                                   "int", "float");
-    gdb_feature_builder_append_reg(&builder, "fpcr", 32, reg++,
-                                   "int", "float");
-
-    /* Define the predicate registers. */
-    for (i = 0; i < 16; i++) {
-        name = g_strdup_printf("p%d", i);
-        gdb_feature_builder_append_reg(&builder, name, pred_width, reg++,
-                                       "svep", NULL);
-    }
-    gdb_feature_builder_append_reg(&builder, "ffr", pred_width, reg++,
-                                   "svep", "vector");
-
-    /* Define the vector length pseudo-register. */
-    gdb_feature_builder_append_reg(&builder, "vg", 64, reg++, "int", NULL);
-
-    gdb_feature_builder_end(&builder);
-
-    return &cpu->dyn_svereg_feature.desc;
-}
diff --git a/target/arm/meson.build b/target/arm/meson.build
index 2e10464dbb..10b7f8d509 100644
--- a/target/arm/meson.build
+++ b/target/arm/meson.build
@@ -13,7 +13,6 @@ arm_ss.add(when: 'CONFIG_HVF', if_true: files('hyp_gdbstub.c'))
 
 arm_ss.add(when: 'TARGET_AARCH64', if_true: files(
   'cpu64.c',
-  'gdbstub64.c',
 ))
 
 arm_system_ss = ss.source_set()
-- 
2.41.0



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/2] target/arm: Always build Aarch64 gdbstub helpers
  2024-06-19 12:49 [PATCH 0/2] target/arm: Always build Aarch64 gdbstub helpers Philippe Mathieu-Daudé
  2024-06-19 12:49 ` [PATCH 1/2] target/arm: Merge gdbstub64.c within gdbstub.c Philippe Mathieu-Daudé
@ 2024-06-19 12:49 ` Philippe Mathieu-Daudé
  2024-06-28  6:19 ` [PATCH 0/2] " Philippe Mathieu-Daudé
  2024-06-28 14:31 ` Richard Henderson
  3 siblings, 0 replies; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-06-19 12:49 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Gustavo Romero, qemu-arm, Anton Johansson,
	Philippe Mathieu-Daudé

In order to have a single binary for ARM and Aarch64,
always build Aarch64 gdbstub support.

Since arm_cpu_register_gdb_regs_for_features() checks
on arm_feature(env, ARM_FEATURE_AARCH64), the Aarch64
gdb registers won't be registered on 32-bit ARM.
There should be no functional changes.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/arm/cpu.h       | 8 +++-----
 target/arm/internals.h | 2 --
 target/arm/gdbstub.c   | 4 ----
 3 files changed, 3 insertions(+), 11 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 3841359d0f..1240754f71 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -180,12 +180,12 @@ typedef struct ARMVectorReg {
     uint64_t d[2 * ARM_MAX_VQ] QEMU_ALIGNED(16);
 } ARMVectorReg;
 
-#ifdef TARGET_AARCH64
 /* In AArch32 mode, predicate registers do not exist at all.  */
 typedef struct ARMPredicateReg {
     uint64_t p[DIV_ROUND_UP(2 * ARM_MAX_VQ, 8)] QEMU_ALIGNED(16);
 } ARMPredicateReg;
 
+#ifdef TARGET_AARCH64
 /* In AArch32 mode, PAC keys do not exist at all.  */
 typedef struct ARMPACKey {
     uint64_t lo, hi;
@@ -606,13 +606,11 @@ typedef struct CPUArchState {
     struct {
         ARMVectorReg zregs[32];
 
-#ifdef TARGET_AARCH64
         /* Store FFR as pregs[16] to make it easier to treat as any other.  */
 #define FFR_PRED_NUM 16
         ARMPredicateReg pregs[17];
         /* Scratch space for aa64 sve predicate temporary.  */
         ARMPredicateReg preg_tmp;
-#endif
 
         /* We store these fpcsr fields separately for convenience.  */
         uint32_t qc[4] QEMU_ALIGNED(16);
@@ -1165,6 +1163,8 @@ hwaddr arm_cpu_get_phys_page_attrs_debug(CPUState *cpu, vaddr addr,
 
 int arm_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
 int arm_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
+int aarch64_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
+int aarch64_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
 
 int arm_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cs,
                              int cpuid, DumpState *s);
@@ -1194,8 +1194,6 @@ int arm_cpu_write_elf32_note(WriteCoreDumpFunction f, CPUState *cs,
 void arm_emulate_firmware_reset(CPUState *cpustate, int target_el);
 
 #ifdef TARGET_AARCH64
-int aarch64_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
-int aarch64_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
 void aarch64_sve_narrow_vq(CPUARMState *env, unsigned vq);
 void aarch64_sve_change_el(CPUARMState *env, int old_el,
                            int new_el, bool el0_a64);
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 11b5da2562..79dd62dd46 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1632,7 +1632,6 @@ static inline uint64_t pmu_counter_mask(CPUARMState *env)
   return (1ULL << 31) | ((1ULL << pmu_num_counters(env)) - 1);
 }
 
-#ifdef TARGET_AARCH64
 GDBFeature *arm_gen_dynamic_svereg_feature(CPUState *cpu, int base_reg);
 int aarch64_gdb_get_sve_reg(CPUState *cs, GByteArray *buf, int reg);
 int aarch64_gdb_set_sve_reg(CPUState *cs, uint8_t *buf, int reg);
@@ -1648,7 +1647,6 @@ void aarch64_max_tcg_initfn(Object *obj);
 void aarch64_add_pauth_properties(Object *obj);
 void aarch64_add_sve_properties(Object *obj);
 void aarch64_add_sme_properties(Object *obj);
-#endif
 
 /* Read the CONTROL register as the MRS instruction would. */
 uint32_t arm_v7m_mrs_control(CPUARMState *env, uint32_t secure);
diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index 508b3d8116..92fa716826 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -311,7 +311,6 @@ static GDBFeature *arm_gen_dynamic_sysreg_feature(CPUState *cs, int base_reg)
     return &cpu->dyn_sysreg_feature.desc;
 }
 
-#ifdef TARGET_AARCH64
 int aarch64_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
 {
     ARMCPU *cpu = ARM_CPU(cs);
@@ -671,7 +670,6 @@ GDBFeature *arm_gen_dynamic_svereg_feature(CPUState *cs, int base_reg)
 
     return &cpu->dyn_svereg_feature.desc;
 }
-#endif
 
 #ifdef CONFIG_TCG
 typedef enum {
@@ -847,7 +845,6 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
          * The lower part of each SVE register aliases to the FPU
          * registers so we don't need to include both.
          */
-#ifdef TARGET_AARCH64
         if (isar_feature_aa64_sve(&cpu->isar)) {
             GDBFeature *feature = arm_gen_dynamic_svereg_feature(cs, cs->gdb_num_regs);
             gdb_register_coprocessor(cs, aarch64_gdb_get_sve_reg,
@@ -870,7 +867,6 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
                                      gdb_find_static_feature("aarch64-pauth.xml"),
                                      0);
         }
-#endif
     } else {
         if (arm_feature(env, ARM_FEATURE_NEON)) {
             gdb_register_coprocessor(cs, vfp_gdb_get_reg, vfp_gdb_set_reg,
-- 
2.41.0



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/2] target/arm: Always build Aarch64 gdbstub helpers
  2024-06-19 12:49 [PATCH 0/2] target/arm: Always build Aarch64 gdbstub helpers Philippe Mathieu-Daudé
  2024-06-19 12:49 ` [PATCH 1/2] target/arm: Merge gdbstub64.c within gdbstub.c Philippe Mathieu-Daudé
  2024-06-19 12:49 ` [PATCH 2/2] target/arm: Always build Aarch64 gdbstub helpers Philippe Mathieu-Daudé
@ 2024-06-28  6:19 ` Philippe Mathieu-Daudé
  2024-06-28 14:31 ` Richard Henderson
  3 siblings, 0 replies; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-06-28  6:19 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Gustavo Romero, qemu-arm, Anton Johansson,
	Alex Bennée

ping?

On 19/6/24 14:49, Philippe Mathieu-Daudé wrote:
> Merge gdbstub64.c in gdbstub.c and remove uses of
> target specific TARGET_AARCH64 definition.
> Small step toward single ARM/Aarch64 binary.
> 
> Philippe Mathieu-Daudé (2):
>    target/arm: Merge gdbstub64.c within gdbstub.c
>    target/arm: Always build Aarch64 gdbstub helpers
> 
>   target/arm/cpu.h       |   8 +-
>   target/arm/internals.h |   2 -
>   target/arm/gdbstub.c   | 363 +++++++++++++++++++++++++++++++++++++-
>   target/arm/gdbstub64.c | 383 -----------------------------------------
>   target/arm/meson.build |   1 -
>   5 files changed, 364 insertions(+), 393 deletions(-)
>   delete mode 100644 target/arm/gdbstub64.c
> 



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/2] target/arm: Always build Aarch64 gdbstub helpers
  2024-06-19 12:49 [PATCH 0/2] target/arm: Always build Aarch64 gdbstub helpers Philippe Mathieu-Daudé
                   ` (2 preceding siblings ...)
  2024-06-28  6:19 ` [PATCH 0/2] " Philippe Mathieu-Daudé
@ 2024-06-28 14:31 ` Richard Henderson
  2024-06-28 16:37   ` Philippe Mathieu-Daudé
  3 siblings, 1 reply; 9+ messages in thread
From: Richard Henderson @ 2024-06-28 14:31 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Peter Maydell, Gustavo Romero, qemu-arm, Anton Johansson

On 6/19/24 05:49, Philippe Mathieu-Daudé wrote:
> Merge gdbstub64.c in gdbstub.c and remove uses of
> target specific TARGET_AARCH64 definition.
> Small step toward single ARM/Aarch64 binary.
> 
> Philippe Mathieu-Daudé (2):
>    target/arm: Merge gdbstub64.c within gdbstub.c
>    target/arm: Always build Aarch64 gdbstub helpers
> 
>   target/arm/cpu.h       |   8 +-
>   target/arm/internals.h |   2 -
>   target/arm/gdbstub.c   | 363 +++++++++++++++++++++++++++++++++++++-
>   target/arm/gdbstub64.c | 383 -----------------------------------------
>   target/arm/meson.build |   1 -
>   5 files changed, 364 insertions(+), 393 deletions(-)
>   delete mode 100644 target/arm/gdbstub64.c
> 

Are we attempting a single binary for user-only as well?


r~


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/2] target/arm: Always build Aarch64 gdbstub helpers
  2024-06-28 14:31 ` Richard Henderson
@ 2024-06-28 16:37   ` Philippe Mathieu-Daudé
  2024-06-28 16:50     ` Richard Henderson
  0 siblings, 1 reply; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-06-28 16:37 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel
  Cc: Peter Maydell, Gustavo Romero, qemu-arm, Anton Johansson

On 28/6/24 16:31, Richard Henderson wrote:
> On 6/19/24 05:49, Philippe Mathieu-Daudé wrote:
>> Merge gdbstub64.c in gdbstub.c and remove uses of
>> target specific TARGET_AARCH64 definition.
>> Small step toward single ARM/Aarch64 binary.
>>
>> Philippe Mathieu-Daudé (2):
>>    target/arm: Merge gdbstub64.c within gdbstub.c
>>    target/arm: Always build Aarch64 gdbstub helpers
>>
>>   target/arm/cpu.h       |   8 +-
>>   target/arm/internals.h |   2 -
>>   target/arm/gdbstub.c   | 363 +++++++++++++++++++++++++++++++++++++-
>>   target/arm/gdbstub64.c | 383 -----------------------------------------
>>   target/arm/meson.build |   1 -
>>   5 files changed, 364 insertions(+), 393 deletions(-)
>>   delete mode 100644 target/arm/gdbstub64.c
>>
> 
> Are we attempting a single binary for user-only as well?

No, due to ABI constraints, right? I did a user-emulation
smoke build, no failure, did I miss something?



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/2] target/arm: Always build Aarch64 gdbstub helpers
  2024-06-28 16:37   ` Philippe Mathieu-Daudé
@ 2024-06-28 16:50     ` Richard Henderson
  2024-07-04 15:01       ` Peter Maydell
  0 siblings, 1 reply; 9+ messages in thread
From: Richard Henderson @ 2024-06-28 16:50 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Peter Maydell, Gustavo Romero, qemu-arm, Anton Johansson

On 6/28/24 09:37, Philippe Mathieu-Daudé wrote:
> On 28/6/24 16:31, Richard Henderson wrote:
>> On 6/19/24 05:49, Philippe Mathieu-Daudé wrote:
>>> Merge gdbstub64.c in gdbstub.c and remove uses of
>>> target specific TARGET_AARCH64 definition.
>>> Small step toward single ARM/Aarch64 binary.
>>>
>>> Philippe Mathieu-Daudé (2):
>>>    target/arm: Merge gdbstub64.c within gdbstub.c
>>>    target/arm: Always build Aarch64 gdbstub helpers
>>>
>>>   target/arm/cpu.h       |   8 +-
>>>   target/arm/internals.h |   2 -
>>>   target/arm/gdbstub.c   | 363 +++++++++++++++++++++++++++++++++++++-
>>>   target/arm/gdbstub64.c | 383 -----------------------------------------
>>>   target/arm/meson.build |   1 -
>>>   5 files changed, 364 insertions(+), 393 deletions(-)
>>>   delete mode 100644 target/arm/gdbstub64.c
>>>
>>
>> Are we attempting a single binary for user-only as well?
> 
> No, due to ABI constraints, right? I did a user-emulation
> smoke build, no failure, did I miss something?

Well, no.  But qemu-arm does not need gdbstub64.c.
Given TARGET_AARCH64 will be set on a combined build, I'm not sure what is the fix?


r~


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/2] target/arm: Always build Aarch64 gdbstub helpers
  2024-06-28 16:50     ` Richard Henderson
@ 2024-07-04 15:01       ` Peter Maydell
  2024-07-04 18:24         ` Richard Henderson
  0 siblings, 1 reply; 9+ messages in thread
From: Peter Maydell @ 2024-07-04 15:01 UTC (permalink / raw)
  To: Richard Henderson
  Cc: Philippe Mathieu-Daudé, qemu-devel, Gustavo Romero, qemu-arm,
	Anton Johansson

On Fri, 28 Jun 2024 at 17:50, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 6/28/24 09:37, Philippe Mathieu-Daudé wrote:
> > On 28/6/24 16:31, Richard Henderson wrote:
> >> On 6/19/24 05:49, Philippe Mathieu-Daudé wrote:
> >>> Merge gdbstub64.c in gdbstub.c and remove uses of
> >>> target specific TARGET_AARCH64 definition.
> >>> Small step toward single ARM/Aarch64 binary.
> >>>
> >>> Philippe Mathieu-Daudé (2):
> >>>    target/arm: Merge gdbstub64.c within gdbstub.c
> >>>    target/arm: Always build Aarch64 gdbstub helpers
> >>>
> >>>   target/arm/cpu.h       |   8 +-
> >>>   target/arm/internals.h |   2 -
> >>>   target/arm/gdbstub.c   | 363 +++++++++++++++++++++++++++++++++++++-
> >>>   target/arm/gdbstub64.c | 383 -----------------------------------------
> >>>   target/arm/meson.build |   1 -
> >>>   5 files changed, 364 insertions(+), 393 deletions(-)
> >>>   delete mode 100644 target/arm/gdbstub64.c
> >>>
> >>
> >> Are we attempting a single binary for user-only as well?
> >
> > No, due to ABI constraints, right? I did a user-emulation
> > smoke build, no failure, did I miss something?
>
> Well, no.  But qemu-arm does not need gdbstub64.c.
> Given TARGET_AARCH64 will be set on a combined build, I'm not sure what is the fix?

Richard: I'm a bit confused about where we are with this
patchset. Do your comments mean:
 * this patchset is OK for system emulation but we
   should (later) think also about user-mode ?
 * this patchset has a problem with user-mode so it
   needs rethinking ?
 * something else ?

thanks
-- PMM


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/2] target/arm: Always build Aarch64 gdbstub helpers
  2024-07-04 15:01       ` Peter Maydell
@ 2024-07-04 18:24         ` Richard Henderson
  0 siblings, 0 replies; 9+ messages in thread
From: Richard Henderson @ 2024-07-04 18:24 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Philippe Mathieu-Daudé, qemu-devel, Gustavo Romero, qemu-arm,
	Anton Johansson

On 7/4/24 08:01, Peter Maydell wrote:
> On Fri, 28 Jun 2024 at 17:50, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> On 6/28/24 09:37, Philippe Mathieu-Daudé wrote:
>>> On 28/6/24 16:31, Richard Henderson wrote:
>>>> On 6/19/24 05:49, Philippe Mathieu-Daudé wrote:
>>>>> Merge gdbstub64.c in gdbstub.c and remove uses of
>>>>> target specific TARGET_AARCH64 definition.
>>>>> Small step toward single ARM/Aarch64 binary.
>>>>>
>>>>> Philippe Mathieu-Daudé (2):
>>>>>     target/arm: Merge gdbstub64.c within gdbstub.c
>>>>>     target/arm: Always build Aarch64 gdbstub helpers
>>>>>
>>>>>    target/arm/cpu.h       |   8 +-
>>>>>    target/arm/internals.h |   2 -
>>>>>    target/arm/gdbstub.c   | 363 +++++++++++++++++++++++++++++++++++++-
>>>>>    target/arm/gdbstub64.c | 383 -----------------------------------------
>>>>>    target/arm/meson.build |   1 -
>>>>>    5 files changed, 364 insertions(+), 393 deletions(-)
>>>>>    delete mode 100644 target/arm/gdbstub64.c
>>>>>
>>>>
>>>> Are we attempting a single binary for user-only as well?
>>>
>>> No, due to ABI constraints, right? I did a user-emulation
>>> smoke build, no failure, did I miss something?
>>
>> Well, no.  But qemu-arm does not need gdbstub64.c.
>> Given TARGET_AARCH64 will be set on a combined build, I'm not sure what is the fix?
> 
> Richard: I'm a bit confused about where we are with this
> patchset. Do your comments mean:
>   * this patchset is OK for system emulation but we
>     should (later) think also about user-mode ?
>   * this patchset has a problem with user-mode so it
>     needs rethinking ?
>   * something else ?

I'm confused about what this patch set improves.

It doesn't remove ifdefs; they're still there in gdbstub.c.  The code that handles aarch64 
is now in gdbstub instead of segregated into gdbstub64.c, so we have one larger file for 
no obvious benefit.

Was there some other build problem that I missed?  Because I don't see how it advances the 
stated goal of "a single ARM/AArch64 binary".


r~


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2024-07-04 18:25 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-19 12:49 [PATCH 0/2] target/arm: Always build Aarch64 gdbstub helpers Philippe Mathieu-Daudé
2024-06-19 12:49 ` [PATCH 1/2] target/arm: Merge gdbstub64.c within gdbstub.c Philippe Mathieu-Daudé
2024-06-19 12:49 ` [PATCH 2/2] target/arm: Always build Aarch64 gdbstub helpers Philippe Mathieu-Daudé
2024-06-28  6:19 ` [PATCH 0/2] " Philippe Mathieu-Daudé
2024-06-28 14:31 ` Richard Henderson
2024-06-28 16:37   ` Philippe Mathieu-Daudé
2024-06-28 16:50     ` Richard Henderson
2024-07-04 15:01       ` Peter Maydell
2024-07-04 18:24         ` Richard Henderson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).