From: tuanphan@os.amperecomputing.com
To: devel@edk2.groups.io, Sunny Wang <Sunny.Wang@arm.com>
Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>,
G Edhaya Chandran <edhaya.chandran@arm.com>,
Barton Gao <gaojie@byosoft.com.cn>
Subject: Re: [edk2-devel] [edk2-test][PATCH v1 1/1] uefi-sct/SctPkg: Update page alignment calculations
Date: Tue, 3 Aug 2021 14:40:22 -0700 [thread overview]
Message-ID: <553E56A0-3016-4B22-88E6-AEBF0F0B17EF@amperemail.onmicrosoft.com> (raw)
In-Reply-To: <20210719080759.824-1-Sunny.Wang@arm.com>
[-- Attachment #1: Type: text/plain, Size: 10032 bytes --]
Reviewed-By: Tuan Phan <tuanphan@os.amperecomputing.com>
> On Jul 19, 2021, at 1:07 AM, Sunny Wang via groups.io <Sunny.Wang=arm.com@groups.io> wrote:
>
> This is to fix the SCT BS.AllocatePages failures (not found) with the
> case that the Start address is not aligned to 64k.
> For example,
> The following is available memory region for testing:
> 0000000082012000-00000000EB6D9FFF 00000000000696C8
> With the current page alignment calculation, we will get:
> Start address is 0x82020000
> PageNum is 0x696B8
> In BS.AllocatePages, it will make the end address align with 64k,
> so PageNum will be changed from 0x696B8 to 0x696C0. Therefore, the
> end address will become 0xEB6E0000 which is larger than 0xEB6D9FFF,
> so we get not found error in the end.
>
> Therefore, the calculation for getting the PageNum should be updated
> to PageNum - (2 * EFI_SIZE_TO_PAGES(0x10000)) so that we won't get a
> wrong PageNum to allocate a memory with a size larger than available
> space's size.
>
> With this solution, the example above will get 0x696A8 as calculated
> PageNum. Then, in BS.AllocatePages, the PageNum will be changed from
> 0x696A8 to 0x696B0. Therefore, the end address will become 0xEB6D0000
> that is smaller than 0xEB6D9FFF, so we get not found error in the end.
>
> I also tested this solution on two ARM platforms (NXP1046A and RPi4).
>
> Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>
> Cc: G Edhaya Chandran <edhaya.chandran@arm.com>
> Cc: Barton Gao <gaojie@byosoft.com.cn>
> Signed-off-by: Sunny Wang <sunny.wang@arm.com>
> ---
> .../MemoryAllocationServicesBBTestFunction.c | 110 +++++++++++-------
> 1 file changed, 66 insertions(+), 44 deletions(-)
>
> diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/MemoryAllocationServices/BlackBoxTest/MemoryAllocationServicesBBTestFunction.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/MemoryAllocationServices/BlackBoxTest/MemoryAllocationServicesBBTestFunction.c
> index bf8cd3b3..cdfac992 100644
> --- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/MemoryAllocationServices/BlackBoxTest/MemoryAllocationServicesBBTestFunction.c
> +++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/MemoryAllocationServices/BlackBoxTest/MemoryAllocationServicesBBTestFunction.c
> @@ -2,6 +2,7 @@
>
> Copyright 2006 - 2013 Unified EFI, Inc.<BR>
> Copyright (c) 2010 - 2013, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2021, ARM Limited. All rights reserved.
>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD License
> @@ -24,7 +25,7 @@ Abstract:
>
> --*/
>
> -#include "SctLib.h"
> +#include "SctLib.h"
> #include "MemoryAllocationServicesBBTestMain.h"
>
> #define ALLOCATEPAGES_MEMORYTYPE_NUM 16
> @@ -700,14 +701,17 @@ BBTestAllocatePagesInterfaceTest (
> PageNum = (UINTN)Descriptor.NumberOfPages;
> Start = Descriptor.PhysicalStart;
>
> - //
> - // Some memory types need more alignment than 4K, so
> - //
> - if (PageNum <= 0x10) {
> + //
> + // Calculate New Start address and PageNum with 64k alignment to
> + // cover the case that some memory types' alignment is more than
> + // 4k. If the available memory is less than 192k, the memory
> + // allocation call will be skipped.
> + //
> + if (PageNum < (3 * EFI_SIZE_TO_PAGES(0x10000))) {
> break;
> }
> - Start = (Start + 0xFFFF) & 0xFFFFFFFFFFFF0000;
> - PageNum = PageNum - EFI_SIZE_TO_PAGES(0x10000);
> + Start = (Start + 0xFFFF) & 0xFFFFFFFFFFFF0000;
> + PageNum = PageNum - (2 * EFI_SIZE_TO_PAGES(0x10000));
>
> Memory = Start;
>
> @@ -830,14 +834,17 @@ BBTestAllocatePagesInterfaceTest (
> PageNum = (UINTN)Descriptor.NumberOfPages;
> Start = Descriptor.PhysicalStart;
>
> - //
> - // Some memory types need more alignment than 4K, so
> - //
> - if (PageNum <= 0x10) {
> + //
> + // Calculate New Start address and PageNum with 64k alignment to
> + // cover the case that some memory types' alignment is more than
> + // 4k. If the available memory is less than 192k, the memory
> + // allocation call will be skipped.
> + //
> + if (PageNum < (3 * EFI_SIZE_TO_PAGES(0x10000))) {
> break;
> }
> - Start = (Start + 0xFFFF) & 0xFFFFFFFFFFFF0000;
> - PageNum = PageNum - EFI_SIZE_TO_PAGES(0x10000);
> + Start = (Start + 0xFFFF) & 0xFFFFFFFFFFFF0000;
> + PageNum = PageNum - (2 * EFI_SIZE_TO_PAGES(0x10000));
>
> Memory = Start;
>
> @@ -953,14 +960,17 @@ BBTestAllocatePagesInterfaceTest (
> PageNum = (UINTN)Descriptor.NumberOfPages;
> Start = Descriptor.PhysicalStart;
>
> - //
> - // Some memory types need more alignment than 4K, so
> - //
> - if (PageNum <= 0x10) {
> + //
> + // Calculate New Start address and PageNum with 64k alignment to
> + // cover the case that some memory types' alignment is more than
> + // 4k. If the available memory is less than 192k, the memory
> + // allocation call will be skipped.
> + //
> + if (PageNum < (3 * EFI_SIZE_TO_PAGES(0x10000))) {
> break;
> }
> - Start = (Start + 0xFFFF) & 0xFFFFFFFFFFFF0000;
> - PageNum = PageNum - EFI_SIZE_TO_PAGES(0x10000);
> + Start = (Start + 0xFFFF) & 0xFFFFFFFFFFFF0000;
> + PageNum = PageNum - (2 * EFI_SIZE_TO_PAGES(0x10000));
>
> Memory = Start + (SctLShiftU64 (PageNum/3, EFI_PAGE_SHIFT) & 0xFFFFFFFFFFFF0000);
>
> @@ -1076,14 +1086,17 @@ BBTestAllocatePagesInterfaceTest (
> PageNum = (UINTN)Descriptor.NumberOfPages;
> Start = Descriptor.PhysicalStart;
>
> - //
> - // Some memory types need more alignment than 4K, so
> - //
> - if (PageNum <= 0x10) {
> + //
> + // Calculate New Start address and PageNum with 64k alignment to
> + // cover the case that some memory types' alignment is more than
> + // 4k. If the available memory is less than 192k, the memory
> + // allocation call will be skipped.
> + //
> + if (PageNum < (3 * EFI_SIZE_TO_PAGES(0x10000))) {
> break;
> }
> - Start = (Start + 0xFFFF) & 0xFFFFFFFFFFFF0000;
> - PageNum = PageNum - EFI_SIZE_TO_PAGES(0x10000);
> + Start = (Start + 0xFFFF) & 0xFFFFFFFFFFFF0000;
> + PageNum = PageNum - (2 * EFI_SIZE_TO_PAGES(0x10000));
>
> Memory = Start + (SctLShiftU64 (PageNum * 2 / 3, EFI_PAGE_SHIFT) & 0xFFFFFFFFFFFF0000);
>
> @@ -1206,14 +1219,17 @@ BBTestAllocatePagesInterfaceTest (
> PageNum = (UINTN)Descriptor.NumberOfPages;
> Start = Descriptor.PhysicalStart;
>
> - //
> - // Some memory types need more alignment than 4K, so
> - //
> - if (PageNum <= 0x10) {
> + //
> + // Calculate New Start address and PageNum with 64k alignment to
> + // cover the case that some memory types' alignment is more than
> + // 4k. If the available memory is less than 192k, the memory
> + // allocation call will be skipped.
> + //
> + if (PageNum < (3 * EFI_SIZE_TO_PAGES(0x10000))) {
> break;
> }
> - Start = (Start + 0xFFFF) & 0xFFFFFFFFFFFF0000;
> - PageNum = PageNum - EFI_SIZE_TO_PAGES(0x10000);
> + Start = (Start + 0xFFFF) & 0xFFFFFFFFFFFF0000;
> + PageNum = PageNum - (2 * EFI_SIZE_TO_PAGES(0x10000));
>
> Memory = Start;
>
> @@ -1329,14 +1345,17 @@ BBTestAllocatePagesInterfaceTest (
> PageNum = (UINTN)Descriptor.NumberOfPages;
> Start = Descriptor.PhysicalStart;
>
> - //
> - // Some memory types need more alignment than 4K, so
> - //
> - if (PageNum <= 0x10) {
> + //
> + // Calculate New Start address and PageNum with 64k alignment to
> + // cover the case that some memory types' alignment is more than
> + // 4k. If the available memory is less than 192k, the memory
> + // allocation call will be skipped.
> + //
> + if (PageNum < (3 * EFI_SIZE_TO_PAGES(0x10000))) {
> break;
> }
> - Start = (Start + 0xFFFF) & 0xFFFFFFFFFFFF0000;
> - PageNum = PageNum - EFI_SIZE_TO_PAGES(0x10000);
> + Start = (Start + 0xFFFF) & 0xFFFFFFFFFFFF0000;
> + PageNum = PageNum - (2 * EFI_SIZE_TO_PAGES(0x10000));
>
> Memory = Start;
>
> @@ -1468,14 +1487,17 @@ BBTestAllocatePagesInterfaceTest (
> PageNum = (UINTN)Descriptor.NumberOfPages;
> Start = Descriptor.PhysicalStart;
>
> - //
> - // Some memory types need more alignment than 4K, so
> - //
> - if (PageNum <= 0x10) {
> + //
> + // Calculate New Start address and PageNum with 64k alignment to
> + // cover the case that some memory types' alignment is more than
> + // 4k. If the available memory is less than 192k, the memory
> + // allocation call will be skipped.
> + //
> + if (PageNum < (3 * EFI_SIZE_TO_PAGES(0x10000))) {
> break;
> }
> - Start = (Start + 0xFFFF) & 0xFFFFFFFFFFFF0000;
> - PageNum = PageNum - EFI_SIZE_TO_PAGES(0x10000);
> + Start = (Start + 0xFFFF) & 0xFFFFFFFFFFFF0000;
> + PageNum = PageNum - (2 * EFI_SIZE_TO_PAGES(0x10000));
>
> Memory = Start;
>
> @@ -1923,4 +1945,4 @@ BBTestFreePoolInterfaceTest (
>
> FreeMemoryMap ();
> return EFI_SUCCESS;
> -}
> +}
> --
> 2.31.0.windows.1
>
>
>
>
>
>
[-- Attachment #2: Type: text/html, Size: 18784 bytes --]
next prev parent reply other threads:[~2021-08-03 21:40 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-19 8:07 [edk2-test][PATCH v1 1/1] uefi-sct/SctPkg: Update page alignment calculations Sunny Wang
2021-07-27 6:12 ` [edk2-devel] " G Edhaya Chandran
2021-08-03 13:10 ` Samer El-Haj-Mahmoud
2021-08-03 21:40 ` tuanphan [this message]
[not found] ` <DB8PR08MB3993FC8E3E4847842808B1C185F19@DB8PR08MB3993.eurprd08.prod.outlook.com>
2021-08-04 8:13 ` Paul Yang
2021-08-09 10:35 ` [edk2-devel] " G Edhaya Chandran
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=553E56A0-3016-4B22-88E6-AEBF0F0B17EF@amperemail.onmicrosoft.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