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

Peter Maydell posted 1 patch 6 years, 9 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1498820791-8130-1-git-send-email-peter.maydell@linaro.org
Test FreeBSD passed
Test checkpatch passed
Test docker passed
Test s390x passed
target/arm/helper.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
[Qemu-devel] [PATCH] target-arm: v7M: ignore writes to CONTROL.SPSEL from Thread mode
Posted by Peter Maydell 6 years, 9 months ago
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 <peter.maydell@linaro.org>
---
 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.
+         */
+        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"
-- 
2.7.4


Re: [Qemu-devel] [Qemu-arm] [PATCH] target-arm: v7M: ignore writes to CONTROL.SPSEL from Thread mode
Posted by Philippe Mathieu-Daudé 6 years, 8 months ago
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 <peter.maydell@linaro.org> > ---
>   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é <f4bug@amsat.org>

Regards,

Phil.