subject:"\[Qemu\-devel\] \[PATCH v8 2\/2\] vhost\: used

Re: [Qemu-devel] [PATCH v8 2/2] vhost: used_memslots refactoring

2018-03-05 Thread Zhoujian (jay)



> -Original Message-
> From: Michael S. Tsirkin [mailto:m...@redhat.com]
> Sent: Friday, March 02, 2018 12:17 AM
> To: Zhoujian (jay) 
> Cc: qemu-devel@nongnu.org; imamm...@redhat.com; Huangweidong (C)
> ; wangxin (U) ; Gonglei
> (Arei) ; Liuzhe (Ahriy, Euler) 
> Subject: Re: [PATCH v8 2/2] vhost: used_memslots refactoring
> 
> On Tue, Feb 27, 2018 at 03:10:05PM +0800, Jay Zhou wrote:
> > Used_memslots is shared by vhost kernel and user, it is equal to
> > dev->mem->nregions, which is correct for vhost kernel, but not for
> > vhost user, the latter one uses memory regions that have file
> > descriptor. E.g. a VM has a vhost-user NIC and 8(vhost user memslot
> > upper limit) memory slots, it will be failed to hotplug a new DIMM
> > device since vhost_has_free_slot() finds no free slot left. It should
> > be successful if only part of memory slots have file descriptor, so
> > setting used memslots for vhost-user and vhost-kernel respectively.
> >
> > Signed-off-by: Igor Mammedov 
> > Signed-off-by: Jay Zhou 
> > Signed-off-by: Liuzhe 
> 
> make check fails with this patch, I dropped it for now.

Hi Michael, pls see the reason inline.

> 
> > ---
> >  hw/virtio/vhost-backend.c | 15 +++-
> >  hw/virtio/vhost-user.c| 77 ++-
> 
> >  hw/virtio/vhost.c | 13 +++
> >  include/hw/virtio/vhost-backend.h |  6 ++-
> >  4 files changed, 75 insertions(+), 36 deletions(-)
> >
> > diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
> > index 7f09efa..59def69 100644
> > --- a/hw/virtio/vhost-backend.c
> > +++ b/hw/virtio/vhost-backend.c
> > @@ -15,6 +15,8 @@
> >  #include "hw/virtio/vhost-backend.h"
> >  #include "qemu/error-report.h"
> >
> > +static unsigned int vhost_kernel_used_memslots;
> > +
> >  static int vhost_kernel_call(struct vhost_dev *dev, unsigned long int
> request,
> >   void *arg)  { @@ -62,6 +64,11 @@ static
> > int vhost_kernel_memslots_limit(struct vhost_dev *dev)
> >  return limit;
> >  }
> >
> > +static bool vhost_kernel_has_free_memslots(struct vhost_dev *dev) {
> > +return vhost_kernel_used_memslots <
> > +vhost_kernel_memslots_limit(dev); }
> > +
> >  static int vhost_kernel_net_set_backend(struct vhost_dev *dev,
> >  struct vhost_vring_file
> > *file)  { @@ -233,11 +240,16 @@ static void
> > vhost_kernel_set_iotlb_callback(struct vhost_dev *dev,
> >  qemu_set_fd_handler((uintptr_t)dev->opaque, NULL, NULL,
> > NULL);  }
> >
> > +static void vhost_kernel_set_used_memslots(struct vhost_dev *dev) {
> > +vhost_kernel_used_memslots = dev->mem->nregions; }
> > +
> >  static const VhostOps kernel_ops = {
> >  .backend_type = VHOST_BACKEND_TYPE_KERNEL,
> >  .vhost_backend_init = vhost_kernel_init,
> >  .vhost_backend_cleanup = vhost_kernel_cleanup,
> > -.vhost_backend_memslots_limit = vhost_kernel_memslots_limit,
> > +.vhost_backend_has_free_memslots =
> > + vhost_kernel_has_free_memslots,
> >  .vhost_net_set_backend = vhost_kernel_net_set_backend,
> >  .vhost_scsi_set_endpoint = vhost_kernel_scsi_set_endpoint,
> >  .vhost_scsi_clear_endpoint =
> > vhost_kernel_scsi_clear_endpoint, @@ -264,6 +276,7 @@ static const
> > VhostOps kernel_ops = {  #endif /* CONFIG_VHOST_VSOCK */
> >  .vhost_set_iotlb_callback = vhost_kernel_set_iotlb_callback,
> >  .vhost_send_device_iotlb_msg =
> > vhost_kernel_send_device_iotlb_msg,
> > +.vhost_set_used_memslots = vhost_kernel_set_used_memslots,
> >  };
> >
> >  int vhost_set_backend_type(struct vhost_dev *dev, VhostBackendType
> > backend_type) diff --git a/hw/virtio/vhost-user.c
> > b/hw/virtio/vhost-user.c index 6eb9798..f732c80 100644
> > --- a/hw/virtio/vhost-user.c
> > +++ b/hw/virtio/vhost-user.c
> > @@ -147,6 +147,8 @@ static VhostUserMsg m __attribute__ ((unused));
> >  /* The version of the protocol we support */
> >  #define VHOST_USER_VERSION(0x1)
> >
> > +static bool vhost_user_free_memslots = true;
> > +
> >  struct vhost_user {
> >  CharBackend *chr;
> >  int slave_fd;
> > @@ -314,12 +316,43 @@ static int vhost_user_set_log_base(struct vhost_dev
> *dev, uint64_t base,
> >  return 0;
> >  }
> >
> > +static int vhost_user_prepare_msg(struct vhost_dev *dev, VhostUserMemory
> *mem,
> > +  int *fds) {
> > +int i, fd;
> > +
> > +vhost_user_free_memslots = true;
> > +for (i = 0, mem->nregions = 0; i < dev->mem->nregions; ++i) {
> > +struct vhost_memory_region *reg = dev->mem->regions + i;
> > +ram_addr_t offset;
> > +MemoryRegion *mr;
> > +
> > +assert((uintptr_t)reg->userspace_addr == reg->userspace_addr);
> > +

Re: [Qemu-devel] [PATCH v8 2/2] vhost: used_memslots refactoring

2018-03-01 Thread Zhoujian (jay)



> -Original Message-
> From: Michael S. Tsirkin [mailto:m...@redhat.com]
> Sent: Friday, March 02, 2018 12:17 AM
> To: Zhoujian (jay) 
> Cc: qemu-devel@nongnu.org; imamm...@redhat.com; Huangweidong (C)
> ; wangxin (U) ; Gonglei
> (Arei) ; Liuzhe (Ahriy, Euler) 
> Subject: Re: [PATCH v8 2/2] vhost: used_memslots refactoring
> 
> On Tue, Feb 27, 2018 at 03:10:05PM +0800, Jay Zhou wrote:
> > Used_memslots is shared by vhost kernel and user, it is equal to
> > dev->mem->nregions, which is correct for vhost kernel, but not for
> > vhost user, the latter one uses memory regions that have file
> > descriptor. E.g. a VM has a vhost-user NIC and 8(vhost user memslot
> > upper limit) memory slots, it will be failed to hotplug a new DIMM
> > device since vhost_has_free_slot() finds no free slot left. It should
> > be successful if only part of memory slots have file descriptor, so
> > setting used memslots for vhost-user and vhost-kernel respectively.
> >
> > Signed-off-by: Igor Mammedov 
> > Signed-off-by: Jay Zhou 
> > Signed-off-by: Liuzhe 
> 
> make check fails with this patch, I dropped it for now.

Maybe something updated on the master tree affects this patch, will
look into and resolve.

Regards,
Jay

> 
> > ---
> >  hw/virtio/vhost-backend.c | 15 +++-
> >  hw/virtio/vhost-user.c| 77 ++-
> 
> >  hw/virtio/vhost.c | 13 +++
> >  include/hw/virtio/vhost-backend.h |  6 ++-
> >  4 files changed, 75 insertions(+), 36 deletions(-)
> >
> > diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
> > index 7f09efa..59def69 100644
> > --- a/hw/virtio/vhost-backend.c
> > +++ b/hw/virtio/vhost-backend.c
> > @@ -15,6 +15,8 @@
> >  #include "hw/virtio/vhost-backend.h"
> >  #include "qemu/error-report.h"
> >
> > +static unsigned int vhost_kernel_used_memslots;
> > +
> >  static int vhost_kernel_call(struct vhost_dev *dev, unsigned long int
> request,
> >   void *arg)  { @@ -62,6 +64,11 @@ static
> > int vhost_kernel_memslots_limit(struct vhost_dev *dev)
> >  return limit;
> >  }
> >
> > +static bool vhost_kernel_has_free_memslots(struct vhost_dev *dev) {
> > +return vhost_kernel_used_memslots <
> > +vhost_kernel_memslots_limit(dev); }
> > +
> >  static int vhost_kernel_net_set_backend(struct vhost_dev *dev,
> >  struct vhost_vring_file
> > *file)  { @@ -233,11 +240,16 @@ static void
> > vhost_kernel_set_iotlb_callback(struct vhost_dev *dev,
> >  qemu_set_fd_handler((uintptr_t)dev->opaque, NULL, NULL,
> > NULL);  }
> >
> > +static void vhost_kernel_set_used_memslots(struct vhost_dev *dev) {
> > +vhost_kernel_used_memslots = dev->mem->nregions; }
> > +
> >  static const VhostOps kernel_ops = {
> >  .backend_type = VHOST_BACKEND_TYPE_KERNEL,
> >  .vhost_backend_init = vhost_kernel_init,
> >  .vhost_backend_cleanup = vhost_kernel_cleanup,
> > -.vhost_backend_memslots_limit = vhost_kernel_memslots_limit,
> > +.vhost_backend_has_free_memslots =
> > + vhost_kernel_has_free_memslots,
> >  .vhost_net_set_backend = vhost_kernel_net_set_backend,
> >  .vhost_scsi_set_endpoint = vhost_kernel_scsi_set_endpoint,
> >  .vhost_scsi_clear_endpoint =
> > vhost_kernel_scsi_clear_endpoint, @@ -264,6 +276,7 @@ static const
> > VhostOps kernel_ops = {  #endif /* CONFIG_VHOST_VSOCK */
> >  .vhost_set_iotlb_callback = vhost_kernel_set_iotlb_callback,
> >  .vhost_send_device_iotlb_msg =
> > vhost_kernel_send_device_iotlb_msg,
> > +.vhost_set_used_memslots = vhost_kernel_set_used_memslots,
> >  };
> >
> >  int vhost_set_backend_type(struct vhost_dev *dev, VhostBackendType
> > backend_type) diff --git a/hw/virtio/vhost-user.c
> > b/hw/virtio/vhost-user.c index 6eb9798..f732c80 100644
> > --- a/hw/virtio/vhost-user.c
> > +++ b/hw/virtio/vhost-user.c
> > @@ -147,6 +147,8 @@ static VhostUserMsg m __attribute__ ((unused));
> >  /* The version of the protocol we support */
> >  #define VHOST_USER_VERSION(0x1)
> >
> > +static bool vhost_user_free_memslots = true;
> > +
> >  struct vhost_user {
> >  CharBackend *chr;
> >  int slave_fd;
> > @@ -314,12 +316,43 @@ static int vhost_user_set_log_base(struct vhost_dev
> *dev, uint64_t base,
> >  return 0;
> >  }
> >
> > +static int vhost_user_prepare_msg(struct vhost_dev *dev, VhostUserMemory
> *mem,
> > +  int *fds) {
> > +int i, fd;
> > +
> > +vhost_user_free_memslots = true;
> > +for (i = 0, mem->nregions = 0; i < dev->mem->nregions; ++i) {
> > +struct vhost_memory_region *reg = dev->mem->regions + i;
> > +ram_addr_t offset;
> > +MemoryRegion *mr;
> > +
> > +

Re: [Qemu-devel] [PATCH v8 2/2] vhost: used_memslots refactoring

2018-03-01 Thread Michael S. Tsirkin

On Tue, Feb 27, 2018 at 03:10:05PM +0800, Jay Zhou wrote:
> Used_memslots is shared by vhost kernel and user, it is equal to
> dev->mem->nregions, which is correct for vhost kernel, but not for
> vhost user, the latter one uses memory regions that have file
> descriptor. E.g. a VM has a vhost-user NIC and 8(vhost user memslot
> upper limit) memory slots, it will be failed to hotplug a new DIMM
> device since vhost_has_free_slot() finds no free slot left. It
> should be successful if only part of memory slots have file
> descriptor, so setting used memslots for vhost-user and
> vhost-kernel respectively.
> 
> Signed-off-by: Igor Mammedov 
> Signed-off-by: Jay Zhou 
> Signed-off-by: Liuzhe 

make check fails with this patch, I dropped it for now.

> ---
>  hw/virtio/vhost-backend.c | 15 +++-
>  hw/virtio/vhost-user.c| 77 
> ++-
>  hw/virtio/vhost.c | 13 +++
>  include/hw/virtio/vhost-backend.h |  6 ++-
>  4 files changed, 75 insertions(+), 36 deletions(-)
> 
> diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
> index 7f09efa..59def69 100644
> --- a/hw/virtio/vhost-backend.c
> +++ b/hw/virtio/vhost-backend.c
> @@ -15,6 +15,8 @@
>  #include "hw/virtio/vhost-backend.h"
>  #include "qemu/error-report.h"
>  
> +static unsigned int vhost_kernel_used_memslots;
> +
>  static int vhost_kernel_call(struct vhost_dev *dev, unsigned long int 
> request,
>   void *arg)
>  {
> @@ -62,6 +64,11 @@ static int vhost_kernel_memslots_limit(struct vhost_dev 
> *dev)
>  return limit;
>  }
>  
> +static bool vhost_kernel_has_free_memslots(struct vhost_dev *dev)
> +{
> +return vhost_kernel_used_memslots < vhost_kernel_memslots_limit(dev);
> +}
> +
>  static int vhost_kernel_net_set_backend(struct vhost_dev *dev,
>  struct vhost_vring_file *file)
>  {
> @@ -233,11 +240,16 @@ static void vhost_kernel_set_iotlb_callback(struct 
> vhost_dev *dev,
>  qemu_set_fd_handler((uintptr_t)dev->opaque, NULL, NULL, NULL);
>  }
>  
> +static void vhost_kernel_set_used_memslots(struct vhost_dev *dev)
> +{
> +vhost_kernel_used_memslots = dev->mem->nregions;
> +}
> +
>  static const VhostOps kernel_ops = {
>  .backend_type = VHOST_BACKEND_TYPE_KERNEL,
>  .vhost_backend_init = vhost_kernel_init,
>  .vhost_backend_cleanup = vhost_kernel_cleanup,
> -.vhost_backend_memslots_limit = vhost_kernel_memslots_limit,
> +.vhost_backend_has_free_memslots = vhost_kernel_has_free_memslots,
>  .vhost_net_set_backend = vhost_kernel_net_set_backend,
>  .vhost_scsi_set_endpoint = vhost_kernel_scsi_set_endpoint,
>  .vhost_scsi_clear_endpoint = vhost_kernel_scsi_clear_endpoint,
> @@ -264,6 +276,7 @@ static const VhostOps kernel_ops = {
>  #endif /* CONFIG_VHOST_VSOCK */
>  .vhost_set_iotlb_callback = vhost_kernel_set_iotlb_callback,
>  .vhost_send_device_iotlb_msg = vhost_kernel_send_device_iotlb_msg,
> +.vhost_set_used_memslots = vhost_kernel_set_used_memslots,
>  };
>  
>  int vhost_set_backend_type(struct vhost_dev *dev, VhostBackendType 
> backend_type)
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 6eb9798..f732c80 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -147,6 +147,8 @@ static VhostUserMsg m __attribute__ ((unused));
>  /* The version of the protocol we support */
>  #define VHOST_USER_VERSION(0x1)
>  
> +static bool vhost_user_free_memslots = true;
> +
>  struct vhost_user {
>  CharBackend *chr;
>  int slave_fd;
> @@ -314,12 +316,43 @@ static int vhost_user_set_log_base(struct vhost_dev 
> *dev, uint64_t base,
>  return 0;
>  }
>  
> +static int vhost_user_prepare_msg(struct vhost_dev *dev, VhostUserMemory 
> *mem,
> +  int *fds)
> +{
> +int i, fd;
> +
> +vhost_user_free_memslots = true;
> +for (i = 0, mem->nregions = 0; i < dev->mem->nregions; ++i) {
> +struct vhost_memory_region *reg = dev->mem->regions + i;
> +ram_addr_t offset;
> +MemoryRegion *mr;
> +
> +assert((uintptr_t)reg->userspace_addr == reg->userspace_addr);
> +mr = memory_region_from_host((void *)(uintptr_t)reg->userspace_addr,
> + );
> +fd = memory_region_get_fd(mr);
> +if (fd > 0) {
> +if (mem->nregions == VHOST_MEMORY_MAX_NREGIONS) {
> +vhost_user_free_memslots = false;
> +return -1;
> +}
> +
> +mem->regions[mem->nregions].userspace_addr = reg->userspace_addr;
> +mem->regions[mem->nregions].memory_size = reg->memory_size;
> +mem->regions[mem->nregions].guest_phys_addr = 
> reg->guest_phys_addr;
> +mem->regions[mem->nregions].mmap_offset = offset;
> +

[Qemu-devel] [PATCH v8 2/2] vhost: used_memslots refactoring

2018-02-26 Thread Jay Zhou

Used_memslots is shared by vhost kernel and user, it is equal to
dev->mem->nregions, which is correct for vhost kernel, but not for
vhost user, the latter one uses memory regions that have file
descriptor. E.g. a VM has a vhost-user NIC and 8(vhost user memslot
upper limit) memory slots, it will be failed to hotplug a new DIMM
device since vhost_has_free_slot() finds no free slot left. It
should be successful if only part of memory slots have file
descriptor, so setting used memslots for vhost-user and
vhost-kernel respectively.

Signed-off-by: Igor Mammedov 
Signed-off-by: Jay Zhou 
Signed-off-by: Liuzhe 
---
 hw/virtio/vhost-backend.c | 15 +++-
 hw/virtio/vhost-user.c| 77 ++-
 hw/virtio/vhost.c | 13 +++
 include/hw/virtio/vhost-backend.h |  6 ++-
 4 files changed, 75 insertions(+), 36 deletions(-)

diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
index 7f09efa..59def69 100644
--- a/hw/virtio/vhost-backend.c
+++ b/hw/virtio/vhost-backend.c
@@ -15,6 +15,8 @@
 #include "hw/virtio/vhost-backend.h"
 #include "qemu/error-report.h"
 
+static unsigned int vhost_kernel_used_memslots;
+
 static int vhost_kernel_call(struct vhost_dev *dev, unsigned long int request,
  void *arg)
 {
@@ -62,6 +64,11 @@ static int vhost_kernel_memslots_limit(struct vhost_dev *dev)
 return limit;
 }
 
+static bool vhost_kernel_has_free_memslots(struct vhost_dev *dev)
+{
+return vhost_kernel_used_memslots < vhost_kernel_memslots_limit(dev);
+}
+
 static int vhost_kernel_net_set_backend(struct vhost_dev *dev,
 struct vhost_vring_file *file)
 {
@@ -233,11 +240,16 @@ static void vhost_kernel_set_iotlb_callback(struct 
vhost_dev *dev,
 qemu_set_fd_handler((uintptr_t)dev->opaque, NULL, NULL, NULL);
 }
 
+static void vhost_kernel_set_used_memslots(struct vhost_dev *dev)
+{
+vhost_kernel_used_memslots = dev->mem->nregions;
+}
+
 static const VhostOps kernel_ops = {
 .backend_type = VHOST_BACKEND_TYPE_KERNEL,
 .vhost_backend_init = vhost_kernel_init,
 .vhost_backend_cleanup = vhost_kernel_cleanup,
-.vhost_backend_memslots_limit = vhost_kernel_memslots_limit,
+.vhost_backend_has_free_memslots = vhost_kernel_has_free_memslots,
 .vhost_net_set_backend = vhost_kernel_net_set_backend,
 .vhost_scsi_set_endpoint = vhost_kernel_scsi_set_endpoint,
 .vhost_scsi_clear_endpoint = vhost_kernel_scsi_clear_endpoint,
@@ -264,6 +276,7 @@ static const VhostOps kernel_ops = {
 #endif /* CONFIG_VHOST_VSOCK */
 .vhost_set_iotlb_callback = vhost_kernel_set_iotlb_callback,
 .vhost_send_device_iotlb_msg = vhost_kernel_send_device_iotlb_msg,
+.vhost_set_used_memslots = vhost_kernel_set_used_memslots,
 };
 
 int vhost_set_backend_type(struct vhost_dev *dev, VhostBackendType 
backend_type)
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 6eb9798..f732c80 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -147,6 +147,8 @@ static VhostUserMsg m __attribute__ ((unused));
 /* The version of the protocol we support */
 #define VHOST_USER_VERSION(0x1)
 
+static bool vhost_user_free_memslots = true;
+
 struct vhost_user {
 CharBackend *chr;
 int slave_fd;
@@ -314,12 +316,43 @@ static int vhost_user_set_log_base(struct vhost_dev *dev, 
uint64_t base,
 return 0;
 }
 
+static int vhost_user_prepare_msg(struct vhost_dev *dev, VhostUserMemory *mem,
+  int *fds)
+{
+int i, fd;
+
+vhost_user_free_memslots = true;
+for (i = 0, mem->nregions = 0; i < dev->mem->nregions; ++i) {
+struct vhost_memory_region *reg = dev->mem->regions + i;
+ram_addr_t offset;
+MemoryRegion *mr;
+
+assert((uintptr_t)reg->userspace_addr == reg->userspace_addr);
+mr = memory_region_from_host((void *)(uintptr_t)reg->userspace_addr,
+ );
+fd = memory_region_get_fd(mr);
+if (fd > 0) {
+if (mem->nregions == VHOST_MEMORY_MAX_NREGIONS) {
+vhost_user_free_memslots = false;
+return -1;
+}
+
+mem->regions[mem->nregions].userspace_addr = reg->userspace_addr;
+mem->regions[mem->nregions].memory_size = reg->memory_size;
+mem->regions[mem->nregions].guest_phys_addr = reg->guest_phys_addr;
+mem->regions[mem->nregions].mmap_offset = offset;
+fds[mem->nregions++] = fd;
+}
+}
+
+return 0;
+}
+
 static int vhost_user_set_mem_table(struct vhost_dev *dev,
 struct vhost_memory *mem)
 {
 int fds[VHOST_MEMORY_MAX_NREGIONS];
-int i, fd;
-size_t fd_num = 0;
+size_t fd_num;
 bool reply_supported =

Re: [Qemu-devel] [PATCH v8 2/2] vhost: used_memslots refactoring

Re: [Qemu-devel] [PATCH v8 2/2] vhost: used_memslots refactoring

Re: [Qemu-devel] [PATCH v8 2/2] vhost: used_memslots refactoring

[Qemu-devel] [PATCH v8 2/2] vhost: used_memslots refactoring

4 matches

Site Navigation

Mail list logo

Footer information