| Message ID | 20190826101643.133750-1-liran.alon@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86: Return to userspace with internal error on unexpected exit reason | expand |
Liran Alon <liran.alon@oracle.com> writes: > Receiving an unexpected exit reason from hardware should be considered > as a severe bug in KVM. Therefore, instead of just injecting #UD to > guest and ignore it, exit to userspace on internal error so that > it could handle it properly (probably by terminating guest). While "this should never happen" on real hardware, it is a possible event for the case when KVM is running as a nested (L1) hypervisor. Misbehaving L0 can try to inject some weird (corrupted) exit reason. > > In addition, prefer to use vcpu_unimpl() instead of WARN_ONCE() > as handling unexpected exit reason should be a rare unexpected > event (that was expected to never happen) and we prefer to print > a message on it every time it occurs to guest. > > Furthermore, dump VMCS/VMCB to dmesg to assist diagnosing such cases. > > Reviewed-by: Mihai Carabas <mihai.carabas@oracle.com> > Reviewed-by: Nikita Leshenko <nikita.leshchenko@oracle.com> > Reviewed-by: Joao Martins <joao.m.martins@oracle.com> > Signed-off-by: Liran Alon <liran.alon@oracle.com> Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com> > --- > arch/x86/kvm/svm.c | 11 ++++++++--- > arch/x86/kvm/vmx/vmx.c | 9 +++++++-- > include/uapi/linux/kvm.h | 2 ++ > 3 files changed, 17 insertions(+), 5 deletions(-) > > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > index d685491fce4d..6462c386015d 100644 > --- a/arch/x86/kvm/svm.c > +++ b/arch/x86/kvm/svm.c > @@ -5026,9 +5026,14 @@ static int handle_exit(struct kvm_vcpu *vcpu) > > if (exit_code >= ARRAY_SIZE(svm_exit_handlers) > || !svm_exit_handlers[exit_code]) { > - WARN_ONCE(1, "svm: unexpected exit reason 0x%x\n", exit_code); > - kvm_queue_exception(vcpu, UD_VECTOR); > - return 1; > + vcpu_unimpl(vcpu, "svm: unexpected exit reason 0x%x\n", exit_code); > + dump_vmcb(vcpu); > + vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR; > + vcpu->run->internal.suberror = > + KVM_INTERNAL_ERROR_UNEXPECTED_EXIT_REASON; > + vcpu->run->internal.ndata = 1; > + vcpu->run->internal.data[0] = exit_code; > + return 0; > } > > return svm_exit_handlers[exit_code](svm); > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > index 42ed3faa6af8..b5b5b2e5dac5 100644 > --- a/arch/x86/kvm/vmx/vmx.c > +++ b/arch/x86/kvm/vmx/vmx.c > @@ -5887,8 +5887,13 @@ static int vmx_handle_exit(struct kvm_vcpu *vcpu) > else { > vcpu_unimpl(vcpu, "vmx: unexpected exit reason 0x%x\n", > exit_reason); > - kvm_queue_exception(vcpu, UD_VECTOR); > - return 1; > + dump_vmcs(); > + vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR; > + vcpu->run->internal.suberror = > + KVM_INTERNAL_ERROR_UNEXPECTED_EXIT_REASON; > + vcpu->run->internal.ndata = 1; > + vcpu->run->internal.data[0] = exit_reason; > + return 0; > } > } > > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h > index 5e3f12d5359e..42070aa5f4e6 100644 > --- a/include/uapi/linux/kvm.h > +++ b/include/uapi/linux/kvm.h > @@ -243,6 +243,8 @@ struct kvm_hyperv_exit { > #define KVM_INTERNAL_ERROR_SIMUL_EX 2 > /* Encounter unexpected vm-exit due to delivery event. */ > #define KVM_INTERNAL_ERROR_DELIVERY_EV 3 > +/* Encounter unexpected vm-exit reason */ > +#define KVM_INTERNAL_ERROR_UNEXPECTED_EXIT_REASON 4 > > /* for KVM_RUN, returned by mmap(vcpu_fd, offset=0) */ > struct kvm_run {
> On 26 Aug 2019, at 13:50, Vitaly Kuznetsov <vkuznets@redhat.com> wrote: > > Liran Alon <liran.alon@oracle.com> writes: > >> Receiving an unexpected exit reason from hardware should be considered >> as a severe bug in KVM. Therefore, instead of just injecting #UD to >> guest and ignore it, exit to userspace on internal error so that >> it could handle it properly (probably by terminating guest). > > While "this should never happen" on real hardware, it is a possible > event for the case when KVM is running as a nested (L1) > hypervisor. Misbehaving L0 can try to inject some weird (corrupted) exit > reason. True. :) That’s true for any vCPU behaviour simulated by L0 to L1. But in that case, I would prefer to terminate L2 guest instead of injecting a bogus #UD to L2 guest... > >> >> In addition, prefer to use vcpu_unimpl() instead of WARN_ONCE() >> as handling unexpected exit reason should be a rare unexpected >> event (that was expected to never happen) and we prefer to print >> a message on it every time it occurs to guest. >> >> Furthermore, dump VMCS/VMCB to dmesg to assist diagnosing such cases. >> >> Reviewed-by: Mihai Carabas <mihai.carabas@oracle.com> >> Reviewed-by: Nikita Leshenko <nikita.leshchenko@oracle.com> >> Reviewed-by: Joao Martins <joao.m.martins@oracle.com> >> Signed-off-by: Liran Alon <liran.alon@oracle.com> > > Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com> Thanks for the review, -Liran > >> --- >> arch/x86/kvm/svm.c | 11 ++++++++--- >> arch/x86/kvm/vmx/vmx.c | 9 +++++++-- >> include/uapi/linux/kvm.h | 2 ++ >> 3 files changed, 17 insertions(+), 5 deletions(-) >> >> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c >> index d685491fce4d..6462c386015d 100644 >> --- a/arch/x86/kvm/svm.c >> +++ b/arch/x86/kvm/svm.c >> @@ -5026,9 +5026,14 @@ static int handle_exit(struct kvm_vcpu *vcpu) >> >> if (exit_code >= ARRAY_SIZE(svm_exit_handlers) >> || !svm_exit_handlers[exit_code]) { >> - WARN_ONCE(1, "svm: unexpected exit reason 0x%x\n", exit_code); >> - kvm_queue_exception(vcpu, UD_VECTOR); >> - return 1; >> + vcpu_unimpl(vcpu, "svm: unexpected exit reason 0x%x\n", exit_code); >> + dump_vmcb(vcpu); >> + vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR; >> + vcpu->run->internal.suberror = >> + KVM_INTERNAL_ERROR_UNEXPECTED_EXIT_REASON; >> + vcpu->run->internal.ndata = 1; >> + vcpu->run->internal.data[0] = exit_code; >> + return 0; >> } >> >> return svm_exit_handlers[exit_code](svm); >> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c >> index 42ed3faa6af8..b5b5b2e5dac5 100644 >> --- a/arch/x86/kvm/vmx/vmx.c >> +++ b/arch/x86/kvm/vmx/vmx.c >> @@ -5887,8 +5887,13 @@ static int vmx_handle_exit(struct kvm_vcpu *vcpu) >> else { >> vcpu_unimpl(vcpu, "vmx: unexpected exit reason 0x%x\n", >> exit_reason); >> - kvm_queue_exception(vcpu, UD_VECTOR); >> - return 1; >> + dump_vmcs(); >> + vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR; >> + vcpu->run->internal.suberror = >> + KVM_INTERNAL_ERROR_UNEXPECTED_EXIT_REASON; >> + vcpu->run->internal.ndata = 1; >> + vcpu->run->internal.data[0] = exit_reason; >> + return 0; >> } >> } >> >> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h >> index 5e3f12d5359e..42070aa5f4e6 100644 >> --- a/include/uapi/linux/kvm.h >> +++ b/include/uapi/linux/kvm.h >> @@ -243,6 +243,8 @@ struct kvm_hyperv_exit { >> #define KVM_INTERNAL_ERROR_SIMUL_EX 2 >> /* Encounter unexpected vm-exit due to delivery event. */ >> #define KVM_INTERNAL_ERROR_DELIVERY_EV 3 >> +/* Encounter unexpected vm-exit reason */ >> +#define KVM_INTERNAL_ERROR_UNEXPECTED_EXIT_REASON 4 >> >> /* for KVM_RUN, returned by mmap(vcpu_fd, offset=0) */ >> struct kvm_run { > > -- > Vitaly
On 26/08/19 12:16, Liran Alon wrote: > Receiving an unexpected exit reason from hardware should be considered > as a severe bug in KVM. Therefore, instead of just injecting #UD to > guest and ignore it, exit to userspace on internal error so that > it could handle it properly (probably by terminating guest). > > In addition, prefer to use vcpu_unimpl() instead of WARN_ONCE() > as handling unexpected exit reason should be a rare unexpected > event (that was expected to never happen) and we prefer to print > a message on it every time it occurs to guest. > > Furthermore, dump VMCS/VMCB to dmesg to assist diagnosing such cases. > > Reviewed-by: Mihai Carabas <mihai.carabas@oracle.com> > Reviewed-by: Nikita Leshenko <nikita.leshchenko@oracle.com> > Reviewed-by: Joao Martins <joao.m.martins@oracle.com> > Signed-off-by: Liran Alon <liran.alon@oracle.com> Queued, thanks. Paolo
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index d685491fce4d..6462c386015d 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -5026,9 +5026,14 @@ static int handle_exit(struct kvm_vcpu *vcpu) if (exit_code >= ARRAY_SIZE(svm_exit_handlers) || !svm_exit_handlers[exit_code]) { - WARN_ONCE(1, "svm: unexpected exit reason 0x%x\n", exit_code); - kvm_queue_exception(vcpu, UD_VECTOR); - return 1; + vcpu_unimpl(vcpu, "svm: unexpected exit reason 0x%x\n", exit_code); + dump_vmcb(vcpu); + vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR; + vcpu->run->internal.suberror = + KVM_INTERNAL_ERROR_UNEXPECTED_EXIT_REASON; + vcpu->run->internal.ndata = 1; + vcpu->run->internal.data[0] = exit_code; + return 0; } return svm_exit_handlers[exit_code](svm); diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index 42ed3faa6af8..b5b5b2e5dac5 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -5887,8 +5887,13 @@ static int vmx_handle_exit(struct kvm_vcpu *vcpu) else { vcpu_unimpl(vcpu, "vmx: unexpected exit reason 0x%x\n", exit_reason); - kvm_queue_exception(vcpu, UD_VECTOR); - return 1; + dump_vmcs(); + vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR; + vcpu->run->internal.suberror = + KVM_INTERNAL_ERROR_UNEXPECTED_EXIT_REASON; + vcpu->run->internal.ndata = 1; + vcpu->run->internal.data[0] = exit_reason; + return 0; } } diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h index 5e3f12d5359e..42070aa5f4e6 100644 --- a/include/uapi/linux/kvm.h +++ b/include/uapi/linux/kvm.h @@ -243,6 +243,8 @@ struct kvm_hyperv_exit { #define KVM_INTERNAL_ERROR_SIMUL_EX 2 /* Encounter unexpected vm-exit due to delivery event. */ #define KVM_INTERNAL_ERROR_DELIVERY_EV 3 +/* Encounter unexpected vm-exit reason */ +#define KVM_INTERNAL_ERROR_UNEXPECTED_EXIT_REASON 4 /* for KVM_RUN, returned by mmap(vcpu_fd, offset=0) */ struct kvm_run {