On Wed, Jan 28, 2015 at 05:07:01PM +0100, Cornelia Huck wrote: > On Thu, 22 Jan 2015 13:06:09 +1100 > David Gibson <[email protected]> wrote: > > > On Thu, Dec 11, 2014 at 02:25:09PM +0100, Cornelia Huck wrote: > > > For virtio-1 devices, we allow a more complex queue layout that doesn't > > > require descriptor table and rings on a physically-contigous memory area: > > > add virtio_queue_set_rings() to allow transports to set this up. > > > > > > Signed-off-by: Cornelia Huck <[email protected]> > > > --- > > > hw/virtio/virtio-mmio.c | 3 +++ > > > hw/virtio/virtio.c | 53 > > > ++++++++++++++++++++++++++++---------------- > > > include/hw/virtio/virtio.h | 3 +++ > > > 3 files changed, 40 insertions(+), 19 deletions(-) > > > > > > diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c > > > index 43b7e02..0c9b63b 100644 > > > --- a/hw/virtio/virtio-mmio.c > > > +++ b/hw/virtio/virtio-mmio.c > > > @@ -244,8 +244,11 @@ static void virtio_mmio_write(void *opaque, hwaddr > > > offset, uint64_t value, > > > case VIRTIO_MMIO_QUEUENUM: > > > DPRINTF("mmio_queue write %d max %d\n", (int)value, > > > VIRTQUEUE_MAX_SIZE); > > > virtio_queue_set_num(vdev, vdev->queue_sel, value); > > > + /* Note: only call this function for legacy devices */ > > > > It's not clear to me if this is an assertion that this *does* only > > call the function for legacy devices or a fixme, that it *should* only > > call the function for legacy devices. > > It's more like a note to whoever takes the virtio-mmio legacy device > code and writes a virtio-1 virtio-mmio device. > > Does > /* Note: this function must only be called for legacy devices */ > make that intention clearer?
Yes, I think that's better.
>
> >
> > > + virtio_queue_update_rings(vdev, vdev->queue_sel);
> > > break;
> > > case VIRTIO_MMIO_QUEUEALIGN:
> > > + /* Note: this is only valid for legacy devices */
> > > virtio_queue_set_align(vdev, vdev->queue_sel, value);
> > > break;
> > > case VIRTIO_MMIO_QUEUEPFN:
>
> (...)
>
> > > /* virt queue functions */
> > > -static void virtqueue_init(VirtQueue *vq)
> > > +void virtio_queue_update_rings(VirtIODevice *vdev, int n)
> >
> > Perhaps something in the name to emphasise that this is only for <v1.0
> > devices?
>
> virtio_queue_legacy_update_rings()? Maybe a bit long...
There aren't many callers, so I think long is ok in this case.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
pgpFnTLmFesIv.pgp
Description: PGP signature
_______________________________________________ Virtualization mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/virtualization
