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

[Klaus Rudolph]

Hi Petr,

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

Sorry, no :-(


> Or should AVR execute one instruction only for `USART_UDRE_vect' vector?

1) My understanding is an interrupt will be executed if the I flag in SR
is set and any hardware interrupt flag is set. From this point the core
loads the next instruction from the vector. During this load stage the
next instruction which was loaded in the step before will be executed,
which means that always one instruction after rising the hardware
interrupt will be executed.

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

Not knowing the details to 1) ( which is only my idea of the things
behind the curtain ) there are some more cases where the I flag in SR is
affected. The code which brake the simulation uses a out instruction to SR.

0x1b56: _Z17HandyHandler100Hzv+0x3d    SUBI R28, 0x80 SREG=[IT------]
0x1b58: _Z17HandyHandler100Hzv+0x3e    SBCI R29, 0xff SREG=[ITH----C]
0x1b5a: _Z17HandyHandler100Hzv+0x3f    IN R0, 0x3f
0x1b5c: _Z17HandyHandler100Hzv+0x40    CLI SREG=[-TH----C]
0x1b5e: _Z17HandyHandler100Hzv+0x41    OUT 0x3e, R29 SP=0x8b2
0x1b60: _Z17HandyHandler100Hzv+0x42    OUT 0x3f, R0   << ENABLE IRQ!
0x1b62:  interrupt on index 12 is pending
0x1b62: _Z17HandyHandler100Hzv+0x43    CPU-waitstate
0x1b62: _Z17HandyHandler100Hzv+0x43    CPU-waitstate
0x1b62: _Z17HandyHandler100Hzv+0x43    CPU-waitstate
0x0030:                                JMP 205c __vector_12


The instruction in 0x1b62 was not executed in simulavr before my patch.

0x1b62:  _Z17HandyHandler100Hzv+0x44    OUT 0x3d, R28   ; 61

But this is instruction is needed to have the correct stack pointer.

gcc generate this code if some bigger vars will be allocated on stack.
While entering the subroutine the stack wil be "added" and before ret
the stack will be subtracted. The irq will be blocked with cli and
switched on by an out instruction to SR.

If my solution is not timing correct, there must be an other change that
will take care about *any* modification of the i flag in sr and not only
by sei instruction.

But my iunderstanding is, that we have a 2 stage pipeline and the
execution flow is interrupted by inserting the instruction from the
vector if the irq is allowed ( i flag in sr ) and the hardware raise the
flag. While we have a 2 stage pipeline, we have one instruction from the
"old flow". I have no idea why there should be another scenario.

But I did not belief that we have additional 4 wait states after the jmp
is loaded. Because that is a 4 byte instruction, I expect one waitstate
to fetch the second 2 bytes half of the instruction before execution.
But not 4 ones.

It would be nice if someone can check that out with jtag on a real device.

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

Yes, this is the code which I have seen above and which could not be
executed without my patch.


> I have not heard that the rule applies also
> to the individual int enable flags of peripherals.

That is also my problem. But I think that the 2 stage pipeline is the
reason why the irq execution is delayed. And this would be the the
machanism if the irq is raised by setting the hardware flag.

OK, I hope that I could spend some time the next days for a test case
which work on real hardware that shows that that the next instruction
will be executed or not. It should be simple to set an output to a logic
level and run into a irq which will never return. That seems to be an
easy and funny job :-)

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

Yes, but maybe not delayed with wait states but with executing the last
instruction after the irq.


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

Feel free to change the testcase. The stack is already set by the c-lib
code. The code is compiled with gcc -xassembler-with-cpp which have the
stack setup in front of main and also irq vectors will be placed well.


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

Yes, I hope I can give a real world testcase which shows the thing.
Maybe you also have an idea which can make the things clear.


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

I think we have two possible solutions:

The patch that a committed yesterday, if the delay is always "true".
Or having a patch which "looks" for all I-flag manipulations, not only
on bset (sei) instruction.

In hope that I or someone else can give a good testcase on a real
hardware. Maybe someone can help with jtag? That would be very nice!

Thanks for your thoughts!


Regards
 Klaus