Re: [Xen-devel] [PATCH v2 2/5] xen/console: allow log level threshold adjustments

[Jan Beulich]

>>> On 04.07.16 at 17:13,  wrote:

In case this (or a variant thereof) is to be used despite the earlier
voiced concerns, a couple of mechanical comments:

> +static const char *loglvl_to_str(int lvl)
> +{
> +unsigned int i;
> +
> +if ( lvl < LOG_LEVEL_MIN || lvl > LOG_LEVEL_MAX )
> +return "???";
> +
> +/* Multiple log levels can use the same number. Return the most
> + * comprehensive log level string.
> + */

Comment style.

> +for ( i = ARRAY_SIZE(log_levels) - 1; i >= 0; i-- )

Possibly infinite loop - i is unsigned and hence always non-negative.

> +int console_loglvl_op(struct xen_sysctl_loglvl_op *op)
> +{
> +int ret;
> +
> +switch ( op->cmd )
> +{
> +default:
> +ret = -EOPNOTSUPP;
> +break;
> +
> +case XEN_SYSCTL_LOGLVL_set:
> +{
> +char *buf;
> +unsigned int buf_size = 0;

Pointless initializer.

> +int host_lower_thresh, host_upper_thresh;
> +int guest_lower_thresh, guest_upper_thresh;
> +
> +buf_size = op->host.lower_thresh_len;
> +if ( buf_size < op->host.upper_thresh_len )
> +buf_size = op->host.upper_thresh_len;
> +if ( buf_size < op->guest.lower_thresh_len )
> +buf_size = op->guest.lower_thresh_len;
> +if ( buf_size < op->guest.upper_thresh_len )
> +buf_size = op->guest.upper_thresh_len;
> +
> +buf = xmalloc_array(char, buf_size);
> +if ( !buf )
> +{
> +ret = -ENOMEM;
> +goto set_done;
> +}
> +
> +#define parse(hg, lu)   \
> +hg##_##lu##_thresh = -1;\
> +if ( op->hg.lu ##_thresh_len )  \
> +{   \
> +if ( copy_from_guest(buf, op->hg.lu ##_thresh,  \
> + op->hg.lu ##_thresh_len) ) \
> +{   \
> +ret = -EFAULT;  \
> +goto set_done;  \
> +}   \
> +\
> +buf[buf_size-1] = 0;\
> +\
> +ret = parse_log_level(buf); \
> +if ( ret == -1 )\
> +{   \
> +ret = -EINVAL;  \
> +goto set_done;  \
> +}   \
> +\
> +hg##_##lu##_thresh = ret;   \
> +}
> +
> +parse(host,  lower);
> +parse(host,  upper);
> +parse(guest, lower);
> +parse(guest, upper);
> +
> +#undef parse
> +
> +if ( (host_lower_thresh >= 0 && host_upper_thresh >= 0 &&
> +  host_lower_thresh > host_upper_thresh) ||
> + (guest_lower_thresh >= 0 && guest_upper_thresh >= 0 &&
> +  guest_lower_thresh > guest_upper_thresh) )
> +{
> +ret = -EINVAL;
> +goto set_done;
> +}
> +
> +do_loglvl_op(host_lower_thresh, host_upper_thresh,
> + _lower_thresh, _upper_thresh,
> + "standard");
> +
> +do_loglvl_op(guest_lower_thresh, guest_upper_thresh,
> + _guest_lower_thresh, _guest_upper_thresh,
> + "guest");
> +
> +ret = 0;
> +set_done:
> +xfree(buf);
> +break;
> +}
> +case XEN_SYSCTL_LOGLVL_get:

Blank line ahead of the case label please.

> +{
> +unsigned int host_lower_thresh_len =
> +strlen(loglvl_to_str(xenlog_lower_thresh)) + 1;
> +unsigned int host_upper_thresh_len =
> +strlen(loglvl_to_str(xenlog_upper_thresh)) + 1;
> +unsigned int guest_lower_thresh_len =
> +strlen(loglvl_to_str(xenlog_guest_lower_thresh)) + 1;
> +unsigned int guest_upper_thresh_len =
> +strlen(loglvl_to_str(xenlog_guest_upper_thresh)) + 1;
> +char scratch[LOG_LEVEL_STRLEN_MAX+1];
> +
> +if ( (op->host.lower_thresh_len &&
> +  op->host.lower_thresh_len < host_lower_thresh_len) ||
> + (op->host.upper_thresh_len &&
> +  op->host.upper_thresh_len < host_upper_thresh_len) ||
> + 

Re: [Xen-devel] [PATCH v2 2/5] xen/console: allow log level threshold adjustments

[Ian Jackson]

Wei Liu writes ("[PATCH v2 2/5] xen/console: allow log level threshold 
adjustments"):
> ... from serial console and via sysctl so that one doesn't always need
> to reboot to see more / less messages.
> 
> Note that upper thresholds driven from the serial console are sticky,
> i.e. while they get adjusted upwards when the lower threshold would
> otherwise end up above the upper one, they don't get adjusted when
> reducing the lower one. Full flexibility is available only via the
> sysctl interface.
> 
> Signed-off-by: Jan Beulich 
> 
> Rework the sysctl interface to pass in / out strings directly. Provide
> some helper functions to transform from log level numbers to strings and
> vice verse. Lower and upper bounds are checked. Add XSM hook.

Is this really the best way to do this ?  This is an awful lot of
hypervisor code.  Perhaps instead there should be a way to request the
table of level strings, and userspace could do the conversion ?

Or, given that this is a sysctl, so does not need a stable ABI, the
table of loglevel strings could be provided in the .h file in the form
of a suitable lifted list #define, and at runtime be implicit in the
ABI version.

Ian.

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

Re: [Xen-devel] [PATCH v2 2/5] xen/console: allow log level threshold adjustments

[Daniel De Graaf]

On 07/04/2016 11:13 AM, Wei Liu wrote:

... from serial console and via sysctl so that one doesn't always need
to reboot to see more / less messages.

Note that upper thresholds driven from the serial console are sticky,
i.e. while they get adjusted upwards when the lower threshold would
otherwise end up above the upper one, they don't get adjusted when
reducing the lower one. Full flexibility is available only via the
sysctl interface.

Signed-off-by: Jan Beulich 

Rework the sysctl interface to pass in / out strings directly. Provide
some helper functions to transform from log level numbers to strings and
vice verse. Lower and upper bounds are checked. Add XSM hook.

Signed-off-by: Wei Liu 


Acked-by: Daniel De Graaf 

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

[Xen-devel] [PATCH v2 2/5] xen/console: allow log level threshold adjustments

[Wei Liu]

... from serial console and via sysctl so that one doesn't always need
to reboot to see more / less messages.

Note that upper thresholds driven from the serial console are sticky,
i.e. while they get adjusted upwards when the lower threshold would
otherwise end up above the upper one, they don't get adjusted when
reducing the lower one. Full flexibility is available only via the
sysctl interface.

Signed-off-by: Jan Beulich 

Rework the sysctl interface to pass in / out strings directly. Provide
some helper functions to transform from log level numbers to strings and
vice verse. Lower and upper bounds are checked. Add XSM hook.

Signed-off-by: Wei Liu 
---
Cc: Jan Beulich 
Cc: Daniel De GRaaf 
Cc: Ian Jackson 
---
 tools/flask/policy/modules/dom0.te  |   2 +-
 xen/common/sysctl.c |   5 +
 xen/drivers/char/console.c  | 291 +++-
 xen/include/public/sysctl.h |  41 +
 xen/include/xen/console.h   |   2 +
 xen/xsm/flask/hooks.c   |   3 +
 xen/xsm/flask/policy/access_vectors |   2 +
 7 files changed, 343 insertions(+), 3 deletions(-)

diff --git a/tools/flask/policy/modules/dom0.te 
b/tools/flask/policy/modules/dom0.te
index 2d982d9..4e45e10 100644
--- a/tools/flask/policy/modules/dom0.te
+++ b/tools/flask/policy/modules/dom0.te
@@ -15,7 +15,7 @@ allow dom0_t xen_t:xen {
 };
 allow dom0_t xen_t:xen2 {
resource_op psr_cmt_op psr_cat_op pmu_ctrl get_symbol
-   get_cpu_levelling_caps get_cpu_featureset livepatch_op
+   get_cpu_levelling_caps get_cpu_featureset livepatch_op loglvl
 };
 
 # Allow dom0 to use all XENVER_ subops that have checks.
diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index 55f2077..cf8fa68 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -467,6 +467,11 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) 
u_sysctl)
 copyback = 1;
 break;
 
+case XEN_SYSCTL_loglvl_op:
+ret = console_loglvl_op(>u.loglvl);
+copyback = 1;
+break;
+
 default:
 ret = arch_do_sysctl(op, u_sysctl);
 copyback = 0;
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 6620a1c..df847c4 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -144,7 +144,7 @@ struct log_level {
 unsigned int num;
 };
 
-static struct log_level __initdata log_levels[] = {
+static struct log_level log_levels[] = {
 { "none",0 },
 { "error",   1 },
 { "warning", 2 },
@@ -152,6 +152,9 @@ static struct log_level __initdata log_levels[] = {
 { "debug",   4 },
 { "all", 4 },
 };
+#define LOG_LEVEL_MIN 0
+#define LOG_LEVEL_MAX 4
+#define LOG_LEVEL_STRLEN_MAX 7  /* "warning" is the longest one */
 
 #define ___parse_loglvl(s, ps, lvlstr, lvlnum)  \
 if ( !strncmp((s), (lvlstr), strlen(lvlstr)) ) {\
@@ -188,7 +191,41 @@ static void __init parse_guest_loglvl(char *s)
 _parse_loglvl(s, _guest_lower_thresh, _guest_upper_thresh);
 }
 
-static char * __init loglvl_str(int lvl)
+/*
+ * A variant used to parse log level during runtime. Returns -1 if it
+ * fails to parse.
+ */
+static int parse_log_level(const char *s)
+{
+unsigned int i;
+
+for ( i = 0; i < ARRAY_SIZE(log_levels); i++ )
+{
+if ( !strcmp(log_levels[i].str, s) )
+return log_levels[i].num;
+}
+
+return -1;
+}
+
+static const char *loglvl_to_str(int lvl)
+{
+unsigned int i;
+
+if ( lvl < LOG_LEVEL_MIN || lvl > LOG_LEVEL_MAX )
+return "???";
+
+/* Multiple log levels can use the same number. Return the most
+ * comprehensive log level string.
+ */
+for ( i = ARRAY_SIZE(log_levels) - 1; i >= 0; i-- )
+{
+if ( log_levels[i].num == lvl )
+return log_levels[i].str;
+}
+}
+
+static char *loglvl_str(int lvl)
 {
 switch ( lvl )
 {
@@ -201,6 +238,250 @@ static char * __init loglvl_str(int lvl)
 return "???";
 }
 
+static int *__read_mostly upper_thresh_adj = _upper_thresh;
+static int *__read_mostly lower_thresh_adj = _lower_thresh;
+static const char *__read_mostly thresh_adj = "standard";
+
+static void do_toggle_guest(unsigned char key, struct cpu_user_regs *regs)
+{
+if ( upper_thresh_adj == _upper_thresh )
+{
+upper_thresh_adj = _guest_upper_thresh;
+lower_thresh_adj = _guest_lower_thresh;
+thresh_adj = "guest";
+}
+else
+{
+upper_thresh_adj = _upper_thresh;
+lower_thresh_adj = _lower_thresh;
+thresh_adj = "standard";
+}
+printk("'%c' pressed -> %s log level adjustments enabled\n",
+   key, thresh_adj);
+}
+
+static void do_adj_thresh(unsigned char key)
+{
+if ( *upper_thresh_adj < *lower_thresh_adj )
+*upper_thresh_adj = *lower_thresh_adj;
+printk("'%c' pressed -> %s log level: %s (rate limited