From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [207.211.31.120]) by mx.groups.io with SMTP id smtpd.web12.11198.1583943302260945868 for ; Wed, 11 Mar 2020 09:15:02 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=iPDSYNC9; spf=pass (domain: redhat.com, ip: 207.211.31.120, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1583943301; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=QuiI1FqgTcoNbT8WJI2lwJVeDajPsFrm9zwb880wsTM=; b=iPDSYNC9sT3J9qAP04m5lDgfXIoYTTFUXXw67c5gDyiuOTii/xq9CShx/AZME8g1Tgfn/0 oTcn1Fv8+wOJNZELU5bbihgKN1+UXZM65hU0EHV+3mgokImo1k8fgK6w7jcUbVKLGcMkcr sSSZIkaimO8YPhm+lZBFodAYxVJmkfs= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-39-h92zK7sEPxG6V-D61u59wQ-1; Wed, 11 Mar 2020 12:14:56 -0400 X-MC-Unique: h92zK7sEPxG6V-D61u59wQ-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 6D66461269; Wed, 11 Mar 2020 16:14:55 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (unknown [10.36.119.12]) by smtp.corp.redhat.com (Postfix) with ESMTP id 3A595512F1; Wed, 11 Mar 2020 16:14:54 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH 3/5] OvmfPkg: set fixed FlashNvStorage base addresses with -D SMM_REQUIRE To: Leif Lindholm , devel@edk2.groups.io Cc: Ard Biesheuvel , Jordan Justen , =?UTF-8?Q?Philippe_Mathieu-Daud=c3=a9?= References: <20200310222739.26717-1-lersek@redhat.com> <20200310222739.26717-4-lersek@redhat.com> <20200311154449.GR23627@bivouac.eciton.net> From: "Laszlo Ersek" Message-ID: <4a412430-d0a3-6310-0d2d-de2da6c00639@redhat.com> Date: Wed, 11 Mar 2020 17:14:53 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20200311154449.GR23627@bivouac.eciton.net> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit 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. 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 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 >> >> >> >> >> >