Skip site navigation (1)Skip section navigation (2) 
Date:      Sun, 17 Jun 2018 20:33:02 +0000 (UTC)
From:      Marius Strobl <marius@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r335303 - head/sys/net
Message-ID:  <201806172033.w5HKX23g021441@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: marius
Date: Sun Jun 17 20:33:02 2018
New Revision: 335303
URL: https://svnweb.freebsd.org/changeset/base/335303

Log:
  Assorted fixes to MSI-X/MSI/INTx setup in iflib(9):
  - In iflib_msix_init(), VMMs with broken MSI-X activation are trying
    to be worked around by manually enabling PCIM_MSIXCTRL_MSIX_ENABLE
    before calling pci_alloc_msix(9). Apart from constituting a layering
    violation, this has the problem of leaving PCIM_MSIXCTRL_MSIX_ENABLE
    enabled when falling back to MSI or INTx when e. g. MSI-X is black-
    listed and initially also when disabled via hw.pci.enable_msix. The
    later in turn was incorrectly worked around in r325166.
    Since r310806, pci(4) itself has code to deal with broken MSI-X
    handling of VMMs, so all of these workarounds in iflib(9) can go,
    fixing non-working interrupts when falling back to MSI/INTx. In
    any case, possibly further adjustments to broken MSI-X activation
    of VMMs like enabling r310806 by default in VM environments need to
    be placed into pci(4), not iflib(9). [1]
  - Also remove the pci_enable_busmaster(9) call from iflib_msix_init(),
    which is already more properly invoked from iflib_device_attach().
  - When falling back to MSI/INTx, release the MSI-X BAR resource again.
  - When falling back to INTx, ensure scctx->isc_vectors is set to 1 and
    not to something higher from a device with more than one MSI message
    supported.
  - Make the nearby ring_state(s) stuff (static) const.
  
  Discussed with:	jhb at BSDCan 2018 [1]
  Reviewed by:	imp, jhb
  Differential Revision:	https://reviews.freebsd.org/D15729

Modified:
  head/sys/net/iflib.c

Modified: head/sys/net/iflib.c
==============================================================================
--- head/sys/net/iflib.c	Sun Jun 17 20:32:43 2018	(r335302)
+++ head/sys/net/iflib.c	Sun Jun 17 20:33:02 2018	(r335303)
@@ -5879,49 +5879,11 @@ iflib_msix_init(if_ctx_t ctx)
 
 	bar = ctx->ifc_softc_ctx.isc_msix_bar;
 	admincnt = sctx->isc_admin_intrcnt;
-	/* Override by global tuneable */
-	{
-		int i;
-		size_t len = sizeof(i);
-		err = kernel_sysctlbyname(curthread, "hw.pci.enable_msix", &i, &len, NULL, 0, NULL, 0);
-		if (err == 0) {
-			if (i == 0)
-				goto msi;
-		}
-		else {
-			device_printf(dev, "unable to read hw.pci.enable_msix.");
-		}
-	}
 	/* Override by tuneable */
 	if (scctx->isc_disable_msix)
 		goto msi;
 
 	/*
-	** When used in a virtualized environment
-	** PCI BUSMASTER capability may not be set
-	** so explicity set it here and rewrite
-	** the ENABLE in the MSIX control register
-	** at this point to cause the host to
-	** successfully initialize us.
-	*/
-	{
-		int msix_ctrl, rid;
-
- 		pci_enable_busmaster(dev);
-		rid = 0;
-		if (pci_find_cap(dev, PCIY_MSIX, &rid) == 0 && rid != 0) {
-			rid += PCIR_MSIX_CTRL;
-			msix_ctrl = pci_read_config(dev, rid, 2);
-			msix_ctrl |= PCIM_MSIXCTRL_MSIX_ENABLE;
-			pci_write_config(dev, rid, msix_ctrl, 2);
-		} else {
-			device_printf(dev, "PCIY_MSIX capability not found; "
-			                   "or rid %d == 0.\n", rid);
-			goto msi;
-		}
-	}
-
-	/*
 	 * bar == -1 => "trust me I know what I'm doing"
 	 * Some drivers are for hardware that is so shoddily
 	 * documented that no one knows which bars are which
@@ -6007,6 +5969,9 @@ iflib_msix_init(if_ctx_t ctx)
 		return (vectors);
 	} else {
 		device_printf(dev, "failed to allocate %d msix vectors, err: %d - using MSI\n", vectors, err);
+		bus_release_resource(dev, SYS_RES_MEMORY, bar,
+		    ctx->ifc_msix_mem);
+		ctx->ifc_msix_mem = NULL;
 	}
 msi:
 	vectors = pci_msi_count(dev);
@@ -6017,6 +5982,7 @@ msi:
 		device_printf(dev,"Using an MSI interrupt\n");
 		scctx->isc_intr = IFLIB_INTR_MSI;
 	} else {
+		scctx->isc_vectors = 1;
 		device_printf(dev,"Using a Legacy interrupt\n");
 		scctx->isc_intr = IFLIB_INTR_LEGACY;
 	}
@@ -6024,7 +5990,7 @@ msi:
 	return (vectors);
 }
 
-char * ring_states[] = { "IDLE", "BUSY", "STALLED", "ABDICATED" };
+static const char *ring_states[] = { "IDLE", "BUSY", "STALLED", "ABDICATED" };
 
 static int
 mp_ring_state_handler(SYSCTL_HANDLER_ARGS)
@@ -6032,7 +5998,7 @@ mp_ring_state_handler(SYSCTL_HANDLER_ARGS)
 	int rc;
 	uint16_t *state = ((uint16_t *)oidp->oid_arg1);
 	struct sbuf *sb;
-	char *ring_state = "UNKNOWN";
+	const char *ring_state = "UNKNOWN";
 
 	/* XXX needed ? */
 	rc = sysctl_wire_old_buffer(req, 0);

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