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 ECD30E7717F for ; Mon, 16 Dec 2024 15:54:59 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Type:MIME-Version: References:In-Reply-To:Subject:Cc:To:From:Message-ID:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=CFseZlhQsBozp7nqVIdMl7CvRh3Xfaeii6VYsDgLoc0=; b=4+kzXpr0D4CrU/Od2M8k86RAOD pSkibz39l8sBCPUomzlYyckcWqulELV4Kp6zwhkFb3L1ZdEpELV0QYeBIi+oMGbZSAsBHCQQEBzqL dW+nQldskaTVeOloUhVFW8cetDVqzwLX6kosAT8qhzyZMF8xBJvhQuJ5Vd4RsiNDy9JuswiC6zCTG IhI3nFfmdXNwBdpPWYsDfm5spn+x082ucFBdkC+1ZAVjjPGvp3zr8Ynsx5WRXRgPGdj7Zop0UUmqS WXf7s5/pNfkY+ZLnTENU0MToGbyuLZVFwVGo8SzJ4g56UUyxi41oezwk0M2xs/0XL4TcMZSELeQBQ kVxaO0Xg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1tNDQo-0000000AU4l-0uHz; Mon, 16 Dec 2024 15:54:50 +0000 Received: from desiato.infradead.org ([2001:8b0:10b:1:d65d:64ff:fe57:4e05]) by bombadil.infradead.org with esmtps (Exim 4.98 #2 (Red Hat Linux)) id 1tNC8W-0000000AGqq-3Kay for linux-arm-kernel@bombadil.infradead.org; Mon, 16 Dec 2024 14:31:52 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=desiato.20200630; h=Content-Type:MIME-Version:References: In-Reply-To:Subject:Cc:To:From:Message-ID:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=CFseZlhQsBozp7nqVIdMl7CvRh3Xfaeii6VYsDgLoc0=; b=BvekzmVKOCUtIdZsxLnz9N255f k2ZSuZkgwKXhD1VD803mBn4ujLWp4x4GNITFYURrVJD4iorjvxC0wa3J4Hpke+7UJFn32jzLDud1N ddE9tHBvaW50qKkM+8fMmGb0MKNWKzXoA5cBb0FT5uaZBmocJmmKBferjJxIh4XBxYDcvn1O/U47M s4vo2IBVZxj1h9Uo9cfkh0b6Gmrm3VgnTayD63w3V4n4Qo1lGBTxQtiiiCw9TZ2/B4/w8xKQwmKXc ZmOE3beZRlLS5H7fI3jj5w/aJUYDLW/k0GFcl8yFmYGo4VUjoeYhHV7jpk95Gtl5Fg+er65toZ49k e/nqffHw==; Received: from dfw.source.kernel.org ([139.178.84.217]) by desiato.infradead.org with esmtps (Exim 4.98 #2 (Red Hat Linux)) id 1tNC8T-00000004rKg-3GHq for linux-arm-kernel@lists.infradead.org; Mon, 16 Dec 2024 14:31:51 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 1B2C45C4989; Mon, 16 Dec 2024 14:31:04 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id EE72EC4CED0; Mon, 16 Dec 2024 14:31:45 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1734359506; bh=q//uXuHw5NNyHRrYJA+t5Do7tOiMP5mB0zY7GpysFuw=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=avK1n6GRfEbGCiA5MGkHoGcE+AINRnA4Gm4Gxqm4ZzgfVfxrRshODochg9mUYzZoA EuaMvt+DZPrqbCSp17+I2Qt+UtkV5f7U2hMg47Xq8me29/Jo4ceCrd0mkFdqAdQ+K+ 0/4PjG18v2Wih8ks4QULfOjTjEiCgWZpwGFG6+dz7MSBwEi6bgyD/7ojs5jH8t8ngb dcL/rq39tKYYQPEFKw5HTN7UEAKAATnPq/+9hL7w3Gz70NEBiydO7aVqNTU54oXK4K wXAS49UjQ6842fF2nciDYfbnEmOJVId1jXhIujipWFqyR5Tf9UQIQlsoEtmCFm47x8 nZLFKKiHX7Gog== Received: from sofa.misterjones.org ([185.219.108.64] helo=goblin-girl.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 1tNC8N-004CsH-N6; Mon, 16 Dec 2024 14:31:43 +0000 Date: Mon, 16 Dec 2024 14:31:43 +0000 Message-ID: <865xnjsnqo.wl-maz@kernel.org> From: Marc Zyngier To: Mark Rutland Cc: Mark Brown , Catalin Marinas , Will Deacon , Peter Collingbourne , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org Subject: Re: [PATCH] arm64/sme: Move storage of reg_smidr to __cpuinfo_store_cpu() In-Reply-To: References: <20241214-arm64-fix-boot-cpu-smidr-v1-1-0745c40772dd@kernel.org> <87a5cysfci.wl-maz@kernel.org> 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/29.4 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) X-TUID: 7+uNRFeFxaHZ MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: mark.rutland@arm.com, broonie@kernel.org, catalin.marinas@arm.com, will@kernel.org, pcc@google.com, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org 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-20241216_143150_224185_4508550D X-CRM114-Status: GOOD ( 32.32 ) 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: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, 16 Dec 2024 12:38:17 +0000, Mark Rutland wrote: > > On Sat, Dec 14, 2024 at 10:56:13AM +0000, Marc Zyngier wrote: > > > Why isn't the following a good enough fix? It makes it plain that > > boot_cpu_data is only a copy of CPU0's initial boot state. > > > > diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c > > index d79e88fccdfce..0cbb42fd48850 100644 > > --- a/arch/arm64/kernel/cpuinfo.c > > +++ b/arch/arm64/kernel/cpuinfo.c > > @@ -497,6 +497,6 @@ void __init cpuinfo_store_boot_cpu(void) > > struct cpuinfo_arm64 *info = &per_cpu(cpu_data, 0); > > __cpuinfo_store_cpu(info); > > > > + init_cpu_features(info); > > boot_cpu_data = *info; > > - init_cpu_features(&boot_cpu_data); > > } > > I think that change in isolation is fine, but I don't think that's the > right fix. > > I think that what we did in commit: > > 892f7237b3ff ("arm64: Delay initialisation of cpuinfo_arm64::reg_{zcr,smcr}") > > ... introduces an anti-pattern that'd be nice to avoid. That broke the > existing split of __cpuinfo_store_cpu() and init_cpu_features(), where > the former read the ID regs, and the latter set up the features > *without* altering the copy of the ID regs that was read. i.e. > init_cpu_features() shouldn't write to its info argument at all. > > I understand that we have to do something as a bodge for broken FW which > traps SME, but I'd much rather we did that within __cpuinfo_store_cpu(). Honestly, I'd rather revert that patch, together with b3000e2133d8 ("arm64: Add the arm64.nosme command line option"). I'm getting tired of the FW nonsense, and we are only allowing vendors to ship untested crap. Furthermore, given the state of SME in the kernel, I don't think this is makes any difference. So maybe this is the right time to reset everything to a sane state. > Can we add something to check whether SME was disabled on the command > line, and use that in __cpuinfo_store_cpu(), effectively reverting > 892f7237b3ff? Maybe, but that'd be before any sanitisation of the overrides, so it would have to severely limit its scope. Something like this, which I haven't tested: diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c index d79e88fccdfce..9e9295e045009 100644 --- a/arch/arm64/kernel/cpuinfo.c +++ b/arch/arm64/kernel/cpuinfo.c @@ -492,10 +492,22 @@ void cpuinfo_store_cpu(void) update_cpu_features(smp_processor_id(), info, &boot_cpu_data); } +static void cpuinfo_apply_overrides(struct cpuinfo_arm64 *info) +{ + if (FIELD_GET(ID_AA64PFR0_EL1_SVE, id_aa64pfr0_override.mask) && + !FIELD_GET(ID_AA64PFR0_EL1_SVE, id_aa64pfr0_override.val)) + info->reg_id_aa64pfr0 &= ~ID_AA64PFR0_EL1_SVE; + + if (FIELD_GET(ID_AA64PFR1_EL1_SME, id_aa64pfr1_override.mask) && + !FIELD_GET(ID_AA64PFR1_EL1_SME, id_aa64pfr1_override.val)) + info->reg_id_aa64pfr1 &= ~ID_AA64PFR1_EL1_SME; +} + void __init cpuinfo_store_boot_cpu(void) { struct cpuinfo_arm64 *info = &per_cpu(cpu_data, 0); __cpuinfo_store_cpu(info); + cpuinfo_apply_overrides(info); boot_cpu_data = *info; init_cpu_features(&boot_cpu_data); But this will have ripple effects on the rest of the override code (the kernel messages are likely to be wrong). M. -- Without deviation from the norm, progress is not possible.