risc-v/mpfs: ihc: cleanup DEBUGASSERTs and reboot

Replace DEBUGASSERTs with sanity checks. DEBUGASSERT()s are
not necessarily enabled at all, thus risking the functionality
especially in that case.

Don't enable the ihc irq too early. If enabled, and the master
is already up, the irq is being issued so that the system gets
stuck or is severely slowed down. Master may be already if this
NuttX hart is rebooted, for example.

Signed-off-by: Eero Nurkkala <[email protected]>

arch/risc-v/src/mpfs/mpfs_ihc.c

@@ -55,7 +55,7 @@
  * Pre-processor Definitions
  ****************************************************************************/
 
-#ifdef CONFIG_MPFS_IHC_DEBUG
+#ifdef CONFIG_DEBUG_ERROR
 #  define ihcerr  _err
 #  define ihcwarn _warn
 #  define ihcinfo _info
@@ -337,9 +337,6 @@ static uint32_t mpfs_ihc_context_to_remote_hart_id(ihc_channel_t channel)
     }
   else
     {
-      DEBUGASSERT(LIBERO_SETTING_CONTEXT_A_HART_EN > 0);
-      DEBUGASSERT(LIBERO_SETTING_CONTEXT_B_HART_EN > 0);
-
       /* Determine context we are in */
 
       if (channel == IHC_CHANNEL_TO_CONTEXTA)
@@ -350,10 +347,6 @@ static uint32_t mpfs_ihc_context_to_remote_hart_id(ihc_channel_t channel)
         {
           harts_in_context = LIBERO_SETTING_CONTEXT_B_HART_EN;
         }
-      else
-        {
-          DEBUGPANIC();
-        }
 
       hart_idx = 0;
 
@@ -369,8 +362,6 @@ static uint32_t mpfs_ihc_context_to_remote_hart_id(ihc_channel_t channel)
         }
     }
 
-  DEBUGASSERT(hart != UNDEFINED_HART_ID);
-
   return hart;
 }
 
@@ -431,8 +422,6 @@ static uint32_t mpfs_ihc_context_to_local_hart_id(ihc_channel_t channel)
         }
     }
 
-  DEBUGASSERT(hart < MPFS_NUM_HARTS);
-
   return hart;
 }
 
@@ -505,7 +494,10 @@ static void mpfs_ihc_worker(void *arg)
     }
   while ((ctrl_reg & (RMP_MESSAGE_PRESENT)) && --retries);
 
-  DEBUGASSERT(retries != 0);
+  if (retries == 0)
+    {
+      ihcerr("Could not send the message!\n");
+    }
 
   modifyreg32(MPFS_IHC_CTRL(g_work_arg.mhartid,
               g_work_arg.rhartid), 0, ACK_INT);
@@ -536,19 +528,25 @@ static void mpfs_ihc_rx_message(ihc_channel_t channel, uint32_t mhartid,
   uint32_t rhartid  = mpfs_ihc_context_to_remote_hart_id(channel);
   uint32_t ctrl_reg = getreg32(MPFS_IHC_CTRL(mhartid, rhartid));
 
+  if (rhartid == UNDEFINED_HART_ID)
+    {
+      ihcerr("Remote hart not identified!\n");
+      return;
+    }
+
   /* Check if we have a message */
 
   if (mhartid == CONTEXTB_HARTID)
     {
       uintptr_t msg_in = MPFS_IHC_MSG_IN(mhartid, rhartid);
-      DEBUGASSERT(msg == NULL);
       mpfs_ihc_rx_handler((uint32_t *)msg_in);
     }
   else
     {
       /* This path is meant for the OpenSBI vendor extension only */
 
-      DEBUGPANIC();
+      ihcerr("Wrong recipient!\n");
+      return;
     }
 
   /* Set MP to 0. Note this generates an interrupt on the other hart
@@ -683,15 +681,19 @@ static int mpfs_ihc_interrupt(int irq, void *context, void *arg)
  *   hart_to_configure   - Hart to be configured
  *
  * Returned Value:
- *   None
+ *   OK if all is good, a negated error code otherwise
  *
  ****************************************************************************/
 
-static void mpfs_ihc_local_context_init(uint32_t hart_to_configure)
+static int mpfs_ihc_local_context_init(uint32_t hart_to_configure)
 {
   uint32_t rhartid = 0;
 
-  DEBUGASSERT(hart_to_configure < MPFS_NUM_HARTS);
+  if (hart_to_configure >= MPFS_NUM_HARTS)
+    {
+      ihcerr("Configuring too many harts!\n");
+      return -EINVAL;
+    }
 
   while (rhartid < MPFS_NUM_HARTS)
     {
@@ -701,6 +703,8 @@ static void mpfs_ihc_local_context_init(uint32_t hart_to_configure)
 
   g_connected_harts     = ihcia_remote_harts[hart_to_configure];
   g_connected_hart_ints = ihcia_remote_hart_ints[hart_to_configure];
+
+  return OK;
 }
 
 /****************************************************************************
@@ -758,7 +762,24 @@ static int mpfs_ihc_tx_message(ihc_channel_t channel, uint32_t *message)
   uint32_t ctrl_reg;
   uint32_t retries      = 10000;
 
-  DEBUGASSERT(message_size <= IHC_MAX_MESSAGE_SIZE);
+  if (mhartid >= MPFS_NUM_HARTS)
+    {
+      ihcerr("Problem finding proper mhartid\n");
+      return -EINVAL;
+    }
+
+  if (rhartid == UNDEFINED_HART_ID)
+    {
+      /* Something went wrong */
+
+      ihcerr("Remote hart not found!\n");
+      return -EINVAL;
+    }
+  else if (message_size > IHC_MAX_MESSAGE_SIZE)
+    {
+      ihcerr("Sent message too large!\n");
+      return -EINVAL;
+    }
 
   /* Check if the system is busy.  All we can try is wait. */
 
@@ -885,8 +906,6 @@ mpfs_rptun_get_resource(struct rptun_dev_s *dev)
 
   /* Only slave supported so far */
 
-  DEBUGASSERT(!priv->master);
-
   if (priv->shmem != NULL)
     {
       return &priv->shmem->rsc;
@@ -942,6 +961,12 @@ mpfs_rptun_get_resource(struct rptun_dev_s *dev)
 
   g_rptun_initialized = true;
 
+  /* Don't enable this too early; if the master is already up, irqs will
+   * likely hang the system as no ACKs may be sent yet.
+   */
+
+  up_enable_irq(g_plic_irq);
+
   return &priv->shmem->rsc;
 }
 
@@ -1063,7 +1088,10 @@ static int mpfs_rptun_notify(struct rptun_dev_s *dev, uint32_t notifyid)
         }
       while ((ret != OK) && --retries);
 
-      DEBUGASSERT(ret == OK);
+      if (retries == 0)
+        {
+          return -EIO;
+        }
     }
 
   return ret;
@@ -1256,7 +1284,11 @@ static void mpfs_rptun_worker(void *arg)
       return;
     }
 
-  DEBUGASSERT((g_vq_idx - VRING0_NOTIFYID) < VRINGS);
+  if (g_vq_idx >= VRINGS)
+    {
+      return;
+    }
+
   info = &g_mpfs_virtqueue_table[g_vq_idx - VRING0_NOTIFYID];
   virtqueue_notification((struct virtqueue *)info->data);
 }
@@ -1293,7 +1325,11 @@ static int mpfs_rptun_thread(int argc, char *argv[])
           return 0;
         }
 
-      DEBUGASSERT((g_vq_idx - VRING0_NOTIFYID) < VRINGS);
+      if (g_vq_idx >= VRINGS)
+        {
+          return -EINVAL;
+        }
+
       info = &g_mpfs_virtqueue_table[g_vq_idx - VRING0_NOTIFYID];
       virtqueue_notification((struct virtqueue *)info->data);
 
@@ -1357,17 +1393,18 @@ int mpfs_ihc_init(void)
 
   /* Initialize IHC FPGA module registers to a known state */
 
-  mpfs_ihc_local_context_init(mhartid);
+  ret = mpfs_ihc_local_context_init(mhartid);
+  if (ret != OK)
+    {
+      return ret;
+    }
+
   mpfs_ihc_local_remote_config(mhartid, rhartid);
 
   /* Attach and enable the applicable irq */
 
   ret = irq_attach(g_plic_irq, mpfs_ihc_interrupt, NULL);
-  if (ret == OK)
-    {
-      up_enable_irq(g_plic_irq);
-    }
-  else
+  if (ret != OK)
     {
       ihcerr("ERROR: Not able to attach irq\n");
       return ret;

arch/risc-v/src/mpfs/mpfs_ihc_sbi.c

@@ -218,8 +218,6 @@ static uint32_t mpfs_ihc_sbi_context_to_remote_hart_id(ihc_channel_t channel)
       hart_idx++;
     }
 
-  DEBUGASSERT(hart != UNDEFINED_HART_ID);
-
   return hart;
 }
 
@@ -244,8 +242,6 @@ static uint32_t mpfs_ihc_sbi_context_to_local_hart_id(ihc_channel_t channel)
   uint32_t harts_in_context = LIBERO_SETTING_CONTEXT_A_HART_EN;
   uint32_t hart_next        = 0;
 
-  DEBUGASSERT(channel > IHC_CHANNEL_TO_HART4);
-
   hart_idx = 0;
   while (hart_idx < MPFS_NUM_HARTS)
     {
@@ -271,7 +267,6 @@ static uint32_t mpfs_ihc_sbi_context_to_local_hart_id(ihc_channel_t channel)
       hart_idx++;
     }
 
-  DEBUGASSERT(hart < MPFS_NUM_HARTS);
   return hart;
 }
 
@@ -303,8 +298,7 @@ static void mpfs_ihc_sbi_message_present_handler(uint32_t *message,
                                                  bool is_mp)
 {
   struct ihc_sbi_rx_msg_s *msg;
-  uintptr_t message_ihc     = (uintptr_t)MPFS_IHC_MSG_IN(mhartid, rhartid);
-  uint32_t message_size_ihc = getreg32(MPFS_IHC_MSG_SIZE(mhartid, rhartid));
+  uintptr_t message_ihc = (uintptr_t)MPFS_IHC_MSG_IN(mhartid, rhartid);
 
   msg = (struct ihc_sbi_rx_msg_s *)message;
 
@@ -324,8 +318,6 @@ static void mpfs_ihc_sbi_message_present_handler(uint32_t *message,
       msg->irq_type = ACK_IRQ | MP_IRQ;
       msg->ihc_msg = *(struct mpfs_ihc_msg_s *)message_ihc;
     }
-
-    DEBUGASSERT(sizeof(msg->ihc_msg) >= message_size_ihc);
 }
 
 /****************************************************************************
@@ -350,17 +342,23 @@ static void mpfs_ihc_sbi_message_present_handler(uint32_t *message,
 static void mpfs_ihc_sbi_rx_message(uint32_t rhartid, uint32_t mhartid,
                                     bool is_ack, bool is_mp, uint32_t *msg)
 {
+  /* Msg must be always present */
+
+  if (msg == NULL)
+    {
+      return;
+    }
+
   if (is_ack && !is_mp)
     {
       if (mhartid == CONTEXTB_HARTID)
         {
-          DEBUGPANIC();
+          return;
         }
       else
         {
           /* This path is meant for the OpenSBI vendor extension only */
 
-          DEBUGASSERT(msg != NULL);
           mpfs_ihc_sbi_message_present_handler(msg, mhartid, rhartid,
                                                is_ack, is_mp);
 
@@ -375,13 +373,12 @@ static void mpfs_ihc_sbi_rx_message(uint32_t rhartid, uint32_t mhartid,
 
       if (mhartid == CONTEXTB_HARTID)
         {
-          DEBUGPANIC();
+          return;
         }
       else
         {
           /* This path is meant for the OpenSBI vendor extension only */
 
-          DEBUGASSERT(msg != NULL);
           mpfs_ihc_sbi_message_present_handler(msg, mhartid, rhartid,
                                                is_ack, is_mp);
         }
@@ -394,7 +391,6 @@ static void mpfs_ihc_sbi_rx_message(uint32_t rhartid, uint32_t mhartid,
     }
   else if (is_ack && is_mp)
     {
-      DEBUGASSERT(msg != NULL);
       mpfs_ihc_sbi_message_present_handler(msg, mhartid, rhartid,
                                            is_ack, is_mp);
 
@@ -431,7 +427,8 @@ void mpfs_ihc_sbi_message_present_indirect_isr(ihc_channel_t channel,
   uint32_t origin_hart = mpfs_ihc_sbi_parse_incoming_hartid(mhartid,
                                                             &is_ack,
                                                             &is_mp);
-  if (origin_hart != UNDEFINED_HART_ID)
+
+  if ((origin_hart != UNDEFINED_HART_ID) && (mhartid < MPFS_NUM_HARTS))
     {
       /* Process incoming packet */
 
@@ -460,8 +457,6 @@ static void mpfs_ihc_sbi_local_context_init(uint32_t hart_to_configure)
 {
   uint32_t rhartid = 0;
 
-  DEBUGASSERT(hart_to_configure < MPFS_NUM_HARTS);
-
   while (rhartid < MPFS_NUM_HARTS)
     {
       putreg32(0, MPFS_IHC_CTRL(hart_to_configure, rhartid));
@@ -551,7 +546,18 @@ static int mpfs_ihc_sbi_tx_message(ihc_channel_t channel, uint32_t *message)
   uint32_t ctrl_reg;
   uint32_t retries      = 10000;
 
-  DEBUGASSERT(message_size <= IHC_MAX_MESSAGE_SIZE);
+  if (message_size > IHC_MAX_MESSAGE_SIZE)
+    {
+      return -EINVAL;
+    }
+  else if (rhartid == UNDEFINED_HART_ID)
+    {
+      return -EINVAL;
+    }
+  else if (mhartid >= MPFS_NUM_HARTS)
+    {
+      return -EINVAL;
+    }
 
   /* Check if the system is busy.  All we can try is wait. */
 
@@ -643,7 +649,19 @@ int mpfs_ihc_sbi_ecall_handler(unsigned long funcid, uint32_t remote_channel,
         /* mhartid = Linux hart id, rhartid = NuttX hart id */
 
         mhartid = mpfs_ihc_sbi_context_to_local_hart_id(remote_channel);
+
+        if (mhartid >= MPFS_NUM_HARTS)
+          {
+            return -EINVAL;
+          }
+
         rhartid = mpfs_ihc_sbi_context_to_remote_hart_id(remote_channel);
+
+        if (rhartid == UNDEFINED_HART_ID)
+          {
+            return -EINVAL;
+          }
+
         if (remote_channel == IHC_CHANNEL_TO_CONTEXTB)
           {
             mpfs_ihc_sbi_local_context_init(mhartid);