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

Sasha Levin
2011-07-06T12:58:59Z

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.