From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c0b::243; helo=mail-it0-x243.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it0-x243.google.com (mail-it0-x243.google.com [IPv6:2607:f8b0:4001:c0b::243]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 132B7220D4C1E for ; Fri, 1 Dec 2017 00:47:28 -0800 (PST) Received: by mail-it0-x243.google.com with SMTP id f143so1686365itb.0 for ; Fri, 01 Dec 2017 00:51:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=9B+QUpY1vDTjGa9I1Gi6PNIw+4nIrfHGPIuK+MzK7SY=; b=iztD1wV4mwk6bEQWgCkWza7kP4Eawv1qVDwj7Y1Fkf9N+oARY3ywPyNq6MYDXgyvOG TSQN8OZfv4HJ5N3u0TeOV5kleWk4+gBoLQx4rbsrlp3KRnuUnIjiPTup3hnZmmIoGG3B NuBB1VdxwEk3aUOxjQQKWFGEenjuV1W7VCmmQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=9B+QUpY1vDTjGa9I1Gi6PNIw+4nIrfHGPIuK+MzK7SY=; b=V2CueqKKPM5HTYTYxaskd9+DUChSebws8HFvMlCcywdhOk4c7rTT9eXYhjUv8pcJxv G5+qSlQtOy7tnGIn4MkoCLZ06xc8lEsN70k8a8Z6yrdmcc6EVepVK/a4IgGkiESfss4C qOg6jcnLrAsX1pf0yJxFUr4+k2fI39V6U+/Ptkzjlhglmj6RrYZZwIW0fThW43yFu1Vn pQ9fas3MHhUIiFt9BXy87HPbmtpCbVmVcj3Zxw6q/zaRzp10M0KnQMjb1Mow1FGRQJH8 WSoXBdhnJ60ufJBC2BeovXQZbqhOamG25Bokj5rg6mOeF/pFZGfSNUl/KCqHHtSr2V01 S4KA== X-Gm-Message-State: AKGB3mI02KYNt91dI+E4bJgPcCAa68vqoKYZyyRCQj2CG9cW5EO9HjM3 3GtvuQSSNZK7QzpmHTVVtnVmkNnpqfh8w71y/UoE/A== X-Google-Smtp-Source: AGs4zMbvcCveregVEO/8U0TJzRWXrhxJyGJXdViT/Tg3fpujFSJpLnDfb5VQDOlw/XgbXtSDOoLia+JB6F/NFNbC/eo= X-Received: by 10.36.31.212 with SMTP id d203mr912412itd.48.1512118314438; Fri, 01 Dec 2017 00:51:54 -0800 (PST) MIME-Version: 1.0 Received: by 10.107.104.16 with HTTP; Fri, 1 Dec 2017 00:51:54 -0800 (PST) In-Reply-To: <20171130163029.19743-4-lersek@redhat.com> References: <20171130163029.19743-1-lersek@redhat.com> <20171130163029.19743-4-lersek@redhat.com> From: Ard Biesheuvel Date: Fri, 1 Dec 2017 08:51:54 +0000 Message-ID: To: Laszlo Ersek Cc: edk2-devel-01 , Anthony Perard , Jordan Justen , Julien Grall Subject: Re: [PATCH v2 3/8] OvmfPkg: conditionally disable reserved memory varstore emulation at build 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: Fri, 01 Dec 2017 08:47:29 -0000 Content-Type: text/plain; charset="UTF-8" On 30 November 2017 at 16:30, Laszlo Ersek wrote: > (All of the below is only relevant for SMM_REQUIRE=FALSE.) > > 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. > > In addition, QemuFlashFvbServicesRuntimeDxe is always launched before > EmuVariableFvbRuntimeDxe, so that if flash variables are available, > QemuFlashFvbServicesRuntimeDxe can set PcdFlashNvStorageVariableBase64 > first, and EmuVariableFvbRuntimeDxe can exit early. This ordering is > currently enforced by adding QemuFlashFvbServicesRuntimeDxe to the APRIORI > DXE file. > > All of this is unnecessary when MEM_VARSTORE_EMU_ENABLE is set to FALSE. > In such a build, > > - (almost) remove the dynamic default for PcdEmuVariableNvStoreReserved > (we can't really do this because the PcdSet64() in > ReserveEmuVariableNvStore() wouldn't compile), > If that is the only concern, and the value is irrelevant, you could make it a patchable PCD instead > - prevent the reserved memory allocation and PCD setting in PlatformPei, > > - exclude EmuVariableFvbRuntimeDxe, > > - and drop QemuFlashFvbServicesRuntimeDxe from the APRIORI DXE file (since > it doesn't have to beat EmuVariableFvbRuntimeDxe in setting > PcdFlashNvStorageVariableBase64 any longer). > > Cc: Anthony Perard > Cc: Ard Biesheuvel > Cc: Jordan Justen > Cc: Julien Grall > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Laszlo Ersek Reviewed-by: Ard Biesheuvel > --- > OvmfPkg/OvmfPkgIa32.dsc | 4 +++- > OvmfPkg/OvmfPkgIa32X64.dsc | 4 +++- > OvmfPkg/OvmfPkgX64.dsc | 4 +++- > OvmfPkg/OvmfPkgIa32.fdf | 4 +++- > OvmfPkg/OvmfPkgIa32X64.fdf | 4 +++- > OvmfPkg/OvmfPkgX64.fdf | 4 +++- > OvmfPkg/PlatformPei/PlatformPei.inf | 1 + > OvmfPkg/PlatformPei/Platform.c | 3 ++- > 8 files changed, 21 insertions(+), 7 deletions(-) > > diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc > index 443da553d0a3..dd6be0de0445 100644 > --- a/OvmfPkg/OvmfPkgIa32.dsc > +++ b/OvmfPkg/OvmfPkgIa32.dsc > @@ -502,7 +502,7 @@ [PcdsFixedAtBuild] > > [PcdsDynamicDefault] > # only set when > - # ($(SMM_REQUIRE) == FALSE) > + # (($(SMM_REQUIRE) == FALSE) && ($(MEM_VARSTORE_EMU_ENABLE) == TRUE)) > gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved|0 > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0 > @@ -871,10 +871,12 @@ [Components] > # Variable driver stack (non-SMM) > # > OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf > +!if $(MEM_VARSTORE_EMU_ENABLE) == TRUE > OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.inf { > > PlatformFvbLib|OvmfPkg/Library/EmuVariableFvbLib/EmuVariableFvbLib.inf > } > +!endif > MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf { > > diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc > index 0fc81743bac4..84c578ac22a4 100644 > --- a/OvmfPkg/OvmfPkgIa32X64.dsc > +++ b/OvmfPkg/OvmfPkgIa32X64.dsc > @@ -508,7 +508,7 @@ [PcdsFixedAtBuild.X64] > > [PcdsDynamicDefault] > # only set when > - # ($(SMM_REQUIRE) == FALSE) > + # (($(SMM_REQUIRE) == FALSE) && ($(MEM_VARSTORE_EMU_ENABLE) == TRUE)) > gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved|0 > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0 > @@ -881,10 +881,12 @@ [Components.X64] > # Variable driver stack (non-SMM) > # > OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf > +!if $(MEM_VARSTORE_EMU_ENABLE) == TRUE > OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.inf { > > PlatformFvbLib|OvmfPkg/Library/EmuVariableFvbLib/EmuVariableFvbLib.inf > } > +!endif > MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf { > > diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc > index db33be4bc0b7..b5d385101411 100644 > --- a/OvmfPkg/OvmfPkgX64.dsc > +++ b/OvmfPkg/OvmfPkgX64.dsc > @@ -507,7 +507,7 @@ [PcdsFixedAtBuild] > > [PcdsDynamicDefault] > # only set when > - # ($(SMM_REQUIRE) == FALSE) > + # (($(SMM_REQUIRE) == FALSE) && ($(MEM_VARSTORE_EMU_ENABLE) == TRUE)) > gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved|0 > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0 > @@ -879,10 +879,12 @@ [Components] > # Variable driver stack (non-SMM) > # > OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf > +!if $(MEM_VARSTORE_EMU_ENABLE) == TRUE > OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.inf { > > PlatformFvbLib|OvmfPkg/Library/EmuVariableFvbLib/EmuVariableFvbLib.inf > } > +!endif > MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf { > > diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf > index ba980834d720..50a2db897bbb 100644 > --- a/OvmfPkg/OvmfPkgIa32.fdf > +++ b/OvmfPkg/OvmfPkgIa32.fdf > @@ -191,7 +191,7 @@ [FV.DXEFV] > APRIORI DXE { > INF MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf > INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf > -!if $(SMM_REQUIRE) == FALSE > +!if ($(SMM_REQUIRE) == FALSE) && ($(MEM_VARSTORE_EMU_ENABLE) == TRUE) > INF OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf > !endif > } > @@ -375,7 +375,9 @@ [FV.DXEFV] > # Variable driver stack (non-SMM) > # > INF OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf > +!if $(MEM_VARSTORE_EMU_ENABLE) == TRUE > INF OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.inf > +!endif > INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf > INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf > !endif > diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf > index 72ac82e76b7b..efa01734b576 100644 > --- a/OvmfPkg/OvmfPkgIa32X64.fdf > +++ b/OvmfPkg/OvmfPkgIa32X64.fdf > @@ -192,7 +192,7 @@ [FV.DXEFV] > INF MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf > INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf > INF OvmfPkg/AmdSevDxe/AmdSevDxe.inf > -!if $(SMM_REQUIRE) == FALSE > +!if ($(SMM_REQUIRE) == FALSE) && ($(MEM_VARSTORE_EMU_ENABLE) == TRUE) > INF OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf > !endif > } > @@ -382,7 +382,9 @@ [FV.DXEFV] > # Variable driver stack (non-SMM) > # > INF OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf > +!if $(MEM_VARSTORE_EMU_ENABLE) == TRUE > INF OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.inf > +!endif > INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf > INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf > !endif > diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf > index 2fc17810eb23..d7a5ea97bda8 100644 > --- a/OvmfPkg/OvmfPkgX64.fdf > +++ b/OvmfPkg/OvmfPkgX64.fdf > @@ -192,7 +192,7 @@ [FV.DXEFV] > INF MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf > INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf > INF OvmfPkg/AmdSevDxe/AmdSevDxe.inf > -!if $(SMM_REQUIRE) == FALSE > +!if ($(SMM_REQUIRE) == FALSE) && ($(MEM_VARSTORE_EMU_ENABLE) == TRUE) > INF OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf > !endif > } > @@ -382,7 +382,9 @@ [FV.DXEFV] > # Variable driver stack (non-SMM) > # > INF OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf > +!if $(MEM_VARSTORE_EMU_ENABLE) == TRUE > INF OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.inf > +!endif > INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf > INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf > !endif > diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf > index de7434d93dc0..4b8626cb2a27 100644 > --- a/OvmfPkg/PlatformPei/PlatformPei.inf > +++ b/OvmfPkg/PlatformPei/PlatformPei.inf > @@ -108,6 +108,7 @@ [FixedPcd] > > [FeaturePcd] > gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire > + gUefiOvmfPkgTokenSpaceGuid.PcdMemVarstoreEmuEnable > > [Ppis] > gEfiPeiMasterBootModePpiGuid > diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c > index 5a78668126b4..34e7e903fc70 100644 > --- a/OvmfPkg/PlatformPei/Platform.c > +++ b/OvmfPkg/PlatformPei/Platform.c > @@ -664,7 +664,8 @@ InitializePlatform ( > } > > if (mBootMode != BOOT_ON_S3_RESUME) { > - if (!FeaturePcdGet (PcdSmmSmramRequire)) { > + if (!FeaturePcdGet (PcdSmmSmramRequire) && > + FeaturePcdGet (PcdMemVarstoreEmuEnable)) { > ReserveEmuVariableNvStore (); > } > PeiFvInitialization (); > -- > 2.14.1.3.gb7cf6e02401b > >