On Fri, Mar 21, 2025 at 01:41:52PM +0100, Arnd Bergmann wrote: > On Thu, Mar 20, 2025, at 19:39, Mark Brown wrote: > > Yes, like I said elsewhere in the thread 16 is a popular number but I > > suspect that was the thining there. > Ok, so do we assume we don't need the GFP_DMA then? That's > fine with me, but in that case we should probably enable > swiotlb on all arm32 platforms that may have ZONE_DMA smaller > than ZONE_NORMAL. I think that makes sense. > >> - the way that spi_map_buf_attrs() is written, it actually > >> supports addresses from both kmap() and vmalloc() and > >> will attempt to correctly map those rather than reject > >> the buffer. While this sounds like a good idea, handling > >> vmalloc data like this risks stack data corruption > >> on non-coherent platforms when failing to map stack > >> buffers would be the better response. > > IIRC that's there to support filesystems on SPI flashes or some other > > application that uses vmalloc()ed buffers, it's definitely not intended > > to support data on stack. If it does anything for stack allocated data > > that's accidental. > Ok, then the question is what we should do about callers that pass > in stack data. I can send a patch that adds a WARN_ONCE() or similar, > but it would trigger on things like ... (single/double register raw I/O from stack) ... > which happens in a number of drivers but is harmless as long > as the driver doesn't actually try to DMA into that buffer. Hrm, right. I think that usage is reasonable so we probably should allow it rather than forcing things to do an allocation for a transfer that ends up being PIOed anyway almost all the time, OTOH the same API is also used for large transfers like firmware downloads where we don't want to trigger a spurious copy. Having different requirements at different times would be miserable though so I think we need to just accept any buffer and then figure it out inside the API, but I've not fully thought that through yet. > >> __spi_map_msg() already handles the case of an external > >> DMA master through ctlr->dma_map_dev, so I think the same > >> could be used to get a temporary buffer using > >> dma_alloc_noncoherent() inside of spi_write_then_read() > >> in place of the kmalloc(, GFP_DMA). > > That only helps spi_write_then_read() though. > Right, we'd need to mirror this in other interfaces, either changing > the existing ones, or adding safer ones that can be used from regmap > and from drivers that currently allocate their own GFP_DMA buffers > for this purpose. Yes, indeed. I don't have a clear sense for what the best solution is there yet. Possibly some libary code for the "I want to DMA this random memory" use case?
Attachment:
signature.asc
Description: PGP signature
