From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Vrabel Subject: [PATCH 2/3] grant-table: refactor grant copy to reduce duplicate code Date: Fri, 23 Jan 2015 10:43:46 +0000 Message-ID: <1422009827-12881-3-git-send-email-david.vrabel@citrix.com> References: <1422009827-12881-1-git-send-email-david.vrabel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" 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 1YEbu2-0005jH-MT for xen-devel@lists.xenproject.org; Fri, 23 Jan 2015 10:55:50 +0000 In-Reply-To: <1422009827-12881-1-git-send-email-david.vrabel@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: xen-devel@lists.xenproject.org Cc: Keir Fraser , Tim Deegan , David Vrabel , Jan Beulich , Ian Campbell List-Id: xen-devel@lists.xenproject.org Much of the grant copy operation is identical for the source and destination buffers. Refactor the code into per-buffer functions. Signed-off-by: David Vrabel --- xen/common/grant_table.c | 289 +++++++++++++++++++++++++------------- xen/include/public/grant_table.h | 2 +- 2 files changed, 192 insertions(+), 99 deletions(-) diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c index fb9d8f7..f5b25bc 100644 --- a/xen/common/grant_table.c +++ b/xen/common/grant_table.c @@ -2077,139 +2077,230 @@ __acquire_grant_for_copy( return rc; } -static void -__gnttab_copy( - struct gnttab_copy *op) -{ - struct domain *sd = NULL, *dd = NULL; - unsigned long s_frame, d_frame; - struct page_info *s_pg = NULL, *d_pg = NULL; - char *sp, *dp; - s16 rc = GNTST_okay; - int have_d_grant = 0, have_s_grant = 0; - int src_is_gref, dest_is_gref; +struct gnttab_copy_buf { + /* Guest provided. */ + struct gnttab_copy_ptr ptr; + uint16_t len; - if ( ((op->source.offset + op->len) > PAGE_SIZE) || - ((op->dest.offset + op->len) > PAGE_SIZE) ) - PIN_FAIL(error_out, GNTST_bad_copy_arg, "copy beyond page area.\n"); + /* Mapped etc. */ + struct domain *domain; + unsigned long frame; + struct page_info *page; + void *virt; + bool_t read_only; + bool_t have_grant; + bool_t have_type; +}; - src_is_gref = op->flags & GNTCOPY_source_gref; - dest_is_gref = op->flags & GNTCOPY_dest_gref; +static int gnttab_copy_lock_domain(domid_t domid, unsigned int gref_flag, + struct gnttab_copy_buf *buf) +{ + int rc; - if ( (op->source.domid != DOMID_SELF && !src_is_gref ) || - (op->dest.domid != DOMID_SELF && !dest_is_gref) ) - PIN_FAIL(error_out, GNTST_permission_denied, + if ( domid != DOMID_SELF && !gref_flag ) + PIN_FAIL(out, GNTST_permission_denied, "only allow copy-by-mfn for DOMID_SELF.\n"); - if ( op->source.domid == DOMID_SELF ) - sd = rcu_lock_current_domain(); - else if ( (sd = rcu_lock_domain_by_id(op->source.domid)) == NULL ) - PIN_FAIL(error_out, GNTST_bad_domain, - "couldn't find %d\n", op->source.domid); + if ( domid == DOMID_SELF ) + buf->domain = rcu_lock_current_domain(); + else + { + buf->domain = rcu_lock_domain_by_id(domid); + if ( buf->domain == NULL ) + PIN_FAIL(out, GNTST_bad_domain, "couldn't find %d\n", domid); + } - if ( op->dest.domid == DOMID_SELF ) - dd = rcu_lock_current_domain(); - else if ( (dd = rcu_lock_domain_by_id(op->dest.domid)) == NULL ) - PIN_FAIL(error_out, GNTST_bad_domain, - "couldn't find %d\n", op->dest.domid); + buf->ptr.domid = domid; + rc = GNTST_okay; + out: + return rc; +} - rc = xsm_grant_copy(XSM_HOOK, sd, dd); - if ( rc ) +static void gnttab_copy_unlock_domains(struct gnttab_copy_buf *src, + struct gnttab_copy_buf *dest) +{ + if ( src->domain ) + { + rcu_unlock_domain(src->domain); + src->domain = NULL; + } + if ( dest->domain ) + { + rcu_unlock_domain(dest->domain); + dest->domain = NULL; + } +} + +static int gnttab_copy_lock_domains(const struct gnttab_copy *op, + struct gnttab_copy_buf *src, + struct gnttab_copy_buf *dest) +{ + int rc; + + rc = gnttab_copy_lock_domain(op->source.domid, + op->flags & GNTCOPY_source_gref, src); + if ( rc < 0 ) + goto error; + rc = gnttab_copy_lock_domain(op->dest.domid, + op->flags & GNTCOPY_dest_gref, dest); + if ( rc < 0 ) + goto error; + + rc = xsm_grant_copy(XSM_HOOK, src->domain, dest->domain); + if ( rc < 0 ) { rc = GNTST_permission_denied; - goto error_out; + goto error; } + return 0; + + error: + gnttab_copy_unlock_domains(src, dest); + return rc; +} - if ( src_is_gref ) +static void gnttab_copy_release_buf(struct gnttab_copy_buf *buf) +{ + if ( buf->virt ) { - uint16_t source_off, source_len; - rc = __acquire_grant_for_copy(sd, op->source.u.ref, - current->domain->domain_id, 1, - &s_frame, &s_pg, - &source_off, &source_len, 1); - if ( rc != GNTST_okay ) - goto error_out; - have_s_grant = 1; - if ( op->source.offset < source_off || - op->len > source_len ) - PIN_FAIL(error_out, GNTST_general_error, - "copy source out of bounds: %d < %d || %d > %d\n", - op->source.offset, source_off, - op->len, source_len); + unmap_domain_page(buf->virt); + buf->virt = NULL; } - else + if ( buf->have_type ) { - rc = __get_paged_frame(op->source.u.gmfn, &s_frame, &s_pg, 1, sd); - if ( rc != GNTST_okay ) - PIN_FAIL(error_out, rc, - "source frame %lx invalid.\n", s_frame); + put_page_type(buf->page); + buf->have_type = 0; + } + if ( buf->page ) + { + put_page(buf->page); + buf->page = NULL; + } + if ( buf->have_grant ) + { + __release_grant_for_copy(buf->domain, buf->ptr.u.ref, buf->read_only); + buf->have_grant = 0; } +} + +static int gnttab_copy_claim_buf(const struct gnttab_copy *op, + const struct gnttab_copy_ptr *ptr, + struct gnttab_copy_buf *buf, + unsigned int gref_flag) +{ + int rc; + + buf->read_only = gref_flag == GNTCOPY_source_gref; - if ( dest_is_gref ) + if ( op->flags & gref_flag ) { - uint16_t dest_off, dest_len; - rc = __acquire_grant_for_copy(dd, op->dest.u.ref, - current->domain->domain_id, 0, - &d_frame, &d_pg, &dest_off, &dest_len, 1); + rc = __acquire_grant_for_copy(buf->domain, ptr->u.ref, + current->domain->domain_id, + buf->read_only, + &buf->frame, &buf->page, + &buf->ptr.offset, &buf->len, 1); if ( rc != GNTST_okay ) - goto error_out; - have_d_grant = 1; - if ( op->dest.offset < dest_off || - op->len > dest_len ) - PIN_FAIL(error_out, GNTST_general_error, - "copy dest out of bounds: %d < %d || %d > %d\n", - op->dest.offset, dest_off, - op->len, dest_len); + goto out; + buf->ptr.u.ref = ptr->u.ref; + buf->have_grant = 1; } else { - rc = __get_paged_frame(op->dest.u.gmfn, &d_frame, &d_pg, 0, dd); + rc = __get_paged_frame(ptr->u.gmfn, &buf->frame, &buf->page, + buf->read_only, buf->domain); if ( rc != GNTST_okay ) - PIN_FAIL(error_out, rc, - "destination frame %lx invalid.\n", d_frame); + PIN_FAIL(out, rc, + "source frame %lx invalid.\n", ptr->u.gmfn); + + buf->ptr.u.gmfn = ptr->u.gmfn; + buf->ptr.offset = 0; + buf->len = PAGE_SIZE; } - if ( !get_page_type(d_pg, PGT_writable_page) ) + if ( !buf->read_only ) { - if ( !dd->is_dying ) - gdprintk(XENLOG_WARNING, "Could not get dst frame %lx\n", d_frame); - rc = GNTST_general_error; - goto error_out; + if ( !get_page_type(buf->page, PGT_writable_page) ) + { + if ( !buf->domain->is_dying ) + gdprintk(XENLOG_WARNING, "Could not get writable frame %lx\n", buf->frame); + rc = GNTST_general_error; + goto out; + } + buf->have_type = 1; } - sp = map_domain_page(s_frame); - dp = map_domain_page(d_frame); + buf->virt = map_domain_page(buf->frame); + rc = GNTST_okay; - memcpy(dp + op->dest.offset, sp + op->source.offset, op->len); + out: + return rc; +} - unmap_domain_page(dp); - unmap_domain_page(sp); +static int gnttab_copy_buf(const struct gnttab_copy *op, + struct gnttab_copy_buf *dest, + const struct gnttab_copy_buf *src) +{ + int rc; - gnttab_mark_dirty(dd, d_frame); + if ( ((op->source.offset + op->len) > PAGE_SIZE) || + ((op->dest.offset + op->len) > PAGE_SIZE) ) + PIN_FAIL(out, GNTST_bad_copy_arg, "copy beyond page area.\n"); + + if ( op->source.offset < src->ptr.offset || + op->source.offset + op->len > src->ptr.offset + src->len ) + PIN_FAIL(out, GNTST_general_error, + "copy source out of bounds: %d < %d || %d > %d\n", + op->source.offset, src->ptr.offset, + op->len, src->len); + + if ( op->dest.offset < dest->ptr.offset || + op->dest.offset + op->len > dest->ptr.offset + dest->len ) + PIN_FAIL(out, GNTST_general_error, + "copy dest out of bounds: %d < %d || %d > %d\n", + op->dest.offset, dest->ptr.offset, + op->len, dest->len); + + memcpy(dest->virt + op->dest.offset, src->virt + op->source.offset, + op->len); + gnttab_mark_dirty(dest->domain, dest->frame); + rc = GNTST_okay; + out: + return rc; +} - put_page_type(d_pg); - error_out: - if ( d_pg ) - put_page(d_pg); - if ( s_pg ) - put_page(s_pg); - if ( have_s_grant ) - __release_grant_for_copy(sd, op->source.u.ref, 1); - if ( have_d_grant ) - __release_grant_for_copy(dd, op->dest.u.ref, 0); - if ( sd ) - rcu_unlock_domain(sd); - if ( dd ) - rcu_unlock_domain(dd); - op->status = rc; +static int gnttab_copy_one(const struct gnttab_copy *op, + struct gnttab_copy_buf *dest, + struct gnttab_copy_buf *src) +{ + int rc; + + rc = gnttab_copy_lock_domains(op, src, dest); + if ( rc < 0 ) + goto out; + + rc = gnttab_copy_claim_buf(op, &op->source, src, GNTCOPY_source_gref); + if ( rc < 0 ) + goto out; + + rc = gnttab_copy_claim_buf(op, &op->dest, dest, GNTCOPY_dest_gref); + if ( rc < 0 ) + goto out; + + rc = gnttab_copy_buf(op, dest, src); + out: + gnttab_copy_release_buf(src); + gnttab_copy_release_buf(dest); + gnttab_copy_unlock_domains(src, dest); + return rc; } -static long -gnttab_copy( +static long gnttab_copy( XEN_GUEST_HANDLE_PARAM(gnttab_copy_t) uop, unsigned int count) { - int i; + unsigned int i; struct gnttab_copy op; + struct gnttab_copy_buf src = {}; + struct gnttab_copy_buf dest = {}; for ( i = 0; i < count; i++ ) { @@ -2217,7 +2308,9 @@ gnttab_copy( return i; if ( unlikely(__copy_from_guest(&op, uop, 1)) ) return -EFAULT; - __gnttab_copy(&op); + + op.status = gnttab_copy_one(&op, &dest, &src); + if ( unlikely(__copy_field_to_guest(uop, &op, status)) ) return -EFAULT; guest_handle_add_offset(uop, 1); diff --git a/xen/include/public/grant_table.h b/xen/include/public/grant_table.h index 20d4e77..c8619ed 100644 --- a/xen/include/public/grant_table.h +++ b/xen/include/public/grant_table.h @@ -453,7 +453,7 @@ DEFINE_XEN_GUEST_HANDLE(gnttab_transfer_t); struct gnttab_copy { /* IN parameters. */ - struct { + struct gnttab_copy_ptr { union { grant_ref_t ref; xen_pfn_t gmfn; -- 1.7.10.4