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

[Vasilis Liaskovitis]

Add a new libxl function to get hypervisor parameters.

Signed-off-by: Vasilis Liaskovitis 
---
 tools/libxl/libxl.c | 15 +++
 tools/libxl/libxl.h |  1 +
 2 files changed, 16 insertions(+)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index ec71574e99..124033e5a3 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -669,6 +669,21 @@ int libxl_set_parameters(libxl_ctx *ctx, char *params)
 return 0;
 }
 
+int libxl_get_parameters(libxl_ctx *ctx, char *params, char *values)
+{
+int ret;
+GC_INIT(ctx);
+
+ret = xc_get_parameters(ctx->xch, params, values);
+if (ret < 0) {
+LOGEV(ERROR, ret, "getting parameters");
+GC_FREE;
+return ret;//ERROR_FAIL;
+}
+GC_FREE;
+return 0;
+}
+
 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 v3 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.

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.

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

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 |  15 
 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 | 113 
 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, 267 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 v3 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 
---
 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 to Xen. It is the same as pressing the 
Xen
 Set hypervisor parameters as specified in I. This allows for some
 boot parameters of the hypervisor to be modified in the running systems.
 
+=item B I
+
+Get hypervisor parameters as specified in I. This allows for some
+boot parameters of the hypervisor to be read in the running systems.
+
 =item B [I]
 
 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",
   " ",
 },
+{ "get-parameters",
+  &main_get_parameters, 0, 1,
+  "Get hypervisor parameters",
+  "",
+},
 };
 
 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 v3 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 
---
 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 v3 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 
---
 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 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -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, ¶m_int);
+len = snprintf(val + bufpos, maxlen - bufpos,
+   "%"PRIu64" ", param_int);
+break;
+
+case OPT_BOOL:
+get_integer_param(param, ¶m_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 ( p

[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 
---
 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 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 
---
 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 to Xen. It is the same as pressing the 
Xen
 Set hypervisor parameters as specified in I. This allows for some
 boot parameters of the hypervisor to be modified in the running systems.
 
+=item B I
+
+Get hypervisor parameters as specified in I. This allows for some
+boot parameters of the hypervisor to be read in the running systems.
+
 =item B [I]
 
 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",
   " ",
 },
+{ "get-parameters",
+  &main_get_parameters, 0, 1,
+  "Get hypervisor parameters",
+  "",
+},
 };
 
 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 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 
---
 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 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 
---
 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 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -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, ¶m_int);
+len = snprintf(val + bufpos, maxlen - bufpos,
+   "%"PRIu64" ", param_int);
+break;
+
+case OPT_BOOL:
+get_integer_param(param, ¶m_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 ( p

[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

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

[Vasilis Liaskovitis]

Add a new libxl function to get hypervisor parameters at runtime.

Signed-off-by: Vasilis Liaskovitis 
---
 tools/libxl/libxl.c | 15 +++
 tools/libxl/libxl.h |  1 +
 2 files changed, 16 insertions(+)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index ec71574e99..124033e5a3 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -669,6 +669,21 @@ int libxl_set_parameters(libxl_ctx *ctx, char *params)
 return 0;
 }
 
+int libxl_get_parameters(libxl_ctx *ctx, char *params, char *values)
+{
+int ret;
+GC_INIT(ctx);
+
+ret = xc_get_parameters(ctx->xch, params, values);
+if (ret < 0) {
+LOGEV(ERROR, ret, "getting parameters");
+GC_FREE;
+return ret;//ERROR_FAIL;
+}
+GC_FREE;
+return 0;
+}
+
 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 v1 2/4] libxc: add function to get hypervisor parameters

[Vasilis Liaskovitis]

Add a new libxc function to get hypervisor parameters at runtime.

Signed-off-by: Vasilis Liaskovitis 
---
 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 31cdda76c6..281185063d 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 v1 4/4] xl: add new xl command get-parameters

[Vasilis Liaskovitis]

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

Signed-off-by: Vasilis Liaskovitis 
---
 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 to Xen. It is the same as pressing the 
Xen
 Set hypervisor parameters as specified in I. This allows for some
 boot parameters of the hypervisor to be modified in the running systems.
 
+=item B I
+
+Get hypervisor parameters as specified in I. This allows for some
+boot parameters of the hypervisor to be read in the running systems.
+
 =item B [I]
 
 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",
   " ",
 },
+{ "get-parameters",
+  &main_get_parameters, 0, 1,
+  "Get hypervisor parameters",
+  "",
+},
 };
 
 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 v1 0/4] Support for reading hypervisor parameters at runtime

[Vasilis Liaskovitis]

Currently parameters of the hypervisor cannot be inspected at runtime
through an xl command. Such a command would be a useful diagnostic
tool e.g. used in conjunction with the "xl set-parameters" command.

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

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 set-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 |  15 
 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 | 109 
 xen/common/sysctl.c |  45 
 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, 258 insertions(+), 1 deletion(-)

-- 
2.20.1


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

[Xen-devel] [PATCH v1 1/4] xen: add hypercall for getting parameters at runtime

[Vasilis Liaskovitis]

Add a sysctl hypercall to support getting hypervisor parameters
at runtime.

Signed-off-by: Vasilis Liaskovitis 
---
 tools/flask/policy/modules/dom0.te  |   2 +-
 xen/common/kernel.c | 109 
 xen/common/sysctl.c |  45 
 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, 179 insertions(+), 1 deletion(-)

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..83225afd93 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -52,6 +52,110 @@ 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();
+}
+
+return 0;
+}
+
+static int get_params(const char *cmdline, char *values,
+  const struct kernel_param *start,
+  const struct kernel_param *end)
+{
+char opt[128], *optkey, *q;
+const char *p = cmdline, *val = values;
+const struct kernel_param *param;
+int len, rc = 0;
+uint64_t param_int;
+bool found;
+
+if (!values)
+return -EFAULT;
+
+for ( ; ; )
+{
+/* Skip whitespace. */
+while ( *p == ' ' )
+p++;
+if ( *p == '\0' )
+break;
+
+/* Grab the next whitespace-delimited option. */
+q = optkey = opt;
+while ( (*p != ' ') && (*p != '\0') )
+{
+if ( (q-opt) < (sizeof(opt)-1) ) /* avoid overflow */
+*q++ = *p;
+p++;
+}
+*q = '\0';
+
+/* Boolean parameters can be inverted with 'no-' prefix. */
+
+found = false;
+for ( param = start; param < end; param++ )
+{
+
+if ( strcmp(param->name, optkey) )
+continue;
+
+found = true;
+switch ( param->type )
+{
+case OPT_STR:
+len = snprintf((char*)val, sizeof(values), "%s ",
+   (char*)param->par.var);
+val += len;
+break;
+case OPT_UINT:
+get_integer_param(param, ¶m_int);
+len = snprintf((char*)val, sizeof(values), "%lu ", param_int);
+val += len;
+break;
+case OPT_BOOL:
+get_integer_param(param, ¶m_int);
+len = snprintf((char*)val, sizeof(values), "%s",
+   param_int ? "true" : "false");
+val += len;
+break;
+case OPT_SIZE:
+case OPT_CUSTOM:
+rc = -EINVAL;
+break;
+default:
+BUG();
+break;
+}
+}
+
+if ( !found )
+{
+printk("get-parameters: parameter \"%s\" unknown!\n", optkey);
+rc = -EINVAL;
+}
+}
+*val = '\0';
+
+return rc;
+}
+
 static int parse_params(const char *cmdline, const struct kernel_param *start,
 const struct kernel_param *end)
 {
@@ -199,6 +303,11 @@ int runtime_parse(const char *line)
 return parse_params(line, __param_start, __param_end);
 }
 
+int runtime_get_parameter(const char *line, char *values)
+{
+return get_params(line, values, __param_start, __param_end);
+}
+
 /**
  *cmdline_parse -- parses the xen command line.
  * If CONFIG_CMDLINE is set, it would be parsed prior to @cmdline.
diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index c0aa6bde4e..d8597de9cd 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -501,6 +501,51 @@ long do_sysctl(XEN_GUEST_HANDL

Re: [Xen-devel] [PATCH v1 1/4] xen: add hypercall for getting parameters at runtime

[Vasilis Liaskovitis]

thanks for the review.

On Wed, 2019-03-13 at 17:35 +0100, Jan Beulich wrote:
> > > > On 06.03.19 at 13:58,  wrote:
> > +static int get_params(const char *cmdline, char *values,
> > +  const struct kernel_param *start,
> > +  const struct kernel_param *end)
> > +{
> > +char opt[128], *optkey, *q;
> > +const char *p = cmdline, *val = values;
> 
> Why is val a pointer to const char? There are several casts below
> because of this, and all these casts should go away.

yes, it should be a pointer to char, and those casts are not needed, I
sent out an older version by mistake.

> 
> > +const struct kernel_param *param;
> > +int len, rc = 0;
> > +uint64_t param_int;
> > +bool found;
> > +
> > +if (!values)
> > +return -EFAULT;
> > +
> > +for ( ; ; )
> > +{
> > +/* Skip whitespace. */
> > +while ( *p == ' ' )
> > +p++;
> 
> For a user exposed interface I think it would be better to at least
> use isspace() here.

ok

> 
> > +found = false;
> 
> I don't think you need this variable here, or if so, it shouldn't
> be boolean: Either you mean to support returning data for
> multiple matching options (there are no runtime ones at
> present, but we at least used to have multiple-instance
> boot time ones), in which case you may need to invent a
> means to disambiguate them. Or you don't, in which case
> you could bail from the loop once you've found a match.

Multiple matching options was not my intention. Yes, the loop going
through the parameters should be exited on a match, but I still need to
detect if there was no parameter matched at all for the current option,
in order to return -EINVAL in that case. This can be detected from the
return value of strcmp, instead of using a boolean variable. 

> 
> > +for ( param = start; param < end; param++ )
> > +{
> > +
> > +if ( strcmp(param->name, optkey) )
> > +continue;
> > +
> > +found = true;
> > +switch ( param->type )
> > +{
> > +case OPT_STR:
> > +len = snprintf((char*)val, sizeof(values), "%s ",
> 
> Here and below, sizeof(values) is wrong in two ways: For one
> it's sizeof(char*), not sizeof(char[nn]). And then you need to
> subtract the amount of space consumed already.

thanks, a basic mistake. I think the caller will need to pass the size
of the values[] buffer as an argument to the function, or I can always
assume a XEN_PARAMETER_SIZE max size for the values[] buffer. The
former sounds better to me. 

> 
> > +   (char*)param->par.var);
> > +val += len;
> > +break;
> > +case OPT_UINT:
> > +get_integer_param(param, ¶m_int);
> > +len = snprintf((char*)val, sizeof(values), "%lu ",
> > param_int);
> 
> This is going to fail to build on 32-bit Arm. 

I will change the formatting to %"PRIu64"


> I'm also
> unconvinced this is appropriate if the value is actually
> a signed quantity.

isn't OPT_UINT only for unsigned integers?

> 
> 
> > +   param_int ? "true" : "false");
> > +val += len;
> > +break;
> > +case OPT_SIZE:
> > +case OPT_CUSTOM:
> > +rc = -EINVAL;
> > +break;
> 
> I can see why custom parameters can't be dealt with yet,
> but why also size ones? As to custom ones - four out of the
> seven runtime parameters we currently have are custom
> ones, so this limits the utility of the change quite a bit.

From what I understand OPT_SIZE can be handled similarly to OPT_UINT,
printing an unsigned integer in the output.

For OPT_CUSTOM, what about adding a get_func function pointer to their
definition in kernel_param, which would be used for reading the current
value? So the union par of struct kernel_param would now have a struct
with 2 function pointers for the custom parameters, something like:

union {
void *var;
struct {
int (*func)(const char *);
const char *(*get_func)(void);
} custom;
} par;


with the new get_func prototype being:

const char *(*get_func)(void);

returning a string with the current value of the parameter.

e.g. the function for loglvl, guest_loglvl could return output using
loglvl_str() :  "Nothing", "Errors", "Errors and warnings" etc.

An alternative prototype could be:

int (*get_func)(char *output);

if we want the function to write the current parameter value into a
caller-provided buffer, and possibly return error codes.  The first
option looks simpler to me.

The new custom function would have to be implemented of course for the
4 existing and any future custom parameters.

> 
> > +default:
> > +BUG();
> > +break;
> > +}
> > +}
> > +
> > +if ( !f

Re: [Xen-devel] [PATCH v1 1/4] xen: add hypercall for getting parameters at runtime

[Vasilis Liaskovitis]

On Mon, 2019-03-18 at 15:02 +0100, Jan Beulich wrote:
> > > > From the return value of strcmp()? I don't think so, because
> > > > you
> may have run past all table entries. Instead it's that property
> that you can use, i.e. checking whether ...
> 
> > > > +for ( param = start; param < end; param++ )
> 
> ... the loop end condition was hit.

right.

> 
> > > I'm also
> > > unconvinced this is appropriate if the value is actually
> > > a signed quantity.
> > 
> > isn't OPT_UINT only for unsigned integers?
> 
> You'll notice that there's no OPT_INT or OPT_SINT. OPT_UINT
> may be slightly misleading as a name. Otoh we probably don't
> have very many signed integer options.

I see. Assuming an unsigned parameter is problematic then.

Doesn't the .data.param section (between __param_start and __param_end)
only include runtime parameters, of which the integer ones are all
unsigned integers at the moment? Or would you like to see this
functionality eventually used for all parameters, including non-runtime 
as well?

> > 
> > with the new get_func prototype being:
> > 
> > const char *(*get_func)(void);
> > 
> > returning a string with the current value of the parameter.
> 
> And where would the storage live for the string?

I was thinking these custom functions would return static strings, as
loglvl_str() does, and these would eventually be copied into the user
provided values[] buffer via snprintf in get_params().

> 
> > e.g. the function for loglvl, guest_loglvl could return output
> > using
> > loglvl_str() :  "Nothing", "Errors", "Errors and warnings" etc.
> > 
> > An alternative prototype could be:
> > 
> > int (*get_func)(char *output);
> > 
> > if we want the function to write the current parameter value into a
> > caller-provided buffer, and possibly return error codes.
> 
> And how would the callee know how much space there is?

yes, an extra size argument would be needed from the caller here.

Let me know if either of these or a different approach is preferred.

> > 
> You certainly shouldn't access them from sysctl.c. You should
> simply make get_params() access them instead of what its
> only caller passes, at which point the function should be
> renamed to it only caller's name.

... of course.


thanks,

- Vasilis


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

Re: [Xen-devel] [PATCH v1 1/4] xen: add hypercall for getting parameters at runtime

[Vasilis Liaskovitis]

On Mon, 2019-03-18 at 18:01 +0100, Jan Beulich wrote:
> > > > An alternative prototype could be:
> > > > int (*get_func)(char *output);
> > > > if we want the function to write the current parameter value
> > > > into a
> > > > caller-provided buffer, and possibly return error codes.
> > > 
> > > And how would the callee know how much space there is?
> > 
> > yes, an extra size argument would be needed from the caller here.
> > 
> > Let me know if either of these or a different approach is
> > preferred.
> 
> Of the two, I clearly prefer the latter. Then again me having pointed
> out the shortcoming doesn't mean we (and hence you for this patch)
> absolutely have to deal with custom parameters. 

Unless there are objections, I think 'll send a v2 about getting non-
custom parameter types first, and deal with custom parameters in a
separate future patchset.

thanks,

- Vasilis


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

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

[Vasilis Liaskovitis]

Currently parameters of the hypervisor cannot be inspected at runtime
through an xl command. Such a command would be a useful diagnostic
tool e.g. used in conjunction with the "xl set-parameters" command.

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.

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.

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

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 |  15 
 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 | 102 
 xen/common/sysctl.c |  55 ++-
 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, 259 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 v2 1/4] xen: add hypercall for getting parameters at runtime

[Vasilis Liaskovitis]

Add a sysctl hypercall to support getting hypervisor parameters
at runtime.

Limitations:
- Custom runtime parameters (OPT_CUSTOM) are not supported yet.
- For integer parameters (OPT_UINT), only unsigned parameters are printed
correctly.

Signed-off-by: Vasilis Liaskovitis 
---
 tools/flask/policy/modules/dom0.te  |   2 +-
 xen/common/kernel.c | 110 
 xen/common/sysctl.c |  55 +-
 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, 188 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..2d12a5bcf6 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -12,6 +12,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -52,6 +53,115 @@ 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();
+}
+
+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;
+
+if (!values)
+return -EFAULT;
+
+for ( ; ; )
+{
+/* Skip whitespace. */
+while ( isspace(*p) )
+p++;
+if ( *p == '\0' )
+break;
+
+/* Grab the next whitespace-delimited option. */
+q = optkey = opt;
+while ( (*p != ' ') && (*p != '\0') )
+{
+if ( (q - opt) < (sizeof(opt)-1) ) /* avoid overflow */
+*q++ = *p;
+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:
+get_integer_param(param, ¶m_int);
+len = snprintf(val + bufpos, maxlen - bufpos,
+   "%"PRIu64" ", param_int);
+break;
+case OPT_BOOL:
+get_integer_param(param, ¶m_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 += (size_t) len;
+else if (len > 0)
+rc = -ENOMEM;
+
+break;
+}
+
+/* no parameter was matched */
+if ( param >= __param_end )
+{
+rc = -EINVAL;
+}
+
+if (rc)
+break;
+}
+
+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..1b65bd617e 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -466,9 +466,9 @@ long

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

[Vasilis Liaskovitis]

Add a new libxl function to get hypervisor parameters at runtime.

Signed-off-by: Vasilis Liaskovitis 
---
 tools/libxl/libxl.c | 15 +++
 tools/libxl/libxl.h |  1 +
 2 files changed, 16 insertions(+)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index ec71574e99..124033e5a3 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -669,6 +669,21 @@ int libxl_set_parameters(libxl_ctx *ctx, char *params)
 return 0;
 }
 
+int libxl_get_parameters(libxl_ctx *ctx, char *params, char *values)
+{
+int ret;
+GC_INIT(ctx);
+
+ret = xc_get_parameters(ctx->xch, params, values);
+if (ret < 0) {
+LOGEV(ERROR, ret, "getting parameters");
+GC_FREE;
+return ret;//ERROR_FAIL;
+}
+GC_FREE;
+return 0;
+}
+
 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 v2 2/4] libxc: add function to get hypervisor parameters

[Vasilis Liaskovitis]

Add a new libxc function to get hypervisor parameters at runtime.

Signed-off-by: Vasilis Liaskovitis 
---
 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 v2 4/4] xl: add new xl command get-parameters

[Vasilis Liaskovitis]

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

Signed-off-by: Vasilis Liaskovitis 
---
 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 to Xen. It is the same as pressing the 
Xen
 Set hypervisor parameters as specified in I. This allows for some
 boot parameters of the hypervisor to be modified in the running systems.
 
+=item B I
+
+Get hypervisor parameters as specified in I. This allows for some
+boot parameters of the hypervisor to be read in the running systems.
+
 =item B [I]
 
 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",
   " ",
 },
+{ "get-parameters",
+  &main_get_parameters, 0, 1,
+  "Get hypervisor parameters",
+  "",
+},
 };
 
 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 v2 1/4] xen: add hypercall for getting parameters at runtime

[Vasilis Liaskovitis]

Sorry for the delay, I was on a long vacation.

On Fri, 2019-04-05 at 17:01 +0200, Jan Beulich wrote:On 22.03.19 at
20:28,  wrote:
> > Limitations:
> > - Custom runtime parameters (OPT_CUSTOM) are not supported yet.
> > - For integer parameters (OPT_UINT), only unsigned parameters are
> > printed
> > correctly.
> 
> For this latter case I wonder whether it wouldn't be better to
> return back the raw binary value, allowing the caller to format
> it in suitable ways (e.g. both signed and unsigned, or dec and
> hex).

Currently the caller is oblivious to the parameters and their types,
and all retrieved values are printed together in a single string (see
patch 4/4). I'd like to keep it like that for simplicity. Otherwise I
think the caller has to find the types of requested parameters to
produce the right formatting. Actually, since OPT_UINT is used for both
signed and unsigned integer parameters, case-by-case parameter
formatting would be required to do this, and the libx* callers do not
have that knowledge. A "get_" per-parameter function pointer, which
would handle formatting for each parameter, be it int, uint, string or
custom, seems cleaner to me than leaving it to the caller.

By the way, the current implementation searches in [__param_start
__param_end), which are only the runtime parameters, not all
parameters, correct? So the series should be renamed to "Support for
reading runtime-only hypervisor parameters". The command is useful for
checking parameters that can be changed at runtime after all. 

I believe there are currently no signed integer runtime parameters. So
alternatively we could add a warning to the commit message and/or to
the code stating that signed integer runtime parameters, if added,
would not be printed correctly at the moment. This would gloss over
rather than solve the issue.

If correct formatting for all possible types is a requirement, the
get_function may be needed. Or we could separate integer from unsigned
integer parameters with an additional OPT_INT parameter type. I don't
know if either of these is desirable or simply overkill to implement
just for this command.

I 'll send a v3 when there is agreement on the above.

[...]

> > +{
> > +case OPT_STR:
> > +len = snprintf(val + bufpos, maxlen - bufpos, "%s
> > ",
> > +   (char*)param->par.var);
> 
> What you return here is the value as set into the buffer when the
> option was parsed, and before that string was actually consumed.
> Is this really what's interesting to the user? I'd rather expect them
> to be after what is actually in effect.
> 
> Since we've decided against a "get" per-option hook, I consider it
> acceptable this way as long as the behavior is spelled out amongst
> the limitations.
> 

I 'll add this limitation to the commit message in the next version.

thanks,
- Vasilis


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

[Xen-devel] [PATCH] xen/blkfront: avoid NULL blkfront_info dereference on device removal

[Vasilis Liaskovitis]

If a block device is hot-added when we are out of grants,
gnttab_grant_foreign_access fails with -ENOSPC (log message "28
granting access to ring page") in this code path:

  talk_to_blkback ->
setup_blkring ->
xenbus_grant_ring ->
gnttab_grant_foreign_access

and the failing path in talk_to_blkback sets the driver_data to NULL:

 destroy_blkring:
blkif_free(info, 0);

mutex_lock(&blkfront_mutex);
free_info(info);
mutex_unlock(&blkfront_mutex);

dev_set_drvdata(&dev->dev, NULL);

This results in a NULL pointer BUG when blkfront_remove and blkif_free
try to access the failing device's NULL struct blkfront_info.

Signed-off-by: Vasilis Liaskovitis 
---
 drivers/block/xen-blkfront.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 429d20131c7e..92cc6cb6b078 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -2493,6 +2493,9 @@ static int blkfront_remove(struct xenbus_device *xbdev)
 
dev_dbg(&xbdev->dev, "%s removed", xbdev->nodename);
 
+   if (!info)
+   goto out;
+
blkif_free(info, 0);
 
mutex_lock(&info->mutex);
@@ -2535,6 +2538,7 @@ static int blkfront_remove(struct xenbus_device *xbdev)
mutex_unlock(&bdev->bd_mutex);
bdput(bdev);
 
+out:
return 0;
 }
 
-- 
2.19.0


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

Re: [Xen-devel] [PATCH] xen/blkfront: avoid NULL blkfront_info dereference on device removal

[Vasilis Liaskovitis]

On Thu, 2018-10-11 at 18:34 +0200, Roger Pau Monné wrote:
> --- a/drivers/block/xen-blkfront.c
> > +++ b/drivers/block/xen-blkfront.c
> > @@ -2493,6 +2493,9 @@ static int blkfront_remove(struct
> > xenbus_device *xbdev)
> >  
> > dev_dbg(&xbdev->dev, "%s removed", xbdev->nodename);
> >  
> > +   if (!info)
> > +   goto out;
> 
> I don't see the point in adding the 'out' label. Can you just return
> 0
> here?

of course, I will send a second version.
 
thanks for the review,

- Vasilis

> 
> Thanks, Roger.
> 
> ___
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel


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

[Xen-devel] [PATCH v2] xen/blkfront: avoid NULL blkfront_info dereference on device removal

[Vasilis Liaskovitis]

If a block device is hot-added when we are out of grants,
gnttab_grant_foreign_access fails with -ENOSPC (log message "28
granting access to ring page") in this code path:

  talk_to_blkback ->
setup_blkring ->
xenbus_grant_ring ->
gnttab_grant_foreign_access

and the failing path in talk_to_blkback sets the driver_data to NULL:

 destroy_blkring:
blkif_free(info, 0);

mutex_lock(&blkfront_mutex);
free_info(info);
mutex_unlock(&blkfront_mutex);

dev_set_drvdata(&dev->dev, NULL);

This results in a NULL pointer BUG when blkfront_remove and blkif_free
try to access the failing device's NULL struct blkfront_info.

Signed-off-by: Vasilis Liaskovitis 
---
 drivers/block/xen-blkfront.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 429d20131c7e..8871bf4c8a2e 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -2493,6 +2493,9 @@ static int blkfront_remove(struct xenbus_device *xbdev)
 
dev_dbg(&xbdev->dev, "%s removed", xbdev->nodename);
 
+   if (!info)
+   return 0;
+
blkif_free(info, 0);
 
mutex_lock(&info->mutex);
-- 
2.19.0


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

Re: [Xen-devel] [PATCH v2] xen/blkfront: avoid NULL blkfront_info dereference on device removal

[Vasilis Liaskovitis]

On Mon, 2018-10-15 at 16:02 +0200, Roger Pau Monné wrote:
> 
> > This results in a NULL pointer BUG when blkfront_remove and
> > blkif_free
> > try to access the failing device's NULL struct blkfront_info.
> > 
> > 
> I guess this is a candidate for backporting?
> 

yes, I think so. At least for kernels >=4.5, which could face this
issue due to commit c31ecf6c12 (this frees the struct blkinfo in the
failing path of talk_to_blkback).

thanks, 

- Vasilis



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