[PATCH v4 0/4] Support for reading runtime hypervisor parameters

[PATCH v4 0/4] Support for reading runtime hypervisor parameters

[Vasilis Liaskovitis]

Currently runtime parameters of the hypervisor cannot be inspected through an
xl command, however they can be changed with the "xl set-parameter" command.
Being able to check these parameters at runtime would be a useful diagnostic
tool.

This patch series implements a new xl command "xl get-parameters"
which takes a string of input parameters and returns their current
values in the hypervisor settings.

Examples:

xl get-parameters "gnttab_max_frames gnttab_max_maptrack_frames"
gnttab_max_frames gnttab_max_maptrack_frames : 64 1024

xl set-parameters gnttab_max_frames=128

xl get-parameters gnttab_max_frames
gnttab_max_frames : 128

xl get-parameters "gnttab_max_frames gnttab_max_maptrack_frames"
gnttab_max_frames gnttab_max_maptrack_frames : 128 1024


Changes v3->v4:

- Fix return value and logging in new libxl function.

Changes v2->v3:

- Several style / formatting fixes
- Limitations for signed integer parameters added in code and commit log.

Changes v1->v2:

- fixed snprintf issues, fixed memory leaks and error handling
- removed unnecessary wrapper function
- OPT_SIZE is handled

Limitations:

- Custom runtime parameters (OPT_CUSTOM) are not supported yet. I'd like
  to do this in a follow-up series. See also discussion at
  https://lists.xenproject.org/archives/html/xen-devel/2019-03/msg01383.html
- For integer parameters (OPT_UINT), only unsigned parameters are printed
  correctly at the moment.

Vasilis Liaskovitis (4):
  xen: add hypercall for getting parameters at runtime
  libxc: add function to get hypervisor parameters
  libxl: add libxl_get_parameters() function
  xl: add new xl command get-parameters

 docs/man/xl.1.pod.in                |   5 ++
 tools/flask/policy/modules/dom0.te  |   2 +-
 tools/libxc/include/xenctrl.h       |   1 +
 tools/libxc/xc_misc.c               |  26 ++++++
 tools/libxl/libxl.c                 |  19 +++++
 tools/libxl/libxl.h                 |   1 +
 tools/xl/xl.h                       |   1 +
 tools/xl/xl_cmdtable.c              |   5 ++
 tools/xl/xl_misc.c                  |  25 ++++++
 xen/common/kernel.c                 | 118 ++++++++++++++++++++++++++++
 xen/common/sysctl.c                 |  52 +++++++++++-
 xen/include/public/sysctl.h         |  18 +++++
 xen/include/xen/lib.h               |   1 +
 xen/xsm/flask/hooks.c               |   3 +
 xen/xsm/flask/policy/access_vectors |   2 +
 15 files changed, 276 insertions(+), 3 deletions(-)

-- 
2.20.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v4 0/4] Support for reading runtime hypervisor parameters

[Vasilis Liaskovitis]

Currently runtime parameters of the hypervisor cannot be inspected through an
xl command, however they can be changed with the "xl set-parameter" command.
Being able to check these parameters at runtime would be a useful diagnostic
tool.

This patch series implements a new xl command "xl get-parameters"
which takes a string of input parameters and returns their current
values in the hypervisor settings.

Examples:

xl get-parameters "gnttab_max_frames gnttab_max_maptrack_frames"
gnttab_max_frames gnttab_max_maptrack_frames : 64 1024

xl set-parameters gnttab_max_frames=128

xl get-parameters gnttab_max_frames
gnttab_max_frames : 128

xl get-parameters "gnttab_max_frames gnttab_max_maptrack_frames"
gnttab_max_frames gnttab_max_maptrack_frames : 128 1024


Changes v3->v4:

- Fix return value and logging in new libxl function.

Changes v2->v3:

- Several style / formatting fixes
- Limitations for signed integer parameters added in code and commit log.

Changes v1->v2:

- fixed snprintf issues, fixed memory leaks and error handling
- removed unnecessary wrapper function
- OPT_SIZE is handled

Limitations:

- Custom runtime parameters (OPT_CUSTOM) are not supported yet. I'd like
  to do this in a follow-up series. See also discussion at
  https://lists.xenproject.org/archives/html/xen-devel/2019-03/msg01383.html
- For integer parameters (OPT_UINT), only unsigned parameters are printed
  correctly at the moment.

Vasilis Liaskovitis (4):
  xen: add hypercall for getting parameters at runtime
  libxc: add function to get hypervisor parameters
  libxl: add libxl_get_parameters() function
  xl: add new xl command get-parameters

 docs/man/xl.1.pod.in                |   5 ++
 tools/flask/policy/modules/dom0.te  |   2 +-
 tools/libxc/include/xenctrl.h       |   1 +
 tools/libxc/xc_misc.c               |  26 ++++++
 tools/libxl/libxl.c                 |  19 +++++
 tools/libxl/libxl.h                 |   1 +
 tools/xl/xl.h                       |   1 +
 tools/xl/xl_cmdtable.c              |   5 ++
 tools/xl/xl_misc.c                  |  25 ++++++
 xen/common/kernel.c                 | 118 ++++++++++++++++++++++++++++
 xen/common/sysctl.c                 |  52 +++++++++++-
 xen/include/public/sysctl.h         |  18 +++++
 xen/include/xen/lib.h               |   1 +
 xen/xsm/flask/hooks.c               |   3 +
 xen/xsm/flask/policy/access_vectors |   2 +
 15 files changed, 276 insertions(+), 3 deletions(-)

-- 
2.20.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v4 1/4] xen: add hypercall for reading runtime parameters

[Vasilis Liaskovitis]

Add a sysctl hypercall to support reading hypervisor runtime parameters.

Limitations:
- Custom runtime parameters (OPT_CUSTOM) are not supported yet.
- For integer parameters (OPT_UINT), only unsigned parameters are printed
correctly.
- The implementation only reads runtime parameters, but it can be changed
to read all hypervisor parameters if needed. 

Signed-off-by: Vasilis Liaskovitis <vliaskovitis@suse.com>
---
 tools/flask/policy/modules/dom0.te  |   2 +-
 xen/common/kernel.c                 | 118 ++++++++++++++++++++++++++++
 xen/common/sysctl.c                 |  52 +++++++++++-
 xen/include/public/sysctl.h         |  18 +++++
 xen/include/xen/lib.h               |   1 +
 xen/xsm/flask/hooks.c               |   3 +
 xen/xsm/flask/policy/access_vectors |   2 +
 7 files changed, 193 insertions(+), 3 deletions(-)

diff --git a/tools/flask/policy/modules/dom0.te b/tools/flask/policy/modules/dom0.te
index a347d664f8..681d1a101b 100644
--- a/tools/flask/policy/modules/dom0.te
+++ b/tools/flask/policy/modules/dom0.te
@@ -16,7 +16,7 @@ allow dom0_t xen_t:xen {
 allow dom0_t xen_t:xen2 {
 	resource_op psr_cmt_op psr_alloc pmu_ctrl get_symbol
 	get_cpu_levelling_caps get_cpu_featureset livepatch_op
-	coverage_op set_parameter
+	coverage_op set_parameter get_parameter
 };
 
 # Allow dom0 to use all XENVER_ subops that have checks.
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index 612575430f..6695ffc372 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -12,6 +12,7 @@
 #include <xen/paging.h>
 #include <xen/guest_access.h>
 #include <xen/hypercall.h>
+#include <xen/ctype.h>
 #include <xsm/xsm.h>
 #include <asm/current.h>
 #include <public/version.h>
@@ -52,6 +53,123 @@ static int assign_integer_param(const struct kernel_param *param, uint64_t val)
     return 0;
 }
 
+static int get_integer_param(const struct kernel_param *param, uint64_t *val)
+{
+    switch ( param->len )
+    {
+    case sizeof(uint8_t):
+        *val = *(uint8_t *)param->par.var;
+        break;
+
+    case sizeof(uint16_t):
+        *val = *(uint16_t *)param->par.var;
+        break;
+
+    case sizeof(uint32_t):
+        *val = *(uint32_t *)param->par.var;
+        break;
+
+    case sizeof(uint64_t):
+        *val = *(uint64_t *)param->par.var;
+        break;
+
+    default:
+        BUG();
+        break;
+    }
+
+    return 0;
+}
+
+int runtime_get_params(const char *cmdline, char *values,
+                      size_t maxlen)
+{
+    char opt[128], *optkey, *q, *val = values;
+    const char *p = cmdline;
+    const struct kernel_param *param;
+    int rc = 0, len = 0;
+    size_t bufpos = 0;
+    uint64_t param_int;
+
+    while ( !rc )
+    {
+        /* Skip whitespace. */
+        while ( isspace(*p) )
+            p++;
+        if ( *p == '\0' )
+            break;
+
+        /* Grab the next whitespace-delimited option. */
+        q = optkey = opt;
+        while ( !isspace(*p) && (*p != '\0') )
+        {
+            if ( (q - opt) < (sizeof(opt) - 1) ) /* avoid overflow */
+                *q++ = *p;
+            else return -ENOMEM;
+            p++;
+        }
+        *q = '\0';
+
+        for ( param = __param_start; param < __param_end; param++ )
+        {
+            if ( strcmp(param->name, optkey) )
+                continue;
+
+            switch ( param->type )
+            {
+            case OPT_STR:
+                len = snprintf(val + bufpos, maxlen - bufpos, "%s ",
+                               (char*)param->par.var);
+                break;
+
+            case OPT_UINT:
+            case OPT_SIZE:
+                /* Signed integer parameters are not supported yet.
+                 * While there are no runtime signed integer parameters
+                 * at the moment, adding one and trying to get its value
+                 * with the current implementation will output the wrong
+                 * value.
+                 */
+                get_integer_param(param, &param_int);
+                len = snprintf(val + bufpos, maxlen - bufpos,
+                               "%"PRIu64" ", param_int);
+                break;
+
+            case OPT_BOOL:
+                get_integer_param(param, &param_int);
+                len = snprintf(val + bufpos, maxlen - bufpos, "%s ",
+                               param_int ? "true" : "false");
+                break;
+
+            case OPT_CUSTOM:
+                /* Custom parameters are not supported yet. */
+                rc = -EINVAL;
+                break;
+
+            default:
+                BUG();
+                break;
+            }
+
+            if ( len < 0 )
+                rc = len;
+            else if ( len < maxlen - bufpos )
+                /* if output was not truncated update buffer position. */
+                bufpos += len;
+            else if ( len > 0 )
+                rc = -ENOMEM;
+
+            break;
+        }
+
+        /* no parameter was matched */
+        if ( param >= __param_end )
+            rc = -EINVAL;
+    }
+
+    return rc;
+}
+
 static int parse_params(const char *cmdline, const struct kernel_param *start,
                         const struct kernel_param *end)
 {
diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index c0aa6bde4e..20be6a6d8d 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -466,9 +466,9 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
             copyback = 1;
         break;
 
+#define XEN_PARAMETER_MAX_SIZE 1023
     case XEN_SYSCTL_set_parameter:
     {
-#define XEN_SET_PARAMETER_MAX_SIZE 1023
         char *params;
 
         if ( op->u.set_parameter.pad[0] || op->u.set_parameter.pad[1] ||
@@ -477,7 +477,7 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
             ret = -EINVAL;
             break;
         }
-        if ( op->u.set_parameter.size > XEN_SET_PARAMETER_MAX_SIZE )
+        if ( op->u.set_parameter.size > XEN_PARAMETER_MAX_SIZE )
         {
             ret = -E2BIG;
             break;
@@ -501,6 +501,54 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
 
         break;
     }
+    case XEN_SYSCTL_get_parameter:
+    {
+        char *params, *values;
+
+        if ( op->u.get_parameter.pad[0] || op->u.get_parameter.pad[1] ||
+             op->u.get_parameter.pad[2] )
+        {
+            ret = -EINVAL;
+            break;
+        }
+        if ( op->u.get_parameter.size > XEN_PARAMETER_MAX_SIZE )
+        {
+            ret = -E2BIG;
+            break;
+        }
+        params = xmalloc_bytes(op->u.get_parameter.size + 1);
+        if ( !params )
+        {
+            ret = -ENOMEM;
+            break;
+        }
+
+        values = xmalloc_bytes(XEN_PARAMETER_MAX_SIZE);
+        if ( !values )
+        {
+            xfree(params);
+            ret = -ENOMEM;
+            break;
+        }
+
+        if ( copy_from_guest(params, op->u.get_parameter.params,
+                             op->u.get_parameter.size) )
+            ret = -EFAULT;
+        else
+        {
+            params[op->u.set_parameter.size] = 0;
+            ret = runtime_get_params(params, values, XEN_PARAMETER_MAX_SIZE);
+
+            if ( !ret && copy_to_guest(op->u.get_parameter.values, values,
+                                       strlen(values)) )
+                ret = -EFAULT;
+        }
+
+        xfree(params);
+        xfree(values);
+
+        break;
+    }
 
     default:
         ret = arch_do_sysctl(op, u_sysctl);
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index c49b4dcc99..7d77d57115 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -1100,6 +1100,22 @@ typedef struct xen_sysctl_cpu_policy xen_sysctl_cpu_policy_t;
 DEFINE_XEN_GUEST_HANDLE(xen_sysctl_cpu_policy_t);
 #endif
 
+/*
+ * XEN_SYSCTL_get_parameter
+ *
+ * Read hypervisor parameters at runtime.
+ * Parameters are a single string terminated by a NUL byte of max. size
+ * characters. Multiple settings can be specified by separating them
+ * with blanks.
+ */
+
+struct xen_sysctl_get_parameter {
+    XEN_GUEST_HANDLE_64(char) params;       /* IN: pointer to parameters. */
+    XEN_GUEST_HANDLE_64(char) values;       /* OUT: pointer to output values. */
+    uint16_t size;                          /* IN: size of parameters. */
+    uint16_t pad[3];                        /* IN: MUST be zero. */
+};
+
 struct xen_sysctl {
     uint32_t cmd;
 #define XEN_SYSCTL_readconsole                    1
@@ -1130,6 +1146,7 @@ struct xen_sysctl {
 #define XEN_SYSCTL_livepatch_op                  27
 #define XEN_SYSCTL_set_parameter                 28
 #define XEN_SYSCTL_get_cpu_policy                29
+#define XEN_SYSCTL_get_parameter                 30
     uint32_t interface_version; /* XEN_SYSCTL_INTERFACE_VERSION */
     union {
         struct xen_sysctl_readconsole       readconsole;
@@ -1162,6 +1179,7 @@ struct xen_sysctl {
 #if defined(__i386__) || defined(__x86_64__)
         struct xen_sysctl_cpu_policy        cpu_policy;
 #endif
+        struct xen_sysctl_get_parameter     get_parameter;
         uint8_t                             pad[128];
     } u;
 };
diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
index e0b7bcb6b7..6e6367bb7a 100644
--- a/xen/include/xen/lib.h
+++ b/xen/include/xen/lib.h
@@ -71,6 +71,7 @@ struct domain;
 void cmdline_parse(const char *cmdline);
 int runtime_parse(const char *line);
 int parse_bool(const char *s, const char *e);
+int runtime_get_params(const char *cmdline, char *values, size_t maxlen);
 
 /**
  * Given a specific name, parses a string of the form:
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 3d00c747f6..1b832e9a4c 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -830,6 +830,9 @@ static int flask_sysctl(int cmd)
     case XEN_SYSCTL_set_parameter:
         return avc_current_has_perm(SECINITSID_XEN, SECCLASS_XEN2,
                                     XEN2__SET_PARAMETER, NULL);
+    case XEN_SYSCTL_get_parameter:
+        return avc_current_has_perm(SECINITSID_XEN, SECCLASS_XEN2,
+                                    XEN2__GET_PARAMETER, NULL);
 
     default:
         return avc_unknown_permission("sysctl", cmd);
diff --git a/xen/xsm/flask/policy/access_vectors b/xen/xsm/flask/policy/access_vectors
index e00448b776..c5ee21d852 100644
--- a/xen/xsm/flask/policy/access_vectors
+++ b/xen/xsm/flask/policy/access_vectors
@@ -103,6 +103,8 @@ class xen2
     coverage_op
 # XEN_SYSCTL_set_parameter
     set_parameter
+# XEN_SYSCTL_get_parameter
+    get_parameter
 }
 
 # Classes domain and domain2 consist of operations that a domain performs on
-- 
2.20.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v4 1/4] xen: add hypercall for reading runtime parameters

[Vasilis Liaskovitis]

Add a sysctl hypercall to support reading hypervisor runtime parameters.

Limitations:
- Custom runtime parameters (OPT_CUSTOM) are not supported yet.
- For integer parameters (OPT_UINT), only unsigned parameters are printed
correctly.
- The implementation only reads runtime parameters, but it can be changed
to read all hypervisor parameters if needed. 

Signed-off-by: Vasilis Liaskovitis <vliaskovitis@suse.com>
---
 tools/flask/policy/modules/dom0.te  |   2 +-
 xen/common/kernel.c                 | 118 ++++++++++++++++++++++++++++
 xen/common/sysctl.c                 |  52 +++++++++++-
 xen/include/public/sysctl.h         |  18 +++++
 xen/include/xen/lib.h               |   1 +
 xen/xsm/flask/hooks.c               |   3 +
 xen/xsm/flask/policy/access_vectors |   2 +
 7 files changed, 193 insertions(+), 3 deletions(-)

diff --git a/tools/flask/policy/modules/dom0.te b/tools/flask/policy/modules/dom0.te
index a347d664f8..681d1a101b 100644
--- a/tools/flask/policy/modules/dom0.te
+++ b/tools/flask/policy/modules/dom0.te
@@ -16,7 +16,7 @@ allow dom0_t xen_t:xen {
 allow dom0_t xen_t:xen2 {
 	resource_op psr_cmt_op psr_alloc pmu_ctrl get_symbol
 	get_cpu_levelling_caps get_cpu_featureset livepatch_op
-	coverage_op set_parameter
+	coverage_op set_parameter get_parameter
 };
 
 # Allow dom0 to use all XENVER_ subops that have checks.
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index 612575430f..6695ffc372 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -12,6 +12,7 @@
 #include <xen/paging.h>
 #include <xen/guest_access.h>
 #include <xen/hypercall.h>
+#include <xen/ctype.h>
 #include <xsm/xsm.h>
 #include <asm/current.h>
 #include <public/version.h>
@@ -52,6 +53,123 @@ static int assign_integer_param(const struct kernel_param *param, uint64_t val)
     return 0;
 }
 
+static int get_integer_param(const struct kernel_param *param, uint64_t *val)
+{
+    switch ( param->len )
+    {
+    case sizeof(uint8_t):
+        *val = *(uint8_t *)param->par.var;
+        break;
+
+    case sizeof(uint16_t):
+        *val = *(uint16_t *)param->par.var;
+        break;
+
+    case sizeof(uint32_t):
+        *val = *(uint32_t *)param->par.var;
+        break;
+
+    case sizeof(uint64_t):
+        *val = *(uint64_t *)param->par.var;
+        break;
+
+    default:
+        BUG();
+        break;
+    }
+
+    return 0;
+}
+
+int runtime_get_params(const char *cmdline, char *values,
+                      size_t maxlen)
+{
+    char opt[128], *optkey, *q, *val = values;
+    const char *p = cmdline;
+    const struct kernel_param *param;
+    int rc = 0, len = 0;
+    size_t bufpos = 0;
+    uint64_t param_int;
+
+    while ( !rc )
+    {
+        /* Skip whitespace. */
+        while ( isspace(*p) )
+            p++;
+        if ( *p == '\0' )
+            break;
+
+        /* Grab the next whitespace-delimited option. */
+        q = optkey = opt;
+        while ( !isspace(*p) && (*p != '\0') )
+        {
+            if ( (q - opt) < (sizeof(opt) - 1) ) /* avoid overflow */
+                *q++ = *p;
+            else return -ENOMEM;
+            p++;
+        }
+        *q = '\0';
+
+        for ( param = __param_start; param < __param_end; param++ )
+        {
+            if ( strcmp(param->name, optkey) )
+                continue;
+
+            switch ( param->type )
+            {
+            case OPT_STR:
+                len = snprintf(val + bufpos, maxlen - bufpos, "%s ",
+                               (char*)param->par.var);
+                break;
+
+            case OPT_UINT:
+            case OPT_SIZE:
+                /* Signed integer parameters are not supported yet.
+                 * While there are no runtime signed integer parameters
+                 * at the moment, adding one and trying to get its value
+                 * with the current implementation will output the wrong
+                 * value.
+                 */
+                get_integer_param(param, &param_int);
+                len = snprintf(val + bufpos, maxlen - bufpos,
+                               "%"PRIu64" ", param_int);
+                break;
+
+            case OPT_BOOL:
+                get_integer_param(param, &param_int);
+                len = snprintf(val + bufpos, maxlen - bufpos, "%s ",
+                               param_int ? "true" : "false");
+                break;
+
+            case OPT_CUSTOM:
+                /* Custom parameters are not supported yet. */
+                rc = -EINVAL;
+                break;
+
+            default:
+                BUG();
+                break;
+            }
+
+            if ( len < 0 )
+                rc = len;
+            else if ( len < maxlen - bufpos )
+                /* if output was not truncated update buffer position. */
+                bufpos += len;
+            else if ( len > 0 )
+                rc = -ENOMEM;
+
+            break;
+        }
+
+        /* no parameter was matched */
+        if ( param >= __param_end )
+            rc = -EINVAL;
+    }
+
+    return rc;
+}
+
 static int parse_params(const char *cmdline, const struct kernel_param *start,
                         const struct kernel_param *end)
 {
diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index c0aa6bde4e..20be6a6d8d 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -466,9 +466,9 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
             copyback = 1;
         break;
 
+#define XEN_PARAMETER_MAX_SIZE 1023
     case XEN_SYSCTL_set_parameter:
     {
-#define XEN_SET_PARAMETER_MAX_SIZE 1023
         char *params;
 
         if ( op->u.set_parameter.pad[0] || op->u.set_parameter.pad[1] ||
@@ -477,7 +477,7 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
             ret = -EINVAL;
             break;
         }
-        if ( op->u.set_parameter.size > XEN_SET_PARAMETER_MAX_SIZE )
+        if ( op->u.set_parameter.size > XEN_PARAMETER_MAX_SIZE )
         {
             ret = -E2BIG;
             break;
@@ -501,6 +501,54 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
 
         break;
     }
+    case XEN_SYSCTL_get_parameter:
+    {
+        char *params, *values;
+
+        if ( op->u.get_parameter.pad[0] || op->u.get_parameter.pad[1] ||
+             op->u.get_parameter.pad[2] )
+        {
+            ret = -EINVAL;
+            break;
+        }
+        if ( op->u.get_parameter.size > XEN_PARAMETER_MAX_SIZE )
+        {
+            ret = -E2BIG;
+            break;
+        }
+        params = xmalloc_bytes(op->u.get_parameter.size + 1);
+        if ( !params )
+        {
+            ret = -ENOMEM;
+            break;
+        }
+
+        values = xmalloc_bytes(XEN_PARAMETER_MAX_SIZE);
+        if ( !values )
+        {
+            xfree(params);
+            ret = -ENOMEM;
+            break;
+        }
+
+        if ( copy_from_guest(params, op->u.get_parameter.params,
+                             op->u.get_parameter.size) )
+            ret = -EFAULT;
+        else
+        {
+            params[op->u.set_parameter.size] = 0;
+            ret = runtime_get_params(params, values, XEN_PARAMETER_MAX_SIZE);
+
+            if ( !ret && copy_to_guest(op->u.get_parameter.values, values,
+                                       strlen(values)) )
+                ret = -EFAULT;
+        }
+
+        xfree(params);
+        xfree(values);
+
+        break;
+    }
 
     default:
         ret = arch_do_sysctl(op, u_sysctl);
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index c49b4dcc99..7d77d57115 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -1100,6 +1100,22 @@ typedef struct xen_sysctl_cpu_policy xen_sysctl_cpu_policy_t;
 DEFINE_XEN_GUEST_HANDLE(xen_sysctl_cpu_policy_t);
 #endif
 
+/*
+ * XEN_SYSCTL_get_parameter
+ *
+ * Read hypervisor parameters at runtime.
+ * Parameters are a single string terminated by a NUL byte of max. size
+ * characters. Multiple settings can be specified by separating them
+ * with blanks.
+ */
+
+struct xen_sysctl_get_parameter {
+    XEN_GUEST_HANDLE_64(char) params;       /* IN: pointer to parameters. */
+    XEN_GUEST_HANDLE_64(char) values;       /* OUT: pointer to output values. */
+    uint16_t size;                          /* IN: size of parameters. */
+    uint16_t pad[3];                        /* IN: MUST be zero. */
+};
+
 struct xen_sysctl {
     uint32_t cmd;
 #define XEN_SYSCTL_readconsole                    1
@@ -1130,6 +1146,7 @@ struct xen_sysctl {
 #define XEN_SYSCTL_livepatch_op                  27
 #define XEN_SYSCTL_set_parameter                 28
 #define XEN_SYSCTL_get_cpu_policy                29
+#define XEN_SYSCTL_get_parameter                 30
     uint32_t interface_version; /* XEN_SYSCTL_INTERFACE_VERSION */
     union {
         struct xen_sysctl_readconsole       readconsole;
@@ -1162,6 +1179,7 @@ struct xen_sysctl {
 #if defined(__i386__) || defined(__x86_64__)
         struct xen_sysctl_cpu_policy        cpu_policy;
 #endif
+        struct xen_sysctl_get_parameter     get_parameter;
         uint8_t                             pad[128];
     } u;
 };
diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
index e0b7bcb6b7..6e6367bb7a 100644
--- a/xen/include/xen/lib.h
+++ b/xen/include/xen/lib.h
@@ -71,6 +71,7 @@ struct domain;
 void cmdline_parse(const char *cmdline);
 int runtime_parse(const char *line);
 int parse_bool(const char *s, const char *e);
+int runtime_get_params(const char *cmdline, char *values, size_t maxlen);
 
 /**
  * Given a specific name, parses a string of the form:
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 3d00c747f6..1b832e9a4c 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -830,6 +830,9 @@ static int flask_sysctl(int cmd)
     case XEN_SYSCTL_set_parameter:
         return avc_current_has_perm(SECINITSID_XEN, SECCLASS_XEN2,
                                     XEN2__SET_PARAMETER, NULL);
+    case XEN_SYSCTL_get_parameter:
+        return avc_current_has_perm(SECINITSID_XEN, SECCLASS_XEN2,
+                                    XEN2__GET_PARAMETER, NULL);
 
     default:
         return avc_unknown_permission("sysctl", cmd);
diff --git a/xen/xsm/flask/policy/access_vectors b/xen/xsm/flask/policy/access_vectors
index e00448b776..c5ee21d852 100644
--- a/xen/xsm/flask/policy/access_vectors
+++ b/xen/xsm/flask/policy/access_vectors
@@ -103,6 +103,8 @@ class xen2
     coverage_op
 # XEN_SYSCTL_set_parameter
     set_parameter
+# XEN_SYSCTL_get_parameter
+    get_parameter
 }
 
 # Classes domain and domain2 consist of operations that a domain performs on
-- 
2.20.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v4 2/4] libxc: add function to get hypervisor parameters

[Vasilis Liaskovitis]

Add a new libxc function to get hypervisor parameters.

Signed-off-by: Vasilis Liaskovitis <vliaskovitis@suse.com>
---
 tools/libxc/include/xenctrl.h |  1 +
 tools/libxc/xc_misc.c         | 26 ++++++++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index a3628e56bb..3482ca1a91 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1228,6 +1228,7 @@ int xc_readconsolering(xc_interface *xch,
 
 int xc_send_debug_keys(xc_interface *xch, char *keys);
 int xc_set_parameters(xc_interface *xch, char *params);
+int xc_get_parameters(xc_interface *xch, char *params, char *values);
 
 typedef struct xen_sysctl_physinfo xc_physinfo_t;
 typedef struct xen_sysctl_cputopo xc_cputopo_t;
diff --git a/tools/libxc/xc_misc.c b/tools/libxc/xc_misc.c
index 5e6714ae2b..439ad91194 100644
--- a/tools/libxc/xc_misc.c
+++ b/tools/libxc/xc_misc.c
@@ -208,6 +208,32 @@ int xc_set_parameters(xc_interface *xch, char *params)
     return ret;
 }
 
+int xc_get_parameters(xc_interface *xch, char *params, char *values)
+{
+    int ret, len = strlen(params);
+    DECLARE_SYSCTL;
+    DECLARE_HYPERCALL_BOUNCE(params, len, XC_HYPERCALL_BUFFER_BOUNCE_IN);
+    DECLARE_HYPERCALL_BOUNCE(values, 1023, XC_HYPERCALL_BUFFER_BOUNCE_OUT);
+
+    if ( xc_hypercall_bounce_pre(xch, params) )
+        return -1;
+    if ( xc_hypercall_bounce_pre(xch, values) )
+        return -1;
+
+    sysctl.cmd = XEN_SYSCTL_get_parameter;
+    set_xen_guest_handle(sysctl.u.get_parameter.params, params);
+    set_xen_guest_handle(sysctl.u.get_parameter.values, values);
+    sysctl.u.get_parameter.size = len;
+    memset(sysctl.u.get_parameter.pad, 0, sizeof(sysctl.u.get_parameter.pad));
+
+    ret = do_sysctl(xch, &sysctl);
+
+    xc_hypercall_bounce_post(xch, params);
+    xc_hypercall_bounce_post(xch, values);
+
+    return ret;
+}
+
 int xc_physinfo(xc_interface *xch,
                 xc_physinfo_t *put_info)
 {
-- 
2.20.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v4 2/4] libxc: add function to get hypervisor parameters

[Vasilis Liaskovitis]

Add a new libxc function to get hypervisor parameters.

Signed-off-by: Vasilis Liaskovitis <vliaskovitis@suse.com>
---
 tools/libxc/include/xenctrl.h |  1 +
 tools/libxc/xc_misc.c         | 26 ++++++++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index a3628e56bb..3482ca1a91 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1228,6 +1228,7 @@ int xc_readconsolering(xc_interface *xch,
 
 int xc_send_debug_keys(xc_interface *xch, char *keys);
 int xc_set_parameters(xc_interface *xch, char *params);
+int xc_get_parameters(xc_interface *xch, char *params, char *values);
 
 typedef struct xen_sysctl_physinfo xc_physinfo_t;
 typedef struct xen_sysctl_cputopo xc_cputopo_t;
diff --git a/tools/libxc/xc_misc.c b/tools/libxc/xc_misc.c
index 5e6714ae2b..439ad91194 100644
--- a/tools/libxc/xc_misc.c
+++ b/tools/libxc/xc_misc.c
@@ -208,6 +208,32 @@ int xc_set_parameters(xc_interface *xch, char *params)
     return ret;
 }
 
+int xc_get_parameters(xc_interface *xch, char *params, char *values)
+{
+    int ret, len = strlen(params);
+    DECLARE_SYSCTL;
+    DECLARE_HYPERCALL_BOUNCE(params, len, XC_HYPERCALL_BUFFER_BOUNCE_IN);
+    DECLARE_HYPERCALL_BOUNCE(values, 1023, XC_HYPERCALL_BUFFER_BOUNCE_OUT);
+
+    if ( xc_hypercall_bounce_pre(xch, params) )
+        return -1;
+    if ( xc_hypercall_bounce_pre(xch, values) )
+        return -1;
+
+    sysctl.cmd = XEN_SYSCTL_get_parameter;
+    set_xen_guest_handle(sysctl.u.get_parameter.params, params);
+    set_xen_guest_handle(sysctl.u.get_parameter.values, values);
+    sysctl.u.get_parameter.size = len;
+    memset(sysctl.u.get_parameter.pad, 0, sizeof(sysctl.u.get_parameter.pad));
+
+    ret = do_sysctl(xch, &sysctl);
+
+    xc_hypercall_bounce_post(xch, params);
+    xc_hypercall_bounce_post(xch, values);
+
+    return ret;
+}
+
 int xc_physinfo(xc_interface *xch,
                 xc_physinfo_t *put_info)
 {
-- 
2.20.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v4 3/4] libxl: add libxl_get_parameters() function

[Vasilis Liaskovitis]

Add a new libxl function to get hypervisor parameters.

Signed-off-by: Vasilis Liaskovitis <vliaskovitis@suse.com>
---
 tools/libxl/libxl.c | 19 +++++++++++++++++++
 tools/libxl/libxl.h |  1 +
 2 files changed, 20 insertions(+)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index ec71574e99..9bb0382c38 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -669,6 +669,25 @@ int libxl_set_parameters(libxl_ctx *ctx, char *params)
     return 0;
 }
 
+int libxl_get_parameters(libxl_ctx *ctx, char *params, char *values)
+{
+    int r, rc;
+    GC_INIT(ctx);
+
+    r = xc_get_parameters(ctx->xch, params, values);
+    if (r < 0) {
+        LOGE(ERROR, "getting parameters");
+        rc = ERROR_FAIL;
+        goto out;
+    }
+
+    rc = 0;
+
+out:
+    GC_FREE;
+    return rc;
+}
+
 static int fd_set_flags(libxl_ctx *ctx, int fd,
                         int fcntlgetop, int fcntlsetop, const char *fl,
                         int flagmask, int set_p)
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index a38e5cdba2..360a757a06 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -2307,6 +2307,7 @@ int libxl_send_trigger(libxl_ctx *ctx, uint32_t domid,
 int libxl_send_sysrq(libxl_ctx *ctx, uint32_t domid, char sysrq);
 int libxl_send_debug_keys(libxl_ctx *ctx, char *keys);
 int libxl_set_parameters(libxl_ctx *ctx, char *params);
+int libxl_get_parameters(libxl_ctx *ctx, char *params, char *values);
 
 typedef struct libxl__xen_console_reader libxl_xen_console_reader;
 
-- 
2.20.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v4 3/4] libxl: add libxl_get_parameters() function

[Vasilis Liaskovitis]

Add a new libxl function to get hypervisor parameters.

Signed-off-by: Vasilis Liaskovitis <vliaskovitis@suse.com>
---
 tools/libxl/libxl.c | 19 +++++++++++++++++++
 tools/libxl/libxl.h |  1 +
 2 files changed, 20 insertions(+)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index ec71574e99..9bb0382c38 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -669,6 +669,25 @@ int libxl_set_parameters(libxl_ctx *ctx, char *params)
     return 0;
 }
 
+int libxl_get_parameters(libxl_ctx *ctx, char *params, char *values)
+{
+    int r, rc;
+    GC_INIT(ctx);
+
+    r = xc_get_parameters(ctx->xch, params, values);
+    if (r < 0) {
+        LOGE(ERROR, "getting parameters");
+        rc = ERROR_FAIL;
+        goto out;
+    }
+
+    rc = 0;
+
+out:
+    GC_FREE;
+    return rc;
+}
+
 static int fd_set_flags(libxl_ctx *ctx, int fd,
                         int fcntlgetop, int fcntlsetop, const char *fl,
                         int flagmask, int set_p)
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index a38e5cdba2..360a757a06 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -2307,6 +2307,7 @@ int libxl_send_trigger(libxl_ctx *ctx, uint32_t domid,
 int libxl_send_sysrq(libxl_ctx *ctx, uint32_t domid, char sysrq);
 int libxl_send_debug_keys(libxl_ctx *ctx, char *keys);
 int libxl_set_parameters(libxl_ctx *ctx, char *params);
+int libxl_get_parameters(libxl_ctx *ctx, char *params, char *values);
 
 typedef struct libxl__xen_console_reader libxl_xen_console_reader;
 
-- 
2.20.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v4 4/4] xl: add new xl command get-parameters

[Vasilis Liaskovitis]

Add a new xl command "get-parameters" to get hypervisor runtime parameters.

Examples:

xl get-parameters "gnttab_max_frames gnttab_max_maptrack_frames"
gnttab_max_frames gnttab_max_maptrack_frames : 64 1024

xl set-parameters gnttab_max_frames=128

xl get-parameters gnttab_max_frames
gnttab_max_frames : 128

xl get-parameters "gnttab_max_frames gnttab_max_maptrack_frames"
gnttab_max_frames gnttab_max_maptrack_frames : 128 1024

Signed-off-by: Vasilis Liaskovitis <vliaskovitis@suse.com>
---
 docs/man/xl.1.pod.in   |  5 +++++
 tools/xl/xl.h          |  1 +
 tools/xl/xl_cmdtable.c |  5 +++++
 tools/xl/xl_misc.c     | 25 +++++++++++++++++++++++++
 4 files changed, 36 insertions(+)

diff --git a/docs/man/xl.1.pod.in b/docs/man/xl.1.pod.in
index 4310fcd818..a1fff4d382 100644
--- a/docs/man/xl.1.pod.in
+++ b/docs/man/xl.1.pod.in
@@ -827,6 +827,11 @@ Send debug I<keys> to Xen. It is the same as pressing the Xen
 Set hypervisor parameters as specified in I<params>. This allows for some
 boot parameters of the hypervisor to be modified in the running systems.
 
+=item B<get-parameters> I<params>
+
+Get hypervisor parameters as specified in I<params>. This allows for some
+boot parameters of the hypervisor to be read in the running systems.
+
 =item B<dmesg> [I<OPTIONS>]
 
 Reads the Xen message buffer, similar to dmesg on a Linux system.  The
diff --git a/tools/xl/xl.h b/tools/xl/xl.h
index cf4202bc89..af3843e5b0 100644
--- a/tools/xl/xl.h
+++ b/tools/xl/xl.h
@@ -219,6 +219,7 @@ int main_psr_mba_set(int argc, char **argv);
 int main_psr_mba_show(int argc, char **argv);
 #endif
 int main_qemu_monitor_command(int argc, char **argv);
+int main_get_parameters(int argc, char **argv);
 
 void help(const char *command);
 
diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
index 89716badcb..a18481619b 100644
--- a/tools/xl/xl_cmdtable.c
+++ b/tools/xl/xl_cmdtable.c
@@ -662,6 +662,11 @@ struct cmd_spec cmd_table[] = {
       "Issue a qemu monitor command to the device model of a domain",
       "<Domain> <Command>",
     },
+    { "get-parameters",
+      &main_get_parameters, 0, 1,
+      "Get hypervisor parameters",
+      "<Params>",
+    },
 };
 
 int cmdtable_len = sizeof(cmd_table)/sizeof(struct cmd_spec);
diff --git a/tools/xl/xl_misc.c b/tools/xl/xl_misc.c
index dcf940a6d4..811f231b78 100644
--- a/tools/xl/xl_misc.c
+++ b/tools/xl/xl_misc.c
@@ -364,6 +364,31 @@ int main_config_update(int argc, char **argv)
     return 0;
 }
 
+int main_get_parameters(int argc, char **argv)
+{
+    int opt, ret;
+    char *params;
+    char values[1023];
+
+    SWITCH_FOREACH_OPT(opt, "", NULL, "get-parameters", 1) {
+        /* No options */
+    }
+
+    params = argv[optind];
+
+    if (!params) {
+	fprintf(stderr, "no parameter specified\n");
+	return EXIT_FAILURE;
+    }
+    else if ((ret = libxl_get_parameters(ctx, params, values))) {
+        fprintf(stderr, "cannot get parameters: %s : %d\n", params, ret);
+        fprintf(stderr, "Use \"xl dmesg\" to look for possible reason.\n");
+        return EXIT_FAILURE;
+    }
+    fprintf(stderr, "%s : %s\n", params, values);
+
+    return EXIT_SUCCESS;
+}
 /*
  * Local variables:
  * mode: C
-- 
2.20.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v4 4/4] xl: add new xl command get-parameters

[Vasilis Liaskovitis]

Add a new xl command "get-parameters" to get hypervisor runtime parameters.

Examples:

xl get-parameters "gnttab_max_frames gnttab_max_maptrack_frames"
gnttab_max_frames gnttab_max_maptrack_frames : 64 1024

xl set-parameters gnttab_max_frames=128

xl get-parameters gnttab_max_frames
gnttab_max_frames : 128

xl get-parameters "gnttab_max_frames gnttab_max_maptrack_frames"
gnttab_max_frames gnttab_max_maptrack_frames : 128 1024

Signed-off-by: Vasilis Liaskovitis <vliaskovitis@suse.com>
---
 docs/man/xl.1.pod.in   |  5 +++++
 tools/xl/xl.h          |  1 +
 tools/xl/xl_cmdtable.c |  5 +++++
 tools/xl/xl_misc.c     | 25 +++++++++++++++++++++++++
 4 files changed, 36 insertions(+)

diff --git a/docs/man/xl.1.pod.in b/docs/man/xl.1.pod.in
index 4310fcd818..a1fff4d382 100644
--- a/docs/man/xl.1.pod.in
+++ b/docs/man/xl.1.pod.in
@@ -827,6 +827,11 @@ Send debug I<keys> to Xen. It is the same as pressing the Xen
 Set hypervisor parameters as specified in I<params>. This allows for some
 boot parameters of the hypervisor to be modified in the running systems.
 
+=item B<get-parameters> I<params>
+
+Get hypervisor parameters as specified in I<params>. This allows for some
+boot parameters of the hypervisor to be read in the running systems.
+
 =item B<dmesg> [I<OPTIONS>]
 
 Reads the Xen message buffer, similar to dmesg on a Linux system.  The
diff --git a/tools/xl/xl.h b/tools/xl/xl.h
index cf4202bc89..af3843e5b0 100644
--- a/tools/xl/xl.h
+++ b/tools/xl/xl.h
@@ -219,6 +219,7 @@ int main_psr_mba_set(int argc, char **argv);
 int main_psr_mba_show(int argc, char **argv);
 #endif
 int main_qemu_monitor_command(int argc, char **argv);
+int main_get_parameters(int argc, char **argv);
 
 void help(const char *command);
 
diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
index 89716badcb..a18481619b 100644
--- a/tools/xl/xl_cmdtable.c
+++ b/tools/xl/xl_cmdtable.c
@@ -662,6 +662,11 @@ struct cmd_spec cmd_table[] = {
       "Issue a qemu monitor command to the device model of a domain",
       "<Domain> <Command>",
     },
+    { "get-parameters",
+      &main_get_parameters, 0, 1,
+      "Get hypervisor parameters",
+      "<Params>",
+    },
 };
 
 int cmdtable_len = sizeof(cmd_table)/sizeof(struct cmd_spec);
diff --git a/tools/xl/xl_misc.c b/tools/xl/xl_misc.c
index dcf940a6d4..811f231b78 100644
--- a/tools/xl/xl_misc.c
+++ b/tools/xl/xl_misc.c
@@ -364,6 +364,31 @@ int main_config_update(int argc, char **argv)
     return 0;
 }
 
+int main_get_parameters(int argc, char **argv)
+{
+    int opt, ret;
+    char *params;
+    char values[1023];
+
+    SWITCH_FOREACH_OPT(opt, "", NULL, "get-parameters", 1) {
+        /* No options */
+    }
+
+    params = argv[optind];
+
+    if (!params) {
+	fprintf(stderr, "no parameter specified\n");
+	return EXIT_FAILURE;
+    }
+    else if ((ret = libxl_get_parameters(ctx, params, values))) {
+        fprintf(stderr, "cannot get parameters: %s : %d\n", params, ret);
+        fprintf(stderr, "Use \"xl dmesg\" to look for possible reason.\n");
+        return EXIT_FAILURE;
+    }
+    fprintf(stderr, "%s : %s\n", params, values);
+
+    return EXIT_SUCCESS;
+}
 /*
  * Local variables:
  * mode: C
-- 
2.20.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v4 1/4] xen: add hypercall for reading runtime parameters

[Jan Beulich]

>>> On 28.05.19 at 16:54, <vliaskovitis@suse.com> wrote:
> +int runtime_get_params(const char *cmdline, char *values,
> +                      size_t maxlen)
> +{
> +    char opt[128], *optkey, *q, *val = values;
> +    const char *p = cmdline;
> +    const struct kernel_param *param;
> +    int rc = 0, len = 0;
> +    size_t bufpos = 0;
> +    uint64_t param_int;
> +
> +    while ( !rc )
> +    {
> +        /* Skip whitespace. */
> +        while ( isspace(*p) )
> +            p++;
> +        if ( *p == '\0' )
> +            break;
> +
> +        /* Grab the next whitespace-delimited option. */
> +        q = optkey = opt;
> +        while ( !isspace(*p) && (*p != '\0') )
> +        {
> +            if ( (q - opt) < (sizeof(opt) - 1) ) /* avoid overflow */
> +                *q++ = *p;
> +            else return -ENOMEM;
> +            p++;
> +        }
> +        *q = '\0';
> +
> +        for ( param = __param_start; param < __param_end; param++ )
> +        {
> +            if ( strcmp(param->name, optkey) )
> +                continue;
> +
> +            switch ( param->type )
> +            {
> +            case OPT_STR:
> +                len = snprintf(val + bufpos, maxlen - bufpos, "%s ",
> +                               (char*)param->par.var);
> +                break;
> +
> +            case OPT_UINT:
> +            case OPT_SIZE:
> +                /* Signed integer parameters are not supported yet.
> +                 * While there are no runtime signed integer parameters
> +                 * at the moment, adding one and trying to get its value
> +                 * with the current implementation will output the wrong
> +                 * value.
> +                 */

Comment style.

> +                get_integer_param(param, &param_int);
> +                len = snprintf(val + bufpos, maxlen - bufpos,
> +                               "%"PRIu64" ", param_int);
> +                break;
> +
> +            case OPT_BOOL:
> +                get_integer_param(param, &param_int);
> +                len = snprintf(val + bufpos, maxlen - bufpos, "%s ",
> +                               param_int ? "true" : "false");
> +                break;
> +
> +            case OPT_CUSTOM:
> +                /* Custom parameters are not supported yet. */
> +                rc = -EINVAL;
> +                break;
> +
> +            default:
> +                BUG();
> +                break;
> +            }
> +
> +            if ( len < 0 )
> +                rc = len;
> +            else if ( len < maxlen - bufpos )
> +                /* if output was not truncated update buffer position. */

Again. It's pretty odd to have a full stop (which isn't mandatory, but
I appreciate it being there) yet start with a lower case letter (which
isn't permitted).

> --- a/xen/common/sysctl.c
> +++ b/xen/common/sysctl.c
> @@ -466,9 +466,9 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
>              copyback = 1;
>          break;
>  
> +#define XEN_PARAMETER_MAX_SIZE 1023
>      case XEN_SYSCTL_set_parameter:
>      {
> -#define XEN_SET_PARAMETER_MAX_SIZE 1023
>          char *params;
>  
>          if ( op->u.set_parameter.pad[0] || op->u.set_parameter.pad[1] ||
> @@ -477,7 +477,7 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
>              ret = -EINVAL;
>              break;
>          }
> -        if ( op->u.set_parameter.size > XEN_SET_PARAMETER_MAX_SIZE )
> +        if ( op->u.set_parameter.size > XEN_PARAMETER_MAX_SIZE )
>          {
>              ret = -E2BIG;
>              break;
> @@ -501,6 +501,54 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
>  
>          break;
>      }
> +    case XEN_SYSCTL_get_parameter:

Blank line between non-fall-through case blocks please.

These are all issues which could easily be fixed while committing,
and other than those I'm now fine with the patch. However, I'm
still not overly happy with the restrictions named, some of which
derive from certain interface decisions (like using ASCII strings
as return values). I'd therefore like to have at least one other
maintainer's opinion as to whether to indeed go with this interface
before giving my ack.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v4 0/4] Support for reading runtime hypervisor parameters

[Juergen Gross]

On 28.05.19 16:54, Vasilis Liaskovitis wrote:
> Currently runtime parameters of the hypervisor cannot be inspected through an
> xl command, however they can be changed with the "xl set-parameter" command.
> Being able to check these parameters at runtime would be a useful diagnostic
> tool.
> 
> This patch series implements a new xl command "xl get-parameters"
> which takes a string of input parameters and returns their current
> values in the hypervisor settings.
> 
> Examples:
> 
> xl get-parameters "gnttab_max_frames gnttab_max_maptrack_frames"
> gnttab_max_frames gnttab_max_maptrack_frames : 64 1024
> 
> xl set-parameters gnttab_max_frames=128
> 
> xl get-parameters gnttab_max_frames
> gnttab_max_frames : 128
> 
> xl get-parameters "gnttab_max_frames gnttab_max_maptrack_frames"
> gnttab_max_frames gnttab_max_maptrack_frames : 128 1024
> 
> 
> Changes v3->v4:
> 
> - Fix return value and logging in new libxl function.
> 
> Changes v2->v3:
> 
> - Several style / formatting fixes
> - Limitations for signed integer parameters added in code and commit log.
> 
> Changes v1->v2:
> 
> - fixed snprintf issues, fixed memory leaks and error handling
> - removed unnecessary wrapper function
> - OPT_SIZE is handled
> 
> Limitations:
> 
> - Custom runtime parameters (OPT_CUSTOM) are not supported yet. I'd like
>    to do this in a follow-up series. See also discussion at
>    https://lists.xenproject.org/archives/html/xen-devel/2019-03/msg01383.html
> - For integer parameters (OPT_UINT), only unsigned parameters are printed
>    correctly at the moment.
> 
> Vasilis Liaskovitis (4):
>    xen: add hypercall for getting parameters at runtime
>    libxc: add function to get hypervisor parameters
>    libxl: add libxl_get_parameters() function
>    xl: add new xl command get-parameters
> 
>   docs/man/xl.1.pod.in                |   5 ++
>   tools/flask/policy/modules/dom0.te  |   2 +-
>   tools/libxc/include/xenctrl.h       |   1 +
>   tools/libxc/xc_misc.c               |  26 ++++++
>   tools/libxl/libxl.c                 |  19 +++++
>   tools/libxl/libxl.h                 |   1 +
>   tools/xl/xl.h                       |   1 +
>   tools/xl/xl_cmdtable.c              |   5 ++
>   tools/xl/xl_misc.c                  |  25 ++++++
>   xen/common/kernel.c                 | 118 ++++++++++++++++++++++++++++
>   xen/common/sysctl.c                 |  52 +++++++++++-
>   xen/include/public/sysctl.h         |  18 +++++
>   xen/include/xen/lib.h               |   1 +
>   xen/xsm/flask/hooks.c               |   3 +
>   xen/xsm/flask/policy/access_vectors |   2 +
>   15 files changed, 276 insertions(+), 3 deletions(-)
> 

At the Xen developers summit last week in Chicago there was a related
discussion. The basic outcome was that there is some need for a generic
framework to report hypervisor settings to dom0, especially for the
currently active mitigation settings of cpu vulnerabilities. The most
appropriate solution seems to be a hierarchical key-value store in the
hypervisor which can be queried via a new interface. The same interface
would be usable for reading hypervisor parameters, too.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel