On 27/11/2014 11:15, Peter Lieven wrote: > vring_map causes a huge overhead by calling memory_region_find everytime. > the vring_map is executed already on vring_setup and there is also the memory > region referenced. > > Signed-off-by: Peter Lieven <p...@kamp.de>
vring_map/unmap is going to disappear and be replaced by address_space_map/unmap, so for now I'd rather keep it... Paolo > --- > hw/virtio/dataplane/vring.c | 51 > +++++-------------------------------------- > 1 file changed, 5 insertions(+), 46 deletions(-) > > diff --git a/hw/virtio/dataplane/vring.c b/hw/virtio/dataplane/vring.c > index 61f6d83..cfd484f 100644 > --- a/hw/virtio/dataplane/vring.c > +++ b/hw/virtio/dataplane/vring.c > @@ -53,15 +53,6 @@ out: > return NULL; > } > > -static void vring_unmap(void *buffer, bool is_write) > -{ > - ram_addr_t addr; > - MemoryRegion *mr; > - > - mr = qemu_ram_addr_from_host(buffer, &addr); > - memory_region_unref(mr); > -} > - > /* Map the guest's vring to host memory */ > bool vring_setup(Vring *vring, VirtIODevice *vdev, int n) > { > @@ -160,7 +151,6 @@ static int get_desc(Vring *vring, VirtQueueElement *elem, > unsigned *num; > struct iovec *iov; > hwaddr *addr; > - MemoryRegion *mr; > > if (desc->flags & VRING_DESC_F_WRITE) { > num = &elem->in_num; > @@ -186,13 +176,10 @@ static int get_desc(Vring *vring, VirtQueueElement > *elem, > } > > /* TODO handle non-contiguous memory across region boundaries */ > - iov->iov_base = vring_map(&mr, desc->addr, desc->len, > - desc->flags & VRING_DESC_F_WRITE); > - if (!iov->iov_base) { > - error_report("Failed to map descriptor addr %#" PRIx64 " len %u", > - (uint64_t)desc->addr, desc->len); > + if (desc->addr + desc->len > memory_region_size(vring->mr)) { > return -EFAULT; > } > + iov->iov_base = (void*) memory_region_get_ram_ptr(vring->mr) + > desc->addr; > > /* The MemoryRegion is looked up again and unref'ed later, leave the > * ref in place. */ > @@ -230,22 +217,14 @@ static int get_indirect(Vring *vring, VirtQueueElement > *elem, > > do { > struct vring_desc *desc_ptr; > - MemoryRegion *mr; > > /* Translate indirect descriptor */ > - desc_ptr = vring_map(&mr, > - indirect->addr + found * sizeof(desc), > - sizeof(desc), false); > - if (!desc_ptr) { > - error_report("Failed to map indirect descriptor " > - "addr %#" PRIx64 " len %zu", > - (uint64_t)indirect->addr + found * sizeof(desc), > - sizeof(desc)); > - vring->broken = true; > + if (indirect->addr + (found + 1) * sizeof(desc) > > memory_region_size(vring->mr)) { > return -EFAULT; > } > + desc_ptr = (void*) memory_region_get_ram_ptr(vring->mr) + > + indirect->addr + found * sizeof(desc); > desc = *desc_ptr; > - memory_region_unref(mr); > > /* Ensure descriptor has been loaded before accessing fields */ > barrier(); /* read_barrier_depends(); */ > @@ -273,23 +252,6 @@ static int get_indirect(Vring *vring, VirtQueueElement > *elem, > return 0; > } > > -static void vring_unmap_element(VirtQueueElement *elem) > -{ > - int i; > - > - /* This assumes that the iovecs, if changed, are never moved past > - * the end of the valid area. This is true if iovec manipulations > - * are done with iov_discard_front and iov_discard_back. > - */ > - for (i = 0; i < elem->out_num; i++) { > - vring_unmap(elem->out_sg[i].iov_base, false); > - } > - > - for (i = 0; i < elem->in_num; i++) { > - vring_unmap(elem->in_sg[i].iov_base, true); > - } > -} > - > /* This looks in the virtqueue and for the first available buffer, and > converts > * it to an iovec for convenient access. Since descriptors consist of some > * number of output then some number of input descriptors, it's actually two > @@ -399,7 +361,6 @@ out: > if (ret == -EFAULT) { > vring->broken = true; > } > - vring_unmap_element(elem); > return ret; > } > > @@ -413,8 +374,6 @@ void vring_push(Vring *vring, VirtQueueElement *elem, int > len) > unsigned int head = elem->index; > uint16_t new; > > - vring_unmap_element(elem); > - > /* Don't touch vring if a fatal error occurred */ > if (vring->broken) { > return; >