Date: Thu, 27 Jun 2013 20:21:54 +0000 (UTC) From: John Baldwin <jhb@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r252315 - in head/sys: dev/pci kern sys Message-ID: <201306272021.r5RKLsLL067264@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: jhb Date: Thu Jun 27 20:21:54 2013 New Revision: 252315 URL: http://svnweb.freebsd.org/changeset/base/252315 Log: Make detaching drivers from PCI devices more robust. While here, fix a bug where a PCI device would be powered down if it failed to probe, but not when its driver was detached (e.g. via kldunload). - Add a new helper method resource_list_release_active() which forcefully releases any active resources of a specified type from a resource list. - Add a bus_child_detached method for the PCI bus driver which forces any active resources to be released (and whines to the console if it finds any) and then powers the device down. - Call pci_child_detached() if we fail to probe a device when a driver is kldloaded. This isn't perfect but can avoid leaking resources from a probe() routine in the kldload case. Reviewed by: imp, brooks MFC after: 1 month Modified: head/sys/dev/pci/pci.c head/sys/dev/pci/pci_private.h head/sys/kern/subr_bus.c head/sys/sys/bus.h Modified: head/sys/dev/pci/pci.c ============================================================================== --- head/sys/dev/pci/pci.c Thu Jun 27 19:47:58 2013 (r252314) +++ head/sys/dev/pci/pci.c Thu Jun 27 20:21:54 2013 (r252315) @@ -152,6 +152,7 @@ static device_method_t pci_methods[] = { DEVMETHOD(bus_release_resource, bus_generic_rl_release_resource), DEVMETHOD(bus_activate_resource, pci_activate_resource), DEVMETHOD(bus_deactivate_resource, pci_deactivate_resource), + DEVMETHOD(bus_child_detached, pci_child_detached), DEVMETHOD(bus_child_pnpinfo_str, pci_child_pnpinfo_str_method), DEVMETHOD(bus_child_location_str, pci_child_location_str_method), DEVMETHOD(bus_remap_intr, pci_remap_intr_method), @@ -3489,7 +3490,7 @@ pci_driver_added(device_t dev, driver_t pci_printf(&dinfo->cfg, "reprobing on driver added\n"); pci_cfg_restore(child, dinfo); if (device_probe_and_attach(child) != 0) - pci_cfg_save(child, dinfo, 1); + pci_child_detached(dev, child); } free(devlist, M_TEMP); } @@ -3804,6 +3805,34 @@ pci_probe_nomatch(device_t dev, device_t pci_cfg_save(child, device_get_ivars(child), 1); } +void +pci_child_detached(device_t dev, device_t child) +{ + struct pci_devinfo *dinfo; + struct resource_list *rl; + + dinfo = device_get_ivars(child); + rl = &dinfo->resources; + + /* + * Have to deallocate IRQs before releasing any MSI messages and + * have to release MSI messages before deallocating any memory + * BARs. + */ + if (resource_list_release_active(rl, dev, child, SYS_RES_IRQ) != 0) + pci_printf(&dinfo->cfg, "Device leaked IRQ resources\n"); + if (dinfo->cfg.msi.msi_alloc != 0 || dinfo->cfg.msix.msix_alloc != 0) { + pci_printf(&dinfo->cfg, "Device leaked MSI vectors\n"); + (void)pci_release_msi(child); + } + if (resource_list_release_active(rl, dev, child, SYS_RES_MEMORY) != 0) + pci_printf(&dinfo->cfg, "Device leaked memory resources\n"); + if (resource_list_release_active(rl, dev, child, SYS_RES_IOPORT) != 0) + pci_printf(&dinfo->cfg, "Device leaked I/O resources\n"); + + pci_cfg_save(child, dinfo, 1); +} + /* * Parse the PCI device database, if loaded, and return a pointer to a * description of the device. Modified: head/sys/dev/pci/pci_private.h ============================================================================== --- head/sys/dev/pci/pci_private.h Thu Jun 27 19:47:58 2013 (r252314) +++ head/sys/dev/pci/pci_private.h Thu Jun 27 20:21:54 2013 (r252315) @@ -106,6 +106,7 @@ struct pci_devinfo *pci_read_device(devi size_t size); void pci_print_verbose(struct pci_devinfo *dinfo); int pci_freecfg(struct pci_devinfo *dinfo); +void pci_child_detached(device_t dev, device_t child); int pci_child_location_str_method(device_t cbdev, device_t child, char *buf, size_t buflen); int pci_child_pnpinfo_str_method(device_t cbdev, device_t child, Modified: head/sys/kern/subr_bus.c ============================================================================== --- head/sys/kern/subr_bus.c Thu Jun 27 19:47:58 2013 (r252314) +++ head/sys/kern/subr_bus.c Thu Jun 27 20:21:54 2013 (r252315) @@ -3316,6 +3316,48 @@ resource_list_release(struct resource_li } /** + * @brief Release all active resources of a given type + * + * Release all active resources of a specified type. This is intended + * to be used to cleanup resources leaked by a driver after detach or + * a failed attach. + * + * @param rl the resource list which was allocated from + * @param bus the parent device of @p child + * @param child the device whose active resources are being released + * @param type the type of resources to release + * + * @retval 0 success + * @retval EBUSY at least one resource was active + */ +int +resource_list_release_active(struct resource_list *rl, device_t bus, + device_t child, int type) +{ + struct resource_list_entry *rle; + int error, retval; + + retval = 0; + STAILQ_FOREACH(rle, rl, link) { + if (rle->type != type) + continue; + if (rle->res == NULL) + continue; + if ((rle->flags & (RLE_RESERVED | RLE_ALLOCATED)) == + RLE_RESERVED) + continue; + retval = EBUSY; + error = resource_list_release(rl, bus, child, type, + rman_get_rid(rle->res), rle->res); + if (error != 0) + device_printf(bus, + "Failed to release active resource: %d\n", error); + } + return (retval); +} + + +/** * @brief Fully release a reserved resource * * Fully releases a resource reserved via resource_list_reserve(). Modified: head/sys/sys/bus.h ============================================================================== --- head/sys/sys/bus.h Thu Jun 27 19:47:58 2013 (r252314) +++ head/sys/sys/bus.h Thu Jun 27 20:21:54 2013 (r252315) @@ -273,6 +273,9 @@ struct resource * int resource_list_release(struct resource_list *rl, device_t bus, device_t child, int type, int rid, struct resource *res); +int resource_list_release_active(struct resource_list *rl, + device_t bus, device_t child, + int type); struct resource * resource_list_reserve(struct resource_list *rl, device_t bus, device_t child,
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201306272021.r5RKLsLL067264>