public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] IntelSiliconPkg/MicrocodeUpdateDxe: Refine debug messages
@ 2018-01-25  2:18 Hao Wu
       [not found] ` <0C09AFA07DD0434D9E2A0C6AEB0483103BA1ED8B@shsmsx102.ccr.corp.intel.com>
  0 siblings, 1 reply; 2+ messages in thread
From: Hao Wu @ 2018-01-25  2:18 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Jiewen Yao, Star Zeng

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



^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] IntelSiliconPkg/MicrocodeUpdateDxe: Refine debug messages
       [not found] ` <0C09AFA07DD0434D9E2A0C6AEB0483103BA1ED8B@shsmsx102.ccr.corp.intel.com>
@ 2018-02-02  2:37   ` Wu, Hao A
  0 siblings, 0 replies; 2+ messages in thread
From: Wu, Hao A @ 2018-02-02  2:37 UTC (permalink / raw)
  To: Zeng, Star, edk2-devel@lists.01.org; +Cc: Yao, Jiewen

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



^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2018-02-02  2:31 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-01-25  2:18 [PATCH] IntelSiliconPkg/MicrocodeUpdateDxe: Refine debug messages Hao Wu
     [not found] ` <0C09AFA07DD0434D9E2A0C6AEB0483103BA1ED8B@shsmsx102.ccr.corp.intel.com>
2018-02-02  2:37   ` Wu, Hao A

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox