Re: [virtio-comment] Re: [PATCH v2 0/2] Add VIRTIO_RING_F_LARGE_INDIRECT_DESC

[Cornelia Huck <cohuck@redhat.com>]

On Tue, Nov 30 2021, Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:

> On Dienstag, 30. November 2021 14:48:50 CET Cornelia Huck wrote:
>> On Tue, Nov 30 2021, Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:

>> > And what about the intended availability of this new virtio_pci_common_cfg
>> > field "queue_indirect_size", should it be optional per se, independent of
>> > the virtio version or rather a mandatory field in upcoming virtio
>> > version?
>> We should keep that field depending upon the feature bit IMHO.
>
> So you are suggesting the existence of "queue_indirect_size" field to be 
> dependent on feature flag VIRTIO_RING_F_LARGE_INDIRECT_DESC instead of being 
> dependent on the virtio version.

Yes.

> Stefan, would that also suit your intended 2nd use case of lowering the max. 
> descriptor count *below* Queue Size? It would be somewhat different from what 
> I suggested here in patch 2 [which was min(QueueSize, queue_indirect_size) if 
> VIRTIO_RING_F_LARGE_INDIRECT_DESC not set], but it could be fine as well. 

I think that a configurable value for the descriptor count needs to
depend on the feature bit as well.

> Maybe the name VIRTIO_RING_F_LARGE_INDIRECT_DESC might then a bit misleading 
> though, because the flag would also be set for forcing small limits.

VIRTIO_RING_F_CONFIGURABLE_INDIRECT_DESC ?

>
> Another issue I just realized: there is also an ambiguity in this v2 what the 
> maximum descriptor count actually relates to. Should it be
>
> 1. max. indirect descriptor count per indirect descriptor table
>
> or
>
> 2. max. indirect descriptor count per vring slot (i.e. the sum from multiple 
> indirect descriptor tables within the same message)
>
> Case 2 applies to QEMU's implementation right now AFAICS. The max. possible 
> bulk transfer size is lower in case 2 accordingly.