From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [207.211.31.120]) by mx.groups.io with SMTP id smtpd.web11.1638.1599029152004007228 for ; Tue, 01 Sep 2020 23:45:52 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=Dic2MGVq; spf=pass (domain: redhat.com, ip: 207.211.31.120, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1599029151; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=/mrkmYOd/2QlKB2mChJxQN/9VOQRpUJHMnhZFUi317o=; b=Dic2MGVqxxGHtDdxwwAA6vJs9jWyG1WnzcZ8WOvQ22m7yhSjkirEPiOC5t2Oe2ZvCga/SP ZUKchEthnVw6IW+aqNXSukfVDtUUyr717Hr3SbSrONw554RAPN+p+dmeZdu5ipgUBflWZX khhfF9cpsznVKlALKdN7vOjSTeebNz8= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-563-YFQs2y1sO8-si7G5b4GP9w-1; Wed, 02 Sep 2020 02:45:43 -0400 X-MC-Unique: YFQs2y1sO8-si7G5b4GP9w-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id EBC4210ABDAB; Wed, 2 Sep 2020 06:45:41 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-113-14.ams2.redhat.com [10.36.113.14]) by smtp.corp.redhat.com (Postfix) with ESMTP id 9A50E6198E; Wed, 2 Sep 2020 06:45:40 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH 1/1] MdeModulePkg/XhciDxe: Fix Broken Timeouts To: devel@edk2.groups.io, patrick.henz@hpe.com Cc: Jian J Wang , Hao A Wu , Ray Ni References: From: "Laszlo Ersek" Message-ID: <17e2b77f-d10c-9953-b588-f03bfa496d74@redhat.com> Date: Wed, 2 Sep 2020 08:45:39 +0200 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0.003 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Content-Language: en-US Hi Patrick, On 09/01/20 20:55, patrick.henz@hpe.com wrote: > From: henz some meta-comments: (1) Please consider setting the "user.name" item in your git config to your full name, "Patrick Henz". Because right now it seems to be just "henz", and that doesn't look very nice in the git commit history. (2) When you update this patch for v2, point (1) will not fix this very patch however. So please update this particular patch with git commit --amend --author='Patrick Henz ' (3) Please run python3 BaseTools/Scripts/SetupGit.py in your edk2 clone -- it automates a bunch of the settings listed at , and more. For example, it sets 8bit content-transfer-encoding for mailing out your edk2 patches, which is helpful because quoted-printable doesn't play nice with the hard CRLF line endings that edk2 uses. Thank you! Laszlo > > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2948 > > Timeouts in the XhciDxe driver are taking longer than > expected due to the timeout loops not accounting for > code execution time. As en example, 5 second timeouts > have been observed to take around 36 seconds to complete. > Use SetTimer and Create/CheckEvent from Boot Services to > determine when timeout occurred. > > Cc: Jian J Wang > Cc: Hao A Wu > Cc: Ray Ni > Signed-off-by: Patrick Henz > --- > MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c | 28 ++++++++++++++++--- > MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 34 +++++++++++++++++------- > 2 files changed, 49 insertions(+), 13 deletions(-) > > diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c > index 42b773ab31..5f7507f0a5 100644 > --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c > +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c > @@ -442,17 +442,37 @@ XhcWaitOpRegBit ( > IN UINT32 Timeout > > ) > > { > > - UINT32 Index; > > - UINT64 Loop; > > + EFI_STATUS Status; > > + EFI_EVENT TimeoutEvent; > > > > - Loop = Timeout * XHC_1_MILLISECOND; > > + if (Timeout == 0) { > > + return EFI_TIMEOUT; > > + } > > + > > + Status = gBS->CreateEvent ( > > + EVT_TIMER, > > + TPL_CALLBACK, > > + NULL, > > + NULL, > > + &TimeoutEvent > > + ); > > > > - for (Index = 0; Index < Loop; Index++) { > > + if (!EFI_ERROR (Status)) { > > + Status = gBS->SetTimer (TimeoutEvent, > > + TimerRelative, > > + EFI_TIMER_PERIOD_MILLISECONDS(Timeout)); > > + } > > + > > + do { > > if (XHC_REG_BIT_IS_SET (Xhc, Offset, Bit) == WaitToSet) { > > return EFI_SUCCESS; > > } > > > > gBS->Stall (XHC_1_MICROSECOND); > > + } while (!EFI_ERROR(Status) && EFI_ERROR(gBS->CheckEvent (TimeoutEvent))); > > + > > + if (TimeoutEvent != NULL) { > > + gBS->CloseEvent (TimeoutEvent); > > } > > > > return EFI_TIMEOUT; > > diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c > index ab8957c546..cf271f0493 100644 > --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c > +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c > @@ -1273,11 +1273,11 @@ XhcExecTransfer ( > ) > > { > > EFI_STATUS Status; > > - UINTN Index; > > - UINT64 Loop; > > UINT8 SlotId; > > UINT8 Dci; > > BOOLEAN Finished; > > + EFI_EVENT TimeoutEvent; > > + EFI_STATUS TimerStatus; > > > > if (CmdTransfer) { > > SlotId = 0; > > @@ -1292,28 +1292,44 @@ XhcExecTransfer ( > } > > > > Status = EFI_SUCCESS; > > - Loop = Timeout * XHC_1_MILLISECOND; > > - if (Timeout == 0) { > > - Loop = 0xFFFFFFFF; > > - } > > + > > + TimerStatus = gBS->CreateEvent ( > > + EVT_TIMER, > > + TPL_CALLBACK, > > + NULL, > > + NULL, > > + &TimeoutEvent > > + ); > > > > XhcRingDoorBell (Xhc, SlotId, Dci); > > > > - for (Index = 0; Index < Loop; Index++) { > > + if (!EFI_ERROR (TimerStatus)) { > > + TimerStatus = gBS->SetTimer (TimeoutEvent, > > + TimerRelative, > > + (0 == Timeout)? > > + (EFI_TIMER_PERIOD_MICROSECONDS(0xFFFFFFFF)): > > + (EFI_TIMER_PERIOD_MILLISECONDS(Timeout))); > > + } > > + > > + do { > > Finished = XhcCheckUrbResult (Xhc, Urb); > > if (Finished) { > > break; > > } > > gBS->Stall (XHC_1_MICROSECOND); > > - } > > + } while (!EFI_ERROR(TimerStatus) && EFI_ERROR(gBS->CheckEvent (TimeoutEvent))); > > > > - if (Index == Loop) { > > + 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; > > } > > >