From owner-svn-src-all@FreeBSD.ORG Tue Mar 3 16:39:00 2009 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id E97C5106567C; Tue, 3 Mar 2009 16:38:59 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id D6FAA8FC0C; Tue, 3 Mar 2009 16:38:59 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.3/8.14.3) with ESMTP id n23Gcx81015527; Tue, 3 Mar 2009 16:38:59 GMT (envelope-from jhb@svn.freebsd.org) Received: (from jhb@localhost) by svn.freebsd.org (8.14.3/8.14.3/Submit) id n23GcxZJ015525; Tue, 3 Mar 2009 16:38:59 GMT (envelope-from jhb@svn.freebsd.org) Message-Id: <200903031638.n23GcxZJ015525@svn.freebsd.org> From: John Baldwin Date: Tue, 3 Mar 2009 16:38:59 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r189306 - head/sys/dev/pci X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 03 Mar 2009 16:39:00 -0000 Author: jhb Date: Tue Mar 3 16:38:59 2009 New Revision: 189306 URL: http://svn.freebsd.org/changeset/base/189306 Log: Further refine the handling of resources for BARs in the PCI bus driver. A while back, Warner changed the PCI bus code to reserve resources when enumerating devices and simply give devices the previously allocated resources when they call bus_alloc_resource(). This ensures that address ranges being decoded by a BAR are always allocated in the nexus0 device (or whatever device the PCI bus gets its address space from) even if a device driver is not attached to the device. This patch extends this behavior further: - To let the PCI bus distinguish between a resource being allocated by a device driver vs. merely being allocated by the bus, use rman_set_device() to assign the device to the bus when it is owned by the bus and to the child device when it is allocated by the child device's driver. We can now prevent a device driver from allocating the same device twice. Doing so could result in odd things like allocating duplicate virtual memory to map the resource on some archs and leaking the original mapping. - When a PCI device driver releases a resource, don't pass the request all the way up the tree and release it in the nexus (or similar device) since the BAR is still active and decoding. Otherwise, another device could later allocate the same range even though it is still in use. Instead, deactivate the resource and assign it back to the PCI bus using rman_set_device(). - pci_delete_resource() will actually completely free a BAR including attemping to disable it. - Disable BAR decoding via the command register when sizing a BAR in pci_alloc_map() which is used to allocate resources for a BAR when the BIOS/firmware did not assign a usable resource range during boot. This mirrors an earlier fix to pci_add_map() which is used when to size BARs during boot. - Move the activation of I/O decoding in the PCI command register into pci_activate_resource() instead of doing it in pci_alloc_resource(). Previously we could actually enable decoding before a BAR was initialized via pci_alloc_map(). Glanced at by: bsdimp Modified: head/sys/dev/pci/pci.c head/sys/dev/pci/pci_private.h Modified: head/sys/dev/pci/pci.c ============================================================================== --- head/sys/dev/pci/pci.c Tue Mar 3 15:50:24 2009 (r189305) +++ head/sys/dev/pci/pci.c Tue Mar 3 16:38:59 2009 (r189306) @@ -136,7 +136,7 @@ static device_method_t pci_methods[] = { DEVMETHOD(bus_delete_resource, pci_delete_resource), DEVMETHOD(bus_alloc_resource, pci_alloc_resource), DEVMETHOD(bus_release_resource, bus_generic_rl_release_resource), - DEVMETHOD(bus_activate_resource, bus_generic_activate_resource), + DEVMETHOD(bus_activate_resource, pci_activate_resource), DEVMETHOD(bus_deactivate_resource, bus_generic_deactivate_resource), DEVMETHOD(bus_child_pnpinfo_str, pci_child_pnpinfo_str_method), DEVMETHOD(bus_child_location_str, pci_child_location_str_method), @@ -2294,8 +2294,8 @@ pci_add_map(device_t pcib, device_t bus, /* * Disable decoding via the command register before - * determining the BARs length since we will be placing them - * in a weird state. + * determining the BAR's length since we will be placing it in + * a weird state. */ cmd = PCIB_READ_CONFIG(pcib, b, s, f, PCIR_COMMAND, 2); PCIB_WRITE_CONFIG(pcib, b, s, f, PCIR_COMMAND, @@ -2336,7 +2336,10 @@ pci_add_map(device_t pcib, device_t bus, return (barlen); if (ln2range == 64) - /* Read the other half of a 64bit map register */ + /* + * Read the other half of a 64bit map register. We + * assume that the size of the BAR is less than 2^32. + */ base |= (uint64_t) PCIB_READ_CONFIG(pcib, b, s, f, reg + 4, 4) << 32; if (bootverbose) { printf("\tmap[%02x]: type %s, range %2d, base %#jx, size %2d", @@ -2422,8 +2425,10 @@ pci_add_map(device_t pcib, device_t bus, */ resource_list_delete(rl, type, reg); start = 0; - } else + } else { start = rman_get_start(res); + rman_set_device(res, bus); + } pci_write_config(dev, reg, start, 4); if (ln2range == 64) pci_write_config(dev, reg + 4, start >> 32, 4); @@ -2441,6 +2446,7 @@ static void pci_ata_maps(device_t pcib, device_t bus, device_t dev, int b, int s, int f, struct resource_list *rl, int force, uint32_t prefetchmask) { + struct resource *r; int rid, type, progif; #if 0 /* if this device supports PCI native addressing use it */ @@ -2463,12 +2469,14 @@ pci_ata_maps(device_t pcib, device_t bus } else { rid = PCIR_BAR(0); resource_list_add(rl, type, rid, 0x1f0, 0x1f7, 8); - resource_list_alloc(rl, bus, dev, type, &rid, 0x1f0, 0x1f7, 8, - 0); + r = resource_list_alloc(rl, bus, dev, type, &rid, 0x1f0, 0x1f7, + 8, 0); + rman_set_device(r, bus); rid = PCIR_BAR(1); resource_list_add(rl, type, rid, 0x3f6, 0x3f6, 1); - resource_list_alloc(rl, bus, dev, type, &rid, 0x3f6, 0x3f6, 1, - 0); + r = resource_list_alloc(rl, bus, dev, type, &rid, 0x3f6, 0x3f6, + 1, 0); + rman_set_device(r, bus); } if (progif & PCIP_STORAGE_IDE_MODESEC) { pci_add_map(pcib, bus, dev, b, s, f, PCIR_BAR(2), rl, force, @@ -2478,12 +2486,14 @@ pci_ata_maps(device_t pcib, device_t bus } else { rid = PCIR_BAR(2); resource_list_add(rl, type, rid, 0x170, 0x177, 8); - resource_list_alloc(rl, bus, dev, type, &rid, 0x170, 0x177, 8, - 0); + r = resource_list_alloc(rl, bus, dev, type, &rid, 0x170, 0x177, + 8, 0); + rman_set_device(r, bus); rid = PCIR_BAR(3); resource_list_add(rl, type, rid, 0x376, 0x376, 1); - resource_list_alloc(rl, bus, dev, type, &rid, 0x376, 0x376, 1, - 0); + r = resource_list_alloc(rl, bus, dev, type, &rid, 0x376, 0x376, + 1, 0); + rman_set_device(r, bus); } pci_add_map(pcib, bus, dev, b, s, f, PCIR_BAR(4), rl, force, prefetchmask & (1 << 4)); @@ -3391,6 +3401,7 @@ pci_alloc_map(device_t dev, device_t chi struct resource_list_entry *rle; struct resource *res; pci_addr_t map, testval; + uint16_t cmd; int mapsize; /* @@ -3402,12 +3413,21 @@ pci_alloc_map(device_t dev, device_t chi */ res = NULL; map = pci_read_config(child, *rid, 4); + + /* + * Disable decoding via the command register before + * determining the BAR's length since we will be placing it in + * a weird state. + */ + cmd = pci_read_config(child, PCIR_COMMAND, 2); + pci_write_config(child, PCIR_COMMAND, + cmd & ~(PCI_BAR_MEM(map) ? PCIM_CMD_MEMEN : PCIM_CMD_PORTEN), 2); + + /* Determine the BAR's length. */ pci_write_config(child, *rid, 0xffffffff, 4); testval = pci_read_config(child, *rid, 4); if (pci_maprange(testval) == 64) map |= (pci_addr_t)pci_read_config(child, *rid + 4, 4) << 32; - if (pci_mapbase(testval) == 0) - goto out; /* * Restore the original value of the BAR. We may have reprogrammed @@ -3415,6 +3435,11 @@ pci_alloc_map(device_t dev, device_t chi * we need the console device addressable. */ pci_write_config(child, *rid, map, 4); + pci_write_config(child, PCIR_COMMAND, cmd, 2); + + /* Ignore a BAR with a base of 0. */ + if (pci_mapbase(testval) == 0) + goto out; if (PCI_BAR_MEM(testval)) { if (type != SYS_RES_MEMORY) { @@ -3452,13 +3477,14 @@ pci_alloc_map(device_t dev, device_t chi * appropriate bar for that resource. */ res = BUS_ALLOC_RESOURCE(device_get_parent(dev), child, type, rid, - start, end, count, flags); + start, end, count, flags & ~RF_ACTIVE); if (res == NULL) { device_printf(child, "%#lx bytes of rid %#x res %d failed (%#lx, %#lx).\n", count, *rid, type, start, end); goto out; } + rman_set_device(res, dev); resource_list_add(rl, type, *rid, start, end, count); rle = resource_list_find(rl, type, *rid); if (rle == NULL) @@ -3472,10 +3498,10 @@ pci_alloc_map(device_t dev, device_t chi "Lazy allocation of %#lx bytes rid %#x type %d at %#lx\n", count, *rid, type, rman_get_start(res)); map = rman_get_start(res); -out:; pci_write_config(child, *rid, map, 4); if (pci_maprange(testval) == 64) pci_write_config(child, *rid + 4, map >> 32, 4); +out:; return (res); } @@ -3487,68 +3513,63 @@ pci_alloc_resource(device_t dev, device_ struct pci_devinfo *dinfo = device_get_ivars(child); struct resource_list *rl = &dinfo->resources; struct resource_list_entry *rle; + struct resource *res; pcicfgregs *cfg = &dinfo->cfg; + if (device_get_parent(child) != dev) + return (BUS_ALLOC_RESOURCE(device_get_parent(dev), child, + type, rid, start, end, count, flags)); + /* * Perform lazy resource allocation */ - if (device_get_parent(child) == dev) { - switch (type) { - case SYS_RES_IRQ: - /* - * Can't alloc legacy interrupt once MSI messages - * have been allocated. - */ - if (*rid == 0 && (cfg->msi.msi_alloc > 0 || - cfg->msix.msix_alloc > 0)) + switch (type) { + case SYS_RES_IRQ: + /* + * Can't alloc legacy interrupt once MSI messages have + * been allocated. + */ + if (*rid == 0 && (cfg->msi.msi_alloc > 0 || + cfg->msix.msix_alloc > 0)) + return (NULL); + + /* + * If the child device doesn't have an interrupt + * routed and is deserving of an interrupt, try to + * assign it one. + */ + if (*rid == 0 && !PCI_INTERRUPT_VALID(cfg->intline) && + (cfg->intpin != 0)) + pci_assign_interrupt(dev, child, 0); + break; + case SYS_RES_IOPORT: + case SYS_RES_MEMORY: + /* Allocate resources for this BAR if needed. */ + rle = resource_list_find(rl, type, *rid); + if (rle == NULL) { + res = pci_alloc_map(dev, child, type, rid, start, end, + count, flags); + if (res == NULL) return (NULL); - /* - * If the child device doesn't have an - * interrupt routed and is deserving of an - * interrupt, try to assign it one. - */ - if (*rid == 0 && !PCI_INTERRUPT_VALID(cfg->intline) && - (cfg->intpin != 0)) - pci_assign_interrupt(dev, child, 0); - break; - case SYS_RES_IOPORT: - case SYS_RES_MEMORY: - if (*rid < PCIR_BAR(cfg->nummaps)) { - /* - * Enable the I/O mode. We should - * also be assigning resources too - * when none are present. The - * resource_list_alloc kind of sorta does - * this... - */ - if (PCI_ENABLE_IO(dev, child, type)) - return (NULL); - } rle = resource_list_find(rl, type, *rid); - if (rle == NULL) - return (pci_alloc_map(dev, child, type, rid, - start, end, count, flags)); - break; } + /* - * If we've already allocated the resource, then - * return it now. But first we may need to activate - * it, since we don't allocate the resource as active - * above. Normally this would be done down in the - * nexus, but since we short-circuit that path we have - * to do its job here. Not sure if we should free the - * resource if it fails to activate. + * If the resource belongs to the bus, then give it to + * the child. We need to activate it if requested + * since the bus always allocates inactive resources. */ - rle = resource_list_find(rl, type, *rid); - if (rle != NULL && rle->res != NULL) { + if (rle != NULL && rle->res != NULL && + rman_get_device(rle->res) == dev) { if (bootverbose) device_printf(child, "Reserved %#lx bytes for rid %#x type %d at %#lx\n", rman_get_size(rle->res), *rid, type, rman_get_start(rle->res)); + rman_set_device(rle->res, child); if ((flags & RF_ACTIVE) && - bus_generic_activate_resource(dev, child, type, - *rid, rle->res) != 0) + bus_activate_resource(child, type, *rid, + rle->res) != 0) return (NULL); return (rle->res); } @@ -3557,6 +3578,59 @@ pci_alloc_resource(device_t dev, device_ start, end, count, flags)); } +int +pci_release_resource(device_t dev, device_t child, int type, int rid, + struct resource *r) +{ + int error; + + if (device_get_parent(child) != dev) + return (BUS_RELEASE_RESOURCE(device_get_parent(dev), child, + type, rid, r)); + + /* + * For BARs we don't actually want to release the resource. + * Instead, we deactivate the resource if needed and then give + * ownership of the BAR back to the bus. + */ + switch (type) { + case SYS_RES_IOPORT: + case SYS_RES_MEMORY: + if (rman_get_device(r) != child) + return (EINVAL); + if (rman_get_flags(r) & RF_ACTIVE) { + error = bus_deactivate_resource(child, type, rid, r); + if (error) + return (error); + } + rman_set_device(r, dev); + return (0); + } + return (bus_generic_rl_release_resource(dev, child, type, rid, r)); +} + +int +pci_activate_resource(device_t dev, device_t child, int type, int rid, + struct resource *r) +{ + int error; + + error = bus_generic_activate_resource(dev, child, type, rid, r); + if (error) + return (error); + + /* Enable decoding in the command register when activating BARs. */ + if (device_get_parent(child) == dev) { + switch (type) { + case SYS_RES_IOPORT: + case SYS_RES_MEMORY: + error = PCI_ENABLE_IO(dev, child, type); + break; + } + } + return (error); +} + void pci_delete_resource(device_t dev, device_t child, int type, int rid) { @@ -3570,27 +3644,34 @@ pci_delete_resource(device_t dev, device dinfo = device_get_ivars(child); rl = &dinfo->resources; rle = resource_list_find(rl, type, rid); - if (rle) { - if (rle->res) { - if (rman_get_device(rle->res) != dev || - rman_get_flags(rle->res) & RF_ACTIVE) { - device_printf(dev, "delete_resource: " - "Resource still owned by child, oops. " - "(type=%d, rid=%d, addr=%lx)\n", - rle->type, rle->rid, - rman_get_start(rle->res)); - return; - } - bus_release_resource(dev, type, rid, rle->res); + if (rle == NULL) + return; + + if (rle->res) { + if (rman_get_device(rle->res) != dev || + rman_get_flags(rle->res) & RF_ACTIVE) { + device_printf(dev, "delete_resource: " + "Resource still owned by child, oops. " + "(type=%d, rid=%d, addr=%lx)\n", + rle->type, rle->rid, + rman_get_start(rle->res)); + return; } - resource_list_delete(rl, type, rid); + + /* + * If this is a BAR, clear the BAR so it stops + * decoding before releasing the resource. + */ + switch (type) { + case SYS_RES_IOPORT: + case SYS_RES_MEMORY: + /* XXX: 64-bit BARs? */ + pci_write_config(child, rid, 0, 4); + break; + } + bus_release_resource(dev, type, rid, rle->res); } - /* - * Why do we turn off the PCI configuration BAR when we delete a - * resource? -- imp - */ - pci_write_config(child, rid, 0, 4); - BUS_DELETE_RESOURCE(device_get_parent(dev), child, type, rid); + resource_list_delete(rl, type, rid); } struct resource_list * Modified: head/sys/dev/pci/pci_private.h ============================================================================== --- head/sys/dev/pci/pci_private.h Tue Mar 3 15:50:24 2009 (r189305) +++ head/sys/dev/pci/pci_private.h Tue Mar 3 16:38:59 2009 (r189306) @@ -82,6 +82,10 @@ int pci_msix_count_method(device_t dev, struct resource *pci_alloc_resource(device_t dev, device_t child, int type, int *rid, u_long start, u_long end, u_long count, u_int flags); +int pci_release_resource(device_t dev, device_t child, int type, + int rid, struct resource *r); +int pci_activate_resource(device_t dev, device_t child, int type, + int rid, struct resource *r); void pci_delete_resource(device_t dev, device_t child, int type, int rid); struct resource_list *pci_get_resource_list (device_t dev, device_t child);