From patchwork Mon Feb 27 14:35:34 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Jon Medhurst (Tixy)" X-Patchwork-Id: 9593287 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 7C873601D7 for ; Mon, 27 Feb 2017 14:36:09 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 631E72838E for ; Mon, 27 Feb 2017 14:36:09 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 54073283ED; Mon, 27 Feb 2017 14:36:09 +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 C0C9F2838E for ; Mon, 27 Feb 2017 14:36:08 +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=/zYdVw4YqXZbJlFlBuhinvKK1X/WU7sux7rkKnIxmSE=; b=BzT6V+gQizVnc6 KdA0/9o2rhUV4yWNF7bW1+WOXkflLYVY2Ox3xwjbm5GjbQ7AuTIs1IfzUDP+3MKceFY3ajP8VhIS2 TLjm9Stkdx32Aip7TSCbiuDLi9nHvrn8y+zeo6MDrDolGLNRjVyEXQgOiOKyjlhGtmtbrsBZGRq49 /dADEBRNFvl/sZItaxdmfiI13sAh1DweTgpBy6b+90ZU866oC82fRzxwm3pyRUKmj0mw0SuJ3/Otn hYrpZSjtv0MzPZ7ZzJTz8oMYA1pLLqrXUZ3t8m8qTut83W1RTEDbAN0UDveT91MJ8/Uh40AuDKe49 jouOnXjEhKFErEdeF9/A==; 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 1ciMPF-0007yR-1r; Mon, 27 Feb 2017 14:36:05 +0000 Received: from mail-wr0-x22d.google.com ([2a00:1450:400c:c0c::22d]) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1ciMP9-0007qW-J5 for linux-arm-kernel@lists.infradead.org; Mon, 27 Feb 2017 14:36:02 +0000 Received: by mail-wr0-x22d.google.com with SMTP id l37so9486749wrc.1 for ; Mon, 27 Feb 2017 06:35:37 -0800 (PST) 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=ZRVD1FDsK+3w5TSq7i0jCRkiM8K7lzZEts0Qhiw4U48=; b=kmHaChOJ6bZqSVTqR2Qe8Mn3qQa7LdCipC+KIxldNxI5tWhDK+Leci2zKzCzOtfP/0 LQ3N1YHnBdan+ZEJa23StDfXAK9NdmLrLRDRndIr/82ynHYhVWInIT3EsWRGBJxHm3KO u2uCoV5jMHGO0rFJ0p+qn2Em3FhCpxeK43zzY= 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=ZRVD1FDsK+3w5TSq7i0jCRkiM8K7lzZEts0Qhiw4U48=; b=gM4396UPKxVCnhLbM+KkQOaUsM6cdGHFcYRvKvfE5FbRfs1rj0oSscyHqMbhq0yl3x mGrpG9ql5GGeA31vCrIrvuz0w8l7c/uo0gpCxdRajn58qunPbSUDHKKZ0owqWIXNkoEt DHLtBaRYk3ZCAyKY82EUXG6gnYLu5h2fu0F2scNPN4lJc9baQp2ejzhXduuIu7kI0ehn GQfZTkm9j0SWUnGOqWNl6EZmw9TZ8fz6qIlBmIv5RRAF11njAMzKh90YAWKB2aR2eNLv ZWUJe2ybRt2b8X0jYq1F9V9/eglhzZ+RsytlXYK6ZyRWmcr4IAtvlWa9T146NsBAjZ1Q j3FA== X-Gm-Message-State: AMke39nVGKHEd9rrcs+zbNwGkgVtWgdi62vuVOv2IsiRTk3VvVxo4EJUvnzB6sRKcz48UF5w X-Received: by 10.223.144.65 with SMTP id h59mr16989601wrh.30.1488206136558; Mon, 27 Feb 2017 06:35:36 -0800 (PST) Received: from linaro1 (82-69-122-217.dsl.in-addr.zen.co.uk. [82.69.122.217]) by smtp.gmail.com with ESMTPSA id q66sm14453320wmb.28.2017.02.27.06.35.35 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 27 Feb 2017 06:35:35 -0800 (PST) Message-ID: <1488206134.9100.7.camel@linaro.org> Subject: Re: [RFC PATCH] arm: Fix cache inconsistency when using fixmap From: "Jon Medhurst (Tixy)" To: Ard Biesheuvel , Stefan Agner Date: Mon, 27 Feb 2017 14:35:34 +0000 In-Reply-To: References: <1487778678.18810.8.camel@linaro.org> <5497800b3271f13583e78710ac39a48e@agner.ch> X-Mailer: Evolution 3.22.5-1 Mime-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20170227_063559_860529_83CD0CF1 X-CRM114-Status: GOOD ( 17.67 ) 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: Rob Herring , Russell King , "linux-arm-kernel@lists.infradead.org" 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 Sun, 2017-02-26 at 14:35 +0000, Ard Biesheuvel wrote: > However, we could make this a bit more private to the fixmap code by > using something like > > > diff --git a/arch/arm/include/asm/fixmap.h b/arch/arm/include/asm/fixmap.h > index 5c17d2dec777..6c375aea8f22 100644 > --- a/arch/arm/include/asm/fixmap.h > +++ b/arch/arm/include/asm/fixmap.h > @@ -41,11 +41,14 @@ static const enum fixed_addresses __end_of_fixed_addresses = > > #define FIXMAP_PAGE_COMMON (L_PTE_YOUNG | L_PTE_PRESENT | > L_PTE_XN | L_PTE_DIRTY) > > -#define FIXMAP_PAGE_NORMAL (FIXMAP_PAGE_COMMON | L_PTE_MT_WRITEBACK) > -#define FIXMAP_PAGE_RO (FIXMAP_PAGE_NORMAL | L_PTE_RDONLY) > +#define __FIXMAP_MT_NORMAL ((pgprot_val(pgprot_kernel) ?: The code above misses out setting L_PTE_XN when using pgprot_kernel > FIXMAP_PAGE_COMMON) | \ > + L_PTE_MT_WRITEBACK) > +#define FIXMAP_PAGE_NORMAL __pgprot(__FIXMAP_MT_NORMAL) > +#define FIXMAP_PAGE_RO __pgprot(__FIXMAP_MT_NORMAL | L_PTE_RDONLY) > > /* Used by set_fixmap_(io|nocache), both meant for mapping a device */ > -#define FIXMAP_PAGE_IO (FIXMAP_PAGE_COMMON | > L_PTE_MT_DEV_SHARED | L_PTE_SHARED) > +#define FIXMAP_PAGE_IO __pgprot(FIXMAP_PAGE_COMMON | > L_PTE_MT_DEV_SHARED | \ > + L_PTE_SHARED) > #define FIXMAP_PAGE_NOCACHE FIXMAP_PAGE_IO > > #define __early_set_fixmap __set_fixmap > > (and drop the change to mm/mmu.c), which boils down to the same for > fixmap but does not affect other users of pgprot_kernel. Also, it > appears these definitions are broken under STRICT_MM_TYPECHECKS so > this is a good opportunity to get that fixed as well. I like this method better because as you say it keeps things private to fixmap, and it doesn't risk affecting other things. As for getting STRICT_MM_TYPECHECKS working that looks good too, but should be a separate cleanup patch, especially as a fix for the cache problem possibly should go to stable kernels. I also think FIXMAP_PAGE_COMMON should be renamed (prefixed with '__') as it's an implementation detail an not a memory type used with fixmap. So I was thinking, one patch as a bugfix: ---------------------------------------------------------------------- ---------------------------------------------------------------------- Reviewed-by: Stefan Agner Reviewed-by: Ard Biesheuvel diff --git a/arch/arm/include/asm/fixmap.h b/arch/arm/include/asm/fixmap.h index 5c17d2dec777..4e6784dc5668 100644 --- a/arch/arm/include/asm/fixmap.h +++ b/arch/arm/include/asm/fixmap.h @@ -41,7 +41,8 @@ static const enum fixed_addresses __end_of_fixed_addresses = #define FIXMAP_PAGE_COMMON (L_PTE_YOUNG | L_PTE_PRESENT | L_PTE_XN | L_PTE_DIRTY) -#define FIXMAP_PAGE_NORMAL (FIXMAP_PAGE_COMMON | L_PTE_MT_WRITEBACK) +#define FIXMAP_PAGE_NORMAL (pgprot_kernel ? pgprot_kernel | L_PTE_XN : \ + FIXMAP_PAGE_COMMON | L_PTE_MT_WRITEBACK) #define FIXMAP_PAGE_RO (FIXMAP_PAGE_NORMAL | L_PTE_RDONLY) /* Used by set_fixmap_(io|nocache), both meant for mapping a device */ ---------------------------------------------------------------------- Second patch as a cleanup. Note this is different to Ard's version as it uses PAGE_KERNEL rather than open coding the same code from pgtable.h to add XN permisions (Also, the default generic definition of FIXMAP_PAGE_NORMAL is PAGE_KERNEL, and the only reason we want to change it is to select an alternate value if that is not yet setup) ---------------------------------------------------------------------- diff --git a/arch/arm/include/asm/fixmap.h b/arch/arm/include/asm/fixmap.h index 4e6784dc5668..ec689c6aa644 100644 --- a/arch/arm/include/asm/fixmap.h +++ b/arch/arm/include/asm/fixmap.h @@ -39,14 +39,15 @@ static const enum fixed_addresses __end_of_fixed_addresses = __end_of_fixmap_region > __end_of_early_ioremap_region ? __end_of_fixmap_region : __end_of_early_ioremap_region; -#define FIXMAP_PAGE_COMMON (L_PTE_YOUNG | L_PTE_PRESENT | L_PTE_XN | L_PTE_DIRTY) +#define __FIXMAP_PAGE_COMMON (L_PTE_YOUNG | L_PTE_PRESENT | L_PTE_XN | L_PTE_DIRTY) -#define FIXMAP_PAGE_NORMAL (pgprot_kernel ? pgprot_kernel | L_PTE_XN : \ - FIXMAP_PAGE_COMMON | L_PTE_MT_WRITEBACK) -#define FIXMAP_PAGE_RO (FIXMAP_PAGE_NORMAL | L_PTE_RDONLY) +#define FIXMAP_PAGE_NORMAL (pgprot_val(pgprot_kernel) ? PAGE_KERNEL : \ + __pgprot(__FIXMAP_PAGE_COMMON | L_PTE_MT_WRITEBACK)) +#define FIXMAP_PAGE_RO _MOD_PROT(FIXMAP_PAGE_NORMAL, L_PTE_RDONLY) /* Used by set_fixmap_(io|nocache), both meant for mapping a device */ -#define FIXMAP_PAGE_IO (FIXMAP_PAGE_COMMON | L_PTE_MT_DEV_SHARED | L_PTE_SHARED) +#define FIXMAP_PAGE_IO __pgprot(__FIXMAP_PAGE_COMMON | L_PTE_MT_DEV_SHARED | \ + L_PTE_SHARED) #define FIXMAP_PAGE_NOCACHE FIXMAP_PAGE_IO #define __early_set_fixmap __set_fixmap