On Thu, 2013-06-20 at 18:48 +1000, Alexey Kardashevskiy wrote: > On 06/20/2013 05:47 PM, Benjamin Herrenschmidt wrote: > > On Thu, 2013-06-20 at 15:28 +1000, David Gibson wrote: > >>> Just out of curiosity - would not get_file() and fput_atomic() on a > >> group's > >>> file* do the right job instead of vfio_group_add_external_user() and > >>> vfio_group_del_external_user()? > >> > >> I was thinking that too. Grabbing a file reference would certainly be > >> the usual way of handling this sort of thing. > > > > But that wouldn't prevent the group ownership to be returned to > > the kernel or another user would it ? > > > Holding the file pointer does not let the group->container_users counter go > to zero
How so? Holding the file pointer means the file won't go away, which means the group release function won't be called. That means the group won't go away, but that doesn't mean it's attached to an IOMMU. A user could call UNSET_CONTAINER. > and this is exactly what vfio_group_add_external_user() and > vfio_group_del_external_user() do. The difference is only in absolute value > - 2 vs. 3. > > No change in behaviour whether I use new vfio API or simply hold file* till > KVM closes fd created when IOMMU was connected to LIOBN. By that notion you could open(/dev/vfio/$GROUP) and you're safe, right? But what about SET_CONTAINER & SET_IOMMU? All that you guarantee holding the file pointer is that the vfio_group exists. > And while this counter is not zero, QEMU cannot take ownership over the group. > > I am definitely still missing the bigger picture... The bigger picture is that the group needs to exist AND it needs to be setup and maintained to have IOMMU protection. Actually, my first stab at add_external_user doesn't look sufficient, it needs to look more like vfio_group_get_device_fd, checking group->container->iommu and group_viable(). As written it would allow an external user after SET_CONTAINER without SET_IOMMU. It should also be part of the API that the external user must hold the file reference between add_external_use and del_external_user and do cleanup on any exit paths. Thanks, Alex -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
