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
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.
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.
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.justen@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
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.justen@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
On 03/16/17 19:51, Jordan Justen wrote:
> 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"?
(1) PlatformPei would have to dynamically detect whether the pflash
address range behaves as flash or ROM. Same as in the current runtime
DXE driver, QemuFlashFvbServicesRuntimeDxe. Code duplication or quite a
bit of code movement, for factoring it out and sharing it.
(2) If it behaves as ROM, the memory block has to be allocated (like
now). If we're not just rebooting within the VM, that block of memory is
empty. The FTW and variable PEIMs will look for headers in there, so it
must be formatted.
(3) The address of the dynamic allocation has to be propagated to the
FTW and variable PEIMs. This introduces an ordering constraint between
PlatformPei and the FTW & variable PEIMs that we'd have to satisfy somehow.
(4) After all of the above, no variables would be found in the allocated
/ formatted memory block, unless we were just rebooting within the VM.
(I assume we wouldn't want to reload \NvVars from FAT in PEI...)
Considering the current use case, all fresh VM startups would produce
the default memory type info HOB, and wouldn't remedy the memory map
fragmentation. And in the longer term, considering S4, when the VM goes
down for S4 / hibernation, the VM actually goes away (QEMU exits), and
S4 resume would likely not work with a different / fragmented memmap (or
it would be obscurely different if you warm-rebooted once before).
It is technically solvable, but the benefit is very little; it would
just perpetuate the current broken emulation.
>> 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.
(a) The memory preservation works across reboot, yes, but it is not
useful for the memory type information HOB, and consequently (in the
longer term) for S4. The memory type information HOB should carry
information (== implement a long-term maximum search) over several cold
boots (= different QEMU instances) of the same virtual machine.
At every normal boot, the emulated varstore will be empty (and
well-formed only because we'd have to do that manually too, see (2)
above), and any access to it within PEI will be useless.
(b) If users don't set up pflash correctly, the default build will
continue to work for them without any changes at all. This covers Xen as
well.
The strict pflash requirements are only live if users build OVMF with
either one of the following non-default switches:
-D SMM_REQUIRE
or
-D MEM_VARSTORE_EMU_ENABLE=FALSE
Both of these imply "I know what I'm doing".
I see no benefit in enabling the FTW & Variable PEIMs without a
persistent flash varstore. I do see it require a whole lot of work.
If someone wants the FTW & Variable PEIMs real hard under the above (IMO
useless) circumstances, they're welcome to generalize the code / make it
dynamic on top of this series.
> 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.
The disk save (\NvVars) feature is irrelevant for this series. That
feature is based off the "PcdOvmfFlashVariablesEnable" PCD, which is set
by QemuFlashFvbServicesRuntimeDxe, and consumed by Platform BDS.
When OVMF is built with either of the above build switches,
PcdOvmfFlashVariablesEnable is guaranteed to end up as TRUE, so the
\NvVars code is always (dynamically) disabled in those builds.
I don't want to prevent users from continuing to *use* the emulated
varstore in the default build. However, that emulation is obscurely
broken, and has been so for 7 years. QEMU 1.6 (with pflash) has been out
for the second half of that time. I certainly want to stop *developing*
features for the emulated variable store. (If someone does want to, on
top of this first-line enablement, I won't stand in their way, of course.)
Thanks
Laszlo
>
> -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.justen@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
Jordan,
On 03/16/17 22:22, Laszlo Ersek wrote:
> On 03/16/17 19:51, Jordan Justen wrote:
>> 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"?
>
> (1) PlatformPei would have to dynamically detect whether the pflash
> address range behaves as flash or ROM. Same as in the current runtime
> DXE driver, QemuFlashFvbServicesRuntimeDxe. Code duplication or quite a
> bit of code movement, for factoring it out and sharing it.
>
> (2) If it behaves as ROM, the memory block has to be allocated (like
> now). If we're not just rebooting within the VM, that block of memory is
> empty. The FTW and variable PEIMs will look for headers in there, so it
> must be formatted.
>
> (3) The address of the dynamic allocation has to be propagated to the
> FTW and variable PEIMs. This introduces an ordering constraint between
> PlatformPei and the FTW & variable PEIMs that we'd have to satisfy somehow.
>
> (4) After all of the above, no variables would be found in the allocated
> / formatted memory block, unless we were just rebooting within the VM.
> (I assume we wouldn't want to reload \NvVars from FAT in PEI...)
> Considering the current use case, all fresh VM startups would produce
> the default memory type info HOB, and wouldn't remedy the memory map
> fragmentation. And in the longer term, considering S4, when the VM goes
> down for S4 / hibernation, the VM actually goes away (QEMU exits), and
> S4 resume would likely not work with a different / fragmented memmap (or
> it would be obscurely different if you warm-rebooted once before).
>
> It is technically solvable, but the benefit is very little; it would
> just perpetuate the current broken emulation.
>
>>> 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.
>
> (a) The memory preservation works across reboot, yes, but it is not
> useful for the memory type information HOB, and consequently (in the
> longer term) for S4. The memory type information HOB should carry
> information (== implement a long-term maximum search) over several cold
> boots (= different QEMU instances) of the same virtual machine.
>
> At every normal boot, the emulated varstore will be empty (and
> well-formed only because we'd have to do that manually too, see (2)
> above), and any access to it within PEI will be useless.
>
> (b) If users don't set up pflash correctly, the default build will
> continue to work for them without any changes at all. This covers Xen as
> well.
>
> The strict pflash requirements are only live if users build OVMF with
> either one of the following non-default switches:
>
> -D SMM_REQUIRE
>
> or
>
> -D MEM_VARSTORE_EMU_ENABLE=FALSE
>
> Both of these imply "I know what I'm doing".
>
> I see no benefit in enabling the FTW & Variable PEIMs without a
> persistent flash varstore. I do see it require a whole lot of work.
>
> If someone wants the FTW & Variable PEIMs real hard under the above (IMO
> useless) circumstances, they're welcome to generalize the code / make it
> dynamic on top of this series.
>
>> 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.
>
> The disk save (\NvVars) feature is irrelevant for this series. That
> feature is based off the "PcdOvmfFlashVariablesEnable" PCD, which is set
> by QemuFlashFvbServicesRuntimeDxe, and consumed by Platform BDS.
>
> When OVMF is built with either of the above build switches,
> PcdOvmfFlashVariablesEnable is guaranteed to end up as TRUE, so the
> \NvVars code is always (dynamically) disabled in those builds.
>
> I don't want to prevent users from continuing to *use* the emulated
> varstore in the default build. However, that emulation is obscurely
> broken, and has been so for 7 years. QEMU 1.6 (with pflash) has been out
> for the second half of that time. I certainly want to stop *developing*
> features for the emulated variable store. (If someone does want to, on
> top of this first-line enablement, I won't stand in their way, of course.)
ping -- what do you think of my arguments above, please?
While this series is not urgent for me at the moment, flushing it (as
in, committing it) would certainly ease my load.
If you disagree with the above points, I'll have to postpone this work
indefinitely. (The downstream churn has arrived.)
Thanks,
Laszlo
>>> 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.justen@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
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On 2017-03-22 09:58:24, Laszlo Ersek wrote:
> Jordan,
>
> On 03/16/17 22:22, Laszlo Ersek wrote:
> > On 03/16/17 19:51, Jordan Justen wrote:
> >> 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"?
> >
> > (1) PlatformPei would have to dynamically detect whether the pflash
> > address range behaves as flash or ROM. Same as in the current runtime
> > DXE driver, QemuFlashFvbServicesRuntimeDxe. Code duplication or quite a
> > bit of code movement, for factoring it out and sharing it.
> >
> > (2) If it behaves as ROM, the memory block has to be allocated (like
> > now). If we're not just rebooting within the VM, that block of memory is
> > empty. The FTW and variable PEIMs will look for headers in there, so it
> > must be formatted.
> >
> > (3) The address of the dynamic allocation has to be propagated to the
> > FTW and variable PEIMs. This introduces an ordering constraint between
> > PlatformPei and the FTW & variable PEIMs that we'd have to satisfy somehow.
> >
> > (4) After all of the above, no variables would be found in the allocated
> > / formatted memory block, unless we were just rebooting within the VM.
> > (I assume we wouldn't want to reload \NvVars from FAT in PEI...)
> > Considering the current use case, all fresh VM startups would produce
> > the default memory type info HOB, and wouldn't remedy the memory map
> > fragmentation. And in the longer term, considering S4, when the VM goes
> > down for S4 / hibernation, the VM actually goes away (QEMU exits), and
> > S4 resume would likely not work with a different / fragmented memmap (or
> > it would be obscurely different if you warm-rebooted once before).
> >
> > It is technically solvable, but the benefit is very little; it would
> > just perpetuate the current broken emulation.
> >
> >>> 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.
> >
> > (a) The memory preservation works across reboot, yes, but it is not
> > useful for the memory type information HOB, and consequently (in the
> > longer term) for S4. The memory type information HOB should carry
> > information (== implement a long-term maximum search) over several cold
> > boots (= different QEMU instances) of the same virtual machine.
> >
> > At every normal boot, the emulated varstore will be empty (and
> > well-formed only because we'd have to do that manually too, see (2)
> > above), and any access to it within PEI will be useless.
> >
> > (b) If users don't set up pflash correctly, the default build will
> > continue to work for them without any changes at all. This covers Xen as
> > well.
> >
> > The strict pflash requirements are only live if users build OVMF with
> > either one of the following non-default switches:
> >
> > -D SMM_REQUIRE
> >
> > or
> >
> > -D MEM_VARSTORE_EMU_ENABLE=FALSE
> >
> > Both of these imply "I know what I'm doing".
> >
> > I see no benefit in enabling the FTW & Variable PEIMs without a
> > persistent flash varstore. I do see it require a whole lot of work.
> >
> > If someone wants the FTW & Variable PEIMs real hard under the above (IMO
> > useless) circumstances, they're welcome to generalize the code / make it
> > dynamic on top of this series.
> >
> >> 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.
> >
> > The disk save (\NvVars) feature is irrelevant for this series. That
> > feature is based off the "PcdOvmfFlashVariablesEnable" PCD, which is set
> > by QemuFlashFvbServicesRuntimeDxe, and consumed by Platform BDS.
> >
> > When OVMF is built with either of the above build switches,
> > PcdOvmfFlashVariablesEnable is guaranteed to end up as TRUE, so the
> > \NvVars code is always (dynamically) disabled in those builds.
> >
> > I don't want to prevent users from continuing to *use* the emulated
> > varstore in the default build. However, that emulation is obscurely
> > broken, and has been so for 7 years. QEMU 1.6 (with pflash) has been out
> > for the second half of that time. I certainly want to stop *developing*
> > features for the emulated variable store. (If someone does want to, on
> > top of this first-line enablement, I won't stand in their way, of course.)
>
> ping -- what do you think of my arguments above, please?
I guess I'd like to see what it would take to enable it for the EMU
FVB path. I hope I can find some time to look at it this weekend.
Regarding depending more on flash by default, is it correct that Xen
is the only significant thing preventing it? I mean, looking at it
from the qemu/kvm side of things, I would say we could discuss just
building that way by default at this point. I still think we would
want a failure path friendlier than a blind crash, which still leaves
some dependence on EMU FVB so we could boot far enough for a user
visible crash message.
-Jordan
> While this series is not urgent for me at the moment, flushing it (as
> in, committing it) would certainly ease my load.
>
> If you disagree with the above points, I'll have to postpone this work
> indefinitely. (The downstream churn has arrived.)
>
> Thanks,
> Laszlo
>
> >>> 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.justen@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
> >
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On 03/24/17 08:38, Jordan Justen wrote:
> On 2017-03-22 09:58:24, Laszlo Ersek wrote:
>> Jordan,
>>
>> On 03/16/17 22:22, Laszlo Ersek wrote:
>>> On 03/16/17 19:51, Jordan Justen wrote:
>>>> 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"?
>>>
>>> (1) PlatformPei would have to dynamically detect whether the pflash
>>> address range behaves as flash or ROM. Same as in the current runtime
>>> DXE driver, QemuFlashFvbServicesRuntimeDxe. Code duplication or quite a
>>> bit of code movement, for factoring it out and sharing it.
>>>
>>> (2) If it behaves as ROM, the memory block has to be allocated (like
>>> now). If we're not just rebooting within the VM, that block of memory is
>>> empty. The FTW and variable PEIMs will look for headers in there, so it
>>> must be formatted.
>>>
>>> (3) The address of the dynamic allocation has to be propagated to the
>>> FTW and variable PEIMs. This introduces an ordering constraint between
>>> PlatformPei and the FTW & variable PEIMs that we'd have to satisfy somehow.
>>>
>>> (4) After all of the above, no variables would be found in the allocated
>>> / formatted memory block, unless we were just rebooting within the VM.
>>> (I assume we wouldn't want to reload \NvVars from FAT in PEI...)
>>> Considering the current use case, all fresh VM startups would produce
>>> the default memory type info HOB, and wouldn't remedy the memory map
>>> fragmentation. And in the longer term, considering S4, when the VM goes
>>> down for S4 / hibernation, the VM actually goes away (QEMU exits), and
>>> S4 resume would likely not work with a different / fragmented memmap (or
>>> it would be obscurely different if you warm-rebooted once before).
>>>
>>> It is technically solvable, but the benefit is very little; it would
>>> just perpetuate the current broken emulation.
>>>
>>>>> 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.
>>>
>>> (a) The memory preservation works across reboot, yes, but it is not
>>> useful for the memory type information HOB, and consequently (in the
>>> longer term) for S4. The memory type information HOB should carry
>>> information (== implement a long-term maximum search) over several cold
>>> boots (= different QEMU instances) of the same virtual machine.
>>>
>>> At every normal boot, the emulated varstore will be empty (and
>>> well-formed only because we'd have to do that manually too, see (2)
>>> above), and any access to it within PEI will be useless.
>>>
>>> (b) If users don't set up pflash correctly, the default build will
>>> continue to work for them without any changes at all. This covers Xen as
>>> well.
>>>
>>> The strict pflash requirements are only live if users build OVMF with
>>> either one of the following non-default switches:
>>>
>>> -D SMM_REQUIRE
>>>
>>> or
>>>
>>> -D MEM_VARSTORE_EMU_ENABLE=FALSE
>>>
>>> Both of these imply "I know what I'm doing".
>>>
>>> I see no benefit in enabling the FTW & Variable PEIMs without a
>>> persistent flash varstore. I do see it require a whole lot of work.
>>>
>>> If someone wants the FTW & Variable PEIMs real hard under the above (IMO
>>> useless) circumstances, they're welcome to generalize the code / make it
>>> dynamic on top of this series.
>>>
>>>> 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.
>>>
>>> The disk save (\NvVars) feature is irrelevant for this series. That
>>> feature is based off the "PcdOvmfFlashVariablesEnable" PCD, which is set
>>> by QemuFlashFvbServicesRuntimeDxe, and consumed by Platform BDS.
>>>
>>> When OVMF is built with either of the above build switches,
>>> PcdOvmfFlashVariablesEnable is guaranteed to end up as TRUE, so the
>>> \NvVars code is always (dynamically) disabled in those builds.
>>>
>>> I don't want to prevent users from continuing to *use* the emulated
>>> varstore in the default build. However, that emulation is obscurely
>>> broken, and has been so for 7 years. QEMU 1.6 (with pflash) has been out
>>> for the second half of that time. I certainly want to stop *developing*
>>> features for the emulated variable store. (If someone does want to, on
>>> top of this first-line enablement, I won't stand in their way, of course.)
>>
>> ping -- what do you think of my arguments above, please?
>
> I guess I'd like to see what it would take to enable it for the EMU
> FVB path. I hope I can find some time to look at it this weekend.
>
> Regarding depending more on flash by default, is it correct that Xen
> is the only significant thing preventing it?
That is my understanding, yes.
Thanks,
Laszlo
> I mean, looking at it
> from the qemu/kvm side of things, I would say we could discuss just
> building that way by default at this point. I still think we would
> want a failure path friendlier than a blind crash, which still leaves
> some dependence on EMU FVB so we could boot far enough for a user
> visible crash message.
>
> -Jordan
>
>> While this series is not urgent for me at the moment, flushing it (as
>> in, committing it) would certainly ease my load.
>>
>> If you disagree with the above points, I'll have to postpone this work
>> indefinitely. (The downstream churn has arrived.)
>>
>> Thanks,
>> Laszlo
>>
>>>>> 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.justen@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
>>>
>>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
© 2016 - 2024 Red Hat, Inc.