* [PATCH 3/5] target/arm: Move gdbstub related code out of helper.c
2021-09-21 16:28 [PATCH 0/5] target/arm: Support MVE in gdbstub Peter Maydell
2021-09-21 16:28 ` [PATCH 1/5] configs: Don't include 32-bit-only GDB XML in aarch64 linux configs Peter Maydell
2021-09-21 16:28 ` [PATCH 2/5] target/arm: Fix coding style issues in gdbstub code in helper.c Peter Maydell
@ 2021-09-21 16:28 ` Peter Maydell
2021-09-21 18:10 ` Philippe Mathieu-Daudé
2021-09-22 3:22 ` Richard Henderson
2021-09-21 16:29 ` [PATCH 4/5] target/arm: Don't put FPEXC and FPSID in org.gnu.gdb.arm.vfp XML Peter Maydell
2021-09-21 16:29 ` [PATCH 5/5] [RFC] target/arm: Advertise MVE to gdb when present Peter Maydell
4 siblings, 2 replies; 13+ messages in thread
From: Peter Maydell @ 2021-09-21 16:28 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Luis Machado
Currently helper.c includes some code which is part of the arm
target's gdbstub support. This code has a better home: in gdbstub.c
and gdbstub64.c. Move it there.
Because aarch64_fpu_gdb_get_reg() and aarch64_fpu_gdb_set_reg() move
into gdbstub64.c, this means that they're now compiled only for
TARGET_AARCH64 rather than always. That is the only case when they
would ever be used, but it does mean that the ifdef in
arm_cpu_register_gdb_regs_for_features() needs to be adjusted to
match.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/internals.h | 7 ++
target/arm/gdbstub.c | 130 ++++++++++++++++++++
target/arm/gdbstub64.c | 140 +++++++++++++++++++++
target/arm/helper.c | 271 -----------------------------------------
4 files changed, 277 insertions(+), 271 deletions(-)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index cd2ea8a3883..d6f8c0bc927 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1226,4 +1226,11 @@ enum MVEECIState {
/* All other values reserved */
};
+#ifdef TARGET_AARCH64
+int arm_gdb_get_svereg(CPUARMState *env, GByteArray *buf, int reg);
+int arm_gdb_set_svereg(CPUARMState *env, uint8_t *buf, int reg);
+int aarch64_fpu_gdb_get_reg(CPUARMState *env, GByteArray *buf, int reg);
+int aarch64_fpu_gdb_set_reg(CPUARMState *env, uint8_t *buf, int reg);
+#endif
+
#endif
diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index 826601b3415..cbf156d192f 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -19,6 +19,7 @@
*/
#include "qemu/osdep.h"
#include "cpu.h"
+#include "internals.h"
#include "exec/gdbstub.h"
typedef struct RegisterSysregXmlParam {
@@ -124,6 +125,98 @@ int arm_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
return 0;
}
+static int vfp_gdb_get_reg(CPUARMState *env, GByteArray *buf, int reg)
+{
+ ARMCPU *cpu = env_archcpu(env);
+ int nregs = cpu_isar_feature(aa32_simd_r32, cpu) ? 32 : 16;
+
+ /* VFP data registers are always little-endian. */
+ if (reg < nregs) {
+ return gdb_get_reg64(buf, *aa32_vfp_dreg(env, reg));
+ }
+ if (arm_feature(env, ARM_FEATURE_NEON)) {
+ /* Aliases for Q regs. */
+ nregs += 16;
+ if (reg < nregs) {
+ uint64_t *q = aa32_vfp_qreg(env, reg - 32);
+ return gdb_get_reg128(buf, q[0], q[1]);
+ }
+ }
+ switch (reg - nregs) {
+ case 0:
+ return gdb_get_reg32(buf, env->vfp.xregs[ARM_VFP_FPSID]);
+ case 1:
+ return gdb_get_reg32(buf, vfp_get_fpscr(env));
+ case 2:
+ return gdb_get_reg32(buf, env->vfp.xregs[ARM_VFP_FPEXC]);
+ }
+ return 0;
+}
+
+static int vfp_gdb_set_reg(CPUARMState *env, uint8_t *buf, int reg)
+{
+ ARMCPU *cpu = env_archcpu(env);
+ int nregs = cpu_isar_feature(aa32_simd_r32, cpu) ? 32 : 16;
+
+ if (reg < nregs) {
+ *aa32_vfp_dreg(env, reg) = ldq_le_p(buf);
+ return 8;
+ }
+ if (arm_feature(env, ARM_FEATURE_NEON)) {
+ nregs += 16;
+ if (reg < nregs) {
+ uint64_t *q = aa32_vfp_qreg(env, reg - 32);
+ q[0] = ldq_le_p(buf);
+ q[1] = ldq_le_p(buf + 8);
+ return 16;
+ }
+ }
+ switch (reg - nregs) {
+ case 0:
+ env->vfp.xregs[ARM_VFP_FPSID] = ldl_p(buf);
+ return 4;
+ case 1:
+ vfp_set_fpscr(env, ldl_p(buf));
+ return 4;
+ case 2:
+ env->vfp.xregs[ARM_VFP_FPEXC] = ldl_p(buf) & (1 << 30);
+ return 4;
+ }
+ return 0;
+}
+
+/**
+ * arm_get/set_gdb_*: get/set a gdb register
+ * @env: the CPU state
+ * @buf: a buffer to copy to/from
+ * @reg: register number (offset from start of group)
+ *
+ * We return the number of bytes copied
+ */
+
+static int arm_gdb_get_sysreg(CPUARMState *env, GByteArray *buf, int reg)
+{
+ ARMCPU *cpu = env_archcpu(env);
+ const ARMCPRegInfo *ri;
+ uint32_t key;
+
+ key = cpu->dyn_sysreg_xml.data.cpregs.keys[reg];
+ ri = get_arm_cp_reginfo(cpu->cp_regs, key);
+ if (ri) {
+ if (cpreg_field_is_64bit(ri)) {
+ return gdb_get_reg64(buf, (uint64_t)read_raw_cp_reg(env, ri));
+ } else {
+ return gdb_get_reg32(buf, (uint32_t)read_raw_cp_reg(env, ri));
+ }
+ }
+ return 0;
+}
+
+static int arm_gdb_set_sysreg(CPUARMState *env, uint8_t *buf, int reg)
+{
+ return 0;
+}
+
static void arm_gen_one_xml_sysreg_tag(GString *s, DynamicGDBXMLInfo *dyn_xml,
ARMCPRegInfo *ri, uint32_t ri_key,
int bitsize, int regnum)
@@ -319,3 +412,40 @@ const char *arm_gdb_get_dynamic_xml(CPUState *cs, const char *xmlname)
}
return NULL;
}
+
+void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
+{
+ CPUState *cs = CPU(cpu);
+ CPUARMState *env = &cpu->env;
+
+ if (arm_feature(env, ARM_FEATURE_AARCH64)) {
+ /*
+ * 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)) {
+ gdb_register_coprocessor(cs, arm_gdb_get_svereg, arm_gdb_set_svereg,
+ arm_gen_dynamic_svereg_xml(cs, cs->gdb_num_regs),
+ "sve-registers.xml", 0);
+ } else {
+ gdb_register_coprocessor(cs, aarch64_fpu_gdb_get_reg,
+ aarch64_fpu_gdb_set_reg,
+ 34, "aarch64-fpu.xml", 0);
+ }
+#endif
+ } else if (arm_feature(env, ARM_FEATURE_NEON)) {
+ gdb_register_coprocessor(cs, vfp_gdb_get_reg, vfp_gdb_set_reg,
+ 51, "arm-neon.xml", 0);
+ } else if (cpu_isar_feature(aa32_simd_r32, cpu)) {
+ gdb_register_coprocessor(cs, vfp_gdb_get_reg, vfp_gdb_set_reg,
+ 35, "arm-vfp3.xml", 0);
+ } else if (cpu_isar_feature(aa32_vfp_simd, cpu)) {
+ gdb_register_coprocessor(cs, vfp_gdb_get_reg, vfp_gdb_set_reg,
+ 19, "arm-vfp.xml", 0);
+ }
+ gdb_register_coprocessor(cs, arm_gdb_get_sysreg, arm_gdb_set_sysreg,
+ arm_gen_dynamic_sysreg_xml(cs, cs->gdb_num_regs),
+ "system-registers.xml", 0);
+
+}
diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
index 251539ef799..596878666d7 100644
--- a/target/arm/gdbstub64.c
+++ b/target/arm/gdbstub64.c
@@ -17,7 +17,9 @@
* 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 "exec/gdbstub.h"
int aarch64_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
@@ -69,3 +71,141 @@ int aarch64_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
/* Unknown register. */
return 0;
}
+
+int aarch64_fpu_gdb_get_reg(CPUARMState *env, GByteArray *buf, int reg)
+{
+ 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_fpu_gdb_set_reg(CPUARMState *env, uint8_t *buf, int reg)
+{
+ switch (reg) {
+ case 0 ... 31:
+ /* 128 bit FP register */
+ {
+ uint64_t *q = aa64_vfp_qreg(env, reg);
+ 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 arm_gdb_get_svereg(CPUARMState *env, GByteArray *buf, int reg)
+{
+ ARMCPU *cpu = env_archcpu(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_zcr_len_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 arm_gdb_set_svereg(CPUARMState *env, uint8_t *buf, int reg)
+{
+ ARMCPU *cpu = env_archcpu(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;
+}
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 179038ad3da..deca59a3e61 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -12,7 +12,6 @@
#include "trace.h"
#include "cpu.h"
#include "internals.h"
-#include "exec/gdbstub.h"
#include "exec/helper-proto.h"
#include "qemu/host-utils.h"
#include "qemu/main-loop.h"
@@ -54,110 +53,6 @@ static bool get_phys_addr_lpae(CPUARMState *env, uint64_t address,
static void switch_mode(CPUARMState *env, int mode);
static int aa64_va_parameter_tbi(uint64_t tcr, ARMMMUIdx mmu_idx);
-static int vfp_gdb_get_reg(CPUARMState *env, GByteArray *buf, int reg)
-{
- ARMCPU *cpu = env_archcpu(env);
- int nregs = cpu_isar_feature(aa32_simd_r32, cpu) ? 32 : 16;
-
- /* VFP data registers are always little-endian. */
- if (reg < nregs) {
- return gdb_get_reg64(buf, *aa32_vfp_dreg(env, reg));
- }
- if (arm_feature(env, ARM_FEATURE_NEON)) {
- /* Aliases for Q regs. */
- nregs += 16;
- if (reg < nregs) {
- uint64_t *q = aa32_vfp_qreg(env, reg - 32);
- return gdb_get_reg128(buf, q[0], q[1]);
- }
- }
- switch (reg - nregs) {
- case 0:
- return gdb_get_reg32(buf, env->vfp.xregs[ARM_VFP_FPSID]);
- case 1:
- return gdb_get_reg32(buf, vfp_get_fpscr(env));
- case 2:
- return gdb_get_reg32(buf, env->vfp.xregs[ARM_VFP_FPEXC]);
- }
- return 0;
-}
-
-static int vfp_gdb_set_reg(CPUARMState *env, uint8_t *buf, int reg)
-{
- ARMCPU *cpu = env_archcpu(env);
- int nregs = cpu_isar_feature(aa32_simd_r32, cpu) ? 32 : 16;
-
- if (reg < nregs) {
- *aa32_vfp_dreg(env, reg) = ldq_le_p(buf);
- return 8;
- }
- if (arm_feature(env, ARM_FEATURE_NEON)) {
- nregs += 16;
- if (reg < nregs) {
- uint64_t *q = aa32_vfp_qreg(env, reg - 32);
- q[0] = ldq_le_p(buf);
- q[1] = ldq_le_p(buf + 8);
- return 16;
- }
- }
- switch (reg - nregs) {
- case 0:
- env->vfp.xregs[ARM_VFP_FPSID] = ldl_p(buf);
- return 4;
- case 1:
- vfp_set_fpscr(env, ldl_p(buf));
- return 4;
- case 2:
- env->vfp.xregs[ARM_VFP_FPEXC] = ldl_p(buf) & (1 << 30);
- return 4;
- }
- return 0;
-}
-
-static int aarch64_fpu_gdb_get_reg(CPUARMState *env, GByteArray *buf, int reg)
-{
- 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;
- }
-}
-
-static int aarch64_fpu_gdb_set_reg(CPUARMState *env, uint8_t *buf, int reg)
-{
- switch (reg) {
- case 0 ... 31:
- /* 128 bit FP register */
- {
- uint64_t *q = aa64_vfp_qreg(env, reg);
- 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;
- }
-}
-
static uint64_t raw_read(CPUARMState *env, const ARMCPRegInfo *ri)
{
assert(ri->fieldoffset);
@@ -217,134 +112,6 @@ static void write_raw_cp_reg(CPUARMState *env, const ARMCPRegInfo *ri,
}
}
-/**
- * arm_get/set_gdb_*: get/set a gdb register
- * @env: the CPU state
- * @buf: a buffer to copy to/from
- * @reg: register number (offset from start of group)
- *
- * We return the number of bytes copied
- */
-
-static int arm_gdb_get_sysreg(CPUARMState *env, GByteArray *buf, int reg)
-{
- ARMCPU *cpu = env_archcpu(env);
- const ARMCPRegInfo *ri;
- uint32_t key;
-
- key = cpu->dyn_sysreg_xml.data.cpregs.keys[reg];
- ri = get_arm_cp_reginfo(cpu->cp_regs, key);
- if (ri) {
- if (cpreg_field_is_64bit(ri)) {
- return gdb_get_reg64(buf, (uint64_t)read_raw_cp_reg(env, ri));
- } else {
- return gdb_get_reg32(buf, (uint32_t)read_raw_cp_reg(env, ri));
- }
- }
- return 0;
-}
-
-static int arm_gdb_set_sysreg(CPUARMState *env, uint8_t *buf, int reg)
-{
- return 0;
-}
-
-#ifdef TARGET_AARCH64
-static int arm_gdb_get_svereg(CPUARMState *env, GByteArray *buf, int reg)
-{
- ARMCPU *cpu = env_archcpu(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_zcr_len_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;
-}
-
-static int arm_gdb_set_svereg(CPUARMState *env, uint8_t *buf, int reg)
-{
- ARMCPU *cpu = env_archcpu(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;
-}
-#endif /* TARGET_AARCH64 */
-
static bool raw_accessors_invalid(const ARMCPRegInfo *ri)
{
/* Return true if the regdef would cause an assertion if you called
@@ -8711,44 +8478,6 @@ void register_cp_regs_for_features(ARMCPU *cpu)
#endif
}
-void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
-{
- CPUState *cs = CPU(cpu);
- CPUARMState *env = &cpu->env;
-
- if (arm_feature(env, ARM_FEATURE_AARCH64)) {
- /*
- * 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)) {
- gdb_register_coprocessor(cs, arm_gdb_get_svereg, arm_gdb_set_svereg,
- arm_gen_dynamic_svereg_xml(cs, cs->gdb_num_regs),
- "sve-registers.xml", 0);
- } else
-#endif
- {
- gdb_register_coprocessor(cs, aarch64_fpu_gdb_get_reg,
- aarch64_fpu_gdb_set_reg,
- 34, "aarch64-fpu.xml", 0);
- }
- } else if (arm_feature(env, ARM_FEATURE_NEON)) {
- gdb_register_coprocessor(cs, vfp_gdb_get_reg, vfp_gdb_set_reg,
- 51, "arm-neon.xml", 0);
- } else if (cpu_isar_feature(aa32_simd_r32, cpu)) {
- gdb_register_coprocessor(cs, vfp_gdb_get_reg, vfp_gdb_set_reg,
- 35, "arm-vfp3.xml", 0);
- } else if (cpu_isar_feature(aa32_vfp_simd, cpu)) {
- gdb_register_coprocessor(cs, vfp_gdb_get_reg, vfp_gdb_set_reg,
- 19, "arm-vfp.xml", 0);
- }
- gdb_register_coprocessor(cs, arm_gdb_get_sysreg, arm_gdb_set_sysreg,
- arm_gen_dynamic_sysreg_xml(cs, cs->gdb_num_regs),
- "system-registers.xml", 0);
-
-}
-
/* Sort alphabetically by type name, except for "any". */
static gint arm_cpu_list_compare(gconstpointer a, gconstpointer b)
{
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 4/5] target/arm: Don't put FPEXC and FPSID in org.gnu.gdb.arm.vfp XML
2021-09-21 16:28 [PATCH 0/5] target/arm: Support MVE in gdbstub Peter Maydell
` (2 preceding siblings ...)
2021-09-21 16:28 ` [PATCH 3/5] target/arm: Move gdbstub related code out of helper.c Peter Maydell
@ 2021-09-21 16:29 ` Peter Maydell
2021-09-22 3:30 ` Richard Henderson
2021-09-21 16:29 ` [PATCH 5/5] [RFC] target/arm: Advertise MVE to gdb when present Peter Maydell
4 siblings, 1 reply; 13+ messages in thread
From: Peter Maydell @ 2021-09-21 16:29 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Luis Machado
Currently we send VFP XML which includes D0..D15 or D0..D31, plus
FPSID, FPSCR and FPEXC. The upstream GDB tolerates this, but its
definition of this XML feature does not include FPSID or FPEXC. In
particular, for M-profile cores there are no FPSID or FPEXC
registers, so advertising those is wrong.
Move FPSID and FPEXC into their own bit of XML which we only send for
A and R profile cores. This brings our definition of the XML
org.gnu.gdb.arm.vfp feature into line with GDB's own (at least for
non-Neon cores...) and means we don't claim to have FPSID and FPEXC
on M-profile.
(It seems unlikely to me that any gdbstub users really care about
being able to look at FPEXC and FPSID; but we've supplied them to gdb
for a decade and it's not hard to keep doing so.)
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
configs/targets/aarch64-softmmu.mak | 2 +-
configs/targets/arm-linux-user.mak | 2 +-
configs/targets/arm-softmmu.mak | 2 +-
configs/targets/armeb-linux-user.mak | 2 +-
target/arm/gdbstub.c | 56 ++++++++++++++++++++--------
gdb-xml/arm-neon.xml | 2 -
gdb-xml/arm-vfp-sysregs.xml | 17 +++++++++
gdb-xml/arm-vfp.xml | 2 -
gdb-xml/arm-vfp3.xml | 2 -
9 files changed, 61 insertions(+), 26 deletions(-)
create mode 100644 gdb-xml/arm-vfp-sysregs.xml
diff --git a/configs/targets/aarch64-softmmu.mak b/configs/targets/aarch64-softmmu.mak
index 7703127674e..13d40b55e6d 100644
--- a/configs/targets/aarch64-softmmu.mak
+++ b/configs/targets/aarch64-softmmu.mak
@@ -1,5 +1,5 @@
TARGET_ARCH=aarch64
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_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-vfp-sysregs.xml gdb-xml/arm-neon.xml gdb-xml/arm-m-profile.xml
TARGET_NEED_FDT=y
diff --git a/configs/targets/arm-linux-user.mak b/configs/targets/arm-linux-user.mak
index e741ffd4d30..acecc339e38 100644
--- a/configs/targets/arm-linux-user.mak
+++ b/configs/targets/arm-linux-user.mak
@@ -1,6 +1,6 @@
TARGET_ARCH=arm
TARGET_SYSTBL_ABI=common,oabi
TARGET_SYSTBL=syscall.tbl
-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_XML_FILES= gdb-xml/arm-core.xml gdb-xml/arm-vfp.xml gdb-xml/arm-vfp3.xml gdb-xml/arm-vfp-sysregs.xml gdb-xml/arm-neon.xml gdb-xml/arm-m-profile.xml
TARGET_HAS_BFLT=y
CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
diff --git a/configs/targets/arm-softmmu.mak b/configs/targets/arm-softmmu.mak
index 84a98f48186..f6c95ba07a4 100644
--- a/configs/targets/arm-softmmu.mak
+++ b/configs/targets/arm-softmmu.mak
@@ -1,4 +1,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_XML_FILES= gdb-xml/arm-core.xml gdb-xml/arm-vfp.xml gdb-xml/arm-vfp3.xml gdb-xml/arm-vfp-sysregs.xml gdb-xml/arm-neon.xml gdb-xml/arm-m-profile.xml
TARGET_NEED_FDT=y
diff --git a/configs/targets/armeb-linux-user.mak b/configs/targets/armeb-linux-user.mak
index 255e44e8b0a..662c73d8fbd 100644
--- a/configs/targets/armeb-linux-user.mak
+++ b/configs/targets/armeb-linux-user.mak
@@ -2,6 +2,6 @@ TARGET_ARCH=arm
TARGET_SYSTBL_ABI=common,oabi
TARGET_SYSTBL=syscall.tbl
TARGET_WORDS_BIGENDIAN=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_XML_FILES= gdb-xml/arm-core.xml gdb-xml/arm-vfp.xml gdb-xml/arm-vfp3.xml gdb-xml/arm-vfp-sysregs.xml gdb-xml/arm-neon.xml gdb-xml/arm-m-profile.xml
TARGET_HAS_BFLT=y
CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index cbf156d192f..e0dcb33e325 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -144,11 +144,7 @@ static int vfp_gdb_get_reg(CPUARMState *env, GByteArray *buf, int reg)
}
switch (reg - nregs) {
case 0:
- return gdb_get_reg32(buf, env->vfp.xregs[ARM_VFP_FPSID]);
- case 1:
return gdb_get_reg32(buf, vfp_get_fpscr(env));
- case 2:
- return gdb_get_reg32(buf, env->vfp.xregs[ARM_VFP_FPEXC]);
}
return 0;
}
@@ -172,13 +168,31 @@ static int vfp_gdb_set_reg(CPUARMState *env, uint8_t *buf, int reg)
}
}
switch (reg - nregs) {
+ case 0:
+ vfp_set_fpscr(env, ldl_p(buf));
+ return 4;
+ }
+ return 0;
+}
+
+static int vfp_gdb_get_sysreg(CPUARMState *env, GByteArray *buf, int reg)
+{
+ switch (reg) {
+ case 0:
+ return gdb_get_reg32(buf, env->vfp.xregs[ARM_VFP_FPSID]);
+ case 1:
+ return gdb_get_reg32(buf, env->vfp.xregs[ARM_VFP_FPEXC]);
+ }
+ return 0;
+}
+
+static int vfp_gdb_set_sysreg(CPUARMState *env, uint8_t *buf, int reg)
+{
+ switch (reg) {
case 0:
env->vfp.xregs[ARM_VFP_FPSID] = ldl_p(buf);
return 4;
case 1:
- vfp_set_fpscr(env, ldl_p(buf));
- return 4;
- case 2:
env->vfp.xregs[ARM_VFP_FPEXC] = ldl_p(buf) & (1 << 30);
return 4;
}
@@ -434,15 +448,25 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
34, "aarch64-fpu.xml", 0);
}
#endif
- } else if (arm_feature(env, ARM_FEATURE_NEON)) {
- gdb_register_coprocessor(cs, vfp_gdb_get_reg, vfp_gdb_set_reg,
- 51, "arm-neon.xml", 0);
- } else if (cpu_isar_feature(aa32_simd_r32, cpu)) {
- gdb_register_coprocessor(cs, vfp_gdb_get_reg, vfp_gdb_set_reg,
- 35, "arm-vfp3.xml", 0);
- } else if (cpu_isar_feature(aa32_vfp_simd, cpu)) {
- gdb_register_coprocessor(cs, vfp_gdb_get_reg, vfp_gdb_set_reg,
- 19, "arm-vfp.xml", 0);
+ } else {
+ if (arm_feature(env, ARM_FEATURE_NEON)) {
+ gdb_register_coprocessor(cs, vfp_gdb_get_reg, vfp_gdb_set_reg,
+ 49, "arm-neon.xml", 0);
+ } else if (cpu_isar_feature(aa32_simd_r32, cpu)) {
+ gdb_register_coprocessor(cs, vfp_gdb_get_reg, vfp_gdb_set_reg,
+ 33, "arm-vfp3.xml", 0);
+ } else if (cpu_isar_feature(aa32_vfp_simd, cpu)) {
+ gdb_register_coprocessor(cs, vfp_gdb_get_reg, vfp_gdb_set_reg,
+ 17, "arm-vfp.xml", 0);
+ }
+ if (!arm_feature(env, ARM_FEATURE_M)) {
+ /*
+ * A and R profile have FP sysregs FPEXC and FPSID that we
+ * expose to gdb.
+ */
+ gdb_register_coprocessor(cs, vfp_gdb_get_sysreg, vfp_gdb_set_sysreg,
+ 2, "arm-vfp-sysregs.xml", 0);
+ }
}
gdb_register_coprocessor(cs, arm_gdb_get_sysreg, arm_gdb_set_sysreg,
arm_gen_dynamic_sysreg_xml(cs, cs->gdb_num_regs),
diff --git a/gdb-xml/arm-neon.xml b/gdb-xml/arm-neon.xml
index ce3ee03ec48..9dce0a996fc 100644
--- a/gdb-xml/arm-neon.xml
+++ b/gdb-xml/arm-neon.xml
@@ -82,7 +82,5 @@
<reg name="q14" bitsize="128" type="neon_q"/>
<reg name="q15" bitsize="128" type="neon_q"/>
- <reg name="fpsid" bitsize="32" type="int" group="float"/>
<reg name="fpscr" bitsize="32" type="int" group="float"/>
- <reg name="fpexc" bitsize="32" type="int" group="float"/>
</feature>
diff --git a/gdb-xml/arm-vfp-sysregs.xml b/gdb-xml/arm-vfp-sysregs.xml
new file mode 100644
index 00000000000..c4aa2721c8d
--- /dev/null
+++ b/gdb-xml/arm-vfp-sysregs.xml
@@ -0,0 +1,17 @@
+<?xml version="1.0"?>
+<!-- Copyright (C) 2021 Linaro Ltd.
+
+ Copying and distribution of this file, with or without modification,
+ are permitted in any medium without royalty provided the copyright
+ notice and this notice are preserved.
+
+ These are A/R profile VFP system registers. Debugger users probably
+ don't really care about these, but because we used to (incorrectly)
+ provide them to gdb in the org.gnu.gdb.arm.vfp XML we continue
+ to do so via this separate XML.
+ -->
+<!DOCTYPE feature SYSTEM "gdb-target.dtd">
+<feature name="org.qemu.gdb.arm.vfp-sysregs">
+ <reg name="fpsid" bitsize="32" type="int" group="float"/>
+ <reg name="fpexc" bitsize="32" type="int" group="float"/>
+</feature>
diff --git a/gdb-xml/arm-vfp.xml b/gdb-xml/arm-vfp.xml
index b20881e9a99..ebed5b3d573 100644
--- a/gdb-xml/arm-vfp.xml
+++ b/gdb-xml/arm-vfp.xml
@@ -23,7 +23,5 @@
<reg name="d14" bitsize="64" type="float"/>
<reg name="d15" bitsize="64" type="float"/>
- <reg name="fpsid" bitsize="32" type="int" group="float"/>
<reg name="fpscr" bitsize="32" type="int" group="float"/>
- <reg name="fpexc" bitsize="32" type="int" group="float"/>
</feature>
diff --git a/gdb-xml/arm-vfp3.xml b/gdb-xml/arm-vfp3.xml
index 227afd8017f..ef391c7144d 100644
--- a/gdb-xml/arm-vfp3.xml
+++ b/gdb-xml/arm-vfp3.xml
@@ -39,7 +39,5 @@
<reg name="d30" bitsize="64" type="float"/>
<reg name="d31" bitsize="64" type="float"/>
- <reg name="fpsid" bitsize="32" type="int" group="float"/>
<reg name="fpscr" bitsize="32" type="int" group="float"/>
- <reg name="fpexc" bitsize="32" type="int" group="float"/>
</feature>
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread