Search code examples
cmultithreadingkernelrtosisr

Atomic disable and restore interrupts from ISR and non-ISR context: may it be different on some platform?


I work with embedded stuff, namely PIC32 Microchip CPUs these days.

I'm familiar with several real-time kernels: AVIX, FreeRTOS, TNKernel, and in all of them we have 2 versions of nearly all functions: one for calling from task, and second one for calling from ISR.

Of course it makes sense for functions that could switch context and/or sleep: obviously, ISR can't sleep, and context switch should be done in different manner. But there are several functions that do not switch context nor sleep: say, it may return system tick count, or set up software timer, etc.

Now, I'm implementing my own kernel: TNeoKernel, which has well-formed code and is carefully tested, and I'm considering to invent "universal" functions sometimes: the ones that can be called from either task or ISR context. But since all three aforementioned kernels use separate functions, I'm afraid I'm going to do something wrong.

Say, in task and ISR context, TNKernel uses different routines for disabling/restoring interrupts, but as far as I see, the only possible difference is that ISR functions may be "compiled out" as an optimization if the target platform doesn't support nested interrupts. But if target platform supports nested interrupts, then disabling/restoring interrupts looks absolutely the same for task and ISR context.

So, my question is: are there platforms on which disabling/restoring interrupts from ISR should be done differently than from non-ISR context?

If there are no such platforms, I'd prefer to go with "universal" functions. If you have any comments on this approach, they are highly appreciated.

UPD: I don't like to have two set of functions because they lead to notable code duplication and complication. Say, I need to provide a function that should start software timer. Here is what it looks like:

enum TN_RCode _tn_timer_start(struct TN_Timer *timer, TN_Timeout timeout)
{
   /* ... real job is done here ... */
}

/*
 * Function to be called from task
 */
enum TN_RCode tn_timer_start(struct TN_Timer *timer, TN_Timeout timeout)
{
   TN_INTSAVE_DATA;  //-- define the variable to store interrupt status,
                     //   it is used by TN_INT_DIS_SAVE()
                     //   and TN_INT_RESTORE()
   enum TN_RCode rc = TN_RC_OK;

   //-- check that function is called from right context
   if (!tn_is_task_context()){
      rc = TN_RC_WCONTEXT;
      goto out;
   }

   //-- disable interrupts
   TN_INT_DIS_SAVE();

   //-- perform real job, after all
   rc = _tn_timer_start(timer, timeout);

   //-- restore interrupts state
   TN_INT_RESTORE();

out:
   return rc;
}

/*
 * Function to be called from ISR
 */
enum TN_RCode tn_timer_istart(struct TN_Timer *timer, TN_Timeout timeout)
{
   TN_INTSAVE_DATA_INT;    //-- define the variable to store interrupt status, 
                           //   it is used by TN_INT_DIS_SAVE()                
                           //   and TN_INT_RESTORE()                           
   enum TN_RCode rc = TN_RC_OK;

   //-- check that function is called from right context
   if (!tn_is_isr_context()){
      rc = TN_RC_WCONTEXT;
      goto out;
   }

   //-- disable interrupts
   TN_INT_IDIS_SAVE();

   //-- perform real job, after all
   rc = _tn_timer_start(timer, timeout);

   //-- restore interrupts state
   TN_INT_IRESTORE();

out:
   return rc;
}

So, we need wrappers like the ones above for nearly all system function. This is a kind of inconvenience, for me as a kernel developer as well as for kernel users.

The only difference is that different macros are used: for task, these are TN_INTSAVE_DATA, TN_INT_DIS_SAVE(), TN_INT_RESTORE(); for interrupts these are TN_INTSAVE_DATA_INT, TN_INT_IDIS_SAVE(), TN_INT_IRESTORE().

For the platforms that support nested interrupts (ARM, PIC32), these macros are identical. For other platforms that don't support nested interrupts, TN_INTSAVE_DATA_INT, TN_INT_IDIS_SAVE() and TN_INT_IRESTORE() are expanded to nothing. So it is a bit of performance optimization, but the cost is too high in my opinion: it's harder to maintain, it's not so convenient to use, and the code size increases.


Solution

  • It's all a matter of design and CPU capabilities. I'm not familiar with any of the PICs but, for example, Freescale (Motorola) MCUs (among many others) have the ability to move the Condition Code Register (CCR) into the accumulator and back. This allows one to save the previous state of the Interrupt Enable/Disable Mask, and restore it at the end, without worrying about bluntly enabling interrupts where they should stay disabled (inside ISRs).

    To answer, however, which platform(s) must do it differently inside and outside ISRs would require one to be familiar with all of them, or at least one that fails this test. If there is a CPU that does not allow saving and restoring the CCR (as mentioned above), one would have no option but to do it differently for each case.