Cortex-M4 interrupt pending behavior and generic_isr design

[Rajiv M Ranganath]

Hi All,

Thank you for TockOS project. I am new to TockOS community, and still
coming up to speed with the code base.

When reading the `cortexm4::generic_isr` code and its interaction with
`sched::{{impl}}::kernel_loop` and
`chip_crate::chip::{{impl}}::service_pending_interrupts()`, I noticed
there could be lost interrupts.

This turned out to be a long email. My apologies in advance for that!

Later in this note, I have pointers to code that demonstrates the issue.
If required, I'll be happy to get on Google Hangouts and a walk you
through the lost interrupt behavior I'm seeing using GDB.

BTW, since I am new to Tock code base, I could be completely wrong in my
understanding of `cortexm4::generic_isr` and related code. If so, please
let me know! :-)

In `cortexm4::generic_isr`, upon entry, we deactivate the interrupt and
ensure that LR (EXC_RETURN) is setup so that on ISR exit, we enter into
kernel mode (thread/privileged).

Then within kernel mode,
`chip_crate::chip::{{impl}}::service_pending_interrupts()` would take
care of handling pending interrupts. This is followed by,

```
let n = cortexm4::nvic::Nvic::new(interrupt);
n.clear_pending();
n.enable();
```

suggesting that `cortexm4::generic_isr` is expected to disable the
interrupt, which would later be re-enabled by
`chip_crate::chip::{{impl}}::service_pending_interrupts()`. Prior to
re-enabling the interrupt, we also clear its pending status, suggesting
that pending status is not automatically cleared.

However, in a Cortex-M microcontroller, upon entry to ISR, the "pending"
status of the interrupt is automatically cleared. This is mentioned in
Section 7.6, Page 247 of Joseph Yiu's ARM book [1].

When are executing `cortexm4::generic_isr`, we see pending status set
only if the NVIC interrupt line is pulsed again. This scenario is
mentioned in Page 250, Figure 7.19. [1]

Following seems to be the implications -

1. There is a very high probability of losing the first interrupt. If
there was a critical peripheral that sent an interrupt only once,
then losing that interrupt could lead to other issues.

The only time we would perhaps not lose the first interrupt was if
`service_pending_interrupts` caught and acted on pending status
before `generic_isr` was triggered, which is very unlikely.

2. Peripheral needs to send multiple interrupts, which many peripherals
do. If we land in `cortexm4::generic_isr`, then we would lose the
interrupt. However, if we are in
`chip_crate::chip::{{impl}}::service_pending_interrupts()` with
interrupt disabled and pending set, we would service it.

3. Upon re-activating the ISR for the interrupt, if we get another
interrupt and land in `cortexm4::generic_isr`, we would lose the
interrupt again because we lose its pending status upon entry.

I've posted code on GitHub to demonstrate this behavior. I am using a
Nucleo-F446RE board. I do not have a Hail/Imix board. If required I will
be happy to adapt code for these boards.

Commit a59547ac [2] adds support for `cortex-m-nvic` crate. Within the
`kernel` crate, due to circular dependency, we cannot directly use
`cortex-m4` or `cortex-m` crates.

Therefore we need to introduce `cortex-m-nvic` crate, which provides the
same functionality as `cortexm::nvic::Nvic`. There are two additional
methods on `Nvic` struct - `set_pending` and `get_pending` to help us
with debugging.

Commit 3efcdff3 [3] adds two code examples with a simplified kernel
loop.

We can investigate interrupt behavior by setting breakpoints at
`kernel_loop` and `generic_isr`.

In the first example [4], using `chip.atomic`, we simulate an interrupt
on IRQ 10.

Prior to pending the interrupt, we can see that IRQ 10 is enabled and
not-pending by examining NVIC.ISER[0] (0xE000E100) and NVIC.ISPR[0]
(0xE000E200)

```
249 irq_10.set_pending();

>>> x/1tw 0xE000E100
0xe000e100: 11111111111111111111111111111111

>>> x/1tw 0xE000E200
0xe000e200: 00000000000000000000000000000000
```

After pending IRQ 10, we can see that its pending state is set by
examining NVIC.ISPR[0] (0xE000E200) again. However, since PRIMASK is set
within the closure, `generic_isr` is not yet invoked.

```
251 ptr::write_volatile(&mut IRQ_10_PEND_ONCE
as *mut bool, true);

>>> x/1tw 0xE000E200
0xe000e200: 00000000000000000000010000000000

>>> monitor reg primask
primask (/1): 0x01
```

As soon as we exit the closure, PRIMASK would get disabled and we should
land at `generic_isr` breakpoint. We can then see that pending status
for IRQ 10 is disabled, by checking NVIC.ISPR[0] (0xE000E200) again.

```
65 pub unsafe extern "C" fn generic_isr() {
66 asm!(

>>> monitor reg primask
primask (/1): 0x00

>>> x/1tw 0xE000E200
0xe000e200: 00000000000000000000000000000000
```

Once we exit `generic_isr` and are back in the `kernel_loop`, we can
confirm once again that IRQ 10 is no longer pending.

```
259 let tmp = irq_10.get_pending();
260
261 asm!("nop" :::: "volatile");

>>> info locals
tmp = false
irq_10 = cortexmnvic::nvic::Nvic (
10
)

>>> x/1tw 0xE000E200
0xe000e200: 00000000000000000000000000000000
```

Continuing through the loop, in the next iteration we can see that our
version of `service_pending_interrupts` code-block would not get a
pending interrupt, therefore leading to an interrupt loss.

In the second example [5], instead of pending IRQ 10 once, we pend it
twice. By pending interrupt twice we can examine the behavior of
`service_pending_interrupts`.

In the first iteration of the loop IRQ 10 is set to pending for the
first time. Upon exiting the closure, as expected `generic_irq` gets
called. Upon entry into `generic_irq` IRQ 10 is no longer pending and
upon exit IRQ 10 is disabled.

In the second iteration of the loop, we pend IRQ 10 again. This time we
can observe that `generic_irq` does not get called upon exiting the
closure, as IRQ 10 is in a disabled state.

In the third iteration of the loop, in our `service_pending_interrupts`,
code-block, we will finally get a pending interrupt and our "actual" ISR
gets called (which happens to be a "nop").

Upon exiting `service_pending_interrupts` block, we clear pending
status, and enable IRQ again.

If IRQ 10 occurs at this stage, which can be simulated by doing
('monitor mww 0xE000E200 0x00000400'), we would enter `generic_isr` and
lose another interrupt.

Best,
Rajiv

[1] https://www.amazon.com/Definitive-Guide-Cortex%C2%AE-M3-Cortex%C2%AE-M4-Processors/dp/0124080820

[2] https://github.com/rajivr/tock/commit/a59547ac1a4c3c9fb0557d860017f62b0e064ae2

[3] https://github.com/rajivr/tock/commit/3efcdff35fe0165080d32a6fb2d44f606ccff8fb

[4] https://github.com/rajivr/tock/blob/nucleo_f446re-wip/kernel/src/sched.rs#L191-L264

[5] https://github.com/rajivr/tock/blob/nucleo_f446re-wip/kernel/src/sched-pend_twice.rs#L191-L285

[Louis Thiery]

Hi Rajiv,

This is a very interesting consideration, notably: "However, in a Cortex-M microcontroller, upon entry to ISR, the "pending" status of the interrupt is automatically cleared."

I think the reason why this "works" so far for Tock is that this generic_isr also does nothing to actually service the condition that triggered the interrupt and interrupts are serviced in kernel-context; in most cases, as you mention, more than one interrupt is "triggered", except this second time, the NVIC has been disabled, so the flag is set and not cleared since the interrupt vector doesn't get jumped to.

This stops to work as you noted, if the interrupt condition naturally resolves itself without requiring a firmware-based routine; a GPIO edge that disappears before the end of the generic_isr would be a good example. Since the edge is gone, the NVIC flag is never set, so nothing it communicated to the kernel context.

I was preparing to submit a pull request next week for a new way of handling interrupts, but since you mention this today, I will go ahead and open it now: https://github.com/tock/tock/pull/1181

My objectives were initially the following:

• allow for higher performance drivers to leverage interrupt context, ie: create a canonical way to service interrupts in interrupt context but then dispatch the event to capsules
• allow for kernel-space yielding to a centralized event dispatcher, eg: a long-running computation or busy/read write can be segmented into smaller chunks, allowing periodic yields
  
• allow for re-prioritization of the kernel-context interrupt handlers beyond their hardware-based NVIC number; currently peripheral interrupts are serviced from lowest IRQn to highest IRQn

However, your observation adds another feature which is, I believe, most critical: we won't be missing interrupts. 

Best,

Louis

--
You received this message because you are subscribed to the Google Groups "Tock Embedded OS Development Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to tock-dev+u...@googlegroups.com.
To post to this group, send email to tock...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/tock-dev/CACW1YzsUHhBD4GLXSXD4PFSLPZto1NFFQRozR0vhfkUgt5a61g%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

[Amit Levy]

Hi Rajiv,

If I'm understanding you correcting, you're basically saying the reason
Tock currently works on the platforms it's been ported to is because all
the peripherals (or at least the ones that have functioning drivers)
re-assert the pending bit until the peripheral believe the interrupt has
been handled. But that this strategy of relying on the pending bit after
the ISR has fired is not generalizable---there are peripherals that will
not reassert the pending bit even if the peripheral still has interrupts.

Is that a fair characterization? That's certainly possible.

If that's the case, the question is how best to deal with this.

-Amit

[Rajiv M Ranganath]

On Sun, Oct 7, 2018 at 1:44 AM Amit Levy <am...@amitlevy.com> wrote:
>
> Hi Rajiv,
>
> If I'm understanding you correcting, you're basically saying the reason
> Tock currently works on the platforms it's been ported to is because all
> the peripherals (or at least the ones that have functioning drivers)
> re-assert the pending bit until the peripheral believe the interrupt has
> been handled.

Hi Amit,

I suspect the reason why current Tock ISR mechanism works is because of
the emergent behavior arising form the interaction between
`generic_isr` and `service_pending_interrupts`.

In Cortex-M, a peripheral cannot assert or de-assert the pending bit.
This means there is no direct link between the status of pending bit and
what might be happening on the NVIC's IRQ line.

The "abstraction" that NVIC provides to a peripheral is -

1. Pull the relevant NVIC IRQ line high or low

2. Pulse the NVIC line

So, a peripheral IP designer/vendor can use either level triggered or
edge triggered logic on the NVIC line.

It is the NVIC that sets the pending bit and that's the _only_ thing it
can do. NVIC _cannot_ clear the pending bit once its set.

Clearing pending bit happens automatically when ARM enters into the
relevant exception handler (`generic_isr` in our case).

We can also set and clear the pending bit by writing to the `NVIC.{ISPR,
ICPR}` registers.

Now for the emergent behavior -

1. `generic_isr` disables the interrupt by setting NVIC.ICER. This seems
to be the key reason why current ISR mechanism works.

Let me explain further.

Had we not disabled the interrupt in `generic_isr`, then upon
`generic_isr` entry, the pending bit would have been cleared.

Upon exit from `generic_isr` into kernel mode, there would be no set
pending bit for `service_pending_interrupts` to do its match.

Following code would always be false.

```
} else if let Some(interrupt) = cortexm4::nvic::next_pending() {
```

If you can remove the line

```
str r0, [r3, r2, lsl #2]
```

from `generic_isr`, should not get to any ISRs from
service_pending_interrupts.

2. Now because `generic_isr` disables the interrupt, it leads to a NVIC
condition which is -

1. The interrupt is disabled

2. Pending bit is set

It is under this condition that `service_pending_interrupts` is able
to process interrupts.

> But that this strategy of relying on the pending bit after
> the ISR has fired is not generalizable---there are peripherals that will
> not reassert the pending bit even if the peripheral still has interrupts.
>
> Is that a fair characterization? That's certainly possible.

Not really. I feel we should separate the two concepts here and discuss
their behavior independently.

1. Peripheral using edge/level triggered logic on the NVIC line

2. NVIC setting the pending bit

> If that's the case, the question is how best to deal with this.

If the intention it to adopt Louis's new design, we can focus on
reviewing and understanding that design.

Otherwise we can work on the current design.

Let me know what is your preference.

Best,
Rajiv

[Brad Campbell]

Correct me if I am wrong, but I think the essential question here is:

"Can, in the current Tock setup, `generic_isr()` return with the corresponding NVIC pending bit not set?"

I agree that if the answer to that question is "yes" then there exists a case or cases where Tock would miss interrupts. Basically, the kernel would ask the chip to see if any interrupts are pending, there wouldn't be any NVIC pending bits set, and the actual interrupt handler for that NVIC interrupt would never get called.

After looking at pages 224-225 of http://infocenter.arm.com/help/topic/com.arm.doc.dui0553a/DUI0553A_cortex_m4_dgug.pdf I'm pretty convinced that there does exist a case where `generic_isr()` could return with the pending bit cleared:

1. A peripheral uses a short pulse interrupt for its connection to the NVIC.
2. On an interrupt, the NVIC detects the edge, sets the pending bit, and runs the ISR.
3. Upon ISR entry the CPU clears the pending status, and since the pulse is short it ends before the ISR ends. Therefore, the core does not mark it as pending again.
4. `generic_isr()` ends, and the pending bit is not set.
   

Now, it seems like we have not encountered this case in practice, as Tock's interrupt handling approach is working. However, maybe someday we would encounter this case (or there are additional cases I have missed). However, could we not just have `generic_isr()` manually set the pending bit via software to ensure it is set at the end of `generic_isr()`?

- Brad

[Rajiv M Ranganath]

On Sat, Dec 8, 2018 at 4:08 AM Brad Campbell <bra...@gmail.com> wrote:
>
> Correct me if I am wrong, but I think the essential question here is:
>
> "Can, in the current Tock setup, `generic_isr()` return with the
> corresponding NVIC pending bit not set?"

Thank you for the pointer to M4 Generic User Guide, section 4.2.9.

I think the essential question is how we can model and understand the
behavior of `generic_isr()`, in relation to

1. NVIC Pending State (Pending, Not Pending)

2. NVIC Enabled State (Enabled, Not Enabled)

3. NVIC Line State (Level High, Level Low, Pulse Rising Edge, Pulse Low)

Let me write some pseudo-code to represent these ideas.

```
// NVIC
enum Pending {
Pending,
NotPending,
}

enum Enabled {
Enabled,
Disabled
}

enum Line {
Level(Level),
Pulse(Pulse),
}

// Peripheral
enum Level {
High,
Low
}

enum Pulse {
RisingEdge,
Low
}

```

To model the behavior of `generic_isr()`, we can consider the tuple of
type `(Line, Enabled, Pending)`.

Lets assume a pulse-triggered peripheral. Initially we would be in the following
state.

(Line::Pulse(Low), Enabled::Enabled, Pending::NotPending)

One possible sequence of state changes that would lead to an interrupt loss is
as follows.

(Line::Pulse(Low), Enabled::Enabled, Pending::NotPending)
-> Peripheral Interrupt Occurs
-> (Line::Pulse(RisingEdge), Enabled::Enabled, Pending::NotPending)
-> NVIC registers the pulse and sets pending
-> (Line::Pulse(Low), Enabled::Enabled, Pending::Pending)
-> upon `generic_isr()` entry (pending is automatically cleared)
-> (Line::Pulse(Low), Enabled::Enabled, Pending::NotPending)
-> within `generic_isr()` we disable the interrupt
-> (Line::Pulse(Low), Enabled::Disabled, Pending::NotPending)
-> (Line::Pulse(Low), Enabled::Disabled, Pending::NotPending)

As the interrupt pending state is now NotPending, in
`service_pending_interrupts()`, `cortexm4::nvic::next_pending()` will no
longer return the interrupt leading to a lost interrupt.

To make things even more interesting, this state can automatically
correct itself. Let me explain with another sequence of state changes
starting with the last state.

(Line::Pulse(Low), Enabled::Disabled, Pending::NotPending)
-> Peripheral Interrupt Occurs
-> (Line::Pulse(RisingEdge), Enabled::Disabled, Pending::NotPending)
-> NVIC registers the pulse and sets pending
-> (Line::Pulse(Low), Enabled::Disabled, Pending::Pending)

In the above state, even though the interrupt is pending,
`generic_isr()` does not get called because the interrupt is disabled.

However, in `service_pending_interrupts()`,
`cortexm4::nvic::next_pending()` would return the interrupt and the
corresponding `handle_interrupt()` would get called.

Upon returning from `handle_interrupt()`, we would,

```
let n = cortexm4::nvic::Nvic::new(interrupt);
n.clear_pending();
n.enable();
```

Bringing the tuple state to,
(Line::Pulse(Low), Enabled::Enabled, Pending::NotPending)

This state is same as what we started with, so if another interrupt
occurs, we would enter into `generic_isr()`, and lose that interrupt.

Please let me know your feedback on this.

We still need to,

1. Model `generic_isr()` behavior for level triggered interrupts.

2. Once that's done, I'll explain the split driver design that I
proposing in #1220.

[...]

> Now, it seems like we have not encountered this case in practice, as
> Tock's interrupt handling approach is working. However, maybe someday
> we would encounter this case (or there are additional cases I have
> missed). However, could we not just have `generic_isr()` manually set
> the pending bit via software to ensure it is set at the end of
> `generic_isr()`?

Below is how we might model your suggestion for a pulse triggered
peripheral.

(Line::Pulse(Low), Enabled::Enabled, Pending::NotPending)
-> Peripheral Interrupt Occurs
-> (Line::Pulse(RisingEdge), Enabled::Enabled, Pending::NotPending)
-> NVIC registers the pulse and sets pending
-> (Line::Pulse(Low), Enabled::Enabled, Pending::Pending)
-> upon `generic_isr()` entry (pending is automatically cleared)
-> (Line::Pulse(Low), Enabled::Enabled, Pending::NotPending)
-> within `generic_isr()` we disable the interrupt
-> (Line::Pulse(Low), Enabled::Disabled, Pending::NotPending)
-> **new** within `generic_isr()` set pending again
-> (Line::Pulse(Low), Enabled::Disabled, Pending::Pending)
-> (Line::Pulse(Low), Enabled::Disabled, Pending::Pending)

My initial thought on this is that,
(Line::Pulse(Low), Enabled::Disabled, Pending::Pending)
would be okay for `service_pending_interrupt()`

However, if you look at section 3.12.12 (WFI instruction), it mentions
the following.

WFI is a hint instruction that suspends execution until one of the
following events occurs:
- a non-masked interrupt occurs and is taken
- an interrupt masked by PRIMASK becomes pending
- a Debug Entry request.

If I understand correctly, because we are masked at NVIC (not by
PRIMASK), I do not think the first two conditions hold when we are in
the state
(Line::Pulse(Low), Enabled::Disabled, Pending::Pending)

So, if we enter sleep in this state, then we'll have to wait for the
next Systick interrupt or another interrupt in a Enabled::Enabled state
to wake us up.

That delay could mean that the status registers of the peripherals could
have changed, and in the interrupt handler, we could be potentially
reading a different value.

Best,
Rajiv

[Brad Campbell]

I think, then, that we agree that `generic_isr()` can return without the corresponding pending bit set, and we will miss the interrupt until the interrupt occurs again. However, does manually setting the pending bit inside of `generic_isr()` address this case and ensure that the pending bit will be set when `generic_isr()` ends?

- Brad

--
You received this message because you are subscribed to the Google Groups "Tock Embedded OS Development Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to tock-dev+u...@googlegroups.com.
To post to this group, send email to tock...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/tock-dev/CACW1Yzt3xFe8fsthZrOrOaOvfLsP%3D8q3KmYtMCN_xBmQiB8Q6Q%40mail.gmail.com.

[Rajiv M Ranganath]

On Tue, Dec 11, 2018 at 1:37 AM Brad Campbell <bra...@gmail.com> wrote:
>
> I think, then, that we agree that `generic_isr()` can return without
> the corresponding pending bit set, and we will miss the interrupt
> until the interrupt occurs again. However, does manually setting the
> pending bit inside of `generic_isr()` address this case and ensure
> that the pending bit will be set when `generic_isr()` ends?

AFAICT, for pulse-triggered peripheral, setting the pending bit inside
of `generic_isr()` should ensure that the pending bit will be set when
`generic_isr()` ends.

*Because* we disable the interrupt at NVIC in `generic_isr()`, it will
not enter `generic_isr()` again, leaving pending bit set for
`service_pending_interrupts()` to be able to process it.

In my mind there are still some open questions.

1. Interrupt latency

In [1] Louis has mentioned the use-cases where he needs top-half
interrupt handlers.

In want to add another use-case. Most secure microcontrollers have
tamper detect mechanisms. When tamper detect interrupts are fired, we
need to be able to take counter measures in a top-half interrupt
handler.

I also agree with the Louis comment -

"..., there are countless situations where embedded systems need to
execute code in the interrupt routine. The quality and safety of
drivers will be enhanced if we provide a canonical way of doing so
rather than having every other project hack in some idiosyncratic
approach."

I feel we should have canonical way of doing split drivers in Tock.

2. Behavior of `wfi` when we are in the state -

(Line::Pulse(Low), Enabled::Disabled, Pending::Pending)

or
(Line::Level(High), Enabled::Disabled, Pending::Pending)

I took another look at the following code, and it seems to me that we
won't be entering sleep in pending state.

```
chip.atomic(|| {
if !chip.has_pending_interrupts() && self.processes_blocked() {
chip.sleep();
}
});
```

However, should this part of the code ever change, there is a
possibility that we might introduce additional latency.

Best,
Rajiv

[1] https://github.com/tock/tock/pull/1220#issuecomment-445467556

[Amit Levy]

tl;dr I think we have agreement that the current implementation of `generic_isr` could yield bugs for pulse-triggered peripherals (which we simply haven't encountered yet), and a simple change should fix it.

On 12/10/18 10:32 PM, Rajiv M Ranganath wrote:

On Tue, Dec 11, 2018 at 1:37 AM Brad Campbell <bra...@gmail.com> wrote:

I think, then, that we agree that `generic_isr()` can return without
the corresponding pending bit set, and we will miss the interrupt
until the interrupt occurs again. However, does manually setting the
pending bit inside of `generic_isr()` address this case and ensure
that the pending bit will be set when `generic_isr()` ends?

AFAICT, for pulse-triggered peripheral, setting the pending bit inside
of `generic_isr()` should ensure that the pending bit will be set when
`generic_isr()` ends.

*Because* we disable the interrupt at NVIC in `generic_isr()`, it will
not enter `generic_isr()` again, leaving pending bit set for
`service_pending_interrupts()` to be able to process it.

Great, so we all at least agree that pushing all/most interrupt handling to ISRs is unnecessary to get correct semantics.

It's not stated above, but since it came up in a related GitHub PR I'll restate that having logic in ISRs puts type-safety at risk since the compiler would be no-help in preventing data-races that can circumvent the type system (see section 2.1 of https://www.tockos.org/assets/papers/rust-kernel-apsys2017.pdf for an example explanation of why this is the case).

In other words, having programmable ISRs prevents the Tock architecture from ensuring type-safety, and thus isolation, within the kernel, for little benefit. So it should be avoided except in extenuating circumstances

In my mind there are still some open questions.

1. Interrupt latency

   In [1] Louis has mentioned the use-cases where he needs top-half
   interrupt handlers.

   In want to add another use-case. Most secure microcontrollers have
   tamper detect mechanisms. When tamper detect interrupts are fired, we
   need to be able to take counter measures in a top-half interrupt
   handler.

Yes, these seem similar to hard faults to me, which are also handled specially. Though I don't think the issue here is latency but rather a need to immediately stop or somehow recover the system in case of tamper.

   I also agree with the Louis comment -

   "..., there are countless situations where embedded systems need to
   execute code in the interrupt routine. The quality and safety of
   drivers will be enhanced if we provide a canonical way of doing so
   rather than having every other project hack in some idiosyncratic
   approach."

The proof will be in the pudding :)

   I feel we should have canonical way of doing split drivers in Tock.

I agree that if it proves useful it makes sense to have a convention for how to break the Tock model. Louis's experiments in CC26x2 are a great start towards that. When we have numbers and use cases that require programmable peripheral ISRs we'll have that as a basis to start from.

2. Behavior of `wfi` when we are in the state -

   (Line::Pulse(Low), Enabled::Disabled, Pending::Pending)
   or
   (Line::Level(High), Enabled::Disabled, Pending::Pending)

   I took another look at the following code, and it seems to me that we
   won't be entering sleep in pending state.

That's correct, and it's part of the design...

   ```
   chip.atomic(|| {
       if !chip.has_pending_interrupts() && self.processes_blocked() {
           chip.sleep();
       }
   });
   ```

   However, should this part of the code ever change, there is a
   possibility that we might introduce additional latency.

...so changing this code in a way that would put the chip to sleep would be a bug that would break most applications.

[Rajiv M Ranganath]

On Tue, Dec 11, 2018 at 9:51 AM Amit Levy <am...@amitlevy.com> wrote:

[...]

> Great, so we all at least agree that pushing all/most interrupt
> handling to ISRs is unnecessary to get correct semantics.
>
> It's not stated above, but since it came up in a related GitHub PR
> I'll restate that having logic in ISRs puts type-safety at risk since
> the compiler would be no-help in preventing data-races that can
> circumvent the type system (see section 2.1 of
> https://www.tockos.org/assets/papers/rust-kernel-apsys2017.pdf for an
> example explanation of why this is the case).
>
> In other words, having programmable ISRs prevents the Tock
> architecture from ensuring type-safety, and thus isolation, within the
> kernel, for little benefit. So it should be avoided except in
> extenuating circumstances

I re-read this paper. In section Section 3.2 there is a very brief
mention about interrupt handlers.

"Interrupt/exception handlers: Handlers are inherently unsafe because
they preempt running code so memory may be in an inconsistent state."

The paper then continues to talk about TakeCell. Neither in this paper,
nor anywhere else could I find documentation mentioning `generic_isr()`
was designed with type-safety in mind.

With type-safety under consideration, the design of `generic_isr()`
makes sense to me.

I have to now agree with your earlier comment that I was sent on a wild
goose chase... :-)

Best,
Rajiv