Date: Thu, 29 Aug 2013 10:07:26 -0400 From: John Baldwin <jhb@freebsd.org> To: "=?iso-8859-15?q?Jean-S=E9bastien?= =?iso-8859-15?q?_P=E9dron?=" <dumbbell@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r254882 - head/sys/dev/pci Message-ID: <201308291007.26828.jhb@freebsd.org> In-Reply-To: <521F1E0C.5000404@FreeBSD.org> References: <201308251809.r7PI9CsE052978@svn.freebsd.org> <201308261055.17964.jhb@freebsd.org> <521F1E0C.5000404@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thursday, August 29, 2013 6:10:20 am Jean-S=E9bastien P=E9dron 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 > >=20 > > This won't actually work (the PCI bus will panic when you do the duplic= ate=20 > > alloc). Why not put this in the softc of the vga_pci device? In fact,= it is=20 > > already there. You just need to bump the vr_refs on the vga_bios resou= rce=20 > > similar to what vga_pci_alloc_resource() does. >=20 > 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 con= sidered > > returning a struct resource * instead (and requiring the caller to use= =20 > > bus_space_* on it)? That would be more consistent with new-bus. >=20 > You're right. >=20 > > I can help with getting the right bus_alloc_resource() incantation to > > alloc a struct resource * for the shadow copy if so. >=20 > 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 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D =2D-- vga_pci.c (revision 255020) +++ vga_pci.c (working copy) @@ -46,11 +46,6 @@ __FBSDID("$FreeBSD$"); #include <sys/sysctl.h> #include <sys/systm.h> =20 =2D#if defined(__amd64__) || defined(__i386__) || defined(__ia64__) =2D#include <vm/vm.h> =2D#include <vm/pmap.h> =2D#endif =2D #include <dev/pci/pcireg.h> #include <dev/pci/pcivar.h> =20 @@ -88,11 +83,10 @@ vga_pci_is_boot_display(device_t dev) device_get_unit(dev) =3D=3D vga_pci_default_unit); } =20 =2Dvoid * =2Dvga_pci_map_bios(device_t dev, size_t *size) +struct resource * +vga_pci_map_bios(device_t dev) { int rid; =2D struct resource *res; =20 #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. */ =2D =2D *size =3D VGA_PCI_BIOS_SHADOW_SIZE; =2D return (pmap_mapbios(VGA_PCI_BIOS_SHADOW_ADDR, *size)); + rid =3D 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 =20 rid =3D PCIR_BIOS; =2D res =3D bus_alloc_resource_any(dev, SYS_RES_MEMORY, &rid, RF_ACTIVE); =2D if (res =3D=3D NULL) { =2D return (NULL); =2D } =2D =2D *size =3D rman_get_size(res); =2D return (rman_get_virtual(res)); + return (vga_pci_alloc_resource(dev, NULL, SYS_RES_MEMORY, &rid, 0ul, + ~0ul, 1, RF_ACTIVE)); } =20 void =2Dvga_pci_unmap_bios(device_t dev, void *bios) +vga_pci_unmap_bios(device_t dev, struct resource *res) { =2D int rid; =2D struct resource *res; =20 =2D if (bios =3D=3D NULL) { + if (res =3D=3D NULL) { return; } =20 @@ -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. */ =2D 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); =20 return; } #endif =2D =2D /* =2D * FIXME: We returned only the virtual address of the resource =2D * to the caller. Now, to get the resource struct back, we =2D * allocate it again: the struct exists once in memory in =2D * device softc. Therefore, we release twice now to release the =2D * reference we just obtained to get the structure back and the =2D * caller's reference. =2D */ =2D =2D rid =3D PCIR_BIOS; =2D res =3D bus_alloc_resource_any(dev, SYS_RES_MEMORY, &rid, RF_ACTIVE); =2D =2D KASSERT(res !=3D NULL, =2D ("%s: Can't get BIOS resource back", __func__)); =2D KASSERT(bios =3D=3D rman_get_virtual(res), =2D ("%s: Given BIOS address doesn't match " =2D "resource virtual address", __func__)); =2D =2D bus_release_resource(dev, SYS_RES_MEMORY, rid, bios); =2D bus_release_resource(dev, SYS_RES_MEMORY, rid, bios); + vga_pci_release_resource(dev, NULL, SYS_RES_MEMORY, PCIR_BIOS, res); } =20 static int =2D-=20 John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201308291007.26828.jhb>