From owner-svn-src-head@FreeBSD.ORG Mon Jul 6 18:23:01 2009 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 5E717106567F; Mon, 6 Jul 2009 18:23:01 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id 4BD648FC0A; Mon, 6 Jul 2009 18:23:01 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.3/8.14.3) with ESMTP id n66IN1XE089087; Mon, 6 Jul 2009 18:23:01 GMT (envelope-from jhb@svn.freebsd.org) Received: (from jhb@localhost) by svn.freebsd.org (8.14.3/8.14.3/Submit) id n66IN1up089082; Mon, 6 Jul 2009 18:23:01 GMT (envelope-from jhb@svn.freebsd.org) Message-Id: <200907061823.n66IN1up089082@svn.freebsd.org> From: John Baldwin Date: Mon, 6 Jul 2009 18:23:01 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r195415 - in head/sys: amd64/amd64 i386/i386 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 06 Jul 2009 18:23:02 -0000 Author: jhb Date: Mon Jul 6 18:23:00 2009 New Revision: 195415 URL: http://svn.freebsd.org/changeset/base/195415 Log: 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) Modified: head/sys/amd64/amd64/io_apic.c head/sys/amd64/amd64/msi.c head/sys/i386/i386/io_apic.c head/sys/i386/i386/msi.c Modified: head/sys/amd64/amd64/io_apic.c ============================================================================== --- head/sys/amd64/amd64/io_apic.c Mon Jul 6 18:18:27 2009 (r195414) +++ head/sys/amd64/amd64/io_apic.c Mon Jul 6 18:23:00 2009 (r195415) @@ -327,7 +327,7 @@ ioapic_assign_cpu(struct intsrc *isrc, u { 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 * 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 * 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); } Modified: head/sys/amd64/amd64/msi.c ============================================================================== --- head/sys/amd64/amd64/msi.c Mon Jul 6 18:18:27 2009 (r195414) +++ head/sys/amd64/amd64/msi.c Mon Jul 6 18:23:00 2009 (r195415) @@ -230,6 +230,8 @@ msi_assign_cpu(struct intsrc *isrc, u_in 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_in 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_in * 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); } Modified: head/sys/i386/i386/io_apic.c ============================================================================== --- head/sys/i386/i386/io_apic.c Mon Jul 6 18:18:27 2009 (r195414) +++ head/sys/i386/i386/io_apic.c Mon Jul 6 18:23:00 2009 (r195415) @@ -327,7 +327,7 @@ ioapic_assign_cpu(struct intsrc *isrc, u { 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 * 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 * 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); } Modified: head/sys/i386/i386/msi.c ============================================================================== --- head/sys/i386/i386/msi.c Mon Jul 6 18:18:27 2009 (r195414) +++ head/sys/i386/i386/msi.c Mon Jul 6 18:23:00 2009 (r195415) @@ -230,6 +230,8 @@ msi_assign_cpu(struct intsrc *isrc, u_in 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_in 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_in * 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); }