Messages in this thread | | | Date | Mon, 27 Apr 2009 13:48:21 +0300 | From | Avi Kivity <> | Subject | Re: [KVM PATCH v2 2/2] kvm: add support for irqfd via eventfd-notification interface |
| |
Gregory Haskins wrote: >>> This allows an eventfd to be registered as an irq source with a >>> guest. Any >>> signaling operation on the eventfd (via userspace or kernel) will inject >>> the registered GSI at the next available window. >>> >>> >>> +struct kvm_irqfd { >>> + __u32 fd; >>> + __u32 gsi; >>> +}; >>> + >>> >>> >> I think it's better to have ioctl create and return the fd. This way >> we aren't tied to eventfd (though it makes a lot of sense to use it). >> > > I dont mind either way, but I am not sure it buys us much as the one > driving the fd would need to understand if the interface is > eventfd-esque or something else anyway. Let me know if you still want > to see this changed. >
Sure, the interface remains the same (write 8 bytes), but the implementation can change. For example, we can implement it to work from interrupt context, once we hack the locking appropriately.
>>> +static void >>> +irqfd_inject(struct work_struct *work) >>> +{ >>> + struct _irqfd *irqfd = container_of(work, struct _irqfd, work); >>> + struct kvm *kvm = irqfd->kvm; >>> + >>> + mutex_lock(&kvm->lock); >>> + kvm_set_irq(kvm, kvm->irqfd.src, irqfd->gsi, 1); >>> >>> >> Need to lower the irq too (though irqfd only supports edge triggered >> interrupts). >> >> > Should I just do back-to-back 1+0 inside the same lock? > >
Yes. Might be nice to add a kvm_toggle_irq(), but let's leave that until later.
>> One day we'll have lockless injection and we'll want to drop this. I >> guess if we create the fd ourselves we can make it work, but I don't >> see how we can do this with eventfd. >> >> > > Hmm...this is a good point. There probably is no way to use eventfd > "off the shelf" in a way that doesn't cause this callback to be in a > critical section. Should we just worry about switching away from > eventfd when this occurs, or should I implement a custom anon-fd now? >
I'd just go with eventfd, and switch when it becomes relevant. As long as the kernel allocates the fd, we're free to do as we like.
-- error compiling committee.c: too many arguments to function
| |