Re: [PATCH v2 6/6] KVM: arm64: Handle DABT caused by LS64* instructions on unsupported memory

[Oliver Upton <oliver.upton@linux.dev>]

On Mon, Apr 07, 2025 at 11:33:01AM +0800, Yicong Yang wrote:
> On 2025/4/2 0:13, Oliver Upton wrote:
> > On Mon, Mar 31, 2025 at 05:43:20PM +0800, Yicong Yang wrote:
> >> @@ -1658,6 +1658,25 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
> > Keep in mind that data aborts with DFSC == 0x35 can happen for a lot
> > more than LS64 instructions, e.g. an atomic on a Device-* mapping.
> > 
> 
> got it. 0x35 should be caused by LS64* or IMPLEMENTATION DEFINED fault, but no
> further hint to distinguish between these two faults. hope it's also the right
> behaviour to inject a DABT back for the latter case.

There isn't exactly a 'right' behavior here. The abort could either be
due to a bug in the guest (doing an access on something knows it can't)
or the VMM creating / describing the IPA memory map incorrectly.

Since KVM can't really work out who's to blame in this situation we should
probably exit to userspace + provide a way to reinject the abort.

> >> @@ -1919,6 +1939,21 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu)
> >>  			goto out_unlock;
> >>  		}
> >>  
> >> +		/*
> >> +		 * If instructions of FEAT_{LS64, LS64_V} operated on
> >> +		 * unsupported memory regions, a DABT for unsupported
> >> +		 * Exclusive or atomic access is generated. It's
> >> +		 * implementation defined whether the exception will
> >> +		 * be taken to, a stage-1 DABT or the final enabled
> >> +		 * stage of translation (stage-2 in this case as we
> >> +		 * hit here). Inject a DABT to the guest to handle it
> >> +		 * if it's implemented as a stage-2 DABT.
> >> +		 */
> >> +		if (esr_fsc_is_excl_atomic_fault(esr)) {
> >> +			kvm_inject_dabt_excl_atomic(vcpu, kvm_vcpu_get_hfar(vcpu));
> >> +			return 1;
> >> +		}
> >> +
> > 
> > A precondition of taking such a data abort is having a valid mapping at
> > stage-2. If KVM can't resolve the HVA of the fault then there couldn't
> > have been a stage-2 mapping.
> > 
> 
> Here's handling the case for emulated mmio, I thought there's no valid stage-2 mapping
> for the emulated MMIO? so this check is put just before entering io_mem_abort(). should
> it be put into io_mem_abort() or we just don't handle the emulated case?

Right -- there's no valid stage-2 translation for _most_ MMIO. If KVM
cannot find an HVA for the fault (look at the condition that gets us
here) then we know there isn't a stage-2 mapping. How would we know what
to map?

In that case I would expect to take a Translation fault with instruction
syndrome that can can be used to construct an exit to the VMM. Marc had
some patches on list to do exactly that [*].

However, after reading this again there's a rather ugly catch. The KVM
ABI has it that writes to a RO memlot generate an MMIO exit, so it *is*
possible to get here w/ a stage-2 mapping. Unfortunately there's no
instruction syndrome with DFSC = 0x35 so no way to decode the access.

This is starting to sound similar an nISV MMIO abort...

[*]: https://lore.kernel.org/kvmarm/20240815125959.2097734-1-maz@kernel.org/

Thanks,
Oliver