[Qemu-devel] [Bug 1226531] [NEW] Incorrect logic in ARMv7M interrupt handler

[Qemu-devel] [Bug 1226531] [NEW] Incorrect logic in ARMv7M interrupt handler

[benno]

Public bug reported:

On ARMv7M interrupts handlers will be called even if emulated code
executes "cpsid i" instruction.

Underlying cause described below:

In cpu-exec.c:cpu_exec there is a block of code that determines if an
interrupt should be raised or not:


                    /* ARMv7-M interrupt return works by loading a magic value
                       into the PC.  On real hardware the load causes the
                       return to occur.  The qemu implementation performs the
                       jump normally, then does the exception return when the
                       CPU tries to execute code at the magic address.
                       This will cause the magic PC value to be pushed to
                       the stack if an interrupt occurred at the wrong time.
                       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->exception_index = EXCP_IRQ;
                        cc->do_interrupt(cpu);
                        next_tb = 0;
                    }

I'm not convinced the logic is correct.
The logic for ARMv7M should be:

If an interrupt is pending (interrupt_request & CPU_INTERRUPT_HARD)
AND
Interrupts are not disabled ( !(env->uncached_cpsr & CPSR_I) )
AND
PC doesn't have a magic value (env->regs[15] < 0xfffffff0)

The current logic seems fires the interrupt if interrupts are enabled OR
the PC isn't magic, which is basically all the time.

I'm not sure what the cleanest patch for this would be.

** Affects: qemu
     Importance: Undecided
         Status: New


** Tags: armv7m

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1226531

Title:
  Incorrect logic in ARMv7M interrupt handler

Status in QEMU:
  New

Bug description:
  On ARMv7M interrupts handlers will be called even if emulated code
  executes "cpsid i" instruction.

  Underlying cause described below:

  In cpu-exec.c:cpu_exec there is a block of code that determines if an
  interrupt should be raised or not:

  
                      /* ARMv7-M interrupt return works by loading a magic value
                         into the PC.  On real hardware the load causes the
                         return to occur.  The qemu implementation performs the
                         jump normally, then does the exception return when the
                         CPU tries to execute code at the magic address.
                         This will cause the magic PC value to be pushed to
                         the stack if an interrupt occurred at the wrong time.
                         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->exception_index = EXCP_IRQ;
                          cc->do_interrupt(cpu);
                          next_tb = 0;
                      }

  I'm not convinced the logic is correct.
  The logic for ARMv7M should be:

  If an interrupt is pending (interrupt_request & CPU_INTERRUPT_HARD)
  AND
  Interrupts are not disabled ( !(env->uncached_cpsr & CPSR_I) )
  AND
  PC doesn't have a magic value (env->regs[15] < 0xfffffff0)

  The current logic seems fires the interrupt if interrupts are enabled
  OR the PC isn't magic, which is basically all the time.

  I'm not sure what the cleanest patch for this would be.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1226531/+subscriptions

Re: [Qemu-devel] [Bug 1226531] [NEW] Incorrect logic in ARMv7M interrupt handler

[Peter Maydell]

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

On 17 September 2013 11:44, benno <benno@benno.id.au> wrote:
> I'm not convinced the logic is correct.

It's not. There have been a few attempts by people to submit patches
to this though, but none of them have actually been sufficiently
convincing. See for instance
http://lists.nongnu.org/archive/html/qemu-devel/2013-05/msg04546.html

If somebody produces a patch which comes with a good rationale
for its change (ie with reference to the architecture manual and to
what QEMU means when it sets "CPSR_I" on M profile) I'll apply
it. But because the v7M code is currently not really maintained and
v7M interrupts are complex I'm reluctant to apply patches which
only come with "seems to fix things for me" levels of justification.

-- PMM

[-- Attachment #2: Type: text/html, Size: 1043 bytes --]

[Qemu-devel] [Bug 1226531] Re: Incorrect logic in ARMv7M interrupt handler

[Peter Maydell]

We finally fixed this longstanding ARMv7M emulation bug in the 2.10
release (by rewriting the NVIC handling entirely).


** Changed in: qemu
       Status: New => Fix Released

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1226531

Title:
  Incorrect logic in ARMv7M interrupt handler

Status in QEMU:
  Fix Released

Bug description:
  On ARMv7M interrupts handlers will be called even if emulated code
  executes "cpsid i" instruction.

  Underlying cause described below:

  In cpu-exec.c:cpu_exec there is a block of code that determines if an
  interrupt should be raised or not:

  
                      /* ARMv7-M interrupt return works by loading a magic value
                         into the PC.  On real hardware the load causes the
                         return to occur.  The qemu implementation performs the
                         jump normally, then does the exception return when the
                         CPU tries to execute code at the magic address.
                         This will cause the magic PC value to be pushed to
                         the stack if an interrupt occurred at the wrong time.
                         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->exception_index = EXCP_IRQ;
                          cc->do_interrupt(cpu);
                          next_tb = 0;
                      }

  I'm not convinced the logic is correct.
  The logic for ARMv7M should be:

  If an interrupt is pending (interrupt_request & CPU_INTERRUPT_HARD)
  AND
  Interrupts are not disabled ( !(env->uncached_cpsr & CPSR_I) )
  AND
  PC doesn't have a magic value (env->regs[15] < 0xfffffff0)

  The current logic seems fires the interrupt if interrupts are enabled
  OR the PC isn't magic, which is basically all the time.

  I'm not sure what the cleanest patch for this would be.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1226531/+subscriptions