From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 04D97C02183 for ; Wed, 15 Jan 2025 08:33:52 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 969E86B0083; Wed, 15 Jan 2025 03:33:51 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 8F340280001; Wed, 15 Jan 2025 03:33:51 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 76CB86B0088; Wed, 15 Jan 2025 03:33:51 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 552C86B0083 for ; Wed, 15 Jan 2025 03:33:51 -0500 (EST) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id C8E724585E for ; Wed, 15 Jan 2025 08:33:50 +0000 (UTC) X-FDA: 83009023020.24.168E06F Received: from nyc.source.kernel.org (nyc.source.kernel.org [147.75.193.91]) by imf23.hostedemail.com (Postfix) with ESMTP id 3A9F914000D for ; Wed, 15 Jan 2025 08:33:49 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=mCWDVkUq; spf=pass (imf23.hostedemail.com: domain of leon@kernel.org designates 147.75.193.91 as permitted sender) smtp.mailfrom=leon@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1736930029; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=ZXFFMzIzsul109ol84Bm1FmobeYD7P8yzT28CIUFEC0=; b=PpF+T6IZPMsM37C4rzqGr28ekYsqe9hbvr02zkHf3u9IEOjVb2Lt1YD465YCGqUuP/oWTg 3FVxL8RxXtimn8urvOR85tmx6PO4FKrHpV1VKKGMm3t2Kv2oE1EhwFT4ktpO7CMIFZCUq1 fe58Ebz+yitKP9T796dJV1qCdBMe1zY= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1736930029; a=rsa-sha256; cv=none; b=69b6/QD4HGTJDgFKZMuKA5UWKKbyVkgPvP8Mx2xjiBjYXNUTw6bYxN/wiqCWcNsSBetTUQ FTQID+IqGkX6keSq1xPCMaSW/eqLj9tM2IQZyeIeO9+YxdW/Mm4QkNghgmlOY9RBVd5BpQ 4LSejul4IaXCx5j35LeG48K+wVez+Ng= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=mCWDVkUq; spf=pass (imf23.hostedemail.com: domain of leon@kernel.org designates 147.75.193.91 as permitted sender) smtp.mailfrom=leon@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by nyc.source.kernel.org (Postfix) with ESMTP id 5B4C6A416DE; Wed, 15 Jan 2025 08:32:00 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id E083DC4CEE2; Wed, 15 Jan 2025 08:33:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1736930028; bh=q//4PX+/kKs0NcRsMaK0IRFD8IgnEeYPZkHGethy6Xc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=mCWDVkUqHQRGTYemu0OT2ih+ESCVIZ/aTGA9LOFnN2apzUXuQ6js3wGRvMvmPD0Q4 qPuVV2jFdCTf9DIs+hAcwNkgetPw+3PG1Lpnlw/6yRfq11aaFbmf0jL/DFWnEBrlVw Ua5VHXN/Wk/lZkcpIs8HKqmwrhDVRgZOiJTe7WsPa5iJP6YN5+bhjqq5Jd10hVAZze QrEQ79dFiDRigierP81hoV+aRHQMIswxIRHmBXxqXzkaq8aaF3N2iCj59qFjsa3k/F 66JPQhMe1Azzt9/n3bF5LnqLwPghqkXaMemOLirAJWtGi32tua3HSrRA0qNoLRzLR7 YnubzClMeJP7Q== Date: Wed, 15 Jan 2025 10:33:40 +0200 From: Leon Romanovsky To: Robin Murphy Cc: Jens Axboe , Jason Gunthorpe , Joerg Roedel , Will Deacon , Christoph Hellwig , Sagi Grimberg , Keith Busch , Bjorn Helgaas , Logan Gunthorpe , Yishai Hadas , Shameer Kolothum , Kevin Tian , Alex Williamson , Marek Szyprowski , =?iso-8859-1?B?Suly9G1l?= Glisse , Andrew Morton , Jonathan Corbet , linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-block@vger.kernel.org, linux-rdma@vger.kernel.org, iommu@lists.linux.dev, linux-nvme@lists.infradead.org, linux-pci@vger.kernel.org, kvm@vger.kernel.org, linux-mm@kvack.org, Randy Dunlap Subject: Re: [PATCH v5 07/17] dma-mapping: Implement link/unlink ranges API Message-ID: <20250115083340.GL3146852@unreal> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 3A9F914000D X-Stat-Signature: b7n7js4rjmeacb3aycyngp5kdwcmjnfu X-Rspam-User: X-HE-Tag: 1736930029-591872 X-HE-Meta: U2FsdGVkX19/2j9+keirGNMFXcpcK+a7FlL4ZTTRajsNJ76I0YcFv1X2ELoFaOI9dAaMybYpKHAXtKK/MFfP/K3Gq9zX/G0N7uyZT+LeCunZMU+5Mv44zNnJOKVONpTfGVZPnXvU1xqiOz4KC5LnYo6IcewL6YRQHajgv4BCgFWW7yOTL2dy+FbCrfTjH9HcR9ER45H7zVZAaPeqjWOoAvI8OudtiGgg1NgLDkpwmqQfhdpqfuzJG+H+xUI8LJvaqtfk7KRSaQ/8LCEqMnfXtGTCWtBoMnL0abfawdF66Om2D6MuKhVPfuP2pRw0yNbHCyhZXmtMbNH4ETLJDPrahQ6TNItbolLRtd6bCaVm3rnFTBuXwJb//FRzfBEHJmc8A9mjbhMqj7lbcOC5ne97yRP4RyBgv2XAlntNtjDcyLuh6nO0sElR9reg8963aHWgJdLd6xDwoEHV0LSe8fDGkGTlaiD11v11rCalGVnXdUfckUwj+UkRhCxxqIeB9OlPsKiG0PtUIapBdib1J4L2NJCZUHl+AFKSO7rRPJRiGfnsHZMkiaNpP2AjDKkPooqnVQxtVXrqDKacSMffFCF6tuPsbl8ad/osjQRC3ItUgKGMP16GvzKDMt1cUhu180r37T64uaiV6c2Sggip0Z4rcH6/JHGwglqJjjZdLCYBQJypKBWodxJd/LpUXXY5kwxJSB+W7PYb5sfZlqTHBz7AMkItDlX3V0l3PqJ4uYYv5qJDU17peT4ZE8kTYUHSl04L5LdIbMWYstCKqWOUsZ1cLvF2ZKeUdGixAZsrsxTvQ9vAzEEiYMFfybxOPdrpf6wk4LMqBUvoAPSmpAGFSaqyOjylXELBYv4LhUDlhWsHNvozxWreQOzqYg8mRGQgdIyqqcdWAihoTfqyXTP5+HHlAZBis4cGNUBPj88GlQk+aAeD5rhkCQsRwJkIAN7EBbOjZsHp24ii8sXugS1nWo5 eRmtvLS8 tDH80Q670jRKd9q/MhqRTOnUmP3SQDJyQ10ow/R59huZDFxZJCz6PfeB643mLnAbybRsZuTlTM/9hKDk7X33/6ONTVAnI7tKTbyuBzbmThlzKubO+3MTPqlnuKZ1w5SUEDe5RG0O0q8W2de5WDh8UNEKu8ARmUtbRWk5FRBpuC5uuh9FLBnOKpFdYAEWganIgqYwCJX0MWrAZ14kcr6RlrmAs0hai2PXPQoDGDhP+7fXS/vik8Q8D+tdy9ESqNoDXALKL/BHzR+I/gJ1p1ud+NSDz5A== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Tue, Jan 14, 2025 at 08:50:35PM +0000, Robin Murphy wrote: > On 17/12/2024 1:00 pm, Leon Romanovsky wrote: > > From: Leon Romanovsky > > > > Introduce new DMA APIs to perform DMA linkage of buffers > > in layers higher than DMA. > > > > In proposed API, the callers will perform the following steps. > > In map path: > > if (dma_can_use_iova(...)) > > dma_iova_alloc() > > for (page in range) > > dma_iova_link_next(...) > > dma_iova_sync(...) > > else > > /* Fallback to legacy map pages */ > > for (all pages) > > dma_map_page(...) > > > > In unmap path: > > if (dma_can_use_iova(...)) > > dma_iova_destroy() > > else > > for (all pages) > > dma_unmap_page(...) > > > > Reviewed-by: Christoph Hellwig > > Signed-off-by: Leon Romanovsky > > --- > > drivers/iommu/dma-iommu.c | 259 ++++++++++++++++++++++++++++++++++++ > > include/linux/dma-mapping.h | 32 +++++ > > 2 files changed, 291 insertions(+) <...> > > +static void iommu_dma_iova_unlink_range_slow(struct device *dev, > > + dma_addr_t addr, size_t size, enum dma_data_direction dir, > > + unsigned long attrs) > > +{ > > + struct iommu_domain *domain = iommu_get_dma_domain(dev); > > + struct iommu_dma_cookie *cookie = domain->iova_cookie; > > + struct iova_domain *iovad = &cookie->iovad; > > + size_t iova_start_pad = iova_offset(iovad, addr); > > + dma_addr_t end = addr + size; > > + > > + do { > > + phys_addr_t phys; > > + size_t len; > > + > > + phys = iommu_iova_to_phys(domain, addr); > > + if (WARN_ON(!phys)) > > + continue; > > Infinite WARN_ON loop, nice. No problem, will change it to WARN_ON_ONCE. > > > + len = min_t(size_t, > > + end - addr, iovad->granule - iova_start_pad); <...> > > + > > + swiotlb_tbl_unmap_single(dev, phys, len, dir, attrs); > > This is still dumb. For everything other than the first and last granule, > either it's definitely not in SWIOTLB, or it is (per the unaligned size > thing above) but then "len" is definitely wrong and SWIOTLB will complain. Like Christoph said, we tested it with NVMe which uses SWIOTLB path and despite having a lot of unaligned sizes, it worked without SWIOTLB complains. > > > + > > + addr += len; > > + iova_start_pad = 0; > > + } while (addr < end); > > +} > > + > > +static void __iommu_dma_iova_unlink(struct device *dev, > > + struct dma_iova_state *state, size_t offset, size_t size, > > + enum dma_data_direction dir, unsigned long attrs, > > + bool free_iova) > > +{ > > + struct iommu_domain *domain = iommu_get_dma_domain(dev); > > + struct iommu_dma_cookie *cookie = domain->iova_cookie; > > + struct iova_domain *iovad = &cookie->iovad; > > + dma_addr_t addr = state->addr + offset; > > + size_t iova_start_pad = iova_offset(iovad, addr); > > + struct iommu_iotlb_gather iotlb_gather; > > + size_t unmapped; > > + > > + if ((state->__size & DMA_IOVA_USE_SWIOTLB) || > > + (!dev_is_dma_coherent(dev) && !(attrs & DMA_ATTR_SKIP_CPU_SYNC))) > > + iommu_dma_iova_unlink_range_slow(dev, addr, size, dir, attrs); > > + > > + iommu_iotlb_gather_init(&iotlb_gather); > > + iotlb_gather.queued = free_iova && READ_ONCE(cookie->fq_domain); > > This makes things needlessly hard to follow, just keep the IOVA freeing > separate. And by that I really mean just have unlink and free, since > dma_iova_destroy() really doesn't seem worth the extra complexity to save > one line in one caller... In initial versions, I didn't implement dma_iova_destroy() and used unlink->free calls directly. Both Jason and Christoph asked me to provide dma_iova_destroy(), so we can reuse same iotlb_gather. Almost all callers (except HMM-like) will use this API call. Let's keep it. Thanks