linux-kernel - Re: [PATCH v3 1/3] arm64: Add BBM Level 2 cpu feature

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: <86frjgojuz.wl-maz@kernel.org>
Date: Thu, 13 Mar 2025 18:39:48 +0000
From: Marc Zyngier <maz@...nel.org>
To: Mikołaj Lenczewski <miko.lenczewski@....com>
Cc: ryan.roberts@....com,
	suzuki.poulose@....com,
	yang@...amperecomputing.com,
	corbet@....net,
	catalin.marinas@....com,
	will@...nel.org,
	jean-philippe@...aro.org,
	robin.murphy@....com,
	joro@...tes.org,
	akpm@...ux-foundation.org,
	mark.rutland@....com,
	joey.gouly@....com,
	james.morse@....com,
	broonie@...nel.org,
	anshuman.khandual@....com,
	oliver.upton@...ux.dev,
	ioworker0@...il.com,
	baohua@...nel.org,
	david@...hat.com,
	jgg@...pe.ca,
	shameerali.kolothum.thodi@...wei.com,
	nicolinc@...dia.com,
	mshavit@...gle.com,
	jsnitsel@...hat.com,
	smostafa@...gle.com,
	linux-doc@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org,
	iommu@...ts.linux.dev
Subject: Re: [PATCH v3 1/3] arm64: Add BBM Level 2 cpu feature

On Thu, 13 Mar 2025 18:20:26 +0000,
Mikołaj Lenczewski <miko.lenczewski@....com> wrote:
> 
> On Thu, Mar 13, 2025 at 05:34:46PM +0000, Marc Zyngier wrote:
> > On Thu, 13 Mar 2025 10:41:10 +0000,
> > Mikołaj Lenczewski <miko.lenczewski@....com> wrote:
> > > 
> > > diff --git a/arch/arm64/kernel/pi/idreg-override.c b/arch/arm64/kernel/pi/idreg-override.c
> > > index c6b185b885f7..9728faa10390 100644
> > > --- a/arch/arm64/kernel/pi/idreg-override.c
> > > +++ b/arch/arm64/kernel/pi/idreg-override.c
> > > @@ -209,6 +209,7 @@ static const struct ftr_set_desc sw_features __prel64_initconst = {
> > >  		FIELD("nokaslr", ARM64_SW_FEATURE_OVERRIDE_NOKASLR, NULL),
> > >  		FIELD("hvhe", ARM64_SW_FEATURE_OVERRIDE_HVHE, hvhe_filter),
> > >  		FIELD("rodataoff", ARM64_SW_FEATURE_OVERRIDE_RODATA_OFF, NULL),
> > > +		FIELD("nobbml2", ARM64_SW_FEATURE_OVERRIDE_NOBBML2, NULL),
> > >  		{}
> > >  	},
> > >  };
> > > @@ -246,6 +247,7 @@ static const struct {
> > >  	{ "rodata=off",			"arm64_sw.rodataoff=1" },
> > >  	{ "arm64.nolva",		"id_aa64mmfr2.varange=0" },
> > >  	{ "arm64.no32bit_el0",		"id_aa64pfr0.el0=1" },
> > > +	{ "arm64.nobbml2",		"arm64_sw.nobbml2=1" },
> > 
> > Why is that a SW feature? This looks very much like a HW feature to
> > me, and you should instead mask out ID_AA64MMFR2_EL1.BBM, and be done
> > with it. Something like:
> > 
> > diff --git a/arch/arm64/kernel/pi/idreg-override.c b/arch/arm64/kernel/pi/idreg-override.c
> > index c6b185b885f70..803a0c99f7b46 100644
> > --- a/arch/arm64/kernel/pi/idreg-override.c
> > +++ b/arch/arm64/kernel/pi/idreg-override.c
> > @@ -102,6 +102,7 @@ static const struct ftr_set_desc mmfr2 __prel64_initconst = {
> >  	.override	= &id_aa64mmfr2_override,
> >  	.fields		= {
> >  		FIELD("varange", ID_AA64MMFR2_EL1_VARange_SHIFT, mmfr2_varange_filter),
> > +		FIELD("bbm", ID_AA64MMFR2_EL1_BBM_SHIFT, NULL),
> >  		{}
> >  	},
> >  };
> > @@ -246,6 +247,7 @@ static const struct {
> >  	{ "rodata=off",			"arm64_sw.rodataoff=1" },
> >  	{ "arm64.nolva",		"id_aa64mmfr2.varange=0" },
> >  	{ "arm64.no32bit_el0",		"id_aa64pfr0.el0=1" },
> > +	{ "arm64.nobbml2",		"id_aa64mmfr2.bbm=0" },
> >  };
> >  
> >  static int __init parse_hexdigit(const char *p, u64 *v)
> > 
> > 
> > Thanks,
> > 
> > 	M.
> > 
> > -- 
> > Without deviation from the norm, progress is not possible.
> 
> Thanks for the review.
> 
> I think part of this confusion is due to me not including a changelog
> (definitely something for the next respin!), but the discussion this
> change is based on is found here:
> 
> https://lore.kernel.org/all/b46dc626-edc9-4d20-99d2-6cd08a01346c@os.amperecomputing.com/
> 
> Essentially, this is a SW feature because we do not check the
> id_aa64mmfr2.bbm register as part of the has_bbml2_noabort() cpucap
> matches filter. This is because certain hardware implementations
> do not actually declare bbml2 via the hardware feature register, despite
> implementing our bbml2_noabort feature, and certain hypervisor setups
> might result in issues so we want to have an override to allow
> potentially disabling the feature for generic kernels.

I replied to Ryan on the same subject: not advertising a feature that
is actually supported is very much an erratum, and we should not
conflate feature control of an architecture feature (which is what the
ID override horror is doing) with implementation-specific workarounds.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.