Rebased and updated to use NULL as a sentinel. David Reiss (2): target/arm/gdbstub: Support reading M system registers from GDB target/arm/gdbstub: Support reading M security extension registers from GDB target/arm/cpu.h | 25 +++- target/arm/gdbstub.c | 241 +++++++++++++++++++++++++++++++++++++++ target/arm/m_helper.c | 29 ++--- tests/lcitool/libvirt-ci | 2 +- 4 files changed, 277 insertions(+), 20 deletions(-) -- 2.34.5 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. `v7m_mrs_control` was renamed `arm_v7m_mrs_control` and made non-static so this logic could be shared between the MRS instruction and the GDB stub. Signed-off-by: David Reiss <dreiss@meta.com> --- target/arm/cpu.h | 12 +++- target/arm/gdbstub.c | 125 +++++++++++++++++++++++++++++++++++++++ target/arm/m_helper.c | 6 +- tests/lcitool/libvirt-ci | 2 +- 4 files changed, 139 insertions(+), 6 deletions(-) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ struct ArchCPU { DynamicGDBXMLInfo dyn_sysreg_xml; DynamicGDBXMLInfo dyn_svereg_xml; + DynamicGDBXMLInfo dyn_m_systemreg_xml; /* Timers used by the generic (architected) timer */ QEMUTimer *gt_timer[NUM_GTIMERS]; @@ -XXX,XX +XXX,XX @@ int arm_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg); int arm_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg); /* - * Helpers to dynamically generates XML descriptions of the sysregs - * and SVE registers. Returns the number of registers in each set. + * Helpers to dynamically generate XML descriptions of the sysregs, + * SVE registers, and M-profile system 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); /* Returns the dynamically generated XML for the gdb stub. * Returns a pointer to the XML contents for the specified XML file or NULL @@ -XXX,XX +XXX,XX @@ FIELD(SVCR, ZA, 1, 1) FIELD(SMCR, LEN, 0, 4) FIELD(SMCR, FA64, 31, 1) +/* + * Read the CONTROL register as the MRS instruction would. + */ +uint32_t arm_v7m_mrs_control(CPUARMState *env, uint32_t secure); + /* Write a new value to v7m.exception, thus transitioning into or out * of Handler mode; this may result in a change of active stack pointer. */ diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/gdbstub.c +++ b/target/arm/gdbstub.c @@ -XXX,XX +XXX,XX @@ int arm_gen_dynamic_sysreg_xml(CPUState *cs, int base_reg) return cpu->dyn_sysreg_xml.num; } +/* + * Helper required because g_array_append_val is a macro + * that cannot handle string literals. + */ +static inline void g_array_append_str_literal(GArray *array, const char *str) +{ + g_array_append_val(array, str); +} + +static int arm_gdb_get_m_systemreg(CPUARMState *env, GByteArray *buf, int reg) +{ + /* NOTE: This implementation shares a lot of logic with v7m_mrs. */ + switch (reg) { + + /* + * NOTE: MSP and PSP technically don't exist if the secure extension + * is present (replaced by MSP_S, MSP_NS, PSP_S, PSP_NS). Similar for + * MSPLIM and PSPLIM. + * However, the MRS instruction is still allowed to read from MSP and PSP, + * and will return the value associated with the current security state. + * We replicate this behavior for the convenience of users, who will see + * GDB behave similarly to their assembly code, even if they are oblivious + * to the security extension. + */ + case 0: /* MSP */ + return gdb_get_reg32(buf, + v7m_using_psp(env) ? env->v7m.other_sp : env->regs[13]); + case 1: /* PSP */ + return gdb_get_reg32(buf, + v7m_using_psp(env) ? env->regs[13] : env->v7m.other_sp); + case 6: /* MSPLIM */ + if (!arm_feature(env, ARM_FEATURE_V8)) { + return 0; + } + return gdb_get_reg32(buf, env->v7m.msplim[env->v7m.secure]); + case 7: /* PSPLIM */ + if (!arm_feature(env, ARM_FEATURE_V8)) { + return 0; + } + return gdb_get_reg32(buf, env->v7m.psplim[env->v7m.secure]); + + /* + * NOTE: PRIMASK, BASEPRI, and FAULTMASK are defined a bit differently + * from the SP family, but have similar banking behavior. + */ + case 2: /* PRIMASK */ + return gdb_get_reg32(buf, env->v7m.primask[env->v7m.secure]); + case 3: /* BASEPRI */ + if (!arm_feature(env, ARM_FEATURE_M_MAIN)) { + return 0; + } + return gdb_get_reg32(buf, env->v7m.basepri[env->v7m.secure]); + case 4: /* FAULTMASK */ + if (!arm_feature(env, ARM_FEATURE_M_MAIN)) { + return 0; + } + return gdb_get_reg32(buf, env->v7m.faultmask[env->v7m.secure]); + + /* + * NOTE: CONTROL has a mix of banked and non-banked bits. We continue + * to emulate the MRS instruction. Unfortunately, this gives GDB no way + * to read the SFPA bit when the CPU is in a non-secure state. + */ + case 5: /* CONTROL */ + return gdb_get_reg32(buf, arm_v7m_mrs_control(env, env->v7m.secure)); + } + + return 0; +} + +static int arm_gdb_set_m_systemreg(CPUARMState *env, uint8_t *buf, int reg) +{ + /* TODO: Implement. */ + return 0; +} + +int arm_gen_dynamic_m_systemreg_xml(CPUState *cs, int base_reg) +{ + ARMCPU *cpu = ARM_CPU(cs); + CPUARMState *env = &cpu->env; + GString *s = g_string_new(NULL); + DynamicGDBXMLInfo *info = &cpu->dyn_m_systemreg_xml; + bool is_v8 = arm_feature(env, ARM_FEATURE_V8); + bool is_main = arm_feature(env, ARM_FEATURE_M_MAIN); + + 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"); + + g_autoptr(GArray) regs = g_array_new(false, true, sizeof(const char *)); + /* 0 */ g_array_append_str_literal(regs, "msp"); + /* 1 */ g_array_append_str_literal(regs, "psp"); + /* 2 */ g_array_append_str_literal(regs, "primask"); + /* 3 */ g_array_append_str_literal(regs, is_main ? "basepri" : NULL); + /* 4 */ g_array_append_str_literal(regs, is_main ? "faultmask" : NULL); + /* 5 */ g_array_append_str_literal(regs, "control"); + /* 6 */ g_array_append_str_literal(regs, is_v8 ? "msplim" : NULL); + /* 7 */ g_array_append_str_literal(regs, is_v8 ? "psplim" : NULL); + + for (int idx = 0; idx < regs->len; idx++) { + const char *name = g_array_index(regs, const char *, idx); + if (name) { + 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; @@ -XXX,XX +XXX,XX @@ 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; } return NULL; } @@ -XXX,XX +XXX,XX @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu) */ gdb_register_coprocessor(cs, vfp_gdb_get_sysreg, vfp_gdb_set_sysreg, 2, "arm-vfp-sysregs.xml", 0); + } else { + /* M-profile coprocessors. */ + 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); } } if (cpu_isar_feature(aa32_mve, cpu)) { diff --git a/target/arm/m_helper.c b/target/arm/m_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/m_helper.c +++ b/target/arm/m_helper.c @@ -XXX,XX +XXX,XX @@ static uint32_t v7m_mrs_xpsr(CPUARMState *env, uint32_t reg, unsigned el) return xpsr_read(env) & mask; } -static uint32_t v7m_mrs_control(CPUARMState *env, uint32_t secure) +uint32_t arm_v7m_mrs_control(CPUARMState *env, uint32_t secure) { uint32_t value = env->v7m.control[secure]; @@ -XXX,XX +XXX,XX @@ uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg) case 0 ... 7: /* xPSR sub-fields */ return v7m_mrs_xpsr(env, reg, 0); case 20: /* CONTROL */ - return v7m_mrs_control(env, 0); + return arm_v7m_mrs_control(env, 0); default: /* Unprivileged reads others as zero. */ return 0; @@ -XXX,XX +XXX,XX @@ uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg) case 0 ... 7: /* xPSR sub-fields */ return v7m_mrs_xpsr(env, reg, el); case 20: /* CONTROL */ - return v7m_mrs_control(env, env->v7m.secure); + return arm_v7m_mrs_control(env, env->v7m.secure); case 0x94: /* CONTROL_NS */ /* * We have to handle this here because unprivileged Secure code diff --git a/tests/lcitool/libvirt-ci b/tests/lcitool/libvirt-ci index XXXXXXX..XXXXXXX 160000 --- a/tests/lcitool/libvirt-ci +++ b/tests/lcitool/libvirt-ci @@ -1 +1 @@ -Subproject commit 319a534c220f53fc8670254cac25d6f662c82112 +Subproject commit e3eb28cf2e17fbcf7fe7e19505ee432b8ec5bbb5 -- 2.34.5 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 | 15 +++++- target/arm/gdbstub.c | 116 ++++++++++++++++++++++++++++++++++++++++++ target/arm/m_helper.c | 23 ++++----- 3 files changed, 139 insertions(+), 15 deletions(-) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ 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]; @@ -XXX,XX +XXX,XX @@ int arm_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg); /* * Helpers to dynamically generate 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 @@ -XXX,XX +XXX,XX @@ static inline void xpsr_write(CPUARMState *env, uint32_t val, uint32_t mask) #endif } +/* + * Return a pointer to the location where we currently store the + * stack pointer for the requested security state and thread mode. + * This pointer will become invalid if the CPU state is updated + * such that the stack pointers are switched around (eg changing + * the SPSEL control bit). + */ +uint32_t *arm_v7m_get_sp_ptr(CPUARMState *env, bool secure, bool threadmode, + bool spsel); + + #define HCR_VM (1ULL << 0) #define HCR_SWIO (1ULL << 1) #define HCR_PTW (1ULL << 2) diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/gdbstub.c +++ b/target/arm/gdbstub.c @@ -XXX,XX +XXX,XX @@ int arm_gen_dynamic_m_systemreg_xml(CPUState *cs, int base_reg) return info->num; } +static int arm_gdb_get_m_secextreg(CPUARMState *env, GByteArray *buf, int reg) +{ + switch (reg) { + case 0: /* MSP_S */ + return gdb_get_reg32(buf, *arm_v7m_get_sp_ptr(env, true, false, true)); + case 1: /* PSP_S */ + return gdb_get_reg32(buf, *arm_v7m_get_sp_ptr(env, true, true, true)); + 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, *arm_v7m_get_sp_ptr(env, false, false, true)); + case 9: /* PSP_NS */ + return gdb_get_reg32(buf, *arm_v7m_get_sp_ptr(env, false, true, true)); + 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); + CPUARMState *env = &cpu->env; + GString *s = g_string_new(NULL); + DynamicGDBXMLInfo *info = &cpu->dyn_m_securereg_xml; + bool is_main = arm_feature(env, ARM_FEATURE_M_MAIN); + + 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, is_main ? "basepri_s" : NULL); + /* 6 */ g_array_append_str_literal(regs, is_main ? "faultmask_s" : NULL); + /* 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, is_main ? "basepri_ns" : NULL); + /* 14 */ g_array_append_str_literal(regs, is_main ? "faultmask_ns" : NULL); + /* 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) { + 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; @@ -XXX,XX +XXX,XX @@ 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; } @@ -XXX,XX +XXX,XX @@ 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)) { diff --git a/target/arm/m_helper.c b/target/arm/m_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/m_helper.c +++ b/target/arm/m_helper.c @@ -XXX,XX +XXX,XX @@ void HELPER(v7m_blxns)(CPUARMState *env, uint32_t dest) arm_rebuild_hflags(env); } -static uint32_t *get_v7m_sp_ptr(CPUARMState *env, bool secure, bool threadmode, - bool spsel) +uint32_t *arm_v7m_get_sp_ptr(CPUARMState *env, bool secure, bool threadmode, + bool spsel) { /* - * Return a pointer to the location where we currently store the - * stack pointer for the requested security state and thread mode. - * This pointer will become invalid if the CPU state is updated - * such that the stack pointers are switched around (eg changing - * the SPSEL control bit). * Compare the v8M ARM ARM pseudocode LookUpSP_with_security_mode(). * Unlike that pseudocode, we require the caller to pass us in the * SPSEL control bit value; this is because we also use this @@ -XXX,XX +XXX,XX @@ static bool v7m_push_callee_stack(ARMCPU *cpu, uint32_t lr, bool dotailchain, !mode; mmu_idx = arm_v7m_mmu_idx_for_secstate_and_priv(env, M_REG_S, priv); - frame_sp_p = get_v7m_sp_ptr(env, M_REG_S, mode, - lr & R_V7M_EXCRET_SPSEL_MASK); + frame_sp_p = arm_v7m_get_sp_ptr(env, M_REG_S, mode, + lr & R_V7M_EXCRET_SPSEL_MASK); want_psp = mode && (lr & R_V7M_EXCRET_SPSEL_MASK); if (want_psp) { limit = env->v7m.psplim[M_REG_S]; @@ -XXX,XX +XXX,XX @@ static void do_v7m_exception_exit(ARMCPU *cpu) * use 'frame_sp_p' after we do something that makes it invalid. */ bool spsel = env->v7m.control[return_to_secure] & R_V7M_CONTROL_SPSEL_MASK; - uint32_t *frame_sp_p = get_v7m_sp_ptr(env, - return_to_secure, - !return_to_handler, - spsel); + uint32_t *frame_sp_p = arm_v7m_get_sp_ptr(env, + return_to_secure, + !return_to_handler, + spsel); uint32_t frameptr = *frame_sp_p; bool pop_ok = true; ARMMMUIdx mmu_idx; @@ -XXX,XX +XXX,XX @@ static bool do_v7m_function_return(ARMCPU *cpu) threadmode = !arm_v7m_is_handler_mode(env); spsel = env->v7m.control[M_REG_S] & R_V7M_CONTROL_SPSEL_MASK; - frame_sp_p = get_v7m_sp_ptr(env, true, threadmode, spsel); + frame_sp_p = arm_v7m_get_sp_ptr(env, true, threadmode, spsel); frameptr = *frame_sp_p; /* -- 2.34.5 New in v4: Moved arm_v7m_mrs_control out of the `#ifdef CONFIG_USER_ONLY` block, unbreaking the user-only build. The downside is that this function is now taking up space in the user-only binary, but it can (presumably?) never be used because there are no user modes for v8m cores. Let me know if you'd prefer for me to wrap `#ifdef CONFIG_USER_ONLY` around the v8m registers in the gdb stub. Also, let me know if you'd prefer a separate commit for renaming and moving v7m_mrs_control. David Reiss (2): target/arm/gdbstub: Support reading M system registers from GDB target/arm/gdbstub: Support reading M security extension registers from GDB target/arm/cpu.h | 25 ++++- target/arm/gdbstub.c | 241 ++++++++++++++++++++++++++++++++++++++++++ target/arm/m_helper.c | 87 +++++++-------- 3 files changed, 305 insertions(+), 48 deletions(-) -- 2.34.5 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. `v7m_mrs_control` was renamed `arm_v7m_mrs_control` and made non-static so this logic could be shared between the MRS instruction and the GDB stub. Signed-off-by: David Reiss <dreiss@meta.com> --- target/arm/cpu.h | 12 +++- target/arm/gdbstub.c | 125 ++++++++++++++++++++++++++++++++++++++++++ target/arm/m_helper.c | 6 +- 3 files changed, 138 insertions(+), 5 deletions(-) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ struct ArchCPU { DynamicGDBXMLInfo dyn_sysreg_xml; DynamicGDBXMLInfo dyn_svereg_xml; + DynamicGDBXMLInfo dyn_m_systemreg_xml; /* Timers used by the generic (architected) timer */ QEMUTimer *gt_timer[NUM_GTIMERS]; @@ -XXX,XX +XXX,XX @@ int arm_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg); int arm_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg); /* - * Helpers to dynamically generates XML descriptions of the sysregs - * and SVE registers. Returns the number of registers in each set. + * Helpers to dynamically generate XML descriptions of the sysregs, + * SVE registers, and M-profile system 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); /* Returns the dynamically generated XML for the gdb stub. * Returns a pointer to the XML contents for the specified XML file or NULL @@ -XXX,XX +XXX,XX @@ FIELD(SVCR, ZA, 1, 1) FIELD(SMCR, LEN, 0, 4) FIELD(SMCR, FA64, 31, 1) +/* + * Read the CONTROL register as the MRS instruction would. + */ +uint32_t arm_v7m_mrs_control(CPUARMState *env, uint32_t secure); + /* Write a new value to v7m.exception, thus transitioning into or out * of Handler mode; this may result in a change of active stack pointer. */ diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/gdbstub.c +++ b/target/arm/gdbstub.c @@ -XXX,XX +XXX,XX @@ int arm_gen_dynamic_sysreg_xml(CPUState *cs, int base_reg) return cpu->dyn_sysreg_xml.num; } +/* + * Helper required because g_array_append_val is a macro + * that cannot handle string literals. + */ +static inline void g_array_append_str_literal(GArray *array, const char *str) +{ + g_array_append_val(array, str); +} + +static int arm_gdb_get_m_systemreg(CPUARMState *env, GByteArray *buf, int reg) +{ + /* NOTE: This implementation shares a lot of logic with v7m_mrs. */ + switch (reg) { + + /* + * NOTE: MSP and PSP technically don't exist if the secure extension + * is present (replaced by MSP_S, MSP_NS, PSP_S, PSP_NS). Similar for + * MSPLIM and PSPLIM. + * However, the MRS instruction is still allowed to read from MSP and PSP, + * and will return the value associated with the current security state. + * We replicate this behavior for the convenience of users, who will see + * GDB behave similarly to their assembly code, even if they are oblivious + * to the security extension. + */ + case 0: /* MSP */ + return gdb_get_reg32(buf, + v7m_using_psp(env) ? env->v7m.other_sp : env->regs[13]); + case 1: /* PSP */ + return gdb_get_reg32(buf, + v7m_using_psp(env) ? env->regs[13] : env->v7m.other_sp); + case 6: /* MSPLIM */ + if (!arm_feature(env, ARM_FEATURE_V8)) { + return 0; + } + return gdb_get_reg32(buf, env->v7m.msplim[env->v7m.secure]); + case 7: /* PSPLIM */ + if (!arm_feature(env, ARM_FEATURE_V8)) { + return 0; + } + return gdb_get_reg32(buf, env->v7m.psplim[env->v7m.secure]); + + /* + * NOTE: PRIMASK, BASEPRI, and FAULTMASK are defined a bit differently + * from the SP family, but have similar banking behavior. + */ + case 2: /* PRIMASK */ + return gdb_get_reg32(buf, env->v7m.primask[env->v7m.secure]); + case 3: /* BASEPRI */ + if (!arm_feature(env, ARM_FEATURE_M_MAIN)) { + return 0; + } + return gdb_get_reg32(buf, env->v7m.basepri[env->v7m.secure]); + case 4: /* FAULTMASK */ + if (!arm_feature(env, ARM_FEATURE_M_MAIN)) { + return 0; + } + return gdb_get_reg32(buf, env->v7m.faultmask[env->v7m.secure]); + + /* + * NOTE: CONTROL has a mix of banked and non-banked bits. We continue + * to emulate the MRS instruction. Unfortunately, this gives GDB no way + * to read the SFPA bit when the CPU is in a non-secure state. + */ + case 5: /* CONTROL */ + return gdb_get_reg32(buf, arm_v7m_mrs_control(env, env->v7m.secure)); + } + + return 0; +} + +static int arm_gdb_set_m_systemreg(CPUARMState *env, uint8_t *buf, int reg) +{ + /* TODO: Implement. */ + return 0; +} + +int arm_gen_dynamic_m_systemreg_xml(CPUState *cs, int base_reg) +{ + ARMCPU *cpu = ARM_CPU(cs); + CPUARMState *env = &cpu->env; + GString *s = g_string_new(NULL); + DynamicGDBXMLInfo *info = &cpu->dyn_m_systemreg_xml; + bool is_v8 = arm_feature(env, ARM_FEATURE_V8); + bool is_main = arm_feature(env, ARM_FEATURE_M_MAIN); + + 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"); + + g_autoptr(GArray) regs = g_array_new(false, true, sizeof(const char *)); + /* 0 */ g_array_append_str_literal(regs, "msp"); + /* 1 */ g_array_append_str_literal(regs, "psp"); + /* 2 */ g_array_append_str_literal(regs, "primask"); + /* 3 */ g_array_append_str_literal(regs, is_main ? "basepri" : NULL); + /* 4 */ g_array_append_str_literal(regs, is_main ? "faultmask" : NULL); + /* 5 */ g_array_append_str_literal(regs, "control"); + /* 6 */ g_array_append_str_literal(regs, is_v8 ? "msplim" : NULL); + /* 7 */ g_array_append_str_literal(regs, is_v8 ? "psplim" : NULL); + + for (int idx = 0; idx < regs->len; idx++) { + const char *name = g_array_index(regs, const char *, idx); + if (name) { + 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; @@ -XXX,XX +XXX,XX @@ 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; } return NULL; } @@ -XXX,XX +XXX,XX @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu) */ gdb_register_coprocessor(cs, vfp_gdb_get_sysreg, vfp_gdb_set_sysreg, 2, "arm-vfp-sysregs.xml", 0); + } else { + /* M-profile coprocessors. */ + 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); } } if (cpu_isar_feature(aa32_mve, cpu)) { diff --git a/target/arm/m_helper.c b/target/arm/m_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/m_helper.c +++ b/target/arm/m_helper.c @@ -XXX,XX +XXX,XX @@ static uint32_t v7m_mrs_xpsr(CPUARMState *env, uint32_t reg, unsigned el) return xpsr_read(env) & mask; } -static uint32_t v7m_mrs_control(CPUARMState *env, uint32_t secure) +uint32_t arm_v7m_mrs_control(CPUARMState *env, uint32_t secure) { uint32_t value = env->v7m.control[secure]; @@ -XXX,XX +XXX,XX @@ uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg) case 0 ... 7: /* xPSR sub-fields */ return v7m_mrs_xpsr(env, reg, 0); case 20: /* CONTROL */ - return v7m_mrs_control(env, 0); + return arm_v7m_mrs_control(env, 0); default: /* Unprivileged reads others as zero. */ return 0; @@ -XXX,XX +XXX,XX @@ uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg) case 0 ... 7: /* xPSR sub-fields */ return v7m_mrs_xpsr(env, reg, el); case 20: /* CONTROL */ - return v7m_mrs_control(env, env->v7m.secure); + return arm_v7m_mrs_control(env, env->v7m.secure); case 0x94: /* CONTROL_NS */ /* * We have to handle this here because unprivileged Secure code -- 2.34.5 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 | 15 +++++- target/arm/gdbstub.c | 116 ++++++++++++++++++++++++++++++++++++++++++ target/arm/m_helper.c | 81 ++++++++++++++--------------- 3 files changed, 168 insertions(+), 44 deletions(-) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ 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]; @@ -XXX,XX +XXX,XX @@ int arm_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg); /* * Helpers to dynamically generate 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 @@ -XXX,XX +XXX,XX @@ static inline void xpsr_write(CPUARMState *env, uint32_t val, uint32_t mask) #endif } +/* + * Return a pointer to the location where we currently store the + * stack pointer for the requested security state and thread mode. + * This pointer will become invalid if the CPU state is updated + * such that the stack pointers are switched around (eg changing + * the SPSEL control bit). + */ +uint32_t *arm_v7m_get_sp_ptr(CPUARMState *env, bool secure, bool threadmode, + bool spsel); + + #define HCR_VM (1ULL << 0) #define HCR_SWIO (1ULL << 1) #define HCR_PTW (1ULL << 2) diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/gdbstub.c +++ b/target/arm/gdbstub.c @@ -XXX,XX +XXX,XX @@ int arm_gen_dynamic_m_systemreg_xml(CPUState *cs, int base_reg) return info->num; } +static int arm_gdb_get_m_secextreg(CPUARMState *env, GByteArray *buf, int reg) +{ + switch (reg) { + case 0: /* MSP_S */ + return gdb_get_reg32(buf, *arm_v7m_get_sp_ptr(env, true, false, true)); + case 1: /* PSP_S */ + return gdb_get_reg32(buf, *arm_v7m_get_sp_ptr(env, true, true, true)); + 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, *arm_v7m_get_sp_ptr(env, false, false, true)); + case 9: /* PSP_NS */ + return gdb_get_reg32(buf, *arm_v7m_get_sp_ptr(env, false, true, true)); + 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); + CPUARMState *env = &cpu->env; + GString *s = g_string_new(NULL); + DynamicGDBXMLInfo *info = &cpu->dyn_m_securereg_xml; + bool is_main = arm_feature(env, ARM_FEATURE_M_MAIN); + + 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, is_main ? "basepri_s" : NULL); + /* 6 */ g_array_append_str_literal(regs, is_main ? "faultmask_s" : NULL); + /* 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, is_main ? "basepri_ns" : NULL); + /* 14 */ g_array_append_str_literal(regs, is_main ? "faultmask_ns" : NULL); + /* 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) { + 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; @@ -XXX,XX +XXX,XX @@ 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; } @@ -XXX,XX +XXX,XX @@ 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)) { diff --git a/target/arm/m_helper.c b/target/arm/m_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/m_helper.c +++ b/target/arm/m_helper.c @@ -XXX,XX +XXX,XX @@ void HELPER(v7m_blxns)(CPUARMState *env, uint32_t dest) arm_rebuild_hflags(env); } -static uint32_t *get_v7m_sp_ptr(CPUARMState *env, bool secure, bool threadmode, - bool spsel) -{ - /* - * Return a pointer to the location where we currently store the - * stack pointer for the requested security state and thread mode. - * This pointer will become invalid if the CPU state is updated - * such that the stack pointers are switched around (eg changing - * the SPSEL control bit). - * Compare the v8M ARM ARM pseudocode LookUpSP_with_security_mode(). - * Unlike that pseudocode, we require the caller to pass us in the - * SPSEL control bit value; this is because we also use this - * function in handling of pushing of the callee-saves registers - * part of the v8M stack frame (pseudocode PushCalleeStack()), - * and in the tailchain codepath the SPSEL bit comes from the exception - * return magic LR value from the previous exception. The pseudocode - * opencodes the stack-selection in PushCalleeStack(), but we prefer - * to make this utility function generic enough to do the job. - */ - bool want_psp = threadmode && spsel; - - if (secure == env->v7m.secure) { - if (want_psp == v7m_using_psp(env)) { - return &env->regs[13]; - } else { - return &env->v7m.other_sp; - } - } else { - if (want_psp) { - return &env->v7m.other_ss_psp; - } else { - return &env->v7m.other_ss_msp; - } - } -} - static bool arm_v7m_load_vector(ARMCPU *cpu, int exc, bool targets_secure, uint32_t *pvec) { @@ -XXX,XX +XXX,XX @@ static bool v7m_push_callee_stack(ARMCPU *cpu, uint32_t lr, bool dotailchain, !mode; mmu_idx = arm_v7m_mmu_idx_for_secstate_and_priv(env, M_REG_S, priv); - frame_sp_p = get_v7m_sp_ptr(env, M_REG_S, mode, - lr & R_V7M_EXCRET_SPSEL_MASK); + frame_sp_p = arm_v7m_get_sp_ptr(env, M_REG_S, mode, + lr & R_V7M_EXCRET_SPSEL_MASK); want_psp = mode && (lr & R_V7M_EXCRET_SPSEL_MASK); if (want_psp) { limit = env->v7m.psplim[M_REG_S]; @@ -XXX,XX +XXX,XX @@ static void do_v7m_exception_exit(ARMCPU *cpu) * use 'frame_sp_p' after we do something that makes it invalid. */ bool spsel = env->v7m.control[return_to_secure] & R_V7M_CONTROL_SPSEL_MASK; - uint32_t *frame_sp_p = get_v7m_sp_ptr(env, - return_to_secure, - !return_to_handler, - spsel); + uint32_t *frame_sp_p = arm_v7m_get_sp_ptr(env, + return_to_secure, + !return_to_handler, + spsel); uint32_t frameptr = *frame_sp_p; bool pop_ok = true; ARMMMUIdx mmu_idx; @@ -XXX,XX +XXX,XX @@ static bool do_v7m_function_return(ARMCPU *cpu) threadmode = !arm_v7m_is_handler_mode(env); spsel = env->v7m.control[M_REG_S] & R_V7M_CONTROL_SPSEL_MASK; - frame_sp_p = get_v7m_sp_ptr(env, true, threadmode, spsel); + frame_sp_p = arm_v7m_get_sp_ptr(env, true, threadmode, spsel); frameptr = *frame_sp_p; /* @@ -XXX,XX +XXX,XX @@ uint32_t HELPER(v7m_tt)(CPUARMState *env, uint32_t addr, uint32_t op) #endif /* !CONFIG_USER_ONLY */ +uint32_t *arm_v7m_get_sp_ptr(CPUARMState *env, bool secure, bool threadmode, + bool spsel) +{ + /* + * Compare the v8M ARM ARM pseudocode LookUpSP_with_security_mode(). + * Unlike that pseudocode, we require the caller to pass us in the + * SPSEL control bit value; this is because we also use this + * function in handling of pushing of the callee-saves registers + * part of the v8M stack frame (pseudocode PushCalleeStack()), + * and in the tailchain codepath the SPSEL bit comes from the exception + * return magic LR value from the previous exception. The pseudocode + * opencodes the stack-selection in PushCalleeStack(), but we prefer + * to make this utility function generic enough to do the job. + */ + bool want_psp = threadmode && spsel; + + if (secure == env->v7m.secure) { + if (want_psp == v7m_using_psp(env)) { + return &env->regs[13]; + } else { + return &env->v7m.other_sp; + } + } else { + if (want_psp) { + return &env->v7m.other_ss_psp; + } else { + return &env->v7m.other_ss_msp; + } + } +} + ARMMMUIdx arm_v7m_mmu_idx_all(CPUARMState *env, bool secstate, bool priv, bool negpri) { -- 2.34.5