public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>, edk2-devel@ml01.01.org
Cc: leif.lindholm@linaro.org, heyi.guo@linaro.org, ashedel@microsoft.com
Subject: Re: [PATCH] ArmPkg/ArmMmuLib: Revert "use a pool allocation for the root table"
Date: Fri, 20 Jan 2017 18:43:31 +0100	[thread overview]
Message-ID: <41af2323-60d6-e494-553e-270fae030c0e@redhat.com> (raw)
In-Reply-To: <1484931946-11648-1-git-send-email-ard.biesheuvel@linaro.org>

On 01/20/17 18:05, Ard Biesheuvel wrote:
> This reverts commit d32702d2c2aa23e828363a7f88829b78ce36c3af.
> 
> Using a pool allocation for the root translation table seemed like
> a good idea at the time, but as it turns out, such allocations are
> handled in a way that makes them unsuitable for this purpose: they
> are backed by HOBs that don't remain in the same place during the
> various PI phase changes, which means the address programmed into
> the TTBR register is no longer valid, and may refer to memory that
> is reported as available to the OS.
> 
> So switch back to using a page based allocation.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 29 ++++----------------
>  1 file changed, 6 insertions(+), 23 deletions(-)

Acked-by: Laszlo Ersek <lersek@redhat.com>

> diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> index c78297084207..540069a59b2e 100644
> --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> @@ -553,12 +553,10 @@ ArmConfigureMmu (
>    )
>  {
>    VOID*                         TranslationTable;
> -  VOID*                         TranslationTableBuffer;
>    UINT32                        TranslationTableAttribute;
>    UINT64                        MaxAddress;
>    UINTN                         T0SZ;
>    UINTN                         RootTableEntryCount;
> -  UINTN                         RootTableEntrySize;
>    UINT64                        TCR;
>    RETURN_STATUS                 Status;
>  
> @@ -643,19 +641,8 @@ ArmConfigureMmu (
>    // Set TCR
>    ArmSetTCR (TCR);
>  
> -  // Allocate pages for translation table. Pool allocations are 8 byte aligned,
> -  // but we may require a higher alignment based on the size of the root table.
> -  RootTableEntrySize = RootTableEntryCount * sizeof(UINT64);
> -  if (RootTableEntrySize < EFI_PAGE_SIZE / 2) {
> -    TranslationTableBuffer = AllocatePool (2 * RootTableEntrySize - 8);
> -    //
> -    // Naturally align the root table. Preserves possible NULL value
> -    //
> -    TranslationTable = (VOID *)((UINTN)(TranslationTableBuffer - 1) | (RootTableEntrySize - 1)) + 1;
> -  } else {
> -    TranslationTable = AllocatePages (1);
> -    TranslationTableBuffer = NULL;
> -  }
> +  // Allocate pages for translation table
> +  TranslationTable = AllocatePages (1);
>    if (TranslationTable == NULL) {
>      return RETURN_OUT_OF_RESOURCES;
>    }
> @@ -669,10 +656,10 @@ ArmConfigureMmu (
>    }
>  
>    if (TranslationTableSize != NULL) {
> -    *TranslationTableSize = RootTableEntrySize;
> +    *TranslationTableSize = RootTableEntryCount * sizeof(UINT64);
>    }
>  
> -  ZeroMem (TranslationTable, RootTableEntrySize);
> +  ZeroMem (TranslationTable, RootTableEntryCount * sizeof(UINT64));
>  
>    // Disable MMU and caches. ArmDisableMmu() also invalidates the TLBs
>    ArmDisableMmu ();
> @@ -689,7 +676,7 @@ ArmConfigureMmu (
>      DEBUG_CODE_BEGIN ();
>        // Find the memory attribute for the Translation Table
>        if ((UINTN)TranslationTable >= MemoryTable->PhysicalBase &&
> -          (UINTN)TranslationTable + RootTableEntrySize <= MemoryTable->PhysicalBase +
> +          (UINTN)TranslationTable + EFI_PAGE_SIZE <= MemoryTable->PhysicalBase +
>                                                            MemoryTable->Length) {
>          TranslationTableAttribute = MemoryTable->Attributes;
>        }
> @@ -718,11 +705,7 @@ ArmConfigureMmu (
>    return RETURN_SUCCESS;
>  
>  FREE_TRANSLATION_TABLE:
> -  if (TranslationTableBuffer != NULL) {
> -    FreePool (TranslationTableBuffer);
> -  } else {
> -    FreePages (TranslationTable, 1);
> -  }
> +  FreePages (TranslationTable, 1);
>    return Status;
>  }
>  
> 



  reply	other threads:[~2017-01-20 17:43 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-20 17:05 [PATCH] ArmPkg/ArmMmuLib: Revert "use a pool allocation for the root table" Ard Biesheuvel
2017-01-20 17:43 ` Laszlo Ersek [this message]
2017-01-20 17:49 ` Leif Lindholm
2017-01-20 17:52   ` Ard Biesheuvel

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=41af2323-60d6-e494-553e-270fae030c0e@redhat.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