From owner-freebsd-arch@FreeBSD.ORG Sun Feb 1 15:35:01 2015 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 08252458; Sun, 1 Feb 2015 15:35:01 +0000 (UTC) Received: from smtp2.ore.mailhop.org (smtp2.ore.mailhop.org [54.186.57.195]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id DD69419F; Sun, 1 Feb 2015 15:35:00 +0000 (UTC) Received: from [73.34.117.227] (helo=ilsoft.org) by smtp2.ore.mailhop.org with esmtpsa (TLSv1.2:DHE-RSA-AES256-GCM-SHA384:256) (Exim 4.82) (envelope-from ) id 1YHvqp-0000fc-NU; Sun, 01 Feb 2015 14:50:15 +0000 Received: from revolution.hippie.lan (revolution.hippie.lan [172.22.42.240]) by ilsoft.org (8.14.9/8.14.9) with ESMTP id t11EoE7C073554; Sun, 1 Feb 2015 07:50:14 -0700 (MST) (envelope-from ian@freebsd.org) X-Mail-Handler: DuoCircle Outbound SMTP X-Originating-IP: 73.34.117.227 X-Report-Abuse-To: abuse@duocircle.com (see https://support.duocircle.com/support/solutions/articles/5000540958-duocircle-standard-smtp-abuse-information for abuse reporting information) X-MHO-User: U2FsdGVkX18uYwlF8eAv8lJAljjxMCGy Message-ID: <1422802214.15718.253.camel@freebsd.org> Subject: Re: Wrapper API for static bus_dma allocations From: Ian Lepore To: John Baldwin Date: Sun, 01 Feb 2015 07:50:14 -0700 In-Reply-To: <2800970.jY4xzTy9Hz@ralph.baldwin.cx> References: <2800970.jY4xzTy9Hz@ralph.baldwin.cx> Content-Type: text/plain; charset="us-ascii" X-Mailer: Evolution 3.12.8 FreeBSD GNOME Team Port Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Cc: 'freebsd-arch' X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 01 Feb 2015 15:35:01 -0000 On Thu, 2015-01-29 at 15:37 -0500, John Baldwin wrote: > The bus_dma API to allocate a chunk of static DMA'able memory (e.g. for > descriptor rings) can be a bit obtuse to use in that it require a bit of > boilerplate to create a tag, allocate memory for the tag, then load it to get > the bus address. Similarly, freeing it also requires several steps. In > addition, some of the steps are a bit subtle (e.g. the need to check for an > error in the bus_dma callback instead of from bus_dmamap_load()) and not all > drivers get those correct. > > To try to make this simpler I've written a little wrapper API that tries to > provide a single call to allocate a buffer and a single call to free a buffer. > Each buffer is described by a structure defined by the API, and if the call to > allocate a buffer succeeds, the structure contains both a pointer to the > buffer in the kernel's address space as well as a bus address of the buffer. > > In the interests of simplicity, this API does not allow the buffer to be quite > as fully configured as the underlying bus_dma API, instead it aims to handle > the most common cases that are used in most drivers. As such, it assumes that > the buffer must be one contiguous range of DMA address space, and the only > parameters that can be specified are the parent tag, the alignment of the > buffer, the lowaddr parameter, the size of the buffer to allocate, and the > flags parameter that is normally passed to bus_dmamem_alloc(). I believe that > this should be sufficient to cover the vast majority of the drivers in our > tree. > > I've included below a patch that contains the wrapper API along with a sample > conversion of the ndis driver (chosen at random). If folks like this idea I > will update the patch to include manpage changes as well. > > --- //depot/vendor/freebsd/src/sys/compat/ndis/subr_ndis.c > +++ //depot/user/jhb/cleanup/sys/compat/ndis/subr_ndis.c [...] > +struct bus_dmamem { > + bus_dma_tag_t dma_tag; > + bus_dmamap_t dma_map; > + void *dma_vaddr; > + bus_addr_t dma_baddr; > +}; > + > +/* > + * Allocate a mapping. On success, zero is returned and the 'dma_vaddr' > + * and 'dma_baddr' fields are populated with the virtual and bus addresses, > + * respectively, of the mapping. > + */ > +int bus_dma_mem_create(struct bus_dmamem *mem, bus_dma_tag_t parent, > + bus_size_t alignment, bus_addr_t lowaddr, > + bus_size_t len, int flags); > + > +/* > + * Release a mapping created by bus_dma_mem_create(). > + */ > +void bus_dma_mem_free(struct bus_dmamem *mem); > + > #endif /* _BUS_DMA_H_ */ > > You introduce new functions named bus_dma_mem_xxxx(), and a new structure bus_dmamem. It's already hard enough to remember the oddball mix of underbars in busdma naming, can the new struct be bus_dma_mem? Also, the combo of create/free is most unsatisfying. Typically alloc/free come in pairs, as do create/destroy. These pairings are used pretty consistantly in busdma now, it'd be nice to not have to remember this pair is an exception. (I'm agnostic about whether the new functionality should be alloc/delete or create/destroy.) -- Ian