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>
This commit is contained in:
Pete Batard 2019-02-25 23:52:02 +00:00 committed by Ard Biesheuvel
parent 9ab4ec5188
commit 1342d7679e

View File

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