| Message ID | 1428477061-1768-5-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 8 April 2015 at 16:25, Laszlo Ersek <lersek@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.biesheuvel@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.
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
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.biesheuvel@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(-)