> From: Alex Williamson <alex.williamson@xxxxxxxxxx> > Sent: Wednesday, March 29, 2023 12:00 AM > [...] > > > > A check. If the device that the _INFIO is invoked is opened via cdev, > but > > > there > > > > are devices in the dev_set that are got via > VFIO_GROUP_GET_DEVICE_FD, > > > should > > > > I fail it or allow it? > > > > > > It's a niche case, but I think it needs to be allowed. > > > > I'm also wondering if it is common in the future. Actually, a user should be > > preferred to either use the group or cdev, but not both. Otherwise, it > looks > > to be bad programming.:-) > > > > Also, as an earlier remark from Jason. If there are affected devices that > are > > opened by other users, the new _INFO should fail with -EPERM. I know > this > > remark was for the new _INFO ioctl. But now, we are going to reuse the > > existing _INFO, so I'd also want to check if we still need this policy? If yes, > > then it is a problem to check the owner of the devices that are opened by > > the group path. > > > > https://lore.kernel.org/kvm/ZBsF950laMs2ldFc@xxxxxxxxxx/ > > Personally I don't like the suggestion to fail with -EPERM if the user > doesn't own all the affected devices. This isn't a "probe if I can do > a reset" ioctl, it's a "provide information about the devices affected > by a reset to know how to call the hot-reset ioctl". We're returning > the bdf to the cdev version of this ioctl for exactly this debugging > purpose when the devices are not owned, that becomes useless if we give > up an return -EPERM if ownership doesn't align. Jason's suggestion makes sense for returning the case of returning dev_id as dev_id is local to iommufd. If there are devices in the same dev_set are opened by multiple users, multiple iommufd would be used. Then the dev_id would have overlap. e.g. a dev_set has three devices. Device A and B are opened by the current user as cdev, dev_id #1 and #2 are generated. While device C opened by another user as cdev, dev_id #n is generated for it. If dev_id #n happens to be #1, then user gets two info entries that have the same dev_id. I know the existing _INFO does not have such policy since the group/bdf info are unique in the system. But for the dev_id case, seems really necessary to fail if the dev_set is not only opened by one user. From this angle, will it be good to have two ioctls. Sorry for the back-forth here. ☹ > > > We'd still > > > report the bdf for those devices, but make use of the invalid/null > > > dev-id. I think this empowers userspace that they could make the same > > > call on a group opened fd if necessary. > > > > For the devices opened via group path, it should have an entry that > > includes invalid_dev_id+bdf. So user can map it to the device. But > > there is no group_id, this may be fine since group is just a shortcut > > to find the device. Is it? > > Yes, it could be argued that the group-id itself is superfluous, the > user can determine the group via the bdf, but it also aligns with the > hot-reset ioctl, which currently requires the group fd. Thanks, I see. For the existing _INFO and HOT_RESET ioctl, I have no doubt. Both of them use the group as a short-cut. Regards, Yi Liu
