x86/IO-APIC: fix guest RTE write corner cases

This fixes two regressions from c/s 20143:a7de5bd776ca ("x86: Make the
hypercall PHYSDEVOP_alloc_irq_vector hypercall dummy"):

For one, IRQs that had their vector set up by Xen internally without a
handler ever having got set (e.g. via "com<n>=..." without a matching
consumer option like "console=com<n>") would wrongly call
add_pin_to_irq() here, triggering the BUG_ON() in that function.

Second, when assign_irq_vector() fails this addition to irq_2_pin[]
needs to be undone.

In the context of this I'm also surprised that the irq_2_pin[]
manipulations here occur without any lock, i.e. rely on Dom0 to do
some sort of serialization.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Keir Fraser <keir@xen.org>
master commit: 30256a0ff17f6f3b1278b85103187341d5b0ac42
master date: 2013-05-15 10:52:02 +0200

diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c
index 4378d8e4dae344172c98434233b5bac70051911f..240da6e7585e1b5af10ce08489533ff03c81f45e 100644 (file)
--- a/xen/arch/x86/io_apic.c
+++ b/xen/arch/x86/io_apic.c
@@ -141,6 +141,37 @@ static void add_pin_to_irq(unsigned int irq, int apic, int pin)
     share_vector_maps(irq_2_pin[irq].apic, apic);
 }
 
+static void remove_pin_from_irq(unsigned int irq, int apic, int pin)
+{
+    struct irq_pin_list *entry, *prev;
+
+    for (entry = &irq_2_pin[irq]; ; entry = &irq_2_pin[entry->next]) {
+        if ((entry->apic == apic) && (entry->pin == pin))
+            break;
+        BUG_ON(!entry->next);
+    }
+
+    entry->pin = entry->apic = -1;
+
+    if (entry != &irq_2_pin[irq]) {
+        /* Removed entry is not at head of list. */
+        prev = &irq_2_pin[irq];
+        while (&irq_2_pin[prev->next] != entry)
+            prev = &irq_2_pin[prev->next];
+        prev->next = entry->next;
+    } else if (entry->next) {
+        /* Removed entry is at head of multi-item list. */
+        prev  = entry;
+        entry = &irq_2_pin[entry->next];
+        *prev = *entry;
+        entry->pin = entry->apic = -1;
+    } else
+        return;
+
+    entry->next = irq_2_pin_free_entry;
+    irq_2_pin_free_entry = entry - irq_2_pin;
+}
+
 /*
  * Reroute an IRQ to a different pin.
  */
@@ -2447,7 +2478,7 @@ int ioapic_guest_read(unsigned long physbase, unsigned int reg, u32 *pval)
 
 int ioapic_guest_write(unsigned long physbase, unsigned int reg, u32 val)
 {
-    int apic, pin, irq, ret, vector, pirq;
+    int apic, pin, irq, ret, pirq;
     struct IO_APIC_route_entry rte = { 0 };
     unsigned long flags;
     struct irq_cfg *cfg;
@@ -2517,13 +2548,25 @@ int ioapic_guest_write(unsigned long physbase, unsigned int reg, u32 val)
         return 0;
     }
 
-    if ( cfg->vector <= 0 || cfg->vector > LAST_DYNAMIC_VECTOR ) {
-        add_pin_to_irq(irq, apic, pin);
-        vector = assign_irq_vector(irq);
-        if ( vector < 0 )
-            return vector;
+    if ( cfg->vector <= 0 || cfg->vector > LAST_DYNAMIC_VECTOR )
+    {
+        int vector = cfg->vector;
+
+        if ( vector < FIRST_HIPRIORITY_VECTOR )
+            add_pin_to_irq(irq, apic, pin);
+        else
+            cfg->vector = IRQ_VECTOR_UNASSIGNED;
+        ret = assign_irq_vector(irq);
+        if ( ret < 0 )
+        {
+            if ( vector < FIRST_HIPRIORITY_VECTOR )
+                remove_pin_from_irq(irq, apic, pin);
+            else
+                cfg->vector = vector;
+            return ret;
+        }
 
-        printk(XENLOG_INFO "allocated vector %02x for irq %d\n", vector, irq);
+        printk(XENLOG_INFO "allocated vector %02x for irq %d\n", ret, irq);
     }
     spin_lock(&pcidevs_lock);
     spin_lock(&dom0->event_lock);

diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index 7e2c212dd537003340f38ddbd0462b7f62331fae..e91c069ce292a4a3413c169f0ea66bbac76b0600 100644 (file)
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -48,8 +48,6 @@ int __read_mostly *irq_status = NULL;
 #define IRQ_USED        (1)
 #define IRQ_RSVD        (2)
 
-#define IRQ_VECTOR_UNASSIGNED (0)
-
 static DECLARE_BITMAP(used_vectors, NR_VECTORS);
 
 struct irq_cfg __read_mostly *irq_cfg = NULL;

diff --git a/xen/include/asm-x86/irq.h b/xen/include/asm-x86/irq.h
index 5d4f7930763dc66ac19bbe6f0ff483a12494cf04..d688f1087e910addf7e3778bd65d2d2e2b8ffb7a 100644 (file)
--- a/xen/include/asm-x86/irq.h
+++ b/xen/include/asm-x86/irq.h
@@ -17,6 +17,8 @@
 
 #define MSI_IRQ(irq)       ((irq) >= nr_irqs_gsi && (irq) < nr_irqs)
 
+#define IRQ_VECTOR_UNASSIGNED 0
+
 #define LEGACY_VECTOR(irq)          ((irq) + FIRST_LEGACY_VECTOR)
 #define LEGACY_IRQ_FROM_VECTOR(vec) ((vec) - FIRST_LEGACY_VECTOR)