From nobody Sat Dec 28 09:52:23 2024 Delivered-To: importer@patchew.org Received-SPF: none (zoho.com: 198.145.21.10 is neither permitted nor denied by domain of lists.01.org) client-ip=198.145.21.10; envelope-from=edk2-devel-bounces@lists.01.org; helo=ml01.01.org; Authentication-Results: mx.zoho.com; spf=none (zoho.com: 198.145.21.10 is neither permitted nor denied by domain of lists.01.org) smtp.mailfrom=edk2-devel-bounces@lists.01.org; Return-Path: Received: from ml01.01.org (ml01.01.org [198.145.21.10]) by mx.zohomail.com with SMTPS id 1498646419477614.4034800535809; Wed, 28 Jun 2017 03:40:19 -0700 (PDT) Received: from [127.0.0.1] (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id 8448521BBC412; Wed, 28 Jun 2017 03:38:41 -0700 (PDT) Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 0B73221CB57D9 for ; Wed, 28 Jun 2017 03:38:40 -0700 (PDT) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 28 Jun 2017 03:40:11 -0700 Received: from ray-dev.ccr.corp.intel.com ([10.239.9.28]) by fmsmga004.fm.intel.com with ESMTP; 28 Jun 2017 03:40:10 -0700 X-Original-To: edk2-devel@lists.01.org X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.40,275,1496127600"; d="scan'208";a="279555774" From: Ruiyu Ni To: edk2-devel@lists.01.org Date: Wed, 28 Jun 2017 18:39:59 +0800 Message-Id: <20170628103959.397244-4-ruiyu.ni@intel.com> X-Mailer: git-send-email 2.12.2.windows.2 In-Reply-To: <20170628103959.397244-1-ruiyu.ni@intel.com> References: <20170628103959.397244-1-ruiyu.ni@intel.com> Subject: [edk2] [PATCH 3/3] MdeModulePkg/XhciDxe: Check timeout URB again after stopping endpoint X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Hao A Wu , Feng Tian , Star Zeng MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Errors-To: edk2-devel-bounces@lists.01.org Sender: "edk2-devel" X-ZohoMail: RSF_4 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" This fixes BULK data loss when transfer is detected as timeout but finished just before stopping endpoint. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ruiyu Ni Cc: Hao A Wu Cc: Star Zeng Cc: Feng Tian --- MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c | 31 ++++++++------ MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h | 3 +- MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 72 +++++++++++++++++++++++++++-= ---- MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h | 9 +++- 4 files changed, 89 insertions(+), 26 deletions(-) diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c b/MdeModulePkg/Bus/Pci/Xhc= iDxe/Xhci.c index 2f6137ef57..998946a168 100644 --- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c +++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c @@ -1249,27 +1249,34 @@ XhcBulkTransfer ( =20 Status =3D XhcExecTransfer (Xhc, FALSE, Urb, Timeout); =20 - *TransferResult =3D Urb->Result; - *DataLength =3D Urb->Completed; =20 if (Status =3D=3D EFI_TIMEOUT) { // // The transfer timed out. Abort the transfer by dequeueing of the TD. // RecoveryStatus =3D XhcDequeueTrbFromEndpoint(Xhc, Urb); - if (EFI_ERROR(RecoveryStatus)) { + if (RecoveryStatus =3D=3D EFI_ALREADY_STARTED) { + // + // The URB is finished just before stopping endpoint. + // Change returning status from EFI_TIMEOUT to EFI_SUCCESS. + // + ASSERT (Urb->Result =3D=3D EFI_USB_NOERROR); + Status =3D EFI_SUCCESS; + DEBUG ((DEBUG_ERROR, "XhcBulkTransfer: pending URB is finished, Leng= th =3D %d.\n", Urb->Completed)); + } else if (EFI_ERROR(RecoveryStatus)) { DEBUG((EFI_D_ERROR, "XhcBulkTransfer: XhcDequeueTrbFromEndpoint fail= ed\n")); } - } else { - if (*TransferResult =3D=3D EFI_USB_NOERROR) { - Status =3D EFI_SUCCESS; - } else if (*TransferResult =3D=3D EFI_USB_ERR_STALL) { - RecoveryStatus =3D XhcRecoverHaltedEndpoint(Xhc, Urb); - if (EFI_ERROR (RecoveryStatus)) { - DEBUG ((EFI_D_ERROR, "XhcBulkTransfer: XhcRecoverHaltedEndpoint fa= iled\n")); - } - Status =3D EFI_DEVICE_ERROR; + } + + *TransferResult =3D Urb->Result; + *DataLength =3D Urb->Completed; + + if (*TransferResult =3D=3D EFI_USB_ERR_STALL) { + RecoveryStatus =3D XhcRecoverHaltedEndpoint(Xhc, Urb); + if (EFI_ERROR (RecoveryStatus)) { + DEBUG ((DEBUG_ERROR, "XhcBulkTransfer: XhcRecoverHaltedEndpoint fail= ed\n")); } + ASSERT (Status =3D=3D EFI_DEVICE_ERROR); } =20 Xhc->PciIo->Flush (Xhc->PciIo); diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h b/MdeModulePkg/Bus/Pci/Xhc= iDxe/Xhci.h index 28e240245b..76daaff4a4 100644 --- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h +++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h @@ -2,7 +2,7 @@ =20 Provides some data structure definitions used by the XHCI host controlle= r driver. =20 -Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2011 - 2017, Intel Corporation. All rights reserved.
This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD = License which accompanies this distribution. The full text of the license may be = found at @@ -243,6 +243,7 @@ struct _USB_XHCI_INSTANCE { UINT64 *DCBAA; VOID *DCBAAMap; UINT32 MaxSlotsEn; + URB *PendingUrb; // // Cmd Transfer Ring // diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c b/MdeModulePkg/Bus/Pc= i/XhciDxe/XhciSched.c index dbc91023e1..3b0d56587f 100644 --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c @@ -696,6 +696,7 @@ Done: @param Urb The urb which doesn't get completed in a s= pecified timeout range. =20 @retval EFI_SUCCESS The dequeuing of the TDs is successful. + @retval EFI_ALREADY_STARTED The Urb is finished so no deque is needed. @retval Others Failed to stop the endpoint and dequeue th= e TDs. =20 **/ @@ -723,7 +724,7 @@ XhcDequeueTrbFromEndpoint ( // // 1) Send Stop endpoint command to stop xHC from executing of the TDs o= n the endpoint // - Status =3D XhcStopEndpoint(Xhc, SlotId, Dci); + Status =3D XhcStopEndpoint(Xhc, SlotId, Dci, Urb); if (EFI_ERROR(Status)) { DEBUG ((EFI_D_ERROR, "XhcDequeueTrbFromEndpoint: Stop Endpoint Failed,= Status =3D %r\n", Status)); goto Done; @@ -732,10 +733,20 @@ XhcDequeueTrbFromEndpoint ( // // 2)Set dequeue pointer // - Status =3D XhcSetTrDequeuePointer(Xhc, SlotId, Dci, Urb); - if (EFI_ERROR(Status)) { - DEBUG ((EFI_D_ERROR, "XhcDequeueTrbFromEndpoint: Set Transfer Ring Deq= ueue Pointer Failed, Status =3D %r\n", Status)); - goto Done; + if (Urb->Finished && Urb->Result =3D=3D EFI_USB_NOERROR) { + // + // Return Already Started to indicate the pending URB is finished. + // This fixes BULK data loss when transfer is detected as timeout + // but finished just before stopping endpoint. + // + Status =3D EFI_ALREADY_STARTED; + DEBUG ((DEBUG_INFO, "XhcDequeueTrbFromEndpoint: Pending URB is finishe= d: Length Actual/Expect =3D %d/%d!\n", Urb->Completed, Urb->DataLen)); + } else { + Status =3D XhcSetTrDequeuePointer(Xhc, SlotId, Dci, Urb); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, "XhcDequeueTrbFromEndpoint: Set Transfer Ring D= equeue Pointer Failed, Status =3D %r\n", Status)); + goto Done; + } } =20 // @@ -1128,12 +1139,14 @@ XhcCheckUrbResult ( TRBPtr =3D (TRB_TEMPLATE *)(UINTN) UsbHcGetHostAddrForPciAddr (Xhc->Me= mPool, (VOID *)(UINTN) PhyAddr, sizeof (TRB_TEMPLATE)); =20 // - // Update the status of Urb according to the finished event regardless= of whether - // the urb is current checked one or in the XHCI's async transfer list. + // Update the status of URB including the pending URB, the URB that is= currently checked, + // and URBs in the XHCI's async interrupt transfer list. // This way is used to avoid that those completed async transfer event= s don't get // handled in time and are flushed by newer coming events. // - if (IsTransferRingTrb (Xhc, TRBPtr, Urb)) { + if (Xhc->PendingUrb !=3D NULL && IsTransferRingTrb (Xhc, TRBPtr, Xhc->= PendingUrb)) { + CheckedUrb =3D Xhc->PendingUrb; + } else if (IsTransferRingTrb (Xhc, TRBPtr, Urb)) { CheckedUrb =3D Urb; } else if (IsAsyncIntTrb (Xhc, TRBPtr, &AsyncUrb)) { =20 CheckedUrb =3D AsyncUrb; @@ -1166,6 +1179,16 @@ XhcCheckUrbResult ( DEBUG ((EFI_D_ERROR, "XhcCheckUrbResult: TRANSACTION_ERROR! Comple= tecode =3D %x\n",EvtTrb->Completecode)); goto EXIT; =20 + case TRB_COMPLETION_STOPPED: + case TRB_COMPLETION_STOPPED_LENGTH_INVALID: + CheckedUrb->Result |=3D EFI_USB_ERR_TIMEOUT; + CheckedUrb->Finished =3D TRUE; + // + // The pending URB is timeout and force stopped when stopping endp= oint. + // Continue the loop to receive the Command Complete Event for sto= pping endpoint. + // + continue; + case TRB_COMPLETION_SHORT_PACKET: case TRB_COMPLETION_SUCCESS: if (EvtTrb->Completecode =3D=3D TRB_COMPLETION_SHORT_PACKET) { @@ -3158,6 +3181,7 @@ XhcSetConfigCmd64 ( @param Xhc The XHCI Instance. @param SlotId The slot id to be configured. @param Dci The device context index of endpoint. + @param PendingUrb The pending URB to check completion status= when stopping the end point. =20 @retval EFI_SUCCESS Stop endpoint successfully. @retval Others Failed to stop endpoint. @@ -3168,7 +3192,8 @@ EFIAPI XhcStopEndpoint ( IN USB_XHCI_INSTANCE *Xhc, IN UINT8 SlotId, - IN UINT8 Dci + IN UINT8 Dci, + IN URB *PendingUrb OPTIONAL ) { EFI_STATUS Status; @@ -3178,6 +3203,29 @@ XhcStopEndpoint ( DEBUG ((EFI_D_INFO, "XhcStopEndpoint: Slot =3D 0x%x, Dci =3D 0x%x\n", Sl= otId, Dci)); =20 // + // When XhcCheckUrbResult waits for the Stop_Endpoint completion, it als= o checks + // the PendingUrb completion status, because it's possible that the Pend= ingUrb is + // finished just before stopping the end point, but after the looping ch= eck. + // + // The PendingUrb could be passed to XhcCmdTransfer to XhcExecTransfer t= o XhcCheckUrbResult + // through function parameter, but That will cause every consumer of Xhc= CmdTransfer, + // XhcExecTransfer and XhcCheckUrbResult pass a NULL PendingUrb. + // But actually only XhcCheckUrbResult is aware of the PendingUrb. + // So we choose to save the PendingUrb into the USB_XHCI_INSTANCE and us= e it in XhcCheckUrbResult. + // + ASSERT (Xhc->PendingUrb =3D=3D NULL); + Xhc->PendingUrb =3D PendingUrb; + // + // Reset the URB result from Timeout to NoError. + // The USB result will be: + // changed to Timeout when Stop/StopInvalidLength Transfer Event is re= ceived, or + // remain NoError when Success/ShortPacket Transfer Event is received. + // + if (PendingUrb !=3D NULL) { + PendingUrb->Result =3D EFI_USB_NOERROR; + } + + // // Send stop endpoint command to transit Endpoint from running to stop s= tate // ZeroMem (&CmdTrbStopED, sizeof (CmdTrbStopED)); @@ -3195,6 +3243,8 @@ XhcStopEndpoint ( DEBUG ((EFI_D_ERROR, "XhcStopEndpoint: Stop Endpoint Failed, Status = =3D %r\n", Status)); } =20 + Xhc->PendingUrb =3D NULL; + return Status; } =20 @@ -3421,7 +3471,7 @@ XhcSetInterface ( // XHCI 4.3.6 - Setting Alternate Interfaces // 1) Stop any Running Transfer Rings affected by the Alternate Inte= rface setting. // - Status =3D XhcStopEndpoint (Xhc, SlotId, Dci); + Status =3D XhcStopEndpoint (Xhc, SlotId, Dci, NULL); if (EFI_ERROR (Status)) { return Status; } @@ -3623,7 +3673,7 @@ XhcSetInterface64 ( // XHCI 4.3.6 - Setting Alternate Interfaces // 1) Stop any Running Transfer Rings affected by the Alternate Inte= rface setting. // - Status =3D XhcStopEndpoint (Xhc, SlotId, Dci); + Status =3D XhcStopEndpoint (Xhc, SlotId, Dci, NULL); if (EFI_ERROR (Status)) { return Status; } diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h b/MdeModulePkg/Bus/Pc= i/XhciDxe/XhciSched.h index 931c7efa0c..4ed43ad57b 100644 --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h @@ -2,7 +2,7 @@ =20 This file contains the definition for XHCI host controller schedule rout= ines. =20 -Copyright (c) 2011 - 2015, Intel Corporation. All rights reserved.
+Copyright (c) 2011 - 2017, Intel Corporation. All rights reserved.
This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD = License which accompanies this distribution. The full text of the license may be = found at @@ -80,6 +80,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER= EXPRESS OR IMPLIED. #define TRB_COMPLETION_TRB_ERROR 5 #define TRB_COMPLETION_STALL_ERROR 6 #define TRB_COMPLETION_SHORT_PACKET 13 +#define TRB_COMPLETION_STOPPED 26 +#define TRB_COMPLETION_STOPPED_LENGTH_INVALID 27 =20 // // The topology string used to present usb device location @@ -1337,12 +1339,14 @@ XhcDequeueTrbFromEndpoint ( IN URB *Urb ); =20 + /** Stop endpoint through XHCI's Stop_Endpoint cmd. =20 @param Xhc The XHCI Instance. @param SlotId The slot id to be configured. @param Dci The device context index of endpoint. + @param PendingUrb The pending URB to check completion status= when stopping the end point. =20 @retval EFI_SUCCESS Stop endpoint successfully. @retval Others Failed to stop endpoint. @@ -1353,7 +1357,8 @@ EFIAPI XhcStopEndpoint ( IN USB_XHCI_INSTANCE *Xhc, IN UINT8 SlotId, - IN UINT8 Dci + IN UINT8 Dci, + IN URB *PendingUrb OPTIONAL ); =20 /** --=20 2.12.2.windows.2 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel