Re: [Patch 1/1] UefiCpuPkg: Remove redundant alignment check when calculate microcode patch size.

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

> -----Original Message-----
> From: Fu, Siyuan
> Sent: Friday, January 03, 2020 3:14 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric; Ni, Ray; Wu, Hao A
> Subject: [Patch 1/1] UefiCpuPkg: Remove redundant alignment check when
> calculate microcode patch size.
> 
> This patch removes the unnecessary alignment check on microcode patch
> TotalSize
> introduced by commit d786a172. The TotalSize has already been checked
> with 1K
> alignment and MAX_ADDRESS in previous code as below:
> 
>     if ( (UINTN)MicrocodeEntryPoint > (MAX_ADDRESS - TotalSize) ||
>          ((UINTN)MicrocodeEntryPoint + TotalSize) > MicrocodeEnd ||
>          (DataSize & 0x3) != 0 ||
>          (TotalSize & (SIZE_1KB - 1)) != 0 ||
>          TotalSize < DataSize
>        ) {
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Signed-off-by: Siyuan Fu <siyuan.fu@intel.com>
> ---
>  UefiCpuPkg/Library/MpInitLib/Microcode.c | 24 +++++-------------------
>  UefiCpuPkg/Library/MpInitLib/MpLib.h     |  3 +--
>  2 files changed, 6 insertions(+), 21 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> index 3da5bfb9cf2f..a9d06dd4099a 100644
> --- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> +++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> @@ -1,7 +1,7 @@
>  /** @file
>    Implementation of loading microcode on processors.
> 
> -  Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2015 - 2020, Intel Corporation. All rights reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -397,16 +397,7 @@ LoadMicrocodePatchWorker (
>        Patches[Index].Size
>        );
> 
> -    //
> -    // Zero-fill the padding area
> -    // Please note that AlignedSize will be no less than Size
> -    //
> -    ZeroMem (
> -      Walker + Patches[Index].Size,
> -      Patches[Index].AlignedSize - Patches[Index].Size
> -      );
> -
> -    Walker += Patches[Index].AlignedSize;
> +    Walker += Patches[Index].Size;
>    }
> 
>    //
> @@ -578,14 +569,9 @@ LoadMicrocodePatch (
>        //
>        // Store the information of this microcode patch
>        //
> -      if (TotalSize > ALIGN_VALUE (TotalSize, SIZE_1KB) ||
> -          ALIGN_VALUE (TotalSize, SIZE_1KB) > MAX_UINTN - TotalLoadSize) {
> -        goto OnExit;
> -      }
> -      PatchInfoBuffer[PatchCount - 1].Address     = (UINTN)
> MicrocodeEntryPoint;
> -      PatchInfoBuffer[PatchCount - 1].Size        = TotalSize;
> -      PatchInfoBuffer[PatchCount - 1].AlignedSize = ALIGN_VALUE (TotalSize,
> SIZE_1KB);
> -      TotalLoadSize += PatchInfoBuffer[PatchCount - 1].AlignedSize;
> +      PatchInfoBuffer[PatchCount - 1].Address = (UINTN)
> MicrocodeEntryPoint;
> +      PatchInfoBuffer[PatchCount - 1].Size    = TotalSize;
> +      TotalLoadSize += TotalSize;
>      }
> 
>      //
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> index 6609c958ce8a..b6e5a1afab00 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> @@ -1,7 +1,7 @@
>  /** @file
>    Common header file for MP Initialize Library.
> 
> -  Copyright (c) 2016 - 2019, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2016 - 2020, Intel Corporation. All rights reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -54,7 +54,6 @@
>  typedef struct {
>    UINTN    Address;
>    UINTN    Size;
> -  UINTN    AlignedSize;
>  } MICROCODE_PATCH_INFO;


Thanks for the catch.
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>

Best Regards,
Hao Wu


> 
>  //
> --
> 2.19.1.windows.1