From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wj0-x22a.google.com (mail-wj0-x22a.google.com [IPv6:2a00:1450:400c:c01::22a]) (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 2D54981D43 for ; Wed, 30 Nov 2016 08:13:52 -0800 (PST) Received: by mail-wj0-x22a.google.com with SMTP id mp19so179511570wjc.1 for ; Wed, 30 Nov 2016 08:13:51 -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=ayl+LWS6Jbk6CzX+ahWlmjP+5GM08pyyHYiqjBwRogU=; b=byFDhqyzAESyrCKQiPIK7XSyVDOuBAwl4IffAsX07GQW+CGyEsh6ntLCHvObVCXrtg Ol8vHEK6Yyp5avdnvnK1ojVazkep2YU5jKaCTJTW3t+M8a+9kOW3aGNosgCscBT0fPqK VJt7zIPBcxg6+WUXKlEO8LHlMT51sDmk7rbFs= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=ayl+LWS6Jbk6CzX+ahWlmjP+5GM08pyyHYiqjBwRogU=; b=CTB3j7J/by8uyq/y+a1GdluVCtiuWLSYnwlK5BBpqKDGUaU431iUa0IwWg9UMB0N14 TIsUlqateuYAlo4uJUNDDkBap06D7k2eDyllGmI/i8A+dZ97ivWSOA0ca6v7ajxYT6J2 vYxfhbH8z38wTcZQpDq47CwQWJ48aTw+SXmKE3jt4ZegpkSy7LdKK77LPtTqtBa4HaZy MywxTAaKpH35AZvWw1SC4czFK/DKBKZ4rJleVDLrTbiLvPmiToNBRDlM/N+RcBIiwkF4 lZQvvEAgHYAaJe/SeEBSGOXrgUwM1uNNaNL9iSESt9TaNEx9ZpZaPP7B4Nt/ywgvWAxO xAIw== X-Gm-Message-State: AKaTC02m+xKRAsS7v1Ib5rAddhwmmZnh6H9lpgboT2+Lvid67MzOo3EiXWyOUXaADwdEbeuM X-Received: by 10.194.104.39 with SMTP id gb7mr28794230wjb.139.1480522430366; Wed, 30 Nov 2016 08:13:50 -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 x188sm8654586wmx.4.2016.11.30.08.13.49 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 30 Nov 2016 08:13:49 -0800 (PST) Date: Wed, 30 Nov 2016 16:13:48 +0000 From: Leif Lindholm To: Ard Biesheuvel Cc: edk2-devel@lists.01.org, heyi.guo@linaro.org Message-ID: <20161130161348.GJ27069@bivouac.eciton.net> References: <1479661970-10517-1-git-send-email-ard.biesheuvel@linaro.org> MIME-Version: 1.0 In-Reply-To: <1479661970-10517-1-git-send-email-ard.biesheuvel@linaro.org> User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [PATCH] ArmPkg/ArmMmuLib: support page tables in cacheable memory only 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: Wed, 30 Nov 2016 16:13:52 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Sun, Nov 20, 2016 at 05:12:50PM +0000, Ard Biesheuvel wrote: > Translation table walks are always cache coherent on ARMv8-A, so cache > maintenance on page tables is never needed. Since there is a risk of > loss of coherency when using mismatched attributes, and given that memory > is mapped cacheable except for extraordinary cases (such as non-coherent > DMA), restrict the page table walker to performing cacheable accesses to > the translation tables. > > For DEBUG builds, retain some of the logic so that we can double check > that the memory holding the root translation table is indeed located in > memory that is mapped cacheable. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel Reviewed-by: Leif Lindholm Thanks, pushed as 35718840efe3a29c981b5b0f4d2f617f9a1f2c2e. > --- > ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 49 ++++++++++---------- > 1 file changed, 24 insertions(+), 25 deletions(-) > > diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c > index 1fb3bbec6347..c78297084207 100644 > --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c > +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c > @@ -627,6 +627,19 @@ ArmConfigureMmu ( > return RETURN_UNSUPPORTED; > } > > + // > + // Translation table walks are always cache coherent on ARMv8-A, so cache > + // maintenance on page tables is never needed. Since there is a risk of > + // loss of coherency when using mismatched attributes, and given that memory > + // is mapped cacheable except for extraordinary cases (such as non-coherent > + // DMA), have the page table walker perform cached accesses as well, and > + // assert below that that matches the attributes we use for CPU accesses to > + // the region. > + // > + TCR |= TCR_SH_INNER_SHAREABLE | > + TCR_RGN_OUTER_WRITE_BACK_ALLOC | > + TCR_RGN_INNER_WRITE_BACK_ALLOC; > + > // Set TCR > ArmSetTCR (TCR); > > @@ -672,11 +685,15 @@ ArmConfigureMmu ( > > TranslationTableAttribute = TT_ATTR_INDX_INVALID; > while (MemoryTable->Length != 0) { > - // Find the memory attribute for the Translation Table > - if (((UINTN)TranslationTable >= MemoryTable->PhysicalBase) && > - ((UINTN)TranslationTable <= MemoryTable->PhysicalBase - 1 + MemoryTable->Length)) { > - TranslationTableAttribute = MemoryTable->Attributes; > - } > + > + DEBUG_CODE_BEGIN (); > + // Find the memory attribute for the Translation Table > + if ((UINTN)TranslationTable >= MemoryTable->PhysicalBase && > + (UINTN)TranslationTable + RootTableEntrySize <= MemoryTable->PhysicalBase + > + MemoryTable->Length) { > + TranslationTableAttribute = MemoryTable->Attributes; > + } > + DEBUG_CODE_END (); > > Status = FillTranslationTable (TranslationTable, MemoryTable); > if (RETURN_ERROR (Status)) { > @@ -685,26 +702,8 @@ ArmConfigureMmu ( > MemoryTable++; > } > > - // Translate the Memory Attributes into Translation Table Register Attributes > - if ((TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED) || > - (TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_UNCACHED_UNBUFFERED)) { > - TCR |= TCR_SH_NON_SHAREABLE | TCR_RGN_OUTER_NON_CACHEABLE | TCR_RGN_INNER_NON_CACHEABLE; > - } else if ((TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK) || > - (TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK)) { > - TCR |= TCR_SH_INNER_SHAREABLE | TCR_RGN_OUTER_WRITE_BACK_ALLOC | TCR_RGN_INNER_WRITE_BACK_ALLOC; > - } else if ((TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_WRITE_THROUGH) || > - (TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_THROUGH)) { > - TCR |= TCR_SH_NON_SHAREABLE | TCR_RGN_OUTER_WRITE_THROUGH | TCR_RGN_INNER_WRITE_THROUGH; > - } else { > - // If we failed to find a mapping that contains the root translation table then it probably means the translation table > - // is not mapped in the given memory map. > - ASSERT (0); > - Status = RETURN_UNSUPPORTED; > - goto FREE_TRANSLATION_TABLE; > - } > - > - // Set again TCR after getting the Translation Table attributes > - ArmSetTCR (TCR); > + ASSERT (TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK || > + TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK); > > ArmSetMAIR (MAIR_ATTR(TT_ATTR_INDX_DEVICE_MEMORY, MAIR_ATTR_DEVICE_MEMORY) | // mapped to EFI_MEMORY_UC > MAIR_ATTR(TT_ATTR_INDX_MEMORY_NON_CACHEABLE, MAIR_ATTR_NORMAL_MEMORY_NON_CACHEABLE) | // mapped to EFI_MEMORY_WC > -- > 2.7.4 >