public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* memory type information HOB / UEFI memmap defrag
@ 2017-02-21 15:24 Laszlo Ersek
  2017-02-21 22:35 ` Jordan Justen
  2017-02-22  0:46 ` Yao, Jiewen
  0 siblings, 2 replies; 12+ messages in thread
From: Laszlo Ersek @ 2017-02-21 15:24 UTC (permalink / raw)
  To: edk2-devel-01
  Cc: Andrew Fish, Ni, Ruiyu, Ard Biesheuvel,
	Jordan Justen (Intel address)

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



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: memory type information HOB / UEFI memmap defrag
  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
  1 sibling, 1 reply; 12+ messages in thread
From: Jordan Justen @ 2017-02-21 22:35 UTC (permalink / raw)
  To: Laszlo Ersek, edk2-devel-01; +Cc: Ni, Ruiyu, Andrew Fish, Ard Biesheuvel

On 2017-02-21 07:24:11, Laszlo Ersek wrote:
> 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?

I think grep for EFI_MEMORY_TYPE_INFORMATION_VARIABLE_NAME.

-Jordan

> - 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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: memory type information HOB / UEFI memmap defrag
  2017-02-21 22:35 ` Jordan Justen
@ 2017-02-21 23:46   ` Laszlo Ersek
  0 siblings, 0 replies; 12+ messages in thread
From: Laszlo Ersek @ 2017-02-21 23:46 UTC (permalink / raw)
  To: Jordan Justen; +Cc: edk2-devel-01, Ni, Ruiyu, Andrew Fish, Ard Biesheuvel

On 02/21/17 23:35, Jordan Justen wrote:
> On 2017-02-21 07:24:11, Laszlo Ersek wrote:
>> 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?
> 
> I think grep for EFI_MEMORY_TYPE_INFORMATION_VARIABLE_NAME.

Thanks, that looks very interesting:

* Apparently, if the PEI phase does not produce the info HOB, but it
includes the VariablePei driver, then the DXE IPL PEIM will itself build
the HOB, from the variable.

This seems to eliminate a lot of trouble: for example, if the platform
includes VariablePei, then that will surely be available by the time the
DXE IPL PEIM looks for it, in DxeLoadCore(). (No ordering or dependency
problems, that is!)

* BmSetMemoryTypeInformationVariable()
[MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c] seems to read the
variable (if it exists) under all boot modes different from
BOOT_WITH_DEFAULT_SETTINGS, and to set the variable if it doesn't exist,
or the page counts were changed.

For BOOT_WITH_FULL_CONFIGURATION, as a special case of the above, the
page counts never decrease, only increase, which looks good too.


Based on the above, solving the fragmentation could be as simple as: (a)
including VariablePei in OvmfPkg, (b) removing the HOB production from
OvmfPkg/PlatformPei.

I'll give it a whirl later on, and see how it affects the fragmentation.

Thank you Jordan, this was *really* helpful!
Laszlo

> 
> -Jordan
> 
>> - 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



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: memory type information HOB / UEFI memmap defrag
  2017-02-21 15:24 memory type information HOB / UEFI memmap defrag Laszlo Ersek
  2017-02-21 22:35 ` Jordan Justen
@ 2017-02-22  0:46 ` Yao, Jiewen
  2017-02-22  1:31   ` Jordan Justen
  1 sibling, 1 reply; 12+ messages in thread
From: Yao, Jiewen @ 2017-02-22  0:46 UTC (permalink / raw)
  To: Laszlo Ersek, edk2-devel-01
  Cc: Ni, Ruiyu, Justen, Jordan L, Andrew Fish, Ard Biesheuvel

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: memory type information HOB / UEFI memmap defrag
  2017-02-22  0:46 ` Yao, Jiewen
@ 2017-02-22  1:31   ` Jordan Justen
  2017-02-22  1:48     ` Kinney, Michael D
  0 siblings, 1 reply; 12+ messages in thread
From: Jordan Justen @ 2017-02-22  1:31 UTC (permalink / raw)
  To: Yao, Jiewen, Laszlo Ersek, edk2-devel-01
  Cc: Ni, Ruiyu, Andrew Fish, Ard Biesheuvel

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
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: memory type information HOB / UEFI memmap defrag
  2017-02-22  1:31   ` Jordan Justen
@ 2017-02-22  1:48     ` Kinney, Michael D
  2017-02-22  2:31       ` Laszlo Ersek
  0 siblings, 1 reply; 12+ messages in thread
From: Kinney, Michael D @ 2017-02-22  1:48 UTC (permalink / raw)
  To: Justen, Jordan L, Yao, Jiewen, Laszlo Ersek, edk2-devel-01,
	Kinney, Michael D
  Cc: Ni, Ruiyu, Andrew Fish, Ard Biesheuvel

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.

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: memory type information HOB / UEFI memmap defrag
  2017-02-22  1:48     ` Kinney, Michael D
@ 2017-02-22  2:31       ` Laszlo Ersek
  2017-02-22  2:46         ` Kinney, Michael D
  0 siblings, 1 reply; 12+ messages in thread
From: Laszlo Ersek @ 2017-02-22  2:31 UTC (permalink / raw)
  To: Kinney, Michael D, Justen, Jordan L, Yao, Jiewen, edk2-devel-01
  Cc: Ni, Ruiyu, Andrew Fish, Ard Biesheuvel

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



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: memory type information HOB / UEFI memmap defrag
  2017-02-22  2:31       ` Laszlo Ersek
@ 2017-02-22  2:46         ` Kinney, Michael D
  2017-02-22  2:54           ` Jordan Justen
  0 siblings, 1 reply; 12+ messages in thread
From: Kinney, Michael D @ 2017-02-22  2:46 UTC (permalink / raw)
  To: Laszlo Ersek, Justen, Jordan L, Yao, Jiewen, edk2-devel-01,
	Kinney, Michael D
  Cc: Ni, Ruiyu, Andrew Fish, Ard Biesheuvel

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.

Mike

> -----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



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: memory type information HOB / UEFI memmap defrag
  2017-02-22  2:46         ` Kinney, Michael D
@ 2017-02-22  2:54           ` Jordan Justen
  2017-02-22  3:14             ` Laszlo Ersek
  0 siblings, 1 reply; 12+ messages in thread
From: Jordan Justen @ 2017-02-22  2:54 UTC (permalink / raw)
  To: Kinney, Michael D, Yao, Jiewen, Laszlo Ersek, edk2-devel-01
  Cc: Ni, Ruiyu, Andrew Fish, Ard Biesheuvel

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 <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
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: memory type information HOB / UEFI memmap defrag
  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
  0 siblings, 2 replies; 12+ messages in thread
From: Laszlo Ersek @ 2017-02-22  3:14 UTC (permalink / raw)
  To: Jordan Justen, Kinney, Michael D, Yao, Jiewen, edk2-devel-01
  Cc: Ni, Ruiyu, Andrew Fish, Ard Biesheuvel

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
>>



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: memory type information HOB / UEFI memmap defrag
  2017-02-22  3:14             ` Laszlo Ersek
@ 2017-02-22  3:23               ` Kinney, Michael D
  2017-02-22  3:31               ` Laszlo Ersek
  1 sibling, 0 replies; 12+ messages in thread
From: Kinney, Michael D @ 2017-02-22  3:23 UTC (permalink / raw)
  To: Laszlo Ersek, Justen, Jordan L, Yao, Jiewen, edk2-devel-01,
	Kinney, Michael D
  Cc: Ni, Ruiyu, Andrew Fish, Ard Biesheuvel

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: memory type information HOB / UEFI memmap defrag
  2017-02-22  3:14             ` Laszlo Ersek
  2017-02-22  3:23               ` Kinney, Michael D
@ 2017-02-22  3:31               ` Laszlo Ersek
  1 sibling, 0 replies; 12+ messages in thread
From: Laszlo Ersek @ 2017-02-22  3:31 UTC (permalink / raw)
  To: Jordan Justen, Kinney, Michael D, Yao, Jiewen, edk2-devel-01
  Cc: Ni, Ruiyu, Andrew Fish, Ard Biesheuvel

On 02/22/17 04:14, Laszlo Ersek wrote:
> 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.

There's at least one big complication though. OVMF still supports a
RAM-emulated varstore, in addition to pflash. The pflash location is
known in advance (it is a set of constants from the build process), but
the area for the emulation is allocated dynamically during PEI. And, we
only really decide which one to use for variables in the DXE phase
(that's when we check for the presence of pflash).

This sort of throws a wrench into VariablePei's gears; we cannot tell it
where to look for the varstore. We could move the flash detection from
DXE to PEI, but that's again too much complication for this -- if we
could mitigate the fragmentation by bumping the constants in our current
HOB, that would look way more attractive to me.

If we removed the RAM-emulated varstore fully, and required pflash no
matter what, then I might feel differently, I think.

Thanks
Laszlo


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2017-02-22  3:31 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox