stm32/pendsv: Remove preemptive keyboard interrupt via PendSV.

Since the very beginning, the stm32 port (first called stm, then stmhal now
stm32) has had a special keyboard interrupt feature which works by using
PendSV to break out of any running code.  This preemptive ctrl-C was added
long ago in commit 01156d5.

The stm32 port still uses that code, and current does this:

- If ctrl-C is received on UART or USB then `mp_sched_keyboard_interrupt()`
  is called (like all other ports) to set a flag for the VM to see, and
  then the VM (or any loop calling `mp_handle_pending(true)`) will
  eventually handle the `KeyboardInterrupt` exception, raising it via NLR.

- If another ctrl-C is received while the existing scheduled keyboard
  interrupt is still pending (ie the VM has not yet processed it) then a
  special hard NLR jump will activate, that preempts the calling code.
  Within the PendSV interrupt the stack is adjusted and an NLR jump is made
  to the most recent `nlr_push()` location.  This is like a normal NLR
  except it is called from an interrupt context and completely annihilates
  the code that was interrupted by the IRQ.

The reason for the preemptive interrupt was to handle ctrl-C before the VM
was able to handle it.  Eventually a mechanism (that's in use today by all
ports) was added to the VM and runtime to be able to check for pending
interrupts.  Then the stm32 port was updated to use this mechanism, with a
fallback to the old preemptive way if a second ctrl-C was received (without
the first one being processed).

This preemptive NLR jump is problematic because it can interrupt
long-running instructions (eg store multiple, usually used at the end of a
function to restore registers and return).  If such an instruction is
interrupted the CPU remembers that with some flags, and can resume the
long-running instruction when the interrupt finishes.  But the preemptive
NLR does a long jump to different code at thread level and so the
long-running interrupt is never resumed.  This leads to a CPU fault.

This fault has been previously reported in issues micropython#3807 and micropython#3842 (see also
issue micropython#294).  It's now possible to easily reproduce this problem, since
commit 69c25ea.  Running the test suite
over and over again on any stm32 board will eventually crash the board (it
can happen on a PYBv1.x, but it happens more regularly on PYBD-SF2/6).

The point is, a skipped test now soft resets the board and so the board
must run `boot.py` again.  The test runner may then interrupt the execution
of `boot.py` with the double-ctrl-C that it sends (in `tools/pyboard.py`,
`enter_raw_repl()`) in order to get the board into a known good state for
the next test.  If the timing is right, this can trigger the preemptive
PendSV in an unfortunate location and hard fault the board.

The fix in this commit is to just remove the preemptive NLR jump feature.
No other port has this feature and it's not needed, ctrl-C works very well
on those ports.  Preemptive NLR jump is a very dangerous thing (eg it may
interrupt and break out of an external SPI flash operation when reading
code from a filesystem) and is obviously buggy.

With this commit, stm32 borads no longer hard fault when running the test
suite (but it does leave an issue, the tests can still interrupt `boot.py`
with a single ctrl-C; that will be fixed separately).

An alternative to this commit would be to clear the CPU state for the
long-running instruction as suggested in issue micropython#3842.  But it's much
simpler to just remove this code, which is now unnecessary and can have
other problems as per issue micropython#294.

Signed-off-by: Damien George <damien@micropython.org>

Author: dpgeorge

ports/stm32/boards/common_isr_ram/common_isr.ld

@@ -14,7 +14,7 @@
        flash (in main.c) along with the isr_vector above.
     */
     . = ALIGN(4);
-    *(.text.pendsv_kbd_intr)
+    *(.text.mp_sched_keyboard_interrupt)
     *(.text.pendsv_schedule_dispatch)
     *(.text.storage_systick_callback)
     *(.text.SysTick_Handler)

ports/stm32/pendsv.c

@@ -31,13 +31,6 @@
 #include "pendsv.h"
 #include "irq.h"
 
-// This variable is used to save the exception object between a ctrl-C and the
-// PENDSV call that actually raises the exception.  It must be non-static
-// otherwise gcc-5 optimises it away.  It can point to the heap but is not
-// traced by GC.  This is okay because we only ever set it to
-// mp_kbd_exception which is in the root-pointer set.
-void *pendsv_object;
-
 #if defined(PENDSV_DISPATCH_NUM_SLOTS)
 uint32_t pendsv_dispatch_active;
 pendsv_dispatch_t pendsv_dispatch_table[PENDSV_DISPATCH_NUM_SLOTS];
@@ -51,24 +44,6 @@ void pendsv_init(void) {
     NVIC_SetPriority(PendSV_IRQn, IRQ_PRI_PENDSV);
 }
 
-// Call this function to raise a pending exception during an interrupt.
-// It will first try to raise the exception "softly" by setting the
-// mp_pending_exception variable and hoping that the VM will notice it.
-// If this function is called a second time (ie with the mp_pending_exception
-// variable already set) then it will force the exception by using the hardware
-// PENDSV feature.  This will wait until all interrupts are finished then raise
-// the given exception object using nlr_jump in the context of the top-level
-// thread.
-void pendsv_kbd_intr(void) {
-    if (MP_STATE_MAIN_THREAD(mp_pending_exception) == MP_OBJ_NULL) {
-        mp_sched_keyboard_interrupt();
-    } else {
-        MP_STATE_MAIN_THREAD(mp_pending_exception) = MP_OBJ_NULL;
-        pendsv_object = &MP_STATE_VM(mp_kbd_exception);
-        SCB->ICSR = SCB_ICSR_PENDSVSET_Msk;
-    }
-}
-
 #if defined(PENDSV_DISPATCH_NUM_SLOTS)
 void pendsv_schedule_dispatch(size_t slot, pendsv_dispatch_t f) {
     pendsv_dispatch_table[slot] = f;
@@ -90,10 +65,7 @@ void pendsv_dispatch_handler(void) {
 __attribute__((naked)) void PendSV_Handler(void) {
     // Handle a PendSV interrupt
     //
-    // For the case of an asynchronous exception, re-jig the
-    // stack so that when we return from this interrupt handler
-    // it returns instead to nlr_jump with argument pendsv_object
-    // note that stack has a different layout if DEBUG is enabled
+    // Calls any pending functions in pendsv_dispatch_table.
     //
     // For the case of a thread switch, swap stacks.
     //
@@ -132,27 +104,6 @@ __attribute__((naked)) void PendSV_Handler(void) {
         ".no_dispatch:\n"
         #endif
 
-        // Check if there is an active object to throw via nlr_jump
-        "ldr r1, pendsv_object_ptr\n"
-        "ldr r0, [r1]\n"
-        "cmp r0, #0\n"
-        "beq .no_obj\n"
-        #if defined(PENDSV_DEBUG)
-        "str r0, [sp, #8]\n"            // store to r0 on stack
-        #else
-        "str r0, [sp, #0]\n"            // store to r0 on stack
-        #endif
-        "mov r0, #0\n"
-        "str r0, [r1]\n"                // clear pendsv_object
-        "ldr r0, nlr_jump_ptr\n"
-        #if defined(PENDSV_DEBUG)
-        "str r0, [sp, #32]\n"           // store to pc on stack
-        #else
-        "str r0, [sp, #24]\n"           // store to pc on stack
-        #endif
-        "bx lr\n"                       // return from interrupt; will return to nlr_jump
-        ".no_obj:\n"                    // pendsv_object==NULL
-
         #if MICROPY_PY_THREAD
         // Do a thread context switch
         "push {r4-r11, lr}\n"
@@ -178,7 +129,5 @@ __attribute__((naked)) void PendSV_Handler(void) {
         #if defined(PENDSV_DISPATCH_NUM_SLOTS)
         "pendsv_dispatch_active_ptr: .word pendsv_dispatch_active\n"
         #endif
-        "pendsv_object_ptr: .word pendsv_object\n"
-        "nlr_jump_ptr: .word nlr_jump\n"
         );
 }

ports/stm32/pendsv.h

@@ -51,7 +51,6 @@ enum {
 typedef void (*pendsv_dispatch_t)(void);
 
 void pendsv_init(void);
-void pendsv_kbd_intr(void);
 void pendsv_schedule_dispatch(size_t slot, pendsv_dispatch_t f);
 
 #endif // MICROPY_INCLUDED_STM32_PENDSV_H

ports/stm32/uart.c

@@ -1229,7 +1229,7 @@ void uart_irq_handler(mp_uint_t uart_id) {
                 data &= self->char_mask;
                 if (self->attached_to_repl && data == mp_interrupt_char) {
                     // Handle interrupt coming in on a UART REPL
-                    pendsv_kbd_intr();
+                    mp_sched_keyboard_interrupt();
                 } else {
                     if (self->char_width == CHAR_WIDTH_9BIT) {
                         ((uint16_t *)self->read_buf)[self->read_buf_head] = data;

ports/stm32/usbd_cdc_interface.c

@@ -315,7 +315,7 @@ int8_t usbd_cdc_receive(usbd_cdc_state_t *cdc_in, size_t len) {
     // copy the incoming data into the circular buffer
     for (const uint8_t *src = cdc->rx_packet_buf, *top = cdc->rx_packet_buf + len; src < top; ++src) {
         if (cdc->attached_to_repl && *src == mp_interrupt_char) {
-            pendsv_kbd_intr();
+            mp_sched_keyboard_interrupt();
         } else {
             uint16_t next_put = (cdc->rx_buf_put + 1) & (MICROPY_HW_USB_CDC_RX_DATA_SIZE - 1);
             if (next_put == cdc->rx_buf_get) {