Re: [Qemu-devel] [PATCH v1] vhost-user: fix not send all hugepage files to vhost-user
On 2015/2/2 7:29, Paolo Bonzini wrote: > > > On 17/12/2014 07:02, haifeng@huawei.com wrote: >> From: linhaifeng >> >> If we create VM with two or more numa nodes qemu will create two >> or more hugepage files but qemu only send one hugepage file fd >> to vhost-user when VM's memory size is 2G and with two numa nodes. >> >> Signed-off-by: linhaifeng > > The bug is in vhost_dev_assign_memory. It doesn't check that the file > descriptor matches when merging regions. Michael, does the merging > trigger in practice? Can we just eliminate it? > > Paolo > qemu-system-x86_64 -name vm1 -enable-kvm -smp 2 -m 2048 -object memory-backend-file,id=mem1,size=1024M,mem-path=/dev/hugepages,share=on -object memory-backend-file,id=mem2,size=1024M,mem-path=/dev/hugepages,share=on -numa node,memdev=mem1 -numa node,memdev=mem2 -chardev socket,id=chr0,path=/var/run/vhost-user/tap10 -netdev type=vhost-user,id=net0,chardev=chr0,vhostforce -device virtio-net-pci,netdev=net0,mac=00:00:00:00:00:01,csum=off,gso=off,guest_tso4=off,guest_tso6=off,guest_ecn=off -drive file=/mnt/sdc/linhf/suse11_sp3_64_0 -vnc :0 hugepage file fd=8 addr=0x7f1ea720 size=1073741824 hugepage file fd=9 addr=0x7f1ee720 size=1073741824 qemu-system-x86_64: -netdev type=vhost-user,id=net0,chardev=chr0,vhostforce: chardev "chr0" went up WARNING: Image format was not specified for '/mnt/sdc/linhf/suse11_sp3_64_0' and probing guessed raw. Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted. Specify the 'raw' format explicitly to remove the restrictions. reg->userspace_addr=0x7f1ea72c ram_addr=0xc fd=8 reg->userspace_addr=0x7f1e9ee0 ram_addr=0x8000 fd=-1 reg->userspace_addr=0x7f1ea720 ram_addr=0x0 fd=8 It seems like the second region's address is invalid(not in the hugepage's area). so we lost this region.
Re: [Qemu-devel] [PATCH v1] vhost-user: fix not send all hugepage files to vhost-user
On 04/02/2015 14:52, Michael S. Tsirkin wrote: > I'm not sure: does memory core ever give us two adjacent > RAM segments that we *can* merge? I don't think so. Memory core merges ranges already if the following holds: - same memory region - same dirty logging mode - same readonly behavior (ignore reads, trap reads as MMIO, read/write) - r2 end address is the same as r1 start address - r2's offset in the memory region is equal to r1's plus the size of r1 Paolo > If yes it would trigger, and extra memory slots slow down lookups > linearly so they aren't free.
Re: [Qemu-devel] [PATCH v1] vhost-user: fix not send all hugepage files to vhost-user
On Mon, Feb 02, 2015 at 12:29:47AM +0100, Paolo Bonzini wrote:
>
>
> On 17/12/2014 07:02, haifeng@huawei.com wrote:
> > From: linhaifeng
> >
> > If we create VM with two or more numa nodes qemu will create two
> > or more hugepage files but qemu only send one hugepage file fd
> > to vhost-user when VM's memory size is 2G and with two numa nodes.
> >
> > Signed-off-by: linhaifeng
>
> The bug is in vhost_dev_assign_memory. It doesn't check that the file
> descriptor matches when merging regions. Michael, does the merging
> trigger in practice? Can we just eliminate it?
>
> Paolo
I'm not sure: does memory core ever give us two adjacent
RAM segments that we *can* merge?
If yes it would trigger, and extra memory slots slow down lookups
linearly so they aren't free.
> > ---
> > hw/virtio/vhost-user.c | 78
> > ++---
> > hw/virtio/vhost.c | 13
> > linux-headers/linux/vhost.h | 7
> > 3 files changed, 73 insertions(+), 25 deletions(-)
> >
> > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> > index aefe0bb..439cbba 100644
> > --- a/hw/virtio/vhost-user.c
> > +++ b/hw/virtio/vhost-user.c
> > @@ -24,6 +24,10 @@
> > #include
> >
> > #define VHOST_MEMORY_MAX_NREGIONS8
> > +/* FIXME: same as the max number of numa node?*/
> > +#define HUGEPAGE_MAX_FILES 8
> > +
> > +#define RAM_SHARED (1 << 1)
> >
> > typedef enum VhostUserRequest {
> > VHOST_USER_NONE = 0,
> > @@ -41,14 +45,15 @@ typedef enum VhostUserRequest {
> > VHOST_USER_SET_VRING_KICK = 12,
> > VHOST_USER_SET_VRING_CALL = 13,
> > VHOST_USER_SET_VRING_ERR = 14,
> > -VHOST_USER_MAX
> > +VHOST_USER_MMAP_HUGEPAGE_FILE = 15,
> > +VHOST_USER_UNMAP_HUGEPAGE_FILE = 16,
> > +VHOST_USER_MAX,
> > } VhostUserRequest;
> >
> > typedef struct VhostUserMemoryRegion {
> > uint64_t guest_phys_addr;
> > uint64_t memory_size;
> > uint64_t userspace_addr;
> > -uint64_t mmap_offset;
> > } VhostUserMemoryRegion;
> >
> > typedef struct VhostUserMemory {
> > @@ -57,6 +62,16 @@ typedef struct VhostUserMemory {
> > VhostUserMemoryRegion regions[VHOST_MEMORY_MAX_NREGIONS];
> > } VhostUserMemory;
> >
> > +typedef struct HugepageMemoryInfo {
> > +uint64_t base_addr;
> > +uint64_t size;
> > +}HugeMemInfo;
> > +
> > +typedef struct HugepageInfo {
> > +uint32_t num;
> > +HugeMemInfo files[HUGEPAGE_MAX_FILES];
> > +}HugepageInfo;
> > +
> > typedef struct VhostUserMsg {
> > VhostUserRequest request;
> >
> > @@ -71,6 +86,7 @@ typedef struct VhostUserMsg {
> > struct vhost_vring_state state;
> > struct vhost_vring_addr addr;
> > VhostUserMemory memory;
> > +HugepageInfo huge_info;
> > };
> > } QEMU_PACKED VhostUserMsg;
> >
> > @@ -104,7 +120,9 @@ static unsigned long int
> > ioctl_to_vhost_user_request[VHOST_USER_MAX] = {
> > VHOST_GET_VRING_BASE, /* VHOST_USER_GET_VRING_BASE */
> > VHOST_SET_VRING_KICK, /* VHOST_USER_SET_VRING_KICK */
> > VHOST_SET_VRING_CALL, /* VHOST_USER_SET_VRING_CALL */
> > -VHOST_SET_VRING_ERR /* VHOST_USER_SET_VRING_ERR */
> > +VHOST_SET_VRING_ERR,/* VHOST_USER_SET_VRING_ERR */
> > +VHOST_MMAP_HUGEPAGE_FILE, /* VHOST_USER_MMAP_HUGEPAGE_FILE */
> > +VHOST_UNMAP_HUGEPAGE_FILE, /* VHOST_USER_UNMAP_HUGEPAGE_FILE */
> > };
> >
> > static VhostUserRequest vhost_user_request_translate(unsigned long int
> > request)
> > @@ -190,6 +208,7 @@ static int vhost_user_call(struct vhost_dev *dev,
> > unsigned long int request,
> > int fds[VHOST_MEMORY_MAX_NREGIONS];
> > int i, fd;
> > size_t fd_num = 0;
> > +RAMBlock *block;
> >
> > assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
> >
> > @@ -213,37 +232,46 @@ static int vhost_user_call(struct vhost_dev *dev,
> > unsigned long int request,
> > case VHOST_RESET_OWNER:
> > break;
> >
> > -case VHOST_SET_MEM_TABLE:
> > -for (i = 0; i < dev->mem->nregions; ++i) {
> > -struct vhost_memory_region *reg = dev->mem->regions + i;
> > -ram_addr_t ram_addr;
> > +case VHOST_MMAP_HUGEPAGE_FILE:
> > +qemu_mutex_lock_ramlist();
> >
> > -assert((uintptr_t)reg->userspace_addr == reg->userspace_addr);
> > -qemu_ram_addr_from_host((void
> > *)(uintptr_t)reg->userspace_addr, &ram_addr);
> > -fd = qemu_get_ram_fd(ram_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(ram_addr);
> > -ass
Re: [Qemu-devel] [PATCH v1] vhost-user: fix not send all hugepage files to vhost-user
On 2015/2/2 7:29, Paolo Bonzini wrote:
>
>
> On 17/12/2014 07:02, haifeng@huawei.com wrote:
>> From: linhaifeng
>>
>> If we create VM with two or more numa nodes qemu will create two
>> or more hugepage files but qemu only send one hugepage file fd
>> to vhost-user when VM's memory size is 2G and with two numa nodes.
>>
>> Signed-off-by: linhaifeng
>
> The bug is in vhost_dev_assign_memory. It doesn't check that the file
> descriptor matches when merging regions. Michael, does the merging
> trigger in practice? Can we just eliminate it?
>
> Paolo
>
zan!
Have this bug fixed?
>> ---
>> hw/virtio/vhost-user.c | 78
>> ++---
>> hw/virtio/vhost.c | 13
>> linux-headers/linux/vhost.h | 7
>> 3 files changed, 73 insertions(+), 25 deletions(-)
>>
>> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
>> index aefe0bb..439cbba 100644
>> --- a/hw/virtio/vhost-user.c
>> +++ b/hw/virtio/vhost-user.c
>> @@ -24,6 +24,10 @@
>> #include
>>
>> #define VHOST_MEMORY_MAX_NREGIONS8
>> +/* FIXME: same as the max number of numa node?*/
>> +#define HUGEPAGE_MAX_FILES 8
>> +
>> +#define RAM_SHARED (1 << 1)
>>
>> typedef enum VhostUserRequest {
>> VHOST_USER_NONE = 0,
>> @@ -41,14 +45,15 @@ typedef enum VhostUserRequest {
>> VHOST_USER_SET_VRING_KICK = 12,
>> VHOST_USER_SET_VRING_CALL = 13,
>> VHOST_USER_SET_VRING_ERR = 14,
>> -VHOST_USER_MAX
>> +VHOST_USER_MMAP_HUGEPAGE_FILE = 15,
>> +VHOST_USER_UNMAP_HUGEPAGE_FILE = 16,
>> +VHOST_USER_MAX,
>> } VhostUserRequest;
>>
>> typedef struct VhostUserMemoryRegion {
>> uint64_t guest_phys_addr;
>> uint64_t memory_size;
>> uint64_t userspace_addr;
>> -uint64_t mmap_offset;
>> } VhostUserMemoryRegion;
>>
>> typedef struct VhostUserMemory {
>> @@ -57,6 +62,16 @@ typedef struct VhostUserMemory {
>> VhostUserMemoryRegion regions[VHOST_MEMORY_MAX_NREGIONS];
>> } VhostUserMemory;
>>
>> +typedef struct HugepageMemoryInfo {
>> +uint64_t base_addr;
>> +uint64_t size;
>> +}HugeMemInfo;
>> +
>> +typedef struct HugepageInfo {
>> +uint32_t num;
>> +HugeMemInfo files[HUGEPAGE_MAX_FILES];
>> +}HugepageInfo;
>> +
>> typedef struct VhostUserMsg {
>> VhostUserRequest request;
>>
>> @@ -71,6 +86,7 @@ typedef struct VhostUserMsg {
>> struct vhost_vring_state state;
>> struct vhost_vring_addr addr;
>> VhostUserMemory memory;
>> +HugepageInfo huge_info;
>> };
>> } QEMU_PACKED VhostUserMsg;
>>
>> @@ -104,7 +120,9 @@ static unsigned long int
>> ioctl_to_vhost_user_request[VHOST_USER_MAX] = {
>> VHOST_GET_VRING_BASE, /* VHOST_USER_GET_VRING_BASE */
>> VHOST_SET_VRING_KICK, /* VHOST_USER_SET_VRING_KICK */
>> VHOST_SET_VRING_CALL, /* VHOST_USER_SET_VRING_CALL */
>> -VHOST_SET_VRING_ERR /* VHOST_USER_SET_VRING_ERR */
>> +VHOST_SET_VRING_ERR,/* VHOST_USER_SET_VRING_ERR */
>> +VHOST_MMAP_HUGEPAGE_FILE, /* VHOST_USER_MMAP_HUGEPAGE_FILE */
>> +VHOST_UNMAP_HUGEPAGE_FILE, /* VHOST_USER_UNMAP_HUGEPAGE_FILE */
>> };
>>
>> static VhostUserRequest vhost_user_request_translate(unsigned long int
>> request)
>> @@ -190,6 +208,7 @@ static int vhost_user_call(struct vhost_dev *dev,
>> unsigned long int request,
>> int fds[VHOST_MEMORY_MAX_NREGIONS];
>> int i, fd;
>> size_t fd_num = 0;
>> +RAMBlock *block;
>>
>> assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
>>
>> @@ -213,37 +232,46 @@ static int vhost_user_call(struct vhost_dev *dev,
>> unsigned long int request,
>> case VHOST_RESET_OWNER:
>> break;
>>
>> -case VHOST_SET_MEM_TABLE:
>> -for (i = 0; i < dev->mem->nregions; ++i) {
>> -struct vhost_memory_region *reg = dev->mem->regions + i;
>> -ram_addr_t ram_addr;
>> +case VHOST_MMAP_HUGEPAGE_FILE:
>> +qemu_mutex_lock_ramlist();
>>
>> -assert((uintptr_t)reg->userspace_addr == reg->userspace_addr);
>> -qemu_ram_addr_from_host((void *)(uintptr_t)reg->userspace_addr,
>> &ram_addr);
>> -fd = qemu_get_ram_fd(ram_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(ram_addr);
>> -assert(fd_num < VHOST_MEMORY_MAX_NREGIONS);
>> -fds[fd_num++] = fd;
>> +/* Get hugepage file informations */
>> +QTAILQ_FOREACH(block, &ram_list.blocks, next) {
>> +if (block->flags & RAM_SHARED && block->fd > 0) {
>> +msg.huge_info.files[fd_num
Re: [Qemu-devel] [PATCH v1] vhost-user: fix not send all hugepage files to vhost-user
On 17/12/2014 07:02, haifeng@huawei.com wrote:
> From: linhaifeng
>
> If we create VM with two or more numa nodes qemu will create two
> or more hugepage files but qemu only send one hugepage file fd
> to vhost-user when VM's memory size is 2G and with two numa nodes.
>
> Signed-off-by: linhaifeng
The bug is in vhost_dev_assign_memory. It doesn't check that the file
descriptor matches when merging regions. Michael, does the merging
trigger in practice? Can we just eliminate it?
Paolo
> ---
> hw/virtio/vhost-user.c | 78
> ++---
> hw/virtio/vhost.c | 13
> linux-headers/linux/vhost.h | 7
> 3 files changed, 73 insertions(+), 25 deletions(-)
>
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index aefe0bb..439cbba 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -24,6 +24,10 @@
> #include
>
> #define VHOST_MEMORY_MAX_NREGIONS8
> +/* FIXME: same as the max number of numa node?*/
> +#define HUGEPAGE_MAX_FILES 8
> +
> +#define RAM_SHARED (1 << 1)
>
> typedef enum VhostUserRequest {
> VHOST_USER_NONE = 0,
> @@ -41,14 +45,15 @@ typedef enum VhostUserRequest {
> VHOST_USER_SET_VRING_KICK = 12,
> VHOST_USER_SET_VRING_CALL = 13,
> VHOST_USER_SET_VRING_ERR = 14,
> -VHOST_USER_MAX
> +VHOST_USER_MMAP_HUGEPAGE_FILE = 15,
> +VHOST_USER_UNMAP_HUGEPAGE_FILE = 16,
> +VHOST_USER_MAX,
> } VhostUserRequest;
>
> typedef struct VhostUserMemoryRegion {
> uint64_t guest_phys_addr;
> uint64_t memory_size;
> uint64_t userspace_addr;
> -uint64_t mmap_offset;
> } VhostUserMemoryRegion;
>
> typedef struct VhostUserMemory {
> @@ -57,6 +62,16 @@ typedef struct VhostUserMemory {
> VhostUserMemoryRegion regions[VHOST_MEMORY_MAX_NREGIONS];
> } VhostUserMemory;
>
> +typedef struct HugepageMemoryInfo {
> +uint64_t base_addr;
> +uint64_t size;
> +}HugeMemInfo;
> +
> +typedef struct HugepageInfo {
> +uint32_t num;
> +HugeMemInfo files[HUGEPAGE_MAX_FILES];
> +}HugepageInfo;
> +
> typedef struct VhostUserMsg {
> VhostUserRequest request;
>
> @@ -71,6 +86,7 @@ typedef struct VhostUserMsg {
> struct vhost_vring_state state;
> struct vhost_vring_addr addr;
> VhostUserMemory memory;
> +HugepageInfo huge_info;
> };
> } QEMU_PACKED VhostUserMsg;
>
> @@ -104,7 +120,9 @@ static unsigned long int
> ioctl_to_vhost_user_request[VHOST_USER_MAX] = {
> VHOST_GET_VRING_BASE, /* VHOST_USER_GET_VRING_BASE */
> VHOST_SET_VRING_KICK, /* VHOST_USER_SET_VRING_KICK */
> VHOST_SET_VRING_CALL, /* VHOST_USER_SET_VRING_CALL */
> -VHOST_SET_VRING_ERR /* VHOST_USER_SET_VRING_ERR */
> +VHOST_SET_VRING_ERR,/* VHOST_USER_SET_VRING_ERR */
> +VHOST_MMAP_HUGEPAGE_FILE, /* VHOST_USER_MMAP_HUGEPAGE_FILE */
> +VHOST_UNMAP_HUGEPAGE_FILE, /* VHOST_USER_UNMAP_HUGEPAGE_FILE */
> };
>
> static VhostUserRequest vhost_user_request_translate(unsigned long int
> request)
> @@ -190,6 +208,7 @@ static int vhost_user_call(struct vhost_dev *dev,
> unsigned long int request,
> int fds[VHOST_MEMORY_MAX_NREGIONS];
> int i, fd;
> size_t fd_num = 0;
> +RAMBlock *block;
>
> assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
>
> @@ -213,37 +232,46 @@ static int vhost_user_call(struct vhost_dev *dev,
> unsigned long int request,
> case VHOST_RESET_OWNER:
> break;
>
> -case VHOST_SET_MEM_TABLE:
> -for (i = 0; i < dev->mem->nregions; ++i) {
> -struct vhost_memory_region *reg = dev->mem->regions + i;
> -ram_addr_t ram_addr;
> +case VHOST_MMAP_HUGEPAGE_FILE:
> +qemu_mutex_lock_ramlist();
>
> -assert((uintptr_t)reg->userspace_addr == reg->userspace_addr);
> -qemu_ram_addr_from_host((void *)(uintptr_t)reg->userspace_addr,
> &ram_addr);
> -fd = qemu_get_ram_fd(ram_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(ram_addr);
> -assert(fd_num < VHOST_MEMORY_MAX_NREGIONS);
> -fds[fd_num++] = fd;
> +/* Get hugepage file informations */
> +QTAILQ_FOREACH(block, &ram_list.blocks, next) {
> +if (block->flags & RAM_SHARED && block->fd > 0) {
> +msg.huge_info.files[fd_num].size = block->length;
> +msg.huge_info.files[fd_num].base_addr = block->host;
> +fds[fd_num++] = block->fd;
> }
> }
> +msg.huge_info.num = fd_num;
>
Re: [Qemu-devel] [PATCH v1] vhost-user: fix not send all hugepage files to vhost-user
On 2015/1/29 18:51, Michael S. Tsirkin wrote:
> On Thu, Jan 29, 2015 at 11:58:08AM +0800, Linhaifeng wrote:
>> Hi,Michael S.Tsirkin
>>
>> The vhost-user device will not work if there are two numa nodes in VM.
>>
>> Should we fix this bug or ignore it ?
>
> I suggest we fix this bug.
> I saw that you responded to self so I assume you will
> send v2 with the modification you listed.
> Did I get it right?
>
> Also, pls Cc qemu-stable on bugfixes.
>
> Thanks!
>
Hi,Michael S.Tsirkin
Yes,in v2 i want to write vhost-user-test.c to show how to mmap and how to
calculate address
then you and other one can review it.But i don't know how to compile and run
the test.
Is there any document to show it?
BTW.I know you wrote virtio_net driver and vhost. Is there any document to
introduce the principle
or how the virtio and vhost work?
>> On 2014/12/18 13:06, Linhaifeng wrote:
>>>
>>>
>>> On 2014/12/17 14:02, haifeng@huawei.com wrote:
From: linhaifeng
If we create VM with two or more numa nodes qemu will create two
or more hugepage files but qemu only send one hugepage file fd
to vhost-user when VM's memory size is 2G and with two numa nodes.
Signed-off-by: linhaifeng
---
hw/virtio/vhost-user.c | 78
++---
hw/virtio/vhost.c | 13
linux-headers/linux/vhost.h | 7
3 files changed, 73 insertions(+), 25 deletions(-)
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index aefe0bb..439cbba 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -24,6 +24,10 @@
#include
#define VHOST_MEMORY_MAX_NREGIONS8
+/* FIXME: same as the max number of numa node?*/
+#define HUGEPAGE_MAX_FILES 8
+
+#define RAM_SHARED (1 << 1)
typedef enum VhostUserRequest {
VHOST_USER_NONE = 0,
@@ -41,14 +45,15 @@ typedef enum VhostUserRequest {
VHOST_USER_SET_VRING_KICK = 12,
VHOST_USER_SET_VRING_CALL = 13,
VHOST_USER_SET_VRING_ERR = 14,
-VHOST_USER_MAX
+VHOST_USER_MMAP_HUGEPAGE_FILE = 15,
+VHOST_USER_UNMAP_HUGEPAGE_FILE = 16,
+VHOST_USER_MAX,
} VhostUserRequest;
typedef struct VhostUserMemoryRegion {
uint64_t guest_phys_addr;
uint64_t memory_size;
uint64_t userspace_addr;
-uint64_t mmap_offset;
} VhostUserMemoryRegion;
typedef struct VhostUserMemory {
@@ -57,6 +62,16 @@ typedef struct VhostUserMemory {
VhostUserMemoryRegion regions[VHOST_MEMORY_MAX_NREGIONS];
} VhostUserMemory;
+typedef struct HugepageMemoryInfo {
+uint64_t base_addr;
+uint64_t size;
+}HugeMemInfo;
+
+typedef struct HugepageInfo {
+uint32_t num;
+HugeMemInfo files[HUGEPAGE_MAX_FILES];
+}HugepageInfo;
+
typedef struct VhostUserMsg {
VhostUserRequest request;
@@ -71,6 +86,7 @@ typedef struct VhostUserMsg {
struct vhost_vring_state state;
struct vhost_vring_addr addr;
VhostUserMemory memory;
+HugepageInfo huge_info;
};
} QEMU_PACKED VhostUserMsg;
@@ -104,7 +120,9 @@ static unsigned long int
ioctl_to_vhost_user_request[VHOST_USER_MAX] = {
VHOST_GET_VRING_BASE, /* VHOST_USER_GET_VRING_BASE */
VHOST_SET_VRING_KICK, /* VHOST_USER_SET_VRING_KICK */
VHOST_SET_VRING_CALL, /* VHOST_USER_SET_VRING_CALL */
-VHOST_SET_VRING_ERR /* VHOST_USER_SET_VRING_ERR */
+VHOST_SET_VRING_ERR,/* VHOST_USER_SET_VRING_ERR */
+VHOST_MMAP_HUGEPAGE_FILE, /* VHOST_USER_MMAP_HUGEPAGE_FILE */
+VHOST_UNMAP_HUGEPAGE_FILE, /* VHOST_USER_UNMAP_HUGEPAGE_FILE */
};
static VhostUserRequest vhost_user_request_translate(unsigned long int
request)
@@ -190,6 +208,7 @@ static int vhost_user_call(struct vhost_dev *dev,
unsigned long int request,
int fds[VHOST_MEMORY_MAX_NREGIONS];
int i, fd;
size_t fd_num = 0;
+RAMBlock *block;
assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
@@ -213,37 +232,46 @@ static int vhost_user_call(struct vhost_dev *dev,
unsigned long int request,
case VHOST_RESET_OWNER:
break;
-case VHOST_SET_MEM_TABLE:
-for (i = 0; i < dev->mem->nregions; ++i) {
-struct vhost_memory_region *reg = dev->mem->regions + i;
-ram_addr_t ram_addr;
+case VHOST_MMAP_HUGEPAGE_FILE:
+qemu_mutex_lock_ramlist();
-assert((uintptr_t)reg->userspace_addr == reg->userspace_addr);
-qemu_ram_addr_from_host((void
*)(uintptr_t)reg->userspace_addr, &ram_addr)
Re: [Qemu-devel] [PATCH v1] vhost-user: fix not send all hugepage files to vhost-user
On Thu, Jan 29, 2015 at 11:58:08AM +0800, Linhaifeng wrote:
> Hi,Michael S.Tsirkin
>
> The vhost-user device will not work if there are two numa nodes in VM.
>
> Should we fix this bug or ignore it ?
I suggest we fix this bug.
I saw that you responded to self so I assume you will
send v2 with the modification you listed.
Did I get it right?
Also, pls Cc qemu-stable on bugfixes.
Thanks!
> On 2014/12/18 13:06, Linhaifeng wrote:
> >
> >
> > On 2014/12/17 14:02, haifeng@huawei.com wrote:
> >> From: linhaifeng
> >>
> >> If we create VM with two or more numa nodes qemu will create two
> >> or more hugepage files but qemu only send one hugepage file fd
> >> to vhost-user when VM's memory size is 2G and with two numa nodes.
> >>
> >> Signed-off-by: linhaifeng
> >> ---
> >> hw/virtio/vhost-user.c | 78
> >> ++---
> >> hw/virtio/vhost.c | 13
> >> linux-headers/linux/vhost.h | 7
> >> 3 files changed, 73 insertions(+), 25 deletions(-)
> >>
> >> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> >> index aefe0bb..439cbba 100644
> >> --- a/hw/virtio/vhost-user.c
> >> +++ b/hw/virtio/vhost-user.c
> >> @@ -24,6 +24,10 @@
> >> #include
> >>
> >> #define VHOST_MEMORY_MAX_NREGIONS8
> >> +/* FIXME: same as the max number of numa node?*/
> >> +#define HUGEPAGE_MAX_FILES 8
> >> +
> >> +#define RAM_SHARED (1 << 1)
> >>
> >> typedef enum VhostUserRequest {
> >> VHOST_USER_NONE = 0,
> >> @@ -41,14 +45,15 @@ typedef enum VhostUserRequest {
> >> VHOST_USER_SET_VRING_KICK = 12,
> >> VHOST_USER_SET_VRING_CALL = 13,
> >> VHOST_USER_SET_VRING_ERR = 14,
> >> -VHOST_USER_MAX
> >> +VHOST_USER_MMAP_HUGEPAGE_FILE = 15,
> >> +VHOST_USER_UNMAP_HUGEPAGE_FILE = 16,
> >> +VHOST_USER_MAX,
> >> } VhostUserRequest;
> >>
> >> typedef struct VhostUserMemoryRegion {
> >> uint64_t guest_phys_addr;
> >> uint64_t memory_size;
> >> uint64_t userspace_addr;
> >> -uint64_t mmap_offset;
> >> } VhostUserMemoryRegion;
> >>
> >> typedef struct VhostUserMemory {
> >> @@ -57,6 +62,16 @@ typedef struct VhostUserMemory {
> >> VhostUserMemoryRegion regions[VHOST_MEMORY_MAX_NREGIONS];
> >> } VhostUserMemory;
> >>
> >> +typedef struct HugepageMemoryInfo {
> >> +uint64_t base_addr;
> >> +uint64_t size;
> >> +}HugeMemInfo;
> >> +
> >> +typedef struct HugepageInfo {
> >> +uint32_t num;
> >> +HugeMemInfo files[HUGEPAGE_MAX_FILES];
> >> +}HugepageInfo;
> >> +
> >> typedef struct VhostUserMsg {
> >> VhostUserRequest request;
> >>
> >> @@ -71,6 +86,7 @@ typedef struct VhostUserMsg {
> >> struct vhost_vring_state state;
> >> struct vhost_vring_addr addr;
> >> VhostUserMemory memory;
> >> +HugepageInfo huge_info;
> >> };
> >> } QEMU_PACKED VhostUserMsg;
> >>
> >> @@ -104,7 +120,9 @@ static unsigned long int
> >> ioctl_to_vhost_user_request[VHOST_USER_MAX] = {
> >> VHOST_GET_VRING_BASE, /* VHOST_USER_GET_VRING_BASE */
> >> VHOST_SET_VRING_KICK, /* VHOST_USER_SET_VRING_KICK */
> >> VHOST_SET_VRING_CALL, /* VHOST_USER_SET_VRING_CALL */
> >> -VHOST_SET_VRING_ERR /* VHOST_USER_SET_VRING_ERR */
> >> +VHOST_SET_VRING_ERR,/* VHOST_USER_SET_VRING_ERR */
> >> +VHOST_MMAP_HUGEPAGE_FILE, /* VHOST_USER_MMAP_HUGEPAGE_FILE */
> >> +VHOST_UNMAP_HUGEPAGE_FILE, /* VHOST_USER_UNMAP_HUGEPAGE_FILE */
> >> };
> >>
> >> static VhostUserRequest vhost_user_request_translate(unsigned long int
> >> request)
> >> @@ -190,6 +208,7 @@ static int vhost_user_call(struct vhost_dev *dev,
> >> unsigned long int request,
> >> int fds[VHOST_MEMORY_MAX_NREGIONS];
> >> int i, fd;
> >> size_t fd_num = 0;
> >> +RAMBlock *block;
> >>
> >> assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
> >>
> >> @@ -213,37 +232,46 @@ static int vhost_user_call(struct vhost_dev *dev,
> >> unsigned long int request,
> >> case VHOST_RESET_OWNER:
> >> break;
> >>
> >> -case VHOST_SET_MEM_TABLE:
> >> -for (i = 0; i < dev->mem->nregions; ++i) {
> >> -struct vhost_memory_region *reg = dev->mem->regions + i;
> >> -ram_addr_t ram_addr;
> >> +case VHOST_MMAP_HUGEPAGE_FILE:
> >> +qemu_mutex_lock_ramlist();
> >>
> >> -assert((uintptr_t)reg->userspace_addr == reg->userspace_addr);
> >> -qemu_ram_addr_from_host((void
> >> *)(uintptr_t)reg->userspace_addr, &ram_addr);
> >> -fd = qemu_get_ram_fd(ram_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_add
Re: [Qemu-devel] [PATCH v1] vhost-user: fix not send all hugepage files to vhost-user
Hi,Michael S.Tsirkin
The vhost-user device will not work if there are two numa nodes in VM.
Should we fix this bug or ignore it ?
On 2014/12/18 13:06, Linhaifeng wrote:
>
>
> On 2014/12/17 14:02, haifeng@huawei.com wrote:
>> From: linhaifeng
>>
>> If we create VM with two or more numa nodes qemu will create two
>> or more hugepage files but qemu only send one hugepage file fd
>> to vhost-user when VM's memory size is 2G and with two numa nodes.
>>
>> Signed-off-by: linhaifeng
>> ---
>> hw/virtio/vhost-user.c | 78
>> ++---
>> hw/virtio/vhost.c | 13
>> linux-headers/linux/vhost.h | 7
>> 3 files changed, 73 insertions(+), 25 deletions(-)
>>
>> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
>> index aefe0bb..439cbba 100644
>> --- a/hw/virtio/vhost-user.c
>> +++ b/hw/virtio/vhost-user.c
>> @@ -24,6 +24,10 @@
>> #include
>>
>> #define VHOST_MEMORY_MAX_NREGIONS8
>> +/* FIXME: same as the max number of numa node?*/
>> +#define HUGEPAGE_MAX_FILES 8
>> +
>> +#define RAM_SHARED (1 << 1)
>>
>> typedef enum VhostUserRequest {
>> VHOST_USER_NONE = 0,
>> @@ -41,14 +45,15 @@ typedef enum VhostUserRequest {
>> VHOST_USER_SET_VRING_KICK = 12,
>> VHOST_USER_SET_VRING_CALL = 13,
>> VHOST_USER_SET_VRING_ERR = 14,
>> -VHOST_USER_MAX
>> +VHOST_USER_MMAP_HUGEPAGE_FILE = 15,
>> +VHOST_USER_UNMAP_HUGEPAGE_FILE = 16,
>> +VHOST_USER_MAX,
>> } VhostUserRequest;
>>
>> typedef struct VhostUserMemoryRegion {
>> uint64_t guest_phys_addr;
>> uint64_t memory_size;
>> uint64_t userspace_addr;
>> -uint64_t mmap_offset;
>> } VhostUserMemoryRegion;
>>
>> typedef struct VhostUserMemory {
>> @@ -57,6 +62,16 @@ typedef struct VhostUserMemory {
>> VhostUserMemoryRegion regions[VHOST_MEMORY_MAX_NREGIONS];
>> } VhostUserMemory;
>>
>> +typedef struct HugepageMemoryInfo {
>> +uint64_t base_addr;
>> +uint64_t size;
>> +}HugeMemInfo;
>> +
>> +typedef struct HugepageInfo {
>> +uint32_t num;
>> +HugeMemInfo files[HUGEPAGE_MAX_FILES];
>> +}HugepageInfo;
>> +
>> typedef struct VhostUserMsg {
>> VhostUserRequest request;
>>
>> @@ -71,6 +86,7 @@ typedef struct VhostUserMsg {
>> struct vhost_vring_state state;
>> struct vhost_vring_addr addr;
>> VhostUserMemory memory;
>> +HugepageInfo huge_info;
>> };
>> } QEMU_PACKED VhostUserMsg;
>>
>> @@ -104,7 +120,9 @@ static unsigned long int
>> ioctl_to_vhost_user_request[VHOST_USER_MAX] = {
>> VHOST_GET_VRING_BASE, /* VHOST_USER_GET_VRING_BASE */
>> VHOST_SET_VRING_KICK, /* VHOST_USER_SET_VRING_KICK */
>> VHOST_SET_VRING_CALL, /* VHOST_USER_SET_VRING_CALL */
>> -VHOST_SET_VRING_ERR /* VHOST_USER_SET_VRING_ERR */
>> +VHOST_SET_VRING_ERR,/* VHOST_USER_SET_VRING_ERR */
>> +VHOST_MMAP_HUGEPAGE_FILE, /* VHOST_USER_MMAP_HUGEPAGE_FILE */
>> +VHOST_UNMAP_HUGEPAGE_FILE, /* VHOST_USER_UNMAP_HUGEPAGE_FILE */
>> };
>>
>> static VhostUserRequest vhost_user_request_translate(unsigned long int
>> request)
>> @@ -190,6 +208,7 @@ static int vhost_user_call(struct vhost_dev *dev,
>> unsigned long int request,
>> int fds[VHOST_MEMORY_MAX_NREGIONS];
>> int i, fd;
>> size_t fd_num = 0;
>> +RAMBlock *block;
>>
>> assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
>>
>> @@ -213,37 +232,46 @@ static int vhost_user_call(struct vhost_dev *dev,
>> unsigned long int request,
>> case VHOST_RESET_OWNER:
>> break;
>>
>> -case VHOST_SET_MEM_TABLE:
>> -for (i = 0; i < dev->mem->nregions; ++i) {
>> -struct vhost_memory_region *reg = dev->mem->regions + i;
>> -ram_addr_t ram_addr;
>> +case VHOST_MMAP_HUGEPAGE_FILE:
>> +qemu_mutex_lock_ramlist();
>>
>> -assert((uintptr_t)reg->userspace_addr == reg->userspace_addr);
>> -qemu_ram_addr_from_host((void *)(uintptr_t)reg->userspace_addr,
>> &ram_addr);
>> -fd = qemu_get_ram_fd(ram_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(ram_addr);
>> -assert(fd_num < VHOST_MEMORY_MAX_NREGIONS);
>> -fds[fd_num++] = fd;
>> +/* Get hugepage file informations */
>> +QTAILQ_FOREACH(block, &ram_list.blocks, next) {
>> +if (block->flags & RAM_SHARED && block->fd > 0) {
>> +msg.huge_info.files[fd_num].size = block->length;
>> +msg.huge_info.files[fd_num].base_addr = block->host;
>> +
Re: [Qemu-devel] [PATCH v1] vhost-user: fix not send all hugepage files to vhost-user
On 2014/12/17 14:02, haifeng@huawei.com wrote:
> From: linhaifeng
>
> If we create VM with two or more numa nodes qemu will create two
> or more hugepage files but qemu only send one hugepage file fd
> to vhost-user when VM's memory size is 2G and with two numa nodes.
>
> Signed-off-by: linhaifeng
> ---
> hw/virtio/vhost-user.c | 78
> ++---
> hw/virtio/vhost.c | 13
> linux-headers/linux/vhost.h | 7
> 3 files changed, 73 insertions(+), 25 deletions(-)
>
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index aefe0bb..439cbba 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -24,6 +24,10 @@
> #include
>
> #define VHOST_MEMORY_MAX_NREGIONS8
> +/* FIXME: same as the max number of numa node?*/
> +#define HUGEPAGE_MAX_FILES 8
> +
> +#define RAM_SHARED (1 << 1)
>
> typedef enum VhostUserRequest {
> VHOST_USER_NONE = 0,
> @@ -41,14 +45,15 @@ typedef enum VhostUserRequest {
> VHOST_USER_SET_VRING_KICK = 12,
> VHOST_USER_SET_VRING_CALL = 13,
> VHOST_USER_SET_VRING_ERR = 14,
> -VHOST_USER_MAX
> +VHOST_USER_MMAP_HUGEPAGE_FILE = 15,
> +VHOST_USER_UNMAP_HUGEPAGE_FILE = 16,
> +VHOST_USER_MAX,
> } VhostUserRequest;
>
> typedef struct VhostUserMemoryRegion {
> uint64_t guest_phys_addr;
> uint64_t memory_size;
> uint64_t userspace_addr;
> -uint64_t mmap_offset;
> } VhostUserMemoryRegion;
>
> typedef struct VhostUserMemory {
> @@ -57,6 +62,16 @@ typedef struct VhostUserMemory {
> VhostUserMemoryRegion regions[VHOST_MEMORY_MAX_NREGIONS];
> } VhostUserMemory;
>
> +typedef struct HugepageMemoryInfo {
> +uint64_t base_addr;
> +uint64_t size;
> +}HugeMemInfo;
> +
> +typedef struct HugepageInfo {
> +uint32_t num;
> +HugeMemInfo files[HUGEPAGE_MAX_FILES];
> +}HugepageInfo;
> +
> typedef struct VhostUserMsg {
> VhostUserRequest request;
>
> @@ -71,6 +86,7 @@ typedef struct VhostUserMsg {
> struct vhost_vring_state state;
> struct vhost_vring_addr addr;
> VhostUserMemory memory;
> +HugepageInfo huge_info;
> };
> } QEMU_PACKED VhostUserMsg;
>
> @@ -104,7 +120,9 @@ static unsigned long int
> ioctl_to_vhost_user_request[VHOST_USER_MAX] = {
> VHOST_GET_VRING_BASE, /* VHOST_USER_GET_VRING_BASE */
> VHOST_SET_VRING_KICK, /* VHOST_USER_SET_VRING_KICK */
> VHOST_SET_VRING_CALL, /* VHOST_USER_SET_VRING_CALL */
> -VHOST_SET_VRING_ERR /* VHOST_USER_SET_VRING_ERR */
> +VHOST_SET_VRING_ERR,/* VHOST_USER_SET_VRING_ERR */
> +VHOST_MMAP_HUGEPAGE_FILE, /* VHOST_USER_MMAP_HUGEPAGE_FILE */
> +VHOST_UNMAP_HUGEPAGE_FILE, /* VHOST_USER_UNMAP_HUGEPAGE_FILE */
> };
>
> static VhostUserRequest vhost_user_request_translate(unsigned long int
> request)
> @@ -190,6 +208,7 @@ static int vhost_user_call(struct vhost_dev *dev,
> unsigned long int request,
> int fds[VHOST_MEMORY_MAX_NREGIONS];
> int i, fd;
> size_t fd_num = 0;
> +RAMBlock *block;
>
> assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
>
> @@ -213,37 +232,46 @@ static int vhost_user_call(struct vhost_dev *dev,
> unsigned long int request,
> case VHOST_RESET_OWNER:
> break;
>
> -case VHOST_SET_MEM_TABLE:
> -for (i = 0; i < dev->mem->nregions; ++i) {
> -struct vhost_memory_region *reg = dev->mem->regions + i;
> -ram_addr_t ram_addr;
> +case VHOST_MMAP_HUGEPAGE_FILE:
> +qemu_mutex_lock_ramlist();
>
> -assert((uintptr_t)reg->userspace_addr == reg->userspace_addr);
> -qemu_ram_addr_from_host((void *)(uintptr_t)reg->userspace_addr,
> &ram_addr);
> -fd = qemu_get_ram_fd(ram_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(ram_addr);
> -assert(fd_num < VHOST_MEMORY_MAX_NREGIONS);
> -fds[fd_num++] = fd;
> +/* Get hugepage file informations */
> +QTAILQ_FOREACH(block, &ram_list.blocks, next) {
> +if (block->flags & RAM_SHARED && block->fd > 0) {
> +msg.huge_info.files[fd_num].size = block->length;
> +msg.huge_info.files[fd_num].base_addr = block->host;
> +fds[fd_num++] = block->fd;
> }
> }
> +msg.huge_info.num = fd_num;
>
> -msg.memory.nregions = fd_num;
> +/* Calculate msg size */
> +msg.size = sizeof(m.huge_info.num);
> +msg.size += fd_num * sizeof(HugeMemInfo);
> +
> +
