From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH] ioeventfd: Introduce KVM_IOEVENTFD_FLAG_PIPE Date: Mon, 04 Jul 2011 13:45:07 +0300 Message-ID: <4E1199B3.2010507@redhat.com> References: <1309712689-4290-1-git-send-email-levinsasha928@gmail.com> <20110704103207.GA11386@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Sasha Levin , kvm@vger.kernel.org, Ingo Molnar , Marcelo Tosatti , Pekka Enberg To: "Michael S. Tsirkin" Return-path: Received: from mx1.redhat.com ([209.132.183.28]:18428 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754712Ab1GDKpU (ORCPT ); Mon, 4 Jul 2011 06:45:20 -0400 In-Reply-To: <20110704103207.GA11386@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 07/04/2011 01:32 PM, Michael S. Tsirkin wrote: > On Sun, Jul 03, 2011 at 08:04:49PM +0300, Sasha Levin wrote: > > The new flag allows passing a write side of a pipe instead of an > > eventfd to be notified of writes to the specified memory region. > > > > Instead of signaling an event, the value written to the memory region > > is written to the pipe. > > > > Using a pipe instead of an eventfd is usefull when any value can be > > written to the memory region but we're interested in recieving the > > actual value instead of just a notification. > > > > A simple example for practical use is the serial port. we are not > > interested in an exit every time a char is written to the port, but > > we do need to know what was written so we could handle it on the guest. > > Looking at this example, how would you handle a pipe full condition? > We can't buffer unlimited amount of data in the host. Stall. > > +static ssize_t kernel_write(struct file *file, const char *buf, size_t count, > > + loff_t pos) > > +{ > > + mm_segment_t old_fs; > > + ssize_t res; > > + > > + old_fs = get_fs(); > > + set_fs(get_ds()); > > + /* The cast to a user pointer is valid due to the set_fs() */ > > Interesting. Is buf really always a user pointer? > Why don't we tag it __user then? It's not a user pointer, this is just to get the tools happy. set_fs() makes it safe. > > + res = vfs_write(file, (const char __user *)buf, count,&pos); > > If pipe is non-blocking, or if we get a signal, > this might fail or return a value< len. > Data will be lost then, won't it? Yes. Need a loop-until-buffer-exhausted-or-error. Error reporting is an interesting question. Typically we have KVM_RUN return the error, but if we use this facility to run something in a separate process, this can cause the device process crash to cause the guest to crash. > > - p->eventfd = eventfd; > > + > > + if (args->flags& KVM_IOEVENTFD_FLAG_PIPE) > > + p->pipe = fget(args->fd); > > This really needs to check that the fd is a pipe. > Otherwise you can do weird things like pass in > the kvm device fd itself. Eww, reference loop. Good catch. We should allow unix domain sockets as well. In fact, for read/write support, we need this to be a unix domain socket. -- error compiling committee.c: too many arguments to function