PcAtChipsetPkg/PcAtChipsetPkg.dec | 7 +++--- .../PcatRealTimeClockRuntimeDxe/PcRtc.c | 25 +++++++++++-------- .../PcatRealTimeClockRuntimeDxe/PcRtc.h | 17 +++++++------ 3 files changed, 27 insertions(+), 22 deletions(-)
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|0x0000000D
+ # @Expression 0x80000001 | gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear >= 2000
+ gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear|2000|UINT16|0x0000000D
## 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|0x0000000E
+ gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear|2099|UINT16|0x0000000E
[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
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#56083): https://edk2.groups.io/g/devel/message/56083
Mute This Topic: https://groups.io/mt/72484290/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
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|0x00000
> 00D
> + # @Expression 0x80000001 |
> gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear >= 2000
> +
> gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear|2000|UINT16|0x00000
> 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|0x00000
> 00E
> +
> gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear|2099|UINT16|0x00000
> 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
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#58160): https://edk2.groups.io/g/devel/message/58160
Mute This Topic: https://groups.io/mt/72484290/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
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|0x00000
> 00D
> + # @Expression 0x80000001 |
> gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear >= 2000
> +
> gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear|2000|UINT16|0x00000
> 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|0x00000
> 00E
> +
> gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear|2099|UINT16|0x00000
> 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
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#58169): https://edk2.groups.io/g/devel/message/58169
Mute This Topic: https://groups.io/mt/72484290/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
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|0x00000
> > 00D
> > + # @Expression 0x80000001 |
> > gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear >= 2000
> > +
> > gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear|2000|UINT16|0x00000
> > 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|0x00000
> > 00E
> > +
> > gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear|2099|UINT16|0x00000
> > 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
> >
> >
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#59229): https://edk2.groups.io/g/devel/message/59229
Mute This Topic: https://groups.io/mt/72484290/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
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
> >
> >
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#59291): https://edk2.groups.io/g/devel/message/59291
Mute This Topic: https://groups.io/mt/72484290/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
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
> > >
> > >
> > >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#59231): https://edk2.groups.io/g/devel/message/59231
Mute This Topic: https://groups.io/mt/72484290/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Hi Ray,
We cannot use 2020 and 2119 if we don't fix the current logic.
If they keep that code, The PCD values have to be "2000 and 2099" (or 1900 and 1999)
Any specific reasons why you don't fix the logic? I will need to update the HSD with your reason.
If we fixed it, we don't need to worry about the PCD values anymore.
Regards
Kai Yau
-----Original Message-----
From: Ni, Ray
Sent: Tuesday, May 12, 2020 1:41 AM
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
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|0x00
> > > 00
> > > 0
> > > 00D
> > > + # @Expression 0x80000001 |
> > > gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear >= 2000
> > > +
> > > gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear|2000|UINT16|0x00
> > > 00
> > > 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|0x00
> > > 00
> > > 0
> > > 00E
> > > +
> > > gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear|2099|UINT16|0x00
> > > 00
> > > 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
> > >
> > >
> > >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#59290): https://edk2.groups.io/g/devel/message/59290
Mute This Topic: https://groups.io/mt/72484290/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Kai,
Can you please explain what logic error in code is wrong?
Thanks,
Ray
> -----Original Message-----
> From: Yau, KaiX <kaix.yau@intel.com>
> Sent: Tuesday, May 12, 2020 1:52 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,
>
> We cannot use 2020 and 2119 if we don't fix the current logic.
>
> If they keep that code, The PCD values have to be "2000 and 2099" (or 1900 and 1999)
>
> Any specific reasons why you don't fix the logic? I will need to update the HSD with your reason.
> If we fixed it, we don't need to worry about the PCD values anymore.
>
> Regards
> Kai Yau
>
>
>
> -----Original Message-----
> From: Ni, Ray
> Sent: Tuesday, May 12, 2020 1:41 AM
> 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
>
> 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|0x00
> > > > 00
> > > > 0
> > > > 00D
> > > > + # @Expression 0x80000001 |
> > > > gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear >= 2000
> > > > +
> > > > gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear|2000|UINT16|0x00
> > > > 00
> > > > 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|0x00
> > > > 00
> > > > 0
> > > > 00E
> > > > +
> > > > gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear|2099|UINT16|0x00
> > > > 00
> > > > 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
> > > >
> > > >
> > > >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#59396): https://edk2.groups.io/g/devel/message/59396
Mute This Topic: https://groups.io/mt/72484290/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Hi,
Lijian Zhou is also able to reproduce the error. You can ask him for more details if you guys are in the same building.
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 PcdMinimalValidYear & PcdMaximalValidYear smaller for testing (such as 1948 and 2047), you can reproduce this issue easily.
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: Tuesday, May 12, 2020 11:27 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
Kai,
Can you please explain what logic error in code is wrong?
Thanks,
Ray
> -----Original Message-----
> From: Yau, KaiX <kaix.yau@intel.com>
> Sent: Tuesday, May 12, 2020 1:52 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,
>
> We cannot use 2020 and 2119 if we don't fix the current logic.
>
> If they keep that code, The PCD values have to be "2000 and 2099" (or
> 1900 and 1999)
>
> Any specific reasons why you don't fix the logic? I will need to update the HSD with your reason.
> If we fixed it, we don't need to worry about the PCD values anymore.
>
> Regards
> Kai Yau
>
>
>
> -----Original Message-----
> From: Ni, Ray
> Sent: Tuesday, May 12, 2020 1:41 AM
> 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
>
> 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|0x
> > > > 00
> > > > 00
> > > > 0
> > > > 00D
> > > > + # @Expression 0x80000001 |
> > > > gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear >= 2000
> > > > +
> > > > gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear|2000|UINT16|0x
> > > > 00
> > > > 00
> > > > 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|0x
> > > > 00
> > > > 00
> > > > 0
> > > > 00E
> > > > +
> > > > gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear|2099|UINT16|0x
> > > > 00
> > > > 00
> > > > 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
> > > >
> > > >
> > > >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#59428): https://edk2.groups.io/g/devel/message/59428
Mute This Topic: https://groups.io/mt/72484290/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Kai,
In your case, the two PCD value should be set to 2020/2119.
With the new PCD value the issue you see will disappear.
I won't approve code changes unless you convince me there is a logic error in code.
+ Lijian
Thanks,
Ray
> -----Original Message-----
> From: Yau, KaiX <kaix.yau@intel.com>
> Sent: Wednesday, May 13, 2020 11:37 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,
>
> Lijian Zhou is also able to reproduce the error. You can ask him for more details if you guys are in the same building.
>
>
> 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 PcdMinimalValidYear &
> PcdMaximalValidYear smaller for testing (such as 1948 and 2047), you can reproduce this issue easily.
>
> 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: Tuesday, May 12, 2020 11:27 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
>
> Kai,
> Can you please explain what logic error in code is wrong?
>
> Thanks,
> Ray
>
> > -----Original Message-----
> > From: Yau, KaiX <kaix.yau@intel.com>
> > Sent: Tuesday, May 12, 2020 1:52 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,
> >
> > We cannot use 2020 and 2119 if we don't fix the current logic.
> >
> > If they keep that code, The PCD values have to be "2000 and 2099" (or
> > 1900 and 1999)
> >
> > Any specific reasons why you don't fix the logic? I will need to update the HSD with your reason.
> > If we fixed it, we don't need to worry about the PCD values anymore.
> >
> > Regards
> > Kai Yau
> >
> >
> >
> > -----Original Message-----
> > From: Ni, Ray
> > Sent: Tuesday, May 12, 2020 1:41 AM
> > 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
> >
> > 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|0x
> > > > > 00
> > > > > 00
> > > > > 0
> > > > > 00D
> > > > > + # @Expression 0x80000001 |
> > > > > gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear >= 2000
> > > > > +
> > > > > gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear|2000|UINT16|0x
> > > > > 00
> > > > > 00
> > > > > 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|0x
> > > > > 00
> > > > > 00
> > > > > 0
> > > > > 00E
> > > > > +
> > > > > gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear|2099|UINT16|0x
> > > > > 00
> > > > > 00
> > > > > 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
> > > > >
> > > > >
> > > > >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#59398): https://edk2.groups.io/g/devel/message/59398
Mute This Topic: https://groups.io/mt/72484290/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
HI Ray,
If you have PCD 2020 and 2119
And the user set 2125 (out of BIOS).
Time->Year = CheckAndConvertBcd8ToDecimal8 ((UINT8) Time->Year); <-- Time->Year = 25
Century = (UINT8) (PcdGet16 (PcdMinimalValidYear) / 100); <-- Century = 20
if (Time->Year < PcdGet16 (PcdMinimalValidYear) % 100) { <-- (25 < 19) = FALSE
Century++; <-- since it is FALSE, we will not get to here
}
Time->Year = (UINT16) (Century * 100 + Time->Year); <-- Time->Year = 2025 (20 x 100 + 25).
2025 is within the RANGE, so it is considered as "VALID" data in BIOS;
however it is wrong because this logic change 2125 to 2025.
If we fixed the PCD values at 2000 and 2099, the current logic is working. That was the original design.
BTW, you can look at gitk. Someone changed them to 1998 and 2097 on purpose after this code are written.
The guy who changed the PCD values does not know the code has problem.
Regards
Kai Yau
-----Original Message-----
From: Ni, Ray
Sent: Wednesday, May 13, 2020 12:46 AM
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>; Zhao, Lijian <lijian.zhao@intel.com>
Subject: RE: [edk2-devel] [PATCH] PcAtChipsetPkg: Wrong System YEAR displayed in SETUP
Kai,
In your case, the two PCD value should be set to 2020/2119.
With the new PCD value the issue you see will disappear.
I won't approve code changes unless you convince me there is a logic error in code.
+ Lijian
Thanks,
Ray
> -----Original Message-----
> From: Yau, KaiX <kaix.yau@intel.com>
> Sent: Wednesday, May 13, 2020 11:37 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,
>
> Lijian Zhou is also able to reproduce the error. You can ask him for more details if you guys are in the same building.
>
>
> 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 PcdMinimalValidYear & PcdMaximalValidYear smaller for testing (such as 1948 and 2047), you can reproduce this issue easily.
>
> 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: Tuesday, May 12, 2020 11:27 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
>
> Kai,
> Can you please explain what logic error in code is wrong?
>
> Thanks,
> Ray
>
> > -----Original Message-----
> > From: Yau, KaiX <kaix.yau@intel.com>
> > Sent: Tuesday, May 12, 2020 1:52 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,
> >
> > We cannot use 2020 and 2119 if we don't fix the current logic.
> >
> > If they keep that code, The PCD values have to be "2000 and 2099"
> > (or
> > 1900 and 1999)
> >
> > Any specific reasons why you don't fix the logic? I will need to update the HSD with your reason.
> > If we fixed it, we don't need to worry about the PCD values anymore.
> >
> > Regards
> > Kai Yau
> >
> >
> >
> > -----Original Message-----
> > From: Ni, Ray
> > Sent: Tuesday, May 12, 2020 1:41 AM
> > 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
> >
> > 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|
> > > > > 0x
> > > > > 00
> > > > > 00
> > > > > 0
> > > > > 00D
> > > > > + # @Expression 0x80000001 |
> > > > > gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear >= 2000
> > > > > +
> > > > > gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear|2000|UINT16|
> > > > > 0x
> > > > > 00
> > > > > 00
> > > > > 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|
> > > > > 0x
> > > > > 00
> > > > > 00
> > > > > 0
> > > > > 00E
> > > > > +
> > > > > gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear|2099|UINT16|
> > > > > 0x
> > > > > 00
> > > > > 00
> > > > > 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
> > > > >
> > > > >
> > > > >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#59429): https://edk2.groups.io/g/devel/message/59429
Mute This Topic: https://groups.io/mt/72484290/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
If the PCD value is 2020/2119, setting year out of this range [2020 - 2119] is not supported.
It's clearly stated in the PCD definition in PcAtChipsetPkg.dec.
The PCD value in PcAtChipsetPkg.dec assumes this driver and RTC HW are not needed in year 2098 and after.
## This PCD specifies the minimal valid year in RTC.
# @Prompt Minimal valid year in RTC.
gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear|1998|UINT16|0x0000000D
## 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|0x0000000E
> -----Original Message-----
> From: Yau, KaiX <kaix.yau@intel.com>
> Sent: Wednesday, May 13, 2020 1:45 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>; Zhao, Lijian <lijian.zhao@intel.com>
> Subject: RE: [edk2-devel] [PATCH] PcAtChipsetPkg: Wrong System YEAR displayed in SETUP
>
> HI Ray,
>
> If you have PCD 2020 and 2119
> And the user set 2125 (out of BIOS).
>
> Time->Year = CheckAndConvertBcd8ToDecimal8 ((UINT8) Time->Year); <-- Time->Year = 25
> Century = (UINT8) (PcdGet16 (PcdMinimalValidYear) / 100); <-- Century = 20
> if (Time->Year < PcdGet16 (PcdMinimalValidYear) % 100) { <-- (25 < 19) = FALSE
> Century++; <-- since it is FALSE, we will not get to here
> }
> Time->Year = (UINT16) (Century * 100 + Time->Year); <-- Time->Year = 2025 (20 x 100 + 25).
> 2025 is within the RANGE, so it is considered as "VALID" data in BIOS;
> however it is wrong because this logic change 2125 to 2025.
>
> If we fixed the PCD values at 2000 and 2099, the current logic is working. That was the original design.
> BTW, you can look at gitk. Someone changed them to 1998 and 2097 on purpose after this code are written.
> The guy who changed the PCD values does not know the code has problem.
>
> Regards
> Kai Yau
>
>
>
>
> -----Original Message-----
> From: Ni, Ray
> Sent: Wednesday, May 13, 2020 12:46 AM
> 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>; Zhao, Lijian <lijian.zhao@intel.com>
> Subject: RE: [edk2-devel] [PATCH] PcAtChipsetPkg: Wrong System YEAR displayed in SETUP
>
> Kai,
> In your case, the two PCD value should be set to 2020/2119.
> With the new PCD value the issue you see will disappear.
> I won't approve code changes unless you convince me there is a logic error in code.
>
> + Lijian
>
> Thanks,
> Ray
>
> > -----Original Message-----
> > From: Yau, KaiX <kaix.yau@intel.com>
> > Sent: Wednesday, May 13, 2020 11:37 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,
> >
> > Lijian Zhou is also able to reproduce the error. You can ask him for more details if you guys are in the same building.
> >
> >
> > 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 PcdMinimalValidYear & PcdMaximalValidYear smaller for testing (such as 1948 and 2047), you can reproduce
> this issue easily.
> >
> > 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: Tuesday, May 12, 2020 11:27 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
> >
> > Kai,
> > Can you please explain what logic error in code is wrong?
> >
> > Thanks,
> > Ray
> >
> > > -----Original Message-----
> > > From: Yau, KaiX <kaix.yau@intel.com>
> > > Sent: Tuesday, May 12, 2020 1:52 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,
> > >
> > > We cannot use 2020 and 2119 if we don't fix the current logic.
> > >
> > > If they keep that code, The PCD values have to be "2000 and 2099"
> > > (or
> > > 1900 and 1999)
> > >
> > > Any specific reasons why you don't fix the logic? I will need to update the HSD with your reason.
> > > If we fixed it, we don't need to worry about the PCD values anymore.
> > >
> > > Regards
> > > Kai Yau
> > >
> > >
> > >
> > > -----Original Message-----
> > > From: Ni, Ray
> > > Sent: Tuesday, May 12, 2020 1:41 AM
> > > 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
> > >
> > > 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|
> > > > > > 0x
> > > > > > 00
> > > > > > 00
> > > > > > 0
> > > > > > 00D
> > > > > > + # @Expression 0x80000001 |
> > > > > > gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear >= 2000
> > > > > > +
> > > > > > gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear|2000|UINT16|
> > > > > > 0x
> > > > > > 00
> > > > > > 00
> > > > > > 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|
> > > > > > 0x
> > > > > > 00
> > > > > > 00
> > > > > > 0
> > > > > > 00E
> > > > > > +
> > > > > > gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear|2099|UINT16|
> > > > > > 0x
> > > > > > 00
> > > > > > 00
> > > > > > 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
> > > > > >
> > > > > >
> > > > > >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#59420): https://edk2.groups.io/g/devel/message/59420
Mute This Topic: https://groups.io/mt/72484290/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Hi,
The current logic has two limitations:
1. it has to be 100 years apart (already mentioned in DEC file)
2. the century numbers have to be same in order to work. (19XX, 20XX will not work) (20xx, 21xx will not work)
Since now is 2020, the value has to be 2000, 2099. But BIOS developers does not know this limitation.
Intel testing team reported that the YEAR in setup changed unexpectedly. That's why I found this issue.
This is just a minor bug because Windows does not rely on CMOS century.
And most servers will pull the Time from time server instead of using CMOS time.
As long as client is using Time server, people may never figure out this problem.
Regards
Kai Yau
-----Original Message-----
From: Ni, Ray <ray.ni@intel.com>
Sent: Wednesday, May 13, 2020 5:03 AM
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>; Zhao, Lijian <lijian.zhao@intel.com>
Subject: RE: [edk2-devel] [PATCH] PcAtChipsetPkg: Wrong System YEAR displayed in SETUP
If the PCD value is 2020/2119, setting year out of this range [2020 - 2119] is not supported.
It's clearly stated in the PCD definition in PcAtChipsetPkg.dec.
The PCD value in PcAtChipsetPkg.dec assumes this driver and RTC HW are not needed in year 2098 and after.
## This PCD specifies the minimal valid year in RTC.
# @Prompt Minimal valid year in RTC.
gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear|1998|UINT16|0x0000000D
## 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|0x0000000E
> -----Original Message-----
> From: Yau, KaiX <kaix.yau@intel.com>
> Sent: Wednesday, May 13, 2020 1:45 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>; Zhao, Lijian <lijian.zhao@intel.com>
> Subject: RE: [edk2-devel] [PATCH] PcAtChipsetPkg: Wrong System YEAR
> displayed in SETUP
>
> HI Ray,
>
> If you have PCD 2020 and 2119
> And the user set 2125 (out of BIOS).
>
> Time->Year = CheckAndConvertBcd8ToDecimal8 ((UINT8) Time->Year); <-- Time->Year = 25
> Century = (UINT8) (PcdGet16 (PcdMinimalValidYear) / 100); <-- Century = 20
> if (Time->Year < PcdGet16 (PcdMinimalValidYear) % 100) { <-- (25 < 19) = FALSE
> Century++; <-- since it is FALSE, we will not get to here
> }
> Time->Year = (UINT16) (Century * 100 + Time->Year); <-- Time->Year = 2025 (20 x 100 + 25).
> 2025 is within the RANGE, so it is considered as "VALID" data in
> BIOS; however it is wrong because this logic change 2125 to 2025.
>
> If we fixed the PCD values at 2000 and 2099, the current logic is working. That was the original design.
> BTW, you can look at gitk. Someone changed them to 1998 and 2097 on purpose after this code are written.
> The guy who changed the PCD values does not know the code has problem.
>
> Regards
> Kai Yau
>
>
>
>
> -----Original Message-----
> From: Ni, Ray
> Sent: Wednesday, May 13, 2020 12:46 AM
> 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>; Zhao, Lijian <lijian.zhao@intel.com>
> Subject: RE: [edk2-devel] [PATCH] PcAtChipsetPkg: Wrong System YEAR
> displayed in SETUP
>
> Kai,
> In your case, the two PCD value should be set to 2020/2119.
> With the new PCD value the issue you see will disappear.
> I won't approve code changes unless you convince me there is a logic error in code.
>
> + Lijian
>
> Thanks,
> Ray
>
> > -----Original Message-----
> > From: Yau, KaiX <kaix.yau@intel.com>
> > Sent: Wednesday, May 13, 2020 11:37 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,
> >
> > Lijian Zhou is also able to reproduce the error. You can ask him for more details if you guys are in the same building.
> >
> >
> > 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 PcdMinimalValidYear & PcdMaximalValidYear smaller for
> > testing (such as 1948 and 2047), you can reproduce
> this issue easily.
> >
> > 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: Tuesday, May 12, 2020 11:27 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
> >
> > Kai,
> > Can you please explain what logic error in code is wrong?
> >
> > Thanks,
> > Ray
> >
> > > -----Original Message-----
> > > From: Yau, KaiX <kaix.yau@intel.com>
> > > Sent: Tuesday, May 12, 2020 1:52 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,
> > >
> > > We cannot use 2020 and 2119 if we don't fix the current logic.
> > >
> > > If they keep that code, The PCD values have to be "2000 and 2099"
> > > (or
> > > 1900 and 1999)
> > >
> > > Any specific reasons why you don't fix the logic? I will need to update the HSD with your reason.
> > > If we fixed it, we don't need to worry about the PCD values anymore.
> > >
> > > Regards
> > > Kai Yau
> > >
> > >
> > >
> > > -----Original Message-----
> > > From: Ni, Ray
> > > Sent: Tuesday, May 12, 2020 1:41 AM
> > > 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
> > >
> > > 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|UINT1
> > > > > > 6|
> > > > > > 0x
> > > > > > 00
> > > > > > 00
> > > > > > 0
> > > > > > 00D
> > > > > > + # @Expression 0x80000001 |
> > > > > > gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear >= 2000
> > > > > > +
> > > > > > gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear|2000|UINT1
> > > > > > 6|
> > > > > > 0x
> > > > > > 00
> > > > > > 00
> > > > > > 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|UINT1
> > > > > > 6|
> > > > > > 0x
> > > > > > 00
> > > > > > 00
> > > > > > 0
> > > > > > 00E
> > > > > > +
> > > > > > gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear|2099|UINT1
> > > > > > 6|
> > > > > > 0x
> > > > > > 00
> > > > > > 00
> > > > > > 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
> > > > > >
> > > > > >
> > > > > >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#59562): https://edk2.groups.io/g/devel/message/59562
Mute This Topic: https://groups.io/mt/72484290/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Kai, I created the initial design and made the initial change: using the two PCD to help code decide the correct century value. More comments below. > -----Original Message----- > From: Yau, KaiX <kaix.yau@intel.com> > Sent: Thursday, May 14, 2020 1:14 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>; Zhao, Lijian <lijian.zhao@intel.com> > Subject: RE: [edk2-devel] [PATCH] PcAtChipsetPkg: Wrong System YEAR displayed in SETUP > > Hi, > > The current logic has two limitations: > 1. it has to be 100 years apart (already mentioned in DEC file) I agree it's a limitation. I don't want to solve this limitation with current RTC HW. > 2. the century numbers have to be same in order to work. (19XX, 20XX will not work) (20xx, 21xx will not work) > Since now is 2020, the value has to be 2000, 2099. But BIOS developers does not know this limitation. I don't agree. Let's forget other argue points and just focus on this point. Can you please explain why with the PCD value (1998,2097) cannot support this year 2020? -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#59482): https://edk2.groups.io/g/devel/message/59482 Mute This Topic: https://groups.io/mt/72484290/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Hi Ray,
I read your comment from gitk.
I understand why you changed them to 1998 and 2097.
May I know why we don't read the Century value from CMOS in the beginning?
Some boards does not support? Or the RTC address may be different for different boards?
Regards
Kai Yau
-----Original Message-----
From: Ni, Ray <ray.ni@intel.com>
Sent: Wednesday, May 13, 2020 10:55 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>; Zhao, Lijian <lijian.zhao@intel.com>
Subject: RE: [edk2-devel] [PATCH] PcAtChipsetPkg: Wrong System YEAR displayed in SETUP
Kai,
I created the initial design and made the initial change: using the two PCD to help code decide the correct century value.
More comments below.
> -----Original Message-----
> From: Yau, KaiX <kaix.yau@intel.com>
> Sent: Thursday, May 14, 2020 1:14 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>; Zhao, Lijian <lijian.zhao@intel.com>
> Subject: RE: [edk2-devel] [PATCH] PcAtChipsetPkg: Wrong System YEAR
> displayed in SETUP
>
> Hi,
>
> The current logic has two limitations:
> 1. it has to be 100 years apart (already mentioned in DEC file)
I agree it's a limitation. I don't want to solve this limitation with current RTC HW.
> 2. the century numbers have to be same in order to work. (19XX, 20XX
> will not work) (20xx, 21xx will not work) Since now is 2020, the value has to be 2000, 2099. But BIOS developers does not know this limitation.
I don't agree. Let's forget other argue points and just focus on this point.
Can you please explain why with the PCD value (1998,2097) cannot support this year 2020?
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#59563): https://edk2.groups.io/g/devel/message/59563
Mute This Topic: https://groups.io/mt/72484290/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Kai, Thanks for telling me the commit message can help understand. RTC HW updates the year/month/day/hour/minute/second every second. But the year stored in RTC is only the last two digits of the 4-digit year. Storing the century (high 2 digits) in CMOS is a choice of platform implementation and cannot work for all the cases. Supposing now we are in the last second of 2099, RTC HW updates the value in RTC to 00(year)/1(month)/1(day)/0(hour)/0(minute)/0(second). But it doesn't update the century value in CMOS leaving that value 20. Then the time will be interpreted as 2000/1/1/0/0/0. That's why RTC driver only updates the century in CMOS when platform reports the century location through the FADT table. Please understand RTC driver updates it because platform requires it (through reporting CMOS location in FADT.). Please also try to understand that the location of century in CMOS is platform dependent. There is no standard to define location for all x86 systems. Thanks, Ray > -----Original Message----- > From: Yau, KaiX <kaix.yau@intel.com> > Sent: Thursday, May 14, 2020 11:36 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>; Zhao, Lijian <lijian.zhao@intel.com> > Subject: RE: [edk2-devel] [PATCH] PcAtChipsetPkg: Wrong System YEAR displayed in SETUP > > Hi Ray, > > I read your comment from gitk. > I understand why you changed them to 1998 and 2097. > > May I know why we don't read the Century value from CMOS in the beginning? > Some boards does not support? Or the RTC address may be different for different boards? > > Regards > Kai Yau > > -----Original Message----- > From: Ni, Ray <ray.ni@intel.com> > Sent: Wednesday, May 13, 2020 10:55 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>; Zhao, Lijian <lijian.zhao@intel.com> > Subject: RE: [edk2-devel] [PATCH] PcAtChipsetPkg: Wrong System YEAR displayed in SETUP > > Kai, > I created the initial design and made the initial change: using the two PCD to help code decide the correct century value. > More comments below. > > > -----Original Message----- > > From: Yau, KaiX <kaix.yau@intel.com> > > Sent: Thursday, May 14, 2020 1:14 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>; Zhao, Lijian <lijian.zhao@intel.com> > > Subject: RE: [edk2-devel] [PATCH] PcAtChipsetPkg: Wrong System YEAR > > displayed in SETUP > > > > Hi, > > > > The current logic has two limitations: > > 1. it has to be 100 years apart (already mentioned in DEC file) > I agree it's a limitation. I don't want to solve this limitation with current RTC HW. > > > 2. the century numbers have to be same in order to work. (19XX, 20XX > > will not work) (20xx, 21xx will not work) Since now is 2020, the value has to be 2000, 2099. But BIOS developers does not > know this limitation. > I don't agree. Let's forget other argue points and just focus on this point. > Can you please explain why with the PCD value (1998,2097) cannot support this year 2020? -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#59491): https://edk2.groups.io/g/devel/message/59491 Mute This Topic: https://groups.io/mt/72484290/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Add commets in your original message
> -----Original Message-----
> From: devel@edk2.groups.io [mailto: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|0x000
> 0000D
> + # @Expression 0x80000001 |
> + gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear >= 2000
Why add above comment in herein? I can get the information from below code, it is redundant information.
> +
> gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear|2000|UINT16|0x000
> 000
> + 0D
>
> ## 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|0x000
> 0000E
> +
> +
> gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear|2099|UINT16|0x000
> 000
> + 0E
>
> [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"));
It is simple so the message is unnecessary, please add debug message for critical situation.
> }
>
> 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)) {
In the practice, the PcdMinimalValidYear and PcdMaximalValidYear is constant, so (PcdGet16(PcdMinimalValidYear))/100) == (PcdGet16(PcdMaximalValidYear)/100) equal 2000/100 == 2099/100 equal 20 == 20, it is always true and so this code is meaningless.
> + Century = (UINT8)(PcdGet16 (PcdMinimalValidYear) / 100); } else {
Arrived here at no time according to the above comment.
> + 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
Please keep the original style, I see you add a space in original code, it is unnecessary.
>
> #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
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#56471): https://edk2.groups.io/g/devel/message/56471
Mute This Topic: https://groups.io/mt/72484290/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.