| Message ID | 20191128125032.902-1-pdurrant@amazon.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [PATCH-for-4.13,v3] Rationalize max_grant_frames and max_maptrack_frames handling | expand |
> -----Original Message----- > From: Paul Durrant <pdurrant@amazon.com> > Sent: 28 November 2019 12:51 > To: xen-devel@lists.xenproject.org > Cc: George Dunlap <george.dunlap@citrix.com>; Durrant, Paul > <pdurrant@amazon.com>; Ian Jackson <ian.jackson@eu.citrix.com>; Wei Liu > <wl@xen.org>; Andrew Cooper <andrew.cooper3@citrix.com>; George Dunlap > <George.Dunlap@eu.citrix.com>; Jan Beulich <jbeulich@suse.com>; Julien > Grall <julien@xen.org>; Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; > Stefano Stabellini <sstabellini@kernel.org>; Anthony PERARD > <anthony.perard@citrix.com>; Marek Marczykowski-Górecki > <marmarek@invisiblethingslab.com>; Volodymyr Babchuk > <Volodymyr_Babchuk@epam.com>; Roger Pau Monné <roger.pau@citrix.com> > Subject: [PATCH-for-4.13 v3] Rationalize max_grant_frames and > max_maptrack_frames handling > > From: George Dunlap <george.dunlap@citrix.com> > > 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> > --- > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > Cc: Wei Liu <wl@xen.org> > Cc: Andrew Cooper <andrew.cooper3@citrix.com> > Cc: George Dunlap <George.Dunlap@eu.citrix.com> > Cc: Jan Beulich <jbeulich@suse.com> > Cc: Julien Grall <julien@xen.org> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > Cc: Stefano Stabellini <sstabellini@kernel.org> > Cc: Anthony PERARD <anthony.perard@citrix.com> > Cc: "Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com> > Cc: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> > Cc: "Roger Pau Monné" <roger.pau@citrix.com> > > v3: > - Make sure that specified values cannot be negative or overflow a > signed int > > v2: > - re-worked George's original commit massage a little > - fixed the text in xl.conf.5.pod > - use -1 as the sentinel value for 'default' and < 0 for checking it > --- > docs/man/xl.conf.5.pod | 6 ++-- > tools/libxl/libxl.h | 4 +-- > tools/libxl/libxl_types.idl | 4 +-- > tools/libxl/libxlu_cfg.c | 24 ++++++++++++++-- > tools/libxl/libxlutil.h | 2 ++ > tools/python/xen/lowlevel/xc/xc.c | 4 +-- > tools/xl/xl.c | 15 ++++------ > tools/xl/xl_parse.c | 9 ++++-- > xen/arch/arm/setup.c | 2 +- > xen/arch/x86/setup.c | 4 +-- > xen/common/grant_table.c | 46 ++++++++++++++++++++++++++++--- > xen/include/public/domctl.h | 10 ++++--- > xen/include/xen/grant_table.h | 8 +++--- > 13 files changed, 100 insertions(+), 38 deletions(-) > > diff --git a/docs/man/xl.conf.5.pod b/docs/man/xl.conf.5.pod > index 962144e38e..207ab3e77a 100644 > --- a/docs/man/xl.conf.5.pod > +++ b/docs/man/xl.conf.5.pod > @@ -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"> > > diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h > index 49b56fa1a3..a2a5d321c5 100644 > --- a/tools/libxl/libxl.h > +++ b/tools/libxl/libxl.h > @@ -364,8 +364,8 @@ > */ > #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_FRAMES_DEFAULT -1 > +#define LIBXL_MAX_MAPTRACK_FRAMES_DEFAULT -1 > > /* > * LIBXL_HAVE_BUILDINFO_* indicates that libxl_domain_build_info has > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl > index 0546d7865a..63e29bb2fb 100644 > --- a/tools/libxl/libxl_types.idl > +++ b/tools/libxl/libxl_types.idl > @@ -511,8 +511,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", integer, {'init_val': > 'LIBXL_MAX_GRANT_FRAMES_DEFAULT'}), > + ("max_maptrack_frames", integer, {'init_val': > 'LIBXL_MAX_MAPTRACK_FRAMES_DEFAULT'}), > > ("device_model_version", libxl_device_model_version), > ("device_model_stubdomain", libxl_defbool), > diff --git a/tools/libxl/libxlu_cfg.c b/tools/libxl/libxlu_cfg.c > index 72815d25dd..09d5c78a46 100644 > --- a/tools/libxl/libxlu_cfg.c > +++ b/tools/libxl/libxlu_cfg.c > @@ -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; > @@ -303,10 +304,29 @@ int xlu_cfg_get_long(const XLU_Config *cfg, const > char *n, > cfg->config_source, set->lineno, n); > return EINVAL; > } > + if (l < min) ...and, of course, there's missing braces here and below. Don't know why the compiler didn't complain... it’s pretty new. Anyway I'll send v4 shortly. Paul > + 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) > { > diff --git a/tools/libxl/libxlutil.h b/tools/libxl/libxlutil.h > index 057cc25cb2..92e35c5462 100644 > --- a/tools/libxl/libxlutil.h > +++ b/tools/libxl/libxlutil.h > @@ -63,6 +63,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); > > diff --git a/tools/python/xen/lowlevel/xc/xc.c > b/tools/python/xen/lowlevel/xc/xc.c > index 44d3606141..a751e85910 100644 > --- a/tools/python/xen/lowlevel/xc/xc.c > +++ b/tools/python/xen/lowlevel/xc/xc.c > @@ -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", > diff --git a/tools/xl/xl.c b/tools/xl/xl.c > index ddd29b3f1b..921c64f5ed 100644 > --- a/tools/xl/xl.c > +++ b/tools/xl/xl.c > @@ -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,16 +197,11 @@ 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)) > + if (!xlu_cfg_get_bounded_long (config, "max_grant_frames", 0, > INT_MAX, > + &l, 0)) > 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)) > + if (!xlu_cfg_get_bounded_long (config, "max_maptrack_frames", 0, > + INT_MAX, &l, 0)) > max_maptrack_frames = l; > > libxl_cpu_bitmap_alloc(ctx, &global_vm_affinity_mask, 0); > diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c > index 112f8ee026..555991dae3 100644 > --- a/tools/xl/xl_parse.c > +++ b/tools/xl/xl_parse.c > @@ -1411,13 +1411,16 @@ 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)) > + if (!xlu_cfg_get_bounded_long (config, "max_grant_frames", 0, > INT_MAX, > + &l, 0)) > 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)) > + > + if (!xlu_cfg_get_bounded_long (config, "max_maptrack_frames", 0, > + INT_MAX, &l, 0)) > b_info->max_maptrack_frames = l; > - else if (max_maptrack_frames != -1) > + else > b_info->max_maptrack_frames = max_maptrack_frames; > > libxl_defbool_set(&b_info->claim_mode, claim_mode); > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c > index 51d32106b7..3c899cd4a0 100644 > --- a/xen/arch/arm/setup.c > +++ b/xen/arch/arm/setup.c > @@ -789,7 +789,7 @@ void __init start_xen(unsigned long boot_phys_offset, > .flags = XEN_DOMCTL_CDF_hvm | 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, > }; > int rc; > > diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c > index 00ee87bde5..7d27f36053 100644 > --- a/xen/arch/x86/setup.c > +++ b/xen/arch/x86/setup.c > @@ -697,8 +697,8 @@ void __init noreturn __start_xen(unsigned long mbi_p) > struct xen_domctl_createdomain dom0_cfg = { > .flags = IS_ENABLED(CONFIG_TBOOT) ? XEN_DOMCTL_CDF_s3_integrity : > 0, > .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! */ > diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c > index b34d520f6d..f5053a6ee8 100644 > --- a/xen/common/grant_table.c > +++ b/xen/common/grant_table.c > @@ -84,11 +84,43 @@ struct grant_table { > struct grant_table_arch arch; > }; > > +static int __init 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 >= 0 && val <= INT_MAX ) > + *valp = val; > + else > + printk("%s: value '%s' is out of range; using value '%u'\n", > + param, arg, *valp); > + > + return 0; > +} > + > unsigned int __read_mostly opt_max_grant_frames = 64; > -integer_runtime_param("gnttab_max_frames", opt_max_grant_frames); > + > +static int __init 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); > > unsigned int __read_mostly opt_max_maptrack_frames = 1024; > -integer_runtime_param("gnttab_max_maptrack_frames", > opt_max_maptrack_frames); > + > +static int __init 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 > @@ -1837,12 +1869,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 ) > diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h > index 9f2cfd602c..e313da499f 100644 > --- a/xen/include/public/domctl.h > +++ b/xen/include/public/domctl.h > @@ -82,13 +82,15 @@ struct xen_domctl_createdomain { > uint32_t iommu_opts; > > /* > - * 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; > }; > diff --git a/xen/include/xen/grant_table.h b/xen/include/xen/grant_table.h > index 6f9345d9ef..34886bb6f8 100644 > --- a/xen/include/xen/grant_table.h > +++ b/xen/include/xen/grant_table.h > @@ -36,8 +36,8 @@ 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); > @@ -68,8 +68,8 @@ int gnttab_get_status_frame(struct domain *d, unsigned > long idx, > #define opt_max_maptrack_frames 0 > > static inline int grant_table_init(struct domain *d, > - unsigned int max_grant_frames, > - unsigned int max_maptrack_frames) > + int max_grant_frames, > + int max_maptrack_frames) > { > return 0; > } > -- > 2.20.1
diff --git a/docs/man/xl.conf.5.pod b/docs/man/xl.conf.5.pod index 962144e38e..207ab3e77a 100644 --- a/docs/man/xl.conf.5.pod +++ b/docs/man/xl.conf.5.pod @@ -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"> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h index 49b56fa1a3..a2a5d321c5 100644 --- a/tools/libxl/libxl.h +++ b/tools/libxl/libxl.h @@ -364,8 +364,8 @@ */ #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_FRAMES_DEFAULT -1 +#define LIBXL_MAX_MAPTRACK_FRAMES_DEFAULT -1 /* * LIBXL_HAVE_BUILDINFO_* indicates that libxl_domain_build_info has diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl index 0546d7865a..63e29bb2fb 100644 --- a/tools/libxl/libxl_types.idl +++ b/tools/libxl/libxl_types.idl @@ -511,8 +511,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", integer, {'init_val': 'LIBXL_MAX_GRANT_FRAMES_DEFAULT'}), + ("max_maptrack_frames", integer, {'init_val': 'LIBXL_MAX_MAPTRACK_FRAMES_DEFAULT'}), ("device_model_version", libxl_device_model_version), ("device_model_stubdomain", libxl_defbool), diff --git a/tools/libxl/libxlu_cfg.c b/tools/libxl/libxlu_cfg.c index 72815d25dd..09d5c78a46 100644 --- a/tools/libxl/libxlu_cfg.c +++ b/tools/libxl/libxlu_cfg.c @@ -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; @@ -303,10 +304,29 @@ 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) { diff --git a/tools/libxl/libxlutil.h b/tools/libxl/libxlutil.h index 057cc25cb2..92e35c5462 100644 --- a/tools/libxl/libxlutil.h +++ b/tools/libxl/libxlutil.h @@ -63,6 +63,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); diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c index 44d3606141..a751e85910 100644 --- a/tools/python/xen/lowlevel/xc/xc.c +++ b/tools/python/xen/lowlevel/xc/xc.c @@ -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", diff --git a/tools/xl/xl.c b/tools/xl/xl.c index ddd29b3f1b..921c64f5ed 100644 --- a/tools/xl/xl.c +++ b/tools/xl/xl.c @@ -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,16 +197,11 @@ 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)) + if (!xlu_cfg_get_bounded_long (config, "max_grant_frames", 0, INT_MAX, + &l, 0)) 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)) + if (!xlu_cfg_get_bounded_long (config, "max_maptrack_frames", 0, + INT_MAX, &l, 0)) max_maptrack_frames = l; libxl_cpu_bitmap_alloc(ctx, &global_vm_affinity_mask, 0); diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c index 112f8ee026..555991dae3 100644 --- a/tools/xl/xl_parse.c +++ b/tools/xl/xl_parse.c @@ -1411,13 +1411,16 @@ 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)) + if (!xlu_cfg_get_bounded_long (config, "max_grant_frames", 0, INT_MAX, + &l, 0)) 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)) + + if (!xlu_cfg_get_bounded_long (config, "max_maptrack_frames", 0, + INT_MAX, &l, 0)) b_info->max_maptrack_frames = l; - else if (max_maptrack_frames != -1) + else b_info->max_maptrack_frames = max_maptrack_frames; libxl_defbool_set(&b_info->claim_mode, claim_mode); diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c index 51d32106b7..3c899cd4a0 100644 --- a/xen/arch/arm/setup.c +++ b/xen/arch/arm/setup.c @@ -789,7 +789,7 @@ void __init start_xen(unsigned long boot_phys_offset, .flags = XEN_DOMCTL_CDF_hvm | 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, }; int rc; diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c index 00ee87bde5..7d27f36053 100644 --- a/xen/arch/x86/setup.c +++ b/xen/arch/x86/setup.c @@ -697,8 +697,8 @@ void __init noreturn __start_xen(unsigned long mbi_p) struct xen_domctl_createdomain dom0_cfg = { .flags = IS_ENABLED(CONFIG_TBOOT) ? XEN_DOMCTL_CDF_s3_integrity : 0, .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! */ diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c index b34d520f6d..f5053a6ee8 100644 --- a/xen/common/grant_table.c +++ b/xen/common/grant_table.c @@ -84,11 +84,43 @@ struct grant_table { struct grant_table_arch arch; }; +static int __init 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 >= 0 && val <= INT_MAX ) + *valp = val; + else + printk("%s: value '%s' is out of range; using value '%u'\n", + param, arg, *valp); + + return 0; +} + unsigned int __read_mostly opt_max_grant_frames = 64; -integer_runtime_param("gnttab_max_frames", opt_max_grant_frames); + +static int __init 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); unsigned int __read_mostly opt_max_maptrack_frames = 1024; -integer_runtime_param("gnttab_max_maptrack_frames", opt_max_maptrack_frames); + +static int __init 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 @@ -1837,12 +1869,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 ) diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h index 9f2cfd602c..e313da499f 100644 --- a/xen/include/public/domctl.h +++ b/xen/include/public/domctl.h @@ -82,13 +82,15 @@ struct xen_domctl_createdomain { uint32_t iommu_opts; /* - * 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; }; diff --git a/xen/include/xen/grant_table.h b/xen/include/xen/grant_table.h index 6f9345d9ef..34886bb6f8 100644 --- a/xen/include/xen/grant_table.h +++ b/xen/include/xen/grant_table.h @@ -36,8 +36,8 @@ 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); @@ -68,8 +68,8 @@ int gnttab_get_status_frame(struct domain *d, unsigned long idx, #define opt_max_maptrack_frames 0 static inline int grant_table_init(struct domain *d, - unsigned int max_grant_frames, - unsigned int max_maptrack_frames) + int max_grant_frames, + int max_maptrack_frames) { return 0; }