From: Jason Gunthorpe <jgg@nvidia.com>
To: Lu Baolu <baolu.lu@linux.intel.com>
Cc: Joerg Roedel <joro@8bytes.org>,
Christoph Hellwig <hch@infradead.org>,
Bjorn Helgaas <bhelgaas@google.com>,
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>,
Zhangfei Gao <zhangfei.gao@linaro.org>,
Zhu Tony <tony.zhu@intel.com>,
iommu@lists.linux.dev, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org,
Jean-Philippe Brucker <jean-philippe@linaro.org>
Subject: Re: [PATCH v11 05/13] iommu: Add attach/detach_dev_pasid iommu interface
Date: Thu, 18 Aug 2022 10:33:39 -0300 [thread overview]
Message-ID: <Yv4/s6lX6Nq+40tu@nvidia.com> (raw)
In-Reply-To: <20220817012024.3251276-6-baolu.lu@linux.intel.com>
On Wed, Aug 17, 2022 at 09:20:16AM +0800, Lu Baolu wrote:
> +static int __iommu_set_group_pasid(struct iommu_domain *domain,
> + struct iommu_group *group, ioasid_t pasid)
> +{
> + struct iommu_domain *ops_domain;
> + struct group_device *device;
> + int ret = 0;
> +
> + if (domain == group->blocking_domain)
> + ops_domain = xa_load(&group->pasid_array, pasid);
> + else
> + ops_domain = domain;
This seems weird, why isn't this just always
domain->ops->set_dev_pasid()?
> + if (curr) {
> + ret = xa_err(curr) ? : -EBUSY;
> + goto out_unlock;
> + }
> +
> + ret = __iommu_set_group_pasid(domain, group, pasid);
> + if (ret) {
> + __iommu_set_group_pasid(group->blocking_domain, group, pasid);
> + xa_erase(&group->pasid_array, pasid);
I was looking at this trying to figure out why we are having
attach/detach semantics vs set and this error handling seems to be the
reason
Lets add a comment because it is subtle thing:
Setting a PASID to a blocking domain cannot fail, so we can always
safely error unwind a failure to attach a domain back to the original
group configuration of the PASID being unused.
> +/*
> + * iommu_detach_device_pasid() - Detach the domain from pasid of device
> + * @domain: the iommu domain.
> + * @dev: the attached device.
> + * @pasid: the pasid of the device.
> + *
> + * The @domain must have been attached to @pasid of the @dev with
> + * iommu_attach_device_pasid().
> + */
> +void iommu_detach_device_pasid(struct iommu_domain *domain, struct device *dev,
> + ioasid_t pasid)
Don't pass domain here?
> +/*
> + * iommu_get_domain_for_dev_pasid() - Retrieve domain for @pasid of @dev
> + * @dev: the queried device
> + * @pasid: the pasid of the device
> + *
> + * This is a variant of iommu_get_domain_for_dev(). It returns the existing
> + * domain attached to pasid of a device. It's only for internal use of the
> + * IOMMU subsystem. The caller must take care to avoid any possible
> + * use-after-free case.
How exactly does the caller manage that?
> + *
> + * Return: attached domain on success, NULL otherwise.
> + */
> +struct iommu_domain *
> +iommu_get_domain_for_dev_pasid(struct device *dev, ioasid_t pasid)
> +{
> + struct iommu_domain *domain;
> + struct iommu_group *group;
> +
> + if (!pasid_valid(pasid))
> + return NULL;
Why bother? If the pasid is not valid then it definitely won't be in the xarray.
But otherwise this overall thing seems fine to me
Jason
next prev parent reply other threads:[~2022-08-18 13:33 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-17 1:20 [PATCH v11 00/13] iommu: SVA and IOPF refactoring Lu Baolu
2022-08-17 1:20 ` [PATCH v11 01/13] iommu: Add max_pasids field in struct iommu_device Lu Baolu
2022-08-17 1:20 ` [PATCH v11 02/13] iommu: Add max_pasids field in struct dev_iommu Lu Baolu
2022-08-17 1:20 ` [PATCH v11 03/13] iommu: Remove SVM_FLAG_SUPERVISOR_MODE support Lu Baolu
2022-08-17 1:20 ` [PATCH v11 04/13] PCI: Allow PASID only when ACS enforced on upstreaming path Lu Baolu
2022-08-17 21:17 ` Bjorn Helgaas
2022-08-17 22:48 ` Jason Gunthorpe
2022-08-18 11:55 ` Baolu Lu
2022-08-18 11:53 ` Baolu Lu
2022-08-18 23:00 ` Bjorn Helgaas
2022-08-22 7:43 ` Ethan Zhao
2022-08-23 7:05 ` Baolu Lu
2022-08-24 16:23 ` Bjorn Helgaas
2022-08-18 13:04 ` Jason Gunthorpe
2022-08-23 7:10 ` Baolu Lu
2022-08-17 1:20 ` [PATCH v11 05/13] iommu: Add attach/detach_dev_pasid iommu interface Lu Baolu
2022-08-18 13:33 ` Jason Gunthorpe [this message]
2022-08-23 7:30 ` Baolu Lu
2022-08-17 1:20 ` [PATCH v11 06/13] iommu: Add IOMMU SVA domain support Lu Baolu
2022-08-17 1:20 ` [PATCH v11 07/13] iommu/vt-d: Add " Lu Baolu
2022-08-18 13:36 ` Jason Gunthorpe
2022-08-23 7:33 ` Baolu Lu
2022-08-17 1:20 ` [PATCH v11 08/13] arm-smmu-v3/sva: " Lu Baolu
2022-08-17 1:20 ` [PATCH v11 09/13] iommu/sva: Refactoring iommu_sva_bind/unbind_device() Lu Baolu
2022-08-18 13:41 ` Jason Gunthorpe
2022-08-23 10:12 ` Baolu Lu
2022-08-17 1:20 ` [PATCH v11 10/13] iommu: Remove SVA related callbacks from iommu ops Lu Baolu
2022-08-17 1:20 ` [PATCH v11 11/13] iommu: Prepare IOMMU domain for IOPF Lu Baolu
2022-08-17 1:20 ` [PATCH v11 12/13] iommu: Per-domain I/O page fault handling Lu Baolu
2022-08-17 1:20 ` [PATCH v11 13/13] iommu: Rename iommu-sva-lib.{c,h} Lu Baolu
2022-08-22 4:49 ` [PATCH v11 00/13] iommu: SVA and IOPF refactoring Zhangfei Gao
2022-08-23 7:00 ` Baolu Lu
2022-08-26 4:17 ` Baolu Lu
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=Yv4/s6lX6Nq+40tu@nvidia.com \
--to=jgg@nvidia.com \
--cc=ashok.raj@intel.com \
--cc=baolu.lu@linux.intel.com \
--cc=bhelgaas@google.com \
--cc=dave.jiang@intel.com \
--cc=eric.auger@redhat.com \
--cc=hch@infradead.org \
--cc=iommu@lists.linux.dev \
--cc=jacob.jun.pan@intel.com \
--cc=jean-philippe@linaro.com \
--cc=jean-philippe@linaro.org \
--cc=joro@8bytes.org \
--cc=kevin.tian@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=robin.murphy@arm.com \
--cc=tony.zhu@intel.com \
--cc=vkoul@kernel.org \
--cc=will@kernel.org \
--cc=yi.l.liu@intel.com \
--cc=zhangfei.gao@linaro.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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).