From owner-freebsd-current@FreeBSD.ORG Fri Jul 13 17:55:55 2012 Return-Path: Delivered-To: current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id C54091065674 for ; Fri, 13 Jul 2012 17:55:55 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from bigwig.baldwin.cx (bigknife-pt.tunnel.tserv9.chi1.ipv6.he.net [IPv6:2001:470:1f10:75::2]) by mx1.freebsd.org (Postfix) with ESMTP id 6E6068FC08 for ; Fri, 13 Jul 2012 17:55:55 +0000 (UTC) Received: from jhbbsd.localnet (unknown [209.249.190.124]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id E0748B926; Fri, 13 Jul 2012 13:55:54 -0400 (EDT) From: John Baldwin To: Scott Long Date: Fri, 13 Jul 2012 13:55:54 -0400 User-Agent: KMail/1.13.5 (FreeBSD/8.2-CBSD-20110714-p17; KDE/4.5.5; amd64; ; ) References: <201207121040.27116.jhb@freebsd.org> <1342111033.198.YahooMailNeo@web45716.mail.sp1.yahoo.com> <201207121351.20951.jhb@freebsd.org> In-Reply-To: <201207121351.20951.jhb@freebsd.org> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201207131355.54432.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Fri, 13 Jul 2012 13:55:55 -0400 (EDT) Cc: Peter Jeremy , "current@freebsd.org" Subject: Re: Adding support for WC (write-combining) memory to bus_dma 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: Fri, 13 Jul 2012 17:55:55 -0000 On Thursday, July 12, 2012 1:51:20 pm John Baldwin wrote: > On Thursday, July 12, 2012 12:37:13 pm Scott Long wrote: > > Yup, this is a problem, and I like your fix; this kind of state is exactly what > > belongs in the map. > > Why don't I break out the fix first as a separate patch. Here is a patch to just fix this. I also mirrored the changes into powerpc since it had copied the same code from x86 (albeit disabled). If you feel the malloc stats via M_DEVBUF are important, I can restore those (probably by adding a contigmalloc_attr() wrapper). Index: powerpc/powerpc/busdma_machdep.c =================================================================== --- powerpc/powerpc/busdma_machdep.c (revision 238365) +++ powerpc/powerpc/busdma_machdep.c (working copy) @@ -46,6 +46,8 @@ #include #include +#include +#include #include #include @@ -130,6 +132,7 @@ bus_dmamap_callback_t *callback; void *callback_arg; STAILQ_ENTRY(bus_dmamap) links; + int contigalloc; }; static STAILQ_HEAD(, bus_dmamap) bounce_map_waitinglist; @@ -489,6 +492,7 @@ bus_dmamem_alloc(bus_dma_tag_t dmat, void** vaddr, int flags, bus_dmamap_t *mapp) { + vm_memattr_t attr; int mflags; if (flags & BUS_DMA_NOWAIT) @@ -500,6 +504,12 @@ if (flags & BUS_DMA_ZERO) mflags |= M_ZERO; +#ifdef NOTYET + if (flags & BUS_DMA_NOCACHE) + attr = VM_MEMATTR_UNCACHEABLE; + else +#endif + attr = VM_MEMATTR_DEFAULT; /* * XXX: @@ -511,7 +521,8 @@ */ if ((dmat->maxsize <= PAGE_SIZE) && (dmat->alignment < dmat->maxsize) && - dmat->lowaddr >= ptoa((vm_paddr_t)Maxmem)) { + dmat->lowaddr >= ptoa((vm_paddr_t)Maxmem) && + attr == VM_MEMATTR_DEFAULT) { *vaddr = malloc(dmat->maxsize, M_DEVBUF, mflags); } else { /* @@ -520,9 +531,10 @@ * multi-seg allocations yet though. * XXX Certain AGP hardware does. */ - *vaddr = contigmalloc(dmat->maxsize, M_DEVBUF, mflags, - 0ul, dmat->lowaddr, dmat->alignment? dmat->alignment : 1ul, - dmat->boundary); + *vaddr = (void *)kmem_alloc_contig(kernel_map, dmat->maxsize, + mflags, 0ul, dmat->lowaddr, dmat->alignment ? + dmat->alignment : 1ul, dmat->boundary, attr); + (*mapp)->contigalloc = 1; } if (*vaddr == NULL) { CTR4(KTR_BUSDMA, "%s: tag %p tag flags 0x%x error %d", @@ -531,11 +543,6 @@ } else if (vtophys(*vaddr) & (dmat->alignment - 1)) { printf("bus_dmamem_alloc failed to align memory properly.\n"); } -#ifdef NOTYET - if (flags & BUS_DMA_NOCACHE) - pmap_change_attr((vm_offset_t)*vaddr, dmat->maxsize, - VM_MEMATTR_UNCACHEABLE); -#endif CTR4(KTR_BUSDMA, "%s: tag %p tag flags 0x%x error %d", __func__, dmat, dmat->flags, 0); return (0); @@ -548,18 +555,12 @@ void bus_dmamem_free(bus_dma_tag_t dmat, void *vaddr, bus_dmamap_t map) { - bus_dmamap_destroy(dmat, map); -#ifdef NOTYET - pmap_change_attr((vm_offset_t)vaddr, dmat->maxsize, VM_MEMATTR_DEFAULT); -#endif - if ((dmat->maxsize <= PAGE_SIZE) && - (dmat->alignment < dmat->maxsize) && - dmat->lowaddr >= ptoa((vm_paddr_t)Maxmem)) + if (!map->contigalloc) free(vaddr, M_DEVBUF); - else { - contigfree(vaddr, dmat->maxsize, M_DEVBUF); - } + else + kmem_free(kernel_map, (vm_offset_t)vaddr, dmat->maxsize); + bus_dmamap_destroy(dmat, map); CTR3(KTR_BUSDMA, "%s: tag %p flags 0x%x", __func__, dmat, dmat->flags); } Index: x86/x86/busdma_machdep.c =================================================================== --- x86/x86/busdma_machdep.c (revision 238365) +++ x86/x86/busdma_machdep.c (working copy) @@ -42,6 +42,8 @@ #include #include +#include +#include #include #include @@ -131,7 +133,7 @@ static STAILQ_HEAD(, bus_dmamap) bounce_map_waitinglist; static STAILQ_HEAD(, bus_dmamap) bounce_map_callbacklist; -static struct bus_dmamap nobounce_dmamap; +static struct bus_dmamap nobounce_dmamap, contig_dmamap; static void init_bounce_pages(void *dummy); static int alloc_bounce_zone(bus_dma_tag_t dmat); @@ -465,7 +467,7 @@ int bus_dmamap_destroy(bus_dma_tag_t dmat, bus_dmamap_t map) { - if (map != NULL && map != &nobounce_dmamap) { + if (map != NULL && map != &nobounce_dmamap && map != &contig_dmamap) { if (STAILQ_FIRST(&map->bpages) != NULL) { CTR3(KTR_BUSDMA, "%s: tag %p error %d", __func__, dmat, EBUSY); @@ -490,6 +492,7 @@ bus_dmamem_alloc(bus_dma_tag_t dmat, void** vaddr, int flags, bus_dmamap_t *mapp) { + vm_memattr_t attr; int mflags; if (flags & BUS_DMA_NOWAIT) @@ -512,6 +515,10 @@ } if (flags & BUS_DMA_ZERO) mflags |= M_ZERO; + if (flags & BUS_DMA_NOCACHE) + attr = VM_MEMATTR_UNCACHEABLE; + else + attr = VM_MEMATTR_DEFAULT; /* * XXX: @@ -523,7 +530,8 @@ */ if ((dmat->maxsize <= PAGE_SIZE) && (dmat->alignment < dmat->maxsize) && - dmat->lowaddr >= ptoa((vm_paddr_t)Maxmem)) { + dmat->lowaddr >= ptoa((vm_paddr_t)Maxmem) && + attr == VM_MEMATTR_DEFAULT) { *vaddr = malloc(dmat->maxsize, M_DEVBUF, mflags); } else { /* @@ -532,9 +540,10 @@ * multi-seg allocations yet though. * XXX Certain AGP hardware does. */ - *vaddr = contigmalloc(dmat->maxsize, M_DEVBUF, mflags, - 0ul, dmat->lowaddr, dmat->alignment? dmat->alignment : 1ul, - dmat->boundary); + *vaddr = (void *)kmem_alloc_contig(kernel_map, dmat->maxsize, + mflags, 0ul, dmat->lowaddr, dmat->alignment ? + dmat->alignment : 1ul, dmat->boundary, attr); + *mapp = &contig_dmamap; } if (*vaddr == NULL) { CTR4(KTR_BUSDMA, "%s: tag %p tag flags 0x%x error %d", @@ -543,9 +552,6 @@ } else if (vtophys(*vaddr) & (dmat->alignment - 1)) { printf("bus_dmamem_alloc failed to align memory properly.\n"); } - if (flags & BUS_DMA_NOCACHE) - pmap_change_attr((vm_offset_t)*vaddr, dmat->maxsize, - PAT_UNCACHEABLE); CTR4(KTR_BUSDMA, "%s: tag %p tag flags 0x%x error %d", __func__, dmat, dmat->flags, 0); return (0); @@ -560,18 +566,15 @@ { /* * dmamem does not need to be bounced, so the map should be - * NULL + * NULL if malloc() was used and contig_dmamap if + * contigmalloc() was used. */ - if (map != NULL) + if (!(map == NULL || map == &contig_dmamap)) panic("bus_dmamem_free: Invalid map freed\n"); - pmap_change_attr((vm_offset_t)vaddr, dmat->maxsize, PAT_WRITE_BACK); - if ((dmat->maxsize <= PAGE_SIZE) && - (dmat->alignment < dmat->maxsize) && - dmat->lowaddr >= ptoa((vm_paddr_t)Maxmem)) + if (map == NULL) free(vaddr, M_DEVBUF); - else { - contigfree(vaddr, dmat->maxsize, M_DEVBUF); - } + else + kmem_free(kernel_map, (vm_offset_t)vaddr, dmat->maxsize); CTR3(KTR_BUSDMA, "%s: tag %p flags 0x%x", __func__, dmat, dmat->flags); } @@ -662,7 +665,7 @@ vm_offset_t vaddr; int seg, error; - if (map == NULL) + if (map == NULL || map == &contig_dmamap) map = &nobounce_dmamap; if ((dmat->flags & BUS_DMA_COULD_BOUNCE) != 0) { @@ -1139,7 +1142,7 @@ struct bounce_page *bpage; KASSERT(dmat->bounce_zone != NULL, ("no bounce zone in dma tag")); - KASSERT(map != NULL && map != &nobounce_dmamap, + KASSERT(map != NULL && map != &nobounce_dmamap && map != &contig_dmamap, ("add_bounce_page: bad map %p", map)); bz = dmat->bounce_zone; -- John Baldwin