On 2017-03-14 16:32:32, Laszlo Ersek wrote:
> This series implements the ideas discussed in the thread
> 
>   [edk2] memory type information HOB / UEFI memmap defrag
>   https://lists.01.org/pipermail/edk2-devel/2017-February/007576.html
> 
> I'm seeing good results, with approx. 20-24 UEFI memmap entries removed.
> 
> The benefits of the patch set are automatically enabled for SMM_REQUIRE
> builds.
> 
> For non-SMM builds (which are the default), another build flag is being
> introduced, namely MEM_VARSTORE_EMU_ENABLE. This build flag controls the
> reserved memory / NvVars file based variable emulation. It remains
> enabled by default (keeping the current status quo). For benefiting from
> the Variable PEIM and the dependent UEFI memmap defragmentation,
> MEM_VARSTORE_EMU_ENABLE has to be *disabled*. There are two reasons for
> this:
> 
> - The PEI phase FTW and Variable modules need immediate (at-startup)
>   access to the variable store, but the reserved memory based emulation
>   depends on dynamic allocation, plus moving the dynamic pflash
>   detection to PEI is out of question.
> 
> - Even more importantly, reading actual variable contents from the
>   non-pflash varstore would require, in PEI, similar hackery that
>   currently happens in BDS -- that's not going to happen.

What prevents us from enabling the PEI variable stack running using
the memory based "emu fvb"?

> In fact one of the longer term goals with MEM_VARSTORE_EMU_ENABLE is to
> identify what we'll rip out once we finally decide to drop the reserved
> memory / NvVars file based emulation.

I think what we could consider ripping out is the disk save feature.
Since QEMU/KVM (at least in the past) seem to preserve RAM contents
across reboot, I think the memory based buffers should continue to be
a fallback for when users don't get pflash setup correctly.

That said, I'm not sure we should decide to rip out the disk save
feature just yet, unless you think it can simplify things greatly.

-Jordan

> A summary of build modes:
> 
> * default build: works as before, with the Variable PEIM and UEFI memmap
>   defragmentation remaining unavailable.
> 
> * -D SMM_REQUIRE: works as before, with the Variable PEIM and UEFI
>   memmap defragmentation enabled.
> 
>   As a reminder, this build is inherently incompatible with QEMU's
>   "-bios" and "-L" parameters, which will trigger the ASSERT() in
>   QemuFlashInitialize()
>   [OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c], added in commit
>   b963ec494c48 ("OvmfPkg: QemuFlashFvbServicesRuntimeDxe: adhere to -D
>   SMM_REQUIRE", 2015-11-30).
> 
> * -D MEM_VARSTORE_EMU_ENABLE=FALSE: eliminates the reserved memory /
>   NvVars based variable emulation, turns pflash into a hard requirement,
>   and enables the Variable PEIM and the UEFI memmap defragmentation.
> 
>   If such a build is executed with -bios or -L, then the FTW and
>   Variable PEIMs will temporarily see a well-formed, but empty varstore
>   (straight from "OvmfPkg/VarStore.fdf.inc"), and then
>   QemuFlashFvbServicesRuntimeDxe will trip a new ASSERT(), in patch #6,
>   that is similar to the one added in commit b963ec494c48 (see above).
> 
> Anatomy of the series:
> 
> * Patches 01 through 04 clean up some innocent warts I noticed while
>   working on the feature.
> 
> * Patches 05 through 07 introduce the new build option and its basic
>   effect, the disabling of reserved memory based variable emulation.
> 
> * Patches 08 through 12 include the FTW and Variable PEIMs.
> 
> * Patch 13 enables UEFI memmap defragmentation.
> 
> * Patch 14 updates the README file.
> 
> The variable PEIM was independently requested in
> <https://bugzilla.tianocore.org/show_bug.cgi?id=386> earlier, but
> without a good upstreamable use case, I disagreed with its inclusion.
> The memmap defragmentation is a good use case however.
> 
> Also, I didn't lie about "downstream pressure" in my previous patch set;
> I wrote this one at Sunday/Monday night (and now it's Tues/Wed night).
> So one could consider this personal diligence. :)
> 
> The series conforms to the multi-line function call syntax outlined in
> <https://bugzilla.tianocore.org/show_bug.cgi?id=425>.
> 
> Repo:   https://github.com/lersek/edk2.git
> Branch: memmap_defrag
> 
> CC: Jordan Justen <jordan.l.jus...@intel.com>
> 
> Thanks
> Laszlo
> 
> Laszlo Ersek (14):
>   OvmfPkg/EmuVariableFvbRuntimeDxe: always format an auth varstore
>     header
>   OvmfPkg: remove gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable
>   OvmfPkg/PlatformPei: remove unused PcdVariableStoreSize dependency
>   OvmfPkg/PlatformPei: don't allocate reserved mem varstore if
>     SMM_REQUIRE
>   OvmfPkg: introduce PcdMemVarstoreEmuEnable feature flag
>   OvmfPkg/QemuFlashFvbServicesRuntimeDxe: check PcdMemVarstoreEmuEnable
>   OvmfPkg: conditionally disable reserved memory varstore emulation at
>     build
>   OvmfPkg: resolve PcdLib for all PEIMs individually
>   OvmfPkg: resolve PcdLib for PEIMs to PeiPcdLib by default
>   OvmfPkg: introduce FlashNvStorageAddressLib
>   OvmfPkg: include FaultTolerantWritePei and VariablePei
>   OvmfPkg/QemuFlashFvbServicesRuntimeDxe: don't set flash PCDs if SMM or
>     no-emu
>   OvmfPkg/PlatformPei: remedy UEFI memory map fragmentation
>   OvmfPkg/README: document MEM_VARSTORE_EMU_ENABLE and memmap defrag
> 
>  OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c                                |  79 
> +---------
>  OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.inf                              |   3 -
>  OvmfPkg/Library/FlashNvStorageAddressLib/FlashNvStorageAddressLib.c   |  53 
> +++++++
>  OvmfPkg/Library/FlashNvStorageAddressLib/FlashNvStorageAddressLib.inf |  48 
> +++++++
>  OvmfPkg/OvmfPkg.dec                                                   |   7 
> +-
>  OvmfPkg/OvmfPkgIa32.dsc                                               |  43 
> +++---
>  OvmfPkg/OvmfPkgIa32.fdf                                               |   8 
> +-
>  OvmfPkg/OvmfPkgIa32X64.dsc                                            |  43 
> +++---
>  OvmfPkg/OvmfPkgIa32X64.fdf                                            |   8 
> +-
>  OvmfPkg/OvmfPkgX64.dsc                                                |  43 
> +++---
>  OvmfPkg/OvmfPkgX64.fdf                                                |   8 
> +-
>  OvmfPkg/PlatformPei/MemTypeInfo.c                                     | 151 
> ++++++++++++++++++++
>  OvmfPkg/PlatformPei/Platform.c                                        |  28 
> +---
>  OvmfPkg/PlatformPei/Platform.h                                        |   5 +
>  OvmfPkg/PlatformPei/PlatformPei.inf                                   |   4 
> +-
>  OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf      |   1 +
>  OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesSmm.inf             |   1 +
>  OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c               |  58 
> +++++---
>  OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c                    |   1 +
>  OvmfPkg/README                                                        |  10 
> ++
>  20 files changed, 425 insertions(+), 177 deletions(-)
>  create mode 100644 
> OvmfPkg/Library/FlashNvStorageAddressLib/FlashNvStorageAddressLib.inf
>  create mode 100644 
> OvmfPkg/Library/FlashNvStorageAddressLib/FlashNvStorageAddressLib.c
>  create mode 100644 OvmfPkg/PlatformPei/MemTypeInfo.c
> 
> -- 
> 2.9.3
> 
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to