Re: [PATCH 00/10] PV-IO v3

Avi Kivity
2007-08-22T11:47:12Z

Rusty Russell wrote:
> On Wed, 2007-08-22 at 02:26 -0700, Dor Laor wrote:
>
>> Actually while playing with virtio for kvm Avi saw that and recommended
>> to do the following:
>> struct desc_pages
>> {
>> /* Page of descriptors. */
>> union {
>> struct virtio_desc desc[NUM_DESCS];
>> char pad1[PAGE_SIZE];
>> };
>>
>> /* Next page: how we tell other side what buffers are available. */
>> union {
>> struct {
>> unsigned int avail_idx;
>> unsigned int available[NUM_DESCS];
>> };
>> char pad2[PAGE_SIZE];
>> };
>>
>> /* Third page: how other side tells us what's used. */
>> union {
>> struct {
>> unsigned int used_idx;
>> struct virtio_used used[NUM_DESCS];
>> };
>> char pad3[PAGE_SIZE];
>> };
>> };
>>
>> It saves useless pointer arithmetic.
>>
>
> Hi Dor!
>
> Please consider moving the "used" field into the descriptor (maybe as a
> ptr for cache reasons, 'cept I'd really like to trim descriptor size).
> That makes the avail and used rings identical, plus the current model
> means we can't trust the used length if we don't trust the other side
> (this is one of my FIXMEs).
>
> Of course, we could go further and break the fixed structure: there's no
> reason for the first and second part to be on separate pages, nor for
> the third to be consecutive. But there is no need until be have an
> untrusted demonstration...
>
>

Another thing that occurs to me is that alignment should be explicit to
64 bits, so that mixed 32/64 bit guest/hosts can be used.

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