diff mbox series

[qemu,3/3] target/arm/gdbstub: Support reading M security extension registers from GDB

Message ID 167330628518.10497.13100425787268927786-2@git.sr.ht
State New
Headers show
Series [qemu,1/3] target/arm: Unify checking for M Main Extension in MRS/MSR | expand

Commit Message

~dreiss-meta Jan. 9, 2023, 11:05 p.m. UTC 
From: David Reiss <dreiss@meta.com>

Follows a fairly similar pattern to the existing special register debug
support.  Only reading is implemented, but it should be possible to
implement writes.

Signed-off-by: David Reiss <dreiss@meta.com>
---
 target/arm/cpu.h     |   4 +-
 target/arm/gdbstub.c | 149 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 152 insertions(+), 1 deletion(-)

Comments

Peter Maydell Jan. 17, 2023, 1:37 p.m. UTC | #1 
On Mon, 9 Jan 2023 at 23:18, ~dreiss-meta <dreiss-meta@git.sr.ht> wrote:
>
> From: David Reiss <dreiss@meta.com>
>
> Follows a fairly similar pattern to the existing special register debug
> support.  Only reading is implemented, but it should be possible to
> implement writes.
>
> Signed-off-by: David Reiss <dreiss@meta.com>

> +static struct v8m_stack_registers get_v8m_stack_registers(CPUARMState *env)
> +{
> +    uint32_t current_ss_msp;
> +    uint32_t current_ss_psp;
> +    if (v7m_using_psp(env)) {
> +        current_ss_msp = env->v7m.other_sp;
> +        current_ss_psp = env->regs[13];
> +    } else {
> +        current_ss_msp = env->regs[13];
> +        current_ss_psp = env->v7m.other_sp;
> +
> +    }
> +
> +    struct v8m_stack_registers ret;
> +    if (env->v7m.secure) {
> +        ret.msp_s = current_ss_msp;
> +        ret.psp_s = current_ss_psp;
> +        ret.msp_ns = env->v7m.other_ss_msp;
> +        ret.psp_ns = env->v7m.other_ss_psp;
> +    } else {
> +        ret.msp_s = env->v7m.other_ss_msp;
> +        ret.psp_s = env->v7m.other_ss_psp;
> +        ret.msp_ns = current_ss_msp;
> +        ret.psp_ns = current_ss_psp;
> +    }
> +
> +    return ret;
> +}

I wondered about using get_v7m_sp_ptr(), but this is fine I guess.

> +static int arm_gdb_get_m_secextreg(CPUARMState *env, GByteArray *buf, int reg)
> +{
> +    switch (reg) {
> +    case  0:  /* MSP_S */
> +        return gdb_get_reg32(buf, get_v8m_stack_registers(env).msp_s);
> +    case  1:  /* PSP_S */
> +        return gdb_get_reg32(buf, get_v8m_stack_registers(env).psp_s);
> +    case  2:  /* MSPLIM_S */
> +        return gdb_get_reg32(buf, env->v7m.msplim[M_REG_S]);
> +    case  3:  /* PSPLIM_S */
> +        return gdb_get_reg32(buf, env->v7m.psplim[M_REG_S]);
> +    case  4:  /* PRIMASK_S */
> +        return gdb_get_reg32(buf, env->v7m.primask[M_REG_S]);
> +    case  5:  /* BASEPRI_S */
> +        if (!arm_feature(env, ARM_FEATURE_M_MAIN)) {
> +            return 0;
> +        }
> +        return gdb_get_reg32(buf, env->v7m.basepri[M_REG_S]);
> +    case  6:  /* FAULTMASK_S */
> +        if (!arm_feature(env, ARM_FEATURE_M_MAIN)) {
> +            return 0;
> +        }
> +        return gdb_get_reg32(buf, env->v7m.faultmask[M_REG_S]);
> +    case  7:  /* CONTROL_S */
> +        return gdb_get_reg32(buf, env->v7m.control[M_REG_S]);
> +    case  8:  /* MSP_NS */
> +        return gdb_get_reg32(buf, get_v8m_stack_registers(env).msp_ns);
> +    case  9:  /* PSP_NS */
> +        return gdb_get_reg32(buf, get_v8m_stack_registers(env).psp_ns);
> +    case 10:  /* MSPLIM_NS */
> +        return gdb_get_reg32(buf, env->v7m.msplim[M_REG_NS]);
> +    case 11:  /* PSPLIM_NS */
> +        return gdb_get_reg32(buf, env->v7m.psplim[M_REG_NS]);
> +    case 12:  /* PRIMASK_NS */
> +        return gdb_get_reg32(buf, env->v7m.primask[M_REG_NS]);
> +    case 13:  /* BASEPRI_NS */
> +        if (!arm_feature(env, ARM_FEATURE_M_MAIN)) {
> +            return 0;
> +        }
> +        return gdb_get_reg32(buf, env->v7m.basepri[M_REG_NS]);
> +    case 14:  /* FAULTMASK_NS */
> +        if (!arm_feature(env, ARM_FEATURE_M_MAIN)) {
> +            return 0;
> +        }
> +        return gdb_get_reg32(buf, env->v7m.faultmask[M_REG_NS]);
> +    case 15:  /* CONTROL_NS */
> +        return gdb_get_reg32(buf, env->v7m.control[M_REG_NS]);
> +    }
> +
> +    return 0;
> +}
> +
> +static int arm_gdb_set_m_secextreg(CPUARMState *env, uint8_t *buf, int reg)
> +{
> +    /* TODO: Implement. */
> +    return 0;
> +}
> +
> +int arm_gen_dynamic_m_securereg_xml(CPUState *cs, int base_reg)
> +{
> +    ARMCPU *cpu = ARM_CPU(cs);
> +    GString *s = g_string_new(NULL);
> +    DynamicGDBXMLInfo *info = &cpu->dyn_m_securereg_xml;
> +    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");
> +
> +    g_autoptr(GArray) regs = g_array_new(false, true, sizeof(const char *));
> +    /*  0 */ g_array_append_str_literal(regs, "msp_s");
> +    /*  1 */ g_array_append_str_literal(regs, "psp_s");
> +    /*  2 */ g_array_append_str_literal(regs, "msplim_s");
> +    /*  3 */ g_array_append_str_literal(regs, "psplim_s");
> +    /*  4 */ g_array_append_str_literal(regs, "primask_s");
> +    /*  5 */ g_array_append_str_literal(regs, "basepri_s");
> +    /*  6 */ g_array_append_str_literal(regs, "faultmask_s");
> +    /*  7 */ g_array_append_str_literal(regs, "control_s");
> +    /*  8 */ g_array_append_str_literal(regs, "msp_ns");
> +    /*  9 */ g_array_append_str_literal(regs, "psp_ns");
> +    /* 10 */ g_array_append_str_literal(regs, "msplim_ns");
> +    /* 11 */ g_array_append_str_literal(regs, "psplim_ns");
> +    /* 12 */ g_array_append_str_literal(regs, "primask_ns");
> +    /* 13 */ g_array_append_str_literal(regs, "basepri_ns");
> +    /* 14 */ g_array_append_str_literal(regs, "faultmask_ns");
> +    /* 15 */ g_array_append_str_literal(regs, "control_ns");

In patch 1 you skip the registers that don't exist without
the main extension, but here you throw them all in regardless.
Why the difference ?

(If we do want to have the xml be fixed and not dependent on
whether the main extension exists, we don't need to runtime
generate it, but can instead use a fixed xml file, as we do with
eg arm-m-profile-mve.xml.)

> +
> +    for (int idx = 0; idx < regs->len; idx++) {
> +        const char *name = g_array_index(regs, const char *, idx);
> +        if (*name != '\0') {
> +            g_string_append_printf(s,
> +                        "<reg name=\"%s\" bitsize=\"32\" regnum=\"%d\"/>\n",
> +                        name, base_reg);
> +        }
> +        base_reg++;
> +    }
> +    info->num = regs->len;
> +
> +    g_string_append_printf(s, "</feature>");
> +    info->desc = g_string_free(s, false);
> +    return info->num;
> +}

thanks
-- PMM
David Reiss Jan. 17, 2023, 6:25 p.m. UTC | #2 
On 1/17/23 5:37 AM, Peter Maydell wrote:

> In patch 1 you skip the registers that don't exist without
> the main extension, but here you throw them all in regardless.
> Why the difference ?
Ah, yes.  This was an oversight.  I'm not sure if there are any
chips that support the security extension but not the main extension,
but it is technically possible.
diff mbox series

Patch

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 5cf86cf2d7..b975c7d7db 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -853,6 +853,7 @@  struct ArchCPU {
     DynamicGDBXMLInfo dyn_sysreg_xml;
     DynamicGDBXMLInfo dyn_svereg_xml;
     DynamicGDBXMLInfo dyn_m_systemreg_xml;
+    DynamicGDBXMLInfo dyn_m_securereg_xml;
 
     /* Timers used by the generic (architected) timer */
     QEMUTimer *gt_timer[NUM_GTIMERS];
@@ -1096,12 +1097,13 @@  int arm_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
 
 /*
  * Helpers to dynamically generates XML descriptions of the sysregs,
- * SVE registers, and M-profile system registers.
+ * SVE registers, M-profile system, and M-profile secure extension registers.
  * Returns the number of registers in each set.
  */
 int arm_gen_dynamic_sysreg_xml(CPUState *cpu, int base_reg);
 int arm_gen_dynamic_svereg_xml(CPUState *cpu, int base_reg);
 int arm_gen_dynamic_m_systemreg_xml(CPUState *cpu, int base_reg);
+int arm_gen_dynamic_m_securereg_xml(CPUState *cpu, int base_reg);
 
 /* Returns the dynamically generated XML for the gdb stub.
  * Returns a pointer to the XML contents for the specified XML file or NULL
diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index 4456892e91..2bf5a63534 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -437,6 +437,143 @@  int arm_gen_dynamic_m_systemreg_xml(CPUState *cs, int base_reg)
     return info->num;
 }
 
+struct v8m_stack_registers {
+    uint32_t msp_s;
+    uint32_t psp_s;
+    uint32_t msp_ns;
+    uint32_t psp_ns;
+};
+
+static struct v8m_stack_registers get_v8m_stack_registers(CPUARMState *env)
+{
+    uint32_t current_ss_msp;
+    uint32_t current_ss_psp;
+    if (v7m_using_psp(env)) {
+        current_ss_msp = env->v7m.other_sp;
+        current_ss_psp = env->regs[13];
+    } else {
+        current_ss_msp = env->regs[13];
+        current_ss_psp = env->v7m.other_sp;
+
+    }
+
+    struct v8m_stack_registers ret;
+    if (env->v7m.secure) {
+        ret.msp_s = current_ss_msp;
+        ret.psp_s = current_ss_psp;
+        ret.msp_ns = env->v7m.other_ss_msp;
+        ret.psp_ns = env->v7m.other_ss_psp;
+    } else {
+        ret.msp_s = env->v7m.other_ss_msp;
+        ret.psp_s = env->v7m.other_ss_psp;
+        ret.msp_ns = current_ss_msp;
+        ret.psp_ns = current_ss_psp;
+    }
+
+    return ret;
+}
+
+static int arm_gdb_get_m_secextreg(CPUARMState *env, GByteArray *buf, int reg)
+{
+    switch (reg) {
+    case  0:  /* MSP_S */
+        return gdb_get_reg32(buf, get_v8m_stack_registers(env).msp_s);
+    case  1:  /* PSP_S */
+        return gdb_get_reg32(buf, get_v8m_stack_registers(env).psp_s);
+    case  2:  /* MSPLIM_S */
+        return gdb_get_reg32(buf, env->v7m.msplim[M_REG_S]);
+    case  3:  /* PSPLIM_S */
+        return gdb_get_reg32(buf, env->v7m.psplim[M_REG_S]);
+    case  4:  /* PRIMASK_S */
+        return gdb_get_reg32(buf, env->v7m.primask[M_REG_S]);
+    case  5:  /* BASEPRI_S */
+        if (!arm_feature(env, ARM_FEATURE_M_MAIN)) {
+            return 0;
+        }
+        return gdb_get_reg32(buf, env->v7m.basepri[M_REG_S]);
+    case  6:  /* FAULTMASK_S */
+        if (!arm_feature(env, ARM_FEATURE_M_MAIN)) {
+            return 0;
+        }
+        return gdb_get_reg32(buf, env->v7m.faultmask[M_REG_S]);
+    case  7:  /* CONTROL_S */
+        return gdb_get_reg32(buf, env->v7m.control[M_REG_S]);
+    case  8:  /* MSP_NS */
+        return gdb_get_reg32(buf, get_v8m_stack_registers(env).msp_ns);
+    case  9:  /* PSP_NS */
+        return gdb_get_reg32(buf, get_v8m_stack_registers(env).psp_ns);
+    case 10:  /* MSPLIM_NS */
+        return gdb_get_reg32(buf, env->v7m.msplim[M_REG_NS]);
+    case 11:  /* PSPLIM_NS */
+        return gdb_get_reg32(buf, env->v7m.psplim[M_REG_NS]);
+    case 12:  /* PRIMASK_NS */
+        return gdb_get_reg32(buf, env->v7m.primask[M_REG_NS]);
+    case 13:  /* BASEPRI_NS */
+        if (!arm_feature(env, ARM_FEATURE_M_MAIN)) {
+            return 0;
+        }
+        return gdb_get_reg32(buf, env->v7m.basepri[M_REG_NS]);
+    case 14:  /* FAULTMASK_NS */
+        if (!arm_feature(env, ARM_FEATURE_M_MAIN)) {
+            return 0;
+        }
+        return gdb_get_reg32(buf, env->v7m.faultmask[M_REG_NS]);
+    case 15:  /* CONTROL_NS */
+        return gdb_get_reg32(buf, env->v7m.control[M_REG_NS]);
+    }
+
+    return 0;
+}
+
+static int arm_gdb_set_m_secextreg(CPUARMState *env, uint8_t *buf, int reg)
+{
+    /* TODO: Implement. */
+    return 0;
+}
+
+int arm_gen_dynamic_m_securereg_xml(CPUState *cs, int base_reg)
+{
+    ARMCPU *cpu = ARM_CPU(cs);
+    GString *s = g_string_new(NULL);
+    DynamicGDBXMLInfo *info = &cpu->dyn_m_securereg_xml;
+    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");
+
+    g_autoptr(GArray) regs = g_array_new(false, true, sizeof(const char *));
+    /*  0 */ g_array_append_str_literal(regs, "msp_s");
+    /*  1 */ g_array_append_str_literal(regs, "psp_s");
+    /*  2 */ g_array_append_str_literal(regs, "msplim_s");
+    /*  3 */ g_array_append_str_literal(regs, "psplim_s");
+    /*  4 */ g_array_append_str_literal(regs, "primask_s");
+    /*  5 */ g_array_append_str_literal(regs, "basepri_s");
+    /*  6 */ g_array_append_str_literal(regs, "faultmask_s");
+    /*  7 */ g_array_append_str_literal(regs, "control_s");
+    /*  8 */ g_array_append_str_literal(regs, "msp_ns");
+    /*  9 */ g_array_append_str_literal(regs, "psp_ns");
+    /* 10 */ g_array_append_str_literal(regs, "msplim_ns");
+    /* 11 */ g_array_append_str_literal(regs, "psplim_ns");
+    /* 12 */ g_array_append_str_literal(regs, "primask_ns");
+    /* 13 */ g_array_append_str_literal(regs, "basepri_ns");
+    /* 14 */ g_array_append_str_literal(regs, "faultmask_ns");
+    /* 15 */ g_array_append_str_literal(regs, "control_ns");
+
+    for (int idx = 0; idx < regs->len; idx++) {
+        const char *name = g_array_index(regs, const char *, idx);
+        if (*name != '\0') {
+            g_string_append_printf(s,
+                        "<reg name=\"%s\" bitsize=\"32\" regnum=\"%d\"/>\n",
+                        name, base_reg);
+        }
+        base_reg++;
+    }
+    info->num = regs->len;
+
+    g_string_append_printf(s, "</feature>");
+    info->desc = g_string_free(s, false);
+    return info->num;
+}
+
 struct TypeSize {
     const char *gdb_type;
     int  size;
@@ -567,6 +704,8 @@  const char *arm_gdb_get_dynamic_xml(CPUState *cs, const char *xmlname)
         return cpu->dyn_svereg_xml.desc;
     } else if (strcmp(xmlname, "arm-m-system.xml") == 0) {
         return cpu->dyn_m_systemreg_xml.desc;
+    } else if (strcmp(xmlname, "arm-m-secext.xml") == 0) {
+        return cpu->dyn_m_securereg_xml.desc;
     }
     return NULL;
 }
@@ -618,6 +757,16 @@  void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
                                      arm_gen_dynamic_m_systemreg_xml(
                                          cs, cs->gdb_num_regs),
                                      "arm-m-system.xml", 0);
+            if (arm_feature(env, ARM_FEATURE_V8) &&
+                    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_securereg_xml(
+                                             cs, cs->gdb_num_regs),
+                                         "arm-m-secext.xml", 0);
+
+            }
         }
     }
     if (cpu_isar_feature(aa32_mve, cpu)) {