1. Patchew
  2. EDK2
  3. View series

[edk2] [PATCH] IntelSiliconPkg/MicrocodeUpdateDxe: Refine debug messages

Hao Wu posted 1 patch 6 years, 11 months ago
Failed in applying to current master (apply log)
.../Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.c    | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
[edk2] [PATCH] IntelSiliconPkg/MicrocodeUpdateDxe: Refine debug messages
Posted by Hao Wu 6 years, 11 months ago 
Refine the debug messages during the verification of microcode to make
them more clear.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 .../Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.c    | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.c b/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.c
index b99221c969..6167e0b584 100644
--- a/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.c
+++ b/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.c
@@ -8,7 +8,7 @@
 
   MicrocodeWrite() and VerifyMicrocode() will receive untrusted input and do basic validation.
 
-  Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR>
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD License
   which accompanies this distribution.  The full text of the license may be found at
@@ -437,7 +437,7 @@ VerifyMicrocode (
     return EFI_VOLUME_CORRUPTED;
   }
   if (TotalSize != ImageSize) {
-    DEBUG((DEBUG_ERROR, "VerifyMicrocode - fail on TotalSize\n"));
+    DEBUG((DEBUG_ERROR, "VerifyMicrocode - TotalSize not equal to ImageSize\n"));
     *LastAttemptStatus = LAST_ATTEMPT_STATUS_ERROR_INVALID_FORMAT;
     if (AbortReason != NULL) {
       *AbortReason = AllocateCopyPool(sizeof(L"InvalidTotalSize"), L"InvalidTotalSize");
@@ -496,16 +496,25 @@ VerifyMicrocode (
       //
       if ((ExtendedTableLength > sizeof(CPU_MICROCODE_EXTENDED_TABLE_HEADER)) && ((ExtendedTableLength & 0x3) == 0)) {
         CheckSum32 = CalculateSum32((UINT32 *)ExtendedTableHeader, ExtendedTableLength);
-        if (CheckSum32 == 0) {
+        if (CheckSum32 != 0) {
+          //
+          // Checksum incorrect
+          //
+          DEBUG((DEBUG_ERROR, "VerifyMicrocode - The extended checksum is incorrect\n"));
+        } else {
           //
           // Checksum correct
           //
           ExtendedTableCount = ExtendedTableHeader->ExtendedSignatureCount;
-          if (ExtendedTableCount <= (ExtendedTableLength - sizeof(CPU_MICROCODE_EXTENDED_TABLE_HEADER)) / sizeof(CPU_MICROCODE_EXTENDED_TABLE)) {
+          if (ExtendedTableCount > (ExtendedTableLength - sizeof(CPU_MICROCODE_EXTENDED_TABLE_HEADER)) / sizeof(CPU_MICROCODE_EXTENDED_TABLE)) {
+            DEBUG((DEBUG_ERROR, "VerifyMicrocode - ExtendedTableCount too big\n"));
+          } else {
             ExtendedTable = (CPU_MICROCODE_EXTENDED_TABLE *)(ExtendedTableHeader + 1);
             for (Index = 0; Index < ExtendedTableCount; Index++) {
               CheckSum32 = CalculateSum32((UINT32 *)ExtendedTable, sizeof(CPU_MICROCODE_EXTENDED_TABLE));
-              if (CheckSum32 == 0) {
+              if (CheckSum32 != 0) {
+                DEBUG((DEBUG_ERROR, "VerifyMicrocode - Checksum is incorrect for ExtendedTable with index 0x%x\n", Index));
+              } else {
                 //
                 // Verify Header
                 //
@@ -526,7 +535,7 @@ VerifyMicrocode (
     }
     if (!CorrectMicrocode) {
       if (TryLoad) {
-        DEBUG((DEBUG_ERROR, "VerifyMicrocode - fail on CurrentProcessorSignature/ProcessorFlags\n"));
+        DEBUG((DEBUG_ERROR, "VerifyMicrocode - fail on Current ProcessorSignature/ProcessorFlags\n"));
       }
       *LastAttemptStatus = LAST_ATTEMPT_STATUS_ERROR_INCORRECT_VERSION;
       if (AbortReason != NULL) {
-- 
2.12.0.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] IntelSiliconPkg/MicrocodeUpdateDxe: Refine debug messages
Posted by Zeng, Star 6 years, 10 months ago 
Minor comments.

How about to update the error message like below?
+          DEBUG((DEBUG_ERROR, "VerifyMicrocode - The extended checksum is incorrect\n"));
->
+          DEBUG((DEBUG_ERROR, "VerifyMicrocode - The checksum for extended table is incorrect\n"));

+                DEBUG((DEBUG_ERROR, "VerifyMicrocode - Checksum is incorrect for ExtendedTable with index 0x%x\n", Index));
->
+                DEBUG((DEBUG_ERROR, "VerifyMicrocode - The checksum for ExtendedTable entry with index 0x%x is incorrect \n", Index));

With the comments handled, Reviewed-by: Star Zeng <star.zeng@intel.com>


Thanks,
Star
-----Original Message-----
From: Wu, Hao A 
Sent: Thursday, January 25, 2018 10:19 AM
To: edk2-devel@lists.01.org
Cc: Wu, Hao A <hao.a.wu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: [PATCH] IntelSiliconPkg/MicrocodeUpdateDxe: Refine debug messages

Refine the debug messages during the verification of microcode to make
them more clear.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 .../Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.c    | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.c b/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.c
index b99221c969..6167e0b584 100644
--- a/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.c
+++ b/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.c
@@ -8,7 +8,7 @@
 
   MicrocodeWrite() and VerifyMicrocode() will receive untrusted input and do basic validation.
 
-  Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR>
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD License
   which accompanies this distribution.  The full text of the license may be found at
@@ -437,7 +437,7 @@ VerifyMicrocode (
     return EFI_VOLUME_CORRUPTED;
   }
   if (TotalSize != ImageSize) {
-    DEBUG((DEBUG_ERROR, "VerifyMicrocode - fail on TotalSize\n"));
+    DEBUG((DEBUG_ERROR, "VerifyMicrocode - TotalSize not equal to ImageSize\n"));
     *LastAttemptStatus = LAST_ATTEMPT_STATUS_ERROR_INVALID_FORMAT;
     if (AbortReason != NULL) {
       *AbortReason = AllocateCopyPool(sizeof(L"InvalidTotalSize"), L"InvalidTotalSize");
@@ -496,16 +496,25 @@ VerifyMicrocode (
       //
       if ((ExtendedTableLength > sizeof(CPU_MICROCODE_EXTENDED_TABLE_HEADER)) && ((ExtendedTableLength & 0x3) == 0)) {
         CheckSum32 = CalculateSum32((UINT32 *)ExtendedTableHeader, ExtendedTableLength);
-        if (CheckSum32 == 0) {
+        if (CheckSum32 != 0) {
+          //
+          // Checksum incorrect
+          //
+          DEBUG((DEBUG_ERROR, "VerifyMicrocode - The extended checksum is incorrect\n"));
+        } else {
           //
           // Checksum correct
           //
           ExtendedTableCount = ExtendedTableHeader->ExtendedSignatureCount;
-          if (ExtendedTableCount <= (ExtendedTableLength - sizeof(CPU_MICROCODE_EXTENDED_TABLE_HEADER)) / sizeof(CPU_MICROCODE_EXTENDED_TABLE)) {
+          if (ExtendedTableCount > (ExtendedTableLength - sizeof(CPU_MICROCODE_EXTENDED_TABLE_HEADER)) / sizeof(CPU_MICROCODE_EXTENDED_TABLE)) {
+            DEBUG((DEBUG_ERROR, "VerifyMicrocode - ExtendedTableCount too big\n"));
+          } else {
             ExtendedTable = (CPU_MICROCODE_EXTENDED_TABLE *)(ExtendedTableHeader + 1);
             for (Index = 0; Index < ExtendedTableCount; Index++) {
               CheckSum32 = CalculateSum32((UINT32 *)ExtendedTable, sizeof(CPU_MICROCODE_EXTENDED_TABLE));
-              if (CheckSum32 == 0) {
+              if (CheckSum32 != 0) {
+                DEBUG((DEBUG_ERROR, "VerifyMicrocode - Checksum is incorrect for ExtendedTable with index 0x%x\n", Index));
+              } else {
                 //
                 // Verify Header
                 //
@@ -526,7 +535,7 @@ VerifyMicrocode (
     }
     if (!CorrectMicrocode) {
       if (TryLoad) {
-        DEBUG((DEBUG_ERROR, "VerifyMicrocode - fail on CurrentProcessorSignature/ProcessorFlags\n"));
+        DEBUG((DEBUG_ERROR, "VerifyMicrocode - fail on Current ProcessorSignature/ProcessorFlags\n"));
       }
       *LastAttemptStatus = LAST_ATTEMPT_STATUS_ERROR_INCORRECT_VERSION;
       if (AbortReason != NULL) {
-- 
2.12.0.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] IntelSiliconPkg/MicrocodeUpdateDxe: Refine debug messages
Posted by Wu, Hao A 6 years, 10 months ago 
Sure, I will modify the messages before pushing the change.


Best Regards,
Hao Wu


> -----Original Message-----
> From: Zeng, Star
> Sent: Friday, February 02, 2018 10:15 AM
> To: Wu, Hao A; edk2-devel@lists.01.org
> Cc: Yao, Jiewen; Zeng, Star
> Subject: RE: [PATCH] IntelSiliconPkg/MicrocodeUpdateDxe: Refine debug
> messages
> 
> Minor comments.
> 
> How about to update the error message like below?
> +          DEBUG((DEBUG_ERROR, "VerifyMicrocode - The extended checksum is
> incorrect\n"));
> ->
> +          DEBUG((DEBUG_ERROR, "VerifyMicrocode - The checksum for
> extended table is incorrect\n"));
> 
> +                DEBUG((DEBUG_ERROR, "VerifyMicrocode - Checksum is incorrect
> for ExtendedTable with index 0x%x\n", Index));
> ->
> +                DEBUG((DEBUG_ERROR, "VerifyMicrocode - The checksum for
> ExtendedTable entry with index 0x%x is incorrect \n", Index));
> 
> With the comments handled, Reviewed-by: Star Zeng <star.zeng@intel.com>
> 
> 
> Thanks,
> Star
> -----Original Message-----
> From: Wu, Hao A
> Sent: Thursday, January 25, 2018 10:19 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Zeng, Star <star.zeng@intel.com>
> Subject: [PATCH] IntelSiliconPkg/MicrocodeUpdateDxe: Refine debug messages
> 
> Refine the debug messages during the verification of microcode to make
> them more clear.
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> ---
>  .../Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.c    | 21
> +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git
> a/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.c
> b/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.c
> index b99221c969..6167e0b584 100644
> ---
> a/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.c
> +++
> b/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.c
> @@ -8,7 +8,7 @@
> 
>    MicrocodeWrite() and VerifyMicrocode() will receive untrusted input and do
> basic validation.
> 
> -  Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR>
>    This program and the accompanying materials
>    are licensed and made available under the terms and conditions of the BSD
> License
>    which accompanies this distribution.  The full text of the license may be found
> at
> @@ -437,7 +437,7 @@ VerifyMicrocode (
>      return EFI_VOLUME_CORRUPTED;
>    }
>    if (TotalSize != ImageSize) {
> -    DEBUG((DEBUG_ERROR, "VerifyMicrocode - fail on TotalSize\n"));
> +    DEBUG((DEBUG_ERROR, "VerifyMicrocode - TotalSize not equal to
> ImageSize\n"));
>      *LastAttemptStatus = LAST_ATTEMPT_STATUS_ERROR_INVALID_FORMAT;
>      if (AbortReason != NULL) {
>        *AbortReason = AllocateCopyPool(sizeof(L"InvalidTotalSize"),
> L"InvalidTotalSize");
> @@ -496,16 +496,25 @@ VerifyMicrocode (
>        //
>        if ((ExtendedTableLength >
> sizeof(CPU_MICROCODE_EXTENDED_TABLE_HEADER)) &&
> ((ExtendedTableLength & 0x3) == 0)) {
>          CheckSum32 = CalculateSum32((UINT32 *)ExtendedTableHeader,
> ExtendedTableLength);
> -        if (CheckSum32 == 0) {
> +        if (CheckSum32 != 0) {
> +          //
> +          // Checksum incorrect
> +          //
> +          DEBUG((DEBUG_ERROR, "VerifyMicrocode - The extended checksum is
> incorrect\n"));
> +        } else {
>            //
>            // Checksum correct
>            //
>            ExtendedTableCount = ExtendedTableHeader->ExtendedSignatureCount;
> -          if (ExtendedTableCount <= (ExtendedTableLength -
> sizeof(CPU_MICROCODE_EXTENDED_TABLE_HEADER)) /
> sizeof(CPU_MICROCODE_EXTENDED_TABLE)) {
> +          if (ExtendedTableCount > (ExtendedTableLength -
> sizeof(CPU_MICROCODE_EXTENDED_TABLE_HEADER)) /
> sizeof(CPU_MICROCODE_EXTENDED_TABLE)) {
> +            DEBUG((DEBUG_ERROR, "VerifyMicrocode - ExtendedTableCount too
> big\n"));
> +          } else {
>              ExtendedTable = (CPU_MICROCODE_EXTENDED_TABLE
> *)(ExtendedTableHeader + 1);
>              for (Index = 0; Index < ExtendedTableCount; Index++) {
>                CheckSum32 = CalculateSum32((UINT32 *)ExtendedTable,
> sizeof(CPU_MICROCODE_EXTENDED_TABLE));
> -              if (CheckSum32 == 0) {
> +              if (CheckSum32 != 0) {
> +                DEBUG((DEBUG_ERROR, "VerifyMicrocode - Checksum is incorrect
> for ExtendedTable with index 0x%x\n", Index));
> +              } else {
>                  //
>                  // Verify Header
>                  //
> @@ -526,7 +535,7 @@ VerifyMicrocode (
>      }
>      if (!CorrectMicrocode) {
>        if (TryLoad) {
> -        DEBUG((DEBUG_ERROR, "VerifyMicrocode - fail on
> CurrentProcessorSignature/ProcessorFlags\n"));
> +        DEBUG((DEBUG_ERROR, "VerifyMicrocode - fail on Current
> ProcessorSignature/ProcessorFlags\n"));
>        }
>        *LastAttemptStatus =
> LAST_ATTEMPT_STATUS_ERROR_INCORRECT_VERSION;
>        if (AbortReason != NULL) {
> --
> 2.12.0.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patchew 2.1-devel-8c64711

© 2016 - 2024 Red Hat, Inc.