linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 2/3] powerpc: convert to generic builtin command line
@ 2018-11-09 17:34 Daniel Walker
  0 siblings, 0 replies; 5+ messages in thread
From: Daniel Walker @ 2018-11-09 17:34 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: Daniel Walker, xe-linux-external, Maksym Kokhan, linuxppc-dev,
	linux-kernel

This updates the powerpc code to use the CONFIG_GENERIC_CMDLINE
option.

[maksym.kokhan@globallogic.com: add strlcat to prom_init_check.sh
whitelist]
Cc: Daniel Walker <dwalker@fifo99.com>
Cc: Daniel Walker <danielwa@cisco.com>
Cc: xe-linux-external@cisco.com
Signed-off-by: Daniel Walker <danielwa@cisco.com>
Signed-off-by: Maksym Kokhan <maksym.kokhan@globallogic.com>
---
 arch/powerpc/Kconfig                   | 23 +----------------------
 arch/powerpc/kernel/prom.c             |  4 ++++
 arch/powerpc/kernel/prom_init.c        |  8 ++++----
 arch/powerpc/kernel/prom_init_check.sh |  2 +-
 4 files changed, 10 insertions(+), 27 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 8be31261aec8..6321b2a0b87b 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -172,6 +172,7 @@ config PPC
 	select GENERIC_STRNCPY_FROM_USER
 	select GENERIC_STRNLEN_USER
 	select GENERIC_TIME_VSYSCALL
+	select GENERIC_CMDLINE
 	select HAVE_ARCH_AUDITSYSCALL
 	select HAVE_ARCH_JUMP_LABEL
 	select HAVE_ARCH_KGDB
@@ -777,28 +778,6 @@ config PPC_DENORMALISATION
 	  Add support for handling denormalisation of single precision
 	  values.  Useful for bare metal only.  If unsure say Y here.
 
-config CMDLINE_BOOL
-	bool "Default bootloader kernel arguments"
-
-config CMDLINE
-	string "Initial kernel command string"
-	depends on CMDLINE_BOOL
-	default "console=ttyS0,9600 console=tty0 root=/dev/sda2"
-	help
-	  On some platforms, there is currently no way for the boot loader to
-	  pass arguments to the kernel. For these platforms, you can supply
-	  some command-line options at build time by entering them here.  In
-	  most cases you will need to specify the root device here.
-
-config CMDLINE_FORCE
-	bool "Always use the default kernel command string"
-	depends on CMDLINE_BOOL
-	help
-	  Always use the default kernel command string, even if the boot
-	  loader passes other arguments to the kernel.
-	  This is useful if you cannot or don't want to change the
-	  command-line options your boot loader passes to the kernel.
-
 config EXTRA_TARGETS
 	string "Additional default image types"
 	help
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index fe758cedb93f..d78b1d6fe1c8 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -34,6 +34,7 @@
 #include <linux/of_fdt.h>
 #include <linux/libfdt.h>
 #include <linux/cpu.h>
+#include <linux/cmdline.h>
 
 #include <asm/prom.h>
 #include <asm/rtas.h>
@@ -716,6 +717,9 @@ void __init early_init_devtree(void *params)
 	 */
 	of_scan_flat_dt(early_init_dt_scan_chosen_ppc, boot_command_line);
 
+	/* append and prepend any arguments built into the kernel. */
+	cmdline_add_builtin(boot_command_line, NULL, COMMAND_LINE_SIZE);
+
 	/* Scan memory nodes and rebuild MEMBLOCKs */
 	of_scan_flat_dt(early_init_dt_scan_root, NULL);
 	of_scan_flat_dt(early_init_dt_scan_memory_ppc, NULL);
diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index f33ff4163a51..41393da56181 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -30,6 +30,7 @@
 #include <linux/delay.h>
 #include <linux/initrd.h>
 #include <linux/bitops.h>
+#include <linux/cmdline.h>
 #include <asm/prom.h>
 #include <asm/rtas.h>
 #include <asm/page.h>
@@ -637,11 +638,10 @@ static void __init early_cmdline_parse(void)
 	p = prom_cmd_line;
 	if ((long)prom.chosen > 0)
 		l = prom_getprop(prom.chosen, "bootargs", p, COMMAND_LINE_SIZE-1);
-#ifdef CONFIG_CMDLINE
+
 	if (l <= 0 || p[0] == '\0') /* dbl check */
-		strlcpy(prom_cmd_line,
-			CONFIG_CMDLINE, sizeof(prom_cmd_line));
-#endif /* CONFIG_CMDLINE */
+		cmdline_add_builtin(prom_cmd_line, NULL, sizeof(prom_cmd_line));
+
 	prom_printf("command line: %s\n", prom_cmd_line);
 
 #ifdef CONFIG_PPC64
diff --git a/arch/powerpc/kernel/prom_init_check.sh b/arch/powerpc/kernel/prom_init_check.sh
index 667df97d2595..ab2acc8d8b5a 100644
--- a/arch/powerpc/kernel/prom_init_check.sh
+++ b/arch/powerpc/kernel/prom_init_check.sh
@@ -18,7 +18,7 @@
 
 WHITELIST="add_reloc_offset __bss_start __bss_stop copy_and_flush
 _end enter_prom memcpy memset reloc_offset __secondary_hold
-__secondary_hold_acknowledge __secondary_hold_spinloop __start
+__secondary_hold_acknowledge __secondary_hold_spinloop __start strlcat
 strcmp strcpy strlcpy strlen strncmp strstr kstrtobool logo_linux_clut224
 reloc_got2 kernstart_addr memstart_addr linux_banner _stext
 __prom_init_toc_start __prom_init_toc_end btext_setup_display TOC."
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/3] powerpc: convert to generic builtin command line
  2019-03-04 14:26 ` Christophe Leroy
  2019-03-04 15:04   ` Christophe Leroy
@ 2019-03-19 18:58   ` Daniel Walker
  1 sibling, 0 replies; 5+ messages in thread
From: Daniel Walker @ 2019-03-19 18:58 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	xe-linux-external, linux-kernel, Maksym Kokhan, Daniel Walker,
	Andrew Morton, linuxppc-dev

On Mon, Mar 04, 2019 at 03:26:59PM +0100, Christophe Leroy wrote:
> 
> 
> Le 01/03/2019 à 20:44, Daniel Walker a écrit :
> > This updates the powerpc code to use the CONFIG_GENERIC_CMDLINE
> > option.
> 
> Please explain more in details how each powerpc option is replaced by one of
> the generic options.

CMDLINE is replace by two options to either which allow static options to either
be appended or prepended to the boot loader arguemnts. If you wanted a lateral
changes you would only fill in CONFIG_CMDLINE_PREPEND. CONFIG_CMDLINE_OVERRIDE
does the same as CMDLINE_FORCE, only with the append and prepend arguemnts
merged without the boot loader arguments.

> > --- a/arch/powerpc/kernel/prom.c
> > +++ b/arch/powerpc/kernel/prom.c
> > @@ -34,6 +34,7 @@
> >   #include <linux/of_fdt.h>
> >   #include <linux/libfdt.h>
> >   #include <linux/cpu.h>
> > +#include <linux/cmdline.h>
> >   #include <asm/prom.h>
> >   #include <asm/rtas.h>
> > @@ -716,6 +717,9 @@ void __init early_init_devtree(void *params)
> >   	 */
> >   	of_scan_flat_dt(early_init_dt_scan_chosen_ppc, boot_command_line);
> > +	/* append and prepend any arguments built into the kernel. */
> > +	cmdline_add_builtin(boot_command_line, NULL, COMMAND_LINE_SIZE);
> > +
> 
> I don't think it is worth an implementation as complex as in the previous
> patch just for the above line.
> Could easily define the temporary buffer in this file directely, then just
> locally do:
> 
> strlcpy(temp_buff, CONFIG_CMDLINE_PREPEND, COMMAND_LINE_SIZE);
> strlcat(temp_buff, boot_command_line, COMMAND_LINE_SIZE);
> strlcat(temp_buff, CONFIG_CMDLINE_APPEND, COMMAND_LINE_SIZE);
> strlcpy(boot_command_line, temp_buff, COMMAND_LINE_SIZE);
 
The point of the code is to have an implementation that other architecture can
use. If we open code it in powerpc we're no better off.

> 
> 
> >   	/* Scan memory nodes and rebuild MEMBLOCKs */
> >   	of_scan_flat_dt(early_init_dt_scan_root, NULL);
> >   	of_scan_flat_dt(early_init_dt_scan_memory_ppc, NULL);
> > diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
> > index f33ff4163a51..e8e9fca22470 100644
> > --- a/arch/powerpc/kernel/prom_init.c
> > +++ b/arch/powerpc/kernel/prom_init.c
> > @@ -30,6 +30,7 @@
> >   #include <linux/delay.h>
> >   #include <linux/initrd.h>
> >   #include <linux/bitops.h>
> > +#include <linux/cmdline.h>
> >   #include <asm/prom.h>
> >   #include <asm/rtas.h>
> >   #include <asm/page.h>
> > @@ -637,11 +638,10 @@ static void __init early_cmdline_parse(void)
> >   	p = prom_cmd_line;
> >   	if ((long)prom.chosen > 0)
> >   		l = prom_getprop(prom.chosen, "bootargs", p, COMMAND_LINE_SIZE-1);
> > -#ifdef CONFIG_CMDLINE
> > +
> >   	if (l <= 0 || p[0] == '\0') /* dbl check */
> > -		strlcpy(prom_cmd_line,
> > -			CONFIG_CMDLINE, sizeof(prom_cmd_line));
> > -#endif /* CONFIG_CMDLINE */
> > +		cmdline_add_builtin_section(prom_cmd_line, NULL, sizeof(prom_cmd_line), __prombss);
> > +
> 
> You don't need something as complex as what your generic code does for that.
> It could be done with the following simple line:
> 
> strlcpy(prom_cmd_line, CONFIG_CMDLINE_PREPEND " " CONFIG_CMDLINE_APPEND,
> sizeof(prom_cmd_line));
> 
> >   	prom_printf("command line: %s\n", prom_cmd_line);
> >   #ifdef CONFIG_PPC64
> > diff --git a/arch/powerpc/kernel/prom_init_check.sh b/arch/powerpc/kernel/prom_init_check.sh
> > index 667df97d2595..ab2acc8d8b5a 100644
> > --- a/arch/powerpc/kernel/prom_init_check.sh
> > +++ b/arch/powerpc/kernel/prom_init_check.sh
> > @@ -18,7 +18,7 @@
> >   WHITELIST="add_reloc_offset __bss_start __bss_stop copy_and_flush
> >   _end enter_prom memcpy memset reloc_offset __secondary_hold
> > -__secondary_hold_acknowledge __secondary_hold_spinloop __start
> > +__secondary_hold_acknowledge __secondary_hold_spinloop __start strlcat
> 
> The above is a big issue. In the scope of KASAN implementation, we are
> getting rid of generic string functions from prom_init because they are
> KASAN instrumented and that's far too early for prom_init. See series
> https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=94949 and
> especially patch [v9,03/11] powerpc/prom_init: don't use string functions
> from lib/

You already re-implemented a number of string functions, seem easy enough to add
another one.

What your doing here is exactly what I'm trying to prevent in my implementation.
Say there is a small, but horrific defect in one of the string functions. Some
other architecture fixes it in lib/strings.c , woops , you just missed it and
now prom_init.c is stuck with it unless powerpc maintainers are watching closely
to keep up with the fixes to the string functions.

You could move these functions into the include/linux/string.h as static
inlines, then use them in lib/strings.c and in prom_init.c. Then you have a
unified implementation. I assume you would regard that as ugly tho.

Something else you would regard as ugly , your not adding an #ifdef on KASAN in
prom_init.c for the string functions. If you have that then any buggy string
functions which you may add (or forget to update) would only cause problems
if you had KASAN enabled. That then isolates any problems you cause to only
debug kernels with KASAN enabled, instead of unilaterally all platforms which
use prom_init.c. That would also only increase the size if KASAN is enabled.
Very desirable, but ugly. I think most of us kernel hackers will take the ugly.

#ifdef CONFIG_KASAN
...
#else
#define prom_strcmp strcmp
...
#endif /* !CONFIG_KASAN */ 

(you would have to change arch/powerpc/kernel/prom_init_check.sh , but that
shouldn't be too hard)


Daniel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/3] powerpc: convert to generic builtin command line
  2019-03-04 14:26 ` Christophe Leroy
@ 2019-03-04 15:04   ` Christophe Leroy
  2019-03-19 18:58   ` Daniel Walker
  1 sibling, 0 replies; 5+ messages in thread
From: Christophe Leroy @ 2019-03-04 15:04 UTC (permalink / raw)
  To: Daniel Walker, Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: xe-linux-external, linux-kernel, Maksym Kokhan, Daniel Walker,
	Andrew Morton, linuxppc-dev



Le 04/03/2019 à 15:26, Christophe Leroy a écrit :
> 
> 
> Le 01/03/2019 à 20:44, Daniel Walker a écrit :
>> This updates the powerpc code to use the CONFIG_GENERIC_CMDLINE
>> option.
> 
> Please explain more in details how each powerpc option is replaced by 
> one of the generic options.
> 
>>
>> [maksym.kokhan@globallogic.com: add strlcat to prom_init_check.sh
>> whitelist]
>> Cc: Daniel Walker <dwalker@fifo99.com>
>> Cc: Daniel Walker <danielwa@cisco.com>
>> Cc: xe-linux-external@cisco.com
>> Signed-off-by: Daniel Walker <danielwa@cisco.com>
>> Signed-off-by: Maksym Kokhan <maksym.kokhan@globallogic.com>
>> ---
>>   arch/powerpc/Kconfig                   | 23 +----------------------
>>   arch/powerpc/kernel/prom.c             |  4 ++++
>>   arch/powerpc/kernel/prom_init.c        |  8 ++++----
>>   arch/powerpc/kernel/prom_init_check.sh |  2 +-
>>   4 files changed, 10 insertions(+), 27 deletions(-)
>>
>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>> index 8be31261aec8..6321b2a0b87b 100644
>> --- a/arch/powerpc/Kconfig
>> +++ b/arch/powerpc/Kconfig
>> @@ -172,6 +172,7 @@ config PPC
>>       select GENERIC_STRNCPY_FROM_USER
>>       select GENERIC_STRNLEN_USER
>>       select GENERIC_TIME_VSYSCALL
>> +    select GENERIC_CMDLINE
>>       select HAVE_ARCH_AUDITSYSCALL
>>       select HAVE_ARCH_JUMP_LABEL
>>       select HAVE_ARCH_KGDB
>> @@ -777,28 +778,6 @@ config PPC_DENORMALISATION
>>         Add support for handling denormalisation of single precision
>>         values.  Useful for bare metal only.  If unsure say Y here.
>> -config CMDLINE_BOOL
>> -    bool "Default bootloader kernel arguments"
>> -
>> -config CMDLINE
>> -    string "Initial kernel command string"
>> -    depends on CMDLINE_BOOL
>> -    default "console=ttyS0,9600 console=tty0 root=/dev/sda2"
>> -    help
>> -      On some platforms, there is currently no way for the boot 
>> loader to
>> -      pass arguments to the kernel. For these platforms, you can supply
>> -      some command-line options at build time by entering them here.  In
>> -      most cases you will need to specify the root device here.
>> -
>> -config CMDLINE_FORCE
>> -    bool "Always use the default kernel command string"
>> -    depends on CMDLINE_BOOL
>> -    help
>> -      Always use the default kernel command string, even if the boot
>> -      loader passes other arguments to the kernel.
>> -      This is useful if you cannot or don't want to change the
>> -      command-line options your boot loader passes to the kernel.
>> -
>>   config EXTRA_TARGETS
>>       string "Additional default image types"
>>       help
>> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
>> index fe758cedb93f..d78b1d6fe1c8 100644
>> --- a/arch/powerpc/kernel/prom.c
>> +++ b/arch/powerpc/kernel/prom.c
>> @@ -34,6 +34,7 @@
>>   #include <linux/of_fdt.h>
>>   #include <linux/libfdt.h>
>>   #include <linux/cpu.h>
>> +#include <linux/cmdline.h>
>>   #include <asm/prom.h>
>>   #include <asm/rtas.h>
>> @@ -716,6 +717,9 @@ void __init early_init_devtree(void *params)
>>        */
>>       of_scan_flat_dt(early_init_dt_scan_chosen_ppc, boot_command_line);
>> +    /* append and prepend any arguments built into the kernel. */
>> +    cmdline_add_builtin(boot_command_line, NULL, COMMAND_LINE_SIZE);
>> +
> 
> I don't think it is worth an implementation as complex as in the 
> previous patch just for the above line.
> Could easily define the temporary buffer in this file directely, then 
> just locally do:
> 
> strlcpy(temp_buff, CONFIG_CMDLINE_PREPEND, COMMAND_LINE_SIZE);
> strlcat(temp_buff, boot_command_line, COMMAND_LINE_SIZE);
> strlcat(temp_buff, CONFIG_CMDLINE_APPEND, COMMAND_LINE_SIZE);
> strlcpy(boot_command_line, temp_buff, COMMAND_LINE_SIZE);

And in fact, what should really be done is not the above but simply 
update early_init_dt_scan_chosen_ppc() in drivers/of/fdt.c as it is 
there that CONFIG_CMDLINE and CONFIG_CMDLINE_FORCE and 
CONFIG_CMDLINE_EXTEND are handled.

It looks to me that the current implementation is rather complete and 
eventually only missing the prepend. Why not just add prepend to the 
current implementation, and also move CONFIG_CMDLINE etc ... into 
arch/Kconfig instead of having it in each arch ?

Christophe

> 
> 
> 
>>       /* Scan memory nodes and rebuild MEMBLOCKs */
>>       of_scan_flat_dt(early_init_dt_scan_root, NULL);
>>       of_scan_flat_dt(early_init_dt_scan_memory_ppc, NULL);
>> diff --git a/arch/powerpc/kernel/prom_init.c 
>> b/arch/powerpc/kernel/prom_init.c
>> index f33ff4163a51..e8e9fca22470 100644
>> --- a/arch/powerpc/kernel/prom_init.c
>> +++ b/arch/powerpc/kernel/prom_init.c
>> @@ -30,6 +30,7 @@
>>   #include <linux/delay.h>
>>   #include <linux/initrd.h>
>>   #include <linux/bitops.h>
>> +#include <linux/cmdline.h>
>>   #include <asm/prom.h>
>>   #include <asm/rtas.h>
>>   #include <asm/page.h>
>> @@ -637,11 +638,10 @@ static void __init early_cmdline_parse(void)
>>       p = prom_cmd_line;
>>       if ((long)prom.chosen > 0)
>>           l = prom_getprop(prom.chosen, "bootargs", p, 
>> COMMAND_LINE_SIZE-1);
>> -#ifdef CONFIG_CMDLINE
>> +
>>       if (l <= 0 || p[0] == '\0') /* dbl check */
>> -        strlcpy(prom_cmd_line,
>> -            CONFIG_CMDLINE, sizeof(prom_cmd_line));
>> -#endif /* CONFIG_CMDLINE */
>> +        cmdline_add_builtin_section(prom_cmd_line, NULL, 
>> sizeof(prom_cmd_line), __prombss);
>> +
> 
> You don't need something as complex as what your generic code does for 
> that. It could be done with the following simple line:
> 
> strlcpy(prom_cmd_line, CONFIG_CMDLINE_PREPEND " " CONFIG_CMDLINE_APPEND, 
> sizeof(prom_cmd_line));
> 
>>       prom_printf("command line: %s\n", prom_cmd_line);
>>   #ifdef CONFIG_PPC64
>> diff --git a/arch/powerpc/kernel/prom_init_check.sh 
>> b/arch/powerpc/kernel/prom_init_check.sh
>> index 667df97d2595..ab2acc8d8b5a 100644
>> --- a/arch/powerpc/kernel/prom_init_check.sh
>> +++ b/arch/powerpc/kernel/prom_init_check.sh
>> @@ -18,7 +18,7 @@
>>   WHITELIST="add_reloc_offset __bss_start __bss_stop copy_and_flush
>>   _end enter_prom memcpy memset reloc_offset __secondary_hold
>> -__secondary_hold_acknowledge __secondary_hold_spinloop __start
>> +__secondary_hold_acknowledge __secondary_hold_spinloop __start strlcat
> 
> The above is a big issue. In the scope of KASAN implementation, we are 
> getting rid of generic string functions from prom_init because they are 
> KASAN instrumented and that's far too early for prom_init. See series 
> https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=94949 and 
> especially patch [v9,03/11] powerpc/prom_init: don't use string 
> functions from lib/
> 
>>   strcmp strcpy strlcpy strlen strncmp strstr kstrtobool 
>> logo_linux_clut224
>>   reloc_got2 kernstart_addr memstart_addr linux_banner _stext
>>   __prom_init_toc_start __prom_init_toc_end btext_setup_display TOC."
>>
> 
> Thanks
> Christophe

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/3] powerpc: convert to generic builtin command line
  2019-03-01 19:44 Daniel Walker
@ 2019-03-04 14:26 ` Christophe Leroy
  2019-03-04 15:04   ` Christophe Leroy
  2019-03-19 18:58   ` Daniel Walker
  0 siblings, 2 replies; 5+ messages in thread
From: Christophe Leroy @ 2019-03-04 14:26 UTC (permalink / raw)
  To: Daniel Walker, Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: xe-linux-external, linux-kernel, Maksym Kokhan, Daniel Walker,
	Andrew Morton, linuxppc-dev



Le 01/03/2019 à 20:44, Daniel Walker a écrit :
> This updates the powerpc code to use the CONFIG_GENERIC_CMDLINE
> option.

Please explain more in details how each powerpc option is replaced by 
one of the generic options.

> 
> [maksym.kokhan@globallogic.com: add strlcat to prom_init_check.sh
> whitelist]
> Cc: Daniel Walker <dwalker@fifo99.com>
> Cc: Daniel Walker <danielwa@cisco.com>
> Cc: xe-linux-external@cisco.com
> Signed-off-by: Daniel Walker <danielwa@cisco.com>
> Signed-off-by: Maksym Kokhan <maksym.kokhan@globallogic.com>
> ---
>   arch/powerpc/Kconfig                   | 23 +----------------------
>   arch/powerpc/kernel/prom.c             |  4 ++++
>   arch/powerpc/kernel/prom_init.c        |  8 ++++----
>   arch/powerpc/kernel/prom_init_check.sh |  2 +-
>   4 files changed, 10 insertions(+), 27 deletions(-)
> 
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 8be31261aec8..6321b2a0b87b 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -172,6 +172,7 @@ config PPC
>   	select GENERIC_STRNCPY_FROM_USER
>   	select GENERIC_STRNLEN_USER
>   	select GENERIC_TIME_VSYSCALL
> +	select GENERIC_CMDLINE
>   	select HAVE_ARCH_AUDITSYSCALL
>   	select HAVE_ARCH_JUMP_LABEL
>   	select HAVE_ARCH_KGDB
> @@ -777,28 +778,6 @@ config PPC_DENORMALISATION
>   	  Add support for handling denormalisation of single precision
>   	  values.  Useful for bare metal only.  If unsure say Y here.
>   
> -config CMDLINE_BOOL
> -	bool "Default bootloader kernel arguments"
> -
> -config CMDLINE
> -	string "Initial kernel command string"
> -	depends on CMDLINE_BOOL
> -	default "console=ttyS0,9600 console=tty0 root=/dev/sda2"
> -	help
> -	  On some platforms, there is currently no way for the boot loader to
> -	  pass arguments to the kernel. For these platforms, you can supply
> -	  some command-line options at build time by entering them here.  In
> -	  most cases you will need to specify the root device here.
> -
> -config CMDLINE_FORCE
> -	bool "Always use the default kernel command string"
> -	depends on CMDLINE_BOOL
> -	help
> -	  Always use the default kernel command string, even if the boot
> -	  loader passes other arguments to the kernel.
> -	  This is useful if you cannot or don't want to change the
> -	  command-line options your boot loader passes to the kernel.
> -
>   config EXTRA_TARGETS
>   	string "Additional default image types"
>   	help
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index fe758cedb93f..d78b1d6fe1c8 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -34,6 +34,7 @@
>   #include <linux/of_fdt.h>
>   #include <linux/libfdt.h>
>   #include <linux/cpu.h>
> +#include <linux/cmdline.h>
>   
>   #include <asm/prom.h>
>   #include <asm/rtas.h>
> @@ -716,6 +717,9 @@ void __init early_init_devtree(void *params)
>   	 */
>   	of_scan_flat_dt(early_init_dt_scan_chosen_ppc, boot_command_line);
>   
> +	/* append and prepend any arguments built into the kernel. */
> +	cmdline_add_builtin(boot_command_line, NULL, COMMAND_LINE_SIZE);
> +

I don't think it is worth an implementation as complex as in the 
previous patch just for the above line.
Could easily define the temporary buffer in this file directely, then 
just locally do:

strlcpy(temp_buff, CONFIG_CMDLINE_PREPEND, COMMAND_LINE_SIZE);
strlcat(temp_buff, boot_command_line, COMMAND_LINE_SIZE);
strlcat(temp_buff, CONFIG_CMDLINE_APPEND, COMMAND_LINE_SIZE);
strlcpy(boot_command_line, temp_buff, COMMAND_LINE_SIZE);



>   	/* Scan memory nodes and rebuild MEMBLOCKs */
>   	of_scan_flat_dt(early_init_dt_scan_root, NULL);
>   	of_scan_flat_dt(early_init_dt_scan_memory_ppc, NULL);
> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
> index f33ff4163a51..e8e9fca22470 100644
> --- a/arch/powerpc/kernel/prom_init.c
> +++ b/arch/powerpc/kernel/prom_init.c
> @@ -30,6 +30,7 @@
>   #include <linux/delay.h>
>   #include <linux/initrd.h>
>   #include <linux/bitops.h>
> +#include <linux/cmdline.h>
>   #include <asm/prom.h>
>   #include <asm/rtas.h>
>   #include <asm/page.h>
> @@ -637,11 +638,10 @@ static void __init early_cmdline_parse(void)
>   	p = prom_cmd_line;
>   	if ((long)prom.chosen > 0)
>   		l = prom_getprop(prom.chosen, "bootargs", p, COMMAND_LINE_SIZE-1);
> -#ifdef CONFIG_CMDLINE
> +
>   	if (l <= 0 || p[0] == '\0') /* dbl check */
> -		strlcpy(prom_cmd_line,
> -			CONFIG_CMDLINE, sizeof(prom_cmd_line));
> -#endif /* CONFIG_CMDLINE */
> +		cmdline_add_builtin_section(prom_cmd_line, NULL, sizeof(prom_cmd_line), __prombss);
> +

You don't need something as complex as what your generic code does for 
that. It could be done with the following simple line:

strlcpy(prom_cmd_line, CONFIG_CMDLINE_PREPEND " " CONFIG_CMDLINE_APPEND, 
sizeof(prom_cmd_line));

>   	prom_printf("command line: %s\n", prom_cmd_line);
>   
>   #ifdef CONFIG_PPC64
> diff --git a/arch/powerpc/kernel/prom_init_check.sh b/arch/powerpc/kernel/prom_init_check.sh
> index 667df97d2595..ab2acc8d8b5a 100644
> --- a/arch/powerpc/kernel/prom_init_check.sh
> +++ b/arch/powerpc/kernel/prom_init_check.sh
> @@ -18,7 +18,7 @@
>   
>   WHITELIST="add_reloc_offset __bss_start __bss_stop copy_and_flush
>   _end enter_prom memcpy memset reloc_offset __secondary_hold
> -__secondary_hold_acknowledge __secondary_hold_spinloop __start
> +__secondary_hold_acknowledge __secondary_hold_spinloop __start strlcat

The above is a big issue. In the scope of KASAN implementation, we are 
getting rid of generic string functions from prom_init because they are 
KASAN instrumented and that's far too early for prom_init. See series 
https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=94949 and 
especially patch [v9,03/11] powerpc/prom_init: don't use string 
functions from lib/

>   strcmp strcpy strlcpy strlen strncmp strstr kstrtobool logo_linux_clut224
>   reloc_got2 kernstart_addr memstart_addr linux_banner _stext
>   __prom_init_toc_start __prom_init_toc_end btext_setup_display TOC."
> 

Thanks
Christophe

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 2/3] powerpc: convert to generic builtin command line
@ 2019-03-01 19:44 Daniel Walker
  2019-03-04 14:26 ` Christophe Leroy
  0 siblings, 1 reply; 5+ messages in thread
From: Daniel Walker @ 2019-03-01 19:44 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: Andrew Morton, linuxppc-dev, Daniel Walker, xe-linux-external,
	Maksym Kokhan, linux-kernel

This updates the powerpc code to use the CONFIG_GENERIC_CMDLINE
option.

[maksym.kokhan@globallogic.com: add strlcat to prom_init_check.sh
whitelist]
Cc: Daniel Walker <dwalker@fifo99.com>
Cc: Daniel Walker <danielwa@cisco.com>
Cc: xe-linux-external@cisco.com
Signed-off-by: Daniel Walker <danielwa@cisco.com>
Signed-off-by: Maksym Kokhan <maksym.kokhan@globallogic.com>
---
 arch/powerpc/Kconfig                   | 23 +----------------------
 arch/powerpc/kernel/prom.c             |  4 ++++
 arch/powerpc/kernel/prom_init.c        |  8 ++++----
 arch/powerpc/kernel/prom_init_check.sh |  2 +-
 4 files changed, 10 insertions(+), 27 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 8be31261aec8..6321b2a0b87b 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -172,6 +172,7 @@ config PPC
 	select GENERIC_STRNCPY_FROM_USER
 	select GENERIC_STRNLEN_USER
 	select GENERIC_TIME_VSYSCALL
+	select GENERIC_CMDLINE
 	select HAVE_ARCH_AUDITSYSCALL
 	select HAVE_ARCH_JUMP_LABEL
 	select HAVE_ARCH_KGDB
@@ -777,28 +778,6 @@ config PPC_DENORMALISATION
 	  Add support for handling denormalisation of single precision
 	  values.  Useful for bare metal only.  If unsure say Y here.
 
-config CMDLINE_BOOL
-	bool "Default bootloader kernel arguments"
-
-config CMDLINE
-	string "Initial kernel command string"
-	depends on CMDLINE_BOOL
-	default "console=ttyS0,9600 console=tty0 root=/dev/sda2"
-	help
-	  On some platforms, there is currently no way for the boot loader to
-	  pass arguments to the kernel. For these platforms, you can supply
-	  some command-line options at build time by entering them here.  In
-	  most cases you will need to specify the root device here.
-
-config CMDLINE_FORCE
-	bool "Always use the default kernel command string"
-	depends on CMDLINE_BOOL
-	help
-	  Always use the default kernel command string, even if the boot
-	  loader passes other arguments to the kernel.
-	  This is useful if you cannot or don't want to change the
-	  command-line options your boot loader passes to the kernel.
-
 config EXTRA_TARGETS
 	string "Additional default image types"
 	help
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index fe758cedb93f..d78b1d6fe1c8 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -34,6 +34,7 @@
 #include <linux/of_fdt.h>
 #include <linux/libfdt.h>
 #include <linux/cpu.h>
+#include <linux/cmdline.h>
 
 #include <asm/prom.h>
 #include <asm/rtas.h>
@@ -716,6 +717,9 @@ void __init early_init_devtree(void *params)
 	 */
 	of_scan_flat_dt(early_init_dt_scan_chosen_ppc, boot_command_line);
 
+	/* append and prepend any arguments built into the kernel. */
+	cmdline_add_builtin(boot_command_line, NULL, COMMAND_LINE_SIZE);
+
 	/* Scan memory nodes and rebuild MEMBLOCKs */
 	of_scan_flat_dt(early_init_dt_scan_root, NULL);
 	of_scan_flat_dt(early_init_dt_scan_memory_ppc, NULL);
diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index f33ff4163a51..e8e9fca22470 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -30,6 +30,7 @@
 #include <linux/delay.h>
 #include <linux/initrd.h>
 #include <linux/bitops.h>
+#include <linux/cmdline.h>
 #include <asm/prom.h>
 #include <asm/rtas.h>
 #include <asm/page.h>
@@ -637,11 +638,10 @@ static void __init early_cmdline_parse(void)
 	p = prom_cmd_line;
 	if ((long)prom.chosen > 0)
 		l = prom_getprop(prom.chosen, "bootargs", p, COMMAND_LINE_SIZE-1);
-#ifdef CONFIG_CMDLINE
+
 	if (l <= 0 || p[0] == '\0') /* dbl check */
-		strlcpy(prom_cmd_line,
-			CONFIG_CMDLINE, sizeof(prom_cmd_line));
-#endif /* CONFIG_CMDLINE */
+		cmdline_add_builtin_section(prom_cmd_line, NULL, sizeof(prom_cmd_line), __prombss);
+
 	prom_printf("command line: %s\n", prom_cmd_line);
 
 #ifdef CONFIG_PPC64
diff --git a/arch/powerpc/kernel/prom_init_check.sh b/arch/powerpc/kernel/prom_init_check.sh
index 667df97d2595..ab2acc8d8b5a 100644
--- a/arch/powerpc/kernel/prom_init_check.sh
+++ b/arch/powerpc/kernel/prom_init_check.sh
@@ -18,7 +18,7 @@
 
 WHITELIST="add_reloc_offset __bss_start __bss_stop copy_and_flush
 _end enter_prom memcpy memset reloc_offset __secondary_hold
-__secondary_hold_acknowledge __secondary_hold_spinloop __start
+__secondary_hold_acknowledge __secondary_hold_spinloop __start strlcat
 strcmp strcpy strlcpy strlen strncmp strstr kstrtobool logo_linux_clut224
 reloc_got2 kernstart_addr memstart_addr linux_banner _stext
 __prom_init_toc_start __prom_init_toc_end btext_setup_display TOC."
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-03-19 18:58 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-09 17:34 [PATCH 2/3] powerpc: convert to generic builtin command line Daniel Walker
2019-03-01 19:44 Daniel Walker
2019-03-04 14:26 ` Christophe Leroy
2019-03-04 15:04   ` Christophe Leroy
2019-03-19 18:58   ` Daniel Walker

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).