From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4864:20::132; helo=mail-it1-x132.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it1-x132.google.com (mail-it1-x132.google.com [IPv6:2607:f8b0:4864:20::132]) (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 81B79211CFBF2 for ; Mon, 25 Feb 2019 23:36:26 -0800 (PST) Received: by mail-it1-x132.google.com with SMTP id r11so2522556itc.2 for ; Mon, 25 Feb 2019 23:36:26 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=CadRTxshnbKXNxnmKta2RZF+ZQ4F7HoiTdQnitCrVuQ=; b=CbnCA7Zt3fUcGz2+DFAJ2bFIOLfSfxHPWqhgTeeHgplR0sWA7oVQvTKKpjp/1yFCnF zk/aWwj+rNb8Cmb+EqBV7uOj8ehojaIWdoy+wZ0lDZPASOW2XBYd5KnkcWdTfSE7T2Xw d6UBZFAtoz5k4wUnHafHzI+VMtbARODqsonI8pPy8QySJaVS3c3cUUeUQ+nI0AGa1oME sfPhnQ8GjYHqfA0pfu8YJYORL+a/EGTlPrne66emm4k2V42ulsplfMIS6tN2LpR8ni2J ePA6r1lS2D6jVl3BJCP6vqLyVM+6a+15ihnuvp14YKeZcGQYFcM4W/IM7QT18HeqnJ4X 9Sog== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=CadRTxshnbKXNxnmKta2RZF+ZQ4F7HoiTdQnitCrVuQ=; b=TUGgwwSXHnaH+pXMbi3uYNZ742OaK47luceoKsVGjQe1UPuDDMkHcbfolGfcu+wqBl pT4h/ifcCq1IXF+Tqi2SeED+GVwJibtPKU9m/+QAkYNpAqMWYVNOgGlak5B0z3j2NvHa r+8HPgsshLXb2tkjXmcP54VsX/v0D3p3aj83DDXBqzGOcserQSnaMQzXwGuu3gepZKuK DL1tGD4jDK98GDdj3Jx+SJdVhWIitomeWskxqakZn7Pc8VUX8anEkp9ico2R5X8cBNTN Z2Q76thEKj/bwEsjKOAZCiTCjvH8N6QdcVM7DwJsWThygzNDJHMUCb97XX0YXMe9GLvA gxhA== X-Gm-Message-State: APjAAAXBeeLPiODyEv0TWsKfoA/ddQb8rZsiQT7J93ys5QfaaIG+9o0V G/ZE7Y62FXuyzjsXtEH4Hmqk0cPgYsQcrcPVdifjNQ== X-Google-Smtp-Source: AHgI3IbPQm1ar4qTjes7DKrqwLYxVPWsqvqycgpvCvJLpL0YcaFJngS60brfRqj+8tYFKnq92lcBjChabSClI+fOOok= X-Received: by 2002:a24:45dd:: with SMTP id c90mr1713325itd.71.1551166585299; Mon, 25 Feb 2019 23:36:25 -0800 (PST) MIME-Version: 1.0 References: <20190225235202.2252-1-pete@akeo.ie> <20190225235202.2252-2-pete@akeo.ie> In-Reply-To: <20190225235202.2252-2-pete@akeo.ie> From: Ard Biesheuvel Date: Tue, 26 Feb 2019 08:36:12 +0100 Message-ID: To: Pete Batard Cc: "edk2-devel@lists.01.org" , Leif Lindholm Subject: Re: [PATCH 1/1] EmbeddedPkg: Fix multiple Virtual RTC issues X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 26 Feb 2019 07:36:26 -0000 Content-Type: text/plain; charset="UTF-8" On Tue, 26 Feb 2019 at 00:52, Pete Batard wrote: > > 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. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Pete Batard Thanks Pete Reviewed-by: Ard Biesheuvel Pushed as 9ab4ec518882..1342d7679e10 > --- > EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c | 34 ++++++++++++++------ > 1 file changed, 25 insertions(+), 9 deletions(-) > > diff --git a/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c b/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c > index 4c354730d02b..5559106b696f 100644 > --- a/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c > +++ b/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c > @@ -54,13 +54,12 @@ LibGetTime ( > ) > { > EFI_STATUS Status; > - UINT32 EpochSeconds; > INT16 TimeZone; > UINT8 Daylight; > UINT64 Freq; > UINT64 Counter; > UINT64 Remainder; > - UINTN ElapsedSeconds; > + UINTN EpochSeconds; > UINTN Size; > > if (Time == NULL) { > @@ -75,13 +74,13 @@ LibGetTime ( > > // Get the epoch time from non-volatile storage > Size = sizeof (UINTN); > - ElapsedSeconds = 0; > + EpochSeconds = 0; > Status = EfiGetVariable ( > (CHAR16 *)mEpochVariableName, > &gEfiCallerIdGuid, > NULL, > &Size, > - (VOID *)&ElapsedSeconds > + (VOID *)&EpochSeconds > ); > // Fall back to compilation-time epoch if not set > if (EFI_ERROR (Status)) { > @@ -93,7 +92,7 @@ LibGetTime ( > // 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. > // > - ElapsedSeconds = BUILD_EPOCH; > + EpochSeconds = BUILD_EPOCH; > DEBUG (( > DEBUG_INFO, > "LibGetTime: %s non volatile variable was not found - Using compilation time epoch.\n", > @@ -101,7 +100,7 @@ LibGetTime ( > )); > } > Counter = GetPerformanceCounter (); > - ElapsedSeconds += DivU64x64Remainder (Counter, Freq, &Remainder); > + EpochSeconds += DivU64x64Remainder (Counter, Freq, &Remainder); > > // Get the current time zone information from non-volatile storage > 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 > // provided that the remainder doesn't overflow 64-bit during multiplication. > @@ -240,6 +239,9 @@ LibSetTime ( > ) > { > EFI_STATUS Status; > + UINT64 Freq; > + UINT64 Counter; > + UINT64 Remainder; > UINTN EpochSeconds; > > if (!IsTimeValid (Time)) { > @@ -249,16 +251,30 @@ LibSetTime ( > EpochSeconds = EfiTimeToEpoch (Time); > > // 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; > } > > // 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 > EpochSeconds -= SEC_PER_HOUR; > } > > + // Use the performance counter to substract 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 > Status = EfiSetVariable ( > (CHAR16 *)mTimeZoneVariableName, > -- > 2.17.0.windows.1 >