Message ID | 20170824192730.8440-16-dgilbert@redhat.com |
---|---|
State | New |
Headers | show |
Use "libvhost-user: " commit title tag/prefix? On Thu, Aug 24, 2017 at 12:27 PM, Dr. David Alan Gilbert (git) <dgilbert@redhat.com> wrote: > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > When new regions are sent to the client using SET_MEM_TABLE, register > them with the userfaultfd. > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- > contrib/libvhost-user/libvhost-user.c | 32 ++++++++++++++++++++++++++++++++ > 1 file changed, 32 insertions(+) > > diff --git a/contrib/libvhost-user/libvhost-user.c b/contrib/libvhost-user/libvhost-user.c > index e8accf11db..e6ab059a03 100644 > --- a/contrib/libvhost-user/libvhost-user.c > +++ b/contrib/libvhost-user/libvhost-user.c > @@ -449,6 +449,38 @@ vu_set_mem_table_exec(VuDev *dev, VhostUserMsg *vmsg) > dev_region->mmap_addr); > } > > + if (dev->postcopy_listening) { > + /* We should already have an open ufd need to mark each memory > + * range as ufd. > + * Note: Do we need any madvises? Well it's not been accessed > + * yet, still probably need no THP to be safe, discard to be safe? > + */ > + struct uffdio_register reg_struct; > + reg_struct.range.start = (uintptr_t)dev_region->mmap_addr; > + reg_struct.range.len = dev_region->size + dev_region->mmap_offset; > + reg_struct.mode = UFFDIO_REGISTER_MODE_MISSING; > + > + if (ioctl(dev->postcopy_ufd, UFFDIO_REGISTER, ®_struct)) { > + vu_panic(dev, "%s: Failed to userfault region %d " > + "@%p + %zx: (ufd=%d)%s\n", > + __func__, i, > + dev_region->mmap_addr, > + dev_region->size + dev_region->mmap_offset, > + dev->postcopy_ufd, strerror(errno)); > + continue; panic is supposed to be unrecoverable errors, so I would suggest to return here > + } > + if (!(reg_struct.ioctls & ((__u64)1 << _UFFDIO_COPY))) { > + vu_panic(dev, "%s Region (%d) doesn't support COPY", > + __func__, i); > + continue; > + } > + DPRINT("%s: region %d: Registered userfault for %llx + %llx\n", > + __func__, i, reg_struct.range.start, reg_struct.range.len); > + /* TODO: Stash 'zero' support flags somewhere */ > + /* TODO: Get address back to QEMU */ > + > + } > + > close(vmsg->fds[i]); > } This patch would be nicer if it compiles on !Linux / without userfault.
* Marc-André Lureau (marcandre.lureau@gmail.com) wrote: > Use "libvhost-user: " commit title tag/prefix? > > On Thu, Aug 24, 2017 at 12:27 PM, Dr. David Alan Gilbert (git) > <dgilbert@redhat.com> wrote: > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > > > When new regions are sent to the client using SET_MEM_TABLE, register > > them with the userfaultfd. > > > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > > --- > > contrib/libvhost-user/libvhost-user.c | 32 ++++++++++++++++++++++++++++++++ > > 1 file changed, 32 insertions(+) > > > > diff --git a/contrib/libvhost-user/libvhost-user.c b/contrib/libvhost-user/libvhost-user.c > > index e8accf11db..e6ab059a03 100644 > > --- a/contrib/libvhost-user/libvhost-user.c > > +++ b/contrib/libvhost-user/libvhost-user.c > > @@ -449,6 +449,38 @@ vu_set_mem_table_exec(VuDev *dev, VhostUserMsg *vmsg) > > dev_region->mmap_addr); > > } > > > > + if (dev->postcopy_listening) { > > + /* We should already have an open ufd need to mark each memory > > + * range as ufd. > > + * Note: Do we need any madvises? Well it's not been accessed > > + * yet, still probably need no THP to be safe, discard to be safe? > > + */ > > + struct uffdio_register reg_struct; > > + reg_struct.range.start = (uintptr_t)dev_region->mmap_addr; > > + reg_struct.range.len = dev_region->size + dev_region->mmap_offset; > > + reg_struct.mode = UFFDIO_REGISTER_MODE_MISSING; > > + > > + if (ioctl(dev->postcopy_ufd, UFFDIO_REGISTER, ®_struct)) { > > + vu_panic(dev, "%s: Failed to userfault region %d " > > + "@%p + %zx: (ufd=%d)%s\n", > > + __func__, i, > > + dev_region->mmap_addr, > > + dev_region->size + dev_region->mmap_offset, > > + dev->postcopy_ufd, strerror(errno)); > > + continue; > > panic is supposed to be unrecoverable errors, so I would suggest to return here Done. > > + } > > + if (!(reg_struct.ioctls & ((__u64)1 << _UFFDIO_COPY))) { > > + vu_panic(dev, "%s Region (%d) doesn't support COPY", > > + __func__, i); > > + continue; > > + } > > + DPRINT("%s: region %d: Registered userfault for %llx + %llx\n", > > + __func__, i, reg_struct.range.start, reg_struct.range.len); > > + /* TODO: Stash 'zero' support flags somewhere */ > > + /* TODO: Get address back to QEMU */ > > + > > + } > > + > > close(vmsg->fds[i]); > > } > > This patch would be nicer if it compiles on !Linux / without userfault. Done; I've just ifdef UFFDIO_REGISTER the inside of this if; I'll add other code that makes sure it doesn't get as far as setting postcopy_listening either. Dave > -- > Marc-André Lureau -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
diff --git a/contrib/libvhost-user/libvhost-user.c b/contrib/libvhost-user/libvhost-user.c index e8accf11db..e6ab059a03 100644 --- a/contrib/libvhost-user/libvhost-user.c +++ b/contrib/libvhost-user/libvhost-user.c @@ -449,6 +449,38 @@ vu_set_mem_table_exec(VuDev *dev, VhostUserMsg *vmsg) dev_region->mmap_addr); } + if (dev->postcopy_listening) { + /* We should already have an open ufd need to mark each memory + * range as ufd. + * Note: Do we need any madvises? Well it's not been accessed + * yet, still probably need no THP to be safe, discard to be safe? + */ + struct uffdio_register reg_struct; + reg_struct.range.start = (uintptr_t)dev_region->mmap_addr; + reg_struct.range.len = dev_region->size + dev_region->mmap_offset; + reg_struct.mode = UFFDIO_REGISTER_MODE_MISSING; + + if (ioctl(dev->postcopy_ufd, UFFDIO_REGISTER, ®_struct)) { + vu_panic(dev, "%s: Failed to userfault region %d " + "@%p + %zx: (ufd=%d)%s\n", + __func__, i, + dev_region->mmap_addr, + dev_region->size + dev_region->mmap_offset, + dev->postcopy_ufd, strerror(errno)); + continue; + } + if (!(reg_struct.ioctls & ((__u64)1 << _UFFDIO_COPY))) { + vu_panic(dev, "%s Region (%d) doesn't support COPY", + __func__, i); + continue; + } + DPRINT("%s: region %d: Registered userfault for %llx + %llx\n", + __func__, i, reg_struct.range.start, reg_struct.range.len); + /* TODO: Stash 'zero' support flags somewhere */ + /* TODO: Get address back to QEMU */ + + } + close(vmsg->fds[i]); }