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

Reply via email to