From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id D5AD8740032 for ; Tue, 31 Oct 2023 12:14:46 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=sw03ji84kfLdh6RfNl53W83uH8j6eLtzNogQ0J0H0x8=; c=relaxed/simple; d=groups.io; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From:In-Reply-To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type:Content-Transfer-Encoding; s=20140610; t=1698754485; v=1; b=E7lQV7Wpgcr+7rufjUAwJ1V5f1q+YLol5TQgzY2QJVjsotPtqbQ+tgCWsvRpBlwv+DjKEVW/ 2NjZTXWz/djxHE50QU5IHSg+a/AkH6z5Ru81XG0x1HR3tcJQ2VskDYoHh9Z4vg1aDQsk8gTfnhM OchSbzhGL043cQZyBSi5wY8s= X-Received: by 127.0.0.2 with SMTP id prmwYY7687511x8NwEIqTt3G; Tue, 31 Oct 2023 05:14:45 -0700 X-Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mx.groups.io with SMTP id smtpd.web10.184389.1698754484682711809 for ; Tue, 31 Oct 2023 05:14:45 -0700 X-Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-648-FjmaJcXNP2CggnEV6kHaKA-1; Tue, 31 Oct 2023 08:14:35 -0400 X-MC-Unique: FjmaJcXNP2CggnEV6kHaKA-1 X-Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.rdu2.redhat.com [10.11.54.2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 2CF9680F92D; Tue, 31 Oct 2023 12:14:35 +0000 (UTC) X-Received: from [10.39.195.34] (unknown [10.39.195.34]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 32F2040C6EB9; Tue, 31 Oct 2023 12:14:33 +0000 (UTC) Message-ID: Date: Tue, 31 Oct 2023 13:14:31 +0100 MIME-Version: 1.0 Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Use Performance Timer for XHCI Timeouts To: devel@edk2.groups.io, hao.a.wu@intel.com, "Henz, Patrick" , Michael Brown Cc: "Kinney, Michael D" , "Gao, Liming" References: From: "Laszlo Ersek" In-Reply-To: X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.2 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,lersek@redhat.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: AeQ6ZChWEMQiOdQXOwCjHDAYx7686176AA= Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=E7lQV7Wp; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=redhat.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io On 10/31/23 02:41, Wu, Hao A wrote: > (Add MdePkg maintainers for their feedbacks) >=20 > Sorry that I do not have strong opinion on this one. >=20 > Some of my thoughts are: > * If you find the to-be-added APIs can be used in serveral places to redu= ce > repetative codes, then it will be worthwhile to add new library APIs. >=20 > * TimerLib has many instance implementations, my take is that many post-m= em > instances use library level global variable to store information like t= imer > 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 perf= ormance > consideration. I wouldn't recommend extending the TimerLib class. TimerLib has a huge amount of instances (implementations), and adding the (effectively) same calculation to each is a waste of developer effort, reviewer effort, and long-term maintenance (lots of duplications). Introducing a separate TimerTickDiffLib class is what I proposed in 2017. It can be small and simple; do one thing and do it well. It would be widely used (*many* sites need tick differences). Yet keeping it separate from TimerLib is absolutely justified, as TimerTickDiffLib would be independent of both timer hardware specifics *and* of firmware phase too (RAM or no RAM). The functions -- or more likely, the single function -- in TimerTickDiffLib can easily take the performance counter characteristics as parameters. Then callers in RAM-based modules may pass those args from global variables in which they cache the perf counter characteristics themselves. No need to move the caching into a particular (RAM-bound) TimerTickDiffLib instance. And callers that XIP from flash can fill in those args from repeated / separate GetPerformanceCounterProperties() calls, or -- in the luckier case -- cache the output of GetPerformanceCounterProperties() for each *call site*, in local variables. A futher API optimization may be to introduce a new (small) structure type in TimerTickDiffLib, for collecting the outputs of GetPerformanceCounterProperties(). Then the sole function in TimerTickDiffLib would take a pointer to a *constant* structure of this type. Thanks Laszlo >=20 > Best Regards, > Hao Wu >=20 >> -----Original Message----- >> From: devel@edk2.groups.io On Behalf Of Henz, >> Patrick >> Sent: Tuesday, October 31, 2023 4:36 AM >> To: Wu, Hao A ; devel@edk2.groups.io; Michael Brown >> >> 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 variable= s 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 a= s >> they originally were, but I wouldn't be able to replace the Xhc function= s >> without adding additional parameters to said TimerLib functions (e.g. ad= ding >> Frequency, StartValue, StopValue to ConvertTimeToTicks), which feels clu= nky >> 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 >> Sent: Thursday, August 10, 2023 8:43 PM >> To: Henz, Patrick ; devel@edk2.groups.io; Michael >> Brown >> 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 >>> Sent: Friday, August 11, 2023 6:45 AM >>> To: devel@edk2.groups.io; Wu, Hao A ; Michael >>> Brown >>> 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 On Behalf Of Wu, >> Hao >>> A >>> Sent: Sunday, July 30, 2023 9:57 PM >>> To: devel@edk2.groups.io; Henz, Patrick ; >>> Michael Brown >>> Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Use >>> Performance Timer for XHCI Timeouts >>> >>> For the 2 occurrences of in XhcWaitOpRegBit & XhcExecTransfer: >>> TimeoutTime =3D XHC_MICROSECOND_TO_NANOSECOND (Timeout * >>> XHC_1_MILLISECOND); How about changing them to: >>> TimeoutTime =3D 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 On Behalf Of Henz, >>>> Patrick >>>> Sent: Thursday, July 6, 2023 4:16 AM >>>> To: devel@edk2.groups.io >>>> Cc: Henz, Patrick >>>> Subject: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Use Performance >>>> Timer for XHCI Timeouts >>>> >>>> REF:INVALID URI REMOVED >>>> bu >>>> g.cgi?id=3D2948__;!!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 >>>> 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
>>>> >>>> Copyright (c) 2011 - 2022, Intel Corporation. All rights >>>> reserved.
>>>> >>>> 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 =3D 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 =3D (EndValue - *PreviousTick) + CurrentTick; >>>> >>>> + } else { >>>> >>>> + Delta =3D CurrentTick - *PreviousTick; >>>> >>>> + } >>>> >>>> + } else { >>>> >>>> + // >>>> >>>> + // Counter counts downwards, check for an underflow condition >>>> >>>> + // >>>> >>>> + if (*PreviousTick < CurrentTick) { >>>> >>>> + Delta =3D (StartValue - CurrentTick) + *PreviousTick; >>>> >>>> + } else { >>>> >>>> + Delta =3D *PreviousTick - CurrentTick; >>>> >>>> + } >>>> >>>> + } >>>> >>>> + >>>> >>>> + // >>>> >>>> + // Set PreviousTick to CurrentTick >>>> >>>> + // >>>> >>>> + *PreviousTick =3D 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
>>>> >>>> Copyright (c) 2011 - 2017, Intel Corporation. All rights >>>> reserved.
>>>> >>>> Copyright (c) Microsoft Corporation.
>>>> >>>> SPDX-License-Identifier: BSD-2-Clause-Patent >>>> >>>> @@ -26,6 +27,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent >>>> #include >>>> >>>> #include >>>> >>>> #include >>>> >>>> +#include >>>> >>>> >>>> >>>> #include >>>> >>>> >>>> >>>> @@ -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
>>>> >>>> # Copyright (c) 2011 - 2018, Intel Corporation. All rights >>>> reserved.
>>>> >>>> # >>>> >>>> # 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
>>>> >>>> Copyright (c) 2011 - 2017, Intel Corporation. All rights >>>> reserved.
>>>> >>>> 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 mi= llisecond, >>>> 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 controlle= r. >>>> >>>> + @retval EFI_TIMEOUT The time out occurred. >>>> >>>> >>>> >>>> **/ >>>> >>>> EFI_STATUS >>>> >>>> @@ -437,54 +437,35 @@ XhcWaitOpRegBit ( >>>> IN UINT32 Timeout >>>> >>>> ) >>>> >>>> { >>>> >>>> - EFI_STATUS Status; >>>> >>>> - EFI_EVENT TimeoutEvent; >>>> >>>> - >>>> >>>> - TimeoutEvent =3D NULL; >>>> >>>> + UINT64 TimeoutTime; >>>> >>>> + UINT64 ElapsedTime; >>>> >>>> + UINT64 TimeDelta; >>>> >>>> + UINT64 CurrentTick; >>>> >>>> >>>> >>>> if (Timeout =3D=3D 0) { >>>> >>>> return EFI_TIMEOUT; >>>> >>>> } >>>> >>>> >>>> >>>> - Status =3D gBS->CreateEvent ( >>>> >>>> - EVT_TIMER, >>>> >>>> - TPL_CALLBACK, >>>> >>>> - NULL, >>>> >>>> - NULL, >>>> >>>> - &TimeoutEvent >>>> >>>> - ); >>>> >>>> - >>>> >>>> - if (EFI_ERROR (Status)) { >>>> >>>> - goto DONE; >>>> >>>> - } >>>> >>>> - >>>> >>>> - Status =3D gBS->SetTimer ( >>>> >>>> - TimeoutEvent, >>>> >>>> - TimerRelative, >>>> >>>> - EFI_TIMER_PERIOD_MILLISECONDS (Timeout) >>>> >>>> - ); >>>> >>>> - >>>> >>>> - if (EFI_ERROR (Status)) { >>>> >>>> - goto DONE; >>>> >>>> - } >>>> >>>> + TimeoutTime =3D XHC_MICROSECOND_TO_NANOSECOND (Timeout * >>>> XHC_1_MILLISECOND); >>>> >>>> + ElapsedTime =3D 0; >>>> >>>> + CurrentTick =3D GetPerformanceCounter (); >>>> >>>> >>>> >>>> do { >>>> >>>> if (XHC_REG_BIT_IS_SET (Xhc, Offset, Bit) =3D=3D WaitToSet) { >>>> >>>> - Status =3D EFI_SUCCESS; >>>> >>>> - goto DONE; >>>> >>>> + return EFI_SUCCESS; >>>> >>>> } >>>> >>>> >>>> >>>> gBS->Stall (XHC_1_MICROSECOND); >>>> >>>> - } while (EFI_ERROR (gBS->CheckEvent (TimeoutEvent))); >>>> >>>> - >>>> >>>> - Status =3D EFI_TIMEOUT; >>>> >>>> + TimeDelta =3D XhcGetElapsedTime (&CurrentTick); >>>> >>>> + // Ensure that ElapsedTime is always incremented to avoid >>>> + indefinite >>>> hangs >>>> >>>> + if (TimeDelta =3D=3D 0) { >>>> >>>> + TimeDelta =3D XHC_MICROSECOND_TO_NANOSECOND >>>> (XHC_1_MICROSECOND); >>>> >>>> + } >>>> >>>> >>>> >>>> -DONE: >>>> >>>> - if (TimeoutEvent !=3D NULL) { >>>> >>>> - gBS->CloseEvent (TimeoutEvent); >>>> >>>> - } >>>> >>>> + ElapsedTime +=3D 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
>>>> >>>> Copyright (c) 2011 - 2020, Intel Corporation. All rights >>>> reserved.
>>>> >>>> Copyright (c) Microsoft Corporation.
>>>> >>>> Copyright (C) 2022 Advanced Micro Devices, Inc. All rights >>>> reserved.
>>>> >>>> @@ -1273,15 +1274,14 @@ EXIT: >>>> /** >>>> >>>> Execute the transfer by polling the URB. This is a synchronous oper= ation. >>>> >>>> >>>> >>>> - @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 mi= llisecond. >>>> >>>> + @param Xhc The XHCI Instance. >>>> >>>> + @param CmdTransfer The executed URB is for cmd transfer or n= ot. >>>> >>>> + @param Urb The URB to execute. >>>> >>>> + @param Timeout The time to wait before abort, in millise= cond. >>>> >>>> >>>> >>>> - @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 =3D EFI_SUCCESS; >>>> >>>> Finished =3D FALSE; >>>> >>>> - TimeoutEvent =3D NULL; >>>> >>>> IndefiniteTimeout =3D FALSE; >>>> >>>> >>>> >>>> if (CmdTransfer) { >>>> >>>> @@ -1319,34 +1321,14 @@ XhcExecTransfer ( >>>> >>>> >>>> if (Timeout =3D=3D 0) { >>>> >>>> IndefiniteTimeout =3D TRUE; >>>> >>>> - goto RINGDOORBELL; >>>> >>>> - } >>>> >>>> - >>>> >>>> - Status =3D gBS->CreateEvent ( >>>> >>>> - EVT_TIMER, >>>> >>>> - TPL_CALLBACK, >>>> >>>> - NULL, >>>> >>>> - NULL, >>>> >>>> - &TimeoutEvent >>>> >>>> - ); >>>> >>>> - >>>> >>>> - if (EFI_ERROR (Status)) { >>>> >>>> - goto DONE; >>>> >>>> } >>>> >>>> >>>> >>>> - Status =3D gBS->SetTimer ( >>>> >>>> - TimeoutEvent, >>>> >>>> - TimerRelative, >>>> >>>> - EFI_TIMER_PERIOD_MILLISECONDS (Timeout) >>>> >>>> - ); >>>> >>>> - >>>> >>>> - if (EFI_ERROR (Status)) { >>>> >>>> - goto DONE; >>>> >>>> - } >>>> >>>> - >>>> >>>> -RINGDOORBELL: >>>> >>>> XhcRingDoorBell (Xhc, SlotId, Dci); >>>> >>>> >>>> >>>> + TimeoutTime =3D XHC_MICROSECOND_TO_NANOSECOND (Timeout * >>>> XHC_1_MILLISECOND); >>>> >>>> + ElapsedTime =3D 0; >>>> >>>> + CurrentTick =3D GetPerformanceCounter (); >>>> >>>> + >>>> >>>> do { >>>> >>>> Finished =3D XhcCheckUrbResult (Xhc, Urb); >>>> >>>> if (Finished) { >>>> >>>> @@ -1354,22 +1336,22 @@ RINGDOORBELL: >>>> } >>>> >>>> >>>> >>>> gBS->Stall (XHC_1_MICROSECOND); >>>> >>>> - } while (IndefiniteTimeout || EFI_ERROR (gBS->CheckEvent >>>> (TimeoutEvent))); >>>> >>>> + TimeDelta =3D XhcGetElapsedTime (&CurrentTick); >>>> >>>> + // Ensure that ElapsedTime is always incremented to avoid >>>> + indefinite >>>> hangs >>>> >>>> + if (TimeDelta =3D=3D 0) { >>>> >>>> + TimeDelta =3D XHC_MICROSECOND_TO_NANOSECOND >>>> (XHC_1_MICROSECOND); >>>> >>>> + } >>>> >>>> >>>> >>>> -DONE: >>>> >>>> - if (EFI_ERROR (Status)) { >>>> >>>> - Urb->Result =3D EFI_USB_ERR_NOTEXECUTE; >>>> >>>> - } else if (!Finished) { >>>> >>>> + ElapsedTime +=3D TimeDelta; >>>> >>>> + } while (IndefiniteTimeout || ElapsedTime < TimeoutTime); >>>> >>>> + >>>> >>>> + if (!Finished) { >>>> >>>> Urb->Result =3D EFI_USB_ERR_TIMEOUT; >>>> >>>> Status =3D EFI_TIMEOUT; >>>> >>>> } else if (Urb->Result !=3D EFI_USB_NOERROR) { >>>> >>>> Status =3D EFI_DEVICE_ERROR; >>>> >>>> } >>>> >>>> >>>> >>>> - if (TimeoutEvent !=3D NULL) { >>>> >>>> - gBS->CloseEvent (TimeoutEvent); >>>> >>>> - } >>>> >>>> - >>>> >>>> return Status; >>>> >>>> } >>>> >>>> >>>> >>>> -- >>>> 2.34.1 >>>> >>>> >>>> >>>> -=3D-=3D-=3D-=3D-=3D-=3D >>>> 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] -=3D-=3D-=3D-=3D-=3D-=3D >>>> >>> >>> >>> >>> >>> >>> >> >> >> >> >> >=20 >=20 >=20 >=20 >=20 >=20 -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#110403): https://edk2.groups.io/g/devel/message/110403 Mute This Topic: https://groups.io/mt/99972791/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/19134562= 12/xyzzy [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-