tests: kernel: interrupt: Rework nested interrupt test

The current nested interrupt test implementation is both buggy and
fundamentally flawed because it does not trigger a higher priority
interrupt from a lower priority interrupt context and relies on the
system timer interrupt, which is not fully governed by the test;
moreover, the current implementation does not properly validate the
test results and can report success if no interrupt is triggered and
serviced at all.

This commit reworks this test to have the following well-defined
and logical procedure:

1. [thread] Trigger IRQ 0 (lower priority)
2. [isr0] Set ISR 0 result token and trigger IRQ 1 (higher priority)
3. [isr1] Set ISR 1 result token and return
4. [isr0] Validate ISR 1 result token and return
5. [thread] Validate ISR 0 result token

The reworked test scenario ensures that the interrupt nesting works
properly and any abnormal conditions are detected (e.g. interrupts not
triggering at all, or ISR 1 not being nested under ISR 0).

Signed-off-by: Stephanos Ioannidis <[email protected]>

Author: stephanosio

tests/kernel/interrupt/src/interrupt_util.h

@@ -33,9 +33,12 @@ static inline u32_t get_available_nvic_line(u32_t initial_offset)
 			NVIC_SetPendingIRQ(i);
 
 			if (NVIC_GetPendingIRQ(i)) {
-				/* If the NVIC line is pending, it is
-				 * guaranteed that it is implemented.
+				/*
+				 * If the NVIC line is pending, it is
+				 * guaranteed that it is implemented; clear the
+				 * line and return the NVIC line number.
 				 */
+				NVIC_ClearPendingIRQ(i);
 				break;
 			}
 		}

tests/kernel/interrupt/src/nested_irq.c

@@ -1,4 +1,5 @@
 /*
+ * Copyright (c) 2020 Stephanos Ioannidis <[email protected]>
  * Copyright (c) 2018 Intel Corporation
  *
  * SPDX-License-Identifier: Apache-2.0
@@ -7,120 +8,122 @@
 #include <ztest.h>
 #include "interrupt_util.h"
 
-#define DURATION 5
-struct k_timer timer;
+#define DURATION	5
 
-/* This tests uses two IRQ lines, selected within the range of IRQ lines
- * available on the target SOC the test executes on (and starting from
- * the maximum available IRQ line index)
- */
-#define IRQ_LINE(offset) (CONFIG_NUM_IRQS - ((offset) + 1))
-
-#define ISR0_OFFSET 1
-#define ISR1_OFFSET 2
+#define ISR0_TOKEN	0xDEADBEEF
+#define ISR1_TOKEN	0xCAFEBABE
 
-/* Keeping isr0 to be lowest priority than system timer
- * so that it can be interrupted by timer triggered.
- * In NRF5, RTC system timer is of priority 1 and
- * in all other architectures, system timer is considered
- * to be in priority 0.
+/*
+ * This test uses two IRQ lines selected within the range of available IRQs on
+ * the target SoC.  These IRQs are platform and interrupt controller-specific,
+ * and must be specified for every supported platform.
+ *
+ * In terms of priority, the IRQ1 is triggered from the ISR of the IRQ0;
+ * therefore, the priority of IRQ1 must be greater than that of the IRQ0.
  */
 #if defined(CONFIG_CPU_CORTEX_M)
-u32_t irq_line_0;
-u32_t irq_line_1;
-#define ISR0_PRIO 2
-#define ISR1_PRIO 1
+/*
+ * For Cortex-M NVIC, unused and available IRQs are automatically detected when
+ * when the test is run.
+ */
+#define IRQ0_PRIO	2
+#define IRQ1_PRIO	1
 #else
-#define ISR0_PRIO 1
-#define ISR1_PRIO 0
-#endif /* CONFIG_CPU_CORTEX_M */
+/*
+ * For all the other platforms, use the last two available IRQ lines for
+ * testing.
+ */
+#define IRQ0_LINE	(CONFIG_NUM_IRQS - 1)
+#define IRQ1_LINE	(CONFIG_NUM_IRQS - 2)
+
+#define IRQ0_PRIO	1
+#define IRQ1_PRIO	0
+#endif
+
+#ifndef NO_TRIGGER_FROM_SW
+static u32_t irq_line_0;
+static u32_t irq_line_1;
 
-volatile u32_t new_val;
-u32_t old_val = 0xDEAD;
+static u32_t isr0_result;
+static u32_t isr1_result;
 
 void isr1(void *param)
 {
 	ARG_UNUSED(param);
-	new_val = 0xDEAD;
-	printk("%s ran !!\n", __func__);
-}
 
-/**
- *
- * triggering interrupt from the timer expiry function while isr0
- * is in busy wait
- */
-#ifndef NO_TRIGGER_FROM_SW
-static void handler(struct k_timer *timer)
-{
-	ARG_UNUSED(timer);
-#if defined(CONFIG_CPU_CORTEX_M)
-	irq_enable(irq_line_1);
-	trigger_irq(irq_line_1);
-#else
-	irq_enable(IRQ_LINE(ISR1_OFFSET));
-	trigger_irq(IRQ_LINE(ISR1_OFFSET));
-#endif /* CONFIG_CPU_CORTEX_M */
-}
-#else
-void handler(void)
-{
-	ztest_test_skip();
+	printk("isr1: Enter\n");
+
+	/* Set verification token */
+	isr1_result = ISR1_TOKEN;
+
+	printk("isr1: Leave\n");
 }
-#endif /* NO_TRIGGER_FROM_SW */
 
 void isr0(void *param)
 {
 	ARG_UNUSED(param);
-	printk("%s running !!\n", __func__);
-#if defined(CONFIG_BOARD_QEMU_CORTEX_M0)
-	/* QEMU Cortex-M0 timer emulation appears to not capturing the
-	 * current time accurately, resulting in erroneous busy wait
-	 * implementation.
-	 *
-	 * Work-around:
-	 * Increase busy-loop duration to ensure the timer interrupt will fire
-	 * during the busy loop waiting.
-	 */
-	k_busy_wait(MS_TO_US(1000));
-#else
-	k_busy_wait(MS_TO_US(10));
-#endif
-	printk("%s execution completed !!\n", __func__);
-	zassert_equal(new_val, old_val, "Nested interrupt is not working\n");
+
+	printk("isr0: Enter\n");
+
+	/* Set verification token */
+	isr0_result = ISR0_TOKEN;
+
+	/* Trigger nested IRQ 1 */
+	trigger_irq(irq_line_1);
+
+	/* Wait for interrupt */
+	k_busy_wait(MS_TO_US(DURATION));
+
+	/* Validate nested ISR result token */
+	zassert_equal(isr1_result, ISR1_TOKEN, "isr1 did not execute");
+
+	printk("isr0: Leave\n");
 }
 
 /**
+ * @brief Test interrupt nesting
+ *
+ * @ingroup kernel_interrupt_tests
  *
- * Interrupt nesting feature allows an ISR to be preempted in mid-execution
- * if a higher priority interrupt is signaled. The lower priority ISR resumes
- * execution once the higher priority ISR has completed its processing.
- * The expected control flow should be isr0 -> handler -> isr1 -> isr0
+ * This routine tests the interrupt nesting feature, which allows an ISR to be
+ * preempted in mid-execution if a higher priority interrupt is signaled. The
+ * lower priority ISR resumes execution once the higher priority ISR has
+ * completed its processing.
+ *
+ * The expected control flow for this test is as follows:
+ *
+ * 1. [thread] Trigger IRQ 0 (lower priority)
+ * 2. [isr0] Set ISR 0 result token and trigger IRQ 1 (higher priority)
+ * 3. [isr1] Set ISR 1 result token and return
+ * 4. [isr0] Validate ISR 1 result token and return
+ * 5. [thread] Validate ISR 0 result token
  */
-#ifndef NO_TRIGGER_FROM_SW
 void test_nested_isr(void)
 {
+	/* Resolve test IRQ line numbers */
 #if defined(CONFIG_CPU_CORTEX_M)
 	irq_line_0 = get_available_nvic_line(CONFIG_NUM_IRQS);
 	irq_line_1 = get_available_nvic_line(irq_line_0);
-	arch_irq_connect_dynamic(irq_line_0, ISR0_PRIO, isr0, NULL, 0);
-	arch_irq_connect_dynamic(irq_line_1, ISR1_PRIO, isr1, NULL, 0);
 #else
-	IRQ_CONNECT(IRQ_LINE(ISR0_OFFSET), ISR0_PRIO, isr0, NULL, 0);
-	IRQ_CONNECT(IRQ_LINE(ISR1_OFFSET), ISR1_PRIO, isr1, NULL, 0);
-#endif /* CONFIG_CPU_CORTEX_M */
+	irq_line_0 = IRQ0_LINE;
+	irq_line_1 = IRQ1_LINE;
+#endif
 
-	k_timer_init(&timer, handler, NULL);
-	k_timer_start(&timer, DURATION, K_NO_WAIT);
+	/* Connect and enable test IRQs */
+	arch_irq_connect_dynamic(irq_line_0, IRQ0_PRIO, isr0, NULL, 0);
+	arch_irq_connect_dynamic(irq_line_1, IRQ1_PRIO, isr1, NULL, 0);
 
-#if defined(CONFIG_CPU_CORTEX_M)
 	irq_enable(irq_line_0);
+	irq_enable(irq_line_1);
+
+	/* Trigger test IRQ 0 */
 	trigger_irq(irq_line_0);
-#else
-	irq_enable(IRQ_LINE(ISR0_OFFSET));
-	trigger_irq(IRQ_LINE(ISR0_OFFSET));
-#endif /* CONFIG_CPU_CORTEX_M */
 
+	/* Wait for interrupt */
+	k_busy_wait(MS_TO_US(DURATION));
+
+	/* Validate ISR result token */
+	zassert_equal(isr0_result, ISR0_TOKEN, "isr0 did not execute");
 }
 #else
 void test_nested_isr(void)