After the per-CPU IDT changes, the IDT vector of an interrupt could change

when the interrupt was moved from one CPU to another.  If the interrupt was
enabled, then the old IDT vector needs to be disabled and the new IDT vector
needs to be enabled.  This was mostly masked prior to the recent MSI changes
since in the older code almost all allocated IDT vectors were already enabled
and the enabled vectors on the BSP during boot covered enough of the IDT
range.  However, after the MSI changes, MSI interrupts that were allocated
but not enabled (e.g. DRM with MSI) during boot could result in an allocated
IDT vector that wasn't enabled.  The round-robin at the end of boot could
place another interrupt at the same IDT vector without enabling the IDT
vector causing trap 30 faults.

Fix this by explicitly disabling/enabling the old and new IDT vectors for
enabled interrupt sources when moving an interrupt between CPUs via the
pic_assign_cpu() method.  While here, fix a bug in my earlier changes so
that an I/O APIC interrupt pin is left unchanged if ioapic_assign_cpu()
fails to allocate a new IDT vector and returns ENOSPC.

Approved by:	re (kensmith)

2 files changed, 22 insertions, 6 deletions

diff --git a/sys/i386/i386/io_apic.c b/sys/i386/i386/io_apic.c
index 87d5481..32e31d8d 100644
--- a/sys/i386/i386/io_apic.c
+++ b/sys/i386/i386/io_apic.c
@@ -327,7 +327,7 @@ ioapic_assign_cpu(struct intsrc *isrc, u_int apic_id)
 {
 	struct ioapic_intsrc *intpin = (struct ioapic_intsrc *)isrc;
 	struct ioapic *io = (struct ioapic *)isrc->is_pic;
-	u_int old_vector;
+	u_int old_vector, new_vector;
 	u_int old_id;
 
 	/*
@@ -348,11 +348,14 @@ ioapic_assign_cpu(struct intsrc *isrc, u_int apic_id)
 	 * 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 (intpin->io_vector == 0)
+	new_vector = apic_alloc_vector(apic_id, intpin->io_irq);
+	if (new_vector == 0)
 		return (ENOSPC);
 
+	intpin->io_cpu = apic_id;
+	intpin->io_vector = new_vector;
+	if (isrc->is_handlers > 0)
+		apic_enable_vector(intpin->io_cpu, intpin->io_vector);
 	if (bootverbose) {
 		printf("ioapic%u: routing intpin %u (", io->io_id,
 		    intpin->io_intpin);
@@ -365,8 +368,11 @@ ioapic_assign_cpu(struct intsrc *isrc, u_int apic_id)
 	 * 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)
+	if (old_vector) {
+		if (isrc->is_handlers > 0)
+			apic_disable_vector(old_id, old_vector);
 		apic_free_vector(old_id, old_vector, intpin->io_irq);
+	}
 	return (0);
 }
 
diff --git a/sys/i386/i386/msi.c b/sys/i386/i386/msi.c
index da5bedb..6a0a0c9 100644
--- a/sys/i386/i386/msi.c
+++ b/sys/i386/i386/msi.c
@@ -230,6 +230,8 @@ msi_assign_cpu(struct intsrc *isrc, u_int apic_id)
 
 	msi->msi_cpu = apic_id;
 	msi->msi_vector = vector;
+	if (msi->msi_intsrc.is_handlers > 0)
+		apic_enable_vector(msi->msi_cpu, msi->msi_vector);
 	if (bootverbose)
 		printf("msi: Assigning %s IRQ %d to local APIC %u vector %u\n",
 		    msi->msi_msix ? "MSI-X" : "MSI", msi->msi_irq,
@@ -238,6 +240,8 @@ msi_assign_cpu(struct intsrc *isrc, u_int apic_id)
 		sib = (struct msi_intsrc *)intr_lookup_source(msi->msi_irqs[i]);
 		sib->msi_cpu = apic_id;
 		sib->msi_vector = vector + i;
+		if (sib->msi_intsrc.is_handlers > 0)
+			apic_enable_vector(sib->msi_cpu, sib->msi_vector);
 		if (bootverbose)
 			printf(
 		    "msi: Assigning MSI IRQ %d to local APIC %u vector %u\n",
@@ -249,9 +253,15 @@ msi_assign_cpu(struct intsrc *isrc, u_int apic_id)
 	 * Free the old vector after the new one is established.  This is done
 	 * to prevent races where we could miss an interrupt.
 	 */
+	if (msi->msi_intsrc.is_handlers > 0)
+		apic_disable_vector(old_id, old_vector);
 	apic_free_vector(old_id, old_vector, msi->msi_irq);
-	for (i = 1; i < msi->msi_count; i++)
+	for (i = 1; i < msi->msi_count; i++) {
+		sib = (struct msi_intsrc *)intr_lookup_source(msi->msi_irqs[i]);
+		if (sib->msi_intsrc.is_handlers > 0)
+			apic_disable_vector(old_id, old_vector + i);
 		apic_free_vector(old_id, old_vector + i, msi->msi_irqs[i]);
+	}
 	return (0);
 }