On Wed, Jan 28, 2015 at 05:07:01PM +0100, Cornelia Huck wrote:
> On Thu, 22 Jan 2015 13:06:09 +1100
> David Gibson <david@xxxxxxxxxxxxxxxxxxxxx> 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 <cornelia.huck@xxxxxxxxxx>
> > > ---
> > > 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
Attachment:
pgpPIkuBiYydW.pgp
Description: PGP signature
_______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization
