public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] UefiCpuPkg/PeiMpLib: Fix a system hang-in-pei issue.
@ 2018-01-22  9:16 Ruiyu Ni
  2018-01-22  9:36 ` 答复: " Fan Jeff
  2018-01-22 11:02 ` Zeng, Star
  0 siblings, 2 replies; 4+ messages in thread
From: Ruiyu Ni @ 2018-01-22  9:16 UTC (permalink / raw)
  To: edk2-devel; +Cc: Eric Dong, Star Zeng

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



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* 答复: [PATCH] UefiCpuPkg/PeiMpLib: Fix a system hang-in-pei issue.
  2018-01-22  9:16 [PATCH] UefiCpuPkg/PeiMpLib: Fix a system hang-in-pei issue Ruiyu Ni
@ 2018-01-22  9:36 ` Fan Jeff
  2018-01-22 11:02 ` Zeng, Star
  1 sibling, 0 replies; 4+ messages in thread
From: Fan Jeff @ 2018-01-22  9:36 UTC (permalink / raw)
  To: Ruiyu Ni, edk2-devel@lists.01.org; +Cc: Eric Dong, Star Zeng

Reviewed-by: Jeff Fan <vanjeff_919@hotmail.com>

发件人: Ruiyu Ni<mailto:ruiyu.ni@intel.com>
发送时间: 2018年1月22日 17:17
收件人: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
抄送: Eric Dong<mailto:eric.dong@intel.com>; Star Zeng<mailto:star.zeng@intel.com>
主题: [edk2] [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

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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] UefiCpuPkg/PeiMpLib: Fix a system hang-in-pei issue.
  2018-01-22  9:16 [PATCH] UefiCpuPkg/PeiMpLib: Fix a system hang-in-pei issue Ruiyu Ni
  2018-01-22  9:36 ` 答复: " Fan Jeff
@ 2018-01-22 11:02 ` Zeng, Star
  2018-01-24  2:57   ` Ni, Ruiyu
  1 sibling, 1 reply; 4+ messages in thread
From: Zeng, Star @ 2018-01-22 11:02 UTC (permalink / raw)
  To: Ni, Ruiyu, edk2-devel@lists.01.org; +Cc: Dong, Eric, 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 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? :)


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



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] UefiCpuPkg/PeiMpLib: Fix a system hang-in-pei issue.
  2018-01-22 11:02 ` Zeng, Star
@ 2018-01-24  2:57   ` Ni, Ruiyu
  0 siblings, 0 replies; 4+ messages in thread
From: Ni, Ruiyu @ 2018-01-24  2:57 UTC (permalink / raw)
  To: Zeng, Star, edk2-devel@lists.01.org; +Cc: Dong, Eric

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-01-24  2:51 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-01-22  9:16 [PATCH] UefiCpuPkg/PeiMpLib: Fix a system hang-in-pei issue Ruiyu Ni
2018-01-22  9:36 ` 答复: " Fan Jeff
2018-01-22 11:02 ` Zeng, Star
2018-01-24  2:57   ` Ni, Ruiyu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox