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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 4EDE7C433EF for ; Thu, 14 Jul 2022 06:49:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Subject:Cc:To:From:Message-ID:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=mCjtPsAQ+zXQ9ys1wEI3qzzR5hwFNDvGHypHDgB+Ybs=; b=blTL2Q2mSW8Q9H SAZGlqXGJbRmIC4q9qiEUpoODlXf/fgS1MDKW722To4QDOHpJexT8V4xTyNiA/ZiqAcMNCfUDT7dN YDZafAahARmcanZl1EZIxD6MQFefMmUhl5rTPtvamxZEYhsYAZsdSOUOs4yDReorwuGlR69g3riXK DwejkfDSto/sfvCCiIWJoNEkI0e5ADsRJPdgzWyOb7FfiVT0CxJCeYCMvrmKxmJEJheNi/W3hOJNe Jwj9zrG3mP7LiJGaGh+PU3KBYPV9Oz9xmm/QZ158ciZFEDYIIZg+hHOGBZopVP8qYj3dy7E/wrbIZ yERngj1G1Xz2FiYL84KA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oBsds-00BYAh-2l; Thu, 14 Jul 2022 06:48:08 +0000 Received: from dfw.source.kernel.org ([139.178.84.217]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1oBsdp-00BY9H-7P for linux-arm-kernel@lists.infradead.org; Thu, 14 Jul 2022 06:48:07 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id E9D68617B2; Thu, 14 Jul 2022 06:48:03 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 340ABC34115; Thu, 14 Jul 2022 06:48:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1657781283; bh=MHf+Tti7P+0iJKKYfYQBBGEFmeCYo3qNRsvRhH/nr00=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=cQAj2JgOvUU2u9//3h2VdGWNazcwm7RM4Gq1VEYNCrL5gr+ornBWf2p32dw9IYHH0 QRm9Fq1Vpdmrtg692bfiWedLaaF9DhXpnIwGkaXD7LM98n790KVHKtu0exVpIGpVC3 GUES/3iSp57nTC1ZuQNDHmAnFU8uCcfqk8X1K1uyeRnmCRP3vese1oqxCQAmOi4d+y kZazRIUtdd3SFquPmsmPzZzdaT2E736KGAAwybRZAyifmnXTQBQ7GRE2yLpk4uK4Nf U4fsExyDI6ytnTb4UV2fFTEbwQWqLCuQ68XagyHnq32URrE4CaUVkGJFgcs1DOOrNN glrgGe5BQMoRw== Received: from ip-185-104-136-29.ptr.icomera.net ([185.104.136.29] helo=wait-a-minute.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1oBsdk-007NOk-Rn; Thu, 14 Jul 2022 07:48:01 +0100 Date: Thu, 14 Jul 2022 07:47:51 +0100 Message-ID: <878rowyyug.wl-maz@kernel.org> From: Marc Zyngier To: Peter Collingbourne Cc: linux-arm-kernel@lists.infradead.org, Will Deacon , Catalin Marinas , Mark Rutland , Ard Biesheuvel , broonie@kernel.org, danielmentz@google.com, saravanak@google.com Subject: Re: [PATCH] arm64: use sanitized feature registers for conditional ZCR_EL1 and SMCR_EL1 reads In-Reply-To: <20220713181852.3652212-1-pcc@google.com> References: <20220713181852.3652212-1-pcc@google.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") X-SA-Exim-Connect-IP: 185.104.136.29 X-SA-Exim-Rcpt-To: pcc@google.com, linux-arm-kernel@lists.infradead.org, will@kernel.org, catalin.marinas@arm.com, mark.rutland@arm.com, ardb@kernel.org, broonie@kernel.org, danielmentz@google.com, saravanak@google.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220713_234805_386226_F140B1CE X-CRM114-Status: GOOD ( 35.74 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , 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 On Wed, 13 Jul 2022 19:18:52 +0100, Peter Collingbourne wrote: > > With arm64.nosve we would still read ZCR_EL1 in __cpuinfo_store_cpu > because the condition for reading it was based on the unsanitized feature > register value info->reg_id_aa64pfr0. Fix the problem by moving the reads > to init_cpu_features after we have computed the sanitized value. Fix > the SMCR_EL1 read for SME similarly. > > Fixes: 504ee23611c4 ("arm64: Add the arm64.nosve command line option") > Signed-off-by: Peter Collingbourne > --- > arch/arm64/kernel/cpufeature.c | 11 +++++++---- > arch/arm64/kernel/cpuinfo.c | 8 -------- > 2 files changed, 7 insertions(+), 12 deletions(-) > > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c > index e5afa9eba85d..71d290fec36f 100644 > --- a/arch/arm64/kernel/cpufeature.c > +++ b/arch/arm64/kernel/cpufeature.c > @@ -999,15 +999,18 @@ void __init init_cpu_features(struct cpuinfo_arm64 *info) > if (id_aa64pfr0_32bit_el0(info->reg_id_aa64pfr0)) > init_32bit_cpu_features(&info->aarch32); > > - if (id_aa64pfr0_sve(info->reg_id_aa64pfr0)) { > + if (IS_ENABLED(CONFIG_ARM64_SVE) && > + id_aa64pfr0_sve(read_sanitised_ftr_reg(SYS_ID_AA64PFR0_EL1))) { > + info->reg_zcr = read_zcr_features(); > init_cpu_ftr_reg(SYS_ZCR_EL1, info->reg_zcr); > vec_init_vq_map(ARM64_VEC_SVE); > } > > - if (id_aa64pfr1_sme(info->reg_id_aa64pfr1)) { > + if (IS_ENABLED(CONFIG_ARM64_SME) && > + id_aa64pfr1_sme(read_sanitised_ftr_reg(SYS_ID_AA64PFR1_EL1))) { > + info->reg_smcr = read_smcr_features(); > init_cpu_ftr_reg(SYS_SMCR_EL1, info->reg_smcr); > - if (IS_ENABLED(CONFIG_ARM64_SME)) > - vec_init_vq_map(ARM64_VEC_SME); > + vec_init_vq_map(ARM64_VEC_SME); > } > > if (id_aa64pfr1_mte(info->reg_id_aa64pfr1)) > diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c > index 8eff0a34ffd4..66bc6f25d3b4 100644 > --- a/arch/arm64/kernel/cpuinfo.c > +++ b/arch/arm64/kernel/cpuinfo.c > @@ -418,14 +418,6 @@ static void __cpuinfo_store_cpu(struct cpuinfo_arm64 *info) > if (id_aa64pfr0_32bit_el0(info->reg_id_aa64pfr0)) > __cpuinfo_store_cpu_32bit(&info->aarch32); > > - if (IS_ENABLED(CONFIG_ARM64_SVE) && > - id_aa64pfr0_sve(info->reg_id_aa64pfr0)) > - info->reg_zcr = read_zcr_features(); > - > - if (IS_ENABLED(CONFIG_ARM64_SME) && > - id_aa64pfr1_sme(info->reg_id_aa64pfr1)) > - info->reg_smcr = read_smcr_features(); Not sure what this applies on, but that's a kernel that doesn't contain d69d564964872 ("arm64/sme: Expose SMIDR through sysfs"). > - > cpuinfo_detect_icache_policy(info); > } > This looks wrong to me. With this change, a secondary CPU never initialises its own view of reg_{zcr,smcr}. I came up with the following patch instead, also updating the cpuinfo_arm64 structure when updating the capabilities on secondary CPU boot (on top of arm64/boot): >From ee8d6a3741229336acedf13aa1304e05ed630ff0 Mon Sep 17 00:00:00 2001 From: Marc Zyngier Date: Mon, 11 Jul 2022 16:43:36 +0100 Subject: [PATCH] arm64: Delay initialisation of cpuinfo_arm64::reg_{zcr,smcr} Even if we are now able to tell the kernel to avoid exposing SVE/SME from the command line, we still have a couple of places where we unconditionally access the ZCR_EL1 (resp. SMCR_EL1) registers. On systems with broken firmwares, this results in a crash even if arm64.nosve (resp. arm64.nosme) was passed on the command-line. To avoid this, only update cpuinfo_arm64::reg_{zcr,smcr} once we have computed the sanitised version for the corresponding feature registers (ID_AA64PFR0 for SVE, and ID_AA64PFR1 for SME). This results in some minor refactoring. Signed-off-by: Marc Zyngier --- arch/arm64/kernel/cpufeature.c | 41 ++++++++++++++++++++++++---------- arch/arm64/kernel/cpuinfo.c | 16 ------------- 2 files changed, 29 insertions(+), 28 deletions(-) diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c index 1707576b7ca0..6adca2f337e8 100644 --- a/arch/arm64/kernel/cpufeature.c +++ b/arch/arm64/kernel/cpufeature.c @@ -1000,15 +1000,24 @@ void __init init_cpu_features(struct cpuinfo_arm64 *info) if (id_aa64pfr0_32bit_el0(info->reg_id_aa64pfr0)) init_32bit_cpu_features(&info->aarch32); - if (id_aa64pfr0_sve(info->reg_id_aa64pfr0)) { + if (IS_ENABLED(CONFIG_ARM64_SVE) && + id_aa64pfr0_sve(read_sanitised_ftr_reg(SYS_ID_AA64PFR0_EL1))) { + info->reg_zcr = read_zcr_features(); init_cpu_ftr_reg(SYS_ZCR_EL1, info->reg_zcr); vec_init_vq_map(ARM64_VEC_SVE); } - if (id_aa64pfr1_sme(info->reg_id_aa64pfr1)) { + if (IS_ENABLED(CONFIG_ARM64_SME) && + id_aa64pfr1_sme(read_sanitised_ftr_reg(SYS_ID_AA64PFR1_EL1))) { + info->reg_smcr = read_smcr_features(); + /* + * We mask out SMPS since even if the hardware + * supports priorities the kernel does not at present + * and we block access to them. + */ + info->reg_smidr = read_cpuid(SMIDR_EL1) & ~SMIDR_EL1_SMPS; init_cpu_ftr_reg(SYS_SMCR_EL1, info->reg_smcr); - if (IS_ENABLED(CONFIG_ARM64_SME)) - vec_init_vq_map(ARM64_VEC_SME); + vec_init_vq_map(ARM64_VEC_SME); } if (id_aa64pfr1_mte(info->reg_id_aa64pfr1)) @@ -1240,23 +1249,31 @@ void update_cpu_features(int cpu, taint |= check_update_ftr_reg(SYS_ID_AA64SMFR0_EL1, cpu, info->reg_id_aa64smfr0, boot->reg_id_aa64smfr0); - if (id_aa64pfr0_sve(info->reg_id_aa64pfr0)) { + if (IS_ENABLED(CONFIG_ARM64_SVE) && + id_aa64pfr0_sve(read_sanitised_ftr_reg(SYS_ID_AA64PFR0_EL1))) { + info->reg_zcr = read_zcr_features(); taint |= check_update_ftr_reg(SYS_ZCR_EL1, cpu, info->reg_zcr, boot->reg_zcr); - /* Probe vector lengths, unless we already gave up on SVE */ - if (id_aa64pfr0_sve(read_sanitised_ftr_reg(SYS_ID_AA64PFR0_EL1)) && - !system_capabilities_finalized()) + /* Probe vector lengths */ + if (!system_capabilities_finalized()) vec_update_vq_map(ARM64_VEC_SVE); } - if (id_aa64pfr1_sme(info->reg_id_aa64pfr1)) { + if (IS_ENABLED(CONFIG_ARM64_SME) && + id_aa64pfr1_sme(read_sanitised_ftr_reg(SYS_ID_AA64PFR1_EL1))) { + info->reg_smcr = read_smcr_features(); + /* + * We mask out SMPS since even if the hardware + * supports priorities the kernel does not at present + * and we block access to them. + */ + info->reg_smidr = read_cpuid(SMIDR_EL1) & ~SMIDR_EL1_SMPS; taint |= check_update_ftr_reg(SYS_SMCR_EL1, cpu, info->reg_smcr, boot->reg_smcr); - /* Probe vector lengths, unless we already gave up on SME */ - if (id_aa64pfr1_sme(read_sanitised_ftr_reg(SYS_ID_AA64PFR1_EL1)) && - !system_capabilities_finalized()) + /* Probe vector lengths */ + if (!system_capabilities_finalized()) vec_update_vq_map(ARM64_VEC_SME); } diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c index 3628b8a28e92..5b53d275adc1 100644 --- a/arch/arm64/kernel/cpuinfo.c +++ b/arch/arm64/kernel/cpuinfo.c @@ -438,22 +438,6 @@ static void __cpuinfo_store_cpu(struct cpuinfo_arm64 *info) if (id_aa64pfr0_32bit_el0(info->reg_id_aa64pfr0)) __cpuinfo_store_cpu_32bit(&info->aarch32); - if (IS_ENABLED(CONFIG_ARM64_SVE) && - id_aa64pfr0_sve(info->reg_id_aa64pfr0)) - info->reg_zcr = read_zcr_features(); - - if (IS_ENABLED(CONFIG_ARM64_SME) && - id_aa64pfr1_sme(info->reg_id_aa64pfr1)) { - info->reg_smcr = read_smcr_features(); - - /* - * We mask out SMPS since even if the hardware - * supports priorities the kernel does not at present - * and we block access to them. - */ - info->reg_smidr = read_cpuid(SMIDR_EL1) & ~SMIDR_EL1_SMPS; - } - cpuinfo_detect_icache_policy(info); } -- 2.34.1 -- Without deviation from the norm, progress is not possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel