Re: memory type information HOB / UEFI memmap defrag

["Kinney, Michael D" <michael.d.kinney@intel.com>]

Laszlo,

The following PCD controls the reset behavior for this case.

  ## Indicates if to reset system when memory type information changes.<BR><BR>
  #   TRUE  - Resets system when memory type information changes.<BR>
  #   FALSE - Does not reset system when memory type information changes.<BR>
  # @Prompt Reset on memory type information change.
  gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|TRUE|BOOLEAN|0x00010056

Mike

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo Ersek
> Sent: Tuesday, February 21, 2017 7:15 PM
> To: Justen, Jordan L <jordan.l.justen@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; edk2-devel-01
> <edk2-devel@ml01.01.org>
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Andrew Fish <afish@apple.com>; Ard Biesheuvel
> <ard.biesheuvel@linaro.org>
> Subject: Re: [edk2] memory type information HOB / UEFI memmap defrag
> 
> Mike,
> 
> On 02/22/17 03:54, Jordan Justen wrote:
> > 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
> 
> We have a permanently fragmented map now :) (Although, I guess it could
> be even worse; the HOB that we produce now likely does help a bit.)
> 
> >> and you may get an extra reboot when the variable is set.
> 
> I think I could live with that.
> 
> In fact, if the reboot is *guaranteed* if the variable does not exist,
> that's likely best, because this way a fragmented memmap is never
> exposed to a guest OS.
> 
> >> A fragmented memory map will also be produced if the variable store
> >> contents are corrupt or zeroed.
> 
> If that was hidden from the OS by a guaranteed reboot, I think I'd
> welcome that reboot. It's a very infrequent occurrence.
> 
> Jordan,
> 
> > Would it be possible to inhibit the reboot until we fully support S4?
> 
> I'm not very worried about the reboot; using pflash and longer-term
> guests, it should happen very rarely.
> 
> For one-off guests though... Hm, I guess it might be somewhat annoying,
> so if we can find a solution for avoiding the reboot even when the
> variable is missing, that would be more elegant.
> 
> > I think it'd be fine to have a fragmented map for one boot if it was
> > corrected on future boots of the machine.
> 
> Is this a trade-off we can control somehow?
> 
> I think you're starting to convince me that it's more user friendly to
> expose a fragmented map *once* than to auto-reboot *once*.
> 
> > 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.
> 
> Sure, we can bump the values in the HOB; however, the problem is with
> identifying "first boot" (= presence of the variable) in PlatformPei, to
> see if we should produce the default HOB. That opens the same can of
> worms :(
> 
> I mean we can install a PPI notify callback for the r/o variable PPI in
> PlatformPei, and then install the HOB in the callback if the variable is
> missing. It's just too much work.
> 
> Hm, wait, we already call PeiServicesNotifyPpi() in PlatformPei, for the
> MP Services PPI (which we use to program the Feature Control MSR on all
> VCPUs if instructed so by QEMU). Then I guess I'm out of arguments; we
> should at least try this. Sigh. :)
> 
> Do you want to reopen
> <https://bugzilla.tianocore.org/show_bug.cgi?id=386>? I just closed it
> because there didn't seem to be an actual use case for VariablePei, but
> now we may have found one, independently of the original reporter's
> (non-upstream) use case.
> 
> Thanks!
> Laszlo
> 
> >
> > -Jordan
> >
> >>
> >>> -----Original Message-----
> >>> From: Laszlo Ersek [mailto:lersek@redhat.com]
> >>> Sent: Tuesday, February 21, 2017 6:31 PM
> >>> To: Kinney, Michael D <michael.d.kinney@intel.com>; Justen, Jordan L
> >>> <jordan.l.justen@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; edk2-devel-01
> <edk2-
> >>> devel@ml01.01.org>
> >>> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Andrew Fish <afish@apple.com>; Ard Biesheuvel
> >>> <ard.biesheuvel@linaro.org>
> >>> 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 applications
> >>>> 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 <jiewen.yao@intel.com>; Laszlo Ersek <lersek@redhat.com>; edk2-
> >>> devel-
> >>>>> 01 <edk2-devel@lists.01.org>
> >>>>> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Andrew Fish <afish@apple.com>; Ard
> Biesheuvel
> >>>>> <ard.biesheuvel@linaro.org>
> >>>>> 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 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.
> >>>>>>
> >>>>>
> >>>>> 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 not
> >>>>> be a big priority, I'd prefer that we try to support it at some point.
> >>>>>
> >>>>> -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 <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>
> >>>>>>> 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>
> >>>>>>>
> >>>>>>> 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
> >>>>>> 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
> >>
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel