From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by mx.groups.io with SMTP id smtpd.web12.7406.1589248327819626041 for ; Mon, 11 May 2020 18:52:07 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.151, mailfrom: ray.ni@intel.com) IronPort-SDR: 4G8TDv9rnjMK1k9T4FkpmARhE1qU52hWu4d0FhzhWVcr90OjLn389EzgvLWziN/lT+P/Lhqpn/ EJOG+TxZkiEA== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 May 2020 18:52:06 -0700 IronPort-SDR: y7Rx+A8wAA3R9t2b2AKaH/vN0MQxsMMAFHfyf3H8pBtxsVpCe4BAKhUPJ71G/zweSj5o8hFz46 yUQaOJ37hQkw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.73,381,1583222400"; d="scan'208";a="279960868" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by orsmga002.jf.intel.com with ESMTP; 11 May 2020 18:52:06 -0700 Received: from shsmsx154.ccr.corp.intel.com (10.239.6.54) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.439.0; Mon, 11 May 2020 18:52:06 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.210]) by SHSMSX154.ccr.corp.intel.com ([169.254.7.181]) with mapi id 14.03.0439.000; Tue, 12 May 2020 09:52:03 +0800 From: "Ni, Ray" To: "Yau, KaiX" , "devel@edk2.groups.io" , "kaiyau@gmail.com" CC: "Jiang, Guomin" , "Gao, Liming" , "Archield, Ralphal" Subject: Re: [edk2-devel] [PATCH] PcAtChipsetPkg: Wrong System YEAR displayed in SETUP Thread-Topic: [edk2-devel] [PATCH] PcAtChipsetPkg: Wrong System YEAR displayed in SETUP Thread-Index: AQHWAMIW/k98ZzzV00mjseooVZSurqiMedbQ//+tKwCAF9ZJMA== Date: Tue, 12 May 2020 01:52:02 +0000 Message-ID: <734D49CCEBEEF84792F5B80ED585239D5C53809A@SHSMSX104.ccr.corp.intel.com> References: <50102ef8b38c0e5287ab4d1cb8fc636213f67299.1584672719.git.kaix.yau@intel.com> <734D49CCEBEEF84792F5B80ED585239D5C51152B@SHSMSX104.ccr.corp.intel.com> <3f6385f623054e24b44a375765246d2f@intel.com> In-Reply-To: <3f6385f623054e24b44a375765246d2f@intel.com> Accept-Language: en-US, zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: ray.ni@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable The PCD range (100 years) assumes that: there is no such a x86 system that can use a bios flashed in year 1998 and= this system will be functional till 2097. Or even it can be functional til= l 2097, its bios should be flashed multiple times during the 100 years. I don't think we need to consider the solution to Y3K because I guess (90%= believe) the RTC hardware will disappear after 10 years at most. Thanks, Ray > -----Original Message----- > From: Yau, KaiX > Sent: Monday, April 27, 2020 1:47 PM > To: Ni, Ray ; devel@edk2.groups.io; kaiyau@gmail.com > Cc: Jiang, Guomin ; Gao, Liming ; Archield, Ralphal > > Subject: RE: [edk2-devel] [PATCH] PcAtChipsetPkg: Wrong System YEAR disp= layed in SETUP >=20 > Hi Ray, >=20 > The current logic in PcRtc.c will give us WRONG year. (I explained below= ) > If we only change the PCD Min. value to 2000 and Max. value to 2099. The= current logic will work. But we still have Y3K bug. >=20 > That is why we need to read from Hardware. > If any reasons you guys doesn't want to read from hardware, at least we = need to change Min. and Max. value ASAP. > (you can go ahead to delete the change in PcRtc.c and just submit the PC= D changes) >=20 > Regards > Kai Yau >=20 >=20 >=20 > ------------------------------------------------------------------------= ----- > The root cause is from the following logic (PcRtc.c): >=20 > Current Default PcdMinimalValidYear =3D 1998 > Current Default PcdMaximalValidYear =3D 2097 >=20 > If we can set the YEAR to 2099, we will end up getting 1999 instead. T= he reason why user have not found this problem > because Windows does not allow user to set the YEAR to 2099. If we make = the PcdMaximalValidYear smaller, you can > reproduce this issue. >=20 > Time->Year =3D CheckAndConvertBcd8ToDecimal8 ((UINT8) Time->Year); = <-- Time->Year =3D 99 > Century =3D (UINT8) (PcdGet16 (PcdMinimalValidYear) / 100); = <-- Century =3D 19 > if (Time->Year < PcdGet16 (PcdMinimalValidYear) % 100) { = <-- (99 < 97) =3D FALSE > Century++; > } > Time->Year =3D (UINT16) (Century * 100 + Time->Year); = <-- Time->Year =3D 1999 (19 x 100 + 99). Also 1999 is > within the RANGE, so it is considered as "VALID" data; however it is wro= ng. >=20 > Regards > Kai >=20 >=20 >=20 >=20 >=20 > -----Original Message----- > From: Ni, Ray > Sent: Monday, April 27, 2020 12:54 AM > To: devel@edk2.groups.io; kaiyau@gmail.com > Cc: Yau, KaiX ; Jiang, Guomin ; Gao, Liming > Subject: RE: [edk2-devel] [PATCH] PcAtChipsetPkg: Wrong System YEAR disp= layed in SETUP >=20 > This patch introduces several changes for different purposes. > At least I can see: > 1. Default PCD value change > 2. Consume century value from HW >=20 > Can you separate the patches to 2 patches? >=20 >=20 > For the purpose #2, can you explain more background/reason? >=20 > The PcdMinimalValidYear/ PcdMaximalValidYear value are chosen that when = a two-digit year is read from HW, there is no > confusion to convert to 4-digit year. > Are you aware of this when you made the patch? >=20 > Thanks, > Ray >=20 >=20 > > -----Original Message----- > > From: devel@edk2.groups.io On Behalf Of Yau > > Sent: Friday, March 20, 2020 11:42 AM > > To: devel@edk2.groups.io > > Cc: Yau, KaiX > > Subject: [edk2-devel] [PATCH] PcAtChipsetPkg: Wrong System YEAR > > displayed in SETUP > > > > Function ConvertRtcTimeToEfiTime() will reset to Century 19XX when the > > year is set larger than the value of PcdMaximalValidYear outside SETUP= . > > > > Signed-off-by: Yau > > --- > > PcAtChipsetPkg/PcAtChipsetPkg.dec | 7 +++--- > > .../PcatRealTimeClockRuntimeDxe/PcRtc.c | 25 +++++++++++-------= - > > .../PcatRealTimeClockRuntimeDxe/PcRtc.h | 17 +++++++------ > > 3 files changed, 27 insertions(+), 22 deletions(-) > > > > diff --git a/PcAtChipsetPkg/PcAtChipsetPkg.dec > > b/PcAtChipsetPkg/PcAtChipsetPkg.dec > > index 88de5cceea..660cb5d52e 100644 > > --- a/PcAtChipsetPkg/PcAtChipsetPkg.dec > > +++ b/PcAtChipsetPkg/PcAtChipsetPkg.dec > > @@ -4,7 +4,7 @@ > > # This package is designed to public interfaces and implementation > > which follows # PcAt defacto standard. > > # > > -# Copyright (c) 2009 - 2019, Intel Corporation. All rights > > reserved.
> > +# Copyright (c) 2009 - 2020, Intel Corporation. All rights > > +reserved.
> > # Copyright (c) 2017, AMD Inc. All rights reserved.
# # > > SPDX-License-Identifier: BSD-2-Clause-Patent @@ -61,12 +61,13 @@ > > > > ## This PCD specifies the minimal valid year in RTC. > > # @Prompt Minimal valid year in RTC. > > - > > gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear|1998|UINT16|0x00000 > > 00D > > + # @Expression 0x80000001 | > > gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear >=3D 2000 > > + > > gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear|2000|UINT16|0x00000 > > 00D > > > > ## This PCD specifies the maximal valid year in RTC. > > # @Prompt Maximal valid year in RTC. > > # @Expression 0x80000001 | > > gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear < > > gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear + 100 > > - > > gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear|2097|UINT16|0x00000 > > 00E > > + > > gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear|2099|UINT16|0x00000 > > 00E > > > > [PcdsFixedAtBuild, PcdsPatchableInModule] > > ## Defines the ACPI register set base address. > > diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c > > b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c > > index 52af179417..38a3521dfc 100644 > > --- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c > > +++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c > > @@ -1,7 +1,7 @@ > > /** @file > > RTC Architectural Protocol GUID as defined in DxeCis 0.96. > > > > -Copyright (c) 2006 - 2018, Intel Corporation. All rights > > reserved.
> > +Copyright (c) 2006 - 2020, Intel Corporation. All rights > > +reserved.
> > Copyright (c) 2017, AMD Inc. All rights reserved.
> > > > SPDX-License-Identifier: BSD-2-Clause-Patent @@ -441,8 +441,8 @@ > > PcRtcGetTime ( **/ EFI_STATUS PcRtcSetTime ( > > - IN EFI_TIME *Time, > > - IN PC_RTC_MODULE_GLOBALS *Global > > + IN EFI_TIME *Time, > > + IN OUT PC_RTC_MODULE_GLOBALS *Global > > ) > > { > > EFI_STATUS Status; > > @@ -525,6 +525,8 @@ PcRtcSetTime ( > > // > > if (Global->CenturyRtcAddress !=3D 0) { > > RtcWrite (Global->CenturyRtcAddress, DecimalToBcd8 ((UINT8) > > (RtcTime.Year / 100))); > > + } else { > > + DEBUG ((EFI_D_INFO, "PcRtc: Century RTC Address is not > > + found\n")); > > } > > > > ConvertEfiTimeToRtcTime (&RtcTime, RegisterB); @@ -868,6 +870,9 @@ > > ConvertRtcTimeToEfiTime ( { > > BOOLEAN IsPM; > > UINT8 Century; > > + UINT8 CenturyRtcAddress; > > + > > + CenturyRtcAddress =3D GetCenturyRtcAddress (); > > > > if ((Time->Hour & 0x80) !=3D 0) { > > IsPM =3D TRUE; > > @@ -891,14 +896,12 @@ ConvertRtcTimeToEfiTime ( > > return EFI_INVALID_PARAMETER; > > } > > > > - // > > - // For minimal/maximum year range [1970, 2069], > > - // Century is 19 if RTC year >=3D 70, > > - // Century is 20 otherwise. > > - // > > - Century =3D (UINT8) (PcdGet16 (PcdMinimalValidYear) / 100); > > - if (Time->Year < PcdGet16 (PcdMinimalValidYear) % 100) { > > - Century++; > > + if (CenturyRtcAddress !=3D 0) { > > + Century =3D CheckAndConvertBcd8ToDecimal8 ((UINT8) (RtcRead > > (CenturyRtcAddress))); > > + } else if ((PcdGet16 (PcdMinimalValidYear) / 100) =3D=3D (PcdGet16 > > (PcdMaximalValidYear) / 100)) { > > + Century =3D (UINT8)(PcdGet16 (PcdMinimalValidYear) / 100); } els= e > > + { > > + Century =3D RTC_INIT_CENTURY; > > } > > Time->Year =3D (UINT16) (Century * 100 + Time->Year); > > > > diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.h > > b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.h > > index 47293ce44c..94926fe73e 100644 > > --- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.h > > +++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.h > > @@ -1,7 +1,7 @@ > > /** @file > > Header file for real time clock driver. > > > > -Copyright (c) 2006 - 2018, Intel Corporation. All rights > > reserved.
> > +Copyright (c) 2006 - 2020, Intel Corporation. All rights > > +reserved.
> > Copyright (c) 2017, AMD Inc. All rights reserved.
> > > > SPDX-License-Identifier: BSD-2-Clause-Patent @@ -62,11 +62,12 @@ > > extern PC_RTC_MODULE_GLOBALS mModuleGlobal; // Date and time initial > > values. > > // They are used if the RTC values are invalid during driver > > initialization // -#define RTC_INIT_SECOND 0 -#define RTC_INIT_MINUTE > > 0 > > -#define RTC_INIT_HOUR 0 > > -#define RTC_INIT_DAY 1 > > -#define RTC_INIT_MONTH 1 > > +#define RTC_INIT_SECOND 0 > > +#define RTC_INIT_MINUTE 0 > > +#define RTC_INIT_HOUR 0 > > +#define RTC_INIT_DAY 1 > > +#define RTC_INIT_MONTH 1 > > +#define RTC_INIT_CENTURY 20 > > > > #pragma pack(1) > > // > > @@ -160,8 +161,8 @@ PcRtcInit ( > > **/ > > EFI_STATUS > > PcRtcSetTime ( > > - IN EFI_TIME *Time, > > - IN PC_RTC_MODULE_GLOBALS *Global > > + IN EFI_TIME *Time, > > + IN OUT PC_RTC_MODULE_GLOBALS *Global > > ); > > > > /** > > -- > > 2.17.1.windows.2 > > > > > >=20