On Mon, 24 Feb 2025 07:39:30 +0000, Aneesh Kumar K.V <aneesh.kumar@xxxxxxxxxx> wrote: > > Marc Zyngier <maz@xxxxxxxxxx> 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@xxxxxxxxxx> > > --- > > 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.
