* RE: [v2] Shared guest irq support @ 2008-10-14 8:45 Zhang, Xiantao 2008-10-15 2:36 ` Sheng Yang 2008-10-15 7:19 ` Amit Shah 0 siblings, 2 replies; 20+ messages in thread From: Zhang, Xiantao @ 2008-10-14 8:45 UTC (permalink / raw) To: Amit Shah, avi; +Cc: Yang, Sheng, kvm 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [v2] Shared guest irq support 2008-10-14 8:45 [v2] Shared guest irq support Zhang, Xiantao @ 2008-10-15 2:36 ` Sheng Yang 2008-10-15 7:03 ` Amit Shah 2008-10-15 7:19 ` Amit Shah 1 sibling, 1 reply; 20+ messages in thread From: Sheng Yang @ 2008-10-15 2:36 UTC (permalink / raw) To: kvm; +Cc: Zhang, Xiantao, Amit Shah, avi 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [v2] Shared guest irq support 2008-10-15 2:36 ` Sheng Yang @ 2008-10-15 7:03 ` Amit Shah 2008-10-15 7:11 ` Sheng Yang 0 siblings, 1 reply; 20+ messages in thread From: Amit Shah @ 2008-10-15 7:03 UTC (permalink / raw) To: Sheng Yang; +Cc: kvm, Zhang, Xiantao, avi * 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [v2] Shared guest irq support 2008-10-15 7:03 ` Amit Shah @ 2008-10-15 7:11 ` Sheng Yang 2008-10-15 10:03 ` Amit Shah 0 siblings, 1 reply; 20+ messages in thread From: Sheng Yang @ 2008-10-15 7:11 UTC (permalink / raw) To: Amit Shah; +Cc: kvm, Zhang, Xiantao, avi 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [v2] Shared guest irq support 2008-10-15 7:11 ` Sheng Yang @ 2008-10-15 10:03 ` Amit Shah 2008-10-15 10:11 ` Zhang, Xiantao 0 siblings, 1 reply; 20+ messages in thread From: Amit Shah @ 2008-10-15 10:03 UTC (permalink / raw) To: Sheng Yang; +Cc: kvm, Zhang, Xiantao, avi * 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? ^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [v2] Shared guest irq support 2008-10-15 10:03 ` Amit Shah @ 2008-10-15 10:11 ` Zhang, Xiantao 2008-10-15 13:44 ` Zhang, Xiantao 0 siblings, 1 reply; 20+ messages in thread From: Zhang, Xiantao @ 2008-10-15 10:11 UTC (permalink / raw) To: Amit Shah, Sheng Yang; +Cc: kvm, avi 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [v2] Shared guest irq support 2008-10-15 10:11 ` Zhang, Xiantao @ 2008-10-15 13:44 ` Zhang, Xiantao 2008-10-15 13:56 ` Zhang, Xiantao 0 siblings, 1 reply; 20+ messages in thread From: Zhang, Xiantao @ 2008-10-15 13:44 UTC (permalink / raw) To: Zhang, Xiantao, Amit Shah, Sheng Yang; +Cc: kvm, avi [-- 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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* RE: [v2] Shared guest irq support 2008-10-15 13:44 ` Zhang, Xiantao @ 2008-10-15 13:56 ` Zhang, Xiantao 2008-10-16 8:21 ` Avi Kivity 0 siblings, 1 reply; 20+ messages in thread From: Zhang, Xiantao @ 2008-10-15 13:56 UTC (permalink / raw) To: Zhang, Xiantao, Amit Shah, Sheng Yang; +Cc: kvm, avi [-- 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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [v2] Shared guest irq support 2008-10-15 13:56 ` Zhang, Xiantao @ 2008-10-16 8:21 ` Avi Kivity 2008-10-16 8:28 ` Sheng Yang 2008-10-16 8:31 ` Amit Shah 0 siblings, 2 replies; 20+ messages in thread From: Avi Kivity @ 2008-10-16 8:21 UTC (permalink / raw) To: Zhang, Xiantao; +Cc: Amit Shah, Sheng Yang, kvm 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [v2] Shared guest irq support 2008-10-16 8:21 ` Avi Kivity @ 2008-10-16 8:28 ` Sheng Yang 2008-10-16 8:45 ` Avi Kivity 2008-10-16 8:31 ` Amit Shah 1 sibling, 1 reply; 20+ messages in thread From: Sheng Yang @ 2008-10-16 8:28 UTC (permalink / raw) To: Avi Kivity; +Cc: Zhang, Xiantao, Amit Shah, kvm 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [v2] Shared guest irq support 2008-10-16 8:28 ` Sheng Yang @ 2008-10-16 8:45 ` Avi Kivity 0 siblings, 0 replies; 20+ messages in thread From: Avi Kivity @ 2008-10-16 8:45 UTC (permalink / raw) To: Sheng Yang; +Cc: Zhang, Xiantao, Amit Shah, kvm 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [v2] Shared guest irq support 2008-10-16 8:21 ` Avi Kivity 2008-10-16 8:28 ` Sheng Yang @ 2008-10-16 8:31 ` Amit Shah 2008-10-16 8:47 ` Avi Kivity 1 sibling, 1 reply; 20+ messages in thread From: Amit Shah @ 2008-10-16 8:31 UTC (permalink / raw) To: Avi Kivity; +Cc: Zhang, Xiantao, Sheng Yang, kvm * 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [v2] Shared guest irq support 2008-10-16 8:31 ` Amit Shah @ 2008-10-16 8:47 ` Avi Kivity 0 siblings, 0 replies; 20+ messages in thread From: Avi Kivity @ 2008-10-16 8:47 UTC (permalink / raw) To: Amit Shah; +Cc: Zhang, Xiantao, Sheng Yang, kvm 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [v2] Shared guest irq support 2008-10-14 8:45 [v2] Shared guest irq support Zhang, Xiantao 2008-10-15 2:36 ` Sheng Yang @ 2008-10-15 7:19 ` Amit Shah 1 sibling, 0 replies; 20+ messages in thread From: Amit Shah @ 2008-10-15 7:19 UTC (permalink / raw) To: Zhang, Xiantao; +Cc: avi, Yang, Sheng, kvm [-- 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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
[parent not found: <305032847.30081223883050196.JavaMail.root@zmail02.collab.prod.int.phx2.redhat.com>]
* [v2] Shared guest irq support [not found] <305032847.30081223883050196.JavaMail.root@zmail02.collab.prod.int.phx2.redhat.com> @ 2008-10-13 7:31 ` Amit Shah 2008-10-13 7:33 ` Zhang, Xiantao ` (2 more replies) 0 siblings, 3 replies; 20+ messages in thread From: Amit Shah @ 2008-10-13 7:31 UTC (permalink / raw) To: Sheng Yang; +Cc: avi, kvm, Xiantao Zhang [-- 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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* RE: [v2] Shared guest irq support 2008-10-13 7:31 ` Amit Shah @ 2008-10-13 7:33 ` Zhang, Xiantao 2008-10-13 7:41 ` Amit Shah 2008-10-13 7:36 ` Sheng Yang 2008-10-14 8:00 ` Amit Shah 2 siblings, 1 reply; 20+ messages in thread From: Zhang, Xiantao @ 2008-10-13 7:33 UTC (permalink / raw) To: Amit Shah, Sheng Yang; +Cc: avi, kvm 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [v2] Shared guest irq support 2008-10-13 7:33 ` Zhang, Xiantao @ 2008-10-13 7:41 ` Amit Shah 2008-10-13 7:44 ` Zhang, Xiantao 0 siblings, 1 reply; 20+ messages in thread From: Amit Shah @ 2008-10-13 7:41 UTC (permalink / raw) To: Xiantao Zhang; +Cc: avi, kvm, Sheng Yang ----- "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. ^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [v2] Shared guest irq support 2008-10-13 7:41 ` Amit Shah @ 2008-10-13 7:44 ` Zhang, Xiantao 0 siblings, 0 replies; 20+ messages in thread From: Zhang, Xiantao @ 2008-10-13 7:44 UTC (permalink / raw) To: Amit Shah; +Cc: avi, kvm, Sheng Yang 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [v2] Shared guest irq support 2008-10-13 7:31 ` Amit Shah 2008-10-13 7:33 ` Zhang, Xiantao @ 2008-10-13 7:36 ` Sheng Yang 2008-10-14 8:00 ` Amit Shah 2 siblings, 0 replies; 20+ messages in thread From: Sheng Yang @ 2008-10-13 7:36 UTC (permalink / raw) To: Amit Shah; +Cc: avi, kvm, Xiantao Zhang 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [v2] Shared guest irq support 2008-10-13 7:31 ` Amit Shah 2008-10-13 7:33 ` Zhang, Xiantao 2008-10-13 7:36 ` Sheng Yang @ 2008-10-14 8:00 ` Amit Shah 2 siblings, 0 replies; 20+ messages in thread From: Amit Shah @ 2008-10-14 8:00 UTC (permalink / raw) To: avi; +Cc: Sheng Yang, kvm, Xiantao Zhang * 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. ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2008-10-16 8:47 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-10-14 8:45 [v2] Shared guest irq support Zhang, Xiantao
2008-10-15 2:36 ` Sheng Yang
2008-10-15 7:03 ` Amit Shah
2008-10-15 7:11 ` Sheng Yang
2008-10-15 10:03 ` Amit Shah
2008-10-15 10:11 ` Zhang, Xiantao
2008-10-15 13:44 ` Zhang, Xiantao
2008-10-15 13:56 ` Zhang, Xiantao
2008-10-16 8:21 ` Avi Kivity
2008-10-16 8:28 ` Sheng Yang
2008-10-16 8:45 ` Avi Kivity
2008-10-16 8:31 ` Amit Shah
2008-10-16 8:47 ` Avi Kivity
2008-10-15 7:19 ` Amit Shah
[not found] <305032847.30081223883050196.JavaMail.root@zmail02.collab.prod.int.phx2.redhat.com>
2008-10-13 7:31 ` Amit Shah
2008-10-13 7:33 ` Zhang, Xiantao
2008-10-13 7:41 ` Amit Shah
2008-10-13 7:44 ` Zhang, Xiantao
2008-10-13 7:36 ` Sheng Yang
2008-10-14 8:00 ` Amit Shah
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).