* [Qemu-devel] [PATCH v1] vhost-user: fix not send all hugepage files to vhost-user
@ 2014-12-17 6:02 haifeng.lin
2014-12-18 5:06 ` Linhaifeng
2015-02-01 23:29 ` Paolo Bonzini
0 siblings, 2 replies; 10+ messages in thread
From: haifeng.lin @ 2014-12-17 6:02 UTC (permalink / raw)
To: qemu-devel
Cc: mst, jerry.lilijun, n.nikolaev, milo.raofei, arei.gonglei,
pbonzini
From: linhaifeng <haifeng.lin@huawei.com>
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 <haifeng.lin@huawei.com>
---
hw/virtio/vhost-user.c | 78 ++++++++++++++++++++++++++++++---------------
hw/virtio/vhost.c | 13 ++++++++
| 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 <linux/vhost.h>
#define VHOST_MEMORY_MAX_NREGIONS 8
+/* 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);
+
+ qemu_mutex_unlock_ramlist();
+ break;
- if (!fd_num) {
- error_report("Failed initializing vhost-user memory map\n"
- "consider using -object memory-backend-file share=on\n");
- return -1;
+ case VHOST_UNMAP_HUGEPAGE_FILE:
+ /* Tell vhost-user to unmap all hugepage files. */
+ break;
+
+ case VHOST_SET_MEM_TABLE:
+ for (i = 0; i < dev->mem->nregions; i++) {
+ struct vhost_memory_region *reg = dev->mem->regions + i;
+
+ assert((uintptr_t)reg->userspace_addr == reg->userspace_addr);
+
+ msg.memory.regions[i].userspace_addr = reg->userspace_addr;
+ msg.memory.regions[i].memory_size = reg->memory_size;
+ msg.memory.regions[i].guest_phys_addr = reg->guest_phys_addr;
+ assert(fd_num < VHOST_MEMORY_MAX_NREGIONS);
}
+ msg.memory.nregions = i;
msg.size = sizeof(m.memory.nregions);
msg.size += sizeof(m.memory.padding);
- msg.size += fd_num * sizeof(VhostUserMemoryRegion);
-
+ msg.size += i * sizeof(VhostUserMemoryRegion);
break;
case VHOST_SET_LOG_FD:
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 5a12861..b8eb341 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1041,6 +1041,14 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev)
if (r < 0) {
goto fail_features;
}
+ if (hdev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER) {
+ r = hdev->vhost_ops->vhost_call(hdev, VHOST_MMAP_HUGEPAGE_FILE,
+ NULL);
+ if (r < 0) {
+ r = -errno;
+ goto fail_mem;
+ }
+ }
r = hdev->vhost_ops->vhost_call(hdev, VHOST_SET_MEM_TABLE, hdev->mem);
if (r < 0) {
r = -errno;
@@ -1101,5 +1109,10 @@ void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev)
g_free(hdev->log);
hdev->log = NULL;
hdev->log_size = 0;
+
+ if (hdev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER) {
+ (void)hdev->vhost_ops->vhost_call(hdev, VHOST_MMAP_HUGEPAGE_FILE,
+ NULL);
+ }
}
--git a/linux-headers/linux/vhost.h b/linux-headers/linux/vhost.h
index c656f61..bb72811 100644
--- a/linux-headers/linux/vhost.h
+++ b/linux-headers/linux/vhost.h
@@ -113,6 +113,13 @@ struct vhost_memory {
/* Set eventfd to signal an error */
#define VHOST_SET_VRING_ERR _IOW(VHOST_VIRTIO, 0x22, struct vhost_vring_file)
+/* Tell vhost-user to mmap hugepage file */
+#define VHOST_MMAP_HUGEPAGE_FILE _IOW(VHOST_VIRTIO, 0x23, int)
+/* Tell vhost-user to unmap hugepage file */
+#define VHOST_UNMAP_HUGEPAGE_FILE _IOW(VHOST_VIRTIO, 0x24, int)
+
+#define VHOST_THREAD_ID _IOR(VHOST_VIRTIO, 0x25, struct vhost_vring_thread)
+
/* VHOST_NET specific defines */
/* Attach virtio net ring to a raw socket, or tap device.
--
1.9.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v1] vhost-user: fix not send all hugepage files to vhost-user
2014-12-17 6:02 [Qemu-devel] [PATCH v1] vhost-user: fix not send all hugepage files to vhost-user haifeng.lin
@ 2014-12-18 5:06 ` Linhaifeng
2015-01-29 3:58 ` Linhaifeng
2015-02-01 23:29 ` Paolo Bonzini
1 sibling, 1 reply; 10+ messages in thread
From: Linhaifeng @ 2014-12-18 5:06 UTC (permalink / raw)
To: qemu-devel
Cc: mst, jerry.lilijun, n.nikolaev, milo.raofei, arei.gonglei,
pbonzini
On 2014/12/17 14:02, haifeng.lin@huawei.com wrote:
> From: linhaifeng <haifeng.lin@huawei.com>
>
> 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 <haifeng.lin@huawei.com>
> ---
> 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 <linux/vhost.h>
>
> #define VHOST_MEMORY_MAX_NREGIONS 8
> +/* 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);
> +
> + qemu_mutex_unlock_ramlist();
> + break;
>
> - if (!fd_num) {
> - error_report("Failed initializing vhost-user memory map\n"
> - "consider using -object memory-backend-file share=on\n");
> - return -1;
> + case VHOST_UNMAP_HUGEPAGE_FILE:
> + /* Tell vhost-user to unmap all hugepage files. */
> + break;
> +
> + case VHOST_SET_MEM_TABLE:
> + for (i = 0; i < dev->mem->nregions; i++) {
> + struct vhost_memory_region *reg = dev->mem->regions + i;
> +
> + assert((uintptr_t)reg->userspace_addr == reg->userspace_addr);
> +
> + msg.memory.regions[i].userspace_addr = reg->userspace_addr;
> + msg.memory.regions[i].memory_size = reg->memory_size;
> + msg.memory.regions[i].guest_phys_addr = reg->guest_phys_addr;
> + assert(fd_num < VHOST_MEMORY_MAX_NREGIONS);
> }
>
> + msg.memory.nregions = i;
> msg.size = sizeof(m.memory.nregions);
> msg.size += sizeof(m.memory.padding);
> - msg.size += fd_num * sizeof(VhostUserMemoryRegion);
> -
> + msg.size += i * sizeof(VhostUserMemoryRegion);
> break;
>
> case VHOST_SET_LOG_FD:
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 5a12861..b8eb341 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -1041,6 +1041,14 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev)
> if (r < 0) {
> goto fail_features;
> }
> + if (hdev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER) {
> + r = hdev->vhost_ops->vhost_call(hdev, VHOST_MMAP_HUGEPAGE_FILE,
> + NULL);
> + if (r < 0) {
> + r = -errno;
> + goto fail_mem;
> + }
> + }
> r = hdev->vhost_ops->vhost_call(hdev, VHOST_SET_MEM_TABLE, hdev->mem);
> if (r < 0) {
> r = -errno;
> @@ -1101,5 +1109,10 @@ void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev)
> g_free(hdev->log);
> hdev->log = NULL;
> hdev->log_size = 0;
> +
> + if (hdev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER) {
> + (void)hdev->vhost_ops->vhost_call(hdev, VHOST_MMAP_HUGEPAGE_FILE,
VHOST_MMAP_HUGEPAGE_FILE -> VHOST_UNMAP_HUGEPAGE_FILE
> + NULL);
> + }
> }
>
> diff --git a/linux-headers/linux/vhost.h b/linux-headers/linux/vhost.h
> index c656f61..bb72811 100644
> --- a/linux-headers/linux/vhost.h
> +++ b/linux-headers/linux/vhost.h
> @@ -113,6 +113,13 @@ struct vhost_memory {
> /* Set eventfd to signal an error */
> #define VHOST_SET_VRING_ERR _IOW(VHOST_VIRTIO, 0x22, struct vhost_vring_file)
>
> +/* Tell vhost-user to mmap hugepage file */
> +#define VHOST_MMAP_HUGEPAGE_FILE _IOW(VHOST_VIRTIO, 0x23, int)
> +/* Tell vhost-user to unmap hugepage file */
> +#define VHOST_UNMAP_HUGEPAGE_FILE _IOW(VHOST_VIRTIO, 0x24, int)
> +
> +#define VHOST_THREAD_ID _IOR(VHOST_VIRTIO, 0x25, struct vhost_vring_thread)
> +
> /* VHOST_NET specific defines */
>
> /* Attach virtio net ring to a raw socket, or tap device.
>
--
Regards,
Haifeng
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v1] vhost-user: fix not send all hugepage files to vhost-user
2014-12-18 5:06 ` Linhaifeng
@ 2015-01-29 3:58 ` Linhaifeng
2015-01-29 10:51 ` Michael S. Tsirkin
0 siblings, 1 reply; 10+ messages in thread
From: Linhaifeng @ 2015-01-29 3:58 UTC (permalink / raw)
To: qemu-devel
Cc: mst, jerry.lilijun, n.nikolaev, milo.raofei, arei.gonglei,
pbonzini
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.lin@huawei.com wrote:
>> From: linhaifeng <haifeng.lin@huawei.com>
>>
>> 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 <haifeng.lin@huawei.com>
>> ---
>> 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 <linux/vhost.h>
>>
>> #define VHOST_MEMORY_MAX_NREGIONS 8
>> +/* 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);
>> +
>> + qemu_mutex_unlock_ramlist();
>> + break;
>>
>> - if (!fd_num) {
>> - error_report("Failed initializing vhost-user memory map\n"
>> - "consider using -object memory-backend-file share=on\n");
>> - return -1;
>> + case VHOST_UNMAP_HUGEPAGE_FILE:
>> + /* Tell vhost-user to unmap all hugepage files. */
>> + break;
>> +
>> + case VHOST_SET_MEM_TABLE:
>> + for (i = 0; i < dev->mem->nregions; i++) {
>> + struct vhost_memory_region *reg = dev->mem->regions + i;
>> +
>> + assert((uintptr_t)reg->userspace_addr == reg->userspace_addr);
>> +
>> + msg.memory.regions[i].userspace_addr = reg->userspace_addr;
>> + msg.memory.regions[i].memory_size = reg->memory_size;
>> + msg.memory.regions[i].guest_phys_addr = reg->guest_phys_addr;
>> + assert(fd_num < VHOST_MEMORY_MAX_NREGIONS);
>> }
>>
>> + msg.memory.nregions = i;
>> msg.size = sizeof(m.memory.nregions);
>> msg.size += sizeof(m.memory.padding);
>> - msg.size += fd_num * sizeof(VhostUserMemoryRegion);
>> -
>> + msg.size += i * sizeof(VhostUserMemoryRegion);
>> break;
>>
>> case VHOST_SET_LOG_FD:
>> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
>> index 5a12861..b8eb341 100644
>> --- a/hw/virtio/vhost.c
>> +++ b/hw/virtio/vhost.c
>> @@ -1041,6 +1041,14 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev)
>> if (r < 0) {
>> goto fail_features;
>> }
>> + if (hdev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER) {
>> + r = hdev->vhost_ops->vhost_call(hdev, VHOST_MMAP_HUGEPAGE_FILE,
>> + NULL);
>> + if (r < 0) {
>> + r = -errno;
>> + goto fail_mem;
>> + }
>> + }
>> r = hdev->vhost_ops->vhost_call(hdev, VHOST_SET_MEM_TABLE, hdev->mem);
>> if (r < 0) {
>> r = -errno;
>> @@ -1101,5 +1109,10 @@ void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev)
>> g_free(hdev->log);
>> hdev->log = NULL;
>> hdev->log_size = 0;
>> +
>> + if (hdev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER) {
>> + (void)hdev->vhost_ops->vhost_call(hdev, VHOST_MMAP_HUGEPAGE_FILE,
>
> VHOST_MMAP_HUGEPAGE_FILE -> VHOST_UNMAP_HUGEPAGE_FILE
>
>> + NULL);
>> + }
>> }
>>
>> diff --git a/linux-headers/linux/vhost.h b/linux-headers/linux/vhost.h
>> index c656f61..bb72811 100644
>> --- a/linux-headers/linux/vhost.h
>> +++ b/linux-headers/linux/vhost.h
>> @@ -113,6 +113,13 @@ struct vhost_memory {
>> /* Set eventfd to signal an error */
>> #define VHOST_SET_VRING_ERR _IOW(VHOST_VIRTIO, 0x22, struct vhost_vring_file)
>>
>> +/* Tell vhost-user to mmap hugepage file */
>> +#define VHOST_MMAP_HUGEPAGE_FILE _IOW(VHOST_VIRTIO, 0x23, int)
>> +/* Tell vhost-user to unmap hugepage file */
>> +#define VHOST_UNMAP_HUGEPAGE_FILE _IOW(VHOST_VIRTIO, 0x24, int)
>> +
>> +#define VHOST_THREAD_ID _IOR(VHOST_VIRTIO, 0x25, struct vhost_vring_thread)
>> +
>> /* VHOST_NET specific defines */
>>
>> /* Attach virtio net ring to a raw socket, or tap device.
>>
>
--
Regards,
Haifeng
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v1] vhost-user: fix not send all hugepage files to vhost-user
2015-01-29 3:58 ` Linhaifeng
@ 2015-01-29 10:51 ` Michael S. Tsirkin
2015-01-29 13:02 ` Linhaifeng
0 siblings, 1 reply; 10+ messages in thread
From: Michael S. Tsirkin @ 2015-01-29 10:51 UTC (permalink / raw)
To: Linhaifeng
Cc: qemu-devel, n.nikolaev, jerry.lilijun, milo.raofei, arei.gonglei,
pbonzini
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.lin@huawei.com wrote:
> >> From: linhaifeng <haifeng.lin@huawei.com>
> >>
> >> 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 <haifeng.lin@huawei.com>
> >> ---
> >> 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 <linux/vhost.h>
> >>
> >> #define VHOST_MEMORY_MAX_NREGIONS 8
> >> +/* 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);
> >> +
> >> + qemu_mutex_unlock_ramlist();
> >> + break;
> >>
> >> - if (!fd_num) {
> >> - error_report("Failed initializing vhost-user memory map\n"
> >> - "consider using -object memory-backend-file share=on\n");
> >> - return -1;
> >> + case VHOST_UNMAP_HUGEPAGE_FILE:
> >> + /* Tell vhost-user to unmap all hugepage files. */
> >> + break;
> >> +
> >> + case VHOST_SET_MEM_TABLE:
> >> + for (i = 0; i < dev->mem->nregions; i++) {
> >> + struct vhost_memory_region *reg = dev->mem->regions + i;
> >> +
> >> + assert((uintptr_t)reg->userspace_addr == reg->userspace_addr);
> >> +
> >> + msg.memory.regions[i].userspace_addr = reg->userspace_addr;
> >> + msg.memory.regions[i].memory_size = reg->memory_size;
> >> + msg.memory.regions[i].guest_phys_addr = reg->guest_phys_addr;
> >> + assert(fd_num < VHOST_MEMORY_MAX_NREGIONS);
> >> }
> >>
> >> + msg.memory.nregions = i;
> >> msg.size = sizeof(m.memory.nregions);
> >> msg.size += sizeof(m.memory.padding);
> >> - msg.size += fd_num * sizeof(VhostUserMemoryRegion);
> >> -
> >> + msg.size += i * sizeof(VhostUserMemoryRegion);
> >> break;
> >>
> >> case VHOST_SET_LOG_FD:
> >> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> >> index 5a12861..b8eb341 100644
> >> --- a/hw/virtio/vhost.c
> >> +++ b/hw/virtio/vhost.c
> >> @@ -1041,6 +1041,14 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev)
> >> if (r < 0) {
> >> goto fail_features;
> >> }
> >> + if (hdev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER) {
> >> + r = hdev->vhost_ops->vhost_call(hdev, VHOST_MMAP_HUGEPAGE_FILE,
> >> + NULL);
> >> + if (r < 0) {
> >> + r = -errno;
> >> + goto fail_mem;
> >> + }
> >> + }
> >> r = hdev->vhost_ops->vhost_call(hdev, VHOST_SET_MEM_TABLE, hdev->mem);
> >> if (r < 0) {
> >> r = -errno;
> >> @@ -1101,5 +1109,10 @@ void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev)
> >> g_free(hdev->log);
> >> hdev->log = NULL;
> >> hdev->log_size = 0;
> >> +
> >> + if (hdev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER) {
> >> + (void)hdev->vhost_ops->vhost_call(hdev, VHOST_MMAP_HUGEPAGE_FILE,
> >
> > VHOST_MMAP_HUGEPAGE_FILE -> VHOST_UNMAP_HUGEPAGE_FILE
> >
> >> + NULL);
> >> + }
> >> }
> >>
> >> diff --git a/linux-headers/linux/vhost.h b/linux-headers/linux/vhost.h
> >> index c656f61..bb72811 100644
> >> --- a/linux-headers/linux/vhost.h
> >> +++ b/linux-headers/linux/vhost.h
> >> @@ -113,6 +113,13 @@ struct vhost_memory {
> >> /* Set eventfd to signal an error */
> >> #define VHOST_SET_VRING_ERR _IOW(VHOST_VIRTIO, 0x22, struct vhost_vring_file)
> >>
> >> +/* Tell vhost-user to mmap hugepage file */
> >> +#define VHOST_MMAP_HUGEPAGE_FILE _IOW(VHOST_VIRTIO, 0x23, int)
> >> +/* Tell vhost-user to unmap hugepage file */
> >> +#define VHOST_UNMAP_HUGEPAGE_FILE _IOW(VHOST_VIRTIO, 0x24, int)
> >> +
> >> +#define VHOST_THREAD_ID _IOR(VHOST_VIRTIO, 0x25, struct vhost_vring_thread)
> >> +
> >> /* VHOST_NET specific defines */
> >>
> >> /* Attach virtio net ring to a raw socket, or tap device.
> >>
> >
>
> --
> Regards,
> Haifeng
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v1] vhost-user: fix not send all hugepage files to vhost-user
2015-01-29 10:51 ` Michael S. Tsirkin
@ 2015-01-29 13:02 ` Linhaifeng
0 siblings, 0 replies; 10+ messages in thread
From: Linhaifeng @ 2015-01-29 13:02 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: qemu-stable, qemu-devel, n.nikolaev, jerry.lilijun, milo.raofei,
arei.gonglei, pbonzini
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.lin@huawei.com wrote:
>>>> From: linhaifeng <haifeng.lin@huawei.com>
>>>>
>>>> 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 <haifeng.lin@huawei.com>
>>>> ---
>>>> 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 <linux/vhost.h>
>>>>
>>>> #define VHOST_MEMORY_MAX_NREGIONS 8
>>>> +/* 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);
>>>> +
>>>> + qemu_mutex_unlock_ramlist();
>>>> + break;
>>>>
>>>> - if (!fd_num) {
>>>> - error_report("Failed initializing vhost-user memory map\n"
>>>> - "consider using -object memory-backend-file share=on\n");
>>>> - return -1;
>>>> + case VHOST_UNMAP_HUGEPAGE_FILE:
>>>> + /* Tell vhost-user to unmap all hugepage files. */
>>>> + break;
>>>> +
>>>> + case VHOST_SET_MEM_TABLE:
>>>> + for (i = 0; i < dev->mem->nregions; i++) {
>>>> + struct vhost_memory_region *reg = dev->mem->regions + i;
>>>> +
>>>> + assert((uintptr_t)reg->userspace_addr == reg->userspace_addr);
>>>> +
>>>> + msg.memory.regions[i].userspace_addr = reg->userspace_addr;
>>>> + msg.memory.regions[i].memory_size = reg->memory_size;
>>>> + msg.memory.regions[i].guest_phys_addr = reg->guest_phys_addr;
>>>> + assert(fd_num < VHOST_MEMORY_MAX_NREGIONS);
>>>> }
>>>>
>>>> + msg.memory.nregions = i;
>>>> msg.size = sizeof(m.memory.nregions);
>>>> msg.size += sizeof(m.memory.padding);
>>>> - msg.size += fd_num * sizeof(VhostUserMemoryRegion);
>>>> -
>>>> + msg.size += i * sizeof(VhostUserMemoryRegion);
>>>> break;
>>>>
>>>> case VHOST_SET_LOG_FD:
>>>> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
>>>> index 5a12861..b8eb341 100644
>>>> --- a/hw/virtio/vhost.c
>>>> +++ b/hw/virtio/vhost.c
>>>> @@ -1041,6 +1041,14 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev)
>>>> if (r < 0) {
>>>> goto fail_features;
>>>> }
>>>> + if (hdev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER) {
>>>> + r = hdev->vhost_ops->vhost_call(hdev, VHOST_MMAP_HUGEPAGE_FILE,
>>>> + NULL);
>>>> + if (r < 0) {
>>>> + r = -errno;
>>>> + goto fail_mem;
>>>> + }
>>>> + }
>>>> r = hdev->vhost_ops->vhost_call(hdev, VHOST_SET_MEM_TABLE, hdev->mem);
>>>> if (r < 0) {
>>>> r = -errno;
>>>> @@ -1101,5 +1109,10 @@ void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev)
>>>> g_free(hdev->log);
>>>> hdev->log = NULL;
>>>> hdev->log_size = 0;
>>>> +
>>>> + if (hdev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER) {
>>>> + (void)hdev->vhost_ops->vhost_call(hdev, VHOST_MMAP_HUGEPAGE_FILE,
>>>
>>> VHOST_MMAP_HUGEPAGE_FILE -> VHOST_UNMAP_HUGEPAGE_FILE
>>>
>>>> + NULL);
>>>> + }
>>>> }
>>>>
>>>> diff --git a/linux-headers/linux/vhost.h b/linux-headers/linux/vhost.h
>>>> index c656f61..bb72811 100644
>>>> --- a/linux-headers/linux/vhost.h
>>>> +++ b/linux-headers/linux/vhost.h
>>>> @@ -113,6 +113,13 @@ struct vhost_memory {
>>>> /* Set eventfd to signal an error */
>>>> #define VHOST_SET_VRING_ERR _IOW(VHOST_VIRTIO, 0x22, struct vhost_vring_file)
>>>>
>>>> +/* Tell vhost-user to mmap hugepage file */
>>>> +#define VHOST_MMAP_HUGEPAGE_FILE _IOW(VHOST_VIRTIO, 0x23, int)
>>>> +/* Tell vhost-user to unmap hugepage file */
>>>> +#define VHOST_UNMAP_HUGEPAGE_FILE _IOW(VHOST_VIRTIO, 0x24, int)
>>>> +
>>>> +#define VHOST_THREAD_ID _IOR(VHOST_VIRTIO, 0x25, struct vhost_vring_thread)
>>>> +
>>>> /* VHOST_NET specific defines */
>>>>
>>>> /* Attach virtio net ring to a raw socket, or tap device.
>>>>
>>>
>>
>> --
>> Regards,
>> Haifeng
>
> .
>
--
Regards,
Haifeng
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v1] vhost-user: fix not send all hugepage files to vhost-user
2014-12-17 6:02 [Qemu-devel] [PATCH v1] vhost-user: fix not send all hugepage files to vhost-user haifeng.lin
2014-12-18 5:06 ` Linhaifeng
@ 2015-02-01 23:29 ` Paolo Bonzini
2015-02-03 13:09 ` Linhaifeng
` (2 more replies)
1 sibling, 3 replies; 10+ messages in thread
From: Paolo Bonzini @ 2015-02-01 23:29 UTC (permalink / raw)
To: haifeng.lin, qemu-devel
Cc: milo.raofei, arei.gonglei, jerry.lilijun, n.nikolaev, mst
On 17/12/2014 07:02, haifeng.lin@huawei.com wrote:
> From: linhaifeng <haifeng.lin@huawei.com>
>
> 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 <haifeng.lin@huawei.com>
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 <linux/vhost.h>
>
> #define VHOST_MEMORY_MAX_NREGIONS 8
> +/* 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);
> +
> + qemu_mutex_unlock_ramlist();
> + break;
>
> - if (!fd_num) {
> - error_report("Failed initializing vhost-user memory map\n"
> - "consider using -object memory-backend-file share=on\n");
> - return -1;
> + case VHOST_UNMAP_HUGEPAGE_FILE:
> + /* Tell vhost-user to unmap all hugepage files. */
> + break;
> +
> + case VHOST_SET_MEM_TABLE:
> + for (i = 0; i < dev->mem->nregions; i++) {
> + struct vhost_memory_region *reg = dev->mem->regions + i;
> +
> + assert((uintptr_t)reg->userspace_addr == reg->userspace_addr);
> +
> + msg.memory.regions[i].userspace_addr = reg->userspace_addr;
> + msg.memory.regions[i].memory_size = reg->memory_size;
> + msg.memory.regions[i].guest_phys_addr = reg->guest_phys_addr;
> + assert(fd_num < VHOST_MEMORY_MAX_NREGIONS);
> }
>
> + msg.memory.nregions = i;
> msg.size = sizeof(m.memory.nregions);
> msg.size += sizeof(m.memory.padding);
> - msg.size += fd_num * sizeof(VhostUserMemoryRegion);
> -
> + msg.size += i * sizeof(VhostUserMemoryRegion);
> break;
>
> case VHOST_SET_LOG_FD:
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 5a12861..b8eb341 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -1041,6 +1041,14 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev)
> if (r < 0) {
> goto fail_features;
> }
> + if (hdev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER) {
> + r = hdev->vhost_ops->vhost_call(hdev, VHOST_MMAP_HUGEPAGE_FILE,
> + NULL);
> + if (r < 0) {
> + r = -errno;
> + goto fail_mem;
> + }
> + }
> r = hdev->vhost_ops->vhost_call(hdev, VHOST_SET_MEM_TABLE, hdev->mem);
> if (r < 0) {
> r = -errno;
> @@ -1101,5 +1109,10 @@ void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev)
> g_free(hdev->log);
> hdev->log = NULL;
> hdev->log_size = 0;
> +
> + if (hdev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER) {
> + (void)hdev->vhost_ops->vhost_call(hdev, VHOST_MMAP_HUGEPAGE_FILE,
> + NULL);
> + }
> }
>
> diff --git a/linux-headers/linux/vhost.h b/linux-headers/linux/vhost.h
> index c656f61..bb72811 100644
> --- a/linux-headers/linux/vhost.h
> +++ b/linux-headers/linux/vhost.h
> @@ -113,6 +113,13 @@ struct vhost_memory {
> /* Set eventfd to signal an error */
> #define VHOST_SET_VRING_ERR _IOW(VHOST_VIRTIO, 0x22, struct vhost_vring_file)
>
> +/* Tell vhost-user to mmap hugepage file */
> +#define VHOST_MMAP_HUGEPAGE_FILE _IOW(VHOST_VIRTIO, 0x23, int)
> +/* Tell vhost-user to unmap hugepage file */
> +#define VHOST_UNMAP_HUGEPAGE_FILE _IOW(VHOST_VIRTIO, 0x24, int)
> +
> +#define VHOST_THREAD_ID _IOR(VHOST_VIRTIO, 0x25, struct vhost_vring_thread)
> +
> /* VHOST_NET specific defines */
>
> /* Attach virtio net ring to a raw socket, or tap device.
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v1] vhost-user: fix not send all hugepage files to vhost-user
2015-02-01 23:29 ` Paolo Bonzini
@ 2015-02-03 13:09 ` Linhaifeng
2015-02-04 13:52 ` Michael S. Tsirkin
2015-02-15 12:07 ` Linhaifeng
2 siblings, 0 replies; 10+ messages in thread
From: Linhaifeng @ 2015-02-03 13:09 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel
Cc: milo.raofei, arei.gonglei, jerry.lilijun, n.nikolaev, mst
On 2015/2/2 7:29, Paolo Bonzini wrote:
>
>
> On 17/12/2014 07:02, haifeng.lin@huawei.com wrote:
>> From: linhaifeng <haifeng.lin@huawei.com>
>>
>> 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 <haifeng.lin@huawei.com>
>
> 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 <linux/vhost.h>
>>
>> #define VHOST_MEMORY_MAX_NREGIONS 8
>> +/* 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);
>> +
>> + qemu_mutex_unlock_ramlist();
>> + break;
>>
>> - if (!fd_num) {
>> - error_report("Failed initializing vhost-user memory map\n"
>> - "consider using -object memory-backend-file share=on\n");
>> - return -1;
>> + case VHOST_UNMAP_HUGEPAGE_FILE:
>> + /* Tell vhost-user to unmap all hugepage files. */
>> + break;
>> +
>> + case VHOST_SET_MEM_TABLE:
>> + for (i = 0; i < dev->mem->nregions; i++) {
>> + struct vhost_memory_region *reg = dev->mem->regions + i;
>> +
>> + assert((uintptr_t)reg->userspace_addr == reg->userspace_addr);
>> +
>> + msg.memory.regions[i].userspace_addr = reg->userspace_addr;
>> + msg.memory.regions[i].memory_size = reg->memory_size;
>> + msg.memory.regions[i].guest_phys_addr = reg->guest_phys_addr;
>> + assert(fd_num < VHOST_MEMORY_MAX_NREGIONS);
>> }
>>
>> + msg.memory.nregions = i;
>> msg.size = sizeof(m.memory.nregions);
>> msg.size += sizeof(m.memory.padding);
>> - msg.size += fd_num * sizeof(VhostUserMemoryRegion);
>> -
>> + msg.size += i * sizeof(VhostUserMemoryRegion);
>> break;
>>
>> case VHOST_SET_LOG_FD:
>> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
>> index 5a12861..b8eb341 100644
>> --- a/hw/virtio/vhost.c
>> +++ b/hw/virtio/vhost.c
>> @@ -1041,6 +1041,14 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev)
>> if (r < 0) {
>> goto fail_features;
>> }
>> + if (hdev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER) {
>> + r = hdev->vhost_ops->vhost_call(hdev, VHOST_MMAP_HUGEPAGE_FILE,
>> + NULL);
>> + if (r < 0) {
>> + r = -errno;
>> + goto fail_mem;
>> + }
>> + }
>> r = hdev->vhost_ops->vhost_call(hdev, VHOST_SET_MEM_TABLE, hdev->mem);
>> if (r < 0) {
>> r = -errno;
>> @@ -1101,5 +1109,10 @@ void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev)
>> g_free(hdev->log);
>> hdev->log = NULL;
>> hdev->log_size = 0;
>> +
>> + if (hdev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER) {
>> + (void)hdev->vhost_ops->vhost_call(hdev, VHOST_MMAP_HUGEPAGE_FILE,
>> + NULL);
>> + }
>> }
>>
>> diff --git a/linux-headers/linux/vhost.h b/linux-headers/linux/vhost.h
>> index c656f61..bb72811 100644
>> --- a/linux-headers/linux/vhost.h
>> +++ b/linux-headers/linux/vhost.h
>> @@ -113,6 +113,13 @@ struct vhost_memory {
>> /* Set eventfd to signal an error */
>> #define VHOST_SET_VRING_ERR _IOW(VHOST_VIRTIO, 0x22, struct vhost_vring_file)
>>
>> +/* Tell vhost-user to mmap hugepage file */
>> +#define VHOST_MMAP_HUGEPAGE_FILE _IOW(VHOST_VIRTIO, 0x23, int)
>> +/* Tell vhost-user to unmap hugepage file */
>> +#define VHOST_UNMAP_HUGEPAGE_FILE _IOW(VHOST_VIRTIO, 0x24, int)
>> +
>> +#define VHOST_THREAD_ID _IOR(VHOST_VIRTIO, 0x25, struct vhost_vring_thread)
>> +
>> /* VHOST_NET specific defines */
>>
>> /* Attach virtio net ring to a raw socket, or tap device.
>>
>
> .
>
--
Regards,
Haifeng
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v1] vhost-user: fix not send all hugepage files to vhost-user
2015-02-01 23:29 ` Paolo Bonzini
2015-02-03 13:09 ` Linhaifeng
@ 2015-02-04 13:52 ` Michael S. Tsirkin
2015-02-04 14:02 ` Paolo Bonzini
2015-02-15 12:07 ` Linhaifeng
2 siblings, 1 reply; 10+ messages in thread
From: Michael S. Tsirkin @ 2015-02-04 13:52 UTC (permalink / raw)
To: Paolo Bonzini
Cc: haifeng.lin, jerry.lilijun, n.nikolaev, qemu-devel, milo.raofei,
arei.gonglei
On Mon, Feb 02, 2015 at 12:29:47AM +0100, Paolo Bonzini wrote:
>
>
> On 17/12/2014 07:02, haifeng.lin@huawei.com wrote:
> > From: linhaifeng <haifeng.lin@huawei.com>
> >
> > 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 <haifeng.lin@huawei.com>
>
> 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 <linux/vhost.h>
> >
> > #define VHOST_MEMORY_MAX_NREGIONS 8
> > +/* 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);
> > +
> > + qemu_mutex_unlock_ramlist();
> > + break;
> >
> > - if (!fd_num) {
> > - error_report("Failed initializing vhost-user memory map\n"
> > - "consider using -object memory-backend-file share=on\n");
> > - return -1;
> > + case VHOST_UNMAP_HUGEPAGE_FILE:
> > + /* Tell vhost-user to unmap all hugepage files. */
> > + break;
> > +
> > + case VHOST_SET_MEM_TABLE:
> > + for (i = 0; i < dev->mem->nregions; i++) {
> > + struct vhost_memory_region *reg = dev->mem->regions + i;
> > +
> > + assert((uintptr_t)reg->userspace_addr == reg->userspace_addr);
> > +
> > + msg.memory.regions[i].userspace_addr = reg->userspace_addr;
> > + msg.memory.regions[i].memory_size = reg->memory_size;
> > + msg.memory.regions[i].guest_phys_addr = reg->guest_phys_addr;
> > + assert(fd_num < VHOST_MEMORY_MAX_NREGIONS);
> > }
> >
> > + msg.memory.nregions = i;
> > msg.size = sizeof(m.memory.nregions);
> > msg.size += sizeof(m.memory.padding);
> > - msg.size += fd_num * sizeof(VhostUserMemoryRegion);
> > -
> > + msg.size += i * sizeof(VhostUserMemoryRegion);
> > break;
> >
> > case VHOST_SET_LOG_FD:
> > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> > index 5a12861..b8eb341 100644
> > --- a/hw/virtio/vhost.c
> > +++ b/hw/virtio/vhost.c
> > @@ -1041,6 +1041,14 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev)
> > if (r < 0) {
> > goto fail_features;
> > }
> > + if (hdev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER) {
> > + r = hdev->vhost_ops->vhost_call(hdev, VHOST_MMAP_HUGEPAGE_FILE,
> > + NULL);
> > + if (r < 0) {
> > + r = -errno;
> > + goto fail_mem;
> > + }
> > + }
> > r = hdev->vhost_ops->vhost_call(hdev, VHOST_SET_MEM_TABLE, hdev->mem);
> > if (r < 0) {
> > r = -errno;
> > @@ -1101,5 +1109,10 @@ void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev)
> > g_free(hdev->log);
> > hdev->log = NULL;
> > hdev->log_size = 0;
> > +
> > + if (hdev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER) {
> > + (void)hdev->vhost_ops->vhost_call(hdev, VHOST_MMAP_HUGEPAGE_FILE,
> > + NULL);
> > + }
> > }
> >
> > diff --git a/linux-headers/linux/vhost.h b/linux-headers/linux/vhost.h
> > index c656f61..bb72811 100644
> > --- a/linux-headers/linux/vhost.h
> > +++ b/linux-headers/linux/vhost.h
> > @@ -113,6 +113,13 @@ struct vhost_memory {
> > /* Set eventfd to signal an error */
> > #define VHOST_SET_VRING_ERR _IOW(VHOST_VIRTIO, 0x22, struct vhost_vring_file)
> >
> > +/* Tell vhost-user to mmap hugepage file */
> > +#define VHOST_MMAP_HUGEPAGE_FILE _IOW(VHOST_VIRTIO, 0x23, int)
> > +/* Tell vhost-user to unmap hugepage file */
> > +#define VHOST_UNMAP_HUGEPAGE_FILE _IOW(VHOST_VIRTIO, 0x24, int)
> > +
> > +#define VHOST_THREAD_ID _IOR(VHOST_VIRTIO, 0x25, struct vhost_vring_thread)
> > +
> > /* VHOST_NET specific defines */
> >
> > /* Attach virtio net ring to a raw socket, or tap device.
> >
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v1] vhost-user: fix not send all hugepage files to vhost-user
2015-02-04 13:52 ` Michael S. Tsirkin
@ 2015-02-04 14:02 ` Paolo Bonzini
0 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2015-02-04 14:02 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: haifeng.lin, jerry.lilijun, n.nikolaev, qemu-devel, milo.raofei,
arei.gonglei
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v1] vhost-user: fix not send all hugepage files to vhost-user
2015-02-01 23:29 ` Paolo Bonzini
2015-02-03 13:09 ` Linhaifeng
2015-02-04 13:52 ` Michael S. Tsirkin
@ 2015-02-15 12:07 ` Linhaifeng
2 siblings, 0 replies; 10+ messages in thread
From: Linhaifeng @ 2015-02-15 12:07 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel
Cc: milo.raofei, arei.gonglei, jerry.lilijun, n.nikolaev, mst
On 2015/2/2 7:29, Paolo Bonzini wrote:
>
>
> On 17/12/2014 07:02, haifeng.lin@huawei.com wrote:
>> From: linhaifeng <haifeng.lin@huawei.com>
>>
>> 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 <haifeng.lin@huawei.com>
>
> 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=0x7f1ea7200000 size=1073741824
hugepage file fd=9 addr=0x7f1ee7200000 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=0x7f1ea72c0000 ram_addr=0xc0000 fd=8
reg->userspace_addr=0x7f1e9ee00000 ram_addr=0x80000000 fd=-1
reg->userspace_addr=0x7f1ea7200000 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.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-02-15 12:08 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-17 6:02 [Qemu-devel] [PATCH v1] vhost-user: fix not send all hugepage files to vhost-user haifeng.lin
2014-12-18 5:06 ` Linhaifeng
2015-01-29 3:58 ` Linhaifeng
2015-01-29 10:51 ` Michael S. Tsirkin
2015-01-29 13:02 ` Linhaifeng
2015-02-01 23:29 ` Paolo Bonzini
2015-02-03 13:09 ` Linhaifeng
2015-02-04 13:52 ` Michael S. Tsirkin
2015-02-04 14:02 ` Paolo Bonzini
2015-02-15 12:07 ` Linhaifeng
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).