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

[Sasha Levin <levinsasha928@gmail.com>]

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.