| Message ID | 64511b5e5b2771e223799b92db40bee71e962b56.1740600272.git.nicolinc@nvidia.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | iommu: Isolate iova_cookie to actual owners | expand |
On Wed, Feb 26, 2025 at 12:16:05PM -0800, Nicolin Chen wrote: > The iommu_put_dma_cookie() will be moved out of iommu_domain_free(). For a > default domain, iommu_put_dma_cookie() can be simply added to this helper. > > Suggested-by: Jason Gunthorpe <jgg@nvidia.com> > Signed-off-by: Nicolin Chen <nicolinc@nvidia.com> > --- > drivers/iommu/iommu.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) Let's try to do what Robin suggested and put a private_data_owner value in the struct then this patch isn't used, we'd just do if (domain->private_data_owner == DMA) iommu_put_dma_cookie(domain); Instead of this change and the similar VFIO change Thanks, Jason
On Thu, Feb 27, 2025 at 03:50:36PM -0400, Jason Gunthorpe wrote: > On Wed, Feb 26, 2025 at 12:16:05PM -0800, Nicolin Chen wrote: > > The iommu_put_dma_cookie() will be moved out of iommu_domain_free(). For a > > default domain, iommu_put_dma_cookie() can be simply added to this helper. > > > > Suggested-by: Jason Gunthorpe <jgg@nvidia.com> > > Signed-off-by: Nicolin Chen <nicolinc@nvidia.com> > > --- > > drivers/iommu/iommu.c | 11 ++++++++--- > > 1 file changed, 8 insertions(+), 3 deletions(-) > > Let's try to do what Robin suggested and put a private_data_owner > value in the struct then this patch isn't used, we'd just do > > if (domain->private_data_owner == DMA) > iommu_put_dma_cookie(domain); > > Instead of this change and the similar VFIO change Ack. I assume I should go with a smaller series starting with this "private_data_owner", and then later a bigger series for the other bits like translation_type that you mentioned in the other thread. Thanks Nicolin
On Thu, Feb 27, 2025 at 12:59:04PM -0800, Nicolin Chen wrote: > On Thu, Feb 27, 2025 at 03:50:36PM -0400, Jason Gunthorpe wrote: > > On Wed, Feb 26, 2025 at 12:16:05PM -0800, Nicolin Chen wrote: > > > The iommu_put_dma_cookie() will be moved out of iommu_domain_free(). For a > > > default domain, iommu_put_dma_cookie() can be simply added to this helper. > > > > > > Suggested-by: Jason Gunthorpe <jgg@nvidia.com> > > > Signed-off-by: Nicolin Chen <nicolinc@nvidia.com> > > > --- > > > drivers/iommu/iommu.c | 11 ++++++++--- > > > 1 file changed, 8 insertions(+), 3 deletions(-) > > > > Let's try to do what Robin suggested and put a private_data_owner > > value in the struct then this patch isn't used, we'd just do > > > > if (domain->private_data_owner == DMA) > > iommu_put_dma_cookie(domain); > > > > Instead of this change and the similar VFIO change > > Ack. I assume I should go with a smaller series starting with this > "private_data_owner", and then later a bigger series for the other > bits like translation_type that you mentioned in the other thread. That could work, you could bitfiled type and steal a few bits for "private_data_owner" ? Then try the sw_msi removal at the same time too? Jason
On Thu, Feb 27, 2025 at 05:03:36PM -0400, Jason Gunthorpe wrote: > On Thu, Feb 27, 2025 at 12:59:04PM -0800, Nicolin Chen wrote: > > On Thu, Feb 27, 2025 at 03:50:36PM -0400, Jason Gunthorpe wrote: > > > On Wed, Feb 26, 2025 at 12:16:05PM -0800, Nicolin Chen wrote: > > > > The iommu_put_dma_cookie() will be moved out of iommu_domain_free(). For a > > > > default domain, iommu_put_dma_cookie() can be simply added to this helper. > > > > > > > > Suggested-by: Jason Gunthorpe <jgg@nvidia.com> > > > > Signed-off-by: Nicolin Chen <nicolinc@nvidia.com> > > > > --- > > > > drivers/iommu/iommu.c | 11 ++++++++--- > > > > 1 file changed, 8 insertions(+), 3 deletions(-) > > > > > > Let's try to do what Robin suggested and put a private_data_owner > > > value in the struct then this patch isn't used, we'd just do > > > > > > if (domain->private_data_owner == DMA) > > > iommu_put_dma_cookie(domain); > > > > > > Instead of this change and the similar VFIO change > > > > Ack. I assume I should go with a smaller series starting with this > > "private_data_owner", and then later a bigger series for the other > > bits like translation_type that you mentioned in the other thread. > > That could work, you could bitfiled type and steal a few bits for > "private_data_owner" ? > > Then try the sw_msi removal at the same time too? Ack. I drafted four patches: iommu: Add private_data_owner to iommu_domain_free iommu: Turn iova_cookie to dma-iommu private pointer iommufd: Move iommufd_sw_msi and related functions to driver.c iommu: Drop sw_msi from iommu_domain Will do some proper build tests and then wrap them up. Thanks Nicolin
On 27/02/2025 11:32 pm, Nicolin Chen wrote: > On Thu, Feb 27, 2025 at 05:03:36PM -0400, Jason Gunthorpe wrote: >> On Thu, Feb 27, 2025 at 12:59:04PM -0800, Nicolin Chen wrote: >>> On Thu, Feb 27, 2025 at 03:50:36PM -0400, Jason Gunthorpe wrote: >>>> On Wed, Feb 26, 2025 at 12:16:05PM -0800, Nicolin Chen wrote: >>>>> The iommu_put_dma_cookie() will be moved out of iommu_domain_free(). For a >>>>> default domain, iommu_put_dma_cookie() can be simply added to this helper. >>>>> >>>>> Suggested-by: Jason Gunthorpe <jgg@nvidia.com> >>>>> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com> >>>>> --- >>>>> drivers/iommu/iommu.c | 11 ++++++++--- >>>>> 1 file changed, 8 insertions(+), 3 deletions(-) >>>> >>>> Let's try to do what Robin suggested and put a private_data_owner >>>> value in the struct then this patch isn't used, we'd just do >>>> >>>> if (domain->private_data_owner == DMA) >>>> iommu_put_dma_cookie(domain); >>>> >>>> Instead of this change and the similar VFIO change >>> >>> Ack. I assume I should go with a smaller series starting with this >>> "private_data_owner", and then later a bigger series for the other >>> bits like translation_type that you mentioned in the other thread. >> >> That could work, you could bitfiled type and steal a few bits for >> "private_data_owner" ? >> >> Then try the sw_msi removal at the same time too? > > Ack. I drafted four patches: > iommu: Add private_data_owner to iommu_domain_free > iommu: Turn iova_cookie to dma-iommu private pointer > iommufd: Move iommufd_sw_msi and related functions to driver.c > iommu: Drop sw_msi from iommu_domain > > Will do some proper build tests and then wrap them up. Ah, I spent yesterday also writing up a patch to sort things out more generally - expect to see that shortly, then we can decide what we like. Cheers, Robin.
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 022bf96a18c5..28cde7007cd7 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -456,6 +456,11 @@ static int iommu_init_device(struct device *dev, const struct iommu_ops *ops) return ret; } +static void iommu_default_domain_free(struct iommu_domain *domain) +{ + iommu_domain_free(domain); +} + static void iommu_deinit_device(struct device *dev) { struct iommu_group *group = dev->iommu_group; @@ -494,7 +499,7 @@ static void iommu_deinit_device(struct device *dev) */ if (list_empty(&group->devices)) { if (group->default_domain) { - iommu_domain_free(group->default_domain); + iommu_default_domain_free(group->default_domain); group->default_domain = NULL; } if (group->blocking_domain) { @@ -3000,7 +3005,7 @@ static int iommu_setup_default_domain(struct iommu_group *group, out_free_old: if (old_dom) - iommu_domain_free(old_dom); + iommu_default_domain_free(old_dom); return ret; err_restore_domain: @@ -3009,7 +3014,7 @@ static int iommu_setup_default_domain(struct iommu_group *group, group, old_dom, IOMMU_SET_DOMAIN_MUST_SUCCEED); err_restore_def_domain: if (old_dom) { - iommu_domain_free(dom); + iommu_default_domain_free(dom); group->default_domain = old_dom; } return ret;
The iommu_put_dma_cookie() will be moved out of iommu_domain_free(). For a default domain, iommu_put_dma_cookie() can be simply added to this helper. Suggested-by: Jason Gunthorpe <jgg@nvidia.com> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com> --- drivers/iommu/iommu.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)