Re: [U-Boot] [PATCH] watchdog: Move watchdog_dev to data section (BSS may not be cleared)

2019-04-02 Thread Stefan Roese

On 02.04.19 12:30, Michal Simek wrote:

On 02. 04. 19 11:57, Stefan Roese wrote:

This patch moves all instances of static "watchdog_dev" declarations to
the "data" section. This may be needed, as the BSS may not be cleared
in the early U-Boot phase, where watchdog_reset() is already beeing
called. This may result in incorrect pointer access, as the check to
"!watchdog_dev" in watchdog_reset() may not be true and the function
may continue to run.

Signed-off-by: Stefan Roese 
Cc: Heiko Schocher 
Cc: Tom Rini 
Cc: Michal Simek 
Cc: "Marek Behún" 
Cc: Daniel Schwierzeck 
---
  arch/mips/mach-mt7620/cpu.c  | 2 +-
  board/CZ.NIC/turris_mox/turris_mox.c | 2 +-
  board/CZ.NIC/turris_omnia/turris_omnia.c | 2 +-
  board/xilinx/microblaze-generic/microblaze-generic.c | 2 +-
  board/xilinx/zynq/board.c| 2 +-
  board/xilinx/zynqmp/zynqmp.c | 2 +-
  6 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/mips/mach-mt7620/cpu.c b/arch/mips/mach-mt7620/cpu.c
index 9e0ca716f7..fe74f26a54 100644
--- a/arch/mips/mach-mt7620/cpu.c
+++ b/arch/mips/mach-mt7620/cpu.c
@@ -70,7 +70,7 @@ int print_cpuinfo(void)
  }
  
  #ifdef CONFIG_WATCHDOG

-static struct udevice *watchdog_dev;
+static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL;
  
  /* Called by macro WATCHDOG_RESET */

  void watchdog_reset(void)
diff --git a/board/CZ.NIC/turris_mox/turris_mox.c 
b/board/CZ.NIC/turris_mox/turris_mox.c
index 65d50a92dd..96cb9c7e5c 100644
--- a/board/CZ.NIC/turris_mox/turris_mox.c
+++ b/board/CZ.NIC/turris_mox/turris_mox.c
@@ -120,7 +120,7 @@ int board_fix_fdt(void *blob)
  #endif
  
  #ifdef CONFIG_WDT_ARMADA_37XX

-static struct udevice *watchdog_dev;
+static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL;
  
  void watchdog_reset(void)

  {
diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c 
b/board/CZ.NIC/turris_omnia/turris_omnia.c
index 1f7650cb36..c7f6479a0c 100644
--- a/board/CZ.NIC/turris_omnia/turris_omnia.c
+++ b/board/CZ.NIC/turris_omnia/turris_omnia.c
@@ -365,7 +365,7 @@ static bool disable_mcu_watchdog(void)
  #endif
  
  #if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_WDT_ORION)

-static struct udevice *watchdog_dev = NULL;
+static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL;
  #endif
  
  int board_init(void)

diff --git a/board/xilinx/microblaze-generic/microblaze-generic.c 
b/board/xilinx/microblaze-generic/microblaze-generic.c
index c946ec3208..28c9efa3a2 100644
--- a/board/xilinx/microblaze-generic/microblaze-generic.c
+++ b/board/xilinx/microblaze-generic/microblaze-generic.c
@@ -25,7 +25,7 @@
  DECLARE_GLOBAL_DATA_PTR;
  
  #if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_WDT)

-static struct udevice *watchdog_dev;
+static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL;
  #endif /* !CONFIG_SPL_BUILD && CONFIG_WDT */
  
  ulong ram_base;

diff --git a/board/xilinx/zynq/board.c b/board/xilinx/zynq/board.c
index b4b84df576..7c8c32a1c7 100644
--- a/board/xilinx/zynq/board.c
+++ b/board/xilinx/zynq/board.c
@@ -19,7 +19,7 @@
  DECLARE_GLOBAL_DATA_PTR;
  
  #if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_WDT)

-static struct udevice *watchdog_dev;
+static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL;
  #endif
  
  #if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_BOARD_EARLY_INIT_F)

diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
index 5e1d2116bc..1c2aa99b97 100644
--- a/board/xilinx/zynqmp/zynqmp.c
+++ b/board/xilinx/zynqmp/zynqmp.c
@@ -25,7 +25,7 @@
  DECLARE_GLOBAL_DATA_PTR;
  
  #if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_WDT)

-static struct udevice *watchdog_dev;
+static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL;
  #endif
  
  #if defined(CONFIG_FPGA) && defined(CONFIG_FPGA_ZYNQMPPL) && \




If you want to do it for all then please remove the same from this file

http://git.denx.de/?p=u-boot.git;a=commitdiff;h=4490e013ee4f2a4b9b6ca9224221fed2788b6940

+#if defined(CONFIG_WDT) && !defined(CONFIG_SPL_BUILD)
+   /* bss is not cleared at time when watchdog_reset() is called */
+   watchdog_dev = NULL;
+#endif


And also from zynq file.
board/xilinx/zynq/board.c


Thanks for spotting. I'll remove it in v2.

Thanks,
Stefan
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] watchdog: Move watchdog_dev to data section (BSS may not be cleared)

2019-04-02 Thread Michal Simek
On 02. 04. 19 11:57, Stefan Roese wrote:
> This patch moves all instances of static "watchdog_dev" declarations to
> the "data" section. This may be needed, as the BSS may not be cleared
> in the early U-Boot phase, where watchdog_reset() is already beeing
> called. This may result in incorrect pointer access, as the check to
> "!watchdog_dev" in watchdog_reset() may not be true and the function
> may continue to run.
> 
> Signed-off-by: Stefan Roese 
> Cc: Heiko Schocher 
> Cc: Tom Rini 
> Cc: Michal Simek 
> Cc: "Marek Behún" 
> Cc: Daniel Schwierzeck 
> ---
>  arch/mips/mach-mt7620/cpu.c  | 2 +-
>  board/CZ.NIC/turris_mox/turris_mox.c | 2 +-
>  board/CZ.NIC/turris_omnia/turris_omnia.c | 2 +-
>  board/xilinx/microblaze-generic/microblaze-generic.c | 2 +-
>  board/xilinx/zynq/board.c| 2 +-
>  board/xilinx/zynqmp/zynqmp.c | 2 +-
>  6 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/mips/mach-mt7620/cpu.c b/arch/mips/mach-mt7620/cpu.c
> index 9e0ca716f7..fe74f26a54 100644
> --- a/arch/mips/mach-mt7620/cpu.c
> +++ b/arch/mips/mach-mt7620/cpu.c
> @@ -70,7 +70,7 @@ int print_cpuinfo(void)
>  }
>  
>  #ifdef CONFIG_WATCHDOG
> -static struct udevice *watchdog_dev;
> +static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL;
>  
>  /* Called by macro WATCHDOG_RESET */
>  void watchdog_reset(void)
> diff --git a/board/CZ.NIC/turris_mox/turris_mox.c 
> b/board/CZ.NIC/turris_mox/turris_mox.c
> index 65d50a92dd..96cb9c7e5c 100644
> --- a/board/CZ.NIC/turris_mox/turris_mox.c
> +++ b/board/CZ.NIC/turris_mox/turris_mox.c
> @@ -120,7 +120,7 @@ int board_fix_fdt(void *blob)
>  #endif
>  
>  #ifdef CONFIG_WDT_ARMADA_37XX
> -static struct udevice *watchdog_dev;
> +static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL;
>  
>  void watchdog_reset(void)
>  {
> diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c 
> b/board/CZ.NIC/turris_omnia/turris_omnia.c
> index 1f7650cb36..c7f6479a0c 100644
> --- a/board/CZ.NIC/turris_omnia/turris_omnia.c
> +++ b/board/CZ.NIC/turris_omnia/turris_omnia.c
> @@ -365,7 +365,7 @@ static bool disable_mcu_watchdog(void)
>  #endif
>  
>  #if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_WDT_ORION)
> -static struct udevice *watchdog_dev = NULL;
> +static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL;
>  #endif
>  
>  int board_init(void)
> diff --git a/board/xilinx/microblaze-generic/microblaze-generic.c 
> b/board/xilinx/microblaze-generic/microblaze-generic.c
> index c946ec3208..28c9efa3a2 100644
> --- a/board/xilinx/microblaze-generic/microblaze-generic.c
> +++ b/board/xilinx/microblaze-generic/microblaze-generic.c
> @@ -25,7 +25,7 @@
>  DECLARE_GLOBAL_DATA_PTR;
>  
>  #if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_WDT)
> -static struct udevice *watchdog_dev;
> +static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL;
>  #endif /* !CONFIG_SPL_BUILD && CONFIG_WDT */
>  
>  ulong ram_base;
> diff --git a/board/xilinx/zynq/board.c b/board/xilinx/zynq/board.c
> index b4b84df576..7c8c32a1c7 100644
> --- a/board/xilinx/zynq/board.c
> +++ b/board/xilinx/zynq/board.c
> @@ -19,7 +19,7 @@
>  DECLARE_GLOBAL_DATA_PTR;
>  
>  #if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_WDT)
> -static struct udevice *watchdog_dev;
> +static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL;
>  #endif
>  
>  #if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_BOARD_EARLY_INIT_F)
> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
> index 5e1d2116bc..1c2aa99b97 100644
> --- a/board/xilinx/zynqmp/zynqmp.c
> +++ b/board/xilinx/zynqmp/zynqmp.c
> @@ -25,7 +25,7 @@
>  DECLARE_GLOBAL_DATA_PTR;
>  
>  #if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_WDT)
> -static struct udevice *watchdog_dev;
> +static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL;
>  #endif
>  
>  #if defined(CONFIG_FPGA) && defined(CONFIG_FPGA_ZYNQMPPL) && \
> 

If you want to do it for all then please remove the same from this file

http://git.denx.de/?p=u-boot.git;a=commitdiff;h=4490e013ee4f2a4b9b6ca9224221fed2788b6940

+#if defined(CONFIG_WDT) && !defined(CONFIG_SPL_BUILD)
+   /* bss is not cleared at time when watchdog_reset() is called */
+   watchdog_dev = NULL;
+#endif


And also from zynq file.
board/xilinx/zynq/board.c

Thanks,
Michal
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


[U-Boot] [PATCH] watchdog: Move watchdog_dev to data section (BSS may not be cleared)

2019-04-02 Thread Stefan Roese
This patch moves all instances of static "watchdog_dev" declarations to
the "data" section. This may be needed, as the BSS may not be cleared
in the early U-Boot phase, where watchdog_reset() is already beeing
called. This may result in incorrect pointer access, as the check to
"!watchdog_dev" in watchdog_reset() may not be true and the function
may continue to run.

Signed-off-by: Stefan Roese 
Cc: Heiko Schocher 
Cc: Tom Rini 
Cc: Michal Simek 
Cc: "Marek Behún" 
Cc: Daniel Schwierzeck 
---
 arch/mips/mach-mt7620/cpu.c  | 2 +-
 board/CZ.NIC/turris_mox/turris_mox.c | 2 +-
 board/CZ.NIC/turris_omnia/turris_omnia.c | 2 +-
 board/xilinx/microblaze-generic/microblaze-generic.c | 2 +-
 board/xilinx/zynq/board.c| 2 +-
 board/xilinx/zynqmp/zynqmp.c | 2 +-
 6 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/mips/mach-mt7620/cpu.c b/arch/mips/mach-mt7620/cpu.c
index 9e0ca716f7..fe74f26a54 100644
--- a/arch/mips/mach-mt7620/cpu.c
+++ b/arch/mips/mach-mt7620/cpu.c
@@ -70,7 +70,7 @@ int print_cpuinfo(void)
 }
 
 #ifdef CONFIG_WATCHDOG
-static struct udevice *watchdog_dev;
+static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL;
 
 /* Called by macro WATCHDOG_RESET */
 void watchdog_reset(void)
diff --git a/board/CZ.NIC/turris_mox/turris_mox.c 
b/board/CZ.NIC/turris_mox/turris_mox.c
index 65d50a92dd..96cb9c7e5c 100644
--- a/board/CZ.NIC/turris_mox/turris_mox.c
+++ b/board/CZ.NIC/turris_mox/turris_mox.c
@@ -120,7 +120,7 @@ int board_fix_fdt(void *blob)
 #endif
 
 #ifdef CONFIG_WDT_ARMADA_37XX
-static struct udevice *watchdog_dev;
+static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL;
 
 void watchdog_reset(void)
 {
diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c 
b/board/CZ.NIC/turris_omnia/turris_omnia.c
index 1f7650cb36..c7f6479a0c 100644
--- a/board/CZ.NIC/turris_omnia/turris_omnia.c
+++ b/board/CZ.NIC/turris_omnia/turris_omnia.c
@@ -365,7 +365,7 @@ static bool disable_mcu_watchdog(void)
 #endif
 
 #if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_WDT_ORION)
-static struct udevice *watchdog_dev = NULL;
+static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL;
 #endif
 
 int board_init(void)
diff --git a/board/xilinx/microblaze-generic/microblaze-generic.c 
b/board/xilinx/microblaze-generic/microblaze-generic.c
index c946ec3208..28c9efa3a2 100644
--- a/board/xilinx/microblaze-generic/microblaze-generic.c
+++ b/board/xilinx/microblaze-generic/microblaze-generic.c
@@ -25,7 +25,7 @@
 DECLARE_GLOBAL_DATA_PTR;
 
 #if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_WDT)
-static struct udevice *watchdog_dev;
+static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL;
 #endif /* !CONFIG_SPL_BUILD && CONFIG_WDT */
 
 ulong ram_base;
diff --git a/board/xilinx/zynq/board.c b/board/xilinx/zynq/board.c
index b4b84df576..7c8c32a1c7 100644
--- a/board/xilinx/zynq/board.c
+++ b/board/xilinx/zynq/board.c
@@ -19,7 +19,7 @@
 DECLARE_GLOBAL_DATA_PTR;
 
 #if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_WDT)
-static struct udevice *watchdog_dev;
+static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL;
 #endif
 
 #if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_BOARD_EARLY_INIT_F)
diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
index 5e1d2116bc..1c2aa99b97 100644
--- a/board/xilinx/zynqmp/zynqmp.c
+++ b/board/xilinx/zynqmp/zynqmp.c
@@ -25,7 +25,7 @@
 DECLARE_GLOBAL_DATA_PTR;
 
 #if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_WDT)
-static struct udevice *watchdog_dev;
+static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL;
 #endif
 
 #if defined(CONFIG_FPGA) && defined(CONFIG_FPGA_ZYNQMPPL) && \
-- 
2.21.0

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot