| Message ID | 20200507134755.13997-1-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | target/arm: Use correct GDB XML for M-profile cores | expand |
On 5/7/20 3:47 PM, Peter Maydell wrote: > GDB's remote protocol requires M-profile cores to use the feature > name 'org.gnu.gdb.arm.m-profile' instead of the 'org.gnu.gdb.arm.core' > feature used for A- and R-profile cores. We weren't doing this, which > meant GDB treated our M-profile cores like A-profile ones. This mostly > doesn't matter, but for instance means that it doesn't correctly > handle backtraces where an M-profile exception frame is involved. > > Ship a copy of GDB's arm-m-profile.xml and use it on the M-profile > cores. The integer registers have the same offsets as the > arm-core.xml, but register 25 is the M-profile XPSR rather than the > A-profile CPSR, so we need to update arm_cpu_gdb_read_register() and > arm_cpu_gdb_write_register() to handle XSPR reads and writes. > > Fixes: https://bugs.launchpad.net/qemu/+bug/1877136 > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > configure | 4 ++-- > target/arm/cpu.c | 1 + > target/arm/gdbstub.c | 22 ++++++++++++++++++---- > gdb-xml/arm-m-profile.xml | 27 +++++++++++++++++++++++++++ > 4 files changed, 48 insertions(+), 6 deletions(-) > create mode 100644 gdb-xml/arm-m-profile.xml > > diff --git a/configure b/configure > index 0d69c360c0e..360e7e6a0a4 100755 > --- a/configure > +++ b/configure > @@ -7806,14 +7806,14 @@ case "$target_name" in > TARGET_SYSTBL_ABI=common,oabi > bflt="yes" > mttcg="yes" > - gdb_xml_files="arm-core.xml arm-vfp.xml arm-vfp3.xml arm-neon.xml" > + gdb_xml_files="arm-core.xml arm-vfp.xml arm-vfp3.xml arm-neon.xml arm-m-profile.xml" > ;; > aarch64|aarch64_be) > TARGET_ARCH=aarch64 > TARGET_BASE_ARCH=arm > bflt="yes" > mttcg="yes" > - gdb_xml_files="aarch64-core.xml aarch64-fpu.xml arm-core.xml arm-vfp.xml arm-vfp3.xml arm-neon.xml" > + gdb_xml_files="aarch64-core.xml aarch64-fpu.xml arm-core.xml arm-vfp.xml arm-vfp3.xml arm-neon.xml arm-m-profile.xml" > ;; > cris) > ;; > diff --git a/target/arm/cpu.c b/target/arm/cpu.c > index 5d64adfe76e..b386bbbfd12 100644 > --- a/target/arm/cpu.c > +++ b/target/arm/cpu.c > @@ -2179,6 +2179,7 @@ static void arm_v7m_class_init(ObjectClass *oc, void *data) > #endif > > cc->cpu_exec_interrupt = arm_v7m_cpu_exec_interrupt; > + cc->gdb_core_xml_file = "arm-m-profile.xml"; > } > > static const ARMCPRegInfo cortexr5_cp_reginfo[] = { > diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c > index 063551df234..ecfa88f8e60 100644 > --- a/target/arm/gdbstub.c > +++ b/target/arm/gdbstub.c > @@ -57,8 +57,12 @@ int arm_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n) > } > return gdb_get_reg32(mem_buf, 0); > case 25: > - /* CPSR */ > - return gdb_get_reg32(mem_buf, cpsr_read(env)); > + /* CPSR, or XPSR for M-profile */ > + if (arm_feature(env, ARM_FEATURE_M)) { > + return gdb_get_reg32(mem_buf, xpsr_read(env)); > + } else { > + return gdb_get_reg32(mem_buf, cpsr_read(env)); > + } > } > /* Unknown register. */ > return 0; > @@ -98,8 +102,18 @@ int arm_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n) > } > return 4; > case 25: > - /* CPSR */ > - cpsr_write(env, tmp, 0xffffffff, CPSRWriteByGDBStub); > + /* CPSR, or XPSR for M-profile */ > + if (arm_feature(env, ARM_FEATURE_M)) { > + /* > + * Don't allow writing to XPSR.Exception as it can cause > + * a transition into or out of handler mode (it's not > + * writeable via the MSR insn so this is a reasonable > + * restriction). Other fields are safe to update. > + */ > + xpsr_write(env, tmp, ~XPSR_EXCP); > + } else { > + cpsr_write(env, tmp, 0xffffffff, CPSRWriteByGDBStub); > + } > return 4; > } > /* Unknown register. */ > diff --git a/gdb-xml/arm-m-profile.xml b/gdb-xml/arm-m-profile.xml > new file mode 100644 > index 00000000000..5319d764eeb > --- /dev/null > +++ b/gdb-xml/arm-m-profile.xml > @@ -0,0 +1,27 @@ > +<?xml version="1.0"?> > +<!-- Copyright (C) 2010-2020 Free Software Foundation, Inc. > + > + 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. --> > + > +<!DOCTYPE feature SYSTEM "gdb-target.dtd"> > +<feature name="org.gnu.gdb.arm.m-profile"> > + <reg name="r0" bitsize="32"/> > + <reg name="r1" bitsize="32"/> > + <reg name="r2" bitsize="32"/> > + <reg name="r3" bitsize="32"/> > + <reg name="r4" bitsize="32"/> > + <reg name="r5" bitsize="32"/> > + <reg name="r6" bitsize="32"/> > + <reg name="r7" bitsize="32"/> > + <reg name="r8" bitsize="32"/> > + <reg name="r9" bitsize="32"/> > + <reg name="r10" bitsize="32"/> > + <reg name="r11" bitsize="32"/> > + <reg name="r12" bitsize="32"/> > + <reg name="sp" bitsize="32" type="data_ptr"/> > + <reg name="lr" bitsize="32"/> > + <reg name="pc" bitsize="32" type="code_ptr"/> > + <reg name="xpsr" bitsize="32" regnum="25"/> > +</feature> > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
diff --git a/configure b/configure index 0d69c360c0e..360e7e6a0a4 100755 --- a/configure +++ b/configure @@ -7806,14 +7806,14 @@ case "$target_name" in TARGET_SYSTBL_ABI=common,oabi bflt="yes" mttcg="yes" - gdb_xml_files="arm-core.xml arm-vfp.xml arm-vfp3.xml arm-neon.xml" + gdb_xml_files="arm-core.xml arm-vfp.xml arm-vfp3.xml arm-neon.xml arm-m-profile.xml" ;; aarch64|aarch64_be) TARGET_ARCH=aarch64 TARGET_BASE_ARCH=arm bflt="yes" mttcg="yes" - gdb_xml_files="aarch64-core.xml aarch64-fpu.xml arm-core.xml arm-vfp.xml arm-vfp3.xml arm-neon.xml" + gdb_xml_files="aarch64-core.xml aarch64-fpu.xml arm-core.xml arm-vfp.xml arm-vfp3.xml arm-neon.xml arm-m-profile.xml" ;; cris) ;; diff --git a/target/arm/cpu.c b/target/arm/cpu.c index 5d64adfe76e..b386bbbfd12 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -2179,6 +2179,7 @@ static void arm_v7m_class_init(ObjectClass *oc, void *data) #endif cc->cpu_exec_interrupt = arm_v7m_cpu_exec_interrupt; + cc->gdb_core_xml_file = "arm-m-profile.xml"; } static const ARMCPRegInfo cortexr5_cp_reginfo[] = { diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c index 063551df234..ecfa88f8e60 100644 --- a/target/arm/gdbstub.c +++ b/target/arm/gdbstub.c @@ -57,8 +57,12 @@ int arm_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n) } return gdb_get_reg32(mem_buf, 0); case 25: - /* CPSR */ - return gdb_get_reg32(mem_buf, cpsr_read(env)); + /* CPSR, or XPSR for M-profile */ + if (arm_feature(env, ARM_FEATURE_M)) { + return gdb_get_reg32(mem_buf, xpsr_read(env)); + } else { + return gdb_get_reg32(mem_buf, cpsr_read(env)); + } } /* Unknown register. */ return 0; @@ -98,8 +102,18 @@ int arm_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n) } return 4; case 25: - /* CPSR */ - cpsr_write(env, tmp, 0xffffffff, CPSRWriteByGDBStub); + /* CPSR, or XPSR for M-profile */ + if (arm_feature(env, ARM_FEATURE_M)) { + /* + * Don't allow writing to XPSR.Exception as it can cause + * a transition into or out of handler mode (it's not + * writeable via the MSR insn so this is a reasonable + * restriction). Other fields are safe to update. + */ + xpsr_write(env, tmp, ~XPSR_EXCP); + } else { + cpsr_write(env, tmp, 0xffffffff, CPSRWriteByGDBStub); + } return 4; } /* Unknown register. */ diff --git a/gdb-xml/arm-m-profile.xml b/gdb-xml/arm-m-profile.xml new file mode 100644 index 00000000000..5319d764eeb --- /dev/null +++ b/gdb-xml/arm-m-profile.xml @@ -0,0 +1,27 @@ +<?xml version="1.0"?> +<!-- Copyright (C) 2010-2020 Free Software Foundation, Inc. + + 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. --> + +<!DOCTYPE feature SYSTEM "gdb-target.dtd"> +<feature name="org.gnu.gdb.arm.m-profile"> + <reg name="r0" bitsize="32"/> + <reg name="r1" bitsize="32"/> + <reg name="r2" bitsize="32"/> + <reg name="r3" bitsize="32"/> + <reg name="r4" bitsize="32"/> + <reg name="r5" bitsize="32"/> + <reg name="r6" bitsize="32"/> + <reg name="r7" bitsize="32"/> + <reg name="r8" bitsize="32"/> + <reg name="r9" bitsize="32"/> + <reg name="r10" bitsize="32"/> + <reg name="r11" bitsize="32"/> + <reg name="r12" bitsize="32"/> + <reg name="sp" bitsize="32" type="data_ptr"/> + <reg name="lr" bitsize="32"/> + <reg name="pc" bitsize="32" type="code_ptr"/> + <reg name="xpsr" bitsize="32" regnum="25"/> +</feature>
GDB's remote protocol requires M-profile cores to use the feature name 'org.gnu.gdb.arm.m-profile' instead of the 'org.gnu.gdb.arm.core' feature used for A- and R-profile cores. We weren't doing this, which meant GDB treated our M-profile cores like A-profile ones. This mostly doesn't matter, but for instance means that it doesn't correctly handle backtraces where an M-profile exception frame is involved. Ship a copy of GDB's arm-m-profile.xml and use it on the M-profile cores. The integer registers have the same offsets as the arm-core.xml, but register 25 is the M-profile XPSR rather than the A-profile CPSR, so we need to update arm_cpu_gdb_read_register() and arm_cpu_gdb_write_register() to handle XSPR reads and writes. Fixes: https://bugs.launchpad.net/qemu/+bug/1877136 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- configure | 4 ++-- target/arm/cpu.c | 1 + target/arm/gdbstub.c | 22 ++++++++++++++++++---- gdb-xml/arm-m-profile.xml | 27 +++++++++++++++++++++++++++ 4 files changed, 48 insertions(+), 6 deletions(-) create mode 100644 gdb-xml/arm-m-profile.xml -- 2.20.1