Re: [PATCH 2/2] ArmPkg/ArmArchTimerLib: Implement GetElapsedTime function of TimerLib

[Laszlo Ersek <lersek@redhat.com>]

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 <leif.lindholm@linaro.org>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
>
> 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