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

2018-01-23 Thread Ni, Ruiyu 

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 0xff with size 0x1000, the high bits 'ff' in base 
0xff will be truncated when UINTN = UINT32.


The code change is good to me, Reviewed-by: Star Zeng 
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 ; Zeng, Star 
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 
Cc: Eric Dong 
Cc: Star Zeng 
---
  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.

+  Copyright (c) 2016 - 2018, Intel Corporation. All rights
+ reserved.
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 UINTNWakeupBufferStart,
-  IN UINTNWakeupBufferEnd
+  IN UINT64   WakeupBufferStart,
+  IN UINT64   WakeupBufferEnd
)
  {
EFI_PEI_HOB_POINTERS  Hob;
EFI_HOB_MEMORY_ALLOCATION *MemoryHob;
BOOLEAN   Overlapped;
-  UINTN MemoryStart;
-  UINTN MemoryEnd;
+  UINT64MemoryStart;
+  UINT64MemoryEnd;
  
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_POINTERSHob;
-  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
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

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

2018-01-22 Thread Zeng, Star 
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 0xff with size 0x1000, the high bits 'ff' in base 
0xff will be truncated when UINTN = UINT32.


The code change is good to me, Reviewed-by: Star Zeng 
How about updating the commit log? :)


Thanks,
Star
-Original Message-
From: Ni, Ruiyu 
Sent: Monday, January 22, 2018 5:17 PM
To: edk2-devel@lists.01.org
Cc: Dong, Eric ; Zeng, Star 
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 
Cc: Eric Dong 
Cc: Star Zeng 
---
 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.
+  Copyright (c) 2016 - 2018, Intel Corporation. All rights 
+ reserved.
   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 UINTNWakeupBufferStart,
-  IN UINTNWakeupBufferEnd
+  IN UINT64   WakeupBufferStart,
+  IN UINT64   WakeupBufferEnd
   )
 {
   EFI_PEI_HOB_POINTERS  Hob;
   EFI_HOB_MEMORY_ALLOCATION *MemoryHob;
   BOOLEAN   Overlapped;
-  UINTN MemoryStart;
-  UINTN MemoryEnd;
+  UINT64MemoryStart;
+  UINT64MemoryEnd;
 
   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_POINTERSHob;
-  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

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel