* [edk2-devel] [PATCH v3] MdeModulePkg/XhciDxe: Use Performance Timer for XHCI Timeouts
@ 2023-09-12 18:05 Henz, Patrick
2023-09-20 7:24 ` Wu, Hao A
0 siblings, 1 reply; 3+ messages in thread
From: Henz, Patrick @ 2023-09-12 18:05 UTC (permalink / raw)
To: devel; +Cc: hao.a.wu, ray.ni, Patrick Henz
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:
---
MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c | 117 +++++++++++++++++++++++
MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h | 32 +++++++
MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf | 2 +
MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c | 68 +++++--------
MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 72 ++++++--------
5 files changed, 204 insertions(+), 87 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
index 62682dd27c..7a2e32a9dd 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 ca223bd20c..4401675872 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>
@@ -37,6 +39,11 @@ typedef struct _USB_DEV_CONTEXT USB_DEV_CONTEXT;
#include "ComponentName.h"
#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.
//
@@ -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 5865d86822..18ef87916a 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 5700fc5fb8..40f2f1f227 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 53421e64a8..613b1485f1 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;
}
--
2.34.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108549): https://edk2.groups.io/g/devel/message/108549
Mute This Topic: https://groups.io/mt/101320913/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [edk2-devel] [PATCH v3] MdeModulePkg/XhciDxe: Use Performance Timer for XHCI Timeouts
2023-09-12 18:05 [edk2-devel] [PATCH v3] MdeModulePkg/XhciDxe: Use Performance Timer for XHCI Timeouts Henz, Patrick
@ 2023-09-20 7:24 ` Wu, Hao A
2023-09-25 2:58 ` Wu, Hao A
0 siblings, 1 reply; 3+ messages in thread
From: Wu, Hao A @ 2023-09-20 7:24 UTC (permalink / raw)
To: devel@edk2.groups.io, Henz, Patrick; +Cc: Ni, Ray
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
Best Regards,
Hao Wu
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Henz,
> Patrick
> Sent: Wednesday, September 13, 2023 2:06 AM
> To: devel@edk2.groups.io
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Henz,
> Patrick <patrick.henz@hpe.com>
> Subject: [edk2-devel] [PATCH v3] 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:
> ---
> MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c | 117
> +++++++++++++++++++++++
> MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h | 32 +++++++
> MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf | 2 +
> MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c | 68 +++++--------
> MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 72 ++++++--------
> 5 files changed, 204 insertions(+), 87 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> index 62682dd27c..7a2e32a9dd 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 ca223bd20c..4401675872 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>
>
>
>
> @@ -37,6 +39,11 @@ typedef struct _USB_DEV_CONTEXT
> USB_DEV_CONTEXT;
> #include "ComponentName.h"
>
> #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.
>
> //
>
> @@ -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 5865d86822..18ef87916a 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 5700fc5fb8..40f2f1f227 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 53421e64a8..613b1485f1 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;
>
> }
>
>
>
> --
> 2.34.1
>
>
>
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#108549):
> https://edk2.groups.io/g/devel/message/108549
> Mute This Topic: https://groups.io/mt/101320913/1768737
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [hao.a.wu@intel.com]
> -=-=-=-=-=-=
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108899): https://edk2.groups.io/g/devel/message/108899
Mute This Topic: https://groups.io/mt/101320913/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [edk2-devel] [PATCH v3] MdeModulePkg/XhciDxe: Use Performance Timer for XHCI Timeouts
2023-09-20 7:24 ` Wu, Hao A
@ 2023-09-25 2:58 ` Wu, Hao A
0 siblings, 0 replies; 3+ messages in thread
From: Wu, Hao A @ 2023-09-25 2:58 UTC (permalink / raw)
To: devel@edk2.groups.io, Wu, Hao A, Henz, Patrick; +Cc: Ni, Ray
Merged via:
PR - https://github.com/tianocore/edk2/pull/4862
Commit - https://github.com/tianocore/edk2/commit/43dcf453fc15ca152945ca41dcce7f2f43a14313
Best Regards,
Hao Wu
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wu, Hao
> A
> Sent: Wednesday, September 20, 2023 3:24 PM
> To: devel@edk2.groups.io; Henz, Patrick <patrick.henz@hpe.com>
> Cc: Ni, Ray <ray.ni@intel.com>
> Subject: Re: [edk2-devel] [PATCH v3] MdeModulePkg/XhciDxe: Use
> Performance Timer for XHCI Timeouts
>
> Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
>
> Best Regards,
> Hao Wu
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Henz,
> > Patrick
> > Sent: Wednesday, September 13, 2023 2:06 AM
> > To: devel@edk2.groups.io
> > Cc: Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Henz,
> > Patrick <patrick.henz@hpe.com>
> > Subject: [edk2-devel] [PATCH v3] 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:
> > ---
> > MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c | 117
> > +++++++++++++++++++++++
> > MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h | 32 +++++++
> > MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf | 2 +
> > MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c | 68 +++++--------
> > MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 72 ++++++--------
> > 5 files changed, 204 insertions(+), 87 deletions(-)
> >
> > diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> > b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> > index 62682dd27c..7a2e32a9dd 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 ca223bd20c..4401675872 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>
> >
> >
> >
> > @@ -37,6 +39,11 @@ typedef struct _USB_DEV_CONTEXT
> > USB_DEV_CONTEXT;
> > #include "ComponentName.h"
> >
> > #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.
> >
> > //
> >
> > @@ -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 5865d86822..18ef87916a 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 5700fc5fb8..40f2f1f227 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 53421e64a8..613b1485f1 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;
> >
> > }
> >
> >
> >
> > --
> > 2.34.1
> >
> >
> >
> > -=-=-=-=-=-=
> > Groups.io Links: You receive all messages sent to this group.
> > View/Reply Online (#108549):
> > https://edk2.groups.io/g/devel/message/108549
> > Mute This Topic: https://groups.io/mt/101320913/1768737
> > Group Owner: devel+owner@edk2.groups.io
> > Unsubscribe: https://edk2.groups.io/g/devel/unsub [hao.a.wu@intel.com]
> > -=-=-=-=-=-=
> >
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109030): https://edk2.groups.io/g/devel/message/109030
Mute This Topic: https://groups.io/mt/101320913/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-09-25 2:58 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-12 18:05 [edk2-devel] [PATCH v3] MdeModulePkg/XhciDxe: Use Performance Timer for XHCI Timeouts Henz, Patrick
2023-09-20 7:24 ` Wu, Hao A
2023-09-25 2:58 ` Wu, Hao A
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox