Re: [Simulavr-devel] irq handling is broken in simulavr

[Petr Hluzín]

On 24 January 2013 19:26, Klaus Rudolph <address@hidden> wrote:

> Hi all,
>
> yesterday I did one real apllication example on simulavr and it runs
> totally instable on it but works on a real device.
>
> After a few hours I found out that the irq handling is broken.
>
> What I expect:
>
> If a irq comes up the core runs exactly one more instruction before
> changing the pc to jump to the irq vector.
>
> This was already implemented in my early versions, but someone wrote the
> AvrDevice::Step method new and brake the irq behavior. :-(
>
> 0100:    nop
> --- now a hardware irq comes up ----
> 0102:    nop     ; one more instruction runs on core
> 0014:    jmp 0x2000 ; vetcor xx jump
>
> actually the implementation only works correct, if the instruction
> before the irq is a sei() :-) I have no idea why this was implemented
> this way.
>
>
> QUESTION:
>
> If I have the wrong idea to the avr core behavior please give me a hint
> to the correct one!
>
> I will now start working on a hot fix for the topic. If there is no
> other advice I will commit after I finished the fix.
>
> In hope that the actually broken implementation is the problem and not
> my idea of avr and the gcc generated code :-)
>
>
Hello

Do you want to say that after enabling of a _peripheral interrupt_ (e.g.
usart data register empty) the AVR should execute one more instruction?
Do you have some more references?
Or should AVR execute one instruction only for `USART_UDRE_vect' vector?

Datasheets mention that SEI and RETI instructions enable the global
interrupt enable flag I and always execute the next instruction even if an
interrupt is pending. Also it is known (though not documented by Atmel,
[1]) that writing 1 to I bit in SREG by an OUT instruction also always
executes the next instruction. I have not heard that the rule applies also
to the individual int enable flags of peripherals.

Your patch [2] however also delays execution of every subsequent interrupt
-- even if the interrupt-enable flags were enabled all the time. Datasheet
ATmega8 revision Z in chapter "Interrupt Response Time" says "The interrupt
execution response for all the enabled Atmel®AVR® interrupts is four clock
cycles minimum."

I read you test case [3] and if I understand your situation correctly, then
on a real chip it executes `out SPH, r11' instruction before it executes
the UART interrupt routine. In such case the test-case would be faster to
understand if written without messing with SP, like this:

.global main
main:
  ldi r16, 0x00
  ldi r17, 0x01
  out SPL, r16
  out SPH, r17   ; Initialize SP with any good address, needed for ISR
  eor r0, r0
  out PORTB, r0  ; PORTB := 0x00
  ser r18        ; r18 := 0xFF
  out DIRB, r18  ; enable outputs on port B
  sei            ; enable interrupts
  ldi r16, (1<<UDRIE)
  out UCSRB, r16  ; lets generate a usart-data-register-empty irq
  ; simulavr-2012-12-31 will execute ISR now
  out PORTB, r18  ; PORTB := 0xFF
  ; real chip will execute ISR now
  nop
  rjmp .-1        ; endless loop, not reached

.global USART_UDRE_vect
USART_UDRE_vect:
  sbis PORTB,0    ; skip next instruction if bit 0 in PORTB is set
  rjmp .-1        ; endless loop, simulavr will loop here, multimeter would
show 0 on RB pins
  rjmp stopsim    ; real chip will go here, multimeter shows 1 on RB pins

.global stopsim
stopsim:
  nop

I am bit suspicious about the following instruction being executed after
enabling a peripheral's interrupt-enable-flag.
I am even more suspicious about 1 cycle delay for each interrupt firing.

BTW: The changes that introduced the behavior you are complaining about
were committed by me ([4] and [5]). I am fine with removing them if real
chips really have 1 cycle delay in addition to the documented 4 cycles for
pushing the return address and jumping to vector table.

[1] https://savannah.nongnu.org/bugs/?29774
[2]
http://git.savannah.gnu.org/cgit/simulavr.git/commit/?id=ae826d06fbe06fa4b376a48a62844928a3d2afa8
[3]
http://git.savannah.gnu.org/cgit/simulavr.git/commit/?id=90689b1164a42ad001649669d4733eb898c1583c
[4]
http://git.savannah.gnu.org/cgit/simulavr.git/diff/?id=a0e59f554ad5caf7f61e8f8eb08ca3a33a85317f&context=24
[5]
http://git.savannah.gnu.org/cgit/simulavr.git/diff/?id=d51f308606b2d4066d4df27474e48f27c81400ae

-- 
Petr Hluzin