Date: Mon, 27 Nov 2006 22:32:12 +0000 From: Ian Dowse <iedowse@iedowse.com> To: Marius Strobl <marius@FreeBSD.org> Cc: cvs-src@FreeBSD.org, src-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/dev/usb usbdi.c Message-ID: <200611272232.aa46722@nowhere.iedowse.com> In-Reply-To: Your message of "Mon, 27 Nov 2006 18:39:03 GMT." <200611271839.kARId33k039747@repoman.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
In message <200611271839.kARId33k039747@repoman.freebsd.org>, Marius Strobl wri
tes:
> Refine the previous change to only call bus_dmamap_sync() in case of
> an URQ_REQUEST when DMA segments are passed to usbd_start_transfer();
> when the request doesn't include the optional data buffer the size of
> the transfer (xfer->length) is 0, in which case usbd_transfer() won't
> create a DMA map but call usbd_start_transfer() with no DMA segments.
> With the previous change this could result in the bus_dmamap_sync()
> implementation dereferencing the NULL-pointer passed as the DMA map
> argument.
> While at it fix what appears to be a typo in usbd_start_transfer();
> in order to determine wheter usbd_start_transfer() was called with
> DMA segments check whether the number of segments is > 0 rather than
> the pointer to them being > 0.
Thanks for spotting the typo - note though that the recently added
bus_dmamap_sync() call appears to be using the wrong bus_dma tag
and a potentially uninitialised map, so it is likely to only work
on architectures where bus_dmamap_sync doesn't depend on the tag
and map.
The only bus_dmamap_sync() calls in the usb tree at the moment are
ones I added as part of the scatter-gather work a while ago, and
they all relate to the data buffer associated with a transfer. For
the control transfer SETUP packet buffer, each host controller driver
has a "reqdma" field that holds the DMA mapping information. It's
probably easiest to make the changes in the individual host controller
drivers so they do something like
bus_dmamap_sync(reqdma.block->tag,
reqdma.block->map, BUS_DMASYNC_PREWRITE);
after filling out the setup packet.
I guess other memory structures such as descriptors and queue heads
might need bus_dmamap_sync calls too - what are the features of the
platform(s) where the original issues were seen? (e.g. is some IOMMU
operation or memory barrier required between host and I/O access
to memory?) Apart from the handling of data buffers, the USB code
appears to currently assume that with BUS_DMA_COHERENT it doesn't
need any further synchronisation, which can't be right in general.
Ian
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200611272232.aa46722>