From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.132.183.28; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 92038222CF1B7 for ; Wed, 10 Jan 2018 06:11:30 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 61AD48762F; Wed, 10 Jan 2018 14:16:32 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-123-164.rdu2.redhat.com [10.10.123.164]) by smtp.corp.redhat.com (Postfix) with ESMTP id C7E17119586; Wed, 10 Jan 2018 14:16:28 +0000 (UTC) To: Pankaj Bansal , edk2-devel@lists.01.org Cc: Leif Lindholm , Ard Biesheuvel , Michael Kinney References: <1515576669-14171-1-git-send-email-pankaj.bansal@nxp.com> <1515576669-14171-2-git-send-email-pankaj.bansal@nxp.com> From: Laszlo Ersek Message-ID: Date: Wed, 10 Jan 2018 15:16:26 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 MIME-Version: 1.0 In-Reply-To: <1515576669-14171-2-git-send-email-pankaj.bansal@nxp.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Wed, 10 Jan 2018 14:16:42 +0000 (UTC) Subject: Re: [PATCH 2/2] ArmPkg/ArmArchTimerLib: Implement GetElapsedTime function of TimerLib X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 10 Jan 2018 14:11:32 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Hello Pankaj, On 01/10/18 10:31, Pankaj Bansal wrote: > This function calculates the time elaped in Naoseconds between call to > this function and BaseTime, which is passed as argument. > > This is particularly useful in detecting timeout conditions. > > Cc: Leif Lindholm > Cc: Ard Biesheuvel > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Pankaj Bansal > > diff --git a/ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.c b/ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.c > index b81293c..0898339 100644 > --- a/ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.c > +++ b/ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.c > @@ -290,3 +290,39 @@ GetTimeInNanoSecond ( > > return NanoSeconds; > } > + > +/** > + Get Elapsed time in Nanoseonds w.r.t BaseTime > + > + This function calculates the time elaped in Naoseconds between call to this > + function and BaseTime, which is passed as argument. > + > + @param BaseTime BaseTime in NanoSeconds. > + > + @return The elapsed time in nanoseconds. > + > +**/ > +UINT64 > +EFIAPI > +GetElapsedTime ( > + IN UINT64 BaseTime > + ) > +{ > + UINT64 NanoSeconds; > + UINT64 Ticks; > + > + // > + // Get current Ticks. > + // > + Ticks = GetPerformanceCounter(); > + > + // > + // Convert Ticks to Nanoseconds > + // > + NanoSeconds = GetTimeInNanoSecond (Ticks); > + > + // > + // return the difference > + // > + return (NanoSeconds - BaseTime); > +} > There are two problems with this patch set: (1) The TimerLib.h file (in the first patch) is a public library class header, for which several library instances (implementations) exist. So, introducing a new API requires adding an implementation (the same implementation, or different ones, as necessary) to *all* instances in the edk2 tree. (2) The calculation in your GetElapsedTime() function is wrong. GetTimeInNanoSecond() converts a small *difference* of ticks to time. It does not convert an absolute tick value to an absolute time. Furthermore, tick differences are less trivial to calculate than one might imagine, because (a) a performance counter may count down, and *independently*, (b) the numeric low bound of the counter range may not be zero. Earlier I proposed a new TimerTickDiffLib class (and implementation) for centralizing exactly this kind of calculation. Please see the thread at: [edk2] TimerTickDiffLib for MdePkg? http://mid.mail-archive.com/8cba2a58-1333-7733-031d-0883dbd844c6@redhat.com Thanks Laszlo