[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v13 05/11] x86/mm: add HYPERVISOR_memory_op to acquire guest resources
>>> On 30.10.17 at 18:48, <paul.durrant@xxxxxxxxxx> wrote: > --- a/xen/common/memory.c > +++ b/xen/common/memory.c > @@ -965,6 +965,94 @@ static long xatp_permission_check(struct domain *d, > unsigned int space) > return xsm_add_to_physmap(XSM_TARGET, current->domain, d); > } > > +static int acquire_resource( > + XEN_GUEST_HANDLE_PARAM(xen_mem_acquire_resource_t) arg) > +{ > + struct domain *d, *currd = current->domain; > + xen_mem_acquire_resource_t xmar; > + /* > + * The mfn_list and gfn_list (below) arrays are ok on stack for the > + * moment since they are small, but if they need to grow in future > + * use-cases then per-CPU arrays or heap allocations may be required. > + */ > + xen_pfn_t mfn_list[2]; > + int rc; > + > + if ( copy_from_guest(&xmar, arg, 1) ) > + return -EFAULT; > + > + if ( xmar.pad != 0 ) > + return -EINVAL; > + > + if ( guest_handle_is_null(xmar.frame_list) ) > + { > + if ( xmar.nr_frames > 0 ) I generally consider "!= 0" (which then could be omitted altogether better than "> 0" when the quantity is unsigned, to avoid giving the impression that negative values might also take the other path. > + return -EINVAL; > + > + xmar.nr_frames = ARRAY_SIZE(mfn_list); > + > + if ( __copy_field_to_guest(arg, &xmar, nr_frames) ) > + return -EFAULT; > + > + return 0; > + } > + > + if ( xmar.nr_frames > ARRAY_SIZE(mfn_list) ) > + return -E2BIG; > + > + rc = rcu_lock_remote_domain_by_id(xmar.domid, &d); > + if ( rc ) > + return rc; > + > + rc = xsm_domain_resource_map(XSM_DM_PRIV, d); > + if ( rc ) > + goto out; > + > + switch ( xmar.type ) > + { > + default: > + rc = -EOPNOTSUPP; > + break; > + } > + > + if ( rc ) > + goto out; > + > + if ( !paging_mode_translate(currd) ) > + { > + if ( copy_to_guest(xmar.frame_list, mfn_list, xmar.nr_frames) ) > + rc = -EFAULT; > + } > + else > + { > + xen_pfn_t gfn_list[ARRAY_SIZE(mfn_list)]; > + unsigned int i; > + > + rc = -EFAULT; > + if ( copy_from_guest(gfn_list, xmar.frame_list, xmar.nr_frames) ) > + goto out; This will result in requests with nr_frames being zero to fail with -EFAULT afaict. Let's please have such no-op requests succeed. > + for ( i = 0; i < xmar.nr_frames; i++ ) > + { > + rc = set_foreign_p2m_entry(currd, gfn_list[i], > + _mfn(mfn_list[i])); > + if ( rc ) > + { > + /* > + * Make sure rc is -EIO for any iteration other than > + * the first. > + */ > + rc = (i != 0) ? -EIO : rc; Along the lines of what I've said above, "!=0" could be dropped here, too. I won't insist on the cosmetic remarks to be taken care of, but the return value aspect should be fixed for Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx> to apply. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |