[PATCH v2 0/8] Fix imbalanced debug macros

[PATCH v2 0/8] Fix imbalanced debug macros

[Michael Kubacki]

From: Michael Kubacki <michael.kubacki@microsoft.com>

After noticing a few occurrences of DEBUG macros with a mismatched
number of arguments, I wrote a script to automatically detect this.

This patch series is the result of matches found by the script.

In some cases it is not obvious what was originally intended so I
have attempted to capture what I believe was the original author's
intention.

V2 Changes:
These additional errors were found while finishing testing of
the script.

1. Add DynamicTablesPkg/AcpiPpttLibArm patch for the change in
   - DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
2. Update MdeModulePkg patch to include the change in
   - MdeModulePkg/Universal/CapsulePei/UefiCapsule.c
3. Add following to the RedfishPkg fix patch:
   - RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
   - RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c
4. Add NetworkPkg patch for the change in
   - NetworkPkg/TcpDxe/SockInterface.c
5. Add OvmfPkg patch for the change in
   - OvmfPkg/Csm/LegacyBootManagerLib/LegacyBm.c

Cc: Abner Chang <abner.chang@amd.com>
Cc: Alexei Fedorov <Alexei.Fedorov@arm.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Guomin Jiang <guomin.jiang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Nickle Wang <nickle.wang@hpe.com>
Cc: Qi Zhang <qi1.zhang@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Sami Mujawar <Sami.Mujawar@arm.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>

Michael Kubacki (8):
  ArmPlatformPkg/NorFlashDxe: Remove unused debug print specifier
  FatPkg/FatPei: Remove extraneous debug message argument
  MdeModulePkg: Fix imbalanced debug macros
  RedfishPkg/RedfishRestExDxe: Remove extra debug macro argument
  SecurityPkg/SmmTcg2PhysicalPresenceLib: Add missing debug print
    specifier
  DynamicTablesPkg/AcpiPpttLibArm: Fix debug macro arguments
  NetworkPkg/TcpDxe: Fix debug macro arguments
  OvmfPkg/LegacyBootManagerLib: Fix debug macro arguments

 ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c                                  |  4 ++--
 DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c                         |  3 +--
 FatPkg/FatPei/Gpt.c                                                                      |  2 +-
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c                                         |  2 +-
 MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c                                               |  8 ++++----
 MdeModulePkg/Core/Dxe/Image/Image.c                                                      |  2 +-
 MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/DisplayUpdateProgressLibGraphics.c |  2 +-
 MdeModulePkg/Universal/CapsulePei/UefiCapsule.c                                          |  2 +-
 NetworkPkg/TcpDxe/SockInterface.c                                                        | 12 ++----------
 OvmfPkg/Csm/LegacyBootManagerLib/LegacyBm.c                                              |  6 +++---
 RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c                                       |  6 +++---
 RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c                             |  2 +-
 RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c                                      |  2 +-
 SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/MmTcg2PhysicalPresenceLibCommon.c         |  2 +-
 14 files changed, 23 insertions(+), 32 deletions(-)

-- 
2.28.0.windows.1

[PATCH v2 1/8] ArmPlatformPkg/NorFlashDxe: Remove unused debug print specifier

[Michael Kubacki]

From: Michael Kubacki <michael.kubacki@microsoft.com>

These debug messages are repeated in both NorFlashBlockIoReadBlocks()
and NorFlashBlockIoWriteBlocks():

  "NorFlashBlockIoWriteBlocks(MediaId=0x%x, Lba=%ld, BufferSize=0x%x"
  "bytes (%d kB), BufferPtr @ 0x%08x)\n"

Although this requires 5 arguments, only 4 are provided. The kilobyte
value was never given.

This change removes that specifier so the 4 arguments match the 4
specifiers in the debug macro.

Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
---
 ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c
index 5afab0a79fa2..e671108e2bcf 100644
--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c
@@ -54,7 +54,7 @@ NorFlashBlockIoReadBlocks (
   Instance = INSTANCE_FROM_BLKIO_THIS (This);
   Media    = This->Media;
 
-  DEBUG ((DEBUG_BLKIO, "NorFlashBlockIoReadBlocks(MediaId=0x%x, Lba=%ld, BufferSize=0x%x bytes (%d kB), BufferPtr @ 0x%08x)\n", MediaId, Lba, BufferSizeInBytes, Buffer));
+  DEBUG ((DEBUG_BLKIO, "NorFlashBlockIoReadBlocks(MediaId=0x%x, Lba=%ld, BufferSize=0x%x bytes, BufferPtr @ 0x%08x)\n", MediaId, Lba, BufferSizeInBytes, Buffer));
 
   if (!Media) {
     Status = EFI_INVALID_PARAMETER;
@@ -89,7 +89,7 @@ NorFlashBlockIoWriteBlocks (
 
   Instance = INSTANCE_FROM_BLKIO_THIS (This);
 
-  DEBUG ((DEBUG_BLKIO, "NorFlashBlockIoWriteBlocks(MediaId=0x%x, Lba=%ld, BufferSize=0x%x bytes (%d kB), BufferPtr @ 0x%08x)\n", MediaId, Lba, BufferSizeInBytes, Buffer));
+  DEBUG ((DEBUG_BLKIO, "NorFlashBlockIoWriteBlocks(MediaId=0x%x, Lba=%ld, BufferSize=0x%x bytes, BufferPtr @ 0x%08x)\n", MediaId, Lba, BufferSizeInBytes, Buffer));
 
   if ( !This->Media->MediaPresent ) {
     Status = EFI_NO_MEDIA;
-- 
2.28.0.windows.1

Re: [edk2-devel] [PATCH v2 1/8] ArmPlatformPkg/NorFlashDxe: Remove unused debug print specifier

[Leif Lindholm]

On Wed, Aug 24, 2022 at 23:48:17 -0400, Michael Kubacki wrote:
> From: Michael Kubacki <michael.kubacki@microsoft.com>
> 
> These debug messages are repeated in both NorFlashBlockIoReadBlocks()
> and NorFlashBlockIoWriteBlocks():
> 
>   "NorFlashBlockIoWriteBlocks(MediaId=0x%x, Lba=%ld, BufferSize=0x%x"
>   "bytes (%d kB), BufferPtr @ 0x%08x)\n"
> 
> Although this requires 5 arguments, only 4 are provided. The kilobyte
> value was never given.
> 
> This change removes that specifier so the 4 arguments match the 4
> specifiers in the debug macro.
>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c
> index 5afab0a79fa2..e671108e2bcf 100644
> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c
> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c
> @@ -54,7 +54,7 @@ NorFlashBlockIoReadBlocks (
>    Instance = INSTANCE_FROM_BLKIO_THIS (This);
>    Media    = This->Media;
>  
> -  DEBUG ((DEBUG_BLKIO, "NorFlashBlockIoReadBlocks(MediaId=0x%x, Lba=%ld, BufferSize=0x%x bytes (%d kB), BufferPtr @ 0x%08x)\n", MediaId, Lba, BufferSizeInBytes, Buffer));
> +  DEBUG ((DEBUG_BLKIO, "NorFlashBlockIoReadBlocks(MediaId=0x%x, Lba=%ld, BufferSize=0x%x bytes, BufferPtr @ 0x%08x)\n", MediaId, Lba, BufferSizeInBytes, Buffer));
>

Line last touched 2013-01-25, which was a line ending fix on top of
the creation of ArmPlatformPkg on 2011-02-01. I mean on the positive
side, no one would appear to have needed to debug BLKIO since then.

I'm not going to bikeshed, but I'm going to mention the bikeshedding
point that the original mistake made was in not repeating
BufferSizeInBytes in the argument list.

Make of this what you will - either way
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>

>    if (!Media) {
>      Status = EFI_INVALID_PARAMETER;
> @@ -89,7 +89,7 @@ NorFlashBlockIoWriteBlocks (
>  
>    Instance = INSTANCE_FROM_BLKIO_THIS (This);
>  
> -  DEBUG ((DEBUG_BLKIO, "NorFlashBlockIoWriteBlocks(MediaId=0x%x, Lba=%ld, BufferSize=0x%x bytes (%d kB), BufferPtr @ 0x%08x)\n", MediaId, Lba, BufferSizeInBytes, Buffer));
> +  DEBUG ((DEBUG_BLKIO, "NorFlashBlockIoWriteBlocks(MediaId=0x%x, Lba=%ld, BufferSize=0x%x bytes, BufferPtr @ 0x%08x)\n", MediaId, Lba, BufferSizeInBytes, Buffer));
>  
>    if ( !This->Media->MediaPresent ) {
>      Status = EFI_NO_MEDIA;
> -- 
> 2.28.0.windows.1
> 
> 
> 
> 
> 
> 

Re: [edk2-devel] [PATCH v2 1/8] ArmPlatformPkg/NorFlashDxe: Remove unused debug print specifier

[Michael Kubacki]

I updated V3 to repeat BufferSizeInBytes.

Regards,
Michael

On 8/25/2022 6:15 AM, Leif Lindholm wrote:
> On Wed, Aug 24, 2022 at 23:48:17 -0400, Michael Kubacki wrote:
>> From: Michael Kubacki <michael.kubacki@microsoft.com>
>>
>> These debug messages are repeated in both NorFlashBlockIoReadBlocks()
>> and NorFlashBlockIoWriteBlocks():
>>
>>    "NorFlashBlockIoWriteBlocks(MediaId=0x%x, Lba=%ld, BufferSize=0x%x"
>>    "bytes (%d kB), BufferPtr @ 0x%08x)\n"
>>
>> Although this requires 5 arguments, only 4 are provided. The kilobyte
>> value was never given.
>>
>> This change removes that specifier so the 4 arguments match the 4
>> specifiers in the debug macro.
>>
>> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
>> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
>> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
>> Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
>> ---
>>   ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c
>> index 5afab0a79fa2..e671108e2bcf 100644
>> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c
>> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c
>> @@ -54,7 +54,7 @@ NorFlashBlockIoReadBlocks (
>>     Instance = INSTANCE_FROM_BLKIO_THIS (This);
>>     Media    = This->Media;
>>   
>> -  DEBUG ((DEBUG_BLKIO, "NorFlashBlockIoReadBlocks(MediaId=0x%x, Lba=%ld, BufferSize=0x%x bytes (%d kB), BufferPtr @ 0x%08x)\n", MediaId, Lba, BufferSizeInBytes, Buffer));
>> +  DEBUG ((DEBUG_BLKIO, "NorFlashBlockIoReadBlocks(MediaId=0x%x, Lba=%ld, BufferSize=0x%x bytes, BufferPtr @ 0x%08x)\n", MediaId, Lba, BufferSizeInBytes, Buffer));
>>
> 
> Line last touched 2013-01-25, which was a line ending fix on top of
> the creation of ArmPlatformPkg on 2011-02-01. I mean on the positive
> side, no one would appear to have needed to debug BLKIO since then.
> 
> I'm not going to bikeshed, but I'm going to mention the bikeshedding
> point that the original mistake made was in not repeating
> BufferSizeInBytes in the argument list.
> 
> Make of this what you will - either way
> Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
> 
>>     if (!Media) {
>>       Status = EFI_INVALID_PARAMETER;
>> @@ -89,7 +89,7 @@ NorFlashBlockIoWriteBlocks (
>>   
>>     Instance = INSTANCE_FROM_BLKIO_THIS (This);
>>   
>> -  DEBUG ((DEBUG_BLKIO, "NorFlashBlockIoWriteBlocks(MediaId=0x%x, Lba=%ld, BufferSize=0x%x bytes (%d kB), BufferPtr @ 0x%08x)\n", MediaId, Lba, BufferSizeInBytes, Buffer));
>> +  DEBUG ((DEBUG_BLKIO, "NorFlashBlockIoWriteBlocks(MediaId=0x%x, Lba=%ld, BufferSize=0x%x bytes, BufferPtr @ 0x%08x)\n", MediaId, Lba, BufferSizeInBytes, Buffer));
>>   
>>     if ( !This->Media->MediaPresent ) {
>>       Status = EFI_NO_MEDIA;
>> -- 
>> 2.28.0.windows.1
>>
>>
>>
>>
>>
>>
> 
> 
> 
> 

[PATCH v2 2/8] FatPkg/FatPei: Remove extraneous debug message argument

[Michael Kubacki]

From: Michael Kubacki <michael.kubacki@microsoft.com>

This debug macro should take one argument based on the number of
print specifiers defined. However, two arguments are given.

It looks like the code may have been refactored such that the
second argument was moved to a new print and this argument was
not removed. In any case, it should not be there now.

Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
---
 FatPkg/FatPei/Gpt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/FatPkg/FatPei/Gpt.c b/FatPkg/FatPei/Gpt.c
index 0a1a25ceeff1..9e17ce88086f 100644
--- a/FatPkg/FatPei/Gpt.c
+++ b/FatPkg/FatPei/Gpt.c
@@ -361,7 +361,7 @@ PartitionCheckGptEntryArray (
 
     PrivateData->BlockDeviceCount++;
 
-    DEBUG ((DEBUG_INFO, "Find GPT Partition [0x%lx", PartitionEntryBuffer[Index].StartingLBA, BlockDevPtr->LastBlock));
+    DEBUG ((DEBUG_INFO, "Find GPT Partition [0x%lx", PartitionEntryBuffer[Index].StartingLBA));
     DEBUG ((DEBUG_INFO, ", 0x%lx]\n", BlockDevPtr->LastBlock));
     DEBUG ((DEBUG_INFO, "         BlockSize %x\n", BlockDevPtr->BlockSize));
   }
-- 
2.28.0.windows.1

[PATCH v2 3/8] MdeModulePkg: Fix imbalanced debug macros

[Michael Kubacki]

From: Michael Kubacki <michael.kubacki@microsoft.com>

Updates debug macros in the package that have an imbalanced number
of print specifiers to arguments. These changes try to preserve
what was likely intended by the author. In cases information was
missing due to the bug, the specifier may be removed since it was
not previously accurately printing the expected value.

Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Guomin Jiang <guomin.jiang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
---
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c                                         | 2 +-
 MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c                                               | 8 ++++----
 MdeModulePkg/Core/Dxe/Image/Image.c                                                      | 2 +-
 MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/DisplayUpdateProgressLibGraphics.c | 2 +-
 MdeModulePkg/Universal/CapsulePei/UefiCapsule.c                                          | 2 +-
 5 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
index 53b63ab52b93..dd45167a009e 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
@@ -64,7 +64,7 @@ DumpCapabilityReg (
   DEBUG ((DEBUG_INFO, "   Driver Type D     %a\n", Capability->DriverTypeD ? "TRUE" : "FALSE"));
   DEBUG ((DEBUG_INFO, "   Driver Type 4     %a\n", Capability->DriverType4 ? "TRUE" : "FALSE"));
   if (Capability->TimerCount == 0) {
-    DEBUG ((DEBUG_INFO, "   Retuning TimerCnt Disabled\n", 2 * (Capability->TimerCount - 1)));
+    DEBUG ((DEBUG_INFO, "   Retuning TimerCnt Disabled\n"));
   } else {
     DEBUG ((DEBUG_INFO, "   Retuning TimerCnt %dseconds\n", 2 * (Capability->TimerCount - 1)));
   }
diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c
index 5495b324b381..aed34596f469 100644
--- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c
+++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c
@@ -941,7 +941,7 @@ UsbEnumeratePort (
       //   which probably is caused by short circuit. It has to wait system hardware
       //   to perform recovery.
       //
-      DEBUG ((DEBUG_ERROR, "UsbEnumeratePort: Critical Over Current\n", Port));
+      DEBUG ((DEBUG_ERROR, "UsbEnumeratePort: Critical Over Current (port %d)\n", Port));
       return EFI_DEVICE_ERROR;
     }
 
@@ -951,7 +951,7 @@ UsbEnumeratePort (
     //   over current. As a result, all ports are nearly power-off, so
     //   it's necessary to detach and enumerate all ports again.
     //
-    DEBUG ((DEBUG_ERROR, "UsbEnumeratePort: 2.0 device Recovery Over Current\n", Port));
+    DEBUG ((DEBUG_ERROR, "UsbEnumeratePort: 2.0 device Recovery Over Current (port %d)\n", Port));
   }
 
   if (USB_BIT_IS_SET (PortState.PortChangeStatus, USB_PORT_STAT_C_ENABLE)) {
@@ -961,7 +961,7 @@ UsbEnumeratePort (
     //   on 2.0 roothub does. When over-current has influence on 1.1 device, the port
     //   would be disabled, so it's also necessary to detach and enumerate again.
     //
-    DEBUG ((DEBUG_ERROR, "UsbEnumeratePort: 1.1 device Recovery Over Current\n", Port));
+    DEBUG ((DEBUG_ERROR, "UsbEnumeratePort: 1.1 device Recovery Over Current (port %d)\n", Port));
   }
 
   if (USB_BIT_IS_SET (PortState.PortChangeStatus, USB_PORT_STAT_C_CONNECTION)) {
@@ -969,7 +969,7 @@ UsbEnumeratePort (
     // Case4:
     //   Device connected or disconnected normally.
     //
-    DEBUG ((DEBUG_INFO, "UsbEnumeratePort: Device Connect/Disconnect Normally\n", Port));
+    DEBUG ((DEBUG_INFO, "UsbEnumeratePort: Device Connect/Disconnect Normally (port %d)\n", Port));
   }
 
   //
diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c b/MdeModulePkg/Core/Dxe/Image/Image.c
index 68bde5c15c52..06cc6744b8c6 100644
--- a/MdeModulePkg/Core/Dxe/Image/Image.c
+++ b/MdeModulePkg/Core/Dxe/Image/Image.c
@@ -1741,7 +1741,7 @@ CoreStartImage (
   if ((Image->ExitDataSize != 0) || (Image->ExitData != NULL)) {
     DEBUG ((DEBUG_LOAD, "StartImage: ExitDataSize %d, ExitData %p", (UINT32)Image->ExitDataSize, Image->ExitData));
     if (Image->ExitData != NULL) {
-      DEBUG ((DEBUG_LOAD, " (%hs)", Image->ExitData));
+      DEBUG ((DEBUG_LOAD, " (%s)", Image->ExitData));
     }
 
     DEBUG ((DEBUG_LOAD, "\n"));
diff --git a/MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/DisplayUpdateProgressLibGraphics.c b/MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/DisplayUpdateProgressLibGraphics.c
index 83053464e06e..6b012fed35db 100644
--- a/MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/DisplayUpdateProgressLibGraphics.c
+++ b/MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/DisplayUpdateProgressLibGraphics.c
@@ -148,7 +148,7 @@ FindDim (
                   (VOID **)&BootLogo
                   );
   if ((BootLogo == NULL) || (EFI_ERROR (Status))) {
-    DEBUG ((DEBUG_ERROR, "Failed to locate gEdkiiBootLogo2ProtocolGuid.  No Progress bar support. \n", Status));
+    DEBUG ((DEBUG_ERROR, "Failed to locate gEdkiiBootLogo2ProtocolGuid Status = %r.  No Progress bar support. \n", Status));
     return;
   }
 
diff --git a/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c b/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c
index ef60d4e1b7b7..cdeffa911311 100644
--- a/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c
+++ b/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c
@@ -976,7 +976,7 @@ GetScatterGatherHeadEntries (
 
     if (EFI_ERROR (Status)) {
       if (Status != EFI_NOT_FOUND) {
-        DEBUG ((DEBUG_ERROR, "Unexpected error getting Capsule Update variable.  Status = %r\n"));
+        DEBUG ((DEBUG_ERROR, "Unexpected error getting Capsule Update variable.  Status = %r\n", Status));
       }
 
       break;
-- 
2.28.0.windows.1

[PATCH v2 4/8] RedfishPkg/RedfishRestExDxe: Remove extra debug macro argument

[Michael Kubacki]

From: Michael Kubacki <michael.kubacki@microsoft.com>

The debug macro argument in this change is removed since it does
have a corresponding print specifier in the debug message string.

Cc: Abner Chang <abner.chang@amd.com>
Cc: Nickle Wang <nickle.wang@hpe.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Abner Chang <abner.chang@amd.com>
---
 RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c           | 6 +++---
 RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c | 2 +-
 RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c          | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
index bf50c78c9280..586ecfadc715 100644
--- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
+++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
@@ -264,12 +264,12 @@ Tcp6GetSubnetInfo (
 
   Status = Tcp6->GetModeData (Tcp6, NULL, NULL, &IpModedata, NULL, NULL);
   if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "%a: Can't get IP mode data information\n"));
+    DEBUG ((DEBUG_ERROR, "%a: Can't get IP mode data information\n", __FUNCTION__));
     return Status;
   }
 
   if (IpModedata.AddressCount == 0) {
-    DEBUG ((DEBUG_INFO, "%a: No IPv6 address configured.\n"));
+    DEBUG ((DEBUG_INFO, "%a: No IPv6 address configured.\n", __FUNCTION__));
   }
 
   if (Instance->SubnetAddrInfoIPv6 != NULL) {
@@ -278,7 +278,7 @@ Tcp6GetSubnetInfo (
 
   Instance->SubnetAddrInfoIPv6 = AllocateZeroPool (IpModedata.AddressCount * sizeof (EFI_IP6_ADDRESS_INFO));
   if (Instance->SubnetAddrInfoIPv6 == NULL) {
-    DEBUG ((DEBUG_ERROR, "%a: Failed to allocate memory fir IPv6 subnet address information\n"));
+    DEBUG ((DEBUG_ERROR, "%a: Failed to allocate memory for IPv6 subnet address information\n", __FUNCTION__));
     return EFI_OUT_OF_RESOURCES;
   }
 
diff --git a/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c b/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c
index 8a05764ac605..623350bc261c 100644
--- a/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c
+++ b/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c
@@ -119,7 +119,7 @@ RedfishCreateSmbiosTable42 (
     } else {
       NewProtocolRecords = ReallocatePool (CurrentProtocolsDataLength, NewProtocolsDataLength, (VOID *)ProtocolRecords);
       if (NewProtocolRecords == NULL) {
-        DEBUG ((DEBUG_ERROR, "%a: Fail to allocate memory for Redfish host interface protocol data."));
+        DEBUG ((DEBUG_ERROR, "%a: Fail to allocate memory for Redfish host interface protocol data.", __FUNCTION__));
         FreePool (ProtocolRecords);
         FreePool (ProtocolRecord);
         return EFI_OUT_OF_RESOURCES;
diff --git a/RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c b/RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c
index f224104ad673..4b61fc01adc4 100644
--- a/RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c
+++ b/RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c
@@ -224,7 +224,7 @@ ReSendRequest:;
     DEBUG ((DEBUG_INFO, "HTTP_STATUS_200_OK\n"));
 
     if (SendChunkProcess == HttpIoSendChunkHeaderZeroContent) {
-      DEBUG ((DEBUG_INFO, "This is chunk transfer, start to send all chunks.", ResponseData->Response.StatusCode));
+      DEBUG ((DEBUG_INFO, "This is chunk transfer, start to send all chunks."));
       SendChunkProcess++;
       goto ReSendRequest;
     }
-- 
2.28.0.windows.1

[PATCH v2 5/8] SecurityPkg/SmmTcg2PhysicalPresenceLib: Add missing debug print specifier

[Michael Kubacki]

From: Michael Kubacki <michael.kubacki@microsoft.com>

The debug macro modified in this change was missing a print specifier
for a debug message argument given.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Qi Zhang <qi1.zhang@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
---
 SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/MmTcg2PhysicalPresenceLibCommon.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/MmTcg2PhysicalPresenceLibCommon.c b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/MmTcg2PhysicalPresenceLibCommon.c
index 1fbfc00547cd..f2ab4f125007 100644
--- a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/MmTcg2PhysicalPresenceLibCommon.c
+++ b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/MmTcg2PhysicalPresenceLibCommon.c
@@ -176,7 +176,7 @@ Tcg2PhysicalPresenceLibSubmitRequestToPreOSFunctionEx (
   // Sync PPRQ/PPRM from PP Variable if PP submission fails
   //
   if (ReturnCode != TCG_PP_SUBMIT_REQUEST_TO_PREOS_SUCCESS) {
-    DEBUG ((DEBUG_ERROR, "[TPM2] Submit PP Request failure! Sync PPRQ/PPRM with PP variable.\n", Status));
+    DEBUG ((DEBUG_ERROR, "[TPM2] Submit PP Request failure! Sync PPRQ/PPRM with PP variable. Status = %r\n", Status));
     DataSize = sizeof (EFI_TCG2_PHYSICAL_PRESENCE);
     ZeroMem (&PpData, DataSize);
     Status = mTcg2PpSmmVariable->SmmGetVariable (
-- 
2.28.0.windows.1

Re: [PATCH v2 5/8] SecurityPkg/SmmTcg2PhysicalPresenceLib: Add missing debug print specifier

[Yao, Jiewen]

Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>

> -----Original Message-----
> From: mikuback@linux.microsoft.com <mikuback@linux.microsoft.com>
> Sent: Thursday, August 25, 2022 11:48 AM
> To: devel@edk2.groups.io
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
> Zhang, Qi1 <qi1.zhang@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>;
> Yao, Jiewen <jiewen.yao@intel.com>
> Subject: [PATCH v2 5/8] SecurityPkg/SmmTcg2PhysicalPresenceLib: Add missing
> debug print specifier
> 
> From: Michael Kubacki <michael.kubacki@microsoft.com>
> 
> The debug macro modified in this change was missing a print specifier
> for a debug message argument given.
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Qi Zhang <qi1.zhang@intel.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
> ---
> 
> SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/MmTcg2PhysicalPresenceLib
> Common.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git
> a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/MmTcg2PhysicalPresenceLi
> bCommon.c
> b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/MmTcg2PhysicalPresenceLi
> bCommon.c
> index 1fbfc00547cd..f2ab4f125007 100644
> ---
> a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/MmTcg2PhysicalPresenceLi
> bCommon.c
> +++
> b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/MmTcg2PhysicalPresenceLi
> bCommon.c
> @@ -176,7 +176,7 @@
> Tcg2PhysicalPresenceLibSubmitRequestToPreOSFunctionEx (
>    // Sync PPRQ/PPRM from PP Variable if PP submission fails
>    //
>    if (ReturnCode != TCG_PP_SUBMIT_REQUEST_TO_PREOS_SUCCESS) {
> -    DEBUG ((DEBUG_ERROR, "[TPM2] Submit PP Request failure! Sync
> PPRQ/PPRM with PP variable.\n", Status));
> +    DEBUG ((DEBUG_ERROR, "[TPM2] Submit PP Request failure! Sync
> PPRQ/PPRM with PP variable. Status = %r\n", Status));
>      DataSize = sizeof (EFI_TCG2_PHYSICAL_PRESENCE);
>      ZeroMem (&PpData, DataSize);
>      Status = mTcg2PpSmmVariable->SmmGetVariable (
> --
> 2.28.0.windows.1

[PATCH v2 6/8] DynamicTablesPkg/AcpiPpttLibArm: Fix debug macro arguments

[Michael Kubacki]

From: Michael Kubacki <michael.kubacki@microsoft.com>

Cc: Sami Mujawar <Sami.Mujawar@arm.com>
Cc: Alexei Fedorov <Alexei.Fedorov@arm.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
---
 DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
index 59001378c4e0..78fa63ff47e5 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
@@ -436,7 +436,6 @@ IsGicCTokenEqual (
       "the same GICC Info object. ACPI Processor IDs are not unique. " \
       "GicCToken = %p.\n",
       Index1,
-      IndexedObject1->Token,
       Index2,
       ProcNode1->GicCToken
       ));
@@ -566,7 +565,7 @@ AddProcHierarchyNodes (
         DEBUG ((
           DEBUG_ERROR,
           "ERROR: PPTT: Failed to get parent processor hierarchy node " \
-          "reference. Token = %p, Status = %r\n",
+          "reference. ParentToken = %p. ChildToken = %p. Status = %r\n",
           ProcInfoNode->ParentToken,
           ProcInfoNode->Token,
           Status
-- 
2.28.0.windows.1

Re: [PATCH v2 6/8] DynamicTablesPkg/AcpiPpttLibArm: Fix debug macro arguments

[Sami Mujawar]

Hi Michael,

Thank you for this fix.

These changes look good to me.

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar

On 25/08/2022 04:48 am, mikuback@linux.microsoft.com wrote:
> From: Michael Kubacki <michael.kubacki@microsoft.com>
>
> Cc: Sami Mujawar <Sami.Mujawar@arm.com>
> Cc: Alexei Fedorov <Alexei.Fedorov@arm.com>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> ---
>   DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
> index 59001378c4e0..78fa63ff47e5 100644
> --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
> +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
> @@ -436,7 +436,6 @@ IsGicCTokenEqual (
>         "the same GICC Info object. ACPI Processor IDs are not unique. " \
>         "GicCToken = %p.\n",
>         Index1,
> -      IndexedObject1->Token,
>         Index2,
>         ProcNode1->GicCToken
>         ));
> @@ -566,7 +565,7 @@ AddProcHierarchyNodes (
>           DEBUG ((
>             DEBUG_ERROR,
>             "ERROR: PPTT: Failed to get parent processor hierarchy node " \
> -          "reference. Token = %p, Status = %r\n",
> +          "reference. ParentToken = %p. ChildToken = %p. Status = %r\n",
>             ProcInfoNode->ParentToken,
>             ProcInfoNode->Token,
>             Status

[PATCH v2 7/8] NetworkPkg/TcpDxe: Fix debug macro arguments

[Michael Kubacki]

From: Michael Kubacki <michael.kubacki@microsoft.com>

Removes Status argument that is not needed from DEBUG macros.

Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
---
 NetworkPkg/TcpDxe/SockInterface.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/NetworkPkg/TcpDxe/SockInterface.c b/NetworkPkg/TcpDxe/SockInterface.c
index 413d6e1373f3..6e8cbb74a86b 100644
--- a/NetworkPkg/TcpDxe/SockInterface.c
+++ b/NetworkPkg/TcpDxe/SockInterface.c
@@ -661,11 +661,7 @@ SockSend (
                   );
 
     if (NULL == SockToken) {
-      DEBUG (
-        (DEBUG_ERROR,
-         "SockSend: Failed to buffer IO token into socket processing SndToken List\n",
-         Status)
-        );
+      DEBUG ((DEBUG_ERROR, "SockSend: Failed to buffer IO token into socket processing SndToken List\n"));
 
       Status = EFI_OUT_OF_RESOURCES;
       goto Exit;
@@ -674,11 +670,7 @@ SockSend (
     Status = SockProcessTcpSndData (Sock, TxData);
 
     if (EFI_ERROR (Status)) {
-      DEBUG (
-        (DEBUG_ERROR,
-         "SockSend: Failed to process Snd Data\n",
-         Status)
-        );
+      DEBUG ((DEBUG_ERROR, "SockSend: Failed to process Snd Data\n"));
 
       RemoveEntryList (&(SockToken->TokenList));
       FreePool (SockToken);
-- 
2.28.0.windows.1

Re: [PATCH v2 7/8] NetworkPkg/TcpDxe: Fix debug macro arguments

[Maciej Rabeda]

Thanks for the patch, Michael.

Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>

On 25 sie 2022 05:48, mikuback@linux.microsoft.com wrote:
> From: Michael Kubacki <michael.kubacki@microsoft.com>
>
> Removes Status argument that is not needed from DEBUG macros.
>
> Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Siyuan Fu <siyuan.fu@intel.com>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> ---
>   NetworkPkg/TcpDxe/SockInterface.c | 12 ++----------
>   1 file changed, 2 insertions(+), 10 deletions(-)
>
> diff --git a/NetworkPkg/TcpDxe/SockInterface.c b/NetworkPkg/TcpDxe/SockInterface.c
> index 413d6e1373f3..6e8cbb74a86b 100644
> --- a/NetworkPkg/TcpDxe/SockInterface.c
> +++ b/NetworkPkg/TcpDxe/SockInterface.c
> @@ -661,11 +661,7 @@ SockSend (
>                     );
>   
>       if (NULL == SockToken) {
> -      DEBUG (
> -        (DEBUG_ERROR,
> -         "SockSend: Failed to buffer IO token into socket processing SndToken List\n",
> -         Status)
> -        );
> +      DEBUG ((DEBUG_ERROR, "SockSend: Failed to buffer IO token into socket processing SndToken List\n"));
>   
>         Status = EFI_OUT_OF_RESOURCES;
>         goto Exit;
> @@ -674,11 +670,7 @@ SockSend (
>       Status = SockProcessTcpSndData (Sock, TxData);
>   
>       if (EFI_ERROR (Status)) {
> -      DEBUG (
> -        (DEBUG_ERROR,
> -         "SockSend: Failed to process Snd Data\n",
> -         Status)
> -        );
> +      DEBUG ((DEBUG_ERROR, "SockSend: Failed to process Snd Data\n"));
>   
>         RemoveEntryList (&(SockToken->TokenList));
>         FreePool (SockToken);

[PATCH v2 8/8] OvmfPkg/LegacyBootManagerLib: Fix debug macro arguments

[Michael Kubacki]

From: Michael Kubacki <michael.kubacki@microsoft.com>

The DEBUG macro updated in this patch previously contained 11 print
specifiers in the debug string but passeed 13 arguments. This change
attempts to update the macro to the author's intention so the number
of specifiers match the number of arguments.

Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
---
 OvmfPkg/Csm/LegacyBootManagerLib/LegacyBm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/OvmfPkg/Csm/LegacyBootManagerLib/LegacyBm.c b/OvmfPkg/Csm/LegacyBootManagerLib/LegacyBm.c
index 032aacba68de..2e92bce726a9 100644
--- a/OvmfPkg/Csm/LegacyBootManagerLib/LegacyBm.c
+++ b/OvmfPkg/Csm/LegacyBootManagerLib/LegacyBm.c
@@ -1159,8 +1159,8 @@ LegacyBmPrintBbsTable (
   UINT16  Index;
 
   DEBUG ((DEBUG_INFO, "\n"));
-  DEBUG ((DEBUG_INFO, " NO  Prio bb/dd/ff cl/sc Type Stat segm:offs\n"));
-  DEBUG ((DEBUG_INFO, "=============================================\n"));
+  DEBUG ((DEBUG_INFO, " NO  Prio bb/dd/ff cl/sc Type Stat segm:offs mseg dseg\n"));
+  DEBUG ((DEBUG_INFO, "======================================================\n"));
   for (Index = 0; Index < BbsCount; Index++) {
     if (!LegacyBmValidBbsEntry (&LocalBbsTable[Index])) {
       continue;
@@ -1168,7 +1168,7 @@ LegacyBmPrintBbsTable (
 
     DEBUG (
       (DEBUG_INFO,
-       " %02x: %04x %02x/%02x/%02x %02x/%02x %04x %04x %04x:%04x\n",
+       " %02x: %04x %02x/%02x/%02x %02x/%02x %04x %04x %04x:%04x %04x %04x\n",
        (UINTN)Index,
        (UINTN)LocalBbsTable[Index].BootPriority,
        (UINTN)LocalBbsTable[Index].Bus,
-- 
2.28.0.windows.1