On Fri, Jun 27, 2014 at 08:02:48AM +0300, Nikolay Nikolaev wrote: > > > > On Fri, Jun 27, 2014 at 12:01 AM, Damjan Marion <damar...@cisco.com> wrote: > > Old code was affected by memory gaps which resulted in buffer pointers > pointing to address outside of the mapped regions. > > Here we are introducing following changes: > - new function qemu_get_ram_block_host_ptr() returns host pointer > to the ram block, it is needed to calculate offset of specific > region in the host memory > - new field mmap_offset is added to the VhostUserMemoryRegion. It > contains offset where specific region starts in the mapped memory. > As there is stil no wider adoption of vhost-user agreement was made > that we will not bump version number due to this change > - other fileds in VhostUserMemoryRegion struct are not changed, as > they are all needed for usermode app implementation > - region data is not taken from ram_list.blocks anymore, instead we > use region data which is alredy calculated for use in vhost-net > - Now multiple regions can have same FD and user applicaton can call > mmap() multiple times with the same FD but with different offset > (user needs to take care for offset page alignment) > > Signed-off-by: Damjan Marion <damar...@cisco.com> > --- > docs/specs/vhost-user.txt | 7 ++++--- > exec.c | 7 +++++++ > hw/virtio/vhost-user.c | 23 ++++++++++++++--------- > include/exec/ram_addr.h | 1 + > 4 files changed, 26 insertions(+), 12 deletions(-) > > diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt > index 2641390..6abb697 100644 > --- a/docs/specs/vhost-user.txt > +++ b/docs/specs/vhost-user.txt > @@ -78,13 +78,14 @@ Depending on the request type, payload can be: > Padding: 32-bit > > A region is: > - --------------------------------------- > - | guest address | size | user address | > - --------------------------------------- > + ----------------------------------------------------- > + | guest address | size | user address | mmap offset | > + ----------------------------------------------------- > > Guest address: a 64-bit guest address of the region > Size: a 64-bit size > User address: a 64-bit user address > + mmmap offset: 64-bit offset where region starts in the mapped memory > > > In QEMU the vhost-user message is implemented with the following struct: > diff --git a/exec.c b/exec.c > index c849405..a94c583 100644 > --- a/exec.c > +++ b/exec.c > @@ -1456,6 +1456,13 @@ int qemu_get_ram_fd(ram_addr_t addr) > return block->fd; > } > > +void *qemu_get_ram_block_host_ptr(ram_addr_t addr) > +{ > + RAMBlock *block = qemu_get_ram_block(addr); > + > + return block->host; > +} > + > /* Return a host pointer to ram allocated with qemu_ram_alloc. > With the exception of the softmmu code in this file, this should > only be used for local memory (e.g. video ram) that the device owns, > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > index 0df6a93..38e5806 100644 > --- a/hw/virtio/vhost-user.c > +++ b/hw/virtio/vhost-user.c > @@ -14,6 +14,7 @@ > #include "sysemu/kvm.h" > #include "qemu/error-report.h" > #include "qemu/sockets.h" > +#include "exec/ram_addr.h" > > #include <fcntl.h> > #include <unistd.h> > @@ -47,6 +48,7 @@ typedef struct VhostUserMemoryRegion { > uint64_t guest_phys_addr; > uint64_t memory_size; > uint64_t userspace_addr; > + uint64_t mmap_offset; > } VhostUserMemoryRegion; > > typedef struct VhostUserMemory { > @@ -183,10 +185,10 @@ static int vhost_user_call(struct vhost_dev *dev, > unsigned long int request, > { > VhostUserMsg msg; > VhostUserRequest msg_request; > - RAMBlock *block = 0; > struct vhost_vring_file *file = 0; > int need_reply = 0; > int fds[VHOST_MEMORY_MAX_NREGIONS]; > + int i, fd; > size_t fd_num = 0; > > assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER); > @@ -212,14 +214,17 @@ static int vhost_user_call(struct vhost_dev *dev, > unsigned long int request, > break; > > case VHOST_SET_MEM_TABLE: > - QTAILQ_FOREACH(block, &ram_list.blocks, next) > - { > - if (block->fd > 0) { > - msg.memory.regions[fd_num].userspace_addr = > - (uintptr_t) block->host; > - msg.memory.regions[fd_num].memory_size = block->length; > - msg.memory.regions[fd_num].guest_phys_addr = block-> > offset; > - fds[fd_num++] = block->fd; > + for (i = 0; i < dev->mem->nregions; ++i) { > + struct vhost_memory_region *reg = dev->mem->regions + i; > + fd = qemu_get_ram_fd(reg->guest_phys_addr); > + if (fd > 0) { > + msg.memory.regions[fd_num].userspace_addr = reg-> > userspace_addr; > + msg.memory.regions[fd_num].memory_size = reg-> > memory_size; > + msg.memory.regions[fd_num].guest_phys_addr = reg-> > guest_phys_addr; > + msg.memory.regions[fd_num].mmap_offset = reg-> > userspace_addr - > + (uintptr_t) qemu_get_ram_block_host_ptr(reg-> > guest_phys_addr); > + assert(fd_num < VHOST_MEMORY_MAX_NREGIONS); > + fds[fd_num++] = fd; > } > } > > diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h > index 55ca676..e9eb831 100644 > --- a/include/exec/ram_addr.h > +++ b/include/exec/ram_addr.h > @@ -29,6 +29,7 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void > *host, > MemoryRegion *mr); > ram_addr_t qemu_ram_alloc(ram_addr_t size, MemoryRegion *mr); > int qemu_get_ram_fd(ram_addr_t addr); > +void *qemu_get_ram_block_host_ptr(ram_addr_t addr); > void *qemu_get_ram_ptr(ram_addr_t addr); > void qemu_ram_free(ram_addr_t addr); > void qemu_ram_free_from_ptr(ram_addr_t addr); > -- > 1.9.1 > > > > Looks OK to me. Thanks Damjan. > > Acked-By: Nikolay Nikolaev <n.nikol...@virtualopensystems.com> >
Nikolay, please don't send HTML mail to list. Plain text only. Thanks, -- MST