| Message ID | 20210922082123.54374-3-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | gnttab: add per-domain controls | expand |
On 22.09.2021 10:21, Roger Pau Monne wrote: > --- a/xen/common/grant_table.c > +++ b/xen/common/grant_table.c > @@ -53,6 +53,7 @@ struct grant_table { > percpu_rwlock_t lock; > /* Lock protecting the maptrack limit */ > spinlock_t maptrack_lock; > + unsigned int max_grant_version; Nit: I realize the version field also has "gt" in its name, but just like I consider that superfluous, I don't think "grant" needs to be in the field name here. > --- a/xen/include/public/domctl.h > +++ b/xen/include/public/domctl.h > @@ -87,14 +87,22 @@ struct xen_domctl_createdomain { > /* > * 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". > + * max_grant_frames, max_maptrack_frames and max_gnttab_version < 0 > + * means "use the default maximum value in the hypervisor". > */ > uint32_t max_vcpus; > uint32_t max_evtchn_port; > int32_t max_grant_frames; > int32_t max_maptrack_frames; > > +/* Grant version, use low 4 bits. */ > +#define XEN_DOMCTL_GRANT_version_mask 0xf > +#define XEN_DOMCTL_GRANT_version_default 0xf > + > +#define XEN_DOMCTLGRANT_MAX XEN_DOMCTL_GRANT_version_mask > + > + uint32_t grant_opts; As it now seems unlikely that this will make 4.16, please don't forget to bump the interface version for 4.17. With that and preferably with the nit above addressed, hypervisor parts: Reviewed-by: Jan Beulich <jbeulich@suse.com> Jan
On 15.10.2021 11:39, Jan Beulich wrote: > On 22.09.2021 10:21, Roger Pau Monne wrote: >> --- a/xen/include/public/domctl.h >> +++ b/xen/include/public/domctl.h >> @@ -87,14 +87,22 @@ struct xen_domctl_createdomain { >> /* >> * 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". >> + * max_grant_frames, max_maptrack_frames and max_gnttab_version < 0 >> + * means "use the default maximum value in the hypervisor". >> */ >> uint32_t max_vcpus; >> uint32_t max_evtchn_port; >> int32_t max_grant_frames; >> int32_t max_maptrack_frames; >> >> +/* Grant version, use low 4 bits. */ >> +#define XEN_DOMCTL_GRANT_version_mask 0xf >> +#define XEN_DOMCTL_GRANT_version_default 0xf >> + >> +#define XEN_DOMCTLGRANT_MAX XEN_DOMCTL_GRANT_version_mask >> + >> + uint32_t grant_opts; > > As it now seems unlikely that this will make 4.16, please don't forget > to bump the interface version for 4.17. With that and preferably with > the nit above addressed, hypervisor parts: > Reviewed-by: Jan Beulich <jbeulich@suse.com> Actually no, I'm afraid there is an issue with migration: If the tool stack remembers the "use default" setting and hands this to the new host, that host's default may be different from the source host's. It is the effective max-version that needs passing on in this case, which in turn requires a means to obtain the value. Jan
On 22.09.2021 10:21, Roger Pau Monne wrote: > @@ -1917,11 +1918,27 @@ active_alloc_failed: > } > > int grant_table_init(struct domain *d, int max_grant_frames, > - int max_maptrack_frames) > + int max_maptrack_frames, unsigned int options) > { > struct grant_table *gt; > + unsigned int max_grant_version = options & XEN_DOMCTL_GRANT_version_mask; > int ret = -ENOMEM; > > + if ( max_grant_version == XEN_DOMCTL_GRANT_version_default ) > + max_grant_version = opt_gnttab_max_version; > + if ( !max_grant_version ) > + { > + dprintk(XENLOG_INFO, "Invalid grant table version 0 requested\n"); > + return -EINVAL; > + } > + if ( max_grant_version > opt_gnttab_max_version ) > + { > + dprintk(XENLOG_INFO, > + "Requested grant version (%u) greater than supported (%u)\n", > + max_grant_version, opt_gnttab_max_version); > + return -EINVAL; > + } To aid diagnosis of issues, maybe also log the affected domain in both cases? Jan
On Fri, Oct 15, 2021 at 11:48:33AM +0200, Jan Beulich wrote: > On 15.10.2021 11:39, Jan Beulich wrote: > > On 22.09.2021 10:21, Roger Pau Monne wrote: > >> --- a/xen/include/public/domctl.h > >> +++ b/xen/include/public/domctl.h > >> @@ -87,14 +87,22 @@ struct xen_domctl_createdomain { > >> /* > >> * 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". > >> + * max_grant_frames, max_maptrack_frames and max_gnttab_version < 0 > >> + * means "use the default maximum value in the hypervisor". > >> */ > >> uint32_t max_vcpus; > >> uint32_t max_evtchn_port; > >> int32_t max_grant_frames; > >> int32_t max_maptrack_frames; > >> > >> +/* Grant version, use low 4 bits. */ > >> +#define XEN_DOMCTL_GRANT_version_mask 0xf > >> +#define XEN_DOMCTL_GRANT_version_default 0xf > >> + > >> +#define XEN_DOMCTLGRANT_MAX XEN_DOMCTL_GRANT_version_mask > >> + > >> + uint32_t grant_opts; > > > > As it now seems unlikely that this will make 4.16, please don't forget > > to bump the interface version for 4.17. With that and preferably with > > the nit above addressed, hypervisor parts: > > Reviewed-by: Jan Beulich <jbeulich@suse.com> > > Actually no, I'm afraid there is an issue with migration: If the tool > stack remembers the "use default" setting and hands this to the new > host, that host's default may be different from the source host's. It > is the effective max-version that needs passing on in this case, which > in turn requires a means to obtain the value. Hm, my thinking was that the admin (or a higer level orchestration tool) would already have performed the necessary adjustments to assert the environments are compatible. This problem already exist today where you can migrate a VM from a host having the max default grant version to one having gnttab=max-ver:1 without complains. Note that adding such a check would then effectively prevent us from lowering the default max grant version, as any incoming migration from a previous hypervisor using the default parameters would be rejected. Thanks, Roger.
On 20.10.2021 10:04, Roger Pau Monné wrote: > On Fri, Oct 15, 2021 at 11:48:33AM +0200, Jan Beulich wrote: >> On 15.10.2021 11:39, Jan Beulich wrote: >>> On 22.09.2021 10:21, Roger Pau Monne wrote: >>>> --- a/xen/include/public/domctl.h >>>> +++ b/xen/include/public/domctl.h >>>> @@ -87,14 +87,22 @@ struct xen_domctl_createdomain { >>>> /* >>>> * 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". >>>> + * max_grant_frames, max_maptrack_frames and max_gnttab_version < 0 >>>> + * means "use the default maximum value in the hypervisor". >>>> */ >>>> uint32_t max_vcpus; >>>> uint32_t max_evtchn_port; >>>> int32_t max_grant_frames; >>>> int32_t max_maptrack_frames; >>>> >>>> +/* Grant version, use low 4 bits. */ >>>> +#define XEN_DOMCTL_GRANT_version_mask 0xf >>>> +#define XEN_DOMCTL_GRANT_version_default 0xf >>>> + >>>> +#define XEN_DOMCTLGRANT_MAX XEN_DOMCTL_GRANT_version_mask >>>> + >>>> + uint32_t grant_opts; >>> >>> As it now seems unlikely that this will make 4.16, please don't forget >>> to bump the interface version for 4.17. With that and preferably with >>> the nit above addressed, hypervisor parts: >>> Reviewed-by: Jan Beulich <jbeulich@suse.com> >> >> Actually no, I'm afraid there is an issue with migration: If the tool >> stack remembers the "use default" setting and hands this to the new >> host, that host's default may be different from the source host's. It >> is the effective max-version that needs passing on in this case, which >> in turn requires a means to obtain the value. > > Hm, my thinking was that the admin (or a higer level orchestration > tool) would already have performed the necessary adjustments to assert > the environments are compatible. I don't think we can rely on this in the hypervisor. We may take this as a prereq for proper working, but I think we ought to detect violations and report errors in such a case. > This problem already exist today where you can migrate a VM from a > host having the max default grant version to one having > gnttab=max-ver:1 without complains. Are you sure about "without complaints"? What would a guest do if it expects to be able to use v2, since it was able to on the prior host? > Note that adding such a check would then effectively prevent us from > lowering the default max grant version, as any incoming migration from > a previous hypervisor using the default parameters would be rejected. Right, guests would need booting anew on a such restricted host. Jan
On 20.10.21 12:57, Jan Beulich wrote: > On 20.10.2021 10:04, Roger Pau Monné wrote: >> On Fri, Oct 15, 2021 at 11:48:33AM +0200, Jan Beulich wrote: >>> On 15.10.2021 11:39, Jan Beulich wrote: >>>> On 22.09.2021 10:21, Roger Pau Monne wrote: >>>>> --- a/xen/include/public/domctl.h >>>>> +++ b/xen/include/public/domctl.h >>>>> @@ -87,14 +87,22 @@ struct xen_domctl_createdomain { >>>>> /* >>>>> * 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". >>>>> + * max_grant_frames, max_maptrack_frames and max_gnttab_version < 0 >>>>> + * means "use the default maximum value in the hypervisor". >>>>> */ >>>>> uint32_t max_vcpus; >>>>> uint32_t max_evtchn_port; >>>>> int32_t max_grant_frames; >>>>> int32_t max_maptrack_frames; >>>>> >>>>> +/* Grant version, use low 4 bits. */ >>>>> +#define XEN_DOMCTL_GRANT_version_mask 0xf >>>>> +#define XEN_DOMCTL_GRANT_version_default 0xf >>>>> + >>>>> +#define XEN_DOMCTLGRANT_MAX XEN_DOMCTL_GRANT_version_mask >>>>> + >>>>> + uint32_t grant_opts; >>>> >>>> As it now seems unlikely that this will make 4.16, please don't forget >>>> to bump the interface version for 4.17. With that and preferably with >>>> the nit above addressed, hypervisor parts: >>>> Reviewed-by: Jan Beulich <jbeulich@suse.com> >>> >>> Actually no, I'm afraid there is an issue with migration: If the tool >>> stack remembers the "use default" setting and hands this to the new >>> host, that host's default may be different from the source host's. It >>> is the effective max-version that needs passing on in this case, which >>> in turn requires a means to obtain the value. >> >> Hm, my thinking was that the admin (or a higer level orchestration >> tool) would already have performed the necessary adjustments to assert >> the environments are compatible. > > I don't think we can rely on this in the hypervisor. We may take this > as a prereq for proper working, but I think we ought to detect > violations and report errors in such a case. > >> This problem already exist today where you can migrate a VM from a >> host having the max default grant version to one having >> gnttab=max-ver:1 without complains. > > Are you sure about "without complaints"? What would a guest do if it > expects to be able to use v2, since it was able to on the prior host? A Linux guest should "just work". On resume it is setting up the grant interface again like on boot (in fact there is one difference: the number of grant frames is kept from before suspending). Guest transparent migration wouldn't work in such a case, and I have no idea how other OS's will react. Juergen
On 22.09.2021 10:21, Roger Pau Monne wrote: > Introduce a new domain create field so that toolstack can specify the > maximum grant table version usable by the domain. This is plumbed into > xl and settable by the user as max_grant_version. > > Previously this was only settable on a per host basis using the > gnttab command line option. One further remark: Already with the command line control is was questionable in how far people are aware that PV guests may need to use v2, if the host has memory beyond the 16Tb address boundary. I wonder whether permitting to restrict to v1 on such hosts isn't counterproductive. I guess at the very least a warning may want logging. Jan
On Wed, Oct 20, 2021 at 12:57:11PM +0200, Jan Beulich wrote: > On 20.10.2021 10:04, Roger Pau Monné wrote: > > On Fri, Oct 15, 2021 at 11:48:33AM +0200, Jan Beulich wrote: > >> On 15.10.2021 11:39, Jan Beulich wrote: > >>> On 22.09.2021 10:21, Roger Pau Monne wrote: > >>>> --- a/xen/include/public/domctl.h > >>>> +++ b/xen/include/public/domctl.h > >>>> @@ -87,14 +87,22 @@ struct xen_domctl_createdomain { > >>>> /* > >>>> * 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". > >>>> + * max_grant_frames, max_maptrack_frames and max_gnttab_version < 0 > >>>> + * means "use the default maximum value in the hypervisor". > >>>> */ > >>>> uint32_t max_vcpus; > >>>> uint32_t max_evtchn_port; > >>>> int32_t max_grant_frames; > >>>> int32_t max_maptrack_frames; > >>>> > >>>> +/* Grant version, use low 4 bits. */ > >>>> +#define XEN_DOMCTL_GRANT_version_mask 0xf > >>>> +#define XEN_DOMCTL_GRANT_version_default 0xf > >>>> + > >>>> +#define XEN_DOMCTLGRANT_MAX XEN_DOMCTL_GRANT_version_mask > >>>> + > >>>> + uint32_t grant_opts; > >>> > >>> As it now seems unlikely that this will make 4.16, please don't forget > >>> to bump the interface version for 4.17. With that and preferably with > >>> the nit above addressed, hypervisor parts: > >>> Reviewed-by: Jan Beulich <jbeulich@suse.com> > >> > >> Actually no, I'm afraid there is an issue with migration: If the tool > >> stack remembers the "use default" setting and hands this to the new > >> host, that host's default may be different from the source host's. It > >> is the effective max-version that needs passing on in this case, which > >> in turn requires a means to obtain the value. > > > > Hm, my thinking was that the admin (or a higer level orchestration > > tool) would already have performed the necessary adjustments to assert > > the environments are compatible. > > I don't think we can rely on this in the hypervisor. We may take this > as a prereq for proper working, but I think we ought to detect > violations and report errors in such a case. > > > This problem already exist today where you can migrate a VM from a > > host having the max default grant version to one having > > gnttab=max-ver:1 without complains. > > Are you sure about "without complaints"? Without hypervisor complaining AFAICT, as the max grant version is not migrated or checked in any way ATM. > What would a guest do if it > expects to be able to use v2, since it was able to on the prior host? IMO any well behaved guest should be capable of handling both v1 and v2, and lacking v2 a guest should fallback to v1 on resume, as the grant table needs to be re-initialized anyway. I think it would be wrong (ie: a bug) for guests to assume v2 to be present on resume based on the fact it was present previously, as it would be wrong for a block frontend to assume the same features to be present on resume for example. If a guest only supports grant v2 then I think it's up to the administrator to set the max version explicitly in the config file and that would be acknowledged on the destination end and migration aborted if not supported. I think the behavior after this patch is more user friendly that the current one, as no checks are performed currently. Thanks, Roger.
diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in index 4b1e3028d2..a4bf2caafa 100644 --- a/docs/man/xl.cfg.5.pod.in +++ b/docs/man/xl.cfg.5.pod.in @@ -580,6 +580,12 @@ to have. This value controls how many pages of foreign domains can be accessed via the grant mechanism by this domain. The default value is settable via L<xl.conf(5)>. +=item B<max_grant_version=NUMBER> + +Specify the maximum grant table version the domain is allowed to use. Current +supported versions are 1 and 2. The default value is settable via +L<xl.conf(5)>. + =item B<nomigrate=BOOLEAN> Disable migration of this domain. This enables certain other features diff --git a/docs/man/xl.conf.5.pod.in b/docs/man/xl.conf.5.pod.in index b48e99131a..0a70698a7c 100644 --- a/docs/man/xl.conf.5.pod.in +++ b/docs/man/xl.conf.5.pod.in @@ -101,6 +101,13 @@ Sets the default value for the C<max_maptrack_frames> domain config value. Default: value of Xen command line B<gnttab_max_maptrack_frames> parameter (or its default value if unspecified). +=item B<max_grant_version=NUMBER> + +Sets the default value for the C<max_grant_version> domain config value. + +Default: value of Xen command line B<gnttab> parameter (or its default value if +unspecified). + =item B<vif.default.script="PATH"> Configures the default hotplug script used by virtual network devices. diff --git a/tools/helpers/init-xenstore-domain.c b/tools/helpers/init-xenstore-domain.c index 6836002f0b..7cd1aa8f7c 100644 --- a/tools/helpers/init-xenstore-domain.c +++ b/tools/helpers/init-xenstore-domain.c @@ -88,6 +88,7 @@ static int build(xc_interface *xch) */ .max_grant_frames = 4, .max_maptrack_frames = 128, + .grant_opts = 1, }; xs_fd = open("/dev/xen/xenbus_backend", O_RDWR); diff --git a/tools/include/libxl.h b/tools/include/libxl.h index b9ba16d698..7a35833312 100644 --- a/tools/include/libxl.h +++ b/tools/include/libxl.h @@ -502,6 +502,13 @@ */ #define LIBXL_HAVE_X86_MSR_RELAXED 1 +/* + * LIBXL_HAVE_MAX_GRANT_VERSION indicates libxl_domain_build_info has a + * max_grant_version field for setting the max grant table version per + * domain. + */ +#define LIBXL_HAVE_MAX_GRANT_VERSION 1 + /* * libxl ABI compatibility * diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c index e356b2106d..1ee86602ae 100644 --- a/tools/libs/light/libxl_create.c +++ b/tools/libs/light/libxl_create.c @@ -606,6 +606,9 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config *d_config, .max_evtchn_port = b_info->event_channels, .max_grant_frames = b_info->max_grant_frames, .max_maptrack_frames = b_info->max_maptrack_frames, + .grant_opts = b_info->max_grant_version == -1 + ? XEN_DOMCTL_GRANT_version_default + : b_info->max_grant_version, .vmtrace_size = ROUNDUP(b_info->vmtrace_buf_kb << 10, XC_PAGE_SHIFT), }; diff --git a/tools/libs/light/libxl_dm.c b/tools/libs/light/libxl_dm.c index 9d93056b5c..9a8ddbe188 100644 --- a/tools/libs/light/libxl_dm.c +++ b/tools/libs/light/libxl_dm.c @@ -2320,6 +2320,7 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss) dm_config->b_info.max_grant_frames = guest_config->b_info.max_grant_frames; dm_config->b_info.max_maptrack_frames = guest_config->b_info.max_maptrack_frames; + dm_config->b_info.max_grant_version = guest_config->b_info.max_grant_version; dm_config->b_info.u.pv.features = ""; diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl index 3f9fff653a..37789a568c 100644 --- a/tools/libs/light/libxl_types.idl +++ b/tools/libs/light/libxl_types.idl @@ -518,6 +518,7 @@ libxl_domain_build_info = Struct("domain_build_info",[ ("max_grant_frames", uint32, {'init_val': 'LIBXL_MAX_GRANT_DEFAULT'}), ("max_maptrack_frames", uint32, {'init_val': 'LIBXL_MAX_GRANT_DEFAULT'}), + ("max_grant_version", integer, {'init_val': '-1'}), ("device_model_version", libxl_device_model_version), ("device_model_stubdomain", libxl_defbool), diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml index a5588c643f..6a8658bfec 100644 --- a/tools/ocaml/libs/xc/xenctrl.ml +++ b/tools/ocaml/libs/xc/xenctrl.ml @@ -83,6 +83,7 @@ type domctl_create_config = max_evtchn_port: int; max_grant_frames: int; max_maptrack_frames: int; + max_grant_version: int; arch: arch_domainconfig; } diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli index 6e94940a8a..5933d32c70 100644 --- a/tools/ocaml/libs/xc/xenctrl.mli +++ b/tools/ocaml/libs/xc/xenctrl.mli @@ -75,6 +75,7 @@ type domctl_create_config = { max_evtchn_port: int; max_grant_frames: int; max_maptrack_frames: int; + max_grant_version: int; arch: arch_domainconfig; } diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c b/tools/ocaml/libs/xc/xenctrl_stubs.c index ad953d36bd..1e60925069 100644 --- a/tools/ocaml/libs/xc/xenctrl_stubs.c +++ b/tools/ocaml/libs/xc/xenctrl_stubs.c @@ -188,7 +188,8 @@ CAMLprim value stub_xc_domain_create(value xch, value wanted_domid, value config #define VAL_MAX_EVTCHN_PORT Field(config, 5) #define VAL_MAX_GRANT_FRAMES Field(config, 6) #define VAL_MAX_MAPTRACK_FRAMES Field(config, 7) -#define VAL_ARCH Field(config, 8) +#define VAL_MAX_GRANT_VERSION Field(config, 8) +#define VAL_ARCH Field(config, 9) uint32_t domid = Int_val(wanted_domid); int result; @@ -198,6 +199,9 @@ CAMLprim value stub_xc_domain_create(value xch, value wanted_domid, value config .max_evtchn_port = Int_val(VAL_MAX_EVTCHN_PORT), .max_grant_frames = Int_val(VAL_MAX_GRANT_FRAMES), .max_maptrack_frames = Int_val(VAL_MAX_MAPTRACK_FRAMES), + .grant_opts = Int_val(VAL_MAX_GRANT_VERSION) == -1 + ? XEN_DOMCTL_GRANT_version_default + : Int_val(VAL_MAX_GRANT_VERSION), }; domain_handle_of_uuid_string(cfg.handle, String_val(VAL_HANDLE)); @@ -251,6 +255,7 @@ CAMLprim value stub_xc_domain_create(value xch, value wanted_domid, value config } #undef VAL_ARCH +#undef VAL_MAX_GRANT_VERSION #undef VAL_MAX_MAPTRACK_FRAMES #undef VAL_MAX_GRANT_FRAMES #undef VAL_MAX_EVTCHN_PORT diff --git a/tools/xl/xl.c b/tools/xl/xl.c index 4107d10fd4..0fde879cf4 100644 --- a/tools/xl/xl.c +++ b/tools/xl/xl.c @@ -55,6 +55,7 @@ bool progress_use_cr = 0; bool timestamps = 0; int max_grant_frames = -1; int max_maptrack_frames = -1; +int max_grant_version = -1; libxl_domid domid_policy = INVALID_DOMID; xentoollog_level minmsglevel = minmsglevel_default; @@ -213,6 +214,13 @@ static void parse_global_config(const char *configfile, else if (e != ESRCH) exit(1); + e = xlu_cfg_get_bounded_long (config, "max_grant_version", 0, + INT_MAX, &l, 1); + if (!e) + max_grant_version = 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); libxl_cpu_bitmap_alloc(ctx, &global_pv_affinity_mask, 0); diff --git a/tools/xl/xl.h b/tools/xl/xl.h index 7e23f30192..cf12c79a9b 100644 --- a/tools/xl/xl.h +++ b/tools/xl/xl.h @@ -282,6 +282,7 @@ extern char *default_colo_proxy_script; extern char *blkdev_start; extern int max_grant_frames; extern int max_maptrack_frames; +extern int max_grant_version; extern libxl_bitmap global_vm_affinity_mask; extern libxl_bitmap global_hvm_affinity_mask; extern libxl_bitmap global_pv_affinity_mask; diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c index 17dddb4cd5..1206d7ea51 100644 --- a/tools/xl/xl_parse.c +++ b/tools/xl/xl_parse.c @@ -1431,6 +1431,15 @@ void parse_config_data(const char *config_source, else exit(1); + e = xlu_cfg_get_bounded_long (config, "max_grant_version", 0, + INT_MAX, &l, 1); + if (e == ESRCH) /* not specified */ + b_info->max_grant_version = max_grant_version; + else if (!e) + b_info->max_grant_version = l; + else + exit(1); + libxl_defbool_set(&b_info->claim_mode, claim_mode); if (xlu_cfg_get_string (config, "on_poweroff", &buf, 0)) diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index d233d634c1..3beb1cbb41 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -2649,6 +2649,7 @@ void __init create_domUs(void) .max_evtchn_port = -1, .max_grant_frames = -1, .max_maptrack_frames = -1, + .grant_opts = XEN_DOMCTL_GRANT_version_default, }; if ( !dt_device_is_compatible(node, "xen,domain") ) @@ -2756,6 +2757,7 @@ void __init create_dom0(void) .max_evtchn_port = -1, .max_grant_frames = gnttab_dom0_frames(), .max_maptrack_frames = -1, + .grant_opts = XEN_DOMCTL_GRANT_version_default, }; /* The vGIC for DOM0 is exactly emulating the hardware GIC */ diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c index b101565f14..b5b6c75447 100644 --- a/xen/arch/x86/setup.c +++ b/xen/arch/x86/setup.c @@ -750,6 +750,7 @@ static struct domain *__init create_dom0(const module_t *image, .max_evtchn_port = -1, .max_grant_frames = -1, .max_maptrack_frames = -1, + .grant_opts = XEN_DOMCTL_GRANT_version_default, .max_vcpus = dom0_max_vcpus(), .arch = { .misc_flags = opt_dom0_msr_relaxed ? XEN_X86_MSR_RELAXED : 0, diff --git a/xen/common/domain.c b/xen/common/domain.c index 6ee5d033b0..6519272c47 100644 --- a/xen/common/domain.c +++ b/xen/common/domain.c @@ -669,7 +669,8 @@ struct domain *domain_create(domid_t domid, init_status |= INIT_evtchn; if ( (err = grant_table_init(d, config->max_grant_frames, - config->max_maptrack_frames)) != 0 ) + config->max_maptrack_frames, + config->grant_opts) != 0 ) ) goto fail; init_status |= INIT_gnttab; diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c index fe1fc11b22..c43e9d5ee4 100644 --- a/xen/common/grant_table.c +++ b/xen/common/grant_table.c @@ -53,6 +53,7 @@ struct grant_table { percpu_rwlock_t lock; /* Lock protecting the maptrack limit */ spinlock_t maptrack_lock; + unsigned int max_grant_version; /* * Defaults to v1. May be changed with GNTTABOP_set_version. All other * values are invalid. @@ -1917,11 +1918,27 @@ active_alloc_failed: } int grant_table_init(struct domain *d, int max_grant_frames, - int max_maptrack_frames) + int max_maptrack_frames, unsigned int options) { struct grant_table *gt; + unsigned int max_grant_version = options & XEN_DOMCTL_GRANT_version_mask; int ret = -ENOMEM; + if ( max_grant_version == XEN_DOMCTL_GRANT_version_default ) + max_grant_version = opt_gnttab_max_version; + if ( !max_grant_version ) + { + dprintk(XENLOG_INFO, "Invalid grant table version 0 requested\n"); + return -EINVAL; + } + if ( max_grant_version > opt_gnttab_max_version ) + { + dprintk(XENLOG_INFO, + "Requested grant version (%u) greater than supported (%u)\n", + max_grant_version, opt_gnttab_max_version); + return -EINVAL; + } + /* Default to maximum value if no value was specified */ if ( max_grant_frames < 0 ) max_grant_frames = opt_max_grant_frames; @@ -1947,6 +1964,7 @@ int grant_table_init(struct domain *d, int max_grant_frames, gt->gt_version = 1; gt->max_grant_frames = max_grant_frames; gt->max_maptrack_frames = max_maptrack_frames; + gt->max_grant_version = max_grant_version; /* Install the structure early to simplify the error path. */ gt->domain = d; @@ -3076,7 +3094,7 @@ gnttab_set_version(XEN_GUEST_HANDLE_PARAM(gnttab_set_version_t) uop) goto out; res = -ENOSYS; - if ( op.version == 2 && opt_gnttab_max_version == 1 ) + if ( op.version == 2 && gt->max_grant_version == 1 ) goto out; /* Behave as before set_version was introduced. */ res = 0; diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h index 96696e3842..e2b47184a0 100644 --- a/xen/include/public/domctl.h +++ b/xen/include/public/domctl.h @@ -87,14 +87,22 @@ struct xen_domctl_createdomain { /* * 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". + * max_grant_frames, max_maptrack_frames and max_gnttab_version < 0 + * means "use the default maximum value in the hypervisor". */ uint32_t max_vcpus; uint32_t max_evtchn_port; int32_t max_grant_frames; int32_t max_maptrack_frames; +/* Grant version, use low 4 bits. */ +#define XEN_DOMCTL_GRANT_version_mask 0xf +#define XEN_DOMCTL_GRANT_version_default 0xf + +#define XEN_DOMCTLGRANT_MAX XEN_DOMCTL_GRANT_version_mask + + uint32_t grant_opts; + /* Per-vCPU buffer size in bytes. 0 to disable. */ uint32_t vmtrace_size; diff --git a/xen/include/xen/grant_table.h b/xen/include/xen/grant_table.h index 41713e2726..fe6225346f 100644 --- a/xen/include/xen/grant_table.h +++ b/xen/include/xen/grant_table.h @@ -36,7 +36,7 @@ extern unsigned int opt_max_grant_frames; /* Create/destroy per-domain grant table context. */ int grant_table_init(struct domain *d, int max_grant_frames, - int max_maptrack_frames); + int max_maptrack_frames, unsigned int options); void grant_table_destroy( struct domain *d); void grant_table_init_vcpu(struct vcpu *v); @@ -67,7 +67,8 @@ int gnttab_acquire_resource( static inline int grant_table_init(struct domain *d, int max_grant_frames, - int max_maptrack_frames) + int max_maptrack_frames, + unsigned int options) { return 0; }
Introduce a new domain create field so that toolstack can specify the maximum grant table version usable by the domain. This is plumbed into xl and settable by the user as max_grant_version. Previously this was only settable on a per host basis using the gnttab command line option. Note the version is specified using 4 bits, which leaves room to specify up to grant table version 14 (15 is used to signal default max version). Given that we only have 2 grant table versions right now, and a new version is unlikely in the near future using 4 bits seems more than enough. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- NB: the stubdom max grant version is cloned from the domain one. Not sure whether long term we might want to use different options for the stubdom and the domain. In any case the attack surface will always be max(stubdom, domain), so maybe it's just pointless to allow more fine setting. --- Changes since v1: - Introduce a grant_opts field and use the low 4 bits to specify the version. Remaining bits will be used for other purposes. --- docs/man/xl.cfg.5.pod.in | 6 ++++++ docs/man/xl.conf.5.pod.in | 7 +++++++ tools/helpers/init-xenstore-domain.c | 1 + tools/include/libxl.h | 7 +++++++ tools/libs/light/libxl_create.c | 3 +++ tools/libs/light/libxl_dm.c | 1 + tools/libs/light/libxl_types.idl | 1 + tools/ocaml/libs/xc/xenctrl.ml | 1 + tools/ocaml/libs/xc/xenctrl.mli | 1 + tools/ocaml/libs/xc/xenctrl_stubs.c | 7 ++++++- tools/xl/xl.c | 8 ++++++++ tools/xl/xl.h | 1 + tools/xl/xl_parse.c | 9 +++++++++ xen/arch/arm/domain_build.c | 2 ++ xen/arch/x86/setup.c | 1 + xen/common/domain.c | 3 ++- xen/common/grant_table.c | 22 ++++++++++++++++++++-- xen/include/public/domctl.h | 12 ++++++++++-- xen/include/xen/grant_table.h | 5 +++-- 19 files changed, 90 insertions(+), 8 deletions(-)