Message ID | 20191129172445.32664-1-pdurrant@amazon.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [PATCH-for-4.13,v7] Rationalize max_grant_frames and max_maptrack_frames handling | expand |
On Fri, Nov 29, 2019 at 05:24:45PM +0000, Paul Durrant wrote: > 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> > Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com> > Reviewed-by: Jan Beulich <jbeulich@suse.com> > Release-acked-by: Juergen Gross <jgross@suse.com> Acked-by: Wei Liu <wl@xen.org>
On Fri, Nov 29, 2019 at 05:36:11PM +0000, Wei Liu wrote: > On Fri, Nov 29, 2019 at 05:24:45PM +0000, Paul Durrant wrote: > > 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> > > Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com> > > Reviewed-by: Jan Beulich <jbeulich@suse.com> > > Release-acked-by: Juergen Gross <jgross@suse.com> > > Acked-by: Wei Liu <wl@xen.org> In theory I should wait for Marek's ack for changes to python binding, but the changes are trivial there so I plan to push this patch later tonight to both staging and staging-4.13 so that it can be tested over the weekend. Marek, I apologise in advance in case you disagree with my assessment. Wei.
> On Nov 29, 2019, at 5:44 PM, Wei Liu <wl@xen.org> wrote: > > On Fri, Nov 29, 2019 at 05:36:11PM +0000, Wei Liu wrote: >> On Fri, Nov 29, 2019 at 05:24:45PM +0000, Paul Durrant wrote: >>> 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> >>> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com> Does this win some kind of award for “most collaborative patch”? -George
On Fri, Nov 29, 2019 at 05:53:39PM +0000, George Dunlap wrote: > > > > On Nov 29, 2019, at 5:44 PM, Wei Liu <wl@xen.org> wrote: > > > > On Fri, Nov 29, 2019 at 05:36:11PM +0000, Wei Liu wrote: > >> On Fri, Nov 29, 2019 at 05:24:45PM +0000, Paul Durrant wrote: > >>> 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> > >>> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com> > > Does this win some kind of award for “most collaborative patch”? In terms of numbers of SoB, not yet. I've seen patch(es) with three SoBs. Someone should've left a typo in somewhere so that I can fix it and put my SoB in. That may make this patch the most collaborative patch I know of. :-) Wei. > > -George >
On Fri, Nov 29, 2019 at 05:44:23PM +0000, Wei Liu wrote: > On Fri, Nov 29, 2019 at 05:36:11PM +0000, Wei Liu wrote: > > On Fri, Nov 29, 2019 at 05:24:45PM +0000, Paul Durrant wrote: > > > 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> > > > Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com> > > > Reviewed-by: Jan Beulich <jbeulich@suse.com> > > > Release-acked-by: Juergen Gross <jgross@suse.com> > > > > Acked-by: Wei Liu <wl@xen.org> > > In theory I should wait for Marek's ack for changes to python binding, > but the changes are trivial there so I plan to push this patch later > tonight to both staging and staging-4.13 so that it can be tested over > the weekend. > > Marek, I apologise in advance in case you disagree with my assessment. FWIW, for python part: Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
On Fri, Nov 29, 2019 at 10:15:33PM +0100, Marek Marczykowski-Górecki wrote: > On Fri, Nov 29, 2019 at 05:44:23PM +0000, Wei Liu wrote: > > On Fri, Nov 29, 2019 at 05:36:11PM +0000, Wei Liu wrote: > > > On Fri, Nov 29, 2019 at 05:24:45PM +0000, Paul Durrant wrote: > > > > 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> > > > > Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com> > > > > Reviewed-by: Jan Beulich <jbeulich@suse.com> > > > > Release-acked-by: Juergen Gross <jgross@suse.com> > > > > > > Acked-by: Wei Liu <wl@xen.org> > > > > In theory I should wait for Marek's ack for changes to python binding, > > but the changes are trivial there so I plan to push this patch later > > tonight to both staging and staging-4.13 so that it can be tested over > > the weekend. > > > > Marek, I apologise in advance in case you disagree with my assessment. > > FWIW, for python part: > Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> Thanks. I will fold this in. Wei.
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..54abb9db1f 100644 --- a/tools/libxl/libxl.h +++ b/tools/libxl/libxl.h @@ -364,8 +364,17 @@ */ #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 diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl index 0546d7865a..7921950f6a 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", 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), diff --git a/tools/libxl/libxlu_cfg.c b/tools/libxl/libxlu_cfg.c index 72815d25dd..cafc632fc1 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,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) { 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..3d4390a46d 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,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); diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c index 112f8ee026..b881184804 100644 --- a/tools/xl/xl_parse.c +++ b/tools/xl/xl_parse.c @@ -1411,14 +1411,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); 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..729f362ea8 100644 --- a/xen/common/grant_table.c +++ b/xen/common/grant_table.c @@ -84,11 +84,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 @@ -1837,12 +1868,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..98603604b8 100644 --- a/xen/include/xen/grant_table.h +++ b/xen/include/xen/grant_table.h @@ -33,11 +33,10 @@ 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); @@ -65,11 +64,10 @@ int gnttab_get_status_frame(struct domain *d, unsigned long idx, #else #define opt_max_grant_frames 0 -#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; }