[Xen-devel] [PATCH v3 4/4] libxl: fix HVM vNUMA

[Wei Liu <wei.liu2@xxxxxxxxxx>]

This patch does two thing:

The original code erroneously fills in xc_hvm_build_args before
generating vmemranges. The effect is that guest memory is populated
without vNUMA information. Move the hunk to right place to fix this.

Move the subtraction of video ram to libxl__vnuma_build_vmemrange_hvm
because it's the central place for generating vmemranges.

Reported-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
Cc: Ian Campbell <ian.campbell@xxxxxxxxxx>
Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Cc: Dario Faggioli <dario.faggioli@xxxxxxxxxx>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
---
 tools/libxl/libxl_dom.c   | 32 ++++++++++----------------------
 tools/libxl/libxl_vnuma.c | 15 ++++++++++++++-
 2 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index dccc9ac..867172a 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -961,6 +961,16 @@ int libxl__build_hvm(libxl__gc *gc, uint32_t domid,
     if (info->num_vnuma_nodes != 0) {
         int i;
 
+        ret = libxl__vnuma_build_vmemrange_hvm(gc, domid, info, state, &args);
+        if (ret) {
+            LOGEV(ERROR, ret, "hvm build vmemranges failed");
+            goto out;
+        }
+        ret = libxl__vnuma_config_check(gc, info, state);
+        if (ret) goto out;
+        ret = set_vnuma_info(gc, domid, info, state);
+        if (ret) goto out;
+
         args.nr_vmemranges = state->num_vmemranges;
         args.vmemranges = libxl__malloc(gc, sizeof(*args.vmemranges) *
                                         args.nr_vmemranges);
@@ -972,17 +982,6 @@ int libxl__build_hvm(libxl__gc *gc, uint32_t domid,
             args.vmemranges[i].nid   = state->vmemranges[i].nid;
         }
 
-        /* Consider video ram belongs to vmemrange 0 -- just shrink it
-         * by the size of video ram.
-         */
-        if (((args.vmemranges[0].end - args.vmemranges[0].start) >> 10)
-            < info->video_memkb) {
-            LOG(ERROR, "vmemrange 0 too small to contain video ram");
-            goto out;
-        }
-
-        args.vmemranges[0].end -= (info->video_memkb << 10);
-
         args.nr_vnodes = info->num_vnuma_nodes;
         args.vnode_to_pnode = libxl__malloc(gc, sizeof(*args.vnode_to_pnode) *
                                             args.nr_vnodes);
@@ -996,17 +995,6 @@ int libxl__build_hvm(libxl__gc *gc, uint32_t domid,
         goto out;
     }
 
-    if (info->num_vnuma_nodes != 0) {
-        ret = libxl__vnuma_build_vmemrange_hvm(gc, domid, info, state, &args);
-        if (ret) {
-            LOGEV(ERROR, ret, "hvm build vmemranges failed");
-            goto out;
-        }
-        ret = libxl__vnuma_config_check(gc, info, state);
-        if (ret) goto out;
-        ret = set_vnuma_info(gc, domid, info, state);
-        if (ret) goto out;
-    }
     ret = hvm_build_set_params(ctx->xch, domid, info, state->store_port,
                                &state->store_mfn, state->console_port,
                                &state->console_mfn, state->store_domid,
diff --git a/tools/libxl/libxl_vnuma.c b/tools/libxl/libxl_vnuma.c
index cac78d7..56856d2 100644
--- a/tools/libxl/libxl_vnuma.c
+++ b/tools/libxl/libxl_vnuma.c
@@ -257,6 +257,7 @@ int libxl__vnuma_build_vmemrange_hvm(libxl__gc *gc,
     uint64_t hole_start, hole_end, next;
     int nid, nr_vmemrange;
     xen_vmemrange_t *vmemranges;
+    int rc;
 
     /* Derive vmemranges from vnode size and memory hole.
      *
@@ -277,6 +278,16 @@ int libxl__vnuma_build_vmemrange_hvm(libxl__gc *gc,
         libxl_vnode_info *p = &b_info->vnuma_nodes[nid];
         uint64_t remaining_bytes = p->memkb << 10;
 
+        /* Consider video ram belongs to vnode 0 */
+        if (nid == 0) {
+            if (p->memkb < b_info->video_memkb) {
+                LOG(ERROR, "vnode 0 too small to contain video ram");
+                rc = ERROR_INVAL;
+                goto out;
+            }
+            remaining_bytes -= (b_info->video_memkb << 10);
+        }
+
         while (remaining_bytes > 0) {
             uint64_t count = remaining_bytes;
 
@@ -300,7 +311,9 @@ int libxl__vnuma_build_vmemrange_hvm(libxl__gc *gc,
     state->vmemranges = vmemranges;
     state->num_vmemranges = nr_vmemrange;
 
-    return 0;
+    rc = 0;
+out:
+    return rc;
 }
 
 /*
-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel