Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 18 Feb 2009 07:46:17 -0500
From:      Andrew Gallatin <gallatin@cs.duke.edu>
To:        "Paul B. Mahol" <onemda@gmail.com>
Cc:        current@freebsd.org
Subject:   Re: r187880 causes fatal trap 30 when unloading drivers
Message-ID:  <499C0319.9040100@cs.duke.edu>
In-Reply-To: <3a142e750902180256p68ef8fc0nbda773d41d42a7a0@mail.gmail.com>
References:  <20090217142615.A77950@grasshopper.cs.duke.edu> <3a142e750902180256p68ef8fc0nbda773d41d42a7a0@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format.
--------------070105040609080801090900
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

Paul B. Mahol wrote:

> Fatal trap 30: reserved (unknown) fault while in kernel modecpuid = 1;

<...>

> The only workaroud is to disable SMP.

Try backing out the commit in question and see if that helps.
I used the attached patch on yesterday's -current.

Drew

--------------070105040609080801090900
Content-Type: text/x-diff;
 name="idt.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="idt.diff"

Index: i386/include/apicvar.h
===================================================================
--- i386/include/apicvar.h	(revision 187880)
+++ i386/include/apicvar.h	(revision 187879)
@@ -187,17 +187,14 @@
 	IDTVEC(apic_isr7), IDTVEC(spuriousint), IDTVEC(timerint);
 
 extern vm_paddr_t lapic_paddr;
-extern int apic_cpuids[];
 
-u_int	apic_alloc_vector(u_int apic_id, u_int irq);
-u_int	apic_alloc_vectors(u_int apic_id, u_int *irqs, u_int count,
-	    u_int align);
-void	apic_disable_vector(u_int apic_id, u_int vector);
-void	apic_enable_vector(u_int apic_id, u_int vector);
-void	apic_free_vector(u_int apic_id, u_int vector, u_int irq);
-u_int	apic_idt_to_irq(u_int apic_id, u_int vector);
+u_int	apic_alloc_vector(u_int irq);
+u_int	apic_alloc_vectors(u_int *irqs, u_int count, u_int align);
+void	apic_disable_vector(u_int vector);
+void	apic_enable_vector(u_int vector);
+void	apic_free_vector(u_int vector, u_int irq);
+u_int	apic_idt_to_irq(u_int vector);
 void	apic_register_enumerator(struct apic_enumerator *enumerator);
-u_int	apic_cpuid(u_int apic_id);
 void	*ioapic_create(vm_paddr_t addr, int32_t apic_id, int intbase);
 int	ioapic_disable_pin(void *cookie, u_int pin);
 int	ioapic_get_vector(void *cookie, u_int pin);
Index: i386/include/intr_machdep.h
===================================================================
--- i386/include/intr_machdep.h	(revision 187880)
+++ i386/include/intr_machdep.h	(revision 187879)
@@ -47,7 +47,7 @@
  * IRQ values beyond 256 are used by MSI.  We leave 255 unused to avoid
  * confusion since 255 is used in PCI to indicate an invalid IRQ.
  */
-#define	NUM_MSI_INTS	512
+#define	NUM_MSI_INTS	128
 #define	FIRST_MSI_INT	256
 #define	NUM_IO_INTS	(FIRST_MSI_INT + NUM_MSI_INTS)
 
Index: i386/i386/io_apic.c
===================================================================
--- i386/i386/io_apic.c	(revision 187880)
+++ i386/i386/io_apic.c	(revision 187879)
@@ -327,56 +327,39 @@
 {
 	struct ioapic_intsrc *intpin = (struct ioapic_intsrc *)isrc;
 	struct ioapic *io = (struct ioapic *)isrc->is_pic;
-	u_int old_vector;
-	u_int old_id;
 
-	/*
-	 * keep 1st core as the destination for NMI
-	 */
-	if (intpin->io_irq == IRQ_NMI)
-		apic_id = 0;
-
-	/*
-	 * Set us up to free the old irq.
-	 */
-	old_vector = intpin->io_vector;
-	old_id = intpin->io_cpu;
-	if (old_vector && apic_id == old_id)
-		return;
-
-	/*
-	 * Allocate an APIC vector for this interrupt pin.  Once
-	 * we have a vector we program the interrupt pin.
-	 */
 	intpin->io_cpu = apic_id;
-	intpin->io_vector = apic_alloc_vector(apic_id, intpin->io_irq);
 	if (bootverbose) {
-		printf("ioapic%u: routing intpin %u (", io->io_id,
-		    intpin->io_intpin);
+		printf("ioapic%u: Assigning ", io->io_id);
 		ioapic_print_irq(intpin);
-		printf(") to lapic %u vector %u\n", intpin->io_cpu,
-		    intpin->io_vector);
+		printf(" to local APIC %u\n", intpin->io_cpu);
 	}
 	ioapic_program_intpin(intpin);
-	/*
-	 * Free the old vector after the new one is established.  This is done
-	 * to prevent races where we could miss an interrupt.
-	 */
-	if (old_vector)
-		apic_free_vector(old_id, old_vector, intpin->io_irq);
 }
 
 static void
 ioapic_enable_intr(struct intsrc *isrc)
 {
 	struct ioapic_intsrc *intpin = (struct ioapic_intsrc *)isrc;
+	struct ioapic *io = (struct ioapic *)isrc->is_pic;
 
-	if (intpin->io_vector == 0)
-		ioapic_assign_cpu(isrc, pcpu_find(0)->pc_apic_id);
-	apic_enable_vector(intpin->io_cpu, intpin->io_vector);
+	if (intpin->io_vector == 0) {
+		/*
+		 * Allocate an APIC vector for this interrupt pin.  Once
+		 * we have a vector we program the interrupt pin.
+		 */
+		intpin->io_vector = apic_alloc_vector(intpin->io_irq);
+		if (bootverbose) {
+			printf("ioapic%u: routing intpin %u (", io->io_id,
+			    intpin->io_intpin);
+			ioapic_print_irq(intpin);
+			printf(") to vector %u\n", intpin->io_vector);
+		}
+		ioapic_program_intpin(intpin);
+		apic_enable_vector(intpin->io_vector);
+	}
 }
 
-
 static void
 ioapic_disable_intr(struct intsrc *isrc)
 {
@@ -386,11 +369,11 @@
 	if (intpin->io_vector != 0) {
 		/* Mask this interrupt pin and free its APIC vector. */
 		vector = intpin->io_vector;
-		apic_disable_vector(intpin->io_cpu, vector);
+		apic_disable_vector(vector);
 		intpin->io_masked = 1;
 		intpin->io_vector = 0;
 		ioapic_program_intpin(intpin);
-		apic_free_vector(intpin->io_cpu, vector, intpin->io_irq);
+		apic_free_vector(vector, intpin->io_irq);
 	}
 }
 
Index: i386/i386/mp_machdep.c
===================================================================
--- i386/i386/mp_machdep.c	(revision 187880)
+++ i386/i386/mp_machdep.c	(revision 187879)
@@ -206,7 +206,6 @@
 	int	cpu_disabled:1;
 } static cpu_info[MAX_APIC_ID + 1];
 int cpu_apic_ids[MAXCPU];
-int apic_cpuids[MAX_APIC_ID + 1];
 
 /* Holds pending bitmap based IPIs per CPU */
 static volatile u_int cpu_ipi_pending[MAXCPU];
@@ -398,7 +397,6 @@
 		KASSERT(boot_cpu_id == PCPU_GET(apic_id),
 		    ("BSP's APIC ID doesn't match boot_cpu_id"));
 	cpu_apic_ids[0] = boot_cpu_id;
-	apic_cpuids[boot_cpu_id] = 0;
 
 	assign_cpu_ids();
 
@@ -707,7 +705,6 @@
 
 		if (mp_ncpus < MAXCPU) {
 			cpu_apic_ids[mp_ncpus] = i;
-			apic_cpuids[i] = mp_ncpus;
 			mp_ncpus++;
 		} else
 			cpu_info[i].cpu_disabled = 1;
Index: i386/i386/local_apic.c
===================================================================
--- i386/i386/local_apic.c	(revision 187880)
+++ i386/i386/local_apic.c	(revision 187879)
@@ -46,8 +46,6 @@
 #include <sys/lock.h>
 #include <sys/mutex.h>
 #include <sys/pcpu.h>
-#include <sys/proc.h>
-#include <sys/sched.h>
 #include <sys/smp.h>
 
 #include <vm/vm.h>
@@ -111,8 +109,6 @@
 	u_long la_hard_ticks;
 	u_long la_stat_ticks;
 	u_long la_prof_ticks;
-	/* Include IDT_SYSCALL to make indexing easier. */
-	u_int la_ioint_irqs[APIC_NUM_IOINTS + 1];
 } static lapics[MAX_APIC_ID + 1];
 
 /* XXX: should thermal be an NMI? */
@@ -138,6 +134,8 @@
 	IDTVEC(apic_isr7),	/* 224 - 255 */
 };
 
+/* Include IDT_SYSCALL to make indexing easier. */
+static u_int ioint_irqs[APIC_NUM_IOINTS + 1];
 
 static u_int32_t lapic_timer_divisors[] = { 
 	APIC_TDCR_1, APIC_TDCR_2, APIC_TDCR_4, APIC_TDCR_8, APIC_TDCR_16,
@@ -218,6 +216,7 @@
 
 	/* Perform basic initialization of the BSP's local APIC. */
 	lapic_enable();
+	ioint_irqs[IDT_SYSCALL - APIC_IO_INTS] = IRQ_SYSCALL;
 
 	/* Set BSP's per-CPU local APIC ID. */
 	PCPU_SET(apic_id, lapic_id());
@@ -225,6 +224,7 @@
 	/* Local APIC timer interrupt. */
 	setidt(APIC_TIMER_INT, IDTVEC(timerint), SDT_SYS386IGT, SEL_KPL,
 	    GSEL(GCODE_SEL, SEL_KPL));
+	ioint_irqs[APIC_TIMER_INT - APIC_IO_INTS] = IRQ_TIMER;
 
 	/* XXX: error/thermal interrupts */
 }
@@ -256,9 +256,6 @@
 		lapics[apic_id].la_lvts[i] = lvts[i];
 		lapics[apic_id].la_lvts[i].lvt_active = 0;
 	}
-	lapics[apic_id].la_ioint_irqs[IDT_SYSCALL - APIC_IO_INTS] = IRQ_SYSCALL;
-	lapics[apic_id].la_ioint_irqs[APIC_TIMER_INT - APIC_IO_INTS] =
-	    IRQ_TIMER;
 
 #ifdef SMP
 	cpu_add(apic_id, boot_cpu);
@@ -669,8 +666,7 @@
 
 	if (vector == -1)
 		panic("Couldn't get vector from ISR!");
-	isrc = intr_lookup_source(apic_idt_to_irq(PCPU_GET(apic_id),
-	    vector));
+	isrc = intr_lookup_source(apic_idt_to_irq(vector));
 	intr_execute_handlers(isrc, frame);
 }
 
@@ -785,19 +781,9 @@
 	lapic->lvt_timer = value;
 }
 
-u_int
-apic_cpuid(u_int apic_id)
-{
-#ifdef SMP
-	return apic_cpuids[apic_id];
-#else
-	return 0;
-#endif
-}
-
 /* Request a free IDT vector to be used by the specified IRQ. */
 u_int
-apic_alloc_vector(u_int apic_id, u_int irq)
+apic_alloc_vector(u_int irq)
 {
 	u_int vector;
 
@@ -809,9 +795,9 @@
 	 */
 	mtx_lock_spin(&icu_lock);
 	for (vector = 0; vector < APIC_NUM_IOINTS; vector++) {
-		if (lapics[apic_id].la_ioint_irqs[vector] != 0)
+		if (ioint_irqs[vector] != 0)
 			continue;
-		lapics[apic_id].la_ioint_irqs[vector] = irq;
+		ioint_irqs[vector] = irq;
 		mtx_unlock_spin(&icu_lock);
 		return (vector + APIC_IO_INTS);
 	}
@@ -826,7 +812,7 @@
  * satisfied, 0 is returned.
  */
 u_int
-apic_alloc_vectors(u_int apic_id, u_int *irqs, u_int count, u_int align)
+apic_alloc_vectors(u_int *irqs, u_int count, u_int align)
 {
 	u_int first, run, vector;
 
@@ -849,7 +835,7 @@
 	for (vector = 0; vector < APIC_NUM_IOINTS; vector++) {
 
 		/* Vector is in use, end run. */
-		if (lapics[apic_id].la_ioint_irqs[vector] != 0) {
+		if (ioint_irqs[vector] != 0) {
 			run = 0;
 			first = 0;
 			continue;
@@ -869,8 +855,7 @@
 
 		/* Found a run, assign IRQs and return the first vector. */
 		for (vector = 0; vector < count; vector++)
-			lapics[apic_id].la_ioint_irqs[first + vector] =
-			    irqs[vector];
+			ioint_irqs[first + vector] = irqs[vector];
 		mtx_unlock_spin(&icu_lock);
 		return (first + APIC_IO_INTS);
 	}
@@ -879,14 +864,8 @@
 	return (0);
 }
 
-/*
- * Enable a vector for a particular apic_id.  Since all lapics share idt
- * entries and ioint_handlers this enables the vector on all lapics.  lapics
- * which do not have the vector configured would report spurious interrupts
- * should it fire.
- */
 void
-apic_enable_vector(u_int apic_id, u_int vector)
+apic_enable_vector(u_int vector)
 {
 
 	KASSERT(vector != IDT_SYSCALL, ("Attempt to overwrite syscall entry"));
@@ -897,7 +876,7 @@
 }
 
 void
-apic_disable_vector(u_int apic_id, u_int vector)
+apic_disable_vector(u_int vector)
 {
 
 	KASSERT(vector != IDT_SYSCALL, ("Attempt to overwrite syscall entry"));
@@ -909,42 +888,27 @@
 
 /* Release an APIC vector when it's no longer in use. */
 void
-apic_free_vector(u_int apic_id, u_int vector, u_int irq)
+apic_free_vector(u_int vector, u_int irq)
 {
-	struct thread *td;
 	KASSERT(vector >= APIC_IO_INTS && vector != IDT_SYSCALL &&
 	    vector <= APIC_IO_INTS + APIC_NUM_IOINTS,
 	    ("Vector %u does not map to an IRQ line", vector));
 	KASSERT(irq < NUM_IO_INTS, ("Invalid IRQ %u", irq));
-	KASSERT(lapics[apic_id].la_ioint_irqs[vector - APIC_IO_INTS] ==
-	    irq, ("IRQ mismatch"));
-
-	/*
-	 * Bind us to the cpu that owned the vector before freeing it so
-	 * we don't lose an interrupt delivery race.
-	 */
-	td = curthread;
-	thread_lock(td);
-	if (sched_is_bound(td))
-		panic("apic_free_vector: Thread already bound.\n");
-	sched_bind(td, apic_cpuid(apic_id));
+	KASSERT(ioint_irqs[vector - APIC_IO_INTS] == irq, ("IRQ mismatch"));
 	mtx_lock_spin(&icu_lock);
-	lapics[apic_id].la_ioint_irqs[vector - APIC_IO_INTS] = 0;
+	ioint_irqs[vector - APIC_IO_INTS] = 0;
 	mtx_unlock_spin(&icu_lock);
-	sched_unbind(td);
-	thread_unlock(td);
-
 }
 
 /* Map an IDT vector (APIC) to an IRQ (interrupt source). */
 u_int
-apic_idt_to_irq(u_int apic_id, u_int vector)
+apic_idt_to_irq(u_int vector)
 {
 
 	KASSERT(vector >= APIC_IO_INTS && vector != IDT_SYSCALL &&
 	    vector <= APIC_IO_INTS + APIC_NUM_IOINTS,
 	    ("Vector %u does not map to an IRQ line", vector));
-	return (lapics[apic_id].la_ioint_irqs[vector - APIC_IO_INTS]);
+	return (ioint_irqs[vector - APIC_IO_INTS]);
 }
 
 #ifdef DDB
@@ -955,7 +919,6 @@
 {
 	struct intsrc *isrc;
 	int i, verbose;
-	u_int apic_id;
 	u_int irq;
 
 	if (strcmp(modif, "vv") == 0)
@@ -964,14 +927,9 @@
 		verbose = 1;
 	else
 		verbose = 0;
-	for (apic_id = 0; apic_id <= MAX_APIC_ID; apic_id++) {
-		if (lapics[apic_id].la_present == 0)
-			continue;
-		db_printf("Interrupts bound to lapic %u\n", apic_id);
-		for (i = 0; i < APIC_NUM_IOINTS + 1 && !db_pager_quit; i++) {
-			irq = lapics[apic_id].la_ioint_irqs[i];
-			if (irq == 0 || irq == IRQ_SYSCALL)
-				continue;
+	for (i = 0; i < APIC_NUM_IOINTS + 1 && !db_pager_quit; i++) {
+		irq = ioint_irqs[i];
+		if (irq != 0 && irq != IRQ_SYSCALL) {
 			db_printf("vec 0x%2x -> ", i + APIC_IO_INTS);
 			if (irq == IRQ_TIMER)
 				db_printf("lapic timer\n");
Index: i386/i386/msi.c
===================================================================
--- i386/i386/msi.c	(revision 187880)
+++ i386/i386/msi.c	(revision 187879)
@@ -161,9 +161,7 @@
 {
 	struct msi_intsrc *msi = (struct msi_intsrc *)isrc;
 
-	if (msi->msi_vector == 0)
-		msi_assign_cpu(isrc, 0);
-	apic_enable_vector(msi->msi_cpu, msi->msi_vector);
+	apic_enable_vector(msi->msi_vector);
 }
 
 static void
@@ -171,7 +169,7 @@
 {
 	struct msi_intsrc *msi = (struct msi_intsrc *)isrc;
 
-	apic_disable_vector(msi->msi_cpu, msi->msi_vector);
+	apic_disable_vector(msi->msi_vector);
 }
 
 static int
@@ -201,35 +199,15 @@
 msi_assign_cpu(struct intsrc *isrc, u_int apic_id)
 {
 	struct msi_intsrc *msi = (struct msi_intsrc *)isrc;
-	int old_vector;
-	u_int old_id;
-	int vector;
 
-	/* Store information to free existing irq. */
-	old_vector = msi->msi_vector;
-	old_id = msi->msi_cpu;
-	if (old_vector && old_id == apic_id)
-		return;
-	/* Allocate IDT vector on this cpu. */
-	vector = apic_alloc_vector(apic_id, msi->msi_irq);
-	if (vector == 0)
-		return; /* XXX alloc_vector panics on failure. */
 	msi->msi_cpu = apic_id;
-	msi->msi_vector = vector;
 	if (bootverbose)
-		printf("msi: Assigning %s IRQ %d to local APIC %u vector %u\n",
+		printf("msi: Assigning %s IRQ %d to local APIC %u\n",
 		    msi->msi_msix ? "MSI-X" : "MSI", msi->msi_irq,
-		    msi->msi_cpu, msi->msi_vector);
+		    msi->msi_cpu);	
 	pci_remap_msi_irq(msi->msi_dev, msi->msi_irq);
-	/*
-	 * Free the old vector after the new one is established.  This is done
-	 * to prevent races where we could miss an interrupt.
-	 */
-	if (old_vector)
-		apic_free_vector(old_id, old_vector, msi->msi_irq);
 }
 
-
 void
 msi_init(void)
 {
@@ -285,7 +263,7 @@
 msi_alloc(device_t dev, int count, int maxcount, int *irqs)
 {
 	struct msi_intsrc *msi, *fsrc;
-	int cnt, i;
+	int cnt, i, vector;
 
 	if (!msi_enabled)
 		return (ENXIO);
@@ -331,12 +309,22 @@
 	/* Ok, we now have the IRQs allocated. */
 	KASSERT(cnt == count, ("count mismatch"));
 
+	/* Allocate 'count' IDT vectors. */
+	vector = apic_alloc_vectors(irqs, count, maxcount);
+	if (vector == 0) {
+		mtx_unlock(&msi_lock);
+		return (ENOSPC);
+	}
+
 	/* Assign IDT vectors and make these messages owned by 'dev'. */
 	fsrc = (struct msi_intsrc *)intr_lookup_source(irqs[0]);
 	for (i = 0; i < count; i++) {
 		msi = (struct msi_intsrc *)intr_lookup_source(irqs[i]);
 		msi->msi_dev = dev;
-		msi->msi_vector = 0;
+		msi->msi_vector = vector + i;
+		if (bootverbose)
+			printf("msi: routing MSI IRQ %d to vector %u\n",
+			    msi->msi_irq, msi->msi_vector);
 		msi->msi_first = fsrc;
 		KASSERT(msi->msi_intsrc.is_handlers == 0,
 		    ("dead MSI has handlers"));
@@ -389,18 +377,14 @@
 		KASSERT(msi->msi_dev == first->msi_dev, ("owner mismatch"));
 		msi->msi_first = NULL;
 		msi->msi_dev = NULL;
-		if (msi->msi_vector)
-			apic_free_vector(msi->msi_cpu, msi->msi_vector,
-			    msi->msi_irq);
+		apic_free_vector(msi->msi_vector, msi->msi_irq);
 		msi->msi_vector = 0;
 	}
 
 	/* Clear out the first message. */
 	first->msi_first = NULL;
 	first->msi_dev = NULL;
-	if (first->msi_vector)
-		apic_free_vector(first->msi_cpu, first->msi_vector,
-		    first->msi_irq);
+	apic_free_vector(first->msi_vector, first->msi_irq);
 	first->msi_vector = 0;
 	first->msi_count = 0;
 
@@ -449,7 +433,7 @@
 msix_alloc(device_t dev, int *irq)
 {
 	struct msi_intsrc *msi;
-	int i;
+	int i, vector;
 
 	if (!msi_enabled)
 		return (ENXIO);
@@ -484,9 +468,15 @@
 		goto again;
 	}
 
+	/* Allocate an IDT vector. */
+	vector = apic_alloc_vector(i);
+	if (bootverbose)
+		printf("msi: routing MSI-X IRQ %d to vector %u\n", msi->msi_irq,
+		    vector);
+
 	/* Setup source. */
 	msi->msi_dev = dev;
-	msi->msi_vector = 0;
+	msi->msi_vector = vector;
 	msi->msi_msix = 1;
 
 	KASSERT(msi->msi_intsrc.is_handlers == 0, ("dead MSI-X has handlers"));
@@ -518,8 +508,7 @@
 
 	/* Clear out the message. */
 	msi->msi_dev = NULL;
-	if (msi->msi_vector)
-		apic_free_vector(msi->msi_cpu, msi->msi_vector, msi->msi_irq);
+	apic_free_vector(msi->msi_vector, msi->msi_irq);
 	msi->msi_vector = 0;
 	msi->msi_msix = 0;
 
Index: amd64/include/apicvar.h
===================================================================
--- amd64/include/apicvar.h	(revision 187880)
+++ amd64/include/apicvar.h	(revision 187879)
@@ -176,17 +176,14 @@
 	IDTVEC(apic_isr7), IDTVEC(spuriousint), IDTVEC(timerint);
 
 extern vm_paddr_t lapic_paddr;
-extern int apic_cpuids[];
 
-u_int	apic_alloc_vector(u_int apic_id, u_int irq);
-u_int	apic_alloc_vectors(u_int apic_id, u_int *irqs, u_int count,
-	    u_int align);
-void	apic_disable_vector(u_int apic_id, u_int vector);
-void	apic_enable_vector(u_int apic_id, u_int vector);
-void	apic_free_vector(u_int apic_id, u_int vector, u_int irq);
-u_int	apic_idt_to_irq(u_int apic_id, u_int vector);
+u_int	apic_alloc_vector(u_int irq);
+u_int	apic_alloc_vectors(u_int *irqs, u_int count, u_int align);
+void	apic_disable_vector(u_int vector);
+void	apic_enable_vector(u_int vector);
+void	apic_free_vector(u_int vector, u_int irq);
+u_int	apic_idt_to_irq(u_int vector);
 void	apic_register_enumerator(struct apic_enumerator *enumerator);
-u_int	apic_cpuid(u_int apic_id);
 void	*ioapic_create(vm_paddr_t addr, int32_t apic_id, int intbase);
 int	ioapic_disable_pin(void *cookie, u_int pin);
 int	ioapic_get_vector(void *cookie, u_int pin);
Index: amd64/include/intr_machdep.h
===================================================================
--- amd64/include/intr_machdep.h	(revision 187880)
+++ amd64/include/intr_machdep.h	(revision 187879)
@@ -47,7 +47,7 @@
  * IRQ values beyond 256 are used by MSI.  We leave 255 unused to avoid
  * confusion since 255 is used in PCI to indicate an invalid IRQ.
  */
-#define	NUM_MSI_INTS	512
+#define	NUM_MSI_INTS	128
 #define	FIRST_MSI_INT	256
 #define	NUM_IO_INTS	(FIRST_MSI_INT + NUM_MSI_INTS)
 
Index: amd64/amd64/io_apic.c
===================================================================
--- amd64/amd64/io_apic.c	(revision 187880)
+++ amd64/amd64/io_apic.c	(revision 187879)
@@ -327,56 +327,39 @@
 {
 	struct ioapic_intsrc *intpin = (struct ioapic_intsrc *)isrc;
 	struct ioapic *io = (struct ioapic *)isrc->is_pic;
-	u_int old_vector;
-	u_int old_id;
 
-	/*
-	 * keep 1st core as the destination for NMI
-	 */
-	if (intpin->io_irq == IRQ_NMI)
-		apic_id = 0;
-
-	/*
-	 * Set us up to free the old irq.
-	 */
-	old_vector = intpin->io_vector;
-	old_id = intpin->io_cpu;
-	if (old_vector && apic_id == old_id)
-		return;
-
-	/*
-	 * Allocate an APIC vector for this interrupt pin.  Once
-	 * we have a vector we program the interrupt pin.
-	 */
 	intpin->io_cpu = apic_id;
-	intpin->io_vector = apic_alloc_vector(apic_id, intpin->io_irq);
 	if (bootverbose) {
-		printf("ioapic%u: routing intpin %u (", io->io_id,
-		    intpin->io_intpin);
+		printf("ioapic%u: Assigning ", io->io_id);
 		ioapic_print_irq(intpin);
-		printf(") to lapic %u vector %u\n", intpin->io_cpu,
-		    intpin->io_vector);
+		printf(" to local APIC %u\n", intpin->io_cpu);
 	}
 	ioapic_program_intpin(intpin);
-	/*
-	 * Free the old vector after the new one is established.  This is done
-	 * to prevent races where we could miss an interrupt.
-	 */
-	if (old_vector)
-		apic_free_vector(old_id, old_vector, intpin->io_irq);
 }
 
 static void
 ioapic_enable_intr(struct intsrc *isrc)
 {
 	struct ioapic_intsrc *intpin = (struct ioapic_intsrc *)isrc;
+	struct ioapic *io = (struct ioapic *)isrc->is_pic;
 
-	if (intpin->io_vector == 0)
-		ioapic_assign_cpu(isrc, pcpu_find(0)->pc_apic_id);
-	apic_enable_vector(intpin->io_cpu, intpin->io_vector);
+	if (intpin->io_vector == 0) {
+		/*
+		 * Allocate an APIC vector for this interrupt pin.  Once
+		 * we have a vector we program the interrupt pin.
+		 */
+		intpin->io_vector = apic_alloc_vector(intpin->io_irq);
+		if (bootverbose) {
+			printf("ioapic%u: routing intpin %u (", io->io_id,
+			    intpin->io_intpin);
+			ioapic_print_irq(intpin);
+			printf(") to vector %u\n", intpin->io_vector);
+		}
+		ioapic_program_intpin(intpin);
+		apic_enable_vector(intpin->io_vector);
+	}
 }
 
-
 static void
 ioapic_disable_intr(struct intsrc *isrc)
 {
@@ -386,11 +369,11 @@
 	if (intpin->io_vector != 0) {
 		/* Mask this interrupt pin and free its APIC vector. */
 		vector = intpin->io_vector;
-		apic_disable_vector(intpin->io_cpu, vector);
+		apic_disable_vector(vector);
 		intpin->io_masked = 1;
 		intpin->io_vector = 0;
 		ioapic_program_intpin(intpin);
-		apic_free_vector(intpin->io_cpu, vector, intpin->io_irq);
+		apic_free_vector(vector, intpin->io_irq);
 	}
 }
 
Index: amd64/amd64/mp_machdep.c
===================================================================
--- amd64/amd64/mp_machdep.c	(revision 187880)
+++ amd64/amd64/mp_machdep.c	(revision 187879)
@@ -152,7 +152,6 @@
 	int	cpu_disabled:1;
 } static cpu_info[MAX_APIC_ID + 1];
 int cpu_apic_ids[MAXCPU];
-int apic_cpuids[MAX_APIC_ID + 1];
 
 /* Holds pending bitmap based IPIs per CPU */
 static volatile u_int cpu_ipi_pending[MAXCPU];
@@ -350,7 +349,6 @@
 		KASSERT(boot_cpu_id == PCPU_GET(apic_id),
 		    ("BSP's APIC ID doesn't match boot_cpu_id"));
 	cpu_apic_ids[0] = boot_cpu_id;
-	apic_cpuids[boot_cpu_id] = 0;
 
 	assign_cpu_ids();
 
@@ -658,7 +656,6 @@
 
 		if (mp_ncpus < MAXCPU) {
 			cpu_apic_ids[mp_ncpus] = i;
-			apic_cpuids[i] = mp_ncpus;
 			mp_ncpus++;
 		} else
 			cpu_info[i].cpu_disabled = 1;
Index: amd64/amd64/local_apic.c
===================================================================
--- amd64/amd64/local_apic.c	(revision 187880)
+++ amd64/amd64/local_apic.c	(revision 187879)
@@ -46,8 +46,6 @@
 #include <sys/lock.h>
 #include <sys/mutex.h>
 #include <sys/pcpu.h>
-#include <sys/proc.h>
-#include <sys/sched.h>
 #include <sys/smp.h>
 
 #include <vm/vm.h>
@@ -111,8 +109,6 @@
 	u_long la_hard_ticks;
 	u_long la_stat_ticks;
 	u_long la_prof_ticks;
-	/* Include IDT_SYSCALL to make indexing easier. */
-	u_int la_ioint_irqs[APIC_NUM_IOINTS + 1];
 } static lapics[MAX_APIC_ID + 1];
 
 /* XXX: should thermal be an NMI? */
@@ -138,6 +134,8 @@
 	IDTVEC(apic_isr7),	/* 224 - 255 */
 };
 
+/* Include IDT_SYSCALL to make indexing easier. */
+static u_int ioint_irqs[APIC_NUM_IOINTS + 1];
 
 static u_int32_t lapic_timer_divisors[] = { 
 	APIC_TDCR_1, APIC_TDCR_2, APIC_TDCR_4, APIC_TDCR_8, APIC_TDCR_16,
@@ -217,12 +215,14 @@
 
 	/* Perform basic initialization of the BSP's local APIC. */
 	lapic_enable();
+	ioint_irqs[IDT_SYSCALL - APIC_IO_INTS] = IRQ_SYSCALL;
 
 	/* Set BSP's per-CPU local APIC ID. */
 	PCPU_SET(apic_id, lapic_id());
 
 	/* Local APIC timer interrupt. */
 	setidt(APIC_TIMER_INT, IDTVEC(timerint), SDT_SYSIGT, SEL_KPL, 0);
+	ioint_irqs[APIC_TIMER_INT - APIC_IO_INTS] = IRQ_TIMER;
 
 	/* XXX: error/thermal interrupts */
 }
@@ -254,9 +254,6 @@
 		lapics[apic_id].la_lvts[i] = lvts[i];
 		lapics[apic_id].la_lvts[i].lvt_active = 0;
 	}
-	lapics[apic_id].la_ioint_irqs[IDT_SYSCALL - APIC_IO_INTS] = IRQ_SYSCALL;
-	lapics[apic_id].la_ioint_irqs[APIC_TIMER_INT - APIC_IO_INTS] =
-	    IRQ_TIMER;
 
 #ifdef SMP
 	cpu_add(apic_id, boot_cpu);
@@ -667,8 +664,7 @@
 
 	if (vector == -1)
 		panic("Couldn't get vector from ISR!");
-	isrc = intr_lookup_source(apic_idt_to_irq(PCPU_GET(apic_id),
-	    vector));
+	isrc = intr_lookup_source(apic_idt_to_irq(vector));
 	intr_execute_handlers(isrc, frame);
 }
 
@@ -783,19 +779,9 @@
 	lapic->lvt_timer = value;
 }
 
-u_int
-apic_cpuid(u_int apic_id)
-{
-#ifdef SMP
-	return apic_cpuids[apic_id];
-#else
-	return 0;
-#endif
-}
-
 /* Request a free IDT vector to be used by the specified IRQ. */
 u_int
-apic_alloc_vector(u_int apic_id, u_int irq)
+apic_alloc_vector(u_int irq)
 {
 	u_int vector;
 
@@ -807,9 +793,9 @@
 	 */
 	mtx_lock_spin(&icu_lock);
 	for (vector = 0; vector < APIC_NUM_IOINTS; vector++) {
-		if (lapics[apic_id].la_ioint_irqs[vector] != 0)
+		if (ioint_irqs[vector] != 0)
 			continue;
-		lapics[apic_id].la_ioint_irqs[vector] = irq;
+		ioint_irqs[vector] = irq;
 		mtx_unlock_spin(&icu_lock);
 		return (vector + APIC_IO_INTS);
 	}
@@ -824,7 +810,7 @@
  * satisfied, 0 is returned.
  */
 u_int
-apic_alloc_vectors(u_int apic_id, u_int *irqs, u_int count, u_int align)
+apic_alloc_vectors(u_int *irqs, u_int count, u_int align)
 {
 	u_int first, run, vector;
 
@@ -847,7 +833,7 @@
 	for (vector = 0; vector < APIC_NUM_IOINTS; vector++) {
 
 		/* Vector is in use, end run. */
-		if (lapics[apic_id].la_ioint_irqs[vector] != 0) {
+		if (ioint_irqs[vector] != 0) {
 			run = 0;
 			first = 0;
 			continue;
@@ -867,8 +853,7 @@
 
 		/* Found a run, assign IRQs and return the first vector. */
 		for (vector = 0; vector < count; vector++)
-			lapics[apic_id].la_ioint_irqs[first + vector] =
-			    irqs[vector];
+			ioint_irqs[first + vector] = irqs[vector];
 		mtx_unlock_spin(&icu_lock);
 		return (first + APIC_IO_INTS);
 	}
@@ -877,14 +862,8 @@
 	return (0);
 }
 
-/*
- * Enable a vector for a particular apic_id.  Since all lapics share idt
- * entries and ioint_handlers this enables the vector on all lapics.  lapics
- * which do not have the vector configured would report spurious interrupts
- * should it fire.
- */
 void
-apic_enable_vector(u_int apic_id, u_int vector)
+apic_enable_vector(u_int vector)
 {
 
 	KASSERT(vector != IDT_SYSCALL, ("Attempt to overwrite syscall entry"));
@@ -894,7 +873,7 @@
 }
 
 void
-apic_disable_vector(u_int apic_id, u_int vector)
+apic_disable_vector(u_int vector)
 {
 
 	KASSERT(vector != IDT_SYSCALL, ("Attempt to overwrite syscall entry"));
@@ -905,42 +884,27 @@
 
 /* Release an APIC vector when it's no longer in use. */
 void
-apic_free_vector(u_int apic_id, u_int vector, u_int irq)
+apic_free_vector(u_int vector, u_int irq)
 {
-	struct thread *td;
 	KASSERT(vector >= APIC_IO_INTS && vector != IDT_SYSCALL &&
 	    vector <= APIC_IO_INTS + APIC_NUM_IOINTS,
 	    ("Vector %u does not map to an IRQ line", vector));
 	KASSERT(irq < NUM_IO_INTS, ("Invalid IRQ %u", irq));
-	KASSERT(lapics[apic_id].la_ioint_irqs[vector - APIC_IO_INTS] ==
-	    irq, ("IRQ mismatch"));
-
-	/*
-	 * Bind us to the cpu that owned the vector before freeing it so
-	 * we don't lose an interrupt delivery race.
-	 */
-	td = curthread;
-	thread_lock(td);
-	if (sched_is_bound(td))
-		panic("apic_free_vector: Thread already bound.\n");
-	sched_bind(td, apic_cpuid(apic_id));
+	KASSERT(ioint_irqs[vector - APIC_IO_INTS] == irq, ("IRQ mismatch"));
 	mtx_lock_spin(&icu_lock);
-	lapics[apic_id].la_ioint_irqs[vector - APIC_IO_INTS] = 0;
+	ioint_irqs[vector - APIC_IO_INTS] = 0;
 	mtx_unlock_spin(&icu_lock);
-	sched_unbind(td);
-	thread_unlock(td);
-
 }
 
 /* Map an IDT vector (APIC) to an IRQ (interrupt source). */
 u_int
-apic_idt_to_irq(u_int apic_id, u_int vector)
+apic_idt_to_irq(u_int vector)
 {
 
 	KASSERT(vector >= APIC_IO_INTS && vector != IDT_SYSCALL &&
 	    vector <= APIC_IO_INTS + APIC_NUM_IOINTS,
 	    ("Vector %u does not map to an IRQ line", vector));
-	return (lapics[apic_id].la_ioint_irqs[vector - APIC_IO_INTS]);
+	return (ioint_irqs[vector - APIC_IO_INTS]);
 }
 
 #ifdef DDB
@@ -951,7 +915,6 @@
 {
 	struct intsrc *isrc;
 	int i, verbose;
-	u_int apic_id;
 	u_int irq;
 
 	if (strcmp(modif, "vv") == 0)
@@ -960,14 +923,9 @@
 		verbose = 1;
 	else
 		verbose = 0;
-	for (apic_id = 0; apic_id <= MAX_APIC_ID; apic_id++) {
-		if (lapics[apic_id].la_present == 0)
-			continue;
-		db_printf("Interrupts bound to lapic %u\n", apic_id);
-		for (i = 0; i < APIC_NUM_IOINTS + 1 && !db_pager_quit; i++) {
-			irq = lapics[apic_id].la_ioint_irqs[i];
-			if (irq == 0 || irq == IRQ_SYSCALL)
-				continue;
+	for (i = 0; i < APIC_NUM_IOINTS + 1 && !db_pager_quit; i++) {
+		irq = ioint_irqs[i];
+		if (irq != 0 && irq != IRQ_SYSCALL) {
 			db_printf("vec 0x%2x -> ", i + APIC_IO_INTS);
 			if (irq == IRQ_TIMER)
 				db_printf("lapic timer\n");
Index: amd64/amd64/msi.c
===================================================================
--- amd64/amd64/msi.c	(revision 187880)
+++ amd64/amd64/msi.c	(revision 187879)
@@ -161,9 +161,7 @@
 {
 	struct msi_intsrc *msi = (struct msi_intsrc *)isrc;
 
-	if (msi->msi_vector == 0)
-		msi_assign_cpu(isrc, 0);
-	apic_enable_vector(msi->msi_cpu, msi->msi_vector);
+	apic_enable_vector(msi->msi_vector);
 }
 
 static void
@@ -171,7 +169,7 @@
 {
 	struct msi_intsrc *msi = (struct msi_intsrc *)isrc;
 
-	apic_disable_vector(msi->msi_cpu, msi->msi_vector);
+	apic_disable_vector(msi->msi_vector);
 }
 
 static int
@@ -201,35 +199,15 @@
 msi_assign_cpu(struct intsrc *isrc, u_int apic_id)
 {
 	struct msi_intsrc *msi = (struct msi_intsrc *)isrc;
-	int old_vector;
-	u_int old_id;
-	int vector;
 
-	/* Store information to free existing irq. */
-	old_vector = msi->msi_vector;
-	old_id = msi->msi_cpu;
-	if (old_vector && old_id == apic_id)
-		return;
-	/* Allocate IDT vector on this cpu. */
-	vector = apic_alloc_vector(apic_id, msi->msi_irq);
-	if (vector == 0)
-		return; /* XXX alloc_vector panics on failure. */
 	msi->msi_cpu = apic_id;
-	msi->msi_vector = vector;
 	if (bootverbose)
-		printf("msi: Assigning %s IRQ %d to local APIC %u vector %u\n",
+		printf("msi: Assigning %s IRQ %d to local APIC %u\n",
 		    msi->msi_msix ? "MSI-X" : "MSI", msi->msi_irq,
-		    msi->msi_cpu, msi->msi_vector);
+		    msi->msi_cpu);	
 	pci_remap_msi_irq(msi->msi_dev, msi->msi_irq);
-	/*
-	 * Free the old vector after the new one is established.  This is done
-	 * to prevent races where we could miss an interrupt.
-	 */
-	if (old_vector)
-		apic_free_vector(old_id, old_vector, msi->msi_irq);
 }
 
-
 void
 msi_init(void)
 {
@@ -285,7 +263,7 @@
 msi_alloc(device_t dev, int count, int maxcount, int *irqs)
 {
 	struct msi_intsrc *msi, *fsrc;
-	int cnt, i;
+	int cnt, i, vector;
 
 	if (!msi_enabled)
 		return (ENXIO);
@@ -331,12 +309,22 @@
 	/* Ok, we now have the IRQs allocated. */
 	KASSERT(cnt == count, ("count mismatch"));
 
+	/* Allocate 'count' IDT vectors. */
+	vector = apic_alloc_vectors(irqs, count, maxcount);
+	if (vector == 0) {
+		mtx_unlock(&msi_lock);
+		return (ENOSPC);
+	}
+
 	/* Assign IDT vectors and make these messages owned by 'dev'. */
 	fsrc = (struct msi_intsrc *)intr_lookup_source(irqs[0]);
 	for (i = 0; i < count; i++) {
 		msi = (struct msi_intsrc *)intr_lookup_source(irqs[i]);
 		msi->msi_dev = dev;
-		msi->msi_vector = 0;
+		msi->msi_vector = vector + i;
+		if (bootverbose)
+			printf("msi: routing MSI IRQ %d to vector %u\n",
+			    msi->msi_irq, msi->msi_vector);
 		msi->msi_first = fsrc;
 		KASSERT(msi->msi_intsrc.is_handlers == 0,
 		    ("dead MSI has handlers"));
@@ -389,18 +377,14 @@
 		KASSERT(msi->msi_dev == first->msi_dev, ("owner mismatch"));
 		msi->msi_first = NULL;
 		msi->msi_dev = NULL;
-		if (msi->msi_vector)
-			apic_free_vector(msi->msi_cpu, msi->msi_vector,
-			    msi->msi_irq);
+		apic_free_vector(msi->msi_vector, msi->msi_irq);
 		msi->msi_vector = 0;
 	}
 
 	/* Clear out the first message. */
 	first->msi_first = NULL;
 	first->msi_dev = NULL;
-	if (first->msi_vector)
-		apic_free_vector(first->msi_cpu, first->msi_vector,
-		    first->msi_irq);
+	apic_free_vector(first->msi_vector, first->msi_irq);
 	first->msi_vector = 0;
 	first->msi_count = 0;
 
@@ -449,7 +433,7 @@
 msix_alloc(device_t dev, int *irq)
 {
 	struct msi_intsrc *msi;
-	int i;
+	int i, vector;
 
 	if (!msi_enabled)
 		return (ENXIO);
@@ -484,9 +468,15 @@
 		goto again;
 	}
 
+	/* Allocate an IDT vector. */
+	vector = apic_alloc_vector(i);
+	if (bootverbose)
+		printf("msi: routing MSI-X IRQ %d to vector %u\n", msi->msi_irq,
+		    vector);
+
 	/* Setup source. */
 	msi->msi_dev = dev;
-	msi->msi_vector = 0;
+	msi->msi_vector = vector;
 	msi->msi_msix = 1;
 
 	KASSERT(msi->msi_intsrc.is_handlers == 0, ("dead MSI-X has handlers"));
@@ -518,8 +508,7 @@
 
 	/* Clear out the message. */
 	msi->msi_dev = NULL;
-	if (msi->msi_vector)
-		apic_free_vector(msi->msi_cpu, msi->msi_vector, msi->msi_irq);
+	apic_free_vector(msi->msi_vector, msi->msi_irq);
 	msi->msi_vector = 0;
 	msi->msi_msix = 0;
 

--------------070105040609080801090900--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?499C0319.9040100>