[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] [PATCH v3 36/38] altp2m: Allow specifying external-only use-case



From: Tamas K Lengyel <tamas.lengyel@xxxxxxxxxxxx>

Currently setting altp2mhvm=1 in the domain configuration allows access to the
altp2m interface for both in-guest and external privileged tools. This poses
a problem for use-cases where only external access should be allowed, requiring
the user to compile Xen with XSM enabled to be able to appropriately restrict
access.

In this patch we deprecate the altp2mhvm domain configuration option and
introduce the altp2m option, which allows specifying if by default the altp2m
interface should be external-only. The information is stored in
HVM_PARAM_ALTP2M which we now define with specific XEN_ALTP2M_* modes.
If external_only mode is selected, the XSM check is shifted to use XSM_DM_PRIV
type check, thus restricting access to the interface by the guest itself. Note
that we keep the default XSM policy untouched. Users of XSM who wish to enforce
external_only mode for altp2m can do so by adjusting their XSM policy directly,
as this domain config option does not override an active XSM policy.

Also, as part of this patch we adjust the hvmop handler to require
HVM_PARAM_ALTP2M to be of a type other then disabled for all ops. This has been
previously only required for get/set altp2m domain state, all other options
were gated on altp2m_enabled. Since altp2m_enabled only gets set during set
altp2m domain state, this change introduces no new requirements to the other
ops but makes it more clear that it is required for all ops.

Signed-off-by: Tamas K Lengyel <tamas.lengyel@xxxxxxxxxxxx>
Signed-off-by: Sergej Proskurin <proskurin@xxxxxxxxxxxxx>
---
Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
Cc: Jan Beulich <jbeulich@xxxxxxxx>
Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Cc: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>

v2: Rename HVMALTP2M_* to XEN_ALTP2M_*
    Relax xsm check to XSM_DM_PRIV for external-only mode

v3: Introduce macro LIBXL_HAVE_ARM_ALTP2M in parallel to the former
    LIBXL_HAVE_ALTP2M to differentiate between altp2m for x86 and and
    altp2m for ARM architectures.

    Document the option "altp2m" in ./docs/man/xl.cfg.pod.5.in.

    Maintain the legacy info->u.hvm.altp2m field for x86 HVM domains in
    parallel to the introduced info->altp2m field for x86 HVM and ARM
    domains.
---
 docs/man/xl.cfg.pod.5.in        | 37 ++++++++++++++++++++++++++++++++++++-
 tools/libxl/libxl.h             | 10 +++++++++-
 tools/libxl/libxl_create.c      |  7 +++++--
 tools/libxl/libxl_dom.c         | 30 ++++++++++++++++++++++++++++--
 tools/libxl/libxl_types.idl     | 13 +++++++++++++
 tools/libxl/xl_cmdimpl.c        | 25 ++++++++++++++++++++++++-
 xen/arch/arm/hvm.c              | 14 +++++++++++++-
 xen/arch/x86/hvm/hvm.c          | 20 ++++++++++----------
 xen/include/public/hvm/params.h | 10 +++++++++-
 xen/include/xsm/dummy.h         | 14 +++++++++++---
 xen/include/xsm/xsm.h           |  6 +++---
 xen/xsm/flask/hooks.c           |  2 +-
 12 files changed, 162 insertions(+), 26 deletions(-)

diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in
index 48c9c0d..bf9a48a 100644
--- a/docs/man/xl.cfg.pod.5.in
+++ b/docs/man/xl.cfg.pod.5.in
@@ -1268,6 +1268,37 @@ enabled by default and you should usually omit it. It 
may be necessary
 to disable the HPET in order to improve compatibility with guest
 Operating Systems (X86 only)
 
+=item B<altp2m=MODE>
+
+Specifies access mode to the alternate-p2m capability. Alternate-p2m allows a
+guest to manage multiple p2m guest physical "memory views" (as opposed to a
+single p2m). This option is disabled by default and is available to x86 hvm and
+ARM domains. You may want this option if you want to access-control/isolate
+access to specific guest physical memory pages accessed by the guest, e.g. for
+domain memory introspection or for isolation/access-control of memory between
+components within a single guest domain.
+
+The valid values are as follows:
+
+=over 4
+
+=item B<"disabled">
+
+Altp2m is disabled for the domain (default).
+
+=item B<"mixed">
+
+The mixed mode allows access to the altp2m interface for both in-guest
+and external tools as well.
+
+=item B<"external_only">
+
+Enables access to the alternate-p2m capability for hvm guests only
+by external privileged tools. Note: if XSM is enabled then the XSM policy
+should be used to specify external-only access to the interface.
+
+=back
+
 =item B<altp2mhvm=BOOLEAN>
 
 Enables or disables hvm guest access to alternate-p2m capability.
@@ -1278,7 +1309,11 @@ You may want this option if you want to 
access-control/isolate
 access to specific guest physical memory pages accessed by
 the guest, e.g. for HVM domain memory introspection or
 for isolation/access-control of memory between components within
-a single guest hvm domain.
+a single guest hvm domain. This option is deprecated, use the option
+"altp2m" instead.
+
+Note: While the option "altp2mhvm" is deprecated, legacy applications for
+x86 systems will continue to work using it.
 
 =item B<nestedhvm=BOOLEAN>
 
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index ae21302..f78d63e 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -847,11 +847,19 @@ typedef struct libxl__ctx libxl_ctx;
 
 /*
  * LIBXL_HAVE_ALTP2M
- * If this is defined, then libxl supports alternate p2m functionality.
+ * If this is defined, then libxl supports alternate p2m functionality for
+ * x86 HVM guests.
  */
 #define LIBXL_HAVE_ALTP2M 1
 
 /*
+ * LIBXL_HAVE_ARM_ALTP2M
+ * If this is defined, then libxl supports alternate p2m functionality for
+ * ARM guests.
+ */
+#define LIBXL_HAVE_ARM_ALTP2M 1
+
+/*
  * LIBXL_HAVE_REMUS
  * If this is defined, then libxl supports remus.
  */
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 04f8ae9..21e32c9 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -406,6 +406,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
             libxl_domain_type_to_string(b_info->type));
         return ERROR_INVAL;
     }
+
     return 0;
 }
 
@@ -918,8 +919,10 @@ static void initiate_domain_create(libxl__egc *egc,
 
     if (d_config->c_info.type == LIBXL_DOMAIN_TYPE_HVM &&
         (libxl_defbool_val(d_config->b_info.u.hvm.nested_hvm) &&
-         libxl_defbool_val(d_config->b_info.u.hvm.altp2m))) {
-        LOG(ERROR, "nestedhvm and altp2mhvm cannot be used together");
+        /* Cannot combine nested_hvm with altp2m or altp2mhvm params. */
+        (libxl_defbool_val(d_config->b_info.u.hvm.altp2m) ||
+        (d_config->b_info.altp2m != LIBXL_ALTP2M_MODE_DISABLED)))) {
+        LOG(ERROR, "nestedhvm and altp2m cannot be used together");
         goto error_out;
     }
 
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index eef5045..32af757 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -291,8 +291,6 @@ static void hvm_set_conf_params(xc_interface *handle, 
uint32_t domid,
                     libxl_defbool_val(info->u.hvm.vpt_align));
     xc_hvm_param_set(handle, domid, HVM_PARAM_NESTEDHVM,
                     libxl_defbool_val(info->u.hvm.nested_hvm));
-    xc_hvm_param_set(handle, domid, HVM_PARAM_ALTP2M,
-                    libxl_defbool_val(info->u.hvm.altp2m));
 }
 
 int libxl__build_pre(libxl__gc *gc, uint32_t domid,
@@ -301,6 +299,7 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
     libxl_domain_build_info *const info = &d_config->b_info;
     libxl_ctx *ctx = libxl__gc_owner(gc);
     char *xs_domid, *con_domid;
+    bool altp2m_support = false;
     int rc;
 
     if (xc_domain_max_vcpus(ctx->xch, domid, info->max_vcpus) != 0) {
@@ -434,6 +433,33 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
 #endif
     }
 
+#if defined(__i386__) || defined(__x86_64__)
+    /* Alternate p2m support on x86 is available only for HVM guests. */
+    if (info->type == LIBXL_DOMAIN_TYPE_HVM)
+        altp2m_support = true;
+#elif defined(__arm__) || defined(__aarch64__)
+    /* Alternate p2m support on ARM is available for all guests. */
+    altp2m_support = true;
+#endif
+
+    if (altp2m_support) {
+        /* The config parameter "altp2m" replaces the parameter "altp2mhvm". 
For
+         * legacy reasons, both parameters are accepted on x86 HVM guests (only
+         * "altp2m" is accepted on ARM guests).
+         *
+         * If the legacy field info->u.hvm.altp2m is set, activate altp2m.
+         * Otherwise set altp2m based on the field info->altp2m. */
+#if defined(__i386__) || defined(__x86_64__)
+        if (info->altp2m == LIBXL_ALTP2M_MODE_DISABLED &&
+            libxl_defbool_val(info->u.hvm.altp2m))
+            xc_hvm_param_set(ctx->xch, domid, HVM_PARAM_ALTP2M,
+                             libxl_defbool_val(info->u.hvm.altp2m));
+        else
+#endif
+            xc_hvm_param_set(ctx->xch, domid, HVM_PARAM_ALTP2M,
+                             info->altp2m);
+    }
+
     rc = libxl__arch_domain_create(gc, d_config, domid);
 
     return rc;
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index ef614be..878d5ae 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -439,6 +439,13 @@ libxl_rdm_reserve = Struct("rdm_reserve", [
     ("policy",      libxl_rdm_reserve_policy),
     ])
 
+# Consistent with the values defined for HVM_PARAM_ALTP2M
+libxl_altp2m_mode = Enumeration("altp2m_mode", [
+    (0, "disabled"),
+    (1, "mixed"),
+    (2, "external_only"),
+    ], init_val = "LIBXL_ALTP2M_MODE_DISABLED")
+
 libxl_domain_build_info = Struct("domain_build_info",[
     ("max_vcpus",       integer),
     ("avail_vcpus",     libxl_bitmap),
@@ -512,6 +519,9 @@ libxl_domain_build_info = Struct("domain_build_info",[
                                        ("mmio_hole_memkb",  MemKB),
                                        ("timer_mode",       libxl_timer_mode),
                                        ("nested_hvm",       libxl_defbool),
+                                       # The u.hvm.altp2m field is used solely
+                                       # for x86 HVM guests and is maintained
+                                       # for legacy purposes.
                                        ("altp2m",           libxl_defbool),
                                        ("smbios_firmware",  string),
                                        ("acpi_firmware",    string),
@@ -561,6 +571,9 @@ libxl_domain_build_info = Struct("domain_build_info",[
 
     ("arch_arm", Struct(None, [("gic_version", libxl_gic_version),
                               ])),
+    # Alternate p2m is not bound to any architecture or guest type, as it is
+    # supported by x86 HVM and ARM domains.
+    ("altp2m", libxl_altp2m_mode),
 
     ], dir=DIR_IN
 )
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 7f961e3..15f4ab6 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1667,7 +1667,14 @@ static void parse_config_data(const char *config_source,
 
         xlu_cfg_get_defbool(config, "nestedhvm", &b_info->u.hvm.nested_hvm, 0);
 
-        xlu_cfg_get_defbool(config, "altp2mhvm", &b_info->u.hvm.altp2m, 0);
+        /*
+         * The config parameter "altp2mhvm" is considered deprecated, however
+         * further considered because of legacy reasons. The config parameter
+         * "altp2m" shall be used instead.
+         */
+        if (!xlu_cfg_get_defbool(config, "altp2mhvm", &b_info->u.hvm.altp2m, 
0))
+            fprintf(stderr, "WARNING: Specifying \"altp2mhvm\" is deprecated. "
+                    "Please use \"altp2m\" instead.\n");
 
         xlu_cfg_replace_string(config, "smbios_firmware",
                                &b_info->u.hvm.smbios_firmware, 0);
@@ -1727,6 +1734,22 @@ static void parse_config_data(const char *config_source,
         abort();
     }
 
+    if (!xlu_cfg_get_long(config, "altp2m", &l, 0)) {
+        if (l < LIBXL_ALTP2M_MODE_DISABLED ||
+            l > LIBXL_ALTP2M_MODE_EXTERNAL_ONLY) {
+            fprintf(stderr, "ERROR: invalid value %ld for \"altp2m\"\n", l);
+            exit (1);
+        }
+
+        b_info->altp2m = l;
+    } else if (!xlu_cfg_get_string(config, "altp2m", &buf, 0)) {
+        if (libxl_altp2m_mode_from_string(buf, &b_info->altp2m)) {
+            fprintf(stderr, "ERROR: invalid value \"%s\" for \"altp2m\"\n",
+                    buf);
+            exit (1);
+        }
+    }
+
     if (!xlu_cfg_get_list(config, "ioports", &ioports, &num_ioports, 0)) {
         b_info->num_ioports = num_ioports;
         b_info->ioports = calloc(num_ioports, sizeof(*b_info->ioports));
diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c
index c754ad1..4018e9a 100644
--- a/xen/arch/arm/hvm.c
+++ b/xen/arch/arm/hvm.c
@@ -72,7 +72,8 @@ static int do_altp2m_op(XEN_GUEST_HANDLE_PARAM(void) arg)
         goto out;
     }
 
-    if ( (rc = xsm_hvm_altp2mhvm_op(XSM_TARGET, d)) )
+    if ( (rc = xsm_hvm_altp2mhvm_op(XSM_OTHER, d,
+                d->arch.hvm_domain.params[HVM_PARAM_ALTP2M])) )
         goto out;
 
     switch ( a.cmd )
@@ -232,6 +233,17 @@ long do_hvm_op(unsigned long op, 
XEN_GUEST_HANDLE_PARAM(void) arg)
             if ( rc )
                 break;
 
+            switch ( a.index )
+            {
+            case HVM_PARAM_ALTP2M:
+                rc = xsm_hvm_param_altp2mhvm(XSM_PRIV, d);
+                if ( rc )
+                    break;
+                if ( a.value > XEN_ALTP2M_external_only )
+                    rc = -EINVAL;
+                break;
+            }
+
             d->arch.hvm_domain.params[a.index] = a.value;
             break;
 
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 0180f26..ce48f8a 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -5042,7 +5042,7 @@ static int hvmop_set_param(
         rc = xsm_hvm_param_altp2mhvm(XSM_PRIV, d);
         if ( rc )
             break;
-        if ( a.value > 1 )
+        if ( a.value > XEN_ALTP2M_external_only )
             rc = -EINVAL;
         if ( a.value &&
              d->arch.hvm_domain.params[HVM_PARAM_NESTEDHVM] )
@@ -5238,18 +5238,19 @@ static int do_altp2m_op(
         goto out;
     }
 
-    if ( (rc = xsm_hvm_altp2mhvm_op(XSM_TARGET, d)) )
+    if ( !d->arch.hvm_domain.params[HVM_PARAM_ALTP2M] )
+    {
+        rc = -EINVAL;
+        goto out;
+    }
+
+    if ( (rc = xsm_hvm_altp2mhvm_op(XSM_OTHER, d,
+                d->arch.hvm_domain.params[HVM_PARAM_ALTP2M])) )
         goto out;
 
     switch ( a.cmd )
     {
     case HVMOP_altp2m_get_domain_state:
-        if ( !d->arch.hvm_domain.params[HVM_PARAM_ALTP2M] )
-        {
-            rc = -EINVAL;
-            break;
-        }
-
         a.u.domain_state.state = altp2m_active(d);
         rc = __copy_to_guest(arg, &a, 1) ? -EFAULT : 0;
         break;
@@ -5259,8 +5260,7 @@ static int do_altp2m_op(
         struct vcpu *v;
         bool_t ostate;
 
-        if ( !d->arch.hvm_domain.params[HVM_PARAM_ALTP2M] ||
-             nestedhvm_enabled(d) )
+        if ( nestedhvm_enabled(d) )
         {
             rc = -EINVAL;
             break;
diff --git a/xen/include/public/hvm/params.h b/xen/include/public/hvm/params.h
index f7338a3..3ffc33b 100644
--- a/xen/include/public/hvm/params.h
+++ b/xen/include/public/hvm/params.h
@@ -225,8 +225,16 @@
 /* Location of the VM Generation ID in guest physical address space. */
 #define HVM_PARAM_VM_GENERATION_ID_ADDR 34
 
-/* Boolean: Enable altp2m */
+/*
+ * Set mode for altp2m:
+ *  disabled: don't activate altp2m (default)
+ *  mixed: allow access to altp2m for both in-guest and external tools
+ *  external_only: allow access to external privileged tools only
+ */
 #define HVM_PARAM_ALTP2M       35
+#define XEN_ALTP2M_disabled      0
+#define XEN_ALTP2M_mixed         1
+#define XEN_ALTP2M_external_only 2
 
 /*
  * Size of the x87 FPU FIP/FDP registers that the hypervisor needs to
diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index 406cd18..a0f6b70 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -554,10 +554,18 @@ static XSM_INLINE int 
xsm_hvm_param_altp2mhvm(XSM_DEFAULT_ARG struct domain *d)
     return xsm_default_action(action, current->domain, d);
 }
 
-static XSM_INLINE int xsm_hvm_altp2mhvm_op(XSM_DEFAULT_ARG struct domain *d)
+static XSM_INLINE int xsm_hvm_altp2mhvm_op(XSM_DEFAULT_ARG struct domain *d, 
int mode)
 {
-    XSM_ASSERT_ACTION(XSM_TARGET);
-    return xsm_default_action(action, current->domain, d);
+    XSM_ASSERT_ACTION(XSM_OTHER);
+    switch ( mode )
+    {
+    case XEN_ALTP2M_mixed:
+        return xsm_default_action(XSM_TARGET, current->domain, d);
+    case XEN_ALTP2M_external_only:
+        return xsm_default_action(XSM_DM_PRIV, current->domain, d);
+    default:
+        return -EPERM;
+    };
 }
 
 static XSM_INLINE int xsm_vm_event_control(XSM_DEFAULT_ARG struct domain *d, 
int mode, int op)
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index 5dc59dd..4f53b27 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -140,7 +140,7 @@ struct xsm_operations {
     int (*hvm_control) (struct domain *d, unsigned long op);
     int (*hvm_param_nested) (struct domain *d);
     int (*hvm_param_altp2mhvm) (struct domain *d);
-    int (*hvm_altp2mhvm_op) (struct domain *d);
+    int (*hvm_altp2mhvm_op) (struct domain *d, int mode);
     int (*get_vnumainfo) (struct domain *d);
 
     int (*vm_event_control) (struct domain *d, int mode, int op);
@@ -583,9 +583,9 @@ static inline int xsm_hvm_param_altp2mhvm (xsm_default_t 
def, struct domain *d)
     return xsm_ops->hvm_param_altp2mhvm(d);
 }
 
-static inline int xsm_hvm_altp2mhvm_op (xsm_default_t def, struct domain *d)
+static inline int xsm_hvm_altp2mhvm_op (xsm_default_t def, struct domain *d, 
int mode)
 {
-    return xsm_ops->hvm_altp2mhvm_op(d);
+    return xsm_ops->hvm_altp2mhvm_op(d, mode);
 }
 
 static inline int xsm_get_vnumainfo (xsm_default_t def, struct domain *d)
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index ec6f5b4..b97396e 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -1197,7 +1197,7 @@ static int flask_hvm_param_altp2mhvm(struct domain *d)
     return current_has_perm(d, SECCLASS_HVM, HVM__ALTP2MHVM);
 }
 
-static int flask_hvm_altp2mhvm_op(struct domain *d)
+static int flask_hvm_altp2mhvm_op(struct domain *d, int mode)
 {
     return current_has_perm(d, SECCLASS_HVM, HVM__ALTP2MHVM_OP);
 }
-- 
2.9.0


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.