From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-x232.google.com (mail-wm0-x232.google.com [IPv6:2a00:1450:400c:c09::232]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 1AB9881E9D for ; Fri, 20 Jan 2017 09:49:54 -0800 (PST) Received: by mail-wm0-x232.google.com with SMTP id c206so55675315wme.0 for ; Fri, 20 Jan 2017 09:49:54 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=lRiqoob32woT6Lj1b81xGtWje35WR+u3ZMHqtXdrFfE=; b=IwzTX9Ya54DF0iQcFY8t9qgs6ZZIpkY4YcvOfar/SVl8vjURY3Mh3AjeAgdUmpbuZM 0wmkhwoUmPn3nD65nZTuUGgvZ2Jxb13Af2sog6iQovkQlNSkLS5ju5hDv0tOywp8PIdS v8zTBFg6+z930K8PDcfzsbVYwYa01iK/gfJcM= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=lRiqoob32woT6Lj1b81xGtWje35WR+u3ZMHqtXdrFfE=; b=b1xpiL6CRdxmfpo3c3TRWh5zcBbft14tkMHXh+hT3sCDDIQADjslxohuE5pIxVZP6l rk/umZThBkEJv32mLdEtj56TmxddHAH9bSskm6G8xF8ALzAbWxNRh0S4kZTcnTabWKbQ 4mgPHDZ2h6SeVh/x3LeZsyyQLGrCQ5Z/Uyx3Pltxmw3AuSwVzoNkWJoFvHVwPRMvhIbq T6sw4viR6x9FOdqx89OdXad5e4jGIrgAM0p4Xo2bnD87OlekERzH76j3IwxlxZ0tsyhd SwIKrlFZ8zwume77fTxuS8cDC6lBhYpgagNOF5b1DALhIsVmVtzrL14cenwLh6+wHp2o ZMVg== X-Gm-Message-State: AIkVDXIy0dQg4tJ34Y0Iz0Yibw1SGvcE+bQf9whoAaUo6TI+IhEJNMPdtt+DV0FFP/eidkhE X-Received: by 10.223.136.109 with SMTP id e42mr13011738wre.14.1484934592682; Fri, 20 Jan 2017 09:49:52 -0800 (PST) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id 63sm7262478wmg.2.2017.01.20.09.49.51 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 20 Jan 2017 09:49:52 -0800 (PST) Date: Fri, 20 Jan 2017 17:49:50 +0000 From: Leif Lindholm To: Ard Biesheuvel Cc: edk2-devel@lists.01.org, lersek@redhat.com, heyi.guo@linaro.org, ashedel@microsoft.com Message-ID: <20170120174949.GH25883@bivouac.eciton.net> References: <1484931946-11648-1-git-send-email-ard.biesheuvel@linaro.org> MIME-Version: 1.0 In-Reply-To: <1484931946-11648-1-git-send-email-ard.biesheuvel@linaro.org> User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [PATCH] ArmPkg/ArmMmuLib: Revert "use a pool allocation for the root table" X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 20 Jan 2017 17:49:54 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Fri, Jan 20, 2017 at 05:05:46PM +0000, 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 Given the discussion on the other thread: Reviewed-by: Leif Lindholm > --- > ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 29 ++++---------------- > 1 file changed, 6 insertions(+), 23 deletions(-) > > 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; > } > > -- > 2.7.4 >