Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 23 Jan 2013 14:35:05 GMT
From:      John Baldwin <jhb@FreeBSD.org>
To:        Perforce Change Reviews <perforce@FreeBSD.org>
Subject:   PERFORCE change 221406 for review
Message-ID:  <201301231435.r0NEZ57M061526@skunkworks.freebsd.org>

next in thread | raw e-mail | index | archive | help
http://p4web.freebsd.org/@@221406?ac=10

Change 221406 by jhb@jhb_jhbbsd on 2013/01/23 14:34:35

	Store this patch here.  This is an (untested) change that adds
	a new method to force all active resources of a given type to
	be released.  I use this in a pci_child_detached() hook to
	clean up for any misbehaving drivers that leave resources attached
	on failure from attach or a successful detach.  I also use this
	to power down PCI devices when a driver is detached.

Affected files ...

.. //depot/projects/pci/sys/dev/pci/pci.c#39 edit
.. //depot/projects/pci/sys/dev/pci/pci_private.h#13 edit
.. //depot/projects/pci/sys/kern/subr_bus.c#9 edit
.. //depot/projects/pci/sys/sys/bus.h#9 edit

Differences ...

==== //depot/projects/pci/sys/dev/pci/pci.c#39 (text+ko) ====

@@ -159,6 +159,7 @@
 	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),
@@ -3501,7 +3502,7 @@
 			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);
 }
@@ -3816,6 +3817,34 @@
 	return;
 }
 
+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.

==== //depot/projects/pci/sys/dev/pci/pci_private.h#13 (text+ko) ====

@@ -109,6 +109,7 @@
 		    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,

==== //depot/projects/pci/sys/kern/subr_bus.c#9 (text+ko) ====

@@ -3316,6 +3316,53 @@
 }
 
 /**
+ * @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;
+
+	/*
+	 * XXX: No way to force teardown of interrupt handlers for
+	 * IRQ resources.  Perhaps bus_deactivate_resource()
+	 * should do that in the nexus?
+	 */
+	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 resouce reserved via resource_list_reserve().

==== //depot/projects/pci/sys/sys/bus.h#9 (text+ko) ====

@@ -276,6 +276,9 @@
 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?201301231435.r0NEZ57M061526>