diff --git a/arch/risc-v/src/mpfs/mpfs_ihc.c b/arch/risc-v/src/mpfs/mpfs_ihc.c index a8c878d622c..e1823b2ab53 100644 --- a/arch/risc-v/src/mpfs/mpfs_ihc.c +++ b/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; } @@ -454,10 +443,20 @@ static uint32_t mpfs_ihc_context_to_local_hart_id(ihc_channel_t channel) static void mpfs_ihc_rx_handler(uint32_t *message) { - g_vq_idx = message[0]; + uint32_t msg = message[0]; - DEBUGASSERT((g_vq_idx == VRING0_NOTIFYID) || - (g_vq_idx == VRING1_NOTIFYID)); + /* After a warm reboot, the message may be initially corrupt as the renote + * doesn't know we restarted and reinitialized the registers. + */ + + if ((msg == VRING0_NOTIFYID) || (msg == VRING1_NOTIFYID)) + { + g_vq_idx = msg; + } + else + { + return; + } #ifdef MPFS_RPTUN_USE_THREAD nxsem_post(&g_mpfs_rx_sig); @@ -495,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); @@ -526,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 @@ -673,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) { @@ -691,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; } /**************************************************************************** @@ -744,7 +758,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. */ @@ -871,8 +902,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; @@ -901,9 +930,12 @@ mpfs_rptun_get_resource(struct rptun_dev_s *dev) rsc->rpmsg_vdev.gfeatures = 1 << VIRTIO_RPMSG_F_NS | 1 << VIRTIO_RPMSG_F_ACK; - /* Set to VIRTIO_CONFIG_STATUS_DRIVER_OK when master is up */ + /* If the master is up already, don't clear the status here */ - rsc->rpmsg_vdev.status = 0; + if (!g_shmem.master_up) + { + rsc->rpmsg_vdev.status = 0; + } rsc->rpmsg_vdev.config_len = sizeof(struct fw_rsc_config); rsc->rpmsg_vdev.num_of_vrings = VRINGS; @@ -925,6 +957,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; } @@ -1046,7 +1084,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; @@ -1232,8 +1273,19 @@ static void mpfs_rptun_worker(void *arg) { struct mpfs_queue_table_s *info; - DEBUGASSERT((g_vq_idx - VRING0_NOTIFYID) < VRINGS); - info = &g_mpfs_virtqueue_table[g_vq_idx - VRING0_NOTIFYID]; + /* Check whether the struct is initialized yet */ + + if (*(uintptr_t *)&g_mpfs_virtqueue_table[0] == 0) + { + return; + } + + if (g_vq_idx >= VRINGS) + { + return; + } + + info = &g_mpfs_virtqueue_table[g_vq_idx]; virtqueue_notification((struct virtqueue *)info->data); } #endif @@ -1262,8 +1314,19 @@ static int mpfs_rptun_thread(int argc, char *argv[]) while (1) { - DEBUGASSERT((g_vq_idx - VRING0_NOTIFYID) < VRINGS); - info = &g_mpfs_virtqueue_table[g_vq_idx - VRING0_NOTIFYID]; + /* Check whether the struct is initialized yet */ + + if (*(uintptr_t *)&g_mpfs_virtqueue_table[0] == 0) + { + return 0; + } + + if (g_vq_idx >= VRINGS) + { + return -EINVAL; + } + + info = &g_mpfs_virtqueue_table[g_vq_idx]; virtqueue_notification((struct virtqueue *)info->data); nxsem_wait(&g_mpfs_rx_sig); @@ -1326,17 +1389,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; @@ -1345,6 +1409,18 @@ int mpfs_ihc_init(void) /* Initialize and wait for the master. This will block until. */ ihcinfo("Waiting for the master online...\n"); + + /* Check if the remote is already up. This is the case after reboot of + * this particular hart only. + */ + + if ((getreg32(MPFS_IHC_CTRL(CONTEXTA_HARTID, CONTEXTB_HARTID)) & (MPIE_EN + | ACKIE_EN)) != 0) + { + g_shmem.master_up = true; + g_shmem.rsc.rpmsg_vdev.status |= VIRTIO_CONFIG_STATUS_DRIVER_OK; + } + ret = mpfs_rptun_init(MPFS_RPTUN_SHMEM_NAME, MPFS_RPTUN_CPU_NAME); if (ret < 0) { diff --git a/arch/risc-v/src/mpfs/mpfs_ihc_sbi.c b/arch/risc-v/src/mpfs/mpfs_ihc_sbi.c index a52f548e328..4031a488420 100644 --- a/arch/risc-v/src/mpfs/mpfs_ihc_sbi.c +++ b/arch/risc-v/src/mpfs/mpfs_ihc_sbi.c @@ -73,8 +73,7 @@ static uint32_t g_connected_harts_c; * Description: * This is a copy of modifyreg32() without spinlock. That function is a * real danger here as it is likely located in eNVM, thus being a real - * bottleneck. All functions called from this file should be located in - * the zero device. + * bottleneck. * * Input Parameters: * addr - Address to perform the operation @@ -219,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; } @@ -245,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) { @@ -272,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; } @@ -304,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; @@ -325,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); } /**************************************************************************** @@ -351,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); @@ -376,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); } @@ -395,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); @@ -432,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 */ @@ -461,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)); @@ -552,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. */ @@ -644,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);