From: "Zeng, Star" <star.zeng@intel.com>
To: Sami Mujawar <sami.mujawar@arm.com>,
"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "ard.biesheuvel@linaro.org" <ard.biesheuvel@linaro.org>,
"leif.lindholm@linaro.org" <leif.lindholm@linaro.org>,
"Wang, Jian J" <jian.j.wang@intel.com>,
"Ni, Ruiyu" <ruiyu.ni@intel.com>,
"evan.lloyd@arm.com" <evan.lloyd@arm.com>,
"Matteo.Carlini@arm.com" <Matteo.Carlini@arm.com>,
"Stephanie.Hughes-Fitt@arm.com" <Stephanie.Hughes-Fitt@arm.com>,
"nd@arm.com" <nd@arm.com>, "Zeng, Star" <star.zeng@intel.com>
Subject: Re: [PATCH v1 3/6] MdeModulePkg: Map persistent (NV) memory
Date: Sat, 13 Oct 2018 09:09:51 +0000 [thread overview]
Message-ID: <0C09AFA07DD0434D9E2A0C6AEB0483103BC1E173@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <20181012144009.48732-4-sami.mujawar@arm.com>
Hi Sami,
Thanks for the patch.
I just checked this patch and roughly checked [PATCH v1 5/6] ArmVirtPkg: Add kvmtool platform driver.
I would like to suggest that the code added in this patch to be moved into KvmtoolPlatformDxe which is added by [PATCH v1 5/6] as KvmtoolPlatformDxe is the producer of KvmtoolPlatformDxe and has full knowledge to that memory usage.
Thanks,
Star
-----Original Message-----
From: Sami Mujawar [mailto:sami.mujawar@arm.com]
Sent: Friday, October 12, 2018 10:40 PM
To: edk2-devel@lists.01.org
Cc: ard.biesheuvel@linaro.org; leif.lindholm@linaro.org; Zeng, Star <star.zeng@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; evan.lloyd@arm.com; Matteo.Carlini@arm.com; Stephanie.Hughes-Fitt@arm.com; nd@arm.com
Subject: [PATCH v1 3/6] MdeModulePkg: Map persistent (NV) memory
Some platforms are able to preserve a memory range across system resets. This memory can be used for the Non-Volatile variables storage. The PcdEmuVariableNvStoreReserved is used to specify this option.
This patch enables mapping of this Non-Volatile memory range as runtime memory so that the variable storage is accessible post ExitBootServices.
Also added PcdMapEmuVariableNvStoreReserved to select if the memory region described by PcdMapEmuVariableNvStoreReserved should be mapped by the Variable Emulation driver.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
The changes can be seen at https://github.com/samimujawar/edk2/commit/a4fbf2336578546031920337239f41544a3a130e
Notes:
v1:
- Add support for mapping memory used for Non-Volatile variable [SAMI]
storage as runtime memory.
MdeModulePkg/MdeModulePkg.dec | 9 +++
MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariable.c | 77 +++++++++++++++++++-
MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDxe.inf | 6 ++
3 files changed, 90 insertions(+), 2 deletions(-)
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec index 6037504fa789d4ca96d45bf3a776dd77bc17a909..077d3682371e1d44b0c86f922f80536080403e52 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -8,6 +8,7 @@
# (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR> # Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> # Copyright (c) 2016, Microsoft Corporation<BR>
+# Copyright (c) 2018, ARM Limited. All rights reserved.<BR>
# This program and the accompanying materials are licensed and made available under # the terms and conditions of the BSD License that accompanies this distribution.
# The full text of the license may be found at @@ -1592,6 +1593,14 @@ [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
# @Prompt Reserved memory range for EMU variable NV storage.
gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved|0|UINT64|0x40000008
+ ## Indicates if the reserved memory range for the EMU Variable
+ driver's # NV Variable Store should be mapped by the driver. The
+ memory range size # must be PcdVariableStoreSize.
+ # TRUE - Map the memory as persistent runtime memory.<BR>
+ # FALSE - Do not map the memory.<BR>
+ # @Prompt Map persistent memory range for EMU variable NV storage.
+
+ gEfiMdeModulePkgTokenSpaceGuid.PcdMapEmuVariableNvStoreReserved|FALSE|
+ BOOLEAN|0x4000000f
+
## This PCD defines the times to print hello world string.
# This PCD is a sample to explain UINT32 PCD usage.
# @Prompt HellowWorld print times.
diff --git a/MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariable.c b/MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariable.c
index 1bcf931b96a670ab1fc66cd66f9d88d68f363e7f..e103a340aea9394d9ff146efa5581d3180c45035 100644
--- a/MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariable.c
+++ b/MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariable.c
@@ -4,6 +4,7 @@
The nonvolatile variable space doesn't exist.
Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2018, ARM Limited. All rights reserved.<BR>
This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License which accompanies this distribution. The full text of the license may be found at @@ -14,6 +15,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
**/
+#include <Library/DxeServicesTableLib.h>
+
#include "Variable.h"
///
@@ -1643,11 +1646,13 @@ EmuQueryVariableInfo (
This function allocates memory space for variable store area and initializes its attributes.
+ @param ImageHandle The Image handle of this driver.
@param VolatileStore Indicates if the variable store is volatile.
**/
EFI_STATUS
InitializeVariableStore (
+ IN EFI_HANDLE ImageHandle,
IN BOOLEAN VolatileStore
)
{
@@ -1693,6 +1698,74 @@ InitializeVariableStore (
VariableStore =
(VARIABLE_STORE_HEADER *)(VOID*)(UINTN)
PcdGet64 (PcdEmuVariableNvStoreReserved);
+
+ if (PcdGetBool (PcdMapEmuVariableNvStoreReserved)) {
+ DEBUG((
+ DEBUG_INFO,
+ "Initializing NV Variable Store at %p, Size = 0x%x\n",
+ VariableStore, PcdGet32 (PcdVariableStoreSize)
+ ));
+
+ // Declare the NV Storage as persistent EFI_MEMORY_RUNTIME memory
+ Status = gDS->AddMemorySpace (
+ EfiGcdMemoryTypePersistent,
+ (EFI_PHYSICAL_ADDRESS)VariableStore,
+ PcdGet32 (PcdVariableStoreSize),
+ EFI_MEMORY_UC | EFI_MEMORY_RUNTIME
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR, "Failed to add memory space. Status = %r\n",
+ Status
+ ));
+ return Status;
+ }
+
+ Status = gDS->AllocateMemorySpace (
+ EfiGcdAllocateAddress,
+ EfiGcdMemoryTypePersistent,
+ 0,
+ PcdGet32 (PcdVariableStoreSize),
+ (EFI_PHYSICAL_ADDRESS*)&VariableStore,
+ ImageHandle,
+ NULL
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "Failed to allocate memory space. Status = %r\n",
+ Status
+ ));
+ gDS->RemoveMemorySpace (
+ (EFI_PHYSICAL_ADDRESS)VariableStore,
+ PcdGet32 (PcdVariableStoreSize)
+ );
+ return Status;
+ }
+
+ Status = gDS->SetMemorySpaceAttributes (
+ (EFI_PHYSICAL_ADDRESS)VariableStore,
+ PcdGet32 (PcdVariableStoreSize),
+ EFI_MEMORY_UC | EFI_MEMORY_RUNTIME
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "Failed to set memory attributes. Status = %r\n",
+ Status
+ ));
+ gDS->FreeMemorySpace (
+ (EFI_PHYSICAL_ADDRESS)VariableStore,
+ PcdGet32 (PcdVariableStoreSize)
+ );
+ gDS->RemoveMemorySpace (
+ (EFI_PHYSICAL_ADDRESS)VariableStore,
+ PcdGet32 (PcdVariableStoreSize)
+ );
+ return Status;
+ }
+ }
+
if (
(VariableStore->Size == PcdGet32 (PcdVariableStoreSize)) &&
(VariableStore->Format == VARIABLE_STORE_FORMATTED) && @@ -1806,7 +1879,7 @@ VariableCommonInitialize (
//
// Intialize volatile variable store
//
- Status = InitializeVariableStore (TRUE);
+ Status = InitializeVariableStore (ImageHandle, TRUE);
if (EFI_ERROR (Status)) {
FreePool(mVariableModuleGlobal);
return Status;
@@ -1814,7 +1887,7 @@ VariableCommonInitialize (
//
// Intialize non volatile variable store
//
- Status = InitializeVariableStore (FALSE);
+ Status = InitializeVariableStore (ImageHandle, FALSE);
return Status;
}
diff --git a/MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDxe.inf b/MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDxe.inf
index 12d52dd130e317c7ed041cf144805b8547bfc62b..1b5902749e1806143e92ccc2009a7512d57b940b 100644
--- a/MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDxe.inf
+++ b/MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDx
+++ e.inf
@@ -5,6 +5,7 @@
# four EFI_RUNTIME_SERVICES: SetVariable, GetVariable, GetNextVariableName and QueryVariableInfo.
#
# Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2018, ARM Limited. All rights reserved.<BR>
#
# This program and the accompanying materials # are licensed and made available under the terms and conditions of the BSD License @@ -53,6 +54,7 @@ [LibraryClasses]
BaseMemoryLib
HobLib
PcdLib
+ DxeServicesTableLib
[Protocols]
gEfiVariableArchProtocolGuid ## PRODUCES
@@ -77,10 +79,14 @@ [Pcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxHardwareErrorVariableSize ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdHwErrStorageSize ## CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdMapEmuVariableNvStoreReserved ## CONSUMES
[FeaturePcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdVariableCollectStatistics ## CONSUMES # statistic the information of variable.
+[Depex.common.DXE_RUNTIME_DRIVER]
+ gEfiCpuArchProtocolGuid
+
[Depex]
TRUE
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
next prev parent reply other threads:[~2018-10-13 9:09 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-10-12 14:40 [PATCH 0/6] Add kvmtool emulated platform support for ARM Sami Mujawar
2018-10-12 14:40 ` [PATCH v1 1/6] PcAtChipsetPkg: Add MMIO Support to SerialIo Lib Sami Mujawar
2018-10-12 14:49 ` Ard Biesheuvel
2018-10-12 15:06 ` Sami Mujawar
2018-10-12 15:31 ` Kinney, Michael D
2018-10-12 15:33 ` Ard Biesheuvel
2018-10-15 2:38 ` Ni, Ruiyu
2018-10-12 14:40 ` [PATCH v1 2/6] PcAtChipsetPkg: Add MMIO Support to RTC driver Sami Mujawar
2018-10-13 10:51 ` Leif Lindholm
2018-10-12 14:40 ` [PATCH v1 3/6] MdeModulePkg: Map persistent (NV) memory Sami Mujawar
2018-10-13 9:09 ` Zeng, Star [this message]
2018-10-13 21:28 ` Laszlo Ersek
2018-10-12 14:40 ` [PATCH v1 4/6] ArmVirtPkg: Save DT base address from X0 in PCD Sami Mujawar
2018-10-13 21:35 ` Laszlo Ersek
2018-10-19 14:01 ` Ard Biesheuvel
2018-10-12 14:40 ` [PATCH v1 5/6] ArmVirtPkg: Add kvmtool platform driver Sami Mujawar
2018-10-13 21:54 ` Laszlo Ersek
2018-10-12 14:40 ` [PATCH v1 6/6] ArmVirtPkg: Support for kvmtool emulated platform Sami Mujawar
2018-10-13 21:57 ` Laszlo Ersek
2018-10-13 21:42 ` [PATCH 0/6] Add kvmtool emulated platform support for ARM Laszlo Ersek
2018-10-16 3:00 ` Leif Lindholm
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0C09AFA07DD0434D9E2A0C6AEB0483103BC1E173@shsmsx102.ccr.corp.intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox