On Thu, 2011-09-01 at 14:10 +1000, David Gibson wrote: > On Tue, Aug 30, 2011 at 08:51:38AM -0600, Alex Williamson wrote: > > On Tue, 2011-08-30 at 17:48 +1000, David Gibson wrote: > > > On Mon, Aug 29, 2011 at 10:24:43PM -0600, Alex Williamson wrote: > > > > On Tue, 2011-08-30 at 13:04 +1000, David Gibson wrote: > > > > > On Fri, Aug 26, 2011 at 11:05:23AM -0600, Alex Williamson wrote: > [snip] > > > > > > Once the group is viable, the user may bind the > > > > > > group to another group, retrieve the iommu fd, or retrieve device fds. > > > > > > Internally, each of these operations will result in an iommu domain > > > > > > being allocated and all of the devices attached to the domain. > > > > > > > > > > > > The purpose of binding groups is to share the iommu domain. Groups > > > > > > making use of incompatible iommu domains will fail to bind. Groups > > > > > > making use of different mm's will fail to bind. The vfio driver may > > > > > > reject some binding based on domain capabilities, but final veto power > > > > > > is left to the iommu driver[1]. If a user makes use of a group > > > > > > independently and later wishes to bind it to another group, all the > > > > > > device fds and the iommu fd must first be closed. This prevents using a > > > > > > stale iommu fd or accessing devices while the iommu is being switched. > > > > > > Operations on any group fds of a merged group are performed globally on > > > > > > the group (ie. enumerating the devices lists all devices in the merged > > > > > > group, retrieving the iommu fd from any group fd results in the same fd, > > > > > > device fds from any group can be retrieved from any group fd[2]). > > > > > > Groups can be merged and unmerged dynamically. Unmerging a group > > > > > > requires the device fds for the outgoing group are closed. The iommu fd > > > > > > will remain persistent for the remaining merged group. > > > > > > > > > > As I've said I prefer a persistent group model, rather than this > > > > > transient group model, but it's not a dealbreaker by itself. How are > > > > > unmerges specified? > > > > > > > > VFIO_GROUP_UNMERGE ioctl taking a group fd parameter. > > > > > > > > > I'm also assuming that in this model closing a > > > > > (bound) group fd will unmerge everything down to atomic groups again. > > > > > > > > Yes, it will unmerge the closed group down to the atomic group. > > > > > > Hrm, not thrilled with the merging semantics, but I can probably live > > > with them. Still some clarifications, though.. > > > > > > If you open a group, merge in a bunch of other groups, then re-open > > > /dev/vfio/NNN for one of the groups mergeed, presumably the new fd > > > must also see the merged group? So presumably you must only unmerge > > > everything when all the fds are closed. > > > > The device fds for the group to be unmerged must be closed before an > > unmerge. The iommu fd is tricky. The iommu fd is really the iommu for > > the merged group, not the individual groups, so it's context stays with > > the remaining group. Therefore I don't enforce a refcnt on the iommu > > fd. The usage model I expect is that if a merge works, the user will > > probably use a single iommu fd for the whole merged group. Maybe that > > should be enforced? > > I thought I recalled you saying earlier that the iommu fd could not be > open when merging new groups in. I would expect that also to be true > when unmerging in that case. We have to support hotplug. The group-to-be-merged can't be in use (no open device or iommu fds). To unmerge a group, we only require that no device fds are in use as the merged-group-iommu may still be in use by the remaining members. > > > If you open groups a and b, then merge a (disjoint) bunch of things > > > into each, then merge b into a, what are the semantics? Wheat about > > > if you then unmerge b from a - does it just remove the atomic group b, > > > or everything you merged into b earlier? Or, what happens if you open > > > group a, merge in some things, then attempt to unmerge a from the > > > merged group? > > > > Simple, don't allow merging and unmerging of merged groups. Merge and > > unmerge only work on singleton groups. > > Ok, in that case I think we should call it "add" and "remove" rather > than merge and unmerge. > > > The last case we must support. > > In that case you just use: > > > > ioctl(a.fd, VFIO_GROUP_MERGE, b.fd) > > ioctl(b.fd, VFIO_GROUP_UNMERGE, a.fd) > > > > The groups are peers when merged, so b can remove a as easily as a can > > remove b. Group b is left with any iommu context setup while > > merged. > > Um *goes cross-eyed*. So, if you open (atomic) groups a and b, then > add group b to a, are the two open fds now functionally identical? Yes. > And likewise if you then open either a or b again straight from from > /dev/vfio? Yes. > Except, that the b fd must then retain the fact that it was originally > for atomic group (b), so that it can be used as a handle for an > unmerge/remove. Right. > The more I dig into the details of these semantics the more I dislike > them. Suggest something better. I spent half a day thinking about what vfio would look like in configfs, it has some very appealing aspects, but since it doesn't support ioctls we'd still have a chardev interface and it gets ugly again. > [snip] > > > > Beyond unbind, we also need to think about hotplug. If a system had > > > > multiple hotplug slots below a P2P bridge and a device was added while > > > > the group is in use, what do we do? Maybe we can somehow disable it or > > > > mark it for vfio in our bus notifier routines(?). > > > > > > That is a very good point. It actually brings into focus a niggling > > > concern I had about this model where the group becomes vfio usable > > > once all the devices in it are bound to vfio. Because of the > > > possibility of hotplug, I think its conceptually more correct to not > > > treat vfio as just another kernel driver which can bind devices, but a > > > special state that the whole group goes into atomically. So the > > > sequence would be: > > > - Admin asks that a group go into vfio state > > > - kernel (attempts to) unbind kernel drivers from every device > > > in the group > > > - group is marked in vfio/limbo state > > > > > > At this point no kernel drivers may bind to anything in the group, > > > including things that are hotplugged into the group after this initial > > > sequence. > > > > It seems like this is a mode that could only be accessible if the group > > is opened w/ admin capabilities, I don't think we'd want to let the vfio > > group chrdev owner be able to do that automatically. > > They have to do something that's just as restrictive automatically. > If new devices enter an atomic group that's in use by a guest, the > kernel must not bind drivers to them. I'm just trying to make the > semantics clearer, than proxying the restrictions by binding a dummy > driver to everything. > > > I don't know of > > any other drivers that behave like this, being able to unbind running > > drivers and pull devices into itself. > > Well, it's not a driver behaving like this, it's an explicit admin > operation to unbind all drivers from the whole group and put it in a > state that's suitable for vfio assignment. > > > > > > > If the device fds are not released and > > > > > > subsequently the iommu fd released as well, vfio will kill the user > > > > > > process after some delay. > > > > > > > > > > Ouch, this seems to me a problematic semantic. Whether the user > > > > > process survives depends on whether it processes the remove requests > > > > > fast enough - and a user process could be slowed down by system load > > > > > or other factors not entirely in its control. > > > > > > > > I was assuming "ample" time to process a hot remove, but yes, it's an > > > > area of concern. I'm not sure how much of a problem it is in practice > > > > though. Yes you can shoot your VM accidentally as root... don't do > > > > that. > > > > > > They can, but with this semantic they can't know in advance whether > > > the command is going to kill the VM or not. I can just see a > > > situation where the admin issues a command to remove the device from > > > the guest, and usually that goes through the hot guest unplug > > > mechanisms, the guest keeps running and everything is happy. Then one > > > time they issue *exactly the same command* and the VM dies, because > > > the system is running really slow for some reason (huge load, or maybe > > > someone switched the VM into full emulation for debugging). > > > > Not sure how to handle this other than leave a trail of bread crumbs. > > I have no idea what you mean by that. printk(KERN_WARNING "vfio: killing processes %d (%s) to release device %s.\n", ...) > > > > > I'd be more comfortable with a model where there was a distinction > > > > > between a "soft" and "hard" remove. The soft would either simply > > > > > fail, if the device is in use by vfio, or block indefinitely. The > > > > > hard would kill the user process without delay. This effectively > > > > > allows your semantics to be implemented in userspace (soft remove, > > > > > wait, hard remove) - where it's easier to tweak the policy of how long > > > > > to wait. > > > > > > > > Your first example is essentially what current vfio does now, request > > > > remove, wait indefinitely and qemu triggers an abort if the guest > > > > doesn't respond. The trouble with moving this policy to userspace is > > > > that we're not protecting the host. > > > > > > How is the host not protected? Bear in mind that when I say > > > "userspace" I'm not thinking qemu, I'm thinking the admin equipped > > > with whatever tools he uses for moving devices between guests. So > > > they go: > > > - Please remove this group from the guest > > > - Waits for an amount of time of their choice > > > - Decide, crap, the guest is broken > > > - Hard remove the group from the guest, killing the guest > > > > > > It's basic in perfect analogy to the old: > > > - kill -15 > > > - *drum fingers* > > > - Damn, it's stuck > > > - kill -9 > > > > And what if the remove is initiated by a hardware admin that walks over > > to the system, and presses the PCI device hot unplug doorbell? It just > > looks like a driver hang. Thanks, > > Hm, true. How is this case handled on the host side? Same as if you attempt to unbind the device from the driver, release callback, iirc. 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
