From da44fe3fc91ad643ec3892cc01a68eb52ed3dc5e Mon Sep 17 00:00:00 2001 From: Jerzy Kasenberg Date: Fri, 14 Jan 2022 09:44:38 +0100 Subject: [PATCH] nrf5x: Fix EP OUT race conditions When dcd_edpt_xfer() starts new transfer two separate problems were observed. For both problems stream of OUT packets was pouring from host. First problem was that total_len and actual_len were not atomic. In case where incoming OUT packets are less (63) than MPS (64), actual_len and total_len are set 63. Then transfer complete from USBD is called that will schedule next 64 bytes transfer. At that point incoming packet would start DMA if there is place in RAM, normally it does not happen since actual_len == total_len. If packets arrives and interrupt is raised after total_len is set (64) but actual_len is still 63 from previous transfer, interrupt code sees that there is place in ram (1 byte) and transfer this 1 byte to buffer that was already filled with previous packet. To remedy this USB interrupt is blocked during transfer setup. Second problem can happen when dcd_edpt_xfer setups xfer->total_len and actual_len correctly but then context switch happens before xfer->data_received is checked. If during this time two packets arrive one will be copied to RAM second will stay in endpoint with data_received set to 1. Then when xfer_edpt_xfer() checks data_receive flag it starts DMA again overwriting data. To remedy this, data_received is checked together with check if data was already transferred. If transfer was complete, there is no need to start DMA yet. In such case data_received will be handled in same place by next xfer_edpt_xfer() correctly. --- src/portable/nordic/nrf5x/dcd_nrf5x.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/portable/nordic/nrf5x/dcd_nrf5x.c b/src/portable/nordic/nrf5x/dcd_nrf5x.c index ed597a34..db3b06ee 100644 --- a/src/portable/nordic/nrf5x/dcd_nrf5x.c +++ b/src/portable/nordic/nrf5x/dcd_nrf5x.c @@ -453,9 +453,11 @@ 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); xfer->buffer = buffer; xfer->total_len = total_bytes; xfer->actual_len = 0; + dcd_int_enable(rhport); // 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)); @@ -476,7 +478,7 @@ bool dcd_edpt_xfer (uint8_t rhport, uint8_t ep_addr, uint8_t * buffer, uint16_t edpt_dma_start(&NRF_USBD->TASKS_EP0RCVOUT); }else { - if ( xfer->data_received ) + if ( xfer->data_received && xfer->total_len > xfer->actual_len) { // Data is already received previously // start DMA to copy to SRAM