From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5076D1F9ECC for ; Fri, 3 Jan 2025 17:58:19 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1735927099; cv=none; b=RwDfgDmMHfkcayfgdVfRtQsp64CTUXGn3iPqqUAezMUhd/Oyu9DrIzLoFSzyP06x6vgtqa8AJmjo6HXvuosug0tNWaym55/40eV0JfazaFqTv2JHl/bwT8OCJOifIVf6Dp8NN0SqSQ+JckUz6jLbha/YBGIrVqK1M2yl9UC9h/c= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1735927099; c=relaxed/simple; bh=xPTgQs/doiFDCJjEZ/ssYGg/MAyYHJOWQhy9vrGE9eg=; h=Date:Message-ID:From:To:Cc:Subject:In-Reply-To:References: MIME-Version:Content-Type; b=ma6xrZxcvZrS5Z1EIpc7DmvzBFb1mFQBORw9smoDyMszXWlweDGUQMPQJ2v0IsCCQ2IYL3s2prC4hMXA3G/N/eEV4KtDEOI+ND5Fb01KTvq73zspgvaSY2/jsNn3LSzyInrAToZvN2erjXfwUahA1UjwcmQGBQAoK6UulM2eK2Y= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=LN+6DiYt; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="LN+6DiYt" Received: by smtp.kernel.org (Postfix) with ESMTPSA id D4F56C4CECE; Fri, 3 Jan 2025 17:58:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1735927098; bh=xPTgQs/doiFDCJjEZ/ssYGg/MAyYHJOWQhy9vrGE9eg=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=LN+6DiYtb1BUu5bUrsoF8G7Yg8CUYivxAYuTlruUIcD8U9vmRN2SPKNhmg0mpt2lv fVx7JnoswGDrEJddnOuPmUG0hiXAjLJj0p8LwOhE/rQGMCKvUAQn0WIpgxwKPr/UCB Qz9vyoWON3wbee88YwkWxJf5wKE756SEjEE0UXh/q2m1qk31WEISvHkSjncqWfMD6O kausnWkxFKqhx2UCjSsmKPtk4ziFlF3v5UVCk7Bu2dZfwprnUkesSvrHp8aDwXzk3M eXXh15bD3SK46QLi+2P23LfpnTm9YdHpevcXazgBb6Zp73vftrO1xsRy9harHJrzBQ fDxRkCIYvP4IQ== Received: from sofa.misterjones.org ([185.219.108.64] 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 1tTlw8-008lRK-Os; Fri, 03 Jan 2025 17:58:16 +0000 Date: Fri, 03 Jan 2025 17:58:18 +0000 Message-ID: <87msg7rd8l.wl-maz@kernel.org> From: Marc Zyngier To: Catalin Marinas Cc: linux-arm-kernel@lists.infradead.org, Will Deacon , Mark Rutland , Mark Brown , stable@vger.kernel.org Subject: Re: [PATCH] arm64: Filter out SVE hwcaps when FEAT_SVE isn't implemented In-Reply-To: References: <20250103142635.1759674-1-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) Precedence: bulk X-Mailing-List: stable@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: 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: catalin.marinas@arm.com, linux-arm-kernel@lists.infradead.org, will@kernel.org, mark.rutland@arm.com, broonie@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 On Fri, 03 Jan 2025 17:20:05 +0000, Catalin Marinas wrote: > > On Fri, Jan 03, 2025 at 02:26:35PM +0000, Marc Zyngier wrote: > > The hwcaps code that exposes SVE features to userspace only > > considers ID_AA64ZFR0_EL1, while this is only valid when > > ID_AA64PFR0_EL1.SVE advertises that SVE is actually supported. > > > > The expectations are that when ID_AA64PFR0_EL1.SVE is 0, the > > ID_AA64ZFR0_EL1 register is also 0. So far, so good. > > > > Things become a bit more interesting if the HW implements SME. > > In this case, a few ID_AA64ZFR0_EL1 fields indicate *SME* > > features. And these fields overlap with their SVE interpretations. > > But the architecture says that the SME and SVE feature sets must > > match, so we're still hunky-dory. > > > > This goes wrong if the HW implements SME, but not SVE. In this > > case, we end-up advertising some SVE features to userspace, even > > if the HW has none. That's because we never consider whether SVE > > is actually implemented. Oh well. > > > > Fix it by restricting all SVE capabilities to ID_AA64PFR0_EL1.SVE > > being non-zero. > > > > Reported-by: Catalin Marinas > > Signed-off-by: Marc Zyngier > > Cc: Will Deacon > > Cc: Mark Rutland > > Cc: Mark Brown > > Cc: stable@vger.kernel.org > > I'd add: > > Fixes: 06a916feca2b ("arm64: Expose SVE2 features for userspace") > > While at the time the code was correct, the architecture messed up our > assumptions with the introduction of SME. Good point. > > > @@ -3022,6 +3027,13 @@ static const struct arm64_cpu_capabilities arm64_features[] = { > > .matches = match, \ > > } > > > > +#define HWCAP_CAP_MATCH_ID(match, reg, field, min_value, cap_type, cap) \ > > + { \ > > + __HWCAP_CAP(#cap, cap_type, cap) \ > > + HWCAP_CPUID_MATCH(reg, field, min_value) \ > > + .matches = match, \ > > + } > > Do we actually need this macro? It is either this macro, or a large-ish switch/case statement doing the same thing. See below. > > > + > > #ifdef CONFIG_ARM64_PTR_AUTH > > static const struct arm64_cpu_capabilities ptr_auth_hwcap_addr_matches[] = { > > { > > @@ -3050,6 +3062,18 @@ static const struct arm64_cpu_capabilities ptr_auth_hwcap_gen_matches[] = { > > }; > > #endif > > > > +#ifdef CONFIG_ARM64_SVE > > +static bool has_sve(const struct arm64_cpu_capabilities *cap, int scope) > > +{ > > + u64 aa64pfr0 = __read_scoped_sysreg(SYS_ID_AA64PFR0_EL1, scope); > > + > > + if (FIELD_GET(ID_AA64PFR0_EL1_SVE, aa64pfr0) < ID_AA64PFR0_EL1_SVE_IMP) > > + return false; > > + > > + return has_user_cpuid_feature(cap, scope); > > +} > > +#endif > > We can name this has_sve_feature() and use it with the existing > HWCAP_CAP_MATCH() macro. I think it would look identical. I don't think that works. HWCAP_CAP_MATCH() doesn't take the reg/field/limit information that we need to compute the capability. Without such information neatly populated in arm64_cpu_capabilities, you can't call has_user_cpuid_feature(). A previous incarnation of the same patch was using that macro. But you then end-up with having to map the cap with the field/limit and perform the check "by hand". Roughly, this would look like this: diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c index d793ca08549cd..76566a8bcdd3c 100644 --- a/arch/arm64/kernel/cpufeature.c +++ b/arch/arm64/kernel/cpufeature.c @@ -3065,12 +3065,22 @@ static const struct arm64_cpu_capabilities ptr_auth_hwcap_gen_matches[] = { #ifdef CONFIG_ARM64_SVE static bool has_sve(const struct arm64_cpu_capabilities *cap, int scope) { - u64 aa64pfr0 = __read_scoped_sysreg(SYS_ID_AA64PFR0_EL1, scope); + u64 zfr0; - if (FIELD_GET(ID_AA64PFR0_EL1_SVE, aa64pfr0) < ID_AA64PFR0_EL1_SVE_IMP) + if (!system_supports_sve()) return false; - return has_user_cpuid_feature(cap, scope); + zfr0 = __read_scoped_sysreg(SYS_ID_AA64ZFR0_EL1, scope); + + switch (cap->cap) { + case KERNEL_HWCAP_SVE2P1: + return SYS_FIELF_GET(SYS_ID_AA64ZFR0_EL1, SVEver) >= SYS_ID_AA64ZFR0_EL1_SVEver_SVE2p1; + case KERNEL_HWCAP_SVE2: + return SYS_FIELF_GET(SYS_ID_AA64ZFR0_EL1, SVEver) >= SYS_ID_AA64ZFR0_EL1_SVEver_SVE2; + case KERNEL_HWCAP_SVEAES: + return SYS_FIELF_GET(SYS_ID_AA64ZFR0_EL1, AES) >= SYS_ID_AA64ZFR0_EL1_AES_IMP; + [...] + } } #endif Frankly, I don't think this is worth it, and you still need to hack read_scoped_sysreg(). > > We might even be able to use system_supports_sve() directly and avoid > changing read_scoped_sysreg(). setup_user_features() is called in > smp_cpus_done() after setup_system_features(), so using > system_supports_sve() directly should be fine here. Yeah, that should work. Thanks, M. -- Without deviation from the norm, progress is not possible.