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:c06::22e; helo=mail-io0-x22e.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-io0-x22e.google.com (mail-io0-x22e.google.com [IPv6:2607:f8b0:4001:c06::22e]) (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 B515321A10986 for ; Sat, 2 Dec 2017 00:49:30 -0800 (PST) Received: by mail-io0-x22e.google.com with SMTP id 81so13758496iof.3 for ; Sat, 02 Dec 2017 00:53:57 -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=9TP1awOUaDXJmDCS6wj5OuisbVTOZSN7QQ7oKLA4JcM=; b=JfyRuRK7V2HB0Zs71MmSG5XqHHjKhcTx2bCBegmxTeJZ8AYcr/M03sNxwS1vTn7GxI G7pQkC2mpEZjQ+GCaT2koMW0RQQEPGCXa8mvyhb4Nb0clS34nbY9iAeU/6r+/w6b6iK6 VyguKLv0FzLipE42UccmrC/fESJNk8i9vkN3M= 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=9TP1awOUaDXJmDCS6wj5OuisbVTOZSN7QQ7oKLA4JcM=; b=JKMqdzYr7tnwKqCr+Maf9tX5PfXcg7AwsRqSaWkwuWA6brqgAG59J/A7PIe4bAqmTb mUWYXoHf+3kEGVB+VBa3elr+dUtGOm3GhO9ar7lc13KWjmQMhrHa7mgSN9t6fBe+qBE1 I/ZdD72fGU4JrcwaBnrQVvvvv5FcEH3fUrC5vP7/Fgr8Lard3TOmq7jKv91qz7qgmfJF yhTKhcMEP0gdWONzuhStpLZSyTeRWyCPrfjRG1xDQcZYo2gFCo6vynBvL1+zH+RNvhql mS3EhMDcaihssAmnrwaZttEgEtZE2iLXjr0Aal9GILAHYWzWhzHLObFhV5qpz74fS/uZ B2wA== X-Gm-Message-State: AJaThX7vpFBhO+qQ2jSxCAH7Agpyyb2whSrTMXVUhBF92qtejH1+lelg uVNdYbj3NnOkCdwn8L4AFsXM5gEvV6cf2bID+Z2NBg== X-Google-Smtp-Source: AGs4zMatQ2aOKJM+mR1jFrPZ3wQ0sNXS1IZ5wX1dBdcYt6k+xMAYnOLLgyjmO/HMJIkI9kJFnceGRmpwdPmDtLbpyeM= X-Received: by 10.107.174.222 with SMTP id n91mr15498802ioo.43.1512204836962; Sat, 02 Dec 2017 00:53:56 -0800 (PST) MIME-Version: 1.0 Received: by 10.107.104.16 with HTTP; Sat, 2 Dec 2017 00:53:56 -0800 (PST) In-Reply-To: References: <20171130163029.19743-1-lersek@redhat.com> <20171130163029.19743-4-lersek@redhat.com> From: Ard Biesheuvel Date: Sat, 2 Dec 2017 08:53:56 +0000 Message-ID: To: Laszlo Ersek Cc: Anthony Perard , Jordan Justen , edk2-devel-01 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: Sat, 02 Dec 2017 08:49:30 -0000 Content-Type: text/plain; charset="UTF-8" On 1 December 2017 at 11:41, Laszlo Ersek wrote: > On 12/01/17 11:53, Ard Biesheuvel wrote: >> On 1 December 2017 at 10:52, Laszlo Ersek wrote: >>> On 12/01/17 09:51, Ard Biesheuvel wrote: >>>> 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 >>> >>> This sounds interesting; I've never used patchable PCDs. Can you please >>> elaborate? >>> >>> Do you mean that for (SMM_REQUIRE==TRUE || >>> MEM_VARSTORE_EMU_ENABLE==FALSE), the DSC file should list >>> PcdEmuVariableNvStoreReserved somewhere else? >>> >> >> Yes. >> >>> Does PcdSet work on patchable PCDs? >>> >> >> Yes. It's a bit icky because the value does not actually propagate to >> other modules, but that doesn't matter in this case. > > Hm, this sounds like a recipe. We have two sets of affected PCDs: > > - PcdEmuVariableNvStoreReserved --> in the "flash required" builds, make > it patchable, don't bother setting a good value at build time, and the > PcdSet calls will compile (but won't ever be reached due to code logic) > > - PcdFlashNvStorageVariableBase64, PcdFlashNvStorageFtwWorkingBase, > PcdFlashNvStorageFtwSpareBase: in the "flash required" builds, make them > patchable, *try* to set the right default values at build time, with SET > commands, and the PcdSet calls will compile (but won't ever be reached > due to code logic) > > That could let us drop FlashNvStorageAddressLib. (On patch #6, only the > commit message would change.) > Yes, something like that. I strongly prefer a NULL library class resolution over A PRIORI, but having neither is even better.