diff mbox series

[v2,14/14] target/arm: Implement gdbstub m-profile systemreg and secext

Message ID 20230221021951.453601-15-richard.henderson@linaro.org
State Superseded
Headers show
Series target/arm: gdbstub cleanups and additions | expand

Commit Message

Richard Henderson Feb. 21, 2023, 2:19 a.m. UTC 
The upstream gdb xml only implements {MSP,PSP}{,_NS,S}, but
go ahead and implement the other system registers as well.

Since there is significant overlap between the two, implement
them with common code.  The only exception is the systemreg
view of CONTROL, which merges the banked bits as per MRS.

Signed-off-by: David Reiss <dreiss@meta.com>
[rth: Substatial rewrite using enumerator and shared code.]
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/cpu.h     |   2 +
 target/arm/gdbstub.c | 194 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 196 insertions(+)

Comments

Philippe Mathieu-Daudé Feb. 21, 2023, 7:32 a.m. UTC | #1 
On 21/2/23 03:19, Richard Henderson wrote:
> The upstream gdb xml only implements {MSP,PSP}{,_NS,S}, but
> go ahead and implement the other system registers as well.
> 
> Since there is significant overlap between the two, implement
> them with common code.  The only exception is the systemreg
> view of CONTROL, which merges the banked bits as per MRS.
> 
> Signed-off-by: David Reiss <dreiss@meta.com>
> [rth: Substatial rewrite using enumerator and shared code.]
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/arm/cpu.h     |   2 +
>   target/arm/gdbstub.c | 194 +++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 196 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Peter Maydell Feb. 21, 2023, 5:25 p.m. UTC | #2 
On Tue, 21 Feb 2023 at 02:21, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> The upstream gdb xml only implements {MSP,PSP}{,_NS,S}, but
> go ahead and implement the other system registers as well.
>
> Since there is significant overlap between the two, implement
> them with common code.  The only exception is the systemreg
> view of CONTROL, which merges the banked bits as per MRS.
>
> Signed-off-by: David Reiss <dreiss@meta.com>
> [rth: Substatial rewrite using enumerator and shared code.]
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/cpu.h     |   2 +
>  target/arm/gdbstub.c | 194 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 196 insertions(+)
>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 059fe62eaa..6e97a256fb 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -869,6 +869,8 @@ struct ArchCPU {
>
>      DynamicGDBXMLInfo dyn_sysreg_xml;
>      DynamicGDBXMLInfo dyn_svereg_xml;
> +    DynamicGDBXMLInfo dyn_m_systemreg_xml;
> +    DynamicGDBXMLInfo dyn_m_secextreg_xml;
>
>      /* Timers used by the generic (architected) timer */
>      QEMUTimer *gt_timer[NUM_GTIMERS];
> diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
> index 062c8d447a..fef53e4ef5 100644
> --- a/target/arm/gdbstub.c
> +++ b/target/arm/gdbstub.c
> @@ -322,6 +322,180 @@ static int arm_gen_dynamic_sysreg_xml(CPUState *cs, int base_reg)
>      return cpu->dyn_sysreg_xml.num;
>  }
>
> +typedef enum {
> +    M_SYSREG_MSP,
> +    M_SYSREG_PSP,
> +    M_SYSREG_PRIMASK,
> +    M_SYSREG_CONTROL,
> +    M_SYSREG_BASEPRI,
> +    M_SYSREG_FAULTMASK,
> +    M_SYSREG_MSPLIM,
> +    M_SYSREG_PSPLIM,
> +} MProfileSysreg;
> +
> +static const struct {
> +    const char *name;
> +    int feature;
> +} m_sysreg_def[] = {
> +    [M_SYSREG_MSP] = { "msp", ARM_FEATURE_M },
> +    [M_SYSREG_PSP] = { "psp", ARM_FEATURE_M },
> +    [M_SYSREG_PRIMASK] = { "primask", ARM_FEATURE_M },
> +    [M_SYSREG_CONTROL] = { "control", ARM_FEATURE_M },
> +    [M_SYSREG_BASEPRI] = { "basepri", ARM_FEATURE_M_MAIN },
> +    [M_SYSREG_FAULTMASK] = { "faultmask", ARM_FEATURE_M_MAIN },
> +    [M_SYSREG_MSPLIM] = { "msplim", ARM_FEATURE_V8 },
> +    [M_SYSREG_PSPLIM] = { "psplim", ARM_FEATURE_V8 },
> +};
> +
> +static uint32_t *m_sysreg_ptr(CPUARMState *env, MProfileSysreg reg, bool sec)
> +{
> +    uint32_t *ptr;
> +
> +    switch (reg) {
> +    case M_SYSREG_MSP:
> +        ptr = arm_v7m_get_sp_ptr(env, sec, false, true);
> +        break;
> +    case M_SYSREG_PSP:
> +        ptr = arm_v7m_get_sp_ptr(env, sec, true, true);
> +        break;
> +    case M_SYSREG_MSPLIM:
> +        ptr = &env->v7m.msplim[sec];
> +        break;
> +    case M_SYSREG_PSPLIM:
> +        ptr = &env->v7m.psplim[sec];
> +        break;
> +    case M_SYSREG_PRIMASK:
> +        ptr = &env->v7m.primask[sec];
> +        break;
> +    case M_SYSREG_BASEPRI:
> +        ptr = &env->v7m.basepri[sec];
> +        break;
> +    case M_SYSREG_FAULTMASK:
> +        ptr = &env->v7m.faultmask[sec];
> +        break;
> +    case M_SYSREG_CONTROL:
> +        ptr = &env->v7m.control[sec];
> +        break;
> +    default:
> +        return NULL;
> +    }
> +    return arm_feature(env, m_sysreg_def[reg].feature) ? ptr : NULL;
> +}
> +
> +static int m_sysreg_get(CPUARMState *env, GByteArray *buf,
> +                        MProfileSysreg reg, bool secure)
> +{
> +    uint32_t *ptr = m_sysreg_ptr(env, reg, secure);
> +
> +    if (ptr == NULL) {
> +        return 0;
> +    }
> +    return gdb_get_reg32(buf, *ptr);
> +}
> +
> +static int m_sysreg_set(CPUARMState *env, uint8_t *buf,
> +                        MProfileSysreg reg, bool secure)
> +{
> +    uint32_t *ptr = m_sysreg_ptr(env, reg, secure);
> +
> +    if (ptr == NULL) {
> +        return 0;
> +    }
> +    *ptr = ldl_p(buf);
> +    return 4;
> +}
> +
> +static int arm_gdb_get_m_systemreg(CPUARMState *env, GByteArray *buf, int reg)
> +{
> +    /*
> +     * Here, we emulate MRS instruction, where CONTROL has a mix of
> +     * banked and non-banked bits.
> +     */
> +    if (reg == M_SYSREG_CONTROL) {
> +        return gdb_get_reg32(buf, arm_v7m_mrs_control(env, env->v7m.secure));
> +    }
> +    return m_sysreg_get(env, buf, reg, env->v7m.secure);
> +}
> +
> +static int arm_gdb_set_m_systemreg(CPUARMState *env, uint8_t *buf, int reg)
> +{
> +    /* Control is a mix of banked and non-banked bits -- disallow write. */
> +    if (reg == M_SYSREG_CONTROL) {
> +        return 0;
> +    }

You also want to enforce the RES0 bits on registers like
PSPLIM, MSPLIM, FAULTMASK, PSP, MSP, if you're going to
implement writes. Effectively you really end up wanting to
get helper_v7m_msr to do the work for you.

> +    return m_sysreg_set(env, buf, reg, env->v7m.secure);
> +}

-- PMM
Richard Henderson Feb. 21, 2023, 5:33 p.m. UTC | #3 
On 2/21/23 07:25, Peter Maydell wrote:
> You also want to enforce the RES0 bits on registers like
> PSPLIM, MSPLIM, FAULTMASK, PSP, MSP, if you're going to
> implement writes. Effectively you really end up wanting to
> get helper_v7m_msr to do the work for you.

Ho hum.  I should have known it was more complicated than all that.
I should probably just drop the write portion of the patch again for now.


r~
diff mbox series

Patch

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 059fe62eaa..6e97a256fb 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -869,6 +869,8 @@  struct ArchCPU {
 
     DynamicGDBXMLInfo dyn_sysreg_xml;
     DynamicGDBXMLInfo dyn_svereg_xml;
+    DynamicGDBXMLInfo dyn_m_systemreg_xml;
+    DynamicGDBXMLInfo dyn_m_secextreg_xml;
 
     /* Timers used by the generic (architected) timer */
     QEMUTimer *gt_timer[NUM_GTIMERS];
diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index 062c8d447a..fef53e4ef5 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -322,6 +322,180 @@  static int arm_gen_dynamic_sysreg_xml(CPUState *cs, int base_reg)
     return cpu->dyn_sysreg_xml.num;
 }
 
+typedef enum {
+    M_SYSREG_MSP,
+    M_SYSREG_PSP,
+    M_SYSREG_PRIMASK,
+    M_SYSREG_CONTROL,
+    M_SYSREG_BASEPRI,
+    M_SYSREG_FAULTMASK,
+    M_SYSREG_MSPLIM,
+    M_SYSREG_PSPLIM,
+} MProfileSysreg;
+
+static const struct {
+    const char *name;
+    int feature;
+} m_sysreg_def[] = {
+    [M_SYSREG_MSP] = { "msp", ARM_FEATURE_M },
+    [M_SYSREG_PSP] = { "psp", ARM_FEATURE_M },
+    [M_SYSREG_PRIMASK] = { "primask", ARM_FEATURE_M },
+    [M_SYSREG_CONTROL] = { "control", ARM_FEATURE_M },
+    [M_SYSREG_BASEPRI] = { "basepri", ARM_FEATURE_M_MAIN },
+    [M_SYSREG_FAULTMASK] = { "faultmask", ARM_FEATURE_M_MAIN },
+    [M_SYSREG_MSPLIM] = { "msplim", ARM_FEATURE_V8 },
+    [M_SYSREG_PSPLIM] = { "psplim", ARM_FEATURE_V8 },
+};
+
+static uint32_t *m_sysreg_ptr(CPUARMState *env, MProfileSysreg reg, bool sec)
+{
+    uint32_t *ptr;
+
+    switch (reg) {
+    case M_SYSREG_MSP:
+        ptr = arm_v7m_get_sp_ptr(env, sec, false, true);
+        break;
+    case M_SYSREG_PSP:
+        ptr = arm_v7m_get_sp_ptr(env, sec, true, true);
+        break;
+    case M_SYSREG_MSPLIM:
+        ptr = &env->v7m.msplim[sec];
+        break;
+    case M_SYSREG_PSPLIM:
+        ptr = &env->v7m.psplim[sec];
+        break;
+    case M_SYSREG_PRIMASK:
+        ptr = &env->v7m.primask[sec];
+        break;
+    case M_SYSREG_BASEPRI:
+        ptr = &env->v7m.basepri[sec];
+        break;
+    case M_SYSREG_FAULTMASK:
+        ptr = &env->v7m.faultmask[sec];
+        break;
+    case M_SYSREG_CONTROL:
+        ptr = &env->v7m.control[sec];
+        break;
+    default:
+        return NULL;
+    }
+    return arm_feature(env, m_sysreg_def[reg].feature) ? ptr : NULL;
+}
+
+static int m_sysreg_get(CPUARMState *env, GByteArray *buf,
+                        MProfileSysreg reg, bool secure)
+{
+    uint32_t *ptr = m_sysreg_ptr(env, reg, secure);
+
+    if (ptr == NULL) {
+        return 0;
+    }
+    return gdb_get_reg32(buf, *ptr);
+}
+
+static int m_sysreg_set(CPUARMState *env, uint8_t *buf,
+                        MProfileSysreg reg, bool secure)
+{
+    uint32_t *ptr = m_sysreg_ptr(env, reg, secure);
+
+    if (ptr == NULL) {
+        return 0;
+    }
+    *ptr = ldl_p(buf);
+    return 4;
+}
+
+static int arm_gdb_get_m_systemreg(CPUARMState *env, GByteArray *buf, int reg)
+{
+    /*
+     * Here, we emulate MRS instruction, where CONTROL has a mix of
+     * banked and non-banked bits.
+     */
+    if (reg == M_SYSREG_CONTROL) {
+        return gdb_get_reg32(buf, arm_v7m_mrs_control(env, env->v7m.secure));
+    }
+    return m_sysreg_get(env, buf, reg, env->v7m.secure);
+}
+
+static int arm_gdb_set_m_systemreg(CPUARMState *env, uint8_t *buf, int reg)
+{
+    /* Control is a mix of banked and non-banked bits -- disallow write. */
+    if (reg == M_SYSREG_CONTROL) {
+        return 0;
+    }
+    return m_sysreg_set(env, buf, reg, env->v7m.secure);
+}
+
+static int arm_gen_dynamic_m_systemreg_xml(CPUState *cs, int orig_base_reg)
+{
+    ARMCPU *cpu = ARM_CPU(cs);
+    CPUARMState *env = &cpu->env;
+    GString *s = g_string_new(NULL);
+    int base_reg = orig_base_reg;
+    int i;
+
+    g_string_printf(s, "<?xml version=\"1.0\"?>");
+    g_string_append_printf(s, "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">");
+    g_string_append_printf(s, "<feature name=\"org.gnu.gdb.arm.m-system\">\n");
+
+    for (i = 0; i < ARRAY_SIZE(m_sysreg_def); i++) {
+        if (arm_feature(env, m_sysreg_def[i].feature)) {
+            g_string_append_printf(s,
+                "<reg name=\"%s\" bitsize=\"32\" regnum=\"%d\"/>\n",
+                m_sysreg_def[i].name, base_reg++);
+        }
+    }
+
+    g_string_append_printf(s, "</feature>");
+    cpu->dyn_m_systemreg_xml.desc = g_string_free(s, false);
+    cpu->dyn_m_systemreg_xml.num = base_reg - orig_base_reg;
+
+    return cpu->dyn_m_systemreg_xml.num;
+}
+
+#ifndef CONFIG_USER_ONLY
+/*
+ * For user-only, we see the non-secure registers via m_systemreg above.
+ * For secext, encode the non-secure view as even and secure view as odd.
+ */
+static int arm_gdb_get_m_secextreg(CPUARMState *env, GByteArray *buf, int reg)
+{
+    return m_sysreg_get(env, buf, reg >> 1, reg & 1);
+}
+
+static int arm_gdb_set_m_secextreg(CPUARMState *env, uint8_t *buf, int reg)
+{
+    return m_sysreg_set(env, buf, reg >> 1, reg & 1);
+}
+
+static int arm_gen_dynamic_m_secextreg_xml(CPUState *cs, int orig_base_reg)
+{
+    ARMCPU *cpu = ARM_CPU(cs);
+    GString *s = g_string_new(NULL);
+    int base_reg = orig_base_reg;
+    int i;
+
+    g_string_printf(s, "<?xml version=\"1.0\"?>");
+    g_string_append_printf(s, "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">");
+    g_string_append_printf(s, "<feature name=\"org.gnu.gdb.arm.secext\">\n");
+
+    for (i = 0; i < ARRAY_SIZE(m_sysreg_def); i++) {
+        g_string_append_printf(s,
+            "<reg name=\"%s_ns\" bitsize=\"32\" regnum=\"%d\"/>\n",
+            m_sysreg_def[i].name, base_reg++);
+        g_string_append_printf(s,
+            "<reg name=\"%s_s\" bitsize=\"32\" regnum=\"%d\"/>\n",
+            m_sysreg_def[i].name, base_reg++);
+    }
+
+    g_string_append_printf(s, "</feature>");
+    cpu->dyn_m_secextreg_xml.desc = g_string_free(s, false);
+    cpu->dyn_m_secextreg_xml.num = base_reg - orig_base_reg;
+
+    return cpu->dyn_m_secextreg_xml.num;
+}
+#endif
+
 const char *arm_gdb_get_dynamic_xml(CPUState *cs, const char *xmlname)
 {
     ARMCPU *cpu = ARM_CPU(cs);
@@ -330,6 +504,12 @@  const char *arm_gdb_get_dynamic_xml(CPUState *cs, const char *xmlname)
         return cpu->dyn_sysreg_xml.desc;
     } else if (strcmp(xmlname, "sve-registers.xml") == 0) {
         return cpu->dyn_svereg_xml.desc;
+    } else if (strcmp(xmlname, "arm-m-system.xml") == 0) {
+        return cpu->dyn_m_systemreg_xml.desc;
+#ifndef CONFIG_USER_ONLY
+    } else if (strcmp(xmlname, "arm-m-secext.xml") == 0) {
+        return cpu->dyn_m_secextreg_xml.desc;
+#endif
     }
     return NULL;
 }
@@ -389,4 +569,18 @@  void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
                              arm_gen_dynamic_sysreg_xml(cs, cs->gdb_num_regs),
                              "system-registers.xml", 0);
 
+    if (arm_feature(env, ARM_FEATURE_M)) {
+        gdb_register_coprocessor(cs,
+            arm_gdb_get_m_systemreg, arm_gdb_set_m_systemreg,
+            arm_gen_dynamic_m_systemreg_xml(cs, cs->gdb_num_regs),
+            "arm-m-system.xml", 0);
+#ifndef CONFIG_USER_ONLY
+        if (arm_feature(env, ARM_FEATURE_M_SECURITY)) {
+            gdb_register_coprocessor(cs,
+                arm_gdb_get_m_secextreg, arm_gdb_set_m_secextreg,
+                arm_gen_dynamic_m_secextreg_xml(cs, cs->gdb_num_regs),
+                "arm-m-secext.xml", 0);
+        }
+#endif
+    }
 }