git: fa5f94140a83 - main - msi: handle error from BUS_REMAP_INTR in msi_assign_cpu

The branch main has been updated by emaste:

URL: https://cgit.FreeBSD.org/src/commit/?id=fa5f94140a83b4704c654ababd67cd9addb7cd29

commit fa5f94140a83b4704c654ababd67cd9addb7cd29
Author:     Ed Maste <emaste@FreeBSD.org>
AuthorDate: 2023-08-14 16:56:12 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2023-08-18 00:03:48 +0000

    msi: handle error from BUS_REMAP_INTR in msi_assign_cpu
    
    Previously errors from BUS_REMAP_INTR were silently ignored, and we
    ended up with non-functional interrupts.
    
    Now we allocate and enable new vectors, but postpone assignment of new
    APIC IDs and vectors where we can, until after BUS_REMAP_INTR is
    successful.  We then disable and free the old vectors.
    
    If BUS_REMAP_INTR fails we restore the old configuration, and disable
    and free the new, unused vectors.
    
    Thanks to AMD for providing hardware (with APIC IDs above 255) for
    testing.
    
    Reviewed by:    jhb
    MFC after:      1 month
    Sponsored by:   The FreeBSD Foundation
    Differential Revision: https://reviews.freebsd.org/D41455
---
 sys/x86/x86/msi.c | 47 ++++++++++++++++++++++++++++++++---------------
 1 file changed, 32 insertions(+), 15 deletions(-)

diff --git a/sys/x86/x86/msi.c b/sys/x86/x86/msi.c
index 8751c621a5e1..7f4d87c09453 100644
--- a/sys/x86/x86/msi.c
+++ b/sys/x86/x86/msi.c
@@ -244,7 +244,7 @@ msi_assign_cpu(struct intsrc *isrc, u_int apic_id)
 	struct msi_intsrc *sib, *msi = (struct msi_intsrc *)isrc;
 	int old_vector;
 	u_int old_id;
-	int i, vector;
+	int error, i, vector;
 
 	/*
 	 * Only allow CPUs to be assigned to the first message for an
@@ -274,31 +274,48 @@ msi_assign_cpu(struct intsrc *isrc, u_int apic_id)
 	if (vector == 0)
 		return (ENOSPC);
 
+	/* Must be set before BUS_REMAP_INTR as it may call back into MSI. */
 	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,
-		    msi->msi_cpu, msi->msi_vector);
 	for (i = 1; i < msi->msi_count; i++) {
 		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",
-			    sib->msi_irq, sib->msi_cpu, sib->msi_vector);
+			apic_enable_vector(apic_id, vector + i);
 	}
-	BUS_REMAP_INTR(device_get_parent(msi->msi_dev), msi->msi_dev,
+	error = BUS_REMAP_INTR(device_get_parent(msi->msi_dev), msi->msi_dev,
 	    msi->msi_irq);
+	if (error == 0) {
+		if (bootverbose) {
+			printf("msi: Assigning %s IRQ %d to local APIC %u vector %u\n",
+			    msi->msi_msix ? "MSI-X" : "MSI", msi->msi_irq,
+			    msi->msi_cpu, msi->msi_vector);
+		}
+		for (i = 1; i < msi->msi_count; i++) {
+			sib = (struct msi_intsrc *)intr_lookup_source(
+			    msi->msi_irqs[i]);
+			sib->msi_cpu = apic_id;
+			sib->msi_vector = vector + i;
+			if (bootverbose)
+				printf("msi: Assigning MSI IRQ %d to local APIC %u vector %u\n",
+				    sib->msi_irq, sib->msi_cpu,
+				    sib->msi_vector);
+		}
+	} else {
+		device_printf(msi->msi_dev,
+		    "remap irq %u to APIC ID %u failed (error %d)\n",
+		    msi->msi_irq, apic_id, error);
+		msi->msi_cpu = old_id;
+		msi->msi_vector = old_vector;
+		old_id = apic_id;
+		old_vector = vector;
+	}
 
 	/*
 	 * Free the old vector after the new one is established.  This is done
-	 * to prevent races where we could miss an interrupt.
+	 * to prevent races where we could miss an interrupt.  If BUS_REMAP_INTR
+	 * failed then we disable and free the new, unused vector(s).
 	 */
 	if (msi->msi_intsrc.is_handlers > 0)
 		apic_disable_vector(old_id, old_vector);
@@ -309,7 +326,7 @@ msi_assign_cpu(struct intsrc *isrc, u_int apic_id)
 			apic_disable_vector(old_id, old_vector + i);
 		apic_free_vector(old_id, old_vector + i, msi->msi_irqs[i]);
 	}
-	return (0);
+	return (error);
 }
 
 void