linux-kernel - Re: [PATCH] dma-direct: avoid redundant memory sync for swiotlb

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]

Message-ID: <20220413045958.GA31209@lst.de>
Date:   Wed, 13 Apr 2022 06:59:58 +0200
From:   Christoph Hellwig <hch@....de>
To:     Chao Gao <chao.gao@...el.com>
Cc:     Robin Murphy <robin.murphy@....com>, linux-kernel@...r.kernel.org,
        iommu@...ts.linux-foundation.org, m.szyprowski@...sung.com,
        hch@....de, Wang Zhaoyang1 <zhaoyang1.wang@...el.com>,
        Gao Liang <liang.gao@...el.com>,
        Kevin Tian <kevin.tian@...el.com>
Subject: Re: [PATCH] dma-direct: avoid redundant memory sync for swiotlb

On Wed, Apr 13, 2022 at 09:02:02AM +0800, Chao Gao wrote:
> dma_direct_sync_single_for_cpu() also calls arch_sync_dma_for_cpu_all()
> and arch_dma_mark_clean() in some cases. if SWIOTLB does sync internally,
> should these two functions be called by SWIOTLB?
> 
> Personally, it might be better if swiotlb can just focus on bounce buffer
> alloc/free. Adding more DMA coherence logic into swiotlb will make it
> a little complicated.
> 
> How about an open-coded version of dma_direct_sync_single_for_cpu
> in dma_direct_unmap_page with swiotlb_sync_single_for_cpu replaced by
> swiotlb_tbl_unmap_single?

I don't think the swiotlb and non-coherent case ever fully worked.
Before the merge of swiotlb into dma-direct they obviously were
mutally exclusive, and even now all the cache maintainance is done
on the physical address of the original data, not the swiotlb buffer.

If we want to fix that properly all the arch dma calls will need to
move into swiotlb, but that is a much bigger patch.

So for now I'd be happy with the one liner presented here, but
eventually the whole area could use an overhaul.