public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
To: "Yao, Jiewen" <jiewen.yao@intel.com>
Cc: "edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
	"Gao, Liming" <liming.gao@intel.com>,
	"Zeng, Star" <star.zeng@intel.com>
Subject: Re: [PATCH v3] MdeModulePkg/PeiCore: honour minimal runtime allocation granularity
Date: Fri, 3 Mar 2017 13:42:10 +0000	[thread overview]
Message-ID: <CAKv+Gu95mBR_1iEB+aBzd8wLEJfA3oyax6qN2tcUh3hy7vQW-g@mail.gmail.com> (raw)
In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C503A8F904F@shsmsx102.ccr.corp.intel.com>

On 3 March 2017 at 13:40, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> Hi Ard
> Thanks to close the gap in PEI memory allocation.
>
> I found we use below in DXE core. The definition is slight different with the one you add in PEI core.
>
> Since the calling convention is defined in UEFI spec, do you think it will be better if we can move below to MdePkg?
> Maybe in ProcessorBind.h, because we already define CPU_STACK_ALIGNMENT there.
>

Yes, that makes sense. I will create a patch for that.

> The benefit is that we do not need duplicate the same definition in PEI core.
> In the future, new ARCH can also define same thing there, instead of checking DXE/PEI core.
>
> =========================================================
> #if defined (MDE_CPU_IPF)
> ///
> /// For Itanium machines make the default allocations 8K aligned
> ///
> #define EFI_ACPI_RUNTIME_PAGE_ALLOCATION_ALIGNMENT  (EFI_PAGE_SIZE * 2)
> #define DEFAULT_PAGE_ALLOCATION                     (EFI_PAGE_SIZE * 2)
>
> #elif defined (MDE_CPU_AARCH64)
> ///
> /// 64-bit ARM systems allow the OS to execute with 64 KB page size,
> /// so for improved interoperability with the firmware, align the
> /// runtime regions to 64 KB as well
> ///
> #define EFI_ACPI_RUNTIME_PAGE_ALLOCATION_ALIGNMENT  (SIZE_64KB)
> #define DEFAULT_PAGE_ALLOCATION                     (EFI_PAGE_SIZE)
>
> #else
> ///
> /// For generic EFI machines make the default allocations 4K aligned
> ///
> #define EFI_ACPI_RUNTIME_PAGE_ALLOCATION_ALIGNMENT  (EFI_PAGE_SIZE)
> #define DEFAULT_PAGE_ALLOCATION                     (EFI_PAGE_SIZE)
>
> #endif
> =========================================================
>
>
>
>
>> -----Original Message-----
>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard
>> Biesheuvel
>> Sent: Friday, March 3, 2017 5:44 PM
>> To: edk2-devel@lists.01.org; Gao, Liming <liming.gao@intel.com>; Zeng, Star
>> <star.zeng@intel.com>
>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Subject: [edk2] [PATCH v3] MdeModulePkg/PeiCore: honour minimal runtime
>> allocation granularity
>>
>> Architectures such as AArch64 may run the OS with 16 KB or 64 KB sized
>> pages, and for this reason, the UEFI spec mandates a minimal allocation
>> granularity of 64 KB for regions that may require different memory
>> attributes at OS runtime.
>>
>> So make PeiCore's implementation of AllocatePages () take this into
>> account as well.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>> v3: allocate a memory allocation HOB to cover the memory lost to rounding
>>
>>  MdeModulePkg/Core/Pei/Memory/MemoryServices.c | 39
>> +++++++++++++++++++-
>>  MdeModulePkg/Core/Pei/PeiMain.h               | 18 +++++++++
>>  2 files changed, 55 insertions(+), 2 deletions(-)
>>
>> diff --git a/MdeModulePkg/Core/Pei/Memory/MemoryServices.c
>> b/MdeModulePkg/Core/Pei/Memory/MemoryServices.c
>> index 4efe14313ca5..573fd606b4cc 100644
>> --- a/MdeModulePkg/Core/Pei/Memory/MemoryServices.c
>> +++ b/MdeModulePkg/Core/Pei/Memory/MemoryServices.c
>> @@ -140,6 +140,8 @@ PeiAllocatePages (
>>    EFI_PHYSICAL_ADDRESS                    *FreeMemoryTop;
>>    EFI_PHYSICAL_ADDRESS                    *FreeMemoryBottom;
>>    UINTN                                   RemainingPages;
>> +  UINTN                                   Granularity;
>> +  UINTN                                   Padding;
>>
>>    if ((MemoryType != EfiLoaderCode) &&
>>        (MemoryType != EfiLoaderData) &&
>> @@ -153,6 +155,20 @@ PeiAllocatePages (
>>      return EFI_INVALID_PARAMETER;
>>    }
>>
>> +  Granularity = DEFAULT_PAGE_ALLOCATION_GRANULARITY;
>> +
>> +  if  (RUNTIME_PAGE_ALLOCATION_GRANULARITY >
>> DEFAULT_PAGE_ALLOCATION_GRANULARITY &&
>> +       (MemoryType == EfiACPIReclaimMemory   ||
>> +        MemoryType == EfiACPIMemoryNVS       ||
>> +        MemoryType == EfiRuntimeServicesCode ||
>> +        MemoryType == EfiRuntimeServicesData)) {
>> +
>> +    Granularity = RUNTIME_PAGE_ALLOCATION_GRANULARITY;
>> +
>> +    DEBUG ((DEBUG_INFO, "AllocatePages: rounding up allocation to %d
>> KB\n",
>> +      Granularity));
>> +  }
>> +
>>    PrivateData = PEI_CORE_INSTANCE_FROM_PS_THIS (PeiServices);
>>    Hob.Raw     = PrivateData->HobList.Raw;
>>
>> @@ -176,9 +192,27 @@ PeiAllocatePages (
>>    }
>>
>>    //
>> -  // Check to see if on 4k boundary, If not aligned, make the allocation aligned.
>> +  // Check to see if on correct boundary for the memory type.
>> +  // If not aligned, make the allocation aligned.
>>    //
>> -  *(FreeMemoryTop) -= *(FreeMemoryTop) & 0xFFF;
>> +  Padding = *(FreeMemoryTop) & (Granularity - 1);
>> +  if ((UINTN) (*FreeMemoryTop - *FreeMemoryBottom) < Padding) {
>> +    DEBUG ((DEBUG_ERROR, "AllocatePages failed: Out of space after
>> padding.\n"));
>> +    return EFI_OUT_OF_RESOURCES;
>> +  }
>> +
>> +  *(FreeMemoryTop) -= Padding;
>> +  if (Padding >= EFI_PAGE_SIZE) {
>> +    //
>> +    // Create a memory allocation HOB to cover
>> +    // the pages that we will lose to rounding
>> +    //
>> +    BuildMemoryAllocationHob (
>> +      *(FreeMemoryTop),
>> +      Padding & ~((UINTN)EFI_PAGE_SIZE - 1),
>> +      EfiConventionalMemory
>> +      );
>> +  }
>>
>>    //
>>    // Verify that there is sufficient memory to satisfy the allocation.
>> @@ -192,6 +226,7 @@ PeiAllocatePages (
>>    //
>>    // The number of remaining pages needs to be greater than or equal to that
>> of the request pages.
>>    //
>> +  Pages = ALIGN_VALUE (Pages, EFI_SIZE_TO_PAGES (Granularity));
>>    if (RemainingPages < Pages) {
>>      DEBUG ((EFI_D_ERROR, "AllocatePages failed: No 0x%lx Pages is
>> available.\n", (UINT64) Pages));
>>      DEBUG ((EFI_D_ERROR, "There is only left 0x%lx pages memory resource to
>> be allocated.\n", (UINT64) RemainingPages));
>> diff --git a/MdeModulePkg/Core/Pei/PeiMain.h
>> b/MdeModulePkg/Core/Pei/PeiMain.h
>> index 69eea514920b..e8358d3c4e6d 100644
>> --- a/MdeModulePkg/Core/Pei/PeiMain.h
>> +++ b/MdeModulePkg/Core/Pei/PeiMain.h
>> @@ -55,6 +55,24 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
>> KIND, EITHER EXPRESS OR IMPLIED.
>>  ///
>>  #define PEI_CORE_INTERNAL_FFS_FILE_DISPATCH_TYPE   0xff
>>
>> +#if defined (MDE_CPU_AARCH64)
>> +///
>> +/// 64-bit ARM systems allow the OS to execute with 64 KB page size,
>> +/// so for improved interoperability with the firmware, align the
>> +/// runtime regions to 64 KB as well
>> +///
>> +#define RUNTIME_PAGE_ALLOCATION_GRANULARITY       (SIZE_64KB)
>> +#define DEFAULT_PAGE_ALLOCATION_GRANULARITY       (EFI_PAGE_SIZE)
>> +
>> +#else
>> +///
>> +/// For generic EFI machines make the default allocations 4K aligned
>> +///
>> +#define RUNTIME_PAGE_ALLOCATION_GRANULARITY       (EFI_PAGE_SIZE)
>> +#define DEFAULT_PAGE_ALLOCATION_GRANULARITY       (EFI_PAGE_SIZE)
>> +
>> +#endif
>> +
>>  ///
>>  /// Pei Core private data structures
>>  ///
>> --
>> 2.7.4
>>
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel


      reply	other threads:[~2017-03-03 13:42 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-03  9:43 [PATCH v3] MdeModulePkg/PeiCore: honour minimal runtime allocation granularity Ard Biesheuvel
2017-03-03  9:59 ` Ard Biesheuvel
2017-03-03 10:07   ` Zeng, Star
2017-03-03 13:40 ` Yao, Jiewen
2017-03-03 13:42   ` Ard Biesheuvel [this message]

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=CAKv+Gu95mBR_1iEB+aBzd8wLEJfA3oyax6qN2tcUh3hy7vQW-g@mail.gmail.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