[Qemu-devel] [PATCH v2] vhost-user: fix VHOST_USER_SET_MEM_TABLE

[Qemu-devel] [PATCH v2] vhost-user: fix VHOST_USER_SET_MEM_TABLE

[Damjan Marion]

Old code was affected by memory gaps which
resulted in buffer pointers pointing to
address outside of the mapped regions.

Signed-off-by: Damjan Marion <damarion@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..c108d07 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 | shared mem offset |
+   -----------------------------------------------------------
 
    Guest address: a 64-bit guest address of the region
    Size: a 64-bit size
    User address: a 64-bit user address
+   Shared mem offset: 64-bit offset where region is located in the shared 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..b9d7baa 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 shm_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,16 +214,19 @@ 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].shm_offset = reg->userspace_addr -
+                    (uint64_t) qemu_get_ram_block_host_ptr(reg->guest_phys_addr);
+                fds[fd_num++] = fd;
             }
         }
+        assert(fd_num <= VHOST_MEMORY_MAX_NREGIONS);
 
         msg.memory.nregions = fd_num;
 
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

Re: [Qemu-devel] [PATCH v2] vhost-user: fix VHOST_USER_SET_MEM_TABLE

[Michael S. Tsirkin]

On Thu, Jun 26, 2014 at 12:22:59PM +0200, Damjan Marion wrote:
> Old code was affected by memory gaps which
> resulted in buffer pointers pointing to
> address outside of the mapped regions.
> 
> Signed-off-by: Damjan Marion <damarion@cisco.com>
> ---

changelog?

does not look like all comments have been addressed.

>  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..c108d07 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 | shared mem offset |
> +   -----------------------------------------------------------
>  
>     Guest address: a 64-bit guest address of the region
>     Size: a 64-bit size
>     User address: a 64-bit user address
> +   Shared mem offset: 64-bit offset where region is located in the shared 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..b9d7baa 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 shm_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,16 +214,19 @@ 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].shm_offset = reg->userspace_addr -
> +                    (uint64_t) qemu_get_ram_block_host_ptr(reg->guest_phys_addr);
> +                fds[fd_num++] = fd;
>              }
>          }
> +        assert(fd_num <= VHOST_MEMORY_MAX_NREGIONS);
>  
>          msg.memory.nregions = fd_num;
>  
> 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

Re: [Qemu-devel] [PATCH v2] vhost-user: fix VHOST_USER_SET_MEM_TABLE

[Damjan Marion (damarion)]

On 26 Jun 2014, at 13:30, Michael S. Tsirkin <mst@redhat.com> wrote:

> On Thu, Jun 26, 2014 at 12:22:59PM +0200, Damjan Marion wrote:
>> Old code was affected by memory gaps which
>> resulted in buffer pointers pointing to
>> address outside of the mapped regions.
>> 
>> Signed-off-by: Damjan Marion <damarion@cisco.com>
>> ---
> 
> changelog?

Ok

> 
> does not look like all comments have been addressed.

Like?

> 
>> 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..c108d07 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 | shared mem offset |
>> +   -----------------------------------------------------------
>> 
>>    Guest address: a 64-bit guest address of the region
>>    Size: a 64-bit size
>>    User address: a 64-bit user address
>> +   Shared mem offset: 64-bit offset where region is located in the shared 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..b9d7baa 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 shm_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,16 +214,19 @@ 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].shm_offset = reg->userspace_addr -
>> +                    (uint64_t) qemu_get_ram_block_host_ptr(reg->guest_phys_addr);
>> +                fds[fd_num++] = fd;
>>             }
>>         }
>> +        assert(fd_num <= VHOST_MEMORY_MAX_NREGIONS);
>> 
>>         msg.memory.nregions = fd_num;
>> 
>> 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

Re: [Qemu-devel] [PATCH v2] vhost-user: fix VHOST_USER_SET_MEM_TABLE

[Michael S. Tsirkin]

On Thu, Jun 26, 2014 at 11:35:13AM +0000, Damjan Marion (damarion) wrote:
> 
> On 26 Jun 2014, at 13:30, Michael S. Tsirkin <mst@redhat.com> wrote:
> 
> > On Thu, Jun 26, 2014 at 12:22:59PM +0200, Damjan Marion wrote:
> >> Old code was affected by memory gaps which
> >> resulted in buffer pointers pointing to
> >> address outside of the mapped regions.
> >> 
> >> Signed-off-by: Damjan Marion <damarion@cisco.com>
> >> ---
> > 
> > changelog?
> 
> Ok
> 
> > 
> > does not look like all comments have been addressed.
> 
> Like?

I'm sorry you will have to look for them on the list.

> > 
> >> 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..c108d07 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 | shared mem offset |
> >> +   -----------------------------------------------------------
> >> 
> >>    Guest address: a 64-bit guest address of the region
> >>    Size: a 64-bit size
> >>    User address: a 64-bit user address
> >> +   Shared mem offset: 64-bit offset where region is located in the shared 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..b9d7baa 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 shm_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,16 +214,19 @@ 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].shm_offset = reg->userspace_addr -
> >> +                    (uint64_t) qemu_get_ram_block_host_ptr(reg->guest_phys_addr);
> >> +                fds[fd_num++] = fd;
> >>             }
> >>         }
> >> +        assert(fd_num <= VHOST_MEMORY_MAX_NREGIONS);
> >> 
> >>         msg.memory.nregions = fd_num;
> >> 
> >> 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

Re: [Qemu-devel] [PATCH v2] vhost-user: fix VHOST_USER_SET_MEM_TABLE

[Damjan Marion (damarion)]

On 26 Jun 2014, at 13:37, Michael S. Tsirkin <mst@redhat.com> wrote:

> On Thu, Jun 26, 2014 at 11:35:13AM +0000, Damjan Marion (damarion) wrote:
>> 
>> On 26 Jun 2014, at 13:30, Michael S. Tsirkin <mst@redhat.com> wrote:
>> 
>>> On Thu, Jun 26, 2014 at 12:22:59PM +0200, Damjan Marion wrote:
>>>> Old code was affected by memory gaps which
>>>> resulted in buffer pointers pointing to
>>>> address outside of the mapped regions.
>>>> 
>>>> Signed-off-by: Damjan Marion <damarion@cisco.com>
>>>> ---
>>> 
>>> changelog?
>> 
>> Ok
>> 
>>> 
>>> does not look like all comments have been addressed.
>> 
>> Like?
> 
> I'm sorry you will have to look for them on the list.

I replied to all comments on the v1 patch thread. 
- ua is needed, i gave you example
- version bump, 3 of us (Nikolay) agreed that it is not needed before 2.1 is released
- ram_addr_t - I changed to uint64_t

What I missed is Nikolay’s proposal on the yesterday’s thread to rename shm_offset to mmap_offset.

Can you be so kind and confirm that we are on the same page before i send patch v3.


> 
>>> 
>>>> 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..c108d07 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 | shared mem offset |
>>>> +   -----------------------------------------------------------
>>>> 
>>>>   Guest address: a 64-bit guest address of the region
>>>>   Size: a 64-bit size
>>>>   User address: a 64-bit user address
>>>> +   Shared mem offset: 64-bit offset where region is located in the shared 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..b9d7baa 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 shm_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,16 +214,19 @@ 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].shm_offset = reg->userspace_addr -
>>>> +                    (uint64_t) qemu_get_ram_block_host_ptr(reg->guest_phys_addr);
>>>> +                fds[fd_num++] = fd;
>>>>            }
>>>>        }
>>>> +        assert(fd_num <= VHOST_MEMORY_MAX_NREGIONS);
>>>> 
>>>>        msg.memory.nregions = fd_num;
>>>> 
>>>> 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
> 

Re: [Qemu-devel] [PATCH v2] vhost-user: fix VHOST_USER_SET_MEM_TABLE

[Michael S. Tsirkin]

On Thu, Jun 26, 2014 at 12:22:59PM +0200, Damjan Marion wrote:
> Old code was affected by memory gaps which
> resulted in buffer pointers pointing to
> address outside of the mapped regions.
> 
> Signed-off-by: Damjan Marion <damarion@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..c108d07 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 | shared mem offset |
> +   -----------------------------------------------------------
>  
>     Guest address: a 64-bit guest address of the region
>     Size: a 64-bit size
>     User address: a 64-bit user address
> +   Shared mem offset: 64-bit offset where region is located in the shared 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..b9d7baa 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 shm_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,16 +214,19 @@ 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].shm_offset = reg->userspace_addr -
> +                    (uint64_t) qemu_get_ram_block_host_ptr(reg->guest_phys_addr);

this will give a warning on 32 bit.
you should use uintptr_t which is same size as integer.

> +                fds[fd_num++] = fd;
>              }
>          }
> +        assert(fd_num <= VHOST_MEMORY_MAX_NREGIONS);
>  
>          msg.memory.nregions = fd_num;
>  
> 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

Re: [Qemu-devel] [PATCH v2] vhost-user: fix VHOST_USER_SET_MEM_TABLE

[Michael S. Tsirkin]

On Thu, Jun 26, 2014 at 11:51:51AM +0000, Damjan Marion (damarion) wrote:
> 
> On 26 Jun 2014, at 13:37, Michael S. Tsirkin <mst@redhat.com> wrote:
> 
> > On Thu, Jun 26, 2014 at 11:35:13AM +0000, Damjan Marion (damarion) wrote:
> >> 
> >> On 26 Jun 2014, at 13:30, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> 
> >>> On Thu, Jun 26, 2014 at 12:22:59PM +0200, Damjan Marion wrote:
> >>>> Old code was affected by memory gaps which
> >>>> resulted in buffer pointers pointing to
> >>>> address outside of the mapped regions.
> >>>> 
> >>>> Signed-off-by: Damjan Marion <damarion@cisco.com>
> >>>> ---
> >>> 
> >>> changelog?
> >> 
> >> Ok
> >> 
> >>> 
> >>> does not look like all comments have been addressed.
> >> 
> >> Like?
> > 
> > I'm sorry you will have to look for them on the list.
> 
> I replied to all comments on the v1 patch thread. 

I'd like to clarify, comments should be addressed
in the patch itself, it is not enough to address
them on hte mailing list thread.
Except when they are completely silly, you should
address them in commit log discussing design
or in code comments.


> - ua is needed, i gave you example
> - version bump, 3 of us (Nikolay) agreed that it is not needed before 2.1 is released
> - ram_addr_t - I changed to uint64_t
> 
> What I missed is Nikolay’s proposal on the yesterday’s thread to rename shm_offset to mmap_offset.
> 
> Can you be so kind and confirm that we are on the same page before i send patch v3.
> 
> 
> > 
> >>> 
> >>>> 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..c108d07 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 | shared mem offset |
> >>>> +   -----------------------------------------------------------
> >>>> 
> >>>>   Guest address: a 64-bit guest address of the region
> >>>>   Size: a 64-bit size
> >>>>   User address: a 64-bit user address
> >>>> +   Shared mem offset: 64-bit offset where region is located in the shared 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..b9d7baa 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 shm_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,16 +214,19 @@ 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].shm_offset = reg->userspace_addr -
> >>>> +                    (uint64_t) qemu_get_ram_block_host_ptr(reg->guest_phys_addr);
> >>>> +                fds[fd_num++] = fd;
> >>>>            }
> >>>>        }
> >>>> +        assert(fd_num <= VHOST_MEMORY_MAX_NREGIONS);
> >>>> 
> >>>>        msg.memory.nregions = fd_num;
> >>>> 
> >>>> 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
> > 

Re: [Qemu-devel] [PATCH v2] vhost-user: fix VHOST_USER_SET_MEM_TABLE

[Nikolay Nikolaev]

On Thu, Jun 26, 2014 at 1:22 PM, Damjan Marion <damarion@cisco.com> wrote:
> Old code was affected by memory gaps which
> resulted in buffer pointers pointing to
> address outside of the mapped regions.
>
> Signed-off-by: Damjan Marion <damarion@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..c108d07 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 | shared mem offset |
> +   -----------------------------------------------------------
>
>     Guest address: a 64-bit guest address of the region
>     Size: a 64-bit size
>     User address: a 64-bit user address
> +   Shared mem offset: 64-bit offset where region is located in the shared 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..b9d7baa 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 shm_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,16 +214,19 @@ 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].shm_offset = reg->userspace_addr -
> +                    (uint64_t) qemu_get_ram_block_host_ptr(reg->guest_phys_addr);
can you please move the assert here
assert(fd_num < VHOST_MEMORY_MAX_NREGIONS);
lets not put the data if it will be at the wrong place.

> +                fds[fd_num++] = fd;
>              }
>          }
> +        assert(fd_num <= VHOST_MEMORY_MAX_NREGIONS);
>
>          msg.memory.nregions = fd_num;
>
> 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
>