Introduce virtqueue_get_avail_bytes()
cover Introduce virtqueue_get_avail_bytes()
The current virtqueue_avail_bytes() is a weird API: it's oddly-named:
doesn't tell us what the API is going to do, and also suits just one use-case (that in virtio-net.c).
Introduce virtqueue_get_avail_bytes(), which returns the number of bytes in the vq available for input as well as output. virtqueue_avail_bytes() is made a wrapper around this new function for now. It should be deprecated soon, though.
Doing this will also help with the virtio-rng patch where a VirtQueueElement is popped only to find out what its size is. With this series applied, the popping (and the subsequent save/load of state for migration) isn't necessary.
The virtio-serial-bus code becomes better too, that's patch 3 here.
Please apply,
Amit Shah (3):
virtio: use unsigned int for counting bytes in vq
virtio: Introduce virtqueue_get_avail_bytes()
virtio-serial-bus: let chardev know the exact number of bytes
requested
hw/virtio-serial-bus.c | 11 +++-------- hw/virtio.c | 30 ++++++++++++++++++++++-------- hw/virtio.h | 5 ++++- 3 files changed, 29 insertions(+), 17 deletions(-)
Signature · 1 line
1.7.7.6
PATCH 1/3 virtio: use unsigned int for counting bytes in vq
The virtqueue_avail_bytes() function counts bytes in an int. Use an unsigned int instead.
--- hw/virtio.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
hw/virtio.c +1 -1
@@ -338,7 +338,7 @@ static unsigned virtqueue_next_desc(target_phys_addr_t desc_pa,int virtqueue_avail_bytes (VirtQueue *vq,int in_bytes,int out_bytes) {unsigned int idx; -int total_bufs, in_total, out_total; +unsigned int total_bufs, in_total, out_total; idx = vq->last_avail_idx;
Signature · 1 line
1.7.7.6
PATCH 2/3 virtio: Introduce virtqueue_get_avail_bytes()
In reply to Amit ShahThe current virtqueue_avail_bytes() is oddly named, and checks if a particular number of bytes are available in a vq. A better API is to fetch the number of bytes available in the vq, and let the caller do what's interesting with the numbers.
Introduce virtqueue_get_avail_bytes(), which returns the number of bytes for buffers marked for both, in as well as out. virtqueue_avail_bytes() is made a wrapper over this new function.
--- hw/virtio.c | 28 +++++++++++++++++++++------- hw/virtio.h | 5 ++++- 2 files changed, 25 insertions(+), 8 deletions(-)
hw/virtio.c +21 -7
@@ -335,7 +335,8 @@ static unsigned virtqueue_next_desc(target_phys_addr_t desc_pa,return next; } -int virtqueue_avail_bytes (VirtQueue *vq,int in_bytes,int out_bytes) +void virtqueue_get_avail_bytes (VirtQueue *vq,unsigned int *in_bytes, +unsigned int *out_bytes) {unsigned int idx;unsigned int total_bufs, in_total, out_total;
@@ -380,13 +381,9 @@ int virtqueue_avail_bytes(VirtQueue *vq, int in_bytes, int out_bytes) }if (vring_desc_flags (desc_pa, i) &VRING_DESC_F_WRITE ) { -if (in_bytes >0 && - (in_total +=vring_desc_len (desc_pa, i)) >= in_bytes) -return 1 ; + in_total +=vring_desc_len (desc_pa, i); }else { -if (out_bytes >0 && - (out_total +=vring_desc_len (desc_pa, i)) >= out_bytes) -return 1 ; + out_total +=vring_desc_len (desc_pa, i); } }while ((i =virtqueue_next_desc (desc_pa, i, max)) != max);
@@ -395,7 +392,24 @@ int virtqueue_avail_bytes(VirtQueue *vq, int in_bytes, int out_bytes)else total_bufs++; } +if (in_bytes) { + *in_bytes = in_total; + } +if (out_bytes) { + *out_bytes = out_total; + } +} +int virtqueue_avail_bytes (VirtQueue *vq,unsigned int in_bytes, +unsigned int out_bytes) +{ +unsigned int in_total, out_total; + +virtqueue_get_avail_bytes (vq, &in_total, &out_total); +if ((in_bytes && in_bytes < in_total) + || (out_bytes && out_bytes < out_total)) { +return 1 ; + }return 0 ; }
hw/virtio.h +4 -1
@@ -148,7 +148,10 @@ void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem,void virtqueue_map_sg (struct iovec *sg,target_phys_addr_t *addr,size_t num_sg,int is_write);int virtqueue_pop (VirtQueue *vq,VirtQueueElement *elem); -int virtqueue_avail_bytes (VirtQueue *vq,int in_bytes,int out_bytes); +int virtqueue_avail_bytes (VirtQueue *vq,unsigned int in_bytes, +unsigned int out_bytes); +void virtqueue_get_avail_bytes (VirtQueue *vq,unsigned int *in_bytes, +unsigned int *out_bytes);void virtio_notify (VirtIODevice *vdev,VirtQueue *vq);
Signature · 1 line
1.7.7.6
PATCH 3/3 virtio-serial-bus: let chardev know the exact number of bytes requested
In reply to Amit ShahUsing the virtqueue_avail_bytes() function had an unnecessarily crippling effect on the number of bytes needed by the guest as reported to the chardev layer in the can_read() callback.
Using the new virtqueue_get_avail_bytes() function will let us advertise the exact number of bytes we can send to the guest.
--- hw/virtio-serial-bus.c | 11 +++-------- 1 files changed, 3 insertions(+), 8 deletions(-)
hw/virtio-serial-bus.c +3 -8
@@ -287,6 +287,7 @@ ssize_t virtio_serial_write(VirtIOSerialPort *port, const uint8_t *buf,size_t virtio_serial_guest_ready (VirtIOSerialPort *port) {VirtQueue *vq = port->ivq; +unsigned int bytes; if (!virtio_queue_ready(vq) || !(port->vser->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK) ||
@@ -296,14 +297,8 @@ size_t virtio_serial_guest_ready(VirtIOSerialPort *port)if (use_multiport (port->vser) && !port->guest_connected) {return 0 ; } - -if (virtqueue_avail_bytes (vq,4096 ,0 )) { -return 4096 ; - } -if (virtqueue_avail_bytes (vq,1 ,0 )) { -return 1 ; - } -return 0 ; +virtqueue_get_avail_bytes (vq, &bytes,NULL ); +return bytes; }static void flush_queued_data_bh (void *opaque)
Signature · 1 line
1.7.7.6
Quoted 9 lines · Amit Shah
The current
virtqueue_avail_bytes()is a weird API: it's oddly-named:
doesn't tell us what the API is going to do, and also suits just one use-case (that invirtio-net.c).Introduce
virtqueue_get_avail_bytes(), which returns the number of bytes in the vq available for input as well as output.virtqueue_avail_bytes()is made a wrapper around this new function for now. It should be deprecated soon, though.
Quoted 6 lines
Doing this will also help with the virtio-rng patch where a VirtQueueElement is popped only to find out what its size is. With this series applied, the popping (and the subsequent save/load of state for migration) isn't necessary.
The virtio-serial-bus code becomes better too, that's patch 3 here.
Ping?
Amit
Quoted 9 lines · Amit Shah
The current
virtqueue_avail_bytes()is a weird API: it's oddly-named:
doesn't tell us what the API is going to do, and also suits just one use-case (that invirtio-net.c).Introduce
virtqueue_get_avail_bytes(), which returns the number of bytes in the vq available for input as well as output.virtqueue_avail_bytes()is made a wrapper around this new function for now. It should be deprecated soon, though.
Quoted 8 lines
Doing this will also help with the virtio-rng patch where a VirtQueueElement is popped only to find out what its size is. With this series applied, the popping (and the subsequent save/load of state for migration) isn't necessary.
The virtio-serial-bus code becomes better too, that's patch 3 here.
Please apply,
Ping again.
Amit