From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by mx.groups.io with SMTP id smtpd.web11.650.1587966442441973545 for ; Sun, 26 Apr 2020 22:47:22 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.20, mailfrom: kaix.yau@intel.com) IronPort-SDR: bryPIVeXadzoi4+dMtXLBQHzhbVqf/aiesLdvrrUOA6hl/9f3Nrts3wFprX3az8GcWNmLh8KsY /KxkqIGPzR+A== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Apr 2020 22:47:21 -0700 IronPort-SDR: 8aDe6OqFH92lyK61cIY4zWBJVubaswG3qys5FJrGQd2+fqyhQH84gWFnTsmxsaDUGPmXd1Ftfb HCETNhuP4Vjw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.73,322,1583222400"; d="scan'208";a="367031991" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by fmsmga001.fm.intel.com with ESMTP; 26 Apr 2020 22:47:21 -0700 Received: from fmsmsx603.amr.corp.intel.com (10.18.126.83) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.439.0; Sun, 26 Apr 2020 22:47:21 -0700 Received: from fmsmsx601.amr.corp.intel.com (10.18.126.81) by fmsmsx603.amr.corp.intel.com (10.18.126.83) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Sun, 26 Apr 2020 22:47:20 -0700 Received: from fmsmsx601.amr.corp.intel.com ([10.18.126.81]) by fmsmsx601.amr.corp.intel.com ([10.18.126.81]) with mapi id 15.01.1713.004; Sun, 26 Apr 2020 22:47:20 -0700 From: "Yau, KaiX" 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 displayed in SETUP Thread-Topic: [edk2-devel] [PATCH] PcAtChipsetPkg: Wrong System YEAR displayed in SETUP Thread-Index: AQHV/mhcwW9QFsxhHkms/DTjASpqC6iNGC0A//+XApA= Date: Mon, 27 Apr 2020 05:47:20 +0000 Message-ID: <3f6385f623054e24b44a375765246d2f@intel.com> References: <50102ef8b38c0e5287ab4d1cb8fc636213f67299.1584672719.git.kaix.yau@intel.com> <734D49CCEBEEF84792F5B80ED585239D5C51152B@SHSMSX104.ccr.corp.intel.com> In-Reply-To: <734D49CCEBEEF84792F5B80ED585239D5C51152B@SHSMSX104.ccr.corp.intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-reaction: no-action dlp-version: 11.2.0.6 dlp-product: dlpe-windows x-originating-ip: [10.1.200.100] MIME-Version: 1.0 Return-Path: kaix.yau@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi Ray, 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 c= urrent logic will work. But we still have Y3K bug. That is why we need to read from Hardware. If any reasons you guys doesn't want to read from hardware, at least we ne= ed to change Min. and Max. value ASAP. (you can go ahead to delete the change in PcRtc.c and just submit the PCD = changes) Regards Kai Yau --------------------------------------------------------------------------= --- The root cause is from the following logic (PcRtc.c): Current Default PcdMinimalValidYear =3D 1998 Current Default PcdMaximalValidYear =3D 2097 If we can set the YEAR to 2099, we will end up getting 1999 instead. The= 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 wrong. Regards Kai -----Original Message----- From: Ni, Ray=20 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 displa= yed in SETUP This patch introduces several changes for different purposes. At least I can see: 1. Default PCD value change 2. Consume century value from HW Can you separate the patches to 2 patches? For the purpose #2, can you explain more background/reason? 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? Thanks, Ray > -----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=20 > displayed in SETUP >=20 > Function ConvertRtcTimeToEfiTime() will reset to Century 19XX when the= =20 > year is set larger than the value of PcdMaximalValidYear outside SETUP. >=20 > Signed-off-by: Yau > --- > PcAtChipsetPkg/PcAtChipsetPkg.dec | 7 +++--- > .../PcatRealTimeClockRuntimeDxe/PcRtc.c | 25 +++++++++++-------- > .../PcatRealTimeClockRuntimeDxe/PcRtc.h | 17 +++++++------ > 3 files changed, 27 insertions(+), 22 deletions(-) >=20 > 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=20 > which follows # PcAt defacto standard. > # > -# Copyright (c) 2009 - 2019, Intel Corporation. All rights=20 > reserved.
> +# Copyright (c) 2009 - 2020, Intel Corporation. All rights=20 > +reserved.
> # Copyright (c) 2017, AMD Inc. All rights reserved.
# #=20 > SPDX-License-Identifier: BSD-2-Clause-Patent @@ -61,12 +61,13 @@ >=20 > ## 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 >=20 > ## This PCD specifies the maximal valid year in RTC. > # @Prompt Maximal valid year in RTC. > # @Expression 0x80000001 | > gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear <=20 > gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear + 100 > - > gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear|2097|UINT16|0x00000 > 00E > + > gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear|2099|UINT16|0x00000 > 00E >=20 > [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. >=20 > -Copyright (c) 2006 - 2018, Intel Corporation. All rights=20 > reserved.
> +Copyright (c) 2006 - 2020, Intel Corporation. All rights=20 > +reserved.
> Copyright (c) 2017, AMD Inc. All rights reserved.
>=20 > SPDX-License-Identifier: BSD-2-Clause-Patent @@ -441,8 +441,8 @@=20 > 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)=20 > (RtcTime.Year / 100))); > + } else { > + DEBUG ((EFI_D_INFO, "PcRtc: Century RTC Address is not=20 > + found\n")); > } >=20 > ConvertEfiTimeToRtcTime (&RtcTime, RegisterB); @@ -868,6 +870,9 @@=20 > ConvertRtcTimeToEfiTime ( { > BOOLEAN IsPM; > UINT8 Century; > + UINT8 CenturyRtcAddress; > + > + CenturyRtcAddress =3D GetCenturyRtcAddress (); >=20 > if ((Time->Hour & 0x80) !=3D 0) { > IsPM =3D TRUE; > @@ -891,14 +896,12 @@ ConvertRtcTimeToEfiTime ( > return EFI_INVALID_PARAMETER; > } >=20 > - // > - // 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); } else= =20 > + { > + Century =3D RTC_INIT_CENTURY; > } > Time->Year =3D (UINT16) (Century * 100 + Time->Year); >=20 > 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. >=20 > -Copyright (c) 2006 - 2018, Intel Corporation. All rights=20 > reserved.
> +Copyright (c) 2006 - 2020, Intel Corporation. All rights=20 > +reserved.
> Copyright (c) 2017, AMD Inc. All rights reserved.
>=20 > SPDX-License-Identifier: BSD-2-Clause-Patent @@ -62,11 +62,12 @@=20 > extern PC_RTC_MODULE_GLOBALS mModuleGlobal; // Date and time initial= =20 > values. > // They are used if the RTC values are invalid during driver=20 > initialization // -#define RTC_INIT_SECOND 0 -#define RTC_INIT_MINUTE= =20 > 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 >=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 > ); >=20 > /** > -- > 2.17.1.windows.2 >=20 >=20 >=20