Re: [Qemu-devel] Disable interrupts on Cortex M3 (lm3s6965evb)

[Sebastian Huber <sebastian.huber@embedded-brains.de>]

[-- Attachment #1: Type: text/plain, Size: 2131 bytes --]

On 05/06/11 15:44, Peter Maydell wrote:
> On 5 June 2011 14:06, Sebastian Huber
> <sebastian.huber@embedded-brains.de> wrote:
>   
>> I think the interrupt handling logic for ARMv7M is wrong in cpu-exec.c
>> line 470.  Please have a look at the attached patch.
>>     
> --- a/cpu-exec.c
> +++ b/cpu-exec.c
> @@ -470,8 +470,8 @@ int cpu_exec(CPUState *env1)
>                         We avoid this by disabling interrupts when
>                         pc contains a magic address.  */
>                      if (interrupt_request & CPU_INTERRUPT_HARD
> -                        && ((IS_M(env) && env->regs[15] < 0xfffffff0)
> -                            || !(env->uncached_cpsr & CPSR_I))) {
> +                        && !(env->uncached_cpsr & CPSR_I)
> +                        && (IS_M(env) && env->regs[15] < 0xfffffff0)) {
>                          env->exception_index = EXCP_IRQ;
>                          do_interrupt(env);
>                          next_tb = 0;
>
> This doesn't look right -- it changes the behaviour in the
> case where we aren't an M profile CPU.
>   

Yes, you are right.  Please see attached version.

> In any case, M profile exception priority handling is sufficiently
> complicated that any change which only looks at PRIMASK (which is
> effectively what the change to look at CPSR_I here is doing) is
> almost certainly wrong. I think that whatever is raising the
> interrupt should be looking at the CPU priority and not raising it
> in the first place.
>   

Yes.  Please have a look at:

http://lists.nongnu.org/archive/html/qemu-devel/2011-05/msg03132.html

It is also not possible to set the priority of the standard exceptions
like PendSC etc. via the  System Handler Priority Register 1-3 (this
part is missing in gic_dist_{read, write}b()).

> (It looks suspiciously as if most of the v7M priority handling
> is simply missing from QEMU, ie you have bigger problems than
> can be fixed by a small patch like this...)
>   

Yes, but the current behaviour is definitely not right.  Since the
PRIMASK is mapped to the I bit in the CPSR I guessed that this was the
right place to fix it.

[-- Attachment #2: 0001-Fixed-interrupt-handling-for-ARMv7M.patch --]
[-- Type: text/x-patch, Size: 1130 bytes --]

>From 917f2491c1dc2525b24c635afe4459e55700149c Mon Sep 17 00:00:00 2001
From: Sebastian Huber <sebastian.huber@embedded-brains.de>
Date: Sun, 5 Jun 2011 14:57:17 +0200
Subject: [PATCH] Fixed interrupt handling for ARMv7M.

Signed-off-by: Sebastian Huber <sebastian.huber@embedded-brains.de>
---
 cpu-exec.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index 6ddd8dd..d1e9816 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -470,8 +470,8 @@ int cpu_exec(CPUState *env1)
                        We avoid this by disabling interrupts when
                        pc contains a magic address.  */
                     if (interrupt_request & CPU_INTERRUPT_HARD
-                        && ((IS_M(env) && env->regs[15] < 0xfffffff0)
-                            || !(env->uncached_cpsr & CPSR_I))) {
+                        && !(env->uncached_cpsr & CPSR_I)
+                        && (!IS_M(env) || env->regs[15] < 0xfffffff0)) {
                         env->exception_index = EXCP_IRQ;
                         do_interrupt(env);
                         next_tb = 0;
-- 
1.7.1