From patchwork Thu Jul 16 14:03:36 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gleb Natapov X-Patchwork-Id: 35879 Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n6GE46Hv031991 for ; Thu, 16 Jul 2009 14:04:08 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932323AbZGPOD5 (ORCPT ); Thu, 16 Jul 2009 10:03:57 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S932286AbZGPOD4 (ORCPT ); Thu, 16 Jul 2009 10:03:56 -0400 Received: from mx2.redhat.com ([66.187.237.31]:40152 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932303AbZGPODu (ORCPT ); Thu, 16 Jul 2009 10:03:50 -0400 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n6GE3oj5025894 for ; Thu, 16 Jul 2009 10:03:50 -0400 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx2.corp.redhat.com (8.13.1/8.13.1) with ESMTP id n6GE3l6g014651; Thu, 16 Jul 2009 10:03:47 -0400 Received: from dhcp-1-237.tlv.redhat.com (dhcp-1-237.tlv.redhat.com [10.35.1.237]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id n6GE3kAM001323; Thu, 16 Jul 2009 10:03:46 -0400 Received: by dhcp-1-237.tlv.redhat.com (Postfix, from userid 13519) id 2B65B18D486; Thu, 16 Jul 2009 17:03:40 +0300 (IDT) From: Gleb Natapov To: kvm@vger.kernel.org Cc: mtosatti@redhat.com Subject: [PATCH 08/11] Move IO APIC to its own lock. Date: Thu, 16 Jul 2009 17:03:36 +0300 Message-Id: <1247753019-11412-9-git-send-email-gleb@redhat.com> In-Reply-To: <1247753019-11412-1-git-send-email-gleb@redhat.com> References: <1247753019-11412-1-git-send-email-gleb@redhat.com> X-Scanned-By: MIMEDefang 2.58 on 172.16.27.26 Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org Introduce new function kvm_notifier_set_irq() that should be used to change irq line level from irq notifiers. When irq notifier change irq line level it calls into irq chip code recursively. The function avoids taking a lock recursively. Signed-off-by: Gleb Natapov --- arch/ia64/kvm/kvm-ia64.c | 27 ++++++++++++++----- arch/x86/include/asm/kvm_host.h | 2 +- arch/x86/kvm/i8259.c | 2 +- arch/x86/kvm/lapic.c | 5 +--- arch/x86/kvm/x86.c | 30 ++++++++++++++------- include/linux/kvm_host.h | 3 +- virt/kvm/ioapic.c | 53 ++++++++++++++++++++++++-------------- virt/kvm/ioapic.h | 4 ++- virt/kvm/irq_comm.c | 25 ++++++++++++++---- virt/kvm/kvm_main.c | 2 +- 10 files changed, 101 insertions(+), 52 deletions(-) diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c index 0ad09f0..dd7ef2d 100644 --- a/arch/ia64/kvm/kvm-ia64.c +++ b/arch/ia64/kvm/kvm-ia64.c @@ -850,9 +850,16 @@ static int kvm_vm_ioctl_get_irqchip(struct kvm *kvm, r = 0; switch (chip->chip_id) { - case KVM_IRQCHIP_IOAPIC: - memcpy(&chip->chip.ioapic, ioapic_irqchip(kvm), - sizeof(struct kvm_ioapic_state)); + case KVM_IRQCHIP_IOAPIC: { + struct kvm_ioapic *ioapic = ioapic_irqchip(kvm); + if (ioapic) { + spin_lock(&ioapic->lock); + memcpy(&chip->chip.ioapic, ioapic, + sizeof(struct kvm_ioapic_state)); + spin_unlock(&ioapic->lock); + } else + r = -EINVAL; + } break; default: r = -EINVAL; @@ -867,10 +874,16 @@ static int kvm_vm_ioctl_set_irqchip(struct kvm *kvm, struct kvm_irqchip *chip) r = 0; switch (chip->chip_id) { - case KVM_IRQCHIP_IOAPIC: - memcpy(ioapic_irqchip(kvm), - &chip->chip.ioapic, - sizeof(struct kvm_ioapic_state)); + case KVM_IRQCHIP_IOAPIC: { + struct kvm_ioapic *ioapic = ioapic_irqchip(kvm); + if (ioapic) { + spin_lock(&ioapic->lock); + memcpy(ioapic, &chip->chip.ioapic, + sizeof(struct kvm_ioapic_state)); + spin_unlock(&ioapic->lock); + } else + r = -EINVAL; + } break; default: r = -EINVAL; diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index eb723a7..cce2071 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -621,7 +621,7 @@ void kvm_queue_exception_e(struct kvm_vcpu *vcpu, unsigned nr, u32 error_code); void kvm_inject_page_fault(struct kvm_vcpu *vcpu, unsigned long cr2, u32 error_code); -int kvm_pic_set_irq(void *opaque, int irq, int level); +int kvm_pic_set_irq(void *opaque, int irq, int level, bool notifier); void kvm_inject_nmi(struct kvm_vcpu *vcpu); diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c index daf4606..d9baad7 100644 --- a/arch/x86/kvm/i8259.c +++ b/arch/x86/kvm/i8259.c @@ -182,7 +182,7 @@ void kvm_pic_update_irq(struct kvm_pic *s) pic_unlock(s); } -int kvm_pic_set_irq(void *opaque, int irq, int level) +int kvm_pic_set_irq(void *opaque, int irq, int level, bool notifier) { struct kvm_pic *s = opaque; int ret = -1; diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index e2e2849..61ffcfc 100644 --- a/arch/x86/kvm/lapic.c +++ b/arch/x86/kvm/lapic.c @@ -471,11 +471,8 @@ static void apic_set_eoi(struct kvm_lapic *apic) trigger_mode = IOAPIC_LEVEL_TRIG; else trigger_mode = IOAPIC_EDGE_TRIG; - if (!(apic_get_reg(apic, APIC_SPIV) & APIC_SPIV_DIRECTED_EOI)) { - mutex_lock(&apic->vcpu->kvm->irq_lock); + if (!(apic_get_reg(apic, APIC_SPIV) & APIC_SPIV_DIRECTED_EOI)) kvm_ioapic_update_eoi(apic->vcpu->kvm, vector, trigger_mode); - mutex_unlock(&apic->vcpu->kvm->irq_lock); - } } static void apic_send_ipi(struct kvm_lapic *apic) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 48567fa..de99b84 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -2010,10 +2010,16 @@ static int kvm_vm_ioctl_get_irqchip(struct kvm *kvm, struct kvm_irqchip *chip) &pic_irqchip(kvm)->pics[1], sizeof(struct kvm_pic_state)); break; - case KVM_IRQCHIP_IOAPIC: - memcpy(&chip->chip.ioapic, - ioapic_irqchip(kvm), - sizeof(struct kvm_ioapic_state)); + case KVM_IRQCHIP_IOAPIC: { + struct kvm_ioapic *ioapic = ioapic_irqchip(kvm); + if (ioapic) { + spin_lock(&ioapic->lock); + memcpy(&chip->chip.ioapic, ioapic, + sizeof(struct kvm_ioapic_state)); + spin_unlock(&ioapic->lock); + } else + r = -EINVAL; + } break; default: r = -EINVAL; @@ -2042,12 +2048,16 @@ static int kvm_vm_ioctl_set_irqchip(struct kvm *kvm, struct kvm_irqchip *chip) sizeof(struct kvm_pic_state)); spin_unlock(&pic_irqchip(kvm)->lock); break; - case KVM_IRQCHIP_IOAPIC: - mutex_lock(&kvm->irq_lock); - memcpy(ioapic_irqchip(kvm), - &chip->chip.ioapic, - sizeof(struct kvm_ioapic_state)); - mutex_unlock(&kvm->irq_lock); + case KVM_IRQCHIP_IOAPIC: { + struct kvm_ioapic *ioapic = ioapic_irqchip(kvm); + if (ioapic) { + spin_lock(&ioapic->lock); + memcpy(ioapic, &chip->chip.ioapic, + sizeof(struct kvm_ioapic_state)); + spin_unlock(&ioapic->lock); + } else + r = -EINVAL; + } break; default: r = -EINVAL; diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index 4eb5c85..c9824e0 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -120,7 +120,7 @@ struct kvm_memory_slot { struct kvm_kernel_irq_routing_entry { u32 gsi; int (*set)(struct kvm_kernel_irq_routing_entry *e, - struct kvm *kvm, int level); + struct kvm *kvm, int level, bool notifier); union { struct { unsigned irqchip; @@ -399,6 +399,7 @@ void kvm_get_intr_delivery_bitmask(struct kvm_ioapic *ioapic, unsigned long *deliver_bitmask); #endif int kvm_set_irq(struct kvm *kvm, int irq_source_id, int irq, int level); +int kvm_notifier_set_irq(struct kvm *kvm, int irq_source_id, int irq, int level); void kvm_notify_acked_irq(struct kvm *kvm, unsigned irqchip, unsigned pin); void kvm_register_irq_ack_notifier(struct kvm *kvm, struct kvm_irq_ack_notifier *kian); diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c index fa05f67..5a74649 100644 --- a/virt/kvm/ioapic.c +++ b/virt/kvm/ioapic.c @@ -175,13 +175,17 @@ static int ioapic_deliver(struct kvm_ioapic *ioapic, int irq) return kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe); } -int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int level) +int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int level, + bool notifier) { u32 old_irr = ioapic->irr; u32 mask = 1 << irq; union kvm_ioapic_redirect_entry entry; int ret = 1; + /* notifier may call ioapic recursively */ + if (!notifier) + spin_lock(&ioapic->lock); if (irq >= 0 && irq < IOAPIC_NUM_PINS) { entry = ioapic->redirtbl[irq]; level ^= entry.fields.polarity; @@ -196,34 +200,42 @@ int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int level) } trace_kvm_ioapic_set_irq(entry.bits, irq, ret == 0); } + if (!notifier) + spin_unlock(&ioapic->lock); + return ret; } -static void __kvm_ioapic_update_eoi(struct kvm_ioapic *ioapic, int pin, - int trigger_mode) +static void __kvm_ioapic_update_eoi(struct kvm_ioapic *ioapic, int vector, + int trigger_mode) { - union kvm_ioapic_redirect_entry *ent; + int i; + + for (i = 0; i < IOAPIC_NUM_PINS; i++) { + union kvm_ioapic_redirect_entry *ent = &ioapic->redirtbl[i]; + + if (ent->fields.vector != vector) + continue; - ent = &ioapic->redirtbl[pin]; + kvm_notify_acked_irq(ioapic->kvm, KVM_IRQCHIP_IOAPIC, i); - kvm_notify_acked_irq(ioapic->kvm, KVM_IRQCHIP_IOAPIC, pin); + if (trigger_mode != IOAPIC_LEVEL_TRIG) + continue; - if (trigger_mode == IOAPIC_LEVEL_TRIG) { ASSERT(ent->fields.trig_mode == IOAPIC_LEVEL_TRIG); ent->fields.remote_irr = 0; - if (!ent->fields.mask && (ioapic->irr & (1 << pin))) - ioapic_service(ioapic, pin); + if (!ent->fields.mask && (ioapic->irr & (1 << i))) + ioapic_service(ioapic, i); } } void kvm_ioapic_update_eoi(struct kvm *kvm, int vector, int trigger_mode) { struct kvm_ioapic *ioapic = kvm->arch.vioapic; - int i; - for (i = 0; i < IOAPIC_NUM_PINS; i++) - if (ioapic->redirtbl[i].fields.vector == vector) - __kvm_ioapic_update_eoi(ioapic, i, trigger_mode); + spin_lock(&ioapic->lock); + __kvm_ioapic_update_eoi(ioapic, vector, trigger_mode); + spin_unlock(&ioapic->lock); } static inline struct kvm_ioapic *to_ioapic(struct kvm_io_device *dev) @@ -248,8 +260,8 @@ static int ioapic_mmio_read(struct kvm_io_device *this, gpa_t addr, int len, ioapic_debug("addr %lx\n", (unsigned long)addr); ASSERT(!(addr & 0xf)); /* check alignment */ - mutex_lock(&ioapic->kvm->irq_lock); addr &= 0xff; + spin_lock(&ioapic->lock); switch (addr) { case IOAPIC_REG_SELECT: result = ioapic->ioregsel; @@ -263,6 +275,8 @@ static int ioapic_mmio_read(struct kvm_io_device *this, gpa_t addr, int len, result = 0; break; } + spin_unlock(&ioapic->lock); + switch (len) { case 8: *(u64 *) val = result; @@ -275,7 +289,6 @@ static int ioapic_mmio_read(struct kvm_io_device *this, gpa_t addr, int len, default: printk(KERN_WARNING "ioapic: wrong length %d\n", len); } - mutex_unlock(&ioapic->kvm->irq_lock); return 0; } @@ -291,15 +304,15 @@ static int ioapic_mmio_write(struct kvm_io_device *this, gpa_t addr, int len, (void*)addr, len, val); ASSERT(!(addr & 0xf)); /* check alignment */ - mutex_lock(&ioapic->kvm->irq_lock); if (len == 4 || len == 8) data = *(u32 *) val; else { printk(KERN_WARNING "ioapic: Unsupported size %d\n", len); - goto unlock; + return 0; } addr &= 0xff; + spin_lock(&ioapic->lock); switch (addr) { case IOAPIC_REG_SELECT: ioapic->ioregsel = data; @@ -310,15 +323,14 @@ static int ioapic_mmio_write(struct kvm_io_device *this, gpa_t addr, int len, break; #ifdef CONFIG_IA64 case IOAPIC_REG_EOI: - kvm_ioapic_update_eoi(ioapic->kvm, data, IOAPIC_LEVEL_TRIG); + __kvm_ioapic_update_eoi(ioapic, data, IOAPIC_LEVEL_TRIG); break; #endif default: break; } -unlock: - mutex_unlock(&ioapic->kvm->irq_lock); + spin_unlock(&ioapic->lock); return 0; } @@ -347,6 +359,7 @@ int kvm_ioapic_init(struct kvm *kvm) ioapic = kzalloc(sizeof(struct kvm_ioapic), GFP_KERNEL); if (!ioapic) return -ENOMEM; + spin_lock_init(&ioapic->lock); kvm->arch.vioapic = ioapic; kvm_ioapic_reset(ioapic); kvm_iodevice_init(&ioapic->dev, &ioapic_mmio_ops); diff --git a/virt/kvm/ioapic.h b/virt/kvm/ioapic.h index 7080b71..2cfa662 100644 --- a/virt/kvm/ioapic.h +++ b/virt/kvm/ioapic.h @@ -44,6 +44,7 @@ struct kvm_ioapic { struct kvm_io_device dev; struct kvm *kvm; void (*ack_notifier)(void *opaque, int irq); + spinlock_t lock; }; #ifdef DEBUG @@ -69,7 +70,8 @@ int kvm_apic_match_dest(struct kvm_vcpu *vcpu, struct kvm_lapic *source, int kvm_apic_compare_prio(struct kvm_vcpu *vcpu1, struct kvm_vcpu *vcpu2); void kvm_ioapic_update_eoi(struct kvm *kvm, int vector, int trigger_mode); int kvm_ioapic_init(struct kvm *kvm); -int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int level); +int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int level, + bool notifier); void kvm_ioapic_reset(struct kvm_ioapic *ioapic); int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src, struct kvm_lapic_irq *irq); diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c index 59c1cde..7a2b9db 100644 --- a/virt/kvm/irq_comm.c +++ b/virt/kvm/irq_comm.c @@ -32,19 +32,21 @@ #include "ioapic.h" static int kvm_set_pic_irq(struct kvm_kernel_irq_routing_entry *e, - struct kvm *kvm, int level) + struct kvm *kvm, int level, bool notifier) { #ifdef CONFIG_X86 - return kvm_pic_set_irq(pic_irqchip(kvm), e->irqchip.pin, level); + return kvm_pic_set_irq(pic_irqchip(kvm), e->irqchip.pin, level, + notifier); #else return -1; #endif } static int kvm_set_ioapic_irq(struct kvm_kernel_irq_routing_entry *e, - struct kvm *kvm, int level) + struct kvm *kvm, int level, bool notifier) { - return kvm_ioapic_set_irq(kvm->arch.vioapic, e->irqchip.pin, level); + return kvm_ioapic_set_irq(kvm->arch.vioapic, e->irqchip.pin, level, + notifier); } inline static bool kvm_is_dm_lowest_prio(struct kvm_lapic_irq *irq) @@ -122,7 +124,8 @@ static int kvm_set_msi(struct kvm_kernel_irq_routing_entry *e, * = 0 Interrupt was coalesced (previous irq is still pending) * > 0 Number of CPUs interrupt was delivered to */ -int kvm_set_irq(struct kvm *kvm, int irq_source_id, int irq, int level) +static int __kvm_set_irq(struct kvm *kvm, int irq_source_id, int irq, int level, + bool notifier) { struct kvm_kernel_irq_routing_entry *e; unsigned long *irq_state, sig_level; @@ -153,7 +156,7 @@ int kvm_set_irq(struct kvm *kvm, int irq_source_id, int irq, int level) rcu_read_lock(); for (e = rcu_dereference(kvm->irq_routing); e && e->set; e++) { if (e->gsi == irq) { - int r = e->set(e, kvm, sig_level); + int r = e->set(e, kvm, sig_level, notifier); if (r < 0) continue; @@ -164,6 +167,16 @@ int kvm_set_irq(struct kvm *kvm, int irq_source_id, int irq, int level) return ret; } +int kvm_set_irq(struct kvm *kvm, int irq_source_id, int irq, int level) +{ + return __kvm_set_irq(kvm, irq_source_id, irq, level, 0); +} + +int kvm_notifier_set_irq(struct kvm *kvm, int irq_source_id, int irq, int level) +{ + return __kvm_set_irq(kvm, irq_source_id, irq, level, 1); +} + void kvm_notify_acked_irq(struct kvm *kvm, unsigned irqchip, unsigned pin) { struct kvm_kernel_irq_routing_entry *e; diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 9553444..d2f2fa5 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -198,7 +198,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->irq_source_id, dev->guest_irq, 0); + kvm_notifier_set_irq(dev->kvm, dev->irq_source_id, dev->guest_irq, 0); /* The guest irq may be shared so this ack may be * from another device.