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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250116104107.GA17031@mazurka.cambridge.arm.com>
Date: Thu, 16 Jan 2025 10:41:07 +0000
From: Mikołaj Lenczewski <miko.lenczewski@....com>
To: Oliver Upton <oliver.upton@...ux.dev>
Cc: kvmarm@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org, ryan.roberts@....com,
	catalin.marinas@....com, mark.rutland@....com, james.morse@....com,
	will@...nel.org, maz@...nel.org, joey.gouly@....com,
	suzuki.poulose@....com, yuzenghui@...wei.com
Subject: Re: [PATCH v1] arm64: Add TLB Conflict Abort Exception handler to KVM

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