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: <20250313182026.GC40525@mazurka.cambridge.arm.com>
Date: Thu, 13 Mar 2025 18:20:26 +0000
From: Mikołaj Lenczewski <miko.lenczewski@....com>
To: Marc Zyngier <maz@...nel.org>
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, 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.

-- 
Kind regards,
Mikołaj Lenczewski