IntelFrameworkModulePkg AcpiS3SaveDxe: Remove ASSERT, add lock and remove RT for AcpiGlobalVariable variable.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>

git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@15337 6f19259b-4bc3-4df7-8a09-765794883524

2 files changed, 63 insertions, 6 deletions

diff --git a/IntelFrameworkModulePkg/Universal/Acpi/AcpiS3SaveDxe/AcpiS3SaveDxe.inf b/IntelFrameworkModulePkg/Universal/Acpi/AcpiS3SaveDxe/AcpiS3SaveDxe.inf
index eb3ade991e..dc6da58f9f 100644
--- a/IntelFrameworkModulePkg/Universal/Acpi/AcpiS3SaveDxe/AcpiS3SaveDxe.inf
+++ b/IntelFrameworkModulePkg/Universal/Acpi/AcpiS3SaveDxe/AcpiS3SaveDxe.inf
@@ -2,7 +2,7 @@
 # Component description file for AcpiS3Save module.

 #

 # This is an implementation of the ACPI S3 Save protocol.

-# Copyright (c) 2006 - 2011, Intel Corporation. All rights reserved.<BR>

+# Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.<BR>

 #

 # This program and the accompanying materials are

 # licensed and made available under the terms and conditions of the BSD License

@@ -64,6 +64,7 @@
   gEfiLegacyBiosProtocolGuid                    # PROTOCOL ALWAYS_CONSUMED

   gEfiLegacyRegion2ProtocolGuid                 # PROTOCOL SOMETIMES_CONSUMED

   gFrameworkEfiMpServiceProtocolGuid            # PROTOCOL SOMETIMES_CONSUMED

+  gEdkiiVariableLockProtocolGuid                ## SOMETIMES_CONSUMES

 

 [FeaturePcd]

   gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdPlatformCsmSupport          ## CONSUMES

diff --git a/IntelFrameworkModulePkg/Universal/Acpi/AcpiS3SaveDxe/AcpiVariableThunkPlatform.c b/IntelFrameworkModulePkg/Universal/Acpi/AcpiS3SaveDxe/AcpiVariableThunkPlatform.c
index 641a0ae0c9..7ad9eb4336 100644
--- a/IntelFrameworkModulePkg/Universal/Acpi/AcpiS3SaveDxe/AcpiVariableThunkPlatform.c
+++ b/IntelFrameworkModulePkg/Universal/Acpi/AcpiS3SaveDxe/AcpiVariableThunkPlatform.c
@@ -1,7 +1,7 @@
 /** @file

   This is an implementation of the AcpiVariable platform field for ECP platform.

 

-Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.<BR>

+Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.<BR>

 

 This program and the accompanying materials

 are licensed and made available under the terms and conditions

@@ -39,7 +39,9 @@ typedef struct {
 #include <Library/HobLib.h>

 #include <Library/PcdLib.h>

 #include <Library/DebugLib.h>

+#include <Library/UefiLib.h>

 #include <Protocol/FrameworkMpService.h>

+#include <Protocol/VariableLock.h>

 #include <Guid/AcpiVariableCompatibility.h>

 #include <Guid/AcpiS3Context.h>

 

@@ -80,6 +82,10 @@ S3ReadyThunkPlatform (
 

   DEBUG ((EFI_D_INFO, "S3ReadyThunkPlatform\n"));

 

+  if (mAcpiVariableSetCompatibility == NULL) {

+    return;

+  }

+

   //

   // Allocate ACPI reserved memory under 4G

   //

@@ -118,6 +124,32 @@ S3ReadyThunkPlatform (
 }

 

 /**

+  Register callback function upon VariableLockProtocol

+  to lock ACPI_GLOBAL_VARIABLE variable to avoid malicious code to update it.

+

+  @param[in] Event    Event whose notification function is being invoked.

+  @param[in] Context  Pointer to the notification function's context.

+**/

+VOID

+EFIAPI

+VariableLockAcpiGlobalVariable (

+  IN  EFI_EVENT                             Event,

+  IN  VOID                                  *Context

+  )

+{

+  EFI_STATUS                    Status;

+  EDKII_VARIABLE_LOCK_PROTOCOL  *VariableLock;

+  //

+  // Mark ACPI_GLOBAL_VARIABLE variable to read-only if the Variable Lock protocol exists

+  //

+  Status = gBS->LocateProtocol (&gEdkiiVariableLockProtocolGuid, NULL, (VOID **) &VariableLock);

+  if (!EFI_ERROR (Status)) {

+    Status = VariableLock->RequestToLock (VariableLock, ACPI_GLOBAL_VARIABLE, &gEfiAcpiVariableCompatiblityGuid);

+    ASSERT_EFI_ERROR (Status);

+  }

+}

+

+/**

   Hook point for AcpiVariableThunkPlatform for InstallAcpiS3Save.

 **/

 VOID

@@ -128,6 +160,7 @@ InstallAcpiS3SaveThunk (
   EFI_STATUS                           Status;

   FRAMEWORK_EFI_MP_SERVICES_PROTOCOL   *FrameworkMpService;

   UINTN                                VarSize;

+  VOID                                 *Registration;

 

   Status = gBS->LocateProtocol (

                   &gFrameworkEfiMpServiceProtocolGuid,

@@ -147,21 +180,44 @@ InstallAcpiS3SaveThunk (
                     &VarSize,

                     &mAcpiVariableSetCompatibility

                     );

-    ASSERT_EFI_ERROR (Status);

+    if (EFI_ERROR (Status) || (VarSize != sizeof (mAcpiVariableSetCompatibility))) {

+      DEBUG ((EFI_D_ERROR, "FATAL ERROR: AcpiVariableSetCompatibility was not saved by CPU driver correctly. OS S3 may fail!\n"));

+      mAcpiVariableSetCompatibility = NULL;

+    }

   } else {

     //

     // Allocate/initialize the compatible version of Acpi Variable Set since Framework chipset/platform 

-    // driver need this variable

+    // driver need this variable. ACPI_GLOBAL_VARIABLE variable is not used in runtime phase,

+    // so RT attribute is not needed for it.

     //

     mAcpiVariableSetCompatibility = AllocateMemoryBelow4G (EfiACPIMemoryNVS, sizeof(ACPI_VARIABLE_SET_COMPATIBILITY));

     Status = gRT->SetVariable (

                     ACPI_GLOBAL_VARIABLE,

                     &gEfiAcpiVariableCompatiblityGuid,

-                    EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,

+                    EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_NON_VOLATILE,

                     sizeof(mAcpiVariableSetCompatibility),

                     &mAcpiVariableSetCompatibility

                     );

-    ASSERT_EFI_ERROR (Status);

+    if (!EFI_ERROR (Status)) {

+      //

+      // Register callback function upon VariableLockProtocol

+      // to lock ACPI_GLOBAL_VARIABLE variable to avoid malicious code to update it.

+      //

+      EfiCreateProtocolNotifyEvent (

+        &gEdkiiVariableLockProtocolGuid,

+        TPL_CALLBACK,

+        VariableLockAcpiGlobalVariable,

+        NULL,

+        &Registration

+        );

+    } else {

+      DEBUG ((EFI_D_ERROR, "FATAL ERROR: AcpiVariableSetCompatibility cannot be saved: %r. OS S3 may fail!\n", Status));

+      gBS->FreePages (

+             (EFI_PHYSICAL_ADDRESS) (UINTN) mAcpiVariableSetCompatibility,

+             EFI_SIZE_TO_PAGES (sizeof (ACPI_VARIABLE_SET_COMPATIBILITY))

+             );

+      mAcpiVariableSetCompatibility = NULL;

+    }

   }

 

   DEBUG((EFI_D_INFO, "AcpiVariableSetCompatibility is 0x%8x\n", mAcpiVariableSetCompatibility));