>>> 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
|
Home