From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Ostrovsky Subject: Re: [PATCH v2] introduce grant copy for user land Date: Fri, 05 Dec 2014 13:05:37 -0500 Message-ID: <5481F3F1.2010701@oracle.com> References: <1417536806-22562-1-git-send-email-thanos.makatos@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1XwxET-0003jg-Bx for xen-devel@lists.xenproject.org; Fri, 05 Dec 2014 18:03:57 +0000 In-Reply-To: <1417536806-22562-1-git-send-email-thanos.makatos@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Thanos Makatos , xen-devel@lists.xenproject.org Cc: david.vrabel@citrix.com List-Id: xen-devel@lists.xenproject.org On 12/02/2014 11:13 AM, Thanos Makatos wrote: > This patch introduces the interface to allow user-space applications > execute grant-copy operations. This is done by sending an ioctl to the > grant device. > > Signed-off-by: Thanos Makatos > --- > drivers/xen/gntdev.c | 171 +++++++++++++++++++++++++++++++++++++++++++++ > include/uapi/xen/gntdev.h | 69 ++++++++++++++++++ > 2 files changed, 240 insertions(+) > > diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c > index 51f4c95..7b4a8e0 100644 > --- a/drivers/xen/gntdev.c > +++ b/drivers/xen/gntdev.c > @@ -705,6 +705,174 @@ static long gntdev_ioctl_notify(struct gntdev_priv *priv, void __user *u) > return rc; > } > > +static int gntdev_gcopy_batch(int nr_segments, unsigned long gcopy_cb, > + struct gntdev_grant_copy_segment __user *__segments, int dir, > + int src, int dst) { > + > + static const int batch_size = PAGE_SIZE / (sizeof(struct page*) + > + sizeof(struct gnttab_copy) + sizeof(struct gntdev_grant_copy_segment)); > + struct page **pages = (struct page **)gcopy_cb; > + struct gnttab_copy *batch = (struct gnttab_copy *)((unsigned long)pages + > + sizeof(struct page*) * batch_size); > + struct gntdev_grant_copy_segment *segments = > + (struct gntdev_grant_copy_segment *)((unsigned long)batch + > + sizeof(struct gnttab_copy) * batch_size); > + unsigned int nr_pinned = 0, nr_segs2cp = 0; > + int err = 0, i; > + const int write = dir == GNTCOPY_IOCTL_g2s; > + > + nr_segments = min(nr_segments, batch_size); > + > + if (unlikely(copy_from_user(segments, __segments, > + sizeof(struct gntdev_grant_copy_segment) * nr_segments))) { > + pr_debug("failed to copy %d segments from user", nr_segments); > + err = -EFAULT; > + goto out; > + } > + > + for (i = 0; i < nr_segments; i++) { > + > + xen_pfn_t pgaddr; > + unsigned long start, offset; > + struct gntdev_grant_copy_segment *seg = &segments[i]; > + > + if (dir == GNTCOPY_IOCTL_s2g || dir == GNTCOPY_IOCTL_g2s) { > + > + start = (unsigned long)seg->self.iov.iov_base & PAGE_MASK; > + offset = (unsigned long)seg->self.iov.iov_base & ~PAGE_MASK; > + if (unlikely(offset + seg->self.iov.iov_len > PAGE_SIZE)) { > + pr_warn("segments crossing page boundaries not yet " > + "implemented\n"); > + err = -ENOSYS; > + goto out; > + } > + > + err = get_user_pages_fast(start, 1, write, &pages[i]); > + if (unlikely(err != 1)) { > + pr_debug("failed to get user page %lu", start); > + err = -EFAULT; > + goto out; > + } > + > + nr_pinned++; > + > + pgaddr = pfn_to_mfn(page_to_pfn(pages[i])); > + } > + > + nr_segs2cp++; > + > + switch (dir) { > + case GNTCOPY_IOCTL_g2s: /* copy from guest */ > + batch[i].len = seg->self.iov.iov_len; > + batch[i].source.u.ref = seg->self.ref; > + batch[i].source.domid = src; > + batch[i].source.offset = seg->self.offset; > + batch[i].dest.u.gmfn = pgaddr; > + batch[i].dest.domid = DOMID_SELF; > + batch[i].dest.offset = offset; > + batch[i].flags = GNTCOPY_source_gref; > + break; > + case GNTCOPY_IOCTL_s2g: /* copy to guest */ > + batch[i].len = seg->self.iov.iov_len; > + batch[i].source.u.gmfn = pgaddr; > + batch[i].source.domid = DOMID_SELF; > + batch[i].source.offset = offset; > + batch[i].dest.u.ref = seg->self.ref; > + batch[i].dest.domid = dst; > + batch[i].dest.offset = seg->self.offset; > + batch[i].flags = GNTCOPY_dest_gref; > + break; > + case GNTCOPY_IOCTL_g2g: /* copy guest to guest */ > + batch[i].len = seg->g2g.len; > + batch[i].source.u.ref = seg->g2g.src.ref; > + batch[i].source.domid = src; > + batch[i].source.offset = seg->g2g.src.offset; > + batch[i].dest.u.ref = seg->g2g.dst.ref; > + batch[i].dest.domid = dst; > + batch[i].dest.offset = seg->g2g.dst.offset; > + batch[i].flags = GNTCOPY_source_gref | GNTCOPY_dest_gref; > + break; > + default: > + pr_debug("invalid grant-copy direction %d\n", dir); > + err = -EINVAL; > + goto out; > + } > + } > + > + gnttab_batch_copy(batch, nr_segs2cp); > + for (i = 0; i < nr_segs2cp; i++) { Can nr_segs2cp be not equal to nr_segments here? If you got to this point you have gone through the full loop. > + err = put_user(batch[i].status, &__segments[i].status); > + if (unlikely(err)) { > + pr_debug("failed to copy error code %d to user: %d\n", > + batch[i].status, err); > + goto out; > + } > + } > + > +out: > + for (i = 0; i < nr_pinned; i++) > + put_page(pages[i]); > + > + if (unlikely(err)) > + return err; > + > + return nr_segs2cp; And I think here it can be either 0 (which is the case of an error) or nr_segments. If you error out of the 'for' loop you haven't actually copied anything, even though nr_segs2cp might be non-zero. -boris