Re: [PATCH 07/10] EmulatorPkg/AutoScanPei: Report the correct CPU address size

["Wu, Hao A" <hao.a.wu@intel.com>]

> -----Original Message-----
> From: Ni, Ruiyu
> Sent: Monday, August 27, 2018 3:53 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A; Andrew Fish
> Subject: [PATCH 07/10] EmulatorPkg/AutoScanPei: Report the correct CPU
> address size
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1119
> 
> Today's implementation unconditionally reports CPU address size
> as 36 through CPU HOB. But when WinHost is running at 64bit,
> the system memory might be allocated above 2^36.
> 
> It causes system asserts when DxeCore code tries to find the
> corresponding GCD entry for a given valid address.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> Cc: Andrew Fish <afish@apple.com>
> ---
>  EmulatorPkg/AutoScanPei/AutoScanPei.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/EmulatorPkg/AutoScanPei/AutoScanPei.c
> b/EmulatorPkg/AutoScanPei/AutoScanPei.c
> index 78a40db3a2..bf9958a4a9 100644
> --- a/EmulatorPkg/AutoScanPei/AutoScanPei.c
> +++ b/EmulatorPkg/AutoScanPei/AutoScanPei.c
> @@ -1,6 +1,6 @@
>  /*++ @file
> 
> -Copyright (c) 2006 - 2008, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
>  Portions copyright (c) 2011, Apple Inc. All rights reserved.
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD
> License
> @@ -51,7 +51,8 @@ Returns:
>    EFI_PHYSICAL_ADDRESS        MemoryBase;
>    UINTN                       Index;
>    EFI_RESOURCE_ATTRIBUTE_TYPE Attributes;
> -
> +  UINTN                       HighBitSet;
> +  UINT8                       SizeOfMemorySpace;
> 
>    DEBUG ((EFI_D_ERROR, "Emu Autoscan PEIM Loaded\n"));
> 
> @@ -66,7 +67,8 @@ Returns:
>               );
>    ASSERT_EFI_ERROR (Status);
> 
> -  Index = 0;
> +  SizeOfMemorySpace = 0;
> +  Index             = 0;
>    do {
>      Status = Thunk->MemoryAutoScan (Index, &MemoryBase, &MemorySize);
>      if (!EFI_ERROR (Status)) {
> @@ -96,6 +98,12 @@ Returns:
>          MemoryBase,
>          MemorySize
>          );
> +      HighBitSet        = HighBitSet64 (MemoryBase + MemorySize);
> +      SizeOfMemorySpace = MAX (SizeOfMemorySpace, (UINT8)HighBitSet + 1);
> +      DEBUG ((
> +        DEBUG_INFO, "Emu Memory Discoverred[%d]: Base = %016lx / Size
> = %016x\n",
> +        Index, MemoryBase, MemorySize
> +        ));
>      }
>      Index++;
>    } while (!EFI_ERROR (Status));
> @@ -103,7 +111,8 @@ Returns:
>    //
>    // Build the CPU hob with 36-bit addressing and 16-bits of IO space.
>    //
> -  BuildCpuHob (36, 16);
> +  DEBUG ((DEBUG_INFO, "Emu SizeOfMemorySpace = %d\n",
> SizeOfMemorySpace));
> +  BuildCpuHob (SizeOfMemorySpace, 16);

Hi Ray,

Just a question, I saw the implementation in Nt32Pkg just hard-code the
address size to 52 to support both 32-bit and 64-bit. Is there a concern
for hard coding it to 52?

Best Regards,
Hao Wu

> 
>    return Status;
>  }
> --
> 2.16.1.windows.1