* [Patch 1/1] PcAtChipsetPkg: Add PCD for RTC default year @ 2023-03-26 3:23 Michael D Kinney 2023-03-26 9:29 ` Ni, Ray 0 siblings, 1 reply; 5+ messages in thread From: Michael D Kinney @ 2023-03-26 3:23 UTC (permalink / raw) To: devel; +Cc: Ray Ni 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|0x0000000E + ## 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|gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear|UINT16|0x0000000F + ## Specifies RTC Index Register address in MMIO space. # @Prompt RTC Index Register address gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister64|0x0|UINT64|0x00000022 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_PROMPT #language en-US "ACPI IO Port Base Address Mask" #string STR_gPcAtChipsetPkgTokenSpaceGuid_PcdAcpiIoPortBaseAddressMask_HELP #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/PcatRealTimeClockRuntimeDxe.inf b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf index 0d8eca28b659..c344b059878c 100644 --- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf +++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Patch 1/1] PcAtChipsetPkg: Add PCD for RTC default year 2023-03-26 3:23 [Patch 1/1] PcAtChipsetPkg: Add PCD for RTC default year Michael D Kinney @ 2023-03-26 9:29 ` Ni, Ray 2023-03-26 17:00 ` Michael D Kinney 0 siblings, 1 reply; 5+ messages in thread From: Ni, Ray @ 2023-03-26 9:29 UTC (permalink / raw) To: Kinney, Michael D, devel@edk2.groups.io 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Patch 1/1] PcAtChipsetPkg: Add PCD for RTC default year 2023-03-26 9:29 ` Ni, Ray @ 2023-03-26 17:00 ` Michael D Kinney 2023-03-26 17:06 ` Michael D Kinney 0 siblings, 1 reply; 5+ messages in thread From: Michael D Kinney @ 2023-03-26 17:00 UTC (permalink / raw) To: Ni, Ray, devel@edk2.groups.io; +Cc: Kinney, Michael D 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Patch 1/1] PcAtChipsetPkg: Add PCD for RTC default year 2023-03-26 17:00 ` Michael D Kinney @ 2023-03-26 17:06 ` Michael D Kinney 2023-03-27 0:58 ` Ni, Ray 0 siblings, 1 reply; 5+ messages in thread From: Michael D Kinney @ 2023-03-26 17:06 UTC (permalink / raw) To: Ni, Ray, devel@edk2.groups.io; +Cc: Kinney, Michael D Hi Ray, A second example from the UEFI SCTs (https://github.com/tianocore/edk2-test) Calls UEFI RT service SetTime() with year 2001. https://github.com/tianocore/edk2-test/blob/39f27f233aeab16f71fc8f5ed6b287eb9f884ec6/uefi-sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/TimeServices/BlackBoxTest/TimeServicesBBTestConformance.c#L572 Mike > -----Original Message----- > From: Kinney, Michael D <michael.d.kinney@intel.com> > Sent: Sunday, March 26, 2023 10:00 AM > To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io > Cc: Kinney, Michael D <michael.d.kinney@intel.com> > Subject: RE: [Patch 1/1] PcAtChipsetPkg: Add PCD for RTC default year > > 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Patch 1/1] PcAtChipsetPkg: Add PCD for RTC default year 2023-03-26 17:06 ` Michael D Kinney @ 2023-03-27 0:58 ` Ni, Ray 0 siblings, 0 replies; 5+ messages in thread From: Ni, Ray @ 2023-03-27 0:58 UTC (permalink / raw) To: Kinney, Michael D, devel@edk2.groups.io I see. Thanks for explanation. Reviewed-by: Ray Ni <ray.ni@intel.com> > -----Original Message----- > From: Kinney, Michael D <michael.d.kinney@intel.com> > Sent: Monday, March 27, 2023 1:06 AM > To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io > Cc: Kinney, Michael D <michael.d.kinney@intel.com> > Subject: RE: [Patch 1/1] PcAtChipsetPkg: Add PCD for RTC default year > > Hi Ray, > > A second example from the UEFI SCTs (https://github.com/tianocore/edk2- > test) > > Calls UEFI RT service SetTime() with year 2001. > > https://github.com/tianocore/edk2- > test/blob/39f27f233aeab16f71fc8f5ed6b287eb9f884ec6/uefi- > sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/TimeServices/BlackBoxTest/ > TimeServicesBBTestConformance.c#L572 > > Mike > > > -----Original Message----- > > From: Kinney, Michael D <michael.d.kinney@intel.com> > > Sent: Sunday, March 26, 2023 10:00 AM > > To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io > > Cc: Kinney, Michael D <michael.d.kinney@intel.com> > > Subject: RE: [Patch 1/1] PcAtChipsetPkg: Add PCD for RTC default year > > > > 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/4e8be7ab0cee56a85845d21ef79 > 6aa618a6a83e3/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 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-03-27 0:58 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-03-26 3:23 [Patch 1/1] PcAtChipsetPkg: Add PCD for RTC default year Michael D Kinney 2023-03-26 9:29 ` Ni, Ray 2023-03-26 17:00 ` Michael D Kinney 2023-03-26 17:06 ` Michael D Kinney 2023-03-27 0:58 ` Ni, Ray
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox