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@xxxxxxx> 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@xxxxxxx>
> > ---
> > 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.
