Mirror of the lttng-dev mailing list
 help / color / mirror / Atom feed
* [ltt-dev] [PATCH V1] ARM: Set bit 0 for thumb mode in kallsyms_lookup_name returned address
@ 2011-09-19  9:00 Avik Sil
  2011-09-19 14:57 ` Mathieu Desnoyers
  2011-09-19 15:10 ` Dave Martin
  0 siblings, 2 replies; 4+ messages in thread
From: Avik Sil @ 2011-09-19  9:00 UTC (permalink / raw)


This patch fixes the undefined instruction oops due to execution
of thumb-2 code in ARM mode. The zero bit in the symbol address
returned by kallsyms_lookup_name is not set, leading to switching
to ARM mode that generates oops while executing thumb-2 code. For
detailed discussion, see [1].
[1] http://lists.casi.polymtl.ca/pipermail/ltt-dev/2011-September/005176.html

v1:
	- include wrapper function kallsyms_lookup_funcptr as suggested
	by Dave Martin

Signed-off-by: Avik Sil <avik.sil at linaro.org>
---
 lttng-context-prio.c |    3 ++-
 wrapper/ftrace.h     |    5 +++--
 wrapper/kallsyms.h   |   28 ++++++++++++++++++++++++++++
 wrapper/splice.c     |    3 ++-
 wrapper/vmalloc.h    |    3 ++-
 5 files changed, 37 insertions(+), 5 deletions(-)
 create mode 100644 wrapper/kallsyms.h

diff --git a/lttng-context-prio.c b/lttng-context-prio.c
index ad1c42f..1ee3a54 100644
--- a/lttng-context-prio.c
+++ b/lttng-context-prio.c
@@ -13,6 +13,7 @@
 #include "ltt-events.h"
 #include "wrapper/ringbuffer/frontend_types.h"
 #include "wrapper/vmalloc.h"
+#include "wrapper/kallsyms.h"
 #include "ltt-tracer.h"
 
 static
@@ -20,7 +21,7 @@ int (*wrapper_task_prio_sym)(struct task_struct *t);
 
 int wrapper_task_prio_init(void)
 {
-	wrapper_task_prio_sym = (void *) kallsyms_lookup_name("task_prio");
+	wrapper_task_prio_sym = (void *) kallsyms_lookup_funcptr("task_prio");
 	if (!wrapper_task_prio_sym) {
 		printk(KERN_WARNING "LTTng: task_prio symbol lookup failed.\n");
 		return -EINVAL;
diff --git a/wrapper/ftrace.h b/wrapper/ftrace.h
index 9c18cc5..ace33c5 100644
--- a/wrapper/ftrace.h
+++ b/wrapper/ftrace.h
@@ -16,6 +16,7 @@
 #ifdef CONFIG_KALLSYMS
 
 #include <linux/kallsyms.h>
+#include "kallsyms.h"
 
 static inline
 int wrapper_register_ftrace_function_probe(char *glob,
@@ -24,7 +25,7 @@ int wrapper_register_ftrace_function_probe(char *glob,
 	int (*register_ftrace_function_probe_sym)(char *glob,
 			struct ftrace_probe_ops *ops, void *data);
 
-	register_ftrace_function_probe_sym = (void *) kallsyms_lookup_name("register_ftrace_function_probe");
+	register_ftrace_function_probe_sym = (void *) kallsyms_lookup_funcptr("register_ftrace_function_probe");
 	if (register_ftrace_function_probe_sym) {
 		return register_ftrace_function_probe_sym(glob, ops, data);
 	} else {
@@ -40,7 +41,7 @@ void wrapper_unregister_ftrace_function_probe(char *glob,
 	void (*unregister_ftrace_function_probe_sym)(char *glob,
 			struct ftrace_probe_ops *ops, void *data);
 
-	unregister_ftrace_function_probe_sym = (void *) kallsyms_lookup_name("unregister_ftrace_function_probe");
+	unregister_ftrace_function_probe_sym = (void *) kallsyms_lookup_funcptr("unregister_ftrace_function_probe");
 	if (unregister_ftrace_function_probe_sym) {
 		unregister_ftrace_function_probe_sym(glob, ops, data);
 	} else {
diff --git a/wrapper/kallsyms.h b/wrapper/kallsyms.h
new file mode 100644
index 0000000..bb45f38
--- /dev/null
+++ b/wrapper/kallsyms.h
@@ -0,0 +1,28 @@
+#ifndef _LTT_WRAPPER_KALLSYMS_H
+#define _LTT_WRAPPER_KALLSYMS_H
+
+/*
+ * Copyright (C) 2011 Avik Sil (avik.sil at linaro.org)
+ *
+ * wrapper around kallsyms_lookup_name. Implements arch-dependent code for
+ * arches where the address of the start of the function body is different
+ * from the pointer which can be used to call the function, e.g. ARM THUMB2.
+ *
+ * Dual LGPL v2.1/GPL v2 license.
+ */
+
+static inline
+unsigned long kallsyms_lookup_funcptr(const char *name)
+{
+	unsigned long addr;
+
+	addr = kallsyms_lookup_name(name);
+#ifdef CONFIG_ARM
+#ifdef CONFIG_THUMB2_KERNEL
+	if (addr)
+		addr |= 1; /* set bit 0 in address for thumb mode */
+#endif
+#endif
+	return addr;
+}
+#endif /* _LTT_WRAPPER_KALLSYMS_H */
diff --git a/wrapper/splice.c b/wrapper/splice.c
index edc499c..ba224ee 100644
--- a/wrapper/splice.c
+++ b/wrapper/splice.c
@@ -13,6 +13,7 @@
 #include <linux/kallsyms.h>
 #include <linux/fs.h>
 #include <linux/splice.h>
+#include "kallsyms.h"
 
 static
 ssize_t (*splice_to_pipe_sym)(struct pipe_inode_info *pipe,
@@ -22,7 +23,7 @@ ssize_t wrapper_splice_to_pipe(struct pipe_inode_info *pipe,
 			       struct splice_pipe_desc *spd)
 {
 	if (!splice_to_pipe_sym)
-		splice_to_pipe_sym = (void *) kallsyms_lookup_name("splice_to_pipe");
+		splice_to_pipe_sym = (void *) kallsyms_lookup_funcptr("splice_to_pipe"); 
 	if (splice_to_pipe_sym) {
 		return splice_to_pipe_sym(pipe, spd);
 	} else {
diff --git a/wrapper/vmalloc.h b/wrapper/vmalloc.h
index 7d87855..765f2ad 100644
--- a/wrapper/vmalloc.h
+++ b/wrapper/vmalloc.h
@@ -14,13 +14,14 @@
 #ifdef CONFIG_KALLSYMS
 
 #include <linux/kallsyms.h>
+#include "kallsyms.h"
 
 static inline
 void wrapper_vmalloc_sync_all(void)
 {
 	void (*vmalloc_sync_all_sym)(void);
 
-	vmalloc_sync_all_sym = (void *) kallsyms_lookup_name("vmalloc_sync_all");
+	vmalloc_sync_all_sym = (void *) kallsyms_lookup_funcptr("vmalloc_sync_all");
 	if (vmalloc_sync_all_sym) {
 		vmalloc_sync_all_sym();
 	} else {
-- 
1.7.0.4





^ permalink raw reply	[flat|nested] 4+ messages in thread

* [ltt-dev] [PATCH V1] ARM: Set bit 0 for thumb mode in kallsyms_lookup_name returned address
  2011-09-19  9:00 [ltt-dev] [PATCH V1] ARM: Set bit 0 for thumb mode in kallsyms_lookup_name returned address Avik Sil
@ 2011-09-19 14:57 ` Mathieu Desnoyers
  2011-09-19 15:10 ` Dave Martin
  1 sibling, 0 replies; 4+ messages in thread
From: Mathieu Desnoyers @ 2011-09-19 14:57 UTC (permalink / raw)


* Avik Sil (avik.sil at linaro.org) wrote:
> This patch fixes the undefined instruction oops due to execution
> of thumb-2 code in ARM mode. The zero bit in the symbol address
> returned by kallsyms_lookup_name is not set, leading to switching
> to ARM mode that generates oops while executing thumb-2 code. For
> detailed discussion, see [1].
> [1] http://lists.casi.polymtl.ca/pipermail/ltt-dev/2011-September/005176.html
> 
> v1:
> 	- include wrapper function kallsyms_lookup_funcptr as suggested
> 	by Dave Martin
> 
> Signed-off-by: Avik Sil <avik.sil at linaro.org>

Merged, thanks!

Mathieu

> ---
>  lttng-context-prio.c |    3 ++-
>  wrapper/ftrace.h     |    5 +++--
>  wrapper/kallsyms.h   |   28 ++++++++++++++++++++++++++++
>  wrapper/splice.c     |    3 ++-
>  wrapper/vmalloc.h    |    3 ++-
>  5 files changed, 37 insertions(+), 5 deletions(-)
>  create mode 100644 wrapper/kallsyms.h
> 
> diff --git a/lttng-context-prio.c b/lttng-context-prio.c
> index ad1c42f..1ee3a54 100644
> --- a/lttng-context-prio.c
> +++ b/lttng-context-prio.c
> @@ -13,6 +13,7 @@
>  #include "ltt-events.h"
>  #include "wrapper/ringbuffer/frontend_types.h"
>  #include "wrapper/vmalloc.h"
> +#include "wrapper/kallsyms.h"
>  #include "ltt-tracer.h"
>  
>  static
> @@ -20,7 +21,7 @@ int (*wrapper_task_prio_sym)(struct task_struct *t);
>  
>  int wrapper_task_prio_init(void)
>  {
> -	wrapper_task_prio_sym = (void *) kallsyms_lookup_name("task_prio");
> +	wrapper_task_prio_sym = (void *) kallsyms_lookup_funcptr("task_prio");
>  	if (!wrapper_task_prio_sym) {
>  		printk(KERN_WARNING "LTTng: task_prio symbol lookup failed.\n");
>  		return -EINVAL;
> diff --git a/wrapper/ftrace.h b/wrapper/ftrace.h
> index 9c18cc5..ace33c5 100644
> --- a/wrapper/ftrace.h
> +++ b/wrapper/ftrace.h
> @@ -16,6 +16,7 @@
>  #ifdef CONFIG_KALLSYMS
>  
>  #include <linux/kallsyms.h>
> +#include "kallsyms.h"
>  
>  static inline
>  int wrapper_register_ftrace_function_probe(char *glob,
> @@ -24,7 +25,7 @@ int wrapper_register_ftrace_function_probe(char *glob,
>  	int (*register_ftrace_function_probe_sym)(char *glob,
>  			struct ftrace_probe_ops *ops, void *data);
>  
> -	register_ftrace_function_probe_sym = (void *) kallsyms_lookup_name("register_ftrace_function_probe");
> +	register_ftrace_function_probe_sym = (void *) kallsyms_lookup_funcptr("register_ftrace_function_probe");
>  	if (register_ftrace_function_probe_sym) {
>  		return register_ftrace_function_probe_sym(glob, ops, data);
>  	} else {
> @@ -40,7 +41,7 @@ void wrapper_unregister_ftrace_function_probe(char *glob,
>  	void (*unregister_ftrace_function_probe_sym)(char *glob,
>  			struct ftrace_probe_ops *ops, void *data);
>  
> -	unregister_ftrace_function_probe_sym = (void *) kallsyms_lookup_name("unregister_ftrace_function_probe");
> +	unregister_ftrace_function_probe_sym = (void *) kallsyms_lookup_funcptr("unregister_ftrace_function_probe");
>  	if (unregister_ftrace_function_probe_sym) {
>  		unregister_ftrace_function_probe_sym(glob, ops, data);
>  	} else {
> diff --git a/wrapper/kallsyms.h b/wrapper/kallsyms.h
> new file mode 100644
> index 0000000..bb45f38
> --- /dev/null
> +++ b/wrapper/kallsyms.h
> @@ -0,0 +1,28 @@
> +#ifndef _LTT_WRAPPER_KALLSYMS_H
> +#define _LTT_WRAPPER_KALLSYMS_H
> +
> +/*
> + * Copyright (C) 2011 Avik Sil (avik.sil at linaro.org)
> + *
> + * wrapper around kallsyms_lookup_name. Implements arch-dependent code for
> + * arches where the address of the start of the function body is different
> + * from the pointer which can be used to call the function, e.g. ARM THUMB2.
> + *
> + * Dual LGPL v2.1/GPL v2 license.
> + */
> +
> +static inline
> +unsigned long kallsyms_lookup_funcptr(const char *name)
> +{
> +	unsigned long addr;
> +
> +	addr = kallsyms_lookup_name(name);
> +#ifdef CONFIG_ARM
> +#ifdef CONFIG_THUMB2_KERNEL
> +	if (addr)
> +		addr |= 1; /* set bit 0 in address for thumb mode */
> +#endif
> +#endif
> +	return addr;
> +}
> +#endif /* _LTT_WRAPPER_KALLSYMS_H */
> diff --git a/wrapper/splice.c b/wrapper/splice.c
> index edc499c..ba224ee 100644
> --- a/wrapper/splice.c
> +++ b/wrapper/splice.c
> @@ -13,6 +13,7 @@
>  #include <linux/kallsyms.h>
>  #include <linux/fs.h>
>  #include <linux/splice.h>
> +#include "kallsyms.h"
>  
>  static
>  ssize_t (*splice_to_pipe_sym)(struct pipe_inode_info *pipe,
> @@ -22,7 +23,7 @@ ssize_t wrapper_splice_to_pipe(struct pipe_inode_info *pipe,
>  			       struct splice_pipe_desc *spd)
>  {
>  	if (!splice_to_pipe_sym)
> -		splice_to_pipe_sym = (void *) kallsyms_lookup_name("splice_to_pipe");
> +		splice_to_pipe_sym = (void *) kallsyms_lookup_funcptr("splice_to_pipe"); 
>  	if (splice_to_pipe_sym) {
>  		return splice_to_pipe_sym(pipe, spd);
>  	} else {
> diff --git a/wrapper/vmalloc.h b/wrapper/vmalloc.h
> index 7d87855..765f2ad 100644
> --- a/wrapper/vmalloc.h
> +++ b/wrapper/vmalloc.h
> @@ -14,13 +14,14 @@
>  #ifdef CONFIG_KALLSYMS
>  
>  #include <linux/kallsyms.h>
> +#include "kallsyms.h"
>  
>  static inline
>  void wrapper_vmalloc_sync_all(void)
>  {
>  	void (*vmalloc_sync_all_sym)(void);
>  
> -	vmalloc_sync_all_sym = (void *) kallsyms_lookup_name("vmalloc_sync_all");
> +	vmalloc_sync_all_sym = (void *) kallsyms_lookup_funcptr("vmalloc_sync_all");
>  	if (vmalloc_sync_all_sym) {
>  		vmalloc_sync_all_sym();
>  	} else {
> -- 
> 1.7.0.4
> 
> 
> _______________________________________________
> ltt-dev mailing list
> ltt-dev at lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com




^ permalink raw reply	[flat|nested] 4+ messages in thread

* [ltt-dev] [PATCH V1] ARM: Set bit 0 for thumb mode in kallsyms_lookup_name returned address
  2011-09-19  9:00 [ltt-dev] [PATCH V1] ARM: Set bit 0 for thumb mode in kallsyms_lookup_name returned address Avik Sil
  2011-09-19 14:57 ` Mathieu Desnoyers
@ 2011-09-19 15:10 ` Dave Martin
  2011-09-20  7:24   ` Jon Medhurst (Tixy)
  1 sibling, 1 reply; 4+ messages in thread
From: Dave Martin @ 2011-09-19 15:10 UTC (permalink / raw)


On Mon, Sep 19, 2011 at 10:00 AM, Avik Sil <avik.sil at linaro.org> wrote:
> This patch fixes the undefined instruction oops due to execution
> of thumb-2 code in ARM mode. The zero bit in the symbol address
> returned by kallsyms_lookup_name is not set, leading to switching
> to ARM mode that generates oops while executing thumb-2 code. For
> detailed discussion, see [1].
> [1] http://lists.casi.polymtl.ca/pipermail/ltt-dev/2011-September/005176.html
>
> v1:
> ? ? ? ?- include wrapper function kallsyms_lookup_funcptr as suggested
> ? ? ? ?by Dave Martin
>
> Signed-off-by: Avik Sil <avik.sil at linaro.org>

Looks reasonable.

Tixy, could it make sense for that definition of
kallsyms_lookup_funcptr to migrate into the kernel headers?  I had the
impression that you might also have used this in some places, if it
had been available.

Cheers
---Dave

> ---
> ?lttng-context-prio.c | ? ?3 ++-
> ?wrapper/ftrace.h ? ? | ? ?5 +++--
> ?wrapper/kallsyms.h ? | ? 28 ++++++++++++++++++++++++++++
> ?wrapper/splice.c ? ? | ? ?3 ++-
> ?wrapper/vmalloc.h ? ?| ? ?3 ++-
> ?5 files changed, 37 insertions(+), 5 deletions(-)
> ?create mode 100644 wrapper/kallsyms.h
>
> diff --git a/lttng-context-prio.c b/lttng-context-prio.c
> index ad1c42f..1ee3a54 100644
> --- a/lttng-context-prio.c
> +++ b/lttng-context-prio.c
> @@ -13,6 +13,7 @@
> ?#include "ltt-events.h"
> ?#include "wrapper/ringbuffer/frontend_types.h"
> ?#include "wrapper/vmalloc.h"
> +#include "wrapper/kallsyms.h"
> ?#include "ltt-tracer.h"
>
> ?static
> @@ -20,7 +21,7 @@ int (*wrapper_task_prio_sym)(struct task_struct *t);
>
> ?int wrapper_task_prio_init(void)
> ?{
> - ? ? ? wrapper_task_prio_sym = (void *) kallsyms_lookup_name("task_prio");
> + ? ? ? wrapper_task_prio_sym = (void *) kallsyms_lookup_funcptr("task_prio");
> ? ? ? ?if (!wrapper_task_prio_sym) {
> ? ? ? ? ? ? ? ?printk(KERN_WARNING "LTTng: task_prio symbol lookup failed.\n");
> ? ? ? ? ? ? ? ?return -EINVAL;
> diff --git a/wrapper/ftrace.h b/wrapper/ftrace.h
> index 9c18cc5..ace33c5 100644
> --- a/wrapper/ftrace.h
> +++ b/wrapper/ftrace.h
> @@ -16,6 +16,7 @@
> ?#ifdef CONFIG_KALLSYMS
>
> ?#include <linux/kallsyms.h>
> +#include "kallsyms.h"
>
> ?static inline
> ?int wrapper_register_ftrace_function_probe(char *glob,
> @@ -24,7 +25,7 @@ int wrapper_register_ftrace_function_probe(char *glob,
> ? ? ? ?int (*register_ftrace_function_probe_sym)(char *glob,
> ? ? ? ? ? ? ? ? ? ? ? ?struct ftrace_probe_ops *ops, void *data);
>
> - ? ? ? register_ftrace_function_probe_sym = (void *) kallsyms_lookup_name("register_ftrace_function_probe");
> + ? ? ? register_ftrace_function_probe_sym = (void *) kallsyms_lookup_funcptr("register_ftrace_function_probe");
> ? ? ? ?if (register_ftrace_function_probe_sym) {
> ? ? ? ? ? ? ? ?return register_ftrace_function_probe_sym(glob, ops, data);
> ? ? ? ?} else {
> @@ -40,7 +41,7 @@ void wrapper_unregister_ftrace_function_probe(char *glob,
> ? ? ? ?void (*unregister_ftrace_function_probe_sym)(char *glob,
> ? ? ? ? ? ? ? ? ? ? ? ?struct ftrace_probe_ops *ops, void *data);
>
> - ? ? ? unregister_ftrace_function_probe_sym = (void *) kallsyms_lookup_name("unregister_ftrace_function_probe");
> + ? ? ? unregister_ftrace_function_probe_sym = (void *) kallsyms_lookup_funcptr("unregister_ftrace_function_probe");
> ? ? ? ?if (unregister_ftrace_function_probe_sym) {
> ? ? ? ? ? ? ? ?unregister_ftrace_function_probe_sym(glob, ops, data);
> ? ? ? ?} else {
> diff --git a/wrapper/kallsyms.h b/wrapper/kallsyms.h
> new file mode 100644
> index 0000000..bb45f38
> --- /dev/null
> +++ b/wrapper/kallsyms.h
> @@ -0,0 +1,28 @@
> +#ifndef _LTT_WRAPPER_KALLSYMS_H
> +#define _LTT_WRAPPER_KALLSYMS_H
> +
> +/*
> + * Copyright (C) 2011 Avik Sil (avik.sil at linaro.org)
> + *
> + * wrapper around kallsyms_lookup_name. Implements arch-dependent code for
> + * arches where the address of the start of the function body is different
> + * from the pointer which can be used to call the function, e.g. ARM THUMB2.
> + *
> + * Dual LGPL v2.1/GPL v2 license.
> + */
> +
> +static inline
> +unsigned long kallsyms_lookup_funcptr(const char *name)
> +{
> + ? ? ? unsigned long addr;
> +
> + ? ? ? addr = kallsyms_lookup_name(name);
> +#ifdef CONFIG_ARM
> +#ifdef CONFIG_THUMB2_KERNEL
> + ? ? ? if (addr)
> + ? ? ? ? ? ? ? addr |= 1; /* set bit 0 in address for thumb mode */
> +#endif
> +#endif
> + ? ? ? return addr;
> +}
> +#endif /* _LTT_WRAPPER_KALLSYMS_H */
> diff --git a/wrapper/splice.c b/wrapper/splice.c
> index edc499c..ba224ee 100644
> --- a/wrapper/splice.c
> +++ b/wrapper/splice.c
> @@ -13,6 +13,7 @@
> ?#include <linux/kallsyms.h>
> ?#include <linux/fs.h>
> ?#include <linux/splice.h>
> +#include "kallsyms.h"
>
> ?static
> ?ssize_t (*splice_to_pipe_sym)(struct pipe_inode_info *pipe,
> @@ -22,7 +23,7 @@ ssize_t wrapper_splice_to_pipe(struct pipe_inode_info *pipe,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct splice_pipe_desc *spd)
> ?{
> ? ? ? ?if (!splice_to_pipe_sym)
> - ? ? ? ? ? ? ? splice_to_pipe_sym = (void *) kallsyms_lookup_name("splice_to_pipe");
> + ? ? ? ? ? ? ? splice_to_pipe_sym = (void *) kallsyms_lookup_funcptr("splice_to_pipe");
> ? ? ? ?if (splice_to_pipe_sym) {
> ? ? ? ? ? ? ? ?return splice_to_pipe_sym(pipe, spd);
> ? ? ? ?} else {
> diff --git a/wrapper/vmalloc.h b/wrapper/vmalloc.h
> index 7d87855..765f2ad 100644
> --- a/wrapper/vmalloc.h
> +++ b/wrapper/vmalloc.h
> @@ -14,13 +14,14 @@
> ?#ifdef CONFIG_KALLSYMS
>
> ?#include <linux/kallsyms.h>
> +#include "kallsyms.h"
>
> ?static inline
> ?void wrapper_vmalloc_sync_all(void)
> ?{
> ? ? ? ?void (*vmalloc_sync_all_sym)(void);
>
> - ? ? ? vmalloc_sync_all_sym = (void *) kallsyms_lookup_name("vmalloc_sync_all");
> + ? ? ? vmalloc_sync_all_sym = (void *) kallsyms_lookup_funcptr("vmalloc_sync_all");
> ? ? ? ?if (vmalloc_sync_all_sym) {
> ? ? ? ? ? ? ? ?vmalloc_sync_all_sym();
> ? ? ? ?} else {
> --
> 1.7.0.4
>
>



-- 
Dave Martin <dave.martin at linaro.org>
Linaro Kernel Working Group

--
http://www.linaro.org/ -- Open source software for ARM SoCs

http://www.facebook.com/pages/Linaro
http://twitter.com/#!/linaroorg
http://www.linaro.org/linaro-blog/




^ permalink raw reply	[flat|nested] 4+ messages in thread

* [ltt-dev] [PATCH V1] ARM: Set bit 0 for thumb mode in kallsyms_lookup_name returned address
  2011-09-19 15:10 ` Dave Martin
@ 2011-09-20  7:24   ` Jon Medhurst (Tixy)
  0 siblings, 0 replies; 4+ messages in thread
From: Jon Medhurst (Tixy) @ 2011-09-20  7:24 UTC (permalink / raw)


On Mon, 2011-09-19 at 16:10 +0100, Dave Martin wrote:
> On Mon, Sep 19, 2011 at 10:00 AM, Avik Sil <avik.sil at linaro.org> wrote:
> > This patch fixes the undefined instruction oops due to execution
> > of thumb-2 code in ARM mode. The zero bit in the symbol address
> > returned by kallsyms_lookup_name is not set, leading to switching
> > to ARM mode that generates oops while executing thumb-2 code. For
> > detailed discussion, see [1].
> > [1] http://lists.casi.polymtl.ca/pipermail/ltt-dev/2011-September/005176.html
> >
> > v1:
> >        - include wrapper function kallsyms_lookup_funcptr as suggested
> >        by Dave Martin
> >
> > Signed-off-by: Avik Sil <avik.sil at linaro.org>
> 
> Looks reasonable.
> 
> Tixy, could it make sense for that definition of
> kallsyms_lookup_funcptr to migrate into the kernel headers?  I had the
> impression that you might also have used this in some places, if it
> had been available.
> 

I don't think so, the problems with kprobes are a bit more involved. The
only candidate for using this new function is in kprobes.c ...

/*
 * Some oddball architectures like 64bit powerpc have function descriptors
 * so this must be overridable.
 */
#ifndef kprobe_lookup_name
#define kprobe_lookup_name(name, addr) \
	addr = ((kprobe_opcode_t *)(kallsyms_lookup_name(name)))
#endif

Replacing the kallsyms_lookup_name() here would make kprobes 'better'
for Thumb (avoid a small runtime overhead and some inconsistencies). But
I don't think that the semantics of the new function match this use, and
if it were desirable, we could overload the above kprobes function from
ARM arch code anyway. In fact, I think I'll propose such a patch...

-- 
Tixy








^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2011-09-20  7:24 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-19  9:00 [ltt-dev] [PATCH V1] ARM: Set bit 0 for thumb mode in kallsyms_lookup_name returned address Avik Sil
2011-09-19 14:57 ` Mathieu Desnoyers
2011-09-19 15:10 ` Dave Martin
2011-09-20  7:24   ` Jon Medhurst (Tixy)

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