At the moment we work with a UC default MTRR type, and set three memory
ranges to WB:
- [0, 640 KB),
- [1 MB, LowerMemorySize),
- [4 GB, 4 GB + UpperMemorySize).

Unfortunately, coverage for the third range can fail with a high
likelihood. If the alignment of the base (ie. 4 GB) and the alignment of
the size (UpperMemorySize) differ, then MtrrLib creates a series of
variable MTRR entries, with power-of-two sized MTRR masks. And, it's
really easy to run out of variable MTRR entries, dependent on the
alignment difference.

This is a problem because a Linux guest will loudly reject any high memory
that is not covered my MTRR.

So, let's follow the inverse pattern (loosely inspired by SeaBIOS):
- flip the MTRR default type to WB,
- set [0, 640 KB) to WB -- fixed MTRRs have precedence over the default
  type and variable MTRRs, so we can't avoid this,
- set [640 KB, 1 MB) to UC -- implemented with fixed MTRRs,
- set [LowerMemorySize, 4 GB) to UC -- should succeed with variable MTRRs
  more likely than the other scheme (due to less chaotic alignment
  differences).

Effects of this patch can be observed by setting DEBUG_CACHE (0x00200000)
in PcdDebugPrintErrorLevel.

Cc: Maoming <maoming.maom...@huawei.com>
Cc: Huangpeng (Peter) <peter.huangp...@huawei.com>
Cc: Wei Liu <wei.l...@citrix.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <ler...@redhat.com>
Tested-by: Maoming <maoming.maom...@huawei.com>
---
 OvmfPkg/PlatformPei/MemDetect.c | 43 +++++++++++++++++++++++++++++++++++++----
 1 file changed, 39 insertions(+), 4 deletions(-)

diff --git a/OvmfPkg/PlatformPei/MemDetect.c b/OvmfPkg/PlatformPei/MemDetect.c
index 1228063..e872126 100644
--- a/OvmfPkg/PlatformPei/MemDetect.c
+++ b/OvmfPkg/PlatformPei/MemDetect.c
@@ -194,6 +194,8 @@ QemuInitializeRam (
 {
   UINT64                      LowerMemorySize;
   UINT64                      UpperMemorySize;
+  MTRR_SETTINGS               MtrrSettings;
+  EFI_STATUS                  Status;
 
   DEBUG ((EFI_D_INFO, "%a called\n", __FUNCTION__));
 
@@ -214,12 +216,45 @@ QemuInitializeRam (
     }
   }
 
-  MtrrSetMemoryAttribute (BASE_1MB, LowerMemorySize - BASE_1MB, 
CacheWriteBack);
+  //
+  // We'd like to keep the following ranges uncached:
+  // - [640 KB, 1 MB)
+  // - [LowerMemorySize, 4 GB)
+  //
+  // Everything else should be WB. Unfortunately, programming the inverse (ie.
+  // keeping the default UC, and configuring the complement set of the above as
+  // WB) is not reliable in general, because the end of the upper RAM can have
+  // practically any alignment, and we may not have enough variable MTRRs to
+  // cover it exactly.
+  //
+  if (IsMtrrSupported ()) {
+    MtrrGetAllMtrrs (&MtrrSettings);
 
-  MtrrSetMemoryAttribute (0, BASE_512KB + BASE_128KB, CacheWriteBack);
+    //
+    // MTRRs disabled, fixed MTRRs disabled, default type is uncached
+    //
+    ASSERT ((MtrrSettings.MtrrDefType & BIT11) == 0);
+    ASSERT ((MtrrSettings.MtrrDefType & BIT10) == 0);
+    ASSERT ((MtrrSettings.MtrrDefType & 0xFF) == 0);
 
-  if (UpperMemorySize != 0) {
-    MtrrSetMemoryAttribute (BASE_4GB, UpperMemorySize, CacheWriteBack);
+    //
+    // flip default type to writeback
+    //
+    SetMem (&MtrrSettings.Fixed, sizeof MtrrSettings.Fixed, 0x06);
+    ZeroMem (&MtrrSettings.Variables, sizeof MtrrSettings.Variables);
+    MtrrSettings.MtrrDefType |= BIT11 | BIT10 | 6;
+    MtrrSetAllMtrrs (&MtrrSettings);
+
+    //
+    // punch holes
+    //
+    Status = MtrrSetMemoryAttribute (BASE_512KB + BASE_128KB,
+               SIZE_256KB + SIZE_128KB, CacheUncacheable);
+    ASSERT_EFI_ERROR (Status);
+
+    Status = MtrrSetMemoryAttribute (LowerMemorySize,
+               SIZE_4GB - LowerMemorySize, CacheUncacheable);
+    ASSERT_EFI_ERROR (Status);
   }
 }
 
-- 
1.8.3.1


------------------------------------------------------------------------------
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel

Reply via email to