* [PATCH v5 1/8] MdeModulePkg: Add Variable Flash Info HOB
2022-04-26 1:29 [PATCH v5 0/8] Add Variable Flash Info HOB Michael Kubacki
@ 2022-04-26 1:29 ` Michael Kubacki
2022-04-26 1:29 ` [PATCH v5 2/8] MdeModulePkg/VariableFlashInfoLib: Add initial library Michael Kubacki
` (7 subsequent siblings)
8 siblings, 0 replies; 26+ messages in thread
From: Michael Kubacki @ 2022-04-26 1:29 UTC (permalink / raw)
To: devel; +Cc: Jian J Wang, Hao A Wu, Liming Gao, Ard Biesheuvel, Sami Mujawar
From: Michael Kubacki <michael.kubacki@microsoft.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3479
Adds a new GUID that is used to identify a HOB that passes variable
flash information to UEFI variable drivers in HOB consumption phases
such as DXE, Traditional MM, and Standalone MM.
This information was previously passed directly with PCDs such
as EfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase
and gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize.
However, the Standalone MM variable driver instance does not have
direct access to the PCD database. Therefore, this HOB will first
be considered as the source for variable flash information and
if platforms do not produce the HOB, reading the information from
the PCDs directly will be a backup to provide backward
compatibility.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Acked-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
---
MdeModulePkg/Include/Guid/VariableFlashInfo.h | 111 ++++++++++++++++++++
MdeModulePkg/MdeModulePkg.dec | 4 +
2 files changed, 115 insertions(+)
diff --git a/MdeModulePkg/Include/Guid/VariableFlashInfo.h b/MdeModulePkg/Include/Guid/VariableFlashInfo.h
new file mode 100644
index 000000000000..992a0dcdd384
--- /dev/null
+++ b/MdeModulePkg/Include/Guid/VariableFlashInfo.h
@@ -0,0 +1,111 @@
+/** @file
+ This file defines the GUID and data structure used to pass information about
+ a variable store mapped on flash (i.e. a MMIO firmware volume) to the modules
+ that consume that information such as the DXE and MM UEFI variable drivers.
+
+ The HOB described in this file is currently optional. It is primarily provided
+ to allow a platform to dynamically describe the flash information to environments
+ such as Standalone MM that cannot access the prior method using dynamic PCDs.
+
+ Even for platforms that use Standalone MM, if the information is only stored
+ statically such as with FixedAtBuild PCDs, the HOB is not required.
+
+ Every point of consumption in this package that uses the PCDs will first check
+ for the HOB and use its value if present.
+
+ Early modules such as the PEI UEFI variable driver might also consume this
+ information. For modules such as these, that execute early in the boot flow,
+ at least two approaches are possible depending on platform design.
+
+ 1. If the information in the HOB exactly matches the information in the PCDs,
+ (i.e. the HOB values are set using the PCD values), let the driver read
+ the information from the PCD and produce the HOB later in boot.
+
+ 2. Produce the HOB very early in boot. For example, the earliest point the HOB
+ is currently consumed is in FaultTolerantWritePei. Note that FaultTolerantWritePei
+ produces gEdkiiFaultTolerantWriteGuid which is a dependency for VariablePei.
+
+ Therefore, attaching a NULL class library to FaultTolerantWritePei with a
+ constructor that produces the HOB will guarantee it is produced before the first
+ point of consumption as the constructor is executed before the module entry point.
+
+ Copyright (c) Microsoft Corporation.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef VARIABLE_FLASH_INFO_H_
+#define VARIABLE_FLASH_INFO_H_
+
+#define VARIABLE_FLASH_INFO_HOB_GUID \
+ { 0x5d11c653, 0x8154, 0x4ac3, { 0xa8, 0xc2, 0xfb, 0xa2, 0x89, 0x20, 0xfc, 0x90 }}
+
+#define VARIABLE_FLASH_INFO_HOB_VERSION 1
+
+extern EFI_GUID gVariableFlashInfoHobGuid;
+
+#pragma pack (push, 1)
+
+///
+/// This structure can be used to describe UEFI variable
+/// flash information.
+///
+typedef struct {
+ ///
+ /// Version of this structure.
+ ///
+ /// Increment the value when the structure is modified.
+ ///
+ UINT32 Version;
+ ///
+ /// Reserved field.
+ ///
+ /// Currently reserved for natural alignment.
+ ///
+ UINT32 Reserved;
+ ///
+ /// Base address of the non-volatile variable range in the flash device.
+ ///
+ /// Note that this address should align with the block size requirements of the flash device.
+ ///
+ EFI_PHYSICAL_ADDRESS NvVariableBaseAddress;
+ ///
+ /// Size of the non-volatile variable range in the flash device.
+ ///
+ /// Note that this value should be less than or equal to FtwSpareLength to support reclaim of
+ /// entire variable store area.
+ /// Note that this address should align with the block size requirements of the flash device.
+ ///
+ UINT64 NvVariableLength;
+ ///
+ /// Base address of the FTW spare block range in the flash device.
+ ///
+ /// Note that this address should align with the block size requirements of the flash device.
+ ///
+ EFI_PHYSICAL_ADDRESS FtwSpareBaseAddress;
+ ///
+ /// Size of the FTW spare block range in the flash device.
+ ///
+ /// Note that this value should be greater than or equal to NvVariableLength.
+ /// Note that this address should align with the block size requirements of the flash device.
+ ///
+ UINT64 FtwSpareLength;
+ ///
+ /// Base address of the FTW working block range in the flash device.
+ ///
+ /// Note that if FtwWorkingLength is larger than on block size, this value should be block size aligned.
+ ///
+ EFI_PHYSICAL_ADDRESS FtwWorkingBaseAddress;
+ ///
+ /// Size of the FTW working block range in the flash device.
+ ///
+ /// Note that if the value is less than on block size, the range should not span blocks.
+ /// Note that if the value is larger than one block size, this value should be block size aligned.
+ ///
+ UINT64 FtwWorkingLength;
+} VARIABLE_FLASH_INFO;
+
+#pragma pack (pop)
+
+#endif
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index cf79292ec877..4e82f5836096 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -226,6 +226,10 @@ [Guids]
# Include/Guid/SmmVariableCommon.h
gSmmVariableWriteGuid = { 0x93ba1826, 0xdffb, 0x45dd, { 0x82, 0xa7, 0xe7, 0xdc, 0xaa, 0x3b, 0xbd, 0xf3 }}
+ ## Guid of the variable flash information HOB.
+ # Include/Guid/VariableFlashInfo.h
+ gVariableFlashInfoHobGuid = { 0x5d11c653, 0x8154, 0x4ac3, { 0xa8, 0xc2, 0xfb, 0xa2, 0x89, 0x20, 0xfc, 0x90 }}
+
## Performance protocol guid that also acts as the performance HOB guid and performance variable GUID
# Include/Guid/Performance.h
gPerformanceProtocolGuid = { 0x76B6BDFA, 0x2ACD, 0x4462, { 0x9E, 0x3F, 0xCB, 0x58, 0xC9, 0x69, 0xD9, 0x37 } }
--
2.28.0.windows.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v5 2/8] MdeModulePkg/VariableFlashInfoLib: Add initial library
2022-04-26 1:29 [PATCH v5 0/8] Add Variable Flash Info HOB Michael Kubacki
2022-04-26 1:29 ` [PATCH v5 1/8] MdeModulePkg: " Michael Kubacki
@ 2022-04-26 1:29 ` Michael Kubacki
2022-04-26 1:29 ` [PATCH v5 3/8] MdeModulePkg/Variable: Consume Variable Flash Info Michael Kubacki
` (6 subsequent siblings)
8 siblings, 0 replies; 26+ messages in thread
From: Michael Kubacki @ 2022-04-26 1:29 UTC (permalink / raw)
To: devel; +Cc: Jian J Wang, Hao A Wu, Liming Gao, Ard Biesheuvel, Sami Mujawar
From: Michael Kubacki <michael.kubacki@microsoft.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3479
Adds a new library class VariableFlashInfoLib that abstracts access
to variable flash information. The instance provided first attempts
to retrieve information from the Variable Flash Info HOB. If that
HOB is not present, it falls back to the PCDs defined in
MdeModulePkg.
This fall back behavior provides backward compatibility for platforms
that only provide PCDs but also allows platforms that need to
dynamically provide the information using the Variable Flash Info HOB
to do so at runtime.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Acked-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
---
MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.c | 179 ++++++++++++++++++++
MdeModulePkg/Include/Library/VariableFlashInfoLib.h | 68 ++++++++
MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.inf | 48 ++++++
MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.uni | 12 ++
MdeModulePkg/MdeModulePkg.dec | 4 +
MdeModulePkg/MdeModulePkg.dsc | 2 +
6 files changed, 313 insertions(+)
diff --git a/MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.c b/MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.c
new file mode 100644
index 000000000000..d5897225897b
--- /dev/null
+++ b/MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.c
@@ -0,0 +1,179 @@
+/** @file
+ Variable Flash Information Library
+
+ Copyright (c) Microsoft Corporation<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Uefi.h>
+#include <Pi/PiMultiPhase.h>
+#include <Guid/VariableFlashInfo.h>
+#include <Library/DebugLib.h>
+#include <Library/HobLib.h>
+#include <Library/VariableFlashInfoLib.h>
+
+/**
+ Get the HOB that contains variable flash information.
+
+ @param[out] VariableFlashInfo Pointer to a pointer to set to the variable flash information structure.
+
+ @retval EFI_SUCCESS Variable flash information was found successfully.
+ @retval EFI_INVALID_PARAMETER The VariableFlashInfo pointer given is NULL.
+ @retval EFI_NOT_FOUND Variable flash information could not be found.
+
+**/
+STATIC
+EFI_STATUS
+GetVariableFlashInfoFromHob (
+ OUT VARIABLE_FLASH_INFO **VariableFlashInfo
+ )
+{
+ EFI_HOB_GUID_TYPE *GuidHob;
+
+ if (VariableFlashInfo == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ GuidHob = GetFirstGuidHob (&gVariableFlashInfoHobGuid);
+ if (GuidHob == NULL) {
+ return EFI_NOT_FOUND;
+ }
+
+ *VariableFlashInfo = GET_GUID_HOB_DATA (GuidHob);
+
+ //
+ // Assert if more than one variable flash information HOB is present.
+ //
+ DEBUG_CODE (
+ if ((GetNextGuidHob (&gVariableFlashInfoHobGuid, GET_NEXT_HOB (GuidHob)) != NULL)) {
+ DEBUG ((DEBUG_ERROR, "ERROR: Found two variable flash information HOBs\n"));
+ ASSERT (FALSE);
+ }
+
+ );
+
+ return EFI_SUCCESS;
+}
+
+/**
+ Get the base address and size for the NV storage area used for UEFI variable storage.
+
+ @param[out] BaseAddress The NV storage base address.
+ @param[out] Length The NV storage length in bytes.
+
+ @retval EFI_SUCCESS NV storage information was found successfully.
+ @retval EFI_INVALID_PARAMETER A required pointer parameter is NULL.
+
+**/
+EFI_STATUS
+EFIAPI
+GetVariableFlashNvStorageInfo (
+ OUT EFI_PHYSICAL_ADDRESS *BaseAddress,
+ OUT UINT64 *Length
+ )
+{
+ EFI_STATUS Status;
+ VARIABLE_FLASH_INFO *VariableFlashInfo;
+
+ if ((BaseAddress == NULL) || (Length == NULL)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ Status = GetVariableFlashInfoFromHob (&VariableFlashInfo);
+ if (!EFI_ERROR (Status)) {
+ *BaseAddress = VariableFlashInfo->NvVariableBaseAddress;
+ *Length = VariableFlashInfo->NvVariableLength;
+ } else {
+ *BaseAddress = (EFI_PHYSICAL_ADDRESS)(PcdGet64 (PcdFlashNvStorageVariableBase64) != 0 ?
+ PcdGet64 (PcdFlashNvStorageVariableBase64) :
+ PcdGet32 (PcdFlashNvStorageVariableBase)
+ );
+ *Length = (UINT64)PcdGet32 (PcdFlashNvStorageVariableSize);
+ }
+
+ return EFI_SUCCESS;
+}
+
+/**
+ Get the base address and size for the fault tolerant write (FTW) spare
+ area used for UEFI variable storage.
+
+ @param[out] BaseAddress The FTW spare base address.
+ @param[out] Length The FTW spare length in bytes.
+
+ @retval EFI_SUCCESS FTW spare information was found successfully.
+ @retval EFI_INVALID_PARAMETER A required pointer parameter is NULL.
+ @retval EFI_NOT_FOUND FTW spare information could not be found.
+
+**/
+EFI_STATUS
+EFIAPI
+GetVariableFlashFtwSpareInfo (
+ OUT EFI_PHYSICAL_ADDRESS *BaseAddress,
+ OUT UINT64 *Length
+ )
+{
+ EFI_STATUS Status;
+ VARIABLE_FLASH_INFO *VariableFlashInfo;
+
+ if ((BaseAddress == NULL) || (Length == NULL)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ Status = GetVariableFlashInfoFromHob (&VariableFlashInfo);
+ if (!EFI_ERROR (Status)) {
+ *BaseAddress = VariableFlashInfo->FtwSpareBaseAddress;
+ *Length = VariableFlashInfo->FtwSpareLength;
+ } else {
+ *BaseAddress = (EFI_PHYSICAL_ADDRESS)(PcdGet64 (PcdFlashNvStorageFtwSpareBase64) != 0 ?
+ PcdGet64 (PcdFlashNvStorageFtwSpareBase64) :
+ PcdGet32 (PcdFlashNvStorageFtwSpareBase)
+ );
+ *Length = (UINT64)PcdGet32 (PcdFlashNvStorageFtwSpareSize);
+ }
+
+ return EFI_SUCCESS;
+}
+
+/**
+ Get the base address and size for the fault tolerant write (FTW) working
+ area used for UEFI variable storage.
+
+ @param[out] BaseAddress The FTW working area base address.
+ @param[out] Length The FTW working area length in bytes.
+
+ @retval EFI_SUCCESS FTW working information was found successfully.
+ @retval EFI_INVALID_PARAMETER A required pointer parameter is NULL.
+ @retval EFI_NOT_FOUND FTW working information could not be found.
+
+**/
+EFI_STATUS
+EFIAPI
+GetVariableFlashFtwWorkingInfo (
+ OUT EFI_PHYSICAL_ADDRESS *BaseAddress,
+ OUT UINT64 *Length
+ )
+{
+ EFI_STATUS Status;
+ VARIABLE_FLASH_INFO *VariableFlashInfo;
+
+ if ((BaseAddress == NULL) || (Length == NULL)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ Status = GetVariableFlashInfoFromHob (&VariableFlashInfo);
+ if (!EFI_ERROR (Status)) {
+ *BaseAddress = VariableFlashInfo->FtwWorkingBaseAddress;
+ *Length = VariableFlashInfo->FtwWorkingLength;
+ } else {
+ *BaseAddress = (EFI_PHYSICAL_ADDRESS)(PcdGet64 (PcdFlashNvStorageFtwWorkingBase64) != 0 ?
+ PcdGet64 (PcdFlashNvStorageFtwWorkingBase64) :
+ PcdGet32 (PcdFlashNvStorageFtwWorkingBase)
+ );
+ *Length = (UINT64)PcdGet32 (PcdFlashNvStorageFtwWorkingSize);
+ }
+
+ return EFI_SUCCESS;
+}
diff --git a/MdeModulePkg/Include/Library/VariableFlashInfoLib.h b/MdeModulePkg/Include/Library/VariableFlashInfoLib.h
new file mode 100644
index 000000000000..1367be9376ea
--- /dev/null
+++ b/MdeModulePkg/Include/Library/VariableFlashInfoLib.h
@@ -0,0 +1,68 @@
+/** @file
+ Variable Flash Information Library
+
+Copyright (c) Microsoft Corporation<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef VARIABLE_FLASH_INFO_LIB_H_
+#define VARIABLE_FLASH_INFO_LIB_H_
+
+/**
+ Get the base address and size for the NV storage area used for UEFI variable storage.
+
+ @param[out] BaseAddress The NV storage base address.
+ @param[out] Length The NV storage length in bytes.
+
+ @retval EFI_SUCCESS NV storage information was found successfully.
+ @retval EFI_INVALID_PARAMETER A required pointer parameter is NULL.
+ @retval EFI_NOT_FOUND NV storage information could not be found.
+
+**/
+EFI_STATUS
+EFIAPI
+GetVariableFlashNvStorageInfo (
+ OUT EFI_PHYSICAL_ADDRESS *BaseAddress,
+ OUT UINT64 *Length
+ );
+
+/**
+ Get the base address and size for the fault tolerant write (FTW) spare
+ area used for UEFI variable storage.
+
+ @param[out] BaseAddress The FTW spare base address.
+ @param[out] Length The FTW spare length in bytes.
+
+ @retval EFI_SUCCESS FTW spare information was found successfully.
+ @retval EFI_INVALID_PARAMETER A required pointer parameter is NULL.
+ @retval EFI_NOT_FOUND FTW spare information could not be found.
+
+**/
+EFI_STATUS
+EFIAPI
+GetVariableFlashFtwSpareInfo (
+ OUT EFI_PHYSICAL_ADDRESS *BaseAddress,
+ OUT UINT64 *Length
+ );
+
+/**
+ Get the base address and size for the fault tolerant write (FTW) working
+ area used for UEFI variable storage.
+
+ @param[out] BaseAddress The FTW working area base address.
+ @param[out] Length The FTW working area length in bytes.
+
+ @retval EFI_SUCCESS FTW working information was found successfully.
+ @retval EFI_INVALID_PARAMETER A required pointer parameter is NULL.
+ @retval EFI_NOT_FOUND FTW working information could not be found.
+
+**/
+EFI_STATUS
+EFIAPI
+GetVariableFlashFtwWorkingInfo (
+ OUT EFI_PHYSICAL_ADDRESS *BaseAddress,
+ OUT UINT64 *Length
+ );
+
+#endif
diff --git a/MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.inf b/MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.inf
new file mode 100644
index 000000000000..70175e75f9b1
--- /dev/null
+++ b/MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.inf
@@ -0,0 +1,48 @@
+## @file
+# Variable Flash Information Library
+#
+# Provides services to access UEFI variable flash information.
+#
+# Copyright (c) Microsoft Corporation<BR>
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = BaseVariableFlashInfoLib
+ MODULE_UNI_FILE = BaseVariableFlashInfoLib.uni
+ FILE_GUID = DEC426C9-C92E-4BAD-8E93-3F61C261118B
+ MODULE_TYPE = BASE
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = VariableFlashInfoLib
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+# VALID_ARCHITECTURES = ANY
+#
+
+[Sources]
+ BaseVariableFlashInfoLib.c
+
+[Packages]
+ MdePkg/MdePkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+
+[LibraryClasses]
+ DebugLib
+ HobLib
+
+[Guids]
+ gVariableFlashInfoHobGuid ## CONSUMES ## HOB
+
+[Pcd]
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase ## SOMETIMES_CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 ## SOMETIMES_CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize ## SOMETIMES_CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase ## SOMETIMES_CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64 ## SOMETIMES_CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize ## SOMETIMES_CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase ## SOMETIMES_CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64 ## SOMETIMES_CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize ## SOMETIMES_CONSUMES
diff --git a/MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.uni b/MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.uni
new file mode 100644
index 000000000000..9a5348fa02a0
--- /dev/null
+++ b/MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.uni
@@ -0,0 +1,12 @@
+// /** @file
+// Variable Flash Information Library
+//
+// Copyright (c) Microsoft Corporation<BR>
+//
+// SPDX-License-Identifier: BSD-2-Clause-Patent
+//
+// **/
+
+#string STR_MODULE_ABSTRACT #language en-US "UEFI variable flash information library"
+
+#string STR_MODULE_DESCRIPTION #language en-US "Provides services to access UEFI variable flash information."
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 4e82f5836096..2bcb9f9453af 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -154,6 +154,10 @@ [LibraryClasses]
#
VariablePolicyHelperLib|Include/Library/VariablePolicyHelperLib.h
+ ## @libraryclass Provides services to access UEFI variable flash information.
+ #
+ VariableFlashInfoLib|Include/Library/VariableFlashInfoLib.h
+
[Guids]
## MdeModule package token space guid
# Include/Guid/MdeModulePkgTokenSpace.h
diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc
index b1d83461865e..90a0a7ec4a7c 100644
--- a/MdeModulePkg/MdeModulePkg.dsc
+++ b/MdeModulePkg/MdeModulePkg.dsc
@@ -103,6 +103,7 @@ [LibraryClasses]
DisplayUpdateProgressLib|MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/DisplayUpdateProgressLibGraphics.inf
VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
MmUnblockMemoryLib|MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.inf
+ VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.inf
[LibraryClasses.EBC.PEIM]
IoLib|MdePkg/Library/PeiIoLibCpuIo/PeiIoLibCpuIo.inf
@@ -440,6 +441,7 @@ [Components]
MdeModulePkg/Library/FmpAuthenticationLibNull/FmpAuthenticationLibNull.inf
MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf
+ MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.inf
[Components.IA32, Components.X64, Components.AARCH64]
MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
--
2.28.0.windows.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v5 3/8] MdeModulePkg/Variable: Consume Variable Flash Info
2022-04-26 1:29 [PATCH v5 0/8] Add Variable Flash Info HOB Michael Kubacki
2022-04-26 1:29 ` [PATCH v5 1/8] MdeModulePkg: " Michael Kubacki
2022-04-26 1:29 ` [PATCH v5 2/8] MdeModulePkg/VariableFlashInfoLib: Add initial library Michael Kubacki
@ 2022-04-26 1:29 ` Michael Kubacki
2022-04-26 1:29 ` [PATCH v5 4/8] MdeModulePkg/FaultTolerantWrite: " Michael Kubacki
` (5 subsequent siblings)
8 siblings, 0 replies; 26+ messages in thread
From: Michael Kubacki @ 2022-04-26 1:29 UTC (permalink / raw)
To: devel; +Cc: Jian J Wang, Hao A Wu, Liming Gao, Ard Biesheuvel, Sami Mujawar
From: Michael Kubacki <michael.kubacki@microsoft.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3479
Updates VariableRuntimeDxe, VariableSmm, and VariableStandaloneMm
to acquire variable flash information from the Variable Flash
Information library.
Note: This introduces a dependency on VariableFlashInfoLib in these
modules. Therefore, a platform building the variable modules must
specify an instance of VariableFlashInfoLib in their platform build.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Acked-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
---
MdeModulePkg/Universal/Variable/Pei/Variable.c | 14 +++++++++-----
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c | 16 ++++++++++++----
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.c | 14 ++++++++++----
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c | 17 +++++++++++++----
MdeModulePkg/Universal/Variable/Pei/Variable.h | 2 ++
MdeModulePkg/Universal/Variable/Pei/VariablePei.inf | 5 ++---
MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h | 7 ++-----
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf | 5 ++---
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf | 5 ++---
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf | 5 ++---
10 files changed, 56 insertions(+), 34 deletions(-)
diff --git a/MdeModulePkg/Universal/Variable/Pei/Variable.c b/MdeModulePkg/Universal/Variable/Pei/Variable.c
index b36dd0de67b2..26a4c73b45a5 100644
--- a/MdeModulePkg/Universal/Variable/Pei/Variable.c
+++ b/MdeModulePkg/Universal/Variable/Pei/Variable.c
@@ -567,11 +567,13 @@ GetVariableStore (
OUT VARIABLE_STORE_INFO *StoreInfo
)
{
+ EFI_STATUS Status;
EFI_HOB_GUID_TYPE *GuidHob;
EFI_FIRMWARE_VOLUME_HEADER *FvHeader;
VARIABLE_STORE_HEADER *VariableStoreHeader;
EFI_PHYSICAL_ADDRESS NvStorageBase;
UINT32 NvStorageSize;
+ UINT64 NvStorageSize64;
FAULT_TOLERANT_WRITE_LAST_WRITE_DATA *FtwLastWriteData;
UINT32 BackUpOffset;
@@ -591,11 +593,13 @@ GetVariableStore (
// Emulated non-volatile variable mode is not enabled.
//
- NvStorageSize = PcdGet32 (PcdFlashNvStorageVariableSize);
- NvStorageBase = (EFI_PHYSICAL_ADDRESS)(PcdGet64 (PcdFlashNvStorageVariableBase64) != 0 ?
- PcdGet64 (PcdFlashNvStorageVariableBase64) :
- PcdGet32 (PcdFlashNvStorageVariableBase)
- );
+ Status = GetVariableFlashNvStorageInfo (&NvStorageBase, &NvStorageSize64);
+ ASSERT_EFI_ERROR (Status);
+
+ Status = SafeUint64ToUint32 (NvStorageSize64, &NvStorageSize);
+ // This driver currently assumes the size will be UINT32 so assert the value is safe for now.
+ ASSERT_EFI_ERROR (Status);
+
ASSERT (NvStorageBase != 0);
//
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
index 03fec3048dc4..d5c409c914d1 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
@@ -423,6 +423,8 @@ FtwNotificationEvent (
EFI_PHYSICAL_ADDRESS VariableStoreBase;
UINT64 VariableStoreLength;
UINTN FtwMaxBlockSize;
+ UINT32 NvStorageVariableSize;
+ UINT64 NvStorageVariableSize64;
//
// Ensure FTW protocol is installed.
@@ -432,14 +434,20 @@ FtwNotificationEvent (
return;
}
+ Status = GetVariableFlashNvStorageInfo (&NvStorageVariableBase, &NvStorageVariableSize64);
+ ASSERT_EFI_ERROR (Status);
+
+ Status = SafeUint64ToUint32 (NvStorageVariableSize64, &NvStorageVariableSize);
+ // This driver currently assumes the size will be UINT32 so assert the value is safe for now.
+ ASSERT_EFI_ERROR (Status);
+
+ VariableStoreBase = NvStorageVariableBase + mNvFvHeaderCache->HeaderLength;
+
Status = FtwProtocol->GetMaxBlockSize (FtwProtocol, &FtwMaxBlockSize);
if (!EFI_ERROR (Status)) {
- ASSERT (PcdGet32 (PcdFlashNvStorageVariableSize) <= FtwMaxBlockSize);
+ ASSERT (NvStorageVariableSize <= FtwMaxBlockSize);
}
- NvStorageVariableBase = NV_STORAGE_VARIABLE_BASE;
- VariableStoreBase = NvStorageVariableBase + mNvFvHeaderCache->HeaderLength;
-
//
// Let NonVolatileVariableBase point to flash variable store base directly after FTW ready.
//
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.c
index 5e9d40b67ac2..9e2d8fe0fe0c 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.c
@@ -142,6 +142,7 @@ InitRealNonVolatileVariableStore (
EFI_PHYSICAL_ADDRESS NvStorageBase;
UINT8 *NvStorageData;
UINT32 NvStorageSize;
+ UINT64 NvStorageSize64;
FAULT_TOLERANT_WRITE_LAST_WRITE_DATA *FtwLastWriteData;
UINT32 BackUpOffset;
UINT32 BackUpSize;
@@ -153,19 +154,24 @@ InitRealNonVolatileVariableStore (
mVariableModuleGlobal->FvbInstance = NULL;
+ Status = GetVariableFlashNvStorageInfo (&NvStorageBase, &NvStorageSize64);
+ ASSERT_EFI_ERROR (Status);
+
+ Status = SafeUint64ToUint32 (NvStorageSize64, &NvStorageSize);
+ // This driver currently assumes the size will be UINT32 so assert the value is safe for now.
+ ASSERT_EFI_ERROR (Status);
+
+ ASSERT (NvStorageBase != 0);
+
//
// Allocate runtime memory used for a memory copy of the FLASH region.
// Keep the memory and the FLASH in sync as updates occur.
//
- NvStorageSize = PcdGet32 (PcdFlashNvStorageVariableSize);
NvStorageData = AllocateRuntimeZeroPool (NvStorageSize);
if (NvStorageData == NULL) {
return EFI_OUT_OF_RESOURCES;
}
- NvStorageBase = NV_STORAGE_VARIABLE_BASE;
- ASSERT (NvStorageBase != 0);
-
//
// Copy NV storage data to the memory buffer.
//
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
index 517cae7b00f8..5253c328dcd9 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
@@ -1084,6 +1084,8 @@ SmmFtwNotificationEvent (
EFI_SMM_FAULT_TOLERANT_WRITE_PROTOCOL *FtwProtocol;
EFI_PHYSICAL_ADDRESS NvStorageVariableBase;
UINTN FtwMaxBlockSize;
+ UINT32 NvStorageVariableSize;
+ UINT64 NvStorageVariableSize64;
if (mVariableModuleGlobal->FvbInstance != NULL) {
return EFI_SUCCESS;
@@ -1097,14 +1099,21 @@ SmmFtwNotificationEvent (
return Status;
}
+ Status = GetVariableFlashNvStorageInfo (&NvStorageVariableBase, &NvStorageVariableSize64);
+ ASSERT_EFI_ERROR (Status);
+
+ Status = SafeUint64ToUint32 (NvStorageVariableSize64, &NvStorageVariableSize);
+ // This driver currently assumes the size will be UINT32 so assert the value is safe for now.
+ ASSERT_EFI_ERROR (Status);
+
+ ASSERT (NvStorageVariableBase != 0);
+ VariableStoreBase = NvStorageVariableBase + mNvFvHeaderCache->HeaderLength;
+
Status = FtwProtocol->GetMaxBlockSize (FtwProtocol, &FtwMaxBlockSize);
if (!EFI_ERROR (Status)) {
- ASSERT (PcdGet32 (PcdFlashNvStorageVariableSize) <= FtwMaxBlockSize);
+ ASSERT (NvStorageVariableSize <= FtwMaxBlockSize);
}
- NvStorageVariableBase = NV_STORAGE_VARIABLE_BASE;
- VariableStoreBase = NvStorageVariableBase + mNvFvHeaderCache->HeaderLength;
-
//
// Let NonVolatileVariableBase point to flash variable store base directly after FTW ready.
//
diff --git a/MdeModulePkg/Universal/Variable/Pei/Variable.h b/MdeModulePkg/Universal/Variable/Pei/Variable.h
index 7f9ad5bfc357..51effbf79987 100644
--- a/MdeModulePkg/Universal/Variable/Pei/Variable.h
+++ b/MdeModulePkg/Universal/Variable/Pei/Variable.h
@@ -20,6 +20,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Library/BaseMemoryLib.h>
#include <Library/PeiServicesTablePointerLib.h>
#include <Library/PeiServicesLib.h>
+#include <Library/SafeIntLib.h>
+#include <Library/VariableFlashInfoLib.h>
#include <Guid/VariableFormat.h>
#include <Guid/VariableIndexTable.h>
diff --git a/MdeModulePkg/Universal/Variable/Pei/VariablePei.inf b/MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
index 7cbdd2385e8f..7264a24bdf71 100644
--- a/MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
+++ b/MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
@@ -39,6 +39,8 @@ [LibraryClasses]
DebugLib
PeiServicesTablePointerLib
PeiServicesLib
+ SafeIntLib
+ VariableFlashInfoLib
[Guids]
## CONSUMES ## GUID # Variable store header
@@ -59,9 +61,6 @@ [Ppis]
gEfiPeiReadOnlyVariable2PpiGuid ## PRODUCES
[Pcd]
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase ## SOMETIMES_CONSUMES
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 ## CONSUMES
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable ## SOMETIMES_CONSUMES
[Depex]
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
index 31e408976a35..a668abb82b15 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
@@ -31,6 +31,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Library/MemoryAllocationLib.h>
#include <Library/AuthVariableLib.h>
#include <Library/VarCheckLib.h>
+#include <Library/VariableFlashInfoLib.h>
+#include <Library/SafeIntLib.h>
#include <Guid/GlobalVariable.h>
#include <Guid/EventGroup.h>
#include <Guid/VariableFormat.h>
@@ -40,11 +42,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include "PrivilegePolymorphic.h"
-#define NV_STORAGE_VARIABLE_BASE (EFI_PHYSICAL_ADDRESS)\
- (PcdGet64 (PcdFlashNvStorageVariableBase64) != 0 ? \
- PcdGet64 (PcdFlashNvStorageVariableBase64) : \
- PcdGet32 (PcdFlashNvStorageVariableBase))
-
#define EFI_VARIABLE_ATTRIBUTES_MASK (EFI_VARIABLE_NON_VOLATILE |\
EFI_VARIABLE_BOOTSERVICE_ACCESS | \
EFI_VARIABLE_RUNTIME_ACCESS | \
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
index c9434df631ee..3858adf6739d 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
@@ -71,8 +71,10 @@ [LibraryClasses]
TpmMeasurementLib
AuthVariableLib
VarCheckLib
+ VariableFlashInfoLib
VariablePolicyLib
VariablePolicyHelperLib
+ SafeIntLib
[Protocols]
gEfiFirmwareVolumeBlockProtocolGuid ## CONSUMES
@@ -125,9 +127,6 @@ [Guids]
gEfiImageSecurityDatabaseGuid
[Pcd]
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize ## CONSUMES
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase ## SOMETIMES_CONSUMES
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVolatileVariableSize ## CONSUMES
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
index eaa97a01c6e5..8c552b87e080 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
@@ -80,8 +80,10 @@ [LibraryClasses]
AuthVariableLib
VarCheckLib
UefiBootServicesTableLib
+ VariableFlashInfoLib
VariablePolicyLib
VariablePolicyHelperLib
+ SafeIntLib
[Protocols]
gEfiSmmFirmwareVolumeBlockProtocolGuid ## CONSUMES
@@ -127,9 +129,6 @@ [Guids]
gEdkiiVarErrorFlagGuid
[Pcd]
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize ## CONSUMES
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase ## SOMETIMES_CONSUMES
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVolatileVariableSize ## CONSUMES
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
index d8c4f77e7f1f..f09bed40cf51 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
@@ -73,9 +73,11 @@ [LibraryClasses]
HobLib
MemoryAllocationLib
MmServicesTableLib
+ SafeIntLib
StandaloneMmDriverEntryPoint
SynchronizationLib
VarCheckLib
+ VariableFlashInfoLib
VariablePolicyLib
VariablePolicyHelperLib
@@ -120,9 +122,6 @@ [Guids]
gEdkiiVarErrorFlagGuid
[Pcd]
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase ## SOMETIMES_CONSUMES
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 ## CONSUMES
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVolatileVariableSize ## CONSUMES
--
2.28.0.windows.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v5 4/8] MdeModulePkg/FaultTolerantWrite: Consume Variable Flash Info
2022-04-26 1:29 [PATCH v5 0/8] Add Variable Flash Info HOB Michael Kubacki
` (2 preceding siblings ...)
2022-04-26 1:29 ` [PATCH v5 3/8] MdeModulePkg/Variable: Consume Variable Flash Info Michael Kubacki
@ 2022-04-26 1:29 ` Michael Kubacki
2022-04-26 1:29 ` [PATCH v5 5/8] ArmVirtPkg/ArmVirt.dsc.inc: Add VariableFlashInfoLib Michael Kubacki
` (4 subsequent siblings)
8 siblings, 0 replies; 26+ messages in thread
From: Michael Kubacki @ 2022-04-26 1:29 UTC (permalink / raw)
To: devel; +Cc: Jian J Wang, Hao A Wu, Liming Gao, Ard Biesheuvel, Sami Mujawar
From: Michael Kubacki <michael.kubacki@microsoft.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3479
Adds support to the UEFI variable fault tolerant write (FTW) drivers
to receive FTW base and size information dynamically via the Variable
Flash Information library.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Acked-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
---
MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c | 41 +++++++++++++-------
MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c | 7 +++-
MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.c | 28 ++++++++-----
MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h | 7 +++-
MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf | 10 +----
MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf | 10 +----
MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf | 10 +----
MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf | 10 +----
8 files changed, 63 insertions(+), 60 deletions(-)
diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c
index 661e1487673b..f1335870e797 100644
--- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c
+++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c
@@ -987,22 +987,43 @@ InitFtwDevice (
OUT EFI_FTW_DEVICE **FtwData
)
{
- EFI_FTW_DEVICE *FtwDevice;
+ EFI_STATUS Status;
+ EFI_PHYSICAL_ADDRESS WorkSpaceAddress;
+ UINT64 Size;
+ UINTN FtwWorkingSize;
+ EFI_FTW_DEVICE *FtwDevice;
+
+ FtwWorkingSize = 0;
+
+ Status = GetVariableFlashFtwWorkingInfo (&WorkSpaceAddress, &Size);
+ ASSERT_EFI_ERROR (Status);
+
+ Status = SafeUint64ToUintn (Size, &FtwWorkingSize);
+ // This driver currently assumes the size will be UINTN so assert the value is safe for now.
+ ASSERT_EFI_ERROR (Status);
//
// Allocate private data of this driver,
// Including the FtwWorkSpace[FTW_WORK_SPACE_SIZE].
//
- FtwDevice = AllocateZeroPool (sizeof (EFI_FTW_DEVICE) + PcdGet32 (PcdFlashNvStorageFtwWorkingSize));
+ FtwDevice = AllocateZeroPool (sizeof (EFI_FTW_DEVICE) + FtwWorkingSize);
if (FtwDevice == NULL) {
return EFI_OUT_OF_RESOURCES;
}
+ FtwDevice->WorkSpaceAddress = WorkSpaceAddress;
+ FtwDevice->WorkSpaceLength = FtwWorkingSize;
+
+ Status = GetVariableFlashFtwSpareInfo (&FtwDevice->SpareAreaAddress, &Size);
+ ASSERT_EFI_ERROR (Status);
+
+ Status = SafeUint64ToUintn (Size, &FtwDevice->SpareAreaLength);
+ // This driver currently assumes the size will be UINTN so assert the value is safe for now.
+ ASSERT_EFI_ERROR (Status);
+
//
// Initialize other parameters, and set WorkSpace as FTW_ERASED_BYTE.
//
- FtwDevice->WorkSpaceLength = (UINTN)PcdGet32 (PcdFlashNvStorageFtwWorkingSize);
- FtwDevice->SpareAreaLength = (UINTN)PcdGet32 (PcdFlashNvStorageFtwSpareSize);
if ((FtwDevice->WorkSpaceLength == 0) || (FtwDevice->SpareAreaLength == 0)) {
DEBUG ((DEBUG_ERROR, "Ftw: Workspace or Spare block does not exist!\n"));
FreePool (FtwDevice);
@@ -1015,16 +1036,6 @@ InitFtwDevice (
FtwDevice->FtwWorkSpaceLba = (EFI_LBA)(-1);
FtwDevice->FtwSpareLba = (EFI_LBA)(-1);
- FtwDevice->WorkSpaceAddress = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdFlashNvStorageFtwWorkingBase64);
- if (FtwDevice->WorkSpaceAddress == 0) {
- FtwDevice->WorkSpaceAddress = (EFI_PHYSICAL_ADDRESS)PcdGet32 (PcdFlashNvStorageFtwWorkingBase);
- }
-
- FtwDevice->SpareAreaAddress = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdFlashNvStorageFtwSpareBase64);
- if (FtwDevice->SpareAreaAddress == 0) {
- FtwDevice->SpareAreaAddress = (EFI_PHYSICAL_ADDRESS)PcdGet32 (PcdFlashNvStorageFtwSpareBase);
- }
-
*FtwData = FtwDevice;
return EFI_SUCCESS;
}
@@ -1277,7 +1288,7 @@ InitFtwProtocol (
FtwDevice->FtwLastWriteHeader = NULL;
FtwDevice->FtwLastWriteRecord = NULL;
- InitializeLocalWorkSpaceHeader ();
+ InitializeLocalWorkSpaceHeader (FtwDevice->WorkSpaceLength);
//
// Refresh the working space data from working block
diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c b/MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c
index 61e7a92ccea1..fd563643eb63 100644
--- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c
+++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c
@@ -16,10 +16,13 @@ EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER mWorkingBlockHeader = { ZERO_GUID, 0, 0
Since Signature and WriteQueueSize have been known, Crc can be calculated out,
then the work space header will be fixed.
+
+ @param[in] WorkSpaceLength Length in bytes of the FTW workspace area.
+
**/
VOID
InitializeLocalWorkSpaceHeader (
- VOID
+ IN UINTN WorkSpaceLength
)
{
//
@@ -46,7 +49,7 @@ InitializeLocalWorkSpaceHeader (
&gEdkiiWorkingBlockSignatureGuid,
sizeof (EFI_GUID)
);
- mWorkingBlockHeader.WriteQueueSize = PcdGet32 (PcdFlashNvStorageFtwWorkingSize) - sizeof (EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER);
+ mWorkingBlockHeader.WriteQueueSize = WorkSpaceLength - sizeof (EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER);
//
// Crc is calculated with all the fields except Crc and STATE, so leave them as FTW_ERASED_BYTE.
diff --git a/MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.c b/MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.c
index 15543f12ed29..8c152dcbad98 100644
--- a/MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.c
+++ b/MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.c
@@ -16,6 +16,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Library/DebugLib.h>
#include <Library/BaseMemoryLib.h>
#include <Library/HobLib.h>
+#include <Library/SafeIntLib.h>
+#include <Library/VariableFlashInfoLib.h>
EFI_PEI_PPI_DESCRIPTOR mPpiListVariable = {
(EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
@@ -212,25 +214,31 @@ PeimFaultTolerantWriteInitialize (
EFI_PHYSICAL_ADDRESS SpareAreaAddress;
UINTN SpareAreaLength;
EFI_PHYSICAL_ADDRESS WorkSpaceInSpareArea;
+ UINT64 Size;
FAULT_TOLERANT_WRITE_LAST_WRITE_DATA FtwLastWrite;
FtwWorkingBlockHeader = NULL;
FtwLastWriteHeader = NULL;
FtwLastWriteRecord = NULL;
- WorkSpaceAddress = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdFlashNvStorageFtwWorkingBase64);
- if (WorkSpaceAddress == 0) {
- WorkSpaceAddress = (EFI_PHYSICAL_ADDRESS)PcdGet32 (PcdFlashNvStorageFtwWorkingBase);
- }
+ SpareAreaAddress = 0;
+ SpareAreaLength = 0;
+ WorkSpaceAddress = 0;
+ WorkSpaceLength = 0;
- WorkSpaceLength = (UINTN)PcdGet32 (PcdFlashNvStorageFtwWorkingSize);
+ Status = GetVariableFlashFtwWorkingInfo (&WorkSpaceAddress, &Size);
+ ASSERT_EFI_ERROR (Status);
- SpareAreaAddress = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdFlashNvStorageFtwSpareBase64);
- if (SpareAreaAddress == 0) {
- SpareAreaAddress = (EFI_PHYSICAL_ADDRESS)PcdGet32 (PcdFlashNvStorageFtwSpareBase);
- }
+ Status = SafeUint64ToUintn (Size, &WorkSpaceLength);
+ // This driver currently assumes the size will be UINTN so assert the value is safe for now.
+ ASSERT_EFI_ERROR (Status);
- SpareAreaLength = (UINTN)PcdGet32 (PcdFlashNvStorageFtwSpareSize);
+ Status = GetVariableFlashFtwSpareInfo (&SpareAreaAddress, &Size);
+ ASSERT_EFI_ERROR (Status);
+
+ Status = SafeUint64ToUintn (Size, &SpareAreaLength);
+ // This driver currently assumes the size will be UINTN so assert the value is safe for now.
+ ASSERT_EFI_ERROR (Status);
//
// The address of FTW working base and spare base must not be 0.
diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h
index c14e47b5c7b2..5b84d062c294 100644
--- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h
+++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h
@@ -26,6 +26,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Library/BaseMemoryLib.h>
#include <Library/MemoryAllocationLib.h>
#include <Library/ReportStatusCodeLib.h>
+#include <Library/SafeIntLib.h>
+#include <Library/VariableFlashInfoLib.h>
//
// Flash erase polarity is 1
@@ -708,10 +710,13 @@ InitFtwProtocol (
Since Signature and WriteQueueSize have been known, Crc can be calculated out,
then the work space header will be fixed.
+
+ @param[in] WorkSpaceLength Length in bytes of the FTW workspace area.
+
**/
VOID
InitializeLocalWorkSpaceHeader (
- VOID
+ IN UINTN WorkSpaceLength
);
/**
diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
index 96165614d178..d524e1849a2e 100644
--- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
+++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
@@ -46,6 +46,8 @@ [LibraryClasses]
UefiLib
PcdLib
ReportStatusCodeLib
+ SafeIntLib
+ VariableFlashInfoLib
[Guids]
#
@@ -65,14 +67,6 @@ [Protocols]
[FeaturePcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdFullFtwServiceEnable ## CONSUMES
-[Pcd]
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase ## SOMETIMES_CONSUMES
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64 ## CONSUMES
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize ## CONSUMES
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase ## SOMETIMES_CONSUMES
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64 ## CONSUMES
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize ## CONSUMES
-
#
# gBS->CalculateCrc32() is consumed in EntryPoint.
# PI spec said: When the DXE Foundation is notified that the EFI_RUNTIME_ARCH_PROTOCOL
diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf
index 8cc6028470d8..8a4b9ad24657 100644
--- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf
+++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf
@@ -52,6 +52,8 @@ [LibraryClasses]
ReportStatusCodeLib
SmmMemLib
BaseLib
+ SafeIntLib
+ VariableFlashInfoLib
[Guids]
#
@@ -74,14 +76,6 @@ [Protocols]
[FeaturePcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdFullFtwServiceEnable ## CONSUMES
-[Pcd]
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase ## SOMETIMES_CONSUMES
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64 ## CONSUMES
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize ## CONSUMES
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase ## SOMETIMES_CONSUMES
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64 ## CONSUMES
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize ## CONSUMES
-
#
# gBS->CalculateCrc32() is consumed in EntryPoint.
# PI spec said: When the DXE Foundation is notified that the EFI_RUNTIME_ARCH_PROTOCOL
diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf
index d0fab7d9414f..0ac6edf771ab 100644
--- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf
+++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf
@@ -50,7 +50,9 @@ [LibraryClasses]
MmServicesTableLib
PcdLib
ReportStatusCodeLib
+ SafeIntLib
StandaloneMmDriverEntryPoint
+ VariableFlashInfoLib
[Guids]
#
@@ -73,13 +75,5 @@ [Protocols]
[FeaturePcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdFullFtwServiceEnable ## CONSUMES
-[Pcd]
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase ## SOMETIMES_CONSUMES
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64 ## CONSUMES
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize ## CONSUMES
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase ## SOMETIMES_CONSUMES
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64 ## CONSUMES
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize ## CONSUMES
-
[Depex]
TRUE
diff --git a/MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf b/MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
index f90892ad4493..230138272c3a 100644
--- a/MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
+++ b/MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
@@ -39,6 +39,8 @@ [LibraryClasses]
HobLib
BaseMemoryLib
PcdLib
+ SafeIntLib
+ VariableFlashInfoLib
[Guids]
## SOMETIMES_PRODUCES ## HOB
@@ -47,14 +49,6 @@ [Guids]
gEdkiiWorkingBlockSignatureGuid ## SOMETIMES_CONSUMES ## GUID
gEfiSystemNvDataFvGuid ## SOMETIMES_CONSUMES ## GUID
-[Pcd]
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase ## SOMETIMES_CONSUMES
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64 ## CONSUMES
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize ## CONSUMES
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase ## SOMETIMES_CONSUMES
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64 ## CONSUMES
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize ## CONSUMES
-
[Depex]
TRUE
--
2.28.0.windows.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v5 5/8] ArmVirtPkg/ArmVirt.dsc.inc: Add VariableFlashInfoLib
2022-04-26 1:29 [PATCH v5 0/8] Add Variable Flash Info HOB Michael Kubacki
` (3 preceding siblings ...)
2022-04-26 1:29 ` [PATCH v5 4/8] MdeModulePkg/FaultTolerantWrite: " Michael Kubacki
@ 2022-04-26 1:29 ` Michael Kubacki
2022-04-26 1:29 ` [PATCH v5 6/8] EmulatorPkg: " Michael Kubacki
` (3 subsequent siblings)
8 siblings, 0 replies; 26+ messages in thread
From: Michael Kubacki @ 2022-04-26 1:29 UTC (permalink / raw)
To: devel
Cc: Ard Biesheuvel, Leif Lindholm, Sami Mujawar, Gerd Hoffmann,
Julien Grall, Ard Biesheuvel, Liming Gao
From: Michael Kubacki <michael.kubacki@microsoft.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3479
Adds an instance of VariableFlashInfoLib to the platform build as
it is a new library class introduced in MdeModulePkg.
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Julien Grall <julien@xen.org>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Acked-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
---
ArmVirtPkg/ArmVirt.dsc.inc | 1 +
1 file changed, 1 insertion(+)
diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index ba711deac025..988c1eb75529 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -177,6 +177,7 @@ [LibraryClasses.common]
AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLibNull.inf
!endif
VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
+ VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.inf
VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
UefiBootManagerLib|MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
--
2.28.0.windows.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v5 6/8] EmulatorPkg: Add VariableFlashInfoLib
2022-04-26 1:29 [PATCH v5 0/8] Add Variable Flash Info HOB Michael Kubacki
` (4 preceding siblings ...)
2022-04-26 1:29 ` [PATCH v5 5/8] ArmVirtPkg/ArmVirt.dsc.inc: Add VariableFlashInfoLib Michael Kubacki
@ 2022-04-26 1:29 ` Michael Kubacki
2022-04-26 1:29 ` [PATCH v5 7/8] OvmfPkg: " Michael Kubacki
` (2 subsequent siblings)
8 siblings, 0 replies; 26+ messages in thread
From: Michael Kubacki @ 2022-04-26 1:29 UTC (permalink / raw)
To: devel
Cc: Andrew Fish, Ray Ni, Abner Chang, Nickle Wang, Ard Biesheuvel,
Liming Gao
From: Michael Kubacki <michael.kubacki@microsoft.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3479
Adds an instance of VariableFlashInfoLib to the platform build as
it is a new library class introduced in MdeModulePkg.
Cc: Andrew Fish <afish@apple.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Abner Chang <abner.chang@hpe.com>
Cc: Nickle Wang <nickle.wang@hpe.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Acked-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
Reviewed-by: Abner Chang <abner.chang@hpe.com>
---
EmulatorPkg/EmulatorPkg.dsc | 1 +
1 file changed, 1 insertion(+)
diff --git a/EmulatorPkg/EmulatorPkg.dsc b/EmulatorPkg/EmulatorPkg.dsc
index 554c13ddb500..4cf886b9eac7 100644
--- a/EmulatorPkg/EmulatorPkg.dsc
+++ b/EmulatorPkg/EmulatorPkg.dsc
@@ -122,6 +122,7 @@ [LibraryClasses]
VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRuntimeDxe.inf
VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
+ VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.inf
SortLib|MdeModulePkg/Library/BaseSortLib/BaseSortLib.inf
ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf
FileHandleLib|MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.inf
--
2.28.0.windows.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v5 7/8] OvmfPkg: Add VariableFlashInfoLib
2022-04-26 1:29 [PATCH v5 0/8] Add Variable Flash Info HOB Michael Kubacki
` (5 preceding siblings ...)
2022-04-26 1:29 ` [PATCH v5 6/8] EmulatorPkg: " Michael Kubacki
@ 2022-04-26 1:29 ` Michael Kubacki
2022-04-26 2:14 ` [edk2-devel] " Yao, Jiewen
2022-04-26 1:29 ` [PATCH v5 8/8] UefiPayloadPkg: " Michael Kubacki
2022-04-29 13:45 ` [PATCH v5 0/8] Add Variable Flash Info HOB Ard Biesheuvel
8 siblings, 1 reply; 26+ messages in thread
From: Michael Kubacki @ 2022-04-26 1:29 UTC (permalink / raw)
To: devel
Cc: Anthony Perard, Ard Biesheuvel, Brijesh Singh, Erdem Aktas,
Gerd Hoffmann, James Bottomley, Jiewen Yao, Jordan Justen,
Julien Grall, Min Xu, Peter Grehan, Rebecca Cran, Sebastien Boeuf,
Tom Lendacky, Ard Biesheuvel, Liming Gao
From: Michael Kubacki <michael.kubacki@microsoft.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3479
Adds an instance of VariableFlashInfoLib to the platform build as
it is a new library class introduced in MdeModulePkg.
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Julien Grall <julien@xen.org>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Peter Grehan <grehan@freebsd.org>
Cc: Rebecca Cran <rebecca@bsdio.com>
Cc: Sebastien Boeuf <sebastien.boeuf@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Acked-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
Reviewed-by: Rebecca Cran <rebecca@bsdio.com>
---
OvmfPkg/AmdSev/AmdSevX64.dsc | 1 +
OvmfPkg/Bhyve/BhyveX64.dsc | 1 +
OvmfPkg/CloudHv/CloudHvX64.dsc | 1 +
OvmfPkg/IntelTdx/IntelTdxX64.dsc | 1 +
OvmfPkg/Microvm/MicrovmX64.dsc | 1 +
OvmfPkg/OvmfPkgIa32.dsc | 1 +
OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
OvmfPkg/OvmfPkgX64.dsc | 1 +
OvmfPkg/OvmfXen.dsc | 1 +
9 files changed, 9 insertions(+)
diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
index f0700035c116..bead9722eab8 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.dsc
+++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
@@ -196,6 +196,7 @@ [LibraryClasses]
VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
+ VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.inf
!if $(BUILD_SHELL) == TRUE
ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf
diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc
index a8fa4d38ab60..d33728cbe773 100644
--- a/OvmfPkg/Bhyve/BhyveX64.dsc
+++ b/OvmfPkg/Bhyve/BhyveX64.dsc
@@ -207,6 +207,7 @@ [LibraryClasses]
VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
+ VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.inf
#
# Network libraries
diff --git a/OvmfPkg/CloudHv/CloudHvX64.dsc b/OvmfPkg/CloudHv/CloudHvX64.dsc
index d1c85f60c768..92664f319be2 100644
--- a/OvmfPkg/CloudHv/CloudHvX64.dsc
+++ b/OvmfPkg/CloudHv/CloudHvX64.dsc
@@ -217,6 +217,7 @@ [LibraryClasses]
VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
+ VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.inf
#
diff --git a/OvmfPkg/IntelTdx/IntelTdxX64.dsc b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
index 73a6c30096a8..01e0ae0ad40a 100644
--- a/OvmfPkg/IntelTdx/IntelTdxX64.dsc
+++ b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
@@ -185,6 +185,7 @@ [LibraryClasses]
VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
+ VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.inf
ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf
ShellCEntryLib|ShellPkg/Library/UefiShellCEntryLib/UefiShellCEntryLib.inf
diff --git a/OvmfPkg/Microvm/MicrovmX64.dsc b/OvmfPkg/Microvm/MicrovmX64.dsc
index 20c3c9c4d862..f8fc977cb205 100644
--- a/OvmfPkg/Microvm/MicrovmX64.dsc
+++ b/OvmfPkg/Microvm/MicrovmX64.dsc
@@ -207,6 +207,7 @@ [LibraryClasses]
VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
+ VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.inf
#
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index f02b371f7427..892ed6c64cf1 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -214,6 +214,7 @@ [LibraryClasses]
VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
+ VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.inf
#
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index cb68e612bd35..d3a80cb56892 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -218,6 +218,7 @@ [LibraryClasses]
VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
+ VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.inf
#
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 45ffa2dbe35f..c05f345a40e1 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -226,6 +226,7 @@ [LibraryClasses]
VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
+ VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.inf
#
diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
index a1626d06dfc3..6ba4bd729ae7 100644
--- a/OvmfPkg/OvmfXen.dsc
+++ b/OvmfPkg/OvmfXen.dsc
@@ -196,6 +196,7 @@ [LibraryClasses]
VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
+ VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.inf
#
--
2.28.0.windows.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [edk2-devel] [PATCH v5 7/8] OvmfPkg: Add VariableFlashInfoLib
2022-04-26 1:29 ` [PATCH v5 7/8] OvmfPkg: " Michael Kubacki
@ 2022-04-26 2:14 ` Yao, Jiewen
2022-04-26 2:27 ` Michael Kubacki
0 siblings, 1 reply; 26+ messages in thread
From: Yao, Jiewen @ 2022-04-26 2:14 UTC (permalink / raw)
To: devel@edk2.groups.io, mikuback@linux.microsoft.com
Cc: Anthony Perard, Ard Biesheuvel, Brijesh Singh, Aktas, Erdem,
Gerd Hoffmann, James Bottomley, Justen, Jordan L, Julien Grall,
Xu, Min M, Peter Grehan, Rebecca Cran, Boeuf, Sebastien,
Tom Lendacky, Ard Biesheuvel, Gao, Liming
Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael
> Kubacki
> Sent: Tuesday, April 26, 2022 9:29 AM
> To: devel@edk2.groups.io
> Cc: Anthony Perard <anthony.perard@citrix.com>; Ard Biesheuvel
> <ardb+tianocore@kernel.org>; Brijesh Singh <brijesh.singh@amd.com>; Aktas,
> Erdem <erdemaktas@google.com>; Gerd Hoffmann <kraxel@redhat.com>;
> James Bottomley <jejb@linux.ibm.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Justen, Jordan L <jordan.l.justen@intel.com>; Julien Grall <julien@xen.org>; Xu,
> Min M <min.m.xu@intel.com>; Peter Grehan <grehan@freebsd.org>; Rebecca
> Cran <rebecca@bsdio.com>; Boeuf, Sebastien <sebastien.boeuf@intel.com>;
> Tom Lendacky <thomas.lendacky@amd.com>; Ard Biesheuvel
> <ardb@kernel.org>; Gao, Liming <gaoliming@byosoft.com.cn>
> Subject: [edk2-devel] [PATCH v5 7/8] OvmfPkg: Add VariableFlashInfoLib
>
> From: Michael Kubacki <michael.kubacki@microsoft.com>
>
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3479
>
> Adds an instance of VariableFlashInfoLib to the platform build as
> it is a new library class introduced in MdeModulePkg.
>
> Cc: Anthony Perard <anthony.perard@citrix.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Cc: Erdem Aktas <erdemaktas@google.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Julien Grall <julien@xen.org>
> Cc: Min Xu <min.m.xu@intel.com>
> Cc: Peter Grehan <grehan@freebsd.org>
> Cc: Rebecca Cran <rebecca@bsdio.com>
> Cc: Sebastien Boeuf <sebastien.boeuf@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> Acked-by: Ard Biesheuvel <ardb@kernel.org>
> Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
> Reviewed-by: Rebecca Cran <rebecca@bsdio.com>
> ---
> OvmfPkg/AmdSev/AmdSevX64.dsc | 1 +
> OvmfPkg/Bhyve/BhyveX64.dsc | 1 +
> OvmfPkg/CloudHv/CloudHvX64.dsc | 1 +
> OvmfPkg/IntelTdx/IntelTdxX64.dsc | 1 +
> OvmfPkg/Microvm/MicrovmX64.dsc | 1 +
> OvmfPkg/OvmfPkgIa32.dsc | 1 +
> OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
> OvmfPkg/OvmfPkgX64.dsc | 1 +
> OvmfPkg/OvmfXen.dsc | 1 +
> 9 files changed, 9 insertions(+)
>
> diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc
> b/OvmfPkg/AmdSev/AmdSevX64.dsc
> index f0700035c116..bead9722eab8 100644
> --- a/OvmfPkg/AmdSev/AmdSevX64.dsc
> +++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
> @@ -196,6 +196,7 @@ [LibraryClasses]
> VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
>
> VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.in
> f
>
> VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/Variab
> lePolicyHelperLib.inf
> +
> VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVar
> iableFlashInfoLib.inf
>
> !if $(BUILD_SHELL) == TRUE
> ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf
> diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc
> index a8fa4d38ab60..d33728cbe773 100644
> --- a/OvmfPkg/Bhyve/BhyveX64.dsc
> +++ b/OvmfPkg/Bhyve/BhyveX64.dsc
> @@ -207,6 +207,7 @@ [LibraryClasses]
> VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
>
> VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.in
> f
>
> VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/Variab
> lePolicyHelperLib.inf
> +
> VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVar
> iableFlashInfoLib.inf
>
> #
> # Network libraries
> diff --git a/OvmfPkg/CloudHv/CloudHvX64.dsc
> b/OvmfPkg/CloudHv/CloudHvX64.dsc
> index d1c85f60c768..92664f319be2 100644
> --- a/OvmfPkg/CloudHv/CloudHvX64.dsc
> +++ b/OvmfPkg/CloudHv/CloudHvX64.dsc
> @@ -217,6 +217,7 @@ [LibraryClasses]
> VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
>
> VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.in
> f
>
> VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/Variab
> lePolicyHelperLib.inf
> +
> VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVar
> iableFlashInfoLib.inf
>
>
> #
> diff --git a/OvmfPkg/IntelTdx/IntelTdxX64.dsc
> b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
> index 73a6c30096a8..01e0ae0ad40a 100644
> --- a/OvmfPkg/IntelTdx/IntelTdxX64.dsc
> +++ b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
> @@ -185,6 +185,7 @@ [LibraryClasses]
> VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
>
> VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.in
> f
>
> VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/Variab
> lePolicyHelperLib.inf
> +
> VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVar
> iableFlashInfoLib.inf
>
> ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf
> ShellCEntryLib|ShellPkg/Library/UefiShellCEntryLib/UefiShellCEntryLib.inf
> diff --git a/OvmfPkg/Microvm/MicrovmX64.dsc
> b/OvmfPkg/Microvm/MicrovmX64.dsc
> index 20c3c9c4d862..f8fc977cb205 100644
> --- a/OvmfPkg/Microvm/MicrovmX64.dsc
> +++ b/OvmfPkg/Microvm/MicrovmX64.dsc
> @@ -207,6 +207,7 @@ [LibraryClasses]
> VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
>
> VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.in
> f
>
> VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/Variab
> lePolicyHelperLib.inf
> +
> VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVar
> iableFlashInfoLib.inf
>
>
> #
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index f02b371f7427..892ed6c64cf1 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -214,6 +214,7 @@ [LibraryClasses]
> VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
>
> VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.in
> f
>
> VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/Variab
> lePolicyHelperLib.inf
> +
> VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVar
> iableFlashInfoLib.inf
>
>
> #
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index cb68e612bd35..d3a80cb56892 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -218,6 +218,7 @@ [LibraryClasses]
> VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
>
> VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.in
> f
>
> VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/Variab
> lePolicyHelperLib.inf
> +
> VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVar
> iableFlashInfoLib.inf
>
>
> #
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index 45ffa2dbe35f..c05f345a40e1 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -226,6 +226,7 @@ [LibraryClasses]
> VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
>
> VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.in
> f
>
> VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/Variab
> lePolicyHelperLib.inf
> +
> VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVar
> iableFlashInfoLib.inf
>
>
> #
> diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
> index a1626d06dfc3..6ba4bd729ae7 100644
> --- a/OvmfPkg/OvmfXen.dsc
> +++ b/OvmfPkg/OvmfXen.dsc
> @@ -196,6 +196,7 @@ [LibraryClasses]
> VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
>
> VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.in
> f
>
> VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/Variab
> lePolicyHelperLib.inf
> +
> VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVar
> iableFlashInfoLib.inf
>
>
> #
> --
> 2.28.0.windows.1
>
>
>
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#89304): https://edk2.groups.io/g/devel/message/89304
> Mute This Topic: https://groups.io/mt/90699684/1772286
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [jiewen.yao@intel.com]
> -=-=-=-=-=-=
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [edk2-devel] [PATCH v5 7/8] OvmfPkg: Add VariableFlashInfoLib
2022-04-26 2:14 ` [edk2-devel] " Yao, Jiewen
@ 2022-04-26 2:27 ` Michael Kubacki
0 siblings, 0 replies; 26+ messages in thread
From: Michael Kubacki @ 2022-04-26 2:27 UTC (permalink / raw)
To: devel, jiewen.yao
Cc: Anthony Perard, Ard Biesheuvel, Brijesh Singh, Aktas, Erdem,
Gerd Hoffmann, James Bottomley, Justen, Jordan L, Julien Grall,
Xu, Min M, Peter Grehan, Rebecca Cran, Boeuf, Sebastien,
Tom Lendacky, Ard Biesheuvel, Gao, Liming
Thanks Jiewen. I added this R-b (and all others) to the V5 PR here:
https://github.com/tianocore/edk2/pull/2828
Regards,
Michael
On 4/25/2022 10:14 PM, Yao, Jiewen wrote:
> Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
>
>> -----Original Message-----
>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael
>> Kubacki
>> Sent: Tuesday, April 26, 2022 9:29 AM
>> To: devel@edk2.groups.io
>> Cc: Anthony Perard <anthony.perard@citrix.com>; Ard Biesheuvel
>> <ardb+tianocore@kernel.org>; Brijesh Singh <brijesh.singh@amd.com>; Aktas,
>> Erdem <erdemaktas@google.com>; Gerd Hoffmann <kraxel@redhat.com>;
>> James Bottomley <jejb@linux.ibm.com>; Yao, Jiewen <jiewen.yao@intel.com>;
>> Justen, Jordan L <jordan.l.justen@intel.com>; Julien Grall <julien@xen.org>; Xu,
>> Min M <min.m.xu@intel.com>; Peter Grehan <grehan@freebsd.org>; Rebecca
>> Cran <rebecca@bsdio.com>; Boeuf, Sebastien <sebastien.boeuf@intel.com>;
>> Tom Lendacky <thomas.lendacky@amd.com>; Ard Biesheuvel
>> <ardb@kernel.org>; Gao, Liming <gaoliming@byosoft.com.cn>
>> Subject: [edk2-devel] [PATCH v5 7/8] OvmfPkg: Add VariableFlashInfoLib
>>
>> From: Michael Kubacki <michael.kubacki@microsoft.com>
>>
>> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3479
>>
>> Adds an instance of VariableFlashInfoLib to the platform build as
>> it is a new library class introduced in MdeModulePkg.
>>
>> Cc: Anthony Perard <anthony.perard@citrix.com>
>> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
>> Cc: Brijesh Singh <brijesh.singh@amd.com>
>> Cc: Erdem Aktas <erdemaktas@google.com>
>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>> Cc: James Bottomley <jejb@linux.ibm.com>
>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Julien Grall <julien@xen.org>
>> Cc: Min Xu <min.m.xu@intel.com>
>> Cc: Peter Grehan <grehan@freebsd.org>
>> Cc: Rebecca Cran <rebecca@bsdio.com>
>> Cc: Sebastien Boeuf <sebastien.boeuf@intel.com>
>> Cc: Tom Lendacky <thomas.lendacky@amd.com>
>> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
>> Acked-by: Ard Biesheuvel <ardb@kernel.org>
>> Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
>> Reviewed-by: Rebecca Cran <rebecca@bsdio.com>
>> ---
>> OvmfPkg/AmdSev/AmdSevX64.dsc | 1 +
>> OvmfPkg/Bhyve/BhyveX64.dsc | 1 +
>> OvmfPkg/CloudHv/CloudHvX64.dsc | 1 +
>> OvmfPkg/IntelTdx/IntelTdxX64.dsc | 1 +
>> OvmfPkg/Microvm/MicrovmX64.dsc | 1 +
>> OvmfPkg/OvmfPkgIa32.dsc | 1 +
>> OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
>> OvmfPkg/OvmfPkgX64.dsc | 1 +
>> OvmfPkg/OvmfXen.dsc | 1 +
>> 9 files changed, 9 insertions(+)
>>
>> diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc
>> b/OvmfPkg/AmdSev/AmdSevX64.dsc
>> index f0700035c116..bead9722eab8 100644
>> --- a/OvmfPkg/AmdSev/AmdSevX64.dsc
>> +++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
>> @@ -196,6 +196,7 @@ [LibraryClasses]
>> VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
>>
>> VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.in
>> f
>>
>> VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/Variab
>> lePolicyHelperLib.inf
>> +
>> VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVar
>> iableFlashInfoLib.inf
>>
>> !if $(BUILD_SHELL) == TRUE
>> ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf
>> diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc
>> index a8fa4d38ab60..d33728cbe773 100644
>> --- a/OvmfPkg/Bhyve/BhyveX64.dsc
>> +++ b/OvmfPkg/Bhyve/BhyveX64.dsc
>> @@ -207,6 +207,7 @@ [LibraryClasses]
>> VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
>>
>> VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.in
>> f
>>
>> VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/Variab
>> lePolicyHelperLib.inf
>> +
>> VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVar
>> iableFlashInfoLib.inf
>>
>> #
>> # Network libraries
>> diff --git a/OvmfPkg/CloudHv/CloudHvX64.dsc
>> b/OvmfPkg/CloudHv/CloudHvX64.dsc
>> index d1c85f60c768..92664f319be2 100644
>> --- a/OvmfPkg/CloudHv/CloudHvX64.dsc
>> +++ b/OvmfPkg/CloudHv/CloudHvX64.dsc
>> @@ -217,6 +217,7 @@ [LibraryClasses]
>> VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
>>
>> VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.in
>> f
>>
>> VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/Variab
>> lePolicyHelperLib.inf
>> +
>> VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVar
>> iableFlashInfoLib.inf
>>
>>
>> #
>> diff --git a/OvmfPkg/IntelTdx/IntelTdxX64.dsc
>> b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
>> index 73a6c30096a8..01e0ae0ad40a 100644
>> --- a/OvmfPkg/IntelTdx/IntelTdxX64.dsc
>> +++ b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
>> @@ -185,6 +185,7 @@ [LibraryClasses]
>> VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
>>
>> VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.in
>> f
>>
>> VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/Variab
>> lePolicyHelperLib.inf
>> +
>> VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVar
>> iableFlashInfoLib.inf
>>
>> ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf
>> ShellCEntryLib|ShellPkg/Library/UefiShellCEntryLib/UefiShellCEntryLib.inf
>> diff --git a/OvmfPkg/Microvm/MicrovmX64.dsc
>> b/OvmfPkg/Microvm/MicrovmX64.dsc
>> index 20c3c9c4d862..f8fc977cb205 100644
>> --- a/OvmfPkg/Microvm/MicrovmX64.dsc
>> +++ b/OvmfPkg/Microvm/MicrovmX64.dsc
>> @@ -207,6 +207,7 @@ [LibraryClasses]
>> VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
>>
>> VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.in
>> f
>>
>> VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/Variab
>> lePolicyHelperLib.inf
>> +
>> VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVar
>> iableFlashInfoLib.inf
>>
>>
>> #
>> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
>> index f02b371f7427..892ed6c64cf1 100644
>> --- a/OvmfPkg/OvmfPkgIa32.dsc
>> +++ b/OvmfPkg/OvmfPkgIa32.dsc
>> @@ -214,6 +214,7 @@ [LibraryClasses]
>> VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
>>
>> VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.in
>> f
>>
>> VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/Variab
>> lePolicyHelperLib.inf
>> +
>> VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVar
>> iableFlashInfoLib.inf
>>
>>
>> #
>> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
>> index cb68e612bd35..d3a80cb56892 100644
>> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
>> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
>> @@ -218,6 +218,7 @@ [LibraryClasses]
>> VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
>>
>> VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.in
>> f
>>
>> VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/Variab
>> lePolicyHelperLib.inf
>> +
>> VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVar
>> iableFlashInfoLib.inf
>>
>>
>> #
>> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
>> index 45ffa2dbe35f..c05f345a40e1 100644
>> --- a/OvmfPkg/OvmfPkgX64.dsc
>> +++ b/OvmfPkg/OvmfPkgX64.dsc
>> @@ -226,6 +226,7 @@ [LibraryClasses]
>> VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
>>
>> VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.in
>> f
>>
>> VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/Variab
>> lePolicyHelperLib.inf
>> +
>> VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVar
>> iableFlashInfoLib.inf
>>
>>
>> #
>> diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
>> index a1626d06dfc3..6ba4bd729ae7 100644
>> --- a/OvmfPkg/OvmfXen.dsc
>> +++ b/OvmfPkg/OvmfXen.dsc
>> @@ -196,6 +196,7 @@ [LibraryClasses]
>> VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
>>
>> VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.in
>> f
>>
>> VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/Variab
>> lePolicyHelperLib.inf
>> +
>> VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVar
>> iableFlashInfoLib.inf
>>
>>
>> #
>> --
>> 2.28.0.windows.1
>>
>>
>>
>> -=-=-=-=-=-=
>> Groups.io Links: You receive all messages sent to this group.
>> View/Reply Online (#89304): https://edk2.groups.io/g/devel/message/89304
>> Mute This Topic: https://groups.io/mt/90699684/1772286
>> Group Owner: devel+owner@edk2.groups.io
>> Unsubscribe: https://edk2.groups.io/g/devel/unsub [jiewen.yao@intel.com]
>> -=-=-=-=-=-=
>>
>
>
>
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v5 8/8] UefiPayloadPkg: Add VariableFlashInfoLib
2022-04-26 1:29 [PATCH v5 0/8] Add Variable Flash Info HOB Michael Kubacki
` (6 preceding siblings ...)
2022-04-26 1:29 ` [PATCH v5 7/8] OvmfPkg: " Michael Kubacki
@ 2022-04-26 1:29 ` Michael Kubacki
2022-04-29 13:45 ` [PATCH v5 0/8] Add Variable Flash Info HOB Ard Biesheuvel
8 siblings, 0 replies; 26+ messages in thread
From: Michael Kubacki @ 2022-04-26 1:29 UTC (permalink / raw)
To: devel
Cc: Guo Dong, Ray Ni, Maurice Ma, Benjamin You, Sean Rhodes,
Ard Biesheuvel, Liming Gao
From: Michael Kubacki <michael.kubacki@microsoft.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3479
Adds an instance of VariableFlashInfoLib to the platform build as
it is a new library class introduced in MdeModulePkg.
Cc: Guo Dong <guo.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Maurice Ma <maurice.ma@intel.com>
Cc: Benjamin You <benjamin.you@intel.com>
Cc: Sean Rhodes <sean@starlabs.systems>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Acked-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
Reviewed-by: Guo Dong <guo.dong@intel.com>
---
UefiPayloadPkg/UefiPayloadPkg.dsc | 1 +
1 file changed, 1 insertion(+)
diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc
index 17b30589e77c..4d9bbc80c866 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
@@ -272,6 +272,7 @@ [LibraryClasses]
VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
+ VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.inf
VmgExitLib|UefiCpuPkg/Library/VmgExitLibNull/VmgExitLibNull.inf
ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
--
2.28.0.windows.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v5 0/8] Add Variable Flash Info HOB
2022-04-26 1:29 [PATCH v5 0/8] Add Variable Flash Info HOB Michael Kubacki
` (7 preceding siblings ...)
2022-04-26 1:29 ` [PATCH v5 8/8] UefiPayloadPkg: " Michael Kubacki
@ 2022-04-29 13:45 ` Ard Biesheuvel
2022-04-29 15:48 ` Michael Kubacki
8 siblings, 1 reply; 26+ messages in thread
From: Ard Biesheuvel @ 2022-04-29 13:45 UTC (permalink / raw)
To: Michael Kubacki
Cc: edk2-devel-groups-io, Abner Chang, Andrew Fish, Anthony Perard,
Ard Biesheuvel, Benjamin You, Brijesh Singh, Erdem Aktas,
Gerd Hoffmann, Guo Dong, Hao A Wu, James Bottomley, Jian J Wang,
Jiewen Yao, Jordan Justen, Julien Grall, Leif Lindholm,
Liming Gao, Maurice Ma, Min Xu, Nickle Wang, Peter Grehan, Ray Ni,
Rebecca Cran, Sami Mujawar, Sean Rhodes, Sebastien Boeuf,
Tom Lendacky
On Tue, 26 Apr 2022 at 03:29, <mikuback@linux.microsoft.com> wrote:
>
> From: Michael Kubacki <michael.kubacki@microsoft.com>
>
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3479
>
> Platforms: This series introduces a new library class called
> VariableFlashInfoLib. Platforms using the variable modules will
> need to specify these library classes. See the patches at the
> end of this series for examples of the change needed in the
> platform DSC file. I have attempted to update all open-source
> platforms in edk2 and edk2-platforms in this series and
> https://edk2.groups.io/g/devel/message/89148.
>
I will make the same remark here as I made in response to the
edk2-platforms series:
Could we please consider introducing a way to define the default
resolution of a library class? That way, all this churn and
unnecessary breakage would not be necessary, as defining a resolution
would only be necessary when deviating from the default. In fact, we
might be able to clean up some .DSCs considerably by defining defaults
for library classes that only have one (or very few) implementations.
> The UEFI variable drivers such as VariableRuntimeDxe, VariableSmm,
> VariableStandaloneMm, etc. (and their dependent protocol/library
> stack), typically acquire UEFI variable store flash information
> with PCDs declared in MdeModulePkg.
>
> For example:
> [Pcd]
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
>
> These PCDs work as-is in the StandaloneMm driver if they are not
> dynamic such as Dynamic or DynamicEx because PCD services are not
> readily available in the Standalone MM environment. Platforms that
> use Standalone MM today, must define these PCDs as FixedAtBuild in
> their platform build. However, the PCDs do allow platforms to treat
> the PCDs as Dynamic/DynamicEx and being able to support that is
> currently a gap for Standalone MM.
>
> This patch series introduces a HOB that can be produced by the
> platform to provide the same information. The HOB list is
> available to Standalone MM.
>
> The PCD declarations are left as-is in MdeModulePkg for backward
> compatibility. This means unless a platform wants to use the HOB,
> their code will continue to work with no change (they do not need
> to produce the HOB). Only if the HOB is found, is its value used
> instead of the PCDs.
>
> Due to the large number of consumers of this information, access
> to the base address and size values is abstracted in a new library
> class (as requested in the v1 series) called VariableFlashInfoLib.
>
> The API of VariableFlashInfoLib does not bind the underlying data
> structure to the information returned to library users to allow
> flexibility in the library implementation in the future.
>
> V5 changes.
> 1. Made GetVariableFlashInfoFromHob() in BaseVariableFlashInfoLib.c
> STATIC.
> 2. Added a section in commit v5 [3/8] to explicitly state that the
> commit introduces a dependency that requires a change in platform
> build. Please see patches v5 [5/8] - v5 [8/8] for examples of
> how to integrate this change (add VariableFlashInfoLib library
> to DSC file).
>
> V4 changes:
> 1. Add a UINT32 "Reserved" field to VARIABLE_FLASH_INFO.
> 2. Add a descriptive comment to VariableFlashInfo.h to explain
> HOB usage.
>
> V3 changes:
> 1. To better clarify usage, renamed the members
> "NvStorageBaseAddress" and "NvStorageLength" in
> "VARIABLE_FLASH_INFO" to "NvVariableBaseAddress" and
> "NvVariableLength".
> 2. Added description comments to the fields in "VARIABLE_FLASH_INFO".
>
> V2 changes:
> 1. Abstracted flash info data access with VariableFlashInfoLib.
> 2. Updated package builds in the repo that build the variable and
> FTW drivers to include VariableFlashInfoLib.
> 3. Removed a redundant variable assignment in VariableSmm.c.
> 4. Updated comments in FtwMisc.c and FaultTolerantWritePei.c to
> indicate driver assumption is UINTN (not UINT32)
> 5. Added a version field to the VARIABLE_FLASH_INFO structure.
>
> Cc: Abner Chang <abner.chang@hpe.com>
> Cc: Andrew Fish <afish@apple.com>
> Cc: Anthony Perard <anthony.perard@citrix.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Benjamin You <benjamin.you@intel.com>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Cc: Erdem Aktas <erdemaktas@google.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Guo Dong <guo.dong@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Julien Grall <julien@xen.org>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Maurice Ma <maurice.ma@intel.com>
> Cc: Min Xu <min.m.xu@intel.com>
> Cc: Nickle Wang <nickle.wang@hpe.com>
> Cc: Peter Grehan <grehan@freebsd.org>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rebecca Cran <rebecca@bsdio.com>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Sean Rhodes <sean@starlabs.systems>
> Cc: Sebastien Boeuf <sebastien.boeuf@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
>
> Michael Kubacki (8):
> MdeModulePkg: Add Variable Flash Info HOB
> MdeModulePkg/VariableFlashInfoLib: Add initial library
> MdeModulePkg/Variable: Consume Variable Flash Info
> MdeModulePkg/FaultTolerantWrite: Consume Variable Flash Info
> ArmVirtPkg/ArmVirt.dsc.inc: Add VariableFlashInfoLib
> EmulatorPkg: Add VariableFlashInfoLib
> OvmfPkg: Add VariableFlashInfoLib
> UefiPayloadPkg: Add VariableFlashInfoLib
>
> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.c | 179 ++++++++++++++++++++
> MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c | 41 +++--
> MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c | 7 +-
> MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.c | 28 +--
> MdeModulePkg/Universal/Variable/Pei/Variable.c | 14 +-
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c | 16 +-
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.c | 14 +-
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c | 17 +-
> ArmVirtPkg/ArmVirt.dsc.inc | 1 +
> EmulatorPkg/EmulatorPkg.dsc | 1 +
> MdeModulePkg/Include/Guid/VariableFlashInfo.h | 111 ++++++++++++
> MdeModulePkg/Include/Library/VariableFlashInfoLib.h | 68 ++++++++
> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.inf | 48 ++++++
> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.uni | 12 ++
> MdeModulePkg/MdeModulePkg.dec | 8 +
> MdeModulePkg/MdeModulePkg.dsc | 2 +
> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h | 7 +-
> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf | 10 +-
> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf | 10 +-
> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf | 10 +-
> MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf | 10 +-
> MdeModulePkg/Universal/Variable/Pei/Variable.h | 2 +
> MdeModulePkg/Universal/Variable/Pei/VariablePei.inf | 5 +-
> MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h | 7 +-
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf | 5 +-
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf | 5 +-
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf | 5 +-
> OvmfPkg/AmdSev/AmdSevX64.dsc | 1 +
> OvmfPkg/Bhyve/BhyveX64.dsc | 1 +
> OvmfPkg/CloudHv/CloudHvX64.dsc | 1 +
> OvmfPkg/IntelTdx/IntelTdxX64.dsc | 1 +
> OvmfPkg/Microvm/MicrovmX64.dsc | 1 +
> OvmfPkg/OvmfPkgIa32.dsc | 1 +
> OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
> OvmfPkg/OvmfPkgX64.dsc | 1 +
> OvmfPkg/OvmfXen.dsc | 1 +
> UefiPayloadPkg/UefiPayloadPkg.dsc | 1 +
> 37 files changed, 559 insertions(+), 94 deletions(-)
> create mode 100644 MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.c
> create mode 100644 MdeModulePkg/Include/Guid/VariableFlashInfo.h
> create mode 100644 MdeModulePkg/Include/Library/VariableFlashInfoLib.h
> create mode 100644 MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.inf
> create mode 100644 MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.uni
>
> --
> 2.28.0.windows.1
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v5 0/8] Add Variable Flash Info HOB
2022-04-29 13:45 ` [PATCH v5 0/8] Add Variable Flash Info HOB Ard Biesheuvel
@ 2022-04-29 15:48 ` Michael Kubacki
2022-05-05 1:27 ` 回复: [edk2-devel] " gaoliming
0 siblings, 1 reply; 26+ messages in thread
From: Michael Kubacki @ 2022-04-29 15:48 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: edk2-devel-groups-io, Abner Chang, Andrew Fish, Anthony Perard,
Ard Biesheuvel, Benjamin You, Brijesh Singh, Erdem Aktas,
Gerd Hoffmann, Guo Dong, Hao A Wu, James Bottomley, Jian J Wang,
Jiewen Yao, Jordan Justen, Julien Grall, Leif Lindholm,
Liming Gao, Maurice Ma, Min Xu, Nickle Wang, Peter Grehan, Ray Ni,
Rebecca Cran, Sami Mujawar, Sean Rhodes, Sebastien Boeuf,
Tom Lendacky
I agree that would be a useful tool and in the case of changes such as
this that provide backward compatibility with existing functionality,
particularly helpful.
Some packages such as MdePkg
(https://github.com/tianocore/edk2/blob/master/MdePkg/MdeLibs.dsc.inc)
and NetworkPkg
(https://github.com/tianocore/edk2/blob/master/NetworkPkg/NetworkComponents.dsc.inc)
provide DSC files that a platform can override if necessary.
However, this does not exist for all edk2 packages. I did not introduce
such a file in MdeModulePkg because I believe that is an independent
package design decision outside the scope of this series and, if that
change was made, it should include libraries other than just this
instance. That would lead to additional churn and a larger platform
integration debate, important to that topic, but separate from the
current state this contribution is based against.
While includes be helpful, it can encourage platform owners to ignore
potential creep in functionality they should be aware of.
For example, the DSC update is mostly being given to platforms to fix
their immediate build problem. But, a platform owner might also choose
to update their FVB driver to use this interface to get flash
information as opposed to directly using PCDs as many do today. That's a
decision they need to evaluate and make but they should be aware of the
interface and make that decision. By directly reviewing/integrating the
change for their platform, they are more explicitly made aware of this
new interface to form that decision.
Also, when many include files get involved, platform build complexity
and developer frustration can increase due to nesting and order of
include files. Values (library classes, PCDs, etc.) can be overridden
more than once. Ultimately, this is technically manageable by utilizing
build reports and understanding the EDK II build output in more detail.
Again, I think this conversation is useful but requires much more time
to address questions such as the following:
1. Should a different mechanism for default library classes be introduced?
2. Should all packages in edk2 provide such an include file? If so, does
it only provide the DSC file (like MdePkg) or other files (like
NetworkPkg which includes FDF)?
3. Which library classes for a given package should be given default
instances?
4. How would each platform package maintainer like to maintain their
platform build?
Example: Would MinPlatformPkg prefer to keep its own "core" include
files
(https://github.com/tianocore/edk2-platforms/tree/master/Platform/Intel/MinPlatformPkg/Include)
or reconcile them with include files (possibly introducing an additional
layer of nesting)? Would others prefer not to use includes at all to
keep a flat, simpler file?
How would someone updating the platform code due to an edk2 change be
aware of this per-platform policy?
To my understanding, the answers to these today are:
1. No
2. No / decided on per-change basis
3. Unknown
4. Unknown
(2) adds friction to the edk2 contribution process as expectation is
unclear.
(3) adds churn into the platform integration process as the integration
process for existing code is subject to change over time (e.g. realize
library class is now in DSC and remove from platform DSC to use include
instance).
(4) adds friction to the edk2 contribution process as expectation is
unclear. Also, somewhat error prone. There's also a level of due
diligence needed to confirm if a platform that already has an include is
overriding that in another DSC file. If so, is that still the correct
behavior or should it be modified.
Regards,
Michael
On 4/29/2022 9:45 AM, Ard Biesheuvel wrote:
> On Tue, 26 Apr 2022 at 03:29, <mikuback@linux.microsoft.com> wrote:
>>
>> From: Michael Kubacki <michael.kubacki@microsoft.com>
>>
>> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3479
>>
>> Platforms: This series introduces a new library class called
>> VariableFlashInfoLib. Platforms using the variable modules will
>> need to specify these library classes. See the patches at the
>> end of this series for examples of the change needed in the
>> platform DSC file. I have attempted to update all open-source
>> platforms in edk2 and edk2-platforms in this series and
>> https://edk2.groups.io/g/devel/message/89148.
>>
>
> I will make the same remark here as I made in response to the
> edk2-platforms series:
>
> Could we please consider introducing a way to define the default
> resolution of a library class? That way, all this churn and
> unnecessary breakage would not be necessary, as defining a resolution
> would only be necessary when deviating from the default. In fact, we
> might be able to clean up some .DSCs considerably by defining defaults
> for library classes that only have one (or very few) implementations.
>
>
>> The UEFI variable drivers such as VariableRuntimeDxe, VariableSmm,
>> VariableStandaloneMm, etc. (and their dependent protocol/library
>> stack), typically acquire UEFI variable store flash information
>> with PCDs declared in MdeModulePkg.
>>
>> For example:
>> [Pcd]
>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase
>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64
>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
>>
>> These PCDs work as-is in the StandaloneMm driver if they are not
>> dynamic such as Dynamic or DynamicEx because PCD services are not
>> readily available in the Standalone MM environment. Platforms that
>> use Standalone MM today, must define these PCDs as FixedAtBuild in
>> their platform build. However, the PCDs do allow platforms to treat
>> the PCDs as Dynamic/DynamicEx and being able to support that is
>> currently a gap for Standalone MM.
>>
>> This patch series introduces a HOB that can be produced by the
>> platform to provide the same information. The HOB list is
>> available to Standalone MM.
>>
>> The PCD declarations are left as-is in MdeModulePkg for backward
>> compatibility. This means unless a platform wants to use the HOB,
>> their code will continue to work with no change (they do not need
>> to produce the HOB). Only if the HOB is found, is its value used
>> instead of the PCDs.
>>
>> Due to the large number of consumers of this information, access
>> to the base address and size values is abstracted in a new library
>> class (as requested in the v1 series) called VariableFlashInfoLib.
>>
>> The API of VariableFlashInfoLib does not bind the underlying data
>> structure to the information returned to library users to allow
>> flexibility in the library implementation in the future.
>>
>> V5 changes.
>> 1. Made GetVariableFlashInfoFromHob() in BaseVariableFlashInfoLib.c
>> STATIC.
>> 2. Added a section in commit v5 [3/8] to explicitly state that the
>> commit introduces a dependency that requires a change in platform
>> build. Please see patches v5 [5/8] - v5 [8/8] for examples of
>> how to integrate this change (add VariableFlashInfoLib library
>> to DSC file).
>>
>> V4 changes:
>> 1. Add a UINT32 "Reserved" field to VARIABLE_FLASH_INFO.
>> 2. Add a descriptive comment to VariableFlashInfo.h to explain
>> HOB usage.
>>
>> V3 changes:
>> 1. To better clarify usage, renamed the members
>> "NvStorageBaseAddress" and "NvStorageLength" in
>> "VARIABLE_FLASH_INFO" to "NvVariableBaseAddress" and
>> "NvVariableLength".
>> 2. Added description comments to the fields in "VARIABLE_FLASH_INFO".
>>
>> V2 changes:
>> 1. Abstracted flash info data access with VariableFlashInfoLib.
>> 2. Updated package builds in the repo that build the variable and
>> FTW drivers to include VariableFlashInfoLib.
>> 3. Removed a redundant variable assignment in VariableSmm.c.
>> 4. Updated comments in FtwMisc.c and FaultTolerantWritePei.c to
>> indicate driver assumption is UINTN (not UINT32)
>> 5. Added a version field to the VARIABLE_FLASH_INFO structure.
>>
>> Cc: Abner Chang <abner.chang@hpe.com>
>> Cc: Andrew Fish <afish@apple.com>
>> Cc: Anthony Perard <anthony.perard@citrix.com>
>> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
>> Cc: Benjamin You <benjamin.you@intel.com>
>> Cc: Brijesh Singh <brijesh.singh@amd.com>
>> Cc: Erdem Aktas <erdemaktas@google.com>
>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>> Cc: Guo Dong <guo.dong@intel.com>
>> Cc: Hao A Wu <hao.a.wu@intel.com>
>> Cc: James Bottomley <jejb@linux.ibm.com>
>> Cc: Jian J Wang <jian.j.wang@intel.com>
>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Julien Grall <julien@xen.org>
>> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
>> Cc: Liming Gao <gaoliming@byosoft.com.cn>
>> Cc: Maurice Ma <maurice.ma@intel.com>
>> Cc: Min Xu <min.m.xu@intel.com>
>> Cc: Nickle Wang <nickle.wang@hpe.com>
>> Cc: Peter Grehan <grehan@freebsd.org>
>> Cc: Ray Ni <ray.ni@intel.com>
>> Cc: Rebecca Cran <rebecca@bsdio.com>
>> Cc: Sami Mujawar <sami.mujawar@arm.com>
>> Cc: Sean Rhodes <sean@starlabs.systems>
>> Cc: Sebastien Boeuf <sebastien.boeuf@intel.com>
>> Cc: Tom Lendacky <thomas.lendacky@amd.com>
>> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
>>
>> Michael Kubacki (8):
>> MdeModulePkg: Add Variable Flash Info HOB
>> MdeModulePkg/VariableFlashInfoLib: Add initial library
>> MdeModulePkg/Variable: Consume Variable Flash Info
>> MdeModulePkg/FaultTolerantWrite: Consume Variable Flash Info
>> ArmVirtPkg/ArmVirt.dsc.inc: Add VariableFlashInfoLib
>> EmulatorPkg: Add VariableFlashInfoLib
>> OvmfPkg: Add VariableFlashInfoLib
>> UefiPayloadPkg: Add VariableFlashInfoLib
>>
>> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.c | 179 ++++++++++++++++++++
>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c | 41 +++--
>> MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c | 7 +-
>> MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.c | 28 +--
>> MdeModulePkg/Universal/Variable/Pei/Variable.c | 14 +-
>> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c | 16 +-
>> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.c | 14 +-
>> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c | 17 +-
>> ArmVirtPkg/ArmVirt.dsc.inc | 1 +
>> EmulatorPkg/EmulatorPkg.dsc | 1 +
>> MdeModulePkg/Include/Guid/VariableFlashInfo.h | 111 ++++++++++++
>> MdeModulePkg/Include/Library/VariableFlashInfoLib.h | 68 ++++++++
>> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.inf | 48 ++++++
>> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.uni | 12 ++
>> MdeModulePkg/MdeModulePkg.dec | 8 +
>> MdeModulePkg/MdeModulePkg.dsc | 2 +
>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h | 7 +-
>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf | 10 +-
>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf | 10 +-
>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf | 10 +-
>> MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf | 10 +-
>> MdeModulePkg/Universal/Variable/Pei/Variable.h | 2 +
>> MdeModulePkg/Universal/Variable/Pei/VariablePei.inf | 5 +-
>> MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h | 7 +-
>> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf | 5 +-
>> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf | 5 +-
>> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf | 5 +-
>> OvmfPkg/AmdSev/AmdSevX64.dsc | 1 +
>> OvmfPkg/Bhyve/BhyveX64.dsc | 1 +
>> OvmfPkg/CloudHv/CloudHvX64.dsc | 1 +
>> OvmfPkg/IntelTdx/IntelTdxX64.dsc | 1 +
>> OvmfPkg/Microvm/MicrovmX64.dsc | 1 +
>> OvmfPkg/OvmfPkgIa32.dsc | 1 +
>> OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
>> OvmfPkg/OvmfPkgX64.dsc | 1 +
>> OvmfPkg/OvmfXen.dsc | 1 +
>> UefiPayloadPkg/UefiPayloadPkg.dsc | 1 +
>> 37 files changed, 559 insertions(+), 94 deletions(-)
>> create mode 100644 MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.c
>> create mode 100644 MdeModulePkg/Include/Guid/VariableFlashInfo.h
>> create mode 100644 MdeModulePkg/Include/Library/VariableFlashInfoLib.h
>> create mode 100644 MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.inf
>> create mode 100644 MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.uni
>>
>> --
>> 2.28.0.windows.1
>>
^ permalink raw reply [flat|nested] 26+ messages in thread
* 回复: [edk2-devel] [PATCH v5 0/8] Add Variable Flash Info HOB
2022-04-29 15:48 ` Michael Kubacki
@ 2022-05-05 1:27 ` gaoliming
2022-05-06 1:52 ` Michael Kubacki
0 siblings, 1 reply; 26+ messages in thread
From: gaoliming @ 2022-05-05 1:27 UTC (permalink / raw)
To: devel, mikuback, 'Ard Biesheuvel'
Cc: 'Abner Chang', 'Andrew Fish',
'Anthony Perard', 'Ard Biesheuvel',
'Benjamin You', 'Brijesh Singh',
'Erdem Aktas', 'Gerd Hoffmann',
'Guo Dong', 'Hao A Wu', 'James Bottomley',
'Jian J Wang', 'Jiewen Yao',
'Jordan Justen', 'Julien Grall',
'Leif Lindholm', 'Maurice Ma', 'Min Xu',
'Nickle Wang', 'Peter Grehan', 'Ray Ni',
'Rebecca Cran', 'Sami Mujawar',
'Sean Rhodes', 'Sebastien Boeuf',
'Tom Lendacky'
Michael:
I would suggest to reuse MdePkg/MdeLibs.dsc.inc to list the library and PCD from the edk2 core packages, such as MdePkg, MdeModulePkg, CryptoPkg, SecurirtyPkg and so on. Those packages are required by every platforms. They can't be separated. So, I think MdePkg/MdeLibs.dsc.inc is for edk2 core packages, not only for MdePkg.
Thanks
Liming
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Michael
> Kubacki
> 发送时间: 2022年4月29日 23:48
> 收件人: Ard Biesheuvel <ardb@kernel.org>
> 抄送: edk2-devel-groups-io <devel@edk2.groups.io>; Abner Chang
> <abner.chang@hpe.com>; Andrew Fish <afish@apple.com>; Anthony Perard
> <anthony.perard@citrix.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>;
> Benjamin You <benjamin.you@intel.com>; Brijesh Singh
> <brijesh.singh@amd.com>; Erdem Aktas <erdemaktas@google.com>; Gerd
> Hoffmann <kraxel@redhat.com>; Guo Dong <guo.dong@intel.com>; Hao A
> Wu <hao.a.wu@intel.com>; James Bottomley <jejb@linux.ibm.com>; Jian J
> Wang <jian.j.wang@intel.com>; Jiewen Yao <jiewen.yao@intel.com>; Jordan
> Justen <jordan.l.justen@intel.com>; Julien Grall <julien@xen.org>; Leif
> Lindholm <quic_llindhol@quicinc.com>; Liming Gao
> <gaoliming@byosoft.com.cn>; Maurice Ma <maurice.ma@intel.com>; Min Xu
> <min.m.xu@intel.com>; Nickle Wang <nickle.wang@hpe.com>; Peter Grehan
> <grehan@freebsd.org>; Ray Ni <ray.ni@intel.com>; Rebecca Cran
> <rebecca@bsdio.com>; Sami Mujawar <sami.mujawar@arm.com>; Sean
> Rhodes <sean@starlabs.systems>; Sebastien Boeuf
> <sebastien.boeuf@intel.com>; Tom Lendacky <thomas.lendacky@amd.com>
> 主题: Re: [edk2-devel] [PATCH v5 0/8] Add Variable Flash Info HOB
>
> I agree that would be a useful tool and in the case of changes such as
> this that provide backward compatibility with existing functionality,
> particularly helpful.
>
> Some packages such as MdePkg
> (https://github.com/tianocore/edk2/blob/master/MdePkg/MdeLibs.dsc.inc)
> and NetworkPkg
> (https://github.com/tianocore/edk2/blob/master/NetworkPkg/NetworkCom
> ponents.dsc.inc)
> provide DSC files that a platform can override if necessary.
>
> However, this does not exist for all edk2 packages. I did not introduce
> such a file in MdeModulePkg because I believe that is an independent
> package design decision outside the scope of this series and, if that
> change was made, it should include libraries other than just this
> instance. That would lead to additional churn and a larger platform
> integration debate, important to that topic, but separate from the
> current state this contribution is based against.
>
> While includes be helpful, it can encourage platform owners to ignore
> potential creep in functionality they should be aware of.
>
> For example, the DSC update is mostly being given to platforms to fix
> their immediate build problem. But, a platform owner might also choose
> to update their FVB driver to use this interface to get flash
> information as opposed to directly using PCDs as many do today. That's a
> decision they need to evaluate and make but they should be aware of the
> interface and make that decision. By directly reviewing/integrating the
> change for their platform, they are more explicitly made aware of this
> new interface to form that decision.
>
> Also, when many include files get involved, platform build complexity
> and developer frustration can increase due to nesting and order of
> include files. Values (library classes, PCDs, etc.) can be overridden
> more than once. Ultimately, this is technically manageable by utilizing
> build reports and understanding the EDK II build output in more detail.
>
> Again, I think this conversation is useful but requires much more time
> to address questions such as the following:
>
> 1. Should a different mechanism for default library classes be introduced?
>
> 2. Should all packages in edk2 provide such an include file? If so, does
> it only provide the DSC file (like MdePkg) or other files (like
> NetworkPkg which includes FDF)?
>
> 3. Which library classes for a given package should be given default
> instances?
>
> 4. How would each platform package maintainer like to maintain their
> platform build?
>
> Example: Would MinPlatformPkg prefer to keep its own "core" include
> files
> (https://github.com/tianocore/edk2-platforms/tree/master/Platform/Intel/Mi
> nPlatformPkg/Include)
> or reconcile them with include files (possibly introducing an additional
> layer of nesting)? Would others prefer not to use includes at all to
> keep a flat, simpler file?
>
> How would someone updating the platform code due to an edk2 change be
> aware of this per-platform policy?
>
> To my understanding, the answers to these today are:
>
> 1. No
> 2. No / decided on per-change basis
> 3. Unknown
> 4. Unknown
>
> (2) adds friction to the edk2 contribution process as expectation is
> unclear.
>
> (3) adds churn into the platform integration process as the integration
> process for existing code is subject to change over time (e.g. realize
> library class is now in DSC and remove from platform DSC to use include
> instance).
>
> (4) adds friction to the edk2 contribution process as expectation is
> unclear. Also, somewhat error prone. There's also a level of due
> diligence needed to confirm if a platform that already has an include is
> overriding that in another DSC file. If so, is that still the correct
> behavior or should it be modified.
>
> Regards,
> Michael
>
> On 4/29/2022 9:45 AM, Ard Biesheuvel wrote:
> > On Tue, 26 Apr 2022 at 03:29, <mikuback@linux.microsoft.com> wrote:
> >>
> >> From: Michael Kubacki <michael.kubacki@microsoft.com>
> >>
> >> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3479
> >>
> >> Platforms: This series introduces a new library class called
> >> VariableFlashInfoLib. Platforms using the variable modules will
> >> need to specify these library classes. See the patches at the
> >> end of this series for examples of the change needed in the
> >> platform DSC file. I have attempted to update all open-source
> >> platforms in edk2 and edk2-platforms in this series and
> >> https://edk2.groups.io/g/devel/message/89148.
> >>
> >
> > I will make the same remark here as I made in response to the
> > edk2-platforms series:
> >
> > Could we please consider introducing a way to define the default
> > resolution of a library class? That way, all this churn and
> > unnecessary breakage would not be necessary, as defining a resolution
> > would only be necessary when deviating from the default. In fact, we
> > might be able to clean up some .DSCs considerably by defining defaults
> > for library classes that only have one (or very few) implementations.
> >
> >
> >> The UEFI variable drivers such as VariableRuntimeDxe, VariableSmm,
> >> VariableStandaloneMm, etc. (and their dependent protocol/library
> >> stack), typically acquire UEFI variable store flash information
> >> with PCDs declared in MdeModulePkg.
> >>
> >> For example:
> >> [Pcd]
> >> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase
> >>
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64
> >> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
> >>
> >> These PCDs work as-is in the StandaloneMm driver if they are not
> >> dynamic such as Dynamic or DynamicEx because PCD services are not
> >> readily available in the Standalone MM environment. Platforms that
> >> use Standalone MM today, must define these PCDs as FixedAtBuild in
> >> their platform build. However, the PCDs do allow platforms to treat
> >> the PCDs as Dynamic/DynamicEx and being able to support that is
> >> currently a gap for Standalone MM.
> >>
> >> This patch series introduces a HOB that can be produced by the
> >> platform to provide the same information. The HOB list is
> >> available to Standalone MM.
> >>
> >> The PCD declarations are left as-is in MdeModulePkg for backward
> >> compatibility. This means unless a platform wants to use the HOB,
> >> their code will continue to work with no change (they do not need
> >> to produce the HOB). Only if the HOB is found, is its value used
> >> instead of the PCDs.
> >>
> >> Due to the large number of consumers of this information, access
> >> to the base address and size values is abstracted in a new library
> >> class (as requested in the v1 series) called VariableFlashInfoLib.
> >>
> >> The API of VariableFlashInfoLib does not bind the underlying data
> >> structure to the information returned to library users to allow
> >> flexibility in the library implementation in the future.
> >>
> >> V5 changes.
> >> 1. Made GetVariableFlashInfoFromHob() in BaseVariableFlashInfoLib.c
> >> STATIC.
> >> 2. Added a section in commit v5 [3/8] to explicitly state that the
> >> commit introduces a dependency that requires a change in platform
> >> build. Please see patches v5 [5/8] - v5 [8/8] for examples of
> >> how to integrate this change (add VariableFlashInfoLib library
> >> to DSC file).
> >>
> >> V4 changes:
> >> 1. Add a UINT32 "Reserved" field to VARIABLE_FLASH_INFO.
> >> 2. Add a descriptive comment to VariableFlashInfo.h to explain
> >> HOB usage.
> >>
> >> V3 changes:
> >> 1. To better clarify usage, renamed the members
> >> "NvStorageBaseAddress" and "NvStorageLength" in
> >> "VARIABLE_FLASH_INFO" to "NvVariableBaseAddress" and
> >> "NvVariableLength".
> >> 2. Added description comments to the fields in "VARIABLE_FLASH_INFO".
> >>
> >> V2 changes:
> >> 1. Abstracted flash info data access with VariableFlashInfoLib.
> >> 2. Updated package builds in the repo that build the variable and
> >> FTW drivers to include VariableFlashInfoLib.
> >> 3. Removed a redundant variable assignment in VariableSmm.c.
> >> 4. Updated comments in FtwMisc.c and FaultTolerantWritePei.c to
> >> indicate driver assumption is UINTN (not UINT32)
> >> 5. Added a version field to the VARIABLE_FLASH_INFO structure.
> >>
> >> Cc: Abner Chang <abner.chang@hpe.com>
> >> Cc: Andrew Fish <afish@apple.com>
> >> Cc: Anthony Perard <anthony.perard@citrix.com>
> >> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> >> Cc: Benjamin You <benjamin.you@intel.com>
> >> Cc: Brijesh Singh <brijesh.singh@amd.com>
> >> Cc: Erdem Aktas <erdemaktas@google.com>
> >> Cc: Gerd Hoffmann <kraxel@redhat.com>
> >> Cc: Guo Dong <guo.dong@intel.com>
> >> Cc: Hao A Wu <hao.a.wu@intel.com>
> >> Cc: James Bottomley <jejb@linux.ibm.com>
> >> Cc: Jian J Wang <jian.j.wang@intel.com>
> >> Cc: Jiewen Yao <jiewen.yao@intel.com>
> >> Cc: Jordan Justen <jordan.l.justen@intel.com>
> >> Cc: Julien Grall <julien@xen.org>
> >> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> >> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> >> Cc: Maurice Ma <maurice.ma@intel.com>
> >> Cc: Min Xu <min.m.xu@intel.com>
> >> Cc: Nickle Wang <nickle.wang@hpe.com>
> >> Cc: Peter Grehan <grehan@freebsd.org>
> >> Cc: Ray Ni <ray.ni@intel.com>
> >> Cc: Rebecca Cran <rebecca@bsdio.com>
> >> Cc: Sami Mujawar <sami.mujawar@arm.com>
> >> Cc: Sean Rhodes <sean@starlabs.systems>
> >> Cc: Sebastien Boeuf <sebastien.boeuf@intel.com>
> >> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> >> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> >>
> >> Michael Kubacki (8):
> >> MdeModulePkg: Add Variable Flash Info HOB
> >> MdeModulePkg/VariableFlashInfoLib: Add initial library
> >> MdeModulePkg/Variable: Consume Variable Flash Info
> >> MdeModulePkg/FaultTolerantWrite: Consume Variable Flash Info
> >> ArmVirtPkg/ArmVirt.dsc.inc: Add VariableFlashInfoLib
> >> EmulatorPkg: Add VariableFlashInfoLib
> >> OvmfPkg: Add VariableFlashInfoLib
> >> UefiPayloadPkg: Add VariableFlashInfoLib
> >>
> >>
> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.c
> | 179 ++++++++++++++++++++
> >> MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c
> | 41 +++--
> >>
> MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c
> | 7 +-
> >>
> MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.c
> | 28 +--
> >> MdeModulePkg/Universal/Variable/Pei/Variable.c
> | 14 +-
> >> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
> | 16 +-
> >> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.c
> | 14 +-
> >> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> | 17 +-
> >> ArmVirtPkg/ArmVirt.dsc.inc
> | 1 +
> >> EmulatorPkg/EmulatorPkg.dsc
> | 1 +
> >> MdeModulePkg/Include/Guid/VariableFlashInfo.h
> | 111 ++++++++++++
> >> MdeModulePkg/Include/Library/VariableFlashInfoLib.h
> | 68 ++++++++
> >>
> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.i
> nf | 48 ++++++
> >>
> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.u
> ni | 12 ++
> >> MdeModulePkg/MdeModulePkg.dec
> | 8 +
> >> MdeModulePkg/MdeModulePkg.dsc
> | 2 +
> >> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h
> | 7 +-
> >>
> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
> | 10 +-
> >>
> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf
> | 10 +-
> >>
> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal
> oneMm.inf | 10 +-
> >>
> MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
> | 10 +-
> >> MdeModulePkg/Universal/Variable/Pei/Variable.h
> | 2 +
> >> MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
> | 5 +-
> >> MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
> | 7 +-
> >>
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> | 5 +-
> >> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> | 5 +-
> >>
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
> | 5 +-
> >> OvmfPkg/AmdSev/AmdSevX64.dsc
> | 1 +
> >> OvmfPkg/Bhyve/BhyveX64.dsc
> | 1 +
> >> OvmfPkg/CloudHv/CloudHvX64.dsc
> | 1 +
> >> OvmfPkg/IntelTdx/IntelTdxX64.dsc
> | 1 +
> >> OvmfPkg/Microvm/MicrovmX64.dsc
> | 1 +
> >> OvmfPkg/OvmfPkgIa32.dsc
> | 1 +
> >> OvmfPkg/OvmfPkgIa32X64.dsc
> | 1 +
> >> OvmfPkg/OvmfPkgX64.dsc
> | 1 +
> >> OvmfPkg/OvmfXen.dsc
> | 1 +
> >> UefiPayloadPkg/UefiPayloadPkg.dsc
> | 1 +
> >> 37 files changed, 559 insertions(+), 94 deletions(-)
> >> create mode 100644
> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.c
> >> create mode 100644 MdeModulePkg/Include/Guid/VariableFlashInfo.h
> >> create mode 100644
> MdeModulePkg/Include/Library/VariableFlashInfoLib.h
> >> create mode 100644
> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.i
> nf
> >> create mode 100644
> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.u
> ni
> >>
> >> --
> >> 2.28.0.windows.1
> >>
>
>
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: 回复: [edk2-devel] [PATCH v5 0/8] Add Variable Flash Info HOB
2022-05-05 1:27 ` 回复: [edk2-devel] " gaoliming
@ 2022-05-06 1:52 ` Michael Kubacki
2022-05-10 15:01 ` Michael Kubacki
0 siblings, 1 reply; 26+ messages in thread
From: Michael Kubacki @ 2022-05-06 1:52 UTC (permalink / raw)
To: gaoliming, devel, 'Ard Biesheuvel'
Cc: 'Abner Chang', 'Andrew Fish',
'Anthony Perard', 'Ard Biesheuvel',
'Benjamin You', 'Brijesh Singh',
'Erdem Aktas', 'Gerd Hoffmann',
'Guo Dong', 'Hao A Wu', 'James Bottomley',
'Jian J Wang', 'Jiewen Yao',
'Jordan Justen', 'Julien Grall',
'Leif Lindholm', 'Maurice Ma', 'Min Xu',
'Nickle Wang', 'Peter Grehan', 'Ray Ni',
'Rebecca Cran', 'Sami Mujawar',
'Sean Rhodes', 'Sebastien Boeuf',
'Tom Lendacky'
I still believe a long term design pattern deserves more focus and
documentation than a quick modification to this series.
Can you confirm that you envision MdePkg/MdeLibs.dsc.inc serving as a
monolithic host of various other default library class instances?
That somewhat inverts the package relationships, the code reviewer
policy would need to clarify when the original package owners are
included on the MdePkg patch (to confirm they agree with the default
instance choice), and "core" packages would have to be clearly defined
in this context for developers to know what packages are allowed.
In addition, this does not mean there still won't be some level of
platform integration thrash. For example, if a new library class
instance added to MdePkg/MdeLibs.dsc.inc requires another library class
(or multiple others), those might not be added to the DSC include file.
They could have been satisfied in the original package DSC (or a test
platform DSC) but that doesn't mean they will be in all platform DSC
files. So when the MdeLibs.dsc.inc file update occurs, those platforms
break and need to add the library class that was already specified in
other DSC files.
So I request that if this is the preferred approach, that it be agreed
upon (e.g. dedicated RFC), documented, and consistently followed by
other contributions as well.
Regards,
Michael
On 5/4/2022 9:27 PM, gaoliming wrote:
> Michael:
> I would suggest to reuse MdePkg/MdeLibs.dsc.inc to list the library and PCD from the edk2 core packages, such as MdePkg, MdeModulePkg, CryptoPkg, SecurirtyPkg and so on. Those packages are required by every platforms. They can't be separated. So, I think MdePkg/MdeLibs.dsc.inc is for edk2 core packages, not only for MdePkg.
>
> Thanks
> Liming
>> -----邮件原件-----
>> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Michael
>> Kubacki
>> 发送时间: 2022年4月29日 23:48
>> 收件人: Ard Biesheuvel <ardb@kernel.org>
>> 抄送: edk2-devel-groups-io <devel@edk2.groups.io>; Abner Chang
>> <abner.chang@hpe.com>; Andrew Fish <afish@apple.com>; Anthony Perard
>> <anthony.perard@citrix.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>;
>> Benjamin You <benjamin.you@intel.com>; Brijesh Singh
>> <brijesh.singh@amd.com>; Erdem Aktas <erdemaktas@google.com>; Gerd
>> Hoffmann <kraxel@redhat.com>; Guo Dong <guo.dong@intel.com>; Hao A
>> Wu <hao.a.wu@intel.com>; James Bottomley <jejb@linux.ibm.com>; Jian J
>> Wang <jian.j.wang@intel.com>; Jiewen Yao <jiewen.yao@intel.com>; Jordan
>> Justen <jordan.l.justen@intel.com>; Julien Grall <julien@xen.org>; Leif
>> Lindholm <quic_llindhol@quicinc.com>; Liming Gao
>> <gaoliming@byosoft.com.cn>; Maurice Ma <maurice.ma@intel.com>; Min Xu
>> <min.m.xu@intel.com>; Nickle Wang <nickle.wang@hpe.com>; Peter Grehan
>> <grehan@freebsd.org>; Ray Ni <ray.ni@intel.com>; Rebecca Cran
>> <rebecca@bsdio.com>; Sami Mujawar <sami.mujawar@arm.com>; Sean
>> Rhodes <sean@starlabs.systems>; Sebastien Boeuf
>> <sebastien.boeuf@intel.com>; Tom Lendacky <thomas.lendacky@amd.com>
>> 主题: Re: [edk2-devel] [PATCH v5 0/8] Add Variable Flash Info HOB
>>
>> I agree that would be a useful tool and in the case of changes such as
>> this that provide backward compatibility with existing functionality,
>> particularly helpful.
>>
>> Some packages such as MdePkg
>> (https://github.com/tianocore/edk2/blob/master/MdePkg/MdeLibs.dsc.inc)
>> and NetworkPkg
>> (https://github.com/tianocore/edk2/blob/master/NetworkPkg/NetworkCom
>> ponents.dsc.inc)
>> provide DSC files that a platform can override if necessary.
>>
>> However, this does not exist for all edk2 packages. I did not introduce
>> such a file in MdeModulePkg because I believe that is an independent
>> package design decision outside the scope of this series and, if that
>> change was made, it should include libraries other than just this
>> instance. That would lead to additional churn and a larger platform
>> integration debate, important to that topic, but separate from the
>> current state this contribution is based against.
>>
>> While includes be helpful, it can encourage platform owners to ignore
>> potential creep in functionality they should be aware of.
>>
>> For example, the DSC update is mostly being given to platforms to fix
>> their immediate build problem. But, a platform owner might also choose
>> to update their FVB driver to use this interface to get flash
>> information as opposed to directly using PCDs as many do today. That's a
>> decision they need to evaluate and make but they should be aware of the
>> interface and make that decision. By directly reviewing/integrating the
>> change for their platform, they are more explicitly made aware of this
>> new interface to form that decision.
>>
>> Also, when many include files get involved, platform build complexity
>> and developer frustration can increase due to nesting and order of
>> include files. Values (library classes, PCDs, etc.) can be overridden
>> more than once. Ultimately, this is technically manageable by utilizing
>> build reports and understanding the EDK II build output in more detail.
>>
>> Again, I think this conversation is useful but requires much more time
>> to address questions such as the following:
>>
>> 1. Should a different mechanism for default library classes be introduced?
>>
>> 2. Should all packages in edk2 provide such an include file? If so, does
>> it only provide the DSC file (like MdePkg) or other files (like
>> NetworkPkg which includes FDF)?
>>
>> 3. Which library classes for a given package should be given default
>> instances?
>>
>> 4. How would each platform package maintainer like to maintain their
>> platform build?
>>
>> Example: Would MinPlatformPkg prefer to keep its own "core" include
>> files
>> (https://github.com/tianocore/edk2-platforms/tree/master/Platform/Intel/Mi
>> nPlatformPkg/Include)
>> or reconcile them with include files (possibly introducing an additional
>> layer of nesting)? Would others prefer not to use includes at all to
>> keep a flat, simpler file?
>>
>> How would someone updating the platform code due to an edk2 change be
>> aware of this per-platform policy?
>>
>> To my understanding, the answers to these today are:
>>
>> 1. No
>> 2. No / decided on per-change basis
>> 3. Unknown
>> 4. Unknown
>>
>> (2) adds friction to the edk2 contribution process as expectation is
>> unclear.
>>
>> (3) adds churn into the platform integration process as the integration
>> process for existing code is subject to change over time (e.g. realize
>> library class is now in DSC and remove from platform DSC to use include
>> instance).
>>
>> (4) adds friction to the edk2 contribution process as expectation is
>> unclear. Also, somewhat error prone. There's also a level of due
>> diligence needed to confirm if a platform that already has an include is
>> overriding that in another DSC file. If so, is that still the correct
>> behavior or should it be modified.
>>
>> Regards,
>> Michael
>>
>> On 4/29/2022 9:45 AM, Ard Biesheuvel wrote:
>>> On Tue, 26 Apr 2022 at 03:29, <mikuback@linux.microsoft.com> wrote:
>>>>
>>>> From: Michael Kubacki <michael.kubacki@microsoft.com>
>>>>
>>>> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3479
>>>>
>>>> Platforms: This series introduces a new library class called
>>>> VariableFlashInfoLib. Platforms using the variable modules will
>>>> need to specify these library classes. See the patches at the
>>>> end of this series for examples of the change needed in the
>>>> platform DSC file. I have attempted to update all open-source
>>>> platforms in edk2 and edk2-platforms in this series and
>>>> https://edk2.groups.io/g/devel/message/89148.
>>>>
>>>
>>> I will make the same remark here as I made in response to the
>>> edk2-platforms series:
>>>
>>> Could we please consider introducing a way to define the default
>>> resolution of a library class? That way, all this churn and
>>> unnecessary breakage would not be necessary, as defining a resolution
>>> would only be necessary when deviating from the default. In fact, we
>>> might be able to clean up some .DSCs considerably by defining defaults
>>> for library classes that only have one (or very few) implementations.
>>>
>>>
>>>> The UEFI variable drivers such as VariableRuntimeDxe, VariableSmm,
>>>> VariableStandaloneMm, etc. (and their dependent protocol/library
>>>> stack), typically acquire UEFI variable store flash information
>>>> with PCDs declared in MdeModulePkg.
>>>>
>>>> For example:
>>>> [Pcd]
>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase
>>>>
>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64
>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
>>>>
>>>> These PCDs work as-is in the StandaloneMm driver if they are not
>>>> dynamic such as Dynamic or DynamicEx because PCD services are not
>>>> readily available in the Standalone MM environment. Platforms that
>>>> use Standalone MM today, must define these PCDs as FixedAtBuild in
>>>> their platform build. However, the PCDs do allow platforms to treat
>>>> the PCDs as Dynamic/DynamicEx and being able to support that is
>>>> currently a gap for Standalone MM.
>>>>
>>>> This patch series introduces a HOB that can be produced by the
>>>> platform to provide the same information. The HOB list is
>>>> available to Standalone MM.
>>>>
>>>> The PCD declarations are left as-is in MdeModulePkg for backward
>>>> compatibility. This means unless a platform wants to use the HOB,
>>>> their code will continue to work with no change (they do not need
>>>> to produce the HOB). Only if the HOB is found, is its value used
>>>> instead of the PCDs.
>>>>
>>>> Due to the large number of consumers of this information, access
>>>> to the base address and size values is abstracted in a new library
>>>> class (as requested in the v1 series) called VariableFlashInfoLib.
>>>>
>>>> The API of VariableFlashInfoLib does not bind the underlying data
>>>> structure to the information returned to library users to allow
>>>> flexibility in the library implementation in the future.
>>>>
>>>> V5 changes.
>>>> 1. Made GetVariableFlashInfoFromHob() in BaseVariableFlashInfoLib.c
>>>> STATIC.
>>>> 2. Added a section in commit v5 [3/8] to explicitly state that the
>>>> commit introduces a dependency that requires a change in platform
>>>> build. Please see patches v5 [5/8] - v5 [8/8] for examples of
>>>> how to integrate this change (add VariableFlashInfoLib library
>>>> to DSC file).
>>>>
>>>> V4 changes:
>>>> 1. Add a UINT32 "Reserved" field to VARIABLE_FLASH_INFO.
>>>> 2. Add a descriptive comment to VariableFlashInfo.h to explain
>>>> HOB usage.
>>>>
>>>> V3 changes:
>>>> 1. To better clarify usage, renamed the members
>>>> "NvStorageBaseAddress" and "NvStorageLength" in
>>>> "VARIABLE_FLASH_INFO" to "NvVariableBaseAddress" and
>>>> "NvVariableLength".
>>>> 2. Added description comments to the fields in "VARIABLE_FLASH_INFO".
>>>>
>>>> V2 changes:
>>>> 1. Abstracted flash info data access with VariableFlashInfoLib.
>>>> 2. Updated package builds in the repo that build the variable and
>>>> FTW drivers to include VariableFlashInfoLib.
>>>> 3. Removed a redundant variable assignment in VariableSmm.c.
>>>> 4. Updated comments in FtwMisc.c and FaultTolerantWritePei.c to
>>>> indicate driver assumption is UINTN (not UINT32)
>>>> 5. Added a version field to the VARIABLE_FLASH_INFO structure.
>>>>
>>>> Cc: Abner Chang <abner.chang@hpe.com>
>>>> Cc: Andrew Fish <afish@apple.com>
>>>> Cc: Anthony Perard <anthony.perard@citrix.com>
>>>> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
>>>> Cc: Benjamin You <benjamin.you@intel.com>
>>>> Cc: Brijesh Singh <brijesh.singh@amd.com>
>>>> Cc: Erdem Aktas <erdemaktas@google.com>
>>>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>>>> Cc: Guo Dong <guo.dong@intel.com>
>>>> Cc: Hao A Wu <hao.a.wu@intel.com>
>>>> Cc: James Bottomley <jejb@linux.ibm.com>
>>>> Cc: Jian J Wang <jian.j.wang@intel.com>
>>>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>>>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>>>> Cc: Julien Grall <julien@xen.org>
>>>> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
>>>> Cc: Liming Gao <gaoliming@byosoft.com.cn>
>>>> Cc: Maurice Ma <maurice.ma@intel.com>
>>>> Cc: Min Xu <min.m.xu@intel.com>
>>>> Cc: Nickle Wang <nickle.wang@hpe.com>
>>>> Cc: Peter Grehan <grehan@freebsd.org>
>>>> Cc: Ray Ni <ray.ni@intel.com>
>>>> Cc: Rebecca Cran <rebecca@bsdio.com>
>>>> Cc: Sami Mujawar <sami.mujawar@arm.com>
>>>> Cc: Sean Rhodes <sean@starlabs.systems>
>>>> Cc: Sebastien Boeuf <sebastien.boeuf@intel.com>
>>>> Cc: Tom Lendacky <thomas.lendacky@amd.com>
>>>> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
>>>>
>>>> Michael Kubacki (8):
>>>> MdeModulePkg: Add Variable Flash Info HOB
>>>> MdeModulePkg/VariableFlashInfoLib: Add initial library
>>>> MdeModulePkg/Variable: Consume Variable Flash Info
>>>> MdeModulePkg/FaultTolerantWrite: Consume Variable Flash Info
>>>> ArmVirtPkg/ArmVirt.dsc.inc: Add VariableFlashInfoLib
>>>> EmulatorPkg: Add VariableFlashInfoLib
>>>> OvmfPkg: Add VariableFlashInfoLib
>>>> UefiPayloadPkg: Add VariableFlashInfoLib
>>>>
>>>>
>> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.c
>> | 179 ++++++++++++++++++++
>>>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c
>> | 41 +++--
>>>>
>> MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c
>> | 7 +-
>>>>
>> MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.c
>> | 28 +--
>>>> MdeModulePkg/Universal/Variable/Pei/Variable.c
>> | 14 +-
>>>> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
>> | 16 +-
>>>> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.c
>> | 14 +-
>>>> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
>> | 17 +-
>>>> ArmVirtPkg/ArmVirt.dsc.inc
>> | 1 +
>>>> EmulatorPkg/EmulatorPkg.dsc
>> | 1 +
>>>> MdeModulePkg/Include/Guid/VariableFlashInfo.h
>> | 111 ++++++++++++
>>>> MdeModulePkg/Include/Library/VariableFlashInfoLib.h
>> | 68 ++++++++
>>>>
>> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.i
>> nf | 48 ++++++
>>>>
>> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.u
>> ni | 12 ++
>>>> MdeModulePkg/MdeModulePkg.dec
>> | 8 +
>>>> MdeModulePkg/MdeModulePkg.dsc
>> | 2 +
>>>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h
>> | 7 +-
>>>>
>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
>> | 10 +-
>>>>
>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf
>> | 10 +-
>>>>
>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal
>> oneMm.inf | 10 +-
>>>>
>> MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
>> | 10 +-
>>>> MdeModulePkg/Universal/Variable/Pei/Variable.h
>> | 2 +
>>>> MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
>> | 5 +-
>>>> MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
>> | 7 +-
>>>>
>> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
>> | 5 +-
>>>> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
>> | 5 +-
>>>>
>> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
>> | 5 +-
>>>> OvmfPkg/AmdSev/AmdSevX64.dsc
>> | 1 +
>>>> OvmfPkg/Bhyve/BhyveX64.dsc
>> | 1 +
>>>> OvmfPkg/CloudHv/CloudHvX64.dsc
>> | 1 +
>>>> OvmfPkg/IntelTdx/IntelTdxX64.dsc
>> | 1 +
>>>> OvmfPkg/Microvm/MicrovmX64.dsc
>> | 1 +
>>>> OvmfPkg/OvmfPkgIa32.dsc
>> | 1 +
>>>> OvmfPkg/OvmfPkgIa32X64.dsc
>> | 1 +
>>>> OvmfPkg/OvmfPkgX64.dsc
>> | 1 +
>>>> OvmfPkg/OvmfXen.dsc
>> | 1 +
>>>> UefiPayloadPkg/UefiPayloadPkg.dsc
>> | 1 +
>>>> 37 files changed, 559 insertions(+), 94 deletions(-)
>>>> create mode 100644
>> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.c
>>>> create mode 100644 MdeModulePkg/Include/Guid/VariableFlashInfo.h
>>>> create mode 100644
>> MdeModulePkg/Include/Library/VariableFlashInfoLib.h
>>>> create mode 100644
>> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.i
>> nf
>>>> create mode 100644
>> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.u
>> ni
>>>>
>>>> --
>>>> 2.28.0.windows.1
>>>>
>>
>>
>>
>>
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: 回复: [edk2-devel] [PATCH v5 0/8] Add Variable Flash Info HOB
2022-05-06 1:52 ` Michael Kubacki
@ 2022-05-10 15:01 ` Michael Kubacki
2022-05-13 18:23 ` Michael Kubacki
0 siblings, 1 reply; 26+ messages in thread
From: Michael Kubacki @ 2022-05-10 15:01 UTC (permalink / raw)
To: gaoliming, devel, 'Ard Biesheuvel'
Cc: 'Abner Chang', 'Andrew Fish',
'Anthony Perard', 'Ard Biesheuvel',
'Benjamin You', 'Brijesh Singh',
'Erdem Aktas', 'Gerd Hoffmann',
'Guo Dong', 'Hao A Wu', 'James Bottomley',
'Jian J Wang', 'Jiewen Yao',
'Jordan Justen', 'Julien Grall',
'Leif Lindholm', 'Maurice Ma', 'Min Xu',
'Nickle Wang', 'Peter Grehan', 'Ray Ni',
'Rebecca Cran', 'Sami Mujawar',
'Sean Rhodes', 'Sebastien Boeuf',
'Tom Lendacky'
What's the plan for next steps? The v5 PR has been up for two weeks with
no changes.
Are we going to try to define a long-term pattern for how to include new
library classes in core packages or merge the patch series?
Thanks,
Michael
On 5/5/2022 9:52 PM, Michael Kubacki wrote:
> I still believe a long term design pattern deserves more focus and
> documentation than a quick modification to this series.
>
> Can you confirm that you envision MdePkg/MdeLibs.dsc.inc serving as a
> monolithic host of various other default library class instances?
>
> That somewhat inverts the package relationships, the code reviewer
> policy would need to clarify when the original package owners are
> included on the MdePkg patch (to confirm they agree with the default
> instance choice), and "core" packages would have to be clearly defined
> in this context for developers to know what packages are allowed.
>
> In addition, this does not mean there still won't be some level of
> platform integration thrash. For example, if a new library class
> instance added to MdePkg/MdeLibs.dsc.inc requires another library class
> (or multiple others), those might not be added to the DSC include file.
> They could have been satisfied in the original package DSC (or a test
> platform DSC) but that doesn't mean they will be in all platform DSC
> files. So when the MdeLibs.dsc.inc file update occurs, those platforms
> break and need to add the library class that was already specified in
> other DSC files.
>
> So I request that if this is the preferred approach, that it be agreed
> upon (e.g. dedicated RFC), documented, and consistently followed by
> other contributions as well.
>
> Regards,
> Michael
>
> On 5/4/2022 9:27 PM, gaoliming wrote:
>> Michael:
>> I would suggest to reuse MdePkg/MdeLibs.dsc.inc to list the library
>> and PCD from the edk2 core packages, such as MdePkg, MdeModulePkg,
>> CryptoPkg, SecurirtyPkg and so on. Those packages are required by
>> every platforms. They can't be separated. So, I think
>> MdePkg/MdeLibs.dsc.inc is for edk2 core packages, not only for MdePkg.
>>
>> Thanks
>> Liming
>>> -----邮件原件-----
>>> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Michael
>>> Kubacki
>>> 发送时间: 2022年4月29日 23:48
>>> 收件人: Ard Biesheuvel <ardb@kernel.org>
>>> 抄送: edk2-devel-groups-io <devel@edk2.groups.io>; Abner Chang
>>> <abner.chang@hpe.com>; Andrew Fish <afish@apple.com>; Anthony Perard
>>> <anthony.perard@citrix.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>;
>>> Benjamin You <benjamin.you@intel.com>; Brijesh Singh
>>> <brijesh.singh@amd.com>; Erdem Aktas <erdemaktas@google.com>; Gerd
>>> Hoffmann <kraxel@redhat.com>; Guo Dong <guo.dong@intel.com>; Hao A
>>> Wu <hao.a.wu@intel.com>; James Bottomley <jejb@linux.ibm.com>; Jian J
>>> Wang <jian.j.wang@intel.com>; Jiewen Yao <jiewen.yao@intel.com>; Jordan
>>> Justen <jordan.l.justen@intel.com>; Julien Grall <julien@xen.org>; Leif
>>> Lindholm <quic_llindhol@quicinc.com>; Liming Gao
>>> <gaoliming@byosoft.com.cn>; Maurice Ma <maurice.ma@intel.com>; Min Xu
>>> <min.m.xu@intel.com>; Nickle Wang <nickle.wang@hpe.com>; Peter Grehan
>>> <grehan@freebsd.org>; Ray Ni <ray.ni@intel.com>; Rebecca Cran
>>> <rebecca@bsdio.com>; Sami Mujawar <sami.mujawar@arm.com>; Sean
>>> Rhodes <sean@starlabs.systems>; Sebastien Boeuf
>>> <sebastien.boeuf@intel.com>; Tom Lendacky <thomas.lendacky@amd.com>
>>> 主题: Re: [edk2-devel] [PATCH v5 0/8] Add Variable Flash Info HOB
>>>
>>> I agree that would be a useful tool and in the case of changes such as
>>> this that provide backward compatibility with existing functionality,
>>> particularly helpful.
>>>
>>> Some packages such as MdePkg
>>> (https://github.com/tianocore/edk2/blob/master/MdePkg/MdeLibs.dsc.inc)
>>> and NetworkPkg
>>> (https://github.com/tianocore/edk2/blob/master/NetworkPkg/NetworkCom
>>> ponents.dsc.inc)
>>> provide DSC files that a platform can override if necessary.
>>>
>>> However, this does not exist for all edk2 packages. I did not introduce
>>> such a file in MdeModulePkg because I believe that is an independent
>>> package design decision outside the scope of this series and, if that
>>> change was made, it should include libraries other than just this
>>> instance. That would lead to additional churn and a larger platform
>>> integration debate, important to that topic, but separate from the
>>> current state this contribution is based against.
>>>
>>> While includes be helpful, it can encourage platform owners to ignore
>>> potential creep in functionality they should be aware of.
>>>
>>> For example, the DSC update is mostly being given to platforms to fix
>>> their immediate build problem. But, a platform owner might also choose
>>> to update their FVB driver to use this interface to get flash
>>> information as opposed to directly using PCDs as many do today. That's a
>>> decision they need to evaluate and make but they should be aware of the
>>> interface and make that decision. By directly reviewing/integrating the
>>> change for their platform, they are more explicitly made aware of this
>>> new interface to form that decision.
>>>
>>> Also, when many include files get involved, platform build complexity
>>> and developer frustration can increase due to nesting and order of
>>> include files. Values (library classes, PCDs, etc.) can be overridden
>>> more than once. Ultimately, this is technically manageable by utilizing
>>> build reports and understanding the EDK II build output in more detail.
>>>
>>> Again, I think this conversation is useful but requires much more time
>>> to address questions such as the following:
>>>
>>> 1. Should a different mechanism for default library classes be
>>> introduced?
>>>
>>> 2. Should all packages in edk2 provide such an include file? If so, does
>>> it only provide the DSC file (like MdePkg) or other files (like
>>> NetworkPkg which includes FDF)?
>>>
>>> 3. Which library classes for a given package should be given default
>>> instances?
>>>
>>> 4. How would each platform package maintainer like to maintain their
>>> platform build?
>>>
>>> Example: Would MinPlatformPkg prefer to keep its own "core" include
>>> files
>>> (https://github.com/tianocore/edk2-platforms/tree/master/Platform/Intel/Mi
>>>
>>> nPlatformPkg/Include)
>>> or reconcile them with include files (possibly introducing an additional
>>> layer of nesting)? Would others prefer not to use includes at all to
>>> keep a flat, simpler file?
>>>
>>> How would someone updating the platform code due to an edk2 change be
>>> aware of this per-platform policy?
>>>
>>> To my understanding, the answers to these today are:
>>>
>>> 1. No
>>> 2. No / decided on per-change basis
>>> 3. Unknown
>>> 4. Unknown
>>>
>>> (2) adds friction to the edk2 contribution process as expectation is
>>> unclear.
>>>
>>> (3) adds churn into the platform integration process as the integration
>>> process for existing code is subject to change over time (e.g. realize
>>> library class is now in DSC and remove from platform DSC to use include
>>> instance).
>>>
>>> (4) adds friction to the edk2 contribution process as expectation is
>>> unclear. Also, somewhat error prone. There's also a level of due
>>> diligence needed to confirm if a platform that already has an include is
>>> overriding that in another DSC file. If so, is that still the correct
>>> behavior or should it be modified.
>>>
>>> Regards,
>>> Michael
>>>
>>> On 4/29/2022 9:45 AM, Ard Biesheuvel wrote:
>>>> On Tue, 26 Apr 2022 at 03:29, <mikuback@linux.microsoft.com> wrote:
>>>>>
>>>>> From: Michael Kubacki <michael.kubacki@microsoft.com>
>>>>>
>>>>> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3479
>>>>>
>>>>> Platforms: This series introduces a new library class called
>>>>> VariableFlashInfoLib. Platforms using the variable modules will
>>>>> need to specify these library classes. See the patches at the
>>>>> end of this series for examples of the change needed in the
>>>>> platform DSC file. I have attempted to update all open-source
>>>>> platforms in edk2 and edk2-platforms in this series and
>>>>> https://edk2.groups.io/g/devel/message/89148.
>>>>>
>>>>
>>>> I will make the same remark here as I made in response to the
>>>> edk2-platforms series:
>>>>
>>>> Could we please consider introducing a way to define the default
>>>> resolution of a library class? That way, all this churn and
>>>> unnecessary breakage would not be necessary, as defining a resolution
>>>> would only be necessary when deviating from the default. In fact, we
>>>> might be able to clean up some .DSCs considerably by defining defaults
>>>> for library classes that only have one (or very few) implementations.
>>>>
>>>>
>>>>> The UEFI variable drivers such as VariableRuntimeDxe, VariableSmm,
>>>>> VariableStandaloneMm, etc. (and their dependent protocol/library
>>>>> stack), typically acquire UEFI variable store flash information
>>>>> with PCDs declared in MdeModulePkg.
>>>>>
>>>>> For example:
>>>>> [Pcd]
>>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase
>>>>>
>>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64
>>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
>>>>>
>>>>> These PCDs work as-is in the StandaloneMm driver if they are not
>>>>> dynamic such as Dynamic or DynamicEx because PCD services are not
>>>>> readily available in the Standalone MM environment. Platforms that
>>>>> use Standalone MM today, must define these PCDs as FixedAtBuild in
>>>>> their platform build. However, the PCDs do allow platforms to treat
>>>>> the PCDs as Dynamic/DynamicEx and being able to support that is
>>>>> currently a gap for Standalone MM.
>>>>>
>>>>> This patch series introduces a HOB that can be produced by the
>>>>> platform to provide the same information. The HOB list is
>>>>> available to Standalone MM.
>>>>>
>>>>> The PCD declarations are left as-is in MdeModulePkg for backward
>>>>> compatibility. This means unless a platform wants to use the HOB,
>>>>> their code will continue to work with no change (they do not need
>>>>> to produce the HOB). Only if the HOB is found, is its value used
>>>>> instead of the PCDs.
>>>>>
>>>>> Due to the large number of consumers of this information, access
>>>>> to the base address and size values is abstracted in a new library
>>>>> class (as requested in the v1 series) called VariableFlashInfoLib.
>>>>>
>>>>> The API of VariableFlashInfoLib does not bind the underlying data
>>>>> structure to the information returned to library users to allow
>>>>> flexibility in the library implementation in the future.
>>>>>
>>>>> V5 changes.
>>>>> 1. Made GetVariableFlashInfoFromHob() in BaseVariableFlashInfoLib.c
>>>>> STATIC.
>>>>> 2. Added a section in commit v5 [3/8] to explicitly state that the
>>>>> commit introduces a dependency that requires a change in platform
>>>>> build. Please see patches v5 [5/8] - v5 [8/8] for examples of
>>>>> how to integrate this change (add VariableFlashInfoLib library
>>>>> to DSC file).
>>>>>
>>>>> V4 changes:
>>>>> 1. Add a UINT32 "Reserved" field to VARIABLE_FLASH_INFO.
>>>>> 2. Add a descriptive comment to VariableFlashInfo.h to explain
>>>>> HOB usage.
>>>>>
>>>>> V3 changes:
>>>>> 1. To better clarify usage, renamed the members
>>>>> "NvStorageBaseAddress" and "NvStorageLength" in
>>>>> "VARIABLE_FLASH_INFO" to "NvVariableBaseAddress" and
>>>>> "NvVariableLength".
>>>>> 2. Added description comments to the fields in "VARIABLE_FLASH_INFO".
>>>>>
>>>>> V2 changes:
>>>>> 1. Abstracted flash info data access with VariableFlashInfoLib.
>>>>> 2. Updated package builds in the repo that build the variable and
>>>>> FTW drivers to include VariableFlashInfoLib.
>>>>> 3. Removed a redundant variable assignment in VariableSmm.c.
>>>>> 4. Updated comments in FtwMisc.c and FaultTolerantWritePei.c to
>>>>> indicate driver assumption is UINTN (not UINT32)
>>>>> 5. Added a version field to the VARIABLE_FLASH_INFO structure.
>>>>>
>>>>> Cc: Abner Chang <abner.chang@hpe.com>
>>>>> Cc: Andrew Fish <afish@apple.com>
>>>>> Cc: Anthony Perard <anthony.perard@citrix.com>
>>>>> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
>>>>> Cc: Benjamin You <benjamin.you@intel.com>
>>>>> Cc: Brijesh Singh <brijesh.singh@amd.com>
>>>>> Cc: Erdem Aktas <erdemaktas@google.com>
>>>>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>>>>> Cc: Guo Dong <guo.dong@intel.com>
>>>>> Cc: Hao A Wu <hao.a.wu@intel.com>
>>>>> Cc: James Bottomley <jejb@linux.ibm.com>
>>>>> Cc: Jian J Wang <jian.j.wang@intel.com>
>>>>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>>>>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>>>>> Cc: Julien Grall <julien@xen.org>
>>>>> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
>>>>> Cc: Liming Gao <gaoliming@byosoft.com.cn>
>>>>> Cc: Maurice Ma <maurice.ma@intel.com>
>>>>> Cc: Min Xu <min.m.xu@intel.com>
>>>>> Cc: Nickle Wang <nickle.wang@hpe.com>
>>>>> Cc: Peter Grehan <grehan@freebsd.org>
>>>>> Cc: Ray Ni <ray.ni@intel.com>
>>>>> Cc: Rebecca Cran <rebecca@bsdio.com>
>>>>> Cc: Sami Mujawar <sami.mujawar@arm.com>
>>>>> Cc: Sean Rhodes <sean@starlabs.systems>
>>>>> Cc: Sebastien Boeuf <sebastien.boeuf@intel.com>
>>>>> Cc: Tom Lendacky <thomas.lendacky@amd.com>
>>>>> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
>>>>>
>>>>> Michael Kubacki (8):
>>>>> MdeModulePkg: Add Variable Flash Info HOB
>>>>> MdeModulePkg/VariableFlashInfoLib: Add initial library
>>>>> MdeModulePkg/Variable: Consume Variable Flash Info
>>>>> MdeModulePkg/FaultTolerantWrite: Consume Variable Flash Info
>>>>> ArmVirtPkg/ArmVirt.dsc.inc: Add VariableFlashInfoLib
>>>>> EmulatorPkg: Add VariableFlashInfoLib
>>>>> OvmfPkg: Add VariableFlashInfoLib
>>>>> UefiPayloadPkg: Add VariableFlashInfoLib
>>>>>
>>>>>
>>> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.c
>>> | 179 ++++++++++++++++++++
>>>>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c
>>> | 41 +++--
>>>>>
>>> MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c
>>> | 7 +-
>>>>>
>>> MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.c
>>> | 28 +--
>>>>> MdeModulePkg/Universal/Variable/Pei/Variable.c
>>> | 14 +-
>>>>> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
>>> | 16 +-
>>>>> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.c
>>> | 14 +-
>>>>> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
>>> | 17 +-
>>>>> ArmVirtPkg/ArmVirt.dsc.inc
>>> | 1 +
>>>>> EmulatorPkg/EmulatorPkg.dsc
>>> | 1 +
>>>>> MdeModulePkg/Include/Guid/VariableFlashInfo.h
>>> | 111 ++++++++++++
>>>>> MdeModulePkg/Include/Library/VariableFlashInfoLib.h
>>> | 68 ++++++++
>>>>>
>>> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.i
>>> nf | 48 ++++++
>>>>>
>>> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.u
>>> ni | 12 ++
>>>>> MdeModulePkg/MdeModulePkg.dec
>>> | 8 +
>>>>> MdeModulePkg/MdeModulePkg.dsc
>>> | 2 +
>>>>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h
>>> | 7 +-
>>>>>
>>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
>>> | 10 +-
>>>>>
>>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf
>>> | 10 +-
>>>>>
>>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal
>>> oneMm.inf | 10 +-
>>>>>
>>> MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
>>> | 10 +-
>>>>> MdeModulePkg/Universal/Variable/Pei/Variable.h
>>> | 2 +
>>>>> MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
>>> | 5 +-
>>>>> MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
>>> | 7 +-
>>>>>
>>> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
>>> | 5 +-
>>>>> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
>>> | 5 +-
>>>>>
>>> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
>>> | 5 +-
>>>>> OvmfPkg/AmdSev/AmdSevX64.dsc
>>> | 1 +
>>>>> OvmfPkg/Bhyve/BhyveX64.dsc
>>> | 1 +
>>>>> OvmfPkg/CloudHv/CloudHvX64.dsc
>>> | 1 +
>>>>> OvmfPkg/IntelTdx/IntelTdxX64.dsc
>>> | 1 +
>>>>> OvmfPkg/Microvm/MicrovmX64.dsc
>>> | 1 +
>>>>> OvmfPkg/OvmfPkgIa32.dsc
>>> | 1 +
>>>>> OvmfPkg/OvmfPkgIa32X64.dsc
>>> | 1 +
>>>>> OvmfPkg/OvmfPkgX64.dsc
>>> | 1 +
>>>>> OvmfPkg/OvmfXen.dsc
>>> | 1 +
>>>>> UefiPayloadPkg/UefiPayloadPkg.dsc
>>> | 1 +
>>>>> 37 files changed, 559 insertions(+), 94 deletions(-)
>>>>> create mode 100644
>>> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.c
>>>>> create mode 100644 MdeModulePkg/Include/Guid/VariableFlashInfo.h
>>>>> create mode 100644
>>> MdeModulePkg/Include/Library/VariableFlashInfoLib.h
>>>>> create mode 100644
>>> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.i
>>> nf
>>>>> create mode 100644
>>> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.u
>>> ni
>>>>>
>>>>> --
>>>>> 2.28.0.windows.1
>>>>>
>>>
>>>
>>>
>>>
>>
>>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: 回复: [edk2-devel] [PATCH v5 0/8] Add Variable Flash Info HOB
2022-05-10 15:01 ` Michael Kubacki
@ 2022-05-13 18:23 ` Michael Kubacki
2022-05-14 1:16 ` 回复: " gaoliming
0 siblings, 1 reply; 26+ messages in thread
From: Michael Kubacki @ 2022-05-13 18:23 UTC (permalink / raw)
To: gaoliming, devel, 'Ard Biesheuvel'
Cc: 'Abner Chang', 'Andrew Fish',
'Anthony Perard', 'Ard Biesheuvel',
'Benjamin You', 'Brijesh Singh',
'Erdem Aktas', 'Gerd Hoffmann',
'Guo Dong', 'Hao A Wu', 'James Bottomley',
'Jian J Wang', 'Jiewen Yao',
'Jordan Justen', 'Julien Grall',
'Leif Lindholm', 'Maurice Ma', 'Min Xu',
'Nickle Wang', 'Peter Grehan', 'Ray Ni',
'Rebecca Cran', 'Sami Mujawar',
'Sean Rhodes', 'Sebastien Boeuf',
'Tom Lendacky'
Can you please respond with your preference?
I am ready to do this but if it is required now, it should be documented
so it becomes a consistent pattern for future changes.
Thanks,
Michael
On 5/10/2022 11:01 AM, Michael Kubacki wrote:
> What's the plan for next steps? The v5 PR has been up for two weeks with
> no changes.
>
> Are we going to try to define a long-term pattern for how to include new
> library classes in core packages or merge the patch series?
>
> Thanks,
> Michael
>
> On 5/5/2022 9:52 PM, Michael Kubacki wrote:
>> I still believe a long term design pattern deserves more focus and
>> documentation than a quick modification to this series.
>>
>> Can you confirm that you envision MdePkg/MdeLibs.dsc.inc serving as a
>> monolithic host of various other default library class instances?
>>
>> That somewhat inverts the package relationships, the code reviewer
>> policy would need to clarify when the original package owners are
>> included on the MdePkg patch (to confirm they agree with the default
>> instance choice), and "core" packages would have to be clearly defined
>> in this context for developers to know what packages are allowed.
>>
>> In addition, this does not mean there still won't be some level of
>> platform integration thrash. For example, if a new library class
>> instance added to MdePkg/MdeLibs.dsc.inc requires another library
>> class (or multiple others), those might not be added to the DSC
>> include file. They could have been satisfied in the original package
>> DSC (or a test platform DSC) but that doesn't mean they will be in all
>> platform DSC files. So when the MdeLibs.dsc.inc file update occurs,
>> those platforms break and need to add the library class that was
>> already specified in other DSC files.
>>
>> So I request that if this is the preferred approach, that it be agreed
>> upon (e.g. dedicated RFC), documented, and consistently followed by
>> other contributions as well.
>>
>> Regards,
>> Michael
>>
>> On 5/4/2022 9:27 PM, gaoliming wrote:
>>> Michael:
>>> I would suggest to reuse MdePkg/MdeLibs.dsc.inc to list the
>>> library and PCD from the edk2 core packages, such as MdePkg,
>>> MdeModulePkg, CryptoPkg, SecurirtyPkg and so on. Those packages are
>>> required by every platforms. They can't be separated. So, I think
>>> MdePkg/MdeLibs.dsc.inc is for edk2 core packages, not only for MdePkg.
>>>
>>> Thanks
>>> Liming
>>>> -----邮件原件-----
>>>> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Michael
>>>> Kubacki
>>>> 发送时间: 2022年4月29日 23:48
>>>> 收件人: Ard Biesheuvel <ardb@kernel.org>
>>>> 抄送: edk2-devel-groups-io <devel@edk2.groups.io>; Abner Chang
>>>> <abner.chang@hpe.com>; Andrew Fish <afish@apple.com>; Anthony Perard
>>>> <anthony.perard@citrix.com>; Ard Biesheuvel
>>>> <ardb+tianocore@kernel.org>;
>>>> Benjamin You <benjamin.you@intel.com>; Brijesh Singh
>>>> <brijesh.singh@amd.com>; Erdem Aktas <erdemaktas@google.com>; Gerd
>>>> Hoffmann <kraxel@redhat.com>; Guo Dong <guo.dong@intel.com>; Hao A
>>>> Wu <hao.a.wu@intel.com>; James Bottomley <jejb@linux.ibm.com>; Jian J
>>>> Wang <jian.j.wang@intel.com>; Jiewen Yao <jiewen.yao@intel.com>; Jordan
>>>> Justen <jordan.l.justen@intel.com>; Julien Grall <julien@xen.org>; Leif
>>>> Lindholm <quic_llindhol@quicinc.com>; Liming Gao
>>>> <gaoliming@byosoft.com.cn>; Maurice Ma <maurice.ma@intel.com>; Min Xu
>>>> <min.m.xu@intel.com>; Nickle Wang <nickle.wang@hpe.com>; Peter Grehan
>>>> <grehan@freebsd.org>; Ray Ni <ray.ni@intel.com>; Rebecca Cran
>>>> <rebecca@bsdio.com>; Sami Mujawar <sami.mujawar@arm.com>; Sean
>>>> Rhodes <sean@starlabs.systems>; Sebastien Boeuf
>>>> <sebastien.boeuf@intel.com>; Tom Lendacky <thomas.lendacky@amd.com>
>>>> 主题: Re: [edk2-devel] [PATCH v5 0/8] Add Variable Flash Info HOB
>>>>
>>>> I agree that would be a useful tool and in the case of changes such as
>>>> this that provide backward compatibility with existing functionality,
>>>> particularly helpful.
>>>>
>>>> Some packages such as MdePkg
>>>> (https://github.com/tianocore/edk2/blob/master/MdePkg/MdeLibs.dsc.inc)
>>>> and NetworkPkg
>>>> (https://github.com/tianocore/edk2/blob/master/NetworkPkg/NetworkCom
>>>> ponents.dsc.inc)
>>>> provide DSC files that a platform can override if necessary.
>>>>
>>>> However, this does not exist for all edk2 packages. I did not introduce
>>>> such a file in MdeModulePkg because I believe that is an independent
>>>> package design decision outside the scope of this series and, if that
>>>> change was made, it should include libraries other than just this
>>>> instance. That would lead to additional churn and a larger platform
>>>> integration debate, important to that topic, but separate from the
>>>> current state this contribution is based against.
>>>>
>>>> While includes be helpful, it can encourage platform owners to ignore
>>>> potential creep in functionality they should be aware of.
>>>>
>>>> For example, the DSC update is mostly being given to platforms to fix
>>>> their immediate build problem. But, a platform owner might also choose
>>>> to update their FVB driver to use this interface to get flash
>>>> information as opposed to directly using PCDs as many do today.
>>>> That's a
>>>> decision they need to evaluate and make but they should be aware of the
>>>> interface and make that decision. By directly reviewing/integrating the
>>>> change for their platform, they are more explicitly made aware of this
>>>> new interface to form that decision.
>>>>
>>>> Also, when many include files get involved, platform build complexity
>>>> and developer frustration can increase due to nesting and order of
>>>> include files. Values (library classes, PCDs, etc.) can be overridden
>>>> more than once. Ultimately, this is technically manageable by utilizing
>>>> build reports and understanding the EDK II build output in more detail.
>>>>
>>>> Again, I think this conversation is useful but requires much more time
>>>> to address questions such as the following:
>>>>
>>>> 1. Should a different mechanism for default library classes be
>>>> introduced?
>>>>
>>>> 2. Should all packages in edk2 provide such an include file? If so,
>>>> does
>>>> it only provide the DSC file (like MdePkg) or other files (like
>>>> NetworkPkg which includes FDF)?
>>>>
>>>> 3. Which library classes for a given package should be given default
>>>> instances?
>>>>
>>>> 4. How would each platform package maintainer like to maintain their
>>>> platform build?
>>>>
>>>> Example: Would MinPlatformPkg prefer to keep its own "core" include
>>>> files
>>>> (https://github.com/tianocore/edk2-platforms/tree/master/Platform/Intel/Mi
>>>>
>>>> nPlatformPkg/Include)
>>>> or reconcile them with include files (possibly introducing an
>>>> additional
>>>> layer of nesting)? Would others prefer not to use includes at all to
>>>> keep a flat, simpler file?
>>>>
>>>> How would someone updating the platform code due to an edk2 change be
>>>> aware of this per-platform policy?
>>>>
>>>> To my understanding, the answers to these today are:
>>>>
>>>> 1. No
>>>> 2. No / decided on per-change basis
>>>> 3. Unknown
>>>> 4. Unknown
>>>>
>>>> (2) adds friction to the edk2 contribution process as expectation is
>>>> unclear.
>>>>
>>>> (3) adds churn into the platform integration process as the integration
>>>> process for existing code is subject to change over time (e.g. realize
>>>> library class is now in DSC and remove from platform DSC to use include
>>>> instance).
>>>>
>>>> (4) adds friction to the edk2 contribution process as expectation is
>>>> unclear. Also, somewhat error prone. There's also a level of due
>>>> diligence needed to confirm if a platform that already has an
>>>> include is
>>>> overriding that in another DSC file. If so, is that still the correct
>>>> behavior or should it be modified.
>>>>
>>>> Regards,
>>>> Michael
>>>>
>>>> On 4/29/2022 9:45 AM, Ard Biesheuvel wrote:
>>>>> On Tue, 26 Apr 2022 at 03:29, <mikuback@linux.microsoft.com> wrote:
>>>>>>
>>>>>> From: Michael Kubacki <michael.kubacki@microsoft.com>
>>>>>>
>>>>>> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3479
>>>>>>
>>>>>> Platforms: This series introduces a new library class called
>>>>>> VariableFlashInfoLib. Platforms using the variable modules will
>>>>>> need to specify these library classes. See the patches at the
>>>>>> end of this series for examples of the change needed in the
>>>>>> platform DSC file. I have attempted to update all open-source
>>>>>> platforms in edk2 and edk2-platforms in this series and
>>>>>> https://edk2.groups.io/g/devel/message/89148.
>>>>>>
>>>>>
>>>>> I will make the same remark here as I made in response to the
>>>>> edk2-platforms series:
>>>>>
>>>>> Could we please consider introducing a way to define the default
>>>>> resolution of a library class? That way, all this churn and
>>>>> unnecessary breakage would not be necessary, as defining a resolution
>>>>> would only be necessary when deviating from the default. In fact, we
>>>>> might be able to clean up some .DSCs considerably by defining defaults
>>>>> for library classes that only have one (or very few) implementations.
>>>>>
>>>>>
>>>>>> The UEFI variable drivers such as VariableRuntimeDxe, VariableSmm,
>>>>>> VariableStandaloneMm, etc. (and their dependent protocol/library
>>>>>> stack), typically acquire UEFI variable store flash information
>>>>>> with PCDs declared in MdeModulePkg.
>>>>>>
>>>>>> For example:
>>>>>> [Pcd]
>>>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase
>>>>>>
>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64
>>>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
>>>>>>
>>>>>> These PCDs work as-is in the StandaloneMm driver if they are not
>>>>>> dynamic such as Dynamic or DynamicEx because PCD services are not
>>>>>> readily available in the Standalone MM environment. Platforms that
>>>>>> use Standalone MM today, must define these PCDs as FixedAtBuild in
>>>>>> their platform build. However, the PCDs do allow platforms to treat
>>>>>> the PCDs as Dynamic/DynamicEx and being able to support that is
>>>>>> currently a gap for Standalone MM.
>>>>>>
>>>>>> This patch series introduces a HOB that can be produced by the
>>>>>> platform to provide the same information. The HOB list is
>>>>>> available to Standalone MM.
>>>>>>
>>>>>> The PCD declarations are left as-is in MdeModulePkg for backward
>>>>>> compatibility. This means unless a platform wants to use the HOB,
>>>>>> their code will continue to work with no change (they do not need
>>>>>> to produce the HOB). Only if the HOB is found, is its value used
>>>>>> instead of the PCDs.
>>>>>>
>>>>>> Due to the large number of consumers of this information, access
>>>>>> to the base address and size values is abstracted in a new library
>>>>>> class (as requested in the v1 series) called VariableFlashInfoLib.
>>>>>>
>>>>>> The API of VariableFlashInfoLib does not bind the underlying data
>>>>>> structure to the information returned to library users to allow
>>>>>> flexibility in the library implementation in the future.
>>>>>>
>>>>>> V5 changes.
>>>>>> 1. Made GetVariableFlashInfoFromHob() in BaseVariableFlashInfoLib.c
>>>>>> STATIC.
>>>>>> 2. Added a section in commit v5 [3/8] to explicitly state that the
>>>>>> commit introduces a dependency that requires a change in
>>>>>> platform
>>>>>> build. Please see patches v5 [5/8] - v5 [8/8] for examples of
>>>>>> how to integrate this change (add VariableFlashInfoLib library
>>>>>> to DSC file).
>>>>>>
>>>>>> V4 changes:
>>>>>> 1. Add a UINT32 "Reserved" field to VARIABLE_FLASH_INFO.
>>>>>> 2. Add a descriptive comment to VariableFlashInfo.h to explain
>>>>>> HOB usage.
>>>>>>
>>>>>> V3 changes:
>>>>>> 1. To better clarify usage, renamed the members
>>>>>> "NvStorageBaseAddress" and "NvStorageLength" in
>>>>>> "VARIABLE_FLASH_INFO" to "NvVariableBaseAddress" and
>>>>>> "NvVariableLength".
>>>>>> 2. Added description comments to the fields in "VARIABLE_FLASH_INFO".
>>>>>>
>>>>>> V2 changes:
>>>>>> 1. Abstracted flash info data access with VariableFlashInfoLib.
>>>>>> 2. Updated package builds in the repo that build the variable and
>>>>>> FTW drivers to include VariableFlashInfoLib.
>>>>>> 3. Removed a redundant variable assignment in VariableSmm.c.
>>>>>> 4. Updated comments in FtwMisc.c and FaultTolerantWritePei.c to
>>>>>> indicate driver assumption is UINTN (not UINT32)
>>>>>> 5. Added a version field to the VARIABLE_FLASH_INFO structure.
>>>>>>
>>>>>> Cc: Abner Chang <abner.chang@hpe.com>
>>>>>> Cc: Andrew Fish <afish@apple.com>
>>>>>> Cc: Anthony Perard <anthony.perard@citrix.com>
>>>>>> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
>>>>>> Cc: Benjamin You <benjamin.you@intel.com>
>>>>>> Cc: Brijesh Singh <brijesh.singh@amd.com>
>>>>>> Cc: Erdem Aktas <erdemaktas@google.com>
>>>>>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>>>>>> Cc: Guo Dong <guo.dong@intel.com>
>>>>>> Cc: Hao A Wu <hao.a.wu@intel.com>
>>>>>> Cc: James Bottomley <jejb@linux.ibm.com>
>>>>>> Cc: Jian J Wang <jian.j.wang@intel.com>
>>>>>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>>>>>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>>>>>> Cc: Julien Grall <julien@xen.org>
>>>>>> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
>>>>>> Cc: Liming Gao <gaoliming@byosoft.com.cn>
>>>>>> Cc: Maurice Ma <maurice.ma@intel.com>
>>>>>> Cc: Min Xu <min.m.xu@intel.com>
>>>>>> Cc: Nickle Wang <nickle.wang@hpe.com>
>>>>>> Cc: Peter Grehan <grehan@freebsd.org>
>>>>>> Cc: Ray Ni <ray.ni@intel.com>
>>>>>> Cc: Rebecca Cran <rebecca@bsdio.com>
>>>>>> Cc: Sami Mujawar <sami.mujawar@arm.com>
>>>>>> Cc: Sean Rhodes <sean@starlabs.systems>
>>>>>> Cc: Sebastien Boeuf <sebastien.boeuf@intel.com>
>>>>>> Cc: Tom Lendacky <thomas.lendacky@amd.com>
>>>>>> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
>>>>>>
>>>>>> Michael Kubacki (8):
>>>>>> MdeModulePkg: Add Variable Flash Info HOB
>>>>>> MdeModulePkg/VariableFlashInfoLib: Add initial library
>>>>>> MdeModulePkg/Variable: Consume Variable Flash Info
>>>>>> MdeModulePkg/FaultTolerantWrite: Consume Variable Flash Info
>>>>>> ArmVirtPkg/ArmVirt.dsc.inc: Add VariableFlashInfoLib
>>>>>> EmulatorPkg: Add VariableFlashInfoLib
>>>>>> OvmfPkg: Add VariableFlashInfoLib
>>>>>> UefiPayloadPkg: Add VariableFlashInfoLib
>>>>>>
>>>>>>
>>>> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.c
>>>>
>>>> | 179 ++++++++++++++++++++
>>>>>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c
>>>> | 41 +++--
>>>>>>
>>>> MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c
>>>> | 7 +-
>>>>>>
>>>> MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.c
>>>> | 28 +--
>>>>>> MdeModulePkg/Universal/Variable/Pei/Variable.c
>>>> | 14 +-
>>>>>> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
>>>> | 16 +-
>>>>>> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.c
>>>> | 14 +-
>>>>>> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
>>>> | 17 +-
>>>>>> ArmVirtPkg/ArmVirt.dsc.inc
>>>> | 1 +
>>>>>> EmulatorPkg/EmulatorPkg.dsc
>>>> | 1 +
>>>>>> MdeModulePkg/Include/Guid/VariableFlashInfo.h
>>>> | 111 ++++++++++++
>>>>>> MdeModulePkg/Include/Library/VariableFlashInfoLib.h
>>>> | 68 ++++++++
>>>>>>
>>>> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.i
>>>>
>>>> nf | 48 ++++++
>>>>>>
>>>> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.u
>>>>
>>>> ni | 12 ++
>>>>>> MdeModulePkg/MdeModulePkg.dec
>>>> | 8 +
>>>>>> MdeModulePkg/MdeModulePkg.dsc
>>>> | 2 +
>>>>>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h
>>>> | 7 +-
>>>>>>
>>>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
>>>> | 10 +-
>>>>>>
>>>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf
>>>> | 10 +-
>>>>>>
>>>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal
>>>> oneMm.inf | 10 +-
>>>>>>
>>>> MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
>>>> | 10 +-
>>>>>> MdeModulePkg/Universal/Variable/Pei/Variable.h
>>>> | 2 +
>>>>>> MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
>>>> | 5 +-
>>>>>> MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
>>>> | 7 +-
>>>>>>
>>>> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
>>>> | 5 +-
>>>>>> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
>>>> | 5 +-
>>>>>>
>>>> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
>>>> | 5 +-
>>>>>> OvmfPkg/AmdSev/AmdSevX64.dsc
>>>> | 1 +
>>>>>> OvmfPkg/Bhyve/BhyveX64.dsc
>>>> | 1 +
>>>>>> OvmfPkg/CloudHv/CloudHvX64.dsc
>>>> | 1 +
>>>>>> OvmfPkg/IntelTdx/IntelTdxX64.dsc
>>>> | 1 +
>>>>>> OvmfPkg/Microvm/MicrovmX64.dsc
>>>> | 1 +
>>>>>> OvmfPkg/OvmfPkgIa32.dsc
>>>> | 1 +
>>>>>> OvmfPkg/OvmfPkgIa32X64.dsc
>>>> | 1 +
>>>>>> OvmfPkg/OvmfPkgX64.dsc
>>>> | 1 +
>>>>>> OvmfPkg/OvmfXen.dsc
>>>> | 1 +
>>>>>> UefiPayloadPkg/UefiPayloadPkg.dsc
>>>> | 1 +
>>>>>> 37 files changed, 559 insertions(+), 94 deletions(-)
>>>>>> create mode 100644
>>>> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.c
>>>>
>>>>>> create mode 100644 MdeModulePkg/Include/Guid/VariableFlashInfo.h
>>>>>> create mode 100644
>>>> MdeModulePkg/Include/Library/VariableFlashInfoLib.h
>>>>>> create mode 100644
>>>> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.i
>>>>
>>>> nf
>>>>>> create mode 100644
>>>> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.u
>>>>
>>>> ni
>>>>>>
>>>>>> --
>>>>>> 2.28.0.windows.1
>>>>>>
>>>>
>>>>
>>>>
>>>>
>>>
>>>
^ permalink raw reply [flat|nested] 26+ messages in thread
* 回复: 回复: [edk2-devel] [PATCH v5 0/8] Add Variable Flash Info HOB
2022-05-13 18:23 ` Michael Kubacki
@ 2022-05-14 1:16 ` gaoliming
2022-05-16 15:27 ` Michael Kubacki
0 siblings, 1 reply; 26+ messages in thread
From: gaoliming @ 2022-05-14 1:16 UTC (permalink / raw)
To: 'Michael Kubacki', devel, 'Ard Biesheuvel'
Cc: 'Abner Chang', 'Andrew Fish',
'Anthony Perard', 'Ard Biesheuvel',
'Benjamin You', 'Brijesh Singh',
'Erdem Aktas', 'Gerd Hoffmann',
'Guo Dong', 'Hao A Wu', 'James Bottomley',
'Jian J Wang', 'Jiewen Yao',
'Jordan Justen', 'Julien Grall',
'Leif Lindholm', 'Maurice Ma', 'Min Xu',
'Nickle Wang', 'Peter Grehan', 'Ray Ni',
'Rebecca Cran', 'Sami Mujawar',
'Sean Rhodes', 'Sebastien Boeuf',
'Tom Lendacky'
Michael:
This is my suggestion to resolve such compatible issue. As you say, it needs more review and discussion. So, it may not be applied immediately.
If this patch set needs to catch this table tag, it has to take current way to update each DSC file. Have the patch set got reviewed-by from Package maintainers? If yes, I think this patch set can still be merged for this stable tag.
Thanks
Liming
> -----邮件原件-----
> 发件人: Michael Kubacki <mikuback@linux.microsoft.com>
> 发送时间: 2022年5月14日 2:24
> 收件人: gaoliming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io; 'Ard
> Biesheuvel' <ardb@kernel.org>
> 抄送: 'Abner Chang' <abner.chang@hpe.com>; 'Andrew Fish'
> <afish@apple.com>; 'Anthony Perard' <anthony.perard@citrix.com>; 'Ard
> Biesheuvel' <ardb+tianocore@kernel.org>; 'Benjamin You'
> <benjamin.you@intel.com>; 'Brijesh Singh' <brijesh.singh@amd.com>; 'Erdem
> Aktas' <erdemaktas@google.com>; 'Gerd Hoffmann' <kraxel@redhat.com>;
> 'Guo Dong' <guo.dong@intel.com>; 'Hao A Wu' <hao.a.wu@intel.com>;
> 'James Bottomley' <jejb@linux.ibm.com>; 'Jian J Wang'
> <jian.j.wang@intel.com>; 'Jiewen Yao' <jiewen.yao@intel.com>; 'Jordan
> Justen' <jordan.l.justen@intel.com>; 'Julien Grall' <julien@xen.org>; 'Leif
> Lindholm' <quic_llindhol@quicinc.com>; 'Maurice Ma'
> <maurice.ma@intel.com>; 'Min Xu' <min.m.xu@intel.com>; 'Nickle Wang'
> <nickle.wang@hpe.com>; 'Peter Grehan' <grehan@freebsd.org>; 'Ray Ni'
> <ray.ni@intel.com>; 'Rebecca Cran' <rebecca@bsdio.com>; 'Sami Mujawar'
> <sami.mujawar@arm.com>; 'Sean Rhodes' <sean@starlabs.systems>;
> 'Sebastien Boeuf' <sebastien.boeuf@intel.com>; 'Tom Lendacky'
> <thomas.lendacky@amd.com>
> 主题: Re: 回复: [edk2-devel] [PATCH v5 0/8] Add Variable Flash Info HOB
>
> Can you please respond with your preference?
>
> I am ready to do this but if it is required now, it should be documented
> so it becomes a consistent pattern for future changes.
>
> Thanks,
> Michael
>
> On 5/10/2022 11:01 AM, Michael Kubacki wrote:
> > What's the plan for next steps? The v5 PR has been up for two weeks with
> > no changes.
> >
> > Are we going to try to define a long-term pattern for how to include new
> > library classes in core packages or merge the patch series?
> >
> > Thanks,
> > Michael
> >
> > On 5/5/2022 9:52 PM, Michael Kubacki wrote:
> >> I still believe a long term design pattern deserves more focus and
> >> documentation than a quick modification to this series.
> >>
> >> Can you confirm that you envision MdePkg/MdeLibs.dsc.inc serving as a
> >> monolithic host of various other default library class instances?
> >>
> >> That somewhat inverts the package relationships, the code reviewer
> >> policy would need to clarify when the original package owners are
> >> included on the MdePkg patch (to confirm they agree with the default
> >> instance choice), and "core" packages would have to be clearly defined
> >> in this context for developers to know what packages are allowed.
> >>
> >> In addition, this does not mean there still won't be some level of
> >> platform integration thrash. For example, if a new library class
> >> instance added to MdePkg/MdeLibs.dsc.inc requires another library
> >> class (or multiple others), those might not be added to the DSC
> >> include file. They could have been satisfied in the original package
> >> DSC (or a test platform DSC) but that doesn't mean they will be in all
> >> platform DSC files. So when the MdeLibs.dsc.inc file update occurs,
> >> those platforms break and need to add the library class that was
> >> already specified in other DSC files.
> >>
> >> So I request that if this is the preferred approach, that it be agreed
> >> upon (e.g. dedicated RFC), documented, and consistently followed by
> >> other contributions as well.
> >>
> >> Regards,
> >> Michael
> >>
> >> On 5/4/2022 9:27 PM, gaoliming wrote:
> >>> Michael:
> >>> I would suggest to reuse MdePkg/MdeLibs.dsc.inc to list the
> >>> library and PCD from the edk2 core packages, such as MdePkg,
> >>> MdeModulePkg, CryptoPkg, SecurirtyPkg and so on. Those packages are
> >>> required by every platforms. They can't be separated. So, I think
> >>> MdePkg/MdeLibs.dsc.inc is for edk2 core packages, not only for MdePkg.
> >>>
> >>> Thanks
> >>> Liming
> >>>> -----邮件原件-----
> >>>> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表
> Michael
> >>>> Kubacki
> >>>> 发送时间: 2022年4月29日 23:48
> >>>> 收件人: Ard Biesheuvel <ardb@kernel.org>
> >>>> 抄送: edk2-devel-groups-io <devel@edk2.groups.io>; Abner Chang
> >>>> <abner.chang@hpe.com>; Andrew Fish <afish@apple.com>; Anthony
> Perard
> >>>> <anthony.perard@citrix.com>; Ard Biesheuvel
> >>>> <ardb+tianocore@kernel.org>;
> >>>> Benjamin You <benjamin.you@intel.com>; Brijesh Singh
> >>>> <brijesh.singh@amd.com>; Erdem Aktas <erdemaktas@google.com>;
> Gerd
> >>>> Hoffmann <kraxel@redhat.com>; Guo Dong <guo.dong@intel.com>; Hao
> A
> >>>> Wu <hao.a.wu@intel.com>; James Bottomley <jejb@linux.ibm.com>;
> Jian J
> >>>> Wang <jian.j.wang@intel.com>; Jiewen Yao <jiewen.yao@intel.com>;
> Jordan
> >>>> Justen <jordan.l.justen@intel.com>; Julien Grall <julien@xen.org>; Leif
> >>>> Lindholm <quic_llindhol@quicinc.com>; Liming Gao
> >>>> <gaoliming@byosoft.com.cn>; Maurice Ma <maurice.ma@intel.com>;
> Min Xu
> >>>> <min.m.xu@intel.com>; Nickle Wang <nickle.wang@hpe.com>; Peter
> Grehan
> >>>> <grehan@freebsd.org>; Ray Ni <ray.ni@intel.com>; Rebecca Cran
> >>>> <rebecca@bsdio.com>; Sami Mujawar <sami.mujawar@arm.com>;
> Sean
> >>>> Rhodes <sean@starlabs.systems>; Sebastien Boeuf
> >>>> <sebastien.boeuf@intel.com>; Tom Lendacky
> <thomas.lendacky@amd.com>
> >>>> 主题: Re: [edk2-devel] [PATCH v5 0/8] Add Variable Flash Info HOB
> >>>>
> >>>> I agree that would be a useful tool and in the case of changes such as
> >>>> this that provide backward compatibility with existing functionality,
> >>>> particularly helpful.
> >>>>
> >>>> Some packages such as MdePkg
> >>>>
> (https://github.com/tianocore/edk2/blob/master/MdePkg/MdeLibs.dsc.inc)
> >>>> and NetworkPkg
> >>>>
> (https://github.com/tianocore/edk2/blob/master/NetworkPkg/NetworkCom
> >>>> ponents.dsc.inc)
> >>>> provide DSC files that a platform can override if necessary.
> >>>>
> >>>> However, this does not exist for all edk2 packages. I did not introduce
> >>>> such a file in MdeModulePkg because I believe that is an independent
> >>>> package design decision outside the scope of this series and, if that
> >>>> change was made, it should include libraries other than just this
> >>>> instance. That would lead to additional churn and a larger platform
> >>>> integration debate, important to that topic, but separate from the
> >>>> current state this contribution is based against.
> >>>>
> >>>> While includes be helpful, it can encourage platform owners to ignore
> >>>> potential creep in functionality they should be aware of.
> >>>>
> >>>> For example, the DSC update is mostly being given to platforms to fix
> >>>> their immediate build problem. But, a platform owner might also choose
> >>>> to update their FVB driver to use this interface to get flash
> >>>> information as opposed to directly using PCDs as many do today.
> >>>> That's a
> >>>> decision they need to evaluate and make but they should be aware of
> the
> >>>> interface and make that decision. By directly reviewing/integrating the
> >>>> change for their platform, they are more explicitly made aware of this
> >>>> new interface to form that decision.
> >>>>
> >>>> Also, when many include files get involved, platform build complexity
> >>>> and developer frustration can increase due to nesting and order of
> >>>> include files. Values (library classes, PCDs, etc.) can be overridden
> >>>> more than once. Ultimately, this is technically manageable by utilizing
> >>>> build reports and understanding the EDK II build output in more detail.
> >>>>
> >>>> Again, I think this conversation is useful but requires much more time
> >>>> to address questions such as the following:
> >>>>
> >>>> 1. Should a different mechanism for default library classes be
> >>>> introduced?
> >>>>
> >>>> 2. Should all packages in edk2 provide such an include file? If so,
> >>>> does
> >>>> it only provide the DSC file (like MdePkg) or other files (like
> >>>> NetworkPkg which includes FDF)?
> >>>>
> >>>> 3. Which library classes for a given package should be given default
> >>>> instances?
> >>>>
> >>>> 4. How would each platform package maintainer like to maintain their
> >>>> platform build?
> >>>>
> >>>> Example: Would MinPlatformPkg prefer to keep its own "core" include
> >>>> files
> >>>>
> (https://github.com/tianocore/edk2-platforms/tree/master/Platform/Intel/Mi
> >>>>
> >>>> nPlatformPkg/Include)
> >>>> or reconcile them with include files (possibly introducing an
> >>>> additional
> >>>> layer of nesting)? Would others prefer not to use includes at all to
> >>>> keep a flat, simpler file?
> >>>>
> >>>> How would someone updating the platform code due to an edk2 change
> be
> >>>> aware of this per-platform policy?
> >>>>
> >>>> To my understanding, the answers to these today are:
> >>>>
> >>>> 1. No
> >>>> 2. No / decided on per-change basis
> >>>> 3. Unknown
> >>>> 4. Unknown
> >>>>
> >>>> (2) adds friction to the edk2 contribution process as expectation is
> >>>> unclear.
> >>>>
> >>>> (3) adds churn into the platform integration process as the integration
> >>>> process for existing code is subject to change over time (e.g. realize
> >>>> library class is now in DSC and remove from platform DSC to use include
> >>>> instance).
> >>>>
> >>>> (4) adds friction to the edk2 contribution process as expectation is
> >>>> unclear. Also, somewhat error prone. There's also a level of due
> >>>> diligence needed to confirm if a platform that already has an
> >>>> include is
> >>>> overriding that in another DSC file. If so, is that still the correct
> >>>> behavior or should it be modified.
> >>>>
> >>>> Regards,
> >>>> Michael
> >>>>
> >>>> On 4/29/2022 9:45 AM, Ard Biesheuvel wrote:
> >>>>> On Tue, 26 Apr 2022 at 03:29, <mikuback@linux.microsoft.com>
> wrote:
> >>>>>>
> >>>>>> From: Michael Kubacki <michael.kubacki@microsoft.com>
> >>>>>>
> >>>>>> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3479
> >>>>>>
> >>>>>> Platforms: This series introduces a new library class called
> >>>>>> VariableFlashInfoLib. Platforms using the variable modules will
> >>>>>> need to specify these library classes. See the patches at the
> >>>>>> end of this series for examples of the change needed in the
> >>>>>> platform DSC file. I have attempted to update all open-source
> >>>>>> platforms in edk2 and edk2-platforms in this series and
> >>>>>> https://edk2.groups.io/g/devel/message/89148.
> >>>>>>
> >>>>>
> >>>>> I will make the same remark here as I made in response to the
> >>>>> edk2-platforms series:
> >>>>>
> >>>>> Could we please consider introducing a way to define the default
> >>>>> resolution of a library class? That way, all this churn and
> >>>>> unnecessary breakage would not be necessary, as defining a resolution
> >>>>> would only be necessary when deviating from the default. In fact, we
> >>>>> might be able to clean up some .DSCs considerably by defining defaults
> >>>>> for library classes that only have one (or very few) implementations.
> >>>>>
> >>>>>
> >>>>>> The UEFI variable drivers such as VariableRuntimeDxe, VariableSmm,
> >>>>>> VariableStandaloneMm, etc. (and their dependent protocol/library
> >>>>>> stack), typically acquire UEFI variable store flash information
> >>>>>> with PCDs declared in MdeModulePkg.
> >>>>>>
> >>>>>> For example:
> >>>>>> [Pcd]
> >>>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariabl
> eBase
> >>>>>>
> >>>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64
> >>>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariabl
> eSize
> >>>>>>
> >>>>>> These PCDs work as-is in the StandaloneMm driver if they are not
> >>>>>> dynamic such as Dynamic or DynamicEx because PCD services are not
> >>>>>> readily available in the Standalone MM environment. Platforms that
> >>>>>> use Standalone MM today, must define these PCDs as FixedAtBuild in
> >>>>>> their platform build. However, the PCDs do allow platforms to treat
> >>>>>> the PCDs as Dynamic/DynamicEx and being able to support that is
> >>>>>> currently a gap for Standalone MM.
> >>>>>>
> >>>>>> This patch series introduces a HOB that can be produced by the
> >>>>>> platform to provide the same information. The HOB list is
> >>>>>> available to Standalone MM.
> >>>>>>
> >>>>>> The PCD declarations are left as-is in MdeModulePkg for backward
> >>>>>> compatibility. This means unless a platform wants to use the HOB,
> >>>>>> their code will continue to work with no change (they do not need
> >>>>>> to produce the HOB). Only if the HOB is found, is its value used
> >>>>>> instead of the PCDs.
> >>>>>>
> >>>>>> Due to the large number of consumers of this information, access
> >>>>>> to the base address and size values is abstracted in a new library
> >>>>>> class (as requested in the v1 series) called VariableFlashInfoLib.
> >>>>>>
> >>>>>> The API of VariableFlashInfoLib does not bind the underlying data
> >>>>>> structure to the information returned to library users to allow
> >>>>>> flexibility in the library implementation in the future.
> >>>>>>
> >>>>>> V5 changes.
> >>>>>> 1. Made GetVariableFlashInfoFromHob() in
> BaseVariableFlashInfoLib.c
> >>>>>> STATIC.
> >>>>>> 2. Added a section in commit v5 [3/8] to explicitly state that the
> >>>>>> commit introduces a dependency that requires a change in
> >>>>>> platform
> >>>>>> build. Please see patches v5 [5/8] - v5 [8/8] for examples of
> >>>>>> how to integrate this change (add VariableFlashInfoLib library
> >>>>>> to DSC file).
> >>>>>>
> >>>>>> V4 changes:
> >>>>>> 1. Add a UINT32 "Reserved" field to VARIABLE_FLASH_INFO.
> >>>>>> 2. Add a descriptive comment to VariableFlashInfo.h to explain
> >>>>>> HOB usage.
> >>>>>>
> >>>>>> V3 changes:
> >>>>>> 1. To better clarify usage, renamed the members
> >>>>>> "NvStorageBaseAddress" and "NvStorageLength" in
> >>>>>> "VARIABLE_FLASH_INFO" to "NvVariableBaseAddress" and
> >>>>>> "NvVariableLength".
> >>>>>> 2. Added description comments to the fields in
> "VARIABLE_FLASH_INFO".
> >>>>>>
> >>>>>> V2 changes:
> >>>>>> 1. Abstracted flash info data access with VariableFlashInfoLib.
> >>>>>> 2. Updated package builds in the repo that build the variable and
> >>>>>> FTW drivers to include VariableFlashInfoLib.
> >>>>>> 3. Removed a redundant variable assignment in VariableSmm.c.
> >>>>>> 4. Updated comments in FtwMisc.c and FaultTolerantWritePei.c to
> >>>>>> indicate driver assumption is UINTN (not UINT32)
> >>>>>> 5. Added a version field to the VARIABLE_FLASH_INFO structure.
> >>>>>>
> >>>>>> Cc: Abner Chang <abner.chang@hpe.com>
> >>>>>> Cc: Andrew Fish <afish@apple.com>
> >>>>>> Cc: Anthony Perard <anthony.perard@citrix.com>
> >>>>>> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> >>>>>> Cc: Benjamin You <benjamin.you@intel.com>
> >>>>>> Cc: Brijesh Singh <brijesh.singh@amd.com>
> >>>>>> Cc: Erdem Aktas <erdemaktas@google.com>
> >>>>>> Cc: Gerd Hoffmann <kraxel@redhat.com>
> >>>>>> Cc: Guo Dong <guo.dong@intel.com>
> >>>>>> Cc: Hao A Wu <hao.a.wu@intel.com>
> >>>>>> Cc: James Bottomley <jejb@linux.ibm.com>
> >>>>>> Cc: Jian J Wang <jian.j.wang@intel.com>
> >>>>>> Cc: Jiewen Yao <jiewen.yao@intel.com>
> >>>>>> Cc: Jordan Justen <jordan.l.justen@intel.com>
> >>>>>> Cc: Julien Grall <julien@xen.org>
> >>>>>> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> >>>>>> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> >>>>>> Cc: Maurice Ma <maurice.ma@intel.com>
> >>>>>> Cc: Min Xu <min.m.xu@intel.com>
> >>>>>> Cc: Nickle Wang <nickle.wang@hpe.com>
> >>>>>> Cc: Peter Grehan <grehan@freebsd.org>
> >>>>>> Cc: Ray Ni <ray.ni@intel.com>
> >>>>>> Cc: Rebecca Cran <rebecca@bsdio.com>
> >>>>>> Cc: Sami Mujawar <sami.mujawar@arm.com>
> >>>>>> Cc: Sean Rhodes <sean@starlabs.systems>
> >>>>>> Cc: Sebastien Boeuf <sebastien.boeuf@intel.com>
> >>>>>> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> >>>>>> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> >>>>>>
> >>>>>> Michael Kubacki (8):
> >>>>>> MdeModulePkg: Add Variable Flash Info HOB
> >>>>>> MdeModulePkg/VariableFlashInfoLib: Add initial library
> >>>>>> MdeModulePkg/Variable: Consume Variable Flash Info
> >>>>>> MdeModulePkg/FaultTolerantWrite: Consume Variable Flash
> Info
> >>>>>> ArmVirtPkg/ArmVirt.dsc.inc: Add VariableFlashInfoLib
> >>>>>> EmulatorPkg: Add VariableFlashInfoLib
> >>>>>> OvmfPkg: Add VariableFlashInfoLib
> >>>>>> UefiPayloadPkg: Add VariableFlashInfoLib
> >>>>>>
> >>>>>>
> >>>>
> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.c
> >>>>
> >>>> | 179 ++++++++++++++++++++
> >>>>>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c
> >>>> | 41 +++--
> >>>>>>
> >>>>
> MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c
> >>>> | 7 +-
> >>>>>>
> >>>>
> MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.c
> >>>> | 28 +--
> >>>>>> MdeModulePkg/Universal/Variable/Pei/Variable.c
> >>>> | 14 +-
> >>>>>> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
> >>>> | 16 +-
> >>>>>> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVo
> latile.c
> >>>> | 14 +-
> >>>>>> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> >>>> | 17 +-
> >>>>>> ArmVirtPkg/ArmVirt.dsc.inc
> >>>> | 1 +
> >>>>>> EmulatorPkg/EmulatorPkg.dsc
> >>>> | 1 +
> >>>>>> MdeModulePkg/Include/Guid/VariableFlashInfo.h
> >>>> | 111 ++++++++++++
> >>>>>> MdeModulePkg/Include/Library/VariableFlashInfoLib.h
> >>>> | 68 ++++++++
> >>>>>>
> >>>>
> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.i
> >>>>
> >>>> nf | 48 ++++++
> >>>>>>
> >>>>
> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.u
> >>>>
> >>>> ni | 12 ++
> >>>>>> MdeModulePkg/MdeModulePkg.dec
> >>>> | 8 +
> >>>>>> MdeModulePkg/MdeModulePkg.dsc
> >>>> | 2 +
> >>>>>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerant
> Write.h
> >>>> | 7 +-
> >>>>>>
> >>>>
> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
> >>>> | 10 +-
> >>>>>>
> >>>>
> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf
> >>>> | 10 +-
> >>>>>>
> >>>>
> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal
> >>>> oneMm.inf | 10 +-
> >>>>>>
> >>>>
> MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
> >>>> | 10 +-
> >>>>>> MdeModulePkg/Universal/Variable/Pei/Variable.h
> >>>> | 2 +
> >>>>>> MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
> >>>> | 5 +-
> >>>>>> MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
> >>>> | 7 +-
> >>>>>>
> >>>>
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> >>>> | 5 +-
> >>>>>> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.i
> nf
> >>>> | 5 +-
> >>>>>>
> >>>>
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
> >>>> | 5 +-
> >>>>>> OvmfPkg/AmdSev/AmdSevX64.dsc
> >>>> | 1 +
> >>>>>> OvmfPkg/Bhyve/BhyveX64.dsc
> >>>> | 1 +
> >>>>>> OvmfPkg/CloudHv/CloudHvX64.dsc
> >>>> | 1 +
> >>>>>> OvmfPkg/IntelTdx/IntelTdxX64.dsc
> >>>> | 1 +
> >>>>>> OvmfPkg/Microvm/MicrovmX64.dsc
> >>>> | 1 +
> >>>>>> OvmfPkg/OvmfPkgIa32.dsc
> >>>> | 1 +
> >>>>>> OvmfPkg/OvmfPkgIa32X64.dsc
> >>>> | 1 +
> >>>>>> OvmfPkg/OvmfPkgX64.dsc
> >>>> | 1 +
> >>>>>> OvmfPkg/OvmfXen.dsc
> >>>> | 1 +
> >>>>>> UefiPayloadPkg/UefiPayloadPkg.dsc
> >>>> | 1 +
> >>>>>> 37 files changed, 559 insertions(+), 94 deletions(-)
> >>>>>> create mode 100644
> >>>>
> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.c
> >>>>
> >>>>>> create mode 100644
> MdeModulePkg/Include/Guid/VariableFlashInfo.h
> >>>>>> create mode 100644
> >>>> MdeModulePkg/Include/Library/VariableFlashInfoLib.h
> >>>>>> create mode 100644
> >>>>
> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.i
> >>>>
> >>>> nf
> >>>>>> create mode 100644
> >>>>
> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.u
> >>>>
> >>>> ni
> >>>>>>
> >>>>>> --
> >>>>>> 2.28.0.windows.1
> >>>>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>
> >>>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: 回复: 回复: [edk2-devel] [PATCH v5 0/8] Add Variable Flash Info HOB
2022-05-14 1:16 ` 回复: " gaoliming
@ 2022-05-16 15:27 ` Michael Kubacki
2022-05-16 17:36 ` Ard Biesheuvel
0 siblings, 1 reply; 26+ messages in thread
From: Michael Kubacki @ 2022-05-16 15:27 UTC (permalink / raw)
To: gaoliming, devel, 'Ard Biesheuvel'
Cc: 'Abner Chang', 'Andrew Fish',
'Anthony Perard', 'Ard Biesheuvel',
'Benjamin You', 'Brijesh Singh',
'Erdem Aktas', 'Gerd Hoffmann',
'Guo Dong', 'Hao A Wu', 'James Bottomley',
'Jian J Wang', 'Jiewen Yao',
'Jordan Justen', 'Julien Grall',
'Leif Lindholm', 'Maurice Ma', 'Min Xu',
'Nickle Wang', 'Peter Grehan', 'Ray Ni',
'Rebecca Cran', 'Sami Mujawar',
'Sean Rhodes', 'Sebastien Boeuf',
'Tom Lendacky'
Yes, it has been reviewed by all maintainers. An announcement of the
change was sent to edk2 on April 29th:
https://edk2.groups.io/g/announce/message/291
The series for edk2-platforms has also been out since April 25th:
https://edk2.groups.io/g/devel/message/89308
Thanks,
Michael
On 5/13/2022 9:16 PM, gaoliming wrote:
> Michael:
> This is my suggestion to resolve such compatible issue. As you say, it needs more review and discussion. So, it may not be applied immediately.
>
> If this patch set needs to catch this table tag, it has to take current way to update each DSC file. Have the patch set got reviewed-by from Package maintainers? If yes, I think this patch set can still be merged for this stable tag.
>
> Thanks
> Liming
>> -----邮件原件-----
>> 发件人: Michael Kubacki <mikuback@linux.microsoft.com>
>> 发送时间: 2022年5月14日 2:24
>> 收件人: gaoliming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io; 'Ard
>> Biesheuvel' <ardb@kernel.org>
>> 抄送: 'Abner Chang' <abner.chang@hpe.com>; 'Andrew Fish'
>> <afish@apple.com>; 'Anthony Perard' <anthony.perard@citrix.com>; 'Ard
>> Biesheuvel' <ardb+tianocore@kernel.org>; 'Benjamin You'
>> <benjamin.you@intel.com>; 'Brijesh Singh' <brijesh.singh@amd.com>; 'Erdem
>> Aktas' <erdemaktas@google.com>; 'Gerd Hoffmann' <kraxel@redhat.com>;
>> 'Guo Dong' <guo.dong@intel.com>; 'Hao A Wu' <hao.a.wu@intel.com>;
>> 'James Bottomley' <jejb@linux.ibm.com>; 'Jian J Wang'
>> <jian.j.wang@intel.com>; 'Jiewen Yao' <jiewen.yao@intel.com>; 'Jordan
>> Justen' <jordan.l.justen@intel.com>; 'Julien Grall' <julien@xen.org>; 'Leif
>> Lindholm' <quic_llindhol@quicinc.com>; 'Maurice Ma'
>> <maurice.ma@intel.com>; 'Min Xu' <min.m.xu@intel.com>; 'Nickle Wang'
>> <nickle.wang@hpe.com>; 'Peter Grehan' <grehan@freebsd.org>; 'Ray Ni'
>> <ray.ni@intel.com>; 'Rebecca Cran' <rebecca@bsdio.com>; 'Sami Mujawar'
>> <sami.mujawar@arm.com>; 'Sean Rhodes' <sean@starlabs.systems>;
>> 'Sebastien Boeuf' <sebastien.boeuf@intel.com>; 'Tom Lendacky'
>> <thomas.lendacky@amd.com>
>> 主题: Re: 回复: [edk2-devel] [PATCH v5 0/8] Add Variable Flash Info HOB
>>
>> Can you please respond with your preference?
>>
>> I am ready to do this but if it is required now, it should be documented
>> so it becomes a consistent pattern for future changes.
>>
>> Thanks,
>> Michael
>>
>> On 5/10/2022 11:01 AM, Michael Kubacki wrote:
>>> What's the plan for next steps? The v5 PR has been up for two weeks with
>>> no changes.
>>>
>>> Are we going to try to define a long-term pattern for how to include new
>>> library classes in core packages or merge the patch series?
>>>
>>> Thanks,
>>> Michael
>>>
>>> On 5/5/2022 9:52 PM, Michael Kubacki wrote:
>>>> I still believe a long term design pattern deserves more focus and
>>>> documentation than a quick modification to this series.
>>>>
>>>> Can you confirm that you envision MdePkg/MdeLibs.dsc.inc serving as a
>>>> monolithic host of various other default library class instances?
>>>>
>>>> That somewhat inverts the package relationships, the code reviewer
>>>> policy would need to clarify when the original package owners are
>>>> included on the MdePkg patch (to confirm they agree with the default
>>>> instance choice), and "core" packages would have to be clearly defined
>>>> in this context for developers to know what packages are allowed.
>>>>
>>>> In addition, this does not mean there still won't be some level of
>>>> platform integration thrash. For example, if a new library class
>>>> instance added to MdePkg/MdeLibs.dsc.inc requires another library
>>>> class (or multiple others), those might not be added to the DSC
>>>> include file. They could have been satisfied in the original package
>>>> DSC (or a test platform DSC) but that doesn't mean they will be in all
>>>> platform DSC files. So when the MdeLibs.dsc.inc file update occurs,
>>>> those platforms break and need to add the library class that was
>>>> already specified in other DSC files.
>>>>
>>>> So I request that if this is the preferred approach, that it be agreed
>>>> upon (e.g. dedicated RFC), documented, and consistently followed by
>>>> other contributions as well.
>>>>
>>>> Regards,
>>>> Michael
>>>>
>>>> On 5/4/2022 9:27 PM, gaoliming wrote:
>>>>> Michael:
>>>>> I would suggest to reuse MdePkg/MdeLibs.dsc.inc to list the
>>>>> library and PCD from the edk2 core packages, such as MdePkg,
>>>>> MdeModulePkg, CryptoPkg, SecurirtyPkg and so on. Those packages are
>>>>> required by every platforms. They can't be separated. So, I think
>>>>> MdePkg/MdeLibs.dsc.inc is for edk2 core packages, not only for MdePkg.
>>>>>
>>>>> Thanks
>>>>> Liming
>>>>>> -----邮件原件-----
>>>>>> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表
>> Michael
>>>>>> Kubacki
>>>>>> 发送时间: 2022年4月29日 23:48
>>>>>> 收件人: Ard Biesheuvel <ardb@kernel.org>
>>>>>> 抄送: edk2-devel-groups-io <devel@edk2.groups.io>; Abner Chang
>>>>>> <abner.chang@hpe.com>; Andrew Fish <afish@apple.com>; Anthony
>> Perard
>>>>>> <anthony.perard@citrix.com>; Ard Biesheuvel
>>>>>> <ardb+tianocore@kernel.org>;
>>>>>> Benjamin You <benjamin.you@intel.com>; Brijesh Singh
>>>>>> <brijesh.singh@amd.com>; Erdem Aktas <erdemaktas@google.com>;
>> Gerd
>>>>>> Hoffmann <kraxel@redhat.com>; Guo Dong <guo.dong@intel.com>; Hao
>> A
>>>>>> Wu <hao.a.wu@intel.com>; James Bottomley <jejb@linux.ibm.com>;
>> Jian J
>>>>>> Wang <jian.j.wang@intel.com>; Jiewen Yao <jiewen.yao@intel.com>;
>> Jordan
>>>>>> Justen <jordan.l.justen@intel.com>; Julien Grall <julien@xen.org>; Leif
>>>>>> Lindholm <quic_llindhol@quicinc.com>; Liming Gao
>>>>>> <gaoliming@byosoft.com.cn>; Maurice Ma <maurice.ma@intel.com>;
>> Min Xu
>>>>>> <min.m.xu@intel.com>; Nickle Wang <nickle.wang@hpe.com>; Peter
>> Grehan
>>>>>> <grehan@freebsd.org>; Ray Ni <ray.ni@intel.com>; Rebecca Cran
>>>>>> <rebecca@bsdio.com>; Sami Mujawar <sami.mujawar@arm.com>;
>> Sean
>>>>>> Rhodes <sean@starlabs.systems>; Sebastien Boeuf
>>>>>> <sebastien.boeuf@intel.com>; Tom Lendacky
>> <thomas.lendacky@amd.com>
>>>>>> 主题: Re: [edk2-devel] [PATCH v5 0/8] Add Variable Flash Info HOB
>>>>>>
>>>>>> I agree that would be a useful tool and in the case of changes such as
>>>>>> this that provide backward compatibility with existing functionality,
>>>>>> particularly helpful.
>>>>>>
>>>>>> Some packages such as MdePkg
>>>>>>
>> (https://github.com/tianocore/edk2/blob/master/MdePkg/MdeLibs.dsc.inc)
>>>>>> and NetworkPkg
>>>>>>
>> (https://github.com/tianocore/edk2/blob/master/NetworkPkg/NetworkCom
>>>>>> ponents.dsc.inc)
>>>>>> provide DSC files that a platform can override if necessary.
>>>>>>
>>>>>> However, this does not exist for all edk2 packages. I did not introduce
>>>>>> such a file in MdeModulePkg because I believe that is an independent
>>>>>> package design decision outside the scope of this series and, if that
>>>>>> change was made, it should include libraries other than just this
>>>>>> instance. That would lead to additional churn and a larger platform
>>>>>> integration debate, important to that topic, but separate from the
>>>>>> current state this contribution is based against.
>>>>>>
>>>>>> While includes be helpful, it can encourage platform owners to ignore
>>>>>> potential creep in functionality they should be aware of.
>>>>>>
>>>>>> For example, the DSC update is mostly being given to platforms to fix
>>>>>> their immediate build problem. But, a platform owner might also choose
>>>>>> to update their FVB driver to use this interface to get flash
>>>>>> information as opposed to directly using PCDs as many do today.
>>>>>> That's a
>>>>>> decision they need to evaluate and make but they should be aware of
>> the
>>>>>> interface and make that decision. By directly reviewing/integrating the
>>>>>> change for their platform, they are more explicitly made aware of this
>>>>>> new interface to form that decision.
>>>>>>
>>>>>> Also, when many include files get involved, platform build complexity
>>>>>> and developer frustration can increase due to nesting and order of
>>>>>> include files. Values (library classes, PCDs, etc.) can be overridden
>>>>>> more than once. Ultimately, this is technically manageable by utilizing
>>>>>> build reports and understanding the EDK II build output in more detail.
>>>>>>
>>>>>> Again, I think this conversation is useful but requires much more time
>>>>>> to address questions such as the following:
>>>>>>
>>>>>> 1. Should a different mechanism for default library classes be
>>>>>> introduced?
>>>>>>
>>>>>> 2. Should all packages in edk2 provide such an include file? If so,
>>>>>> does
>>>>>> it only provide the DSC file (like MdePkg) or other files (like
>>>>>> NetworkPkg which includes FDF)?
>>>>>>
>>>>>> 3. Which library classes for a given package should be given default
>>>>>> instances?
>>>>>>
>>>>>> 4. How would each platform package maintainer like to maintain their
>>>>>> platform build?
>>>>>>
>>>>>> Example: Would MinPlatformPkg prefer to keep its own "core" include
>>>>>> files
>>>>>>
>> (https://github.com/tianocore/edk2-platforms/tree/master/Platform/Intel/Mi
>>>>>>
>>>>>> nPlatformPkg/Include)
>>>>>> or reconcile them with include files (possibly introducing an
>>>>>> additional
>>>>>> layer of nesting)? Would others prefer not to use includes at all to
>>>>>> keep a flat, simpler file?
>>>>>>
>>>>>> How would someone updating the platform code due to an edk2 change
>> be
>>>>>> aware of this per-platform policy?
>>>>>>
>>>>>> To my understanding, the answers to these today are:
>>>>>>
>>>>>> 1. No
>>>>>> 2. No / decided on per-change basis
>>>>>> 3. Unknown
>>>>>> 4. Unknown
>>>>>>
>>>>>> (2) adds friction to the edk2 contribution process as expectation is
>>>>>> unclear.
>>>>>>
>>>>>> (3) adds churn into the platform integration process as the integration
>>>>>> process for existing code is subject to change over time (e.g. realize
>>>>>> library class is now in DSC and remove from platform DSC to use include
>>>>>> instance).
>>>>>>
>>>>>> (4) adds friction to the edk2 contribution process as expectation is
>>>>>> unclear. Also, somewhat error prone. There's also a level of due
>>>>>> diligence needed to confirm if a platform that already has an
>>>>>> include is
>>>>>> overriding that in another DSC file. If so, is that still the correct
>>>>>> behavior or should it be modified.
>>>>>>
>>>>>> Regards,
>>>>>> Michael
>>>>>>
>>>>>> On 4/29/2022 9:45 AM, Ard Biesheuvel wrote:
>>>>>>> On Tue, 26 Apr 2022 at 03:29, <mikuback@linux.microsoft.com>
>> wrote:
>>>>>>>>
>>>>>>>> From: Michael Kubacki <michael.kubacki@microsoft.com>
>>>>>>>>
>>>>>>>> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3479
>>>>>>>>
>>>>>>>> Platforms: This series introduces a new library class called
>>>>>>>> VariableFlashInfoLib. Platforms using the variable modules will
>>>>>>>> need to specify these library classes. See the patches at the
>>>>>>>> end of this series for examples of the change needed in the
>>>>>>>> platform DSC file. I have attempted to update all open-source
>>>>>>>> platforms in edk2 and edk2-platforms in this series and
>>>>>>>> https://edk2.groups.io/g/devel/message/89148.
>>>>>>>>
>>>>>>>
>>>>>>> I will make the same remark here as I made in response to the
>>>>>>> edk2-platforms series:
>>>>>>>
>>>>>>> Could we please consider introducing a way to define the default
>>>>>>> resolution of a library class? That way, all this churn and
>>>>>>> unnecessary breakage would not be necessary, as defining a resolution
>>>>>>> would only be necessary when deviating from the default. In fact, we
>>>>>>> might be able to clean up some .DSCs considerably by defining defaults
>>>>>>> for library classes that only have one (or very few) implementations.
>>>>>>>
>>>>>>>
>>>>>>>> The UEFI variable drivers such as VariableRuntimeDxe, VariableSmm,
>>>>>>>> VariableStandaloneMm, etc. (and their dependent protocol/library
>>>>>>>> stack), typically acquire UEFI variable store flash information
>>>>>>>> with PCDs declared in MdeModulePkg.
>>>>>>>>
>>>>>>>> For example:
>>>>>>>> [Pcd]
>>>>>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariabl
>> eBase
>>>>>>>>
>>>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64
>>>>>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariabl
>> eSize
>>>>>>>>
>>>>>>>> These PCDs work as-is in the StandaloneMm driver if they are not
>>>>>>>> dynamic such as Dynamic or DynamicEx because PCD services are not
>>>>>>>> readily available in the Standalone MM environment. Platforms that
>>>>>>>> use Standalone MM today, must define these PCDs as FixedAtBuild in
>>>>>>>> their platform build. However, the PCDs do allow platforms to treat
>>>>>>>> the PCDs as Dynamic/DynamicEx and being able to support that is
>>>>>>>> currently a gap for Standalone MM.
>>>>>>>>
>>>>>>>> This patch series introduces a HOB that can be produced by the
>>>>>>>> platform to provide the same information. The HOB list is
>>>>>>>> available to Standalone MM.
>>>>>>>>
>>>>>>>> The PCD declarations are left as-is in MdeModulePkg for backward
>>>>>>>> compatibility. This means unless a platform wants to use the HOB,
>>>>>>>> their code will continue to work with no change (they do not need
>>>>>>>> to produce the HOB). Only if the HOB is found, is its value used
>>>>>>>> instead of the PCDs.
>>>>>>>>
>>>>>>>> Due to the large number of consumers of this information, access
>>>>>>>> to the base address and size values is abstracted in a new library
>>>>>>>> class (as requested in the v1 series) called VariableFlashInfoLib.
>>>>>>>>
>>>>>>>> The API of VariableFlashInfoLib does not bind the underlying data
>>>>>>>> structure to the information returned to library users to allow
>>>>>>>> flexibility in the library implementation in the future.
>>>>>>>>
>>>>>>>> V5 changes.
>>>>>>>> 1. Made GetVariableFlashInfoFromHob() in
>> BaseVariableFlashInfoLib.c
>>>>>>>> STATIC.
>>>>>>>> 2. Added a section in commit v5 [3/8] to explicitly state that the
>>>>>>>> commit introduces a dependency that requires a change in
>>>>>>>> platform
>>>>>>>> build. Please see patches v5 [5/8] - v5 [8/8] for examples of
>>>>>>>> how to integrate this change (add VariableFlashInfoLib library
>>>>>>>> to DSC file).
>>>>>>>>
>>>>>>>> V4 changes:
>>>>>>>> 1. Add a UINT32 "Reserved" field to VARIABLE_FLASH_INFO.
>>>>>>>> 2. Add a descriptive comment to VariableFlashInfo.h to explain
>>>>>>>> HOB usage.
>>>>>>>>
>>>>>>>> V3 changes:
>>>>>>>> 1. To better clarify usage, renamed the members
>>>>>>>> "NvStorageBaseAddress" and "NvStorageLength" in
>>>>>>>> "VARIABLE_FLASH_INFO" to "NvVariableBaseAddress" and
>>>>>>>> "NvVariableLength".
>>>>>>>> 2. Added description comments to the fields in
>> "VARIABLE_FLASH_INFO".
>>>>>>>>
>>>>>>>> V2 changes:
>>>>>>>> 1. Abstracted flash info data access with VariableFlashInfoLib.
>>>>>>>> 2. Updated package builds in the repo that build the variable and
>>>>>>>> FTW drivers to include VariableFlashInfoLib.
>>>>>>>> 3. Removed a redundant variable assignment in VariableSmm.c.
>>>>>>>> 4. Updated comments in FtwMisc.c and FaultTolerantWritePei.c to
>>>>>>>> indicate driver assumption is UINTN (not UINT32)
>>>>>>>> 5. Added a version field to the VARIABLE_FLASH_INFO structure.
>>>>>>>>
>>>>>>>> Cc: Abner Chang <abner.chang@hpe.com>
>>>>>>>> Cc: Andrew Fish <afish@apple.com>
>>>>>>>> Cc: Anthony Perard <anthony.perard@citrix.com>
>>>>>>>> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
>>>>>>>> Cc: Benjamin You <benjamin.you@intel.com>
>>>>>>>> Cc: Brijesh Singh <brijesh.singh@amd.com>
>>>>>>>> Cc: Erdem Aktas <erdemaktas@google.com>
>>>>>>>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>>>>>>>> Cc: Guo Dong <guo.dong@intel.com>
>>>>>>>> Cc: Hao A Wu <hao.a.wu@intel.com>
>>>>>>>> Cc: James Bottomley <jejb@linux.ibm.com>
>>>>>>>> Cc: Jian J Wang <jian.j.wang@intel.com>
>>>>>>>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>>>>>>>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>>>>>>>> Cc: Julien Grall <julien@xen.org>
>>>>>>>> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
>>>>>>>> Cc: Liming Gao <gaoliming@byosoft.com.cn>
>>>>>>>> Cc: Maurice Ma <maurice.ma@intel.com>
>>>>>>>> Cc: Min Xu <min.m.xu@intel.com>
>>>>>>>> Cc: Nickle Wang <nickle.wang@hpe.com>
>>>>>>>> Cc: Peter Grehan <grehan@freebsd.org>
>>>>>>>> Cc: Ray Ni <ray.ni@intel.com>
>>>>>>>> Cc: Rebecca Cran <rebecca@bsdio.com>
>>>>>>>> Cc: Sami Mujawar <sami.mujawar@arm.com>
>>>>>>>> Cc: Sean Rhodes <sean@starlabs.systems>
>>>>>>>> Cc: Sebastien Boeuf <sebastien.boeuf@intel.com>
>>>>>>>> Cc: Tom Lendacky <thomas.lendacky@amd.com>
>>>>>>>> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
>>>>>>>>
>>>>>>>> Michael Kubacki (8):
>>>>>>>> MdeModulePkg: Add Variable Flash Info HOB
>>>>>>>> MdeModulePkg/VariableFlashInfoLib: Add initial library
>>>>>>>> MdeModulePkg/Variable: Consume Variable Flash Info
>>>>>>>> MdeModulePkg/FaultTolerantWrite: Consume Variable Flash
>> Info
>>>>>>>> ArmVirtPkg/ArmVirt.dsc.inc: Add VariableFlashInfoLib
>>>>>>>> EmulatorPkg: Add VariableFlashInfoLib
>>>>>>>> OvmfPkg: Add VariableFlashInfoLib
>>>>>>>> UefiPayloadPkg: Add VariableFlashInfoLib
>>>>>>>>
>>>>>>>>
>>>>>>
>> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.c
>>>>>>
>>>>>> | 179 ++++++++++++++++++++
>>>>>>>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c
>>>>>> | 41 +++--
>>>>>>>>
>>>>>>
>> MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c
>>>>>> | 7 +-
>>>>>>>>
>>>>>>
>> MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.c
>>>>>> | 28 +--
>>>>>>>> MdeModulePkg/Universal/Variable/Pei/Variable.c
>>>>>> | 14 +-
>>>>>>>> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
>>>>>> | 16 +-
>>>>>>>> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVo
>> latile.c
>>>>>> | 14 +-
>>>>>>>> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
>>>>>> | 17 +-
>>>>>>>> ArmVirtPkg/ArmVirt.dsc.inc
>>>>>> | 1 +
>>>>>>>> EmulatorPkg/EmulatorPkg.dsc
>>>>>> | 1 +
>>>>>>>> MdeModulePkg/Include/Guid/VariableFlashInfo.h
>>>>>> | 111 ++++++++++++
>>>>>>>> MdeModulePkg/Include/Library/VariableFlashInfoLib.h
>>>>>> | 68 ++++++++
>>>>>>>>
>>>>>>
>> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.i
>>>>>>
>>>>>> nf | 48 ++++++
>>>>>>>>
>>>>>>
>> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.u
>>>>>>
>>>>>> ni | 12 ++
>>>>>>>> MdeModulePkg/MdeModulePkg.dec
>>>>>> | 8 +
>>>>>>>> MdeModulePkg/MdeModulePkg.dsc
>>>>>> | 2 +
>>>>>>>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerant
>> Write.h
>>>>>> | 7 +-
>>>>>>>>
>>>>>>
>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
>>>>>> | 10 +-
>>>>>>>>
>>>>>>
>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf
>>>>>> | 10 +-
>>>>>>>>
>>>>>>
>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal
>>>>>> oneMm.inf | 10 +-
>>>>>>>>
>>>>>>
>> MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
>>>>>> | 10 +-
>>>>>>>> MdeModulePkg/Universal/Variable/Pei/Variable.h
>>>>>> | 2 +
>>>>>>>> MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
>>>>>> | 5 +-
>>>>>>>> MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
>>>>>> | 7 +-
>>>>>>>>
>>>>>>
>> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
>>>>>> | 5 +-
>>>>>>>> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.i
>> nf
>>>>>> | 5 +-
>>>>>>>>
>>>>>>
>> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
>>>>>> | 5 +-
>>>>>>>> OvmfPkg/AmdSev/AmdSevX64.dsc
>>>>>> | 1 +
>>>>>>>> OvmfPkg/Bhyve/BhyveX64.dsc
>>>>>> | 1 +
>>>>>>>> OvmfPkg/CloudHv/CloudHvX64.dsc
>>>>>> | 1 +
>>>>>>>> OvmfPkg/IntelTdx/IntelTdxX64.dsc
>>>>>> | 1 +
>>>>>>>> OvmfPkg/Microvm/MicrovmX64.dsc
>>>>>> | 1 +
>>>>>>>> OvmfPkg/OvmfPkgIa32.dsc
>>>>>> | 1 +
>>>>>>>> OvmfPkg/OvmfPkgIa32X64.dsc
>>>>>> | 1 +
>>>>>>>> OvmfPkg/OvmfPkgX64.dsc
>>>>>> | 1 +
>>>>>>>> OvmfPkg/OvmfXen.dsc
>>>>>> | 1 +
>>>>>>>> UefiPayloadPkg/UefiPayloadPkg.dsc
>>>>>> | 1 +
>>>>>>>> 37 files changed, 559 insertions(+), 94 deletions(-)
>>>>>>>> create mode 100644
>>>>>>
>> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.c
>>>>>>
>>>>>>>> create mode 100644
>> MdeModulePkg/Include/Guid/VariableFlashInfo.h
>>>>>>>> create mode 100644
>>>>>> MdeModulePkg/Include/Library/VariableFlashInfoLib.h
>>>>>>>> create mode 100644
>>>>>>
>> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.i
>>>>>>
>>>>>> nf
>>>>>>>> create mode 100644
>>>>>>
>> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.u
>>>>>>
>>>>>> ni
>>>>>>>>
>>>>>>>> --
>>>>>>>> 2.28.0.windows.1
>>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: 回复: 回复: [edk2-devel] [PATCH v5 0/8] Add Variable Flash Info HOB
2022-05-16 15:27 ` Michael Kubacki
@ 2022-05-16 17:36 ` Ard Biesheuvel
2022-05-17 4:14 ` Michael Kubacki
0 siblings, 1 reply; 26+ messages in thread
From: Ard Biesheuvel @ 2022-05-16 17:36 UTC (permalink / raw)
To: Michael Kubacki
Cc: gaoliming, edk2-devel-groups-io, Abner Chang, Andrew Fish,
Anthony Perard, Ard Biesheuvel, Benjamin You, Brijesh Singh,
Erdem Aktas, Gerd Hoffmann, Guo Dong, Hao A Wu, James Bottomley,
Jian J Wang, Jiewen Yao, Jordan Justen, Julien Grall,
Leif Lindholm, Maurice Ma, Min Xu, Nickle Wang, Peter Grehan,
Ray Ni, Rebecca Cran, Sami Mujawar, Sean Rhodes, Sebastien Boeuf,
Tom Lendacky
On Mon, 16 May 2022 at 17:27, Michael Kubacki
<mikuback@linux.microsoft.com> wrote:
>
> Yes, it has been reviewed by all maintainers. An announcement of the
> change was sent to edk2 on April 29th:
> https://edk2.groups.io/g/announce/message/291
>
> The series for edk2-platforms has also been out since April 25th:
> https://edk2.groups.io/g/devel/message/89308
>
> Thanks,
> Michael
>
I am fine with merging this as is, but please merge the edk2-platforms
changes as soon as the edk2 changes are in, so platforms there are not
left in a broken state.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: 回复: 回复: [edk2-devel] [PATCH v5 0/8] Add Variable Flash Info HOB
2022-05-16 17:36 ` Ard Biesheuvel
@ 2022-05-17 4:14 ` Michael Kubacki
2022-05-17 5:22 ` 回复: " gaoliming
0 siblings, 1 reply; 26+ messages in thread
From: Michael Kubacki @ 2022-05-17 4:14 UTC (permalink / raw)
To: devel, ardb
Cc: gaoliming, Abner Chang, Andrew Fish, Anthony Perard,
Ard Biesheuvel, Benjamin You, Brijesh Singh, Erdem Aktas,
Gerd Hoffmann, Guo Dong, Hao A Wu, James Bottomley, Jian J Wang,
Jiewen Yao, Jordan Justen, Julien Grall, Leif Lindholm,
Maurice Ma, Min Xu, Nickle Wang, Peter Grehan, Ray Ni,
Rebecca Cran, Sami Mujawar, Sean Rhodes, Sebastien Boeuf,
Tom Lendacky
Liming, will you push both?
I put a rebased edk2-platforms with all the review tags for that series
here:
https://github.com/makubacki/edk2-platforms/commits/add_variableflashinfolib_to_platforms
Thanks,
Michael
On 5/16/2022 1:36 PM, Ard Biesheuvel wrote:
> On Mon, 16 May 2022 at 17:27, Michael Kubacki
> <mikuback@linux.microsoft.com> wrote:
>>
>> Yes, it has been reviewed by all maintainers. An announcement of the
>> change was sent to edk2 on April 29th:
>> https://edk2.groups.io/g/announce/message/291
>>
>> The series for edk2-platforms has also been out since April 25th:
>> https://edk2.groups.io/g/devel/message/89308
>>
>> Thanks,
>> Michael
>>
>
> I am fine with merging this as is, but please merge the edk2-platforms
> changes as soon as the edk2 changes are in, so platforms there are not
> left in a broken state.
>
>
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* 回复: 回复: 回复: [edk2-devel] [PATCH v5 0/8] Add Variable Flash Info HOB
2022-05-17 4:14 ` Michael Kubacki
@ 2022-05-17 5:22 ` gaoliming
2022-05-17 13:06 ` Michael Kubacki
0 siblings, 1 reply; 26+ messages in thread
From: gaoliming @ 2022-05-17 5:22 UTC (permalink / raw)
To: 'Michael Kubacki', devel, ardb
Cc: 'Abner Chang', 'Andrew Fish',
'Anthony Perard', 'Ard Biesheuvel',
'Benjamin You', 'Brijesh Singh',
'Erdem Aktas', 'Gerd Hoffmann',
'Guo Dong', 'Hao A Wu', 'James Bottomley',
'Jian J Wang', 'Jiewen Yao',
'Jordan Justen', 'Julien Grall',
'Leif Lindholm', 'Maurice Ma', 'Min Xu',
'Nickle Wang', 'Peter Grehan', 'Ray Ni',
'Rebecca Cran', 'Sami Mujawar',
'Sean Rhodes', 'Sebastien Boeuf',
'Tom Lendacky'
Michael:
Yes. I can. Where is the change for Edk2?
Thanks
Liming
> -----邮件原件-----
> 发件人: Michael Kubacki <mikuback@linux.microsoft.com>
> 发送时间: 2022年5月17日 12:14
> 收件人: devel@edk2.groups.io; ardb@kernel.org
> 抄送: gaoliming <gaoliming@byosoft.com.cn>; Abner Chang
> <abner.chang@hpe.com>; Andrew Fish <afish@apple.com>; Anthony Perard
> <anthony.perard@citrix.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>;
> Benjamin You <benjamin.you@intel.com>; Brijesh Singh
> <brijesh.singh@amd.com>; Erdem Aktas <erdemaktas@google.com>; Gerd
> Hoffmann <kraxel@redhat.com>; Guo Dong <guo.dong@intel.com>; Hao A
> Wu <hao.a.wu@intel.com>; James Bottomley <jejb@linux.ibm.com>; Jian J
> Wang <jian.j.wang@intel.com>; Jiewen Yao <jiewen.yao@intel.com>; Jordan
> Justen <jordan.l.justen@intel.com>; Julien Grall <julien@xen.org>; Leif
> Lindholm <quic_llindhol@quicinc.com>; Maurice Ma
> <maurice.ma@intel.com>; Min Xu <min.m.xu@intel.com>; Nickle Wang
> <nickle.wang@hpe.com>; Peter Grehan <grehan@freebsd.org>; Ray Ni
> <ray.ni@intel.com>; Rebecca Cran <rebecca@bsdio.com>; Sami Mujawar
> <sami.mujawar@arm.com>; Sean Rhodes <sean@starlabs.systems>;
> Sebastien Boeuf <sebastien.boeuf@intel.com>; Tom Lendacky
> <thomas.lendacky@amd.com>
> 主题: Re: 回复: 回复: [edk2-devel] [PATCH v5 0/8] Add Variable Flash Info
> HOB
>
> Liming, will you push both?
>
> I put a rebased edk2-platforms with all the review tags for that series
> here:
> https://github.com/makubacki/edk2-platforms/commits/add_variableflashinf
> olib_to_platforms
>
> Thanks,
> Michael
>
> On 5/16/2022 1:36 PM, Ard Biesheuvel wrote:
> > On Mon, 16 May 2022 at 17:27, Michael Kubacki
> > <mikuback@linux.microsoft.com> wrote:
> >>
> >> Yes, it has been reviewed by all maintainers. An announcement of the
> >> change was sent to edk2 on April 29th:
> >> https://edk2.groups.io/g/announce/message/291
> >>
> >> The series for edk2-platforms has also been out since April 25th:
> >> https://edk2.groups.io/g/devel/message/89308
> >>
> >> Thanks,
> >> Michael
> >>
> >
> > I am fine with merging this as is, but please merge the edk2-platforms
> > changes as soon as the edk2 changes are in, so platforms there are not
> > left in a broken state.
> >
> >
> >
> >
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: 回复: 回复: 回复: [edk2-devel] [PATCH v5 0/8] Add Variable Flash Info HOB
2022-05-17 5:22 ` 回复: " gaoliming
@ 2022-05-17 13:06 ` Michael Kubacki
2022-05-17 16:10 ` Michael Kubacki
` (2 more replies)
0 siblings, 3 replies; 26+ messages in thread
From: Michael Kubacki @ 2022-05-17 13:06 UTC (permalink / raw)
To: devel, gaoliming, ardb
Cc: 'Abner Chang', 'Andrew Fish',
'Anthony Perard', 'Ard Biesheuvel',
'Benjamin You', 'Brijesh Singh',
'Erdem Aktas', 'Gerd Hoffmann',
'Guo Dong', 'Hao A Wu', 'James Bottomley',
'Jian J Wang', 'Jiewen Yao',
'Jordan Justen', 'Julien Grall',
'Leif Lindholm', 'Maurice Ma', 'Min Xu',
'Nickle Wang', 'Peter Grehan', 'Ray Ni',
'Rebecca Cran', 'Sami Mujawar',
'Sean Rhodes', 'Sebastien Boeuf',
'Tom Lendacky'
It is here:
https://github.com/tianocore/edk2/pull/2828
I just rebased and pushed the branch.
Thanks,
Michael
On 5/17/2022 1:22 AM, gaoliming wrote:
> Michael:
> Yes. I can. Where is the change for Edk2?
>
> Thanks
> Liming
>> -----邮件原件-----
>> 发件人: Michael Kubacki <mikuback@linux.microsoft.com>
>> 发送时间: 2022年5月17日 12:14
>> 收件人: devel@edk2.groups.io; ardb@kernel.org
>> 抄送: gaoliming <gaoliming@byosoft.com.cn>; Abner Chang
>> <abner.chang@hpe.com>; Andrew Fish <afish@apple.com>; Anthony Perard
>> <anthony.perard@citrix.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>;
>> Benjamin You <benjamin.you@intel.com>; Brijesh Singh
>> <brijesh.singh@amd.com>; Erdem Aktas <erdemaktas@google.com>; Gerd
>> Hoffmann <kraxel@redhat.com>; Guo Dong <guo.dong@intel.com>; Hao A
>> Wu <hao.a.wu@intel.com>; James Bottomley <jejb@linux.ibm.com>; Jian J
>> Wang <jian.j.wang@intel.com>; Jiewen Yao <jiewen.yao@intel.com>; Jordan
>> Justen <jordan.l.justen@intel.com>; Julien Grall <julien@xen.org>; Leif
>> Lindholm <quic_llindhol@quicinc.com>; Maurice Ma
>> <maurice.ma@intel.com>; Min Xu <min.m.xu@intel.com>; Nickle Wang
>> <nickle.wang@hpe.com>; Peter Grehan <grehan@freebsd.org>; Ray Ni
>> <ray.ni@intel.com>; Rebecca Cran <rebecca@bsdio.com>; Sami Mujawar
>> <sami.mujawar@arm.com>; Sean Rhodes <sean@starlabs.systems>;
>> Sebastien Boeuf <sebastien.boeuf@intel.com>; Tom Lendacky
>> <thomas.lendacky@amd.com>
>> 主题: Re: 回复: 回复: [edk2-devel] [PATCH v5 0/8] Add Variable Flash Info
>> HOB
>>
>> Liming, will you push both?
>>
>> I put a rebased edk2-platforms with all the review tags for that series
>> here:
>> https://github.com/makubacki/edk2-platforms/commits/add_variableflashinf
>> olib_to_platforms
>>
>> Thanks,
>> Michael
>>
>> On 5/16/2022 1:36 PM, Ard Biesheuvel wrote:
>>> On Mon, 16 May 2022 at 17:27, Michael Kubacki
>>> <mikuback@linux.microsoft.com> wrote:
>>>>
>>>> Yes, it has been reviewed by all maintainers. An announcement of the
>>>> change was sent to edk2 on April 29th:
>>>> https://edk2.groups.io/g/announce/message/291
>>>>
>>>> The series for edk2-platforms has also been out since April 25th:
>>>> https://edk2.groups.io/g/devel/message/89308
>>>>
>>>> Thanks,
>>>> Michael
>>>>
>>>
>>> I am fine with merging this as is, but please merge the edk2-platforms
>>> changes as soon as the edk2 changes are in, so platforms there are not
>>> left in a broken state.
>>>
>>>
>>>
>>>
>
>
>
>
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: 回复: 回复: 回复: [edk2-devel] [PATCH v5 0/8] Add Variable Flash Info HOB
2022-05-17 13:06 ` Michael Kubacki
@ 2022-05-17 16:10 ` Michael Kubacki
2022-05-19 3:45 ` 回复: " gaoliming
[not found] ` <16F064E8C9D4EA0D.2722@groups.io>
2 siblings, 0 replies; 26+ messages in thread
From: Michael Kubacki @ 2022-05-17 16:10 UTC (permalink / raw)
To: devel, gaoliming, ardb
Cc: 'Abner Chang', 'Andrew Fish',
'Anthony Perard', 'Ard Biesheuvel',
'Benjamin You', 'Brijesh Singh',
'Erdem Aktas', 'Gerd Hoffmann',
'Guo Dong', 'Hao A Wu', 'James Bottomley',
'Jian J Wang', 'Jiewen Yao',
'Jordan Justen', 'Julien Grall',
'Leif Lindholm', 'Maurice Ma', 'Min Xu',
'Nickle Wang', 'Peter Grehan', 'Ray Ni',
'Rebecca Cran', 'Sami Mujawar',
'Sean Rhodes', 'Sebastien Boeuf',
'Tom Lendacky'
A lot of new spelling errors began appearing in various packages in that
PR (unrelated to this change).
As a community service to unblock edk2 PRs, I have sent the following
series to address these newly reported errors:
https://edk2.groups.io/g/devel/message/89833
The PR with the CI results is available here:
https://github.com/tianocore/edk2/pull/2903
Please help prioritize reviewing this so we can unblock changes.
Thanks,
Michael
On 5/17/2022 9:06 AM, Michael Kubacki wrote:
> It is here:
> https://github.com/tianocore/edk2/pull/2828
>
> I just rebased and pushed the branch.
>
> Thanks,
> Michael
>
> On 5/17/2022 1:22 AM, gaoliming wrote:
>> Michael:
>> Yes. I can. Where is the change for Edk2?
>>
>> Thanks
>> Liming
>>> -----邮件原件-----
>>> 发件人: Michael Kubacki <mikuback@linux.microsoft.com>
>>> 发送时间: 2022年5月17日 12:14
>>> 收件人: devel@edk2.groups.io; ardb@kernel.org
>>> 抄送: gaoliming <gaoliming@byosoft.com.cn>; Abner Chang
>>> <abner.chang@hpe.com>; Andrew Fish <afish@apple.com>; Anthony Perard
>>> <anthony.perard@citrix.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>;
>>> Benjamin You <benjamin.you@intel.com>; Brijesh Singh
>>> <brijesh.singh@amd.com>; Erdem Aktas <erdemaktas@google.com>; Gerd
>>> Hoffmann <kraxel@redhat.com>; Guo Dong <guo.dong@intel.com>; Hao A
>>> Wu <hao.a.wu@intel.com>; James Bottomley <jejb@linux.ibm.com>; Jian J
>>> Wang <jian.j.wang@intel.com>; Jiewen Yao <jiewen.yao@intel.com>; Jordan
>>> Justen <jordan.l.justen@intel.com>; Julien Grall <julien@xen.org>; Leif
>>> Lindholm <quic_llindhol@quicinc.com>; Maurice Ma
>>> <maurice.ma@intel.com>; Min Xu <min.m.xu@intel.com>; Nickle Wang
>>> <nickle.wang@hpe.com>; Peter Grehan <grehan@freebsd.org>; Ray Ni
>>> <ray.ni@intel.com>; Rebecca Cran <rebecca@bsdio.com>; Sami Mujawar
>>> <sami.mujawar@arm.com>; Sean Rhodes <sean@starlabs.systems>;
>>> Sebastien Boeuf <sebastien.boeuf@intel.com>; Tom Lendacky
>>> <thomas.lendacky@amd.com>
>>> 主题: Re: 回复: 回复: [edk2-devel] [PATCH v5 0/8] Add Variable Flash
>>> Info
>>> HOB
>>>
>>> Liming, will you push both?
>>>
>>> I put a rebased edk2-platforms with all the review tags for that series
>>> here:
>>> https://github.com/makubacki/edk2-platforms/commits/add_variableflashinf
>>> olib_to_platforms
>>>
>>> Thanks,
>>> Michael
>>>
>>> On 5/16/2022 1:36 PM, Ard Biesheuvel wrote:
>>>> On Mon, 16 May 2022 at 17:27, Michael Kubacki
>>>> <mikuback@linux.microsoft.com> wrote:
>>>>>
>>>>> Yes, it has been reviewed by all maintainers. An announcement of the
>>>>> change was sent to edk2 on April 29th:
>>>>> https://edk2.groups.io/g/announce/message/291
>>>>>
>>>>> The series for edk2-platforms has also been out since April 25th:
>>>>> https://edk2.groups.io/g/devel/message/89308
>>>>>
>>>>> Thanks,
>>>>> Michael
>>>>>
>>>>
>>>> I am fine with merging this as is, but please merge the edk2-platforms
>>>> changes as soon as the edk2 changes are in, so platforms there are not
>>>> left in a broken state.
>>>>
>>>>
>>>>
>>>>
>>
>>
>>
>>
>>
>>
^ permalink raw reply [flat|nested] 26+ messages in thread* 回复: 回复: 回复: 回复: [edk2-devel] [PATCH v5 0/8] Add Variable Flash Info HOB
2022-05-17 13:06 ` Michael Kubacki
2022-05-17 16:10 ` Michael Kubacki
@ 2022-05-19 3:45 ` gaoliming
[not found] ` <16F064E8C9D4EA0D.2722@groups.io>
2 siblings, 0 replies; 26+ messages in thread
From: gaoliming @ 2022-05-19 3:45 UTC (permalink / raw)
To: 'Michael Kubacki', devel, ardb
Cc: 'Abner Chang', 'Andrew Fish',
'Anthony Perard', 'Ard Biesheuvel',
'Benjamin You', 'Brijesh Singh',
'Erdem Aktas', 'Gerd Hoffmann',
'Guo Dong', 'Hao A Wu', 'James Bottomley',
'Jian J Wang', 'Jiewen Yao',
'Jordan Justen', 'Julien Grall',
'Leif Lindholm', 'Maurice Ma', 'Min Xu',
'Nickle Wang', 'Peter Grehan', 'Ray Ni',
'Rebecca Cran', 'Sami Mujawar',
'Sean Rhodes', 'Sebastien Boeuf',
'Tom Lendacky'
Michael:
I create PR https://github.com/tianocore/edk2/pull/2908 for this patch set. Once it is merged, I will immediately update edk2-platforms.
Thanks
Liming
> -----邮件原件-----
> 发件人: Michael Kubacki <mikuback@linux.microsoft.com>
> 发送时间: 2022年5月17日 21:07
> 收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn; ardb@kernel.org
> 抄送: 'Abner Chang' <abner.chang@hpe.com>; 'Andrew Fish'
> <afish@apple.com>; 'Anthony Perard' <anthony.perard@citrix.com>; 'Ard
> Biesheuvel' <ardb+tianocore@kernel.org>; 'Benjamin You'
> <benjamin.you@intel.com>; 'Brijesh Singh' <brijesh.singh@amd.com>; 'Erdem
> Aktas' <erdemaktas@google.com>; 'Gerd Hoffmann' <kraxel@redhat.com>;
> 'Guo Dong' <guo.dong@intel.com>; 'Hao A Wu' <hao.a.wu@intel.com>;
> 'James Bottomley' <jejb@linux.ibm.com>; 'Jian J Wang'
> <jian.j.wang@intel.com>; 'Jiewen Yao' <jiewen.yao@intel.com>; 'Jordan
> Justen' <jordan.l.justen@intel.com>; 'Julien Grall' <julien@xen.org>; 'Leif
> Lindholm' <quic_llindhol@quicinc.com>; 'Maurice Ma'
> <maurice.ma@intel.com>; 'Min Xu' <min.m.xu@intel.com>; 'Nickle Wang'
> <nickle.wang@hpe.com>; 'Peter Grehan' <grehan@freebsd.org>; 'Ray Ni'
> <ray.ni@intel.com>; 'Rebecca Cran' <rebecca@bsdio.com>; 'Sami Mujawar'
> <sami.mujawar@arm.com>; 'Sean Rhodes' <sean@starlabs.systems>;
> 'Sebastien Boeuf' <sebastien.boeuf@intel.com>; 'Tom Lendacky'
> <thomas.lendacky@amd.com>
> 主题: Re: 回复: 回复: 回复: [edk2-devel] [PATCH v5 0/8] Add Variable
> Flash Info HOB
>
> It is here:
> https://github.com/tianocore/edk2/pull/2828
>
> I just rebased and pushed the branch.
>
> Thanks,
> Michael
>
> On 5/17/2022 1:22 AM, gaoliming wrote:
> > Michael:
> > Yes. I can. Where is the change for Edk2?
> >
> > Thanks
> > Liming
> >> -----邮件原件-----
> >> 发件人: Michael Kubacki <mikuback@linux.microsoft.com>
> >> 发送时间: 2022年5月17日 12:14
> >> 收件人: devel@edk2.groups.io; ardb@kernel.org
> >> 抄送: gaoliming <gaoliming@byosoft.com.cn>; Abner Chang
> >> <abner.chang@hpe.com>; Andrew Fish <afish@apple.com>; Anthony
> Perard
> >> <anthony.perard@citrix.com>; Ard Biesheuvel
> <ardb+tianocore@kernel.org>;
> >> Benjamin You <benjamin.you@intel.com>; Brijesh Singh
> >> <brijesh.singh@amd.com>; Erdem Aktas <erdemaktas@google.com>;
> Gerd
> >> Hoffmann <kraxel@redhat.com>; Guo Dong <guo.dong@intel.com>; Hao A
> >> Wu <hao.a.wu@intel.com>; James Bottomley <jejb@linux.ibm.com>; Jian
> J
> >> Wang <jian.j.wang@intel.com>; Jiewen Yao <jiewen.yao@intel.com>;
> Jordan
> >> Justen <jordan.l.justen@intel.com>; Julien Grall <julien@xen.org>; Leif
> >> Lindholm <quic_llindhol@quicinc.com>; Maurice Ma
> >> <maurice.ma@intel.com>; Min Xu <min.m.xu@intel.com>; Nickle Wang
> >> <nickle.wang@hpe.com>; Peter Grehan <grehan@freebsd.org>; Ray Ni
> >> <ray.ni@intel.com>; Rebecca Cran <rebecca@bsdio.com>; Sami Mujawar
> >> <sami.mujawar@arm.com>; Sean Rhodes <sean@starlabs.systems>;
> >> Sebastien Boeuf <sebastien.boeuf@intel.com>; Tom Lendacky
> >> <thomas.lendacky@amd.com>
> >> 主题: Re: 回复: 回复: [edk2-devel] [PATCH v5 0/8] Add Variable Flash
> Info
> >> HOB
> >>
> >> Liming, will you push both?
> >>
> >> I put a rebased edk2-platforms with all the review tags for that series
> >> here:
> >>
> https://github.com/makubacki/edk2-platforms/commits/add_variableflashinf
> >> olib_to_platforms
> >>
> >> Thanks,
> >> Michael
> >>
> >> On 5/16/2022 1:36 PM, Ard Biesheuvel wrote:
> >>> On Mon, 16 May 2022 at 17:27, Michael Kubacki
> >>> <mikuback@linux.microsoft.com> wrote:
> >>>>
> >>>> Yes, it has been reviewed by all maintainers. An announcement of the
> >>>> change was sent to edk2 on April 29th:
> >>>> https://edk2.groups.io/g/announce/message/291
> >>>>
> >>>> The series for edk2-platforms has also been out since April 25th:
> >>>> https://edk2.groups.io/g/devel/message/89308
> >>>>
> >>>> Thanks,
> >>>> Michael
> >>>>
> >>>
> >>> I am fine with merging this as is, but please merge the edk2-platforms
> >>> changes as soon as the edk2 changes are in, so platforms there are not
> >>> left in a broken state.
> >>>
> >>>
> >>>
> >>>
> >
> >
> >
> >
> >
> >
^ permalink raw reply [flat|nested] 26+ messages in thread[parent not found: <16F064E8C9D4EA0D.2722@groups.io>]
* 回复: 回复: 回复: 回复: [edk2-devel] [PATCH v5 0/8] Add Variable Flash Info HOB
[not found] ` <16F064E8C9D4EA0D.2722@groups.io>
@ 2022-05-19 6:20 ` gaoliming
0 siblings, 0 replies; 26+ messages in thread
From: gaoliming @ 2022-05-19 6:20 UTC (permalink / raw)
To: devel, gaoliming, 'Michael Kubacki', ardb
Cc: 'Abner Chang', 'Andrew Fish',
'Anthony Perard', 'Ard Biesheuvel',
'Benjamin You', 'Brijesh Singh',
'Erdem Aktas', 'Gerd Hoffmann',
'Guo Dong', 'Hao A Wu', 'James Bottomley',
'Jian J Wang', 'Jiewen Yao',
'Jordan Justen', 'Julien Grall',
'Leif Lindholm', 'Maurice Ma', 'Min Xu',
'Nickle Wang', 'Peter Grehan', 'Ray Ni',
'Rebecca Cran', 'Sami Mujawar',
'Sean Rhodes', 'Sebastien Boeuf',
'Tom Lendacky'
Hi, all
VariableFlashInfo HOB has been merged in edk2 2189c71026..1f026ababf and edk2-platform fdaf4eb69a..3b896d1a32.
Thanks
Liming
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 gaoliming
> 发送时间: 2022年5月19日 11:46
> 收件人: 'Michael Kubacki' <mikuback@linux.microsoft.com>;
> devel@edk2.groups.io; ardb@kernel.org
> 抄送: 'Abner Chang' <abner.chang@hpe.com>; 'Andrew Fish'
> <afish@apple.com>; 'Anthony Perard' <anthony.perard@citrix.com>; 'Ard
> Biesheuvel' <ardb+tianocore@kernel.org>; 'Benjamin You'
> <benjamin.you@intel.com>; 'Brijesh Singh' <brijesh.singh@amd.com>; 'Erdem
> Aktas' <erdemaktas@google.com>; 'Gerd Hoffmann' <kraxel@redhat.com>;
> 'Guo Dong' <guo.dong@intel.com>; 'Hao A Wu' <hao.a.wu@intel.com>;
> 'James Bottomley' <jejb@linux.ibm.com>; 'Jian J Wang'
> <jian.j.wang@intel.com>; 'Jiewen Yao' <jiewen.yao@intel.com>; 'Jordan
> Justen' <jordan.l.justen@intel.com>; 'Julien Grall' <julien@xen.org>; 'Leif
> Lindholm' <quic_llindhol@quicinc.com>; 'Maurice Ma'
> <maurice.ma@intel.com>; 'Min Xu' <min.m.xu@intel.com>; 'Nickle Wang'
> <nickle.wang@hpe.com>; 'Peter Grehan' <grehan@freebsd.org>; 'Ray Ni'
> <ray.ni@intel.com>; 'Rebecca Cran' <rebecca@bsdio.com>; 'Sami Mujawar'
> <sami.mujawar@arm.com>; 'Sean Rhodes' <sean@starlabs.systems>;
> 'Sebastien Boeuf' <sebastien.boeuf@intel.com>; 'Tom Lendacky'
> <thomas.lendacky@amd.com>
> 主题: 回复: 回复: 回复: 回复: [edk2-devel] [PATCH v5 0/8] Add Variable
> Flash Info HOB
>
> Michael:
> I create PR https://github.com/tianocore/edk2/pull/2908 for this patch set.
> Once it is merged, I will immediately update edk2-platforms.
>
> Thanks
> Liming
> > -----邮件原件-----
> > 发件人: Michael Kubacki <mikuback@linux.microsoft.com>
> > 发送时间: 2022年5月17日 21:07
> > 收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn;
> ardb@kernel.org
> > 抄送: 'Abner Chang' <abner.chang@hpe.com>; 'Andrew Fish'
> > <afish@apple.com>; 'Anthony Perard' <anthony.perard@citrix.com>; 'Ard
> > Biesheuvel' <ardb+tianocore@kernel.org>; 'Benjamin You'
> > <benjamin.you@intel.com>; 'Brijesh Singh' <brijesh.singh@amd.com>;
> 'Erdem
> > Aktas' <erdemaktas@google.com>; 'Gerd Hoffmann' <kraxel@redhat.com>;
> > 'Guo Dong' <guo.dong@intel.com>; 'Hao A Wu' <hao.a.wu@intel.com>;
> > 'James Bottomley' <jejb@linux.ibm.com>; 'Jian J Wang'
> > <jian.j.wang@intel.com>; 'Jiewen Yao' <jiewen.yao@intel.com>; 'Jordan
> > Justen' <jordan.l.justen@intel.com>; 'Julien Grall' <julien@xen.org>; 'Leif
> > Lindholm' <quic_llindhol@quicinc.com>; 'Maurice Ma'
> > <maurice.ma@intel.com>; 'Min Xu' <min.m.xu@intel.com>; 'Nickle Wang'
> > <nickle.wang@hpe.com>; 'Peter Grehan' <grehan@freebsd.org>; 'Ray Ni'
> > <ray.ni@intel.com>; 'Rebecca Cran' <rebecca@bsdio.com>; 'Sami Mujawar'
> > <sami.mujawar@arm.com>; 'Sean Rhodes' <sean@starlabs.systems>;
> > 'Sebastien Boeuf' <sebastien.boeuf@intel.com>; 'Tom Lendacky'
> > <thomas.lendacky@amd.com>
> > 主题: Re: 回复: 回复: 回复: [edk2-devel] [PATCH v5 0/8] Add Variable
> > Flash Info HOB
> >
> > It is here:
> > https://github.com/tianocore/edk2/pull/2828
> >
> > I just rebased and pushed the branch.
> >
> > Thanks,
> > Michael
> >
> > On 5/17/2022 1:22 AM, gaoliming wrote:
> > > Michael:
> > > Yes. I can. Where is the change for Edk2?
> > >
> > > Thanks
> > > Liming
> > >> -----邮件原件-----
> > >> 发件人: Michael Kubacki <mikuback@linux.microsoft.com>
> > >> 发送时间: 2022年5月17日 12:14
> > >> 收件人: devel@edk2.groups.io; ardb@kernel.org
> > >> 抄送: gaoliming <gaoliming@byosoft.com.cn>; Abner Chang
> > >> <abner.chang@hpe.com>; Andrew Fish <afish@apple.com>; Anthony
> > Perard
> > >> <anthony.perard@citrix.com>; Ard Biesheuvel
> > <ardb+tianocore@kernel.org>;
> > >> Benjamin You <benjamin.you@intel.com>; Brijesh Singh
> > >> <brijesh.singh@amd.com>; Erdem Aktas <erdemaktas@google.com>;
> > Gerd
> > >> Hoffmann <kraxel@redhat.com>; Guo Dong <guo.dong@intel.com>; Hao
> A
> > >> Wu <hao.a.wu@intel.com>; James Bottomley <jejb@linux.ibm.com>;
> Jian
> > J
> > >> Wang <jian.j.wang@intel.com>; Jiewen Yao <jiewen.yao@intel.com>;
> > Jordan
> > >> Justen <jordan.l.justen@intel.com>; Julien Grall <julien@xen.org>; Leif
> > >> Lindholm <quic_llindhol@quicinc.com>; Maurice Ma
> > >> <maurice.ma@intel.com>; Min Xu <min.m.xu@intel.com>; Nickle Wang
> > >> <nickle.wang@hpe.com>; Peter Grehan <grehan@freebsd.org>; Ray Ni
> > >> <ray.ni@intel.com>; Rebecca Cran <rebecca@bsdio.com>; Sami
> Mujawar
> > >> <sami.mujawar@arm.com>; Sean Rhodes <sean@starlabs.systems>;
> > >> Sebastien Boeuf <sebastien.boeuf@intel.com>; Tom Lendacky
> > >> <thomas.lendacky@amd.com>
> > >> 主题: Re: 回复: 回复: [edk2-devel] [PATCH v5 0/8] Add Variable Flash
> > Info
> > >> HOB
> > >>
> > >> Liming, will you push both?
> > >>
> > >> I put a rebased edk2-platforms with all the review tags for that series
> > >> here:
> > >>
> >
> https://github.com/makubacki/edk2-platforms/commits/add_variableflashinf
> > >> olib_to_platforms
> > >>
> > >> Thanks,
> > >> Michael
> > >>
> > >> On 5/16/2022 1:36 PM, Ard Biesheuvel wrote:
> > >>> On Mon, 16 May 2022 at 17:27, Michael Kubacki
> > >>> <mikuback@linux.microsoft.com> wrote:
> > >>>>
> > >>>> Yes, it has been reviewed by all maintainers. An announcement of the
> > >>>> change was sent to edk2 on April 29th:
> > >>>> https://edk2.groups.io/g/announce/message/291
> > >>>>
> > >>>> The series for edk2-platforms has also been out since April 25th:
> > >>>> https://edk2.groups.io/g/devel/message/89308
> > >>>>
> > >>>> Thanks,
> > >>>> Michael
> > >>>>
> > >>>
> > >>> I am fine with merging this as is, but please merge the edk2-platforms
> > >>> changes as soon as the edk2 changes are in, so platforms there are not
> > >>> left in a broken state.
> > >>>
> > >>>
> > >>>
> > >>>
> > >
> > >
> > >
> > >
> > >
> > >
>
>
>
>
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread