Could you please create a separate function for 'Set dequeue pointer' since it's already in XhcRecoverHaltedEndpoint as well?
-Baranee > -----Original Message----- > From: edk2-devel [mailto:[email protected]] On Behalf Of Tian > Feng > Sent: Monday, August 24, 2015 12:53 AM > To: [email protected] > Cc: [email protected]; Feng Tian > Subject: [edk2] [patch] MdeModulePkg/Xhci: Remove TDs from transfer ring when > timeout happens > > The error handling for timeout case is enhanced to remove TDs from transfer > ring. > The original code only removed s/w URB, but the h/w transfer descriptor TDs > didn't > get removed. It would cause data lost for data stream peripheral, such as > usb-to- > serial device, from the s/w perspective. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Feng Tian <[email protected]> > Cc: Star Zeng <[email protected]> > --- > MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c | 75 ++++++++++++++++++++-------- > MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 77 ++++++++++++++++++++++++++++ > MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h | 39 +++++++++++++++ > MdeModulePkg/Bus/Pci/XhciPei/XhcPeim.c | 51 +++++++++++++------ > MdeModulePkg/Bus/Pci/XhciPei/XhciSched.c | 86 > ++++++++++++++++++++++++++++++++ MdeModulePkg/Bus/Pci/XhciPei/XhciSched.h > | 21 +++++++- > 6 files changed, 311 insertions(+), 38 deletions(-) > > diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c > b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c > index 390ca0a..39c28ab 100644 > --- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c > +++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c > @@ -905,17 +905,28 @@ XhcControlTransfer ( > *TransferResult = Urb->Result; > *DataLength = Urb->Completed; > > - if (*TransferResult == EFI_USB_NOERROR) { > - Status = EFI_SUCCESS; > - } else if (*TransferResult == EFI_USB_ERR_STALL) { > - RecoveryStatus = XhcRecoverHaltedEndpoint(Xhc, Urb); > - if (EFI_ERROR (RecoveryStatus)) { > - DEBUG ((EFI_D_ERROR, "XhcControlTransfer: XhcRecoverHaltedEndpoint > failed\n")); > + if (Status == EFI_TIMEOUT) { > + // > + // The transfer timed out. Abort the transfer by dequeueing of the TD. > + // > + RecoveryStatus = XhcDequeueTrbFromEndpoint(Xhc, Urb); > + if (EFI_ERROR(RecoveryStatus)) { > + DEBUG((EFI_D_ERROR, "XhcControlTransfer: > + XhcDequeueTrbFromEndpoint failed\n")); > } > - Status = EFI_DEVICE_ERROR; > goto FREE_URB; > } else { > - goto FREE_URB; > + if (*TransferResult == EFI_USB_NOERROR) { > + Status = EFI_SUCCESS; > + } else if (*TransferResult == EFI_USB_ERR_STALL) { > + RecoveryStatus = XhcRecoverHaltedEndpoint(Xhc, Urb); > + if (EFI_ERROR (RecoveryStatus)) { > + DEBUG ((EFI_D_ERROR, "XhcControlTransfer: XhcRecoverHaltedEndpoint > failed\n")); > + } > + Status = EFI_DEVICE_ERROR; > + goto FREE_URB; > + } else { > + goto FREE_URB; > + } > } > > Xhc->PciIo->Flush (Xhc->PciIo); > @@ -1241,14 +1252,24 @@ XhcBulkTransfer ( > *TransferResult = Urb->Result; > *DataLength = Urb->Completed; > > - if (*TransferResult == EFI_USB_NOERROR) { > - Status = EFI_SUCCESS; > - } else if (*TransferResult == EFI_USB_ERR_STALL) { > - RecoveryStatus = XhcRecoverHaltedEndpoint(Xhc, Urb); > - if (EFI_ERROR (RecoveryStatus)) { > - DEBUG ((EFI_D_ERROR, "XhcBulkTransfer: XhcRecoverHaltedEndpoint > failed\n")); > + if (Status == EFI_TIMEOUT) { > + // > + // The transfer timed out. Abort the transfer by dequeueing of the TD. > + // > + RecoveryStatus = XhcDequeueTrbFromEndpoint(Xhc, Urb); > + if (EFI_ERROR(RecoveryStatus)) { > + DEBUG((EFI_D_ERROR, "XhcBulkTransfer: XhcDequeueTrbFromEndpoint > failed\n")); > + } > + } else { > + if (*TransferResult == EFI_USB_NOERROR) { > + Status = EFI_SUCCESS; > + } else if (*TransferResult == EFI_USB_ERR_STALL) { > + RecoveryStatus = XhcRecoverHaltedEndpoint(Xhc, Urb); > + if (EFI_ERROR (RecoveryStatus)) { > + DEBUG ((EFI_D_ERROR, "XhcBulkTransfer: XhcRecoverHaltedEndpoint > failed\n")); > + } > + Status = EFI_DEVICE_ERROR; > } > - Status = EFI_DEVICE_ERROR; > } > > Xhc->PciIo->Flush (Xhc->PciIo); > @@ -1538,14 +1559,24 @@ XhcSyncInterruptTransfer ( > *TransferResult = Urb->Result; > *DataLength = Urb->Completed; > > - if (*TransferResult == EFI_USB_NOERROR) { > - Status = EFI_SUCCESS; > - } else if (*TransferResult == EFI_USB_ERR_STALL) { > - RecoveryStatus = XhcRecoverHaltedEndpoint(Xhc, Urb); > - if (EFI_ERROR (RecoveryStatus)) { > - DEBUG ((EFI_D_ERROR, "XhcSyncInterruptTransfer: > XhcRecoverHaltedEndpoint > failed\n")); > + if (Status == EFI_TIMEOUT) { > + // > + // The transfer timed out. Abort the transfer by dequeueing of the TD. > + // > + RecoveryStatus = XhcDequeueTrbFromEndpoint(Xhc, Urb); > + if (EFI_ERROR(RecoveryStatus)) { > + DEBUG((EFI_D_ERROR, "XhcSyncInterruptTransfer: > XhcDequeueTrbFromEndpoint > failed\n")); > + } > + } else { > + if (*TransferResult == EFI_USB_NOERROR) { > + Status = EFI_SUCCESS; > + } else if (*TransferResult == EFI_USB_ERR_STALL) { > + RecoveryStatus = XhcRecoverHaltedEndpoint(Xhc, Urb); > + if (EFI_ERROR (RecoveryStatus)) { > + DEBUG ((EFI_D_ERROR, "XhcSyncInterruptTransfer: > XhcRecoverHaltedEndpoint > failed\n")); > + } > + Status = EFI_DEVICE_ERROR; > } > - Status = EFI_DEVICE_ERROR; > } > > Xhc->PciIo->Flush (Xhc->PciIo); > diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c > b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c > index 05cd616..bbaa81f 100644 > --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c > +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c > @@ -713,6 +713,83 @@ Done: > } > > /** > + System software shall use a Stop Endpoint Command (section 4.6.9) and > + the Set TR Dequeue Pointer Command (section 4.6.10) to remove the > + timed-out TDs from the xHC transfer ring. The next write to the > + Doorbell of the Endpoint will transition the Endpoint Context from the > Stopped to > the Running state. > + > + @param Xhc The XHCI Instance. > + @param Urb The urb which doesn't get completed in a > specified timeout > range. > + > + @retval EFI_SUCCESS The dequeuing of the TDs is successful. > + @retval Others Failed to stop the endpoint and dequeue the > TDs. > + > +**/ > +EFI_STATUS > +EFIAPI > +XhcDequeueTrbFromEndpoint ( > + IN USB_XHCI_INSTANCE *Xhc, > + IN URB *Urb > + ) > +{ > + EFI_STATUS Status; > + EVT_TRB_COMMAND_COMPLETION *EvtTrb; > + CMD_SET_TR_DEQ_POINTER CmdSetTRDeq; > + UINT8 Dci; > + UINT8 SlotId; > + EFI_PHYSICAL_ADDRESS PhyAddr; > + > + Status = EFI_SUCCESS; > + SlotId = XhcBusDevAddrToSlotId (Xhc, Urb->Ep.BusAddr); if (SlotId == > + 0) { > + return EFI_DEVICE_ERROR; > + } > + Dci = XhcEndpointToDci (Urb->Ep.EpAddr, (UINT8)(Urb->Ep.Direction)); > + ASSERT (Dci < 32); > + > + DEBUG ((EFI_D_INFO, "Stop Slot = %x,Dci = %x\n", SlotId, Dci)); > + > + // > + // 1) Send Stop endpoint command to stop xHC from executing of the > + TDs on the endpoint // Status = XhcStopEndpoint(Xhc, SlotId, Dci); > + if (EFI_ERROR(Status)) { > + DEBUG ((EFI_D_ERROR, "XhcDequeueTrbFromEndpoint: Stop Endpoint Failed, > Status = %r\n", Status)); > + goto Done; > + } > + > + // > + // 2)Set dequeue pointer > + // > + ZeroMem (&CmdSetTRDeq, sizeof (CmdSetTRDeq)); PhyAddr = > + UsbHcGetPciAddrForHostAddr (Xhc->MemPool, Urb->Ring->RingEnqueue, sizeof > (CMD_SET_TR_DEQ_POINTER)); > + CmdSetTRDeq.PtrLo = XHC_LOW_32BIT (PhyAddr) | Urb->Ring->RingPCS; > + CmdSetTRDeq.PtrHi = XHC_HIGH_32BIT (PhyAddr); > + CmdSetTRDeq.CycleBit = 1; > + CmdSetTRDeq.Type = TRB_TYPE_SET_TR_DEQUE; > + CmdSetTRDeq.Endpoint = Dci; > + CmdSetTRDeq.SlotId = SlotId; > + Status = XhcCmdTransfer ( > + Xhc, > + (TRB_TEMPLATE *) (UINTN) &CmdSetTRDeq, > + XHC_GENERIC_TIMEOUT, > + (TRB_TEMPLATE **) (UINTN) &EvtTrb > + ); > + if (EFI_ERROR(Status)) { > + DEBUG ((EFI_D_ERROR, "XhcDequeueTrbFromEndpoint: Set Dequeue Pointer > Failed, > Status = %r\n", Status)); > + goto Done; > + } > + > + // > + // 3)Ring the doorbell to transit from stop to active // > + XhcRingDoorBell (Xhc, SlotId, Dci); > + > +Done: > + return Status; > +} > + > +/** > Create XHCI event ring. > > @param Xhc The XHCI Instance. > diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h > b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h > index 1b6e345..68b2977 100644 > --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h > +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h > @@ -1318,6 +1318,45 @@ XhcRecoverHaltedEndpoint ( > ); > > /** > + System software shall use a Stop Endpoint Command (section 4.6.9) and > + the Set TR Dequeue Pointer Command (section 4.6.10) to remove the > + timed-out TDs from the xHC transfer ring. The next write to the > + Doorbell of the Endpoint will transition the Endpoint Context from the > Stopped to > the Running state. > + > + @param Xhc The XHCI Instance. > + @param Urb The urb which doesn't get completed in a > specified timeout > range. > + > + @retval EFI_SUCCESS The dequeuing of the TDs is successful. > + @retval Others Failed to stop the endpoint and dequeue the > TDs. > + > +**/ > +EFI_STATUS > +EFIAPI > +XhcDequeueTrbFromEndpoint ( > + IN USB_XHCI_INSTANCE *Xhc, > + IN URB *Urb > + ); > + > +/** > + Stop endpoint through XHCI's Stop_Endpoint cmd. > + > + @param Xhc The XHCI Instance. > + @param SlotId The slot id to be configured. > + @param Dci The device context index of endpoint. > + > + @retval EFI_SUCCESS Stop endpoint successfully. > + @retval Others Failed to stop endpoint. > + > +**/ > +EFI_STATUS > +EFIAPI > +XhcStopEndpoint ( > + IN USB_XHCI_INSTANCE *Xhc, > + IN UINT8 SlotId, > + IN UINT8 Dci > + ); > + > +/** > Create a new URB for a new transaction. > > @param Xhc The XHCI Instance > diff --git a/MdeModulePkg/Bus/Pci/XhciPei/XhcPeim.c > b/MdeModulePkg/Bus/Pci/XhciPei/XhcPeim.c > index 2f16b82..eaea38d 100644 > --- a/MdeModulePkg/Bus/Pci/XhciPei/XhcPeim.c > +++ b/MdeModulePkg/Bus/Pci/XhciPei/XhcPeim.c > @@ -648,17 +648,28 @@ XhcPeiControlTransfer ( > *TransferResult = Urb->Result; > *DataLength = Urb->Completed; > > - if (*TransferResult == EFI_USB_NOERROR) { > - Status = EFI_SUCCESS; > - } else if (*TransferResult == EFI_USB_ERR_STALL) { > - RecoveryStatus = XhcPeiRecoverHaltedEndpoint(Xhc, Urb); > - if (EFI_ERROR (RecoveryStatus)) { > - DEBUG ((EFI_D_ERROR, "XhcPeiControlTransfer: > XhcPeiRecoverHaltedEndpoint > failed\n")); > + if (Status == EFI_TIMEOUT) { > + // > + // The transfer timed out. Abort the transfer by dequeueing of the TD. > + // > + RecoveryStatus = XhcPeiDequeueTrbFromEndpoint(Xhc, Urb); > + if (EFI_ERROR(RecoveryStatus)) { > + DEBUG((EFI_D_ERROR, "XhcPeiControlTransfer: > + XhcPeiDequeueTrbFromEndpoint failed\n")); > } > - Status = EFI_DEVICE_ERROR; > goto FREE_URB; > } else { > - goto FREE_URB; > + if (*TransferResult == EFI_USB_NOERROR) { > + Status = EFI_SUCCESS; > + } else if (*TransferResult == EFI_USB_ERR_STALL) { > + RecoveryStatus = XhcPeiRecoverHaltedEndpoint(Xhc, Urb); > + if (EFI_ERROR (RecoveryStatus)) { > + DEBUG ((EFI_D_ERROR, "XhcPeiControlTransfer: > XhcPeiRecoverHaltedEndpoint > failed\n")); > + } > + Status = EFI_DEVICE_ERROR; > + goto FREE_URB; > + } else { > + goto FREE_URB; > + } > } > > // > @@ -960,14 +971,24 @@ XhcPeiBulkTransfer ( > *TransferResult = Urb->Result; > *DataLength = Urb->Completed; > > - if (*TransferResult == EFI_USB_NOERROR) { > - Status = EFI_SUCCESS; > - } else if (*TransferResult == EFI_USB_ERR_STALL) { > - RecoveryStatus = XhcPeiRecoverHaltedEndpoint(Xhc, Urb); > - if (EFI_ERROR (RecoveryStatus)) { > - DEBUG ((EFI_D_ERROR, "XhcPeiBulkTransfer: XhcPeiRecoverHaltedEndpoint > failed\n")); > + if (Status == EFI_TIMEOUT) { > + // > + // The transfer timed out. Abort the transfer by dequeueing of the TD. > + // > + RecoveryStatus = XhcPeiDequeueTrbFromEndpoint(Xhc, Urb); > + if (EFI_ERROR(RecoveryStatus)) { > + DEBUG((EFI_D_ERROR, "XhcPeiBulkTransfer: XhcPeiDequeueTrbFromEndpoint > failed\n")); > + } > + } else { > + if (*TransferResult == EFI_USB_NOERROR) { > + Status = EFI_SUCCESS; > + } else if (*TransferResult == EFI_USB_ERR_STALL) { > + RecoveryStatus = XhcPeiRecoverHaltedEndpoint(Xhc, Urb); > + if (EFI_ERROR (RecoveryStatus)) { > + DEBUG ((EFI_D_ERROR, "XhcPeiBulkTransfer: XhcPeiRecoverHaltedEndpoint > failed\n")); > + } > + Status = EFI_DEVICE_ERROR; > } > - Status = EFI_DEVICE_ERROR; > } > > XhcPeiFreeUrb (Xhc, Urb); > diff --git a/MdeModulePkg/Bus/Pci/XhciPei/XhciSched.c > b/MdeModulePkg/Bus/Pci/XhciPei/XhciSched.c > index eedf377..207b328 100644 > --- a/MdeModulePkg/Bus/Pci/XhciPei/XhciSched.c > +++ b/MdeModulePkg/Bus/Pci/XhciPei/XhciSched.c > @@ -511,6 +511,92 @@ Done: > } > > /** > + System software shall use a Stop Endpoint Command (section 4.6.9) and > + the Set TR Dequeue Pointer Command (section 4.6.10) to remove the > + timed-out TDs from the xHC transfer ring. The next write to the > + Doorbell of the Endpoint will transition the Endpoint Context from the > Stopped to > the Running state. > + > + @param Xhc The XHCI device. > + @param Urb The urb which doesn't get completed in a > specified timeout > range. > + > + @retval EFI_SUCCESS The dequeuing of the TDs is successful. > + @retval Others Failed to stop the endpoint and dequeue the > TDs. > + > +**/ > +EFI_STATUS > +XhcPeiDequeueTrbFromEndpoint ( > + IN PEI_XHC_DEV *Xhc, > + IN URB *Urb > + ) > +{ > + EFI_STATUS Status; > + EVT_TRB_COMMAND_COMPLETION *EvtTrb; > + CMD_TRB_STOP_ENDPOINT CmdTrbStopED; > + CMD_SET_TR_DEQ_POINTER CmdSetTRDeq; > + UINT8 Dci; > + UINT8 SlotId; > + EFI_PHYSICAL_ADDRESS PhyAddr; > + > + Status = EFI_SUCCESS; > + SlotId = XhcPeiBusDevAddrToSlotId (Xhc, Urb->Ep.BusAddr); if (SlotId > + == 0) { > + return EFI_DEVICE_ERROR; > + } > + Dci = XhcPeiEndpointToDci (Urb->Ep.EpAddr, (UINT8) > + (Urb->Ep.Direction)); > + > + DEBUG ((EFI_D_INFO, "XhcPeiDequeueTrbFromEndpoint: Stop Slot = %x, > + Dci = %x\n", SlotId, Dci)); > + > + // > + // 1) Send Stop endpoint command to stop endpoint. > + // > + ZeroMem (&CmdTrbStopED, sizeof (CmdTrbStopED)); > + CmdTrbStopED.CycleBit = 1; > + CmdTrbStopED.Type = TRB_TYPE_STOP_ENDPOINT; > + CmdTrbStopED.EDID = Dci; > + CmdTrbStopED.SlotId = SlotId; > + Status = XhcPeiCmdTransfer ( > + Xhc, > + (TRB_TEMPLATE *) (UINTN) &CmdTrbStopED, > + XHC_GENERIC_TIMEOUT, > + (TRB_TEMPLATE **) (UINTN) &EvtTrb > + ); > + if (EFI_ERROR(Status)) { > + DEBUG ((EFI_D_ERROR, "XhcPeiDequeueTrbFromEndpoint: Stop Endpoint Failed, > Status = %r\n", Status)); > + goto Done; > + } > + > + // > + // 2) Set dequeue pointer > + // > + ZeroMem (&CmdSetTRDeq, sizeof (CmdSetTRDeq)); PhyAddr = > + UsbHcGetPciAddrForHostAddr (Xhc->MemPool, Urb->Ring->RingEnqueue, sizeof > (CMD_SET_TR_DEQ_POINTER)); > + CmdSetTRDeq.PtrLo = XHC_LOW_32BIT (PhyAddr) | Urb->Ring->RingPCS; > + CmdSetTRDeq.PtrHi = XHC_HIGH_32BIT (PhyAddr); > + CmdSetTRDeq.CycleBit = 1; > + CmdSetTRDeq.Type = TRB_TYPE_SET_TR_DEQUE; > + CmdSetTRDeq.Endpoint = Dci; > + CmdSetTRDeq.SlotId = SlotId; > + Status = XhcPeiCmdTransfer ( > + Xhc, > + (TRB_TEMPLATE *) (UINTN) &CmdSetTRDeq, > + XHC_GENERIC_TIMEOUT, > + (TRB_TEMPLATE **) (UINTN) &EvtTrb > + ); > + if (EFI_ERROR(Status)) { > + DEBUG ((EFI_D_ERROR, "XhcPeiDequeueTrbFromEndpoint: Set Dequeue Pointer > Failed, Status = %r\n", Status)); > + goto Done; > + } > + > + // > + // 3) Ring the doorbell to transit from stop to active // > + XhcPeiRingDoorBell (Xhc, SlotId, Dci); > + > +Done: > + return Status; > +} > + > +/** > Check if the Trb is a transaction of the URB. > > @param Trb The TRB to be checked > diff --git a/MdeModulePkg/Bus/Pci/XhciPei/XhciSched.h > b/MdeModulePkg/Bus/Pci/XhciPei/XhciSched.h > index 19672d0..e0e1938 100644 > --- a/MdeModulePkg/Bus/Pci/XhciPei/XhciSched.h > +++ b/MdeModulePkg/Bus/Pci/XhciPei/XhciSched.h > @@ -1,7 +1,7 @@ > /** @file > Private Header file for Usb Host Controller PEIM > > -Copyright (c) 2014, Intel Corporation. All rights reserved.<BR> > +Copyright (c) 2014 - 2015, Intel Corporation. All rights reserved.<BR> > > This program and the accompanying materials are licensed and made available > under the terms and conditions @@ -1067,6 +1067,25 @@ > XhcPeiRecoverHaltedEndpoint ( > ); > > /** > + System software shall use a Stop Endpoint Command (section 4.6.9) and > + the Set TR Dequeue Pointer Command (section 4.6.10) to remove the > + timed-out TDs from the xHC transfer ring. The next write to the > + Doorbell of the Endpoint will transition the Endpoint Context from the > Stopped to > the Running state. > + > + @param Xhc The XHCI device. > + @param Urb The urb which doesn't get completed in a > specified timeout > range. > + > + @retval EFI_SUCCESS The dequeuing of the TDs is successful. > + @retval Others Failed to stop the endpoint and dequeue the > TDs. > + > +**/ > +EFI_STATUS > +XhcPeiDequeueTrbFromEndpoint ( > + IN PEI_XHC_DEV *Xhc, > + IN URB *Urb > + ); > + > +/** > Create a new URB for a new transaction. > > @param Xhc The XHCI device > -- > 1.9.5.msysgit.0 > > _______________________________________________ > edk2-devel mailing list > [email protected] > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
