From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) (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 BBD588224C for ; Tue, 21 Feb 2017 18:54:04 -0800 (PST) Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 21 Feb 2017 18:54:03 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.35,192,1484035200"; d="scan'208";a="68083483" Received: from ahirst-mobl.amr.corp.intel.com (HELO localhost) ([10.252.140.121]) by fmsmga005.fm.intel.com with ESMTP; 21 Feb 2017 18:54:03 -0800 MIME-Version: 1.0 To: "Kinney, Michael D" , "Yao, Jiewen" , Laszlo Ersek , edk2-devel-01 Message-ID: <148773204321.13093.9257287652423723407@jljusten-ivb> From: Jordan Justen In-Reply-To: Cc: "Ni, Ruiyu" , Andrew Fish , Ard Biesheuvel References: <74D8A39837DF1E4DA445A8C0B3885C503A8F2DE0@shsmsx102.ccr.corp.intel.com> <148772709277.12591.2606094062546040536@jljusten-ivb> <234e56b7-7eb4-3220-8335-2887a084b30f@redhat.com> User-Agent: alot/0.5.1 Date: Tue, 21 Feb 2017 18:54:03 -0800 Subject: Re: memory type information HOB / UEFI memmap defrag X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 22 Feb 2017 02:54:04 -0000 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable On 2017-02-21 18:46:01, Kinney, Michael D wrote: > Laszlo, > = > The only side effect of not producing the HOB when the variable does > not exist is that the first boot of a platform has a fragmented > memory map and you may get an extra reboot when the variable is set. > A fragmented memory map will also be produced if the variable store = > contents are corrupt or zeroed. > = Would it be possible to inhibit the reboot until we fully support S4? I think it'd be fine to have a fragmented map for one boot if it was corrected on future boots of the machine. I think we should consider continuing to produce the HOB in PlatformPei if we can fairly easily reduce the fragmentation on the first boot via the HOB. -Jordan > = > > -----Original Message----- > > From: Laszlo Ersek [mailto:lersek@redhat.com] > > Sent: Tuesday, February 21, 2017 6:31 PM > > To: Kinney, Michael D ; Justen, Jordan L > > ; Yao, Jiewen ; edk2-d= evel-01 > devel@ml01.01.org> > > Cc: Ni, Ruiyu ; Andrew Fish ; Ard = Biesheuvel > > > > Subject: Re: [edk2] memory type information HOB / UEFI memmap defrag > > = > > On 02/22/17 02:48, Kinney, Michael D wrote: > > > Jordan, > > > > > > The usage of EfiLoaderCode/ EfiBootServicesCode/ EfiBootServicesData > > > may vary from boot to boot, especially if the shell or other applicat= ions > > > are run or different OSes are booted. A change in the bin size causes > > > extra variable writes and potentially extra reboots. > > = > > As I wrote elsewere, in a few days (or, well, weeks) I would like to > > research the simpler-looking avenue of (a) simply not producing this HOB > > in OVMF's PlatformPei at all, and (b) pulling in VariablePei. As far as > > I understand the code in the DXE IPL PEIM and BDS DXE, this should > > enable those two modules to communicate with each other through the > > variable highlighted by Jordan, and to create the HOB automatically. The > > code seems to track / maintain the maximum memory usage seen during > > previous boots, which I believe is appropriate for OVMF. > > = > > If this worked without any more platform cooperation than (a) and (b), > > that would be awesome & my preference. > > = > > Thanks! > > Laszlo > > = > > > > > > Mike > > > > > >> -----Original Message----- > > >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf = Of Jordan > > Justen > > >> Sent: Tuesday, February 21, 2017 5:32 PM > > >> To: Yao, Jiewen ; Laszlo Ersek ; edk2- > > devel- > > >> 01 > > >> Cc: Ni, Ruiyu ; Andrew Fish ; A= rd Biesheuvel > > >> > > >> Subject: Re: [edk2] memory type information HOB / UEFI memmap defrag > > >> > > >> On 2017-02-21 16:46:40, Yao, Jiewen wrote: > > >>> HI Laszlo > > >>> > > >>> The purpose of this table to put OS consumed memory together to avo= id S4 resume > > >>> issue. EfiLoaderCode/ EfiBootServicesCode/ EfiBootServicesData are = not used by > > >>> OS. There is no need to put them here. > > >>> > > >>> I suggest we remove EfiLoaderCode/ EfiBootServicesCode/ EfiBootServ= icesData to > > >>> avoid confusing. > > >>> > > >> > > >> Is there any other advantage to removing them? > > >> > > >> I guess it would be easy enough to re-add them, but I don't think we > > >> need to move away from supporting S4. While I agree that S4 should n= ot > > >> be a big priority, I'd prefer that we try to support it at some poin= t. > > >> > > >> -Jordan > > >> > > >>> > > >>>> + { EfiReservedMemoryType, EFI_SIZE_TO_PAGES ((UINTN)SIZE_1KB * = 202) }, > > >>>> + { EfiLoaderCode, EFI_SIZE_TO_PAGES ((UINTN)SIZE_1KB * = 1439) }, > > >>>> + { EfiBootServicesCode, EFI_SIZE_TO_PAGES ((UINTN)SIZE_1KB * = 5980) }, > > >>>> + { EfiBootServicesData, EFI_SIZE_TO_PAGES ((UINTN)SIZE_1KB * = 41643) }, > > >>>> + { EfiRuntimeServicesCode, EFI_SIZE_TO_PAGES ((UINTN)SIZE_1KB * = 1025) }, > > >>>> + { EfiRuntimeServicesData, EFI_SIZE_TO_PAGES ((UINTN)SIZE_1KB * = 3629) }, > > >>>> + { EfiACPIReclaimMemory, EFI_SIZE_TO_PAGES ((UINTN)SIZE_1KB * = 36) }, > > >>>> + { EfiACPIMemoryNVS, EFI_SIZE_TO_PAGES ((UINTN)SIZE_1KB * = 1301) }, > > >>>> + { EfiMaxMemoryType, 0 = } > > >>> > > >>> > > >>> > > >>> Thank you > > >>> > > >>> Yao Jiewen > > >>> > > >>> > > >>> > > >>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf= Of Laszlo > > >>> Ersek > > >>> Sent: Tuesday, February 21, 2017 11:24 PM > > >>> To: edk2-devel-01 > > >>> Cc: Ni, Ruiyu ; Justen, Jordan L > > >>> ; Andrew Fish ; Ard Bie= sheuvel > > >>> > > >>> Subject: [edk2] memory type information HOB / UEFI memmap defrag > > >>> > > >>> > > >>> > > >>> Hi, > > >>> > > >>> the UEFI memmap under OVMF is getting very fragmented, I'm now coun= ting > > >>> ~80 entries in it, under various circumstances. > > >>> > > >>> I recall that a platform's PlatformPei can "prime" the DXE/UEFI mem= ory > > >>> allocation system (not the GCD services) for various memory types, = by > > >>> producing a memory type information HOB. > > >>> > > >>> My vague understanding is that BDS will in turn check if the actual > > >>> allocations fit in the allotments from the HOB, and if not, it will= try > > >>> to feed back the increased amount to PEI, for the next boot. > > >>> > > >>> As far as I understand, this requires the VariablePei (read only dr= iver) > > >>> for a platform (so that its PlatformPei can read the info from BDS,= and > > >>> produce the HOB accordingly). Some questions: > > >>> > > >>> - how big is VariablePei in binary form? > > >>> - does it depend on permanent RAM being installed / discovered? > > >>> - If so, is that dependency implemented with a static DEPEX, or wit= h a > > >>> callback? > > >>> > > >>> Further questions: > > >>> - what is the variable (GUID and Name) that BDS uses for this > > >>> information? > > >>> - What is the format of the variable? > > >>> - Does the logic depend on particular boot modes? OVMF only support= s two > > >>> boot modes, BOOT_WITH_FULL_CONFIGURATION and BOOT_ON_S3_RESUME. > > >>> > > >>> In OVMF we currently use a static array for populating the HOB (see > > >>> "mDefaultMemoryTypeInformation" in "PlatformPei/Platform.c"). If ma= king > > >>> it all dynamic is easy, I think I'd like to do it (sometime later). > > >>> > > >>> If, however, it would require us to up-end OVMF's PlatformPei, then= I > > >>> think it's not worth it; we can just bump the values in > > >>> "mDefaultMemoryTypeInformation" suitably. > > >>> > > >>> Some examples I consider as up-ending OVMF's PlatformPei: > > >>> > > >>> (1) If VariablePei needs permanent RAM with a hard DEPEX. In OVMF, > > >>> permanent RAM is installed by PlatformPei (thereby potentially > > >>> unblocking VariablePei's dispatch); however, it is also Platfor= mPei > > >>> that would require the r/o variable service to work, because > > >>> PlatformPei produces the memory type information HOB. So, such a > > >>> DEPEX in VariablePei would require splitting up PlatformPei, wh= ich > > >>> makes the dynamism totally not worth it. > > >>> > > >>> *Maybe* we could add a callback for when the variable service P= PI is > > >>> installed. Dunno. > > >>> > > >>> (2) Supporting a third boot mode beyond BOOT_WITH_FULL_CONFIGURATIO= N and > > >>> BOOT_ON_S3_RESUME. Not even worth the audit of current boot mode > > >>> checks. > > >>> > > >>> Further remarks: > > >>> > > >>> - OVMF doesn't care about supporting S4 at the moment, and I person= ally > > >>> have no plans to work on that. (I'm saying this because I vaguely > > >>> recall that the memory type info HOB is related to S4 resume, so = an > > >>> argument could perhaps be made, "this could enable S4 for OVMF". > > >>> Personally, I'm not interested. Still carrying the scars of S3.) > > >>> > > >>> - I actually tried to bump the values in "mDefaultMemoryTypeInforma= tion" > > >>> quite a few months back, but the benefits I saw were negligible. = I was > > >>> left confused about the memory type info HOB, and that was the re= ason > > >>> I didn't ultimately post any patch (and why I stopped pursuing th= is > > >>> question). For reference, this was the patch: > > >>> > > >>>> commit b357e8d88c0304ea2b31aefafe53d06c9769fb78 > > >>>> Author: Laszlo Ersek > > >>>> Date: Thu Sep 17 16:18:46 2015 +0200 > > >>>> > > >>>> OvmfPkg: PlatformPei: decrease memmap fragmentation > > >>>> > > >>>> Inspired by ArmVirtPkg commit c199315 ("ArmVirtPkg: increase m= emory > > >>>> preallocations to reduce region count"), I checked the number = of entries > > >>>> in the UEFI memory map, as dumped by the UEFI shell's MEMMAP c= ommand, and > > >>>> by the Linux kernel. The number of entries is quite high, abou= t 50-55. > > >>>> > > >>>> I calculated the new preallocations as follows: > > >>>> - added 15% to each byte count usage reported by the MEMMAP co= mmand, for > > >>>> some future-proofing, > > >>>> - expressed the result in kilobytes (both pages and byte count= s are hard > > >>>> to read), > > >>>> - just for our information, I calculated the ratio between the= new > > >>>> preallocation and the old one. > > >>>> > > >>>> For example, the UEFI shell reported 44 pages (180224 bytes) o= f reserved > > >>>> memory usage. The new preallocation, expressed in kilobytes, is > > >>>> trunc(180224 * 1.15 / 1024) =3D 202. This preallocation is app= rox. 12.62 > > >>>> times the previous preallocation (which was 4 pages, ie. 16384= bytes). > > >>>> > > >>>> Here's the full table: > > >>>> > > >>>> memory type pages from bytes from new KB factor of for= mer > > >>>> MEMMAP cmd MEMMAP cmd prealloc prealloc > > >>>> ----------- ---------- ---------- -------- -------------= --- > > >>>> Reserved 44 180224 202 12= .62 > > >>>> LoaderCode 313 1282048 1439 = n/a > > >>>> BS_Code 1300 5324800 5980 3= .89 > > >>>> BS_Data 9053 37081088 41643 2= .71 > > >>>> RT_Code 223 913408 1025 5= .33 > > >>>> RT_Data 789 3231744 3629 25= .20 > > >>>> ACPI_Recl 8 32768 36 1= .12 > > >>>> ACPI_NVS 283 1159168 1301 81= .31 > > >>>> > > >>>> ... Unfortunately, when the patch is applied, the memory map r= emains > > >>>> fragmented; > > >>> mostly due to small unused Conventional Memory entries between > > >>>> other types of allocations. The entry count doesn't go below 4= 0. > > >>>> > > >>>> Contributed-under: TianoCore Contribution Agreement 1.0 > > >>>> Signed-off-by: Laszlo Ersek > > >>>> > > >>>> diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/= Platform.c > > >>>> index a6d961673d3a..38abf3811600 100644 > > >>>> --- a/OvmfPkg/PlatformPei/Platform.c > > >>>> +++ b/OvmfPkg/PlatformPei/Platform.c > > >>>> @@ -41,14 +41,15 @@ > > >>>> #include "Cmos.h" > > >>>> > > >>>> EFI_MEMORY_TYPE_INFORMATION mDefaultMemoryTypeInformation[] =3D { > > >>>> - { EfiACPIMemoryNVS, 0x004 }, > > >>>> - { EfiACPIReclaimMemory, 0x008 }, > > >>>> - { EfiReservedMemoryType, 0x004 }, > > >>>> - { EfiRuntimeServicesData, 0x024 }, > > >>>> - { EfiRuntimeServicesCode, 0x030 }, > > >>>> - { EfiBootServicesCode, 0x180 }, > > >>>> - { EfiBootServicesData, 0xF00 }, > > >>>> - { EfiMaxMemoryType, 0x000 } > > >>>> + { EfiReservedMemoryType, EFI_SIZE_TO_PAGES ((UINTN)SIZE_1KB * = 202) }, > > >>>> + { EfiLoaderCode, EFI_SIZE_TO_PAGES ((UINTN)SIZE_1KB * = 1439) }, > > >>>> + { EfiBootServicesCode, EFI_SIZE_TO_PAGES ((UINTN)SIZE_1KB * = 5980) }, > > >>>> + { EfiBootServicesData, EFI_SIZE_TO_PAGES ((UINTN)SIZE_1KB * = 41643) }, > > >>>> + { EfiRuntimeServicesCode, EFI_SIZE_TO_PAGES ((UINTN)SIZE_1KB * = 1025) }, > > >>>> + { EfiRuntimeServicesData, EFI_SIZE_TO_PAGES ((UINTN)SIZE_1KB * = 3629) }, > > >>>> + { EfiACPIReclaimMemory, EFI_SIZE_TO_PAGES ((UINTN)SIZE_1KB * = 36) }, > > >>>> + { EfiACPIMemoryNVS, EFI_SIZE_TO_PAGES ((UINTN)SIZE_1KB * = 1301) }, > > >>>> + { EfiMaxMemoryType, 0 = } > > >>>> }; > > >>>> > > >>>> > > >>> > > >>> As you can see in the commit message, at that time the patch only > > >>> managed to decrease the number of memmap entries from ~55 to ~40, w= hich > > >>> I found "meh". I figured I'd ask again, because now I'm seeing abou= t 80 > > >>> entries in the memmap. (I wonder if that is related to OVMF's recen= tly > > >>> increased ACPI S3 boot script usage!) > > >>> > > >>> Thanks, > > >>> Laszlo > > >>> > > >>> _______________________________________________ > > >>> edk2-devel mailing list > > >>> edk2-devel@lists.01.org > > >>> https://lists.01.org/mailman/listinfo/edk2-devel > > >>> > > >> _______________________________________________ > > >> edk2-devel mailing list > > >> edk2-devel@lists.01.org > > >> https://lists.01.org/mailman/listinfo/edk2-devel >=20