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 2617DC02183 for ; Wed, 15 Jan 2025 06:13:37 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id AFC16280005; Wed, 15 Jan 2025 01:13:36 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id AAC94280001; Wed, 15 Jan 2025 01:13:36 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 99AD3280005; Wed, 15 Jan 2025 01:13:36 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 79299280001 for ; Wed, 15 Jan 2025 01:13:36 -0500 (EST) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 2A32512107F for ; Wed, 15 Jan 2025 06:13:36 +0000 (UTC) X-FDA: 83008669632.13.449CA78 Received: from verein.lst.de (verein.lst.de [213.95.11.211]) by imf08.hostedemail.com (Postfix) with ESMTP id 39F9F160002 for ; Wed, 15 Jan 2025 06:13:33 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=none; dmarc=none; spf=pass (imf08.hostedemail.com: domain of hch@lst.de designates 213.95.11.211 as permitted sender) smtp.mailfrom=hch@lst.de ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1736921614; 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; bh=dsiBNa4jmcXpNJmEILcR7ols7kbwqQXgDLiyk9mvB1E=; b=wbkUAOP1aAGtglSi1lK7Uw8DHHKrwQjC7t3mILUnKut33+XyzKza+w9ejhr4nSIskrfTC5 WG6l91C7x8+KRUUL4A+AXzvRvg7iYgVI2Mqt7Vo686C+JPo4OQ2OyGlSqGN125pXT+B7Lb 3IJnSjXgp9MGO1Meftx6jYw56mcAnj4= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=none; dmarc=none; spf=pass (imf08.hostedemail.com: domain of hch@lst.de designates 213.95.11.211 as permitted sender) smtp.mailfrom=hch@lst.de ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1736921614; a=rsa-sha256; cv=none; b=iXsj7xeu7Y5kdV4w4w17/5nmOAIpuwyrrNPgu9l2lbSqBvgtjBND3VGecKxNiVUqucJo4n VilD75W/aI1+orfLzqfaMCMJc2XE9FDBW7LNelq3Q4AmlN7cwi8mbnnL1cFj1vFBQOu7Dt W6r9eknzsqiwgvGXT1FxB3yy7c1UQOQ= Received: by verein.lst.de (Postfix, from userid 2407) id F412068B05; Wed, 15 Jan 2025 07:13:26 +0100 (CET) Date: Wed, 15 Jan 2025 07:13:26 +0100 From: Christoph Hellwig To: Robin Murphy Cc: Leon Romanovsky , Jens Axboe , Jason Gunthorpe , Joerg Roedel , Will Deacon , Christoph Hellwig , Sagi Grimberg , Leon Romanovsky , 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: <20250115061326.GA29643@lst.de> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.17 (2007-11-01) X-Rspamd-Queue-Id: 39F9F160002 X-Rspamd-Server: rspam12 X-Stat-Signature: 4auhkxbtaarti5dcx9nshoux4zbrs8xa X-Rspam-User: X-HE-Tag: 1736921613-832884 X-HE-Meta: U2FsdGVkX19ytsYLpcT4Q9sVECQ0mYmC1UiawQ8yvMHElezUBvfZzGQ44dqlxX5L82vTcGqfkXVcDWXZUAgAWQCAug2WZmYDW2ifTcG//kanxeXLkEQ9SoJHP3eU6gsmhjAxeFW+95ePds/ME9SVZ/lpN+0n2BcyaFrZ6m/GQWMUmYJ3UEB2CaizX9UgS0IICvbMmn1jPAqcyaSlanexr7klvfpzBrJl3XHB7x0OKmO9FfHYsiiDcixN0Hci82pB98lZn3Tze5qUx0JAcJe2WMClTgttRaY2Bo/xU7ljEKbiqEg85OS4ybThttY5UCJF0HDdyV9dxONos2BbAtzI26829W4d1n1qd/Y16DcnE7ocM7TQ+e0GxmpRTAnKW8ItjBGx5+uXkQZCi04BGnluifqFfG9lObauQS8av1jHG3aK/MFt+SgTUnltZXEVLVoI7vfeDG24OcHQhy0njg+gir1zPdjn7EHTVMhoI35uftKvRccP175vNeBbNnP4MF30Bwm6rARQa6O7ssiN7wa7de/UlMzXNOJ0nM5uzhcuR3uZMLN+YdtQGYrqj/esOrURmPyqz4/Wlu2k77VNoHRXKGCOBs/LweKFj4cZYPjqiOGQoldv8ARBOFDG29QjYDuq1O2HALpBeILAFdLRFN4kEx2HSPkoVgJtuTS1kjuWPRcQrXEeeLJBXNz2iyPsFMD6LPKxqcWjZ+T0duPp7lJP9KybH9gQkM7sXcO4+Q+U1AROZaDjAZJ+PsfunR3Edw2vMxKP0hRL2/KWLF+ZyuztFWMf2SJVPQKpk1P71+dyuM/8j//Lyg29a2PFSsIjbQj1h2AFWsMaKNZxxjUJMsKKddPfMgERTQAaDQ/ZxHVaXRuLw2vJnto8Wfs0fA8EEMirVxNO2O1GExBp9otifcJncH37NU7ONBS+hSMCYLdGWsm42jqtro3mGHyWjHgT6DhdnwM7JSnUMLAXjj/Gnv2 pH0hNYCw nHVVj9qKMW8perGXEjcJEZtHaP7a6fL4ZBNVB9BB1RR7Ipupth8BmEREXfPCM1immeRfwbCNZwZ78aAk= 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: >> +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? It is regularly taken. Now that it's quoted this way it would probably good to split the thing up to not do the deferferences above, as they might cause problems if the compiler wasn't smart enough to only perform them after the check.. >> + 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. As the person who added it - this is to catch a user passing in a value that would set it. To me this looks obvious, but should we add a comment? > (Which also makes me consider iommu_dma_max_mapping_size() returning > SIZE_MAX isn't strictly accurate, ho hum...) You can still map SIZE_MAX, just not using this interface. Assuming no other real life limitations get in way, which I bet they will. >> @@ -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). Good point. I guess __size should simply become a u64.