STM8 interrupts - how to properly disable/re-enable within an ISR

[HwAoRrDk]

I'm trying to figure out an issue with STM8 interrupts and ISRs, and I'm wondering if anybody here has any insights or experience.

I have some code where there is a function that needs to atomically modify a global struct variable, so in that function I was disabling interrupts (with SIM instruction) before I modified values, then re-enabling them again (with RIM) before exiting the function. However, I also need to call this function from within an ISR. Naively I just went ahead and did that, but was puzzled to then encounter a problem where the microcontroller appeared to lock up, getting perpetually stuck in the ISR. I soon figured it was due to the use of SIM/RIM within the ISR.

I'm trying to figure out why exactly this causes things to go haywire.

Best I can figure is that it essentially boils down to the use of RIM. My hypothesis is that when I call RIM before the ISR is finished, the I0/I1 flags in the CC register get reset (to 1/0), which re-enables interrupts (or, technically, sets the interrupt priority level to 0), which causes the ISR to immediately be called again, basically interrupting itself. This then repeats ad-infinitum. Are my thoughts correct?

So, how can I avoid this?

In the STM8S Reference Manual (RM0016), there is a part in section 6.2 that talks about a procedure for disabling and enabling interrupts within an ISR. But there are some things about this I'm not sure about.

Their recommendation is:

Code: [Select]

; To disable interrupts
PUSH CC
POP ISR_CC
SIM

; To enable interrupts
PUSH ISR_CC
POP CC

; ISR_CC is a memory location/variable used to store the CC value.

First of all, I don't understand why they are putting the CC value on the stack, then popping it off again to store in a memory location. Why not just leave it on the stack?

Second, when you return from an ISR with IRET, that automatically restores the CC value that was originally saved automatically when calling the ISR. So, why bother saving/restoring it yourself separately? I'm thinking maybe this is aimed at the scenario where you want to disable interrupts only for a certain sub-section of the ISR, not the entire thing. I'm thinking that if you just wanted to disable all other higher-priority interrupts within an ISR, just execute SIM at the start and leave IRET to restore things for you?

So, a solution to my problem (needing to temporarily disable interrupts inside a function that accesses a non-atomic value, but this function is general-purpose, potentially called both outside and within ISRs) may be some variant of the above code. I was thinking of doing the following:

Code: [Select]

#define atomic_begin() do { __asm__("push cc"); __asm__("sim"); } while(0)
#define atomic_end() do { __asm__("pop cc"); } while(0)

These macros can then wrap the relevant section of code within my atomic-access function, and should work both within and outside ISRs. Is this going to work as I expect? Can anyone see any pitfalls in this?

[HwAoRrDk]

First of all, I don't understand why they are putting the CC value on the stack, then popping it off again to store in a memory location. Why not just leave it on the stack?

Answering my own question here:

I've just realised why they're doing this with their recommended code. It makes it so the stack pointer is effectively unchanged. That way, if you're writing in assembly, you don't have to accommodate any change in offset from SP when accessing things that were put on the stack before this 'disable' code. And for C, you don't have to worry about interfering with whatever the compiler's idea of SP offsets are for local variables.

This means my proposed macros won't work - or, rather, may work but are risky due to potentially messing up the compiler's SP offsets.

[newbrain]

I've just realised why they're doing this with their recommended code.

But wait, there's more!
One hint is given in the note just above the code:

Caution:
If the interrupt mask bits I0 and I1 are set within an interrupt service routine (ISR) with the instruction SIM, removal of the interrupt mask with RIM causes the software priority to be set to level 0.

So RIM should not be used in an ISR.
A second possible (first time I have a look at an STM-8, so I'm a bit out on a limb) use of this code is if you need to nest functions that might disable interrupts: by saving and restoring the CC you need not care whether interrupts were disabled or not before entering this section, the right value will be restored with the POP.

[Peabody]

I have no experience with STM8, but aren't interrupts disabled automatically when an interrupt occurs and the ISR is executed.  If so, then it seems all you need to do is call a version of your function from within the ISR which does nothing to the interrupts.  That same version could be called from outside the ISR by another function which first disables interrupts, calls the function, then re-enables them.

[newbrain]

aren't interrupts disabled automatically when an interrupt occurs and the ISR is executed

No. The OP provided a link to the RM.

[HwAoRrDk]

I have no experience with STM8, but aren't interrupts disabled automatically when an interrupt occurs and the ISR is executed.  If so, then it seems all you need to do is call a version of your function from within the ISR which does nothing to the interrupts.  That same version could be called from outside the ISR by another function which first disables interrupts, calls the function, then re-enables them.

Yes, normally they are disabled automatically. However, the STM8 also optionally supports a nested interrupt scheme, where you can assign one of 3 priority levels to each interrupt vector. So it means that if you've done that, your ISR could itself be interrupted by a higher-priority interrupt and jump off to another ISR in the middle of the current ISR. At least, that's how I understand it.

So there is application for wanting to disable all interrupts inside an ISR besides my case of trying to have a general-purpose function that disables interrupts properly regardless of whether it is called inside or outside an ISR.

[HwAoRrDk]

I have been ruminating on this subject some more, and I think there is a different approach I can take besides macros.

I came up with the following code:

Code: [Select]

uint8_t atomic_begin(void) __naked {
__asm
; Copy CC value, put in A reg for return value, and disable all interrupts.
push cc
pop a
sim
ret
__endasm;
}

void atomic_end(const uint8_t istate) {
(void)istate;

__asm
; Restore CC from arg value in A reg.
push a
pop cc
__endasm;
}

This approach defines two functions: one to save (and return) the CC register value before disabling all interrupts, and another to restore it (from argument given). This way I don't have to worry about inadvertently interfering with the compiler's stack pointer offsets, and it can be up to the caller to store the saved value however or wherever they like (global static var, local stack var, whatever).

My only slight concern with this approach is that the 'end' function will clobber all other status flags in CC (e.g. carry, overflow, etc.). But, I believe the compiler should expect that CC flags could have changed after returning from a function call, so I think this should be okay.