Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Use Performance Timer for XHCI Timeouts

["Henz, Patrick" <patrick.henz@hpe.com>]

That’s a great catch, thank you for bringing this to my attention Laszlo. Work is in progress.

Patrick Henz

-----Original Message-----
From: Laszlo Ersek <lersek@redhat.com> 
Sent: Tuesday, October 31, 2023 7:16 AM
To: devel@edk2.groups.io; hao.a.wu@intel.com; Henz, Patrick <patrick.henz@hpe.com>; Michael Brown <mcb30@ipxe.org>
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Use Performance Timer for XHCI Timeouts

On 10/31/23 02:41, Wu, Hao A wrote:
> (Add MdePkg maintainers for their feedbacks)
> 
> Sorry that I do not have strong opinion on this one.
> 
> Some of my thoughts are:
> * If you find the to-be-added APIs can be used in serveral places to reduce
>   repetative codes, then it will be worthwhile to add new library APIs.
> 
> * TimerLib has many instance implementations, my take is that many post-mem
>   instances use library level global variable to store information like timer
>   frequency to avoid calculating it every time.
>   For pre-mem instances, such caching is not feasible. I think it will be the
>   API consumer's responsibility to limit the usage of these APIs for performance
>   consideration.
> 
> Best Regards,
> Hao Wu

Patrick, while at it: I've recently filed the bug

  potentially invalid elapsed tick count calculation from commit 43dcf453fc15 

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

and assigned it to you (given that that commit is yours). I proposed the solution in the bugzilla too; please check it out if you can find the time.

Thanks
Laszlo

> 
>> -----Original Message-----
>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Henz, 
>> Patrick
>> Sent: Tuesday, October 31, 2023 4:36 AM
>> To: Wu, Hao A <hao.a.wu@intel.com>; devel@edk2.groups.io; Michael 
>> Brown <mcb30@ipxe.org>
>> Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Use 
>> Performance Timer for XHCI Timeouts
>>
>> My changes have been in for a little over a month now, I've been 
>> looking into updating the TimerLib API but I'm questioning if it 
>> still makes sense to go down that path.  The functions I had 
>> implemented, XhcGetElapsedTicks () and XhcConvertTimeToTicks (), now 
>> rely heavily on the use of global variables for caching return values 
>> from GetPerformanceCounterProperties. As-is these functions won't 
>> work for all instances of TimerLib, specifically if the instance is 
>> used before memory (RAM) is available. I could implement the 
>> functions as they originally were, but I wouldn't be able to replace 
>> the Xhc functions without adding additional parameters to said 
>> TimerLib functions (e.g. adding Frequency, StartValue, StopValue to 
>> ConvertTimeToTicks), which feels clunky to me. Would a new library 
>> make sense here? Something that sits between the driver and the TimerLib library? Or do we just leave things as they currently are?
>>
>> Thanks,
>> Patrick Henz
>>
>> -----Original Message-----
>> From: Wu, Hao A <hao.a.wu@intel.com>
>> Sent: Thursday, August 10, 2023 8:43 PM
>> To: Henz, Patrick <patrick.henz@hpe.com>; devel@edk2.groups.io; 
>> Michael Brown <mcb30@ipxe.org>
>> Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Use 
>> Performance Timer for XHCI Timeouts
>>
>> My personal preference is to do this as two steps.
>> Address the issue in XhciDxe first. Then add new TimerLib API to 
>> replace all driver/library internal implementations.
>>
>> Best Regards,
>> Hao Wu
>>
>>> -----Original Message-----
>>> From: Henz, Patrick <patrick.henz@hpe.com>
>>> Sent: Friday, August 11, 2023 6:45 AM
>>> To: devel@edk2.groups.io; Wu, Hao A <hao.a.wu@intel.com>; Michael 
>>> Brown <mcb30@ipxe.org>
>>> Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Use 
>>> Performance Timer for XHCI Timeouts
>>>
>>> I can certainly make that change.
>>>
>>> For what it's worth I have been working on adding the new function, 
>>> GetElapsedTicks, to the various implementations of TimerLib. I've 
>>> finished up testing, I would just need to clean up the commits for a 
>>> patch. Should I move forward with that, or would we rather I just 
>>> add XhcGetElapsedTime to XhciDxe for the time being?
>>>
>>> Thanks,
>>> Patrick Henz
>>>
>>> -----Original Message-----
>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wu,
>> Hao
>>> A
>>> Sent: Sunday, July 30, 2023 9:57 PM
>>> To: devel@edk2.groups.io; Henz, Patrick <patrick.henz@hpe.com>; 
>>> Michael Brown <mcb30@ipxe.org>
>>> Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Use 
>>> Performance Timer for XHCI Timeouts
>>>
>>> For the 2 occurrences of in XhcWaitOpRegBit & XhcExecTransfer:
>>>   TimeoutTime = XHC_MICROSECOND_TO_NANOSECOND (Timeout * 
>>> XHC_1_MILLISECOND); How about changing them to:
>>>   TimeoutTime = XHC_MICROSECOND_TO_NANOSECOND ((UINT64)
>> Timeout
>>> * XHC_1_MILLISECOND); to address possible overflow during "Timeout * 
>>> XHC_1_MILLISECOND"?
>>>
>>> For extending XhcGetElapsedTime as a TimerLib API, I am fine to put 
>>> it in XhciDxe at this moment.
>>> If package maintainers suggest to make it as a public library API, 
>>> my take is that this should be done in a separate commit.
>>>
>>> Best Regards,
>>> Hao Wu
>>>
>>>> -----Original Message-----
>>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of 
>>>> Henz, Patrick
>>>> Sent: Thursday, July 6, 2023 4:16 AM
>>>> To: devel@edk2.groups.io
>>>> Cc: Henz, Patrick <patrick.henz@hpe.com>
>>>> Subject: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Use Performance 
>>>> Timer for XHCI Timeouts
>>>>
>>>> REF:INVALID URI REMOVED
>>>> bu
>>>> g.cgi?id=2948__;!!NpxR!kyFQM5IkKYAG9CRBO4xphwBnzi_jhb2oU-
>>> NKbMjOV-lctg5
>>>> _B3K1Lcta452Gx-1twRt8At3cueAYDq_n$
>>>>
>>>> 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.
>>>>
>>>> Signed-off-by: Patrick Henz <patrick.henz@hpe.com>
>>>> Reviewed-by:
>>>> ---
>>>>  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c      | 56 +++++++++++++++++++
>>>>  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h      | 22 ++++++++
>>>>  MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf |  2 +
>>>>  MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c   | 67 +++++++++--------------
>>>>  MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 68
>>>> +++++++++---------------
>>>>  5 files changed, 129 insertions(+), 86 deletions(-)
>>>>
>>>> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
>>>> b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
>>>> index 62682dd27c..1dcbe20512 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
>>>>
>>>>
>>>>
>>>> @@ -2294,3 +2295,58 @@ XhcDriverBindingStop (
>>>>
>>>>
>>>>    return EFI_SUCCESS;
>>>>
>>>>  }
>>>>
>>>> +
>>>>
>>>> +/**
>>>>
>>>> +  Computes and returns the elapsed time in nanoseconds since
>>>>
>>>> +  PreviousTick. The value of PreviousTick is overwritten with the
>>>>
>>>> +  current performance counter value.
>>>>
>>>> +
>>>>
>>>> +  @param  PreviousTick    Pointer to PreviousTick count.
>>>>
>>>> +  @return The elapsed time in nanoseconds since PreviousCount.
>>>>
>>>> +          PreviousCount is overwritten with the current 
>>>> + performance
>>>>
>>>> +          counter value.
>>>>
>>>> +**/
>>>>
>>>> +UINT64
>>>>
>>>> +XhcGetElapsedTime (
>>>>
>>>> +  IN OUT UINT64  *PreviousTick
>>>>
>>>> +  )
>>>>
>>>> +{
>>>>
>>>> +  UINT64  StartValue;
>>>>
>>>> +  UINT64  EndValue;
>>>>
>>>> +  UINT64  CurrentTick;
>>>>
>>>> +  UINT64  Delta;
>>>>
>>>> +
>>>>
>>>> +  GetPerformanceCounterProperties (&StartValue, &EndValue);
>>>>
>>>> +
>>>>
>>>> +  CurrentTick = GetPerformanceCounter ();
>>>>
>>>> +
>>>>
>>>> +  //
>>>>
>>>> +  // Determine if the counter is counting up or down
>>>>
>>>> +  //
>>>>
>>>> +  if (StartValue < EndValue) {
>>>>
>>>> +    //
>>>>
>>>> +    // Counter counts upwards, check for an overflow condition
>>>>
>>>> +    //
>>>>
>>>> +    if (*PreviousTick > CurrentTick) {
>>>>
>>>> +      Delta = (EndValue - *PreviousTick) + CurrentTick;
>>>>
>>>> +    } else {
>>>>
>>>> +      Delta = CurrentTick - *PreviousTick;
>>>>
>>>> +    }
>>>>
>>>> +  } else {
>>>>
>>>> +    //
>>>>
>>>> +    // Counter counts downwards, check for an underflow condition
>>>>
>>>> +    //
>>>>
>>>> +    if (*PreviousTick < CurrentTick) {
>>>>
>>>> +      Delta = (StartValue - CurrentTick) + *PreviousTick;
>>>>
>>>> +    } else {
>>>>
>>>> +      Delta = *PreviousTick - CurrentTick;
>>>>
>>>> +    }
>>>>
>>>> +  }
>>>>
>>>> +
>>>>
>>>> +  //
>>>>
>>>> +  // Set PreviousTick to CurrentTick
>>>>
>>>> +  //
>>>>
>>>> +  *PreviousTick = CurrentTick;
>>>>
>>>> +
>>>>
>>>> +  return GetTimeInNanoSecond (Delta);
>>>>
>>>> +}
>>>>
>>>> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h
>>>> b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h
>>>> index ca223bd20c..77feb66647 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,10 @@ 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)  ((UINT64)(Time)
>> *
>>>> 1000)
>>>>
>>>>  //
>>>>
>>>>  // The unit is microsecond, setting it as 1us.
>>>>
>>>>  //
>>>>
>>>> @@ -720,4 +726,20 @@ XhcAsyncIsochronousTransfer (
>>>>    IN     VOID                                *Context
>>>>
>>>>    );
>>>>
>>>>
>>>>
>>>> +/**
>>>>
>>>> +  Computes and returns the elapsed time in nanoseconds since
>>>>
>>>> +  PreviousTick. The value of PreviousTick is overwritten with the
>>>>
>>>> +  current performance counter value.
>>>>
>>>> +
>>>>
>>>> +  @param  PreviousTick    Pointer to PreviousTick count.
>>>>
>>>> +                          before calling this function.
>>>>
>>>> +  @return The elapsed time in nanoseconds since PreviousCount.
>>>>
>>>> +          PreviousCount is overwritten with the current 
>>>> + performance
>>>>
>>>> +          counter value.
>>>>
>>>> +**/
>>>>
>>>> +UINT64
>>>>
>>>> +XhcGetElapsedTime (
>>>>
>>>> +  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..2499698715 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,35 @@ XhcWaitOpRegBit (
>>>>    IN UINT32             Timeout
>>>>
>>>>    )
>>>>
>>>>  {
>>>>
>>>> -  EFI_STATUS  Status;
>>>>
>>>> -  EFI_EVENT   TimeoutEvent;
>>>>
>>>> -
>>>>
>>>> -  TimeoutEvent = NULL;
>>>>
>>>> +  UINT64  TimeoutTime;
>>>>
>>>> +  UINT64  ElapsedTime;
>>>>
>>>> +  UINT64  TimeDelta;
>>>>
>>>> +  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;
>>>>
>>>> -  }
>>>>
>>>> +  TimeoutTime = XHC_MICROSECOND_TO_NANOSECOND (Timeout *
>>>> XHC_1_MILLISECOND);
>>>>
>>>> +  ElapsedTime = 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;
>>>>
>>>> +    TimeDelta = XhcGetElapsedTime (&CurrentTick);
>>>>
>>>> +    // Ensure that ElapsedTime is always incremented to avoid 
>>>> + indefinite
>>>> hangs
>>>>
>>>> +    if (TimeDelta == 0) {
>>>>
>>>> +      TimeDelta = XHC_MICROSECOND_TO_NANOSECOND
>>>> (XHC_1_MICROSECOND);
>>>>
>>>> +    }
>>>>
>>>>
>>>>
>>>> -DONE:
>>>>
>>>> -  if (TimeoutEvent != NULL) {
>>>>
>>>> -    gBS->CloseEvent (TimeoutEvent);
>>>>
>>>> -  }
>>>>
>>>> +    ElapsedTime += TimeDelta;
>>>>
>>>> +  } while (ElapsedTime < TimeoutTime);
>>>>
>>>>
>>>>
>>>> -  return Status;
>>>>
>>>> +  return EFI_TIMEOUT;
>>>>
>>>>  }
>>>>
>>>>
>>>>
>>>>  /**
>>>>
>>>> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
>>>> b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
>>>> index 298fb88b81..5177886e52 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>
>>>>
>>>> @@ -1273,15 +1274,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
>>>>
>>>> @@ -1296,12 +1296,14 @@ XhcExecTransfer (
>>>>    UINT8       SlotId;
>>>>
>>>>    UINT8       Dci;
>>>>
>>>>    BOOLEAN     Finished;
>>>>
>>>> -  EFI_EVENT   TimeoutEvent;
>>>>
>>>> +  UINT64      TimeoutTime;
>>>>
>>>> +  UINT64      ElapsedTime;
>>>>
>>>> +  UINT64      TimeDelta;
>>>>
>>>> +  UINT64      CurrentTick;
>>>>
>>>>    BOOLEAN     IndefiniteTimeout;
>>>>
>>>>
>>>>
>>>>    Status            = EFI_SUCCESS;
>>>>
>>>>    Finished          = FALSE;
>>>>
>>>> -  TimeoutEvent      = NULL;
>>>>
>>>>    IndefiniteTimeout = FALSE;
>>>>
>>>>
>>>>
>>>>    if (CmdTransfer) {
>>>>
>>>> @@ -1319,34 +1321,14 @@ 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);
>>>>
>>>>
>>>>
>>>> +  TimeoutTime = XHC_MICROSECOND_TO_NANOSECOND (Timeout *
>>>> XHC_1_MILLISECOND);
>>>>
>>>> +  ElapsedTime = 0;
>>>>
>>>> +  CurrentTick = GetPerformanceCounter ();
>>>>
>>>> +
>>>>
>>>>    do {
>>>>
>>>>      Finished = XhcCheckUrbResult (Xhc, Urb);
>>>>
>>>>      if (Finished) {
>>>>
>>>> @@ -1354,22 +1336,22 @@ RINGDOORBELL:
>>>>      }
>>>>
>>>>
>>>>
>>>>      gBS->Stall (XHC_1_MICROSECOND);
>>>>
>>>> -  } while (IndefiniteTimeout || EFI_ERROR (gBS->CheckEvent 
>>>> (TimeoutEvent)));
>>>>
>>>> +    TimeDelta = XhcGetElapsedTime (&CurrentTick);
>>>>
>>>> +    // Ensure that ElapsedTime is always incremented to avoid 
>>>> + indefinite
>>>> hangs
>>>>
>>>> +    if (TimeDelta == 0) {
>>>>
>>>> +      TimeDelta = XHC_MICROSECOND_TO_NANOSECOND
>>>> (XHC_1_MICROSECOND);
>>>>
>>>> +    }
>>>>
>>>>
>>>>
>>>> -DONE:
>>>>
>>>> -  if (EFI_ERROR (Status)) {
>>>>
>>>> -    Urb->Result = EFI_USB_ERR_NOTEXECUTE;
>>>>
>>>> -  } else if (!Finished) {
>>>>
>>>> +    ElapsedTime += TimeDelta;
>>>>
>>>> +  } while (IndefiniteTimeout || ElapsedTime < TimeoutTime);
>>>>
>>>> +
>>>>
>>>> +  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 (#106668):
>>>> INVALID URI REMOVED
>>>> 06
>>>> 668__;!!NpxR!kyFQM5IkKYAG9CRBO4xphwBnzi_jhb2oU-NKbMjOV-
>>> lctg5_B3K1Lcta4
>>>> 52Gx-1twRt8At3cuWxQsO18$ Mute This Topic:
>>>> https://groups.io/mt/99972791/1768737
>>>> NpxR!kyFQM5IkKYAG9CRBO4xphwBnzi_jhb2oU-NKbMjOV-
>>> lctg5_B3K1Lcta452Gx-1tw
>>>> Rt8At3cubh2HDyk$ Group Owner: devel+owner@edk2.groups.io
>>>> Unsubscribe:
>>>> https://edk2.groups.io/g/devel/unsub
>>>> !N
>>>> pxR!kyFQM5IkKYAG9CRBO4xphwBnzi_jhb2oU-NKbMjOV-
>>> lctg5_B3K1Lcta452Gx-1twR
>>>> t8At3cuWYMMXVU$  [hao.a.wu@intel.com] -=-=-=-=-=-=
>>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>
>>
>>
>>
>>
> 
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110411): https://edk2.groups.io/g/devel/message/110411
Mute This Topic: https://groups.io/mt/99972791/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-