From 252b2b4f4fb7f7749f90c83e5e01ba68b5424ffa Mon Sep 17 00:00:00 2001 From: Scott Long Date: Sun, 30 Jul 2017 06:53:58 +0000 Subject: [PATCH] Split the interrupt setup code into two parts: allocation and configuration. Do the allocation before requesting the IOCFacts message. This triggers the LSI firmware to recognize the multiqueue should be enabled if available. Multiqueue isn't used by the driver yet, but this also fixes a problem with the cached IOCFacts not matching latter checks, leading to potential problems with error recovery. As a side-effect, fetch the driver tunables as early as possible. Reviewed by: slm Obtained from: Netflix Differential Revision: D9243 --- sys/dev/mpr/mpr.c | 4 +--- sys/dev/mpr/mpr_pci.c | 44 ++++++++++++++++++++++++++++++++++--------- sys/dev/mpr/mprvar.h | 2 ++ sys/dev/mps/mps.c | 4 +--- sys/dev/mps/mps_pci.c | 44 ++++++++++++++++++++++++++++++++++--------- sys/dev/mps/mpsvar.h | 2 ++ 6 files changed, 76 insertions(+), 24 deletions(-) diff --git a/sys/dev/mpr/mpr.c b/sys/dev/mpr/mpr.c index 67a2f6de802..0a50fb8e0ac 100644 --- a/sys/dev/mpr/mpr.c +++ b/sys/dev/mpr/mpr.c @@ -1482,7 +1482,7 @@ mpr_init_queues(struct mpr_softc *sc) * Next are the global settings, if they exist. Highest are the per-unit * settings, if they exist. */ -static void +void mpr_get_tunables(struct mpr_softc *sc) { char tmpstr[80]; @@ -1658,8 +1658,6 @@ mpr_attach(struct mpr_softc *sc) { int error; - mpr_get_tunables(sc); - MPR_FUNCTRACE(sc); mtx_init(&sc->mpr_mtx, "MPR lock", NULL, MTX_DEF); diff --git a/sys/dev/mpr/mpr_pci.c b/sys/dev/mpr/mpr_pci.c index 4b9f0aaa151..ce7e56c807d 100644 --- a/sys/dev/mpr/mpr_pci.c +++ b/sys/dev/mpr/mpr_pci.c @@ -69,6 +69,7 @@ static int mpr_pci_resume(device_t); static void mpr_pci_free(struct mpr_softc *); static int mpr_alloc_msix(struct mpr_softc *sc, int msgs); static int mpr_alloc_msi(struct mpr_softc *sc, int msgs); +static int mpr_pci_alloc_interrupts(struct mpr_softc *sc); static device_method_t mpr_methods[] = { DEVMETHOD(device_probe, mpr_pci_probe), @@ -191,6 +192,8 @@ mpr_pci_attach(device_t dev) m = mpr_find_ident(dev); sc->mpr_flags = m->flags; + mpr_get_tunables(sc); + /* Twiddle basic PCI config bits for a sanity check */ pci_enable_busmaster(dev); @@ -240,28 +243,51 @@ mpr_pci_attach(device_t dev) return (ENOMEM); } - if ((error = mpr_attach(sc)) != 0) + if (((error = mpr_pci_alloc_interrupts(sc)) != 0) || + ((error = mpr_attach(sc)) != 0)) mpr_pci_free(sc); return (error); } +/* + * Allocate, but don't assign interrupts early. Doing it before requesting + * the IOCFacts message informs the firmware that we want to do MSI-X + * multiqueue. We might not use all of the available messages, but there's + * no reason to re-alloc if we don't. + */ +int +mpr_pci_alloc_interrupts(struct mpr_softc *sc) +{ + device_t dev; + int error, msgs; + + dev = sc->mpr_dev; + error = 0; + + if ((sc->disable_msix == 0) && + ((msgs = pci_msix_count(dev)) >= MPR_MSI_COUNT)) + error = mpr_alloc_msix(sc, MPR_MSI_COUNT); + if ((error != 0) && (sc->disable_msi == 0) && + ((msgs = pci_msi_count(dev)) >= MPR_MSI_COUNT)) + error = mpr_alloc_msi(sc, MPR_MSI_COUNT); + else + msgs = 0; + + sc->msi_msgs = msgs; + return (error); +} + int mpr_pci_setup_interrupts(struct mpr_softc *sc) { device_t dev; - int i, error, msgs; + int i, error; dev = sc->mpr_dev; error = ENXIO; - if ((sc->disable_msix == 0) && - ((msgs = pci_msix_count(dev)) >= MPR_MSI_COUNT)) - error = mpr_alloc_msix(sc, MPR_MSI_COUNT); - if ((error != 0) && (sc->disable_msi == 0) && - ((msgs = pci_msi_count(dev)) >= MPR_MSI_COUNT)) - error = mpr_alloc_msi(sc, MPR_MSI_COUNT); - if (error != 0) { + if (sc->msi_msgs == 0) { sc->mpr_flags |= MPR_FLAGS_INTX; sc->mpr_irq_rid[0] = 0; sc->mpr_irq[0] = bus_alloc_resource_any(dev, SYS_RES_IRQ, diff --git a/sys/dev/mpr/mprvar.h b/sys/dev/mpr/mprvar.h index 1e97a75398c..bab76604549 100644 --- a/sys/dev/mpr/mprvar.h +++ b/sys/dev/mpr/mprvar.h @@ -278,6 +278,7 @@ struct mpr_softc { u_int mpr_debug; u_int disable_msix; u_int disable_msi; + int msi_msgs; u_int atomic_desc_capable; int tm_cmds_active; int io_cmds_active; @@ -702,6 +703,7 @@ mpr_unmask_intr(struct mpr_softc *sc) int mpr_pci_setup_interrupts(struct mpr_softc *sc); int mpr_pci_restore(struct mpr_softc *sc); +void mpr_get_tunables(struct mpr_softc *sc); int mpr_attach(struct mpr_softc *sc); int mpr_free(struct mpr_softc *sc); void mpr_intr(void *); diff --git a/sys/dev/mps/mps.c b/sys/dev/mps/mps.c index 8939d0e8587..3969d3e2767 100644 --- a/sys/dev/mps/mps.c +++ b/sys/dev/mps/mps.c @@ -1341,7 +1341,7 @@ mps_init_queues(struct mps_softc *sc) * Next are the global settings, if they exist. Highest are the per-unit * settings, if they exist. */ -static void +void mps_get_tunables(struct mps_softc *sc) { char tmpstr[80]; @@ -1513,8 +1513,6 @@ mps_attach(struct mps_softc *sc) { int error; - mps_get_tunables(sc); - MPS_FUNCTRACE(sc); mtx_init(&sc->mps_mtx, "MPT2SAS lock", NULL, MTX_DEF); diff --git a/sys/dev/mps/mps_pci.c b/sys/dev/mps/mps_pci.c index cee43760372..fbd8de01d8e 100644 --- a/sys/dev/mps/mps_pci.c +++ b/sys/dev/mps/mps_pci.c @@ -68,6 +68,7 @@ static int mps_pci_resume(device_t); static void mps_pci_free(struct mps_softc *); static int mps_alloc_msix(struct mps_softc *sc, int msgs); static int mps_alloc_msi(struct mps_softc *sc, int msgs); +static int mps_pci_alloc_interrupts(struct mps_softc *sc); static device_method_t mps_methods[] = { DEVMETHOD(device_probe, mps_pci_probe), @@ -191,6 +192,8 @@ mps_pci_attach(device_t dev) m = mps_find_ident(dev); sc->mps_flags = m->flags; + mps_get_tunables(sc); + /* Twiddle basic PCI config bits for a sanity check */ pci_enable_busmaster(dev); @@ -221,28 +224,51 @@ mps_pci_attach(device_t dev) return (ENOMEM); } - if ((error = mps_attach(sc)) != 0) + if (((error = mps_pci_alloc_interrupts(sc)) != 0) || + ((error = mps_attach(sc)) != 0)) mps_pci_free(sc); return (error); } +/* + * Allocate, but don't assign interrupts early. Doing it before requesting + * the IOCFacts message informs the firmware that we want to do MSI-X + * multiqueue. We might not use all of the available messages, but there's + * no reason to re-alloc if we don't. + */ +static int +mps_pci_alloc_interrupts(struct mps_softc *sc) +{ + device_t dev; + int error, msgs; + + dev = sc->mps_dev; + error = 0; + + if ((sc->disable_msix == 0) && + ((msgs = pci_msix_count(dev)) >= MPS_MSI_COUNT)) + error = mps_alloc_msix(sc, MPS_MSI_COUNT); + if ((error != 0) && (sc->disable_msi == 0) && + ((msgs = pci_msi_count(dev)) >= MPS_MSI_COUNT)) + error = mps_alloc_msi(sc, MPS_MSI_COUNT); + else + msgs = 0; + + sc->msi_msgs = msgs; + return (error); +} + int mps_pci_setup_interrupts(struct mps_softc *sc) { device_t dev; - int i, error, msgs; + int i, error; dev = sc->mps_dev; error = ENXIO; - if ((sc->disable_msix == 0) && - ((msgs = pci_msix_count(dev)) >= MPS_MSI_COUNT)) - error = mps_alloc_msix(sc, MPS_MSI_COUNT); - if ((error != 0) && (sc->disable_msi == 0) && - ((msgs = pci_msi_count(dev)) >= MPS_MSI_COUNT)) - error = mps_alloc_msi(sc, MPS_MSI_COUNT); - if (error != 0) { + if (sc->msi_msgs == 0) { sc->mps_flags |= MPS_FLAGS_INTX; sc->mps_irq_rid[0] = 0; sc->mps_irq[0] = bus_alloc_resource_any(dev, SYS_RES_IRQ, diff --git a/sys/dev/mps/mpsvar.h b/sys/dev/mps/mpsvar.h index 9b6d72f5a37..b7cc549d3a9 100644 --- a/sys/dev/mps/mpsvar.h +++ b/sys/dev/mps/mpsvar.h @@ -274,6 +274,7 @@ struct mps_softc { u_int mps_debug; u_int disable_msix; u_int disable_msi; + u_int msi_msgs; int tm_cmds_active; int io_cmds_active; int io_cmds_highwater; @@ -671,6 +672,7 @@ mps_unmask_intr(struct mps_softc *sc) int mps_pci_setup_interrupts(struct mps_softc *sc); int mps_pci_restore(struct mps_softc *sc); +void mps_get_tunables(struct mps_softc *sc); int mps_attach(struct mps_softc *sc); int mps_free(struct mps_softc *sc); void mps_intr(void *);