>>> Keir Fraser <keir.fraser@xxxxxxxxxxxxx> 23.09.08 13:27 >>>
>On 23/9/08 11:34, "Jan Beulich" <jbeulich@xxxxxxxxxx> wrote:
>
>> Simply removing the BUG_ON() seems inappropriate, though. But I'm
>> uncertain whether it would be reasonable to call pirq_guest_unbind()
>> instead of the BUG_ON(), and if so, how to properly deal with
>> irq_desc[vector].lock (the immediate idea would be to factor out the
>> locking into a wrapper function, but an alternative would be to use
>> recursive locks, and perhaps there are other possibilities).
>
>Well, this hypercall doesn't do pirq_guest_unbind() on IO-APIC-routed
>interrupts either, so I think the problem is wider ranging than just MSI
>interrupts. Consider unmap_irq() followed by pirq_guest_unbind() later.
>We'll BUG_ON(vector == 0) in the latter function. Looks a bit of a mess to
>me. I would say that if there are bindings remaining we should re-direct the
>event-channel to a 'no-op' pirq (e.g., -1) and indeed call
>pirq_guest_unbind() or similar.
How about this one? It doesn't exactly follow the path you suggested,
i.e. doesn't mess with event channels, but rather marks the
irq<->vector mapping as invalid, allowing only a subsequent event
channel unbind (i.e. close) to recover from that state (which seems
better in terms of requiring proper discipline in the guest, as it
prevents re-using the irq or vector without properly cleaning up).
Signed-off-by: Jan Beulich <jbeulich@xxxxxxxxxx>
Index: 2008年09月19日/xen/arch/x86/io_apic.c
===================================================================
--- 2008年09月19日.orig/xen/arch/x86/io_apic.c 2008年09月17日 09:26:41.000000000 
+0200
+++ 2008年09月19日/xen/arch/x86/io_apic.c 2008年09月24日 09:19:41.000000000 +0200
@@ -721,7 +721,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
Index: 2008年09月19日/xen/arch/x86/irq.c
===================================================================
--- 2008年09月19日.orig/xen/arch/x86/irq.c 2008年09月24日 09:17:33.000000000 +0200
+++ 2008年09月19日/xen/arch/x86/irq.c 2008年09月23日 15:26:26.000000000 +0200
@@ -343,6 +343,8 @@ static void __pirq_guest_eoi(struct doma
 int vector;
 
 vector = domain_irq_to_vector(d, irq);
+ if ( vector < 0 )
+ return;
 desc = &irq_desc[vector];
 action = (irq_guest_action_t *)desc->action;
 
@@ -418,7 +420,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];
@@ -469,7 +471,7 @@ int pirq_shared(struct domain *d, int ir
 int shared;
 
 vector = domain_irq_to_vector(d, irq);
- if ( vector == 0 )
+ if ( vector <= 0 )
 return 0;
 
 desc = &irq_desc[vector];
@@ -493,7 +495,7 @@ int pirq_guest_bind(struct vcpu *v, int 
 
 retry:
 vector = domain_irq_to_vector(v->domain, irq);
- if ( vector == 0 )
+ if ( vector <= 0 )
 return -EINVAL;
 
 desc = &irq_desc[vector];
@@ -575,20 +577,15 @@ int pirq_guest_bind(struct vcpu *v, int 
 return rc;
 }
 
-void pirq_guest_unbind(struct domain *d, int irq)
+void __pirq_guest_unbind(struct domain *d, int irq, unsigned int vector,
+ unsigned long flags)
 {
- unsigned int vector;
- irq_desc_t *desc;
+ irq_desc_t *desc = &irq_desc[vector];
 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);
+ ASSERT(spin_is_locked(&desc->lock));
 
 action = (irq_guest_action_t *)desc->action;
 
@@ -626,7 +623,7 @@ void pirq_guest_unbind(struct domain *d,
 BUG_ON(test_bit(irq, d->pirq_mask));
 
 if ( action->nr_guests != 0 )
- goto out;
+ return;
 
 BUG_ON(action->in_flight != 0);
 
@@ -659,9 +656,26 @@ void pirq_guest_unbind(struct domain *d,
 desc->status &= ~IRQ_INPROGRESS;
 kill_timer(&irq_guest_eoi_timer[vector]);
 desc->handler->shutdown(vector);
+}
 
- out:
- spin_unlock_irqrestore(&desc->lock, flags); 
+void pirq_guest_unbind(struct domain *d, int irq)
+{
+ int vector = domain_irq_to_vector(d, irq);
+ unsigned long flags;
+
+ if ( likely(vector > 0) )
+ {
+ spin_lock_irqsave(&irq_desc[vector].lock, flags);
+ __pirq_guest_unbind(d, irq, vector, flags);
+ spin_unlock_irqrestore(&irq_desc[vector].lock, flags);
+ }
+ else
+ {
+ BUG_ON(vector == 0);
+ spin_lock_irqsave(&d->arch.irq_lock, flags);
+ d->arch.pirq_vector[irq] = d->arch.vector_pirq[-vector] = 0;
+ spin_unlock_irqrestore(&d->arch.irq_lock, flags);
+ }
 }
 
 extern void dump_ioapic_irq_info(void);
Index: 2008年09月19日/xen/arch/x86/msi.c
===================================================================
--- 2008年09月19日.orig/xen/arch/x86/msi.c 2008年08月15日 16:18:55.000000000 +0200
+++ 2008年09月19日/xen/arch/x86/msi.c 2008年09月24日 09:19:47.000000000 +0200
@@ -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;
Index: 2008年09月19日/xen/arch/x86/physdev.c
===================================================================
--- 2008年09月19日.orig/xen/arch/x86/physdev.c 2008年09月24日 09:17:33.000000000 
+0200
+++ 2008年09月19日/xen/arch/x86/physdev.c 2008年09月24日 09:19:57.000000000 +0200
@@ -27,8 +27,6 @@ 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;
@@ -150,7 +148,7 @@ static int unmap_domain_pirq(struct doma
 
 vector = d->arch.pirq_vector[pirq];
 
- if ( !vector )
+ if ( vector <= 0 )
 {
 gdprintk(XENLOG_G_ERR, "domain %X: pirq %x not mapped still\n",
 d->domain_id, pirq);
@@ -160,18 +158,30 @@ static int unmap_domain_pirq(struct doma
 
 desc = &irq_desc[vector];
 spin_lock_irqsave(&desc->lock, flags);
+
+ if ( desc->status & IRQ_GUEST )
+ {
+ dprintk(XENLOG_G_WARNING, "dom%d: forcing unbind of pirq %d\n",
+ d->domain_id, pirq);
+ __pirq_guest_unbind(d, pirq, vector, flags);
+ vector = -vector;
+ }
+
 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;
+
 spin_unlock_irqrestore(&desc->lock, flags);
 
- d->arch.pirq_vector[pirq] = d->arch.vector_pirq[vector] = 0;
+ if ( vector > 0 )
+ d->arch.pirq_vector[pirq] = d->arch.vector_pirq[vector] = 0;
+ else
+ {
+ d->arch.pirq_vector[pirq] = vector;
+ d->arch.vector_pirq[-vector] = -pirq;
+ }
 
 ret = irq_deny_access(d, pirq);
 if ( ret )
@@ -244,7 +254,7 @@ static int physdev_map_pirq(struct physd
 }
 
 spin_lock_irqsave(&d->arch.irq_lock, flags);
- if ( map->pirq == -1 )
+ if ( map->pirq < 0 )
 {
 if ( d->arch.vector_pirq[vector] )
 {
@@ -252,6 +262,11 @@ static int physdev_map_pirq(struct physd
 map->index, map->pirq,
 d->arch.vector_pirq[vector]);
 pirq = d->arch.vector_pirq[vector];
+ if ( pirq < 0 )
+ {
+ ret = -EBUSY;
+ goto done;
+ }
 }
 else
 {
Index: 2008年09月19日/xen/include/asm-x86/irq.h
===================================================================
--- 2008年09月19日.orig/xen/include/asm-x86/irq.h 2008年09月24日 09:17:33.000000000 
+0200
+++ 2008年09月19日/xen/include/asm-x86/irq.h 2008年09月23日 15:25:53.000000000 
+0200
@@ -52,6 +52,10 @@ extern atomic_t irq_mis_count;
 int pirq_acktype(struct domain *d, int irq);
 int pirq_shared(struct domain *d , int irq);
 
+/* May only be called be irq_desc[vector].lock held. */
+void __pirq_guest_unbind(struct domain *d, int irq, unsigned int vector,
+ unsigned long flags);
+
 extern int domain_irq_to_vector(struct domain *d, int irq);
 extern int domain_vector_to_irq(struct domain *d, int vector);
 #endif /* _ASM_HW_IRQ_H */
Index: 2008年09月19日/xen/include/asm-x86/msi.h
===================================================================
--- 2008年09月19日.orig/xen/include/asm-x86/msi.h 2008年08月15日 16:18:55.000000000 
+0200
+++ 2008年09月19日/xen/include/asm-x86/msi.h 2008年09月24日 09:21:44.000000000 
+0200
@@ -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
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel