linux-kernel - Re: [PATCH] iommu/dma: Add support for DMA_ATTR_FORCE

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, 13 Jan 2017 12:17:24 +0000
From:   Robin Murphy <robin.murphy@....com>
To:     Geert Uytterhoeven <geert@...ux-m68k.org>
Cc:     Geert Uytterhoeven <geert+renesas@...der.be>,
        Joerg Roedel <joro@...tes.org>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will.deacon@....com>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        Magnus Damm <magnus.damm@...il.com>,
        iommu@...ts.linux-foundation.org,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        Linux-Renesas <linux-renesas-soc@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] iommu/dma: Add support for DMA_ATTR_FORCE_CONTIGUOUS

On 13/01/17 11:59, Geert Uytterhoeven wrote:
> Hi Robin,
> 
> On Fri, Jan 13, 2017 at 12:32 PM, Robin Murphy <robin.murphy@....com> wrote:
>> On 13/01/17 11:07, Geert Uytterhoeven wrote:
>>> Add support for DMA_ATTR_FORCE_CONTIGUOUS to the generic IOMMU DMA code.
>>> This allows to allocate physically contiguous DMA buffers on arm64
>>> systems with an IOMMU.
>>
>> Can anyone explain what this attribute is actually used for? I've never
>> quite figured it out.
> 
> My understanding is that DMA_ATTR_FORCE_CONTIGUOUS is needed when using
> an IOMMU but wanting the buffers to be both contiguous in IOVA space and
> physically contiguous to allow passing to devices without IOMMU.
> 
> Main users are graphic and remote processors.

Sure, I assumed it must be to do with buffer sharing, but the systems
I'm aware of which have IOMMUs in their media subsystems tend to have
them in front of every IP block involved, so I was curious as to what
bit of non-IOMMU hardware wanted to play too. The lone in-tree use in
the Exynos DRM driver was never very revealing, and the new one I see in
the Qualcomm PIL driver frankly looks redundant to me.

Robin.

>>> --- a/drivers/iommu/dma-iommu.c
>>> +++ b/drivers/iommu/dma-iommu.c
> 
>>> @@ -265,6 +272,20 @@ static struct page **__iommu_dma_alloc_pages(unsigned int count,
>>>       /* IOMMU can map any pages, so himem can also be used here */
>>>       gfp |= __GFP_NOWARN | __GFP_HIGHMEM;
>>>
>>> +     if (attrs & DMA_ATTR_FORCE_CONTIGUOUS) {
>>> +             int order = get_order(count << PAGE_SHIFT);
>>> +             struct page *page;
>>> +
>>> +             page = dma_alloc_from_contiguous(dev, count, order);
>>> +             if (!page)
>>> +                     return NULL;
>>> +
>>> +             while (count--)
>>> +                     pages[i++] = page++;
>>> +
>>> +             return pages;
>>> +     }
>>> +
>>
>> This is really yuck. Plus it's entirely pointless to go through the
>> whole page array/scatterlist dance when we know the buffer is going to
>> be physically contiguous - it should just be allocate, map, done. I'd
>> much rather see standalone iommu_dma_{alloc,free}_contiguous()
>> functions, and let the arch code handle dispatching appropriately.
> 
> Fair enough.
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
>