Re: [PATCH 1/3] mtd: spi-nor: add optional DMA-safe bounce buffer for data transfer

On Friday 29 December 2017 11:33 PM, Trent Piepho wrote:

On Fri, 2017-12-29 at 15:46 +0530, Vignesh R wrote:

quoted

On Friday 29 December 2017 12:24 AM, Trent Piepho wrote:

quoted

quoted

Vignesh has suggested to call virt_addr_valid() instead.
I think Boris has also told me about this function.
So it might be the right solution. What do you think about their proposal?

Not sure what exactly the differences are between these methods.  The
fact that each of the many existing DMA fixes uses slightly different
code to detect what is unsafe speaks to the difficulty of this problem!

My understanding based on Documentation/DMA-API-HOWTO.txt and
Documentation/arm/memory.txt is that
virt_addr_valid() will guarantee that address is in range of
PAGE_OFFSET to high_memory-1 (Kernel direct-mapped RAM region) which is
address range of buffers that are DMA'able.

There's code in gpmi-nand.c that does:

        /* first try to map the upper buffer directly */
        if (virt_addr_valid(this->upper_buf) &&
                !object_is_on_stack(this->upper_buf)) {
                sg_init_one(sgl, this->upper_buf, this->upper_len);

So whoever wrote that thought that stack objects needed an additional
test beyond virt_addr_valid.  But it does appear to be far more common
to depend on just virt_addr_valid, so perhaps the code in gpmi-nand is
in error.

The test in gpmi-nand.c is right. Enabling CONFIG_DMA_API_DEBUG does
warn about DMA'ing into stack objects (in lib/dma-debug.c). So other
places needs to be fixed, I guess.

quoted

quoted

 virt_addr_valid() is already used by spi-ti-qspi.  spi core uses for
the buffer map helper, but that code path is for buffers which are NOT
vmalloc or highmem, but are still not virt_addr_valid() for some other
reason.

       if (vmalloced_buf || kmap_buf) {
               /* Handle vmalloc'd or kmap'd buffers */
               ...

This stuff does get DMAed.  So I have to wonder, if spi.c thinks it can
use DMA with vmalloc or highmem, couldn't spi-not do the same instead
of the bounce buffer?

SPI core does try to DMA into underlying physical pages of vmalloc'd
buffer. But this has two problems:

1. Does not work well with VIVT caches[1].
2. On ARM LPAE systems, vmalloc'd buffers can be from highmem region
that are not addressable using 32 bit addresses and is backed by LPAE.
So, a 32 bit DMA cannot access these buffers.

Both these issues lead to random crashes and errors with UBIFS and JFFS2
flash file systems which this patch series tries to address using bounce
buffer

[1] https://patchwork.kernel.org/patch/9579553/


-- 
Regards
Vignesh