Current-Users archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Interrupt routing patch for testing



Hi all,
attached is a critical patch that potentially effects all x86 users
(beside Xen domU). It changes the way interrupt slots are allocated.

The old code could allocate two different interrupt vectors for the same
hardware interrupt when IOAPIC is used and the BIOS doesn't initialise
all devices. This typically happened for cbb0 on newer laptops.

With the patch, the code is simplified and I can actually start on
making the round robin scheduling work :-)

Please test the patch on as much systems as you can. If you run into
problems, e.g. system hangs, please provide me (offlist) with a dmesg of
a kernel build with -DINTRDEBUG and MPVERBOSE option w/ and w/o the
patch.

Thanks,
Joerg
Index: intr.c
===================================================================
RCS file: /data/repo/netbsd/src/sys/arch/x86/x86/intr.c,v
retrieving revision 1.49
diff -u -p -r1.49 intr.c
--- intr.c      7 May 2008 07:00:16 -0000       1.49
+++ intr.c      8 May 2008 21:07:09 -0000
@@ -468,28 +468,33 @@ static int
 intr_allocate_slot_cpu(struct cpu_info *ci, struct pic *pic, int pin,
                       int *index)
 {
-       int start, slot, i;
+       int slot, i;
        struct intrsource *isp;
 
-       start = CPU_IS_PRIMARY(ci) ? NUM_LEGACY_IRQS : 0;
-       slot = -1;
+       if (pic == &i8259_pic) {
+               if (!CPU_IS_PRIMARY(ci))
+                       return EBUSY;
+               slot = pin;
+               mutex_enter(&x86_intr_lock);
+       } else {
+               slot = -1;
 
-       mutex_enter(&x86_intr_lock);
-       for (i = start; i < MAX_INTR_SOURCES ; i++) {
-               isp = ci->ci_isources[i];
-               if (isp != NULL && isp->is_pic == pic && isp->is_pin == pin) {
-                       slot = i;
-                       break;
+               mutex_enter(&x86_intr_lock);
+               /*
+                * intr_allocate_slot has checked for an existing mapping.
+                * Now look for a free slot.
+                */
+               for (i = 0; i < MAX_INTR_SOURCES ; i++) {
+                       if (ci->ci_isources[i] == NULL) {
+                               slot = i;
+                               break;
+                       }
                }
-               if (isp == NULL && slot == -1) {
-                       slot = i;
-                       continue;
+               if (slot == -1) {
+                       mutex_exit(&x86_intr_lock);
+                       return EBUSY;
                }
        }
-       if (slot == -1) {
-               mutex_exit(&x86_intr_lock);
-               return EBUSY;
-       }
 
        isp = ci->ci_isources[slot];
        if (isp == NULL) {
@@ -515,7 +520,7 @@ intr_allocate_slot_cpu(struct cpu_info *
  * A simple round-robin allocator to assign interrupts to CPUs.
  */
 static int
-intr_allocate_slot(struct pic *pic, int legacy_irq, int pin, int level,
+intr_allocate_slot(struct pic *pic, int pin, int level,
                   struct cpu_info **cip, int *index, int *idt_slot)
 {
        CPU_INFO_ITERATOR cii;
@@ -523,69 +528,44 @@ intr_allocate_slot(struct pic *pic, int 
        struct intrsource *isp;
        int slot, idtvec, error;
 
-       /*
-        * If a legacy IRQ is wanted, try to use a fixed slot pointing
-        * at the primary CPU. In the case of IO APICs, multiple pins
-        * may map to one legacy IRQ, but they should not be shared
-        * in that case, so the first one gets the legacy slot, but
-        * a subsequent allocation with a different pin will get
-        * a different slot.
-        */
-       if (legacy_irq != -1) {
-               ci = &cpu_info_primary;
-               /* must check for duplicate pic + pin first */
+       /* First check if this pin is already used by an interrupt vector. */
+       for (CPU_INFO_FOREACH(cii, ci)) {
                for (slot = 0 ; slot < MAX_INTR_SOURCES ; slot++) {
-                       isp = ci->ci_isources[slot];
-                       if (isp != NULL && isp->is_pic == pic &&
-                           isp->is_pin == pin ) {
-                               goto duplicate;
-                       }
-               }
-               slot = legacy_irq;
-               isp = ci->ci_isources[slot];
-               if (isp == NULL) {
-                       MALLOC(isp, struct intrsource *,
-                           sizeof (struct intrsource), M_DEVBUF,
-                            M_NOWAIT|M_ZERO);
-                       if (isp == NULL)
-                               return ENOMEM;
-                       snprintf(isp->is_evname, sizeof (isp->is_evname),
-                           "pin %d", pin);
-                       evcnt_attach_dynamic(&isp->is_evcnt, EVCNT_TYPE_INTR,
-                           NULL, device_xname(&pic->pic_dev), isp->is_evname);
-                       mutex_enter(&x86_intr_lock);
-                       ci->ci_isources[slot] = isp;
-                       mutex_exit(&x86_intr_lock);
-               } else {
-                       if (isp->is_pin != pin) {
-                               if (pic == &i8259_pic)
-                                       return EINVAL;
-                               goto other;
+                       if ((isp = ci->ci_isources[slot]) == NULL)
+                               continue;
+                       if (isp->is_pic == pic && isp->is_pin == pin) {
+                               *idt_slot = isp->is_idtvec;
+                               *index = slot;
+                               *cip = ci;
+                               return 0;
                        }
                }
-duplicate:
-               if (pic == &i8259_pic)
-                       idtvec = ICU_OFFSET + legacy_irq;
-               else {
-                       if (isp->is_minlevel == 0 || level < isp->is_minlevel) {
-                               idtvec = idt_vec_alloc(APIC_LEVEL(level),
-                                   IDT_INTR_HIGH);
-                               if (idtvec == 0)
-                                       return EBUSY;
-                       } else
-                               idtvec = isp->is_idtvec;
-               }
-       } else {
-other:
-               /*
-                * Otherwise, look for a free slot elsewhere. Do the primary
-                * CPU first.
-                */
-               ci = &cpu_info_primary;
-               error = intr_allocate_slot_cpu(ci, pic, pin, &slot);
-               if (error == 0)
-                       goto found;
+       }
 
+       /*
+        * The pic/pin combination doesn't have an existing mapping.
+        * Find a slot for a new interrupt source and allocate an IDT
+        * vector.
+        *
+        * For the i8259 case, this always uses the reserved slots
+        * of the primary CPU and fixed IDT vectors.  This is required
+        * by other parts of the code, see x86/intr.h for more details.
+        *
+        * For the IOAPIC case, interrupts are assigned to the
+        * primary CPU by default, until it runs out of slots.
+        *
+        * PIC and APIC usage are essentially exclusive, so the reservation
+        * of the ISA slots is ignored when assigning IOAPIC slots.
+        *
+        * XXX Fix interrupt allocation to Application Processors.
+        * XXX Check how many interrupts each CPU got and assign it to
+        * XXX the least loaded CPU.  Consider adding options to bind
+        * XXX interrupts to specific CPUs.
+        * XXX Drop apic level support, just assign IDT vectors sequentially.
+        */
+       ci = &cpu_info_primary;
+       error = intr_allocate_slot_cpu(ci, pic, pin, &slot);
+       if (error != 0) {
                /*
                 * ..now try the others.
                 */
@@ -594,19 +574,27 @@ other:
                                continue;
                        error = intr_allocate_slot_cpu(ci, pic, pin, &slot);
                        if (error == 0)
-                               goto found;
+                               break;
                }
-               return EBUSY;
-found:
-               idtvec = idt_vec_alloc(APIC_LEVEL(level), IDT_INTR_HIGH);
-               if (idtvec == 0) {
-                       mutex_enter(&x86_intr_lock);
-                       FREE(ci->ci_isources[slot], M_DEVBUF);
-                       ci->ci_isources[slot] = NULL;
-                       mutex_exit(&x86_intr_lock);
+               if (error != 0)
                        return EBUSY;
-               }
        }
+
+       if (pic == &i8259_pic)
+               idtvec = ICU_OFFSET + pin;
+       else
+               idtvec = idt_vec_alloc(APIC_LEVEL(level), IDT_INTR_HIGH);
+
+       if (idtvec == 0) {
+               mutex_enter(&x86_intr_lock);
+               evcnt_detach(&ci->ci_isources[slot]->is_evcnt);
+               FREE(ci->ci_isources[slot], M_DEVBUF);
+               ci->ci_isources[slot] = NULL;
+               mutex_exit(&x86_intr_lock);
+               return EBUSY;
+       }
+
+       ci->ci_isources[slot]->is_idtvec = idtvec;
        *idt_slot = idtvec;
        *index = slot;
        *cip = ci;
@@ -673,7 +661,7 @@ intr_establish(int legacy_irq, struct pi
                panic("intr_establish: non-legacy IRQ on i8259");
 #endif
 
-       error = intr_allocate_slot(pic, legacy_irq, pin, level, &ci, &slot,
+       error = intr_allocate_slot(pic, pin, level, &ci, &slot,
            &idt_vec);
        if (error != 0) {
                printf("failed to allocate interrupt slot for PIC %s pin %d\n",


Home | Main Index | Thread Index | Old Index