2019-04-24 04:43:53

by Dongjiu Geng

Subject: [PATCH] KVM: arm/arm64: inject an async abort to host if SEA handling failed

If host failed to handle the SEA, KVM should inject an async abort
to guest for both SEA data and instruction abort, but it currently
only handles the data abort, so correct it.

Cc: James Morse <[email protected]>
Cc: Xiang Zheng <[email protected]>
Signed-off-by: Dongjiu Geng <[email protected]>
---
virt/kvm/arm/mmu.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
index 27c9583..5882516 100644
--- a/virt/kvm/arm/mmu.c
+++ b/virt/kvm/arm/mmu.c
@@ -1924,14 +1924,13 @@ 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 host failed to handle it, inject an async abort
+ * to guest.
*/
- if (!kvm_handle_guest_sea(fault_ipa, kvm_vcpu_get_hsr(vcpu)))
- return 1;
-
- if (unlikely(!is_iabt)) {
+ if (kvm_handle_guest_sea(fault_ipa, kvm_vcpu_get_hsr(vcpu)))
kvm_inject_vabt(vcpu);
- return 1;
- }
+
+ return 1;
}

trace_kvm_guest_fault(*vcpu_pc(vcpu), kvm_vcpu_get_hsr(vcpu),
--
2.7.4

2019-04-24 10:12:35

by Marc Zyngier

Subject: Re: [PATCH] KVM: arm/arm64: inject an async abort to host if SEA handling failed

On 24/04/2019 05:41, Dongjiu Geng wrote:
> If host failed to handle the SEA, KVM should inject an async abort
> to guest for both SEA data and instruction abort, but it currently
> only handles the data abort, so correct it.
>
> Cc: James Morse <[email protected]>
> Cc: Xiang Zheng <[email protected]>
> Signed-off-by: Dongjiu Geng <[email protected]>
> ---
> virt/kvm/arm/mmu.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
> index 27c9583..5882516 100644
> --- a/virt/kvm/arm/mmu.c
> +++ b/virt/kvm/arm/mmu.c
> @@ -1924,14 +1924,13 @@ 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 host failed to handle it, inject an async abort
> + * to guest.
> */
> - if (!kvm_handle_guest_sea(fault_ipa, kvm_vcpu_get_hsr(vcpu)))
> - return 1;
> -
> - if (unlikely(!is_iabt)) {
> + if (kvm_handle_guest_sea(fault_ipa, kvm_vcpu_get_hsr(vcpu)))

We've so far excluded instruction aborts from the delivery of a virtual
SError. You now decide to inject an SError in all cases, but your commit
message doesn't explain *why* this is a sensible thing to do.

I'm not saying that this patch is wrong (the IABT handling predates RAS
by several years and was designed to deal with with badly behaved guest
rather than flaky HW), but I'd like to understand why you think it is right.

Thanks,

M.
--
Jazz is not dead. It just smells funny...