* [PATCH v2 1/2] ARC: U-boot: check arguments paranoidly
2019-02-14 15:07 [PATCH v2 0/2] ARC: rework U-boot arguments handling Eugeniy Paltsev
@ 2019-02-14 15:07 ` Eugeniy Paltsev
2019-02-15 23:55 ` Vineet Gupta
2019-02-14 15:07 ` [PATCH v2 2/2] ARC: enable uboot support unconditionally Eugeniy Paltsev
2019-02-15 11:12 ` [PATCH v2 0/2] ARC: rework U-boot arguments handling LABBE Corentin
2 siblings, 1 reply; 9+ messages in thread
From: Eugeniy Paltsev @ 2019-02-14 15:07 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.
* 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.
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 | 4 +--
arch/arc/kernel/setup.c | 90 +++++++++++++++++++++++++++++++++++--------------
2 files changed, 66 insertions(+), 28 deletions(-)
diff --git a/arch/arc/kernel/head.S b/arch/arc/kernel/head.S
index 8b90d25a15cc..d45b862be05c 100644
--- a/arch/arc/kernel/head.S
+++ b/arch/arc/kernel/head.S
@@ -93,9 +93,9 @@ 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 r2, [@uboot_arg]
#endif
diff --git a/arch/arc/kernel/setup.c b/arch/arc/kernel/setup.c
index feb90093e6b1..88a885db9bb8 100644
--- a/arch/arc/kernel/setup.c
+++ b/arch/arc/kernel/setup.c
@@ -36,7 +36,7 @@ unsigned int intr_to_DE_cnt;
/* Part of U-boot ABI: see head.S */
int __initdata uboot_tag;
-char __initdata *uboot_arg;
+unsigned int __initdata uboot_arg;
const struct machine_desc *machine_desc;
@@ -462,43 +462,81 @@ 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 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
+
+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_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);
- /* See if u-boot passed an external Device Tree blob */
- machine_desc = setup_machine_fdt(uboot_arg); /* uboot_tag == 2 */
- if (!machine_desc)
+ /* external Device Tree blob is invalid - use embedded one */
+ use_embedded_dtb = !machine_desc;
+ }
+
+ if (uboot_tag == UBOOT_TAG_CMDLINE)
+ append_cmdline = true;
+
+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 we are here, it is established that @uboot_arg points to cmdline,
+ * which should be appended to embedded DT cmdline.
+ * NOTE: Don't move the cmdline processing before embedded DT processing
+ * since we rely on @boot_command_line populating by setup_machine_fdt()
+ */
+ 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] 9+ messages in thread* Re: [PATCH v2 1/2] ARC: U-boot: check arguments paranoidly
2019-02-14 15:07 ` [PATCH v2 1/2] ARC: U-boot: check arguments paranoidly Eugeniy Paltsev
@ 2019-02-15 23:55 ` Vineet Gupta
0 siblings, 0 replies; 9+ messages in thread
From: Vineet Gupta @ 2019-02-15 23:55 UTC (permalink / raw)
To: Eugeniy Paltsev, linux-snps-arc
Cc: khilman, Alexey Brodkin, linux-kernel, Corentin Labbe
[...]
> -char __initdata *uboot_arg;
> +unsigned int __initdata uboot_arg;
Why ?
In both places it is actually used, it is intended as a pointer. The cast for
range check is needed but lets cast there. See below for real reason.
> -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;
> +
> + /* Check that address doesn't clobber resident kernel image */
> + return addr >= (unsigned int)_stext && addr <= (unsigned int)_end;
...
> +
> + /* see if U-boot passed an external Device Tree blob */
> + if (uboot_tag == UBOOT_TAG_DTB) {
> + machine_desc = setup_machine_fdt((void *)uboot_arg);
On a 64-bit paradigm, with LP64 ABI, this will break since int will be 4b, while
pointer 8b.
I'll fix it up locally and push !
-Vineet
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 2/2] ARC: enable uboot support unconditionally
2019-02-14 15:07 [PATCH v2 0/2] ARC: rework U-boot arguments handling Eugeniy Paltsev
2019-02-14 15:07 ` [PATCH v2 1/2] ARC: U-boot: check arguments paranoidly Eugeniy Paltsev
@ 2019-02-14 15:07 ` Eugeniy Paltsev
2019-07-18 20:51 ` Alexey Brodkin
2019-02-15 11:12 ` [PATCH v2 0/2] ARC: rework U-boot arguments handling LABBE Corentin
2 siblings, 1 reply; 9+ messages in thread
From: Eugeniy Paltsev @ 2019-02-14 15:07 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 d45b862be05c..6fb1a35f9a29 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)
@@ -98,7 +97,6 @@ ENTRY(stext)
; These are handled later in handle_uboot_args()
st r0, [@uboot_tag]
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 88a885db9bb8..dbab88c8bbb3 100644
--- a/arch/arc/kernel/setup.c
+++ b/arch/arc/kernel/setup.c
@@ -487,7 +487,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 &&
@@ -513,7 +512,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] 9+ messages in thread* RE: [PATCH v2 2/2] ARC: enable uboot support unconditionally
2019-02-14 15:07 ` [PATCH v2 2/2] ARC: enable uboot support unconditionally Eugeniy Paltsev
@ 2019-07-18 20:51 ` Alexey Brodkin
2019-08-02 7:40 ` Greg Kroah-Hartman
0 siblings, 1 reply; 9+ messages in thread
From: Alexey Brodkin @ 2019-07-18 20:51 UTC (permalink / raw)
To: linux-stable
Cc: linux-kernel, Corentin Labbe, khilman, linux-snps-arc,
Eugeniy Paltsev, Greg Kroah-Hartman, Vineet Gupta
Hi Greg,
> -----Original Message-----
> From: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> Sent: Thursday, February 14, 2019 6:08 PM
> To: linux-snps-arc@lists.infradead.org; Vineet Gupta <vgupta@synopsys.com>
> Cc: linux-kernel@vger.kernel.org; Alexey Brodkin <abrodkin@synopsys.com>; Corentin Labbe
> <clabbe@baylibre.com>; khilman@baylibre.com; Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> Subject: [PATCH v2 2/2] ARC: enable uboot support unconditionally
>
> 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>
May we have this one back-ported to linux-4.19.y?
It was initially applied to Linus' tree during 5.0 development
cycle [1] but was never back-ported.
Now w/o that patch in KernelCI we see boot failure on ARC HSDK
board [2] as opposed to normally working later kernel versions.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=493a2f812446e92bcb1e69a77381b4d39808d730
[2] https://storage.kernelci.org/stable/linux-4.19.y/v4.19.59/arc/hsdk_defconfig/gcc-8/lab-baylibre/boot-hsdk.txt
Below is that same patch but rebased on linux-4.19 as in its pristine
form it won't apply due to offset of one of hunks.
-Alexey
------------------------------------>8--------------------------------
From 3e565355f6a2d1a82bc9ecd47a46d1fa3c0cd2c1 Mon Sep 17 00:00:00 2001
From: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
Date: Thu, 14 Feb 2019 18:07:45 +0300
Subject: [PATCH] ARC: enable uboot support unconditionally
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.
Cc: stable@vger.kernel.org
Tested-by: Corentin LABBE <clabbe@baylibre.com>
Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
---
arch/arc/Kconfig | 13 -------------
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, 21 deletions(-)
diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig
index 85eb7fc2e241..97b45fe8f0c2 100644
--- a/arch/arc/Kconfig
+++ b/arch/arc/Kconfig
@@ -199,7 +199,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
@@ -538,18 +537,6 @@ config ARC_DBG_TLB_PARANOIA
endif
-config ARC_UBOOT_SUPPORT
- bool "Support uboot arg Handling"
- default n
- 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 208bf2c9e7b0..a72bbda2f7aa 100644
--- a/arch/arc/kernel/head.S
+++ b/arch/arc/kernel/head.S
@@ -100,7 +100,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)
@@ -109,7 +108,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 a1218937abd6..89c97dcfa360 100644
--- a/arch/arc/kernel/setup.c
+++ b/arch/arc/kernel/setup.c
@@ -493,7 +493,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 &&
@@ -525,7 +524,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.16.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v2 2/2] ARC: enable uboot support unconditionally
2019-07-18 20:51 ` Alexey Brodkin
@ 2019-08-02 7:40 ` Greg Kroah-Hartman
2019-08-02 16:25 ` Alexey Brodkin
0 siblings, 1 reply; 9+ messages in thread
From: Greg Kroah-Hartman @ 2019-08-02 7:40 UTC (permalink / raw)
To: Alexey Brodkin
Cc: linux-stable, linux-kernel, Corentin Labbe, khilman,
linux-snps-arc, Eugeniy Paltsev, Vineet Gupta
On Thu, Jul 18, 2019 at 08:51:23PM +0000, Alexey Brodkin wrote:
> Hi Greg,
>
> > -----Original Message-----
> > From: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> > Sent: Thursday, February 14, 2019 6:08 PM
> > To: linux-snps-arc@lists.infradead.org; Vineet Gupta <vgupta@synopsys.com>
> > Cc: linux-kernel@vger.kernel.org; Alexey Brodkin <abrodkin@synopsys.com>; Corentin Labbe
> > <clabbe@baylibre.com>; khilman@baylibre.com; Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> > Subject: [PATCH v2 2/2] ARC: enable uboot support unconditionally
> >
> > 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>
>
> May we have this one back-ported to linux-4.19.y?
>
> It was initially applied to Linus' tree during 5.0 development
> cycle [1] but was never back-ported.
>
> Now w/o that patch in KernelCI we see boot failure on ARC HSDK
> board [2] as opposed to normally working later kernel versions.
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=493a2f812446e92bcb1e69a77381b4d39808d730
> [2] https://storage.kernelci.org/stable/linux-4.19.y/v4.19.59/arc/hsdk_defconfig/gcc-8/lab-baylibre/boot-hsdk.txt
>
> Below is that same patch but rebased on linux-4.19 as in its pristine
> form it won't apply due to offset of one of hunks.
Why is this patch ok for stable kernel trees? Are you not removing
existing support in 4.19 for a feature that people might be using there?
What bug is this fixing that requires this removal?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH v2 2/2] ARC: enable uboot support unconditionally
2019-08-02 7:40 ` Greg Kroah-Hartman
@ 2019-08-02 16:25 ` Alexey Brodkin
2019-08-05 11:17 ` Greg Kroah-Hartman
0 siblings, 1 reply; 9+ messages in thread
From: Alexey Brodkin @ 2019-08-02 16:25 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-stable, linux-kernel, Corentin Labbe, khilman,
linux-snps-arc, Eugeniy Paltsev, Vineet Gupta
Hi Greg,
> > May we have this one back-ported to linux-4.19.y?
> >
> > It was initially applied to Linus' tree during 5.0 development
> > cycle [1] but was never back-ported.
> >
> > Now w/o that patch in KernelCI we see boot failure on ARC HSDK
> > board [2] as opposed to normally working later kernel versions.
> >
> > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=493a2f812446e92bcb1e69a77381b4d39808d730
> > [2] https://storage.kernelci.org/stable/linux-4.19.y/v4.19.59/arc/hsdk_defconfig/gcc-8/lab-baylibre/boot-hsdk.txt
> >
> > Below is that same patch but rebased on linux-4.19 as in its pristine
> > form it won't apply due to offset of one of hunks.
>
> Why is this patch ok for stable kernel trees? Are you not removing
> existing support in 4.19 for a feature that people might be using there?
> What bug is this fixing that requires this removal?
This patch removes a Kconfig option in a trade for properly working
detection of arguments passed from U-Boot.
Back in the day [3] we had to add that option to get kernel reliably working
in use-cases w/o U-Boot (those were typically loading kernel image via JTAG).
But with a couple of fixes applied to linux-4.19.y already we no longer need
that explicit toggle as we may rely on data passed via dedicated registers
and thus automatically know if there was U-Boot which passed some info to
the kernel or there was no U-Boot and we don't need to mess with garbage in
those registers.
Main reason is to make vanilla 4.19.y kernels usable on HSDK board in KernelCI
environment. Now they don't boot, see [2] as in HSDK's defconfig ARC_UBOOT_SUPPORT
is not set. So we have 2 solutions:
1. Add ARC_UBOOT_SUPPORT to arch/arc/configs/hsdk_defconfig
But we cannot do it for vanilla kernel because we simply cannot even submit that
change to the Linus' tree as that Kconfig option was removed.
Which means we cannot back-port it, right :)
2. Back-port proposed patch which already exists in the Linus'tree and thus is
perfectly back-portable.
Makes sense?
-Alexey
[2] https://storage.kernelci.org/stable/linux-4.19.y/v4.19.59/arc/hsdk_defconfig/gcc-8/lab-baylibre/boot-hsdk.txt
[3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=036b2c5664281e7da5a89c9f742491f30966485f
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] ARC: enable uboot support unconditionally
2019-08-02 16:25 ` Alexey Brodkin
@ 2019-08-05 11:17 ` Greg Kroah-Hartman
0 siblings, 0 replies; 9+ messages in thread
From: Greg Kroah-Hartman @ 2019-08-05 11:17 UTC (permalink / raw)
To: Alexey Brodkin
Cc: linux-stable, linux-kernel, Corentin Labbe, khilman,
linux-snps-arc, Eugeniy Paltsev, Vineet Gupta
On Fri, Aug 02, 2019 at 04:25:39PM +0000, Alexey Brodkin wrote:
> Hi Greg,
>
> > > May we have this one back-ported to linux-4.19.y?
> > >
> > > It was initially applied to Linus' tree during 5.0 development
> > > cycle [1] but was never back-ported.
> > >
> > > Now w/o that patch in KernelCI we see boot failure on ARC HSDK
> > > board [2] as opposed to normally working later kernel versions.
> > >
> > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=493a2f812446e92bcb1e69a77381b4d39808d730
> > > [2] https://storage.kernelci.org/stable/linux-4.19.y/v4.19.59/arc/hsdk_defconfig/gcc-8/lab-baylibre/boot-hsdk.txt
> > >
> > > Below is that same patch but rebased on linux-4.19 as in its pristine
> > > form it won't apply due to offset of one of hunks.
> >
> > Why is this patch ok for stable kernel trees? Are you not removing
> > existing support in 4.19 for a feature that people might be using there?
> > What bug is this fixing that requires this removal?
>
> This patch removes a Kconfig option in a trade for properly working
> detection of arguments passed from U-Boot.
>
> Back in the day [3] we had to add that option to get kernel reliably working
> in use-cases w/o U-Boot (those were typically loading kernel image via JTAG).
> But with a couple of fixes applied to linux-4.19.y already we no longer need
> that explicit toggle as we may rely on data passed via dedicated registers
> and thus automatically know if there was U-Boot which passed some info to
> the kernel or there was no U-Boot and we don't need to mess with garbage in
> those registers.
>
> Main reason is to make vanilla 4.19.y kernels usable on HSDK board in KernelCI
> environment. Now they don't boot, see [2] as in HSDK's defconfig ARC_UBOOT_SUPPORT
> is not set. So we have 2 solutions:
>
> 1. Add ARC_UBOOT_SUPPORT to arch/arc/configs/hsdk_defconfig
> But we cannot do it for vanilla kernel because we simply cannot even submit that
> change to the Linus' tree as that Kconfig option was removed.
> Which means we cannot back-port it, right :)
>
> 2. Back-port proposed patch which already exists in the Linus'tree and thus is
> perfectly back-portable.
>
> Makes sense?
Ok, it's your arch, you get to deal with the angry users if you have any
:)
now queued up.
greg k-h
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 0/2] ARC: rework U-boot arguments handling
2019-02-14 15:07 [PATCH v2 0/2] ARC: rework U-boot arguments handling Eugeniy Paltsev
2019-02-14 15:07 ` [PATCH v2 1/2] ARC: U-boot: check arguments paranoidly Eugeniy Paltsev
2019-02-14 15:07 ` [PATCH v2 2/2] ARC: enable uboot support unconditionally Eugeniy Paltsev
@ 2019-02-15 11:12 ` LABBE Corentin
2 siblings, 0 replies; 9+ messages in thread
From: LABBE Corentin @ 2019-02-15 11:12 UTC (permalink / raw)
To: Eugeniy Paltsev
Cc: linux-snps-arc, Vineet Gupta, linux-kernel, Alexey Brodkin, khilman
On Thu, Feb 14, 2019 at 06:07:43PM +0300, Eugeniy Paltsev wrote:
> Reworking U-boot args handling and enable uboot support
> unconditionally.
>
> Changes v1->v2:
> * Drop magic number check [in this patch series]
> * Keep comment about cndline appending
>
> 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
>
Hello
Tested-by: Corentin LABBE <clabbe@baylibre.com>
It worked on our LAVA lab along with my "arc: hsdk_defconfig: Enable CONFIG_BLK_DEV_RAM" patch.
Regards
^ permalink raw reply [flat|nested] 9+ messages in thread