Re: [PATCH V4 1/3] iommu: Add support to change default domain of an iommu group

[Joerg Roedel <joro@8bytes.org>]

On Tue, Jul 14, 2020 at 06:23:54PM +0000, Prakhya, Sai Praneeth wrote:
> Q1:
> > Presently, iommu_change_dev_def_domain() checks if the iommu group still has
> > only one device or not. Hence, checking if iommu group has one device or not is
> > done twice, once before taking device_lock() and the other, after taking
> > device_lock().
> > 
> > I agree that the code isn't checking if the iommu group still has the _same_
> > device or not.
> > One way, I could think of doing it is by storing "dev" temporarily and checking
> > for it.
> > Do you think that's ok? Or would you rather suggest something else?

That sounds reasonable, get the device from the group, lock it, take
group->mutex, and check whether the same device is still alone in the
group.


> Q2:
> > The reason for taking iommu_group->mutex in the beginning of
> > iommu_change_dev_def_domain() is that the function
> > 
> > 1. Checks if the group is being directly used by user level drivers (i.e. if (group-
> > >default_domain != group->domain))
> > 
> > 2. Uses iommu_ops
> > (prev_dom = group->default_domain;
> > if (!prev_dom || !prev_dom->ops || !prev_dom->ops->def_domain_type))
> > 
> > 3. Sets iomu_group->domain to iommu_group->default_domain
> > 
> > I wanted to make sure that iommu_group->domain and iommu_group-
> > >default_domain aren't changed by some other thread while this thread is
> > working on it. So, please let me know if I misunderstood something.

This looks correct as well.

Regards,

	Joerg
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu