* [PATCH 1/2] ArmPkg: DebugPeCoffExtraActionLib: debugger commands are not errors
2019-07-29 18:03 [PATCH 0/2] ArmPkg: DebugPeCoffExtraActionLib: debugger commands are not errors Philippe Mathieu-Daudé
@ 2019-07-29 18:03 ` Philippe Mathieu-Daudé
2019-07-30 13:25 ` Laszlo Ersek
2019-07-29 18:03 ` [PATCH 2/2] ArmPkg: DebugPeCoffExtraActionLib: fix trivial typos Philippe Mathieu-Daudé
2019-07-30 14:01 ` [PATCH 0/2] ArmPkg: DebugPeCoffExtraActionLib: debugger commands are not errors Leif Lindholm
2 siblings, 1 reply; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-07-29 18:03 UTC (permalink / raw)
To: devel
Cc: Leif Lindholm, Laszlo Ersek, Ard Biesheuvel,
Philippe Mathieu-Daude, Andrew Jones
In commit 1fce963d89f3e we reduced the level of information printed
by PeCoffLoaderRelocateImageExtraAction() but we did not update the
similar PeCoffLoaderUnloadImageExtraAction() function.
PeCoffLoaderUnloadImageExtraAction() prints helpful debugger commands
for source level debugging. These messages should not be printed on the
EFI_D_ERROR level; they don't report errors. Change the debug level
(bitmask, actually) to DEBUG_LOAD | DEBUG_INFO, because the messages are
printed in relation to image loading, and they are informative.
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reported-by: Andrew Jones <drjones@redhat.com>
Suggested-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Philippe Mathieu-Daude <philmd@redhat.com>
---
| 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
--git a/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.c b/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.c
index a1cb99677fe6..3379744aa185 100644
--- a/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.c
+++ b/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.c
@@ -122,14 +122,14 @@ PeCoffLoaderUnloadImageExtraAction (
if (ImageContext->PdbPointer) {
#ifdef __CC_ARM
// Print out the command for the RVD debugger to load symbols for this image
- DEBUG ((EFI_D_ERROR, "unload symbols_only %a\n", DeCygwinPathIfNeeded (ImageContext->PdbPointer, Temp, sizeof (Temp))));
+ DEBUG ((DEBUG_LOAD | DEBUG_INFO, "unload symbols_only %a\n", DeCygwinPathIfNeeded (ImageContext->PdbPointer, Temp, sizeof (Temp))));
#elif __GNUC__
// This may not work correctly if you generate PE/COFF directlyas then the Offset would not be required
- DEBUG ((EFI_D_ERROR, "remove-symbol-file %a 0x%08x\n", DeCygwinPathIfNeeded (ImageContext->PdbPointer, Temp, sizeof (Temp)), (UINTN)(ImageContext->ImageAddress + ImageContext->SizeOfHeaders)));
+ DEBUG ((DEBUG_LOAD | DEBUG_INFO, "remove-symbol-file %a 0x%08x\n", DeCygwinPathIfNeeded (ImageContext->PdbPointer, Temp, sizeof (Temp)), (UINTN)(ImageContext->ImageAddress + ImageContext->SizeOfHeaders)));
#else
- DEBUG ((EFI_D_ERROR, "Unloading %a\n", ImageContext->PdbPointer));
+ DEBUG ((DEBUG_LOAD | DEBUG_INFO, "Unloading %a\n", ImageContext->PdbPointer));
#endif
} else {
- DEBUG ((EFI_D_ERROR, "Unloading driver at 0x%11p\n", (VOID *)(UINTN) ImageContext->ImageAddress));
+ DEBUG ((DEBUG_LOAD | DEBUG_INFO, "Unloading driver at 0x%11p\n", (VOID *)(UINTN) ImageContext->ImageAddress));
}
}
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] ArmPkg: DebugPeCoffExtraActionLib: debugger commands are not errors
2019-07-29 18:03 ` [PATCH 1/2] " Philippe Mathieu-Daudé
@ 2019-07-30 13:25 ` Laszlo Ersek
0 siblings, 0 replies; 7+ messages in thread
From: Laszlo Ersek @ 2019-07-30 13:25 UTC (permalink / raw)
To: Philippe Mathieu-Daude, devel; +Cc: Leif Lindholm, Ard Biesheuvel, Andrew Jones
On 07/29/19 20:03, Philippe Mathieu-Daude wrote:
> In commit 1fce963d89f3e we reduced the level of information printed
> by PeCoffLoaderRelocateImageExtraAction() but we did not update the
> similar PeCoffLoaderUnloadImageExtraAction() function.
>
> PeCoffLoaderUnloadImageExtraAction() prints helpful debugger commands
> for source level debugging. These messages should not be printed on the
> EFI_D_ERROR level; they don't report errors. Change the debug level
> (bitmask, actually) to DEBUG_LOAD | DEBUG_INFO, because the messages are
> printed in relation to image loading, and they are informative.
>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Reported-by: Andrew Jones <drjones@redhat.com>
> Suggested-by: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Philippe Mathieu-Daude <philmd@redhat.com>
> ---
> .../DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.c b/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.c
> index a1cb99677fe6..3379744aa185 100644
> --- a/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.c
> +++ b/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.c
> @@ -122,14 +122,14 @@ PeCoffLoaderUnloadImageExtraAction (
> if (ImageContext->PdbPointer) {
> #ifdef __CC_ARM
> // Print out the command for the RVD debugger to load symbols for this image
> - DEBUG ((EFI_D_ERROR, "unload symbols_only %a\n", DeCygwinPathIfNeeded (ImageContext->PdbPointer, Temp, sizeof (Temp))));
> + DEBUG ((DEBUG_LOAD | DEBUG_INFO, "unload symbols_only %a\n", DeCygwinPathIfNeeded (ImageContext->PdbPointer, Temp, sizeof (Temp))));
> #elif __GNUC__
> // This may not work correctly if you generate PE/COFF directlyas then the Offset would not be required
> - DEBUG ((EFI_D_ERROR, "remove-symbol-file %a 0x%08x\n", DeCygwinPathIfNeeded (ImageContext->PdbPointer, Temp, sizeof (Temp)), (UINTN)(ImageContext->ImageAddress + ImageContext->SizeOfHeaders)));
> + DEBUG ((DEBUG_LOAD | DEBUG_INFO, "remove-symbol-file %a 0x%08x\n", DeCygwinPathIfNeeded (ImageContext->PdbPointer, Temp, sizeof (Temp)), (UINTN)(ImageContext->ImageAddress + ImageContext->SizeOfHeaders)));
> #else
> - DEBUG ((EFI_D_ERROR, "Unloading %a\n", ImageContext->PdbPointer));
> + DEBUG ((DEBUG_LOAD | DEBUG_INFO, "Unloading %a\n", ImageContext->PdbPointer));
> #endif
> } else {
> - DEBUG ((EFI_D_ERROR, "Unloading driver at 0x%11p\n", (VOID *)(UINTN) ImageContext->ImageAddress));
> + DEBUG ((DEBUG_LOAD | DEBUG_INFO, "Unloading driver at 0x%11p\n", (VOID *)(UINTN) ImageContext->ImageAddress));
> }
> }
>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] ArmPkg: DebugPeCoffExtraActionLib: fix trivial typos
2019-07-29 18:03 [PATCH 0/2] ArmPkg: DebugPeCoffExtraActionLib: debugger commands are not errors Philippe Mathieu-Daudé
2019-07-29 18:03 ` [PATCH 1/2] " Philippe Mathieu-Daudé
@ 2019-07-29 18:03 ` Philippe Mathieu-Daudé
2019-07-30 13:26 ` Laszlo Ersek
2019-07-30 14:01 ` [PATCH 0/2] ArmPkg: DebugPeCoffExtraActionLib: debugger commands are not errors Leif Lindholm
2 siblings, 1 reply; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-07-29 18:03 UTC (permalink / raw)
To: devel; +Cc: Leif Lindholm, Laszlo Ersek, Ard Biesheuvel,
Philippe Mathieu-Daude
Fix a pair of trivial typos by inserting a space.
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Philippe Mathieu-Daude <philmd@redhat.com>
---
| 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--git a/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.c b/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.c
index 3379744aa185..3f88e84372ee 100644
--- a/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.c
+++ b/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.c
@@ -87,7 +87,7 @@ PeCoffLoaderRelocateImageExtraAction (
DEBUG ((EFI_D_LOAD | EFI_D_INFO, "add-symbol-file %a 0x%p\n", DeCygwinPathIfNeeded (ImageContext->PdbPointer, Temp, sizeof (Temp)), (UINTN)(ImageContext->ImageAddress + ImageContext->SizeOfHeaders)));
#endif
#elif __GNUC__
- // This may not work correctly if you generate PE/COFF directlyas then the Offset would not be required
+ // This may not work correctly if you generate PE/COFF directly as then the Offset would not be required
DEBUG ((EFI_D_LOAD | EFI_D_INFO, "add-symbol-file %a 0x%p\n", DeCygwinPathIfNeeded (ImageContext->PdbPointer, Temp, sizeof (Temp)), (UINTN)(ImageContext->ImageAddress + ImageContext->SizeOfHeaders)));
#else
DEBUG ((EFI_D_LOAD | EFI_D_INFO, "Loading driver at 0x%11p EntryPoint=0x%11p\n", (VOID *)(UINTN) ImageContext->ImageAddress, FUNCTION_ENTRY_POINT (ImageContext->EntryPoint)));
@@ -124,7 +124,7 @@ PeCoffLoaderUnloadImageExtraAction (
// Print out the command for the RVD debugger to load symbols for this image
DEBUG ((DEBUG_LOAD | DEBUG_INFO, "unload symbols_only %a\n", DeCygwinPathIfNeeded (ImageContext->PdbPointer, Temp, sizeof (Temp))));
#elif __GNUC__
- // This may not work correctly if you generate PE/COFF directlyas then the Offset would not be required
+ // This may not work correctly if you generate PE/COFF directly as then the Offset would not be required
DEBUG ((DEBUG_LOAD | DEBUG_INFO, "remove-symbol-file %a 0x%08x\n", DeCygwinPathIfNeeded (ImageContext->PdbPointer, Temp, sizeof (Temp)), (UINTN)(ImageContext->ImageAddress + ImageContext->SizeOfHeaders)));
#else
DEBUG ((DEBUG_LOAD | DEBUG_INFO, "Unloading %a\n", ImageContext->PdbPointer));
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] ArmPkg: DebugPeCoffExtraActionLib: fix trivial typos
2019-07-29 18:03 ` [PATCH 2/2] ArmPkg: DebugPeCoffExtraActionLib: fix trivial typos Philippe Mathieu-Daudé
@ 2019-07-30 13:26 ` Laszlo Ersek
0 siblings, 0 replies; 7+ messages in thread
From: Laszlo Ersek @ 2019-07-30 13:26 UTC (permalink / raw)
To: Philippe Mathieu-Daude, devel; +Cc: Leif Lindholm, Ard Biesheuvel
On 07/29/19 20:03, Philippe Mathieu-Daude wrote:
> Fix a pair of trivial typos by inserting a space.
>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Signed-off-by: Philippe Mathieu-Daude <philmd@redhat.com>
> ---
> .../DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.c b/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.c
> index 3379744aa185..3f88e84372ee 100644
> --- a/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.c
> +++ b/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.c
> @@ -87,7 +87,7 @@ PeCoffLoaderRelocateImageExtraAction (
> DEBUG ((EFI_D_LOAD | EFI_D_INFO, "add-symbol-file %a 0x%p\n", DeCygwinPathIfNeeded (ImageContext->PdbPointer, Temp, sizeof (Temp)), (UINTN)(ImageContext->ImageAddress + ImageContext->SizeOfHeaders)));
> #endif
> #elif __GNUC__
> - // This may not work correctly if you generate PE/COFF directlyas then the Offset would not be required
> + // This may not work correctly if you generate PE/COFF directly as then the Offset would not be required
> DEBUG ((EFI_D_LOAD | EFI_D_INFO, "add-symbol-file %a 0x%p\n", DeCygwinPathIfNeeded (ImageContext->PdbPointer, Temp, sizeof (Temp)), (UINTN)(ImageContext->ImageAddress + ImageContext->SizeOfHeaders)));
> #else
> DEBUG ((EFI_D_LOAD | EFI_D_INFO, "Loading driver at 0x%11p EntryPoint=0x%11p\n", (VOID *)(UINTN) ImageContext->ImageAddress, FUNCTION_ENTRY_POINT (ImageContext->EntryPoint)));
> @@ -124,7 +124,7 @@ PeCoffLoaderUnloadImageExtraAction (
> // Print out the command for the RVD debugger to load symbols for this image
> DEBUG ((DEBUG_LOAD | DEBUG_INFO, "unload symbols_only %a\n", DeCygwinPathIfNeeded (ImageContext->PdbPointer, Temp, sizeof (Temp))));
> #elif __GNUC__
> - // This may not work correctly if you generate PE/COFF directlyas then the Offset would not be required
> + // This may not work correctly if you generate PE/COFF directly as then the Offset would not be required
> DEBUG ((DEBUG_LOAD | DEBUG_INFO, "remove-symbol-file %a 0x%08x\n", DeCygwinPathIfNeeded (ImageContext->PdbPointer, Temp, sizeof (Temp)), (UINTN)(ImageContext->ImageAddress + ImageContext->SizeOfHeaders)));
> #else
> DEBUG ((DEBUG_LOAD | DEBUG_INFO, "Unloading %a\n", ImageContext->PdbPointer));
>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] ArmPkg: DebugPeCoffExtraActionLib: debugger commands are not errors
2019-07-29 18:03 [PATCH 0/2] ArmPkg: DebugPeCoffExtraActionLib: debugger commands are not errors Philippe Mathieu-Daudé
2019-07-29 18:03 ` [PATCH 1/2] " Philippe Mathieu-Daudé
2019-07-29 18:03 ` [PATCH 2/2] ArmPkg: DebugPeCoffExtraActionLib: fix trivial typos Philippe Mathieu-Daudé
@ 2019-07-30 14:01 ` Leif Lindholm
2019-07-30 14:07 ` [edk2-devel] " Philippe Mathieu-Daudé
2 siblings, 1 reply; 7+ messages in thread
From: Leif Lindholm @ 2019-07-30 14:01 UTC (permalink / raw)
To: Philippe Mathieu-Daude; +Cc: devel, Laszlo Ersek, Ard Biesheuvel
On Mon, Jul 29, 2019 at 08:03:19PM +0200, Philippe Mathieu-Daude wrote:
> Hi,
>
> This series completes the cleanup from commit 1fce963d89f3 ("ArmPkg:
> DebugPeCoffExtraActionLib: debugger commands are not errors", from
> 2015-03-02), on PeCoffLoaderUnloadImageExtraAction().
>
> This solves an issue when building in silent mode with
> -D DEBUG_PRINT_ERROR_LEVEL=0x80000000. With the QemuRamFbDxe platform
> DXE driver, when we get a driver dispatch failure it is reported as
> an error level:
>
> qemu-system-aarch64 -machine virt,gic-version=3,accel=kvm -cpu host \
> -display none -serial stdio -bios /usr/share/AAVMF/AAVMF_CODE.fd
> remove-symbol-file /builddir/build/BUILD/edk2-89910a39dcfd/Build/ArmVirtQemu-AARCH64/DEBUG_GCC5/AARCH64/OvmfPkg/QemuRamfbDxe/QemuRamfbDxe/DEBUG/QemuRamfbDxe.dll 0x3F5F6000
>
> The second patch is an obvious typo fix.
I added the word "comment" in subject/message of 2/2, to make
it explicit that the typo does not affect functionality.
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
Pushed as 8fed4e47d9a6..3d34b5f32692.
Thanks!
> Regards,
>
> Phil.
>
> Philippe Mathieu-Daudé (2):
> ArmPkg: DebugPeCoffExtraActionLib: debugger commands are not errors
> ArmPkg: DebugPeCoffExtraActionLib: fix trivial typos
>
> .../DebugPeCoffExtraActionLib.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH 0/2] ArmPkg: DebugPeCoffExtraActionLib: debugger commands are not errors
2019-07-30 14:01 ` [PATCH 0/2] ArmPkg: DebugPeCoffExtraActionLib: debugger commands are not errors Leif Lindholm
@ 2019-07-30 14:07 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-07-30 14:07 UTC (permalink / raw)
To: devel, leif.lindholm; +Cc: Laszlo Ersek, Ard Biesheuvel
On 7/30/19 4:01 PM, Leif Lindholm wrote:
> On Mon, Jul 29, 2019 at 08:03:19PM +0200, Philippe Mathieu-Daude wrote:
>> Hi,
>>
>> This series completes the cleanup from commit 1fce963d89f3 ("ArmPkg:
>> DebugPeCoffExtraActionLib: debugger commands are not errors", from
>> 2015-03-02), on PeCoffLoaderUnloadImageExtraAction().
>>
>> This solves an issue when building in silent mode with
>> -D DEBUG_PRINT_ERROR_LEVEL=0x80000000. With the QemuRamFbDxe platform
>> DXE driver, when we get a driver dispatch failure it is reported as
>> an error level:
>>
>> qemu-system-aarch64 -machine virt,gic-version=3,accel=kvm -cpu host \
>> -display none -serial stdio -bios /usr/share/AAVMF/AAVMF_CODE.fd
>> remove-symbol-file /builddir/build/BUILD/edk2-89910a39dcfd/Build/ArmVirtQemu-AARCH64/DEBUG_GCC5/AARCH64/OvmfPkg/QemuRamfbDxe/QemuRamfbDxe/DEBUG/QemuRamfbDxe.dll 0x3F5F6000
>>
>> The second patch is an obvious typo fix.
>
> I added the word "comment" in subject/message of 2/2, to make
> it explicit that the typo does not affect functionality.
Good idea, thanks.
>
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> Pushed as 8fed4e47d9a6..3d34b5f32692.
Thanks Laszlo and Leif!
>
> Thanks!
>
>> Regards,
>>
>> Phil.
>>
>> Philippe Mathieu-Daudé (2):
>> ArmPkg: DebugPeCoffExtraActionLib: debugger commands are not errors
>> ArmPkg: DebugPeCoffExtraActionLib: fix trivial typos
>>
>> .../DebugPeCoffExtraActionLib.c | 12 ++++++------
>> 1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> --
>> 2.20.1
>>
>
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread