ArmPlatformPkg/PrePi: replace set/way cache ops with by-VA ones

Cache maintenance operations by set/way are only intended to be used
in the context of on/offlining a core, while it has been taken out of
the coherency domain. Any use intended to ensure that the contents of
the cache have made it to main memory is unreliable, since cacheline
migration and non-architected system caches may cause these contents
to linger elsewhere, without being visible in main memory once the
MMU and caches are disabled.

In KVM on Linux, there are horrid hacks in place to ensure that such
set/way operations are trapped, and replaced with a single by-VA
clean/invalidate of the entire guest VA space once the MMU state
changes, which can be costly, and is unnecessary if we manage the
caches a bit more carefully, and perform maintenance by virtual
address only.

So let's get rid of the call to ArmInvalidateDataCache () in the
PrePeiCore startup code, and instead, invalidate the UEFI memory
region by virtual address, which is the only memory region we will
be touching with the caches and MMU both disabled and enabled.
(This will lead to data corruption if data written with the MMU off
is shadowed by clean, stale cachelines that stick around when the
MMU is enabled again.)

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Pete Batard <pete@akeo.ie>
Reviewed-by: Leif Lindholm <leif@nuviainc.com>

diff --git a/ArmPlatformPkg/PrePi/PeiMPCore.inf b/ArmPlatformPkg/PrePi/PeiMPCore.inf
index 9c5da0d42a7b5d142af126b81c644d4d0a4c4d92..053f9fd9e616c99cc38dce21a5ec4cd25fdd708b 100644 (file)
--- a/ArmPlatformPkg/PrePi/PeiMPCore.inf
+++ b/ArmPlatformPkg/PrePi/PeiMPCore.inf
@@ -37,6 +37,7 @@
 \r
 [LibraryClasses]\r
   BaseLib\r
 \r
 [LibraryClasses]\r
   BaseLib\r

+  CacheMaintenanceLib\r

   DebugLib\r
   DebugAgentLib\r
   ArmLib\r
   DebugLib\r
   DebugAgentLib\r
   ArmLib\r

diff --git a/ArmPlatformPkg/PrePi/PeiUniCore.inf b/ArmPlatformPkg/PrePi/PeiUniCore.inf
index ee9b05b253372ed9ba3696b4fbea84071116b097..78d218ae09ca86aa2d77759174118146b97b4576 100644 (file)
--- a/ArmPlatformPkg/PrePi/PeiUniCore.inf
+++ b/ArmPlatformPkg/PrePi/PeiUniCore.inf
@@ -37,6 +37,7 @@
 \r
 [LibraryClasses]\r
   BaseLib\r
 \r
 [LibraryClasses]\r
   BaseLib\r

+  CacheMaintenanceLib\r

   DebugLib\r
   DebugAgentLib\r
   ArmLib\r
   DebugLib\r
   DebugAgentLib\r
   ArmLib\r

diff --git a/ArmPlatformPkg/PrePi/PrePi.c b/ArmPlatformPkg/PrePi/PrePi.c
index 74284f18830c559bb19b248935e32f42ccab58e8..5129dd09a85ef71d2e30ecff605cf49c4e78119b 100644 (file)
--- a/ArmPlatformPkg/PrePi/PrePi.c
+++ b/ArmPlatformPkg/PrePi/PrePi.c
@@ -8,6 +8,7 @@
 \r
 #include <PiPei.h>\r
 \r
 \r
 #include <PiPei.h>\r
 \r

+#include <Library/CacheMaintenanceLib.h>\r

 #include <Library/DebugAgentLib.h>\r
 #include <Library/PrePiLib.h>\r
 #include <Library/PrintLib.h>\r
 #include <Library/DebugAgentLib.h>\r
 #include <Library/PrePiLib.h>\r
 #include <Library/PrintLib.h>\r


@@ -178,8 +179,6 @@ CEntryPoint (
 \r
   // Data Cache enabled on Primary core when MMU is enabled.\r
   ArmDisableDataCache ();\r
 \r
   // Data Cache enabled on Primary core when MMU is enabled.\r
   ArmDisableDataCache ();\r

-  // Invalidate Data cache\r
-  ArmInvalidateDataCache ();\r

   // Invalidate instruction cache\r
   ArmInvalidateInstructionCache ();\r
   // Enable Instruction Caches on all cores.\r
   // Invalidate instruction cache\r
   ArmInvalidateInstructionCache ();\r
   // Enable Instruction Caches on all cores.\r


@@ -200,6 +199,10 @@ CEntryPoint (
 \r
   // If not primary Jump to Secondary Main\r
   if (ArmPlatformIsPrimaryCore (MpId)) {\r
 \r
   // If not primary Jump to Secondary Main\r
   if (ArmPlatformIsPrimaryCore (MpId)) {\r

+\r
+    InvalidateDataCacheRange ((VOID *)UefiMemoryBase,\r
+                              FixedPcdGet32 (PcdSystemMemoryUefiRegionSize));\r
+\r

     // Goto primary Main.\r
     PrimaryMain (UefiMemoryBase, StacksBase, StartTimeStamp);\r
   } else {\r
     // Goto primary Main.\r
     PrimaryMain (UefiMemoryBase, StacksBase, StartTimeStamp);\r
   } else {\r


@@ -209,4 +212,3 @@ CEntryPoint (
   // DXE Core should always load and never return\r
   ASSERT (FALSE);\r
 }\r
   // DXE Core should always load and never return\r
   ASSERT (FALSE);\r
 }\r

-\r