Re: [PATCH 1/1] EmbeddedPkg/Library: Add VirtualRealTimeClockLib

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On Fri, 15 Feb 2019 at 11:07, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> On Tue, 12 Feb 2019 at 19:14, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> >
> > On Mon, Feb 04, 2019 at 12:47:36PM +0000, Pete Batard wrote:
> > > This is designed to be used on platforms where a a real RTC is not
> > > available and relies on an RtcEpochSeconds variable having been set or,
> > > if that is not the case, falls back to using the epoch embedded at
> > > compilation time.
> > >
> > > Note that, in order to keep things simple for the setting of the
> > > compilation time variable, only GCC environments with UNIX-like shells
> > > and where a 'date' command is available are meant to be supported for
> > > now.
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Pete Batard <pete@akeo.ie>
> >
> > On the whole, this looks good to me.
> > One addition we'll need, so that we can build this library standalone
> > is an entry in EmbeddedPkg.dsc:
> >
> > diff --git a/EmbeddedPkg/EmbeddedPkg.dsc b/EmbeddedPkg/EmbeddedPkg.dsc
> > index 4d9e6399d5..dc5040e611 100644
> > --- a/EmbeddedPkg/EmbeddedPkg.dsc
> > +++ b/EmbeddedPkg/EmbeddedPkg.dsc
> > @@ -218,6 +218,7 @@ [Components.common]
> >    EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.inf
> >    EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.inf
> >    EmbeddedPkg/Library/DxeDtPlatformDtbLoaderLibDefault/DxeDtPlatformDtbLoaderLibDefault.inf
> > +  EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.inf
> >    EmbeddedPkg/EmbeddedMonotonicCounter/EmbeddedMonotonicCounter.inf
> >    EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf
> >
> > I don't have any strong opinions on either of Phil's suggestions, but
> > if you could give some feedback on those and fold the above in, this
> > could go in.
> >
>
> WIth this addition
>
> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> Pushed as 1b261a705f94..64a17fadcb79

OK, there is a problem with this code:

+EFI_STATUS
+EFIAPI
+LibGetTime (
+  OUT EFI_TIME               *Time,
+  OUT EFI_TIME_CAPABILITIES  *Capabilities
+  )
+{
+  EFI_STATUS  Status;
+  UINT32      EpochSeconds;

EpochSeconds is declared here, and updated depending on time zone and
DST settings. However, the resulting value is never used anywhere.

It is not clear to me what the correct fix is, so Pete, could you
please look into this?