* [PATCH v2 2/7] libxc: Plumb Xen with vNUMA topology for domain.
@ 2013-11-14 3:26 Elena Ufimtseva
2013-11-14 11:40 ` Dario Faggioli
0 siblings, 1 reply; 2+ messages in thread
From: Elena Ufimtseva @ 2013-11-14 3:26 UTC (permalink / raw)
To: xen-devel
Cc: keir, Elena Ufimtseva, stefano.stabellini, george.dunlap, msw,
dario.faggioli, lccycc123, JBeulich
Per-domain vNUMA topology initialization.
domctl hypercall is used to set vNUMA topology
per domU during domain build time.
Signed-off-by: Elena Ufimtseva <ufimtseva@gmail.com>
---
tools/libxc/xc_dom.h | 9 +++++++
tools/libxc/xc_domain.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++
tools/libxc/xenctrl.h | 9 +++++++
3 files changed, 79 insertions(+)
diff --git a/tools/libxc/xc_dom.h b/tools/libxc/xc_dom.h
index 86e23ee..42a16c9 100644
--- a/tools/libxc/xc_dom.h
+++ b/tools/libxc/xc_dom.h
@@ -114,6 +114,15 @@ struct xc_dom_image {
struct xc_dom_phys *phys_pages;
int realmodearea_log;
+ /*
+ * vNUMA topology and memory allocation structure.
+ * Defines the way to allocate memory on per NUMA
+ * physical nodes that is defined by vnode_numamap.
+ */
+ uint32_t nr_vnodes;
+ uint64_t *vnuma_memszs;
+ unsigned int *vnode_numamap;
+
/* malloc memory pool */
struct xc_dom_mem *memblocks;
diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
index 1ccafc5..afa8667 100644
--- a/tools/libxc/xc_domain.c
+++ b/tools/libxc/xc_domain.c
@@ -1776,6 +1776,67 @@ int xc_domain_set_max_evtchn(xc_interface *xch, uint32_t domid,
return do_domctl(xch, &domctl);
}
+/* Plumbs Xen with vNUMA topology */
+int xc_domain_setvnodes(xc_interface *xch,
+ uint32_t domid,
+ uint16_t nr_vnodes,
+ uint16_t nr_vcpus,
+ vmemrange_t *vmemrange,
+ unsigned int *vdistance,
+ unsigned int *vcpu_to_vnode,
+ unsigned int *vnode_numamap)
+{
+ int rc;
+ DECLARE_DOMCTL;
+ DECLARE_HYPERCALL_BOUNCE(vmemrange, sizeof(*vmemrange) * nr_vnodes,
+ XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
+ DECLARE_HYPERCALL_BOUNCE(vdistance, sizeof(*vdistance) *
+ nr_vnodes * nr_vnodes,
+ XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
+ DECLARE_HYPERCALL_BOUNCE(vcpu_to_vnode, sizeof(*vcpu_to_vnode) * nr_vcpus,
+ XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
+ DECLARE_HYPERCALL_BOUNCE(vnode_numamap, sizeof(*vnode_numamap) *
+ nr_vnodes,
+ XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
+ if ( nr_vnodes == 0 )
+ return -EINVAL;
+
+ if ( vdistance == NULL || vcpu_to_vnode == NULL ||
+ vmemrange == NULL || vnode_numamap == NULL ) {
+ PERROR("Incorrect parameters for XEN_DOMCTL_setvnumainfo.\n");
+ return -EINVAL;
+ }
+
+ rc = -EFAULT;
+
+ if ( xc_hypercall_bounce_pre(xch, vmemrange) ||
+ xc_hypercall_bounce_pre(xch, vdistance) ||
+ xc_hypercall_bounce_pre(xch, vcpu_to_vnode) ||
+ xc_hypercall_bounce_pre(xch, vnode_numamap) ) {
+ PERROR("Could not bounce buffer for xc_domain_setvnodes.\n");
+ return rc;
+ }
+
+ set_xen_guest_handle(domctl.u.vnuma.vmemrange, vmemrange);
+ set_xen_guest_handle(domctl.u.vnuma.vdistance, vdistance);
+ set_xen_guest_handle(domctl.u.vnuma.vcpu_to_vnode, vcpu_to_vnode);
+ set_xen_guest_handle(domctl.u.vnuma.vnode_numamap, vnode_numamap);
+
+ domctl.cmd = XEN_DOMCTL_setvnumainfo;
+ domctl.domain = (domid_t)domid;
+ domctl.u.vnuma.nr_vnodes = nr_vnodes;
+ domctl.u.vnuma.__pad = 0;
+
+ rc = do_domctl(xch, &domctl);
+
+ xc_hypercall_bounce_post(xch, vmemrange);
+ xc_hypercall_bounce_post(xch, vdistance);
+ xc_hypercall_bounce_post(xch, vcpu_to_vnode);
+ xc_hypercall_bounce_post(xch, vnode_numamap);
+
+ return rc;
+}
+
/*
* Local variables:
* mode: C
diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h
index 8cf3f3b..451660e 100644
--- a/tools/libxc/xenctrl.h
+++ b/tools/libxc/xenctrl.h
@@ -1108,6 +1108,15 @@ int xc_domain_set_memmap_limit(xc_interface *xch,
uint32_t domid,
unsigned long map_limitkb);
+int xc_domain_setvnodes(xc_interface *xch,
+ uint32_t domid,
+ uint16_t nr_vnodes,
+ uint16_t nr_vcpus,
+ vmemrange_t *vmemrange,
+ unsigned int *vdistance,
+ unsigned int *vcpu_to_vnode,
+ unsigned int *vnode_numamap);
+
#if defined(__i386__) || defined(__x86_64__)
/*
* PC BIOS standard E820 types and structure.
--
1.7.10.4
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v2 2/7] libxc: Plumb Xen with vNUMA topology for domain.
2013-11-14 3:26 [PATCH v2 2/7] libxc: Plumb Xen with vNUMA topology for domain Elena Ufimtseva
@ 2013-11-14 11:40 ` Dario Faggioli
0 siblings, 0 replies; 2+ messages in thread
From: Dario Faggioli @ 2013-11-14 11:40 UTC (permalink / raw)
To: Elena Ufimtseva
Cc: keir, stefano.stabellini, george.dunlap, msw, lccycc123,
xen-devel, JBeulich
[-- Attachment #1.1: Type: text/plain, Size: 4875 bytes --]
On mer, 2013-11-13 at 22:26 -0500, Elena Ufimtseva wrote:
> diff --git a/tools/libxc/xc_dom.h b/tools/libxc/xc_dom.h
> @@ -114,6 +114,15 @@ struct xc_dom_image {
> struct xc_dom_phys *phys_pages;
> int realmodearea_log;
>
> + /*
> + * vNUMA topology and memory allocation structure.
> + * Defines the way to allocate memory on per NUMA
> + * physical nodes that is defined by vnode_numamap.
> + */
> + uint32_t nr_vnodes;
> + uint64_t *vnuma_memszs;
> + unsigned int *vnode_numamap;
> +
>
This vnode_numamap is, basically vnode-to-pnode mapping, isn't it? If
yes, considering you have vcpu_to_vnode already, what about actually
calling it vnode_to_pnode (or something similar)?
Here in Xen (either hypervisor or toolstack), hinting to 'map' and
'mapping', especially when there is memory involved, tend to have a
completely different meaning...
> diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
>
> +/* Plumbs Xen with vNUMA topology */
> +int xc_domain_setvnodes(xc_interface *xch,
> + uint32_t domid,
> + uint16_t nr_vnodes,
> + uint16_t nr_vcpus,
> + vmemrange_t *vmemrange,
> + unsigned int *vdistance,
> + unsigned int *vcpu_to_vnode,
> + unsigned int *vnode_numamap)
> +{
> + int rc;
> + DECLARE_DOMCTL;
> + DECLARE_HYPERCALL_BOUNCE(vmemrange, sizeof(*vmemrange) * nr_vnodes,
> + XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
>
These all should be XC_HYPERCALL_BUFFER_BOUNCE_IN, I think.
You don't really do any copy_to_guest() in 1/7 for any of these
parameters, and, at the same time, you really don't expect, here, to get
something back from the hypervisor in them, do you?
> + DECLARE_HYPERCALL_BOUNCE(vdistance, sizeof(*vdistance) *
> + nr_vnodes * nr_vnodes,
> + XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
> + DECLARE_HYPERCALL_BOUNCE(vcpu_to_vnode, sizeof(*vcpu_to_vnode) * nr_vcpus,
> + XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
> + DECLARE_HYPERCALL_BOUNCE(vnode_numamap, sizeof(*vnode_numamap) *
> + nr_vnodes,
> + XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
> + if ( nr_vnodes == 0 )
> + return -EINVAL;
> +
>
Oh, how funny, we were just talking about this with Konrad the other
day. :-)
So, my impression is that, in libxc, we return 0 on success and, in case
of error, we return -1 and set errno. So, whenever it is another xc call
that fails, you can rely on it having set errno properly and just return
-1. If it's you, like in this case, you should be the one setting errno
(and returning -1).
Again, this is what I've always done and what I find the most common in
libxc. Perhaps we need a word from one of the maintainer to enlighten us
about what's the best practise here.
If I'm right, pretty much all the error handling should be changed in
this patch, and I'll avoid pointing at all the occurrences here.
> + if ( vdistance == NULL || vcpu_to_vnode == NULL ||
> + vmemrange == NULL || vnode_numamap == NULL ) {
>
Mmm... didn't we say in the previous patch (1/7, the one implementing
this DOMCTL in xen) that it is fine for vnode_numamap to be NULL? If we
error out like this here, how will ever Xen see it being NULL?
> + PERROR("Incorrect parameters for XEN_DOMCTL_setvnumainfo.\n");
> + return -EINVAL;
> + }
> +
> + rc = -EFAULT;
> diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h
> index 8cf3f3b..451660e 100644
> --- a/tools/libxc/xenctrl.h
> +++ b/tools/libxc/xenctrl.h
> @@ -1108,6 +1108,15 @@ int xc_domain_set_memmap_limit(xc_interface *xch,
> uint32_t domid,
> unsigned long map_limitkb);
>
> +int xc_domain_setvnodes(xc_interface *xch,
> + uint32_t domid,
> + uint16_t nr_vnodes,
> + uint16_t nr_vcpus,
> + vmemrange_t *vmemrange,
> + unsigned int *vdistance,
> + unsigned int *vcpu_to_vnode,
> + unsigned int *vnode_numamap);
> +
>
This is a minor thing, but I wonder whether xc_domain_setvnuma()
wouldn't make a better name for this. Thoughts?
Regards,
Dario
--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2013-11-14 11:40 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-14 3:26 [PATCH v2 2/7] libxc: Plumb Xen with vNUMA topology for domain Elena Ufimtseva
2013-11-14 11:40 ` Dario Faggioli
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).