From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c0b::242; helo=mail-it0-x242.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it0-x242.google.com (mail-it0-x242.google.com [IPv6:2607:f8b0:4001:c0b::242]) (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 A96FF210E2DA8 for ; Thu, 21 Jun 2018 07:10:35 -0700 (PDT) Received: by mail-it0-x242.google.com with SMTP id m194-v6so4871233itg.2 for ; Thu, 21 Jun 2018 07:10:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=vChnKOjEM1tMJyoPfCd8fBk0wlP7JJg6Q3QqVLIBi/8=; b=ZQika89GYFB4QdXXvtAk88S8OQgQwaJCREBCfxlVjxmAoh1OVmx4r9zHu9a9/GvU1z zJohvqijFuSDUTGeE4OmBLp8y2tjAXdkC6h0jDkqrWr3wXsPf9+WAVRXhBJVJ4cZVaHh O0NbNua+jgA8BcYSljypr8+gheSjnVj/UE2og= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=vChnKOjEM1tMJyoPfCd8fBk0wlP7JJg6Q3QqVLIBi/8=; b=ABpMsqXF7wLgCsGJqN3bXwXOymGTnmXji9Y/0RWqNKfqFa+s9OWrKfy8TDAwF5CoiZ LBfFx25J2tshjsw3vJEUgi82SXo5CBHCkyisvVYnXTUztriooLWmxFh/XNK0HnNPYEWV I1VWjy6iiMt1XQGtmMe7VWq6EaYdvmCYeLKFGDRHezoWjbaYnz8yxjwsrdQVFbdgo1sa TwUAdWFStsZ66hZC6hu3ziH4jlDMf7GxweBSL8NS0c0rOeddoXvSwincDoAvNegNNPzK iawyM3s8XpT7QmoJgUtpSuet8TpUiwUXEKRqIv/MwN3jYmFygCswEmHdvlt4rPMAKizC UE9A== X-Gm-Message-State: APt69E02UMs5+UlSN6IFQqh7I61za+ICJlWiaHDpYLCX5QkxiuD/79t8 Dqafmxeo7MGE1qyID7binBMcEUS5lC3JnBwvorAhB+mGFqM= X-Google-Smtp-Source: ADUXVKImqhr9RPHiv5Vlar/JXLi0yAeBfuIy05hkR/xeOHjre63YMhXr/8UcGJN3mGdTQMOco1HY8x6AMTnH1Ktj78w= X-Received: by 2002:a24:1d0e:: with SMTP id 14-v6mr5234914itj.50.1529590234258; Thu, 21 Jun 2018 07:10:34 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a6b:bbc7:0:0:0:0:0 with HTTP; Thu, 21 Jun 2018 07:10:33 -0700 (PDT) In-Reply-To: <20180621140138.pvco42ieamb2cl5x@bivouac.eciton.net> References: <20180621081315.16228-1-ard.biesheuvel@linaro.org> <20180621081315.16228-3-ard.biesheuvel@linaro.org> <20180621140138.pvco42ieamb2cl5x@bivouac.eciton.net> From: Ard Biesheuvel Date: Thu, 21 Jun 2018 16:10:33 +0200 Message-ID: To: Leif Lindholm Cc: "edk2-devel@lists.01.org" , Chris Co Subject: Re: [PATCH v2 2/2] ArmPkg/ArmMmuLib ARM: assume page tables are in writeback cacheable memory X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 21 Jun 2018 14:10:35 -0000 Content-Type: text/plain; charset="UTF-8" On 21 June 2018 at 16:01, Leif Lindholm wrote: > On Thu, Jun 21, 2018 at 10:13:15AM +0200, Ard Biesheuvel wrote: >> Given that these days, our ARM port only supports ARMv7 and later, we >> can assume that the page table walker's memory accesses are cache >> coherent, and so there is no need to perform cache maintenance. It >> does require the page tables themselves to reside in memory mapped as >> writeback cacheable so ASSERT() that this is the case. > > One minor nit. > >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Ard Biesheuvel >> --- >> ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S | 2 -- >> ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c | 14 +++----------- >> 2 files changed, 3 insertions(+), 13 deletions(-) >> >> diff --git a/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S b/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S >> index 149b57e059ee..f2a517671f0a 100644 >> --- a/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S >> +++ b/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S >> @@ -100,8 +100,6 @@ ASM_FUNC(ArmGetTTBR0BaseAddress) >> // IN VOID *MVA // R1 >> // ); >> ASM_FUNC(ArmUpdateTranslationTableEntry) >> - mcr p15,0,R0,c7,c14,1 @ DCCIMVAC Clean data cache by MVA >> - dsb >> mcr p15,0,R1,c8,c7,1 @ TLBIMVA TLB Invalidate MVA >> mcr p15,0,R9,c7,c5,6 @ BPIALL Invalidate Branch predictor array. R9 == NoOp >> dsb >> diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c >> index 9c2578979e44..3037b642d40c 100644 >> --- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c >> +++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c >> @@ -343,17 +343,12 @@ ArmConfigureMmu ( >> } >> >> // 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)) { >> - TTBRAttributes = ArmHasMpExtensions () ? TTBR_MP_NON_CACHEABLE : TTBR_NON_CACHEABLE; >> - } else if ((TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK) || >> + if ((TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK) || >> (TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK)) { >> TTBRAttributes = ArmHasMpExtensions () ? TTBR_MP_WRITE_BACK_ALLOC : TTBR_WRITE_BACK_ALLOC; >> - } else if ((TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_WRITE_THROUGH) || >> - (TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_THROUGH)) { >> - TTBRAttributes = ArmHasMpExtensions () ? TTBR_MP_WRITE_THROUGH : TTBR_WRITE_THROUGH; >> } else { >> - ASSERT (0); // No support has been found for the attributes of the memory region that the translation table belongs to. >> + // Page tables must reside in memory mapped as writeback cacheable > > ARM ARM always uses "write-back" - could you add the hyphen to assist > greppability? > > If so, for the series: > Reviewed-by: Leif Lindholm > Thanks Pushed as c2d6e2bc12b2..6e275c613e15 >> + ASSERT (0); >> return RETURN_UNSUPPORTED; >> } >> >> @@ -461,9 +456,6 @@ ConvertSectionToPages ( >> PageTable[Index] = TT_DESCRIPTOR_PAGE_BASE_ADDRESS(BaseAddress + (Index << 12)) | PageDescriptor; >> } >> >> - // Flush d-cache so descriptors make it back to uncached memory for subsequent table walks >> - WriteBackInvalidateDataCacheRange ((VOID *)PageTable, TT_DESCRIPTOR_PAGE_SIZE); >> - >> // Formulate page table entry, Domain=0, NS=0 >> PageTableDescriptor = (((UINTN)PageTable) & TT_DESCRIPTOR_SECTION_PAGETABLE_ADDRESS_MASK) | TT_DESCRIPTOR_SECTION_TYPE_PAGE_TABLE; >> >> -- >> 2.17.1 >>