* [PATCH v2 0/5] OvmfPkg: add FD_SIZE_4MB for Windows HCK SB tests, and for future proofing
@ 2017-05-03 21:39 Laszlo Ersek
2017-05-03 21:39 ` [PATCH v2 1/5] OvmfPkg: introduce the FD_SIZE_IN_KB macro / build flag Laszlo Ersek
` (4 more replies)
0 siblings, 5 replies; 17+ messages in thread
From: Laszlo Ersek @ 2017-05-03 21:39 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Gary Ching-Pang Lin, Jordan Justen
This is version 2 of the series posted at
<https://lists.01.org/pipermail/edk2-devel/2017-April/010334.html>.
Changes in v2:
- consolidate FD_SIZE_1MB, FD_SIZE_2MB, FD_SIZE_4MB, basing them all on
the new macro FD_SIZE_IN_KB,
- increase the variable store from 248 KB to 256 KB, and propagate that
change as necessary, at the expense of FVMAIN_COMPACT [Jordan],
- make the 4MB build the general default [Jordan].
V1->v2 changes are also marked on the individual patches.
Thoroughly retested with the usual S3 stuff, the Secure Boot Logo Test,
multiple purposefully triggered reclaims, hexdumps of varstore files,
etc.
Formatted with 13 lines of context for easier review.
Repo: https://github.com/lersek/edk2.git
Branch: fd_size_4mb_v2
Cc: Gary Ching-Pang Lin <glin@suse.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Thanks,
Laszlo
Laszlo Ersek (5):
OvmfPkg: introduce the FD_SIZE_IN_KB macro / build flag
OvmfPkg/OvmfPkg.fdf.inc: extract VARS_LIVE_SIZE and VARS_SPARE_SIZE
macros
OvmfPkg: introduce 4MB flash image (mainly) for Windows HCK
OvmfPkg: raise max variable size (auth & non-auth) to 33KB for
FD_SIZE_4MB
OvmfPkg: make the 4MB flash size the default
OvmfPkg/OvmfPkgIa32.dsc | 25 +++++++++
OvmfPkg/OvmfPkgIa32X64.dsc | 25 +++++++++
OvmfPkg/OvmfPkgX64.dsc | 25 +++++++++
OvmfPkg/OvmfPkg.fdf.inc | 46 ++++++++++++-----
OvmfPkg/VarStore.fdf.inc | 54 +++++++++++++++++++-
5 files changed, 161 insertions(+), 14 deletions(-)
--
2.9.3
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH v2 1/5] OvmfPkg: introduce the FD_SIZE_IN_KB macro / build flag 2017-05-03 21:39 [PATCH v2 0/5] OvmfPkg: add FD_SIZE_4MB for Windows HCK SB tests, and for future proofing Laszlo Ersek @ 2017-05-03 21:39 ` Laszlo Ersek 2017-05-03 21:39 ` [PATCH v2 2/5] OvmfPkg/OvmfPkg.fdf.inc: extract VARS_LIVE_SIZE and VARS_SPARE_SIZE macros Laszlo Ersek ` (3 subsequent siblings) 4 siblings, 0 replies; 17+ messages in thread From: Laszlo Ersek @ 2017-05-03 21:39 UTC (permalink / raw) To: edk2-devel-01; +Cc: Gary Ching-Pang Lin, Jordan Justen FD_SIZE_xMB defines have existed for flash size selection. They can be passed as "-D FD_SIZE_xMB" on the command line. Passing multiple of them at the same time has never been supported; earlier settings on the command line cannot be overridden. Introduce the integer valued FD_SIZE_IN_KB macro, which provides the following improvements: - several instances of it are permitted on the command line, with the last one taking effect, - conditional statements in the DSC and FDF files need only check a single macro, and multiple values can be checked in a single !if with the || operator, - nested !ifdef / !else ladders can be replaced with flat equality tests, - in the future, flash sizes can be expressed with a finer than MB granularity, if necessary. For now, we're going to preserve the FD_SIZE_xMB defines as convenience wrappers for FD_SIZE_IN_KB. FD_SIZE_IN_KB is being added to the DSC files because this way we can depend on it in both the DSC and FDF files. Cc: Gary Ching-Pang Lin <glin@suse.com> Cc: Jordan Justen <jordan.l.justen@intel.com> Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- Notes: v2: - new in v2 OvmfPkg/OvmfPkgIa32.dsc | 15 +++++++++++++++ OvmfPkg/OvmfPkgIa32X64.dsc | 15 +++++++++++++++ OvmfPkg/OvmfPkgX64.dsc | 15 +++++++++++++++ OvmfPkg/OvmfPkg.fdf.inc | 14 +++----------- 4 files changed, 48 insertions(+), 11 deletions(-) diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc index 0796b0db816b..5a21840a55c9 100644 --- a/OvmfPkg/OvmfPkgIa32.dsc +++ b/OvmfPkg/OvmfPkgIa32.dsc @@ -30,26 +30,41 @@ [Defines] SKUID_IDENTIFIER = DEFAULT FLASH_DEFINITION = OvmfPkg/OvmfPkgIa32.fdf # # Defines for default states. These can be changed on the command line. # -D FLAG=VALUE # DEFINE SECURE_BOOT_ENABLE = FALSE DEFINE NETWORK_IP6_ENABLE = FALSE DEFINE HTTP_BOOT_ENABLE = FALSE DEFINE SMM_REQUIRE = FALSE DEFINE TLS_ENABLE = FALSE + # + # Flash size selection. Setting FD_SIZE_IN_KB on the command line directly to + # one of the supported values, in place of any of the convenience macros, is + # permitted. + # +!ifdef $(FD_SIZE_1MB) + DEFINE FD_SIZE_IN_KB = 1024 +!else +!ifdef $(FD_SIZE_2MB) + DEFINE FD_SIZE_IN_KB = 2048 +!else + DEFINE FD_SIZE_IN_KB = 2048 +!endif +!endif + [BuildOptions] GCC:*_UNIXGCC_*_CC_FLAGS = -DMDEPKG_NDEBUG GCC:RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG INTEL:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG MSFT:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG GCC:*_*_*_CC_FLAGS = -mno-mmx -mno-sse # # Disable deprecated APIs. # MSFT:*_*_*_CC_FLAGS = /D DISABLE_NEW_DEPRECATED_INTERFACES INTEL:*_*_*_CC_FLAGS = /D DISABLE_NEW_DEPRECATED_INTERFACES GCC:*_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc index 71ac62f023b5..11866b7207c7 100644 --- a/OvmfPkg/OvmfPkgIa32X64.dsc +++ b/OvmfPkg/OvmfPkgIa32X64.dsc @@ -30,26 +30,41 @@ [Defines] SKUID_IDENTIFIER = DEFAULT FLASH_DEFINITION = OvmfPkg/OvmfPkgIa32X64.fdf # # Defines for default states. These can be changed on the command line. # -D FLAG=VALUE # DEFINE SECURE_BOOT_ENABLE = FALSE DEFINE NETWORK_IP6_ENABLE = FALSE DEFINE HTTP_BOOT_ENABLE = FALSE DEFINE SMM_REQUIRE = FALSE DEFINE TLS_ENABLE = FALSE + # + # Flash size selection. Setting FD_SIZE_IN_KB on the command line directly to + # one of the supported values, in place of any of the convenience macros, is + # permitted. + # +!ifdef $(FD_SIZE_1MB) + DEFINE FD_SIZE_IN_KB = 1024 +!else +!ifdef $(FD_SIZE_2MB) + DEFINE FD_SIZE_IN_KB = 2048 +!else + DEFINE FD_SIZE_IN_KB = 2048 +!endif +!endif + [BuildOptions] GCC:*_UNIXGCC_*_CC_FLAGS = -DMDEPKG_NDEBUG GCC:RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG INTEL:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG MSFT:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG GCC:*_*_*_CC_FLAGS = -mno-mmx -mno-sse !ifdef $(SOURCE_DEBUG_ENABLE) MSFT:*_*_X64_GENFW_FLAGS = --keepexceptiontable GCC:*_*_X64_GENFW_FLAGS = --keepexceptiontable INTEL:*_*_X64_GENFW_FLAGS = --keepexceptiontable !endif # diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc index 2ceb31d7ffd5..2fab544600f5 100644 --- a/OvmfPkg/OvmfPkgX64.dsc +++ b/OvmfPkg/OvmfPkgX64.dsc @@ -30,26 +30,41 @@ [Defines] SKUID_IDENTIFIER = DEFAULT FLASH_DEFINITION = OvmfPkg/OvmfPkgX64.fdf # # Defines for default states. These can be changed on the command line. # -D FLAG=VALUE # DEFINE SECURE_BOOT_ENABLE = FALSE DEFINE NETWORK_IP6_ENABLE = FALSE DEFINE HTTP_BOOT_ENABLE = FALSE DEFINE SMM_REQUIRE = FALSE DEFINE TLS_ENABLE = FALSE + # + # Flash size selection. Setting FD_SIZE_IN_KB on the command line directly to + # one of the supported values, in place of any of the convenience macros, is + # permitted. + # +!ifdef $(FD_SIZE_1MB) + DEFINE FD_SIZE_IN_KB = 1024 +!else +!ifdef $(FD_SIZE_2MB) + DEFINE FD_SIZE_IN_KB = 2048 +!else + DEFINE FD_SIZE_IN_KB = 2048 +!endif +!endif + [BuildOptions] GCC:*_UNIXGCC_*_CC_FLAGS = -DMDEPKG_NDEBUG GCC:RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG INTEL:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG MSFT:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG GCC:*_*_*_CC_FLAGS = -mno-mmx -mno-sse !ifdef $(SOURCE_DEBUG_ENABLE) MSFT:*_*_X64_GENFW_FLAGS = --keepexceptiontable GCC:*_*_X64_GENFW_FLAGS = --keepexceptiontable INTEL:*_*_X64_GENFW_FLAGS = --keepexceptiontable !endif # diff --git a/OvmfPkg/OvmfPkg.fdf.inc b/OvmfPkg/OvmfPkg.fdf.inc index 9cc0578d6430..f12d61c2b682 100644 --- a/OvmfPkg/OvmfPkg.fdf.inc +++ b/OvmfPkg/OvmfPkg.fdf.inc @@ -5,60 +5,52 @@ # Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.<BR> # # This program and the accompanying materials are licensed and made available # under the terms and conditions of the BSD License which accompanies this # distribution. The full text of the license may be found at # http://opensource.org/licenses/bsd-license.php # # THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, # WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR # IMPLIED. # ## -# -# Default flash size is 2MB. -# -# Defining FD_SIZE_1MB on the build command line can override this. -# - DEFINE BLOCK_SIZE = 0x1000 DEFINE VARS_SIZE = 0x20000 DEFINE VARS_BLOCKS = 0x20 -!ifdef $(FD_SIZE_1MB) - +!if $(FD_SIZE_IN_KB) == 1024 DEFINE FW_BASE_ADDRESS = 0xFFF00000 DEFINE FW_SIZE = 0x00100000 DEFINE FW_BLOCKS = 0x100 DEFINE CODE_BASE_ADDRESS = 0xFFF20000 DEFINE CODE_SIZE = 0x000E0000 DEFINE CODE_BLOCKS = 0xE0 DEFINE FVMAIN_SIZE = 0x000CC000 DEFINE SECFV_OFFSET = 0x000EC000 DEFINE SECFV_SIZE = 0x14000 +!endif -!else - +!if $(FD_SIZE_IN_KB) == 2048 DEFINE FW_BASE_ADDRESS = 0xFFE00000 DEFINE FW_SIZE = 0x00200000 DEFINE FW_BLOCKS = 0x200 DEFINE CODE_BASE_ADDRESS = 0xFFE20000 DEFINE CODE_SIZE = 0x001E0000 DEFINE CODE_BLOCKS = 0x1E0 DEFINE FVMAIN_SIZE = 0x001AC000 DEFINE SECFV_OFFSET = 0x001CC000 DEFINE SECFV_SIZE = 0x34000 - !endif SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFdBaseAddress = $(FW_BASE_ADDRESS) SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareFdSize = $(FW_SIZE) SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareBlockSize = $(BLOCK_SIZE) SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageVariableBase = $(FW_BASE_ADDRESS) SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize = 0xE000 SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageEventLogBase = gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageVariableBase + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageEventLogSize = $(BLOCK_SIZE) SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageFtwWorkingBase = gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageEventLogBase + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageEventLogSize -- 2.9.3 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 2/5] OvmfPkg/OvmfPkg.fdf.inc: extract VARS_LIVE_SIZE and VARS_SPARE_SIZE macros 2017-05-03 21:39 [PATCH v2 0/5] OvmfPkg: add FD_SIZE_4MB for Windows HCK SB tests, and for future proofing Laszlo Ersek 2017-05-03 21:39 ` [PATCH v2 1/5] OvmfPkg: introduce the FD_SIZE_IN_KB macro / build flag Laszlo Ersek @ 2017-05-03 21:39 ` Laszlo Ersek 2017-05-03 21:39 ` [PATCH v2 3/5] OvmfPkg: introduce 4MB flash image (mainly) for Windows HCK Laszlo Ersek ` (2 subsequent siblings) 4 siblings, 0 replies; 17+ messages in thread From: Laszlo Ersek @ 2017-05-03 21:39 UTC (permalink / raw) To: edk2-devel-01; +Cc: Gary Ching-Pang Lin, Jordan Justen Cc: Gary Ching-Pang Lin <glin@suse.com> Cc: Jordan Justen <jordan.l.justen@intel.com> Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- Notes: v2: - no changes OvmfPkg/OvmfPkg.fdf.inc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/OvmfPkg/OvmfPkg.fdf.inc b/OvmfPkg/OvmfPkg.fdf.inc index f12d61c2b682..4e72e35678a2 100644 --- a/OvmfPkg/OvmfPkg.fdf.inc +++ b/OvmfPkg/OvmfPkg.fdf.inc @@ -8,26 +8,28 @@ # under the terms and conditions of the BSD License which accompanies this # distribution. The full text of the license may be found at # http://opensource.org/licenses/bsd-license.php # # THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, # WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR # IMPLIED. # ## DEFINE BLOCK_SIZE = 0x1000 DEFINE VARS_SIZE = 0x20000 DEFINE VARS_BLOCKS = 0x20 +DEFINE VARS_LIVE_SIZE = 0xE000 +DEFINE VARS_SPARE_SIZE = 0x10000 !if $(FD_SIZE_IN_KB) == 1024 DEFINE FW_BASE_ADDRESS = 0xFFF00000 DEFINE FW_SIZE = 0x00100000 DEFINE FW_BLOCKS = 0x100 DEFINE CODE_BASE_ADDRESS = 0xFFF20000 DEFINE CODE_SIZE = 0x000E0000 DEFINE CODE_BLOCKS = 0xE0 DEFINE FVMAIN_SIZE = 0x000CC000 DEFINE SECFV_OFFSET = 0x000EC000 DEFINE SECFV_SIZE = 0x14000 !endif @@ -38,25 +40,25 @@ DEFINE CODE_BASE_ADDRESS = 0xFFE20000 DEFINE CODE_SIZE = 0x001E0000 DEFINE CODE_BLOCKS = 0x1E0 DEFINE FVMAIN_SIZE = 0x001AC000 DEFINE SECFV_OFFSET = 0x001CC000 DEFINE SECFV_SIZE = 0x34000 !endif SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFdBaseAddress = $(FW_BASE_ADDRESS) SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareFdSize = $(FW_SIZE) SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareBlockSize = $(BLOCK_SIZE) SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageVariableBase = $(FW_BASE_ADDRESS) -SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize = 0xE000 +SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize = $(VARS_LIVE_SIZE) SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageEventLogBase = gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageVariableBase + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageEventLogSize = $(BLOCK_SIZE) SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageFtwWorkingBase = gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageEventLogBase + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageEventLogSize SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize = $(BLOCK_SIZE) SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageFtwSpareBase = gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageFtwWorkingBase + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize -SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize = 0x10000 +SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize = $(VARS_SPARE_SIZE) DEFINE MEMFD_BASE_ADDRESS = 0x800000 -- 2.9.3 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 3/5] OvmfPkg: introduce 4MB flash image (mainly) for Windows HCK 2017-05-03 21:39 [PATCH v2 0/5] OvmfPkg: add FD_SIZE_4MB for Windows HCK SB tests, and for future proofing Laszlo Ersek 2017-05-03 21:39 ` [PATCH v2 1/5] OvmfPkg: introduce the FD_SIZE_IN_KB macro / build flag Laszlo Ersek 2017-05-03 21:39 ` [PATCH v2 2/5] OvmfPkg/OvmfPkg.fdf.inc: extract VARS_LIVE_SIZE and VARS_SPARE_SIZE macros Laszlo Ersek @ 2017-05-03 21:39 ` Laszlo Ersek 2017-05-05 4:07 ` Laszlo Ersek 2017-05-03 21:39 ` [PATCH v2 4/5] OvmfPkg: raise max variable size (auth & non-auth) to 33KB for FD_SIZE_4MB Laszlo Ersek 2017-05-03 21:39 ` [PATCH v2 5/5] OvmfPkg: make the 4MB flash size the default Laszlo Ersek 4 siblings, 1 reply; 17+ messages in thread From: Laszlo Ersek @ 2017-05-03 21:39 UTC (permalink / raw) To: edk2-devel-01; +Cc: Gary Ching-Pang Lin, Jordan Justen The "Confirm64KilobytesOfUnauthenticatedVariableStorage" test case of the Secure Boot Logo Test ("Microsoft.UefiSecureBootLogo.Tests") suite in the Microsoft Hardware Certification Kit expects to be able to populate the variable store up to roughly 64 KB, with a series of 1 KB sized, unauthenticated variables. OVMF's current live varstore area is too small for this: 56 KB. Introduce the FD_SIZE_4MB build macro (equivalently, FD_SIZE_IN_KB=4096), which - enlarges the full flash image to 4MB -- QEMU supports up to 8MB, see FLASH_MAP_BASE_MIN in "hw/i386/pc_sysfw.c" --, - inside that, grows the varstore area / pflash chip to 528 KB, and within it, the live area from 56 KB to 256 KB. Importantly, a firmware binary built with -D FD_SIZE_4MB will *not* be compatible with a variable store that originates from a variable store template built *without* -D FD_SIZE_4MB. This is the reason for the large increase, as every such change breaks compatibility between a new firmware binary and old varstore files. Enlarging the varstore does not impact the performance of normal operations, as we keep the varstore block size 4KB. The performance of reclaim is affected, but that is expected (since reclaim has to rework the full live area). And, reclaim occurs proportionally less frequently. While at it, the FVMAIN_COMPACT volume (with the compressed FFS file in it) is also enlarged significantly, so that we have plenty of room for future DXEFV (and perhaps PEIFV) increments -- DXEFV has been growing steadily, and that increase shows through compression too. Right now the PEIFV and DXEFV volumes need no resizing. Here's a summary: Description Compression type Size [KB] ------------------------- ----------------- ---------------------- Non-volatile data storage open-coded binary 128 -> 528 ( +400) data Variable store 56 -> 256 ( +200) Event log 4 -> 4 ( +0) Working block 4 -> 4 ( +0) Spare area 64 -> 264 ( +200) FVMAIN_COMPACT uncompressed 1712 -> 3360 (+1648) FV FFS file LZMA compressed PEIFV uncompressed 896 -> 896 ( +0) individual PEI uncompressed modules DXEFV uncompressed 10240 -> 10240 ( +0) individual DXE uncompressed modules SECFV uncompressed 208 -> 208 ( +0) SEC driver reset vector code For now, the 2MB flash image remains the default. Cc: Gary Ching-Pang Lin <glin@suse.com> Cc: Jordan Justen <jordan.l.justen@intel.com> Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- Notes: v2: - use $(FD_SIZE_IN_KB) in conditional statements - Raise VARS_LIVE_SIZE by 8KB to 256KB, VARS_SPARE_SIZE by 8KB to 264KB, thereby raising the containing VARS_SIZE by 16KB to 528KB. To compensate, raise CODE_BASE_ADDRESS by 16KB, and shrink both FVMAIN_SIZE and the containing CODE_SIZE by 16KB. No change to FW_BASE_ADDRESS, FW_SIZE, SECFV_OFFSET, SECFV_SIZE. [Jordan] OvmfPkg/OvmfPkgIa32.dsc | 4 ++ OvmfPkg/OvmfPkgIa32X64.dsc | 4 ++ OvmfPkg/OvmfPkgX64.dsc | 4 ++ OvmfPkg/OvmfPkg.fdf.inc | 28 ++++++++++ OvmfPkg/VarStore.fdf.inc | 54 +++++++++++++++++++- 5 files changed, 92 insertions(+), 2 deletions(-) diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc index 5a21840a55c9..26b807dde9fa 100644 --- a/OvmfPkg/OvmfPkgIa32.dsc +++ b/OvmfPkg/OvmfPkgIa32.dsc @@ -41,29 +41,33 @@ [Defines] DEFINE TLS_ENABLE = FALSE # # Flash size selection. Setting FD_SIZE_IN_KB on the command line directly to # one of the supported values, in place of any of the convenience macros, is # permitted. # !ifdef $(FD_SIZE_1MB) DEFINE FD_SIZE_IN_KB = 1024 !else !ifdef $(FD_SIZE_2MB) DEFINE FD_SIZE_IN_KB = 2048 !else +!ifdef $(FD_SIZE_4MB) + DEFINE FD_SIZE_IN_KB = 4096 +!else DEFINE FD_SIZE_IN_KB = 2048 !endif !endif +!endif [BuildOptions] GCC:*_UNIXGCC_*_CC_FLAGS = -DMDEPKG_NDEBUG GCC:RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG INTEL:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG MSFT:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG GCC:*_*_*_CC_FLAGS = -mno-mmx -mno-sse # # Disable deprecated APIs. # MSFT:*_*_*_CC_FLAGS = /D DISABLE_NEW_DEPRECATED_INTERFACES INTEL:*_*_*_CC_FLAGS = /D DISABLE_NEW_DEPRECATED_INTERFACES diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc index 11866b7207c7..41f06a6b6a66 100644 --- a/OvmfPkg/OvmfPkgIa32X64.dsc +++ b/OvmfPkg/OvmfPkgIa32X64.dsc @@ -41,29 +41,33 @@ [Defines] DEFINE TLS_ENABLE = FALSE # # Flash size selection. Setting FD_SIZE_IN_KB on the command line directly to # one of the supported values, in place of any of the convenience macros, is # permitted. # !ifdef $(FD_SIZE_1MB) DEFINE FD_SIZE_IN_KB = 1024 !else !ifdef $(FD_SIZE_2MB) DEFINE FD_SIZE_IN_KB = 2048 !else +!ifdef $(FD_SIZE_4MB) + DEFINE FD_SIZE_IN_KB = 4096 +!else DEFINE FD_SIZE_IN_KB = 2048 !endif !endif +!endif [BuildOptions] GCC:*_UNIXGCC_*_CC_FLAGS = -DMDEPKG_NDEBUG GCC:RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG INTEL:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG MSFT:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG GCC:*_*_*_CC_FLAGS = -mno-mmx -mno-sse !ifdef $(SOURCE_DEBUG_ENABLE) MSFT:*_*_X64_GENFW_FLAGS = --keepexceptiontable GCC:*_*_X64_GENFW_FLAGS = --keepexceptiontable INTEL:*_*_X64_GENFW_FLAGS = --keepexceptiontable !endif diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc index 2fab544600f5..053c84b685c5 100644 --- a/OvmfPkg/OvmfPkgX64.dsc +++ b/OvmfPkg/OvmfPkgX64.dsc @@ -41,29 +41,33 @@ [Defines] DEFINE TLS_ENABLE = FALSE # # Flash size selection. Setting FD_SIZE_IN_KB on the command line directly to # one of the supported values, in place of any of the convenience macros, is # permitted. # !ifdef $(FD_SIZE_1MB) DEFINE FD_SIZE_IN_KB = 1024 !else !ifdef $(FD_SIZE_2MB) DEFINE FD_SIZE_IN_KB = 2048 !else +!ifdef $(FD_SIZE_4MB) + DEFINE FD_SIZE_IN_KB = 4096 +!else DEFINE FD_SIZE_IN_KB = 2048 !endif !endif +!endif [BuildOptions] GCC:*_UNIXGCC_*_CC_FLAGS = -DMDEPKG_NDEBUG GCC:RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG INTEL:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG MSFT:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG GCC:*_*_*_CC_FLAGS = -mno-mmx -mno-sse !ifdef $(SOURCE_DEBUG_ENABLE) MSFT:*_*_X64_GENFW_FLAGS = --keepexceptiontable GCC:*_*_X64_GENFW_FLAGS = --keepexceptiontable INTEL:*_*_X64_GENFW_FLAGS = --keepexceptiontable !endif diff --git a/OvmfPkg/OvmfPkg.fdf.inc b/OvmfPkg/OvmfPkg.fdf.inc index 4e72e35678a2..b3e0c472a1a8 100644 --- a/OvmfPkg/OvmfPkg.fdf.inc +++ b/OvmfPkg/OvmfPkg.fdf.inc @@ -6,55 +6,83 @@ # # This program and the accompanying materials are licensed and made available # under the terms and conditions of the BSD License which accompanies this # distribution. The full text of the license may be found at # http://opensource.org/licenses/bsd-license.php # # THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, # WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR # IMPLIED. # ## DEFINE BLOCK_SIZE = 0x1000 + +# +# 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 +# firmware binaries. +# +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) DEFINE VARS_SIZE = 0x20000 DEFINE VARS_BLOCKS = 0x20 DEFINE VARS_LIVE_SIZE = 0xE000 DEFINE VARS_SPARE_SIZE = 0x10000 +!endif !if $(FD_SIZE_IN_KB) == 1024 DEFINE FW_BASE_ADDRESS = 0xFFF00000 DEFINE FW_SIZE = 0x00100000 DEFINE FW_BLOCKS = 0x100 DEFINE CODE_BASE_ADDRESS = 0xFFF20000 DEFINE CODE_SIZE = 0x000E0000 DEFINE CODE_BLOCKS = 0xE0 DEFINE FVMAIN_SIZE = 0x000CC000 DEFINE SECFV_OFFSET = 0x000EC000 DEFINE SECFV_SIZE = 0x14000 !endif !if $(FD_SIZE_IN_KB) == 2048 DEFINE FW_BASE_ADDRESS = 0xFFE00000 DEFINE FW_SIZE = 0x00200000 DEFINE FW_BLOCKS = 0x200 DEFINE CODE_BASE_ADDRESS = 0xFFE20000 DEFINE CODE_SIZE = 0x001E0000 DEFINE CODE_BLOCKS = 0x1E0 DEFINE FVMAIN_SIZE = 0x001AC000 DEFINE SECFV_OFFSET = 0x001CC000 DEFINE SECFV_SIZE = 0x34000 !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 +DEFINE CODE_BASE_ADDRESS = 0xFFC84000 +DEFINE CODE_SIZE = 0x0037C000 +DEFINE CODE_BLOCKS = 0x37C +DEFINE FVMAIN_SIZE = 0x00348000 +DEFINE SECFV_OFFSET = 0x003CC000 +DEFINE SECFV_SIZE = 0x34000 +!endif + SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFdBaseAddress = $(FW_BASE_ADDRESS) SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareFdSize = $(FW_SIZE) SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareBlockSize = $(BLOCK_SIZE) SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageVariableBase = $(FW_BASE_ADDRESS) SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize = $(VARS_LIVE_SIZE) SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageEventLogBase = gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageVariableBase + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageEventLogSize = $(BLOCK_SIZE) SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageFtwWorkingBase = gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageEventLogBase + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageEventLogSize SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize = $(BLOCK_SIZE) diff --git a/OvmfPkg/VarStore.fdf.inc b/OvmfPkg/VarStore.fdf.inc index ce901c0109b1..742fed105334 100644 --- a/OvmfPkg/VarStore.fdf.inc +++ b/OvmfPkg/VarStore.fdf.inc @@ -5,68 +5,118 @@ # Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.<BR> # # This program and the accompanying materials are licensed and made available # under the terms and conditions of the BSD License which accompanies this # distribution. The full text of the license may be found at # http://opensource.org/licenses/bsd-license.php # # THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, # WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR # IMPLIED. # ## +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) 0x00000000|0x0000e000 +!endif +!if $(FD_SIZE_IN_KB) == 4096 +0x00000000|0x00040000 +!endif #NV_VARIABLE_STORE DATA = { ## This is the EFI_FIRMWARE_VOLUME_HEADER # ZeroVector [] 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, # FileSystemGuid: gEfiSystemNvDataFvGuid = # { 0xFFF12B8D, 0x7696, 0x4C8B, # { 0xA9, 0x85, 0x27, 0x47, 0x07, 0x5B, 0x4F, 0x50 }} 0x8D, 0x2B, 0xF1, 0xFF, 0x96, 0x76, 0x8B, 0x4C, 0xA9, 0x85, 0x27, 0x47, 0x07, 0x5B, 0x4F, 0x50, +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) # FvLength: 0x20000 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, +!endif +!if $(FD_SIZE_IN_KB) == 4096 + # FvLength: 0x84000 + 0x00, 0x40, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, +!endif # Signature "_FVH" # Attributes 0x5f, 0x46, 0x56, 0x48, 0xff, 0xfe, 0x04, 0x00, - # HeaderLength # CheckSum # ExtHeaderOffset #Reserved #Revision - 0x48, 0x00, 0x19, 0xF9, 0x00, 0x00, 0x00, 0x02, + # HeaderLength + 0x48, 0x00, +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) + # CheckSum + 0x19, 0xF9, +!endif +!if $(FD_SIZE_IN_KB) == 4096 + # CheckSum + 0xAF, 0xB8, +!endif + # ExtHeaderOffset #Reserved #Revision + 0x00, 0x00, 0x00, 0x02, +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) # Blockmap[0]: 0x20 Blocks * 0x1000 Bytes / Block 0x20, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00, +!endif +!if $(FD_SIZE_IN_KB) == 4096 + # Blockmap[0]: 0x84 Blocks * 0x1000 Bytes / Block + 0x84, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00, +!endif # Blockmap[1]: End 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, ## This is the VARIABLE_STORE_HEADER # It is compatible with SECURE_BOOT_ENABLE == FALSE as well. # Signature: gEfiAuthenticatedVariableGuid = # { 0xaaf32c78, 0x947b, 0x439a, # { 0xa1, 0x80, 0x2e, 0x14, 0x4e, 0xc3, 0x77, 0x92 }} 0x78, 0x2c, 0xf3, 0xaa, 0x7b, 0x94, 0x9a, 0x43, 0xa1, 0x80, 0x2e, 0x14, 0x4e, 0xc3, 0x77, 0x92, +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) # Size: 0xe000 (gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize) - # 0x48 (size of EFI_FIRMWARE_VOLUME_HEADER) = 0xdfb8 # This can speed up the Variable Dispatch a bit. 0xB8, 0xDF, 0x00, 0x00, +!endif +!if $(FD_SIZE_IN_KB) == 4096 + # Size: 0x40000 (gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize) - + # 0x48 (size of EFI_FIRMWARE_VOLUME_HEADER) = 0x3ffb8 + # This can speed up the Variable Dispatch a bit. + 0xB8, 0xFF, 0x03, 0x00, +!endif # FORMATTED: 0x5A #HEALTHY: 0xFE #Reserved: UINT16 #Reserved1: UINT32 0x5A, 0xFE, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 } +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) 0x0000e000|0x00001000 +!endif +!if $(FD_SIZE_IN_KB) == 4096 +0x00040000|0x00001000 +!endif #NV_EVENT_LOG +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) 0x0000f000|0x00001000 +!endif +!if $(FD_SIZE_IN_KB) == 4096 +0x00041000|0x00001000 +!endif #NV_FTW_WORKING DATA = { # EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER->Signature = gEdkiiWorkingBlockSignatureGuid = # { 0x9e58292b, 0x7c68, 0x497d, { 0xa0, 0xce, 0x65, 0x0, 0xfd, 0x9f, 0x1b, 0x95 }} 0x2b, 0x29, 0x58, 0x9e, 0x68, 0x7c, 0x7d, 0x49, 0xa0, 0xce, 0x65, 0x0, 0xfd, 0x9f, 0x1b, 0x95, # Crc:UINT32 #WorkingBlockValid:1, WorkingBlockInvalid:1, Reserved 0x2c, 0xaf, 0x2c, 0x64, 0xFE, 0xFF, 0xFF, 0xFF, # WriteQueueSize: UINT64 0xE0, 0x0F, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 } +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) 0x00010000|0x00010000 +!endif +!if $(FD_SIZE_IN_KB) == 4096 +0x00042000|0x00042000 +!endif #NV_FTW_SPARE -- 2.9.3 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 3/5] OvmfPkg: introduce 4MB flash image (mainly) for Windows HCK 2017-05-03 21:39 ` [PATCH v2 3/5] OvmfPkg: introduce 4MB flash image (mainly) for Windows HCK Laszlo Ersek @ 2017-05-05 4:07 ` Laszlo Ersek 2017-05-05 8:57 ` Jordan Justen 0 siblings, 1 reply; 17+ messages in thread From: Laszlo Ersek @ 2017-05-05 4:07 UTC (permalink / raw) To: Jordan Justen; +Cc: edk2-devel-01, Gary Ching-Pang Lin On 05/03/17 23:39, Laszlo Ersek wrote: > The "Confirm64KilobytesOfUnauthenticatedVariableStorage" test case of the > Secure Boot Logo Test ("Microsoft.UefiSecureBootLogo.Tests") suite in the > Microsoft Hardware Certification Kit expects to be able to populate the > variable store up to roughly 64 KB, with a series of 1 KB sized, > unauthenticated variables. OVMF's current live varstore area is too small > for this: 56 KB. > > Introduce the FD_SIZE_4MB build macro (equivalently, FD_SIZE_IN_KB=4096), > which > > - enlarges the full flash image to 4MB -- QEMU supports up to 8MB, see > FLASH_MAP_BASE_MIN in "hw/i386/pc_sysfw.c" --, > > - inside that, grows the varstore area / pflash chip to 528 KB, and within > it, the live area from 56 KB to 256 KB. > > Importantly, a firmware binary built with -D FD_SIZE_4MB will *not* be > compatible with a variable store that originates from a variable store > template built *without* -D FD_SIZE_4MB. This is the reason for the large > increase, as every such change breaks compatibility between a new firmware > binary and old varstore files. > > Enlarging the varstore does not impact the performance of normal > operations, as we keep the varstore block size 4KB. The performance of > reclaim is affected, but that is expected (since reclaim has to rework the > full live area). And, reclaim occurs proportionally less frequently. > > While at it, the FVMAIN_COMPACT volume (with the compressed FFS file in > it) is also enlarged significantly, so that we have plenty of room for > future DXEFV (and perhaps PEIFV) increments -- DXEFV has been growing > steadily, and that increase shows through compression too. Right now the > PEIFV and DXEFV volumes need no resizing. > > Here's a summary: > > Description Compression type Size [KB] > ------------------------- ----------------- ---------------------- > Non-volatile data storage open-coded binary 128 -> 528 ( +400) > data > Variable store 56 -> 256 ( +200) > Event log 4 -> 4 ( +0) > Working block 4 -> 4 ( +0) > Spare area 64 -> 264 ( +200) > > FVMAIN_COMPACT uncompressed 1712 -> 3360 (+1648) > FV FFS file LZMA compressed > PEIFV uncompressed 896 -> 896 ( +0) > individual PEI uncompressed > modules > DXEFV uncompressed 10240 -> 10240 ( +0) > individual DXE uncompressed > modules > > SECFV uncompressed 208 -> 208 ( +0) > SEC driver > reset vector code > > For now, the 2MB flash image remains the default. > > Cc: Gary Ching-Pang Lin <glin@suse.com> > Cc: Jordan Justen <jordan.l.justen@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Laszlo Ersek <lersek@redhat.com> > --- > > Notes: > v2: > - use $(FD_SIZE_IN_KB) in conditional statements > > - Raise VARS_LIVE_SIZE by 8KB to 256KB, VARS_SPARE_SIZE by 8KB to 264KB, Unfortunately, we have a terrible regression here. :( It has to do with the emulated variable store. The OvmfPkg/EmuVariableFvbRuntimeDxe driver exposes a two-block flash device (FVB protocol) that uses the NV spare area size as internal block size. And thereby covers the full NV area with just two blocks in total. In addition, that driver keeps cutting corners everywhere, exploiting very heavily that there are just two blocks. This assumption is open-coded all over it. This is also why we have the exact allocation logic in ReserveEmuVariableNvStore() that we have, in PlatformPei. The AllocateAlignedRuntimePages() call in ReserveEmuVariableNvStore() depends on the NV spare area size being a power of two, because only such values are accepted as Alignment. And the FTW driver that consumes the FVB from EmuVariableFvbRuntimeDxe also expects the FVB block size to be a power of two, which again translates to the NV spare area size having to be a power of two. (Because that's what EmuVariableFvbRuntimeDxe exposes as FVB block size, for "all two" of its blocks.) Changing the spare area size to 264KB breaks this extremely. Booting the 4MB image with -bios is completely broken. I've been working for a few hours on it now, but it's not just about refining the granularity of the internal block size of the EmuVariableFvbRuntimeDxe driver, to 4KB. As I said, the driver open-codes the two-block assumption everywhere. For example, in the FvbProtocolEraseBlocks() function, a complete loop is missing. :( Adapting EmuVariableFvbRuntimeDxe to a non-power-of-two NV spare area size is hopeless. (Definitely hopeless for the time frame & resources I'm looking at.) Worse, even -pflash is broken in the 4MB build, actually. The non-power-of-two NV spare area size, when used as Alignment for AllocateAlignedRuntimePages() in ReserveEmuVariableNvStore(), triggers an assertion. And this path is taken for the -pflash boot as well. So why didn't I notice this earlier? This is why: I tested v1 (which had a 248KB NV varstore size and a 256KB NV spare area size) against the pure upstream master branch, as well as against some of my personal development (long term in progresss) patches. Because the spare area size was a power of two, v1 worked fine on the master branch (with pflash). Unfortunately, when testing v2, I failed to test on the master branch *without* also heaping my personal dev patches on top. And some of those patches skip the ReserveEmuVariableNvStore() call, hence the non-power-of-two spare area size didn't cause any problems. Obviously, I didn't test the emu variable case, because I haven't used that in years. I apologize for not noticing how hard the 264KB spare would conflict with the EMU variable driver -- my only excuse is that I haven't used the latter in years, and I've been carrying private patches that disable ReserveEmuVariableNvStore() so that I don't even lose memory to a (misleading) feature that I otherwise never use. This is why the error in v2 on the common code path remained hidden from me. So here's the only thing I can propose now. Flip the NV layout back to the one seen in v1 -- 248KB varstore, 256KB spare, 3376KB FVMAIN_COMPACT. That will keep all the assumptions about the spare area being a power of two satisfied, in PlatformPei, and in the FTW driver through EmuVariableFvbRuntimeDxe's FVB protocol. This can be done with one additional patch. Another patch is necessary for synchronizing PcdVariableStoreSize with PcdFlashNvStorageVariableSize. The former gives the varstore size for *volatile* variables, and it normally doesn't have to match the size of the non-volatile varstore (as evidenced by the -pflash case, with v1). But, EmuVariableFvbRuntimeDxe abuses PcdVariableStoreSize for setting the size field in the NV varstore header that it produces, and if both PCDs aren't in sync, then in the -bios case, the variable driver blows up with an assertion failure. Jordan, does this plan look acceptable to you? The 2MB build continues to work with -bios; at least I don't seem to have broken that. Thanks Laszlo > thereby raising the containing VARS_SIZE by 16KB to 528KB. To > compensate, raise CODE_BASE_ADDRESS by 16KB, and shrink both > FVMAIN_SIZE and the containing CODE_SIZE by 16KB. No change to > FW_BASE_ADDRESS, FW_SIZE, SECFV_OFFSET, SECFV_SIZE. [Jordan] > > OvmfPkg/OvmfPkgIa32.dsc | 4 ++ > OvmfPkg/OvmfPkgIa32X64.dsc | 4 ++ > OvmfPkg/OvmfPkgX64.dsc | 4 ++ > OvmfPkg/OvmfPkg.fdf.inc | 28 ++++++++++ > OvmfPkg/VarStore.fdf.inc | 54 +++++++++++++++++++- > 5 files changed, 92 insertions(+), 2 deletions(-) > > diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc > index 5a21840a55c9..26b807dde9fa 100644 > --- a/OvmfPkg/OvmfPkgIa32.dsc > +++ b/OvmfPkg/OvmfPkgIa32.dsc > @@ -41,29 +41,33 @@ [Defines] > DEFINE TLS_ENABLE = FALSE > > # > # Flash size selection. Setting FD_SIZE_IN_KB on the command line directly to > # one of the supported values, in place of any of the convenience macros, is > # permitted. > # > !ifdef $(FD_SIZE_1MB) > DEFINE FD_SIZE_IN_KB = 1024 > !else > !ifdef $(FD_SIZE_2MB) > DEFINE FD_SIZE_IN_KB = 2048 > !else > +!ifdef $(FD_SIZE_4MB) > + DEFINE FD_SIZE_IN_KB = 4096 > +!else > DEFINE FD_SIZE_IN_KB = 2048 > !endif > !endif > +!endif > > [BuildOptions] > GCC:*_UNIXGCC_*_CC_FLAGS = -DMDEPKG_NDEBUG > GCC:RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG > INTEL:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG > MSFT:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG > GCC:*_*_*_CC_FLAGS = -mno-mmx -mno-sse > > # > # Disable deprecated APIs. > # > MSFT:*_*_*_CC_FLAGS = /D DISABLE_NEW_DEPRECATED_INTERFACES > INTEL:*_*_*_CC_FLAGS = /D DISABLE_NEW_DEPRECATED_INTERFACES > diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc > index 11866b7207c7..41f06a6b6a66 100644 > --- a/OvmfPkg/OvmfPkgIa32X64.dsc > +++ b/OvmfPkg/OvmfPkgIa32X64.dsc > @@ -41,29 +41,33 @@ [Defines] > DEFINE TLS_ENABLE = FALSE > > # > # Flash size selection. Setting FD_SIZE_IN_KB on the command line directly to > # one of the supported values, in place of any of the convenience macros, is > # permitted. > # > !ifdef $(FD_SIZE_1MB) > DEFINE FD_SIZE_IN_KB = 1024 > !else > !ifdef $(FD_SIZE_2MB) > DEFINE FD_SIZE_IN_KB = 2048 > !else > +!ifdef $(FD_SIZE_4MB) > + DEFINE FD_SIZE_IN_KB = 4096 > +!else > DEFINE FD_SIZE_IN_KB = 2048 > !endif > !endif > +!endif > > [BuildOptions] > GCC:*_UNIXGCC_*_CC_FLAGS = -DMDEPKG_NDEBUG > GCC:RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG > INTEL:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG > MSFT:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG > GCC:*_*_*_CC_FLAGS = -mno-mmx -mno-sse > !ifdef $(SOURCE_DEBUG_ENABLE) > MSFT:*_*_X64_GENFW_FLAGS = --keepexceptiontable > GCC:*_*_X64_GENFW_FLAGS = --keepexceptiontable > INTEL:*_*_X64_GENFW_FLAGS = --keepexceptiontable > !endif > > diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc > index 2fab544600f5..053c84b685c5 100644 > --- a/OvmfPkg/OvmfPkgX64.dsc > +++ b/OvmfPkg/OvmfPkgX64.dsc > @@ -41,29 +41,33 @@ [Defines] > DEFINE TLS_ENABLE = FALSE > > # > # Flash size selection. Setting FD_SIZE_IN_KB on the command line directly to > # one of the supported values, in place of any of the convenience macros, is > # permitted. > # > !ifdef $(FD_SIZE_1MB) > DEFINE FD_SIZE_IN_KB = 1024 > !else > !ifdef $(FD_SIZE_2MB) > DEFINE FD_SIZE_IN_KB = 2048 > !else > +!ifdef $(FD_SIZE_4MB) > + DEFINE FD_SIZE_IN_KB = 4096 > +!else > DEFINE FD_SIZE_IN_KB = 2048 > !endif > !endif > +!endif > > [BuildOptions] > GCC:*_UNIXGCC_*_CC_FLAGS = -DMDEPKG_NDEBUG > GCC:RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG > INTEL:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG > MSFT:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG > GCC:*_*_*_CC_FLAGS = -mno-mmx -mno-sse > !ifdef $(SOURCE_DEBUG_ENABLE) > MSFT:*_*_X64_GENFW_FLAGS = --keepexceptiontable > GCC:*_*_X64_GENFW_FLAGS = --keepexceptiontable > INTEL:*_*_X64_GENFW_FLAGS = --keepexceptiontable > !endif > > diff --git a/OvmfPkg/OvmfPkg.fdf.inc b/OvmfPkg/OvmfPkg.fdf.inc > index 4e72e35678a2..b3e0c472a1a8 100644 > --- a/OvmfPkg/OvmfPkg.fdf.inc > +++ b/OvmfPkg/OvmfPkg.fdf.inc > @@ -6,55 +6,83 @@ > # > # This program and the accompanying materials are licensed and made available > # under the terms and conditions of the BSD License which accompanies this > # distribution. The full text of the license may be found at > # http://opensource.org/licenses/bsd-license.php > # > # THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > # WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR > # IMPLIED. > # > ## > > DEFINE BLOCK_SIZE = 0x1000 > + > +# > +# 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 > +# firmware binaries. > +# > +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) > DEFINE VARS_SIZE = 0x20000 > DEFINE VARS_BLOCKS = 0x20 > DEFINE VARS_LIVE_SIZE = 0xE000 > DEFINE VARS_SPARE_SIZE = 0x10000 > +!endif > > !if $(FD_SIZE_IN_KB) == 1024 > DEFINE FW_BASE_ADDRESS = 0xFFF00000 > DEFINE FW_SIZE = 0x00100000 > DEFINE FW_BLOCKS = 0x100 > DEFINE CODE_BASE_ADDRESS = 0xFFF20000 > DEFINE CODE_SIZE = 0x000E0000 > DEFINE CODE_BLOCKS = 0xE0 > DEFINE FVMAIN_SIZE = 0x000CC000 > DEFINE SECFV_OFFSET = 0x000EC000 > DEFINE SECFV_SIZE = 0x14000 > !endif > > !if $(FD_SIZE_IN_KB) == 2048 > DEFINE FW_BASE_ADDRESS = 0xFFE00000 > DEFINE FW_SIZE = 0x00200000 > DEFINE FW_BLOCKS = 0x200 > DEFINE CODE_BASE_ADDRESS = 0xFFE20000 > DEFINE CODE_SIZE = 0x001E0000 > DEFINE CODE_BLOCKS = 0x1E0 > DEFINE FVMAIN_SIZE = 0x001AC000 > DEFINE SECFV_OFFSET = 0x001CC000 > DEFINE SECFV_SIZE = 0x34000 > !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 > +DEFINE CODE_BASE_ADDRESS = 0xFFC84000 > +DEFINE CODE_SIZE = 0x0037C000 > +DEFINE CODE_BLOCKS = 0x37C > +DEFINE FVMAIN_SIZE = 0x00348000 > +DEFINE SECFV_OFFSET = 0x003CC000 > +DEFINE SECFV_SIZE = 0x34000 > +!endif > + > SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFdBaseAddress = $(FW_BASE_ADDRESS) > SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareFdSize = $(FW_SIZE) > SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareBlockSize = $(BLOCK_SIZE) > > SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageVariableBase = $(FW_BASE_ADDRESS) > SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize = $(VARS_LIVE_SIZE) > > SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageEventLogBase = gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageVariableBase + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize > SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageEventLogSize = $(BLOCK_SIZE) > > SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageFtwWorkingBase = gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageEventLogBase + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageEventLogSize > SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize = $(BLOCK_SIZE) > > diff --git a/OvmfPkg/VarStore.fdf.inc b/OvmfPkg/VarStore.fdf.inc > index ce901c0109b1..742fed105334 100644 > --- a/OvmfPkg/VarStore.fdf.inc > +++ b/OvmfPkg/VarStore.fdf.inc > @@ -5,68 +5,118 @@ > # Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.<BR> > # > # This program and the accompanying materials are licensed and made available > # under the terms and conditions of the BSD License which accompanies this > # distribution. The full text of the license may be found at > # http://opensource.org/licenses/bsd-license.php > # > # THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > # WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR > # IMPLIED. > # > ## > > +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) > 0x00000000|0x0000e000 > +!endif > +!if $(FD_SIZE_IN_KB) == 4096 > +0x00000000|0x00040000 > +!endif > #NV_VARIABLE_STORE > DATA = { > ## This is the EFI_FIRMWARE_VOLUME_HEADER > # ZeroVector [] > 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > # FileSystemGuid: gEfiSystemNvDataFvGuid = > # { 0xFFF12B8D, 0x7696, 0x4C8B, > # { 0xA9, 0x85, 0x27, 0x47, 0x07, 0x5B, 0x4F, 0x50 }} > 0x8D, 0x2B, 0xF1, 0xFF, 0x96, 0x76, 0x8B, 0x4C, > 0xA9, 0x85, 0x27, 0x47, 0x07, 0x5B, 0x4F, 0x50, > +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) > # FvLength: 0x20000 > 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, > +!endif > +!if $(FD_SIZE_IN_KB) == 4096 > + # FvLength: 0x84000 > + 0x00, 0x40, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, > +!endif > # Signature "_FVH" # Attributes > 0x5f, 0x46, 0x56, 0x48, 0xff, 0xfe, 0x04, 0x00, > - # HeaderLength # CheckSum # ExtHeaderOffset #Reserved #Revision > - 0x48, 0x00, 0x19, 0xF9, 0x00, 0x00, 0x00, 0x02, > + # HeaderLength > + 0x48, 0x00, > +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) > + # CheckSum > + 0x19, 0xF9, > +!endif > +!if $(FD_SIZE_IN_KB) == 4096 > + # CheckSum > + 0xAF, 0xB8, > +!endif > + # ExtHeaderOffset #Reserved #Revision > + 0x00, 0x00, 0x00, 0x02, > +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) > # Blockmap[0]: 0x20 Blocks * 0x1000 Bytes / Block > 0x20, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00, > +!endif > +!if $(FD_SIZE_IN_KB) == 4096 > + # Blockmap[0]: 0x84 Blocks * 0x1000 Bytes / Block > + 0x84, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00, > +!endif > # Blockmap[1]: End > 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > ## This is the VARIABLE_STORE_HEADER > # It is compatible with SECURE_BOOT_ENABLE == FALSE as well. > # Signature: gEfiAuthenticatedVariableGuid = > # { 0xaaf32c78, 0x947b, 0x439a, > # { 0xa1, 0x80, 0x2e, 0x14, 0x4e, 0xc3, 0x77, 0x92 }} > 0x78, 0x2c, 0xf3, 0xaa, 0x7b, 0x94, 0x9a, 0x43, > 0xa1, 0x80, 0x2e, 0x14, 0x4e, 0xc3, 0x77, 0x92, > +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) > # Size: 0xe000 (gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize) - > # 0x48 (size of EFI_FIRMWARE_VOLUME_HEADER) = 0xdfb8 > # This can speed up the Variable Dispatch a bit. > 0xB8, 0xDF, 0x00, 0x00, > +!endif > +!if $(FD_SIZE_IN_KB) == 4096 > + # Size: 0x40000 (gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize) - > + # 0x48 (size of EFI_FIRMWARE_VOLUME_HEADER) = 0x3ffb8 > + # This can speed up the Variable Dispatch a bit. > + 0xB8, 0xFF, 0x03, 0x00, > +!endif > # FORMATTED: 0x5A #HEALTHY: 0xFE #Reserved: UINT16 #Reserved1: UINT32 > 0x5A, 0xFE, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 > } > > +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) > 0x0000e000|0x00001000 > +!endif > +!if $(FD_SIZE_IN_KB) == 4096 > +0x00040000|0x00001000 > +!endif > #NV_EVENT_LOG > > +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) > 0x0000f000|0x00001000 > +!endif > +!if $(FD_SIZE_IN_KB) == 4096 > +0x00041000|0x00001000 > +!endif > #NV_FTW_WORKING > DATA = { > # EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER->Signature = gEdkiiWorkingBlockSignatureGuid = > # { 0x9e58292b, 0x7c68, 0x497d, { 0xa0, 0xce, 0x65, 0x0, 0xfd, 0x9f, 0x1b, 0x95 }} > 0x2b, 0x29, 0x58, 0x9e, 0x68, 0x7c, 0x7d, 0x49, > 0xa0, 0xce, 0x65, 0x0, 0xfd, 0x9f, 0x1b, 0x95, > # Crc:UINT32 #WorkingBlockValid:1, WorkingBlockInvalid:1, Reserved > 0x2c, 0xaf, 0x2c, 0x64, 0xFE, 0xFF, 0xFF, 0xFF, > # WriteQueueSize: UINT64 > 0xE0, 0x0F, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 > } > > +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) > 0x00010000|0x00010000 > +!endif > +!if $(FD_SIZE_IN_KB) == 4096 > +0x00042000|0x00042000 > +!endif > #NV_FTW_SPARE > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 3/5] OvmfPkg: introduce 4MB flash image (mainly) for Windows HCK 2017-05-05 4:07 ` Laszlo Ersek @ 2017-05-05 8:57 ` Jordan Justen 2017-05-05 12:07 ` Laszlo Ersek 0 siblings, 1 reply; 17+ messages in thread From: Jordan Justen @ 2017-05-05 8:57 UTC (permalink / raw) To: Laszlo Ersek; +Cc: edk2-devel-01, Gary Ching-Pang Lin [-- Attachment #1: Type: text/plain, Size: 1093 bytes --] On 2017-05-04 21:07:24, Laszlo Ersek wrote: > On 05/03/17 23:39, Laszlo Ersek wrote: > > - Raise VARS_LIVE_SIZE by 8KB to 256KB, VARS_SPARE_SIZE by 8KB to 264KB, > > Unfortunately, we have a terrible regression here. :( > <snip> > Adapting EmuVariableFvbRuntimeDxe to a non-power-of-two NV spare area > size is hopeless. (Definitely hopeless for the time frame & resources > I'm looking at.) > > Worse, even -pflash is broken in the 4MB build, actually. The > non-power-of-two NV spare area size, when used as Alignment for > AllocateAlignedRuntimePages() in ReserveEmuVariableNvStore(), triggers > an assertion. And this path is taken for the -pflash boot as well. For a short term fix, would something like this work? 1. Force emu fvb buffer alignment to next power-of-two Something like the attachment, but I'm guessing you already wrote something similar. 2. Revert 4MB by default patch This should allow you to start using the 4MB layout for your builds, and we can fix the non-flash path before re-enabling 4MB as the default. -Jordan [-- Attachment #2: 0001-PlatformPei-Force-EmuVariableNvStore-alignment-size-.patch --] [-- Type: text/x-diff, Size: 1847 bytes --] >From 58ba393adf60caf806b26dec9aa56d936743f595 Mon Sep 17 00:00:00 2001 From: Jordan Justen <jordan.l.justen@intel.com> Date: Fri, 5 May 2017 01:43:31 -0700 Subject: [PATCH] PlatformPei: Force EmuVariableNvStore alignment size to power of two Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Jordan Justen <jordan.l.justen@intel.com> --- OvmfPkg/PlatformPei/Platform.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c index 77a8a16c15..97dce8de92 100644 --- a/OvmfPkg/PlatformPei/Platform.c +++ b/OvmfPkg/PlatformPei/Platform.c @@ -504,6 +504,14 @@ ReserveEmuVariableNvStore ( { EFI_PHYSICAL_ADDRESS VariableStore; RETURN_STATUS PcdStatus; + UINT32 EmuFvbSize; + INTN SizeHighBit; + + EmuFvbSize = 2 * PcdGet32 (PcdFlashNvStorageFtwSpareSize); + SizeHighBit = HighBitSet32 (EmuFvbSize); + if ((EmuFvbSize & (EmuFvbSize - 1)) != 0) { + SizeHighBit++; + } // // Allocate storage for NV variables early on so it will be @@ -514,13 +522,13 @@ ReserveEmuVariableNvStore ( VariableStore = (EFI_PHYSICAL_ADDRESS)(UINTN) AllocateAlignedRuntimePages ( - EFI_SIZE_TO_PAGES (2 * PcdGet32 (PcdFlashNvStorageFtwSpareSize)), - PcdGet32 (PcdFlashNvStorageFtwSpareSize) + EFI_SIZE_TO_PAGES (EmuFvbSize), + 1 << (SizeHighBit - 1) ); DEBUG ((EFI_D_INFO, "Reserved variable store memory: 0x%lX; size: %dkb\n", VariableStore, - (2 * PcdGet32 (PcdFlashNvStorageFtwSpareSize)) / 1024 + EmuFvbSize / 1024 )); PcdStatus = PcdSet64S (PcdEmuVariableNvStoreReserved, VariableStore); ASSERT_RETURN_ERROR (PcdStatus); -- 2.11.0 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 3/5] OvmfPkg: introduce 4MB flash image (mainly) for Windows HCK 2017-05-05 8:57 ` Jordan Justen @ 2017-05-05 12:07 ` Laszlo Ersek 2017-05-05 15:43 ` Jordan Justen 0 siblings, 1 reply; 17+ messages in thread From: Laszlo Ersek @ 2017-05-05 12:07 UTC (permalink / raw) To: Jordan Justen; +Cc: edk2-devel-01, Gary Ching-Pang Lin [-- Attachment #1: Type: text/plain, Size: 5024 bytes --] On 05/05/17 10:57, Jordan Justen wrote: > On 2017-05-04 21:07:24, Laszlo Ersek wrote: >> On 05/03/17 23:39, Laszlo Ersek wrote: >>> - Raise VARS_LIVE_SIZE by 8KB to 256KB, VARS_SPARE_SIZE by 8KB to 264KB, >> Unfortunately, we have a terrible regression here. :( >> > <snip> > >> Adapting EmuVariableFvbRuntimeDxe to a non-power-of-two NV spare area >> size is hopeless. (Definitely hopeless for the time frame & resources >> I'm looking at.) >> >> Worse, even -pflash is broken in the 4MB build, actually. The >> non-power-of-two NV spare area size, when used as Alignment for >> AllocateAlignedRuntimePages() in ReserveEmuVariableNvStore(), triggers >> an assertion. And this path is taken for the -pflash boot as well. > For a short term fix, would something like this work? Absolutely. I didn't dare ask for it. > 1. Force emu fvb buffer alignment to next power-of-two > > Something like the attachment, but I'm guessing you already wrote > something similar. Yes, I have almost exactly that; please see the attached "OvmfPkg/PlatformPei: handle non-power-of-two spare size for emu variables". The difference is that your patch uses HighBitSet32() directly, which mine uses through GetPowerOfTwo32(). Also yours starts with the full size, and then subtracts one for the shift in the alignment; mine starts with half the size (i.e., the spare area size) and uses that in the alignment. One other comment on the patch: > 0001-PlatformPei-Force-EmuVariableNvStore-alignment-size-.patch > > > From 58ba393adf60caf806b26dec9aa56d936743f595 Mon Sep 17 00:00:00 2001 > From: Jordan Justen <jordan.l.justen@intel.com> > Date: Fri, 5 May 2017 01:43:31 -0700 > Subject: [PATCH] PlatformPei: Force EmuVariableNvStore alignment size to power > of two > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Jordan Justen <jordan.l.justen@intel.com> > --- > OvmfPkg/PlatformPei/Platform.c | 14 +++++++++++--- > 1 file changed, 11 insertions(+), 3 deletions(-) > > diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c > index 77a8a16c15..97dce8de92 100644 > --- a/OvmfPkg/PlatformPei/Platform.c > +++ b/OvmfPkg/PlatformPei/Platform.c > @@ -504,6 +504,14 @@ ReserveEmuVariableNvStore ( > { > EFI_PHYSICAL_ADDRESS VariableStore; > RETURN_STATUS PcdStatus; > + UINT32 EmuFvbSize; > + INTN SizeHighBit; > + > + EmuFvbSize = 2 * PcdGet32 (PcdFlashNvStorageFtwSpareSize); > + SizeHighBit = HighBitSet32 (EmuFvbSize); > + if ((EmuFvbSize & (EmuFvbSize - 1)) != 0) { > + SizeHighBit++; > + } > > // > // Allocate storage for NV variables early on so it will be > @@ -514,13 +522,13 @@ ReserveEmuVariableNvStore ( > VariableStore = > (EFI_PHYSICAL_ADDRESS)(UINTN) > AllocateAlignedRuntimePages ( > - EFI_SIZE_TO_PAGES (2 * PcdGet32 (PcdFlashNvStorageFtwSpareSize)), > - PcdGet32 (PcdFlashNvStorageFtwSpareSize) > + EFI_SIZE_TO_PAGES (EmuFvbSize), I think we should cast EmuFvbSize to UINTN first; EFI_SIZE_TO_PAGES() likes the "Size" parameter to be UINTN. > + 1 << (SizeHighBit - 1) > ); > DEBUG ((EFI_D_INFO, > "Reserved variable store memory: 0x%lX; size: %dkb\n", > VariableStore, > - (2 * PcdGet32 (PcdFlashNvStorageFtwSpareSize)) / 1024 > + EmuFvbSize / 1024 > )); > PcdStatus = PcdSet64S (PcdEmuVariableNvStoreReserved, VariableStore); > ASSERT_RETURN_ERROR (PcdStatus); > -- 2.11.0 > Which one do you prefer: (1) I can take your patch, stick in the UINTN cast, and expand the commit message a bit (similarly to what's on my patch), (2) we can go with my patch as well. I'm tempted to do (1) and commit it with my R-b immediately, but I realize that "rushing it" is the root of all evil. So I won't rush it. On 05/05/17 10:57, Jordan Justen wrote: > 2. Revert 4MB by default patch > > This should allow you to start using the 4MB layout for your builds, > and we can fix the non-flash path before re-enabling 4MB as the > default. This works for me, yes. Thank you. Regarding the non-flash path, I have the attached work-in-progress patches: - "OvmfPkg: sync PcdVariableStoreSize with PcdFlashNvStorageVariableSize", - and "wip". I think that the "wip" patch does all the "simple" fixes in EmuVariableFvbRuntimeDxe, and what remains is to add code so that the FVB protocol members actually do their job. Also, the "wip" patch eliminates any special alignment in the AllocateRuntimePages() call of PlatformPei, since after the "block size = page size" change, no such alignment will be necessary. What do you think of this? So here's the plan: - based on what you prefer for (1) vs. (2), I'll post that patch as first patch, - I'll post the revert of the 4MB default as second patch, - I think I'll immediately post the patch that syncs PcdVariableStoreSize as well, as third patch, because it is a small improvement for the pflash case too. Does this work for you? Thank you, Laszlo [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-OvmfPkg-PlatformPei-handle-non-power-of-two-spare-si.patch --] [-- Type: text/x-patch; name="0001-OvmfPkg-PlatformPei-handle-non-power-of-two-spare-si.patch", Size: 2833 bytes --] From 7bd8738556abb86bdbfdd11dd30ad78663925c09 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek <lersek@redhat.com> Date: Fri, 5 May 2017 02:35:21 +0200 Subject: [PATCH] OvmfPkg/PlatformPei: handle non-power-of-two spare size for emu variables In commit b24fca05751f ("OvmfPkg: introduce 4MB flash image (mainly) for Windows HCK", 2017-04-29), I changed PcdFlashNvStorageFtwSpareSize to 264KB, in the (new default) 4MB build. While PcdFlashNvStorageFtwSpareSize remains exactly half of the entire non-volatile store (which is 528KB), 264KB isn't itself a power of two. This triggers an assertion failure in AllocateAlignedRuntimePages() when PlatformPei calls it from the ReserveEmuVariableNvStore() function, passing PcdFlashNvStorageFtwSpareSize as the Alignment parameter: > ASSERT MdePkg/Library/PeiMemoryAllocationLib/MemoryAllocationLib.c(196): > (Alignment & (Alignment - 1)) == 0 Round up the alignment to the next power of two if necessary. Cc: Jordan Justen <jordan.l.justen@intel.com> Fixes: b24fca05751f8222acf264853709012e0ab7bf49 Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- OvmfPkg/PlatformPei/Platform.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c index 77a8a16c15b8..5e983a8dcea9 100644 --- a/OvmfPkg/PlatformPei/Platform.c +++ b/OvmfPkg/PlatformPei/Platform.c @@ -504,6 +504,7 @@ ReserveEmuVariableNvStore ( { EFI_PHYSICAL_ADDRESS VariableStore; RETURN_STATUS PcdStatus; + UINT32 Alignment; // // Allocate storage for NV variables early on so it will be @@ -511,16 +512,26 @@ ReserveEmuVariableNvStore ( // across reboots, this allows the NV variable storage to survive // a VM reboot. // + Alignment = PcdGet32 (PcdFlashNvStorageFtwSpareSize); + if ((Alignment & (Alignment - 1)) != 0) { + // + // Round up Alignment to the next power of two. + // + Alignment = GetPowerOfTwo32 (Alignment) << 1; + } + VariableStore = (EFI_PHYSICAL_ADDRESS)(UINTN) AllocateAlignedRuntimePages ( EFI_SIZE_TO_PAGES (2 * PcdGet32 (PcdFlashNvStorageFtwSpareSize)), - PcdGet32 (PcdFlashNvStorageFtwSpareSize) + Alignment ); DEBUG ((EFI_D_INFO, - "Reserved variable store memory: 0x%lX; size: %dkb\n", + "Reserved variable store memory: 0x%lX; size: %dkb, " + "alignment: 0x%x\n", VariableStore, - (2 * PcdGet32 (PcdFlashNvStorageFtwSpareSize)) / 1024 + (2 * PcdGet32 (PcdFlashNvStorageFtwSpareSize)) / 1024, + Alignment )); PcdStatus = PcdSet64S (PcdEmuVariableNvStoreReserved, VariableStore); ASSERT_RETURN_ERROR (PcdStatus); -- 2.9.3 [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #3: 0001-OvmfPkg-sync-PcdVariableStoreSize-with-PcdFlashNvSto.patch --] [-- Type: text/x-patch; name="0001-OvmfPkg-sync-PcdVariableStoreSize-with-PcdFlashNvSto.patch", Size: 4236 bytes --] From 2d6500afb5870e577bab93ef9f54cab9e930703e Mon Sep 17 00:00:00 2001 From: Laszlo Ersek <lersek@redhat.com> Date: Fri, 5 May 2017 03:31:32 +0200 Subject: [PATCH 1/2] OvmfPkg: sync PcdVariableStoreSize with PcdFlashNvStorageVariableSize "MdeModulePkg/MdeModulePkg.dec" declares PcdVariableStoreSize like this: > The size of volatile buffer. This buffer is used to store VOLATILE > attribute variables. There is no inherent reason why the size of the volatile variable store should match the same of the non-volatile variable store. Indeed flash variables in the 4MB build work fine without this equality. However, OvmfPkg/EmuVariableFvbRuntimeDxe uses PcdVariableStoreSize to initialize the non-volatile VARIABLE_STORE_HEADER too. (Presumably based on the fact that ultimately that storage will not be permanent.) When using EmuVariableFvbRuntimeDxe in the 4MB build, the mismatch between the two mentioned PCDs (which is apparent through EmuVariableFvbRuntimeDxe's VARIABLE_STORE_HEADER) triggers an assertion in the variable driver: > ASSERT MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c(3772): > mNvVariableCache->Size == VariableStoreLength Bringing PcdVariableStoreSize in sync with PcdFlashNvStorageVariableSize fixes this. It also happens to ensure a volatile store size in the 4MB build that equals the non-volatile store size, which likely doesn't hurt for symmetry. Cc: Jordan Justen <jordan.l.justen@intel.com> Fixes: b24fca05751f8222acf264853709012e0ab7bf49 Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- OvmfPkg/OvmfPkgIa32.dsc | 3 ++- OvmfPkg/OvmfPkgIa32X64.dsc | 3 ++- OvmfPkg/OvmfPkgX64.dsc | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc index e0ff7ead034e..4f518661e3b6 100644 --- a/OvmfPkg/OvmfPkgIa32.dsc +++ b/OvmfPkg/OvmfPkgIa32.dsc @@ -417,12 +417,13 @@ [PcdsFixedAtBuild] !if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000 gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800 + gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000 !endif !if $(FD_SIZE_IN_KB) == 4096 gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400 gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400 + gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0x40000 !endif - gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000 gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0 diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc index 41922f581c98..4551021f5efa 100644 --- a/OvmfPkg/OvmfPkgIa32X64.dsc +++ b/OvmfPkg/OvmfPkgIa32X64.dsc @@ -422,12 +422,13 @@ [PcdsFixedAtBuild] !if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000 gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800 + gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000 !endif !if $(FD_SIZE_IN_KB) == 4096 gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400 gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400 + gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0x40000 !endif - gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000 gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0 diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc index 9ba033956c88..f993d392ce9c 100644 --- a/OvmfPkg/OvmfPkgX64.dsc +++ b/OvmfPkg/OvmfPkgX64.dsc @@ -422,12 +422,13 @@ [PcdsFixedAtBuild] !if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000 gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800 + gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000 !endif !if $(FD_SIZE_IN_KB) == 4096 gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400 gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400 + gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0x40000 !endif - gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000 gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0 -- 2.9.3 [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #4: 0002-wip.patch --] [-- Type: text/x-patch; name="0002-wip.patch", Size: 5165 bytes --] From 065349ecf62e0ec8b0cb40384bf43e6af863a4de Mon Sep 17 00:00:00 2001 From: Laszlo Ersek <lersek@redhat.com> Date: Fri, 5 May 2017 04:26:47 +0200 Subject: [PATCH 2/2] wip Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h | 10 +++++++-- OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c | 23 +++++++++++--------- OvmfPkg/PlatformPei/Platform.c | 5 ++--- 3 files changed, 23 insertions(+), 15 deletions(-) diff --git a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h index f34fad2cdd48..090b218aaf55 100644 --- a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h +++ b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h @@ -58,8 +58,14 @@ typedef struct { // // Constants // -#define EMU_FVB_BLOCK_SIZE (FixedPcdGet32 (PcdFlashNvStorageFtwSpareSize)) -#define EMU_FVB_SIZE (2 * FixedPcdGet32 (PcdFlashNvStorageFtwSpareSize)) +#define EMU_FVB_BLOCK_SIZE \ + EFI_PAGE_SIZE +#define EMU_FVB_NUM_SPARE_BLOCKS \ + EFI_SIZE_TO_PAGES ((UINTN)FixedPcdGet32 (PcdFlashNvStorageFtwSpareSize)) +#define EMU_FVB_NUM_TOTAL_BLOCKS \ + (2 * EMU_FVB_NUM_SPARE_BLOCKS) +#define EMU_FVB_SIZE \ + (EMU_FVB_NUM_TOTAL_BLOCKS * EMU_FVB_BLOCK_SIZE) #define FTW_WRITE_QUEUE_SIZE \ (FixedPcdGet32 (PcdFlashNvStorageFtwWorkingSize) - \ sizeof (EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER)) diff --git a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c index dec6d4af50df..990910deb227 100644 --- a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c +++ b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c @@ -74,8 +74,8 @@ EFI_FW_VOL_BLOCK_DEVICE mEmuVarsFvb = { } }, NULL, // BufferPtr - FixedPcdGet32 (PcdFlashNvStorageFtwSpareSize), // BlockSize - 2 * FixedPcdGet32 (PcdFlashNvStorageFtwSpareSize), // Size + EMU_FVB_BLOCK_SIZE, // BlockSize + EMU_FVB_SIZE, // Size { // FwVolBlockInstance FvbProtocolGetAttributes, FvbProtocolSetAttributes, @@ -185,14 +185,14 @@ FvbProtocolGetBlockSize ( { EFI_FW_VOL_BLOCK_DEVICE *FvbDevice; - if (Lba > 1) { + if (Lba > EMU_FVB_NUM_TOTAL_BLOCKS - 1) { return EFI_INVALID_PARAMETER; } FvbDevice = FVB_DEVICE_FROM_THIS (This); *BlockSize = FvbDevice->BlockSize; - *NumberOfBlocks = (UINTN) (2 - (UINTN) Lba); + *NumberOfBlocks = (UINTN) (EMU_FVB_NUM_TOTAL_BLOCKS - (UINTN) Lba); return EFI_SUCCESS; } @@ -345,7 +345,9 @@ FvbProtocolEraseBlocks ( // // Check input parameters // - if ((NumOfLba == 0) || (StartingLba > 1) || ((StartingLba + NumOfLba) > 2)) { + if ((NumOfLba == 0) || + (StartingLba > EMU_FVB_NUM_TOTAL_BLOCKS - 1) || + ((StartingLba + NumOfLba) > EMU_FVB_NUM_TOTAL_BLOCKS)) { VA_END (args); return EFI_INVALID_PARAMETER; } @@ -353,7 +355,7 @@ FvbProtocolEraseBlocks ( if (StartingLba == 0) { Erase = (UINT8) (Erase | BIT0); } - if ((StartingLba + NumOfLba) == 2) { + if ((StartingLba + NumOfLba) == EMU_FVB_NUM_TOTAL_BLOCKS) { Erase = (UINT8) (Erase | BIT1); } @@ -663,7 +665,7 @@ InitializeFvAndVariableStoreHeaders ( // EFI_FV_BLOCK_MAP_ENTRY BlockMap[1]; { { - 2, // UINT32 NumBlocks; + EMU_FVB_NUM_TOTAL_BLOCKS, // UINT32 NumBlocks; EMU_FVB_BLOCK_SIZE // UINT32 Length; } } @@ -732,7 +734,7 @@ InitializeFvAndVariableStoreHeaders ( // EFI_FV_BLOCK_MAP_ENTRY BlockMap[1]; { { - 2, // UINT32 NumBlocks; + EMU_FVB_NUM_TOTAL_BLOCKS, // UINT32 NumBlocks; EMU_FVB_BLOCK_SIZE // UINT32 Length; } } @@ -814,7 +816,7 @@ FvbInitialize ( (PcdGet32 (PcdVariableStoreSize) + PcdGet32 (PcdFlashNvStorageFtwWorkingSize) ) > - EMU_FVB_BLOCK_SIZE + EMU_FVB_BLOCK_SIZE * EMU_FVB_NUM_SPARE_BLOCKS ) { DEBUG ((EFI_D_ERROR, "EMU Variable invalid PCD sizes\n")); return EFI_INVALID_PARAMETER; @@ -877,7 +879,8 @@ FvbInitialize ( // // Initialize the Fault Tolerant Write spare block // - SubPtr = (VOID*) ((UINT8*) Ptr + EMU_FVB_BLOCK_SIZE); + SubPtr = (VOID*) ((UINT8*) Ptr + + EMU_FVB_BLOCK_SIZE * EMU_FVB_NUM_SPARE_BLOCKS); PcdStatus = PcdSet32S (PcdFlashNvStorageFtwSpareBase, (UINT32)(UINTN) SubPtr); ASSERT_RETURN_ERROR (PcdStatus); diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c index 77a8a16c15b8..c2829dbcf5c7 100644 --- a/OvmfPkg/PlatformPei/Platform.c +++ b/OvmfPkg/PlatformPei/Platform.c @@ -513,9 +513,8 @@ ReserveEmuVariableNvStore ( // VariableStore = (EFI_PHYSICAL_ADDRESS)(UINTN) - AllocateAlignedRuntimePages ( - EFI_SIZE_TO_PAGES (2 * PcdGet32 (PcdFlashNvStorageFtwSpareSize)), - PcdGet32 (PcdFlashNvStorageFtwSpareSize) + AllocateRuntimePages ( + EFI_SIZE_TO_PAGES (2 * PcdGet32 (PcdFlashNvStorageFtwSpareSize)) ); DEBUG ((EFI_D_INFO, "Reserved variable store memory: 0x%lX; size: %dkb\n", -- 2.9.3 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 3/5] OvmfPkg: introduce 4MB flash image (mainly) for Windows HCK 2017-05-05 12:07 ` Laszlo Ersek @ 2017-05-05 15:43 ` Jordan Justen 2017-05-05 16:46 ` Laszlo Ersek 0 siblings, 1 reply; 17+ messages in thread From: Jordan Justen @ 2017-05-05 15:43 UTC (permalink / raw) To: Laszlo Ersek; +Cc: edk2-devel-01, Gary Ching-Pang Lin On 2017-05-05 05:07:25, Laszlo Ersek wrote: > > Which one do you prefer: > (1) I can take your patch, stick in the UINTN cast, and expand the > commit message a bit (similarly to what's on my patch), > (2) we can go with my patch as well. I prefer your patch. Re: "0001-OvmfPkg-PlatformPei-handle-non-power-of-two-spare-si.patch" > + Alignment = GetPowerOfTwo32 (Alignment) << 1; Do you think this might end up needing a UINT32 cast with 64-bit PEI? Reviewed-by: Jordan Justen <jordan.l.justen@intel.com> > Regarding the non-flash path, I have the attached work-in-progress patches: > > - "OvmfPkg: sync PcdVariableStoreSize with PcdFlashNvStorageVariableSize", > - and "wip". I looked over the non-wip patch, and it seems good, but maybe we should just revert the default size for now. For a revert of bba8dfbec3bbc4fba7fa6398ba3cf76593e0725e: Reviewed-by: Jordan Justen <jordan.l.justen@intel.com> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 3/5] OvmfPkg: introduce 4MB flash image (mainly) for Windows HCK 2017-05-05 15:43 ` Jordan Justen @ 2017-05-05 16:46 ` Laszlo Ersek 0 siblings, 0 replies; 17+ messages in thread From: Laszlo Ersek @ 2017-05-05 16:46 UTC (permalink / raw) To: Jordan Justen; +Cc: edk2-devel-01, Gary Ching-Pang Lin On 05/05/17 17:43, Jordan Justen wrote: > On 2017-05-05 05:07:25, Laszlo Ersek wrote: >> >> Which one do you prefer: >> (1) I can take your patch, stick in the UINTN cast, and expand the >> commit message a bit (similarly to what's on my patch), >> (2) we can go with my patch as well. > > I prefer your patch. > > Re: "0001-OvmfPkg-PlatformPei-handle-non-power-of-two-spare-si.patch" > >> + Alignment = GetPowerOfTwo32 (Alignment) << 1; > > Do you think this might end up needing a UINT32 cast with 64-bit PEI? No, the prototype is UINT32 EFIAPI GetPowerOfTwo32 ( IN UINT32 Operand ) and we can safely shift the returned UINT32 in both 32-bit PEI and 64-bit PEI with the << operator. The result of the shift will also have type UINT32, which we can safely re-assign to the UINT32 Alignment variable. Shifting the result of GetPowerOfTwo64() with << would be problematic in 32-bit PEI. > > Reviewed-by: Jordan Justen <jordan.l.justen@intel.com> Thanks! I verified this in the (now again default 2MB build) with the -bios command line like this: (1) boot to the UEFI shell normally. The log says: > Reserved variable store memory: 0x7F50000; size: 128kb, > alignment: 0x10000 > ... > EMU Variable FVB Started > EMU Variable FVB: Using pre-reserved block at 7F50000 > EMU Variable FVB: Basic FV headers were invalid > Installing FVB for EMU Variable support > ... > Ftw: FtwWorkSpaceLba - 0x0, WorkBlockSize - 0x10000, > FtwWorkSpaceBase - 0xE000 > Ftw: FtwSpareLba - 0x1, SpareBlockSize - 0x10000 > Ftw: NumberOfWorkBlock - 0x1, FtwWorkBlockLba - 0x0 > Ftw: WorkSpaceLbaInSpare - 0x0, WorkSpaceBaseInSpare - 0xE000 > Ftw: Remaining work space size - FE0 > Ftw: Work block header check mismatch > Ftw: Work block header check mismatch > Ftw: Both working and spare blocks are invalid, init workspace > Ftw: start to reclaim work space > Ftw: reclaim work space successfully (2) run the following shell commands: Shell> setvar TestVar0000 -guid ebe29c42-f3d1-4f96-a7c2-5585ce88f056 -bs -nv ="0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" Shell> reset (3) after reboot, the log says: > Reserved variable store memory: 0x7F50000; size: 128kb, > alignment: 0x10000 > ... > EMU Variable FVB Started > EMU Variable FVB: Using pre-reserved block at 7F50000 > EMU Variable FVB: Found valid pre-existing FV > ... > Ftw: FtwWorkSpaceLba - 0x0, WorkBlockSize - 0x10000, > FtwWorkSpaceBase - 0xE000 > Ftw: FtwSpareLba - 0x1, SpareBlockSize - 0x10000 > Ftw: NumberOfWorkBlock - 0x1, FtwWorkBlockLba - 0x0 > Ftw: WorkSpaceLbaInSpare - 0x0, WorkSpaceBaseInSpare - 0xE000 > Ftw: Remaining work space size - FE0 (4) run the following shell command: Shell> setvar TestVar0000 -guid ebe29c42-f3d1-4f96-a7c2-5585ce88f056 EBE29C42-F3D1-4F96-A7C2-5585CE88F056 - TestVar0000 - 0040 Bytes 01 23 45 67 89 AB CD EF 01 23 45 67 89 AB CD EF 01 23 45 67 89 AB CD EF 01 23 45 67 89 AB CD EF 01 23 45 67 89 AB CD EF 01 23 45 67 89 AB CD EF 01 23 45 67 89 AB CD EF 01 23 45 67 89 AB CD EF Also tested this with the 4MB build, using pflash. The log says > Reserved variable store memory: 0xBFE80000; size: 528kb, > alignment: 0x80000 The alignment is 512KB, which is the next power of two after 264KB. > >> Regarding the non-flash path, I have the attached work-in-progress patches: >> >> - "OvmfPkg: sync PcdVariableStoreSize with PcdFlashNvStorageVariableSize", >> - and "wip". > > I looked over the non-wip patch, and it seems good, but maybe we > should just revert the default size for now. Works for me. > For a revert of bba8dfbec3bbc4fba7fa6398ba3cf76593e0725e: > > Reviewed-by: Jordan Justen <jordan.l.justen@intel.com> > Thanks! Commits: 1 6e49d01cfb43 Revert "OvmfPkg: make the 4MB flash size the default" 2 0c79471d6a98 OvmfPkg/PlatformPei: handle non-power-of-two spare size for emu variables I've also filed the following TianoCore Feature Request: https://bugzilla.tianocore.org/show_bug.cgi?id=527 If you or Gary feel inclined to work on this, that would be awesome. I have no idea when I'll get to it; my TODO list hasn't been this long in ages. Of course I cannot really *request* that you guys please pick up my slack, just sayin' that you please feel free to... :) Thanks! Laszlo ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 4/5] OvmfPkg: raise max variable size (auth & non-auth) to 33KB for FD_SIZE_4MB 2017-05-03 21:39 [PATCH v2 0/5] OvmfPkg: add FD_SIZE_4MB for Windows HCK SB tests, and for future proofing Laszlo Ersek ` (2 preceding siblings ...) 2017-05-03 21:39 ` [PATCH v2 3/5] OvmfPkg: introduce 4MB flash image (mainly) for Windows HCK Laszlo Ersek @ 2017-05-03 21:39 ` Laszlo Ersek 2017-05-04 16:36 ` Jordan Justen 2017-05-03 21:39 ` [PATCH v2 5/5] OvmfPkg: make the 4MB flash size the default Laszlo Ersek 4 siblings, 1 reply; 17+ messages in thread From: Laszlo Ersek @ 2017-05-03 21:39 UTC (permalink / raw) To: edk2-devel-01; +Cc: Gary Ching-Pang Lin, Jordan Justen The "ConfirmSetOfLargeVariable" test case of the Secure Boot Logo Test ("Microsoft.UefiSecureBootLogo.Tests") suite in the Microsoft Hardware Certification Kit sets a 32 KB large non-authenticated variable. In the FD_SIZE_4MB build, our live varstore is now 256 KB big, so we can accommodate this. Set both PcdMaxVariableSize and PcdMaxAuthVariableSize to 0x8400 -- beyond DataSize=0x8000 from the HCK test, we need some room for the variable name and attributes as well. Cc: Gary Ching-Pang Lin <glin@suse.com> Cc: Jordan Justen <jordan.l.justen@intel.com> Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- Notes: v2: - adjust to FD_SIZE_IN_KB - update commit msg to state 256 KB for the varstore [Jordan] OvmfPkg/OvmfPkgIa32.dsc | 6 ++++++ OvmfPkg/OvmfPkgIa32X64.dsc | 6 ++++++ OvmfPkg/OvmfPkgX64.dsc | 6 ++++++ 3 files changed, 18 insertions(+) diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc index 26b807dde9fa..e0779ddaa426 100644 --- a/OvmfPkg/OvmfPkgIa32.dsc +++ b/OvmfPkg/OvmfPkgIa32.dsc @@ -404,28 +404,34 @@ [PcdsFeatureFlag] gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE !endif !if $(SMM_REQUIRE) == TRUE gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE !endif [PcdsFixedAtBuild] gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1 gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10 gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6 gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32 +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000 gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800 +!endif +!if $(FD_SIZE_IN_KB) == 4096 + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400 + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400 +!endif gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000 gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0 gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07 # DEBUG_INIT 0x00000001 // Initialization # DEBUG_WARN 0x00000002 // Warnings # DEBUG_LOAD 0x00000004 // Load events # DEBUG_FS 0x00000008 // EFI File system # DEBUG_POOL 0x00000010 // Alloc & Free (pool) # DEBUG_PAGE 0x00000020 // Alloc & Free (page) # DEBUG_INFO 0x00000040 // Informational debug messages diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc index 41f06a6b6a66..bbe26e2cf452 100644 --- a/OvmfPkg/OvmfPkgIa32X64.dsc +++ b/OvmfPkg/OvmfPkgIa32X64.dsc @@ -409,28 +409,34 @@ [PcdsFeatureFlag] gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE !endif !if $(SMM_REQUIRE) == TRUE gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE !endif [PcdsFixedAtBuild] gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1 gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10 gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6 gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32 +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000 gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800 +!endif +!if $(FD_SIZE_IN_KB) == 4096 + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400 + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400 +!endif gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000 gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0 gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07 # DEBUG_INIT 0x00000001 // Initialization # DEBUG_WARN 0x00000002 // Warnings # DEBUG_LOAD 0x00000004 // Load events # DEBUG_FS 0x00000008 // EFI File system # DEBUG_POOL 0x00000010 // Alloc & Free (pool) # DEBUG_PAGE 0x00000020 // Alloc & Free (page) # DEBUG_INFO 0x00000040 // Informational debug messages diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc index 053c84b685c5..ff795815f65f 100644 --- a/OvmfPkg/OvmfPkgX64.dsc +++ b/OvmfPkg/OvmfPkgX64.dsc @@ -409,28 +409,34 @@ [PcdsFeatureFlag] gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE !endif !if $(SMM_REQUIRE) == TRUE gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE !endif [PcdsFixedAtBuild] gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1 gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10 gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6 gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32 +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000 gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800 +!endif +!if $(FD_SIZE_IN_KB) == 4096 + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400 + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400 +!endif gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000 gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0 gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07 # DEBUG_INIT 0x00000001 // Initialization # DEBUG_WARN 0x00000002 // Warnings # DEBUG_LOAD 0x00000004 // Load events # DEBUG_FS 0x00000008 // EFI File system # DEBUG_POOL 0x00000010 // Alloc & Free (pool) # DEBUG_PAGE 0x00000020 // Alloc & Free (page) # DEBUG_INFO 0x00000040 // Informational debug messages -- 2.9.3 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 4/5] OvmfPkg: raise max variable size (auth & non-auth) to 33KB for FD_SIZE_4MB 2017-05-03 21:39 ` [PATCH v2 4/5] OvmfPkg: raise max variable size (auth & non-auth) to 33KB for FD_SIZE_4MB Laszlo Ersek @ 2017-05-04 16:36 ` Jordan Justen 2017-05-04 16:52 ` Laszlo Ersek 0 siblings, 1 reply; 17+ messages in thread From: Jordan Justen @ 2017-05-04 16:36 UTC (permalink / raw) To: Laszlo Ersek, edk2-devel-01; +Cc: Gary Ching-Pang Lin On 2017-05-03 14:39:46, Laszlo Ersek wrote: > The "ConfirmSetOfLargeVariable" test case of the Secure Boot Logo Test > ("Microsoft.UefiSecureBootLogo.Tests") suite in the Microsoft Hardware > Certification Kit sets a 32 KB large non-authenticated variable. According to http://www.uefi.org/sites/default/files/resources/UEFI_Plugfest_Security_Microsoft_Fall_2016.pdf "The maximum supported variable size must be at least 64kB" Should we just bump the size to match this? We should be able to make this change later once it is in a test/spec, but for some reason I thought the requirement was already 64k. Aside from this question: Series Reviewed-by: Jordan Justen <jordan.l.justen@intel.com> > In the FD_SIZE_4MB build, our live varstore is now 256 KB big, so we can > accommodate this. Set both PcdMaxVariableSize and PcdMaxAuthVariableSize > to 0x8400 -- beyond DataSize=0x8000 from the HCK test, we need some room > for the variable name and attributes as well. > > Cc: Gary Ching-Pang Lin <glin@suse.com> > Cc: Jordan Justen <jordan.l.justen@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Laszlo Ersek <lersek@redhat.com> > --- > > Notes: > v2: > - adjust to FD_SIZE_IN_KB > - update commit msg to state 256 KB for the varstore [Jordan] > > OvmfPkg/OvmfPkgIa32.dsc | 6 ++++++ > OvmfPkg/OvmfPkgIa32X64.dsc | 6 ++++++ > OvmfPkg/OvmfPkgX64.dsc | 6 ++++++ > 3 files changed, 18 insertions(+) > > diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc > index 26b807dde9fa..e0779ddaa426 100644 > --- a/OvmfPkg/OvmfPkgIa32.dsc > +++ b/OvmfPkg/OvmfPkgIa32.dsc > @@ -404,28 +404,34 @@ [PcdsFeatureFlag] > gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE > !endif > !if $(SMM_REQUIRE) == TRUE > gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE > gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE > !endif > > [PcdsFixedAtBuild] > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1 > gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE > gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10 > gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6 > gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32 > +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000 > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800 > +!endif > +!if $(FD_SIZE_IN_KB) == 4096 > + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400 > + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400 > +!endif > gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000 > > gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0 > > gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07 > > # DEBUG_INIT 0x00000001 // Initialization > # DEBUG_WARN 0x00000002 // Warnings > # DEBUG_LOAD 0x00000004 // Load events > # DEBUG_FS 0x00000008 // EFI File system > # DEBUG_POOL 0x00000010 // Alloc & Free (pool) > # DEBUG_PAGE 0x00000020 // Alloc & Free (page) > # DEBUG_INFO 0x00000040 // Informational debug messages > diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc > index 41f06a6b6a66..bbe26e2cf452 100644 > --- a/OvmfPkg/OvmfPkgIa32X64.dsc > +++ b/OvmfPkg/OvmfPkgIa32X64.dsc > @@ -409,28 +409,34 @@ [PcdsFeatureFlag] > gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE > !endif > !if $(SMM_REQUIRE) == TRUE > gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE > gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE > !endif > > [PcdsFixedAtBuild] > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1 > gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE > gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10 > gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6 > gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32 > +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000 > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800 > +!endif > +!if $(FD_SIZE_IN_KB) == 4096 > + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400 > + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400 > +!endif > gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000 > > gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0 > > gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07 > > # DEBUG_INIT 0x00000001 // Initialization > # DEBUG_WARN 0x00000002 // Warnings > # DEBUG_LOAD 0x00000004 // Load events > # DEBUG_FS 0x00000008 // EFI File system > # DEBUG_POOL 0x00000010 // Alloc & Free (pool) > # DEBUG_PAGE 0x00000020 // Alloc & Free (page) > # DEBUG_INFO 0x00000040 // Informational debug messages > diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc > index 053c84b685c5..ff795815f65f 100644 > --- a/OvmfPkg/OvmfPkgX64.dsc > +++ b/OvmfPkg/OvmfPkgX64.dsc > @@ -409,28 +409,34 @@ [PcdsFeatureFlag] > gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE > !endif > !if $(SMM_REQUIRE) == TRUE > gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE > gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE > !endif > > [PcdsFixedAtBuild] > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1 > gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE > gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10 > gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6 > gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32 > +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000 > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800 > +!endif > +!if $(FD_SIZE_IN_KB) == 4096 > + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400 > + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400 > +!endif > gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000 > > gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0 > > gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07 > > # DEBUG_INIT 0x00000001 // Initialization > # DEBUG_WARN 0x00000002 // Warnings > # DEBUG_LOAD 0x00000004 // Load events > # DEBUG_FS 0x00000008 // EFI File system > # DEBUG_POOL 0x00000010 // Alloc & Free (pool) > # DEBUG_PAGE 0x00000020 // Alloc & Free (page) > # DEBUG_INFO 0x00000040 // Informational debug messages > -- > 2.9.3 > > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 4/5] OvmfPkg: raise max variable size (auth & non-auth) to 33KB for FD_SIZE_4MB 2017-05-04 16:36 ` Jordan Justen @ 2017-05-04 16:52 ` Laszlo Ersek 2017-05-04 18:50 ` Jordan Justen 2017-05-04 23:00 ` Laszlo Ersek 0 siblings, 2 replies; 17+ messages in thread From: Laszlo Ersek @ 2017-05-04 16:52 UTC (permalink / raw) To: Jordan Justen, edk2-devel-01; +Cc: Gary Ching-Pang Lin On 05/04/17 18:36, Jordan Justen wrote: > On 2017-05-03 14:39:46, Laszlo Ersek wrote: >> The "ConfirmSetOfLargeVariable" test case of the Secure Boot Logo Test >> ("Microsoft.UefiSecureBootLogo.Tests") suite in the Microsoft Hardware >> Certification Kit sets a 32 KB large non-authenticated variable. > > According to > http://www.uefi.org/sites/default/files/resources/UEFI_Plugfest_Security_Microsoft_Fall_2016.pdf > > "The maximum supported variable size must be at least 64kB" > > Should we just bump the size to match this? We should be able to make > this change later once it is in a test/spec, but for some reason I > thought the requirement was already 64k. The 32KB requirement comes from the most recent Secure Boot Logo Test. I installed both the Windows Server 2008 R2 SP1 test controller and the Windows 2016 Server test client just the other day, together with the most recent filters, using the following descriptions: https://msdn.microsoft.com/en-us/library/windows/hardware/jj123537.aspx https://github.com/daynix/VirtHCK/wiki#Checklist_for_a_New_Controller_VM https://github.com/daynix/VirtHCK/wiki#Checklist_for_a_New_Client_VM Given that this limit can be bumped without breaking compatibility, as you say, I'd like to remain frugal with it, same as we were in James's commit f5404a3eba1d ("OvmfPkg: Increase the maximum size for Authenticated variables", 2016-03-24). I don't understand why the plugfest presentation and the SB Logo Test require different limits... But, I'm certain our QE will find out in short order once the SB Logo Test catches up with the presentation, and I expect I'll submit the corresponding patch soon after. I dislike the speculation in this series, but breaking compatibility is even worse. (A lot worse, to me at least.) So I consider the varstore restructuring the smaller of two wrongs. However, wrt. PcdMaxVariableSize, it seems we're not being forced to either of those wrongs (i.e., breaking compat or speculation), so we can delay the increase. > > Aside from this question: > > Series Reviewed-by: Jordan Justen <jordan.l.justen@intel.com> Thanks a lot! I'll await your ACK for the above argument before pushing the series. Thanks, Laszlo >> In the FD_SIZE_4MB build, our live varstore is now 256 KB big, so we can >> accommodate this. Set both PcdMaxVariableSize and PcdMaxAuthVariableSize >> to 0x8400 -- beyond DataSize=0x8000 from the HCK test, we need some room >> for the variable name and attributes as well. >> >> Cc: Gary Ching-Pang Lin <glin@suse.com> >> Cc: Jordan Justen <jordan.l.justen@intel.com> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Laszlo Ersek <lersek@redhat.com> >> --- >> >> Notes: >> v2: >> - adjust to FD_SIZE_IN_KB >> - update commit msg to state 256 KB for the varstore [Jordan] >> >> OvmfPkg/OvmfPkgIa32.dsc | 6 ++++++ >> OvmfPkg/OvmfPkgIa32X64.dsc | 6 ++++++ >> OvmfPkg/OvmfPkgX64.dsc | 6 ++++++ >> 3 files changed, 18 insertions(+) >> >> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc >> index 26b807dde9fa..e0779ddaa426 100644 >> --- a/OvmfPkg/OvmfPkgIa32.dsc >> +++ b/OvmfPkg/OvmfPkgIa32.dsc >> @@ -404,28 +404,34 @@ [PcdsFeatureFlag] >> gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE >> !endif >> !if $(SMM_REQUIRE) == TRUE >> gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE >> gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE >> !endif >> >> [PcdsFixedAtBuild] >> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1 >> gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE >> gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10 >> gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6 >> gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32 >> +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) >> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000 >> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800 >> +!endif >> +!if $(FD_SIZE_IN_KB) == 4096 >> + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400 >> + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400 >> +!endif >> gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000 >> >> gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0 >> >> gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07 >> >> # DEBUG_INIT 0x00000001 // Initialization >> # DEBUG_WARN 0x00000002 // Warnings >> # DEBUG_LOAD 0x00000004 // Load events >> # DEBUG_FS 0x00000008 // EFI File system >> # DEBUG_POOL 0x00000010 // Alloc & Free (pool) >> # DEBUG_PAGE 0x00000020 // Alloc & Free (page) >> # DEBUG_INFO 0x00000040 // Informational debug messages >> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc >> index 41f06a6b6a66..bbe26e2cf452 100644 >> --- a/OvmfPkg/OvmfPkgIa32X64.dsc >> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc >> @@ -409,28 +409,34 @@ [PcdsFeatureFlag] >> gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE >> !endif >> !if $(SMM_REQUIRE) == TRUE >> gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE >> gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE >> !endif >> >> [PcdsFixedAtBuild] >> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1 >> gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE >> gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10 >> gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6 >> gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32 >> +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) >> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000 >> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800 >> +!endif >> +!if $(FD_SIZE_IN_KB) == 4096 >> + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400 >> + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400 >> +!endif >> gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000 >> >> gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0 >> >> gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07 >> >> # DEBUG_INIT 0x00000001 // Initialization >> # DEBUG_WARN 0x00000002 // Warnings >> # DEBUG_LOAD 0x00000004 // Load events >> # DEBUG_FS 0x00000008 // EFI File system >> # DEBUG_POOL 0x00000010 // Alloc & Free (pool) >> # DEBUG_PAGE 0x00000020 // Alloc & Free (page) >> # DEBUG_INFO 0x00000040 // Informational debug messages >> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc >> index 053c84b685c5..ff795815f65f 100644 >> --- a/OvmfPkg/OvmfPkgX64.dsc >> +++ b/OvmfPkg/OvmfPkgX64.dsc >> @@ -409,28 +409,34 @@ [PcdsFeatureFlag] >> gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE >> !endif >> !if $(SMM_REQUIRE) == TRUE >> gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE >> gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE >> !endif >> >> [PcdsFixedAtBuild] >> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1 >> gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE >> gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10 >> gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6 >> gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32 >> +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) >> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000 >> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800 >> +!endif >> +!if $(FD_SIZE_IN_KB) == 4096 >> + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400 >> + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400 >> +!endif >> gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000 >> >> gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0 >> >> gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07 >> >> # DEBUG_INIT 0x00000001 // Initialization >> # DEBUG_WARN 0x00000002 // Warnings >> # DEBUG_LOAD 0x00000004 // Load events >> # DEBUG_FS 0x00000008 // EFI File system >> # DEBUG_POOL 0x00000010 // Alloc & Free (pool) >> # DEBUG_PAGE 0x00000020 // Alloc & Free (page) >> # DEBUG_INFO 0x00000040 // Informational debug messages >> -- >> 2.9.3 >> >> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 4/5] OvmfPkg: raise max variable size (auth & non-auth) to 33KB for FD_SIZE_4MB 2017-05-04 16:52 ` Laszlo Ersek @ 2017-05-04 18:50 ` Jordan Justen 2017-05-04 19:27 ` Laszlo Ersek 2017-05-04 23:00 ` Laszlo Ersek 1 sibling, 1 reply; 17+ messages in thread From: Jordan Justen @ 2017-05-04 18:50 UTC (permalink / raw) To: Laszlo Ersek, edk2-devel-01; +Cc: Gary Ching-Pang Lin On 2017-05-04 09:52:48, Laszlo Ersek wrote: > On 05/04/17 18:36, Jordan Justen wrote: > > On 2017-05-03 14:39:46, Laszlo Ersek wrote: > >> The "ConfirmSetOfLargeVariable" test case of the Secure Boot Logo Test > >> ("Microsoft.UefiSecureBootLogo.Tests") suite in the Microsoft Hardware > >> Certification Kit sets a 32 KB large non-authenticated variable. > > > > According to > > http://www.uefi.org/sites/default/files/resources/UEFI_Plugfest_Security_Microsoft_Fall_2016.pdf > > > > "The maximum supported variable size must be at least 64kB" > > > > Should we just bump the size to match this? We should be able to make > > this change later once it is in a test/spec, but for some reason I > > thought the requirement was already 64k. > > The 32KB requirement comes from the most recent Secure Boot Logo Test. I If the limit is 32k, why go with 33k? Does the test fail with a 32k limit? -Jordan > installed both the Windows Server 2008 R2 SP1 test controller and the > Windows 2016 Server test client just the other day, together with the > most recent filters, using the following descriptions: > > https://msdn.microsoft.com/en-us/library/windows/hardware/jj123537.aspx > https://github.com/daynix/VirtHCK/wiki#Checklist_for_a_New_Controller_VM > https://github.com/daynix/VirtHCK/wiki#Checklist_for_a_New_Client_VM > > Given that this limit can be bumped without breaking compatibility, as > you say, I'd like to remain frugal with it, same as we were in James's > commit f5404a3eba1d ("OvmfPkg: Increase the maximum size for > Authenticated variables", 2016-03-24). > > I don't understand why the plugfest presentation and the SB Logo Test > require different limits... But, I'm certain our QE will find out in > short order once the SB Logo Test catches up with the presentation, and > I expect I'll submit the corresponding patch soon after. > > I dislike the speculation in this series, but breaking compatibility is > even worse. (A lot worse, to me at least.) So I consider the varstore > restructuring the smaller of two wrongs. However, wrt. > PcdMaxVariableSize, it seems we're not being forced to either of those > wrongs (i.e., breaking compat or speculation), so we can delay the increase. > > > > > Aside from this question: > > > > Series Reviewed-by: Jordan Justen <jordan.l.justen@intel.com> > > Thanks a lot! > > I'll await your ACK for the above argument before pushing the series. > > Thanks, > Laszlo > > >> In the FD_SIZE_4MB build, our live varstore is now 256 KB big, so we can > >> accommodate this. Set both PcdMaxVariableSize and PcdMaxAuthVariableSize > >> to 0x8400 -- beyond DataSize=0x8000 from the HCK test, we need some room > >> for the variable name and attributes as well. > >> > >> Cc: Gary Ching-Pang Lin <glin@suse.com> > >> Cc: Jordan Justen <jordan.l.justen@intel.com> > >> Contributed-under: TianoCore Contribution Agreement 1.0 > >> Signed-off-by: Laszlo Ersek <lersek@redhat.com> > >> --- > >> > >> Notes: > >> v2: > >> - adjust to FD_SIZE_IN_KB > >> - update commit msg to state 256 KB for the varstore [Jordan] > >> > >> OvmfPkg/OvmfPkgIa32.dsc | 6 ++++++ > >> OvmfPkg/OvmfPkgIa32X64.dsc | 6 ++++++ > >> OvmfPkg/OvmfPkgX64.dsc | 6 ++++++ > >> 3 files changed, 18 insertions(+) > >> > >> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc > >> index 26b807dde9fa..e0779ddaa426 100644 > >> --- a/OvmfPkg/OvmfPkgIa32.dsc > >> +++ b/OvmfPkg/OvmfPkgIa32.dsc > >> @@ -404,28 +404,34 @@ [PcdsFeatureFlag] > >> gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE > >> !endif > >> !if $(SMM_REQUIRE) == TRUE > >> gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE > >> gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE > >> !endif > >> > >> [PcdsFixedAtBuild] > >> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1 > >> gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE > >> gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10 > >> gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6 > >> gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32 > >> +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) > >> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000 > >> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800 > >> +!endif > >> +!if $(FD_SIZE_IN_KB) == 4096 > >> + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400 > >> + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400 > >> +!endif > >> gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000 > >> > >> gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0 > >> > >> gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07 > >> > >> # DEBUG_INIT 0x00000001 // Initialization > >> # DEBUG_WARN 0x00000002 // Warnings > >> # DEBUG_LOAD 0x00000004 // Load events > >> # DEBUG_FS 0x00000008 // EFI File system > >> # DEBUG_POOL 0x00000010 // Alloc & Free (pool) > >> # DEBUG_PAGE 0x00000020 // Alloc & Free (page) > >> # DEBUG_INFO 0x00000040 // Informational debug messages > >> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc > >> index 41f06a6b6a66..bbe26e2cf452 100644 > >> --- a/OvmfPkg/OvmfPkgIa32X64.dsc > >> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc > >> @@ -409,28 +409,34 @@ [PcdsFeatureFlag] > >> gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE > >> !endif > >> !if $(SMM_REQUIRE) == TRUE > >> gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE > >> gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE > >> !endif > >> > >> [PcdsFixedAtBuild] > >> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1 > >> gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE > >> gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10 > >> gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6 > >> gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32 > >> +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) > >> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000 > >> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800 > >> +!endif > >> +!if $(FD_SIZE_IN_KB) == 4096 > >> + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400 > >> + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400 > >> +!endif > >> gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000 > >> > >> gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0 > >> > >> gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07 > >> > >> # DEBUG_INIT 0x00000001 // Initialization > >> # DEBUG_WARN 0x00000002 // Warnings > >> # DEBUG_LOAD 0x00000004 // Load events > >> # DEBUG_FS 0x00000008 // EFI File system > >> # DEBUG_POOL 0x00000010 // Alloc & Free (pool) > >> # DEBUG_PAGE 0x00000020 // Alloc & Free (page) > >> # DEBUG_INFO 0x00000040 // Informational debug messages > >> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc > >> index 053c84b685c5..ff795815f65f 100644 > >> --- a/OvmfPkg/OvmfPkgX64.dsc > >> +++ b/OvmfPkg/OvmfPkgX64.dsc > >> @@ -409,28 +409,34 @@ [PcdsFeatureFlag] > >> gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE > >> !endif > >> !if $(SMM_REQUIRE) == TRUE > >> gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE > >> gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE > >> !endif > >> > >> [PcdsFixedAtBuild] > >> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1 > >> gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE > >> gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10 > >> gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6 > >> gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32 > >> +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) > >> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000 > >> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800 > >> +!endif > >> +!if $(FD_SIZE_IN_KB) == 4096 > >> + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400 > >> + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400 > >> +!endif > >> gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000 > >> > >> gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0 > >> > >> gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07 > >> > >> # DEBUG_INIT 0x00000001 // Initialization > >> # DEBUG_WARN 0x00000002 // Warnings > >> # DEBUG_LOAD 0x00000004 // Load events > >> # DEBUG_FS 0x00000008 // EFI File system > >> # DEBUG_POOL 0x00000010 // Alloc & Free (pool) > >> # DEBUG_PAGE 0x00000020 // Alloc & Free (page) > >> # DEBUG_INFO 0x00000040 // Informational debug messages > >> -- > >> 2.9.3 > >> > >> > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 4/5] OvmfPkg: raise max variable size (auth & non-auth) to 33KB for FD_SIZE_4MB 2017-05-04 18:50 ` Jordan Justen @ 2017-05-04 19:27 ` Laszlo Ersek 2017-05-04 19:30 ` Laszlo Ersek 0 siblings, 1 reply; 17+ messages in thread From: Laszlo Ersek @ 2017-05-04 19:27 UTC (permalink / raw) To: Jordan Justen, edk2-devel-01; +Cc: Gary Ching-Pang Lin On 05/04/17 20:50, Jordan Justen wrote: > On 2017-05-04 09:52:48, Laszlo Ersek wrote: >> On 05/04/17 18:36, Jordan Justen wrote: >>> On 2017-05-03 14:39:46, Laszlo Ersek wrote: >>>> The "ConfirmSetOfLargeVariable" test case of the Secure Boot Logo Test >>>> ("Microsoft.UefiSecureBootLogo.Tests") suite in the Microsoft Hardware >>>> Certification Kit sets a 32 KB large non-authenticated variable. >>> >>> According to >>> http://www.uefi.org/sites/default/files/resources/UEFI_Plugfest_Security_Microsoft_Fall_2016.pdf >>> >>> "The maximum supported variable size must be at least 64kB" >>> >>> Should we just bump the size to match this? We should be able to make >>> this change later once it is in a test/spec, but for some reason I >>> thought the requirement was already 64k. >> >> The 32KB requirement comes from the most recent Secure Boot Logo Test. I > > If the limit is 32k, why go with 33k? Does the test fail with a 32k > limit? Yes, it does. First I tried setting the PCDs to 0x8000 sharp, and then the test failed. Then I remembered that the PCDs present a limit on data, name, and attributes together: (1) In "MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c", function InitNonVolatileVariableStore(), we stash the PCDs like this: > mVariableModuleGlobal->MaxVariableSize = PcdGet32 (PcdMaxVariableSize); > mVariableModuleGlobal->MaxAuthVariableSize = ((PcdGet32 (PcdMaxAuthVariableSize) != 0) ? PcdGet32 (PcdMaxAuthVariableSize) : mVariableModuleGlobal->MaxVariableSize); (2) In the same file, function VariableServiceSetVariable(), we have the following size checks: > // > // The size of the VariableName, including the Unicode Null in bytes plus > // the DataSize is limited to maximum size of PcdGet32 (PcdMaxHardwareErrorVariableSize) > // bytes for HwErrRec#### variable. > // > if ((Attributes & EFI_VARIABLE_HARDWARE_ERROR_RECORD) == EFI_VARIABLE_HARDWARE_ERROR_RECORD) { > if (StrSize (VariableName) + PayloadSize > PcdGet32 (PcdMaxHardwareErrorVariableSize) - GetVariableHeaderSize ()) { > return EFI_INVALID_PARAMETER; > } > } else { > // > // The size of the VariableName, including the Unicode Null in bytes plus > // the DataSize is limited to maximum size of Max(Auth)VariableSize bytes. > // > if ((Attributes & VARIABLE_ATTRIBUTE_AT_AW) != 0) { > if (StrSize (VariableName) + PayloadSize > mVariableModuleGlobal->MaxAuthVariableSize - GetVariableHeaderSize ()) { > return EFI_INVALID_PARAMETER; > } > } else { > if (StrSize (VariableName) + PayloadSize > mVariableModuleGlobal->MaxVariableSize - GetVariableHeaderSize ()) { > return EFI_INVALID_PARAMETER; > } > } > } That is, the PCDs limit the sum of: - the variable name (including the terminating L'\0'), - the variable contents, - the internal header (which carries the attributes as well). In our case that's AUTHENTICATED_VARIABLE_HEADER, because we always format the auth varstore GUID into the template. Furthermore, the 32KB data size for the contents is not based on "reverse engineering"; the Secure Boot Logo Test tells you exactly what it is trying to do. (In fact, there is no other way to fix failures in any of the tests; you can only rely on the messages in order to see what the test attempted.) In this case, we have (see <https://bugzilla.redhat.com/show_bug.cgi?id=1443351#c27>): > NOTE: if this testcase fails, try restarting the machine and then > running this test case on its own, for example: "te.exe > UefiSecureBootLogo.dll > /name:Microsoft.UefiSecureBootLogo.Tests.ConfirmSetOfLargeVariable" > > Test creation of 1 large variable of size 32768 bytes. > > Test creation of testLargeVariable Unexpected Error - SetVariable > failed with status 0xC000000D VariableName "testLargeVariable" > VendorGuid 77fa9abd-0359-4d32-bd60-28f4e78f784b Attributes 0x00000007 > DataSize 0x00008000 It reports all the parameters of the gRT->SetVariable() call that failed, except "Data". In the successful test (with this patch applied), the messages are (see <https://bugzilla.redhat.com/attachment.cgi?id=1275172>): > NOTE: if this testcase fails, try restarting the machine and then > running this test case on its own, for example: "te.exe > UefiSecureBootLogo.dll > /name:Microsoft.UefiSecureBootLogo.Tests.ConfirmSetOfLargeVariable" > > Test creation of 1 large variable of size 32768 bytes. > > AreEqual(7, 7) - testLargeVariable: Attributes from GetVariable() > match SetVariable() > > AreEqual(32768, 32768) - testLargeVariable: DataSize from > GetVariable() matches SetVariable() Thanks Laszlo > >> installed both the Windows Server 2008 R2 SP1 test controller and the >> Windows 2016 Server test client just the other day, together with the >> most recent filters, using the following descriptions: >> >> https://msdn.microsoft.com/en-us/library/windows/hardware/jj123537.aspx >> https://github.com/daynix/VirtHCK/wiki#Checklist_for_a_New_Controller_VM >> https://github.com/daynix/VirtHCK/wiki#Checklist_for_a_New_Client_VM >> >> Given that this limit can be bumped without breaking compatibility, as >> you say, I'd like to remain frugal with it, same as we were in James's >> commit f5404a3eba1d ("OvmfPkg: Increase the maximum size for >> Authenticated variables", 2016-03-24). >> >> I don't understand why the plugfest presentation and the SB Logo Test >> require different limits... But, I'm certain our QE will find out in >> short order once the SB Logo Test catches up with the presentation, and >> I expect I'll submit the corresponding patch soon after. >> >> I dislike the speculation in this series, but breaking compatibility is >> even worse. (A lot worse, to me at least.) So I consider the varstore >> restructuring the smaller of two wrongs. However, wrt. >> PcdMaxVariableSize, it seems we're not being forced to either of those >> wrongs (i.e., breaking compat or speculation), so we can delay the increase. >> >>> >>> Aside from this question: >>> >>> Series Reviewed-by: Jordan Justen <jordan.l.justen@intel.com> >> >> Thanks a lot! >> >> I'll await your ACK for the above argument before pushing the series. >> >> Thanks, >> Laszlo >> >>>> In the FD_SIZE_4MB build, our live varstore is now 256 KB big, so we can >>>> accommodate this. Set both PcdMaxVariableSize and PcdMaxAuthVariableSize >>>> to 0x8400 -- beyond DataSize=0x8000 from the HCK test, we need some room >>>> for the variable name and attributes as well. >>>> >>>> Cc: Gary Ching-Pang Lin <glin@suse.com> >>>> Cc: Jordan Justen <jordan.l.justen@intel.com> >>>> Contributed-under: TianoCore Contribution Agreement 1.0 >>>> Signed-off-by: Laszlo Ersek <lersek@redhat.com> >>>> --- >>>> >>>> Notes: >>>> v2: >>>> - adjust to FD_SIZE_IN_KB >>>> - update commit msg to state 256 KB for the varstore [Jordan] >>>> >>>> OvmfPkg/OvmfPkgIa32.dsc | 6 ++++++ >>>> OvmfPkg/OvmfPkgIa32X64.dsc | 6 ++++++ >>>> OvmfPkg/OvmfPkgX64.dsc | 6 ++++++ >>>> 3 files changed, 18 insertions(+) >>>> >>>> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc >>>> index 26b807dde9fa..e0779ddaa426 100644 >>>> --- a/OvmfPkg/OvmfPkgIa32.dsc >>>> +++ b/OvmfPkg/OvmfPkgIa32.dsc >>>> @@ -404,28 +404,34 @@ [PcdsFeatureFlag] >>>> gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE >>>> !endif >>>> !if $(SMM_REQUIRE) == TRUE >>>> gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE >>>> gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE >>>> !endif >>>> >>>> [PcdsFixedAtBuild] >>>> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1 >>>> gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE >>>> gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10 >>>> gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6 >>>> gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32 >>>> +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) >>>> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000 >>>> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800 >>>> +!endif >>>> +!if $(FD_SIZE_IN_KB) == 4096 >>>> + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400 >>>> + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400 >>>> +!endif >>>> gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000 >>>> >>>> gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0 >>>> >>>> gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07 >>>> >>>> # DEBUG_INIT 0x00000001 // Initialization >>>> # DEBUG_WARN 0x00000002 // Warnings >>>> # DEBUG_LOAD 0x00000004 // Load events >>>> # DEBUG_FS 0x00000008 // EFI File system >>>> # DEBUG_POOL 0x00000010 // Alloc & Free (pool) >>>> # DEBUG_PAGE 0x00000020 // Alloc & Free (page) >>>> # DEBUG_INFO 0x00000040 // Informational debug messages >>>> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc >>>> index 41f06a6b6a66..bbe26e2cf452 100644 >>>> --- a/OvmfPkg/OvmfPkgIa32X64.dsc >>>> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc >>>> @@ -409,28 +409,34 @@ [PcdsFeatureFlag] >>>> gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE >>>> !endif >>>> !if $(SMM_REQUIRE) == TRUE >>>> gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE >>>> gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE >>>> !endif >>>> >>>> [PcdsFixedAtBuild] >>>> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1 >>>> gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE >>>> gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10 >>>> gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6 >>>> gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32 >>>> +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) >>>> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000 >>>> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800 >>>> +!endif >>>> +!if $(FD_SIZE_IN_KB) == 4096 >>>> + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400 >>>> + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400 >>>> +!endif >>>> gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000 >>>> >>>> gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0 >>>> >>>> gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07 >>>> >>>> # DEBUG_INIT 0x00000001 // Initialization >>>> # DEBUG_WARN 0x00000002 // Warnings >>>> # DEBUG_LOAD 0x00000004 // Load events >>>> # DEBUG_FS 0x00000008 // EFI File system >>>> # DEBUG_POOL 0x00000010 // Alloc & Free (pool) >>>> # DEBUG_PAGE 0x00000020 // Alloc & Free (page) >>>> # DEBUG_INFO 0x00000040 // Informational debug messages >>>> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc >>>> index 053c84b685c5..ff795815f65f 100644 >>>> --- a/OvmfPkg/OvmfPkgX64.dsc >>>> +++ b/OvmfPkg/OvmfPkgX64.dsc >>>> @@ -409,28 +409,34 @@ [PcdsFeatureFlag] >>>> gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE >>>> !endif >>>> !if $(SMM_REQUIRE) == TRUE >>>> gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE >>>> gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE >>>> !endif >>>> >>>> [PcdsFixedAtBuild] >>>> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1 >>>> gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE >>>> gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10 >>>> gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6 >>>> gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32 >>>> +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) >>>> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000 >>>> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800 >>>> +!endif >>>> +!if $(FD_SIZE_IN_KB) == 4096 >>>> + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400 >>>> + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400 >>>> +!endif >>>> gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000 >>>> >>>> gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0 >>>> >>>> gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07 >>>> >>>> # DEBUG_INIT 0x00000001 // Initialization >>>> # DEBUG_WARN 0x00000002 // Warnings >>>> # DEBUG_LOAD 0x00000004 // Load events >>>> # DEBUG_FS 0x00000008 // EFI File system >>>> # DEBUG_POOL 0x00000010 // Alloc & Free (pool) >>>> # DEBUG_PAGE 0x00000020 // Alloc & Free (page) >>>> # DEBUG_INFO 0x00000040 // Informational debug messages >>>> -- >>>> 2.9.3 >>>> >>>> >> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 4/5] OvmfPkg: raise max variable size (auth & non-auth) to 33KB for FD_SIZE_4MB 2017-05-04 19:27 ` Laszlo Ersek @ 2017-05-04 19:30 ` Laszlo Ersek 0 siblings, 0 replies; 17+ messages in thread From: Laszlo Ersek @ 2017-05-04 19:30 UTC (permalink / raw) To: Jordan Justen, edk2-devel-01; +Cc: Gary Ching-Pang Lin On 05/04/17 21:27, Laszlo Ersek wrote: > On 05/04/17 20:50, Jordan Justen wrote: >> On 2017-05-04 09:52:48, Laszlo Ersek wrote: >>> On 05/04/17 18:36, Jordan Justen wrote: >>>> On 2017-05-03 14:39:46, Laszlo Ersek wrote: >>>>> The "ConfirmSetOfLargeVariable" test case of the Secure Boot Logo Test >>>>> ("Microsoft.UefiSecureBootLogo.Tests") suite in the Microsoft Hardware >>>>> Certification Kit sets a 32 KB large non-authenticated variable. >>>> >>>> According to >>>> http://www.uefi.org/sites/default/files/resources/UEFI_Plugfest_Security_Microsoft_Fall_2016.pdf >>>> >>>> "The maximum supported variable size must be at least 64kB" >>>> >>>> Should we just bump the size to match this? We should be able to make >>>> this change later once it is in a test/spec, but for some reason I >>>> thought the requirement was already 64k. >>> >>> The 32KB requirement comes from the most recent Secure Boot Logo Test. I >> >> If the limit is 32k, why go with 33k? Does the test fail with a 32k >> limit? > > Yes, it does. First I tried setting the PCDs to 0x8000 sharp, and then > the test failed. > > Then I remembered that the PCDs present a limit on data, name, and > attributes together: > > (1) In "MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c", function > InitNonVolatileVariableStore(), we stash the PCDs like this: > >> mVariableModuleGlobal->MaxVariableSize = PcdGet32 (PcdMaxVariableSize); >> mVariableModuleGlobal->MaxAuthVariableSize = ((PcdGet32 (PcdMaxAuthVariableSize) != 0) ? PcdGet32 (PcdMaxAuthVariableSize) : mVariableModuleGlobal->MaxVariableSize); > > (2) In the same file, function VariableServiceSetVariable(), we have the > following size checks: > >> // >> // The size of the VariableName, including the Unicode Null in bytes plus >> // the DataSize is limited to maximum size of PcdGet32 (PcdMaxHardwareErrorVariableSize) >> // bytes for HwErrRec#### variable. >> // >> if ((Attributes & EFI_VARIABLE_HARDWARE_ERROR_RECORD) == EFI_VARIABLE_HARDWARE_ERROR_RECORD) { >> if (StrSize (VariableName) + PayloadSize > PcdGet32 (PcdMaxHardwareErrorVariableSize) - GetVariableHeaderSize ()) { >> return EFI_INVALID_PARAMETER; >> } >> } else { >> // >> // The size of the VariableName, including the Unicode Null in bytes plus >> // the DataSize is limited to maximum size of Max(Auth)VariableSize bytes. >> // >> if ((Attributes & VARIABLE_ATTRIBUTE_AT_AW) != 0) { >> if (StrSize (VariableName) + PayloadSize > mVariableModuleGlobal->MaxAuthVariableSize - GetVariableHeaderSize ()) { >> return EFI_INVALID_PARAMETER; >> } >> } else { >> if (StrSize (VariableName) + PayloadSize > mVariableModuleGlobal->MaxVariableSize - GetVariableHeaderSize ()) { >> return EFI_INVALID_PARAMETER; >> } >> } >> } > > That is, the PCDs limit the sum of: > - the variable name (including the terminating L'\0'), > - the variable contents, > - the internal header (which carries the attributes as well). In our > case that's AUTHENTICATED_VARIABLE_HEADER, because we always format > the auth varstore GUID into the template. (NB I mentioned this in the commit message, just not in such detail.) Thanks Laszlo > > Furthermore, the 32KB data size for the contents is not based on > "reverse engineering"; the Secure Boot Logo Test tells you exactly what > it is trying to do. (In fact, there is no other way to fix failures in > any of the tests; you can only rely on the messages in order to see what > the test attempted.) > > In this case, we have (see > <https://bugzilla.redhat.com/show_bug.cgi?id=1443351#c27>): > >> NOTE: if this testcase fails, try restarting the machine and then >> running this test case on its own, for example: "te.exe >> UefiSecureBootLogo.dll >> /name:Microsoft.UefiSecureBootLogo.Tests.ConfirmSetOfLargeVariable" >> >> Test creation of 1 large variable of size 32768 bytes. >> >> Test creation of testLargeVariable Unexpected Error - SetVariable >> failed with status 0xC000000D VariableName "testLargeVariable" >> VendorGuid 77fa9abd-0359-4d32-bd60-28f4e78f784b Attributes 0x00000007 >> DataSize 0x00008000 > > It reports all the parameters of the gRT->SetVariable() call that > failed, except "Data". > > In the successful test (with this patch applied), the messages are (see > <https://bugzilla.redhat.com/attachment.cgi?id=1275172>): > >> NOTE: if this testcase fails, try restarting the machine and then >> running this test case on its own, for example: "te.exe >> UefiSecureBootLogo.dll >> /name:Microsoft.UefiSecureBootLogo.Tests.ConfirmSetOfLargeVariable" >> >> Test creation of 1 large variable of size 32768 bytes. >> >> AreEqual(7, 7) - testLargeVariable: Attributes from GetVariable() >> match SetVariable() >> >> AreEqual(32768, 32768) - testLargeVariable: DataSize from >> GetVariable() matches SetVariable() > > Thanks > Laszlo > >> >>> installed both the Windows Server 2008 R2 SP1 test controller and the >>> Windows 2016 Server test client just the other day, together with the >>> most recent filters, using the following descriptions: >>> >>> https://msdn.microsoft.com/en-us/library/windows/hardware/jj123537.aspx >>> https://github.com/daynix/VirtHCK/wiki#Checklist_for_a_New_Controller_VM >>> https://github.com/daynix/VirtHCK/wiki#Checklist_for_a_New_Client_VM >>> >>> Given that this limit can be bumped without breaking compatibility, as >>> you say, I'd like to remain frugal with it, same as we were in James's >>> commit f5404a3eba1d ("OvmfPkg: Increase the maximum size for >>> Authenticated variables", 2016-03-24). >>> >>> I don't understand why the plugfest presentation and the SB Logo Test >>> require different limits... But, I'm certain our QE will find out in >>> short order once the SB Logo Test catches up with the presentation, and >>> I expect I'll submit the corresponding patch soon after. >>> >>> I dislike the speculation in this series, but breaking compatibility is >>> even worse. (A lot worse, to me at least.) So I consider the varstore >>> restructuring the smaller of two wrongs. However, wrt. >>> PcdMaxVariableSize, it seems we're not being forced to either of those >>> wrongs (i.e., breaking compat or speculation), so we can delay the increase. >>> >>>> >>>> Aside from this question: >>>> >>>> Series Reviewed-by: Jordan Justen <jordan.l.justen@intel.com> >>> >>> Thanks a lot! >>> >>> I'll await your ACK for the above argument before pushing the series. >>> >>> Thanks, >>> Laszlo >>> >>>>> In the FD_SIZE_4MB build, our live varstore is now 256 KB big, so we can >>>>> accommodate this. Set both PcdMaxVariableSize and PcdMaxAuthVariableSize >>>>> to 0x8400 -- beyond DataSize=0x8000 from the HCK test, we need some room >>>>> for the variable name and attributes as well. >>>>> >>>>> Cc: Gary Ching-Pang Lin <glin@suse.com> >>>>> Cc: Jordan Justen <jordan.l.justen@intel.com> >>>>> Contributed-under: TianoCore Contribution Agreement 1.0 >>>>> Signed-off-by: Laszlo Ersek <lersek@redhat.com> >>>>> --- >>>>> >>>>> Notes: >>>>> v2: >>>>> - adjust to FD_SIZE_IN_KB >>>>> - update commit msg to state 256 KB for the varstore [Jordan] >>>>> >>>>> OvmfPkg/OvmfPkgIa32.dsc | 6 ++++++ >>>>> OvmfPkg/OvmfPkgIa32X64.dsc | 6 ++++++ >>>>> OvmfPkg/OvmfPkgX64.dsc | 6 ++++++ >>>>> 3 files changed, 18 insertions(+) >>>>> >>>>> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc >>>>> index 26b807dde9fa..e0779ddaa426 100644 >>>>> --- a/OvmfPkg/OvmfPkgIa32.dsc >>>>> +++ b/OvmfPkg/OvmfPkgIa32.dsc >>>>> @@ -404,28 +404,34 @@ [PcdsFeatureFlag] >>>>> gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE >>>>> !endif >>>>> !if $(SMM_REQUIRE) == TRUE >>>>> gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE >>>>> gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE >>>>> !endif >>>>> >>>>> [PcdsFixedAtBuild] >>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1 >>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE >>>>> gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10 >>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6 >>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32 >>>>> +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) >>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000 >>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800 >>>>> +!endif >>>>> +!if $(FD_SIZE_IN_KB) == 4096 >>>>> + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400 >>>>> + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400 >>>>> +!endif >>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000 >>>>> >>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0 >>>>> >>>>> gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07 >>>>> >>>>> # DEBUG_INIT 0x00000001 // Initialization >>>>> # DEBUG_WARN 0x00000002 // Warnings >>>>> # DEBUG_LOAD 0x00000004 // Load events >>>>> # DEBUG_FS 0x00000008 // EFI File system >>>>> # DEBUG_POOL 0x00000010 // Alloc & Free (pool) >>>>> # DEBUG_PAGE 0x00000020 // Alloc & Free (page) >>>>> # DEBUG_INFO 0x00000040 // Informational debug messages >>>>> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc >>>>> index 41f06a6b6a66..bbe26e2cf452 100644 >>>>> --- a/OvmfPkg/OvmfPkgIa32X64.dsc >>>>> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc >>>>> @@ -409,28 +409,34 @@ [PcdsFeatureFlag] >>>>> gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE >>>>> !endif >>>>> !if $(SMM_REQUIRE) == TRUE >>>>> gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE >>>>> gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE >>>>> !endif >>>>> >>>>> [PcdsFixedAtBuild] >>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1 >>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE >>>>> gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10 >>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6 >>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32 >>>>> +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) >>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000 >>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800 >>>>> +!endif >>>>> +!if $(FD_SIZE_IN_KB) == 4096 >>>>> + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400 >>>>> + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400 >>>>> +!endif >>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000 >>>>> >>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0 >>>>> >>>>> gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07 >>>>> >>>>> # DEBUG_INIT 0x00000001 // Initialization >>>>> # DEBUG_WARN 0x00000002 // Warnings >>>>> # DEBUG_LOAD 0x00000004 // Load events >>>>> # DEBUG_FS 0x00000008 // EFI File system >>>>> # DEBUG_POOL 0x00000010 // Alloc & Free (pool) >>>>> # DEBUG_PAGE 0x00000020 // Alloc & Free (page) >>>>> # DEBUG_INFO 0x00000040 // Informational debug messages >>>>> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc >>>>> index 053c84b685c5..ff795815f65f 100644 >>>>> --- a/OvmfPkg/OvmfPkgX64.dsc >>>>> +++ b/OvmfPkg/OvmfPkgX64.dsc >>>>> @@ -409,28 +409,34 @@ [PcdsFeatureFlag] >>>>> gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE >>>>> !endif >>>>> !if $(SMM_REQUIRE) == TRUE >>>>> gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE >>>>> gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE >>>>> !endif >>>>> >>>>> [PcdsFixedAtBuild] >>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1 >>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE >>>>> gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10 >>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6 >>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32 >>>>> +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) >>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000 >>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800 >>>>> +!endif >>>>> +!if $(FD_SIZE_IN_KB) == 4096 >>>>> + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400 >>>>> + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400 >>>>> +!endif >>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000 >>>>> >>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0 >>>>> >>>>> gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07 >>>>> >>>>> # DEBUG_INIT 0x00000001 // Initialization >>>>> # DEBUG_WARN 0x00000002 // Warnings >>>>> # DEBUG_LOAD 0x00000004 // Load events >>>>> # DEBUG_FS 0x00000008 // EFI File system >>>>> # DEBUG_POOL 0x00000010 // Alloc & Free (pool) >>>>> # DEBUG_PAGE 0x00000020 // Alloc & Free (page) >>>>> # DEBUG_INFO 0x00000040 // Informational debug messages >>>>> -- >>>>> 2.9.3 >>>>> >>>>> >>> > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 4/5] OvmfPkg: raise max variable size (auth & non-auth) to 33KB for FD_SIZE_4MB 2017-05-04 16:52 ` Laszlo Ersek 2017-05-04 18:50 ` Jordan Justen @ 2017-05-04 23:00 ` Laszlo Ersek 1 sibling, 0 replies; 17+ messages in thread From: Laszlo Ersek @ 2017-05-04 23:00 UTC (permalink / raw) To: edk2-devel-01; +Cc: Jordan Justen, Gary Ching-Pang Lin On 05/04/17 18:52, Laszlo Ersek wrote: > On 05/04/17 18:36, Jordan Justen wrote: >> On 2017-05-03 14:39:46, Laszlo Ersek wrote: >>> The "ConfirmSetOfLargeVariable" test case of the Secure Boot Logo Test >>> ("Microsoft.UefiSecureBootLogo.Tests") suite in the Microsoft Hardware >>> Certification Kit sets a 32 KB large non-authenticated variable. >> >> According to >> http://www.uefi.org/sites/default/files/resources/UEFI_Plugfest_Security_Microsoft_Fall_2016.pdf >> >> "The maximum supported variable size must be at least 64kB" >> >> Should we just bump the size to match this? We should be able to make >> this change later once it is in a test/spec, but for some reason I >> thought the requirement was already 64k. > > The 32KB requirement comes from the most recent Secure Boot Logo Test. I > installed both the Windows Server 2008 R2 SP1 test controller and the > Windows 2016 Server test client just the other day, together with the > most recent filters, using the following descriptions: > > https://msdn.microsoft.com/en-us/library/windows/hardware/jj123537.aspx > https://github.com/daynix/VirtHCK/wiki#Checklist_for_a_New_Controller_VM > https://github.com/daynix/VirtHCK/wiki#Checklist_for_a_New_Client_VM > > Given that this limit can be bumped without breaking compatibility, as > you say, I'd like to remain frugal with it, same as we were in James's > commit f5404a3eba1d ("OvmfPkg: Increase the maximum size for > Authenticated variables", 2016-03-24). > > I don't understand why the plugfest presentation and the SB Logo Test > require different limits... But, I'm certain our QE will find out in > short order once the SB Logo Test catches up with the presentation, and > I expect I'll submit the corresponding patch soon after. > > I dislike the speculation in this series, but breaking compatibility is > even worse. (A lot worse, to me at least.) So I consider the varstore > restructuring the smaller of two wrongs. However, wrt. > PcdMaxVariableSize, it seems we're not being forced to either of those > wrongs (i.e., breaking compat or speculation), so we can delay the increase. > >> >> Aside from this question: >> >> Series Reviewed-by: Jordan Justen <jordan.l.justen@intel.com> > > Thanks a lot! > > I'll await your ACK for the above argument before pushing the series. Based on the ack I got from Jordan on IRC, I pushed the series. Commit range 4bf3b994e8b2..bba8dfbec3bb. Thanks! Laszlo >>> In the FD_SIZE_4MB build, our live varstore is now 256 KB big, so we can >>> accommodate this. Set both PcdMaxVariableSize and PcdMaxAuthVariableSize >>> to 0x8400 -- beyond DataSize=0x8000 from the HCK test, we need some room >>> for the variable name and attributes as well. >>> >>> Cc: Gary Ching-Pang Lin <glin@suse.com> >>> Cc: Jordan Justen <jordan.l.justen@intel.com> >>> Contributed-under: TianoCore Contribution Agreement 1.0 >>> Signed-off-by: Laszlo Ersek <lersek@redhat.com> >>> --- >>> >>> Notes: >>> v2: >>> - adjust to FD_SIZE_IN_KB >>> - update commit msg to state 256 KB for the varstore [Jordan] >>> >>> OvmfPkg/OvmfPkgIa32.dsc | 6 ++++++ >>> OvmfPkg/OvmfPkgIa32X64.dsc | 6 ++++++ >>> OvmfPkg/OvmfPkgX64.dsc | 6 ++++++ >>> 3 files changed, 18 insertions(+) >>> >>> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc >>> index 26b807dde9fa..e0779ddaa426 100644 >>> --- a/OvmfPkg/OvmfPkgIa32.dsc >>> +++ b/OvmfPkg/OvmfPkgIa32.dsc >>> @@ -404,28 +404,34 @@ [PcdsFeatureFlag] >>> gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE >>> !endif >>> !if $(SMM_REQUIRE) == TRUE >>> gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE >>> gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE >>> !endif >>> >>> [PcdsFixedAtBuild] >>> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1 >>> gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE >>> gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10 >>> gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6 >>> gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32 >>> +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) >>> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000 >>> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800 >>> +!endif >>> +!if $(FD_SIZE_IN_KB) == 4096 >>> + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400 >>> + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400 >>> +!endif >>> gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000 >>> >>> gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0 >>> >>> gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07 >>> >>> # DEBUG_INIT 0x00000001 // Initialization >>> # DEBUG_WARN 0x00000002 // Warnings >>> # DEBUG_LOAD 0x00000004 // Load events >>> # DEBUG_FS 0x00000008 // EFI File system >>> # DEBUG_POOL 0x00000010 // Alloc & Free (pool) >>> # DEBUG_PAGE 0x00000020 // Alloc & Free (page) >>> # DEBUG_INFO 0x00000040 // Informational debug messages >>> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc >>> index 41f06a6b6a66..bbe26e2cf452 100644 >>> --- a/OvmfPkg/OvmfPkgIa32X64.dsc >>> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc >>> @@ -409,28 +409,34 @@ [PcdsFeatureFlag] >>> gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE >>> !endif >>> !if $(SMM_REQUIRE) == TRUE >>> gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE >>> gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE >>> !endif >>> >>> [PcdsFixedAtBuild] >>> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1 >>> gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE >>> gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10 >>> gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6 >>> gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32 >>> +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) >>> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000 >>> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800 >>> +!endif >>> +!if $(FD_SIZE_IN_KB) == 4096 >>> + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400 >>> + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400 >>> +!endif >>> gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000 >>> >>> gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0 >>> >>> gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07 >>> >>> # DEBUG_INIT 0x00000001 // Initialization >>> # DEBUG_WARN 0x00000002 // Warnings >>> # DEBUG_LOAD 0x00000004 // Load events >>> # DEBUG_FS 0x00000008 // EFI File system >>> # DEBUG_POOL 0x00000010 // Alloc & Free (pool) >>> # DEBUG_PAGE 0x00000020 // Alloc & Free (page) >>> # DEBUG_INFO 0x00000040 // Informational debug messages >>> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc >>> index 053c84b685c5..ff795815f65f 100644 >>> --- a/OvmfPkg/OvmfPkgX64.dsc >>> +++ b/OvmfPkg/OvmfPkgX64.dsc >>> @@ -409,28 +409,34 @@ [PcdsFeatureFlag] >>> gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE >>> !endif >>> !if $(SMM_REQUIRE) == TRUE >>> gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE >>> gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE >>> !endif >>> >>> [PcdsFixedAtBuild] >>> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1 >>> gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE >>> gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10 >>> gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6 >>> gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32 >>> +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) >>> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000 >>> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800 >>> +!endif >>> +!if $(FD_SIZE_IN_KB) == 4096 >>> + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400 >>> + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400 >>> +!endif >>> gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000 >>> >>> gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0 >>> >>> gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07 >>> >>> # DEBUG_INIT 0x00000001 // Initialization >>> # DEBUG_WARN 0x00000002 // Warnings >>> # DEBUG_LOAD 0x00000004 // Load events >>> # DEBUG_FS 0x00000008 // EFI File system >>> # DEBUG_POOL 0x00000010 // Alloc & Free (pool) >>> # DEBUG_PAGE 0x00000020 // Alloc & Free (page) >>> # DEBUG_INFO 0x00000040 // Informational debug messages >>> -- >>> 2.9.3 >>> >>> > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel > ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 5/5] OvmfPkg: make the 4MB flash size the default 2017-05-03 21:39 [PATCH v2 0/5] OvmfPkg: add FD_SIZE_4MB for Windows HCK SB tests, and for future proofing Laszlo Ersek ` (3 preceding siblings ...) 2017-05-03 21:39 ` [PATCH v2 4/5] OvmfPkg: raise max variable size (auth & non-auth) to 33KB for FD_SIZE_4MB Laszlo Ersek @ 2017-05-03 21:39 ` Laszlo Ersek 4 siblings, 0 replies; 17+ messages in thread From: Laszlo Ersek @ 2017-05-03 21:39 UTC (permalink / raw) To: edk2-devel-01; +Cc: Gary Ching-Pang Lin, Jordan Justen The previously default 2MB can be explicitly selected with -D FD_SIZE_2MB or -D FD_SIZE_IN_KB=2048 Cc: Gary Ching-Pang Lin <glin@suse.com> Cc: Jordan Justen <jordan.l.justen@intel.com> Suggested-by: Jordan Justen <jordan.l.justen@intel.com> Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- Notes: v2: - new in v2 [Jordan] OvmfPkg/OvmfPkgIa32.dsc | 2 +- OvmfPkg/OvmfPkgIa32X64.dsc | 2 +- OvmfPkg/OvmfPkgX64.dsc | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc index e0779ddaa426..e0ff7ead034e 100644 --- a/OvmfPkg/OvmfPkgIa32.dsc +++ b/OvmfPkg/OvmfPkgIa32.dsc @@ -44,27 +44,27 @@ [Defines] # Flash size selection. Setting FD_SIZE_IN_KB on the command line directly to # one of the supported values, in place of any of the convenience macros, is # permitted. # !ifdef $(FD_SIZE_1MB) DEFINE FD_SIZE_IN_KB = 1024 !else !ifdef $(FD_SIZE_2MB) DEFINE FD_SIZE_IN_KB = 2048 !else !ifdef $(FD_SIZE_4MB) DEFINE FD_SIZE_IN_KB = 4096 !else - DEFINE FD_SIZE_IN_KB = 2048 + DEFINE FD_SIZE_IN_KB = 4096 !endif !endif !endif [BuildOptions] GCC:*_UNIXGCC_*_CC_FLAGS = -DMDEPKG_NDEBUG GCC:RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG INTEL:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG MSFT:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG GCC:*_*_*_CC_FLAGS = -mno-mmx -mno-sse # # Disable deprecated APIs. diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc index bbe26e2cf452..41922f581c98 100644 --- a/OvmfPkg/OvmfPkgIa32X64.dsc +++ b/OvmfPkg/OvmfPkgIa32X64.dsc @@ -44,27 +44,27 @@ [Defines] # Flash size selection. Setting FD_SIZE_IN_KB on the command line directly to # one of the supported values, in place of any of the convenience macros, is # permitted. # !ifdef $(FD_SIZE_1MB) DEFINE FD_SIZE_IN_KB = 1024 !else !ifdef $(FD_SIZE_2MB) DEFINE FD_SIZE_IN_KB = 2048 !else !ifdef $(FD_SIZE_4MB) DEFINE FD_SIZE_IN_KB = 4096 !else - DEFINE FD_SIZE_IN_KB = 2048 + DEFINE FD_SIZE_IN_KB = 4096 !endif !endif !endif [BuildOptions] GCC:*_UNIXGCC_*_CC_FLAGS = -DMDEPKG_NDEBUG GCC:RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG INTEL:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG MSFT:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG GCC:*_*_*_CC_FLAGS = -mno-mmx -mno-sse !ifdef $(SOURCE_DEBUG_ENABLE) MSFT:*_*_X64_GENFW_FLAGS = --keepexceptiontable GCC:*_*_X64_GENFW_FLAGS = --keepexceptiontable diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc index ff795815f65f..9ba033956c88 100644 --- a/OvmfPkg/OvmfPkgX64.dsc +++ b/OvmfPkg/OvmfPkgX64.dsc @@ -44,27 +44,27 @@ [Defines] # Flash size selection. Setting FD_SIZE_IN_KB on the command line directly to # one of the supported values, in place of any of the convenience macros, is # permitted. # !ifdef $(FD_SIZE_1MB) DEFINE FD_SIZE_IN_KB = 1024 !else !ifdef $(FD_SIZE_2MB) DEFINE FD_SIZE_IN_KB = 2048 !else !ifdef $(FD_SIZE_4MB) DEFINE FD_SIZE_IN_KB = 4096 !else - DEFINE FD_SIZE_IN_KB = 2048 + DEFINE FD_SIZE_IN_KB = 4096 !endif !endif !endif [BuildOptions] GCC:*_UNIXGCC_*_CC_FLAGS = -DMDEPKG_NDEBUG GCC:RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG INTEL:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG MSFT:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG GCC:*_*_*_CC_FLAGS = -mno-mmx -mno-sse !ifdef $(SOURCE_DEBUG_ENABLE) MSFT:*_*_X64_GENFW_FLAGS = --keepexceptiontable GCC:*_*_X64_GENFW_FLAGS = --keepexceptiontable -- 2.9.3 ^ permalink raw reply related [flat|nested] 17+ messages in thread
end of thread, other threads:[~2017-05-05 16:46 UTC | newest] Thread overview: 17+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-05-03 21:39 [PATCH v2 0/5] OvmfPkg: add FD_SIZE_4MB for Windows HCK SB tests, and for future proofing Laszlo Ersek 2017-05-03 21:39 ` [PATCH v2 1/5] OvmfPkg: introduce the FD_SIZE_IN_KB macro / build flag Laszlo Ersek 2017-05-03 21:39 ` [PATCH v2 2/5] OvmfPkg/OvmfPkg.fdf.inc: extract VARS_LIVE_SIZE and VARS_SPARE_SIZE macros Laszlo Ersek 2017-05-03 21:39 ` [PATCH v2 3/5] OvmfPkg: introduce 4MB flash image (mainly) for Windows HCK Laszlo Ersek 2017-05-05 4:07 ` Laszlo Ersek 2017-05-05 8:57 ` Jordan Justen 2017-05-05 12:07 ` Laszlo Ersek 2017-05-05 15:43 ` Jordan Justen 2017-05-05 16:46 ` Laszlo Ersek 2017-05-03 21:39 ` [PATCH v2 4/5] OvmfPkg: raise max variable size (auth & non-auth) to 33KB for FD_SIZE_4MB Laszlo Ersek 2017-05-04 16:36 ` Jordan Justen 2017-05-04 16:52 ` Laszlo Ersek 2017-05-04 18:50 ` Jordan Justen 2017-05-04 19:27 ` Laszlo Ersek 2017-05-04 19:30 ` Laszlo Ersek 2017-05-04 23:00 ` Laszlo Ersek 2017-05-03 21:39 ` [PATCH v2 5/5] OvmfPkg: make the 4MB flash size the default Laszlo Ersek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox