[PATCH v2 00/52] Support for modifying parameters at runtime

[PATCH v2 00/52] Support for modifying parameters at runtime

[Juergen Gross]

Currently parameters of the hypervisor (e.g. console log level) can be
set via boot command line. Instead of having to reboot the system in
case another setting is desired, being able to modify many of those
parameters at runtime would be the better option.

This patch series addresses this by adding a new xl command
"xl set-parameters" which takes a string similar to the boot command
line as parameter and passes this string to the hypervisor which will
then use the same parsing infrastructure as for the command line in
order to apply the parameter settings.

As error checks for invalid parameters or parameter values have been
very sparse if present at all in the hypervisor, a major part of this
patch series addresses this problem first: all custom parameter parsing
functions are being changed to return success or an error. The main
parsing function tests for generic parameter value errors (like e.g.
overflow) or invalid parameters and issues a message in case an error
has been detected. Most error messages in the custom parsing functions
are removed then.

While not strictly required for runtime parameter modification I
believe an improved parameter validation is a win with or without the
runtime parameter modification support.

* Patches 1-38 are modifying the custom parameter parsing functions to
  return success or error
* Patch 39 adds generic parameter error tests in the core parameter
  parsing functions
* Patches 40-45 remove custom parsing function error messages
* Patches 46-51 add the runtime parameter modification support
* Patch 52 adds support for runtime modification of some console related
  parameters 

Changes in V2:
- patch 39 (xen: check parameter validity when parsing command line):
  replaced literal 8 by BITS_PER_BYTE (Wei Liu)
  added test for empty string to parse_bool()
- patch 47 (xen: add basic support for runtime parameter changing):
  added modification of ARM linker script (Wei Liu)
- patch 48 (xen: add hypercall for setting parameters at runtime):
  corrected XSM test (Daniel De Graaf)
- patch 50 (libxl: add libxl_set_parameters() function):
  corrected coding style (Wei Liu)

  
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
Cc: George Dunlap <george.dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Jun Nakajima <jun.nakajima@intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Paul Durrant <paul.durrant@citrix.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>

Juergen Gross (52):
  xen/arch/arm/acpi/boot.c: let custom parameter parsing routines return
    errno
  xen/arch/arm/domain_build.c: let custom parameter parsing routines
    return errno
  xen/arch/arm/traps.c: let custom parameter parsing routines return
    errno
  xen/arch/x86/apic.c: let custom parameter parsing routines return
    errno
  xen/arch/x86/cpu/mcheck/mce.c: let custom parameter parsing routines
    return errno
  xen/arch/x86/cpu/vpmu.c: let custom parameter parsing routines return
    errno
  xen/arch/x86/dom0_build.c: let custom parameter parsing routines
    return errno
  xen/arch/x86/genapic/probe.c: let custom parameter parsing routines
    return errno
  xen/arch/x86/hvm/viridian.c: let custom parameter parsing routines
    return errno
  xen/arch/x86/hvm/vmx/vmcs.c: let custom parameter parsing routines
    return errno
  xen/arch/x86/io_apic.c: let custom parameter parsing routines return
    errno
  xen/arch/x86/irq.c: let custom parameter parsing routines return errno
  xen/arch/x86/microcode.c: let custom parameter parsing routines return
    errno
  xen/arch/x86/mm.c: let custom parameter parsing routines return errno
  xen/arch/x86/nmi.c: let custom parameter parsing routines return errno
  xen/arch/x86/numa.c: let custom parameter parsing routines return
    errno
  xen/arch/x86/oprofile/nmi_int.c: let custom parameter parsing routines
    return errno
  xen/arch/x86/psr.c: let custom parameter parsing routines return errno
  xen/arch/x86/setup.c: let custom parameter parsing routines return
    errno
  xen/arch/x86/shutdown.c: let custom parameter parsing routines return
    errno
  xen/arch/x86/time.c: let custom parameter parsing routines return
    errno
  xen/arch/x86/x86_64/mmconfig-shared.c: let custom parameter parsing
    routines return errno
  xen/common/core_parking.c: let custom parameter parsing routines
    return errno
  xen/common/domain.c: let custom parameter parsing routines return
    errno
  xen/common/efi/boot.c: let custom parameter parsing routines return
    errno
  xen/common/kexec.c: let custom parameter parsing routines return errno
  xen/common/memory.c: let custom parameter parsing routines return
    errno
  xen/common/sched_credit2.c: let custom parameter parsing routines
    return errno
  xen/drivers/acpi/tables.c: let custom parameter parsing routines
    return errno
  xen/drivers/char/console.c: let custom parameter parsing routines
    return errno
  xen/drivers/cpufreq/cpufreq.c: let custom parameter parsing routines
    return errno
  xen/drivers/passthrough/amd/iommu_acpi.c: let custom parameter parsing
    routines return errno
  xen/drivers/passthrough/iommu.c: let custom parameter parsing routines
    return errno
  xen/drivers/passthrough/pci.c: let custom parameter parsing routines
    return errno
  xen/drivers/passthrough/vtd/dmar.c: let custom parameter parsing
    routines return errno
  xen/drivers/passthrough/vtd/quirks.c: let custom parameter parsing
    routines return errno
  xen/drivers/video/vesa.c: let custom parameter parsing routines return
    errno
  xen/xsm/flask/flask_op.c: let custom parameter parsing routines return
    errno
  xen: check parameter validity when parsing command line
  xen/arch/x86/apic.c: remove custom_param() error messages
  xen/arch/x86/cpu/mcheck/mce.c: remove custom_param() error messages
  xen/arch/x86/hvm/viridian.c: remove custom_param() error messages
  xen/arch/x86/io_apic.c: remove custom_param() error messages
  xen/common/kexec.c: remove custom_param() error messages
  xen/common/sched_credit2.c: remove custom_param() error messages
  xen: carve out a generic parsing function from _cmdline_parse()
  xen: add basic support for runtime parameter changing
  xen: add hypercall for setting parameters at runtime
  libxc: add function to set hypervisor parameters
  libxl: add libxl_set_parameters() function
  xl: add new xl command set-parameters
  xen: make some console related parameters settable at runtime

 docs/man/xl.pod.1.in                     |  5 +++
 docs/misc/xen-command-line.markdown      |  8 ++++
 tools/flask/policy/modules/dom0.te       |  2 +-
 tools/libxc/include/xenctrl.h            |  1 +
 tools/libxc/xc_misc.c                    | 20 +++++++++
 tools/libxl/libxl.c                      | 15 +++++++
 tools/libxl/libxl.h                      |  8 ++++
 tools/xl/xl.h                            |  1 +
 tools/xl/xl_cmdtable.c                   |  5 +++
 tools/xl/xl_misc.c                       | 20 +++++++++
 xen/arch/arm/acpi/boot.c                 |  8 +++-
 xen/arch/arm/domain_build.c              |  4 +-
 xen/arch/arm/traps.c                     |  8 ++--
 xen/arch/arm/xen.lds.S                   |  4 ++
 xen/arch/x86/apic.c                      | 10 +++--
 xen/arch/x86/cpu/mcheck/mce.c            |  7 +--
 xen/arch/x86/cpu/vpmu.c                  |  7 +--
 xen/arch/x86/dom0_build.c                | 19 +++++++--
 xen/arch/x86/genapic/probe.c             |  8 +++-
 xen/arch/x86/hvm/viridian.c              | 13 +++---
 xen/arch/x86/hvm/vmx/vmcs.c              |  7 ++-
 xen/arch/x86/io_apic.c                   |  6 ++-
 xen/arch/x86/irq.c                       |  9 +++-
 xen/arch/x86/microcode.c                 | 10 +++--
 xen/arch/x86/mm.c                        |  7 ++-
 xen/arch/x86/nmi.c                       | 20 ++++++---
 xen/arch/x86/numa.c                      | 10 +++--
 xen/arch/x86/oprofile/nmi_int.c          |  2 +
 xen/arch/x86/psr.c                       | 28 +++++++++---
 xen/arch/x86/setup.c                     | 32 +++++++++-----
 xen/arch/x86/shutdown.c                  |  6 ++-
 xen/arch/x86/time.c                      |  6 ++-
 xen/arch/x86/x86_64/mmconfig-shared.c    | 19 +++++++--
 xen/arch/x86/xen.lds.S                   |  4 ++
 xen/common/core_parking.c                |  6 ++-
 xen/common/domain.c                      |  4 +-
 xen/common/efi/boot.c                    |  6 ++-
 xen/common/kernel.c                      | 73 +++++++++++++++++++++++++-------
 xen/common/kexec.c                       | 31 +++++++++++---
 xen/common/memory.c                      |  4 +-
 xen/common/sched_credit2.c               |  6 +--
 xen/common/sysctl.c                      | 29 +++++++++++++
 xen/drivers/acpi/tables.c                |  5 ++-
 xen/drivers/char/console.c               | 35 +++++++++------
 xen/drivers/cpufreq/cpufreq.c            | 18 +++++---
 xen/drivers/passthrough/amd/iommu_acpi.c | 26 ++++++++----
 xen/drivers/passthrough/iommu.c          | 15 ++++---
 xen/drivers/passthrough/pci.c            | 17 +++++---
 xen/drivers/passthrough/vtd/dmar.c       |  4 +-
 xen/drivers/passthrough/vtd/quirks.c     |  7 +--
 xen/drivers/video/vesa.c                 |  4 +-
 xen/include/acpi/cpufreq/cpufreq.h       |  2 +-
 xen/include/public/sysctl.h              | 19 +++++++++
 xen/include/xen/init.h                   | 22 +++++++++-
 xen/include/xen/lib.h                    |  1 +
 xen/xsm/flask/flask_op.c                 |  6 ++-
 xen/xsm/flask/hooks.c                    |  3 ++
 xen/xsm/flask/policy/access_vectors      |  2 +
 58 files changed, 533 insertions(+), 151 deletions(-)

-- 
2.12.3


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

[PATCH v2 01/52] xen/arch/arm/acpi/boot.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/arm/acpi/boot.c

to indicate whether the parameter value was parsed successfully.

Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/arm/acpi/boot.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/acpi/boot.c b/xen/arch/arm/acpi/boot.c
index 889208a0ea..0b90cf3a15 100644
--- a/xen/arch/arm/acpi/boot.c
+++ b/xen/arch/arm/acpi/boot.c
@@ -193,16 +193,20 @@ static int __init acpi_parse_fadt(struct acpi_table_header *table)
 static bool_t __initdata param_acpi_off;
 static bool_t __initdata param_acpi_force;
 
-static void __init parse_acpi_param(char *arg)
+static int __init parse_acpi_param(char *arg)
 {
     if ( !arg )
-        return;
+        return -EINVAL;
 
     /* Interpret the parameter for use within Xen. */
     if ( !parse_bool(arg) )
         param_acpi_off = true;
     else if ( !strcmp(arg, "force") ) /* force ACPI to be enabled */
         param_acpi_force = true;
+    else
+        return -EINVAL;
+
+    return 0;
 }
 custom_param("acpi", parse_acpi_param);
 
-- 
2.12.3


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

[PATCH v2 02/52] xen/arch/arm/domain_build.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/arm/domain_build.c

to indicate whether the parameter value was parsed successfully.

Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/arm/domain_build.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 1bec4fa23d..d6f9585503 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -33,9 +33,11 @@ int dom0_11_mapping = 1;
 
 static u64 __initdata dom0_mem;
 
-static void __init parse_dom0_mem(const char *s)
+static int __init parse_dom0_mem(const char *s)
 {
     dom0_mem = parse_size_and_unit(s, &s);
+
+    return *s ? -EINVAL : 0;
 }
 custom_param("dom0_mem", parse_dom0_mem);
 
-- 
2.12.3


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

[PATCH v2 03/52] xen/arch/arm/traps.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/arm/traps.c

to indicate whether the parameter value was parsed successfully.

Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/arm/traps.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index c07999b518..f94ca1af02 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -108,12 +108,14 @@ static enum {
 	NATIVE,
 } vwfi;
 
-static void __init parse_vwfi(const char *s)
+static int __init parse_vwfi(const char *s)
 {
 	if ( !strcmp(s, "native") )
 		vwfi = NATIVE;
 	else
 		vwfi = TRAP;
+
+	return 0;
 }
 custom_param("vwfi", parse_vwfi);
 
@@ -130,7 +132,7 @@ static enum {
     SERRORS_PANIC,
 } serrors_op;
 
-static void __init parse_serrors_behavior(const char *str)
+static int __init parse_serrors_behavior(const char *str)
 {
     if ( !strcmp(str, "forward") )
         serrors_op = SERRORS_FORWARD;
@@ -139,7 +141,7 @@ static void __init parse_serrors_behavior(const char *str)
     else
         serrors_op = SERRORS_DIVERSE;
 
-    return;
+    return 0;
 }
 custom_param("serrors", parse_serrors_behavior);
 
-- 
2.12.3


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

[PATCH v2 04/52] xen/arch/x86/apic.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/apic.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/apic.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c
index 851a6cc6cb..fdb0a99927 100644
--- a/xen/arch/x86/apic.c
+++ b/xen/arch/x86/apic.c
@@ -785,23 +785,28 @@ int lapic_resume(void)
  * Original code written by Keir Fraser.
  */
 
-static void __init lapic_disable(char *str)
+static int __init lapic_disable(char *str)
 {
     enable_local_apic = -1;
     setup_clear_cpu_cap(X86_FEATURE_APIC);
+    return 0;
 }
 custom_param("nolapic", lapic_disable);
 boolean_param("lapic", enable_local_apic);
 
-static void __init apic_set_verbosity(char *str)
+static int __init apic_set_verbosity(char *str)
 {
     if (strcmp("debug", str) == 0)
         apic_verbosity = APIC_DEBUG;
     else if (strcmp("verbose", str) == 0)
         apic_verbosity = APIC_VERBOSE;
-    else
+    else {
         printk(KERN_WARNING "APIC Verbosity level %s not recognised"
                " use apic_verbosity=verbose or apic_verbosity=debug", str);
+        return -EINVAL;
+    }
+
+    return 0;
 }
 custom_param("apic_verbosity", apic_set_verbosity);
 
-- 
2.12.3


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

[PATCH v2 05/52] xen/arch/x86/cpu/mcheck/mce.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/cpu/mcheck/mce.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/cpu/mcheck/mce.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
index 30525dd78b..e577a9db11 100644
--- a/xen/arch/x86/cpu/mcheck/mce.c
+++ b/xen/arch/x86/cpu/mcheck/mce.c
@@ -62,13 +62,18 @@ struct mca_banks *mca_allbanks;
 #endif
 
 int mce_verbosity;
-static void __init mce_set_verbosity(char *str)
+static int __init mce_set_verbosity(char *str)
 {
     if (strcmp("verbose", str) == 0)
         mce_verbosity = MCE_VERBOSE;
     else
+    {
         printk(KERN_DEBUG "Machine Check verbosity level %s not recognised"
                "use mce_verbosity=verbose", str);
+        return -EINVAL;
+    }
+
+    return 0;
 }
 custom_param("mce_verbosity", mce_set_verbosity);
 
-- 
2.12.3


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

[PATCH v2 06/52] xen/arch/x86/cpu/vpmu.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/cpu/vpmu.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/cpu/vpmu.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c
index 90954ca884..8f56e2bb6c 100644
--- a/xen/arch/x86/cpu/vpmu.c
+++ b/xen/arch/x86/cpu/vpmu.c
@@ -53,7 +53,7 @@ CHECK_pmu_params;
 static unsigned int __read_mostly opt_vpmu_enabled;
 unsigned int __read_mostly vpmu_mode = XENPMU_MODE_OFF;
 unsigned int __read_mostly vpmu_features = 0;
-static void parse_vpmu_params(char *s);
+static int parse_vpmu_params(char *s);
 custom_param("vpmu", parse_vpmu_params);
 
 static DEFINE_SPINLOCK(vpmu_lock);
@@ -76,7 +76,7 @@ static int parse_vpmu_param(char *s, unsigned int len)
     return 0;
 }
 
-static void __init parse_vpmu_params(char *s)
+static int __init parse_vpmu_params(char *s)
 {
     char *sep, *p = s;
 
@@ -104,10 +104,11 @@ static void __init parse_vpmu_params(char *s)
         opt_vpmu_enabled = 1;
         break;
     }
-    return;
+    return 0;
 
  error:
     printk("VPMU: unknown flags: %s - vpmu disabled!\n", s);
+    return -EINVAL;
 }
 
 void vpmu_lvtpc_update(uint32_t val)
-- 
2.12.3


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

[PATCH v2 07/52] xen/arch/x86/dom0_build.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/dom0_build.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/dom0_build.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
index 0c125e61eb..21eb640a48 100644
--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -47,7 +47,7 @@ static long __init parse_amt(const char *s, const char **ps)
     long pages = parse_size_and_unit((*s == '-') ? s+1 : s, ps) >> PAGE_SHIFT;
     return (*s == '-') ? -pages : pages;
 }
-static void __init parse_dom0_mem(const char *s)
+static int __init parse_dom0_mem(const char *s)
 {
     do {
         if ( !strncmp(s, "min:", 4) )
@@ -57,13 +57,15 @@ static void __init parse_dom0_mem(const char *s)
         else
             dom0_nrpages = parse_amt(s, &s);
     } while ( *s++ == ',' );
+
+    return *s ? -EINVAL : 0;
 }
 custom_param("dom0_mem", parse_dom0_mem);
 
 static unsigned int __initdata opt_dom0_max_vcpus_min = 1;
 static unsigned int __initdata opt_dom0_max_vcpus_max = UINT_MAX;
 
-static void __init parse_dom0_max_vcpus(const char *s)
+static int __init parse_dom0_max_vcpus(const char *s)
 {
     if ( *s == '-' )                   /* -M */
         opt_dom0_max_vcpus_max = simple_strtoul(s + 1, &s, 0);
@@ -77,6 +79,8 @@ static void __init parse_dom0_max_vcpus(const char *s)
         else if ( *s++ == '-' && *s ) /* N-M */
             opt_dom0_max_vcpus_max = simple_strtoul(s, &s, 0);
     }
+
+    return *s ? -EINVAL : 0;
 }
 custom_param("dom0_max_vcpus", parse_dom0_max_vcpus);
 
@@ -85,7 +89,7 @@ static __initdata unsigned int dom0_pxms[MAX_NUMNODES] =
     { [0 ... MAX_NUMNODES - 1] = ~0 };
 static __initdata bool dom0_affinity_relaxed;
 
-static void __init parse_dom0_nodes(const char *s)
+static int __init parse_dom0_nodes(const char *s)
 {
     do {
         if ( isdigit(*s) )
@@ -103,6 +107,8 @@ static void __init parse_dom0_nodes(const char *s)
         else
             break;
     } while ( ++dom0_nr_pxms < ARRAY_SIZE(dom0_pxms) && *s++ == ',' );
+
+    return *s ? -EINVAL : 0;
 }
 custom_param("dom0_nodes", parse_dom0_nodes);
 
@@ -183,9 +189,10 @@ bool __initdata dom0_pvh;
  *  - pvh               Create a PVHv2 Dom0.
  *  - shadow            Use shadow paging for Dom0.
  */
-static void __init parse_dom0_param(char *s)
+static int __init parse_dom0_param(char *s)
 {
     char *ss;
+    int rc = 0;
 
     do {
 
@@ -199,9 +206,13 @@ static void __init parse_dom0_param(char *s)
         else if ( !strcmp(s, "shadow") )
             opt_dom0_shadow = true;
 #endif
+        else
+            rc = -EINVAL;
 
         s = ss + 1;
     } while ( ss );
+
+    return rc;
 }
 custom_param("dom0", parse_dom0_param);
 
-- 
2.12.3


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

[PATCH v2 08/52] xen/arch/x86/genapic/probe.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/genapic/probe.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/genapic/probe.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/genapic/probe.c b/xen/arch/x86/genapic/probe.c
index 9a147ff64a..c3a3b85cd6 100644
--- a/xen/arch/x86/genapic/probe.c
+++ b/xen/arch/x86/genapic/probe.c
@@ -44,12 +44,16 @@ void __init generic_bigsmp_probe(void)
 		}
 }
 
-static void __init genapic_apic_force(char *str)
+static int __init genapic_apic_force(char *str)
 {
 	int i;
 	for (i = 0; apic_probe[i]; i++)
-		if (!strcmp(apic_probe[i]->name, str))
+		if (!strcmp(apic_probe[i]->name, str)) {
 			genapic = apic_probe[i];
+			return 0;
+		}
+
+	return -EINVAL;
 }
 custom_param("apic", genapic_apic_force);
 
-- 
2.12.3


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

[PATCH v2 09/52] xen/arch/x86/hvm/viridian.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/hvm/viridian.c

to indicate whether the parameter value was parsed successfully.

Cc: Paul Durrant <paul.durrant@citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Paul Durrant <paul.durrant@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/hvm/viridian.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/hvm/viridian.c b/xen/arch/x86/hvm/viridian.c
index aa9b87c0ab..6f012bcb62 100644
--- a/xen/arch/x86/hvm/viridian.c
+++ b/xen/arch/x86/hvm/viridian.c
@@ -1083,7 +1083,7 @@ static int viridian_load_vcpu_ctxt(struct domain *d, hvm_domain_context_t *h)
 HVM_REGISTER_SAVE_RESTORE(VIRIDIAN_VCPU, viridian_save_vcpu_ctxt,
                           viridian_load_vcpu_ctxt, 1, HVMSR_PER_VCPU);
 
-static void __init parse_viridian_version(char *arg)
+static int __init parse_viridian_version(char *arg)
 {
     const char *t;
     unsigned int n[3];
@@ -1118,10 +1118,11 @@ static void __init parse_viridian_version(char *arg)
 
     printk("viridian-version = %#x,%#x,%#x\n",
            viridian_major, viridian_minor, viridian_build);
-    return;
+    return 0;
 
  fail:
     printk(XENLOG_WARNING "Invalid viridian-version, using default\n");
+    return -EINVAL;
 }
 custom_param("viridian-version", parse_viridian_version);
 
-- 
2.12.3


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

[PATCH v2 10/52] xen/arch/x86/hvm/vmx/vmcs.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/hvm/vmx/vmcs.c

to indicate whether the parameter value was parsed successfully.

Cc: Jun Nakajima <jun.nakajima@intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/hvm/vmx/vmcs.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index 2008fee280..4ddcbd7e5e 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -74,9 +74,10 @@ static s8 __read_mostly opt_ept_ad = -1;
  *  pml                 Enable PML
  *  ad                  Use A/D bits
  */
-static void __init parse_ept_param(char *s)
+static int __init parse_ept_param(char *s)
 {
     char *ss;
+    int rc = 0;
 
     do {
         bool_t val = !!strncmp(s, "no-", 3);
@@ -92,9 +93,13 @@ static void __init parse_ept_param(char *s)
             opt_pml_enabled = val;
         else if ( !strcmp(s, "ad") )
             opt_ept_ad = val;
+        else
+            rc = -EINVAL;
 
         s = ss + 1;
     } while ( ss );
+
+    return rc;
 }
 custom_param("ept", parse_ept_param);
 
-- 
2.12.3


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

[PATCH v2 11/52] xen/arch/x86/io_apic.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/io_apic.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/io_apic.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c
index 2838f6bd99..f790579dbd 100644
--- a/xen/arch/x86/io_apic.c
+++ b/xen/arch/x86/io_apic.c
@@ -1581,7 +1581,7 @@ static unsigned int startup_level_ioapic_irq(struct irq_desc *desc)
     return 0; /* don't check for pending */
 }
 
-static void __init setup_ioapic_ack(char *s)
+static int __init setup_ioapic_ack(char *s)
 {
     if ( !strcmp(s, "old") )
     {
@@ -1594,7 +1594,12 @@ static void __init setup_ioapic_ack(char *s)
         ioapic_ack_forced = true;
     }
     else
+    {
         printk("Unknown ioapic_ack value specified: '%s'\n", s);
+        return -EINVAL;
+    }
+
+    return 0;
 }
 custom_param("ioapic_ack", setup_ioapic_ack);
 
-- 
2.12.3


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

[PATCH v2 12/52] xen/arch/x86/irq.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/irq.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/irq.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index 57e6c18970..5d68e4252a 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -26,7 +26,7 @@
 #include <asm/mach-generic/mach_apic.h>
 #include <public/physdev.h>
 
-static void parse_irq_vector_map_param(char *s);
+static int parse_irq_vector_map_param(char *s);
 
 /* opt_noirqbalance: If true, software IRQ balancing/affinity is disabled. */
 bool __read_mostly opt_noirqbalance;
@@ -60,9 +60,10 @@ static struct timer irq_ratelimit_timer;
 static unsigned int __read_mostly irq_ratelimit_threshold = 10000;
 integer_param("irq_ratelimit", irq_ratelimit_threshold);
 
-static void __init parse_irq_vector_map_param(char *s)
+static int __init parse_irq_vector_map_param(char *s)
 {
     char *ss;
+    int rc = 0;
 
     do {
         ss = strchr(s, ',');
@@ -75,9 +76,13 @@ static void __init parse_irq_vector_map_param(char *s)
             opt_irq_vector_map=OPT_IRQ_VECTOR_MAP_GLOBAL;
         else if ( !strcmp(s, "per-device"))
             opt_irq_vector_map=OPT_IRQ_VECTOR_MAP_PERDEV;
+        else
+            rc = -EINVAL;
 
         s = ss + 1;
     } while ( ss );
+
+    return rc;
 }
 
 /* Must be called when irq disabled */
-- 
2.12.3


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

[PATCH v2 13/52] xen/arch/x86/microcode.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/microcode.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/microcode.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c
index 7558202efa..78ea6b53bd 100644
--- a/xen/arch/x86/microcode.c
+++ b/xen/arch/x86/microcode.c
@@ -73,15 +73,19 @@ void __init microcode_set_module(unsigned int idx)
  * If the EFI has forced which of the multiboot payloads is to be used,
  * no parsing will be attempted.
  */
-static void __init parse_ucode(char *s)
+static int __init parse_ucode(char *s)
 {
+    const char *q = NULL;
+
     if ( ucode_mod_forced ) /* Forced by EFI */
-       return;
+       return 0;
 
     if ( !strncmp(s, "scan", 4) )
         ucode_scan = 1;
     else
-        ucode_mod_idx = simple_strtol(s, NULL, 0);
+        ucode_mod_idx = simple_strtol(s, &q, 0);
+
+    return (q && *q) ? -EINVAL : 0;
 }
 custom_param("ucode", parse_ucode);
 
-- 
2.12.3


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

[PATCH v2 14/52] xen/arch/x86/mm.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/mm.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/mm.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 97b3b4ba2c..28d7ecf5df 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -170,14 +170,19 @@ static uint32_t base_disallow_mask;
      L1_DISALLOW_MASK : (L1_DISALLOW_MASK & ~PAGE_CACHE_ATTRS))
 
 static s8 __read_mostly opt_mmio_relax;
-static void __init parse_mmio_relax(const char *s)
+static int __init parse_mmio_relax(const char *s)
 {
     if ( !*s )
         opt_mmio_relax = 1;
     else
         opt_mmio_relax = parse_bool(s);
     if ( opt_mmio_relax < 0 && strcmp(s, "all") )
+    {
         opt_mmio_relax = 0;
+        return -EINVAL;
+    }
+
+    return 0;
 }
 custom_param("mmio-relax", parse_mmio_relax);
 
-- 
2.12.3


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

[PATCH v2 15/52] xen/arch/x86/nmi.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/nmi.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/nmi.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/xen/arch/x86/nmi.c b/xen/arch/x86/nmi.c
index 8914581f66..038c5608e1 100644
--- a/xen/arch/x86/nmi.c
+++ b/xen/arch/x86/nmi.c
@@ -46,35 +46,43 @@ bool __initdata opt_watchdog;
 /* watchdog_force: If true, process unknown NMIs when running the watchdog. */
 bool watchdog_force;
 
-static void __init parse_watchdog(char *s)
+static int __init parse_watchdog(char *s)
 {
     if ( !*s )
     {
         opt_watchdog = true;
-        return;
+        return 0;
     }
 
     switch ( parse_bool(s) )
     {
     case 0:
         opt_watchdog = false;
-        return;
+        return 0;
     case 1:
         opt_watchdog = true;
-        return;
+        return 0;
     }
 
     if ( !strcmp(s, "force") )
         watchdog_force = opt_watchdog = true;
+    else
+        return -EINVAL;
+
+    return 0;
 }
 custom_param("watchdog", parse_watchdog);
 
 /* opt_watchdog_timeout: Number of seconds to wait before panic. */
 static unsigned int opt_watchdog_timeout = 5;
-static void parse_watchdog_timeout(char * s)
+static int parse_watchdog_timeout(char * s)
 {
-    opt_watchdog_timeout = simple_strtoull(s, NULL, 0);
+    const char *q;
+
+    opt_watchdog_timeout = simple_strtoull(s, &q, 0);
     opt_watchdog = !!opt_watchdog_timeout;
+
+    return *q ? -EINVAL : 0;
 }
 custom_param("watchdog_timeout", parse_watchdog_timeout);
 
-- 
2.12.3


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

[PATCH v2 16/52] xen/arch/x86/numa.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/numa.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/numa.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/numa.c b/xen/arch/x86/numa.c
index d45196fafc..9454f5f9f4 100644
--- a/xen/arch/x86/numa.c
+++ b/xen/arch/x86/numa.c
@@ -303,10 +303,10 @@ static __init int numa_setup(char *opt)
 { 
     if ( !strncmp(opt,"off",3) )
         numa_off = true;
-    if ( !strncmp(opt,"on",2) )
+    else if ( !strncmp(opt,"on",2) )
         numa_off = false;
 #ifdef CONFIG_NUMA_EMU
-    if ( !strncmp(opt, "fake=", 5) )
+    else if ( !strncmp(opt, "fake=", 5) )
     {
         numa_off = false;
         numa_fake = simple_strtoul(opt+5,NULL,0);
@@ -315,14 +315,16 @@ static __init int numa_setup(char *opt)
     }
 #endif
 #ifdef CONFIG_ACPI_NUMA
-    if ( !strncmp(opt,"noacpi",6) )
+    else if ( !strncmp(opt,"noacpi",6) )
     {
         numa_off = false;
         acpi_numa = -1;
     }
 #endif
+    else
+        return -EINVAL;
 
-    return 1;
+    return 0;
 } 
 
 /*
-- 
2.12.3


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

[PATCH v2 17/52] xen/arch/x86/oprofile/nmi_int.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/oprofile/nmi_int.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/oprofile/nmi_int.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/xen/arch/x86/oprofile/nmi_int.c b/xen/arch/x86/oprofile/nmi_int.c
index 126f7a8d9f..91965ac7df 100644
--- a/xen/arch/x86/oprofile/nmi_int.c
+++ b/xen/arch/x86/oprofile/nmi_int.c
@@ -329,6 +329,8 @@ static int force_cpu_type(const char *str)
 		force_arch_perfmon = 1;
 		printk(KERN_INFO "oprofile: forcing architectural perfmon\n");
 	}
+	else
+		return -EINVAL;
 
 	return 0;
 }
-- 
2.12.3


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

[PATCH v2 18/52] xen/arch/x86/psr.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/psr.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/psr.c | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index 9ce8f17a18..397963c667 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -420,7 +420,7 @@ static const struct feat_props l2_cat_props = {
 };
 
 static void __init parse_psr_bool(char *s, char *value, char *feature,
-                                  unsigned int mask)
+                                  unsigned int mask, int *rc)
 {
     if ( !strcmp(s, feature) )
     {
@@ -434,13 +434,17 @@ static void __init parse_psr_bool(char *s, char *value, char *feature,
                 opt_psr &= ~mask;
             else if ( val_int == 1 )
                 opt_psr |= mask;
+            else
+                *rc = -EINVAL;
         }
     }
 }
 
-static void __init parse_psr_param(char *s)
+static int __init parse_psr_param(char *s)
 {
     char *ss, *val_str;
+    const char *q;
+    int rc = 0;
 
     do {
         ss = strchr(s, ',');
@@ -451,18 +455,28 @@ static void __init parse_psr_param(char *s)
         if ( val_str )
             *val_str++ = '\0';
 
-        parse_psr_bool(s, val_str, "cmt", PSR_CMT);
-        parse_psr_bool(s, val_str, "cat", PSR_CAT);
-        parse_psr_bool(s, val_str, "cdp", PSR_CDP);
+        parse_psr_bool(s, val_str, "cmt", PSR_CMT, &rc);
+        parse_psr_bool(s, val_str, "cat", PSR_CAT, &rc);
+        parse_psr_bool(s, val_str, "cdp", PSR_CDP, &rc);
 
         if ( val_str && !strcmp(s, "rmid_max") )
-            opt_rmid_max = simple_strtoul(val_str, NULL, 0);
+        {
+            opt_rmid_max = simple_strtoul(val_str, &q, 0);
+            if ( *q )
+                rc = -EINVAL;
+        }
 
         if ( val_str && !strcmp(s, "cos_max") )
-            opt_cos_max = simple_strtoul(val_str, NULL, 0);
+        {
+            opt_cos_max = simple_strtoul(val_str, &q, 0);
+            if ( *q )
+                rc = -EINVAL;
+        }
 
         s = ss + 1;
     } while ( ss );
+
+    return rc;
 }
 custom_param("psr", parse_psr_param);
 
-- 
2.12.3


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

[PATCH v2 19/52] xen/arch/x86/setup.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/setup.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/setup.c | 32 ++++++++++++++++++++++----------
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index db5df6956d..edb6d44dc6 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -67,7 +67,7 @@ unsigned long __read_mostly cr4_pv32_mask;
 /* "acpi=force":  Override the disable blacklist.                   */
 /* "acpi=ht":     Limit ACPI just to boot-time to enable HT.        */
 /* "acpi=noirq":  Disables ACPI interrupt routing.                  */
-static void parse_acpi_param(char *s);
+static int parse_acpi_param(char *s);
 custom_param("acpi", parse_acpi_param);
 
 /* **** Linux config option: propagated to domain0. */
@@ -102,59 +102,67 @@ unsigned long __read_mostly mmu_cr4_features = XEN_MINIMAL_CR4;
 /* smep: Enable/disable Supervisor Mode Execution Protection (default on). */
 #define SMEP_HVM_ONLY (-1)
 static s8 __initdata opt_smep = 1;
-static void __init parse_smep_param(char *s)
+static int __init parse_smep_param(char *s)
 {
     if ( !*s )
     {
         opt_smep = 1;
-        return;
+        return 0;
     }
 
     switch ( parse_bool(s) )
     {
     case 0:
         opt_smep = 0;
-        return;
+        return 0;
     case 1:
         opt_smep = 1;
-        return;
+        return 0;
     }
 
     if ( !strcmp(s, "hvm") )
         opt_smep = SMEP_HVM_ONLY;
+    else
+        return -EINVAL;
+
+    return 0;
 }
 custom_param("smep", parse_smep_param);
 
 /* smap: Enable/disable Supervisor Mode Access Prevention (default on). */
 #define SMAP_HVM_ONLY (-1)
 static s8 __initdata opt_smap = 1;
-static void __init parse_smap_param(char *s)
+static int __init parse_smap_param(char *s)
 {
     if ( !*s )
     {
         opt_smap = 1;
-        return;
+        return 0;
     }
 
     switch ( parse_bool(s) )
     {
     case 0:
         opt_smap = 0;
-        return;
+        return 0;
     case 1:
         opt_smap = 1;
-        return;
+        return 0;
     }
 
     if ( !strcmp(s, "hvm") )
         opt_smap = SMAP_HVM_ONLY;
+    else
+        return -EINVAL;
+
+    return 0;
 }
 custom_param("smap", parse_smap_param);
 
 bool __read_mostly acpi_disabled;
 bool __initdata acpi_force;
 static char __initdata acpi_param[10] = "";
-static void __init parse_acpi_param(char *s)
+static int __init parse_acpi_param(char *s)
 {
     /* Save the parameter so it can be propagated to domain0. */
     safe_strcpy(acpi_param, s);
@@ -180,6 +188,10 @@ static void __init parse_acpi_param(char *s)
     {
         acpi_noirq_set();
     }
+    else
+        return -EINVAL;
+
+    return 0;
 }
 
 static const module_t *__initdata initial_images;
-- 
2.12.3


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

[PATCH v2 20/52] xen/arch/x86/shutdown.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/shutdown.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/shutdown.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/shutdown.c b/xen/arch/x86/shutdown.c
index f63b8a668f..db9060e5e2 100644
--- a/xen/arch/x86/shutdown.c
+++ b/xen/arch/x86/shutdown.c
@@ -51,7 +51,7 @@ static int reboot_mode;
  * efi    Use the EFI reboot (if running under EFI)
  */
 static enum reboot_type reboot_type = BOOT_INVALID;
-static void __init set_reboot_type(char *str)
+static int __init set_reboot_type(char *str)
 {
     for ( ; ; )
     {
@@ -74,6 +74,8 @@ static void __init set_reboot_type(char *str)
         case 't':
             reboot_type = *str;
             break;
+        default:
+            return -EINVAL;
         }
         if ( (str = strchr(str, ',')) == NULL )
             break;
@@ -82,6 +84,8 @@ static void __init set_reboot_type(char *str)
 
     if ( reboot_type == BOOT_EFI && !efi_enabled(EFI_RS) )
         reboot_type = BOOT_INVALID;
+
+    return 0;
 }
 custom_param("reboot", set_reboot_type);
 
-- 
2.12.3


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

[PATCH v2 21/52] xen/arch/x86/time.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/time.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/time.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index b988b94d2e..eba7aed72d 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -1870,7 +1870,7 @@ int hwdom_pit_access(struct ioreq *ioreq)
  * tsc=skewed: Assume TSCs are individually reliable, but skewed across CPUs.
  * tsc=stable:socket: Assume TSCs are reliable across sockets.
  */
-static void __init tsc_parse(const char *s)
+static int __init tsc_parse(const char *s)
 {
     if ( !strcmp(s, "unstable") )
     {
@@ -1882,6 +1882,10 @@ static void __init tsc_parse(const char *s)
         setup_clear_cpu_cap(X86_FEATURE_TSC_RELIABLE);
     else if ( !strcmp(s, "stable:socket") )
         tsc_flags |= TSC_RELIABLE_SOCKET;
+    else
+        return -EINVAL;
+
+    return 0;
 }
 custom_param("tsc", tsc_parse);
 
-- 
2.12.3


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

[PATCH v2 22/52] xen/arch/x86/x86_64/mmconfig-shared.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/x86_64/mmconfig-shared.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/x86_64/mmconfig-shared.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/x86_64/mmconfig-shared.c b/xen/arch/x86/x86_64/mmconfig-shared.c
index 488470bfeb..4e1e354aab 100644
--- a/xen/arch/x86/x86_64/mmconfig-shared.c
+++ b/xen/arch/x86/x86_64/mmconfig-shared.c
@@ -28,7 +28,7 @@
 
 unsigned int pci_probe = PCI_PROBE_CONF1 | PCI_PROBE_MMCONF;
 
-static void __init parse_mmcfg(char *s)
+static int __init parse_mmcfg(char *s)
 {
     char *ss;
 
@@ -37,13 +37,24 @@ static void __init parse_mmcfg(char *s)
         if ( ss )
             *ss = '\0';
 
-        if ( !parse_bool(s) )
+        switch ( parse_bool(s) ) {
+        case 0:
             pci_probe &= ~PCI_PROBE_MMCONF;
-        else if ( !strcmp(s, "amd_fam10") || !strcmp(s, "amd-fam10") )
-            pci_probe |= PCI_CHECK_ENABLE_AMD_MMCONF;
+            break;
+        case 1:
+            break;
+        default:
+            if ( !strcmp(s, "amd_fam10") || !strcmp(s, "amd-fam10") )
+                pci_probe |= PCI_CHECK_ENABLE_AMD_MMCONF;
+            else
+                return -EINVAL;
+            break;
+        }
 
         s = ss + 1;
     } while ( ss );
+
+    return 0;
 }
 custom_param("mmcfg", parse_mmcfg);
 
-- 
2.12.3


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

[PATCH v2 23/52] xen/common/core_parking.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/common/core_parking.c

to indicate whether the parameter value was parsed successfully.

Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/common/core_parking.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/xen/common/core_parking.c b/xen/common/core_parking.c
index de269e06c2..b07a157ff9 100644
--- a/xen/common/core_parking.c
+++ b/xen/common/core_parking.c
@@ -41,14 +41,16 @@ static enum core_parking_controller {
     PERFORMANCE_FIRST
 } core_parking_controller = POWER_FIRST;
 
-static void __init setup_core_parking_option(char *str)
+static int __init setup_core_parking_option(char *str)
 {
     if ( !strcmp(str, "power") )
         core_parking_controller = POWER_FIRST;
     else if ( !strcmp(str, "performance") )
         core_parking_controller = PERFORMANCE_FIRST;
     else
-        return;
+        return -EINVAL;
+
+    return 0;
 }
 custom_param("core_parking", setup_core_parking_option);
 
-- 
2.12.3


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

[PATCH v2 24/52] xen/common/domain.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/common/domain.c

to indicate whether the parameter value was parsed successfully.

Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/common/domain.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/xen/common/domain.c b/xen/common/domain.c
index b22aacc57e..29a652268f 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -245,12 +245,14 @@ static int late_hwdom_init(struct domain *d)
 
 static unsigned int __read_mostly extra_hwdom_irqs;
 static unsigned int __read_mostly extra_domU_irqs = 32;
-static void __init parse_extra_guest_irqs(const char *s)
+static int __init parse_extra_guest_irqs(const char *s)
 {
     if ( isdigit(*s) )
         extra_domU_irqs = simple_strtoul(s, &s, 0);
     if ( *s == ',' && isdigit(*++s) )
         extra_hwdom_irqs = simple_strtoul(s, &s, 0);
+
+    return *s ? -EINVAL : 0;
 }
 custom_param("extra_guest_irqs", parse_extra_guest_irqs);
 
-- 
2.12.3


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

[PATCH v2 25/52] xen/common/efi/boot.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/common/efi/boot.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/common/efi/boot.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 11bdc7a2a4..25c249ae42 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -1306,7 +1306,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
 
 static bool __initdata efi_map_uc;
 
-static void __init parse_efi_param(char *s)
+static int __init parse_efi_param(char *s)
 {
     char *ss;
 
@@ -1329,9 +1329,13 @@ static void __init parse_efi_param(char *s)
         }
         else if ( !strcmp(s, "attr=uc") )
             efi_map_uc = val;
+        else
+            return -EINVAL;
 
         s = ss + 1;
     } while ( ss );
+
+    return 0;
 }
 custom_param("efi", parse_efi_param);
 
-- 
2.12.3


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

[PATCH v2 26/52] xen/common/kexec.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/common/kexec.c

to indicate whether the parameter value was parsed successfully.

Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/common/kexec.c | 30 ++++++++++++++++++++++++++----
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/xen/common/kexec.c b/xen/common/kexec.c
index a52c30ba1e..d647a6c30e 100644
--- a/xen/common/kexec.c
+++ b/xen/common/kexec.c
@@ -102,9 +102,10 @@ static void *crash_heap_current = NULL, *crash_heap_end = NULL;
  * < and below are synonyomous, the latter being useful for grub2 systems
  * which would otherwise require escaping of the < option
  */
-static void __init parse_crashkernel(const char *str)
+static int __init parse_crashkernel(const char *str)
 {
     const char *cur;
+    int rc = 0;
 
     if ( strchr(str, ':' ) )
     {
@@ -116,6 +117,7 @@ static void __init parse_crashkernel(const char *str)
                 printk(XENLOG_WARNING "crashkernel: too many ranges\n");
                 cur = NULL;
                 str = strpbrk(str, "@,<");
+                rc = -EINVAL;
                 break;
             }
 
@@ -126,6 +128,7 @@ static void __init parse_crashkernel(const char *str)
             if ( *str != '-' )
             {
                 printk(XENLOG_WARNING "crashkernel: '-' expected\n");
+                rc = -EINVAL;
                 break;
             }
 
@@ -137,6 +140,7 @@ static void __init parse_crashkernel(const char *str)
                 if ( ranges[idx].end <= ranges[idx].start )
                 {
                     printk(XENLOG_WARNING "crashkernel: end <= start\n");
+                    rc = -EINVAL;
                     break;
                 }
             }
@@ -146,6 +150,7 @@ static void __init parse_crashkernel(const char *str)
             if ( *str != ':' )
             {
                 printk(XENLOG_WARNING "crashkernel: ':' expected\n");
+                rc = -EINVAL;
                 break;
             }
 
@@ -169,10 +174,18 @@ static void __init parse_crashkernel(const char *str)
         else if ( !strncmp(str, ",below=", 7) )
             kexec_crash_area_limit = parse_size_and_unit(cur = str + 7, &str);
         else
+        {
             printk(XENLOG_WARNING "crashkernel: '%s' ignored\n", str);
+            rc = -EINVAL;
+        }
     }
     if ( cur && cur == str )
+    {
         printk(XENLOG_WARNING "crashkernel: memory value expected\n");
+        rc = -EINVAL;
+    }
+
+    return rc;
 }
 custom_param("crashkernel", parse_crashkernel);
 
@@ -186,7 +199,7 @@ custom_param("crashkernel", parse_crashkernel);
  * - all will allocate additional structures such as domain and vcpu structs
  *       low so the crash kernel can perform an extended analysis of state.
  */
-static void __init parse_low_crashinfo(const char * str)
+static int __init parse_low_crashinfo(const char * str)
 {
 
     if ( !strlen(str) )
@@ -202,7 +215,10 @@ static void __init parse_low_crashinfo(const char * str)
     {
         printk("Unknown low_crashinfo parameter '%s'.  Defaulting to min.\n", str);
         low_crashinfo_mode = LOW_CRASHINFO_MIN;
+        return -EINVAL;
     }
+
+    return 0;
 }
 custom_param("low_crashinfo", parse_low_crashinfo);
 
@@ -212,19 +228,25 @@ custom_param("low_crashinfo", parse_low_crashinfo);
  *
  * <addr> will be rounded down to the nearest power of two.  Defaults to 64G
  */
-static void __init parse_crashinfo_maxaddr(const char * str)
+static int __init parse_crashinfo_maxaddr(const char * str)
 {
     u64 addr;
+    const char *q;
 
     /* if low_crashinfo_mode is unset, default to min. */
     if ( low_crashinfo_mode == LOW_CRASHINFO_INVALID )
         low_crashinfo_mode = LOW_CRASHINFO_MIN;
 
-    if ( (addr = parse_size_and_unit(str, NULL)) )
+    if ( (addr = parse_size_and_unit(str, &q)) )
         crashinfo_maxaddr = addr;
     else
+    {
         printk("Unable to parse crashinfo_maxaddr. Defaulting to %"PRIpaddr"\n",
                crashinfo_maxaddr);
+        return -EINVAL;
+    }
+
+    return *q ? -EINVAL : 0;
 }
 custom_param("crashinfo_maxaddr", parse_crashinfo_maxaddr);
 
-- 
2.12.3


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

[PATCH v2 27/52] xen/common/memory.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/common/memory.c

to indicate whether the parameter value was parsed successfully.

Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/common/memory.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/xen/common/memory.c b/xen/common/memory.c
index b2066db07e..87a33c0e62 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -55,7 +55,7 @@ static unsigned int __read_mostly hwdom_max_order = CONFIG_HWDOM_MAX_ORDER;
 #ifdef HAS_PASSTHROUGH
 static unsigned int __read_mostly ptdom_max_order = CONFIG_PTDOM_MAX_ORDER;
 #endif
-static void __init parse_max_order(const char *s)
+static int __init parse_max_order(const char *s)
 {
     if ( *s != ',' )
         domu_max_order = simple_strtoul(s, &s, 0);
@@ -67,6 +67,8 @@ static void __init parse_max_order(const char *s)
     if ( *s == ',' && *++s != ',' )
         ptdom_max_order = simple_strtoul(s, &s, 0);
 #endif
+
+    return *s ? -EINVAL : 0;
 }
 custom_param("memop-max-order", parse_max_order);
 
-- 
2.12.3


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

[PATCH v2 28/52] xen/common/sched_credit2.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/common/sched_credit2.c

to indicate whether the parameter value was parsed successfully.

Cc: George Dunlap <george.dunlap@eu.citrix.com>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Dario Faggioli <dario.faggioli@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/common/sched_credit2.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index 29c002a63e..9b1db1351f 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -339,7 +339,7 @@ static const char *const opt_runqueue_str[] = {
 };
 static int __read_mostly opt_runqueue = OPT_RUNQUEUE_SOCKET;
 
-static void parse_credit2_runqueue(const char *s)
+static int parse_credit2_runqueue(const char *s)
 {
     unsigned int i;
 
@@ -348,11 +348,13 @@ static void parse_credit2_runqueue(const char *s)
         if ( !strcmp(s, opt_runqueue_str[i]) )
         {
             opt_runqueue = i;
-            return;
+            return 0;
         }
     }
 
     printk("WARNING, unrecognized value of credit2_runqueue option!\n");
+
+    return -EINVAL;
 }
 custom_param("credit2_runqueue", parse_credit2_runqueue);
 
-- 
2.12.3


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

[PATCH v2 29/52] xen/drivers/acpi/tables.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/drivers/acpi/tables.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/drivers/acpi/tables.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/xen/drivers/acpi/tables.c b/xen/drivers/acpi/tables.c
index dd2031f36a..ea600248aa 100644
--- a/xen/drivers/acpi/tables.c
+++ b/xen/drivers/acpi/tables.c
@@ -458,12 +458,13 @@ int __init acpi_table_init(void)
 
 static int __init acpi_parse_apic_instance(char *str)
 {
+	const char *q;
 
-	acpi_apic_instance = simple_strtoul(str, NULL, 0);
+	acpi_apic_instance = simple_strtoul(str, &q, 0);
 
 	printk(KERN_NOTICE PREFIX "Shall use APIC/MADT table %d\n",
 	       acpi_apic_instance);
 
-	return 0;
+	return *q ? -EINVAL : 0;
 }
 custom_param("acpi_apic_instance", acpi_parse_apic_instance);
-- 
2.12.3


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

[PATCH v2 30/52] xen/drivers/char/console.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/drivers/char/console.c

to indicate whether the parameter value was parsed successfully.

Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/drivers/char/console.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index f0659fba1b..daf0e1878d 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -67,7 +67,7 @@ enum con_timestamp_mode
 
 static enum con_timestamp_mode __read_mostly opt_con_timestamp_mode = TSM_NONE;
 
-static void parse_console_timestamps(char *s);
+static int parse_console_timestamps(char *s);
 custom_param("console_timestamps", parse_console_timestamps);
 
 /* conring_size: allows a large console ring than default (16kB). */
@@ -123,8 +123,8 @@ static int __read_mostly xenlog_guest_upper_thresh =
 static int __read_mostly xenlog_guest_lower_thresh =
     XENLOG_GUEST_LOWER_THRESHOLD;
 
-static void parse_loglvl(char *s);
-static void parse_guest_loglvl(char *s);
+static int parse_loglvl(char *s);
+static int parse_guest_loglvl(char *s);
 
 /*
  * <lvl> := none|error|warning|info|debug|all
@@ -156,23 +156,26 @@ static int __init __parse_loglvl(char *s, char **ps)
     return 2; /* sane fallback */
 }
 
-static void __init _parse_loglvl(char *s, int *lower, int *upper)
+static int __init _parse_loglvl(char *s, int *lower, int *upper)
 {
     *lower = *upper = __parse_loglvl(s, &s);
     if ( *s == '/' )
         *upper = __parse_loglvl(s+1, &s);
     if ( *upper < *lower )
         *upper = *lower;
+
+    return *s ? -EINVAL : 0;
 }
 
-static void __init parse_loglvl(char *s)
+static int __init parse_loglvl(char *s)
 {
-    _parse_loglvl(s, &xenlog_lower_thresh, &xenlog_upper_thresh);
+    return _parse_loglvl(s, &xenlog_lower_thresh, &xenlog_upper_thresh);
 }
 
-static void __init parse_guest_loglvl(char *s)
+static int __init parse_guest_loglvl(char *s)
 {
-    _parse_loglvl(s, &xenlog_guest_lower_thresh, &xenlog_guest_upper_thresh);
+    return _parse_loglvl(s, &xenlog_guest_lower_thresh,
+                         &xenlog_guest_upper_thresh);
 }
 
 static char *loglvl_str(int lvl)
@@ -603,16 +606,16 @@ static int printk_prefix_check(char *p, char **pp)
             ((loglvl < upper_thresh) && printk_ratelimit()));
 } 
 
-static void __init parse_console_timestamps(char *s)
+static int __init parse_console_timestamps(char *s)
 {
     switch ( parse_bool(s) )
     {
     case 0:
         opt_con_timestamp_mode = TSM_NONE;
-        return;
+        return 0;
     case 1:
         opt_con_timestamp_mode = TSM_DATE;
-        return;
+        return 0;
     }
     if ( *s == '\0' || /* Compat for old booleanparam() */
          !strcmp(s, "date") )
@@ -623,6 +626,10 @@ static void __init parse_console_timestamps(char *s)
         opt_con_timestamp_mode = TSM_BOOT;
     else if ( !strcmp(s, "none") )
         opt_con_timestamp_mode = TSM_NONE;
+    else
+        return -EINVAL;
+
+    return 0;
 }
 
 static void printk_start_of_line(const char *prefix)
-- 
2.12.3


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

[PATCH v2 31/52] xen/drivers/cpufreq/cpufreq.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/drivers/cpufreq/cpufreq.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/drivers/cpufreq/cpufreq.c      | 18 +++++++++++++-----
 xen/include/acpi/cpufreq/cpufreq.h |  2 +-
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/xen/drivers/cpufreq/cpufreq.c b/xen/drivers/cpufreq/cpufreq.c
index fd82ef5dce..daec558f97 100644
--- a/xen/drivers/cpufreq/cpufreq.c
+++ b/xen/drivers/cpufreq/cpufreq.c
@@ -62,7 +62,7 @@ LIST_HEAD_READ_MOSTLY(cpufreq_governor_list);
 /* set xen as default cpufreq */
 enum cpufreq_controller cpufreq_controller = FREQCTL_xen;
 
-static void __init setup_cpufreq_option(char *str)
+static int __init setup_cpufreq_option(char *str)
 {
     char *arg = strpbrk(str, ",:");
     int choice;
@@ -76,14 +76,14 @@ static void __init setup_cpufreq_option(char *str)
         xen_processor_pmbits &= ~XEN_PROCESSOR_PM_PX;
         cpufreq_controller = FREQCTL_dom0_kernel;
         opt_dom0_vcpus_pin = 1;
-        return;
+        return 0;
     }
 
     if ( choice == 0 || !strcmp(str, "none") )
     {
         xen_processor_pmbits &= ~XEN_PROCESSOR_PM_PX;
         cpufreq_controller = FREQCTL_none;
-        return;
+        return 0;
     }
 
     if ( choice > 0 || !strcmp(str, "xen") )
@@ -91,8 +91,10 @@ static void __init setup_cpufreq_option(char *str)
         xen_processor_pmbits |= XEN_PROCESSOR_PM_PX;
         cpufreq_controller = FREQCTL_xen;
         if ( arg && *arg )
-            cpufreq_cmdline_parse(arg);
+            return cpufreq_cmdline_parse(arg);
     }
+
+    return (choice < 0) ? -EINVAL : 0;
 }
 custom_param("cpufreq", setup_cpufreq_option);
 
@@ -571,7 +573,7 @@ static int __init cpufreq_handle_common_option(const char *name, const char *val
     return 0;
 }
 
-void __init cpufreq_cmdline_parse(char *str)
+int __init cpufreq_cmdline_parse(char *str)
 {
     static struct cpufreq_governor *__initdata cpufreq_governors[] =
     {
@@ -582,6 +584,7 @@ void __init cpufreq_cmdline_parse(char *str)
         &cpufreq_gov_powersave
     };
     unsigned int gov_index = 0;
+    int rc = 0;
 
     do {
         char *val, *end = strchr(str, ',');
@@ -611,11 +614,16 @@ void __init cpufreq_cmdline_parse(char *str)
         if (str && !cpufreq_handle_common_option(str, val) &&
             (!cpufreq_governors[gov_index]->handle_option ||
              !cpufreq_governors[gov_index]->handle_option(str, val)))
+        {
             printk(XENLOG_WARNING "cpufreq/%s: option '%s' not recognized\n",
                    cpufreq_governors[gov_index]->name, str);
+            rc = -EINVAL;
+        }
 
         str = end;
     } while (str);
+
+    return rc;
 }
 
 static int cpu_callback(
diff --git a/xen/include/acpi/cpufreq/cpufreq.h b/xen/include/acpi/cpufreq/cpufreq.h
index 48ad1d0004..5c41747cbf 100644
--- a/xen/include/acpi/cpufreq/cpufreq.h
+++ b/xen/include/acpi/cpufreq/cpufreq.h
@@ -79,7 +79,7 @@ DECLARE_PER_CPU(struct cpufreq_policy *, cpufreq_cpu_policy);
 extern int __cpufreq_set_policy(struct cpufreq_policy *data,
                                 struct cpufreq_policy *policy);
 
-void cpufreq_cmdline_parse(char *);
+int cpufreq_cmdline_parse(char *);
 
 #define CPUFREQ_SHARED_TYPE_NONE (0) /* None */
 #define CPUFREQ_SHARED_TYPE_HW   (1) /* HW does needed coordination */
-- 
2.12.3


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

[PATCH v2 32/52] xen/drivers/passthrough/amd/iommu_acpi.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/drivers/passthrough/amd/iommu_acpi.c

to indicate whether the parameter value was parsed successfully.

Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/drivers/passthrough/amd/iommu_acpi.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/xen/drivers/passthrough/amd/iommu_acpi.c b/xen/drivers/passthrough/amd/iommu_acpi.c
index f4c7206c2a..cd27bcef66 100644
--- a/xen/drivers/passthrough/amd/iommu_acpi.c
+++ b/xen/drivers/passthrough/amd/iommu_acpi.c
@@ -632,21 +632,23 @@ static u16 __init parse_ivhd_device_extended_range(
     return dev_length;
 }
 
-static void __init parse_ivrs_ioapic(char *str)
+static int __init parse_ivrs_ioapic(char *str)
 {
     const char *s = str;
     unsigned long id;
     unsigned int seg, bus, dev, func;
     unsigned int idx;
 
-    ASSERT(*s == '[');
+    if ( *s != '[' )
+        return -EINVAL;
+
     id = simple_strtoul(s + 1, &s, 0);
     if ( *s != ']' || *++s != '=' )
-        return;
+        return -EINVAL;
 
     s = parse_pci(s + 1, &seg, &bus, &dev, &func);
     if ( !s || *s )
-        return;
+        return -EINVAL;
 
     idx = ioapic_id_to_index(id);
     if ( idx == MAX_IO_APICS )
@@ -655,7 +657,7 @@ static void __init parse_ivrs_ioapic(char *str)
         if ( idx == MAX_IO_APICS )
         {
             printk(XENLOG_ERR "Error: %s: Too many IO APICs.\n", __func__);
-            return;
+            return -EINVAL;
         }
     }
 
@@ -663,28 +665,34 @@ static void __init parse_ivrs_ioapic(char *str)
     ioapic_sbdf[idx].seg = seg;
     ioapic_sbdf[idx].id = id;
     ioapic_sbdf[idx].cmdline = true;
+
+    return 0;
 }
 custom_param("ivrs_ioapic[", parse_ivrs_ioapic);
 
-static void __init parse_ivrs_hpet(char *str)
+static int __init parse_ivrs_hpet(char *str)
 {
     const char *s = str;
     unsigned long id;
     unsigned int seg, bus, dev, func;
 
-    ASSERT(*s == '[');
+    if ( *s != '[' )
+        return -EINVAL;
+
     id = simple_strtoul(s + 1, &s, 0);
     if ( id != (typeof(hpet_sbdf.id))id || *s != ']' || *++s != '=' )
-        return;
+        return -EINVAL;
 
     s = parse_pci(s + 1, &seg, &bus, &dev, &func);
     if ( !s || *s )
-        return;
+        return -EINVAL;
 
     hpet_sbdf.id = id;
     hpet_sbdf.bdf = PCI_BDF(bus, dev, func);
     hpet_sbdf.seg = seg;
     hpet_sbdf.init = HPET_CMDL;
+
+    return 0;
 }
 custom_param("ivrs_hpet[", parse_ivrs_hpet);
 
-- 
2.12.3


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

[PATCH v2 33/52] xen/drivers/passthrough/iommu.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/drivers/passthrough/iommu.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/drivers/passthrough/iommu.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index 5e81813942..8a333e177e 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -21,7 +21,7 @@
 #include <xen/keyhandler.h>
 #include <xsm/xsm.h>
 
-static void parse_iommu_param(char *s);
+static int parse_iommu_param(char *s);
 static void iommu_dump_p2m_table(unsigned char key);
 
 unsigned int __read_mostly iommu_dev_iotlb_timeout = 1000;
@@ -78,10 +78,10 @@ DEFINE_SPINLOCK(iommu_pt_cleanup_lock);
 PAGE_LIST_HEAD(iommu_pt_cleanup_list);
 static struct tasklet iommu_pt_cleanup_tasklet;
 
-static void __init parse_iommu_param(char *s)
+static int __init parse_iommu_param(char *s)
 {
     char *ss;
-    int val;
+    int val, b, rc = 0;
 
     do {
         val = !!strncmp(s, "no-", 3);
@@ -92,8 +92,9 @@ static void __init parse_iommu_param(char *s)
         if ( ss )
             *ss = '\0';
 
-        if ( !parse_bool(s) )
-            iommu_enable = 0;
+        b = parse_bool(s);
+        if ( b >= 0 )
+            iommu_enable = b;
         else if ( !strcmp(s, "force") || !strcmp(s, "required") )
             force_iommu = val;
         else if ( !strcmp(s, "workaround_bios_bug") )
@@ -124,9 +125,13 @@ static void __init parse_iommu_param(char *s)
             iommu_dom0_strict = val;
         else if ( !strcmp(s, "sharept") )
             iommu_hap_pt_share = val;
+        else
+            rc = -EINVAL;
 
         s = ss + 1;
     } while ( ss );
+
+    return rc;
 }
 
 int iommu_domain_init(struct domain *d)
-- 
2.12.3


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

[PATCH v2 34/52] xen/drivers/passthrough/pci.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/drivers/passthrough/pci.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/drivers/passthrough/pci.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index b02d48953b..82aa86b220 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -149,17 +149,17 @@ static struct phantom_dev {
 } phantom_devs[8];
 static unsigned int nr_phantom_devs;
 
-static void __init parse_phantom_dev(char *str) {
+static int __init parse_phantom_dev(char *str) {
     const char *s = str;
     unsigned int seg, bus, slot;
     struct phantom_dev phantom;
 
     if ( !s || !*s || nr_phantom_devs >= ARRAY_SIZE(phantom_devs) )
-        return;
+        return -EINVAL;
 
     s = parse_pci(s, &seg, &bus, &slot, NULL);
     if ( !s || *s != ',' )
-        return;
+        return -EINVAL;
 
     phantom.seg = seg;
     phantom.bus = bus;
@@ -170,10 +170,12 @@ static void __init parse_phantom_dev(char *str) {
     case 1: case 2: case 4:
         if ( *s )
     default:
-            return;
+            return -EINVAL;
     }
 
     phantom_devs[nr_phantom_devs++] = phantom;
+
+    return 0;
 }
 custom_param("pci-phantom", parse_phantom_dev);
 
@@ -189,9 +191,10 @@ static u16 __read_mostly bridge_ctl_mask;
  *   perr                       don't suppress parity errors (default)
  *   no-perr                    suppress parity errors
  */
-static void __init parse_pci_param(char *s)
+static int __init parse_pci_param(char *s)
 {
     char *ss;
+    int rc = 0;
 
     do {
         bool_t on = !!strncmp(s, "no-", 3);
@@ -214,6 +217,8 @@ static void __init parse_pci_param(char *s)
             cmd_mask = PCI_COMMAND_PARITY;
             brctl_mask = PCI_BRIDGE_CTL_PARITY;
         }
+        else
+            rc = -EINVAL;
 
         if ( on )
         {
@@ -228,6 +233,8 @@ static void __init parse_pci_param(char *s)
 
         s = ss + 1;
     } while ( ss );
+
+    return rc;
 }
 custom_param("pci", parse_pci_param);
 
-- 
2.12.3


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

[PATCH v2 35/52] xen/drivers/passthrough/vtd/dmar.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/drivers/passthrough/vtd/dmar.c

to indicate whether the parameter value was parsed successfully.

Cc: Kevin Tian <kevin.tian@intel.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/drivers/passthrough/vtd/dmar.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/xen/drivers/passthrough/vtd/dmar.c b/xen/drivers/passthrough/vtd/dmar.c
index 82040ddc05..dd122da730 100644
--- a/xen/drivers/passthrough/vtd/dmar.c
+++ b/xen/drivers/passthrough/vtd/dmar.c
@@ -1090,7 +1090,7 @@ int intel_iommu_get_reserved_device_memory(iommu_grdm_t *func, void *ctxt)
  * If a segment is specified for other than the first device, and it does not
  * match the one specified for the first one, an error will be reported.
  */
-static void __init parse_rmrr_param(const char *str)
+static int __init parse_rmrr_param(const char *str)
 {
     const char *s = str, *cur, *stmp;
     unsigned int seg, bus, dev, func, dev_count;
@@ -1143,5 +1143,7 @@ static void __init parse_rmrr_param(const char *str)
             nr_rmrr++;
 
     } while ( *s++ == ';' && nr_rmrr < MAX_USER_RMRR );
+
+    return *s ? -EINVAL : 0;
 }
 custom_param("rmrr", parse_rmrr_param);
-- 
2.12.3


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

[PATCH v2 36/52] xen/drivers/passthrough/vtd/quirks.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/drivers/passthrough/vtd/quirks.c

to indicate whether the parameter value was parsed successfully.

Cc: Kevin Tian <kevin.tian@intel.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/drivers/passthrough/vtd/quirks.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/xen/drivers/passthrough/vtd/quirks.c b/xen/drivers/passthrough/vtd/quirks.c
index 5bbbd96d51..4e61ac9ddb 100644
--- a/xen/drivers/passthrough/vtd/quirks.c
+++ b/xen/drivers/passthrough/vtd/quirks.c
@@ -247,9 +247,10 @@ void vtd_ops_postamble_quirk(struct iommu* iommu)
     }
 }
 
-static void __init parse_snb_timeout(const char *s)
+static int __init parse_snb_timeout(const char *s)
 {
     int t;
+    const char *q = NULL;
 
     t = parse_bool(s);
     if ( t < 0 )
@@ -259,13 +260,13 @@ static void __init parse_snb_timeout(const char *s)
         else if ( strcmp(s, "cap") == 0 )
             t = SNB_IGD_TIMEOUT;
         else
-            t = strtoul(s, NULL, 0);
+            t = strtoul(s, &q, 0);
     }
     else
         t = t ? SNB_IGD_TIMEOUT_LEGACY : 0;
     snb_igd_timeout = MILLISECS(t);
 
-    return;
+    return (q && *q) ? -EINVAL : 0;
 }
 custom_param("snb_igd_quirk", parse_snb_timeout);
 
-- 
2.12.3


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

[PATCH v2 37/52] xen/drivers/video/vesa.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/drivers/video/vesa.c

to indicate whether the parameter value was parsed successfully.

Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/drivers/video/vesa.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/xen/drivers/video/vesa.c b/xen/drivers/video/vesa.c
index 09d344c021..c92497e0bc 100644
--- a/xen/drivers/video/vesa.c
+++ b/xen/drivers/video/vesa.c
@@ -29,12 +29,14 @@ static unsigned int vram_remap;
 integer_param("vesa-map", vram_remap);
 
 static int font_height;
-static void __init parse_font_height(const char *s)
+static int __init parse_font_height(const char *s)
 {
     if ( simple_strtoul(s, &s, 10) == 8 && (*s++ == 'x') )
         font_height = simple_strtoul(s, &s, 10);
     if ( *s != '\0' )
         font_height = 0;
+
+    return 0;
 }
 custom_param("font", parse_font_height);
 
-- 
2.12.3


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

[PATCH v2 38/52] xen/xsm/flask/flask_op.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/xsm/flask/flask_op.c

to indicate whether the parameter value was parsed successfully.

Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/xsm/flask/flask_op.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c
index 719c2d7efc..35598b5fd8 100644
--- a/xen/xsm/flask/flask_op.c
+++ b/xen/xsm/flask/flask_op.c
@@ -26,7 +26,7 @@
 #define _copy_from_guest copy_from_guest
 
 enum flask_bootparam_t __read_mostly flask_bootparam = FLASK_BOOTPARAM_ENFORCING;
-static void parse_flask_param(char *s);
+static int parse_flask_param(char *s);
 custom_param("flask", parse_flask_param);
 
 bool_t __read_mostly flask_enforcing = 1;
@@ -58,7 +58,7 @@ static int flask_security_make_bools(void);
 
 extern int ss_initialized;
 
-static void __init parse_flask_param(char *s)
+static int __init parse_flask_param(char *s)
 {
     if ( !strcmp(s, "enforcing") )
         flask_bootparam = FLASK_BOOTPARAM_ENFORCING;
@@ -70,6 +70,8 @@ static void __init parse_flask_param(char *s)
         flask_bootparam = FLASK_BOOTPARAM_PERMISSIVE;
     else
         flask_bootparam = FLASK_BOOTPARAM_INVALID;
+
+    return (flask_bootparam == FLASK_BOOTPARAM_INVALID) ? -EINVAL : 0;
 }
 
 static int domain_has_security(struct domain *d, u32 perms)
-- 
2.12.3


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

[PATCH v2 39/52] xen: check parameter validity when parsing command line

[Juergen Gross]

Where possible check validity of parameters in _cmdline_parse() and
issue a warning message in case of an error detected.

Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
---
V2:
- replaced literal 8 by BITS_PER_BYTE (Wei Liu)
- added test for empty string to parse_bool()
---
 xen/common/kernel.c | 47 ++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 36 insertions(+), 11 deletions(-)

diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index ce7cb8adb5..756380be5b 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -23,9 +23,11 @@ enum system_state system_state = SYS_STATE_early_boot;
 xen_commandline_t saved_cmdline;
 static const char __initconst opt_builtin_cmdline[] = CONFIG_CMDLINE;
 
-static void __init assign_integer_param(
+static int __init assign_integer_param(
     const struct kernel_param *param, uint64_t val)
 {
+    unsigned int bits = param->len * BITS_PER_BYTE;
+
     switch ( param->len )
     {
     case sizeof(uint8_t):
@@ -43,14 +45,17 @@ static void __init assign_integer_param(
     default:
         BUG();
     }
+
+    return ( (val & (~0ULL << bits)) && ~(val | (~0ULL >> (65 - bits))) ) ?
+           -EOVERFLOW : 0;
 }
 
 static void __init _cmdline_parse(const char *cmdline)
 {
     char opt[128], *optval, *optkey, *q;
-    const char *p = cmdline;
+    const char *p = cmdline, *s;
     const struct kernel_param *param;
-    int bool_assert;
+    int bool_assert, rc = 0;
 
     for ( ; ; )
     {
@@ -97,8 +102,9 @@ static void __init _cmdline_parse(const char *cmdline)
                      !strncmp(param->name, opt, q + 1 - opt) )
                 {
                     optval[-1] = '=';
-                    ((void (*)(const char *))param->var)(q);
+                    rc = ((int (*)(const char *))param->var)(q);
                     optval[-1] = '\0';
+                    break;
                 }
                 continue;
             }
@@ -106,24 +112,34 @@ static void __init _cmdline_parse(const char *cmdline)
             switch ( param->type )
             {
             case OPT_STR:
+                rc = 0;
                 strlcpy(param->var, optval, param->len);
                 break;
             case OPT_UINT:
-                assign_integer_param(
+                rc = assign_integer_param(
                     param,
-                    simple_strtoll(optval, NULL, 0));
+                    simple_strtoll(optval, &s, 0));
+                if ( *s )
+                    rc = -EINVAL;
                 break;
             case OPT_BOOL:
-                if ( !parse_bool(optval) )
+                rc = parse_bool(optval);
+                if ( rc == -1 )
+                    break;
+                if ( !rc )
                     bool_assert = !bool_assert;
+                rc = 0;
                 assign_integer_param(param, bool_assert);
                 break;
             case OPT_SIZE:
-                assign_integer_param(
+                rc = assign_integer_param(
                     param,
-                    parse_size_and_unit(optval, NULL));
+                    parse_size_and_unit(optval, &s));
+                if ( *s )
+                    rc = -EINVAL;
                 break;
             case OPT_CUSTOM:
+                rc = -EINVAL;
                 if ( !bool_assert )
                 {
                     if ( *optval )
@@ -131,13 +147,21 @@ static void __init _cmdline_parse(const char *cmdline)
                     safe_strcpy(opt, "no");
                     optval = opt;
                 }
-                ((void (*)(const char *))param->var)(optval);
+                rc = ((int (*)(const char *))param->var)(optval);
                 break;
             default:
                 BUG();
                 break;
             }
+
+            break;
         }
+
+        if ( rc )
+            printk("parameter \"%s\" has invalid value \"%s\"!\n", optkey,
+                   optval);
+        if ( param >= __setup_end )
+            printk("parameter \"%s\" unknown!\n", optkey);
     }
 }
 
@@ -176,7 +200,8 @@ int __init parse_bool(const char *s)
          !strcmp("on", s) ||
          !strcmp("true", s) ||
          !strcmp("enable", s) ||
-         !strcmp("1", s) )
+         !strcmp("1", s) ||
+         !*s )
         return 1;
 
     return -1;
-- 
2.12.3


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

[PATCH v2 40/52] xen/arch/x86/apic.c: remove custom_param() error messages

[Juergen Gross]

With _cmdline_parse() now issuing error messages in case of illegal
parameters signalled by parsing functions specified in custom_param()
the message issued by apic_set_verbosity() can be removed.

Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
---
 xen/arch/x86/apic.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c
index fdb0a99927..5ffbd8f54f 100644
--- a/xen/arch/x86/apic.c
+++ b/xen/arch/x86/apic.c
@@ -800,11 +800,8 @@ static int __init apic_set_verbosity(char *str)
         apic_verbosity = APIC_DEBUG;
     else if (strcmp("verbose", str) == 0)
         apic_verbosity = APIC_VERBOSE;
-    else {
-        printk(KERN_WARNING "APIC Verbosity level %s not recognised"
-               " use apic_verbosity=verbose or apic_verbosity=debug", str);
+    else
         return -EINVAL;
-    }
 
     return 0;
 }
-- 
2.12.3


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

[PATCH v2 41/52] xen/arch/x86/cpu/mcheck/mce.c: remove custom_param() error messages

[Juergen Gross]

With _cmdline_parse() now issuing error messages in case of illegal
parameters signalled by parsing functions specified in custom_param()
the message issued by mce_set_verbosity() can be removed.

Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
---
 xen/arch/x86/cpu/mcheck/mce.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
index e577a9db11..507535b094 100644
--- a/xen/arch/x86/cpu/mcheck/mce.c
+++ b/xen/arch/x86/cpu/mcheck/mce.c
@@ -67,11 +67,7 @@ static int __init mce_set_verbosity(char *str)
     if (strcmp("verbose", str) == 0)
         mce_verbosity = MCE_VERBOSE;
     else
-    {
-        printk(KERN_DEBUG "Machine Check verbosity level %s not recognised"
-               "use mce_verbosity=verbose", str);
         return -EINVAL;
-    }
 
     return 0;
 }
-- 
2.12.3


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

[PATCH v2 42/52] xen/arch/x86/hvm/viridian.c: remove custom_param() error messages

[Juergen Gross]

With _cmdline_parse() now issuing error messages in case of illegal
parameters signalled by parsing functions specified in custom_param()
the message issued by parse_viridian_version() can be removed.

Cc: Paul Durrant <paul.durrant@citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Paul Durrant <paul.durrant@citrix.com>
---
 xen/arch/x86/hvm/viridian.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/xen/arch/x86/hvm/viridian.c b/xen/arch/x86/hvm/viridian.c
index 6f012bcb62..b15556b98b 100644
--- a/xen/arch/x86/hvm/viridian.c
+++ b/xen/arch/x86/hvm/viridian.c
@@ -1102,15 +1102,15 @@ static int __init parse_viridian_version(char *arg)
 
         n[i++] = simple_strtoul(t, &e, 0);
         if ( *e != '\0' )
-            goto fail;
+            return -EINVAL;
     }
     if ( i != 3 )
-        goto fail;
+        return -EINVAL;
 
     if ( ((typeof(viridian_major))n[0] != n[0]) ||
          ((typeof(viridian_minor))n[1] != n[1]) ||
          ((typeof(viridian_build))n[2] != n[2]) )
-        goto fail;
+        return -EINVAL;
 
     viridian_major = n[0];
     viridian_minor = n[1];
@@ -1119,10 +1119,6 @@ static int __init parse_viridian_version(char *arg)
     printk("viridian-version = %#x,%#x,%#x\n",
            viridian_major, viridian_minor, viridian_build);
     return 0;
-
- fail:
-    printk(XENLOG_WARNING "Invalid viridian-version, using default\n");
-    return -EINVAL;
 }
 custom_param("viridian-version", parse_viridian_version);
 
-- 
2.12.3


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

[PATCH v2 43/52] xen/arch/x86/io_apic.c: remove custom_param() error messages

[Juergen Gross]

With _cmdline_parse() now issuing error messages in case of illegal
parameters signalled by parsing functions specified in custom_param()
the message issued by setup_ioapic_ack() can be removed.

Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
---
 xen/arch/x86/io_apic.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c
index f790579dbd..76ba752f23 100644
--- a/xen/arch/x86/io_apic.c
+++ b/xen/arch/x86/io_apic.c
@@ -1594,10 +1594,7 @@ static int __init setup_ioapic_ack(char *s)
         ioapic_ack_forced = true;
     }
     else
-    {
-        printk("Unknown ioapic_ack value specified: '%s'\n", s);
         return -EINVAL;
-    }
 
     return 0;
 }
-- 
2.12.3


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

[PATCH v2 44/52] xen/common/kexec.c: remove custom_param() error messages

[Juergen Gross]

With _cmdline_parse() now issuing error messages in case of illegal
parameters signalled by parsing functions specified in custom_param()
some messages issued by parse_low_crashinfo() and
parse_crashinfo_maxaddr() can be removed.

Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
---
 xen/common/kexec.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/xen/common/kexec.c b/xen/common/kexec.c
index d647a6c30e..d2fbf9eb80 100644
--- a/xen/common/kexec.c
+++ b/xen/common/kexec.c
@@ -213,7 +213,6 @@ static int __init parse_low_crashinfo(const char * str)
         low_crashinfo_mode = LOW_CRASHINFO_ALL;
     else
     {
-        printk("Unknown low_crashinfo parameter '%s'.  Defaulting to min.\n", str);
         low_crashinfo_mode = LOW_CRASHINFO_MIN;
         return -EINVAL;
     }
@@ -240,11 +239,7 @@ static int __init parse_crashinfo_maxaddr(const char * str)
     if ( (addr = parse_size_and_unit(str, &q)) )
         crashinfo_maxaddr = addr;
     else
-    {
-        printk("Unable to parse crashinfo_maxaddr. Defaulting to %"PRIpaddr"\n",
-               crashinfo_maxaddr);
         return -EINVAL;
-    }
 
     return *q ? -EINVAL : 0;
 }
-- 
2.12.3


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

[PATCH v2 45/52] xen/common/sched_credit2.c: remove custom_param() error messages

[Juergen Gross]

With _cmdline_parse() now issuing error messages in case of illegal
parameters signalled by parsing functions specified in custom_param()
the message issued by parse_credit2_runqueue() can be removed.

Cc: George Dunlap <george.dunlap@eu.citrix.com>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Dario Faggioli <dario.faggioli@citrix.com>
---
 xen/common/sched_credit2.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index 9b1db1351f..2da9cc2ffb 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -352,8 +352,6 @@ static int parse_credit2_runqueue(const char *s)
         }
     }
 
-    printk("WARNING, unrecognized value of credit2_runqueue option!\n");
-
     return -EINVAL;
 }
 custom_param("credit2_runqueue", parse_credit2_runqueue);
-- 
2.12.3


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

[PATCH v2 46/52] xen: carve out a generic parsing function from _cmdline_parse()

[Juergen Gross]

In order to support generic parameter parsing carve out the parser from
_cmdline_parse(). As this generic function might be called after boot
remove the __init annotations from all called sub-functions.

Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/common/kernel.c | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index 756380be5b..c05198b226 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -23,8 +23,7 @@ enum system_state system_state = SYS_STATE_early_boot;
 xen_commandline_t saved_cmdline;
 static const char __initconst opt_builtin_cmdline[] = CONFIG_CMDLINE;
 
-static int __init assign_integer_param(
-    const struct kernel_param *param, uint64_t val)
+static int assign_integer_param(const struct kernel_param *param, uint64_t val)
 {
     unsigned int bits = param->len * BITS_PER_BYTE;
 
@@ -50,12 +49,13 @@ static int __init assign_integer_param(
            -EOVERFLOW : 0;
 }
 
-static void __init _cmdline_parse(const char *cmdline)
+static int parse_params(const char *cmdline, const struct kernel_param *start,
+                        const struct kernel_param *end)
 {
     char opt[128], *optval, *optkey, *q;
     const char *p = cmdline, *s;
     const struct kernel_param *param;
-    int bool_assert, rc = 0;
+    int bool_assert, rc = 0, final_rc = 0;
 
     for ( ; ; )
     {
@@ -93,7 +93,7 @@ static void __init _cmdline_parse(const char *cmdline)
         if ( !bool_assert )
             optkey += 3;
 
-        for ( param = __setup_start; param < __setup_end; param++ )
+        for ( param = start; param < end; param++ )
         {
             if ( strcmp(param->name, optkey) )
             {
@@ -158,11 +158,24 @@ static void __init _cmdline_parse(const char *cmdline)
         }
 
         if ( rc )
+        {
             printk("parameter \"%s\" has invalid value \"%s\"!\n", optkey,
                    optval);
-        if ( param >= __setup_end )
+            final_rc = rc;
+        }
+        if ( param >= end )
+        {
             printk("parameter \"%s\" unknown!\n", optkey);
+            final_rc = -EINVAL;
+        }
     }
+
+    return final_rc;
+}
+
+static void __init _cmdline_parse(const char *cmdline)
+{
+    parse_params(cmdline, __setup_start, __setup_end);
 }
 
 /**
@@ -187,7 +200,7 @@ void __init cmdline_parse(const char *cmdline)
 #endif
 }
 
-int __init parse_bool(const char *s)
+int parse_bool(const char *s)
 {
     if ( !strcmp("no", s) ||
          !strcmp("off", s) ||
-- 
2.12.3


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

[PATCH v2 47/52] xen: add basic support for runtime parameter changing

[Juergen Gross]

Add the needed infrastructure for runtime parameter changing similar
to that used at boot time via cmdline. We are using the same parsing
functions as for cmdline parsing, but with a different array of
parameter definitions.

Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- added modification of ARM linker script (Wei Liu)
---
 xen/arch/arm/xen.lds.S |  4 ++++
 xen/arch/x86/xen.lds.S |  4 ++++
 xen/common/kernel.c    |  5 +++++
 xen/include/xen/init.h | 22 ++++++++++++++++++++--
 xen/include/xen/lib.h  |  1 +
 5 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index 2d54f224ec..75c41354ac 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -81,6 +81,10 @@ SECTIONS
        __start_schedulers_array = .;
        *(.data.schedulers)
        __end_schedulers_array = .;
+       . = ALIGN(POINTER_ALIGN);
+       __param_start = .;
+       *(.data.param)
+       __param_end = .;
        *(.data.rel)
        *(.data.rel.*)
        CONSTRUCTORS
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index ff08bbe42a..5bd7912759 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -226,6 +226,10 @@ SECTIONS
        __start_schedulers_array = .;
        *(.data.schedulers)
        __end_schedulers_array = .;
+       . = ALIGN(POINTER_ALIGN);
+       __param_start = .;
+       *(.data.param)
+       __param_end = .;
   } :text
 
   .data : {                    /* Data */
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index c05198b226..9bb9e5c73a 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -178,6 +178,11 @@ static void __init _cmdline_parse(const char *cmdline)
     parse_params(cmdline, __setup_start, __setup_end);
 }
 
+int runtime_parse(const char *line)
+{
+    return parse_params(line, __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/include/xen/init.h b/xen/include/xen/init.h
index 25d2eef8dd..7c7f92c6b2 100644
--- a/xen/include/xen/init.h
+++ b/xen/include/xen/init.h
@@ -87,11 +87,16 @@ struct kernel_param {
 };
 
 extern const struct kernel_param __setup_start[], __setup_end[];
+extern const struct kernel_param __param_start[], __param_end[];
+
+#define __dataparam       __used_section(".data.param")
+
+#define __param(att)      static const att \
+    __attribute__((__aligned__(sizeof(void *)))) struct kernel_param
 
 #define __setup_str static const __initconst \
     __attribute__((__aligned__(1))) char
-#define __kparam static const __initsetup \
-    __attribute__((__aligned__(sizeof(void *)))) struct kernel_param
+#define __kparam          __param(__initsetup)
 
 #define custom_param(_name, _var) \
     __setup_str __setup_str_##_var[] = _name; \
@@ -113,6 +118,19 @@ extern const struct kernel_param __setup_start[], __setup_end[];
     __kparam __setup_##_var = \
         { __setup_str_##_var, OPT_STR, sizeof(_var), &_var }
 
+#define __rtparam         __param(__dataparam)
+
+#define custom_param_runtime(_name, _var) \
+    __rtparam __rtpar_##_var = { _name, OPT_CUSTOM, 0, _var }
+#define boolean_param_runtime(_name, _var) \
+    __rtparam __rtpar_##_var = { _name, OPT_BOOL, sizeof(_var), &_var }
+#define integer_param_runtime(_name, _var) \
+    __rtparam __rtpar_##_var = { _name, OPT_UINT, sizeof(_var), &_var }
+#define size_param_runtime(_name, _var) \
+    __rtparam __rtpar_##_var = { _name, OPT_SIZE, sizeof(_var), &_var }
+#define string_param_runtime(_name, _var) \
+    __rtparam __rtpar_##_var = { _name, OPT_STR, sizeof(_var), &_var }
+
 #endif /* __ASSEMBLY__ */
 
 #ifdef CONFIG_LATE_HWDOM
diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
index 995a85a7db..5651498de2 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);
 
 /*#define DEBUG_TRACE_DUMP*/
-- 
2.12.3


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

[PATCH v2 48/52] xen: add hypercall for setting parameters at runtime

[Juergen Gross]

Add a sysctl hypercall to support setting parameters similar to
command line parameters, but at runtime. The parameters to set are
specified as a string, just like the boot parameters.

Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- corrected XSM test (Daniel De Graaf)
---
 tools/flask/policy/modules/dom0.te  |  2 +-
 xen/common/sysctl.c                 | 29 +++++++++++++++++++++++++++++
 xen/include/public/sysctl.h         | 19 +++++++++++++++++++
 xen/xsm/flask/hooks.c               |  3 +++
 xen/xsm/flask/policy/access_vectors |  2 ++
 5 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/tools/flask/policy/modules/dom0.te b/tools/flask/policy/modules/dom0.te
index d0a4d91ac0..338caaf41e 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_cat_op pmu_ctrl get_symbol
 	get_cpu_levelling_caps get_cpu_featureset livepatch_op
-	gcov_op
+	gcov_op set_parameter
 };
 
 # Allow dom0 to use all XENVER_ subops that have checks.
diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index ae58a0f650..a3237fe9be 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -467,6 +467,35 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
             copyback = 1;
         break;
 
+    case XEN_SYSCTL_set_parameter:
+    {
+        char *params;
+
+        if ( op->u.set_parameter.size > XEN_SET_PARAMETER_MAX_SIZE )
+        {
+            ret = -EINVAL;
+            break;
+        }
+        params = xmalloc_bytes(op->u.set_parameter.size + 1);
+        if ( !params )
+        {
+            ret = -ENOMEM;
+            break;
+        }
+        if ( __copy_from_guest(params, op->u.set_parameter.params,
+                               op->u.set_parameter.size) )
+            ret = -EFAULT;
+        else
+        {
+            params[op->u.set_parameter.size] = 0;
+            ret = runtime_parse(params);
+        }
+
+        xfree(params);
+
+        break;
+    }
+
     default:
         ret = arch_do_sysctl(op, u_sysctl);
         copyback = 0;
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 9e51af61e1..43b18bdb9b 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -1096,6 +1096,23 @@ struct xen_sysctl_livepatch_op {
 typedef struct xen_sysctl_livepatch_op xen_sysctl_livepatch_op_t;
 DEFINE_XEN_GUEST_HANDLE(xen_sysctl_livepatch_op_t);
 
+/*
+ * XEN_SYSCTL_set_parameter
+ *
+ * Change hypervisor parameters at runtime.
+ * The input string is parsed similar to the boot parameters.
+ */
+
+#define XEN_SET_PARAMETER_MAX_SIZE 1023
+struct xen_sysctl_set_parameter {
+    XEN_GUEST_HANDLE_64(char) params;       /* IN: pointer to parameters. */
+    uint16_t size;                          /* IN: size of parameters. Max.
+                                               XEN_SET_PARAMETER_MAX_SIZE. */
+    uint16_t pad[3];                        /* IN: MUST be zero. */
+};
+typedef struct xen_sysctl_set_parameter xen_sysctl_set_parameter_t;
+DEFINE_XEN_GUEST_HANDLE(xen_sysctl_set_parameter_t);
+
 struct xen_sysctl {
     uint32_t cmd;
 #define XEN_SYSCTL_readconsole                    1
@@ -1124,6 +1141,7 @@ struct xen_sysctl {
 #define XEN_SYSCTL_get_cpu_levelling_caps        25
 #define XEN_SYSCTL_get_cpu_featureset            26
 #define XEN_SYSCTL_livepatch_op                  27
+#define XEN_SYSCTL_set_parameter                 28
     uint32_t interface_version; /* XEN_SYSCTL_INTERFACE_VERSION */
     union {
         struct xen_sysctl_readconsole       readconsole;
@@ -1152,6 +1170,7 @@ struct xen_sysctl {
         struct xen_sysctl_cpu_levelling_caps cpu_levelling_caps;
         struct xen_sysctl_cpu_featureset    cpu_featureset;
         struct xen_sysctl_livepatch_op      livepatch;
+        struct xen_sysctl_set_parameter     set_parameter;
         uint8_t                             pad[128];
     } u;
 };
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index fd84ac0f09..c9c275bf3b 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -825,6 +825,9 @@ static int flask_sysctl(int cmd)
     case XEN_SYSCTL_gcov_op:
         return avc_current_has_perm(SECINITSID_XEN, SECCLASS_XEN2,
                                     XEN2__GCOV_OP, NULL);
+    case XEN_SYSCTL_set_parameter:
+        return avc_current_has_perm(SECINITSID_XEN, SECCLASS_XEN2,
+                                    XEN2__SET_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 1f7eb35fc8..b80fca1ec0 100644
--- a/xen/xsm/flask/policy/access_vectors
+++ b/xen/xsm/flask/policy/access_vectors
@@ -101,6 +101,8 @@ class xen2
     livepatch_op
 # XEN_SYSCTL_gcov_op
     gcov_op
+# XEN_SYSCTL_set_parameter
+    set_parameter
 }
 
 # Classes domain and domain2 consist of operations that a domain performs on
-- 
2.12.3


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

[PATCH v2 49/52] libxc: add function to set hypervisor parameters

[Juergen Gross]

Add a new libxc function to set hypervisor parameters at runtime
similar to boot time parameters via command line.

Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxc/include/xenctrl.h |  1 +
 tools/libxc/xc_misc.c         | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index c7710b8f36..ad5e6b3d77 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1210,6 +1210,7 @@ int xc_readconsolering(xc_interface *xch,
                        int clear, int incremental, uint32_t *pindex);
 
 int xc_send_debug_keys(xc_interface *xch, char *keys);
+int xc_set_parameters(xc_interface *xch, char *params);
 
 typedef xen_sysctl_physinfo_t xc_physinfo_t;
 typedef xen_sysctl_cputopo_t xc_cputopo_t;
diff --git a/tools/libxc/xc_misc.c b/tools/libxc/xc_misc.c
index 2303293c6c..5a34151dd3 100644
--- a/tools/libxc/xc_misc.c
+++ b/tools/libxc/xc_misc.c
@@ -187,6 +187,26 @@ int xc_send_debug_keys(xc_interface *xch, char *keys)
     return ret;
 }
 
+int xc_set_parameters(xc_interface *xch, char *params)
+{
+    int ret, len = strlen(params);
+    DECLARE_SYSCTL;
+    DECLARE_HYPERCALL_BOUNCE(params, len, XC_HYPERCALL_BUFFER_BOUNCE_IN);
+
+    if ( xc_hypercall_bounce_pre(xch, params) )
+        return -1;
+
+    sysctl.cmd = XEN_SYSCTL_set_parameter;
+    set_xen_guest_handle(sysctl.u.set_parameter.params, params);
+    sysctl.u.set_parameter.size = len;
+
+    ret = do_sysctl(xch, &sysctl);
+
+    xc_hypercall_bounce_post(xch, params);
+
+    return ret;
+}
+
 int xc_physinfo(xc_interface *xch,
                 xc_physinfo_t *put_info)
 {
-- 
2.12.3


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

[PATCH v2 50/52] libxl: add libxl_set_parameters() function

[Juergen Gross]

Add a new libxl function to set hypervisor parameters at runtime
similar to boot time parameters via command line.

Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- corrected coding style (Wei Liu)
- removed superfluous #ifdef (Wei Liu)
---
 tools/libxl/libxl.c | 15 +++++++++++++++
 tools/libxl/libxl.h |  8 ++++++++
 2 files changed, 23 insertions(+)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 0ef874406f..5e6f95536e 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -652,6 +652,21 @@ int libxl_send_debug_keys(libxl_ctx *ctx, char *keys)
     return 0;
 }
 
+int libxl_set_parameters(libxl_ctx *ctx, char *params)
+{
+    int ret;
+    GC_INIT(ctx);
+
+    ret = xc_set_parameters(ctx->xch, params);
+    if (ret < 0) {
+        LOGE(ERROR, "setting parameters");
+        GC_FREE;
+        return 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 229e289750..17045253ab 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -1051,6 +1051,13 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, const libxl_mac *src);
  */
 #define LIBXL_HAVE_QED 1
 
+/*
+ * LIBXL_HAVE_SET_PARAMETERS
+ *
+ * If this is defined setting hypervisor parameters is supported.
+ */
+#define LIBXL_HAVE_SET_PARAMETERS 1
+
 typedef char **libxl_string_list;
 void libxl_string_list_dispose(libxl_string_list *sl);
 int libxl_string_list_length(const libxl_string_list *sl);
@@ -2105,6 +2112,7 @@ int libxl_send_trigger(libxl_ctx *ctx, uint32_t domid,
                        libxl_trigger trigger, uint32_t vcpuid);
 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);
 
 typedef struct libxl__xen_console_reader libxl_xen_console_reader;
 
-- 
2.12.3


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

[PATCH v2 51/52] xl: add new xl command set-parameters

[Juergen Gross]

Add a new xl command "set-parameters" to set hypervisor parameters at
runtime similar to boot time parameters via command line.

Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 docs/man/xl.pod.1.in   |  5 +++++
 tools/xl/xl.h          |  1 +
 tools/xl/xl_cmdtable.c |  5 +++++
 tools/xl/xl_misc.c     | 20 ++++++++++++++++++++
 4 files changed, 31 insertions(+)

diff --git a/docs/man/xl.pod.1.in b/docs/man/xl.pod.1.in
index 16c83066fe..3d5f2f7359 100644
--- a/docs/man/xl.pod.1.in
+++ b/docs/man/xl.pod.1.in
@@ -822,6 +822,11 @@ Pass the VNC password to vncviewer via stdin.
 Send debug I<keys> to Xen. It is the same as pressing the Xen
 "conswitch" (Ctrl-A by default) three times and then pressing "keys".
 
+=item B<set-parameters> I<params>
+
+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<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 aa95b77146..5d3d2a4835 100644
--- a/tools/xl/xl.h
+++ b/tools/xl/xl.h
@@ -154,6 +154,7 @@ int main_rename(int argc, char **argv);
 int main_trigger(int argc, char **argv);
 int main_sysrq(int argc, char **argv);
 int main_debug_keys(int argc, char **argv);
+int main_set_parameters(int argc, char **argv);
 int main_dmesg(int argc, char **argv);
 int main_top(int argc, char **argv);
 int main_networkattach(int argc, char **argv);
diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
index 2c71a9f776..ba0159df67 100644
--- a/tools/xl/xl_cmdtable.c
+++ b/tools/xl/xl_cmdtable.c
@@ -309,6 +309,11 @@ struct cmd_spec cmd_table[] = {
       "Send debug keys to Xen",
       "<Keys>",
     },
+    { "set-parameters",
+      &main_set_parameters, 0, 1,
+      "Set hypervisor parameters",
+      "<Params>",
+    },
     { "dmesg",
       &main_dmesg, 0, 0,
       "Read and/or clear dmesg buffer",
diff --git a/tools/xl/xl_misc.c b/tools/xl/xl_misc.c
index 9c6227af23..dcf940a6d4 100644
--- a/tools/xl/xl_misc.c
+++ b/tools/xl/xl_misc.c
@@ -155,6 +155,26 @@ int main_debug_keys(int argc, char **argv)
     return EXIT_SUCCESS;
 }
 
+int main_set_parameters(int argc, char **argv)
+{
+    int opt;
+    char *params;
+
+    SWITCH_FOREACH_OPT(opt, "", NULL, "set-parameters", 1) {
+        /* No options */
+    }
+
+    params = argv[optind];
+
+    if (libxl_set_parameters(ctx, params)) {
+        fprintf(stderr, "cannot set parameters: %s\n", params);
+        fprintf(stderr, "Use \"xl dmesg\" to look for possible reason.\n");
+        return EXIT_FAILURE;
+    }
+
+    return EXIT_SUCCESS;
+}
+
 int main_devd(int argc, char **argv)
 {
     int ret = 0, opt = 0, daemonize = 1;
-- 
2.12.3


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

[PATCH v2 52/52] xen: make some console related parameters settable at runtime

[Juergen Gross]

Support modifying conswitch, console_timestamps, loglvl and
guest_loglvl at runtime.

Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
---
 docs/misc/xen-command-line.markdown |  8 ++++++++
 xen/drivers/char/console.c          | 14 +++++++++-----
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index 4002eab08b..9797c8db2d 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -391,6 +391,8 @@ makes sense on its own.
 
 > Default: `none`
 
+> Can be modified at runtime
+
 Specify which timestamp format Xen should use for each console line.
 
 * `none`: No timestamps
@@ -417,6 +419,8 @@ into the console ring buffer.
 
 > Default: `conswitch=a`
 
+> Can be modified at runtime
+
 Specify which character should be used to switch serial input between
 Xen and dom0.  The required sequence is CTRL-&lt;switch char&gt; three
 times.
@@ -898,6 +902,8 @@ maximum number of maptrack frames domain.
 
 > Default: `guest_loglvl=none/warning`
 
+> Can be modified at runtime
+
 Set the logging level for Xen guests.  Any log message with equal more
 more importance will be printed.
 
@@ -1164,6 +1170,8 @@ if left disabled by the BIOS.
 
 > Default: `loglvl=warning`
 
+> Can be modified at runtime
+
 Set the logging level for Xen.  Any log message with equal more more
 importance will be printed.
 
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index daf0e1878d..283bd3ac6c 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -41,6 +41,7 @@ string_param("console", opt_console);
 /*         boots. Any other value, or omitting the char, enables auto-switch */
 static unsigned char __read_mostly opt_conswitch[3] = "a";
 string_param("conswitch", opt_conswitch);
+string_param_runtime("conswitch", opt_conswitch);
 
 /* sync_console: force synchronous console output (useful for debugging). */
 static bool_t __initdata opt_sync_console;
@@ -69,6 +70,7 @@ static enum con_timestamp_mode __read_mostly opt_con_timestamp_mode = TSM_NONE;
 
 static int parse_console_timestamps(char *s);
 custom_param("console_timestamps", parse_console_timestamps);
+custom_param_runtime("console_timestamps", parse_console_timestamps);
 
 /* conring_size: allows a large console ring than default (16kB). */
 static uint32_t __initdata opt_conring_size;
@@ -136,6 +138,8 @@ static int parse_guest_loglvl(char *s);
  */
 custom_param("loglvl", parse_loglvl);
 custom_param("guest_loglvl", parse_guest_loglvl);
+custom_param_runtime("loglvl", parse_loglvl);
+custom_param_runtime("guest_loglvl", parse_guest_loglvl);
 
 static atomic_t print_everything = ATOMIC_INIT(0);
 
@@ -145,7 +149,7 @@ static atomic_t print_everything = ATOMIC_INIT(0);
         return (lvlnum);                                \
     }
 
-static int __init __parse_loglvl(char *s, char **ps)
+static int __parse_loglvl(char *s, char **ps)
 {
     ___parse_loglvl(s, ps, "none",    0);
     ___parse_loglvl(s, ps, "error",   1);
@@ -156,7 +160,7 @@ static int __init __parse_loglvl(char *s, char **ps)
     return 2; /* sane fallback */
 }
 
-static int __init _parse_loglvl(char *s, int *lower, int *upper)
+static int _parse_loglvl(char *s, int *lower, int *upper)
 {
     *lower = *upper = __parse_loglvl(s, &s);
     if ( *s == '/' )
@@ -167,12 +171,12 @@ static int __init _parse_loglvl(char *s, int *lower, int *upper)
     return *s ? -EINVAL : 0;
 }
 
-static int __init parse_loglvl(char *s)
+static int parse_loglvl(char *s)
 {
     return _parse_loglvl(s, &xenlog_lower_thresh, &xenlog_upper_thresh);
 }
 
-static int __init parse_guest_loglvl(char *s)
+static int parse_guest_loglvl(char *s)
 {
     return _parse_loglvl(s, &xenlog_guest_lower_thresh,
                          &xenlog_guest_upper_thresh);
@@ -606,7 +610,7 @@ static int printk_prefix_check(char *p, char **pp)
             ((loglvl < upper_thresh) && printk_ratelimit()));
 } 
 
-static int __init parse_console_timestamps(char *s)
+static int parse_console_timestamps(char *s)
 {
     switch ( parse_bool(s) )
     {
-- 
2.12.3


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

Re: [PATCH v2 44/52] xen/common/kexec.c: remove custom_param() error messages

[Jan Beulich]

>>> On 14.08.17 at 09:08, <jgross@suse.com> wrote:
> With _cmdline_parse() now issuing error messages in case of illegal
> parameters signalled by parsing functions specified in custom_param()
> some messages issued by parse_low_crashinfo() and
> parse_crashinfo_maxaddr() can be removed.

But you realize this results in less information being conveyed
(the defaults being fallen back to)?

Jan


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

Re: [PATCH v2 44/52] xen/common/kexec.c: remove custom_param() error messages

[Juergen Gross]

On 14/08/17 10:39, Jan Beulich wrote:
>>>> On 14.08.17 at 09:08, <jgross@suse.com> wrote:
>> With _cmdline_parse() now issuing error messages in case of illegal
>> parameters signalled by parsing functions specified in custom_param()
>> some messages issued by parse_low_crashinfo() and
>> parse_crashinfo_maxaddr() can be removed.
> 
> But you realize this results in less information being conveyed
> (the defaults being fallen back to)?

Yes. I guess it is a matter of taste which messages to keep. You will
have noticed I kept some other messages especially for the crashkernel
parameter which are not so obvious.

In case you want me to keep other messages I'm fine to do so, of course.

I could modify the messages to just print the used defaults if you like
that better.

Another possibility would be to always print the used parameters at the
end of parse_crashkernel().


Juergen

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

Re: [PATCH v2 44/52] xen/common/kexec.c: remove custom_param() error messages

[Jan Beulich]

>>> On 14.08.17 at 11:07, <jgross@suse.com> wrote:
> On 14/08/17 10:39, Jan Beulich wrote:
>>>>> On 14.08.17 at 09:08, <jgross@suse.com> wrote:
>>> With _cmdline_parse() now issuing error messages in case of illegal
>>> parameters signalled by parsing functions specified in custom_param()
>>> some messages issued by parse_low_crashinfo() and
>>> parse_crashinfo_maxaddr() can be removed.
>> 
>> But you realize this results in less information being conveyed
>> (the defaults being fallen back to)?
> 
> Yes. I guess it is a matter of taste which messages to keep. You will
> have noticed I kept some other messages especially for the crashkernel
> parameter which are not so obvious.
> 
> In case you want me to keep other messages I'm fine to do so, of course.
> 
> I could modify the messages to just print the used defaults if you like
> that better.
> 
> Another possibility would be to always print the used parameters at the
> end of parse_crashkernel().

Let's see what Andrew thinks.

Jan


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

Re: [PATCH v2 39/52] xen: check parameter validity when parsing command line

[Jan Beulich]

>>> On 14.08.17 at 09:08, <jgross@suse.com> wrote:
> --- a/xen/common/kernel.c
> +++ b/xen/common/kernel.c
> @@ -23,9 +23,11 @@ enum system_state system_state = SYS_STATE_early_boot;
>  xen_commandline_t saved_cmdline;
>  static const char __initconst opt_builtin_cmdline[] = CONFIG_CMDLINE;
>  
> -static void __init assign_integer_param(
> +static int __init assign_integer_param(
>      const struct kernel_param *param, uint64_t val)
>  {
> +    unsigned int bits = param->len * BITS_PER_BYTE;
> +
>      switch ( param->len )
>      {
>      case sizeof(uint8_t):
> @@ -43,14 +45,17 @@ static void __init assign_integer_param(
>      default:
>          BUG();
>      }
> +
> +    return ( (val & (~0ULL << bits)) && ~(val | (~0ULL >> (65 - bits))) ) ?

The left part has undefined behavior when param->len == 8
(and on x86 I'd expect it to produce just "val"). The right part
I guess is meant to be a sign check, but that's rather obscure.
As iirc it is signed-to-unsigned conversion which has uniformly
defined behavior it may end up being better for the parameter
to be of signed type and to allow values in the range
[<type>_MIN,U<type>_MAX]. Anything more precise would
require signedness to be communicated from the *_param()
users.

Also - stray blanks inside the outermost parentheses.

And finally, wouldn't it be better to check for overflow _before_
assigning to *param->var?

> @@ -97,8 +102,9 @@ static void __init _cmdline_parse(const char *cmdline)
>                       !strncmp(param->name, opt, q + 1 - opt) )
>                  {
>                      optval[-1] = '=';
> -                    ((void (*)(const char *))param->var)(q);
> +                    rc = ((int (*)(const char *))param->var)(q);

Neither here nor in the earlier "let custom parameter parsing
routines return errno" nor in the overview you mention why this
is safe - it is not a given that caller and callee disagreeing on
return type is going to work. Just think of functions returning
aggregates or (on ix86) ones returning floating point values in
st(0).

>                      optval[-1] = '\0';
> +                    break;

Why? Applies to further break-s you add: At least in the past we
had command line options with two handlers, where each of them
needed to be invoked. I don't think we should make such impossible
even if right now there aren't any such examples. Yet if you really
mean to, then the behavioral change needs to be called out in the
description.

> @@ -106,24 +112,34 @@ static void __init _cmdline_parse(const char *cmdline)
>              switch ( param->type )
>              {
>              case OPT_STR:
> +                rc = 0;
>                  strlcpy(param->var, optval, param->len);
>                  break;
>              case OPT_UINT:
> -                assign_integer_param(
> +                rc = assign_integer_param(
>                      param,
> -                    simple_strtoll(optval, NULL, 0));
> +                    simple_strtoll(optval, &s, 0));
> +                if ( *s )
> +                    rc = -EINVAL;
>                  break;
>              case OPT_BOOL:
> -                if ( !parse_bool(optval) )
> +                rc = parse_bool(optval);
> +                if ( rc == -1 )

Maybe "rc < 0"?

> @@ -131,13 +147,21 @@ static void __init _cmdline_parse(const char *cmdline)
>                      safe_strcpy(opt, "no");
>                      optval = opt;
>                  }
> -                ((void (*)(const char *))param->var)(optval);
> +                rc = ((int (*)(const char *))param->var)(optval);
>                  break;
>              default:
>                  BUG();
>                  break;
>              }
> +
> +            break;
>          }
> +
> +        if ( rc )
> +            printk("parameter \"%s\" has invalid value \"%s\"!\n", optkey,
> +                   optval);

With the changes made to optval in OPT_CUSTOM handling this
may end up being confusing.

Jan


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

Re: [PATCH v2 07/52] xen/arch/x86/dom0_build.c: let custom parameter parsing routines return errno

[Jan Beulich]

>>> On 14.08.17 at 09:08, <jgross@suse.com> wrote:
> --- a/xen/arch/x86/dom0_build.c
> +++ b/xen/arch/x86/dom0_build.c
> @@ -47,7 +47,7 @@ static long __init parse_amt(const char *s, const char **ps)
>      long pages = parse_size_and_unit((*s == '-') ? s+1 : s, ps) >> PAGE_SHIFT;
>      return (*s == '-') ? -pages : pages;
>  }
> -static void __init parse_dom0_mem(const char *s)
> +static int __init parse_dom0_mem(const char *s)

Please take the opportunity and add the missing blank line above
the one you modify.

Jan


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

Re: [PATCH v2 15/52] xen/arch/x86/nmi.c: let custom parameter parsing routines return errno

[Jan Beulich]

>>> On 14.08.17 at 09:08, <jgross@suse.com> wrote:
>  /* opt_watchdog_timeout: Number of seconds to wait before panic. */
>  static unsigned int opt_watchdog_timeout = 5;
> -static void parse_watchdog_timeout(char * s)
> +static int parse_watchdog_timeout(char * s)

Please also remove the stray blank.

Jan


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

Re: [PATCH v2 39/52] xen: check parameter validity when parsing command line

[Juergen Gross]

On 14/08/17 14:46, Jan Beulich wrote:
>>>> On 14.08.17 at 09:08, <jgross@suse.com> wrote:
>> --- a/xen/common/kernel.c
>> +++ b/xen/common/kernel.c
>> @@ -23,9 +23,11 @@ enum system_state system_state = SYS_STATE_early_boot;
>>  xen_commandline_t saved_cmdline;
>>  static const char __initconst opt_builtin_cmdline[] = CONFIG_CMDLINE;
>>  
>> -static void __init assign_integer_param(
>> +static int __init assign_integer_param(
>>      const struct kernel_param *param, uint64_t val)
>>  {
>> +    unsigned int bits = param->len * BITS_PER_BYTE;
>> +
>>      switch ( param->len )
>>      {
>>      case sizeof(uint8_t):
>> @@ -43,14 +45,17 @@ static void __init assign_integer_param(
>>      default:
>>          BUG();
>>      }
>> +
>> +    return ( (val & (~0ULL << bits)) && ~(val | (~0ULL >> (65 - bits))) ) ?
> 
> The left part has undefined behavior when param->len == 8
> (and on x86 I'd expect it to produce just "val"). The right part
> I guess is meant to be a sign check, but that's rather obscure.

Hmm, okay.

> As iirc it is signed-to-unsigned conversion which has uniformly
> defined behavior it may end up being better for the parameter
> to be of signed type and to allow values in the range
> [<type>_MIN,U<type>_MAX]. Anything more precise would
> require signedness to be communicated from the *_param()
> users.

Okay, I'll have a try.

> Also - stray blanks inside the outermost parentheses.
> 
> And finally, wouldn't it be better to check for overflow _before_
> assigning to *param->var?

I didn't want to change existing behavior. OTOH thinking twice you are
right. Better using the default value than an unexpected small one.

> 
>> @@ -97,8 +102,9 @@ static void __init _cmdline_parse(const char *cmdline)
>>                       !strncmp(param->name, opt, q + 1 - opt) )
>>                  {
>>                      optval[-1] = '=';
>> -                    ((void (*)(const char *))param->var)(q);
>> +                    rc = ((int (*)(const char *))param->var)(q);
> 
> Neither here nor in the earlier "let custom parameter parsing
> routines return errno" nor in the overview you mention why this
> is safe - it is not a given that caller and callee disagreeing on
> return type is going to work. Just think of functions returning
> aggregates or (on ix86) ones returning floating point values in
> st(0).

I thought about using a union in struct kernel_param and removing
above type cast. This would require modifying the initialization of
the kernel_param struct via the *_param() macros, though.

The other possibility would be using __builtin_types_compatible_p()
to check the function to be of proper type.

What would you like best?

> 
>>                      optval[-1] = '\0';
>> +                    break;
> 
> Why? Applies to further break-s you add: At least in the past we
> had command line options with two handlers, where each of them
> needed to be invoked. I don't think we should make such impossible
> even if right now there aren't any such examples. Yet if you really
> mean to, then the behavioral change needs to be called out in the
> description.

I wasn't aware of such a usage.

I'm fine for both alternatives. As you seem to prefer to keep support
for multiple handlers I'll modify the patch to allow that.

>> @@ -106,24 +112,34 @@ static void __init _cmdline_parse(const char *cmdline)
>>              switch ( param->type )
>>              {
>>              case OPT_STR:
>> +                rc = 0;
>>                  strlcpy(param->var, optval, param->len);
>>                  break;
>>              case OPT_UINT:
>> -                assign_integer_param(
>> +                rc = assign_integer_param(
>>                      param,
>> -                    simple_strtoll(optval, NULL, 0));
>> +                    simple_strtoll(optval, &s, 0));
>> +                if ( *s )
>> +                    rc = -EINVAL;
>>                  break;
>>              case OPT_BOOL:
>> -                if ( !parse_bool(optval) )
>> +                rc = parse_bool(optval);
>> +                if ( rc == -1 )
> 
> Maybe "rc < 0"?

Okay.

> 
>> @@ -131,13 +147,21 @@ static void __init _cmdline_parse(const char *cmdline)
>>                      safe_strcpy(opt, "no");
>>                      optval = opt;
>>                  }
>> -                ((void (*)(const char *))param->var)(optval);
>> +                rc = ((int (*)(const char *))param->var)(optval);
>>                  break;
>>              default:
>>                  BUG();
>>                  break;
>>              }
>> +
>> +            break;
>>          }
>> +
>> +        if ( rc )
>> +            printk("parameter \"%s\" has invalid value \"%s\"!\n", optkey,
>> +                   optval);
> 
> With the changes made to optval in OPT_CUSTOM handling this
> may end up being confusing.

Oh yes, good catch.


Juergen


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

Re: [PATCH v2 07/52] xen/arch/x86/dom0_build.c: let custom parameter parsing routines return errno

[Juergen Gross]

On 14/08/17 15:24, Jan Beulich wrote:
>>>> On 14.08.17 at 09:08, <jgross@suse.com> wrote:
>> --- a/xen/arch/x86/dom0_build.c
>> +++ b/xen/arch/x86/dom0_build.c
>> @@ -47,7 +47,7 @@ static long __init parse_amt(const char *s, const char **ps)
>>      long pages = parse_size_and_unit((*s == '-') ? s+1 : s, ps) >> PAGE_SHIFT;
>>      return (*s == '-') ? -pages : pages;
>>  }
>> -static void __init parse_dom0_mem(const char *s)
>> +static int __init parse_dom0_mem(const char *s)
> 
> Please take the opportunity and add the missing blank line above
> the one you modify.

Okay.


Juergen

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

Re: [PATCH v2 15/52] xen/arch/x86/nmi.c: let custom parameter parsing routines return errno

[Juergen Gross]

On 14/08/17 15:31, Jan Beulich wrote:
>>>> On 14.08.17 at 09:08, <jgross@suse.com> wrote:
>>  /* opt_watchdog_timeout: Number of seconds to wait before panic. */
>>  static unsigned int opt_watchdog_timeout = 5;
>> -static void parse_watchdog_timeout(char * s)
>> +static int parse_watchdog_timeout(char * s)
> 
> Please also remove the stray blank.

Okay.


Juergen

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

Re: [PATCH v2 18/52] xen/arch/x86/psr.c: let custom parameter parsing routines return errno

[Jan Beulich]

>>> On 14.08.17 at 09:08, <jgross@suse.com> wrote:
> --- a/xen/arch/x86/psr.c
> +++ b/xen/arch/x86/psr.c
> @@ -420,7 +420,7 @@ static const struct feat_props l2_cat_props = {
>  };
>  
>  static void __init parse_psr_bool(char *s, char *value, char *feature,
> -                                  unsigned int mask)
> +                                  unsigned int mask, int *rc)

Please make the function return a value (perhaps bool) instead.

> @@ -451,18 +455,28 @@ static void __init parse_psr_param(char *s)
>          if ( val_str )
>              *val_str++ = '\0';
>  
> -        parse_psr_bool(s, val_str, "cmt", PSR_CMT);
> -        parse_psr_bool(s, val_str, "cat", PSR_CAT);
> -        parse_psr_bool(s, val_str, "cdp", PSR_CDP);
> +        parse_psr_bool(s, val_str, "cmt", PSR_CMT, &rc);
> +        parse_psr_bool(s, val_str, "cat", PSR_CAT, &rc);
> +        parse_psr_bool(s, val_str, "cdp", PSR_CDP, &rc);
>  
>          if ( val_str && !strcmp(s, "rmid_max") )
> -            opt_rmid_max = simple_strtoul(val_str, NULL, 0);
> +        {
> +            opt_rmid_max = simple_strtoul(val_str, &q, 0);
> +            if ( *q )
> +                rc = -EINVAL;
> +        }
>  
>          if ( val_str && !strcmp(s, "cos_max") )
> -            opt_cos_max = simple_strtoul(val_str, NULL, 0);
> +        {
> +            opt_cos_max = simple_strtoul(val_str, &q, 0);
> +            if ( *q )
> +                rc = -EINVAL;
> +        }
>  
>          s = ss + 1;
>      } while ( ss );

So if val_str didn't match any of the five strings you won't indicate
an error here, which seems inconsistent.

Jan


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

Re: [PATCH v2 19/52] xen/arch/x86/setup.c: let custom parameter parsing routines return errno

[Jan Beulich]

>>> On 14.08.17 at 09:08, <jgross@suse.com> wrote:
>  bool __read_mostly acpi_disabled;
>  bool __initdata acpi_force;
>  static char __initdata acpi_param[10] = "";
> -static void __init parse_acpi_param(char *s)
> +static int __init parse_acpi_param(char *s)

Again please take the opportunity and add the missing blank line
ahead of the one you modify.

Jan


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

Re: [PATCH v2 20/52] xen/arch/x86/shutdown.c: let custom parameter parsing routines return errno

[Jan Beulich]

>>> On 14.08.17 at 09:08, <jgross@suse.com> wrote:
> --- a/xen/arch/x86/shutdown.c
> +++ b/xen/arch/x86/shutdown.c
> @@ -51,7 +51,7 @@ static int reboot_mode;
>   * efi    Use the EFI reboot (if running under EFI)
>   */
>  static enum reboot_type reboot_type = BOOT_INVALID;
> -static void __init set_reboot_type(char *str)
> +static int __init set_reboot_type(char *str)
>  {
>      for ( ; ; )
>      {
> @@ -74,6 +74,8 @@ static void __init set_reboot_type(char *str)
>          case 't':
>              reboot_type = *str;
>              break;
> +        default:
> +            return -EINVAL;

This alters behavior - you want to store -EINVAL in a local variable,
but continue the loop.

Jan


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

Re: [PATCH v2 22/52] xen/arch/x86/x86_64/mmconfig-shared.c: let custom parameter parsing routines return errno

[Jan Beulich]

>>> On 14.08.17 at 09:08, <jgross@suse.com> wrote:
> @@ -37,13 +37,24 @@ static void __init parse_mmcfg(char *s)
>          if ( ss )
>              *ss = '\0';
>  
> -        if ( !parse_bool(s) )
> +        switch ( parse_bool(s) ) {
> +        case 0:
>              pci_probe &= ~PCI_PROBE_MMCONF;
> -        else if ( !strcmp(s, "amd_fam10") || !strcmp(s, "amd-fam10") )
> -            pci_probe |= PCI_CHECK_ENABLE_AMD_MMCONF;
> +            break;
> +        case 1:
> +            break;
> +        default:
> +            if ( !strcmp(s, "amd_fam10") || !strcmp(s, "amd-fam10") )
> +                pci_probe |= PCI_CHECK_ENABLE_AMD_MMCONF;
> +            else
> +                return -EINVAL;
> +            break;
> +        }
>  
>          s = ss + 1;
>      } while ( ss );

Same here (and wherever else I may have overlooked it) - you
should not return out of loops.

Jan


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

Re: [PATCH v2 00/52] Support for modifying parameters at runtime

[Jan Beulich]

>>> On 14.08.17 at 09:07, <jgross@suse.com> wrote:
> Currently parameters of the hypervisor (e.g. console log level) can be
> set via boot command line. Instead of having to reboot the system in
> case another setting is desired, being able to modify many of those
> parameters at runtime would be the better option.
> 
> This patch series addresses this by adding a new xl command
> "xl set-parameters" which takes a string similar to the boot command
> line as parameter and passes this string to the hypervisor which will
> then use the same parsing infrastructure as for the command line in
> order to apply the parameter settings.
> 
> As error checks for invalid parameters or parameter values have been
> very sparse if present at all in the hypervisor, a major part of this
> patch series addresses this problem first: all custom parameter parsing
> functions are being changed to return success or an error. The main
> parsing function tests for generic parameter value errors (like e.g.
> overflow) or invalid parameters and issues a message in case an error
> has been detected. Most error messages in the custom parsing functions
> are removed then.
> 
> While not strictly required for runtime parameter modification I
> believe an improved parameter validation is a win with or without the
> runtime parameter modification support.
> 
> * Patches 1-38 are modifying the custom parameter parsing functions to
>   return success or error

Patches 4-37 (where relevant, i.e. where I've been Cc-ed)
Acked-by: Jan Beulich <jbeulich@suse.com>
including ones where I've asked for merely cosmetic changes.
Ones where I've pointed out more are excluded. I also think
that while doing things this fine grained has its merits, you
may have gone a little too far here - splitting at maintainer
boundaries would have made the series quite a bit shorter I
think, but no worse to review. My ack stands in case you
decide to combine some of the patches.

Jan


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

Re: [PATCH v2 39/52] xen: check parameter validity when parsing command line

[Jan Beulich]

>>> On 14.08.17 at 15:31, <jgross@suse.com> wrote:
> On 14/08/17 14:46, Jan Beulich wrote:
>>>>> On 14.08.17 at 09:08, <jgross@suse.com> wrote:
>>> @@ -97,8 +102,9 @@ static void __init _cmdline_parse(const char *cmdline)
>>>                       !strncmp(param->name, opt, q + 1 - opt) )
>>>                  {
>>>                      optval[-1] = '=';
>>> -                    ((void (*)(const char *))param->var)(q);
>>> +                    rc = ((int (*)(const char *))param->var)(q);
>> 
>> Neither here nor in the earlier "let custom parameter parsing
>> routines return errno" nor in the overview you mention why this
>> is safe - it is not a given that caller and callee disagreeing on
>> return type is going to work. Just think of functions returning
>> aggregates or (on ix86) ones returning floating point values in
>> st(0).
> 
> I thought about using a union in struct kernel_param and removing
> above type cast. This would require modifying the initialization of
> the kernel_param struct via the *_param() macros, though.
> 
> The other possibility would be using __builtin_types_compatible_p()
> to check the function to be of proper type.
> 
> What would you like best?

I'd prefer the union approach; I was actually surprised to see
we still only have a void * there.

Jan


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

Re: [PATCH v2 40/52] xen/arch/x86/apic.c: remove custom_param() error messages

[Jan Beulich]

>>> On 14.08.17 at 09:08, <jgross@suse.com> wrote:
> With _cmdline_parse() now issuing error messages in case of illegal
> parameters signalled by parsing functions specified in custom_param()
> the message issued by apic_set_verbosity() can be removed.
> 
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Juergen Gross <jgross@suse.com>

Acked-by: Jan Beulich <jbeulich@suse.com>



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

Re: [PATCH v2 41/52] xen/arch/x86/cpu/mcheck/mce.c: remove custom_param() error messages

[Jan Beulich]

>>> On 14.08.17 at 09:08, <jgross@suse.com> wrote:
> With _cmdline_parse() now issuing error messages in case of illegal
> parameters signalled by parsing functions specified in custom_param()
> the message issued by mce_set_verbosity() can be removed.
> 
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Juergen Gross <jgross@suse.com>

Acked-by: Jan Beulich <jbeulich@suse.com>



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

Re: [PATCH v2 43/52] xen/arch/x86/io_apic.c: remove custom_param() error messages

[Jan Beulich]

>>> On 14.08.17 at 09:08, <jgross@suse.com> wrote:
> With _cmdline_parse() now issuing error messages in case of illegal
> parameters signalled by parsing functions specified in custom_param()
> the message issued by setup_ioapic_ack() can be removed.
> 
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Juergen Gross <jgross@suse.com>

Acked-by: Jan Beulich <jbeulich@suse.com>



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

Re: [PATCH v2 18/52] xen/arch/x86/psr.c: let custom parameter parsing routines return errno

[Juergen Gross]

On 14/08/17 15:35, Jan Beulich wrote:
>>>> On 14.08.17 at 09:08, <jgross@suse.com> wrote:
>> --- a/xen/arch/x86/psr.c
>> +++ b/xen/arch/x86/psr.c
>> @@ -420,7 +420,7 @@ static const struct feat_props l2_cat_props = {
>>  };
>>  
>>  static void __init parse_psr_bool(char *s, char *value, char *feature,
>> -                                  unsigned int mask)
>> +                                  unsigned int mask, int *rc)
> 
> Please make the function return a value (perhaps bool) instead.

Okay.

> 
>> @@ -451,18 +455,28 @@ static void __init parse_psr_param(char *s)
>>          if ( val_str )
>>              *val_str++ = '\0';
>>  
>> -        parse_psr_bool(s, val_str, "cmt", PSR_CMT);
>> -        parse_psr_bool(s, val_str, "cat", PSR_CAT);
>> -        parse_psr_bool(s, val_str, "cdp", PSR_CDP);
>> +        parse_psr_bool(s, val_str, "cmt", PSR_CMT, &rc);
>> +        parse_psr_bool(s, val_str, "cat", PSR_CAT, &rc);
>> +        parse_psr_bool(s, val_str, "cdp", PSR_CDP, &rc);
>>  
>>          if ( val_str && !strcmp(s, "rmid_max") )
>> -            opt_rmid_max = simple_strtoul(val_str, NULL, 0);
>> +        {
>> +            opt_rmid_max = simple_strtoul(val_str, &q, 0);
>> +            if ( *q )
>> +                rc = -EINVAL;
>> +        }
>>  
>>          if ( val_str && !strcmp(s, "cos_max") )
>> -            opt_cos_max = simple_strtoul(val_str, NULL, 0);
>> +        {
>> +            opt_cos_max = simple_strtoul(val_str, &q, 0);
>> +            if ( *q )
>> +                rc = -EINVAL;
>> +        }
>>  
>>          s = ss + 1;
>>      } while ( ss );
> 
> So if val_str didn't match any of the five strings you won't indicate
> an error here, which seems inconsistent.

Okay, I'll change it.


Juergen

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

Re: [PATCH v2 19/52] xen/arch/x86/setup.c: let custom parameter parsing routines return errno

[Juergen Gross]

On 14/08/17 15:37, Jan Beulich wrote:
>>>> On 14.08.17 at 09:08, <jgross@suse.com> wrote:
>>  bool __read_mostly acpi_disabled;
>>  bool __initdata acpi_force;
>>  static char __initdata acpi_param[10] = "";
>> -static void __init parse_acpi_param(char *s)
>> +static int __init parse_acpi_param(char *s)
> 
> Again please take the opportunity and add the missing blank line
> ahead of the one you modify.

Okay.


Juergen

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

Re: [PATCH v2 01/52] xen/arch/arm/acpi/boot.c: let custom parameter parsing routines return errno

[Julien Grall]

Hi Juergen,

On 14/08/17 08:07, Juergen Gross wrote:
> Modify the custom parameter parsing routines in:
>
> xen/arch/arm/acpi/boot.c
>
> to indicate whether the parameter value was parsed successfully.
>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Julien Grall <julien.grall@arm.com>
> Signed-off-by: Juergen Gross <jgross@suse.com>
> Acked-by: Wei Liu <wei.liu2@citrix.com>

Acked-by: Julien Grall <julien.grall@arm.com>

Cheers,

-- 
Julien Grall

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

Re: [PATCH v2 02/52] xen/arch/arm/domain_build.c: let custom parameter parsing routines return errno

[Julien Grall]

Hi Juergen,

On 14/08/17 08:07, Juergen Gross wrote:
> Modify the custom parameter parsing routines in:
>
> xen/arch/arm/domain_build.c
>
> to indicate whether the parameter value was parsed successfully.
>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Julien Grall <julien.grall@arm.com>
> Signed-off-by: Juergen Gross <jgross@suse.com>
> Acked-by: Wei Liu <wei.liu2@citrix.com>

Acked-by: Julien Grall <julien.grall@arm.com>

Cheers,

-- 
Julien Grall

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

Re: [PATCH v2 03/52] xen/arch/arm/traps.c: let custom parameter parsing routines return errno

[Julien Grall]

Hi Juergen,

On 14/08/17 08:08, Juergen Gross wrote:
> Modify the custom parameter parsing routines in:
>
> xen/arch/arm/traps.c
>
> to indicate whether the parameter value was parsed successfully.
>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Julien Grall <julien.grall@arm.com>
> Signed-off-by: Juergen Gross <jgross@suse.com>
> Acked-by: Wei Liu <wei.liu2@citrix.com>

Acked-by: Julien Grall <julien.grall@arm.com>

Cheers,

-- 
Julien Grall

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

Re: [PATCH v2 20/52] xen/arch/x86/shutdown.c: let custom parameter parsing routines return errno

[Juergen Gross]

On 14/08/17 15:39, Jan Beulich wrote:
>>>> On 14.08.17 at 09:08, <jgross@suse.com> wrote:
>> --- a/xen/arch/x86/shutdown.c
>> +++ b/xen/arch/x86/shutdown.c
>> @@ -51,7 +51,7 @@ static int reboot_mode;
>>   * efi    Use the EFI reboot (if running under EFI)
>>   */
>>  static enum reboot_type reboot_type = BOOT_INVALID;
>> -static void __init set_reboot_type(char *str)
>> +static int __init set_reboot_type(char *str)
>>  {
>>      for ( ; ; )
>>      {
>> @@ -74,6 +74,8 @@ static void __init set_reboot_type(char *str)
>>          case 't':
>>              reboot_type = *str;
>>              break;
>> +        default:
>> +            return -EINVAL;
> 
> This alters behavior - you want to store -EINVAL in a local variable,
> but continue the loop.

Aah, yes. Okay.


Juergen



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

Re: [PATCH v2 22/52] xen/arch/x86/x86_64/mmconfig-shared.c: let custom parameter parsing routines return errno

[Juergen Gross]

On 14/08/17 15:40, Jan Beulich wrote:
>>>> On 14.08.17 at 09:08, <jgross@suse.com> wrote:
>> @@ -37,13 +37,24 @@ static void __init parse_mmcfg(char *s)
>>          if ( ss )
>>              *ss = '\0';
>>  
>> -        if ( !parse_bool(s) )
>> +        switch ( parse_bool(s) ) {
>> +        case 0:
>>              pci_probe &= ~PCI_PROBE_MMCONF;
>> -        else if ( !strcmp(s, "amd_fam10") || !strcmp(s, "amd-fam10") )
>> -            pci_probe |= PCI_CHECK_ENABLE_AMD_MMCONF;
>> +            break;
>> +        case 1:
>> +            break;
>> +        default:
>> +            if ( !strcmp(s, "amd_fam10") || !strcmp(s, "amd-fam10") )
>> +                pci_probe |= PCI_CHECK_ENABLE_AMD_MMCONF;
>> +            else
>> +                return -EINVAL;
>> +            break;
>> +        }
>>  
>>          s = ss + 1;
>>      } while ( ss );
> 
> Same here (and wherever else I may have overlooked it) - you
> should not return out of loops.

Okay.


Juergen


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

Re: [PATCH v2 38/52] xen/xsm/flask/flask_op.c: let custom parameter parsing routines return errno

[Daniel De Graaf]

On 08/14/2017 03:08 AM, Juergen Gross wrote:
> Modify the custom parameter parsing routines in:
> 
> xen/xsm/flask/flask_op.c
> 
> to indicate whether the parameter value was parsed successfully.

Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>

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

Re: [PATCH v2 48/52] xen: add hypercall for setting parameters at runtime

[Daniel De Graaf]

On 08/14/2017 03:08 AM, Juergen Gross wrote:
> Add a sysctl hypercall to support setting parameters similar to
> command line parameters, but at runtime. The parameters to set are
> specified as a string, just like the boot parameters.

Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>

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

Re: [PATCH v2 47/52] xen: add basic support for runtime parameter changing

[Wei Liu]

On Mon, Aug 14, 2017 at 09:08:44AM +0200, Juergen Gross wrote:
> Add the needed infrastructure for runtime parameter changing similar
> to that used at boot time via cmdline. We are using the same parsing
> functions as for cmdline parsing, but with a different array of
> parameter definitions.
> 
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Signed-off-by: Juergen Gross <jgross@suse.com>

Reviewed-by: Wei Liu <wei.liu2@citrix.com>

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

Re: [PATCH v2 39/52] xen: check parameter validity when parsing command line

[Juergen Gross]

On 14/08/17 14:46, Jan Beulich wrote:
>>>> On 14.08.17 at 09:08, <jgross@suse.com> wrote:
>> --- a/xen/common/kernel.c
>> +++ b/xen/common/kernel.c
>>                      optval[-1] = '\0';
>> +                    break;
> 
> Why? Applies to further break-s you add: At least in the past we
> had command line options with two handlers, where each of them
> needed to be invoked. I don't think we should make such impossible
> even if right now there aren't any such examples. Yet if you really
> mean to, then the behavioral change needs to be called out in the
> description.

While working on this I realized that this functionality has been
working only in some cases. The custom parsing functions are being
called with a copy of the option value, which they modify in some
cases. So a second handler being called would see another value as
the first handler, as long as modifying the option value keeps to be
allowed.

I see three possibilities here:

1. don't allow multiple handlers for the same parameter
2. restore the option value before calling each handler (as the
   error message I'm adding with this patch requires access to the
   whole option value this wouldn't be too hard)
3. don't allow a handler to modify the option value (solves my error
   message problem, too)

Any preferences?


Juergen

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

Re: [PATCH v2 39/52] xen: check parameter validity when parsing command line

[Jan Beulich]

>>> On 15.08.17 at 14:54, <jgross@suse.com> wrote:
> On 14/08/17 14:46, Jan Beulich wrote:
>>>>> On 14.08.17 at 09:08, <jgross@suse.com> wrote:
>>> --- a/xen/common/kernel.c
>>> +++ b/xen/common/kernel.c
>>>                      optval[-1] = '\0';
>>> +                    break;
>> 
>> Why? Applies to further break-s you add: At least in the past we
>> had command line options with two handlers, where each of them
>> needed to be invoked. I don't think we should make such impossible
>> even if right now there aren't any such examples. Yet if you really
>> mean to, then the behavioral change needs to be called out in the
>> description.
> 
> While working on this I realized that this functionality has been
> working only in some cases. The custom parsing functions are being
> called with a copy of the option value, which they modify in some
> cases. So a second handler being called would see another value as
> the first handler, as long as modifying the option value keeps to be
> allowed.
> 
> I see three possibilities here:
> 
> 1. don't allow multiple handlers for the same parameter
> 2. restore the option value before calling each handler (as the
>    error message I'm adding with this patch requires access to the
>    whole option value this wouldn't be too hard)
> 3. don't allow a handler to modify the option value (solves my error
>    message problem, too)
> 
> Any preferences?

I have no particular preference between 2 and 3, but both are
better than 1.

Jan


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

Re: [PATCH v2 39/52] xen: check parameter validity when parsing command line

[Wei Liu]

On Tue, Aug 15, 2017 at 02:54:07PM +0200, Juergen Gross wrote:
> On 14/08/17 14:46, Jan Beulich wrote:
> >>>> On 14.08.17 at 09:08, <jgross@suse.com> wrote:
> >> --- a/xen/common/kernel.c
> >> +++ b/xen/common/kernel.c
> >>                      optval[-1] = '\0';
> >> +                    break;
> > 
> > Why? Applies to further break-s you add: At least in the past we
> > had command line options with two handlers, where each of them
> > needed to be invoked. I don't think we should make such impossible
> > even if right now there aren't any such examples. Yet if you really
> > mean to, then the behavioral change needs to be called out in the
> > description.
> 
> While working on this I realized that this functionality has been
> working only in some cases. The custom parsing functions are being
> called with a copy of the option value, which they modify in some
> cases. So a second handler being called would see another value as
> the first handler, as long as modifying the option value keeps to be
> allowed.
> 
> I see three possibilities here:
> 
> 1. don't allow multiple handlers for the same parameter
> 2. restore the option value before calling each handler (as the
>    error message I'm adding with this patch requires access to the
>    whole option value this wouldn't be too hard)
> 3. don't allow a handler to modify the option value (solves my error
>    message problem, too)

Either 2 or 3 please.

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

Re: [PATCH v2 46/52] xen: carve out a generic parsing function from _cmdline_parse()

[Jan Beulich]

>>> On 14.08.17 at 09:08, <jgross@suse.com> wrote:
> In order to support generic parameter parsing carve out the parser from
> _cmdline_parse(). As this generic function might be called after boot
> remove the __init annotations from all called sub-functions.
> 
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Signed-off-by: Juergen Gross <jgross@suse.com>
> Reviewed-by: Wei Liu <wei.liu2@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>
albeit due to the dropping of the __init only together with later
patches actually calling the function post-init.

Jan


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

Re: [PATCH v2 47/52] xen: add basic support for runtime parameter changing

[Jan Beulich]

>>> On 14.08.17 at 09:08, <jgross@suse.com> wrote:
> --- a/xen/arch/x86/xen.lds.S
> +++ b/xen/arch/x86/xen.lds.S
> @@ -226,6 +226,10 @@ SECTIONS
>         __start_schedulers_array = .;
>         *(.data.schedulers)
>         __end_schedulers_array = .;
> +       . = ALIGN(POINTER_ALIGN);
> +       __param_start = .;
> +       *(.data.param)
> +       __param_end = .;
>    } :text

Why do you add this to .data.read_mostly instead of .rodata (or
next to .data.rel, as there may be relocations)? Everything else
looks okay; I'm slightly tempted to ask for ...

> @@ -113,6 +118,19 @@ extern const struct kernel_param __setup_start[], __setup_end[];
>      __kparam __setup_##_var = \
>          { __setup_str_##_var, OPT_STR, sizeof(_var), &_var }
>  
> +#define __rtparam         __param(__dataparam)
> +
> +#define custom_param_runtime(_name, _var) \
> +    __rtparam __rtpar_##_var = { _name, OPT_CUSTOM, 0, _var }
> +#define boolean_param_runtime(_name, _var) \
> +    __rtparam __rtpar_##_var = { _name, OPT_BOOL, sizeof(_var), &_var }
> +#define integer_param_runtime(_name, _var) \
> +    __rtparam __rtpar_##_var = { _name, OPT_UINT, sizeof(_var), &_var }
> +#define size_param_runtime(_name, _var) \
> +    __rtparam __rtpar_##_var = { _name, OPT_SIZE, sizeof(_var), &_var }
> +#define string_param_runtime(_name, _var) \
> +    __rtparam __rtpar_##_var = { _name, OPT_STR, sizeof(_var), &_var }

... these to become xyz_runtime_param(), but that's really minor
and a matter of taste.

Jan


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

Re: [PATCH v2 48/52] xen: add hypercall for setting parameters at runtime

[Jan Beulich]

>>> On 14.08.17 at 09:08, <jgross@suse.com> wrote:
> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -1096,6 +1096,23 @@ struct xen_sysctl_livepatch_op {
>  typedef struct xen_sysctl_livepatch_op xen_sysctl_livepatch_op_t;
>  DEFINE_XEN_GUEST_HANDLE(xen_sysctl_livepatch_op_t);
>  
> +/*
> + * XEN_SYSCTL_set_parameter
> + *
> + * Change hypervisor parameters at runtime.
> + * The input string is parsed similar to the boot parameters.
> + */
> +
> +#define XEN_SET_PARAMETER_MAX_SIZE 1023

Does this really need to be in the public interface, i.e. isn't this limit
an implementation detail?

> +struct xen_sysctl_set_parameter {
> +    XEN_GUEST_HANDLE_64(char) params;       /* IN: pointer to parameters. */
> +    uint16_t size;                          /* IN: size of parameters. Max.
> +                                               XEN_SET_PARAMETER_MAX_SIZE. */

You could even allow querying the size by passing in a null handle
and returning the value in the size field.

> +    uint16_t pad[3];                        /* IN: MUST be zero. */

But you don't check the field is zero afaics.

Jan


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

Re: [PATCH v2 52/52] xen: make some console related parameters settable at runtime

[Jan Beulich]

>>> On 14.08.17 at 09:08, <jgross@suse.com> wrote:
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -41,6 +41,7 @@ string_param("console", opt_console);
>  /*         boots. Any other value, or omitting the char, enables auto-switch 
> */
>  static unsigned char __read_mostly opt_conswitch[3] = "a";
>  string_param("conswitch", opt_conswitch);
> +string_param_runtime("conswitch", opt_conswitch);

Do you envision parameters which can only be set at runtime?
Otherwise, to avoid the two going out of sync (as well as the
redundancy) wouldn't it make sense for xyz_param_runtime()
to do what it does now _and_ invoke xyz_param()?

Jan


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

Re: [PATCH v2 52/52] xen: make some console related parameters settable at runtime

[Juergen Gross]

On 15/08/17 17:45, Jan Beulich wrote:
>>>> On 14.08.17 at 09:08, <jgross@suse.com> wrote:
>> --- a/xen/drivers/char/console.c
>> +++ b/xen/drivers/char/console.c
>> @@ -41,6 +41,7 @@ string_param("console", opt_console);
>>  /*         boots. Any other value, or omitting the char, enables auto-switch 
>> */
>>  static unsigned char __read_mostly opt_conswitch[3] = "a";
>>  string_param("conswitch", opt_conswitch);
>> +string_param_runtime("conswitch", opt_conswitch);
> 
> Do you envision parameters which can only be set at runtime?
> Otherwise, to avoid the two going out of sync (as well as the
> redundancy) wouldn't it make sense for xyz_param_runtime()
> to do what it does now _and_ invoke xyz_param()?

There might be params requiring another handler (e.g. taking a lock,
allocating some memory, ...).

Having a macro for doing both (like above case) seems appropriate.
Any naming ideas? E.g.:

string_param_anytime() ?

Not sure whether I should add ;-)


Juergen


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

Re: [PATCH v2 48/52] xen: add hypercall for setting parameters at runtime

[Juergen Gross]

On 15/08/17 17:39, Jan Beulich wrote:
>>>> On 14.08.17 at 09:08, <jgross@suse.com> wrote:
>> --- a/xen/include/public/sysctl.h
>> +++ b/xen/include/public/sysctl.h
>> @@ -1096,6 +1096,23 @@ struct xen_sysctl_livepatch_op {
>>  typedef struct xen_sysctl_livepatch_op xen_sysctl_livepatch_op_t;
>>  DEFINE_XEN_GUEST_HANDLE(xen_sysctl_livepatch_op_t);
>>  
>> +/*
>> + * XEN_SYSCTL_set_parameter
>> + *
>> + * Change hypervisor parameters at runtime.
>> + * The input string is parsed similar to the boot parameters.
>> + */
>> +
>> +#define XEN_SET_PARAMETER_MAX_SIZE 1023
> 
> Does this really need to be in the public interface, i.e. isn't this limit
> an implementation detail?

Hmm, yes.

> 
>> +struct xen_sysctl_set_parameter {
>> +    XEN_GUEST_HANDLE_64(char) params;       /* IN: pointer to parameters. */
>> +    uint16_t size;                          /* IN: size of parameters. Max.
>> +                                               XEN_SET_PARAMETER_MAX_SIZE. */
> 
> You could even allow querying the size by passing in a null handle
> and returning the value in the size field.

Would this help in any way?

Maybe just returning E2BIG would be enough?

> 
>> +    uint16_t pad[3];                        /* IN: MUST be zero. */
> 
> But you don't check the field is zero afaics.

Right, I should do this.


Juergen

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

Re: [PATCH v2 52/52] xen: make some console related parameters settable at runtime

[Jan Beulich]

>>> On 15.08.17 at 17:52, <jgross@suse.com> wrote:
> On 15/08/17 17:45, Jan Beulich wrote:
>>>>> On 14.08.17 at 09:08, <jgross@suse.com> wrote:
>>> --- a/xen/drivers/char/console.c
>>> +++ b/xen/drivers/char/console.c
>>> @@ -41,6 +41,7 @@ string_param("console", opt_console);
>>>  /*         boots. Any other value, or omitting the char, enables 
> auto-switch 
>>> */
>>>  static unsigned char __read_mostly opt_conswitch[3] = "a";
>>>  string_param("conswitch", opt_conswitch);
>>> +string_param_runtime("conswitch", opt_conswitch);
>> 
>> Do you envision parameters which can only be set at runtime?
>> Otherwise, to avoid the two going out of sync (as well as the
>> redundancy) wouldn't it make sense for xyz_param_runtime()
>> to do what it does now _and_ invoke xyz_param()?
> 
> There might be params requiring another handler (e.g. taking a lock,
> allocating some memory, ...).
> 
> Having a macro for doing both (like above case) seems appropriate.
> Any naming ideas? E.g.:
> 
> string_param_anytime() ?

How about xyz_param_runtime_only() to cover the case where
you really need separate handlers? Yet even then one would have
to specify the string twice, i.e. the name you suggest might be
good to use when it takes two handler arguments.

> Not sure whether I should add ;-)

;-)

Jan


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

Re: [PATCH v2 48/52] xen: add hypercall for setting parameters at runtime

[Jan Beulich]

>>> On 15.08.17 at 17:57, <jgross@suse.com> wrote:
> On 15/08/17 17:39, Jan Beulich wrote:
>>>>> On 14.08.17 at 09:08, <jgross@suse.com> wrote:
>>> +struct xen_sysctl_set_parameter {
>>> +    XEN_GUEST_HANDLE_64(char) params;       /* IN: pointer to parameters. */
>>> +    uint16_t size;                          /* IN: size of parameters. Max.
>>> +                                               XEN_SET_PARAMETER_MAX_SIZE. */
>> 
>> You could even allow querying the size by passing in a null handle
>> and returning the value in the size field.
> 
> Would this help in any way?

Since the caller won't want to dimension the buffer dynamically,
perhaps not much. The only use I could see would be to give a
better error message for too long strings than the one resulting
from strerror(). I.e. ...

> Maybe just returning E2BIG would be enough?

... maybe yes.

Jan


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

Re: [PATCH v2 47/52] xen: add basic support for runtime parameter changing

[Juergen Gross]

On 15/08/17 17:31, Jan Beulich wrote:
>>>> On 14.08.17 at 09:08, <jgross@suse.com> wrote:
>> --- a/xen/arch/x86/xen.lds.S
>> +++ b/xen/arch/x86/xen.lds.S
>> @@ -226,6 +226,10 @@ SECTIONS
>>         __start_schedulers_array = .;
>>         *(.data.schedulers)
>>         __end_schedulers_array = .;
>> +       . = ALIGN(POINTER_ALIGN);
>> +       __param_start = .;
>> +       *(.data.param)
>> +       __param_end = .;
>>    } :text
> 
> Why do you add this to .data.read_mostly instead of .rodata (or
> next to .data.rel, as there may be relocations)? Everything else
> looks okay; I'm slightly tempted to ask for ...

Hmm, I just put it next to the schedulers array. I can move it,
of course.

> 
>> @@ -113,6 +118,19 @@ extern const struct kernel_param __setup_start[], __setup_end[];
>>      __kparam __setup_##_var = \
>>          { __setup_str_##_var, OPT_STR, sizeof(_var), &_var }
>>  
>> +#define __rtparam         __param(__dataparam)
>> +
>> +#define custom_param_runtime(_name, _var) \
>> +    __rtparam __rtpar_##_var = { _name, OPT_CUSTOM, 0, _var }
>> +#define boolean_param_runtime(_name, _var) \
>> +    __rtparam __rtpar_##_var = { _name, OPT_BOOL, sizeof(_var), &_var }
>> +#define integer_param_runtime(_name, _var) \
>> +    __rtparam __rtpar_##_var = { _name, OPT_UINT, sizeof(_var), &_var }
>> +#define size_param_runtime(_name, _var) \
>> +    __rtparam __rtpar_##_var = { _name, OPT_SIZE, sizeof(_var), &_var }
>> +#define string_param_runtime(_name, _var) \
>> +    __rtparam __rtpar_##_var = { _name, OPT_STR, sizeof(_var), &_var }
> 
> ... these to become xyz_runtime_param(), but that's really minor
> and a matter of taste.

I don't mind changing the name according to your suggestion.

Juergen

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

Re: [PATCH v2 52/52] xen: make some console related parameters settable at runtime

[Juergen Gross]

On 15/08/17 17:59, Jan Beulich wrote:
>>>> On 15.08.17 at 17:52, <jgross@suse.com> wrote:
>> On 15/08/17 17:45, Jan Beulich wrote:
>>>>>> On 14.08.17 at 09:08, <jgross@suse.com> wrote:
>>>> --- a/xen/drivers/char/console.c
>>>> +++ b/xen/drivers/char/console.c
>>>> @@ -41,6 +41,7 @@ string_param("console", opt_console);
>>>>  /*         boots. Any other value, or omitting the char, enables 
>> auto-switch 
>>>> */
>>>>  static unsigned char __read_mostly opt_conswitch[3] = "a";
>>>>  string_param("conswitch", opt_conswitch);
>>>> +string_param_runtime("conswitch", opt_conswitch);
>>>
>>> Do you envision parameters which can only be set at runtime?
>>> Otherwise, to avoid the two going out of sync (as well as the
>>> redundancy) wouldn't it make sense for xyz_param_runtime()
>>> to do what it does now _and_ invoke xyz_param()?
>>
>> There might be params requiring another handler (e.g. taking a lock,
>> allocating some memory, ...).
>>
>> Having a macro for doing both (like above case) seems appropriate.
>> Any naming ideas? E.g.:
>>
>> string_param_anytime() ?
> 
> How about xyz_param_runtime_only() to cover the case where
> you really need separate handlers? Yet even then one would have
> to specify the string twice, i.e. the name you suggest might be
> good to use when it takes two handler arguments.

I think for now we could use *_param_runtime() for boot- and
runtime-changes. We can add another set of macros if we need them.


Juergen

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