On Fri, 24 Jul 2020 15:35:05 +0100, Will Deacon <[email protected]> wrote: > > Stage-2 faults on stage-1 page-table walks can occur on both the I-side > and the D-side. It is IMPLEMENTATATION DEFINED whether they are reported > as reads or writes and, in the case that they are generated by an AT > instruction, they are reported with the CM bit set. > > All of this deeply confuses the logic in kvm_handle_guest_abort(); > userspace may or may not see the fault, depending on whether it occurs > on the data or the instruction side, and an AT instruction may be skipped > if the translation tables are held in a read-only memslot.
Yuk, that's indeed ugly. Well spotted. I guess the saving grace is that a S2 trap caused by an ATS1 instruction will be reported as S1PTW+CM, while the fault caused by a CMO is reported as *either* S1PTW *or* CM, but never both. > > Move the handling of stage-2 faults on stage-1 page-table walks earlier > so that they consistently result in either a data or an instruction abort > being re-injected back to the guest. The instruction abort seems to be happening as the side effect of executing outside of a memslot, not really because of a S1PTW. I wonder whether these S1PTW faults should be classified as external aborts instead (because putting your page tables outside of a memslot seems a bit bonkers). > > Cc: Marc Zyngier <[email protected]> > Cc: Quentin Perret <[email protected]> > Signed-off-by: Will Deacon <[email protected]> > --- > arch/arm64/kvm/mmu.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c > index adb933ecd177..9e72e7f4a2c2 100644 > --- a/arch/arm64/kvm/mmu.c > +++ b/arch/arm64/kvm/mmu.c > @@ -2124,6 +2124,11 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu, > struct kvm_run *run) > goto out; > } > > + if (kvm_vcpu_dabt_iss1tw(vcpu)) { > + ret = -ENXIO; > + goto out; > + } > + > /* > * Check for a cache maintenance operation. Since we > * ended-up here, we know it is outside of any memory > @@ -2157,11 +2162,6 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu, > struct kvm_run *run) > goto out_unlock; > } > > - if (kvm_vcpu_dabt_iss1tw(vcpu)) { > - ret = -ENXIO; > - goto out; > - } > - > ret = io_mem_abort(vcpu, run, fault_ipa); > goto out_unlock; > } > -- > 2.28.0.rc0.142.g3c755180ce-goog > > Thanks, M. -- Without deviation from the norm, progress is not possible. _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
