Messages in this thread | | | From | "Tian, Kevin" <> | Subject | RE: [PATCH v2 3/3] iommufd/device: Change iommufd_hw_pagetable_has_group to device centric | Date | Mon, 30 Jan 2023 00:44:48 +0000 |
| |
> From: Nicolin Chen <nicolinc@nvidia.com> > Sent: Sunday, January 29, 2023 6:39 PM > > On Sun, Jan 29, 2023 at 09:37:00AM +0000, Tian, Kevin wrote: > > External email: Use caution opening links or attachments > > > > > > > From: Nicolin Chen <nicolinc@nvidia.com> > > > Sent: Sunday, January 29, 2023 5:18 AM > > > > > > -static bool iommufd_hw_pagetable_has_group(struct > > > iommufd_hw_pagetable *hwpt, > > > - struct iommu_group *group) > > > +static bool iommufd_hw_pagetable_has_device(struct > > > iommufd_hw_pagetable *hwpt, > > > + struct device *dev) > > > { > > > - struct iommufd_device *cur_dev; > > > - > > > - list_for_each_entry(cur_dev, &hwpt->devices, devices_item) > > > - if (cur_dev->group == group) > > > - return true; > > > - return false; > > > + /* > > > + * iommu_get_domain_for_dev() returns an iommu_group->domain > > > ptr, if it > > > + * is the same domain as the hwpt->domain, it means that this hwpt > > > has > > > + * the iommu_group/device. > > > + */ > > > + return hwpt->domain == iommu_get_domain_for_dev(dev); > > > } > > > > Here we could have three scenarios: > > > > 1) the device is attached to blocked domain; > > 2) the device is attached to hwpt->domain; > > 3) the device is attached to another hwpt->domain; > > > > if this function returns false then iommufd_device_do_attach() will attach > > the device to the specified hwpt. But then it's wrong for 3). > > > > Has 3) been denied in earlier path? If yes at least a WARN_ON for > > case 3) makes sense here. > > The case #3 means the device is already attached to some other > domain? Then vfio_iommufd_physical_attach_ioas returns -EBUSY > at the sanity of vdev->iommufd_attached. And the case #3 feels > like a domain replacement use case to me. So probably not that > necessary to add a wARN_ON? >
You are right. I thought about the cdev case where the device is not attached in vfio but has a valid domain due to attach status of other devices in the group. But even in this case it's user's responsibility to not break group boundary. So yes it's just a domain replacement and WARN_ON is not required.
| |