[Xen-devel] [PATCH] x86/IO-APIC: fix guest RTE write corner cases

["Jan Beulich" <JBeulich@xxxxxxxx>]

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@xxxxxxxx>

--- a/xen/arch/x86/io_apic.c
+++ b/xen/arch/x86/io_apic.c
@@ -133,6 +133,37 @@ static void add_pin_to_irq(unsigned int 
     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.
  */
@@ -2280,7 +2311,7 @@ int ioapic_guest_read(unsigned long phys
 
 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_desc *desc;
@@ -2348,13 +2379,25 @@ int ioapic_guest_write(unsigned long phy
         return 0;
     }
 
-    if ( desc->arch.vector <= 0 || desc->arch.vector > LAST_DYNAMIC_VECTOR ) {
-        add_pin_to_irq(irq, apic, pin);
-        vector = assign_irq_vector(irq, NULL);
-        if ( vector < 0 )
-            return vector;
+    if ( desc->arch.vector <= 0 || desc->arch.vector > LAST_DYNAMIC_VECTOR )
+    {
+        int vector = desc->arch.vector;
+
+        if ( vector < FIRST_HIPRIORITY_VECTOR )
+            add_pin_to_irq(irq, apic, pin);
+        else
+            desc->arch.vector = IRQ_VECTOR_UNASSIGNED;
+        ret = assign_irq_vector(irq, NULL);
+        if ( ret < 0 )
+        {
+            if ( vector < FIRST_HIPRIORITY_VECTOR )
+                remove_pin_from_irq(irq, apic, pin);
+            else
+                desc->arch.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(&dom0->event_lock);
     ret = map_domain_pirq(dom0, pirq, irq,

Attachment: x86-ioapic-unused-hiprio-irq.patch
Description: Text document

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel