adjust a few RCU domain locking calls x86's do_physdev_op() had a case where the locking was entirely superfluous. Its physdev_map_pirq() further had a case where the lock was being obtained too early, needlessly complicating early exit paths. Grant table code had two open coded instances of rcu_lock_target_domain_by_id(), and a third code section could be consolidated by using the newly introduced helper function. The memory hypercall code had two more instances of open coding rcu_lock_target_domain_by_id(), but note that here this is not just cleanup, but also fixes an error return path in memory_exchange() to actually return an error. Signed-off-by: Jan Beulich --- a/xen/arch/x86/physdev.c +++ b/xen/arch/x86/physdev.c @@ -90,14 +90,10 @@ static int physdev_hvm_map_pirq( int physdev_map_pirq(domid_t domid, int type, int *index, int *pirq_p, struct msi_info *msi) { - struct domain *d; + struct domain *d = current->domain; int pirq, irq, ret = 0; void *map_data = NULL; - ret = rcu_lock_target_domain_by_id(domid, &d); - if ( ret ) - return ret; - if ( domid == DOMID_SELF && is_hvm_domain(d) ) { /* @@ -105,14 +101,15 @@ int physdev_map_pirq(domid_t domid, int * calls back into itself and deadlocks on hvm_domain.irq_lock. */ if ( !is_hvm_pv_evtchn_domain(d) ) - { - ret = -EINVAL; - goto free_domain; - } - ret = physdev_hvm_map_pirq(d, type, index, pirq_p); - goto free_domain; + return -EINVAL; + + return physdev_hvm_map_pirq(d, type, index, pirq_p); } + ret = rcu_lock_target_domain_by_id(domid, &d); + if ( ret ) + return ret; + if ( !IS_PRIV_FOR(current->domain, d) ) { ret = -EPERM; @@ -696,13 +693,12 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_H } case PHYSDEVOP_get_free_pirq: { struct physdev_get_free_pirq out; - struct domain *d; + struct domain *d = v->domain; ret = -EFAULT; if ( copy_from_guest(&out, arg, 1) != 0 ) break; - d = rcu_lock_current_domain(); spin_lock(&d->event_lock); ret = get_free_pirq(d, out.type); @@ -717,7 +713,6 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_H } spin_unlock(&d->event_lock); - rcu_unlock_domain(d); if ( ret >= 0 ) { --- a/xen/common/grant_table.c +++ b/xen/common/grant_table.c @@ -24,7 +24,7 @@ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ -#include +#include #include #include #include @@ -195,6 +195,30 @@ double_gt_unlock(struct grant_table *lgt spin_unlock(&rgt->lock); } +static struct domain *gt_lock_target_domain_by_id(domid_t dom) +{ + struct domain *d; + int rc = GNTST_general_error; + + switch ( rcu_lock_target_domain_by_id(dom, &d) ) + { + case 0: + return d; + + case -ESRCH: + gdprintk(XENLOG_INFO, "Bad domid %d.\n", dom); + rc = GNTST_bad_domain; + break; + + case -EPERM: + rc = GNTST_permission_denied; + break; + } + + ASSERT(rc < 0 && -rc <= MAX_ERRNO); + return ERR_PTR(rc); +} + static inline int __get_maptrack_handle( struct grant_table *t) @@ -1261,7 +1285,6 @@ gnttab_setup_table( struct grant_table *gt; int i; unsigned long gmfn; - domid_t dom; if ( count != 1 ) return -EINVAL; @@ -1281,25 +1304,11 @@ gnttab_setup_table( goto out1; } - dom = op.dom; - if ( dom == DOMID_SELF ) + d = gt_lock_target_domain_by_id(op.dom); + if ( IS_ERR(d) ) { - d = rcu_lock_current_domain(); - } - else - { - if ( unlikely((d = rcu_lock_domain_by_id(dom)) == NULL) ) - { - gdprintk(XENLOG_INFO, "Bad domid %d.\n", dom); - op.status = GNTST_bad_domain; - goto out1; - } - - if ( unlikely(!IS_PRIV_FOR(current->domain, d)) ) - { - op.status = GNTST_permission_denied; - goto out2; - } + op.status = PTR_ERR(d); + goto out1; } if ( xsm_grant_setup(current->domain, d) ) @@ -1352,7 +1361,6 @@ gnttab_query_size( { struct gnttab_query_size op; struct domain *d; - domid_t dom; int rc; if ( count != 1 ) @@ -1364,25 +1372,11 @@ gnttab_query_size( return -EFAULT; } - dom = op.dom; - if ( dom == DOMID_SELF ) - { - d = rcu_lock_current_domain(); - } - else + d = gt_lock_target_domain_by_id(op.dom); + if ( IS_ERR(d) ) { - if ( unlikely((d = rcu_lock_domain_by_id(dom)) == NULL) ) - { - gdprintk(XENLOG_INFO, "Bad domid %d.\n", dom); - op.status = GNTST_bad_domain; - goto query_out; - } - - if ( unlikely(!IS_PRIV_FOR(current->domain, d)) ) - { - op.status = GNTST_permission_denied; - goto query_out_unlock; - } + op.status = PTR_ERR(d); + goto query_out; } rc = xsm_grant_query_size(current->domain, d); @@ -2240,15 +2234,10 @@ gnttab_get_status_frames(XEN_GUEST_HANDL return -EFAULT; } - rc = rcu_lock_target_domain_by_id(op.dom, &d); - if ( rc < 0 ) + d = gt_lock_target_domain_by_id(op.dom); + if ( IS_ERR(d) ) { - if ( rc == -ESRCH ) - op.status = GNTST_bad_domain; - else if ( rc == -EPERM ) - op.status = GNTST_permission_denied; - else - op.status = GNTST_general_error; + op.status = PTR_ERR(d); goto out1; } rc = xsm_grant_setup(current->domain, d); --- a/xen/common/memory.c +++ b/xen/common/memory.c @@ -329,22 +329,9 @@ static long memory_exchange(XEN_GUEST_HA out_chunk_order = exch.in.extent_order - exch.out.extent_order; } - if ( likely(exch.in.domid == DOMID_SELF) ) - { - d = rcu_lock_current_domain(); - } - else - { - if ( (d = rcu_lock_domain_by_id(exch.in.domid)) == NULL ) - goto fail_early; - - if ( !IS_PRIV_FOR(current->domain, d) ) - { - rcu_unlock_domain(d); - rc = -EPERM; - goto fail_early; - } - } + rc = rcu_lock_target_domain_by_id(exch.in.domid, &d); + if ( rc ) + goto fail_early; memflags |= MEMF_bits(domain_clamp_alloc_bitsize( d, @@ -583,20 +570,8 @@ long do_memory_op(unsigned long cmd, XEN && (reservation.mem_flags & XENMEMF_populate_on_demand) ) args.memflags |= MEMF_populate_on_demand; - if ( likely(reservation.domid == DOMID_SELF) ) - { - d = rcu_lock_current_domain(); - } - else - { - if ( (d = rcu_lock_domain_by_id(reservation.domid)) == NULL ) - return start_extent; - if ( !IS_PRIV_FOR(current->domain, d) ) - { - rcu_unlock_domain(d); - return start_extent; - } - } + if ( unlikely(rcu_lock_target_domain_by_id(reservation.domid, &d)) ) + return start_extent; args.domain = d; rc = xsm_memory_adjust_reservation(current->domain, d);