From: Baolu Lu <baolu.lu@linux.intel.com> To: Jason Gunthorpe <jgg@nvidia.com> Cc: Joerg Roedel <joro@8bytes.org>, Christoph Hellwig <hch@infradead.org>, Kevin Tian <kevin.tian@intel.com>, Ashok Raj <ashok.raj@intel.com>, Will Deacon <will@kernel.org>, Robin Murphy <robin.murphy@arm.com>, Jean-Philippe Brucker <jean-philippe@linaro.com>, Dave Jiang <dave.jiang@intel.com>, Vinod Koul <vkoul@kernel.org>, Eric Auger <eric.auger@redhat.com>, Liu Yi L <yi.l.liu@intel.com>, Jacob jun Pan <jacob.jun.pan@intel.com>, iommu@lists.linux-foundation.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v6 08/12] iommu/sva: Use attach/detach_pasid_dev in SVA interfaces Date: Wed, 11 May 2022 15:21:31 +0800 [thread overview] Message-ID: <749a7d62-3e6c-ef5c-beaf-6b7add495740@linux.intel.com> (raw) In-Reply-To: <20220510152330.GG49344@nvidia.com> On 2022/5/10 23:23, Jason Gunthorpe wrote: > On Tue, May 10, 2022 at 02:17:34PM +0800, Lu Baolu wrote: > >> +/** >> + * iommu_sva_bind_device() - Bind a process address space to a device >> + * @dev: the device >> + * @mm: the mm to bind, caller must hold a reference to mm_users >> + * @drvdata: opaque data pointer to pass to bind callback >> + * >> + * Create a bond between device and address space, allowing the device to access >> + * the mm using the returned PASID. If a bond already exists between @device and >> + * @mm, it is returned and an additional reference is taken. Caller must call >> + * iommu_sva_unbind_device() to release each reference. >> + * >> + * iommu_dev_enable_feature(dev, IOMMU_DEV_FEAT_SVA) must be called first, to >> + * initialize the required SVA features. >> + * >> + * On error, returns an ERR_PTR value. >> + */ >> +struct iommu_sva * >> +iommu_sva_bind_device(struct device *dev, struct mm_struct *mm, void *drvdata) >> +{ >> + int ret = -EINVAL; >> + struct iommu_sva *handle; >> + struct iommu_domain *domain; >> + >> + /* >> + * TODO: Remove the drvdata parameter after kernel PASID support is >> + * enabled for the idxd driver. >> + */ >> + if (drvdata) >> + return ERR_PTR(-EOPNOTSUPP); > > Why is this being left behind? Clean up the callers too please. Okay, let me try to. > >> + /* Allocate mm->pasid if necessary. */ >> + ret = iommu_sva_alloc_pasid(mm, 1, (1U << dev->iommu->pasid_bits) - 1); >> + if (ret) >> + return ERR_PTR(ret); >> + >> + mutex_lock(&iommu_sva_lock); >> + /* Search for an existing bond. */ >> + handle = xa_load(&dev->iommu->sva_bonds, mm->pasid); >> + if (handle) { >> + refcount_inc(&handle->users); >> + goto out_success; >> + } > > How can there be an existing bond? > > dev->iommu is per-device > > The device_group_immutable_singleton() insists on a single device > group > > Basically 'sva_bonds' is the same thing as the group->pasid_array. Yes, really. > > Assuming we leave room for multi-device groups this logic should just > be > > group = iommu_group_get(dev); > if (!group) > return -ENODEV; > > mutex_lock(&group->mutex); > domain = xa_load(&group->pasid_array, mm->pasid); > if (!domain || domain->type != IOMMU_DOMAIN_SVA || domain->mm != mm) > domain = iommu_sva_alloc_domain(dev, mm); > > ? Agreed. As a helper in iommu core, how about making it more generic like below? +struct iommu_domain *iommu_get_domain_for_dev_pasid(struct device *dev, + iosid_t pasid, + unsigned int type) +{ + struct iommu_domain *domain; + struct iommu_group *group; + + if (!pasid_valid(pasid)) + return NULL; + + group = iommu_group_get(dev); + if (!group) + return NULL; + + mutex_lock(&group->mutex); + domain = xa_load(&group->pasid_array, pasid); + if (domain && domain->type != type) + domain = NULL; + mutex_unlock(&group->mutex); + iommu_group_put(group); + + return domain; +} > > And stick the refcount in the sva_domain > > Also, given the current arrangement it might make sense to have a > struct iommu_domain_sva given that no driver is wrappering this in > something else. Fair enough. How about below wrapper? +struct iommu_sva_domain { + /* + * Common iommu domain header, *must* be put at the top + * of the structure. + */ + struct iommu_domain domain; + struct mm_struct *mm; + struct iommu_sva bond; +} The refcount is wrapped in bond. Best regards, baolu
WARNING: multiple messages have this Message-ID (diff)
From: Baolu Lu <baolu.lu@linux.intel.com> To: Jason Gunthorpe <jgg@nvidia.com> Cc: Kevin Tian <kevin.tian@intel.com>, Dave Jiang <dave.jiang@intel.com>, Ashok Raj <ashok.raj@intel.com>, Will Deacon <will@kernel.org>, iommu@lists.linux-foundation.org, linux-kernel@vger.kernel.org, Christoph Hellwig <hch@infradead.org>, Jean-Philippe Brucker <jean-philippe@linaro.com>, Vinod Koul <vkoul@kernel.org>, Jacob jun Pan <jacob.jun.pan@intel.com>, Robin Murphy <robin.murphy@arm.com> Subject: Re: [PATCH v6 08/12] iommu/sva: Use attach/detach_pasid_dev in SVA interfaces Date: Wed, 11 May 2022 15:21:31 +0800 [thread overview] Message-ID: <749a7d62-3e6c-ef5c-beaf-6b7add495740@linux.intel.com> (raw) In-Reply-To: <20220510152330.GG49344@nvidia.com> On 2022/5/10 23:23, Jason Gunthorpe wrote: > On Tue, May 10, 2022 at 02:17:34PM +0800, Lu Baolu wrote: > >> +/** >> + * iommu_sva_bind_device() - Bind a process address space to a device >> + * @dev: the device >> + * @mm: the mm to bind, caller must hold a reference to mm_users >> + * @drvdata: opaque data pointer to pass to bind callback >> + * >> + * Create a bond between device and address space, allowing the device to access >> + * the mm using the returned PASID. If a bond already exists between @device and >> + * @mm, it is returned and an additional reference is taken. Caller must call >> + * iommu_sva_unbind_device() to release each reference. >> + * >> + * iommu_dev_enable_feature(dev, IOMMU_DEV_FEAT_SVA) must be called first, to >> + * initialize the required SVA features. >> + * >> + * On error, returns an ERR_PTR value. >> + */ >> +struct iommu_sva * >> +iommu_sva_bind_device(struct device *dev, struct mm_struct *mm, void *drvdata) >> +{ >> + int ret = -EINVAL; >> + struct iommu_sva *handle; >> + struct iommu_domain *domain; >> + >> + /* >> + * TODO: Remove the drvdata parameter after kernel PASID support is >> + * enabled for the idxd driver. >> + */ >> + if (drvdata) >> + return ERR_PTR(-EOPNOTSUPP); > > Why is this being left behind? Clean up the callers too please. Okay, let me try to. > >> + /* Allocate mm->pasid if necessary. */ >> + ret = iommu_sva_alloc_pasid(mm, 1, (1U << dev->iommu->pasid_bits) - 1); >> + if (ret) >> + return ERR_PTR(ret); >> + >> + mutex_lock(&iommu_sva_lock); >> + /* Search for an existing bond. */ >> + handle = xa_load(&dev->iommu->sva_bonds, mm->pasid); >> + if (handle) { >> + refcount_inc(&handle->users); >> + goto out_success; >> + } > > How can there be an existing bond? > > dev->iommu is per-device > > The device_group_immutable_singleton() insists on a single device > group > > Basically 'sva_bonds' is the same thing as the group->pasid_array. Yes, really. > > Assuming we leave room for multi-device groups this logic should just > be > > group = iommu_group_get(dev); > if (!group) > return -ENODEV; > > mutex_lock(&group->mutex); > domain = xa_load(&group->pasid_array, mm->pasid); > if (!domain || domain->type != IOMMU_DOMAIN_SVA || domain->mm != mm) > domain = iommu_sva_alloc_domain(dev, mm); > > ? Agreed. As a helper in iommu core, how about making it more generic like below? +struct iommu_domain *iommu_get_domain_for_dev_pasid(struct device *dev, + iosid_t pasid, + unsigned int type) +{ + struct iommu_domain *domain; + struct iommu_group *group; + + if (!pasid_valid(pasid)) + return NULL; + + group = iommu_group_get(dev); + if (!group) + return NULL; + + mutex_lock(&group->mutex); + domain = xa_load(&group->pasid_array, pasid); + if (domain && domain->type != type) + domain = NULL; + mutex_unlock(&group->mutex); + iommu_group_put(group); + + return domain; +} > > And stick the refcount in the sva_domain > > Also, given the current arrangement it might make sense to have a > struct iommu_domain_sva given that no driver is wrappering this in > something else. Fair enough. How about below wrapper? +struct iommu_sva_domain { + /* + * Common iommu domain header, *must* be put at the top + * of the structure. + */ + struct iommu_domain domain; + struct mm_struct *mm; + struct iommu_sva bond; +} The refcount is wrapped in bond. Best regards, baolu _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
next prev parent reply other threads:[~2022-05-11 7:22 UTC|newest] Thread overview: 80+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-05-10 6:17 [PATCH v6 00/12] iommu: SVA and IOPF refactoring Lu Baolu 2022-05-10 6:17 ` Lu Baolu 2022-05-10 6:17 ` [PATCH v6 01/12] dmaengine: idxd: Separate user and kernel pasid enabling Lu Baolu 2022-05-10 6:17 ` Lu Baolu 2022-05-10 6:17 ` [PATCH v6 02/12] iommu: Add pasid_bits field in struct dev_iommu Lu Baolu 2022-05-10 6:17 ` Lu Baolu 2022-05-10 14:34 ` Jason Gunthorpe via iommu 2022-05-10 14:34 ` Jason Gunthorpe 2022-05-11 2:25 ` Baolu Lu 2022-05-11 2:25 ` Baolu Lu 2022-05-11 8:00 ` Jean-Philippe Brucker 2022-05-11 8:00 ` Jean-Philippe Brucker 2022-05-11 11:59 ` Jason Gunthorpe 2022-05-11 11:59 ` Jason Gunthorpe via iommu 2022-05-10 6:17 ` [PATCH v6 03/12] iommu: Add attach/detach_dev_pasid domain ops Lu Baolu 2022-05-10 6:17 ` Lu Baolu 2022-05-10 14:02 ` Jason Gunthorpe via iommu 2022-05-10 14:02 ` Jason Gunthorpe 2022-05-11 2:32 ` Baolu Lu 2022-05-11 2:32 ` Baolu Lu 2022-05-11 4:09 ` Tian, Kevin 2022-05-11 4:09 ` Tian, Kevin 2022-05-11 7:54 ` Jean-Philippe Brucker 2022-05-11 7:54 ` Jean-Philippe Brucker 2022-05-11 12:02 ` Jason Gunthorpe 2022-05-11 12:02 ` Jason Gunthorpe via iommu 2022-05-12 7:00 ` Jean-Philippe Brucker 2022-05-12 7:00 ` Jean-Philippe Brucker 2022-05-12 11:51 ` Jason Gunthorpe 2022-05-12 11:51 ` Jason Gunthorpe via iommu 2022-05-16 2:03 ` Baolu Lu 2022-05-16 2:03 ` Baolu Lu 2022-05-10 6:17 ` [PATCH v6 04/12] iommu/sva: Basic data structures for SVA Lu Baolu 2022-05-10 6:17 ` Lu Baolu 2022-05-10 6:17 ` [PATCH v6 05/12] iommu/vt-d: Remove SVM_FLAG_SUPERVISOR_MODE support Lu Baolu 2022-05-10 6:17 ` Lu Baolu 2022-05-10 14:35 ` Jason Gunthorpe via iommu 2022-05-10 14:35 ` Jason Gunthorpe 2022-05-10 6:17 ` [PATCH v6 06/12] iommu/vt-d: Add SVA domain support Lu Baolu 2022-05-10 6:17 ` Lu Baolu 2022-05-10 6:17 ` [PATCH v6 07/12] arm-smmu-v3/sva: " Lu Baolu 2022-05-10 6:17 ` Lu Baolu 2022-05-10 6:17 ` [PATCH v6 08/12] iommu/sva: Use attach/detach_pasid_dev in SVA interfaces Lu Baolu 2022-05-10 6:17 ` Lu Baolu 2022-05-10 15:23 ` Jason Gunthorpe via iommu 2022-05-10 15:23 ` Jason Gunthorpe 2022-05-11 7:21 ` Baolu Lu [this message] 2022-05-11 7:21 ` Baolu Lu 2022-05-11 14:53 ` Jason Gunthorpe 2022-05-11 14:53 ` Jason Gunthorpe via iommu 2022-05-12 3:02 ` Baolu Lu 2022-05-12 3:02 ` Baolu Lu 2022-05-12 5:01 ` Tian, Kevin 2022-05-12 5:01 ` Tian, Kevin 2022-05-12 5:17 ` Baolu Lu 2022-05-12 5:17 ` Baolu Lu 2022-05-12 5:44 ` Tian, Kevin 2022-05-12 5:44 ` Tian, Kevin 2022-05-12 6:16 ` Baolu Lu 2022-05-12 6:16 ` Baolu Lu 2022-05-12 11:48 ` Jason Gunthorpe 2022-05-12 11:48 ` Jason Gunthorpe via iommu 2022-05-12 11:59 ` Baolu Lu 2022-05-12 11:59 ` Baolu Lu 2022-05-12 12:03 ` Jason Gunthorpe 2022-05-12 12:03 ` Jason Gunthorpe via iommu 2022-05-12 12:47 ` Baolu Lu 2022-05-12 12:47 ` Baolu Lu 2022-05-12 11:51 ` Jason Gunthorpe 2022-05-12 11:51 ` Jason Gunthorpe via iommu 2022-05-12 12:39 ` Baolu Lu 2022-05-12 12:39 ` Baolu Lu 2022-05-10 6:17 ` [PATCH v6 09/12] iommu: Remove SVA related callbacks from iommu ops Lu Baolu 2022-05-10 6:17 ` Lu Baolu 2022-05-10 6:17 ` [PATCH v6 10/12] iommu: Prepare IOMMU domain for IOPF Lu Baolu 2022-05-10 6:17 ` Lu Baolu 2022-05-10 6:17 ` [PATCH v6 11/12] iommu: Per-domain I/O page fault handling Lu Baolu 2022-05-10 6:17 ` Lu Baolu 2022-05-10 6:17 ` [PATCH v6 12/12] iommu: Rename iommu-sva-lib.{c,h} Lu Baolu 2022-05-10 6:17 ` Lu Baolu
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=749a7d62-3e6c-ef5c-beaf-6b7add495740@linux.intel.com \ --to=baolu.lu@linux.intel.com \ --cc=ashok.raj@intel.com \ --cc=dave.jiang@intel.com \ --cc=eric.auger@redhat.com \ --cc=hch@infradead.org \ --cc=iommu@lists.linux-foundation.org \ --cc=jacob.jun.pan@intel.com \ --cc=jean-philippe@linaro.com \ --cc=jgg@nvidia.com \ --cc=joro@8bytes.org \ --cc=kevin.tian@intel.com \ --cc=linux-kernel@vger.kernel.org \ --cc=robin.murphy@arm.com \ --cc=vkoul@kernel.org \ --cc=will@kernel.org \ --cc=yi.l.liu@intel.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.