Re: [Qemu-arm] [PATCH] target-arm: v7M: ignore writes to CONTROL.SPSEL from Thread mode

[Philippe Mathieu-Daudé]

Hi Peter,

On 06/30/2017 08:06 AM, Peter Maydell wrote:
> For v7M, writes to the CONTROL register are only permitted for
> privileged code. However even if the code is privileged, the
> write must not affect the SPSEL bit in the CONTROL register
> if the CPU is in Thread mode (as documented in the pseudocode
> for the MSR instruction). Implement this, instead of permitting
> SPSEL to be written in all cases.
>
> This was causing mbed applications not to run, because the
> RTX RTOS they use relies on this behaviour.
>
> Signed-off-by: Peter Maydell <address@hidden> > ---
>   target/arm/helper.c | 13 ++++++++++---
>   1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 2594faa..4ed32c5 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -8768,9 +8768,16 @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t maskreg, 
> uint32_t val)
>           }
>           break;
>       case 20: /* CONTROL */
> -        switch_v7m_sp(env, (val & R_V7M_CONTROL_SPSEL_MASK) != 0);
> -        env->v7m.control = val & (R_V7M_CONTROL_SPSEL_MASK |
> -                                  R_V7M_CONTROL_NPRIV_MASK);
> +        /* Writing to the SPSEL bit only has an effect if we are in
> +         * thread mode; other bits can be updated by any privileged code.
> +         * switch_v7m_sp() deals with updating the SPSEL bit in
> +         * env->v7m.control, so we only need update the others.
> +         */

I'v been thinking about adding some function like v7m_is_privileged() 
v7m_is_thread_mode() !v7m_exception_pending() to ease code readability, 
like armv7m_nvic_can_take_pending_exception() or is_singlestepping().
Not much inspired yet :(

> +        if (env->v7m.exception == 0) {
> +            switch_v7m_sp(env, (val & R_V7M_CONTROL_SPSEL_MASK) != 0);
> +        }
> +        env->v7m.control &= ~R_V7M_CONTROL_NPRIV_MASK;
> +        env->v7m.control |= val & R_V7M_CONTROL_NPRIV_MASK;
>           break;
>       default:
>           qemu_log_mask(LOG_GUEST_ERROR, "Attempt to write unknown special"

Reviewed-by: Philippe Mathieu-Daudé <address@hidden>

Regards,

Phil.