Received: by 10.223.185.116 with SMTP id b49csp2827264wrg; Mon, 5 Mar 2018 09:16:44 -0800 (PST) X-Google-Smtp-Source: AG47ELuTvfxjvNig2j1pq1RPtn6eZr4+KPuoH30GksoBhb40lltVFHEqlflFWSHZ2e17POCSfLQE X-Received: by 2002:a17:902:8c92:: with SMTP id t18-v6mr13793200plo.449.1520270204046; Mon, 05 Mar 2018 09:16:44 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1520270204; cv=none; d=google.com; s=arc-20160816; b=k/N1jbGS0M60RjHq9pKllRtXW9OyZSjrvj3rFkwEFt6zSSjl+J2R4KEXapBNgADZgq e126CCNhY9I8fW7VUGF/r6p+nuy56DUibTUKlVNhLNojf4aCZXBybkwo4VWDGA8biMHl eS87eGtjT/cpahkackLIsmYj7DmLG1wHbdD1fsSlJKS/5lx+/Jpeoogjyh+dtNPD5yX8 xd1+XUwTZRT+1YoxzxufzzNXo0ZvWBsMwJcTRRs33FDjpdmg7BFPLzZjza6mx4k1LxPA RlxigJWOc6s/6Y+xZj1vVi78uYo+m5yw7EbGK3bB/4eyWizfYZcaDWs079OhIcqQ12dL j6GA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:in-reply-to :content-disposition:mime-version:references:message-id:subject:cc :to:from:date:arc-authentication-results; bh=uUOpXTQDuJtt24c4wc9kWbLaDAe6a6q2E0Udbydr1Ew=; b=v/O6oJcn1vlIyYihNqADvmOw4FZGE2y9S066PVQTdijLndjc2tCAHPHFYVH/JSMWf+ wo1L5EnbbKxpMuT6s5yGwyYlSh59mk26vqo8Dj8662lGeuJhFogKZYayu7HhmzAhQJDq APhVmKlN4vNtCkhLBWp0OIMhuWfnjE5T3lyUXl+Eo9cnVffbJQD9E3lqn09qEOHXGGSw 1A3sflORLmvFpoCIeZWPQqPLimzd4LnzLglmYRK+av5Ov/NcqC9dGThsZs4fvN1hAGQy unM6whphjT4NfAikgCyIhTBE7pC11bCAABsGezmzsOAUXrHbMJ/BQbD9zN0PEKXbsiNW zlCw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id r2si8553905pgp.704.2018.03.05.09.16.29; Mon, 05 Mar 2018 09:16:44 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752110AbeCERP1 (ORCPT + 99 others); Mon, 5 Mar 2018 12:15:27 -0500 Received: from usa-sjc-mx-foss1.foss.arm.com ([217.140.101.70]:53740 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751835AbeCERP0 (ORCPT ); Mon, 5 Mar 2018 12:15:26 -0500 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 0A13214; Mon, 5 Mar 2018 09:15:26 -0800 (PST) Received: from edgewater-inn.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com [10.72.51.249]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id CFCBC3F25C; Mon, 5 Mar 2018 09:15:25 -0800 (PST) Received: by edgewater-inn.cambridge.arm.com (Postfix, from userid 1000) id C38FD1AE527E; Mon, 5 Mar 2018 17:15:29 +0000 (GMT) Date: Mon, 5 Mar 2018 17:15:29 +0000 From: Will Deacon To: Shanker Donthineni Cc: Thomas Speier , Vikram Sethi , Sean Campbell , Marc Zyngier , Catalin Marinas , linux-kernel , linux-arm-kernel , kvmarm , Christoffer Dall Subject: Re: [PATCH] arm64: KVM: Use SMCCC_ARCH_WORKAROUND_1 for Falkor BP hardening Message-ID: <20180305171529.GA12869@arm.com> References: <1520027418-10646-1-git-send-email-shankerd@codeaurora.org> <20180305155614.GG6618@arm.com> <05c25982-4939-593f-ad30-bf496e879885@codeaurora.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <05c25982-4939-593f-ad30-bf496e879885@codeaurora.org> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Mar 05, 2018 at 10:57:58AM -0600, Shanker Donthineni wrote: > Hi Will, > > On 03/05/2018 09:56 AM, Will Deacon wrote: > > Hi Shanker, > > > > On Fri, Mar 02, 2018 at 03:50:18PM -0600, Shanker Donthineni wrote: > >> The function SMCCC_ARCH_WORKAROUND_1 was introduced as part of SMC > >> V1.1 Calling Convention to mitigate CVE-2017-5715. This patch uses > >> the standard call SMCCC_ARCH_WORKAROUND_1 for Falkor chips instead > >> of Silicon provider service ID 0xC2001700. > >> > >> Signed-off-by: Shanker Donthineni > >> --- > >> arch/arm64/include/asm/cpucaps.h | 2 +- > >> arch/arm64/include/asm/kvm_asm.h | 2 -- > >> arch/arm64/kernel/bpi.S | 8 ------ > >> arch/arm64/kernel/cpu_errata.c | 55 ++++++++++++++-------------------------- > >> arch/arm64/kvm/hyp/entry.S | 12 --------- > >> arch/arm64/kvm/hyp/switch.c | 10 -------- > >> 6 files changed, 20 insertions(+), 69 deletions(-) > > > > I'm happy to take this via arm64 if I get an ack from Marc/Christoffer. > > > >> diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h > >> index bb26382..6ecc249 100644 > >> --- a/arch/arm64/include/asm/cpucaps.h > >> +++ b/arch/arm64/include/asm/cpucaps.h > >> @@ -43,7 +43,7 @@ > >> #define ARM64_SVE 22 > >> #define ARM64_UNMAP_KERNEL_AT_EL0 23 > >> #define ARM64_HARDEN_BRANCH_PREDICTOR 24 > >> -#define ARM64_HARDEN_BP_POST_GUEST_EXIT 25 > >> +/* #define ARM64_UNALLOCATED_ENTRY 25 */ > >> #define ARM64_HAS_RAS_EXTN 26 > >> > >> #define ARM64_NCAPS 27 > > > > These aren't ABI, so I think you can just drop > > ARM64_HARDEN_BP_POST_GUEST_EXIT and repack the others accordingly. > > > Sure, I'll remove it completely in v2 patch. > > >> diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h > >> index 24961b7..ab4d0a9 100644 > >> --- a/arch/arm64/include/asm/kvm_asm.h > >> +++ b/arch/arm64/include/asm/kvm_asm.h > >> @@ -68,8 +68,6 @@ > >> > >> extern u32 __init_stage2_translation(void); > >> > >> -extern void __qcom_hyp_sanitize_btac_predictors(void); > >> - > >> #endif > >> > >> #endif /* __ARM_KVM_ASM_H__ */ > >> diff --git a/arch/arm64/kernel/bpi.S b/arch/arm64/kernel/bpi.S > >> index e5de335..dc4eb15 100644 > >> --- a/arch/arm64/kernel/bpi.S > >> +++ b/arch/arm64/kernel/bpi.S > >> @@ -55,14 +55,6 @@ ENTRY(__bp_harden_hyp_vecs_start) > >> .endr > >> ENTRY(__bp_harden_hyp_vecs_end) > >> > >> -ENTRY(__qcom_hyp_sanitize_link_stack_start) > >> - stp x29, x30, [sp, #-16]! > >> - .rept 16 > >> - bl . + 4 > >> - .endr > >> - ldp x29, x30, [sp], #16 > >> -ENTRY(__qcom_hyp_sanitize_link_stack_end) > >> - > >> .macro smccc_workaround_1 inst > >> sub sp, sp, #(8 * 4) > >> stp x2, x3, [sp, #(8 * 0)] > >> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c > >> index 52f15cd..d779ffd4 100644 > >> --- a/arch/arm64/kernel/cpu_errata.c > >> +++ b/arch/arm64/kernel/cpu_errata.c > >> @@ -67,8 +67,6 @@ static int cpu_enable_trap_ctr_access(void *__unused) > >> DEFINE_PER_CPU_READ_MOSTLY(struct bp_hardening_data, bp_hardening_data); > >> > >> #ifdef CONFIG_KVM > >> -extern char __qcom_hyp_sanitize_link_stack_start[]; > >> -extern char __qcom_hyp_sanitize_link_stack_end[]; > >> extern char __smccc_workaround_1_smc_start[]; > >> extern char __smccc_workaround_1_smc_end[]; > >> extern char __smccc_workaround_1_hvc_start[]; > >> @@ -115,8 +113,6 @@ static void __install_bp_hardening_cb(bp_hardening_cb_t fn, > >> spin_unlock(&bp_lock); > >> } > >> #else > >> -#define __qcom_hyp_sanitize_link_stack_start NULL > >> -#define __qcom_hyp_sanitize_link_stack_end NULL > >> #define __smccc_workaround_1_smc_start NULL > >> #define __smccc_workaround_1_smc_end NULL > >> #define __smccc_workaround_1_hvc_start NULL > >> @@ -161,12 +157,25 @@ static void call_hvc_arch_workaround_1(void) > >> arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_WORKAROUND_1, NULL); > >> } > >> > >> +static void qcom_link_stack_sanitization(void) > >> +{ > >> + u64 tmp; > >> + > >> + asm volatile("mov %0, x30 \n" > >> + ".rept 16 \n" > >> + "bl . + 4 \n" > >> + ".endr \n" > >> + "mov x30, %0 \n" > >> + : "=&r" (tmp)); > >> +} > >> + > >> static int enable_smccc_arch_workaround_1(void *data) > >> { > >> const struct arm64_cpu_capabilities *entry = data; > >> bp_hardening_cb_t cb; > >> void *smccc_start, *smccc_end; > >> struct arm_smccc_res res; > >> + u32 midr = read_cpuid_id(); > >> > >> if (!entry->matches(entry, SCOPE_LOCAL_CPU)) > >> return 0; > >> @@ -199,33 +208,15 @@ static int enable_smccc_arch_workaround_1(void *data) > >> return 0; > >> } > >> > >> + if (((midr & MIDR_CPU_MODEL_MASK) == MIDR_QCOM_FALKOR) || > >> + ((midr & MIDR_CPU_MODEL_MASK) == MIDR_QCOM_FALKOR_V1)) > >> + cb = qcom_link_stack_sanitization; > > > > Is this just a performance thing? Do you actually see an advantage over > > always making the firmware call? We've seen minimal impact in our testing. > > > > Yes, we've couple of advantages using the standard SMCCC_ARCH_WOKAROUND_1 framework. > - Improves the code readability. > - Avoid the unnecessary MIDR checks on each vCPU exit. > - Validates ID_AA64PFR0_CVS2 feature for Falkor chips. > - Avoids the 2nd link stack sanitization workaround in firmware. What I mean is, can we drop qcom_link_stack_sanitization altogether and use the SMCCC interface for everything? Will