From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: [PATCH v2 1/7] xen: vNUMA support for guests. Date: Thu, 14 Nov 2013 21:59:28 +0000 Message-ID: <528547C0.3020007@eu.citrix.com> References: <1384399569-23969-1-git-send-email-ufimtseva@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1384399569-23969-1-git-send-email-ufimtseva@gmail.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: Elena Ufimtseva Cc: keir@xen.org, stefano.stabellini@eu.citrix.com, msw@linux.com, dario.faggioli@citrix.com, lccycc123@gmail.com, xen-devel@lists.xen.org, JBeulich@suse.com List-Id: xen-devel@lists.xenproject.org On 11/14/2013 03:26 AM, Elena Ufimtseva wrote: > Defines interface, structures and hypercalls for guests that wish > to retreive vNUMA topology from Xen. > Two subop hypercalls introduced by patch: > XEN_DOMCTL_setvnumainfo to define vNUMA domain topology per domain > and XENMEM_get_vnuma_info to retreive that topology by guest. > > Signed-off-by: Elena Ufimtseva Thanks, Elena -- this looks like it's much closer to being ready to be checked in. A few comments: > @@ -871,6 +872,87 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl) > } > break; > > + case XEN_DOMCTL_setvnumainfo: > + { > + unsigned int i = 0, dist_size; > + uint nr_vnodes; > + ret = -EFAULT; > + > + /* Already set? */ > + if ( d->vnuma.nr_vnodes > 0 ) > + return 0; > + > + nr_vnodes = op->u.vnuma.nr_vnodes; > + > + if ( nr_vnodes == 0 ) > + return ret; > + if ( nr_vnodes * nr_vnodes > UINT_MAX ) > + return ret; > + > + /* > + * If null, vnode_numamap will set default to > + * point to allocation mechanism to dont use > + * per physical node allocation or this is for > + * cases when there is no physical NUMA. > + */ > + if ( guest_handle_is_null(op->u.vnuma.vdistance) || > + guest_handle_is_null(op->u.vnuma.vmemrange) || > + guest_handle_is_null(op->u.vnuma.vcpu_to_vnode) ) > + goto err_dom; > + > + dist_size = nr_vnodes * nr_vnodes; > + > + d->vnuma.vdistance = xmalloc_array(unsigned int, dist_size); > + d->vnuma.vmemrange = xmalloc_array(vmemrange_t, nr_vnodes); > + d->vnuma.vcpu_to_vnode = xmalloc_array(unsigned int, d->max_vcpus); > + d->vnuma.vnode_numamap = xmalloc_array(unsigned int, nr_vnodes); Is there a risk here that you'll leak memory if a buggy toolstack calls setvnuma_info multiple times? > diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h > index d4e479f..da458d3 100644 > --- a/xen/include/public/domctl.h > +++ b/xen/include/public/domctl.h > @@ -35,6 +35,7 @@ > #include "xen.h" > #include "grant_table.h" > #include "hvm/save.h" > +#include "vnuma.h" > > #define XEN_DOMCTL_INTERFACE_VERSION 0x00000009 > > @@ -863,6 +864,27 @@ struct xen_domctl_set_max_evtchn { > typedef struct xen_domctl_set_max_evtchn xen_domctl_set_max_evtchn_t; > DEFINE_XEN_GUEST_HANDLE(xen_domctl_set_max_evtchn_t); > > +/* > + * XEN_DOMCTL_setvnumainfo: sets the vNUMA topology > + * parameters a guest may request. > + */ > +struct xen_domctl_vnuma { > + uint32_t nr_vnodes; > + uint32_t __pad; > + XEN_GUEST_HANDLE_64(uint) vdistance; > + XEN_GUEST_HANDLE_64(uint) vcpu_to_vnode; > + /* domain memory mapping map to physical NUMA nodes */ > + XEN_GUEST_HANDLE_64(uint) vnode_numamap; What is this for? We don't pass this to the guest or seem to use it in any way. -George