[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] RE: [Xen-changelog] [xen-unstable] x86: Properly synchronise updates to pirq-to-vector mapping.
@@ -491,16 +512,15 @@ int pirq_guest_bind(struct vcpu *v, int int rc = 0; cpumask_t cpumask = CPU_MASK_NONE; + WARN_ON(!spin_is_locked(&v->domain->evtchn_lock)); I find this WARN_ON() is triggered harmlessly when I assign device to HVM guest. The call trace is XEN_DOMCTL_bind_pt_irq -> pt_irq_create_bind_vtd() -> pirq_guest_bind(). Should we remove the WARN_ON here? Thanks, -- Dexuan -----Original Message----- From: xen-changelog-bounces@xxxxxxxxxxxxxxxxxxx [mailto:xen-changelog-bounces@xxxxxxxxxxxxxxxxxxx] On Behalf Of Xen patchbot-unstable Sent: 2008年9月26日 11:20 To: xen-changelog@xxxxxxxxxxxxxxxxxxx Subject: [Xen-changelog] [xen-unstable] x86: Properly synchronise updates to pirq-to-vector mapping. # HG changeset patch # User Keir Fraser <keir.fraser@xxxxxxxxxx> # Date 1222256215 -3600 # Node ID 31f09a5e24cf8eb8a9d73acc6c23262fe9d463d7 # Parent 7750906b06b3ebbba529e6d1042d7a2a2712623c x86: Properly synchronise updates to pirq-to-vector mapping. Per-domain irq mappings are now protected by d->evtchn_lock and by the per-vector irq_desc lock. Signed-off-by: Jan Beulich <jbeulich@xxxxxxxxxx> Signed-off-by: Keir Fraser <keir.fraser@xxxxxxxxxx> --- xen/arch/ia64/xen/irq.c | 16 ++++-- xen/arch/x86/domain.c | 2 xen/arch/x86/io_apic.c | 11 ---- xen/arch/x86/irq.c | 112 ++++++++++++++++++++++++++++--------------- xen/arch/x86/msi.c | 1 xen/arch/x86/physdev.c | 96 +++++++++++++++++++----------------- xen/common/event_channel.c | 3 - xen/include/asm-x86/domain.h | 2 xen/include/asm-x86/irq.h | 5 + xen/include/asm-x86/msi.h | 2 xen/include/xen/irq.h | 3 - 11 files changed, 142 insertions(+), 111 deletions(-) diff -r 7750906b06b3 -r 31f09a5e24cf xen/arch/ia64/xen/irq.c --- a/xen/arch/ia64/xen/irq.c Wed Sep 24 10:23:51 2008 +0100 +++ b/xen/arch/ia64/xen/irq.c Wed Sep 24 12:36:55 2008 +0100 @@ -459,20 +459,24 @@ int pirq_guest_bind(struct vcpu *v, int return rc; } -void pirq_guest_unbind(struct domain *d, int irq) +int pirq_guest_unbind(struct domain *d, int irq) { irq_desc_t *desc = &irq_desc[irq]; irq_guest_action_t *action; unsigned long flags; - int i; + int i, rc = 0; spin_lock_irqsave(&desc->lock, flags); action = (irq_guest_action_t *)desc->action; - i = 0; - while ( action->guest[i] && (action->guest[i] != d) ) - i++; + for ( i = 0; (i < action->nr_guests) && (action->guest[i] != d); i++ ) + continue; + if ( i == action->nr_guests ) + { + rc = -EINVAL; + goto out; + } memmove(&action->guest[i], &action->guest[i+1], IRQ_MAX_GUESTS-i-1); action->nr_guests--; @@ -492,7 +496,9 @@ void pirq_guest_unbind(struct domain *d, desc->handler->shutdown(irq); } + out: spin_unlock_irqrestore(&desc->lock, flags); + return rc; } void diff -r 7750906b06b3 -r 31f09a5e24cf xen/arch/x86/domain.c --- a/xen/arch/x86/domain.c Wed Sep 24 10:23:51 2008 +0100 +++ b/xen/arch/x86/domain.c Wed Sep 24 12:36:55 2008 +0100 @@ -414,8 +414,6 @@ int arch_domain_create(struct domain *d, goto fail; } - spin_lock_init(&d->arch.irq_lock); - if ( is_hvm_domain(d) ) { if ( (rc = hvm_domain_initialise(d)) != 0 ) diff -r 7750906b06b3 -r 31f09a5e24cf xen/arch/x86/io_apic.c --- a/xen/arch/x86/io_apic.c Wed Sep 24 10:23:51 2008 +0100 +++ b/xen/arch/x86/io_apic.c Wed Sep 24 12:36:55 2008 +0100 @@ -45,16 +45,6 @@ int (*ioapic_renumber_irq)(int ioapic, i int (*ioapic_renumber_irq)(int ioapic, int irq); atomic_t irq_mis_count; -int domain_irq_to_vector(struct domain *d, int irq) -{ - return d->arch.pirq_vector[irq]; -} - -int domain_vector_to_irq(struct domain *d, int vector) -{ - return d->arch.vector_pirq[vector]; -} - /* Where if anywhere is the i8259 connect in external int mode */ static struct { int pin, apic; } ioapic_i8259 = { -1, -1 }; @@ -721,7 +711,6 @@ next: static struct hw_interrupt_type ioapic_level_type; static struct hw_interrupt_type ioapic_edge_type; -struct hw_interrupt_type pci_msi_type; #define IOAPIC_AUTO -1 #define IOAPIC_EDGE 0 diff -r 7750906b06b3 -r 31f09a5e24cf xen/arch/x86/irq.c --- a/xen/arch/x86/irq.c Wed Sep 24 10:23:51 2008 +0100 +++ b/xen/arch/x86/irq.c Wed Sep 24 12:36:55 2008 +0100 @@ -277,6 +277,35 @@ static void __do_IRQ_guest(int vector) } } +/* + * Retrieve Xen irq-descriptor corresponding to a domain-specific irq. + * The descriptor is returned locked. This function is safe against changes + * to the per-domain irq-to-vector mapping. + */ +static irq_desc_t *domain_spin_lock_irq_desc( + struct domain *d, int irq, unsigned long *pflags) +{ + unsigned int vector; + unsigned long flags; + irq_desc_t *desc; + + for ( ; ; ) + { + vector = domain_irq_to_vector(d, irq); + if ( vector <= 0 ) + return NULL; + desc = &irq_desc[vector]; + spin_lock_irqsave(&desc->lock, flags); + if ( vector == domain_irq_to_vector(d, irq) ) + break; + spin_unlock_irqrestore(&desc->lock, flags); + } + + if ( pflags != NULL ) + *pflags = flags; + return desc; +} + /* Flush all ready EOIs from the top of this CPU's pending-EOI stack. */ static void flush_ready_eoi(void *unused) { @@ -342,11 +371,13 @@ static void __pirq_guest_eoi(struct doma cpumask_t cpu_eoi_map; int vector; - vector = domain_irq_to_vector(d, irq); - desc = &irq_desc[vector]; + ASSERT(local_irq_is_enabled()); + desc = domain_spin_lock_irq_desc(d, irq, NULL); + if ( desc == NULL ) + return; + action = (irq_guest_action_t *)desc->action; - - spin_lock_irq(&desc->lock); + vector = desc - irq_desc; ASSERT(!test_bit(irq, d->pirq_mask) || (action->ack_type != ACKTYPE_NONE)); @@ -418,7 +449,7 @@ int pirq_acktype(struct domain *d, int i unsigned int vector; vector = domain_irq_to_vector(d, irq); - if ( vector == 0 ) + if ( vector <= 0 ) return ACKTYPE_NONE; desc = &irq_desc[vector]; @@ -447,13 +478,6 @@ int pirq_acktype(struct domain *d, int i if ( !strcmp(desc->handler->typename, "XT-PIC") ) return ACKTYPE_UNMASK; - if ( strstr(desc->handler->typename, "MPIC") ) - { - if ( desc->status & IRQ_LEVEL ) - return (desc->status & IRQ_PER_CPU) ? ACKTYPE_EOI : ACKTYPE_UNMASK; - return ACKTYPE_NONE; /* edge-triggered => no final EOI */ - } - printk("Unknown PIC type '%s' for IRQ %d\n", desc->handler->typename, irq); BUG(); @@ -462,21 +486,18 @@ int pirq_acktype(struct domain *d, int i int pirq_shared(struct domain *d, int irq) { - unsigned int vector; irq_desc_t *desc; irq_guest_action_t *action; unsigned long flags; int shared; - vector = domain_irq_to_vector(d, irq); - if ( vector == 0 ) + desc = domain_spin_lock_irq_desc(d, irq, &flags); + if ( desc == NULL ) return 0; - desc = &irq_desc[vector]; - - spin_lock_irqsave(&desc->lock, flags); action = (irq_guest_action_t *)desc->action; shared = ((desc->status & IRQ_GUEST) && (action->nr_guests > 1)); + spin_unlock_irqrestore(&desc->lock, flags); return shared; @@ -491,16 +512,15 @@ int pirq_guest_bind(struct vcpu *v, int int rc = 0; cpumask_t cpumask = CPU_MASK_NONE; + WARN_ON(!spin_is_locked(&v->domain->evtchn_lock)); + retry: - vector = domain_irq_to_vector(v->domain, irq); - if ( vector == 0 ) + desc = domain_spin_lock_irq_desc(v->domain, irq, &flags); + if ( desc == NULL ) return -EINVAL; - desc = &irq_desc[vector]; - - spin_lock_irqsave(&desc->lock, flags); - action = (irq_guest_action_t *)desc->action; + vector = desc - irq_desc; if ( !(desc->status & IRQ_GUEST) ) { @@ -575,26 +595,39 @@ int pirq_guest_bind(struct vcpu *v, int return rc; } -void pirq_guest_unbind(struct domain *d, int irq) -{ - unsigned int vector; +int pirq_guest_unbind(struct domain *d, int irq) +{ + int vector; irq_desc_t *desc; irq_guest_action_t *action; cpumask_t cpu_eoi_map; unsigned long flags; - int i; - - vector = domain_irq_to_vector(d, irq); - desc = &irq_desc[vector]; - BUG_ON(vector == 0); - - spin_lock_irqsave(&desc->lock, flags); + int i, rc = 0; + + WARN_ON(!spin_is_locked(&d->evtchn_lock)); + + desc = domain_spin_lock_irq_desc(d, irq, &flags); + if ( unlikely(desc == NULL) ) + { + if ( (vector = -domain_irq_to_vector(d, irq)) == 0 ) + return -EINVAL; + BUG_ON(vector <= 0); + desc = &irq_desc[vector]; + spin_lock_irqsave(&desc->lock, flags); + d->arch.pirq_vector[irq] = d->arch.vector_pirq[vector] = 0; + goto out; + } action = (irq_guest_action_t *)desc->action; - - i = 0; - while ( action->guest[i] && (action->guest[i] != d) ) - i++; + vector = desc - irq_desc; + + for ( i = 0; (i < action->nr_guests) && (action->guest[i] != d); i++ ) + continue; + if ( i == action->nr_guests ) + { + rc = -EINVAL; + goto out; + } memmove(&action->guest[i], &action->guest[i+1], IRQ_MAX_GUESTS-i-1); action->nr_guests--; @@ -661,7 +694,8 @@ void pirq_guest_unbind(struct domain *d, desc->handler->shutdown(vector); out: - spin_unlock_irqrestore(&desc->lock, flags); + spin_unlock_irqrestore(&desc->lock, flags); + return rc; } extern void dump_ioapic_irq_info(void); diff -r 7750906b06b3 -r 31f09a5e24cf xen/arch/x86/msi.c --- a/xen/arch/x86/msi.c Wed Sep 24 10:23:51 2008 +0100 +++ b/xen/arch/x86/msi.c Wed Sep 24 12:36:55 2008 +0100 @@ -727,7 +727,6 @@ void pci_disable_msi(int vector) __pci_disable_msix(vector); } -extern struct hw_interrupt_type pci_msi_type; static void msi_free_vectors(struct pci_dev* dev) { struct msi_desc *entry, *tmp; diff -r 7750906b06b3 -r 31f09a5e24cf xen/arch/x86/physdev.c --- a/xen/arch/x86/physdev.c Wed Sep 24 10:23:51 2008 +0100 +++ b/xen/arch/x86/physdev.c Wed Sep 24 12:36:55 2008 +0100 @@ -26,17 +26,11 @@ ioapic_guest_write( ioapic_guest_write( unsigned long physbase, unsigned int reg, u32 pval); - -extern struct hw_interrupt_type pci_msi_type; - static int get_free_pirq(struct domain *d, int type, int index) { int i; - if ( d == NULL ) - return -EINVAL; - - ASSERT(spin_is_locked(&d->arch.irq_lock)); + ASSERT(spin_is_locked(&d->evtchn_lock)); if ( type == MAP_PIRQ_TYPE_GSI ) { @@ -64,11 +58,10 @@ static int map_domain_pirq(struct domain int ret = 0; int old_vector, old_pirq; struct msi_info msi; - - if ( d == NULL ) - return -EINVAL; - - ASSERT(spin_is_locked(&d->arch.irq_lock)); + irq_desc_t *desc; + unsigned long flags; + + ASSERT(spin_is_locked(&d->evtchn_lock)); if ( !IS_PRIV(current->domain) ) return -EPERM; @@ -88,8 +81,7 @@ static int map_domain_pirq(struct domain { dprintk(XENLOG_G_ERR, "dom%d: pirq %d or vector %d already mapped\n", d->domain_id, pirq, vector); - ret = -EINVAL; - goto done; + return -EINVAL; } ret = irq_permit_access(d, pirq); @@ -97,17 +89,14 @@ static int map_domain_pirq(struct domain { dprintk(XENLOG_G_ERR, "dom%d: could not permit access to irq %d\n", d->domain_id, pirq); - goto done; - } + return ret; + } + + desc = &irq_desc[vector]; + spin_lock_irqsave(&desc->lock, flags); if ( map && MAP_PIRQ_TYPE_MSI == map->type ) { - irq_desc_t *desc; - unsigned long flags; - - desc = &irq_desc[vector]; - - spin_lock_irqsave(&desc->lock, flags); if ( desc->handler != &no_irq_type ) dprintk(XENLOG_G_ERR, "dom%d: vector %d in use\n", d->domain_id, vector); @@ -120,8 +109,6 @@ static int map_domain_pirq(struct domain msi.vector = vector; ret = pci_enable_msi(&msi); - - spin_unlock_irqrestore(&desc->lock, flags); if ( ret ) goto done; } @@ -130,6 +117,7 @@ static int map_domain_pirq(struct domain d->arch.vector_pirq[vector] = pirq; done: + spin_unlock_irqrestore(&desc->lock, flags); return ret; } @@ -139,18 +127,18 @@ static int unmap_domain_pirq(struct doma unsigned long flags; irq_desc_t *desc; int vector, ret = 0; - - if ( d == NULL || pirq < 0 || pirq >= NR_PIRQS ) + bool_t forced_unbind; + + if ( (pirq < 0) || (pirq >= NR_PIRQS) ) return -EINVAL; if ( !IS_PRIV(current->domain) ) return -EINVAL; - ASSERT(spin_is_locked(&d->arch.irq_lock)); + ASSERT(spin_is_locked(&d->evtchn_lock)); vector = d->arch.pirq_vector[pirq]; - - if ( !vector ) + if ( vector <= 0 ) { dprintk(XENLOG_G_ERR, "dom%d: pirq %d not mapped\n", d->domain_id, pirq); @@ -158,20 +146,34 @@ static int unmap_domain_pirq(struct doma goto done; } + forced_unbind = (pirq_guest_unbind(d, pirq) == 0); + if ( forced_unbind ) + dprintk(XENLOG_G_WARNING, "dom%d: forcing unbind of pirq %d\n", + d->domain_id, pirq); + desc = &irq_desc[vector]; spin_lock_irqsave(&desc->lock, flags); + + BUG_ON(vector != d->arch.pirq_vector[pirq]); + if ( desc->msi_desc ) pci_disable_msi(vector); if ( desc->handler == &pci_msi_type ) - { - /* MSI is not shared, so should be released already */ - BUG_ON(desc->status & IRQ_GUEST); - irq_desc[vector].handler = &no_irq_type; - } + desc->handler = &no_irq_type; + + if ( !forced_unbind ) + { + d->arch.pirq_vector[pirq] = 0; + d->arch.vector_pirq[vector] = 0; + } + else + { + d->arch.pirq_vector[pirq] = -vector; + d->arch.vector_pirq[vector] = -pirq; + } + spin_unlock_irqrestore(&desc->lock, flags); - - d->arch.pirq_vector[pirq] = d->arch.vector_pirq[vector] = 0; ret = irq_deny_access(d, pirq); if ( ret ) @@ -186,7 +188,6 @@ static int physdev_map_pirq(struct physd { struct domain *d; int vector, pirq, ret = 0; - unsigned long flags; if ( !IS_PRIV(current->domain) ) return -EPERM; @@ -243,8 +244,8 @@ static int physdev_map_pirq(struct physd goto free_domain; } - spin_lock_irqsave(&d->arch.irq_lock, flags); - if ( map->pirq == -1 ) + spin_lock(&d->evtchn_lock); + if ( map->pirq < 0 ) { if ( d->arch.vector_pirq[vector] ) { @@ -252,6 +253,11 @@ static int physdev_map_pirq(struct physd d->domain_id, map->index, map->pirq, d->arch.vector_pirq[vector]); pirq = d->arch.vector_pirq[vector]; + if ( pirq < 0 ) + { + ret = -EBUSY; + goto done; + } } else { @@ -284,7 +290,7 @@ static int physdev_map_pirq(struct physd if ( !ret ) map->pirq = pirq; done: - spin_unlock_irqrestore(&d->arch.irq_lock, flags); + spin_unlock(&d->evtchn_lock); free_domain: rcu_unlock_domain(d); return ret; @@ -293,7 +299,6 @@ static int physdev_unmap_pirq(struct phy static int physdev_unmap_pirq(struct physdev_unmap_pirq *unmap) { struct domain *d; - unsigned long flags; int ret; if ( !IS_PRIV(current->domain) ) @@ -307,9 +312,9 @@ static int physdev_unmap_pirq(struct phy if ( d == NULL ) return -ESRCH; - spin_lock_irqsave(&d->arch.irq_lock, flags); + spin_lock(&d->evtchn_lock); ret = unmap_domain_pirq(d, unmap->pirq); - spin_unlock_irqrestore(&d->arch.irq_lock, flags); + spin_unlock(&d->evtchn_lock); rcu_unlock_domain(d); @@ -416,7 +421,6 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_H case PHYSDEVOP_alloc_irq_vector: { struct physdev_irq irq_op; - unsigned long flags; ret = -EFAULT; if ( copy_from_guest(&irq_op, arg, 1) != 0 ) @@ -437,9 +441,9 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_H irq_op.vector = assign_irq_vector(irq); - spin_lock_irqsave(&dom0->arch.irq_lock, flags); + spin_lock(&dom0->evtchn_lock); ret = map_domain_pirq(dom0, irq_op.irq, irq_op.vector, NULL); - spin_unlock_irqrestore(&dom0->arch.irq_lock, flags); + spin_unlock(&dom0->evtchn_lock); if ( copy_to_guest(arg, &irq_op, 1) != 0 ) ret = -EFAULT; diff -r 7750906b06b3 -r 31f09a5e24cf xen/common/event_channel.c --- a/xen/common/event_channel.c Wed Sep 24 10:23:51 2008 +0100 +++ b/xen/common/event_channel.c Wed Sep 24 12:36:55 2008 +0100 @@ -387,7 +387,8 @@ static long __evtchn_close(struct domain break; case ECS_PIRQ: - pirq_guest_unbind(d1, chn1->u.pirq); + if ( pirq_guest_unbind(d1, chn1->u.pirq) != 0 ) + BUG(); d1->pirq_to_evtchn[chn1->u.pirq] = 0; break; diff -r 7750906b06b3 -r 31f09a5e24cf xen/include/asm-x86/domain.h --- a/xen/include/asm-x86/domain.h Wed Sep 24 10:23:51 2008 +0100 +++ b/xen/include/asm-x86/domain.h Wed Sep 24 12:36:55 2008 +0100 @@ -235,7 +235,7 @@ struct arch_domain /* Shadow translated domain: P2M mapping */ pagetable_t phys_table; - spinlock_t irq_lock; + /* NB. protected by d->evtchn_lock and by irq_desc[vector].lock */ int vector_pirq[NR_VECTORS]; int pirq_vector[NR_PIRQS]; diff -r 7750906b06b3 -r 31f09a5e24cf xen/include/asm-x86/irq.h --- a/xen/include/asm-x86/irq.h Wed Sep 24 10:23:51 2008 +0100 +++ b/xen/include/asm-x86/irq.h Wed Sep 24 12:36:55 2008 +0100 @@ -52,6 +52,7 @@ int pirq_acktype(struct domain *d, int i int pirq_acktype(struct domain *d, int irq); int pirq_shared(struct domain *d , int irq); -extern int domain_irq_to_vector(struct domain *d, int irq); -extern int domain_vector_to_irq(struct domain *d, int vector); +#define domain_irq_to_vector(d, irq) ((d)->arch.pirq_vector[(irq)]) +#define domain_vector_to_irq(d, vec) ((d)->arch.vector_pirq[(vec)]) + #endif /* _ASM_HW_IRQ_H */ diff -r 7750906b06b3 -r 31f09a5e24cf xen/include/asm-x86/msi.h --- a/xen/include/asm-x86/msi.h Wed Sep 24 10:23:51 2008 +0100 +++ b/xen/include/asm-x86/msi.h Wed Sep 24 12:36:55 2008 +0100 @@ -106,7 +106,7 @@ struct msi_desc { */ #define NR_HP_RESERVED_VECTORS 20 -extern int vector_irq[NR_VECTORS]; +extern struct hw_interrupt_type pci_msi_type; /* * MSI-X Address Register diff -r 7750906b06b3 -r 31f09a5e24cf xen/include/xen/irq.h --- a/xen/include/xen/irq.h Wed Sep 24 10:23:51 2008 +0100 +++ b/xen/include/xen/irq.h Wed Sep 24 12:36:55 2008 +0100 @@ -22,7 +22,6 @@ struct irqaction #define IRQ_PENDING 4 /* IRQ pending - replay on enable */ #define IRQ_REPLAY 8 /* IRQ has been replayed but not acked yet */ #define IRQ_GUEST 16 /* IRQ is handled by guest OS(es) */ -#define IRQ_LEVEL 64 /* IRQ level triggered */ #define IRQ_PER_CPU 256 /* IRQ is per CPU */ /* @@ -78,7 +77,7 @@ extern int pirq_guest_eoi(struct domain extern int pirq_guest_eoi(struct domain *d, int irq); extern int pirq_guest_unmask(struct domain *d); extern int pirq_guest_bind(struct vcpu *v, int irq, int will_share); -extern void pirq_guest_unbind(struct domain *d, int irq); +extern int pirq_guest_unbind(struct domain *d, int irq); static inline void set_native_irq_info(int irq, cpumask_t mask) { _______________________________________________ Xen-changelog mailing list Xen-changelog@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-changelog _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |