From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.132.183.28; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (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 443EC20352A86 for ; Fri, 1 Dec 2017 03:24:20 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D8FEFC059B61; Fri, 1 Dec 2017 11:28:46 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-33.rdu2.redhat.com [10.10.120.33]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5FC505C1AB; Fri, 1 Dec 2017 11:28:45 +0000 (UTC) From: Laszlo Ersek To: Ard Biesheuvel Cc: edk2-devel-01 , Anthony Perard , Jordan Justen , Julien Grall References: <20171130163029.19743-1-lersek@redhat.com> <20171130163029.19743-5-lersek@redhat.com> <60ab02c6-4a27-0a28-7e01-550415b26f1b@redhat.com> Message-ID: Date: Fri, 1 Dec 2017 12:28:44 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <60ab02c6-4a27-0a28-7e01-550415b26f1b@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Fri, 01 Dec 2017 11:28:46 +0000 (UTC) Subject: Re: [PATCH v2 4/8] OvmfPkg: introduce FlashNvStorageAddressLib 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 11:24:21 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 12/01/17 12:03, Laszlo Ersek wrote: > On 12/01/17 09:55, Ard Biesheuvel wrote: >> On 30 November 2017 at 16:30, Laszlo Ersek wrote: >>> An OVMF binary built with >>> >>> -D SMM_REQUIRE >>> >>> or with >>> >>> -D MEM_VARSTORE_EMU_ENABLE=FALSE >>> >>> requires flash to be present at the expected (build-time determined) >>> location; falling back to the emulated varstore is not possible. >>> >>> In such builds, we can replace the settings of the >>> - varstore, >>> - FTW working block, >>> - and FTW spare area >>> address PCDs in QemuFlashFvbServicesRuntimeDxe with identical settings in >>> a new plug-in (NULL class) library, to be linked into variable-related >>> PEIMs. >>> >>> This will enable such builds to access variables during the PEI phase, >>> without dynamic flash detection and without ordering constraints against >>> other PEIMs. >>> >> >> Why can't we set these at build time in this case? > > Hmm, let me think... Oh yes, I think I remember. > > The PCDs that are used as *source* here (i.e., passed as arguments to > PcdGet32()) *are* set at build time. However, they take some calculation > at build time too, and they are set in the "OvmfPkg/OvmfPkg.fdf.inc" > file, with explicit SET statments. > > This is because you can build OVMF for a 1MB, 2MB, or 4MB unified flash > image size, and the "source PCDs" in question depend on them. > Furthermore, these "source PCDs" are fixed-at-build. > > I think I couldn't find a way to set the defaults for the "target", > dynamic, PCDs, from the build-time-calculated "source PCDs". The usual > dynamic-default setting in the DSC does not work (because the default > value is not a numeric constant), and, as far as I remember, when I > tried the SET command on the dynamic ("target") PCDs as well, it simply > didn't work. > > The SET statements *could* work if the target PCDs could be made > (conditionally) fixed-at-build as well, but then we run into the same > build problem of ReserveEmuVariableNvStore() not compiling. > > In short: > > - the "target" PCDs have to remain dynamic, or else > ReserveEmuVariableNvStore() won't compile, correction -- ReserveEmuVariableNvStore() is totally irrelevant here; what I meant here were the PcdSet calls for these "target" PCDs that wouldn't compile in QemuFlashFvbServicesRuntimeDxe. (See patch 6.) Thanks, Laszlo > > - as long as the "target" PCDs are dynamic, I cannot set their defaults > at build time *from* the "source" fixed-at-build PCDs, which are > calculated at each build separately, with the SET statement. > > Thanks > Laszlo > > >> >>> Cc: Anthony Perard >>> Cc: Ard Biesheuvel >>> Cc: Jordan Justen >>> Cc: Julien Grall >>> Contributed-under: TianoCore Contribution Agreement 1.0 >>> Signed-off-by: Laszlo Ersek >>> --- >>> OvmfPkg/Library/FlashNvStorageAddressLib/FlashNvStorageAddressLib.inf | 48 ++++++++++++++++++ >>> OvmfPkg/Library/FlashNvStorageAddressLib/FlashNvStorageAddressLib.c | 53 ++++++++++++++++++++ >>> 2 files changed, 101 insertions(+) >>> >>> diff --git a/OvmfPkg/Library/FlashNvStorageAddressLib/FlashNvStorageAddressLib.inf b/OvmfPkg/Library/FlashNvStorageAddressLib/FlashNvStorageAddressLib.inf >>> new file mode 100644 >>> index 000000000000..f79194f80de9 >>> --- /dev/null >>> +++ b/OvmfPkg/Library/FlashNvStorageAddressLib/FlashNvStorageAddressLib.inf >>> @@ -0,0 +1,48 @@ >>> +## @file >>> +# >>> +# A hook-in library for variable-related PEIMs, in order to set >>> +# - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64, >>> +# - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase, >>> +# - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase, >>> +# from their gUefiOvmfPkgTokenSpaceGuid counterparts, just before those PEIMs >>> +# consume them. >>> +# >>> +# Copyright (C) 2017, Red Hat, Inc. >>> +# >>> +# This program and the accompanying materials are licensed and made available >>> +# under the terms and conditions of the BSD License which accompanies this >>> +# distribution. The full text of the license may be found at >>> +# http://opensource.org/licenses/bsd-license.php >>> +# >>> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT >>> +# WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. >>> +# >>> +## >>> + >>> +[Defines] >>> + INF_VERSION = 1.25 >>> + BASE_NAME = FlashNvStorageAddressLib >>> + FILE_GUID = 5FF5A9F9-D01E-49EC-9A17-1682FC85122F >>> + MODULE_TYPE = BASE >>> + VERSION_STRING = 1.0 >>> + LIBRARY_CLASS = FlashNvStorageAddressLib|PEIM >>> + CONSTRUCTOR = SetFlashNvStorageAddresses >>> + >>> +[Sources] >>> + FlashNvStorageAddressLib.c >>> + >>> +[Packages] >>> + MdePkg/MdePkg.dec >>> + MdeModulePkg/MdeModulePkg.dec >>> + OvmfPkg/OvmfPkg.dec >>> + >>> +[LibraryClasses] >>> + PcdLib >>> + >>> +[Pcd] >>> + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase >>> + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase >>> + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 >>> + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageFtwSpareBase >>> + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageFtwWorkingBase >>> + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageVariableBase >>> diff --git a/OvmfPkg/Library/FlashNvStorageAddressLib/FlashNvStorageAddressLib.c b/OvmfPkg/Library/FlashNvStorageAddressLib/FlashNvStorageAddressLib.c >>> new file mode 100644 >>> index 000000000000..dc1280cc23f1 >>> --- /dev/null >>> +++ b/OvmfPkg/Library/FlashNvStorageAddressLib/FlashNvStorageAddressLib.c >>> @@ -0,0 +1,53 @@ >>> +/** @file >>> + >>> + A hook-in library for variable-related PEIMs, in order to set >>> + - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64, >>> + - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase, >>> + - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase, >>> + from their gUefiOvmfPkgTokenSpaceGuid counterparts, just before those PEIMs >>> + consume them. >>> + >>> + Copyright (C) 2017, Red Hat, Inc. >>> + >>> + This program and the accompanying materials are licensed and made available >>> + under the terms and conditions of the BSD License which accompanies this >>> + distribution. The full text of the license may be found at >>> + http://opensource.org/licenses/bsd-license.php >>> + >>> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT >>> + WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. >>> + >>> +**/ >>> + >>> +#include >>> + >>> +RETURN_STATUS >>> +EFIAPI >>> +SetFlashNvStorageAddresses ( >>> + VOID >>> + ) >>> +{ >>> + RETURN_STATUS PcdStatus; >>> + >>> + PcdStatus = PcdSet64S ( >>> + PcdFlashNvStorageVariableBase64, >>> + PcdGet32 (PcdOvmfFlashNvStorageVariableBase) >>> + ); >>> + if (RETURN_ERROR (PcdStatus)) { >>> + return PcdStatus; >>> + } >>> + >>> + PcdStatus = PcdSet32S ( >>> + PcdFlashNvStorageFtwWorkingBase, >>> + PcdGet32 (PcdOvmfFlashNvStorageFtwWorkingBase) >>> + ); >>> + if (RETURN_ERROR (PcdStatus)) { >>> + return PcdStatus; >>> + } >>> + >>> + PcdStatus = PcdSet32S ( >>> + PcdFlashNvStorageFtwSpareBase, >>> + PcdGet32 (PcdOvmfFlashNvStorageFtwSpareBase) >>> + ); >>> + return PcdStatus; >>> +} >>> -- >>> 2.14.1.3.gb7cf6e02401b >>> >>> >