[PATCH v2 0/1] OVMF: Introduce 16MiB Flash Size

[PATCH v2 0/1] OVMF: Introduce 16MiB Flash Size

[Devon Bautista]

A 4MiB OVMF image is too small for building Linuxboot firmware images
to test with QEMU. This patchset adds the ability to build 16MiB OVMF
images.

Bugzilla:            https://bugzilla.tianocore.org/show_bug.cgi?id=3420
Patch v1:            https://edk2.groups.io/g/devel/topic/83106841
Varstore Discussion: https://edk2.groups.io/g/devel/topic/85034796
Branch:              https://github.com/synackd/edk2/tree/ovmf-16mb-v2

Devon Bautista (1):
  OvmfPkg: Introduce 16MiB flash size for (primarily) Linuxboot

 OvmfPkg/OvmfPkgIa32.dsc        |  4 +++
 OvmfPkg/OvmfPkgIa32X64.dsc     |  4 +++
 OvmfPkg/OvmfPkgX64.dsc         |  4 +++
 OvmfPkg/OvmfXen.dsc            |  4 +++
 OvmfPkg/OvmfPkgDefines.fdf.inc | 28 +++++++++++++++-----
 OvmfPkg/VarStore.fdf.inc       | 16 +++++------
 6 files changed, 45 insertions(+), 15 deletions(-)

-- 
2.33.0

[PATCH v2 1/1] OvmfPkg: Introduce 16MiB flash size for (primarily) Linuxboot

[Devon Bautista]

The largest size flash image currently available for OVMF builds, 4MiB,
is too small to insert a Linux kernel and initramfs into the DXEFV, and
is thus insufficient for testing Linuxboot builds via OVMF.

Introduce the FD_SIZE_16MB build macro (equivalently,
FD_SIZE_IN_KB=16384), which enlarges the full flash image to 16MiB, the
maximum size available for x86. Since QEMU commit 0657c65 (hw/i386/pc:
add max combined fw size as machine configuration option), QEMU supports
flash sizes up to 16MiB using the "max-fw-size" property.

This new 16MiB flash size uses the same non-volatile variable store size
and layout as the default 4MiB flash size to ensure compatibility when
switching to the larger flash size. Since the 4MiB target was created in
commit b24fca0 (OvmfPkg: introduce 4MB flash image (mainly) for Windows
HCK), the variable store size increased by 200KiB to 256KiB, which
should provide an adequate safety margin.

The FVMAIN_COMPACT is significantly enlarged to provide the extra space
in the DXEFV (and PEIFV, if needed).

For now, the 4MiB target remains the default.

In summary:

  Description                Compression type                 Size [KB]
  -------------------------  -----------------  -----------------------
  Non-volatile data storage  open-coded binary    528 ->   528 (    +0)
                               data
    Variable store                                256 ->   256 (    +0)
    Event log                                       4 ->     4 (    +0)
    Working block                                   4 ->     4 (    +0)
    Spare area                                    264 ->   264 (    +0)

  FVMAIN_COMPACT             uncompressed        3360 -> 15648 (+12288)
    FV FFS file              LZMA compressed
      PEIFV                  uncompressed         896 ->   896 (    +0)
        individual PEI       uncompressed
          modules
      DXEFV                  uncompressed       12288 -> 12288 (    +0)
        individual DXE       uncompressed
          modules

  SECFV                      uncompressed         208 ->   208 (    +0)
    SEC driver
    reset vector code

Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Devon Bautista <dbautista@newmexicoconsortium.org>
---
 OvmfPkg/OvmfPkgIa32.dsc        |  4 +++
 OvmfPkg/OvmfPkgIa32X64.dsc     |  4 +++
 OvmfPkg/OvmfPkgX64.dsc         |  4 +++
 OvmfPkg/OvmfXen.dsc            |  4 +++
 OvmfPkg/OvmfPkgDefines.fdf.inc | 28 +++++++++++++++-----
 OvmfPkg/VarStore.fdf.inc       | 16 +++++------
 6 files changed, 45 insertions(+), 15 deletions(-)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index d1d92c97bae3..fadad1c1efda 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -67,11 +67,15 @@ [Defines]
 !else
 !ifdef $(FD_SIZE_4MB)
   DEFINE FD_SIZE_IN_KB           = 4096
+!else
+!ifdef $(FD_SIZE_16MB)
+  DEFINE FD_SIZE_IN_KB           = 16384
 !else
   DEFINE FD_SIZE_IN_KB           = 4096
 !endif
 !endif
 !endif
+!endif
 
 [BuildOptions]
   GCC:RELEASE_*_*_CC_FLAGS             = -DMDEPKG_NDEBUG
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index a467ab7090fb..0e8eae845901 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -66,11 +66,15 @@ [Defines]
 !else
 !ifdef $(FD_SIZE_4MB)
   DEFINE FD_SIZE_IN_KB           = 4096
+!else
+!ifdef $(FD_SIZE_16MB)
+  DEFINE FD_SIZE_IN_KB           = 16384
 !else
   DEFINE FD_SIZE_IN_KB           = 4096
 !endif
 !endif
 !endif
+!endif
 
 [BuildOptions]
   GCC:RELEASE_*_*_CC_FLAGS             = -DMDEPKG_NDEBUG
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index e56b83d95e09..36ccb4418f67 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -66,11 +66,15 @@ [Defines]
 !else
 !ifdef $(FD_SIZE_4MB)
   DEFINE FD_SIZE_IN_KB           = 4096
+!else
+!ifdef $(FD_SIZE_16MB)
+  DEFINE FD_SIZE_IN_KB           = 16384
 !else
   DEFINE FD_SIZE_IN_KB           = 4096
 !endif
 !endif
 !endif
+!endif
 
 [BuildOptions]
   GCC:RELEASE_*_*_CC_FLAGS             = -DMDEPKG_NDEBUG
diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
index 1a9c06c164a8..6a7ce481e895 100644
--- a/OvmfPkg/OvmfXen.dsc
+++ b/OvmfPkg/OvmfXen.dsc
@@ -56,11 +56,15 @@ [Defines]
 !else
 !ifdef $(FD_SIZE_4MB)
   DEFINE FD_SIZE_IN_KB           = 4096
+!else
+!ifdef $(FD_SIZE_16MB)
+  DEFINE FD_SIZE_IN_KB           = 16384
 !else
   DEFINE FD_SIZE_IN_KB           = 2048
 !endif
 !endif
 !endif
+!endif
 
 [BuildOptions]
   GCC:RELEASE_*_*_CC_FLAGS             = -DMDEPKG_NDEBUG
diff --git a/OvmfPkg/OvmfPkgDefines.fdf.inc b/OvmfPkg/OvmfPkgDefines.fdf.inc
index 3b5e45253916..301600c4ef84 100644
--- a/OvmfPkg/OvmfPkgDefines.fdf.inc
+++ b/OvmfPkg/OvmfPkgDefines.fdf.inc
@@ -14,8 +14,8 @@
 # A firmware binary built with FD_SIZE_IN_KB=1024, and a firmware binary built
 # with FD_SIZE_IN_KB=2048, use the same variable store layout.
 #
-# Setting FD_SIZE_IN_KB to 4096 results in a different (much larger) variable
-# store structure that is incompatible with both of the above-mentioned
+# Setting FD_SIZE_IN_KB to 4096 or 16384 results in a different (much larger)
+# variable store structure that is incompatible with both of the above-mentioned
 # firmware binaries.
 #
 !if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
@@ -23,6 +23,13 @@
 DEFINE VARS_BLOCKS       = 0x20
 DEFINE VARS_LIVE_SIZE    = 0xE000
 DEFINE VARS_SPARE_SIZE   = 0x10000
+!else
+!if ($(FD_SIZE_IN_KB) == 16384) || ($(FD_SIZE_IN_KB) == 4096)
+DEFINE VARS_SIZE         = 0x84000
+DEFINE VARS_BLOCKS       = 0x84
+DEFINE VARS_LIVE_SIZE    = 0x40000
+DEFINE VARS_SPARE_SIZE   = 0x42000
+!endif
 !endif
 
 !if $(FD_SIZE_IN_KB) == 1024
@@ -50,11 +57,6 @@
 !endif
 
 !if $(FD_SIZE_IN_KB) == 4096
-DEFINE VARS_SIZE         = 0x84000
-DEFINE VARS_BLOCKS       = 0x84
-DEFINE VARS_LIVE_SIZE    = 0x40000
-DEFINE VARS_SPARE_SIZE   = 0x42000
-
 DEFINE FW_BASE_ADDRESS   = 0xFFC00000
 DEFINE FW_SIZE           = 0x00400000
 DEFINE FW_BLOCKS         = 0x400
@@ -66,6 +68,18 @@
 DEFINE SECFV_SIZE        = 0x34000
 !endif
 
+!if $(FD_SIZE_IN_KB) == 16384
+DEFINE FW_BASE_ADDRESS   = 0xFF000000
+DEFINE FW_SIZE           = 0x01000000
+DEFINE FW_BLOCKS         = 0x1000
+DEFINE CODE_BASE_ADDRESS = 0xFF084000
+DEFINE CODE_SIZE         = 0x00F7C000
+DEFINE CODE_BLOCKS       = 0xF7C
+DEFINE FVMAIN_SIZE       = 0x00F48000
+DEFINE SECFV_OFFSET      = 0x00FCC000
+DEFINE SECFV_SIZE        = 0x34000
+!endif
+
 SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFdBaseAddress     = $(FW_BASE_ADDRESS)
 SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareFdSize    = $(FW_SIZE)
 SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareBlockSize = $(BLOCK_SIZE)
diff --git a/OvmfPkg/VarStore.fdf.inc b/OvmfPkg/VarStore.fdf.inc
index a1e524e39329..179ab64d4642 100644
--- a/OvmfPkg/VarStore.fdf.inc
+++ b/OvmfPkg/VarStore.fdf.inc
@@ -11,7 +11,7 @@
 !if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
 0x00000000|0x0000e000
 !endif
-!if $(FD_SIZE_IN_KB) == 4096
+!if ($(FD_SIZE_IN_KB) == 4096) || ($(FD_SIZE_IN_KB) == 16384)
 0x00000000|0x00040000
 !endif
 #NV_VARIABLE_STORE
@@ -29,7 +29,7 @@
   # FvLength: 0x20000
   0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00,
 !endif
-!if $(FD_SIZE_IN_KB) == 4096
+!if ($(FD_SIZE_IN_KB) == 4096) || ($(FD_SIZE_IN_KB) == 16384)
   # FvLength: 0x84000
   0x00, 0x40, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00,
 !endif
@@ -41,7 +41,7 @@
   # CheckSum
   0x19, 0xF9,
 !endif
-!if $(FD_SIZE_IN_KB) == 4096
+!if ($(FD_SIZE_IN_KB) == 4096) || ($(FD_SIZE_IN_KB) == 16384)
   # CheckSum
   0xAF, 0xB8,
 !endif
@@ -51,7 +51,7 @@
   # Blockmap[0]: 0x20 Blocks * 0x1000 Bytes / Block
   0x20, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00,
 !endif
-!if $(FD_SIZE_IN_KB) == 4096
+!if ($(FD_SIZE_IN_KB) == 4096) || ($(FD_SIZE_IN_KB) == 16384)
   # Blockmap[0]: 0x84 Blocks * 0x1000 Bytes / Block
   0x84, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00,
 !endif
@@ -70,7 +70,7 @@
   # This can speed up the Variable Dispatch a bit.
   0xB8, 0xDF, 0x00, 0x00,
 !endif
-!if $(FD_SIZE_IN_KB) == 4096
+!if ($(FD_SIZE_IN_KB) == 4096) || ($(FD_SIZE_IN_KB) == 16384)
   # Size: 0x40000 (gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize) -
   #          0x48 (size of EFI_FIRMWARE_VOLUME_HEADER) = 0x3ffb8
   # This can speed up the Variable Dispatch a bit.
@@ -83,7 +83,7 @@
 !if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
 0x0000e000|0x00001000
 !endif
-!if $(FD_SIZE_IN_KB) == 4096
+!if ($(FD_SIZE_IN_KB) == 4096) || ($(FD_SIZE_IN_KB) == 16384)
 0x00040000|0x00001000
 !endif
 #NV_EVENT_LOG
@@ -91,7 +91,7 @@
 !if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
 0x0000f000|0x00001000
 !endif
-!if $(FD_SIZE_IN_KB) == 4096
+!if ($(FD_SIZE_IN_KB) == 4096) || ($(FD_SIZE_IN_KB) == 16384)
 0x00041000|0x00001000
 !endif
 #NV_FTW_WORKING
@@ -109,7 +109,7 @@
 !if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
 0x00010000|0x00010000
 !endif
-!if $(FD_SIZE_IN_KB) == 4096
+!if ($(FD_SIZE_IN_KB) == 4096) || ($(FD_SIZE_IN_KB) == 16384)
 0x00042000|0x00042000
 !endif
 #NV_FTW_SPARE
-- 
2.33.0

Re: [PATCH v2 1/1] OvmfPkg: Introduce 16MiB flash size for (primarily) Linuxboot

[Gerd Hoffmann]

On Fri, Sep 03, 2021 at 05:26:20AM +0000, Devon Bautista wrote:
> The largest size flash image currently available for OVMF builds, 4MiB,
> is too small to insert a Linux kernel and initramfs into the DXEFV, and
> is thus insufficient for testing Linuxboot builds via OVMF.

>   FVMAIN_COMPACT             uncompressed        3360 -> 15648 (+12288)
>     FV FFS file              LZMA compressed
>       PEIFV                  uncompressed         896 ->   896 (    +0)
>         individual PEI       uncompressed
>           modules
>       DXEFV                  uncompressed       12288 -> 12288 (    +0)
>         individual DXE       uncompressed
>           modules

Patch looks sane to me.

Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>

How you are going to use the extra space you got?
Do you add kernel + initrd as ffs files to FVMAIN_COMPACT?

take care,
  Gerd

Re: [PATCH v2 1/1] OvmfPkg: Introduce 16MiB flash size for (primarily) Linuxboot

[Devon Bautista]

On 9/3/21 12:17 AM, Gerd Hoffmann wrote:
> On Fri, Sep 03, 2021 at 05:26:20AM +0000, Devon Bautista wrote:
>> The largest size flash image currently available for OVMF builds, 4MiB,
>> is too small to insert a Linux kernel and initramfs into the DXEFV, and
>> is thus insufficient for testing Linuxboot builds via OVMF.
> 
>>   FVMAIN_COMPACT             uncompressed        3360 -> 15648 (+12288)
>>     FV FFS file              LZMA compressed
>>       PEIFV                  uncompressed         896 ->   896 (    +0)
>>         individual PEI       uncompressed
>>           modules
>>       DXEFV                  uncompressed       12288 -> 12288 (    +0)
>>         individual DXE       uncompressed
>>           modules
> 
> Patch looks sane to me.
> 
> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
> 
Thank you for your review.

> How you are going to use the extra space you got?
> Do you add kernel + initrd as ffs files to FVMAIN_COMPACT?
> 
> take care,
>   Gerd
> 
The kernel + initrd are added as an application in DXEFV, and a
"linuxboot" DXE driver is added that dispatches the bzImage.

The Linuxboot build system tries to remove some of the DXE drivers that
are redundant to Linux drivers, but even after this there is still not
enough room to insert the kernel + initrd with the 4MiB build target,
even with XZ kernel and initrd compression.

Best,
Devon

Re: [PATCH v2 1/1] OvmfPkg: Introduce 16MiB flash size for (primarily) Linuxboot

[Gerd Hoffmann]

> >>   FVMAIN_COMPACT             uncompressed        3360 -> 15648 (+12288)
> >>       DXEFV                  uncompressed       12288 -> 12288 (    +0)

> > How you are going to use the extra space you got?
> > Do you add kernel + initrd as ffs files to FVMAIN_COMPACT?

> The kernel + initrd are added as an application in DXEFV, and a
> "linuxboot" DXE driver is added that dispatches the bzImage.

So DXEFV needs more space then.  I'm wondering that the size doesn't
change according to the commit message.  Looking at the fdf files it
seems PEIFV and DXEFV don't have a fixed size, seems everything is
fine as long as the compressed image fits into FVMAIN_COMPACT.

take care,
  Gerd

Re: [PATCH v2 1/1] OvmfPkg: Introduce 16MiB flash size for (primarily) Linuxboot

[Devon Bautista]

> So DXEFV needs more space then.  I'm wondering that the size doesn't
> change according to the commit message.  Looking at the fdf files it
> seems PEIFV and DXEFV don't have a fixed size, seems everything is
> fine as long as the compressed image fits into FVMAIN_COMPACT.
> 
> take care,
>   Gerd
> 
DXEFV would indeed need more space, but I recall that in the first
version of this patch, Laszlo commented:

> (2) [FD.MEMFD] should immediately benefit from this change, even if
> your downstream populates FVMAIN_COMPACT with something else than
> PEIFV and DXEFV. First, we're almost out of (uncompressed) DXEFV space
> again.
> Second, especially the confidential computing technologies have been
> gobbling up the nice, low, free space in FD.MEMFD the way a kid with a
> sweet tooth empties a cookie jar. This change is already compat
> breaking, so I'd like to see *some* proposal (separate patches) for
> enlarging *and pushing up* PEIFV and DXEFV.

For the latter point, I figured that it might be beneficial to expand
FVMAIN_COMPACT as a whole to allow for the possibility of growing either
DXEFV _or_ PEIFV, or both.

Best,
Devon

Re: [edk2-devel] [PATCH v2 1/1] OvmfPkg: Introduce 16MiB flash size for (primarily) Linuxboot

[Philippe Mathieu-Daudé]

On 9/3/21 7:26 AM, Devon Bautista wrote:
> The largest size flash image currently available for OVMF builds, 4MiB,
> is too small to insert a Linux kernel and initramfs into the DXEFV, and
> is thus insufficient for testing Linuxboot builds via OVMF.
> 
> Introduce the FD_SIZE_16MB build macro (equivalently,
> FD_SIZE_IN_KB=16384), which enlarges the full flash image to 16MiB, the
> maximum size available for x86.

I understand this is unavoidable to remove the restriction, but this
will have a negative impact on clouds memory consumption. ARM clouds
are already suffering from having 64MiB flashes, see:
https://lore.kernel.org/all/20201116104216.439650-1-david.edmondson@oracle.com/


Some notes to extend the discussion.

* Why is QEMU using 2 flashes (CODE & DATA)?

  My historical understanding is when OVMF was started, QEMU flash
  model was not supporting sector/bank (write/erase) protection.

  OVMF requirements were:
  - CODE section ("secure", not modifiable by the guest)
  - DATA section (modifiable)

  The easier way to get the CODE secure is to use different devices,
  one enforced in "read-only" mode.

  Being a firmware for virtualized guests, it makes no sense to have
  the guest upgrade the CODE: it is error-prone, and cheaper to do
  directly on the host, rebooting the guest.

* Why not use a ROM for the CODE section and flash for the DATA one?

  This is not clear to me. I suppose the firmware wanted to be able
  to poll the hardware size, and the pflash allow that with CFI I/O
  requests?

* Could we replace the CODE pflash by a ROM device?

  QEMU provides the -fw_cfg device and versioned machines. To the best
  of my knowledge it seems doable, with careful modifications in OVMF
  and ArmVirt.

* What are the benefits of using a ROM for the CODE section?

  - simpler code path, simpler to audit / review, safer
  - reduce migration burden (no pflash device state)
  - reduce memory consumption (backing file mmaped with MAP_SHARED)

* Is there similar problems with DATA section?

  The biggest problem is the memory waste, the pflash model was
  designed to be executable, modifiable, and as fast as possible
  (for execution). This is achieved by copying the whole flash
  content in an internal buffer. For DATA flash this is no speed
  gain but high memory penalty.

* Can the DATA section memory consumption be reduced?

  Yes, various suggestions were posted on QEMU mailing list, but
  nothing accepted so far, this is still a work in progress, and
  ideas are welcomed.

> Since QEMU commit 0657c65 (hw/i386/pc:
> add max combined fw size as machine configuration option), QEMU supports
> flash sizes up to 16MiB using the "max-fw-size" property.
> 
> This new 16MiB flash size uses the same non-volatile variable store size
> and layout as the default 4MiB flash size to ensure compatibility when
> switching to the larger flash size. Since the 4MiB target was created in
> commit b24fca0 (OvmfPkg: introduce 4MB flash image (mainly) for Windows
> HCK), the variable store size increased by 200KiB to 256KiB, which
> should provide an adequate safety margin.

Re: [edk2-devel] [PATCH v2 1/1] OvmfPkg: Introduce 16MiB flash size for (primarily) Linuxboot

[Gerd Hoffmann]

  Hi,

> * Why is QEMU using 2 flashes (CODE & DATA)?
> 
>   My historical understanding is when OVMF was started, QEMU flash
>   model was not supporting sector/bank (write/erase) protection.
> 
>   OVMF requirements were:
>   - CODE section ("secure", not modifiable by the guest)
>   - DATA section (modifiable)
> 
>   The easier way to get the CODE secure is to use different devices,
>   one enforced in "read-only" mode.

Matches my understanding.

>   Being a firmware for virtualized guests, it makes no sense to have
>   the guest upgrade the CODE: it is error-prone, and cheaper to do
>   directly on the host, rebooting the guest.

Having separate binaries also simplifies the host-side firmware update,
you can easily update CODE for all guests while the keeping per-guest
VARS file.

> * Why not use a ROM for the CODE section and flash for the DATA one?
> 
>   This is not clear to me. I suppose the firmware wanted to be able
>   to poll the hardware size, and the pflash allow that with CFI I/O
>   requests?

Size and mapping location are compile-time constants, I don't think ovmf
talks to the pflash to figure the size.

> * Could we replace the CODE pflash by a ROM device?
> 
>   QEMU provides the -fw_cfg device and versioned machines. To the best
>   of my knowledge it seems doable, with careful modifications in OVMF
>   and ArmVirt.

I think ovmf doesn't care.  It'll be more of an issue for qemu, IIRC the
pflash driver has some code to automagically place code and vars at the
correct location, but that'll require both parts of the firmware being
flash.  Might be easier to teach the pflash device to just map read-only
flash like a rom.

If you don't need persistent vars you can also try "-bios OVMF.fd".

> * Is there similar problems with DATA section?
> 
>   The biggest problem is the memory waste, the pflash model was
>   designed to be executable, modifiable, and as fast as possible
>   (for execution). This is achieved by copying the whole flash
>   content in an internal buffer. For DATA flash this is no speed
>   gain but high memory penalty.

Hmm, isn't that long solved?  IIRC kvm memslots can be configured to
only trap on writes since years.  Or is that something unrelated?

[ Disclaimer: I don't know much about the pflash internals ]

take care,
  Gerd

Re: [edk2-devel] [PATCH v2 1/1] OvmfPkg: Introduce 16MiB flash size for (primarily) Linuxboot

[Devon Bautista]

On 9/9/21 03:09, Philippe Mathieu-Daudé wrote:
> On 9/3/21 7:26 AM, Devon Bautista wrote:
>> The largest size flash image currently available for OVMF builds, 4MiB,
>> is too small to insert a Linux kernel and initramfs into the DXEFV, and
>> is thus insufficient for testing Linuxboot builds via OVMF.
>>
>> Introduce the FD_SIZE_16MB build macro (equivalently,
>> FD_SIZE_IN_KB=16384), which enlarges the full flash image to 16MiB, the
>> maximum size available for x86.
> 
> I understand this is unavoidable to remove the restriction, but this
> will have a negative impact on clouds memory consumption. ARM clouds
> are already suffering from having 64MiB flashes, see:
> https://lore.kernel.org/all/20201116104216.439650-1-david.edmondson@oracle.com/
Is ARM still padding flash images with zeros up to 64MB?

Even so, this patch merely introduces the 16MB macro but does not make 
it the default. Genuinely, I am wondering how having this optional build 
macro would conflict with ARM's memory consumption if ARM is already 
using the default 4MB build target for OVMF, unless ARM is already using 
a 16MB build target downstream and this would conflict with that.
> Some notes to extend the discussion.
> 
> * Why is QEMU using 2 flashes (CODE & DATA)?
> 
>    My historical understanding is when OVMF was started, QEMU flash
>    model was not supporting sector/bank (write/erase) protection.
> 
>    OVMF requirements were:
>    - CODE section ("secure", not modifiable by the guest)
>    - DATA section (modifiable)
> 
>    The easier way to get the CODE secure is to use different devices,
>    one enforced in "read-only" mode.
> 
>    Being a firmware for virtualized guests, it makes no sense to have
>    the guest upgrade the CODE: it is error-prone, and cheaper to do
>    directly on the host, rebooting the guest.
> 
> * Why not use a ROM for the CODE section and flash for the DATA one?
> 
>    This is not clear to me. I suppose the firmware wanted to be able
>    to poll the hardware size, and the pflash allow that with CFI I/O
>    requests?
> 
> * Could we replace the CODE pflash by a ROM device?
> 
>    QEMU provides the -fw_cfg device and versioned machines. To the best
>    of my knowledge it seems doable, with careful modifications in OVMF
>    and ArmVirt.
> 
> * What are the benefits of using a ROM for the CODE section?
> 
>    - simpler code path, simpler to audit / review, safer
>    - reduce migration burden (no pflash device state)
>    - reduce memory consumption (backing file mmaped with MAP_SHARED)
> 
> * Is there similar problems with DATA section?
> 
>    The biggest problem is the memory waste, the pflash model was
>    designed to be executable, modifiable, and as fast as possible
>    (for execution). This is achieved by copying the whole flash
>    content in an internal buffer. For DATA flash this is no speed
>    gain but high memory penalty.
> 
> * Can the DATA section memory consumption be reduced?
> 
>    Yes, various suggestions were posted on QEMU mailing list, but
>    nothing accepted so far, this is still a work in progress, and
>    ideas are welcomed.
I'm unsure I have the experience necessary to make an informed comment 
on these points; I think Gerd and/or the other OVMF 
maintainers/reviewers would have better insight.

Best,
Devon