From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c0b::244; helo=mail-it0-x244.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it0-x244.google.com (mail-it0-x244.google.com [IPv6:2607:f8b0:4001:c0b::244]) (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 7C21B20356897 for ; Mon, 13 Nov 2017 05:30:49 -0800 (PST) Received: by mail-it0-x244.google.com with SMTP id 72so9391913itl.5 for ; Mon, 13 Nov 2017 05:34:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=PUB+6hk2mZpRrYGfobEhvYd+cxKIGAnoCM54flKO2oU=; b=SQ3xc9FQUZT3mclSQOjzFe4KTkbxI/M+xJI0BvTjzpFWQCgSnBklIuxAGG1V+wdHyL KqlWYuLxkPI7S3bzrG060caj9/thFG3uiebBFCgMmdT1P1outSF9M9aLuSc9FsmhupmJ PciEPC49rOXIekBUQRaasIK5Ij/CGw8nihUqQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=PUB+6hk2mZpRrYGfobEhvYd+cxKIGAnoCM54flKO2oU=; b=fKtuiieZo1BNO+1g4POiDmSIb41p0ja+qQCMX2h4cxl3DzmJ3DapMRYiYvK04ARIzj Xgqj4rwIR2nlHu3YxYfF8YT6HVBtUnqPPzt8f4rhLTCOI9J0ICQpdQ5HosubWY2gOPyX EtQJv0XhrfxdjcWKu5sGSJKgTg+pYK1yEftngsj+AXv4wF5TlmrtM69WI4rrQKcokEzL NlyfQWZQURvKLATNHJcZDUIAMz2xP3STPziMenM/srKAIa10lcK8125+cF0kiA22XsGC Jv5dDznYQBAjfeFliTRejmBhFnvThspXqZZPwrdhfvOQHRXRpWgpzOY3G14aUTxWbyYQ Z5WQ== X-Gm-Message-State: AJaThX5c7705LSSk9pYoOEuXnHiZTDk6svZkSCaZbU9/nD8nRrEYyaqI 29B/umVsccsXpkYzhf0H3D6BqexghZIcovWTzkw9rA== X-Google-Smtp-Source: AGs4zMbyAGY9Iz/zR2gdDlsQXTP+ctYycuJRhWGw5eq9Tl39EuNVbiI7FxzefyaaVVBb4EzpRHhcC7klWVV8YiVoS4Y= X-Received: by 10.36.210.198 with SMTP id z189mr10208525itf.65.1510580094776; Mon, 13 Nov 2017 05:34:54 -0800 (PST) MIME-Version: 1.0 Received: by 10.107.104.20 with HTTP; Mon, 13 Nov 2017 05:34:54 -0800 (PST) In-Reply-To: <20171113120305.fquorwxstucscz4t@bivouac.eciton.net> References: <20171110080925.28599-1-ard.biesheuvel@linaro.org> <20171110080925.28599-2-ard.biesheuvel@linaro.org> <20171113120305.fquorwxstucscz4t@bivouac.eciton.net> From: Ard Biesheuvel Date: Mon, 13 Nov 2017 13:34:54 +0000 Message-ID: To: Leif Lindholm Cc: "edk2-devel@lists.01.org" , Udit Kumar 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 13:30:49 -0000 Content-Type: text/plain; charset="UTF-8" On 13 November 2017 at 12:03, Leif Lindholm wrote: > 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) > Thanks. Pushed as 517cf877946b EmbeddedPkg/RealTimeClockRuntimeDxe: move common functionality into core 26f9ef3ab3b3 ArmPlatformPkg/PL031RealTimeClockLib: remove validation and DST handling 207bc6a38c0a ArmPlatformPkg/PL031RealTimeClockLib: ignore DST setting when timezone is set >> --- >> 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 >>