diff options
| author | Patrick Henz <patrick.henz@hpe.com> | 2020-09-24 03:36:03 +0800 |
|---|---|---|
| committer | mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> | 2020-09-29 01:28:58 +0000 |
| commit | 71dd80f14f2724ccc99dd7d349b7392adf114019 (patch) | |
| tree | daaaba88d0c97a2c69f3208362a7174a389c7c13 /MdeModulePkg/Bus | |
| parent | 2793a49565488e419d10ba029c838f4b7efdba38 (diff) | |
| download | edk2-71dd80f14f2724ccc99dd7d349b7392adf114019.tar.gz edk2-71dd80f14f2724ccc99dd7d349b7392adf114019.tar.bz2 edk2-71dd80f14f2724ccc99dd7d349b7392adf114019.zip | |
MdeModulePkg/XhciDxe: Fix Broken Timeouts
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2948
Timeouts in the XhciDxe driver are taking longer than
expected due to the timeout loops not accounting for
code execution time. As en example, 5 second timeouts
have been observed to take around 36 seconds to complete.
Use SetTimer and Create/CheckEvent from Boot Services to
determine when timeout occurred.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Patrick Henz <patrick.henz@hpe.com>
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
Diffstat (limited to 'MdeModulePkg/Bus')
| -rw-r--r-- | MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c | 59 | ||||
| -rw-r--r-- | MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 63 |
2 files changed, 94 insertions, 28 deletions
diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c index 42b773ab31..2bab09415b 100644 --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c @@ -423,14 +423,15 @@ XhcClearOpRegBit ( Wait the operation register's bit as specified by Bit
to become set (or clear).
- @param Xhc The XHCI Instance.
- @param Offset The offset of the operation register.
- @param Bit The bit of the register to wait for.
- @param WaitToSet Wait the bit to set or clear.
- @param Timeout The time to wait before abort (in millisecond, ms).
+ @param Xhc The XHCI Instance.
+ @param Offset The offset of the operation register.
+ @param Bit The bit of the register to wait for.
+ @param WaitToSet Wait the bit to set or clear.
+ @param Timeout The time to wait before abort (in millisecond, ms).
- @retval EFI_SUCCESS The bit successfully changed by host controller.
- @retval EFI_TIMEOUT The time out occurred.
+ @retval EFI_SUCCESS The bit successfully changed by host controller.
+ @retval EFI_TIMEOUT The time out occurred.
+ @retval EFI_OUT_OF_RESOURCES Memory for the timer event could not be allocated.
**/
EFI_STATUS
@@ -442,20 +443,52 @@ XhcWaitOpRegBit ( IN UINT32 Timeout
)
{
- UINT32 Index;
- UINT64 Loop;
+ EFI_STATUS Status;
+ EFI_EVENT TimeoutEvent;
+
+ TimeoutEvent = NULL;
+
+ if (Timeout == 0) {
+ return EFI_TIMEOUT;
+ }
+
+ Status = gBS->CreateEvent (
+ EVT_TIMER,
+ TPL_CALLBACK,
+ NULL,
+ NULL,
+ &TimeoutEvent
+ );
+
+ if (EFI_ERROR(Status)) {
+ goto DONE;
+ }
- Loop = Timeout * XHC_1_MILLISECOND;
+ Status = gBS->SetTimer (TimeoutEvent,
+ TimerRelative,
+ EFI_TIMER_PERIOD_MILLISECONDS(Timeout));
- for (Index = 0; Index < Loop; Index++) {
+ if (EFI_ERROR(Status)) {
+ goto DONE;
+ }
+
+ do {
if (XHC_REG_BIT_IS_SET (Xhc, Offset, Bit) == WaitToSet) {
- return EFI_SUCCESS;
+ Status = EFI_SUCCESS;
+ goto DONE;
}
gBS->Stall (XHC_1_MICROSECOND);
+ } while (EFI_ERROR(gBS->CheckEvent (TimeoutEvent)));
+
+ Status = EFI_TIMEOUT;
+
+DONE:
+ if (TimeoutEvent != NULL) {
+ gBS->CloseEvent (TimeoutEvent);
}
- return EFI_TIMEOUT;
+ return Status;
}
/**
diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c index ab8957c546..9cb115363c 100644 --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c @@ -1254,14 +1254,15 @@ EXIT: /**
Execute the transfer by polling the URB. This is a synchronous operation.
- @param Xhc The XHCI Instance.
- @param CmdTransfer The executed URB is for cmd transfer or not.
- @param Urb The URB to execute.
- @param Timeout The time to wait before abort, in millisecond.
+ @param Xhc The XHCI Instance.
+ @param CmdTransfer The executed URB is for cmd transfer or not.
+ @param Urb The URB to execute.
+ @param Timeout The time to wait before abort, in millisecond.
- @return EFI_DEVICE_ERROR The transfer failed due to transfer error.
- @return EFI_TIMEOUT The transfer failed due to time out.
- @return EFI_SUCCESS The transfer finished OK.
+ @return EFI_DEVICE_ERROR The transfer failed due to transfer error.
+ @return EFI_TIMEOUT The transfer failed due to time out.
+ @return EFI_SUCCESS The transfer finished OK.
+ @retval EFI_OUT_OF_RESOURCES Memory for the timer event could not be allocated.
**/
EFI_STATUS
@@ -1273,11 +1274,16 @@ XhcExecTransfer ( )
{
EFI_STATUS Status;
- UINTN Index;
- UINT64 Loop;
UINT8 SlotId;
UINT8 Dci;
BOOLEAN Finished;
+ EFI_EVENT TimeoutEvent;
+ BOOLEAN IndefiniteTimeout;
+
+ Status = EFI_SUCCESS;
+ Finished = FALSE;
+ TimeoutEvent = NULL;
+ IndefiniteTimeout = FALSE;
if (CmdTransfer) {
SlotId = 0;
@@ -1291,29 +1297,56 @@ XhcExecTransfer ( ASSERT (Dci < 32);
}
- Status = EFI_SUCCESS;
- Loop = Timeout * XHC_1_MILLISECOND;
if (Timeout == 0) {
- Loop = 0xFFFFFFFF;
+ IndefiniteTimeout = TRUE;
+ goto RINGDOORBELL;
+ }
+
+ Status = gBS->CreateEvent (
+ EVT_TIMER,
+ TPL_CALLBACK,
+ NULL,
+ NULL,
+ &TimeoutEvent
+ );
+
+ if (EFI_ERROR (Status)) {
+ goto DONE;
}
+ Status = gBS->SetTimer (TimeoutEvent,
+ TimerRelative,
+ EFI_TIMER_PERIOD_MILLISECONDS(Timeout));
+
+ if (EFI_ERROR (Status)) {
+ goto DONE;
+ }
+
+RINGDOORBELL:
XhcRingDoorBell (Xhc, SlotId, Dci);
- for (Index = 0; Index < Loop; Index++) {
+ do {
Finished = XhcCheckUrbResult (Xhc, Urb);
if (Finished) {
break;
}
gBS->Stall (XHC_1_MICROSECOND);
- }
+ } while (IndefiniteTimeout || EFI_ERROR(gBS->CheckEvent (TimeoutEvent)));
- if (Index == Loop) {
+DONE:
+ if (EFI_ERROR(Status)) {
+ Urb->Result = EFI_USB_ERR_NOTEXECUTE;
+ } else if (!Finished) {
Urb->Result = EFI_USB_ERR_TIMEOUT;
Status = EFI_TIMEOUT;
} else if (Urb->Result != EFI_USB_NOERROR) {
Status = EFI_DEVICE_ERROR;
}
+ if (TimeoutEvent != NULL) {
+ gBS->CloseEvent (TimeoutEvent);
+ }
+
return Status;
}
|