[PATCH 0/3] parse_bool fixes

[PATCH 0/3] parse_bool fixes

[Don Slutz]

console_timestamps use to be a boolean_param.  Now it is more.  This
patch series is to make old xen command lines with
console_timestamps in them continue to work the way they did at 4.4.


Don Slutz (3):
  cmdline_parse: Also pass bool_assert to OPT_CUSTOM so that parse_bool
    can be used correctly.
  xen/console: Better handing of console_timestamps as a boolean_param
  Adjust parse_vpmu_param use of parse_bool

 xen/arch/arm/domain_build.c              |  2 +-
 xen/arch/x86/apic.c                      |  6 ++--
 xen/arch/x86/cpu/mcheck/mce.c            |  2 +-
 xen/arch/x86/domain_build.c              |  4 +--
 xen/arch/x86/genapic/probe.c             |  2 +-
 xen/arch/x86/hvm/vpmu.c                  | 18 ++++++++----
 xen/arch/x86/io_apic.c                   |  2 +-
 xen/arch/x86/irq.c                       |  4 +--
 xen/arch/x86/microcode.c                 |  2 +-
 xen/arch/x86/nmi.c                       |  2 +-
 xen/arch/x86/numa.c                      |  4 +--
 xen/arch/x86/oprofile/nmi_int.c          |  2 +-
 xen/arch/x86/setup.c                     |  4 +--
 xen/arch/x86/shutdown.c                  |  2 +-
 xen/arch/x86/time.c                      |  2 +-
 xen/arch/x86/x86_64/mmconfig-shared.c    |  2 +-
 xen/common/core_parking.c                |  2 +-
 xen/common/domain.c                      |  2 +-
 xen/common/kernel.c                      |  4 +--
 xen/common/kexec.c                       |  6 ++--
 xen/drivers/acpi/tables.c                |  2 +-
 xen/drivers/char/console.c               | 49 ++++++++++++++++++++++----------
 xen/drivers/cpufreq/cpufreq.c            |  2 +-
 xen/drivers/passthrough/amd/iommu_acpi.c |  4 +--
 xen/drivers/passthrough/iommu.c          |  4 +--
 xen/drivers/passthrough/pci.c            |  4 +--
 xen/drivers/video/vesa.c                 |  2 +-
 27 files changed, 84 insertions(+), 57 deletions(-)

-- 
1.8.4

[PATCH 1/3] cmdline_parse: Also pass bool_assert to OPT_CUSTOM so that parse_bool can be used correctly.

[Don Slutz]

Based on:

commit 2fcb51bc7a4fcb7534265d7bb155c6ddf03952b8
Author: kfraser@localhost.localdomain <kfraser@localhost.localdomain>
Date:   Mon Jul 9 14:29:53 2007 +0100

    Also allow boolean cmdline params to be inverted in two other ways.

    Now, a standard boolean (e.g., watchdog) can be inverted in three
    ways:
     1. no-watchdog
     2. watchdog=no
     3. watchdog=off

    Stacked inversions cancel each other: no-watchdog=no turns on the
    watchdog.

bool_assert is needed to do stacked inversions with parse_bool.

Signed-off-by: Don Slutz <dslutz@verizon.com>
---
 xen/arch/arm/domain_build.c              |  2 +-
 xen/arch/x86/apic.c                      |  6 +++---
 xen/arch/x86/cpu/mcheck/mce.c            |  2 +-
 xen/arch/x86/domain_build.c              |  4 ++--
 xen/arch/x86/genapic/probe.c             |  2 +-
 xen/arch/x86/hvm/vpmu.c                  |  4 ++--
 xen/arch/x86/io_apic.c                   |  2 +-
 xen/arch/x86/irq.c                       |  4 ++--
 xen/arch/x86/microcode.c                 |  2 +-
 xen/arch/x86/nmi.c                       |  2 +-
 xen/arch/x86/numa.c                      |  4 ++--
 xen/arch/x86/oprofile/nmi_int.c          |  2 +-
 xen/arch/x86/setup.c                     |  4 ++--
 xen/arch/x86/shutdown.c                  |  2 +-
 xen/arch/x86/time.c                      |  2 +-
 xen/arch/x86/x86_64/mmconfig-shared.c    |  2 +-
 xen/common/core_parking.c                |  2 +-
 xen/common/domain.c                      |  2 +-
 xen/common/kernel.c                      |  4 ++--
 xen/common/kexec.c                       |  6 +++---
 xen/drivers/acpi/tables.c                |  2 +-
 xen/drivers/char/console.c               | 10 +++++-----
 xen/drivers/cpufreq/cpufreq.c            |  2 +-
 xen/drivers/passthrough/amd/iommu_acpi.c |  4 ++--
 xen/drivers/passthrough/iommu.c          |  4 ++--
 xen/drivers/passthrough/pci.c            |  4 ++--
 xen/drivers/video/vesa.c                 |  2 +-
 27 files changed, 44 insertions(+), 44 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 23261e4..2be7a93 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -30,7 +30,7 @@ int dom0_11_mapping = 1;
 #define DOM0_MEM_DEFAULT 0x8000000 /* 128 MiB */
 static u64 __initdata dom0_mem = DOM0_MEM_DEFAULT;
 
-static void __init parse_dom0_mem(const char *s)
+static void __init parse_dom0_mem(const char *s, int bool_assert)
 {
     dom0_mem = parse_size_and_unit(s, &s);
     if ( dom0_mem == 0 )
diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c
index bbcc0a1..e082ad1 100644
--- a/xen/arch/x86/apic.c
+++ b/xen/arch/x86/apic.c
@@ -809,20 +809,20 @@ int lapic_resume(void)
  * Original code written by Keir Fraser.
  */
 
-static void __init lapic_disable(char *str)
+static void __init lapic_disable(char *str, int bool_assert)
 {
     enable_local_apic = -1;
     setup_clear_cpu_cap(X86_FEATURE_APIC);
 }
 custom_param("nolapic", lapic_disable);
 
-static void __init lapic_enable(char *str)
+static void __init lapic_enable(char *str, int bool_assert)
 {
     enable_local_apic = 1;
 }
 custom_param("lapic", lapic_enable);
 
-static void __init apic_set_verbosity(char *str)
+static void __init apic_set_verbosity(char *str, int bool_assert)
 {
     if (strcmp("debug", str) == 0)
         apic_verbosity = APIC_DEBUG;
diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
index 812daf6..3d7b2e8 100644
--- a/xen/arch/x86/cpu/mcheck/mce.c
+++ b/xen/arch/x86/cpu/mcheck/mce.c
@@ -59,7 +59,7 @@ static int x86_mcerr(const char *msg, int err)
 #endif
 
 int mce_verbosity;
-static void __init mce_set_verbosity(char *str)
+static void __init mce_set_verbosity(char *str, int bool_assert)
 {
     if (strcmp("verbose", str) == 0)
         mce_verbosity = MCE_VERBOSE;
diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c
index d4473c1..c824623 100644
--- a/xen/arch/x86/domain_build.c
+++ b/xen/arch/x86/domain_build.c
@@ -68,7 +68,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 void __init parse_dom0_mem(const char *s, int bool_assert)
 {
     do {
         if ( !strncmp(s, "min:", 4) )
@@ -86,7 +86,7 @@ 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 void __init parse_dom0_max_vcpus(const char *s, int bool_assert)
 {
     if (*s == '-')              /* -M */
         opt_dom0_max_vcpus_max = simple_strtoul(s + 1, &s, 0);
diff --git a/xen/arch/x86/genapic/probe.c b/xen/arch/x86/genapic/probe.c
index a5f2a24..35c8577 100644
--- a/xen/arch/x86/genapic/probe.c
+++ b/xen/arch/x86/genapic/probe.c
@@ -45,7 +45,7 @@ void __init generic_bigsmp_probe(void)
 		}
 }
 
-static void __init genapic_apic_force(char *str)
+static void __init genapic_apic_force(char *str, int bool_assert)
 {
 	int i;
 	for (i = 0; apic_probe[i]; i++)
diff --git a/xen/arch/x86/hvm/vpmu.c b/xen/arch/x86/hvm/vpmu.c
index 63765fa..e02edcf 100644
--- a/xen/arch/x86/hvm/vpmu.c
+++ b/xen/arch/x86/hvm/vpmu.c
@@ -38,12 +38,12 @@
  * "vpmu=bts" : vpmu enabled and Intel BTS feature switched on.
  */
 static unsigned int __read_mostly opt_vpmu_enabled;
-static void parse_vpmu_param(char *s);
+static void parse_vpmu_param(char *s, int bool_assert);
 custom_param("vpmu", parse_vpmu_param);
 
 static DEFINE_PER_CPU(struct vcpu *, last_vcpu);
 
-static void __init parse_vpmu_param(char *s)
+static void __init parse_vpmu_param(char *s, int bool_assert)
 {
     switch ( parse_bool(s) )
     {
diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c
index 4e6fe2b..a62f61b 100644
--- a/xen/arch/x86/io_apic.c
+++ b/xen/arch/x86/io_apic.c
@@ -1579,7 +1579,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 void __init setup_ioapic_ack(char *s, int bool_assert)
 {
     if ( !strcmp(s, "old") )
     {
diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index dafd338..a514397 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 void parse_irq_vector_map_param(char *s, int bool_assert);
 
 /* opt_noirqbalance: If true, software IRQ balancing/affinity is disabled. */
 bool_t __read_mostly opt_noirqbalance = 0;
@@ -60,7 +60,7 @@ 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 void __init parse_irq_vector_map_param(char *s, int bool_assert)
 {
     char *ss;
 
diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c
index 091d5d1..8619ae2 100644
--- a/xen/arch/x86/microcode.c
+++ b/xen/arch/x86/microcode.c
@@ -75,7 +75,7 @@ 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 void __init parse_ucode(char *s, int bool_assert)
 {
     if ( ucode_mod_forced ) /* Forced by EFI */
        return;
diff --git a/xen/arch/x86/nmi.c b/xen/arch/x86/nmi.c
index c4427a6..09938fb 100644
--- a/xen/arch/x86/nmi.c
+++ b/xen/arch/x86/nmi.c
@@ -47,7 +47,7 @@ boolean_param("watchdog", opt_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 void parse_watchdog_timeout(char * s, int bool_assert)
 {
     opt_watchdog_timeout = simple_strtoull(s, NULL, 0);
     opt_watchdog = !!opt_watchdog_timeout;
diff --git a/xen/arch/x86/numa.c b/xen/arch/x86/numa.c
index b141877..7bdb741 100644
--- a/xen/arch/x86/numa.c
+++ b/xen/arch/x86/numa.c
@@ -17,7 +17,7 @@
 #include <asm/acpi.h>
 #include <xen/sched.h>
 
-static int numa_setup(char *s);
+static int numa_setup(char *s, int bool_assert);
 custom_param("numa", numa_setup);
 
 #ifndef Dprintk
@@ -289,7 +289,7 @@ void __cpuinit numa_set_node(int cpu, int node)
 }
 
 /* [numa=off] */
-static __init int numa_setup(char *opt) 
+static __init int numa_setup(char *opt, int bool_assert)
 { 
 	if (!strncmp(opt,"off",3))
 		numa_off = 1;
diff --git a/xen/arch/x86/oprofile/nmi_int.c b/xen/arch/x86/oprofile/nmi_int.c
index eb12fcb..700e39c 100644
--- a/xen/arch/x86/oprofile/nmi_int.c
+++ b/xen/arch/x86/oprofile/nmi_int.c
@@ -323,7 +323,7 @@ static int __init p4_init(char ** cpu_type)
 
 
 static int force_arch_perfmon;
-static int force_cpu_type(const char *str)
+static int force_cpu_type(const char *str, int bool_assert)
 {
 	if (!strcmp(str, "arch_perfmon")) {
 		force_arch_perfmon = 1;
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 599cf04..ec7328a 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -76,7 +76,7 @@ boolean_param("dom0pvh", opt_dom0pvh);
 /* "acpi=strict": Disables out-of-spec workarounds.                 */
 /* "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 void parse_acpi_param(char *s, int bool_assert);
 custom_param("acpi", parse_acpi_param);
 
 /* **** Linux config option: propagated to domain0. */
@@ -110,7 +110,7 @@ unsigned long __read_mostly mmu_cr4_features = XEN_MINIMAL_CR4;
 bool_t __initdata acpi_disabled;
 bool_t __initdata acpi_force;
 static char __initdata acpi_param[10] = "";
-static void __init parse_acpi_param(char *s)
+static void __init parse_acpi_param(char *s, int bool_assert)
 {
     /* Save the parameter so it can be propagated to domain0. */
     safe_strcpy(acpi_param, s);
diff --git a/xen/arch/x86/shutdown.c b/xen/arch/x86/shutdown.c
index 81dfadf..588d902 100644
--- a/xen/arch/x86/shutdown.c
+++ b/xen/arch/x86/shutdown.c
@@ -46,7 +46,7 @@ static int reboot_mode;
  * pci    Use the so-called "PCI reset register", CF9
  */
 static enum reboot_type reboot_type = BOOT_ACPI;
-static void __init set_reboot_type(char *str)
+static void __init set_reboot_type(char *str, int bool_assert)
 {
     for ( ; ; )
     {
diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index a4e1656..efe8ef0 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -1680,7 +1680,7 @@ struct tm wallclock_time(uint64_t *ns)
  * tsc=unstable: Override all tests; assume TSC is unreliable.
  * tsc=skewed: Assume TSCs are individually reliable, but skewed across CPUs.
  */
-static void __init tsc_parse(const char *s)
+static void __init tsc_parse(const char *s, int bool_assert)
 {
     if ( !strcmp(s, "unstable") )
     {
diff --git a/xen/arch/x86/x86_64/mmconfig-shared.c b/xen/arch/x86/x86_64/mmconfig-shared.c
index 742bc18..8fa38b4 100644
--- a/xen/arch/x86/x86_64/mmconfig-shared.c
+++ b/xen/arch/x86/x86_64/mmconfig-shared.c
@@ -29,7 +29,7 @@
 
 unsigned int pci_probe = PCI_PROBE_CONF1 | PCI_PROBE_MMCONF;
 
-static void __init parse_mmcfg(char *s)
+static void __init parse_mmcfg(char *s, int bool_assert)
 {
     char *ss;
 
diff --git a/xen/common/core_parking.c b/xen/common/core_parking.c
index 3190fb7..43c16cf 100644
--- a/xen/common/core_parking.c
+++ b/xen/common/core_parking.c
@@ -41,7 +41,7 @@ static enum core_parking_controller {
     PERFORMANCE_FIRST
 } core_parking_controller = POWER_FIRST;
 
-static void __init setup_core_parking_option(char *str)
+static void __init setup_core_parking_option(char *str, int bool_assert)
 {
     if ( !strcmp(str, "power") )
         core_parking_controller = POWER_FIRST;
diff --git a/xen/common/domain.c b/xen/common/domain.c
index d7a84cf..ac36d83 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -232,7 +232,7 @@ static int late_hwdom_init(struct domain *d)
 
 static unsigned int __read_mostly extra_dom0_irqs = 256;
 static unsigned int __read_mostly extra_domU_irqs = 32;
-static void __init parse_extra_guest_irqs(const char *s)
+static void __init parse_extra_guest_irqs(const char *s, int bool_assert)
 {
     if ( isdigit(*s) )
         extra_domU_irqs = simple_strtoul(s, &s, 0);
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index 7e83353..0b1919b 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -105,7 +105,7 @@ void __init cmdline_parse(const char *cmdline)
                      !strncmp(param->name, opt, q + 1 - opt) )
                 {
                     optval[-1] = '=';
-                    ((void (*)(const char *))param->var)(q);
+                    ((void (*)(const char *, int))param->var)(q, bool_assert);
                     optval[-1] = '\0';
                 }
                 continue;
@@ -135,7 +135,7 @@ void __init cmdline_parse(const char *cmdline)
                     parse_size_and_unit(optval, NULL));
                 break;
             case OPT_CUSTOM:
-                ((void (*)(const char *))param->var)(optval);
+                ((void (*)(const char *, int))param->var)(optval, bool_assert);
                 break;
             default:
                 BUG();
diff --git a/xen/common/kexec.c b/xen/common/kexec.c
index 2239ee8..4afb358 100644
--- a/xen/common/kexec.c
+++ b/xen/common/kexec.c
@@ -96,7 +96,7 @@ static void *crash_heap_current = NULL, *crash_heap_end = NULL;
  *
  *   crashkernel=<size>[@<offset>]
  */
-static void __init parse_crashkernel(const char *str)
+static void __init parse_crashkernel(const char *str, int bool_assert)
 {
     const char *cur;
 
@@ -171,7 +171,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 void __init parse_low_crashinfo(const char * str, int bool_assert)
 {
 
     if ( !strlen(str) )
@@ -197,7 +197,7 @@ 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 void __init parse_crashinfo_maxaddr(const char * str, int bool_assert)
 {
     u64 addr;
 
diff --git a/xen/drivers/acpi/tables.c b/xen/drivers/acpi/tables.c
index 1beca79..d201f50 100644
--- a/xen/drivers/acpi/tables.c
+++ b/xen/drivers/acpi/tables.c
@@ -332,7 +332,7 @@ int __init acpi_table_init(void)
 	return 0;
 }
 
-static int __init acpi_parse_apic_instance(char *str)
+static int __init acpi_parse_apic_instance(char *str, int bool_assert)
 {
 
 	acpi_apic_instance = simple_strtoul(str, NULL, 0);
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 2f6c090..ec871c0 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -60,7 +60,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 void parse_console_timestamps(char *s, int bool_assert);
 custom_param("console_timestamps", parse_console_timestamps);
 
 /* conring_size: allows a large console ring than default (16kB). */
@@ -116,8 +116,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 void parse_loglvl(char *s, int bool_assert);
+static void parse_guest_loglvl(char *s, int bool_assert);
 
 /*
  * <lvl> := none|error|warning|info|debug|all
@@ -158,12 +158,12 @@ static void __init _parse_loglvl(char *s, int *lower, int *upper)
         *upper = *lower;
 }
 
-static void __init parse_loglvl(char *s)
+static void __init parse_loglvl(char *s, int bool_assert)
 {
     _parse_loglvl(s, &xenlog_lower_thresh, &xenlog_upper_thresh);
 }
 
-static void __init parse_guest_loglvl(char *s)
+static void __init parse_guest_loglvl(char *s, int bool_assert)
 {
     _parse_loglvl(s, &xenlog_guest_lower_thresh, &xenlog_guest_upper_thresh);
 }
diff --git a/xen/drivers/cpufreq/cpufreq.c b/xen/drivers/cpufreq/cpufreq.c
index ab66884..fe9e08c 100644
--- a/xen/drivers/cpufreq/cpufreq.c
+++ b/xen/drivers/cpufreq/cpufreq.c
@@ -63,7 +63,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 void __init setup_cpufreq_option(char *str, int bool_assert)
 {
     char *arg;
 
diff --git a/xen/drivers/passthrough/amd/iommu_acpi.c b/xen/drivers/passthrough/amd/iommu_acpi.c
index 5634eac..568e47f 100644
--- a/xen/drivers/passthrough/amd/iommu_acpi.c
+++ b/xen/drivers/passthrough/amd/iommu_acpi.c
@@ -636,7 +636,7 @@ static u16 __init parse_ivhd_device_extended_range(
 
 static DECLARE_BITMAP(ioapic_cmdline, ARRAY_SIZE(ioapic_sbdf)) __initdata;
 
-static void __init parse_ivrs_ioapic(char *str)
+static void __init parse_ivrs_ioapic(char *str, int bool_assert)
 {
     const char *s = str;
     unsigned long id;
@@ -657,7 +657,7 @@ static void __init parse_ivrs_ioapic(char *str)
 }
 custom_param("ivrs_ioapic[", parse_ivrs_ioapic);
 
-static void __init parse_ivrs_hpet(char *str)
+static void __init parse_ivrs_hpet(char *str, int bool_assert)
 {
     const char *s = str;
     unsigned long id;
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index cc12735..2e04e47 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -23,7 +23,7 @@
 #include <xen/keyhandler.h>
 #include <xsm/xsm.h>
 
-static void parse_iommu_param(char *s);
+static void parse_iommu_param(char *s, int bool_assert);
 static void iommu_dump_p2m_table(unsigned char key);
 
 /*
@@ -67,7 +67,7 @@ static struct keyhandler iommu_p2m_table = {
     .desc = "dump iommu p2m table"
 };
 
-static void __init parse_iommu_param(char *s)
+static void __init parse_iommu_param(char *s, int bool_assert)
 {
     char *ss;
     int val;
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 1eba833..afa9a1a 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -129,7 +129,7 @@ static struct phantom_dev {
 } phantom_devs[8];
 static unsigned int nr_phantom_devs;
 
-static void __init parse_phantom_dev(char *str) {
+static void __init parse_phantom_dev(char *str, int bool_assert) {
     const char *s = str;
     unsigned int seg, bus, slot;
     struct phantom_dev phantom;
@@ -169,7 +169,7 @@ 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 void __init parse_pci_param(char *s, int bool_assert)
 {
     char *ss;
 
diff --git a/xen/drivers/video/vesa.c b/xen/drivers/video/vesa.c
index 575db62..5a591ae 100644
--- a/xen/drivers/video/vesa.c
+++ b/xen/drivers/video/vesa.c
@@ -30,7 +30,7 @@ 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 void __init parse_font_height(const char *s, int bool_assert)
 {
     if ( simple_strtoul(s, &s, 10) == 8 && (*s++ == 'x') )
         font_height = simple_strtoul(s, &s, 10);
-- 
1.8.4

Re: [PATCH 1/3] cmdline_parse: Also pass bool_assert to OPT_CUSTOM so that parse_bool can be used correctly.

[Jan Beulich]

>>> On 28.07.14 at 17:59, <dslutz@verizon.com> wrote:
> --- a/xen/arch/x86/apic.c
> +++ b/xen/arch/x86/apic.c
> @@ -809,20 +809,20 @@ int lapic_resume(void)
>   * Original code written by Keir Fraser.
>   */
>  
> -static void __init lapic_disable(char *str)
> +static void __init lapic_disable(char *str, int bool_assert)
>  {
>      enable_local_apic = -1;
>      setup_clear_cpu_cap(X86_FEATURE_APIC);
>  }
>  custom_param("nolapic", lapic_disable);
>  
> -static void __init lapic_enable(char *str)
> +static void __init lapic_enable(char *str, int bool_assert)
>  {
>      enable_local_apic = 1;
>  }
>  custom_param("lapic", lapic_enable);
>  
> -static void __init apic_set_verbosity(char *str)
> +static void __init apic_set_verbosity(char *str, int bool_assert)
>  {
>      if (strcmp("debug", str) == 0)
>          apic_verbosity = APIC_DEBUG;

This adding of a new parameter to all custom parameter parsers,
with rarely any actually using it is a no-go as far as I'm concerned.
That said, being of boolean type, this would need to be bool_t
anyway.

Jan

Re: [PATCH 1/3] cmdline_parse: Also pass bool_assert to OPT_CUSTOM so that parse_bool can be used correctly.

[Don Slutz]

On 07/28/14 12:06, Jan Beulich wrote:
>>>> On 28.07.14 at 17:59, <dslutz@verizon.com> wrote:
>> --- a/xen/arch/x86/apic.c
>> +++ b/xen/arch/x86/apic.c
>> @@ -809,20 +809,20 @@ int lapic_resume(void)
>>    * Original code written by Keir Fraser.
>>    */
>>   
>> -static void __init lapic_disable(char *str)
>> +static void __init lapic_disable(char *str, int bool_assert)
>>   {
>>       enable_local_apic = -1;
>>       setup_clear_cpu_cap(X86_FEATURE_APIC);
>>   }
>>   custom_param("nolapic", lapic_disable);
>>   
>> -static void __init lapic_enable(char *str)
>> +static void __init lapic_enable(char *str, int bool_assert)
>>   {
>>       enable_local_apic = 1;
>>   }
>>   custom_param("lapic", lapic_enable);
>>   
>> -static void __init apic_set_verbosity(char *str)
>> +static void __init apic_set_verbosity(char *str, int bool_assert)
>>   {
>>       if (strcmp("debug", str) == 0)
>>           apic_verbosity = APIC_DEBUG;
> This adding of a new parameter to all custom parameter parsers,
> with rarely any actually using it is a no-go as far as I'm concerned.
> That said, being of boolean type, this would need to be bool_t
> anyway.

I considered adding a new custom type, but when this way.  I would think 
that if a custom parameter parser is ignoring the "no-" (which they all do)
they should be reporting on it.


I.E. "no-lapic" currently means "lapic" and "no-nolapic" means "nolapic"
with out any message to that effect.

     -Don Slutz

> Jan
>

Re: [PATCH 1/3] cmdline_parse: Also pass bool_assert to OPT_CUSTOM so that parse_bool can be used correctly.

[Keir Fraser]

Don Slutz wrote:
>> This adding of a new parameter to all custom parameter parsers,
>> with rarely any actually using it is a no-go as far as I'm concerned.
>> That said, being of boolean type, this would need to be bool_t
>> anyway.
>
> I considered adding a new custom type, but when this way.  I would think
> that if a custom parameter parser is ignoring the "no-" (which they all do)
> they should be reporting on it.
>
> I.E. "no-lapic" currently means "lapic" and "no-nolapic" means "nolapic"
> with out any message to that effect.

The better fix here would be to reject options such as "no-lapic" as 
invalid. This could be done within cmdline_parse() by rejecting matches 
on anything other than OPT_{BOOL,INVBOOL} options if optkey has a "no-" 
prefix.

Adding a bool parameter to every custom parameter parser is not really 
going to fly.

  -- Keir

Re: [PATCH 1/3] cmdline_parse: Also pass bool_assert to OPT_CUSTOM so that parse_bool can be used correctly.

[Don Slutz]

On 07/28/14 15:19, Keir Fraser wrote:
> Don Slutz wrote:
>>> This adding of a new parameter to all custom parameter parsers,
>>> with rarely any actually using it is a no-go as far as I'm concerned.
>>> That said, being of boolean type, this would need to be bool_t
>>> anyway.
>>
>> I considered adding a new custom type, but when this way.  I would think
>> that if a custom parameter parser is ignoring the "no-" (which they 
>> all do)
>> they should be reporting on it.
>>
>> I.E. "no-lapic" currently means "lapic" and "no-nolapic" means "nolapic"
>> with out any message to that effect.
>
> The better fix here would be to reject options such as "no-lapic" as 
> invalid. This could be done within cmdline_parse() by rejecting 
> matches on anything other than OPT_{BOOL,INVBOOL} options if optkey 
> has a "no-" prefix.
>

I read this as reject "no-" prefix for all custom parameters. Not
sure which way to go.  Jan says (on a different thread):


>>> On 28.07.14 at 18:18, <dslutz@verizon.com> wrote:
> The simpler case of no-console_timestamps no longer works also.

Which I believe would be made work by faking up a "no" string when
calling the custom handler, without touching dozens of files.

Jan




I was reading this as "no-lapic" is the same as "lapic=no".  I can reject
"no-lapic=no" (or any other "=<value>")for custom parameters (since

commit 6328e728f6d6589a10d7e9f97a47f566f63743c2
Author: Andrew Cooper <Andrew.Cooper3@citrix.com>
Date:   Mon Sep 3 11:22:00 2012 +0100

     docs/command line: Clarify the behavior with invalid input.

) says that:


Explicitly specifying any value other than those listed above is
undefined, as is stacking a `no-` prefix with an explicit value.


    -Don Slutz

> Adding a bool parameter to every custom parameter parser is not really 
> going to fly.
>
>  -- Keir

Re: [PATCH 1/3] cmdline_parse: Also pass bool_assert to OPT_CUSTOM so that parse_bool can be used correctly.

[Jan Beulich]

>>> On 28.07.14 at 22:30, <dslutz@verizon.com> wrote:
> On 07/28/14 15:19, Keir Fraser wrote:
>> Don Slutz wrote:
>>>> This adding of a new parameter to all custom parameter parsers,
>>>> with rarely any actually using it is a no-go as far as I'm concerned.
>>>> That said, being of boolean type, this would need to be bool_t
>>>> anyway.
>>>
>>> I considered adding a new custom type, but when this way.  I would think
>>> that if a custom parameter parser is ignoring the "no-" (which they 
>>> all do)
>>> they should be reporting on it.
>>>
>>> I.E. "no-lapic" currently means "lapic" and "no-nolapic" means "nolapic"
>>> with out any message to that effect.
>>
>> The better fix here would be to reject options such as "no-lapic" as 
>> invalid. This could be done within cmdline_parse() by rejecting 
>> matches on anything other than OPT_{BOOL,INVBOOL} options if optkey 
>> has a "no-" prefix.
>>
> 
> I read this as reject "no-" prefix for all custom parameters. Not
> sure which way to go.  Jan says (on a different thread):

I'd say we should allow what is sensible, and reject nonsense. E.g.
"no-" on any option that has a value (following a = separator) makes
no sense. But custom options allowing other than boolean values
as their value can, when no value (and no = separator) was given,
meaningfully be prefixed by "no-", which could be resolved by the
faked up "=no" value I was suggesting in my earlier reply (at once
dealing with "no-" prefixes on custom options not allowing boolean
values: their parsing routine will simply reject the faked up "no").

Jan

Re: [PATCH 1/3] cmdline_parse: Also pass bool_assert to OPT_CUSTOM so that parse_bool can be used correctly.

[Don Slutz]

On 07/29/14 02:35, Jan Beulich wrote:
>>>> On 28.07.14 at 22:30, <dslutz@verizon.com> wrote:
>> On 07/28/14 15:19, Keir Fraser wrote:
>>> Don Slutz wrote:


> I'd say we should allow what is sensible, and reject nonsense. E.g.
> "no-" on any option that has a value (following a = separator) makes
> no sense. But custom options allowing other than boolean values
> as their value can, when no value (and no = separator) was given,
> meaningfully be prefixed by "no-", which could be resolved by the
> faked up "=no" value I was suggesting in my earlier reply (at once
> dealing with "no-" prefixes on custom options not allowing boolean
> values: their parsing routine will simply reject the faked up "no").

v2 posted with this suggestion.

    -Don Slutz

> Jan
>

Re: [PATCH 1/3] cmdline_parse: Also pass bool_assert to OPT_CUSTOM so that parse_bool can be used correctly.

[Andrew Cooper]

[-- Attachment #1.1: Type: text/plain, Size: 875 bytes --]

On 28/07/14 16:59, Don Slutz wrote:
> Based on:
>
> commit 2fcb51bc7a4fcb7534265d7bb155c6ddf03952b8
> Author: kfraser@localhost.localdomain <kfraser@localhost.localdomain>
> Date:   Mon Jul 9 14:29:53 2007 +0100
>
>     Also allow boolean cmdline params to be inverted in two other ways.
>
>     Now, a standard boolean (e.g., watchdog) can be inverted in three
>     ways:
>      1. no-watchdog
>      2. watchdog=no
>      3. watchdog=off
>
>     Stacked inversions cancel each other: no-watchdog=no turns on the
>     watchdog.
>
> bool_assert is needed to do stacked inversions with parse_bool.
>
> Signed-off-by: Don Slutz <dslutz@verizon.com>

Why do we need to care about stacked inversions?

>From xen-command-line.markdown:

"Explicitly specifying any value other than those listed above is
undefined, as is stacking a |no-| prefix with an explicit value."

~Andrew

[-- Attachment #1.2: Type: text/html, Size: 1535 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH 1/3] cmdline_parse: Also pass bool_assert to OPT_CUSTOM so that parse_bool can be used correctly.

[Don Slutz]

[-- Attachment #1.1: Type: text/plain, Size: 1306 bytes --]


On 07/28/14 12:08, Andrew Cooper wrote:
> On 28/07/14 16:59, Don Slutz wrote:
>> Based on:
>>
>> commit 2fcb51bc7a4fcb7534265d7bb155c6ddf03952b8
>> Author:kfraser@localhost.localdomain  <kfraser@localhost.localdomain>
>> Date:   Mon Jul 9 14:29:53 2007 +0100
>>
>>      Also allow boolean cmdline params to be inverted in two other ways.
>>
>>      Now, a standard boolean (e.g., watchdog) can be inverted in three
>>      ways:
>>       1. no-watchdog
>>       2. watchdog=no
>>       3. watchdog=off
>>
>>      Stacked inversions cancel each other: no-watchdog=no turns on the
>>      watchdog.
>>
>> bool_assert is needed to do stacked inversions with parse_bool.
>>
>> Signed-off-by: Don Slutz<dslutz@verizon.com>
>
> Why do we need to care about stacked inversions?
>
> From xen-command-line.markdown:
>
> "Explicitly specifying any value other than those listed above is 
> undefined, as is stacking a |no-| prefix with an explicit value."
>

The command line:

no-console_timestamps dom0_mem=3G loglvl=all guest_loglvl=all 
com1=9600,8n1 console=com1 crashkernel=256M@256M no-console_timestamps=no

in 4.4 and before ends up with date style console_timestamps.  In xen 
4.5 you get no console_timestamps.

The simpler case of no-console_timestamps no longer works also.

    -Don Slutz

> ~Andrew


[-- Attachment #1.2: Type: text/html, Size: 2468 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH 1/3] cmdline_parse: Also pass bool_assert to OPT_CUSTOM so that parse_bool can be used correctly.

[Andrew Cooper]

[-- Attachment #1.1: Type: text/plain, Size: 1700 bytes --]

On 28/07/14 17:18, Don Slutz wrote:
>
> On 07/28/14 12:08, Andrew Cooper wrote:
>> On 28/07/14 16:59, Don Slutz wrote:
>>> Based on:
>>>
>>> commit 2fcb51bc7a4fcb7534265d7bb155c6ddf03952b8
>>> Author: kfraser@localhost.localdomain <kfraser@localhost.localdomain>
>>> Date:   Mon Jul 9 14:29:53 2007 +0100
>>>
>>>     Also allow boolean cmdline params to be inverted in two other ways.
>>>
>>>     Now, a standard boolean (e.g., watchdog) can be inverted in three
>>>     ways:
>>>      1. no-watchdog
>>>      2. watchdog=no
>>>      3. watchdog=off
>>>
>>>     Stacked inversions cancel each other: no-watchdog=no turns on the
>>>     watchdog.
>>>
>>> bool_assert is needed to do stacked inversions with parse_bool.
>>>
>>> Signed-off-by: Don Slutz <dslutz@verizon.com>
>>
>> Why do we need to care about stacked inversions?
>>
>> From xen-command-line.markdown:
>>
>> "Explicitly specifying any value other than those listed above is
>> undefined, as is stacking a |no-| prefix with an explicit value."
>>
>
> The command line:
>
> no-console_timestamps dom0_mem=3G loglvl=all guest_loglvl=all
> com1=9600,8n1 console=com1 crashkernel=256M@256M no-console_timestamps=no
>
> in 4.4 and before ends up with date style console_timestamps.  In xen
> 4.5 you get no console_timestamps. 
>
> The simpler case of no-console_timestamps no longer works also.
>
>    -Don Slutz

Ok, so my "compat for older console_timestamp boolparam" needs improving
a bit.

However, "no-console_timestamps=no" is undefined, and we make no
guarentees as to what happens if you give repeated (different) command
line options to Xen.  Both of these are user/admin error and should not
be worked around in Xen code.

~Andrew

[-- Attachment #1.2: Type: text/html, Size: 2993 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH 1/3] cmdline_parse: Also pass bool_assert to OPT_CUSTOM so that parse_bool can be used correctly.

[Don Slutz]

On 07/28/14 12:23, Andrew Cooper wrote:
> On 28/07/14 17:18, Don Slutz wrote:
>>
>> On 07/28/14 12:08, Andrew Cooper wrote:
>>> On 28/07/14 16:59, Don Slutz wrote:
>>>> Based on:
>>>>
>>>> commit 2fcb51bc7a4fcb7534265d7bb155c6ddf03952b8
>>>> Author:kfraser@localhost.localdomain  <kfraser@localhost.localdomain>
>>>> Date:   Mon Jul 9 14:29:53 2007 +0100
>>>>
>>>>      Also allow boolean cmdline params to be inverted in two other ways.
>>>>
>>>>      Now, a standard boolean (e.g., watchdog) can be inverted in three
>>>>      ways:
>>>>       1. no-watchdog
>>>>       2. watchdog=no
>>>>       3. watchdog=off
>>>>
>>>>      Stacked inversions cancel each other: no-watchdog=no turns on the
>>>>      watchdog.
>>>>
>>>> bool_assert is needed to do stacked inversions with parse_bool.
>>>>
>>>> Signed-off-by: Don Slutz<dslutz@verizon.com>
>>>
>>> Why do we need to care about stacked inversions?
>>>
>>> From xen-command-line.markdown:
>>>
>>> "Explicitly specifying any value other than those listed above is 
>>> undefined, as is stacking a |no-| prefix with an explicit value."


Ah, I see that:


commit 6328e728f6d6589a10d7e9f97a47f566f63743c2
Author: Andrew Cooper <Andrew.Cooper3@citrix.com>
Date:   Mon Sep 3 11:22:00 2012 +0100

     docs/command line: Clarify the behavior with invalid input.

     Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
     Acked-by: Jan Beulich <jbeulich@suse.de>
     Committed-by: Ian Campbell <ian.campbell@citrix.com>

was when this changed.

>> The command line:
>>
>> no-console_timestamps dom0_mem=3G loglvl=all guest_loglvl=all 
>> com1=9600,8n1 console=com1 crashkernel=256M@256M no-console_timestamps=no
>>
>> in 4.4 and before ends up with date style console_timestamps. In xen 
>> 4.5 you get no console_timestamps.
>>
>> The simpler case of no-console_timestamps no longer works also.
>>
>>    -Don Slutz
>
> Ok, so my "compat for older console_timestamp boolparam" needs 
> improving a bit.
>

Yes, that is what patch #2 does.


> However, "no-console_timestamps=no" is undefined, and we make no 
> guarentees as to what happens if you give repeated (different) command 
> line options to Xen.  Both of these are user/admin error and should 
> not be worked around in Xen code.
>
> ~Andrew

Ok,  I was not aware of this change in "defined" behavior.  I will 
re-work this
to include Jan's 'pass "no" if "no-" removed.'

    -Don Slutz

Re: [PATCH 1/3] cmdline_parse: Also pass bool_assert to OPT_CUSTOM so that parse_bool can be used correctly.

[Jan Beulich]

>>> On 28.07.14 at 18:18, <dslutz@verizon.com> wrote:
> The simpler case of no-console_timestamps no longer works also.

Which I believe would be made work by faking up a "no" string when
calling the custom handler, without touching dozens of files.

Jan

[PATCH 2/3] xen/console: Better handing of console_timestamps as a boolean_param

[Don Slutz]

In order to handle all the old ways, change to use parse_bool().

Signed-off-by: Don Slutz <dslutz@verizon.com>
---
 xen/drivers/char/console.c | 39 +++++++++++++++++++++++++++++----------
 1 file changed, 29 insertions(+), 10 deletions(-)

diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index ec871c0..44d6a2b 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -557,17 +557,36 @@ static int printk_prefix_check(char *p, char **pp)
             ((loglvl < upper_thresh) && printk_ratelimit()));
 } 
 
-static void __init parse_console_timestamps(char *s)
+static void __init parse_console_timestamps(char *s, int bool_assert)
 {
-    if ( *s == '\0' || /* Compat for old booleanparam() */
-         !strcmp(s, "date") )
-        opt_con_timestamp_mode = TSM_DATE;
-    else if ( !strcmp(s, "datems") )
-        opt_con_timestamp_mode = TSM_DATE_MS;
-    else if ( !strcmp(s, "boot") )
-        opt_con_timestamp_mode = TSM_BOOT;
-    else if ( !strcmp(s, "none") )
-        opt_con_timestamp_mode = TSM_NONE;
+    switch ( parse_bool(s) )
+    {
+    case 0:
+        bool_assert = !bool_assert;
+        /* fall through */
+    case 1:
+        if ( bool_assert )
+            opt_con_timestamp_mode = TSM_DATE;
+        else
+            opt_con_timestamp_mode = TSM_NONE;
+        break;
+    default:
+        if ( *s == '\0' ) /* Compat for old booleanparam() */
+        {
+            if ( bool_assert )
+                opt_con_timestamp_mode = TSM_DATE;
+            else
+                opt_con_timestamp_mode = TSM_NONE;
+        }
+        else if ( !strcmp(s, "date") )
+            opt_con_timestamp_mode = TSM_DATE;
+        else if ( !strcmp(s, "datems") )
+            opt_con_timestamp_mode = TSM_DATE_MS;
+        else if ( !strcmp(s, "boot") )
+            opt_con_timestamp_mode = TSM_BOOT;
+        else if ( !strcmp(s, "none") )
+            opt_con_timestamp_mode = TSM_NONE;
+    }
 }
 
 static void printk_start_of_line(const char *prefix)
-- 
1.8.4

[PATCH 3/3] Adjust parse_vpmu_param use of parse_bool

[Don Slutz]

Signed-off-by: Don Slutz <dslutz@verizon.com>
---
 xen/arch/x86/hvm/vpmu.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/hvm/vpmu.c b/xen/arch/x86/hvm/vpmu.c
index e02edcf..fb99748 100644
--- a/xen/arch/x86/hvm/vpmu.c
+++ b/xen/arch/x86/hvm/vpmu.c
@@ -48,6 +48,13 @@ static void __init parse_vpmu_param(char *s, int bool_assert)
     switch ( parse_bool(s) )
     {
     case 0:
+        bool_assert = !bool_assert;
+        /* fall through */
+    case 1:
+        if ( bool_assert )
+            opt_vpmu_enabled |= VPMU_BOOT_ENABLED;
+	else
+            opt_vpmu_enabled &= ~VPMU_BOOT_ENABLED;
         break;
     default:
         if ( !strcmp(s, "bts") )
@@ -57,9 +64,10 @@ static void __init parse_vpmu_param(char *s, int bool_assert)
             printk("VPMU: unknown flag: %s - vpmu disabled!\n", s);
             break;
         }
-        /* fall through */
-    case 1:
-        opt_vpmu_enabled |= VPMU_BOOT_ENABLED;
+        if ( bool_assert )
+            opt_vpmu_enabled |= VPMU_BOOT_ENABLED;
+	else
+            opt_vpmu_enabled &= ~VPMU_BOOT_ENABLED;
         break;
     }
 }
-- 
1.8.4