Message ID | 20221128064648.1934720-19-baolu.lu@linux.intel.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:f944:0:0:0:0:0 with SMTP id q4csp5476035wrr; Sun, 27 Nov 2022 22:59:31 -0800 (PST) X-Google-Smtp-Source: AA0mqf7eHDui2cxn+pligA2XUcC6KlV13+tY0FqW+xWoA4l21dzb8gmdRTfkXghV7ChaBkpfjKzj X-Received: by 2002:aa7:cc12:0:b0:462:79ec:55eb with SMTP id q18-20020aa7cc12000000b0046279ec55ebmr45037825edt.151.1669618771625; Sun, 27 Nov 2022 22:59:31 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1669618771; cv=none; d=google.com; s=arc-20160816; b=A+le4t60M8K0e1Jr/LW57jFG2qsqXBj7eJMXzdJpayjqXZkZJwMY0raKv/lcfd7xDi RgBdO4igmICY1BwnjOPnRzeqW5/eqPnnJaY8iXNr/LI+CCY1XPBB7g8mw7wC7DACURqG dINUHh73h9ZUZqt/tB+Nt6KTqN7FpG5mg4QtKLGsxL7E5571m231bCcxACQr0mew+uAf 4Sf4O3rzkOdYLJbzfLOrNsjT/aPYoFrnoDas8wwf471zUlDEhFNl6u2qEn/Fhlxdk8sq Cu1nLPm9ySdArez8KX8TMjQJdYwLpsgWLYZEjn0Fi4ICQNBx5x5SpcxiG7bCiojW9KUJ AVlw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=brdbahYQyV2jwDdygf0gWag5Vzeo/4P3LbhpqOZjHm8=; b=Q7/KAL3aCEhcFzJ3fgmpoGV5uTukNEFizrZxdqNoVXVkxkkhj2qGYG9rkQr7j25SnY M9rreUi4DV1tdHlCs7g5I+zANCY+j++hW/ZMxF/pvOc+ILkHguaYtg/o8qoo0f6aZE3y Ntz6++LXwY46CWxb+LOMbsq2lK1rUxcZ+9CY0VG+6gXjw6IM5BBxKuw2+blExIh09fRf q5Pfsf2GxOLr0ElpGC7Toyj73/eX5fEd/u+3Ae3Q3J2l15fz7CQhfo1xs2OqMdsElX4b b03qaGVBfeix9gKrBSyI+vxjz+fISz62D5P7Hw0o1y/KdS7+2LHbWEkdjqTcMXNWvWUG mzbw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=bZDvXmg3; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id qw24-20020a1709066a1800b007baa6e2275asi9545091ejc.236.2022.11.27.22.59.08; Sun, 27 Nov 2022 22:59:31 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=bZDvXmg3; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229868AbiK1G5C (ORCPT <rfc822;gah0developer@gmail.com> + 99 others); Mon, 28 Nov 2022 01:57:02 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50714 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229934AbiK1G4U (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Mon, 28 Nov 2022 01:56:20 -0500 Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C202B15826 for <linux-kernel@vger.kernel.org>; Sun, 27 Nov 2022 22:56:17 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1669618577; x=1701154577; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=2u7/GIfAe0DIonm/0hJIJlmbZhf+CBwAyKoVx4wlxOs=; b=bZDvXmg33vtWikJdBBjf8FS/AKg5ydS+avyP9b74slaqHwNHfGqyLSjA 0ZAN5mtyoGWT/T868EM1b26pgRDIA4+x+hX8T5eyhae53gVsW/J4Kmh6L 21p6y+eRxiPnMtyFlF2op1VCh8vtuJb8w6aHkYJWmdoIiDmcu0WeR9aw2 nZbzFaPULH8MMjoOhNuCXdsKRMFmnRAY4z0RIlBaWFxvOtLNfn6+3rdQL 8i26w0GHpaIKyGe7Knr92WnGjRtV5ZCwAGe1649NCGcaXc4Uo7yI+iokX RQACHuwiJN3+kAMrvysnqay0hOC+FtrSynvXs/7Sf9ziV+a/kSCL5qQWy Q==; X-IronPort-AV: E=McAfee;i="6500,9779,10544"; a="379016449" X-IronPort-AV: E=Sophos;i="5.96,199,1665471600"; d="scan'208";a="379016449" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Nov 2022 22:56:12 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6500,9779,10544"; a="674121303" X-IronPort-AV: E=Sophos;i="5.96,199,1665471600"; d="scan'208";a="674121303" Received: from allen-box.sh.intel.com ([10.239.159.48]) by orsmga008.jf.intel.com with ESMTP; 27 Nov 2022 22:56:05 -0800 From: Lu Baolu <baolu.lu@linux.intel.com> To: Joerg Roedel <joro@8bytes.org>, Jason Gunthorpe <jgg@nvidia.com>, Christoph Hellwig <hch@infradead.org>, Kevin Tian <kevin.tian@intel.com>, Will Deacon <will@kernel.org>, Robin Murphy <robin.murphy@arm.com>, Jean-Philippe Brucker <jean-philippe@linaro.org> Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>, Hector Martin <marcan@marcan.st>, Sven Peter <sven@svenpeter.dev>, Rob Clark <robdclark@gmail.com>, Marek Szyprowski <m.szyprowski@samsung.com>, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>, Andy Gross <agross@kernel.org>, Bjorn Andersson <andersson@kernel.org>, Yong Wu <yong.wu@mediatek.com>, Matthias Brugger <matthias.bgg@gmail.com>, Heiko Stuebner <heiko@sntech.de>, Matthew Rosato <mjrosato@linux.ibm.com>, Orson Zhai <orsonzhai@gmail.com>, Baolin Wang <baolin.wang@linux.alibaba.com>, Chunyan Zhang <zhang.lyra@gmail.com>, Chen-Yu Tsai <wens@csie.org>, Thierry Reding <thierry.reding@gmail.com>, iommu@lists.linux.dev, linux-kernel@vger.kernel.org, Lu Baolu <baolu.lu@linux.intel.com> Subject: [PATCH v3 18/20] iommu: Call set_platform_dma if default domain is unavailable Date: Mon, 28 Nov 2022 14:46:46 +0800 Message-Id: <20221128064648.1934720-19-baolu.lu@linux.intel.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20221128064648.1934720-1-baolu.lu@linux.intel.com> References: <20221128064648.1934720-1-baolu.lu@linux.intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-4.3 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,SPF_HELO_NONE, SPF_NONE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1750722173040599797?= X-GMAIL-MSGID: =?utf-8?q?1750722173040599797?= |
Series |
iommu: Retire detach_dev callback
|
|
Commit Message
Baolu Lu
Nov. 28, 2022, 6:46 a.m. UTC
If the IOMMU driver has no default domain support, call set_platform_dma
explicitly to return the kernel DMA control back to the platform DMA ops.
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
drivers/iommu/iommu.c | 28 ++++++++--------------------
1 file changed, 8 insertions(+), 20 deletions(-)
Comments
On Mon, Nov 28, 2022 at 02:46:46PM +0800, Lu Baolu wrote: > If the IOMMU driver has no default domain support, call set_platform_dma > explicitly to return the kernel DMA control back to the platform DMA ops. > > Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com> > --- > drivers/iommu/iommu.c | 28 ++++++++-------------------- > 1 file changed, 8 insertions(+), 20 deletions(-) > > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c > index 7c99d8eb3182..e4966f088184 100644 > --- a/drivers/iommu/iommu.c > +++ b/drivers/iommu/iommu.c > @@ -2040,16 +2040,6 @@ int iommu_deferred_attach(struct device *dev, struct iommu_domain *domain) > return 0; > } > > -static void __iommu_detach_device(struct iommu_domain *domain, > - struct device *dev) > -{ > - if (iommu_is_attach_deferred(dev)) > - return; This removal might want to be its own patch with an explanation. It looks like at the current moment __iommu_detach_device() is only called via call chains that are after the device driver is attached - eg via explicit attach APIs called by the device driver. So it should just unconditionally work. It is actually looks like a bug that we were blocking detach on these paths since the attach was unconditional and the caller is going to free the (probably) UNAMANGED domain once this returns. The only place we should be testing for deferred attach is during the initial point the dma device is linked to the group, and then again during the dma api calls to check if the device This maybe the patch that is needed to explain this: diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index d69ebba81bebd8..06f1fe6563bb30 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -993,8 +993,8 @@ int iommu_group_add_device(struct iommu_group *group, struct device *dev) mutex_lock(&group->mutex); list_add_tail(&device->list, &group->devices); - if (group->domain && !iommu_is_attach_deferred(dev)) - ret = __iommu_attach_device(group->domain, dev); + if (group->domain) + ret = iommu_group_do_dma_first_attach(dev, group->domain); mutex_unlock(&group->mutex); if (ret) goto err_put_group; @@ -1760,21 +1760,24 @@ static void probe_alloc_default_domain(struct bus_type *bus, } -static int iommu_group_do_dma_attach(struct device *dev, void *data) +static int iommu_group_do_dma_first_attach(struct device *dev, void *data) { struct iommu_domain *domain = data; - int ret = 0; - if (!iommu_is_attach_deferred(dev)) - ret = __iommu_attach_device(domain, dev); + lockdep_assert_held(&dev->iommu_group->mutex); - return ret; + if (iommu_is_attach_deferred(dev)) { + dev->iommu->attach_deferred = 1; + return 0; + } + + return __iommu_attach_device(domain, dev); } -static int __iommu_group_dma_attach(struct iommu_group *group) +static int __iommu_group_dma_first_attach(struct iommu_group *group) { return __iommu_group_for_each_dev(group, group->default_domain, - iommu_group_do_dma_attach); + iommu_group_do_dma_first_attach); } static int iommu_group_do_probe_finalize(struct device *dev, void *data) @@ -1839,7 +1842,7 @@ int bus_iommu_probe(struct bus_type *bus) iommu_group_create_direct_mappings(group); - ret = __iommu_group_dma_attach(group); + ret = __iommu_group_dma_first_attach(group); mutex_unlock(&group->mutex); @@ -1971,9 +1974,11 @@ 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; + dev->iommu->attach_deferred = 0; + trace_attach_device_to_domain(dev); + return 0; } /** @@ -2018,7 +2023,7 @@ EXPORT_SYMBOL_GPL(iommu_attach_device); int iommu_deferred_attach(struct device *dev, struct iommu_domain *domain) { - if (iommu_is_attach_deferred(dev)) + if (dev->iommu && dev->iommu->attach_deferred) return __iommu_attach_device(domain, dev); return 0; @@ -2027,9 +2032,6 @@ int iommu_deferred_attach(struct device *dev, struct iommu_domain *domain) static void __iommu_detach_device(struct iommu_domain *domain, struct device *dev) { - if (iommu_is_attach_deferred(dev)) - return; - domain->ops->detach_dev(domain, dev); trace_detach_device_from_domain(dev); } diff --git a/include/linux/iommu.h b/include/linux/iommu.h index 1690c334e51631..ebac04a13fff68 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -413,6 +413,7 @@ struct dev_iommu { struct iommu_device *iommu_dev; void *priv; u32 max_pasids; + u8 attach_deferred; }; int iommu_device_register(struct iommu_device *iommu,
Hi Jason, On 11/28/22 10:57 PM, Jason Gunthorpe wrote: > On Mon, Nov 28, 2022 at 02:46:46PM +0800, Lu Baolu wrote: >> If the IOMMU driver has no default domain support, call set_platform_dma >> explicitly to return the kernel DMA control back to the platform DMA ops. >> >> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com> >> --- >> drivers/iommu/iommu.c | 28 ++++++++-------------------- >> 1 file changed, 8 insertions(+), 20 deletions(-) >> >> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c >> index 7c99d8eb3182..e4966f088184 100644 >> --- a/drivers/iommu/iommu.c >> +++ b/drivers/iommu/iommu.c >> @@ -2040,16 +2040,6 @@ int iommu_deferred_attach(struct device *dev, struct iommu_domain *domain) >> return 0; >> } >> >> -static void __iommu_detach_device(struct iommu_domain *domain, >> - struct device *dev) >> -{ >> - if (iommu_is_attach_deferred(dev)) >> - return; > > This removal might want to be its own patch with an explanation. > > It looks like at the current moment __iommu_detach_device() is only > called via call chains that are after the device driver is attached - > eg via explicit attach APIs called by the device driver. > > So it should just unconditionally work. It is actually looks like a > bug that we were blocking detach on these paths since the attach was > unconditional and the caller is going to free the (probably) UNAMANGED > domain once this returns. > > The only place we should be testing for deferred attach is during the > initial point the dma device is linked to the group, and then again > during the dma api calls to check if the device > > This maybe the patch that is needed to explain this: > > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c > index d69ebba81bebd8..06f1fe6563bb30 100644 > --- a/drivers/iommu/iommu.c > +++ b/drivers/iommu/iommu.c > @@ -993,8 +993,8 @@ int iommu_group_add_device(struct iommu_group *group, struct device *dev) > > mutex_lock(&group->mutex); > list_add_tail(&device->list, &group->devices); > - if (group->domain && !iommu_is_attach_deferred(dev)) > - ret = __iommu_attach_device(group->domain, dev); > + if (group->domain) > + ret = iommu_group_do_dma_first_attach(dev, group->domain); > mutex_unlock(&group->mutex); > if (ret) > goto err_put_group; > @@ -1760,21 +1760,24 @@ static void probe_alloc_default_domain(struct bus_type *bus, > > } > > -static int iommu_group_do_dma_attach(struct device *dev, void *data) > +static int iommu_group_do_dma_first_attach(struct device *dev, void *data) > { > struct iommu_domain *domain = data; > - int ret = 0; > > - if (!iommu_is_attach_deferred(dev)) > - ret = __iommu_attach_device(domain, dev); > + lockdep_assert_held(&dev->iommu_group->mutex); > > - return ret; > + if (iommu_is_attach_deferred(dev)) { > + dev->iommu->attach_deferred = 1; > + return 0; > + } > + > + return __iommu_attach_device(domain, dev); > } > > -static int __iommu_group_dma_attach(struct iommu_group *group) > +static int __iommu_group_dma_first_attach(struct iommu_group *group) > { > return __iommu_group_for_each_dev(group, group->default_domain, > - iommu_group_do_dma_attach); > + iommu_group_do_dma_first_attach); > } > > static int iommu_group_do_probe_finalize(struct device *dev, void *data) > @@ -1839,7 +1842,7 @@ int bus_iommu_probe(struct bus_type *bus) > > iommu_group_create_direct_mappings(group); > > - ret = __iommu_group_dma_attach(group); > + ret = __iommu_group_dma_first_attach(group); > > mutex_unlock(&group->mutex); > > @@ -1971,9 +1974,11 @@ 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; > + dev->iommu->attach_deferred = 0; > + trace_attach_device_to_domain(dev); > + return 0; > } > > /** > @@ -2018,7 +2023,7 @@ EXPORT_SYMBOL_GPL(iommu_attach_device); > > int iommu_deferred_attach(struct device *dev, struct iommu_domain *domain) > { > - if (iommu_is_attach_deferred(dev)) > + if (dev->iommu && dev->iommu->attach_deferred) > return __iommu_attach_device(domain, dev); > > return 0; > @@ -2027,9 +2032,6 @@ int iommu_deferred_attach(struct device *dev, struct iommu_domain *domain) > static void __iommu_detach_device(struct iommu_domain *domain, > struct device *dev) > { > - if (iommu_is_attach_deferred(dev)) > - return; > - > domain->ops->detach_dev(domain, dev); > trace_detach_device_from_domain(dev); > } > diff --git a/include/linux/iommu.h b/include/linux/iommu.h > index 1690c334e51631..ebac04a13fff68 100644 > --- a/include/linux/iommu.h > +++ b/include/linux/iommu.h > @@ -413,6 +413,7 @@ struct dev_iommu { > struct iommu_device *iommu_dev; > void *priv; > u32 max_pasids; > + u8 attach_deferred; > }; > > int iommu_device_register(struct iommu_device *iommu, Thanks for the patch! It seems that we also need to call iommu_group_do_dma_first_attach() in the iommu_probe_device() path? @@ -401,7 +425,7 @@ int iommu_probe_device(struct device *dev) * attach the default domain. */ if (group->default_domain && !group->owner) { - ret = __iommu_attach_device(group->default_domain, dev); + ret = iommu_group_do_dma_first_attach(dev, group->default_domain); if (ret) { mutex_unlock(&group->mutex); iommu_group_put(group); By the way, I'd like to put above code in a separated patch of the next version, can I add your signed-off-by? -- Best regards, baolu
On Tue, Jan 03, 2023 at 10:45:24AM +0800, Baolu Lu wrote: > Thanks for the patch! It seems that we also need to call > iommu_group_do_dma_first_attach() in the iommu_probe_device() path? Yes > By the way, I'd like to put above code in a separated patch of the next > version, can I add your signed-off-by? Yes Jason
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 7c99d8eb3182..e4966f088184 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -2040,16 +2040,6 @@ int iommu_deferred_attach(struct device *dev, struct iommu_domain *domain) return 0; } -static void __iommu_detach_device(struct iommu_domain *domain, - struct device *dev) -{ - if (iommu_is_attach_deferred(dev)) - return; - - domain->ops->detach_dev(domain, dev); - trace_detach_device_from_domain(dev); -} - void iommu_detach_device(struct iommu_domain *domain, struct device *dev) { struct iommu_group *group; @@ -2154,11 +2144,12 @@ int iommu_attach_group(struct iommu_domain *domain, struct iommu_group *group) } EXPORT_SYMBOL_GPL(iommu_attach_group); -static int iommu_group_do_detach_device(struct device *dev, void *data) +static int iommu_group_do_set_platform_dma(struct device *dev, void *data) { - struct iommu_domain *domain = data; + const struct iommu_ops *ops = dev_iommu_ops(dev); - __iommu_detach_device(domain, dev); + if (!WARN_ON(!ops->set_platform_dma)) + ops->set_platform_dma(dev); return 0; } @@ -2172,15 +2163,12 @@ static int __iommu_group_set_domain(struct iommu_group *group, return 0; /* - * New drivers should support default domains and so the detach_dev() op - * will never be called. Otherwise the NULL domain represents some - * platform specific behavior. + * New drivers should support default domains. Otherwise the NULL + * domain represents returning control back to the platform DMA. */ if (!new_domain) { - if (WARN_ON(!group->domain->ops->detach_dev)) - return -EINVAL; - __iommu_group_for_each_dev(group, group->domain, - iommu_group_do_detach_device); + __iommu_group_for_each_dev(group, NULL, + iommu_group_do_set_platform_dma); group->domain = NULL; return 0; }