Make vIRQ functions claim IRQs on first call

Author: niklarm

core/system/inc/unvic.h

@@ -52,7 +52,7 @@ extern uint32_t unvic_irq_priority_get(uint32_t irqn);
 extern int      unvic_irq_level_get(void);
 extern int      unvic_default(uint32_t isr_id);
 
-extern void unvic_init(void);
+extern void unvic_init(uint32_t const * const user_vtor);
 
 /** Perform a context switch-in as a result of an interrupt request.
  *

core/system/src/main.c

@@ -21,7 +21,7 @@
 #include "unvic.h"
 #include "vmpu.h"
 
-UVISOR_NOINLINE void uvisor_init_pre(void)
+UVISOR_NOINLINE void uvisor_init_pre(uint32_t const * const user_vtor)
 {
     /* Reset the uVisor own BSS section. */
     memset(&__bss_start__, 0, VMPU_REGION_SIZE(&__bss_start__, &__bss_end__));
@@ -30,7 +30,7 @@ UVISOR_NOINLINE void uvisor_init_pre(void)
     memcpy(&__data_start__, &__data_start_src__, VMPU_REGION_SIZE(&__data_start__, &__data_end__));
 
     /* Initialize the unprivileged NVIC module. */
-    unvic_init();
+    unvic_init(user_vtor);
 
     /* Initialize the debugging features. */
     DEBUG_INIT();
@@ -135,7 +135,7 @@ void main_entry(void)
     }
 
     /* Early uVisor initialization. */
-    uvisor_init_pre();
+    uvisor_init_pre((uint32_t *) SCB->VTOR);
 
     /* Run basic sanity checks. */
     if (vmpu_init_pre() == 0) {

core/system/src/unvic.c

@@ -32,7 +32,7 @@ uint8_t g_nvic_prio_bits;
  * @internal
  *
  * If multiple disable-/re-enable-all functions are called in the same box, a
- * counter is respectively incremented/drecremented so that it never happens
+ * counter is respectively incremented/decremented so that it never happens
  * that a nested function re-enables IRQs for the caller. */
 uint32_t g_irq_disable_all_counter[UVISOR_MAX_BOXES];
 
@@ -68,7 +68,7 @@ void unvic_acl_add(uint8_t box_id, void *function, uint32_t irqn)
     uv = &g_unvic_vector[irqn];
 
     /* check if IRQ entry is populated */
-    if(uv->id || uv->hdlr)
+    if(uv->id != UVISOR_BOX_ID_INVALID)
     {
         HALT_ERROR(PERMISSION_DENIED,
                    "Permission denied: IRQ %d is owned by box %d\n\r", irqn,
@@ -80,9 +80,15 @@ void unvic_acl_add(uint8_t box_id, void *function, uint32_t irqn)
     uv->hdlr = function;
 }
 
+#define UNVIC_ISR_OWNER_OTHER 0
+#define UNVIC_ISR_OWNER_NONE  1
+#define UNVIC_ISR_OWNER_SELF  2
+
+/* @retval 0 you do not own this interrupt
+ * @retval 1 you own or you can claim this interrupt
+ */
 static int unvic_acl_check(int irqn)
 {
-    int is_irqn_registered;
     TIsrUVector *uv;
 
     /* don't allow to modify uVisor-owned IRQs */
@@ -91,113 +97,83 @@ static int unvic_acl_check(int irqn)
     /* get vector entry */
     uv = &g_unvic_vector[irqn];
 
-    /* an IRQn slot is considered as registered if the ISR entry in the unprivileged
-     * table is not 0 */
-    is_irqn_registered = uv->hdlr ? 1 : 0;
+    if (uv->id == g_active_box || uv->id == UVISOR_BOX_ID_INVALID) {
+        return 1;
+    }
+    DPRINTF("Disallowed to access IRQ %d from box %d!\n\r", irqn, g_active_box);
+    return 0;
+}
 
-    /* check if the same box that registered the IRQn is accessing it
-     * note: if the vector entry for a certain IRQn is 0, it means that no secure
-     *       box claimed exclusive ownership for it. So, another box can claim it
-     *       if it is currently un-registered (that is, if the registered handler
-     *       is NULL) */
-    if(uv->id != g_active_box && (uv->id || is_irqn_registered))
+/* @retval 0 you do not own this interrupt
+ * @retval 1 you own this interrupt
+ */
+static int unvic_isr_register(uint32_t irqn)
+{
+    int isr_status = unvic_acl_check(irqn);
+    switch(isr_status)
     {
-        HALT_ERROR(PERMISSION_DENIED,
-                   "Permission denied: IRQ %d is owned by box %d\n\r", irqn,
-                                                                       uv->id);
+        case UNVIC_ISR_OWNER_NONE:
+            g_unvic_vector[irqn].id = g_active_box;
+            DPRINTF("IRQ %d registered to box %d\n\r", irqn, g_active_box);
+        case UNVIC_ISR_OWNER_SELF:
+            return 1;
+        default:
+            break;
     }
-
-    return is_irqn_registered;
+    return 0;
 }
 
 void unvic_isr_set(uint32_t irqn, uint32_t vector)
 {
     /* verify IRQ access privileges */
-    unvic_acl_check(irqn);
-
-    /* save unprivileged handler
-     * note: if the handler is NULL the IRQn gets de-registered for the current
-     *       box, meaning that other boxes can still use it. In this way boxes
-     *       can share un-exclusive IRQs. A secure box should not do that if it
-     *       wants to hold exclusivity over an IRQn
-     * note: when an IRQ is released (de-registered) the corresponding IRQn is
-     *       disabled, to ensure that no spurious interrupts are served */
-    if (vector) {
-        g_unvic_vector[irqn].id = g_active_box;
-    }
-    else {
-        NVIC_DisableIRQ(irqn);
-        g_unvic_vector[irqn].id = 0;
+    if (!unvic_isr_register(irqn)) {
+        return;
     }
-    g_unvic_vector[irqn].hdlr = (TIsrVector) vector;
-
-    /* set default priority (SVC must always be higher) */
-    NVIC_SetPriority(irqn, __UVISOR_NVIC_MIN_PRIORITY);
 
-    DPRINTF("IRQ %d %s box %d\n\r",
-            irqn,
-            vector ? "registered to" : "released by",
-            g_active_box);
+    /* save unprivileged handler */
+    g_unvic_vector[irqn].hdlr = (TIsrVector) vector;
 }
 
 uint32_t unvic_isr_get(uint32_t irqn)
 {
     /* verify IRQ access privileges */
-    unvic_acl_check(irqn);
+    if (!unvic_isr_register(irqn)) {
+        return 0;
+    }
 
     return (uint32_t) g_unvic_vector[irqn].hdlr;
 }
 
 void unvic_irq_enable(uint32_t irqn)
 {
-    int is_irqn_registered;
-
     /* verify IRQ access privileges */
-    is_irqn_registered = unvic_acl_check(irqn);
-
-    /* Enable the IRQ, but only if IRQs are not globally disabled for the
-     * currently active box. */
-    if (is_irqn_registered) {
-        /* If the counter of nested disable-all IRQs is set to 0, it means that
-         * IRQs are not globally disabled for the current box. */
-        if (!g_irq_disable_all_counter[g_active_box]) {
-            DPRINTF("IRQ %d enabled\n\r", irqn);
-            NVIC_EnableIRQ(irqn);
-        } else {
-            /* We do not enable the IRQ directly, but notify uVisor to enable it
-             * when IRQs will be re-enabled globally for the current box. */
-            g_unvic_vector[irqn].was_enabled = true;
-        }
-        return;
-    }
-    else if (UNVIC_IS_IRQ_ENABLED(irqn)) {
-        DPRINTF("IRQ %d is unregistered; state unchanged\n\r", irqn);
+    if (!unvic_isr_register(irqn)) {
         return;
     }
-    else {
-        HALT_ERROR(NOT_ALLOWED, "IRQ %d is unregistered; state cannot be changed", irqn);
+
+    /* If the counter of nested disable-all IRQs is set to 0, it means that
+     * IRQs are not globally disabled for the current box. */
+    if (!g_irq_disable_all_counter[g_active_box]) {
+        DPRINTF("IRQ %d enabled\n\r", irqn);
+        NVIC_EnableIRQ(irqn);
+    } else {
+        /* We do not enable the IRQ directly, but notify uVisor to enable it
+         * when IRQs will be re-enabled globally for the current box. */
+        g_unvic_vector[irqn].was_enabled = true;
     }
+    return;
 }
 
 void unvic_irq_disable(uint32_t irqn)
 {
-    int is_irqn_registered;
-
     /* verify IRQ access privileges */
-    is_irqn_registered = unvic_acl_check(irqn);
-
-    if (is_irqn_registered) {
-        DPRINTF("IRQ %d disabled, but still owned by box %d\n\r", irqn, g_unvic_vector[irqn].id);
-        NVIC_DisableIRQ(irqn);
-        return;
-    }
-    else if (!UNVIC_IS_IRQ_ENABLED(irqn)) {
-        DPRINTF("IRQ %d is unregistered; state unchanged\n\r", irqn);
+    if (!unvic_isr_register(irqn)) {
         return;
     }
-    else {
-        HALT_ERROR(NOT_ALLOWED, "IRQ %d is unregistered; state cannot be changed", irqn);
-    }
+
+    DPRINTF("IRQ %d disabled, but still owned by box %d\n\r", irqn, g_unvic_vector[irqn].id);
+    NVIC_DisableIRQ(irqn);
+    return;
 }
 
 /** Disable all interrupts for the currently active box.
@@ -296,77 +272,63 @@ void unvic_irq_enable_all(void)
 
 void unvic_irq_pending_clr(uint32_t irqn)
 {
-    int is_irqn_registered;
-
     /* verify IRQ access privileges */
-    is_irqn_registered = unvic_acl_check(irqn);
-
-    /* enable IRQ */
-    if (is_irqn_registered) {
-        DPRINTF("IRQ %d pending status cleared\n\r", irqn);
-        NVIC_ClearPendingIRQ(irqn);
+    if (!unvic_isr_register(irqn)) {
         return;
     }
-    else {
-        HALT_ERROR(NOT_ALLOWED, "IRQ %d is unregistered; state cannot be changed", irqn);
-    }
+
+    /* clear pending IRQ */
+    DPRINTF("IRQ %d pending status cleared\n\r", irqn);
+    NVIC_ClearPendingIRQ(irqn);
 }
 
 void unvic_irq_pending_set(uint32_t irqn)
 {
-    int is_irqn_registered;
-
     /* verify IRQ access privileges */
-    is_irqn_registered = unvic_acl_check(irqn);
-
-    /* enable IRQ */
-    if (is_irqn_registered) {
-        DPRINTF("IRQ %d pending status set (will be served as soon as possible)\n\r", irqn);
-        NVIC_SetPendingIRQ(irqn);
+    if (!unvic_isr_register(irqn)) {
         return;
     }
-    else {
-        HALT_ERROR(NOT_ALLOWED, "IRQ %d is unregistered; state cannot be changed", irqn);
-    }
+
+    /* set pending IRQ */
+    DPRINTF("IRQ %d pending status set (will be served as soon as possible)\n\r", irqn);
+    NVIC_SetPendingIRQ(irqn);
 }
 
 uint32_t unvic_irq_pending_get(uint32_t irqn)
 {
     /* verify IRQ access privileges */
-    unvic_acl_check(irqn);
+    if (!unvic_isr_register(irqn)) {
+        return 0;
+    }
 
     /* get priority for device specific interrupts  */
     return NVIC_GetPendingIRQ(irqn);
 }
 
 void unvic_irq_priority_set(uint32_t irqn, uint32_t priority)
 {
-    int is_irqn_registered;
-
     /* verify IRQ access privileges */
-    is_irqn_registered = unvic_acl_check(irqn);
+    if (!unvic_isr_register(irqn)) {
+        return;
+    }
 
     /* check for maximum priority */
     if (priority > UVISOR_VIRQ_MAX_PRIORITY) {
         HALT_ERROR(NOT_ALLOWED, "NVIC priority overflow; max priority allowed: %d\n\r", UVISOR_VIRQ_MAX_PRIORITY);
     }
 
     /* set priority for device specific interrupts */
-    if (is_irqn_registered) {
-        DPRINTF("IRQ %d priority set to %d (NVIC), %d (virtual)\n\r", irqn, __UVISOR_NVIC_MIN_PRIORITY + priority,
-                                                                            priority);
-        NVIC_SetPriority(irqn, __UVISOR_NVIC_MIN_PRIORITY + priority);
-        return;
-    }
-    else {
-        HALT_ERROR(NOT_ALLOWED, "IRQ %d is unregistered; state cannot be changed", irqn);
-    }
+    DPRINTF("IRQ %d priority set to %d (NVIC), %d (virtual)\n\r", irqn, __UVISOR_NVIC_MIN_PRIORITY + priority,
+                                                                        priority);
+    NVIC_SetPriority(irqn, __UVISOR_NVIC_MIN_PRIORITY + priority);
 }
 
 uint32_t unvic_irq_priority_get(uint32_t irqn)
 {
     /* verify IRQ access privileges */
-    unvic_acl_check(irqn);
+    if (!unvic_isr_register(irqn)) {
+        return 0;
+    }
 
     /* get priority for device specific interrupts  */
     return NVIC_GetPriority(irqn) - __UVISOR_NVIC_MIN_PRIORITY;
@@ -560,7 +522,7 @@ void unvic_gateway_context_switch_out(uint32_t svc_sp, uint32_t msp)
     __set_CONTROL(__get_CONTROL() & ~2);
 }
 
-void unvic_init(void)
+void unvic_init(uint32_t const * const user_vtor)
 {
     uint8_t prio_bits;
     uint8_t volatile *prio;
@@ -604,4 +566,12 @@ void unvic_init(void)
      * by IRQ number. For example, IRQ 0 has precedence over IRQ 1 if both have
      * the same priority level. */
     NVIC_SetPriorityGrouping(0);
+
+    /* Copy the user interrupt table into the handlers. */
+    for (uint32_t ii=0; ii < NVIC_VECTORS; ii++) {
+        g_unvic_vector[ii].id = UVISOR_BOX_ID_INVALID;
+        g_unvic_vector[ii].hdlr = (TIsrVector) user_vtor[ii + NVIC_OFFSET];
+        /* set default priority (SVC must always be higher) */
+        NVIC_SetPriority(ii, __UVISOR_NVIC_MIN_PRIORITY);
+    }
 }