From: "Michael D Kinney" <michael.d.kinney@intel.com>
To: "Ni, Ray" <ray.ni@intel.com>,
"devel@edk2.groups.io" <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
Date: Sun, 26 Mar 2023 17:06:20 +0000 [thread overview]
Message-ID: <CO1PR11MB49297755E8A7D791F184DD4BD28A9@CO1PR11MB4929.namprd11.prod.outlook.com> (raw)
In-Reply-To: <CO1PR11MB4929D2B7CB841C51033E8381D28A9@CO1PR11MB4929.namprd11.prod.outlook.com>
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
next prev parent reply other threads:[~2023-03-26 17:06 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2023-03-27 0:58 ` Ni, Ray
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CO1PR11MB49297755E8A7D791F184DD4BD28A9@CO1PR11MB4929.namprd11.prod.outlook.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox