From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.24; helo=mga09.intel.com; envelope-from=hao.a.wu@intel.com; receiver=edk2-devel@lists.01.org Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 6EE47211B5A5E for ; Mon, 14 Jan 2019 21:48:10 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 14 Jan 2019 21:48:10 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,480,1539673200"; d="scan'208";a="126110273" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by orsmga002.jf.intel.com with ESMTP; 14 Jan 2019 21:48:09 -0800 Received: from fmsmsx111.amr.corp.intel.com (10.18.116.5) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.408.0; Mon, 14 Jan 2019 21:48:09 -0800 Received: from shsmsx154.ccr.corp.intel.com (10.239.6.54) by fmsmsx111.amr.corp.intel.com (10.18.116.5) with Microsoft SMTP Server (TLS) id 14.3.408.0; Mon, 14 Jan 2019 21:48:08 -0800 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.196]) by SHSMSX154.ccr.corp.intel.com ([169.254.7.46]) with mapi id 14.03.0415.000; Tue, 15 Jan 2019 13:48:07 +0800 From: "Wu, Hao A" To: "Zeng, Star" , "edk2-devel@lists.01.org" Thread-Topic: [PATCH V2 03/15] MdeModulePkg Variable: Not get NV PCD in VariableWriteServiceInitialize Thread-Index: AQHUrByoLdMsS0OdtEC8m27NrrV73aWv0uwg Date: Tue, 15 Jan 2019 05:48:06 +0000 Message-ID: References: <1547479196-40248-1-git-send-email-star.zeng@intel.com> <1547479196-40248-4-git-send-email-star.zeng@intel.com> In-Reply-To: <1547479196-40248-4-git-send-email-star.zeng@intel.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH V2 03/15] MdeModulePkg Variable: Not get NV PCD in VariableWriteServiceInitialize X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 15 Jan 2019 05:48:11 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi Star, One minor comment below. > -----Original Message----- > From: Zeng, Star > Sent: Monday, January 14, 2019 11:20 PM > To: edk2-devel@lists.01.org > Cc: Zeng, Star; Wang, Jian J; Wu, Hao A > Subject: [PATCH V2 03/15] MdeModulePkg Variable: Not get NV PCD in > VariableWriteServiceInitialize >=20 > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1323 > Merge EmuVariable and Real variable driver. >=20 > Add macro NV_STORAGE_VARIABLE_BASE. > Not get NV PCD in VariableWriteServiceInitialize, but in > FtwNotificationEvent/SmmFtwNotificationEvent, then > VariableWriteServiceInitialize could be not aware the NV > storage is real or emulated. >=20 > This patch prepares for adding emulated variable NV mode > support in VariableRuntimeDxe. >=20 > Cc: Jian J Wang > Cc: Hao Wu > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Star Zeng > --- > .../Universal/Variable/RuntimeDxe/Variable.c | 20 ++------------= ------ > .../Universal/Variable/RuntimeDxe/Variable.h | 9 +++++++-- > .../Universal/Variable/RuntimeDxe/VariableDxe.c | 12 ++++++++---- > .../Universal/Variable/RuntimeDxe/VariableSmm.c | 16 +++++++++++---= - > - > 4 files changed, 28 insertions(+), 29 deletions(-) >=20 > diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > index 0b675c8f36df..424f92a53757 100644 > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > @@ -3770,10 +3770,7 @@ InitRealNonVolatileVariableStore ( > return EFI_OUT_OF_RESOURCES; > } >=20 > - NvStorageBase =3D (EFI_PHYSICAL_ADDRESS) PcdGet64 > (PcdFlashNvStorageVariableBase64); > - if (NvStorageBase =3D=3D 0) { > - NvStorageBase =3D (EFI_PHYSICAL_ADDRESS) PcdGet32 > (PcdFlashNvStorageVariableBase); > - } > + NvStorageBase =3D NV_STORAGE_VARIABLE_BASE; > ASSERT (NvStorageBase !=3D 0); >=20 > // > @@ -4027,7 +4024,7 @@ FlushHobVariableToFlash ( > } >=20 > /** > - Initializes variable write service after FTW was ready. > + Initializes variable write service. >=20 > @retval EFI_SUCCESS Function successfully executed. > @retval Others Fail to initialize the variable service. > @@ -4041,23 +4038,10 @@ VariableWriteServiceInitialize ( > EFI_STATUS Status; > UINTN Index; > UINT8 Data; > - EFI_PHYSICAL_ADDRESS VariableStoreBase; > - EFI_PHYSICAL_ADDRESS NvStorageBase; > VARIABLE_ENTRY_PROPERTY *VariableEntry; >=20 > AcquireLockOnlyAtBootTime(&mVariableModuleGlobal- > >VariableGlobal.VariableServicesLock); >=20 > - NvStorageBase =3D (EFI_PHYSICAL_ADDRESS) PcdGet64 > (PcdFlashNvStorageVariableBase64); > - if (NvStorageBase =3D=3D 0) { > - NvStorageBase =3D (EFI_PHYSICAL_ADDRESS) PcdGet32 > (PcdFlashNvStorageVariableBase); > - } > - VariableStoreBase =3D NvStorageBase + (mNvFvHeaderCache- > >HeaderLength); > - > - // > - // Let NonVolatileVariableBase point to flash variable store base dire= ctly > after FTW ready. > - // > - mVariableModuleGlobal->VariableGlobal.NonVolatileVariableBase =3D > VariableStoreBase; > - > // > // Check if the free area is really free. > // > diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h > b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h > index 938eb5de61fa..566e7268d187 100644 > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h > @@ -2,7 +2,7 @@ > The internal header file includes the common header files, defines > internal structure and functions used by Variable modules. >=20 > -Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
> +Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
> This program and the accompanying materials > are licensed and made available under the terms and conditions of the BS= D > License > which accompanies this distribution. The full text of the license may b= e > found at > @@ -46,6 +46,11 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY > KIND, EITHER EXPRESS OR IMPLIED. >=20 > #include "PrivilegePolymorphic.h" >=20 > +#define NV_STORAGE_VARIABLE_BASE (EFI_PHYSICAL_ADDRESS) \ > + (PcdGet64 (PcdFlashNvStorageVariableB= ase64) !=3D 0 ? \ > + PcdGet64 (PcdFlashNvStorageVariableB= ase64) : \ > + PcdGet32 (PcdFlashNvStorageVariableB= ase)) > + > #define EFI_VARIABLE_ATTRIBUTES_MASK (EFI_VARIABLE_NON_VOLATILE > | \ > EFI_VARIABLE_BOOTSERVICE_ACCESS | = \ > EFI_VARIABLE_RUNTIME_ACCESS | \ > @@ -473,7 +478,7 @@ GetMaxVariableSize ( > ); >=20 > /** > - Initializes variable write service after FVB was ready. > + Initializes variable write service. >=20 > @retval EFI_SUCCESS Function successfully executed. > @retval Others Fail to initialize the variable service. > diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c > index f7185df3a7eb..baba6729c1c2 100644 > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c > @@ -386,13 +386,17 @@ FtwNotificationEvent ( > ASSERT (PcdGet32 (PcdFlashNvStorageVariableSize) <=3D FtwMaxBlockSiz= e); > } >=20 > + NvStorageVariableBase =3D NV_STORAGE_VARIABLE_BASE; > + VariableStoreBase =3D NvStorageVariableBase + (mNvFvHeaderCache- > >HeaderLength); I think you can remove the below duplicate assignment to variable 'VariableStoreBase' within function FtwNotificationEvent(): // // Mark the variable storage region of the FLASH as RUNTIME. // VariableStoreBase =3D NvStorageVariableBase + mNvFvHeaderCache->HeaderL= ength; Best Regards, Hao Wu > + > + // > + // Let NonVolatileVariableBase point to flash variable store base dire= ctly > after FTW ready. > + // > + mVariableModuleGlobal->VariableGlobal.NonVolatileVariableBase =3D > VariableStoreBase; > + > // > // Find the proper FVB protocol for variable. > // > - NvStorageVariableBase =3D (EFI_PHYSICAL_ADDRESS) PcdGet64 > (PcdFlashNvStorageVariableBase64); > - if (NvStorageVariableBase =3D=3D 0) { > - NvStorageVariableBase =3D (EFI_PHYSICAL_ADDRESS) PcdGet32 > (PcdFlashNvStorageVariableBase); > - } > Status =3D GetFvbInfoByAddress (NvStorageVariableBase, NULL, > &FvbProtocol); > if (EFI_ERROR (Status)) { > return ; > diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c > index 8c53f84ff6e8..018587ed7373 100644 > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c > @@ -14,7 +14,7 @@ > VariableServiceSetVariable(), VariableServiceQueryVariableInfo(), > ReclaimForOS(), > SmmVariableGetStatistics() should also do validation based on its own > knowledge. >=20 > -Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.
> +Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.
> This program and the accompanying materials > are licensed and made available under the terms and conditions of the BS= D > License > which accompanies this distribution. The full text of the license may b= e > found at > @@ -37,6 +37,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY > KIND, EITHER EXPRESS OR IMPLIED. > #include > #include "Variable.h" >=20 > +extern EFI_FIRMWARE_VOLUME_HEADER *mNvFvHeaderCache; > extern VARIABLE_INFO_ENTRY *gVariableInfo; > EFI_HANDLE mSmmVariableHandle = =3D NULL; > EFI_HANDLE mVariableHandle = =3D NULL; > @@ -867,6 +868,7 @@ SmmFtwNotificationEvent ( > ) > { > EFI_STATUS Status; > + EFI_PHYSICAL_ADDRESS VariableStoreBase; > EFI_SMM_FIRMWARE_VOLUME_BLOCK_PROTOCOL *FvbProtocol; > EFI_SMM_FAULT_TOLERANT_WRITE_PROTOCOL *FtwProtocol; > EFI_PHYSICAL_ADDRESS NvStorageVariableBase; > @@ -889,13 +891,17 @@ SmmFtwNotificationEvent ( > ASSERT (PcdGet32 (PcdFlashNvStorageVariableSize) <=3D FtwMaxBlockSiz= e); > } >=20 > + NvStorageVariableBase =3D NV_STORAGE_VARIABLE_BASE; > + VariableStoreBase =3D NvStorageVariableBase + (mNvFvHeaderCache- > >HeaderLength); > + > + // > + // Let NonVolatileVariableBase point to flash variable store base dire= ctly > after FTW ready. > + // > + mVariableModuleGlobal->VariableGlobal.NonVolatileVariableBase =3D > VariableStoreBase; > + > // > // Find the proper FVB protocol for variable. > // > - NvStorageVariableBase =3D (EFI_PHYSICAL_ADDRESS) PcdGet64 > (PcdFlashNvStorageVariableBase64); > - if (NvStorageVariableBase =3D=3D 0) { > - NvStorageVariableBase =3D (EFI_PHYSICAL_ADDRESS) PcdGet32 > (PcdFlashNvStorageVariableBase); > - } > Status =3D GetFvbInfoByAddress (NvStorageVariableBase, NULL, > &FvbProtocol); > if (EFI_ERROR (Status)) { > return EFI_NOT_FOUND; > -- > 2.7.0.windows.1