From: Jacob Pan <jacob.jun.pan@linux.intel.com> To: "Tian, Kevin" <kevin.tian@intel.com> Cc: "vkoul@kernel.org" <vkoul@kernel.org>, "Jiang, Dave" <dave.jiang@intel.com>, "Raj, Ashok" <ashok.raj@intel.com>, "will@kernel.org" <will@kernel.org>, David Woodhouse <dwmw2@infradead.org>, LKML <linux-kernel@vger.kernel.org>, Christoph Hellwig <hch@infradead.org>, "iommu@lists.linux-foundation.org" <iommu@lists.linux-foundation.org>, Jason Gunthorpe <jgg@nvidia.com>, "dmaengine@vger.kernel.org" <dmaengine@vger.kernel.org>, "robin.murphy@arm.com" <robin.murphy@arm.com>, Jean-Philippe Brucker <jean-philippe@linaro.com> Subject: Re: [PATCH v4 2/6] iommu: Add a helper to do PASID lookup from domain Date: Mon, 23 May 2022 11:01:58 -0700 [thread overview] Message-ID: <20220523110158.3382b5fd@jacob-builder> (raw) In-Reply-To: <BN9PR11MB5276622272BCA2ED982EE3C18CD49@BN9PR11MB5276.namprd11.prod.outlook.com> Hi Kevin, On Mon, 23 May 2022 09:14:04 +0000, "Tian, Kevin" <kevin.tian@intel.com> wrote: > > From: Tian, Kevin > > Sent: Monday, May 23, 2022 3:55 PM > > > > > From: Jacob Pan <jacob.jun.pan@linux.intel.com> > > > +ioasid_t iommu_get_pasid_from_domain(struct device *dev, struct > > > iommu_domain *domain) > > > +{ > > > + struct iommu_domain *tdomain; > > > + struct iommu_group *group; > > > + unsigned long index; > > > + ioasid_t pasid = INVALID_IOASID; > > > + > > > + group = iommu_group_get(dev); > > > + if (!group) > > > + return pasid; > > > + > > > + xa_for_each(&group->pasid_array, index, tdomain) { > > > + if (domain == tdomain) { > > > + pasid = index; > > > + break; > > > + } > > > + } > > > > Don't we need to acquire the group lock here? > > pasid_array is under RCU read lock so it is protected though may have stale data. It also used in atomic context for TLB flush, cannot take the group mutex. If the caller does detach_dev_pasid while doing TLB flush, it could result in extra flush but harmless. > > Btw the intention of this function is a bit confusing. Patch01 already > > stores the pasid under domain hence it's redundant to get it > > indirectly from xarray index. You could simply introduce a flag bit > > (e.g. dma_pasid_enabled) in device_domain_info and then directly > > use domain->dma_pasid once the flag is true. > > > > Just saw your discussion with Jason about v3. While it makes sense > to not specialize DMA domain in iommu driver, the use of this function > should only be that when the call chain doesn't pass down a pasid > value e.g. when doing cache invalidation for domain map/unmap. If > the upper interface already carries a pasid e.g. in detach_dev_pasid() > iommu driver can simply verify that the corresponding pasid xarray > entry points to the specified domain instead of using this function to > loop xarray and then verify the returned pasid (as done in patch03/04). Excellent point, I could just use xa_load(pasid) to compare the domain instead of loop through xa. I will add another helper. bool iommu_is_pasid_domain_attached(struct device *dev, struct iommu_domain *domain, ioasid_t pasid) { struct iommu_group *group; bool ret = false; group = iommu_group_get(dev); if (WARN_ON(!group)) return false; if (domain == xa_load(&group->pasid_array, pasid)) ret = true; iommu_group_put(group); return ret; } Thanks, Jacob _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
WARNING: multiple messages have this Message-ID (diff)
From: Jacob Pan <jacob.jun.pan@linux.intel.com> To: "Tian, Kevin" <kevin.tian@intel.com> Cc: "iommu@lists.linux-foundation.org" <iommu@lists.linux-foundation.org>, LKML <linux-kernel@vger.kernel.org>, "dmaengine@vger.kernel.org" <dmaengine@vger.kernel.org>, Joerg Roedel <joro@8bytes.org>, David Woodhouse <dwmw2@infradead.org>, Jean-Philippe Brucker <jean-philippe@linaro.com>, "Lu Baolu" <baolu.lu@linux.intel.com>, Jason Gunthorpe <jgg@nvidia.com>, Christoph Hellwig <hch@infradead.org>, "vkoul@kernel.org" <vkoul@kernel.org>, "robin.murphy@arm.com" <robin.murphy@arm.com>, "will@kernel.org" <will@kernel.org>, "Liu, Yi L" <yi.l.liu@intel.com>, "Jiang, Dave" <dave.jiang@intel.com>, "Raj, Ashok" <ashok.raj@intel.com>, Eric Auger <eric.auger@redhat.com>, jacob.jun.pan@linux.intel.com Subject: Re: [PATCH v4 2/6] iommu: Add a helper to do PASID lookup from domain Date: Mon, 23 May 2022 11:01:58 -0700 [thread overview] Message-ID: <20220523110158.3382b5fd@jacob-builder> (raw) In-Reply-To: <BN9PR11MB5276622272BCA2ED982EE3C18CD49@BN9PR11MB5276.namprd11.prod.outlook.com> Hi Kevin, On Mon, 23 May 2022 09:14:04 +0000, "Tian, Kevin" <kevin.tian@intel.com> wrote: > > From: Tian, Kevin > > Sent: Monday, May 23, 2022 3:55 PM > > > > > From: Jacob Pan <jacob.jun.pan@linux.intel.com> > > > +ioasid_t iommu_get_pasid_from_domain(struct device *dev, struct > > > iommu_domain *domain) > > > +{ > > > + struct iommu_domain *tdomain; > > > + struct iommu_group *group; > > > + unsigned long index; > > > + ioasid_t pasid = INVALID_IOASID; > > > + > > > + group = iommu_group_get(dev); > > > + if (!group) > > > + return pasid; > > > + > > > + xa_for_each(&group->pasid_array, index, tdomain) { > > > + if (domain == tdomain) { > > > + pasid = index; > > > + break; > > > + } > > > + } > > > > Don't we need to acquire the group lock here? > > pasid_array is under RCU read lock so it is protected though may have stale data. It also used in atomic context for TLB flush, cannot take the group mutex. If the caller does detach_dev_pasid while doing TLB flush, it could result in extra flush but harmless. > > Btw the intention of this function is a bit confusing. Patch01 already > > stores the pasid under domain hence it's redundant to get it > > indirectly from xarray index. You could simply introduce a flag bit > > (e.g. dma_pasid_enabled) in device_domain_info and then directly > > use domain->dma_pasid once the flag is true. > > > > Just saw your discussion with Jason about v3. While it makes sense > to not specialize DMA domain in iommu driver, the use of this function > should only be that when the call chain doesn't pass down a pasid > value e.g. when doing cache invalidation for domain map/unmap. If > the upper interface already carries a pasid e.g. in detach_dev_pasid() > iommu driver can simply verify that the corresponding pasid xarray > entry points to the specified domain instead of using this function to > loop xarray and then verify the returned pasid (as done in patch03/04). Excellent point, I could just use xa_load(pasid) to compare the domain instead of loop through xa. I will add another helper. bool iommu_is_pasid_domain_attached(struct device *dev, struct iommu_domain *domain, ioasid_t pasid) { struct iommu_group *group; bool ret = false; group = iommu_group_get(dev); if (WARN_ON(!group)) return false; if (domain == xa_load(&group->pasid_array, pasid)) ret = true; iommu_group_put(group); return ret; } Thanks, Jacob
next prev parent reply other threads:[~2022-05-23 17:58 UTC|newest] Thread overview: 70+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-05-18 18:21 [PATCH v4 0/6] Enable PASID for DMA API users Jacob Pan 2022-05-18 18:21 ` Jacob Pan 2022-05-18 18:21 ` [PATCH v4 1/6] iommu: Add a per domain PASID for DMA API Jacob Pan 2022-05-18 18:21 ` Jacob Pan 2022-05-19 6:50 ` Baolu Lu 2022-05-19 6:50 ` Baolu Lu 2022-05-24 13:50 ` Jason Gunthorpe 2022-05-24 13:50 ` Jason Gunthorpe via iommu 2022-05-24 15:17 ` Jacob Pan 2022-05-24 15:17 ` Jacob Pan 2022-05-30 12:22 ` Jason Gunthorpe 2022-05-30 12:22 ` Jason Gunthorpe via iommu 2022-05-31 10:12 ` Tian, Kevin 2022-05-31 10:12 ` Tian, Kevin 2022-05-31 12:45 ` Baolu Lu 2022-05-31 12:45 ` Baolu Lu 2022-05-31 16:03 ` Jason Gunthorpe 2022-05-31 16:03 ` Jason Gunthorpe via iommu 2022-05-31 17:29 ` Jacob Pan 2022-05-31 17:29 ` Jacob Pan 2022-05-31 19:05 ` Jason Gunthorpe 2022-05-31 19:05 ` Jason Gunthorpe via iommu 2022-05-31 20:44 ` Jacob Pan 2022-05-31 20:44 ` Jacob Pan 2022-06-01 1:50 ` Tian, Kevin 2022-06-01 1:50 ` Tian, Kevin 2022-06-01 1:43 ` Tian, Kevin 2022-06-01 1:43 ` Tian, Kevin 2022-06-01 9:37 ` Baolu Lu 2022-06-01 9:37 ` Baolu Lu 2022-06-01 10:05 ` Tian, Kevin 2022-06-01 10:05 ` Tian, Kevin 2022-05-18 18:21 ` [PATCH v4 2/6] iommu: Add a helper to do PASID lookup from domain Jacob Pan 2022-05-18 18:21 ` Jacob Pan 2022-05-19 6:41 ` Baolu Lu 2022-05-19 6:41 ` Baolu Lu 2022-05-19 20:10 ` Jacob Pan 2022-05-19 20:10 ` Jacob Pan 2022-05-19 6:48 ` Christoph Hellwig 2022-05-19 6:48 ` Christoph Hellwig 2022-05-20 15:18 ` Jacob Pan 2022-05-20 15:18 ` Jacob Pan 2022-05-23 7:55 ` Tian, Kevin 2022-05-23 7:55 ` Tian, Kevin 2022-05-23 9:14 ` Tian, Kevin 2022-05-23 9:14 ` Tian, Kevin 2022-05-23 18:01 ` Jacob Pan [this message] 2022-05-23 18:01 ` Jacob Pan 2022-05-18 18:21 ` [PATCH v4 3/6] iommu/vt-d: Implement domain ops for attach_dev_pasid Jacob Pan 2022-05-18 18:21 ` Jacob Pan 2022-05-24 13:51 ` Jason Gunthorpe 2022-05-24 13:51 ` Jason Gunthorpe via iommu 2022-05-24 16:12 ` Jacob Pan 2022-05-24 16:12 ` Jacob Pan 2022-05-24 18:02 ` Jason Gunthorpe 2022-05-24 18:02 ` Jason Gunthorpe via iommu 2022-05-24 20:45 ` Jacob Pan 2022-05-24 20:45 ` Jacob Pan 2022-05-24 21:10 ` Jason Gunthorpe 2022-05-24 21:10 ` Jason Gunthorpe via iommu 2022-05-18 18:21 ` [PATCH v4 4/6] iommu: Add PASID support for DMA mapping API users Jacob Pan 2022-05-18 18:21 ` Jacob Pan 2022-05-23 8:25 ` Tian, Kevin 2022-05-23 8:25 ` Tian, Kevin 2022-05-23 15:23 ` Jacob Pan 2022-05-23 15:23 ` Jacob Pan 2022-05-18 18:21 ` [PATCH v4 5/6] dmaengine: idxd: Use DMA API for in-kernel DMA with PASID Jacob Pan 2022-05-18 18:21 ` Jacob Pan 2022-05-18 18:21 ` [PATCH v4 6/6] iommu/vt-d: Delete unused SVM flag Jacob Pan 2022-05-18 18:21 ` Jacob Pan
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=20220523110158.3382b5fd@jacob-builder \ --to=jacob.jun.pan@linux.intel.com \ --cc=ashok.raj@intel.com \ --cc=dave.jiang@intel.com \ --cc=dmaengine@vger.kernel.org \ --cc=dwmw2@infradead.org \ --cc=hch@infradead.org \ --cc=iommu@lists.linux-foundation.org \ --cc=jean-philippe@linaro.com \ --cc=jgg@nvidia.com \ --cc=kevin.tian@intel.com \ --cc=linux-kernel@vger.kernel.org \ --cc=robin.murphy@arm.com \ --cc=vkoul@kernel.org \ --cc=will@kernel.org \ /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.