On (Wed) Mar 03 2010 [19:16:09], Michael S. Tsirkin wrote: > vhost needs physical addresses for ring and other queue fields, > so add APIs for these.
Already discussed on IRC, but mentioning here so that it doesn't get lost: > +target_phys_addr_t virtio_queue_get_desc(VirtIODevice *vdev, int n) > +{ > + return vdev->vq[n].vring.desc; > +} > + > +target_phys_addr_t virtio_queue_get_avail(VirtIODevice *vdev, int n) > +{ > + return vdev->vq[n].vring.avail; > +} > + > +target_phys_addr_t virtio_queue_get_used(VirtIODevice *vdev, int n) > +{ > + return vdev->vq[n].vring.used; > +} > + > +target_phys_addr_t virtio_queue_get_ring(VirtIODevice *vdev, int n) > +{ > + return vdev->vq[n].vring.desc; > +} All these functions return the start address of these fields; any better way to name them? eg, just by looking at, eg, 'virtio_queue_get_used()', I'd expect that the function returns the number of used buffers in the ring, not the start address of the used buffers. > +target_phys_addr_t virtio_queue_get_desc_size(VirtIODevice *vdev, int n) > +{ > + return sizeof(VRingDesc) * vdev->vq[n].vring.num; > +} > + > +target_phys_addr_t virtio_queue_get_avail_size(VirtIODevice *vdev, int n) > +{ > + return offsetof(VRingAvail, ring) + > + sizeof(u_int64_t) * vdev->vq[n].vring.num; > +} > + > +target_phys_addr_t virtio_queue_get_used_size(VirtIODevice *vdev, int n) > +{ > + return offsetof(VRingUsed, ring) + > + sizeof(VRingUsedElem) * vdev->vq[n].vring.num; > +} > + > + Extra newline > +target_phys_addr_t virtio_queue_get_ring_size(VirtIODevice *vdev, int n) > +{ > + return vdev->vq[n].vring.used - vdev->vq[n].vring.desc + > + virtio_queue_get_used_size(vdev, n); > +} > + > +uint16_t virtio_queue_last_avail_idx(VirtIODevice *vdev, int n) > +{ > + return vdev->vq[n].last_avail_idx; > +} > + > +void virtio_queue_set_last_avail_idx(VirtIODevice *vdev, int n, uint16_t idx) > +{ > + vdev->vq[n].last_avail_idx = idx; > +} virtio_queue_last_avail_idx() does make sense, but since you have a 'set_last_avail_idx', better name the previous one 'get_..'? > +VirtQueue *virtio_queue(VirtIODevice *vdev, int n) > +{ > + return vdev->vq + n; > +} This really doesn't mean anything; I suggest virtio_queue_get_vq(). > +EventNotifier *virtio_queue_guest_notifier(VirtQueue *vq) > +{ > + return &vq->guest_notifier; > +} > +EventNotifier *virtio_queue_host_notifier(VirtQueue *vq) > +{ > + return &vq->host_notifier; > +} Why drop the 'get_' for these functions? virtio_queue_get_guest_notifier() and virtio_queue_get_host_notifier() might be better. Amit