Re: [edk2-devel] [PATCH 2/2] OvmfPkg/PlatformPei: prefer etc/e820 for memory detection

["Yao, Jiewen" <jiewen.yao@intel.com>]

Hi
Would you please follow EDKII process?
1) File an EDKII Bugzilla.
2) CC all reviewers in OVMF package.


Please also describe what the reason of change, what is the benefit we can get from the change?

I just feel it is confusing. Previously, the data is consistent from CMOS. Now, we have two ways to get one data from different sources.

Please help me understand:

A) What if the data are different from different source?

B) Why we choose to trust E820 at first, the CMOS? Not verse versa.

C) If we trust E820 (in B), then why we need go back to CMOS, if LowMemorySize is 0?

Thank you
Yao Jiewen


> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gerd
> Hoffmann
> Sent: Thursday, August 19, 2021 4:11 PM
> To: devel@edk2.groups.io
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Subject: [edk2-devel] [PATCH 2/2] OvmfPkg/PlatformPei: prefer etc/e820 for
> memory detection
> 
> Prefer the e820 map provided via qemu firmware config interface
> for memory detection.  Use rtc cmos only as fallback, which should
> be rarely needed these days as qemu supports etc/e820 since 2013.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  OvmfPkg/PlatformPei/MemDetect.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/OvmfPkg/PlatformPei/MemDetect.c
> b/OvmfPkg/PlatformPei/MemDetect.c
> index d7fb3e742be3..71bd28bd1d2b 100644
> --- a/OvmfPkg/PlatformPei/MemDetect.c
> +++ b/OvmfPkg/PlatformPei/MemDetect.c
> @@ -293,9 +293,16 @@ GetSystemMemorySizeBelow4gb (
>    VOID
>    )
>  {
> +  EFI_STATUS Status;
> +  UINT64 LowerMemorySize;
>    UINT8 Cmos0x34;
>    UINT8 Cmos0x35;
> 
> +  Status = ScanOrAdd64BitE820Ram (FALSE, &LowerMemorySize, NULL);
> +  if (Status == EFI_SUCCESS || LowerMemorySize > 0) {
> +    return LowerMemorySize;
> +  }
> +
>    //
>    // CMOS 0x34/0x35 specifies the system memory above 16 MB.
>    // * CMOS(0x35) is the high byte
> @@ -722,7 +729,6 @@ QemuInitializeRam (
>    // Determine total memory size available
>    //
>    LowerMemorySize = GetSystemMemorySizeBelow4gb ();
> -  UpperMemorySize = GetSystemMemorySizeAbove4gb ();
> 
>    if (mBootMode == BOOT_ON_S3_RESUME) {
>      //
> @@ -769,8 +775,11 @@ QemuInitializeRam (
>      // memory size read from the CMOS.
>      //
>      Status = ScanOrAdd64BitE820Ram (TRUE, NULL, NULL);
> -    if (EFI_ERROR (Status) && UpperMemorySize != 0) {
> -      AddMemoryBaseSizeHob (BASE_4GB, UpperMemorySize);
> +    if (EFI_ERROR (Status)) {
> +      UpperMemorySize = GetSystemMemorySizeAbove4gb ();
> +      if (UpperMemorySize != 0) {
> +        AddMemoryBaseSizeHob (BASE_4GB, UpperMemorySize);
> +      }
>      }
>    }
> 
> --
> 2.31.1
> 
> 
> 
> 
>