Re: virtio 4M limit

[Christian Schoenebeck]

On Montag, 4. Oktober 2021 21:59:18 CEST Michael S. Tsirkin wrote:
> On Mon, Oct 04, 2021 at 12:44:21PM +0200, Christian Schoenebeck wrote:
> > On Sonntag, 3. Oktober 2021 22:27:03 CEST Michael S. Tsirkin wrote:
> > > On Sun, Oct 03, 2021 at 08:14:55PM +0200, Christian Schoenebeck wrote:
> > > > On Freitag, 1. Oktober 2021 13:21:23 CEST Christian Schoenebeck wrote:
> > > > > Hi Michael,
> > > > > 
> > > > > while testing the following kernel patches I realized there is
> > > > > currently
> > > > > a
> > > > > size limitation of 4 MB with virtio on QEMU side:
> > > > > https://lore.kernel.org/netdev/cover.1632327421.git.linux_oss@crudeb
> > > > > yte.
> > > > > com/
> > > > > 
> > > > > So with those kernel patches applied I can mount 9pfs on Linux guest
> > > > > with
> > > > > the 9p 'msize' (maximum message size) option with a value of up to
> > > > > 4186112
> > > > > successfully. If I try to go higher with 'msize' then the system
> > > > > would
> > > > > hang
> > > > > 
> > > > > with the following QEMU error:
> > > > >   qemu-system-x86_64: virtio: too many write descriptors in indirect
> > > > >   table
> > > > > 
> > > > > Which apparently is due to the amount of scatter gather lists on
> > > > > QEMU
> > > > > virtio side currently being hard coded to 1024 (i.e. multiplied by
> > > > > 4k
> > > > > page size =>> >
> > > > > 
> > > > > 4 MB):
> > > > >   ./include/hw/virtio/virtio.h:
> > > > >   #define VIRTQUEUE_MAX_SIZE 1024
> > > > > 
> > > > > Is that hard coded limit carved into stone for some reason or would
> > > > > it
> > > > > be OK if I change that into a runtime variable?
> > > > 
> > > > After reviewing the code and protocol specs, it seems that this value
> > > > is
> > > > simply too small. I will therefore send a patch suggsting to raise
> > > > this
> > > > value to 32768, as this is the maximum possible value according to the
> > > > virtio specs.
> > > > 
> > > > https://docs.oasis-open.org/virtio/virtio/v1.1/cs01/virtio-v1.1-cs01.h
> > > > tml#
> > > > x1-240006
> > > 
> > > I think it's too aggressive to change it for all devices.
> > > Pls find a way to only have it affect 9pfs.
> > 
> > So basically I should rather introduce a variable that would be used at
> > most places instead of using the macro VIRTQUEUE_MAX_SIZE?
> 
> I guess so.

Good, because I just sent out a v2 series minutes ago.

> > > > > If that would be Ok, maybe something similar that I did with those
> > > > > kernel
> > > > > patches, i.e. retaining 1024 as an initial default value and if
> > > > > indicated
> > > > > from guest side that more is needed, increasing the SG list amount
> > > > > subsequently according to whatever is needed by guest?
> > > > 
> > > > Further changes are probably not necessary.
> > > > 
> > > > The only thing I have spotted that probably should be changed is that
> > > > at
> > > > some few locations, a local array is allocated on the stack with
> > > > VIRTQUEUE_MAX_SIZE as array size, e.g.:
> > > > 
> > > > static void *virtqueue_split_pop(VirtQueue *vq, size_t sz)
> > > > {
> > > > 
> > > >     ...
> > > >     hwaddr addr[VIRTQUEUE_MAX_SIZE];
> > > >     struct iovec iov[VIRTQUEUE_MAX_SIZE];
> > > >     ...
> > > > 
> > > > }
> > 
> > What about these allocations on the stack? Is it Ok to disregard this as
> > theoretical issue for now and just retain them on the stack, just with the
> > runtime variable instead of macro as array size?
> 
> I think it's not a big deal ... why do you think it is? Are we running
> out of stack?

No no. :) That was just a theoretical consideration for who knows what kind of 
platform and usage. I have preserved it on stack in today's v2.
 
> > > > > And as I am not too familiar with the virtio protocol, is that
> > > > > current
> > > > > limit already visible to guest side? Because obviously it would make
> > > > > sense if I change my kernel patches so that they automatically limit
> > > > > to
> > > > > whatever QEMU supports instead of causing a hang.
> > > > 
> > > > Apparently the value of VIRTQUEUE_MAX_SIZE (the maximum amount of
> > > > scatter
> > > > gather lists or the maximum queue size ever possible) is not visible
> > > > to
> > > > guest.
> > > > 
> > > > I thought about making a hack to make the guest Linux kernel aware
> > > > whether
> > > > host side has the old limit of 1024 or rather the correct value 32768,
> > > > but
> > > > probably not worth it.
> > > > 
> > > > Best regards,
> > > > Christian Schoenebeck