From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2a00:1450:400c:c09::235; helo=mail-wm0-x235.google.com; envelope-from=leif.lindholm@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wm0-x235.google.com (mail-wm0-x235.google.com [IPv6:2a00:1450:400c:c09::235]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 5B549203555F8 for ; Mon, 13 Nov 2017 03:59:04 -0800 (PST) Received: by mail-wm0-x235.google.com with SMTP id b189so7516985wmd.5 for ; Mon, 13 Nov 2017 04:03:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=VFeQnigHTWZdW4iY9z+T/m3FN0+C5Ve4XJs0K0Y7nWY=; b=YQC1j93UPookV9RMWqzxBTuyaZweKYM17z61Kp1zGjBVTKhFfUABGL16JPgUdBcPMf VrDlm+fexQA6DDTCtFPdhha67/yLfUyr7PLw0HBhlQho8xCF71TqA9wgcnRjAEjpWkQj fj1197xm7UUVvY80dVgEOEs3ElDnkEgsv5uvs= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=VFeQnigHTWZdW4iY9z+T/m3FN0+C5Ve4XJs0K0Y7nWY=; b=DhURUd2u0QsTgyCfRdcL5g4RQH90IeZqMLhy44qOzzQP813GKt8rqVOCyyUHtkvr3z cMAwcQC0KXlisxcxm/POMPhhC/wQVgUj8pCH0aIwzkaOwkCofxTTKnV8c8D03lqeMBuA h8zRXZ5Y8YRa6jcipwSqztGHGFruaU6lBqKa0zFcSomMKO+SpYFSJAh5FeO1w33z0kCD Q1Z0DHZF+6CGbokAYi7mK9M7QI9S2kXG0JbVRmaONUrTyc6bj3wy1T28zA8fEV9AaGVb txkaNnRhuAB3WDhLkFucQy8H9wcPy7e/GRP8ZKkBB9kZh0qu6pgDp3QbvLSea+1KxFsX rksQ== X-Gm-Message-State: AJaThX7kkjFnc4gb6e9qdtJ/T3igGHK1XUpjR9WqxezwsRiIYUjS9MUC iIdvB/X2qojKZQDcRagrW9Ue3A== X-Google-Smtp-Source: AGs4zMaoargfCT1TQwQuXpdkaf/nvMBR4MhWTASUlWSGMrs1M1TYWFt6V69+16S+SHg1amJWBGgsTQ== X-Received: by 10.28.108.3 with SMTP id h3mr5007179wmc.129.1510574588780; Mon, 13 Nov 2017 04:03:08 -0800 (PST) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id y15sm18970535wrc.96.2017.11.13.04.03.07 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 13 Nov 2017 04:03:07 -0800 (PST) Date: Mon, 13 Nov 2017 12:03:05 +0000 From: Leif Lindholm To: Ard Biesheuvel Cc: edk2-devel@lists.01.org, udit.kumar@nxp.com Message-ID: <20171113120305.fquorwxstucscz4t@bivouac.eciton.net> References: <20171110080925.28599-1-ard.biesheuvel@linaro.org> <20171110080925.28599-2-ard.biesheuvel@linaro.org> MIME-Version: 1.0 In-Reply-To: <20171110080925.28599-2-ard.biesheuvel@linaro.org> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [PATCH v3 1/3] EmbeddedPkg/RealTimeClockRuntimeDxe: move common functionality into core X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 13 Nov 2017 11:59:04 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Fri, Nov 10, 2017 at 08:09:23AM +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. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel Reviewed-by: Leif Lindholm (which makes the whole series reviewed by me) > --- > 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..8323a4b4b848 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.
> + Copyright (c) 2017, Linaro, Ltd. All rights reserved.
> > 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 > +#include > +#include > #include > #include > -#include > +#include > #include > > 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 <= 28); > + > + 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); > + 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.
> +# Copyright (c) 2006 - 2007, Intel Corporation. All rights reserved.
> +# Copyright (c) 2017, Linaro, Ltd. All rights reserved.
> # > # 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 >