Re: [virtio-comment] Re: [PATCH] Add VIRTIO_RING_F_LARGE_INDIRECT_DESC

[Stefan Hajnoczi <stefanha@redhat.com>]

On Thu, Nov 25, 2021 at 03:58:32PM +0100, Cornelia Huck wrote:
> On Thu, Nov 25 2021, Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> 
> > On Mittwoch, 24. November 2021 18:14:59 CET Stefan Hajnoczi wrote:
> >> On Fri, Nov 19, 2021 at 02:21:12PM +0100, Christian Schoenebeck wrote:
> >> > This new feature flag allows indirect descriptor tables to
> >> > exceed the queue size.
> >> > 
> >> > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> >> > ---
> >> > Stefan noted that there should also be a numeric configuration field
> >> > reflecting a precise limit of indirect descriptors. The question is where
> >> > should that go to exactly? Some devices currently handle this in their
> >> > device specific configuration space. Wouldn't it make sense to handle that
> >> > in the common configuration space instead?
> >> 
> >> It could be added as a read-only struct virtio_pci_common_cfg le16
> >> queue_indirect_size field. The same needs to be done for the other
> >> transports.
> >
> > OK, do I have to make it clear that it is an optional field in 
> > virtio_pci_common_cfg?
> 
> It would need to be something like "this field only exists if
> VIRTIO_RING_F_LARGE_INDIRECT_DESC has been negotiated".
> 
> I'm not quite sure how MMIO expresses optional registers.
> 
> For CCW, it's a bit more complicated: You would need to extend two
> command payloads (vq_config_block and vq_info_block, assuming you want
> to make this read/write); to do that, we will likely want to introduce
> revision 3 (and make the feature bit dependent on revision 3).
> 
> >
> > Are you sure about read-only? QEMU currently reserves the worst case expected 
> > amount of descriptors on stack on every vring entry being processed. If that 
> > field was read-write instead, and if guest driver never uses the amount of 
> > descriptors as advertised by host device being support, guest driver could 
> > simply lower that value and reduce the pressure on host device.
> >
> > So maybe it would make sense making it read-write with the requirement that 
> > guest driver must not increase the value? In the end that write option would 
> > be a soft feature, device could still ignore it and allocate more, it wouldn't 
> > hurt IMO and avoids another feature flag being required for this in future.
> 
> I agree, this should be read/write. It's similar to the queue size,
> where the driver may also configure a lower number.

Sounds good.

Stefan

Attachment: signature.asc
Description: PGP signature