Ersek,

I have one comment for PCD PcdCpuSyncMtrrToAcpiNvs.

I knew OvmfPkg implemented LockBox based on ACPI NVS. Saving MTRR setting in 
AcpiNVS is OK for OvmfPkg.
But other platform may want to use more safe solution to save MTRR based on in 
SMM. 

I think that, for long term, saving MTRR setting by LockBox instead of using 
ACPI NVS memory directly.  Then, different platforms could provide the 
different LockBox solutions.
For short term, still saving MTRR setting in ACPI NVS in CpuDxe, and removing 
this PCD. That means we could CpuDxe implementation to use the long term 
solution in the future and needn't to remove one un-used PCD more.

Thanks!
Jeff

-----Original Message-----
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Saturday, July 25, 2015 7:01 AM
To: edk2-de...@ml01.01.org
Cc: Fan, Jeff; Chen Fan; Justen, Jordan L
Subject: [PATCH 38/58] UefiCpuPkg: CpuDxe: optionally save MTRR settings to 
AcpiNVS memory block

The

  Quark_EDKII_v1.1.0/IA32FamilyCpuBasePkg/CpuArchDxe

driver provides the following capability in its implementation of
EFI_CPU_ARCH_PROTOCOL: whenever the SetMemoryAttributes() member is used 
(directly, or indirectly via gDS->SetMemorySpaceAttributes()) to change MTRR 
settings, the complete set of settings is immediately reflected to an AcpiNVS 
memory block.

(The address of said block is published via a dynamic PCD.)

This feature is necessary for supporting SMM-capable S3 resume, hence port it 
to UefiCpuPkg. In IA32FamilyCpuBasePkg the feature is always built in; here we 
make it controllable with a feature PCD.

Cc: Jeff Fan <jeff....@intel.com>
Cc: Chen Fan <chen.fan.f...@cn.fujitsu.com>
Cc: Jordan Justen <jordan.l.jus...@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <ler...@redhat.com>
---
 UefiCpuPkg/CpuDxe/CpuDxe.inf |   6 +
 UefiCpuPkg/CpuDxe/MtrrSync.h |  86 ++++++++++++++
 UefiCpuPkg/CpuDxe/CpuDxe.c   |  17 +++
 UefiCpuPkg/CpuDxe/MtrrSync.c | 118 ++++++++++++++++++++
 UefiCpuPkg/UefiCpuPkg.dec    |  11 ++
 5 files changed, 238 insertions(+)

diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.inf b/UefiCpuPkg/CpuDxe/CpuDxe.inf index 
a251922..2450a20 100644
--- a/UefiCpuPkg/CpuDxe/CpuDxe.inf
+++ b/UefiCpuPkg/CpuDxe/CpuDxe.inf
@@ -54,6 +54,8 @@ [Sources]
   CpuGdt.h
   CpuMp.c
   CpuMp.h
+  MtrrSync.c
+  MtrrSync.h
 
 [Sources.IA32]
   Ia32/CpuAsm.asm | MSFT
@@ -86,6 +88,10 @@ [Ppis]
 [Pcd]
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber   ## CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize                 ## CONSUMES
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuMtrrTableAddress            ## 
SOMETIMES_PRODUCES
+
+[FeaturePcd]
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuSyncMtrrToAcpiNvs
 
 [Depex]
   TRUE
diff --git a/UefiCpuPkg/CpuDxe/MtrrSync.h b/UefiCpuPkg/CpuDxe/MtrrSync.h new 
file mode 100644 index 0000000..1c20a32
--- /dev/null
+++ b/UefiCpuPkg/CpuDxe/MtrrSync.h
@@ -0,0 +1,86 @@
+/** @file
+
+  Include file for MTRR synchronzation.
+
+  Copyright (C) 2015, Red Hat, Inc.
+  Copyright (c) 2013-2015 Intel Corporation.
+
+  Redistribution and use in source and binary forms, with or without  
+ modification, are permitted provided that the following conditions  
+ are met:
+
+  * Redistributions of source code must retain the above copyright  
+ notice, this list of conditions and the following disclaimer.
+  * Redistributions in binary form must reproduce the above copyright  
+ notice, this list of conditions and the following disclaimer in  the 
+ documentation and/or other materials provided with the  distribution.
+  * Neither the name of Intel Corporation nor the names of its  
+ contributors may be used to endorse or promote products derived  from 
+ this software without specific prior written permission.
+
+  THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS  
+ "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT  
+ LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR  
+ A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT  
+ OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,  
+ SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT  
+ LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,  
+ DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY  
+ THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT  
+ (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE  
+ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+  Module Name:  MtrrSync.h
+
+**/
+
+#ifndef _EFI_MTRR_SYNC_H_
+#define _EFI_MTRR_SYNC_H_
+
+#include <Library/MtrrLib.h>
+
+extern MTRR_SETTINGS   *mMtrrTable;
+
+/**
+  Initialize memory region for MTRR data.
+  
+  This function allocates ACPI NVS memory for MTRR data, and fills the 
+ region  with current MTRR data. Each time MTRRs are written, this 
+ memory region  will be updated accordingly.
+
+**/
+VOID
+InitializeMtrrData (
+  VOID
+  );
+
+/**
+  Synchronzies up the MTRR values with BSP for calling processor.
+
+  This function synchronzies up the MTRR values with BSP for calling processor.
+
+  @param  Buffer         Mtrr table address.
+
+**/
+VOID
+EFIAPI
+LoadMtrrData (
+  VOID    *Buffer
+  );
+
+/**
+  Allocate EfiACPIMemoryNVS below 4G memory address.
+
+  This function allocates EfiACPIMemoryNVS below 4G memory address.
+
+  @param  Size         Size of memory to allocate.
+  
+  @return Allocated address for output.
+
+**/
+VOID*
+AllocateAcpiNvsMemoryBelow4G (
+  IN   UINTN   Size
+  );
+#endif
diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.c b/UefiCpuPkg/CpuDxe/CpuDxe.c index 
c9df4e1..3d11807 100644
--- a/UefiCpuPkg/CpuDxe/CpuDxe.c
+++ b/UefiCpuPkg/CpuDxe/CpuDxe.c
@@ -14,6 +14,7 @@
 
 #include "CpuDxe.h"
 #include "CpuMp.h"
+#include "MtrrSync.h"
 
 //
 // Global Variables
@@ -405,6 +406,14 @@ CpuSetMemoryAttributes (
              CacheType
              );
 
+  if (!RETURN_ERROR (Status)) {
+    if (FeaturePcdGet (PcdCpuSyncMtrrToAcpiNvs)) {
+      //
+      // Sync saved MTRR settings
+      //
+      MtrrGetAllMtrrs (mMtrrTable);
+    }
+  }
   return (EFI_STATUS) Status;
 }
 
@@ -870,6 +879,14 @@ InitializeCpu (
   //
   ProgramVirtualWireMode ();
 
+  if (FeaturePcdGet (PcdCpuSyncMtrrToAcpiNvs)) {
+    //
+    // Allocates ACPI NVS memory for MTRR data.
+    //
+    InitializeMtrrData ();
+    MtrrGetAllMtrrs (mMtrrTable);
+  }
+
   //
   // Install CPU Architectural Protocol
   //
diff --git a/UefiCpuPkg/CpuDxe/MtrrSync.c b/UefiCpuPkg/CpuDxe/MtrrSync.c new 
file mode 100644 index 0000000..8538014
--- /dev/null
+++ b/UefiCpuPkg/CpuDxe/MtrrSync.c
@@ -0,0 +1,118 @@
+/** @file
+
+  Code for MTRR synchronzation.
+
+  Copyright (C) 2015, Red Hat, Inc.
+  Copyright (c) 2013-2015 Intel Corporation.
+
+  Redistribution and use in source and binary forms, with or without  
+ modification, are permitted provided that the following conditions  
+ are met:
+
+  * Redistributions of source code must retain the above copyright  
+ notice, this list of conditions and the following disclaimer.
+  * Redistributions in binary form must reproduce the above copyright  
+ notice, this list of conditions and the following disclaimer in  the 
+ documentation and/or other materials provided with the  distribution.
+  * Neither the name of Intel Corporation nor the names of its  
+ contributors may be used to endorse or promote products derived  from 
+ this software without specific prior written permission.
+
+  THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS  
+ "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT  
+ LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR  
+ A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT  
+ OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,  
+ SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT  
+ LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,  
+ DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY  
+ THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT  
+ (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE  
+ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+  Module Name:  MtrrSync.c
+
+**/
+
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/DebugLib.h>
+#include <Library/BaseMemoryLib.h>
+
+#include "MtrrSync.h"
+
+MTRR_SETTINGS   *mMtrrTable;
+
+/**
+  Initialize memory region for MTRR data.
+  
+  This function allocates ACPI NVS memory for MTRR data.
+  Each time MTRRs are written, this memory region will be updated accordingly.
+
+**/
+VOID
+InitializeMtrrData (
+  VOID
+  )
+{
+  //
+  // Allocate memory for fixed MTRRs, variable MTRRs and MTRR_DEF_TYPE
+  //
+  mMtrrTable = AllocateAcpiNvsMemoryBelow4G (sizeof (MTRR_SETTINGS));
+
+  PcdSet64 (PcdCpuMtrrTableAddress, (UINT64) (UINTN) mMtrrTable); }
+
+/**
+  Synchronzies up the MTRR values with BSP for calling processor.
+
+  This function synchronzies up the MTRR values with BSP for calling processor.
+
+  @param  Buffer         Mtrr table address.
+
+**/
+VOID
+EFIAPI
+LoadMtrrData (
+  VOID    *Buffer
+  )
+{
+  MtrrSetAllMtrrs (Buffer);
+}
+
+/**
+  Allocate EfiACPIMemoryNVS below 4G memory address.
+
+  This function allocates EfiACPIMemoryNVS below 4G memory address.
+
+  @param  Size         Size of memory to allocate.
+  
+  @return Allocated address for output.
+
+**/
+VOID*
+AllocateAcpiNvsMemoryBelow4G (
+  IN   UINTN   Size
+  )
+{
+  UINTN                 Pages;
+  EFI_PHYSICAL_ADDRESS  Address;
+  EFI_STATUS            Status;
+  VOID*                 Buffer;
+
+  Pages = EFI_SIZE_TO_PAGES (Size);
+  Address = 0xffffffff;
+
+  Status  = gBS->AllocatePages (
+                   AllocateMaxAddress,
+                   EfiACPIMemoryNVS,
+                   Pages,
+                   &Address
+                   );
+  ASSERT_EFI_ERROR (Status);
+
+  Buffer = (VOID *) (UINTN) Address;
+  ZeroMem (Buffer, Size);
+
+  return Buffer;
+}
diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec index 
202e719..9c5da3c 100644
--- a/UefiCpuPkg/UefiCpuPkg.dec
+++ b/UefiCpuPkg/UefiCpuPkg.dec
@@ -73,5 +73,16 @@ [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, 
PcdsDynamicEx]
   # @Prompt Microcode Region size.
   
gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize|0x0|UINT64|0x00000006
 
+[PcdsFeatureFlag]
+  ## This flag controls whether the EFI_CPU_ARCH_PROTOCOL 
+implementation
+  #  reflects the updated MTRR settings to a persistent (AcpiNVS) memory block.
+  # @Prompt Keep MTRR settings reflected to an AcpiNVS memory block
+  
+gUefiCpuPkgTokenSpaceGuid.PcdCpuSyncMtrrToAcpiNvs|FALSE|BOOLEAN|0x00000
+007
+
+[PcdsDynamic, PcdsDynamicEx]
+  ## If PcdCpuSyncMtrrToAcpiNvs is TRUE, the PCD below will be set to 
+the
+  #  address of the AcpiNVS memory block in question.
+  
+gUefiCpuPkgTokenSpaceGuid.PcdCpuMtrrTableAddress|0x0|UINT64|0x00000008
+
 [UserExtensions.TianoCore."ExtraFiles"]
   UefiCpuPkgExtra.uni
--
1.8.3.1


_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to