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. (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. Thanks Laszlo ------------------------------------------------------------------------------ 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
