public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Michael D Kinney" <michael.d.kinney@intel.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>,
	"graf@amazon.com" <graf@amazon.com>,
	"Kinney, Michael D" <michael.d.kinney@intel.com>
Cc: Leif Lindholm <leif@nuviainc.com>,
	Laszlo Ersek <lersek@redhat.com>,
	"Ard Biesheuvel" <ardb+tianocore@kernel.org>,
	"Justen, Jordan L" <jordan.l.justen@intel.com>,
	"Woodhouse, David" <dwmw@amazon.co.uk>,
	"Hendrik Borghorst" <hborghor@amazon.de>
Subject: Re: [edk2-devel] [PATCH 1/2] MdeModulePkg/Core/Dxe: Allow to force runtime allocations at separate range
Date: Thu, 18 Feb 2021 22:28:40 +0000	[thread overview]
Message-ID: <CO1PR11MB49294F1100253B00127635B0D2859@CO1PR11MB4929.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20210218200953.20943-2-graf@amazon.com>

Hi Alex,

This feature is already available from the DXE Core using the MemoryTypeInformation and was
specifically added to support hibernation use case.

There is an optional HOB that is passed into DXE Core that can provide bin sizes for any supported memory types.  Not just Runtime and ACPI.

This feature can use a fixed HOB or use a dynamic HOB from a UEFI Variable, so that changes in the memory usage of 
the critical memory types for hibernation can be tracked and stored in the UEFI Variables and be used to populate
the HOB.  It is a platform choice to use fixed or dynamic HOB.

https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Include/Guid/MemoryTypeInformation.h

https://github.com/tianocore/edk2/blob/978b9d511f5b9cb7bc5b09749f86c39bec51525d/MdeModulePkg/Core/Dxe/Gcd/Gcd.c#L2233

https://github.com/tianocore/edk2/blob/978b9d511f5b9cb7bc5b09749f86c39bec51525d/MdeModulePkg/Core/Dxe/Mem/Page.c#L45

The DXE Core use the HOB to pre-allocate bins for the memory types and do allocations from those bins.

The UEFI Memory Map provides an entry for the entire bin (no just the allocated space), so small
differences in allocations from boot to boot do not change the UEFI memory map.  This also
reduces the total number of memory map entries in the UEFI memory map.

If dynamic HOB is used, then the Boot Manager compares the actual memory usage to the HOB.
If the usage is larger or smaller by more than a threshold, then the UEFI Variable is
updated.  The platform has the choice to reboot or continue booting after this UEIF Variable
update based on a PCD.  The reboot can help make sure the first boot to the OS has the right
bin sizes to support future hibernate boot flows.


Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Alexander Graf via groups.io
> Sent: Thursday, February 18, 2021 12:10 PM
> To: devel@edk2.groups.io
> Cc: Leif Lindholm <leif@nuviainc.com>; Laszlo Ersek <lersek@redhat.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>;
> Justen, Jordan L <jordan.l.justen@intel.com>; Woodhouse, David <dwmw@amazon.co.uk>; Hendrik Borghorst <hborghor@amazon.de>
> Subject: [edk2-devel] [PATCH 1/2] MdeModulePkg/Core/Dxe: Allow to force runtime allocations at separate range
> 
> Operating Systems that get hibernated expect all non-boot-time allocations
> to be identical before and after hibernation.
> 
> In edk2, we create pools and allocate pages starting from the highest
> allowed address for the allocation, usually 0xFFFFFFFF. Typically, that
> means we allocate a few pages of boot time data, then a few pages of
> runtime data, then another few pages of boot time data and again runtime
> data. Every allocation has direct impact on the following allocations.
> 
> The problem with this scheme is that small code changes in boot time code
> already can have significant impact on runtime allocations, which then
> break hibernation.
> 
> This patch adds a mechanism to override the MaxAddress for runtime
> allocations with a target defined Pcd value. With this feature enabled,
> we can have different allocation ranges for runtime and boot time
> allocations.
> 
> This allows us to determine at boot time whether to load an old,
> hibernation compatible runtime allocation path or a new, hibernation
> unsafe runtime allocation. All within the same edk2 target binary.
> It also allows us to modify boot time behavior, such as modifying
> buffer allocation mechanisms without compromising on hibernation safety.
> 
> Signed-off-by: Alexander Graf <graf@amazon.com>
> ---
>  MdeModulePkg/Core/Dxe/DxeMain.inf |  4 +++
>  MdeModulePkg/Core/Dxe/Mem/Page.c  | 70 +++++++++++++++++++++++++++++++++++++++
>  MdeModulePkg/MdeModulePkg.dec     | 16 +++++++++
>  MdeModulePkg/MdeModulePkg.uni     | 12 +++++++
>  4 files changed, 102 insertions(+)
> 
> diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf b/MdeModulePkg/Core/Dxe/DxeMain.inf
> index e4bca89577..0696246970 100644
> --- a/MdeModulePkg/Core/Dxe/DxeMain.inf
> +++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
> @@ -186,6 +186,10 @@
>    gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask                   ## CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard                           ## CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFwVolDxeMaxEncapsulationDepth           ## CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdEnforceMaxACPIReclaimMemory             ## CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdEnforceMaxACPIMemoryNVS                 ## CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdEnforceMaxEfiRuntimeServicesCode        ## CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdEnforceMaxEfiRuntimeServicesData        ## CONSUMES
> 
>  # [Hob]
>  # RESOURCE_DESCRIPTOR   ## CONSUMES
> diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c b/MdeModulePkg/Core/Dxe/Mem/Page.c
> index 731bf08bc9..91599adccb 100644
> --- a/MdeModulePkg/Core/Dxe/Mem/Page.c
> +++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
> @@ -1007,6 +1007,74 @@ CoreUpdateMemoryAttributes (
>    CoreReleaseMemoryLock ();
>  }
> 
> +UINT64
> +EnforceMaxAddress (
> +  IN UINT64           MaxAddress,
> +  IN EFI_MEMORY_TYPE  NewType,
> +  IN UINT64           NumberOfPages
> +  )
> +{
> +  UINT64     NumberOfBytes = LShiftU64 (NumberOfPages, EFI_PAGE_SHIFT);
> +  UINT64     LowestPossible = MaxAddress;
> +  UINT64     ForceMaxAddress;
> +  LIST_ENTRY *Link;
> +  MEMORY_MAP *Entry;
> +
> +  switch (NewType) {
> +  case EfiACPIReclaimMemory:
> +    ForceMaxAddress = PcdGet64(PcdEnforceMaxACPIReclaimMemory);
> +    break;
> +  case EfiACPIMemoryNVS:
> +    ForceMaxAddress = PcdGet64(PcdEnforceMaxACPIMemoryNVS);
> +    break;
> +  case EfiRuntimeServicesCode:
> +    ForceMaxAddress = PcdGet64(PcdEnforceMaxEfiRuntimeServicesCode);
> +    break;
> +  case EfiRuntimeServicesData:
> +    ForceMaxAddress = PcdGet64(PcdEnforceMaxEfiRuntimeServicesData);
> +    break;
> +  default:
> +    ForceMaxAddress = MaxAddress;
> +    break;
> +  }
> +
> +  //
> +  // The currently requested address already fits our forced max constraint?
> +  // Great, let's use that then.
> +  //
> +  if (ForceMaxAddress >= MaxAddress) {
> +    return MaxAddress;
> +  }
> +
> +  //
> +  // Check if the allocation would fit. If not, don't force it.
> +  //
> +  for (Link = gMemoryMap.ForwardLink; Link != &gMemoryMap; Link = Link->ForwardLink) {
> +    Entry = CR (Link, MEMORY_MAP, Link, MEMORY_MAP_SIGNATURE);
> +
> +    //
> +    // If it's not a free entry, don't bother with it
> +    //
> +    if (Entry->Type != EfiConventionalMemory) {
> +      continue;
> +    }
> +
> +    if ((Entry->Start < LowestPossible) &&
> +        ((Entry->End - Entry->Start) >= NumberOfBytes)) {
> +      LowestPossible = Entry->End;
> +    }
> +  }
> +  DEBUG ((DEBUG_ERROR | DEBUG_PAGE, "Force=%lx Lowest=%lx Max=%lx\n", ForceMaxAddress, LowestPossible, MaxAddress));
> +
> +  //
> +  // We don't have free RAM available in the desired target area? Bail out!
> +  //
> +  if (ForceMaxAddress < LowestPossible) {
> +    return MaxAddress;
> +  }
> +
> +  return ForceMaxAddress;
> +}
> 
>  /**
>    Internal function. Finds a consecutive free page range below
> @@ -1041,6 +1109,8 @@ CoreFindFreePagesI (
>    LIST_ENTRY      *Link;
>    MEMORY_MAP      *Entry;
> 
> +  MaxAddress = EnforceMaxAddress(MaxAddress, NewType, NumberOfPages);
> +
>    if ((MaxAddress < EFI_PAGE_MASK) ||(NumberOfPages == 0)) {
>      return 0;
>    }
> diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
> index 1483955110..cbad48af5e 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -1535,6 +1535,22 @@
>    # @Prompt Maximum permitted FwVol section nesting depth (exclusive).
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFwVolDxeMaxEncapsulationDepth|0x10|UINT32|0x00000030
> 
> +  ## Maximum address that a dynamic EfiACPIReclaimMemory allocation can be requested at
> +  # @Prompt Maximum address for EfiACPIReclaimMemory allocations
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdEnforceMaxACPIReclaimMemory|0xFFFFFFFFFFFFFFFF|UINT64|0x30001016
> +
> +  ## Maximum address that a dynamic EfiACPIMemoryNVS allocation can be requested at
> +  # @Prompt Maximum address for EfiACPIMemoryNVS allocations
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdEnforceMaxACPIMemoryNVS|0xFFFFFFFFFFFFFFFF|UINT64|0x30001017
> +
> +  ## Maximum address that a dynamic EfiRuntimeServicesCode allocation can be requested at
> +  # @Prompt Maximum address for EfiRuntimeServicesCode allocations
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdEnforceMaxEfiRuntimeServicesCode|0xFFFFFFFFFFFFFFFF|UINT64|0x30001018
> +
> +  ## Maximum address that a dynamic EfiRuntimeServicesData allocation can be requested at
> +  # @Prompt Maximum address for EfiRuntimeServicesData allocations
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdEnforceMaxEfiRuntimeServicesData|0xFFFFFFFFFFFFFFFF|UINT64|0x30001019
> +
>  [PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
>    ## This PCD defines the Console output row. The default value is 25 according to UEFI spec.
>    #  This PCD could be set to 0 then console output would be at max column and max row.
> diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni
> index ef9f4d97b9..0dc5c1960b 100644
> --- a/MdeModulePkg/MdeModulePkg.uni
> +++ b/MdeModulePkg/MdeModulePkg.uni
> @@ -1330,3 +1330,15 @@
>  #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdPcieResizableBarSupport_HELP #language en-US "Indicates if the PCIe
> Resizable BAR Capability Supported.<BR><BR>\n"
>                                                                                              "TRUE  - PCIe Resizable BAR
> Capability is supported.<BR>\n"
>                                                                                              "FALSE - PCIe Resizable BAR
> Capability is not supported.<BR>"
> +
> +#string STR_gEfiMdeModulePkgTokenSpaceGuid.PcdEnforceMaxACPIReclaimMemory_PROMPT #language en-US "Maximum address for
> EfiACPIReclaimMemory allocations"
> +#string STR_gEfiMdeModulePkgTokenSpaceGuid.PcdEnforceMaxACPIReclaimMemory_HELP #language en-US "Maximum address that a
> dynamic EfiACPIReclaimMemory allocation can be requested at"
> +
> +#string STR_gEfiMdeModulePkgTokenSpaceGuid.PcdEnforceMaxACPIMemoryNVS_PROMPT #language en-US "Maximum address for
> EfiACPIMemoryNVS allocations"
> +#string STR_gEfiMdeModulePkgTokenSpaceGuid.PcdEnforceMaxACPIMemoryNVS_HELP #language en-US "Maximum address that a
> dynamic EfiACPIMemoryNVS allocation can be requested at"
> +
> +#string STR_gEfiMdeModulePkgTokenSpaceGuid.PcdEnforceMaxEfiRuntimeServicesCode_PROMPT #language en-US "Maximum address
> for EfiRuntimeServicesCode allocations"
> +#string STR_gEfiMdeModulePkgTokenSpaceGuid.PcdEnforceMaxEfiRuntimeServicesCode_HELP #language en-US "Maximum address that
> a dynamic EfiRuntimeServicesCode allocation can be requested at"
> +
> +#string STR_gEfiMdeModulePkgTokenSpaceGuid.PcdEnforceMaxEfiRuntimeServicesData_PROMPT #language en-US "Maximum address
> for EfiRuntimeServicesData allocations"
> +#string STR_gEfiMdeModulePkgTokenSpaceGuid.PcdEnforceMaxEfiRuntimeServicesData_HELP #language en-US "Maximum address that
> a dynamic EfiRuntimeServicesData allocation can be requested at"
> --
> 2.16.4
> 
> 
> 
> 
> Amazon Development Center Germany GmbH
> Krausenstr. 38
> 10117 Berlin
> Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
> Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
> Sitz: Berlin
> Ust-ID: DE 289 237 879
> 
> 
> 
> 
> 
> 
> 


  reply	other threads:[~2021-02-18 22:28 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-18 20:09 [PATCH 0/2] Improve hibernation safety Alexander Graf
2021-02-18 20:09 ` [PATCH 1/2] MdeModulePkg/Core/Dxe: Allow to force runtime allocations at separate range Alexander Graf
2021-02-18 22:28   ` Michael D Kinney [this message]
2021-02-19 14:10     ` [edk2-devel] " Alexander Graf
2021-02-19 16:47       ` Laszlo Ersek
2021-02-19 17:31         ` Alexander Graf
2021-02-18 20:09 ` [PATCH 2/2] OvmfPkg: Make hibernation critical allocations at own ranges Alexander Graf

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=CO1PR11MB49294F1100253B00127635B0D2859@CO1PR11MB4929.namprd11.prod.outlook.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

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

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