[PATCH v3 1/3] remove CONFIG_DEBUG_INFO

[PATCH v3 1/3] remove CONFIG_DEBUG_INFO

[Rouven Czerwinski]

From: Oleksij Rempel <linux@rempel-privat.de>

The ELF file should have debug symbols, the binary should have no symbols any way.

Signed-off-by: Oleksij Rempel <linux@rempel-privat.de>
Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>
---
 Makefile       | 4 +---
 common/Kconfig | 6 ------
 2 files changed, 1 insertion(+), 9 deletions(-)

diff --git a/Makefile b/Makefile
index 29347b6780..900117e727 100644
--- a/Makefile
+++ b/Makefile
@@ -457,9 +457,7 @@ endif # $(dot-config)
 
 include $(srctree)/arch/$(ARCH)/Makefile
 
-ifdef CONFIG_DEBUG_INFO
-CFLAGS		+= -g
-endif
+CFLAGS		+= -ggdb3
 
 # Force gcc to behave correct even for buggy distributions
 CFLAGS          += $(call cc-option, -fno-stack-protector)
diff --git a/common/Kconfig b/common/Kconfig
index 7832df5c55..ffb7537592 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -999,12 +999,6 @@ config DEFAULT_LOGLEVEL
 	  7    debug-level messages (debug)
 	  8    verbose debug messages (vdebug)
 
-config DEBUG_INFO
-	bool
-	prompt "enable debug symbols"
-	help
-	  Enable build of barebox with -g.
-
 config DEBUG_LL
 	bool
 	depends on HAS_DEBUG_LL
-- 
2.21.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v3 2/3] add CONFIG_PBL_BREAK option

[Rouven Czerwinski]

From: Oleksij Rempel <linux@rempel-privat.de>

With this option barebox will be build with breakpoint instruction
in early pbl stage and on main barebox entry. Otherwise two nop
instructions are generated which allow binary patching of existing
barebox binaries.
In case production barebox images should be debugged, the corresponding
ELF binaries need to be kept to allow later JTAG debugging.
This option only works with an attached JTAG debugger, the board will
hang otherwise.

Signed-off-by: Oleksij Rempel <linux@rempel-privat.de>
Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>
---
 arch/arm/cpu/common.c              | 13 +++++++++++++
 arch/arm/cpu/start.c               |  2 ++
 arch/arm/include/asm/barebox-arm.h |  2 ++
 common/Kconfig                     |  9 +++++++++
 4 files changed, 26 insertions(+)

diff --git a/arch/arm/cpu/common.c b/arch/arm/cpu/common.c
index 51fe7ed988..6fe3476aa2 100644
--- a/arch/arm/cpu/common.c
+++ b/arch/arm/cpu/common.c
@@ -30,6 +30,19 @@
 #define R_ARM_RELATIVE 23
 #define R_AARCH64_RELATIVE 1027
 
+void pbl_barebox_break(void)
+{
+	__asm__ __volatile__ (
+#ifdef CONFIG_PBL_BREAK
+		"bkpt #17\n"
+		"nop\n"
+#else
+		"nop\n"
+		"nop\n"
+#endif
+	);
+}
+
 /*
  * relocate binary to the currently running address
  */
diff --git a/arch/arm/cpu/start.c b/arch/arm/cpu/start.c
index 6573c2ef74..6bfc4da34c 100644
--- a/arch/arm/cpu/start.c
+++ b/arch/arm/cpu/start.c
@@ -168,6 +168,8 @@ __noreturn void barebox_non_pbl_start(unsigned long membase,
 
 	barrier();
 
+	pbl_barebox_break();
+
 	pr_debug("memory at 0x%08lx, size 0x%08lx\n", membase, memsize);
 
 	arm_endmem = endmem;
diff --git a/arch/arm/include/asm/barebox-arm.h b/arch/arm/include/asm/barebox-arm.h
index a11d34923d..12765b7277 100644
--- a/arch/arm/include/asm/barebox-arm.h
+++ b/arch/arm/include/asm/barebox-arm.h
@@ -55,6 +55,7 @@ static inline unsigned long global_variable_offset(void)
 }
 
 void setup_c(void);
+void pbl_barebox_break(void);
 void relocate_to_current_adr(void);
 void relocate_to_adr(unsigned long target);
 void __noreturn barebox_arm_entry(unsigned long membase, unsigned long memsize, void *boarddata);
@@ -177,6 +178,7 @@ static inline unsigned long arm_mem_barebox_image(unsigned long membase,
 				(uint32_t r0, uint32_t r1, uint32_t r2)	\
 		{							\
 			__barebox_arm_head();				\
+			pbl_barebox_break();				\
 			__##name(r0, r1, r2);				\
 		}							\
 		static void NAKED noinline __##name			\
diff --git a/common/Kconfig b/common/Kconfig
index ffb7537592..59f6f59dcf 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -1245,6 +1245,15 @@ config DEBUG_INITCALLS
 	help
 	  If enabled this will print initcall traces.
 
+
+config PBL_BREAK
+	bool "Execute software break on pbl start"
+	help
+	  If this enabled, barebox will be compiled with BKPT instruction
+	  during early pbl init and entry of the main barebox.
+	  This option can only be used with a JTAG debugger,
+	  the board hangs otherwise.
+
 endmenu
 
 config HAS_DEBUG_LL
-- 
2.21.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v3 3/3] add gdb helper scripts to load barebox symbols

[Rouven Czerwinski]

Add a gdb script with two helper functions which use the new
pbl_barebox_break symbol to calculate the current barebox load address
on the device and loads a barebox ELF file to this address.

Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>
---
 Documentation/user/debugging.rst   | 21 +++++++++++
 Documentation/user/user-manual.rst |  1 +
 scripts/gdb/helper.py              | 60 ++++++++++++++++++++++++++++++
 3 files changed, 82 insertions(+)
 create mode 100644 Documentation/user/debugging.rst
 create mode 100644 scripts/gdb/helper.py

diff --git a/Documentation/user/debugging.rst b/Documentation/user/debugging.rst
new file mode 100644
index 0000000000..15cb439043
--- /dev/null
+++ b/Documentation/user/debugging.rst
@@ -0,0 +1,21 @@
+Debugging with OpenOCD
+======================
+
+Barebox can be configured to break on prebootloader and main barebox entry. This
+breakpoint can not be resumed and will stop the board to allow the user to
+attach a JTAG debugger with OpenOCD. Additionally, barebox provides helper
+scripts to load the symbols from the ELF binaries.
+The python scripts require `pyelftools`.
+To load the scripts into your gdb session, run the following command in the
+barebox directory:
+
+.. code-block:: none
+
+    (gdb) source scripts/gdb/helper.py
+
+This makes two new commands available in gdb, `bb-load-symbols` and
+`bb-skip-break`. `bb-load-symbols` can load either the main `barebox` file or
+one of the .pbl files in the image directories. The board needs to be stopped in
+either the prebootloader or main barebox breakpoint, and gdb needs to be
+connected to OpenOCD. To continue booting the board, `bb-skip-break` jumps over
+the breakpoint and continues the barebox execution.
diff --git a/Documentation/user/user-manual.rst b/Documentation/user/user-manual.rst
index 516b760b1b..f04981c3f0 100644
--- a/Documentation/user/user-manual.rst
+++ b/Documentation/user/user-manual.rst
@@ -33,6 +33,7 @@ Contents:
    system-reset
    state
    random
+   debugging
 
 * :ref:`search`
 * :ref:`genindex`
diff --git a/scripts/gdb/helper.py b/scripts/gdb/helper.py
new file mode 100644
index 0000000000..4041789890
--- /dev/null
+++ b/scripts/gdb/helper.py
@@ -0,0 +1,60 @@
+#!/usr/bin/env python
+
+import gdb
+
+try:
+    from elftools.elf.elffile import ELFFile
+    from elftools.common.exceptions import ELFError
+except ImportError:
+    gdb.write("The barebox helper requires pyelftools\n", gdb.STDERR)
+    exit(1)
+
+
+class BBSymbols(gdb.Command):
+
+    def __init__(self):
+        super(BBSymbols, self).__init__("bb-load-symbols", gdb.COMMAND_FILES,
+                                        gdb.COMPLETE_FILENAME)
+
+    def invoke(self, argument, from_tty):
+        path = argument
+        f = open(path, 'rb')
+
+        try:
+            elf = ELFFile(f)
+        except ELFError:
+            gdb.write("Selected file is not an ELF file\n", gdb.STDERR)
+            return
+
+        section = elf.get_section_by_name(".symtab")
+        if section is None:
+            gdb.write("Section .symtab not found\n", gdb.STDERR)
+            return
+        symbol = section.get_symbol_by_name("pbl_barebox_break")
+        if not symbol:
+            gdb.write("Symbol pbl_barebox_break in section {} in file {} not found\n"
+                      .format(section.name, self.path), gdb.STDERR)
+            return
+        symbol = symbol[0]
+        pc = gdb.parse_and_eval("$pc")
+        symbol_address = int(symbol.entry.st_value)
+        address = int(pc) - symbol_address + 1
+        gdb.execute("symbol-file")
+        gdb.execute("add-symbol-file {} {}".format(path, address))
+
+
+BBSymbols()
+
+
+class BBSkip(gdb.Command):
+
+    def __init__(self):
+        super(BBSkip, self).__init__("bb-skip-break", gdb.COMMAND_BREAKPOINTS)
+
+    def invoke(self, arg, from_tty):
+        pc = gdb.parse_and_eval("$pc")
+        nop_address = int(pc) + 2
+        gdb.execute("jump *{}".format(nop_address))
+
+
+BBSkip()
-- 
2.21.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v3 1/3] remove CONFIG_DEBUG_INFO

[Sascha Hauer]

On Fri, Jun 07, 2019 at 09:06:57AM +0200, Rouven Czerwinski wrote:
> From: Oleksij Rempel <linux@rempel-privat.de>
> 
> The ELF file should have debug symbols, the binary should have no symbols any way.
> 
> Signed-off-by: Oleksij Rempel <linux@rempel-privat.de>
> Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> ---
>  Makefile       | 4 +---
>  common/Kconfig | 6 ------
>  2 files changed, 1 insertion(+), 9 deletions(-)

Applied, thanks

Sascha

> 
> diff --git a/Makefile b/Makefile
> index 29347b6780..900117e727 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -457,9 +457,7 @@ endif # $(dot-config)
>  
>  include $(srctree)/arch/$(ARCH)/Makefile
>  
> -ifdef CONFIG_DEBUG_INFO
> -CFLAGS		+= -g
> -endif
> +CFLAGS		+= -ggdb3
>  
>  # Force gcc to behave correct even for buggy distributions
>  CFLAGS          += $(call cc-option, -fno-stack-protector)
> diff --git a/common/Kconfig b/common/Kconfig
> index 7832df5c55..ffb7537592 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -999,12 +999,6 @@ config DEFAULT_LOGLEVEL
>  	  7    debug-level messages (debug)
>  	  8    verbose debug messages (vdebug)
>  
> -config DEBUG_INFO
> -	bool
> -	prompt "enable debug symbols"
> -	help
> -	  Enable build of barebox with -g.
> -
>  config DEBUG_LL
>  	bool
>  	depends on HAS_DEBUG_LL
> -- 
> 2.21.0
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v3 2/3] add CONFIG_PBL_BREAK option

[Ahmad Fatoum]

Hello Oleksij,

On 7/6/19 09:06, Rouven Czerwinski wrote:
> From: Oleksij Rempel <linux@rempel-privat.de>
> 
> With this option barebox will be build with breakpoint instruction
> in early pbl stage and on main barebox entry. Otherwise two nop
> instructions are generated which allow binary patching of existing
> barebox binaries.

At least arm-linux-gnueabihf-gcc (Debian 8.3.0-2) does some funny
reordering with the result that the barebox header may no longer be
at the very start of the image.
This results e.g. in bootm not working for stm32 bareboxes, because
file_detect_type no longer detects them as valid barebox ARM images.

Steps to reproduce with current next:

$ make stm32mp1_defconfig
$ make
$ xxd -l64 -g1 images/start_stm32mp157c_dk2.pblb
00000000: 04 46 0d 46 16 46 00 00 01 90 8f e2 19 ff 2f e1  .F.F.F......../.
00000010: 00 f0 22 f8 fe e7 fe e7 fe e7 fe e7 fe e7 fe e7  ..".............
00000020: fe e7 fe e7 fe e7 fe e7 62 61 72 65 62 6f 78 00  ........barebox.
00000030: 00 00 00 00 00 00 00 00 55 55 55 55 55 55 55 55  ........UUUUUUUU

Note how the new 8 bytes are not after the __barebox_arm_head as they should.
This leads to the barebox string being shifted from the
ARM_HEAD_MAGICWORD_OFFSET, where it's expected.

Could you take a look?
Thanks. 

> In case production barebox images should be debugged, the corresponding
> ELF binaries need to be kept to allow later JTAG debugging.
> This option only works with an attached JTAG debugger, the board will
> hang otherwise.
> 
> Signed-off-by: Oleksij Rempel <linux@rempel-privat.de>
> Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> ---
>  arch/arm/cpu/common.c              | 13 +++++++++++++
>  arch/arm/cpu/start.c               |  2 ++
>  arch/arm/include/asm/barebox-arm.h |  2 ++
>  common/Kconfig                     |  9 +++++++++
>  4 files changed, 26 insertions(+)
> 
> diff --git a/arch/arm/cpu/common.c b/arch/arm/cpu/common.c
> index 51fe7ed988..6fe3476aa2 100644
> --- a/arch/arm/cpu/common.c
> +++ b/arch/arm/cpu/common.c
> @@ -30,6 +30,19 @@
>  #define R_ARM_RELATIVE 23
>  #define R_AARCH64_RELATIVE 1027
>  
> +void pbl_barebox_break(void)
> +{
> +	__asm__ __volatile__ (
> +#ifdef CONFIG_PBL_BREAK
> +		"bkpt #17\n"
> +		"nop\n"
> +#else
> +		"nop\n"
> +		"nop\n"
> +#endif
> +	);
> +}
> +
>  /*
>   * relocate binary to the currently running address
>   */
> diff --git a/arch/arm/cpu/start.c b/arch/arm/cpu/start.c
> index 6573c2ef74..6bfc4da34c 100644
> --- a/arch/arm/cpu/start.c
> +++ b/arch/arm/cpu/start.c
> @@ -168,6 +168,8 @@ __noreturn void barebox_non_pbl_start(unsigned long membase,
>  
>  	barrier();
>  
> +	pbl_barebox_break();
> +
>  	pr_debug("memory at 0x%08lx, size 0x%08lx\n", membase, memsize);
>  
>  	arm_endmem = endmem;
> diff --git a/arch/arm/include/asm/barebox-arm.h b/arch/arm/include/asm/barebox-arm.h
> index a11d34923d..12765b7277 100644
> --- a/arch/arm/include/asm/barebox-arm.h
> +++ b/arch/arm/include/asm/barebox-arm.h
> @@ -55,6 +55,7 @@ static inline unsigned long global_variable_offset(void)
>  }
>  
>  void setup_c(void);
> +void pbl_barebox_break(void);
>  void relocate_to_current_adr(void);
>  void relocate_to_adr(unsigned long target);
>  void __noreturn barebox_arm_entry(unsigned long membase, unsigned long memsize, void *boarddata);
> @@ -177,6 +178,7 @@ static inline unsigned long arm_mem_barebox_image(unsigned long membase,
>  				(uint32_t r0, uint32_t r1, uint32_t r2)	\
>  		{							\
>  			__barebox_arm_head();				\
> +			pbl_barebox_break();				\
>  			__##name(r0, r1, r2);				\
>  		}							\
>  		static void NAKED noinline __##name			\
> diff --git a/common/Kconfig b/common/Kconfig
> index ffb7537592..59f6f59dcf 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -1245,6 +1245,15 @@ config DEBUG_INITCALLS
>  	help
>  	  If enabled this will print initcall traces.
>  
> +
> +config PBL_BREAK
> +	bool "Execute software break on pbl start"
> +	help
> +	  If this enabled, barebox will be compiled with BKPT instruction
> +	  during early pbl init and entry of the main barebox.
> +	  This option can only be used with a JTAG debugger,
> +	  the board hangs otherwise.
> +
>  endmenu
>  
>  config HAS_DEBUG_LL
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v3 2/3] add CONFIG_PBL_BREAK option

[Sascha Hauer]

On Fri, Jun 07, 2019 at 09:06:59AM +0200, Rouven Czerwinski wrote:
> From: Oleksij Rempel <linux@rempel-privat.de>
> 
> With this option barebox will be build with breakpoint instruction
> in early pbl stage and on main barebox entry. Otherwise two nop
> instructions are generated which allow binary patching of existing
> barebox binaries.
> In case production barebox images should be debugged, the corresponding
> ELF binaries need to be kept to allow later JTAG debugging.
> This option only works with an attached JTAG debugger, the board will
> hang otherwise.
> 
>  
> +
> +config PBL_BREAK
> +	bool "Execute software break on pbl start"
> +	help
> +	  If this enabled, barebox will be compiled with BKPT instruction
> +	  during early pbl init and entry of the main barebox.
> +	  This option can only be used with a JTAG debugger,
> +	  the board hangs otherwise.

This option is presented to everyone but implemented only on ARM.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v3 2/3] add CONFIG_PBL_BREAK option

[Sascha Hauer]

On Sun, Jun 09, 2019 at 11:36:35PM +0200, Ahmad Fatoum wrote:
> Hello Oleksij,
> 
> On 7/6/19 09:06, Rouven Czerwinski wrote:
> > From: Oleksij Rempel <linux@rempel-privat.de>
> > 
> > With this option barebox will be build with breakpoint instruction
> > in early pbl stage and on main barebox entry. Otherwise two nop
> > instructions are generated which allow binary patching of existing
> > barebox binaries.
> 
> At least arm-linux-gnueabihf-gcc (Debian 8.3.0-2) does some funny
> reordering with the result that the barebox header may no longer be
> at the very start of the image.
> This results e.g. in bootm not working for stm32 bareboxes, because
> file_detect_type no longer detects them as valid barebox ARM images.
> 
> Steps to reproduce with current next:
> 
> $ make stm32mp1_defconfig
> $ make
> $ xxd -l64 -g1 images/start_stm32mp157c_dk2.pblb
> 00000000: 04 46 0d 46 16 46 00 00 01 90 8f e2 19 ff 2f e1  .F.F.F......../.
> 00000010: 00 f0 22 f8 fe e7 fe e7 fe e7 fe e7 fe e7 fe e7  ..".............
> 00000020: fe e7 fe e7 fe e7 fe e7 62 61 72 65 62 6f 78 00  ........barebox.
> 00000030: 00 00 00 00 00 00 00 00 55 55 55 55 55 55 55 55  ........UUUUUUUU
> 
> Note how the new 8 bytes are not after the __barebox_arm_head as they should.
> This leads to the barebox string being shifted from the
> ARM_HEAD_MAGICWORD_OFFSET, where it's expected.
> 
> Could you take a look?

Yes, it doesn't work like this properly. I dropped 2/3 and 3/3 for now.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox