>-static inline void kvm_do_inject_irq(struct kvm_vcpu *vcpu)
>-{
>- struct vmcb_control_area *control;
>-
>- control = &vcpu->svm->vmcb->control;
>- control->int_vector = pop_irq(vcpu);
>- control->int_ctl &= ~V_INTR_PRIO_MASK;
>- control->int_ctl |= V_IRQ_MASK |
>- ((/*control->int_vector >> 4*/ 0xf) <<
V_INTR_PRIO_SHIFT);
>-}
>-
Keep up the good work, looks like its converging.
BTW, what types of VMs are running with your apic?
I have some comments below:
> static void kvm_reput_irq(struct kvm_vcpu *vcpu)
> {
> struct vmcb_control_area *control = &vcpu->svm->vmcb->control;
>
> if (control->int_ctl & V_IRQ_MASK) {
> control->int_ctl &= ~V_IRQ_MASK;
>- push_irq(vcpu, control->int_vector);
>+ kvm_vcpu_irq_push(vcpu, control->int_vector);
> }
>
> vcpu->interrupt_window_open =
> !(control->int_state & SVM_INTERRUPT_SHADOW_MASK);
> }
>
>-static void do_interrupt_requests(struct kvm_vcpu *vcpu,
>- struct kvm_run *kvm_run)
>+static int do_intr_requests(struct kvm_vcpu *vcpu,
>+ struct kvm_run *kvm_run,
>+ kvm_irqpin_t pin)
> {
> struct vmcb_control_area *control = &vcpu->svm->vmcb->control;
>+ int handled = 0;
>
> vcpu->interrupt_window_open =
> (!(control->int_state & SVM_INTERRUPT_SHADOW_MASK) &&
> (vcpu->svm->vmcb->save.rflags & X86_EFLAGS_IF));
>
>- if (vcpu->interrupt_window_open && vcpu->irq_summary)
>+ if (vcpu->interrupt_window_open) {
> /*
>- * If interrupts enabled, and not blocked by sti or mov
ss.
>Good.
>+ * If interrupts enabled, and not blocked by sti or mov
ss.
>+ * Good.
> */
>- kvm_do_inject_irq(vcpu);
>+ struct kvm_irqack_data ack;
>+ int r = 0;
>+
>+ memset(&ack, 0, sizeof(ack));
>+
>+ switch (pin) {
>+ case kvm_irqpin_localint:
>+ r = kvm_vcpu_irq_pop(vcpu, &ack);
>+ break;
>+ case kvm_irqpin_extint:
>+ printk(KERN_WARNING "KVM: external-interrupts
not " \
>+ "handled yet\n");
>+ __clear_bit(pin, &vcpu->irq.pending);
>+ break;
>+ case kvm_irqpin_nmi:
>+ /*
>+ * FIXME: Someday we will handle this using the
>+ * specific SVN NMI features. For now, just
inject
>+ * the NMI as a standard interrupt on vector 2
>+ */
>+ ack.flags |= KVM_IRQACKDATA_VECTOR_VALID;
>+ ack.vector = 2;
>+ __clear_bit(pin, &vcpu->irq.pending);
>+ break;
>+ default:
>+ panic("KVM: unknown interrupt pin raised: %d\n",
pin);
>+ break;
>+ }
>+
>+ BUG_ON(r < 0);
>+
The above code should be arch-generic.
>+ if (ack.flags & KVM_IRQACKDATA_VECTOR_VALID) {
>+ control = &vcpu->svm->vmcb->control;
>+ control->int_vector = ack.vector;
>+ control->int_ctl &= ~V_INTR_PRIO_MASK;
>+ control->int_ctl |= V_IRQ_MASK |
>+ ((/*control->int_vector >> 4*/ 0xf) <<
>+ V_INTR_PRIO_SHIFT);
>+
>+ handled = 1;
>+ }
>+ }
>
> /*
> * Interrupts blocked. Wait for unblock.
> */
> if (!vcpu->interrupt_window_open &&
>- (vcpu->irq_summary || kvm_run->request_interrupt_window)) {
>+ (__kvm_vcpu_irq_pending(vcpu) ||
>+ kvm_run->request_interrupt_window))
> control->intercept |= 1ULL << INTERCEPT_VINTR;
>- } else
>- control->intercept &= ~(1ULL << INTERCEPT_VINTR);
>+
>+ return handled;
>+}
>+
>+static void clear_pending_controls(struct kvm_vcpu *vcpu)
>+{
>+ struct vmcb_control_area *control = &vcpu->svm->vmcb->control;
>+
>+ control->intercept &= ~(1ULL << INTERCEPT_VINTR);
>+}
>+
IMHO I think that do_interrupt_requests and do_intr_requests can be
united into one. The switch(pin) in both of them is unnatural.
>+static void do_interrupt_requests(struct kvm_vcpu *vcpu,
>+ struct kvm_run *kvm_run)
>+{
>+ int pending = __kvm_vcpu_irq_all_pending(vcpu);
>+
>+ clear_pending_controls(vcpu);
>+
>+ while (pending) {
>+ kvm_irqpin_t pin = __fls(pending);
>+
>+ switch (pin) {
>+ case kvm_irqpin_localint:
>+ case kvm_irqpin_extint:
>+ case kvm_irqpin_nmi:
>+ do_intr_requests(vcpu, kvm_run, pin);
>+ break;
>+ case kvm_irqpin_smi:
>+ /* ignored (for now) */
>+ printk(KERN_WARNING "KVM: dropping unhandled
SMI\n");
>+ __clear_bit(pin, &vcpu->irq.pending);
>+ break;
>+ case kvm_irqpin_invalid:
>+ /* drop */
>+ break;
>+ default:
>+ panic("KVM: unknown interrupt pin raised: %d\n",
pin);
>+ break;
>+ }
>+
>+ __clear_bit(pin, &pending);
>+ }
> }
Seems like you can inject several irq at once using the above while
loop, but you only do one push in case external interrupt got in the way
and prevented the injection.
>
> static void post_kvm_run_save(struct kvm_vcpu *vcpu,
> struct kvm_run *kvm_run)
> {
>- kvm_run->ready_for_interrupt_injection =
(vcpu->interrupt_window_open
>&&
>- vcpu->irq_summary ==
0);
>+ kvm_run->ready_for_interrupt_injection =
>+ (vcpu->interrupt_window_open &&
>+ !kvm_vcpu_irq_pending(vcpu));
> kvm_run->if_flag = (vcpu->svm->vmcb->save.rflags &
X86_EFLAGS_IF) !=
>0;
> kvm_run->cr8 = vcpu->cr8;
> kvm_run->apic_base = vcpu->apic_base;
>@@ -1452,7 +1514,7 @@ static void post_kvm_run_save(struct kvm_vcpu
*vcpu,
> static int dm_request_for_irq_injection(struct kvm_vcpu *vcpu,
> struct kvm_run *kvm_run)
> {
>- return (!vcpu->irq_summary &&
>+ return (!kvm_vcpu_irq_pending(vcpu) &&
> kvm_run->request_interrupt_window &&
> vcpu->interrupt_window_open &&
> (vcpu->svm->vmcb->save.rflags & X86_EFLAGS_IF));
>@@ -1482,9 +1544,17 @@ static int svm_vcpu_run(struct kvm_vcpu *vcpu,
>struct kvm_run *kvm_run)
> int r;
>
> again:
>+ spin_lock(&vcpu->irq.lock);
>+
>+ /*
>+ * We must inject interrupts (if any) while the irq_lock
>+ * is held
>+ */
> if (!vcpu->mmio_read_completed)
> do_interrupt_requests(vcpu, kvm_run);
>
>+ spin_unlock(&vcpu->irq.lock);
>+
> clgi();
>
> pre_svm_run(vcpu);
>diff --git a/drivers/kvm/userint.c b/drivers/kvm/userint.c
>new file mode 100644
>index 0000000..08d26fa
>--- /dev/null
>+++ b/drivers/kvm/userint.c
>@@ -0,0 +1,223 @@
>+/*
>+ * User Interrupts IRQ device
>+ *
>+ * This acts as an extention of an interrupt controller that exists
>elsewhere
>+ * (typically in userspace/QEMU). Because this PIC is a pseudo device
>that
>+ * is downstream from a real emulated PIC, the "IRQ-to-vector" mapping
has
>+ * already occured. Therefore, this PIC has the following unusal
>properties:
>+ *
>+ * 1) It has 256 "pins" which are literal vectors (i.e. no
translation)
>+ * 2) It only supports "auto-EOI" behavior since it is expected that
the
>+ * upstream emulated PIC will handle the real EOIs (if applicable)
>+ * 3) It only listens to "asserts" on the pins (deasserts are dropped)
>+ * because its an auto-EOI device anyway.
>+ *
>+ * Copyright (C) 2007 Novell
>+ *
>+ * bitarray code based on original vcpu->irq_pending code,
>+ * Copyright (C) 2007 Qumranet
>+ *
>+ * Authors:
>+ * Gregory Haskins <[EMAIL PROTECTED]>
>+ *
>+ * This work is licensed under the terms of the GNU GPL, version 2.
See
>+ * the COPYING file in the top-level directory.
>+ *
>+ */
>+
>+#include "kvm.h"
>+
>+/*
>+
*----------------------------------------------------------------------
>+ * optimized bitarray object - works like bitarrays in bitops, but
uses
>+ * a summary field to accelerate lookups. Assumes external locking
>+
*---------------------------------------------------------------------
>+ */
>+
>+struct bitarray {
>+ unsigned long summary; /* 1 per word in pending */
>+ unsigned long pending[NR_IRQ_WORDS];
>+};
>+
>+static inline int bitarray_pending(struct bitarray *this)
>+{
>+ return this->summary ? 1 : 0;
>+}
>+
>+static inline int bitarray_findhighest(struct bitarray *this)
>+{
>+ if (!this->summary)
>+ return -1;
>+ else {
No need for else, simpler.
>+ int word_index = __fls(this->summary);
>+ int bit_index = __fls(this->pending[word_index]);
>+
>+ return word_index * BITS_PER_LONG + bit_index;
>+ }
>+}
>+
>+static inline void bitarray_set(struct bitarray *this, int nr)
>+{
>+ __set_bit(nr, &this->pending);
>+ __set_bit(nr / BITS_PER_LONG, &this->summary);
>+}
>+
>+static inline void bitarray_clear(struct bitarray *this, int nr)
>+{
>+ int word = nr / BITS_PER_LONG;
>+
>+ __clear_bit(nr, &this->pending);
>+ if (!this->pending[word])
>+ __clear_bit(word, &this->summary);
>+}
>+
>+static inline int bitarray_test(struct bitarray *this, int nr)
>+{
>+ return test_bit(nr, &this->pending);
>+}
>+
>+static inline int bitarray_test_and_set(struct bitarray *this, int nr,
int
>val)
>+{
>+ if (bitarray_test(this, nr) != val) {
>+ if (val)
>+ bitarray_set(this, nr);
>+ else
>+ bitarray_clear(this, nr);
>+ return 1;
>+ }
>+
>+ return 0;
>+}
>+
>+/*
>+
*----------------------------------------------------------------------
>+ * userint interface - provides the actual kvm_irqdevice
implementation
>+
*---------------------------------------------------------------------
>+ */
>+
>+struct kvm_user_irqdev {
>+ spinlock_t lock;
>+ atomic_t ref_count;
>+ struct bitarray pending;
>+};
>+
>+static int user_irqdev_ack(struct kvm_irqdevice *this, int flags,
>+ struct kvm_irqack_data *data)
>+{
>+ struct kvm_user_irqdev *s = (struct
kvm_user_irqdev*)this->private;
>+
>+ spin_lock(&s->lock);
>+
>+ if (!(flags & KVM_IRQACK_FLAG_PEEK)) {
>+ int irq = bitarray_findhighest(&s->pending);
>+
>+ if (irq > -1) {
>+ /*
>+ * Automatically clear the interrupt as the EOI
>+ * mechanism (if any) will take place in
userspace
>+ */
>+ bitarray_clear(&s->pending, irq);
>+
>+ data->flags |= KVM_IRQACKDATA_VECTOR_VALID;
>+ }
>+
>+ data->vector = irq;
>+ }
>+
>+ if (bitarray_pending(&s->pending))
>+ data->flags |= KVM_IRQACKDATA_VECTOR_PENDING;
>+
>+ spin_unlock(&s->lock);
>+
>+ return 0;
>+}
>+
>+static int user_irqdev_set_pin(struct kvm_irqdevice *this, int irq,
int
>level)
>+{
>+ struct kvm_user_irqdev *s = (struct
kvm_user_irqdev*)this->private;
>+ int forward = 0;
>+
>+ spin_lock(&s->lock);
>+ forward = bitarray_test_and_set(&s->pending, irq, level);
>+ spin_unlock(&s->lock);
>+
>+ /*
>+ * alert the higher layer software we have changes
>+ */
>+ if (forward)
>+ kvm_irqdevice_set_intr(this, kvm_irqpin_localint);
>+
>+ return 0;
>+}
>+
>+static void user_irqdev_destructor(struct kvm_irqdevice *this)
>+{
>+ struct kvm_user_irqdev *s = (struct
kvm_user_irqdev*)this->private;
>+
>+ if (atomic_dec_and_test(&s->ref_count))
>+ kfree(s);
>+}
>+
>+int kvm_user_irqdev_init(struct kvm_irqdevice *irqdev)
>+{
>+ struct kvm_user_irqdev *s;
>+
>+ s = kzalloc(sizeof(*s), GFP_KERNEL);
>+ if (!s)
>+ return -ENOMEM;
>+
>+ spin_lock_init(&s->lock);
>+
>+ irqdev->ack = user_irqdev_ack;
>+ irqdev->set_pin = user_irqdev_set_pin;
>+ irqdev->destructor = user_irqdev_destructor;
>+
>+ irqdev->private = s;
>+ atomic_inc(&s->ref_count);
>+
>+ return 0;
>+}
>+
>+int kvm_user_irqdev_save(struct kvm_irqdevice *this, void *data)
>+{
>+ struct kvm_user_irqdev *s = (struct
kvm_user_irqdev*)this->private;
>+
>+ spin_lock(&s->lock);
>+ memcpy(data, s->pending.pending, sizeof s->pending.pending);
>+ spin_unlock(&s->lock);
>+
>+ return 0;
>+}
>+
>+int kvm_user_irqdev_restore(struct kvm_irqdevice *this, void *data)
>+{
>+ struct kvm_user_irqdev *s = (struct
kvm_user_irqdev*)this->private;
>+ int i;
>+ int forward = 0;
>+
>+ spin_lock(&s->lock);
>+
>+ /*
>+ * walk the interrupt-bitmap and inject an IRQ for each bit
found
>+ */
>+ for (i = 0; i < 256; ++i) {
>+ int val = test_bit(i, data);
>+ forward = bitarray_test_and_set(&s->pending, i, val);
>+ }
>+
You run over forward each loop but below you use it.
>+ spin_unlock(&s->lock);
>+
>+ /*
>+ * alert the higher layer software we have changes
>+ */
>+ if (forward)
>+ kvm_irqdevice_set_intr(this, kvm_irqpin_localint);
>+
>+ return 0;
>+}
>+
>+int kvm_userint_init(struct kvm_vcpu *vcpu)
>+{
>+ return kvm_user_irqdev_init(&vcpu->irq.dev);
>+}
>+
>diff --git a/drivers/kvm/vmx.c b/drivers/kvm/vmx.c
>index 19edb34..ca858cb 100644
>--- a/drivers/kvm/vmx.c
>+++ b/drivers/kvm/vmx.c
>@@ -1301,52 +1301,118 @@ static void inject_rmode_irq(struct kvm_vcpu
>*vcpu, int irq)
> vmcs_writel(GUEST_RSP, (vmcs_readl(GUEST_RSP) & ~0xffff) | (sp -
6));
> }
>
>-static void kvm_do_inject_irq(struct kvm_vcpu *vcpu)
>+static int do_intr_requests(struct kvm_vcpu *vcpu,
>+ struct kvm_run *kvm_run,
>+ kvm_irqpin_t pin)
> {
>- int word_index = __ffs(vcpu->irq_summary);
>- int bit_index = __ffs(vcpu->irq_pending[word_index]);
>- int irq = word_index * BITS_PER_LONG + bit_index;
>-
>- clear_bit(bit_index, &vcpu->irq_pending[word_index]);
>- if (!vcpu->irq_pending[word_index])
>- clear_bit(word_index, &vcpu->irq_summary);
>-
>- if (vcpu->rmode.active) {
>- inject_rmode_irq(vcpu, irq);
>- return;
>- }
>- vmcs_write32(VM_ENTRY_INTR_INFO_FIELD,
>- irq | INTR_TYPE_EXT_INTR |
INTR_INFO_VALID_MASK);
>-}
>-
>-
>-static void do_interrupt_requests(struct kvm_vcpu *vcpu,
>- struct kvm_run *kvm_run)
>-{
>- u32 cpu_based_vm_exec_control;
>+ int handled = 0;
>
> vcpu->interrupt_window_open =
> ((vmcs_readl(GUEST_RFLAGS) & X86_EFLAGS_IF) &&
> (vmcs_read32(GUEST_INTERRUPTIBILITY_INFO) & 3) == 0);
>
> if (vcpu->interrupt_window_open &&
>- vcpu->irq_summary &&
>- !(vmcs_read32(VM_ENTRY_INTR_INFO_FIELD) &
INTR_INFO_VALID_MASK))
>+ !(vmcs_read32(VM_ENTRY_INTR_INFO_FIELD) &
INTR_INFO_VALID_MASK))
>{
> /*
>- * If interrupts enabled, and not blocked by sti or mov
ss.
>Good.
>+ * If interrupts enabled, and not blocked by sti or mov
ss.
>+ * Good.
> */
>- kvm_do_inject_irq(vcpu);
>+ struct kvm_irqack_data ack;
>+ int r = 0;
>+
>+ memset(&ack, 0, sizeof(ack));
>+
>+ switch (pin) {
>+ case kvm_irqpin_localint:
>+ r = kvm_vcpu_irq_pop(vcpu, &ack);
>+ break;
>+ case kvm_irqpin_extint:
>+ printk(KERN_WARNING "KVM: external-interrupts
not " \
>+ "handled yet\n");
>+ __clear_bit(pin, &vcpu->irq.pending);
>+ break;
>+ case kvm_irqpin_nmi:
>+ /*
>+ * FIXME: Someday we will handle this using the
>+ * specific VMX NMI features. For now, just
inject
>+ * the NMI as a standard interrupt on vector 2
>+ */
>+ ack.flags |= KVM_IRQACKDATA_VECTOR_VALID;
>+ ack.vector = 2;
>+ __clear_bit(pin, &vcpu->irq.pending);
>+ break;
>+ default:
>+ panic("KVM: unknown interrupt pin raised: %d\n",
pin);
>+ break;
>+ }
>+
>+ BUG_ON(r < 0);
>+
>+ if (ack.flags & KVM_IRQACKDATA_VECTOR_VALID) {
>+ if (vcpu->rmode.active)
>+ inject_rmode_irq(vcpu, ack.vector);
>+ else
>+ vmcs_write32(VM_ENTRY_INTR_INFO_FIELD,
>+ ack.vector |
>+ INTR_TYPE_EXT_INTR |
>+ INTR_INFO_VALID_MASK);
>+
>+ handled = 1;
>+ }
>+ }
>
>- cpu_based_vm_exec_control =
vmcs_read32(CPU_BASED_VM_EXEC_CONTROL);
> if (!vcpu->interrupt_window_open &&
>- (vcpu->irq_summary || kvm_run->request_interrupt_window))
>+ (__kvm_vcpu_irq_pending(vcpu) ||
>+ kvm_run->request_interrupt_window)) {
> /*
> * Interrupts blocked. Wait for unblock.
> */
>- cpu_based_vm_exec_control |=
CPU_BASED_VIRTUAL_INTR_PENDING;
>- else
>- cpu_based_vm_exec_control &=
~CPU_BASED_VIRTUAL_INTR_PENDING;
>- vmcs_write32(CPU_BASED_VM_EXEC_CONTROL,
cpu_based_vm_exec_control);
>+ u32 cbvec = vmcs_read32(CPU_BASED_VM_EXEC_CONTROL);
>+ cbvec |= CPU_BASED_VIRTUAL_INTR_PENDING;
>+ vmcs_write32(CPU_BASED_VM_EXEC_CONTROL, cbvec);
>+ }
>+
>+ return handled;
>+}
>+
>+static void clear_pending_controls(struct kvm_vcpu *vcpu)
>+{
>+ u32 cbvec = vmcs_read32(CPU_BASED_VM_EXEC_CONTROL);
>+ cbvec &= ~CPU_BASED_VIRTUAL_INTR_PENDING;
>+ vmcs_write32(CPU_BASED_VM_EXEC_CONTROL, cbvec);
>+}
>+
>+static void do_interrupt_requests(struct kvm_vcpu *vcpu,
>+ struct kvm_run *kvm_run)
>+{
>+ int pending = __kvm_vcpu_irq_all_pending(vcpu);
>+
>+ clear_pending_controls(vcpu);
>+
>+ while (pending) {
>+ kvm_irqpin_t pin = __fls(pending);
>+
>+ switch (pin) {
>+ case kvm_irqpin_localint:
>+ case kvm_irqpin_extint:
>+ case kvm_irqpin_nmi:
>+ do_intr_requests(vcpu, kvm_run, pin);
>+ break;
>+ case kvm_irqpin_smi:
>+ /* ignored (for now) */
>+ printk(KERN_WARNING "KVM: dropping unhandled
SMI\n");
>+ __clear_bit(pin, &vcpu->irq.pending);
>+ break;
>+ case kvm_irqpin_invalid:
>+ /* drop */
>+ break;
>+ default:
>+ panic("KVM: unknown interrupt pin raised: %d\n",
pin);
>+ break;
>+ }
>+
>+ __clear_bit(pin, &pending);
>+ }
> }
Now the do_interrupt_requests function is totally arch-generalized.
I think that do_intr_requests should be generalized too.
-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
