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

[Sasha Levin <levinsasha928@gmail.com>]

On Wed, 2011-07-06 at 15:39 +0300, Avi Kivity wrote:
> On 07/06/2011 07:37 AM, 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.
> >
> >
> >
> > @@ -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);
> > +	} else {
> > +		set_val(val, len, p->datamatch);
> > +		eventfd_signal(p->eventfd, 1);
> > +	}
> > +
> >   	return 0;
> >   }
> 
> If there are two reads on the same ioeventfd range, then the responses 
> can mix up.  Need to make sure we get the right response.
> 
> Note that a mutex on the ioeventfd structure is insufficient, since the 
> same socket may be used for multiple ranges.
> 
> One way out is to require that sockets not be shared among vcpus (there 
> can be only one outstanding read per vcpu).  It seems heavy handed though.
> 

What about something as follows:

This requires an addition of a mutex to struct ioeventfd.

1. When adding a new ioeventfd, scan exiting ioeventfds (we already do
it anyway) and check whether another ioeventfd is using the socket
already.

2. If the existing ioeventfd doesn't have a mutex assigned, create a new
mutex and assign it to both ioeventfds.

3. If the existing ioeventfd already has a mutex assigned, copy it to
the new ioeventfd.

4. When removing an ioeventfd, do everything the other way around :)

This mutex can be used to lock the write/read pair.

-- 

Sasha.