svn commit: r254882 - head/sys/dev/pci

[John Baldwin]

On Thursday, August 29, 2013 6:10:20 am Jean-Sébastien Pédron wrote:
> On 26.08.2013 16:55, John Baldwin wrote:
> > On Sunday, August 25, 2013 2:09:12 pm Jean-Sebastien Pedron wrote:
> >> Author: dumbbell
> >> Date: Sun Aug 25 18:09:11 2013
> >> New Revision: 254882
> >> URL: http://svnweb.freebsd.org/changeset/base/254882
> >>
> >> Log:
> >>   vga_pci: Add API to map the Video BIOS
> > 
> > This won't actually work (the PCI bus will panic when you do the duplicate 
> > alloc).  Why not put this in the softc of the vga_pci device?  In fact, it is 
> > already there.  You just need to bump the vr_refs on the vga_bios resource 
> > similar to what vga_pci_alloc_resource() does.
> 
> Ok, so just calling vga_pci_alloc_resource() instead of
> bus_alloc_resource_any() in vga_pci_map_bios() would be enough to fix
> this first problem.

Yes.

> > Also, returning a void * for this API seems rather hacky.  Have you considered
> > returning a struct resource * instead (and requiring the caller to use 
> > bus_space_* on it)?  That would be more consistent with new-bus.
> 
> You're right.
> 
> > I can help with getting the right bus_alloc_resource() incantation to
> > alloc a struct resource * for the shadow copy if so.
> 
> Yes please, I'd appreciate it :) I see that the function allocates
> resources from the parent device, but I'm not sure what to do with this,
> because the shadow copy is at a fixed physical address.

You can use bus_alloc_resource() to allocate a fixed range, but the trick in
this case is that we need to bypass the PCI bus as this isn't a BAR.

Here is an untested cut at this, it only changes these functions and doesn't
fix the callers to work with the returned struct resource, etc.:

Index: vga_pci.c
===================================================================
--- vga_pci.c	(revision 255020)
+++ vga_pci.c	(working copy)
@@ -46,11 +46,6 @@ __FBSDID("$FreeBSD$");
 #include <sys/sysctl.h>
 #include <sys/systm.h>
 
-#if defined(__amd64__) || defined(__i386__) || defined(__ia64__)
-#include <vm/vm.h>
-#include <vm/pmap.h>
-#endif
-
 #include <dev/pci/pcireg.h>
 #include <dev/pci/pcivar.h>
 
@@ -88,11 +83,10 @@ vga_pci_is_boot_display(device_t dev)
 	    device_get_unit(dev) == vga_pci_default_unit);
 }
 
-void *
-vga_pci_map_bios(device_t dev, size_t *size)
+struct resource *
+vga_pci_map_bios(device_t dev)
 {
 	int rid;
-	struct resource *res;
 
 #if defined(__amd64__) || defined(__i386__) || defined(__ia64__)
 	if (vga_pci_is_boot_display(dev)) {
@@ -103,30 +97,30 @@ vga_pci_is_boot_display(device_t dev)
 		 *
 		 * We use this copy for the default boot device, because
 		 * the original ROM may not be valid after boot.
+		 *
+		 * Allocate this from our grandparent to by-pass the
+		 * checks for a valid rid in the PCI bus driver as this
+		 * is not a BAR.
 		 */
-
-		*size = VGA_PCI_BIOS_SHADOW_SIZE;
-		return (pmap_mapbios(VGA_PCI_BIOS_SHADOW_ADDR, *size));
+		rid = 1;
+		return (BUS_ALLOC_RESOURCE(device_get_parent(
+		    device_get_parent(dev)), dev, SYS_RES_MEMORY, &rid,
+		    VGA_PCI_BIOS_SHADOW_ADDR,
+		    VGA_PCI_BIOS_SHADOW_ADDR + VGA_PCI_BIOS_SHADOW_SIZE - 1,
+		    VGA_PCI_BIOS_SHADOW_SIZE, RF_ACTIVE));
 	}
 #endif
 
 	rid = PCIR_BIOS;
-	res = bus_alloc_resource_any(dev, SYS_RES_MEMORY, &rid, RF_ACTIVE);
-	if (res == NULL) {
-		return (NULL);
-	}
-
-	*size = rman_get_size(res);
-	return (rman_get_virtual(res));
+	return (vga_pci_alloc_resource(dev, NULL, SYS_RES_MEMORY, &rid, 0ul,
+	    ~0ul, 1, RF_ACTIVE));
 }
 
 void
-vga_pci_unmap_bios(device_t dev, void *bios)
+vga_pci_unmap_bios(device_t dev, struct resource *res)
 {
-	int rid;
-	struct resource *res;
 
-	if (bios == NULL) {
+	if (res == NULL) {
 		return;
 	}
 
@@ -133,32 +127,13 @@ void
 #if defined(__amd64__) || defined(__i386__) || defined(__ia64__)
 	if (vga_pci_is_boot_display(dev)) {
 		/* We mapped the BIOS shadow copy located at 0xC0000. */
-		pmap_unmapdev((vm_offset_t)bios, VGA_PCI_BIOS_SHADOW_SIZE);
+		BUS_RELEASE_RESOURCE(device_get_parent(
+		    device_get_parent(dev)), dev, SYS_RES_MEMORY, 1, res);
 
 		return;
 	}
 #endif
-
-	/*
-	 * FIXME: We returned only the virtual address of the resource
-	 * to the caller. Now, to get the resource struct back, we
-	 * allocate it again: the struct exists once in memory in
-	 * device softc. Therefore, we release twice now to release the
-	 * reference we just obtained to get the structure back and the
-	 * caller's reference.
-	 */
-
-	rid = PCIR_BIOS;
-	res = bus_alloc_resource_any(dev, SYS_RES_MEMORY, &rid, RF_ACTIVE);
-
-	KASSERT(res != NULL,
-	    ("%s: Can't get BIOS resource back", __func__));
-	KASSERT(bios == rman_get_virtual(res),
-	    ("%s: Given BIOS address doesn't match "
-	     "resource virtual address", __func__));
-
-	bus_release_resource(dev, SYS_RES_MEMORY, rid, bios);
-	bus_release_resource(dev, SYS_RES_MEMORY, rid, bios);
+	vga_pci_release_resource(dev, NULL, SYS_RES_MEMORY, PCIR_BIOS, res);
 }
 
 static int

-- 
John Baldwin