[PATCH] UefiCpuPkg/MtrrLib: Revert "Skip MSR access when the pair is invalid"

* [PATCH] UefiCpuPkg/MtrrLib: Revert "Skip MSR access when the pair is invalid"
@ 2018-09-25  5:22 Ruiyu Ni
  2018-09-25  8:14 ` Dong, Eric
  0 siblings, 1 reply; 2+ messages in thread
From: Ruiyu Ni @ 2018-09-25  5:22 UTC (permalink / raw)
  To: edk2-devel; +Cc: Michael D Kinney, Eric Dong, Sean Brogan

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1187

The patch reverts 9c8c4478cfcacaf5fd60b75ff78d26732d93a5b8
"UefiCpuPkg/MtrrLib: Skip Base MSR access when the pair is invalid".

Microsoft Windows will report an error in event manager if MTRR
usage is different across hibernate even when the difference is
in an non valid MTRR pair. This seems like a bug in Windows but
for compatibility and servicing reasons we think a change in UEFI
would wise.
A Windows change has already been submitted for the next iteration
(2019 time frame).

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
---
 UefiCpuPkg/Library/MtrrLib/MtrrLib.c | 29 +++++++++++++----------------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
index dfce9a996b..086f7ad8f0 100644
--- a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
+++ b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
@@ -449,13 +449,10 @@ MtrrGetVariableMtrrWorker (
 
   for (Index = 0; Index < VariableMtrrCount; Index++) {
     if (MtrrSetting == NULL) {
-      VariableSettings->Mtrr[Index].Mask = AsmReadMsr64 (MSR_IA32_MTRR_PHYSMASK0 + (Index << 1));
-      //
-      // Skip to read the Base MSR when the Mask.V is not set.
-      //
-      if (((MSR_IA32_MTRR_PHYSMASK_REGISTER *)&VariableSettings->Mtrr[Index].Mask)->Bits.V != 0) {
-        VariableSettings->Mtrr[Index].Base = AsmReadMsr64 (MSR_IA32_MTRR_PHYSBASE0 + (Index << 1));
-      }
+      VariableSettings->Mtrr[Index].Base =
+        AsmReadMsr64 (MSR_IA32_MTRR_PHYSBASE0 + (Index << 1));
+      VariableSettings->Mtrr[Index].Mask =
+        AsmReadMsr64 (MSR_IA32_MTRR_PHYSMASK0 + (Index << 1));
     } else {
       VariableSettings->Mtrr[Index].Base = MtrrSetting->Variables.Mtrr[Index].Base;
       VariableSettings->Mtrr[Index].Mask = MtrrSetting->Variables.Mtrr[Index].Mask;
@@ -2604,14 +2601,14 @@ MtrrSetVariableMtrrWorker (
   ASSERT (VariableMtrrCount <= ARRAY_SIZE (VariableSettings->Mtrr));
 
   for (Index = 0; Index < VariableMtrrCount; Index++) {
-    //
-    // Mask MSR is always updated since caller might need to invalidate the MSR pair.
-    // Base MSR is skipped when Mask.V is not set.
-    //
-    AsmWriteMsr64 (MSR_IA32_MTRR_PHYSMASK0 + (Index << 1), VariableSettings->Mtrr[Index].Mask);
-    if (((MSR_IA32_MTRR_PHYSMASK_REGISTER *)&VariableSettings->Mtrr[Index].Mask)->Bits.V != 0) {
-      AsmWriteMsr64 (MSR_IA32_MTRR_PHYSBASE0 + (Index << 1), VariableSettings->Mtrr[Index].Base);
-    }
+    AsmWriteMsr64 (
+      MSR_IA32_MTRR_PHYSBASE0 + (Index << 1),
+      VariableSettings->Mtrr[Index].Base
+      );
+    AsmWriteMsr64 (
+      MSR_IA32_MTRR_PHYSMASK0 + (Index << 1),
+      VariableSettings->Mtrr[Index].Mask
+      );
   }
 }
 
@@ -2868,7 +2865,7 @@ MtrrDebugPrintAllMtrrsWorker (
     }
     ContainVariableMtrr = FALSE;
     for (Index = 0; Index < VariableMtrrCount; Index++) {
-      if (((MSR_IA32_MTRR_PHYSMASK_REGISTER *)&Mtrrs->Variables.Mtrr[Index].Mask)->Bits.V == 0) {
+      if ((Mtrrs->Variables.Mtrr[Index].Mask & BIT11) == 0) {
         //
         // If mask is not valid, then do not display range
         //
-- 
2.16.1.windows.1



^ permalink raw reply related	[flat|nested] 2+ messages in thread