linux-kernel - Re: [PATCH 00/39] mtd: nand: denali: 2nd round of Denali NAND IP patch bomb

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 10 Mar 2017 20:00:03 +0900
From:   Masahiro Yamada <yamada.masahiro@...ionext.com>
To:     Boris Brezillon <boris.brezillon@...e-electrons.com>
Cc:     linux-mtd@...ts.infradead.org, devicetree@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Marek Vasut <marek.vasut@...il.com>,
        Brian Norris <computersforpeace@...il.com>,
        Richard Weinberger <richard@....at>,
        David Woodhouse <dwmw2@...radead.org>,
        Cyrille Pitchen <cyrille.pitchen@...el.com>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Subject: Re: [PATCH 00/39] mtd: nand: denali: 2nd round of Denali NAND IP
 patch bomb

Hi Boris,

I am almost getting v2 done,
and now I am testing it.

I am having one problem.  Please teach me.

2016-11-30 17:17 GMT+09:00 Boris Brezillon <boris.brezillon@...e-electrons.com>:
>> [2]
>> Remove driver-internal bounce buffer.
>> The current Denali driver allocate DMA_BIDIRECTIONAL buffer
>> to use it as a driver-internal bounce buffer.
>>
>> The hardware transfer page data into the bounce buffer,
>> then CPU copies from the bounce buffer to a given buf (and oob_poi).
>> This is not efficient.
>>
>> So, I want to set NAND_USE_BOUNCE_BUFFER flag
>> and do dma_map_single directly for a given buffer.
>
> Sounds good. Be careful though, when you use the generic bounce buffer
> interface you might have to clear the page cache info (->pagebuf = -1).

Instead of memcpy() of the whole page,
I am trying to use dma_map_single()  in ecc->read_page() / ecc->write_page().
This will allow direct transfer between the buffer and the device by DMA.

But, this does not work for Denali if use_bufpoi is set in nand_do_read_ops().

In the following code in nand_scan_tail(),

        if (!(chip->options & NAND_OWN_BUFFERS)) {
                nbuf = kzalloc(sizeof(*nbuf) + mtd->writesize
                                + mtd->oobsize * 3, GFP_KERNEL);
                if (!nbuf)
                        return -ENOMEM;
                nbuf->ecccalc = (uint8_t *)(nbuf + 1);
                nbuf->ecccode = nbuf->ecccalc + mtd->oobsize;
                nbuf->databuf = nbuf->ecccode + mtd->oobsize;

                chip->buffers = nbuf;

chip->buffers->databuf has no guarantee for DMA'able alignment.
(actually it has unwanted offset 0xc because sizeof(*nbuf) == 0xc on
32bit systems)

If we could change the code as follows,

                nbuf->ecccalc = kmalloc(mtd->oobsize, GFP_KERNEL);
                nbuf->ecccode = kmalloc(mtd->oobsize, GFP_KERNEL);
                nbuf->databuf = kmalloc(mtd->writesize + mtd->oobsize,
GFP_KERNEL);

chip->buffers->databuf would have DMA'able alignment in most cases
without NAND_OWN_BUFFERS.  (but, I am not sure if this is a good idea)

So, the idea of NAND_OWN_BUFFERS is that
drivers should allocate own buffers if they need to perform DMA-mapping
in read_page(), write_page(), right?

However, "git grep NAND_OWN_BUFFERS" shows
cafe_nand.c is the only driver that does so.

On the other hand, "git grep dma_map_single" has more hits,
i.e. some drivers perform dma_map_single() for read/write without
NAND_OWN_BUFFERS.

I have no idea how they are working.

-- 
Best Regards
Masahiro Yamada