linux-kernel - Re: [RFC PATCH v1 3/5] arm64: Add errata and workarounds for systems with broken BBML2

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241213162103.GB30314@mazurka.cambridge.arm.com>
Date: Fri, 13 Dec 2024 16:21:03 +0000
From: Mikołaj Lenczewski <miko.lenczewski@....com>
To: Marc Zyngier <maz@...nel.org>
Cc: catalin.marinas@....com, will@...nel.org, corbet@....net,
	oliver.upton@...ux.dev, joey.gouly@....com, suzuki.poulose@....com,
	yuzenghui@...wei.com, linux-arm-kernel@...ts.infradead.org,
	linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
	kvmarm@...ts.linux.dev
Subject: Re: [RFC PATCH v1 3/5] arm64: Add errata and workarounds for systems
 with broken BBML2

On Wed, Dec 11, 2024 at 04:52:49PM +0000, Marc Zyngier wrote:
> On Wed, 11 Dec 2024 15:45:04 +0000,
> Mikołaj Lenczewski <miko.lenczewski@....com> wrote:
> > 
> > There are systems which claim support for BBML2, but whose
> > implementation of this support is broken. Add a Kconfig erratum for each
> > of these systems, and a cpufeature workaround that forces the supported
> > BBM level on these systems to 0.
> > 
> > Signed-off-by: Mikołaj Lenczewski <miko.lenczewski@....com>
> > ---
> >  Documentation/arch/arm64/silicon-errata.rst |  32 ++++
> >  arch/arm64/Kconfig                          | 164 ++++++++++++++++++++
> >  arch/arm64/kernel/cpufeature.c              |  32 +++-
> >  3 files changed, 227 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > index 100570a048c5..9ef8418e8410 100644
> > --- a/arch/arm64/Kconfig
> > +++ b/arch/arm64/Kconfig
> > @@ -1127,6 +1127,170 @@ config ARM64_ERRATUM_3194386
> >  
> >  	  If unsure, say Y.
> >  
> > +config ARM64_WORKAROUND_BROKEN_BBML2_SUPPORT
> > +	bool
> > +
> > +config ARM64_ERRATUM_3696250
> > +	bool "Neoverse-N2: workaround for broken BBM level 2 support"
> > +	default y
> > +	select ARM64_WORKAROUND_BROKEN_BBML2_SUPPORT
> > +	help
> > +	  Affected Neoverse-N2 cores (r0p0, r0p1, r0p2, r0p3) declare
> 
> So you list a number of affected revisions...
> 
> [...]
> 
> > +static bool has_bbml2(const struct arm64_cpu_capabilities *entry,
> > +		      int scope)
> > +{
> > +	if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_BROKEN_BBML2_SUPPORT)) {
> > +		static const struct midr_range broken_bbml2_list[] = {
> > +			MIDR_ALL_VERSIONS(MIDR_CORTEX_A76),
> > +			MIDR_ALL_VERSIONS(MIDR_CORTEX_A77),
> > +			MIDR_ALL_VERSIONS(MIDR_CORTEX_A78),
> > +			MIDR_ALL_VERSIONS(MIDR_CORTEX_A78C),
> > +			MIDR_ALL_VERSIONS(MIDR_CORTEX_A710),
> > +			MIDR_ALL_VERSIONS(MIDR_CORTEX_X1),
> > +			MIDR_ALL_VERSIONS(MIDR_CORTEX_X2),
> > +			MIDR_ALL_VERSIONS(MIDR_CORTEX_X3),
> > +			MIDR_ALL_VERSIONS(MIDR_CORTEX_X4),
> > +			MIDR_ALL_VERSIONS(MIDR_CORTEX_X925),
> > +			MIDR_ALL_VERSIONS(MIDR_NEOVERSE_N1),
> > +			MIDR_ALL_VERSIONS(MIDR_NEOVERSE_N2),
> > +			MIDR_ALL_VERSIONS(MIDR_NEOVERSE_V1),
> > +			MIDR_ALL_VERSIONS(MIDR_NEOVERSE_V2),
> > +			MIDR_ALL_VERSIONS(MIDR_NEOVERSE_V3),
> > +			{}
> 
> ... and yet you flag all versions as broken? So which one is it? If it
> is really the case that all versions are broken, then the text should
> be simplified. Otherwise, this should really list the broken versions.
> 
> The other thing is that I find it incredibly dangerous to rely on
> some config option to disable a feature that will absolutely eat your
> data if it is broken. I'd rather see the whole BBM-L2 being behind an
> option, and unconditionally check for b0rken CPUs.
> 
> Specially when it looks like there isn't a single CPU on the planet
> that implemented the feature correctly... :-/
> 
> Thanks,
> 
> 	M.
> 
> -- 
> Without deviation from the norm, progress is not possible.

Thanks for reviewing. Apologies for the delay in responding, and for
spam (replied instead of group-replied).

The workaround kconfig entries should be correct here. The MIDR
revisions will be fixed in the next respin.

I agree that having a BBML2 option and unconditionally guarding against
broken cpus is better, will make that change.