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 054A7E77188 for ; Tue, 14 Jan 2025 20:50:38 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 40027280003; Tue, 14 Jan 2025 15:50:37 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 3AFF1280002; Tue, 14 Jan 2025 15:50:37 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 27791280003; Tue, 14 Jan 2025 15:50:37 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 09032280002 for ; Tue, 14 Jan 2025 15:50:37 -0500 (EST) Received: from smtpin16.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id AAD0A1C7D4E for ; Tue, 14 Jan 2025 20:50:36 +0000 (UTC) X-FDA: 83007250872.16.66C70B7 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf19.hostedemail.com (Postfix) with ESMTP id 711371A0011 for ; Tue, 14 Jan 2025 20:50:34 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf19.hostedemail.com: domain of robin.murphy@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=robin.murphy@arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1736887835; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=aWoJ1VeHd27DJ6o+jQDOT7KCZ+x9aBB3LNPmlNINcVo=; b=AUkDs0cZTF+nAdAoeMpu1QS59BwnAXwVT6rwIYAq4o0A9CaxL3+gaWHst1vYCz5tyMiTz+ glCWAQ6tSvTc2Umz+Smr/4/3Flv/neRxtBlBMoAQrZgpb3wdp93ibbmYCbNzvzRzitVtJY BIt7C89sKSsLTbf/dqxWEr3y0Zi6978= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf19.hostedemail.com: domain of robin.murphy@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=robin.murphy@arm.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1736887835; a=rsa-sha256; cv=none; b=KkSwXcYkKWsu7vmfdoBZWOaYGPY31COsNyay72FRlObbyr7rOHEIq3PMbGdVTfafoqWTGE 0hEIlRyku7OtddOIkp8U33MkCLTnBukAdV3Ny1IT25M3gu4RWcNRP3IsccjpY5MXOAB3ko lHwPWWr4SONjIYglQXhzd8XDg7rZqN4= Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id CEEE512FC; Tue, 14 Jan 2025 12:51:01 -0800 (PST) Received: from [10.1.196.40] (e121345-lin.cambridge.arm.com [10.1.196.40]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 3E7B93F673; Tue, 14 Jan 2025 12:50:30 -0800 (PST) Message-ID: Date: Tue, 14 Jan 2025 20:50:28 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v5 05/17] dma-mapping: Provide an interface to allow allocate IOVA To: Leon Romanovsky , Jens Axboe , Jason Gunthorpe , Joerg Roedel , Will Deacon , Christoph Hellwig , Sagi Grimberg Cc: Leon Romanovsky , Keith Busch , Bjorn Helgaas , Logan Gunthorpe , Yishai Hadas , Shameer Kolothum , Kevin Tian , Alex Williamson , Marek Szyprowski , =?UTF-8?B?SsOpcsO0bWUgR2xpc3Nl?= , 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 References: From: Robin Murphy Content-Language: en-GB In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: 711371A0011 X-Stat-Signature: biaz7j9n9f7nygqau75e76a8oiceuxbt X-Rspam-User: X-Rspamd-Server: rspam11 X-HE-Tag: 1736887834-774238 X-HE-Meta: U2FsdGVkX1/USFCtA/PGdYTzKBWIrnWBhJEFjTHO1gYnXBrv8aag5c2vYbfIv4qfp2Pvb/HETlGKKAK2aQ12Zbu4ggNVcXsKBbFjZsx18rPClfqk9yDy8LJtTazf8MGbzBhDTHiJDlEVqn39f8lN3hbhpNkHbHREYvoe5DcbeT/WhFKiANzAWoYNYGFGmkHTRUFo+soQmJZI4cInM8rKkemIKYR9eqT1N61NvcANREuHv8sygsQ4qrxhRorycESlxH1QLOYiJLguG26sBJI9+rFIbNWvBsNGdBbG+e/Ni2hLCzfd7Wb7VrEwzSYie6XwN4MOQSpNJFWt4EX+e7BXCd9ZvsuYVb6j1KrFP8qsaj1f61RnptivcNXn57KjsR7ZHC93WyIgSXUWTkph/eb14cZBBCR5s4y638wC9nm2FDbc4f8G2IVvz+FsruBhRyW9qE8joaznlgX3dTu8YMozmpR34B62FZTmyMfugIyLaXIWxjvl+bZ0RnCKUR7GIhSw0aQdJygUDfqa1UhfrWld2fw5Fu1ruTzNeaSuhEG1BcG5KgXiy0sWKNKxOQy+eAfwLFiQqLWQHsL0CcqdVJXhtdwlnCsEAlf3odJD4QmuJnp/S/OuZDd9WnE9c+wX7tb8tYqJPm133ozRBkVUWrZMrHqh5Kn+wcR+wUFB/ydcDjlamL+gajpCK7EYskSS2p7GQwPe5KLGetAyuMzExVTTdn6/1nCRJ9WstWg2vItgz/uLIHXHqe3yqWb/VmLnglXiqj9jyep98WpJXPLV6yZtKf3ms0pGgnz5S9pEA87msS8hxbZV+IJqryFU+b5fEYJ22lK2ymHiCJ5RS1E1P0LII584FNqX6kCxlwxg7lEZ1tYHdGAIUkG9ENdt2CyWNuBGzzZlhq1HXFRc+d7g7LtIJ9XHLbkqMxhbqTlEpRp0n6UZR67yVk+MrrfqtRCrREGxxQZDFKcij8OgXc7l/5x iU8lAgY3 5r1j9rpSiun7iaWquauJIjl0aWLGE5H9AMlNkcCzIO6jnNSsV+vM6p0CsTyW5dez1pPYWdC6I6FwJkeW1ux6s8Fcrxi9Mjs5IUHxyfO0VhXk5p44xjZ7fcgKDwfA7Z7R0oXoaFtTrAeIX/fbPvVH6N4qzbh4dGpFoXneMPQ6wAxL1HeA= 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 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? > + 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. (Which also makes me consider iommu_dma_max_mapping_size() returning SIZE_MAX isn't strictly accurate, ho hum...) > + return false; > + > + addr = iommu_dma_alloc_iova(domain, > + iova_align(iovad, size + iova_off), > + dma_get_mask(dev), dev); > + if (!addr) > + return false; > + > + state->addr = addr + iova_off; > + state->__size = size; > + return true; > +} > +EXPORT_SYMBOL_GPL(dma_iova_try_alloc); > + > +/** > + * 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. > + */ > +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? > /** > * 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). Thanks, Robin. > + > +static inline size_t dma_iova_size(struct dma_iova_state *state) > +{ > + return state->__size & ~DMA_IOVA_USE_SWIOTLB; > +} > + > #ifdef CONFIG_DMA_API_DEBUG > void debug_dma_mapping_error(struct device *dev, dma_addr_t dma_addr); > void debug_dma_map_single(struct device *dev, const void *addr, > @@ -277,6 +294,38 @@ static inline int dma_mmap_noncontiguous(struct device *dev, > } > #endif /* CONFIG_HAS_DMA */ > > +#ifdef CONFIG_IOMMU_DMA > +/** > + * dma_use_iova - check if the IOVA API is used for this state > + * @state: IOVA state > + * > + * Return %true if the DMA transfers uses the dma_iova_*() calls or %false if > + * they can't be used. > + */ > +static inline bool dma_use_iova(struct dma_iova_state *state) > +{ > + return state->__size != 0; > +} > + > +bool dma_iova_try_alloc(struct device *dev, struct dma_iova_state *state, > + phys_addr_t phys, size_t size); > +void dma_iova_free(struct device *dev, struct dma_iova_state *state); > +#else /* CONFIG_IOMMU_DMA */ > +static inline bool dma_use_iova(struct dma_iova_state *state) > +{ > + return false; > +} > +static inline bool dma_iova_try_alloc(struct device *dev, > + struct dma_iova_state *state, phys_addr_t phys, size_t size) > +{ > + return false; > +} > +static inline void dma_iova_free(struct device *dev, > + struct dma_iova_state *state) > +{ > +} > +#endif /* CONFIG_IOMMU_DMA */ > + > #if defined(CONFIG_HAS_DMA) && defined(CONFIG_DMA_NEED_SYNC) > void __dma_sync_single_for_cpu(struct device *dev, dma_addr_t addr, size_t size, > enum dma_data_direction dir);