qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] target/arm: Use correct GDB XML for M-profile cores
@ 2020-05-07 13:47 Peter Maydell
  2020-05-07 14:08 ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 2+ messages in thread
From: Peter Maydell @ 2020-05-07 13:47 UTC (permalink / raw)
  To: qemu-arm, qemu-devel

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>
-- 
2.20.1



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

* Re: [PATCH] target/arm: Use correct GDB XML for M-profile cores
  2020-05-07 13:47 [PATCH] target/arm: Use correct GDB XML for M-profile cores Peter Maydell
@ 2020-05-07 14:08 ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 2+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-05-07 14:08 UTC (permalink / raw)
  To: Peter Maydell, qemu-arm, qemu-devel

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>


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

end of thread, other threads:[~2020-05-07 14:09 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-07 13:47 [PATCH] target/arm: Use correct GDB XML for M-profile cores Peter Maydell
2020-05-07 14:08 ` Philippe Mathieu-Daudé

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).