[PATCH v9 0/8] reduce memory consumption for powerpc firmware-assisted capture kernel

[PATCH v9 0/8] reduce memory consumption for powerpc firmware-assisted capture kernel

[Hari Bathini]

I posted the initial version [1] of patchset [2] adding support to enforce
additional parameters when firmware-assisted dump capture kernel is active.
Michal reposted it with few improvements to parameter processing to make
it more robust. He further posted patchset [3] with few more improvements.

This patch series clubs the work from these two patch-sets while segregating
the generic and arch-specific code to ease the review process.

[1] http://patchwork.ozlabs.org/patch/758176/
[2] http://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=2733
[3] http://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=3338

---

Hari Bathini (2):
      powerpc/fadump: reduce memory consumption for capture kernel
      powerpc/fadump: update documentation about 'fadump_extra_args=' parameter

Michal Suchanek (6):
      lib/cmdline.c: remove quotes symmetrically
      boot/param: add pointer to current and next argument to unknown parameter callback
      lib/cmdline.c: add backslash support to kernel commandline parsing
      Documentation/admin-guide: backslash support in commandline
      lib/cmdline.c: implement single quotes in commandline argument parsing
      Documentation/admin-guide: single quotes in kernel arguments


 Documentation/admin-guide/kernel-parameters.rst  |    5 +
 Documentation/powerpc/firmware-assisted-dump.txt |   20 ++++-
 arch/powerpc/include/asm/fadump.h                |    2 
 arch/powerpc/kernel/fadump.c                     |   97 +++++++++++++++++++++-
 arch/powerpc/kernel/prom.c                       |    7 ++
 include/linux/moduleparam.h                      |    1 
 init/main.c                                      |    8 +-
 kernel/module.c                                  |    5 +
 kernel/params.c                                  |   18 +++-
 lib/cmdline.c                                    |   54 +++++++-----
 lib/dynamic_debug.c                              |    1 
 11 files changed, 179 insertions(+), 39 deletions(-)

[PATCH v9 1/8] lib/cmdline.c: remove quotes symmetrically

[Hari Bathini]

From: Michal Suchanek <msuchanek@suse.de>

Remove quotes from argument value only if there is qoute on both sides.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 lib/cmdline.c |   10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/lib/cmdline.c b/lib/cmdline.c
index 171c19b..6d398a8 100644
--- a/lib/cmdline.c
+++ b/lib/cmdline.c
@@ -227,14 +227,12 @@ char *next_arg(char *args, char **param, char **val)
 		*val = args + equals + 1;
 
 		/* Don't include quotes in value. */
-		if (**val == '"') {
-			(*val)++;
-			if (args[i-1] == '"')
-				args[i-1] = '\0';
+		if ((args[i-1] == '"') && ((quoted) || (**val == '"'))) {
+			args[i-1] = '\0';
+			if (!quoted)
+				(*val)++;
 		}
 	}
-	if (quoted && args[i-1] == '"')
-		args[i-1] = '\0';
 
 	if (args[i]) {
 		args[i] = '\0';

Re: [PATCH v9 1/8] lib/cmdline.c: remove quotes symmetrically

[Michal Suchánek]

On Wed, 15 Nov 2017 20:46:56 +0530
Hari Bathini <hbathini@linux.vnet.ibm.com> wrote:

> From: Michal Suchanek <msuchanek@suse.de>
> 
> Remove quotes from argument value only if there is qoute on both
> sides.
> 
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
>  lib/cmdline.c |   10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/cmdline.c b/lib/cmdline.c
> index 171c19b..6d398a8 100644
> --- a/lib/cmdline.c
> +++ b/lib/cmdline.c
> @@ -227,14 +227,12 @@ char *next_arg(char *args, char **param, char
> **val) *val = args + equals + 1;
>  
>  		/* Don't include quotes in value. */
> -		if (**val == '"') {
> -			(*val)++;
> -			if (args[i-1] == '"')
> -				args[i-1] = '\0';
> +		if ((args[i-1] == '"') && ((quoted) || (**val ==
> '"'))) {
> +			args[i-1] = '\0';
> +			if (!quoted)
> +				(*val)++;
>  		}
>  	}
> -	if (quoted && args[i-1] == '"')
> -		args[i-1] = '\0';
>  
>  	if (args[i]) {
>  		args[i] = '\0';
> 

This was only useful as separate patch with the incremental fadump
update. Since the fadump update is squashed in this refresh series this
can be squashed with the following lib/cmdline patch as well.

Thanks

Michal

[PATCH v9 2/8] boot/param: add pointer to current and next argument to unknown parameter callback

[Hari Bathini]

From: Michal Suchanek <msuchanek@suse.de>

Add pointer to current and next argument to make parameter processing
more robust. This can make parameter processing easier and less error
prone in cases where the parameters need to be enforced/ignored based
on firmware/system state.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
---

Changes in v9:
* Fixed messages like below observed while loading modules with no parameters.
    - iptable_filter: unknown parameter '' ignored
    - ip_tables: unknown parameter '' ignored


 include/linux/moduleparam.h |    1 +
 init/main.c                 |    8 ++++++--
 kernel/module.c             |    5 +++--
 kernel/params.c             |   18 ++++++++++++------
 lib/dynamic_debug.c         |    1 +
 5 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index 1d7140f..50a19e6 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -328,6 +328,7 @@ extern char *parse_args(const char *name,
 		      s16 level_max,
 		      void *arg,
 		      int (*unknown)(char *param, char *val,
+				     char *currant, char *next,
 				     const char *doing, void *arg));
 
 /* Called by module remove. */
diff --git a/init/main.c b/init/main.c
index 3bdd8da..3ba3eed 100644
--- a/init/main.c
+++ b/init/main.c
@@ -241,6 +241,7 @@ early_param("loglevel", loglevel);
 
 /* Change NUL term back to "=", to make "param" the whole string. */
 static int __init repair_env_string(char *param, char *val,
+				    char *unused3, char *unused2,
 				    const char *unused, void *arg)
 {
 	if (val) {
@@ -259,6 +260,7 @@ static int __init repair_env_string(char *param, char *val,
 
 /* Anything after -- gets handed straight to init. */
 static int __init set_init_arg(char *param, char *val,
+			       char *unused3, char *unused2,
 			       const char *unused, void *arg)
 {
 	unsigned int i;
@@ -266,7 +268,7 @@ static int __init set_init_arg(char *param, char *val,
 	if (panic_later)
 		return 0;
 
-	repair_env_string(param, val, unused, NULL);
+	repair_env_string(param, val, unused3, unused2, unused, NULL);
 
 	for (i = 0; argv_init[i]; i++) {
 		if (i == MAX_INIT_ARGS) {
@@ -284,9 +286,10 @@ static int __init set_init_arg(char *param, char *val,
  * unused parameters (modprobe will find them in /proc/cmdline).
  */
 static int __init unknown_bootoption(char *param, char *val,
+				     char *unused3, char *unused2,
 				     const char *unused, void *arg)
 {
-	repair_env_string(param, val, unused, NULL);
+	repair_env_string(param, val, unused3, unused2, unused, NULL);
 
 	/* Handle obsolete-style parameters */
 	if (obsolete_checksetup(param))
@@ -438,6 +441,7 @@ static noinline void __ref rest_init(void)
 
 /* Check for early params. */
 static int __init do_early_param(char *param, char *val,
+				 char *unused3, char *unused2,
 				 const char *unused, void *arg)
 {
 	const struct obs_kernel_param *p;
diff --git a/kernel/module.c b/kernel/module.c
index 32c2cda..ffe7520 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3619,8 +3619,9 @@ static int prepare_coming_module(struct module *mod)
 	return 0;
 }
 
-static int unknown_module_param_cb(char *param, char *val, const char *modname,
-				   void *arg)
+static int unknown_module_param_cb(char *param, char *val,
+				   char *unused, char *unused2,
+				   const char *modname, void *arg)
 {
 	struct module *mod = arg;
 	int ret;
diff --git a/kernel/params.c b/kernel/params.c
index cc9108c..69ff58e 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -119,6 +119,8 @@ static void param_check_unsafe(const struct kernel_param *kp)
 
 static int parse_one(char *param,
 		     char *val,
+		     char *currant,
+		     char *next,
 		     const char *doing,
 		     const struct kernel_param *params,
 		     unsigned num_params,
@@ -126,7 +128,8 @@ static int parse_one(char *param,
 		     s16 max_level,
 		     void *arg,
 		     int (*handle_unknown)(char *param, char *val,
-				     const char *doing, void *arg))
+					   char *currant, char *next,
+					   const char *doing, void *arg))
 {
 	unsigned int i;
 	int err;
@@ -153,7 +156,7 @@ static int parse_one(char *param,
 
 	if (handle_unknown) {
 		pr_debug("doing %s: %s='%s'\n", doing, param, val);
-		return handle_unknown(param, val, doing, arg);
+		return handle_unknown(param, val, currant, next, doing, arg);
 	}
 
 	pr_debug("Unknown argument '%s'\n", param);
@@ -169,9 +172,10 @@ char *parse_args(const char *doing,
 		 s16 max_level,
 		 void *arg,
 		 int (*unknown)(char *param, char *val,
+				char *currant, char *next,
 				const char *doing, void *arg))
 {
-	char *param, *val, *err = NULL;
+	char *param, *val, *next, *err = NULL;
 
 	/* Chew leading spaces */
 	args = skip_spaces(args);
@@ -179,16 +183,18 @@ char *parse_args(const char *doing,
 	if (*args)
 		pr_debug("doing %s, parsing ARGS: '%s'\n", doing, args);
 
-	while (*args) {
+	next = next_arg(args, &param, &val);
+	while (*next) {
 		int ret;
 		int irq_was_disabled;
 
-		args = next_arg(args, &param, &val);
+		args = next;
+		next = next_arg(args, &param, &val);
 		/* Stop at -- */
 		if (!val && strcmp(param, "--") == 0)
 			return err ?: args;
 		irq_was_disabled = irqs_disabled();
-		ret = parse_one(param, val, doing, params, num,
+		ret = parse_one(param, val, args, next, doing, params, num,
 				min_level, max_level, arg, unknown);
 		if (irq_was_disabled && !irqs_disabled())
 			pr_warn("%s: option '%s' enabled irq's!\n",
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index da796e2..dec7f40 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -889,6 +889,7 @@ static int ddebug_dyndbg_param_cb(char *param, char *val,
 
 /* handle both dyndbg and $module.dyndbg params at boot */
 static int ddebug_dyndbg_boot_param_cb(char *param, char *val,
+				char *unused3, char *unused2,
 				const char *unused, void *arg)
 {
 	vpr_info("%s=\"%s\"\n", param, val);

Re: [PATCH v9 2/8] boot/param: add pointer to current and next argument to unknown parameter callback

[Michal Suchánek]

Hello,

On Wed, 15 Nov 2017 20:47:14 +0530
Hari Bathini <hbathini@linux.vnet.ibm.com> wrote:

> From: Michal Suchanek <msuchanek@suse.de>
> 
> Add pointer to current and next argument to make parameter processing
> more robust. This can make parameter processing easier and less error
> prone in cases where the parameters need to be enforced/ignored based
> on firmware/system state.
> 
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>

> @@ -179,16 +183,18 @@ char *parse_args(const char *doing,
>  	if (*args)
>  		pr_debug("doing %s, parsing ARGS: '%s'\n", doing,
> args); 
> -	while (*args) {
> +	next = next_arg(args, &param, &val);
> +	while (*next) {
>  		int ret;
>  		int irq_was_disabled;
>  
> -		args = next_arg(args, &param, &val);
> +		args = next;
> +		next = next_arg(args, &param, &val);
>  		/* Stop at -- */

The [PATCH v8 5/6] you refreshed here moves the while(*next) to the end
of the cycle for a reason. Checking *args at the start is mostly
equivalent checking *next at the end. Checking *next at the start on
the other hand skips the last argument.

The "mostly" part is that there is a bug here because *args is not
checked at the start of the cycle making it possible to crash if it is
0. To fix that the if(*args) above should be extended to wrap the cycle.

Thanks

Michal

[PATCH] Fix parse_args cycle limit check.

[Michal Suchanek]

Actually args are supposed to be renamed to next so both and args hold the
previous argument so both can be passed to the callback. This additionla patch
should fix up the rename.

---
 kernel/params.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/kernel/params.c b/kernel/params.c
index 69ff58e69887..efb4dfaa6bc5 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -182,17 +182,18 @@ char *parse_args(const char *doing,
 
 	if (*args)
 		pr_debug("doing %s, parsing ARGS: '%s'\n", doing, args);
+	else
+		return err;
 
-	next = next_arg(args, &param, &val);
-	while (*next) {
+	do {
 		int ret;
 		int irq_was_disabled;
 
-		args = next;
 		next = next_arg(args, &param, &val);
+
 		/* Stop at -- */
 		if (!val && strcmp(param, "--") == 0)
-			return err ?: args;
+			return err ?: next;
 		irq_was_disabled = irqs_disabled();
 		ret = parse_one(param, val, args, next, doing, params, num,
 				min_level, max_level, arg, unknown);
@@ -215,9 +216,10 @@ char *parse_args(const char *doing,
 			       doing, val ?: "", param);
 			break;
 		}
-
 		err = ERR_PTR(ret);
-	}
+
+		args = next;
+	} while (*args);
 
 	return err;
 }
-- 
2.13.6

Re: [PATCH] Fix parse_args cycle limit check.

[Randy Dunlap]

On 12/15/2017 01:41 PM, Michal Suchanek wrote:
> Actually args are supposed to be renamed to next so both and args hold the
> previous argument so both can be passed to the callback. This additionla patch

                                                                additional

> should fix up the rename.

Would you try rewriting the first sentence, please? I don't get it.

> ---
>  kernel/params.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)


-- 
~Randy

Re: [PATCH] Fix parse_args cycle limit check.

[Michal Suchánek]

On Fri, 15 Dec 2017 15:49:09 -0800
Randy Dunlap <rdunlap@infradead.org> wrote:

> On 12/15/2017 01:41 PM, Michal Suchanek wrote:
> > Actually args are supposed to be renamed to next so both and args
> > hold the previous argument so both can be passed to the callback.
> > This additionla patch  
> 
>                                                                 additional
> 
> > should fix up the rename.  
> 
> Would you try rewriting the first sentence, please? I don't get it.

Ok, I guess this should be clarified. For the original patch and the
fixup squashed together this is what the patch is supposed to do:

This patch adds variable for tracking the parameter which is currently
being processed. There is "args" variable which tracks the parameter
which will be processed next so this patch adds "next" variable to
track that and uses "args" to track the current argument.

Thanks

Michal

Re: [PATCH] Fix parse_args cycle limit check.

[Randy Dunlap]

On 12/18/2017 09:34 AM, Michal Suchánek wrote:
> On Fri, 15 Dec 2017 15:49:09 -0800
> Randy Dunlap <rdunlap@infradead.org> wrote:
> 
>> On 12/15/2017 01:41 PM, Michal Suchanek wrote:
>>> Actually args are supposed to be renamed to next so both and args
>>> hold the previous argument so both can be passed to the callback.
>>> This additionla patch  
>>
>>                                                                 additional
>>
>>> should fix up the rename.  
>>
>> Would you try rewriting the first sentence, please? I don't get it.
> 
> Ok, I guess this should be clarified. For the original patch and the
> fixup squashed together this is what the patch is supposed to do:
> 
> This patch adds variable for tracking the parameter which is currently
> being processed. There is "args" variable which tracks the parameter
> which will be processed next so this patch adds "next" variable to
> track that and uses "args" to track the current argument.

OK, thanks.


-- 
~Randy

[PATCH v9 3/8] lib/cmdline.c: add backslash support to kernel commandline parsing

[Hari Bathini]

From: Michal Suchanek <msuchanek@suse.de>

This allows passing quotes in kernel arguments. It is useful for passing
nested arguemnts and might be useful if somebody wanted to pass a double
quote directly as part of an argument. It is also useful to have quoting
grammar more similar to shells and bootloaders.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
---
 lib/cmdline.c |   41 ++++++++++++++++++++---------------------
 1 file changed, 20 insertions(+), 21 deletions(-)

diff --git a/lib/cmdline.c b/lib/cmdline.c
index 6d398a8..d98bdc0 100644
--- a/lib/cmdline.c
+++ b/lib/cmdline.c
@@ -193,30 +193,36 @@ bool parse_option_str(const char *str, const char *option)
 
 /*
  * Parse a string to get a param value pair.
- * You can use " around spaces, but can't escape ".
+ * You can use " around spaces, and you can escape with \
  * Hyphens and underscores equivalent in parameter names.
  */
 char *next_arg(char *args, char **param, char **val)
 {
 	unsigned int i, equals = 0;
-	int in_quote = 0, quoted = 0;
+	int in_quote = 0, backslash = 0;
 	char *next;
 
-	if (*args == '"') {
-		args++;
-		in_quote = 1;
-		quoted = 1;
-	}
-
 	for (i = 0; args[i]; i++) {
-		if (isspace(args[i]) && !in_quote)
+		if (isspace(args[i]) && !in_quote && !backslash)
 			break;
-		if (equals == 0) {
-			if (args[i] == '=')
-				equals = i;
+
+		if ((equals == 0) && (args[i] == '='))
+			equals = i;
+
+		if (!backslash) {
+			if ((args[i] == '"') || (args[i] == '\\')) {
+				if (args[i] == '"')
+					in_quote = !in_quote;
+				if (args[i] == '\\')
+					backslash = 1;
+
+				memmove(args + 1, args, i);
+				args++;
+				i--;
+			}
+		} else {
+			backslash = 0;
 		}
-		if (args[i] == '"')
-			in_quote = !in_quote;
 	}
 
 	*param = args;
@@ -225,13 +231,6 @@ char *next_arg(char *args, char **param, char **val)
 	else {
 		args[equals] = '\0';
 		*val = args + equals + 1;
-
-		/* Don't include quotes in value. */
-		if ((args[i-1] == '"') && ((quoted) || (**val == '"'))) {
-			args[i-1] = '\0';
-			if (!quoted)
-				(*val)++;
-		}
 	}
 
 	if (args[i]) {

[PATCH] init/main.c: simplify repair_env_string

[Michal Suchanek]

Quoting characters are now removed from the parameter so value always
follows directly after the NUL terminating parameter name.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 init/main.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

Since the previous "[PATCH v9 3/8] lib/cmdline.c: add backslash support to
kernel commandline parsing" adds the memmove in lib/cmdline.c it is now
superfluous in init/main.c

diff --git a/init/main.c b/init/main.c
index 1f5fdedbb293..1e5b1dc940d9 100644
--- a/init/main.c
+++ b/init/main.c
@@ -244,15 +244,10 @@ static int __init repair_env_string(char *param, char *val,
 				    const char *unused, void *arg)
 {
 	if (val) {
-		/* param=val or param="val"? */
-		if (val == param+strlen(param)+1)
-			val[-1] = '=';
-		else if (val == param+strlen(param)+2) {
-			val[-2] = '=';
-			memmove(val-1, val, strlen(val)+1);
-			val--;
-		} else
-			BUG();
+		int parm_len = strlen(param);
+
+		param[parm_len] = '=';
+		BUG_ON(val != param + parm_len + 1);
 	}
 	return 0;
 }
-- 
2.13.6

Re: [PATCH] init/main.c: simplify repair_env_string

[Steven Rostedt]

Cleaning out my INBOX, I stumbled across this old patch.

On Fri, 15 Dec 2017 22:33:17 +0100
Michal Suchanek <msuchanek@suse.de> wrote:

> Quoting characters are now removed from the parameter so value always
> follows directly after the NUL terminating parameter name.
> 
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
>  init/main.c | 13 ++++---------
>  1 file changed, 4 insertions(+), 9 deletions(-)
> 
> Since the previous "[PATCH v9 3/8] lib/cmdline.c: add backslash support to
> kernel commandline parsing" adds the memmove in lib/cmdline.c it is now
> superfluous in init/main.c

I don't believe the above patches were ever applied. Were they?

-- Steve

> 
> diff --git a/init/main.c b/init/main.c
> index 1f5fdedbb293..1e5b1dc940d9 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -244,15 +244,10 @@ static int __init repair_env_string(char *param, char *val,
>  				    const char *unused, void *arg)
>  {
>  	if (val) {
> -		/* param=val or param="val"? */
> -		if (val == param+strlen(param)+1)
> -			val[-1] = '=';
> -		else if (val == param+strlen(param)+2) {
> -			val[-2] = '=';
> -			memmove(val-1, val, strlen(val)+1);
> -			val--;
> -		} else
> -			BUG();
> +		int parm_len = strlen(param);
> +
> +		param[parm_len] = '=';
> +		BUG_ON(val != param + parm_len + 1);
>  	}
>  	return 0;
>  }

Re: [PATCH] init/main.c: simplify repair_env_string

[Michal Suchánek]

On Fri, 4 May 2018 11:40:35 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> Cleaning out my INBOX, I stumbled across this old patch.
> 
> On Fri, 15 Dec 2017 22:33:17 +0100
> Michal Suchanek <msuchanek@suse.de> wrote:
> 
> > Quoting characters are now removed from the parameter so value
> > always follows directly after the NUL terminating parameter name.
> > 
> > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > ---
> >  init/main.c | 13 ++++---------
> >  1 file changed, 4 insertions(+), 9 deletions(-)
> > 
> > Since the previous "[PATCH v9 3/8] lib/cmdline.c: add backslash
> > support to kernel commandline parsing" adds the memmove in
> > lib/cmdline.c it is now superfluous in init/main.c  
> 
> I don't believe the above patches were ever applied. Were they?

No, they weren't.

The reason to write them was to support the fadump_extra_args argument
with peculiar semantics that required the quoting cleanup in kernel
argument parsing.

A different solution for the fadump memory consumption is in the works
so I dropped this. There was lack of interest from reviewers, too.

Thanks

Michal

> 
> -- Steve
> 
> > 
> > diff --git a/init/main.c b/init/main.c
> > index 1f5fdedbb293..1e5b1dc940d9 100644
> > --- a/init/main.c
> > +++ b/init/main.c
> > @@ -244,15 +244,10 @@ static int __init repair_env_string(char
> > *param, char *val, const char *unused, void *arg)
> >  {
> >  	if (val) {
> > -		/* param=val or param="val"? */
> > -		if (val == param+strlen(param)+1)
> > -			val[-1] = '=';
> > -		else if (val == param+strlen(param)+2) {
> > -			val[-2] = '=';
> > -			memmove(val-1, val, strlen(val)+1);
> > -			val--;
> > -		} else
> > -			BUG();
> > +		int parm_len = strlen(param);
> > +
> > +		param[parm_len] = '=';
> > +		BUG_ON(val != param + parm_len + 1);
> >  	}
> >  	return 0;
> >  }  
> 

[PATCH v9 4/8] Documentation/admin-guide: backslash support in commandline

[Hari Bathini]

From: Michal Suchanek <msuchanek@suse.de>

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 Documentation/admin-guide/kernel-parameters.rst |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.rst b/Documentation/admin-guide/kernel-parameters.rst
index b2598cc..722d3f7 100644
--- a/Documentation/admin-guide/kernel-parameters.rst
+++ b/Documentation/admin-guide/kernel-parameters.rst
@@ -35,9 +35,9 @@ can also be entered as::
 
 	log-buf-len=1M print_fatal_signals=1
 
-Double-quotes can be used to protect spaces in values, e.g.::
+Double-quotes and backslashes can be used to protect spaces in values, e.g.::
 
-	param="spaces in here"
+	param="spaces in here" param2=spaces\ in\ here
 
 cpu lists:
 ----------

[PATCH v9 5/8] lib/cmdline.c: implement single quotes in commandline argument parsing

[Hari Bathini]

From: Michal Suchanek <msuchanek@suse.de>

This brings the kernel parser about on par with bourne shell, grub, and
other tools that chew the arguments before kernel does.

This should make it easier to deal with multiple levels of
nesting/quoting. With same quoting grammar on each level there is less
room for confusion.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 lib/cmdline.c |   29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/lib/cmdline.c b/lib/cmdline.c
index d98bdc0..c5335a7 100644
--- a/lib/cmdline.c
+++ b/lib/cmdline.c
@@ -191,34 +191,45 @@ bool parse_option_str(const char *str, const char *option)
 	return false;
 }
 
+#define squash_char { \
+	memmove(args + 1, args, i); \
+	args++; \
+	i--; \
+}
+
 /*
  * Parse a string to get a param value pair.
- * You can use " around spaces, and you can escape with \
+ * You can use " or ' around spaces, and you can escape with \
  * Hyphens and underscores equivalent in parameter names.
  */
 char *next_arg(char *args, char **param, char **val)
 {
 	unsigned int i, equals = 0;
-	int in_quote = 0, backslash = 0;
+	int in_quote = 0, backslash = 0, in_single = 0;
 	char *next;
 
 	for (i = 0; args[i]; i++) {
-		if (isspace(args[i]) && !in_quote && !backslash)
+		if (isspace(args[i]) && !in_quote && !backslash && !in_single)
 			break;
 
 		if ((equals == 0) && (args[i] == '='))
 			equals = i;
 
-		if (!backslash) {
-			if ((args[i] == '"') || (args[i] == '\\')) {
+		if (in_single) {
+			if (args[i] == '\'') {
+				in_single = 0;
+				squash_char;
+			}
+		} else if (!backslash) {
+			if ((args[i] == '"') || (args[i] == '\\') ||
+					(args[i] == '\'')) {
 				if (args[i] == '"')
 					in_quote = !in_quote;
 				if (args[i] == '\\')
 					backslash = 1;
-
-				memmove(args + 1, args, i);
-				args++;
-				i--;
+				if (args[i] == '\'')
+					in_single = 1;
+				squash_char;
 			}
 		} else {
 			backslash = 0;

[PATCH] Optimize final quote removal.

[Michal Suchanek]

This is additional patch that avoids the memmove when processing the quote on
the end of the parameter.

---
 lib/cmdline.c                                   | 9 +++++++--
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/lib/cmdline.c b/lib/cmdline.c
index c5335a79a177..b1d8a0dc60fc 100644
--- a/lib/cmdline.c
+++ b/lib/cmdline.c
@@ -191,7 +191,13 @@ bool parse_option_str(const char *str, const char *option)
 	return false;
 }
 
+#define break_arg_end(i) { \
+	if (isspace(args[i]) && !in_quote && !backslash && !in_single) \
+		break; \
+	}
+
 #define squash_char { \
+	break_arg_end(i + 1); \
 	memmove(args + 1, args, i); \
 	args++; \
 	i--; \
@@ -209,8 +215,7 @@ char *next_arg(char *args, char **param, char **val)
 	char *next;
 
 	for (i = 0; args[i]; i++) {
-		if (isspace(args[i]) && !in_quote && !backslash && !in_single)
-			break;
+		break_arg_end(i);
 
 		if ((equals == 0) && (args[i] == '='))
 			equals = i;
-- 
2.13.6

[PATCH v9 6/8] Documentation/admin-guide: single quotes in kernel arguments

[Hari Bathini]

From: Michal Suchanek <msuchanek@suse.de>

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 Documentation/admin-guide/kernel-parameters.rst |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.rst b/Documentation/admin-guide/kernel-parameters.rst
index 722d3f7..1f98372 100644
--- a/Documentation/admin-guide/kernel-parameters.rst
+++ b/Documentation/admin-guide/kernel-parameters.rst
@@ -35,9 +35,10 @@ can also be entered as::
 
 	log-buf-len=1M print_fatal_signals=1
 
-Double-quotes and backslashes can be used to protect spaces in values, e.g.::
+Double-quotes single-quotes and backslashes can be used to protect spaces
+in values, e.g.::
 
-	param="spaces in here" param2=spaces\ in\ here
+	param="spaces in here" param2=spaces\ in\ here param3='@%# !\'
 
 cpu lists:
 ----------

[PATCH v9 7/8] powerpc/fadump: reduce memory consumption for capture kernel

[Hari Bathini]

With fadump (dump capture) kernel booting like a regular kernel, it needs
almost the same amount of memory to boot as the production kernel, which is
unwarranted for a dump capture kernel. But with no option to disable some
of the unnecessary subsystems in fadump kernel, that much memory is wasted
on fadump, depriving the production kernel of that memory.

Introduce kernel parameter 'fadump_extra_args=' that would take regular
parameters as a space separated quoted string, to be enforced when fadump
is active. This 'fadump_extra_args=' parameter can be leveraged to pass
parameters like nr_cpus=1, cgroup_disable=memory and numa=off, to disable
unwarranted resources/subsystems.

Also, ensure the log "Firmware-assisted dump is active" is printed early
in the boot process to put the subsequent fadump messages in context.

Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 arch/powerpc/include/asm/fadump.h |    2 +
 arch/powerpc/kernel/fadump.c      |   97 ++++++++++++++++++++++++++++++++++++-
 arch/powerpc/kernel/prom.c        |    7 +++
 3 files changed, 103 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/include/asm/fadump.h b/arch/powerpc/include/asm/fadump.h
index 5a23010..41b50b3 100644
--- a/arch/powerpc/include/asm/fadump.h
+++ b/arch/powerpc/include/asm/fadump.h
@@ -208,12 +208,14 @@ extern int early_init_dt_scan_fw_dump(unsigned long node,
 		const char *uname, int depth, void *data);
 extern int fadump_reserve_mem(void);
 extern int setup_fadump(void);
+extern void enforce_fadump_extra_args(char *cmdline);
 extern int is_fadump_active(void);
 extern int should_fadump_crash(void);
 extern void crash_fadump(struct pt_regs *, const char *);
 extern void fadump_cleanup(void);
 
 #else	/* CONFIG_FA_DUMP */
+static inline void enforce_fadump_extra_args(char *cmdline) { }
 static inline int is_fadump_active(void) { return 0; }
 static inline int should_fadump_crash(void) { return 0; }
 static inline void crash_fadump(struct pt_regs *regs, const char *str) { }
diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index e143180..275ea42 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c
@@ -78,8 +78,10 @@ int __init early_init_dt_scan_fw_dump(unsigned long node,
 	 * dump data waiting for us.
 	 */
 	fdm_active = of_get_flat_dt_prop(node, "ibm,kernel-dump", NULL);
-	if (fdm_active)
+	if (fdm_active) {
+		pr_info("Firmware-assisted dump is active.\n");
 		fw_dump.dump_active = 1;
+	}
 
 	/* Get the sizes required to store dump data for the firmware provided
 	 * dump sections.
@@ -339,8 +341,11 @@ int __init fadump_reserve_mem(void)
 {
 	unsigned long base, size, memory_boundary;
 
-	if (!fw_dump.fadump_enabled)
+	if (!fw_dump.fadump_enabled) {
+		if (fw_dump.dump_active)
+			pr_warn("Firmware-assisted dump was active but kernel booted with fadump disabled!\n");
 		return 0;
+	}
 
 	if (!fw_dump.fadump_supported) {
 		printk(KERN_INFO "Firmware-assisted dump is not supported on"
@@ -380,7 +385,6 @@ int __init fadump_reserve_mem(void)
 		memory_boundary = memblock_end_of_DRAM();
 
 	if (fw_dump.dump_active) {
-		printk(KERN_INFO "Firmware-assisted dump is active.\n");
 		/*
 		 * If last boot has crashed then reserve all the memory
 		 * above boot_memory_size so that we don't touch it until
@@ -467,6 +471,93 @@ static int __init early_fadump_reserve_mem(char *p)
 }
 early_param("fadump_reserve_mem", early_fadump_reserve_mem);
 
+#define FADUMP_EXTRA_ARGS_PARAM		"fadump_extra_args="
+#define FADUMP_EXTRA_ARGS_LEN		(strlen(FADUMP_EXTRA_ARGS_PARAM) - 1)
+
+struct param_info {
+	char		*cmdline;
+	char		*tmp_cmdline;
+	int		 shortening;
+};
+
+static void __init fadump_update_params(struct param_info *param_info,
+					char *param, char *val,
+					char *currant, char *next)
+{
+	ptrdiff_t param_offset = currant - param_info->tmp_cmdline;
+	size_t vallen = val ? strlen(val) : 0;
+	char *tgt = param_info->cmdline + param_offset
+				- param_info->shortening;
+	int shortening = ((next - 1) - (currant))
+		- (FADUMP_EXTRA_ARGS_LEN + 1 + vallen);
+
+	if (!val)
+		return;
+
+	strncpy(tgt, FADUMP_EXTRA_ARGS_PARAM, FADUMP_EXTRA_ARGS_LEN);
+	tgt += FADUMP_EXTRA_ARGS_LEN;
+	*tgt++ = ' ';
+	strncpy(tgt, val, vallen);
+	tgt += vallen;
+
+	if (shortening) {
+		char *src = tgt + shortening;
+		memmove(tgt, src, strlen(src) + 1);
+	}
+
+	param_info->shortening += shortening;
+}
+
+/*
+ * Reworks command line parameters and splits 'fadump_extra_args=' param
+ * to enforce the parameters passed through it
+ */
+static int __init fadump_rework_cmdline_params(char *param, char *val,
+					       char *currant, char *next,
+					       const char *unused, void *arg)
+{
+	struct param_info *param_info = (struct param_info *)arg;
+
+	if (strncmp(param, FADUMP_EXTRA_ARGS_PARAM,
+		     strlen(FADUMP_EXTRA_ARGS_PARAM) - 1))
+		return 0;
+
+	fadump_update_params(param_info, param, val, currant, next);
+
+	return 0;
+}
+
+/*
+ * Replace every occurrence of 'fadump_extra_args="param1 param2 param3"'
+ * in cmdline with 'fadump_extra_args param1 param2 param3' by stripping
+ * off '=' and quotes, if any. This ensures that the additional parameters
+ * passed with 'fadump_extra_args=' are enforced.
+ */
+void __init enforce_fadump_extra_args(char *cmdline)
+{
+	static char tmp_cmdline[COMMAND_LINE_SIZE] __initdata;
+	static char init_cmdline[COMMAND_LINE_SIZE] __initdata;
+	struct param_info param_info;
+
+	if (strstr(cmdline, FADUMP_EXTRA_ARGS_PARAM) == NULL)
+		return;
+
+	pr_info("Modifying command line to enforce the additional parameters passed through 'fadump_extra_args='");
+
+	param_info.cmdline = cmdline;
+	param_info.tmp_cmdline = tmp_cmdline;
+	param_info.shortening = 0;
+
+	strlcpy(init_cmdline, cmdline, COMMAND_LINE_SIZE);
+
+	strlcpy(tmp_cmdline, cmdline, COMMAND_LINE_SIZE);
+	parse_args("fadump params", tmp_cmdline, NULL, 0, 0, 0,
+			&param_info, &fadump_rework_cmdline_params);
+
+	pr_info("Original command line: %s\n", init_cmdline);
+	pr_info("Modified command line: %s\n", cmdline);
+}
+
 static int register_fw_dump(struct fadump_mem_struct *fdm)
 {
 	int rc, err;
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index f830562..2e6f4021 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -693,6 +693,13 @@ void __init early_init_devtree(void *params)
 	of_scan_flat_dt(early_init_dt_scan_root, NULL);
 	of_scan_flat_dt(early_init_dt_scan_memory_ppc, NULL);
 
+	/*
+	 * Look for 'fadump_extra_args=' parameter and enfore the additional
+	 * parameters passed to it if fadump is active.
+	 */
+	if (is_fadump_active())
+		enforce_fadump_extra_args(boot_command_line);
+
 	parse_early_param();
 
 	/* make sure we've parsed cmdline for mem= before this */

[PATCH v9 8/8] powerpc/fadump: update documentation about 'fadump_extra_args=' parameter

[Hari Bathini]

With the introduction of 'fadump_extra_args=' parameter to pass additional
parameters to fadump (capture) kernel, update documentation about it.

Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 Documentation/powerpc/firmware-assisted-dump.txt |   20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/Documentation/powerpc/firmware-assisted-dump.txt b/Documentation/powerpc/firmware-assisted-dump.txt
index bdd344a..5705f55 100644
--- a/Documentation/powerpc/firmware-assisted-dump.txt
+++ b/Documentation/powerpc/firmware-assisted-dump.txt
@@ -162,7 +162,19 @@ How to enable firmware-assisted dump (fadump):
 
 1. Set config option CONFIG_FA_DUMP=y and build kernel.
 2. Boot into linux kernel with 'fadump=on' kernel cmdline option.
-3. Optionally, user can also set 'crashkernel=' kernel cmdline
+3. A user can pass additional command line parameters as a space
+   separated quoted list through 'fadump_extra_args=' parameter,
+   to be enforced when fadump is active. For example, parameter
+   'fadump_extra_args="nr_cpus=1 numa=off udev.children-max=2"'
+   will be changed to 'fadump_extra_args nr_cpus=1  numa=off
+   udev.children-max=2' in-place when fadump is active. This
+   parameter has no affect when fadump is not active. Multiple
+   instances of 'fadump_extra_args=' can be passed. This provision
+   can be used to reduce memory consumption during dump capture by
+   disabling unwarranted resources/subsystems like CPUs, NUMA
+   and such. Value with spaces can be passed as
+   'fadump_extra_args="parameter=\"value with spaces\""'
+4. Optionally, user can also set 'crashkernel=' kernel cmdline
    to specify size of the memory to reserve for boot memory dump
    preservation.
 
@@ -172,6 +184,12 @@ NOTE: 1. 'fadump_reserve_mem=' parameter has been deprecated. Instead
       2. If firmware-assisted dump fails to reserve memory then it
          will fallback to existing kdump mechanism if 'crashkernel='
          option is set at kernel cmdline.
+      3. Special parameters like '--' passed inside fadump_extra_args are also
+         just left in-place. So, the user is advised to consider this while
+         specifying such parameters. It may be required to quote the argument
+         to fadump_extra_args when the bootloader uses double-quotes as
+         argument delimiter as well. eg
+        append = " fadump_extra_args=\"nr_cpus=1 numa=off udev.children-max=2\""
 
 Sysfs/debugfs files:
 ------------

Re: [PATCH v9 0/8] reduce memory consumption for powerpc firmware-assisted capture kernel

[Christophe Leroy]

Le 15/11/2017 à 16:16, Hari Bathini a écrit :
> I posted the initial version [1] of patchset [2] adding support to enforce
> additional parameters when firmware-assisted dump capture kernel is active.
> Michal reposted it with few improvements to parameter processing to make
> it more robust. He further posted patchset [3] with few more improvements.
> 
> This patch series clubs the work from these two patch-sets while segregating
> the generic and arch-specific code to ease the review process.
> 
> [1] http://patchwork.ozlabs.org/patch/758176/
> [2] http://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=2733
> [3] http://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=3338
> 

Hello,

We still have this series flagged as "new" in powerpc's patchwork.

It is still of interest ?

Patch 1 applies but patch 2 hardly conflicts in init/main.c

So I'll flag it as "change requested", fell free to resubmit if still 
relevant.

Or if you think that's still required but don't plan to handle it, tell 
me and I'll open an issue in our issue board so that we don't loose history.

Thanks
Christophe


> ---
> 
> Hari Bathini (2):
>        powerpc/fadump: reduce memory consumption for capture kernel
>        powerpc/fadump: update documentation about 'fadump_extra_args=' parameter
> 
> Michal Suchanek (6):
>        lib/cmdline.c: remove quotes symmetrically
>        boot/param: add pointer to current and next argument to unknown parameter callback
>        lib/cmdline.c: add backslash support to kernel commandline parsing
>        Documentation/admin-guide: backslash support in commandline
>        lib/cmdline.c: implement single quotes in commandline argument parsing
>        Documentation/admin-guide: single quotes in kernel arguments
> 
> 
>   Documentation/admin-guide/kernel-parameters.rst  |    5 +
>   Documentation/powerpc/firmware-assisted-dump.txt |   20 ++++-
>   arch/powerpc/include/asm/fadump.h                |    2
>   arch/powerpc/kernel/fadump.c                     |   97 +++++++++++++++++++++-
>   arch/powerpc/kernel/prom.c                       |    7 ++
>   include/linux/moduleparam.h                      |    1
>   init/main.c                                      |    8 +-
>   kernel/module.c                                  |    5 +
>   kernel/params.c                                  |   18 +++-
>   lib/cmdline.c                                    |   54 +++++++-----
>   lib/dynamic_debug.c                              |    1
>   11 files changed, 179 insertions(+), 39 deletions(-)

Re: [PATCH v9 0/8] reduce memory consumption for powerpc firmware-assisted capture kernel

[Christophe Leroy]

Le 15/11/2017 à 16:16, Hari Bathini a écrit :
> I posted the initial version [1] of patchset [2] adding support to enforce
> additional parameters when firmware-assisted dump capture kernel is active.
> Michal reposted it with few improvements to parameter processing to make
> it more robust. He further posted patchset [3] with few more improvements.
> 
> This patch series clubs the work from these two patch-sets while segregating
> the generic and arch-specific code to ease the review process.
> 
> [1] http://patchwork.ozlabs.org/patch/758176/
> [2] http://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=2733
> [3] http://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=3338
> 

Hello,

We still have this series flagged as "new" in powerpc's patchwork.

It is still of interest ?

Patch 1 applies but patch 2 hardly conflicts in init/main.c

So I'll flag it as "change requested", fell free to resubmit if still 
relevant.

Or if you think that's still required but don't plan to handle it, tell 
me and I'll open an issue in our issue board so that we don't loose history.

Thanks
Christophe


> ---
> 
> Hari Bathini (2):
>        powerpc/fadump: reduce memory consumption for capture kernel
>        powerpc/fadump: update documentation about 'fadump_extra_args=' parameter
> 
> Michal Suchanek (6):
>        lib/cmdline.c: remove quotes symmetrically
>        boot/param: add pointer to current and next argument to unknown parameter callback
>        lib/cmdline.c: add backslash support to kernel commandline parsing
>        Documentation/admin-guide: backslash support in commandline
>        lib/cmdline.c: implement single quotes in commandline argument parsing
>        Documentation/admin-guide: single quotes in kernel arguments
> 
> 
>   Documentation/admin-guide/kernel-parameters.rst  |    5 +
>   Documentation/powerpc/firmware-assisted-dump.txt |   20 ++++-
>   arch/powerpc/include/asm/fadump.h                |    2
>   arch/powerpc/kernel/fadump.c                     |   97 +++++++++++++++++++++-
>   arch/powerpc/kernel/prom.c                       |    7 ++
>   include/linux/moduleparam.h                      |    1
>   init/main.c                                      |    8 +-
>   kernel/module.c                                  |    5 +
>   kernel/params.c                                  |   18 +++-
>   lib/cmdline.c                                    |   54 +++++++-----
>   lib/dynamic_debug.c                              |    1
>   11 files changed, 179 insertions(+), 39 deletions(-)