[Patch] MdeModulePkg PeiCore: Add error message to describe PEIM load failure

[Patch] MdeModulePkg PeiCore: Add error message to describe PEIM load failure

[Liming Gao]

If PEIM image address doesn't meet with its section alignment, it will
load fail. PeiCore adds more debug message to report it.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao <liming.gao@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
---
 MdeModulePkg/Core/Pei/Image/Image.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Core/Pei/Image/Image.c b/MdeModulePkg/Core/Pei/Image/Image.c
index 1985411..f41d3ac 100644
--- a/MdeModulePkg/Core/Pei/Image/Image.c
+++ b/MdeModulePkg/Core/Pei/Image/Image.c
@@ -1,7 +1,7 @@
 /** @file
   Pei Core Load Image Support
 
-Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 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
@@ -506,6 +506,9 @@ LoadAndRelocatePeCoffImage (
   //
   Status = PeCoffLoaderLoadImage (&ImageContext);
   if (EFI_ERROR (Status)) {
+    if (ImageContext.ImageError == IMAGE_ERROR_INVALID_SECTION_ALIGNMENT) {
+      DEBUG ((DEBUG_ERROR, "PEIM Image Address 0x%11p doesn't meet with section alignment 0x%x.\n", (VOID*)(UINTN)ImageContext.ImageAddress, ImageContext.SectionAlignment));
+    }
     return Status;
   }
   //
@@ -612,6 +615,8 @@ PeiLoadImageLoadImage (
     }
   }
 
+  DEBUG ((DEBUG_INFO, "Loading PEIM %g\n", FileHandle));
+
   //
   // If memory is installed, perform the shadow operations
   //
-- 
2.8.0.windows.1

Re: [Patch] MdeModulePkg PeiCore: Add error message to describe PEIM load failure

[Laszlo Ersek]

On 02/08/18 09:18, Liming Gao wrote:
> If PEIM image address doesn't meet with its section alignment, it will
> load fail. PeiCore adds more debug message to report it.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Liming Gao <liming.gao@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> ---
>  MdeModulePkg/Core/Pei/Image/Image.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Core/Pei/Image/Image.c b/MdeModulePkg/Core/Pei/Image/Image.c
> index 1985411..f41d3ac 100644
> --- a/MdeModulePkg/Core/Pei/Image/Image.c
> +++ b/MdeModulePkg/Core/Pei/Image/Image.c
> @@ -1,7 +1,7 @@
>  /** @file
>    Pei Core Load Image Support
>  
> -Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2006 - 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
> @@ -506,6 +506,9 @@ LoadAndRelocatePeCoffImage (
>    //
>    Status = PeCoffLoaderLoadImage (&ImageContext);
>    if (EFI_ERROR (Status)) {
> +    if (ImageContext.ImageError == IMAGE_ERROR_INVALID_SECTION_ALIGNMENT) {
> +      DEBUG ((DEBUG_ERROR, "PEIM Image Address 0x%11p doesn't meet with section alignment 0x%x.\n", (VOID*)(UINTN)ImageContext.ImageAddress, ImageContext.SectionAlignment));
> +    }

Just curious, is there any particular reason for the 11 chars field
width? Personally I would print ImageContext.ImageAddress (which has
type PHYSICAL_ADDRESS, i.e. UINT64), without any type conversion, and
with the following format string:

  0x%Lx

or maybe

  0x%16Lx

... There's no need to resubmit the patch because of my question; I'm
just curious if I'm missing something.

Thanks!
Laszlo


>      return Status;
>    }
>    //
> @@ -612,6 +615,8 @@ PeiLoadImageLoadImage (
>      }
>    }
>  
> +  DEBUG ((DEBUG_INFO, "Loading PEIM %g\n", FileHandle));
> +
>    //
>    // If memory is installed, perform the shadow operations
>    //
> 

I'

Re: [Patch] MdeModulePkg PeiCore: Add error message to describe PEIM load failure

[Zeng, Star]

Hi Liming,

Because of new added "+  DEBUG ((DEBUG_INFO, "Loading PEIM %g\n", FileHandle));", the debug message will have two "Loading PEIM XXX" for one PEIM, that is very confusing.
Loading PEIM 9B3ADA4F-AE56-4C24-8DEA-F03B7558AE50
Loading PEIM at 0x00002353000 EntryPoint=0x000003C10C5 PcdPeim.efi

How about like below change?
1. Also print FFS GUID when section alignment error.
2. Print FFS GUID when PDB not found.


fb0030045fe4ae561177d9cff01f32088ae9975e
 MdeModulePkg/Core/Pei/Image/Image.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Core/Pei/Image/Image.c b/MdeModulePkg/Core/Pei/Image/Image.c
index 198541128512..179c77ef2194 100644
--- a/MdeModulePkg/Core/Pei/Image/Image.c
+++ b/MdeModulePkg/Core/Pei/Image/Image.c
@@ -1,7 +1,7 @@
 /** @file
   Pei Core Load Image Support
 
-Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 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
@@ -506,6 +506,9 @@ LoadAndRelocatePeCoffImage (
   //
   Status = PeCoffLoaderLoadImage (&ImageContext);
   if (EFI_ERROR (Status)) {
+    if (ImageContext.ImageError == IMAGE_ERROR_INVALID_SECTION_ALIGNMENT) {
+      DEBUG ((DEBUG_ERROR, "PEIM(%g) Image Address 0x%11p doesn't meet with section alignment 0x%x.\n", FileHandle, (VOID*)(UINTN)ImageContext.ImageAddress, ImageContext.SectionAlignment));
+    }
     return Status;
   }
   //
@@ -707,6 +710,11 @@ PeiLoadImageLoadImage (
       }
 
       DEBUG ((EFI_D_INFO | EFI_D_LOAD, "%a", EfiFileName));
+    } else {
+      //
+      // Print FFS GUID when PDB not found.
+      //
+      DEBUG ((EFI_D_INFO | EFI_D_LOAD, "%g", FileHandle));
     }
 
   DEBUG_CODE_END ();


Thanks,
Star
-----Original Message-----
From: Gao, Liming 
Sent: Thursday, February 8, 2018 4:19 PM
To: edk2-devel@lists.01.org
Cc: Zeng, Star <star.zeng@intel.com>
Subject: [Patch] MdeModulePkg PeiCore: Add error message to describe PEIM load failure

If PEIM image address doesn't meet with its section alignment, it will load fail. PeiCore adds more debug message to report it.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao <liming.gao@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
---
 MdeModulePkg/Core/Pei/Image/Image.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Core/Pei/Image/Image.c b/MdeModulePkg/Core/Pei/Image/Image.c
index 1985411..f41d3ac 100644
--- a/MdeModulePkg/Core/Pei/Image/Image.c
+++ b/MdeModulePkg/Core/Pei/Image/Image.c
@@ -1,7 +1,7 @@
 /** @file
   Pei Core Load Image Support
 
-Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 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 @@ -506,6 +506,9 @@ LoadAndRelocatePeCoffImage (
   //
   Status = PeCoffLoaderLoadImage (&ImageContext);
   if (EFI_ERROR (Status)) {
+    if (ImageContext.ImageError == IMAGE_ERROR_INVALID_SECTION_ALIGNMENT) {
+      DEBUG ((DEBUG_ERROR, "PEIM Image Address 0x%11p doesn't meet with section alignment 0x%x.\n", (VOID*)(UINTN)ImageContext.ImageAddress, ImageContext.SectionAlignment));
+    }
     return Status;
   }
   //
@@ -612,6 +615,8 @@ PeiLoadImageLoadImage (
     }
   }
 
+  DEBUG ((DEBUG_INFO, "Loading PEIM %g\n", FileHandle));
+
   //
   // If memory is installed, perform the shadow operations
   //
--
2.8.0.windows.1

Re: [Patch] MdeModulePkg PeiCore: Add error message to describe PEIM load failure

[Gao, Liming]

Laszlo:
  11p address is big to cover most real address. It can align and save the message length. 

Thanks
Liming
>-----Original Message-----
>From: Laszlo Ersek [mailto:lersek@redhat.com]
>Sent: Thursday, February 08, 2018 5:31 PM
>To: Gao, Liming <liming.gao@intel.com>; edk2-devel@lists.01.org
>Cc: Zeng, Star <star.zeng@intel.com>
>Subject: Re: [edk2] [Patch] MdeModulePkg PeiCore: Add error message to
>describe PEIM load failure
>
>On 02/08/18 09:18, Liming Gao wrote:
>> If PEIM image address doesn't meet with its section alignment, it will
>> load fail. PeiCore adds more debug message to report it.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Liming Gao <liming.gao@intel.com>
>> Cc: Star Zeng <star.zeng@intel.com>
>> ---
>>  MdeModulePkg/Core/Pei/Image/Image.c | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/MdeModulePkg/Core/Pei/Image/Image.c
>b/MdeModulePkg/Core/Pei/Image/Image.c
>> index 1985411..f41d3ac 100644
>> --- a/MdeModulePkg/Core/Pei/Image/Image.c
>> +++ b/MdeModulePkg/Core/Pei/Image/Image.c
>> @@ -1,7 +1,7 @@
>>  /** @file
>>    Pei Core Load Image Support
>>
>> -Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
>> +Copyright (c) 2006 - 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
>> @@ -506,6 +506,9 @@ LoadAndRelocatePeCoffImage (
>>    //
>>    Status = PeCoffLoaderLoadImage (&ImageContext);
>>    if (EFI_ERROR (Status)) {
>> +    if (ImageContext.ImageError ==
>IMAGE_ERROR_INVALID_SECTION_ALIGNMENT) {
>> +      DEBUG ((DEBUG_ERROR, "PEIM Image Address 0x%11p doesn't meet
>with section alignment 0x%x.\n",
>(VOID*)(UINTN)ImageContext.ImageAddress,
>ImageContext.SectionAlignment));
>> +    }
>
>Just curious, is there any particular reason for the 11 chars field
>width? Personally I would print ImageContext.ImageAddress (which has
>type PHYSICAL_ADDRESS, i.e. UINT64), without any type conversion, and
>with the following format string:
>
>  0x%Lx
>
>or maybe
>
>  0x%16Lx
>
>... There's no need to resubmit the patch because of my question; I'm
>just curious if I'm missing something.
>
>Thanks!
>Laszlo
>
>
>>      return Status;
>>    }
>>    //
>> @@ -612,6 +615,8 @@ PeiLoadImageLoadImage (
>>      }
>>    }
>>
>> +  DEBUG ((DEBUG_INFO, "Loading PEIM %g\n", FileHandle));
>> +
>>    //
>>    // If memory is installed, perform the shadow operations
>>    //
>>
>
>I'

Re: [Patch] MdeModulePkg PeiCore: Add error message to describe PEIM load failure

[Gao, Liming]

Star:
  This change is to align Loading Driver in DXE phase. You can find below message in DXE boot. 

Loading driver 80CF7257-87AB-47F9-A3FE-D50B76D89541
InstallProtocolInterface: 5B1B31A1-9562-11D2-8E3F-00A0C969723B 88014240
Loading driver at 0x00087FE6000 EntryPoint=0x00087FE70B4 PcdDxe.efi

Thanks
Liming
>-----Original Message-----
>From: Zeng, Star
>Sent: Thursday, February 08, 2018 6:12 PM
>To: Gao, Liming <liming.gao@intel.com>; edk2-devel@lists.01.org
>Cc: Zeng, Star <star.zeng@intel.com>
>Subject: RE: [Patch] MdeModulePkg PeiCore: Add error message to describe
>PEIM load failure
>
>Hi Liming,
>
>Because of new added "+  DEBUG ((DEBUG_INFO, "Loading PEIM %g\n",
>FileHandle));", the debug message will have two "Loading PEIM XXX" for one
>PEIM, that is very confusing.
>Loading PEIM 9B3ADA4F-AE56-4C24-8DEA-F03B7558AE50
>Loading PEIM at 0x00002353000 EntryPoint=0x000003C10C5 PcdPeim.efi
>
>How about like below change?
>1. Also print FFS GUID when section alignment error.
>2. Print FFS GUID when PDB not found.
>
>
>fb0030045fe4ae561177d9cff01f32088ae9975e
> MdeModulePkg/Core/Pei/Image/Image.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
>diff --git a/MdeModulePkg/Core/Pei/Image/Image.c
>b/MdeModulePkg/Core/Pei/Image/Image.c
>index 198541128512..179c77ef2194 100644
>--- a/MdeModulePkg/Core/Pei/Image/Image.c
>+++ b/MdeModulePkg/Core/Pei/Image/Image.c
>@@ -1,7 +1,7 @@
> /** @file
>   Pei Core Load Image Support
>
>-Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
>+Copyright (c) 2006 - 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
>@@ -506,6 +506,9 @@ LoadAndRelocatePeCoffImage (
>   //
>   Status = PeCoffLoaderLoadImage (&ImageContext);
>   if (EFI_ERROR (Status)) {
>+    if (ImageContext.ImageError ==
>IMAGE_ERROR_INVALID_SECTION_ALIGNMENT) {
>+      DEBUG ((DEBUG_ERROR, "PEIM(%g) Image Address 0x%11p doesn't
>meet with section alignment 0x%x.\n", FileHandle,
>(VOID*)(UINTN)ImageContext.ImageAddress,
>ImageContext.SectionAlignment));
>+    }
>     return Status;
>   }
>   //
>@@ -707,6 +710,11 @@ PeiLoadImageLoadImage (
>       }
>
>       DEBUG ((EFI_D_INFO | EFI_D_LOAD, "%a", EfiFileName));
>+    } else {
>+      //
>+      // Print FFS GUID when PDB not found.
>+      //
>+      DEBUG ((EFI_D_INFO | EFI_D_LOAD, "%g", FileHandle));
>     }
>
>   DEBUG_CODE_END ();
>
>
>Thanks,
>Star
>-----Original Message-----
>From: Gao, Liming
>Sent: Thursday, February 8, 2018 4:19 PM
>To: edk2-devel@lists.01.org
>Cc: Zeng, Star <star.zeng@intel.com>
>Subject: [Patch] MdeModulePkg PeiCore: Add error message to describe
>PEIM load failure
>
>If PEIM image address doesn't meet with its section alignment, it will load fail.
>PeiCore adds more debug message to report it.
>
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: Liming Gao <liming.gao@intel.com>
>Cc: Star Zeng <star.zeng@intel.com>
>---
> MdeModulePkg/Core/Pei/Image/Image.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
>diff --git a/MdeModulePkg/Core/Pei/Image/Image.c
>b/MdeModulePkg/Core/Pei/Image/Image.c
>index 1985411..f41d3ac 100644
>--- a/MdeModulePkg/Core/Pei/Image/Image.c
>+++ b/MdeModulePkg/Core/Pei/Image/Image.c
>@@ -1,7 +1,7 @@
> /** @file
>   Pei Core Load Image Support
>
>-Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
>+Copyright (c) 2006 - 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
>@@ -506,6 +506,9 @@ LoadAndRelocatePeCoffImage (
>   //
>   Status = PeCoffLoaderLoadImage (&ImageContext);
>   if (EFI_ERROR (Status)) {
>+    if (ImageContext.ImageError ==
>IMAGE_ERROR_INVALID_SECTION_ALIGNMENT) {
>+      DEBUG ((DEBUG_ERROR, "PEIM Image Address 0x%11p doesn't meet
>with section alignment 0x%x.\n",
>(VOID*)(UINTN)ImageContext.ImageAddress,
>ImageContext.SectionAlignment));
>+    }
>     return Status;
>   }
>   //
>@@ -612,6 +615,8 @@ PeiLoadImageLoadImage (
>     }
>   }
>
>+  DEBUG ((DEBUG_INFO, "Loading PEIM %g\n", FileHandle));
>+
>   //
>   // If memory is installed, perform the shadow operations
>   //
>--
>2.8.0.windows.1

Re: [Patch] MdeModulePkg PeiCore: Add error message to describe PEIM load failure

[Zeng, Star]

How about combining it to one for DXE?
Either print FFS GUID or file name from PDB path, or both.

Thanks,
Star
-----Original Message-----
From: Gao, Liming 
Sent: Thursday, February 8, 2018 6:25 PM
To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
Subject: RE: [Patch] MdeModulePkg PeiCore: Add error message to describe PEIM load failure

Star:
  This change is to align Loading Driver in DXE phase. You can find below message in DXE boot. 

Loading driver 80CF7257-87AB-47F9-A3FE-D50B76D89541
InstallProtocolInterface: 5B1B31A1-9562-11D2-8E3F-00A0C969723B 88014240
Loading driver at 0x00087FE6000 EntryPoint=0x00087FE70B4 PcdDxe.efi

Thanks
Liming
>-----Original Message-----
>From: Zeng, Star
>Sent: Thursday, February 08, 2018 6:12 PM
>To: Gao, Liming <liming.gao@intel.com>; edk2-devel@lists.01.org
>Cc: Zeng, Star <star.zeng@intel.com>
>Subject: RE: [Patch] MdeModulePkg PeiCore: Add error message to describe
>PEIM load failure
>
>Hi Liming,
>
>Because of new added "+  DEBUG ((DEBUG_INFO, "Loading PEIM %g\n",
>FileHandle));", the debug message will have two "Loading PEIM XXX" for one
>PEIM, that is very confusing.
>Loading PEIM 9B3ADA4F-AE56-4C24-8DEA-F03B7558AE50
>Loading PEIM at 0x00002353000 EntryPoint=0x000003C10C5 PcdPeim.efi
>
>How about like below change?
>1. Also print FFS GUID when section alignment error.
>2. Print FFS GUID when PDB not found.
>
>
>fb0030045fe4ae561177d9cff01f32088ae9975e
> MdeModulePkg/Core/Pei/Image/Image.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
>diff --git a/MdeModulePkg/Core/Pei/Image/Image.c
>b/MdeModulePkg/Core/Pei/Image/Image.c
>index 198541128512..179c77ef2194 100644
>--- a/MdeModulePkg/Core/Pei/Image/Image.c
>+++ b/MdeModulePkg/Core/Pei/Image/Image.c
>@@ -1,7 +1,7 @@
> /** @file
>   Pei Core Load Image Support
>
>-Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
>+Copyright (c) 2006 - 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
>@@ -506,6 +506,9 @@ LoadAndRelocatePeCoffImage (
>   //
>   Status = PeCoffLoaderLoadImage (&ImageContext);
>   if (EFI_ERROR (Status)) {
>+    if (ImageContext.ImageError ==
>IMAGE_ERROR_INVALID_SECTION_ALIGNMENT) {
>+      DEBUG ((DEBUG_ERROR, "PEIM(%g) Image Address 0x%11p doesn't
>meet with section alignment 0x%x.\n", FileHandle,
>(VOID*)(UINTN)ImageContext.ImageAddress,
>ImageContext.SectionAlignment));
>+    }
>     return Status;
>   }
>   //
>@@ -707,6 +710,11 @@ PeiLoadImageLoadImage (
>       }
>
>       DEBUG ((EFI_D_INFO | EFI_D_LOAD, "%a", EfiFileName));
>+    } else {
>+      //
>+      // Print FFS GUID when PDB not found.
>+      //
>+      DEBUG ((EFI_D_INFO | EFI_D_LOAD, "%g", FileHandle));
>     }
>
>   DEBUG_CODE_END ();
>
>
>Thanks,
>Star
>-----Original Message-----
>From: Gao, Liming
>Sent: Thursday, February 8, 2018 4:19 PM
>To: edk2-devel@lists.01.org
>Cc: Zeng, Star <star.zeng@intel.com>
>Subject: [Patch] MdeModulePkg PeiCore: Add error message to describe
>PEIM load failure
>
>If PEIM image address doesn't meet with its section alignment, it will load fail.
>PeiCore adds more debug message to report it.
>
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: Liming Gao <liming.gao@intel.com>
>Cc: Star Zeng <star.zeng@intel.com>
>---
> MdeModulePkg/Core/Pei/Image/Image.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
>diff --git a/MdeModulePkg/Core/Pei/Image/Image.c
>b/MdeModulePkg/Core/Pei/Image/Image.c
>index 1985411..f41d3ac 100644
>--- a/MdeModulePkg/Core/Pei/Image/Image.c
>+++ b/MdeModulePkg/Core/Pei/Image/Image.c
>@@ -1,7 +1,7 @@
> /** @file
>   Pei Core Load Image Support
>
>-Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
>+Copyright (c) 2006 - 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
>@@ -506,6 +506,9 @@ LoadAndRelocatePeCoffImage (
>   //
>   Status = PeCoffLoaderLoadImage (&ImageContext);
>   if (EFI_ERROR (Status)) {
>+    if (ImageContext.ImageError ==
>IMAGE_ERROR_INVALID_SECTION_ALIGNMENT) {
>+      DEBUG ((DEBUG_ERROR, "PEIM Image Address 0x%11p doesn't meet
>with section alignment 0x%x.\n",
>(VOID*)(UINTN)ImageContext.ImageAddress,
>ImageContext.SectionAlignment));
>+    }
>     return Status;
>   }
>   //
>@@ -612,6 +615,8 @@ PeiLoadImageLoadImage (
>     }
>   }
>
>+  DEBUG ((DEBUG_INFO, "Loading PEIM %g\n", FileHandle));
>+
>   //
>   // If memory is installed, perform the shadow operations
>   //
>--
>2.8.0.windows.1

Re: [Patch] MdeModulePkg PeiCore: Add error message to describe PEIM load failure

[Laszlo Ersek]

On 02/08/18 11:24, Gao, Liming wrote:
> Laszlo:
>   11p address is big to cover most real address. It can align and save the message length. 

Good point, it can go up to 2^44 - 1, covering 16 TB of RAM.

Thanks
Laszlo

> Thanks
> Liming
>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>> Sent: Thursday, February 08, 2018 5:31 PM
>> To: Gao, Liming <liming.gao@intel.com>; edk2-devel@lists.01.org
>> Cc: Zeng, Star <star.zeng@intel.com>
>> Subject: Re: [edk2] [Patch] MdeModulePkg PeiCore: Add error message to
>> describe PEIM load failure
>>
>> On 02/08/18 09:18, Liming Gao wrote:
>>> If PEIM image address doesn't meet with its section alignment, it will
>>> load fail. PeiCore adds more debug message to report it.
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Liming Gao <liming.gao@intel.com>
>>> Cc: Star Zeng <star.zeng@intel.com>
>>> ---
>>>  MdeModulePkg/Core/Pei/Image/Image.c | 7 ++++++-
>>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/MdeModulePkg/Core/Pei/Image/Image.c
>> b/MdeModulePkg/Core/Pei/Image/Image.c
>>> index 1985411..f41d3ac 100644
>>> --- a/MdeModulePkg/Core/Pei/Image/Image.c
>>> +++ b/MdeModulePkg/Core/Pei/Image/Image.c
>>> @@ -1,7 +1,7 @@
>>>  /** @file
>>>    Pei Core Load Image Support
>>>
>>> -Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
>>> +Copyright (c) 2006 - 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
>>> @@ -506,6 +506,9 @@ LoadAndRelocatePeCoffImage (
>>>    //
>>>    Status = PeCoffLoaderLoadImage (&ImageContext);
>>>    if (EFI_ERROR (Status)) {
>>> +    if (ImageContext.ImageError ==
>> IMAGE_ERROR_INVALID_SECTION_ALIGNMENT) {
>>> +      DEBUG ((DEBUG_ERROR, "PEIM Image Address 0x%11p doesn't meet
>> with section alignment 0x%x.\n",
>> (VOID*)(UINTN)ImageContext.ImageAddress,
>> ImageContext.SectionAlignment));
>>> +    }
>>
>> Just curious, is there any particular reason for the 11 chars field
>> width? Personally I would print ImageContext.ImageAddress (which has
>> type PHYSICAL_ADDRESS, i.e. UINT64), without any type conversion, and
>> with the following format string:
>>
>>  0x%Lx
>>
>> or maybe
>>
>>  0x%16Lx
>>
>> ... There's no need to resubmit the patch because of my question; I'm
>> just curious if I'm missing something.
>>
>> Thanks!
>> Laszlo
>>
>>
>>>      return Status;
>>>    }
>>>    //
>>> @@ -612,6 +615,8 @@ PeiLoadImageLoadImage (
>>>      }
>>>    }
>>>
>>> +  DEBUG ((DEBUG_INFO, "Loading PEIM %g\n", FileHandle));
>>> +
>>>    //
>>>    // If memory is installed, perform the shadow operations
>>>    //
>>>
>>
>> I'

Re: [Patch] MdeModulePkg PeiCore: Add error message to describe PEIM load failure

[Gao, Liming]

They are reported in different phase. The first message is print before load image. The second message is print after load image successfully. If load image failure, only first message is print. So, I don't think we can combine them. 

>-----Original Message-----
>From: Zeng, Star
>Sent: Thursday, February 08, 2018 6:31 PM
>To: Gao, Liming <liming.gao@intel.com>; edk2-devel@lists.01.org
>Cc: Zeng, Star <star.zeng@intel.com>
>Subject: RE: [Patch] MdeModulePkg PeiCore: Add error message to describe
>PEIM load failure
>
>How about combining it to one for DXE?
>Either print FFS GUID or file name from PDB path, or both.
>
>Thanks,
>Star
>-----Original Message-----
>From: Gao, Liming
>Sent: Thursday, February 8, 2018 6:25 PM
>To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
>Subject: RE: [Patch] MdeModulePkg PeiCore: Add error message to describe
>PEIM load failure
>
>Star:
>  This change is to align Loading Driver in DXE phase. You can find below
>message in DXE boot.
>
>Loading driver 80CF7257-87AB-47F9-A3FE-D50B76D89541
>InstallProtocolInterface: 5B1B31A1-9562-11D2-8E3F-00A0C969723B 88014240
>Loading driver at 0x00087FE6000 EntryPoint=0x00087FE70B4 PcdDxe.efi
>
>Thanks
>Liming
>>-----Original Message-----
>>From: Zeng, Star
>>Sent: Thursday, February 08, 2018 6:12 PM
>>To: Gao, Liming <liming.gao@intel.com>; edk2-devel@lists.01.org
>>Cc: Zeng, Star <star.zeng@intel.com>
>>Subject: RE: [Patch] MdeModulePkg PeiCore: Add error message to describe
>>PEIM load failure
>>
>>Hi Liming,
>>
>>Because of new added "+  DEBUG ((DEBUG_INFO, "Loading PEIM %g\n",
>>FileHandle));", the debug message will have two "Loading PEIM XXX" for one
>>PEIM, that is very confusing.
>>Loading PEIM 9B3ADA4F-AE56-4C24-8DEA-F03B7558AE50
>>Loading PEIM at 0x00002353000 EntryPoint=0x000003C10C5 PcdPeim.efi
>>
>>How about like below change?
>>1. Also print FFS GUID when section alignment error.
>>2. Print FFS GUID when PDB not found.
>>
>>
>>fb0030045fe4ae561177d9cff01f32088ae9975e
>> MdeModulePkg/Core/Pei/Image/Image.c | 10 +++++++++-
>> 1 file changed, 9 insertions(+), 1 deletion(-)
>>
>>diff --git a/MdeModulePkg/Core/Pei/Image/Image.c
>>b/MdeModulePkg/Core/Pei/Image/Image.c
>>index 198541128512..179c77ef2194 100644
>>--- a/MdeModulePkg/Core/Pei/Image/Image.c
>>+++ b/MdeModulePkg/Core/Pei/Image/Image.c
>>@@ -1,7 +1,7 @@
>> /** @file
>>   Pei Core Load Image Support
>>
>>-Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
>>+Copyright (c) 2006 - 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
>>@@ -506,6 +506,9 @@ LoadAndRelocatePeCoffImage (
>>   //
>>   Status = PeCoffLoaderLoadImage (&ImageContext);
>>   if (EFI_ERROR (Status)) {
>>+    if (ImageContext.ImageError ==
>>IMAGE_ERROR_INVALID_SECTION_ALIGNMENT) {
>>+      DEBUG ((DEBUG_ERROR, "PEIM(%g) Image Address 0x%11p doesn't
>>meet with section alignment 0x%x.\n", FileHandle,
>>(VOID*)(UINTN)ImageContext.ImageAddress,
>>ImageContext.SectionAlignment));
>>+    }
>>     return Status;
>>   }
>>   //
>>@@ -707,6 +710,11 @@ PeiLoadImageLoadImage (
>>       }
>>
>>       DEBUG ((EFI_D_INFO | EFI_D_LOAD, "%a", EfiFileName));
>>+    } else {
>>+      //
>>+      // Print FFS GUID when PDB not found.
>>+      //
>>+      DEBUG ((EFI_D_INFO | EFI_D_LOAD, "%g", FileHandle));
>>     }
>>
>>   DEBUG_CODE_END ();
>>
>>
>>Thanks,
>>Star
>>-----Original Message-----
>>From: Gao, Liming
>>Sent: Thursday, February 8, 2018 4:19 PM
>>To: edk2-devel@lists.01.org
>>Cc: Zeng, Star <star.zeng@intel.com>
>>Subject: [Patch] MdeModulePkg PeiCore: Add error message to describe
>>PEIM load failure
>>
>>If PEIM image address doesn't meet with its section alignment, it will load fail.
>>PeiCore adds more debug message to report it.
>>
>>Contributed-under: TianoCore Contribution Agreement 1.1
>>Signed-off-by: Liming Gao <liming.gao@intel.com>
>>Cc: Star Zeng <star.zeng@intel.com>
>>---
>> MdeModulePkg/Core/Pei/Image/Image.c | 7 ++++++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>>diff --git a/MdeModulePkg/Core/Pei/Image/Image.c
>>b/MdeModulePkg/Core/Pei/Image/Image.c
>>index 1985411..f41d3ac 100644
>>--- a/MdeModulePkg/Core/Pei/Image/Image.c
>>+++ b/MdeModulePkg/Core/Pei/Image/Image.c
>>@@ -1,7 +1,7 @@
>> /** @file
>>   Pei Core Load Image Support
>>
>>-Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
>>+Copyright (c) 2006 - 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
>>@@ -506,6 +506,9 @@ LoadAndRelocatePeCoffImage (
>>   //
>>   Status = PeCoffLoaderLoadImage (&ImageContext);
>>   if (EFI_ERROR (Status)) {
>>+    if (ImageContext.ImageError ==
>>IMAGE_ERROR_INVALID_SECTION_ALIGNMENT) {
>>+      DEBUG ((DEBUG_ERROR, "PEIM Image Address 0x%11p doesn't meet
>>with section alignment 0x%x.\n",
>>(VOID*)(UINTN)ImageContext.ImageAddress,
>>ImageContext.SectionAlignment));
>>+    }
>>     return Status;
>>   }
>>   //
>>@@ -612,6 +615,8 @@ PeiLoadImageLoadImage (
>>     }
>>   }
>>
>>+  DEBUG ((DEBUG_INFO, "Loading PEIM %g\n", FileHandle));
>>+
>>   //
>>   // If memory is installed, perform the shadow operations
>>   //
>>--
>>2.8.0.windows.1

Re: [Patch] MdeModulePkg PeiCore: Add error message to describe PEIM load failure

[Zeng, Star]

Ok, got it.

Reviewed-by: Star Zeng <star.zeng@intel.com>

Thanks,
Star
-----Original Message-----
From: Gao, Liming 
Sent: Thursday, February 8, 2018 6:40 PM
To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
Subject: RE: [Patch] MdeModulePkg PeiCore: Add error message to describe PEIM load failure

They are reported in different phase. The first message is print before load image. The second message is print after load image successfully. If load image failure, only first message is print. So, I don't think we can combine them. 

>-----Original Message-----
>From: Zeng, Star
>Sent: Thursday, February 08, 2018 6:31 PM
>To: Gao, Liming <liming.gao@intel.com>; edk2-devel@lists.01.org
>Cc: Zeng, Star <star.zeng@intel.com>
>Subject: RE: [Patch] MdeModulePkg PeiCore: Add error message to 
>describe PEIM load failure
>
>How about combining it to one for DXE?
>Either print FFS GUID or file name from PDB path, or both.
>
>Thanks,
>Star
>-----Original Message-----
>From: Gao, Liming
>Sent: Thursday, February 8, 2018 6:25 PM
>To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
>Subject: RE: [Patch] MdeModulePkg PeiCore: Add error message to 
>describe PEIM load failure
>
>Star:
>  This change is to align Loading Driver in DXE phase. You can find 
>below message in DXE boot.
>
>Loading driver 80CF7257-87AB-47F9-A3FE-D50B76D89541
>InstallProtocolInterface: 5B1B31A1-9562-11D2-8E3F-00A0C969723B 88014240 
>Loading driver at 0x00087FE6000 EntryPoint=0x00087FE70B4 PcdDxe.efi
>
>Thanks
>Liming
>>-----Original Message-----
>>From: Zeng, Star
>>Sent: Thursday, February 08, 2018 6:12 PM
>>To: Gao, Liming <liming.gao@intel.com>; edk2-devel@lists.01.org
>>Cc: Zeng, Star <star.zeng@intel.com>
>>Subject: RE: [Patch] MdeModulePkg PeiCore: Add error message to 
>>describe PEIM load failure
>>
>>Hi Liming,
>>
>>Because of new added "+  DEBUG ((DEBUG_INFO, "Loading PEIM %g\n", 
>>FileHandle));", the debug message will have two "Loading PEIM XXX" for 
>>one PEIM, that is very confusing.
>>Loading PEIM 9B3ADA4F-AE56-4C24-8DEA-F03B7558AE50
>>Loading PEIM at 0x00002353000 EntryPoint=0x000003C10C5 PcdPeim.efi
>>
>>How about like below change?
>>1. Also print FFS GUID when section alignment error.
>>2. Print FFS GUID when PDB not found.
>>
>>
>>fb0030045fe4ae561177d9cff01f32088ae9975e
>> MdeModulePkg/Core/Pei/Image/Image.c | 10 +++++++++-
>> 1 file changed, 9 insertions(+), 1 deletion(-)
>>
>>diff --git a/MdeModulePkg/Core/Pei/Image/Image.c
>>b/MdeModulePkg/Core/Pei/Image/Image.c
>>index 198541128512..179c77ef2194 100644
>>--- a/MdeModulePkg/Core/Pei/Image/Image.c
>>+++ b/MdeModulePkg/Core/Pei/Image/Image.c
>>@@ -1,7 +1,7 @@
>> /** @file
>>   Pei Core Load Image Support
>>
>>-Copyright (c) 2006 - 2017, Intel Corporation. All rights 
>>reserved.<BR>
>>+Copyright (c) 2006 - 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
>>@@ -506,6 +506,9 @@ LoadAndRelocatePeCoffImage (
>>   //
>>   Status = PeCoffLoaderLoadImage (&ImageContext);
>>   if (EFI_ERROR (Status)) {
>>+    if (ImageContext.ImageError ==
>>IMAGE_ERROR_INVALID_SECTION_ALIGNMENT) {
>>+      DEBUG ((DEBUG_ERROR, "PEIM(%g) Image Address 0x%11p doesn't
>>meet with section alignment 0x%x.\n", FileHandle, 
>>(VOID*)(UINTN)ImageContext.ImageAddress,
>>ImageContext.SectionAlignment));
>>+    }
>>     return Status;
>>   }
>>   //
>>@@ -707,6 +710,11 @@ PeiLoadImageLoadImage (
>>       }
>>
>>       DEBUG ((EFI_D_INFO | EFI_D_LOAD, "%a", EfiFileName));
>>+    } else {
>>+      //
>>+      // Print FFS GUID when PDB not found.
>>+      //
>>+      DEBUG ((EFI_D_INFO | EFI_D_LOAD, "%g", FileHandle));
>>     }
>>
>>   DEBUG_CODE_END ();
>>
>>
>>Thanks,
>>Star
>>-----Original Message-----
>>From: Gao, Liming
>>Sent: Thursday, February 8, 2018 4:19 PM
>>To: edk2-devel@lists.01.org
>>Cc: Zeng, Star <star.zeng@intel.com>
>>Subject: [Patch] MdeModulePkg PeiCore: Add error message to describe 
>>PEIM load failure
>>
>>If PEIM image address doesn't meet with its section alignment, it will load fail.
>>PeiCore adds more debug message to report it.
>>
>>Contributed-under: TianoCore Contribution Agreement 1.1
>>Signed-off-by: Liming Gao <liming.gao@intel.com>
>>Cc: Star Zeng <star.zeng@intel.com>
>>---
>> MdeModulePkg/Core/Pei/Image/Image.c | 7 ++++++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>>diff --git a/MdeModulePkg/Core/Pei/Image/Image.c
>>b/MdeModulePkg/Core/Pei/Image/Image.c
>>index 1985411..f41d3ac 100644
>>--- a/MdeModulePkg/Core/Pei/Image/Image.c
>>+++ b/MdeModulePkg/Core/Pei/Image/Image.c
>>@@ -1,7 +1,7 @@
>> /** @file
>>   Pei Core Load Image Support
>>
>>-Copyright (c) 2006 - 2017, Intel Corporation. All rights 
>>reserved.<BR>
>>+Copyright (c) 2006 - 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 @@ -506,6 +506,9 @@ LoadAndRelocatePeCoffImage (
>>   //
>>   Status = PeCoffLoaderLoadImage (&ImageContext);
>>   if (EFI_ERROR (Status)) {
>>+    if (ImageContext.ImageError ==
>>IMAGE_ERROR_INVALID_SECTION_ALIGNMENT) {
>>+      DEBUG ((DEBUG_ERROR, "PEIM Image Address 0x%11p doesn't meet
>>with section alignment 0x%x.\n",
>>(VOID*)(UINTN)ImageContext.ImageAddress,
>>ImageContext.SectionAlignment));
>>+    }
>>     return Status;
>>   }
>>   //
>>@@ -612,6 +615,8 @@ PeiLoadImageLoadImage (
>>     }
>>   }
>>
>>+  DEBUG ((DEBUG_INFO, "Loading PEIM %g\n", FileHandle));
>>+
>>   //
>>   // If memory is installed, perform the shadow operations
>>   //
>>--
>>2.8.0.windows.1

Re: [Patch] MdeModulePkg PeiCore: Add error message to describe PEIM load failure

[Brian J. Johnson]

On 02/08/2018 04:31 AM, Laszlo Ersek wrote:
> On 02/08/18 11:24, Gao, Liming wrote:
>> Laszlo:
>>    11p address is big to cover most real address. It can align and save the message length.
> > Good point, it can go up to 2^44  - 1, covering 16 TB of RAM.

Tiny little 16-banger.  Buy a real computer.  :)  :)

https://www.hpe.com/us/en/product-catalog/servers/mission-critical-x86-servers/pip.hpe-superdome-flex-server.1010323140.html

(With many smilies)
Thanks,
-- 
Brian J. Johnson
Enterprise X86 Lab

Hewlett Packard Enterprise

brian.johnson@hpe.com