From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) (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 C6BA521A1349E for ; Mon, 15 May 2017 11:09:52 -0700 (PDT) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 15 May 2017 11:09:52 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.38,346,1491289200"; d="scan'208";a="1148063383" Received: from casander-mobl.amr.corp.intel.com (HELO localhost) ([10.254.34.31]) by fmsmga001.fm.intel.com with ESMTP; 15 May 2017 11:09:51 -0700 MIME-Version: 1.0 To: Laszlo Ersek , edk2-devel-01 Message-ID: <149487179135.31444.17454219357311957333@jljusten-skl> From: Jordan Justen In-Reply-To: <20170505210258.28141-6-lersek@redhat.com> References: <20170505210258.28141-1-lersek@redhat.com> <20170505210258.28141-6-lersek@redhat.com> User-Agent: alot/0.5.1 Date: Mon, 15 May 2017 11:09:51 -0700 Subject: Re: [PATCH 5/7] OvmfPkg/PlatformPei: don't allocate reserved mem varstore if SMM_REQUIRE X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 15 May 2017 18:09:53 -0000 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable On 2017-05-05 14:02:56, Laszlo Ersek wrote: > For the emulated variable store, PlatformPei allocates reserved memory (as > early as possible, so that the address remains the same during reboot), > and PcdEmuVariableNvStoreReserved carries the address to > EmuVariableFvbRuntimeDxe. > = > However, EmuVariableFvbRuntimeDxe is excluded from the SMM_REQUIRE build, > and then noone consumes PcdEmuVariableNvStoreReserved. Don't waste > reserved memory whenever that's the case. > = > (Even a dynamic default for PcdEmuVariableNvStoreReserved would be > unnecessary; but that way the PcdSet64S() call in the > ReserveEmuVariableNvStore() function doesn't compile.) > = > Cc: Jordan Justen > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Laszlo Ersek > --- > OvmfPkg/OvmfPkgIa32.dsc | 3 +++ > OvmfPkg/OvmfPkgIa32X64.dsc | 3 +++ > OvmfPkg/OvmfPkgX64.dsc | 3 +++ > OvmfPkg/PlatformPei/Platform.c | 4 +++- > 4 files changed, 12 insertions(+), 1 deletion(-) > = > diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc > index 64427716c53c..b46eef6cabc3 100644 > --- a/OvmfPkg/OvmfPkgIa32.dsc > +++ b/OvmfPkg/OvmfPkgIa32.dsc > @@ -495,7 +495,10 @@ [PcdsFixedAtBuild] > ########################################################################= ######## > = > [PcdsDynamicDefault] > + # only set when > + # ($(SMM_REQUIRE) =3D=3D FALSE) > gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved|0 > + > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0 > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0 > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0 > diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc > index 887964cd27c2..08f471fbc542 100644 > --- a/OvmfPkg/OvmfPkgIa32X64.dsc > +++ b/OvmfPkg/OvmfPkgIa32X64.dsc > @@ -501,7 +501,10 @@ [PcdsFixedAtBuild.X64] > ########################################################################= ######## > = > [PcdsDynamicDefault] > + # only set when > + # ($(SMM_REQUIRE) =3D=3D FALSE) I don't think we should bother adding these comments into the .dsc. Ultimately, I would prefer to always allocate this, even when SMM is set to be required. It'd be nice if we could always fallback to EmuFvb, but I understand that this might not be possible given how difficult it is to determine if QEMU actually has SMM enabled. Anyway, I think this patch makes sense until we can potentially fix that. (Which may or may not be worth fixing.) Series Reviewed-by: Jordan Justen > gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved|0 > + > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0 > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0 > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0 > diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc > index dc5fea3577d4..24053e5ff82d 100644 > --- a/OvmfPkg/OvmfPkgX64.dsc > +++ b/OvmfPkg/OvmfPkgX64.dsc > @@ -500,7 +500,10 @@ [PcdsFixedAtBuild] > ########################################################################= ######## > = > [PcdsDynamicDefault] > + # only set when > + # ($(SMM_REQUIRE) =3D=3D FALSE) > gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved|0 > + > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0 > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0 > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0 > diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platfor= m.c > index 5e983a8dcea9..1b4dc00b0180 100644 > --- a/OvmfPkg/PlatformPei/Platform.c > +++ b/OvmfPkg/PlatformPei/Platform.c > @@ -672,7 +672,9 @@ InitializePlatform ( > mHostBridgeDevId =3D PciRead16 (OVMF_HOSTBRIDGE_DID); > = > if (mBootMode !=3D BOOT_ON_S3_RESUME) { > - ReserveEmuVariableNvStore (); > + if (!FeaturePcdGet (PcdSmmSmramRequire)) { > + ReserveEmuVariableNvStore (); > + } > PeiFvInitialization (); > MemMapInitialization (); > NoexecDxeInitialization (); > -- = > 2.9.3 > = >=20