From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.61]) by mx.groups.io with SMTP id smtpd.web10.11974.1583944874403827937 for ; Wed, 11 Mar 2020 09:41:14 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=cOFxR9VG; spf=pass (domain: redhat.com, ip: 205.139.110.61, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1583944873; 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=eFWYhs38bC6+t4z0hkl3ybMWKl5Gq/JGaIVbIPPy3tg=; b=cOFxR9VGHesdNNsOx5HcAtnAjYhccLoReBAzC/4fkqkaK5/ogUdDr1vnm4McVHi5PJAJhx 9rj3b5zmDjrqMsjnMHPhYeqRz2BElNeSHaq4DZtyJxs0VvPiCBw6d+cXUxy9UuRXng5+3H W5vndhtidR0jQhzVJnve/+6g9mfrpTI= 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-178-9s5X4yAZNoOByfqyxJiAYg-1; Wed, 11 Mar 2020 12:41:11 -0400 X-MC-Unique: 9s5X4yAZNoOByfqyxJiAYg-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 9FC84800EBC; Wed, 11 Mar 2020 16:41:10 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (unknown [10.36.119.12]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4B79173880; Wed, 11 Mar 2020 16:41:06 +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> <4a412430-d0a3-6310-0d2d-de2da6c00639@redhat.com> <20200311162026.GX23627@bivouac.eciton.net> From: "Laszlo Ersek" Message-ID: Date: Wed, 11 Mar 2020 17:41:04 +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: <20200311162026.GX23627@bivouac.eciton.net> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 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 17:20, Leif Lindholm wrote: > 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? There are three FDF include files: (1) DecomprScratchEnd.fdf.inc (included under [FV.FVMAIN_COMPACT], per commit 9beac0d847bf9): ## @file # This FDF include file computes the end of the scratch buffer used in # DecompressMemFvs() [OvmfPkg/Sec/SecMain.c]. It is based on the decompressed # (ie. original) size of the LZMA-compressed section of the one FFS file in # the FVMAIN_COMPACT firmware volume. # # Copyright (C) 2015, Red Hat, Inc. # # SPDX-License-Identifier: BSD-2-Clause-Patent ## This include file contains DEFINE and SET statements (for setting macros and PCDs, accordingly). I don't think either a "Defines" or "Pcds" suffix would apply. (2) OvmfPkg.fdf.inc (included under [Defines]): ## @file # FDF include file that defines the main macros and sets the dependent PCDs. # # Copyright (C) 2014, Red Hat, Inc. # Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.
# # SPDX-License-Identifier: BSD-2-Clause-Patent # ## Same situation (both DEFINE and SET statements). (3) VarStore.fdf.inc (included under [FD.OVMF] and [FD.OVMF_VARS]): ## @file # FDF include file with Layout Regions that define an empty variable store. # # Copyright (C) 2014, Red Hat, Inc. # Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.
# # SPDX-License-Identifier: BSD-2-Clause-Patent # ## I guess we could rename this to "VarStoreLayoutRegions.fdf.inc" -- but would that really help? I'm not against introducing helpful name suffixes, I just can't find anything that really fits and significantly improves upon the current names. I vaguely remember racking my brain when I was introducing these files, for better names, and this is what I had come up with. :) Thanks, Laszlo