On Mon, Nov 06, 2017 at 06:20:36PM +0000, Ard Biesheuvel wrote:
> RealTimeClockRuntimeDxe defers the hardware/platform specific handling
> of reading/setting the hardware clock to RealTimeClockLib, but for
> unknown reasons, it also defers common functionality such as input
> validation and recording the timezone and DST settings (which are
> informational only and not managed by hardware)
>
> This has led to a lot of duplication in implementations of RealTimeClockLib
> as well as TimeBaseLib, to the point where each library implementation
> has its own set of UEFI variables to record the timezone and DST settings.
> This makes little sense, and so let's update RealTimeClockRuntimeDxe now
> to allow future implementations to rely on the core driver to take care of
> these things.
>
> Note that reading the timezone and DST settings occurs before calling into
> the library, so we can phase out this behavior gradually from library
> implementations in EDK2, edk2-platforms or out of tree.
Great consolidation, couple of minor questions/comments below:
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
> ---
> EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c | 171
> +++++++++++++++++++-
> EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf | 11 +-
> 2 files changed, 171 insertions(+), 11 deletions(-)
>
> diff --git a/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c
> b/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c
> index f1e067c0b59e..6b7cc876fa33 100644
> --- a/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c
> +++ b/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c
> @@ -1,10 +1,8 @@
> /** @file
> Implement EFI RealTimeClock runtime services via RTC Lib.
>
> - Currently this driver does not support runtime virtual calling.
> -
> -
> Copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
> + Copyright (c) 2017, Linaro, Ltd. All rights reserved.<BR>
>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
> License
> @@ -17,14 +15,116 @@
> **/
>
> #include <PiDxe.h>
> +#include <Library/DebugLib.h>
> +#include <Library/RealTimeClockLib.h>
> #include <Library/UefiLib.h>
> #include <Library/UefiBootServicesTableLib.h>
> -#include <Library/RealTimeClockLib.h>
> +#include <Library/UefiRuntimeLib.h>
> #include <Protocol/RealTimeClock.h>
>
> EFI_HANDLE mHandle = NULL;
>
> +//
> +// These values can be set by SetTime () and need to be returned by GetTime
> ()
> +// but cannot usually be kept by the RTC hardware, so we store them in a UEFI
> +// variable instead.
> +//
> +typedef struct {
> + INT16 TimeZone;
> + UINT8 Daylight;
> +} NON_VOLATILE_TIME_SETTINGS;
> +
> +STATIC CONST CHAR16 mTimeSettingsVariableName[] = L"RtcTimeSettings";
> +STATIC NON_VOLATILE_TIME_SETTINGS mTimeSettings;
> +
> +STATIC
> +BOOLEAN
> +IsValidTimeZone (
> + IN INT16 TimeZone
> + )
> +{
> + return TimeZone == EFI_UNSPECIFIED_TIMEZONE ||
> + (TimeZone >= -1440 && TimeZone <= 1440);
> +}
> +
> +STATIC
> +BOOLEAN
> +IsValidDaylight (
> + IN INT8 Daylight
> + )
> +{
> + return Daylight == 0 ||
> + Daylight == EFI_TIME_ADJUST_DAYLIGHT ||
> + Daylight == (EFI_TIME_ADJUST_DAYLIGHT | EFI_TIME_IN_DAYLIGHT);
> +}
>
> +STATIC
> +BOOLEAN
> +EFIAPI
> +IsLeapYear (
> + IN EFI_TIME *Time
> + )
> +{
> + if (Time->Year % 4 == 0) {
> + if (Time->Year % 100 == 0) {
> + if (Time->Year % 400 == 0) {
> + return TRUE;
> + } else {
> + return FALSE;
> + }
> + } else {
> + return TRUE;
> + }
> + } else {
> + return FALSE;
> + }
> +}
> +
> +STATIC CONST INTN mDayOfMonth[12] = {
> + 31, 29, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31
> +};
> +
> +STATIC
> +BOOLEAN
> +EFIAPI
> +IsDayValid (
> + IN EFI_TIME *Time
> + )
> +{
> + ASSERT (Time->Day >= 1);
> + ASSERT (Time->Day <= mDayOfMonth[Time->Month - 1]);
> + ASSERT (Time->Month != 2 || (IsLeapYear (Time) && Time->Day <= 29));
Is this me getting confused by basic logic operations, or should the
above be something like
ASSERT (Time->Month != 2 || (Time->Day <= (28 + IsLeapYear (Time) ? 1 : 0)));
?
> +
> + if (Time->Day < 1 ||
> + Time->Day > mDayOfMonth[Time->Month - 1] ||
> + (Time->Month == 2 && (!IsLeapYear (Time) && Time->Day > 28))) {
> + return FALSE;
> + }
> + return TRUE;
> +}
> +
> +STATIC
> +BOOLEAN
> +EFIAPI
> +IsTimeValid(
> + IN EFI_TIME *Time
> + )
> +{
> + // Check the input parameters are within the range specified by UEFI
> + if (Time->Year < 1900 ||
> + Time->Year > 9999 ||
> + Time->Month < 1 ||
> + Time->Month > 12 ||
> + !IsDayValid (Time) ||
> + Time->Hour > 23 ||
> + Time->Minute > 59 ||
> + Time->Second > 59 ||
> + !IsValidTimeZone (Time->TimeZone) ||
> + !IsValidDaylight (Time->Daylight)) {
> + return FALSE;
> + }
> + return TRUE;
> +}
>
> /**
> Returns the current time and date information, and the time-keeping
> capabilities
> @@ -43,9 +143,20 @@ EFI_STATUS
> EFIAPI
> GetTime (
> OUT EFI_TIME *Time,
> - OUT EFI_TIME_CAPABILITIES *Capabilities
> + OUT EFI_TIME_CAPABILITIES *Capabilities
> )
> {
> + if (Time == NULL) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + //
> + // Set these first so the RealTimeClockLib implementation
> + // can override them based on its own settings.
> + //
> + Time->TimeZone = mTimeSettings.TimeZone;
> + Time->Daylight = mTimeSettings.Daylight;
> +
> return LibGetTime (Time, Capabilities);
> }
>
> @@ -67,7 +178,41 @@ SetTime (
> IN EFI_TIME *Time
> )
> {
> - return LibSetTime (Time);
> + EFI_STATUS Status;
> + BOOLEAN TimeSettingsChanged;
> +
> + if (Time == NULL || !IsTimeValid (Time)) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + TimeSettingsChanged = FALSE;
> + if (mTimeSettings.TimeZone != Time->TimeZone ||
> + mTimeSettings.Daylight != Time->Daylight) {
> +
> + mTimeSettings.TimeZone = Time->TimeZone;
> + mTimeSettings.Daylight = Time->Daylight;
> + TimeSettingsChanged = TRUE;
> + }
> +
> + Status = LibSetTime (Time);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + if (TimeSettingsChanged) {
> + Status = EfiSetVariable (
> + (CHAR16 *)mTimeSettingsVariableName,
> + &gEfiCallerIdGuid,
> + EFI_VARIABLE_NON_VOLATILE |
> + EFI_VARIABLE_BOOTSERVICE_ACCESS |
> + EFI_VARIABLE_RUNTIME_ACCESS,
> + sizeof (mTimeSettings),
> + (VOID *)&mTimeSettings);
> + if (EFI_ERROR (Status)) {
> + return EFI_DEVICE_ERROR;
> + }
> + }
> + return EFI_SUCCESS;
> }
>
>
> @@ -138,12 +283,26 @@ InitializeRealTimeClock (
> )
> {
> EFI_STATUS Status;
> + UINTN Size;
>
> Status = LibRtcInitialize (ImageHandle, SystemTable);
> if (EFI_ERROR (Status)) {
> return Status;
> }
>
> + Size = sizeof (mTimeSettings);
> + Status = EfiGetVariable ((CHAR16 *)mTimeSettingsVariableName,
> + &gEfiCallerIdGuid, NULL, &Size, (VOID *)&mTimeSettings);
Funky indentation?
/
Leif
> + if (EFI_ERROR (Status) ||
> + !IsValidTimeZone (mTimeSettings.TimeZone) ||
> + !IsValidDaylight (mTimeSettings.Daylight)) {
> + DEBUG ((DEBUG_WARN, "%a: using default timezone/daylight settings\n",
> + __FUNCTION__));
> +
> + mTimeSettings.TimeZone = EFI_UNSPECIFIED_TIMEZONE;
> + mTimeSettings.Daylight = 0;
> + }
> +
> SystemTable->RuntimeServices->GetTime = GetTime;
> SystemTable->RuntimeServices->SetTime = SetTime;
> SystemTable->RuntimeServices->GetWakeupTime = GetWakeupTime;
> diff --git a/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf
> b/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf
> index 186d4610bd42..d0520392f145 100644
> --- a/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf
> +++ b/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf
> @@ -1,8 +1,8 @@
> #/** @file
> -# Reset Architectural Protocol Driver as defined in PI
> +# Real Time Clock Architectural Protocol Driver as defined in PI
> #
> -# This Reset module simulates system reset by process exit on NT.
> -# Copyright (c) 2006 - 2007, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2006 - 2007, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2017, Linaro, Ltd. All rights reserved.<BR>
> #
> # This program and the accompanying materials
> # are licensed and made available under the terms and conditions of the BSD
> License
> @@ -31,10 +31,11 @@ [Packages]
> EmbeddedPkg/EmbeddedPkg.dec
>
> [LibraryClasses]
> - UefiBootServicesTableLib
> - UefiDriverEntryPoint
> DebugLib
> RealTimeClockLib
> + UefiBootServicesTableLib
> + UefiDriverEntryPoint
> + UefiRuntimeLib
>
> [Protocols]
> gEfiRealTimeClockArchProtocolGuid
> --
> 2.11.0
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
