[Xen-devel] Re: [kvm-devel] [PATCH RFC 1/3] virtio infrastructure

[Rusty Russell <rusty@xxxxxxxxxxxxxxx>]

On Sat, 2007-06-02 at 09:30 +0300, Avi Kivity wrote:
> Rusty Russell wrote:
> > + * virtio_ops - virtio abstraction layer
> > + * @add_outbuf: prepare to send data to the other end:
> > + * vdev: the virtio_device
> > + * sg: the description of the buffer(s).
> > + * num: the size of the sg array.
> > + * used: the length sent (set once sending is done).
> > + *      Returns an identifier or an error.
> > + * @add_inbuf: prepare to receive data from the other end:
> > + * vdev: the virtio_device
> > + * sg: the description of the buffer(s).
> > + * num: the size of the sg array.
> > + * used: the length sent (set once data received).
> > + *      Returns an identifier or an error (eg. -ENOSPC).
> >   
> 
> Instead of 'used', how about a completion callback (with associated data
> pointer)?  A new helper, virtio_complete(), would call the callback for
> all completed requests.  It would eliminate all the tedious scanning
> used to match the identifier.

Hi Avi,

        There were several considerations here.  My first was that the drivers
look much more like normal devices than getting a callback for every
buffer.   Secondly, "used" batches much more nicely than a completion.
Finally, it's also something you really want to know, so the driver
doesn't have to zero its inbufs (an untrusted other side says it sends
you 1500 bytes but actually sent nothing, and now you spray kernel
memory out the NIC).

        I also considered some scheme like:

        struct virtio_used_info
        {
                unsigned long len;
                void *next_token;
        };
        ...
        unsigned long (*add_outbuf)(struct virtio_device *vdev,
                                    const struct scatterlist sg[],
                                    unsigned int num,
                                    void *token,
                                    struct virtio_used_info *used_info);

So the "used" becomes a "used/next" pair and you can just walk the
linked list.  But I wasn't convinced that walking the buffers is going
to be a performance issue (tho the net driver puts them in a continuous
array for cache friendliness as a nod to this concern).

> It would also be nice to support a bit of non-buffer data, like a set of
> bitflags.

        I expect this might be necessary, but it wasn't so far.  The non-buffer
data tends to go in sg[0]: the block driver works this way, and the
network driver will for GSO.  Of course, a specialized virtio_ops
backend might well take this and put the info somewhere else.

        I also considered a separate "publish"/"examine" interface for things
which aren't really messages, but again, haven't needed it yet.

Thanks!
Rusty.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel