]> xenbits.xensource.com Git - xen.git/commitdiff
Rationalize max_grant_frames and max_maptrack_frames handling
authorGeorge Dunlap <george.dunlap@citrix.com>
Fri, 6 Dec 2019 11:47:08 +0000 (12:47 +0100)
committerJan Beulich <jbeulich@suse.com>
Fri, 6 Dec 2019 11:47:08 +0000 (12:47 +0100)
Xen used to have single, system-wide limits for the number of grant
frames and maptrack frames a guest was allowed to create. Increasing
or decreasing this single limit on the Xen command-line would change
the limit for all guests on the system.

Later, per-domain limits for these values was created. The system-wide
limits became strict limits: domains could not be created with higher
limits, but could be created with lower limits. However, that change
also introduced a range of different "default" values into various
places in the toolstack:

- The python libxc bindings hard-coded these values to 32 and 1024,
  respectively
- The libxl default values are 32 and 1024 respectively.
- xl will use the libxl default for maptrack, but does its own default
  calculation for grant frames: either 32 or 64, based on the max
  possible mfn.

These defaults interact poorly with the hypervisor command-line limit:

- The hypervisor command-line limit cannot be used to raise the limit
  for all guests anymore, as the default in the toolstack will
  effectively override this.
- If you use the hypervisor command-line limit to *reduce* the limit,
  then the "default" values generated by the toolstack are too high,
  and all guest creations will fail.

In other words, the toolstack defaults require any change to be
effected by having the admin explicitly specify a new value in every
guest.

In order to address this, have grant_table_init treat negative values
for max_grant_frames and max_maptrack_frames as instructions to use the
system-wide default, and have all the above toolstacks default to passing
-1 unless a different value is explicitly configured.

This restores the old behavior in that changing the hypervisor command-line
option can change the behavior for all guests, while retaining the ability
to set per-guest values.  It also removes the bug that reducing the
system-wide max will cause all domains without explicit limits to fail.

NOTE: - The Ocaml bindings require the caller to always specify a value,
        and the code to start a xenstored stubdomain hard-codes these to 4
and 128 respectively; this behavour will not be modified.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Wei Liu <wl@xen.org>
Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
master commit: f2ae59bc4b9b5c3f12de86aa42cdf413d2c3ffbf
master date: 2019-11-29 21:43:49 +0000

13 files changed:
docs/man/xl.conf.5.pod
tools/libxl/libxl.h
tools/libxl/libxl_types.idl
tools/libxl/libxlu_cfg.c
tools/libxl/libxlutil.h
tools/python/xen/lowlevel/xc/xc.c
tools/xl/xl.c
tools/xl/xl_parse.c
xen/arch/arm/setup.c
xen/arch/x86/setup.c
xen/common/grant_table.c
xen/include/public/domctl.h
xen/include/xen/grant_table.h

index 37262a7ef84dccda30c1bea8f2cf89121bcf6c0b..398b6cdf41dd295a4d0759f002aeeed9e35f077c 100644 (file)
@@ -81,13 +81,15 @@ Default: C</var/lock/xl>
 
 Sets the default value for the C<max_grant_frames> domain config value.
 
-Default: C<32> on hosts up to 16TB of memory, C<64> on hosts larger than 16TB
+Default: value of Xen command line B<gnttab_max_frames> parameter (or its
+default value if unspecified).
 
 =item B<max_maptrack_frames=NUMBER>
 
 Sets the default value for the C<max_maptrack_frames> domain config value.
 
-Default: C<1024>
+Default: value of Xen command line B<gnttab_max_maptrack_frames>
+parameter (or its default value if unspecified).
 
 =item B<vif.default.script="PATH">
 
index a38e5cdba2d60e8c82b9cc4aed81163f31509cb7..29208ec8ee1aad5a0aa791bc858c71e83261d3ef 100644 (file)
  */
 #define LIBXL_HAVE_BUILDINFO_GRANT_LIMITS 1
 
-#define LIBXL_MAX_GRANT_FRAMES_DEFAULT 32
-#define LIBXL_MAX_MAPTRACK_FRAMES_DEFAULT 1024
+#define LIBXL_MAX_GRANT_DEFAULT (~(uint32_t)0)
+#define LIBXL_MAX_GRANT_FRAMES_DEFAULT 32 /* deprecated */
+#define LIBXL_MAX_MAPTRACK_FRAMES_DEFAULT 1024 /* deprecated */
+/*
+ * LIBXL_HAVE_BUILDINFO_GRANT_DEFAULT indicates that the default
+ * values of max_grant_frames and max_maptrack_frames fields in
+ * libxl_domain_build_info are the special sentinel value
+ * LIBXL_MAX_GRANT_DEFAULT rather than the fixed values above.
+ * This means to use the hypervisor's default.
+ */
+#define LIBXL_HAVE_BUILDINFO_GRANT_DEFAULT 1
 
 /*
  * LIBXL_HAVE_BUILDINFO_* indicates that libxl_domain_build_info has
index b685ac47acb3f0426697a9dac3def4816121a83b..359932060582ac7fe87bf2bb48c422eeb73682bf 100644 (file)
@@ -493,8 +493,8 @@ libxl_domain_build_info = Struct("domain_build_info",[
 
     ("vnuma_nodes", Array(libxl_vnode_info, "num_vnuma_nodes")),
 
-    ("max_grant_frames",    uint32, {'init_val': 'LIBXL_MAX_GRANT_FRAMES_DEFAULT'}),
-    ("max_maptrack_frames", uint32, {'init_val': 'LIBXL_MAX_MAPTRACK_FRAMES_DEFAULT'}),
+    ("max_grant_frames",    uint32, {'init_val': 'LIBXL_MAX_GRANT_DEFAULT'}),
+    ("max_maptrack_frames", uint32, {'init_val': 'LIBXL_MAX_GRANT_DEFAULT'}),
     
     ("device_model_version", libxl_device_model_version),
     ("device_model_stubdomain", libxl_defbool),
index 5838f6885e82118f0d1ef7b608bee26d77367e93..239ca1f26b8743bd8f4ceb3ed3fcaad0d3657fca 100644 (file)
@@ -268,8 +268,9 @@ int xlu_cfg_replace_string(const XLU_Config *cfg, const char *n,
     return 0;
 }
 
-int xlu_cfg_get_long(const XLU_Config *cfg, const char *n,
-                     long *value_r, int dont_warn) {
+int xlu_cfg_get_bounded_long(const XLU_Config *cfg, const char *n,
+                             long min, long max, long *value_r,
+                             int dont_warn) {
     long l;
     XLU_ConfigSetting *set;
     int e;
@@ -295,10 +296,31 @@ int xlu_cfg_get_long(const XLU_Config *cfg, const char *n,
                     cfg->config_source, set->lineno, n);
         return EINVAL;
     }
+    if (l < min) {
+        if (!dont_warn)
+            fprintf(cfg->report,
+                    "%s:%d: warning: value `%ld' is smaller than minimum bound '%ld'\n",
+                    cfg->config_source, set->lineno, l, min);
+        return EINVAL;
+    }
+    if (l > max) {
+        if (!dont_warn)
+            fprintf(cfg->report,
+                    "%s:%d: warning: value `%ld' is greater than maximum bound '%ld'\n",
+                    cfg->config_source, set->lineno, l, max);
+        return EINVAL;
+    }
+
     *value_r= l;
     return 0;
 }
 
+int xlu_cfg_get_long(const XLU_Config *cfg, const char *n,
+                     long *value_r, int dont_warn) {
+    return xlu_cfg_get_bounded_long(cfg, n, LONG_MIN, LONG_MAX, value_r,
+                                    dont_warn);
+}
+
 int xlu_cfg_get_defbool(const XLU_Config *cfg, const char *n, libxl_defbool *b,
                      int dont_warn)
 {
index e81b644c01f1ed37c88d726422bf296d61a06580..2d066bf5153516dbaef84d9734a68a82b182c286 100644 (file)
@@ -58,6 +58,8 @@ int xlu_cfg_replace_string(const XLU_Config *cfg, const char *n,
                            char **value_r, int dont_warn);
 int xlu_cfg_get_long(const XLU_Config*, const char *n, long *value_r,
                      int dont_warn);
+int xlu_cfg_get_bounded_long(const XLU_Config*, const char *n, long min,
+                             long max, long *value_r, int dont_warn);
 int xlu_cfg_get_defbool(const XLU_Config*, const char *n, libxl_defbool *b,
                      int dont_warn);
 
index cc8175a11e06e21f29ef9727fcff19797cd878d7..2dd25f5498491630567810ed40dc205c646fb16d 100644 (file)
@@ -127,8 +127,8 @@ static PyObject *pyxc_domain_create(XcObject *self,
         },
         .max_vcpus = 1,
         .max_evtchn_port = -1, /* No limit. */
-        .max_grant_frames = 32,
-        .max_maptrack_frames = 1024,
+        .max_grant_frames = -1,
+        .max_maptrack_frames = -1,
     };
 
     static char *kwd_list[] = { "domid", "ssidref", "handle", "flags",
index ddd29b3f1b2d591e2aef1aa883dc367587f3e2bb..3d4390a46dcb554475f7d34bcb91c671db47f2cc 100644 (file)
@@ -23,6 +23,7 @@
 #include <ctype.h>
 #include <inttypes.h>
 #include <regex.h>
+#include <limits.h>
 
 #include <libxl.h>
 #include <libxl_utils.h>
@@ -96,7 +97,6 @@ static void parse_global_config(const char *configfile,
     XLU_Config *config;
     int e;
     const char *buf;
-    libxl_physinfo physinfo;
 
     config = xlu_cfg_init(stderr, configfile);
     if (!config) {
@@ -197,17 +197,19 @@ static void parse_global_config(const char *configfile,
     xlu_cfg_replace_string (config, "colo.default.proxyscript",
         &default_colo_proxy_script, 0);
 
-    if (!xlu_cfg_get_long (config, "max_grant_frames", &l, 0))
+    e = xlu_cfg_get_bounded_long (config, "max_grant_frames", 0, INT_MAX,
+                                  &l, 1);
+    if (!e)
         max_grant_frames = l;
-    else {
-        libxl_physinfo_init(&physinfo);
-        max_grant_frames = (libxl_get_physinfo(ctx, &physinfo) != 0 ||
-                            !(physinfo.max_possible_mfn >> 32))
-                           ? 32 : 64;
-        libxl_physinfo_dispose(&physinfo);
-    }
-    if (!xlu_cfg_get_long (config, "max_maptrack_frames", &l, 0))
+    else if (e != ESRCH)
+        exit(1);
+
+    e = xlu_cfg_get_bounded_long (config, "max_maptrack_frames", 0,
+                                  INT_MAX, &l, 1);
+    if (!e)
         max_maptrack_frames = l;
+    else if (e != ESRCH)
+        exit(1);
 
     libxl_cpu_bitmap_alloc(ctx, &global_vm_affinity_mask, 0);
     libxl_cpu_bitmap_alloc(ctx, &global_hvm_affinity_mask, 0);
index 352cd214ddf58b94a752d1748e23493d2155e2d2..a03992067b0fc56556b0139e9ef6557382d9e5f0 100644 (file)
@@ -1401,14 +1401,23 @@ void parse_config_data(const char *config_source,
         !xlu_cfg_get_string (config, "cpus_soft", &buf, 0))
         parse_vcpu_affinity(b_info, cpus, buf, num_cpus, false);
 
-    if (!xlu_cfg_get_long (config, "max_grant_frames", &l, 0))
+    e = xlu_cfg_get_bounded_long (config, "max_grant_frames", 0, INT_MAX,
+                                  &l, 1);
+    if (e == ESRCH) /* not specified */
+        b_info->max_grant_frames = max_grant_frames;
+    else if (!e)
         b_info->max_grant_frames = l;
     else
-        b_info->max_grant_frames = max_grant_frames;
-    if (!xlu_cfg_get_long (config, "max_maptrack_frames", &l, 0))
-        b_info->max_maptrack_frames = l;
-    else if (max_maptrack_frames != -1)
+        exit(1);
+
+    e = xlu_cfg_get_bounded_long (config, "max_maptrack_frames", 0,
+                                  INT_MAX, &l, 1);
+    if (e == ESRCH) /* not specified */
         b_info->max_maptrack_frames = max_maptrack_frames;
+    else if (!e)
+        b_info->max_maptrack_frames = l;
+    else
+        exit(1);
 
     libxl_defbool_set(&b_info->claim_mode, claim_mode);
 
index 7fe02120a735db23a0922d952cae402e2bb7cd00..d986f84f8d2a689beb799731274c298784b06e4e 100644 (file)
@@ -741,7 +741,7 @@ void __init start_xen(unsigned long boot_phys_offset,
         .flags = XEN_DOMCTL_CDF_hvm_guest | XEN_DOMCTL_CDF_hap,
         .max_evtchn_port = -1,
         .max_grant_frames = gnttab_dom0_frames(),
-        .max_maptrack_frames = opt_max_maptrack_frames,
+        .max_maptrack_frames = -1,
     };
 
     dcache_line_bytes = read_dcache_line_bytes();
index c1c7c4400083d9ff7fac6a607e26154b8fa6db7c..5ab53e3d8560e5b191f4ad519c87a964d4e648db 100644 (file)
@@ -689,8 +689,8 @@ void __init noreturn __start_xen(unsigned long mbi_p)
     struct xen_domctl_createdomain dom0_cfg = {
         .flags = XEN_DOMCTL_CDF_s3_integrity,
         .max_evtchn_port = -1,
-        .max_grant_frames = opt_max_grant_frames,
-        .max_maptrack_frames = opt_max_maptrack_frames,
+        .max_grant_frames = -1,
+        .max_maptrack_frames = -1,
     };
 
     /* Critical region without IDT or TSS.  Any fault is deadly! */
index e9ce0ac4735516a29c0b64c12dd4ee54b0d465d2..daac9924fc7d3760f3d10405f9226e802d49bce4 100644 (file)
@@ -83,11 +83,42 @@ struct grant_table {
     struct grant_table_arch arch;
 };
 
+static int parse_gnttab_limit(const char *param, const char *arg,
+                              unsigned int *valp)
+{
+    const char *e;
+    unsigned long val;
+
+    val = simple_strtoul(arg, &e, 0);
+    if ( *e )
+        return -EINVAL;
+
+    if ( val > INT_MAX )
+        return -ERANGE;
+
+    *valp = val;
+
+    return 0;
+}
+
 unsigned int __read_mostly opt_max_grant_frames = 64;
-integer_runtime_param("gnttab_max_frames", opt_max_grant_frames);
 
-unsigned int __read_mostly opt_max_maptrack_frames = 1024;
-integer_runtime_param("gnttab_max_maptrack_frames", opt_max_maptrack_frames);
+static int parse_gnttab_max_frames(const char *arg)
+{
+    return parse_gnttab_limit("gnttab_max_frames", arg,
+                              &opt_max_grant_frames);
+}
+custom_runtime_param("gnttab_max_frames", parse_gnttab_max_frames);
+
+static unsigned int __read_mostly opt_max_maptrack_frames = 1024;
+
+static int parse_gnttab_max_maptrack_frames(const char *arg)
+{
+    return parse_gnttab_limit("gnttab_max_maptrack_frames", arg,
+                              &opt_max_maptrack_frames);
+}
+custom_runtime_param("gnttab_max_maptrack_frames",
+                     parse_gnttab_max_maptrack_frames);
 
 #ifndef GNTTAB_MAX_VERSION
 #define GNTTAB_MAX_VERSION 2
@@ -1802,12 +1833,18 @@ active_alloc_failed:
     return -ENOMEM;
 }
 
-int grant_table_init(struct domain *d, unsigned int max_grant_frames,
-                     unsigned int max_maptrack_frames)
+int grant_table_init(struct domain *d, int max_grant_frames,
+                     int max_maptrack_frames)
 {
     struct grant_table *gt;
     int ret = -ENOMEM;
 
+    /* Default to maximum value if no value was specified */
+    if ( max_grant_frames < 0 )
+        max_grant_frames = opt_max_grant_frames;
+    if ( max_maptrack_frames < 0 )
+        max_maptrack_frames = opt_max_maptrack_frames;
+
     if ( max_grant_frames < INITIAL_NR_GRANT_FRAMES ||
          max_grant_frames > opt_max_grant_frames ||
          max_maptrack_frames > opt_max_maptrack_frames )
index 6f9be8166ee7057c11df9b77aa5290962f31f681..01f142289e6e10c98f21b294094ccd4da4a925d2 100644 (file)
@@ -67,13 +67,15 @@ struct xen_domctl_createdomain {
     uint32_t flags;
 
     /*
-     * Various domain limits, which impact the quantity of resources (global
-     * mapping space, xenheap, etc) a guest may consume.
+     * Various domain limits, which impact the quantity of resources
+     * (global mapping space, xenheap, etc) a guest may consume.  For
+     * max_grant_frames and max_maptrack_frames, < 0 means "use the
+     * default maximum value in the hypervisor".
      */
     uint32_t max_vcpus;
     uint32_t max_evtchn_port;
-    uint32_t max_grant_frames;
-    uint32_t max_maptrack_frames;
+    int32_t max_grant_frames;
+    int32_t max_maptrack_frames;
 
     struct xen_arch_domainconfig arch;
 };
index 12e8a4b80b1fd3235c33112ac5edc9b05d6d1f49..119b8f7e2ba8c9fa58b2b7a14a5a908533d6cbee 100644 (file)
 struct grant_table;
 
 extern unsigned int opt_max_grant_frames;
-extern unsigned int opt_max_maptrack_frames;
 
 /* Create/destroy per-domain grant table context. */
-int grant_table_init(struct domain *d, unsigned int max_grant_frames,
-                     unsigned int max_maptrack_frames);
+int grant_table_init(struct domain *d, int max_grant_frames,
+                     int max_maptrack_frames);
 void grant_table_destroy(
     struct domain *d);
 void grant_table_init_vcpu(struct vcpu *v);