From patchwork Tue Apr 4 08:48:50 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: "Jon Medhurst (Tixy)" X-Patchwork-Id: 9660971 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 9E767602B9 for ; Tue, 4 Apr 2017 08:49:22 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9362C284CB for ; Tue, 4 Apr 2017 08:49:22 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 88127284D6; Tue, 4 Apr 2017 08:49:22 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID autolearn=ham version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 7EB79284CB for ; Tue, 4 Apr 2017 08:49:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Mime-Version:References:In-Reply-To: Date:To:From:Subject:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=IKKCyyuSF9Y7R3pDJgtkxug2Oq70aOlDK5W6qjZ9HUw=; b=c1S7GYF2PAACoC RBB4HcDKIe1VSjZLeTChBx689ss/MpodK1oGdXsLOCh19NAqX0FKR1ryICU6EncmOHzgoMQr59ycc Ap0YobgZqjCQFPuoBCfy4VL4y1HSBISDTJTTASynlsmNES/t1NuJlG5+N2H5836ilohNI+eXHVChp LD5wQTWzB/+n02mxVr52lrZzSCyYKn+0Cxq4JO0e/nWq7Gdz4tuph2UYFMxVoS3BsOstyUueHTE4t ycVjDSQwAC/X6t/5sN4h332wyL1nPU8/3fO4pChntoA5JTq9q9jsEZ27Rpg6rCKLDdtPGfXR/SxqE kHItHO06zBkD7fEZgsrA==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1cvK9O-0004mr-G8; Tue, 04 Apr 2017 08:49:18 +0000 Received: from mail-wr0-x232.google.com ([2a00:1450:400c:c0c::232]) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1cvK9K-0004lr-SJ for linux-arm-kernel@lists.infradead.org; Tue, 04 Apr 2017 08:49:17 +0000 Received: by mail-wr0-x232.google.com with SMTP id k6so199731152wre.2 for ; Tue, 04 Apr 2017 01:48:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=message-id:subject:from:to:cc:date:in-reply-to:references :mime-version:content-transfer-encoding; bh=I/0/BrgPBrLfuRlORwU87bp4c4inhfD6ZIKr538G8Ww=; b=avsNSiWwU/51eXaJOEnm04gn/NA7n/1y1cfpl/XIl7UbDUtDKYTman0v3EQyPRVwWM ryqWhCme5KsnKmTgg0u2bKFxal8gvPH50/zGLlCoIZhX/GTIfeWLLBbBgm1y8zFku6VL UXL0fizRyOYUMtt0xfolihEqj7InQdeWjXNj4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to :references:mime-version:content-transfer-encoding; bh=I/0/BrgPBrLfuRlORwU87bp4c4inhfD6ZIKr538G8Ww=; b=FFRNNNAvnLY9XOvQf1gLARDkYScRbKxiAhAADqdyIkoXXWVrmBZhU9kqdCzu4WGPf+ 8P+pcJbNP4gkUoF8cXwbQFWiYZJCd9wxkVhWwU8wWjJL7UEMBUwYkF7U11M3QQyUDRCK eSdOlTr2l6BC3WaPRjbxT5+50Z2lYGkHjYy+2a5WM0xIt+enDjr5tMPeEkgRB6qD57xO 0mZ/g7U3rVt4hFOunBjTqNVjb/MMwJxUpu2fkSgEuTD6WnC6IgHS30UzqpC7QfDZsZly ggYDmriaoM7f94Eznj9wvblx6MBxNgcSQ/aqOQRc7HSSm2auPPWcGWHZj722AhP+ozd/ Serw== X-Gm-Message-State: AFeK/H1E+4u5AQjIsr98U6dY95yjDH6ZCxlkoUR0Z1wWZKhD4+h3JK1/ FVh/f0Yv9qx+uXu+ X-Received: by 10.28.227.133 with SMTP id a127mr12589976wmh.93.1491295732763; Tue, 04 Apr 2017 01:48:52 -0700 (PDT) Received: from linaro1 (82-69-122-217.dsl.in-addr.zen.co.uk. [82.69.122.217]) by smtp.gmail.com with ESMTPSA id 81sm17568110wmj.9.2017.04.04.01.48.51 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 04 Apr 2017 01:48:52 -0700 (PDT) Message-ID: <1491295730.2995.1.camel@linaro.org> Subject: Re: [PATCH v3] arm: Fix memory attribute inconsistencies when using fixmap From: "Jon Medhurst (Tixy)" To: Ard Biesheuvel , linux-arm-kernel@lists.infradead.org, linux@armlinux.org.uk Date: Tue, 04 Apr 2017 09:48:50 +0100 In-Reply-To: <1491291087-17450-1-git-send-email-ard.biesheuvel@linaro.org> References: <1491291087-17450-1-git-send-email-ard.biesheuvel@linaro.org> X-Mailer: Evolution 3.22.6-1 Mime-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20170404_014915_082322_ED540910 X-CRM114-Status: GOOD ( 23.61 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: vladimir.murzin@arm.com, afzal.mohd.ma@gmail.com, stefan@agner.ch Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP On Tue, 2017-04-04 at 08:31 +0100, Ard Biesheuvel wrote: > From: Jon Medhurst > > To cope with the variety in ARM architectures and configurations, the > pagetable attributes for kernel memory are generated at runtime to match > the system the kernel finds itself on. This calculated value is stored > in pgprot_kernel. > > However, when early fixmap support was added for ARM (commit > a5f4c561b3b1) the attributes used for mappings were hard coded because > pgprot_kernel is not set up early enough. Unfortunately, when fixmap is > used after early boot this means the memory being mapped can have > different attributes to existing mappings, potentially leading to > unpredictable behaviour. A specific problem also exists due to the hard > coded values not include the 'shareable' attribute which means on > systems where this matters (e.g. those with multiple CPU clusters) the > cache contents for a memory location can become inconsistent between > CPUs. > > To resolve these issues we change fixmap to use the same memory > attributes (from pgprot_kernel) that the rest of the kernel uses. To > enable this we need to refactor the initialisation code so > build_mem_type_table() is called early enough. Note, that relies on early > param parsing for memory type overrides passed via the kernel command > line, so we need to make sure this call is still after > parse_early_params(). > > Fixes: a5f4c561b3b1 ("ARM: 8415/1: early fixmap support for earlycon") > Cc: # v4.3+ > Signed-off-by: Jon Medhurst > [ardb: keep early_fixmap_init() before param parsing, for earlycon] > Signed-off-by: Ard Biesheuvel > --- [...] > diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c > index 4e016d7f37b3..ec81a30479aa 100644 > --- a/arch/arm/mm/mmu.c > +++ b/arch/arm/mm/mmu.c > @@ -414,6 +414,11 @@ void __set_fixmap(enum fixed_addresses idx, phys_addr_t phys, pgprot_t prot) > FIXADDR_END); > BUG_ON(idx >= __end_of_fixed_addresses); > > + /* we only support device mappings until pgprot_kernel has been set */ > + if (WARN_ON(pgprot_val(prot) != pgprot_val(FIXMAP_PAGE_IO) && > + pgprot_val(pgprot_kernel) == 0)) > + return; > + Hmm, on return from this function, isn't the caller then going to try and access the memory at the fixmap address we didn't map, and so get a page fault? If so, would a BUG_ON here be better? > if (pgprot_val(prot)) > set_pte_at(NULL, vaddr, pte, > pfn_pte(phys >> PAGE_SHIFT, prot)); > @@ -1616,7 +1621,6 @@ void __init paging_init(const struct machine_desc *mdesc) > { > void *zero_page; > > - build_mem_type_table(); > prepare_page_table(); > map_lowmem(); > memblock_set_current_limit(arm_lowmem_limit); > @@ -1636,3 +1640,9 @@ void __init paging_init(const struct machine_desc *mdesc) > empty_zero_page = virt_to_page(zero_page); > __flush_dcache_page(NULL, empty_zero_page); > } > + > +void __init early_mm_init(const struct machine_desc *mdesc) > +{ > + build_mem_type_table(); > + early_paging_init(mdesc); > +} I tested this change with kprobes tests on TC2 (the setup that showed the original bug) and compile tested for a nommu device (I will boot test that in a bit as that's not quick to setup). Also, with this patch we can now make early_paging_init() static now, and remove the extern... diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c index f5db5548ad42..8ad1e4414024 100644 --- a/arch/arm/kernel/setup.c +++ b/arch/arm/kernel/setup.c @@ -80,7 +80,6 @@ __setup("fpe=", fpe_setup); extern void init_default_cache_policy(unsigned long); extern void paging_init(const struct machine_desc *desc); -extern void early_paging_init(const struct machine_desc *); extern void adjust_lowmem_bounds(void); extern enum reboot_mode reboot_mode; extern void setup_dma_zone(const struct machine_desc *desc); diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c index ec81a30479aa..347cca965783 100644 --- a/arch/arm/mm/mmu.c +++ b/arch/arm/mm/mmu.c @@ -1497,7 +1497,7 @@ pgtables_remap lpae_pgtables_remap_asm; * early_paging_init() recreates boot time page table setup, allowing machines * to switch over to a high (>4G) address space on LPAE systems */ -void __init early_paging_init(const struct machine_desc *mdesc) +static void __init early_paging_init(const struct machine_desc *mdesc) { pgtables_remap *lpae_pgtables_remap; unsigned long pa_pgd; @@ -1565,7 +1565,7 @@ void __init early_paging_init(const struct machine_desc *mdesc) #else -void __init early_paging_init(const struct machine_desc *mdesc) +static void __init early_paging_init(const struct machine_desc *mdesc) { long long offset;