Quoting Robin Murphy (2018-09-26 06:24:42) > On 21/09/18 18:39, Stephen Boyd wrote: > > Quoting Robin Murphy (2018-09-21 04:09:50) > >> Hi Stephen, > >> > >> On 20/09/18 23:35, Stephen Boyd wrote: > >>> I recently debugged a DMA mapping oops where a driver was trying to map > >>> a buffer returned from request_firmware() with dma_map_single(). Memory > >>> returned from request_firmware() is mapped into the vmalloc region and > >>> this isn't a valid region to map with dma_map_single() per the DMA > >>> documentation's "What memory is DMA'able?" section. > >>> > >>> Unfortunately, we don't really check that in the DMA debugging code, so > >>> enabling DMA debugging doesn't help catch this problem. Let's add a new > >>> DMA debug function to check for a vmalloc address and print a warning if > >>> this happens. This makes it a little easier to debug these sorts of > >>> problems, instead of seeing odd behavior or crashes when drivers attempt > >>> to map the vmalloc space for DMA. > >> > >> Good idea! > >> > >>> Cc: Marek Szyprowski <[email protected]> > >>> Cc: Robin Murphy <[email protected]> > >>> Signed-off-by: Stephen Boyd <[email protected]> > >>> --- > >>> include/linux/dma-debug.h | 8 ++++++++ > >>> include/linux/dma-mapping.h | 1 + > >>> kernel/dma/debug.c | 12 ++++++++++++ > >>> 3 files changed, 21 insertions(+) > >> > >> However I can't help thinking this looks a little heavyweight for a > >> single specific check. It seems like it would be enough to simply pass > >> the VA as an extra argument to debug_dma_map_page(), since we already > >> have the map_single argument which would indicate when it's valid. What > >> do you reckon? > > > > I thought about augmenting debug_dma_map_page() but that has another > > problem where those checks are done after the page has been mapped. The > > kernel can oops when it's operating on the incorrect page so we need to > > check things before calling the dma ops. > > Oh, right, as usual I've managed to overlook a subtlety :) > > In fact, with memory now jogged, I think the virt_to_page() itself is > allowed to blow up if !virt_addr_valid(), so we probably do need > something like your original idea to be properly robust. I guess it > probably makes sense to implement that as debug_dma_map_single() then, > since the purpose is really to cover any VA checks specific to that case > which debug_dma_map_page() can't be expected to do after the fact. >
Ok. I can rename it to dma_debug_map_single() and throw in a virt_addr_valid() check too. Thanks for the suggestion. _______________________________________________ iommu mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/iommu
