On Sat, 2013-06-22 at 13:16 +1000, Alexey Kardashevskiy wrote: > On 06/22/2013 12:57 PM, Alex Williamson wrote: > > On Sat, 2013-06-22 at 12:44 +1000, Alexey Kardashevskiy wrote: > >> On 06/22/2013 11:26 AM, Alex Williamson wrote: > >>> On Sat, 2013-06-22 at 11:16 +1000, Alexey Kardashevskiy wrote: > >>>> Cool, thanks! > >>>> > >>>> So we will need only this (to be called from KVM), and that will be it, right? > >>> > >>> For what? This is not the external lock you're looking for. As I've > >>> mentioned, the file can only hold the group, but that doesn't give you > >>> any guarantee that the group is protected by the IOMMU. Thanks, > >> > >> > >> I am confused, sorry :) With this patch, a group fd cannot be reopened if > >> already opened, and this is the only way for user space to take control > >> over a group. If it is not an external lock, then what is it? And all I > >> have to do now is to verify that the group fd passed to KVM is correct and > >> I am happy. Who and how can break anything (group? KVM?) now? > > > > By that logic all a user needs to do is open() a group and they they're > > free to pass the fd to KVM, right? But the IOMMU protection isn't > > enabled until the user calls SET_CONTAINER and SET_IOMMU, so you'd be > > giving KVM access to the IOMMU that the user hasn't enabled. The group > > may still have devices attached to host drivers. Likewise, a user need > > only call UNSET_CONTAINER to teardown the IOMMU protection. At that > > point a device could be re-bound to host drivers, thus making it unsafe > > for KVM to be directly poking the IOMMU. > > > > This patch is just a bug fix for inconsistent behavior. Thanks, > > Oh. Thanks for the detailed explanation, I was missing this one. > > Yeah. Looks like we need some other brand new lock now... Something like a > notifier from VFIO to KVM to inform KVM that it can or cannot use the group > now (on VFIO_IOMMU_ENABLE/VFIO_IOMMU_DISABLE or some new ioctls) so KVM > would not touch the group in not allowed. > > typedef int notifier_fn(bool enable); > int vfio_group_iommu_id_from_file(struct file *filep, notifier_fn fn); > > ? I don't think we need either a new lock or a notifier. If we do as Ben suggested and allow KVM to keep the group active then the requirements are pretty much identical to having an open vfio device file descriptor. This is why I suggested container_users. I think the code I proposed will work, it just needs a few more conditions tested when the external user is added to make sure the group is iommu protected. Thanks, Alex -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
