Re: [Patch 1/1] PcAtChipsetPkg: Add PCD for RTC default year

["Michael D Kinney" <michael.d.kinney@intel.com>]

Hi Ray,

Firmware tests such as FWTS (https://github.com/ColinIanKing/fwts) have test
Cases for the UEFI time services.  In this case, FWTS has test cases to set
the year to 2012 and 2016.  If the minimum year is set to 2023, these test
cases fail.

https://github.com/ColinIanKing/fwts/blob/4e8be7ab0cee56a85845d21ef796aa618a6a83e3/src/uefi/uefirttime/uefirttime.c#L328

I agree that a reasonable approach is to set the default year to the year that
a system is manufactured.  However, if this is done, the FWTS tests fail
because the current edk2 sources use the same setting for both the minimum
year and the default year.

The min/max year should really be based on the hardware capabilities of the
device that provides time/date information.  The default year should be a
platform policy setting.

Adding a new PCD for the default year provides a platform setting that that
can be set independent of the min/max year hardware capabilities.

Mike

> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Sunday, March 26, 2023 2:30 AM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io
> Subject: RE: [Patch 1/1] PcAtChipsetPkg: Add PCD for RTC default year
> 
> Mike,
> When adding the Min/Max year PCD, the idea was Min year is the default year value.
> The idea assumed that when a PC is shipped to customers the Min year is set to
> the current year.
> 
> Can we explain why need the default year PCD?
> 
> Thanks,
> Ray
> 
> > -----Original Message-----
> > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > Sent: Sunday, March 26, 2023 11:23 AM
> > To: devel@edk2.groups.io
> > Cc: Ni, Ray <ray.ni@intel.com>
> > Subject: [Patch 1/1] PcAtChipsetPkg: Add PCD for RTC default year
> >
> > Add PcdRtcDefaultYear to specify the default year to use when
> > the RTC is in an invalid state. Make sure PcdRtcDefaultYear is
> > >= PcdMinimalValidYear and <= PcdMaximalValidYear.  Set the
> > default value for this PCD to PcdMinimalValidYear to preserve
> > the existing behavior. A platform DSC file can override this
> > default value setting.
> >
> > Cc: Ray Ni <ray.ni@intel.com>
> > Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> > ---
> >  PcAtChipsetPkg/PcAtChipsetPkg.dec                           | 6 ++++++
> >  PcAtChipsetPkg/PcAtChipsetPkg.uni                           | 4 ++++
> >  PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c          | 4 ++--
> >  .../PcatRealTimeClockRuntimeDxe.inf                         | 1 +
> >  4 files changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/PcAtChipsetPkg/PcAtChipsetPkg.dec
> > b/PcAtChipsetPkg/PcAtChipsetPkg.dec
> > index a53ca777e85a..0db385fb901f 100644
> > --- a/PcAtChipsetPkg/PcAtChipsetPkg.dec
> > +++ b/PcAtChipsetPkg/PcAtChipsetPkg.dec
> > @@ -76,6 +76,12 @@ [PcdsFixedAtBuild, PcdsDynamic, PcdsDynamicEx,
> > PcdsPatchableInModule]
> >    # @Expression 0x80000001 |
> > gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear <
> > gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear + 100
> >
> > gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear|2097|UINT16|0x000
> > 0000E
> >
> > +  ## This PCD specifies the RTC default year when the RTC is in an invalid
> > state.
> > +  # @Prompt Default year in RTC.
> > +  # @Expression 0x80000001 |
> > gPcAtChipsetPkgTokenSpaceGuid.PcdRtcDefaultYear >=
> > gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear
> > +  # @Expression 0x80000001 |
> > gPcAtChipsetPkgTokenSpaceGuid.PcdRtcDefaultYear <=
> > gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear
> > +
> > gPcAtChipsetPkgTokenSpaceGuid.PcdRtcDefaultYear|gPcAtChipsetPkgToke
> > nSpaceGuid.PcdMinimalValidYear|UINT16|0x0000000F
> > +
> >    ## Specifies RTC Index Register address in MMIO space.
> >    # @Prompt RTC Index Register address
> >
> > gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister64|0x0|UINT64|0x00
> > 000022
> > diff --git a/PcAtChipsetPkg/PcAtChipsetPkg.uni
> > b/PcAtChipsetPkg/PcAtChipsetPkg.uni
> > index d290dcf1650f..8eb7970c58aa 100644
> > --- a/PcAtChipsetPkg/PcAtChipsetPkg.uni
> > +++ b/PcAtChipsetPkg/PcAtChipsetPkg.uni
> > @@ -79,6 +79,10 @@
> >
> >  #string STR_gPcAtChipsetPkgTokenSpaceGuid_PcdMaximalValidYear_HELP
> > #language en-US "This PCD specifies the maximal valid year in RTC."
> >
> > +#string
> > STR_gPcAtChipsetPkgTokenSpaceGuid_PcdRtcDefaultYear_PROMPT
> > #language en-US "Default year in RTC"
> > +
> > +#string STR_gPcAtChipsetPkgTokenSpaceGuid_PcdRtcDefaultYear_HELP
> > #language en-US "This PCD specifies the RTC default year when the RTC is in
> > an invalid state."
> > +
> >  #string
> > STR_gPcAtChipsetPkgTokenSpaceGuid_PcdAcpiIoPortBaseAddressMask_PR
> > OMPT   #language en-US  "ACPI IO Port Base Address Mask"
> >
> >  #string
> > STR_gPcAtChipsetPkgTokenSpaceGuid_PcdAcpiIoPortBaseAddressMask_HE
> > LP     #language en-US  "Defines the bit mask to retrieve ACPI IO Port Base
> > Address."
> > diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
> > b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
> > index 9242a2e82600..b059e92f02dc 100644
> > --- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
> > +++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
> > @@ -317,7 +317,7 @@ PcRtcInit (
> >      Time.Hour       = RTC_INIT_HOUR;
> >      Time.Day        = RTC_INIT_DAY;
> >      Time.Month      = RTC_INIT_MONTH;
> > -    Time.Year       = PcdGet16 (PcdMinimalValidYear);
> > +    Time.Year       = PcdGet16 (PcdRtcDefaultYear);
> >      Time.Nanosecond = 0;
> >      Time.TimeZone   = EFI_UNSPECIFIED_TIMEZONE;
> >      Time.Daylight   = 0;
> > @@ -357,7 +357,7 @@ PcRtcInit (
> >    Time.Hour       = RTC_INIT_HOUR;
> >    Time.Day        = RTC_INIT_DAY;
> >    Time.Month      = RTC_INIT_MONTH;
> > -  Time.Year       = PcdGet16 (PcdMinimalValidYear);
> > +  Time.Year       = PcdGet16 (PcdRtcDefaultYear);
> >    Time.Nanosecond = 0;
> >    Time.TimeZone   = Global->SavedTimeZone;
> >    Time.Daylight   = Global->Daylight;
> > diff --git
> > a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntim
> > eDxe.inf
> > b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntim
> > eDxe.inf
> > index 0d8eca28b659..c344b059878c 100644
> > ---
> > a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntim
> > eDxe.inf
> > +++
> > b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntim
> > eDxe.inf
> > @@ -76,6 +76,7 @@ [Pcd]
> >    gPcAtChipsetPkgTokenSpaceGuid.PcdRealTimeClockUpdateTimeout   ##
> > CONSUMES
> >    gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear             ##
> > CONSUMES
> >    gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear             ##
> > CONSUMES
> > +  gPcAtChipsetPkgTokenSpaceGuid.PcdRtcDefaultYear               ##
> > CONSUMES
> >    gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister             ##
> > CONSUMES
> >    gPcAtChipsetPkgTokenSpaceGuid.PcdRtcTargetRegister            ##
> > CONSUMES
> >    gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister64           ##
> > CONSUMES
> > --
> > 2.39.1.windows.1