Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752257AbdLCNfh (ORCPT ); Sun, 3 Dec 2017 08:35:37 -0500 Received: from smtp.codeaurora.org ([198.145.29.96]:58682 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751558AbdLCNfe (ORCPT ); Sun, 3 Dec 2017 08:35:34 -0500 DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org EA14460241 Authentication-Results: pdx-caf-mail.web.codeaurora.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: pdx-caf-mail.web.codeaurora.org; spf=none smtp.mailfrom=shankerd@codeaurora.org Reply-To: shankerd@codeaurora.org Subject: Re: [PATCH v4 2/2] arm64: Add software workaround for Falkor erratum 1041 To: Will Deacon Cc: linux-efi@vger.kernel.org, Ard Biesheuvel , Marc Zyngier , Catalin Marinas , linux-kernel@vger.kernel.org, Matt Fleming , James Morse , Christoffer Dall , Robin Murphy , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org References: <1511824680-16397-1-git-send-email-shankerd@codeaurora.org> <1511824680-16397-3-git-send-email-shankerd@codeaurora.org> <20171201112457.GE18083@arm.com> From: Shanker Donthineni Message-ID: <12cb0f4a-577b-ffd5-21b7-26ce0e46505a@codeaurora.org> Date: Sun, 3 Dec 2017 07:35:28 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: <20171201112457.GE18083@arm.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3885 Lines: 82 Hi Will, thanks for your review comments. On 12/01/2017 05:24 AM, Will Deacon wrote: > On Mon, Nov 27, 2017 at 05:18:00PM -0600, Shanker Donthineni wrote: >> The ARM architecture defines the memory locations that are permitted >> to be accessed as the result of a speculative instruction fetch from >> an exception level for which all stages of translation are disabled. >> Specifically, the core is permitted to speculatively fetch from the >> 4KB region containing the current program counter 4K and next 4K. >> >> When translation is changed from enabled to disabled for the running >> exception level (SCTLR_ELn[M] changed from a value of 1 to 0), the >> Falkor core may errantly speculatively access memory locations outside >> of the 4KB region permitted by the architecture. The errant memory >> access may lead to one of the following unexpected behaviors. >> >> 1) A System Error Interrupt (SEI) being raised by the Falkor core due >> to the errant memory access attempting to access a region of memory >> that is protected by a slave-side memory protection unit. >> 2) Unpredictable device behavior due to a speculative read from device >> memory. This behavior may only occur if the instruction cache is >> disabled prior to or coincident with translation being changed from >> enabled to disabled. >> >> The conditions leading to this erratum will not occur when either of the >> following occur: >> 1) A higher exception level disables translation of a lower exception level >> (e.g. EL2 changing SCTLR_EL1[M] from a value of 1 to 0). >> 2) An exception level disabling its stage-1 translation if its stage-2 >> translation is enabled (e.g. EL1 changing SCTLR_EL1[M] from a value of 1 >> to 0 when HCR_EL2[VM] has a value of 1). >> >> To avoid the errant behavior, software must execute an ISB immediately >> prior to executing the MSR that will change SCTLR_ELn[M] from 1 to 0. >> >> Signed-off-by: Shanker Donthineni >> --- >> Changes since v3: >> Rebased to kernel v4.15-rc1. >> Changes since v2: >> Repost the corrected patches. >> Changes since v1: >> Apply the workaround where it's required. >> >> Documentation/arm64/silicon-errata.txt | 1 + >> arch/arm64/Kconfig | 12 +++++++++++- >> arch/arm64/include/asm/assembler.h | 19 +++++++++++++++++++ >> arch/arm64/include/asm/cpucaps.h | 3 ++- >> arch/arm64/kernel/cpu-reset.S | 1 + >> arch/arm64/kernel/cpu_errata.c | 16 ++++++++++++++++ >> arch/arm64/kernel/efi-entry.S | 2 ++ >> arch/arm64/kernel/head.S | 1 + >> arch/arm64/kernel/relocate_kernel.S | 1 + >> arch/arm64/kvm/hyp-init.S | 1 + > > This is an awful lot of code just to add an ISB instruction prior to > disabling the MMU. Why do you need to go through the alternatives framework > for this? Just do it with an #ifdef; this isn't a fastpath. > We can avoid changes to only two files cpu_errata.c and cpucaps.h without using the alternatives framework. Even though it's in slow path, cpu-errata.c changes provides a nice debug message which indicates the erratum E1041 is applied. Erratum log information would be very useful to conform our customers using the right kernel with E1014 patch by looking at dmesg. Other than that I don't have any other strong opinion to avoid alternatives and handle using #idef. Should I go head and post v5 patch without alternatives? > Will > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > -- Shanker Donthineni Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.