diff mbox series

[2/6] gnttab: allow per-domain control over transitive grants

Message ID 20210917154625.89315-3-roger.pau@citrix.com (mailing list archive)
State Superseded
Headers show
Series gnttab: add per-domain controls | expand

Commit Message

Roger Pau Monné Sept. 17, 2021, 3:46 p.m. UTC
Introduce a new grant options flags field in domain create and use it
to signal whether transitive grants are allowed on the domain. This is
settable from xl using the transitive_grants option.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
 docs/man/xl.cfg.5.pod.in            |  6 ++++++
 docs/man/xl.conf.5.pod.in           |  7 +++++++
 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      |  4 ++++
 tools/ocaml/libs/xc/xenctrl.mli     |  4 ++++
 tools/ocaml/libs/xc/xenctrl_stubs.c |  9 ++++++++-
 tools/xl/xl.c                       |  7 +++++++
 tools/xl/xl.h                       |  1 +
 tools/xl/xl_parse.c                 |  4 ++++
 xen/arch/arm/domain_build.c         |  2 ++
 xen/arch/x86/setup.c                |  1 +
 xen/common/domain.c                 |  3 ++-
 xen/common/grant_table.c            | 11 +++++++++--
 xen/include/public/domctl.h         |  8 ++++++++
 xen/include/xen/grant_table.h       |  6 ++++--
 18 files changed, 79 insertions(+), 6 deletions(-)

Comments

Andrew Cooper Sept. 20, 2021, 9:32 a.m. UTC | #1
On 17/09/2021 16:46, Roger Pau Monne wrote:
> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> index 7f8456c50e..fe2201fca1 100644
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -96,6 +96,14 @@ struct xen_domctl_createdomain {
>      int32_t max_maptrack_frames;
>      int32_t max_grant_version;
>  
> +/* Allow transitive grants. */
> +#define _XEN_DOMCTL_GRANT_transitive  0
> +#define XEN_DOMCTL_GRANT_transitive   (1U << _XEN_DOMCTL_GRANT_transitive)

There's no need for bit position variables.

> +
> +#define XEN_DOMCTL_GRANT_MAX XEN_DOMCTL_GRANT_transitive
> +
> +    uint32_t grant_opts;

So far, we've got 3 bits of information, v1, v2 and transitive, and
we're tight on space in the structure with loads more to fit in.

I was thinking grant_flags or equiv to contain these 3 settings, and any
further which might appear.


One thing which is missing however is the enumeration of which settings
are available, and rejection of bad settings.  If v2 is disabled
globally, trying to create a VM with v2 needs to fail.

~Andrew
Roger Pau Monné Sept. 20, 2021, 11:45 a.m. UTC | #2
On Mon, Sep 20, 2021 at 10:32:24AM +0100, Andrew Cooper wrote:
> On 17/09/2021 16:46, Roger Pau Monne wrote:
> > diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> > index 7f8456c50e..fe2201fca1 100644
> > --- a/xen/include/public/domctl.h
> > +++ b/xen/include/public/domctl.h
> > @@ -96,6 +96,14 @@ struct xen_domctl_createdomain {
> >      int32_t max_maptrack_frames;
> >      int32_t max_grant_version;
> >  
> > +/* Allow transitive grants. */
> > +#define _XEN_DOMCTL_GRANT_transitive  0
> > +#define XEN_DOMCTL_GRANT_transitive   (1U << _XEN_DOMCTL_GRANT_transitive)
> 
> There's no need for bit position variables.
> 
> > +
> > +#define XEN_DOMCTL_GRANT_MAX XEN_DOMCTL_GRANT_transitive
> > +
> > +    uint32_t grant_opts;
> 
> So far, we've got 3 bits of information, v1, v2 and transitive, and
> we're tight on space in the structure with loads more to fit in.
> 
> I was thinking grant_flags or equiv to contain these 3 settings, and any
> further which might appear.

What about using something like the below?

We also need to consider selecting the default version (whatever is
set on the hypervisor) and no grant table at all.

/* Grant version, use low 4 bits. */
#define XEN_DOMCTL_GRANT_disable         0
#define XEN_DOMCTL_GRANT_version_v1      1
#define XEN_DOMCTL_GRANT_version_v2      2
#define XEN_DOMCTL_GRANT_version_default 0xf
/* Allow transitive grants. */
#define _XEN_DOMCTL_GRANT_transitive  4
#define XEN_DOMCTL_GRANT_transitive   (1U << _XEN_DOMCTL_GRANT_transitive)

#define XEN_DOMCTLGRANT_MAX XEN_DOMCTL_GRANT_transitive

    uint32_t grant_opts;

> 
> 
> One thing which is missing however is the enumeration of which settings
> are available, and rejection of bad settings.  If v2 is disabled
> globally, trying to create a VM with v2 needs to fail.

Right, I think this is already the case with the current
implementation. This doesn't happen however with the transitive
option, as I implemented it and'ing the hypervisor selection to the
tools provided one, partially due to the lack of a 'use hypervisor
default' option.

Thanks, Roger.
diff mbox series

Patch

diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
index a4bf2caafa..c5a447dfcd 100644
--- a/docs/man/xl.cfg.5.pod.in
+++ b/docs/man/xl.cfg.5.pod.in
@@ -586,6 +586,12 @@  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<transitive_grants=BOOLEAN>
+
+Specify whether transitive grants should be available to the domain. Note such
+functionality only applies to grant table version 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 0a70698a7c..88efbee5fe 100644
--- a/docs/man/xl.conf.5.pod.in
+++ b/docs/man/xl.conf.5.pod.in
@@ -108,6 +108,13 @@  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<transitive_grants=BOOLEAN>
+
+Sets the default value for the C<transitive_grants> 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/include/libxl.h b/tools/include/libxl.h
index 7a35833312..d23586f2cc 100644
--- a/tools/include/libxl.h
+++ b/tools/include/libxl.h
@@ -509,6 +509,13 @@ 
  */
 #define LIBXL_HAVE_MAX_GRANT_VERSION 1
 
+/*
+ * LIBXL_HAVE_TRANSITIVE_GRANTS indicates libxl_domain_build_info has a
+ * transitive_grants field for setting whether such functionality should be
+ * available to the domain.
+ */
+#define LIBXL_HAVE_TRANSITIVE_GRANTS 1
+
 /*
  * libxl ABI compatibility
  *
diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
index 69b5419416..61d65d1342 100644
--- a/tools/libs/light/libxl_create.c
+++ b/tools/libs/light/libxl_create.c
@@ -633,6 +633,9 @@  int libxl__domain_make(libxl__gc *gc, libxl_domain_config *d_config,
         if (info->passthrough == LIBXL_PASSTHROUGH_SYNC_PT)
             create.iommu_opts |= XEN_DOMCTL_IOMMU_no_sharept;
 
+        if (libxl_defbool_val(b_info->transitive_grants))
+            create.grant_opts |= XEN_DOMCTL_GRANT_transitive;
+
         /* Ultimately, handle is an array of 16 uint8_t, same as uuid */
         libxl_uuid_copy(ctx, (libxl_uuid *)&create.handle, &info->uuid);
 
diff --git a/tools/libs/light/libxl_dm.c b/tools/libs/light/libxl_dm.c
index 9a8ddbe188..4ade437257 100644
--- a/tools/libs/light/libxl_dm.c
+++ b/tools/libs/light/libxl_dm.c
@@ -2321,6 +2321,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.transitive_grants = guest_config->b_info.transitive_grants;
 
     dm_config->b_info.u.pv.features = "";
 
diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
index 37789a568c..d05b5d2abc 100644
--- a/tools/libs/light/libxl_types.idl
+++ b/tools/libs/light/libxl_types.idl
@@ -519,6 +519,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'}),
+    ("transitive_grants",   libxl_defbool),
     
     ("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 6a8658bfec..ec7a296776 100644
--- a/tools/ocaml/libs/xc/xenctrl.ml
+++ b/tools/ocaml/libs/xc/xenctrl.ml
@@ -73,6 +73,9 @@  type domain_create_flag =
 type domain_create_iommu_opts =
 	| IOMMU_NO_SHAREPT
 
+type domain_create_grant_opts =
+	| GRANT_TRANSITIVE
+
 type domctl_create_config =
 {
 	ssidref: int32;
@@ -84,6 +87,7 @@  type domctl_create_config =
 	max_grant_frames: int;
 	max_maptrack_frames: int;
 	max_grant_version: int;
+	grant_opts: domain_create_grant_opts list;
 	arch: arch_domainconfig;
 }
 
diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
index 5933d32c70..e47fd1947f 100644
--- a/tools/ocaml/libs/xc/xenctrl.mli
+++ b/tools/ocaml/libs/xc/xenctrl.mli
@@ -66,6 +66,9 @@  type domain_create_flag =
 type domain_create_iommu_opts =
   | IOMMU_NO_SHAREPT
 
+type domain_create_grant_opts =
+  | GRANT_TRANSITIVE
+
 type domctl_create_config = {
   ssidref: int32;
   handle: string;
@@ -76,6 +79,7 @@  type domctl_create_config = {
   max_grant_frames: int;
   max_maptrack_frames: int;
   max_grant_version: int;
+  grant_opts: domain_create_grant_opts list;
   arch: arch_domainconfig;
 }
 
diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c b/tools/ocaml/libs/xc/xenctrl_stubs.c
index aad3c6a726..772704759d 100644
--- a/tools/ocaml/libs/xc/xenctrl_stubs.c
+++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
@@ -189,7 +189,8 @@  CAMLprim value stub_xc_domain_create(value xch, value wanted_domid, value config
 #define VAL_MAX_GRANT_FRAMES    Field(config, 6)
 #define VAL_MAX_MAPTRACK_FRAMES Field(config, 7)
 #define VAL_MAX_GRANT_VERSION   Field(config, 8)
-#define VAL_ARCH                Field(config, 9)
+#define VAL_GRANT_OPTS          Field(config, 9)
+#define VAL_ARCH                Field(config, 10)
 
 	uint32_t domid = Int_val(wanted_domid);
 	int result;
@@ -214,6 +215,11 @@  CAMLprim value stub_xc_domain_create(value xch, value wanted_domid, value config
 		/* ! XEN_DOMCTL_IOMMU_ XEN_DOMCTL_IOMMU_MAX max */
 		(VAL_IOMMU_OPTS);
 
+	cfg.grant_opts = ocaml_list_to_c_bitmap
+		/* ! domain_create_grant_opts GRANT_ lc */
+		/* ! XEN_DOMCTL_GRANT_ XEN_DOMCTL_GRANT_MAX max */
+		(VAL_GRANT_OPTS);
+
 	arch_domconfig = Field(VAL_ARCH, 0);
 	switch ( Tag_val(VAL_ARCH) )
 	{
@@ -253,6 +259,7 @@  CAMLprim value stub_xc_domain_create(value xch, value wanted_domid, value config
 	}
 
 #undef VAL_ARCH
+#undef VAL_GRANT_OPTS
 #undef VAL_MAX_GRANT_VERSION
 #undef VAL_MAX_MAPTRACK_FRAMES
 #undef VAL_MAX_GRANT_FRAMES
diff --git a/tools/xl/xl.c b/tools/xl/xl.c
index 0fde879cf4..9bd398f8c9 100644
--- a/tools/xl/xl.c
+++ b/tools/xl/xl.c
@@ -56,6 +56,7 @@  bool timestamps = 0;
 int max_grant_frames = -1;
 int max_maptrack_frames = -1;
 int max_grant_version = -1;
+bool transitive_grants = true;
 libxl_domid domid_policy = INVALID_DOMID;
 
 xentoollog_level minmsglevel = minmsglevel_default;
@@ -221,6 +222,12 @@  static void parse_global_config(const char *configfile,
     else if (e != ESRCH)
         exit(1);
 
+    e = xlu_cfg_get_long (config, "transitive_grants", &l, 0);
+    if (!e)
+        transitive_grants = 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 cf12c79a9b..d7f83c9abd 100644
--- a/tools/xl/xl.h
+++ b/tools/xl/xl.h
@@ -283,6 +283,7 @@  extern char *blkdev_start;
 extern int max_grant_frames;
 extern int max_maptrack_frames;
 extern int max_grant_version;
+extern bool transitive_grants;
 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 1206d7ea51..8e4e561df8 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -1440,6 +1440,10 @@  void parse_config_data(const char *config_source,
     else
         exit(1);
 
+    xlu_cfg_get_defbool(config, "transitive_grants", &b_info->transitive_grants,
+                        0);
+    libxl_defbool_setdefault(&b_info->transitive_grants, transitive_grants);
+
     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 e9a34f2f8d..b4763b5ec6 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2485,6 +2485,7 @@  void __init create_domUs(void)
             .max_grant_frames = -1,
             .max_maptrack_frames = -1,
             .max_grant_version = -1,
+            .grant_opts = XEN_DOMCTL_GRANT_transitive,
         };
 
         if ( !dt_device_is_compatible(node, "xen,domain") )
@@ -2593,6 +2594,7 @@  void __init create_dom0(void)
         .max_grant_frames = gnttab_dom0_frames(),
         .max_maptrack_frames = -1,
         .max_grant_version = -1,
+        .grant_opts = XEN_DOMCTL_GRANT_transitive,
     };
 
     /* 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 af69e20029..c743a88592 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -751,6 +751,7 @@  static struct domain *__init create_dom0(const module_t *image,
         .max_grant_frames = -1,
         .max_maptrack_frames = -1,
         .max_grant_version = -1,
+        .grant_opts = XEN_DOMCTL_GRANT_transitive,
         .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 0c85d89419..ab16c422f7 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -670,7 +670,8 @@  struct domain *domain_create(domid_t domid,
 
         if ( (err = grant_table_init(d, config->max_grant_frames,
                                      config->max_maptrack_frames,
-                                     config->max_grant_version)) != 0 )
+                                     config->max_grant_version,
+                                     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 d01c6813d1..280dbc850a 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -74,6 +74,10 @@  struct grant_table {
      * progress.
      */
     unsigned int          maptrack_limit;
+
+    /* Allow transitive grants. Only applies to grant v2. */
+    bool                  allow_transitive:1;
+
     /* Shared grant table (see include/public/grant_table.h). */
     union {
         void **shared_raw;
@@ -1918,7 +1922,8 @@  active_alloc_failed:
 }
 
 int grant_table_init(struct domain *d, int max_grant_frames,
-                     int max_maptrack_frames, int max_grant_version)
+                     int max_maptrack_frames, int max_grant_version,
+                     unsigned int options)
 {
     struct grant_table *gt;
     int ret = -ENOMEM;
@@ -1964,6 +1969,8 @@  int grant_table_init(struct domain *d, int max_grant_frames,
     gt->max_grant_frames = max_grant_frames;
     gt->max_maptrack_frames = max_maptrack_frames;
     gt->max_grant_version = max_grant_version;
+    gt->allow_transitive = !!(options & XEN_DOMCTL_GRANT_transitive) &&
+                           opt_transitive_grants;
 
     /* Install the structure early to simplify the error path. */
     gt->domain = d;
@@ -2886,7 +2893,7 @@  static int gnttab_copy_claim_buf(const struct gnttab_copy *op,
                                     buf->read_only,
                                     &buf->mfn, &buf->page,
                                     &buf->ptr.offset, &buf->len,
-                                    opt_transitive_grants);
+                                    buf->domain->grant_table->allow_transitive);
         if ( rc != GNTST_okay )
             goto out;
         buf->ptr.u.ref = ptr->u.ref;
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 7f8456c50e..fe2201fca1 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -96,6 +96,14 @@  struct xen_domctl_createdomain {
     int32_t max_maptrack_frames;
     int32_t max_grant_version;
 
+/* Allow transitive grants. */
+#define _XEN_DOMCTL_GRANT_transitive  0
+#define XEN_DOMCTL_GRANT_transitive   (1U << _XEN_DOMCTL_GRANT_transitive)
+
+#define XEN_DOMCTL_GRANT_MAX XEN_DOMCTL_GRANT_transitive
+
+    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 d6da067fc1..f264b1c3fc 100644
--- a/xen/include/xen/grant_table.h
+++ b/xen/include/xen/grant_table.h
@@ -36,7 +36,8 @@  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_grant_version);
+                     int max_maptrack_frames, int max_grant_version,
+                     unsigned int options);
 void grant_table_destroy(
     struct domain *d);
 void grant_table_init_vcpu(struct vcpu *v);
@@ -68,7 +69,8 @@  int gnttab_acquire_resource(
 static inline int grant_table_init(struct domain *d,
                                    int max_grant_frames,
                                    int max_maptrack_frames,
-                                   int max_grant_version)
+                                   int max_grant_version,
+                                   unsigned int options)
 {
     return 0;
 }