From 980ffe3b4e41ce5e43c3e8ae13f605f7d7b50b7f Mon Sep 17 00:00:00 2001 From: Jerzy Kasenberg Date: Fri, 14 Jan 2022 09:14:49 +0100 Subject: [PATCH 1/2] nrf5x: Fix DMA access race condition In multi-thread mode starting DMA in thread mode was prone to race condition resulting in infinite loop. It may happen on single core CPU with strict priority based tasks scheduler where ready high prio task never yields to ready low prio task (Mynewt). Sequence that failed (T1 - low priority task, T2 - high priority task) - T1 called start_dma() - T1 set _dcd.dma_running (DMA not started yet, context switch happens) - T2 took CPU and saw that _dcd.dma_running is set, so waits for _dcd.dma_running to be 0 - T1 never gets CPU again, DMA is not started T2 waits forever OSAL mutex resolves problem of DMA starting from thread-context. --- src/portable/nordic/nrf5x/dcd_nrf5x.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/portable/nordic/nrf5x/dcd_nrf5x.c b/src/portable/nordic/nrf5x/dcd_nrf5x.c index 6b2cd83fd..e70565a02 100644 --- a/src/portable/nordic/nrf5x/dcd_nrf5x.c +++ b/src/portable/nordic/nrf5x/dcd_nrf5x.c @@ -79,6 +79,9 @@ typedef struct } xfer_td_t; +static osal_mutex_def_t dcd_mutex_def; +static osal_mutex_t dcd_mutex; + // Data for managing dcd static struct { @@ -154,6 +157,7 @@ static void edpt_dma_start(volatile uint32_t* reg_startep) // Should be safe to blocking wait until previous DMA transfer complete uint8_t const rhport = 0; bool started = false; + osal_mutex_lock(dcd_mutex, OSAL_TIMEOUT_WAIT_FOREVER); while(!started) { // LDREX/STREX may be needed in form of std atomic (required C11) or @@ -170,6 +174,7 @@ static void edpt_dma_start(volatile uint32_t* reg_startep) // osal_yield(); } + osal_mutex_unlock(dcd_mutex); } } @@ -243,6 +248,7 @@ static void xact_in_dma(uint8_t epnum) void dcd_init (uint8_t rhport) { TU_LOG1("dcd init\r\n"); + dcd_mutex = osal_mutex_create(&dcd_mutex_def); (void) rhport; } From 36b6ed8ff9035ad0c2e75de92e08799dc3a4b7ed Mon Sep 17 00:00:00 2001 From: Jerzy Kasenberg Date: Fri, 14 Jan 2022 09:44:38 +0100 Subject: [PATCH 2/2] nrf5x: Fix EP OUT race conditions in OS build When two tasks entered dcd_edpt_xfer() it was possible that first disabled interrupt to setup total_len and actual_len but second task for another endpoint enabled interrupt between total_len and actual_len resulting in race condition with interrupt, hence mutex is added on top of interrupt being blocked. --- src/portable/nordic/nrf5x/dcd_nrf5x.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/portable/nordic/nrf5x/dcd_nrf5x.c b/src/portable/nordic/nrf5x/dcd_nrf5x.c index e70565a02..024ea0d90 100644 --- a/src/portable/nordic/nrf5x/dcd_nrf5x.c +++ b/src/portable/nordic/nrf5x/dcd_nrf5x.c @@ -463,11 +463,17 @@ bool dcd_edpt_xfer (uint8_t rhport, uint8_t ep_addr, uint8_t * buffer, uint16_t xfer_td_t* xfer = get_td(epnum, dir); - dcd_int_disable(rhport); + if (!is_in_isr()) { + osal_mutex_lock(dcd_mutex, OSAL_TIMEOUT_WAIT_FOREVER); + dcd_int_disable(rhport); + } xfer->buffer = buffer; xfer->total_len = total_bytes; xfer->actual_len = 0; - dcd_int_enable(rhport); + if (!is_in_isr()) { + dcd_int_enable(rhport); + osal_mutex_unlock(dcd_mutex); + } // Control endpoint with zero-length packet and opposite direction to 1st request byte --> status stage bool const control_status = (epnum == 0 && total_bytes == 0 && dir != tu_edpt_dir(NRF_USBD->BMREQUESTTYPE));