From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH v5 10/24] libxl: functions to build vmemranges for PV guest Date: Fri, 13 Feb 2015 15:49:44 +0000 Message-ID: <54DE1D18.8070004@citrix.com> References: <1423770294-9779-1-git-send-email-wei.liu2@citrix.com> <1423770294-9779-11-git-send-email-wei.liu2@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1423770294-9779-11-git-send-email-wei.liu2@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: Wei Liu , xen-devel@lists.xen.org Cc: dario.faggioli@citrix.com, JBeulich@suse.com, ian.jackson@eu.citrix.com, ian.campbell@citrix.com, ufimtseva@gmail.com List-Id: xen-devel@lists.xenproject.org On 12/02/15 19:44, Wei Liu wrote: > Introduce a arch-independent routine to generate one vmemrange per > vnode. Also introduce arch-dependent routines for different > architectures because part of the process is arch-specific -- ARM has > yet have NUMA support and E820 is x86 only. > > For those x86 guests who care about machine E820 map (i.e. with > e820_host=1), vnode is further split into several vmemranges to > accommodate memory holes. A few stubs for libxl_arm.c are created. > > Signed-off-by: Wei Liu > Cc: Ian Campbell > Cc: Ian Jackson > Cc: Dario Faggioli > Cc: Elena Ufimtseva > --- > Changes in v5: > 1. Allocate array all in one go. > 2. Reverse the logic of vmemranges generation. > > Changes in v4: > 1. Adapt to new interface. > 2. Address Ian Jackson's comments. > > Changes in v3: > 1. Rewrite commit log. > --- > tools/libxl/libxl_arch.h | 6 ++++ > tools/libxl/libxl_arm.c | 8 +++++ > tools/libxl/libxl_internal.h | 8 +++++ > tools/libxl/libxl_vnuma.c | 41 +++++++++++++++++++++++++ > tools/libxl/libxl_x86.c | 73 ++++++++++++++++++++++++++++++++++++++++++++ > 5 files changed, 136 insertions(+) > > diff --git a/tools/libxl/libxl_arch.h b/tools/libxl/libxl_arch.h > index d3bc136..e249048 100644 > --- a/tools/libxl/libxl_arch.h > +++ b/tools/libxl/libxl_arch.h > @@ -27,4 +27,10 @@ int libxl__arch_domain_init_hw_description(libxl__gc *gc, > int libxl__arch_domain_finalise_hw_description(libxl__gc *gc, > libxl_domain_build_info *info, > struct xc_dom_image *dom); > + > +/* build vNUMA vmemrange with arch specific information */ > +int libxl__arch_vnuma_build_vmemrange(libxl__gc *gc, > + uint32_t domid, > + libxl_domain_build_info *b_info, > + libxl__domain_build_state *state); > #endif > diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c > index 65a762b..7da254f 100644 > --- a/tools/libxl/libxl_arm.c > +++ b/tools/libxl/libxl_arm.c > @@ -707,6 +707,14 @@ int libxl__arch_domain_finalise_hw_description(libxl__gc *gc, > return 0; > } > > +int libxl__arch_vnuma_build_vmemrange(libxl__gc *gc, > + uint32_t domid, > + libxl_domain_build_info *info, > + libxl__domain_build_state *state) > +{ > + return libxl__vnuma_build_vmemrange_pv_generic(gc, domid, info, state); > +} > + > /* > * Local variables: > * mode: C > diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h > index 258be0d..7d1e1cf 100644 > --- a/tools/libxl/libxl_internal.h > +++ b/tools/libxl/libxl_internal.h > @@ -3400,6 +3400,14 @@ void libxl__numa_candidate_put_nodemap(libxl__gc *gc, > int libxl__vnuma_config_check(libxl__gc *gc, > const libxl_domain_build_info *b_info, > const libxl__domain_build_state *state); > +int libxl__vnuma_build_vmemrange_pv_generic(libxl__gc *gc, > + uint32_t domid, > + libxl_domain_build_info *b_info, > + libxl__domain_build_state *state); > +int libxl__vnuma_build_vmemrange_pv(libxl__gc *gc, > + uint32_t domid, > + libxl_domain_build_info *b_info, > + libxl__domain_build_state *state); > > _hidden int libxl__ms_vm_genid_set(libxl__gc *gc, uint32_t domid, > const libxl_ms_vm_genid *id); > diff --git a/tools/libxl/libxl_vnuma.c b/tools/libxl/libxl_vnuma.c > index fa5aa8d..3d46239 100644 > --- a/tools/libxl/libxl_vnuma.c > +++ b/tools/libxl/libxl_vnuma.c > @@ -14,6 +14,7 @@ > */ > #include "libxl_osdeps.h" /* must come before any other headers */ > #include "libxl_internal.h" > +#include "libxl_arch.h" > #include > > /* Sort vmemranges in ascending order with "start" */ > @@ -122,6 +123,46 @@ out: > return rc; > } > > + > +int libxl__vnuma_build_vmemrange_pv_generic(libxl__gc *gc, > + uint32_t domid, > + libxl_domain_build_info *b_info, > + libxl__domain_build_state *state) > +{ > + int i; > + uint64_t next; > + xen_vmemrange_t *v = NULL; > + > + /* Generate one vmemrange for each virtual node. */ > + GCREALLOC_ARRAY(v, b_info->num_vnuma_nodes); > + next = 0; > + for (i = 0; i < b_info->num_vnuma_nodes; i++) { > + libxl_vnode_info *p = &b_info->vnuma_nodes[i]; > + > + v[i].start = next; > + v[i].end = next + (p->memkb << 10); > + v[i].flags = 0; > + v[i].nid = i; > + > + next = v[i].end; Using "start" and "end", this would appear to have a fencepost error which a start/size pair wouldn't have. ~Andrew > + } > + > + state->vmemranges = v; > + state->num_vmemranges = i; > + > + return 0; > +} > + > +/* Build vmemranges for PV guest */ > +int libxl__vnuma_build_vmemrange_pv(libxl__gc *gc, > + uint32_t domid, > + libxl_domain_build_info *b_info, > + libxl__domain_build_state *state) > +{ > + assert(state->vmemranges == NULL); > + return libxl__arch_vnuma_build_vmemrange(gc, domid, b_info, state); > +} > + > /* > * Local variables: > * mode: C > diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c > index d012b4d..d37cca1 100644 > --- a/tools/libxl/libxl_x86.c > +++ b/tools/libxl/libxl_x86.c > @@ -339,6 +339,79 @@ int libxl__arch_domain_finalise_hw_description(libxl__gc *gc, > return 0; > } > > +/* Return 0 on success, ERROR_* on failure. */ > +int libxl__arch_vnuma_build_vmemrange(libxl__gc *gc, > + uint32_t domid, > + libxl_domain_build_info *b_info, > + libxl__domain_build_state *state) > +{ > + int nid, nr_vmemrange, rc; > + uint32_t nr_e820, e820_count; > + struct e820entry map[E820MAX]; > + xen_vmemrange_t *vmemranges; > + > + /* If e820_host is not set, call the generic function */ > + if (!(b_info->type == LIBXL_DOMAIN_TYPE_PV && > + libxl_defbool_val(b_info->u.pv.e820_host))) > + return libxl__vnuma_build_vmemrange_pv_generic(gc, domid, b_info, > + state); > + > + assert(state->vmemranges == NULL); > + > + nr_e820 = E820MAX; > + rc = e820_host_sanitize(gc, b_info, map, &nr_e820); > + if (rc) goto out; > + > + e820_count = 0; > + nr_vmemrange = 0; > + vmemranges = NULL; > + for (nid = 0; nid < b_info->num_vnuma_nodes; nid++) { > + libxl_vnode_info *p = &b_info->vnuma_nodes[nid]; > + uint64_t remaining_bytes = (p->memkb << 10), bytes; > + > + while (remaining_bytes > 0) { > + if (e820_count >= nr_e820) { > + rc = ERROR_NOMEM; > + goto out; > + } > + > + /* Skip non RAM region */ > + if (map[e820_count].type != E820_RAM) { > + e820_count++; > + continue; > + } > + > + GCREALLOC_ARRAY(vmemranges, nr_vmemrange+1); > + > + bytes = map[e820_count].size >= remaining_bytes ? > + remaining_bytes : map[e820_count].size; > + > + vmemranges[nr_vmemrange].start = map[e820_count].addr; > + vmemranges[nr_vmemrange].end = map[e820_count].addr + bytes; > + > + if (map[e820_count].size >= remaining_bytes) { > + map[e820_count].addr += bytes; > + map[e820_count].size -= bytes; > + } else { > + e820_count++; > + } > + > + remaining_bytes -= bytes; > + > + vmemranges[nr_vmemrange].flags = 0; > + vmemranges[nr_vmemrange].nid = nid; > + nr_vmemrange++; > + } > + } > + > + state->vmemranges = vmemranges; > + state->num_vmemranges = nr_vmemrange; > + > + rc = 0; > +out: > + return rc; > +} > + > /* > * Local variables: > * mode: C