From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.132.183.28; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (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 961D0202E59C5 for ; Thu, 19 Oct 2017 15:05:12 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 4C0FF883D6; Thu, 19 Oct 2017 22:08:50 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 4C0FF883D6 Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=lersek@redhat.com Received: from lacos-laptop-7.usersys.redhat.com (ovpn-124-219.rdu2.redhat.com [10.10.124.219]) by smtp.corp.redhat.com (Postfix) with ESMTP id C8A4060BE3; Thu, 19 Oct 2017 22:08:48 +0000 (UTC) To: "Gao, Liming" , Star Zeng , "Jordan Justen (Intel address)" , Ard Biesheuvel Cc: edk2-devel-01 From: Laszlo Ersek Message-ID: Date: Fri, 20 Oct 2017 00:08:47 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Thu, 19 Oct 2017 22:08:50 +0000 (UTC) Subject: dynamic PCD impact on temporary PEI memory 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: Thu, 19 Oct 2017 22:05:12 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Hi, I've encountered an interesting phenomenon, and I'd like to discuss it before I submit an OvmfPkg patch. * Background #1: OVMF uses 32KB total memory for temporary SEC/PEI heap and stack, combined. The fixed PCDs for the base address of this (combined) area, and for its size, are set in the OVMF FDF files, uniformly between IA32 / X64 / IA32X64: - PcdOvmfSecPeiTempRamBase = 0x00810000 (8MB + 64KB) - PcdOvmfSecPeiTempRamSize = 0x00008000 (32KB) See: > [FD.MEMFD] > BaseAddress = $(MEMFD_BASE_ADDRESS) > [...] > 0x007000|0x001000 > gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize > > 0x010000|0x008000 > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize > > 0x020000|0x0E0000 > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvSize > [...] and "MEMFD_BASE_ADDRESS" comes from "OvmfPkg.fdf.inc": > DEFINE MEMFD_BASE_ADDRESS = 0x800000 (There is a reason why I also quoted "PcdGuidedExtractHandlerTableAddress" and "PcdOvmfPeiMemFvBase" above; but more on that later.) Half of this memory (16KB) is used for stack, the other half is used for heap. * Background #2: "MdeModulePkg/Universal/PCD/Pei/Pcd.inf" writes (rewrapping it here for better readability): > # 3.1 PcdPeim and PcdDxe > # PEI PCD database is maintained by PcdPeim driver run from > # flash. PcdPeim driver build guid hob in temporary memory and > # copy the binary data base from flash to temporary memory for > # PEI PCD database. > # DXE PCD database is maintained by PcdDxe driver.At entry point > # of PcdDxe driver, a new PCD database is allocated in boot-time > # memory which including all PEI PCD and DXE PCD entry. This means that the more dynamic PCDs we have in the PEI phase (cumulatively across all PEIMs), the larger the PCD HOB will be, and the more temporary PEI RAM (heap) will be needed. * Background #3: Recently we added yet another dynamic PCD to "OvmfPkg/PlatformPei/PlatformPei.inf" -- and with that, to OVMF's PEI phase at all --, namely gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy which has type UINT32. Unfortunately, I messed up the patch review a bit, so I cannot point to direct neighbor commits, from which the first doesn't have the PCD in PlatformPei just yet, and the other commit (right after) does. There's a build error between these two points in the git history, so the commits that can be tested like described above are not adjacent. Anyway, the nearest points in history that *can* be used for comparison are: - the commit just before adding the PCD to PlatformPei; namely 071f1d19ddbc ("SecurityPkg: make PcdOptionRomImageVerificationPolicy dynamic", 2017-10-05); - the commit when OVMF can be built again (without -D SECURE_BOOT_ENABLE); namely 1958124a6cb0 ("OvmfPkg: fix dynamic default for oprom verification policy PCD without SB", 2017-10-17). Below I'm going to refer to these commits as "before" and "after". * Symptom #1: I built OVMF for IA32, IA32X64 and X64, both "before" and "after". Then I compared the log files, to see the impact of the addition of exactly one UINT32 dynamic PCD to the PCD HOB, on temporary PEI memory usage. - Diff between "before" and "after", for IA32: > Temp Stack : BaseAddress=0x814000 Length=0x4000 > Temp Heap : BaseAddress=0x810000 Length=0x4000 > Total temporary memory: 32768 bytes. > temporary memory stack ever used: 16384 bytes. > - temporary memory heap used for HobList: 5616 bytes. > + temporary memory heap used for HobList: 5664 bytes. > temporary memory heap occupied by memory pages: 0 bytes. - Diff between "before" and "after", for IA32X64: > Temp Stack : BaseAddress=0x814000 Length=0x4000 > Temp Heap : BaseAddress=0x810000 Length=0x4000 > Total temporary memory: 32768 bytes. > temporary memory stack ever used: 16384 bytes. > - temporary memory heap used for HobList: 5616 bytes. > + temporary memory heap used for HobList: 5664 bytes. > temporary memory heap occupied by memory pages: 0 bytes. - Diff between "before" and "after", for IA32X64: > Temp Stack : BaseAddress=0x814000 Length=0x4000 > Temp Heap : BaseAddress=0x810000 Length=0x4000 > Total temporary memory: 32768 bytes. > temporary memory stack ever used: 16384 bytes. > - temporary memory heap used for HobList: 7952 bytes. > + temporary memory heap used for HobList: 8032 bytes. > temporary memory heap occupied by memory pages: 0 bytes. Observations: - The IA32 and IA32X64 builds are identical in this regard (not surprisingly; the PEI phase is 32-bit in both). - When PEI is 32-bit, the increase is 48 bytes. - When PEI is 64-bit, the increase is 80 bytes. - The *basis* (i.e., "before") heap consumption in 64-bit PEI is significantly higher than in 32-bit PEI (7952 bytes vs. 5616 bytes). And, the addition of the new dynamic PCD only widens the gap. * Symptom #2: (This is actually the symptom that causes me to write this email :) , I'm just laying things out in reverse order of my "discoveries".) In my local tree, I have *another* dynamic PCD (of type BOOLEAN) in PlatformPei (and thereby in the PEI phase). Today I've found that the addition of gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy in *combination* with that dynamic PCD that I have privately, prevents X64 OVMF from booting -- and only X64; IA32 and IA32X64 are fine. For X64, the exact failure symptoms appear to vary between FD_SIZE_2MB and FD_SIZE_4MB builds. In one case the PEI phase gets stuck with 100% CPU consumption, in the other case the DXE core fails to initialize with the following message: > ASSERT MdeModulePkg/Core/Dxe/Gcd/Gcd.c(2208): Found Anyway, let me quote the same comparisons: - Diff between "after" and "(not really) broken", IA32: > Temp Stack : BaseAddress=0x814000 Length=0x4000 > Temp Heap : BaseAddress=0x810000 Length=0x4000 > Total temporary memory: 32768 bytes. > temporary memory stack ever used: 16384 bytes. > - temporary memory heap used for HobList: 5664 bytes. > + temporary memory heap used for HobList: 5696 bytes. > temporary memory heap occupied by memory pages: 0 bytes. - Diff between "after" and "(not really) broken", IA32X64: > Temp Stack : BaseAddress=0x814000 Length=0x4000 > Temp Heap : BaseAddress=0x810000 Length=0x4000 > Total temporary memory: 32768 bytes. > temporary memory stack ever used: 16384 bytes. > - temporary memory heap used for HobList: 5664 bytes. > + temporary memory heap used for HobList: 5696 bytes. > temporary memory heap occupied by memory pages: 0 bytes. - Diff between "after" and "broken", X64: > Temp Stack : BaseAddress=0x814000 Length=0x4000 > Temp Heap : BaseAddress=0x810000 Length=0x4000 > Total temporary memory: 32768 bytes. > temporary memory stack ever used: 16384 bytes. > - temporary memory heap used for HobList: 8032 bytes. > + temporary memory heap used for HobList: 8096 bytes. > temporary memory heap occupied by memory pages: 0 bytes. Observations: - The IA32 and IA32X64 builds again behave identically. - The increase for the IA32 and IA32X64 builds each is 32 bytes. - The IA32 and IA32X64 builds boot fine. - The increase for the X64 build is 64 bytes. - With a temp heap usage of 8096 bytes (for HobList), X64 no longer boots. This value looks suspiciously close to the half of the temp heap (8192 bytes), but maybe that *proportion* doesn't matter -- we might just run out of heap due to other (non-HobList) temp heap needs. And, perhaps, corrupt the temp stack above the temp heap. * What to do: At this point we are one dynamic PCD apart from breaking the PEI phase of upstream OVMF (X64); we should prevent that. If you scroll back to where I quoted [FD.MEMFD], you'll see that we have 32KB un-occupied both below and above the temp RAM. - Regarding the gap below the temp RAM, it has never been used. The last time we took some memory from there was in 2014. See the following commit: ad43bc6b2e35 OvmfPkg: PlatformPei: protect SEC's GUIDed section handler table thru S3 - Regarding the gap above the temp RAM, it used to be occupied, but we freed it up in the following commits (parts of a larger series): 45d870815156 OvmfPkg/PlatformPei: rebase and resize the permanent PEI memory for S3 6b04cca4d697 OvmfPkg: remove PcdS3AcpiReservedMemoryBase, PcdS3AcpiReservedMemorySize I suggest assigning 8KB to the temp PEI RAM from the gap *above*. Adding 8KB would increase both the temp stack and the temp heap by 1 page (4KB) each. What do you guys think? Thanks, Laszlo