Re: [edk2-devel] [PATCH 3/5] OvmfPkg: set fixed FlashNvStorage base addresses with -D SMM_REQUIRE

["Leif Lindholm" <leif@nuviainc.com>]

On Wed, Mar 11, 2020 at 17:41:04 +0100, Laszlo Ersek wrote:
> >> 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.

Would FvmainCompactScratchEnd.fdf.inc be an option?

> (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.<BR>
> #
> #  SPDX-License-Identifier: BSD-2-Clause-Patent
> #
> ##
> 
> Same situation (both DEFINE and SET statements).

However, it *needs* to be included inside the [Defines] section, so
there is no need to get philosofical:
It's OvmfPkgDefines.fdf.inc (or OvmfDefines.fdf.inc).
This aligns 100% with the NetworkPkg example.

> (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.<BR>
> #
> #  SPDX-License-Identifier: BSD-2-Clause-Patent
> #
> ##
> 
> I guess we could rename this to "VarStoreLayoutRegions.fdf.inc" -- but
> would that really help?

OK, so this one is different because it can be included in more than
one location. Also, it's not the sort of thing you need to touch if
you're not already in the sixth circle of hell.

> 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. :)

Sure, this all predates the NetworkPkg fragments (or really any
fragments). It would just be nice to be able to start aligning this
across packages. Splitting up the ARM fragment files is also totally
on the table here.

/
    Leif