linux-kernel - Re: [PATCH v5] drm/i915: stop using 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]

Date:   Mon, 8 Aug 2022 15:48:02 +0000
From:   "Hellstrom, Thomas" <thomas.hellstrom@...el.com>
To:     "daniel@...ll.ch" <daniel@...ll.ch>,
        "joonas.lahtinen@...ux.intel.com" <joonas.lahtinen@...ux.intel.com>,
        "Vivi, Rodrigo" <rodrigo.vivi@...el.com>,
        "bob.beckett@...labora.com" <bob.beckett@...labora.com>,
        "tvrtko.ursulin@...ux.intel.com" <tvrtko.ursulin@...ux.intel.com>,
        "jani.nikula@...ux.intel.com" <jani.nikula@...ux.intel.com>,
        "airlied@...ux.ie" <airlied@...ux.ie>
CC:     "hch@....de" <hch@....de>,
        "Ursulin, Tvrtko" <tvrtko.ursulin@...el.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
        "kernel@...labora.com" <kernel@...labora.com>,
        "Auld, Matthew" <matthew.auld@...el.com>,
        "intel-gfx@...ts.freedesktop.org" <intel-gfx@...ts.freedesktop.org>
Subject: Re: [PATCH v5] drm/i915: stop using swiotlb

Hi, [back from vacation]

On Tue, 2022-07-26 at 16:39 +0100, Robert Beckett wrote:
> Calling swiotlb functions directly is nowadays considered harmful.
> See
> https://lore.kernel.org/intel-gfx/20220711082614.GA29487@lst.de/
> 
> Replace swiotlb_max_segment() calls with dma_max_mapping_size().
> In i915_gem_object_get_pages_internal() no longer consider
> max_segment
> only if CONFIG_SWIOTLB is enabled. There can be other (iommu related)
> causes of specific max segment sizes.
> 
> Cc: Christoph Hellwig <hch@....de>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@...ux.intel.com>
> Cc: Thomas Hellstrom <thomas.hellstrom@...el.com>
> Cc: Matthew Auld <matthew.auld@...el.com>
> 
> v2: - restore UINT_MAX clamp in i915_sg_segment_size()
>     - drop PAGE_SIZE check as it will always be >= PAGE_SIZE
> v3: - actually clamp to UINT_MAX in i915_sg_segment_size()
> v4: - round down max segment size to PAGE_SIZE
> v5: - fix checkpatch whitespace issue
> 
> Reviewed-by: Christoph Hellwig <hch@....de>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@...el.com>
> Signed-off-by: Robert Beckett <bob.beckett@...labora.com>

Hmm,

This whole thing looks a bit strange to me since with SWIOTLB actually
used for i915, the driver should malfunction anyway as it doesn't do
any dma_sync_sg_for_cpu() or dma_sync_sg_for_device(), and the driver
assumes all coherent dma. Is that SWIOTLB=force kernel option still
available?

Also, correct me if I'm wrong, but the original driver segment size
appears to mean "the largest contiguous area that can be handled either
by the device or the dma mapping layer" rather than the total space
available for dma mappings? Not completely sure what
dma_max_mapping_size() is returning, though?

/Thomas