MdeModulePkg/XhciDxe: Use Performance Timer for XHCI Timeouts

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2948

XhciDxe uses the timer functionality provided by the
boot services table to detect timeout conditions. This
breaks the driver's ExitBootServices call back, as
CoreExitBootServices halts the timer before signaling
the ExitBootServices event. If the host controller
fails to halt in the call back, the timeout condition
will never occur and the boot gets stuck in an indefinite
spin loop. Use the free running timer provided by
TimerLib to calculate timeouts, avoiding the potential
hang.

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>

5 files changed, 204 insertions, 87 deletions

diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
index 62682dd..7a2e32a 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
@@ -1,6 +1,7 @@
 /** @file

   The XHCI controller driver.

 

+(C) Copyright 2023 Hewlett Packard Enterprise Development LP<BR>

 Copyright (c) 2011 - 2022, Intel Corporation. All rights reserved.<BR>

 SPDX-License-Identifier: BSD-2-Clause-Patent

 

@@ -85,6 +86,11 @@ EFI_USB2_HC_PROTOCOL  gXhciUsb2HcTemplate = {
   0x0

 };

 

+UINT64   mPerformanceCounterStartValue;

+UINT64   mPerformanceCounterEndValue;

+UINT64   mPerformanceCounterFrequency;

+BOOLEAN  mPerformanceCounterValuesCached = FALSE;

+

 /**

   Retrieves the capability of root hub ports.

 

@@ -2294,3 +2300,114 @@ XhcDriverBindingStop (
 

   return EFI_SUCCESS;

 }

+

+/**

+  Converts a time in nanoseconds to a performance counter tick count.

+

+  @param  Time  The time in nanoseconds to be converted to performance counter ticks.

+  @return Time in nanoseconds converted to ticks.

+**/

+UINT64

+XhcConvertTimeToTicks (

+  IN UINT64  Time

+  )

+{

+  UINT64  Ticks;

+  UINT64  Remainder;

+  UINT64  Divisor;

+  UINTN   Shift;

+

+  // Cache the return values to avoid repeated calls to GetPerformanceCounterProperties ()

+  if (!mPerformanceCounterValuesCached) {

+    mPerformanceCounterFrequency = GetPerformanceCounterProperties (

+                                     &mPerformanceCounterStartValue,

+                                     &mPerformanceCounterEndValue

+                                     );

+

+    mPerformanceCounterValuesCached = TRUE;

+  }

+

+  // Prevent returning a tick value of 0, unless Time is already 0

+  if (0 == mPerformanceCounterFrequency) {

+    return Time;

+  }

+

+  // Nanoseconds per second

+  Divisor = 1000000000;

+

+  //

+  //           Frequency

+  // Ticks = ------------- x Time

+  //         1,000,000,000

+  //

+  Ticks = MultU64x64 (

+            DivU64x64Remainder (

+              mPerformanceCounterFrequency,

+              Divisor,

+              &Remainder

+              ),

+            Time

+            );

+

+  //

+  // Ensure (Remainder * Time) will not overflow 64-bit.

+  //

+  // HighBitSet64 (Remainder) + 1 + HighBitSet64 (Time) + 1 <= 64

+  //

+  Shift     = MAX (0, HighBitSet64 (Remainder) + HighBitSet64 (Time) - 62);

+  Remainder = RShiftU64 (Remainder, (UINTN)Shift);

+  Divisor   = RShiftU64 (Divisor, (UINTN)Shift);

+  Ticks    += DivU64x64Remainder (MultU64x64 (Remainder, Time), Divisor, NULL);

+

+  return Ticks;

+}

+

+/**

+  Computes and returns the elapsed ticks since PreviousTick. The

+  value of PreviousTick is overwritten with the current performance

+  counter value.

+

+  @param  PreviousTick    Pointer to PreviousTick count.

+  @return The elapsed ticks since PreviousCount. PreviousCount is

+          overwritten with the current performance counter value.

+**/

+UINT64

+XhcGetElapsedTicks (

+  IN OUT UINT64  *PreviousTick

+  )

+{

+  UINT64  CurrentTick;

+  UINT64  Delta;

+

+  CurrentTick = GetPerformanceCounter ();

+

+  //

+  // Determine if the counter is counting up or down

+  //

+  if (mPerformanceCounterStartValue < mPerformanceCounterEndValue) {

+    //

+    // Counter counts upwards, check for an overflow condition

+    //

+    if (*PreviousTick > CurrentTick) {

+      Delta = (mPerformanceCounterEndValue - *PreviousTick) + CurrentTick;

+    } else {

+      Delta = CurrentTick - *PreviousTick;

+    }

+  } else {

+    //

+    // Counter counts downwards, check for an underflow condition

+    //

+    if (*PreviousTick < CurrentTick) {

+      Delta = (mPerformanceCounterStartValue - CurrentTick) + *PreviousTick;

+    } else {

+      Delta = *PreviousTick - CurrentTick;

+    }

+  }

+

+  //

+  // Set PreviousTick to CurrentTick

+  //

+  *PreviousTick = CurrentTick;

+

+  return Delta;

+}

diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h
index ca223bd..4401675 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h
@@ -2,6 +2,7 @@
 

   Provides some data structure definitions used by the XHCI host controller driver.

 

+(C) Copyright 2023 Hewlett Packard Enterprise Development LP<BR>

 Copyright (c) 2011 - 2017, Intel Corporation. All rights reserved.<BR>

 Copyright (c) Microsoft Corporation.<BR>

 SPDX-License-Identifier: BSD-2-Clause-Patent

@@ -26,6 +27,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Library/UefiLib.h>

 #include <Library/DebugLib.h>

 #include <Library/ReportStatusCodeLib.h>

+#include <Library/TimerLib.h>

 

 #include <IndustryStandard/Pci.h>

 

@@ -38,6 +40,11 @@ typedef struct _USB_DEV_CONTEXT    USB_DEV_CONTEXT;
 #include "UsbHcMem.h"

 

 //

+// Converts a count from microseconds to nanoseconds

+//

+#define XHC_MICROSECOND_TO_NANOSECOND(Time)  (MultU64x32((Time), 1000))

+

+//

 // The unit is microsecond, setting it as 1us.

 //

 #define XHC_1_MICROSECOND  (1)

@@ -720,4 +727,29 @@ XhcAsyncIsochronousTransfer (
   IN     VOID                                *Context

   );

 

+/**

+  Converts a time in nanoseconds to a performance counter tick count.

+

+  @param  Time  The time in nanoseconds to be converted to performance counter ticks.

+  @return Time in nanoseconds converted to ticks.

+**/

+UINT64

+XhcConvertTimeToTicks (

+  UINT64  Time

+  );

+

+/**

+  Computes and returns the elapsed ticks since PreviousTick. The

+  value of PreviousTick is overwritten with the current performance

+  counter value.

+

+  @param  PreviousTick    Pointer to PreviousTick count.

+  @return The elapsed ticks since PreviousCount. PreviousCount is

+          overwritten with the current performance counter value.

+**/

+UINT64

+XhcGetElapsedTicks (

+  IN OUT UINT64  *PreviousTick

+  );

+

 #endif

diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf b/MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf
index 5865d86..18ef879 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf
@@ -3,6 +3,7 @@
 #  It implements the interfaces of monitoring the status of all ports and transferring

 #  Control, Bulk, Interrupt and Isochronous requests to those attached usb LS/FS/HS/SS devices.

 #

+#  (C) Copyright 2023 Hewlett Packard Enterprise Development LP<BR>

 #  Copyright (c) 2011 - 2018, Intel Corporation. All rights reserved.<BR>

 #

 #  SPDX-License-Identifier: BSD-2-Clause-Patent

@@ -54,6 +55,7 @@
   BaseMemoryLib

   DebugLib

   ReportStatusCodeLib

+  TimerLib

 

 [Guids]

   gEfiEventExitBootServicesGuid                 ## SOMETIMES_CONSUMES ## Event

diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
index 5700fc5..40f2f1f 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
@@ -2,6 +2,7 @@
 

   The XHCI register operation routines.

 

+(C) Copyright 2023 Hewlett Packard Enterprise Development LP<BR>

 Copyright (c) 2011 - 2017, Intel Corporation. All rights reserved.<BR>

 SPDX-License-Identifier: BSD-2-Clause-Patent

 

@@ -417,15 +418,14 @@ 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_OUT_OF_RESOURCES   Memory for the timer event could not be allocated.

+  @retval EFI_SUCCESS  The bit successfully changed by host controller.

+  @retval EFI_TIMEOUT  The time out occurred.

 

 **/

 EFI_STATUS

@@ -437,54 +437,34 @@ XhcWaitOpRegBit (
   IN UINT32             Timeout

   )

 {

-  EFI_STATUS  Status;

-  EFI_EVENT   TimeoutEvent;

-

-  TimeoutEvent = NULL;

+  UINT64  TimeoutTicks;

+  UINT64  ElapsedTicks;

+  UINT64  TicksDelta;

+  UINT64  CurrentTick;

 

   if (Timeout == 0) {

     return EFI_TIMEOUT;

   }

 

-  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;

-  }

-

+  TimeoutTicks = XhcConvertTimeToTicks (XHC_MICROSECOND_TO_NANOSECOND (Timeout * XHC_1_MILLISECOND));

+  ElapsedTicks = 0;

+  CurrentTick  = GetPerformanceCounter ();

   do {

     if (XHC_REG_BIT_IS_SET (Xhc, Offset, Bit) == WaitToSet) {

-      Status = EFI_SUCCESS;

-      goto DONE;

+      return EFI_SUCCESS;

     }

 

     gBS->Stall (XHC_1_MICROSECOND);

-  } while (EFI_ERROR (gBS->CheckEvent (TimeoutEvent)));

-

-  Status = EFI_TIMEOUT;

+    TicksDelta = XhcGetElapsedTicks (&CurrentTick);

+    // Ensure that ElapsedTicks is always incremented to avoid indefinite hangs

+    if (TicksDelta == 0) {

+      TicksDelta = XhcConvertTimeToTicks (XHC_MICROSECOND_TO_NANOSECOND (XHC_1_MICROSECOND));

+    }

 

-DONE:

-  if (TimeoutEvent != NULL) {

-    gBS->CloseEvent (TimeoutEvent);

-  }

+    ElapsedTicks += TicksDelta;

+  } while (ElapsedTicks < TimeoutTicks);

 

-  return Status;

+  return EFI_TIMEOUT;

 }

 

 /**

diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
index 53421e6..613b148 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
@@ -2,6 +2,7 @@
 

   XHCI transfer scheduling routines.

 

+(C) Copyright 2023 Hewlett Packard Enterprise Development LP<BR>

 Copyright (c) 2011 - 2020, Intel Corporation. All rights reserved.<BR>

 Copyright (c) Microsoft Corporation.<BR>

 Copyright (C) 2022 Advanced Micro Devices, Inc. All rights reserved.<BR>

@@ -1276,15 +1277,14 @@ 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.

-  @retval EFI_OUT_OF_RESOURCES   Memory for the timer event could not be allocated.

+  @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.

 

 **/

 EFI_STATUS

@@ -1299,12 +1299,14 @@ XhcExecTransfer (
   UINT8       SlotId;

   UINT8       Dci;

   BOOLEAN     Finished;

-  EFI_EVENT   TimeoutEvent;

+  UINT64      TimeoutTicks;

+  UINT64      ElapsedTicks;

+  UINT64      TicksDelta;

+  UINT64      CurrentTick;

   BOOLEAN     IndefiniteTimeout;

 

   Status            = EFI_SUCCESS;

   Finished          = FALSE;

-  TimeoutEvent      = NULL;

   IndefiniteTimeout = FALSE;

 

   if (CmdTransfer) {

@@ -1322,34 +1324,18 @@ XhcExecTransfer (
 

   if (Timeout == 0) {

     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);

 

+  TimeoutTicks = XhcConvertTimeToTicks (

+                   XHC_MICROSECOND_TO_NANOSECOND (

+                     Timeout * XHC_1_MILLISECOND

+                     )

+                   );

+  ElapsedTicks = 0;

+  CurrentTick  = GetPerformanceCounter ();

+

   do {

     Finished = XhcCheckUrbResult (Xhc, Urb);

     if (Finished) {

@@ -1357,22 +1343,22 @@ RINGDOORBELL:
     }

 

     gBS->Stall (XHC_1_MICROSECOND);

-  } while (IndefiniteTimeout || EFI_ERROR (gBS->CheckEvent (TimeoutEvent)));

+    TicksDelta = XhcGetElapsedTicks (&CurrentTick);

+    // Ensure that ElapsedTicks is always incremented to avoid indefinite hangs

+    if (TicksDelta == 0) {

+      TicksDelta = XhcConvertTimeToTicks (XHC_MICROSECOND_TO_NANOSECOND (XHC_1_MICROSECOND));

+    }

 

-DONE:

-  if (EFI_ERROR (Status)) {

-    Urb->Result = EFI_USB_ERR_NOTEXECUTE;

-  } else if (!Finished) {

+    ElapsedTicks += TicksDelta;

+  } while (IndefiniteTimeout || ElapsedTicks < TimeoutTicks);

+

+  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;

 }