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

Yau posted 1 patch 2 weeks ago
Failed in applying to current master (apply log)
PcAtChipsetPkg/PcAtChipsetPkg.dec             |  7 +++---
.../PcatRealTimeClockRuntimeDxe/PcRtc.c       | 25 +++++++++++--------
.../PcatRealTimeClockRuntimeDxe/PcRtc.h       | 17 +++++++------
3 files changed, 27 insertions(+), 22 deletions(-)

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

Posted by Yau 2 weeks ago
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]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Guomin Jiang 1 week ago
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]
-=-=-=-=-=-=-=-=-=-=-=-