> On 26 Aug 2019, at 13:50, Vitaly Kuznetsov <vkuznets@xxxxxxxxxx> wrote:
>
> Liran Alon <liran.alon@xxxxxxxxxx> 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@xxxxxxxxxx>
>> Reviewed-by: Nikita Leshenko <nikita.leshchenko@xxxxxxxxxx>
>> Reviewed-by: Joao Martins <joao.m.martins@xxxxxxxxxx>
>> Signed-off-by: Liran Alon <liran.alon@xxxxxxxxxx>
>
> Reviewed-by: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx>
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
