From 1533a42bd7cb85ab01da4d093e083a55299666bc Mon Sep 17 00:00:00 2001 From: Cristian Manca Date: Mon, 8 Jun 2026 10:39:44 +0200 Subject: [PATCH] qcwdfserial. Add clearing ring buffer at D3Final MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit QCRD.c — D0Exit race: Clear ring buffer and completion list inside the read thread's D0_EXIT_EVENT handler before signaling D0ExitReadyEvent, removing the race where stale URB completions wrote old AT responses into the ring buffer after D0Exit, causing garbage messages on re-enumeration. Reset bBufferOverflow/bDeviceGone/devErrCnt on D0Exit and FileClose for clean pipeline state. QCRD.c — surprise removal URB retry storm: Add bDeviceGone flag (set on STATUS_NO_SUCH_DEVICE completion) and guard free-list re-dispatch in REQUEST_ARRIVE_EVENT to skip when the device is gone. Prevents the ~40000/sec URB retry loop during surprise removal. QCRD.c — fixes: - EvtIoReadCompletionAsync: use IoStatus.Information for availableLength instead of PipeRead.Length (requested vs actual). QCPNP.c — D3Final safety net: Call QCUTIL_RingBufferClear in EvtDeviceD0Exit for D3Final as redundant guard after the read-thread fix above. Signed-off-by: Cristian Manca --- src/windows/wdfserial/QCPNP.c | 20 +++++++++ src/windows/wdfserial/QCRD.c | 79 ++++++++++++++++++++++++++++++++++- 2 files changed, 97 insertions(+), 2 deletions(-) diff --git a/src/windows/wdfserial/QCPNP.c b/src/windows/wdfserial/QCPNP.c index 907c6ae..da4aaec 100644 --- a/src/windows/wdfserial/QCPNP.c +++ b/src/windows/wdfserial/QCPNP.c @@ -2280,6 +2280,26 @@ NTSTATUS QCPNP_EvtDeviceD0Exit ); KeClearEvent(&pDevContext->ReadThreadD0ExitReadyEvent); } + + if (TargetState == WdfPowerDeviceD3Final) + { + QCSER_DbgPrint + ( + QCSER_DBG_MASK_POWER, + QCSER_DBG_LEVEL_TRACE, + ("<%ws> QCPNP_EvtDeviceD0Exit D3Final: clearing ring buffer\n", pDevContext->PortName) + ); + QCUTIL_RingBufferClear(&pDevContext->ReadRingBuffer); + pDevContext->AmountInInQueue = 0; + QCSER_DbgPrint + ( + QCSER_DBG_MASK_POWER, + QCSER_DBG_LEVEL_TRACE, + ("<%ws> QCPNP_EvtDeviceD0Exit D3Final: ring buffer cleared, bytes used: %llu\n", + pDevContext->PortName, QCUTIL_RingBufferBytesUsed(&pDevContext->ReadRingBuffer)) + ); + } + if (pDevContext->interruptThread != NULL) { KeClearEvent(&pDevContext->IntThreadD0EntryEvent); diff --git a/src/windows/wdfserial/QCRD.c b/src/windows/wdfserial/QCRD.c index 9a33902..e4b7135 100644 --- a/src/windows/wdfserial/QCRD.c +++ b/src/windows/wdfserial/QCRD.c @@ -138,6 +138,7 @@ void QCRD_ReadRequestHandlerThread BOOLEAN bDeviceOpened = FALSE; BOOLEAN bDeviceAwaken = FALSE; BOOLEAN bBufferOverflow = FALSE; + BOOLEAN bDeviceGone = FALSE; PLIST_ENTRY head; PLIST_ENTRY peek; PREAD_BUFFER_PARAM pBufferParam; // for read urbs @@ -221,6 +222,8 @@ void QCRD_ReadRequestHandlerThread WdfIoQueuePurgeSynchronously(pDevContext->TimeoutReadQueue); WdfIoTargetStop(ioTarget, WdfIoTargetCancelSentIo); QCRD_ClearBuffer(pDevContext); + bDeviceGone = FALSE; + devErrCnt = 0; KeSetEvent(&pDevContext->ReadThreadFileCloseReadyEvent, IO_NO_INCREMENT, FALSE); break; } @@ -468,6 +471,19 @@ void QCRD_ReadRequestHandlerThread WdfDeviceSetFailed(pDevContext->Device, WdfDeviceFailedNoRestart); break; } + /* + * STATUS_NO_SUCH_DEVICE means the device is gone before + * WdfIoTargetStop had a chance to cancel in-flight URBs + * (typical in UDE / surprise removal). Mark device gone so + * REQUEST_ARRIVE_EVENT does not re-dispatch URBs from the + * free list — that would create a ~40K/sec retry storm + * until D0Exit finally fires. The pipeline is restarted + * cleanly in D0_ENTRY_EVENT after re-enumeration. + */ + if (WdfRequestGetStatus(request) == STATUS_NO_SUCH_DEVICE) + { + bDeviceGone = TRUE; + } } else { @@ -535,7 +551,7 @@ void QCRD_ReadRequestHandlerThread QCSER_DBG_MASK_READ, QCSER_DBG_LEVEL_TRACE, ("<%ws> RIRP: QCRD_ReadRequestHandlerThread copied %llu bytes into ring buffer, bytes used: %llu, bytes available: %llu\n", - pDevContext->PortName, pBufferParam->Capacity, QCUTIL_RingBufferBytesUsed(rxBuffer), QCUTIL_RingBufferBytesFree(rxBuffer)) + pDevContext->PortName, pBufferParam->AvailableBytes, QCUTIL_RingBufferBytesUsed(rxBuffer), QCUTIL_RingBufferBytesFree(rxBuffer)) ); } pDevContext->AmountInInQueue = QCUTIL_RingBufferBytesUsed(rxBuffer); @@ -730,6 +746,36 @@ void QCRD_ReadRequestHandlerThread } else { + // Guard: check if TimeoutReadQueue has pending request and ReadQueue is empty + // If so, skip immediate drain to preserve inter-byte gap semantics (QUD-1837) + NTSTATUS timeoutCheckStatus = WdfIoQueueRetrieveNextRequest(pDevContext->TimeoutReadQueue, &pendingTimeoutRequest); + if (pendingTimeoutRequest != NULL && QCUTIL_RingBufferBytesUsed(rxBuffer) > 0) + { + // TimeoutReadQueue has pending request with data in buffer + // Check if ReadQueue also has a request + WDFREQUEST readQueueRequest = NULL; + NTSTATUS readQueueStatus = WdfIoQueueRetrieveNextRequest(pDevContext->ReadQueue, &readQueueRequest); + + if (readQueueStatus != STATUS_SUCCESS || readQueueRequest == NULL) + { + // ReadQueue is empty, don't bypass - wait for timeout timer + QCSER_DbgPrint + ( + QCSER_DBG_MASK_READ, + QCSER_DBG_LEVEL_TRACE, + ("<%ws> RIRP: QCRD_ReadRequestHandlerThread skip immediate drain - TimeoutReadQueue pending, ReadQueue empty, waiting for timer\n", pDevContext->PortName) + ); + WdfRequestForwardToIoQueue(pendingTimeoutRequest, pDevContext->TimeoutReadQueue); + pendingTimeoutRequest = NULL; + break; // Wait for timeout to fire + } + else + { + // ReadQueue has a fresh request, put it back and proceed with drain + WdfRequestForwardToIoQueue(readQueueRequest, pDevContext->ReadQueue); + } + } + // iterate through the ring buffer while (QCUTIL_RingBufferBytesUsed(rxBuffer) > 0) { @@ -851,6 +897,25 @@ void QCRD_ReadRequestHandlerThread ("<%ws> RIRP: QCRD_ReadRequestHandlerThread start to process free list\n", pDevContext->PortName) ); + /* + * Do not re-dispatch URBs when the device is gone + * (STATUS_NO_SUCH_DEVICE seen during surprise removal). + * Re-dispatching would immediately re-fail each URB and + * create a ~40 000/sec retry storm until D0Exit fires. + * Leave URBs in the free list; D0_ENTRY_EVENT will + * restart the pipeline cleanly after re-enumeration. + */ + if (bDeviceGone) + { + QCSER_DbgPrint + ( + QCSER_DBG_MASK_READ, + QCSER_DBG_LEVEL_DETAIL, + ("<%ws> RIRP: QCRD_ReadRequestHandlerThread skip free list dispatch, device gone\n", pDevContext->PortName) + ); + break; + } + head = &pDevContext->UrbReadFreeList; while (!IsListEmpty(head)) { @@ -950,6 +1015,16 @@ void QCRD_ReadRequestHandlerThread KeClearEvent(&pDevContext->ReadThreadD0ExitEvent); bDeviceAwaken = FALSE; WdfIoTargetStop(ioTarget, WdfIoTargetCancelSentIo); + /* + * Discard stale URB completions that arrived before D0Exit + * to prevent old AT responses from leaking into the ring buffer after + * re-enumeration and appearing as garbage messages. + */ + QCRD_ClearBuffer(pDevContext); + KeClearEvent(&pDevContext->ReadRequestCompletionEvent); + bBufferOverflow = FALSE; + bDeviceGone = FALSE; + devErrCnt = 0; KeSetEvent(&pDevContext->ReadThreadD0ExitReadyEvent, IO_NO_INCREMENT, FALSE); break; } @@ -1723,7 +1798,7 @@ VOID QCRD_EvtIoReadCompletionAsync PDEVICE_CONTEXT pDevContext = (PDEVICE_CONTEXT)Context; PREQUEST_CONTEXT pReqContext = QCReqGetContext(Request); NTSTATUS status = WdfRequestGetStatus(Request); - size_t availableLength = Params->Parameters.Usb.Completion->Parameters.PipeRead.Length; + size_t availableLength = (NT_SUCCESS(status) ? Params->IoStatus.Information : 0); if (NT_SUCCESS(status)) {