Handle data aborts gracefully

Handle data aborts gracefully

[Sascha Hauer]

With this series the md command no longer aborts on inaccessible memory
but instead prints a nice 'XXXXXXXX' for this area. Based on original work
by Jan. I changed it to be compilable on !ARM. It happened that the i.MX6
PCIe controller needs exactly this support, so I continued working on this
patch.

Sascha

----------------------------------------------------------------
Jan Luebbe (1):
      ARM/mem: handle data aborts gracefully for md

Sascha Hauer (1):
      ARM: Allow to mask data aborts

 arch/arm/cpu/exceptions.S      | 22 ++++++++++++++++++++++
 arch/arm/cpu/interrupts.c      | 17 +++++++++++++++++
 arch/arm/include/asm/barebox.h |  4 ++++
 common/memory_display.c        | 34 +++++++++++++++++++++++++++++-----
 include/abort.h                | 37 +++++++++++++++++++++++++++++++++++++
 5 files changed, 109 insertions(+), 5 deletions(-)
 create mode 100644 include/abort.h

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

[PATCH 1/2] ARM: Allow to mask data aborts

[Sascha Hauer]

Sometimes it's useful to test if a memory operation works or
aborts. This adds data_abort_mask() to ignore data aborts and
data_abort_unmask() to enable them again. This is used in the
next step for the 'md' command so that illegal addresses just
show 'xxxxxxxx' instead of crashing the system.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/cpu/exceptions.S      | 22 ++++++++++++++++++++++
 arch/arm/cpu/interrupts.c      | 17 +++++++++++++++++
 arch/arm/include/asm/barebox.h |  4 ++++
 include/abort.h                | 37 +++++++++++++++++++++++++++++++++++++
 4 files changed, 80 insertions(+)
 create mode 100644 include/abort.h

diff --git a/arch/arm/cpu/exceptions.S b/arch/arm/cpu/exceptions.S
index 167c8d1..51c3903 100644
--- a/arch/arm/cpu/exceptions.S
+++ b/arch/arm/cpu/exceptions.S
@@ -88,6 +88,21 @@
 	movs	pc, lr
 	.endm
 
+	.macro try_data_abort
+	ldr	r13, =arm_ignore_data_abort	@ check try mode
+	ldr	r13, [r13]
+	cmp	r13, #0
+	beq	do_abort_\@
+	ldr	r13, =arm_data_abort_occured
+	str	r13, [r13]
+	mrs	r13, spsr			@ read saved CPSR
+	tst	r13, #1<<5			@ check Thumb mode
+	subeq	lr, #4				@ next ARM instr
+	subne	lr, #6				@ next Thumb instr
+	movs	pc, lr
+do_abort_\@:
+	.endm
+
 	.macro get_irq_stack			@ setup IRQ stack
 	ldr	sp, IRQ_STACK_START
 	.endm
@@ -122,6 +137,7 @@ prefetch_abort:
 
 	.align	5
 data_abort:
+	try_data_abort
 	get_bad_stack
 	bad_save_user_regs
 	bl 	do_data_abort
@@ -202,5 +218,11 @@ _fiq: .word fiq
 
 .section .data
 .align 4
+.global arm_ignore_data_abort
+arm_ignore_data_abort:
+.word arm_ignore_data_abort /* When != 0 data aborts are ignored */
+.global arm_data_abort_occured
+arm_data_abort_occured:
+.word arm_data_abort_occured /* set != 0 by the data abort handler */
 abort_stack:
 .space 8
diff --git a/arch/arm/cpu/interrupts.c b/arch/arm/cpu/interrupts.c
index 6e60adc..8064776 100644
--- a/arch/arm/cpu/interrupts.c
+++ b/arch/arm/cpu/interrupts.c
@@ -23,6 +23,7 @@
  */
 
 #include <common.h>
+#include <abort.h>
 #include <asm/ptrace.h>
 #include <asm/unwind.h>
 
@@ -161,3 +162,19 @@ void do_irq (struct pt_regs *pt_regs)
 	printf ("interrupt request\n");
 	do_exception(pt_regs);
 }
+
+extern volatile int arm_ignore_data_abort;
+extern volatile int arm_data_abort_occured;
+
+void data_abort_mask(void)
+{
+	arm_data_abort_occured = 0;
+	arm_ignore_data_abort = 1;
+}
+
+int data_abort_unmask(void)
+{
+	arm_ignore_data_abort = 0;
+
+	return arm_data_abort_occured;
+}
diff --git a/arch/arm/include/asm/barebox.h b/arch/arm/include/asm/barebox.h
index 2b08d68..31a8e15 100644
--- a/arch/arm/include/asm/barebox.h
+++ b/arch/arm/include/asm/barebox.h
@@ -5,4 +5,8 @@
 #define ARCH_HAS_STACK_DUMP
 #endif
 
+#ifdef CONFIG_ARM_EXCEPTIONS
+#define ARCH_HAS_DATA_ABORT_MASK
+#endif
+
 #endif	/* _BAREBOX_H_ */
diff --git a/include/abort.h b/include/abort.h
new file mode 100644
index 0000000..7f14cb0
--- /dev/null
+++ b/include/abort.h
@@ -0,0 +1,37 @@
+#ifndef __ABORT_H
+#define __ABORT_H
+
+#include <asm/barebox.h>
+
+#ifdef ARCH_HAS_DATA_ABORT_MASK
+
+/*
+ * data_abort_mask - ignore data aborts
+ *
+ * If data aborts are ignored the data abort handler
+ * will just return.
+ */
+void data_abort_mask(void);
+
+/*
+ * data_abort_unmask - Enable data aborts
+ *
+ * returns true if a data abort has happened between calling data_abort_mask()
+ * and data_abort_unmask()
+ */
+int data_abort_unmask(void);
+
+#else
+
+static inline void data_abort_mask(void)
+{
+}
+
+static inline int data_abort_unmask(void)
+{
+	return 0;
+}
+
+#endif
+
+#endif /* __ABORT_H */
-- 
2.1.4


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

[PATCH 2/2] ARM/mem: handle data aborts gracefully for md

[Sascha Hauer]

From: Jan Luebbe <jlu@pengutronix.de>

Sometimes memory ranges contain inaccessible registers which trigger a
data abort when accessed. To handle this gracefully, we extend the data
abort exception handler to ignore the exception when configured to do
so.

This allows detecting inaccessible memory from the md command. It will
show XX for unreadable bytes instead.

The previous behaviour:
barebox@TI AM335x BeagleBone:/ md 0x50000000
unable to handle paging request at address 0x500000ac
pc : [<8fe2e0dc>]    lr : [<8fe2e0b9>]
sp : 8ffff898  ip : 00000024  fp : 00000000
r10: 8bfa0204  r9 : 00000000  r8 : 8bfa0204
r7 : 00000100  r6 : 50000000  r5 : 8bfa0204  r4 : 00000004
r3 : 00000f00  r2 : 000000ac  r1 : 00000004  r0 : 00000014
Flags: nZCv  IRQs off  FIQs on  Mode SVC_32
[<8fe2e0dc>] (memcpy_sz+0x40/0x48) from [<8fe2f337>] (mem_read+0x3b/0x48)
[<8fe2f337>] (mem_read+0x3b/0x48) from [<8fe2bb13>] (cdev_read+0x25/0x2e)
[<8fe2bb13>] (cdev_read+0x25/0x2e) from [<8fe2c15b>] (devfs_read+0x1b/0x1e)
[<8fe2c15b>] (devfs_read+0x1b/0x1e) from [<8fe2df5b>] (__read+0x43/0x5c)
[<8fe2df5b>] (__read+0x43/0x5c) from [<8fe2e61f>] (read+0x2b/0x48)
[<8fe2e61f>] (read+0x2b/0x48) from [<8fe1e4a5>] (do_mem_md+0xc1/0x144)
[<8fe1e4a5>] (do_mem_md+0xc1/0x144) from [<8fe02889>] (execute_command+0x21/0x48)
[<8fe02889>] (execute_command+0x21/0x48) from [<8fe061c1>] (run_list_real+0x549/0x634)
[<8fe061c1>] (run_list_real+0x549/0x634) from [<8fe05b43>] (parse_stream_outer+0xdb/0x174)
[<8fe05b43>] (parse_stream_outer+0xdb/0x174) from [<8fe06435>] (run_shell+0x29/0x54)
[<8fe06435>] (run_shell+0x29/0x54) from [<8fe02889>] (execute_command+0x21/0x48)
[<8fe02889>] (execute_command+0x21/0x48) from [<8fe061c1>] (run_list_real+0x549/0x634)
[<8fe061c1>] (run_list_real+0x549/0x634) from [<8fe05efb>] (run_list_real+0x283/0x634)

[<8fe31e1d>] (unwind_backtrace+0x1/0x64) from [<8fe24e61>] (panic+0x1d/0x34)
[<8fe24e61>] (panic+0x1d/0x34) from [<8fe322c1>] (do_exception+0xd/0x10)
[<8fe322c1>] (do_exception+0xd/0x10) from [<8fe32329>] (do_data_abort+0x21/0x2c)
[<8fe32329>] (do_data_abort+0x21/0x2c) from [<8fe31fe8>] (data_abort+0x48/0x60)

The new behaviour:
barebox@TI AM335x BeagleBone:/ md 0x50000000
50000000: 00000060 00000000 00000000 00000000                `...............
50000010: 00000000 00000001 00000000 00000000                ................
50000020: 00000000 00000000 00000000 00000000                ................
50000030: 00000000 00000000 00000000 00000000                ................
50000040: 00001ff0 400000ac 00000211 00000000                .......@........
50000050: 00000a00 00000001 00000000 00000000                ................
50000060: 00000000 00101001 22060514 10057016                ...........".p..
50000070: 010f1111 8f070000 00000f40 00000000                ........@.......
50000080: 00000000 00000000 00000000 00000000                ................
50000090: 00001000 00101001 22060514 10057016                ...........".p..
500000a0: 010f1111 8f070000 00000f00 XXXXXXXX                ................
500000b0: XXXXXXXX XXXXXXXX 00000000 00000000                ................
500000c0: 00001000 00101001 22060514 10057016                ...........".p..
500000d0: 010f1111 8f070000 00000f00 XXXXXXXX                ................
500000e0: XXXXXXXX XXXXXXXX 00000000 00000000                ................
500000f0: 00001000 00101001 22060514 10057016                ...........".p..

The current implementation breaks everything except ARM and takes several
shortcuts which need to be implemented more cleanly. Suggestions are very
welcome!

Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/memory_display.c | 34 +++++++++++++++++++++++++++++-----
 1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/common/memory_display.c b/common/memory_display.c
index c8ae57a..7b1d35e 100644
--- a/common/memory_display.c
+++ b/common/memory_display.c
@@ -1,5 +1,6 @@
 #include <common.h>
 #include <errno.h>
+#include <abort.h>
 
 #define DISP_LINE_LEN	16
 
@@ -26,18 +27,41 @@ int memory_display(const void *addr, loff_t offs, unsigned nbytes, int size, int
 		for (i = 0; i < linebytes; i += size) {
 			if (size == 4) {
 				u32 res;
-				res = (*uip++ = *((uint *)addr));
+				data_abort_mask();
+				res = *((uint *)addr);
 				if (swab)
 					res = __swab32(res);
-				count -= printf(" %08x", res);
+				if (data_abort_unmask()) {
+					res = 0xffffffff;
+					count -= printf(" xxxxxxxx");
+				} else {
+					count -= printf(" %08x", res);
+				}
+				*uip++ = res;
 			} else if (size == 2) {
 				u16 res;
-				res = (*usp++ = *((ushort *)addr));
+				data_abort_mask();
+				res = *((ushort *)addr);
 				if (swab)
 					res = __swab16(res);
-				count -= printf(" %04x", res);
+				if (data_abort_unmask()) {
+					res = 0xffff;
+					count -= printf(" xxxx");
+				} else {
+					count -= printf(" %04x", res);
+				}
+				*usp++ = res;
 			} else {
-				count -= printf(" %02x", (*ucp++ = *((u_char *)addr)));
+				u8 res;
+				data_abort_mask();
+				res = *((u_char *)addr);
+				if (data_abort_unmask()) {
+					res = 0xff;
+					count -= printf(" xx");
+				} else {
+					count -= printf(" %02x", res);
+				}
+				*ucp++ = res;
 			}
 			addr += size;
 			offs += size;
-- 
2.1.4


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

Re: [PATCH 1/2] ARM: Allow to mask data aborts

[Jan Lübbe]

On Do, 2015-02-26 at 19:05 +0100, Sascha Hauer wrote:
> +int data_abort_unmask(void)
> +{
> +       arm_ignore_data_abort = 0;
> +
> +       return arm_data_abort_occured;
> +}

arm_data_abort_occured is set to &arm_data_abort_occured when an abort
occurs. Should we return arm_data_abort_occured != 0?

Also occured is actually spelled occurred.

Thanks,
Jan
-- 
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 1/2] ARM: Allow to mask data aborts

[Sascha Hauer]

On Fri, Feb 27, 2015 at 01:04:53PM +0100, Jan Lübbe wrote:
> On Do, 2015-02-26 at 19:05 +0100, Sascha Hauer wrote:
> > +int data_abort_unmask(void)
> > +{
> > +       arm_ignore_data_abort = 0;
> > +
> > +       return arm_data_abort_occured;
> > +}
> 
> arm_data_abort_occured is set to &arm_data_abort_occured when an abort
> occurs. Should we return arm_data_abort_occured != 0?

Yes, sounds good. Changed that.

> 
> Also occured is actually spelled occurred.

Fixed, thanks

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