Author: rdivacky Date: Mon Nov 9 20:29:10 2009 New Revision: 199104 URL: http://svn.freebsd.org/changeset/base/199104
Log: Make isa_dma functions MPSAFE by introducing its own private lock. These functions are selfcontained (ie. they touch only isa_dma.c static variables and hardware) so a private lock is sufficient to prevent races. This changes only i386/amd64 while there are also isa_dma functions for ia64/sparc64. Sparc64 are ones empty stubs and ia64 ones are unused as ia64 does not have isa (says marcel). This patch removes explicit locking of Giant from a few drivers (there are some that requires this but lack ones - this patch fixes this) and also removes the need for implicit locking of Giant from attach routines where it's provided by newbus. Approved by: ed (mentor, implicit) Reviewed by: jhb, attilio (glanced by) Tested by: Giovanni Trematerra <giovanni.trematerra gmail com> IA64 clue: marcel Modified: head/sys/amd64/isa/isa_dma.c head/sys/dev/fdc/fdc.c head/sys/dev/ieee488/ibfoo.c head/sys/i386/isa/isa_dma.c Modified: head/sys/amd64/isa/isa_dma.c ============================================================================== --- head/sys/amd64/isa/isa_dma.c Mon Nov 9 19:56:53 2009 (r199103) +++ head/sys/amd64/isa/isa_dma.c Mon Nov 9 20:29:10 2009 (r199104) @@ -71,6 +71,8 @@ static u_int8_t dma_bounced = 0; static u_int8_t dma_busy = 0; /* Used in isa_dmastart() */ static u_int8_t dma_inuse = 0; /* User for acquire/release */ static u_int8_t dma_auto_mode = 0; +static struct mtx isa_dma_lock; +MTX_SYSINIT(isa_dma_lock, &isa_dma_lock, "isa DMA lock", MTX_DEF); #define VALID_DMA_MASK (7) @@ -84,39 +86,56 @@ int isa_dma_init(int chan, u_int bouncebufsize, int flag) { void *buf; - - /* - * If a DMA channel is shared, both drivers have to call isa_dma_init - * since they don't know that the other driver will do it. - * Just return if we're already set up good. - * XXX: this only works if they agree on the bouncebuf size. This - * XXX: is typically the case since they are multiple instances of - * XXX: the same driver. - */ - if (dma_bouncebuf[chan] != NULL) - return (0); + int contig; #ifdef DIAGNOSTIC if (chan & ~VALID_DMA_MASK) panic("isa_dma_init: channel out of range"); #endif - dma_bouncebufsize[chan] = bouncebufsize; /* Try malloc() first. It works better if it works. */ buf = malloc(bouncebufsize, M_DEVBUF, flag); if (buf != NULL) { - if (isa_dmarangecheck(buf, bouncebufsize, chan) == 0) { - dma_bouncebuf[chan] = buf; - return (0); + if (isa_dmarangecheck(buf, bouncebufsize, chan) != 0) { + free(buf, M_DEVBUF); + buf = NULL; } - free(buf, M_DEVBUF); + contig = 0; } - buf = contigmalloc(bouncebufsize, M_DEVBUF, flag, 0ul, 0xfffffful, + + if (buf == NULL) { + buf = contigmalloc(bouncebufsize, M_DEVBUF, flag, 0ul, 0xfffffful, 1ul, chan & 4 ? 0x20000ul : 0x10000ul); + contig = 1; + } + if (buf == NULL) return (ENOMEM); + + mtx_lock(&isa_dma_lock); + /* + * If a DMA channel is shared, both drivers have to call isa_dma_init + * since they don't know that the other driver will do it. + * Just return if we're already set up good. + * XXX: this only works if they agree on the bouncebuf size. This + * XXX: is typically the case since they are multiple instances of + * XXX: the same driver. + */ + if (dma_bouncebuf[chan] != NULL) { + if (contig) + contigfree(buf, bouncebufsize, M_DEVBUF); + else + free(buf, M_DEVBUF); + mtx_unlock(&isa_dma_lock); + return (0); + } + + dma_bouncebufsize[chan] = bouncebufsize; dma_bouncebuf[chan] = buf; + + mtx_unlock(&isa_dma_lock); + return (0); } @@ -133,12 +152,15 @@ isa_dma_acquire(chan) panic("isa_dma_acquire: channel out of range"); #endif + mtx_lock(&isa_dma_lock); if (dma_inuse & (1 << chan)) { printf("isa_dma_acquire: channel %d already in use\n", chan); + mtx_unlock(&isa_dma_lock); return (EBUSY); } dma_inuse |= (1 << chan); dma_auto_mode &= ~(1 << chan); + mtx_unlock(&isa_dma_lock); return (0); } @@ -155,8 +177,11 @@ isa_dma_release(chan) if (chan & ~VALID_DMA_MASK) panic("isa_dma_release: channel out of range"); + mtx_lock(&isa_dma_lock); if ((dma_inuse & (1 << chan)) == 0) printf("isa_dma_release: channel %d not in use\n", chan); +#else + mtx_lock(&isa_dma_lock); #endif if (dma_busy & (1 << chan)) { @@ -171,6 +196,8 @@ isa_dma_release(chan) dma_inuse &= ~(1 << chan); dma_auto_mode &= ~(1 << chan); + + mtx_unlock(&isa_dma_lock); } /* @@ -186,6 +213,7 @@ isa_dmacascade(chan) panic("isa_dmacascade: channel out of range"); #endif + mtx_lock(&isa_dma_lock); /* set dma channel mode, and set dma channel mode */ if ((chan & 4) == 0) { outb(DMA1_MODE, DMA37MD_CASCADE | chan); @@ -194,6 +222,7 @@ isa_dmacascade(chan) outb(DMA2_MODE, DMA37MD_CASCADE | (chan & 3)); outb(DMA2_SMSK, chan & 3); } + mtx_unlock(&isa_dma_lock); } /* @@ -206,8 +235,11 @@ isa_dmastart(int flags, caddr_t addr, u_ vm_paddr_t phys; int waport; caddr_t newaddr; + int dma_range_checked; - GIANT_REQUIRED; + /* translate to physical */ + phys = pmap_extract(kernel_pmap, (vm_offset_t)addr); + dma_range_checked = isa_dmarangecheck(addr, nbytes, chan); #ifdef DIAGNOSTIC if (chan & ~VALID_DMA_MASK) @@ -217,8 +249,11 @@ isa_dmastart(int flags, caddr_t addr, u_ || (chan >= 4 && (nbytes > (1<<17) || (uintptr_t)addr & 1))) panic("isa_dmastart: impossible request"); + mtx_lock(&isa_dma_lock); if ((dma_inuse & (1 << chan)) == 0) printf("isa_dmastart: channel %d not acquired\n", chan); +#else + mtx_lock(&isa_dma_lock); #endif #if 0 @@ -233,7 +268,7 @@ isa_dmastart(int flags, caddr_t addr, u_ dma_busy |= (1 << chan); - if (isa_dmarangecheck(addr, nbytes, chan)) { + if (dma_range_checked) { if (dma_bouncebuf[chan] == NULL || dma_bouncebufsize[chan] < nbytes) panic("isa_dmastart: bad bounce buffer"); @@ -246,9 +281,6 @@ isa_dmastart(int flags, caddr_t addr, u_ addr = newaddr; } - /* translate to physical */ - phys = pmap_extract(kernel_pmap, (vm_offset_t)addr); - if (flags & ISADMA_RAW) { dma_auto_mode |= (1 << chan); } else { @@ -323,6 +355,7 @@ isa_dmastart(int flags, caddr_t addr, u_ /* unmask channel */ outb(DMA2_SMSK, chan & 3); } + mtx_unlock(&isa_dma_lock); } void @@ -336,6 +369,7 @@ isa_dmadone(int flags, caddr_t addr, int printf("isa_dmadone: channel %d not acquired\n", chan); #endif + mtx_lock(&isa_dma_lock); if (((dma_busy & (1 << chan)) == 0) && (dma_auto_mode & (1 << chan)) == 0 ) printf("isa_dmadone: channel %d not busy\n", chan); @@ -351,6 +385,7 @@ isa_dmadone(int flags, caddr_t addr, int dma_bounced &= ~(1 << chan); } dma_busy &= ~(1 << chan); + mtx_unlock(&isa_dma_lock); } /* @@ -367,8 +402,6 @@ isa_dmarangecheck(caddr_t va, u_int leng vm_offset_t endva; u_int dma_pgmsk = (chan & 4) ? ~(128*1024-1) : ~(64*1024-1); - GIANT_REQUIRED; - endva = (vm_offset_t)round_page((vm_offset_t)va + length); for (; va < (caddr_t) endva ; va += PAGE_SIZE) { phys = trunc_page(pmap_extract(kernel_pmap, (vm_offset_t)va)); @@ -420,13 +453,15 @@ isa_dmarangecheck(caddr_t va, u_int leng * or -1 if the channel requested is not active. * */ -int -isa_dmastatus(int chan) +static int +isa_dmastatus_locked(int chan) { u_long cnt = 0; int ffport, waport; u_long low1, high1, low2, high2; + mtx_assert(&isa_dma_lock, MA_OWNED); + /* channel active? */ if ((dma_inuse & (1 << chan)) == 0) { printf("isa_dmastatus: channel %d not active\n", chan); @@ -472,6 +507,18 @@ isa_dmastatus(int chan) return(cnt); } +int +isa_dmastatus(int chan) +{ + int status; + + mtx_lock(&isa_dma_lock); + status = isa_dmastatus_locked(chan); + mtx_unlock(&isa_dma_lock); + + return (status); +} + /* * Reached terminal count yet ? */ @@ -491,12 +538,16 @@ isa_dmatc(int chan) int isa_dmastop(int chan) { + int status; + + mtx_lock(&isa_dma_lock); if ((dma_inuse & (1 << chan)) == 0) printf("isa_dmastop: channel %d not acquired\n", chan); if (((dma_busy & (1 << chan)) == 0) && ((dma_auto_mode & (1 << chan)) == 0)) { printf("chan %d not busy\n", chan); + mtx_unlock(&isa_dma_lock); return -2 ; } @@ -505,7 +556,12 @@ isa_dmastop(int chan) } else { outb(DMA2_SMSK, (chan & 3) | 4 /* disable mask */); } - return(isa_dmastatus(chan)); + + status = isa_dmastatus_locked(chan); + + mtx_unlock(&isa_dma_lock); + + return (status); } /* Modified: head/sys/dev/fdc/fdc.c ============================================================================== --- head/sys/dev/fdc/fdc.c Mon Nov 9 19:56:53 2009 (r199103) +++ head/sys/dev/fdc/fdc.c Mon Nov 9 20:29:10 2009 (r199104) @@ -781,11 +781,9 @@ fdc_worker(struct fdc_data *fdc) /* Disable ISADMA if we bailed while it was active */ if (fd != NULL && (fd->flags & FD_ISADMA)) { - mtx_lock(&Giant); isa_dmadone( bp->bio_cmd & BIO_READ ? ISADMA_READ : ISADMA_WRITE, fd->fd_ioptr, fd->fd_iosize, fdc->dmachan); - mtx_unlock(&Giant); mtx_lock(&fdc->fdc_mtx); fd->flags &= ~FD_ISADMA; mtx_unlock(&fdc->fdc_mtx); @@ -958,11 +956,9 @@ fdc_worker(struct fdc_data *fdc) /* Setup ISADMA if we need it and have it */ if ((bp->bio_cmd & (BIO_READ|BIO_WRITE|BIO_FMT)) && !(fdc->flags & FDC_NODMA)) { - mtx_lock(&Giant); isa_dmastart( bp->bio_cmd & BIO_READ ? ISADMA_READ : ISADMA_WRITE, fd->fd_ioptr, fd->fd_iosize, fdc->dmachan); - mtx_unlock(&Giant); mtx_lock(&fdc->fdc_mtx); fd->flags |= FD_ISADMA; mtx_unlock(&fdc->fdc_mtx); @@ -1040,11 +1036,9 @@ fdc_worker(struct fdc_data *fdc) /* Finish DMA */ if (fd->flags & FD_ISADMA) { - mtx_lock(&Giant); isa_dmadone( bp->bio_cmd & BIO_READ ? ISADMA_READ : ISADMA_WRITE, fd->fd_ioptr, fd->fd_iosize, fdc->dmachan); - mtx_unlock(&Giant); mtx_lock(&fdc->fdc_mtx); fd->flags &= ~FD_ISADMA; mtx_unlock(&fdc->fdc_mtx); Modified: head/sys/dev/ieee488/ibfoo.c ============================================================================== --- head/sys/dev/ieee488/ibfoo.c Mon Nov 9 19:56:53 2009 (r199103) +++ head/sys/dev/ieee488/ibfoo.c Mon Nov 9 20:29:10 2009 (r199104) @@ -397,18 +397,14 @@ dma_idata(struct upd7210 *u, u_char *dat KASSERT(u->dmachan >= 0, ("Bogus dmachan %d", u->dmachan)); ib = u->ibfoo; ib->mode = DMA_IDATA; - mtx_lock(&Giant); isa_dmastart(ISADMA_READ, data, len, u->dmachan); - mtx_unlock(&Giant); mtx_lock(&u->mutex); upd7210_wr(u, IMR1, IXR1_ENDRX); upd7210_wr(u, IMR2, IMR2_DMAI); gpib_ib_wait_xfer(u, ib); mtx_unlock(&u->mutex); - mtx_lock(&Giant); j = isa_dmastatus(u->dmachan); isa_dmadone(ISADMA_READ, data, len, u->dmachan); - mtx_unlock(&Giant); return (len - j); } @@ -790,14 +786,12 @@ gpib_ib_open(struct cdev *dev, int oflag mtx_unlock(&u->mutex); if (u->dmachan >= 0) { - mtx_lock(&Giant); error = isa_dma_acquire(u->dmachan); if (!error) { error = isa_dma_init(u->dmachan, PAGE_SIZE, M_WAITOK); if (error) isa_dma_release(u->dmachan); } - mtx_unlock(&Giant); } if (error) { @@ -855,9 +849,7 @@ gpib_ib_close(struct cdev *dev, int ofla free(ib, M_IBFOO); if (u->dmachan >= 0) { - mtx_lock(&Giant); isa_dma_release(u->dmachan); - mtx_unlock(&Giant); } mtx_lock(&u->mutex); u->busy = 0; Modified: head/sys/i386/isa/isa_dma.c ============================================================================== --- head/sys/i386/isa/isa_dma.c Mon Nov 9 19:56:53 2009 (r199103) +++ head/sys/i386/isa/isa_dma.c Mon Nov 9 20:29:10 2009 (r199104) @@ -69,6 +69,8 @@ static u_int8_t dma_bounced = 0; static u_int8_t dma_busy = 0; /* Used in isa_dmastart() */ static u_int8_t dma_inuse = 0; /* User for acquire/release */ static u_int8_t dma_auto_mode = 0; +static struct mtx isa_dma_lock; +MTX_SYSINIT(isa_dma_lock, &isa_dma_lock, "isa DMA lock", MTX_DEF); #define VALID_DMA_MASK (7) @@ -82,39 +84,56 @@ int isa_dma_init(int chan, u_int bouncebufsize, int flag) { void *buf; - - /* - * If a DMA channel is shared, both drivers have to call isa_dma_init - * since they don't know that the other driver will do it. - * Just return if we're already set up good. - * XXX: this only works if they agree on the bouncebuf size. This - * XXX: is typically the case since they are multiple instances of - * XXX: the same driver. - */ - if (dma_bouncebuf[chan] != NULL) - return (0); + int contig; #ifdef DIAGNOSTIC if (chan & ~VALID_DMA_MASK) panic("isa_dma_init: channel out of range"); #endif - dma_bouncebufsize[chan] = bouncebufsize; /* Try malloc() first. It works better if it works. */ buf = malloc(bouncebufsize, M_DEVBUF, flag); if (buf != NULL) { - if (isa_dmarangecheck(buf, bouncebufsize, chan) == 0) { - dma_bouncebuf[chan] = buf; - return (0); + if (isa_dmarangecheck(buf, bouncebufsize, chan) != 0) { + free(buf, M_DEVBUF); + buf = NULL; } - free(buf, M_DEVBUF); + contig = 0; } - buf = contigmalloc(bouncebufsize, M_DEVBUF, flag, 0ul, 0xfffffful, + + if (buf == NULL) { + buf = contigmalloc(bouncebufsize, M_DEVBUF, flag, 0ul, 0xfffffful, 1ul, chan & 4 ? 0x20000ul : 0x10000ul); + contig = 1; + } + if (buf == NULL) return (ENOMEM); + + mtx_lock(&isa_dma_lock); + /* + * If a DMA channel is shared, both drivers have to call isa_dma_init + * since they don't know that the other driver will do it. + * Just return if we're already set up good. + * XXX: this only works if they agree on the bouncebuf size. This + * XXX: is typically the case since they are multiple instances of + * XXX: the same driver. + */ + if (dma_bouncebuf[chan] != NULL) { + if (contig) + contigfree(buf, bouncebufsize, M_DEVBUF); + else + free(buf, M_DEVBUF); + mtx_unlock(&isa_dma_lock); + return (0); + } + + dma_bouncebufsize[chan] = bouncebufsize; dma_bouncebuf[chan] = buf; + + mtx_unlock(&isa_dma_lock); + return (0); } @@ -131,12 +150,15 @@ isa_dma_acquire(chan) panic("isa_dma_acquire: channel out of range"); #endif + mtx_lock(&isa_dma_lock); if (dma_inuse & (1 << chan)) { printf("isa_dma_acquire: channel %d already in use\n", chan); + mtx_unlock(&isa_dma_lock); return (EBUSY); } dma_inuse |= (1 << chan); dma_auto_mode &= ~(1 << chan); + mtx_unlock(&isa_dma_lock); return (0); } @@ -153,8 +175,11 @@ isa_dma_release(chan) if (chan & ~VALID_DMA_MASK) panic("isa_dma_release: channel out of range"); + mtx_lock(&isa_dma_lock); if ((dma_inuse & (1 << chan)) == 0) printf("isa_dma_release: channel %d not in use\n", chan); +#else + mtx_lock(&isa_dma_lock); #endif if (dma_busy & (1 << chan)) { @@ -169,6 +194,8 @@ isa_dma_release(chan) dma_inuse &= ~(1 << chan); dma_auto_mode &= ~(1 << chan); + + mtx_unlock(&isa_dma_lock); } /* @@ -184,6 +211,7 @@ isa_dmacascade(chan) panic("isa_dmacascade: channel out of range"); #endif + mtx_lock(&isa_dma_lock); /* set dma channel mode, and set dma channel mode */ if ((chan & 4) == 0) { outb(DMA1_MODE, DMA37MD_CASCADE | chan); @@ -192,6 +220,7 @@ isa_dmacascade(chan) outb(DMA2_MODE, DMA37MD_CASCADE | (chan & 3)); outb(DMA2_SMSK, chan & 3); } + mtx_unlock(&isa_dma_lock); } /* @@ -204,8 +233,11 @@ isa_dmastart(int flags, caddr_t addr, u_ vm_paddr_t phys; int waport; caddr_t newaddr; + int dma_range_checked; - GIANT_REQUIRED; + /* translate to physical */ + phys = pmap_extract(kernel_pmap, (vm_offset_t)addr); + dma_range_checked = isa_dmarangecheck(addr, nbytes, chan); #ifdef DIAGNOSTIC if (chan & ~VALID_DMA_MASK) @@ -215,8 +247,11 @@ isa_dmastart(int flags, caddr_t addr, u_ || (chan >= 4 && (nbytes > (1<<17) || (u_int)addr & 1))) panic("isa_dmastart: impossible request"); + mtx_lock(&isa_dma_lock); if ((dma_inuse & (1 << chan)) == 0) printf("isa_dmastart: channel %d not acquired\n", chan); +#else + mtx_lock(&isa_dma_lock); #endif #if 0 @@ -231,7 +266,7 @@ isa_dmastart(int flags, caddr_t addr, u_ dma_busy |= (1 << chan); - if (isa_dmarangecheck(addr, nbytes, chan)) { + if (dma_range_checked) { if (dma_bouncebuf[chan] == NULL || dma_bouncebufsize[chan] < nbytes) panic("isa_dmastart: bad bounce buffer"); @@ -244,9 +279,6 @@ isa_dmastart(int flags, caddr_t addr, u_ addr = newaddr; } - /* translate to physical */ - phys = pmap_extract(kernel_pmap, (vm_offset_t)addr); - if (flags & ISADMA_RAW) { dma_auto_mode |= (1 << chan); } else { @@ -321,6 +353,7 @@ isa_dmastart(int flags, caddr_t addr, u_ /* unmask channel */ outb(DMA2_SMSK, chan & 3); } + mtx_unlock(&isa_dma_lock); } void @@ -334,6 +367,7 @@ isa_dmadone(int flags, caddr_t addr, int printf("isa_dmadone: channel %d not acquired\n", chan); #endif + mtx_lock(&isa_dma_lock); if (((dma_busy & (1 << chan)) == 0) && (dma_auto_mode & (1 << chan)) == 0 ) printf("isa_dmadone: channel %d not busy\n", chan); @@ -349,6 +383,7 @@ isa_dmadone(int flags, caddr_t addr, int dma_bounced &= ~(1 << chan); } dma_busy &= ~(1 << chan); + mtx_unlock(&isa_dma_lock); } /* @@ -365,8 +400,6 @@ isa_dmarangecheck(caddr_t va, u_int leng vm_offset_t endva; u_int dma_pgmsk = (chan & 4) ? ~(128*1024-1) : ~(64*1024-1); - GIANT_REQUIRED; - endva = (vm_offset_t)round_page((vm_offset_t)va + length); for (; va < (caddr_t) endva ; va += PAGE_SIZE) { phys = trunc_page(pmap_extract(kernel_pmap, (vm_offset_t)va)); @@ -419,13 +452,15 @@ isa_dmarangecheck(caddr_t va, u_int leng * or -1 if the channel requested is not active. * */ -int -isa_dmastatus(int chan) +static int +isa_dmastatus_locked(int chan) { u_long cnt = 0; int ffport, waport; u_long low1, high1, low2, high2; + mtx_assert(&isa_dma_lock, MA_OWNED); + /* channel active? */ if ((dma_inuse & (1 << chan)) == 0) { printf("isa_dmastatus: channel %d not active\n", chan); @@ -471,6 +506,18 @@ isa_dmastatus(int chan) return(cnt); } +int +isa_dmastatus(int chan) +{ + int status; + + mtx_lock(&isa_dma_lock); + status = isa_dmastatus_locked(chan); + mtx_unlock(&isa_dma_lock); + + return (status); +} + /* * Reached terminal count yet ? */ @@ -490,12 +537,16 @@ isa_dmatc(int chan) int isa_dmastop(int chan) { + int status; + + mtx_lock(&isa_dma_lock); if ((dma_inuse & (1 << chan)) == 0) printf("isa_dmastop: channel %d not acquired\n", chan); if (((dma_busy & (1 << chan)) == 0) && ((dma_auto_mode & (1 << chan)) == 0)) { printf("chan %d not busy\n", chan); + mtx_unlock(&isa_dma_lock); return -2 ; } @@ -504,7 +555,12 @@ isa_dmastop(int chan) } else { outb(DMA2_SMSK, (chan & 3) | 4 /* disable mask */); } - return(isa_dmastatus(chan)); + + status = isa_dmastatus_locked(chan); + + mtx_unlock(&isa_dma_lock); + + return (status); } /* _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"