Re: [Qemu-devel] [PATCH v3 21/29] vhost+postcopy: Add vhost waker

2018-03-05 Thread Peter Xu 
On Mon, Mar 05, 2018 at 08:16:44PM +, Dr. David Alan Gilbert wrote:
> * Peter Xu (pet...@redhat.com) wrote:
> > On Fri, Feb 16, 2018 at 01:16:17PM +, Dr. David Alan Gilbert (git) 
> > wrote:
> > > From: "Dr. David Alan Gilbert" 
> > > 
> > > Register a waker function in vhost-user code to be notified when
> > > pages arrive or requests to previously mapped pages get requested.
> > > 
> > > Signed-off-by: Dr. David Alan Gilbert 
> > > ---
> > >  hw/virtio/trace-events |  3 +++
> > >  hw/virtio/vhost-user.c | 30 ++
> > >  2 files changed, 33 insertions(+)
> > > 
> > > diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> > > index 3afd12cfea..fe5e0ff856 100644
> > > --- a/hw/virtio/trace-events
> > > +++ b/hw/virtio/trace-events
> > > @@ -13,6 +13,9 @@ vhost_user_postcopy_fault_handler_found(int i, uint64_t 
> > > region_offset, uint64_t
> > >  vhost_user_postcopy_listen(void) ""
> > >  vhost_user_set_mem_table_postcopy(uint64_t client_addr, uint64_t qhva, 
> > > int reply_i, int region_i) "client:0x%"PRIx64" for hva: 0x%"PRIx64" reply 
> > > %d region %d"
> > >  vhost_user_set_mem_table_withfd(int index, const char *name, uint64_t 
> > > memory_size, uint64_t guest_phys_addr, uint64_t userspace_addr, uint64_t 
> > > offset) "%d:%s: size:0x%"PRIx64" GPA:0x%"PRIx64" 
> > > QVA/userspace:0x%"PRIx64" RB offset:0x%"PRIx64
> > > +vhost_user_postcopy_waker(const char *rb, uint64_t rb_offset) "%s + 
> > > 0x%"PRIx64
> > > +vhost_user_postcopy_waker_found(uint64_t client_addr) "0x%"PRIx64
> > > +vhost_user_postcopy_waker_nomatch(const char *rb, uint64_t rb_offset) 
> > > "%s + 0x%"PRIx64
> > >  
> > >  # hw/virtio/virtio.c
> > >  virtqueue_alloc_element(void *elem, size_t sz, unsigned in_num, unsigned 
> > > out_num) "elem %p size %zd in_num %u out_num %u"
> > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> > > index 4589bfd92e..74807091a0 100644
> > > --- a/hw/virtio/vhost-user.c
> > > +++ b/hw/virtio/vhost-user.c
> > > @@ -990,6 +990,35 @@ static int vhost_user_postcopy_fault_handler(struct 
> > > PostCopyFD *pcfd,
> > >  return -1;
> > >  }
> > >  
> > > +static int vhost_user_postcopy_waker(struct PostCopyFD *pcfd, RAMBlock 
> > > *rb,
> > > + uint64_t offset)
> > > +{
> > > +struct vhost_dev *dev = pcfd->data;
> > > +struct vhost_user *u = dev->opaque;
> > > +int i;
> > > +
> > > +trace_vhost_user_postcopy_waker(qemu_ram_get_idstr(rb), offset);
> > > +
> > > +if (!u) {
> > > +return 0;
> > > +}
> > > +/* Translate the offset into an address in the clients address space 
> > > */
> > > +for (i = 0; i < MIN(dev->mem->nregions, u->region_rb_len); i++) {
> > > +if (u->region_rb[i] == rb &&
> > > +offset >= u->region_rb_offset[i] &&
> > > +offset < (u->region_rb_offset[i] +
> > > +  dev->mem->regions[i].memory_size)) {
> > > +uint64_t client_addr = (offset - u->region_rb_offset[i]) +
> > > +   u->postcopy_client_bases[i];
> > > +trace_vhost_user_postcopy_waker_found(client_addr);
> > > +return postcopy_wake_shared(pcfd, client_addr, rb);
> > > +}
> > > +}
> > > +
> > > +trace_vhost_user_postcopy_waker_nomatch(qemu_ram_get_idstr(rb), 
> > > offset);
> > > +return 0;
> > 
> > Can we really reach here?
> 
> Yes; note that all the waker's registered get called for all pages
> received
> so that:
>   a) A page not in shared memory, or not actually registered with a
> device, still calls the waker's and it's upto the waker to figure out
> whether it's interested for the device it belongs to.
> 
>   b) With two devices registered, they might each have registered
> different areas of shared memory, and thus it's upto the waker to figure
> out if it's interested in this specific page.

Indeed.

Again, if we note down faulted addresses for reach PostcopyFD, IMHO we
can even ignore this check, since if the copied page covers any of the
faulted addresses of the FD we'll definitely need to send the wake,
otherwise we don't need to.  But current patch is also okay to me now.

Thanks,

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v3 21/29] vhost+postcopy: Add vhost waker

2018-03-05 Thread Dr. David Alan Gilbert 
* Peter Xu (pet...@redhat.com) wrote:
> On Fri, Feb 16, 2018 at 01:16:17PM +, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" 
> > 
> > Register a waker function in vhost-user code to be notified when
> > pages arrive or requests to previously mapped pages get requested.
> > 
> > Signed-off-by: Dr. David Alan Gilbert 
> > ---
> >  hw/virtio/trace-events |  3 +++
> >  hw/virtio/vhost-user.c | 30 ++
> >  2 files changed, 33 insertions(+)
> > 
> > diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> > index 3afd12cfea..fe5e0ff856 100644
> > --- a/hw/virtio/trace-events
> > +++ b/hw/virtio/trace-events
> > @@ -13,6 +13,9 @@ vhost_user_postcopy_fault_handler_found(int i, uint64_t 
> > region_offset, uint64_t
> >  vhost_user_postcopy_listen(void) ""
> >  vhost_user_set_mem_table_postcopy(uint64_t client_addr, uint64_t qhva, int 
> > reply_i, int region_i) "client:0x%"PRIx64" for hva: 0x%"PRIx64" reply %d 
> > region %d"
> >  vhost_user_set_mem_table_withfd(int index, const char *name, uint64_t 
> > memory_size, uint64_t guest_phys_addr, uint64_t userspace_addr, uint64_t 
> > offset) "%d:%s: size:0x%"PRIx64" GPA:0x%"PRIx64" QVA/userspace:0x%"PRIx64" 
> > RB offset:0x%"PRIx64
> > +vhost_user_postcopy_waker(const char *rb, uint64_t rb_offset) "%s + 
> > 0x%"PRIx64
> > +vhost_user_postcopy_waker_found(uint64_t client_addr) "0x%"PRIx64
> > +vhost_user_postcopy_waker_nomatch(const char *rb, uint64_t rb_offset) "%s 
> > + 0x%"PRIx64
> >  
> >  # hw/virtio/virtio.c
> >  virtqueue_alloc_element(void *elem, size_t sz, unsigned in_num, unsigned 
> > out_num) "elem %p size %zd in_num %u out_num %u"
> > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> > index 4589bfd92e..74807091a0 100644
> > --- a/hw/virtio/vhost-user.c
> > +++ b/hw/virtio/vhost-user.c
> > @@ -990,6 +990,35 @@ static int vhost_user_postcopy_fault_handler(struct 
> > PostCopyFD *pcfd,
> >  return -1;
> >  }
> >  
> > +static int vhost_user_postcopy_waker(struct PostCopyFD *pcfd, RAMBlock *rb,
> > + uint64_t offset)
> > +{
> > +struct vhost_dev *dev = pcfd->data;
> > +struct vhost_user *u = dev->opaque;
> > +int i;
> > +
> > +trace_vhost_user_postcopy_waker(qemu_ram_get_idstr(rb), offset);
> > +
> > +if (!u) {
> > +return 0;
> > +}
> > +/* Translate the offset into an address in the clients address space */
> > +for (i = 0; i < MIN(dev->mem->nregions, u->region_rb_len); i++) {
> > +if (u->region_rb[i] == rb &&
> > +offset >= u->region_rb_offset[i] &&
> > +offset < (u->region_rb_offset[i] +
> > +  dev->mem->regions[i].memory_size)) {
> > +uint64_t client_addr = (offset - u->region_rb_offset[i]) +
> > +   u->postcopy_client_bases[i];
> > +trace_vhost_user_postcopy_waker_found(client_addr);
> > +return postcopy_wake_shared(pcfd, client_addr, rb);
> > +}
> > +}
> > +
> > +trace_vhost_user_postcopy_waker_nomatch(qemu_ram_get_idstr(rb), 
> > offset);
> > +return 0;
> 
> Can we really reach here?

Yes; note that all the waker's registered get called for all pages
received
so that:
  a) A page not in shared memory, or not actually registered with a
device, still calls the waker's and it's upto the waker to figure out
whether it's interested for the device it belongs to.

  b) With two devices registered, they might each have registered
different areas of shared memory, and thus it's upto the waker to figure
out if it's interested in this specific page.

Dave

> > +}
> > +
> >  /*
> >   * Called at the start of an inbound postcopy on reception of the
> >   * 'advise' command.
> > @@ -1035,6 +1064,7 @@ static int vhost_user_postcopy_advise(struct 
> > vhost_dev *dev, Error **errp)
> >  u->postcopy_fd.fd = ufd;
> >  u->postcopy_fd.data = dev;
> >  u->postcopy_fd.handler = vhost_user_postcopy_fault_handler;
> > +u->postcopy_fd.waker = vhost_user_postcopy_waker;
> >  u->postcopy_fd.idstr = "vhost-user"; /* Need to find unique name */
> >  postcopy_register_shared_ufd(>postcopy_fd);
> >  return 0;
> > -- 
> > 2.14.3
> > 
> 
> Thanks,
> 
> -- 
> Peter Xu
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v3 21/29] vhost+postcopy: Add vhost waker

2018-03-01 Thread Peter Xu 
On Fri, Feb 16, 2018 at 01:16:17PM +, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> Register a waker function in vhost-user code to be notified when
> pages arrive or requests to previously mapped pages get requested.
> 
> Signed-off-by: Dr. David Alan Gilbert 
> ---
>  hw/virtio/trace-events |  3 +++
>  hw/virtio/vhost-user.c | 30 ++
>  2 files changed, 33 insertions(+)
> 
> diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> index 3afd12cfea..fe5e0ff856 100644
> --- a/hw/virtio/trace-events
> +++ b/hw/virtio/trace-events
> @@ -13,6 +13,9 @@ vhost_user_postcopy_fault_handler_found(int i, uint64_t 
> region_offset, uint64_t
>  vhost_user_postcopy_listen(void) ""
>  vhost_user_set_mem_table_postcopy(uint64_t client_addr, uint64_t qhva, int 
> reply_i, int region_i) "client:0x%"PRIx64" for hva: 0x%"PRIx64" reply %d 
> region %d"
>  vhost_user_set_mem_table_withfd(int index, const char *name, uint64_t 
> memory_size, uint64_t guest_phys_addr, uint64_t userspace_addr, uint64_t 
> offset) "%d:%s: size:0x%"PRIx64" GPA:0x%"PRIx64" QVA/userspace:0x%"PRIx64" RB 
> offset:0x%"PRIx64
> +vhost_user_postcopy_waker(const char *rb, uint64_t rb_offset) "%s + 
> 0x%"PRIx64
> +vhost_user_postcopy_waker_found(uint64_t client_addr) "0x%"PRIx64
> +vhost_user_postcopy_waker_nomatch(const char *rb, uint64_t rb_offset) "%s + 
> 0x%"PRIx64
>  
>  # hw/virtio/virtio.c
>  virtqueue_alloc_element(void *elem, size_t sz, unsigned in_num, unsigned 
> out_num) "elem %p size %zd in_num %u out_num %u"
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 4589bfd92e..74807091a0 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -990,6 +990,35 @@ static int vhost_user_postcopy_fault_handler(struct 
> PostCopyFD *pcfd,
>  return -1;
>  }
>  
> +static int vhost_user_postcopy_waker(struct PostCopyFD *pcfd, RAMBlock *rb,
> + uint64_t offset)
> +{
> +struct vhost_dev *dev = pcfd->data;
> +struct vhost_user *u = dev->opaque;
> +int i;
> +
> +trace_vhost_user_postcopy_waker(qemu_ram_get_idstr(rb), offset);
> +
> +if (!u) {
> +return 0;
> +}
> +/* Translate the offset into an address in the clients address space */
> +for (i = 0; i < MIN(dev->mem->nregions, u->region_rb_len); i++) {
> +if (u->region_rb[i] == rb &&
> +offset >= u->region_rb_offset[i] &&
> +offset < (u->region_rb_offset[i] +
> +  dev->mem->regions[i].memory_size)) {
> +uint64_t client_addr = (offset - u->region_rb_offset[i]) +
> +   u->postcopy_client_bases[i];
> +trace_vhost_user_postcopy_waker_found(client_addr);
> +return postcopy_wake_shared(pcfd, client_addr, rb);
> +}
> +}
> +
> +trace_vhost_user_postcopy_waker_nomatch(qemu_ram_get_idstr(rb), offset);
> +return 0;

Can we really reach here?

> +}
> +
>  /*
>   * Called at the start of an inbound postcopy on reception of the
>   * 'advise' command.
> @@ -1035,6 +1064,7 @@ static int vhost_user_postcopy_advise(struct vhost_dev 
> *dev, Error **errp)
>  u->postcopy_fd.fd = ufd;
>  u->postcopy_fd.data = dev;
>  u->postcopy_fd.handler = vhost_user_postcopy_fault_handler;
> +u->postcopy_fd.waker = vhost_user_postcopy_waker;
>  u->postcopy_fd.idstr = "vhost-user"; /* Need to find unique name */
>  postcopy_register_shared_ufd(>postcopy_fd);
>  return 0;
> -- 
> 2.14.3
> 

Thanks,

-- 
Peter Xu

[Qemu-devel] [PATCH v3 21/29] vhost+postcopy: Add vhost waker

2018-02-16 Thread Dr. David Alan Gilbert (git) 
From: "Dr. David Alan Gilbert" 

Register a waker function in vhost-user code to be notified when
pages arrive or requests to previously mapped pages get requested.

Signed-off-by: Dr. David Alan Gilbert 
---
 hw/virtio/trace-events |  3 +++
 hw/virtio/vhost-user.c | 30 ++
 2 files changed, 33 insertions(+)

diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 3afd12cfea..fe5e0ff856 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -13,6 +13,9 @@ vhost_user_postcopy_fault_handler_found(int i, uint64_t 
region_offset, uint64_t
 vhost_user_postcopy_listen(void) ""
 vhost_user_set_mem_table_postcopy(uint64_t client_addr, uint64_t qhva, int 
reply_i, int region_i) "client:0x%"PRIx64" for hva: 0x%"PRIx64" reply %d region 
%d"
 vhost_user_set_mem_table_withfd(int index, const char *name, uint64_t 
memory_size, uint64_t guest_phys_addr, uint64_t userspace_addr, uint64_t 
offset) "%d:%s: size:0x%"PRIx64" GPA:0x%"PRIx64" QVA/userspace:0x%"PRIx64" RB 
offset:0x%"PRIx64
+vhost_user_postcopy_waker(const char *rb, uint64_t rb_offset) "%s + 0x%"PRIx64
+vhost_user_postcopy_waker_found(uint64_t client_addr) "0x%"PRIx64
+vhost_user_postcopy_waker_nomatch(const char *rb, uint64_t rb_offset) "%s + 
0x%"PRIx64
 
 # hw/virtio/virtio.c
 virtqueue_alloc_element(void *elem, size_t sz, unsigned in_num, unsigned 
out_num) "elem %p size %zd in_num %u out_num %u"
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 4589bfd92e..74807091a0 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -990,6 +990,35 @@ static int vhost_user_postcopy_fault_handler(struct 
PostCopyFD *pcfd,
 return -1;
 }
 
+static int vhost_user_postcopy_waker(struct PostCopyFD *pcfd, RAMBlock *rb,
+ uint64_t offset)
+{
+struct vhost_dev *dev = pcfd->data;
+struct vhost_user *u = dev->opaque;
+int i;
+
+trace_vhost_user_postcopy_waker(qemu_ram_get_idstr(rb), offset);
+
+if (!u) {
+return 0;
+}
+/* Translate the offset into an address in the clients address space */
+for (i = 0; i < MIN(dev->mem->nregions, u->region_rb_len); i++) {
+if (u->region_rb[i] == rb &&
+offset >= u->region_rb_offset[i] &&
+offset < (u->region_rb_offset[i] +
+  dev->mem->regions[i].memory_size)) {
+uint64_t client_addr = (offset - u->region_rb_offset[i]) +
+   u->postcopy_client_bases[i];
+trace_vhost_user_postcopy_waker_found(client_addr);
+return postcopy_wake_shared(pcfd, client_addr, rb);
+}
+}
+
+trace_vhost_user_postcopy_waker_nomatch(qemu_ram_get_idstr(rb), offset);
+return 0;
+}
+
 /*
  * Called at the start of an inbound postcopy on reception of the
  * 'advise' command.
@@ -1035,6 +1064,7 @@ static int vhost_user_postcopy_advise(struct vhost_dev 
*dev, Error **errp)
 u->postcopy_fd.fd = ufd;
 u->postcopy_fd.data = dev;
 u->postcopy_fd.handler = vhost_user_postcopy_fault_handler;
+u->postcopy_fd.waker = vhost_user_postcopy_waker;
 u->postcopy_fd.idstr = "vhost-user"; /* Need to find unique name */
 postcopy_register_shared_ufd(>postcopy_fd);
 return 0;
-- 
2.14.3