From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 78D70C388F7 for ; Tue, 10 Nov 2020 10:41:23 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 069D2206B2 for ; Tue, 10 Nov 2020 10:41:22 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="uOuWgKzW" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 069D2206B2 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=QrS4/m0VHw17q7HkshlrhAuW+4I438r2XeVDwNGXTlU=; b=uOuWgKzWAI8rT7gWjoE1+wGmx GxI0MjibN2K4pXk8hi0igGk2ETavetFW8CteygYRFFltv9+2k2dPbHnUoykp7Z9eCEtLseky6YkP6 ygcHUEkTlWqh7JSfMraz2LOPtQWJzziFd0gsJip57MLA5rcgpRieuTUtQl7blOjVqqqJuYgzQIzdv /dLHvM/RBhW2NxNE/zJVPojEyRIxlYWO0qH9gHNjLoX/NFlxiGf/+s3EP+vBP1m5caADGS4OAJRjJ u2N2hL6XiOVC2MlREHHj5yRUsvH8QIYabCjx2NgBtT25YydjOFfDty20u9OXEiomYIGv0feRlnMoD zyAKs9fsA==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kcR4M-0005ck-Mr; Tue, 10 Nov 2020 10:40:10 +0000 Received: from foss.arm.com ([217.140.110.172]) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kcR4K-0005bn-7A for linux-arm-kernel@lists.infradead.org; Tue, 10 Nov 2020 10:40:09 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 974A711D4; Tue, 10 Nov 2020 02:40:04 -0800 (PST) Received: from ewhatever.cambridge.arm.com (ewhatever.cambridge.arm.com [10.1.197.1]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 625913F6CF; Tue, 10 Nov 2020 02:40:03 -0800 (PST) Date: Tue, 10 Nov 2020 10:39:57 +0000 From: Suzuki K Poulose To: Catalin Marinas Subject: Re: [PATCH] arm64: errata: Fix handling of 1418040 with late CPU onlining Message-ID: <20201110103957.GA1104295@ewhatever.cambridge.arm.com> References: <20201106114952.10032-1-will@kernel.org> <20201106124400.GF29329@gaia> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20201106124400.GF29329@gaia> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201110_054008_410599_8521031A X-CRM114-Status: GOOD ( 35.76 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , Sai Prakash Ranjan , suzuki.poulose@arm.com, Will Deacon , Stephen Boyd , Marc Zyngier , kernel-team@android.com, linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Catalin On Fri, Nov 06, 2020 at 12:44:00PM +0000, Catalin Marinas wrote: > On Fri, Nov 06, 2020 at 12:18:32PM +0000, Suzuki K Poulose wrote: > > On 11/6/20 11:49 AM, Will Deacon wrote: > > > In a surprising turn of events, it transpires that CPU capabilities > > > configured as ARM64_CPUCAP_WEAK_LOCAL_CPU_FEATURE are never set as the > > > result of late-onlining. Therefore our handling of erratum 1418040 does > > > not get activated if it is not required by any of the boot CPUs, even > > > though we allow late-onlining of an affected CPU. > > > > The capability state is not altered after the SMP boot for all types > > of caps. The weak caps are there to allow a late CPU to turn online > > without getting "banned". This may be something we could relax with > > a new flag in the scope. > > Like this? Of course, it needs some testing. Yes, this is exactly what I had in mind. However, there is some concern over how we expose it. > > diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h > index 97244d4feca9..b896e72131d7 100644 > --- a/arch/arm64/include/asm/cpufeature.h > +++ b/arch/arm64/include/asm/cpufeature.h > @@ -246,6 +246,8 @@ extern struct arm64_ftr_reg arm64_ftr_reg_ctrel0; > #define ARM64_CPUCAP_OPTIONAL_FOR_LATE_CPU ((u16)BIT(5)) > /* Panic when a conflict is detected */ > #define ARM64_CPUCAP_PANIC_ON_CONFLICT ((u16)BIT(6)) > +/* Together with PERMITTED_FOR_LATE_CPU, set the corresponding cpu_hwcaps bit */ > +#define ARM64_CPUCAP_SET_FOR_LATE_CPU ((u16)BIT(7)) > > /* > * CPU errata workarounds that need to be enabled at boot time if one or > @@ -481,6 +483,16 @@ static __always_inline bool cpus_have_const_cap(int num) > return cpus_have_cap(num); > } > > +/* > + * Test for a capability with a runtime check. This is an alias for > + * cpus_have_cap() but with the name chosen to emphasize the applicability to > + * late capability setting. > + */ > +static __always_inline bool cpus_have_late_cap(int num) > +{ > + return cpus_have_cap(num); > +} > + It is quite easy for people to misuse the ubiquitous cpus_have_const_cap(). So, unless we make sure that we fix that to handle these "caps" we might see subtle bugs. How about the following fix on top of your patch, to add another (sigh!) set of keys to detect if we can use the const caps or not. Untested... ---8>--- diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h index 849b2691fadd..84dd43f1f322 100644 --- a/arch/arm64/include/asm/cpufeature.h +++ b/arch/arm64/include/asm/cpufeature.h @@ -379,6 +379,7 @@ cpucap_multi_entry_cap_matches(const struct arm64_cpu_capabilities *entry, extern DECLARE_BITMAP(cpu_hwcaps, ARM64_NCAPS); extern struct static_key_false cpu_hwcap_keys[ARM64_NCAPS]; +extern struct static_key_false cpu_hwcap_const_key[ARM64_NCAPS]; extern struct static_key_false arm64_const_caps_ready; /* ARM64 CAPS + alternative_cb */ @@ -414,6 +415,14 @@ static inline bool cpus_have_cap(unsigned int num) return test_bit(num, cpu_hwcaps); } +static __always_inline bool cpucap_has_const_key(int num) +{ + if (num >= ARM64_NCAPS) + return false; + + return static_branch_unlikely(&cpu_hwcap_const_key[num]); +} + /* * Test for a capability without a runtime check. * @@ -424,7 +433,7 @@ static inline bool cpus_have_cap(unsigned int num) */ static __always_inline bool __cpus_have_const_cap(int num) { - if (num >= ARM64_NCAPS) + if (num >= ARM64_NCAPS && WARN_ON(!cpucap_has_const_key(num))) return false; return static_branch_unlikely(&cpu_hwcap_keys[num]); } @@ -439,7 +448,7 @@ static __always_inline bool __cpus_have_const_cap(int num) */ static __always_inline bool cpus_have_const_cap(int num) { - if (system_capabilities_finalized()) + if (cpucap_has_const_key(num) && system_capabilities_finalized()) return __cpus_have_const_cap(num); else return cpus_have_cap(num); diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c index 51e63be41ea5..a6c3e4e102c9 100644 --- a/arch/arm64/kernel/cpufeature.c +++ b/arch/arm64/kernel/cpufeature.c @@ -127,7 +127,9 @@ void dump_cpu_features(void) } DEFINE_STATIC_KEY_ARRAY_FALSE(cpu_hwcap_keys, ARM64_NCAPS); +DEFINE_STATIC_KEY_ARRAY_FALSE(cpu_hwcap_const_key, ARM64_NCAPS); EXPORT_SYMBOL(cpu_hwcap_keys); +EXPORT_SYMBOL(cpu_hwcap_const_key); #define __ARM64_FTR_BITS(SIGNED, VISIBLE, STRICT, TYPE, SHIFT, WIDTH, SAFE_VAL) \ { \ @@ -2426,7 +2428,10 @@ static void __init enable_cpu_capabilities(u16 scope_mask) continue; /* Ensure cpus_have_const_cap(num) works */ - static_branch_enable(&cpu_hwcap_keys[num]); + if (!cpucap_set_for_late_cpu(caps)) { + static_branch_enable(&cpu_hwcap_const_key[num]); + static_branch_enable(&cpu_hwcap_keys[num]); + } if (boot_scope && caps->cpu_enable) /* diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h index 843ecfb16a69..0ff9329670b7 100644 --- a/arch/arm64/kernel/image-vars.h +++ b/arch/arm64/kernel/image-vars.h @@ -90,6 +90,7 @@ KVM_NVHE_ALIAS(__icache_flags); /* Kernel symbols needed for cpus_have_final/const_caps checks. */ KVM_NVHE_ALIAS(arm64_const_caps_ready); KVM_NVHE_ALIAS(cpu_hwcap_keys); +KVM_NVHE_ALIAS(cpu_hwcap_const_key); KVM_NVHE_ALIAS(cpu_hwcaps); /* Static keys which are set if a vGIC trap should be handled in hyp. */ -- > static inline void cpus_set_cap(unsigned int num) > { > if (num >= ARM64_NCAPS) { > diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c > index 61314fd70f13..6b7de7292e8c 100644 > --- a/arch/arm64/kernel/cpu_errata.c > +++ b/arch/arm64/kernel/cpu_errata.c > @@ -481,7 +481,8 @@ const struct arm64_cpu_capabilities arm64_errata[] = { > * also need the non-affected CPUs to be able to come > * in at any point in time. Wonderful. > */ > - .type = ARM64_CPUCAP_WEAK_LOCAL_CPU_FEATURE, > + .type = ARM64_CPUCAP_WEAK_LOCAL_CPU_FEATURE | > + ARM64_CPUCAP_SET_FOR_LATE_CPU, > }, > #endif > #ifdef CONFIG_ARM64_WORKAROUND_SPECULATIVE_AT > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c > index dcc165b3fc04..51e63be41ea5 100644 > --- a/arch/arm64/kernel/cpufeature.c > +++ b/arch/arm64/kernel/cpufeature.c > @@ -1720,6 +1720,12 @@ cpucap_late_cpu_permitted(const struct arm64_cpu_capabilities *cap) > return !!(cap->type & ARM64_CPUCAP_PERMITTED_FOR_LATE_CPU); > } > > +static bool > +cpucap_set_for_late_cpu(const struct arm64_cpu_capabilities *cap) > +{ > + return !!(cap->type & ARM64_CPUCAP_SET_FOR_LATE_CPU); > +} > + > static bool > cpucap_panic_on_conflict(const struct arm64_cpu_capabilities *cap) > { > @@ -2489,6 +2495,11 @@ static void verify_local_cpu_caps(u16 scope_mask) > */ > if (cpu_has_cap && !cpucap_late_cpu_permitted(caps)) > break; > + /* > + * Set the capability bit if it allows late setting. > + */ > + if (cpucap_set_for_late_cpu(caps)) > + cpus_set_cap(caps->capability); > } > } > > diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c > index 4784011cecac..152639962845 100644 > --- a/arch/arm64/kernel/process.c > +++ b/arch/arm64/kernel/process.c > @@ -523,7 +523,7 @@ static void erratum_1418040_thread_switch(struct task_struct *prev, > u64 val; > > if (!(IS_ENABLED(CONFIG_ARM64_ERRATUM_1418040) && > - cpus_have_const_cap(ARM64_WORKAROUND_1418040))) > + cpus_have_late_cap(ARM64_WORKAROUND_1418040))) > return; > > prev32 = is_compat_thread(task_thread_info(prev)); _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel