Porting help, possible bug in AVR port

[swinchen]

Hi All,


I have been poking around in the AVR port to see what I would need to to adjust it to work with the xmega line of devices. Looking at the disassembly I noticed that on devices with > 64kB of flash the RAMPZ register gets saved by the compiler in the interrupt service routines. This does not appear to be included in the extctx structure. Is this a bug, or am I not understanding something correctly?

I am going to have troubles wording this question, but here I go: There seems to be two paths that lead to a context switch; either from an ISR calling the reschedule routine (which calls port_switch in addition to PORT_IRQ_PROLOGUE), or a thread calling one of the sleep routines (also calls port_switch). How can I save RAMPZ when from a non-ISR context switch so I do not double-push RAMPZ when an ISR causes a context switch?

If you need more clarity just ask. Sorry that sounds so confusing!

Thanks,
Sam

[Giovanni]

Hi,

The AVR port has not been updated for a while and devices with more than 64KB are not currently supported.

About the RAMPZ register, the rule is that registers that are preserved through function calls by the compiler must be included in the intctx structure, the register that are not preserver by function calls go into the extctx, I believe RAMPZ should only go in extctx but better check what the compiler does when calling a function.

The changes to the port would be limited:
1) Introduction of an option switch for large devices.
2) Change to exctx based on the switch.

Giovanni

[swinchen]

So I wrote a quick and dirty test:

Code: Select all

#include <avr/interrupt.h>
#include <avr/io.h>

ISR(PORTB_INT0_vect) {
    EIND  = 0x01;
    RAMPD = 0x01;
    RAMPX = 0x01;
    RAMPY = 0x01;
    RAMPZ = 0x01;
}

void testZ() {
    EIND  = 0x02;
    RAMPD = 0x02;
    RAMPX = 0x02;
    RAMPY = 0x02;
    RAMPZ = 0x02;
}

int main(int argc, char* argv[]) {

    EIND  = 0x03;
    RAMPD = 0x03;
    RAMPX = 0x03;
    RAMPY = 0x03;
    RAMPZ = 0x03;
    testZ();

    for(;;) {
   
    }   
}


compiled with:

Code: Select all

avr-gcc -mmcu=atxmega128a1 -O1 main.c

results:

Code: Select all

----- snip -----
00000244 <__vector_34>:
 244:   1f 92          push   r1
 246:   0f 92          push   r0
 248:   0f b6          in   r0, 0x3f   ; 63
 24a:   0f 92          push   r0
 24c:   08 b6          in   r0, 0x38   ; 56
 24e:   0f 92          push   r0
 250:   11 24          eor   r1, r1
 252:   8f 93          push   r24
 254:   81 e0          ldi   r24, 0x01   ; 1
 256:   8c bf          out   0x3c, r24   ; 60
 258:   88 bf          out   0x38, r24   ; 56
 25a:   89 bf          out   0x39, r24   ; 57
 25c:   8a bf          out   0x3a, r24   ; 58
 25e:   8b bf          out   0x3b, r24   ; 59
 260:   8f 91          pop   r24
 262:   0f 90          pop   r0
 264:   08 be          out   0x38, r0   ; 56
 266:   0f 90          pop   r0
 268:   0f be          out   0x3f, r0   ; 63
 26a:   0f 90          pop   r0
 26c:   1f 90          pop   r1
 26e:   18 95          reti

00000270 <testZ>:
 270:   82 e0          ldi   r24, 0x02   ; 2
 272:   8c bf          out   0x3c, r24   ; 60
 274:   88 bf          out   0x38, r24   ; 56
 276:   89 bf          out   0x39, r24   ; 57
 278:   8a bf          out   0x3a, r24   ; 58
 27a:   8b bf          out   0x3b, r24   ; 59
 27c:   08 95          ret

0000027e <main>:
 27e:   83 e0          ldi   r24, 0x03   ; 3
 280:   8c bf          out   0x3c, r24   ; 60
 282:   88 bf          out   0x38, r24   ; 56
 284:   89 bf          out   0x39, r24   ; 57
 286:   8a bf          out   0x3a, r24   ; 58
 288:   8b bf          out   0x3b, r24   ; 59
 28a:   0e 94 38 01    call   0x270   ; 0x270 <testZ>
 28e:   ff cf          rjmp   .-2         ; 0x28e <main+0x10>
----- snip -----


So it looks like functions do not save any of the RAMP registers, or EIND register. RAMPD _IS_ saved by the ISR (starting at 0x24C). Does this mean I should put all of these in extctx and add the appropriate save code to port_switch? If I do that it seems like RAMPD will be saved twice during a preemptive context switch (once by the compiler and another in port_switch).

Thanks for the feedback Giovanni!

Sam

[Giovanni]

If the registers are not saved by normal functions then it is just matter to update the extctx structure. If the compiler saves the special registers in ISR() functions prologue/epilogue then you don't even need to add any special code.

Also note that the extctx structure in that port is only used to calculate stack sizes so even the order of the fields is not really important, it is better to have it accurate anyway.

Giovanni

[swinchen]

So let us assume that a thread has modified several of the special RAMP registers and then calls chThdSleepUntil. This is the order that functions get called to do a context switch:

chThdSleepUntil -> chSchGoSleepTimeoutS -> chSchGoSleepS -> port_switch

So those special registers must be saved in the port_switch function correct? Even if they are saved by the compiler during an ISR.

I feel like I am very close to understanding this, but I do not understand:

If the compiler saves the special registers in ISR() functions prologue/epilogue then you don't even need to add any special code.

because it seems like you still have to save these registers if it gets to a context switch without going through an interrupt service routine.

I must be missing something....

Sorry to be such a pain!
Sam

[Giovanni]

It is simple: if a register is not one of those preserved by functions when the compiler calls port_switch() it expects it altered after the call because port_switch() *is* a function, it does not matter it switches to another thread, on return the compiler expects those registers to contain trash.

The compiler never tries to pass info across function calls in registers that function calls are allowed to not save.

Giovanni

[swinchen]

Ohhhhhhh. I can't believe I didn't see that! That is so simple.

I will have to add code in PORT_IRQ_PROLOGUE and PORT_IRQ_EPILOGUE to save some of these special registers (not RAMPD, because the compiler does that for me).


Ok... That makes a lot more sense now. Thanks Giovanni

[swinchen]

Ugh, this has gotten more complicated. So I was porting over the system tick ISR, and noticed that the compiler pushes these special registers based on the code in the ISR:

Code: Select all

----- snip -----
00005a6c <__vector_14>:
    5a6c:       1f 92           push    r1
    5a6e:       0f 92           push    r0
    5a70:       0f b6           in      r0, 0x3f        ; 63
    5a72:       0f 92           push    r0
    5a74:       08 b6           in      r0, 0x38        ; 56
    5a76:       0f 92           push    r0
    5a78:       09 b6           in      r0, 0x39        ; 57
    5a7a:       0f 92           push    r0
    5a7c:       0b b6           in      r0, 0x3b        ; 59
    5a7e:       0f 92           push    r0
    5a80:       11 24           eor     r1, r1
    5a82:       2f 93           push    r18
    5a84:       3f 93           push    r19
    5a86:       4f 93           push    r20
    5a88:       5f 93           push    r21
    5a8a:       6f 93           push    r22
    5a8c:       7f 93           push    r23
    5a8e:       8f 93           push    r24
    5a90:       9f 93           push    r25
    5a92:       af 93           push    r26
    5a94:       bf 93           push    r27
    5a96:       ef 93           push    r30
    5a98:       ff 93           push    r31
----- snip -----


Seeing these special "registers" can not be saved with the asm trick in PORT_IRQ_PROLOGUE I am not sure how to keep track what is saved in the service routines. Do you have any tips? All I can think of is declare the service routines naked and write the entire prologue/epilogue manually. There is also the OS_main/OS_task attributes I could look at... seems like they do a bit more than naked, but still do not save registers.

Is this something you have run into before?

Thanks again.

[Giovanni]

Hi,

There is no need to track what is actually saved in the prologue, extctx should reflect the case when all registers are saved. The structure is only used to determine the worst case overhead on the thread stack caused by ISRs. The only use of that structure is sizeof(struct extctx), individual fields are never accessed.

Giovanni

[swinchen]

Alright, that makes sense... but I should still save all of special "registers" in the ISR prologue right? That way when a task is switched back in these registers won't contain the garbage from a previous thread.

If that is true the problem I am facing is that the compiler saves some of these registers depending on the contents of the ISR (varies based on what is going on in the ISR). If I save all of these special registers in the PORT_IRQ_PROLOGUE I will be pushing some duplicates onto the stack. If I don't save any of them in PORT_IRQ_PROLOGUE it is possible that the next task will trash one or more of the special registers that are not saved on the stack. Does that make sense?