From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f67.google.com (mail-wm1-f67.google.com [209.85.128.67]) by mx.groups.io with SMTP id smtpd.web11.11513.1583943631011924617 for ; Wed, 11 Mar 2020 09:20:31 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20150623.gappssmtp.com header.s=20150623 header.b=ZSZX6DQr; spf=pass (domain: nuviainc.com, ip: 209.85.128.67, mailfrom: leif@nuviainc.com) Received: by mail-wm1-f67.google.com with SMTP id 25so2759851wmk.3 for ; Wed, 11 Mar 2020 09:20:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nuviainc-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=UwPsXCTIhkkNrxtGS8AszqUl9OUt0jkW3/3EnvQ81x0=; b=ZSZX6DQrJjlyxT8i6uYtbu8zH5BXjUY1WgwBH8rlN+DZ4D3ibuBORFXKU7nnImO0Hy Y7XaHayvHC9Xq39lu2H0mw/UtpF2eiuiF2hbAoNAJMxJ5A/3ZcU4U+mNMMlxCIDqBAvR GEJlOAxJKqbMLv2f80ObB99m2N8TWL6ucR87zN43m02qQRa62PZlhYboz15pxcG+vE9Y ynbExlJ8EoothxsGSUXEx+WFUd4x/DbkCZt8WkyPTT6pNhnieRiO+Qn67UBZReTGFOvC dqMZRq+BoRitL3UsgbLJQz1mgAJFTt8vB2TC1z4GjJjRm08fUYezgCDJwTVKraMAmX0m LAEg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=UwPsXCTIhkkNrxtGS8AszqUl9OUt0jkW3/3EnvQ81x0=; b=VY00u91IXpkIXsEsuy8sXc+P968iUYzYkVBMSain6KjGxry2YwHWO+pEFX0AMvNdKc 0VIkHb44wjwDQBlveflVIkty5X1HATDWShe2yveWIZC3BK/GbdpzjTQ0Ctm5yupOmL2g oIh2Jv3wecRB+sr6MG2mc4VyRAl9hbhzQfjS0pSy91rw7oCXLTjApusNqBy4wk+cowCw FexU4r+mG/MmFZi1yO7co7B0kLtxZ1yoaRrDtyhcGxKHEovK18j8WjIw8JqMblq7Ioh8 eGBsNfsyaRtPHlewDnLu0UnP01MIP7NKZqxC6K52Ckk8kNOSvtk7cQUiKSkzRdYb1dII +4VA== X-Gm-Message-State: ANhLgQ3HONkUIvkRILo5Uo1eQD56sDU0htI678vWanNM63AFjDTKJ43x qKcyd6Vqfabjuw0huHp9q/IogSvlOWDSFfqBia5T1S+0P8iXfiKgWrAKvCT8CpudJDYoqy6MGYz 3YyDFBI1iXdrEghb//abdHewZyWs3YDyz+ItIVGfUIWJL2Qw1fCTiikLvwOw35uo= X-Google-Smtp-Source: ADFU+vvPnIuqaTLzixRlQqKKgQlfr6BGnAbmh41Qw+IKzybVrH4WMitxKjyaFQGWUYQmj3sbSKwJKw== X-Received: by 2002:a1c:ac8a:: with SMTP id v132mr4267832wme.64.1583943628848; Wed, 11 Mar 2020 09:20:28 -0700 (PDT) Return-Path: Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id x16sm5379380wrg.44.2020.03.11.09.20.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 11 Mar 2020 09:20:27 -0700 (PDT) Date: Wed, 11 Mar 2020 16:20:26 +0000 From: "Leif Lindholm" To: devel@edk2.groups.io, lersek@redhat.com Cc: Ard Biesheuvel , Jordan Justen , Philippe =?iso-8859-1?Q?Mathieu-Daud=E9?= Subject: Re: [edk2-devel] [PATCH 3/5] OvmfPkg: set fixed FlashNvStorage base addresses with -D SMM_REQUIRE Message-ID: <20200311162026.GX23627@bivouac.eciton.net> References: <20200310222739.26717-1-lersek@redhat.com> <20200310222739.26717-4-lersek@redhat.com> <20200311154449.GR23627@bivouac.eciton.net> <4a412430-d0a3-6310-0d2d-de2da6c00639@redhat.com> MIME-Version: 1.0 In-Reply-To: <4a412430-d0a3-6310-0d2d-de2da6c00639@redhat.com> User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Wed, Mar 11, 2020 at 17:14:53 +0100, Laszlo Ersek wrote: > On 03/11/20 16:44, Leif Lindholm wrote: > > One comment, not on this patch but prompted by it: > > > > On Tue, Mar 10, 2020 at 23:27:37 +0100, Laszlo Ersek wrote: > >> diff --git a/OvmfPkg/OvmfPkg.fdf.inc b/OvmfPkg/OvmfPkg.fdf.inc > >> index 66e0e4d270f5..35fd454b97ab 100644 > >> --- a/OvmfPkg/OvmfPkg.fdf.inc > >> +++ b/OvmfPkg/OvmfPkg.fdf.inc > >> @@ -82,4 +82,10 @@ > > > > I was surprised at not seeing the section header here, so had a look > > at the file, noticed it doesn't have any. And that all files that > > include it do it by: > > > > [Defines] > > !include OvmfPkg.fdf.inc > > > > That looks a bit error-prone and inflexible - could we move/copy the > > header into this file? > > No, please let us not -- I strive to keep all FDF and DSC include files > under OvmfPkg header-free. It gives more flexibility to the includer. I see your point. The generic name suggested to me that it might be *intended* to hold multiple sections, and currently just happened not to. However, to follow a rule of least surprise... > A recent example of this was my request for NetworkPkg to expose its > include snippets header-less, for DSC files. Please see the "!include > NetworkPkg/..." directives in the OVMF DSC files; those are also by > design header-less: > > NetworkPkg/NetworkComponents.dsc.inc > NetworkPkg/NetworkDefines.dsc.inc > NetworkPkg/NetworkLibs.dsc.inc > NetworkPkg/NetworkPcds.dsc.inc ...could OvmfPkg use a similar naming scheme to this? That would also remove the drawback of not having the section name as part of the hunk header, as you'd have it anyway immediately above as part of the file name? Regards, Leif > NetworkPkg does offer (as a convenience) more "canned" includes too, but > those were not flexible enough for OVMF. > > Same for the other FDF include files under OvmfPkg: > - DecomprScratchEnd.fdf.inc > - VarStore.fdf.inc > > An example of where this is actively being put to use is > "VarStore.fdf.inc": it is included both under [FD.OVMF], and [FD.OVMF_VARS]. > > So the treatment of the include files is consistent (I'd not want some > includes with headers, and some without). > > > I realize the include files under ArmVirtPkg do not follow this pattern > (they contain headers); however, out of the files: > > - ArmVirt.dsc.inc > - ArmVirtQemuFvMain.fdf.inc > - ArmVirtRules.fdf.inc > - VarStore.fdf.inc > > the first and the third contain *multiple* headers, so the application > area is arguably different. > > Thanks, > Laszlo > > > > > > / > > Leif > > > >> SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageFtwSpareBase = gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageFtwWorkingBase + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize > >> SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize = $(VARS_SPARE_SIZE) > >> > >> +!if $(SMM_REQUIRE) == TRUE > >> +SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 = gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageVariableBase > >> +SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase = gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageFtwWorkingBase > >> +SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase = gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageFtwSpareBase > >> +!endif > >> + > >> DEFINE MEMFD_BASE_ADDRESS = 0x800000 > >> -- > >> 2.19.1.3.g30247aa5d201 > >> > >> > >> > >> > >> > > > > > >