Re: [PATCH 2/2] KVM: X86: Fix loss of exception which has not yet injected

[Wanpeng Li]

2017-08-23 18:12 GMT+08:00 Wanpeng Li <kernellwp@xxxxxxxxx>:
> From: Wanpeng Li <wanpeng.li@xxxxxxxxxxx>
>
> vmx_complete_interrupts() assumes that the exception is always injected,
> so it would be dropped by kvm_clear_exception_queue(). This patch separates
> exception.pending from exception.injected, exception.inject represents the
> exception is injected or the exception should be reinjected due to vmexit
> occurs during event delivery in VMX non-root operation. exception.pending
> represents the exception is queued and will be cleared when injecting the
> exception to the guest. So exception.pending and exception.injected can
> cooperate to guarantee exception will not be lost.
>
> Reported-by: Radim KrÄmÃÅ <rkrcmar@xxxxxxxxxx>
> Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> Cc: Radim KrÄmÃÅ <rkrcmar@xxxxxxxxxx>
> Signed-off-by: Wanpeng Li <wanpeng.li@xxxxxxxxxxx>

I will complete the qemu part for VCPU_EVENTS after this patch is in
an acceptable state.

Regards,
Wanpeng Li

> ---
>  Documentation/virtual/kvm/api.txt     |  2 +-
>  arch/x86/include/asm/kvm_host.h       |  2 +-
>  arch/x86/include/uapi/asm/kvm.h       |  2 +-
>  arch/x86/kvm/vmx.c                    |  4 ++--
>  arch/x86/kvm/x86.c                    | 21 ++++++++++++++++-----
>  arch/x86/kvm/x86.h                    |  4 ++--
>  tools/arch/x86/include/uapi/asm/kvm.h |  2 +-
>  7 files changed, 24 insertions(+), 13 deletions(-)
>
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index e63a35f..6bfb178 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -830,9 +830,9 @@ states of the vcpu.
>  struct kvm_vcpu_events {
>         struct {
>                 __u8 injected;
> +               __u8 pending;
>                 __u8 nr;
>                 __u8 has_error_code;
> -               __u8 pad;
>                 __u32 error_code;
>         } exception;
>         struct {
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index e1e6f00..f3899a3 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -546,8 +546,8 @@ struct kvm_vcpu_arch {
>
>         struct kvm_queued_exception {
>                 bool pending;
> +               bool injected;
>                 bool has_error_code;
> -               bool reinject;
>                 u8 nr;
>                 u32 error_code;
>                 u8 nested_apf;
> diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
> index c2824d0..7c55916 100644
> --- a/arch/x86/include/uapi/asm/kvm.h
> +++ b/arch/x86/include/uapi/asm/kvm.h
> @@ -296,9 +296,9 @@ struct kvm_reinject_control {
>  struct kvm_vcpu_events {
>         struct {
>                 __u8 injected;
> +               __u8 pending;
>                 __u8 nr;
>                 __u8 has_error_code;
> -               __u8 pad;
>                 __u32 error_code;
>         } exception;
>         struct {
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index a56e7f3..f96829c 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -2510,7 +2510,7 @@ static int vmx_queue_exception(struct kvm_vcpu *vcpu)
>         struct vcpu_vmx *vmx = to_vmx(vcpu);
>         unsigned nr = vcpu->arch.exception.nr;
>         bool has_error_code = vcpu->arch.exception.has_error_code;
> -       bool reinject = vcpu->arch.exception.reinject;
> +       bool reinject = vcpu->arch.exception.injected;
>         u32 error_code = vcpu->arch.exception.error_code;
>         u32 intr_info = nr | INTR_INFO_VALID_MASK;
>
> @@ -10891,7 +10891,7 @@ static void vmcs12_save_pending_event(struct kvm_vcpu *vcpu,
>         u32 idt_vectoring;
>         unsigned int nr;
>
> -       if (vcpu->arch.exception.pending && vcpu->arch.exception.reinject) {
> +       if (vcpu->arch.exception.injected) {
>                 nr = vcpu->arch.exception.nr;
>                 idt_vectoring = nr | VECTORING_INFO_VALID_MASK;
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 7414c26..5c680f1 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -389,15 +389,18 @@ static void kvm_multiple_exception(struct kvm_vcpu *vcpu,
>
>         kvm_make_request(KVM_REQ_EVENT, vcpu);
>
> -       if (!vcpu->arch.exception.pending) {
> +       if (!vcpu->arch.exception.pending ||
> +               !vcpu->arch.exception.injected) {
>         queue:
>                 if (has_error && !is_protmode(vcpu))
>                         has_error = false;
> -               vcpu->arch.exception.pending = true;
> +               if (reinject)
> +                       vcpu->arch.exception.injected = true;
> +               else
> +                       vcpu->arch.exception.pending = true;
>                 vcpu->arch.exception.has_error_code = has_error;
>                 vcpu->arch.exception.nr = nr;
>                 vcpu->arch.exception.error_code = error_code;
> -               vcpu->arch.exception.reinject = reinject;
>                 return;
>         }
>
> @@ -3069,12 +3072,12 @@ static void kvm_vcpu_ioctl_x86_get_vcpu_events(struct kvm_vcpu *vcpu,
>                                                struct kvm_vcpu_events *events)
>  {
>         process_nmi(vcpu);
> +       events->exception.injected = vcpu->arch.exception.injected;
>         events->exception.injected =
>                 vcpu->arch.exception.pending &&
>                 !kvm_exception_is_soft(vcpu->arch.exception.nr);
>         events->exception.nr = vcpu->arch.exception.nr;
>         events->exception.has_error_code = vcpu->arch.exception.has_error_code;
> -       events->exception.pad = 0;
>         events->exception.error_code = vcpu->arch.exception.error_code;
>
>         events->interrupt.injected =
> @@ -3125,6 +3128,7 @@ static int kvm_vcpu_ioctl_x86_set_vcpu_events(struct kvm_vcpu *vcpu,
>                 return -EINVAL;
>
>         process_nmi(vcpu);
> +       vcpu->arch.exception.injected = events->exception.injected;
>         vcpu->arch.exception.pending = events->exception.injected;
>         vcpu->arch.exception.nr = events->exception.nr;
>         vcpu->arch.exception.has_error_code = events->exception.has_error_code;
> @@ -6341,7 +6345,8 @@ static int inject_pending_event(struct kvm_vcpu *vcpu, bool req_int_win)
>         int r;
>
>         /* try to reinject previous events if any */
> -       if (vcpu->arch.exception.pending) {
> +       if (vcpu->arch.exception.pending ||
> +               vcpu->arch.exception.injected) {
>                 trace_kvm_inj_exception(vcpu->arch.exception.nr,
>                                         vcpu->arch.exception.has_error_code,
>                                         vcpu->arch.exception.error_code);
> @@ -6357,6 +6362,11 @@ static int inject_pending_event(struct kvm_vcpu *vcpu, bool req_int_win)
>                 }
>
>                 r = kvm_x86_ops->queue_exception(vcpu);
> +               if (r == 0 && vcpu->arch.exception.pending &&
> +                       !vcpu->arch.exception.injected) {
> +                       vcpu->arch.exception.pending = false;
> +                       vcpu->arch.exception.injected = true;
> +               }
>                 return r;
>         }
>
> @@ -7727,6 +7737,7 @@ void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
>         vcpu->arch.nmi_injected = false;
>         kvm_clear_interrupt_queue(vcpu);
>         kvm_clear_exception_queue(vcpu);
> +       vcpu->arch.exception.pending = false;
>
>         memset(vcpu->arch.db, 0, sizeof(vcpu->arch.db));
>         kvm_update_dr0123(vcpu);
> diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
> index 1134603..e6ec0de 100644
> --- a/arch/x86/kvm/x86.h
> +++ b/arch/x86/kvm/x86.h
> @@ -11,7 +11,7 @@
>
>  static inline void kvm_clear_exception_queue(struct kvm_vcpu *vcpu)
>  {
> -       vcpu->arch.exception.pending = false;
> +       vcpu->arch.exception.injected = false;
>  }
>
>  static inline void kvm_queue_interrupt(struct kvm_vcpu *vcpu, u8 vector,
> @@ -29,7 +29,7 @@ static inline void kvm_clear_interrupt_queue(struct kvm_vcpu *vcpu)
>
>  static inline bool kvm_event_needs_reinjection(struct kvm_vcpu *vcpu)
>  {
> -       return vcpu->arch.exception.pending || vcpu->arch.interrupt.pending ||
> +       return vcpu->arch.exception.injected || vcpu->arch.interrupt.pending ||
>                 vcpu->arch.nmi_injected;
>  }
>
> diff --git a/tools/arch/x86/include/uapi/asm/kvm.h b/tools/arch/x86/include/uapi/asm/kvm.h
> index c2824d0..7c55916 100644
> --- a/tools/arch/x86/include/uapi/asm/kvm.h
> +++ b/tools/arch/x86/include/uapi/asm/kvm.h
> @@ -296,9 +296,9 @@ struct kvm_reinject_control {
>  struct kvm_vcpu_events {
>         struct {
>                 __u8 injected;
> +               __u8 pending;
>                 __u8 nr;
>                 __u8 has_error_code;
> -               __u8 pad;
>                 __u32 error_code;
>         } exception;
>         struct {
> --
> 2.7.4
>