On Wed, Jun 25, 2014 at 5:06 PM, Damjan Marion (damarion) < damar...@cisco.com> wrote:
> > On 25 Jun 2014, at 15:53, Michael S. Tsirkin <m...@redhat.com> wrote: > > > On Wed, Jun 25, 2014 at 01:45:09PM +0000, Damjan Marion (damarion) wrote: > >> > >> Michael, > >> > >> there is another issue with commited vhost-user code. > > > > I'm answering just this once, but I have a policy against > > answering off-list mail. > > Pls send follow-ups to the list. > > ok, sorry, adding list... > > > > >> If there is bigger mem allocation (i.e. 4G or 6G of RAM) then > >> we have memory gap and then it happens that buffer pointer points to > >> memory which is not mmaped. I already filled bug report: > >> > >> https://bugs.launchpad.net/qemu/+bug/1333688 > > > > FYI I mostly ignore launchpad. > > Because of the unfortunate association with Ubuntu, most bugs > > there are distro-specific. > > > >> Bellow is my proposed change to the code. Two things: > >> - it will require changes on the user side also > > > > why would it? > > format seems unchanged, right? > > yes, but it will happen that multiple regions have same FD so call to > mmap() > should look different, I’m still playing with this on user side... > but then you shoudl somehow accumulate the sizes and send just a single fd, something along these lines. > > > > >> - not sure if qemu_get_ram_fd() is the best way to get FD > > > > Paolo, what do you think? > > > >> Any comments or better idea how to fix this? > >> > >> Thanks, > >> > >> Damjan > >> > >> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > >> index 0df6a93..89fe5c7 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> > >> @@ -183,10 +184,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 +213,14 @@ 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->memory_size; > >> + fds[fd_num++] = fd; > >> } > >> } > >> > >