[PATCH v2] IntelSiliconPkg/MicrocodeUpdateDxe: Error message enhancement

[PATCH v2] IntelSiliconPkg/MicrocodeUpdateDxe: Error message enhancement

[Shenglei Zhang]

The error message of ExtendedTableCount is not clear. Add the count
number into the debug message.
https://bugzilla.tianocore.org/show_bug.cgi?id=1500

v2: Change the judgment condition to return error message when
    ExtendedTableCount is not equal to target value.

Cc: Ray Ni <ray.ni@intel.com>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
---
 .../Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.c      | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.c b/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.c
index 9098712c2f..037b2433a6 100644
--- a/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.c
+++ b/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.c
@@ -517,8 +517,8 @@ VerifyMicrocode (
           // Checksum correct
           //
           ExtendedTableCount = ExtendedTableHeader->ExtendedSignatureCount;
-          if (ExtendedTableCount > (ExtendedTableLength - sizeof(CPU_MICROCODE_EXTENDED_TABLE_HEADER)) / sizeof(CPU_MICROCODE_EXTENDED_TABLE)) {
-            DEBUG((DEBUG_ERROR, "VerifyMicrocode - ExtendedTableCount too big\n"));
+          if (ExtendedTableCount != (ExtendedTableLength - sizeof(CPU_MICROCODE_EXTENDED_TABLE_HEADER)) / sizeof(CPU_MICROCODE_EXTENDED_TABLE)) {
+            DEBUG((DEBUG_ERROR, "VerifyMicrocode - ExtendedTableCount %d is incorrect\n", ExtendedTableCount));
           } else {
             ExtendedTable = (CPU_MICROCODE_EXTENDED_TABLE *)(ExtendedTableHeader + 1);
             for (Index = 0; Index < ExtendedTableCount; Index++) {
-- 
2.18.0.windows.1

Re: [PATCH v2] IntelSiliconPkg/MicrocodeUpdateDxe: Error message enhancement

[Ni, Ray]

> -----Original Message-----
> From: Zhang, Shenglei <shenglei.zhang@intel.com>
> Sent: Tuesday, February 19, 2019 3:57 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@intel.com>
> Subject: [PATCH v2] IntelSiliconPkg/MicrocodeUpdateDxe: Error message
> enhancement
> 
> The error message of ExtendedTableCount is not clear. Add the count
> number into the debug message.
> https://bugzilla.tianocore.org/show_bug.cgi?id=1500
> 
> v2: Change the judgment condition to return error message when
>     ExtendedTableCount is not equal to target value.
> 
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
> ---
>  .../Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.c      | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git
> a/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdate
> .c
> b/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdate
> .c
> index 9098712c2f..037b2433a6 100644
> ---
> a/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdate
> .c
> +++
> b/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdate
> +++ .c
> @@ -517,8 +517,8 @@ VerifyMicrocode (
>            // Checksum correct
>            //
>            ExtendedTableCount = ExtendedTableHeader-
> >ExtendedSignatureCount;
> -          if (ExtendedTableCount > (ExtendedTableLength -
> sizeof(CPU_MICROCODE_EXTENDED_TABLE_HEADER)) /
> sizeof(CPU_MICROCODE_EXTENDED_TABLE)) {
> -            DEBUG((DEBUG_ERROR, "VerifyMicrocode - ExtendedTableCount too
> big\n"));
> +          if (ExtendedTableCount != (ExtendedTableLength -
> sizeof(CPU_MICROCODE_EXTENDED_TABLE_HEADER)) /
> sizeof(CPU_MICROCODE_EXTENDED_TABLE)) {
> +            DEBUG((DEBUG_ERROR, "VerifyMicrocode - ExtendedTableCount
> + %d is incorrect\n", ExtendedTableCount));

I am not sure if changing ">" to "!=" is ok here.
Because the total size is multiple of 1024, ExtendedTableLength calculated from
the total length may contain padding bytes.
The original ">" is ok because it makes sure the extended table header doesn't
declare more entries than the ExtendedTableLength can cover.
But ExtendedTableLength may be bigger.

So I am wondering this change may cause some originally good uCode be denied.

>            } else {
>              ExtendedTable = (CPU_MICROCODE_EXTENDED_TABLE
> *)(ExtendedTableHeader + 1);
>              for (Index = 0; Index < ExtendedTableCount; Index++) {
> --
> 2.18.0.windows.1