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