[{"id":2644199,"web_url":"http://patchwork.ozlabs.org/comment/2644199/","msgid":"<c5c8b57e-7954-ec02-188a-7f85cb0af731@csgroup.eu>","date":"2021-03-09T07:56:47","subject":"Re: [PATCH v2 4/7] CMDLINE: powerpc: convert to generic builtin\n command line","submitter":{"id":79086,"url":"http://patchwork.ozlabs.org/api/people/79086/","name":"Christophe Leroy","email":"christophe.leroy@csgroup.eu"},"content":"Le 09/03/2021 à 01:02, Daniel Walker a écrit :\n> This updates the powerpc code to use the CONFIG_GENERIC_CMDLINE\n> option.\n> \n> Cc: xe-linux-external@cisco.com\n> Signed-off-by: Ruslan Ruslichenko <rruslich@cisco.com>\n> Signed-off-by: Ruslan Bilovol <rbilovol@cisco.com>\n> Signed-off-by: Daniel Walker <danielwa@cisco.com>\n> ---\n>   arch/powerpc/Kconfig            | 37 +--------------------------------\n>   arch/powerpc/kernel/prom.c      |  1 +\n>   arch/powerpc/kernel/prom_init.c | 35 ++++++++++++++++++-------------\n>   3 files changed, 23 insertions(+), 50 deletions(-)\n> \n> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig\n> index 107bb4319e0e..276b06d5c961 100644\n> --- a/arch/powerpc/Kconfig\n> +++ b/arch/powerpc/Kconfig\n> @@ -167,6 +167,7 @@ config PPC\n>   \tselect EDAC_SUPPORT\n>   \tselect GENERIC_ATOMIC64\t\t\tif PPC32\n>   \tselect GENERIC_CLOCKEVENTS_BROADCAST\tif SMP\n> +\tselect GENERIC_CMDLINE\n>   \tselect GENERIC_CMOS_UPDATE\n>   \tselect GENERIC_CPU_AUTOPROBE\n>   \tselect GENERIC_CPU_VULNERABILITIES\tif PPC_BARRIER_NOSPEC\n> @@ -906,42 +907,6 @@ config PPC_DENORMALISATION\n>   \t  Add support for handling denormalisation of single precision\n>   \t  values.  Useful for bare metal only.  If unsure say Y here.\n>   \n> -config CMDLINE\n> -\tstring \"Initial kernel command string\"\n> -\tdefault \"\"\n> -\thelp\n> -\t  On some platforms, there is currently no way for the boot loader to\n> -\t  pass arguments to the kernel. For these platforms, you can supply\n> -\t  some command-line options at build time by entering them here.  In\n> -\t  most cases you will need to specify the root device here.\n> -\n> -choice\n> -\tprompt \"Kernel command line type\" if CMDLINE != \"\"\n> -\tdefault CMDLINE_FROM_BOOTLOADER\n> -\n> -config CMDLINE_FROM_BOOTLOADER\n> -\tbool \"Use bootloader kernel arguments if available\"\n> -\thelp\n> -\t  Uses the command-line options passed by the boot loader. If\n> -\t  the boot loader doesn't provide any, the default kernel command\n> -\t  string provided in CMDLINE will be used.\n> -\n> -config CMDLINE_EXTEND\n> -\tbool \"Extend bootloader kernel arguments\"\n> -\thelp\n> -\t  The command-line arguments provided by the boot loader will be\n> -\t  appended to the default kernel command string.\n> -\n> -config CMDLINE_FORCE\n> -\tbool \"Always use the default kernel command string\"\n> -\thelp\n> -\t  Always use the default kernel command string, even if the boot\n> -\t  loader passes other arguments to the kernel.\n> -\t  This is useful if you cannot or don't want to change the\n> -\t  command-line options your boot loader passes to the kernel.\n> -\n> -endchoice\n> -\n>   config EXTRA_TARGETS\n>   \tstring \"Additional default image types\"\n>   \thelp\n> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c\n> index ae3c41730367..96d0a01be1b4 100644\n> --- a/arch/powerpc/kernel/prom.c\n> +++ b/arch/powerpc/kernel/prom.c\n> @@ -27,6 +27,7 @@\n>   #include <linux/irq.h>\n>   #include <linux/memblock.h>\n>   #include <linux/of.h>\n> +#include <linux/cmdline.h>\n\nWhy is this needed in prom.c ?\n\n>   #include <linux/of_fdt.h>\n>   #include <linux/libfdt.h>\n>   #include <linux/cpu.h>\n> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c\n> index e9d4eb6144e1..657241534d69 100644\n> --- a/arch/powerpc/kernel/prom_init.c\n> +++ b/arch/powerpc/kernel/prom_init.c\n> @@ -27,6 +27,7 @@\n>   #include <linux/initrd.h>\n>   #include <linux/bitops.h>\n>   #include <linux/pgtable.h>\n> +#include <linux/cmdline.h>\n>   #include <asm/prom.h>\n>   #include <asm/rtas.h>\n>   #include <asm/page.h>\n> @@ -242,15 +243,6 @@ static int __init prom_strcmp(const char *cs, const char *ct)\n>   \treturn 0;\n>   }\n>   \n> -static char __init *prom_strcpy(char *dest, const char *src)\n> -{\n> -\tchar *tmp = dest;\n> -\n> -\twhile ((*dest++ = *src++) != '\\0')\n> -\t\t/* nothing */;\n> -\treturn tmp;\n> -}\n> -\n\nThis game with prom_strcpy() should go a separate preceeding patch.\n\nAlso, it looks like checkpatch.pl recommends to use strscpy() instead of strlcpy().\n\n>   static int __init prom_strncmp(const char *cs, const char *ct, size_t count)\n>   {\n>   \tunsigned char c1, c2;\n> @@ -276,6 +268,20 @@ static size_t __init prom_strlen(const char *s)\n>   \treturn sc - s;\n>   }\n>   \n> +static size_t __init prom_strlcpy(char *dest, const char *src, size_t size)\n> +{\n> +\tsize_t ret = prom_strlen(src);\n> +\n> +\tif (size) {\n> +\t\tsize_t len = (ret >= size) ? size - 1 : ret;\n> +\n> +\t\tmemcpy(dest, src, len);\n> +\t\tdest[len] = '\\0';\n> +\t}\n> +\treturn ret;\n> +}\n> +\n> +\n>   static int __init prom_memcmp(const void *cs, const void *ct, size_t count)\n>   {\n>   \tconst unsigned char *su1, *su2;\n> @@ -304,6 +310,7 @@ static char __init *prom_strstr(const char *s1, const char *s2)\n>   \treturn NULL;\n>   }\n>   \n> +#ifdef GENERIC_CMDLINE_NEED_STRLCAT\n>   static size_t __init prom_strlcat(char *dest, const char *src, size_t count)\n>   {\n>   \tsize_t dsize = prom_strlen(dest);\n> @@ -323,6 +330,7 @@ static size_t __init prom_strlcat(char *dest, const char *src, size_t count)\n>   \treturn res;\n>   \n>   }\n> +#endif\n>   \n>   #ifdef CONFIG_PPC_PSERIES\n>   static int __init prom_strtobool(const char *s, bool *res)\n> @@ -775,12 +783,11 @@ static void __init early_cmdline_parse(void)\n>   \tprom_cmd_line[0] = 0;\n>   \tp = prom_cmd_line;\n>   \n> -\tif (!IS_ENABLED(CONFIG_CMDLINE_FORCE) && (long)prom.chosen > 0)\n> +\tif ((long)prom.chosen > 0)\n>   \t\tl = prom_getprop(prom.chosen, \"bootargs\", p, COMMAND_LINE_SIZE-1);\n>   \n> -\tif (IS_ENABLED(CONFIG_CMDLINE_EXTEND) || l <= 0 || p[0] == '\\0')\n> -\t\tprom_strlcat(prom_cmd_line, \" \" CONFIG_CMDLINE,\n> -\t\t\t     sizeof(prom_cmd_line));\n> +\tcmdline_add_builtin_custom(prom_cmd_line, (l > 0 ? p : NULL), sizeof(prom_cmd_line),\n> +\t\t\t\t\t__prombss, prom_strlcpy, prom_strlcat);\n\nSo we are referencing a function that doesn't exist (namely prom_strlcat).\nBut it works because cmdline_add_builtin_custom() looks like a function but is in fact an obscure \nmacro that doesn't use prom_strlcat() unless GENERIC_CMDLINE_NEED_STRLCAT is defined.\n\nIMHO that's awful for readability and code maintenance.\n\n>   \n>   \tprom_printf(\"command line: %s\\n\", prom_cmd_line);\n>   \n> @@ -2706,7 +2713,7 @@ static void __init flatten_device_tree(void)\n>   \n>   \t/* Add \"phandle\" in there, we'll need it */\n>   \tnamep = make_room(&mem_start, &mem_end, 16, 1);\n> -\tprom_strcpy(namep, \"phandle\");\n> +\tprom_strlcpy(namep, \"phandle\", 8);\n\nShould be in a separate patch.\n\n>   \tmem_start = (unsigned long)namep + prom_strlen(namep) + 1;\n>   \n>   \t/* Build string array */\n> \n\nChristophe","headers":{"Return-Path":"\n <linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org>","X-Original-To":["patchwork-incoming@ozlabs.org","linuxppc-dev@lists.ozlabs.org"],"Delivered-To":["patchwork-incoming@ozlabs.org","linuxppc-dev@lists.ozlabs.org"],"Authentication-Results":["ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=lists.ozlabs.org\n (client-ip=2404:9400:2:0:216:3eff:fee1:b9f1; helo=lists.ozlabs.org;\n envelope-from=linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org;\n receiver=<UNKNOWN>)","lists.ozlabs.org; spf=pass (sender SPF authorized)\n smtp.mailfrom=csgroup.eu (client-ip=93.17.236.30; helo=pegase1.c-s.fr;\n envelope-from=christophe.leroy@csgroup.eu; receiver=<UNKNOWN>)"],"Received":["from lists.ozlabs.org (lists.ozlabs.org\n [IPv6:2404:9400:2:0:216:3eff:fee1:b9f1])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange X25519 server-signature RSA-PSS (4096 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 4Dvnc848ctz9sW1\n\tfor <patchwork-incoming@ozlabs.org>; Tue,  9 Mar 2021 18:57:12 +1100 (AEDT)","from boromir.ozlabs.org (localhost [IPv6:::1])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 4Dvnc83JX6z3cRj\n\tfor <patchwork-incoming@ozlabs.org>; Tue,  9 Mar 2021 18:57:12 +1100 (AEDT)","from pegase1.c-s.fr (pegase1.c-s.fr [93.17.236.30])\n (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))\n (No client certificate requested)\n by lists.ozlabs.org (Postfix) with ESMTPS id 4Dvnbp0QBSz30L3\n for <linuxppc-dev@lists.ozlabs.org>; Tue,  9 Mar 2021 18:56:52 +1100 (AEDT)","from localhost (mailhub1-int [192.168.12.234])\n by localhost (Postfix) with ESMTP id 4Dvnbh1xgVz9txlX;\n Tue,  9 Mar 2021 08:56:48 +0100 (CET)","from pegase1.c-s.fr ([192.168.12.234])\n by localhost (pegase1.c-s.fr [192.168.12.234]) (amavisd-new, port 10024)\n with ESMTP id w6xYnDel4M2J; Tue,  9 Mar 2021 08:56:48 +0100 (CET)","from messagerie.si.c-s.fr (messagerie.si.c-s.fr [192.168.25.192])\n by pegase1.c-s.fr (Postfix) with ESMTP id 4Dvnbh0qTtz9txlW;\n Tue,  9 Mar 2021 08:56:48 +0100 (CET)","from localhost (localhost [127.0.0.1])\n by messagerie.si.c-s.fr (Postfix) with ESMTP id 2035B8B7CE;\n Tue,  9 Mar 2021 08:56:49 +0100 (CET)","from messagerie.si.c-s.fr ([127.0.0.1])\n by localhost (messagerie.si.c-s.fr [127.0.0.1]) (amavisd-new, port 10023)\n with ESMTP id ebcmzNe1X4b6; Tue,  9 Mar 2021 08:56:49 +0100 (CET)","from [192.168.4.90] (unknown [192.168.4.90])\n by messagerie.si.c-s.fr (Postfix) with ESMTP id 340548B773;\n Tue,  9 Mar 2021 08:56:48 +0100 (CET)"],"X-Virus-Scanned":["Debian amavisd-new at c-s.fr","amavisd-new at c-s.fr"],"Subject":"Re: [PATCH v2 4/7] CMDLINE: powerpc: convert to generic builtin\n command line","To":"Daniel Walker <danielwa@cisco.com>, Will Deacon <will@kernel.org>,\n Rob Herring <robh@kernel.org>,\n Daniel Gimpelevich <daniel@gimpelevich.san-francisco.ca.us>,\n Andrew Morton <akpm@linux-foundation.org>, x86@kernel.org,\n linux-mips@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,\n xe-linux-external@cisco.com, Michael Ellerman <mpe@ellerman.id.au>,\n Benjamin Herrenschmidt <benh@kernel.crashing.org>,\n Paul Mackerras <paulus@samba.org>","References":"<20210309000247.2989531-5-danielwa@cisco.com>","From":"Christophe Leroy <christophe.leroy@csgroup.eu>","Message-ID":"<c5c8b57e-7954-ec02-188a-7f85cb0af731@csgroup.eu>","Date":"Tue, 9 Mar 2021 08:56:47 +0100","User-Agent":"Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:78.0) Gecko/20100101\n Thunderbird/78.8.0","MIME-Version":"1.0","In-Reply-To":"<20210309000247.2989531-5-danielwa@cisco.com>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"fr","Content-Transfer-Encoding":"8bit","X-BeenThere":"linuxppc-dev@lists.ozlabs.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"Linux on PowerPC Developers Mail List <linuxppc-dev.lists.ozlabs.org>","List-Unsubscribe":"<https://lists.ozlabs.org/options/linuxppc-dev>,\n <mailto:linuxppc-dev-request@lists.ozlabs.org?subject=unsubscribe>","List-Archive":"<http://lists.ozlabs.org/pipermail/linuxppc-dev/>","List-Post":"<mailto:linuxppc-dev@lists.ozlabs.org>","List-Help":"<mailto:linuxppc-dev-request@lists.ozlabs.org?subject=help>","List-Subscribe":"<https://lists.ozlabs.org/listinfo/linuxppc-dev>,\n <mailto:linuxppc-dev-request@lists.ozlabs.org?subject=subscribe>","Cc":"Ruslan Ruslichenko <rruslich@cisco.com>,\n Ruslan Bilovol <rbilovol@cisco.com>, linux-kernel@vger.kernel.org","Errors-To":"linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org","Sender":"\"Linuxppc-dev\"\n <linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org>"}},{"id":2644913,"web_url":"http://patchwork.ozlabs.org/comment/2644913/","msgid":"<20210309214051.GS109100@zorba>","date":"2021-03-09T21:40:51","subject":"Re: [PATCH v2 4/7] CMDLINE: powerpc: convert to generic builtin\n command line","submitter":{"id":67374,"url":"http://patchwork.ozlabs.org/api/people/67374/","name":"Daniel Walker (danielwa)","email":"danielwa@cisco.com"},"content":"On Tue, Mar 09, 2021 at 08:56:47AM +0100, Christophe Leroy wrote:\n> \n> \n> Le 09/03/2021 à 01:02, Daniel Walker a écrit :\n> > This updates the powerpc code to use the CONFIG_GENERIC_CMDLINE\n> > option.\n> > \n> > Cc: xe-linux-external@cisco.com\n> > Signed-off-by: Ruslan Ruslichenko <rruslich@cisco.com>\n> > Signed-off-by: Ruslan Bilovol <rbilovol@cisco.com>\n> > Signed-off-by: Daniel Walker <danielwa@cisco.com>\n> > ---\n> >   arch/powerpc/Kconfig            | 37 +--------------------------------\n> >   arch/powerpc/kernel/prom.c      |  1 +\n> >   arch/powerpc/kernel/prom_init.c | 35 ++++++++++++++++++-------------\n> >   3 files changed, 23 insertions(+), 50 deletions(-)\n> > \n> > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig\n> > index 107bb4319e0e..276b06d5c961 100644\n> > --- a/arch/powerpc/Kconfig\n> > +++ b/arch/powerpc/Kconfig\n> > @@ -167,6 +167,7 @@ config PPC\n> >   \tselect EDAC_SUPPORT\n> >   \tselect GENERIC_ATOMIC64\t\t\tif PPC32\n> >   \tselect GENERIC_CLOCKEVENTS_BROADCAST\tif SMP\n> > +\tselect GENERIC_CMDLINE\n> >   \tselect GENERIC_CMOS_UPDATE\n> >   \tselect GENERIC_CPU_AUTOPROBE\n> >   \tselect GENERIC_CPU_VULNERABILITIES\tif PPC_BARRIER_NOSPEC\n> > @@ -906,42 +907,6 @@ config PPC_DENORMALISATION\n> >   \t  Add support for handling denormalisation of single precision\n> >   \t  values.  Useful for bare metal only.  If unsure say Y here.\n> > -config CMDLINE\n> > -\tstring \"Initial kernel command string\"\n> > -\tdefault \"\"\n> > -\thelp\n> > -\t  On some platforms, there is currently no way for the boot loader to\n> > -\t  pass arguments to the kernel. For these platforms, you can supply\n> > -\t  some command-line options at build time by entering them here.  In\n> > -\t  most cases you will need to specify the root device here.\n> > -\n> > -choice\n> > -\tprompt \"Kernel command line type\" if CMDLINE != \"\"\n> > -\tdefault CMDLINE_FROM_BOOTLOADER\n> > -\n> > -config CMDLINE_FROM_BOOTLOADER\n> > -\tbool \"Use bootloader kernel arguments if available\"\n> > -\thelp\n> > -\t  Uses the command-line options passed by the boot loader. If\n> > -\t  the boot loader doesn't provide any, the default kernel command\n> > -\t  string provided in CMDLINE will be used.\n> > -\n> > -config CMDLINE_EXTEND\n> > -\tbool \"Extend bootloader kernel arguments\"\n> > -\thelp\n> > -\t  The command-line arguments provided by the boot loader will be\n> > -\t  appended to the default kernel command string.\n> > -\n> > -config CMDLINE_FORCE\n> > -\tbool \"Always use the default kernel command string\"\n> > -\thelp\n> > -\t  Always use the default kernel command string, even if the boot\n> > -\t  loader passes other arguments to the kernel.\n> > -\t  This is useful if you cannot or don't want to change the\n> > -\t  command-line options your boot loader passes to the kernel.\n> > -\n> > -endchoice\n> > -\n> >   config EXTRA_TARGETS\n> >   \tstring \"Additional default image types\"\n> >   \thelp\n> > diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c\n> > index ae3c41730367..96d0a01be1b4 100644\n> > --- a/arch/powerpc/kernel/prom.c\n> > +++ b/arch/powerpc/kernel/prom.c\n> > @@ -27,6 +27,7 @@\n> >   #include <linux/irq.h>\n> >   #include <linux/memblock.h>\n> >   #include <linux/of.h>\n> > +#include <linux/cmdline.h>\n> \n> Why is this needed in prom.c ?\n \nMust have been a mistake, I don't think it's needed.\n\n\n> >   #include <linux/of_fdt.h>\n> >   #include <linux/libfdt.h>\n> >   #include <linux/cpu.h>\n> > diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c\n> > index e9d4eb6144e1..657241534d69 100644\n> > --- a/arch/powerpc/kernel/prom_init.c\n> > +++ b/arch/powerpc/kernel/prom_init.c\n> > @@ -27,6 +27,7 @@\n> >   #include <linux/initrd.h>\n> >   #include <linux/bitops.h>\n> >   #include <linux/pgtable.h>\n> > +#include <linux/cmdline.h>\n> >   #include <asm/prom.h>\n> >   #include <asm/rtas.h>\n> >   #include <asm/page.h>\n> > @@ -242,15 +243,6 @@ static int __init prom_strcmp(const char *cs, const char *ct)\n> >   \treturn 0;\n> >   }\n> > -static char __init *prom_strcpy(char *dest, const char *src)\n> > -{\n> > -\tchar *tmp = dest;\n> > -\n> > -\twhile ((*dest++ = *src++) != '\\0')\n> > -\t\t/* nothing */;\n> > -\treturn tmp;\n> > -}\n> > -\n> \n> This game with prom_strcpy() should go a separate preceeding patch.\n> \n> Also, it looks like checkpatch.pl recommends to use strscpy() instead of strlcpy().\n\nstrscpy() is very large. I'm not sure it's compatible with this prom_init.c\nenvironment.\n\n> >   static int __init prom_strncmp(const char *cs, const char *ct, size_t count)\n> >   {\n> >   \tunsigned char c1, c2;\n> > @@ -276,6 +268,20 @@ static size_t __init prom_strlen(const char *s)\n> >   \treturn sc - s;\n> >   }\n> > +static size_t __init prom_strlcpy(char *dest, const char *src, size_t size)\n> > +{\n> > +\tsize_t ret = prom_strlen(src);\n> > +\n> > +\tif (size) {\n> > +\t\tsize_t len = (ret >= size) ? size - 1 : ret;\n> > +\n> > +\t\tmemcpy(dest, src, len);\n> > +\t\tdest[len] = '\\0';\n> > +\t}\n> > +\treturn ret;\n> > +}\n> > +\n> > +\n> >   static int __init prom_memcmp(const void *cs, const void *ct, size_t count)\n> >   {\n> >   \tconst unsigned char *su1, *su2;\n> > @@ -304,6 +310,7 @@ static char __init *prom_strstr(const char *s1, const char *s2)\n> >   \treturn NULL;\n> >   }\n> > +#ifdef GENERIC_CMDLINE_NEED_STRLCAT\n> >   static size_t __init prom_strlcat(char *dest, const char *src, size_t count)\n> >   {\n> >   \tsize_t dsize = prom_strlen(dest);\n> > @@ -323,6 +330,7 @@ static size_t __init prom_strlcat(char *dest, const char *src, size_t count)\n> >   \treturn res;\n> >   }\n> > +#endif\n> >   #ifdef CONFIG_PPC_PSERIES\n> >   static int __init prom_strtobool(const char *s, bool *res)\n> > @@ -775,12 +783,11 @@ static void __init early_cmdline_parse(void)\n> >   \tprom_cmd_line[0] = 0;\n> >   \tp = prom_cmd_line;\n> > -\tif (!IS_ENABLED(CONFIG_CMDLINE_FORCE) && (long)prom.chosen > 0)\n> > +\tif ((long)prom.chosen > 0)\n> >   \t\tl = prom_getprop(prom.chosen, \"bootargs\", p, COMMAND_LINE_SIZE-1);\n> > -\tif (IS_ENABLED(CONFIG_CMDLINE_EXTEND) || l <= 0 || p[0] == '\\0')\n> > -\t\tprom_strlcat(prom_cmd_line, \" \" CONFIG_CMDLINE,\n> > -\t\t\t     sizeof(prom_cmd_line));\n> > +\tcmdline_add_builtin_custom(prom_cmd_line, (l > 0 ? p : NULL), sizeof(prom_cmd_line),\n> > +\t\t\t\t\t__prombss, prom_strlcpy, prom_strlcat);\n> \n> So we are referencing a function that doesn't exist (namely prom_strlcat).\n> But it works because cmdline_add_builtin_custom() looks like a function but\n> is in fact an obscure macro that doesn't use prom_strlcat() unless\n> GENERIC_CMDLINE_NEED_STRLCAT is defined.\n> \n> IMHO that's awful for readability and code maintenance.\n\npowerpc is a special case, there's no other users like this. The reason is\nbecause of all the difficulty in this prom_init.c code. A lot of the generic\ncode has similar kind of changes to work across architectures.\n\n\n> >   \tprom_printf(\"command line: %s\\n\", prom_cmd_line);\n> > @@ -2706,7 +2713,7 @@ static void __init flatten_device_tree(void)\n> >   \t/* Add \"phandle\" in there, we'll need it */\n> >   \tnamep = make_room(&mem_start, &mem_end, 16, 1);\n> > -\tprom_strcpy(namep, \"phandle\");\n> > +\tprom_strlcpy(namep, \"phandle\", 8);\n> \n> Should be in a separate patch.\n\nI can move it, I missed that from the first round.\n\nDaniel","headers":{"Return-Path":"\n <linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org>","X-Original-To":["patchwork-incoming@ozlabs.org","linuxppc-dev@lists.ozlabs.org"],"Delivered-To":["patchwork-incoming@ozlabs.org","linuxppc-dev@lists.ozlabs.org"],"Authentication-Results":["ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=lists.ozlabs.org\n (client-ip=112.213.38.117; helo=lists.ozlabs.org;\n envelope-from=linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org;\n receiver=<UNKNOWN>)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n unprotected) header.d=cisco.com header.i=@cisco.com header.a=rsa-sha256\n header.s=iport header.b=bzhTS6vn;\n\tdkim-atps=neutral","lists.ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n unprotected) header.d=cisco.com header.i=@cisco.com header.a=rsa-sha256\n header.s=iport header.b=bzhTS6vn;\n\tdkim-atps=neutral","lists.ozlabs.org; spf=pass (sender SPF authorized)\n smtp.mailfrom=cisco.com (client-ip=173.37.86.72; helo=rcdn-iport-1.cisco.com;\n envelope-from=danielwa@cisco.com; receiver=<UNKNOWN>)","lists.ozlabs.org; dkim=pass (1024-bit key;\n unprotected) header.d=cisco.com header.i=@cisco.com header.a=rsa-sha256\n header.s=iport header.b=bzhTS6vn; dkim-atps=neutral"],"Received":["from lists.ozlabs.org (lists.ozlabs.org [112.213.38.117])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange X25519 server-signature RSA-PSS (4096 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 4Dw7v30qkYz9sWQ\n\tfor <patchwork-incoming@ozlabs.org>; Wed, 10 Mar 2021 08:41:19 +1100 (AEDT)","from boromir.ozlabs.org (localhost [IPv6:::1])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 4Dw7v26mnHz3cZ2\n\tfor <patchwork-incoming@ozlabs.org>; Wed, 10 Mar 2021 08:41:18 +1100 (AEDT)","from rcdn-iport-1.cisco.com (rcdn-iport-1.cisco.com [173.37.86.72])\n (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n bits)) (No client certificate requested)\n by lists.ozlabs.org (Postfix) with ESMTPS id 4Dw7tf2fKDz3cGx\n for <linuxppc-dev@lists.ozlabs.org>; Wed, 10 Mar 2021 08:40:57 +1100 (AEDT)","from alln-core-6.cisco.com ([173.36.13.139])\n by rcdn-iport-1.cisco.com with ESMTP/TLS/DHE-RSA-SEED-SHA;\n 09 Mar 2021 21:40:54 +0000","from zorba ([10.24.4.5])\n by alln-core-6.cisco.com (8.15.2/8.15.2) with ESMTPS id 129Lepbs004030\n (version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO);\n Tue, 9 Mar 2021 21:40:52 GMT"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple;\n d=cisco.com; i=@cisco.com; l=7313; q=dns/txt; s=iport;\n t=1615326058; x=1616535658;\n h=date:from:to:cc:subject:message-id:references:\n mime-version:content-transfer-encoding:in-reply-to;\n bh=rW2dGFdz1Viogy9BvbMFOe3/vVZakbjcs4YVqhr4xkM=;\n b=bzhTS6vncYDmiGRICpLZGpKyFjxLnjIl8ietbzGVBC4bpF5rhncw+mhC\n KWzroLxuZCod7eO8yODHJQNlsLEZcQu0GKPLG5IO2gaqRkf6zSrlvhqAX\n Jnnye+uAduCM7InusmBOoCo8oQS7QFgWLYaJUqpPAfH01vsMV3Rn+DtP2 A=;","X-IronPort-AV":"E=Sophos;i=\"5.81,236,1610409600\"; d=\"scan'208\";a=\"861832335\"","Date":"Tue, 9 Mar 2021 13:40:51 -0800","From":"Daniel Walker <danielwa@cisco.com>","To":"Christophe Leroy <christophe.leroy@csgroup.eu>","Subject":"Re: [PATCH v2 4/7] CMDLINE: powerpc: convert to generic builtin\n command line","Message-ID":"<20210309214051.GS109100@zorba>","References":"<20210309000247.2989531-5-danielwa@cisco.com>\n <c5c8b57e-7954-ec02-188a-7f85cb0af731@csgroup.eu>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<c5c8b57e-7954-ec02-188a-7f85cb0af731@csgroup.eu>","X-Auto-Response-Suppress":"DR, OOF, AutoReply","X-Outbound-SMTP-Client":"10.24.4.5, [10.24.4.5]","X-Outbound-Node":"alln-core-6.cisco.com","X-BeenThere":"linuxppc-dev@lists.ozlabs.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"Linux on PowerPC Developers Mail List <linuxppc-dev.lists.ozlabs.org>","List-Unsubscribe":"<https://lists.ozlabs.org/options/linuxppc-dev>,\n <mailto:linuxppc-dev-request@lists.ozlabs.org?subject=unsubscribe>","List-Archive":"<http://lists.ozlabs.org/pipermail/linuxppc-dev/>","List-Post":"<mailto:linuxppc-dev@lists.ozlabs.org>","List-Help":"<mailto:linuxppc-dev-request@lists.ozlabs.org?subject=help>","List-Subscribe":"<https://lists.ozlabs.org/listinfo/linuxppc-dev>,\n <mailto:linuxppc-dev-request@lists.ozlabs.org?subject=subscribe>","Cc":"Rob Herring <robh@kernel.org>, Ruslan Ruslichenko <rruslich@cisco.com>,\n Ruslan Bilovol <rbilovol@cisco.com>,\n Daniel Gimpelevich <daniel@gimpelevich.san-francisco.ca.us>,\n linuxppc-dev@lists.ozlabs.org, x86@kernel.org, linux-mips@vger.kernel.org,\n linux-kernel@vger.kernel.org, Paul Mackerras <paulus@samba.org>,\n xe-linux-external@cisco.com, Andrew Morton <akpm@linux-foundation.org>,\n Will Deacon <will@kernel.org>","Errors-To":"linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org","Sender":"\"Linuxppc-dev\"\n <linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org>"}},{"id":2647614,"web_url":"http://patchwork.ozlabs.org/comment/2647614/","msgid":"<3cabc11d-96d1-962c-ab11-43a8c6d00657@csgroup.eu>","date":"2021-03-13T09:29:26","subject":"Re: [PATCH v2 4/7] CMDLINE: powerpc: convert to generic builtin\n command line","submitter":{"id":79086,"url":"http://patchwork.ozlabs.org/api/people/79086/","name":"Christophe Leroy","email":"christophe.leroy@csgroup.eu"},"content":"Le 09/03/2021 à 22:40, Daniel Walker a écrit :\n> On Tue, Mar 09, 2021 at 08:56:47AM +0100, Christophe Leroy wrote:\n>>\n>> So we are referencing a function that doesn't exist (namely prom_strlcat).\n>> But it works because cmdline_add_builtin_custom() looks like a function but\n>> is in fact an obscure macro that doesn't use prom_strlcat() unless\n>> GENERIC_CMDLINE_NEED_STRLCAT is defined.\n>>\n>> IMHO that's awful for readability and code maintenance.\n> \n> powerpc is a special case, there's no other users like this. The reason is\n> because of all the difficulty in this prom_init.c code. A lot of the generic\n> code has similar kind of changes to work across architectures.\n> \n\nI'd suggest the following (sorry if Thunderbird damages whitespaces, you'll get the idea anyway)\n\n\n\ndiff --git a/include/linux/cmdline.h b/include/linux/cmdline.h\nnew file mode 100644\nindex 000000000000..30b9eefc802f\n--- /dev/null\n+++ b/include/linux/cmdline.h\n@@ -0,0 +1,42 @@\n+/* SPDX-License-Identifier: GPL-2.0 */\n+#ifndef _LINUX_CMDLINE_H\n+#define _LINUX_CMDLINE_H\n+\n+#ifdef CONFIG_GENERIC_CMDLINE\n+\n+#ifndef cmdline_strlcpy\n+#define cmdline_strlcpy strlcpy\n+#endif\n+#ifndef cmdline_strlcat\n+#define cmdline_strlcat strlcat\n+#endif\n+\n+static __always_inline void\n+cmdline_add_builtin_custom(char *dest, const char *src, char *tmp, unsigned long length)\n+{\n+\tif (WARN_ON(sizeof(CONFIG_CMDLINE_PREPEND) > 1 &&\n+\t\t    !IS_ENABLED(CONFIG_CMDLINE_OVERRIDE) &&\n+\t\t    !tmp && src == dest))\n+\t\treturn;\n+\n+\tif (sizeof(CONFIG_CMDLINE_PREPEND) > 1 &&\n+\t    !IS_ENABLED(CONFIG_CMDLINE_OVERRIDE) && src == dest)\n+\t\tcmdline_strlcpy(tmp, src, length);\n+\telse\n+\t\ttmp = (char *)src;\n+\n+\tcmdline_strlcpy(dest, CONFIG_CMDLINE_PREPEND \" \", length);\n+\tif (!IS_ENABLED(CONFIG_CMDLINE_OVERRIDE) && tmp)\n+\t\tcmdline_strlcat(dest, tmp, length);\n+\tcmdline_strlcat(dest, \" \" CONFIG_CMDLINE_APPEND, length);\n+}\n+\n+#define cmdline_add_builtin(dest, src, length) do {\t\t\t\\\n+\tstatic __init char cmdline_tmp[length];\t\t\t\t\\\n+\t\t\t\t\t\t\t\t\t\\\n+\tcmdline_add_builtin_custom(dest, src, cmdline_tmp, length);\t\\\n+} while (0)\n+\n+#endif /* CONFIG_GENERIC_CMDLINE */\n+\n+#endif /* _LINUX_CMDLINE_H */\ndiff --git a/init/Kconfig b/init/Kconfig\nindex 22946fe5ded9..aeb134f0703b 100644\n--- a/init/Kconfig\n+++ b/init/Kconfig\n@@ -2035,6 +2035,27 @@ config PROFILING\n  config TRACEPOINTS\n  \tbool\n\n+config GENERIC_CMDLINE\n+\tbool\n+\n+if GENERIC_CMDLINE\n+\n+config CMDLINE_BOOL\n+\tbool \"Built-in kernel command line\"\n+\n+config CMDLINE_APPEND\n+\tstring \"Built-in kernel command string append\" if CMDLINE_BOOL\n+\tdefault \"\"\n+\n+config CMDLINE_PREPEND\n+\tstring \"Built-in kernel command string prepend\" if CMDLINE_BOOL\n+\tdefault \"\"\n+\n+config CMDLINE_OVERRIDE\n+\tbool \"Built-in command line overrides boot loader arguments\" if CMDLINE_BOOL\n+\n+endif\n+\n  endmenu\t\t# General setup\n\n  source \"arch/Kconfig\"","headers":{"Return-Path":"\n <linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org>","X-Original-To":["patchwork-incoming@ozlabs.org","linuxppc-dev@lists.ozlabs.org"],"Delivered-To":["patchwork-incoming@ozlabs.org","linuxppc-dev@lists.ozlabs.org"],"Authentication-Results":["ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=lists.ozlabs.org\n (client-ip=112.213.38.117; helo=lists.ozlabs.org;\n envelope-from=linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org;\n receiver=<UNKNOWN>)","lists.ozlabs.org; spf=pass (sender SPF authorized)\n smtp.mailfrom=csgroup.eu (client-ip=93.17.236.30; helo=pegase1.c-s.fr;\n envelope-from=christophe.leroy@csgroup.eu; receiver=<UNKNOWN>)"],"Received":["from lists.ozlabs.org (lists.ozlabs.org [112.213.38.117])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange X25519 server-signature RSA-PSS (4096 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 4DyHTG1Zltz9sW4\n\tfor <patchwork-incoming@ozlabs.org>; Sat, 13 Mar 2021 20:29:54 +1100 (AEDT)","from boromir.ozlabs.org (localhost [IPv6:::1])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 4DyHTG0VhVz3dKR\n\tfor <patchwork-incoming@ozlabs.org>; Sat, 13 Mar 2021 20:29:54 +1100 (AEDT)","from pegase1.c-s.fr (pegase1.c-s.fr [93.17.236.30])\n (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))\n (No client certificate requested)\n by lists.ozlabs.org (Postfix) with ESMTPS id 4DyHSx6Gdcz3cT1\n for <linuxppc-dev@lists.ozlabs.org>; Sat, 13 Mar 2021 20:29:33 +1100 (AEDT)","from localhost (mailhub1-int [192.168.12.234])\n by localhost (Postfix) with ESMTP id 4DyHSp14h4z9tvr3;\n Sat, 13 Mar 2021 10:29:30 +0100 (CET)","from pegase1.c-s.fr ([192.168.12.234])\n by localhost (pegase1.c-s.fr [192.168.12.234]) (amavisd-new, port 10024)\n with ESMTP id zaAMf4DmfJvl; Sat, 13 Mar 2021 10:29:30 +0100 (CET)","from messagerie.si.c-s.fr (messagerie.si.c-s.fr [192.168.25.192])\n by pegase1.c-s.fr (Postfix) with ESMTP id 4DyHSn63HLz9tvr2;\n Sat, 13 Mar 2021 10:29:29 +0100 (CET)","from localhost (localhost [127.0.0.1])\n by messagerie.si.c-s.fr (Postfix) with ESMTP id D50A08B769;\n Sat, 13 Mar 2021 10:29:30 +0100 (CET)","from messagerie.si.c-s.fr ([127.0.0.1])\n by localhost (messagerie.si.c-s.fr [127.0.0.1]) (amavisd-new, port 10023)\n with ESMTP id Wc7bhFyTgWK9; Sat, 13 Mar 2021 10:29:30 +0100 (CET)","from [192.168.4.90] (unknown [192.168.4.90])\n by messagerie.si.c-s.fr (Postfix) with ESMTP id E04948B75B;\n Sat, 13 Mar 2021 10:29:29 +0100 (CET)"],"X-Virus-Scanned":["Debian amavisd-new at c-s.fr","amavisd-new at c-s.fr"],"Subject":"Re: [PATCH v2 4/7] CMDLINE: powerpc: convert to generic builtin\n command line","To":"Daniel Walker <danielwa@cisco.com>","References":"<20210309000247.2989531-5-danielwa@cisco.com>\n <c5c8b57e-7954-ec02-188a-7f85cb0af731@csgroup.eu>\n <20210309214051.GS109100@zorba>","From":"Christophe Leroy <christophe.leroy@csgroup.eu>","Message-ID":"<3cabc11d-96d1-962c-ab11-43a8c6d00657@csgroup.eu>","Date":"Sat, 13 Mar 2021 10:29:26 +0100","User-Agent":"Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:78.0) Gecko/20100101\n Thunderbird/78.8.0","MIME-Version":"1.0","In-Reply-To":"<20210309214051.GS109100@zorba>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"fr","Content-Transfer-Encoding":"8bit","X-BeenThere":"linuxppc-dev@lists.ozlabs.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"Linux on PowerPC Developers Mail List <linuxppc-dev.lists.ozlabs.org>","List-Unsubscribe":"<https://lists.ozlabs.org/options/linuxppc-dev>,\n <mailto:linuxppc-dev-request@lists.ozlabs.org?subject=unsubscribe>","List-Archive":"<http://lists.ozlabs.org/pipermail/linuxppc-dev/>","List-Post":"<mailto:linuxppc-dev@lists.ozlabs.org>","List-Help":"<mailto:linuxppc-dev-request@lists.ozlabs.org?subject=help>","List-Subscribe":"<https://lists.ozlabs.org/listinfo/linuxppc-dev>,\n <mailto:linuxppc-dev-request@lists.ozlabs.org?subject=subscribe>","Cc":"Rob Herring <robh@kernel.org>, Ruslan Ruslichenko <rruslich@cisco.com>,\n Ruslan Bilovol <rbilovol@cisco.com>,\n Daniel Gimpelevich <daniel@gimpelevich.san-francisco.ca.us>,\n linuxppc-dev@lists.ozlabs.org, x86@kernel.org, linux-mips@vger.kernel.org,\n linux-kernel@vger.kernel.org, Paul Mackerras <paulus@samba.org>,\n xe-linux-external@cisco.com, Andrew Morton <akpm@linux-foundation.org>,\n Will Deacon <will@kernel.org>","Errors-To":"linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org","Sender":"\"Linuxppc-dev\"\n <linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org>"}},{"id":2654685,"web_url":"http://patchwork.ozlabs.org/comment/2654685/","msgid":"<9c5b8e33-026e-c9d6-c267-a5dd4a2b999c@csgroup.eu>","date":"2021-03-24T15:31:35","subject":"Re: [PATCH v2 4/7] CMDLINE: powerpc: convert to generic builtin\n command line","submitter":{"id":79086,"url":"http://patchwork.ozlabs.org/api/people/79086/","name":"Christophe Leroy","email":"christophe.leroy@csgroup.eu"},"content":"Le 09/03/2021 à 22:40, Daniel Walker a écrit :\n> On Tue, Mar 09, 2021 at 08:56:47AM +0100, Christophe Leroy wrote:\n>>\n>>\n>> Le 09/03/2021 à 01:02, Daniel Walker a écrit :\n>>> This updates the powerpc code to use the CONFIG_GENERIC_CMDLINE\n>>> option.\n>>>\n>>> Cc: xe-linux-external@cisco.com\n>>> Signed-off-by: Ruslan Ruslichenko <rruslich@cisco.com>\n>>> Signed-off-by: Ruslan Bilovol <rbilovol@cisco.com>\n>>> Signed-off-by: Daniel Walker <danielwa@cisco.com>\n>>> ---\n>>>    arch/powerpc/Kconfig            | 37 +--------------------------------\n>>>    arch/powerpc/kernel/prom.c      |  1 +\n>>>    arch/powerpc/kernel/prom_init.c | 35 ++++++++++++++++++-------------\n>>>    3 files changed, 23 insertions(+), 50 deletions(-)\n>>>\n>>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig\n>>> index 107bb4319e0e..276b06d5c961 100644\n>>> --- a/arch/powerpc/Kconfig\n>>> +++ b/arch/powerpc/Kconfig\n>>> @@ -167,6 +167,7 @@ config PPC\n>>>    \tselect EDAC_SUPPORT\n>>>    \tselect GENERIC_ATOMIC64\t\t\tif PPC32\n>>>    \tselect GENERIC_CLOCKEVENTS_BROADCAST\tif SMP\n>>> +\tselect GENERIC_CMDLINE\n>>>    \tselect GENERIC_CMOS_UPDATE\n>>>    \tselect GENERIC_CPU_AUTOPROBE\n>>>    \tselect GENERIC_CPU_VULNERABILITIES\tif PPC_BARRIER_NOSPEC\n>>> @@ -906,42 +907,6 @@ config PPC_DENORMALISATION\n>>>    \t  Add support for handling denormalisation of single precision\n>>>    \t  values.  Useful for bare metal only.  If unsure say Y here.\n>>> -config CMDLINE\n>>> -\tstring \"Initial kernel command string\"\n>>> -\tdefault \"\"\n>>> -\thelp\n>>> -\t  On some platforms, there is currently no way for the boot loader to\n>>> -\t  pass arguments to the kernel. For these platforms, you can supply\n>>> -\t  some command-line options at build time by entering them here.  In\n>>> -\t  most cases you will need to specify the root device here.\n>>> -\n>>> -choice\n>>> -\tprompt \"Kernel command line type\" if CMDLINE != \"\"\n>>> -\tdefault CMDLINE_FROM_BOOTLOADER\n>>> -\n>>> -config CMDLINE_FROM_BOOTLOADER\n>>> -\tbool \"Use bootloader kernel arguments if available\"\n>>> -\thelp\n>>> -\t  Uses the command-line options passed by the boot loader. If\n>>> -\t  the boot loader doesn't provide any, the default kernel command\n>>> -\t  string provided in CMDLINE will be used.\n\nI can't see how the above is supported in the generic builtin.\n\nTaking into account that it is the default on powerpc, I'm having hardtime with that.\n\n>>> -\n>>> -config CMDLINE_EXTEND\n>>> -\tbool \"Extend bootloader kernel arguments\"\n>>> -\thelp\n>>> -\t  The command-line arguments provided by the boot loader will be\n>>> -\t  appended to the default kernel command string.\n>>> -\n>>> -config CMDLINE_FORCE\n>>> -\tbool \"Always use the default kernel command string\"\n>>> -\thelp\n>>> -\t  Always use the default kernel command string, even if the boot\n>>> -\t  loader passes other arguments to the kernel.\n>>> -\t  This is useful if you cannot or don't want to change the\n>>> -\t  command-line options your boot loader passes to the kernel.\n>>> -\n>>> -endchoice\n>>> -\n>>>    config EXTRA_TARGETS\n>>>    \tstring \"Additional default image types\"\n>>>    \thelp\n>>> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c\n>>> index ae3c41730367..96d0a01be1b4 100644\n>>> --- a/arch/powerpc/kernel/prom.c\n>>> +++ b/arch/powerpc/kernel/prom.c\n>>> @@ -27,6 +27,7 @@\n>>>    #include <linux/irq.h>\n>>>    #include <linux/memblock.h>\n>>>    #include <linux/of.h>\n>>> +#include <linux/cmdline.h>\n>>\n>> Why is this needed in prom.c ?\n>   \n> Must have been a mistake, I don't think it's needed.\n> \n> \n>>>    #include <linux/of_fdt.h>\n>>>    #include <linux/libfdt.h>\n>>>    #include <linux/cpu.h>\n>>> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c\n>>> index e9d4eb6144e1..657241534d69 100644\n>>> --- a/arch/powerpc/kernel/prom_init.c\n>>> +++ b/arch/powerpc/kernel/prom_init.c\n>>> @@ -27,6 +27,7 @@\n>>>    #include <linux/initrd.h>\n>>>    #include <linux/bitops.h>\n>>>    #include <linux/pgtable.h>\n>>> +#include <linux/cmdline.h>\n>>>    #include <asm/prom.h>\n>>>    #include <asm/rtas.h>\n>>>    #include <asm/page.h>\n>>> @@ -242,15 +243,6 @@ static int __init prom_strcmp(const char *cs, const char *ct)\n>>>    \treturn 0;\n>>>    }\n>>> -static char __init *prom_strcpy(char *dest, const char *src)\n>>> -{\n>>> -\tchar *tmp = dest;\n>>> -\n>>> -\twhile ((*dest++ = *src++) != '\\0')\n>>> -\t\t/* nothing */;\n>>> -\treturn tmp;\n>>> -}\n>>> -\n>>\n>> This game with prom_strcpy() should go a separate preceeding patch.\n>>\n>> Also, it looks like checkpatch.pl recommends to use strscpy() instead of strlcpy().\n> \n> strscpy() is very large. I'm not sure it's compatible with this prom_init.c\n> environment.\n\nOk you are right, lets keep strlcpy() and ignore checkpatch complaints\n\n> \n>>>    static int __init prom_strncmp(const char *cs, const char *ct, size_t count)\n>>>    {\n>>>    \tunsigned char c1, c2;\n>>> @@ -276,6 +268,20 @@ static size_t __init prom_strlen(const char *s)\n>>>    \treturn sc - s;\n>>>    }\n>>> +static size_t __init prom_strlcpy(char *dest, const char *src, size_t size)\n>>> +{\n>>> +\tsize_t ret = prom_strlen(src);\n>>> +\n>>> +\tif (size) {\n>>> +\t\tsize_t len = (ret >= size) ? size - 1 : ret;\n>>> +\n>>> +\t\tmemcpy(dest, src, len);\n>>> +\t\tdest[len] = '\\0';\n>>> +\t}\n>>> +\treturn ret;\n>>> +}\n>>> +\n>>> +\n>>>    static int __init prom_memcmp(const void *cs, const void *ct, size_t count)\n>>>    {\n>>>    \tconst unsigned char *su1, *su2;\n>>> @@ -304,6 +310,7 @@ static char __init *prom_strstr(const char *s1, const char *s2)\n>>>    \treturn NULL;\n>>>    }\n>>> +#ifdef GENERIC_CMDLINE_NEED_STRLCAT\n>>>    static size_t __init prom_strlcat(char *dest, const char *src, size_t count)\n>>>    {\n>>>    \tsize_t dsize = prom_strlen(dest);\n>>> @@ -323,6 +330,7 @@ static size_t __init prom_strlcat(char *dest, const char *src, size_t count)\n>>>    \treturn res;\n>>>    }\n>>> +#endif\n>>>    #ifdef CONFIG_PPC_PSERIES\n>>>    static int __init prom_strtobool(const char *s, bool *res)\n>>> @@ -775,12 +783,11 @@ static void __init early_cmdline_parse(void)\n>>>    \tprom_cmd_line[0] = 0;\n>>>    \tp = prom_cmd_line;\n>>> -\tif (!IS_ENABLED(CONFIG_CMDLINE_FORCE) && (long)prom.chosen > 0)\n>>> +\tif ((long)prom.chosen > 0)\n>>>    \t\tl = prom_getprop(prom.chosen, \"bootargs\", p, COMMAND_LINE_SIZE-1);\n>>> -\tif (IS_ENABLED(CONFIG_CMDLINE_EXTEND) || l <= 0 || p[0] == '\\0')\n>>> -\t\tprom_strlcat(prom_cmd_line, \" \" CONFIG_CMDLINE,\n>>> -\t\t\t     sizeof(prom_cmd_line));\n>>> +\tcmdline_add_builtin_custom(prom_cmd_line, (l > 0 ? p : NULL), sizeof(prom_cmd_line),\n>>> +\t\t\t\t\t__prombss, prom_strlcpy, prom_strlcat);\n>>\n>> So we are referencing a function that doesn't exist (namely prom_strlcat).\n>> But it works because cmdline_add_builtin_custom() looks like a function but\n>> is in fact an obscure macro that doesn't use prom_strlcat() unless\n>> GENERIC_CMDLINE_NEED_STRLCAT is defined.\n>>\n>> IMHO that's awful for readability and code maintenance.\n> \n> powerpc is a special case, there's no other users like this. The reason is\n> because of all the difficulty in this prom_init.c code. A lot of the generic\n> code has similar kind of changes to work across architectures.\n\nAny feedback on the proposed changes I made on the 13th ? I know it is partly buggy but that was \nmore for the principle. I can make clean working patch if it helps.\n\n> \n> \n>>>    \tprom_printf(\"command line: %s\\n\", prom_cmd_line);\n>>> @@ -2706,7 +2713,7 @@ static void __init flatten_device_tree(void)\n>>>    \t/* Add \"phandle\" in there, we'll need it */\n>>>    \tnamep = make_room(&mem_start, &mem_end, 16, 1);\n>>> -\tprom_strcpy(namep, \"phandle\");\n>>> +\tprom_strlcpy(namep, \"phandle\", 8);\n>>\n>> Should be in a separate patch.\n> \n> I can move it, I missed that from the first round.\n> \n> Daniel\n> \n\nChristophe","headers":{"Return-Path":"\n <linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org>","X-Original-To":["patchwork-incoming@ozlabs.org","linuxppc-dev@lists.ozlabs.org"],"Delivered-To":["patchwork-incoming@ozlabs.org","linuxppc-dev@lists.ozlabs.org"],"Authentication-Results":["ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=lists.ozlabs.org\n (client-ip=112.213.38.117; helo=lists.ozlabs.org;\n envelope-from=linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org;\n receiver=<UNKNOWN>)","lists.ozlabs.org; spf=pass (sender SPF authorized)\n smtp.mailfrom=csgroup.eu (client-ip=93.17.236.30; helo=pegase1.c-s.fr;\n envelope-from=christophe.leroy@csgroup.eu; receiver=<UNKNOWN>)"],"Received":["from lists.ozlabs.org (lists.ozlabs.org [112.213.38.117])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange X25519 server-signature RSA-PSS (4096 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 4F5C0p3v8Yz9sTD\n\tfor <patchwork-incoming@ozlabs.org>; Thu, 25 Mar 2021 02:32:42 +1100 (AEDT)","from boromir.ozlabs.org (localhost [IPv6:::1])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 4F5C0p31kWz30RJ\n\tfor <patchwork-incoming@ozlabs.org>; Thu, 25 Mar 2021 02:32:42 +1100 (AEDT)","from pegase1.c-s.fr (pegase1.c-s.fr [93.17.236.30])\n (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))\n (No client certificate requested)\n by lists.ozlabs.org (Postfix) with ESMTPS id 4F5C0W1BRyz2yxb\n for <linuxppc-dev@lists.ozlabs.org>; Thu, 25 Mar 2021 02:32:23 +1100 (AEDT)","from localhost (mailhub1-int [192.168.12.234])\n by localhost (Postfix) with ESMTP id 4F5C0L2sW7z9v23n;\n Wed, 24 Mar 2021 16:32:18 +0100 (CET)","from pegase1.c-s.fr ([192.168.12.234])\n by localhost (pegase1.c-s.fr [192.168.12.234]) (amavisd-new, port 10024)\n with ESMTP id FoMssj87XpiI; Wed, 24 Mar 2021 16:32:18 +0100 (CET)","from messagerie.si.c-s.fr (messagerie.si.c-s.fr [192.168.25.192])\n by pegase1.c-s.fr (Postfix) with ESMTP id 4F5C0L1pWyz9v23m;\n Wed, 24 Mar 2021 16:32:18 +0100 (CET)","from localhost (localhost [127.0.0.1])\n by messagerie.si.c-s.fr (Postfix) with ESMTP id C1C068B82B;\n Wed, 24 Mar 2021 16:32:19 +0100 (CET)","from messagerie.si.c-s.fr ([127.0.0.1])\n by localhost (messagerie.si.c-s.fr [127.0.0.1]) (amavisd-new, port 10023)\n with ESMTP id IYkaSLfBGIjX; Wed, 24 Mar 2021 16:32:19 +0100 (CET)","from [192.168.4.90] (unknown [192.168.4.90])\n by messagerie.si.c-s.fr (Postfix) with ESMTP id A77598B812;\n Wed, 24 Mar 2021 16:32:18 +0100 (CET)"],"X-Virus-Scanned":["Debian amavisd-new at c-s.fr","amavisd-new at c-s.fr"],"Subject":"Re: [PATCH v2 4/7] CMDLINE: powerpc: convert to generic builtin\n command line","To":"Daniel Walker <danielwa@cisco.com>","References":"<20210309000247.2989531-5-danielwa@cisco.com>\n <c5c8b57e-7954-ec02-188a-7f85cb0af731@csgroup.eu>\n <20210309214051.GS109100@zorba>","From":"Christophe Leroy <christophe.leroy@csgroup.eu>","Message-ID":"<9c5b8e33-026e-c9d6-c267-a5dd4a2b999c@csgroup.eu>","Date":"Wed, 24 Mar 2021 16:31:35 +0100","User-Agent":"Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:78.0) Gecko/20100101\n Thunderbird/78.8.1","MIME-Version":"1.0","In-Reply-To":"<20210309214051.GS109100@zorba>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"fr","Content-Transfer-Encoding":"8bit","X-BeenThere":"linuxppc-dev@lists.ozlabs.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"Linux on PowerPC Developers Mail List <linuxppc-dev.lists.ozlabs.org>","List-Unsubscribe":"<https://lists.ozlabs.org/options/linuxppc-dev>,\n <mailto:linuxppc-dev-request@lists.ozlabs.org?subject=unsubscribe>","List-Archive":"<http://lists.ozlabs.org/pipermail/linuxppc-dev/>","List-Post":"<mailto:linuxppc-dev@lists.ozlabs.org>","List-Help":"<mailto:linuxppc-dev-request@lists.ozlabs.org?subject=help>","List-Subscribe":"<https://lists.ozlabs.org/listinfo/linuxppc-dev>,\n <mailto:linuxppc-dev-request@lists.ozlabs.org?subject=subscribe>","Cc":"Rob Herring <robh@kernel.org>, Ruslan Ruslichenko <rruslich@cisco.com>,\n Ruslan Bilovol <rbilovol@cisco.com>,\n Daniel Gimpelevich <daniel@gimpelevich.san-francisco.ca.us>,\n linuxppc-dev@lists.ozlabs.org, x86@kernel.org, linux-mips@vger.kernel.org,\n linux-kernel@vger.kernel.org, Paul Mackerras <paulus@samba.org>,\n xe-linux-external@cisco.com, Andrew Morton <akpm@linux-foundation.org>,\n Will Deacon <will@kernel.org>","Errors-To":"linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org","Sender":"\"Linuxppc-dev\"\n <linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org>"}},{"id":2655696,"web_url":"http://patchwork.ozlabs.org/comment/2655696/","msgid":"<20210325200357.GN109100@zorba>","date":"2021-03-25T20:03:57","subject":"Re: [PATCH v2 4/7] CMDLINE: powerpc: convert to generic builtin\n command line","submitter":{"id":67374,"url":"http://patchwork.ozlabs.org/api/people/67374/","name":"Daniel Walker (danielwa)","email":"danielwa@cisco.com"},"content":"On Wed, Mar 24, 2021 at 04:31:35PM +0100, Christophe Leroy wrote:\n> \n> \n> Le 09/03/2021 à 22:40, Daniel Walker a écrit :\n> > On Tue, Mar 09, 2021 at 08:56:47AM +0100, Christophe Leroy wrote:\n> > > \n> > > \n> > > Le 09/03/2021 à 01:02, Daniel Walker a écrit :\n> > > > This updates the powerpc code to use the CONFIG_GENERIC_CMDLINE\n> > > > option.\n> > > > \n> > > > Cc: xe-linux-external@cisco.com\n> > > > Signed-off-by: Ruslan Ruslichenko <rruslich@cisco.com>\n> > > > Signed-off-by: Ruslan Bilovol <rbilovol@cisco.com>\n> > > > Signed-off-by: Daniel Walker <danielwa@cisco.com>\n> > > > ---\n> > > >    arch/powerpc/Kconfig            | 37 +--------------------------------\n> > > >    arch/powerpc/kernel/prom.c      |  1 +\n> > > >    arch/powerpc/kernel/prom_init.c | 35 ++++++++++++++++++-------------\n> > > >    3 files changed, 23 insertions(+), 50 deletions(-)\n> > > > \n> > > > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig\n> > > > index 107bb4319e0e..276b06d5c961 100644\n> > > > --- a/arch/powerpc/Kconfig\n> > > > +++ b/arch/powerpc/Kconfig\n> > > > @@ -167,6 +167,7 @@ config PPC\n> > > >    \tselect EDAC_SUPPORT\n> > > >    \tselect GENERIC_ATOMIC64\t\t\tif PPC32\n> > > >    \tselect GENERIC_CLOCKEVENTS_BROADCAST\tif SMP\n> > > > +\tselect GENERIC_CMDLINE\n> > > >    \tselect GENERIC_CMOS_UPDATE\n> > > >    \tselect GENERIC_CPU_AUTOPROBE\n> > > >    \tselect GENERIC_CPU_VULNERABILITIES\tif PPC_BARRIER_NOSPEC\n> > > > @@ -906,42 +907,6 @@ config PPC_DENORMALISATION\n> > > >    \t  Add support for handling denormalisation of single precision\n> > > >    \t  values.  Useful for bare metal only.  If unsure say Y here.\n> > > > -config CMDLINE\n> > > > -\tstring \"Initial kernel command string\"\n> > > > -\tdefault \"\"\n> > > > -\thelp\n> > > > -\t  On some platforms, there is currently no way for the boot loader to\n> > > > -\t  pass arguments to the kernel. For these platforms, you can supply\n> > > > -\t  some command-line options at build time by entering them here.  In\n> > > > -\t  most cases you will need to specify the root device here.\n> > > > -\n> > > > -choice\n> > > > -\tprompt \"Kernel command line type\" if CMDLINE != \"\"\n> > > > -\tdefault CMDLINE_FROM_BOOTLOADER\n> > > > -\n> > > > -config CMDLINE_FROM_BOOTLOADER\n> > > > -\tbool \"Use bootloader kernel arguments if available\"\n> > > > -\thelp\n> > > > -\t  Uses the command-line options passed by the boot loader. If\n> > > > -\t  the boot loader doesn't provide any, the default kernel command\n> > > > -\t  string provided in CMDLINE will be used.\n> \n> I can't see how the above is supported in the generic builtin.\n> \n> Taking into account that it is the default on powerpc, I'm having hardtime with that.\n\nHmm, so this ignores the built in changes. You just don't enable it, or you\ndon't add PREPEND or APPEND.\n\n> Any feedback on the proposed changes I made on the 13th ? I know it is\n> partly buggy but that was more for the principle. I can make clean working\n> patch if it helps.\n\n\nThe reason I added it into the function parameters is because I can get free\ntype checking on the functions. If you use macro's then you don't know if the\nfunction is compatible.\n\nDaniel","headers":{"Return-Path":"\n <linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org>","X-Original-To":["patchwork-incoming@ozlabs.org","linuxppc-dev@lists.ozlabs.org"],"Delivered-To":["patchwork-incoming@ozlabs.org","linuxppc-dev@lists.ozlabs.org"],"Authentication-Results":["ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=lists.ozlabs.org\n (client-ip=112.213.38.117; helo=lists.ozlabs.org;\n envelope-from=linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org;\n receiver=<UNKNOWN>)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n unprotected) header.d=cisco.com header.i=@cisco.com header.a=rsa-sha256\n header.s=iport header.b=ZZgTJSHV;\n\tdkim-atps=neutral","lists.ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n unprotected) header.d=cisco.com header.i=@cisco.com header.a=rsa-sha256\n header.s=iport header.b=ZZgTJSHV;\n\tdkim-atps=neutral","lists.ozlabs.org; spf=pass (sender SPF authorized)\n smtp.mailfrom=cisco.com (client-ip=173.37.86.72; helo=rcdn-iport-1.cisco.com;\n envelope-from=danielwa@cisco.com; receiver=<UNKNOWN>)","lists.ozlabs.org; dkim=pass (1024-bit key;\n unprotected) header.d=cisco.com header.i=@cisco.com header.a=rsa-sha256\n header.s=iport header.b=ZZgTJSHV; dkim-atps=neutral"],"Received":["from lists.ozlabs.org (lists.ozlabs.org [112.213.38.117])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange X25519 server-signature RSA-PSS (4096 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 4F5wzq6KnFz9sSC\n\tfor <patchwork-incoming@ozlabs.org>; Fri, 26 Mar 2021 07:04:23 +1100 (AEDT)","from boromir.ozlabs.org (localhost [IPv6:::1])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 4F5wzq5VRPz3bxs\n\tfor <patchwork-incoming@ozlabs.org>; Fri, 26 Mar 2021 07:04:23 +1100 (AEDT)","from rcdn-iport-1.cisco.com (rcdn-iport-1.cisco.com [173.37.86.72])\n (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n bits)) (No client certificate requested)\n by lists.ozlabs.org (Postfix) with ESMTPS id 4F5wzR6PpCz3bcN\n for <linuxppc-dev@lists.ozlabs.org>; Fri, 26 Mar 2021 07:04:03 +1100 (AEDT)","from alln-core-4.cisco.com ([173.36.13.137])\n by rcdn-iport-1.cisco.com with ESMTP/TLS/DHE-RSA-SEED-SHA;\n 25 Mar 2021 20:04:00 +0000","from zorba ([10.24.0.17])\n by alln-core-4.cisco.com (8.15.2/8.15.2) with ESMTPS id 12PK3w7e004051\n (version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO);\n Thu, 25 Mar 2021 20:03:59 GMT"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple;\n d=cisco.com; i=@cisco.com; l=3186; q=dns/txt; s=iport;\n t=1616702644; x=1617912244;\n h=date:from:to:cc:subject:message-id:references:\n mime-version:content-transfer-encoding:in-reply-to;\n bh=BEUcHvcrI68gqzYOcJB+2Eo5dzEnIS+cOJz0im7Ye0s=;\n b=ZZgTJSHVAEC67SN90+G70RIRbSPGcG+2uWPBTc20R9dxGpZ2gtROcUB3\n GV7GaAKoz4Z5p2SrAWX+ErVMQ3okewsKFHcBVHPjBk5IE+ZGwXVMalnDu\n xb0213ojD0fCA+xJ7F4QWp0Nxe6fzlv2/N9qjZjk/G5WWTTsjc3gM4tsn I=;","X-IronPort-AV":"E=Sophos;i=\"5.81,278,1610409600\"; d=\"scan'208\";a=\"867855113\"","Date":"Thu, 25 Mar 2021 13:03:57 -0700","From":"Daniel Walker <danielwa@cisco.com>","To":"Christophe Leroy <christophe.leroy@csgroup.eu>","Subject":"Re: [PATCH v2 4/7] CMDLINE: powerpc: convert to generic builtin\n command line","Message-ID":"<20210325200357.GN109100@zorba>","References":"<20210309000247.2989531-5-danielwa@cisco.com>\n <c5c8b57e-7954-ec02-188a-7f85cb0af731@csgroup.eu>\n <20210309214051.GS109100@zorba>\n <9c5b8e33-026e-c9d6-c267-a5dd4a2b999c@csgroup.eu>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<9c5b8e33-026e-c9d6-c267-a5dd4a2b999c@csgroup.eu>","X-Auto-Response-Suppress":"DR, OOF, AutoReply","X-Outbound-SMTP-Client":"10.24.0.17, [10.24.0.17]","X-Outbound-Node":"alln-core-4.cisco.com","X-BeenThere":"linuxppc-dev@lists.ozlabs.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"Linux on PowerPC Developers Mail List <linuxppc-dev.lists.ozlabs.org>","List-Unsubscribe":"<https://lists.ozlabs.org/options/linuxppc-dev>,\n <mailto:linuxppc-dev-request@lists.ozlabs.org?subject=unsubscribe>","List-Archive":"<http://lists.ozlabs.org/pipermail/linuxppc-dev/>","List-Post":"<mailto:linuxppc-dev@lists.ozlabs.org>","List-Help":"<mailto:linuxppc-dev-request@lists.ozlabs.org?subject=help>","List-Subscribe":"<https://lists.ozlabs.org/listinfo/linuxppc-dev>,\n <mailto:linuxppc-dev-request@lists.ozlabs.org?subject=subscribe>","Cc":"Rob Herring <robh@kernel.org>, Ruslan Ruslichenko <rruslich@cisco.com>,\n Ruslan Bilovol <rbilovol@cisco.com>,\n Daniel Gimpelevich <daniel@gimpelevich.san-francisco.ca.us>,\n linuxppc-dev@lists.ozlabs.org, x86@kernel.org, linux-mips@vger.kernel.org,\n linux-kernel@vger.kernel.org, Paul Mackerras <paulus@samba.org>,\n xe-linux-external@cisco.com, Andrew Morton <akpm@linux-foundation.org>,\n Will Deacon <will@kernel.org>","Errors-To":"linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org","Sender":"\"Linuxppc-dev\"\n <linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org>"}}]