* 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