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 19091D80C7F for ; Mon, 14 Aug 2023 15:52:32 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=ssgYDgXALenhg7ebyZE9y0z2Pv4Oa6lLqjKaA9tjYa0=; c=relaxed/simple; d=groups.io; h=From:To:Cc:Subject:Date:Message-Id:MIME-Version:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Transfer-Encoding; s=20140610; t=1692028351; v=1; b=lBR6aER5JJ3WZyRmLZL4AYqZek6wAxCkvsqJHk4l7vBnU5txtpWI3fSXnNIL4CrWGhySoyF+ XDaw/Lt5czTK7SyvqkXjbRJq7w+qhus7EOzVVl7eP345nAtEFy1nn3cHVaCyWFL9pNVpxERjb1A PQLraxZ/yzM0mMVsK8qe/ebE= X-Received: by 127.0.0.2 with SMTP id zoDLYY7687511xqyW3CrKkQe; Mon, 14 Aug 2023 08:52:31 -0700 X-Received: from mx0b-002e3701.pphosted.com (mx0b-002e3701.pphosted.com [148.163.143.35]) by mx.groups.io with SMTP id smtpd.web11.111047.1692028330180748811 for ; Mon, 14 Aug 2023 08:52:30 -0700 X-Received: from pps.filterd (m0134423.ppops.net [127.0.0.1]) by mx0b-002e3701.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 37E872jA031362 for ; Mon, 14 Aug 2023 15:51:59 GMT X-Received: from p1lg14880.it.hpe.com ([16.230.97.201]) by mx0b-002e3701.pphosted.com (PPS) with ESMTPS id 3sfgfpuxfb-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Mon, 14 Aug 2023 15:51:59 +0000 X-Received: from p1lg14886.dc01.its.hpecorp.net (unknown [10.119.18.237]) (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 p1lg14880.it.hpe.com (Postfix) with ESMTPS id 4319D800189 for ; Mon, 14 Aug 2023 15:51:58 +0000 (UTC) X-Received: from cat.eag.rdlabs.hpecorp.net (unknown [16.231.227.36]) by p1lg14886.dc01.its.hpecorp.net (Postfix) with ESMTP id ED9EC80ADB3; Mon, 14 Aug 2023 15:51:57 +0000 (UTC) X-Received: by cat.eag.rdlabs.hpecorp.net (Postfix, from userid 178665) id A9B9EB48A3; Mon, 14 Aug 2023 10:51:56 -0500 (CDT) From: "Henz, Patrick" To: devel@edk2.groups.io Cc: Patrick Henz Subject: [edk2-devel] [PATCH v2] MdeModulePkg/XhciDxe: Use Performance Timer for XHCI Timeouts Date: Mon, 14 Aug 2023 10:51:42 -0500 Message-Id: <08d6a4801dd0b0d6eeb5786ce66504f2b478fd4f.1692023941.git.patrick.henz@hpe.com> X-Proofpoint-ORIG-GUID: RsCPiz5b3IxCI6gjUJLNKqxBahCzn1kG X-Proofpoint-GUID: RsCPiz5b3IxCI6gjUJLNKqxBahCzn1kG X-Proofpoint-UnRewURL: 0 URL was un-rewritten MIME-Version: 1.0 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,patrick.henz@hpe.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: COn1tkG6WezIUDHasT6dgXYcx7686176AA= 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=lBR6aER5; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=hpe.com (policy=none) REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3D2948 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/Xhc= iDxe/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=0D The XHCI controller driver.=0D =0D +(C) Copyright 2023 Hewlett Packard Enterprise Development LP
=0D Copyright (c) 2011 - 2022, Intel Corporation. All rights reserved.
=0D SPDX-License-Identifier: BSD-2-Clause-Patent=0D =0D @@ -2294,3 +2295,58 @@ XhcDriverBindingStop ( =0D return EFI_SUCCESS;=0D }=0D +=0D +/**=0D + Computes and returns the elapsed time in nanoseconds since=0D + PreviousTick. The value of PreviousTick is overwritten with the=0D + current performance counter value.=0D +=0D + @param PreviousTick Pointer to PreviousTick count.=0D + @return The elapsed time in nanoseconds since PreviousCount.=0D + PreviousCount is overwritten with the current performance=0D + counter value.=0D +**/=0D +UINT64=0D +XhcGetElapsedTime (=0D + IN OUT UINT64 *PreviousTick=0D + )=0D +{=0D + UINT64 StartValue;=0D + UINT64 EndValue;=0D + UINT64 CurrentTick;=0D + UINT64 Delta;=0D +=0D + GetPerformanceCounterProperties (&StartValue, &EndValue);=0D +=0D + CurrentTick =3D GetPerformanceCounter ();=0D +=0D + //=0D + // Determine if the counter is counting up or down=0D + //=0D + if (StartValue < EndValue) {=0D + //=0D + // Counter counts upwards, check for an overflow condition=0D + //=0D + if (*PreviousTick > CurrentTick) {=0D + Delta =3D (EndValue - *PreviousTick) + CurrentTick;=0D + } else {=0D + Delta =3D CurrentTick - *PreviousTick;=0D + }=0D + } else {=0D + //=0D + // Counter counts downwards, check for an underflow condition=0D + //=0D + if (*PreviousTick < CurrentTick) {=0D + Delta =3D (StartValue - CurrentTick) + *PreviousTick;=0D + } else {=0D + Delta =3D *PreviousTick - CurrentTick;=0D + }=0D + }=0D +=0D + //=0D + // Set PreviousTick to CurrentTick=0D + //=0D + *PreviousTick =3D CurrentTick;=0D +=0D + return GetTimeInNanoSecond (Delta);=0D +}=0D diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h b/MdeModulePkg/Bus/Pci/Xhc= iDxe/Xhci.h index ca223bd20c..77feb66647 100644 --- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h +++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h @@ -2,6 +2,7 @@ =0D Provides some data structure definitions used by the XHCI host controlle= r driver.=0D =0D +(C) Copyright 2023 Hewlett Packard Enterprise Development LP
=0D Copyright (c) 2011 - 2017, Intel Corporation. All rights reserved.
=0D Copyright (c) Microsoft Corporation.
=0D SPDX-License-Identifier: BSD-2-Clause-Patent=0D @@ -26,6 +27,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #include =0D #include =0D #include =0D +#include =0D =0D #include =0D =0D @@ -37,6 +39,10 @@ typedef struct _USB_DEV_CONTEXT USB_DEV_CONTEXT; #include "ComponentName.h"=0D #include "UsbHcMem.h"=0D =0D +//=0D +// Converts a count from microseconds to nanoseconds=0D +//=0D +#define XHC_MICROSECOND_TO_NANOSECOND(Time) ((UINT64)(Time) * 1000)=0D //=0D // The unit is microsecond, setting it as 1us.=0D //=0D @@ -720,4 +726,20 @@ XhcAsyncIsochronousTransfer ( IN VOID *Context=0D );=0D =0D +/**=0D + Computes and returns the elapsed time in nanoseconds since=0D + PreviousTick. The value of PreviousTick is overwritten with the=0D + current performance counter value.=0D +=0D + @param PreviousTick Pointer to PreviousTick count.=0D + before calling this function.=0D + @return The elapsed time in nanoseconds since PreviousCount.=0D + PreviousCount is overwritten with the current performance=0D + counter value.=0D +**/=0D +UINT64=0D +XhcGetElapsedTime (=0D + IN OUT UINT64 *PreviousTick=0D + );=0D +=0D #endif=0D diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf b/MdeModulePkg/Bus/Pc= i/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=0D # Control, Bulk, Interrupt and Isochronous requests to those attached usb= LS/FS/HS/SS devices.=0D #=0D +# (C) Copyright 2023 Hewlett Packard Enterprise Development LP
=0D # Copyright (c) 2011 - 2018, Intel Corporation. All rights reserved.
= =0D #=0D # SPDX-License-Identifier: BSD-2-Clause-Patent=0D @@ -54,6 +55,7 @@ BaseMemoryLib=0D DebugLib=0D ReportStatusCodeLib=0D + TimerLib=0D =0D [Guids]=0D gEfiEventExitBootServicesGuid ## SOMETIMES_CONSUMES ## E= vent=0D diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c b/MdeModulePkg/Bus/Pci/= XhciDxe/XhciReg.c index 5700fc5fb8..07e57772b6 100644 --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c @@ -2,6 +2,7 @@ =0D The XHCI register operation routines.=0D =0D +(C) Copyright 2023 Hewlett Packard Enterprise Development LP
=0D Copyright (c) 2011 - 2017, Intel Corporation. All rights reserved.
=0D SPDX-License-Identifier: BSD-2-Clause-Patent=0D =0D @@ -417,15 +418,14 @@ XhcClearOpRegBit ( Wait the operation register's bit as specified by Bit=0D to become set (or clear).=0D =0D - @param Xhc The XHCI Instance.=0D - @param Offset The offset of the operation register.=0D - @param Bit The bit of the register to wait for.=0D - @param WaitToSet Wait the bit to set or clear.=0D - @param Timeout The time to wait before abort (in millise= cond, ms).=0D + @param Xhc The XHCI Instance.=0D + @param Offset The offset of the operation register.=0D + @param Bit The bit of the register to wait for.=0D + @param WaitToSet Wait the bit to set or clear.=0D + @param Timeout The time to wait before abort (in millisecond, ms).= =0D =0D - @retval EFI_SUCCESS The bit successfully changed by host cont= roller.=0D - @retval EFI_TIMEOUT The time out occurred.=0D - @retval EFI_OUT_OF_RESOURCES Memory for the timer event could not be a= llocated.=0D + @retval EFI_SUCCESS The bit successfully changed by host controller.=0D + @retval EFI_TIMEOUT The time out occurred.=0D =0D **/=0D EFI_STATUS=0D @@ -437,54 +437,35 @@ XhcWaitOpRegBit ( IN UINT32 Timeout=0D )=0D {=0D - EFI_STATUS Status;=0D - EFI_EVENT TimeoutEvent;=0D -=0D - TimeoutEvent =3D NULL;=0D + UINT64 TimeoutTime;=0D + UINT64 ElapsedTime;=0D + UINT64 TimeDelta;=0D + UINT64 CurrentTick;=0D =0D if (Timeout =3D=3D 0) {=0D return EFI_TIMEOUT;=0D }=0D =0D - Status =3D gBS->CreateEvent (=0D - EVT_TIMER,=0D - TPL_CALLBACK,=0D - NULL,=0D - NULL,=0D - &TimeoutEvent=0D - );=0D -=0D - if (EFI_ERROR (Status)) {=0D - goto DONE;=0D - }=0D -=0D - Status =3D gBS->SetTimer (=0D - TimeoutEvent,=0D - TimerRelative,=0D - EFI_TIMER_PERIOD_MILLISECONDS (Timeout)=0D - );=0D -=0D - if (EFI_ERROR (Status)) {=0D - goto DONE;=0D - }=0D + TimeoutTime =3D XHC_MICROSECOND_TO_NANOSECOND ((UINT64)Timeout * XHC_1_M= ILLISECOND);=0D + ElapsedTime =3D 0;=0D + CurrentTick =3D GetPerformanceCounter ();=0D =0D do {=0D if (XHC_REG_BIT_IS_SET (Xhc, Offset, Bit) =3D=3D WaitToSet) {=0D - Status =3D EFI_SUCCESS;=0D - goto DONE;=0D + return EFI_SUCCESS;=0D }=0D =0D gBS->Stall (XHC_1_MICROSECOND);=0D - } while (EFI_ERROR (gBS->CheckEvent (TimeoutEvent)));=0D -=0D - Status =3D EFI_TIMEOUT;=0D + TimeDelta =3D XhcGetElapsedTime (&CurrentTick);=0D + // Ensure that ElapsedTime is always incremented to avoid indefinite h= angs=0D + if (TimeDelta =3D=3D 0) {=0D + TimeDelta =3D XHC_MICROSECOND_TO_NANOSECOND (XHC_1_MICROSECOND);=0D + }=0D =0D -DONE:=0D - if (TimeoutEvent !=3D NULL) {=0D - gBS->CloseEvent (TimeoutEvent);=0D - }=0D + ElapsedTime +=3D TimeDelta;=0D + } while (ElapsedTime < TimeoutTime);=0D =0D - return Status;=0D + return EFI_TIMEOUT;=0D }=0D =0D /**=0D diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c b/MdeModulePkg/Bus/Pc= i/XhciDxe/XhciSched.c index 53421e64a8..7d4b7a769d 100644 --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c @@ -2,6 +2,7 @@ =0D XHCI transfer scheduling routines.=0D =0D +(C) Copyright 2023 Hewlett Packard Enterprise Development LP
=0D Copyright (c) 2011 - 2020, Intel Corporation. All rights reserved.
=0D Copyright (c) Microsoft Corporation.
=0D Copyright (C) 2022 Advanced Micro Devices, Inc. All rights reserved.
=0D @@ -1276,15 +1277,14 @@ EXIT: /**=0D Execute the transfer by polling the URB. This is a synchronous operation= .=0D =0D - @param Xhc The XHCI Instance.=0D - @param CmdTransfer The executed URB is for cmd transfer or n= ot.=0D - @param Urb The URB to execute.=0D - @param Timeout The time to wait before abort, in millise= cond.=0D + @param Xhc The XHCI Instance.=0D + @param CmdTransfer The executed URB is for cmd transfer or not.=0D + @param Urb The URB to execute.=0D + @param Timeout The time to wait before abort, in millisecond.= =0D =0D - @return EFI_DEVICE_ERROR The transfer failed due to transfer error= .=0D - @return EFI_TIMEOUT The transfer failed due to time out.=0D - @return EFI_SUCCESS The transfer finished OK.=0D - @retval EFI_OUT_OF_RESOURCES Memory for the timer event could not be a= llocated.=0D + @return EFI_DEVICE_ERROR The transfer failed due to transfer error.=0D + @return EFI_TIMEOUT The transfer failed due to time out.=0D + @return EFI_SUCCESS The transfer finished OK.=0D =0D **/=0D EFI_STATUS=0D @@ -1299,12 +1299,14 @@ XhcExecTransfer ( UINT8 SlotId;=0D UINT8 Dci;=0D BOOLEAN Finished;=0D - EFI_EVENT TimeoutEvent;=0D + UINT64 TimeoutTime;=0D + UINT64 ElapsedTime;=0D + UINT64 TimeDelta;=0D + UINT64 CurrentTick;=0D BOOLEAN IndefiniteTimeout;=0D =0D Status =3D EFI_SUCCESS;=0D Finished =3D FALSE;=0D - TimeoutEvent =3D NULL;=0D IndefiniteTimeout =3D FALSE;=0D =0D if (CmdTransfer) {=0D @@ -1322,34 +1324,14 @@ XhcExecTransfer ( =0D if (Timeout =3D=3D 0) {=0D IndefiniteTimeout =3D TRUE;=0D - goto RINGDOORBELL;=0D - }=0D -=0D - Status =3D gBS->CreateEvent (=0D - EVT_TIMER,=0D - TPL_CALLBACK,=0D - NULL,=0D - NULL,=0D - &TimeoutEvent=0D - );=0D -=0D - if (EFI_ERROR (Status)) {=0D - goto DONE;=0D }=0D =0D - Status =3D gBS->SetTimer (=0D - TimeoutEvent,=0D - TimerRelative,=0D - EFI_TIMER_PERIOD_MILLISECONDS (Timeout)=0D - );=0D -=0D - if (EFI_ERROR (Status)) {=0D - goto DONE;=0D - }=0D -=0D -RINGDOORBELL:=0D XhcRingDoorBell (Xhc, SlotId, Dci);=0D =0D + TimeoutTime =3D XHC_MICROSECOND_TO_NANOSECOND ((UINT64)Timeout * XHC_1_M= ILLISECOND);=0D + ElapsedTime =3D 0;=0D + CurrentTick =3D GetPerformanceCounter ();=0D +=0D do {=0D Finished =3D XhcCheckUrbResult (Xhc, Urb);=0D if (Finished) {=0D @@ -1357,22 +1339,22 @@ RINGDOORBELL: }=0D =0D gBS->Stall (XHC_1_MICROSECOND);=0D - } while (IndefiniteTimeout || EFI_ERROR (gBS->CheckEvent (TimeoutEvent))= );=0D + TimeDelta =3D XhcGetElapsedTime (&CurrentTick);=0D + // Ensure that ElapsedTime is always incremented to avoid indefinite h= angs=0D + if (TimeDelta =3D=3D 0) {=0D + TimeDelta =3D XHC_MICROSECOND_TO_NANOSECOND (XHC_1_MICROSECOND);=0D + }=0D =0D -DONE:=0D - if (EFI_ERROR (Status)) {=0D - Urb->Result =3D EFI_USB_ERR_NOTEXECUTE;=0D - } else if (!Finished) {=0D + ElapsedTime +=3D TimeDelta;=0D + } while (IndefiniteTimeout || ElapsedTime < TimeoutTime);=0D +=0D + if (!Finished) {=0D Urb->Result =3D EFI_USB_ERR_TIMEOUT;=0D Status =3D EFI_TIMEOUT;=0D } else if (Urb->Result !=3D EFI_USB_NOERROR) {=0D Status =3D EFI_DEVICE_ERROR;=0D }=0D =0D - if (TimeoutEvent !=3D NULL) {=0D - gBS->CloseEvent (TimeoutEvent);=0D - }=0D -=0D return Status;=0D }=0D =0D --=20 2.34.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#107733): https://edk2.groups.io/g/devel/message/107733 Mute This Topic: https://groups.io/mt/100739508/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-