Re: [PATCH v1] arm64: Add TLB Conflict Abort Exception handler to KVM

[Mikołaj Lenczewski]

On Fri, Jan 10, 2025 at 10:49:56AM -0800, Oliver Upton wrote:
> On Fri, Jan 10, 2025 at 05:24:07PM +0000, Mikołaj Lenczewski wrote:
> > Currently, KVM does not handle the case of a stage 2 TLB conflict abort
> > exception. This can legitimately occurs when the guest is eliding full
> > BBM semantics as permitted by BBM level 2. In this case it is possible
> > for a confclit abort to be delivered to EL2. We handle that by
> > invalidating the full TLB.
> 
> typo: conflict
> 
> Also, a bit of a nitpick, but mentioning that TLB conflict abort routing
> is implementation defined when S2 is enabled is valuable information.
> 
> > @@ -1756,6 +1756,19 @@ 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)) {
> > +
> > +		/* Architecturely, at this stage 2 tlb conflict abort, we must
> > +		 * either perform a `tlbi vmalls12e1`, or a `tlbi alle1`. Due
> > +		 * to nesting of VMs, we would have to iterate all flattened
> > +		 * VMIDs to clean out a single guest, so we perform a `tlbi alle1`
> > +		 * instead to save time.
> > +		 */
> 
> I'm not sure I follow this.
> 
> At this point we've taken a TLB conflict abort out of a specific
> hardware MMU context, and it's unclear to me why a conflict abort in one
> stage-2 MMU has any bearing on the other stage-2 MMUs that could be
> associated with this guest.
> 
> Even in NV, KVM is always responsible for the maintenance of hardware
> stage-2 MMUs. So stage-2 TLBI elision in the guest hypervisor should
> not lead to a stage-2 TLB conflict abort.
> 
> TLBI ALLE1 is a larger hammer than what's actually necessary here. Could
> you perhaps introduce a new invalidation routine,
> __kvm_tlb_flush_vmid_nsh(), that does a TLBI VMALLS12E1 behind the
> scenes?
> 
> If an NV guest is playing games at stage-1 across VMIDs then it gets to
> suffer the consequences (additional TLB conflict aborts).
> 
> -- 
> Thanks,
> Oliver

Apologies for the very late reply. Thank you for taking the time to
review my patch. Will fix the type, and work on fixing the logic there
to use a TLBI VMALLS12E1.

-- 
Kind regards,
Mikołaj Lenczewski