Re: [PATCH v6 13/37] arm64/sme: Basic enumeration support

From: Mark Brown <broonie@kernel.org>
To: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>,
	Shuah Khan <skhan@linuxfoundation.org>,
	Shuah Khan <shuah@kernel.org>,
	Alan Hayward <alan.hayward@arm.com>,
	Luis Machado <luis.machado@arm.com>,
	Salil Akerkar <Salil.Akerkar@arm.com>,
	Basant Kumar Dwivedi <Basant.KumarDwivedi@arm.com>,
	Szabolcs Nagy <szabolcs.nagy@arm.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-kselftest@vger.kernel.org
Subject: Re: [PATCH v6 13/37] arm64/sme: Basic enumeration support
Date: Thu, 9 Dec 2021 19:28:16 +0000	[thread overview]
Message-ID: <YbJY0P1KZjSFfwtY@sirena.org.uk> (raw)
In-Reply-To: <YbJN1ujpDP1RG1Ll@arm.com>

[-- Attachment #1: Type: text/plain, Size: 1832 bytes --]

On Thu, Dec 09, 2021 at 06:41:26PM +0000, Catalin Marinas wrote:
> On Mon, Nov 15, 2021 at 03:28:11PM +0000, Mark Brown wrote:

> > +#define HWCAP2_SME		(1 << 20)
> > +#define HWCAP2_SME_I16I64	(1 << 21)
> > +#define HWCAP2_SME_F64F64	(1 << 22)
> > +#define HWCAP2_SME_I8I32	(1 << 23)
> > +#define HWCAP2_SME_F16F32	(1 << 24)
> > +#define HWCAP2_SME_B16F32	(1 << 25)
> > +#define HWCAP2_SME_F32F32	(1 << 26)
> > +#define HWCAP2_SME_FA64		(1 << 27)

> At this pace we'll need HWCAP3 pretty soon (since we only allocated
> 32-bit in each). I wonder whether we could instead not bother at all and
> just provide user-space emulation for ID_AA64SMFR0_EL1.

I think so if people are willing to go along with just having userspace
check the ID register (IIRC access to it already does the right thing
but I need to confirm).  We'll also need to think about how we handle
any new SVE features, that's got a similar thing going on and is most of
the existing usage of HWCAP2.

> > +	{
> > +		.desc = "FA64",
> > +		.type = ARM64_CPUCAP_SYSTEM_FEATURE,
> > +		.capability = ARM64_SME_FA64,
> > +		.sys_reg = SYS_ID_AA64SMFR0_EL1,
> > +		.sign = FTR_UNSIGNED,
> > +		.field_pos = ID_AA64SMFR0_FA64_SHIFT,
> > +		.min_field_value = ID_AA64SMFR0_FA64,
> > +		.matches = has_feature_flag,
> > +		.cpu_enable = fa64_kernel_enable,
> > +	},

> I'll comment here rather than the patch introducing has_feature_flag():
> an alternative would be to add a .field_width option and in
> feature_matches() use cpuid_feature_extract_field_width() directly. All
> the arm64_ftr_bits entries already have a width, so just generalise it
> for arm64_cpu_capabilities.

Sure, if people are happy with that - it's a more invasive change since
we don't currently set the widths, I wasn't clear if that was a case of
not needing it right now or a design decision.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]