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 X-Spam-Level: X-Spam-Status: No, score=-8.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_2 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7EFFDCA9EC0 for ; Mon, 28 Oct 2019 22:41:49 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 49D6520679 for ; Mon, 28 Oct 2019 22:41:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729589AbfJ1Wls (ORCPT ); Mon, 28 Oct 2019 18:41:48 -0400 Received: from mga07.intel.com ([134.134.136.100]:23723 "EHLO mga07.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725867AbfJ1Wlr (ORCPT ); Mon, 28 Oct 2019 18:41:47 -0400 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 28 Oct 2019 15:41:47 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.68,241,1569308400"; d="scan'208";a="374361343" Received: from jacob-builder.jf.intel.com (HELO jacob-builder) ([10.7.199.155]) by orsmga005.jf.intel.com with ESMTP; 28 Oct 2019 15:41:47 -0700 Date: Mon, 28 Oct 2019 15:46:11 -0700 From: Jacob Pan To: "Tian, Kevin" Cc: "iommu@lists.linux-foundation.org" , LKML , Joerg Roedel , "David Woodhouse" , Alex Williamson , Jean-Philippe Brucker , "Liu, Yi L" , "Raj, Ashok" , Christoph Hellwig , Lu Baolu , Jonathan Cameron , Eric Auger , jacob.jun.pan@linux.intel.com Subject: Re: [PATCH v7 04/11] iommu/vt-d: Replace Intel specific PASID allocator with IOASID Message-ID: <20191028154611.5c9979fd@jacob-builder> In-Reply-To: References: <1571946904-86776-1-git-send-email-jacob.jun.pan@linux.intel.com> <1571946904-86776-5-git-send-email-jacob.jun.pan@linux.intel.com> Organization: OTC X-Mailer: Claws Mail 3.13.2 (GTK+ 2.24.30; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 25 Oct 2019 06:41:16 +0000 "Tian, Kevin" wrote: > > From: Jacob Pan [mailto:jacob.jun.pan@linux.intel.com] > > Sent: Friday, October 25, 2019 3:55 AM > > > > Make use of generic IOASID code to manage PASID allocation, > > free, and lookup. Replace Intel specific code. > > > > Signed-off-by: Jacob Pan > > better push this patch separately. It's a generic cleanup. > True but might be more efficient to have this cleanup patch paved way. Since the follow up new guest SVA code uses the new API. So I wanted to get rid of the old code completely. > > --- > > drivers/iommu/intel-iommu.c | 12 ++++++------ > > drivers/iommu/intel-pasid.c | 36 > > ------------------------------------ drivers/iommu/intel-svm.c | > > 39 +++++++++++++++++++++++---------------- 3 files changed, 29 > > insertions(+), 58 deletions(-) > > > > diff --git a/drivers/iommu/intel-iommu.c > > b/drivers/iommu/intel-iommu.c index ced1d89ef977..2ea09b988a23 > > 100644 --- a/drivers/iommu/intel-iommu.c > > +++ b/drivers/iommu/intel-iommu.c > > @@ -5311,7 +5311,7 @@ static void auxiliary_unlink_device(struct > > dmar_domain *domain, > > domain->auxd_refcnt--; > > > > if (!domain->auxd_refcnt && domain->default_pasid > 0) > > - intel_pasid_free_id(domain->default_pasid); > > + ioasid_free(domain->default_pasid); > > } > > > > static int aux_domain_add_dev(struct dmar_domain *domain, > > @@ -5329,10 +5329,10 @@ static int aux_domain_add_dev(struct > > dmar_domain *domain, > > if (domain->default_pasid <= 0) { > > int pasid; > > > > - pasid = intel_pasid_alloc_id(domain, PASID_MIN, > > - > > pci_max_pasids(to_pci_dev(dev)), > > - GFP_KERNEL); > > - if (pasid <= 0) { > > + /* No private data needed for the default pasid */ > > + pasid = ioasid_alloc(NULL, PASID_MIN, > > pci_max_pasids(to_pci_dev(dev)) - 1, > > + NULL); > > + if (pasid == INVALID_IOASID) { > > pr_err("Can't allocate default pasid\n"); > > return -ENODEV; > > } > > @@ -5368,7 +5368,7 @@ static int aux_domain_add_dev(struct > > dmar_domain *domain, > > spin_unlock(&iommu->lock); > > spin_unlock_irqrestore(&device_domain_lock, flags); > > if (!domain->auxd_refcnt && domain->default_pasid > 0) > > - intel_pasid_free_id(domain->default_pasid); > > + ioasid_free(domain->default_pasid); > > > > return ret; > > } > > diff --git a/drivers/iommu/intel-pasid.c > > b/drivers/iommu/intel-pasid.c index d81e857d2b25..e79d680fe300 > > 100644 --- a/drivers/iommu/intel-pasid.c > > +++ b/drivers/iommu/intel-pasid.c > > @@ -26,42 +26,6 @@ > > */ > > static DEFINE_SPINLOCK(pasid_lock); > > u32 intel_pasid_max_id = PASID_MAX; > > -static DEFINE_IDR(pasid_idr); > > - > > -int intel_pasid_alloc_id(void *ptr, int start, int end, gfp_t gfp) > > -{ > > - int ret, min, max; > > - > > - min = max_t(int, start, PASID_MIN); > > - max = min_t(int, end, intel_pasid_max_id); > > - > > - WARN_ON(in_interrupt()); > > - idr_preload(gfp); > > - spin_lock(&pasid_lock); > > - ret = idr_alloc(&pasid_idr, ptr, min, max, GFP_ATOMIC); > > - spin_unlock(&pasid_lock); > > - idr_preload_end(); > > - > > - return ret; > > -} > > - > > -void intel_pasid_free_id(int pasid) > > -{ > > - spin_lock(&pasid_lock); > > - idr_remove(&pasid_idr, pasid); > > - spin_unlock(&pasid_lock); > > -} > > - > > -void *intel_pasid_lookup_id(int pasid) > > -{ > > - void *p; > > - > > - spin_lock(&pasid_lock); > > - p = idr_find(&pasid_idr, pasid); > > - spin_unlock(&pasid_lock); > > - > > - return p; > > -} > > > > int vcmd_alloc_pasid(struct intel_iommu *iommu, unsigned int > > *pasid) { > > diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c > > index 9b159132405d..a9a7f85a09bc 100644 > > --- a/drivers/iommu/intel-svm.c > > +++ b/drivers/iommu/intel-svm.c > > @@ -17,6 +17,7 @@ > > #include > > #include > > #include > > +#include > > #include > > > > #include "intel-pasid.h" > > @@ -318,16 +319,15 @@ int intel_svm_bind_mm(struct device *dev, int > > *pasid, int flags, struct svm_dev_ > > if (pasid_max > intel_pasid_max_id) > > pasid_max = intel_pasid_max_id; > > > > - /* Do not use PASID 0 in caching mode (virtualised > > IOMMU) */ > > - ret = intel_pasid_alloc_id(svm, > > - !!cap_caching_mode(iommu->cap), > > - pasid_max - 1, > > GFP_KERNEL); > > - if (ret < 0) { > > + /* Do not use PASID 0, reserved for RID to PASID */ > > + svm->pasid = ioasid_alloc(NULL, PASID_MIN, > > + pasid_max - 1, svm); > > + if (svm->pasid == INVALID_IOASID) { > > kfree(svm); > > kfree(sdev); > > + ret = ENOSPC; > > goto out; > > } > > - svm->pasid = ret; > > svm->notifier.ops = &intel_mmuops; > > svm->mm = mm; > > svm->flags = flags; > > @@ -337,7 +337,7 @@ int intel_svm_bind_mm(struct device *dev, int > > *pasid, int flags, struct svm_dev_ > > if (mm) { > > ret = > > mmu_notifier_register(&svm->notifier, mm); if (ret) { > > - intel_pasid_free_id(svm->pasid); > > + ioasid_free(svm->pasid); > > kfree(svm); > > kfree(sdev); > > goto out; > > @@ -353,7 +353,7 @@ int intel_svm_bind_mm(struct device *dev, int > > *pasid, int flags, struct svm_dev_ > > if (ret) { > > if (mm) > > mmu_notifier_unregister(&svm->notifier, > > mm); > > - intel_pasid_free_id(svm->pasid); > > + ioasid_free(svm->pasid); > > kfree(svm); > > kfree(sdev); > > goto out; > > @@ -401,7 +401,12 @@ int intel_svm_unbind_mm(struct device *dev, int > > pasid) > > if (!iommu) > > goto out; > > > > - svm = intel_pasid_lookup_id(pasid); > > + svm = ioasid_find(NULL, pasid, NULL); > > + if (IS_ERR(svm)) { > > + ret = PTR_ERR(svm); > > + goto out; > > + } > > + > > if (!svm) > > goto out; > > > > @@ -423,7 +428,9 @@ int intel_svm_unbind_mm(struct device *dev, int > > pasid) > > kfree_rcu(sdev, rcu); > > > > if (list_empty(&svm->devs)) { > > - > > intel_pasid_free_id(svm->pasid); > > + /* Clear private data so > > that free pass check */ > > + > > ioasid_set_data(svm->pasid, NULL); > > + ioasid_free(svm->pasid); > > if (svm->mm) > > > > mmu_notifier_unregister(&svm->notifier, svm->mm); > > > > @@ -458,10 +465,11 @@ int intel_svm_is_pasid_valid(struct device > > *dev, int pasid) > > if (!iommu) > > goto out; > > > > - svm = intel_pasid_lookup_id(pasid); > > - if (!svm) > > + svm = ioasid_find(NULL, pasid, NULL); > > + if (IS_ERR(svm)) { > > + ret = PTR_ERR(svm); > > goto out; > > - > > + } > > /* init_mm is used in this case */ > > if (!svm->mm) > > ret = 1; > > @@ -568,13 +576,12 @@ static irqreturn_t prq_event_thread(int irq, > > void *d) > > > > if (!svm || svm->pasid != req->pasid) { > > rcu_read_lock(); > > - svm = intel_pasid_lookup_id(req->pasid); > > + svm = ioasid_find(NULL, req->pasid, NULL); > > /* It *can't* go away, because the driver > > is not permitted > > * to unbind the mm while any page faults > > are outstanding. > > * So we only need RCU to protect the > > internal idr code. */ > > rcu_read_unlock(); > > - > > - if (!svm) { > > + if (IS_ERR(svm) || !svm) { > > pr_err("%s: Page request for > > invalid PASID %d: %08llx %08llx\n", > > iommu->name, req->pasid, > > ((unsigned long long *)req)[0], > > ((unsigned long long > > *)req)[1]); -- > > 2.7.4 > [Jacob Pan]