The DEBUG print that outputs the base and size of the page table
allocation always prints 0x0 for the size, given that BufferSize will be
updated by PageTableMap () and contain the unused allocation on return.

So move the DEBUG print right after the allocation.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 UefiCpuPkg/CpuMpPei/CpuPaging.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c b/UefiCpuPkg/CpuMpPei/CpuPaging.c
index b7ddb0005b6fbcac..175e47ccd737a0c1 100644
--- a/UefiCpuPkg/CpuMpPei/CpuPaging.c
+++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c
@@ -396,6 +396,13 @@ EnablePaePageTable (
     return EFI_OUT_OF_RESOURCES;
   }
 
+  DEBUG ((
+    DEBUG_INFO,
+    "EnablePaePageTable: Created PageTable = 0x%x, BufferSize = %x\n",
+    PageTable,
+    BufferSize
+    ));
+
   Status = PageTableMap (&PageTable, PagingPae, Buffer, &BufferSize, 0, SIZE_4GB, &MapAttribute, &MapMask, NULL);
   ASSERT_EFI_ERROR (Status);
   if (EFI_ERROR (Status) || (PageTable == 0)) {
@@ -417,13 +424,6 @@ EnablePaePageTable (
   //
   AsmWriteCr0 (AsmReadCr0 () | BIT31);
 
-  DEBUG ((
-    DEBUG_INFO,
-    "EnablePaePageTable: Created PageTable = 0x%x, BufferSize = %x\n",
-    PageTable,
-    BufferSize
-    ));
-
   return Status;
 }
 
-- 
2.39.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105942): https://edk2.groups.io/g/devel/message/105942
Mute This Topic: https://groups.io/mt/99411874/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Reviewed-by: Michael Kubacki <michael.kubacki@microsoft.com>

A couple comments below.

On 6/8/2023 1:23 PM, Ard Biesheuvel wrote:
> The DEBUG print that outputs the base and size of the page table
> allocation always prints 0x0 for the size, given that BufferSize will be
> updated by PageTableMap () and contain the unused allocation on return.
> 
> So move the DEBUG print right after the allocation.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>   UefiCpuPkg/CpuMpPei/CpuPaging.c | 14 +++++++-------
>   1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c b/UefiCpuPkg/CpuMpPei/CpuPaging.c
> index b7ddb0005b6fbcac..175e47ccd737a0c1 100644
> --- a/UefiCpuPkg/CpuMpPei/CpuPaging.c
> +++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c
> @@ -396,6 +396,13 @@ EnablePaePageTable (
>       return EFI_OUT_OF_RESOURCES;
> 
>     }
> 
>   
> 
> +  DEBUG ((
> 
> +    DEBUG_INFO,
> 
> +    "EnablePaePageTable: Created PageTable = 0x%x, BufferSize = %x\n",
> 
> +    PageTable,
> 
> +    BufferSize
> 
> +    ));
> 
> +

In the past, a point was made to improve portability between 32-bit and 
64-bit architectures by casting UINTN values to UINT64 and then printing 
them %Lx. If this is a DEBUG only change that might be worth adding as well.

In any case, can you please prefix the print specifier for BufferSize 
with "0x"?

> 
>     Status = PageTableMap (&PageTable, PagingPae, Buffer, &BufferSize, 0, SIZE_4GB, &MapAttribute, &MapMask, NULL);
> 
>     ASSERT_EFI_ERROR (Status);
> 
>     if (EFI_ERROR (Status) || (PageTable == 0)) {
> 
> @@ -417,13 +424,6 @@ EnablePaePageTable (
>     //
> 
>     AsmWriteCr0 (AsmReadCr0 () | BIT31);
> 
>   
> 
> -  DEBUG ((
> 
> -    DEBUG_INFO,
> 
> -    "EnablePaePageTable: Created PageTable = 0x%x, BufferSize = %x\n",
> 
> -    PageTable,
> 
> -    BufferSize
> 
> -    ));
> 
> -
> 
>     return Status;
> 
>   }
> 
>   
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105951): https://edk2.groups.io/g/devel/message/105951
Mute This Topic: https://groups.io/mt/99411874/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-