On Wed, Aug 02, 2017 at 05:43:55PM +0100, Peter Maydell wrote:
> We currently store the M profile CPU register state PRIMASK and
> FAULTMASK in the daif field of the CPU state in its I and F
> bits. This is a legacy from the original implementation, which
> tried to share the cpu_exec_interrupt code between A profile
> and M profile. We've since separated out the two cases because
> they are significantly different, so now there is no common
> code between M and A profile which looks at env->daif: all the
> uses are either in A-only or M-only code paths. Sharing the state
> fields now is just confusing, and will make things awkward
> when we implement v8M, where the PRIMASK and FAULTMASK
> registers are banked between security states.
>
> Switch M profile over to using v7m.faultmask and v7m.primask
> fields for these registers.
>
> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
> ---
> hw/intc/armv7m_nvic.c | 4 ++--
> target/arm/cpu.c | 5 -----
> target/arm/cpu.h | 4 +++-
> target/arm/helper.c | 18 +++++-------------
> target/arm/machine.c | 33 +++++++++++++++++++++++++++++++++
> 5 files changed, 43 insertions(+), 21 deletions(-)
>
> diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
> index 2e8166a..343bc16 100644
> --- a/hw/intc/armv7m_nvic.c
> +++ b/hw/intc/armv7m_nvic.c
> @@ -167,9 +167,9 @@ static inline int nvic_exec_prio(NVICState *s)
> CPUARMState *env = &s->cpu->env;
> int running;
>
> - if (env->daif & PSTATE_F) { /* FAULTMASK */
> + if (env->v7m.faultmask) {
> running = -1;
> - } else if (env->daif & PSTATE_I) { /* PRIMASK */
> + } else if (env->v7m.primask) {
> running = 0;
> } else if (env->v7m.basepri > 0) {
> running = env->v7m.basepri & nvic_gprio_mask(s);
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 05c038b..b241a63 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -185,11 +185,6 @@ static void arm_cpu_reset(CPUState *s)
> uint32_t initial_pc; /* Loaded from 0x4 */
> uint8_t *rom;
>
> - /* For M profile we store FAULTMASK and PRIMASK in the
> - * PSTATE F and I bits; these are both clear at reset.
> - */
> - env->daif &= ~(PSTATE_I | PSTATE_F);
> -
> /* The reset value of this bit is IMPDEF, but ARM recommends
> * that it resets to 1, so QEMU always does that rather than making
> * it dependent on CPU model.
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 1f06de0..da90b7a 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -418,6 +418,8 @@ typedef struct CPUARMState {
> uint32_t bfar; /* BusFault Address */
> unsigned mpu_ctrl; /* MPU_CTRL */
> int exception;
> + uint32_t primask;
> + uint32_t faultmask;
It seems like these could be booleans?
Cheers,
Edgar
> } v7m;
>
> /* Information associated with an exception about to be taken:
> @@ -2179,7 +2181,7 @@ static inline int cpu_mmu_index(CPUARMState *env, bool
> ifetch)
> * we're in a HardFault or NMI handler.
> */
> if ((env->v7m.exception > 0 && env->v7m.exception <= 3)
> - || env->daif & PSTATE_F) {
> + || env->v7m.faultmask) {
> return arm_to_core_mmu_idx(ARMMMUIdx_MNegPri);
> }
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index f087d42..b64ddb1 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -6172,7 +6172,7 @@ static void do_v7m_exception_exit(ARMCPU *cpu)
>
> if (env->v7m.exception != ARMV7M_EXCP_NMI) {
> /* Auto-clear FAULTMASK on return from other than NMI */
> - env->daif &= ~PSTATE_F;
> + env->v7m.faultmask = 0;
> }
>
> switch (armv7m_nvic_complete_irq(env->nvic, env->v7m.exception)) {
> @@ -8718,12 +8718,12 @@ uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t
> reg)
> return (env->v7m.control & R_V7M_CONTROL_SPSEL_MASK) ?
> env->regs[13] : env->v7m.other_sp;
> case 16: /* PRIMASK */
> - return (env->daif & PSTATE_I) != 0;
> + return env->v7m.primask;
> case 17: /* BASEPRI */
> case 18: /* BASEPRI_MAX */
> return env->v7m.basepri;
> case 19: /* FAULTMASK */
> - return (env->daif & PSTATE_F) != 0;
> + return env->v7m.faultmask;
> default:
> qemu_log_mask(LOG_GUEST_ERROR, "Attempt to read unknown special"
> " register %d\n", reg);
> @@ -8778,11 +8778,7 @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t
> maskreg, uint32_t val)
> }
> break;
> case 16: /* PRIMASK */
> - if (val & 1) {
> - env->daif |= PSTATE_I;
> - } else {
> - env->daif &= ~PSTATE_I;
> - }
> + env->v7m.primask = val & 1;
> break;
> case 17: /* BASEPRI */
> env->v7m.basepri = val & 0xff;
> @@ -8793,11 +8789,7 @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t
> maskreg, uint32_t val)
> env->v7m.basepri = val;
> break;
> case 19: /* FAULTMASK */
> - if (val & 1) {
> - env->daif |= PSTATE_F;
> - } else {
> - env->daif &= ~PSTATE_F;
> - }
> + env->v7m.faultmask = val & 1;
> break;
> case 20: /* CONTROL */
> /* Writing to the SPSEL bit only has an effect if we are in
> diff --git a/target/arm/machine.c b/target/arm/machine.c
> index 1f66da4..2fb4b76 100644
> --- a/target/arm/machine.c
> +++ b/target/arm/machine.c
> @@ -97,6 +97,17 @@ static bool m_needed(void *opaque)
> return arm_feature(env, ARM_FEATURE_M);
> }
>
> +static const VMStateDescription vmstate_m_faultmask_primask = {
> + .name = "cpu/m/faultmask-primask",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .fields = (VMStateField[]) {
> + VMSTATE_UINT32(env.v7m.faultmask, ARMCPU),
> + VMSTATE_UINT32(env.v7m.primask, ARMCPU),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> static const VMStateDescription vmstate_m = {
> .name = "cpu/m",
> .version_id = 4,
> @@ -115,6 +126,10 @@ static const VMStateDescription vmstate_m = {
> VMSTATE_UINT32(env.v7m.mpu_ctrl, ARMCPU),
> VMSTATE_INT32(env.v7m.exception, ARMCPU),
> VMSTATE_END_OF_LIST()
> + },
> + .subsections = (const VMStateDescription*[]) {
> + &vmstate_m_faultmask_primask,
> + NULL
> }
> };
>
> @@ -201,6 +216,24 @@ static int get_cpsr(QEMUFile *f, void *opaque, size_t
> size,
> CPUARMState *env = &cpu->env;
> uint32_t val = qemu_get_be32(f);
>
> + if (arm_feature(env, ARM_FEATURE_M)) {
> + /* If the I or F bits are set then this is a migration from
> + * an old QEMU which still stored the M profile FAULTMASK
> + * and PRIMASK in env->daif. Set v7m.faultmask and v7m.primask
> + * accordingly, and then clear the bits so they don't confuse
> + * cpsr_write(). For a new QEMU, the bits here will always be
> + * clear, and the data is transferred using the
> + * vmstate_m_faultmask_primask subsection.
> + */
> + if (val & CPSR_F) {
> + env->v7m.faultmask = 1;
> + }
> + if (val & CPSR_I) {
> + env->v7m.primask = 1;
> + }
> + val &= ~(CPSR_F | CPSR_I);
> + }
> +
> env->aarch64 = ((val & PSTATE_nRW) == 0);
>
> if (is_a64(env)) {
> --
> 2.7.4
>
>
