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

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

[Sebastian Huber]

Hello,

how can I disable interrupts on the Cortex M3 based board LM3S6965EVB?
Even if I set PRIMASK, FAULTMASK and BASEPRI to 1 the SYSTICK interrupt
handler gets called.

Have a nice day!

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

[Sebastian Huber]

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

On 05/06/11 12:31, Sebastian Huber wrote:
> Hello,
>
> how can I disable interrupts on the Cortex M3 based board LM3S6965EVB?
> Even if I set PRIMASK, FAULTMASK and BASEPRI to 1 the SYSTICK interrupt
> handler gets called.
>
> Have a nice day!
>   

I think the interrupt handling logic for ARMv7M is wrong in cpu-exec.c
line 470.  Please have a look at the attached patch.

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

>From b424c34d0202950307b3a12778a06834917b2947 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.

---
 cpu-exec.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index 6ddd8dd..2782076 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

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

[Peter Maydell]

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.

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.

(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...)

-- PMM

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

[Sebastian Huber]

[-- 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

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

[Peter Maydell]

On 5 June 2011 15:17, Sebastian Huber
<sebastian.huber@embedded-brains.de> wrote:
> On 05/06/11 15:44, Peter Maydell wrote:

>> 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

That patch does correct an error in the decode of the basepri
registers (and so we might as well apply it), but it does not
affect the fact that the underlying v7m.basepri field is not
actually used for anything.

>> (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.

I agree that the current behaviour is not right. However, to fix
this problem you need to work on a larger scale than attempting
to apply two line patches which fix your particular use case.

-- PMM

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

[Sebastian Huber]

On 05/06/11 16:57, Peter Maydell wrote:
> On 5 June 2011 15:17, Sebastian Huber
> <sebastian.huber@embedded-brains.de> wrote:
>   
>> On 05/06/11 15:44, Peter Maydell wrote:
[...]
>>> (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.
>>     
> I agree that the current behaviour is not right. However, to fix
> this problem you need to work on a larger scale than attempting
> to apply two line patches which fix your particular use case.
>   

I agree, but you have to start somewhere.  What is "this problem"?  Is
that we have no execution priority (in the sense of the ARMv7
architecture, B1.3.2 Exceptions), but instead use a mapping to CPSR_I
and CPSR_F?

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

[Peter Maydell]

On 5 June 2011 17:32, Sebastian Huber
<sebastian.huber@embedded-brains.de> wrote:
> On 05/06/11 16:57, Peter Maydell wrote:
>> I agree that the current behaviour is not right. However, to fix
>> this problem you need to work on a larger scale than attempting
>> to apply two line patches which fix your particular use case.
>
> I agree, but you have to start somewhere.  What is "this problem"?  Is
> that we have no execution priority (in the sense of the ARMv7
> architecture, B1.3.2 Exceptions), but instead use a mapping to CPSR_I
> and CPSR_F?

There is some notion of priority, see gic_update()
in hw/arm_gic.c; but it is only within the gic and is not
dealing with v7M specific issues.

At the moment I am mostly just warning you that you're entering
difficult territory; if I have time to read the qemu v7m code
more carefully next week I may have more concrete opinions.

-- PMM