Re: [PATCH 6/6] virtio ring helper

Avi Kivity
2007-09-23T10:05:24Z

Rusty Russell wrote:
> On Thu, 2007-09-20 at 14:43 +0200, Avi Kivity wrote:
>
>> Rusty Russell wrote:
>>
>>> These helper routines supply most of the virtqueue_ops for hypervisors
>>> which want to use a ring for virtio. Unlike the previous lguest
>>> implementation:
>>>
>>> 3) The page numbers are always 64 bit (PAE anyone?)
>>>
>>>
>> 32 bits of page numbers give 44 bits of physical address on x86. That's
>> 16TB per guest. Admittedly it's smaller on a VAX.
>>
>
> I like to feel that I make these mistakes to ensure others are paying
> attention. However, it does mean that I can just put an address in
> there and increase the length field to 32 bits. Much rejoicing.
>
>

Why are we sending page numbers anyway? See below.

>> I don't like the chaining and would prefer the descriptor to refer to an
>> array of page descriptors. However I can't complain since this isn't
>> common code.
>>
>
> The intent is for kvm to use it. I'll certainly consider your patches,
> although I suspect that managing descriptor pages will make things ugly
> enough to cause you to reconsider.
>

How about, instead:

struct vring_desc {
__u64 address;
__u32 length;
__u32 flags;
};

Where one of the flags is VRING_DESC_INDIRECT, which means that the
memory within (address, length) is a bunch of descriptors instead of raw
data.

This has the following nice properties:
- no dependency on page size, for archs where the page size definition
is muddy
- no indirection for ethernet rx on Linux, even with jumbograms (a
buffer can span pages in one descriptor, as long as they're physically
contiguous)
- reward OSes that can do contiguous I/O (with a 32-bit length, too)
- keeps the interface simple at the expense of the implementation
(address vs. page+offset)
- no nasty 16-bit fields, which some archs don't like
- be free! cast off your chains!

--
error compiling committee.c: too many arguments to function