* [PATCH qemu v4 1/2] target/arm/gdbstub: Support reading M system registers from GDB
2023-02-07 17:37 [PATCH qemu v4 0/2] ARM: Add support for V8M special registers in GDB stub ~dreiss-meta
@ 2023-01-09 23:05 ` ~dreiss-meta
2023-01-09 23:05 ` [PATCH qemu v4 2/2] target/arm/gdbstub: Support reading M security extension " ~dreiss-meta
2023-02-07 17:51 ` [PATCH qemu v4 0/2] ARM: Add support for V8M special registers in GDB stub Peter Maydell
2 siblings, 0 replies; 5+ messages in thread
From: ~dreiss-meta @ 2023-01-09 23:05 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Peter Maydell
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 7bc97fece9..2ba64811a0 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -867,6 +867,7 @@ 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];
@@ -1111,11 +1112,13 @@ 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
@@ -1507,6 +1510,11 @@ 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 2f806512d0..2780a089ec 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -322,6 +322,121 @@ 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;
@@ -450,6 +565,8 @@ 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;
}
@@ -493,6 +610,14 @@ 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 e7e746ea18..c20bcac977 100644
--- a/target/arm/m_helper.c
+++ b/target/arm/m_helper.c
@@ -53,7 +53,7 @@ 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];
@@ -90,7 +90,7 @@ 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;
@@ -2420,7 +2420,7 @@ 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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH qemu v4 2/2] target/arm/gdbstub: Support reading M security extension registers from GDB
2023-02-07 17:37 [PATCH qemu v4 0/2] ARM: Add support for V8M special registers in GDB stub ~dreiss-meta
2023-01-09 23:05 ` [PATCH qemu v4 1/2] target/arm/gdbstub: Support reading M system registers from GDB ~dreiss-meta
@ 2023-01-09 23:05 ` ~dreiss-meta
2023-02-07 17:51 ` [PATCH qemu v4 0/2] ARM: Add support for V8M special registers in GDB stub Peter Maydell
2 siblings, 0 replies; 5+ messages in thread
From: ~dreiss-meta @ 2023-01-09 23:05 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Peter Maydell
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 2ba64811a0..788eb31286 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -868,6 +868,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];
@@ -1113,12 +1114,13 @@ 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
@@ -1618,6 +1620,17 @@ 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 2780a089ec..3d4ca8a008 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -437,6 +437,110 @@ 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;
@@ -567,6 +671,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 +724,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)) {
diff --git a/target/arm/m_helper.c b/target/arm/m_helper.c
index c20bcac977..5cbe2dc92e 100644
--- a/target/arm/m_helper.c
+++ b/target/arm/m_helper.c
@@ -605,42 +605,6 @@ 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)
{
@@ -765,8 +729,8 @@ 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];
@@ -1611,10 +1575,10 @@ 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;
@@ -1920,7 +1884,7 @@ 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;
/*
@@ -2856,6 +2820,37 @@ 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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH qemu v4 0/2] ARM: Add support for V8M special registers in GDB stub
@ 2023-02-07 17:37 ~dreiss-meta
2023-01-09 23:05 ` [PATCH qemu v4 1/2] target/arm/gdbstub: Support reading M system registers from GDB ~dreiss-meta
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: ~dreiss-meta @ 2023-02-07 17:37 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Peter Maydell
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH qemu v4 0/2] ARM: Add support for V8M special registers in GDB stub
2023-02-07 17:37 [PATCH qemu v4 0/2] ARM: Add support for V8M special registers in GDB stub ~dreiss-meta
2023-01-09 23:05 ` [PATCH qemu v4 1/2] target/arm/gdbstub: Support reading M system registers from GDB ~dreiss-meta
2023-01-09 23:05 ` [PATCH qemu v4 2/2] target/arm/gdbstub: Support reading M security extension " ~dreiss-meta
@ 2023-02-07 17:51 ` Peter Maydell
2023-02-14 16:12 ` Peter Maydell
2 siblings, 1 reply; 5+ messages in thread
From: Peter Maydell @ 2023-02-07 17:51 UTC (permalink / raw)
To: ~dreiss-meta; +Cc: qemu-devel, qemu-arm
On Tue, 7 Feb 2023 at 17:37, ~dreiss-meta <dreiss-meta@git.sr.ht> wrote:
>
> 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.
We do support the M-profile cores in the linux-user executables,
so this code is "live". The view that the guest program and
the debugger should see is that the core is always in
user mode (aka unprivileged, Thread mode).
thanks
-- PMM
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH qemu v4 0/2] ARM: Add support for V8M special registers in GDB stub
2023-02-07 17:51 ` [PATCH qemu v4 0/2] ARM: Add support for V8M special registers in GDB stub Peter Maydell
@ 2023-02-14 16:12 ` Peter Maydell
0 siblings, 0 replies; 5+ messages in thread
From: Peter Maydell @ 2023-02-14 16:12 UTC (permalink / raw)
To: ~dreiss-meta; +Cc: qemu-devel, qemu-arm, Richard Henderson
On Tue, 7 Feb 2023 at 17:51, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Tue, 7 Feb 2023 at 17:37, ~dreiss-meta <dreiss-meta@git.sr.ht> wrote:
> >
> > 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.
>
> We do support the M-profile cores in the linux-user executables,
> so this code is "live". The view that the guest program and
> the debugger should see is that the core is always in
> user mode (aka unprivileged, Thread mode).
Having looked more closely at the registers in the
org.gnu.gdb.arm.secext set, I think the right answer here is
that for CONFIG_USER_ONLY we should never expose the second
org.gnu.gdb.arm.secext block. Because the guest is restricted
to non-secure mode only, the registers in the
org.gnu.gdb.arm.m-system block are sufficient to get all the
interesting information, and the debugger shouldn't be confused
by only seeing one of the two, because that's what it sees when
talking to a CPU which doesn't have the security extension.
The register state in the other block is OK.
(I don't think the code as it stands will be broken as such,
but it will let the debugger read a lot of confusing 0s, and
it leaves the door open for us forgetting to prevent writes
if we add system register write suport later, which would let
gdb put QEMU into states it doesn't expect to be in.)
That change is simple (couple of ifdefs in gdbstub.c, and
then it lets us avoid moving the definition of the
arm_v7m_get_sp_ptr() function). But Richard has just told
me he has a rework of some of the gdbstub handling which
he incorporated this patchset into, so I think the best
thing is if he can post that series.
thanks
-- PMM
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-02-14 16:13 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-07 17:37 [PATCH qemu v4 0/2] ARM: Add support for V8M special registers in GDB stub ~dreiss-meta
2023-01-09 23:05 ` [PATCH qemu v4 1/2] target/arm/gdbstub: Support reading M system registers from GDB ~dreiss-meta
2023-01-09 23:05 ` [PATCH qemu v4 2/2] target/arm/gdbstub: Support reading M security extension " ~dreiss-meta
2023-02-07 17:51 ` [PATCH qemu v4 0/2] ARM: Add support for V8M special registers in GDB stub Peter Maydell
2023-02-14 16:12 ` Peter Maydell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).