Re: [Qemu-devel] vhost-user: broken mem regions

[Nikolay Nikolaev]

On Wed, Jun 25, 2014 at 5:06 PM, Damjan Marion (damarion) <address@hidden> wrote:

On 25 Jun 2014, at 15:53, Michael S. Tsirkin <address@hidden> 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;
>>             }
>>         }
>>