From 1ccf2fe35c30f79102ad129c5aa71059daaaed7f Mon Sep 17 00:00:00 2001 From: Alexander Graf Date: Thu, 30 Jul 2020 11:44:41 +0200 Subject: [PATCH 1/5] KVM: arm: Add trace name for ARM_NISV Commit c726200dd106d ("KVM: arm/arm64: Allow reporting non-ISV data aborts to userspace") introduced a mechanism to deflect MMIO traffic the kernel can not handle to user space. For that, it introduced a new exit reason. However, it did not update the trace point array that gives human readable names to these exit reasons inside the trace log. Let's fix that up after the fact, so that trace logs are pretty even when we get user space MMIO traps on ARM. Fixes: c726200dd106d ("KVM: arm/arm64: Allow reporting non-ISV data aborts to userspace") Signed-off-by: Alexander Graf Signed-off-by: Marc Zyngier Link: https://lore.kernel.org/r/20200730094441.18231-1-graf@amazon.com --- include/trace/events/kvm.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/trace/events/kvm.h b/include/trace/events/kvm.h index 2c735a3e6613..9417a34aad08 100644 --- a/include/trace/events/kvm.h +++ b/include/trace/events/kvm.h @@ -17,7 +17,7 @@ ERSN(NMI), ERSN(INTERNAL_ERROR), ERSN(OSI), ERSN(PAPR_HCALL), \ ERSN(S390_UCONTROL), ERSN(WATCHDOG), ERSN(S390_TSCH), ERSN(EPR),\ ERSN(SYSTEM_EVENT), ERSN(S390_STSI), ERSN(IOAPIC_EOI), \ - ERSN(HYPERV) + ERSN(HYPERV), ERSN(ARM_NISV) TRACE_EVENT(kvm_userspace_exit, TP_PROTO(__u32 reason, int errno), From c9a636f29b5f236441ff059cef0b2fe734c05afd Mon Sep 17 00:00:00 2001 From: Will Deacon Date: Wed, 29 Jul 2020 11:28:18 +0100 Subject: [PATCH 2/5] KVM: arm64: Rename kvm_vcpu_dabt_isextabt() kvm_vcpu_dabt_isextabt() is not specific to data aborts and, unlike kvm_vcpu_dabt_issext(), has nothing to do with sign extension. Rename it to 'kvm_vcpu_abt_issea()'. Signed-off-by: Will Deacon Signed-off-by: Marc Zyngier Cc: Marc Zyngier Cc: Quentin Perret Link: https://lore.kernel.org/r/20200729102821.23392-2-will@kernel.org --- arch/arm64/include/asm/kvm_emulate.h | 2 +- arch/arm64/kvm/hyp/switch.c | 2 +- arch/arm64/kvm/mmu.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h index c9ba0df47f7d..e0536946abed 100644 --- a/arch/arm64/include/asm/kvm_emulate.h +++ b/arch/arm64/include/asm/kvm_emulate.h @@ -366,7 +366,7 @@ static __always_inline u8 kvm_vcpu_trap_get_fault_type(const struct kvm_vcpu *vc return kvm_vcpu_get_esr(vcpu) & ESR_ELx_FSC_TYPE; } -static __always_inline bool kvm_vcpu_dabt_isextabt(const struct kvm_vcpu *vcpu) +static __always_inline bool kvm_vcpu_abt_issea(const struct kvm_vcpu *vcpu) { switch (kvm_vcpu_trap_get_fault(vcpu)) { case FSC_SEA: diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c index 5164074c1ae1..36a1f0a9ce2e 100644 --- a/arch/arm64/kvm/hyp/switch.c +++ b/arch/arm64/kvm/hyp/switch.c @@ -594,7 +594,7 @@ static bool __hyp_text fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code) valid = kvm_vcpu_trap_get_class(vcpu) == ESR_ELx_EC_DABT_LOW && kvm_vcpu_trap_get_fault_type(vcpu) == FSC_FAULT && kvm_vcpu_dabt_isvalid(vcpu) && - !kvm_vcpu_dabt_isextabt(vcpu) && + !kvm_vcpu_abt_issea(vcpu) && !kvm_vcpu_dabt_iss1tw(vcpu); if (valid) { diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c index 36506112480e..8004f1f68946 100644 --- a/arch/arm64/kvm/mmu.c +++ b/arch/arm64/kvm/mmu.c @@ -2074,7 +2074,7 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu, struct kvm_run *run) is_iabt = kvm_vcpu_trap_is_iabt(vcpu); /* Synchronous External Abort? */ - if (kvm_vcpu_dabt_isextabt(vcpu)) { + if (kvm_vcpu_abt_issea(vcpu)) { /* * For RAS the host kernel may handle this abort. * There is no need to pass the error into the guest. From 84b951a803a5464b0bff2fb1366e96f07f75b066 Mon Sep 17 00:00:00 2001 From: Will Deacon Date: Wed, 29 Jul 2020 11:28:19 +0100 Subject: [PATCH 3/5] KVM: arm64: Handle data and instruction external aborts the same way If the guest generates a synchronous external abort which is not handled by the host, we inject it back into the guest as a virtual SError, but only if the original fault was reported on the data side. Instruction faults are reported as "Unsupported FSC", causing the vCPU run loop to bail with -EFAULT. Although synchronous external aborts from a guest are pretty unusual, treat them the same regardless of whether they are taken as data or instruction aborts by EL2. Signed-off-by: Will Deacon Signed-off-by: Marc Zyngier Cc: Marc Zyngier Cc: Quentin Perret Link: https://lore.kernel.org/r/20200729102821.23392-3-will@kernel.org --- arch/arm64/kvm/mmu.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c index 8004f1f68946..14c6a9df5c9f 100644 --- a/arch/arm64/kvm/mmu.c +++ b/arch/arm64/kvm/mmu.c @@ -2079,13 +2079,10 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu, struct kvm_run *run) * For RAS the host kernel may handle this abort. * There is no need to pass the error into the guest. */ - if (!kvm_handle_guest_sea(fault_ipa, kvm_vcpu_get_esr(vcpu))) - return 1; - - if (unlikely(!is_iabt)) { + if (kvm_handle_guest_sea(fault_ipa, kvm_vcpu_get_esr(vcpu))) kvm_inject_vabt(vcpu); - return 1; - } + + return 1; } trace_kvm_guest_fault(*vcpu_pc(vcpu), kvm_vcpu_get_esr(vcpu), From 54dc0d2404dd7aa0dd4e4f388a65622b68c6eaff Mon Sep 17 00:00:00 2001 From: Will Deacon Date: Wed, 29 Jul 2020 11:28:20 +0100 Subject: [PATCH 4/5] KVM: arm64: Don't skip cache maintenance for read-only memslots If a guest performs cache maintenance on a read-only memslot, we should inform userspace rather than skip the instruction altogether. Signed-off-by: Will Deacon Signed-off-by: Marc Zyngier Cc: Marc Zyngier Cc: Quentin Perret Link: https://lore.kernel.org/r/20200729102821.23392-4-will@kernel.org --- arch/arm64/kvm/mmu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c index 14c6a9df5c9f..85b0ec9dd9ef 100644 --- a/arch/arm64/kvm/mmu.c +++ b/arch/arm64/kvm/mmu.c @@ -2121,7 +2121,7 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu, struct kvm_run *run) * So let's assume that the guest is just being * cautious, and skip the instruction. */ - if (kvm_vcpu_dabt_is_cm(vcpu)) { + if (kvm_is_error_hva(hva) && kvm_vcpu_dabt_is_cm(vcpu)) { kvm_skip_instr(vcpu, kvm_vcpu_trap_il_is32bit(vcpu)); ret = 1; goto out_unlock; From 022c8328dc8021248047b373b9f67790641b8f2d Mon Sep 17 00:00:00 2001 From: Will Deacon Date: Wed, 29 Jul 2020 11:28:21 +0100 Subject: [PATCH 5/5] KVM: arm64: Move S1PTW S2 fault logic out of io_mem_abort() To allow for re-injection of stage-2 faults on stage-1 page-table walks due to either a missing or read-only memslot, move the triage logic out of io_mem_abort() and into kvm_handle_guest_abort(), where these aborts can be handled before anything else. Signed-off-by: Will Deacon Signed-off-by: Marc Zyngier Cc: Marc Zyngier Cc: Quentin Perret Link: https://lore.kernel.org/r/20200729102821.23392-5-will@kernel.org --- arch/arm64/kvm/mmio.c | 6 ------ arch/arm64/kvm/mmu.c | 13 ++++++++++++- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/arch/arm64/kvm/mmio.c b/arch/arm64/kvm/mmio.c index 4e0366759726..58de2ae4f6bb 100644 --- a/arch/arm64/kvm/mmio.c +++ b/arch/arm64/kvm/mmio.c @@ -145,12 +145,6 @@ int io_mem_abort(struct kvm_vcpu *vcpu, struct kvm_run *run, return -ENOSYS; } - /* Page table accesses IO mem: tell guest to fix its TTBR */ - if (kvm_vcpu_dabt_iss1tw(vcpu)) { - kvm_inject_dabt(vcpu, kvm_vcpu_get_hfar(vcpu)); - return 1; - } - /* * Prepare MMIO operation. First decode the syndrome data we get * from the CPU. Then try if some in-kernel emulation feels diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c index 85b0ec9dd9ef..dc8464669efd 100644 --- a/arch/arm64/kvm/mmu.c +++ b/arch/arm64/kvm/mmu.c @@ -2105,12 +2105,23 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu, struct kvm_run *run) hva = gfn_to_hva_memslot_prot(memslot, gfn, &writable); write_fault = kvm_is_write_fault(vcpu); if (kvm_is_error_hva(hva) || (write_fault && !writable)) { + /* + * The guest has put either its instructions or its page-tables + * somewhere it shouldn't have. Userspace won't be able to do + * anything about this (there's no syndrome for a start), so + * re-inject the abort back into the guest. + */ if (is_iabt) { - /* Prefetch Abort on I/O address */ ret = -ENOEXEC; goto out; } + if (kvm_vcpu_dabt_iss1tw(vcpu)) { + kvm_inject_dabt(vcpu, kvm_vcpu_get_hfar(vcpu)); + ret = 1; + goto out_unlock; + } + /* * Check for a cache maintenance operation. Since we * ended-up here, we know it is outside of any memory