<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<p>Laszlo,</p>
<p>I apologize for the delay. I've been heavily preoccupied in
another work program I am involved in.<br>
</p>
<div class="moz-cite-prefix">On 5/27/21 3:12 AM, Laszlo Ersek wrote:<br>
</div>
<blockquote type="cite"
cite="mid:c08d6d1b-b1ae-c6f7-3db4-562ccbaac5e8@redhat.com">
<pre class="moz-quote-pre" wrap="">On 05/26/21 19:08, Devon Bautista wrote:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">Currently, the largest volume size for building OVMF images is 4MB. With
the growth of the Linuxboot project, maintainers have had to maintain a
fork containing this patch which allows larger image sizes in order for
Linuxboot developers/users to have enough space to experiment with
and test including their own Linux kernel in the DXE section of OVMF
firmware. Testing using OVMF is valuable since it allows testing in QEMU
and thus does not require any hardware to do so.
This patch allows specifying '-D FD_SIZE_8MB' or '-D FD_SIZE_16MB' to
the OVMF build script in order to add the ability to build 8MB or 16MB
x86_64 (X64) OVMF images, respectively.
Signed-off-by: Devon Bautista <a class="moz-txt-link-rfc2396E" href="mailto:dbautista@newmexicoconsortium.org"><dbautista@newmexicoconsortium.org></a>
---
OvmfPkg/OvmfPkgDefines.fdf.inc | 34 ++++++++++++++++++++++++++++++++++
OvmfPkg/OvmfPkgX64.dsc | 10 +++++++++-
OvmfPkg/VarStore.fdf.inc | 16 ++++++++--------
3 files changed, 51 insertions(+), 9 deletions(-)
diff --git a/OvmfPkg/OvmfPkgDefines.fdf.inc b/OvmfPkg/OvmfPkgDefines.fdf.inc
index 35fd454b97..da37758934 100644
--- a/OvmfPkg/OvmfPkgDefines.fdf.inc
+++ b/OvmfPkg/OvmfPkgDefines.fdf.inc
@@ -66,6 +66,40 @@ DEFINE SECFV_OFFSET = 0x003CC000
DEFINE SECFV_SIZE = 0x34000
!endif
+!if $(FD_SIZE_IN_KB) == 8192
+DEFINE VARS_SIZE = 0x84000
+DEFINE VARS_BLOCKS = 0x84
+DEFINE VARS_LIVE_SIZE = 0x40000
+DEFINE VARS_SPARE_SIZE = 0x42000
+
+DEFINE FW_BASE_ADDRESS = 0xFF800000
+DEFINE FW_SIZE = 0x00800000
+DEFINE FW_BLOCKS = 0x800
+DEFINE CODE_BASE_ADDRESS = 0xFF884000
+DEFINE CODE_SIZE = 0x0077C000
+DEFINE CODE_BLOCKS = 0x77C
+DEFINE FVMAIN_SIZE = 0x00748000
+DEFINE SECFV_OFFSET = 0x007CC000
+DEFINE SECFV_SIZE = 0x34000
+!endif
+
+!if $(FD_SIZE_IN_KB) == 16384
+DEFINE VARS_SIZE = 0x84000
+DEFINE VARS_BLOCKS = 0x84
+DEFINE VARS_LIVE_SIZE = 0x40000
+DEFINE VARS_SPARE_SIZE = 0x42000
+
+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/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 999738dc39..28351e2f56 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -66,11 +66,19 @@
!else
!ifdef $(FD_SIZE_4MB)
DEFINE FD_SIZE_IN_KB = 4096
+!else
+!ifdef $(FD_SIZE_8MB)
+ DEFINE FD_SIZE_IN_KB = 8192
+!else
+!ifdef $(FD_SIZE_16MB)
+ DEFINE FD_SIZE_IN_KB = 16384
!else
DEFINE FD_SIZE_IN_KB = 4096
!endif
!endif
!endif
+!endif
+!endif
[BuildOptions]
GCC:RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG
@@ -501,7 +509,7 @@
gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000
!endif
!endif
-!if $(FD_SIZE_IN_KB) == 4096
+!if $(FD_SIZE_IN_KB) == 4096 || $(FD_SIZE_IN_KB) == 8196 || $(FD_SIZE_IN_KB) == 16384
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400
!if $(NETWORK_TLS_ENABLE) == FALSE
diff --git a/OvmfPkg/VarStore.fdf.inc b/OvmfPkg/VarStore.fdf.inc
index a1e524e393..70db929478 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) == 8192) || ($(FD_SIZE_IN_KB) == 16384)
0x00000000|0x00040000
!endif
#NV_VARIABLE_STORE
@@ -29,7 +29,7 @@ DATA = {
# 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) == 8192) || ($(FD_SIZE_IN_KB) == 16384)
# FvLength: 0x84000
0x00, 0x40, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00,
!endif
@@ -41,7 +41,7 @@ DATA = {
# CheckSum
0x19, 0xF9,
!endif
-!if $(FD_SIZE_IN_KB) == 4096
+!if ($(FD_SIZE_IN_KB) == 4096) || ($(FD_SIZE_IN_KB) == 8192) || ($(FD_SIZE_IN_KB) == 16384)
# CheckSum
0xAF, 0xB8,
!endif
@@ -51,7 +51,7 @@ DATA = {
# 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) == 8192) || ($(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 @@ DATA = {
# 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) == 8192) || ($(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 @@ DATA = {
!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) == 8192) || ($(FD_SIZE_IN_KB) == 16384)
0x00040000|0x00001000
!endif
#NV_EVENT_LOG
@@ -91,7 +91,7 @@ DATA = {
!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) == 8192) || ($(FD_SIZE_IN_KB) == 16384)
0x00041000|0x00001000
!endif
#NV_FTW_WORKING
@@ -109,7 +109,7 @@ DATA = {
!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) == 8192) || ($(FD_SIZE_IN_KB) == 16384)
0x00042000|0x00042000
!endif
#NV_FTW_SPARE
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
I'm providing minimal feedback here just to get this review off my plate
as quickly as possible. Sorry, I'm collapsing under my TODO list.
(1) Every such change is compatibility breaking, so we *must* use the
opportunity at once to *significantly increase* the non-volatile
variable store size as well.
We need to discuss this question with OS vendors and hardware platform
vendors on this list, to see what physical flash sizes are expected in
the future, and we must add a good safety margin on top of that.
The primary concern is with the dbx variable growing without bounds over
time.
Once we introduce a new FD_SIZE_IN_KB option, we're stuck with its
varstore layout forever, so we'd better get it right and future-proof at
once.
</pre>
</blockquote>
<p>Would you prefer to see a new patchset version addressing points
(3) through (5) and adding the plaintext spreadsheet of the region
offsets/sizes based on Brian's suggestion and discussing this
point on that new thread? Or would you prefer a new discussion on
the "discuss' list? Or use this thread?<br>
</p>
<p>I've already started on the next patchset version, but can wait
on submitting for more discussion if desired.
</p>
<blockquote type="cite"
cite="mid:c08d6d1b-b1ae-c6f7-3db4-562ccbaac5e8@redhat.com">
<pre class="moz-quote-pre" wrap="">(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.</pre>
</blockquote>
I suppose it would be sensible to work this out after this patchset
gets finalized, but your input is greatly welcomed here.
<blockquote type="cite"
cite="mid:c08d6d1b-b1ae-c6f7-3db4-562ccbaac5e8@redhat.com">
<pre class="moz-quote-pre" wrap="">(3) Unfortunately, I have to agree that introducing *both* a 8MB option
*and* a 16MB option is justified, per QEMU commit 0657c657eb37
("hw/i386/pc: add max combined fw size as machine configuration option",
2020-12-09).
However, please add each option in a separate patch.
</pre>
</blockquote>
That makes sense for logical separation and atomic commits. Will do.
<blockquote type="cite"
cite="mid:c08d6d1b-b1ae-c6f7-3db4-562ccbaac5e8@redhat.com">
<pre class="moz-quote-pre" wrap="">(4) Dumping a bunch of magic numbers on reviewers is unhelpful. I'll
need to sit down with a calculator and go through the patch with a
magnifying glass. Please support that work by creating a commit message
(summary table) similar to the one in commit b24fca05751f ("OvmfPkg:
introduce 4MB flash image (mainly) for Windows HCK", 2017-05-05).
</pre>
</blockquote>
In light of Brian's suggestion, would you prefer having such a
summary table both in the commit message <i>and</i> in the OVMF
source tree or just the latter?
<blockquote type="cite"
cite="mid:c08d6d1b-b1ae-c6f7-3db4-562ccbaac5e8@redhat.com">
<pre class="moz-quote-pre" wrap="">(5) Modifying *only* "OvmfPkg/OvmfPkgX64.dsc" doesn't seem right, there
are other DSC files (platforms) in OvmfPkg that would benefit. Without
much thinking for now, I'd say the new options should be available in
each DSC (platform description), even the 32-bit ones.
</pre>
</blockquote>
You're right. I'll update the rest of the DSC files with the changes
in the next patchset version.
<blockquote type="cite"
cite="mid:c08d6d1b-b1ae-c6f7-3db4-562ccbaac5e8@redhat.com">
<pre class="moz-quote-pre" wrap="">I'm extremely annoyed by the general trend that the firmware (the OS
under the OS) keeps growing. Because of that, Linuxboot is a fantastic
project. I'd like OVMF to support the development of Linuxboot. I
welcome this patch for that reason.
But I'd also like OVMF to benefit from this change even when it is built
with a traditional -- and regrettably, ever-growing -- DXE phase. I
welcome this patch for that reason too.
</pre>
</blockquote>
<p>Thank you for your feedback. I'm joyed to hear of your support.
I, too, think both Linuxboot and OVMF proper could benefit from
this.</p>
<p>Best,<br>
Devon Bautista<br>
</p>
<br>
</body>
</html>
<div width="1" style="color:white;clear:both">_._,_._,_</div> <hr> Groups.io Links:<p> You receive all messages sent to this group. <p> <a target="_blank" href="https://edk2.groups.io/g/devel/message/78308">View/Reply Online (#78308)</a> | | <a target="_blank" href="https://groups.io/mt/83106841/1813853">Mute This Topic</a> | <a href="https://edk2.groups.io/g/devel/post">New Topic</a><br> <a href="https://edk2.groups.io/g/devel/editsub/1813853">Your Subscription</a> | <a href="mailto:devel+owner@edk2.groups.io">Contact Group Owner</a> | <a href="https://edk2.groups.io/g/devel/unsub">Unsubscribe</a> [edk2-devel-archive@redhat.com]<br> <div width="1" style="color:white;clear:both">_._,_._,_</div>