On Wed, Jul 27, 2016 at 02:52:37AM -0700, Prerna Saxena wrote: > From: Prerna Saxena <prerna.sax...@nutanix.com> > > The set_mem_table command currently does not seek a reply. Hence, there is > no easy way for a remote application to notify to QEMU when it finished > setting up memory, or if there were errors doing so. > > As an example: > (1) Qemu sends a SET_MEM_TABLE to the backend (eg, a vhost-user net > application). SET_MEM_TABLE does not require a reply according to the spec. > (2) Qemu commits the memory to the guest. > (3) Guest issues an I/O operation over a new memory region which was > configured on (1). > (4) The application has not yet remapped the memory, but it sees the I/O > request. > (5) The application cannot satisfy the request because it does not know about > those GPAs. > > While a guaranteed fix would require a protocol extension (committed > separately), > a best-effort workaround for existing applications is to send a GET_FEATURES > message before completing the vhost_user_set_mem_table() call. > Since GET_FEATURES requires a reply, an application that processes vhost-user > messages synchronously would probably have completed the SET_MEM_TABLE before > replying. > > Signed-off-by: Prerna Saxena <prerna.sax...@nutanix.com>
Could you pls reorder patchset so this is 1/2? 1/1 is still under review but I'd like to make sure we have some kind of fix in place for 2.7. > --- > hw/virtio/vhost-user.c | 123 > ++++++++++++++++++++++++++----------------------- > 1 file changed, 65 insertions(+), 58 deletions(-) > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > index 0cdb918..f96607e 100644 > --- a/hw/virtio/vhost-user.c > +++ b/hw/virtio/vhost-user.c > @@ -254,64 +254,6 @@ static int vhost_user_set_log_base(struct vhost_dev > *dev, uint64_t base, > return 0; > } > > -static int vhost_user_set_mem_table(struct vhost_dev *dev, > - struct vhost_memory *mem) > -{ > - int fds[VHOST_MEMORY_MAX_NREGIONS]; > - int i, fd; > - size_t fd_num = 0; > - bool reply_supported = virtio_has_feature(dev->protocol_features, > - VHOST_USER_PROTOCOL_F_REPLY_ACK); > - > - VhostUserMsg msg = { > - .request = VHOST_USER_SET_MEM_TABLE, > - .flags = VHOST_USER_VERSION, > - }; > - > - if (reply_supported) { > - msg.flags |= VHOST_USER_NEED_RESPONSE_MASK; > - } > - > - for (i = 0; i < dev->mem->nregions; ++i) { > - struct vhost_memory_region *reg = dev->mem->regions + i; > - ram_addr_t offset; > - MemoryRegion *mr; > - > - assert((uintptr_t)reg->userspace_addr == reg->userspace_addr); > - mr = memory_region_from_host((void *)(uintptr_t)reg->userspace_addr, > - &offset); > - fd = memory_region_get_fd(mr); > - if (fd > 0) { > - msg.payload.memory.regions[fd_num].userspace_addr = > reg->userspace_addr; > - msg.payload.memory.regions[fd_num].memory_size = > reg->memory_size; > - msg.payload.memory.regions[fd_num].guest_phys_addr = > reg->guest_phys_addr; > - msg.payload.memory.regions[fd_num].mmap_offset = offset; > - assert(fd_num < VHOST_MEMORY_MAX_NREGIONS); > - fds[fd_num++] = fd; > - } > - } > - > - msg.payload.memory.nregions = fd_num; > - > - if (!fd_num) { > - error_report("Failed initializing vhost-user memory map, " > - "consider using -object memory-backend-file share=on"); > - return -1; > - } > - > - msg.size = sizeof(msg.payload.memory.nregions); > - msg.size += sizeof(msg.payload.memory.padding); > - msg.size += fd_num * sizeof(VhostUserMemoryRegion); > - > - vhost_user_write(dev, &msg, fds, fd_num); > - > - if (reply_supported) { > - return process_message_reply(dev, msg.request); > - } > - > - return 0; > -} > - > static int vhost_user_set_vring_addr(struct vhost_dev *dev, > struct vhost_vring_addr *addr) > { > @@ -514,6 +456,71 @@ static int vhost_user_get_features(struct vhost_dev > *dev, uint64_t *features) > return vhost_user_get_u64(dev, VHOST_USER_GET_FEATURES, features); > } > > +static int vhost_user_set_mem_table(struct vhost_dev *dev, > + struct vhost_memory *mem) > +{ > + int fds[VHOST_MEMORY_MAX_NREGIONS]; > + int i, fd; > + size_t fd_num = 0; > + uint64_t features; > + bool reply_supported = virtio_has_feature(dev->protocol_features, > + VHOST_USER_PROTOCOL_F_REPLY_ACK); > + > + VhostUserMsg msg = { > + .request = VHOST_USER_SET_MEM_TABLE, > + .flags = VHOST_USER_VERSION, > + }; > + > + if (reply_supported) { > + msg.flags |= VHOST_USER_NEED_RESPONSE_MASK; > + } > + > + for (i = 0; i < dev->mem->nregions; ++i) { > + struct vhost_memory_region *reg = dev->mem->regions + i; > + ram_addr_t offset; > + MemoryRegion *mr; > + > + assert((uintptr_t)reg->userspace_addr == reg->userspace_addr); > + mr = memory_region_from_host((void *)(uintptr_t)reg->userspace_addr, > + &offset); > + fd = memory_region_get_fd(mr); > + if (fd > 0) { > + msg.payload.memory.regions[fd_num].userspace_addr = > reg->userspace_addr; > + msg.payload.memory.regions[fd_num].memory_size = > reg->memory_size; > + msg.payload.memory.regions[fd_num].guest_phys_addr = > reg->guest_phys_addr; > + msg.payload.memory.regions[fd_num].mmap_offset = offset; > + assert(fd_num < VHOST_MEMORY_MAX_NREGIONS); > + fds[fd_num++] = fd; > + } > + } > + > + msg.payload.memory.nregions = fd_num; > + > + if (!fd_num) { > + error_report("Failed initializing vhost-user memory map, " > + "consider using -object memory-backend-file share=on"); > + return -1; > + } > + > + msg.size = sizeof(msg.payload.memory.nregions); > + msg.size += sizeof(msg.payload.memory.padding); > + msg.size += fd_num * sizeof(VhostUserMemoryRegion); > + > + vhost_user_write(dev, &msg, fds, fd_num); > + > + if (reply_supported) { > + return process_message_reply(dev, msg.request); > + } else { > + /* Note: It is (yet) unknown when the client application has finished > + * remapping the GPA. > + * Attempt to prevent a race by sending a command that requires a > reply. > + */ > + vhost_user_get_features(dev, &features); > + } > + > + return 0; > +} > + > static int vhost_user_set_owner(struct vhost_dev *dev) > { > VhostUserMsg msg = { > -- > 1.8.1.2