On 04/21/16 08:57, Ruiyu Ni wrote:
> Since PlatformBootManagerLib do not run memory test
> to convert untested memory to tested.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ruiyu Ni <ruiyu...@intel.com>
> Cc: Jordan Justen <jordan.l.jus...@intel.com>
> Cc: Laszlo Ersek <ler...@redhat.com>
> ---
> OvmfPkg/PlatformPei/MemDetect.c | 4 ++--
> OvmfPkg/PlatformPei/Platform.c | 29 -----------------------------
> OvmfPkg/PlatformPei/Platform.h | 14 +-------------
> OvmfPkg/PlatformPei/Xen.c | 8 ++------
> 4 files changed, 5 insertions(+), 50 deletions(-)
>
> diff --git a/OvmfPkg/PlatformPei/MemDetect.c b/OvmfPkg/PlatformPei/MemDetect.c
> index ed13c57..7991ba2 100644
> --- a/OvmfPkg/PlatformPei/MemDetect.c
> +++ b/OvmfPkg/PlatformPei/MemDetect.c
> @@ -1,7 +1,7 @@
> /**@file
> Memory Detection for Virtual Machines.
>
> - Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
> License
> which accompanies this distribution. The full text of the license may be
> found at
> @@ -422,7 +422,7 @@ QemuInitializeRam (
> }
>
> if (UpperMemorySize != 0) {
> - AddUntestedMemoryBaseSizeHob (BASE_4GB, UpperMemorySize);
> + AddMemoryBaseSizeHob (BASE_4GB, UpperMemorySize);
> }
> }
Hmmm. I'm reminded of the following discussions:
http://thread.gmane.org/gmane.comp.bios.edk2.devel/1023/focus=1124
http://thread.gmane.org/gmane.comp.bios.edk2.devel/2385/focus=2425
The worry here is that adding the >=4GB memory descriptor as tested
(rather than as untested) could cause CoreInitializeMemoryServices() to
change its behavior, moving the very first memory region -- that is
declared at the end of the function -- from within OVMF's permanent PEI
RAM to someplace else. We don't want that.
... Actually, I think this should be safe at this point. The worry I
described was real before Star's commit 3a05b13106d15 (= the subject of
the second thread that I linked above). However, with commit
3a05b13106d15 in place, I think the DXE core will prefer OVMF's
permanent PEI RAM for initializing the memory services *even if* a >=4GB
*tested* memory descriptor is present.
(1) Ray, can you please verify this? It is not hard:
(1a) first boot OVMF without your patches applied, and from the debug
log, save the following lines:
PeiInstallPeiMemory MemoryBegin ..., MemoryLength ...
CoreInitializeMemoryServices:
BaseAddress - ... Length - ...
The area printed by CoreInitializeMemoryServices is currently inside the
area printed by PeiInstallPeiMemory. That's what we should preserve.
(1b) Then please boot OVMF with your patches applied, and look at the
same lines. The addresses need not be exactly identical, but the area
printed by CoreInitializeMemoryServices should continue to be a subset
of the area printed by PeiInstallPeiMemory. Can you confirm this?
... I cherry-picked this patch (commit 9e6e5876471d) from your Ovmf_Bds2
branch, on top of current master, and I tested it in isolation. (I
couldn't test the Xen change, but I tested the QEMU change.)
I checked a number of RAM sizes (4GB, 6GB, 16GB) and a number of configs
(i440fx (-> without SMM), including S3, and Q35 (with SMM), including
S3). Everything seemed to work.
I also compared the OVMF logs; there were practically no differences
(definitely not in the area where I worried). The amount of RAM seen
within the guest OSes was fine as well.
For this patch:
Tested-by: Laszlo Ersek <ler...@redhat.com>
Reviewed-by: Laszlo Ersek <ler...@redhat.com>
(2) A general question: why has the memory testing (= conversion from
untested to tested) been removed from BDS?
It wasn't doing anything useful for OVMF anyway, I'm just curious what
else is supposed to do this memory testing on physical platforms, when
using BDS from MdeModulePkg.
Thanks
Laszlo
>
> diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
> index ef654c4..4be9922 100644
> --- a/OvmfPkg/PlatformPei/Platform.c
> +++ b/OvmfPkg/PlatformPei/Platform.c
> @@ -152,35 +152,6 @@ AddMemoryRangeHob (
>
>
> VOID
> -AddUntestedMemoryBaseSizeHob (
> - EFI_PHYSICAL_ADDRESS MemoryBase,
> - UINT64 MemorySize
> - )
> -{
> - BuildResourceDescriptorHob (
> - EFI_RESOURCE_SYSTEM_MEMORY,
> - EFI_RESOURCE_ATTRIBUTE_PRESENT |
> - EFI_RESOURCE_ATTRIBUTE_INITIALIZED |
> - EFI_RESOURCE_ATTRIBUTE_UNCACHEABLE |
> - EFI_RESOURCE_ATTRIBUTE_WRITE_COMBINEABLE |
> - EFI_RESOURCE_ATTRIBUTE_WRITE_THROUGH_CACHEABLE |
> - EFI_RESOURCE_ATTRIBUTE_WRITE_BACK_CACHEABLE,
> - MemoryBase,
> - MemorySize
> - );
> -}
> -
> -
> -VOID
> -AddUntestedMemoryRangeHob (
> - EFI_PHYSICAL_ADDRESS MemoryBase,
> - EFI_PHYSICAL_ADDRESS MemoryLimit
> - )
> -{
> - AddUntestedMemoryBaseSizeHob (MemoryBase, (UINT64)(MemoryLimit -
> MemoryBase));
> -}
> -
> -VOID
> MemMapInitialization (
> VOID
> )
> diff --git a/OvmfPkg/PlatformPei/Platform.h b/OvmfPkg/PlatformPei/Platform.h
> index dad3c61..bb988ea 100644
> --- a/OvmfPkg/PlatformPei/Platform.h
> +++ b/OvmfPkg/PlatformPei/Platform.h
> @@ -1,7 +1,7 @@
> /** @file
> Platform PEI module include file.
>
> - Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
> License
> which accompanies this distribution. The full text of the license may be
> found at
> @@ -42,12 +42,6 @@ AddMemoryRangeHob (
> );
>
> VOID
> -AddUntestedMemoryBaseSizeHob (
> - EFI_PHYSICAL_ADDRESS MemoryBase,
> - UINT64 MemorySize
> - );
> -
> -VOID
> AddReservedMemoryBaseSizeHob (
> EFI_PHYSICAL_ADDRESS MemoryBase,
> UINT64 MemorySize,
> @@ -55,12 +49,6 @@ AddReservedMemoryBaseSizeHob (
> );
>
> VOID
> -AddUntestedMemoryRangeHob (
> - EFI_PHYSICAL_ADDRESS MemoryBase,
> - EFI_PHYSICAL_ADDRESS MemoryLimit
> - );
> -
> -VOID
> AddressWidthInitialization (
> VOID
> );
> diff --git a/OvmfPkg/PlatformPei/Xen.c b/OvmfPkg/PlatformPei/Xen.c
> index 7fa9019..3a43582 100644
> --- a/OvmfPkg/PlatformPei/Xen.c
> +++ b/OvmfPkg/PlatformPei/Xen.c
> @@ -1,7 +1,7 @@
> /**@file
> Xen Platform PEI support
>
> - Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
> Copyright (c) 2011, Andrei Warkentin <andr...@motorola.com>
>
> This program and the accompanying materials
> @@ -189,11 +189,7 @@ XenPublishRamRegions (
> continue;
> }
>
> - if (Entry->BaseAddr >= BASE_4GB) {
> - AddUntestedMemoryBaseSizeHob (Entry->BaseAddr, Entry->Length);
> - } else {
> - AddMemoryBaseSizeHob (Entry->BaseAddr, Entry->Length);
> - }
> + AddMemoryBaseSizeHob (Entry->BaseAddr, Entry->Length);
>
> MtrrSetMemoryAttribute (Entry->BaseAddr, Entry->Length,
> CacheWriteBack);
> }
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
