Re: [PATCH v2 09/14] KVM: arm64: Hide FEAT_NMI from guests

[Marc Zyngier <maz@kernel.org>]

On Mon, 05 Dec 2022 19:03:50 +0000,
Mark Brown <broonie@kernel.org> wrote:
> 
> [1  <text/plain; us-ascii (7bit)>]
> On Mon, Dec 05, 2022 at 06:06:24PM +0000, Marc Zyngier wrote:
> > Mark Brown <broonie@kernel.org> wrote:
> 
> > > FEAT_NMI is not yet useful to guests pending implementation of vGIC
> > > support. Mask out the feature from the ID register and prevent guests
> > > creating state in ALLINT.ALLINT by activating the trap on write provided
> > > in HCRX_EL2.TALLINT when they are running. There is no trap available
> > > for reads from ALLINT.
> 
> > > We do not need to check for FEAT_HCRX since it is mandatory since v8.7
> > > and FEAT_NMI is a v8.8 feature.
> 
> > And yet you check for it in hyp-stub.S after having checked for
> > FEAT_NMI. What gives?
> 
> Being aware that you have a strong preference for not having safety
> checks for mandatory features I didn't add any here but noted it so
> people could see why they were omitted.  The checks in hyp-stub.S were
> probably written before I'd checked the dependency situation out.
> 
> I can remove those checks if preferred but TBH given that the failure
> mode in hyp-stub.S is typically going to be to die with no output if
> something goes wrong it does feel like it's worth the extra couple of
> instructions to double check things just in case, especially with the
> virtual platforms being so easy to misconfigure.

I'm not hell bent on it, and if we can spot the issue early, that's
fine by me. But let's then disable the feature if the implementation
lacks some essential dependencies.

A simple check on ID_AA64MMFR1_EL1.HCX when detecting FEAT_NMI should
do the trick.

	M.

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