From patchwork Sat Feb 25 01:06:28 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Agner X-Patchwork-Id: 9591395 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 29C1D60471 for ; Sat, 25 Feb 2017 01:07:16 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 05E5B1FF28 for ; Sat, 25 Feb 2017 01:07:16 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id EE894228C9; Sat, 25 Feb 2017 01:07:15 +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 A1FBC1FF28 for ; Sat, 25 Feb 2017 01:07:14 +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:Message-ID:References:In-Reply-To: Subject:To:From:Date:MIME-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=6fSUlehByEbjKLxHSrgja8B2FUDVwsjnpsJY+HqYxsY=; b=qdlqPnchelxXOE ePK/GYtkLx/rfKD/hGwBWGSjTTlBw+gS00d163vL+uGLT7CHxEuQ9IM+PJ3pgkRnamd9NFGJz86im QlIN1ORBmq89Z0FWAOa+2HdDngZy0swhHX6h8rceaCL7AHs8hmQJym1o633kct6g3gqHNXCOLZ2/p DLQymwRos6GOpenm/vb3fkgwN3FAPy9jahIOQy1JMZRD2w50SWJLnDAyJVfh5QFg6GgNlehAVOh2W mjsGq/LWFVSAftXXowOdwjf4tuBP3CPN8iHy7/omVhRrgd53IfYrqY7Uu11xaPJ75g5luzizATEGJ MaArCDNIb0jWqOZYUW8g==; 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 1chQpH-0004Kc-KJ; Sat, 25 Feb 2017 01:07:07 +0000 Received: from mail.kmu-office.ch ([178.209.48.109]) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1chQpC-0004JZ-GK for linux-arm-kernel@lists.infradead.org; Sat, 25 Feb 2017 01:07:05 +0000 Received: from webmail.kmu-office.ch (unknown [178.209.48.103]) by mail.kmu-office.ch (Postfix) with ESMTPSA id 225CE5C104A; Sat, 25 Feb 2017 01:57:02 +0100 (CET) MIME-Version: 1.0 Date: Fri, 24 Feb 2017 17:06:28 -0800 From: Stefan Agner To: "Jon Medhurst (Tixy)" , ard.biesheuvel@linaro.org Subject: Re: [RFC PATCH] arm: Fix cache inconsistency when using fixmap In-Reply-To: <1487778678.18810.8.camel@linaro.org> References: <1487778678.18810.8.camel@linaro.org> Message-ID: <5497800b3271f13583e78710ac39a48e@agner.ch> X-Sender: stefan@agner.ch User-Agent: Roundcube Webmail/1.1.3 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=agner.ch; s=dkim; t=1487984222; bh=r9pyZCmF3pAQBFNFGLS9jSDYlBDWiJtz0+8uDRXD97M=; h=MIME-Version:Content-Type:Content-Transfer-Encoding:Date:From:To:Cc:Subject:In-Reply-To:References:Message-ID; b=rxGPKgdnoYLyi8lmas9w7dylQgXY6KIdhrmLO7z1j0WWTaJKpf1qu5zkuSZyETBJs9IVVumiie+ozqmuDwTYS4xavA5W/0ct7a6l+RLVihSt7voMV9iO6XpUlNjamsZhN+Qtdhywj7zBlMFv/1Ng+KjKGDKS4Urs1TYv+Quqpbs= X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20170224_170703_040653_EF52C50A X-CRM114-Status: GOOD ( 19.78 ) 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 [also added Ard] On 2017-02-22 07:51, Jon Medhurst (Tixy) wrote: > Cacheable memory mappings need to be marked as shareable otherwise the > CPU accessing fixmap memory may end up with local cachelines for that > memory, resulting in different CPUs in the system seeing different > values for the same memory location. > > This issue was discovered when investigating failures in the ARM kprobe > tests. kprobes uses patch_text() to modify the kernel image, which > when CONFIG_DEBUG_RODATA is enabled makes use of fixmap to map the page > to be modified. As the shareable attribute wasn't being set in the PTE > entry for that page, a write to it wasn't being broadcast to other > caches in the system, which in the the case being investigated was > CPU's in the other cluster of a big.LITTLE system. > > Fixes: a5f4c561b3b1 ("ARM: 8415/1: early fixmap support for earlycon") > Cc: stable@vger.kernel.org # v4.3+ > > Signed-off-by: Jon Medhurst > --- > > The fixmap changes in the original commit a5f4c561b3b1 look a bit iffy > to me, shouldn't it have have made use of PAGE_KERNEL or pgprot_kernel > or something like that to get PTE attributes suitable for the system > being run, rather than rolling it's own set of values? The PAGE_KERNEL points to pgprot_kernel, which in turn is dynamically setup in build_mem_type_table. This initialization is taking place late in the boot process, too late for early fixmap... Early fixmap gets shut down before going SMP, so non shared mappings are not an issue there. But FIXMAP_PAGE_NORMAL is also used for regular fixmap, which is essentially the problem. Also, regular fixmap does not make sure of the cache policy due to that, but just uses the hardcoded writeback policy... With that in mind, populating kern_pgprot with a reasonable default which works for early fixmap might be a reasonable approach. Later kern_pgprot gets setup with the appropriate shared attribute and cache policy... > That's why I'm marking this as an RFC, perhaps the correct fix is to > rework the code to use the defines from arch/arm/include/asm/pgtable.h > > arch/arm/include/asm/fixmap.h | 2 +- > arch/arm/probes/kprobes/test-core.h | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/include/asm/fixmap.h b/arch/arm/include/asm/fixmap.h > index 5c17d2dec777..26d4a4677cd7 100644 > --- a/arch/arm/include/asm/fixmap.h > +++ b/arch/arm/include/asm/fixmap.h > @@ -41,7 +41,7 @@ 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 (FIXMAP_PAGE_COMMON | L_PTE_SHARED | > 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 */ > diff --git a/arch/arm/probes/kprobes/test-core.h > b/arch/arm/probes/kprobes/test-core.h > index 94285203e9f7..cde2b4e9358a 100644 > --- a/arch/arm/probes/kprobes/test-core.h > +++ b/arch/arm/probes/kprobes/test-core.h > @@ -8,7 +8,7 @@ > * published by the Free Software Foundation. > */ > > -#define VERBOSE 0 /* Set to '1' for more logging of test cases */ > +#define VERBOSE 1 /* Set to '1' for more logging of test cases */ That seems to be unrelated. --- Stefan > > #ifdef CONFIG_THUMB2_KERNEL > #define NORMAL_ISA "16" diff --git a/arch/arm/include/asm/fixmap.h b/arch/arm/include/asm/fixmap.h index 5c17d2dec777..c663e562716c 100644 --- a/arch/arm/include/asm/fixmap.h +++ b/arch/arm/include/asm/fixmap.h @@ -39,12 +39,11 @@ 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_NORMAL (FIXMAP_PAGE_COMMON | L_PTE_MT_WRITEBACK) +#define FIXMAP_PAGE_NORMAL (pgprot_kernel | L_PTE_XN) #define FIXMAP_PAGE_RO (FIXMAP_PAGE_NORMAL | L_PTE_RDONLY) /* Used by set_fixmap_(io|nocache), both meant for mapping a device */ +#define FIXMAP_PAGE_COMMON (L_PTE_YOUNG | L_PTE_PRESENT | L_PTE_XN | L_PTE_DIRTY) #define FIXMAP_PAGE_IO (FIXMAP_PAGE_COMMON | L_PTE_MT_DEV_SHARED | L_PTE_SHARED) #define FIXMAP_PAGE_NOCACHE FIXMAP_PAGE_IO diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c index 4001dd15818d..9cb7c559263f 100644 --- a/arch/arm/mm/mmu.c +++ b/arch/arm/mm/mmu.c @@ -65,7 +65,8 @@ pmdval_t user_pmd_table = _PAGE_USER_TABLE; static unsigned int cachepolicy __initdata = CPOLICY_WRITEBACK; static unsigned int ecc_mask __initdata = 0; pgprot_t pgprot_user; -pgprot_t pgprot_kernel; +pgprot_t pgprot_kernel = __pgprot(L_PTE_PRESENT | L_PTE_YOUNG | + L_PTE_DIRTY | L_PTE_MT_WRITEBACK); pgprot_t pgprot_hyp_device; pgprot_t pgprot_s2; pgprot_t pgprot_s2_device;