[Qemu-devel] [RFC] qemu-system-arm: cortex-m gdb registers

[Qemu-devel] [RFC] qemu-system-arm: cortex-m gdb registers

[Christopher Friedt]

At least for Cortex-M3 devices (but also M0, M0+, M4, ...), while
JTAG debugging using OpenOCD's built-in GDB server, the general purpose
register layout (i.e. `info reg' in GDB) should contain slightly more than
the usual ARM core registers.
    
The non-addressable core registers that appear in OpenOCD's listing are:
    
    r0, r1, r2, r3, r4, r5, r6, r7, r8, r9, r10, r11, r12, sp (r13), lr (r14),
    pc (r15), xpsr, primask, basepri, faultmask, and control.
    
These registers are well documented in the ARMv7M Architecture Reference
Manual.
    
This change addes preliminary support for those registers via a custom 
qXfer:features:read+ and XML response that follows the GNU convention
documented here [1] for org.gnu.gdb.arm.m-profile and
org.gnu.gdb.arm.m-system.
    
[1] https://goo.gl/NMxlC5

The default behaviour for non-cortex-m will be to mimic the original ARM
behaviour of referring the GDB client to an <xi:include href="arm-core.xml">.

A simple test environment for this patch is to compile and link the following
assembly fragment:

		.syntax unified
		.cpu cortex-m3

		.section .interp
		.word 0x20020000
		.word 0x9

		.text

		.global _start
		.thumb
		.thumb_func
		.type _start, %function
	_start:
		b _start
		.size _start, .-_start

Using the command below:

    arm-none-eabi-gcc -g -O0 -mthumb -march=armv7-m -Wl,-Ttext-segment,0 \
        -static -nostartfiles -o foo foo.S

Launch the resulting binary with qemu:

    qemu-system-arm -S -s -M netduino2 -nographic -kernel foo

Run gdb, attaching to the qemu's GDB server:

    arm-none-eabi-gdb foo
    (gdb) target remote localhost:1234
    Remote debugging using localhost:1234
    0x00000000 in _start ()
    (gdb) info all-registers
    r0             0x0	0
    r1             0x0	0
    r2             0x0	0
    r3             0x0	0
    r4             0x0	0
    r5             0x0	0
    r6             0x0	0
    r7             0x0	0
    r8             0x0	0
    r9             0x0	0
    r10            0x0	0
    r11            0x0	0
    r12            0x0	0
    sp             0xbffef7fc	0xbffef7fc
    lr             0x0	0
    pc             0x0	0x0 <_start>
    xpsr           0x40000000	1073741824
    msp            0xbffef7fc	0xbffef7fc
    psp            0x0	0x0 <_start>
    primask        0x0	0
    basepri        0x0	0
    faultmask      0x1	1
    control        0x0	0

The changes have been tested with other firmware images and also via Eclipse
Mars.

Christopher Friedt (1):
  qemu-system-arm: cortex-m gdb registers

 gdbstub.c            |  29 ++++---
 include/qom/cpu.h    |   1 +
 target-arm/cpu-qom.h |   4 +
 target-arm/cpu.c     |   5 +-
 target-arm/gdbstub.c | 215 +++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 241 insertions(+), 13 deletions(-)

-- 
2.5.4 (Apple Git-61)

[Qemu-devel] [RFC] qemu-system-arm: cortex-m gdb registers

[Christopher Friedt]

* allow overriding the default xml descriptor with gdb_xml_descriptor()
* read cortex-m registers using arm_cortexm_gdb_read_register()
* write cortex-m registers using arm_cortexm_gdb_write_register()
* correct the number of cortex-m core regs to 23

Signed-off-by: Christopher Friedt <chrisfriedt@gmail.com>
---
 gdbstub.c            |  29 ++++---
 include/qom/cpu.h    |   1 +
 target-arm/cpu-qom.h |   4 +
 target-arm/cpu.c     |   5 +-
 target-arm/gdbstub.c | 215 +++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 241 insertions(+), 13 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 9c29aa0..4684a4b 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -540,19 +540,24 @@ static const char *get_feature_xml(const char *p, const char **newp,
             GDBRegisterState *r;
             CPUState *cpu = first_cpu;
 
-            snprintf(target_xml, sizeof(target_xml),
-                     "<?xml version=\"1.0\"?>"
-                     "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">"
-                     "<target>"
-                     "<xi:include href=\"%s\"/>",
-                     cc->gdb_core_xml_file);
-
-            for (r = cpu->gdb_regs; r; r = r->next) {
-                pstrcat(target_xml, sizeof(target_xml), "<xi:include href=\"");
-                pstrcat(target_xml, sizeof(target_xml), r->xml);
-                pstrcat(target_xml, sizeof(target_xml), "\"/>");
+            if (cc->gdb_xml_descriptor) {
+                cc->gdb_xml_descriptor(cpu, target_xml, sizeof(target_xml));
+            } else {
+                snprintf(target_xml, sizeof(target_xml),
+                         "<?xml version=\"1.0\"?>"
+                         "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">"
+                         "<target>"
+                         "<xi:include href=\"%s\"/>",
+                         cc->gdb_core_xml_file);
+
+                for (r = cpu->gdb_regs; r; r = r->next) {
+                    pstrcat(target_xml, sizeof(target_xml),
+                        "<xi:include href=\"");
+                    pstrcat(target_xml, sizeof(target_xml), r->xml);
+                    pstrcat(target_xml, sizeof(target_xml), "\"/>");
+                }
+                pstrcat(target_xml, sizeof(target_xml), "</target>");
             }
-            pstrcat(target_xml, sizeof(target_xml), "</target>");
         }
         return target_xml;
     }
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 51a1323..7b8d875 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -152,6 +152,7 @@ typedef struct CPUClass {
     int (*handle_mmu_fault)(CPUState *cpu, vaddr address, int rw,
                             int mmu_index);
     hwaddr (*get_phys_page_debug)(CPUState *cpu, vaddr addr);
+    int (*gdb_xml_descriptor)(CPUState *cpu, char *buf, size_t len);
     int (*gdb_read_register)(CPUState *cpu, uint8_t *buf, int reg);
     int (*gdb_write_register)(CPUState *cpu, uint8_t *buf, int reg);
     void (*debug_excp_handler)(CPUState *cpu);
diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
index 25fb1ce..eafae6b 100644
--- a/target-arm/cpu-qom.h
+++ b/target-arm/cpu-qom.h
@@ -221,6 +221,10 @@ hwaddr arm_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
 int arm_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
 int arm_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
 
+int arm_cortexm_gdb_xml_descriptor(CPUState *cpu, char *buf, size_t len);
+int arm_cortexm_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
+int arm_cortexm_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
+
 /* Callback functions for the generic timer's timers. */
 void arm_gt_ptimer_cb(void *opaque);
 void arm_gt_vtimer_cb(void *opaque);
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 30739fc..e56b77a 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -553,7 +553,6 @@ static void arm_cpu_post_init(Object *obj)
                                      &error_abort);
         }
     }
-
 }
 
 static void arm_cpu_finalizefn(Object *obj)
@@ -910,6 +909,10 @@ static void arm_v7m_class_init(ObjectClass *oc, void *data)
 #endif
 
     cc->cpu_exec_interrupt = arm_v7m_cpu_exec_interrupt;
+    cc->gdb_num_core_regs = 23;
+    cc->gdb_xml_descriptor = arm_cortexm_gdb_xml_descriptor;
+    cc->gdb_read_register = arm_cortexm_gdb_read_register;
+    cc->gdb_write_register = arm_cortexm_gdb_write_register;
 }
 
 static const ARMCPRegInfo cortexr5_cp_reginfo[] = {
diff --git a/target-arm/gdbstub.c b/target-arm/gdbstub.c
index 1c34396..eb39757 100644
--- a/target-arm/gdbstub.c
+++ b/target-arm/gdbstub.c
@@ -3,6 +3,7 @@
  *
  * Copyright (c) 2003-2005 Fabrice Bellard
  * Copyright (c) 2013 SUSE LINUX Products GmbH
+ * Copyright (c) 2015 Christopher Friedt
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -100,3 +101,217 @@ int arm_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
     /* Unknown register.  */
     return 0;
 }
+
+enum cm_gdb_regnr {
+    R0, R1, R2, R3,
+    R4, R5, R6, R7,
+    R8, R9, R10, R11,
+    R12, SP, LR, PC,
+    XPSR, MSP, PSP, PRIMASK,
+    BASEPRI, FAULTMASK, CONTROL,
+};
+struct cm_gdb_regdesc {
+    const char *const name;
+    int bitsz;
+    int nr;
+    const char *const type;
+    const char *const grp;
+};
+static const struct cm_gdb_regdesc cm_gdb_regdesc[] = {
+    { "r0",        32,        R0, "int",      "general", },
+    { "r1",        32,        R1, "int",      "general", },
+    { "r2",        32,        R2, "int",      "general", },
+    { "r3",        32,        R3, "int",      "general", },
+    { "r4",        32,        R4, "int",      "general", },
+    { "r5",        32,        R5, "int",      "general", },
+    { "r6",        32,        R6, "int",      "general", },
+    { "r7",        32,        R7, "int",      "general", },
+    { "r8",        32,        R8, "int",      "general", },
+    { "r9",        32,        R9, "int",      "general", },
+    { "r10",       32,       R10, "int",      "general", },
+    { "r11",       32,       R11, "int",      "general", },
+    { "r12",       32,       R12, "int",      "general", },
+    { "sp",        32,        SP, "data_ptr", "general", },
+    { "lr",        32,        LR, "int",      "general", },
+    { "pc",        32,        PC, "code_ptr", "general", },
+    { "xpsr",      32,      XPSR, "int",      "general", },
+    { "msp",       32,       MSP, "data_ptr", "system",  },
+    { "psp",       32,       PSP, "data_ptr", "system",  },
+    { "primask",    1,   PRIMASK, "int8",     "system",  },
+    { "basepri",    8,   BASEPRI, "int8",     "system",  },
+    { "faultmask",  1, FAULTMASK, "int8",     "system",  },
+    { "control",    2,   CONTROL, "int8",     "system",  },
+};
+
+int arm_cortexm_gdb_xml_descriptor(CPUState *cs, char *buf, size_t len)
+{
+    int r;
+    int i;
+    ssize_t l;
+    char *d;
+    ARMCPU *cpu;
+    CPUARMState *env;
+
+    l = len;
+    d = buf;
+    cpu = ARM_CPU(cs);
+    env = &cpu->env;
+
+#define _update_buf_or_err() \
+    do { \
+        d += r; \
+        len -= r; \
+        if (r < 0 || l <= 0) { \
+            goto nospace; \
+        } \
+    } while (0);
+
+    r = snprintf(d, l,
+        "<?xml version=\"1.0\"?>"
+        "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">"
+        "<target version=\"1.0\">"
+        "<feature name=\"org.gnu.gdb.arm.m-profile\">"
+    );
+    _update_buf_or_err();
+
+    for (
+        i = 0;
+        i < sizeof(cm_gdb_regdesc) / sizeof(cm_gdb_regdesc[0]);
+        i++
+    ) {
+        r = snprintf(d, l,
+            "<reg name=\"%s\" bitsize=\"%d\" "
+            "regnum=\"%d\" type=\"%s\" group=\"%s\"/>",
+            cm_gdb_regdesc[i].name,
+            cm_gdb_regdesc[i].bitsz,
+            cm_gdb_regdesc[i].nr,
+            cm_gdb_regdesc[i].type,
+            cm_gdb_regdesc[i].grp
+        );
+        _update_buf_or_err();
+
+        if (XPSR == i) {
+            r = snprintf(d, l,
+                "</feature>"
+                "<feature name=\"org.gnu.gdb.arm.m-system\">"
+            );
+            _update_buf_or_err();
+        }
+    }
+
+    r = snprintf(d, l,
+        "</feature>"
+        "</target>"
+    );
+    _update_buf_or_err();
+
+    r = 0;
+    goto out;
+
+nospace:
+    buf[0] = '\0';
+    r = -1;
+out:
+    return r;
+
+#undef _update_buf_or_err
+}
+
+#ifndef PSTATE_F
+#define PSTATE_F (1U << 6)
+#endif
+#ifndef PSTATE_I
+#define PSTATE_I (1U << 7)
+#endif
+
+int arm_cortexm_gdb_read_register(CPUState *cs, uint8_t *mem_buf, int n)
+{
+    ARMCPU *cpu = ARM_CPU(cs);
+    CPUARMState *env = &cpu->env;
+
+    if (n < XPSR) {
+        /* Core integer register.  */
+        return gdb_get_reg32(mem_buf, env->regs[n]);
+    }
+
+    switch (n) {
+    case XPSR:
+        return gdb_get_reg32(mem_buf, xpsr_read(env));
+    case MSP:
+        return gdb_get_reg32(mem_buf,
+            env->v7m.current_sp ? env->v7m.other_sp : env->regs[SP]);
+    case PSP:
+        return gdb_get_reg32(mem_buf,
+            env->v7m.current_sp ? env->regs[SP] : env->v7m.other_sp);
+    case PRIMASK:
+        return gdb_get_reg8(mem_buf, !!(env->daif & PSTATE_I));
+    case BASEPRI:
+        return gdb_get_reg8(mem_buf, env->v7m.basepri);
+    case FAULTMASK:
+        return gdb_get_reg8(mem_buf, !!(env->daif & PSTATE_F));
+    case CONTROL:
+        return gdb_get_reg8(mem_buf, env->v7m.control);
+    }
+    /* Unknown register.  */
+    return 0;
+}
+
+int arm_cortexm_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
+{
+    ARMCPU *cpu = ARM_CPU(cs);
+    CPUARMState *env = &cpu->env;
+    uint32_t tmp;
+
+    tmp = ldl_p(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.  */
+    if (n == 15) {
+        tmp &= ~1;
+    }
+
+    if (n < 16) {
+        /* Core integer register.  */
+        env->regs[n] = tmp;
+        return 4;
+    }
+    switch (n) {
+    case XPSR: return 4;
+    case MSP:
+        if (env->v7m.current_sp) {
+            env->v7m.other_sp = tmp;
+        } else {
+            env->regs[SP] = tmp;
+        }
+        return 4;
+    case PSP:
+        if (env->v7m.current_sp) {
+            env->regs[SP] = tmp;
+        } else {
+            env->v7m.other_sp = tmp;
+        }
+        return 4;
+    case PRIMASK:
+        if (tmp & 1) {
+            env->daif |= PSTATE_I;
+        } else {
+            env->daif &= ~PSTATE_I;
+        }
+        return 1;
+    case BASEPRI:
+        env->v7m.basepri = tmp & 0xff;
+        return 1;
+    case FAULTMASK:
+        if (tmp & 1) {
+            env->daif |= PSTATE_F;
+        } else {
+            env->daif &= ~PSTATE_F;
+        }
+        return 1;
+    case CONTROL:
+        env->v7m.control = tmp & 0x3;
+        return 1;
+    }
+    /* Unknown register.  */
+    return 0;
+}
-- 
2.5.4 (Apple Git-61)

Re: [Qemu-devel] [RFC] qemu-system-arm: cortex-m gdb registers

[Peter Maydell]

On 14 December 2015 at 06:36, Christopher Friedt <chrisfriedt@gmail.com> wrote:
> * allow overriding the default xml descriptor with gdb_xml_descriptor()
> * read cortex-m registers using arm_cortexm_gdb_read_register()
> * write cortex-m registers using arm_cortexm_gdb_write_register()
> * correct the number of cortex-m core regs to 23
>
> Signed-off-by: Christopher Friedt <chrisfriedt@gmail.com>
> ---
>  gdbstub.c            |  29 ++++---
>  include/qom/cpu.h    |   1 +
>  target-arm/cpu-qom.h |   4 +
>  target-arm/cpu.c     |   5 +-
>  target-arm/gdbstub.c | 215 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 241 insertions(+), 13 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index 9c29aa0..4684a4b 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -540,19 +540,24 @@ static const char *get_feature_xml(const char *p, const char **newp,
>              GDBRegisterState *r;
>              CPUState *cpu = first_cpu;
>
> -            snprintf(target_xml, sizeof(target_xml),
> -                     "<?xml version=\"1.0\"?>"
> -                     "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">"
> -                     "<target>"
> -                     "<xi:include href=\"%s\"/>",
> -                     cc->gdb_core_xml_file);
> -
> -            for (r = cpu->gdb_regs; r; r = r->next) {
> -                pstrcat(target_xml, sizeof(target_xml), "<xi:include href=\"");
> -                pstrcat(target_xml, sizeof(target_xml), r->xml);
> -                pstrcat(target_xml, sizeof(target_xml), "\"/>");
> +            if (cc->gdb_xml_descriptor) {
> +                cc->gdb_xml_descriptor(cpu, target_xml, sizeof(target_xml));
> +            } else {
> +                snprintf(target_xml, sizeof(target_xml),
> +                         "<?xml version=\"1.0\"?>"
> +                         "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">"
> +                         "<target>"
> +                         "<xi:include href=\"%s\"/>",
> +                         cc->gdb_core_xml_file);
> +
> +                for (r = cpu->gdb_regs; r; r = r->next) {
> +                    pstrcat(target_xml, sizeof(target_xml),
> +                        "<xi:include href=\"");
> +                    pstrcat(target_xml, sizeof(target_xml), r->xml);
> +                    pstrcat(target_xml, sizeof(target_xml), "\"/>");
> +                }
> +                pstrcat(target_xml, sizeof(target_xml), "</target>");

This patch seems to be creating a completely new method of
constructing the XML to send to gdb. Is it really not possible
to do this using the existing mechanisms we have for selecting
XML to send to gdb and handling the registers it implies?

thanks
-- PMM

Re: [Qemu-devel] [RFC] qemu-system-arm: cortex-m gdb registers

[Christopher Friedt]

On Mon, Dec 14, 2015 at 3:31 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> This patch seems to be creating a completely new method of
> constructing the XML to send to gdb. Is it really not possible
> to do this using the existing mechanisms we have for selecting
> XML to send to gdb and handling the registers it implies?

Hehehe... I only just saw the gdb-xml/ directory. I'll create a
gdb-xml/arm-cortex-m-core.xml then, and refactor some of the remaining
lines of code.

Thanks for the feedback ;-)

Re: [Qemu-devel] [RFC] qemu-system-arm: cortex-m gdb registers

[Peter Maydell]

On 14 December 2015 at 13:07, Christopher Friedt <chrisfriedt@gmail.com> wrote:
> On Mon, Dec 14, 2015 at 3:31 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> This patch seems to be creating a completely new method of
>> constructing the XML to send to gdb. Is it really not possible
>> to do this using the existing mechanisms we have for selecting
>> XML to send to gdb and handling the registers it implies?
>
> Hehehe... I only just saw the gdb-xml/ directory. I'll create a
> gdb-xml/arm-cortex-m-core.xml then, and refactor some of the remaining
> lines of code.

Note that our XML files are from gdb itself, so you should start
by checking whether gdb has a suitable Cortex-M xml file.

thanks
-- PMM

Re: [Qemu-devel] [RFC] qemu-system-arm: cortex-m gdb registers

[Christopher Friedt]

On Mon, Dec 14, 2015 at 8:14 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> Note that our XML files are from gdb itself, so you should start
> by checking whether gdb has a suitable Cortex-M xml file.

They do indeed. Thanks for the tip.

Re: [Qemu-devel] [RFC] qemu-system-arm: cortex-m gdb registers

[Christopher Friedt]

On Mon, Dec 14, 2015 at 8:16 AM, Christopher Friedt
<chrisfriedt@gmail.com> wrote:
> On Mon, Dec 14, 2015 at 8:14 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> Note that our XML files are from gdb itself, so you should start
>> by checking whether gdb has a suitable Cortex-M xml file.
>
> They do indeed. Thanks for the tip.

binutils-gdb arm-m-profile.xml: https://goo.gl/hpTye8
openocd armv7m.c: http://goo.gl/FFn56X

There are 2 (major) differences from what I've seen:

1) xpsr is regnum 25 instead of 16 (what OpenOCD uses), and I'm fine with that.
2) binutils-gdb does not specify anything for the
org.gnu.gdb.arm.m-system group of core registers in any xml file.

It also seems very clear that the binutils people and the openocd
people have diverged at some point in their assignment of regnum
values; in openocd, the registers are mostly all consecutive with
moderate reuse between cores, whereas in binutils-gdb, their are
occasional gaps and extensive reuse between cores. The differences
seem primarily technical, but it's unclear as to why binutils-gdb does
*not* include the m-system group of core registers.

The m-system group of core registers are *incredibly* useful, but I'm
also inclined not to clobber binutils-gdb's register numbering
convention.

I think it would be most ideal to append the crucial m-system
information directly [1] in arm-m-profile.xml from binutils-gdb (or
possibly declare it as an include [2]):

<feature name="org.gnu.gdb.arm.m-system">
  <reg name="msp" bitsize="32" type="data_ptr"/>
  <reg name="psp" bitsize="32" type="data_ptr"/>
  <reg name="primask" bitsize="1" type="int8"/>
  <reg name="basepri" bitsize="8" type="int8"/>
  <reg name="faultmask" bitsize="1" type="int8"/>
  <reg name="control" bitsize="3" type="int8"/>
</feature>

However, if the worry there is that it diverges from binutils-gdb,
then the next best solution would be to create a separate
arm-m-system.xml, and to append that to the cpu->gdb_reg linked list
in cortex_m3_initfn(), cortex_m4_initfn(), and any other m's [3].

Which solution would work best for qemu?

C

Re: [Qemu-devel] [RFC] qemu-system-arm: cortex-m gdb registers

[Peter Maydell]

On 14 December 2015 at 14:22, Christopher Friedt <chrisfriedt@gmail.com> wrote:
> On Mon, Dec 14, 2015 at 8:16 AM, Christopher Friedt
> <chrisfriedt@gmail.com> wrote:
>> On Mon, Dec 14, 2015 at 8:14 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> Note that our XML files are from gdb itself, so you should start
>>> by checking whether gdb has a suitable Cortex-M xml file.
>>
>> They do indeed. Thanks for the tip.
>
> binutils-gdb arm-m-profile.xml: https://goo.gl/hpTye8
> openocd armv7m.c: http://goo.gl/FFn56X
>
> There are 2 (major) differences from what I've seen:
>
> 1) xpsr is regnum 25 instead of 16 (what OpenOCD uses), and I'm fine with that.
> 2) binutils-gdb does not specify anything for the
> org.gnu.gdb.arm.m-system group of core registers in any xml file.
>
> It also seems very clear that the binutils people and the openocd
> people have diverged at some point in their assignment of regnum
> values; in openocd, the registers are mostly all consecutive with
> moderate reuse between cores, whereas in binutils-gdb, their are
> occasional gaps and extensive reuse between cores. The differences
> seem primarily technical, but it's unclear as to why binutils-gdb does
> *not* include the m-system group of core registers.

My guess would be because gdb is primarily thinking of itself
as a user-mode debugger, and system registers aren't accessible
from there. And/or "nobody asked for it".

> The m-system group of core registers are *incredibly* useful, but I'm
> also inclined not to clobber binutils-gdb's register numbering
> convention.
>
> I think it would be most ideal to append the crucial m-system
> information directly [1] in arm-m-profile.xml from binutils-gdb (or
> possibly declare it as an include [2]):
>
> <feature name="org.gnu.gdb.arm.m-system">
>   <reg name="msp" bitsize="32" type="data_ptr"/>
>   <reg name="psp" bitsize="32" type="data_ptr"/>
>   <reg name="primask" bitsize="1" type="int8"/>
>   <reg name="basepri" bitsize="8" type="int8"/>
>   <reg name="faultmask" bitsize="1" type="int8"/>
>   <reg name="control" bitsize="3" type="int8"/>
> </feature>
>
> However, if the worry there is that it diverges from binutils-gdb,
> then the next best solution would be to create a separate
> arm-m-system.xml, and to append that to the cpu->gdb_reg linked list
> in cortex_m3_initfn(), cortex_m4_initfn(), and any other m's [3].
>
> Which solution would work best for qemu?

I'd rather we didn't diverge from upstream gdb too. On the
other hand I'm not sure how much it matters if we all end up
using different XML to describe the same target hardware. It
would be nice to ask the gdb folks first though, maybe.

rth: do you know how this stuff works?

thanks
-- PMM

Re: [Qemu-devel] [RFC] qemu-system-arm: cortex-m gdb registers

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> On 14 December 2015 at 14:22, Christopher Friedt <chrisfriedt@gmail.com> wrote:
>> On Mon, Dec 14, 2015 at 8:16 AM, Christopher Friedt
>> <chrisfriedt@gmail.com> wrote:
>>> On Mon, Dec 14, 2015 at 8:14 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>>> Note that our XML files are from gdb itself, so you should start
>>>> by checking whether gdb has a suitable Cortex-M xml file.
>>>
>>> They do indeed. Thanks for the tip.
>>
>> binutils-gdb arm-m-profile.xml: https://goo.gl/hpTye8
>> openocd armv7m.c: http://goo.gl/FFn56X
>>
>> There are 2 (major) differences from what I've seen:
>>
>> 1) xpsr is regnum 25 instead of 16 (what OpenOCD uses), and I'm fine with that.
>> 2) binutils-gdb does not specify anything for the
>> org.gnu.gdb.arm.m-system group of core registers in any xml file.
>>
>> It also seems very clear that the binutils people and the openocd
>> people have diverged at some point in their assignment of regnum
>> values; in openocd, the registers are mostly all consecutive with
>> moderate reuse between cores, whereas in binutils-gdb, their are
>> occasional gaps and extensive reuse between cores. The differences
>> seem primarily technical, but it's unclear as to why binutils-gdb does
>> *not* include the m-system group of core registers.
>
> My guess would be because gdb is primarily thinking of itself
> as a user-mode debugger, and system registers aren't accessible
> from there. And/or "nobody asked for it".
>
>> The m-system group of core registers are *incredibly* useful, but I'm
>> also inclined not to clobber binutils-gdb's register numbering
>> convention.
>>
>> I think it would be most ideal to append the crucial m-system
>> information directly [1] in arm-m-profile.xml from binutils-gdb (or
>> possibly declare it as an include [2]):
>>
>> <feature name="org.gnu.gdb.arm.m-system">
>>   <reg name="msp" bitsize="32" type="data_ptr"/>
>>   <reg name="psp" bitsize="32" type="data_ptr"/>
>>   <reg name="primask" bitsize="1" type="int8"/>
>>   <reg name="basepri" bitsize="8" type="int8"/>
>>   <reg name="faultmask" bitsize="1" type="int8"/>
>>   <reg name="control" bitsize="3" type="int8"/>
>> </feature>
>>
>> However, if the worry there is that it diverges from binutils-gdb,
>> then the next best solution would be to create a separate
>> arm-m-system.xml, and to append that to the cpu->gdb_reg linked list
>> in cortex_m3_initfn(), cortex_m4_initfn(), and any other m's [3].
>>
>> Which solution would work best for qemu?
>
> I'd rather we didn't diverge from upstream gdb too. On the
> other hand I'm not sure how much it matters if we all end up
> using different XML to describe the same target hardware. It
> would be nice to ask the gdb folks first though, maybe.
>
> rth: do you know how this stuff works?

IIRC last time I played with this when adding aarch64 system registers
for debugging is the number is irrelevant to gdb, its all dependant on
what the stub sends. As long as the coprocessor get/set functions agree
on the order with the xml everything should be fine.

>
> thanks
> -- PMM


--
Alex Bennée

Re: [Qemu-devel] [RFC] qemu-system-arm: cortex-m gdb registers

[Christopher Friedt]

On Mon, Dec 14, 2015 at 10:56 AM, Alex Bennée <alex.bennee@linaro.org> wrote:
> IIRC last time I played with this when adding aarch64 system registers
> for debugging is the number is irrelevant to gdb, its all dependant on
> what the stub sends. As long as the coprocessor get/set functions agree
> on the order with the xml everything should be fine.

That's right - the question isn't really how get the two to
communicate, but really which registers numbers to choose and how to
choose them in such a way that it doesn't make anyone angry :-)

And Peter is likely right that we should ask the gdb folks input -
Peter, do you have someone particular in mind?

Re: [Qemu-devel] [RFC] qemu-system-arm: cortex-m gdb registers

[Christopher Friedt]

Just to update everyone, there is a thread on gdb-patches here [1]
that is awaiting consensus before a patch is submitted that we may
pull into qemu.

[1] https://goo.gl/FyUvQu

Re: [Qemu-devel] [RFC] qemu-system-arm: cortex-m gdb registers

[Peter Maydell]

On 16 December 2015 at 00:16, Christopher Friedt <chrisfriedt@gmail.com> wrote:
> Just to update everyone, there is a thread on gdb-patches here [1]
> that is awaiting consensus before a patch is submitted that we may
> pull into qemu.
>
> [1] https://goo.gl/FyUvQu

Thanks for following up with the GDB developers.

-- PMM