Re: [edk2-devel] [PATCH] PcAtChipsetPkg: Wrong System YEAR displayed in SETUP

["Ni, Ray" <ray.ni@intel.com>]

I am ok if you change the PCD default value to [2020, 2119].

Please send out separate patch for the PCD default value change if you want.

Thanks,
Ray

> -----Original Message-----
> From: Yau, KaiX <kaix.yau@intel.com>
> Sent: Tuesday, May 12, 2020 10:02 AM
> To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io; kaiyau@gmail.com
> Cc: Jiang, Guomin <guomin.jiang@intel.com>; Gao, Liming <liming.gao@intel.com>; Archield, Ralphal
> <ralphal.archield@intel.com>
> Subject: RE: [edk2-devel] [PATCH] PcAtChipsetPkg: Wrong System YEAR displayed in SETUP
> 
> Hi,
> 
> The current default PCD values has issue with the current logic.
> 
> How do you want to resolve this?
> If you decide not to fix the logic, at least we need to change the Min value to 2000 and Max value to 2099.
> 
> If you agreed, you can remove my logic fix (PcRtc.c) from the Gerrit, and just keep the PCD changes and check-in the code.
> 
> Regards
> Kai Yau
> 
> 
> 
> 
> 
> 
> 
> -----Original Message-----
> From: Ni, Ray
> Sent: Monday, May 11, 2020 9:52 PM
> To: Yau, KaiX <kaix.yau@intel.com>; devel@edk2.groups.io; kaiyau@gmail.com
> Cc: Jiang, Guomin <guomin.jiang@intel.com>; Gao, Liming <liming.gao@intel.com>; Archield, Ralphal
> <ralphal.archield@intel.com>
> Subject: RE: [edk2-devel] [PATCH] PcAtChipsetPkg: Wrong System YEAR displayed in SETUP
> 
> 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 till 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 <kaix.yau@intel.com>
> > Sent: Monday, April 27, 2020 1:47 PM
> > To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io; kaiyau@gmail.com
> > Cc: Jiang, Guomin <guomin.jiang@intel.com>; Gao, Liming
> > <liming.gao@intel.com>; Archield, Ralphal <ralphal.archield@intel.com>
> > Subject: RE: [edk2-devel] [PATCH] PcAtChipsetPkg: Wrong System YEAR
> > displayed in SETUP
> >
> > 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 current 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 need 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 = 1998 Current Default
> > PcdMaximalValidYear = 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.
> >
> >  Time->Year    = CheckAndConvertBcd8ToDecimal8 ((UINT8) Time->Year);   <-- Time->Year  = 99
> >   Century = (UINT8) (PcdGet16 (PcdMinimalValidYear) / 100);            <-- Century = 19
> >   if (Time->Year < PcdGet16 (PcdMinimalValidYear) % 100) {               <-- (99 < 97) = FALSE
> >     Century++;
> >   }
> >   Time->Year = (UINT16) (Century * 100 + Time->Year);                      <-- Time->Year = 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
> > Sent: Monday, April 27, 2020 12:54 AM
> > To: devel@edk2.groups.io; kaiyau@gmail.com
> > Cc: Yau, KaiX <kaix.yau@intel.com>; Jiang, Guomin
> > <guomin.jiang@intel.com>; Gao, Liming <liming.gao@intel.com>
> > Subject: RE: [edk2-devel] [PATCH] PcAtChipsetPkg: Wrong System YEAR
> > displayed 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 <devel@edk2.groups.io> On Behalf Of Yau
> > > Sent: Friday, March 20, 2020 11:42 AM
> > > To: devel@edk2.groups.io
> > > Cc: Yau, KaiX <kaix.yau@intel.com>
> > > 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 <kaix.yau@intel.com>
> > > ---
> > >  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.<BR>
> > > +# Copyright (c) 2009 - 2020, Intel Corporation. All rights
> > > +reserved.<BR>
> > >  # Copyright (c) 2017, AMD Inc. All rights reserved.<BR>  #  #
> > > 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|0x0000
> > > 0
> > > 00D
> > > +  # @Expression 0x80000001 |
> > > gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear >= 2000
> > > +
> > > gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear|2000|UINT16|0x0000
> > > 0
> > > 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|0x0000
> > > 0
> > > 00E
> > > +
> > > gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear|2099|UINT16|0x0000
> > > 0
> > > 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.<BR>
> > > +Copyright (c) 2006 - 2020, Intel Corporation. All rights
> > > +reserved.<BR>
> > >  Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
> > >
> > >  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 != 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 = GetCenturyRtcAddress ();
> > >
> > >    if ((Time->Hour & 0x80) != 0) {
> > >      IsPM = TRUE;
> > > @@ -891,14 +896,12 @@ ConvertRtcTimeToEfiTime (
> > >      return EFI_INVALID_PARAMETER;
> > >    }
> > >
> > > -  //
> > > -  // For minimal/maximum year range [1970, 2069],
> > > -  //   Century is 19 if RTC year >= 70,
> > > -  //   Century is 20 otherwise.
> > > -  //
> > > -  Century = (UINT8) (PcdGet16 (PcdMinimalValidYear) / 100);
> > > -  if (Time->Year < PcdGet16 (PcdMinimalValidYear) % 100) {
> > > -    Century++;
> > > +  if (CenturyRtcAddress != 0) {
> > > +    Century = CheckAndConvertBcd8ToDecimal8 ((UINT8) (RtcRead
> > > (CenturyRtcAddress)));
> > > +  } else if ((PcdGet16 (PcdMinimalValidYear) / 100) == (PcdGet16
> > > (PcdMaximalValidYear) / 100)) {
> > > +    Century = (UINT8)(PcdGet16 (PcdMinimalValidYear) / 100);  }
> > > + else {
> > > +    Century = RTC_INIT_CENTURY;
> > >    }
> > >    Time->Year = (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.<BR>
> > > +Copyright (c) 2006 - 2020, Intel Corporation. All rights
> > > +reserved.<BR>
> > >  Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
> > >
> > >  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
> > >
> > >
> > >