On Sun, Jun 14, 2009 at 08:40:57AM -0400, Gregory Haskins wrote:
> Michael S. Tsirkin wrote:
> > On Wed, May 20, 2009 at 10:30:49AM -0400, Gregory Haskins wrote:
> >
> > ...
> >
> >
> >> diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
> >> +static int
> >> +kvm_assign_irqfd(struct kvm *kvm, int fd, int gsi)
> >> +{
> >> + struct _irqfd *irqfd;
> >> + struct file *file = NULL;
> >> + int ret;
> >> +
> >> + irqfd = kzalloc(sizeof(*irqfd), GFP_KERNEL);
> >> + if (!irqfd)
> >> + return -ENOMEM;
> >> +
> >> + irqfd->kvm = kvm;
> >> + irqfd->gsi = gsi;
> >> + INIT_LIST_HEAD(&irqfd->list);
> >> + INIT_WORK(&irqfd->work, irqfd_inject);
> >> +
> >> + /*
> >> + * Embed the file* lifetime in the irqfd.
> >> + */
> >> + file = fget(fd);
> >> + if (IS_ERR(file)) {
> >> + ret = PTR_ERR(file);
> >> + goto fail;
> >> + }
> >> +
> >> + /*
> >> + * Install our own custom wake-up handling so we are notified via
> >> + * a callback whenever someone signals the underlying eventfd
> >> + */
> >> + init_waitqueue_func_entry(&irqfd->wait, irqfd_wakeup);
> >> + init_poll_funcptr(&irqfd->pt, irqfd_ptable_queue_proc);
> >> +
> >> + ret = file->f_op->poll(file, &irqfd->pt);
> >> + if (ret < 0)
> >> + goto fail;
> >> +
> >> + irqfd->file = file;
> >> +
> >> + mutex_lock(&kvm->lock);
> >> + list_add_tail(&irqfd->list, &kvm->irqfds);
> >> + mutex_unlock(&kvm->lock);
> >> +
> >> + return 0;
> >> +
> >> +fail:
> >> + if (irqfd->wqh)
> >> + remove_wait_queue(irqfd->wqh, &irqfd->wait);
> >> +
> >> + if (file && !IS_ERR(file))
> >> + fput(file);
> >> +
> >> + kfree(irqfd);
> >> + return ret;
> >> +}
> >>
> >
> > It seems that this lets the guest assign an unlimited number of fds
> > to the same gsi, potentially using up all of kernel memory.
> >
> > Since we don't need multiple fds assigned to the same gsi (instead,
> > multiple processes can write to the same eventfd to trigger an
> > interrupt) let's simply check that no fd is yet assigned to this gsi.
> >
>
> I think Avi asked for this specific feature during review which is the
> reason why its there today. However, I agree that it would probably be
> a good idea to put an upper limit on the number of supported aliases
> that can be registered. Will fix.
>
> Thanks Michael,
>
> -Greg
>
>
Avi, can you elaborate on why do we want to map multiple fds
to the same gsi? I think it's better to allow a 1:1 mapping
only: if many processes want to trigger interrupts they can
all write to the same fd.
--
MST
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
