"Michael S. Tsirkin" <m...@redhat.com> writes:
> On Mon, Jan 21, 2013 at 10:22:03PM +1030, Rusty Russell wrote:
>> Rusty Russell <ru...@rustcorp.com.au> writes:
>> > "Michael S. Tsirkin" <m...@redhat.com> writes:
>> >>> +                iov->iov[iov->i].iov_base = (__force __user void *)addr;
>> >>> +                iov->iov[iov->i].iov_len = desc.len;
>> >>
>> >> The following comment from the previous version still applies:
>> >>   > This looks like it won't do the right thing if desc.len spans multiple
>> >>   > ranges. I don't know if this happens in practice but this is something
>> >>   > vhost supports ATM.
>> >> in otgher words, we might need to split a single desc to multiple
>> >> iov entries.
>> >
>> > Ah, separate offsets for consecutive ranges, right.  I'd prefer to say
>> > "don't do that", but qemu is rarely sane.  I'll fix it.
>> 
>> Actually, you make the same assumption for vhost, with your use of
>> getuser and putuser for accessing the ring.
>
> There's no requirement that ring is mapped directly into guest
> memory. If a ring is not contigious qemu can allocate
> it's own virtuall contigious rings and copy data back and forth.

True, but it's the guest which allocates the ring.  If QEMU sets up a
guest with a offset-discontiguous mapping, vhost would be unreliable
today.

>> The complexity and cost of handling this is significant,
>
> Why is it? Just add a while loop, increment desc.addr
> and decrement desc.len.

Indirect handling now needs to re-check the boundaries for every
descriptor it fetches, and handle the case where a single descriptor
overlaps the boundary.

The ability to verify the indirect descriptor table all at once makes
the code fast and neat.

>From your other mail:

> It seems it could happen if e.g. one region is ROM and another one is
> RAM.  This means it won't be used for the ring (which is R/W) but could
> be for the data.

That case wouldn't be so bad, I think, since it's a simple loop.

I'll create some patches and see if it's too ugly to live...

Thanks,
Rusty.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Reply via email to