Gregory Haskins wrote:
> The VCPU executes synchronously w.r.t. userspace today, and therefore
> interrupt injection is pretty straight forward. However, we will soon need
> to be able to inject interrupts asynchronous to the execution of the VCPU
> due to the introduction of SMP, paravirtualized drivers, and asynchronous
> hypercalls. This patch adds support to the interrupt mechanism to force
> a VCPU to VMEXIT when a new interrupt is pending.
>
> Signed-off-by: Gregory Haskins <[EMAIL PROTECTED]>
> ---
>
> drivers/kvm/kvm.h | 5 +++
> drivers/kvm/kvm_main.c | 74
> +++++++++++++++++++++++++++++++++++++++++++++++-
> drivers/kvm/svm.c | 43 ++++++++++++++++++++++++++++
> drivers/kvm/vmx.c | 43 ++++++++++++++++++++++++++++
> 4 files changed, 164 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/kvm/kvm.h b/drivers/kvm/kvm.h
> index d41d653..15c8bec 100644
> --- a/drivers/kvm/kvm.h
> +++ b/drivers/kvm/kvm.h
> @@ -321,6 +321,8 @@ void kvm_io_bus_register_dev(struct kvm_io_bus *bus,
>
> #define NR_IRQ_WORDS KVM_IRQ_BITMAP_SIZE(unsigned long)
>
> +#define KVM_SIGNAL_VIRTUAL_INTERRUPT 33 /* Hardcoded for now */
> +
> /*
> * structure for maintaining info for interrupting an executing VCPU
> */
> @@ -329,6 +331,9 @@ struct kvm_vcpu_irq {
> struct kvm_irqdevice dev;
> int pending;
> int deferred;
> + struct task_struct *task;
> + int signo;
> + int guest_mode;
> };
>
> struct kvm_vcpu {
> diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c
> index 9aeb2f7..6acbd9b 100644
> --- a/drivers/kvm/kvm_main.c
> +++ b/drivers/kvm/kvm_main.c
> @@ -304,6 +304,10 @@ static struct kvm *kvm_create_vm(void)
> memset(&vcpu->irq, 0, sizeof(vcpu->irq));
> spin_lock_init(&vcpu->irq.lock);
> vcpu->irq.deferred = -1;
> + /*
> + * This should be settable by userspace someday
> + */
> + vcpu->irq.signo = KVM_SIGNAL_VIRTUAL_INTERRUPT;
>
This needs to be fixed prior to merging. Hopefully not by setting the
signal number, bit by making the vcpu fd writable (userspace can attach
a signal to the fd if it wishes).
>
> vcpu->cpu = -1;
> vcpu->kvm = kvm;
> @@ -366,13 +370,20 @@ static void free_pio_guest_pages(struct kvm_vcpu *vcpu)
>
> static void kvm_free_vcpu(struct kvm_vcpu *vcpu)
> {
> + unsigned long irqsave;
> +
> if (!vcpu->vmcs)
> return;
>
> vcpu_load(vcpu);
> kvm_mmu_destroy(vcpu);
> vcpu_put(vcpu);
> +
> + spin_lock_irqsave(&vcpu->irq.lock, irqsave);
> + vcpu->irq.task = NULL;
> + spin_unlock_irqrestore(&vcpu->irq.lock, irqsave);
>
Can irq.task be non-NULL here at all? Also, we only free vcpus when we
destroy the vm, and paravirt drivers would hopefully hold a ref to the
vm, so there's nobody to race against here.
> kvm_irqdevice_destructor(&vcpu->irq.dev);
>
> @@ -1868,6 +1880,10 @@ static int kvm_vcpu_ioctl_run(struct kvm_vcpu *vcpu,
> struct kvm_run *kvm_run)
> kvm_arch_ops->decache_regs(vcpu);
> }
>
> + spin_lock_irqsave(&vcpu->irq.lock, irqsaved);
> + vcpu->irq.task = current;
> + spin_unlock_irqrestore(&vcpu->irq.lock, irqsaved);
> +
>
Just assignment + __smp_wmb().
> +/*
> * This function will be invoked whenever the vcpu->irq.dev raises its INTR
> * line
> */
> @@ -2318,10 +2348,52 @@ static void kvm_vcpu_intr(struct kvm_irqsink *this,
> {
> struct kvm_vcpu *vcpu = (struct kvm_vcpu*)this->private;
> unsigned long flags;
> + int direct_ipi = -1;
>
> spin_lock_irqsave(&vcpu->irq.lock, flags);
> - __set_bit(pin, &vcpu->irq.pending);
> +
> + if (!test_bit(pin, &vcpu->irq.pending)) {
> + /*
> + * Record the change..
> + */
> + __set_bit(pin, &vcpu->irq.pending);
> +
> + /*
> + * then wake up the vcpu (if necessary)
> + */
> + if (vcpu->irq.task && (vcpu->irq.task != current)) {
> + if (vcpu->irq.guest_mode) {
> + /*
> + * If we are in guest mode, we can optimize
> + * the IPI by executing a function directly
> + * on the owning processor.
> + */
> + direct_ipi = task_cpu(vcpu->irq.task);
> + BUG_ON(direct_ipi == smp_processor_id());
> + } else
> + /*
> + * otherwise, we must assume that we could be
> + * blocked anywhere, including userspace. Send
> + * a signal to give everyone a chance to get
> + * notification
> + */
> + send_sig(vcpu->irq.signo, vcpu->irq.task, 0);
> + }
> + }
> +
> spin_unlock_irqrestore(&vcpu->irq.lock, flags);
> +
> + if (direct_ipi != -1) {
> + /*
> + * Not sure if disabling preemption is needed.
> + * The kick_process() code does this so I copied it
> + */
> + preempt_disable();
> + smp_call_function_single(direct_ipi,
> + kvm_vcpu_guest_intr,
> + vcpu, 0, 0);
> + preempt_enable();
> + }
>
I see why you must issue the IPI outside the spin_lock_irqsave(), but
aren't you now opening a race? vcpu enters guest mode, irq on other
cpu, irq sets direct_ipi to wakeup guest, releases lock, vcpu exits to
userspace (or migrates to another cpu), ipi is issued but nobody cares.
> /*
> + * Signal that we have transitioned back to host mode
> + */
> + spin_lock_irqsave(&vcpu->irq.lock, irq_flags);
> + vcpu->irq.guest_mode = 0;
> + spin_unlock_irqrestore(&vcpu->irq.lock, irq_flags);
> +
>
Assign + __smp_wmb().
> + /*
> + * Signal that we have transitioned back to host mode
> + */
> + spin_lock_irqsave(&vcpu->irq.lock, irq_flags);
> + vcpu->irq.guest_mode = 0;
> + spin_unlock_irqrestore(&vcpu->irq.lock, irq_flags);
>
Again.
--
error compiling committee.c: too many arguments to function
-------------------------------------------------------------------------
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
