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 8847AC02180 for ; Wed, 15 Jan 2025 08:17:35 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A28016B007B; Wed, 15 Jan 2025 03:17:34 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 9D85F6B0082; Wed, 15 Jan 2025 03:17:34 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 89FEE6B0083; Wed, 15 Jan 2025 03:17:34 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 6E9696B007B for ; Wed, 15 Jan 2025 03:17:34 -0500 (EST) Received: from smtpin15.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id DCFE4C10CF for ; Wed, 15 Jan 2025 08:17:33 +0000 (UTC) X-FDA: 83008981986.15.08087F4 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf25.hostedemail.com (Postfix) with ESMTP id 33F04A000D for ; Wed, 15 Jan 2025 08:17:32 +0000 (UTC) Authentication-Results: imf25.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=jtS4YogD; spf=pass (imf25.hostedemail.com: domain of leon@kernel.org designates 139.178.84.217 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=1736929052; 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=ESjo+RlRvVUNk+hmOGUo1t6i4d8a+/1VNeaK8yB+Zfc=; b=fICLL4BoC9iHhVC28hsI2615ca1xH4rEyns1vH2Rck9YffgtpiQ0FLuEQ3/qmghGhWhb3b Mt+1ZJC1KxCrK59PyoePdseEGLD3z0A2kNtF5/neQVQRxhiSDtUfd5JQHaNIE2SYjrvvEr wN7dbWsFKtdjlCpWKu3G4dKSsnFXgfU= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1736929052; a=rsa-sha256; cv=none; b=z5tMXD++Fiq6nK2cLFoCpOowzsxagRSD8NdRwUIlbagyA2mGjgMtDyRCeuStBiXGi6OA2U ZoHgMrJPhkM8OmntIthl3+RjdDOhKiHVJ/QGtMEZrOaiC1jQwi/1BKwHNLnEOKvavMCexW gAiayX+oIPNj6jjhHkzH1HGBdTVHZxQ= ARC-Authentication-Results: i=1; imf25.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=jtS4YogD; spf=pass (imf25.hostedemail.com: domain of leon@kernel.org designates 139.178.84.217 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 dfw.source.kernel.org (Postfix) with ESMTP id F3F2A5C5B34; Wed, 15 Jan 2025 08:16:49 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id DCD6DC4CEDF; Wed, 15 Jan 2025 08:17:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1736929050; bh=Yu7S/jLyUGhfFZjVFlk4eprtZYRxfla2Nw7nZEhy9no=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=jtS4YogDlGBJKPE4uxCOLjxH7XCi5QYsOo4leoD+xgmN8tj70qVx5829WIg+sShgX yiakEsBadvxORmzL2tNvh64PTbxN2Yn7ZjPr4SYb84VfV3NxFD7kGP/KdoZOcX+tov rQJGthlfV+pWswtBJZw2US2akwQ3E9tQt/jwG/ICtUFlz/2ySVXe68KJv7W8xdw1OU pIcG2l3K1LSF8572QwfNaFSMofL4ExapBuk12HqOU0whbiq9SaXUul7IQT4rV47kiH Y6/VezMg1RWwWJ+XN6OfK0HDAZ0irI3FF3+Tw83vfWul8FBVAnUy0tZrZ3zZK1Ybu0 cyRrXOGYV/nDQ== Date: Wed, 15 Jan 2025 10:17:26 +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 05/17] dma-mapping: Provide an interface to allow allocate IOVA Message-ID: <20250115081726.GK3146852@unreal> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: 33F04A000D X-Stat-Signature: kgcdf6roxa5rwi7pafbp61wziguskh9f X-Rspam-User: X-HE-Tag: 1736929052-980244 X-HE-Meta: U2FsdGVkX1/JPC7AA/nZnCxQ+Iefuch/LsoPh2hDEyLrc9CJBKG2IaCnOQtP8ItUoxEUXmglfL90MvXXqGRizppZQ8G0aO+AULnt21cKkhjZteHFVpkEyvVIJ6erJc9dVxrJ4aV0ZiaWvboaLrB3rbcNLu966a4AD9Fos3AWjtcPqUsf8XN5bURFZ/cfa9uEH1pRKeg+8/UYIkylXu3ofW1jx2Hb5iqrgFnKVfafolSYFwvCNH4wEF/1G9GLNjDVlKGiWtNAcfzVOjR+p9S1vLCX4q2PXifKaT+jII79V1pVK80bruLSy9XKs0iB194ZuUqxZ0qFyT4T/DW9vhTDnVkzd6IYP1NJhqaIQUJCsrpgnZJkDoyzWgODOki/3uFHQIEFi/gPTz4eCS/vUZBc9Pq0RB6UsIAOLLcrHN1jpBCBqieKQTDH+EIxsoRiPCb0JPPjzwTLamN6rSAUC2cJAkJo289gQat9JOW29Scht0AibIHjWbhbFqBlSzNa78wO7a1xbREdKk0fE0vo6g4v9Sr2kG5cfTZUyuC8BOBxix79qzxgcY8ah2uTvpzGKuwnKHubcdx+jCqExZM/u4LPA1H5gmkDne3UZSTwWGbUk8P6X0D0sGWQrdvkFJWROFP+d409/MGT6OQpcQ3Knobsj48jFMlkgWMKtgH3aa8gYSK/9X9f/alKzjaWCR+UXyo3n5HWgC0NitxFvINGXAFwjTBeBKX3kXoClozubjuJvrlWOLpVpyKXDhCQ+00qpzMLeejM2fv7iqvz4Uz1O+fLIgp+wc7FuHZG5iu36NBnJiggYUxS+UMd2FlVZ45em6bpdlcUAEbSbkVsoeSOrPSF90693mUnOQSoF5sbfMKL6IVvJdj4+xDsM5Dc9PSTVGyWmsqqo8htNfkXiDyt7/IRFEK2pzHynQvDeuGmTNmKkeFjZ2iJ4zeK2I0AV8oljOoaDWKCKM+ldyNvnOE+lJI a0lMlKmL Pc7K7Fr9In6j5FAI01+pOuRauqblXh8OklY1cYg5NMB1Z0u2aplFAYh7lZUsPWH+FQtLMYPZYURYRTqQk1bbfe0L/tgsWNLlVjVP/STaNqdxVqZeYrZXWsyw+qePkjhOfJE32XJOv9npi85McKIiztYX5o9It00xis3IYErKrgWVhjaUydscgn2fnP6AGAm/BP14yTUEqlDq7o+0rwv3hVgFhC+NN9jHy+I50s6wVOBGgz47nHSwF5A+sFM+Ws213fCYANNmUVYcLtvTbCJP9nhxDEQ== 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:28PM +0000, Robin Murphy wrote: > On 17/12/2024 1:00 pm, Leon Romanovsky wrote: > > From: Leon Romanovsky > > > > The existing .map_page() callback provides both allocating of IOVA > > and linking DMA pages. That combination works great for most of the > > callers who use it in control paths, but is less effective in fast > > paths where there may be multiple calls to map_page(). > > > > These advanced callers already manage their data in some sort of > > database and can perform IOVA allocation in advance, leaving range > > linkage operation to be in fast path. > > > > Provide an interface to allocate/deallocate IOVA and next patch > > link/unlink DMA ranges to that specific IOVA. > > > > In the new API a DMA mapping transaction is identified by a > > struct dma_iova_state, which holds some recomputed information > > for the transaction which does not change for each page being > > mapped, so add a check if IOVA can be used for the specific > > transaction. > > > > The API is exported from dma-iommu as it is the only implementation > > supported, the namespace is clearly different from iommu_* functions > > which are not allowed to be used. This code layout allows us to save > > function call per API call used in datapath as well as a lot of boilerplate > > code. > > > > Reviewed-by: Christoph Hellwig > > Signed-off-by: Leon Romanovsky > > --- > > drivers/iommu/dma-iommu.c | 74 +++++++++++++++++++++++++++++++++++++ > > include/linux/dma-mapping.h | 49 ++++++++++++++++++++++++ > > 2 files changed, 123 insertions(+) > > > > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c > > index 853247c42f7d..5906b47a300c 100644 > > --- a/drivers/iommu/dma-iommu.c > > +++ b/drivers/iommu/dma-iommu.c > > @@ -1746,6 +1746,80 @@ size_t iommu_dma_max_mapping_size(struct device *dev) > > return SIZE_MAX; > > } > > +/** > > + * dma_iova_try_alloc - Try to allocate an IOVA space > > + * @dev: Device to allocate the IOVA space for > > + * @state: IOVA state > > + * @phys: physical address > > + * @size: IOVA size > > + * > > + * Check if @dev supports the IOVA-based DMA API, and if yes allocate IOVA space > > + * for the given base address and size. > > + * > > + * Note: @phys is only used to calculate the IOVA alignment. Callers that always > > + * do PAGE_SIZE aligned transfers can safely pass 0 here. > > + * > > + * Returns %true if the IOVA-based DMA API can be used and IOVA space has been > > + * allocated, or %false if the regular DMA API should be used. > > + */ > > +bool dma_iova_try_alloc(struct device *dev, struct dma_iova_state *state, > > + phys_addr_t phys, size_t size) > > +{ > > + 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_off = iova_offset(iovad, phys); > > + dma_addr_t addr; > > + > > + memset(state, 0, sizeof(*state)); > > + if (!use_dma_iommu(dev)) > > + return false; > > Can you guess why that return won't ever be taken? I will move references to pointers after this check, but like Christoph said, this "if ..." is taken a lot and we didn't see any issues with inbox GCC versions. > > > + if (static_branch_unlikely(&iommu_deferred_attach_enabled) && > > + iommu_deferred_attach(dev, iommu_get_domain_for_dev(dev))) > > + return false; > > + > > + if (WARN_ON_ONCE(!size)) > > + return false; > > + if (WARN_ON_ONCE(size & DMA_IOVA_USE_SWIOTLB)) > > This looks weird. Why would a caller ever set an effectively-private flag in > the first place? If it's actually supposed to be a maximum size check, > please make it look like a maximum size check. It is in-kernel API and the idea is to warn developers of something that is not right and not perform defensive programming by doing size checks. <...> > > +/** > > + * dma_iova_free - Free an IOVA space > > + * @dev: Device to free the IOVA space for > > + * @state: IOVA state > > + * > > + * Undoes a successful dma_try_iova_alloc(). > > + * > > + * Note that all dma_iova_link() calls need to be undone first. For callers > > + * that never call dma_iova_unlink(), dma_iova_destroy() can be used instead > > + * which unlinks all ranges and frees the IOVA space in a single efficient > > + * operation. > > That's only true if they *also* call dma_iova_link() in just the right way > too. We can update the comment section to any wording, feel free to propose. > > > + */ > > +void dma_iova_free(struct device *dev, struct dma_iova_state *state) > > +{ > > + 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, state->addr); > > + size_t size = dma_iova_size(state); > > + > > + iommu_dma_free_iova(cookie, state->addr - iova_start_pad, > > + iova_align(iovad, size + iova_start_pad), NULL); > > +} > > +EXPORT_SYMBOL_GPL(dma_iova_free); > > + > > void iommu_setup_dma_ops(struct device *dev) > > { > > struct iommu_domain *domain = iommu_get_domain_for_dev(dev); > > diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h > > index b79925b1c433..55899d65668b 100644 > > --- a/include/linux/dma-mapping.h > > +++ b/include/linux/dma-mapping.h > > @@ -7,6 +7,8 @@ > > #include > > #include > > #include > > +#include > > +#include > > Why are these being pulled in here? It is rebase leftover. > > > /** > > * List of possible attributes associated with a DMA mapping. The semantics > > @@ -72,6 +74,21 @@ > > #define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1)) > > +struct dma_iova_state { > > + dma_addr_t addr; > > + size_t __size; > > +}; > > + > > +/* > > + * Use the high bit to mark if we used swiotlb for one or more ranges. > > + */ > > +#define DMA_IOVA_USE_SWIOTLB (1ULL << 63) > > This will give surprising results for 32-bit size_t (in fact I guess it > might fire some build warnings already). We got none, I will change to u64. > > Thanks, > Robin. Thanks