linux-kernel - Re: [mm PATCH v2 03/26] swiotlb: Add support for DMA_ATTR_SKIP_CPU

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] [day] [month] [year] [list]

Message-ID: <CAKgT0UdjHb=49jSdzNoDaukZi6_jpqWziSDtM3Gy_6ZCC=5efQ@mail.gmail.com>
Date:   Sat, 5 Nov 2016 16:13:31 -0700
From:   Alexander Duyck <alexander.duyck@...il.com>
To:     Konrad Rzeszutek Wilk <konrad@...nok.org>
Cc:     Alexander Duyck <alexander.h.duyck@...el.com>,
        linux-mm <linux-mm@...ck.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Netdev <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
Subject: Re: [mm PATCH v2 03/26] swiotlb: Add support for DMA_ATTR_SKIP_CPU_SYNC

On Sat, Nov 5, 2016 at 12:39 PM, Konrad Rzeszutek Wilk
<konrad@...nok.org> wrote:
> .. snip..
>> @@ -561,6 +565,7 @@ void swiotlb_tbl_unmap_single(struct device *hwdev, phys_addr_t tlb_addr,
>>        * First, sync the memory before unmapping the entry
>>        */
>>       if (orig_addr != INVALID_PHYS_ADDR &&
>> +         !(attrs & DMA_ATTR_SKIP_CPU_SYNC) &&
>>           ((dir == DMA_FROM_DEVICE) || (dir == DMA_BIDIRECTIONAL)))
>>               swiotlb_bounce(orig_addr, tlb_addr, size, DMA_FROM_DEVICE);
>>
>> @@ -654,7 +659,8 @@ void swiotlb_tbl_sync_single(struct device *hwdev, phys_addr_t tlb_addr,
>>                * GFP_DMA memory; fall back on map_single(), which
>>                * will grab memory from the lowest available address range.
>>                */
>> -             phys_addr_t paddr = map_single(hwdev, 0, size, DMA_FROM_DEVICE);
>> +             phys_addr_t paddr = map_single(hwdev, 0, size,
>> +                                            DMA_FROM_DEVICE, 0);
>>               if (paddr == SWIOTLB_MAP_ERROR)
>>                       goto err_warn;
>>
>> @@ -669,7 +675,8 @@ void swiotlb_tbl_sync_single(struct device *hwdev, phys_addr_t tlb_addr,
>>
>>                       /* DMA_TO_DEVICE to avoid memcpy in unmap_single */
>>                       swiotlb_tbl_unmap_single(hwdev, paddr,
>> -                                              size, DMA_TO_DEVICE);
>> +                                              size, DMA_TO_DEVICE,
>> +                                              DMA_ATTR_SKIP_CPU_SYNC);
>
> This I believe is redundant. That is swiotlb_tbl_unmap_single only
> does an bounce if the dir is DMA_FROM_DEVICE or DMA_BIDIRECTIONAL.
>
> I added /* optional. */

You are probably right.  I don't need to add the DMA_ATTR_SKIP_CPU_SYNC here.

>>                       goto err_warn;
>>               }
>>       }
>> @@ -699,7 +706,7 @@ void swiotlb_tbl_sync_single(struct device *hwdev, phys_addr_t tlb_addr,
>>               free_pages((unsigned long)vaddr, get_order(size));
>>       else
>>               /* DMA_TO_DEVICE to avoid memcpy in swiotlb_tbl_unmap_single */
>> -             swiotlb_tbl_unmap_single(hwdev, paddr, size, DMA_TO_DEVICE);
>> +             swiotlb_tbl_unmap_single(hwdev, paddr, size, DMA_TO_DEVICE, 0);
>
> .. but here you choose to put 0? I changed that to
> DMA_ATTR_SKIP_CPU_SYNC and expanded the comment above.
>
> Time to test the patches.

I think I had probably realized the fact that I didn't need it above
and so just used 0 here.  I can clean this up and resubmit if you
want.

Do you want me to just split this patch set up so that I submit the
swiotlb patches to you and leave the rest of the patches for the mm
tree?

Thanks.

- Alex