public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Yao, Jiewen" <jiewen.yao@intel.com>
To: Laszlo Ersek <lersek@redhat.com>, edk2-devel-01 <edk2-devel@ml01.01.org>
Cc: "Ni, Ruiyu" <ruiyu.ni@intel.com>,
	"Justen, Jordan L" <jordan.l.justen@intel.com>,
	Andrew Fish <afish@apple.com>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>
Subject: Re: memory type information HOB / UEFI memmap defrag
Date: Wed, 22 Feb 2017 00:46:40 +0000	[thread overview]
Message-ID: <74D8A39837DF1E4DA445A8C0B3885C503A8F2DE0@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <d27d02e3-d2c7-015d-4997-33b23d0f729a@redhat.com>

HI Laszlo
The purpose of this table to put OS consumed memory together to avoid 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/ EfiBootServicesData to avoid confusing.


> +  { 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 <edk2-devel@ml01.01.org>
Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Andrew Fish <afish@apple.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>
Subject: [edk2] memory type information HOB / UEFI memmap defrag

Hi,

the UEFI memmap under OVMF is getting very fragmented, I'm now counting
~80 entries in it, under various circumstances.

I recall that a platform's PlatformPei can "prime" the DXE/UEFI memory
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 driver)
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 with 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 supports 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 making
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 PlatformPei
    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, which
    makes the dynamism totally not worth it.

    *Maybe* we could add a callback for when the variable service PPI is
    installed. Dunno.

(2) Supporting a third boot mode beyond BOOT_WITH_FULL_CONFIGURATION 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 personally
  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 "mDefaultMemoryTypeInformation"
  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 reason
  I didn't ultimately post any patch (and why I stopped pursuing this
  question). For reference, this was the patch:

> commit b357e8d88c0304ea2b31aefafe53d06c9769fb78
> Author: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>
> Date:   Thu Sep 17 16:18:46 2015 +0200
>
>     OvmfPkg: PlatformPei: decrease memmap fragmentation
>
>     Inspired by ArmVirtPkg commit c199315 ("ArmVirtPkg: increase memory
>     preallocations to reduce region count"), I checked the number of entries
>     in the UEFI memory map, as dumped by the UEFI shell's MEMMAP command, and
>     by the Linux kernel. The number of entries is quite high, about 50-55.
>
>     I calculated the new preallocations as follows:
>     - added 15% to each byte count usage reported by the MEMMAP command, for
>       some future-proofing,
>     - expressed the result in kilobytes (both pages and byte counts 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) of reserved
>     memory usage. The new preallocation, expressed in kilobytes, is
>     trunc(180224 * 1.15 / 1024) = 202. This preallocation is approx. 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 former
>                    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 remains
>     fragmented; mostly due to small unused Conventional Memory entries between
>     other types of allocations. The entry count doesn't go below 40.
>
>     Contributed-under: TianoCore Contribution Agreement 1.0
>     Signed-off-by: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>
>
> 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[] = {
> -  { 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, which
I found "meh". I figured I'd ask again, because now I'm seeing about 80
entries in the memmap. (I wonder if that is related to OVMF's recently
increased ACPI S3 boot script usage!)

Thanks,
Laszlo

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
https://lists.01.org/mailman/listinfo/edk2-devel


  parent reply	other threads:[~2017-02-22  0:46 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-21 15:24 memory type information HOB / UEFI memmap defrag Laszlo Ersek
2017-02-21 22:35 ` Jordan Justen
2017-02-21 23:46   ` Laszlo Ersek
2017-02-22  0:46 ` Yao, Jiewen [this message]
2017-02-22  1:31   ` Jordan Justen
2017-02-22  1:48     ` Kinney, Michael D
2017-02-22  2:31       ` Laszlo Ersek
2017-02-22  2:46         ` Kinney, Michael D
2017-02-22  2:54           ` Jordan Justen
2017-02-22  3:14             ` Laszlo Ersek
2017-02-22  3:23               ` Kinney, Michael D
2017-02-22  3:31               ` Laszlo Ersek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=74D8A39837DF1E4DA445A8C0B3885C503A8F2DE0@shsmsx102.ccr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox