From owner-freebsd-current@FreeBSD.ORG Thu Oct 25 19:19:36 2007 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 6847616A418 for ; Thu, 25 Oct 2007 19:19:36 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from speedfactory.net (mail6.speedfactory.net [66.23.216.219]) by mx1.freebsd.org (Postfix) with ESMTP id C829113C49D for ; Thu, 25 Oct 2007 19:19:35 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from server.baldwin.cx (unverified [66.23.211.162]) by speedfactory.net (SurgeMail 3.8p) with ESMTP id 216080997-1834499 for multiple; Thu, 25 Oct 2007 15:22:09 -0400 Received: from localhost.corp.yahoo.com (john@localhost [127.0.0.1]) (authenticated bits=0) by server.baldwin.cx (8.13.8/8.13.8) with ESMTP id l9PJJVVR024502; Thu, 25 Oct 2007 15:19:31 -0400 (EDT) (envelope-from jhb@freebsd.org) From: John Baldwin To: freebsd-current@freebsd.org Date: Thu, 25 Oct 2007 14:34:18 -0400 User-Agent: KMail/1.9.6 References: In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200710251434.18764.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH authentication, not delayed by milter-greylist-2.0.2 (server.baldwin.cx [127.0.0.1]); Thu, 25 Oct 2007 15:19:31 -0400 (EDT) X-Virus-Scanned: ClamAV version 0.91.2, clamav-milter version 0.91.2 on server.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-4.4 required=4.2 tests=ALL_TRUSTED,AWL,BAYES_00 autolearn=ham version=3.1.3 X-Spam-Checker-Version: SpamAssassin 3.1.3 (2006-06-01) on server.baldwin.cx Cc: anholt@freebsd.org, pluknet Subject: Re: agp.ko panic: vm_fault: fault on nofault entry, addr: deadc000 X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 25 Oct 2007 19:19:36 -0000 On Saturday 20 October 2007 10:30:31 pm pluknet wrote: > Hello all. > > I could produce panic while kldunload'ing agp.ko. > > It's on 7.0-CURRENT from Oct 11 (just before RELENG_7), i386, UP, > Intel 82855GME (855GME GMCH) SVGA controller. > Some debugging below: > > panic: vm_fault: fault on nofault entry, addr: deadc000 > KDB: enter: panic This is caused by the agp_i810() driver calling agp_generic_detach() first which frees the memory resource for the aperture. Other AGP drivers do the same thing. I think the proper fix is to split agp_generic_detach() into two pieces. The first part would do a destroy_dev() of the /dev node and the various agp_foo_detach() methods would call that first. The rest agp_generic_detach() would be called at the end of the agp_foo_detach() methods. A possible patch (untested) is at http://www.FreeBSD.org/~jhb/patches/agp_detach.patch and included below: Index: agp.c =================================================================== RCS file: /usr/cvs/src/sys/pci/agp.c,v retrieving revision 1.56 diff -u -r1.56 agp.c --- agp.c 13 Jul 2007 16:28:11 -0000 1.56 +++ agp.c 25 Oct 2007 18:28:04 -0000 @@ -261,16 +261,31 @@ return 0; } -int -agp_generic_detach(device_t dev) +void +agp_free_cdev(device_t dev) { struct agp_softc *sc = device_get_softc(dev); destroy_dev(sc->as_devnode); +} + +void +agp_free_res(device_t dev) +{ + struct agp_softc *sc = device_get_softc(dev); + bus_release_resource(dev, SYS_RES_MEMORY, sc->as_aperture_rid, sc->as_aperture); mtx_destroy(&sc->as_lock); agp_flush_cache(); +} + +int +agp_generic_detach(device_t dev) +{ + + agp_free_cdev(dev); + agp_free_res(dev); return 0; } Index: agp_ali.c =================================================================== RCS file: /usr/cvs/src/sys/pci/agp_ali.c,v retrieving revision 1.18 diff -u -r1.18 agp_ali.c --- agp_ali.c 20 Dec 2005 21:12:26 -0000 1.18 +++ agp_ali.c 25 Oct 2007 18:29:55 -0000 @@ -141,12 +141,9 @@ agp_ali_detach(device_t dev) { struct agp_ali_softc *sc = device_get_softc(dev); - int error; u_int32_t attbase; - error = agp_generic_detach(dev); - if (error) - return error; + agp_free_cdev(dev); /* Disable the TLB.. */ pci_write_config(dev, AGP_ALI_TLBCTRL, 0x90, 1); @@ -157,6 +154,7 @@ pci_write_config(dev, AGP_ALI_ATTBASE, attbase & 0xfff, 4); agp_free_gatt(sc->gatt); + agp_free_res(dev); return 0; } Index: agp_amd.c =================================================================== RCS file: /usr/cvs/src/sys/pci/agp_amd.c,v retrieving revision 1.23 diff -u -r1.23 agp_amd.c --- agp_amd.c 20 Dec 2005 21:12:26 -0000 1.23 +++ agp_amd.c 25 Oct 2007 18:30:05 -0000 @@ -277,11 +277,8 @@ agp_amd_detach(device_t dev) { struct agp_amd_softc *sc = device_get_softc(dev); - int error; - error = agp_generic_detach(dev); - if (error) - return error; + agp_free_cdev(dev); /* Disable the TLB.. */ WRITE2(AGP_AMD751_STATUS, @@ -297,6 +294,7 @@ AGP_SET_APERTURE(dev, sc->initial_aperture); agp_amd_free_gatt(sc->gatt); + agp_free_res(dev); bus_release_resource(dev, SYS_RES_MEMORY, AGP_AMD751_REGISTERS, sc->regs); Index: agp_amd64.c =================================================================== RCS file: /usr/cvs/src/sys/pci/agp_amd64.c,v retrieving revision 1.14 diff -u -r1.14 agp_amd64.c --- agp_amd64.c 9 Oct 2006 20:26:32 -0000 1.14 +++ agp_amd64.c 25 Oct 2007 18:29:48 -0000 @@ -250,10 +250,9 @@ agp_amd64_detach(device_t dev) { struct agp_amd64_softc *sc = device_get_softc(dev); - int i, error; + int i; - if ((error = agp_generic_detach(dev))) - return (error); + agp_free_cdev(dev); for (i = 0; i < sc->n_mctrl; i++) pci_cfgregwrite(0, sc->mctrl[i], 3, AGP_AMD64_APCTRL, @@ -262,6 +261,7 @@ AGP_SET_APERTURE(dev, sc->initial_aperture); agp_free_gatt(sc->gatt); + agp_free_res(dev); return (0); } Index: agp_ati.c =================================================================== RCS file: /usr/cvs/src/sys/pci/agp_ati.c,v retrieving revision 1.3 diff -u -r1.3 agp_ati.c --- agp_ati.c 1 Sep 2006 02:22:17 -0000 1.3 +++ agp_ati.c 25 Oct 2007 18:30:50 -0000 @@ -248,18 +248,15 @@ agp_ati_detach(device_t dev) { struct agp_ati_softc *sc = device_get_softc(dev); - int error; u_int32_t apsize_reg, temp; + agp_free_cdev(dev); + if (sc->is_rs300) apsize_reg = ATI_RS300_APSIZE; else apsize_reg = ATI_RS100_APSIZE; - error = agp_generic_detach(dev); - if (error) - return error; - /* Clear the GATT base */ WRITE4(ATI_GART_BASE, 0); @@ -273,6 +270,7 @@ free(sc->ag_virtual, M_AGP); bus_release_resource(dev, SYS_RES_MEMORY, ATI_GART_MMADDR, sc->regs); + agp_free_res(dev); return 0; } Index: agp_i810.c =================================================================== RCS file: /usr/cvs/src/sys/pci/agp_i810.c,v retrieving revision 1.41 diff -u -r1.41 agp_i810.c --- agp_i810.c 15 Sep 2007 18:16:35 -0000 1.41 +++ agp_i810.c 25 Oct 2007 18:31:48 -0000 @@ -583,11 +583,8 @@ agp_i810_detach(device_t dev) { struct agp_i810_softc *sc = device_get_softc(dev); - int error; - error = agp_generic_detach(dev); - if (error) - return error; + agp_free_cdev(dev); /* Clear the GATT base. */ if ( sc->chiptype == CHIP_I810 ) { @@ -608,6 +605,7 @@ free(sc->gatt, M_AGP); bus_release_resources(dev, sc->sc_res_spec, sc->sc_res); + agp_free_res(dev); return 0; } Index: agp_intel.c =================================================================== RCS file: /usr/cvs/src/sys/pci/agp_intel.c,v retrieving revision 1.34 diff -u -r1.34 agp_intel.c --- agp_intel.c 6 Jan 2007 08:31:31 -0000 1.34 +++ agp_intel.c 25 Oct 2007 18:32:13 -0000 @@ -262,13 +262,10 @@ { struct agp_intel_softc *sc; u_int32_t reg; - int error; sc = device_get_softc(dev); - error = agp_generic_detach(dev); - if (error) - return (error); + agp_free_cdev(dev); /* Disable aperture accesses. */ switch (pci_get_devid(dev)) { @@ -305,6 +302,7 @@ pci_write_config(dev, AGP_INTEL_ATTBASE, 0, 4); AGP_SET_APERTURE(dev, sc->initial_aperture); agp_free_gatt(sc->gatt); + agp_free_res(dev); return (0); } Index: agp_nvidia.c =================================================================== RCS file: /usr/cvs/src/sys/pci/agp_nvidia.c,v retrieving revision 1.11 diff -u -r1.11 agp_nvidia.c --- agp_nvidia.c 20 Dec 2005 21:12:26 -0000 1.11 +++ agp_nvidia.c 25 Oct 2007 18:32:37 -0000 @@ -247,12 +247,9 @@ agp_nvidia_detach (device_t dev) { struct agp_nvidia_softc *sc = device_get_softc(dev); - int error; u_int32_t temp; - error = agp_generic_detach(dev); - if (error) - return (error); + agp_free_cdev(dev); /* GART Control */ temp = pci_read_config(sc->dev, AGP_NVIDIA_0_APSIZE, 4); @@ -270,6 +267,7 @@ sc->initial_aperture); agp_free_gatt(sc->gatt); + agp_free_res(dev); return (0); } Index: agp_sis.c =================================================================== RCS file: /usr/cvs/src/sys/pci/agp_sis.c,v retrieving revision 1.20 diff -u -r1.20 agp_sis.c --- agp_sis.c 30 May 2006 18:41:26 -0000 1.20 +++ agp_sis.c 25 Oct 2007 18:32:57 -0000 @@ -173,11 +173,8 @@ agp_sis_detach(device_t dev) { struct agp_sis_softc *sc = device_get_softc(dev); - int error; - error = agp_generic_detach(dev); - if (error) - return error; + agp_free_cdev(dev); /* Disable the aperture.. */ pci_write_config(dev, AGP_SIS_WINCTRL, @@ -190,6 +187,7 @@ AGP_SET_APERTURE(dev, sc->initial_aperture); agp_free_gatt(sc->gatt); + agp_free_res(dev); return 0; } Index: agp_via.c =================================================================== RCS file: /usr/cvs/src/sys/pci/agp_via.c,v retrieving revision 1.24 diff -u -r1.24 agp_via.c --- agp_via.c 21 Sep 2007 02:10:13 -0000 1.24 +++ agp_via.c 25 Oct 2007 18:33:18 -0000 @@ -240,16 +240,14 @@ agp_via_detach(device_t dev) { struct agp_via_softc *sc = device_get_softc(dev); - int error; - error = agp_generic_detach(dev); - if (error) - return error; + agp_free_cdev(dev); pci_write_config(dev, sc->regs[REG_GARTCTRL], 0, 4); pci_write_config(dev, sc->regs[REG_ATTBASE], 0, 4); AGP_SET_APERTURE(dev, sc->initial_aperture); agp_free_gatt(sc->gatt); + agp_free_res(dev); return 0; } Index: agppriv.h =================================================================== RCS file: /usr/cvs/src/sys/pci/agppriv.h,v retrieving revision 1.6 diff -u -r1.6 agppriv.h --- agppriv.h 13 Jul 2007 16:28:12 -0000 1.6 +++ agppriv.h 25 Oct 2007 18:28:29 -0000 @@ -90,7 +90,9 @@ u_int8_t agp_find_caps(device_t dev); struct agp_gatt *agp_alloc_gatt(device_t dev); void agp_set_aperture_resource(device_t dev, int rid); +void agp_free_cdev(device_t dev); void agp_free_gatt(struct agp_gatt *gatt); +void agp_free_res(device_t dev); int agp_generic_attach(device_t dev); int agp_generic_detach(device_t dev); int agp_generic_get_aperture(device_t dev); -- John Baldwin