Apologies again for spam (replied instead of group-replied).
On Wed, Dec 11, 2024 at 05:40:36PM +0000, Marc Zyngier wrote:
> On Wed, 11 Dec 2024 16:01:37 +0000,
> Mikołaj Lenczewski <miko.lenczewski@xxxxxxx> wrote:
> >
> > Currently, KVM does not handle the case of a stage 2 TLB conflict abort
> > exception. The Arm ARM specifies that the worst-case handling of such an
> > exception requires a `tlbi vmalls12e1`.
>
> Not quite. It says (I_JCCRT):
>
> <quote>
> * For the EL1&0 translation regime, when stage 2 translations are in
> use, either VMALLS12E1 or ALLE1.
> </quote>
>
> > Perform such an invalidation when this exception is encountered.
>
> What you fail to describe is *why* this is needed. You know it, I know
> it, but not everybody does. A reference to the ARM ARM would
> definitely be helpful.
>
You are correct. Will update the commit message.
> > diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> > index c9d46ad57e52..c8c6f5a97a1b 100644
> > --- a/arch/arm64/kvm/mmu.c
> > +++ b/arch/arm64/kvm/mmu.c
> > @@ -1756,6 +1756,12 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu)
> > ipa = fault_ipa = kvm_vcpu_get_fault_ipa(vcpu);
> > is_iabt = kvm_vcpu_trap_is_iabt(vcpu);
> >
> > + if (esr_fsc_is_tlb_conflict_abort(esr)) {
> > + // does a `tlbi vmalls12e1is`
>
> nit: this isn't a very useful comment.
>
Will remove it.
> > + __kvm_tlb_flush_vmid(&vcpu->kvm->arch.mmu);
> > + return 1;
> > + }
>
> That's not enough, unfortunately. A nested VM has *many* VMIDs (the
> flattening of all translation contexts that the guest uses).
>
> So you can either iterate over all the valid VMIDs owned by this
> guest, or more simply issue a TLBI ALLE1, which will do the trick in a
> much more efficient way.
>
> The other thing is that you are using an IS invalidation, which is
> farther reaching than necessary. Why would you invalidate the TLBs for
> CPUs that are only innocent bystanders? A non-shareable invalidation
> seems preferable to me.
>
You are completely correct here. I had forgotten about nested VMs, and
agree that issuing a `tlbi alle1` is simpler and more efficient. I agree
also on not using an IS invalidation.
