From: "Pankaj Bansal" <pankaj.bansal@nxp.com>
To: "Pankaj Bansal (OSS)" <pankaj.bansal@oss.nxp.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>,
Eric Jin <eric.jin@intel.com>,
G Edhaya Chandran <Edhaya.Chandran@arm.com>
Cc: Pankaj Bansal <pankaj.bansal@nxp.com>,
Paul Yang <Paul.Yang@arm.com>,
Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>,
Gaurav Jain <gaurav.jain@nxp.com>
Subject: Re: [PATCH edk2-test 1/1] SctPkg: fix page alignment calculations
Date: Thu, 23 Jul 2020 13:41:41 +0000 [thread overview]
Message-ID: <VI1PR04MB593352769A5D034F5B82C6D0F1760@VI1PR04MB5933.eurprd04.prod.outlook.com> (raw)
In-Reply-To: <20200704155224.28526-1-pankaj.bansal@oss.nxp.com>
ping!!
> -----Original Message-----
> From: Pankaj Bansal (OSS) <pankaj.bansal@oss.nxp.com>
> Sent: Saturday, July 4, 2020 9:22 PM
> To: devel@edk2.groups.io; Eric Jin <eric.jin@intel.com>; G Edhaya Chandran
> <Edhaya.Chandran@arm.com>
> Cc: Pankaj Bansal <pankaj.bansal@nxp.com>; Paul Yang <Paul.Yang@arm.com>;
> Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; Gaurav Jain
> <gaurav.jain@nxp.com>
> Subject: [PATCH edk2-test 1/1] SctPkg: fix page alignment calculations
>
> From: Pankaj Bansal <pankaj.bansal@nxp.com>
>
> The BBTestAllocatePagesInterfaceTest tries to allocate pages for
> different memory types.
> While doing so, it tries to fix up the Start and PageNum for 64K
> Page size. There are multiple issues with this:
>
> 1. 64K alignment is being done regardless of Processor type and Memory
> type. while this is correct for ARM64 Processor, it might not be so
> for other Processor types. Also 64K alignment for ARM64 Processor
> is needed for some Memory types not all.
> 2. The Start is being incremented by 64K, even if Start is already 64K
> aligned.
> 3. PageNum is being decreased by 16 pages indiscriminately, which might
> not be needed in all cases.
>
> fix all these issues by correctly doing the alignment in all needed
> cases.
>
> Cc: Paul Yang <Paul.Yang@arm.com>
> Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
> Cc: Gaurav Jain <gaurav.jain@nxp.com>
> Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
> ---
> .../MemoryAllocationServicesBBTestFunction.c | 148 +++++++++++++-----
> 1 file changed, 106 insertions(+), 42 deletions(-)
>
> diff --git a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/BootServices/MemoryAllocationServices/BlackB
> oxTest/MemoryAllocationServicesBBTestFunction.c b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/BootServices/MemoryAllocationServices/BlackB
> oxTest/MemoryAllocationServicesBBTestFunction.c
> index d18fe1fc2b94..9ed9e6e0de74 100644
> --- a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/BootServices/MemoryAllocationServices/BlackB
> oxTest/MemoryAllocationServicesBBTestFunction.c
> +++ b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/BootServices/MemoryAllocationServices/BlackB
> oxTest/MemoryAllocationServicesBBTestFunction.c
> @@ -354,6 +354,7 @@ BBTestAllocatePagesInterfaceTest (
> EFI_TPL OldTpl;
> EFI_MEMORY_DESCRIPTOR Descriptor;
> UINTN PageNum;
> + UINTN Alignment;
>
> //
> // Get the Standard Library Interface
> @@ -700,14 +701,23 @@ BBTestAllocatePagesInterfaceTest (
> PageNum = (UINTN)Descriptor.NumberOfPages;
> Start = Descriptor.PhysicalStart;
>
> - //
> - // Some memory types need more alignment than 4K, so
> - //
> - if (PageNum <= 0x10) {
> + Alignment = DEFAULT_PAGE_ALLOCATION_GRANULARITY;
> +
> + if (AllocatePagesMemoryType[TypeIndex] == EfiACPIReclaimMemory ||
> + AllocatePagesMemoryType[TypeIndex] == EfiACPIMemoryNVS ||
> + AllocatePagesMemoryType[TypeIndex] == EfiRuntimeServicesCode ||
> + AllocatePagesMemoryType[TypeIndex] == EfiRuntimeServicesData) {
> +
> + Alignment = RUNTIME_PAGE_ALLOCATION_GRANULARITY;
> + }
> +
> + Start = (Start + Alignment - 1) & ~(Alignment - 1);
> + PageNum -= EFI_SIZE_TO_PAGES (Start - Descriptor.PhysicalStart);
> +
> + PageNum &= ~(EFI_SIZE_TO_PAGES (Alignment) - 1);
> + if (PageNum <= EFI_SIZE_TO_PAGES (Alignment)) {
> break;
> }
> - Start = (Start + 0x10000) & 0xFFFFFFFFFFFF0000;
> - PageNum = PageNum - EFI_SIZE_TO_PAGES(0x10000);
>
> Memory = Start;
>
> @@ -830,14 +840,23 @@ BBTestAllocatePagesInterfaceTest (
> PageNum = (UINTN)Descriptor.NumberOfPages;
> Start = Descriptor.PhysicalStart;
>
> - //
> - // Some memory types need more alignment than 4K, so
> - //
> - if (PageNum <= 0x10) {
> + Alignment = DEFAULT_PAGE_ALLOCATION_GRANULARITY;
> +
> + if (AllocatePagesMemoryType[TypeIndex] == EfiACPIReclaimMemory ||
> + AllocatePagesMemoryType[TypeIndex] == EfiACPIMemoryNVS ||
> + AllocatePagesMemoryType[TypeIndex] == EfiRuntimeServicesCode ||
> + AllocatePagesMemoryType[TypeIndex] == EfiRuntimeServicesData) {
> +
> + Alignment = RUNTIME_PAGE_ALLOCATION_GRANULARITY;
> + }
> +
> + Start = (Start + Alignment - 1) & ~(Alignment - 1);
> + PageNum -= EFI_SIZE_TO_PAGES (Start - Descriptor.PhysicalStart);
> +
> + PageNum &= ~(EFI_SIZE_TO_PAGES (Alignment) - 1);
> + if (PageNum <= EFI_SIZE_TO_PAGES (Alignment)) {
> break;
> }
> - Start = (Start + 0x10000) & 0xFFFFFFFFFFFF0000;
> - PageNum = PageNum - EFI_SIZE_TO_PAGES(0x10000);
>
> Memory = Start;
>
> @@ -953,14 +972,23 @@ BBTestAllocatePagesInterfaceTest (
> PageNum = (UINTN)Descriptor.NumberOfPages;
> Start = Descriptor.PhysicalStart;
>
> - //
> - // Some memory types need more alignment than 4K, so
> - //
> - if (PageNum <= 0x10) {
> + Alignment = DEFAULT_PAGE_ALLOCATION_GRANULARITY;
> +
> + if (AllocatePagesMemoryType[TypeIndex] == EfiACPIReclaimMemory ||
> + AllocatePagesMemoryType[TypeIndex] == EfiACPIMemoryNVS ||
> + AllocatePagesMemoryType[TypeIndex] == EfiRuntimeServicesCode ||
> + AllocatePagesMemoryType[TypeIndex] == EfiRuntimeServicesData) {
> +
> + Alignment = RUNTIME_PAGE_ALLOCATION_GRANULARITY;
> + }
> +
> + Start = (Start + Alignment - 1) & ~(Alignment - 1);
> + PageNum -= EFI_SIZE_TO_PAGES (Start - Descriptor.PhysicalStart);
> +
> + PageNum &= ~(EFI_SIZE_TO_PAGES (Alignment) - 1);
> + if (PageNum <= EFI_SIZE_TO_PAGES (Alignment)) {
> break;
> }
> - Start = (Start + 0x10000) & 0xFFFFFFFFFFFF0000;
> - PageNum = PageNum - EFI_SIZE_TO_PAGES(0x10000);
>
> Memory = Start + (SctLShiftU64 (PageNum/3, EFI_PAGE_SHIFT) &
> 0xFFFFFFFFFFFF0000);
>
> @@ -1076,14 +1104,23 @@ BBTestAllocatePagesInterfaceTest (
> PageNum = (UINTN)Descriptor.NumberOfPages;
> Start = Descriptor.PhysicalStart;
>
> - //
> - // Some memory types need more alignment than 4K, so
> - //
> - if (PageNum <= 0x10) {
> + Alignment = DEFAULT_PAGE_ALLOCATION_GRANULARITY;
> +
> + if (AllocatePagesMemoryType[TypeIndex] == EfiACPIReclaimMemory ||
> + AllocatePagesMemoryType[TypeIndex] == EfiACPIMemoryNVS ||
> + AllocatePagesMemoryType[TypeIndex] == EfiRuntimeServicesCode ||
> + AllocatePagesMemoryType[TypeIndex] == EfiRuntimeServicesData) {
> +
> + Alignment = RUNTIME_PAGE_ALLOCATION_GRANULARITY;
> + }
> +
> + Start = (Start + Alignment - 1) & ~(Alignment - 1);
> + PageNum -= EFI_SIZE_TO_PAGES (Start - Descriptor.PhysicalStart);
> +
> + PageNum &= ~(EFI_SIZE_TO_PAGES (Alignment) - 1);
> + if (PageNum <= EFI_SIZE_TO_PAGES (Alignment)) {
> break;
> }
> - Start = (Start + 0x10000) & 0xFFFFFFFFFFFF0000;
> - PageNum = PageNum - EFI_SIZE_TO_PAGES(0x10000);
>
> Memory = Start + (SctLShiftU64 (PageNum * 2 / 3, EFI_PAGE_SHIFT) &
> 0xFFFFFFFFFFFF0000);
>
> @@ -1206,14 +1243,23 @@ BBTestAllocatePagesInterfaceTest (
> PageNum = (UINTN)Descriptor.NumberOfPages;
> Start = Descriptor.PhysicalStart;
>
> - //
> - // Some memory types need more alignment than 4K, so
> - //
> - if (PageNum <= 0x10) {
> + Alignment = DEFAULT_PAGE_ALLOCATION_GRANULARITY;
> +
> + if (AllocatePagesMemoryType[TypeIndex] == EfiACPIReclaimMemory ||
> + AllocatePagesMemoryType[TypeIndex] == EfiACPIMemoryNVS ||
> + AllocatePagesMemoryType[TypeIndex] == EfiRuntimeServicesCode ||
> + AllocatePagesMemoryType[TypeIndex] == EfiRuntimeServicesData) {
> +
> + Alignment = RUNTIME_PAGE_ALLOCATION_GRANULARITY;
> + }
> +
> + Start = (Start + Alignment - 1) & ~(Alignment - 1);
> + PageNum -= EFI_SIZE_TO_PAGES (Start - Descriptor.PhysicalStart);
> +
> + PageNum &= ~(EFI_SIZE_TO_PAGES (Alignment) - 1);
> + if (PageNum <= EFI_SIZE_TO_PAGES (Alignment)) {
> break;
> }
> - Start = (Start + 0x10000) & 0xFFFFFFFFFFFF0000;
> - PageNum = PageNum - EFI_SIZE_TO_PAGES(0x10000);
>
> Memory = Start;
>
> @@ -1329,14 +1375,23 @@ BBTestAllocatePagesInterfaceTest (
> PageNum = (UINTN)Descriptor.NumberOfPages;
> Start = Descriptor.PhysicalStart;
>
> - //
> - // Some memory types need more alignment than 4K, so
> - //
> - if (PageNum <= 0x10) {
> + Alignment = DEFAULT_PAGE_ALLOCATION_GRANULARITY;
> +
> + if (AllocatePagesMemoryType[TypeIndex] == EfiACPIReclaimMemory ||
> + AllocatePagesMemoryType[TypeIndex] == EfiACPIMemoryNVS ||
> + AllocatePagesMemoryType[TypeIndex] == EfiRuntimeServicesCode ||
> + AllocatePagesMemoryType[TypeIndex] == EfiRuntimeServicesData) {
> +
> + Alignment = RUNTIME_PAGE_ALLOCATION_GRANULARITY;
> + }
> +
> + Start = (Start + Alignment - 1) & ~(Alignment - 1);
> + PageNum -= EFI_SIZE_TO_PAGES (Start - Descriptor.PhysicalStart);
> +
> + PageNum &= ~(EFI_SIZE_TO_PAGES (Alignment) - 1);
> + if (PageNum <= EFI_SIZE_TO_PAGES (Alignment)) {
> break;
> }
> - Start = (Start + 0x10000) & 0xFFFFFFFFFFFF0000;
> - PageNum = PageNum - EFI_SIZE_TO_PAGES(0x10000);
>
> Memory = Start;
>
> @@ -1468,14 +1523,23 @@ BBTestAllocatePagesInterfaceTest (
> PageNum = (UINTN)Descriptor.NumberOfPages;
> Start = Descriptor.PhysicalStart;
>
> - //
> - // Some memory types need more alignment than 4K, so
> - //
> - if (PageNum <= 0x10) {
> + Alignment = DEFAULT_PAGE_ALLOCATION_GRANULARITY;
> +
> + if (AllocatePagesMemoryType[TypeIndex] == EfiACPIReclaimMemory ||
> + AllocatePagesMemoryType[TypeIndex] == EfiACPIMemoryNVS ||
> + AllocatePagesMemoryType[TypeIndex] == EfiRuntimeServicesCode ||
> + AllocatePagesMemoryType[TypeIndex] == EfiRuntimeServicesData) {
> +
> + Alignment = RUNTIME_PAGE_ALLOCATION_GRANULARITY;
> + }
> +
> + Start = (Start + Alignment - 1) & ~(Alignment - 1);
> + PageNum -= EFI_SIZE_TO_PAGES (Start - Descriptor.PhysicalStart);
> +
> + PageNum &= ~(EFI_SIZE_TO_PAGES (Alignment) - 1);
> + if (PageNum <= EFI_SIZE_TO_PAGES (Alignment)) {
> break;
> }
> - Start = (Start + 0x10000) & 0xFFFFFFFFFFFF0000;
> - PageNum = PageNum - EFI_SIZE_TO_PAGES(0x10000);
>
> Memory = Start;
>
> --
> 2.17.1
next prev parent reply other threads:[~2020-07-23 13:41 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-04 15:52 [PATCH edk2-test 1/1] SctPkg: fix page alignment calculations Pankaj Bansal
2020-07-23 13:41 ` Pankaj Bansal [this message]
2020-07-24 2:32 ` Samer El-Haj-Mahmoud
[not found] ` <16248F836B15C59B.28930@groups.io>
2020-08-04 11:28 ` [edk2-devel] " Samer El-Haj-Mahmoud
2020-08-04 16:56 ` G Edhaya Chandran
2020-08-12 13:25 ` 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=VI1PR04MB593352769A5D034F5B82C6D0F1760@VI1PR04MB5933.eurprd04.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