Re: [PATCH] UefiCpuPkg/PeiMpLib: Fix a system hang-in-pei issue.

["Ni, Ruiyu" <ruiyu.ni@Intel.com>]

On 1/22/2018 7:02 PM, Zeng, Star wrote:
> In fact, the failure case is like below.
> 
> GetWakeupBuffer() could find < 1M range, but it will always fail with the check in CheckOverlapWithAllocatedBuffer() when there is a memory allocation hob for a range like base 0xff00000000 with size 0x10000000, the high bits 'ff' in base 0xff00000000 will be truncated when UINTN = UINT32.
> 
> 
> The code change is good to me, Reviewed-by: Star Zeng <star.zeng@intel.com>
> How about updating the commit log? :)

Sure. I will update the commit message.
> 
> 
> Thanks,
> Star
> -----Original Message-----
> From: Ni, Ruiyu
> Sent: Monday, January 22, 2018 5:17 PM
> To: edk2-devel@lists.01.org
> Cc: Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: [PATCH] UefiCpuPkg/PeiMpLib: Fix a system hang-in-pei issue.
> 
> When HOB contains a system memory resource which is above 4GB, the (UINTN) typecast truncates the high-32 bits.
> It causes a memory range above 4GB be used by CpuMpPei code as the waking up buffer.
> 
> The patch fixes this issue by using UINT64 type.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> ---
>   UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 23 +++++++++++------------
>   1 file changed, 11 insertions(+), 12 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> index ad43bd33f5..791ae9db6e 100644
> --- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> @@ -1,7 +1,7 @@
>   /** @file
>     MP initialize support functions for PEI phase.
>   
> -  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 @@ -75,15 +75,15 @@ SaveCpuMpData (  **/  BOOLEAN  CheckOverlapWithAllocatedBuffer (
> -  IN UINTN                WakeupBufferStart,
> -  IN UINTN                WakeupBufferEnd
> +  IN UINT64               WakeupBufferStart,
> +  IN UINT64               WakeupBufferEnd
>     )
>   {
>     EFI_PEI_HOB_POINTERS      Hob;
>     EFI_HOB_MEMORY_ALLOCATION *MemoryHob;
>     BOOLEAN                   Overlapped;
> -  UINTN                     MemoryStart;
> -  UINTN                     MemoryEnd;
> +  UINT64                    MemoryStart;
> +  UINT64                    MemoryEnd;
>   
>     Overlapped = FALSE;
>     //
> @@ -96,9 +96,8 @@ CheckOverlapWithAllocatedBuffer (
>     while (!END_OF_HOB_LIST (Hob)) {
>       if (Hob.Header->HobType == EFI_HOB_TYPE_MEMORY_ALLOCATION) {
>         MemoryHob   = Hob.MemoryAllocation;
> -      MemoryStart = (UINTN) MemoryHob->AllocDescriptor.MemoryBaseAddress;
> -      MemoryEnd   = (UINTN) (MemoryHob->AllocDescriptor.MemoryBaseAddress +
> -                             MemoryHob->AllocDescriptor.MemoryLength);
> +      MemoryStart = MemoryHob->AllocDescriptor.MemoryBaseAddress;
> +      MemoryEnd   = MemoryHob->AllocDescriptor.MemoryBaseAddress + MemoryHob->AllocDescriptor.MemoryLength;
>         if (!((WakeupBufferStart >= MemoryEnd) || (WakeupBufferEnd <= MemoryStart))) {
>           Overlapped = TRUE;
>           break;
> @@ -123,8 +122,8 @@ GetWakeupBuffer (
>     )
>   {
>     EFI_PEI_HOB_POINTERS    Hob;
> -  UINTN                   WakeupBufferStart;
> -  UINTN                   WakeupBufferEnd;
> +  UINT64                  WakeupBufferStart;
> +  UINT64                  WakeupBufferEnd;
>   
>     WakeupBufferSize = (WakeupBufferSize + SIZE_4KB - 1) & ~(SIZE_4KB - 1);
>   
> @@ -149,7 +148,7 @@ GetWakeupBuffer (
>           //
>           // Need memory under 1MB to be collected here
>           //
> -        WakeupBufferEnd = (UINTN) (Hob.ResourceDescriptor->PhysicalStart + Hob.ResourceDescriptor->ResourceLength);
> +        WakeupBufferEnd = Hob.ResourceDescriptor->PhysicalStart +
> + Hob.ResourceDescriptor->ResourceLength;
>           if (WakeupBufferEnd > BASE_1MB) {
>             //
>             // Wakeup buffer should be under 1MB @@ -174,7 +173,7 @@ GetWakeupBuffer (
>             }
>             DEBUG ((DEBUG_INFO, "WakeupBufferStart = %x, WakeupBufferSize = %x\n",
>                                  WakeupBufferStart, WakeupBufferSize));
> -          return WakeupBufferStart;
> +          return (UINTN)WakeupBufferStart;
>           }
>         }
>       }
> --
> 2.15.1.windows.2
> 


-- 
Thanks,
Ray