Re: [PATCH 03/14] KVM: arm64: Mark HCR.EL2.E2H RES0 when ID_AA64MMFR1_EL1.VH is zero

[Marc Zyngier <maz@kernel.org>]

On Mon, 24 Feb 2025 07:39:30 +0000,
Aneesh Kumar K.V <aneesh.kumar@kernel.org> wrote:
> 
> Marc Zyngier <maz@kernel.org> writes:
> 
> > Enforce HCR_EL2.E2H being RES0 when VHE is disabled, so that we can
> > actually rely on that bit never being flipped behind our back.
> >
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > ---
> >  arch/arm64/kvm/nested.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> > index 0c9387d2f5070..ed3add7d32f66 100644
> > --- a/arch/arm64/kvm/nested.c
> > +++ b/arch/arm64/kvm/nested.c
> > @@ -1034,6 +1034,8 @@ int kvm_init_nv_sysregs(struct kvm_vcpu *vcpu)
> >  		res0 |= (HCR_TEA | HCR_TERR);
> >  	if (!kvm_has_feat(kvm, ID_AA64MMFR1_EL1, LO, IMP))
> >  		res0 |= HCR_TLOR;
> > +	if (!kvm_has_feat(kvm, ID_AA64MMFR1_EL1, VH, IMP))
> > +		res0 |= HCR_E2H;
> >  	if (!kvm_has_feat(kvm, ID_AA64MMFR4_EL1, E2H0, IMP))
> >  		res1 |= HCR_E2H;
> >
> 
> Does it make sense to check for E2H0 if MMFR1_EL1.VH == 0 ?
> Should the above check be
> 	if (!kvm_has_feat(kvm, ID_AA64MMFR1_EL1, VH, IMP))
> 		res0 |= HCR_E2H;
> 	else if (!kvm_has_feat(kvm, ID_AA64MMFR4_EL1, E2H0, IMP))
>  		res1 |= HCR_E2H;

What difference does it make? This bit can only have a reserved value,
and can never be actively modified. If you *really* wanted to optimise
this for reasons that I really cannot fathom, you could have this:

 	if (!kvm_has_feat(kvm, ID_AA64MMFR1_EL1, VH, IMP))
 		res0 |= HCR_E2H;
 	else
  		res1 |= HCR_E2H;

because that's what we really implement.

Does it matter? I don't think so. If anything, I'd rather we keep the
code as is and have a run-time warning if a bit is simultaneously RES0
and RES1, because that'd be indicative of a much bigger problem.

Thanks,

	M.

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