Download raw body.
amd64 bus dma useless lastaddr variable
Hi,
On Mon, Aug 26, 2024 at 02:23:16PM +0200, Alexander Bluhm wrote:
> Hi,
>
> I think the lastaddr variable in amd64 bus dma is redundant. Instead
> of storing the final address of a segment in the caller,
> _bus_dmamap_load_buffer() can inspect the previous segment. Also
> in _bus_dmamap_load_raw() and _bus_dmamem_alloc_range() the previous
> segement is available and can be used directly.
>
> ok?
looks reasonable; light testing shows no obvious problems.
> bluhm
>
> Index: arch/amd64/amd64/bus_dma.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/arch/amd64/amd64/bus_dma.c,v
> diff -u -p -r1.57 bus_dma.c
> --- arch/amd64/amd64/bus_dma.c 22 Aug 2024 11:36:24 -0000 1.57
> +++ arch/amd64/amd64/bus_dma.c 23 Aug 2024 17:48:50 -0000
> @@ -102,7 +102,7 @@
> #endif
>
> int _bus_dmamap_load_buffer(bus_dma_tag_t, bus_dmamap_t, void *, bus_size_t,
> - struct proc *, int, paddr_t *, int *, int *, int);
> + struct proc *, int, int *, int *, int);
>
> /*
> * Common function for DMA map creation. May be called by bus-specific
> @@ -254,7 +254,6 @@ int
> _bus_dmamap_load(bus_dma_tag_t t, bus_dmamap_t map, void *buf,
> bus_size_t buflen, struct proc *p, int flags)
> {
> - bus_addr_t lastaddr = 0;
> int seg, used, error;
>
> /*
> @@ -269,7 +268,7 @@ _bus_dmamap_load(bus_dma_tag_t t, bus_dm
> seg = 0;
> used = 0;
> error = _bus_dmamap_load_buffer(t, map, buf, buflen, p, flags,
> - &lastaddr, &seg, &used, 1);
> + &seg, &used, 1);
> if (error == 0) {
> map->dm_mapsize = buflen;
> map->dm_nsegs = seg + 1;
> @@ -285,7 +284,6 @@ int
> _bus_dmamap_load_mbuf(bus_dma_tag_t t, bus_dmamap_t map, struct mbuf *m0,
> int flags)
> {
> - paddr_t lastaddr = 0;
> int seg, used, error, first;
> struct mbuf *m;
>
> @@ -311,7 +309,7 @@ _bus_dmamap_load_mbuf(bus_dma_tag_t t, b
> if (m->m_len == 0)
> continue;
> error = _bus_dmamap_load_buffer(t, map, m->m_data, m->m_len,
> - NULL, flags, &lastaddr, &seg, &used, first);
> + NULL, flags, &seg, &used, first);
> first = 0;
> }
> if (error == 0) {
> @@ -329,7 +327,6 @@ int
> _bus_dmamap_load_uio(bus_dma_tag_t t, bus_dmamap_t map, struct uio *uio,
> int flags)
> {
> - paddr_t lastaddr = 0;
> int seg, used, i, error, first;
> bus_size_t minlen, resid;
> struct proc *p = NULL;
> @@ -366,7 +363,7 @@ _bus_dmamap_load_uio(bus_dma_tag_t t, bu
> addr = (caddr_t)iov[i].iov_base;
>
> error = _bus_dmamap_load_buffer(t, map, addr, minlen,
> - p, flags, &lastaddr, &seg, &used, first);
> + p, flags, &seg, &used, first);
> first = 0;
>
> resid -= minlen;
> @@ -387,7 +384,7 @@ int
> _bus_dmamap_load_raw(bus_dma_tag_t t, bus_dmamap_t map, bus_dma_segment_t *segs,
> int nsegs, bus_size_t size, int flags)
> {
> - bus_addr_t paddr, baddr, bmask, lastaddr = 0;
> + bus_addr_t paddr, baddr, bmask;
> bus_size_t plen, sgsize, mapsize;
> int first = 1;
> int i, seg = 0;
> @@ -439,14 +436,15 @@ _bus_dmamap_load_raw(bus_dma_tag_t t, bu
> map->dm_segs[seg].ds_len = sgsize;
> first = 0;
> } else {
> - if (paddr == lastaddr &&
> + if (paddr == (map->dm_segs[seg].ds_addr +
> + map->dm_segs[seg].ds_len) &&
> (map->dm_segs[seg].ds_len + sgsize) <=
> map->_dm_maxsegsz &&
> (map->_dm_boundary == 0 ||
> (map->dm_segs[seg].ds_addr & bmask) ==
> - (paddr & bmask)))
> + (paddr & bmask))) {
> map->dm_segs[seg].ds_len += sgsize;
> - else {
> + } else {
> if (++seg >= map->_dm_segcnt)
> return (EINVAL);
> map->dm_segs[seg].ds_addr = paddr;
> @@ -457,8 +455,6 @@ _bus_dmamap_load_raw(bus_dma_tag_t t, bu
> paddr += sgsize;
> plen -= sgsize;
> size -= sgsize;
> -
> - lastaddr = paddr;
> }
> }
>
> @@ -686,18 +682,17 @@ _bus_dmamem_mmap(bus_dma_tag_t t, bus_dm
> * DMA utility functions
> **********************************************************************/
> /*
> - * Utility function to load a linear buffer. lastaddrp holds state
> - * between invocations (for multiple-buffer loads). segp contains
> + * Utility function to load a linear buffer. segp contains
> * the starting segment on entrance, and the ending segment on exit.
> * first indicates if this is the first invocation of this function.
> */
> int
> _bus_dmamap_load_buffer(bus_dma_tag_t t, bus_dmamap_t map, void *buf,
> - bus_size_t buflen, struct proc *p, int flags, paddr_t *lastaddrp,
> + bus_size_t buflen, struct proc *p, int flags,
> int *segp, int *usedp, int first)
> {
> bus_size_t sgsize;
> - bus_addr_t curaddr, lastaddr, baddr, bmask;
> + bus_addr_t curaddr, baddr, bmask;
> vaddr_t pgva = -1, vaddr = (vaddr_t)buf;
> int seg, page, off;
> pmap_t pmap;
> @@ -710,7 +705,6 @@ _bus_dmamap_load_buffer(bus_dma_tag_t t,
> pmap = pmap_kernel();
>
> page = *usedp;
> - lastaddr = *lastaddrp;
> bmask = ~(map->_dm_boundary - 1);
>
> for (seg = *segp; buflen > 0 ; ) {
> @@ -762,7 +756,8 @@ _bus_dmamap_load_buffer(bus_dma_tag_t t,
> map->dm_segs[seg]._ds_bounce_va = pgva;
> first = 0;
> } else {
> - if (curaddr == lastaddr &&
> + if (curaddr == (map->dm_segs[seg].ds_addr +
> + map->dm_segs[seg].ds_len) &&
> (map->dm_segs[seg].ds_len + sgsize) <=
> map->_dm_maxsegsz &&
> (map->_dm_boundary == 0 ||
> @@ -781,14 +776,12 @@ _bus_dmamap_load_buffer(bus_dma_tag_t t,
> }
> }
>
> - lastaddr = curaddr + sgsize;
> vaddr += sgsize;
> buflen -= sgsize;
> }
>
> *segp = seg;
> *usedp = page;
> - *lastaddrp = lastaddr;
>
> /*
> * Did we fit?
> @@ -807,7 +800,7 @@ _bus_dmamem_alloc_range(bus_dma_tag_t t,
> bus_size_t boundary, bus_dma_segment_t *segs, int nsegs, int *rsegs,
> int flags, bus_addr_t low, bus_addr_t high)
> {
> - paddr_t curaddr, lastaddr;
> + paddr_t curaddr;
> struct vm_page *m;
> struct pglist mlist;
> int curseg, error, plaflag;
> @@ -837,7 +830,7 @@ _bus_dmamem_alloc_range(bus_dma_tag_t t,
> */
> m = TAILQ_FIRST(&mlist);
> curseg = 0;
> - lastaddr = segs[curseg].ds_addr = VM_PAGE_TO_PHYS(m);
> + segs[curseg].ds_addr = VM_PAGE_TO_PHYS(m);
> segs[curseg].ds_len = PAGE_SIZE;
>
> for (m = TAILQ_NEXT(m, pageq); m != NULL; m = TAILQ_NEXT(m, pageq)) {
> @@ -853,14 +846,13 @@ _bus_dmamem_alloc_range(bus_dma_tag_t t,
> panic("_bus_dmamem_alloc_range");
> }
> #endif
> - if (curaddr == (lastaddr + PAGE_SIZE))
> + if (curaddr == (segs[curseg].ds_addr + segs[curseg].ds_len)) {
> segs[curseg].ds_len += PAGE_SIZE;
> - else {
> + } else {
> curseg++;
> segs[curseg].ds_addr = curaddr;
> segs[curseg].ds_len = PAGE_SIZE;
> }
> - lastaddr = curaddr;
> }
>
> *rsegs = curseg + 1;
>
--
Dr. Hans-Jörg Höxer Hans-Joerg_Hoexer@genua.de
Senior Expert Kryptographie
eXtreme Kernel and Crypto Development
genua GmbH
Domagkstrasse 7, 85551 Kirchheim bei München
tel +49 89 991950-0, fax -999, www.genua.de
Geschäftsführer: Matthias Ochs, Marc Tesch
Amtsgericht München HRB 98238
genua ist ein Unternehmen der Bundesdruckerei-Gruppe.
amd64 bus dma useless lastaddr variable