From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 1186A21A6F106 for ; Sat, 29 Apr 2017 17:48:07 -0700 (PDT) Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga105.fm.intel.com with ESMTP; 29 Apr 2017 17:48:06 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.37,394,1488873600"; d="scan'208";a="82210535" Received: from pguermon-mobl3.ger.corp.intel.com (HELO localhost) ([10.255.77.18]) by orsmga004.jf.intel.com with ESMTP; 29 Apr 2017 17:48:06 -0700 MIME-Version: 1.0 To: Laszlo Ersek , edk2-devel-01 Message-ID: <149351328512.20670.1563878734495138189@jljusten-skl> From: Jordan Justen In-Reply-To: <20170429201500.18496-3-lersek@redhat.com> Cc: Gary Ching-Pang Lin References: <20170429201500.18496-1-lersek@redhat.com> <20170429201500.18496-3-lersek@redhat.com> User-Agent: alot/0.5.1 Date: Sat, 29 Apr 2017 17:48:05 -0700 Subject: Re: [PATCH 2/3] OvmfPkg: introduce FD_SIZE_4MB (mainly) for Windows HCK X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 30 Apr 2017 00:48:07 -0000 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable On 2017-04-29 13:14:59, 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, 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 512 KB, and within > it, the live area from 56 KB to 248 KB. > = > (For the latter, Hyper-V considers 128 KB generous: Today, we give 128k to non-volatile storage, which due to various reasons results in 56k of variable storage. Personally I already consider this generous, especially when I look at the 128k size of the seabios bios.bin. :) > Re: [edk2] Secure Boot & NV storage size > http://mid.mail-archive.com/24f63595e68c476eb98cf87e7abfa1bc@BL2PR03MB2= 42.namprd03.prod.outlook.com > = > so we should be larger than that, for future proofing.) I don't want to try to predict the future of Microsoft requirements, nor be overly generous in the meantime. :) As pointed out by a colleague, as of Fall 2016, Microsoft expects 120 KB as the current requirement: http://www.uefi.org/sites/default/files/resources/UEFI_Plugfest_Security_Mi= crosoft_Fall_2016.pdf I tested a 2MB RELEASE build of OvmfIa32X64 on GCC 4.9 with: * SECURE_BOOT_ENABLE=3D1 * SMM_REQUIRE=3D1 There was > 600k available. I also tested a 2MB RELEASE build of OvmfIa32X64 on GCC 4.9 with: * SECURE_BOOT_ENABLE=3D1 * NETWORK_IP6_ENABLE=3D1 * HTTP_BOOT_ENABLE=3D1 * SMM_REQUIRE=3D1 * TLS_ENABLE=3D1 I don't think we consider those network items as standard requirements, yet there was still ~373k available. In order to bump the variables to 120k, we need to use 2 * (120 - 56) =3D> 128k. So, what about adjusting our 2MB image to this? Regarding how to 'upgrade' a system from using the smaller storage size, I don't think there are any good answers. (Which also applies to the 4MB case.) Does fw-cfg tell us the rom/flash regions? -Jordan > 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 should 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 should occur 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 -> 512 ( +384) > data > Variable store 56 -> 248 ( +192) > Event log 4 -> 4 ( +0) > Working block 4 -> 4 ( +0) > Spare area 64 -> 256 ( +192) > = > FVMAIN_COMPACT uncompressed 1712 -> 3376 (+1664) > 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 > = > Cc: Gary Ching-Pang Lin > Cc: Jordan Justen > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Laszlo Ersek > --- > OvmfPkg/OvmfPkg.fdf.inc | 29 ++++++++++++ > OvmfPkg/VarStore.fdf.inc | 46 +++++++++++++++++++- > 2 files changed, 73 insertions(+), 2 deletions(-) > = > diff --git a/OvmfPkg/OvmfPkg.fdf.inc b/OvmfPkg/OvmfPkg.fdf.inc > index fce6c7b8ce64..a1eb2d380167 100644 > --- a/OvmfPkg/OvmfPkg.fdf.inc > +++ b/OvmfPkg/OvmfPkg.fdf.inc > @@ -20,8 +20,35 @@ > # > # Defining FD_SIZE_1MB on the build command line can override this. > # > +# A firmware binary built for the default 2MB flash size, and a firmware= binary > +# built with FD_SIZE_1MB defined, use the same variable store layout. > +# > +# Defining FD_SIZE_4MB results in a different (much larger) variable sto= re > +# structure that is incompatible with both the default and the FD_SIZE_1= MB > +# firmware binaries. > +# > = > DEFINE BLOCK_SIZE =3D 0x1000 > + > +!ifdef $(FD_SIZE_4MB) > + > +DEFINE VARS_SIZE =3D 0x80000 > +DEFINE VARS_BLOCKS =3D 0x80 > +DEFINE VARS_LIVE_SIZE =3D 0x3E000 > +DEFINE VARS_SPARE_SIZE =3D 0x40000 > + > +DEFINE FW_BASE_ADDRESS =3D 0xFFC00000 > +DEFINE FW_SIZE =3D 0x00400000 > +DEFINE FW_BLOCKS =3D 0x400 > +DEFINE CODE_BASE_ADDRESS =3D 0xFFC80000 > +DEFINE CODE_SIZE =3D 0x00380000 > +DEFINE CODE_BLOCKS =3D 0x380 > +DEFINE FVMAIN_SIZE =3D 0x0034C000 > +DEFINE SECFV_OFFSET =3D 0x003CC000 > +DEFINE SECFV_SIZE =3D 0x34000 > + > +!else > + > DEFINE VARS_SIZE =3D 0x20000 > DEFINE VARS_BLOCKS =3D 0x20 > DEFINE VARS_LIVE_SIZE =3D 0xE000 > @@ -53,6 +80,8 @@ > = > !endif > = > +!endif > + > SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFdBaseAddress =3D $(FW_BASE_AD= DRESS) > SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareFdSize =3D $(FW_SIZE) > SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareBlockSize =3D $(BLOCK_SIZE) > diff --git a/OvmfPkg/VarStore.fdf.inc b/OvmfPkg/VarStore.fdf.inc > index ce901c0109b1..7325b698157d 100644 > --- a/OvmfPkg/VarStore.fdf.inc > +++ b/OvmfPkg/VarStore.fdf.inc > @@ -15,7 +15,11 @@ > # > ## > = > +!ifdef $(FD_SIZE_4MB) > +0x00000000|0x0003e000 > +!else > 0x00000000|0x0000e000 > +!endif > #NV_VARIABLE_STORE > DATA =3D { > ## This is the EFI_FIRMWARE_VOLUME_HEADER > @@ -27,14 +31,33 @@ > # { 0xA9, 0x85, 0x27, 0x47, 0x07, 0x5B, 0x4F, 0x50 }} > 0x8D, 0x2B, 0xF1, 0xFF, 0x96, 0x76, 0x8B, 0x4C, > 0xA9, 0x85, 0x27, 0x47, 0x07, 0x5B, 0x4F, 0x50, > +!ifdef $(FD_SIZE_4MB) > + # FvLength: 0x80000 > + 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, > +!else > # FvLength: 0x20000 > 0x00, 0x00, 0x02, 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, > +!ifdef $(FD_SIZE_4MB) > + # CheckSum > + 0xB3, 0xF8, > +!else > + # CheckSum > + 0x19, 0xF9, > +!endif > + # ExtHeaderOffset #Reserved #Revision > + 0x00, 0x00, 0x00, 0x02, > +!ifdef $(FD_SIZE_4MB) > + # Blockmap[0]: 0x80 Blocks * 0x1000 Bytes / Block > + 0x80, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00, > +!else > # Blockmap[0]: 0x20 Blocks * 0x1000 Bytes / Block > 0x20, 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 > @@ -44,18 +67,33 @@ > # { 0xa1, 0x80, 0x2e, 0x14, 0x4e, 0xc3, 0x77, 0x92 }} > 0x78, 0x2c, 0xf3, 0xaa, 0x7b, 0x94, 0x9a, 0x43, > 0xa1, 0x80, 0x2e, 0x14, 0x4e, 0xc3, 0x77, 0x92, > +!ifdef $(FD_SIZE_4MB) > + # Size: 0x3e000 (gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVaria= bleSize) - > + # 0x48 (size of EFI_FIRMWARE_VOLUME_HEADER) =3D 0x3dfb8 > + # This can speed up the Variable Dispatch a bit. > + 0xB8, 0xDF, 0x03, 0x00, > +!else > # Size: 0xe000 (gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariab= leSize) - > # 0x48 (size of EFI_FIRMWARE_VOLUME_HEADER) =3D 0xdfb8 > # This can speed up the Variable Dispatch a bit. > 0xB8, 0xDF, 0x00, 0x00, > +!endif > # FORMATTED: 0x5A #HEALTHY: 0xFE #Reserved: UINT16 #Reserved1: UINT32 > 0x5A, 0xFE, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 > } > = > +!ifdef $(FD_SIZE_4MB) > +0x0003e000|0x00001000 > +!else > 0x0000e000|0x00001000 > +!endif > #NV_EVENT_LOG > = > +!ifdef $(FD_SIZE_4MB) > +0x0003f000|0x00001000 > +!else > 0x0000f000|0x00001000 > +!endif > #NV_FTW_WORKING > DATA =3D { > # EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER->Signature =3D gEdkiiWorking= BlockSignatureGuid =3D > @@ -68,5 +106,9 @@ > 0xE0, 0x0F, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 > } > = > +!ifdef $(FD_SIZE_4MB) > +0x00040000|0x00040000 > +!else > 0x00010000|0x00010000 > +!endif > #NV_FTW_SPARE > -- = > 2.9.3 > = >=20