[v2] Shared guest irq support

[v2] Shared guest irq support

[Amit Shah]

[-- Attachment #1: Type: text/plain, Size: 78 bytes --]

Sheng, can you check if this is fine? I'll need some time to test this.

Amit

[-- Attachment #2: 0001-KVM-Fix-guest-shared-interrupt-with-in-kernel-irqch.patch --]
[-- Type: application/octet-stream, Size: 8893 bytes --]

>From 920dced6c7ef35959924321ab8d8a9f036fee59c Mon Sep 17 00:00:00 2001
From: Sheng Yang <sheng@linux.intel.com>
Date: Mon, 13 Oct 2008 12:21:35 +0530
Subject: [PATCH 1/1] KVM: Fix guest shared interrupt with in-kernel irqchip

Every call of kvm_set_irq() should offer an irq_source_id, which is
allocated by kvm_request_irq_source_id(). Based on irq_source_id, we
identify the irq source and implement logical OR for shared level
interrupts.

The allocated irq_source_id can be freed by kvm_free_irq_source_id().

Currently, we support at most sizeof(unsigned long) different irq sources.

[Amit: - rebase to kvm.git HEAD
       - move kvm_request_irq_source_id to the update_irq ioctl]

Signed-off-by: Sheng Yang <sheng@linux.intel.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 arch/x86/kvm/i8254.c       |   11 +++++++++--
 arch/x86/kvm/i8254.h       |    1 +
 arch/x86/kvm/irq.h         |    2 ++
 arch/x86/kvm/x86.c         |    6 +++++-
 include/asm-x86/kvm_host.h |    3 +++
 include/linux/kvm_host.h   |    5 ++++-
 virt/kvm/irq_comm.c        |   42 +++++++++++++++++++++++++++++++++++++++---
 virt/kvm/kvm_main.c        |   12 ++++++++----
 8 files changed, 71 insertions(+), 11 deletions(-)

diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
index 6144d3f..da06919 100644
--- a/arch/x86/kvm/i8254.c
+++ b/arch/x86/kvm/i8254.c
@@ -545,6 +545,12 @@ struct kvm_pit *kvm_create_pit(struct kvm *kvm)
 	if (!pit)
 		return NULL;
 
+	mutex_lock(&kvm->lock);
+	pit->irq_source_id = kvm_request_irq_source_id(kvm);
+	mutex_unlock(&kvm->lock);
+	if (pit->irq_source_id < 0)
+		return NULL;
+
 	mutex_init(&pit->pit_state.lock);
 	mutex_lock(&pit->pit_state.lock);
 	spin_lock_init(&pit->pit_state.inject_lock);
@@ -587,6 +593,7 @@ void kvm_free_pit(struct kvm *kvm)
 		mutex_lock(&kvm->arch.vpit->pit_state.lock);
 		timer = &kvm->arch.vpit->pit_state.pit_timer.timer;
 		hrtimer_cancel(timer);
+		kvm_free_irq_source_id(kvm, kvm->arch.vpit->irq_source_id);
 		mutex_unlock(&kvm->arch.vpit->pit_state.lock);
 		kfree(kvm->arch.vpit);
 	}
@@ -598,8 +605,8 @@ static void __inject_pit_timer_intr(struct kvm *kvm)
 	int i;
 
 	mutex_lock(&kvm->lock);
-	kvm_set_irq(kvm, 0, 1);
-	kvm_set_irq(kvm, 0, 0);
+	kvm_set_irq(kvm, kvm->arch.vpit->irq_source_id, 0, 1);
+	kvm_set_irq(kvm, kvm->arch.vpit->irq_source_id, 0, 0);
 	mutex_unlock(&kvm->lock);
 
 	/*
diff --git a/arch/x86/kvm/i8254.h b/arch/x86/kvm/i8254.h
index e436d49..4178022 100644
--- a/arch/x86/kvm/i8254.h
+++ b/arch/x86/kvm/i8254.h
@@ -44,6 +44,7 @@ struct kvm_pit {
 	struct kvm_io_device speaker_dev;
 	struct kvm *kvm;
 	struct kvm_kpit_state pit_state;
+	int irq_source_id;
 };
 
 #define KVM_PIT_BASE_ADDRESS	    0x40
diff --git a/arch/x86/kvm/irq.h b/arch/x86/kvm/irq.h
index 71e37a5..6f0af16 100644
--- a/arch/x86/kvm/irq.h
+++ b/arch/x86/kvm/irq.h
@@ -32,6 +32,8 @@
 
 #define PIC_NUM_PINS 16
 
+#define KVM_USERSPACE_IRQ_SOURCE_ID	0
+
 struct kvm;
 struct kvm_vcpu;
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index dda478e..5d29c50 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1816,7 +1816,8 @@ long kvm_arch_vm_ioctl(struct file *filp,
 			goto out;
 		if (irqchip_in_kernel(kvm)) {
 			mutex_lock(&kvm->lock);
-			kvm_set_irq(kvm, irq_event.irq, irq_event.level);
+			kvm_set_irq(kvm, KVM_USERSPACE_IRQ_SOURCE_ID,
+				    irq_event.irq, irq_event.level);
 			mutex_unlock(&kvm->lock);
 			r = 0;
 		}
@@ -4115,6 +4116,9 @@ struct  kvm *kvm_arch_create_vm(void)
 	INIT_LIST_HEAD(&kvm->arch.active_mmu_pages);
 	INIT_LIST_HEAD(&kvm->arch.assigned_dev_head);
 
+	/* Reserve bit 0 of irq_sources_bitmap for userspace irq source */
+	kvm->arch.irq_sources_bitmap = 1;
+
 	return kvm;
 }
 
diff --git a/include/asm-x86/kvm_host.h b/include/asm-x86/kvm_host.h
index 4b06ca8..f01e92e 100644
--- a/include/asm-x86/kvm_host.h
+++ b/include/asm-x86/kvm_host.h
@@ -368,6 +368,9 @@ struct kvm_arch{
 
 	struct page *ept_identity_pagetable;
 	bool ept_identity_pagetable_done;
+
+	unsigned long irq_sources_bitmap;
+	unsigned long irq_states[KVM_IOAPIC_NUM_PINS];
 };
 
 struct kvm_vm_stat {
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 3833c48..30f575d 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -306,15 +306,18 @@ struct kvm_assigned_dev_kernel {
 	int host_irq;
 	int guest_irq;
 	int irq_requested;
+	int irq_source_id;
 	struct pci_dev *dev;
 	struct kvm *kvm;
 };
-void kvm_set_irq(struct kvm *kvm, int irq, int level);
+void kvm_set_irq(struct kvm *kvm, int irq_source_id, int irq, int level);
 void kvm_notify_acked_irq(struct kvm *kvm, unsigned gsi);
 void kvm_register_irq_ack_notifier(struct kvm *kvm,
 				   struct kvm_irq_ack_notifier *kian);
 void kvm_unregister_irq_ack_notifier(struct kvm *kvm,
 				     struct kvm_irq_ack_notifier *kian);
+int kvm_request_irq_source_id(struct kvm *kvm);
+void kvm_free_irq_source_id(struct kvm *kvm, int irq_source_id);
 
 #ifdef CONFIG_DMAR
 int kvm_iommu_map_pages(struct kvm *kvm, gfn_t base_gfn,
diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
index d0169f5..55ad76e 100644
--- a/virt/kvm/irq_comm.c
+++ b/virt/kvm/irq_comm.c
@@ -25,15 +25,23 @@
 #include "ioapic.h"
 
 /* This should be called with the kvm->lock mutex held */
-void kvm_set_irq(struct kvm *kvm, int irq, int level)
+void kvm_set_irq(struct kvm *kvm, int irq_source_id, int irq, int level)
 {
+	unsigned long *irq_state = (unsigned long *)&kvm->arch.irq_states[irq];
+
+	/* Logical OR for level trig interrupt */
+	if (level)
+		set_bit(irq_source_id, irq_state);
+	else
+		clear_bit(irq_source_id, irq_state);
+
 	/* Not possible to detect if the guest uses the PIC or the
 	 * IOAPIC.  So set the bit in both. The guest will ignore
 	 * writes to the unused one.
 	 */
-	kvm_ioapic_set_irq(kvm->arch.vioapic, irq, level);
+	kvm_ioapic_set_irq(kvm->arch.vioapic, irq, !!(*irq_state));
 #ifdef CONFIG_X86
-	kvm_pic_set_irq(pic_irqchip(kvm), irq, level);
+	kvm_pic_set_irq(pic_irqchip(kvm), irq, !!(*irq_state));
 #endif
 }
 
@@ -58,3 +66,31 @@ void kvm_unregister_irq_ack_notifier(struct kvm *kvm,
 {
 	hlist_del(&kian->link);
 }
+
+/* The caller must hold kvm->lock mutex */
+int kvm_request_irq_source_id(struct kvm *kvm)
+{
+	unsigned long *bitmap = &kvm->arch.irq_sources_bitmap;
+	int irq_source_id = find_first_zero_bit(bitmap,
+				sizeof(kvm->arch.irq_sources_bitmap));
+	if (irq_source_id >= sizeof(kvm->arch.irq_sources_bitmap)) {
+		printk(KERN_WARNING "kvm: exhaust allocatable IRQ sources!\n");
+		irq_source_id = -EFAULT;
+	} else
+		set_bit(irq_source_id, bitmap);
+	return irq_source_id;
+}
+
+void kvm_free_irq_source_id(struct kvm *kvm, int irq_source_id)
+{
+	int i;
+
+	if (irq_source_id <= 0 ||
+	    irq_source_id >= sizeof(kvm->arch.irq_sources_bitmap)) {
+		printk(KERN_ERR "kvm: IRQ source ID out of range!\n");
+		return;
+	}
+	for (i = 0; i < KVM_IOAPIC_NUM_PINS; i++)
+		clear_bit(irq_source_id, &kvm->arch.irq_states[i]);
+	clear_bit(irq_source_id, &kvm->arch.irq_sources_bitmap);
+}
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index cf0ab8e..a87f45e 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -105,14 +105,12 @@ static void kvm_assigned_dev_interrupt_work_handler(struct work_struct *work)
 	 */
 	mutex_lock(&assigned_dev->kvm->lock);
 	kvm_set_irq(assigned_dev->kvm,
+		    assigned_dev->irq_source_id,
 		    assigned_dev->guest_irq, 1);
 	mutex_unlock(&assigned_dev->kvm->lock);
 	kvm_put_kvm(assigned_dev->kvm);
 }
 
-/* FIXME: Implement the OR logic needed to make shared interrupts on
- * this line behave properly
- */
 static irqreturn_t kvm_assigned_dev_intr(int irq, void *dev_id)
 {
 	struct kvm_assigned_dev_kernel *assigned_dev =
@@ -134,7 +132,7 @@ static void kvm_assigned_dev_ack_irq(struct kvm_irq_ack_notifier *kian)
 
 	dev = container_of(kian, struct kvm_assigned_dev_kernel,
 			   ack_notifier);
-	kvm_set_irq(dev->kvm, dev->guest_irq, 0);
+	kvm_set_irq(dev->kvm, dev->irq_source_id, dev->guest_irq, 0);
 	enable_irq(dev->host_irq);
 }
 
@@ -146,6 +144,7 @@ static void kvm_free_assigned_device(struct kvm *kvm,
 		free_irq(assigned_dev->host_irq, (void *)assigned_dev);
 
 	kvm_unregister_irq_ack_notifier(kvm, &assigned_dev->ack_notifier);
+	kvm_free_irq_source_id(kvm, assigned_dev->irq_source_id);
 
 	if (cancel_work_sync(&assigned_dev->interrupt_work))
 		/* We had pending work. That means we will have to take
@@ -215,6 +214,11 @@ static int kvm_vm_ioctl_assign_irq(struct kvm *kvm,
 		match->ack_notifier.gsi = assigned_irq->guest_irq;
 		match->ack_notifier.irq_acked = kvm_assigned_dev_ack_irq;
 		kvm_register_irq_ack_notifier(kvm, &match->ack_notifier);
+		r = kvm_request_irq_source_id(kvm);
+		if (r < 0)
+			goto out_release;
+		else
+			match->irq_source_id = r;
 
 		/* Even though this is PCI, we don't want to use shared
 		 * interrupts. Sharing host devices with guest-assigned devices
-- 
1.5.5.1

RE: [v2] Shared guest irq support

[Zhang, Xiantao]

Hi, Amit
	If your test goes well, I will check it for ia64 side. Thanks!
Xiantao 

-----Original Message-----
From: kvm-owner@vger.kernel.org [mailto:kvm-owner@vger.kernel.org] On
Behalf Of Amit Shah
Sent: Monday, October 13, 2008 3:31 PM
To: Sheng Yang
Cc: avi; kvm; Zhang, Xiantao
Subject: [v2] Shared guest irq support

Sheng, can you check if this is fine? I'll need some time to test this.

Amit

Re: [v2] Shared guest irq support

[Sheng Yang]

On Monday 13 October 2008 15:31:12 Amit Shah wrote:
> Sheng, can you check if this is fine? I'll need some time to test this.
>
I think it's fine.

Thanks!

--
regards
Yang, Sheng

Re: [v2] Shared guest irq support

[Amit Shah]

----- "Xiantao Zhang" <xiantao.zhang@intel.com> wrote:

> Hi, Amit
> 	If your test goes well, I will check it for ia64 side. Thanks!
> Xiantao 

Hello Xiantao,

You can test the build and the working with the previous patch as well (which surely works) -- this patch removes some code that wasn't used.

Amit.

RE: [v2] Shared guest irq support

[Zhang, Xiantao]

Amit Shah wrote:
> ----- "Xiantao Zhang" <xiantao.zhang@intel.com> wrote:
> 
>> Hi, Amit
>> 	If your test goes well, I will check it for ia64 side. Thanks!
>> Xiantao
> 
> Hello Xiantao,
> 
> You can test the build and the working with the previous patch as
> well (which surely works) -- this patch removes some code that wasn't
> used.  

Okay, thanks, I will verify its status on ia64.  :)
Xiantao

Re: [v2] Shared guest irq support

[Amit Shah]

* On Monday 13 Oct 2008 13:01:12 Amit Shah wrote:
> Sheng, can you check if this is fine? I'll need some time to test this.

I tested this; it works fine. Avi, please merge.

Amit.

RE: [v2] Shared guest irq support

[Zhang, Xiantao]

Hi, Amit/Sheng
	See the comments below. 
Xiantao
> Amit Shah wrote:
>> Sheng, can you check if this is fine? I'll need some time to test
>> this. 
>> 
>> Amit
> 
> index 71e37a5..6f0af16 100644
> --- a/arch/x86/kvm/irq.h
> +++ b/arch/x86/kvm/irq.h
> @@ -32,6 +32,8 @@
> 
>  #define PIC_NUM_PINS 16
> 
> +#define KVM_USERSPACE_IRQ_SOURCE_ID	0
> +
>  struct kvm;
>  struct kvm_vcpu

IA64 side also needs this macro, maybe we can put it in ioapic.h or
kvm_host.h ?

> 
> diff --git a/include/asm-x86/kvm_host.h b/include/asm-x86/kvm_host.h
> index 4b06ca8..f01e92e 100644
> --- a/include/asm-x86/kvm_host.h
> +++ b/include/asm-x86/kvm_host.h
> @@ -368,6 +368,9 @@ struct kvm_arch{
> 
>  	struct page *ept_identity_pagetable;
>  	bool ept_identity_pagetable_done;
> +
> +	unsigned long irq_sources_bitmap;
> +	unsigned long irq_states[KVM_IOAPIC_NUM_PINS];
>  };

Asm-ia64/kvm_host.h also needs these two fields to add.  Also do we need
logic to initialize these two fields? 

> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index dda478e..5d29c50 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1816,7 +1816,8 @@ long kvm_arch_vm_ioctl(struct file *filp,
>                         goto out;
>                 if (irqchip_in_kernel(kvm)) {
>                         mutex_lock(&kvm->lock);
> -                       kvm_set_irq(kvm, irq_event.irq,
> irq_event.level); +                       kvm_set_irq(kvm,
> KVM_USERSPACE_IRQ_SOURCE_ID, +                                  
>                         irq_event.irq, irq_event.level);
>                         mutex_unlock(&kvm->lock); r = 0;
>                 }
> @@ -4115,6 +4116,9 @@ struct  kvm *kvm_arch_create_vm(void)
>         INIT_LIST_HEAD(&kvm->arch.active_mmu_pages);
>         INIT_LIST_HEAD(&kvm->arch.assigned_dev_head);
> 
> +       /* Reserve bit 0 of irq_sources_bitmap for userspace irq
> source */ +       kvm->arch.irq_sources_bitmap = 1;
> +
>         return kvm;
>  }

This change should be applied to kvm-ia64.c aslo. 

> diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
> index d0169f5..55ad76e 100644
> --- a/virt/kvm/irq_comm.c
> +++ b/virt/kvm/irq_comm.c
> @@ -25,15 +25,23 @@
>  #include "ioapic.h"
> 
>  /* This should be called with the kvm->lock mutex held */
> -void kvm_set_irq(struct kvm *kvm, int irq, int level)
> +void kvm_set_irq(struct kvm *kvm, int irq_source_id, int irq, int
>  level) {
> +       unsigned long *irq_state = (unsigned long
> *)&kvm->arch.irq_states[irq]; +
> +       /* Logical OR for level trig interrupt */
> +       if (level)
> +               set_bit(irq_source_id, irq_state);
> +       else
> +               clear_bit(irq_source_id, irq_state);
> +
>         /* Not possible to detect if the guest uses the PIC or the
>          * IOAPIC.  So set the bit in both. The guest will ignore
>          * writes to the unused one.
>          */
> -       kvm_ioapic_set_irq(kvm->arch.vioapic, irq, level);
> +       kvm_ioapic_set_irq(kvm->arch.vioapic, irq, !!(*irq_state));
>  #ifdef CONFIG_X86
> -       kvm_pic_set_irq(pic_irqchip(kvm), irq, level);
> +       kvm_pic_set_irq(pic_irqchip(kvm), irq, !!(*irq_state));
>  #endif
>  }
Seems the logic is strange.  Could you help to elaborate it ? Why not
implment the logic same with userspace? 
Thanks
Xiantao

Re: [v2] Shared guest irq support

[Sheng Yang]

On Tuesday 14 October 2008 16:45:19 Zhang, Xiantao wrote:
> Hi, Amit/Sheng
> 	See the comments below.

Hi Amit

Can you help to update the patch? Thanks!

And minor comments below. 

> Xiantao
>
> > Amit Shah wrote:
> >> Sheng, can you check if this is fine? I'll need some time to test
> >> this.
> >>
> >> Amit
> >
> > index 71e37a5..6f0af16 100644
> > --- a/arch/x86/kvm/irq.h
> > +++ b/arch/x86/kvm/irq.h
> > @@ -32,6 +32,8 @@
> >
> >  #define PIC_NUM_PINS 16
> >
> > +#define KVM_USERSPACE_IRQ_SOURCE_ID	0
> > +
> >  struct kvm;
> >  struct kvm_vcpu
>
> IA64 side also needs this macro, maybe we can put it in ioapic.h or
> kvm_host.h ?
>
> > diff --git a/include/asm-x86/kvm_host.h b/include/asm-x86/kvm_host.h
> > index 4b06ca8..f01e92e 100644
> > --- a/include/asm-x86/kvm_host.h
> > +++ b/include/asm-x86/kvm_host.h
> > @@ -368,6 +368,9 @@ struct kvm_arch{
> >
> >  	struct page *ept_identity_pagetable;
> >  	bool ept_identity_pagetable_done;
> > +
> > +	unsigned long irq_sources_bitmap;
> > +	unsigned long irq_states[KVM_IOAPIC_NUM_PINS];
> >  };
>
> Asm-ia64/kvm_host.h also needs these two fields to add.  Also do we need
> logic to initialize these two fields?

Seems no need to initialize explicitly, the kzalloc ensure that they would be 
initialized to 0, and we would reserve 0 in source bitmap for userspace 
later.
>
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index dda478e..5d29c50 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -1816,7 +1816,8 @@ long kvm_arch_vm_ioctl(struct file *filp,
> >                         goto out;
> >                 if (irqchip_in_kernel(kvm)) {
> >                         mutex_lock(&kvm->lock);
> > -                       kvm_set_irq(kvm, irq_event.irq,
> > irq_event.level); +                       kvm_set_irq(kvm,
> > KVM_USERSPACE_IRQ_SOURCE_ID, +
> >                         irq_event.irq, irq_event.level);
> >                         mutex_unlock(&kvm->lock); r = 0;
> >                 }
> > @@ -4115,6 +4116,9 @@ struct  kvm *kvm_arch_create_vm(void)
> >         INIT_LIST_HEAD(&kvm->arch.active_mmu_pages);
> >         INIT_LIST_HEAD(&kvm->arch.assigned_dev_head);
> >
> > +       /* Reserve bit 0 of irq_sources_bitmap for userspace irq
> > source */ +       kvm->arch.irq_sources_bitmap = 1;
> > +

Amit, could you help to use 1 << KVM_USERSPACE_IRQ_SOURCE_ID here, I come to 
think it's more proper. :)

Thanks!
-- 
regards
Yang, Sheng
> >         return kvm;
> >  }
>
> This change should be applied to kvm-ia64.c aslo.
>
> > diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
> > index d0169f5..55ad76e 100644
> > --- a/virt/kvm/irq_comm.c
> > +++ b/virt/kvm/irq_comm.c
> > @@ -25,15 +25,23 @@
> >  #include "ioapic.h"
> >
> >  /* This should be called with the kvm->lock mutex held */
> > -void kvm_set_irq(struct kvm *kvm, int irq, int level)
> > +void kvm_set_irq(struct kvm *kvm, int irq_source_id, int irq, int
> >  level) {
> > +       unsigned long *irq_state = (unsigned long
> > *)&kvm->arch.irq_states[irq]; +
> > +       /* Logical OR for level trig interrupt */
> > +       if (level)
> > +               set_bit(irq_source_id, irq_state);
> > +       else
> > +               clear_bit(irq_source_id, irq_state);
> > +
> >         /* Not possible to detect if the guest uses the PIC or the
> >          * IOAPIC.  So set the bit in both. The guest will ignore
> >          * writes to the unused one.
> >          */
> > -       kvm_ioapic_set_irq(kvm->arch.vioapic, irq, level);
> > +       kvm_ioapic_set_irq(kvm->arch.vioapic, irq, !!(*irq_state));
> >  #ifdef CONFIG_X86
> > -       kvm_pic_set_irq(pic_irqchip(kvm), irq, level);
> > +       kvm_pic_set_irq(pic_irqchip(kvm), irq, !!(*irq_state));
> >  #endif
> >  }
>
> Seems the logic is strange.  Could you help to elaborate it ? Why not
> implment the logic same with userspace?
> Thanks
> Xiantao
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [v2] Shared guest irq support

[Amit Shah]

* On Wednesday 15 Oct 2008 08:06:30 Sheng Yang wrote:
> > > @@ -4115,6 +4116,9 @@ struct  kvm *kvm_arch_create_vm(void)
> > >         INIT_LIST_HEAD(&kvm->arch.active_mmu_pages);
> > >         INIT_LIST_HEAD(&kvm->arch.assigned_dev_head);
> > >
> > > +       /* Reserve bit 0 of irq_sources_bitmap for userspace irq
> > > source */ +       kvm->arch.irq_sources_bitmap = 1;
> > > +
>
> Amit, could you help to use 1 << KVM_USERSPACE_IRQ_SOURCE_ID here, I come
> to think it's more proper. :)

This is fine; if you think there has to be a change, the change could be done 
in the .h:

- #define KVM_USERSPACE_IRQ_SOURCE_ID        0
+ #define KVM_USERSPACE_IRQ_SOURCE_ID        1 << 0

Re: [v2] Shared guest irq support

[Sheng Yang]

On Wednesday 15 October 2008 15:03:39 Amit Shah wrote:
> * On Wednesday 15 Oct 2008 08:06:30 Sheng Yang wrote:
> > > > @@ -4115,6 +4116,9 @@ struct  kvm *kvm_arch_create_vm(void)
> > > >         INIT_LIST_HEAD(&kvm->arch.active_mmu_pages);
> > > >         INIT_LIST_HEAD(&kvm->arch.assigned_dev_head);
> > > >
> > > > +       /* Reserve bit 0 of irq_sources_bitmap for userspace irq
> > > > source */ +       kvm->arch.irq_sources_bitmap = 1;
> > > > +
> >
> > Amit, could you help to use 1 << KVM_USERSPACE_IRQ_SOURCE_ID here, I come
> > to think it's more proper. :)
>
> This is fine; if you think there has to be a change, the change could be
> done in the .h:
>
> - #define KVM_USERSPACE_IRQ_SOURCE_ID        0
> + #define KVM_USERSPACE_IRQ_SOURCE_ID        1 << 0

Well, I think it's a semantics thing... irq_sources_bitmap is a bitmap, it 
can't equal to a source_id... So maybe

set_bit(KVM_USERSPACE_IRQ_SOURCE_ID, &kvm->arch.irq_sources_bitmap);

is the best way here. :)

--
regards
Yang, Sheng

Re: [v2] Shared guest irq support

[Amit Shah]

[-- Attachment #1: Type: text/plain, Size: 3814 bytes --]

* On Tuesday 14 Oct 2008 14:15:19 Zhang, Xiantao wrote:
> Hi, Amit/Sheng
> 	See the comments below.
> Xiantao

Hello Xiantao, can you send a followup patch to this to accomodate these 
changes? We can merge the two patches and then ask Avi to apply so that the 
ia64 build doesn't break unless Avi wants to apply both separately.

Attached patch moves the definition of KVM_USERSPACE_IRQ_SOURCE_ID to a common 
file.

> > Amit Shah wrote:
> >> Sheng, can you check if this is fine? I'll need some time to test
> >> this.
> >>
> >> Amit
> >
> > index 71e37a5..6f0af16 100644
> > --- a/arch/x86/kvm/irq.h
> > +++ b/arch/x86/kvm/irq.h
> > @@ -32,6 +32,8 @@
> >
> >  #define PIC_NUM_PINS 16
> >
> > +#define KVM_USERSPACE_IRQ_SOURCE_ID	0
> > +
> >  struct kvm;
> >  struct kvm_vcpu
>
> IA64 side also needs this macro, maybe we can put it in ioapic.h or
> kvm_host.h ?
>
> > diff --git a/include/asm-x86/kvm_host.h b/include/asm-x86/kvm_host.h
> > index 4b06ca8..f01e92e 100644
> > --- a/include/asm-x86/kvm_host.h
> > +++ b/include/asm-x86/kvm_host.h
> > @@ -368,6 +368,9 @@ struct kvm_arch{
> >
> >  	struct page *ept_identity_pagetable;
> >  	bool ept_identity_pagetable_done;
> > +
> > +	unsigned long irq_sources_bitmap;
> > +	unsigned long irq_states[KVM_IOAPIC_NUM_PINS];
> >  };
>
> Asm-ia64/kvm_host.h also needs these two fields to add.  Also do we need
> logic to initialize these two fields?
>
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index dda478e..5d29c50 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -1816,7 +1816,8 @@ long kvm_arch_vm_ioctl(struct file *filp,
> >                         goto out;
> >                 if (irqchip_in_kernel(kvm)) {
> >                         mutex_lock(&kvm->lock);
> > -                       kvm_set_irq(kvm, irq_event.irq,
> > irq_event.level); +                       kvm_set_irq(kvm,
> > KVM_USERSPACE_IRQ_SOURCE_ID, +
> >                         irq_event.irq, irq_event.level);
> >                         mutex_unlock(&kvm->lock); r = 0;
> >                 }
> > @@ -4115,6 +4116,9 @@ struct  kvm *kvm_arch_create_vm(void)
> >         INIT_LIST_HEAD(&kvm->arch.active_mmu_pages);
> >         INIT_LIST_HEAD(&kvm->arch.assigned_dev_head);
> >
> > +       /* Reserve bit 0 of irq_sources_bitmap for userspace irq
> > source */ +       kvm->arch.irq_sources_bitmap = 1;
> > +
> >         return kvm;
> >  }
>
> This change should be applied to kvm-ia64.c aslo.
>
> > diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
> > index d0169f5..55ad76e 100644
> > --- a/virt/kvm/irq_comm.c
> > +++ b/virt/kvm/irq_comm.c
> > @@ -25,15 +25,23 @@
> >  #include "ioapic.h"
> >
> >  /* This should be called with the kvm->lock mutex held */
> > -void kvm_set_irq(struct kvm *kvm, int irq, int level)
> > +void kvm_set_irq(struct kvm *kvm, int irq_source_id, int irq, int
> >  level) {
> > +       unsigned long *irq_state = (unsigned long
> > *)&kvm->arch.irq_states[irq]; +
> > +       /* Logical OR for level trig interrupt */
> > +       if (level)
> > +               set_bit(irq_source_id, irq_state);
> > +       else
> > +               clear_bit(irq_source_id, irq_state);
> > +
> >         /* Not possible to detect if the guest uses the PIC or the
> >          * IOAPIC.  So set the bit in both. The guest will ignore
> >          * writes to the unused one.
> >          */
> > -       kvm_ioapic_set_irq(kvm->arch.vioapic, irq, level);
> > +       kvm_ioapic_set_irq(kvm->arch.vioapic, irq, !!(*irq_state));
> >  #ifdef CONFIG_X86
> > -       kvm_pic_set_irq(pic_irqchip(kvm), irq, level);
> > +       kvm_pic_set_irq(pic_irqchip(kvm), irq, !!(*irq_state));
> >  #endif
> >  }
>
> Seems the logic is strange.  Could you help to elaborate it ? Why not
> implment the logic same with userspace?
> Thanks
> Xiantao

[-- Attachment #2: 0001-KVM-Fix-guest-shared-interrupt-with-in-kernel-irqch.patch --]
[-- Type: text/x-diff, Size: 9142 bytes --]

From e85ed47cf8eb6d1dc669bc6267bc62c33d4c02d5 Mon Sep 17 00:00:00 2001
From: Sheng Yang <sheng@linux.intel.com>
Date: Mon, 13 Oct 2008 12:21:35 +0530
Subject: [PATCH 1/1] KVM: Fix guest shared interrupt with in-kernel irqchip

Every call of kvm_set_irq() should offer an irq_source_id, which is
allocated by kvm_request_irq_source_id(). Based on irq_source_id, we
identify the irq source and implement logical OR for shared level
interrupts.

The allocated irq_source_id can be freed by kvm_free_irq_source_id().

Currently, we support at most sizeof(unsigned long) different irq sources.

[Amit: - rebase to kvm.git HEAD
       - move definition of KVM_USERSPACE_IRQ_SOURCE_ID to common file
       - move kvm_request_irq_source_id to the update_irq ioctl]

Signed-off-by: Sheng Yang <sheng@linux.intel.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 arch/x86/kvm/i8254.c       |   11 +++++++++--
 arch/x86/kvm/i8254.h       |    1 +
 arch/x86/kvm/x86.c         |    6 +++++-
 include/asm-x86/kvm_host.h |    3 +++
 include/linux/kvm_host.h   |    7 ++++++-
 virt/kvm/irq_comm.c        |   42 +++++++++++++++++++++++++++++++++++++++---
 virt/kvm/kvm_main.c        |   12 ++++++++----
 7 files changed, 71 insertions(+), 11 deletions(-)

diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
index 6144d3f..da06919 100644
--- a/arch/x86/kvm/i8254.c
+++ b/arch/x86/kvm/i8254.c
@@ -545,6 +545,12 @@ struct kvm_pit *kvm_create_pit(struct kvm *kvm)
 	if (!pit)
 		return NULL;
 
+	mutex_lock(&kvm->lock);
+	pit->irq_source_id = kvm_request_irq_source_id(kvm);
+	mutex_unlock(&kvm->lock);
+	if (pit->irq_source_id < 0)
+		return NULL;
+
 	mutex_init(&pit->pit_state.lock);
 	mutex_lock(&pit->pit_state.lock);
 	spin_lock_init(&pit->pit_state.inject_lock);
@@ -587,6 +593,7 @@ void kvm_free_pit(struct kvm *kvm)
 		mutex_lock(&kvm->arch.vpit->pit_state.lock);
 		timer = &kvm->arch.vpit->pit_state.pit_timer.timer;
 		hrtimer_cancel(timer);
+		kvm_free_irq_source_id(kvm, kvm->arch.vpit->irq_source_id);
 		mutex_unlock(&kvm->arch.vpit->pit_state.lock);
 		kfree(kvm->arch.vpit);
 	}
@@ -598,8 +605,8 @@ static void __inject_pit_timer_intr(struct kvm *kvm)
 	int i;
 
 	mutex_lock(&kvm->lock);
-	kvm_set_irq(kvm, 0, 1);
-	kvm_set_irq(kvm, 0, 0);
+	kvm_set_irq(kvm, kvm->arch.vpit->irq_source_id, 0, 1);
+	kvm_set_irq(kvm, kvm->arch.vpit->irq_source_id, 0, 0);
 	mutex_unlock(&kvm->lock);
 
 	/*
diff --git a/arch/x86/kvm/i8254.h b/arch/x86/kvm/i8254.h
index e436d49..4178022 100644
--- a/arch/x86/kvm/i8254.h
+++ b/arch/x86/kvm/i8254.h
@@ -44,6 +44,7 @@ struct kvm_pit {
 	struct kvm_io_device speaker_dev;
 	struct kvm *kvm;
 	struct kvm_kpit_state pit_state;
+	int irq_source_id;
 };
 
 #define KVM_PIT_BASE_ADDRESS	    0x40
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index dda478e..5d29c50 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1816,7 +1816,8 @@ long kvm_arch_vm_ioctl(struct file *filp,
 			goto out;
 		if (irqchip_in_kernel(kvm)) {
 			mutex_lock(&kvm->lock);
-			kvm_set_irq(kvm, irq_event.irq, irq_event.level);
+			kvm_set_irq(kvm, KVM_USERSPACE_IRQ_SOURCE_ID,
+				    irq_event.irq, irq_event.level);
 			mutex_unlock(&kvm->lock);
 			r = 0;
 		}
@@ -4115,6 +4116,9 @@ struct  kvm *kvm_arch_create_vm(void)
 	INIT_LIST_HEAD(&kvm->arch.active_mmu_pages);
 	INIT_LIST_HEAD(&kvm->arch.assigned_dev_head);
 
+	/* Reserve bit 0 of irq_sources_bitmap for userspace irq source */
+	kvm->arch.irq_sources_bitmap = 1;
+
 	return kvm;
 }
 
diff --git a/include/asm-x86/kvm_host.h b/include/asm-x86/kvm_host.h
index 4b06ca8..f01e92e 100644
--- a/include/asm-x86/kvm_host.h
+++ b/include/asm-x86/kvm_host.h
@@ -368,6 +368,9 @@ struct kvm_arch{
 
 	struct page *ept_identity_pagetable;
 	bool ept_identity_pagetable_done;
+
+	unsigned long irq_sources_bitmap;
+	unsigned long irq_states[KVM_IOAPIC_NUM_PINS];
 };
 
 struct kvm_vm_stat {
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 3833c48..bd5b16d 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -37,6 +37,8 @@
 #define KVM_REQ_UNHALT             6
 #define KVM_REQ_MMU_SYNC           7
 
+#define KVM_USERSPACE_IRQ_SOURCE_ID	1 << 0
+
 struct kvm_vcpu;
 extern struct kmem_cache *kvm_vcpu_cache;
 
@@ -306,15 +308,18 @@ struct kvm_assigned_dev_kernel {
 	int host_irq;
 	int guest_irq;
 	int irq_requested;
+	int irq_source_id;
 	struct pci_dev *dev;
 	struct kvm *kvm;
 };
-void kvm_set_irq(struct kvm *kvm, int irq, int level);
+void kvm_set_irq(struct kvm *kvm, int irq_source_id, int irq, int level);
 void kvm_notify_acked_irq(struct kvm *kvm, unsigned gsi);
 void kvm_register_irq_ack_notifier(struct kvm *kvm,
 				   struct kvm_irq_ack_notifier *kian);
 void kvm_unregister_irq_ack_notifier(struct kvm *kvm,
 				     struct kvm_irq_ack_notifier *kian);
+int kvm_request_irq_source_id(struct kvm *kvm);
+void kvm_free_irq_source_id(struct kvm *kvm, int irq_source_id);
 
 #ifdef CONFIG_DMAR
 int kvm_iommu_map_pages(struct kvm *kvm, gfn_t base_gfn,
diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
index d0169f5..55ad76e 100644
--- a/virt/kvm/irq_comm.c
+++ b/virt/kvm/irq_comm.c
@@ -25,15 +25,23 @@
 #include "ioapic.h"
 
 /* This should be called with the kvm->lock mutex held */
-void kvm_set_irq(struct kvm *kvm, int irq, int level)
+void kvm_set_irq(struct kvm *kvm, int irq_source_id, int irq, int level)
 {
+	unsigned long *irq_state = (unsigned long *)&kvm->arch.irq_states[irq];
+
+	/* Logical OR for level trig interrupt */
+	if (level)
+		set_bit(irq_source_id, irq_state);
+	else
+		clear_bit(irq_source_id, irq_state);
+
 	/* Not possible to detect if the guest uses the PIC or the
 	 * IOAPIC.  So set the bit in both. The guest will ignore
 	 * writes to the unused one.
 	 */
-	kvm_ioapic_set_irq(kvm->arch.vioapic, irq, level);
+	kvm_ioapic_set_irq(kvm->arch.vioapic, irq, !!(*irq_state));
 #ifdef CONFIG_X86
-	kvm_pic_set_irq(pic_irqchip(kvm), irq, level);
+	kvm_pic_set_irq(pic_irqchip(kvm), irq, !!(*irq_state));
 #endif
 }
 
@@ -58,3 +66,31 @@ void kvm_unregister_irq_ack_notifier(struct kvm *kvm,
 {
 	hlist_del(&kian->link);
 }
+
+/* The caller must hold kvm->lock mutex */
+int kvm_request_irq_source_id(struct kvm *kvm)
+{
+	unsigned long *bitmap = &kvm->arch.irq_sources_bitmap;
+	int irq_source_id = find_first_zero_bit(bitmap,
+				sizeof(kvm->arch.irq_sources_bitmap));
+	if (irq_source_id >= sizeof(kvm->arch.irq_sources_bitmap)) {
+		printk(KERN_WARNING "kvm: exhaust allocatable IRQ sources!\n");
+		irq_source_id = -EFAULT;
+	} else
+		set_bit(irq_source_id, bitmap);
+	return irq_source_id;
+}
+
+void kvm_free_irq_source_id(struct kvm *kvm, int irq_source_id)
+{
+	int i;
+
+	if (irq_source_id <= 0 ||
+	    irq_source_id >= sizeof(kvm->arch.irq_sources_bitmap)) {
+		printk(KERN_ERR "kvm: IRQ source ID out of range!\n");
+		return;
+	}
+	for (i = 0; i < KVM_IOAPIC_NUM_PINS; i++)
+		clear_bit(irq_source_id, &kvm->arch.irq_states[i]);
+	clear_bit(irq_source_id, &kvm->arch.irq_sources_bitmap);
+}
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index cf0ab8e..a87f45e 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -105,14 +105,12 @@ static void kvm_assigned_dev_interrupt_work_handler(struct work_struct *work)
 	 */
 	mutex_lock(&assigned_dev->kvm->lock);
 	kvm_set_irq(assigned_dev->kvm,
+		    assigned_dev->irq_source_id,
 		    assigned_dev->guest_irq, 1);
 	mutex_unlock(&assigned_dev->kvm->lock);
 	kvm_put_kvm(assigned_dev->kvm);
 }
 
-/* FIXME: Implement the OR logic needed to make shared interrupts on
- * this line behave properly
- */
 static irqreturn_t kvm_assigned_dev_intr(int irq, void *dev_id)
 {
 	struct kvm_assigned_dev_kernel *assigned_dev =
@@ -134,7 +132,7 @@ static void kvm_assigned_dev_ack_irq(struct kvm_irq_ack_notifier *kian)
 
 	dev = container_of(kian, struct kvm_assigned_dev_kernel,
 			   ack_notifier);
-	kvm_set_irq(dev->kvm, dev->guest_irq, 0);
+	kvm_set_irq(dev->kvm, dev->irq_source_id, dev->guest_irq, 0);
 	enable_irq(dev->host_irq);
 }
 
@@ -146,6 +144,7 @@ static void kvm_free_assigned_device(struct kvm *kvm,
 		free_irq(assigned_dev->host_irq, (void *)assigned_dev);
 
 	kvm_unregister_irq_ack_notifier(kvm, &assigned_dev->ack_notifier);
+	kvm_free_irq_source_id(kvm, assigned_dev->irq_source_id);
 
 	if (cancel_work_sync(&assigned_dev->interrupt_work))
 		/* We had pending work. That means we will have to take
@@ -215,6 +214,11 @@ static int kvm_vm_ioctl_assign_irq(struct kvm *kvm,
 		match->ack_notifier.gsi = assigned_irq->guest_irq;
 		match->ack_notifier.irq_acked = kvm_assigned_dev_ack_irq;
 		kvm_register_irq_ack_notifier(kvm, &match->ack_notifier);
+		r = kvm_request_irq_source_id(kvm);
+		if (r < 0)
+			goto out_release;
+		else
+			match->irq_source_id = r;
 
 		/* Even though this is PCI, we don't want to use shared
 		 * interrupts. Sharing host devices with guest-assigned devices
-- 
1.5.4.3

Re: [v2] Shared guest irq support

[Amit Shah]

* On Wednesday 15 Oct 2008 12:41:17 Sheng Yang wrote:
> On Wednesday 15 October 2008 15:03:39 Amit Shah wrote:
> > * On Wednesday 15 Oct 2008 08:06:30 Sheng Yang wrote:
> > > > > @@ -4115,6 +4116,9 @@ struct  kvm *kvm_arch_create_vm(void)
> > > > >         INIT_LIST_HEAD(&kvm->arch.active_mmu_pages);
> > > > >         INIT_LIST_HEAD(&kvm->arch.assigned_dev_head);
> > > > >
> > > > > +       /* Reserve bit 0 of irq_sources_bitmap for userspace irq
> > > > > source */ +       kvm->arch.irq_sources_bitmap = 1;
> > > > > +
> > >
> > > Amit, could you help to use 1 << KVM_USERSPACE_IRQ_SOURCE_ID here, I
> > > come to think it's more proper. :)
> >
> > This is fine; if you think there has to be a change, the change could be
> > done in the .h:
> >
> > - #define KVM_USERSPACE_IRQ_SOURCE_ID        0
> > + #define KVM_USERSPACE_IRQ_SOURCE_ID        1 << 0
>
> Well, I think it's a semantics thing... irq_sources_bitmap is a bitmap, it
> can't equal to a source_id... So maybe
>
> set_bit(KVM_USERSPACE_IRQ_SOURCE_ID, &kvm->arch.irq_sources_bitmap);
>
> is the best way here. :)

OK; I see your point. Can you work with Xiantao and update the patch?

RE: [v2] Shared guest irq support

[Zhang, Xiantao]

Amit Shah wrote:
> * On Wednesday 15 Oct 2008 12:41:17 Sheng Yang wrote:
>> On Wednesday 15 October 2008 15:03:39 Amit Shah wrote:
>>> * On Wednesday 15 Oct 2008 08:06:30 Sheng Yang wrote:
>>>>>> @@ -4115,6 +4116,9 @@ struct  kvm *kvm_arch_create_vm(void)
>>>>>>         INIT_LIST_HEAD(&kvm->arch.active_mmu_pages);
>>>>>>         INIT_LIST_HEAD(&kvm->arch.assigned_dev_head);
>>>>>> 
>>>>>> +       /* Reserve bit 0 of irq_sources_bitmap for userspace irq
>>>>>> source */ +       kvm->arch.irq_sources_bitmap = 1;
>>>>>> +
>>>> 
>>>> Amit, could you help to use 1 << KVM_USERSPACE_IRQ_SOURCE_ID here,
>>>> I come to think it's more proper. :)
>>> 
>>> This is fine; if you think there has to be a change, the change
>>> could be done in the .h: 
>>> 
>>> - #define KVM_USERSPACE_IRQ_SOURCE_ID        0
>>> + #define KVM_USERSPACE_IRQ_SOURCE_ID        1 << 0
>> 
>> Well, I think it's a semantics thing... irq_sources_bitmap is a
>> bitmap, it can't equal to a source_id... So maybe
>> 
>> set_bit(KVM_USERSPACE_IRQ_SOURCE_ID, &kvm->arch.irq_sources_bitmap);
>> 
>> is the best way here. :)
> 
> OK; I see your point. Can you work with Xiantao and update the patch?

Okay, I will update the patch to make it work for ia64. Thanks!
Xiantao

RE: [v2] Shared guest irq support

[Zhang, Xiantao]

[-- Attachment #1: Type: text/plain, Size: 12141 bytes --]

Hi, Avi
     I have made it work on kvm/ia64. Please apply! Thanks!
Xiantao

>From c0d1ad6327c01ba0584922022bef48c971bbf18a Mon Sep 17 00:00:00 2001
From: Xiantao Zhang <xiantao.zhang@intel.com>
Date: Wed, 15 Oct 2008 20:15:06 +0800
Subject: [PATCH] KVM: Fix guest shared interrupt with in-kernel irqchip

Every call of kvm_set_irq() should offer an irq_source_id, which is
allocated by kvm_request_irq_source_id(). Based on irq_source_id, we
identify the irq source and implement logical OR for shared level
interrupts.

The allocated irq_source_id can be freed by kvm_free_irq_source_id().

Currently, we support at most sizeof(unsigned long) different irq
sources.

[Amit: - rebase to kvm.git HEAD
       - move definition of KVM_USERSPACE_IRQ_SOURCE_ID to common file
       - move kvm_request_irq_source_id to the update_irq ioctl]

[Xiantao: - Add kvm/ia64 stuff and make it work for kvm/ia64 guests]

Signed-off-by: Sheng Yang <sheng@linux.intel.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
Signed-off-by: Xiantao Zhang <xiantao.zhang@intel.com>
---
 arch/ia64/include/asm/kvm_host.h |    3 ++
 arch/ia64/kvm/kvm-ia64.c         |    8 ++++--
 arch/x86/kvm/i8254.c             |   11 ++++++++-
 arch/x86/kvm/i8254.h             |    1 +
 arch/x86/kvm/x86.c               |    6 ++++-
 include/asm-x86/kvm_host.h       |    3 ++
 include/linux/kvm_host.h         |    7 +++++-
 virt/kvm/irq_comm.c              |   42
+++++++++++++++++++++++++++++++++++--
 virt/kvm/kvm_main.c              |   12 +++++++---
 9 files changed, 79 insertions(+), 14 deletions(-)

diff --git a/arch/ia64/include/asm/kvm_host.h
b/arch/ia64/include/asm/kvm_host.h
index e98f6f0..15f7703 100644
--- a/arch/ia64/include/asm/kvm_host.h
+++ b/arch/ia64/include/asm/kvm_host.h
@@ -466,6 +466,9 @@ struct kvm_arch {
 	struct list_head assigned_dev_head;
 	struct dmar_domain *intel_iommu_domain;
 	struct hlist_head irq_ack_notifier_list;
+
+	unsigned long irq_sources_bitmap;
+	unsigned long irq_states[KVM_IOAPIC_NUM_PINS];
 };
 
 union cpuid3_t {
diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c
index 1343781..961e6d1 100644
--- a/arch/ia64/kvm/kvm-ia64.c
+++ b/arch/ia64/kvm/kvm-ia64.c
@@ -775,6 +775,9 @@ static void kvm_init_vm(struct kvm *kvm)
 	kvm_build_io_pmt(kvm);
 
 	INIT_LIST_HEAD(&kvm->arch.assigned_dev_head);
+
+	/* Reserve bit 0 of irq_sources_bitmap for userspace irq source
*/
+	kvm->arch.irq_sources_bitmap = 1;
 }
 
 struct  kvm *kvm_arch_create_vm(void)
@@ -938,9 +941,8 @@ long kvm_arch_vm_ioctl(struct file *filp,
 			goto out;
 		if (irqchip_in_kernel(kvm)) {
 			mutex_lock(&kvm->lock);
-			kvm_ioapic_set_irq(kvm->arch.vioapic,
-						irq_event.irq,
-						irq_event.level);
+			kvm_set_irq(kvm, KVM_USERSPACE_IRQ_SOURCE_ID,
+				    irq_event.irq, irq_event.level);
 			mutex_unlock(&kvm->lock);
 			r = 0;
 		}
diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
index 6144d3f..da06919 100644
--- a/arch/x86/kvm/i8254.c
+++ b/arch/x86/kvm/i8254.c
@@ -545,6 +545,12 @@ struct kvm_pit *kvm_create_pit(struct kvm *kvm)
 	if (!pit)
 		return NULL;
 
+	mutex_lock(&kvm->lock);
+	pit->irq_source_id = kvm_request_irq_source_id(kvm);
+	mutex_unlock(&kvm->lock);
+	if (pit->irq_source_id < 0)
+		return NULL;
+
 	mutex_init(&pit->pit_state.lock);
 	mutex_lock(&pit->pit_state.lock);
 	spin_lock_init(&pit->pit_state.inject_lock);
@@ -587,6 +593,7 @@ void kvm_free_pit(struct kvm *kvm)
 		mutex_lock(&kvm->arch.vpit->pit_state.lock);
 		timer = &kvm->arch.vpit->pit_state.pit_timer.timer;
 		hrtimer_cancel(timer);
+		kvm_free_irq_source_id(kvm,
kvm->arch.vpit->irq_source_id);
 		mutex_unlock(&kvm->arch.vpit->pit_state.lock);
 		kfree(kvm->arch.vpit);
 	}
@@ -598,8 +605,8 @@ static void __inject_pit_timer_intr(struct kvm *kvm)
 	int i;
 
 	mutex_lock(&kvm->lock);
-	kvm_set_irq(kvm, 0, 1);
-	kvm_set_irq(kvm, 0, 0);
+	kvm_set_irq(kvm, kvm->arch.vpit->irq_source_id, 0, 1);
+	kvm_set_irq(kvm, kvm->arch.vpit->irq_source_id, 0, 0);
 	mutex_unlock(&kvm->lock);
 
 	/*
diff --git a/arch/x86/kvm/i8254.h b/arch/x86/kvm/i8254.h
index e436d49..4178022 100644
--- a/arch/x86/kvm/i8254.h
+++ b/arch/x86/kvm/i8254.h
@@ -44,6 +44,7 @@ struct kvm_pit {
 	struct kvm_io_device speaker_dev;
 	struct kvm *kvm;
 	struct kvm_kpit_state pit_state;
+	int irq_source_id;
 };
 
 #define KVM_PIT_BASE_ADDRESS	    0x40
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index dda478e..5d29c50 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1816,7 +1816,8 @@ long kvm_arch_vm_ioctl(struct file *filp,
 			goto out;
 		if (irqchip_in_kernel(kvm)) {
 			mutex_lock(&kvm->lock);
-			kvm_set_irq(kvm, irq_event.irq,
irq_event.level);
+			kvm_set_irq(kvm, KVM_USERSPACE_IRQ_SOURCE_ID,
+				    irq_event.irq, irq_event.level);
 			mutex_unlock(&kvm->lock);
 			r = 0;
 		}
@@ -4115,6 +4116,9 @@ struct  kvm *kvm_arch_create_vm(void)
 	INIT_LIST_HEAD(&kvm->arch.active_mmu_pages);
 	INIT_LIST_HEAD(&kvm->arch.assigned_dev_head);
 
+	/* Reserve bit 0 of irq_sources_bitmap for userspace irq source
*/
+	kvm->arch.irq_sources_bitmap = 1;
+
 	return kvm;
 }
 
diff --git a/include/asm-x86/kvm_host.h b/include/asm-x86/kvm_host.h
index 4b06ca8..f01e92e 100644
--- a/include/asm-x86/kvm_host.h
+++ b/include/asm-x86/kvm_host.h
@@ -368,6 +368,9 @@ struct kvm_arch{
 
 	struct page *ept_identity_pagetable;
 	bool ept_identity_pagetable_done;
+
+	unsigned long irq_sources_bitmap;
+	unsigned long irq_states[KVM_IOAPIC_NUM_PINS];
 };
 
 struct kvm_vm_stat {
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 3833c48..bd5b16d 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -37,6 +37,8 @@
 #define KVM_REQ_UNHALT             6
 #define KVM_REQ_MMU_SYNC           7
 
+#define KVM_USERSPACE_IRQ_SOURCE_ID	1 << 0
+
 struct kvm_vcpu;
 extern struct kmem_cache *kvm_vcpu_cache;
 
@@ -306,15 +308,18 @@ struct kvm_assigned_dev_kernel {
 	int host_irq;
 	int guest_irq;
 	int irq_requested;
+	int irq_source_id;
 	struct pci_dev *dev;
 	struct kvm *kvm;
 };
-void kvm_set_irq(struct kvm *kvm, int irq, int level);
+void kvm_set_irq(struct kvm *kvm, int irq_source_id, int irq, int
level);
 void kvm_notify_acked_irq(struct kvm *kvm, unsigned gsi);
 void kvm_register_irq_ack_notifier(struct kvm *kvm,
 				   struct kvm_irq_ack_notifier *kian);
 void kvm_unregister_irq_ack_notifier(struct kvm *kvm,
 				     struct kvm_irq_ack_notifier *kian);
+int kvm_request_irq_source_id(struct kvm *kvm);
+void kvm_free_irq_source_id(struct kvm *kvm, int irq_source_id);
 
 #ifdef CONFIG_DMAR
 int kvm_iommu_map_pages(struct kvm *kvm, gfn_t base_gfn,
diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
index d0169f5..55ad76e 100644
--- a/virt/kvm/irq_comm.c
+++ b/virt/kvm/irq_comm.c
@@ -25,15 +25,23 @@
 #include "ioapic.h"
 
 /* This should be called with the kvm->lock mutex held */
-void kvm_set_irq(struct kvm *kvm, int irq, int level)
+void kvm_set_irq(struct kvm *kvm, int irq_source_id, int irq, int
level)
 {
+	unsigned long *irq_state = (unsigned long
*)&kvm->arch.irq_states[irq];
+
+	/* Logical OR for level trig interrupt */
+	if (level)
+		set_bit(irq_source_id, irq_state);
+	else
+		clear_bit(irq_source_id, irq_state);
+
 	/* Not possible to detect if the guest uses the PIC or the
 	 * IOAPIC.  So set the bit in both. The guest will ignore
 	 * writes to the unused one.
 	 */
-	kvm_ioapic_set_irq(kvm->arch.vioapic, irq, level);
+	kvm_ioapic_set_irq(kvm->arch.vioapic, irq, !!(*irq_state));
 #ifdef CONFIG_X86
-	kvm_pic_set_irq(pic_irqchip(kvm), irq, level);
+	kvm_pic_set_irq(pic_irqchip(kvm), irq, !!(*irq_state));
 #endif
 }
 
@@ -58,3 +66,31 @@ void kvm_unregister_irq_ack_notifier(struct kvm *kvm,
 {
 	hlist_del(&kian->link);
 }
+
+/* The caller must hold kvm->lock mutex */
+int kvm_request_irq_source_id(struct kvm *kvm)
+{
+	unsigned long *bitmap = &kvm->arch.irq_sources_bitmap;
+	int irq_source_id = find_first_zero_bit(bitmap,
+				sizeof(kvm->arch.irq_sources_bitmap));
+	if (irq_source_id >= sizeof(kvm->arch.irq_sources_bitmap)) {
+		printk(KERN_WARNING "kvm: exhaust allocatable IRQ
sources!\n");
+		irq_source_id = -EFAULT;
+	} else
+		set_bit(irq_source_id, bitmap);
+	return irq_source_id;
+}
+
+void kvm_free_irq_source_id(struct kvm *kvm, int irq_source_id)
+{
+	int i;
+
+	if (irq_source_id <= 0 ||
+	    irq_source_id >= sizeof(kvm->arch.irq_sources_bitmap)) {
+		printk(KERN_ERR "kvm: IRQ source ID out of range!\n");
+		return;
+	}
+	for (i = 0; i < KVM_IOAPIC_NUM_PINS; i++)
+		clear_bit(irq_source_id, &kvm->arch.irq_states[i]);
+	clear_bit(irq_source_id, &kvm->arch.irq_sources_bitmap);
+}
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index cf0ab8e..a87f45e 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -105,14 +105,12 @@ static void
kvm_assigned_dev_interrupt_work_handler(struct work_struct *work)
 	 */
 	mutex_lock(&assigned_dev->kvm->lock);
 	kvm_set_irq(assigned_dev->kvm,
+		    assigned_dev->irq_source_id,
 		    assigned_dev->guest_irq, 1);
 	mutex_unlock(&assigned_dev->kvm->lock);
 	kvm_put_kvm(assigned_dev->kvm);
 }
 
-/* FIXME: Implement the OR logic needed to make shared interrupts on
- * this line behave properly
- */
 static irqreturn_t kvm_assigned_dev_intr(int irq, void *dev_id)
 {
 	struct kvm_assigned_dev_kernel *assigned_dev =
@@ -134,7 +132,7 @@ static void kvm_assigned_dev_ack_irq(struct
kvm_irq_ack_notifier *kian)
 
 	dev = container_of(kian, struct kvm_assigned_dev_kernel,
 			   ack_notifier);
-	kvm_set_irq(dev->kvm, dev->guest_irq, 0);
+	kvm_set_irq(dev->kvm, dev->irq_source_id, dev->guest_irq, 0);
 	enable_irq(dev->host_irq);
 }
 
@@ -146,6 +144,7 @@ static void kvm_free_assigned_device(struct kvm
*kvm,
 		free_irq(assigned_dev->host_irq, (void *)assigned_dev);
 
 	kvm_unregister_irq_ack_notifier(kvm,
&assigned_dev->ack_notifier);
+	kvm_free_irq_source_id(kvm, assigned_dev->irq_source_id);
 
 	if (cancel_work_sync(&assigned_dev->interrupt_work))
 		/* We had pending work. That means we will have to take
@@ -215,6 +214,11 @@ static int kvm_vm_ioctl_assign_irq(struct kvm *kvm,
 		match->ack_notifier.gsi = assigned_irq->guest_irq;
 		match->ack_notifier.irq_acked =
kvm_assigned_dev_ack_irq;
 		kvm_register_irq_ack_notifier(kvm,
&match->ack_notifier);
+		r = kvm_request_irq_source_id(kvm);
+		if (r < 0)
+			goto out_release;
+		else
+			match->irq_source_id = r;
 
 		/* Even though this is PCI, we don't want to use shared
 		 * interrupts. Sharing host devices with guest-assigned
devices
-- 
1.5.1




Zhang, Xiantao wrote:
> Amit Shah wrote:
>> * On Wednesday 15 Oct 2008 12:41:17 Sheng Yang wrote:
>>> On Wednesday 15 October 2008 15:03:39 Amit Shah wrote:
>>>> * On Wednesday 15 Oct 2008 08:06:30 Sheng Yang wrote:
>>>>>>> @@ -4115,6 +4116,9 @@ struct  kvm *kvm_arch_create_vm(void)
>>>>>>>         INIT_LIST_HEAD(&kvm->arch.active_mmu_pages);
>>>>>>>         INIT_LIST_HEAD(&kvm->arch.assigned_dev_head);
>>>>>>> 
>>>>>>> +       /* Reserve bit 0 of irq_sources_bitmap for userspace irq
>>>>>>> source */ +       kvm->arch.irq_sources_bitmap = 1;
>>>>>>> +
>>>>> 
>>>>> Amit, could you help to use 1 << KVM_USERSPACE_IRQ_SOURCE_ID here,
>>>>> I come to think it's more proper. :)
>>>> 
>>>> This is fine; if you think there has to be a change, the change
>>>> could be done in the .h: 
>>>> 
>>>> - #define KVM_USERSPACE_IRQ_SOURCE_ID        0
>>>> + #define KVM_USERSPACE_IRQ_SOURCE_ID        1 << 0
>>> 
>>> Well, I think it's a semantics thing... irq_sources_bitmap is a
>>> bitmap, it can't equal to a source_id... So maybe
>>> 
>>> set_bit(KVM_USERSPACE_IRQ_SOURCE_ID, &kvm->arch.irq_sources_bitmap);
>>> 
>>> is the best way here. :)
>> 
>> OK; I see your point. Can you work with Xiantao and update the patch?
> 
> Okay, I will update the patch to make it work for ia64. Thanks!
> Xiantao


[-- Attachment #2: 0003-KVM-Fix-guest-shared-interrupt-with-in-kernel-irqch.patch --]
[-- Type: application/octet-stream, Size: 10393 bytes --]

From c0d1ad6327c01ba0584922022bef48c971bbf18a Mon Sep 17 00:00:00 2001
From: Xiantao Zhang <xiantao.zhang@intel.com>
Date: Wed, 15 Oct 2008 20:15:06 +0800
Subject: [PATCH] KVM: Fix guest shared interrupt with in-kernel irqchip

Every call of kvm_set_irq() should offer an irq_source_id, which is
allocated by kvm_request_irq_source_id(). Based on irq_source_id, we
identify the irq source and implement logical OR for shared level
interrupts.

The allocated irq_source_id can be freed by kvm_free_irq_source_id().

Currently, we support at most sizeof(unsigned long) different irq sources.

[Amit: - rebase to kvm.git HEAD
       - move definition of KVM_USERSPACE_IRQ_SOURCE_ID to common file
       - move kvm_request_irq_source_id to the update_irq ioctl]

[Xiantao: - Add kvm/ia64 stuff and make it work for kvm/ia64 guests]

Signed-off-by: Sheng Yang <sheng@linux.intel.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
Signed-off-by: Xiantao Zhang <xiantao.zhang@intel.com>
---
 arch/ia64/include/asm/kvm_host.h |    3 ++
 arch/ia64/kvm/kvm-ia64.c         |    8 ++++--
 arch/x86/kvm/i8254.c             |   11 ++++++++-
 arch/x86/kvm/i8254.h             |    1 +
 arch/x86/kvm/x86.c               |    6 ++++-
 include/asm-x86/kvm_host.h       |    3 ++
 include/linux/kvm_host.h         |    7 +++++-
 virt/kvm/irq_comm.c              |   42 +++++++++++++++++++++++++++++++++++--
 virt/kvm/kvm_main.c              |   12 +++++++---
 9 files changed, 79 insertions(+), 14 deletions(-)

diff --git a/arch/ia64/include/asm/kvm_host.h b/arch/ia64/include/asm/kvm_host.h
index e98f6f0..15f7703 100644
--- a/arch/ia64/include/asm/kvm_host.h
+++ b/arch/ia64/include/asm/kvm_host.h
@@ -466,6 +466,9 @@ struct kvm_arch {
 	struct list_head assigned_dev_head;
 	struct dmar_domain *intel_iommu_domain;
 	struct hlist_head irq_ack_notifier_list;
+
+	unsigned long irq_sources_bitmap;
+	unsigned long irq_states[KVM_IOAPIC_NUM_PINS];
 };
 
 union cpuid3_t {
diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c
index 1343781..961e6d1 100644
--- a/arch/ia64/kvm/kvm-ia64.c
+++ b/arch/ia64/kvm/kvm-ia64.c
@@ -775,6 +775,9 @@ static void kvm_init_vm(struct kvm *kvm)
 	kvm_build_io_pmt(kvm);
 
 	INIT_LIST_HEAD(&kvm->arch.assigned_dev_head);
+
+	/* Reserve bit 0 of irq_sources_bitmap for userspace irq source */
+	kvm->arch.irq_sources_bitmap = 1;
 }
 
 struct  kvm *kvm_arch_create_vm(void)
@@ -938,9 +941,8 @@ long kvm_arch_vm_ioctl(struct file *filp,
 			goto out;
 		if (irqchip_in_kernel(kvm)) {
 			mutex_lock(&kvm->lock);
-			kvm_ioapic_set_irq(kvm->arch.vioapic,
-						irq_event.irq,
-						irq_event.level);
+			kvm_set_irq(kvm, KVM_USERSPACE_IRQ_SOURCE_ID,
+				    irq_event.irq, irq_event.level);
 			mutex_unlock(&kvm->lock);
 			r = 0;
 		}
diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
index 6144d3f..da06919 100644
--- a/arch/x86/kvm/i8254.c
+++ b/arch/x86/kvm/i8254.c
@@ -545,6 +545,12 @@ struct kvm_pit *kvm_create_pit(struct kvm *kvm)
 	if (!pit)
 		return NULL;
 
+	mutex_lock(&kvm->lock);
+	pit->irq_source_id = kvm_request_irq_source_id(kvm);
+	mutex_unlock(&kvm->lock);
+	if (pit->irq_source_id < 0)
+		return NULL;
+
 	mutex_init(&pit->pit_state.lock);
 	mutex_lock(&pit->pit_state.lock);
 	spin_lock_init(&pit->pit_state.inject_lock);
@@ -587,6 +593,7 @@ void kvm_free_pit(struct kvm *kvm)
 		mutex_lock(&kvm->arch.vpit->pit_state.lock);
 		timer = &kvm->arch.vpit->pit_state.pit_timer.timer;
 		hrtimer_cancel(timer);
+		kvm_free_irq_source_id(kvm, kvm->arch.vpit->irq_source_id);
 		mutex_unlock(&kvm->arch.vpit->pit_state.lock);
 		kfree(kvm->arch.vpit);
 	}
@@ -598,8 +605,8 @@ static void __inject_pit_timer_intr(struct kvm *kvm)
 	int i;
 
 	mutex_lock(&kvm->lock);
-	kvm_set_irq(kvm, 0, 1);
-	kvm_set_irq(kvm, 0, 0);
+	kvm_set_irq(kvm, kvm->arch.vpit->irq_source_id, 0, 1);
+	kvm_set_irq(kvm, kvm->arch.vpit->irq_source_id, 0, 0);
 	mutex_unlock(&kvm->lock);
 
 	/*
diff --git a/arch/x86/kvm/i8254.h b/arch/x86/kvm/i8254.h
index e436d49..4178022 100644
--- a/arch/x86/kvm/i8254.h
+++ b/arch/x86/kvm/i8254.h
@@ -44,6 +44,7 @@ struct kvm_pit {
 	struct kvm_io_device speaker_dev;
 	struct kvm *kvm;
 	struct kvm_kpit_state pit_state;
+	int irq_source_id;
 };
 
 #define KVM_PIT_BASE_ADDRESS	    0x40
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index dda478e..5d29c50 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1816,7 +1816,8 @@ long kvm_arch_vm_ioctl(struct file *filp,
 			goto out;
 		if (irqchip_in_kernel(kvm)) {
 			mutex_lock(&kvm->lock);
-			kvm_set_irq(kvm, irq_event.irq, irq_event.level);
+			kvm_set_irq(kvm, KVM_USERSPACE_IRQ_SOURCE_ID,
+				    irq_event.irq, irq_event.level);
 			mutex_unlock(&kvm->lock);
 			r = 0;
 		}
@@ -4115,6 +4116,9 @@ struct  kvm *kvm_arch_create_vm(void)
 	INIT_LIST_HEAD(&kvm->arch.active_mmu_pages);
 	INIT_LIST_HEAD(&kvm->arch.assigned_dev_head);
 
+	/* Reserve bit 0 of irq_sources_bitmap for userspace irq source */
+	kvm->arch.irq_sources_bitmap = 1;
+
 	return kvm;
 }
 
diff --git a/include/asm-x86/kvm_host.h b/include/asm-x86/kvm_host.h
index 4b06ca8..f01e92e 100644
--- a/include/asm-x86/kvm_host.h
+++ b/include/asm-x86/kvm_host.h
@@ -368,6 +368,9 @@ struct kvm_arch{
 
 	struct page *ept_identity_pagetable;
 	bool ept_identity_pagetable_done;
+
+	unsigned long irq_sources_bitmap;
+	unsigned long irq_states[KVM_IOAPIC_NUM_PINS];
 };
 
 struct kvm_vm_stat {
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 3833c48..bd5b16d 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -37,6 +37,8 @@
 #define KVM_REQ_UNHALT             6
 #define KVM_REQ_MMU_SYNC           7
 
+#define KVM_USERSPACE_IRQ_SOURCE_ID	1 << 0
+
 struct kvm_vcpu;
 extern struct kmem_cache *kvm_vcpu_cache;
 
@@ -306,15 +308,18 @@ struct kvm_assigned_dev_kernel {
 	int host_irq;
 	int guest_irq;
 	int irq_requested;
+	int irq_source_id;
 	struct pci_dev *dev;
 	struct kvm *kvm;
 };
-void kvm_set_irq(struct kvm *kvm, int irq, int level);
+void kvm_set_irq(struct kvm *kvm, int irq_source_id, int irq, int level);
 void kvm_notify_acked_irq(struct kvm *kvm, unsigned gsi);
 void kvm_register_irq_ack_notifier(struct kvm *kvm,
 				   struct kvm_irq_ack_notifier *kian);
 void kvm_unregister_irq_ack_notifier(struct kvm *kvm,
 				     struct kvm_irq_ack_notifier *kian);
+int kvm_request_irq_source_id(struct kvm *kvm);
+void kvm_free_irq_source_id(struct kvm *kvm, int irq_source_id);
 
 #ifdef CONFIG_DMAR
 int kvm_iommu_map_pages(struct kvm *kvm, gfn_t base_gfn,
diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
index d0169f5..55ad76e 100644
--- a/virt/kvm/irq_comm.c
+++ b/virt/kvm/irq_comm.c
@@ -25,15 +25,23 @@
 #include "ioapic.h"
 
 /* This should be called with the kvm->lock mutex held */
-void kvm_set_irq(struct kvm *kvm, int irq, int level)
+void kvm_set_irq(struct kvm *kvm, int irq_source_id, int irq, int level)
 {
+	unsigned long *irq_state = (unsigned long *)&kvm->arch.irq_states[irq];
+
+	/* Logical OR for level trig interrupt */
+	if (level)
+		set_bit(irq_source_id, irq_state);
+	else
+		clear_bit(irq_source_id, irq_state);
+
 	/* Not possible to detect if the guest uses the PIC or the
 	 * IOAPIC.  So set the bit in both. The guest will ignore
 	 * writes to the unused one.
 	 */
-	kvm_ioapic_set_irq(kvm->arch.vioapic, irq, level);
+	kvm_ioapic_set_irq(kvm->arch.vioapic, irq, !!(*irq_state));
 #ifdef CONFIG_X86
-	kvm_pic_set_irq(pic_irqchip(kvm), irq, level);
+	kvm_pic_set_irq(pic_irqchip(kvm), irq, !!(*irq_state));
 #endif
 }
 
@@ -58,3 +66,31 @@ void kvm_unregister_irq_ack_notifier(struct kvm *kvm,
 {
 	hlist_del(&kian->link);
 }
+
+/* The caller must hold kvm->lock mutex */
+int kvm_request_irq_source_id(struct kvm *kvm)
+{
+	unsigned long *bitmap = &kvm->arch.irq_sources_bitmap;
+	int irq_source_id = find_first_zero_bit(bitmap,
+				sizeof(kvm->arch.irq_sources_bitmap));
+	if (irq_source_id >= sizeof(kvm->arch.irq_sources_bitmap)) {
+		printk(KERN_WARNING "kvm: exhaust allocatable IRQ sources!\n");
+		irq_source_id = -EFAULT;
+	} else
+		set_bit(irq_source_id, bitmap);
+	return irq_source_id;
+}
+
+void kvm_free_irq_source_id(struct kvm *kvm, int irq_source_id)
+{
+	int i;
+
+	if (irq_source_id <= 0 ||
+	    irq_source_id >= sizeof(kvm->arch.irq_sources_bitmap)) {
+		printk(KERN_ERR "kvm: IRQ source ID out of range!\n");
+		return;
+	}
+	for (i = 0; i < KVM_IOAPIC_NUM_PINS; i++)
+		clear_bit(irq_source_id, &kvm->arch.irq_states[i]);
+	clear_bit(irq_source_id, &kvm->arch.irq_sources_bitmap);
+}
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index cf0ab8e..a87f45e 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -105,14 +105,12 @@ static void kvm_assigned_dev_interrupt_work_handler(struct work_struct *work)
 	 */
 	mutex_lock(&assigned_dev->kvm->lock);
 	kvm_set_irq(assigned_dev->kvm,
+		    assigned_dev->irq_source_id,
 		    assigned_dev->guest_irq, 1);
 	mutex_unlock(&assigned_dev->kvm->lock);
 	kvm_put_kvm(assigned_dev->kvm);
 }
 
-/* FIXME: Implement the OR logic needed to make shared interrupts on
- * this line behave properly
- */
 static irqreturn_t kvm_assigned_dev_intr(int irq, void *dev_id)
 {
 	struct kvm_assigned_dev_kernel *assigned_dev =
@@ -134,7 +132,7 @@ static void kvm_assigned_dev_ack_irq(struct kvm_irq_ack_notifier *kian)
 
 	dev = container_of(kian, struct kvm_assigned_dev_kernel,
 			   ack_notifier);
-	kvm_set_irq(dev->kvm, dev->guest_irq, 0);
+	kvm_set_irq(dev->kvm, dev->irq_source_id, dev->guest_irq, 0);
 	enable_irq(dev->host_irq);
 }
 
@@ -146,6 +144,7 @@ static void kvm_free_assigned_device(struct kvm *kvm,
 		free_irq(assigned_dev->host_irq, (void *)assigned_dev);
 
 	kvm_unregister_irq_ack_notifier(kvm, &assigned_dev->ack_notifier);
+	kvm_free_irq_source_id(kvm, assigned_dev->irq_source_id);
 
 	if (cancel_work_sync(&assigned_dev->interrupt_work))
 		/* We had pending work. That means we will have to take
@@ -215,6 +214,11 @@ static int kvm_vm_ioctl_assign_irq(struct kvm *kvm,
 		match->ack_notifier.gsi = assigned_irq->guest_irq;
 		match->ack_notifier.irq_acked = kvm_assigned_dev_ack_irq;
 		kvm_register_irq_ack_notifier(kvm, &match->ack_notifier);
+		r = kvm_request_irq_source_id(kvm);
+		if (r < 0)
+			goto out_release;
+		else
+			match->irq_source_id = r;
 
 		/* Even though this is PCI, we don't want to use shared
 		 * interrupts. Sharing host devices with guest-assigned devices
-- 
1.5.1

RE: [v2] Shared guest irq support

[Zhang, Xiantao]

[-- Attachment #1: Type: text/plain, Size: 23200 bytes --]

Forget to modify the from item generated automatically by git.  Modified
to Sheng Yang <sheng@linux.intel.com>  :)
Xiantao

>From c0d1ad6327c01ba0584922022bef48c971bbf18a Mon Sep 17 00:00:00 2001
From: Sheng Yang <sheng@linux.intel.com>
Date: Wed, 15 Oct 2008 20:15:06 +0800
Subject: [PATCH] KVM: Fix guest shared interrupt with in-kernel irqchip

Every call of kvm_set_irq() should offer an irq_source_id, which is
allocated by kvm_request_irq_source_id(). Based on irq_source_id, we
identify the irq source and implement logical OR for shared level
interrupts.

The allocated irq_source_id can be freed by kvm_free_irq_source_id().

Currently, we support at most sizeof(unsigned long) different irq
sources.

[Amit: - rebase to kvm.git HEAD
       - move definition of KVM_USERSPACE_IRQ_SOURCE_ID to common file
       - move kvm_request_irq_source_id to the update_irq ioctl]

[Xiantao: - Add kvm/ia64 stuff and make it work for kvm/ia64 guests]

Signed-off-by: Sheng Yang <sheng@linux.intel.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
Signed-off-by: Xiantao Zhang <xiantao.zhang@intel.com>
---
 arch/ia64/include/asm/kvm_host.h |    3 ++
 arch/ia64/kvm/kvm-ia64.c         |    8 ++++--
 arch/x86/kvm/i8254.c             |   11 ++++++++-
 arch/x86/kvm/i8254.h             |    1 +
 arch/x86/kvm/x86.c               |    6 ++++-
 include/asm-x86/kvm_host.h       |    3 ++
 include/linux/kvm_host.h         |    7 +++++-
 virt/kvm/irq_comm.c              |   42
+++++++++++++++++++++++++++++++++++--
 virt/kvm/kvm_main.c              |   12 +++++++---
 9 files changed, 79 insertions(+), 14 deletions(-)

diff --git a/arch/ia64/include/asm/kvm_host.h
b/arch/ia64/include/asm/kvm_host.h
index e98f6f0..15f7703 100644
--- a/arch/ia64/include/asm/kvm_host.h
+++ b/arch/ia64/include/asm/kvm_host.h
@@ -466,6 +466,9 @@ struct kvm_arch {
 	struct list_head assigned_dev_head;
 	struct dmar_domain *intel_iommu_domain;
 	struct hlist_head irq_ack_notifier_list;
+
+	unsigned long irq_sources_bitmap;
+	unsigned long irq_states[KVM_IOAPIC_NUM_PINS];
 };
 
 union cpuid3_t {
diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c
index 1343781..961e6d1 100644
--- a/arch/ia64/kvm/kvm-ia64.c
+++ b/arch/ia64/kvm/kvm-ia64.c
@@ -775,6 +775,9 @@ static void kvm_init_vm(struct kvm *kvm)
 	kvm_build_io_pmt(kvm);
 
 	INIT_LIST_HEAD(&kvm->arch.assigned_dev_head);
+
+	/* Reserve bit 0 of irq_sources_bitmap for userspace irq source
*/
+	kvm->arch.irq_sources_bitmap = 1;
 }
 
 struct  kvm *kvm_arch_create_vm(void)
@@ -938,9 +941,8 @@ long kvm_arch_vm_ioctl(struct file *filp,
 			goto out;
 		if (irqchip_in_kernel(kvm)) {
 			mutex_lock(&kvm->lock);
-			kvm_ioapic_set_irq(kvm->arch.vioapic,
-						irq_event.irq,
-						irq_event.level);
+			kvm_set_irq(kvm, KVM_USERSPACE_IRQ_SOURCE_ID,
+				    irq_event.irq, irq_event.level);
 			mutex_unlock(&kvm->lock);
 			r = 0;
 		}
diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
index 6144d3f..da06919 100644
--- a/arch/x86/kvm/i8254.c
+++ b/arch/x86/kvm/i8254.c
@@ -545,6 +545,12 @@ struct kvm_pit *kvm_create_pit(struct kvm *kvm)
 	if (!pit)
 		return NULL;
 
+	mutex_lock(&kvm->lock);
+	pit->irq_source_id = kvm_request_irq_source_id(kvm);
+	mutex_unlock(&kvm->lock);
+	if (pit->irq_source_id < 0)
+		return NULL;
+
 	mutex_init(&pit->pit_state.lock);
 	mutex_lock(&pit->pit_state.lock);
 	spin_lock_init(&pit->pit_state.inject_lock);
@@ -587,6 +593,7 @@ void kvm_free_pit(struct kvm *kvm)
 		mutex_lock(&kvm->arch.vpit->pit_state.lock);
 		timer = &kvm->arch.vpit->pit_state.pit_timer.timer;
 		hrtimer_cancel(timer);
+		kvm_free_irq_source_id(kvm,
kvm->arch.vpit->irq_source_id);
 		mutex_unlock(&kvm->arch.vpit->pit_state.lock);
 		kfree(kvm->arch.vpit);
 	}
@@ -598,8 +605,8 @@ static void __inject_pit_timer_intr(struct kvm *kvm)
 	int i;
 
 	mutex_lock(&kvm->lock);
-	kvm_set_irq(kvm, 0, 1);
-	kvm_set_irq(kvm, 0, 0);
+	kvm_set_irq(kvm, kvm->arch.vpit->irq_source_id, 0, 1);
+	kvm_set_irq(kvm, kvm->arch.vpit->irq_source_id, 0, 0);
 	mutex_unlock(&kvm->lock);
 
 	/*
diff --git a/arch/x86/kvm/i8254.h b/arch/x86/kvm/i8254.h
index e436d49..4178022 100644
--- a/arch/x86/kvm/i8254.h
+++ b/arch/x86/kvm/i8254.h
@@ -44,6 +44,7 @@ struct kvm_pit {
 	struct kvm_io_device speaker_dev;
 	struct kvm *kvm;
 	struct kvm_kpit_state pit_state;
+	int irq_source_id;
 };
 
 #define KVM_PIT_BASE_ADDRESS	    0x40
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index dda478e..5d29c50 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1816,7 +1816,8 @@ long kvm_arch_vm_ioctl(struct file *filp,
 			goto out;
 		if (irqchip_in_kernel(kvm)) {
 			mutex_lock(&kvm->lock);
-			kvm_set_irq(kvm, irq_event.irq,
irq_event.level);
+			kvm_set_irq(kvm, KVM_USERSPACE_IRQ_SOURCE_ID,
+				    irq_event.irq, irq_event.level);
 			mutex_unlock(&kvm->lock);
 			r = 0;
 		}
@@ -4115,6 +4116,9 @@ struct  kvm *kvm_arch_create_vm(void)
 	INIT_LIST_HEAD(&kvm->arch.active_mmu_pages);
 	INIT_LIST_HEAD(&kvm->arch.assigned_dev_head);
 
+	/* Reserve bit 0 of irq_sources_bitmap for userspace irq source
*/
+	kvm->arch.irq_sources_bitmap = 1;
+
 	return kvm;
 }
 
diff --git a/include/asm-x86/kvm_host.h b/include/asm-x86/kvm_host.h
index 4b06ca8..f01e92e 100644
--- a/include/asm-x86/kvm_host.h
+++ b/include/asm-x86/kvm_host.h
@@ -368,6 +368,9 @@ struct kvm_arch{
 
 	struct page *ept_identity_pagetable;
 	bool ept_identity_pagetable_done;
+
+	unsigned long irq_sources_bitmap;
+	unsigned long irq_states[KVM_IOAPIC_NUM_PINS];
 };
 
 struct kvm_vm_stat {
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 3833c48..bd5b16d 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -37,6 +37,8 @@
 #define KVM_REQ_UNHALT             6
 #define KVM_REQ_MMU_SYNC           7
 
+#define KVM_USERSPACE_IRQ_SOURCE_ID	1 << 0
+
 struct kvm_vcpu;
 extern struct kmem_cache *kvm_vcpu_cache;
 
@@ -306,15 +308,18 @@ struct kvm_assigned_dev_kernel {
 	int host_irq;
 	int guest_irq;
 	int irq_requested;
+	int irq_source_id;
 	struct pci_dev *dev;
 	struct kvm *kvm;
 };
-void kvm_set_irq(struct kvm *kvm, int irq, int level);
+void kvm_set_irq(struct kvm *kvm, int irq_source_id, int irq, int
level);
 void kvm_notify_acked_irq(struct kvm *kvm, unsigned gsi);
 void kvm_register_irq_ack_notifier(struct kvm *kvm,
 				   struct kvm_irq_ack_notifier *kian);
 void kvm_unregister_irq_ack_notifier(struct kvm *kvm,
 				     struct kvm_irq_ack_notifier *kian);
+int kvm_request_irq_source_id(struct kvm *kvm);
+void kvm_free_irq_source_id(struct kvm *kvm, int irq_source_id);
 
 #ifdef CONFIG_DMAR
 int kvm_iommu_map_pages(struct kvm *kvm, gfn_t base_gfn,
diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
index d0169f5..55ad76e 100644
--- a/virt/kvm/irq_comm.c
+++ b/virt/kvm/irq_comm.c
@@ -25,15 +25,23 @@
 #include "ioapic.h"
 
 /* This should be called with the kvm->lock mutex held */
-void kvm_set_irq(struct kvm *kvm, int irq, int level)
+void kvm_set_irq(struct kvm *kvm, int irq_source_id, int irq, int
level)
 {
+	unsigned long *irq_state = (unsigned long
*)&kvm->arch.irq_states[irq];
+
+	/* Logical OR for level trig interrupt */
+	if (level)
+		set_bit(irq_source_id, irq_state);
+	else
+		clear_bit(irq_source_id, irq_state);
+
 	/* Not possible to detect if the guest uses the PIC or the
 	 * IOAPIC.  So set the bit in both. The guest will ignore
 	 * writes to the unused one.
 	 */
-	kvm_ioapic_set_irq(kvm->arch.vioapic, irq, level);
+	kvm_ioapic_set_irq(kvm->arch.vioapic, irq, !!(*irq_state));
 #ifdef CONFIG_X86
-	kvm_pic_set_irq(pic_irqchip(kvm), irq, level);
+	kvm_pic_set_irq(pic_irqchip(kvm), irq, !!(*irq_state));
 #endif
 }
 
@@ -58,3 +66,31 @@ void kvm_unregister_irq_ack_notifier(struct kvm *kvm,
 {
 	hlist_del(&kian->link);
 }
+
+/* The caller must hold kvm->lock mutex */
+int kvm_request_irq_source_id(struct kvm *kvm)
+{
+	unsigned long *bitmap = &kvm->arch.irq_sources_bitmap;
+	int irq_source_id = find_first_zero_bit(bitmap,
+				sizeof(kvm->arch.irq_sources_bitmap));
+	if (irq_source_id >= sizeof(kvm->arch.irq_sources_bitmap)) {
+		printk(KERN_WARNING "kvm: exhaust allocatable IRQ
sources!\n");
+		irq_source_id = -EFAULT;
+	} else
+		set_bit(irq_source_id, bitmap);
+	return irq_source_id;
+}
+
+void kvm_free_irq_source_id(struct kvm *kvm, int irq_source_id)
+{
+	int i;
+
+	if (irq_source_id <= 0 ||
+	    irq_source_id >= sizeof(kvm->arch.irq_sources_bitmap)) {
+		printk(KERN_ERR "kvm: IRQ source ID out of range!\n");
+		return;
+	}
+	for (i = 0; i < KVM_IOAPIC_NUM_PINS; i++)
+		clear_bit(irq_source_id, &kvm->arch.irq_states[i]);
+	clear_bit(irq_source_id, &kvm->arch.irq_sources_bitmap);
+}
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index cf0ab8e..a87f45e 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -105,14 +105,12 @@ static void
kvm_assigned_dev_interrupt_work_handler(struct work_struct *work)
 	 */
 	mutex_lock(&assigned_dev->kvm->lock);
 	kvm_set_irq(assigned_dev->kvm,
+		    assigned_dev->irq_source_id,
 		    assigned_dev->guest_irq, 1);
 	mutex_unlock(&assigned_dev->kvm->lock);
 	kvm_put_kvm(assigned_dev->kvm);
 }
 
-/* FIXME: Implement the OR logic needed to make shared interrupts on
- * this line behave properly
- */
 static irqreturn_t kvm_assigned_dev_intr(int irq, void *dev_id)
 {
 	struct kvm_assigned_dev_kernel *assigned_dev =
@@ -134,7 +132,7 @@ static void kvm_assigned_dev_ack_irq(struct
kvm_irq_ack_notifier *kian)
 
 	dev = container_of(kian, struct kvm_assigned_dev_kernel,
 			   ack_notifier);
-	kvm_set_irq(dev->kvm, dev->guest_irq, 0);
+	kvm_set_irq(dev->kvm, dev->irq_source_id, dev->guest_irq, 0);
 	enable_irq(dev->host_irq);
 }
 
@@ -146,6 +144,7 @@ static void kvm_free_assigned_device(struct kvm
*kvm,
 		free_irq(assigned_dev->host_irq, (void *)assigned_dev);
 
 	kvm_unregister_irq_ack_notifier(kvm,
&assigned_dev->ack_notifier);
+	kvm_free_irq_source_id(kvm, assigned_dev->irq_source_id);
 
 	if (cancel_work_sync(&assigned_dev->interrupt_work))
 		/* We had pending work. That means we will have to take
@@ -215,6 +214,11 @@ static int kvm_vm_ioctl_assign_irq(struct kvm *kvm,
 		match->ack_notifier.gsi = assigned_irq->guest_irq;
 		match->ack_notifier.irq_acked =
kvm_assigned_dev_ack_irq;
 		kvm_register_irq_ack_notifier(kvm,
&match->ack_notifier);
+		r = kvm_request_irq_source_id(kvm);
+		if (r < 0)
+			goto out_release;
+		else
+			match->irq_source_id = r;
 
 		/* Even though this is PCI, we don't want to use shared
 		 * interrupts. Sharing host devices with guest-assigned
devices
-- 
1.5.1


-----Original Message-----
From: Zhang, Xiantao 
Sent: Wednesday, October 15, 2008 9:44 PM
To: Zhang, Xiantao; Amit Shah; Sheng Yang
Cc: kvm@vger.kernel.org; avi
Subject: RE: [v2] Shared guest irq support

Hi, Avi
     I have made it work on kvm/ia64. Please apply! Thanks!
Xiantao

>From c0d1ad6327c01ba0584922022bef48c971bbf18a Mon Sep 17 00:00:00 2001
From: Xiantao Zhang <xiantao.zhang@intel.com>
Date: Wed, 15 Oct 2008 20:15:06 +0800
Subject: [PATCH] KVM: Fix guest shared interrupt with in-kernel irqchip

Every call of kvm_set_irq() should offer an irq_source_id, which is
allocated by kvm_request_irq_source_id(). Based on irq_source_id, we
identify the irq source and implement logical OR for shared level
interrupts.

The allocated irq_source_id can be freed by kvm_free_irq_source_id().

Currently, we support at most sizeof(unsigned long) different irq
sources.

[Amit: - rebase to kvm.git HEAD
       - move definition of KVM_USERSPACE_IRQ_SOURCE_ID to common file
       - move kvm_request_irq_source_id to the update_irq ioctl]

[Xiantao: - Add kvm/ia64 stuff and make it work for kvm/ia64 guests]

Signed-off-by: Sheng Yang <sheng@linux.intel.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
Signed-off-by: Xiantao Zhang <xiantao.zhang@intel.com>
---
 arch/ia64/include/asm/kvm_host.h |    3 ++
 arch/ia64/kvm/kvm-ia64.c         |    8 ++++--
 arch/x86/kvm/i8254.c             |   11 ++++++++-
 arch/x86/kvm/i8254.h             |    1 +
 arch/x86/kvm/x86.c               |    6 ++++-
 include/asm-x86/kvm_host.h       |    3 ++
 include/linux/kvm_host.h         |    7 +++++-
 virt/kvm/irq_comm.c              |   42
+++++++++++++++++++++++++++++++++++--
 virt/kvm/kvm_main.c              |   12 +++++++---
 9 files changed, 79 insertions(+), 14 deletions(-)

diff --git a/arch/ia64/include/asm/kvm_host.h
b/arch/ia64/include/asm/kvm_host.h
index e98f6f0..15f7703 100644
--- a/arch/ia64/include/asm/kvm_host.h
+++ b/arch/ia64/include/asm/kvm_host.h
@@ -466,6 +466,9 @@ struct kvm_arch {
 	struct list_head assigned_dev_head;
 	struct dmar_domain *intel_iommu_domain;
 	struct hlist_head irq_ack_notifier_list;
+
+	unsigned long irq_sources_bitmap;
+	unsigned long irq_states[KVM_IOAPIC_NUM_PINS];
 };
 
 union cpuid3_t {
diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c
index 1343781..961e6d1 100644
--- a/arch/ia64/kvm/kvm-ia64.c
+++ b/arch/ia64/kvm/kvm-ia64.c
@@ -775,6 +775,9 @@ static void kvm_init_vm(struct kvm *kvm)
 	kvm_build_io_pmt(kvm);
 
 	INIT_LIST_HEAD(&kvm->arch.assigned_dev_head);
+
+	/* Reserve bit 0 of irq_sources_bitmap for userspace irq source
*/
+	kvm->arch.irq_sources_bitmap = 1;
 }
 
 struct  kvm *kvm_arch_create_vm(void)
@@ -938,9 +941,8 @@ long kvm_arch_vm_ioctl(struct file *filp,
 			goto out;
 		if (irqchip_in_kernel(kvm)) {
 			mutex_lock(&kvm->lock);
-			kvm_ioapic_set_irq(kvm->arch.vioapic,
-						irq_event.irq,
-						irq_event.level);
+			kvm_set_irq(kvm, KVM_USERSPACE_IRQ_SOURCE_ID,
+				    irq_event.irq, irq_event.level);
 			mutex_unlock(&kvm->lock);
 			r = 0;
 		}
diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
index 6144d3f..da06919 100644
--- a/arch/x86/kvm/i8254.c
+++ b/arch/x86/kvm/i8254.c
@@ -545,6 +545,12 @@ struct kvm_pit *kvm_create_pit(struct kvm *kvm)
 	if (!pit)
 		return NULL;
 
+	mutex_lock(&kvm->lock);
+	pit->irq_source_id = kvm_request_irq_source_id(kvm);
+	mutex_unlock(&kvm->lock);
+	if (pit->irq_source_id < 0)
+		return NULL;
+
 	mutex_init(&pit->pit_state.lock);
 	mutex_lock(&pit->pit_state.lock);
 	spin_lock_init(&pit->pit_state.inject_lock);
@@ -587,6 +593,7 @@ void kvm_free_pit(struct kvm *kvm)
 		mutex_lock(&kvm->arch.vpit->pit_state.lock);
 		timer = &kvm->arch.vpit->pit_state.pit_timer.timer;
 		hrtimer_cancel(timer);
+		kvm_free_irq_source_id(kvm,
kvm->arch.vpit->irq_source_id);
 		mutex_unlock(&kvm->arch.vpit->pit_state.lock);
 		kfree(kvm->arch.vpit);
 	}
@@ -598,8 +605,8 @@ static void __inject_pit_timer_intr(struct kvm *kvm)
 	int i;
 
 	mutex_lock(&kvm->lock);
-	kvm_set_irq(kvm, 0, 1);
-	kvm_set_irq(kvm, 0, 0);
+	kvm_set_irq(kvm, kvm->arch.vpit->irq_source_id, 0, 1);
+	kvm_set_irq(kvm, kvm->arch.vpit->irq_source_id, 0, 0);
 	mutex_unlock(&kvm->lock);
 
 	/*
diff --git a/arch/x86/kvm/i8254.h b/arch/x86/kvm/i8254.h
index e436d49..4178022 100644
--- a/arch/x86/kvm/i8254.h
+++ b/arch/x86/kvm/i8254.h
@@ -44,6 +44,7 @@ struct kvm_pit {
 	struct kvm_io_device speaker_dev;
 	struct kvm *kvm;
 	struct kvm_kpit_state pit_state;
+	int irq_source_id;
 };
 
 #define KVM_PIT_BASE_ADDRESS	    0x40
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index dda478e..5d29c50 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1816,7 +1816,8 @@ long kvm_arch_vm_ioctl(struct file *filp,
 			goto out;
 		if (irqchip_in_kernel(kvm)) {
 			mutex_lock(&kvm->lock);
-			kvm_set_irq(kvm, irq_event.irq,
irq_event.level);
+			kvm_set_irq(kvm, KVM_USERSPACE_IRQ_SOURCE_ID,
+				    irq_event.irq, irq_event.level);
 			mutex_unlock(&kvm->lock);
 			r = 0;
 		}
@@ -4115,6 +4116,9 @@ struct  kvm *kvm_arch_create_vm(void)
 	INIT_LIST_HEAD(&kvm->arch.active_mmu_pages);
 	INIT_LIST_HEAD(&kvm->arch.assigned_dev_head);
 
+	/* Reserve bit 0 of irq_sources_bitmap for userspace irq source
*/
+	kvm->arch.irq_sources_bitmap = 1;
+
 	return kvm;
 }
 
diff --git a/include/asm-x86/kvm_host.h b/include/asm-x86/kvm_host.h
index 4b06ca8..f01e92e 100644
--- a/include/asm-x86/kvm_host.h
+++ b/include/asm-x86/kvm_host.h
@@ -368,6 +368,9 @@ struct kvm_arch{
 
 	struct page *ept_identity_pagetable;
 	bool ept_identity_pagetable_done;
+
+	unsigned long irq_sources_bitmap;
+	unsigned long irq_states[KVM_IOAPIC_NUM_PINS];
 };
 
 struct kvm_vm_stat {
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 3833c48..bd5b16d 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -37,6 +37,8 @@
 #define KVM_REQ_UNHALT             6
 #define KVM_REQ_MMU_SYNC           7
 
+#define KVM_USERSPACE_IRQ_SOURCE_ID	1 << 0
+
 struct kvm_vcpu;
 extern struct kmem_cache *kvm_vcpu_cache;
 
@@ -306,15 +308,18 @@ struct kvm_assigned_dev_kernel {
 	int host_irq;
 	int guest_irq;
 	int irq_requested;
+	int irq_source_id;
 	struct pci_dev *dev;
 	struct kvm *kvm;
 };
-void kvm_set_irq(struct kvm *kvm, int irq, int level);
+void kvm_set_irq(struct kvm *kvm, int irq_source_id, int irq, int
level);
 void kvm_notify_acked_irq(struct kvm *kvm, unsigned gsi);
 void kvm_register_irq_ack_notifier(struct kvm *kvm,
 				   struct kvm_irq_ack_notifier *kian);
 void kvm_unregister_irq_ack_notifier(struct kvm *kvm,
 				     struct kvm_irq_ack_notifier *kian);
+int kvm_request_irq_source_id(struct kvm *kvm);
+void kvm_free_irq_source_id(struct kvm *kvm, int irq_source_id);
 
 #ifdef CONFIG_DMAR
 int kvm_iommu_map_pages(struct kvm *kvm, gfn_t base_gfn,
diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
index d0169f5..55ad76e 100644
--- a/virt/kvm/irq_comm.c
+++ b/virt/kvm/irq_comm.c
@@ -25,15 +25,23 @@
 #include "ioapic.h"
 
 /* This should be called with the kvm->lock mutex held */
-void kvm_set_irq(struct kvm *kvm, int irq, int level)
+void kvm_set_irq(struct kvm *kvm, int irq_source_id, int irq, int
level)
 {
+	unsigned long *irq_state = (unsigned long
*)&kvm->arch.irq_states[irq];
+
+	/* Logical OR for level trig interrupt */
+	if (level)
+		set_bit(irq_source_id, irq_state);
+	else
+		clear_bit(irq_source_id, irq_state);
+
 	/* Not possible to detect if the guest uses the PIC or the
 	 * IOAPIC.  So set the bit in both. The guest will ignore
 	 * writes to the unused one.
 	 */
-	kvm_ioapic_set_irq(kvm->arch.vioapic, irq, level);
+	kvm_ioapic_set_irq(kvm->arch.vioapic, irq, !!(*irq_state));
 #ifdef CONFIG_X86
-	kvm_pic_set_irq(pic_irqchip(kvm), irq, level);
+	kvm_pic_set_irq(pic_irqchip(kvm), irq, !!(*irq_state));
 #endif
 }
 
@@ -58,3 +66,31 @@ void kvm_unregister_irq_ack_notifier(struct kvm *kvm,
 {
 	hlist_del(&kian->link);
 }
+
+/* The caller must hold kvm->lock mutex */
+int kvm_request_irq_source_id(struct kvm *kvm)
+{
+	unsigned long *bitmap = &kvm->arch.irq_sources_bitmap;
+	int irq_source_id = find_first_zero_bit(bitmap,
+				sizeof(kvm->arch.irq_sources_bitmap));
+	if (irq_source_id >= sizeof(kvm->arch.irq_sources_bitmap)) {
+		printk(KERN_WARNING "kvm: exhaust allocatable IRQ
sources!\n");
+		irq_source_id = -EFAULT;
+	} else
+		set_bit(irq_source_id, bitmap);
+	return irq_source_id;
+}
+
+void kvm_free_irq_source_id(struct kvm *kvm, int irq_source_id)
+{
+	int i;
+
+	if (irq_source_id <= 0 ||
+	    irq_source_id >= sizeof(kvm->arch.irq_sources_bitmap)) {
+		printk(KERN_ERR "kvm: IRQ source ID out of range!\n");
+		return;
+	}
+	for (i = 0; i < KVM_IOAPIC_NUM_PINS; i++)
+		clear_bit(irq_source_id, &kvm->arch.irq_states[i]);
+	clear_bit(irq_source_id, &kvm->arch.irq_sources_bitmap);
+}
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index cf0ab8e..a87f45e 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -105,14 +105,12 @@ static void
kvm_assigned_dev_interrupt_work_handler(struct work_struct *work)
 	 */
 	mutex_lock(&assigned_dev->kvm->lock);
 	kvm_set_irq(assigned_dev->kvm,
+		    assigned_dev->irq_source_id,
 		    assigned_dev->guest_irq, 1);
 	mutex_unlock(&assigned_dev->kvm->lock);
 	kvm_put_kvm(assigned_dev->kvm);
 }
 
-/* FIXME: Implement the OR logic needed to make shared interrupts on
- * this line behave properly
- */
 static irqreturn_t kvm_assigned_dev_intr(int irq, void *dev_id)
 {
 	struct kvm_assigned_dev_kernel *assigned_dev =
@@ -134,7 +132,7 @@ static void kvm_assigned_dev_ack_irq(struct
kvm_irq_ack_notifier *kian)
 
 	dev = container_of(kian, struct kvm_assigned_dev_kernel,
 			   ack_notifier);
-	kvm_set_irq(dev->kvm, dev->guest_irq, 0);
+	kvm_set_irq(dev->kvm, dev->irq_source_id, dev->guest_irq, 0);
 	enable_irq(dev->host_irq);
 }
 
@@ -146,6 +144,7 @@ static void kvm_free_assigned_device(struct kvm
*kvm,
 		free_irq(assigned_dev->host_irq, (void *)assigned_dev);
 
 	kvm_unregister_irq_ack_notifier(kvm,
&assigned_dev->ack_notifier);
+	kvm_free_irq_source_id(kvm, assigned_dev->irq_source_id);
 
 	if (cancel_work_sync(&assigned_dev->interrupt_work))
 		/* We had pending work. That means we will have to take
@@ -215,6 +214,11 @@ static int kvm_vm_ioctl_assign_irq(struct kvm *kvm,
 		match->ack_notifier.gsi = assigned_irq->guest_irq;
 		match->ack_notifier.irq_acked =
kvm_assigned_dev_ack_irq;
 		kvm_register_irq_ack_notifier(kvm,
&match->ack_notifier);
+		r = kvm_request_irq_source_id(kvm);
+		if (r < 0)
+			goto out_release;
+		else
+			match->irq_source_id = r;
 
 		/* Even though this is PCI, we don't want to use shared
 		 * interrupts. Sharing host devices with guest-assigned
devices
-- 
1.5.1




Zhang, Xiantao wrote:
> Amit Shah wrote:
>> * On Wednesday 15 Oct 2008 12:41:17 Sheng Yang wrote:
>>> On Wednesday 15 October 2008 15:03:39 Amit Shah wrote:
>>>> * On Wednesday 15 Oct 2008 08:06:30 Sheng Yang wrote:
>>>>>>> @@ -4115,6 +4116,9 @@ struct  kvm *kvm_arch_create_vm(void)
>>>>>>>         INIT_LIST_HEAD(&kvm->arch.active_mmu_pages);
>>>>>>>         INIT_LIST_HEAD(&kvm->arch.assigned_dev_head);
>>>>>>> 
>>>>>>> +       /* Reserve bit 0 of irq_sources_bitmap for userspace irq
>>>>>>> source */ +       kvm->arch.irq_sources_bitmap = 1;
>>>>>>> +
>>>>> 
>>>>> Amit, could you help to use 1 << KVM_USERSPACE_IRQ_SOURCE_ID here,
>>>>> I come to think it's more proper. :)
>>>> 
>>>> This is fine; if you think there has to be a change, the change
>>>> could be done in the .h: 
>>>> 
>>>> - #define KVM_USERSPACE_IRQ_SOURCE_ID        0
>>>> + #define KVM_USERSPACE_IRQ_SOURCE_ID        1 << 0
>>> 
>>> Well, I think it's a semantics thing... irq_sources_bitmap is a
>>> bitmap, it can't equal to a source_id... So maybe
>>> 
>>> set_bit(KVM_USERSPACE_IRQ_SOURCE_ID, &kvm->arch.irq_sources_bitmap);
>>> 
>>> is the best way here. :)
>> 
>> OK; I see your point. Can you work with Xiantao and update the patch?
> 
> Okay, I will update the patch to make it work for ia64. Thanks!
> Xiantao


[-- Attachment #2: 0001-KVM-Fix-guest-shared-interrupt-with-in-kernel-irqch.patch --]
[-- Type: application/octet-stream, Size: 10388 bytes --]

From c0d1ad6327c01ba0584922022bef48c971bbf18a Mon Sep 17 00:00:00 2001
From: Sheng Yang <sheng@linux.intel.com>
Date: Wed, 15 Oct 2008 20:15:06 +0800
Subject: [PATCH] KVM: Fix guest shared interrupt with in-kernel irqchip

Every call of kvm_set_irq() should offer an irq_source_id, which is
allocated by kvm_request_irq_source_id(). Based on irq_source_id, we
identify the irq source and implement logical OR for shared level
interrupts.

The allocated irq_source_id can be freed by kvm_free_irq_source_id().

Currently, we support at most sizeof(unsigned long) different irq sources.

[Amit: - rebase to kvm.git HEAD
       - move definition of KVM_USERSPACE_IRQ_SOURCE_ID to common file
       - move kvm_request_irq_source_id to the update_irq ioctl]

[Xiantao: - Add kvm/ia64 stuff and make it work for kvm/ia64 guests]

Signed-off-by: Sheng Yang <sheng@linux.intel.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
Signed-off-by: Xiantao Zhang <xiantao.zhang@intel.com>
---
 arch/ia64/include/asm/kvm_host.h |    3 ++
 arch/ia64/kvm/kvm-ia64.c         |    8 ++++--
 arch/x86/kvm/i8254.c             |   11 ++++++++-
 arch/x86/kvm/i8254.h             |    1 +
 arch/x86/kvm/x86.c               |    6 ++++-
 include/asm-x86/kvm_host.h       |    3 ++
 include/linux/kvm_host.h         |    7 +++++-
 virt/kvm/irq_comm.c              |   42 +++++++++++++++++++++++++++++++++++--
 virt/kvm/kvm_main.c              |   12 +++++++---
 9 files changed, 79 insertions(+), 14 deletions(-)

diff --git a/arch/ia64/include/asm/kvm_host.h b/arch/ia64/include/asm/kvm_host.h
index e98f6f0..15f7703 100644
--- a/arch/ia64/include/asm/kvm_host.h
+++ b/arch/ia64/include/asm/kvm_host.h
@@ -466,6 +466,9 @@ struct kvm_arch {
 	struct list_head assigned_dev_head;
 	struct dmar_domain *intel_iommu_domain;
 	struct hlist_head irq_ack_notifier_list;
+
+	unsigned long irq_sources_bitmap;
+	unsigned long irq_states[KVM_IOAPIC_NUM_PINS];
 };
 
 union cpuid3_t {
diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c
index 1343781..961e6d1 100644
--- a/arch/ia64/kvm/kvm-ia64.c
+++ b/arch/ia64/kvm/kvm-ia64.c
@@ -775,6 +775,9 @@ static void kvm_init_vm(struct kvm *kvm)
 	kvm_build_io_pmt(kvm);
 
 	INIT_LIST_HEAD(&kvm->arch.assigned_dev_head);
+
+	/* Reserve bit 0 of irq_sources_bitmap for userspace irq source */
+	kvm->arch.irq_sources_bitmap = 1;
 }
 
 struct  kvm *kvm_arch_create_vm(void)
@@ -938,9 +941,8 @@ long kvm_arch_vm_ioctl(struct file *filp,
 			goto out;
 		if (irqchip_in_kernel(kvm)) {
 			mutex_lock(&kvm->lock);
-			kvm_ioapic_set_irq(kvm->arch.vioapic,
-						irq_event.irq,
-						irq_event.level);
+			kvm_set_irq(kvm, KVM_USERSPACE_IRQ_SOURCE_ID,
+				    irq_event.irq, irq_event.level);
 			mutex_unlock(&kvm->lock);
 			r = 0;
 		}
diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
index 6144d3f..da06919 100644
--- a/arch/x86/kvm/i8254.c
+++ b/arch/x86/kvm/i8254.c
@@ -545,6 +545,12 @@ struct kvm_pit *kvm_create_pit(struct kvm *kvm)
 	if (!pit)
 		return NULL;
 
+	mutex_lock(&kvm->lock);
+	pit->irq_source_id = kvm_request_irq_source_id(kvm);
+	mutex_unlock(&kvm->lock);
+	if (pit->irq_source_id < 0)
+		return NULL;
+
 	mutex_init(&pit->pit_state.lock);
 	mutex_lock(&pit->pit_state.lock);
 	spin_lock_init(&pit->pit_state.inject_lock);
@@ -587,6 +593,7 @@ void kvm_free_pit(struct kvm *kvm)
 		mutex_lock(&kvm->arch.vpit->pit_state.lock);
 		timer = &kvm->arch.vpit->pit_state.pit_timer.timer;
 		hrtimer_cancel(timer);
+		kvm_free_irq_source_id(kvm, kvm->arch.vpit->irq_source_id);
 		mutex_unlock(&kvm->arch.vpit->pit_state.lock);
 		kfree(kvm->arch.vpit);
 	}
@@ -598,8 +605,8 @@ static void __inject_pit_timer_intr(struct kvm *kvm)
 	int i;
 
 	mutex_lock(&kvm->lock);
-	kvm_set_irq(kvm, 0, 1);
-	kvm_set_irq(kvm, 0, 0);
+	kvm_set_irq(kvm, kvm->arch.vpit->irq_source_id, 0, 1);
+	kvm_set_irq(kvm, kvm->arch.vpit->irq_source_id, 0, 0);
 	mutex_unlock(&kvm->lock);
 
 	/*
diff --git a/arch/x86/kvm/i8254.h b/arch/x86/kvm/i8254.h
index e436d49..4178022 100644
--- a/arch/x86/kvm/i8254.h
+++ b/arch/x86/kvm/i8254.h
@@ -44,6 +44,7 @@ struct kvm_pit {
 	struct kvm_io_device speaker_dev;
 	struct kvm *kvm;
 	struct kvm_kpit_state pit_state;
+	int irq_source_id;
 };
 
 #define KVM_PIT_BASE_ADDRESS	    0x40
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index dda478e..5d29c50 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1816,7 +1816,8 @@ long kvm_arch_vm_ioctl(struct file *filp,
 			goto out;
 		if (irqchip_in_kernel(kvm)) {
 			mutex_lock(&kvm->lock);
-			kvm_set_irq(kvm, irq_event.irq, irq_event.level);
+			kvm_set_irq(kvm, KVM_USERSPACE_IRQ_SOURCE_ID,
+				    irq_event.irq, irq_event.level);
 			mutex_unlock(&kvm->lock);
 			r = 0;
 		}
@@ -4115,6 +4116,9 @@ struct  kvm *kvm_arch_create_vm(void)
 	INIT_LIST_HEAD(&kvm->arch.active_mmu_pages);
 	INIT_LIST_HEAD(&kvm->arch.assigned_dev_head);
 
+	/* Reserve bit 0 of irq_sources_bitmap for userspace irq source */
+	kvm->arch.irq_sources_bitmap = 1;
+
 	return kvm;
 }
 
diff --git a/include/asm-x86/kvm_host.h b/include/asm-x86/kvm_host.h
index 4b06ca8..f01e92e 100644
--- a/include/asm-x86/kvm_host.h
+++ b/include/asm-x86/kvm_host.h
@@ -368,6 +368,9 @@ struct kvm_arch{
 
 	struct page *ept_identity_pagetable;
 	bool ept_identity_pagetable_done;
+
+	unsigned long irq_sources_bitmap;
+	unsigned long irq_states[KVM_IOAPIC_NUM_PINS];
 };
 
 struct kvm_vm_stat {
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 3833c48..bd5b16d 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -37,6 +37,8 @@
 #define KVM_REQ_UNHALT             6
 #define KVM_REQ_MMU_SYNC           7
 
+#define KVM_USERSPACE_IRQ_SOURCE_ID	1 << 0
+
 struct kvm_vcpu;
 extern struct kmem_cache *kvm_vcpu_cache;
 
@@ -306,15 +308,18 @@ struct kvm_assigned_dev_kernel {
 	int host_irq;
 	int guest_irq;
 	int irq_requested;
+	int irq_source_id;
 	struct pci_dev *dev;
 	struct kvm *kvm;
 };
-void kvm_set_irq(struct kvm *kvm, int irq, int level);
+void kvm_set_irq(struct kvm *kvm, int irq_source_id, int irq, int level);
 void kvm_notify_acked_irq(struct kvm *kvm, unsigned gsi);
 void kvm_register_irq_ack_notifier(struct kvm *kvm,
 				   struct kvm_irq_ack_notifier *kian);
 void kvm_unregister_irq_ack_notifier(struct kvm *kvm,
 				     struct kvm_irq_ack_notifier *kian);
+int kvm_request_irq_source_id(struct kvm *kvm);
+void kvm_free_irq_source_id(struct kvm *kvm, int irq_source_id);
 
 #ifdef CONFIG_DMAR
 int kvm_iommu_map_pages(struct kvm *kvm, gfn_t base_gfn,
diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
index d0169f5..55ad76e 100644
--- a/virt/kvm/irq_comm.c
+++ b/virt/kvm/irq_comm.c
@@ -25,15 +25,23 @@
 #include "ioapic.h"
 
 /* This should be called with the kvm->lock mutex held */
-void kvm_set_irq(struct kvm *kvm, int irq, int level)
+void kvm_set_irq(struct kvm *kvm, int irq_source_id, int irq, int level)
 {
+	unsigned long *irq_state = (unsigned long *)&kvm->arch.irq_states[irq];
+
+	/* Logical OR for level trig interrupt */
+	if (level)
+		set_bit(irq_source_id, irq_state);
+	else
+		clear_bit(irq_source_id, irq_state);
+
 	/* Not possible to detect if the guest uses the PIC or the
 	 * IOAPIC.  So set the bit in both. The guest will ignore
 	 * writes to the unused one.
 	 */
-	kvm_ioapic_set_irq(kvm->arch.vioapic, irq, level);
+	kvm_ioapic_set_irq(kvm->arch.vioapic, irq, !!(*irq_state));
 #ifdef CONFIG_X86
-	kvm_pic_set_irq(pic_irqchip(kvm), irq, level);
+	kvm_pic_set_irq(pic_irqchip(kvm), irq, !!(*irq_state));
 #endif
 }
 
@@ -58,3 +66,31 @@ void kvm_unregister_irq_ack_notifier(struct kvm *kvm,
 {
 	hlist_del(&kian->link);
 }
+
+/* The caller must hold kvm->lock mutex */
+int kvm_request_irq_source_id(struct kvm *kvm)
+{
+	unsigned long *bitmap = &kvm->arch.irq_sources_bitmap;
+	int irq_source_id = find_first_zero_bit(bitmap,
+				sizeof(kvm->arch.irq_sources_bitmap));
+	if (irq_source_id >= sizeof(kvm->arch.irq_sources_bitmap)) {
+		printk(KERN_WARNING "kvm: exhaust allocatable IRQ sources!\n");
+		irq_source_id = -EFAULT;
+	} else
+		set_bit(irq_source_id, bitmap);
+	return irq_source_id;
+}
+
+void kvm_free_irq_source_id(struct kvm *kvm, int irq_source_id)
+{
+	int i;
+
+	if (irq_source_id <= 0 ||
+	    irq_source_id >= sizeof(kvm->arch.irq_sources_bitmap)) {
+		printk(KERN_ERR "kvm: IRQ source ID out of range!\n");
+		return;
+	}
+	for (i = 0; i < KVM_IOAPIC_NUM_PINS; i++)
+		clear_bit(irq_source_id, &kvm->arch.irq_states[i]);
+	clear_bit(irq_source_id, &kvm->arch.irq_sources_bitmap);
+}
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index cf0ab8e..a87f45e 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -105,14 +105,12 @@ static void kvm_assigned_dev_interrupt_work_handler(struct work_struct *work)
 	 */
 	mutex_lock(&assigned_dev->kvm->lock);
 	kvm_set_irq(assigned_dev->kvm,
+		    assigned_dev->irq_source_id,
 		    assigned_dev->guest_irq, 1);
 	mutex_unlock(&assigned_dev->kvm->lock);
 	kvm_put_kvm(assigned_dev->kvm);
 }
 
-/* FIXME: Implement the OR logic needed to make shared interrupts on
- * this line behave properly
- */
 static irqreturn_t kvm_assigned_dev_intr(int irq, void *dev_id)
 {
 	struct kvm_assigned_dev_kernel *assigned_dev =
@@ -134,7 +132,7 @@ static void kvm_assigned_dev_ack_irq(struct kvm_irq_ack_notifier *kian)
 
 	dev = container_of(kian, struct kvm_assigned_dev_kernel,
 			   ack_notifier);
-	kvm_set_irq(dev->kvm, dev->guest_irq, 0);
+	kvm_set_irq(dev->kvm, dev->irq_source_id, dev->guest_irq, 0);
 	enable_irq(dev->host_irq);
 }
 
@@ -146,6 +144,7 @@ static void kvm_free_assigned_device(struct kvm *kvm,
 		free_irq(assigned_dev->host_irq, (void *)assigned_dev);
 
 	kvm_unregister_irq_ack_notifier(kvm, &assigned_dev->ack_notifier);
+	kvm_free_irq_source_id(kvm, assigned_dev->irq_source_id);
 
 	if (cancel_work_sync(&assigned_dev->interrupt_work))
 		/* We had pending work. That means we will have to take
@@ -215,6 +214,11 @@ static int kvm_vm_ioctl_assign_irq(struct kvm *kvm,
 		match->ack_notifier.gsi = assigned_irq->guest_irq;
 		match->ack_notifier.irq_acked = kvm_assigned_dev_ack_irq;
 		kvm_register_irq_ack_notifier(kvm, &match->ack_notifier);
+		r = kvm_request_irq_source_id(kvm);
+		if (r < 0)
+			goto out_release;
+		else
+			match->irq_source_id = r;
 
 		/* Even though this is PCI, we don't want to use shared
 		 * interrupts. Sharing host devices with guest-assigned devices
-- 
1.5.1

Re: [v2] Shared guest irq support

[Avi Kivity]

Zhang, Xiantao wrote:
> Forget to modify the from item generated automatically by git.  Modified
> to Sheng Yang <sheng@linux.intel.com>  :)
> Xiantao
>
> From c0d1ad6327c01ba0584922022bef48c971bbf18a Mon Sep 17 00:00:00 2001
> From: Sheng Yang <sheng@linux.intel.com>
> Date: Wed, 15 Oct 2008 20:15:06 +0800
> Subject: [PATCH] KVM: Fix guest shared interrupt with in-kernel irqchip
>
> Every call of kvm_set_irq() should offer an irq_source_id, which is
> allocated by kvm_request_irq_source_id(). Based on irq_source_id, we
> identify the irq source and implement logical OR for shared level
> interrupts.
>
> The allocated irq_source_id can be freed by kvm_free_irq_source_id().
>
> Currently, we support at most sizeof(unsigned long) different irq
> sources.
>
>
>  
> +#define KVM_USERSPACE_IRQ_SOURCE_ID	1 << 0
> +
>   

This is dangerous, if used in an expression together with a higher 
precedence operator.  I fixed it and applied.  Thanks to all involved.

-- 
error compiling committee.c: too many arguments to function

Re: [v2] Shared guest irq support

[Sheng Yang]

On Thursday 16 October 2008 16:21:38 Avi Kivity wrote:
> Zhang, Xiantao wrote:
> > Forget to modify the from item generated automatically by git.  Modified
> > to Sheng Yang <sheng@linux.intel.com>  :)
> > Xiantao
> >
> > From c0d1ad6327c01ba0584922022bef48c971bbf18a Mon Sep 17 00:00:00 2001
> > From: Sheng Yang <sheng@linux.intel.com>
> > Date: Wed, 15 Oct 2008 20:15:06 +0800
> > Subject: [PATCH] KVM: Fix guest shared interrupt with in-kernel irqchip
> >
> > Every call of kvm_set_irq() should offer an irq_source_id, which is
> > allocated by kvm_request_irq_source_id(). Based on irq_source_id, we
> > identify the irq source and implement logical OR for shared level
> > interrupts.
> >
> > The allocated irq_source_id can be freed by kvm_free_irq_source_id().
> >
> > Currently, we support at most sizeof(unsigned long) different irq
> > sources.
> >
> >
> >
> > +#define KVM_USERSPACE_IRQ_SOURCE_ID	1 << 0
> > +
>
> This is dangerous, if used in an expression together with a higher
> precedence operator.  I fixed it and applied.  Thanks to all involved.

Wait a minute....

I think I reserved source id 0 to userspace, rather than 1<<0=1... It's 
strange... 

 + kvm->arch.irq_sources_bitmap = 1;

What really should be addressed is here... 

I will post a patch to fix it as soon as I saw the commit...

--
regards
Yang, Sheng

Re: [v2] Shared guest irq support

[Amit Shah]

* On Thursday 16 Oct 2008 13:51:38 Avi Kivity wrote:
> Zhang, Xiantao wrote:
> > Forget to modify the from item generated automatically by git.  Modified
> > to Sheng Yang <sheng@linux.intel.com>  :)
> > Xiantao
> >
> > From c0d1ad6327c01ba0584922022bef48c971bbf18a Mon Sep 17 00:00:00 2001
> > From: Sheng Yang <sheng@linux.intel.com>
> > Date: Wed, 15 Oct 2008 20:15:06 +0800
> > Subject: [PATCH] KVM: Fix guest shared interrupt with in-kernel irqchip
> >
> > Every call of kvm_set_irq() should offer an irq_source_id, which is
> > allocated by kvm_request_irq_source_id(). Based on irq_source_id, we
> > identify the irq source and implement logical OR for shared level
> > interrupts.
> >
> > The allocated irq_source_id can be freed by kvm_free_irq_source_id().
> >
> > Currently, we support at most sizeof(unsigned long) different irq
> > sources.
> >
> >
> >
> > +#define KVM_USERSPACE_IRQ_SOURCE_ID	1 << 0
> > +
>
> This is dangerous, if used in an expression together with a higher
> precedence operator.  I fixed it and applied.  Thanks to all involved.

Thanks; however, Sheng suggested we should use set_bit to set the 
corresponding bit since we're operating on bitfields in a bitmap.

Amit

Re: [v2] Shared guest irq support

[Avi Kivity]

Sheng Yang wrote:
>>>
>>> +#define KVM_USERSPACE_IRQ_SOURCE_ID	1 << 0
>>> +
>>>       
>> This is dangerous, if used in an expression together with a higher
>> precedence operator.  I fixed it and applied.  Thanks to all involved.
>>     
>
> Wait a minute....
>
> I think I reserved source id 0 to userspace, rather than 1<<0=1... It's 
> strange... 
>
>  + kvm->arch.irq_sources_bitmap = 1;
>
> What really should be addressed is here... 
>
>   

Oh right.  I changed it to 0.

-- 
error compiling committee.c: too many arguments to function

Re: [v2] Shared guest irq support

[Avi Kivity]

Amit Shah wrote:
>>>
>>>
>>> +#define KVM_USERSPACE_IRQ_SOURCE_ID	1 << 0
>>> +
>>>       
>> This is dangerous, if used in an expression together with a higher
>> precedence operator.  I fixed it and applied.  Thanks to all involved.
>>     
>
> Thanks; however, Sheng suggested we should use set_bit to set the 
> corresponding bit since we're operating on bitfields in a bitmap.
>   

We are actually using set_bit, so the value should be zero.

-- 
error compiling committee.c: too many arguments to function