Re: [PATCH v2 0/8] OvmfPkg: add the Variable PEIM, defragment the UEFI memmap

[Laszlo Ersek <lersek@redhat.com>]

On 11/30/17 20:00, Jordan Justen wrote:
> As I recall, this enables a modest (non-essential) improvement in
> fragmentation,

My testing from 2 minutes ago shows that the UEFI memmap (as dumped by
the Linux guest) goes from 82 entries to 56 entries. I consider the
defragmentation a QoS question, and the patch set an avenue to further
features like S4.

> at the cost of more ways to configure OVMF to hang
> during boot. (With info available via debugcon.)

Yes.

The configuration is static, the default is sane, and distros can pretty
well decide if they want it or not, same as with SMM_REQUIRE. (Some
package providers offer multiple builds.)

> I would prefer if were able to always fallback and continue boot.

Sure, I would prefer that too. Can someone write it?

- Flash detection in the PEI phase, using the SMM_REQUIRE build, would
require OVMF to raise an SMI, enter SMM, do the detection, and propagate
the result.

- Assuming it works, all we get with it is that OVMF guests launched
with the "-bios" flag (including all OVMF/Xen guests) will get a PEI
phase r/o variable driver that has *zero* chance at returning actual
variable contents (because it cannot read the \NvVars file from the ESP,
even if there is such a partition and/or file.)

Is the above worth it, for users and packagers that consciously set
MEM_VARSTORE_EMU_ENABLE to FALSE at build, and then expect the binary to
boot with "-bios"?

> Later, after GOP has started we could visibly warn the user that they
> haven't properly enabled SMM and/or flash.

GOPs are provided by UEFI_DRIVER modules, generally. SMM/flash are
required much earlier than that.

Perhaps we could use status codes, and report them to the serial port.
Then I assume the complaint would be that (a) users don't understand
status codes, (b) users don't look at the serial port.

> If we had a way to do this, I would be in favor of displaying an error
> message and delaying for 30 seconds before booting if flash has not
> been enabled. (Or, if SMM was enabled in the build, but not set on the
> command line.)

An SMM build *must not* boot on a misconfigured QEMU.

> At that point, I think we could start to transition to making flash an
> boot requirement for OVMF. But first we need to be able to visibly
> warn the user why they are not able to boot.

I don't think this is a reasonable requirement.

First, the series does not make flash a requirement for OVMF, it allows
packagers to make that choice. Which they already have to make if they
consider SMM_REQUIRE.

Second, up till very recent commit d1de487dd2e7 ("MdeModulePkg/BdsDxe:
fall back to a Boot Manager Menu loop before hanging", 2017-11-22), OVMF
(and edk2) wouldn't report a boot failure to the console that was caused
by *much higher level* problems, such as booting a traditional BIOS / OS
disk image without having a CSM and a built-in shell. And when I tried
to introduce edk2-specific status codes for emitting more detail about
the boot process to the user, to the system console, I was asked to talk
to PIWG for standardizing the status codes first.

So on one hand, you want me to report very low level boot requirement
violations through a later, highly abstracted interface; and on the
other hand, when I try to report details of the *same* high abstraction
level, I'm directed to a standards body. I guess I give up. Thanks for
your time.

Laszlo

> 
> -Jordan
> 
> On 2017-11-30 08:30:21, Laszlo Ersek wrote:
>> Repo:   https://github.com/lersek/edk2.git
>> Branch: memmap_defrag_v2
>>
>> This version is identical to the v1 seres I posted in March. Some
>> patches of the v1 series have been committed. I've been rebasing and
>> using the rest since, and am now posting it as v2. References:
>>
>> * https://bugzilla.tianocore.org/show_bug.cgi?id=386
>>
>> * http://mid.mail-archive.com/20170314233246.17864-1-lersek@redhat.com
>>   https://lists.01.org/pipermail/edk2-devel/2017-March/008525.html
>>
>> * http://mid.mail-archive.com/20170327080544.24748-1-jordan.l.justen@intel.com
>>   https://lists.01.org/pipermail/edk2-devel/2017-March/009049.html
>>
>> Cc: Anthony Perard <anthony.perard@citrix.com>
>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Julien Grall <julien.grall@linaro.org>
>>
>> Thanks
>> Laszlo
>>
>> Laszlo Ersek (8):
>>   OvmfPkg: introduce PcdMemVarstoreEmuEnable feature flag
>>   OvmfPkg/QemuFlashFvbServicesRuntimeDxe: check PcdMemVarstoreEmuEnable
>>   OvmfPkg: conditionally disable reserved memory varstore emulation at
>>     build
>>   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/Library/FlashNvStorageAddressLib/FlashNvStorageAddressLib.c   |  53 +++++++
>>  OvmfPkg/Library/FlashNvStorageAddressLib/FlashNvStorageAddressLib.inf |  48 +++++++
>>  OvmfPkg/OvmfPkg.dec                                                   |   6 +
>>  OvmfPkg/OvmfPkgIa32.dsc                                               |  15 +-
>>  OvmfPkg/OvmfPkgIa32.fdf                                               |   8 +-
>>  OvmfPkg/OvmfPkgIa32X64.dsc                                            |  15 +-
>>  OvmfPkg/OvmfPkgIa32X64.fdf                                            |   8 +-
>>  OvmfPkg/OvmfPkgX64.dsc                                                |  15 +-
>>  OvmfPkg/OvmfPkgX64.fdf                                                |   8 +-
>>  OvmfPkg/PlatformPei/MemTypeInfo.c                                     | 151 ++++++++++++++++++++
>>  OvmfPkg/PlatformPei/Platform.c                                        |  26 +---
>>  OvmfPkg/PlatformPei/Platform.h                                        |   5 +
>>  OvmfPkg/PlatformPei/PlatformPei.inf                                   |   3 +
>>  OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf      |   1 +
>>  OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesSmm.inf             |   1 +
>>  OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c               |  58 +++++---
>>  OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c                    |   1 +
>>  OvmfPkg/README                                                        |  10 ++
>>  18 files changed, 385 insertions(+), 47 deletions(-)
>>  create mode 100644 OvmfPkg/Library/FlashNvStorageAddressLib/FlashNvStorageAddressLib.c
>>  create mode 100644 OvmfPkg/Library/FlashNvStorageAddressLib/FlashNvStorageAddressLib.inf
>>  create mode 100644 OvmfPkg/PlatformPei/MemTypeInfo.c
>>
>> -- 
>> 2.14.1.3.gb7cf6e02401b
>>