* [PATCH 0/2] RC: rework U-boot arguments handling @ 2019-02-12 15:39 Eugeniy Paltsev 2019-02-12 15:39 ` [PATCH 1/2] ARC: U-boot: check arguments paranoidly Eugeniy Paltsev 2019-02-12 15:39 ` [PATCH 2/2] ARC: enable uboot support unconditionally Eugeniy Paltsev 0 siblings, 2 replies; 8+ messages in thread From: Eugeniy Paltsev @ 2019-02-12 15:39 UTC (permalink / raw) To: linux-snps-arc, Vineet Gupta Cc: linux-kernel, Alexey Brodkin, Corentin Labbe, khilman, Eugeniy Paltsev Reworking U-boot args handling and enable uboot support unconditionally. Changes RFC->v1: * Don't add new ABI contract between kernel and uboot * Eliminate CONFIG_ARC_UBOOT_SUPPORT Kconfig option and enable uboot support unconditionally * Skip invalid U-boot args instead of panic * Check existing U-boot magic value * Improve uboot_arg validating * Minor code changes Eugeniy Paltsev (2): ARC: U-boot: check arguments paranoidly ARC: enable uboot support unconditionally arch/arc/Kconfig | 12 ----- arch/arc/configs/nps_defconfig | 1 - arch/arc/configs/vdk_hs38_defconfig | 1 - arch/arc/configs/vdk_hs38_smp_defconfig | 2 - arch/arc/kernel/head.S | 7 ++- arch/arc/kernel/setup.c | 96 +++++++++++++++++++++++---------- 6 files changed, 70 insertions(+), 49 deletions(-) -- 2.14.5 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] ARC: U-boot: check arguments paranoidly 2019-02-12 15:39 [PATCH 0/2] RC: rework U-boot arguments handling Eugeniy Paltsev @ 2019-02-12 15:39 ` Eugeniy Paltsev 2019-02-12 16:39 ` LABBE Corentin 2019-02-12 16:45 ` Vineet Gupta 2019-02-12 15:39 ` [PATCH 2/2] ARC: enable uboot support unconditionally Eugeniy Paltsev 1 sibling, 2 replies; 8+ messages in thread From: Eugeniy Paltsev @ 2019-02-12 15:39 UTC (permalink / raw) To: linux-snps-arc, Vineet Gupta Cc: linux-kernel, Alexey Brodkin, Corentin Labbe, khilman, Eugeniy Paltsev Handle U-boot arguments paranoidly: * don't allow to pass unknown tag. * try to use external device tree blob only if corresponding tag (TAG_DTB) is set. * check that magic number is correct. * don't check uboot_tag if kernel build with no ARC_UBOOT_SUPPORT. NOTE: If U-boot args are invalid we skip them and try to use embedded device tree blob. We can't panic on invalid U-boot args as we really pass invalid args due to bug in U-boot code. This happens if we don't provide external DTB to U-boot and don't set 'bootargs' U-boot environment variable (which is default case at least for HSDK board) In that case we will pass {r0 = 1 (bootargs in r2); r1 = 0; r2 = 0;} to linux which is invalid. NOTE: We can safely check U-boot magic value (0x0) in linux passed via r1 register as U-boot pass it from the beginning. While I'm at it refactor U-boot arguments handling code. Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> --- arch/arc/kernel/head.S | 5 +-- arch/arc/kernel/setup.c | 92 +++++++++++++++++++++++++++++++++++-------------- 2 files changed, 69 insertions(+), 28 deletions(-) diff --git a/arch/arc/kernel/head.S b/arch/arc/kernel/head.S index 8b90d25a15cc..fccea361e896 100644 --- a/arch/arc/kernel/head.S +++ b/arch/arc/kernel/head.S @@ -93,10 +93,11 @@ ENTRY(stext) #ifdef CONFIG_ARC_UBOOT_SUPPORT ; Uboot - kernel ABI ; r0 = [0] No uboot interaction, [1] cmdline in r2, [2] DTB in r2 - ; r1 = magic number (board identity, unused as of now + ; r1 = magic number (always zero as of now) ; r2 = pointer to uboot provided cmdline or external DTB in mem - ; These are handled later in setup_arch() + ; These are handled later in handle_uboot_args() st r0, [@uboot_tag] + st r1, [@uboot_magic] st r2, [@uboot_arg] #endif diff --git a/arch/arc/kernel/setup.c b/arch/arc/kernel/setup.c index feb90093e6b1..84d394a37e79 100644 --- a/arch/arc/kernel/setup.c +++ b/arch/arc/kernel/setup.c @@ -36,7 +36,8 @@ unsigned int intr_to_DE_cnt; /* Part of U-boot ABI: see head.S */ int __initdata uboot_tag; -char __initdata *uboot_arg; +int __initdata uboot_magic; +unsigned int __initdata uboot_arg; const struct machine_desc *machine_desc; @@ -462,43 +463,82 @@ void setup_processor(void) arc_chk_core_config(); } -static inline int is_kernel(unsigned long addr) +static inline bool uboot_arg_invalid(unsigned int addr) { - if (addr >= (unsigned long)_stext && addr <= (unsigned long)_end) - return 1; - return 0; + /* + * Check that it is a untranslated address (although MMU is not enabled + * yet, it being a high address ensures this is not by fluke) + */ + if (addr < PAGE_OFFSET) + return true; + + /* Check that address doesn't clobber resident kernel image */ + return addr >= (unsigned int)_stext && addr <= (unsigned int)_end; } -void __init setup_arch(char **cmdline_p) +#define IGNORE_ARGS "Ignore U-boot args: " + +/* uboot_{tag, magic} values for U-boot - kernel ABI revision 0; see head.S */ +#define UBOOT_TAG_NONE 0 +#define UBOOT_TAG_CMDLINE 1 +#define UBOOT_TAG_DTB 2 +/* We always pass 0 as magic from U-boot */ +#define UBOOT_MAGIC_VAL 0 + +void __init handle_uboot_args(void) { + bool use_embedded_dtb = true; + bool append_cmdline = false; + #ifdef CONFIG_ARC_UBOOT_SUPPORT - /* make sure that uboot passed pointer to cmdline/dtb is valid */ - if (uboot_tag && is_kernel((unsigned long)uboot_arg)) - panic("Invalid uboot arg\n"); + /* check that we know this tag */ + if (uboot_tag != UBOOT_TAG_NONE && + uboot_tag != UBOOT_TAG_CMDLINE && + uboot_tag != UBOOT_TAG_DTB) { + pr_warn(IGNORE_ARGS "invalid uboot tag: '%08x'\n", uboot_tag); + goto ignore_uboot_args; + } + + if (uboot_magic != UBOOT_MAGIC_VAL) { + pr_warn(IGNORE_ARGS "non zero uboot magic\n"); + goto ignore_uboot_args; + } + + if (uboot_tag != UBOOT_TAG_NONE && uboot_arg_invalid(uboot_arg)) { + pr_warn(IGNORE_ARGS "invalid uboot arg: '%08x'\n", uboot_arg); + goto ignore_uboot_args; + } + + /* see if U-boot passed an external Device Tree blob */ + if (uboot_tag == UBOOT_TAG_DTB) { + machine_desc = setup_machine_fdt((void *)uboot_arg); + + /* external Device Tree blob is invalid - use embedded one */ + use_embedded_dtb = !machine_desc; + } + + if (uboot_tag == UBOOT_TAG_CMDLINE) + append_cmdline = true; - /* See if u-boot passed an external Device Tree blob */ - machine_desc = setup_machine_fdt(uboot_arg); /* uboot_tag == 2 */ - if (!machine_desc) +ignore_uboot_args: #endif - { - /* No, so try the embedded one */ + + if (use_embedded_dtb) { machine_desc = setup_machine_fdt(__dtb_start); if (!machine_desc) panic("Embedded DT invalid\n"); + } - /* - * If we are here, it is established that @uboot_arg didn't - * point to DT blob. Instead if u-boot says it is cmdline, - * append to embedded DT cmdline. - * setup_machine_fdt() would have populated @boot_command_line - */ - if (uboot_tag == 1) { - /* Ensure a whitespace between the 2 cmdlines */ - strlcat(boot_command_line, " ", COMMAND_LINE_SIZE); - strlcat(boot_command_line, uboot_arg, - COMMAND_LINE_SIZE); - } + if (append_cmdline) { + /* Ensure a whitespace between the 2 cmdlines */ + strlcat(boot_command_line, " ", COMMAND_LINE_SIZE); + strlcat(boot_command_line, (char *)uboot_arg, COMMAND_LINE_SIZE); } +} + +void __init setup_arch(char **cmdline_p) +{ + handle_uboot_args(); /* Save unparsed command line copy for /proc/cmdline */ *cmdline_p = boot_command_line; -- 2.14.5 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] ARC: U-boot: check arguments paranoidly 2019-02-12 15:39 ` [PATCH 1/2] ARC: U-boot: check arguments paranoidly Eugeniy Paltsev @ 2019-02-12 16:39 ` LABBE Corentin 2019-02-12 16:41 ` Vineet Gupta 2019-02-12 16:45 ` Vineet Gupta 1 sibling, 1 reply; 8+ messages in thread From: LABBE Corentin @ 2019-02-12 16:39 UTC (permalink / raw) To: Eugeniy Paltsev Cc: linux-snps-arc, Vineet Gupta, linux-kernel, Alexey Brodkin, khilman On Tue, Feb 12, 2019 at 06:39:31PM +0300, Eugeniy Paltsev wrote: > Handle U-boot arguments paranoidly: > * don't allow to pass unknown tag. > * try to use external device tree blob only if corresponding tag > (TAG_DTB) is set. > * check that magic number is correct. > * don't check uboot_tag if kernel build with no ARC_UBOOT_SUPPORT. > > NOTE: > If U-boot args are invalid we skip them and try to use embedded device > tree blob. We can't panic on invalid U-boot args as we really pass > invalid args due to bug in U-boot code. > This happens if we don't provide external DTB to U-boot and > don't set 'bootargs' U-boot environment variable (which is default > case at least for HSDK board) In that case we will pass > {r0 = 1 (bootargs in r2); r1 = 0; r2 = 0;} to linux which is invalid. > > NOTE: > We can safely check U-boot magic value (0x0) in linux passed via > r1 register as U-boot pass it from the beginning. > > While I'm at it refactor U-boot arguments handling code. > Hello I have tried to test this serie, but this patch does not apply anymore on current next tree. It conflicts with "ARC: boot: robustify u-boot arg referencing". Regards ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] ARC: U-boot: check arguments paranoidly 2019-02-12 16:39 ` LABBE Corentin @ 2019-02-12 16:41 ` Vineet Gupta 0 siblings, 0 replies; 8+ messages in thread From: Vineet Gupta @ 2019-02-12 16:41 UTC (permalink / raw) To: LABBE Corentin, Eugeniy Paltsev Cc: linux-snps-arc, Vineet Gupta, linux-kernel, Alexey Brodkin, khilman On 2/12/19 8:39 AM, LABBE Corentin wrote: >> While I'm at it refactor U-boot arguments handling code. >> > Hello > > I have tried to test this serie, but this patch does not apply anymore on current next tree. > It conflicts with "ARC: boot: robustify u-boot arg referencing". I was carrying that patch in the interim - now dropped and pushed. -Vineet ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] ARC: U-boot: check arguments paranoidly 2019-02-12 15:39 ` [PATCH 1/2] ARC: U-boot: check arguments paranoidly Eugeniy Paltsev 2019-02-12 16:39 ` LABBE Corentin @ 2019-02-12 16:45 ` Vineet Gupta 2019-02-12 17:25 ` Eugeniy Paltsev 1 sibling, 1 reply; 8+ messages in thread From: Vineet Gupta @ 2019-02-12 16:45 UTC (permalink / raw) To: Eugeniy Paltsev, linux-snps-arc Cc: linux-kernel, Alexey Brodkin, Corentin Labbe, khilman On 2/12/19 7:39 AM, Eugeniy Paltsev wrote: > Handle U-boot arguments paranoidly: > * don't allow to pass unknown tag. > * try to use external device tree blob only if corresponding tag > (TAG_DTB) is set. > * check that magic number is correct. > * don't check uboot_tag if kernel build with no ARC_UBOOT_SUPPORT. > > NOTE: > If U-boot args are invalid we skip them and try to use embedded device > tree blob. We can't panic on invalid U-boot args as we really pass > invalid args due to bug in U-boot code. > This happens if we don't provide external DTB to U-boot and > don't set 'bootargs' U-boot environment variable (which is default > case at least for HSDK board) In that case we will pass > {r0 = 1 (bootargs in r2); r1 = 0; r2 = 0;} to linux which is invalid. > > NOTE: > We can safely check U-boot magic value (0x0) in linux passed via > r1 register as U-boot pass it from the beginning. > > While I'm at it refactor U-boot arguments handling code. > > Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> > --- > arch/arc/kernel/head.S | 5 +-- > arch/arc/kernel/setup.c | 92 +++++++++++++++++++++++++++++++++++-------------- > 2 files changed, 69 insertions(+), 28 deletions(-) > > diff --git a/arch/arc/kernel/head.S b/arch/arc/kernel/head.S > index 8b90d25a15cc..fccea361e896 100644 > --- a/arch/arc/kernel/head.S > +++ b/arch/arc/kernel/head.S > @@ -93,10 +93,11 @@ ENTRY(stext) > #ifdef CONFIG_ARC_UBOOT_SUPPORT > ; Uboot - kernel ABI > ; r0 = [0] No uboot interaction, [1] cmdline in r2, [2] DTB in r2 > - ; r1 = magic number (board identity, unused as of now > + ; r1 = magic number (always zero as of now) This is technically changing the ABI - I think we don't need to enforce this - keep ignoring this > ; r2 = pointer to uboot provided cmdline or external DTB in mem > - ; These are handled later in setup_arch() > + ; These are handled later in handle_uboot_args() > st r0, [@uboot_tag] > + st r1, [@uboot_magic] > st r2, [@uboot_arg] > #endif > > diff --git a/arch/arc/kernel/setup.c b/arch/arc/kernel/setup.c > index feb90093e6b1..84d394a37e79 100644 > --- a/arch/arc/kernel/setup.c > +++ b/arch/arc/kernel/setup.c > @@ -36,7 +36,8 @@ unsigned int intr_to_DE_cnt; > > /* Part of U-boot ABI: see head.S */ > int __initdata uboot_tag; > -char __initdata *uboot_arg; > +int __initdata uboot_magic; > +unsigned int __initdata uboot_arg; > > const struct machine_desc *machine_desc; > > @@ -462,43 +463,82 @@ void setup_processor(void) > arc_chk_core_config(); > } > > -static inline int is_kernel(unsigned long addr) > +static inline bool uboot_arg_invalid(unsigned int addr) > { > - if (addr >= (unsigned long)_stext && addr <= (unsigned long)_end) > - return 1; > - return 0; > + /* > + * Check that it is a untranslated address (although MMU is not enabled > + * yet, it being a high address ensures this is not by fluke) > + */ > + if (addr < PAGE_OFFSET) > + return true; > + > + /* Check that address doesn't clobber resident kernel image */ > + return addr >= (unsigned int)_stext && addr <= (unsigned int)_end; > } > > -void __init setup_arch(char **cmdline_p) > +#define IGNORE_ARGS "Ignore U-boot args: " > + > +/* uboot_{tag, magic} values for U-boot - kernel ABI revision 0; see head.S */ > +#define UBOOT_TAG_NONE 0 > +#define UBOOT_TAG_CMDLINE 1 > +#define UBOOT_TAG_DTB 2 > +/* We always pass 0 as magic from U-boot */ > +#define UBOOT_MAGIC_VAL 0 > + > +void __init handle_uboot_args(void) > { > + bool use_embedded_dtb = true; > + bool append_cmdline = false; > + > #ifdef CONFIG_ARC_UBOOT_SUPPORT > - /* make sure that uboot passed pointer to cmdline/dtb is valid */ > - if (uboot_tag && is_kernel((unsigned long)uboot_arg)) > - panic("Invalid uboot arg\n"); > + /* check that we know this tag */ > + if (uboot_tag != UBOOT_TAG_NONE && > + uboot_tag != UBOOT_TAG_CMDLINE && > + uboot_tag != UBOOT_TAG_DTB) { > + pr_warn(IGNORE_ARGS "invalid uboot tag: '%08x'\n", uboot_tag); > + goto ignore_uboot_args; > + } > + > + if (uboot_magic != UBOOT_MAGIC_VAL) { > + pr_warn(IGNORE_ARGS "non zero uboot magic\n"); > + goto ignore_uboot_args; > + } Not needed per above. > + > + if (uboot_tag != UBOOT_TAG_NONE && uboot_arg_invalid(uboot_arg)) { > + pr_warn(IGNORE_ARGS "invalid uboot arg: '%08x'\n", uboot_arg); > + goto ignore_uboot_args; > + } > + > + /* see if U-boot passed an external Device Tree blob */ > + if (uboot_tag == UBOOT_TAG_DTB) { > + machine_desc = setup_machine_fdt((void *)uboot_arg); > + > + /* external Device Tree blob is invalid - use embedded one */ > + use_embedded_dtb = !machine_desc; > + } > + > + if (uboot_tag == UBOOT_TAG_CMDLINE) > + append_cmdline = true; > > - /* See if u-boot passed an external Device Tree blob */ > - machine_desc = setup_machine_fdt(uboot_arg); /* uboot_tag == 2 */ > - if (!machine_desc) > +ignore_uboot_args: > #endif > - { > - /* No, so try the embedded one */ > + > + if (use_embedded_dtb) { > machine_desc = setup_machine_fdt(__dtb_start); > if (!machine_desc) > panic("Embedded DT invalid\n"); > + } > > - /* > - * If we are here, it is established that @uboot_arg didn't > - * point to DT blob. Instead if u-boot says it is cmdline, > - * append to embedded DT cmdline. > - * setup_machine_fdt() would have populated @boot_command_line > - */ Don't drop this comment, specially the last line. If was tempted to move the cmd line processing before but this saved me since we rely on setup_machine_fdt() being called aprioiri. > - if (uboot_tag == 1) { > - /* Ensure a whitespace between the 2 cmdlines */ > - strlcat(boot_command_line, " ", COMMAND_LINE_SIZE); > - strlcat(boot_command_line, uboot_arg, > - COMMAND_LINE_SIZE); > - } > + if (append_cmdline) { > + /* Ensure a whitespace between the 2 cmdlines */ > + strlcat(boot_command_line, " ", COMMAND_LINE_SIZE); > + strlcat(boot_command_line, (char *)uboot_arg, COMMAND_LINE_SIZE); > } > +} > + > +void __init setup_arch(char **cmdline_p) > +{ > + handle_uboot_args(); > > /* Save unparsed command line copy for /proc/cmdline */ > *cmdline_p = boot_command_line; ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] ARC: U-boot: check arguments paranoidly 2019-02-12 16:45 ` Vineet Gupta @ 2019-02-12 17:25 ` Eugeniy Paltsev 2019-02-12 17:34 ` Vineet Gupta 0 siblings, 1 reply; 8+ messages in thread From: Eugeniy Paltsev @ 2019-02-12 17:25 UTC (permalink / raw) To: Eugeniy.Paltsev@synopsys.com, Vineet Gupta, linux-snps-arc Cc: linux-kernel, Alexey Brodkin, khilman, clabbe On Tue, 2019-02-12 at 16:45 +0000, Vineet Gupta wrote: > On 2/12/19 7:39 AM, Eugeniy Paltsev wrote: > > Handle U-boot arguments paranoidly: > > * don't allow to pass unknown tag. > > * try to use external device tree blob only if corresponding tag > > (TAG_DTB) is set. > > * check that magic number is correct. > > * don't check uboot_tag if kernel build with no ARC_UBOOT_SUPPORT. > > > > NOTE: > > If U-boot args are invalid we skip them and try to use embedded device > > tree blob. We can't panic on invalid U-boot args as we really pass > > invalid args due to bug in U-boot code. > > This happens if we don't provide external DTB to U-boot and > > don't set 'bootargs' U-boot environment variable (which is default > > case at least for HSDK board) In that case we will pass > > {r0 = 1 (bootargs in r2); r1 = 0; r2 = 0;} to linux which is invalid. > > > > NOTE: > > We can safely check U-boot magic value (0x0) in linux passed via > > r1 register as U-boot pass it from the beginning. > > > > While I'm at it refactor U-boot arguments handling code. > > > > Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> > > --- > > arch/arc/kernel/head.S | 5 +-- > > arch/arc/kernel/setup.c | 92 +++++++++++++++++++++++++++++++++++-------------- > > 2 files changed, 69 insertions(+), 28 deletions(-) > > > > diff --git a/arch/arc/kernel/head.S b/arch/arc/kernel/head.S > > index 8b90d25a15cc..fccea361e896 100644 > > --- a/arch/arc/kernel/head.S > > +++ b/arch/arc/kernel/head.S > > @@ -93,10 +93,11 @@ ENTRY(stext) > > #ifdef CONFIG_ARC_UBOOT_SUPPORT > > ; Uboot - kernel ABI > > ; r0 = [0] No uboot interaction, [1] cmdline in r2, [2] DTB in r2 > > - ; r1 = magic number (board identity, unused as of now > > + ; r1 = magic number (always zero as of now) > > This is technically changing the ABI - I think we don't need to enforce this - > keep ignoring this I think it's better to add this check: * This check doesn't break backward compatibility. ARC U-boot pass zero to r1 from the beginnings, I specially checked this. So we doesn't change ABI, we only document it ;) * By adding this check we can cheap and easily minimize problems in JTAG case. > > + > > + if (use_embedded_dtb) { > > machine_desc = setup_machine_fdt(__dtb_start); > > if (!machine_desc) > > panic("Embedded DT invalid\n"); > > + } > > > > - /* > > - * If we are here, it is established that @uboot_arg didn't > > - * point to DT blob. Instead if u-boot says it is cmdline, > > - * append to embedded DT cmdline. > > - * setup_machine_fdt() would have populated @boot_command_line > > - */ > > Don't drop this comment, specially the last line. If was tempted to move the cmd > line processing before but this saved me since we rely on setup_machine_fdt() > being called aprioiri. Ok, will fix in v2 > > - if (uboot_tag == 1) { > > - /* Ensure a whitespace between the 2 cmdlines */ > > - strlcat(boot_command_line, " ", COMMAND_LINE_SIZE); > > - strlcat(boot_command_line, uboot_arg, > > - COMMAND_LINE_SIZE); > > - } > > + if (append_cmdline) { > > + /* Ensure a whitespace between the 2 cmdlines */ > > + strlcat(boot_command_line, " ", COMMAND_LINE_SIZE); > > + strlcat(boot_command_line, (char *)uboot_arg, COMMAND_LINE_SIZE); > > } > > +} > > + > > +void __init setup_arch(char **cmdline_p) > > +{ > > + handle_uboot_args(); > > > > /* Save unparsed command line copy for /proc/cmdline */ > > *cmdline_p = boot_command_line; > > -- Eugeniy Paltsev ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] ARC: U-boot: check arguments paranoidly 2019-02-12 17:25 ` Eugeniy Paltsev @ 2019-02-12 17:34 ` Vineet Gupta 0 siblings, 0 replies; 8+ messages in thread From: Vineet Gupta @ 2019-02-12 17:34 UTC (permalink / raw) To: Eugeniy Paltsev, Eugeniy.Paltsev@synopsys.com, linux-snps-arc Cc: linux-kernel, Alexey Brodkin, khilman, clabbe On 2/12/19 9:25 AM, Eugeniy Paltsev wrote: >> This is technically changing the ABI - I think we don't need to enforce this - >> keep ignoring this > I think it's better to add this check: > * This check doesn't break backward compatibility. ARC U-boot pass zero to r1 > from the beginnings, I specially checked this. So we doesn't change ABI, > we only document it ;) OK good. > * By adding this check we can cheap and easily minimize problems in JTAG case. Prior to your patch this register was irrelevant - it didn't matter for jtag or uboot cause what its value was since it was not being checked at all. Now you enforce this be 0. Simple reasoning tells me it will likely cause problems, if any, but won't reduce them at all. So I'd insist we keep ignoring it even if uboot was zeroing it out. Atleast this leaves the door open any future changes. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] ARC: enable uboot support unconditionally 2019-02-12 15:39 [PATCH 0/2] RC: rework U-boot arguments handling Eugeniy Paltsev 2019-02-12 15:39 ` [PATCH 1/2] ARC: U-boot: check arguments paranoidly Eugeniy Paltsev @ 2019-02-12 15:39 ` Eugeniy Paltsev 1 sibling, 0 replies; 8+ messages in thread From: Eugeniy Paltsev @ 2019-02-12 15:39 UTC (permalink / raw) To: linux-snps-arc, Vineet Gupta Cc: linux-kernel, Alexey Brodkin, Corentin Labbe, khilman, Eugeniy Paltsev After reworking U-boot args handling code and adding paranoid arguments check we can eliminate CONFIG_ARC_UBOOT_SUPPORT and enable uboot support unconditionally. For JTAG case we can assume that core registers will come up reset value of 0 or in worst case we rely on user passing '-on=clear_regs' to Metaware debugger. Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> --- arch/arc/Kconfig | 12 ------------ arch/arc/configs/nps_defconfig | 1 - arch/arc/configs/vdk_hs38_defconfig | 1 - arch/arc/configs/vdk_hs38_smp_defconfig | 2 -- arch/arc/kernel/head.S | 2 -- arch/arc/kernel/setup.c | 2 -- 6 files changed, 20 deletions(-) diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig index 376366a7db81..f9534417b201 100644 --- a/arch/arc/Kconfig +++ b/arch/arc/Kconfig @@ -191,7 +191,6 @@ config NR_CPUS config ARC_SMP_HALT_ON_RESET bool "Enable Halt-on-reset boot mode" - default y if ARC_UBOOT_SUPPORT help In SMP configuration cores can be configured as Halt-on-reset or they could all start at same time. For Halt-on-reset, non @@ -515,17 +514,6 @@ config ARC_DBG_TLB_PARANOIA endif -config ARC_UBOOT_SUPPORT - bool "Support uboot arg Handling" - help - ARC Linux by default checks for uboot provided args as pointers to - external cmdline or DTB. This however breaks in absence of uboot, - when booting from Metaware debugger directly, as the registers are - not zeroed out on reset by mdb and/or ARCv2 based cores. The bogus - registers look like uboot args to kernel which then chokes. - So only enable the uboot arg checking/processing if users are sure - of uboot being in play. - config ARC_BUILTIN_DTB_NAME string "Built in DTB" help diff --git a/arch/arc/configs/nps_defconfig b/arch/arc/configs/nps_defconfig index 6e84060e7c90..621f59407d76 100644 --- a/arch/arc/configs/nps_defconfig +++ b/arch/arc/configs/nps_defconfig @@ -31,7 +31,6 @@ CONFIG_ARC_CACHE_LINE_SHIFT=5 # CONFIG_ARC_HAS_LLSC is not set CONFIG_ARC_KVADDR_SIZE=402 CONFIG_ARC_EMUL_UNALIGNED=y -CONFIG_ARC_UBOOT_SUPPORT=y CONFIG_PREEMPT=y CONFIG_NET=y CONFIG_UNIX=y diff --git a/arch/arc/configs/vdk_hs38_defconfig b/arch/arc/configs/vdk_hs38_defconfig index 1e59a2e9c602..e447ace6fa1c 100644 --- a/arch/arc/configs/vdk_hs38_defconfig +++ b/arch/arc/configs/vdk_hs38_defconfig @@ -13,7 +13,6 @@ CONFIG_PARTITION_ADVANCED=y CONFIG_ARC_PLAT_AXS10X=y CONFIG_AXS103=y CONFIG_ISA_ARCV2=y -CONFIG_ARC_UBOOT_SUPPORT=y CONFIG_ARC_BUILTIN_DTB_NAME="vdk_hs38" CONFIG_PREEMPT=y CONFIG_NET=y diff --git a/arch/arc/configs/vdk_hs38_smp_defconfig b/arch/arc/configs/vdk_hs38_smp_defconfig index b5c3f6c54b03..c82cdb10aaf4 100644 --- a/arch/arc/configs/vdk_hs38_smp_defconfig +++ b/arch/arc/configs/vdk_hs38_smp_defconfig @@ -15,8 +15,6 @@ CONFIG_AXS103=y CONFIG_ISA_ARCV2=y CONFIG_SMP=y # CONFIG_ARC_TIMERS_64BIT is not set -# CONFIG_ARC_SMP_HALT_ON_RESET is not set -CONFIG_ARC_UBOOT_SUPPORT=y CONFIG_ARC_BUILTIN_DTB_NAME="vdk_hs38_smp" CONFIG_PREEMPT=y CONFIG_NET=y diff --git a/arch/arc/kernel/head.S b/arch/arc/kernel/head.S index fccea361e896..4b0deaff001c 100644 --- a/arch/arc/kernel/head.S +++ b/arch/arc/kernel/head.S @@ -90,7 +90,6 @@ ENTRY(stext) st.ab 0, [r5, 4] 1: -#ifdef CONFIG_ARC_UBOOT_SUPPORT ; Uboot - kernel ABI ; r0 = [0] No uboot interaction, [1] cmdline in r2, [2] DTB in r2 ; r1 = magic number (always zero as of now) @@ -99,7 +98,6 @@ ENTRY(stext) st r0, [@uboot_tag] st r1, [@uboot_magic] st r2, [@uboot_arg] -#endif ; setup "current" tsk and optionally cache it in dedicated r25 mov r9, @init_task diff --git a/arch/arc/kernel/setup.c b/arch/arc/kernel/setup.c index 84d394a37e79..fff946b0ab4f 100644 --- a/arch/arc/kernel/setup.c +++ b/arch/arc/kernel/setup.c @@ -490,7 +490,6 @@ void __init handle_uboot_args(void) bool use_embedded_dtb = true; bool append_cmdline = false; -#ifdef CONFIG_ARC_UBOOT_SUPPORT /* check that we know this tag */ if (uboot_tag != UBOOT_TAG_NONE && uboot_tag != UBOOT_TAG_CMDLINE && @@ -521,7 +520,6 @@ void __init handle_uboot_args(void) append_cmdline = true; ignore_uboot_args: -#endif if (use_embedded_dtb) { machine_desc = setup_machine_fdt(__dtb_start); -- 2.14.5 ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-02-12 17:34 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-02-12 15:39 [PATCH 0/2] RC: rework U-boot arguments handling Eugeniy Paltsev 2019-02-12 15:39 ` [PATCH 1/2] ARC: U-boot: check arguments paranoidly Eugeniy Paltsev 2019-02-12 16:39 ` LABBE Corentin 2019-02-12 16:41 ` Vineet Gupta 2019-02-12 16:45 ` Vineet Gupta 2019-02-12 17:25 ` Eugeniy Paltsev 2019-02-12 17:34 ` Vineet Gupta 2019-02-12 15:39 ` [PATCH 2/2] ARC: enable uboot support unconditionally Eugeniy Paltsev
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).