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>

diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
index 42b773ab31bea8d5264b6afa708f848014b538fd..2bab09415b28dc6127503f169783164ff5bb6101 100644 (file)
--- 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\r
   to become set (or clear).\r
 \r
-  @param  Xhc          The XHCI Instance.\r
-  @param  Offset       The offset of the operation register.\r
-  @param  Bit          The bit of the register to wait for.\r
-  @param  WaitToSet    Wait the bit to set or clear.\r
-  @param  Timeout      The time to wait before abort (in millisecond, ms).\r
+  @param  Xhc                    The XHCI Instance.\r
+  @param  Offset                 The offset of the operation register.\r
+  @param  Bit                    The bit of the register to wait for.\r
+  @param  WaitToSet              Wait the bit to set or clear.\r
+  @param  Timeout                The time to wait before abort (in millisecond, ms).\r
 \r
-  @retval EFI_SUCCESS  The bit successfully changed by host controller.\r
-  @retval EFI_TIMEOUT  The time out occurred.\r
+  @retval EFI_SUCCESS            The bit successfully changed by host controller.\r
+  @retval EFI_TIMEOUT            The time out occurred.\r
+  @retval EFI_OUT_OF_RESOURCES   Memory for the timer event could not be allocated.\r
 \r
 **/\r
 EFI_STATUS\r
@@ -442,20 +443,52 @@ XhcWaitOpRegBit (
   IN UINT32               Timeout\r
   )\r
 {\r
-  UINT32                  Index;\r
-  UINT64                  Loop;\r
+  EFI_STATUS Status;\r
+  EFI_EVENT  TimeoutEvent;\r
+\r
+  TimeoutEvent = NULL;\r
+\r
+  if (Timeout == 0) {\r
+    return EFI_TIMEOUT;\r
+  }\r
+\r
+  Status = gBS->CreateEvent (\r
+                  EVT_TIMER,\r
+                  TPL_CALLBACK,\r
+                  NULL,\r
+                  NULL,\r
+                  &TimeoutEvent\r
+                  );\r
+\r
+  if (EFI_ERROR(Status)) {\r
+    goto DONE;\r
+  }\r
 \r
-  Loop   = Timeout * XHC_1_MILLISECOND;\r
+  Status = gBS->SetTimer (TimeoutEvent,\r
+                          TimerRelative,\r
+                          EFI_TIMER_PERIOD_MILLISECONDS(Timeout));\r
 \r
-  for (Index = 0; Index < Loop; Index++) {\r
+  if (EFI_ERROR(Status)) {\r
+    goto DONE;\r
+  }\r
+\r
+  do {\r
     if (XHC_REG_BIT_IS_SET (Xhc, Offset, Bit) == WaitToSet) {\r
-      return EFI_SUCCESS;\r
+      Status = EFI_SUCCESS;\r
+      goto DONE;\r
     }\r
 \r
     gBS->Stall (XHC_1_MICROSECOND);\r
+  } while (EFI_ERROR(gBS->CheckEvent (TimeoutEvent)));\r
+\r
+  Status = EFI_TIMEOUT;\r
+\r
+DONE:\r
+  if (TimeoutEvent != NULL) {\r
+    gBS->CloseEvent (TimeoutEvent);\r
   }\r
 \r
-  return EFI_TIMEOUT;\r
+  return Status;\r
 }\r
 \r
 /**\r

diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
index ab8957c546ee2f66ddb7dd0eb3cfc653848b18e4..9cb115363c8b7ef5992750515dd59839df50dce3 100644 (file)
--- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
@@ -1254,14 +1254,15 @@ EXIT:
 /**\r
   Execute the transfer by polling the URB. This is a synchronous operation.\r
 \r
-  @param  Xhc               The XHCI Instance.\r
-  @param  CmdTransfer       The executed URB is for cmd transfer or not.\r
-  @param  Urb               The URB to execute.\r
-  @param  Timeout           The time to wait before abort, in millisecond.\r
+  @param  Xhc                    The XHCI Instance.\r
+  @param  CmdTransfer            The executed URB is for cmd transfer or not.\r
+  @param  Urb                    The URB to execute.\r
+  @param  Timeout                The time to wait before abort, in millisecond.\r
 \r
-  @return EFI_DEVICE_ERROR  The transfer failed due to transfer error.\r
-  @return EFI_TIMEOUT       The transfer failed due to time out.\r
-  @return EFI_SUCCESS       The transfer finished OK.\r
+  @return EFI_DEVICE_ERROR       The transfer failed due to transfer error.\r
+  @return EFI_TIMEOUT            The transfer failed due to time out.\r
+  @return EFI_SUCCESS            The transfer finished OK.\r
+  @retval EFI_OUT_OF_RESOURCES   Memory for the timer event could not be allocated.\r
 \r
 **/\r
 EFI_STATUS\r
@@ -1273,11 +1274,16 @@ XhcExecTransfer (
   )\r
 {\r
   EFI_STATUS              Status;\r
-  UINTN                   Index;\r
-  UINT64                  Loop;\r
   UINT8                   SlotId;\r
   UINT8                   Dci;\r
   BOOLEAN                 Finished;\r
+  EFI_EVENT               TimeoutEvent;\r
+  BOOLEAN                 IndefiniteTimeout;\r
+\r
+  Status            = EFI_SUCCESS;\r
+  Finished          = FALSE;\r
+  TimeoutEvent      = NULL;\r
+  IndefiniteTimeout = FALSE;\r
 \r
   if (CmdTransfer) {\r
     SlotId = 0;\r
@@ -1291,29 +1297,56 @@ XhcExecTransfer (
     ASSERT (Dci < 32);\r
   }\r
 \r
-  Status = EFI_SUCCESS;\r
-  Loop   = Timeout * XHC_1_MILLISECOND;\r
   if (Timeout == 0) {\r
-    Loop = 0xFFFFFFFF;\r
+    IndefiniteTimeout = TRUE;\r
+    goto RINGDOORBELL;\r
+  }\r
+\r
+  Status = gBS->CreateEvent (\r
+                  EVT_TIMER,\r
+                  TPL_CALLBACK,\r
+                  NULL,\r
+                  NULL,\r
+                  &TimeoutEvent\r
+                  );\r
+\r
+  if (EFI_ERROR (Status)) {\r
+    goto DONE;\r
   }\r
 \r
+  Status = gBS->SetTimer (TimeoutEvent,\r
+                          TimerRelative,\r
+                          EFI_TIMER_PERIOD_MILLISECONDS(Timeout));\r
+\r
+  if (EFI_ERROR (Status)) {\r
+    goto DONE;\r
+  }\r
+\r
+RINGDOORBELL:\r
   XhcRingDoorBell (Xhc, SlotId, Dci);\r
 \r
-  for (Index = 0; Index < Loop; Index++) {\r
+  do {\r
     Finished = XhcCheckUrbResult (Xhc, Urb);\r
     if (Finished) {\r
       break;\r
     }\r
     gBS->Stall (XHC_1_MICROSECOND);\r
-  }\r
+  } while (IndefiniteTimeout || EFI_ERROR(gBS->CheckEvent (TimeoutEvent)));\r
 \r
-  if (Index == Loop) {\r
+DONE:\r
+  if (EFI_ERROR(Status)) {\r
+    Urb->Result = EFI_USB_ERR_NOTEXECUTE;\r
+  } else if (!Finished) {\r
     Urb->Result = EFI_USB_ERR_TIMEOUT;\r
     Status      = EFI_TIMEOUT;\r
   } else if (Urb->Result != EFI_USB_NOERROR) {\r
     Status      = EFI_DEVICE_ERROR;\r
   }\r
 \r
+  if (TimeoutEvent != NULL) {\r
+    gBS->CloseEvent (TimeoutEvent);\r
+  }\r
+\r
   return Status;\r
 }\r
 \r