On 8 April 2015 at 16:25, Laszlo Ersek <ler...@redhat.com> wrote: > On 04/08/15 09:11, Ard Biesheuvel wrote: >> This updates ArmVirtualizationMemoryInitPeiLib so that the PEI memory >> region, i.e., the region that is used both before and after the MMU >> and caches are enabled, is invalidated by virtual address before >> enabling the MMU. >> >> This prevents issues where data we modified with the caches and MMU >> off may be shadowed by clean cachelines in system caches or in lower >> level caches on other CPUs, resulting in the this data to become >> invisible once we turn the MMU and caches on. >> >> Also reduce the size of the region to 16 MB (from 64 MB), to reduce >> the potential performance hit from invalidating the entire region by >> virtual address. >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org> >> --- >> ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationQemu.dsc >> | 4 ++-- >> ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationXen.dsc >> | 4 ++-- >> >> ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationMemoryInitPeiLib/ArmVirtualizationMemoryInitPeiLib.c >> | 10 ++++++++++ >> >> ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationMemoryInitPeiLib/ArmVirtualizationMemoryInitPeiLib.inf >> | 1 + >> 4 files changed, 15 insertions(+), 4 deletions(-) >> >> diff --git a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationQemu.dsc >> b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationQemu.dsc >> index cad7353442f7..0b9a46f0197f 100644 >> --- a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationQemu.dsc >> +++ b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationQemu.dsc >> @@ -103,8 +103,8 @@ >> gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x4007c000 >> gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0x4000 >> >> - # Size of the region used by UEFI in permanent memory (Reserved 64MB) >> - gArmPlatformTokenSpaceGuid.PcdSystemMemoryUefiRegionSize|0x04000000 >> + # Size of the region used by UEFI in permanent memory (Reserved 16MB) >> + gArmPlatformTokenSpaceGuid.PcdSystemMemoryUefiRegionSize|0x01000000 >> >> # >> # ARM Pcds >> diff --git a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationXen.dsc >> b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationXen.dsc >> index d01b3e9c8494..71869845a857 100644 >> --- a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationXen.dsc >> +++ b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationXen.dsc >> @@ -89,8 +89,8 @@ >> >> gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0x4000 >> >> - # Size of the region used by UEFI in permanent memory (Reserved 64MB) >> - gArmPlatformTokenSpaceGuid.PcdSystemMemoryUefiRegionSize|0x04000000 >> + # Size of the region used by UEFI in permanent memory (Reserved 16MB) >> + gArmPlatformTokenSpaceGuid.PcdSystemMemoryUefiRegionSize|0x01000000 >> >> # >> # ARM Virtual Architectural Timer >> diff --git >> a/ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationMemoryInitPeiLib/ArmVirtualizationMemoryInitPeiLib.c >> >> b/ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationMemoryInitPeiLib/ArmVirtualizationMemoryInitPeiLib.c >> index 5f6cd059c47f..8ce63b4596e2 100644 >> --- >> a/ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationMemoryInitPeiLib/ArmVirtualizationMemoryInitPeiLib.c >> +++ >> b/ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationMemoryInitPeiLib/ArmVirtualizationMemoryInitPeiLib.c >> @@ -20,6 +20,7 @@ >> #include <Library/HobLib.h> >> #include <Library/MemoryAllocationLib.h> >> #include <Library/PcdLib.h> >> +#include <Library/CacheMaintenanceLib.h> >> >> VOID >> BuildMemoryTypeInformationHob ( >> @@ -79,6 +80,15 @@ MemoryPeim ( >> PcdGet64 (PcdSystemMemorySize) >> ); >> >> + // >> + // When running under virtualization, the PI/UEFI memory region may be >> + // clean but not invalidated in system caches or in lower level caches >> + // on other CPUs. So invalidate the region by virtual address, to ensure >> + // that the contents we put there with the caches and MMU off will still >> + // be visible after turning them on. >> + // >> + InvalidateDataCacheRange ((VOID*)(UINTN)UefiMemoryBase, UefiMemorySize); >> + >> // Build Memory Allocation Hob >> InitMmu (); >> >> diff --git >> a/ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationMemoryInitPeiLib/ArmVirtualizationMemoryInitPeiLib.inf >> >> b/ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationMemoryInitPeiLib/ArmVirtualizationMemoryInitPeiLib.inf >> index fcdae06de7c2..b8a19c993d91 100644 >> --- >> a/ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationMemoryInitPeiLib/ArmVirtualizationMemoryInitPeiLib.inf >> +++ >> b/ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationMemoryInitPeiLib/ArmVirtualizationMemoryInitPeiLib.inf >> @@ -35,6 +35,7 @@ >> HobLib >> ArmLib >> ArmPlatformLib >> + CacheMaintenanceLib >> >> [Guids] >> gEfiMemoryTypeInformationGuid >> > > Unfortunately, although I theoretically verified the correctness of this > patch (by eyeballing), in practice 16 MB is not enough for the DXE > Initial Program Load PEIM, in the QEMU build: > >> Loading PEIM at 0x000BFF4B160 EntryPoint=0x000BFF4B260 DxeIpl.efi >> Install PPI: [LzmaCustomDecompress] >> Install PPI: [EfiDxeIplPpi] >> Install PPI: [EfiPeiDecompressPpi] >> Customized Guided section Memory Size required is 0x90ECD0 and address >> is 0xBF62B000 >> AllocatePages failed: No 0x90F Pages is available. >> There is only left 0x608 pages memory resource to be allocated. >> Out of memory resource! >> AllocatePages failed: No 0x90F Pages is available. >> There is only left 0x608 pages memory resource to be allocated. >> Out of memory resource! >> DXE IPL Entry >> >> ASSERT_EFI_ERROR (Status = Not Found) >> ASSERT MdeModulePkg/Core/DxeIplPeim/DxeLoad.c(399): !EFI_ERROR >> (Status) > > If I set PcdSystemMemoryUefiRegionSize to 32MB (0x02000000) in > "ArmVirtualizationQemu.dsc", then the error disappears, and then the > patchset even helps the QEMU build -- I can then boot the > ArmVirtualizationQemu binary on Seattle (KVM) too, not just on Mustang. >
Great! > (Otherwise, without the patch series, ArmVirtualizationQemu hangs on > Seattle, after printing the following message: > >> Loading PEIM at 0x000BFF89160 EntryPoint=0x000BFF89260 CpuPei.efi > ) > > So, the cache invalidation certainly seems beneficial for even the QEMU > build, but I'm now doubting if we should decrease > PcdSystemMemoryUefiRegionSize *for that build* at all. > > The commit message says "potential performance hit" -- I'd say until we > get hard numbers or complaints that performance is bad, let's just drop > the "ArmVirtualizationQemu.dsc" hunk. Even if there's some penalty, it's > one time only. > OK, I will drop it. The performance hit, while noticeable, is not /that/ bad, so let's not optimize prematurely. -- Ard. ------------------------------------------------------------------------------ BPM Camp - Free Virtual Workshop May 6th at 10am PDT/1PM EDT Develop your own process in accordance with the BPMN 2 standard Learn Process modeling best practices with Bonita BPM through live exercises http://www.bonitasoft.com/be-part-of-it/events/bpm-camp-virtual- event?utm_ source=Sourceforge_BPM_Camp_5_6_15&utm_medium=email&utm_campaign=VA_SF _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel