EmbeddedPkg/VirtualRealTimeClockLib: Fix correctness issues

LibGetTime():
- Two variables were used for the epoch, where only one should have been [*].
- Also harmonize variable name to match the one used in LibSetTime.
LibSetTime():
- Address possible underflows if time is set to start of epoch.
- Ensure that time being read does actually match time that was manually
  set (plus the time elapsed since), by subtracting number of seconds
  since reset.

[*] This fixes a build breakage, since one of these variables was set but
    never used, triggering a compiler diagnostic at some optimization levels.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Pete Batard <pete@akeo.ie>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

diff --git a/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c b/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c
index 4c354730d02b2d8a29d3c299f1230e2546187b53..8ad2df90e702ede2c0562f8091a5cd114fe49439 100644 (file)
--- a/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c
+++ b/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c
@@ -54,13 +54,12 @@ LibGetTime (
   )\r
 {\r
   EFI_STATUS  Status;\r
-  UINT32      EpochSeconds;\r
   INT16       TimeZone;\r
   UINT8       Daylight;\r
   UINT64      Freq;\r
   UINT64      Counter;\r
   UINT64      Remainder;\r
-  UINTN       ElapsedSeconds;\r
+  UINTN       EpochSeconds;\r
   UINTN       Size;\r
 \r
   if (Time == NULL) {\r
@@ -75,13 +74,13 @@ LibGetTime (
 \r
   // Get the epoch time from non-volatile storage\r
   Size = sizeof (UINTN);\r
-  ElapsedSeconds = 0;\r
+  EpochSeconds = 0;\r
   Status = EfiGetVariable (\r
              (CHAR16 *)mEpochVariableName,\r
              &gEfiCallerIdGuid,\r
              NULL,\r
              &Size,\r
-             (VOID *)&ElapsedSeconds\r
+             (VOID *)&EpochSeconds\r
              );\r
   // Fall back to compilation-time epoch if not set\r
   if (EFI_ERROR (Status)) {\r
@@ -93,7 +92,7 @@ LibGetTime (
     // If you are attempting to use this library on such an environment, please\r
     // contact the edk2 mailing list, so we can try to add support for it.\r
     //\r
-    ElapsedSeconds = BUILD_EPOCH;\r
+    EpochSeconds = BUILD_EPOCH;\r
     DEBUG ((\r
       DEBUG_INFO,\r
       "LibGetTime: %s non volatile variable was not found - Using compilation time epoch.\n",\r
@@ -101,7 +100,7 @@ LibGetTime (
       ));\r
   }\r
   Counter = GetPerformanceCounter ();\r
-  ElapsedSeconds += DivU64x64Remainder (Counter, Freq, &Remainder);\r
+  EpochSeconds += DivU64x64Remainder (Counter, Freq, &Remainder);\r
 \r
   // Get the current time zone information from non-volatile storage\r
   Size = sizeof (TimeZone);\r
@@ -204,7 +203,7 @@ LibGetTime (
     }\r
   }\r
 \r
-  EpochToEfiTime (ElapsedSeconds, Time);\r
+  EpochToEfiTime (EpochSeconds, Time);\r
 \r
   // Because we use the performance counter, we can fill the Nanosecond attribute\r
   // provided that the remainder doesn't overflow 64-bit during multiplication.\r
@@ -240,6 +239,9 @@ LibSetTime (
   )\r
 {\r
   EFI_STATUS  Status;\r
+  UINT64      Freq;\r
+  UINT64      Counter;\r
+  UINT64      Remainder;\r
   UINTN       EpochSeconds;\r
 \r
   if (!IsTimeValid (Time)) {\r
@@ -249,16 +251,30 @@ LibSetTime (
   EpochSeconds = EfiTimeToEpoch (Time);\r
 \r
   // Adjust for the correct time zone, i.e. convert to UTC time zone\r
-  if (Time->TimeZone != EFI_UNSPECIFIED_TIMEZONE) {\r
+  if ((Time->TimeZone != EFI_UNSPECIFIED_TIMEZONE)\r
+      && (EpochSeconds > Time->TimeZone * SEC_PER_MIN)) {\r
     EpochSeconds -= Time->TimeZone * SEC_PER_MIN;\r
   }\r
 \r
   // Adjust for the correct period\r
-  if ((Time->Daylight & EFI_TIME_IN_DAYLIGHT) == EFI_TIME_IN_DAYLIGHT) {\r
+  if (((Time->Daylight & EFI_TIME_IN_DAYLIGHT) == EFI_TIME_IN_DAYLIGHT)\r
+      && (EpochSeconds > SEC_PER_HOUR)) {\r
     // Convert to un-adjusted time, i.e. fall back one hour\r
     EpochSeconds -= SEC_PER_HOUR;\r
   }\r
 \r
+  // Use the performance counter to subtract the number of seconds\r
+  // since platform reset. Without this, setting time from the shell\r
+  // and immediately reading it back would result in a forward time\r
+  // offset, of the duration during which the platform has been up.\r
+  Freq = GetPerformanceCounterProperties (NULL, NULL);\r
+  if (Freq != 0) {\r
+    Counter = GetPerformanceCounter ();\r
+    if (EpochSeconds > DivU64x64Remainder (Counter, Freq, &Remainder)) {\r
+      EpochSeconds -= DivU64x64Remainder (Counter, Freq, &Remainder);\r
+    }\r
+  }\r
+\r
   // Save the current time zone information into non-volatile storage\r
   Status = EfiSetVariable (\r
              (CHAR16 *)mTimeZoneVariableName,\r