On Fri, 24 Jul 2020 15:35:00 +0100, Will Deacon <will@xxxxxxxxxx> wrote: > > If a 32-bit guest accesses MMIO using a 16-bit Thumb-2 instruction that > is reported to the hypervisor without a valid syndrom (for example, > because of the addressing mode), then we may hand off the fault to > userspace. When resuming the guest, we unconditionally advance the PC > by 4 bytes, since ESR_EL2.IL is always 1 for data aborts generated without > a valid syndrome. This is a bit rubbish, but it's also difficult to see > how we can fix it without potentially introducing regressions in userspace > MMIO fault handling. Not quite, see below. > > Update the comment when skipping a guest MMIO access instruction so that > this corner case is at least written down. > > Cc: Marc Zyngier <maz@xxxxxxxxxx> > Cc: Quentin Perret <qperret@xxxxxxxxxx> > Signed-off-by: Will Deacon <will@xxxxxxxxxx> > --- > arch/arm64/kvm/mmio.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/arch/arm64/kvm/mmio.c b/arch/arm64/kvm/mmio.c > index 4e0366759726..b54ea5aa6c06 100644 > --- a/arch/arm64/kvm/mmio.c > +++ b/arch/arm64/kvm/mmio.c > @@ -113,6 +113,13 @@ int kvm_handle_mmio_return(struct kvm_vcpu *vcpu, struct kvm_run *run) > /* > * The MMIO instruction is emulated and should not be re-executed > * in the guest. > + * > + * Note: If user space handled the emulation because the abort > + * symdrome information was not valid (ISV set in the ESR), then nits: syndrome, ISV *clear*. > + * this will assume that the faulting instruction was 32-bit. > + * If the faulting instruction was a 16-bit Thumb instruction, > + * then userspace would need to rewind the PC by 2 bytes prior to > + * resuming the vCPU (yuck!). > */ > kvm_skip_instr(vcpu, kvm_vcpu_trap_il_is32bit(vcpu)); > That's not how I read it. On ESR_EL2.ISV being clear, and in the absence of KVM_CAP_ARM_NISV_TO_USER being set, we return a -ENOSYS from io_mem_abort(), exiting back to userspace *without* advertising a MMIO access. The VMM is free to do whatever it can to handle it (i.e. not much), but crucially we don't go via kvm_handle_mmio_return() on resuming the vcpu (unless the VMM sets run->exit_reason to KVM_EXIT_MMIO, but that's clearly its own decision). Instead, the expectation is that userspace willing to handle an exit resulting in ESR_EL2.ISV being clear would instead request a KVM_EXIT_ARM_NISV exit type (by enabling KVM_CAP_ARM_NISV_TO_USER), getting extra information in the process such as as the fault IPA). KVM_EXIT_ARM_NISV clearly states in the documentation: "Note that KVM does not skip the faulting instruction as it does for KVM_EXIT_MMIO, but userspace has to emulate any change to the processing state if it decides to decode and emulate the instruction." Thanks, M. -- Without deviation from the norm, progress is not possible. _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
