linux-kernel - Re: [PATCH] [v2] ARM: sa1100/assabet: move dmabounce hack to ohci driver

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:   Tue, 8 Feb 2022 13:49:21 +0100
From:   Arnd Bergmann <arnd@...nel.org>
To:     Ard Biesheuvel <ardb@...nel.org>
Cc:     Russell King <linux@...linux.org.uk>,
        Arnd Bergmann <arnd@...db.de>,
        Linus Walleij <linus.walleij@...aro.org>,
        Christoph Hellwig <hch@...radead.org>,
        Laurentiu Tudor <laurentiu.tudor@....com>,
        USB list <linux-usb@...r.kernel.org>,
        Alan Stern <stern@...land.harvard.edu>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] [v2] ARM: sa1100/assabet: move dmabounce hack to ohci driver

On Thu, Feb 3, 2022 at 9:47 AM Ard Biesheuvel <ardb@...nel.org> wrote:
> On Thu, 3 Feb 2022 at 09:38, Arnd Bergmann <arnd@...nel.org> wrote:
>
> > There are two main downsides:
> >
> >  - rather than using a dynamically sized pool, this buffer needs
> >    to be allocated at probe time using a fixed size. Without
> >    having any idea of what it should be, I picked a size of
> >    64KB, which is between what the other two OHCI front-ends use
> >    in their SRAM. If anyone has a better idea what that size
> >    is reasonable, this can be trivially changed.
> >
>
> I suppose this is a problem if the driver falls back to ordinary DRAM
> once the allocation runs out?

>From what I can tell, there is no such fallback. If the localmem_pool
runs out, the allocation fails, which may cause other problems, but
it never falls back to the wrong DMA address.

> >  - Previously, only USB transfers to unaddressable memory needed
> >    to go through the bounce buffer, now all of them do, which may
> >    impact runtime performance for USB endpoints that do a lot of
> >    transfers.
> >
> > On the upside, the local_mem support uses write-combining buffers,
> > which should be a bit faster for transfers to the device compared to
> > normal uncached coherent memory as used in dmabounce.
> >
>
> Talking from past experience using this trick on a NXP ARM9 SoC ~10
> years ago, using on-chip SRAM for USB DMA likely results in a
> significant performance boost, even without write combining, although
> the exact scenario obviously matters.

Right, that makes sense, but it won't help here because there is
no SRAM. One detail  I noticed is that the localmem pool normally
gets mapped as WC, which is what I did in the new code as well, but
dma_alloc_flags(..., DMA_ATTR_WRITE_COMBINE) does not always
honor this flag. I think it will do it here because a GFP_KERNEL
allocation should be served by the remap_allocator, while
GFP_ATOMIC allocations would be served by pool_allocator_alloc(),
which ignores the flag.

        Arnd