Re: [PATCH 5/5] ioeventfd: Introduce KVM_IOEVENTFD_FLAG_SOCKET

Michael S. Tsirkin
2011-07-06T17:57:50Z

On Wed, Jul 06, 2011 at 06:01:46PM +0300, Sasha Levin wrote:
> On Wed, 2011-07-06 at 14:42 +0300, Michael S. Tsirkin wrote:
> > On Wed, Jul 06, 2011 at 07:37:58AM +0300, Sasha Levin wrote:
> > > The new flag allows passing a connected socket instead of an
> > > eventfd to be notified of writes or reads to the specified memory region.
> > >
> > > Instead of signaling an event, On write - the value written to the memory
> > > region is written to the pipe.
> > > On read - a notification of the read is sent to the host, and a response
> > > is expected with the value to be 'read'.
> > >
> > > Using a socket 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.
> > >
> > > Cc: Avi Kivity <avi@redhat.com>
> > > Cc: Ingo Molnar <mingo@elte.hu>
> > > Cc: Marcelo Tosatti <mtosatti@redhat.com>
> > > Cc: Michael S. Tsirkin <mst@redhat.com>
> > > Cc: Pekka Enberg <penberg@kernel.org>
> > > Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> > > ---
> > > Documentation/virtual/kvm/api.txt | 18 ++++-
> > > include/linux/kvm.h | 9 ++
> > > virt/kvm/eventfd.c | 153 ++++++++++++++++++++++++++++++++-----
> > > 3 files changed, 161 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> > > index 317d86a..74f0946 100644
> > > --- a/Documentation/virtual/kvm/api.txt
> > > +++ b/Documentation/virtual/kvm/api.txt
> > > @@ -1330,7 +1330,7 @@ Returns: 0 on success, !0 on error
> > >
> > > This ioctl attaches or detaches an ioeventfd to a legal pio/mmio address
> > > within the guest. A guest write in the registered address will signal the
> > > -provided event instead of triggering an exit.
> > > +provided event or write to the provided socket instead of triggering an exit.
> > >
> > > struct kvm_ioeventfd {
> > > __u64 datamatch;
> > > @@ -1341,6 +1341,13 @@ struct kvm_ioeventfd {
> > > __u8 pad[36];
> > > };
> > >
> > > +struct kvm_ioeventfd_data {
> > > + __u64 data;
> > > + __u64 addr;
> > > + __u32 len;
> > > + __u8 is_write;
> > > +};
> > > +
> > > The following flags are defined:
> > >
> > > #define KVM_IOEVENTFD_FLAG_DATAMATCH (1 << kvm_ioeventfd_flag_nr_datamatch)
> > > @@ -1348,6 +1355,7 @@ The following flags are defined:
> > > #define KVM_IOEVENTFD_FLAG_DEASSIGN (1 << kvm_ioeventfd_flag_nr_deassign)
> > > #define KVM_IOEVENTFD_FLAG_READ (1 << kvm_ioeventfd_flag_nr_read)
> > > #define KVM_IOEVENTFD_FLAG_NOWRITE (1 << kvm_ioeventfd_flag_nr_nowrite)
> > > +#define KVM_IOEVENTFD_FLAG_SOCKET (1 << kvm_ioeventfd_flag_nr_socket)
> > >
> > > If datamatch flag is set, the event will be signaled only if the written value
> > > to the registered address is equal to datamatch in struct kvm_ioeventfd.
> > > @@ -1359,6 +1367,14 @@ passed in datamatch.
> > > If the nowrite flag is set, the event won't be signaled when the specified address
> > > is being written to.
> > >
> > > +If the socket flag is set, fd is expected to be a connected AF_UNIX
> > > +SOCK_SEQPACKET socket.
> >
> > Let's verify that then?
> >
> > > +
> > > + if (p->sock)
> > > + socket_write(p->sock, &data, sizeof(data));
> > > + else
> > > + eventfd_signal(p->eventfd, 1);
> > > +
> > > return 0;
> > > }
> >
> > This still loses the data if socket would block and there's a signal.
> > I think we agreed to use non blocking operations and exit to
> > userspace in that case?
> >
> >
> > >
> > > @@ -534,6 +607,7 @@ ioeventfd_read(struct kvm_io_device *this, gpa_t addr, int len,
> > > void *val)
> > > {
> > > struct _ioeventfd *p = to_ioeventfd(this);
> > > + struct kvm_ioeventfd_data data;
> > >
> > > /* Exit if signaling on reads isn't requested */
> > > if (!p->track_reads)
> > > @@ -542,7 +616,21 @@ ioeventfd_read(struct kvm_io_device *this, gpa_t addr, int len,
> > > if (!ioeventfd_in_range(p, addr, len, val))
> > > return -EOPNOTSUPP;
> > >
> > > - eventfd_signal(p->eventfd, 1);
> > > + data = (struct kvm_ioeventfd_data) {
> > > + .addr = addr,
> > > + .len = len,
> > > + .is_write = 0,
> > > + };
> > > +
> > > + if (p->sock) {
> > > + socket_write(p->sock, &data, sizeof(data));
> > > + socket_read(p->sock, &data, sizeof(data));
> > > + set_val(val, len, data.data);
> >
> > Same here.
>
> The socket_read() here I should leave blocking, and spin on it until I
> read something - right?

I think it's best to exit to userspace.

> > > + } else {
> > > + set_val(val, len, p->datamatch);
> > > + eventfd_signal(p->eventfd, 1);
> > > + }
> > > +
> > > return 0;
> > > }
> > >
> >
> >
>
>
> --
>
> Sasha.