From owner-svn-src-head@freebsd.org Thu Dec 14 03:41:13 2017 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id EA048E91DED; Thu, 14 Dec 2017 03:41:13 +0000 (UTC) (envelope-from landonf@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id C139D70765; Thu, 14 Dec 2017 03:41:13 +0000 (UTC) (envelope-from landonf@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id vBE3fCVW040453; Thu, 14 Dec 2017 03:41:12 GMT (envelope-from landonf@FreeBSD.org) Received: (from landonf@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id vBE3fCNK040452; Thu, 14 Dec 2017 03:41:12 GMT (envelope-from landonf@FreeBSD.org) Message-Id: <201712140341.vBE3fCNK040452@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: landonf set sender to landonf@FreeBSD.org using -f From: "Landon J. Fuller" Date: Thu, 14 Dec 2017 03:41:12 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r326839 - head/sys/dev/bhnd/bhndb X-SVN-Group: head X-SVN-Commit-Author: landonf X-SVN-Commit-Paths: head/sys/dev/bhnd/bhndb X-SVN-Commit-Revision: 326839 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.25 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: Thu, 14 Dec 2017 03:41:14 -0000 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); }