diff mbox

[edk2,v4,4/5] ArmVirtualizationPkg: invalidate PEI memory region by VA

Message ID 1428578932-24079-5-git-send-email-ard.biesheuvel@linaro.org
State New
Headers show

Commit Message

Ard Biesheuvel April 9, 2015, 11:28 a.m. UTC 
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
Reviewed-By: Olivier Martin <Olivier.Martin@arm.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationMemoryInitPeiLib/ArmVirtualizationMemoryInitPeiLib.c   | 10 ++++++++++
 ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationMemoryInitPeiLib/ArmVirtualizationMemoryInitPeiLib.inf |  1 +
 2 files changed, 11 insertions(+)

Comments

Roy Franz April 9, 2015, 5:12 p.m. UTC | #1 
On Thu, Apr 9, 2015 at 4:28 AM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> 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.
This bit of the patch is removed, so this bit of the commit message
should go too :)
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Reviewed-By: Olivier Martin <Olivier.Martin@arm.com>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationMemoryInitPeiLib/ArmVirtualizationMemoryInitPeiLib.c   | 10 ++++++++++
>  ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationMemoryInitPeiLib/ArmVirtualizationMemoryInitPeiLib.inf |  1 +
>  2 files changed, 11 insertions(+)
>
> 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
> --
> 1.8.3.2
>

------------------------------------------------------------------------------
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
Ard Biesheuvel April 9, 2015, 5:48 p.m. UTC | #2 
On 9 April 2015 at 19:12, Roy Franz <roy.franz@linaro.org> wrote:
> On Thu, Apr 9, 2015 at 4:28 AM, Ard Biesheuvel
> <ard.biesheuvel@linaro.org> 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.
> This bit of the patch is removed, so this bit of the commit message
> should go too :)

Ah yes, thanks for catching that.

Perhaps Olivier can just remove this paragraph when committing?
(assuming he is happy with the series, and patch #2 doesn't still
break most of the bare metal platforms)

Cheers,
Ard.

>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Reviewed-By: Olivier Martin <Olivier.Martin@arm.com>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationMemoryInitPeiLib/ArmVirtualizationMemoryInitPeiLib.c   | 10 ++++++++++
>>  ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationMemoryInitPeiLib/ArmVirtualizationMemoryInitPeiLib.inf |  1 +
>>  2 files changed, 11 insertions(+)
>>
>> 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
>> --
>> 1.8.3.2
>>

------------------------------------------------------------------------------
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
diff mbox

Patch

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