Re: virtio 4M limit

[Christian Schoenebeck]

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@crudebyte.
> > > 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.html#
> > 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?

> > > 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?

> > 
> > > 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