[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCHv4 05/12] virtio: add APIs for queue fields
From: |
Amit Shah |
Subject: |
[Qemu-devel] Re: [PATCHv4 05/12] virtio: add APIs for queue fields |
Date: |
Fri, 5 Mar 2010 17:40:18 +0530 |
User-agent: |
Mutt/1.5.19 (2009-01-05) |
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
[Qemu-devel] [PATCHv4 03/12] notifier: event notifier implementation, Michael S. Tsirkin, 2010/03/03
[Qemu-devel] [PATCHv4 06/12] virtio: add set_status callback, Michael S. Tsirkin, 2010/03/03