Re: [PATCH v8 25/38] KVM: arm64: Trap SME usage in guest

From: Marc Zyngier <maz@kernel.org>
To: Mark Brown <broonie@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	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>,
	James Morse <james.morse@arm.com>,
	Alexandru Elisei <alexandru.elisei@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-kselftest@vger.kernel.org, kvmarm@lists.cs.columbia.edu
Subject: Re: [PATCH v8 25/38] KVM: arm64: Trap SME usage in guest
Date: Tue, 25 Jan 2022 13:21:47 +0000	[thread overview]
Message-ID: <87v8y86jdg.wl-maz@kernel.org> (raw)
In-Reply-To: <Ye/sS0jEXnBuumeG@sirena.org.uk>

On Tue, 25 Jan 2022 12:25:47 +0000,
Mark Brown <broonie@kernel.org> wrote:
> 
> [1  <text/plain; us-ascii (7bit)>]
> On Tue, Jan 25, 2022 at 11:27:55AM +0000, Marc Zyngier wrote:
> > Mark Brown <broonie@kernel.org> wrote:
> 
> > > +	if (IS_ENABLED(CONFIG_ARM64_SME) && cpus_have_final_cap(ARM64_SME))
> 
> > Please drop the IS_ENABLED(). We purposely avoid conditional
> > compilation in KVM in order to avoid bitrot, and the amount of code
> > you save isn't significant. Having a static key is more than enough to
> > avoid runtime costs.
> 
> Sure, I wanted to be extra careful here as this is all in hot paths and
> going to get moved elsewhere when we have real guest support.
> 
> > > +	if (IS_ENABLED(CONFIG_ARM64_SME) && cpus_have_final_cap(ARM64_SME) &&
> > > +	    cpus_have_final_cap(ARM64_HAS_FGT)) {
> > > +		val = read_sysreg_s(SYS_HFGRTR_EL2);
> > > +		val &= ~(HFGxTR_EL2_nTPIDR_EL0_MASK |
> > > +			 HFGxTR_EL2_nSMPRI_EL1_MASK);
> > > +		write_sysreg_s(val, SYS_HFGRTR_EL2);
> > > +
> > > +		val = read_sysreg_s(SYS_HFGWTR_EL2);
> > > +		val &= ~(HFGxTR_EL2_nTPIDR_EL0_MASK |
> > > +			 HFGxTR_EL2_nSMPRI_EL1_MASK);
> > > +		write_sysreg_s(val, SYS_HFGWTR_EL2);
> > > +	}
> 
> > If the CPUs do not have FGT, what provides the equivalent trapping?
> 
> Nothing for nVHE mode.

That's what I feared.

> 
> > If FGT is mandatory when SME exists, then you should simplify the
> > condition.
> 
> OK, I'll remove the defensiveness here.  FGT is mandatory from v8.6 and
> SME is a v9 feature so people shouldn't build a SME implementation that
> lacks FGT.

Can you then please make it that SME doesn't get enabled at all if FGT
isn't present? It would also be good to have a clarification in the
architecture that it isn't allowed to build SME without FGT (specially
given that v9.0 is congruent to v8.5, and thus doesn't have FGT).

Thanks,

	M.

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel