target/arm/cpu.h | 2 + target/arm/m_helper.c | 110 ++++++++++++++++++++++++++---------------- 2 files changed, 70 insertions(+), 42 deletions(-)
Simply moving the non-stub helper_v7m_mrs/msr outside of
!CONFIG_USER_ONLY is not an option, because of all of the
other system-mode helpers that are called.
But we can split out a few subroutines to handle the few
EL0 accessible registers without duplicating code.
Reported-by: Christophe Lyon <christophe.lyon@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/cpu.h | 2 +
target/arm/m_helper.c | 110 ++++++++++++++++++++++++++----------------
2 files changed, 70 insertions(+), 42 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index e1a66a2d1c..81f5b5b75f 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1317,6 +1317,7 @@ static inline void xpsr_write(CPUARMState *env, uint32_t val, uint32_t mask)
if (mask & XPSR_GE) {
env->GE = (val & XPSR_GE) >> 16;
}
+#ifndef CONFIG_USER_ONLY
if (mask & XPSR_T) {
env->thumb = ((val & XPSR_T) != 0);
}
@@ -1332,6 +1333,7 @@ static inline void xpsr_write(CPUARMState *env, uint32_t val, uint32_t mask)
/* Note that this only happens on exception exit */
write_v7m_exception(env, val & XPSR_EXCP);
}
+#endif
}
#define HCR_VM (1ULL << 0)
diff --git a/target/arm/m_helper.c b/target/arm/m_helper.c
index f2512e448e..7c7e8eb62c 100644
--- a/target/arm/m_helper.c
+++ b/target/arm/m_helper.c
@@ -33,22 +33,80 @@
#include "exec/cpu_ldst.h"
#endif
+static void v7m_msr_xpsr(CPUARMState *env, uint32_t mask,
+ uint32_t reg, uint32_t val)
+{
+ /* Only APSR is actually writable */
+ if (!(reg & 4)) {
+ uint32_t apsrmask = 0;
+
+ if (mask & 8) {
+ apsrmask |= XPSR_NZCV | XPSR_Q;
+ }
+ if ((mask & 4) && arm_feature(env, ARM_FEATURE_THUMB_DSP)) {
+ apsrmask |= XPSR_GE;
+ }
+ xpsr_write(env, val, apsrmask);
+ }
+}
+
+static uint32_t v7m_mrs_xpsr(CPUARMState *env, uint32_t reg, unsigned el)
+{
+ uint32_t mask = 0;
+
+ if ((reg & 1) && el) {
+ mask |= XPSR_EXCP; /* IPSR (unpriv. reads as zero) */
+ }
+ if (!(reg & 4)) {
+ mask |= XPSR_NZCV | XPSR_Q; /* APSR */
+ if (arm_feature(env, ARM_FEATURE_THUMB_DSP)) {
+ mask |= XPSR_GE;
+ }
+ }
+ /* EPSR reads as zero */
+ return xpsr_read(env) & mask;
+}
+
+static uint32_t v7m_mrs_control(CPUARMState *env, uint32_t secure)
+{
+ uint32_t value = env->v7m.control[secure];
+
+ if (!secure) {
+ /* SFPA is RAZ/WI from NS; FPCA is stored in the M_REG_S bank */
+ value |= env->v7m.control[M_REG_S] & R_V7M_CONTROL_FPCA_MASK;
+ }
+ return value;
+}
+
#ifdef CONFIG_USER_ONLY
/* These should probably raise undefined insn exceptions. */
-void HELPER(v7m_msr)(CPUARMState *env, uint32_t reg, uint32_t val)
+void HELPER(v7m_msr)(CPUARMState *env, uint32_t maskreg, uint32_t val)
{
- ARMCPU *cpu = env_archcpu(env);
+ uint32_t mask = extract32(maskreg, 8, 4);
+ uint32_t reg = extract32(maskreg, 0, 8);
- cpu_abort(CPU(cpu), "v7m_msr %d\n", reg);
+ switch (reg) {
+ case 0 ... 7: /* xPSR sub-fields */
+ v7m_msr_xpsr(env, mask, reg, val);
+ break;
+ case 20: /* CONTROL */
+ /* There are no sub-fields that are actually writable from EL0. */
+ break;
+ }
}
uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg)
{
- ARMCPU *cpu = env_archcpu(env);
-
- cpu_abort(CPU(cpu), "v7m_mrs %d\n", reg);
- return 0;
+ switch (reg) {
+ case 0 ... 7: /* xPSR sub-fields */
+ return v7m_mrs_xpsr(env, reg, 0);
+ case 20: /* CONTROL */
+ return v7m_mrs_control(env, 0);
+ default:
+ /* Unprivileged reads others as zero. */
+ return 0;
+ }
}
void HELPER(v7m_bxns)(CPUARMState *env, uint32_t dest)
@@ -2196,35 +2254,14 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg)
{
- uint32_t mask;
unsigned el = arm_current_el(env);
/* First handle registers which unprivileged can read */
-
switch (reg) {
case 0 ... 7: /* xPSR sub-fields */
- mask = 0;
- if ((reg & 1) && el) {
- mask |= XPSR_EXCP; /* IPSR (unpriv. reads as zero) */
- }
- if (!(reg & 4)) {
- mask |= XPSR_NZCV | XPSR_Q; /* APSR */
- if (arm_feature(env, ARM_FEATURE_THUMB_DSP)) {
- mask |= XPSR_GE;
- }
- }
- /* EPSR reads as zero */
- return xpsr_read(env) & mask;
- break;
+ return v7m_mrs_xpsr(env, reg, el);
case 20: /* CONTROL */
- {
- uint32_t value = env->v7m.control[env->v7m.secure];
- if (!env->v7m.secure) {
- /* SFPA is RAZ/WI from NS; FPCA is stored in the M_REG_S bank */
- value |= env->v7m.control[M_REG_S] & R_V7M_CONTROL_FPCA_MASK;
- }
- return value;
- }
+ return v7m_mrs_control(env, env->v7m.secure);
case 0x94: /* CONTROL_NS */
/*
* We have to handle this here because unprivileged Secure code
@@ -2454,18 +2491,7 @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t maskreg, uint32_t val)
switch (reg) {
case 0 ... 7: /* xPSR sub-fields */
- /* only APSR is actually writable */
- if (!(reg & 4)) {
- uint32_t apsrmask = 0;
-
- if (mask & 8) {
- apsrmask |= XPSR_NZCV | XPSR_Q;
- }
- if ((mask & 4) && arm_feature(env, ARM_FEATURE_THUMB_DSP)) {
- apsrmask |= XPSR_GE;
- }
- xpsr_write(env, val, apsrmask);
- }
+ v7m_msr_xpsr(env, mask, reg, val);
break;
case 8: /* MSP */
if (v7m_using_psp(env)) {
--
2.17.1
On Mon, 18 Nov 2019 at 19:49, Richard Henderson <richard.henderson@linaro.org> wrote: > > Simply moving the non-stub helper_v7m_mrs/msr outside of > !CONFIG_USER_ONLY is not an option, because of all of the > other system-mode helpers that are called. > > But we can split out a few subroutines to handle the few > EL0 accessible registers without duplicating code. > > Reported-by: Christophe Lyon <christophe.lyon@linaro.org> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/arm/cpu.h | 2 + > target/arm/m_helper.c | 110 ++++++++++++++++++++++++++---------------- > 2 files changed, 70 insertions(+), 42 deletions(-) > > /* These should probably raise undefined insn exceptions. */ This comment is now wrong -- all the 'dummy version for user mode' helpers below it are either "known to never be called" or have a "works for user-mode" implementation, so we can just delete it. > -void HELPER(v7m_msr)(CPUARMState *env, uint32_t reg, uint32_t val) > +void HELPER(v7m_msr)(CPUARMState *env, uint32_t maskreg, uint32_t val) > { > - ARMCPU *cpu = env_archcpu(env); > + uint32_t mask = extract32(maskreg, 8, 4); > + uint32_t reg = extract32(maskreg, 0, 8); > > - cpu_abort(CPU(cpu), "v7m_msr %d\n", reg); > + switch (reg) { > + case 0 ... 7: /* xPSR sub-fields */ > + v7m_msr_xpsr(env, mask, reg, val); > + break; > + case 20: /* CONTROL */ > + /* There are no sub-fields that are actually writable from EL0. */ > + break; I feel like somebody's static analysis is probably going to complain if we don't have a default case here: default: /* Writes to all other registers from EL0 are ignored */ break; Looks good otherwise, so I'll apply it to target-arm.next with those fixups. thanks -- PMM
© 2016 - 2024 Red Hat, Inc.