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

["Wu, Hao A" <hao.a.wu@intel.com>]

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