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

Sasha Levin
2011-07-06T15:01:53Z

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?

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

--

Sasha.