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