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 >> >> >> >> >> > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#55762): https://edk2.groups.io/g/devel/message/55762 Mute This Topic: https://groups.io/mt/71867510/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
