KVM: x86: Rename interrupt.pending to interrupt.injected
authorLiran Alon <liran.alon@oracle.com>
Fri, 23 Mar 2018 00:01:31 +0000 (03:01 +0300)
committerRadim Krčmář <rkrcmar@redhat.com>
Wed, 28 Mar 2018 20:47:06 +0000 (22:47 +0200)
For exceptions & NMIs events, KVM code use the following
coding convention:
*) "pending" represents an event that should be injected to guest at
some point but it's side-effects have not yet occurred.
*) "injected" represents an event that it's side-effects have already
occurred.

However, interrupts don't conform to this coding convention.
All current code flows mark interrupt.pending when it's side-effects
have already taken place (For example, bit moved from LAPIC IRR to
ISR). Therefore, it makes sense to just rename
interrupt.pending to interrupt.injected.

This change follows logic of previous commit 664f8e26b00c ("KVM: X86:
Fix loss of exception which has not yet been injected") which changed
exception to follow this coding convention as well.

It is important to note that in case !lapic_in_kernel(vcpu),
interrupt.pending usage was and still incorrect.
In this case, interrrupt.pending can only be set using one of the
following ioctls: KVM_INTERRUPT, KVM_SET_VCPU_EVENTS and
KVM_SET_SREGS. Looking at how QEMU uses these ioctls, one can see that
QEMU uses them either to re-set an "interrupt.pending" state it has
received from KVM (via KVM_GET_VCPU_EVENTS interrupt.pending or
via KVM_GET_SREGS interrupt_bitmap) or by dispatching a new interrupt
from QEMU's emulated LAPIC which reset bit in IRR and set bit in ISR
before sending ioctl to KVM. So it seems that indeed "interrupt.pending"
in this case is also suppose to represent "interrupt.injected".
However, kvm_cpu_has_interrupt() & kvm_cpu_has_injectable_intr()
is misusing (now named) interrupt.injected in order to return if
there is a pending interrupt.
This leads to nVMX/nSVM not be able to distinguish if it should exit
from L2 to L1 on EXTERNAL_INTERRUPT on pending interrupt or should
re-inject an injected interrupt.
Therefore, add a FIXME at these functions for handling this issue.

This patch introduce no semantics change.

Signed-off-by: Liran Alon <liran.alon@oracle.com>
Reviewed-by: Nikita Leshenko <nikita.leshchenko@oracle.com>
Reviewed-by: Jim Mattson <jmattson@google.com>
Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
arch/x86/include/asm/kvm_host.h
arch/x86/kvm/irq.c
arch/x86/kvm/vmx.c
arch/x86/kvm/x86.c
arch/x86/kvm/x86.h

index 74b5b3e518df9aee96092d90305924c70dab72d1..949c977bc4c941b5b1ba9db039b948ce22a0ae13 100644 (file)
@@ -574,7 +574,7 @@ struct kvm_vcpu_arch {
        } exception;
 
        struct kvm_queued_interrupt {
-               bool pending;
+               bool injected;
                bool soft;
                u8 nr;
        } interrupt;
index f171051eecf3473f529abeebeef80e6893ed672a..faa264822cee3c658280d122cc3495fc93587cf5 100644 (file)
@@ -73,8 +73,19 @@ static int kvm_cpu_has_extint(struct kvm_vcpu *v)
  */
 int kvm_cpu_has_injectable_intr(struct kvm_vcpu *v)
 {
+       /*
+        * FIXME: interrupt.injected represents an interrupt that it's
+        * side-effects have already been applied (e.g. bit from IRR
+        * already moved to ISR). Therefore, it is incorrect to rely
+        * on interrupt.injected to know if there is a pending
+        * interrupt in the user-mode LAPIC.
+        * This leads to nVMX/nSVM not be able to distinguish
+        * if it should exit from L2 to L1 on EXTERNAL_INTERRUPT on
+        * pending interrupt or should re-inject an injected
+        * interrupt.
+        */
        if (!lapic_in_kernel(v))
-               return v->arch.interrupt.pending;
+               return v->arch.interrupt.injected;
 
        if (kvm_cpu_has_extint(v))
                return 1;
@@ -91,8 +102,19 @@ int kvm_cpu_has_injectable_intr(struct kvm_vcpu *v)
  */
 int kvm_cpu_has_interrupt(struct kvm_vcpu *v)
 {
+       /*
+        * FIXME: interrupt.injected represents an interrupt that it's
+        * side-effects have already been applied (e.g. bit from IRR
+        * already moved to ISR). Therefore, it is incorrect to rely
+        * on interrupt.injected to know if there is a pending
+        * interrupt in the user-mode LAPIC.
+        * This leads to nVMX/nSVM not be able to distinguish
+        * if it should exit from L2 to L1 on EXTERNAL_INTERRUPT on
+        * pending interrupt or should re-inject an injected
+        * interrupt.
+        */
        if (!lapic_in_kernel(v))
-               return v->arch.interrupt.pending;
+               return v->arch.interrupt.injected;
 
        if (kvm_cpu_has_extint(v))
                return 1;
index a0492cdd48915140d816764233ebcb519e07d633..7d2537ab262bfa44a700a0451a85287f09f2dc0a 100644 (file)
@@ -11629,7 +11629,7 @@ static void vmcs12_save_pending_event(struct kvm_vcpu *vcpu,
        } else if (vcpu->arch.nmi_injected) {
                vmcs12->idt_vectoring_info_field =
                        INTR_TYPE_NMI_INTR | INTR_INFO_VALID_MASK | NMI_VECTOR;
-       } else if (vcpu->arch.interrupt.pending) {
+       } else if (vcpu->arch.interrupt.injected) {
                nr = vcpu->arch.interrupt.nr;
                idt_vectoring = nr | VECTORING_INFO_VALID_MASK;
 
index dfa135bb0e5a30914be7871f0b10868a6074a252..d15dc8cd6b9da6ae8bb89e96936ebcf1120546bb 100644 (file)
@@ -3312,7 +3312,7 @@ static void kvm_vcpu_ioctl_x86_get_vcpu_events(struct kvm_vcpu *vcpu,
        events->exception.error_code = vcpu->arch.exception.error_code;
 
        events->interrupt.injected =
-               vcpu->arch.interrupt.pending && !vcpu->arch.interrupt.soft;
+               vcpu->arch.interrupt.injected && !vcpu->arch.interrupt.soft;
        events->interrupt.nr = vcpu->arch.interrupt.nr;
        events->interrupt.soft = 0;
        events->interrupt.shadow = kvm_x86_ops->get_interrupt_shadow(vcpu);
@@ -3365,7 +3365,7 @@ static int kvm_vcpu_ioctl_x86_set_vcpu_events(struct kvm_vcpu *vcpu,
        vcpu->arch.exception.has_error_code = events->exception.has_error_code;
        vcpu->arch.exception.error_code = events->exception.error_code;
 
-       vcpu->arch.interrupt.pending = events->interrupt.injected;
+       vcpu->arch.interrupt.injected = events->interrupt.injected;
        vcpu->arch.interrupt.nr = events->interrupt.nr;
        vcpu->arch.interrupt.soft = events->interrupt.soft;
        if (events->flags & KVM_VCPUEVENT_VALID_SHADOW)
@@ -6767,7 +6767,7 @@ static int inject_pending_event(struct kvm_vcpu *vcpu, bool req_int_win)
                        return 0;
                }
 
-               if (vcpu->arch.interrupt.pending) {
+               if (vcpu->arch.interrupt.injected) {
                        kvm_x86_ops->set_irq(vcpu);
                        return 0;
                }
@@ -7818,7 +7818,7 @@ static void __get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs)
 
        memset(sregs->interrupt_bitmap, 0, sizeof sregs->interrupt_bitmap);
 
-       if (vcpu->arch.interrupt.pending && !vcpu->arch.interrupt.soft)
+       if (vcpu->arch.interrupt.injected && !vcpu->arch.interrupt.soft)
                set_bit(vcpu->arch.interrupt.nr,
                        (unsigned long *)sregs->interrupt_bitmap);
 }
index 42350391b62f53b210b21267969827eefc5f8646..1e8617414ee4b920678f661c09228eac31ecce4a 100644 (file)
@@ -55,19 +55,19 @@ static inline void kvm_clear_exception_queue(struct kvm_vcpu *vcpu)
 static inline void kvm_queue_interrupt(struct kvm_vcpu *vcpu, u8 vector,
        bool soft)
 {
-       vcpu->arch.interrupt.pending = true;
+       vcpu->arch.interrupt.injected = true;
        vcpu->arch.interrupt.soft = soft;
        vcpu->arch.interrupt.nr = vector;
 }
 
 static inline void kvm_clear_interrupt_queue(struct kvm_vcpu *vcpu)
 {
-       vcpu->arch.interrupt.pending = false;
+       vcpu->arch.interrupt.injected = false;
 }
 
 static inline bool kvm_event_needs_reinjection(struct kvm_vcpu *vcpu)
 {
-       return vcpu->arch.exception.injected || vcpu->arch.interrupt.pending ||
+       return vcpu->arch.exception.injected || vcpu->arch.interrupt.injected ||
                vcpu->arch.nmi_injected;
 }