Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 8/9] ARM: support uprobe handling
From: Rabin Vincent @ 2012-10-14 19:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1350242593-17761-1-git-send-email-rabin@rab.in>

Extend the kprobes code to handle user-space probes.  Much of the code
can be reused so currently the ARM uprobes code reuses the kprobes
structures.  The decode tables are reused, with the modification that
for those instruction that require custom decoding for uprobes, a new
element is added in the table to specify a custom decoder function.

Thumb is not handled.

Cc: Jon Medhurst <tixy@yxit.co.uk>
Signed-off-by: Rabin Vincent <rabin@rab.in>
---
 arch/arm/include/asm/kprobes.h   |   17 +---
 arch/arm/include/asm/probes.h    |   23 +++++
 arch/arm/kernel/kprobes-arm.c    |   27 +++++-
 arch/arm/kernel/kprobes-common.c |   63 ++++++++++----
 arch/arm/kernel/kprobes-test.c   |   12 ++-
 arch/arm/kernel/kprobes-thumb.c  |   31 ++++---
 arch/arm/kernel/kprobes.c        |    2 +-
 arch/arm/kernel/kprobes.h        |   36 +++++---
 arch/arm/kernel/uprobes-arm.c    |  178 ++++++++++++++++++++++++++++++++++++++
 arch/arm/kernel/uprobes.h        |   23 +++++
 10 files changed, 349 insertions(+), 63 deletions(-)
 create mode 100644 arch/arm/include/asm/probes.h
 create mode 100644 arch/arm/kernel/uprobes-arm.c
 create mode 100644 arch/arm/kernel/uprobes.h

diff --git a/arch/arm/include/asm/kprobes.h b/arch/arm/include/asm/kprobes.h
index f82ec22..53b1a80 100644
--- a/arch/arm/include/asm/kprobes.h
+++ b/arch/arm/include/asm/kprobes.h
@@ -27,22 +27,7 @@
 #define flush_insn_slot(p)		do { } while (0)
 #define kretprobe_blacklist_size	0
 
-typedef u32 kprobe_opcode_t;
-
-struct kprobe;
-typedef void (kprobe_insn_handler_t)(struct kprobe *, struct pt_regs *);
-typedef unsigned long (kprobe_check_cc)(unsigned long);
-typedef void (kprobe_insn_singlestep_t)(struct kprobe *, struct pt_regs *);
-typedef void (kprobe_insn_fn_t)(void);
-
-/* Architecture specific copy of original instruction. */
-struct arch_specific_insn {
-	kprobe_opcode_t			*insn;
-	kprobe_insn_handler_t		*insn_handler;
-	kprobe_check_cc			*insn_check_cc;
-	kprobe_insn_singlestep_t	*insn_singlestep;
-	kprobe_insn_fn_t		*insn_fn;
-};
+#include <asm/probes.h>
 
 struct prev_kprobe {
 	struct kprobe *kp;
diff --git a/arch/arm/include/asm/probes.h b/arch/arm/include/asm/probes.h
new file mode 100644
index 0000000..df46994
--- /dev/null
+++ b/arch/arm/include/asm/probes.h
@@ -0,0 +1,23 @@
+#ifndef _ASM_PROBES_H
+#define _ASM_PROBES_H
+
+#ifdef CONFIG_KPROBES
+typedef u32 kprobe_opcode_t;
+
+struct kprobe;
+typedef void (kprobe_insn_handler_t)(struct kprobe *, struct pt_regs *);
+typedef unsigned long (kprobe_check_cc)(unsigned long);
+typedef void (kprobe_insn_singlestep_t)(struct kprobe *, struct pt_regs *);
+typedef void (kprobe_insn_fn_t)(void);
+
+/* Architecture specific copy of original instruction. */
+struct arch_specific_insn {
+	kprobe_opcode_t			*insn;
+	kprobe_insn_handler_t		*insn_handler;
+	kprobe_check_cc			*insn_check_cc;
+	kprobe_insn_singlestep_t	*insn_singlestep;
+	kprobe_insn_fn_t		*insn_fn;
+};
+#endif
+
+#endif
diff --git a/arch/arm/kernel/kprobes-arm.c b/arch/arm/kernel/kprobes-arm.c
index 8a30c89..d9cf0e2 100644
--- a/arch/arm/kernel/kprobes-arm.c
+++ b/arch/arm/kernel/kprobes-arm.c
@@ -62,6 +62,7 @@
 #include <linux/kprobes.h>
 #include <linux/module.h>
 
+#include "uprobes.h"
 #include "kprobes.h"
 
 #define sign_extend(x, signbit) ((x) | (0 - ((x) & (1 << (signbit)))))
@@ -545,31 +546,37 @@ static const union decode_item arm_cccc_000x_____1xx1_table[] = {
 
 	/* LDRD (register)	cccc 000x x0x0 xxxx xxxx xxxx 1101 xxxx */
 	/* STRD (register)	cccc 000x x0x0 xxxx xxxx xxxx 1111 xxxx */
+	UDECODE_NEXT (decode_pc_ro)
 	DECODE_EMULATEX	(0x0e5000d0, 0x000000d0, emulate_ldrdstrd,
 						 REGS(NOPCWB, NOPCX, 0, 0, NOPC)),
 
 	/* LDRD (immediate)	cccc 000x x1x0 xxxx xxxx xxxx 1101 xxxx */
 	/* STRD (immediate)	cccc 000x x1x0 xxxx xxxx xxxx 1111 xxxx */
+	UDECODE_NEXT (decode_pc_ro)
 	DECODE_EMULATEX	(0x0e5000d0, 0x004000d0, emulate_ldrdstrd,
 						 REGS(NOPCWB, NOPCX, 0, 0, 0)),
 
 	/* STRH (register)	cccc 000x x0x0 xxxx xxxx xxxx 1011 xxxx */
+	UDECODE_NEXT (decode_pc_ro)
 	DECODE_EMULATEX	(0x0e5000f0, 0x000000b0, emulate_str,
 						 REGS(NOPCWB, NOPC, 0, 0, NOPC)),
 
 	/* LDRH (register)	cccc 000x x0x1 xxxx xxxx xxxx 1011 xxxx */
 	/* LDRSB (register)	cccc 000x x0x1 xxxx xxxx xxxx 1101 xxxx */
 	/* LDRSH (register)	cccc 000x x0x1 xxxx xxxx xxxx 1111 xxxx */
+	UDECODE_NEXT (decode_pc_ro)
 	DECODE_EMULATEX	(0x0e500090, 0x00100090, emulate_ldr,
 						 REGS(NOPCWB, NOPC, 0, 0, NOPC)),
 
 	/* STRH (immediate)	cccc 000x x1x0 xxxx xxxx xxxx 1011 xxxx */
+	UDECODE_NEXT (decode_pc_ro)
 	DECODE_EMULATEX	(0x0e5000f0, 0x004000b0, emulate_str,
 						 REGS(NOPCWB, NOPC, 0, 0, 0)),
 
 	/* LDRH (immediate)	cccc 000x x1x1 xxxx xxxx xxxx 1011 xxxx */
 	/* LDRSB (immediate)	cccc 000x x1x1 xxxx xxxx xxxx 1101 xxxx */
 	/* LDRSH (immediate)	cccc 000x x1x1 xxxx xxxx xxxx 1111 xxxx */
+	UDECODE_NEXT (decode_pc_ro)
 	DECODE_EMULATEX	(0x0e500090, 0x00500090, emulate_ldr,
 						 REGS(NOPCWB, NOPC, 0, 0, 0)),
 
@@ -589,11 +596,13 @@ static const union decode_item arm_cccc_000x_table[] = {
 	/* TEQ (register)	cccc 0001 0011 xxxx xxxx xxxx xxx0 xxxx */
 	/* CMP (register)	cccc 0001 0101 xxxx xxxx xxxx xxx0 xxxx */
 	/* CMN (register)	cccc 0001 0111 xxxx xxxx xxxx xxx0 xxxx */
+	UDECODE_NEXT (decode_rd12rn16rm0rs8_rwflags)
 	DECODE_EMULATEX	(0x0f900010, 0x01100000, emulate_rd12rn16rm0rs8_rwflags,
 						 REGS(ANY, 0, 0, 0, ANY)),
 
 	/* MOV (register)	cccc 0001 101x xxxx xxxx xxxx xxx0 xxxx */
 	/* MVN (register)	cccc 0001 111x xxxx xxxx xxxx xxx0 xxxx */
+	UDECODE_NEXT (decode_rd12rn16rm0rs8_rwflags)
 	DECODE_EMULATEX	(0x0fa00010, 0x01a00000, emulate_rd12rn16rm0rs8_rwflags,
 						 REGS(0, ANY, 0, 0, ANY)),
 
@@ -607,6 +616,7 @@ static const union decode_item arm_cccc_000x_table[] = {
 	/* RSC (register)	cccc 0000 111x xxxx xxxx xxxx xxx0 xxxx */
 	/* ORR (register)	cccc 0001 100x xxxx xxxx xxxx xxx0 xxxx */
 	/* BIC (register)	cccc 0001 110x xxxx xxxx xxxx xxx0 xxxx */
+	UDECODE_NEXT (decode_rd12rn16rm0rs8_rwflags)
 	DECODE_EMULATEX	(0x0e000010, 0x00000000, emulate_rd12rn16rm0rs8_rwflags,
 						 REGS(ANY, ANY, 0, 0, ANY)),
 
@@ -614,11 +624,13 @@ static const union decode_item arm_cccc_000x_table[] = {
 	/* TEQ (reg-shift reg)	cccc 0001 0011 xxxx xxxx xxxx 0xx1 xxxx */
 	/* CMP (reg-shift reg)	cccc 0001 0101 xxxx xxxx xxxx 0xx1 xxxx */
 	/* CMN (reg-shift reg)	cccc 0001 0111 xxxx xxxx xxxx 0xx1 xxxx */
+	UDECODE_NEXT (decode_rd12rn16rm0rs8_rwflags)
 	DECODE_EMULATEX	(0x0f900090, 0x01100010, emulate_rd12rn16rm0rs8_rwflags,
 						 REGS(ANY, 0, NOPC, 0, ANY)),
 
 	/* MOV (reg-shift reg)	cccc 0001 101x xxxx xxxx xxxx 0xx1 xxxx */
 	/* MVN (reg-shift reg)	cccc 0001 111x xxxx xxxx xxxx 0xx1 xxxx */
+	UDECODE_NEXT (decode_rd12rn16rm0rs8_rwflags)
 	DECODE_EMULATEX	(0x0fa00090, 0x01a00010, emulate_rd12rn16rm0rs8_rwflags,
 						 REGS(0, ANY, NOPC, 0, ANY)),
 
@@ -632,6 +644,7 @@ static const union decode_item arm_cccc_000x_table[] = {
 	/* RSC (reg-shift reg)	cccc 0000 111x xxxx xxxx xxxx 0xx1 xxxx */
 	/* ORR (reg-shift reg)	cccc 0001 100x xxxx xxxx xxxx 0xx1 xxxx */
 	/* BIC (reg-shift reg)	cccc 0001 110x xxxx xxxx xxxx 0xx1 xxxx */
+	UDECODE_NEXT (decode_rd12rn16rm0rs8_rwflags)
 	DECODE_EMULATEX	(0x0e000090, 0x00000010, emulate_rd12rn16rm0rs8_rwflags,
 						 REGS(ANY, ANY, NOPC, 0, ANY)),
 
@@ -666,11 +679,13 @@ static const union decode_item arm_cccc_001x_table[] = {
 	/* TEQ (immediate)	cccc 0011 0011 xxxx xxxx xxxx xxxx xxxx */
 	/* CMP (immediate)	cccc 0011 0101 xxxx xxxx xxxx xxxx xxxx */
 	/* CMN (immediate)	cccc 0011 0111 xxxx xxxx xxxx xxxx xxxx */
+	UDECODE_NEXT (decode_rd12rn16rm0rs8_rwflags)
 	DECODE_EMULATEX	(0x0f900000, 0x03100000, emulate_rd12rn16rm0rs8_rwflags,
 						 REGS(ANY, 0, 0, 0, 0)),
 
 	/* MOV (immediate)	cccc 0011 101x xxxx xxxx xxxx xxxx xxxx */
 	/* MVN (immediate)	cccc 0011 111x xxxx xxxx xxxx xxxx xxxx */
+	UDECODE_NEXT (decode_rd12rn16rm0rs8_rwflags)
 	DECODE_EMULATEX	(0x0fa00000, 0x03a00000, emulate_rd12rn16rm0rs8_rwflags,
 						 REGS(0, ANY, 0, 0, 0)),
 
@@ -684,6 +699,7 @@ static const union decode_item arm_cccc_001x_table[] = {
 	/* RSC (immediate)	cccc 0010 111x xxxx xxxx xxxx xxxx xxxx */
 	/* ORR (immediate)	cccc 0011 100x xxxx xxxx xxxx xxxx xxxx */
 	/* BIC (immediate)	cccc 0011 110x xxxx xxxx xxxx xxxx xxxx */
+	UDECODE_NEXT (decode_rd12rn16rm0rs8_rwflags)
 	DECODE_EMULATEX	(0x0e000000, 0x02000000, emulate_rd12rn16rm0rs8_rwflags,
 						 REGS(ANY, ANY, 0, 0, 0)),
 
@@ -850,21 +866,25 @@ static const union decode_item arm_cccc_01xx_table[] = {
 
 	/* STR (immediate)	cccc 010x x0x0 xxxx xxxx xxxx xxxx xxxx */
 	/* STRB (immediate)	cccc 010x x1x0 xxxx xxxx xxxx xxxx xxxx */
+	UDECODE_NEXT (decode_pc_ro)
 	DECODE_EMULATEX	(0x0e100000, 0x04000000, emulate_str,
 						 REGS(NOPCWB, ANY, 0, 0, 0)),
 
 	/* LDR (immediate)	cccc 010x x0x1 xxxx xxxx xxxx xxxx xxxx */
 	/* LDRB (immediate)	cccc 010x x1x1 xxxx xxxx xxxx xxxx xxxx */
+	UDECODE_NEXT (decode_ldr)
 	DECODE_EMULATEX	(0x0e100000, 0x04100000, emulate_ldr,
 						 REGS(NOPCWB, ANY, 0, 0, 0)),
 
 	/* STR (register)	cccc 011x x0x0 xxxx xxxx xxxx xxxx xxxx */
 	/* STRB (register)	cccc 011x x1x0 xxxx xxxx xxxx xxxx xxxx */
+	UDECODE_NEXT (decode_pc_ro)
 	DECODE_EMULATEX	(0x0e100000, 0x06000000, emulate_str,
 						 REGS(NOPCWB, ANY, 0, 0, NOPC)),
 
 	/* LDR (register)	cccc 011x x0x1 xxxx xxxx xxxx xxxx xxxx */
 	/* LDRB (register)	cccc 011x x1x1 xxxx xxxx xxxx xxxx xxxx */
+	UDECODE_NEXT (decode_ldr)
 	DECODE_EMULATEX	(0x0e100000, 0x06100000, emulate_ldr,
 						 REGS(NOPCWB, ANY, 0, 0, NOPC)),
 
@@ -876,6 +896,7 @@ static const union decode_item arm_cccc_100x_table[] = {
 
 	/* LDM			cccc 100x x0x1 xxxx xxxx xxxx xxxx xxxx */
 	/* STM			cccc 100x x0x0 xxxx xxxx xxxx xxxx xxxx */
+	UDECODE_NEXT (uprobe_decode_ldmstm)
 	DECODE_CUSTOM	(0x0e400000, 0x08000000, kprobe_decode_ldmstm),
 
 	/* STM (user registers)	cccc 100x x1x0 xxxx xxxx xxxx xxxx xxxx */
@@ -997,9 +1018,11 @@ static void __kprobes arm_singlestep(struct kprobe *p, struct pt_regs *regs)
  *   should also be very rare.
  */
 enum kprobe_insn __kprobes
-arm_kprobe_decode_insn(kprobe_opcode_t insn, struct arch_specific_insn *asi)
+arm_kprobe_decode_insn(kprobe_opcode_t insn, struct arch_specific_insn *asi,
+		       bool uprobe)
 {
 	asi->insn_singlestep = arm_singlestep;
 	asi->insn_check_cc = kprobe_condition_checks[insn>>28];
-	return kprobe_decode_insn(insn, asi, kprobe_decode_arm_table, false);
+	return kprobe_decode_insn(insn, asi, kprobe_decode_arm_table, false,
+				  uprobe);
 }
diff --git a/arch/arm/kernel/kprobes-common.c b/arch/arm/kernel/kprobes-common.c
index 18a7628..f811bfb 100644
--- a/arch/arm/kernel/kprobes-common.c
+++ b/arch/arm/kernel/kprobes-common.c
@@ -277,7 +277,8 @@ emulate_ldm_r3_15(struct kprobe *p, struct pt_regs *regs)
 }
 
 enum kprobe_insn __kprobes
-kprobe_decode_ldmstm(kprobe_opcode_t insn, struct arch_specific_insn *asi)
+kprobe_decode_ldmstm(kprobe_opcode_t insn, struct arch_specific_insn *asi,
+		     void *d)
 {
 	kprobe_insn_handler_t *handler = 0;
 	unsigned reglist = insn & 0xffff;
@@ -389,7 +390,7 @@ set_emulated_insn(kprobe_opcode_t insn, struct arch_specific_insn *asi,
  * non-zero value, the corresponding nibble in pinsn is validated and modified
  * according to the type.
  */
-static bool __kprobes decode_regs(kprobe_opcode_t* pinsn, u32 regs)
+static bool __kprobes decode_regs(kprobe_opcode_t *pinsn, u32 regs, bool modify)
 {
 	kprobe_opcode_t insn = *pinsn;
 	kprobe_opcode_t mask = 0xf; /* Start at least significant nibble */
@@ -450,12 +451,16 @@ static bool __kprobes decode_regs(kprobe_opcode_t* pinsn, u32 regs)
 			break;
 		}
 
-		/* Replace value of nibble with new register number... */
-		insn &= ~mask;
-		insn |= new_bits & mask;
+		if (modify) {
+			/* Replace value of nibble with new register number */
+			insn &= ~mask;
+			insn |= new_bits & mask;
+		}
 	}
 
-	*pinsn = insn;
+	if (modify)
+		*pinsn = insn;
+
 	return true;
 
 reject:
@@ -468,7 +473,8 @@ static const int decode_struct_sizes[NUM_DECODE_TYPES] = {
 	[DECODE_TYPE_SIMULATE]	= sizeof(struct decode_simulate),
 	[DECODE_TYPE_EMULATE]	= sizeof(struct decode_emulate),
 	[DECODE_TYPE_OR]	= sizeof(struct decode_or),
-	[DECODE_TYPE_REJECT]	= sizeof(struct decode_reject)
+	[DECODE_TYPE_REJECT]	= sizeof(struct decode_reject),
+	[UDECODE_TYPE_NEXT]	= sizeof(struct decode_header)
 };
 
 /*
@@ -516,13 +522,17 @@ static const int decode_struct_sizes[NUM_DECODE_TYPES] = {
  */
 int __kprobes
 kprobe_decode_insn(kprobe_opcode_t insn, struct arch_specific_insn *asi,
-				const union decode_item *table, bool thumb)
+				const union decode_item *table, bool thumb,
+				bool uprobe)
 {
+	enum kprobe_insn (*decoder)(kprobe_opcode_t,
+			  struct arch_specific_insn *, void *d) = NULL;
 	const struct decode_header *h = (struct decode_header *)table;
 	const struct decode_header *next;
 	bool matched = false;
 
-	insn = prepare_emulated_insn(insn, asi, thumb);
+	if (!uprobe)
+		insn = prepare_emulated_insn(insn, asi, thumb);
 
 	for (;; h = next) {
 		enum decode_type type = h->type_regs.bits & DECODE_TYPE_MASK;
@@ -534,13 +544,24 @@ kprobe_decode_insn(kprobe_opcode_t insn, struct arch_specific_insn *asi,
 		next = (struct decode_header *)
 				((uintptr_t)h + decode_struct_sizes[type]);
 
-		if (!matched && (insn & h->mask.bits) != h->value.bits)
+		if (!uprobe && type == UDECODE_TYPE_NEXT)
 			continue;
 
-		if (!decode_regs(&insn, regs))
-			return INSN_REJECTED;
+		if (type != UDECODE_TYPE_NEXT) {
+			if (!matched &&
+			    (insn & h->mask.bits) != h->value.bits) {
+				decoder = NULL;
+				continue;
+			}
+
+			if (!decode_regs(&insn, regs, !uprobe))
+				return INSN_REJECTED;
+		}
 
 		switch (type) {
+		case UDECODE_TYPE_NEXT:
+			decoder = h->mask.decoder;
+			break;
 
 		case DECODE_TYPE_TABLE: {
 			struct decode_table *d = (struct decode_table *)h;
@@ -550,7 +571,11 @@ kprobe_decode_insn(kprobe_opcode_t insn, struct arch_specific_insn *asi,
 
 		case DECODE_TYPE_CUSTOM: {
 			struct decode_custom *d = (struct decode_custom *)h;
-			return (*d->decoder.decoder)(insn, asi);
+
+			if (decoder)
+				return decoder(insn, asi, d);
+
+			return (*d->decoder.decoder)(insn, asi, d);
 		}
 
 		case DECODE_TYPE_SIMULATE: {
@@ -561,8 +586,14 @@ kprobe_decode_insn(kprobe_opcode_t insn, struct arch_specific_insn *asi,
 
 		case DECODE_TYPE_EMULATE: {
 			struct decode_emulate *d = (struct decode_emulate *)h;
-			asi->insn_handler = d->handler.handler;
-			set_emulated_insn(insn, asi, thumb);
+
+			if (decoder)
+				return decoder(insn, asi, d);
+
+			if (!uprobe) {
+				asi->insn_handler = d->handler.handler;
+				set_emulated_insn(insn, asi, thumb);
+			}
 			return INSN_GOOD;
 		}
 
@@ -574,5 +605,5 @@ kprobe_decode_insn(kprobe_opcode_t insn, struct arch_specific_insn *asi,
 		default:
 			return INSN_REJECTED;
 		}
-		}
 	}
+}
diff --git a/arch/arm/kernel/kprobes-test.c b/arch/arm/kernel/kprobes-test.c
index 1862d8f..a2d4213 100644
--- a/arch/arm/kernel/kprobes-test.c
+++ b/arch/arm/kernel/kprobes-test.c
@@ -630,7 +630,8 @@ static const int decode_struct_sizes[NUM_DECODE_TYPES] = {
 	[DECODE_TYPE_SIMULATE]	= sizeof(struct decode_simulate),
 	[DECODE_TYPE_EMULATE]	= sizeof(struct decode_emulate),
 	[DECODE_TYPE_OR]	= sizeof(struct decode_or),
-	[DECODE_TYPE_REJECT]	= sizeof(struct decode_reject)
+	[DECODE_TYPE_REJECT]	= sizeof(struct decode_reject),
+	[UDECODE_TYPE_NEXT]	= sizeof(struct decode_header)
 };
 
 static int table_iter(const union decode_item *table,
@@ -646,9 +647,11 @@ static int table_iter(const union decode_item *table,
 		if (type == DECODE_TYPE_END)
 			return 0;
 
-		result = fn(h, args);
-		if (result)
-			return result;
+		if (type != UDECODE_TYPE_NEXT) {
+			result = fn(h, args);
+			if (result)
+				return result;
+		}
 
 		h = (struct decode_header *)
 			((uintptr_t)h + decode_struct_sizes[type]);
@@ -918,6 +921,7 @@ static void coverage_add(kprobe_opcode_t insn)
 			break;
 
 		case DECODE_TYPE_REJECT:
+		case UDECODE_TYPE_NEXT:
 		default:
 			return;
 		}
diff --git a/arch/arm/kernel/kprobes-thumb.c b/arch/arm/kernel/kprobes-thumb.c
index 6123daf..761aa55 100644
--- a/arch/arm/kernel/kprobes-thumb.c
+++ b/arch/arm/kernel/kprobes-thumb.c
@@ -83,7 +83,8 @@ t32_simulate_cond_branch(struct kprobe *p, struct pt_regs *regs)
 }
 
 static enum kprobe_insn __kprobes
-t32_decode_cond_branch(kprobe_opcode_t insn, struct arch_specific_insn *asi)
+t32_decode_cond_branch(kprobe_opcode_t insn, struct arch_specific_insn *asi,
+		       void *d)
 {
 	int cc = (insn >> 22) & 0xf;
 	asi->insn_check_cc = kprobe_condition_checks[cc];
@@ -158,9 +159,10 @@ t32_simulate_ldr_literal(struct kprobe *p, struct pt_regs *regs)
 }
 
 static enum kprobe_insn __kprobes
-t32_decode_ldmstm(kprobe_opcode_t insn, struct arch_specific_insn *asi)
+t32_decode_ldmstm(kprobe_opcode_t insn, struct arch_specific_insn *asi,
+		  void *d)
 {
-	enum kprobe_insn ret = kprobe_decode_ldmstm(insn, asi);
+	enum kprobe_insn ret = kprobe_decode_ldmstm(insn, asi, d);
 
 	/* Fixup modified instruction to have halfwords in correct order...*/
 	insn = asi->insn[0];
@@ -1046,7 +1048,7 @@ t16_singlestep_it(struct kprobe *p, struct pt_regs *regs)
 }
 
 static enum kprobe_insn __kprobes
-t16_decode_it(kprobe_opcode_t insn, struct arch_specific_insn *asi)
+t16_decode_it(kprobe_opcode_t insn, struct arch_specific_insn *asi, void *d)
 {
 	asi->insn_singlestep = t16_singlestep_it;
 	return INSN_GOOD_NO_SLOT;
@@ -1063,7 +1065,8 @@ t16_simulate_cond_branch(struct kprobe *p, struct pt_regs *regs)
 }
 
 static enum kprobe_insn __kprobes
-t16_decode_cond_branch(kprobe_opcode_t insn, struct arch_specific_insn *asi)
+t16_decode_cond_branch(kprobe_opcode_t insn, struct arch_specific_insn *asi,
+		       void *d)
 {
 	int cc = (insn >> 8) & 0xf;
 	asi->insn_check_cc = kprobe_condition_checks[cc];
@@ -1149,7 +1152,7 @@ t16_emulate_hiregs(struct kprobe *p, struct pt_regs *regs)
 }
 
 static enum kprobe_insn __kprobes
-t16_decode_hiregs(kprobe_opcode_t insn, struct arch_specific_insn *asi)
+t16_decode_hiregs(kprobe_opcode_t insn, struct arch_specific_insn *asi, void *d)
 {
 	insn &= ~0x00ff;
 	insn |= 0x001; /* Set Rdn = R1 and Rm = R0 */
@@ -1175,7 +1178,7 @@ t16_emulate_push(struct kprobe *p, struct pt_regs *regs)
 }
 
 static enum kprobe_insn __kprobes
-t16_decode_push(kprobe_opcode_t insn, struct arch_specific_insn *asi)
+t16_decode_push(kprobe_opcode_t insn, struct arch_specific_insn *asi, void *d)
 {
 	/*
 	 * To simulate a PUSH we use a Thumb-2 "STMDB R9!, {registers}"
@@ -1225,7 +1228,7 @@ t16_emulate_pop_pc(struct kprobe *p, struct pt_regs *regs)
 }
 
 static enum kprobe_insn __kprobes
-t16_decode_pop(kprobe_opcode_t insn, struct arch_specific_insn *asi)
+t16_decode_pop(kprobe_opcode_t insn, struct arch_specific_insn *asi, void *d)
 {
 	/*
 	 * To simulate a POP we use a Thumb-2 "LDMDB R9!, {registers}"
@@ -1453,17 +1456,21 @@ static void __kprobes thumb32_singlestep(struct kprobe *p, struct pt_regs *regs)
 }
 
 enum kprobe_insn __kprobes
-thumb16_kprobe_decode_insn(kprobe_opcode_t insn, struct arch_specific_insn *asi)
+thumb16_kprobe_decode_insn(kprobe_opcode_t insn, struct arch_specific_insn *asi,
+			   bool uprobes)
 {
 	asi->insn_singlestep = thumb16_singlestep;
 	asi->insn_check_cc = thumb_check_cc;
-	return kprobe_decode_insn(insn, asi, kprobe_decode_thumb16_table, true);
+	return kprobe_decode_insn(insn, asi, kprobe_decode_thumb16_table, true,
+				  uprobes);
 }
 
 enum kprobe_insn __kprobes
-thumb32_kprobe_decode_insn(kprobe_opcode_t insn, struct arch_specific_insn *asi)
+thumb32_kprobe_decode_insn(kprobe_opcode_t insn, struct arch_specific_insn *asi,
+			   bool uprobes)
 {
 	asi->insn_singlestep = thumb32_singlestep;
 	asi->insn_check_cc = thumb_check_cc;
-	return kprobe_decode_insn(insn, asi, kprobe_decode_thumb32_table, true);
+	return kprobe_decode_insn(insn, asi, kprobe_decode_thumb32_table, true,
+				  uprobes);
 }
diff --git a/arch/arm/kernel/kprobes.c b/arch/arm/kernel/kprobes.c
index 4dd41fc..1b2a324 100644
--- a/arch/arm/kernel/kprobes.c
+++ b/arch/arm/kernel/kprobes.c
@@ -80,7 +80,7 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)
 	p->opcode = insn;
 	p->ainsn.insn = tmp_insn;
 
-	switch ((*decode_insn)(insn, &p->ainsn)) {
+	switch ((*decode_insn)(insn, &p->ainsn, false)) {
 	case INSN_REJECTED:	/* not supported */
 		return -EINVAL;
 
diff --git a/arch/arm/kernel/kprobes.h b/arch/arm/kernel/kprobes.h
index 38945f7..442a161 100644
--- a/arch/arm/kernel/kprobes.h
+++ b/arch/arm/kernel/kprobes.h
@@ -35,20 +35,19 @@ enum kprobe_insn {
 };
 
 typedef enum kprobe_insn (kprobe_decode_insn_t)(kprobe_opcode_t,
-						struct arch_specific_insn *);
-
-#ifdef CONFIG_THUMB2_KERNEL
+						struct arch_specific_insn *,
+						bool uprobes);
 
 enum kprobe_insn thumb16_kprobe_decode_insn(kprobe_opcode_t,
-						struct arch_specific_insn *);
+					    struct arch_specific_insn *,
+					    bool uprobes);
 enum kprobe_insn thumb32_kprobe_decode_insn(kprobe_opcode_t,
-						struct arch_specific_insn *);
-
-#else /* !CONFIG_THUMB2_KERNEL */
+					    struct arch_specific_insn *,
+					    bool uprobes);
 
 enum kprobe_insn arm_kprobe_decode_insn(kprobe_opcode_t,
-					struct arch_specific_insn *);
-#endif
+					struct arch_specific_insn *,
+					bool uprobes);
 
 void __init arm_kprobe_decode_init(void);
 
@@ -165,7 +164,8 @@ void __kprobes kprobe_simulate_nop(struct kprobe *p, struct pt_regs *regs);
 void __kprobes kprobe_emulate_none(struct kprobe *p, struct pt_regs *regs);
 
 enum kprobe_insn __kprobes
-kprobe_decode_ldmstm(kprobe_opcode_t insn, struct arch_specific_insn *asi);
+kprobe_decode_ldmstm(kprobe_opcode_t insn, struct arch_specific_insn *asi,
+		     void *d);
 
 /*
  * Test if load/store instructions writeback the address register.
@@ -292,6 +292,7 @@ enum decode_type {
 	DECODE_TYPE_EMULATE,
 	DECODE_TYPE_OR,
 	DECODE_TYPE_REJECT,
+	UDECODE_TYPE_NEXT,
 	NUM_DECODE_TYPES /* Must be last enum */
 };
 
@@ -331,7 +332,9 @@ union decode_item {
 	u32			bits;
 	const union decode_item	*table;
 	kprobe_insn_handler_t	*handler;
-	kprobe_decode_insn_t	*decoder;
+	enum kprobe_insn (*decoder)(kprobe_opcode_t,
+				    struct arch_specific_insn *,
+				    void *d);
 };
 
 
@@ -396,6 +399,14 @@ struct decode_emulate {
 #define DECODE_EMULATE(_mask, _value, _handler)		\
 	DECODE_EMULATEX(_mask, _value, _handler, 0)
 
+#ifdef CONFIG_UPROBES
+#define UDECODE_NEXT(_decoder)				\
+	{.bits = UDECODE_TYPE_NEXT },			\
+	{.decoder = _decoder},				\
+	{.bits = 0 },
+#else
+#define UDECODE_NEXT(_decoder)
+#endif
 
 struct decode_or {
 	struct decode_header	header;
@@ -422,7 +433,8 @@ extern const union decode_item kprobe_decode_arm_table[];
 
 
 int kprobe_decode_insn(kprobe_opcode_t insn, struct arch_specific_insn *asi,
-			const union decode_item *table, bool thumb16);
+			const union decode_item *table, bool thumb16,
+			bool uprobe);
 
 
 #endif /* _ARM_KERNEL_KPROBES_H */
diff --git a/arch/arm/kernel/uprobes-arm.c b/arch/arm/kernel/uprobes-arm.c
new file mode 100644
index 0000000..8d7de10
--- /dev/null
+++ b/arch/arm/kernel/uprobes-arm.c
@@ -0,0 +1,178 @@
+#include <linux/kernel.h>
+#include <linux/kprobes.h>
+#include <linux/uprobes.h>
+#include <linux/module.h>
+
+#include "uprobes.h"
+#include "kprobes.h"
+
+static int uprobes_substitute_pc(kprobe_opcode_t *pinsn, u32 oregs)
+{
+	kprobe_opcode_t insn = *pinsn;
+	kprobe_opcode_t temp;
+	kprobe_opcode_t mask;
+	int freereg;
+	u32 free = 0xffff;
+	u32 regs;
+
+	for (regs = oregs; regs; regs >>= 4, insn >>= 4) {
+		if ((regs & 0xf) == REG_TYPE_NONE)
+			continue;
+
+		free &= ~(1 << (insn & 0xf));
+	}
+
+	/* No PC, no problem */
+	if (free & (1 << 15))
+		return 15;
+
+	if (!free)
+		return -1;
+
+	/*
+	 * fls instead of ffs ensures that for "ldrd r0, r1, [pc]" we would
+	 * pick LR instead of R1.
+	 */
+	freereg = free = fls(free) - 1;
+
+	temp = *pinsn;
+	insn = *pinsn;
+	regs = oregs;
+	mask = 0xf;
+
+	for (; regs; regs >>= 4, mask <<= 4, free <<= 4, temp >>= 4) {
+		if ((regs & 0xf) == REG_TYPE_NONE)
+			continue;
+
+		if ((temp & 0xf) != 15)
+			continue;
+
+		insn &= ~mask;
+		insn |= free & mask;
+	}
+
+	*pinsn = insn;
+	return freereg;
+}
+
+static void uprobe_set_pc(struct arch_uprobe *auprobe,
+			  struct arch_uprobe_task *autask,
+			  struct pt_regs *regs)
+{
+	u32 pcreg = auprobe->pcreg;
+
+	autask->backup = regs->uregs[pcreg];
+	regs->uregs[pcreg] = regs->ARM_pc + 8;
+}
+
+static void uprobe_unset_pc(struct arch_uprobe *auprobe,
+			    struct arch_uprobe_task *autask,
+			    struct pt_regs *regs)
+{
+	/* PC will be taken care of by common code */
+	regs->uregs[auprobe->pcreg] = autask->backup;
+}
+
+static void uprobe_aluwrite_pc(struct arch_uprobe *auprobe,
+			       struct arch_uprobe_task *autask,
+			       struct pt_regs *regs)
+{
+	u32 pcreg = auprobe->pcreg;
+
+	alu_write_pc(regs->uregs[pcreg], regs);
+	regs->uregs[pcreg] = autask->backup;
+}
+
+static void uprobe_write_pc(struct arch_uprobe *auprobe,
+			    struct arch_uprobe_task *autask,
+			    struct pt_regs *regs)
+{
+	u32 pcreg = auprobe->pcreg;
+
+	load_write_pc(regs->uregs[pcreg], regs);
+	regs->uregs[pcreg] = autask->backup;
+}
+
+enum kprobe_insn
+decode_pc_ro(kprobe_opcode_t insn, struct arch_specific_insn *asi, void *d)
+{
+	struct arch_uprobe *auprobe = container_of(asi, struct arch_uprobe, asi);
+	struct decode_emulate *decode = d;
+	u32 regs = decode->header.type_regs.bits >> DECODE_TYPE_BITS;
+	int reg;
+
+	reg = uprobes_substitute_pc(&auprobe->modinsn, regs);
+	if (reg == 15)
+		return INSN_GOOD;
+
+	if (reg == -1)
+		return INSN_REJECTED;
+
+	auprobe->pcreg = reg;
+	auprobe->prehandler = uprobe_set_pc;
+	auprobe->posthandler = uprobe_unset_pc;
+
+	return INSN_GOOD;
+}
+
+enum kprobe_insn
+decode_wb_pc(kprobe_opcode_t insn, struct arch_specific_insn *asi,
+	     void *d, bool alu)
+{
+	struct arch_uprobe *auprobe = container_of(asi, struct arch_uprobe, asi);
+	enum kprobe_insn ret = decode_pc_ro(insn, asi, d);
+
+	if (((insn >> 12) & 0xf) == 15)
+		auprobe->posthandler = alu ? uprobe_aluwrite_pc
+					   : uprobe_write_pc;
+
+	return ret;
+}
+
+enum kprobe_insn
+decode_rd12rn16rm0rs8_rwflags(kprobe_opcode_t insn,
+			      struct arch_specific_insn *asi, void *d)
+{
+	return decode_wb_pc(insn, asi, d, true);
+}
+
+enum kprobe_insn
+decode_ldr(kprobe_opcode_t insn, struct arch_specific_insn *asi, void *d)
+{
+	return decode_wb_pc(insn, asi, d, false);
+}
+
+enum kprobe_insn
+uprobe_decode_ldmstm(kprobe_opcode_t insn,
+		     struct arch_specific_insn *asi, void *d)
+{
+	struct arch_uprobe *auprobe = container_of(asi, struct arch_uprobe,
+						   asi);
+	unsigned reglist = insn & 0xffff;
+	int rn = (insn >> 16) & 0xf;
+	int lbit = insn & (1 << 20);
+	unsigned used = reglist | (1 << rn);
+
+	if (rn == 15)
+		return INSN_REJECTED;
+
+	if (!(used & (1 << 15)))
+		return INSN_GOOD;
+
+	if (used & (1 << 14))
+		return INSN_REJECTED;
+
+	/* Use LR instead of PC */
+	insn ^= 0xc000;
+
+	auprobe->pcreg = 14;
+	auprobe->modinsn = insn;
+
+	auprobe->prehandler = uprobe_set_pc;
+	if (lbit)
+		auprobe->posthandler = uprobe_write_pc;
+	else
+		auprobe->posthandler = uprobe_unset_pc;
+
+	return INSN_GOOD;
+}
diff --git a/arch/arm/kernel/uprobes.h b/arch/arm/kernel/uprobes.h
new file mode 100644
index 0000000..a1a4897
--- /dev/null
+++ b/arch/arm/kernel/uprobes.h
@@ -0,0 +1,23 @@
+#ifndef __ARM_KERNEL_UPROBES_H
+#define __ARM_KERNEL_UPROBES_H
+
+enum kprobe_insn uprobe_decode_ldmstm(kprobe_opcode_t insn,
+				      struct arch_specific_insn *asi,
+				      void *d);
+
+enum kprobe_insn decode_ldr(kprobe_opcode_t insn,
+			    struct arch_specific_insn *asi,
+			    void *d);
+
+enum kprobe_insn
+decode_rd12rn16rm0rs8_rwflags(kprobe_opcode_t insn,
+			      struct arch_specific_insn *asi, void *d);
+
+enum kprobe_insn
+decode_wb_pc(kprobe_opcode_t insn, struct arch_specific_insn *asi,
+	     void *d, bool alu);
+
+enum kprobe_insn
+decode_pc_ro(kprobe_opcode_t insn, struct arch_specific_insn *asi, void *d);
+
+#endif
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 7/9] uprobes: add arch write opcode hook
From: Rabin Vincent @ 2012-10-14 19:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1350242593-17761-1-git-send-email-rabin@rab.in>

Allow arches to write the opcode with a custom function.  ARM needs to
customize the swbp instruction depending on the condition code of the
instruction it replaces.

Signed-off-by: Rabin Vincent <rabin@rab.in>
---
 include/linux/uprobes.h |    3 +++
 kernel/events/uprobes.c |    8 +++++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/include/linux/uprobes.h b/include/linux/uprobes.h
index c3dc5de..35b9490 100644
--- a/include/linux/uprobes.h
+++ b/include/linux/uprobes.h
@@ -131,6 +131,9 @@ extern void arch_uprobe_abort_xol(struct arch_uprobe *aup, struct pt_regs *regs)
 extern bool __weak arch_uprobe_ignore(struct arch_uprobe *aup, struct pt_regs *regs);
 extern void __weak arch_uprobe_xol_copy(struct arch_uprobe *auprobe, void *vaddr);
 extern int __weak arch_uprobes_init(void);
+extern void __weak arch_uprobe_write_opcode(struct arch_uprobe *auprobe,
+					    void *vaddr,
+					    uprobe_opcode_t opcode);
 #else /* !CONFIG_UPROBES */
 struct uprobes_state {
 };
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 8c52f93..95ea618 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -203,6 +203,12 @@ bool __weak is_swbp_insn(uprobe_opcode_t *insn)
  * have fixed length instructions.
  */
 
+void __weak arch_uprobe_write_opcode(struct arch_uprobe *auprobe, void *vaddr,
+				     uprobe_opcode_t opcode)
+{
+	memcpy(vaddr, &opcode, UPROBE_SWBP_INSN_SIZE);
+}
+
 /*
  * write_opcode - write the opcode at a given virtual address.
  * @auprobe: arch breakpointing information.
@@ -242,7 +248,7 @@ retry:
 	vaddr_new = kmap_atomic(new_page);
 
 	memcpy(vaddr_new, vaddr_old, PAGE_SIZE);
-	memcpy(vaddr_new + (vaddr & ~PAGE_MASK), &opcode, UPROBE_SWBP_INSN_SIZE);
+	arch_uprobe_write_opcode(auprobe, vaddr_new + (vaddr & ~PAGE_MASK), opcode);
 
 	kunmap_atomic(vaddr_new);
 	kunmap_atomic(vaddr_old);
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 6/9] uprobes: flush cache after xol write
From: Rabin Vincent @ 2012-10-14 19:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1350242593-17761-1-git-send-email-rabin@rab.in>

Flush the cache so that the instructions written to the XOL area are
visible.

Signed-off-by: Rabin Vincent <rabin@rab.in>
---
 kernel/events/uprobes.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index ca000a9..8c52f93 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -1246,6 +1246,7 @@ static unsigned long xol_get_insn_slot(struct uprobe *uprobe, unsigned long slot
 	offset = current->utask->xol_vaddr & ~PAGE_MASK;
 	vaddr = kmap_atomic(area->page);
 	arch_uprobe_xol_copy(&uprobe->arch, vaddr + offset);
+	flush_dcache_page(area->page);
 	kunmap_atomic(vaddr);
 
 	return current->utask->xol_vaddr;
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 5/9] uprobes: allow arch-specific initialization
From: Rabin Vincent @ 2012-10-14 19:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1350242593-17761-1-git-send-email-rabin@rab.in>

Add a weak function for any architecture-specific initialization.  ARM
will use this to register the handlers for the undefined instructions it
uses to implement uprobes.

Signed-off-by: Rabin Vincent <rabin@rab.in>
---
 include/linux/uprobes.h |    1 +
 kernel/events/uprobes.c |   10 ++++++++++
 2 files changed, 11 insertions(+)

diff --git a/include/linux/uprobes.h b/include/linux/uprobes.h
index b4380ad..c3dc5de 100644
--- a/include/linux/uprobes.h
+++ b/include/linux/uprobes.h
@@ -130,6 +130,7 @@ extern int  arch_uprobe_exception_notify(struct notifier_block *self, unsigned l
 extern void arch_uprobe_abort_xol(struct arch_uprobe *aup, struct pt_regs *regs);
 extern bool __weak arch_uprobe_ignore(struct arch_uprobe *aup, struct pt_regs *regs);
 extern void __weak arch_uprobe_xol_copy(struct arch_uprobe *auprobe, void *vaddr);
+extern int __weak arch_uprobes_init(void);
 #else /* !CONFIG_UPROBES */
 struct uprobes_state {
 };
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index f7ff3a4..ca000a9 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -1634,8 +1634,14 @@ static struct notifier_block uprobe_exception_nb = {
 	.priority		= INT_MAX-1,	/* notified after kprobes, kgdb */
 };
 
+int __weak __init arch_uprobes_init(void)
+{
+	return 0;
+}
+
 static int __init init_uprobes(void)
 {
+	int ret;
 	int i;
 
 	for (i = 0; i < UPROBES_HASH_SZ; i++) {
@@ -1643,6 +1649,10 @@ static int __init init_uprobes(void)
 		mutex_init(&uprobes_mmap_mutex[i]);
 	}
 
+	ret = arch_uprobes_init();
+	if (ret)
+		return ret;
+
 	return register_die_notifier(&uprobe_exception_nb);
 }
 module_init(init_uprobes);
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 4/9] uprobes: allow arch access to xol slot
From: Rabin Vincent @ 2012-10-14 19:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1350242593-17761-1-git-send-email-rabin@rab.in>

Allow arches to customize how the instruction is filled into the xol
slot.  ARM will use this to insert an undefined instruction after the
real instruction in order to simulate a single step of the instruction
without hardware support.

Signed-off-by: Rabin Vincent <rabin@rab.in>
---
 include/linux/uprobes.h |    1 +
 kernel/events/uprobes.c |    7 ++++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/include/linux/uprobes.h b/include/linux/uprobes.h
index da21b66..b4380ad 100644
--- a/include/linux/uprobes.h
+++ b/include/linux/uprobes.h
@@ -129,6 +129,7 @@ extern bool arch_uprobe_xol_was_trapped(struct task_struct *tsk);
 extern int  arch_uprobe_exception_notify(struct notifier_block *self, unsigned long val, void *data);
 extern void arch_uprobe_abort_xol(struct arch_uprobe *aup, struct pt_regs *regs);
 extern bool __weak arch_uprobe_ignore(struct arch_uprobe *aup, struct pt_regs *regs);
+extern void __weak arch_uprobe_xol_copy(struct arch_uprobe *auprobe, void *vaddr);
 #else /* !CONFIG_UPROBES */
 struct uprobes_state {
 };
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index a0e1a38..f7ff3a4 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -1211,6 +1211,11 @@ static unsigned long xol_take_insn_slot(struct xol_area *area)
 	return slot_addr;
 }
 
+void __weak arch_uprobe_xol_copy(struct arch_uprobe *auprobe, void *vaddr)
+{
+	memcpy(vaddr, auprobe->insn, MAX_UINSN_BYTES);
+}
+
 /*
  * xol_get_insn_slot - If was not allocated a slot, then
  * allocate a slot.
@@ -1240,7 +1245,7 @@ static unsigned long xol_get_insn_slot(struct uprobe *uprobe, unsigned long slot
 	current->utask->vaddr = slot_addr;
 	offset = current->utask->xol_vaddr & ~PAGE_MASK;
 	vaddr = kmap_atomic(area->page);
-	memcpy(vaddr + offset, uprobe->arch.insn, MAX_UINSN_BYTES);
+	arch_uprobe_xol_copy(&uprobe->arch, vaddr + offset);
 	kunmap_atomic(vaddr);
 
 	return current->utask->xol_vaddr;
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 3/9] uprobes: allow ignoring of probe hits
From: Rabin Vincent @ 2012-10-14 19:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1350242593-17761-1-git-send-email-rabin@rab.in>

Allow arches to decided to ignore a probe hit.  ARM will use this to
only call handlers if the conditions to execute a conditionally executed
instruction are satisfied.

Signed-off-by: Rabin Vincent <rabin@rab.in>
---
 include/linux/uprobes.h |    1 +
 kernel/events/uprobes.c |   14 +++++++++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/include/linux/uprobes.h b/include/linux/uprobes.h
index ac90704..da21b66 100644
--- a/include/linux/uprobes.h
+++ b/include/linux/uprobes.h
@@ -128,6 +128,7 @@ extern int  arch_uprobe_post_xol(struct arch_uprobe *aup, struct pt_regs *regs);
 extern bool arch_uprobe_xol_was_trapped(struct task_struct *tsk);
 extern int  arch_uprobe_exception_notify(struct notifier_block *self, unsigned long val, void *data);
 extern void arch_uprobe_abort_xol(struct arch_uprobe *aup, struct pt_regs *regs);
+extern bool __weak arch_uprobe_ignore(struct arch_uprobe *aup, struct pt_regs *regs);
 #else /* !CONFIG_UPROBES */
 struct uprobes_state {
 };
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index db4e3ab..a0e1a38 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -1419,6 +1419,11 @@ static void mmf_recalc_uprobes(struct mm_struct *mm)
 	clear_bit(MMF_HAS_UPROBES, &mm->flags);
 }
 
+bool __weak arch_uprobe_ignore(struct arch_uprobe *aup, struct pt_regs *regs)
+{
+	return false;
+}
+
 static struct uprobe *find_active_uprobe(unsigned long bp_vaddr, int *is_swbp)
 {
 	struct mm_struct *mm = current->mm;
@@ -1469,6 +1474,7 @@ static void handle_swbp(struct pt_regs *regs)
 	struct uprobe *uprobe;
 	unsigned long bp_vaddr;
 	int uninitialized_var(is_swbp);
+	bool ignored = false;
 
 	bp_vaddr = uprobe_get_swbp_addr(regs);
 	uprobe = find_active_uprobe(bp_vaddr, &is_swbp);
@@ -1499,6 +1505,12 @@ static void handle_swbp(struct pt_regs *regs)
 			goto cleanup_ret;
 	}
 	utask->active_uprobe = uprobe;
+
+	if (arch_uprobe_ignore(&uprobe->arch, regs)) {
+		ignored = true;
+		goto cleanup_ret;
+	}
+
 	handler_chain(uprobe, regs);
 	if (uprobe->flags & UPROBE_SKIP_SSTEP && can_skip_sstep(uprobe, regs))
 		goto cleanup_ret;
@@ -1514,7 +1526,7 @@ cleanup_ret:
 		utask->active_uprobe = NULL;
 		utask->state = UTASK_RUNNING;
 	}
-	if (!(uprobe->flags & UPROBE_SKIP_SSTEP))
+	if (!ignored && !(uprobe->flags & UPROBE_SKIP_SSTEP))
 
 		/*
 		 * cannot singlestep; cannot skip instruction;
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 2/9] uprobes: check for single step support
From: Rabin Vincent @ 2012-10-14 19:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1350242593-17761-1-git-send-email-rabin@rab.in>

Check for single step support before calling user_enable_single_step(),
since user_enable_single_step() just BUG()s if support does not exist.
Needed by ARM.

Signed-off-by: Rabin Vincent <rabin@rab.in>
---
 kernel/events/uprobes.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 98256bc..db4e3ab 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -1450,7 +1450,8 @@ static struct uprobe *find_active_uprobe(unsigned long bp_vaddr, int *is_swbp)
 
 void __weak arch_uprobe_enable_step(struct arch_uprobe *arch)
 {
-	user_enable_single_step(current);
+	if (arch_has_single_step())
+		user_enable_single_step(current);
 }
 
 void __weak arch_uprobe_disable_step(struct arch_uprobe *arch)
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 1/9] uprobes: move function declarations out of arch
From: Rabin Vincent @ 2012-10-14 19:23 UTC (permalink / raw)
  To: linux-arm-kernel

It seems odd to keep the function declarations in the arch header where
they will need to be copy/pasted verbatim across arches.  Move them to
the common header.

Signed-off-by: Rabin Vincent <rabin@rab.in>
---
 arch/x86/include/asm/uprobes.h |    6 ------
 include/linux/uprobes.h        |    8 ++++++++
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/uprobes.h b/arch/x86/include/asm/uprobes.h
index 8ff8be7..b20b4d6 100644
--- a/arch/x86/include/asm/uprobes.h
+++ b/arch/x86/include/asm/uprobes.h
@@ -49,10 +49,4 @@ struct arch_uprobe_task {
 	unsigned int			saved_tf;
 };
 
-extern int  arch_uprobe_analyze_insn(struct arch_uprobe *aup, struct mm_struct *mm, unsigned long addr);
-extern int  arch_uprobe_pre_xol(struct arch_uprobe *aup, struct pt_regs *regs);
-extern int  arch_uprobe_post_xol(struct arch_uprobe *aup, struct pt_regs *regs);
-extern bool arch_uprobe_xol_was_trapped(struct task_struct *tsk);
-extern int  arch_uprobe_exception_notify(struct notifier_block *self, unsigned long val, void *data);
-extern void arch_uprobe_abort_xol(struct arch_uprobe *aup, struct pt_regs *regs);
 #endif	/* _ASM_UPROBES_H */
diff --git a/include/linux/uprobes.h b/include/linux/uprobes.h
index e6f0331..ac90704 100644
--- a/include/linux/uprobes.h
+++ b/include/linux/uprobes.h
@@ -30,6 +30,7 @@
 struct vm_area_struct;
 struct mm_struct;
 struct inode;
+struct notifier_block;
 
 #ifdef CONFIG_ARCH_SUPPORTS_UPROBES
 # include <asm/uprobes.h>
@@ -120,6 +121,13 @@ extern void uprobe_notify_resume(struct pt_regs *regs);
 extern bool uprobe_deny_signal(void);
 extern bool __weak arch_uprobe_skip_sstep(struct arch_uprobe *aup, struct pt_regs *regs);
 extern void uprobe_clear_state(struct mm_struct *mm);
+extern void uprobe_reset_state(struct mm_struct *mm);
+extern int  arch_uprobe_analyze_insn(struct arch_uprobe *aup, struct mm_struct *mm,unsigned long addr);
+extern int  arch_uprobe_pre_xol(struct arch_uprobe *aup, struct pt_regs *regs);
+extern int  arch_uprobe_post_xol(struct arch_uprobe *aup, struct pt_regs *regs);
+extern bool arch_uprobe_xol_was_trapped(struct task_struct *tsk);
+extern int  arch_uprobe_exception_notify(struct notifier_block *self, unsigned long val, void *data);
+extern void arch_uprobe_abort_xol(struct arch_uprobe *aup, struct pt_regs *regs);
 #else /* !CONFIG_UPROBES */
 struct uprobes_state {
 };
-- 
1.7.9.5

^ permalink raw reply related

* [revert request for commit 9fff2fa] Re: [git pull] signals pile 3
From: Al Viro @ 2012-10-14 19:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <507B02C1.1090101@gmail.com>

On Sun, Oct 14, 2012 at 08:21:53PM +0200, Daniel Mack wrote:
> On 14.10.2012 19:55, Al Viro wrote:
> > On Sun, Oct 14, 2012 at 06:26:40PM +0100, Al Viro wrote:
> >> On Sun, Oct 14, 2012 at 06:44:12PM +0200, Daniel Mack wrote:
> >>> On Oct 14, 2012 6:40 PM, "Al Viro" <viro@zeniv.linux.org.uk> wrote:
> >>>>
> >>>> On Sun, Oct 14, 2012 at 05:35:23PM +0200, Daniel Mack wrote:
> >>>>
> >>>>> I rebased my ARM development branch and figured that your patch 9fff2fa
> >>>>> ("arm: switch to saner kernel_execve() semantics") breaks the boot on my
> >>>>> board right after init is invoked via NFS:
> >>>>
> >>>> OK, revert it is, then.  Nothing in the tree has dependencies on that
> >>> sucker
> >>>> and while it survives testing here, it's obviously not ready for mainline.
> >>>> So, with abject apologies to everyone involved, please revert.
> >>>
> >>> Reverting it is not straight forward, and half of this patch doesn't seem
> >>> to cause issues.
> >>>
> >>> I can resend my patch with an S-o-b if you want me to.
> >>
> >> Um...  That's _really_ interesting.  First of all, revert is absolutely
> >> straightforward; the only change in Kconfig is "remove the damn select"
> >> and it's not hard to resolve.  But I actually wonder what the hell is
> >> going on with that breakage - the *only* thing your revert changes is
> >> that instead of letting the kernel_thread callback return all the way
> >> to returning 0 to ret_from_kernel_thread() on do_execve() success you
> >> have it do ret_from_kernel_execve() instead.  Hmm...
> >>
> >> Could you try to print current_pt_regs()->ARM_r0 in kernel_execve() before
> >> calling ret_from_kernel_execve() with your patch applied?  If that somehow
> >> got non-zero, we'd see trouble, all right, but I don't see any places where
> >> it could.
> >>
> >> Wait a minute...  I think I see what might be going on, but I don't
> >> understand it at all.  Look: arm start_thread() is
> >> #define start_thread(regs,pc,sp)                                        \
> >> ({                                                                      \
> >>         unsigned long *stack = (unsigned long *)sp;                     \
> >>         memset(regs->uregs, 0, sizeof(regs->uregs));                    \
> >>         if (current->personality & ADDR_LIMIT_32BIT)                    \
> >>                 regs->ARM_cpsr = USR_MODE;                              \
> >>         else                                                            \
> >>                 regs->ARM_cpsr = USR26_MODE;                            \
> >>         if (elf_hwcap & HWCAP_THUMB && pc & 1)                          \
> >>                 regs->ARM_cpsr |= PSR_T_BIT;                            \
> >>         regs->ARM_cpsr |= PSR_ENDSTATE;                                 \
> >>         regs->ARM_pc = pc & ~1;         /* pc */                        \
> >>         regs->ARM_sp = sp;              /* sp */                        \
> >>         regs->ARM_r2 = stack[2];        /* r2 (envp) */                 \
> >>         regs->ARM_r1 = stack[1];        /* r1 (argv) */                 \
> >>         regs->ARM_r0 = stack[0];        /* r0 (argc) */                 \
> >>         nommu_start_thread(regs);                                       \
> >> })
> >> and the last 3 make no sense whatsoever.  Note that on normal execve() we'll
> >> be going through the syscall return, so the userland will see 0 in there,
> >> no matter what do we do here.  Theoretically, it might've been done for
> >> ptrace sake (it will be able to observe the values in those registers before
> >> the tracee reaches userland), but there's another oddity involved - "stack"
> >> is a userland pointer here.  Granted, it's been recently written to, so
> >> we are not likely to hit a pagefault here, but...  What happens if we are
> >> under enough memory pressure to swap those pages out?  PF in the kernel
> >> mode with no exception table entries for those insns?
> > 
> > FWIW, if you don't mind an experiment, try to take mainline (with that
> > commit not reverted) and add
> > 	strne	r0, [sp, #S_R0]
> > right before
> > 	get_thread_info tsk
> > in ret_from_fork().  And see if that changes behaviour.
> > 
> 
> I don't mind experiments at all :)
> 
> However, with that extra line in place as described, I'm still getting
> the Oops below. If you want me to test anything else, please let me know.
> 
> 
> [    4.683182] VFS: Mounted root (nfs filesystem) on device 0:12.
> [    4.742007] devtmpfs: mounted
> [    4.745746] Freeing init memory: 172K
> [    5.038724] Internal error: Oops - undefined instruction: 0 [#1] SMP
> THUMB2
> [    5.046044] Modules linked in:
> [    5.049263] CPU: 0    Not tainted  (3.6.0-11053-g56c8535-dirty #136)
> [    5.055925] PC is at cpsw_probe+0x422/0x9ac
> [    5.060314] LR is at trace_hardirqs_on_caller+0x8f/0xfc
> [    5.065790] pc : [<c03493de>]    lr : [<c005e81f>]    psr: 60000113
> [    5.065790] sp : cf055fb0  ip : 00000000  fp : 00000000
> [    5.077800] r10: 00000000  r9 : 00000000  r8 : 00000000
> [    5.083270] r7 : 00000000  r6 : 00000000  r5 : c034458d  r4 : 00000000
> [    5.090101] r3 : cf057a40  r2 : 00000000  r1 : 00000001  r0 : 00000000
> [    5.096936] Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM
> Segment user
> [    5.104406] Control: 50c5387d  Table: 8f434019  DAC: 00000015
> [    5.110422] Process init (pid: 1, stack limit = 0xcf054240)
> [    5.116257] Stack: (0xcf055fb0 to 0xcf056000)
> [    5.120824] 5fa0:                                     00000001
> 00000000 00000000 00000000
> [    5.129390] 5fc0: cf055fb0 c000d1a8 00000000 00000000 00000000
> 00000000 00000000 00000000
> [    5.137957] 5fe0: 00000000 becedf10 00000000 b6f81dd0 00000010
> 00000000 aaaabfaf a8babbaa
> [    5.146529] Code: 2206a010 718ef508 0184f8da f8b1f65d (3070f8d8)
> [    5.152915] ---[ end trace 7362bbe8e73e6b07 ]---
> [    5.158324] Kernel panic - not syncing: Attempted to kill init!
> exitcode=0x0000000b
> [    5.158324]

Very interesting...  So we have kernel_thread() payload called and we have
it reach kernel_execve() (otherwise your reverting kernel_execve() change
would've had no effect).  Said payload returns, and sp value seems to be
sane.  Buggered return address, perhaps?  But that would be killing the
damn thing everywhere...

Just in case - print __builtin_return_address(0) in the beginning of
kernel_init(); it ought to point at the end of ret_from_fork...

And kill that strne along with assignment to ->ARM_r0 in start_thread().
I've missed the obvious problem with strne - flag values won't survive the
call of payload ;-/  IOW, it's still possible that we are getting bitten
by strange value left in there.  Removing the assignment in start_thread()
would check that possiblity...

^ permalink raw reply

* [revert request for commit 9fff2fa] Re: [git pull] signals pile 3
From: Daniel Mack @ 2012-10-14 18:21 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20121014175541.GX2616@ZenIV.linux.org.uk>

On 14.10.2012 19:55, Al Viro wrote:
> On Sun, Oct 14, 2012 at 06:26:40PM +0100, Al Viro wrote:
>> On Sun, Oct 14, 2012 at 06:44:12PM +0200, Daniel Mack wrote:
>>> On Oct 14, 2012 6:40 PM, "Al Viro" <viro@zeniv.linux.org.uk> wrote:
>>>>
>>>> On Sun, Oct 14, 2012 at 05:35:23PM +0200, Daniel Mack wrote:
>>>>
>>>>> I rebased my ARM development branch and figured that your patch 9fff2fa
>>>>> ("arm: switch to saner kernel_execve() semantics") breaks the boot on my
>>>>> board right after init is invoked via NFS:
>>>>
>>>> OK, revert it is, then.  Nothing in the tree has dependencies on that
>>> sucker
>>>> and while it survives testing here, it's obviously not ready for mainline.
>>>> So, with abject apologies to everyone involved, please revert.
>>>
>>> Reverting it is not straight forward, and half of this patch doesn't seem
>>> to cause issues.
>>>
>>> I can resend my patch with an S-o-b if you want me to.
>>
>> Um...  That's _really_ interesting.  First of all, revert is absolutely
>> straightforward; the only change in Kconfig is "remove the damn select"
>> and it's not hard to resolve.  But I actually wonder what the hell is
>> going on with that breakage - the *only* thing your revert changes is
>> that instead of letting the kernel_thread callback return all the way
>> to returning 0 to ret_from_kernel_thread() on do_execve() success you
>> have it do ret_from_kernel_execve() instead.  Hmm...
>>
>> Could you try to print current_pt_regs()->ARM_r0 in kernel_execve() before
>> calling ret_from_kernel_execve() with your patch applied?  If that somehow
>> got non-zero, we'd see trouble, all right, but I don't see any places where
>> it could.
>>
>> Wait a minute...  I think I see what might be going on, but I don't
>> understand it at all.  Look: arm start_thread() is
>> #define start_thread(regs,pc,sp)                                        \
>> ({                                                                      \
>>         unsigned long *stack = (unsigned long *)sp;                     \
>>         memset(regs->uregs, 0, sizeof(regs->uregs));                    \
>>         if (current->personality & ADDR_LIMIT_32BIT)                    \
>>                 regs->ARM_cpsr = USR_MODE;                              \
>>         else                                                            \
>>                 regs->ARM_cpsr = USR26_MODE;                            \
>>         if (elf_hwcap & HWCAP_THUMB && pc & 1)                          \
>>                 regs->ARM_cpsr |= PSR_T_BIT;                            \
>>         regs->ARM_cpsr |= PSR_ENDSTATE;                                 \
>>         regs->ARM_pc = pc & ~1;         /* pc */                        \
>>         regs->ARM_sp = sp;              /* sp */                        \
>>         regs->ARM_r2 = stack[2];        /* r2 (envp) */                 \
>>         regs->ARM_r1 = stack[1];        /* r1 (argv) */                 \
>>         regs->ARM_r0 = stack[0];        /* r0 (argc) */                 \
>>         nommu_start_thread(regs);                                       \
>> })
>> and the last 3 make no sense whatsoever.  Note that on normal execve() we'll
>> be going through the syscall return, so the userland will see 0 in there,
>> no matter what do we do here.  Theoretically, it might've been done for
>> ptrace sake (it will be able to observe the values in those registers before
>> the tracee reaches userland), but there's another oddity involved - "stack"
>> is a userland pointer here.  Granted, it's been recently written to, so
>> we are not likely to hit a pagefault here, but...  What happens if we are
>> under enough memory pressure to swap those pages out?  PF in the kernel
>> mode with no exception table entries for those insns?
> 
> FWIW, if you don't mind an experiment, try to take mainline (with that
> commit not reverted) and add
> 	strne	r0, [sp, #S_R0]
> right before
> 	get_thread_info tsk
> in ret_from_fork().  And see if that changes behaviour.
> 

I don't mind experiments at all :)

However, with that extra line in place as described, I'm still getting
the Oops below. If you want me to test anything else, please let me know.


[    4.683182] VFS: Mounted root (nfs filesystem) on device 0:12.
[    4.742007] devtmpfs: mounted
[    4.745746] Freeing init memory: 172K
[    5.038724] Internal error: Oops - undefined instruction: 0 [#1] SMP
THUMB2
[    5.046044] Modules linked in:
[    5.049263] CPU: 0    Not tainted  (3.6.0-11053-g56c8535-dirty #136)
[    5.055925] PC is at cpsw_probe+0x422/0x9ac
[    5.060314] LR is at trace_hardirqs_on_caller+0x8f/0xfc
[    5.065790] pc : [<c03493de>]    lr : [<c005e81f>]    psr: 60000113
[    5.065790] sp : cf055fb0  ip : 00000000  fp : 00000000
[    5.077800] r10: 00000000  r9 : 00000000  r8 : 00000000
[    5.083270] r7 : 00000000  r6 : 00000000  r5 : c034458d  r4 : 00000000
[    5.090101] r3 : cf057a40  r2 : 00000000  r1 : 00000001  r0 : 00000000
[    5.096936] Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM
Segment user
[    5.104406] Control: 50c5387d  Table: 8f434019  DAC: 00000015
[    5.110422] Process init (pid: 1, stack limit = 0xcf054240)
[    5.116257] Stack: (0xcf055fb0 to 0xcf056000)
[    5.120824] 5fa0:                                     00000001
00000000 00000000 00000000
[    5.129390] 5fc0: cf055fb0 c000d1a8 00000000 00000000 00000000
00000000 00000000 00000000
[    5.137957] 5fe0: 00000000 becedf10 00000000 b6f81dd0 00000010
00000000 aaaabfaf a8babbaa
[    5.146529] Code: 2206a010 718ef508 0184f8da f8b1f65d (3070f8d8)
[    5.152915] ---[ end trace 7362bbe8e73e6b07 ]---
[    5.158324] Kernel panic - not syncing: Attempted to kill init!
exitcode=0x0000000b
[    5.158324]

^ permalink raw reply

* [PATCH] lib: decompress_unlzo: Change include file name
From: Fabio Estevam @ 2012-10-14 18:13 UTC (permalink / raw)
  To: linux-arm-kernel

From: Fabio Estevam <fabio.estevam@freescale.com>

Commit 7c979cebc (lib/lzo: Rename lzo1x_decompress.c to lzo1x_decompress_safe.c) 
renamed lzo1x_decompress.c file, but missed to update lib/decompress_unlzo.c,
which breaks imx_v6_v7_defconfig build:

arch/arm/boot/compressed/../../../../lib/decompress_unlzo.c:34:34: fatal error: lzo/lzo1x_decompress.c: No such file or directory

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
 lib/decompress_unlzo.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/decompress_unlzo.c b/lib/decompress_unlzo.c
index 4531294..960183d 100644
--- a/lib/decompress_unlzo.c
+++ b/lib/decompress_unlzo.c
@@ -31,7 +31,7 @@
  */
 
 #ifdef STATIC
-#include "lzo/lzo1x_decompress.c"
+#include "lzo/lzo1x_decompress_safe.c"
 #else
 #include <linux/decompress/unlzo.h>
 #endif
-- 
1.7.9.5

^ permalink raw reply related

* [revert request for commit 9fff2fa] Re: [git pull] signals pile 3
From: Al Viro @ 2012-10-14 17:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20121014172640.GW2616@ZenIV.linux.org.uk>

On Sun, Oct 14, 2012 at 06:26:40PM +0100, Al Viro wrote:
> On Sun, Oct 14, 2012 at 06:44:12PM +0200, Daniel Mack wrote:
> > On Oct 14, 2012 6:40 PM, "Al Viro" <viro@zeniv.linux.org.uk> wrote:
> > >
> > > On Sun, Oct 14, 2012 at 05:35:23PM +0200, Daniel Mack wrote:
> > >
> > > > I rebased my ARM development branch and figured that your patch 9fff2fa
> > > > ("arm: switch to saner kernel_execve() semantics") breaks the boot on my
> > > > board right after init is invoked via NFS:
> > >
> > > OK, revert it is, then.  Nothing in the tree has dependencies on that
> > sucker
> > > and while it survives testing here, it's obviously not ready for mainline.
> > > So, with abject apologies to everyone involved, please revert.
> > 
> > Reverting it is not straight forward, and half of this patch doesn't seem
> > to cause issues.
> > 
> > I can resend my patch with an S-o-b if you want me to.
> 
> Um...  That's _really_ interesting.  First of all, revert is absolutely
> straightforward; the only change in Kconfig is "remove the damn select"
> and it's not hard to resolve.  But I actually wonder what the hell is
> going on with that breakage - the *only* thing your revert changes is
> that instead of letting the kernel_thread callback return all the way
> to returning 0 to ret_from_kernel_thread() on do_execve() success you
> have it do ret_from_kernel_execve() instead.  Hmm...
> 
> Could you try to print current_pt_regs()->ARM_r0 in kernel_execve() before
> calling ret_from_kernel_execve() with your patch applied?  If that somehow
> got non-zero, we'd see trouble, all right, but I don't see any places where
> it could.
> 
> Wait a minute...  I think I see what might be going on, but I don't
> understand it at all.  Look: arm start_thread() is
> #define start_thread(regs,pc,sp)                                        \
> ({                                                                      \
>         unsigned long *stack = (unsigned long *)sp;                     \
>         memset(regs->uregs, 0, sizeof(regs->uregs));                    \
>         if (current->personality & ADDR_LIMIT_32BIT)                    \
>                 regs->ARM_cpsr = USR_MODE;                              \
>         else                                                            \
>                 regs->ARM_cpsr = USR26_MODE;                            \
>         if (elf_hwcap & HWCAP_THUMB && pc & 1)                          \
>                 regs->ARM_cpsr |= PSR_T_BIT;                            \
>         regs->ARM_cpsr |= PSR_ENDSTATE;                                 \
>         regs->ARM_pc = pc & ~1;         /* pc */                        \
>         regs->ARM_sp = sp;              /* sp */                        \
>         regs->ARM_r2 = stack[2];        /* r2 (envp) */                 \
>         regs->ARM_r1 = stack[1];        /* r1 (argv) */                 \
>         regs->ARM_r0 = stack[0];        /* r0 (argc) */                 \
>         nommu_start_thread(regs);                                       \
> })
> and the last 3 make no sense whatsoever.  Note that on normal execve() we'll
> be going through the syscall return, so the userland will see 0 in there,
> no matter what do we do here.  Theoretically, it might've been done for
> ptrace sake (it will be able to observe the values in those registers before
> the tracee reaches userland), but there's another oddity involved - "stack"
> is a userland pointer here.  Granted, it's been recently written to, so
> we are not likely to hit a pagefault here, but...  What happens if we are
> under enough memory pressure to swap those pages out?  PF in the kernel
> mode with no exception table entries for those insns?

FWIW, if you don't mind an experiment, try to take mainline (with that
commit not reverted) and add
	strne	r0, [sp, #S_R0]
right before
	get_thread_info tsk
in ret_from_fork().  And see if that changes behaviour.

^ permalink raw reply

* [RFC PATCH v2] prevent top pte being overwritten before flushing
From: Andrew Yan-Pai Chen @ 2012-10-14 17:42 UTC (permalink / raw)
  To: linux-arm-kernel

From: Yan-Pai Chen <ypchen@faraday-tech.com>

Since flush_pfn_alias() is preemptible, it is possible to be
preempted just after set_top_pte() is done. If the process
which preempts the previous happened to invoke flush_pfn_alias()
with the same colour vaddr as that of the previous, the same
top pte will be overwritten. When switching back to the previous,
it attempts to flush cache lines with incorrect mapping. Then
no lines (or wrong lines) will be flushed because of the nature
of vipt caches.

flush_icache_alias() has the same problem as well. However, as it
could be called in SMP setups, we prevent concurrent overwrites of
top pte by having a lock on it.

Signed-off-by: JasonLin <wwlin@faraday-tech.com>
Signed-off-by: Yan-Pai Chen <ypchen@faraday-tech.com>
---
 arch/arm/mm/flush.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mm/flush.c b/arch/arm/mm/flush.c
index 40ca11e..b6510f4 100644
--- a/arch/arm/mm/flush.c
+++ b/arch/arm/mm/flush.c
@@ -11,6 +11,7 @@
 #include <linux/mm.h>
 #include <linux/pagemap.h>
 #include <linux/highmem.h>
+#include <linux/spinlock.h>
 
 #include <asm/cacheflush.h>
 #include <asm/cachetype.h>
@@ -22,11 +23,15 @@
 
 #ifdef CONFIG_CPU_CACHE_VIPT
 
+static DEFINE_RAW_SPINLOCK(flush_lock);
+
+/* Beware that this function is not to be called for SMP setups. */
 static void flush_pfn_alias(unsigned long pfn, unsigned long vaddr)
 {
 	unsigned long to = FLUSH_ALIAS_START + (CACHE_COLOUR(vaddr) << PAGE_SHIFT);
 	const int zero = 0;
 
+	preempt_disable();
 	set_top_pte(to, pfn_pte(pfn, PAGE_KERNEL));
 
 	asm(	"mcrr	p15, 0, %1, %0, c14\n"
@@ -34,6 +39,8 @@ static void flush_pfn_alias(unsigned long pfn, unsigned long vaddr)
 	    :
 	    : "r" (to), "r" (to + PAGE_SIZE - L1_CACHE_BYTES), "r" (zero)
 	    : "cc");
+
+	preempt_enable();
 }
 
 static void flush_icache_alias(unsigned long pfn, unsigned long vaddr, unsigned long len)
@@ -42,9 +49,13 @@ static void flush_icache_alias(unsigned long pfn, unsigned long vaddr, unsigned
 	unsigned long offset = vaddr & (PAGE_SIZE - 1);
 	unsigned long to;
 
+	raw_spin_lock(&flush_lock);
+
 	set_top_pte(va, pfn_pte(pfn, PAGE_KERNEL));
 	to = va + offset;
 	flush_icache_range(to, to + len);
+
+	raw_spin_unlock(&flush_lock);
 }
 
 void flush_cache_mm(struct mm_struct *mm)
-- 
1.7.4.1

^ permalink raw reply related

* [revert request for commit 9fff2fa] Re: [git pull] signals pile 3
From: Al Viro @ 2012-10-14 17:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CACTFLAOpcLtHNx-TCHQkUOPNAkTO8EgDhbyZSGQ9ag+Wu_cNRw@mail.gmail.com>

On Sun, Oct 14, 2012 at 06:44:12PM +0200, Daniel Mack wrote:
> On Oct 14, 2012 6:40 PM, "Al Viro" <viro@zeniv.linux.org.uk> wrote:
> >
> > On Sun, Oct 14, 2012 at 05:35:23PM +0200, Daniel Mack wrote:
> >
> > > I rebased my ARM development branch and figured that your patch 9fff2fa
> > > ("arm: switch to saner kernel_execve() semantics") breaks the boot on my
> > > board right after init is invoked via NFS:
> >
> > OK, revert it is, then.  Nothing in the tree has dependencies on that
> sucker
> > and while it survives testing here, it's obviously not ready for mainline.
> > So, with abject apologies to everyone involved, please revert.
> 
> Reverting it is not straight forward, and half of this patch doesn't seem
> to cause issues.
> 
> I can resend my patch with an S-o-b if you want me to.

Um...  That's _really_ interesting.  First of all, revert is absolutely
straightforward; the only change in Kconfig is "remove the damn select"
and it's not hard to resolve.  But I actually wonder what the hell is
going on with that breakage - the *only* thing your revert changes is
that instead of letting the kernel_thread callback return all the way
to returning 0 to ret_from_kernel_thread() on do_execve() success you
have it do ret_from_kernel_execve() instead.  Hmm...

Could you try to print current_pt_regs()->ARM_r0 in kernel_execve() before
calling ret_from_kernel_execve() with your patch applied?  If that somehow
got non-zero, we'd see trouble, all right, but I don't see any places where
it could.

Wait a minute...  I think I see what might be going on, but I don't
understand it at all.  Look: arm start_thread() is
#define start_thread(regs,pc,sp)                                        \
({                                                                      \
        unsigned long *stack = (unsigned long *)sp;                     \
        memset(regs->uregs, 0, sizeof(regs->uregs));                    \
        if (current->personality & ADDR_LIMIT_32BIT)                    \
                regs->ARM_cpsr = USR_MODE;                              \
        else                                                            \
                regs->ARM_cpsr = USR26_MODE;                            \
        if (elf_hwcap & HWCAP_THUMB && pc & 1)                          \
                regs->ARM_cpsr |= PSR_T_BIT;                            \
        regs->ARM_cpsr |= PSR_ENDSTATE;                                 \
        regs->ARM_pc = pc & ~1;         /* pc */                        \
        regs->ARM_sp = sp;              /* sp */                        \
        regs->ARM_r2 = stack[2];        /* r2 (envp) */                 \
        regs->ARM_r1 = stack[1];        /* r1 (argv) */                 \
        regs->ARM_r0 = stack[0];        /* r0 (argc) */                 \
        nommu_start_thread(regs);                                       \
})
and the last 3 make no sense whatsoever.  Note that on normal execve() we'll
be going through the syscall return, so the userland will see 0 in there,
no matter what do we do here.  Theoretically, it might've been done for
ptrace sake (it will be able to observe the values in those registers before
the tracee reaches userland), but there's another oddity involved - "stack"
is a userland pointer here.  Granted, it's been recently written to, so
we are not likely to hit a pagefault here, but...  What happens if we are
under enough memory pressure to swap those pages out?  PF in the kernel
mode with no exception table entries for those insns?

^ permalink raw reply

* [revert request for commit 9fff2fa] Re: [git pull] signals pile 3
From: Daniel Mack @ 2012-10-14 16:44 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20121014164020.GV2616@ZenIV.linux.org.uk>

On Oct 14, 2012 6:40 PM, "Al Viro" <viro@zeniv.linux.org.uk> wrote:
>
> On Sun, Oct 14, 2012 at 05:35:23PM +0200, Daniel Mack wrote:
>
> > I rebased my ARM development branch and figured that your patch 9fff2fa
> > ("arm: switch to saner kernel_execve() semantics") breaks the boot on my
> > board right after init is invoked via NFS:
>
> OK, revert it is, then.  Nothing in the tree has dependencies on that
sucker
> and while it survives testing here, it's obviously not ready for mainline.
> So, with abject apologies to everyone involved, please revert.

Reverting it is not straight forward, and half of this patch doesn't seem
to cause issues.

I can resend my patch with an S-o-b if you want me to.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121014/0fc4e6e4/attachment-0001.html>

^ permalink raw reply

* [revert request for commit 9fff2fa] Re: [git pull] signals pile 3
From: Al Viro @ 2012-10-14 16:40 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <507ADBBB.9090209@gmail.com>

On Sun, Oct 14, 2012 at 05:35:23PM +0200, Daniel Mack wrote:

> I rebased my ARM development branch and figured that your patch 9fff2fa
> ("arm: switch to saner kernel_execve() semantics") breaks the boot on my
> board right after init is invoked via NFS:

OK, revert it is, then.  Nothing in the tree has dependencies on that sucker
and while it survives testing here, it's obviously not ready for mainline.
So, with abject apologies to everyone involved, please revert.

^ permalink raw reply

* [PATCH 3/4] ARM: AT91: Add AT91RM9200 support to DT board
From: Joachim Eastwood @ 2012-10-14 16:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20121014145407.GN12801@game.jcrosoft.org>

On Sun, Oct 14, 2012 at 4:54 PM, Jean-Christophe PLAGNIOL-VILLARD
<plagnioj@jcrosoft.com> wrote:
> On 19:08 Fri 12 Oct     , Joachim Eastwood wrote:
>> On Fri, Oct 12, 2012 at 6:27 PM, Jean-Christophe PLAGNIOL-VILLARD
>> <plagnioj@jcrosoft.com> wrote:
>> > On 17:28 Fri 12 Oct     , ludovic.desroches wrote:
>> >> Le 10/12/2012 04:22 PM, Jean-Christophe PLAGNIOL-VILLARD a ?crit :
>> >> >On 00:05 Fri 12 Oct     , Joachim Eastwood wrote:
>> >> >>Signed-off-by: Joachim Eastwood <manabian@gmail.com>
>> >> >>---
>> >> >>
>> >> >>Hi,
>> >> >>
>> >> >>This patch has some potential issues.
>> >> >>Before this patch board-dt would fail building when only AT91RM9200 was enabled because at91sam926x_timer symbol would be missing. This patch uses the at91rm9200_timer which
>> >> >>will fail if AT91RM9200 is not enabled.
>> >> >this need work with ot wtihout rm9200
>>
>> btw, to solve the build issue with board-dt in mainline now we need to
>> add a select CONFIG_SOC_AT91SAM9 to config MACH_AT91SAM_DT.
>>
>> >> >>
>> >> >>Any thoughts on solving this? As mention above this bug exists in mainline now.
>> >> >duplicate the board-dt with one for rm9200 only
>> >> >as rm9200 ans sam9 are 2 distict familly
>> >>
>> >> Why not adding a new machine descriptor for rm9200 in order to
>> >> prevent file duplication?
>> > because the soc are different and can only be compile if the timer is enable
>> > and I do not want to enable the rm9200 timer on sam9 so instead of a ifdef i
>> > the board-dt create a new board is better as we have a 50 lines file
>> >
>> > with different board_compat and different machine descriptor
>>
>> I am okey with either approach, but I would like to hear what Nicolas
>> Ferre has to say since he is the on the one that added board-dt. It
>> would be nice to have everything in one board DT file, but I
>> understand your concern with the RM9200 timer.
>>
>> We will also bump into this again on AT91X40 I guess.
> simple on x40 forget about it the x40 is no MMU only SoC
>
> so you can not enable  it by default as the all other at91 are use with MMU
>
> I did the necessary to make the board-dt nearly empty and the same for all the
> sam9 but the board-dt is sam9 only and need to be keeped this way
>
> Nico will tell you the same

okay. I'll make the necessary changes to the patch set.

I'll also include a fix for board-dt (select CONFIG_SOC_AT91SAM9) to
stop the build failure when it's enabled on a RM9200 only config.

Secondly I'll rebase on linux-next which includes your at91-pinctrl stuff.

regards
Joachim Eastwood


> Best Regards,
> J.

^ permalink raw reply

* [RFC] Common clock framework for external clock generators
From: Sebastian Hesselbarth @ 2012-10-14 16:16 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <507A9E72.2030800@gmail.com>

On 10/14/2012 01:13 PM, Daniel Mack wrote:
> I think the sub-node style above it nicer because it allows referencing
> the individual clocks outputs with a phandle. We use this chip to
> generate base-frequencies for audio clocks, and so we have to switch
> between two freqs for the multiples of 22.5KHz and 24KHz at runtime.

Both examples allow you to have a phandle for all individual clock-outputs.
The examples weren't complete but with the sub-node style you'll reference
with e.g. <&clkout0> while the flat one will use <&si5351 0>. I still prefer
the flat-style as it will not allow to have a phandle of plls.

Sebastian

^ permalink raw reply

* [git pull] signals pile 3
From: Daniel Mack @ 2012-10-14 15:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20121013005334.GM2616@ZenIV.linux.org.uk>

Hi Al,

On 13.10.2012 02:53, Al Viro wrote:
> The last bits of infrastructure for kernel_thread() et.al., with alpha/arm/x86
> use of those.  Plus sanitizing the asm glue and do_notify_resume() on alpha,
> fixing the "disabled irq while running task_work stuff" breakage there.
> 
> At that point the rest of kernel_thread/kernel_execve/sys_execve work can
> be done independently for different architectures.  The only pending bits
> that do depend on having all architectures converted are restrictred to
> fs/* and kernel/* - that'll obviously have to wait for the next cycle.
> I thought we'd have to wait for all of them done before we start eliminating
> the longjump-style insanity in kernel_execve(), but it turned out there's
> a very simple way to do that without flagday-style changes.
> 
> Please, pull from
> git://git.kernel.org/pub/scm/linux/kernel/git/viro/signal for-linus
> 
> Shortlog:
> Al Viro (12):

[...]

>       arm: switch to saner kernel_execve() semantics

[...]

> Diffstat:

[...]

>  arch/arm/include/asm/unistd.h              |    1 -
>  arch/arm/kernel/entry-common.S             |   29 +--------
>  arch/arm/kernel/process.c                  |    5 +-

I rebased my ARM development branch and figured that your patch 9fff2fa
("arm: switch to saner kernel_execve() semantics") breaks the boot on my
board right after init is invoked via NFS:

[    4.682072] VFS: Mounted root (nfs filesystem) on device 0:12.
[    4.690744] devtmpfs: mounted
[    4.694395] Freeing init memory: 172K
[    5.291417] Internal error: Oops - undefined instruction: 0 [#1] SMP
THUMB2
[    5.298734] Modules linked in:
[    5.301952] CPU: 0    Not tainted  (3.6.0-11053-g56c8535 #128)
[    5.308071] PC is at cpsw_probe+0x422/0x9ac
[    5.312459] LR is at trace_hardirqs_on_caller+0x8f/0xfc
[    5.317934] pc : [<c03493de>]    lr : [<c005e81f>]    psr: 60000113
[    5.317934] sp : cf055fb0  ip : 00000000  fp : 00000000
[    5.329944] r10: 00000000  r9 : 00000000  r8 : 00000000
[    5.335413] r7 : 00000000  r6 : 00000000  r5 : c034458d  r4 : 00000000
[    5.342244] r3 : cf057a40  r2 : 00000000  r1 : 00000001  r0 : 00000000
[    5.349078] Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM
Segment user
[    5.356546] Control: 50c5387d  Table: 8f434019  DAC: 00000015
[    5.362562] Process init (pid: 1, stack limit = 0xcf054240)
[    5.368395] Stack: (0xcf055fb0 to 0xcf056000)
[    5.372961] 5fa0:                                     00000001
00000000 00000000 00000000
[    5.381525] 5fc0: cf055fb0 c000d1a8 00000000 00000000 00000000
00000000 00000000 00000000
[    5.390091] 5fe0: 00000000 bee83f10 00000000 b6fdedd0 00000010
00000000 aaaabfaf a8babbaa
[    5.398664] Code: 2206a010 718ef508 0184f8da f8b1f65d (3070f8d8)
[    5.405049] ---[ end trace f92e44d0ab15d037 ]---
[    5.410424] Kernel panic - not syncing: Attempted to kill init!
exitcode=0x0000000b


Looking at the patch, I see it does two things:

 a) kill the special treatment formerly done by ret_from_kernel_thread
 b) switch over to generic execve for ARM

I know too little about the internals here, but reverting the latter
part fixes the boot for me. Find attached the patch I used locally for
that, on top of Linus' current merge tree (4d7127d).

FWIW, the config I'm using is here: http://pastebin.com/JPwAYmrD

I can test patches if you have any for me.


Thanks,
Daniel

-------------- next part --------------
A non-text attachment was scrubbed...
Name: arm-execve-revert.diff
Type: text/x-patch
Size: 1511 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121014/bbce05db/attachment.bin>

^ permalink raw reply

* [PATCH 3/4] ARM: AT91: Add AT91RM9200 support to DT board
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-10-14 14:54 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAGhQ9VzOQ2V6Cn5tY1REcGzpjjs53Lu6tT7=fNKqJQkBhR8HDA@mail.gmail.com>

On 19:08 Fri 12 Oct     , Joachim Eastwood wrote:
> On Fri, Oct 12, 2012 at 6:27 PM, Jean-Christophe PLAGNIOL-VILLARD
> <plagnioj@jcrosoft.com> wrote:
> > On 17:28 Fri 12 Oct     , ludovic.desroches wrote:
> >> Le 10/12/2012 04:22 PM, Jean-Christophe PLAGNIOL-VILLARD a ?crit :
> >> >On 00:05 Fri 12 Oct     , Joachim Eastwood wrote:
> >> >>Signed-off-by: Joachim Eastwood <manabian@gmail.com>
> >> >>---
> >> >>
> >> >>Hi,
> >> >>
> >> >>This patch has some potential issues.
> >> >>Before this patch board-dt would fail building when only AT91RM9200 was enabled because at91sam926x_timer symbol would be missing. This patch uses the at91rm9200_timer which
> >> >>will fail if AT91RM9200 is not enabled.
> >> >this need work with ot wtihout rm9200
> 
> btw, to solve the build issue with board-dt in mainline now we need to
> add a select CONFIG_SOC_AT91SAM9 to config MACH_AT91SAM_DT.
> 
> >> >>
> >> >>Any thoughts on solving this? As mention above this bug exists in mainline now.
> >> >duplicate the board-dt with one for rm9200 only
> >> >as rm9200 ans sam9 are 2 distict familly
> >>
> >> Why not adding a new machine descriptor for rm9200 in order to
> >> prevent file duplication?
> > because the soc are different and can only be compile if the timer is enable
> > and I do not want to enable the rm9200 timer on sam9 so instead of a ifdef i
> > the board-dt create a new board is better as we have a 50 lines file
> >
> > with different board_compat and different machine descriptor
> 
> I am okey with either approach, but I would like to hear what Nicolas
> Ferre has to say since he is the on the one that added board-dt. It
> would be nice to have everything in one board DT file, but I
> understand your concern with the RM9200 timer.
> 
> We will also bump into this again on AT91X40 I guess.
simple on x40 forget about it the x40 is no MMU only SoC

so you can not enable  it by default as the all other at91 are use with MMU

I did the necessary to make the board-dt nearly empty and the same for all the
sam9 but the board-dt is sam9 only and need to be keeped this way

Nico will tell you the same

Best Regards,
J.

^ permalink raw reply

* [RFC] Common clock framework for external clock generators
From: Daniel Mack @ 2012-10-14 11:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <507A9AF6.4030008@gmail.com>

Hi,

On 14.10.2012 12:59, Sebastian Hesselbarth wrote:
> Adding LAKML and devtree as there might be people willing to comment about
> DT representation of i2c-attached clock generators, too.
> 
> On 10/12/2012 08:17 PM, Daniel Mack wrote:
>> On 11.10.2012 18:00, Sebastian Hesselbarth wrote:
>  >> [...]
>>> Does any of you work rely on a working si5351 driver?
>>
>> Yes, it does actually. I can hack around it for now, but at some point,
>> a proper driver is needed. And yours looks quite feature complete, so it
>> would be easiest to finish this one :)
> 
> Well, yes it is some kind of feature complete except regmap and DT. Adding
> regmap to the driver should be quite easy but with DT I am still thinking
> of the best way to represent the internal connections between OSC, PLLs, and
> CLKOUTs. In the last version of the driver I had a callback that was
> board specific to setup these connections but with DT there will be no board
> specific code anymore.
> 
> Maybe one of the common clk maintainers can give a hint how this could be
> done in a clean way. The question is how to represent a i2c-attached clock
> generator config in DT where you want to setup clock parents of CLKOUTs and
> PLLs.
> 
> A possible solution would be something like this:
> 
> si5351a at 60 {
> 	compatible = "silabs,si5351a";
> 	reg = <0x60>;
> 
> 	si_osc: osc {
> 		compatible = "fixed-clock";
> 		clock-frequency = <270000000>;
> 	};
> 
> 	si_plla: pll at 0 {
> 		compatible = "silabs,si5351-pll";
> 		/* hook-up plla to osc input */
> 		clocks = <&si_osc>;
> 	};
> 
> 	si_clkout0: clkout at 0 {
> 		compatible = "silabs,si5351-clkout";
> 		/* hook-up clkout0 to plla */
> 		clocks = <&si_plla>;
> 		/* request target frequency */
> 		clock-frequency = <148500000>;
> 	};	
> };
> 
> Although this perfectly describes the clock hierarchy I still don't like
> the sub-node style. Another flat solution would be something like:

I think the sub-node style above it nicer because it allows referencing
the individual clocks outputs with a phandle. We use this chip to
generate base-frequencies for audio clocks, and so we have to switch
between two freqs for the multiples of 22.5KHz and 24KHz at runtime.

>> Do you still have access to hardware you wrote the driver for? Let me
>> know if you need any help around here.
> 
> Yes, hardware is still available although I only have access to the Si5351a
> with 3 clkouts. The code should be compatible for Si5351a with 8 clkouts and
> code skeleton for 5351b (OSC and VXCO input) and 5351c (OSC and CLKIN) is
> there but untested.

The 3 clkout model is the only one I have access to as well.

> I've transferred the current driver version to my repository to work on. As soon
> as I have regmap done, I will push it online and give you a note.

That's great. Let me know if you want me to test anything.


Thanks,
Daniel

^ permalink raw reply

* [RFC] Common clock framework for external clock generators
From: Sebastian Hesselbarth @ 2012-10-14 10:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <50785EBF.3060508@gmail.com>

Adding LAKML and devtree as there might be people willing to comment about
DT representation of i2c-attached clock generators, too.

On 10/12/2012 08:17 PM, Daniel Mack wrote:
> On 11.10.2012 18:00, Sebastian Hesselbarth wrote:
 >> [...]
>> Does any of you work rely on a working si5351 driver?
>
> Yes, it does actually. I can hack around it for now, but at some point,
> a proper driver is needed. And yours looks quite feature complete, so it
> would be easiest to finish this one :)

Well, yes it is some kind of feature complete except regmap and DT. Adding
regmap to the driver should be quite easy but with DT I am still thinking
of the best way to represent the internal connections between OSC, PLLs, and
CLKOUTs. In the last version of the driver I had a callback that was
board specific to setup these connections but with DT there will be no board
specific code anymore.

Maybe one of the common clk maintainers can give a hint how this could be
done in a clean way. The question is how to represent a i2c-attached clock
generator config in DT where you want to setup clock parents of CLKOUTs and
PLLs.

A possible solution would be something like this:

si5351a at 60 {
	compatible = "silabs,si5351a";
	reg = <0x60>;

	si_osc: osc {
		compatible = "fixed-clock";
		clock-frequency = <270000000>;
	};

	si_plla: pll at 0 {
		compatible = "silabs,si5351-pll";
		/* hook-up plla to osc input */
		clocks = <&si_osc>;
	};

	si_clkout0: clkout at 0 {
		compatible = "silabs,si5351-clkout";
		/* hook-up clkout0 to plla */
		clocks = <&si_plla>;
		/* request target frequency */
		clock-frequency = <148500000>;
	};	
};

Although this perfectly describes the clock hierarchy I still don't like
the sub-node style. Another flat solution would be something like:

si_osc: osc {
	compatible = "fixed-clock";
	clock-frequency = <270000000>;
};

si5351a at 60 {
	compatible = "silabs,si5351a";
	reg = <0x60>;
	clocks = <&si_osc>;

	si5351-pll-config = <0 0   /* pll A to osc */
			     1 0>; /* pll B to osc */

	si5351-clock-config = <0 0 148500000   /* clkout 0 to pll A, 148.5MHz */
			       1 0         0   /* clkout 1 to pll A, disabled */
			       2 1  24000000>; /* clkout 2 to pll B,  24.0Mhz */
};

> Do you still have access to hardware you wrote the driver for? Let me
> know if you need any help around here.

Yes, hardware is still available although I only have access to the Si5351a
with 3 clkouts. The code should be compatible for Si5351a with 8 clkouts and
code skeleton for 5351b (OSC and VXCO input) and 5351c (OSC and CLKIN) is
there but untested.

I've transferred the current driver version to my repository to work on. As soon
as I have regmap done, I will push it online and give you a note.

Sebastian

^ permalink raw reply

* No subject
From: Alexey Dobriyan @ 2012-10-14 10:05 UTC (permalink / raw)
  To: linux-arm-kernel

  http://www.hzsonic.com/en/wp-content/themes/twentyten/career.html

^ permalink raw reply

* [PATCH V2 3/3] ARM: SPEAr13xx: Pass DW DMAC platform data from DT
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-10-14 10:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAKohpokj0Up23YEDTuSOHDgqcCuM3Q21cwnkAz5Ya=JhPKM98A@mail.gmail.com>

On 19:40 Sat 13 Oct     , Viresh Kumar wrote:
> On 13 October 2012 19:38, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > On 13 October 2012 17:52, Jean-Christophe PLAGNIOL-VILLARD
> > <plagnioj@jcrosoft.com> wrote:
> >> On 14:18 Sat 13 Oct     , Viresh Kumar wrote:
> >>>    On Oct 13, 2012 12:16 PM, "Jean-Christophe PLAGNIOL-VILLARD"
> >>>    <plagnioj@jcrosoft.com> wrote:
> >>>    >
> >>>    > On 22:42 Fri 12 Oct     , Viresh Kumar wrote:
> >>>    > > On 12 October 2012 21:51, Jean-Christophe PLAGNIOL-VILLARD
> >>>    > > <plagnioj@jcrosoft.com> wrote:
> >>>    > > >> >> +     OF_DEV_AUXDATA("arasan,cf-spear1340", MCIF_CF_BASE, NULL,
> >>>    "cf"),
> >>>    > > >> > ?/
> >>>    > > >>
> >>>    > > >> Sorry. can't get it :(
> >>>    > > > what is the "cf" as paltfrom data
> >>>    > >
> >>>    > > This is dma bus_id string, which matches with what is passed from dtb.
> >>>    > so pass if via dtb too
> >>>
> >>>    Yes. Already passed in 13xx.dtsi.
> 
> Probably some confusion here. What i meant to say here is, dmac's
> DT slave info has a node for cf and cf driver expects this string to come
> via platform data currently.

so use a phandle to connect them

Best Regards,
J.

^ permalink raw reply

* pm: add suspend_mem and suspend_standby support
From: Rafael J. Wysocki @ 2012-10-14  7:12 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20121010094353.GR12801@game.jcrosoft.org>

[...]

> > > I understand that, there is trade-off between power and latency numbers,
> > > but for certain use-cases latency is more important and it is not
> > > possible without telling driver about the low-power state.
> > 
> > I understand the problem at hand, but the question is who's going to tell
> > the driver about that.
> > 
> > Arguably, the PM core is not the right source of that information, because
> > if a driver is told "we're going into standby", it will have to ask the
> > platform about what "standby" actually means.
> 
> who is going to tell the kernel to go to standby and which
> 
> the sysfs handle via PM core
> 
> so here the one who alwasy have the information is the pm core

The core tells the _platform_ "the user wants us to go into standby" or
"the user wants us to go into suspend", but it doesn't really know what
"standby" or "suspend" actually mean. It can't, because these things are
not universally well defined and there are differences between platforms
in that area.  Different platforms may require different things to be
done to devices for "standby" and so drivers should not assume that
"standby" will always mean the same thing.

For example, if your driver is written for and ARM SoC and then the same
IP block happens to be used on an x86-based thing, whatever the driver did
and was specific to ARM is no longer valid.  So if your ARM SoC required
drivers to do something special during a "suspend" transition which wasn't
necessary to do during a "standby" transition, it doesn't necessarily mean that
the new platform will do so as well. The "mem" and "standby" things may have
different meanings for the new platform and for this reason drivers _have_ _to_
ask the _platform_ what states to put the devices into _or_, alternatively,
leave the platform-specific handling of the devices to the platform (that's
what device drivers on ACPI-based x86 do).  They should not expect the PM core
to tell them that, simply because the PM core has no idea what that may be.

And by the way, ACPI-based ARM platforms are reportedly in the works and they
_will_ use the same IP blocks that are already in use on ARM.  On those
platforms drivers will be supposed to use ACPI for power management and
the whole design you're proposing will not make sense any more for them.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox