Peter Maydell <peter.mayd...@linaro.org> writes:
> Implement the exception return consistency checks
> described in the v7M pseudocode ExceptionReturn().
>
> Inspired by a patch from Michael Davidsaver's series, but
> this is a reimplementation from scratch based on the
> ARM ARM pseudocode.
>
> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
Reviewed-by: Alex Bennée <alex.ben...@linaro.org>
> ---
> target/arm/cpu.h | 12 +++++-
> hw/intc/armv7m_nvic.c | 12 +++++-
> target/arm/helper.c | 112
> +++++++++++++++++++++++++++++++++++++++++++++-----
> 3 files changed, 123 insertions(+), 13 deletions(-)
>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index ab46c0c..017e301 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -1352,7 +1352,17 @@ static inline bool
> armv7m_nvic_can_take_pending_exception(void *opaque)
> #endif
> void armv7m_nvic_set_pending(void *opaque, int irq);
> void armv7m_nvic_acknowledge_irq(void *opaque);
> -void armv7m_nvic_complete_irq(void *opaque, int irq);
> +/**
> + * armv7m_nvic_complete_irq: complete specified interrupt or exception
> + * @opaque: the NVIC
> + * @irq: the exception number to complete
> + *
> + * Returns: -1 if the irq was not active
> + * 1 if completing this irq brought us back to base (no active
> irqs)
> + * 0 if there is still an irq active after this one was completed
> + * (Ignoring -1, this is the same as the RETTOBASE value before completion.)
> + */
> +int armv7m_nvic_complete_irq(void *opaque, int irq);
>
> /* Interface for defining coprocessor registers.
> * Registers are defined in tables of arm_cp_reginfo structs
> diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
> index ca4fb49..a8c5a9e 100644
> --- a/hw/intc/armv7m_nvic.c
> +++ b/hw/intc/armv7m_nvic.c
> @@ -441,10 +441,11 @@ void armv7m_nvic_acknowledge_irq(void *opaque)
> nvic_irq_update(s);
> }
>
> -void armv7m_nvic_complete_irq(void *opaque, int irq)
> +int armv7m_nvic_complete_irq(void *opaque, int irq)
> {
> NVICState *s = (NVICState *)opaque;
> VecInfo *vec;
> + int ret;
>
> assert(irq > ARMV7M_EXCP_RESET && irq < s->num_irq);
>
> @@ -452,6 +453,13 @@ void armv7m_nvic_complete_irq(void *opaque, int irq)
>
> trace_nvic_complete_irq(irq);
>
> + if (!vec->active) {
> + /* Tell the caller this was an illegal exception return */
> + return -1;
> + }
> +
> + ret = nvic_rettobase(s);
> +
> vec->active = 0;
> if (vec->level) {
> /* Re-pend the exception if it's still held high; only
> @@ -462,6 +470,8 @@ void armv7m_nvic_complete_irq(void *opaque, int irq)
> }
>
> nvic_irq_update(s);
> +
> + return ret;
> }
>
> /* callback when external interrupt line is changed */
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index f94d1c7..6a476b4 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -6067,22 +6067,99 @@ static void v7m_push_stack(ARMCPU *cpu)
> v7m_push(env, env->regs[0]);
> }
>
> -static void do_v7m_exception_exit(CPUARMState *env)
> +static void do_v7m_exception_exit(ARMCPU *cpu)
> {
> + CPUARMState *env = &cpu->env;
> uint32_t type;
> uint32_t xpsr;
> -
> + bool ufault = false;
> + bool return_to_sp_process = false;
> + bool return_to_handler = false;
> + bool rettobase = false;
> +
> + /* We can only get here from an EXCP_EXCEPTION_EXIT, and
> + * arm_v7m_do_unassigned_access() enforces the architectural rule
> + * that jumps to magic addresses don't have magic behaviour unless
> + * we're in Handler mode (compare pseudocode BXWritePC()).
> + */
> + assert(env->v7m.exception != 0);
> +
> + /* In the spec pseudocode ExceptionReturn() is called directly
> + * from BXWritePC() and gets the full target PC value including
> + * bit zero. In QEMU's implementation we treat it as a normal
> + * jump-to-register (which is then caught later on), and so split
> + * the target value up between env->regs[15] and env->thumb in
> + * gen_bx(). Reconstitute it.
> + */
> type = env->regs[15];
> + if (env->thumb) {
> + type |= 1;
> + }
> +
> + qemu_log_mask(CPU_LOG_INT, "Exception return: magic PC %" PRIx32
> + " previous exception %d\n",
> + type, env->v7m.exception);
> +
> + if (extract32(type, 5, 23) != extract32(-1, 5, 23)) {
> + qemu_log_mask(LOG_GUEST_ERROR, "M profile: zero high bits in
> exception "
> + "exit PC value 0x%" PRIx32 " are UNPREDICTABLE\n",
> type);
> + }
> +
> if (env->v7m.exception != ARMV7M_EXCP_NMI) {
> /* Auto-clear FAULTMASK on return from other than NMI */
> env->daif &= ~PSTATE_F;
> }
> - if (env->v7m.exception != 0) {
> - armv7m_nvic_complete_irq(env->nvic, env->v7m.exception);
> +
> + switch (armv7m_nvic_complete_irq(env->nvic, env->v7m.exception)) {
> + case -1:
> + /* attempt to exit an exception that isn't active */
> + ufault = true;
> + break;
> + case 0:
> + /* still an irq active now */
> + break;
> + case 1:
> + /* we returned to base exception level, no nesting.
> + * (In the pseudocode this is written using "NestedActivation != 1"
> + * where we have 'rettobase == false'.)
> + */
> + rettobase = true;
> + break;
> + default:
> + g_assert_not_reached();
> + }
> +
> + switch (type & 0xf) {
> + case 1: /* Return to Handler */
> + return_to_handler = true;
> + break;
> + case 13: /* Return to Thread using Process stack */
> + return_to_sp_process = true;
> + /* fall through */
> + case 9: /* Return to Thread using Main stack */
> + if (!rettobase &&
> + !(env->v7m.ccr & R_V7M_CCR_NONBASETHRDENA_MASK)) {
> + ufault = true;
> + }
> + break;
> + default:
> + ufault = true;
> + }
> +
> + if (ufault) {
> + /* Bad exception return: instead of popping the exception
> + * stack, directly take a usage fault on the current stack.
> + */
> + env->v7m.cfsr |= R_V7M_CFSR_INVPC_MASK;
> + armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE);
> + v7m_exception_taken(cpu, type | 0xf0000000);
> + qemu_log_mask(CPU_LOG_INT, "...taking UsageFault on existing "
> + "stackframe: failed exception return integrity
> check\n");
> + return;
> }
>
> /* Switch to the target stack. */
> - switch_v7m_sp(env, (type & 4) != 0);
> + switch_v7m_sp(env, return_to_sp_process);
> /* Pop registers. */
> env->regs[0] = v7m_pop(env);
> env->regs[1] = v7m_pop(env);
> @@ -6106,11 +6183,24 @@ static void do_v7m_exception_exit(CPUARMState *env)
> /* Undo stack alignment. */
> if (xpsr & 0x200)
> env->regs[13] |= 4;
> - /* ??? The exception return type specifies Thread/Handler mode. However
> - this is also implied by the xPSR value. Not sure what to do
> - if there is a mismatch. */
> - /* ??? Likewise for mismatches between the CONTROL register and the stack
> - pointer. */
> +
> + /* The restored xPSR exception field will be zero if we're
> + * resuming in Thread mode. If that doesn't match what the
> + * exception return type specified then this is a UsageFault.
> + */
> + if (return_to_handler == (env->v7m.exception == 0)) {
> + /* Take an INVPC UsageFault by pushing the stack again. */
> + armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE);
> + env->v7m.cfsr |= R_V7M_CFSR_INVPC_MASK;
> + v7m_push_stack(cpu);
> + v7m_exception_taken(cpu, type | 0xf0000000);
> + qemu_log_mask(CPU_LOG_INT, "...taking UsageFault on new stackframe: "
> + "failed exception return integrity check\n");
> + return;
> + }
> +
> + /* Otherwise, we have a successful exception exit. */
> + qemu_log_mask(CPU_LOG_INT, "...successful exception return\n");
> }
>
> static void arm_log_exception(int idx)
> @@ -6183,7 +6273,7 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
> case EXCP_IRQ:
> break;
> case EXCP_EXCEPTION_EXIT:
> - do_v7m_exception_exit(env);
> + do_v7m_exception_exit(cpu);
> return;
> default:
> cpu_abort(cs, "Unhandled exception 0x%x\n", cs->exception_index);
--
Alex Bennée
