Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 14 Dec 2017 03:41:12 +0000 (UTC)
From:      "Landon J. Fuller" <landonf@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r326839 - head/sys/dev/bhnd/bhndb
Message-ID:  <201712140341.vBE3fCNK040452@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: landonf
Date: Thu Dec 14 03:41:12 2017
New Revision: 326839
URL: https://svnweb.freebsd.org/changeset/base/326839

Log:
  bhndb(4): Fix two register window overcommit bugs introduced in r326297:
  
  - The window target must always be updated when stealing a register window.
  - Fix missing initialization of bhndb(4) region alloc_flags when
    registering statically mapped port regions (caught by scan-build).
  
  Approved by:	adrian (mentor, implicit)
  Sponsored by:	The FreeBSD Foundation

Modified:
  head/sys/dev/bhnd/bhndb/bhndb.c

Modified: head/sys/dev/bhnd/bhndb/bhndb.c
==============================================================================
--- head/sys/dev/bhnd/bhndb/bhndb.c	Thu Dec 14 03:12:05 2017	(r326838)
+++ head/sys/dev/bhnd/bhndb/bhndb.c	Thu Dec 14 03:41:12 2017	(r326839)
@@ -340,7 +340,7 @@ bhndb_init_region_cfg(struct bhndb_softc *sc, bhnd_ero
 			 * always HIGH.
 			 */
 			error = bhndb_add_resource_region(br, addr, size,
-			    BHNDB_PRIORITY_HIGH, 0, regw);
+			    BHNDB_PRIORITY_HIGH, alloc_flags, regw);
 			if (error)
 				return (error);
 		}
@@ -1634,14 +1634,33 @@ bhndb_deactivate_bhnd_resource(device_t dev, device_t 
 }
 
 /**
- * Slow path for bhndb_io_resource().
- *
- * Iterates over the existing allocated dynamic windows looking for a viable
- * in-use region; the first matching region is returned.
+ * Find the best available bridge resource allocation record capable of handling
+ * bus I/O requests of @p size at @p addr.
+ * 
+ * In order of preference, this function will either:
+ * 
+ * - Configure and return a free allocation record
+ * - Return an existing allocation record mapping the requested space, or
+ * - Steal, configure, and return an in-use allocation record.
+ * 
+ * Will panic if a usable record cannot be found.
+ * 
+ * @param sc Bridge driver state.
+ * @param addr The I/O target address.
+ * @param size The size of the I/O operation to be performed at @p addr. 
+ * @param[out] borrowed Set to true if the allocation record was borrowed to
+ * fulfill this request; the borrowed record maps the target address range,
+ * and must not be modified.
+ * @param[out] stolen Set to true if the allocation record was stolen to fulfill
+ * this request. If a stolen allocation record is returned,
+ * bhndb_io_resource_restore() must be called upon completion of the bus I/O
+ * request.
+ * @param[out] restore If the allocation record was stolen, this will be set
+ * to the target that must be restored.
  */
 static struct bhndb_dw_alloc *
-bhndb_io_resource_slow(struct bhndb_softc *sc, bus_addr_t addr, bus_size_t size,
-    bus_size_t *offset, bool *stolen, bus_addr_t *restore)
+bhndb_io_resource_get_window(struct bhndb_softc *sc, bus_addr_t addr,
+    bus_size_t size, bool *borrowed, bool *stolen, bus_addr_t *restore)
 {
 	struct bhndb_resources	*br;
 	struct bhndb_dw_alloc	*dwa;
@@ -1650,7 +1669,13 @@ bhndb_io_resource_slow(struct bhndb_softc *sc, bus_add
 	BHNDB_LOCK_ASSERT(sc, MA_OWNED);
 
 	br = sc->bus_res;
+	*borrowed = false;
+	*stolen = false;
 
+	/* Try to fetch a free window */
+	if ((dwa = bhndb_dw_next_free(br)) != NULL)
+		return (dwa);
+
 	/* Search for an existing dynamic mapping of this address range.
 	 * Static regions are not searched, as a statically mapped
 	 * region would never be allocated as an indirect resource. */
@@ -1671,16 +1696,12 @@ bhndb_io_resource_slow(struct bhndb_softc *sc, bus_add
 			continue;
 
 		/* Found */
-		*offset = dwa->win->win_offset;
-		*offset += addr - dwa->target;
-
-		*stolen = false;
+		*borrowed = true;
 		return (dwa);
 	}
 
-	/* No existing dynamic mapping found. We'll need to check for a defined
-	 * region to determine whether we can fulfill this request by
-	 * stealing from an existing allocated register window */
+	/* Try to steal a window; this should only be required on very early
+	 * PCI_V0 (BCM4318, etc) Wi-Fi chipsets */
 	region = bhndb_find_resource_region(br, addr, size);
 	if (region == NULL)
 		return (NULL);
@@ -1688,12 +1709,16 @@ bhndb_io_resource_slow(struct bhndb_softc *sc, bus_add
 	if ((region->alloc_flags & BHNDB_ALLOC_FULFILL_ON_OVERCOMMIT) == 0)
 		return (NULL);
 
+	/* Steal a window. This acquires our backing spinlock, disabling
+	 * interrupts; the spinlock will be released by
+	 * bhndb_dw_return_stolen() */
 	if ((dwa = bhndb_dw_steal(br, restore)) != NULL) {
 		*stolen = true;
 		return (dwa);
 	}
 
-	return (NULL);
+	panic("register windows exhausted attempting to map 0x%llx-0x%llx\n", 
+	    (unsigned long long) addr, (unsigned long long) addr+size-1);
 }
 
 /**
@@ -1721,50 +1746,29 @@ static inline struct bhndb_dw_alloc *
 bhndb_io_resource(struct bhndb_softc *sc, bus_addr_t addr, bus_size_t size,
     bus_size_t *offset, bool *stolen, bus_addr_t *restore)
 {
-	struct bhndb_resources	*br;
 	struct bhndb_dw_alloc	*dwa;
+	bool			 borrowed;
 	int			 error;
 
 	BHNDB_LOCK_ASSERT(sc, MA_OWNED);
 
-	br = sc->bus_res;
+	dwa = bhndb_io_resource_get_window(sc, addr, size, &borrowed, stolen,
+	    restore);
 
-	/* Try to fetch a free window */
-	dwa = bhndb_dw_next_free(br);
-
-	/*
-	 * If no dynamic windows are available, look for an existing
-	 * region that maps the target range. 
-	 * 
-	 * If none are found, this is a child driver bug -- our window
-	 * over-commit should only fail in the case where a child driver leaks
-	 * resources, or perform operations out-of-order.
-	 * 
-	 * Broadcom HND chipsets are designed to not require register window
-	 * swapping during execution; as long as the child devices are
-	 * attached/detached correctly, using the hardware's required order
-	 * of operations, there should always be a window available for the
-	 * current operation.
-	 */
-	if (dwa == NULL) {
-		dwa = bhndb_io_resource_slow(sc, addr, size, offset, stolen,
-		    restore);
-		if (dwa == NULL) {
-			panic("register windows exhausted attempting to map "
-			    "0x%llx-0x%llx\n", 
-			    (unsigned long long) addr,
-			    (unsigned long long) addr+size-1);
-		}
-
-		return (dwa);
-	}
-
 	/* Adjust the window if the I/O request won't fit in the current
 	 * target range. */
 	if (addr < dwa->target ||
 	    addr > dwa->target + dwa->win->win_size ||
 	    (dwa->target + dwa->win->win_size) - addr < size)
 	{
+		/* Cannot modify target of borrowed windows */
+		if (borrowed) {
+			panic("borrowed register window does not map expected "
+			    "range 0x%llx-0x%llx\n", 
+			    (unsigned long long) addr,
+			    (unsigned long long) addr+size-1);
+		}
+	
 		error = bhndb_dw_set_addr(sc->dev, sc->bus_res, dwa, addr,
 		    size);
 		if (error) {
@@ -1777,7 +1781,6 @@ bhndb_io_resource(struct bhndb_softc *sc, bus_addr_t a
 
 	/* Calculate the offset and return */
 	*offset = (addr - dwa->target) + dwa->win->win_offset;
-	*stolen = false;
 	return (dwa);
 }
 



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