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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id EE53AC433EF for ; Wed, 20 Oct 2021 14:23:02 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C8BC76136F for ; Wed, 20 Oct 2021 14:23:02 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230200AbhJTOZP (ORCPT ); Wed, 20 Oct 2021 10:25:15 -0400 Received: from mail.kernel.org ([198.145.29.99]:57874 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230082AbhJTOZN (ORCPT ); Wed, 20 Oct 2021 10:25:13 -0400 Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id F27F86112D; Wed, 20 Oct 2021 14:22:58 +0000 (UTC) Received: from sofa.misterjones.org ([185.219.108.64] helo=why.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1mdCUa-000S7a-Uq; Wed, 20 Oct 2021 15:22:57 +0100 Date: Wed, 20 Oct 2021 15:22:56 +0100 Message-ID: <8735ovdbcv.wl-maz@kernel.org> From: Marc Zyngier To: Lu Baolu Cc: Sven Peter , iommu@lists.linux-foundation.org, Robin Murphy , Arnd Bergmann , Hector Martin , linux-kernel@vger.kernel.org, Alexander Graf , Mohamed Mediouni , Will Deacon , Alyssa Rosenzweig Subject: Re: [PATCH v3 4/6] iommu: Move IOMMU pagesize check to attach_device In-Reply-To: <9e25f2c0-d9d3-475d-e973-63be1891f9a5@linux.intel.com> References: <20211019163737.46269-1-sven@svenpeter.dev> <20211019163737.46269-5-sven@svenpeter.dev> <9e25f2c0-d9d3-475d-e973-63be1891f9a5@linux.intel.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: baolu.lu@linux.intel.com, sven@svenpeter.dev, iommu@lists.linux-foundation.org, robin.murphy@arm.com, arnd@kernel.org, marcan@marcan.st, linux-kernel@vger.kernel.org, graf@amazon.com, mohamed.mediouni@caramail.com, will@kernel.org, alyssa@rosenzweig.io X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 20 Oct 2021 06:21:44 +0100, Lu Baolu wrote: > > On 2021/10/20 0:37, Sven Peter via iommu wrote: > > The iova allocator is capable of handling any granularity which is a power > > of two. Remove the much stronger condition that the granularity must be > > smaller or equal to the CPU page size from a BUG_ON there. > > Instead, check this condition during __iommu_attach_device and fail > > gracefully. > > > > Signed-off-by: Sven Peter > > --- > > drivers/iommu/iommu.c | 35 ++++++++++++++++++++++++++++++++--- > > drivers/iommu/iova.c | 7 ++++--- > > include/linux/iommu.h | 5 +++++ > > 3 files changed, 41 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c > > index dd7863e453a5..28896739964b 100644 > > --- a/drivers/iommu/iommu.c > > +++ b/drivers/iommu/iommu.c > > @@ -80,6 +80,8 @@ static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus, > > unsigned type); > > static int __iommu_attach_device(struct iommu_domain *domain, > > struct device *dev); > > +static void __iommu_detach_device(struct iommu_domain *domain, > > + struct device *dev); > > static int __iommu_attach_group(struct iommu_domain *domain, > > struct iommu_group *group); > > static void __iommu_detach_group(struct iommu_domain *domain, > > @@ -1974,6 +1976,19 @@ void iommu_domain_free(struct iommu_domain *domain) > > } > > EXPORT_SYMBOL_GPL(iommu_domain_free); > > +static int iommu_check_page_size(struct iommu_domain *domain) > > +{ > > + if (!iommu_is_paging_domain(domain)) > > + return 0; > > + > > + if (!(domain->pgsize_bitmap & (PAGE_SIZE | (PAGE_SIZE - 1)))) { > > + pr_warn("IOMMU pages cannot exactly represent CPU pages.\n"); > > + return -EFAULT; > > + } > > + > > + return 0; > > +} > > + > > static int __iommu_attach_device(struct iommu_domain *domain, > > struct device *dev) > > { > > @@ -1983,9 +1998,23 @@ static int __iommu_attach_device(struct iommu_domain *domain, > > return -ENODEV; > > ret = domain->ops->attach_dev(domain, dev); > > - if (!ret) > > - trace_attach_device_to_domain(dev); > > - return ret; > > + if (ret) > > + return ret; > > + > > + /* > > + * Check that CPU pages can be represented by the IOVA granularity. > > + * This has to be done after ops->attach_dev since many IOMMU drivers > > + * only limit domain->pgsize_bitmap after having attached the first > > + * device. > > + */ > > + ret = iommu_check_page_size(domain); > > + if (ret) { > > + __iommu_detach_device(domain, dev); > > + return ret; > > + } > > It looks odd. __iommu_attach_device() attaches an I/O page table for a > device. How does it relate to CPU pages? Why is it a failure case if CPU > page size is not covered? If you allocate a CPU PAGE_SIZE'd region, and point it at a device that now can DMA to more than what you have allocated because the IOMMU's own page size is larger, the device has now access to data it shouldn't see. In my book, that's a pretty bad thing. M. -- Without deviation from the norm, progress is not possible.